Uh oh!
There was an error while loading. Please reload this page.
fix(identity): API keys are minted against the minter's active organization (#8287) - #8709
Draft
qq9340100 wants to merge 6 commits into
Draft
fix(identity): API keys are minted against the minter's active organization (#8287)#8709qq9340100 wants to merge 6 commits into
qq9340100 wants to merge 6 commits into
Conversation
…#8287) `sys_api_key` carried no organization, so under `OS_TENANCY_POSTURE=isolated` a minted key authenticated a user with no active organization and the Layer 0 wall (`organization_id = activeOrganizationId`) could match nothing: every org-scoped read answered `200` with `total 0` while the console went on offering minting. The column was absent by an inherited rule, not by oversight — `resolveInjectedSystemColumns` skips `managedBy: 'better-auth'` objects, and `sys_api_key` carries that flag even though better-auth's `apiKey` plugin is not loaded and the table is hand-rolled ObjectStack. - declare `active_organization_id` on `sys_api_key` (+ index, list columns) - register it as an ADR-0105 D7 managed extension field, and correct that registry's long-standing drift (its comment said every column here is an extension field; the set listed one) - mint (`POST /keys`) inherits the caller's active organization, re-checks membership against `sys_member` at mint time, and refuses under a walled posture rather than handing back a key that cannot read - the verifier reads ONE spelling (PD #12), refuses an org-less key under `isolated`, and the shared resolver fails an ex-member's key closed using the membership set it had already read — zero extra queries The column is deliberately NOT named `organization_id`: that name would make `sys_api_key` itself org-walled, hiding pre-existing org-less rows from their own owners while they keep authenticating under `group`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
…ts (#8287) Adding the dependency made `check:test-source-alias` and `check:type-source-resolution` both fire: a unit test or typecheck that reads a sibling's built `dist/` is a verdict about a build, not about the code in this checkout, and the dangerous direction is the one that PASSES. Anchored regex aliases (array form) so the bare entry cannot swallow the `/node` subpath. Also adds the changeset. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
Contributor
📓 Docs Drift CheckThis PR changes 5 package(s): 40 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also reference the affected code. These are read-only:
|
…ot the env (#8287) Fixes the red `Build Core` at 2b993c9 and, underneath it, a correctness bug the build failure exposed. The build break: `check:type-source-resolution` requires a cross-package type import to resolve to SOURCE, so adding `@objectstack/types` to `core` forced a `paths` rule — which collides with core's `rootDir: "./src"` under the tsup DTS build (TS6059). That gate's own header documents this exact cost. The bug it exposed is the more important half. `resolveTenancyPosture()` reads `OS_TENANCY_POSTURE`, which is what the operator ASKED for — not what is ENFORCED. Under ADR-0093 D4/D5 a deployment requesting `isolated` without the enterprise organizations runtime resolves to `single` and runs with no wall at all, so the env-reading version would have refused org-less API keys on a deployment that has no organization boundary to enforce. The posture is now an explicit input, resolved from the kernel's `tenancy` service — the same source plugin-security reconciles before handing a posture to the Layer 0 wall, so admission and the wall can never disagree. `core` drops the `@objectstack/types` dependency entirely, and both gates go quiet on their own rather than by registry widening. An ABSENT posture disables the two posture-conditional refusals, leaving behaviour exactly as before: that is a question about the deployment, not about the credential, so an unwired transport is never made worse — only less strict. Wired here: the runtime dispatcher/MCP path and the REST data API, which are the surfaces the card measured. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
#8287) The tsconfig `paths` and vitest `alias` entries were added to satisfy check:type-source-resolution / check:test-source-alias when core briefly depended on @objectstack/types. That dependency is gone — the tenancy posture now arrives from the kernel's `tenancy` service — so both entries resolve nothing, and their comments describe a `resolveTenancyPosture` call that no longer exists. Left in place they would mislead the next author and re-arm the TS6059 rootDir collision the moment anyone re-added the import. Both gates stay green without them, because the predicate is the IMPORT, not the file. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
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 freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
Fixes#8287
Implements the maintainer's option-2 minimal v1 ruling (comment
5281696102) as one integrated cross-lane PR, per the cross-lane designation (comment5293093118, 「同意」). The prior dev's STOP report (comment5287641437) was inherited rather than re-derived; its measurements are re-verified below and two are corrected.The defect
On
OS_TENANCY_POSTURE=isolated, a minted API key read nothing.sys_api_keycarried no organization column, so key auth established a user but no active organization — and theisolatedLayer 0 wall isorganization_id = activeOrganizationId, which with no active organization matches no row. Every org-scoped read answered200withtotal 0while the console went on offering minting. No cross-tenant leak; the failure was in the other direction.The column was absent by an inherited rule, not oversight:
resolveInjectedSystemColumnsinjectsorganization_idinto every object exceptmanagedBy: 'better-auth'ones, whichsys_api_keyis — even though better-auth'sapiKeyplugin is not loaded and the table is hand-rolled ObjectStack. Confirmed atpackages/spec/src/data/injected-system-columns.ts(rule 2 returnsnothing).The read side was already wired (also confirmed):
resolveApiKeyPrincipalalready read an organization intotenantId, andresolveAuthzContextalready adopted it. It was reading a column no mint path ever wrote.What changed, by lane
domain:metadataplatform-objects/src/identity/sys-api-key.object.tsactive_organization_id, index, list-view columnsdomain:metadataplatform-objects/src/apps/translations/*.objects.generated.tscheck:i18n)domain:identityplugin-auth/src/managed-extension-fields.tsdomain:cliruntime/src/domains/keys.tsdomain:engine-corecore/src/security/api-key.ts,resolve-authz-context.tsruntime/src/security/resolve-execution-context.ts,rest/src/rest-server.ts⛔
packages/specis not touched.active_organization_id, notorganization_id— please read this bitThis is the one place I departed from what the card and the prior dev's report assumed, and it is deliberate. The ruling says "the organization is recorded on the
sys_api_keyrow" and does not name the column.Semantics. This value is not "the organization that owns this row", it is "the organization this credential makes ACTIVE" — which is the ruling's own third clause.
sys_sessionalready carries exactly that fact under exactly that name, and both are read intoExecutionContext.tenantIdby the one shared resolver. One concept, one name.Measured consequence, which is the load-bearing half.
objectHasOrgIdField(plugin-securitysecurity-plugin.ts) tests the registered field set for the literalorganization_id, andcomputeTenantLayer0Filter(tenant-layer.ts) exempts objects without it. Naming the columnorganization_idwould therefore have madesys_api_keyitself org-walled, and both walled postures exclude NULL:groupthose keys still authenticate, so it becomes a live credential nobody can see or revoke;That is a fresh instance of the exact silent-empty class this card exists to remove.
sys_api_keyis an owner-scoped credential table likesys_user/sys_session/sys_account, scoped by the Layer 1sys_api_key_selfpolicy (user_id == current_user.id); keeping it there is what makes this fix additive rather than a trade. Pinned inplugin-security/src/tenant-layer.test.tsagainst the real field set, with the counterfactual.If the maintainer prefers
organization_id, it is a one-line rename plus accepting the Layer-0 consequences above.tenancyservice, never fromOS_TENANCY_POSTUREThe first push of this branch read the env var via
resolveTenancyPosture(). CI's redBuild Coreexposed that as wrong on two counts, and the second is the one that matters:check:type-source-resolutionrequires a cross-package type import to resolve to SOURCE, so adding@objectstack/typestocoreforced apathsrule that collides with core'srootDir: "./src"under the tsup DTS build (TS6059). That gate's own header documents this exact cost.OS_TENANCY_POSTUREis what the operator asked for, not what is enforced. Under ADR-0093 D4/D5 a deployment requestingisolatedwithout the enterprise@objectstack/organizationsruntime resolves tosingleand runs with no wall at all — so the env-reading version would have refused org-less API keys on a deployment with no organization boundary to enforce, breaking working automation to uphold a wall that does not exist.The posture is now an explicit input, resolved via
effectiveTenancyPosture()from the kernel'stenancyservice — the same sourceplugin-securityreconciles before handing a posture to Layer 0, so admission and the wall can never disagree.coredrops the@objectstack/typesdependency entirely and both gates go quiet on their own, not by registry widening.An absent posture disables both posture-conditional refusals, leaving behaviour exactly as before. That is deliberate: it is a question about the deployment, not about the credential, so an unwired transport is never made worse — only less strict. Wired here: the runtime dispatcher/MCP path and the REST data API, the two surfaces the card measured. Four other
resolveAuthzContextcallers (plugin-sharing, service-storage, service-settings, mcp stdio) are not yet wired and therefore keep today's behaviour — declared rather than silent.The three follow-through decisions
200 + total 0). At verify rather than revoke-on-event because membership ends through many paths — better-auth org endpoints, SCIM, a directsys_memberdelete, an ADR-0091 window lapsing — and a hook must catch every one or it silently misses. Zero extra queries:resolveUserAuthzGrantshas already readsys_memberfor this user. Pinned by a query-counting test. Scoped to walled postures: undersinglethere is no boundary to cross.single— no wall, left working.group— the wall derives from the owner's memberships independently oftenantId, so they already work there; left working.isolated— provably dead today, refused at verify time.isolateddoes break the one behaviour the card measured as working —GET /data/sys_userreturning the key owner's own row, which is walled by member-id enumeration rather than the org column. Decided: refuse.Distinguishability of the refusal — scoped down, and why
The decision asked for "a distinguishable error code". A new
error.codemust be registered inERROR_CODE_LEDGER, which lives inpackages/spec/src/api/error-code-ledger.zod.ts— off-limits to this seat. So the refusal uses the standard catalog memberUNAUTHENTICATED(401), which is also what that ledger's own admission rule prescribes: "If the condition is generic (not found / permission / validation / rate limit), use the standard catalog instead of registering a synonym" — and since #8211 that rule is mechanically enforced, so a synonym might well be refused admission anyway. The behavioural requirement is met in full: the failure is loud at call time (401) instead of200 + total 0. A machine-readable discriminator rides onResolvedAuthzContext.authRefusal.reason, deliberately lowercase so it can never be mistaken for a wire code. Filed for the spec seat as #8708.Verification
Union of derived gates run after the final commit, at
5f129e53f, clean tree — all green:check:authz-resolver·check:route-envelope·check:cross-package-test-inputs·check:kernel-hook-pairs·check:test-source-alias·check:type-source-resolution·check:changeset-gate-self-tests·check:objectui-changeset·check:i18n·check:query-options-erasure·check:type-check-coverage·check:nul-bytes·check:error-code-casing·check-adr-0087-registration·check-changeset-no-major·check-empty-changeset·check-cross-package-test-inputsGates the dispatch prompt did not name, surfaced by re-deriving against the actual changed paths:
check:authz-resolver,check:route-envelope,check:cross-package-test-inputs,check:kernel-hook-pairs,check:test-source-alias,check:type-source-resolution.Build, as CI runs it: 31/31 turbo build tasks green,
Build Coreincluded.Tests at
5f129e53f: core 803 · runtime 2379 · rest 1903 · plugin-auth 1204 · plugin-security 1138 · platform-objects 369 · mcp 185 — 7,981 passed, 0 failed.corehas notypecheckscript (pre-existing ledger entry); its types are checked by the tsup DTS build, which passes.Reverse verification (fix committed first, direction predicted before running): restoring the old
row.organization_id ?? row.organizationIdchain turned the canonical-read pins RED — 4 failed / 15 passed, exactly the spelling and org-read assertions — then restored from the commit and re-confirmed green.Fixture triage: four pre-existing fixtures spelled the retired column. All four were re-spelled rather than replaced — each merely used the alias, and each assertion still reads a value the mint path really produces. Two lived in
runtime, outside the packages this change edits, and were found by sweeping the verifier's consumption radius rather than the edited package.packages/platform-objectsi18n bundles were regenerated after mergingorigin/main(twice, most recently at5f129e53f), and #8149'ssys_emailrows were confirmed still present. Both regenerations were no-ops.Docs screened
All hand-written docs carrying API-key vocabulary were screened. Accurate and unaffected:
api/index.mdx,api/client-sdk.mdx,ai/agents.mdx,ai/connect-mcp.mdx,getting-started/build-with-claude-code.mdx,deployment/environment-variables.mdx. Screened, zero API-key vocabulary:permissions/authentication.mdx,permissions/system-context.mdx. Different subsystem (outbound connector auth, not inbound keys):automation/connectors.mdx,references/integration/connector.mdx.permissions/authorization.mdxdeserves its own line: it states thatsys_api_key"deliberately stay[s] public-posture … row scoping is their guard" — still true, and only because of the naming choice above.organization_idwould have falsified that sentence. Two pre-existing docs defects found and filed, not fixed here (#8715).Findings filed, not fixed here
sys_api_key.active_organization_id#8707 — audit rows are stamped from the actor's active organization in preference to the record's own, and the record-side fallback cannot seeactive_organization_id. The unimplemented half of follow-through decision 2: the decision stands, but its site isplugin-audit(a fifth lane) and the precedence flip touches every audited object. Landing only the narrow half would have been inert, i.e. declared-but-unenforced.UNAUTHENTICATED#8708 — whether a refused key deserves a registered error code, routed to thepackages/specseat.ApiKeyreference table documents better-auth's apiKey-plugin schema — a plugin this platform does not load and a shapesys_api_keydoes not have #8715 — theApiKeyreference table documents better-auth's apiKey-plugin schema (rate limits,enabled,metadata) for a plugin this platform does not load; plus ask_live_…key prefix in the HTTP-protocol example whereosk_is load-bearing.Generated by Claude Code