Repository navigation
Conversation
Three fixes to the unauthenticated surface: - Info disclosure: GET /api/system/stats (total user/content/media counts) and GET /api/system/env (which bindings/integrations are configured) now require authentication. /health, /info, /ping stay public for probes. - Rate limiting: POST /api/forms/:id/submit (20/min/IP) and POST /api/events (60/min/IP) are now per-IP rate-limited, bounding anonymous write amplification. (The limiter fail-open-skips when CACHE_KV is unbound, as before.) - Form-submit DoS: reject bodies over 512KB by Content-Length before parsing, and bound the recursive sanitizer (depth 32, 10k nodes) — a hostile deeply nested / oversized JSON payload now returns 413 instead of exhausting the stack or CPU. Deferred (noted in the review, not in this PR): stripping error.message from public read handlers (low-severity, shared error handling) and core account lockout (entangled with the security-audit plugin). Tests: api-system anon-401 cases; public-forms depth/node/size 413 cases; E2E spec 110. Full core suite green.
11 tasks done
…wn CI infra flake, unrelated to PR changes) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
6 tasks
Collaborator
|
Thanks for this security hardening PR @mmcintosh — the anonymous surface audit was really valuable. 🙏 Your A4 patch was cherry-picked into #1134 (with your attribution preserved) and expanded with additional ACL fixes and bootstrap hardening. That PR has been merged to Closing this in favour of the merged cherry-pick — it will ship in the next release. |
This branch was previously deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Hardens anonymous, unauthenticated endpoints: system info disclosure, missing rate limits on public writes, and a form-submit DoS.
Changes
GET /api/system/stats(includes total user count) andGET /api/system/env(reveals configured bindings) now userequireAuth()./health,/infoand/pingstay public.POST /api/forms/:identifier/submit20/min,POST /api/events60/min. As before, the limiter is skipped whenCACHE_KVis unbound.Content-Lengthbefore parsing (413).sanitizeDeep()is bounded to depth 32 and 10,000 nodes; exceeding either returns 413 and nothing is persisted.Breaking: anonymous callers now get 401 from
/api/system/statsand/api/system/env.Out of scope: stripping
error.messagefrom public read handlers; core account lockout.Testing
Unit Tests
api-system.test.ts(anonymous 401 on/statsand/env);public-forms.test.ts(deep nesting, too many fields and oversizedContent-Lengthreturn 413; normal submission still 200)E2E Tests
@smoke @api), anonymous 401 on stats/env,/healthstays public, oversized submit 413Checklist