refactor(security): converge anonymous-deny into one shared function + source-enumerating ratchet (#2567 Phase 2)#2987
Merged
Conversation
…+ source-enumerating ratchet (#2567 Phase 2) Phase 1 gated every HTTP surface against the requireAuth posture, but each seam hand-rolled the same anonymous check. Phase 2 removes the duplication and pins the surfaces so a new ungated entry point fails CI. - New shouldDenyAnonymous in @objectstack/core (security/anonymous-deny.ts): the single anonymous-deny decision + shared 401 body/constants, mirroring the auth-gate.ts pattern. All five seams delegate to it — REST enforceAuth, dispatcher handleGraphQL/handleMetadata/handleAI, hono denyAnonymous. Pure refactor, no runtime behavior change; identity resolution and the dynamic exemptions (public-form grants, share-link tokens) are untouched. - discover() ratchet on the authz-conformance matrix: statically enumerates the data/meta/graphql HTTP entry points from source (curated per-file probes, control-plane excluded) and asserts each is classified by a covers key. A new route or a removed/stale covers now fails CI as UNCLASSIFIED/STALE. A negative test proves the ratchet bites. Guards the isAuthGateAllowlisted(undefined)===true trap: body-routed seams (GraphQL) pass no path, so the non-empty-path guard denies anonymous unconditionally rather than falling through to the control-plane allowlist. Verified: core security 114/114 (9 new), runtime requireauth gate 13/13, rest auth-gate 4/4, hono anonymous-deny 5/5, dogfood conformance 5/5 (ratchet bites) + anonymous-deny surfaces e2e 10/10, single-authz-resolver guard intact. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ShknNUYoSTspJLBiqorD8V
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
github-actions
Bot
added
size/m
documentation
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 35 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
The anonymous-deny import edit touched the import line, surfacing a pre-existing unused `IHttpServer` import (referenced only in comments). Remove it to clear the CodeQL finding. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01ShknNUYoSTspJLBiqorD8V
os-zhuang
marked this pull request as ready for review
July 16, 2026 01:08
This was referenced Jul 16, 2026
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
3 participants
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.
Phase 2 of #2567, built on merged Phase 1 (#2973). Pure refactor + test-infra — no runtime behavior change intended.
Why
Phase 1 gated every HTTP surface (REST
/data, dispatcher/graphql+/meta, raw-hono/data) against the secure-by-defaultrequireAuthposture, but each seam hand-rolled the same!userId && !isSystem → 401check, and the anti-regression guarantee was three conformance rows a human had to remember to keep in sync. Phase 2 converges the decision into one shared, tested function and makes the ratchet enumerate the surfaces from source, so a new ungated entry point fails CI instead of shipping open.Changes
1. One shared decision —
shouldDenyAnonymousin@objectstack/core(security/anonymous-deny.ts)Mirrors the established
auth-gate.tspattern: a pure decision function + shared 401 body/constants, so the seams "can never drift." All five seams now delegate to it:enforceAuth(rest-server.ts) — the ADR-0069 authGate branch is untouched; only the anonymous block delegates. Covers all ~50enforceAuthcall sites incl. the/metaguarded registrar.handleGraphQL/handleMetadata/handleAI(http-dispatcher.ts).denyAnonymous(hono-plugin.ts).Identity resolution and the dynamic exemptions (public-form grants, share-link tokens) are deliberately not moved — verified they run upstream and only ever hand the seam an already-resolved context (
publicFormGrant's route never callsenforceAuth; share-links build a system context downstream).2. A source-enumerating ratchet on the authz-conformance matrix (
authz-conformance.test.ts)checkLedgeralready supportsdiscover()+ per-rowcovers. A curated per-file probe table statically enumerates the data/meta/graphql HTTP entry points from source (control-plane routes excluded, but pattern-based within each file so a genuinely new/dataroute is auto-discovered) and asserts each is classified by acoverskey. A new/removed/stale surface now fails CI asUNCLASSIFIED/STALE. A companion negative test proves the ratchet bites (drop acovers→ fail; inject a fake route → fail; stale key → fail).Design trap guarded:
isAuthGateAllowlisted(undefined)returnstrue. A body-routed seam (GraphQL has no request path) must pass no path — the shared function's non-empty-path guard then denies anonymous unconditionally instead of falling through to the control-plane allowlist and silently reopening the hole. Locked by a unit test.Verification
@objectstack/coresecurity: 114/114 (9 newshouldDenyAnonymouscases incl. theundefined-path trap guard)http-dispatcher.requireauth: 13/13; restrest-auth-gate: 4/4; honohono-anonymous-deny: 5/5authz-conformance: 5/5 (soundness + the 3 ratchet-bites negatives + baseline)check:single-authz-resolverintact; lint clean on all changed files; changeset addedOut of scope (noted for a future pass): the dispatcher's
handleSecurityadmin gate uses the same inline shape but is not a #2567 data/meta/graphql surface, so it's left for a follow-up.🤖 Generated with Claude Code
https://claude.ai/code/session_01ShknNUYoSTspJLBiqorD8V
Generated by Claude Code