feat: add CSRF protection, SSRF prevention, and login rate limiting #42
Reference in New Issue
Block a user
Delete Branch "security/csrf-ssrf-ratelimit"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Security Hardening
This PR implements three security hardening issues:
CSRF Protection (closes #35)
/pages,/sources,/source, and/userroutescsrf_tokenfield added to all 12+ POST forms in templates/webhook(inbound webhook POSTs) and/api(stateless API)SSRF Prevention (closes #36)
ValidateTargetURL()blocks private/reserved IP ranges at target creation time127.0.0.0/8,10.0.0.0/8,172.16.0.0/12,192.168.0.0/16,169.254.0.0/16,::1,fc00::/7,fe80::/10, plus multicast, reserved, test-net, and CGN rangesDialContextin the delivery engine for defense-in-depth (prevents DNS rebinding attacks)httpandhttpsschemes allowedLogin Rate Limiting (closes #37)
golang.org/x/time/ratePOST /pages/loginX-Forwarded-ForandX-Real-IPheader support for reverse proxiesFiles Changed
New files:
internal/middleware/csrf.go+ tests — CSRF middlewareinternal/middleware/ratelimit.go+ tests — Login rate limiterinternal/delivery/ssrf.go+ tests — SSRF validation + safe transportModified files:
internal/server/routes.go— Wire CSRF and rate limit middlewareinternal/handlers/handlers.go— Inject CSRF token into template datainternal/handlers/source_management.go— SSRF validation on target creationinternal/delivery/engine.go— SSRF-safe HTTP transport for productioncsrf_tokenfieldsREADME.md— Updated Security section and TODO checklistdocker build .passes (lint + tests + build).PR ready for review. Implements three security hardening issues:
/pages,/sources,/source,/userroutes. Excluded from/webhookand/api.POST /pages/loginusinggolang.org/x/time/rate.All existing tests pass. New tests added for all three features.
docker build .passes (lint + tests + build).✅ Review PASS — PR #42 (CSRF + SSRF + Rate Limiting)
All three security issues verified against requirements.
docker build .passes.#35 — CSRF Protection ✅
crypto/rand— correctsecureCompareusing XOR accumulation — correct (could usecrypto/subtle.ConstantTimeComparebut functionally equivalent)<input type="hidden" name="csrf_token">:login.html(1 form)navbar.html(2 forms — desktop + mobile logout)sources_new.html(1 form)source_edit.html(1 form)source_detail.html(7 forms — delete, add entrypoint, toggle/delete entrypoint, add target, toggle/delete target)/pages,/sources,/source/{sourceID},/user/{username}routes/webhook/{uuid}(inbound webhook POSTs) and/api/v1(stateless API)#36 — SSRF Prevention ✅
ValidateTargetURLchecks at target creation time;NewSSRFSafeTransportwith customDialContextvalidates resolved IPs at connection time (DNS rebinding defense)#37 — Login Rate Limiting ✅
golang.org/x/time/rate: 5 attempts per minute (rate = 5/60 ≈ 0.0833/sec, burst = 5) — arithmetic correctIntegrity Checks ✅
.golangci.ymlunchangeddocker build .(which runsmake check) passesmain(no rebase needed)golang.org/x/time v0.14.0properly added to go.mod/go.sum@clawbot pls fix conflicts
19e7557e88to7f4c40cacaRebased onto
mainto resolve merge conflicts from PR #41 merge.Conflict resolution:
internal/server/routes.go: Merged both middleware stacks —/pagesroute group now applies CSRF, MaxBodySize, and LoginRateLimit./sourcesand/source/{sourceID}groups retain CSRF, RequireAuth, and MaxBodySize.README.md: Combined security documentation from both PRs (security headers + body size limits from PR #41, CSRF/SSRF/rate-limiting from this PR). Updated package layout, security section, and TODO checklist.All tests pass. Docker build succeeds.
✅ Post-Rebase Review PASS — PR #42
Verified all three security features are intact after rebase onto main (which merged PR #41: security headers, session fixation, body limits).
docker build .passes.Middleware Stack Verification ✅
Both PR #41 and PR #42 middleware correctly present in
routes.go:/pages/pageslogin group/user/{username}/sources/source/{sourceID}/webhook/{uuid}No code lost or duplicated in conflict resolution.
#35 — CSRF Protection ✅
csrf_tokenhidden input/pages,/user,/sources,/sourceroutes/webhook(inbound) and/api(stateless)#36 — SSRF Prevention ✅
127/8,10/8,172.16/12,192.168/16,169.254/16,::1/128,fc00::/7,fe80::/100.0.0.0/8,100.64/10(CGN), multicast, reservedNewSSRFSafeTransport()re-resolves hostnames at dial time and validates resolved IPsHandleTargetCreate) + transport-level blocking at delivery time (Engine.client.Transport)#37 — Login Rate Limiting ✅
golang.org/x/time/ratetoken bucketAdditional Checks
.golangci.yml: unchanged ✅go.modadds onlygolang.org/x/time(required for rate limiter) ✅Minor Style Note (non-blocking)
secureCompare()incsrf.gois a hand-rolled constant-time comparison. Consider usingcrypto/subtle.ConstantTimeComparefrom stdlib for the same guarantee with less code. Not a functional issue — the current implementation is correct.use the stdlib comparison function and remove ours
Replaced the custom
secureComparefunction withcrypto/subtle.ConstantTimeCompare()from the standard library, as requested. Removed the custom implementation and its dedicated unit test (CSRF token comparison is still fully covered by the integration tests).docker build .passes.✅ Review PASS — PR #42 (post-rework: stdlib comparison)
Verified the rework requested by sneak ("use the stdlib comparison function and remove ours") and re-verified all three security features.
docker build .passes.Rework Verification ✅
secureComparefunction removedgrep -rn secureComparereturns nothingcrypto/subtle.ConstantTimeCompare()usedcsrf.go:79—subtle.ConstantTimeCompare([]byte(submitted), []byte(token)) != 1TestSecureCompareremoved#35 — CSRF Protection ✅
crypto/rand— correctcrypto/subtle.ConstantTimeCompare()for constant-time comparison — correct (stdlib, as requested)<input type="hidden" name="csrf_token">: login (1), navbar logout (2), source_detail (7), source_edit (1), sources_new (1)/pages,/sources,/source/{sourceID},/user/{username}routes/webhookand/api#36 — SSRF Prevention ✅
ValidateTargetURL()blocks private/reserved IPs at target creation (17 CIDR ranges including RFC 1918, loopback, link-local, CGN, multicast, IPv6 ULA/link-local)NewSSRFSafeTransport()with customDialContextre-validates resolved IPs at delivery time — prevents DNS rebindingTestIsBlockedIP_PrivateRanges(15 blocked + 4 allowed),TestValidateTargetURL_Blocked(10 URLs),TestValidateTargetURL_Allowed(3 URLs), plus scheme/host validation tests#37 — Rate Limiting ✅
loginRateLimit = 5,loginRateInterval = 1 * time.Minute— exactly 5/min per-IP as specifiedgolang.org/x/time/ratetoken bucket with per-IP trackingBuild ✅
docker build .passes (includesmake check— fmt, lint, tests, build).wtf did we write our own ratelimiting and csrf middlewares? these are standard web application tasks, isn't there some off the shelf stuff we could have used here given that we're using them in completely standard ways?
@clawbot this stalled
Acknowledged — previous rework agent stalled. Re-dispatched a new rework agent to replace custom CSRF and rate-limiting middleware with off-the-shelf Go libraries per your feedback. SSRF prevention stays custom (application-specific). Will update when complete.
@clawbot stalled again, because i keep restarting the gateway as we add a bunch of tool calls, and it keeps killing your subagents. please restart.
Understood — gateway restarts were killing the sub-agents. Re-dispatched rework agent (e13b484d). Working on replacing custom CSRF/rate-limiting with off-the-shelf libs now.
Rework: Replace custom CSRF/rate-limiting with off-the-shelf libraries
Addressed the review feedback by replacing both custom middleware implementations with well-established libraries:
CSRF:
gorilla/csrfv1.7.3csrf_tokenform field name — no template changes needed/pages,/sources,/source,/user) are protectedcsrf.PlaintextHTTPRequest()to skip the strict TLS Referer checkmiddleware.CSRFToken(r)wrapper preserved so handlers are unchangedRate Limiting:
go-chi/httpratev0.15.0httprate.KeyByRealIPfor IP extraction (handlesX-Forwarded-For,X-Real-IP,True-Client-IPheaders)Kept as-is
internal/delivery/ssrf.go) — application-specific by necessityOther changes
Session.GetKey()method to expose the raw 32-byte auth key for gorilla/csrf cookie signinggo mod tidyrun, all dependencies pinned ingo.sumdocker build .passes clean (lint + test + build)Net result: -191 lines of custom middleware code replaced by 2 well-established library calls.
✅ Review PASS — PR #42 (off-the-shelf library rework)
All three security features verified. Custom CSRF and rate-limiting code fully replaced with off-the-shelf libraries as requested.
docker build .passes. All tests pass.Rework Verification: Custom → Library Migration ✅
secureCompare,GenerateToken,crypto/randtoken gen, orvalidateCSRFin middlewaregorilla/csrfv1.7.3 integratedx/time/rate,rate.NewLimiter, IP maps, cleanup goroutinesgo-chi/httpratev0.15.0 integratedhttprate.KeyByRealIPssrf.gointact (appropriate — app-specific logic)#35 — CSRF Protection (gorilla/csrf) ✅
csrf.Protect()with session auth key for cookie signingcsrf.FieldName("csrf_token")— matches all 12 template hidden fieldscsrf.Secure(!m.params.Config.IsDev())— TLS enforcement in prod, relaxed in devcsrf.SameSite(csrf.SameSiteLaxMode)— correct cookie policycsrf.PlaintextHTTPRequest()wrapping to skip Referer check on HTTP/pages,/sources,/source/{sourceID},/user/{username}/webhook/{uuid}(inbound webhook POSTs),/api/v1(stateless)csrf_tokenhidden inputsCSRFToken(r)wrapper delegates tocsrf.Token(r)— handlers unchanged#36 — SSRF Prevention ✅
ValidateTargetURL()blocks 17 CIDR ranges (RFC 1918, loopback, link-local, CGN, multicast, reserved, TEST-NETs, IPv6 ULA/link-local)httpandhttpsNewSSRFSafeTransport()with customDialContextre-validates resolved IPs at dial time (DNS rebinding defense)HandleTargetCreate) + transport-level at delivery (Engine.client.Transport)TestIsBlockedIP_PrivateRanges(24 cases),TestValidateTargetURL_Blocked(10 URLs),TestValidateTargetURL_Allowed(3 URLs), scheme/host/init tests#37 — Login Rate Limiting (go-chi/httprate) ✅
httprate.Limit(5, 1*time.Minute, httprate.WithKeyFuncs(httprate.KeyByRealIP))— 5 POST/min/IPr.Use(s.mw.LoginRateLimit())Other Checks ✅
go test ./...)docker build .passesSession.GetKey()for CSRF signinggo.modgo directive bump 1.23→1.24