@@ -0,0 +1,125 @@ |
| 1 | +# Security checklist |
| 2 | + |
| 3 | +The S35 baseline. Each row names the control, the artifact that |
| 4 | +enforces it (test/lint), and the sprint that landed it. Update on |
| 5 | +every security-relevant change; the checklist is the canonical |
| 6 | +"what does shithub claim to defend against, and where do we prove it" |
| 7 | +document. |
| 8 | + |
| 9 | +## Identity & authentication |
| 10 | + |
| 11 | +| Control | Enforced by | Sprint | |
| 12 | +|---|---|---| |
| 13 | +| Argon2id password hashing | `internal/auth/password` + tests | S05 | |
| 14 | +| Common-password blocklist | `internal/passwords` + signup gate | S05 | |
| 15 | +| Per-IP login throttle | `internal/auth/throttle` + tests | S05 | |
| 16 | +| Per-IP signup throttle | `auth.throttleSignup` (S05 layer) | S05 | |
| 17 | +| Per-/24 signup throttle | `internal/ratelimit.AllowSignupIP` + tests | S35 | |
| 18 | +| TOTP 2FA + recovery codes | `internal/auth/totp` + tests | S06 | |
| 19 | +| TOTP secret AEAD-encrypted at rest | `internal/auth/secretbox` (chacha20poly1305) | S06 | |
| 20 | +| Session cookie `Secure; HttpOnly; SameSite=Lax` | `internal/auth/session/CookieStore` | S02 | |
| 21 | +| CSRF cookie `Secure; SameSite=Strict` | nosurf middleware config | S02 | |
| 22 | +| Session epoch invalidation ("log out everywhere") | `users.session_epoch` + middleware bump check | S05 | |
| 23 | +| Constant-time login (no timing oracle) | `password.Compare` HMAC-cmp | S05 | |
| 24 | +| Reset / verification token entropy = 32 bytes | `internal/auth/token` + tests | S05 | |
| 25 | +| Tokens hashed at rest | `token.HashOf` + sqlc storage | S05 | |
| 26 | + |
| 27 | +## Authorization |
| 28 | + |
| 29 | +| Control | Enforced by | Sprint | |
| 30 | +|---|---|---| |
| 31 | +| Single policy entrypoint (`policy.Can`) | `scripts/lint-policy-boundary.sh` in `make ci` | S15 + audit | |
| 32 | +| 404 (not 403) for non-admin `/admin` access | `middleware.RequireSiteAdmin` + tests | S34 | |
| 33 | +| Impersonation defaults read-only | `policy.Can` + `DenyImpersonationReadOnly` + tests | S34 | |
| 34 | +| Impersonation audit captures real + impersonated id | `admin.recordAdminAction` | S34 | |
| 35 | +| Org suspension blocks writes | `policy.Can` + `DenyOrgSuspended` | S30 | |
| 36 | +| Repo soft-delete blocks all actions | `policy.Can` + `DenyRepoDeleted` | S15 | |
| 37 | +| Author-self-close on issues/PRs | `policy.Can` author branch | S21/S22 | |
| 38 | + |
| 39 | +## Input handling |
| 40 | + |
| 41 | +| Control | Enforced by | Sprint | |
| 42 | +|---|---|---| |
| 43 | +| Markdown sanitizer is the only `template.HTML(...)` user-content path | `scripts/lint-markdown-boundary.sh` in `make ci` | S25 + audit | |
| 44 | +| `goldmark`/`bluemonday` only in `internal/markdown` | same as above | S25 | |
| 45 | +| Body-size cap on auth POSTs | `middleware.MaxBodySize` | S05 | |
| 46 | +| URL parameter validation (e.g. `next=`) | `internal/security/openredirect` + tests | S35 | |
| 47 | +| File-extension validation on uploads | `internal/avatars` (avatars only path today) | S10 | |
| 48 | + |
| 49 | +## Outbound HTTP / SSRF |
| 50 | + |
| 51 | +| Control | Enforced by | Sprint | |
| 52 | +|---|---|---| |
| 53 | +| DNS resolve + IP block-list (RFC1918, loopback, ULA, CGNAT, …) | `internal/security/ssrf.IsForbiddenIP` + tests | S33 → S35 | |
| 54 | +| Dial-the-IP transport (defeats DNS rebinding) | `ssrf.Config.HTTPClient` | S33 → S35 | |
| 55 | +| No follow on 3xx (SSRF amplifier) | `ssrf.Config.HTTPClient.CheckRedirect` | S33 | |
| 56 | +| Scheme/port allow-list | `ssrf.Config.Validate` + tests | S33 → S35 | |
| 57 | +| Per-webhook signing (HMAC-SHA256) | `internal/webhook.SignSHA256` + tests | S33 | |
| 58 | +| Webhook secrets AEAD-encrypted at rest | `internal/webhook.SealSecret` | S33 | |
| 59 | + |
| 60 | +## Rate limiting & anti-abuse |
| 61 | + |
| 62 | +| Control | Enforced by | Sprint | |
| 63 | +|---|---|---| |
| 64 | +| Generalised counter table | `internal/ratelimit` + `rate_limits` migration | S35 | |
| 65 | +| `X-RateLimit-Limit/Remaining/Reset` headers + `Retry-After` | `ratelimit.Middleware` + `StampHeaders` | S35 | |
| 66 | +| Per-IP rate limit (anonymous) | `ratelimit.IPKey` + middleware | S35 | |
| 67 | +| Per-/24 signup throttle | `ratelimit.AllowSignupIP` (above) | S35 | |
| 68 | +| Repo-create throttle (10/hour/user) | `repos.Create` throttle hit | S11 | |
| 69 | +| Star/unstar throttle (100/hour/user) | `social` package throttle | S26 | |
| 70 | +| Email-bomb defense (per-email reset/verify caps) | `internal/auth/email` per-address gates | S05 | |
| 71 | + |
| 72 | +## Headers, cookies, CSP |
| 73 | + |
| 74 | +| Control | Enforced by | Sprint | |
| 75 | +|---|---|---| |
| 76 | +| Content-Security-Policy (default-src 'self', `frame-ancestors 'none'`, …) | `middleware.SecureHeaders` + tests | S02 | |
| 77 | +| HSTS (when TLS or trusted proxy) | `SecureHeaders` + `requestIsTLS` | S02 | |
| 78 | +| `X-Frame-Options: DENY` | `SecureHeaders` | S02 | |
| 79 | +| `Referrer-Policy: strict-origin-when-cross-origin` | `SecureHeaders` | S02 | |
| 80 | +| `Permissions-Policy` (deny risky surfaces) | `SecureHeaders` | S02 | |
| 81 | +| `Cross-Origin-Opener-Policy: same-origin` | `SecureHeaders` | S02 | |
| 82 | +| `Cross-Origin-Resource-Policy: same-origin` | `SecureHeaders` | S02 | |
| 83 | +| `X-Content-Type-Options: nosniff` | `SecureHeaders` | S02 | |
| 84 | +| Content-Type set BEFORE WriteHeader on 4xx/5xx paths | `auth.writeRetryAfter`, `render.Render` | S05 fix | |
| 85 | + |
| 86 | +## Secrets hygiene |
| 87 | + |
| 88 | +| Control | Enforced by | Sprint | |
| 89 | +|---|---|---| |
| 90 | +| No token-prefix patterns in non-exempt source | `scripts/lint-secret-logs.sh` in `make ci` | S35 | |
| 91 | +| URL-redactor strips `user:pat@host` from logged URLs | `internal/auth/pat` redactor | S08 | |
| 92 | +| AEAD key rotation procedure documented | `docs/internal/2fa.md` | S06 | |
| 93 | +| Webhook secret-decryption failure auto-disables hook | `webhook.Deliver` + `AutoDisableWebhook` | S33 | |
| 94 | + |
| 95 | +## Operator controls |
| 96 | + |
| 97 | +| Control | Enforced by | Sprint | |
| 98 | +|---|---|---| |
| 99 | +| `bootstrap-admin` CLI | `cmd/shithubd/admin.go` | S34 | |
| 100 | +| Force-archive / force-delete (admin) | `internal/web/handlers/admin/repos.go` | S34 | |
| 101 | +| Job retry/discard (admin) | `internal/web/handlers/admin/jobs.go` | S34 | |
| 102 | +| Audit-log viewer | `internal/web/handlers/admin/audit.go` | S34 | |
| 103 | +| Site-admin CLI bootstrap audit row | `audit.ActionAdminSiteAdminGranted` | S34 | |
| 104 | + |
| 105 | +## Future tightening (deferred) |
| 106 | + |
| 107 | +These are tracked here so they don't atrophy. Each is small enough |
| 108 | +to land in a follow-up sprint but didn't make S35's cut: |
| 109 | + |
| 110 | +- **Tighten CSP `script-src` to drop `'unsafe-inline'`.** The S02 |
| 111 | + default still accepts inline scripts because of the theme-flash |
| 112 | + avoider in `_layout.html`. Replace with `'sha256-…'` of the |
| 113 | + exact inline block (or move the script to an external file). |
| 114 | +- **Captcha integration on signup.** Per-/24 throttle is the |
| 115 | + current defense; captcha vendor decision (hCaptcha vs Cloudflare |
| 116 | + Turnstile) is deferred to a follow-up. The throttle is the gate |
| 117 | + the captcha plugs into. |
| 118 | +- **Authorization-scope lint for API routes.** Per-route scope |
| 119 | + decoration + a `scripts/lint-scopes.sh` that asserts every |
| 120 | + registered API handler declares its required scope. Deferred |
| 121 | + until the API surface grows past the current minimal shape. |
| 122 | +- **Open-redirect allowlist plumbing into handlers.** The validator |
| 123 | + exists (`internal/security/openredirect`); per-handler use sites |
| 124 | + (`?next=` consumers in auth, settings) will adopt it as the |
| 125 | + surfaces evolve. |
