| 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 |
| Paid org feature gates stay behind entitlements | `scripts/lint-org-plan-boundary.sh` in `make ci` | PAYMENTS SP01 | |
| 39 |
|
| 40 |
## Input handling |
| 41 |
|
| 42 |
| Control | Enforced by | Sprint | |
| 43 |
|---|---|---| |
| 44 |
| Markdown sanitizer is the only `template.HTML(...)` user-content path | `scripts/lint-markdown-boundary.sh` in `make ci` | S25 + audit | |
| 45 |
| `goldmark`/`bluemonday` only in `internal/markdown` | same as above | S25 | |
| 46 |
| Body-size cap on auth POSTs | `middleware.MaxBodySize` | S05 | |
| 47 |
| URL parameter validation (e.g. `next=`) | `internal/security/openredirect` + tests | S35 | |
| 48 |
| File-extension validation on uploads | `internal/avatars` (avatars only path today) | S10 | |
| 49 |
|
| 50 |
## Outbound HTTP / SSRF |
| 51 |
|
| 52 |
| Control | Enforced by | Sprint | |
| 53 |
|---|---|---| |
| 54 |
| DNS resolve + IP block-list (RFC1918, loopback, ULA, CGNAT, …) | `internal/security/ssrf.IsForbiddenIP` + tests | S33 → S35 | |
| 55 |
| Dial-the-IP transport (defeats DNS rebinding) | `ssrf.Config.HTTPClient` | S33 → S35 | |
| 56 |
| No follow on 3xx (SSRF amplifier) | `ssrf.Config.HTTPClient.CheckRedirect` | S33 | |
| 57 |
| Scheme/port allow-list | `ssrf.Config.Validate` + tests | S33 → S35 | |
| 58 |
| Per-webhook signing (HMAC-SHA256) | `internal/webhook.SignSHA256` + tests | S33 | |
| 59 |
| Webhook secrets AEAD-encrypted at rest | `internal/webhook.SealSecret` | S33 | |
| 60 |
|
| 61 |
## Rate limiting & anti-abuse |
| 62 |
|
| 63 |
| Control | Enforced by | Sprint | |
| 64 |
|---|---|---| |
| 65 |
| Generalised counter table | `internal/ratelimit` + `rate_limits` migration | S35 | |
| 66 |
| `X-RateLimit-Limit/Remaining/Reset` headers + `Retry-After` | `ratelimit.Middleware` + `StampHeaders` | S35 | |
| 67 |
| Per-IP rate limit (anonymous) | `ratelimit.IPKey` + middleware | S35 | |
| 68 |
| Per-/24 signup throttle | `ratelimit.AllowSignupIP` (above) | S35 | |
| 69 |
| Repo-create throttle (10/hour/user) | `repos.Create` throttle hit | S11 | |
| 70 |
| Star/unstar throttle (100/hour/user) | `social` package throttle | S26 | |
| 71 |
| Email-bomb defense (per-email reset/verify caps) | `internal/auth/email` per-address gates | S05 | |
| 72 |
|
| 73 |
## Headers, cookies, CSP |
| 74 |
|
| 75 |
| Control | Enforced by | Sprint | |
| 76 |
|---|---|---| |
| 77 |
| Content-Security-Policy (default-src 'self', `frame-ancestors 'none'`, …) | `middleware.SecureHeaders` + tests | S02 | |
| 78 |
| HSTS (when TLS or trusted proxy) | `SecureHeaders` + `requestIsTLS` | S02 | |
| 79 |
| `X-Frame-Options: DENY` | `SecureHeaders` | S02 | |
| 80 |
| `Referrer-Policy: strict-origin-when-cross-origin` | `SecureHeaders` | S02 | |
| 81 |
| `Permissions-Policy` (deny risky surfaces) | `SecureHeaders` | S02 | |
| 82 |
| `Cross-Origin-Opener-Policy: same-origin` | `SecureHeaders` | S02 | |
| 83 |
| `Cross-Origin-Resource-Policy: same-origin` | `SecureHeaders` | S02 | |
| 84 |
| `X-Content-Type-Options: nosniff` | `SecureHeaders` | S02 | |
| 85 |
| Content-Type set BEFORE WriteHeader on 4xx/5xx paths | `auth.writeRetryAfter`, `render.Render` | S05 fix | |
| 86 |
|
| 87 |
## Secrets hygiene |
| 88 |
|
| 89 |
| Control | Enforced by | Sprint | |
| 90 |
|---|---|---| |
| 91 |
| No token-prefix patterns in non-exempt source | `scripts/lint-secret-logs.sh` in `make ci` | S35 | |
| 92 |
| URL-redactor strips `user:pat@host` from logged URLs | `internal/auth/pat` redactor | S08 | |
| 93 |
| AEAD key rotation procedure documented | `docs/internal/2fa.md` | S06 | |
| 94 |
| Webhook secret-decryption failure auto-disables hook | `webhook.Deliver` + `AutoDisableWebhook` | S33 | |
| 95 |
|
| 96 |
## Actions / CI runner |
| 97 |
|
| 98 |
| Control | Enforced by | Sprint | |
| 99 |
|---|---|---| |
| 100 |
| Workflow parser rejects unsupported `uses:` aliases and unknown keys | `internal/actions/workflow` + parser tests | S41a | |
| 101 |
| Event-derived expressions are tainted and never spliced directly into shell text | `internal/actions/expr` + runner env-binding tests | S41a/S41d | |
| 102 |
| Actions secrets encrypted at rest | `internal/actions/secrets` + secretbox round-trip tests | S41c | |
| 103 |
| Claim-time secret mask snapshots survive later secret rotation/deletion | `workflow_job_secret_masks` + runner API log tests | S41e | |
| 104 |
| Runner job API JWTs are short-lived and single-use | `runner_jwt_used`, `runnerjwt`, runner API replay tests | S41c | |
| 105 |
| Checkout JWTs are separate, repo-scoped, and read-only | git HTTP checkout-token tests | S41h | |
| 106 |
| Runner step containers drop privileges and capabilities | Docker engine argv tests + runner deploy runbook | S41d/S41e | |
| 107 |
| Runner bridge blocks direct-IP egress and workflow-supplied DNS bypasses | `deploy/runner-config/{dnsmasq,firewall}.j2` + deploy runbook smoke | S41e | |
| 108 |
| Logs are scrubbed runner-side and server-side | `internal/runner/scrub`, server log path tests, scrub metrics | S41e | |
| 109 |
| Actions observability alerts cover stale runners, queue depth, p99 regression, and scrubber health | Prometheus rules + `make audit-actions-ga` | S41h | |
| 110 |
| Full project CI dogfood remains canary-only until marketplace/toolchain gaps close | `docs/internal/actions-ga-readiness.md` + `make audit-actions-ga` | S41h | |
| 111 |
|
| 112 |
## Operator controls |
| 113 |
|
| 114 |
| Control | Enforced by | Sprint | |
| 115 |
|---|---|---| |
| 116 |
| `bootstrap-admin` CLI | `cmd/shithubd/admin.go` | S34 | |
| 117 |
| Force-archive / force-delete (admin) | `internal/web/handlers/admin/repos.go` | S34 | |
| 118 |
| Job retry/discard (admin) | `internal/web/handlers/admin/jobs.go` | S34 | |
| 119 |
| Audit-log viewer | `internal/web/handlers/admin/audit.go` | S34 | |
| 120 |
| Site-admin CLI bootstrap audit row | `audit.ActionAdminSiteAdminGranted` | S34 | |
| 121 |
|
| 122 |
## Future tightening (deferred) |
| 123 |
|
| 124 |
These are tracked here so they don't atrophy. Each is small enough |
| 125 |
to land in a follow-up sprint but didn't make S35's cut: |
| 126 |
|
| 127 |
- **Tighten CSP `script-src` to drop `'unsafe-inline'`.** The S02 |
| 128 |
default still accepts inline scripts because of the theme-flash |
| 129 |
avoider in `_layout.html`. Replace with `'sha256-…'` of the |
| 130 |
exact inline block (or move the script to an external file). |
| 131 |
- **Captcha integration on signup.** Per-/24 throttle is the |
| 132 |
current defense; captcha vendor decision (hCaptcha vs Cloudflare |
| 133 |
Turnstile) is deferred to a follow-up. The throttle is the gate |
| 134 |
the captcha plugs into. |
| 135 |
- **Authorization-scope lint for API routes.** Per-route scope |
| 136 |
decoration + a `scripts/lint-scopes.sh` that asserts every |
| 137 |
registered API handler declares its required scope. Deferred |
| 138 |
until the API surface grows past the current minimal shape. |
| 139 |
- **Open-redirect allowlist plumbing into handlers.** The validator |
| 140 |
exists (`internal/security/openredirect`); per-handler use sites |
| 141 |
(`?next=` consumers in auth, settings) will adopt it as the |
| 142 |
surfaces evolve. |