Files
curo1305andClaude Sonnet 4.6 8629bc0854 docs(phase-7): add security threat verification — 12/12 CLOSED
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>
2026-06-05 10:50:44 +02:00

78 lines
5.0 KiB
Markdown
Raw Permalink Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
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:5670` 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:902906`; `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:730732`; `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)