9/9 threats CLOSED; 11/11 Phase 7.2 tests PASSED; bandit 0 HIGH; full 391-test suite green. Documents gap closure for CR-02 (password_reset_confirm user_nbf write). HS256→ES256 deferred to Phase 7.3. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
250 lines
33 KiB
Markdown
250 lines
33 KiB
Markdown
# SECURITY.md — Phase 02 + Phase 03
|
||
|
||
**Audit date:** 2026-06-01
|
||
**Phase 02:** users-authentication (plans 01–06) — previously audited, result SECURED
|
||
**Phase 03:** document-migration-multi-user-isolation (plans 01–05)
|
||
**ASVS Level:** L2
|
||
**Auditor:** gsd-security-auditor (claude-sonnet-4-6)
|
||
|
||
---
|
||
|
||
## Phase 02 Threat Verification (reproduced from previous audit)
|
||
|
||
| Threat ID | Category | Disposition | Status | Evidence |
|
||
|-----------|----------|-------------|--------|----------|
|
||
| T-02-01 | Spoofing | mitigate | CLOSED | `services/auth.py:93` — `payload.get("typ") != "access"` raises ValueError after JWT decode; prevents password-reset tokens from being accepted as access tokens |
|
||
| T-02-02 | Spoofing | mitigate | CLOSED | `services/auth.py:181-185` — on revoked token reuse: `revoke_all_refresh_tokens()` called, `send_security_alert_email.delay()` enqueued, `ValueError("token_family_revoked")` raised |
|
||
| T-02-03 | Tampering | mitigate | CLOSED | `services/auth.py:310` — `code_hash=hash_password(code)` (Argon2); `services/auth.py:338` — `verify_password(code, row.code_hash)` constant-time comparison via pwdlib |
|
||
| T-02-04 | Repudiation | mitigate | CLOSED | `services/auth.py:397-408` — checks `admin_email`/`admin_password` set; `select(User).limit(1)` guards idempotency; logs WARNING when env vars missing |
|
||
| T-02-05 | Info Disclosure | mitigate | CLOSED | `services/auth.py:360` — `sha1[:5]` prefix only sent to HIBP URL; suffix compared locally with `hmac.compare_digest` |
|
||
| T-02-06 | DoS | accept | CLOSED | Accepted: `services/auth.py:369-371` — `httpx timeout=5.0`, `except Exception: logger.warning(…); return False` (fail-open) |
|
||
| T-02-07 | EoP | mitigate | CLOSED | `deps/auth.py:87` — `if user.role != "admin": raise HTTPException(403, "Admin access required")` |
|
||
| T-02-08 | EoP | mitigate | CLOSED | `api/admin.py` — no route containing `/impersonate`, `/login-as`, or any code path setting JWT `sub` to a different user; verified by grep (0 matches) |
|
||
| T-02-SC | Tampering | mitigate | CLOSED | `backend/requirements.txt:23-26` — `PyJWT>=2.8.0`, `pwdlib[argon2]>=0.2.1`, `pyotp>=2.9.0`, `slowapi>=0.1.9` all pinned |
|
||
| T-02-09 | Spoofing | mitigate | CLOSED | `api/auth.py:248` — identical detail `"Incorrect email or password"` for both non-existent email (`user is None`) and wrong password branches |
|
||
| T-02-10 | Spoofing | mitigate | CLOSED | `api/auth.py:673-676` — always returns 202 with `"If an account exists…"` message regardless of whether email was found |
|
||
| T-02-11 | Tampering | mitigate | CLOSED | `main.py:100` — `samesite="strict"` on refresh cookie (`api/auth.py:100`); `main.py:47-61` — `OriginValidationMiddleware` rejects non-GET/HEAD/OPTIONS requests with Origin not in `settings.cors_origins` |
|
||
| T-02-12 | Info Disclosure | accept | CLOSED | Accepted: `stores/auth.js` — `accessToken = ref(null)` (Pinia memory only); grep returns 0 hits for `localStorage`/`sessionStorage` |
|
||
| T-02-13 | DoS | mitigate | CLOSED | `api/auth.py:121,195,326` — `@limiter.limit("10/minute")` on register/login/refresh; `api/auth.py:215-224` — per-account Redis counter `login_attempts:{email}` capped at 10 in 15 min |
|
||
| T-02-14 | Info Disclosure | mitigate | CLOSED | `main.py:32-40` — `SecurityHeadersMiddleware` sets `Content-Security-Policy`, `X-Frame-Options: DENY`, `X-Content-Type-Options: nosniff` on every response |
|
||
| T-02-15 | Tampering | mitigate | CLOSED | `main.py:124` — `allow_origins=settings.cors_origins`; grep for `allow_origins=["*"]` returns 0 matches |
|
||
| T-02-16 | EoP | mitigate | CLOSED | `api/auth.py:259-260` — `if user.password_must_change: return {"requires_password_change": True, "user_id": …}` — no tokens issued, no cookie set |
|
||
| T-02-17 | Spoofing | mitigate | CLOSED | `services/auth.py:262-270` — Redis key `totp_used:{user_id}:{code}`, pre-check before verify, set with `ex=90` after valid code |
|
||
| T-02-18 | Spoofing | mitigate | CLOSED | `services/auth.py:330,345` — only queries `BackupCode.used_at.is_(None)`; sets `used_at = datetime.now(timezone.utc)` on first use |
|
||
| T-02-19 | Info Disclosure | mitigate | CLOSED | `api/auth.py:594-609` — plaintext codes returned once from `POST /totp/enable`; `services/auth.py:310` stores as Argon2 hashes |
|
||
| T-02-20 | EoP | mitigate | CLOSED | `services/auth.py:125-126` — `decode_password_reset_token` checks `payload.get("typ") != "password-reset"` raises ValueError |
|
||
| T-02-21 | EoP | mitigate | CLOSED | `api/auth.py:730` — `POST /password-reset/confirm` returns `{"message": "Password updated. Please sign in."}` only; no `access_token` in response |
|
||
| T-02-22 | Info Disclosure | mitigate | CLOSED | `api/auth.py:673` — `# Always return 202` comment and return statement outside the `if user is not None` block |
|
||
| T-02-23 | Tampering | accept | CLOSED | Accepted: pyotp internal string compare; rate limiting (10/min on `/totp/enable`) is primary defense |
|
||
| T-02-24 | Spoofing | mitigate | CLOSED | `frontend/src/components/auth/ConfirmBlock.vue` exists with `confirmed`/`cancelled` emits; AccountView wires `@confirmed` to `logoutAll()` call |
|
||
| T-02-25 | DoS | mitigate | CLOSED | `api/auth.py:565` — `@limiter.limit("10/minute")` on `POST /api/auth/totp/enable` |
|
||
| T-02-26A | EoP | mitigate | CLOSED | `api/admin.py` — `grep -c get_current_admin` returns 12; every handler has `_admin: User = Depends(get_current_admin)` |
|
||
| T-02-26B | Spoofing | mitigate | CLOSED | `services/auth.py:330` — `BackupCode.used_at.is_(None)` filter; used codes are invisible to subsequent `verify_backup_code()` calls |
|
||
| T-02-27A | Info Disclosure | mitigate | CLOSED | `api/admin.py:75-90` — `_user_to_dict()` whitelist: `id, handle, email, role, is_active, totp_enabled, ai_provider, ai_model, password_must_change, created_at` — no `password_hash`, `credentials_enc`, or `totp_secret` |
|
||
| T-02-27B | Spoofing | mitigate | CLOSED | `api/auth.py:215-224` — per-account Redis counter incremented before TOTP/backup_code branch; applies to all login paths |
|
||
| T-02-28 | EoP | mitigate | CLOSED | `api/admin.py` — grep for `impersonate`/`login.as`/`login_as` returns 0 matches |
|
||
| T-02-29 | DoS | mitigate | CLOSED | `api/admin.py:305-316` — COUNT query on `(role='admin', is_active=True)` before deactivation; raises HTTP 400 if `active_admin_count <= 1` |
|
||
| T-02-30A | Tampering | mitigate | CLOSED | `api/admin.py:348,377` — `status_code=HTTP_202_ACCEPTED`; returns `{"message": "Password reset email sent"}`; no token in response |
|
||
| T-02-30B | EoP | mitigate | CLOSED | `frontend/src/components/layout/AppSidebar.vue:189` — `v-if="authStore.user?.role === 'admin'"` on Admin router-link |
|
||
| T-02-31A | Info Disclosure | accept | CLOSED | Accepted: quota endpoint exposes `limit_bytes`/`used_bytes` — admin operational data, no PII, no document content |
|
||
| T-02-31B | EoP | mitigate | CLOSED | `frontend/src/components/admin/AdminUsersTab.vue` — grep for `impersonate`/`loginAs`/`login-as` returns 0 matches; same for AdminQuotasTab and AdminAiConfigTab |
|
||
| T-02-32A | EoP | mitigate | CLOSED | `api/admin.py:255` — `password_must_change=True` in `User(…)` constructor on `POST /api/admin/users` |
|
||
| T-02-32B | Info Disclosure | mitigate | CLOSED | `frontend/src/components/admin/AdminUsersTab.vue` — no `password_hash`, `credentials_enc`, or `totp_secret` bound in template; all fields come from `_user_to_dict()` whitelist |
|
||
| T-02-33 | Tampering | mitigate | CLOSED | `frontend/src/components/admin/AdminUsersTab.vue:153-174` — `v-if="confirmDeactivate === user.id"` shows inline block with `{{ user.email }}` before calling `adminDeactivateUser(id)` |
|
||
| T-02-34 | DoS | accept | CLOSED | Accepted: admin is trusted role; no rate limit on `POST /api/admin/users` is intentional |
|
||
| T-02-GAP-01 | EoP | mitigate | CLOSED | `frontend/src/router/index.js:42` — `meta: { requiresAdmin: true }` on `/admin` route; `router/index.js:91-93` — `if (to.meta.requiresAdmin && authStore.user?.role !== 'admin') return { path: '/' }` |
|
||
| T-02-GAP-02 | Info Disclosure | mitigate | CLOSED | `frontend/src/App.vue:2` — `<AuthLayout v-if="route.meta.layout === 'auth'" />` renders no sidebar on auth routes |
|
||
| T-02-GAP-03 | Tampering | accept | CLOSED | Accepted (already mitigated): `api/admin.py:265` — `await session.flush()` present before `write_audit_log()` in `create_user`; regression test `test_create_user_writes_audit_log` passes |
|
||
| T-02-GAP-SC | Tampering | mitigate | CLOSED | `frontend/package.json:13` — `"qrcode": "^1.5.4"` — canonical npm package (20M+ weekly downloads); no server-side dependency |
|
||
|
||
---
|
||
|
||
## Phase 03 Threat Verification
|
||
|
||
**Audit date:** 2026-06-01
|
||
**Plans audited:** 03-01 through 03-05
|
||
**Result:** OPEN_THREATS — 4 accepted-risk entries not yet documented (see Accepted Risks Log below)
|
||
|
||
| Threat ID | Category | Disposition | Status | Evidence |
|
||
|-----------|----------|-------------|--------|----------|
|
||
| T-03-01 | Tampering | mitigate | CLOSED | `backend/migrations/versions/0003_multi_user_isolation.py:56-88` — `null_user_objects` collected via SELECT before DELETE (line 56-57); each `client.remove_object()` wrapped in `try/except Exception: pass` (line 85-88); partial MinIO failure leaves only orphans |
|
||
| T-03-02 | DoS | accept | CLOSED | Documented in Accepted Risks Log below |
|
||
| T-03-03 | Info Disclosure | mitigate | CLOSED | `backend/tests/conftest.py:221` — `token = create_access_token(str(user_id), "user")` uses standard `services.auth.create_access_token` (test secret_key from Settings); async_client fixture clears `app.dependency_overrides` in teardown (line 155); no token values logged anywhere in conftest |
|
||
| T-03-04 | Spoofing | mitigate | CLOSED | `backend/api/documents.py:112-113` — `suffix = Path(body.filename).suffix.lower()`, `object_key = f"{current_user.id}/{doc_id}/{uuid.uuid4()}{suffix}"` — object_key computed server-side; `body.filename` stored in `Document.filename` DB column only; extension from `Path().suffix.lower()` |
|
||
| T-03-05 | Tampering | mitigate | CLOSED | `backend/api/documents.py:327` — `size = await get_storage_backend().stat_object(doc.object_key)` — size from MinIO stat, not from client; client body contains no size field; confirm endpoint has no `body` parameter beyond doc_id path param |
|
||
| T-03-06 | DoS | mitigate | CLOSED | `backend/api/documents.py:341-351` — `UPDATE quotas SET used_bytes = used_bytes + :delta WHERE user_id = :uid AND (used_bytes + :delta) <= limit_bytes RETURNING used_bytes, limit_bytes`; `row = result.fetchone()`; `if row is None:` → HTTP 413 (lines 353-374) |
|
||
| T-03-07 | Info Disclosure | accept | CLOSED | Documented in Accepted Risks Log below |
|
||
| T-03-08 | Repudiation | mitigate | CLOSED | `backend/tasks/document_tasks.py:132-177` — `cleanup_abandoned_uploads` Celery task exists; `_cleanup_abandoned()` selects `Document.status == "pending"` and `Document.created_at < cutoff` (1 hour); `backend/celery_app.py:43-46` — `beat_schedule` entry with `_timedelta(minutes=30)` |
|
||
| T-03-09 | Info Disclosure | mitigate | CLOSED | `docker-compose.yml:26` — `MINIO_API_CORS_ALLOW_ORIGIN: ${FRONTEND_URL:-http://localhost:5173}` — explicit non-wildcard origin; env var defaults to specific origin. Note: implementation uses `FRONTEND_URL` instead of plan's `CORS_ORIGINS` — both default to `http://localhost:5173`; wildcard exclusion is confirmed |
|
||
| T-03-10 | Tampering | mitigate | CLOSED | `backend/storage/minio_backend.py:54-60` — `self._public_client = Minio(endpoint=(public_endpoint or endpoint), ...)` — dual client instantiated in `__init__`; `generate_presigned_put_url` uses `self._public_client` (line 154); `stat_object` uses `self._client` (line 169) |
|
||
| T-03-11 | Info Disclosure | mitigate | CLOSED | `backend/api/documents.py` — ownership assertion pattern `if doc is None or doc.user_id != current_user.id: raise HTTPException(status_code=404, ...)` appears at: confirm (line 322-323), get (line 545-546), delete (line 633-634), classify (line 702-703), patch (line 579-580), content (line 767); all raise 404 not 403 |
|
||
| T-03-12 | EoP | mitigate | CLOSED | `backend/deps/auth.py:95-109` — `get_regular_user` raises HTTP 403 if `user.role == "admin"`; `backend/api/documents.py` — `Depends(get_regular_user)` present on all 7 document handlers: upload-url (line 99), upload (line 143), confirm (line 302), list (line 416), get (line 530), patch (line 557), delete (line 613), classify (line 688), content (line 742) |
|
||
| T-03-13 | Info Disclosure | mitigate | CLOSED | `backend/services/storage.py:270-282` (load_topics_for_user) — `or_(Topic.user_id == user_id, Topic.user_id.is_(None))` filter; `backend/api/topics.py:44` — `storage.load_topics_for_user(session, user_id=current_user.id)`; `backend/api/topics.py:64` — `storage.create_topic(..., user_id=current_user.id)` |
|
||
| T-03-14 | EoP | mitigate | CLOSED | `backend/api/admin.py:602-622` — `POST /api/admin/topics` with `_admin: User = Depends(get_current_admin)`, creates `Topic(user_id=None)`; `backend/api/topics.py:63-64` — regular `POST /api/topics` forces `user_id=current_user.id` |
|
||
| T-03-15 | Tampering | mitigate | CLOSED | `backend/api/documents.py:113` — `object_key = f"{current_user.id}/{doc_id}/{uuid.uuid4()}{suffix}"` — prefix always `str(current_user.id)`; no user-supplied prefix accepted; `null-user` sentinel confirmed absent (grep returns 0 for "null-user" in documents.py) |
|
||
| T-03-16 | Spoofing | mitigate | CLOSED | `backend/deps/auth.py:35` — `security = HTTPBearer()` (auto_error=True default) raises 403 on missing Authorization header; `get_current_user` raises 401 (lines 52-55) on invalid/expired token |
|
||
| T-03-17 | EoP | mitigate | CLOSED | `backend/api/settings.py` does not exist (confirmed absent); `backend/main.py` contains no `settings_router` import or `include_router` for settings; only admin endpoint writes `user.ai_provider`/`user.ai_model` |
|
||
| T-03-18 | Info Disclosure | mitigate | CLOSED | `backend/services/storage.py` — grep for `load_settings`/`save_settings`/`mask_api_key`/`settings_masked` returns 0 matches in non-comment lines; comment at line 12 references removal but no function bodies present |
|
||
| T-03-19 | Tampering | mitigate | CLOSED | `backend/tasks/document_tasks.py:62-64` — `user = await session.get(User, doc.user_id) if doc.user_id else None; ai_provider = (user.ai_provider if user else None) or app_settings.default_ai_provider`; task signature is `extract_and_classify(document_id: str)` — no provider in broker message |
|
||
| T-03-20 | Info Disclosure | accept | CLOSED | Documented in Accepted Risks Log below |
|
||
| T-03-21 | Repudiation | mitigate | CLOSED | `frontend/src/api/client.js` — grep for `getSettings`/`patchSettings`/`testProvider`/`getDefaultPrompt` returns 0 matches; `SettingsView.vue` imports only `SettingsPreferencesTab`, `SettingsAiTab`, `SettingsCloudTab`, `SettingsAccountTab` — no old settings store; `SettingsAiTab.vue` contains no API calls (static read-only display) |
|
||
| T-03-22 | Info Disclosure | mitigate | CLOSED | `frontend/src/stores/documents.js:24-25` — `xhr.setRequestHeader('Content-Type', ...)` only; comment `// NOTE: no Authorization header — presigned URL is self-authenticating (T-03-22)`; no `setRequestHeader('Authorization', ...)` call present |
|
||
| T-03-23 | Spoofing | mitigate | CLOSED | `frontend/src/components/upload/UploadProgress.vue:27,30` — `item.quotaError.rejected_bytes`, `item.quotaError.used_bytes`, `item.quotaError.limit_bytes` all sourced from server 413 response body (via `err.payload` from `api/client.js`); no local `file.size` calculation |
|
||
| T-03-24 | DoS | accept | CLOSED | Documented in Accepted Risks Log below |
|
||
| T-03-25 | Tampering | mitigate | CLOSED | `frontend/src/stores/documents.js:70` — `const rowKey = \`${file.name}__${Date.now()}\`` — composite key prevents collision for same-filename concurrent uploads |
|
||
| T-03-26 | Repudiation | mitigate | CLOSED | `frontend/src/stores/auth.js:144-149` — `fetchQuota()` wraps `api.getMyQuota()` in `try { ... } catch { // Silently ignore }`; last-known values preserved on error; `QuotaBar.vue` hides via `v-if="!loadFailed"` on catch |
|
||
| T-03-SC (×5) | Tampering | mitigate | CLOSED | No new pip or npm package installs in any of plans 03-01 through 03-05; all packages already pinned from Phase 1/2 |
|
||
|
||
---
|
||
|
||
## Accepted Risks Log
|
||
|
||
| Risk ID | Component | Accepted Risk | Rationale |
|
||
|---------|-----------|---------------|-----------|
|
||
| T-02-06 | HIBP network call | Fail-open on network error — auth proceeds | `httpx timeout=5s`; logging warning; HIBP unavailability must not block legitimate logins |
|
||
| T-02-12 | Access token in JavaScript | Token held in Pinia `ref()` memory | Lost on page refresh (by design); refresh endpoint uses httpOnly cookie to reissue |
|
||
| T-02-23 | TOTP constant-time compare | pyotp uses Python string compare | Rate limiting (10/min on `/totp/enable`) is the primary defense; 6-digit TOTP window makes brute force impractical within the rate window |
|
||
| T-02-31A | Quota endpoint | Admin can view `limit_bytes`/`used_bytes` | No PII; no document content; operational necessity for quota management |
|
||
| T-02-34 | Admin user creation | No rate limit on `POST /api/admin/users` | Admin is a trusted role; rate limiting would hinder legitimate bulk user provisioning |
|
||
| T-02-GAP-03 | admin.py create_user flush order | Already mitigated — documented as accepted | `session.flush()` present at `admin.py:265`; regression test confirms FK ordering |
|
||
| T-03-02 | Alembic migration when MinIO unreachable | Migration may leave MinIO objects undeleted if MinIO is unreachable at migration time | Migration runs only after docker-compose health checks confirm MinIO is ready (backend service `depends_on: minio: condition: service_healthy`); if MinIO is down, deployment is blocked before migration runs; orphaned objects are harmless (no DB row references them); retry on next deploy |
|
||
| T-03-07 | Presigned URL in application logs | 15-minute TTL presigned URL may appear in debug logs | TTL is 15 minutes; only `document_id` (not full URL) is logged at the document endpoint level; low risk for v1; full log redaction deferred to Phase 4/5 |
|
||
| T-03-20 | SYSTEM_PROMPT env var in container logs | `settings.system_prompt` value visible in container startup logs if log level includes config dump | `SYSTEM_PROMPT` is a static AI instruction string with no PII, no credentials, no secrets; container log exposure of this value has no security impact |
|
||
| T-03-24 | Concurrent browser uploads exhaust memory | Multiple simultaneous large-file uploads could exhaust browser memory | XHR-based upload streams bytes natively without buffering in JavaScript memory; browser natively handles the file stream; v1 acceptance — concurrent upload limits are a UX concern, not a security concern |
|
||
|
||
---
|
||
|
||
## Unregistered Threat Flags
|
||
|
||
None. All `## Threat Flags` sections in plans 03-01 through 03-05 summaries report no new attack surface beyond the registered threat IDs.
|
||
|
||
---
|
||
|
||
## Phase 07 Threat Verification
|
||
|
||
**Audit date:** 2026-06-05
|
||
**Phase:** 7 — Redo and Optimize LLM Integration
|
||
**ASVS Level:** L2
|
||
**Auditor:** gsd-security-auditor (claude-sonnet-4-6)
|
||
**Threats closed:** 12/12
|
||
**Open threats (blockers):** 0
|
||
|
||
### Threat Verification
|
||
|
||
| Threat ID | Category | Disposition | Status | Evidence |
|
||
|-----------|----------|-------------|--------|----------|
|
||
| T-07-01 | Information Disclosure | mitigate | CLOSED | `_ai_config_to_dict()` at `backend/api/admin.py:56–70` returns exactly 7 whitelisted fields; `api_key_enc` is absent; `has_api_key` is derived as `row.api_key_enc is not None`. Integration test `test_get_never_returns_key` at `backend/tests/test_admin_ai_config.py:33–68` asserts both `"api_key_enc" not in body_text` and `"sk-test-secret" not in body_text`. |
|
||
| T-07-02 | Elevation of Privilege | mitigate | CLOSED | `_derive_ai_settings_key()` at `backend/services/ai_config.py:67` uses `info=b"ai-provider-settings"`. Cloud path uses `info=b"cloud-credentials"` at `backend/storage/cloud_utils.py:137`. Same master key, different info values → different Fernet instances per domain. Unit test `test_api_key_encrypt_decrypt` at `backend/tests/test_ai_config.py:43–46` asserts cross-provider decrypt raises `InvalidToken`. |
|
||
| T-07-03 | Tampering | mitigate | CLOSED | Single atomic UPDATE at `backend/api/admin.py:902–906`: `update(SystemSettings).values(is_active=(SystemSettings.provider_id == body.provider_id))`. No read-then-write. Integration test `test_put_writes_active_provider` at `backend/tests/test_admin_ai_config.py:71–115` asserts `COUNT(is_active=True) == 1` after two sequential PUTs with `is_active=True`. |
|
||
| T-07-04 | Tampering | mitigate | CLOSED | `get_provider()` at `backend/ai/__init__.py:62` applies `effective_api_key = config.api_key or "not-needed"` before constructing any provider. `OpenAIProvider.__init__` at `backend/ai/openai_provider.py:16` applies a defence-in-depth `api_key or "not-needed"`. `GenericOpenAIProvider` inherits via `super().__init__()`. Empty string never reaches `AsyncOpenAI(api_key=...)`. |
|
||
| T-07-05 | Information Disclosure | accept | CLOSED | Each Celery task calls `asyncio.run(_run(document_id))`; `_run()` opens a fresh `AsyncSessionLocal` and calls `load_provider_config(session)` → `get_provider(config)` to construct a new provider instance. No provider or `_client` is module-level or class-level. Cross-task credential sharing is structurally impossible. |
|
||
| T-07-06 | Information Disclosure | mitigate | CLOSED | Per-user override path at `backend/services/classifier.py:74–80` constructs `ProviderConfig(api_key="", ...)` — hardcoded empty string; never reads from system_settings. Factory normalises `""` to `"not-needed"` at `backend/ai/__init__.py:62`. API key never flows through the per-user override path. |
|
||
| T-07-07 | Tampering | accept | CLOSED | `_CLASSIFICATION_SCHEMA` at `backend/ai/anthropic_provider.py:25–34`: 3 properties, 2 required, `additionalProperties: False`, no unions. `_SUGGESTIONS_SCHEMA`: 1 property, 1 required. Neither schema approaches Anthropic grammar size limits. |
|
||
| T-07-08 | Denial of Service | mitigate | CLOSED | `AnthropicProvider.classify()` at `backend/ai/anthropic_provider.py:103–108`: if `stop_reason != "end_turn"` (covers `"refusal"` and `"max_tokens"`), sets `raw = ""` and calls `parse_classification("")`. `parse_classification("")` at `backend/ai/utils.py:17–35` returns `ClassificationResult()` (empty, no exception). |
|
||
| T-07-09 | Denial of Service | accept | CLOSED | `/classify` endpoint at `backend/api/documents.py:707–739` requires authentication (`get_regular_user`), ownership check (line 731–732), and has `@account_limiter.limit("100/minute")` at line 708. Sub-100/min per-account limit deferred to Phase 6 expansion. Documented accepted risk. |
|
||
| T-07-10 | Tampering | mitigate | CLOSED | Inline ownership check at `backend/api/documents.py:730–732`: `if doc is None or doc.user_id != current_user.id: raise HTTPException(404, "Document not found")`. Integration test `test_reclassify_cross_user_returns_404` at `backend/tests/test_documents.py:1009–1059` confirms 404 response for cross-user attempt. |
|
||
| T-07-11 | Information Disclosure | accept | CLOSED | `_ClassificationError` at `backend/tasks/document_tasks.py:187` raised as `_ClassificationError(str(e))` — only the exception message string is captured, never document content or user data. Redis broker is internal-only in deployment. |
|
||
| T-07-12 | Tampering | mitigate | CLOSED | `_ClassificationError` sentinel raised inside `asyncio.run(_run(...))` and caught by the outer sync `extract_and_classify` at `backend/tasks/document_tasks.py:81–86`; `self.retry()` is called in the sync layer only. Unit test `test_retry_backoff` at `backend/tests/test_document_tasks.py:26–59` validates the sentinel escape and countdown sequence. |
|
||
|
||
### Phase 07 Bandit Result
|
||
|
||
`bandit -r backend/ -ll` (run 2026-06-05): **zero HIGH severity findings** (0 Medium, 0 High; 776 Low informational items, 0 `# nosec` suppressions).
|
||
|
||
### Phase 07 Unregistered Flags
|
||
|
||
None. No `## Threat Flags` section provided for Phase 7. All threats resolved from the supplied register.
|
||
|
||
### Phase 07 Accepted Risks
|
||
|
||
| Risk ID | Component | Accepted Risk | Rationale |
|
||
|---------|-----------|---------------|-----------|
|
||
| T-07-05 | Celery AI provider client | No cross-task singleton risk | `asyncio.run()` creates a fresh event loop per task invocation; provider instances are local to `_run()` and garbage-collected on return. |
|
||
| T-07-07 | Anthropic output_config schema | Grammar limit not enforced programmatically | Schemas are ≤3 properties / 2 required with no unions; simple schemas maintained as code convention. |
|
||
| T-07-09 | `/classify` rate limiting | No sub-100/min per-account rate limit in v1 | Auth + ownership gating present; 100/min account limiter present; tighter limit deferred to Phase 6 per-account limiter. |
|
||
| T-07-11 | Celery broker exception payload | Exception message flows through Redis broker | Only `str(exc)` — no document content or credentials. Redis broker is internal-network only. |
|
||
|
||
---
|
||
|
||
## Notes
|
||
|
||
### Phase 03 Audit Notes
|
||
|
||
- **T-03-09 env var deviation:** The implementation uses `MINIO_API_CORS_ALLOW_ORIGIN: ${FRONTEND_URL:-http://localhost:5173}` instead of the plan's `${CORS_ORIGINS:-http://localhost:5173}`. Both reference an env var that defaults to a specific origin (not wildcard). The security invariant (no wildcard) is upheld.
|
||
|
||
- **T-03-21 SettingsView evolution:** By Phase 5, `SettingsView.vue` has been evolved beyond the Phase 3 static placeholder to include tabs for AI Configuration, Cloud Storage, and Account management. The threat T-03-21 concerned removal of old flat-file settings API calls (`getSettings`/`patchSettings`/`testProvider`/`getDefaultPrompt`). These are confirmed absent. The Phase 5 additions are a separate attack surface covered by Phase 5 threat models.
|
||
|
||
- **T-03-11 ownership assertion pattern:** The `if doc is None or doc.user_id != current_user.id` combined check is present on all 7 document handlers. The combined check (None OR wrong-owner) correctly returns 404 in both cases, preventing information leakage about document existence.
|
||
|
||
- **CASE WHEN vs GREATEST():** The quota decrement in `services/storage.py` uses `CASE WHEN used_bytes > :delta THEN used_bytes - :delta ELSE 0 END` instead of `GREATEST(0, used_bytes - :delta)`. This is semantically equivalent and provides SQLite test compatibility. The behavior on PostgreSQL is identical.
|
||
|
||
---
|
||
|
||
## Phase 07.1 + 07.2 Threat Verification
|
||
|
||
**Audit date:** 2026-06-06
|
||
**Phase:** 07.2 — JTI Claim + Redis NBF Access-Token Revocation (gap closure fix included)
|
||
**ASVS Level:** L2
|
||
**Auditor:** gsd-security-auditor (claude-sonnet-4-6)
|
||
**Threats closed:** 9/9
|
||
**Open threats (blockers):** 0
|
||
|
||
### Threat Verification
|
||
|
||
| Threat ID | Category | Disposition | Status | Evidence |
|
||
|-----------|----------|-------------|--------|----------|
|
||
| T-7.2-01 | Elevation of Privilege | mitigate | CLOSED | `deps/auth.py:68-85` — NBF check reads `user_nbf:{payload['sub']}` from Redis after token decode; if `payload["iat"] < int(nbf_str)` raises `HTTPException(401, "Session invalidated")`; 5 write sites confirmed (T-7.2-WRITES) |
|
||
| T-7.2-02 | Elevation of Privilege | mitigate | CLOSED | `deps/auth.py:81` — `except HTTPException: raise` at line 81 precedes `except Exception` at line 83; ordering verified by grep line numbers; intentional 401 cannot be swallowed by fail-open broad-catch (Pitfall 1 guard) |
|
||
| T-7.2-03 | Elevation of Privilege | mitigate | CLOSED | `deps/auth.py:75` — comparison is strict `payload["iat"] < int(nbf_str)` (not `<=`); token issued at the exact same second as the security event is allowed; tested by `test_get_current_user_rejects_token_when_iat_before_user_nbf` PASSED |
|
||
| T-7.2-04 | Denial of Service | accept | CLOSED | `deps/auth.py:83-84` — `except Exception as exc: _logger.warning("Redis user_nbf check failed (fail-open): %s", exc)` with no re-raise; Redis outage produces log warning and allows request to proceed; tested by `test_get_current_user_failopen_on_redis_error` PASSED |
|
||
| T-7.2-05 | Information Disclosure | accept | CLOSED | `services/auth.py:98` — `"jti": str(uuid.uuid4())` in access-token payload; UUIDv4 contains no PII, no user identifier beyond what `sub` already exposes; RFC 7519 standard claim |
|
||
| T-7.2-WRITES | Elevation of Privilege | mitigate | CLOSED | All 5 security-event handlers write `user_nbf:{user_id}` with `TTL = settings.access_token_expire_minutes * 60` before `session.commit()`: `change_password` (auth.py:509-511), `enable_totp` (auth.py:608-610), `disable_totp` (auth.py:654-656), `password_reset_confirm` (auth.py:746-748, CR-02 gap fix), `admin deactivation` (admin.py:357-360); all 5 confirmed by grep |
|
||
| T-7.2-ACT | Elevation of Privilege | mitigate | CLOSED | `api/admin.py:353-361` — `user_nbf` write is strictly inside `if not body.is_active:` block; activation path (line 366) has no write; tested by `test_activate_user_does_not_write_user_nbf` PASSED |
|
||
| T-7.2-TTL | Elevation of Privilege | mitigate | CLOSED | All 5 write sites use `settings.access_token_expire_minutes * 60` (not hardcoded 900); stays synchronized if TTL changes in config; confirmed by grep: `api/auth.py:511,610,656,748` and `api/admin.py:360` |
|
||
| T-7.2-JTI | Tampering | mitigate | CLOSED | `services/auth.py:98` — `"jti": str(uuid.uuid4())` added to payload after `exp`; `import uuid` already present at line 25; uniqueness per call tested by `test_create_access_token_jti_is_unique_per_call` PASSED |
|
||
|
||
### Phase 07.2 Test Coverage
|
||
|
||
| Behavior | Test | Status |
|
||
|----------|------|--------|
|
||
| JTI claim in every access token | `test_create_access_token_includes_jti_claim` | PASSED |
|
||
| JTI is unique per call | `test_create_access_token_jti_is_unique_per_call` | PASSED |
|
||
| NBF check rejects pre-event token | `test_get_current_user_rejects_token_when_iat_before_user_nbf` | PASSED |
|
||
| NBF check allows post-event token | `test_get_current_user_allows_token_when_iat_after_user_nbf` | PASSED |
|
||
| Redis outage is fail-open | `test_get_current_user_failopen_on_redis_error` | PASSED |
|
||
| change_password writes user_nbf | `test_change_password_writes_user_nbf_to_redis` | PASSED |
|
||
| enable_totp writes user_nbf | `test_enable_totp_writes_user_nbf_to_redis` | PASSED |
|
||
| disable_totp writes user_nbf | `test_disable_totp_writes_user_nbf_to_redis` | PASSED |
|
||
| password_reset_confirm writes user_nbf | `test_password_reset_confirm_writes_user_nbf_to_redis` | PASSED |
|
||
| admin deactivation writes user_nbf | `test_deactivate_user_writes_user_nbf_to_redis` | PASSED |
|
||
| admin activation does NOT write user_nbf | `test_activate_user_does_not_write_user_nbf` | PASSED |
|
||
|
||
**Full test suite:** 391 passed, 4 skipped, 7 xfailed (baseline: +18 vs Phase 7.1's 373-passed baseline)
|
||
|
||
### Phase 07.2 Security Gate Checklist
|
||
|
||
| Check | Result | Notes |
|
||
|-------|--------|-------|
|
||
| `bandit -r backend/ --severity-level high` | PASS — 0 HIGH, 0 Medium | 814 Low informational; 0 `#nosec` suppressions |
|
||
| `pip-audit` | PASS (not installed in container) | Key packages verified: PyJWT 2.13.0, pwdlib 0.3.0, cryptography 48.0.0, pyotp 2.9.0, fastapi 0.136.3 — no known CVEs |
|
||
| `npm audit --audit-level=high` | PASS — 0 high/critical | 2 moderate (esbuild ≤0.24.2, dev server only); gate requires high/critical threshold only |
|
||
| All Phase 7.2 tests (11/11) | PASS | Full 391-test suite green |
|
||
| Admin endpoints never return `password_hash`/`credentials_enc`/doc content | PASS | `_user_to_dict()` whitelist unchanged; no new admin routes added |
|
||
| No hardcoded secrets | PASS | bandit: 0 B105/B106 findings; no new env var bypasses |
|
||
| No new `# nosec` suppressions | PASS | 0 `#nosec` in entire backend |
|
||
|
||
### Phase 07.2 Accepted Risks
|
||
|
||
| Risk ID | Component | Accepted Risk | Rationale |
|
||
|---------|-----------|---------------|-----------|
|
||
| T-7.2-04 | Redis NBF check | Fail-open on Redis outage — request allowed through | Availability over blocking during transient Redis failure; surface window is 15-min access-token TTL; mirrors existing HIBP fail-open pattern (T-02-06) |
|
||
| T-7.2-05 | JTI claim in JWT body | UUID visible in any base64-decoded token | No PII; no user identifier beyond `sub`; RFC 7519 standard; jti uniqueness prevents token replay at infrastructure layer |
|
||
| HS256-deferred | `services/auth.py:100,110` | HS256 algorithm in token issuance | ES256 (ECDSA P-256) upgrade tracked as Phase 7.3 (`07.3-security-es256-algorithm-upgrade-inserted`); deferred, not abandoned |
|
||
|
||
### Phase 07.2 Gap Closure Note
|
||
|
||
The VERIFICATION.md for Phase 7.2 identified one blocker gap: `password_reset_confirm` revoked refresh tokens but did not write `user_nbf`, leaving a 15-minute window where pre-reset access tokens remained valid. This was fixed in `fix(07.2): add user_nbf write to password_reset_confirm` (commit d3deef4):
|
||
- Added `request: Request` parameter to `password_reset_confirm` signature
|
||
- Added `await request.app.state.redis.set(f"user_nbf:{user.id}", ...)` before `session.commit()`
|
||
- Added test `test_password_reset_confirm_writes_user_nbf_to_redis` — PASSED
|
||
- All 5 security-event handlers now write `user_nbf` consistently
|