cowork-local/agent/knowledge/secrets_and_config.md

# 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.<nhóm>` |
| 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<timestamp>.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", "<literal đã bị gỡ>")     # → "" , KHÔNG phải "<literal đã bị gỡ>"
```

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.