Repository navigation
auditMetaItem's organizationId is dead on both ends — the audit read returns every org's rows for a (type, name), while its comment describes a scope filter that is not in the query #8747
Description
Activity
Evidence added, ⛔ not graded and no label changed — grading is triage's single-producer field and this is an execution seat. PM session
session_01XeQRiAa7vYRVX5Fog7Zby8, from reviewing PR #8746 (#8595) where this was filed.The filed claim is confirmed on
origin/main, both endsmeasured organizationIdis declaredprotocol.ts:6063—organizationId?: string | nullinauditMetaItem's request type…and never reaches the query the whereis built as exactly{ type: singular, name: request.name }— no org term…and is never passed by the caller rest-server.ts:6410passestype,name,environmentId,limit— noorganizationIdSo the parameter is dead on both ends exactly as filed, and the comment above that
wheredescribes an org-scope$orthat is not in it.One measurement that narrows the question without answering it
The card frames the fork as "either a real cross-tenant disclosure, or a comment that over-claims plus a parameter to delete". This is the input that decides which, and it does not point the way the card's more alarming reading assumes:
sys_metadata_auditdoes carry the tenant column.packages/metadata-core/src/objects/sys-metadata-audit.object.ts:101declaresorganization_id: Field.lookup('sys_organization', …), and:190declares the index{ fields: ['organization_id', 'occurred_at'] }— described at:29as "for the per-org timeline".⇒ Layer 0's field gate (
objectFields.has('organization_id')) can therefore engage on this object. If the wall applies under the deployment's posture, the engine scopes this read by organization regardless ofauditMetaItemignoring its own parameter — and the org scoping the comment describes is real, just enforced a layer below where the comment puts it.That would leave a narrower, still-worth-fixing defect: a dead parameter and a comment attributing the scoping to the wrong layer — plus a genuine gap under
single, where there is no wall to carry it.⚠️ I have not run the two-org probe, so this does not settle it — the wall's applicability to this object under each posture is exactly the measurement the filing dev correctly declined to guess at. It only means the severity should not be assumed from the query alone.Related and worth reading together when this is graded: #8608 is the card about the two derivations of "is this object tenant-scoped" disagreeing on
systemFields: false. Ifsys_metadata_auditever lands in that gap, the wall stops carrying this and the alarming reading becomes the correct one. #8707 remains distinct — that is write-time stamping, this is the read discarding the stamp.
Generated by Claude Code
Triage: lands in
packages/metadata-protocol⇒domain:metadata. Queued. TypeBug—declared ≠ enforcedon both ends (Prime Directive #10).Premise re-verified on
origin/main@7246d63, not carried from the filingclaim reading param declared protocol.ts:6063—organizationId?: string | nullinauditMetaItem's request type ✅comment describes an org $or:6087-6089— "include rows for the specific org AND env-wide (organization_id IS NULL) rows" ✅the wherehas no org term:6090-6093— built as exactly{ type: singular, name: request.name }✅caller never passes it rest-server.ts:6410-6415—type,name,environmentId,limitonly ✅All four hold verbatim. Queued on the measurement, not on either reading.
Scope, as fenced by triage
Deliverable #1 is the two-org probe, and the fix shape follows from it — do not pick one first. The execution seat comment on this card (17:53Z) already narrowed the question usefully and must be read before starting:
sys_metadata_auditdoes declareorganization_id(sys-metadata-audit.object.ts:101) with a per-org timeline index at:190, so Layer 0's field gate can engage and the wall may be carrying the scoping a layer below where the comment puts it. That makes the "cleanup" reading more likely than the "disclosure" reading — but it is not measured, and the two outcomes have different severities:- Wall carries it ⇒ the defect is a dead parameter plus a comment attributing the scoping to the wrong layer. Fix: delete the param, correct the comment to name the enforcing layer, pin it.
⚠️ Still note thesingle-posture gap the seat flagged — no wall there to carry it. - Wall does not carry it ⇒ real cross-tenant disclosure of
actor/note/lock_statethrough Studio's audit tab. Fix: wire the$orand threadorganizationIdfrom the REST route. In that case stop and re-escalate — a security-boundary change is the maintainer floor, not a dev's call.
⛔ Out of scope either way: changing what the write path stamps. That is #8707 (
pm:queue,domain:identity), which carries a maintainer ruling from #8287 about stamping from the record's own org. Distinct card, distinct lane.Serial constraints for the claiming seat
- [finding] a THIRD copy of the union-branch selection ranking lives in packages/metadata-protocol/src/protocol.ts — outside the reach of #8318's package-internal consolidation, with no mechanical parity enforcement #8660 (
pm:queue, same lane, promoted this fire) also namespackages/metadata-protocol/src/protocol.ts. Its cheapest option adds a parity test and may not touchprotocol.tsat all — but declare file surface to the region level and check its claim comment before batching the two together. - Two derivations of "is this object tenant-scoped" disagree on
systemFields: false: the wall composes its predicate, the platform withholds the index #8608 (pm:dispatched,domain:engine-core) is the card about the two derivations of "is this object tenant-scoped" disagreeing. Ifsys_metadata_auditever lands in that gap, the wall stops carrying this and the alarming reading becomes the correct one. Read its outcome before concluding "the wall has it".
Generated by Claude Code
- Wall carries it ⇒ the defect is a dead parameter plus a comment attributing the scoping to the wrong layer. Fix: delete the param, correct the comment to name the enforcing layer, pin it.
Claim: PM loop round 8
Session:session_01XeQRiAa7vYRVX5Fog7Zby8
Branch:claude/issue-8747-audit-org-scope
Worktree:objectstack-issue-8747
Domain:domain:metadata
File surface:packages/metadata-protocol/src/protocol.ts— region:auditMetaItemonly,:6091–~:6170— and, only if the measurement calls for threading the parameter,packages/rest/src/rest-server.tsaround:6411–:6421, plus tests (stop on breach; explain in the report)
Container & model:M,mode:subagent,model: opus
Serial constraints cleared: region-declared, becauseprotocol.tsis contended three ways. Line numbers taken fromorigin/mainat dispatch, not from the card:region lines holder canonicalizeMetaRequestType+ call sites + thesaveMetaItemdoor:211, 6 sitesPR #8770, open and unlanded searchAll~ :7177–:7411#7643, in flight auditMetaItem:6091–~:6170this card Three disjoint regions in a 15k-line file. Each dispatch carries a merge-
origin/main-before-PR instruction and a stop-and-report if the work reaches outside its region; the merge queue arbitrates loudly if I have judged the separation wrong. This is deliberate parallelism under a maintainer instruction to keep the slots full, not an accident — recording it so the rebase churn, if it comes, is legible rather than mysterious.⚠️ This card is measure-first and may be a security finding: if reading 1 holds (a tenant admin sees another tenant'sactor/note/lock_statefor a same-named metadata item), that is a live cross-tenant disclosure and I want it reported the moment it is measured, before any fix. If reading 2 holds, it is a comment that over-claims plus a dead parameter. The fix direction is narrowing — restoring the scoping the signature and comment already advertise — so it stays in the repair lane rather than the human floor either way.
Generated by Claude Code
🚨 MEASURED: reading 1 holds — this is a live cross-tenant disclosure. No fix pushed; escalating first, as dispatched.
Dev seat
session_01XeQRiAa7vYRVX5Fog7Zby8, branchclaude/issue-8747-audit-org-scope(branch exists, no commits, no PR — the dispatch said report before implementing, and this is that report).The 17:53Z execution-seat comment narrowed the question toward the cleanup reading on the grounds that
sys_metadata_auditdeclaresorganization_id, so Layer 0's field gate can engage. That input is correct and the conclusion it pointed at is falsified: the field gate never gets asked, because the middleware that would ask it short-circuits several steps earlier.
1. The probe the card prescribed — two orgs, same
(type, name), one audit readReal
ObjectQL+ realSqlDriverover on-disk better-sqlite3 + realObjectStackProtocolImplementation+ the realSysMetadataObject/SysMetadataHistoryObject/SysMetadataAuditObjectdeclarations. Rows seeded through the realsaveMetaItemwrite path, not hand-inserted. Harness copied frompackages/runtime/src/package-uninstall-org-scope.integration.test.ts, which is the existing precedent for exactly this shape.Three saves of one view named
shared_grid:save organizationIdactor 1 org_alphaalice@alpha.example2 org_betabob@beta.example3 (none — env-wide) package-installerThen one read, spelled exactly as the production route spells it:
await protocol.auditMetaItem({ type: 'view', name: 'shared_grid' })
Measured output:
auditMetaItem() ACTORS: [ 'package-installer', 'bob@beta.example', 'alice@alpha.example' ]Three rows, three organizations, one caller. A tenant admin in
org_alphaopening Studio's audit tab receivesorg_beta'sactor, and with it that row'snote,lock_state,code,operation,sourceandrequest_id.2. The mechanism, measured layer by layer — nothing compensates
Three candidate compensators, all eliminated:
(a) The driver's implicit tenant wall does not engage — it is never handed a tenant.
SqlDriver.applyTenantScopebuilds(organization_id = :tenant OR organization_id IS NULL)only whenDriverOptions.tenantIdis set. The engine sets it fromexecCtx.tenantIdinbuildDriverOptions(packages/objectql/src/engine.ts:2850—hasTenantrequiresexecCtx?.tenantId !== undefined).auditMetaItempasses no context at all. Measured on a recording driver, with a positive control so the null result cannot be a harness artifact:DRIVEROPTIONS context-less: {"where":{"type":"view","name":"shared_grid"}} DRIVEROPTIONS tenant-scoped: {"where":{...},"context":{"userId":"u1","tenantId":"org_alpha"},"tenantId":"org_alpha"}The wall works. It is simply not asked.
(b) plugin-security's Layer 0 does not engage either — the middleware falls open before reaching it.
security-plugin.ts:1230-1236:if (positions.length === 0 && explicitPermissionSets.length === 0 && !opCtx.context?.userId) { return next(); }
A context-less
engine.findhas no positions, no permission sets and nouserId, so it takes thisreturn next().computeLayeredRlsFilter— and with it theobjectFields.has('organization_id')gate at:4537that the 17:53Z comment identified — sits ~3300 lines further down the same function and is unreachable on this path. The field gate's readiness was real and is irrelevant.(c) The posture question is moot, but worth recording. Even if the middleware did run,
computeTenantLayer0Filter(tenant-layer.ts:119) returnsnulloutright under thesingleposture, and underisolatedit readscontext?.tenantId— which is absent here, so:146would return the fail-closed deny sentinel rather than a scope. So there is no posture in which the current call shape yields correct scoping: it is unscoped in every one.3. Blast radius
GET /api/v1/meta/:type/:name/audit(rest-server.ts:6404) carries no capability gate in its handler — unlike itsPUT /meta/:type/:namesibling at:6100, which gates onmanage_metadata. The only guard is the anonymous floor (enforceAuth→shouldDenyAnonymous). So the reachable cohort is any authenticated principal, of any tenant, with no capability required. Route ledger dispositionsdk, clientmeta.getAudit, so it is also on the published SDK surface.Exposure requires a
(type, name)collision across tenants. That is not exotic: metadata names are authored from a shared vocabulary and the same package installed into two tenants produces identical(type, name)pairs by construction.Not affected: the dispatcher twin (
packages/runtime/src/domains/meta.ts) has no audit route, sorest-server.tsis the sole production door, exactly as filed.4. Why I stopped here
The dispatch was explicit — measure first, report a positive reading 1 immediately, and do not fold a security fix into a routine cleanup PR. It also flagged that this changes who decides. Beyond that, the fix genuinely forks in a way this card did not settle, and one branch runs into a recorded precedent:
rest-server.ts:6731-6742states, about the sibling/publishedroute, thatpackages/restcarries noresolveActiveOrganizationIdand no org plumbing at all, and that inventing it there "would be a new seam smuggled in under a bug fix". So the card's option A as literally worded — threadorganizationIdfrom the REST route — walks straight into that.But there is a third option the card did not name, and I think it is the right one. The obstacle above is specific to the
organizationId-as-protocol-argument seam. The ExecutionContext seam already exists in this very file:computeExecCtx(rest-server.ts:2915) assemblestenantId: authz.tenantId, andresolveExecCtxis already called by 15+ handlers inrest-server.ts— including thePUT /meta/:type/:nametwin at:6133. Threading that context into the audit read is using the seam that is already there, not adding one.option shape closes it? cost A — wire $or, threadorganizationIdfrom RESThand-built $or [{organization_id: oid}, {organization_id: null}]; new org-resolution plumbing inpackages/restyes hits the :6731precedent; hand-reimplements the wall the platform already owns; a second place to get tenancy rightB — delete the param, correct the comment signature and comment stop advertising scoping no — leaves the disclosure open ruled out by measurement; this was only viable under reading 2 C — thread the caller's ExecutionContextinto the engine readauditMetaItemaccepts a context and passes{ context }toengine.find; route supplies it via the existingresolveExecCtxyes the platform's own Layer 0 + driver wall do the scoping My recommendation is C, on the three axes:
- Real business need. The need is measured, not speculative: the disclosure above is the demand. C also delivers the comment's exact promised semantics for free — the driver wall is literally
(organization_id = :tenant OR organization_id IS NULL), i.e. own-org plus env-wide, which is the both-halves behaviour the editor needs so package-level env-wide writes stay visible. A hand-built$orre-derives that by hand and can drift from it. - Long-term soundness. Contract-first and no workaround: tenant isolation stays owned by the one layer that owns it everywhere else, instead of gaining a second implementation inside one protocol method. A hand-built predicate in
auditMetaItemis exactly the "second implementation of the same contract, blind to every widening mechanism" shape the security plugin's own:5492comment warns about. - Hard to get wrong. C makes the absence of scoping the thing you have to write explicitly, rather than the thing you get by forgetting an argument — which is precisely how this defect happened. An optional
organizationId?: string | nullthat every caller may silently omit is a declared-not-enforced trap by construction; a threaded context is the shape every other scoped read in this codebase already uses, so the next author copies a correct pattern rather than an inert one.
⚠️ One caveat C does not solve, and A does not either: under thesingletenancy posture Layer 0 contributes nothing by design, so on asingle-posture deployment C leaves the read unscoped. That is correct-by-design forsingle(one tenant, nothing to isolate) — but it means C's guarantee is "scoped wherever the platform scopes anything", not "scoped unconditionally". If the intent is the stronger unconditional guarantee, that is A, and it needs saying out loud rather than assuming. This is the sharpest thing I want a decision on.5. What I deliberately did not do
- No fix, no commits, no PR. Reporting first was the instruction, and the option fork above is a real decision, not a detail.
- Did not touch the write path. Audit rows are stamped from the ACTOR's active organization in preference to the record's own — and the record-side fallback cannot see
sys_api_key.active_organization_id#8707 owns stamping; every measurement above reads rows the current write path already stamps correctly. - Stayed inside the dispatched region. Nothing read or written near
canonicalizeMetaRequestType(PR fix(metadata): refuse a /meta type name the platform does not have, instead of minting a namespace for it (#8421) #8770) orsearchAll(⌘K global-search palette recall is strictly narrower than the$searchexecutor (no pinyin/companion recall) #7643). - Filed nothing new. One adjacent observation, recorded here rather than as a card because it is inside this card's own region: the route also passes
environmentIdtoauditMetaItem, which its request type does not declare and its body never reads — a second dead parameter on the same call, and whichever option lands should sweep it.
Probe kept and re-runnable; say the word and I will land the chosen option with the both-directions pins the card asks for (own-org and env-wide visible, third org not visible).
Generated by Claude Code
Generated by Claude Code
- Real business need. The need is measured, not speculative: the disclosure above is the demand. C also delivers the comment's exact promised semantics for free — the driver wall is literally
{ "issue": 8747, "status": "needs_decision", "branch": "claude/issue-8747-audit-org-scope", "pr": null, "premise_still_valid": true, "summary": "MEASURED reading 1: this is a live cross-tenant disclosure, not a cleanup. Real ObjectQL + real SqlDriver (sqlite) + real ObjectStackProtocolImplementation, three saves of one view name under org_alpha / org_beta / env-wide through the real saveMetaItem path, then one auditMetaItem({type,name}) read returned ALL THREE rows: actors package-installer, bob@beta.example, alice@alpha.example. Nothing compensates lower down and all three candidates were eliminated by measurement, not by reading: (a) the driver's tenant wall never engages because buildDriverOptions sets DriverOptions.tenantId only from execCtx.tenantId and auditMetaItem passes no context (recorded DriverOptions were bare, with a tenant-scoped positive control proving the wall does engage when a context IS threaded); (b) plugin-security's Layer 0 never engages because the middleware hits its principal-less `return next()` at security-plugin.ts:1230-1236, ~3300 lines before the objectFields.has('organization_id') gate at :4537 that the triage comment expected to carry this; (c) even if it ran, computeTenantLayer0Filter returns null under `single` and the deny sentinel under `isolated` with no tenantId, so no posture yields correct scoping. Reachable by ANY authenticated principal of any tenant: GET /api/v1/meta/:type/:name/audit carries no capability gate, only the anonymous floor, unlike its PUT twin which gates on manage_metadata. NO fix written and NO PR opened, per the dispatch's instruction to report a positive reading 1 before implementing. NOTE the card's option A runs into a recorded precedent (rest-server.ts:6731-6742 states packages/rest carries no org plumbing and that adding it under a bug fix would be a smuggled seam) and its option B is now falsified by measurement, so the fork needs a decision.", "tests": "No production code changed, so no gate union and no HEAD to quote — this run is a measurement, not a diff. Probe run: `pnpm --filter @objectstack/runtime exec vitest run src/zz-probe-8747.integration.test.ts --maxWorkers=2` under flock /tmp/os-heavy-verify.lock, on a fresh worktree at origin/main ab8b10f82 after `pnpm --workspace-concurrency=2 --filter '@objectstack/runtime^...' build` (EXIT=0 — the dogfood-adjacent point that the probe reads the built dist of objectql/driver-sql/metadata-protocol/metadata-core, which is why the closure was built first). Both probe cases were written to assert an impossible value so vitest would print the real output rather than let a green mask it; measured output verbatim: `auditMetaItem() ACTORS: [ 'package-installer', 'bob@beta.example', 'alice@alpha.example' ]`, `DRIVEROPTIONS context-less: {\"where\":{\"type\":\"view\",\"name\":\"shared_grid\"}}`, `DRIVEROPTIONS tenant-scoped: {\"where\":{...},\"context\":{\"userId\":\"u1\",\"tenantId\":\"org_alpha\"},\"tenantId\":\"org_alpha\"}`. The tenant-scoped line is the positive control: it proves the empty DriverOptions above is the omission under test and not an artifact of the harness. Probe file removed from the worktree afterwards (git status clean, no commits) and kept at /tmp/claude-0/-home-user/fa8d55b6-f99c-53bb-bdc4-f2b492970093/scratchpad/issue-8747/probe.test.ts. No reverse verification: there is no fix to revert, and the disclosure is established by the forward measurement plus its control.", "open_questions": [ { "question": "How should the audit read be scoped, given that the card's option A collides with a recorded precedent and its option B is falsified by measurement?", "options": [ "A - wire the $or the comment describes and thread organizationId from the REST route. Closes it, but rest-server.ts:6731-6742 explicitly states packages/rest carries no resolveActiveOrganizationId and no org plumbing at all, and that inventing it to close a bug would be 'a new seam smuggled in under a bug fix'. Also hand-reimplements the wall the platform already owns, creating a second place tenancy must be kept right.", "B - delete the parameter and correct the comment. FALSIFIED: this was only viable under reading 2 and would leave the disclosure open. Listed so the record shows it was measured out, not skipped.", "C - thread the caller's ExecutionContext into the engine read: auditMetaItem accepts a context and passes { context } to engine.find, the route supplies it via the resolveExecCtx that rest-server.ts already calls in 15+ handlers including the PUT /meta twin at :6133, and the platform's own Layer 0 plus driver wall do the scoping." ], "recommendation": "C. Real business need: the need is the measured disclosure, and C delivers the comment's exact promised semantics for free because the driver wall is literally (organization_id = :tenant OR organization_id IS NULL) — own-org PLUS env-wide, the both-halves behaviour the editor needs so package-level env-wide writes stay visible, which a hand-built $or re-derives and can drift from. Long-term soundness: contract-first, no second implementation of tenant isolation inside one protocol method — exactly the 'second implementation blind to every widening mechanism' shape the security plugin's own :5492 comment warns about. Hard to get wrong: an optional organizationId?: string | null that every caller may silently omit is a declared-not-enforced trap by construction and is precisely how this defect happened, whereas a threaded context is the shape every other scoped read here already uses, so the next author copies a correct pattern instead of an inert one. CAVEAT the maintainer should rule on explicitly: under the `single` tenancy posture Layer 0 contributes nothing by design, so C's guarantee is 'scoped wherever the platform scopes anything', not 'scoped unconditionally'. That is correct-by-design for single-tenant deployments, but if the intent is the stronger unconditional guarantee then A is the option that provides it and the extra seam is the price. This is the sharpest open point." } ], "out_of_scope_findings": [] }Full evidence, mechanism and blast radius in the sibling comment posted immediately above.
Generated by Claude Code
Generated by Claude Code
- addedbugSomething isn't workingSomething isn't workingand removed
on Aug 14, 2026 1 remaining item
🚨 Reading 1 CONFIRMED — this is a live cross-tenant disclosure, measured.
pm:dispatched→needs-user-decision,security+target:v17applied, unassigned.⛔ No fix was written, no commits, no PR. The dispatch required an immediate report on a positive reading 1 and forbade folding a security fix into a routine cleanup PR. That instruction was followed.
The measurement
Real
ObjectQL+ realSqlDriver(sqlite) + realObjectStackProtocolImplementation. Three saves of one view name — underorg_alpha, underorg_beta, and env-wide — through the realsaveMetaItemwrite path. Then oneauditMetaItem({type, name})read:auditMetaItem() ACTORS: [ 'package-installer', 'bob@beta.example', 'alice@alpha.example' ]All three orgs' rows, from one tenant's read.
Reading 2 is falsified — all three "compensated lower down" candidates eliminated by measurement, not by reading
- The driver's tenant wall never engages.
buildDriverOptionssetsDriverOptions.tenantIdonly fromexecCtx.tenantId(engine.ts:2850), andauditMetaItempasses no context. RecordedDriverOptionscame back bare:The second line is a positive control and is what makes the first readable — it proves the bare options are the omission under test, not an artifact of the harness.DRIVEROPTIONS context-less: {"where":{"type":"view","name":"shared_grid"}} DRIVEROPTIONS tenant-scoped: {"where":{...},"context":{"userId":"u1","tenantId":"org_alpha"},"tenantId":"org_alpha"} plugin-security's Layer 0 never engages. The middleware takes its principal-lessreturn next()atsecurity-plugin.ts:1230-1236— roughly 3,300 lines before theobjectFields.has('organization_id')gate at:4537.⚠️ The 17:53Z triage comment on this card pointed at that gate; the input was right and the conclusion it implied is falsified, because the gate is never asked.- No posture would save it anyway.
computeTenantLayer0Filterreturnsnullundersingle(tenant-layer.ts:119) and the fail-closed deny sentinel underisolatedwith no tenantId (:146).
Blast radius — wider than the card assumed
GET /api/v1/meta/:type/:name/audit(rest-server.ts:6404) carries no capability gate in its handler — only the anonymous floor — unlike itsPUTtwin at:6100, which gates onmanage_metadata. So the cohort is any authenticated principal of any tenant, not "a tenant admin". The route is on the published SDK surface (ledger dispositionsdk,client meta.getAudit). The dispatcher twin has no audit route, sorest-server.tsis the sole production door, exactly as filed.Disclosed fields:
actor,note,lock_state.The decision — ⛔ human floor, not mine
A security/permissions boundary, and the fork moved under measurement: the card's option B is falsified, and its option A collides with a recorded precedent.
A — wire the
$orand threadorganizationIdfrom the REST route. Closes it, butrest-server.ts:6731-6742explicitly records thatpackages/restcarries noresolveActiveOrganizationIdand no org plumbing at all, and that inventing it to close a bug would be "a new seam smuggled in under a bug fix". It also hand-reimplements the wall the platform already owns.B — delete the parameter, correct the comment. ⛔ Falsified. Viable only under reading 2; would leave the disclosure open. Recorded so it shows as measured out rather than skipped.
C — thread the caller's
ExecutionContextinto the engine read.auditMetaItemaccepts a context and passes{ context }toengine.find; the route supplies it via theresolveExecCtxthatrest-server.tsalready calls in 15+ handlers, including thePUTtwin at:6133. The platform's Layer 0 and the driver wall then do the scoping.Four prisms
① Platform coherence → C. Tenant isolation stays owned by the one layer that owns it everywhere else, instead of gaining a second implementation inside one protocol method — the "second implementation of the same contract, blind to every widening mechanism" shape that
plugin-security:5492's own comment warns about.② Measured business pull → C. The driver wall is literally
(organization_id = :tenant OR organization_id IS NULL)— own-org plus env-wide, which is exactly the both-halves behaviour the card's own verification bar demands so the editor keeps seeing package-level env-wide writes. A hand-built$orre-derives that by hand and can drift from it.③ AI-agent error-resistance → C, decisively. An optional
organizationId?: string | nullthat every caller may silently omit is a declared-not-enforced trap by construction — it is precisely how this defect happened. A threaded context is the shape every other scoped read in this codebase already uses, so the next author copies a correct pattern instead of an inert one.④ Startup scope discipline → C. No new seam; A's cost is a plumbing layer
packages/restdeliberately does not have.⚠️ The caveat that needs ruling explicitly, not assumingUnder the
singletenancy posture Layer 0 contributes nothing by design. So C's guarantee is "scoped wherever the platform scopes anything", not "scoped unconditionally". That is correct-by-design for a single-tenant deployment — but if you want the stronger unconditional guarantee, A is the option that provides it and the extra seam is its price. This is the sharpest point on the card and the dev seat correctly refused to settle it alone.Recommendation: C, with that caveat ruled explicitly either way.
One more, to sweep in the same diff
rest-server.ts:6421-6426also passesenvironmentIdtoauditMetaItem— which the request type does not declare and the method body never reads. A second dead parameter on the same call. Whichever option lands should take it out or wire it.The worktree is left in place (no PR to clean up behind, decision pending, and its built dependency closure would otherwise need rebuilding for the follow-up lap).
Generated by Claude Code
- The driver's tenant wall never engages.
Cross-lane note while this awaits a ruling: the write half is in flight right now.
PR #8777 — "fix(audit): stamp audit rows from the record's own organization (#8707)" — is open and marked ready, from another seat.
This card's body already names the relationship ("#8707 is about how audit rows are stamped at write time; this one is about the read discarding the stamp"). What is new is that the write half is no longer hypothetical — it is a live PR, and it lands while this read-side decision is still open.
Two consequences worth having on the record before this is ruled:
-
It may sharpen the read-side fix rather than conflict with it. If fix(audit): stamp audit rows from the record's own organization (#8707) #8777 changes which organization a row is stamped with, then what "correctly scoped" means for the read is defined against the post-fix(audit): stamp audit rows from the record's own organization (#8707) #8777 stamping, not today's. Whoever implements the ruling here should re-measure against
origin/mainafter fix(audit): stamp audit rows from the record's own organization (#8707) #8777 lands rather than against the transcript in my escalation above — the measurement I posted was taken atab8b10f82. -
It does not change the disclosure verdict. The measured fact — one context-less
auditMetaItemread returning three organizations' rows — is about the read discarding a dimension the rows already carry. Stamping them more accurately at write time does not make an unscoped read scoped. So the severity and the fork stand as reported; only the fix's target state may move.
⛔ I have not touched #8777 and am not coordinating this card with it — it belongs to another seat. Recording the overlap so it is visible to whoever rules, rather than discovered afterwards.
Generated by Claude Code
-
Maintainer ruling (delegated adjudication; delegation 2026-08-15 verbatim 「决策你直接帮我做」, batch confirmed 「同意」): wire it, do not delete it.
Implement the
$orthe method's own comment describes — rows for the specific organization AND env-wide (organization_id IS NULL) rows — and threadorganizationIdfrom the REST route (GET /api/v1/meta/:type/:name/audit) into the request. The two readings the card could not settle (real disclosure vs compensated downstream) converge on the same fix; the two-org probe runs first only to classify severity for the record, never to choose the remedy. Pin both: the scoped read, and the comment now describing behaviour that exists.Rationale: the write path stamps
organization_idcarefully — a read door discarding the dimension is the asymmetry; declared=enforced is restored by making the described filter real; deleting the parameter instead would leave "the layer below happens to compensate" as a loaded gun for any future caller that bypasses it.needs-user-decision→pm:queue;domain:metadataseat dispatches (target:v17retained).
Generated by Claude Code
Claim: PM loop round 5
Session:session_01XeQRiAa7vYRVX5Fog7Zby8
Branch:claude/issue-8747-audit-org-scope(the branch the measurement lap already created — no commits on it)
Worktree:objectstack-issue-8747
Domain:domain:metadata
File surface:packages/metadata-protocol/src/protocol.ts— region-declared:auditMetaItemonly, from:6091; pluspackages/rest/src/rest-server.ts(theGET /api/v1/meta/:type/:name/audithandler at:6404and its request assembly at:6421-6426), plus the co-located pins. ⛔ Stop on breach and report.
Container & model:M,mode:subagent,model: opus— judgement tier (ruling implementation on a security boundary). Notclaude-fable-5: the mandatory clause is scoped to accept/reject changes onpackages/spec, which this card does not touch; the accept-set decision here was already ruled, not delegated to the dev.
Serial constraints cleared:protocol.tsis a contended file and is running under region-level declaration. Concurrent regions this round, measured onorigin/main84cb121ebat claim time, not copied from a card:diffMetaItem:15290(#8671, this batch) — ~9,200 lines away;searchAll:8520(#7643/PR #8787, all 26 checks green, landing now);canonicalizeMetaRequestType:211(#8421/PR #8770, held draft, parked, not landing).packages/rest/src/rest-server.ts: no other in-flight card in this lane declares it.The ruling is settled — ⛔ do not re-adjudicate
The maintainer ruled option A in comment
5299824146(delegated adjudication, delegation 2026-08-15 verbatim 「决策你直接帮我做」, batch confirmed 「同意」):wire it, do not delete it. Implement the
$orthe method's own comment describes — rows for the specific organization AND env-wide (organization_id IS NULL) rows — and threadorganizationIdfrom the REST route (GET /api/v1/meta/:type/:name/audit) into the request. […] Pin both: the scoped read, and the comment now describing behaviour that exists.This overrides the previous dev seat's recommendation of option C, and mine. I recommended C in
5299183155; the ruling went to A. Recording that plainly so the divergence is visible rather than quietly dropped — the ruling stands and A is what gets built.Known obstacle you must navigate under A (not a licence to switch options)
The measurement lap found a recorded precedent that A walks into, and you should read it before writing:
rest-server.ts:6731-6742states, about the sibling/publishedroute, thatpackages/restcarries noresolveActiveOrganizationIdand no org plumbing at all, and that inventing it there "would be a new seam smuggled in under a bug fix".That is a real constraint on how you thread the organization id, not a reason to build C instead. Read that comment, then choose the least-seam way to satisfy the ruling —
computeExecCtx(:2915) already assemblestenantId: authz.tenantIdandresolveExecCtxis already called by 15+ handlers in this file including thePUT /metatwin at:6133, so an organization id is derivable from machinery that already exists. If after measuring you conclude the ruling is genuinely unimplementable without minting the seam that comment forbids, stop and report the conflict — do not pick a different option and do not smuggle the seam.Carried facts (measured last lap — re-verify, do not inherit)
- The disclosure is live and measured: one context-less
auditMetaItem({type, name})read returned all three orgs' rows —ACTORS: [ 'package-installer', 'bob@beta.example', 'alice@alpha.example' ]. - All three "compensated lower down" candidates were eliminated by measurement with a positive control: the driver wall never engages (
buildDriverOptionssetsDriverOptions.tenantIdonly fromexecCtx.tenantId,engine.ts:2850); plugin-security Layer 0 falls open at its principal-lessreturn next()(security-plugin.ts:1230-1236); andcomputeTenantLayer0Filteryields nothing useful under either posture. - Blast radius is wider than the card as filed: the handler at
rest-server.ts:6404has no capability gate, unlike its PUT twin at:6100which gates onmanage_metadata. Cohort = any authenticated principal of any tenant, on the published SDK surface (meta.getAudit).
⚠️ Re-measure against the currentorigin/mainbefore you start. PR #8777 ("stamp audit rows from the record's own organization", #8707 — the write half) was open and ready as of 00:17Z. If it has landed, "correctly scoped" is defined against the post-#8777 stamping, and the transcript above was taken atab8b10f82. The disclosure verdict does not change — an unscoped read stays unscoped however well rows are stamped — but the target state may have moved.Sweep in the same diff
rest-server.ts:6421-6426also passesenvironmentIdtoauditMetaItem, which the request type does not declare and the body never reads — a second dead parameter on the same call. Take it out or wire it; do not leave it.Verification bar
Pin both directions, per the ruling and the card: own-org rows and env-wide (
organization_id IS NULL) rows visible; a third org's rows not visible. A refusal pin with no positive control cannot show the filter discriminates rather than hiding everything. Also pin that the method comment now describes behaviour that exists.This is a security fix on a live cross-tenant disclosure — it is the highest-value card in the lane this round. ⛔ Do not fold anything unrelated into the diff.
Generated by Claude Code
- The disclosure is live and measured: one context-less
{ "issue": 8747, "status": "done", "branch": "claude/issue-8747-audit-org-scope", "pr": "https://github.com/objectstack-ai/objectstack/pull/8803", "premise_still_valid": true, "summary": "Built the maintainer's ruled option A. auditMetaItem now constructs the $or its own comment describes — the caller's organization PLUS env-wide (organization_id IS NULL) rows — and GET /api/v1/meta/:type/:name/audit threads the organization into it. PM mechanism assumption 1 CONFIRMED, not assumed: an organization id is derivable from machinery that already exists, so no forbidden seam was minted. resolveExecCtx is already called by 40+ handlers in rest-server.ts (including the PUT /meta twin at :6133) and computeExecCtx assembles tenantId from the shared resolveAuthzContext (an API key's principal tenant, else the session's activeOrganizationId) — the route reads a field the execution context already carries and adds no resolveActiveOrganizationId and no org plumbing to packages/rest. PM assumption 2 MEASURED and it did NOT move the target state: #8777 merged at 00:31Z, but it changed plugin-audit's sys_audit_log stamping, whereas sys_metadata_audit is stamped by metadata-protocol's own recordMetadataAudit (organization_id: entry.organizationId ?? null), untouched — and the disclosure reproduces identically on the post-#8777 tree. An unresolved organization is fail-closed onto env-wide via ?? null, so omitting the parameter is no longer a skeleton key, and the scope is unconditional rather than posture-dependent. The dead environmentId was swept from the same call. One assumption of MINE was falsified by measurement and the test now pins the real behaviour instead: the handler's .catch(() => undefined) branch is unreachable through the route, because an unresolvable context is refused by the anonymous floor with a 401 before the handler body runs.", "tests": "All verification below was run AFTER the final commit (the origin/main merge) against HEAD = 22c032dd9, and that sha is quoted in the PR body too. Full suites: `pnpm --filter @objectstack/metadata-protocol test` — 92 files, 1376 tests, all passing; `pnpm --filter @objectstack/rest test` — 118 files, 1948 tests, all passing; `pnpm --filter @objectstack/runtime test` — 158 files, 2400 tests, all passing. `pnpm --filter @objectstack/rest --filter @objectstack/runtime typecheck` clean — metadata-protocol declares NO typecheck script, stated rather than implied. New coverage, 20 cases over three layers: runtime integration (7, real ObjectQL + real SqlDriver over sqlite, three orgs seeded on one (type,name) through the real saveMetaItem path), metadata-protocol query-shape (7, every call spelling plus a comment-vs-code drift guard), rest route (6). PREMISE RE-MEASUREMENT on the current tree, verbatim: `auditMetaItem() ACTORS: [ 'package-installer', 'bob@beta.example', 'alice@alpha.example' ]` — three orgs, one read, still live post-#8777. REVERSE VERIFICATION with the direction predicted BEFORE running: ablating the scope should turn 6 of 7 integration cases red and leave exactly 1 green (the seeding precondition, which asserts only write-side stamps and is read-independent); observed exactly `Tests 6 failed | 1 passed (7)`. ABLATION REBUILD, stated explicitly because it is the failure mode that hides: this suite resolves @objectstack/metadata-protocol from its BUILT dist (runtime's vitest.config.ts aliases core/rest/spec/types/platform-objects to src but NOT metadata-protocol), so every leg was mutate -> `pnpm --filter @objectstack/metadata-protocol build` -> prove the marker in the artifact -> run. Marker `organization_id: null } : {` counted 1 in dist/index.js with the fix, 0 after ablation, 1 again after restore; the restore was `git checkout HEAD -- ...` from the fix commit and `git diff HEAD` was empty (byte-identical, not a retype), and dist was rebuilt so no ablated artifact survives in the worktree. GATES re-derived against the ACTUAL changed paths with `node scripts/pm/dispatch-gates.mjs` (the dispatch named no families, so this derivation is the whole list), all green: check:nul-bytes, check:authz-resolver, check:route-envelope, check:filter-alias-parity, check:durability-log-level, check:cross-package-test-inputs, check:query-options-erasure, check:type-check-coverage, check:changeset-gate-self-tests, check:objectui-changeset, plus check-adr-0087-registration, check-changeset-no-major, check-empty-changeset. check:type-check-debt (the RATCHET half) was NOT skipped — the full workspace closure was built first as lint.yml does (70/70 turbo tasks successful) and it reports 33 ledger entries re-measured, 1926 raw errors, `none above its recorded number`; the -1 surplus it notes is pre-existing on @objectstack/lint, a package this diff does not touch. Control-character self-scan (`grep -naP` over all six changed files) clean, beyond the gate.", "open_questions": [], "out_of_scope_findings": [] }Notes for the PM, beyond the JSON
Your assumption 1 was right, and I verified it rather than trusting it.
computeExecCtx(rest-server.ts:2915) assemblestenantId: authz.tenantId, andresolveAuthzContext(packages/core/src/security/resolve-authz-context.ts) derives that fromkeyPrincipal.tenantId ?? sessionData.session.activeOrganizationId— i.e.tenantIdis the active organization id. So option A is implementable without the seamrest-server.ts:6731-6742forbids, and no conflict needed reporting.The env-wide half turned out to be more load-bearing than the card argued. Measured: the REST
PUT /meta/:type/:namedoor passes noorganizationIdtosaveMetaItem, so every audit row that door writes is stampedorganization_id: null. An equality-only filter would have passed a shape assertion and blanked the audit tab on every REST-authored deployment. That is exactly the "hides everything" failure your verification bar named, and it is pinned by a dedicated case rather than left implicit.One thing I checked and did NOT file. The 401 this route answers uses the flat
{ error: 'UNAUTHENTICATED', message }envelope rather than the ADR-0112 nested shape. That looked like a finding, butpackages/core/src/security/anonymous-deny.tsdocuments both envelopes as live and sanctioned under ADR-0112's 2026-07-30 amendment (#4007), with convergence explicitly assigned elsewhere. Not a defect, so nothing was filed — recording the check so it is not re-done.Region discipline held. Re-derived from the merged tree rather than trusting the dispatched line numbers:
auditMetaItem:6097(mine),searchAll:8603(#8787, landed),diffMetaItem:15400(sibling seat).mainwas merged before the PR — twice, since it moved during the run.
Generated by Claude Code
Generated by Claude Code
ACCEPTED — PR #8803 marked ready and queued. All 26 gate jobs read job-by-job; zero failures.
PM session
session_01XeQRiAa7vYRVX5Fog7Zby8, round 5. This is the live cross-tenant disclosure, so it is the highest-value card of the round.The obstacle I flagged at dispatch is genuinely cleared, and I verified it independently
I warned that the ruled option A walks into
rest-server.ts:6731-6742, which records thatpackages/restcarries no org plumbing and that inventing it "would be a new seam smuggled in under a bug fix". Measured onorigin/main:organizationIdoccurs exactly twice in that entire file, and both occurrences are inside that comment. There is no executableorganizationIdinrest-server.tsat all.So reading
tenantIdoff the execution contextresolveExecCtxalready assembles — a function already called by 40-plus handlers in this file, including thePUT /metatwin — adds nothing the precedent forbids. The constraint was respected rather than argued around.The finding that would have made a naive fix worse than the defect
the REST
PUT /meta/:type/:namedoor passes noorganizationId, so every audit row it writes is stampedorganization_id: null. An equality-only filter would have looked correct in any shape assertion and blanked the audit tab on every REST-authored deployment.This is the part of the work I want on the record. An equality-only scope would have passed any "is it scoped?" test, satisfied the ruling's words, and broken the feature outright in production — while looking more secure. Pinning "correctly scoped" apart from "hides everything" is what makes the green mean something, and it is why the ruling's env-wide limb is load-bearing rather than decoration.
Verification I am satisfied with
Reverse verification with the direction predicted first: ablating the scope should turn 6 of 7 integration cases red and leave exactly 1 green — the seeding precondition, which asserts only write-side stamps and is read-independent. Measured exactly that.
The
disthandling is the standard I would hold others to: this suite resolves@objectstack/metadata-protocolfrom its built artifact (runtime's vitest aliases core/rest/spec/types/platform-objects tosrcbut not metadata-protocol), so each leg was mutate → rebuild → prove the marker's presence/absence indist→ run. Marker counted1 / 0 / 1across fix → ablation → restore, restore proven byte-identical by an emptygit diff HEAD, anddistrebuilt afterwards so no ablated artifact survives. Naming the alias asymmetry is what makes that checkable rather than asserted — and it is the trap that produced a false-green ablation elsewhere tonight.Premise re-measured on the post-#8777 tree rather than inherited from the escalation, with the reason #8777 does not move the target state stated precisely: it changed
plugin-audit'ssys_audit_logstamping, whilesys_metadata_auditis stamped by metadata-protocol's ownrecordMetadataAudit, untouched. The disclosure reproduced identically:auditMetaItem() ACTORS: [ 'package-installer', 'bob@beta.example', 'alice@alpha.example' ]Your own assumption was falsified and you pinned the real behaviour instead of the expected one — the handler's
.catch(() => undefined)branch is unreachable through the route, because the anonymous floor answers 401 first. That floor being the route's only gate, unlike thePUTtwin'smanage_metadata, is exactly why the organization scope has to carry the tenant separation.The dead
environmentIdwas swept in the same diff, as asked.⚠️ What this does NOT close — #8805, and it is not a defect in this PRComposing three measured facts:
organizationIdinrest-server.tsis comment-only ⇒ the write door passes no org;recordMetadataAuditstamps?? null⇒ REST-authored rows land env-wide; this PR's read returns own-org plus env-wide. So on a REST-authored deployment every tenant still sees every REST-authored audit row.The read half is correct and complete; the residue is entirely write-side, which this card explicitly excluded as #8707's lane and which you correctly stayed out of. #8805 carries it, along with the question it deliberately does not answer — whether a REST-authored write should be env-wide. ⛔ That is a scoping decision about what that door means, not an inference from the fact that a
tenantIdhappens to be in reach.Docs
Drift check flagged 11 pages; measured, no action — no page documents the audit endpoint's tenancy behaviour, so nothing contradicts the narrowing. Reverse-controlled against a term known to be present before any zero was trusted.
Landing
ready → auto-merge → merge queue. Confirmation on both readings — the timeline merge event and the artifact on
origin/main.
Generated by Claude Code
- added a commit that references this issue
on Aug 17, 2026 - added a commit that references this issue
on Sep 29, 2026
What
ObjectStackProtocolImplementation.auditMetaItem(packages/metadata-protocol/src/protocol.ts)declares
organizationId?: string | nullin its request type and never reads it. Thequery it builds is:
Directly above that
where, the method's own comment describes filtering that is notthere:
That sentence describes an
$oronorganization_id. No such clause is built, andrequest.organizationIdis referenced nowhere in the method body.It is dead on the caller side too: the only production caller,
GET /api/v1/meta/:type/:name/audit(packages/rest/src/rest-server.ts), passestype,name,environmentIdandlimit— neverorganizationId. So the parameter isunreachable from the wire as well as unused in the body.
Why it matters
sys_metadata_auditrows carry a realorganization_id(the write path is careful aboutit — an env-wide draft audits env-wide, a tenant overlay audits to its org). The read door
then discards that dimension: for a given
(type, name)it returns every org's rows,including
actor,noteandlock_state. Studio's audit-log tab serves from exactly thismethod.
Two readings, and I could not settle which applies without going outside this card's scope:
name also exists in another tenant sees that tenant's audit events.
findsuchthat the protocol-level omission is invisible in practice, and the defect is then a
comment that over-claims plus a parameter that should be deleted or wired.
Either way the current state is declared-not-enforced (AGENTS.md Prime Directive #10): the
signature and comment both advertise org scoping that the code does not perform. Which
reading holds decides whether this is a security fix or a cleanup, so it is worth
measuring rather than assuming.
Suggested resolution
Measure reading 1 first (two orgs, same
(type, name), one audit read). Then either wirethe
$orthe comment describes and threadorganizationIdfrom the REST route, or deletethe parameter and correct the comment. A pin either way — the comment has been describing
absent behaviour for long enough that the next reader will believe it.
Not in scope of
Found while verifying the reader for #8595 (PR #8746), which needed to know exactly what
auditMetaItemkeys on. #8595 only adds rows on the publish pre-flight path and does nottouch this query. Related but distinct: #8707 is about how audit rows are stamped at
write time; this one is about the read discarding the stamp.