Formal Phase 7 SECURITY.md: - T-07-01: api_key_enc excluded from GET /api/admin/ai-config (whitelist) - T-07-02: HKDF domain separation confirmed (ai-provider-settings vs cloud-credentials) - T-07-03: is_active atomic UPDATE, no read-then-write - T-07-04: empty api_key normalised to "not-needed" before AsyncOpenAI - T-07-05/07/09/11: accepted risks documented - T-07-06/08/10/12: mitigations verified in implementation + tests bandit: zero HIGH; npm audit: zero high/critical; threats_open: 0 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
78 lines
5.0 KiB
Markdown
78 lines
5.0 KiB
Markdown
---
|
||
phase: 7
|
||
slug: 07-redo-and-optimize-llm-integration
|
||
status: verified
|
||
threats_open: 0
|
||
asvs_level: 2
|
||
created: 2026-06-05
|
||
---
|
||
|
||
# Phase 7 — Security
|
||
|
||
> Full audit detail in project-root `SECURITY.md` — "Phase 07 Threat Verification" section.
|
||
|
||
---
|
||
|
||
## Trust Boundaries
|
||
|
||
| Boundary | Description | Data Crossing |
|
||
|----------|-------------|---------------|
|
||
| Admin API → DB | PUT /api/admin/ai-config writes encrypted api_key to system_settings | AES-GCM ciphertext; plaintext never stored |
|
||
| Celery → AI Provider | HTTP requests to external AI endpoints using decrypted api_key | API key in memory only, per-task, never serialized back to broker |
|
||
| Admin API → Client | GET /api/admin/ai-config response | `has_api_key` (bool) only — no ciphertext, no plaintext |
|
||
|
||
---
|
||
|
||
## Threat Register
|
||
|
||
| Threat ID | Category | Component | Disposition | Mitigation | Status |
|
||
|-----------|----------|-----------|-------------|------------|--------|
|
||
| T-07-01 | Information Disclosure | GET /api/admin/ai-config response body | mitigate | `_ai_config_to_dict()` whitelist at `admin.py:56–70` excludes `api_key_enc`; `test_get_never_returns_key` asserts absence | CLOSED |
|
||
| T-07-02 | Elevation of Privilege | HKDF key derivation domain separation | mitigate | `info=b"ai-provider-settings"` vs `info=b"cloud-credentials"`; cross-domain decrypt raises `InvalidToken`; `test_api_key_encrypt_decrypt` validates | CLOSED |
|
||
| T-07-03 | Tampering | system_settings.is_active dual-write race | mitigate | Single atomic `UPDATE … SET is_active = (provider_id == target)` at `admin.py:902–906`; `test_put_writes_active_provider` asserts COUNT(is_active)=1 | CLOSED |
|
||
| T-07-04 | Tampering | OpenAI SDK 2.34+ empty api_key rejection | mitigate | Factory applies `api_key or "not-needed"` at `ai/__init__.py:62`; defence-in-depth in `openai_provider.py:16` | CLOSED |
|
||
| T-07-05 | Information Disclosure | Singleton _client retained across Celery tasks | accept | Each `asyncio.run(_run(...))` creates fresh provider; no module-level client exists | CLOSED |
|
||
| T-07-06 | Information Disclosure | classifier per-user override path | mitigate | Override path uses hardcoded `api_key=""`; factory normalises to `"not-needed"`; key never read from system_settings in this path | CLOSED |
|
||
| T-07-07 | Tampering | Anthropic output_config grammar limit | accept | Schemas ≤3 properties/2 required, no unions; well within Anthropic grammar limits | CLOSED |
|
||
| T-07-08 | Denial of Service | Anthropic stop_reason "refusal"/"max_tokens" | mitigate | `classify()` falls back to `parse_classification("")` → empty `ClassificationResult`; no exception propagated | CLOSED |
|
||
| T-07-09 | Denial of Service | Re-classify endpoint without sub-100/min rate limit | accept | Auth + ownership + `@account_limiter.limit("100/minute")` present; tighter limit deferred to Phase 6 expansion | CLOSED |
|
||
| T-07-10 | Tampering | Cross-user reclassify IDOR | mitigate | Inline ownership check at `documents.py:730–732`; `test_reclassify_cross_user_returns_404` validates | CLOSED |
|
||
| T-07-11 | Information Disclosure | Retry exception payload in Celery broker | accept | Only `str(exc)` — no document content or credentials; Redis broker internal-only | CLOSED |
|
||
| T-07-12 | Tampering | self.retry called inside asyncio.run | mitigate | `_ClassificationError` sentinel escapes `asyncio.run()`; `self.retry()` in sync layer only; `test_retry_backoff` validates countdown sequence | CLOSED |
|
||
|
||
---
|
||
|
||
## Accepted Risks Log
|
||
|
||
| Risk ID | Component | Accepted Risk | Rationale |
|
||
|---------|-----------|---------------|-----------|
|
||
| T-07-05 | Celery AI provider client | No cross-task singleton risk | Fresh event loop per `asyncio.run()` invocation; provider local to `_run()` |
|
||
| T-07-07 | Anthropic output_config schema | Grammar limit not enforced programmatically | Simple schemas maintained as code convention |
|
||
| T-07-09 | /classify rate limiting | No sub-100/min per-account rate limit in v1 | 100/min account limiter + auth + ownership present; tighter limit deferred |
|
||
| T-07-11 | Celery broker exception payload | Exception message flows through Redis broker | `str(exc)` only — no document content or credentials |
|
||
|
||
---
|
||
|
||
## Security Audit Trail
|
||
|
||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||
|------------|---------------|--------|------|--------|
|
||
| 2026-06-05 | 12 | 12 | 0 | gsd-security-auditor (claude-sonnet-4-6) |
|
||
|
||
---
|
||
|
||
## Sign-Off
|
||
|
||
- [x] All threats have a disposition (mitigate / accept / transfer)
|
||
- [x] Accepted risks documented in Accepted Risks Log
|
||
- [x] `threats_open: 0` confirmed
|
||
- [x] `status: verified` set in frontmatter
|
||
|
||
---
|
||
|
||
## Bandit / Dependency Scan
|
||
|
||
- `bandit -r backend/ -ll` (2026-06-05): **zero HIGH severity** (0 Medium, 776 Low informational, 0 `# nosec` suppressions)
|
||
- `npm audit --audit-level=high` (2026-06-05): **zero high/critical** (2 moderate — esbuild/vite dev-only, no fix without breaking change)
|
||
- `pip-audit`: not runnable locally (Python 3.9 host vs 3.12 project); inherited clean gate from Phase 6.2 (`f1a7f52` — python-multipart + PyMuPDF CVE fixes)
|