Uh oh!
There was an error while loading. Please reload this page.
fix(rest): carry the caller organization on the /meta write doors so audit rows are not stamped env-wide (#8805) - #8918
Conversation
) The REST `/meta` write doors passed no organization, so every `sys_metadata_audit` row a REST-authored metadata write produced was stamped `organization_id: null` (`recordMetadataAudit`: `entry.organizationId ?? null`). Composed with the scoped audit read — own-org rows PLUS env-wide ones, a limb that is required rather than optional — that left every REST-authored audit row readable by every tenant, carrying its actor, note, lock_state and request_id. The read half could not close it: the rows were genuinely unscoped. The organization comes from the execution context these doors already resolve (`resolveExecCtx`), threaded through `organizationIdForMetaWrite` — the same registry-derived predicate the runtime dispatcher uses. Threading the raw tenant would have traded the disclosure for an outage: the protocol REFUSES an org-scoped write of an `allowOrgOverride: false` type (`NOT_OVERRIDABLE`, 403), so every `PUT /meta/object/*` from a tenant-admin session would have started failing. Non-overridable types keep writing env-wide, which is their truthful audit scope. `publish` and `rollback` move with the save rather than after it: both resolve their row through the organization, so scoping the save alone would break the draft to publish loop. `GET /meta/:type/:name/published` is scoped for the same reason -- its previous comment justified omitting the organization by a symmetry ("this door resolves exactly the publishes this door can produce") that the write-side fix ends. `organizationIdForMetaWrite` / `declaresOrgOverride` move from `@objectstack/runtime` to `@objectstack/metadata-core` so both doors share one implementation: `@objectstack/rest` cannot import `runtime`, which depends on it. No new org-resolution seam is minted in `packages/rest`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NaS1PAHJcPfAA2acnV53Tn
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 3 package(s): 24 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 3 release-owned page(s) also reference the affected code. These are read-only:
|
Uh oh!
There was an error while loading. Please reload this page.
Fixes#8805
The REST
/metawrite doors passed no organization, so everysys_metadata_auditrow a REST-authored metadata write produced was stampedorganization_id: null(recordMetadataAudit:entry.organizationId ?? null). Composed with #8803's scoped read — own-org rows plus env-wide ones, a limb that is required rather than optional — that left every REST-authored audit row readable by every tenant, carrying itsactor,note,lock_stateandrequest_id. The read half could not close it: the rows were genuinely unscoped, so no filter could separate them.The premises were measured first, and both hold — one in a narrower form than the card assumed
The ruling on this card is explicitly premised, so this is the first deliverable.
Premise 1 — does
resolveExecCtxreliably carrytenantIdat this door? Yes, and stronger than "reliably": it is the same source the dispatcher twin reads.computeExecCtxdelegates to the sharedresolveAuthzContext, which setstenantIdfrom an API key's principal tenant, elsesessionData.session.activeOrganizationId— andactiveOrganizationIdis the exact field the dispatcher'sresolveActiveOrganizationIdreads. The REST door's answer is therefore the twin's answer plus API-key principals. BothPUTandDELETEalready resolve this context two lines above, for theirmanage_metadatagate.Premise 2 — does the dispatcher twin actually stamp a non-
nullorganization end-to-end? This is the half the triage seat flagged as unmeasured and would not assert. Measured, executably, by driving the realHttpDispatcher→ realObjectStackProtocolImplementation→ realSysMetadataRepositorywith a session carrying an active organization, then reading thesys_metadata_auditrows back:sys_metadata.organization_idsys_metadata_audit.organization_idPUT /metadata/view/overdue_gridorg_alphaorg_alphaPUT /metadata/flow/escalate_overduenullnullSo the twin does stamp a non-
nullorganization end to end, and the asymmetry the card rests on is real. But it is narrower than the card assumed, and that changes the size of the claim: the twin stamps an organization only for types the registry declaresallowOrgOverride: true—view,dashboard,report,translation,email_template. For every other type the twin also writesnull, deliberately, under the #6190 ruling. The fork clause did not fire (the twin is not env-wide in practice), but the disclosure this PR closes is confined to those five types.The shape, and why the obvious one is wrong
Threading
ctx.tenantIdraw would close the disclosure and open an outage.saveMetaItem'sorganizationIdis one value feeding two things — thesys_metadatapartition the row lands in and the audit row — and the protocol refuses an org-scoped write of anallowOrgOverride: falsetype (orgScopedWriteRefusal→NOT_OVERRIDABLE, 403). That refusal is itself a maintainer ruling (#6190, option A): coercing such a row to env-wide was considered and rejected, because it rewrites the tenancy statement the author made. So everyPUT /meta/object/*from a tenant-admin session would have started answering 403 — trading a disclosure for a regression.The organization is therefore threaded through
organizationIdForMetaWrite, the registry-derived predicate that already answers this question — and it is the dispatcher's own. The two doors now behave identically for the same request, which is the property the card is actually about. A consequence worth stating plainly: a non-overridable type still audits env-wide, and that is correct rather than residue — its write genuinely is environment-wide, sonullis its truthful scope.nullis now reserved for writes that really are environment-wide.No new org-resolution seam
The
/publishedcomment's seam warning stands and is respected:packages/reststill mints noresolveActiveOrganizationId. The doors readtenantIdoff the execution contextresolveExecCtxalready resolves — memoised per request, called in 40+ handlers in this file — which is exactly the precedent #8803 set for the audit read on the sibling route.resolveActiveOrganizationIdexists inmetadata-protocol. It does not — all five occurrences there are inside comments. The only executable one isHttpDispatcher's private method inpackages/runtime. Nothing depended on this: the fix needs no org resolution, only the org the context already carries.Three changes ride along, each required rather than adjacent
publishandrollbackmove with the save.promoteDraftForPublishresolves the draft throughgetOverlayRepo(orgId), so oncePUT ?mode=draftlands org-scoped, an unscoped publish looks in the env-wide partition and answersno_draft— the Studio designer's save→publish loop, broken. Scoping the save alone is not a smaller change, it is a broken one.rollbackresolves its row and history the same way.DELETEmoves with it too, and on that verb the organization is not only the audit row: it decides which row a reset destroys. Unscoped, a tenant's "reset to default" would reach past its own overlay and blank the env-wide row for every other tenant.GET /meta/:type/:name/publishedis now scoped. Its comment justified omitting the organization by a symmetry — "this door resolves exactly the publishes this door can produce" — and the write-side fix is precisely what ends that symmetry. Left alone it would answer 404 about aviewthe same caller published moments earlier through the same transport, reopening thepackages/restserves/meta/:type/:name/publishedfrom the code snapshot too — #8031's fix does not reach the REST transport #8278 defect one partition over. It passes the raw tenant (not the write predicate), which is fail-open in the safe direction:getMetaItemLayered's overlay layer is org-scoped-first, then env-wide, so nothing that resolves today stops resolving. That comment is rewritten in the diff rather than left to mislead.Lane branch: this is
domain:metadata, notdomain:cliThe escalation recorded the re-route as a known branch, and it fires — but on a third landing site neither option named. The derivation lands in
packages/rest(the doors) while its single implementation lands inpackages/metadata-core:organizationIdForMetaWrite/declaresOrgOverridemove there frompackages/runtime, because@objectstack/restcannot import@objectstack/runtime— runtime depends on rest, so that edge is a cycle turbo refuses.metadata-coreis the package both already depend on and that depends on neither; its barrel records four prior sinkings for exactly this criterion (the FLS projection, the audit-governance table, the write-verb dispatch predicates, the i18n discriminators). Runtime behaviour is unchanged and there is no second copy to drift.Verification
Gate union derived from the actual changed paths after the final commit and run at
9ce963a67:check:authz-resolver,check:route-envelope,check:cross-package-test-inputs,check:changeset-gate-self-tests,check:objectui-changeset,check:query-options-erasure,check:type-check-coverage,check:nul-bytes— all greencheck-adr-0087-registration,check-changeset-no-major,check-cross-package-test-inputs,check-empty-changeset— all greencheck:type-check-debt --re-measure— green, 33 ledger entries, none above its recorded ceiling. It first refused to run (rest'sdistolder than its sources after the ablation below) and was re-run against a rebuilt package rather than read as a passcheck:engine-double-contract,check:error-code-casing— green (no new fake engine, no new error code)Suites:
@objectstack/rest1962 passed (119 files),@objectstack/runtime2452 passed (164 files),@objectstack/metadata-core162 passed (10 files).typecheckgreen for all three.Reverse verification, direction predicted in writing before running. Ablation:
packages/rest/src/rest-server.tsreverted toorigin/main(the doors pass no organization again). Predicted per-case: 9 red, 5 green. Measured: 9 red, 5 green, case for case — a prose summary line in my prediction file miscounted its own table as "8/6", which is worth stating rather than quietly correcting. The five greens are the load-bearing half, not slack: the non-overridable-type cases pass on the unfixed code precisely because the unfixed door threads nothing, so they are what distinguishes "scopes correctly" from "scopes nothing" — a fix that simply kept passing nothing would satisfy every red case's inverse and fail none of them. Restore was verified byte-identical (git hash-objectagainst the committed blob), not by insertion count.Severity, honestly
This narrows a disclosure; it does not close a demonstrated live exploit. Whether any multi-tenant deployment authors metadata through the REST door is unmeasured in either direction, as the card says — and the measurement above narrows the exposed surface further, to the five
allowOrgOverride: truetypes.#8707is not addressed here; it remains open as the sibling of this composition.Generated by Claude Code