docs(phase-5): add security threat verification

This commit is contained in:
Arthur Belleville 2026-05-15 12:54:45 +02:00
parent 3a7726e6ed
commit 7fed6d1049
No known key found for this signature in database

View file

@ -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