diff --git a/.planning/phases/05-files/05-SECURITY.md b/.planning/phases/05-files/05-SECURITY.md new file mode 100644 index 0000000..eae2851 --- /dev/null +++ b/.planning/phases/05-files/05-SECURITY.md @@ -0,0 +1,72 @@ +--- +phase: 05 +slug: files +status: verified +threats_open: 0 +asvs_level: 1 +created: 2026-05-15 +--- + +# Phase 05 — Security + +> Per-phase security contract: threat register, accepted risks, and audit trail. + +--- + +## Trust Boundaries + +| Boundary | Description | Data Crossing | +|----------|-------------|---------------| +| browser → FileUploadHandler | Multipart form body; attacker controls filename, file content, MIME hints, and Content-Length | Untrusted file bytes, attacker-supplied filename | +| FilesDeps.Files (FileStorer) | Nil-checked as the first statement of every file handler; returns 503 before touching request body | S3 credentials / objects | +| FileDownloadHandler → S3 presigned URL | URL generated server-side, 5-minute TTL; not stored in DB | Time-limited S3 object access | +| FileDeleteHandler → dual-system delete | S3 and Postgres are separate; S3 failure leaves orphan objects (T-05-03-04 accepted) | S3 object key, DB row | + +--- + +## Threat Register + +| Threat ID | Category | Component | Disposition | Mitigation | Status | +|-----------|----------|-----------|-------------|------------|--------| +| T-05-02-01 | Tampering | FileUploadHandler — filename | mitigate | `header.Filename` stored in DB as display string only; S3 key is `"files/{tablo_id}/{uuid}"` — filename never reaches S3 key (`handlers_files.go:185`) | closed | +| T-05-02-02 | Denial of Service | FileUploadHandler — upload size | mitigate | `http.MaxBytesReader(w, r.Body, maxBytes)` at `handlers_files.go:151` before `ParseMultipartForm` at line 153; `*http.MaxBytesError` detected, returns 422 with friendly message | closed | +| T-05-02-03 | Spoofing | FileUploadHandler — content-type | mitigate | Browser `Content-Type` ignored; `store.go:79` calls `http.DetectContentType(sniffBuf[:n])` on first 512 bytes and uses the result as the authoritative content type | closed | +| T-05-02-04 | Elevation of Privilege | TabloFilesTabHandler + FileUploadHandler — IDOR | mitigate | `loadOwnedTablo` called as first step in every handler; `GetTabloByID` query includes `WHERE id = $1 AND user_id = $2` filter (`tablos.sql.go:32`); non-owner gets 404 | closed | +| T-05-02-05 | Tampering | File upload routes — CSRF | mitigate | `gorilla/csrf` middleware applied globally via `auth.Mount` in `router.go:56`; `@ui.CSRFField(csrfToken)` rendered in `FileUploadForm` at `files.templ:43` | closed | +| T-05-02-06 | Denial of Service | TabloFilesTabHandler + FileUploadHandler — nil FileStorer | mitigate | `deps.Files == nil` guard is the first statement in `TabloFilesTabHandler` (`handlers_files.go:75`) and `FileUploadHandler` (`handlers_files.go:141`); returns 503 before any body read or DB call | closed | +| T-05-03-01 | Information Disclosure | FileDownloadHandler — presigned URL TTL | mitigate | `store.go:116` sets `o.Expires = 5 * time.Minute` in `PresignGetObject` options; URL not stored in DB; generated per-request in `FileDownloadHandler` | closed | +| T-05-03-02 | Elevation of Privilege | FileDownloadHandler + FileDeleteConfirmHandler — IDOR | mitigate | `loadOwnedTabloForFile` at `handlers_files.go:39` chains `loadOwnedTablo` (user_id filter) then `GetTabloFileByID` with `WHERE id = $1 AND tablo_id = $2` (`files.sql.go:31`) — two-layer ownership check | closed | +| T-05-03-03 | Elevation of Privilege | FileDeleteHandler — IDOR | mitigate | Same `loadOwnedTabloForFile` preamble; `DeleteTabloFile` parameterized with `{ID: file.ID, TabloID: tablo.ID}` at `handlers_files.go:306-309` — non-owner cannot delete with a valid file UUID | closed | +| T-05-03-04 | Denial of Service | FileDeleteHandler — S3 partial failure | accept | S3 delete failure logged at `handlers_files.go:302`; no early return — DB row always deleted; orphan S3 objects accumulate and are cleaned by Phase 6 worker. See Accepted Risks Log. | closed | +| T-05-03-05 | Tampering | FileDeleteConfirmFragment — CSRF on delete form | mitigate | `@ui.CSRFField(csrfToken)` present in `FileDeleteConfirmFragment` at `files.templ:118`; gorilla/csrf rejects POST without valid token | closed | +| T-05-03-06 | Denial of Service | FileDownloadHandler + FileDeleteConfirmHandler + FileDeleteHandler — nil FileStorer | mitigate | `deps.Files == nil` guard is the first statement in `FileDownloadHandler` (`handlers_files.go:242`), `FileDeleteConfirmHandler` (`handlers_files.go:269`), and `FileDeleteHandler` (`handlers_files.go:292`); returns 503 before any S3 or DB call | 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-05-01 | T-05-03-04 | S3 delete failure is logged but does not abort DB delete. Orphan S3 objects accumulate and are reconciled by the Phase 6 background worker (explicit design decision from 05-CONTEXT.md deferred items). Impact is storage waste, not data exposure or availability loss — any file whose DB row is gone is inaccessible to users regardless of S3 state. | Arthur Belleville | 2026-05-15 | + +--- + +## Security Audit Trail + +| Audit Date | Threats Total | Closed | Open | Run By | +|------------|---------------|--------|------|--------| +| 2026-05-15 | 12 | 12 | 0 | claude-sonnet-4-6 (automated) | + +--- + +## Sign-Off + +- [x] All threats have a disposition (mitigate / accept / transfer) +- [x] Accepted risks documented in Accepted Risks Log (AR-05-01 for T-05-03-04) +- [x] `threats_open: 0` confirmed +- [x] `status: verified` set in frontmatter + +**Approval:** verified 2026-05-15