Skip to content

fix(runtime): consult anonymous-deny gate before /security's 503 - #7958

Merged
hotlong merged 2 commits into
mainfrom
claude/issue-7911-security-anonymous-deny-ordering
Aug 12, 2026
Merged

fix(runtime): consult anonymous-deny gate before /security's 503#7958
hotlong merged 2 commits into
mainfrom
claude/issue-7911-security-anonymous-deny-ordering

Conversation

@hotlong

Copy link
Copy Markdown
Contributor

Fixes#7911

The defect

handleSecurityRequest (packages/runtime/src/domains/security.ts) resolved the security service and returned 503 "Security service not available" for an empty or non-duck-typing slot — before it reached the !ec || shouldDenyAnonymous(...) gate ~20 lines below. So on a deployment where the security slot is empty or stubbed, an unauthenticated caller to /api/v1/security/suggested-bindings got a 503 capability disclosure instead of the 401 UNAUTHENTICATED refusal this handler's own comment calls unconditional (#2567, #3963).

/security stands on the same anonymous-deny floor as /data, /meta, /actions and /automation (ADR-0056 D2 → #3963). This was the last of the six dispatcher domains still ordered the wrong way — the sibling inversion on /ai/** was #7653, fixed in PR #7910.

The fix

Straight hoist, mirroring #7910's shape on domains/ai.ts: move the !ec || shouldDenyAnonymous(...) block above the resolveService probe. The !ec arm is documented (#4127 batch 3) as behaviour-preserving, so the hoist changes when the decision is made, not what it decides. No route-level auth: false opt-out exists on this domain, so there is a single consult site — simpler than /ai. All pre-existing explanatory comments are preserved, re-sited with the code they now sit next to.

Pins

  • Anonymous + empty/stubbed security slot → 401, asserting both error.code === 'UNAUTHENTICATED' and the HTTP status (ADR-0112).
  • Authenticated + empty slot → the 503 "Security service not available" answer is unchanged (the negative pin proving this is a hoist, not a deletion).
  • Serveable slot: still works for an authenticated caller, still denies anonymous.

Tests

New packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts (10 cases), mirroring ai-anonymous-deny-ordering.test.ts's group shape:

  • Group A: anonymous + empty/stubbed slot → 401 (list + confirm + dismiss routes).
  • Group B: authenticated caller (incl. internal SYSTEM context) + empty/stubbed slot → 503 unchanged.
  • Group C: serveable slot — anonymous still denied, authenticated caller still served (list/confirm/dismiss).
pnpm --filter '@objectstack/runtime^...' build → green
pnpm --filter '@objectstack/runtime' typecheck → clean
vitest run src/domains → 23 files, 461 tests passed

Reverse verification: reverted the hoist (git checkout origin/main -- packages/runtime/src/domains/security.ts), re-ran the new test file — the 4 Group A (401) cases failed with expected 503 to be 401 (right reason, not a compile error), Groups B/C stayed green. Restored the fix and re-ran green.

check:type-check-debt ratchet: re-measured @objectstack/runtime's TEST_DEBT the same way the gate does (sibling tsconfig.debt-remeasure.json lifting the test exclusion, tsc --noEmit) — 227 errors, unchanged from the recorded ledger value; the new test file adds zero.

Scope

Does not touch packages/rest/src/rest-server.ts (registerSecurityEndpoints is in flight on #7678, a different defect).


Generated by Claude Code

handleSecurityRequest resolved the security service and returned 503
"Security service not available" for an empty/non-duck-typing slot
BEFORE reaching the !ec || shouldDenyAnonymous(...) gate ~20 lines
below, so an unauthenticated caller to
/api/v1/security/suggested-bindings got a capability disclosure (503)
instead of the admin-surface refusal (401 UNAUTHENTICATED) this
handler's own comment calls unconditional (#2567, #3963).
Straight hoist, mirroring the #7653/#7910 fix on domains/ai.ts: the
gate now runs first and decides once. The !ec arm is unchanged
(documented #4127 batch 3 as behaviour-preserving) so this changes
WHEN the decision is made, not WHAT it decides. The 503 answer stays
unchanged for an authenticated caller against an empty/stubbed slot,
and a serveable slot still works authenticated and still denies
anonymous. No route-level auth: false opt-out exists on this domain,
so there is a single consult site.
Adds packages/runtime/src/domains/security-anonymous-deny-ordering.test.ts
(10 cases, mirroring ai-anonymous-deny-ordering.test.ts's group shape)
and a patch changeset.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@vercel

vercelBot commented Aug 12, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 12, 2026 9:12am

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/runtime.

20 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/api/client-sdk.mdx(via packages/runtime)
  • content/docs/api/index.mdx(via @objectstack/runtime)
  • content/docs/api/wire-format.mdx(via @objectstack/runtime)
  • content/docs/automation/hook-bodies.mdx(via @objectstack/runtime)
  • content/docs/concepts/metadata-lifecycle.mdx(via @objectstack/runtime)
  • content/docs/concepts/north-star.mdx(via packages/runtime)
  • content/docs/data-modeling/drivers.mdx(via @objectstack/runtime)
  • content/docs/deployment/index.mdx(via @objectstack/runtime)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/runtime)
  • content/docs/deployment/single-project-mode.mdx(via @objectstack/runtime)
  • content/docs/deployment/vercel.mdx(via @objectstack/runtime)
  • content/docs/getting-started/your-first-project.mdx(via @objectstack/runtime)
  • content/docs/kernel/cluster.mdx(via @objectstack/runtime)
  • content/docs/permissions/authentication.mdx(via @objectstack/runtime)
  • content/docs/permissions/authorization.mdx(via packages/runtime)
  • content/docs/permissions/system-context.mdx(via packages/runtime)
  • content/docs/plugins/packages.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/http-protocol.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/index.mdx(via @objectstack/runtime)
  • content/docs/protocol/kernel/lifecycle.mdx(via @objectstack/runtime)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/runtime)
  • content/docs/releases/v17.mdx(via @objectstack/runtime)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 12, 2026
@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

PM review — domain:cli seat (#6024). Verdict: accept, pending CI. Two notes, neither a change request.

The docs-drift advisory is a false positive — and it corroborates the fix

Checked with a control (a zero-hit grep proves nothing unless the scanner is known to work):

  • Target: Security service not available / suggested-bindings across content/docs/** → the only hand-written hit is permissions/permission-sets.mdx:202-204, which lists the three routes and says nothing about 503-vs-401 ordering. Nothing to update.
  • Control: UNAUTHENTICATED → hits 5 files. Scanner works, so the absence above is real absence.

Better than neutral, though: content/docs/releases/v15.mdx:815 already states this surface "hardcodes requireAuth: true, 401". So main has been shipping a release note that claims the 401 this handler was not delivering for an empty slot. This PR makes the code agree with a document that has been right all along. (That page is release-owned and read-only — recording the corroboration, not touching it.)

One fidelity note on the test double, for whoever copies it next

The header's claim — that makeDeps().error is the real builder the dispatcher wires, not a stub that drops the third argument — is right on the argument it is defending, and the reasoning about vacuous code assertions is exactly correct. It is worth knowing it is not general:

http-dispatcher.ts:295 wires error: (message, code, details) => this.error(message, code, details), and this.error (:690) is not a bare apiErrorResponse — for httpStatus >= 500 it first runs looksLikeInternalErrorLeak(message) and substitutes INTERNAL_ERROR_MESSAGE on a match. The double skips that step, and Group B asserts a 503 message — precisely the status class where the two could diverge.

Verified harmless today: looksLikeInternalErrorLeak('Security service not available') returns false on every arm of the predicate (packages/types/src/error-leak.ts:51-65) — no sqlite_, no sqlstate, no constraint failed / unique constraint / foreign key, and it does not start with select /update /insert into /delete from . So the double and production agree on this message, and the assertion is real.

The latent edge is only this: if that 503's wording ever changes to something the heuristic matches, Group B keeps passing while production ships Internal server error. Not worth a change here — noting it so the next test that copies this double for a 5xx path knows which axis it is thin on.

Everything the dispatch asked for is present

Hoist with all #4127 batch 3 reasoning preserved and re-sited; the authenticated-caller 503 pinned as the negative control that distinguishes a hoist from a deletion; anonymous refusal asserting codeand status; the isSystem arm covered; reverse verification reported with the right failure mode (expected 503 to be 401, not a compile error); and the check:type-check-debt ledger re-measured the way the gate measures it — @objectstack/runtime 227, unchanged.

Holding out of the merge queue until the queued checks finish.


Generated by Claude Code

@hotlongClaude

Copy link
Copy Markdown
ContributorAuthor

CI red — Test Core (2/3), one test. Diagnosed from the job log; patch round dispatched.

FAIL src/domain-handler-registry.test.ts:208
> /security responds 503 when no security service is wired (legacy in-handler semantics)
AssertionError: expected 401 to be 503

139 files / 2137 tests otherwise green.

This is a good failure. A pre-existing test pinned the exact behaviour this PR intentionally changes, and it caught it — which is the ratchet doing its job, not a defect in the fix. It was missed locally because the run was scoped to vitest run src/domains; the failing test sits one level up, in src/domain-handler-registry.test.ts. The changed file is exercised from outside its own directory.

On the repair, since the tempting fixes are both wrong. Read the test's name: its purpose is to prove the 503 originates inside the handler — it is not a statement about anonymous ordering, and it only ever reached that 503 anonymously as an artefact of the harness. The test immediately below it already covers anonymous denial against a wired service.

So: ⛔ not deleted, and ⛔ not re-pointed at 401 — that would collapse it into a duplicate of its neighbour and leave nothing in the suite proving the in-handler 503 path survives, which is precisely the negative control this change needs.

Instead it adopts a pattern already present and documented in the same file: dispatch() re-resolves identity from the mock, auth-less kernel and overwrites any seeded executionContext with an anonymous one (see the comment at :188-190 on the /notifications tests, which call the public delegate directly for this reason). Reaching the handler that way with an authenticated context lets the test prove what its name claims, without asserting the ordering this PR fixes. The name is being updated too — "legacy in-handler semantics" described a world where the 503 came before the gate; it now sits after it.

Ledger constraint restated for the patch round: @objectstack/runtime TEST_DEBT stays at 227. Staying draft until green.


Generated by Claude Code

CI caught this correctly: domain-handler-registry.test.ts's
"/security responds 503 when no security service is wired (legacy
in-handler semantics)" reached its 503 via dispatch(), which
re-resolves identity from the mock kernel and answers anonymously —
so after #7911's hoist the anonymous-deny gate now intercepts it
first and returns 401.
The test's NAME says its job is to prove the 503 comes from INSIDE
the handler, not to assert anonymous-vs-authenticated ordering (the
next test down already covers anonymous denial with a wired
service). Flipping the assertion to 401 would have destroyed that
purpose and left nothing pinning the in-handler 503 path #7911's
report calls out as the required negative control.
Repaired using the pattern already established in this file at
:188-190 for /notifications: call the public handleSecurity()
delegate directly with a seeded AUTHENTICATED executionContext,
bypassing dispatch()'s identity re-resolution, so the test proves
what its (renamed) name claims — no service wired => 503 from
inside the handler, once the gate has been cleared — without also
asserting the ordering #7911 just fixed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01B3Kurx8qufrDzNjk4rag7V
@hotlong
hotlong marked this pull request as ready for review August 12, 2026 09:33
@hotlong
hotlong added this pull request to the merge queueAug 12, 2026
Merged via the queue into main with commit 86d2e5eAug 12, 2026
26 checks passed
@hotlong
hotlong deleted the claude/issue-7911-security-anonymous-deny-ordering branch August 12, 2026 10:06
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

2 participants

@hotlong@claude