fix(core)!: a banned user's credentials are refused while the ban holds, at every door - #30287

📓 Docs Drift Check

This PR changes 3 package(s): @objectstack/core, @objectstack/platform-objects, @objectstack/plugin-auth, touching 18 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/core/src/security/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

1 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

• content/docs/permissions/system-context.mdx (via mintPrincipalSession (symbol, a top-level function))

⛔ 1 release-owned page(s) also name something this change touched. These are read-only:

• content/docs/releases/v17/17-4.mdx (via organization_membership_ended (literal, a string literal in ApiKeyRefusalReason))

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.

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 44f85c01a8df9249b7969e1a69e431069523328d && git checkout 44f85c01a8df9249b7969e1a69e431069523328d
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin c1240a38ac2c23c8d526e476a20edeabbbefd6f6 f064a0e4180e2d9c4f3d5a98302552a615d3a6f7 && git checkout -B drift-repro c1240a38ac2c23c8d526e476a20edeabbbefd6f6 && git merge --no-ff f064a0e4180e2d9c4f3d5a98302552a615d3a6f7

node scripts/docs-audit/affected-docs.mjs --json c1240a38ac2c23c8d526e476a20edeabbbefd6f6

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs c1240a38ac2c23c8d526e476a20edeabbbefd6f6 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj

Contract review

Served-tier: CONTRACT_REVIEW_TIER
Head-sha: 15b303e0bfc3aaa636cded8c42e4e9171884c404
Local-runs: none
Scope: packages/core/src/security/api-key.ts, packages/core/src/security/user-ban.ts, packages/core/src/security/index.ts, packages/core/src/security/api-key-banned-owner.test.ts, packages/plugins/plugin-auth/src/agent-forward-identity-view.ts, packages/plugins/plugin-auth/src/principal-session-mint.ts, packages/qa/dogfood/test/api-key-banned-owner.dogfood.test.ts + packages/core/src/security/resolve-authz-context.ts, packages/core/src/security/request-execution-context.ts, packages/core/src/security/assemble-execution-context.ts, packages/core/src/index.ts, packages/runtime/src/security/resolve-execution-context.ts, packages/runtime/src/domains/mcp.ts, packages/runtime/src/domains/data.ts (and every runtime domain's shouldDenyAnonymous call), packages/mcp/src/plugin.ts, packages/rest/src/rest-server.ts, packages/services/service-storage/src/storage-service-plugin.ts, packages/types/src/driver-error-classification.ts, packages/plugins/plugin-auth/src/admin-ban-endpoints.ts, packages/plugins/plugin-auth/package.json, better-auth 1.7.12 packages/better-auth/src/plugins/admin/admin.ts (the vendor sign-in hook the predicate mirrors), scripts/check-adr-0087-registration.mjs, docs/adr/0087-metadata-protocol-upgrade-contract.md, scripts/adr-anchors/packages__plugins__plugin-auth__src__agent-forward-identity-view.ts.json. Prose checked sentence by sentence in ③ only: .changeset/banned-user-credentials-refused.md, content/docs/permissions/authentication.mdx, content/docs/permissions/administrator-guide.mdx.

Face: the committed changeset and the PR body both carry Clause-②: yes (narrowing); reviewFace reads a (narrowing) arm on either value as the narrowing face, so this record is at CONTRACT_REVIEW_TIER and --face declaration does not apply. Read-only throughout: diff, PR body, head check-runs, cited code; no worktree, build, test or gate run.

Check-runs read on this head (two readings, the second at the end of the review): every completed run is success — Lint & Repo Gates, Lint · ESLint, Lint · docs & code guards, Lint · repo & tooling gates, Lint · CI/release/engine gates, Lint · PM dispatch-gates, the three scoped lints, Type Check · source gates / consumer gates / debt ledger, Build Core, Build Docs, Build UI Deps, Build & E2E UI, Live E2E UI, Governed Surface Queue Guard, Check Changeset, Check PR Size, Check Documentation Links, PR-body guards, Spec property liveness, Dogfood Regression Gate, Plan Test Core shards, Auto Label, filter. Still in_progress at the last reading: Test Core (8 shards), Test UI (16 shards), Type Check UI, Type Check · workspace. No red. The known main red (permission-set-clone-door dogfood) touches permission-set-clone.ts grants on sys_comment / sys_activity; nothing in this diff reaches that path, so it is not attributable here.

① Derived judgments

• Accept-set narrowing (the rule). At the one key-admission seam (resolveApiKeyAdmission), after the key's own lifecycle verdict and before the tenancy and account-posture questions, the owner row is read by id as the system (id, banned, ban_expires) and a key whose owner's ban holds is answered on the refused arm with the new reason owner_banned: no principal, no fall-through to a session sent beside the key, nothing written (the use is not stamped). Verified in the tree: hashApiKey and the sys_api_key authentication read exist nowhere else at head (non-test, across packages/, ui/, examples/), so every door that resolves a key passes this check.
• Wire answer per door (judged from the cited code, not the PR's list): REST data door (rest-server.ts, anonymous: 'fail-closed') — no principal ⇒ its anonymous 401, byte-identical to an unknown key, WWW-Authenticate included (dogfood pin compares all four fields). Runtime dispatcher, MCP HTTP door included (resolve-execution-context.ts ⇒ resolveRequestExecutionContext, per request) — every domain whose first statement is shouldDenyAnonymous answers the same 401 for both; handleMcpRequest refuses with one message for both. MCP stdio — refused at start ("did not resolve to a valid identity …", one generic sentence) and, for a live transport, on the next call (resolvePrincipal re-resolves per read; initial is reused for localization only). Storage family gate reader — acts only on held; the download door's identity comes from the shared composition, so a refused key resolves nobody there. service-print, service-datasource admin routes, cloud-connection, plugin-sharing — all through resolveAuthzContext, same empty context as an unknown key. Server-side the reason is one warn line (reason, key row id, owner id; never the key or hash).
• Public surface widening. @objectstack/core (root barrel re-exports ./security/index.js) gains isUserBanInForce, USER_BAN_FIELDS, type UserBanFields; the published union ApiKeyRefusalReason gains the member 'owner_banned' (an exhaustive consumer switch on that type will need the new arm — covered by the yes arm of the declaration). ApiKeyAdmission shape is unchanged.
• plugin-auth's two existing ban checks now call the shared predicate. principal-session-mint.ts: the flag is read in two further spellings ('1', 'true') and a ban whose expiry equals the instant now still holds (previously lifted at equality) — one millisecond stricter, toward the vendor. agent-forward-identity-view.ts: the flag was read by truthiness and is now the closed set {true, 1, '1', 'true'}; equality at the instant now holds (previously lifted). Both rows arrive through the better-auth adapter as booleans, so the practical behaviour is unchanged; the direction of every change is toward the vendor's own rule. Both call sites keep their own refusal text.
• Predicate parity with the vendor (read at better-auth 1.7.12, session.create.before): flag set and no expiry ⇒ holds; expiry strictly past ⇒ lifts; expiry unreadable ⇒ holds. The predicate matches on each. Two deliberate differences the PR states correctly: it is read-only (the vendor clears a lapsed ban on the row; here a lapsed ban simply stops binding) and its flag test is a closed set rather than truthiness.

② Semver level

• Changeset .changeset/banned-user-credentials-refused.md: @objectstack/core: patch, @objectstack/plugin-auth: patch; Clause-②: yes (narrowing) and Metadata-protocol: none at line start in both the changeset and the PR body; **BREAKING** banner; one ADR-0087 marker not-required (no-migration-prescription) with its why. Level follows the Metadata-protocol kind alone: none ⇒ patch on both moved packages — correct. The body prescribes no code rewrite (the remedy is operational: lift the ban or mint a key under another user), so the no-migration-prescription category stands; Check Changeset and the repo/tooling lint lane that carries the ADR-0087 gate are green on this head.
• Packages moved by the diff: core and plugin-auth only; the dogfood package is private and takes no changeset. No spec change, no generated artefact, no ui/ consumer of the changed exports (git grep of the three new names and of isBanned under ui/ at head: none).
• ADR-0087 D3 obligations: nothing at rest changes, no authorable key moves, so no conversion or migration-chain entry is owed; the disposition says so in one line. ADR-0090 D10 rule 5 anchor on agent-forward-identity-view.ts (scripts/adr-anchors/…) still holds — the invariant "none for a banned or missing user" is implemented by the shared predicate, and the anchored ADR marker remains in the file header.

③ Boundary flags

PR-body open questions, each answered or escalated:

• Revoked key answers 403 API_KEY_REVOKED (distinguishable) while the banned owner's key matches the unknown key's 401 — answered: the lifecycle arm is distinct by ruling A on the lifecycle card; the ban is an owner question and takes the generic answer, as the org-less refusal does. Correct ordering (lifecycle first) is read in the tree and pinned.
• The inactivity window keeps running through a ban — answered in the body; prose flag (should-fix, see ④ item 3): the changeset's "Lifting the ban (or letting it expire) admits the same keys again, with nothing reissued" and authentication.mdx's "restoring … its API keys" are unqualified; a key idle past the inactivity window during a long ban answers as inactive after the unban. One clause fixes both.
• The ban flag is read whether or not the admin plugin is on — true of the tree (the seam reads the row; nothing conditions on the plugin).
• Whether a ban also revokes keys — escalated as a maintainer question by the rule itself; the PR correctly does not decide it.
• A key whose owner row is absent keeps today's answer — stated in the body and pinned; escalated (④ item 2): now that the owner row is read on every admission, admitting a credential whose owner no longer exists is a choice the maintainer should make explicitly.
• Not raised by the PR, noted here as a boundary (not a finding): a banned user's non-credential execution paths (an automation runAs: 'user' through resolveUserAuthzGrants) are outside "credentials" and untouched; a maintainer question if the ban is meant to hold the user's actions as well as their credentials.

Prose, sentence by sentence against the diff:

• Changeset title and rule paragraph — true: every key resolution passes the seam; the REST and MCP doors answer the generic 401; a ban with an expiry stops binding on its own (no sign-in needed; the predicate is read-only). "At every door that accepts a key … answered as a key that does not exist" — true on every face whose first statement denies anonymous callers; see ④ item 1 for the one face class where the two answers can differ. "No key row is revoked or rewritten" — true (stampAdmittedKey sits below the refusal; pinned). "Who moves" and "The remedy" — true.
• authentication.mdx ban-user / unban-user lines — true, subject to the inactivity-window qualification above.
• administrator-guide.mdx "Ban / Unban (blocks sign-in, ends the user's sessions, and refuses their API keys while the ban holds)" and the offboarding row — true: the admin ban endpoint deletes the user's sessions as the vendor handler does, and the key refusal is read in the tree.
• Disclosure (security-findings.md §2/§6): PR title, body, commits, changeset, docs and both test files stay at rule level — they state what a ban holds and what each door answers, never how a door could be made to misbehave; test names are rules. The ablation paragraph records a mutation and a red count, not an attack. Meets the rule; nothing in the public text needs withdrawing.

④ Design judgment

The narrowing is judged against the published spec on main and the cited doors: it makes a refusal the spec already implies (a ban "holds every credential") real at the one seam every key passes, without taking any declared capability away from a consumer whose owner is not banned (controls: peer key served, non-banned owner admitted, cookie-only session unchanged). The ordering is right (a dead key answers for its own state first; the ban before tenancy and posture, so a banned owner's key never reaches the held arm or the stamp). Failure modes are the existing ones: an owner read that throws is the branded 503, a store with no sys_user table holds no ban, no auth service means no policy but the row's own facts. Timed bans bind until strictly past the expiry and not after, exactly as sign-in; nothing is cached between admissions (the request-grants memo is scoped to one resolution and sits below admission; the stdio transport re-resolves per call). Findings:

1. Should-fix — guest-serving faces. On the runtime dispatcher (composed with anonymous: 'guest'), an unknown key with no session is served the guest anchor's grants, while a refused key returns before the guest block and is assembled as a guest entry with no grants. Every domain that opens with shouldDenyAnonymous answers both the same 401, so the pinned doors hold; but a face that admits a guest for some method/path and then reads the guest's grants answers the two keys differently whenever the guest anchor grants anything — the refused key gets less (fail-closed), yet the difference is observable. This is the same shape the existing org-less refusal already has, so it is not a regression; it is the one place where the PR's "answered as a key that does not exist at every door" is narrower than written. Fix in a follow-up: on the refused arm resolve the same guest entry an unknown key resolves, or qualify the claim to anonymous-denying faces.
2. Should-fix (maintainer question) — absent owner row. The seam now reads the owner row on every admission and still admits a key whose owner row does not exist. Keeping today's answer is a legitimate scope boundary and the PR says so; it should be an explicit decision rather than an inherited default.
3. Should-fix — one prose clause. Changeset and authentication.mdx: qualify "admits the same keys again" with the inactivity window (a key idle past the window during a long ban answers as inactive after the unban, remedy a new key), which the PR body already states.
4. Observations, no action: the predicate's flag test is a closed set where the vendor uses truthiness (equivalent for boolean columns; a non-canonical truthy spelling would read as not banned); a refused key's timing class differs from an unknown key's by the owner read and the policy read, as the org-less refusal's already does; isMissingTableError recognises a missing table, not an unregistered object — a kernel registering sys_api_key without sys_user would answer 503 on every key, a composition that does not exist (both ship together in platform-objects) and the same classification tryFind already applies.

No must-fix: the rule holds at every door that resolves a key, the refusal is indistinguishable from an unknown key on every door the PR names and on every anonymous-denying face, the key row is untouched, timed bans lapse as sign-in does, and the public text stays at rule level.

Implemented-by: claude/sec-hardening-r4
Reviewed-by: session_01QSeKYn5CmmdShCJifm6WYj

VERDICT: PASS