- CLAUDE.md: add Code Standards section with backend and frontend shared module maps, component architecture rules, duplication checklist, and no-dead-code enforcement rule - SECURITY.md: Phase 02 + 03 security audit results (all threats CLOSED) - .planning: update milestone audit, config, and add plan/UAT files for phases 01, 02-06, and 06.2-05 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
20 KiB
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.
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.vuehas 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.idcombined 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.pyusesCASE WHEN used_bytes > :delta THEN used_bytes - :delta ELSE 0 ENDinstead ofGREATEST(0, used_bytes - :delta). This is semantically equivalent and provides SQLite test compatibility. The behavior on PostgreSQL is identical.