docs(phase-04): add security threat verification — 41/41 threats closed
This commit is contained in:
curo1305
@@ -0,0 +1,127 @@
|
||||
---
|
||||
phase: 04
|
||||
slug: folders-sharing-quotas-document-ux
|
||||
status: verified
|
||||
threats_open: 0
|
||||
asvs_level: 2
|
||||
created: 2026-06-01
|
||||
---
|
||||
|
||||
# Phase 04 — Security
|
||||
|
||||
> Per-phase security contract: threat register, accepted risks, and audit trail.
|
||||
|
||||
---
|
||||
|
||||
## Trust Boundaries
|
||||
|
||||
| Boundary | Description | Data Crossing |
|
||||
|----------|-------------|---------------|
|
||||
| Client → folder endpoints | Untrusted folder name / parent_id; ownership asserted on every operation | Folder metadata (name, parent hierarchy) |
|
||||
| Client → share endpoints | Untrusted document_id, recipient_handle; ownership asserted; handle is exact-match only | Share grant / revoke |
|
||||
| Client → document proxy | Untrusted doc_id; ownership or active share required; bytes from MinIO through FastAPI only | Document bytes |
|
||||
| Client → audit log | Admin-authenticated; regular users blocked at dep level | Audit event metadata (no document content) |
|
||||
| Client → preferences | Any authenticated user; Pydantic Literal guards allowed values | pdf_open_mode setting |
|
||||
| Client → Range header | Untrusted byte range bounds validated by _parse_range() | Partial document bytes |
|
||||
| Frontend → /folders/:folderId route | Vue Router requiresAuth guard | Navigation token validation |
|
||||
| Admin → DELETE /api/admin/users | Must not delete admin accounts; MinIO objects cleaned before DB | User data + MinIO objects |
|
||||
| Celery worker → MinIO audit-logs | Service-to-service; env-var credentials; private bucket | Audit CSV |
|
||||
|
||||
---
|
||||
|
||||
## Threat Register
|
||||
|
||||
| Threat ID | Category | Component | Disposition | Mitigation | Status |
|
||||
|-----------|----------|-----------|-------------|------------|--------|
|
||||
| T-04-00-01 | Tampering | test stub files | mitigate | xfail(strict=False) on all Wave-0 stubs confirmed via git commit e007598; original test_folders.py had 28 xfail marks | closed |
|
||||
| T-04-00-02 | Information Disclosure | test fixture reuse | accept | Phase 3 conftest fixtures use ephemeral DB + mock MinIO; no real credentials in tests | closed |
|
||||
| T-04-02-01 | Tampering | migration 0004 GIN index | mitigate | Raw SQL `CREATE INDEX ix_documents_fts ON documents USING GIN(...)` with `# managed manually — do not autogenerate` comment; `migration 0004:56` | closed |
|
||||
| T-04-02-02 | Information Disclosure | audit-logs MinIO bucket | mitigate | Bucket creation gated on `os.environ.get("MINIO_ENDPOINT")` with no public policy set; `migration 0004:64` | closed |
|
||||
| T-04-02-03 | Tampering | put_object_raw caller-supplied key | accept | put_object_raw called only from trusted Celery task; key constructed from application logic, not user input | closed |
|
||||
| T-04-03-01 | Elevation of Privilege | POST/PATCH/DELETE /api/folders | mitigate | `Depends(get_regular_user)` on all 6 endpoints in folders.py; `folders.py:98,164,221,270,333,458` | closed |
|
||||
| T-04-03-02 | Information Disclosure | GET /api/documents?q= FTS scope | mitigate | `stmt = select(Document).where(Document.user_id == current_user.id)` applied before FTS clause; `documents.py:456` | closed |
|
||||
| T-04-03-03 | Tampering | DELETE /api/folders cascade | mitigate | Atomic CASE WHEN quota decrement `folders.py:409-410`; MinIO best-effort per-object try/except | closed |
|
||||
| T-04-03-04 | Information Disclosure | folder IDOR | mitigate | All folder ownership failures raise `HTTPException(status_code=404)`; `folders.py:234,238,280,284,349,353` | closed |
|
||||
| T-04-03-05 | Information Disclosure | PATCH /api/documents/{id}/folder cross-user folder | mitigate | Both doc and target folder ownership checked separately → 404; `folders.py:475,483-486` | closed |
|
||||
| T-04-03-06 | Tampering | folder name UniqueConstraint | mitigate | `IntegrityError` caught → `HTTP_409_CONFLICT`; `folders.py:126,138-141,306-309` | closed |
|
||||
| T-04-04-01 | Elevation of Privilege | POST /api/shares | mitigate | `Depends(get_regular_user)` on all share endpoints; `shares.py:89,167,217` | closed |
|
||||
| T-04-04-02 | Information Disclosure | Share IDOR DELETE | mitigate | `share.owner_id != current_user.id → raise HTTPException(404)`; `shares.py:274,315` | closed |
|
||||
| T-04-04-03 | Information Disclosure | GET /api/shares/received leaks extracted_text | mitigate | Return dict explicitly excludes extracted_text; comment `# T-04-04-03: extracted_text is intentionally excluded here`; `shares.py:237` | closed |
|
||||
| T-04-04-04 | Information Disclosure | Recipient quota modified by share | mitigate | No `UPDATE quotas` or `used_bytes` anywhere in shares.py; grep returns empty; comment confirms `T-04-04-04: No quota table is touched anywhere in this module`; `shares.py:13,222` | closed |
|
||||
| T-04-04-05 | DoS | Duplicate share flooding | mitigate | `IntegrityError` (UniqueConstraint on document_id+recipient_id) caught → `HTTP_409_CONFLICT`; `shares.py:95` | closed |
|
||||
| T-04-04-06 | Information Disclosure | Share reveals doc existence | mitigate | `doc.user_id != current_user.id → 404` on POST /api/shares; `shares.py:105` | closed |
|
||||
| T-04-05-01 | Broken Access Control | GET /api/documents/{id}/content admin access | mitigate | `Depends(get_regular_user)` on stream_document_content; `documents.py:742`; comment at 746 confirms intent | closed |
|
||||
| T-04-05-02 | Information Disclosure | Presigned URL exposure in proxy | mitigate | `file_bytes = await storage_backend.get_object(doc.object_key)` — no presigned URL call in handler; `documents.py:783`; comment at 780 confirms | closed |
|
||||
| T-04-05-03 | Information Disclosure | Range header bypass | mitigate | `_parse_range()` validates start ≤ end, start ≥ 0, end < file_size → `HTTP_416_RANGE_NOT_SATISFIABLE`; `documents.py:716-731` | closed |
|
||||
| T-04-05-04 | Information Disclosure | Non-recipient accessing shared doc | mitigate | `doc.user_id != current_user.id` then Share query `Share.recipient_id == current_user.id`; neither → 404; `documents.py:767-776` | closed |
|
||||
| T-04-05-05 | Tampering | pdf_open_mode mass assignment | mitigate | `pdf_open_mode: Literal["in_app", "new_tab"]` in PreferencesUpdate Pydantic model; `auth.py:740` | closed |
|
||||
| T-04-06-01 | Broken Access Control | GET /api/admin/audit-log | mitigate | `Depends(get_current_admin)` on all 4 audit endpoints; `audit.py:171,206,254,318` | closed |
|
||||
| T-04-06-02 | Sensitive Data Exposure | Audit log returning document content | mitigate | `_audit_to_dict()` whitelist: id, event_type, user_id, actor_id, resource_id, ip_address, metadata_, created_at — no filename/extracted_text; `audit.py:56-65` | closed |
|
||||
| T-04-06-03 | Information Disclosure | CSV export sensitive data | mitigate | CSV export uses `_audit_to_dict_with_handles()` — same whitelist plus user_handle/actor_handle only; `audit.py:82-93,375` | closed |
|
||||
| T-04-06-04 | Tampering | audit-logs MinIO bucket public | mitigate | `client.make_bucket("audit-logs")` with no policy call; MinIO private by default; `migration 0004:73-74` | closed |
|
||||
| T-04-06-05 | DoS | Unbounded CSV export | accept | Export scoped by date/user/event_type filters; admin-only endpoint; T-04-06-05 accepted risk | closed |
|
||||
| T-04-07-01 | Sensitive Data Exposure | login_failed logs email | mitigate | `metadata_=None` on auth.login_failed write_audit_log call; `auth.py:243`; email never logged | closed |
|
||||
| T-04-07-02 | Sensitive Data Exposure | document.uploaded has sensitive data | mitigate | `metadata_={"size_bytes": size, "storage_backend": "minio"}` — no filename, no extracted_text; `documents.py:379,386-391` | closed |
|
||||
| T-04-07-03 | Sensitive Data Exposure | credentials_enc in response | mitigate | `CloudConnectionOut` Pydantic model excludes credentials_enc by omission; `admin.py:167-176` | closed |
|
||||
| T-04-07-04 | Tampering | Admin deletes own account | mitigate | `if user.role == "admin": raise HTTPException(HTTP_400_BAD_REQUEST)` before any deletion; `admin.py:529-533` | closed |
|
||||
| T-04-07-05 | Information Disclosure | Orphaned MinIO objects after user delete | mitigate | `storage.delete_object(doc.object_key)` called best-effort for each document before `session.delete(user)`; `admin.py:555,582` | closed |
|
||||
| T-04-07-06 | Repudiation | Auth events not logged | mitigate | 9 write_audit_log calls in auth.py covering: login_failed, backup_code_used, login, logout, sign_out_all, password_changed, totp_enrolled, totp_revoked; `auth.py:236,285,300,403,430,512,599,633` | closed |
|
||||
| T-04-08-01 | Broken Access Control | /folders/:folderId unguarded | mitigate | `meta: { requiresAuth: true }` on both /folders/:folderId and /shared routes; `router/index.js:63,69`; `beforeEach` at line 81 enforces auth | closed |
|
||||
| T-04-08-02 | Information Disclosure | Access token in localStorage via new store | accept | All token handling in existing auth store + request() helper; folders and documents stores do not touch auth state | closed |
|
||||
| T-04-08-03 | Tampering | Debounced search < 2-char | mitigate | `if (newVal.length < 2)` check before API call fires; `documents.js:144`; 300ms debounce applied | closed |
|
||||
| T-04-09-01 | Information Disclosure | iframe src presigned URL | mitigate | DocumentPreviewModal calls `fetchDocumentContent(docId)` → `fetch('/api/documents/${docId}/content')` → blob URL; no presigned URL in request chain; `DocumentPreviewModal.vue:92,532` | closed |
|
||||
| T-04-09-02 | Information Disclosure | Share modal autocomplete reveals handles | mitigate | ShareModal: plain text input only, no API call on keypress, no autocomplete or suggestions endpoint; `ShareModal.vue:32-38,115-128` | closed |
|
||||
| T-04-09-03 | Broken Access Control | Audit log tab visible to regular users | mitigate | AuditLogTab rendered only inside AdminView behind tab guard; AdminView route guarded by `requiresAdmin: true` (inferred from beforeEach admin check); `AdminView.vue:24,33` | closed |
|
||||
| T-04-09-04 | Information Disclosure | CSV export URL params sensitive | accept | window.location.href params are filter values only (dates, event types, user IDs) — not auth tokens; auth via httpOnly cookie | closed |
|
||||
| T-04-SC (×9) | Tampering | npm/pip/cargo installs | accept | No new packages installed in plans 01-09; dependency surface unchanged | 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-04-01 | T-04-00-02 | Phase 3 conftest fixtures are ephemeral (in-memory SQLite + mock MinIO). No production credentials or real user data used in tests. Risk is negligible. | gsd-security-auditor | 2026-06-01 |
|
||||
| AR-04-02 | T-04-02-03 | put_object_raw is called exclusively from the Celery audit export task. The key is constructed from application-controlled date logic, not from user input. No user-facing path reaches this method. | gsd-security-auditor | 2026-06-01 |
|
||||
| AR-04-03 | T-04-06-05 | Unbounded CSV export is admin-only and scoped by configurable date/user/event_type filters. In the daily export task, a 24-hour time window bounds row count. Admin session is separately rate-limited by auth tier. | gsd-security-auditor | 2026-06-01 |
|
||||
| AR-04-04 | T-04-08-02 | The access token is stored in Pinia memory only (no localStorage). The new folders and documents stores import from the existing auth store; they do not introduce new storage paths. Existing security guarantee is preserved. | gsd-security-auditor | 2026-06-01 |
|
||||
| AR-04-05 | T-04-09-04 | CSV export URL contains only filter values (ISO dates, event type strings, UUID user IDs). No authentication material is embedded in the URL. The authenticated httpOnly cookie is sent automatically by the browser on the same-origin request. | gsd-security-auditor | 2026-06-01 |
|
||||
| AR-04-06 | T-04-SC (×9) | Nine plans in Phase 4 declared no new package installations. Dependency surface is unchanged from Phase 3 baseline, which passed pip audit and npm audit. | gsd-security-auditor | 2026-06-01 |
|
||||
|
||||
*Accepted risks do not resurface in future audit runs.*
|
||||
|
||||
---
|
||||
|
||||
## Unregistered Flags from SUMMARY.md
|
||||
|
||||
No unregistered threat flags. All SUMMARY.md `## Threat Flags` sections mapped to existing threat IDs or reported no new attack surface:
|
||||
|
||||
| SUMMARY | Threat Flags Reported |
|
||||
|---------|----------------------|
|
||||
| 04-01-SUMMARY | "No new security-relevant surface introduced — test files only" |
|
||||
| 04-03-SUMMARY | All 6 mitigations confirmed applied (per executor threat surface scan table) |
|
||||
| 04-07-SUMMARY | All 6 T-04-07-xx mitigations confirmed applied |
|
||||
| 04-08-SUMMARY | T-04-08-01 and T-04-08-03 confirmed applied |
|
||||
| 04-09-SUMMARY | T-04-09-01 through T-04-09-04 confirmed applied |
|
||||
|
||||
---
|
||||
|
||||
## Security Audit Trail
|
||||
|
||||
| Audit Date | Threats Total | Closed | Open | Run By |
|
||||
|------------|---------------|--------|------|--------|
|
||||
| 2026-06-01 | 41 | 41 | 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
|
||||
|
||||
**Approval:** verified 2026-06-01
|
||||
Reference in New Issue
Block a user