fix(analytics): ask the object-level read grant before serving an inline dataset — one admission verdict on every driver - #16860
Conversation
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…m-context census Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…dation it is about Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…alytics-native-sql-authz
📓 Docs Drift CheckThis PR changes 4 package(s): 24 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: ⛔ 5 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails. What this run could not see
Coarse fallback — 135 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): Which tree this was computed onThis run read A worktree cut from an older # while this PR is open — GitHub drops the merge commit once it closes
git fetch origin b85825d20bab8d37c3594844b6d89c467145244a && git checkout b85825d20bab8d37c3594844b6d89c467145244a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 44c849c7d66977ed28814f15853d3e9f984275b7 6d6fbf51eac22f3e16269f03369ae3f9a2a4657a && git checkout -B drift-repro 44c849c7d66977ed28814f15853d3e9f984275b7 && git merge --no-ff 6d6fbf51eac22f3e16269f03369ae3f9a2a4657a
node scripts/docs-audit/affected-docs.mjs --json 44c849c7d66977ed28814f15853d3e9f984275b7
|
Contract review (
|
Pile-up patrol: this PR and #16755 collide on one fileMeasured just now by the
Synthetic controls in the same run: a same-path/different-content pair exits 1 (positive fired), a disjoint-path pair exits 0 (negative clean). ⇒ the reading is the instrument working, ⛔ not a false alarm. ⭐ The conflict is the repo's merge driver doing its job, not a defect. Its own message comes back in the merge output:
⇒ whichever of the two lands second will meet a hand-resolvable prose conflict in that one file. Nothing is broken today and nothing is owed before either merges — it is simply cheaper to know now than to discover it at the merge queue. Both branches are otherwise clean against Generated by Claude Code |
…alytics-native-sql-authz Bring the patch round onto a fresh tree so the gate union is derived from current main rather than a 25-commit-old snapshot. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
… the two silent bridge corners Contract review CHANGES REQUIRED on PR #16860, findings F1, F3 and F9. F1 — `AdmissionLogger` declared `error?` and `warn?`, which is a contract that permits silence (#9754). `warn` is now REQUIRED, so every value of the type carries a destination for a refusal report, and the fail-closed branch reaches for it when `error` is absent instead of dropping the report. F3 — the analytics -> `security` admission bridge collapsed three resolutions into one. A `getService('security')` that THROWS, and a registered service carrying neither `canReadObject` nor `explain`, both returned `undefined` and were then read as "no security service" — admitting the query silently. Those are wired-but-broken providers and `/data`'s middleware does not fall open in either state, so both now DENY and report at `error`. An ABSENT service still admits: that deployment has no object-level gate on `/data` either, so the two doors still agree, which is the property being defended. F9 — `raw-sql-object-routing.test.ts` now asserts that every warning in the fixture IS the deliberate admission-bridge init report, rather than only excluding the routing phrase. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…alytics-native-sql-authz #16755 landed on main and reached the one file both diffs touch, content/docs/permissions/system-context.mdx, exactly as the cross-PR pile-up patrol predicted on this PR. The merge driver declined to defer it (MIXED — a generated half plus hand-written prose) and text-merged it into a conflict. Resolved by hand in the only direction that keeps both sides' prose: main's enriched automation row is adopted whole, including its fourth anchor `#refuseUngrantedRunLifecycleWrite` and the operator run-lifecycle sentence, carried onto this branch's row number (this branch inserted a row upstream in the same table, so every row below it is offset by one). The census counts are re-derived from the merged tree in the follow-up commit, per the driver's own instruction and the regen ordering rule — never regenerated while in MERGE state. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
…tree The merge of #16755 is the case the census gate exists to catch and the merge driver cannot: both sides had independently bumped the SAME declared counts from 106 to 107, so every one of them text-merged cleanly to 107 while the merged tree now holds 108 elevation reads — main's new `#refuseUngrantedRunLifecycleWrite` plus this branch's `canReadObject`. Two correct edits, one wrong sum, and no conflict marker anywhere near it. Re-derived from the merged tree, after the merge was committed and never during MERGE state. Seven declared counts move by one; the gate names each and states there is no mechanical repair, so each was corrected by hand: check-system-context-census: OK — 108 elevation read sites in 20 packages across 45 files, living in 91 symbol(s); the page cites 105 symbol(s) against 105 required, over 129 anchors and 8 file-level citation(s) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37
Contract review (
|
| # | status | evidence on 6d6fbf51e |
|---|---|---|
| F1 (blocking) | closed | read-admission.ts:128-131: AdmissionLogger { error?(…): void; warn(message): void } — warn required, error optional. Fail-closed branch: if (logger?.error) logger.error(report, cause); else logger?.warn(…). Ran check-optional-error-sink-contract.mjs on the extracted head: --self-test exit 0 (19 cases); check exit 0, census 47 sink type(s) … 32 declare it optional beside a REQUIRED warn, 1 permit silence — the 1 is a pre-existing baselined sink in a file this PR does not touch. Ablated on the scratch copy (warn → warn?): exit 1, census 2 permit silence, names read-admission.ts:129; restored byte-equal. CI Lint & Repo Gates job 102130234646 success. |
| F2 (blocking carrier) | closed | Changeset summary line fix(service-analytics)!: …, **BREAKING** banner, <!-- adr-0087: not-required (no-migration-prescription) … -->; levels unchanged (minor ×4). check-adr-0087-registration.mjs --base origin/main --head refs/review/16860 (script blob identical on both refs): exit 0, 1 declared-breaking changeset(s) … [BREAKING+bang] not-required (no-migration-prescription). check-changeset-no-major.mjs same refs: exit 0, no major bump; level axis NOT APPLICABLE locally (needs the pull_request payload) — Check Changeset jobs success on this head cover it. |
| F3 (record, two corners) | closed | plugin.ts:559-609: SecurityAdmissionResolution = usable / absent / unusable. Throwing getService('security') → unusable → ctx.logger.error(…) + return false → PERMISSION_DENIED/403 via assertObjectsReadable. Service with neither canReadObject nor explain → same. Absent → admit, kept as negative control (deliberate; /data has no object gate there either). Logger.error is required on the kernel contract (spec/src/contracts/logger.ts), so the unguarded ctx.logger.error is sound. New admission-bridge-resolution.test.ts (6 cases): both deny cases assert {code:'PERMISSION_DENIED', status:403}, reads empty (DB never reached), and the error sink matched on the resolution message; absent-admit, canReadObject true/false and explain true/false cover the working spellings. Can they fail: yes — under either reverted corner the bridge returns true, the fake engine serves {cnt: 24}, and rejects fails (the body's ablation reading is consistent with the code). Test Core 6/6 shards success on this head, and vitest's default include collects src/__tests__/*.test.ts. |
| F3 (absent-provider default) | accepted-as-recorded | Stated in read-admission.ts docblock, plugin.ts comment, changeset and body. |
| F4 | accepted | security-plugin.ts byte-unchanged since a3fa4e20c; arm-for-arm reading stands. |
| F5 | accepted | analytics-service.ts byte-unchanged since a3fa4e20c; assertReadAdmitted still at callCtx (:901, ahead of strategy selection) and the draft-preview branch (:1221). |
| F6 | accepted | Condition 5 remains escalated on the card; no maintainer ruling as of the card's last comment (5588412885, 16:24Z). Condition 3 stays the honest unit-level reading. |
| F7 | accepted | security-service.ts byte-unchanged since a3fa4e20c. |
| F8 | accepted | verify/src/harness.ts byte-unchanged since a3fa4e20c. |
| F9 (observation) | closed | raw-sql-object-routing.test.ts:266-273: warnings toHaveLength(1), warnings[0] matches the init message No admitObjectRead configured and no "security" service registered at init (verbatim in plugin.ts:947) and does not match the routing degradation phrase. The fixture emits exactly one warn: the read-scope bridge reports the same absent-at-init state at info (plugin.ts:923-930). |
| F10 | accepted | No .skip/.only/.todo in the new test file. |
| F11 | accepted | No other open PR may claim the same single-writer path job 102144417762 success at 16:24Z; #16805 landing-order note stands. |
| F12 | accepted-as-recorded | REST exposure gate (apiEnabled/apiMethods) stated out of scope in the body. |
The patch round itself
- Scope. PR-own commits since
a3fa4e20care08825847fand6d6fbf51e(plus twoorigin/mainmerges,65264f718andc291fc7ce). Their combined touch set is exactly:read-admission.ts,plugin.ts, the newadmission-bridge-resolution.test.ts,raw-sql-object-routing.test.ts, the changeset, andsystem-context.mdx. Every other path ina3fa4e20c..headis main's (the 28 changesets,packages/qa/dogfood/test/authz-*), none in the merge-base diff. No unexplained file. - The feat(runtime): give the two operator run-lifecycle verbs a door (cancel, restore-suspension) #16755 collision.
git diff-tree --cc c291fc7cenames one hand-resolved file:content/docs/permissions/system-context.mdx.6d6fbf51emoves exactly seven declared counts (107→108 at lines 12, 135, 281, 342; 113→114; 103→104; 90→91). Head-vs-origin/mainon that file is the PR's own row-6 insertion and the renumber cascade, nothing else. Rancheck-system-context-census.mjson the extracted head (scratch-onlygit initso itsgit ls-filessweep works):--self-testall cases passed; check exit 0,OK — 108 elevation read sites in 20 packages across 45 files, living in 91 symbol(s); the page cites 105 symbol(s) against 105 required, over 129 anchors …, 18 row reference(s) resolve to their keyed row, 2 declared unheld.The "109 sites" prose at lines 361-362 is the page's dated, explicitly unenforced decomposition, identical on main — not a count this PR owes. - Condition 5 / condition 10. No source line in the diff touches
organization_id,tenantId, Layer 0,buildDriverOptionsorisTenancyDisabled(the only hit is the docblock quotingsql-driver.ts);strategies/, drivers, core and engine are untouched;security-plugin.tsis additive only. Tenant behaviour is byte-identical to main and the PR picks no side. - Trailers. All four new commits (two merges included) carry
Claude-Session:and the harness-writtenCo-Authored-By:trailer — the exemption AGENTS.md names.
CI on 6d6fbf51e
50 check runs: every one success or skipped; none failure, none in progress. The Test Core shard artifact-upload FinalizeArtifact … 403 signature (#16928) is not present on this head — all six shards, the rollup, Dogfood Regression Gate ×3, Dogfood Verify CLI, Temporal Conformance, Build Core, Build Docs, all four Type Check jobs, Spec property liveness, Governed Surface Queue Guard, both single-writer/same-issue guards and Part-of are success. mergeable_state: clean (the body's "unknown" is stale). Head is 9 commits behind origin/main, 11 ahead (8 non-merge + 3 merges); merge-tree clean per the body, consistent with clean. Draft; labels size/xl, needs:contract-review.
New findings
- record — init-report asymmetry between the two bridges. For the same state (no
securityservice registered at init, bridge resolves per query) the read-scope bridge reports atinfo(plugin.ts:923-930) and the admission bridge atwarn(:945-952); the per-queryabsent → admitresolution is not reported at call time at all, only at init. Consistent with the F9 assertion and deliberate as written; recording so nobody reads the two levels as two different states. Expectation: none for this PR; align the levels if either bridge is next touched. - record — the row-scope bridge's collapsed three-way. The body names it ("Noted, not filed"): a throwing
getService('security')leavestrySecurity()undefined, so the query runs unscoped rather than refusing. Correctly left out of this PR (it would move tenant behaviour under condition 5), but "noted, not filed" is the wrong end state for an open fail-open corner on a security bridge. Expectation: a card, filed alongside the condition-5 decision ([finding] The InMemory driver silently ignores the engine's tenant scope for objects that OMIT atenancyblock — its guard only refuses an explicitenabled: true, so memory-driver runs show cross-organization rows a SQL driver refuses #16589 / ats#39 family), not a rider here. - record — quote fidelity. The body's census quote drops the gate's trailing
2 declared unheldclause; the value is unchanged from main and the self-test pins it. Cosmetic.
Maintainer-only merge: yes
The two gates the prior review hung its "yes" on are now closed (CI green, breaking carrier present), so this is a ruling rather than a fix — but the reasons for the maintainer's eyes are unchanged and are not the kind a seat clears: a p0 security-boundary narrowing on a published route; a new member on a published contract (ISecurityService.canReadObject); a declared **BREAKING** shipped as minor under the launch-window lockstep convention, which the maintainer should confirm applies here; condition 5 (which tenant wall is the platform's) sitting undecided in the decision box, with this PR deliberately neutral on it; and the #16805 single-writer ordering on security-plugin.ts. Per the director's handoff (5584760938): on this PASS the card goes pm:awaiting-maintainer, the PR stays draft.
Generated by Claude Code
Fixes #16645
POST /api/v1/analytics/dataset/queryaccepts an inline dataset definition (body.dataset) from any authenticated caller. On a SQL driver the compiled statement ran through the driver's rawexecute()— documented insql-driver.tsas a tenant-isolation bypass ("Unlikefind/update/deleteetc., rawexecute()does NOT inject theorganization_idpredicate") and with no middleware in front of it — so the request reached the database having passed exactly one of the three read layers: the row scope, threaded since ADR-0021 D-C throughgetReadScope. A caller with no grant of any kind on an object received its row count, and withdimensionsits grouped counts by any column. The memory driver refused the identical request, because there the query falls through to the ObjectQL engine and the engine applies all three layers in one place.What changed
One gate, at the door, ahead of strategy selection. ⛔ Not a layer per strategy: two strategies each enforcing their own copy is the cause of this defect, not its remedy —
driver-memoryis correct today precisely because it hands the request to the engine.AnalyticsServicenow asks the object-level read admission once, for the base object and every joined object, before any strategy is chosen. Both strategies — and any strategy added later — inherit one verdict by construction.packages/spec/src/contracts/security-service.tsISecurityService.canReadObject(object, context)— the object-level half of a read, declared besidegetReadFilter's row-level half. Optional; absence falls back toexplain(not optional), never to "admit".packages/plugins/plugin-security/src/security-plugin.tsisSystembypass, no-permission-sets skip, #3545 unresolved-posture deny, ADR-0066 D3 capability AND-gate,allowReadCRUD grant, ADR-0090 D10 delegator intersection. Same primitives the middleware calls. Exposed on the registeredsecurityservice.packages/services/service-analytics/src/read-admission.tsPERMISSION_DENIED/ 403 refusal. A wired provider that throws or answersfalsedenies. The refusal names only the object the caller themselves named. Its log sink declareswarnrequired beside an optionalerror, so a refusal always has somewhere to land (#9754).packages/services/service-analytics/src/analytics-service.tsadmitObjectReadconfig hook; the gate called fromcallCtx(whichquery(),generateSql()and — viaDatasetExecutor— every dataset door share) and from the draft-preview branch, the one dataset path that never reachescallCtx.queryObjects/cubeObjectsis now one derivation feeding both the admission set and the read-scope set, so the set that is admitted and the set that is row-scoped are provably the same set.packages/services/service-analytics/src/plugin.tsadmitObjectReadto thesecurityservice, exactly asgetReadScopeis bridged. PreferscanReadObject; falls back toexplain({object, operation:'read'}).allowedfor a security service that predates the method; warns loudly at init when no security service is registered. Tells three resolutions apart — an absent service admits, a wired-but-unusable one (a throwinggetService, or a service carrying neither method) denies and reports aterror.packages/verify/src/harness.tsbootStack(app, { databaseDriver: 'sqlite-wasm' | 'memory' }). A two-driver equivalence property cannot be measured on one driver — which is how the strategies were allowed to disagree in the first place.Direction: this narrows the accept set. Requests already refused by
/dataare now refused by analytics too; nothing refused becomes admitted. No exemption, no carve-out, no compatible fallback.What "fails closed" does and does not claim. It is a claim about a wired provider. A deployment that registers no
securityservice keeps its pre-PR analytics behaviour, deliberately: that deployment has no object-level gate onGET /data/OBJECTeither — that gate is the absent middleware — so the two doors still agree, and agreement is the property the card asks for rather than refusal for its own sake. What is not left open any more is asecurityservice that is wired and cannot be used; see the patch round below.Patch round — contract review CHANGES REQUIRED (
claude-fable-5-1, isolated seat)Review: #16860 (comment), on head
a3fa4e20c. The substance was judged sound (F4 through F8, F10); four findings were owed and all four are answered here. Patched head:6d6fbf51e.origin/mainwas merged in twice — the branch is published, so ⛔ no rebase, no amend, no force-push at any point. The second merge is the collision the cross-PR pile-up patrol predicted on this PR: #16755 landed and reachedcontent/docs/permissions/system-context.mdx, the one file both diffs touch. It is written up under Verification, because the way it went wrong is worth a reviewer's minute.F1 —
check:optional-error-sinkwas RED, and the union claim was not true of that headThe gate.
AdmissionLoggerdeclarederror?andwarn?. An optionalerrorwith no guaranteed alternative is a contract that permits silence (#9754), and the gate's own fix line is to makewarnrequired. Done:warn(message: string): voidis now non-optional,errorstays optional (hosts legitimately inject reduced sinks, which is why the gate deliberately never asks forerrorto be required), and the fail-closed branch reaches forwarnwhenerroris absent instead of dropping the report. The gate's census line on the patched head:AdmissionLoggeris no longer among the sinks that permit silence, and the single remaining one is a pre-existing ledger entry this PR does not touch. Ablated: reverting the one tokenwarntowarn?on disk (anchor count1 → 0, injected0 → 1, blobbf385d5adiffering from the HEAD blob) turns the gate RED and it names the line —packages/services/service-analytics/src/read-admission.ts:129 · interface AdmissionLogger { error? warn? }— then restores to blob equality with an emptygit diff HEAD.The attestation error, which is the more serious half. The previous revision of this body claimed "121 derived families … 0 NOT-MEASURED, every family green". That was not a reading of that head. It is restated in Verification below from an actual run on
6d6fbf51e, exit code by exit code, and ⛔ nothing is rounded up: a family that answers a prerequisite refusal is NOT MEASURED and is never reported as a pass.F2 — the changeset carrier
The body declared an accept-set narrowing on a published route while the changeset carried neither a
**BREAKING**banner nor an ADR-0087 disposition, socheck:adr-0087-registrationhad nothing to check. Fixed on the shape sibling #16805 uses — a!in the summary line plus the disposition marker. ⛔ No level changed:minoronspec/plugin-security/service-analytics/verifywas already right, and during the launch window the bump level is not the carrier.check-changeset-no-major.mjs's own header says so:The gate's verdict on the patched head:
[BREAKING+bang]is the gate reading both halves. The disposition argues the category on its own terms: nothing authorable is renamed, retired or re-typed, no stored shape moves, and the remedy for a refused caller is a grant held in permission-set data rather than in an authored file — soobjectstack migrate metahas nothing to rewrite. The changeset body carries no migration prescription, which is the one thing that category is mechanically refused for.F3 — the two silent-admit corners now DENY
assertReadAdmittedno-ops with no hook wired, and the bridge returnedtruein three states that were spelled identically (return undefined, thenif (!svc) return truethree lines later). Two of those three were wrong:securityservice resolves/datahas no object gate on that deployment either, so the two doors still agreectx.getService('security')throwserrorcanReadObjectnorexplainerrorBoth new denials are wired-but-broken providers, and
/data's middleware does not fall open in either state — admitting there would reopen exactly the divergence this PR closes, and would do it invisibly.explainis non-optional onISecurityService, so a conforming provider never reaches the second row at all.New cases in
packages/services/service-analytics/src/__tests__/admission-bridge-resolution.test.ts(6 cases). Each denial asserts the ADR-0112 envelope (code: 'PERMISSION_DENIED',status: 403), that the database was never reached (a gate that refuses after running the statement has refused nothing), and that the refusal was reported — a security refusal nobody can see is indistinguishable from a gate that never ran. The absent-service case is kept as the negative control: tightening it would refuse every analytics query on every deployment shipping noplugin-security, which is a different and wrong answer from the one/datagives there.Ablation for both new denial paths is in Verification below.
F9 — the loosened warn assertion, tightened
raw-sql-object-routing.test.ts, case "leaves default-datasource objects exactly where they were", had gone from a blanketnot.toHaveBeenCalled()tonot.toMatch(/is unavailable/)— which admits every other new warning into a case whose whole job is to prove this object's routing did not regress. It now asserts the fixture emits exactly one warning and that it is the deliberate admission-bridge init report, named; a second warning of any wording fails the case.F3's third corner, F11 and F12 — recorded, nothing owed
plugin-securityworking exactly as before.checkon the row that will be stored, afterbeforeInsert#16805 also editssecurity-plugin.ts; the single-writer check is green on this head and the collision is a landing-order fact with a definite end. ⛔ Not resolved here, and no other region of that file is touched by this round./dataalso refuses objects the REST exposure gate (apiEnabled/apiMethods) withholds, and an inline dataset over such an object is not covered bycanReadObject. Outside the card's stated hole and outside this PR; recorded here so it is not mistaken for covered.验收备注
Triage's rubric, verbatim, each with the evidence that answers it.
1. Equivalence assertion, ⛔ not per-layer assertions — on sqlite AND memory
packages/qa/dogfood/test/analytics-inline-dataset-admission.dogfood.test.tsboots the same fixture twice, once per driver, and compares the analytics verdict to/data's verdict rather than to a hard-coded expectation. 10/10 green on both drivers, re-run on the final head.GET /data/OBJECTPOST /analytics/dataset/queryadmission_walledadmission_walledGROUP BY region)admission_walledadmission_openadmission_openadmission_openadmission_walledThe same equivalence is asserted at unit level across both strategy paths (
read-admission-gate.test.ts) and against the real middleware inplugin-security(can-read-object-admission.test.ts— its first block drives the registered middleware with afindand assertscanReadObjectreturns exactly what the middleware decided, for 7 cases).2. The zero-declaration leg
The fixture app declares 0 datasets and 0 dashboards — stated in its header as the load-bearing property, not an omission. One case asserts a
datasetNamelookup 404s on that deployment, which is what proves the refusals are not "the dataset was not found"; the inlinebody.datasetnaming an ungranted object is refused on both drivers.3. The walled-posture leg — turned from inference into a measurement
Measured as the two halves of one chain, each against its real producer:
security.getReadFiltercomposes the Layer-0 wall:getReadFilter('crm_task', MEMBER_CTX)yields{ organization_id: 'org-1' }(plugin-security/src/tenant-layer0-verdict-on-operation.test.ts, pre-existing, unchanged by this PR).read-admission-gate.test.ts, "the tenant wall reaches the inline dataset on the raw-SQL path": the compiled SQL carriesorganization_idbound to the caller's organization; a joined object is walled too (at least 2 occurrences and 2 bindings — a single occurrence would mean a cross-org row can reach theGROUP BYthrough the join); and the wall holds for a caller the admission gate admits, so the two layers are shown to be independent rather than collapsed into one another.@objectstack/verifydocuments that cross-tenant isolation has exactly one honest proof in this repo —multiTenant: truewith the enterprise@objectstack/organizationspackage, which is not in the open core, and whose stand-in'posture-only'"performs no tenant isolation whatsoever". Asserting cross-org isolation on that stand-in would assert nothing and pass. The chain above is the strongest honest measurement available here, and the reviewer's F6 records the same reading: this is unit-level, and whether it satisfies condition 3 or rides with the condition-5 card is the maintainer's call.4. Negative controls
admission_open(owner policy oncreated_by) is 2 on both routes and both drivers —security.getReadFilteris still threaded and still doing the narrowing.read-admission-gate.test.tscarries the same controls at unit level: "ADMITS a granted caller and serves the number unchanged", "leaves behaviour unchanged when NO admission provider is wired", and "the admitted path stays identical across both strategies".securityservice is registered at all" keeps the two new deny corners from being tightened into a blanket refusal.5. The platform-admin disagreement — ⛔ NOT decided in this PR; escalated to the card
This PR does not make sqlite analytics
12/30/40/14agree with/data's0/0/0/0, and states plainly why the choice is not a dev judgement. Measured onorigin/main:/datazeros are not Layer 0. Under the app'ssingleposturecomputeTenantLayer0Verdictreturnsnoneby design (postureEnforcesWall('single') === false), and the boot log says so: "tenancy posture 'single' — Layer 0 is inert". The zeros come from a second, posture-independent wall one layer down:Engine.buildDriverOptionssetsDriverOptions.tenantIdwheneverexecCtx.tenantId !== undefined && !isTenancyDisabled(schema) && !isFederated— no posture is read — anddriver-sqlauto-scopes(organization_id = :tenant OR organization_id IS NULL).service-analytics: plugin-security's posture-gated Layer 0, and driver-sql's posture-independent auto-scope. It is visible on/dataon every driver (On the sqlite driver, tenant-scoped objects (ats_employer,ats_interview,ats_offer) return 0 rows to platform admins holdingviewAllRecords; the memory driver returns all of them ats#39 is that side; [finding] The InMemory driver silently ignores the engine's tenant scope for objects that OMIT atenancyblock — its guard only refuses an explicitenabled: true, so memory-driver runs show cross-organization rows a SQL driver refuses #16589 is the memory driver ignoring the sametenantId), and analytics is only where the two happened to be compared.12/30/40/14become0/0/0/0, but that same predicate also zeroes the seeker's RLS-scopedats_employer9 (its/datacell is already0 (tenant wall)in the card's own table) — which is exactly the negative control condition 4 forbids losing. Changing/datainstead would widen an accept set, which the dispatch rules is the maintainer's floor, never a dev's.single-posture deployments have an organization wall at all, for every read path — far beyond analytics.⇒ Reported back to #16645 as a
needs_decisionrather than settled here, and the PM seat concurred. ⛔ Nothing in this PR silently selects the wider side: the object-level layer this PR adds only refuses; the tenant behaviour of every persona in the acceptance table is byte-identical toorigin/main. The patch round changes nothing about it.6. The immediate-mitigation question — answered: no
Should the native strategy fall back to the engine path whenever it cannot confirm the object-level grant, as an interim?
No, and it is not needed, because the shape shipped here is strictly better on both axes.
NativeSQLStrategy— which is the arrangement that produced this defect. The gate here sits ahead of strategy selection, so there is nothing for a strategy to "fail to confirm": the verdict is already made when the chain is entered, and it is made once for every present and future strategy.canReadObjectper object per query, resolved from a permission-set resolutionplugin-securityalready memoises per execution context, and it runs before any statement is compiled.7. Evidence files
Not rebuilt —
docs/evidence/issue-8/22-…-memory.mdand23-…-sqlite.mdonclaude/issue-8-dashboardsofobjectstack-ai/atsare cited as given. The fixture above reproduces the shape of that matrix inside this repo so it stays measured on every CI run.Noted, not filed — one adjacent corner deliberately left alone
The row-scope bridge (
getReadScope) has the same three-way shape and still collapses it: a throwinggetService('security')leavestrySecurity()returningundefined, so the query runs with no row scope rather than refusing. It is pre-existing, the reviewer examined this bridge and did not name it, and closing it would change whether queries run unscoped or refuse — which would move the tenant behaviour this PR deliberately keeps byte-identical toorigin/mainfor condition 5. ⛔ Out of scope for this round, recorded rather than silently fixed or silently ignored.Clause ② — derived, not inherited
Clause-②: yes— declared by the claiming seat on #16645 (comment 5584629769), which is where that declaration belongs; this PR does not fill it in on anyone's behalf.node scripts/pm/check-widening-tells.mjs --declaration no --diff PR.DIFFanswered✓ 14 changed file(s) read, no widening tell on any declared surface.So there is no widening tell, which matches the direction: this narrows.node scripts/pm/dispatch-gates.mjs --tier --repo objectstack-ai/objectstackanswers no path-derived mandate, withpackages/spec/src/contracts/security-service.tsflagged Clause ② SUSPECT surface.yes. This diff adds a new member to a published contract (ISecurityService.canReadObject) and new keys to three published payloads (AnalyticsServiceConfig.admitObjectRead,AnalyticsServicePluginOptions.admitObjectRead,BootOptions.databaseDriver). Absence of a widening tell is not a clearance, and the gate's own output says so.node scripts/pm/check-governed-merges.mjs --testover the final path list answered0 of 14 path(s) hit the register (5 surfaces, repo-agnostic) … NOT governed. The contract reviewer re-derived the same verdict independently on all 14 paths.needs:contract-reviewwas dropped from both carriers when the review concluded, and ⛔ this round does not re-hang it — re-hanging is the dispatching seat's act once this patched head exists, and the review then runs again on the moved head.Verification
All readings below are from head
6d6fbf51e, whose merge base withorigin/mainis44c849c7d—dispatch-gatesprints noSTALE TREEwarning on it and names that base itself (15 path(s) vs merge base 44c849c7d of 'origin/main' and HEAD). Every command ran in the foreground; heavy runs went throughscripts/pm/os-verify-lock.shon a shared box. Every verdict is quoted from the gate's own line and every exit code was captured with a redirect before any pipe — ⛔ never from a bare$?behind| tail.The #16755 collision, and the thing the merge driver could not catch
The cross-PR pile-up patrol predicted (comment 5586321991) that whichever of this PR and #16755 landed second would meet a hand-resolvable prose conflict in
content/docs/permissions/system-context.mdx. #16755 landed; this is that conflict, and it went wrong in a way worth naming.The file is MIXED — a generated half plus hand-written prose — so the
merge=os-regendriver deliberately declined to defer it and text-merged instead. Two hunks conflicted and were resolved by hand: main's enriched automation row was adopted whole, including its fourth anchor#refuseUngrantedRunLifecycleWrite, carried onto this branch's row number (this branch inserted a row upstream in the same table, so every row below it is offset by one).canReadObject. Identical edits text-merge clean, so every one of them silently survived as 107 while the merged tree holds 108. Two correct edits, one wrong sum, no conflict.check:system-context-censusis what catches it, and it says outright that there is no mechanical repair; the seven were corrected by hand, after the merge was committed and ⛔ never while the tree was in MERGE state (regenerating there rolls the anchor back to the old fork point, with every gate still green):git merge-tree --write-tree origin/main HEADnow exits 0 — the head is clean against current main.Suites, types and lint
@objectstack/service-analyticssuiteTest Files 98 passed (98)·Tests 2190 passed (2190)@objectstack/service-analyticstypecheck(tsc --noEmit)VERDICT command-exit 0Test Files 1 passed (1)·Tests 10 passed (10)pnpm lint(eslint . --no-inline-config), whole repoVERDICT command-exit 0@objectstack/service-analytics. The public byte surface of the other three packages is unchanged by it, so their suites are not re-owed; the counts forspec(465 files / 12955 tests),plugin-security(103 / 1917) andverify(12 / 67) in the earlier revision of this body were measured ona3fa4e20cand are ⛔ not restated as readings of this head. CI runs them on the moved head, which is where that reading belongs.The derived gate union — restated, and this time it is a reading of this head
The previous revision claimed "121 derived families … 0 NOT-MEASURED, every family green." That was not true of
a3fa4e20c—check:optional-error-sinkwas red on it. That claim is the more serious of the two defects the review found, because it is what a reviewer trusts instead of re-running. The honest reading on6d6fbf51e:115 families answered exit 0 on the first pass. Six did not, and none of the six is reported as a pass on the strength of that first answer — every one was a prerequisite refusal against a
packages/spec/distthat no longer matchedsrcafter the merge, which the gates detect and refuse on rather than measuring ("a verdict now would be computed against a build that no longer matches src … a FALSE GREEN on exactly the change it exists to catch", #7122):@objectstack/speccheck:api-surface@objectstack/speccheck:browser-reachable-entries@objectstack/speccheck:dual-source-exports@objectstack/speccheck:entry-nameability@objectstack/speccheck:exported-any@objectstack/speccheck:skill-examples258 prose examples type-check across 3 surface(s)⛔ A prerequisite refusal is
NOT MEASURED, never a pass. The gates say so themselves — "Nothing was measured: this gate exited before running a single check … It is NOT a finding" — so the workspace closure was rebuilt (pnpm exec turbo run build --filter='./packages/*' --filter='./packages/*/*', exactly aslint.ymldoes) and each was re-run to a real verdict. An earlier pass on08825847fhit the same class on seven other families (check:dual-build-cjs-loads,check:i18n,check:type-check-debt,check:docs-transcript-driftand the two@objectstack/lintdoc gates, all wanting an unbuiltdist/); each was likewise built and re-run to exit 0 — e.g.check:type-check-debt→--re-measure: OK — 5 ledger entr(ies) re-measured, 55 raw tsc error(s) total, none above its recorded number.What the 121 does NOT cover — the part the earlier claim left out
dispatch-gates's own reconciliation says the 121 is "NOT a complete account of what CI runs on the PR", and this card reaches families outside it that are honestly NOT MEASURED here:check-shard-attestation.mjs(three jobs, each pinning a shard index and a runner temp path) andcheck-test-completeness.mjs(two jobs, each naming a runner temp log). The tool refuses to invent an invocation, and so does this body: they are measured on CI, not here.check:changeset-no-majoris NOT MEASURED locally, and is not claimed green. Its launch-window half passes (✓ This diff introduces no 'major' bump), but its level axis printsLEVEL AXIS: NOT MEASURED — no clause-② declaration was readable for this PR … no 'pull_request' payload was available to read a declaration from. That limb reads the GitHub event payload, which exists only in CI. Recorded as unmeasured rather than folded into a pass.check:route-envelopewas run explicitly, because it lands indispatch-gates' Silent bucket even when it applies (#16828): exit 0 (17 dispatcher domains · 13 plugin route modules · 4 express-style modules audited). This change adds no response-writing module — the onlyres.json(...)additions in the whole diff are two reads of afetchResponse inside the dogfood fixture — soPLUGIN_ROUTE_MODULESis untouched.⛔ No test was skipped, disabled or quarantined in this round or the previous one.
Ablation — three enforcement points, each proven able to fail
Two legs are new to this round (the F3 corners) and one re-proves the F1 gate reads the type it now constrains. Resolution path matters and is stated per leg: the analytics bridge cases import
../plugin.js— a relative, in-package specifier that resolves tosrc/, sodist/is not on the path and no rebuild applies; the earlier dogfood legs (A/B/C in the previous revision, unchanged and still valid) resolve through builtexportsand were rebuilt withscripts/ablation-dist-preflight.mjsproving the marker reacheddist/.getServicecorner stops denyingunusablereturn becomes{ kind: 'absent' }1 → 0, marker0 → 1; blob84ebcfe9differs from HEADb5d20d49promise resolved "{ rows: [ { cnt: 24 } ], …(1) }" instead of rejecting1 → 0, marker0 → 1; blobe8048555differs from HEADb5d20d49warnbecomeswarn?1 → 0,0 → 1; blobf0d201cfdiffers from HEADbf385d5aread-admission.ts:129 · interface AdmissionLogger { error? warn? }Every leg restored with
git checkout HEAD -- ABSOLUTE_PATHunder atrap … EXIT INT TERM, and restoration is proven by two readings, never by an exit code: the worktree blob equals the HEAD blob (b5d20d49/bf385d5a) andgit diff HEADis empty. The script refuses outright on an empty hash or an unmoved anchor, so a no-op "ablation" cannot be reported as one. No ablation artefact remains in the tree.{cnt: 24}— the ablation reproduces the reported exposure itself, which is what makes those two cases able to fail rather than merely present.🤖 Generated with Claude Code
https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37