fix(security): chặn mở khoá Sandbox Security bằng ô trống
DEFAULT_CONFIG ship agent_security.sandbox_pw = "" kể từ commit3827552, và cấu hình đưa tới dialog luôn được deep-merge với defaults đó. Nghĩa là trên mọi bản cài không đặt COWORK_SANDBOX_PASSWORD, mật khẩu đã lưu là chuỗi rỗng — và phép so `pw == self._sandbox_pw` nhận luôn ô nhập trống. Bấm Unlock với ô trống là mở được toàn bộ nhóm Sandbox Security. Commit3827552chỉ sửa config.py nên bỏ sót bản sao thứ hai của literal nằm trong ui/settings_dialog.py, và chính nó tạo ra lỗ hổng rỗng này. Sửa: - gỡ literal credential khỏi mã nguồn (nó vốn là code chết: deep-merge làm tham số mặc định của .get() không bao giờ chạy); - chặn rỗng trước khi so, theo đúng mẫu mà MS365 unlock đã dùng; - so sánh timing-safe trên BYTES, không trên str — secrets.compare_digest ném TypeError với str ngoài ASCII, mà app mặc định tiếng Việt và phục vụ khách Nhật nên mật khẩu có dấu là input bình thường; - chưa đặt mật khẩu thì báo đúng trạng thái đó, không báo "sai mật khẩu" — người dùng sẽ gõ lại mãi một thứ không tồn tại. Root cause: ui/settings_dialog.py:104 (bản cũ) Test: tests/ui/test_sandbox_unlock_security.py LƯU Ý CHO REVIEWER: literal cũ vẫn nằm trong 6 commit của Git history. Gỡ ở đây không gỡ khỏi lịch sử — cần xoay credential trên các máy còn giá trị đó trong config.json. Không rewrite history (SECURITY.md). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
+43
-7
@@ -13,6 +13,8 @@ chưa từng được gán nên gọi vào là AttributeError.
|
||||
"""
|
||||
from __future__ import annotations
|
||||
|
||||
import secrets
|
||||
|
||||
from PySide6.QtCore import Qt
|
||||
from PySide6.QtGui import QGuiApplication
|
||||
from PySide6.QtWidgets import (
|
||||
@@ -31,6 +33,27 @@ from ..presentation.settings.general_settings_widget import GeneralSettingsWidge
|
||||
from ..presentation.settings.provider_settings_widget import ProviderSettingsWidget
|
||||
from ..presentation.settings.parameter_settings_widget import ParameterSettingsWidget
|
||||
from ..presentation.settings.routing_settings_widget import RoutingSettingsWidget
|
||||
from ..presentation.settings.about_widget import AboutSettingsWidget
|
||||
|
||||
|
||||
def _sandbox_password_matches(entered: str, stored: str) -> bool:
|
||||
"""Whether ``entered`` unlocks the Sandbox Security group.
|
||||
|
||||
An empty ``stored`` must never match. ``DEFAULT_CONFIG`` ships
|
||||
``agent_security.sandbox_pw = ""`` and the config handed to this dialog is
|
||||
always deep-merged with those defaults, so a plain ``entered == stored``
|
||||
accepts an empty field on every install that never set a password. The MS365
|
||||
unlock guards the same way — see ``json_config_repository.unlock_ms365``.
|
||||
|
||||
Both sides are compared as UTF-8 bytes, not as ``str``:
|
||||
``compare_digest`` raises ``TypeError`` on ``str`` holding anything outside
|
||||
ASCII, and this app defaults to Vietnamese and ships to Japanese customers,
|
||||
so an accented password is ordinary input rather than an edge case.
|
||||
"""
|
||||
if not entered or not stored:
|
||||
return False
|
||||
return secrets.compare_digest(entered.encode("utf-8"), stored.encode("utf-8"))
|
||||
|
||||
|
||||
class SettingsDialog(QDialog):
|
||||
"""Hộp thoại Cài đặt: cột mục lục bên trái, các trang bên phải
|
||||
@@ -101,7 +124,7 @@ class SettingsDialog(QDialog):
|
||||
pw_row.addWidget(self.sandbox_locked_status)
|
||||
sbl.addLayout(pw_row)
|
||||
self._sandbox_unlocked = False # Start LOCKED — must enter password first
|
||||
self._sandbox_pw = sec.get("sandbox_pw", "quandh14")
|
||||
self._sandbox_pw = sec.get("sandbox_pw", "")
|
||||
|
||||
# Separator line between pw section and sandbox settings
|
||||
pw_sep = QLabel("────────────────")
|
||||
@@ -122,15 +145,15 @@ class SettingsDialog(QDialog):
|
||||
# they belong with the other tool toggles — see ToolsAdminTab).
|
||||
|
||||
# --- Enable/Disable Agent Security ---
|
||||
self.sec_enabled = ToggleSwitch("Enable Agent Security (command validation)")
|
||||
self.sec_enabled = ToggleSwitch(tr("settings.sec_enabled"))
|
||||
self.sec_enabled.setChecked(bool(sec.get("enabled", True)))
|
||||
self.sec_enabled.setToolTip("Bật/tắt toàn bộ Agent Security")
|
||||
self.sec_enabled.setToolTip(tr("settings.sec_enabled_tooltip"))
|
||||
sbl.addWidget(self.sec_enabled)
|
||||
|
||||
# --- AI Command Check toggle ---
|
||||
self.ai_check = ToggleSwitch("AI check commands")
|
||||
self.ai_check = ToggleSwitch(tr("settings.ai_check"))
|
||||
self.ai_check.setChecked(bool(sec.get("command_ai_check", False)))
|
||||
self.ai_check.setToolTip("Cho AI control-agent xét lệnh trước khi chạy")
|
||||
self.ai_check.setToolTip(tr("settings.ai_check_tooltip"))
|
||||
sbl.addWidget(self.ai_check)
|
||||
|
||||
# Resource limits (CPU/Memory/Disk I/O) moved to the Parameter group
|
||||
@@ -169,12 +192,18 @@ class SettingsDialog(QDialog):
|
||||
from .widgets import section_panels
|
||||
|
||||
|
||||
# Giới thiệu đứng CUỐI: nó không có thiết lập nào để đổi, nên đặt trước
|
||||
# các mục thao tác được sẽ đẩy chúng xuống mà không được gì.
|
||||
self._about_page = AboutSettingsWidget(self.ctx)
|
||||
root.addWidget(self._about_page)
|
||||
|
||||
pages = []
|
||||
for label, widget in ((tr("settings.group.general"), self._general_box),
|
||||
(tr("settings.group.provider"), prov_group),
|
||||
(tr("settings.group.sandbox"), self.sandbox_group),
|
||||
(tr("settings.group.parameter"), param_group),
|
||||
(tr("routing.settings_group"), routing_group)):
|
||||
(tr("routing.settings_group"), routing_group),
|
||||
(tr("settings.group.about"), self._about_page)):
|
||||
root.removeWidget(widget)
|
||||
page = QWidget()
|
||||
pv = QVBoxLayout(page)
|
||||
@@ -276,7 +305,14 @@ class SettingsDialog(QDialog):
|
||||
phải cơ chế bảo mật thật.
|
||||
"""
|
||||
pw = self.sandbox_pw_edit.text()
|
||||
if pw == self._sandbox_pw:
|
||||
if not self._sandbox_pw:
|
||||
# No password configured. Refusing with "wrong password" would be a
|
||||
# dead end — the user would keep retrying a password that cannot
|
||||
# exist — so name the actual state instead.
|
||||
QMessageBox.warning(self, tr("settings.sandbox_pw_unset_title"),
|
||||
tr("settings.sandbox_pw_unset_body"))
|
||||
return
|
||||
if _sandbox_password_matches(pw, self._sandbox_pw):
|
||||
self._sandbox_unlocked = True
|
||||
self.sandbox_locked_status.setText("Unlocked")
|
||||
self.sandbox_locked_status.set_icon("unlock", "#090")
|
||||
|
||||
Reference in New Issue
Block a user