# Secret & Config — nơi credential được phép nằm Nguồn: `infrastructure/secrets/secret_store.py`, `infrastructure/secrets/keyring_adapter.py`, `infrastructure/config/schema_migration.py`, `config.py`, `SECURITY.md`. Đây là knowledge module của `security-defect-fixer`. Ba module UI (`theme_tokens`, `i18n_rules`, `screen_map`) không đụng tới phần này. --- ## 1. Thang bậc: credential được phép nằm ở đâu Từ an toàn nhất xuống: | Bậc | Nơi | Dùng cho | API | |---|---|---|---| | 1 | **OS Keyring** qua `SecretStore` | API key, token, mật khẩu thật | `secrets.set/get/has/delete` | | 2 | **Biến môi trường** | Giá trị do quản trị viên đặt lúc triển khai | `_apply_env_overrides` | | 3 | **`config.json`** | Cấu hình **không bí mật** | `ctx.config.` | | 4 | **Hằng số trong mã nguồn** | ❌ Không bao giờ cho credential | — | Bậc 4 là lỗi bị Gate A bắt, và tệ hơn: nó đi vào Git history vĩnh viễn. ## 2. `SecretStore` — interface, không phải hàm tiện ích ```python # infrastructure/secrets/secret_store.py @runtime_checkable class SecretStore(Protocol): def get(self, key: str) -> str | None: ... # thiếu key KHÔNG được ném lỗi def set(self, key: str, value: str) -> None: ... def delete(self, key: str) -> None: ... # không có sẵn thì im lặng def has(self, key: str) -> bool: ... # kiểm tra mà không đọc giá trị ra def provider_key(name: str) -> str: return f"provider:{name}" # quy ước đặt key ``` Lý do là Protocol chứ không phải hàm: bản thật gọi OS Keyring — chậm, có thể ném lỗi, và **test không được đụng keyring máy thật**. Có interface thì test tiêm `FakeSecretStore`. Bản thật: `KeyringAdapter`, `SERVICE = "cowork-local"`, có property `available`. **Luật khi thêm secret mới:** - Đặt key theo quy ước có sẵn, không tự nghĩ kiểu mới. Chưa có quy ước cho loại của bạn → thêm một hàm `*_key()` cạnh `provider_key`, đừng rải chuỗi literal khắp nơi. - Màn Settings hiển thị trạng thái bằng `has()`, **không** bằng `get()`. Không đọc giá trị bí mật ra chỉ để vẽ dấu tích. - `KeyringAdapter.available` là False (Linux thiếu backend, CI) → phải có đường thoái lui không làm hỏng app. ## 3. Schema migration — cách đổi hình dạng config an toàn ```python # infrastructure/config/schema_migration.py CURRENT_VERSION = 2 ASSUMED_VERSION = 1 # file thiếu schema_version ⇒ coi là 1 STEPS = {1: _v1_to_v2} # mỗi bước v(n) → v(n+1), chạy tuần tự, không nhảy cóc ``` Bốn luật đã chốt: 1. **Sao lưu trước khi nâng** — `backup()` tạo `config.json.v.bak`. Người dùng lùi về bản app cũ vẫn còn đường về. 2. **Chỉ nâng, không hạ.** File mới hơn app → log cảnh báo, dùng nguyên trạng, không đoán ngược. 3. **Mỗi bước là một hàm riêng** trong `STEPS`, không viết logic đoán mò kiểu "có khoá `office` nghĩa là file cũ". 4. **Bước không nâng được version thì dừng**, không lặp vô hạn. ### Tiền lệ cần bắt chước: `_v1_to_v2` Đây **chính là** bước đã gỡ `api_key` khỏi đĩa đẩy vào `SecretStore`. Đọc nó trước khi thiết kế bất kỳ migration credential nào: ```python def _v1_to_v2(data, secrets): if secrets is None or not getattr(secrets, "available", True): log.info("bỏ qua v1→v2: máy này chưa có kho bí mật dùng được") return data # KHÔNG chuyển — thà để khoá nằm nguyên còn hơn # xoá đi rồi người dùng mất khoá không hiểu vì sao ... secrets.set(provider_key(name), key) conf["api_key"] = "" out["schema_version"] = 2 ``` Hai quyết định đáng học: - **Không có keyring thì không chuyển.** Giữ nguyên version 1, lần chạy sau trên máy có keyring sẽ chuyển. Mất dữ liệu người dùng tệ hơn là hoãn migration. - **Bỏ qua giá trị bù nhìn.** `api_key == "ollama"` là placeholder, đẩy vào keyring chỉ tổ rác. ## 4. ⚠️ Bẫy `.get(key, fallback)` trên config đã deep-merge Đây là bẫy sinh ra cả một lớp lỗi, và nó **không hiển nhiên**. ```python # config.py:265 def _deep_merge(base, override): ... # infrastructure/config/json_config_repository.py:90 merged = _deep_merge(merged, stored) # bắt đầu từ DEFAULT_CONFIG ``` Config đưa tới UI **luôn** đã được deep-merge với `DEFAULT_CONFIG`. Nghĩa là: > Mọi key có trong `DEFAULT_CONFIG` thì **luôn tồn tại** trong dict. Tham số thứ hai của > `.get()` **không bao giờ chạy**. ```python # DEFAULT_CONFIG có "sandbox_pw": "" sec.get("sandbox_pw", "") # → "" , KHÔNG phải "" ``` Hệ quả: - Fallback trông như "mặc định an toàn" thực ra là **code chết**. - Giá trị thật sự đang chạy là giá trị trong `DEFAULT_CONFIG` — thường là `""`. - Chuỗi rỗng đem đi so sánh mật khẩu là **mở khoá cho input rỗng**. **Luật:** đọc credential từ config thì **không** dùng fallback trong `.get()`. Đọc giá trị thật, rồi xử lý tường minh trường hợp rỗng — xem §9 về cách so sánh. ## 5. Ghi đè bằng biến môi trường `config.py::_apply_env_overrides` (dòng 276) — các biến hiện có: | Biến | Ghi vào | |---|---| | `COWORK_SANDBOX_PASSWORD` | `agent_security.sandbox_pw` | | `COWORK_MS365_UNLOCK_CODE` | `ms365.unlock_code` | | `COWORK_TEAMS_WEBHOOK` | `teams.webhook_url` | | `COWORK_ACTIVE_PROVIDER` | `active_provider` | | `COWORK_CA_BUNDLE` | `tls_ca_bundle` | Env override chạy **sau** deep-merge, nên nó thắng cả default lẫn file. Thêm secret mới thì cân nhắc có cần đường env cho triển khai theo tổ chức không. ## 6. Sinh giá trị ngẫu nhiên — dùng lại thứ có sẵn ```python # core/accounts.py:89 _CODE_ALPHABET = "ABCDEFGHJKMNPQRSTUVWXYZ23456789" # bỏ I, L, O, 0, 1 dễ đọc nhầm CODE_LENGTH = 12 def generate_code(existing_codes=None) -> str: """A random, non-repeating 12-character access code.""" code = "".join(secrets.choice(_CODE_ALPHABET) for _ in range(CODE_LENGTH)) ``` Dùng `secrets`, **không** `random`. Bảng chữ đã loại ký tự dễ nhầm vì mã này được người đọc bằng mắt rồi gõ lại. Cần mã cho người dùng đọc → gọi lại hàm này, đừng viết bản thứ hai. Không cần người đọc (token nội bộ) → `secrets.token_urlsafe(32)`. ## 7. Gate A và Git history ```bash python scripts/audit_security.py ``` Quét file `.py` và file config. Hiện có 3 phát hiện **có sẵn** trong `tests/test_project_context_*.py` — đừng nhận nhầm là do bản vá của mình. **Nếu secret đã nằm trong Git history** (`SECURITY.md`): 1. Dừng phân phối. 2. Báo Cowork Team. 3. **Không** rewrite history, **không** force-push nếu chưa có kế hoạch khắc phục phối hợp. 4. Xoay (rotate) credential có thể đã lộ. Gỡ literal khỏi code ở commit hôm nay **không** gỡ nó khỏi lịch sử. Luôn nêu điều này trong plan. ## 8. Câu hỏi phải hỏi người, không được tự quyết `docs/governance/review-policy.md`: thay đổi chạm credential cần Cowork Team soi thêm, và **CI xanh không đủ để merge**. Bốn câu sau là quyết định sản phẩm/bảo mật, agent chỉ được đề xuất: 1. Đây là **khoá chống bấm nhầm** hay **cơ chế bảo mật thật**? (quyết định mức đầu tư) 2. Lưu plaintext trong Keyring, hay lưu **hash** để cả admin cũng không đọc được? 3. Người dùng hiện có sẽ ra sao — giữ mật khẩu cũ, hay bị buộc đặt lại? 4. Giá trị sinh ra hiển thị cho người dùng thế nào, và hiện **mấy lần**? --- ## 9. So sánh credential — hai bẫy đi liền nhau Ghi lại từ defect `SEC-20260907-01`. Cả hai đều là bug **thật** đã xảy ra trong repo này. ### 9.1 Chuỗi rỗng phải bị chặn TRƯỚC khi so sánh `DEFAULT_CONFIG` cho credential thường là `""`, và §4 giải thích vì sao giá trị đó luôn đến tay chỗ dùng. Nên `entered == stored` biến ô nhập trống thành mật khẩu hợp lệ. Mẫu đúng đã có sẵn trong repo — `infrastructure/config/json_config_repository.py`: ```python if (code or "") and code == self.ms365.get("unlock_code", ""): ``` `(code or "") and ...` là chốt chặn. Bên sandbox thiếu đúng chốt này và thành lỗ hổng S1. ### 9.2 ⚠️ `secrets.compare_digest` KHÔNG nhận `str` ngoài ASCII Đổi `==` sang `compare_digest` là nâng cấp đúng hướng (timing-safe), nhưng nó mang theo một ràng buộc mới mà `==` không có: ```python >>> secrets.compare_digest("mật khẩu", "mật khẩu") TypeError: comparing strings with non-ASCII characters is not supported ``` Cowork Local mặc định **tiếng Việt** và phục vụ **khách Nhật**. Mật khẩu có dấu ở đây là input bình thường, không phải trường hợp biên. Để nguyên là exception thoát ra khỏi Qt slot. **Luật:** so sánh trên bytes. ```python return secrets.compare_digest(entered.encode("utf-8"), stored.encode("utf-8")) ``` ### 9.3 Bài học tổng quát — quan trọng hơn hai mục trên > Một API "an toàn hơn" thường có **miền đầu vào hẹp hơn** thứ nó thay thế. `compare_digest` an toàn hơn `==` về timing, nhưng chỉ nhận ASCII-`str` hoặc bytes. Trước khi thay một phép toán bằng phiên bản "chuẩn bảo mật", luôn hỏi: - [ ] Nó nhận những kiểu nào? Có hẹp hơn cái cũ không? - [ ] Dữ liệu thật của app có nằm trọn trong miền đó không? (ngôn ngữ, độ dài, `None`) - [ ] Nó ném exception hay trả `False` khi gặp đầu vào ngoài miền? - [ ] Có test cho đúng đầu vào ngoài miền đó chưa? Ba dòng đầu của checklist này chính là thứ đã bị bỏ qua ở `SEC-20260907-01`, và nó lọt qua vòng review đầu tiên.