kite/SECURITY.md

# 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.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.