diff --git a/.planning/phases/03-document-migration-multi-user-isolation/03-SECURITY.md b/.planning/phases/03-document-migration-multi-user-isolation/03-SECURITY.md new file mode 100644 index 0000000..5f09e8b --- /dev/null +++ b/.planning/phases/03-document-migration-multi-user-isolation/03-SECURITY.md @@ -0,0 +1,97 @@ +--- +phase: "03" +slug: document-migration-multi-user-isolation +status: verified +threats_open: 0 +asvs_level: 2 +created: 2026-06-01 +--- + +# Phase 03 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| migration runtime → MinIO | DDL transaction holds DB rows; MinIO deletes happen outside any DB transaction | Object keys (UUIDs) | +| test fixtures → backend code | Fixtures fabricate JWTs that hit every guarded endpoint — must not leak across tests | Short-lived JWT tokens | +| browser → MinIO (presigned PUT) | Time-limited presigned URL; MinIO authenticates via HMAC; no Authorization header from browser | File bytes | +| browser → FastAPI /confirm | Authenticated user provides only document_id; FastAPI reads size_bytes from MinIO stat | Document ID | +| FastAPI /confirm → quotas table | Concurrent /confirm calls race against Quota row; atomic UPDATE WHERE prevents overflow | Byte counts | +| Celery beat → DB+MinIO | Runs as docuvault_app role; deletes only its own pending rows and MinIO objects | Document metadata, object keys | +| browser → /api/documents/* | Bearer JWT; ownership assertion gates every resource read/write | Document content, metadata | +| browser → /api/topics/* | Bearer JWT; namespace filter prevents cross-user topic enumeration | Topic labels | +| admin token → /api/documents/* | Admin role explicitly rejected 403 — cannot access document content | (blocked) | +| Celery task → users table | AI config resolved via doc.user_id → users row; no user-controlled provider input | AI provider/model config | +| admin → /api/admin/users/{id}/ai-config | Only admin write path to user.ai_provider / user.ai_model | AI provider selection | + +--- + +## Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation | Status | +|-----------|----------|-----------|-------------|------------|--------| +| T-03-01 | Tampering | `migrations/versions/0003_multi_user_isolation.py` | mitigate | `null_user_objects` collected via SELECT before DELETE; each `remove_object()` wrapped in `try/except Exception: pass` (lines 56–88) | closed | +| T-03-02 | Denial of Service | Alembic migration when MinIO unreachable | accept | See Accepted Risks Log | closed | +| T-03-03 | Information Disclosure | `tests/conftest.py` — xfail fixture JWTs | mitigate | `create_access_token` uses standard auth service with test secret; `async_client` clears `dependency_overrides` on teardown (line 155); no token values logged | closed | +| T-03-04 | Spoofing | `api/documents.py` — upload-url endpoint | mitigate | `object_key = f"{current_user.id}/{doc_id}/{uuid.uuid4()}{suffix}"` computed server-side (lines 112–113); filename stored in DB only; extension from `Path(body.filename).suffix.lower()` | closed | +| T-03-05 | Tampering | `api/documents.py` — confirm endpoint | mitigate | `size = await get_storage_backend().stat_object(doc.object_key)` from MinIO authoritative source (line 327); no size param in confirm body | closed | +| T-03-06 | Denial of Service | `api/documents.py` — concurrent /confirm at quota boundary | mitigate | Atomic SQL: `UPDATE quotas SET used_bytes = used_bytes + :delta WHERE … AND (used_bytes + :delta) <= limit_bytes RETURNING`; `fetchone() None` → HTTP 413 (lines 341–351) | closed | +| T-03-07 | Information Disclosure | Presigned URL leakage in logs | accept | See Accepted Risks Log | closed | +| T-03-08 | Repudiation | `tasks/document_tasks.py` — abandoned upload orphans | mitigate | `cleanup_abandoned_uploads` Celery beat task present (lines 132–177); `celery_app.py` beat_schedule runs every 30 min (lines 43–46) | closed | +| T-03-09 | Information Disclosure | `docker-compose.yml` — MinIO CORS | mitigate | `MINIO_API_CORS_ALLOW_ORIGIN: ${FRONTEND_URL:-http://localhost:5173}` — explicit non-wildcard origin (line 26) | closed | +| T-03-10 | Tampering | `storage/minio_backend.py` — Docker hostname in presigned URL | mitigate | Dual MinIO client: `self._client` for internal ops (stat/get/delete), `self._public_client` for `generate_presigned_put_url` (lines 54–60, 154, 169) | closed | +| T-03-11 | Information Disclosure | `api/documents.py` — cross-user doc access | mitigate | `if doc is None or doc.user_id != current_user.id: raise HTTPException(404)` at lines 322–323, 545–546, 579–580, 633–634, 702–703, 767 — returns 404 not 403 | closed | +| T-03-12 | Elevation of Privilege | `api/documents.py` — admin reading doc content | mitigate | `get_regular_user` raises 403 for admin role (deps/auth.py:95–109); `Depends(get_regular_user)` on all document handlers at lines 99, 143, 302, 416, 530, 557, 613, 688, 742 | closed | +| T-03-13 | Information Disclosure | `api/topics.py` — cross-user topic enumeration | mitigate | All queries filter: `or_(Topic.user_id == current_user.id, Topic.user_id.is_(None))`; `create_topic` scoped by `user_id=current_user.id` | closed | +| T-03-14 | Elevation of Privilege | `api/topics.py`, `api/admin.py` — regular user creating system topic | mitigate | `POST /api/admin/topics` uses `Depends(get_current_admin)` and creates `user_id=None`; regular `POST /api/topics` forces `user_id=current_user.id` | closed | +| T-03-15 | Tampering | `api/documents.py` — object_key forged with another user's UUID prefix | mitigate | `object_key = f"{current_user.id}/{doc_id}/{uuid.uuid4()}{suffix}"` — prefix always from `current_user.id`; no user-supplied prefix accepted | closed | +| T-03-16 | Spoofing | `api/documents.py` — anonymous traffic | mitigate | `HTTPBearer()` with `auto_error=True` raises 403 on missing header (deps/auth.py:35); `get_current_user` raises 401 on invalid/expired token (lines 52–55) | closed | +| T-03-17 | Elevation of Privilege | `/api/settings` endpoint | mitigate | `backend/api/settings.py` absent; `main.py` contains no `settings_router` reference — endpoint fully removed | closed | +| T-03-18 | Information Disclosure | `services/storage.py` — settings.json flat file | mitigate | No `load_settings`, `save_settings` function bodies present; settings.json no longer read or written; API keys in env only | closed | +| T-03-19 | Tampering | `tasks/document_tasks.py` — Celery task ai_provider injection | mitigate | Task signature is `document_id: str` only; `user.ai_provider` resolved inside `_run()` from DB lookup (lines 62–64) | closed | +| T-03-20 | Information Disclosure | `system_prompt` env var in container logs | accept | See Accepted Risks Log | closed | +| T-03-21 | Repudiation | `frontend/src/views/SettingsView.vue` — old API calls | mitigate | `getSettings`/`patchSettings`/`testProvider`/`getDefaultPrompt` absent from `api/client.js`; `SettingsView.vue` is static display only | closed | +| T-03-22 | Information Disclosure | `stores/documents.js` — XHR PUT Authorization header | mitigate | Only `Content-Type` header set via `setRequestHeader`; no `Authorization` header in `uploadToMinIO` helper (line 24–25, comment cites T-03-22) | closed | +| T-03-23 | Spoofing | `components/upload/UploadProgress.vue` — client-side quota from file.size | mitigate | All three values (`rejected_bytes`, `used_bytes`, `limit_bytes`) read from `item.quotaError` populated by 413 response body; no `file.size` used (lines 27, 30) | closed | +| T-03-24 | Denial of Service | Concurrent uploads exhaust browser memory | accept | See Accepted Risks Log | closed | +| T-03-25 | Tampering | `stores/documents.js` — upload state race condition | mitigate | `const rowKey = \`${file.name}__${Date.now()}\`` composite key prevents collisions on duplicate filename uploads (line 70) | closed | +| T-03-26 | Repudiation | `stores/auth.js` — quota refetch silent failure | mitigate | `fetchQuota()` wrapped in `try/catch`; `QuotaBar.vue` uses `v-if="!loadFailed"` to hide on error (auth.js:144–149) | closed | +| T-03-SC | Tampering | Package managers (pip/npm) — all 5 plans | mitigate | No new pip or npm dependencies added across all 5 plans; all packages were already pinned in Phase 1/2 requirements.txt and package-lock.json | closed | + +*Status: open · closed* +*Disposition: mitigate (implementation required) · accept (documented risk) · transfer (third-party)* + +--- + +## Accepted Risks Log + +| Risk ID | Threat Ref | Rationale | Accepted By | Date | +|---------|------------|-----------|-------------|------| +| AR-03-01 | T-03-02 | Alembic migration runs only after docker-compose health checks confirm MinIO availability. If MinIO is unreachable during migration, the migration exits without data loss (DB-side changes roll back; MinIO deletes are skipped). The deploy can be retried on next startup. Documented in upgrade() docstring. | orchestrator | 2026-06-01 | +| AR-03-02 | T-03-07 | Presigned PUT URLs have a 15-minute TTL and are single-use per object key. If leaked, the worst-case outcome is an attacker completing an already-authorized upload within the window. Full URL is never logged — only document_id is logged. Risk is low for v1. | orchestrator | 2026-06-01 | +| AR-03-03 | T-03-20 | SYSTEM_PROMPT is a static AI instruction string containing no PII, credentials, or user data. It is safe to appear in container logs. Not a sensitive env var. | orchestrator | 2026-06-01 | +| AR-03-04 | T-03-24 | XHR-based uploads stream bytes natively without buffering to JavaScript memory. Browser memory exhaustion from concurrent uploads is a user-driven concurrency issue acceptable for v1 scope. No concurrent upload limit enforced in the frontend. | orchestrator | 2026-06-01 | + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-06-01 | 27 (26 functional + T-03-SC) | 27 | 0 | gsd-security-auditor (sonnet) | + +--- + +## 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 + +**Approval:** verified 2026-06-01