Skip to content

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

@hotlong

What

ObjectStackProtocolImplementation.auditMetaItem (packages/metadata-protocol/src/protocol.ts)
declares organizationId?: string | null in its request type and never reads it. The
query it builds is:

const where: Record[string, unknown] = {
    type: singular,
    name: request.name,
};

Directly above that where, the method's own comment describes filtering that is not
there:

Org-scoped lookup: include rows for the specific org AND env-wide (organization_id IS
NULL) rows so the editor sees both tenant overlays and env-level package writes.

That sentence describes an $or on organization_id. No such clause is built, and
request.organizationId is 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), passes type,
name, environmentId and limit — never organizationId. So the parameter is
unreachable from the wire as well as unused in the body.

Why it matters

sys_metadata_audit rows carry a real organization_id (the write path is careful about
it — 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, note and lock_state. Studio's audit-log tab serves from exactly this
method.

Two readings, and I could not settle which applies without going outside this card's scope:

  1. Real disclosure — a tenant admin viewing the audit tab for a metadata item whose
    name also exists in another tenant sees that tenant's audit events.
  2. Compensated lower down — the engine/driver applies tenant scoping to find such
    that 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 wire
the $or the comment describes and thread organizationId from the REST route, or delete
the 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
auditMetaItem keys on. #8595 only adds rows on the publish pre-flight path and does not
touch 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.

Activity

  1. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    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 ends

    measured
    organizationId is declared protocol.ts:6063 — organizationId?: string | null in auditMetaItem's request type
    …and never reaches the query the where is built as exactly { type: singular, name: request.name } — no org term
    …and is never passed by the caller rest-server.ts:6410 passes type, name, environmentId, limit — no organizationId

    So the parameter is dead on both ends exactly as filed, and the comment above that where describes an org-scope $or that 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_audit does carry the tenant column. packages/metadata-core/src/objects/sys-metadata-audit.object.ts:101 declares organization_id: Field.lookup('sys_organization', …), and :190 declares the index { fields: ['organization_id', 'occurred_at'] } — described at :29 as "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 of auditMetaItem ignoring 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. If sys_metadata_audit ever 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

  2. added theissue type on Aug 14, 2026
  3. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    Triage: lands in packages/metadata-protocol ⇒ domain:metadata. Queued. Type Bug — declared ≠ enforced on both ends (Prime Directive #10).

    Premise re-verified on origin/main @ 7246d63, not carried from the filing

    claim reading
    param declared protocol.ts:6063 — organizationId?: string | null in auditMetaItem'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 where has 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, limit only ✅

    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_audit does declare organization_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 the single-posture gap the seat flagged — no wall there to carry it.
    • Wall does not carry it ⇒ real cross-tenant disclosure of actor / note / lock_state through Studio's audit tab. Fix: wire the $or and thread organizationId from 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


    Generated by Claude Code

  4. self-assigned this
    on Aug 14, 2026
  5. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    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: auditMetaItem only, :6091–~:6170 — and, only if the measurement calls for threading the parameter, packages/rest/src/rest-server.ts around :6411–:6421, plus tests (stop on breach; explain in the report)
    Container & model: M, mode:subagent, model: opus
    Serial constraints cleared: region-declared, because protocol.ts is contended three ways. Line numbers taken from origin/main at dispatch, not from the card:

    region lines holder
    canonicalizeMetaRequestType + call sites + the saveMetaItem door :211, 6 sites PR #8770, open and unlanded
    searchAll ~:7177–:7411 #7643, in flight
    auditMetaItem :6091–~:6170 this 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's actor / note / lock_state for 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

  6. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    🚨 MEASURED: reading 1 holds — this is a live cross-tenant disclosure. No fix pushed; escalating first, as dispatched.

    Dev seat session_01XeQRiAa7vYRVX5Fog7Zby8, branch claude/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_audit declares organization_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 read

    Real ObjectQL + real SqlDriver over on-disk better-sqlite3 + real ObjectStackProtocolImplementation + the real SysMetadataObject / SysMetadataHistoryObject / SysMetadataAuditObject declarations. Rows seeded through the real saveMetaItem write path, not hand-inserted. Harness copied from packages/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 organizationId actor
    1 org_alpha alice@alpha.example
    2 org_beta bob@beta.example
    3 (none — env-wide) package-installer

    Then 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_alpha opening Studio's audit tab receives org_beta's actor, and with it that row's note, lock_state, code, operation, source and request_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.applyTenantScope builds (organization_id = :tenant OR organization_id IS NULL) only when DriverOptions.tenantId is set. The engine sets it from execCtx.tenantId in buildDriverOptions (packages/objectql/src/engine.ts:2850 — hasTenant requires execCtx?.tenantId !== undefined). auditMetaItem passes 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.find has no positions, no permission sets and no userId, so it takes this return next(). computeLayeredRlsFilter — and with it the objectFields.has('organization_id') gate at :4537 that 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) returns null outright under the single posture, and under isolated it reads context?.tenantId — which is absent here, so :146 would 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 its PUT /meta/:type/:name sibling at :6100, which gates on manage_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 disposition sdk, client meta.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, so rest-server.ts is 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-6742 states, about the sibling /published route, that packages/rest carries no resolveActiveOrganizationId and 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 — thread organizationId from 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) assembles tenantId: authz.tenantId, and resolveExecCtx is already called by 15+ handlers in rest-server.ts — including the PUT /meta/:type/:name twin 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, thread organizationId from REST hand-built $or [{organization_id: oid}, {organization_id: null}]; new org-resolution plumbing in packages/rest yes hits the :6731 precedent; hand-reimplements the wall the platform already owns; a second place to get tenancy right
    B — 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 ExecutionContext into the engine read auditMetaItem accepts a context and passes { context } to engine.find; route supplies it via the existing resolveExecCtx yes 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 $or re-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 auditMetaItem is exactly the "second implementation of the same contract, blind to every widening mechanism" shape the security plugin's own :5492 comment 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 | null that 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 the single tenancy posture Layer 0 contributes nothing by design, so on a single-posture deployment C leaves the read unscoped. That is correct-by-design for single (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

    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

  7. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor
    {
      "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

  8. 1 remaining item

  9. removed their assignment
    on Aug 14, 2026
  10. hotlong commented on Aug 14, 2026

    @hotlong
    ContributorAuthor

    🚨 Reading 1 CONFIRMED — this is a live cross-tenant disclosure, measured. pm:dispatched → needs-user-decision, security + target:v17 applied, 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 + real SqlDriver (sqlite) + real ObjectStackProtocolImplementation. Three saves of one view name — under org_alpha, under org_beta, and env-wide — through the real saveMetaItem write path. Then one auditMetaItem({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. buildDriverOptions sets DriverOptions.tenantId only from execCtx.tenantId (engine.ts:2850), and auditMetaItem passes no context. Recorded DriverOptions came back bare:
      DRIVEROPTIONS context-less:   {"where":{"type":"view","name":"shared_grid"}}
      DRIVEROPTIONS tenant-scoped:  {"where":{...},"context":{"userId":"u1","tenantId":"org_alpha"},"tenantId":"org_alpha"}
      
      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.
    • plugin-security's Layer 0 never engages. The middleware takes its principal-less return next() at security-plugin.ts:1230-1236 — roughly 3,300 lines before the objectFields.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. computeTenantLayer0Filter returns null under single (tenant-layer.ts:119) and the fail-closed deny sentinel under isolated with 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 its PUT twin at :6100, which gates on manage_metadata. So the cohort is any authenticated principal of any tenant, not "a tenant admin". The route is on the published SDK surface (ledger disposition sdk, client meta.getAudit). The dispatcher twin has no audit route, so rest-server.ts is 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 $or and thread organizationId from the REST route. Closes it, but rest-server.ts:6731-6742 explicitly records that 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". 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 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 twin 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 $or re-derives that by hand and can drift from it.

    ③ AI-agent error-resistance → C, decisively. An optional organizationId?: string | null that 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/rest deliberately does not have.

    ⚠️ The caveat that needs ruling explicitly, not assuming

    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 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-6426 also passes environmentId to auditMetaItem — 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

  11. hotlong commented on Aug 15, 2026

    @hotlong
    ContributorAuthor

    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:

    1. 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/main after 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 at ab8b10f82.

    2. It does not change the disclosure verdict. The measured fact — one context-less auditMetaItem read 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

  12. os-zhuang commented on Aug 15, 2026

    @os-zhuang
    Contributor

    Maintainer ruling (delegated adjudication; delegation 2026-08-15 verbatim 「决策你直接帮我做」, batch confirmed 「同意」): wire it, do not delete it.

    Implement the $or the method's own comment describes — rows for the specific organization AND env-wide (organization_id IS NULL) rows — and thread organizationId from 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_id carefully — 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:metadata seat dispatches (target:v17 retained).


    Generated by Claude Code

  13. self-assigned this
    on Aug 15, 2026
  14. hotlong commented on Aug 15, 2026

    @hotlong
    ContributorAuthor

    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: auditMetaItem only, from :6091; plus packages/rest/src/rest-server.ts (the GET /api/v1/meta/:type/:name/audit handler at :6404 and 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). Not claude-fable-5: the mandatory clause is scoped to accept/reject changes on packages/spec, which this card does not touch; the accept-set decision here was already ruled, not delegated to the dev.
    Serial constraints cleared: protocol.ts is a contended file and is running under region-level declaration. Concurrent regions this round, measured on origin/main 84cb121eb at 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 $or the method's own comment describes — rows for the specific organization AND env-wide (organization_id IS NULL) rows — and thread organizationId from 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-6742 states, about the sibling /published route, that packages/rest carries no resolveActiveOrganizationId and 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 assembles tenantId: authz.tenantId and resolveExecCtx is already called by 15+ handlers in this file including the PUT /meta twin 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 (buildDriverOptions sets DriverOptions.tenantId only from execCtx.tenantId, engine.ts:2850); plugin-security Layer 0 falls open at its principal-less return next() (security-plugin.ts:1230-1236); and computeTenantLayer0Filter yields nothing useful under either posture.
    • Blast radius is wider than the card as filed: the handler at rest-server.ts:6404 has no capability gate, unlike its PUT twin at :6100 which gates on manage_metadata. Cohort = any authenticated principal of any tenant, on the published SDK surface (meta.getAudit).

    ⚠️ Re-measure against the current origin/main before 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 at ab8b10f82. 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-6426 also passes environmentId to auditMetaItem, 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

  15. hotlong commented on Aug 15, 2026

    @hotlong
    ContributorAuthor
    {
      "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) assembles tenantId: authz.tenantId, and resolveAuthzContext (packages/core/src/security/resolve-authz-context.ts) derives that from keyPrincipal.tenantId ?? sessionData.session.activeOrganizationId — i.e. tenantId is the active organization id. So option A is implementable without the seam rest-server.ts:6731-6742 forbids, 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/:name door passes no organizationId to saveMetaItem, so every audit row that door writes is stamped organization_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, but packages/core/src/security/anonymous-deny.ts documents 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). main was merged before the PR — twice, since it moved during the run.


    Generated by Claude Code


    Generated by Claude Code

  16. hotlong commented on Aug 15, 2026

    @hotlong
    ContributorAuthor

    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 that packages/rest carries no org plumbing and that inventing it "would be a new seam smuggled in under a bug fix". Measured on origin/main: organizationId occurs exactly twice in that entire file, and both occurrences are inside that comment. There is no executable organizationId in rest-server.ts at all.

    So reading tenantId off the execution context resolveExecCtx already assembles — a function already called by 40-plus handlers in this file, including the PUT /meta twin — 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/:name door passes no organizationId, so every audit row it writes is stamped organization_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 dist handling is the standard I would hold others to: this suite resolves @objectstack/metadata-protocol from its built artifact (runtime's vitest aliases core/rest/spec/types/platform-objects to src but not metadata-protocol), so each leg was mutate → rebuild → prove the marker's presence/absence in dist → run. Marker counted 1 / 0 / 1 across fix → ablation → restore, restore proven byte-identical by an empty git diff HEAD, and dist rebuilt 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's sys_audit_log stamping, while sys_metadata_audit is stamped by metadata-protocol's own recordMetadataAudit, 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 the PUT twin's manage_metadata, is exactly why the organization scope has to carry the tenant separation.

    The dead environmentId was swept in the same diff, as asked.

    ⚠️ What this does NOT close — #8805, and it is not a defect in this PR

    Composing three measured facts: organizationId in rest-server.ts is comment-only ⇒ the write door passes no org; recordMetadataAudit stamps ?? 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 tenantId happens 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions