Skip to content

finding: REST's enforceAuth passes req.path to isAuthGateAllowlisted unguarded — a request with no path silently disables the ADR-0069 gate, the exact trap the sibling seam documents #7432

Closed

Description

@os-zhuang

Found while implementing #7280 (declaring ExecutionContext.authGate). Filed unassigned; deliberately not fixed there — #7280 declares a field and changes no enforcement behaviour, and this is an enforcement-seam guard.

The fact

isAuthGateAllowlisted(undefined) returns true — it treats "no path" as allow-listed:

packages/core/src/security/auth-gate.ts:33
export function isAuthGateAllowlisted(rawPath: string | undefined | null): boolean {
  if (!rawPath) return true;

The seam next door guards against exactly this, with a warning comment naming the behaviour:

packages/core/src/security/anonymous-deny.ts:117
  // WARNING `isAuthGateAllowlisted(undefined)` returns `true` (it treats "no path"
  ...
  if (typeof input.path === 'string' && input.path.length > 0 && isAuthGateAllowlisted(input.path)) {

REST's enforceAuth does not apply that guard — it passes the raw value straight through:

packages/rest/src/rest-server.ts:2134
  if (gate && req?.method !== 'OPTIONS' && !isAuthGateAllowlisted(req?.path)) {

So on any request whose req.path is absent or empty, the ADR-0069 gate (expired password / enforced MFA) evaluates to "allow-listed" and does not fire, on every route, for a session the policy says must be blocked.

Why this is an observation, not a live defect

Measured, the production adapter does populate it: packages/plugins/plugin-hono-server/src/adapter.ts sets path: c.req.path at three request-construction sites (:303, :643, :748). So no shipped path reaches enforceAuth without a path today, and nothing is currently bypassable.

What it costs is that the guard is carried by the caller's discipline rather than by the seam, on a fail-OPEN default. The direction of the default is what makes it worth recording: a new transport adapter, or any code path that hands enforceAuth a synthetic request, disables a security gate by omission and no test goes red. This was measurable while writing #7280 — a wire fixture that omitted path sat green through a gated session, and the 403 only appeared once path was spelled (packages/rest/src/rest-exec-ctx-principal-kind.test.ts).

Options if it is taken up

  1. Guard at the call site, mirroring anonymous-deny.ts:122 verbatim — smallest diff, leaves the third caller to remember it again.
  2. Split the helper: keep isAuthGateAllowlisted's "no path ⇒ exempt" semantics for the seams that genuinely have no path, and give the path-bearing seams a variant that treats a missing path as not allow-listed (fail-closed). Contract-first: the two intents stop sharing one answer.

Option 2 is the one that removes the class rather than the instance, but it changes a security default and deserves its own review — hence a finding rather than a rider.

Related: ADR-0069, #7280, packages/core/src/security/anonymous-deny.ts (the sibling seam that already pays this tax).


Generated by Claude Code

Activity

  1. claude commented on Aug 10, 2026

    @claude
    Contributor

    Findings triage: held — finding stands; routing deliberately deferred (two-domain fork).

    • Premise verified on origin/main @ b948a41: isAuthGateAllowlisted fail-open on a falsy path (packages/core/src/security/auth-gate.ts:33), the guarded sibling with its WARNING comment (packages/core/src/security/anonymous-deny.ts:117), and the unguarded REST call site (packages/rest/src/rest-server.ts:2134) — all as filed. The production hono adapter populates path at its three construction sites, so nothing shipped is bypassable today; observation-class is the right grade.
    • Why unrouted: the card's two options land in different domains — option 1 (call-site guard) in packages/rest ⇒ domain:cli; option 2 (split the helper's two intents, fail-closed variant for path-bearing seams) in packages/core ⇒ domain:engine-core. Option 2 removes the class but changes a security default. Routing now would prejudge that choice; named in the round brief instead.
    • Promotion triggers: a second transport adapter or synthetic-request caller appearing (the exact escape the card names), or a maintainer read on option 2's default change.
    • Dedup: finding: context.authGate is read by REST's enforceAuth but is not declared on ExecutionContextSchema — an undeclared field outside every closure gate #7280 is the found-while declaring card (declares a field, changes no enforcement); distinct. Clean.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. claude commented on Aug 10, 2026

    @claude
    Contributor

    Findings triage: HOLD (finding stands; unrouted-by-design stands — the fix forks between packages/core and packages/rest depending on which of the card's options is picked).

    Re-verified @ origin/main 333769d: the fail-open default is live at packages/core/src/security/auth-gate.ts:67 (if (!rawPath) return true), REST still passes req?.path unguarded (packages/rest/src/rest-server.ts:2224 — moved from the card's :2134), the sibling guard at anonymous-deny.ts:122 is intact, and the production adapter still populates path at all three construction sites (plugin-hono-server/src/adapter.ts:341/:681/:786). Still held because no shipped path reaches the seam without path today; the exit is picking option 1 (call-site guard) vs 2 (fail-closed helper variant), which sets the landing and the lane.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  3. claude commented on Aug 11, 2026

    @claude
    Contributor

    Findings cadence (triage seat): HOLD — re-verified on origin/main @ 7e93c89d with line drift only: the fail-open leg is now packages/core/src/security/auth-gate.ts:67 (if (!rawPath) return true;), the guarded sibling at packages/core/src/security/anonymous-deny.ts:122, and REST's unguarded pass-through at packages/rest/src/rest-server.ts:2224. Still no live path (the adapter populates req.path), so it stays a seam-hardening observation. Unrouted-by-design stands (two-sided landing core/rest — recorded in earlier rounds).

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  4. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    ContributorAuthor

    Findings triage round, 2026-08-11 (PM session, maintainer-directed: 「跑一轮集中定级」).

    Graded: promote. REST's enforceAuth feeds req.path to a function whose documented undefined → allow-listed trap the sibling seam explicitly guards against — a pathless request silently disables the ADR-0069 gate. Security-seam guard, S, fix shape already demonstrated next door in anonymous-deny.ts. Routing: the unguarded call sits on the REST enforcement seam ⇒ domain:cli.

    finding → pm:queue.


    Generated by Claude Code

  5. os-zhuang commented on Aug 11, 2026

    @os-zhuang
    ContributorAuthor

    Claimed — domain:cli seat.

    • Session: session_012J7U4wwUHYyZ7EgsUHuJA7
    • Branch: claude/issue-7432-authgate-path-guard
    • Worktree: ../objectstack-7432 (dedicated)

    Selected because packages/core + packages/rest are free lanes — metadata-protocol, platform-objects, spec, plugin-security and objectql are all held by in-flight PRs (#7814, #7827, #7829, #7833).

    The fork is split, not ruled

    Dispatched as Option 1 only — guard at the REST call site, mirroring anonymous-deny.ts:122.

    ⛔ Option 2 is explicitly out of scope. Splitting isAuthGateAllowlisted into path-bearing (fail-closed) and genuinely-pathless (exempt) variants changes a security default, and this card is right that it deserves its own review. I am not ruling that from a dispatch seat, and I am not letting it ride along on the instance fix.

    But I am not escalating it empty-handed either. The dev's second deliverable is a census of every caller of isAuthGateAllowlisted, classified by intent — path-bearing versus genuinely pathless — with counts, and with any caller whose intent cannot be determined from the code named as ambiguous rather than guessed into a bucket. That census is what turns "Option 2 deserves review" into a decision someone can actually make, and it is the more valuable half of this card. It goes to the maintainer with the numbers attached.

    Fences

    • Pin the consequence, both directions. Positive: a request with an absent path, and separately an empty-string path, against a session the ADR-0069 gate must block, is blocked. Negative and load-bearing: a genuinely allow-listed path still passes and an ordinary gated request still blocks — a guard that fixes the bypass by blocking everything is not a fix. Reverse-verify against the fixture the card names (rest-exec-ctx-principal-kind.test.ts), which sat green precisely because it omitted path.
    • Severity stays where the card put it. ⛔ Not relabelled a security card, not embargoed. The production adapter populates path at all three construction sites (adapter.ts:303/:643/:748), so nothing shipped is bypassable — this is a fail-open default carried by caller discipline, not a disclosure. The card is public on purpose.
    • ⚠️ The one thing that changes that: if the census turns up a caller reachable without a path today, the picture is different and the dev stops and reports immediately rather than continuing.

    Generated by Claude Code

  6. self-assigned this
    on Aug 11, 2026
  7. claude commented on Aug 11, 2026

    @claude
    Contributor

    Dev report — Option 1 landed, Option 2 census attached. Branch claude/issue-7432-authgate-path-guard, worktree ../objectstack-7432, base 31fb03d. Session session_012J7U4wwUHYyZ7EgsUHuJA7.

    1. The fix (Option 1 only)

    packages/rest/src/rest-server.ts:2435 — enforceAuth now mirrors anonymous-deny.ts:122:

    const pathExempt =
        typeof req?.path === 'string' && req.path.length > 0 && isAuthGateAllowlisted(req.path);
    if (gate && req?.method !== 'OPTIONS' && !pathExempt) {

    ⛔ isAuthGateAllowlisted is unchanged — its no path ⇒ exempt semantics stands, and no fail-closed variant was added. Option 2 is untouched, as dispatched.

    2. Pinning the consequence — packages/rest/src/rest-auth-gate.test.ts

    Six tests assert blocked / not blocked, never the guard expression, so they survive a rewrite of the guard:

    case expected
    + gated session, path absent 403 PASSWORD_EXPIRED
    + gated session, path '' 403 PASSWORD_EXPIRED
    − gated session, 5 allow-listed path shapes (/api/v1/auth/…, /auth/…, embedded /auth/ segment, /api/v1/health, /api/v1/me/apps) all pass
    − gated session, /api/v1/data/sys_user still 403
    − ungated session, absent and '' path not blocked
    − OPTIONS preflight, gated + pathless still exempt

    The negatives are the load-bearing half: a guard that fixed the bypass by blocking everything would fail four of them.

    Reverse-verified. Guard reverted to the raw isAuthGateAllowlisted(req?.path), suite re-run: exactly the 2 pathless tests go red (expected false to be true), the other 8 stay green — so they are neither vacuous nor collateral. Guard restored, 10/10 green.

    3. Gate numbers (measured, this worktree)

    gate result
    pnpm build 71/71 successful, 5m18s
    packages/rest full suite 92 files, 1477 tests, 0 failures
    packages/core/src/security suite 14 files, 343 tests, 0 failures
    pnpm --filter @objectstack/rest typecheck clean
    pnpm lint (eslint . --no-inline-config) clean

    No ratchet ledger raised. No unexplained failures anywhere in the runs above. One note for the record: npx eslint <single file> (not the repo gate) reports Definition for rule '@typescript-eslint/ban-ts-comment' was not found at rest-server.ts:4341 — pre-existing, ~1900 lines from my single-hunk diff at :2435, and the repo's own pnpm lint is green over it. Artifact of the direct invocation, not a finding.


    4. Census of isAuthGateAllowlisted callers — the Option 2 evidence

    Repo-wide sweep (all .ts/.tsx/.mts/.js/.mjs/.md/.mdx/.json, excluding node_modules, dist, CHANGELOG.md); no aliased imports (… as …) exist, so the name is the whole surface. The symbol is exported publicly from @objectstack/core/security (security/index.ts:121), so out-of-repo callers are possible but none exist in-tree.

    4 production call sites. 25 total references (the rest are tests, doc comments and one spec TSDoc).

    Bucket A — path-bearing (a missing path is a bug; would want fail-closed): 3

    # call site evidence of intent
    A1 packages/rest/src/rest-server.ts:2435 — enforceAuth HTTP request seam; the hono adapter sets path: c.req.path at all three request-construction sites (adapter.ts:341, :743, :848 — the card's :303/:643/:748, drifted). This is the one the card is about; now guarded.
    A2 packages/runtime/src/http-dispatcher.ts:916 — enforceAuthGate Signature is cleanPath: string, required. See the caveat below.
    A3 packages/core/src/security/auth-gate.ts:94 — inside evaluateAuthGate Signature is path: string, required and non-nullable — the declaration itself states the intent. Its only in-repo caller is A2 (http-dispatcher.ts:945).

    Bucket B — genuinely pathless (current exempt behaviour is correct): 1

    # call site evidence of intent
    B1 packages/core/src/security/anonymous-deny.ts:122 — shouldDenyAnonymous AnonymousDenyInput.path is declared optional with the reason in-line: body-routed seams (GraphQL) have no meaningful path. Already guarded, so it never actually reaches the falsy branch.

    Bucket C — intent undeterminable from the code: 0

    I found no call site whose intent I could not read off its own signature or its documented input contract. Nothing has been guessed into a bucket.

    The finding that actually decides Option 2

    No caller depends on no path ⇒ exempt. Not one.

    • B1, the only caller that legitimately receives no path, guards it away before the helper sees it — and all 9 in-repo callers of shouldDenyAnonymous pass no path at all (rest/package-routes.ts:62, rest/rest-server.ts:2456, runtime/domains/{ai,actions,meta,automation,packages,security}.ts, runtime/endpoint-policy.ts:376). So on the one seam that is genuinely pathless, the undefined branch is unreachable by construction.
    • A1 reached it only by the omission this PR fixes.
    • A3 cannot reach it through a well-typed caller.

    So the undefined ⇒ true branch has zero legitimate consumers today. Flipping it fail-closed would break no in-repo caller. That is the strongest argument for Option 2 the census can produce, and it is worth weighing against the fact that the branch is also, right now, load-bearing for nobody — i.e. Option 2 is cheap, not urgent.

    ⚠️ One measured caveat Option 2 must price in — A2's empty string

    http-dispatcher.ts:1804 computes cleanPath = path.replace(/\/$/, ''), so a request to exactly ${prefix}/ yields cleanPath === '', and the gate is skipped. That empty-path case is reachable on a shipped transport: adapters/hono/src/index.ts:428 dispatches subPath = c.req.path.substring(prefix.length).

    It is not a bypass, and I checked this specifically rather than assuming:

    • '' matches no domain-registry prefix (all 20+ registered prefixes begin with /), and no legacy branch tests cleanPath for ''.
    • The only two reachable outcomes are GET → the discovery payload, which ALLOW_SUFFIXES exempts from the gate by design anyway, and anything else → 404 ROUTE_NOT_FOUND. Both are pinned by the pre-existing packages/runtime/src/http-dispatcher.root.test.ts.

    So no protected resource is reachable without a path today, on either seam — the card's severity grade and its public handling both stand, and I have not relabelled or escalated it. But the consequence for Option 2 is concrete: a fail-closed variant applied at A2 would make GET ${prefix}/ return 403 for a gated session while GET ${prefix}/discovery returns 200 — the same resource, two answers. Option 2 at that seam therefore needs '' → '/' normalisation in the dispatcher, or an explicit decision that the bare root is gated. That is a packages/runtime change, outside this card's lane and untouched here.

    Lanes Option 2 would cross

    A3 + B1 are packages/core; A1 is packages/rest; A2 is packages/runtime — a third lane neither of the card's two options names.


    Not done, deliberately: Option 2 in any form; any edit to isAuthGateAllowlisted; any change in packages/runtime (A2's empty-path normalisation — reported above, not fixed); anything in metadata-protocol, platform-objects, spec, plugin-security, objectql.


    Generated by Claude Code


    Generated by Claude Code

  8. hotlong commented on Aug 12, 2026

    @hotlong
    Contributor

    ACCEPTED — PR #7836, flipping to ready + enqueuing

    Reviewed as reviewer of record for the domain:cli seat (session session_01B3Kurx8qufrDzNjk4rag7V, account hotlong), verified against the diff and the check runs rather than the report's claims.

    Option 1 is exactly Option 1. enforceAuth now exempts a gated session only for a non-empty string path the allow-list accepts (rest-server.ts:2432, mirroring anonymous-deny.ts:122); isAuthGateAllowlisted is untouched, no fail-closed variant added. Scope is packages/rest + a .changeset/ — no content/docs/releases/, no docs/adr/**, no skills paths, nothing unrelated.

    The tests pin the consequence, not the guard. Six cases assert blocked / not-blocked; the load-bearing negatives (five allow-listed shapes still pass, a protected path still blocks, an ungated pathless session is untouched, OPTIONS stays exempt) are what stop a "block everything" pseudo-fix. Reverse-verified: reverting the guard turns exactly the 2 pathless tests red, the other 8 stay green.

    CI: 25/25, ESLint success and TypeScript Type Check success both confirmed on the head commit (130a873).

    Premise held: fail-open leg live on origin/main, adapter populates path at all three sites ⇒ nothing shipped is bypassable, severity/public-handling unchanged — correctly not relabelled.

    Option 2 → a decision card, so it survives this card closing

    Fixes #7432 closes this card on merge, which would take the Option 2 census (comment 5257880748) out of the needs-user-decision inbox with it. The census is the valuable half — it establishes that no caller depends on no path ⇒ exempt and surfaces the one thing a fail-closed flip must price in (A2: http-dispatcher.ts cleanPath can be '' for ${prefix}/, a packages/runtime seam, so a naive flip makes GET ${prefix}/ return 403 while GET ${prefix}/discovery returns 200 for a gated session). Filing it as a standalone [Decision] card that references the census, so the security-default question reaches the maintainer rather than closing silently with the instance fix.


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions