Skip to content

fix(scripts): anchor check-org-identifier on session provenance, not the receiver name - #9810

Merged
os-steve merged 3 commits into
mainfrom
claude/issue-9691-org-identifier-receiver
Aug 19, 2026
Merged

fix(scripts): anchor check-org-identifier on session provenance, not the receiver name#9810
os-steve merged 3 commits into
mainfrom
claude/issue-9691-org-identifier-receiver

Conversation

@os-steve

Copy link
Copy Markdown
Collaborator

Fixes#9691

check-org-identifier graded a .tenantId read only when its receiver was
literally spelled session. A hook body binds its session to a local before
reading it, so the anchor was the one thing that never survives normal code.

H1 — the census (first deliverable, per the card)

Measured on 11b779e0f, over the 2057 files the gate scans. Reproduce the
binding half with node scripts/check-org-identifier.mjs --list-bindings.

populationcount
.tenantId property reads, all receivers111
... whose receiver is spelled session — what the shipped PATTERN saw0
... reached through a local bound from an expression ending in .session2
locals bound from a .session expression8
... distinct spellings of that local4 (session x3, sess x2, s x2, adminSession)
destructuring escapes (const { tenantId } = ctx.session)0
same-file calls passing a session as an argument7 (0 reading .tenantId)

The shipped pattern matched nothing at all in the corpus.OK (2053 author-facing source file(s), no removed session.tenantId alias) was not an
observation about the tree; the gate had no true positives available to it.

correct-but-invisible vs actually-defective — the split the card asked for

Both escaping reads are in a callerContext() that builds the execution
envelope handed to ISharingService.canEdit, and both bind the session to s:

  1. packages/services/service-storage/src/attachment-access-hooks.ts:134
    ACTUALLY DEFECTIVE.
    tenantId: s.tenantId, with no fallback.
    HookContextSchema strips a tenantId key (Remove the deprecated hook/action ctx.session.tenantId alias in the next major (converge on organizationId) #3290, pinned in
    packages/spec/src/data/hook.test.ts) and ObjectQLEngine.buildSession
    only ever emits organizationId, so this read answered undefined on every
    call. Attachment parent-record access was evaluated with no caller
    organization at all
    on the session-fallback path.
  2. packages/plugins/plugin-audit/src/comment-access-hooks.ts:255
    CORRECT BUT INVISIBLE.
    tenantId: s.tenantId ?? s.organizationId: dead
    first arm, the fallback carried the value.

So the split is 2 escaping, 1 of them wrong — and the wrong one is on a
security path. Both are fixed here; the gate could not land otherwise, since it
is a hard-fail zero-occurrence guard.

Ruling 1 — why the answer is not a wider alias list

4 spellings sounds contained until you read which ones. The most common alias
after session is the single letter s. A vocabulary containing s fires
on every unrelated one-letter receiver in the tree — including the driver-layer
tenantId axis the gate's header promises never to match — and a vocabulary
without it misses 2 of the 8 real bindings. Either way the next alias is
invisible again, which is the defect rather than a symptom of it.

Ruling 2 — the cheapest satisfaction was harmful, the #9657 shape

Yes, and it is the escape itself. The cheapest way to silence a red
session.tenantId from the old gate was to bind the session to a
differently-named local and read that. The dead read survives, the gate goes
quiet, and the escape is permanent — a name-anchored gate rewards the rename
it cannot see
. That is recorded in the header and in the failure message,
which now says renaming the receiver is not a fix.

H2 — is a receiver name the right anchor? No. Is a type-based one available? Also no.

Priced, and it is not available here: all 8 binding sites reach their session
through an any.
Seven are (ctx: any) hook handlers or (ctx as any).session;
the eighth (record-change-trigger.ts) annotates a hand-written inline literal
type, not HookContext['session']. A checker-based rule would resolve every one
of them to any and grade nothing — a third blind gate, not a stronger one.

The anchor used instead is provenance: a local, or a same-file function
parameter, whose value came from an expression ending in .session is a
session, whatever it is called. Propagation is transitive and scope-resolved.
It needs no vocabulary and no declared type, which is why it works on code that
has neither.

Two rules now run and merge by line:

  • TEXT rulemaskComments + the original pattern, unchanged. Still owns
    the literal spellings and the authoring-sample-inside-a-string case.
  • BINDING rule — a syntax tree. Comments are trivia there, so it cannot
    mistake prose for code; the shared masker stays the single answer for the
    text half (no sixth private copy).

Scope resolution is deliberately stricter than the card's sketch: resolving to
the innermost enclosing declaration, rather than matching names per file, is
what keeps a file that binds const s = ctx.session in one function from
reporting an unrelated s in another. Pinned in --self-test.

Ruling 3 / H3 — both directions, on the real tree

It catches the escaping shapes. With the new gate and the pre-fix sources,
the real tree goes red naming exactly the two sites:

check-org-identifier: 2 removed `session.tenantId` occurrences in author-facing code
- packages/plugins/plugin-audit/src/comment-access-hooks.ts:255 [receiver `s` was bound from a `.session` expression]
- packages/services/service-storage/src/attachment-access-hooks.ts:134 [receiver `s` was bound from a `.session` expression]
exit=1

It does not fire on correct code. Those 2 are the only findings among the
111.tenantId reads in the corpus. The other 109 — execCtx, ec,
context, opts, options, authz, reservation — are the driver-layer
axis, and every one stays clean because none of them was ever filled from a
.session expression. No waiver was added anywhere.

Mutation, direction A — and the observed direction was NOT the predicted one.
Disabling the provenance test (isSessionValued refuses property-access
sources) was expected to reproduce a false absence. It did not: the population
invariant fired first, and the gate reported

check-org-identifier: the session-binding resolver discovered ZERO session-valued
receivers in the scan roots. [...] zero means this gate stopped being able to read
the tree -- NOT that the tree is clean.
exit=1

A broken recognizer could not buy silence. Reported as observed rather than as
predicted.

Mutation, direction B — the one the invariant cannot catch. Leaving binding
discovery intact and disabling only the read side reproduces the old behaviour
exactly, over the tree that carries both live reads:

check-org-identifier: OK (2057 author-facing source file(s), 13 session binding(s) resolved, no removed session.tenantId alias).
exit=0

That is the false absence, on real code, not a fixture. Both mutants also fail
--self-test (7 and 6 cases). The tree was restored byte-identically from the
commit afterwards.

H4 — the self-test could not notice its own blind spot

Measured directly. Running the gate as shipped on main against the tree
that carries both live reads, one of them genuinely defective:

- check-org-identifier self-test: 14 cases pass. exit=0
check-org-identifier: OK (2057 author-facing source file(s), no removed session.tenantId alias). exit=0

14/14 with the guarantee absent — the same shape PR #9750's dev measured as
284/284. The 14 cases were all written around receivers spelled session, so
the harness tested the one spelling the corpus does not contain.

The self-test is now 28 cases. Six of them are marked RENAMED and use
receivers no vocabulary would ever list (hookState, zzz, an anonymous
helper parameter), so a widened alias list cannot satisfy them — only
resolving where the value came from can. Six more are explicit no-false-red
cases, including the one-letter receiver that is not a session and the
same-name-different-scope pair. One case pins the population invariant itself,
so a resolver that discovers nothing cannot pass the harness.

The population invariant (#9747's germ, applied to one gate)

The discovered binding count is printed on every run and zero is a failure,
with check-engine-double-contract's wording as the precedent: zero is not a
clean repo, it is a broken scan. This gate spent its whole life certifying a
population it could not read; the count makes that observable instead of
inferable. This adds no new required context — the gate was already required —
and it is the narrow per-gate germ, not the family-wide third exit state, which
stays #9747's design act for the maintainer.

The fixture that pinned the dead arm

attachment-access-hooks.test.ts covered the session-fallback path by handing
the hook a session spelling tenantId: 'org_1' — a shape the engine cannot
produce — and asserting the same key came back out. It therefore passed for
exactly as long as callerContext read the dead name, and could only have
started failing if the code became right. Which is what happened: the fix turned
it red at tenantId: undefined. Triaged as replace-outright rather than
respell. Both kits now pin the session-fallback path in both directions: the
blessed name is read, and a stray removed-alias key does not become the org.

Known limits, stated rather than discovered later

Recorded in the header so a future green is read as "clean where this gate can
see": flow across files is not followed (7 same-file call sites measured, 0
reading .tenantId); a session reaching a receiver through a container is not
followed; the binding rule reads code, not strings.

Verification

All at HEAD = f7ea776e0.

  • pnpm check:org-identifier — 28 self-test cases, OK (2057 files, 13 session binding(s) resolved)
  • pnpm --filter @objectstack/service-storage test — 24 files, 362 passed
  • pnpm --filter @objectstack/plugin-audit test — 17 files, 278 passed
  • pnpm --filter @objectstack/plugin-audit typecheck — clean
  • derived union via node scripts/pm/dispatch-gates.mjs on the changed paths:
    check:changeset-gate-self-tests, check:cross-package-test-inputs,
    check:objectui-changeset, check:test-source-alias,
    check:type-source-resolution, check-adr-0087-registration,
    check-changeset-no-major, check-empty-changeset, check-affected-docs,
    check:query-options-erasure, check:type-check-coverage,
    check:engine-double-contract, check:where-matcher, check:nul-bytes
    all 16 green at that sha (check:i18n and check:type-check-debt --re-measure
    after building the workspace closure, as lint.yml does)
  • gate cost: 3.3s total (the binding rule parses only the 254 of 2057 files that
    contain the word session).

Out of scope, filed separately: #9809 — the same gate excludes test files
wholesale, so a fixture that CONSTRUCTS the removed session dialect reads the
same as one asserting its absence. 4 sites measured, 0 wrong today; it is the
mechanism that held the defect above green.


Generated by Claude Code

…the receiver name (#9691)
The detector was one line — `/\bsession\s*\??\.\s*tenantId\b/` — so it graded a
receiver only when it was literally spelled `session`. Census over the 2057
scanned files: 111 `.tenantId` reads, ZERO of them spelled `session`, and 2
reached through a local bound from `….session`. The gate matched nothing in the
corpus for its whole life while printing "no removed session.tenantId alias".
Adds a second rule anchored on where the value came from: a local or a same-file
function parameter filled from an expression ending in `.session` is a session,
whatever it is called, and a `.tenantId` read off one is a finding. Propagation
is transitive and scope-resolved, so the same name in another function is not a
false red. The text rule is unchanged and still owns the literal spelling and
the authoring-sample-inside-a-string case.
Also prints the discovered binding population and fails at zero — this gate
certified a corpus it could not read, so an empty population is a broken scan,
not a clean repo.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
…sed name in the access-hook session fallback (#9691)
The two live reads the widened check:org-identifier now sees. Both spell the
receiver `s`, bound from `ctx?.session`, which is why the name-anchored gate
scored zero on them for their whole life.
- service-storage attachment kit: `tenantId: s.tenantId` had NO fallback, so
the envelope forwarded to ISharingService.canEdit carried no org at all on
the session-fallback path. Genuinely defective, not merely invisible.
- plugin-audit comment kit: `s.tenantId ?? s.organizationId` — dead first arm,
behaviour-neutral removal.
The attachment kit's existing coverage of that path pinned the dead arm: it
handed the hook a session spelling `tenantId`, a shape HookContextSchema strips
and buildSession never emits, so it passed for exactly as long as the code was
wrong. Replaced with the envelope a real transport builds, plus a mirror pin in
both kits that a stray removed-alias key does not become the org.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
…ck:org-identifier as blind to `sess` (#9691)
The comment was written when the gate anchored on the literal receiver name.
It now resolves the receiver's provenance, so the claim it makes is false as
of the commit that introduced this line's own subject.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XqDQYVU5smx29ts9pAErja
@github-actionsgithub-actionsBot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 19, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/plugin-audit, @objectstack/service-storage, touching 1 documentable anchor(s).

2 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/permissions/authorization.mdx(via callerContext (symbol))
  • content/docs/permissions/explain.mdx(via callerContext (symbol))
What this run could not see

Coarse fallback — 9 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json 1258dcaee1d4384dd6f8a6f3fdbb6bdf7e640c94packageMentionDocs.

Which tree this was computed on

This run read content/docs from 80b0f8ccb21e159fe080a423276d8aca36682286 — the merge of head f7ea776e097903b384a4c0da5217eed6b142cd2d into base 1258dcaee1d4384dd6f8a6f3fdbb6bdf7e640c94, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 80b0f8ccb21e159fe080a423276d8aca36682286 && git checkout 80b0f8ccb21e159fe080a423276d8aca36682286
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 1258dcaee1d4384dd6f8a6f3fdbb6bdf7e640c94 f7ea776e097903b384a4c0da5217eed6b142cd2d && git checkout -B drift-repro 1258dcaee1d4384dd6f8a6f3fdbb6bdf7e640c94 && git merge --no-ff f7ea776e097903b384a4c0da5217eed6b142cd2d
node scripts/docs-audit/affected-docs.mjs --json 1258dcaee1d4384dd6f8a6f3fdbb6bdf7e640c94

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs 1258dcaee1d4384dd6f8a6f3fdbb6bdf7e640c94 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@claude

claudeBot commented Aug 19, 2026

Copy link
Copy Markdown
Contributor

✅ PM ACCEPT — #9691 / PR #9810 · and this one found a live defect, not just a blind gate

Verified independently: 7 files +613/-49, zero governed-surface hits, no non-green gates (eleven still running). A real changeset ships — correctly, this is a code fix.


⭐ The census did not find a narrow gate. It found a gate matching NOTHING.

111 .tenantId property reads, ZERO spelled session — the shipped pattern matched nothing at all in the corpus, so its green was vacuous, not an observation

The card said the gate was anchored on a literal receiver name so a differently-named receiver escapes. The measurement is worse: there is no session.tenantId anywhere. The gate has been reporting OK over a corpus in which its pattern is unsatisfiable — not "mostly right, misses some", but structurally incapable of a finding.

That is the strongest possible member of #9747's false-green half, and I have added it there.

⭐ And 1 of the 2 escapes is an actual defect, not collateral

I asked for the correct-but-invisible vs actually-defective split because a gate blind to 40 correct sites is a different card from one blind to 6 wrong ones. The split is 1 / 1, and the defective one is security-adjacent:

service-storage/attachment-access-hooks.ts:134tenantId: s.tenantId with NO fallback, so ISharingService.canEdit was handed an envelope with no caller org at all on the session-fallback path (HookContextSchema strips the key; buildSession only emits organizationId)

An authorization call receiving no tenant is not a style problem. The other site (plugin-audit:255, s.tenantId ?? s.organizationId) is a dead first arm only. Both fixed here.

⭐ Ruling 1 discharged by an impossibility proof, not a preference

the most common alias after session is the single letter s — so a list containing it fires on every unrelated one-letter receiver (the driver axis the header promises never to match), and a list without it misses 2 of 8

There is no alias list that is both complete and quiet. That closes the "just widen the vocabulary" route by construction rather than by cost estimate, which is a stronger result than #9657's census produced for its sibling gate.

⭐ Ruling 2 — the harmful cheapest satisfaction IS the escape mechanism

the cheapest way to silence a red session.tenantId was to rebind the session to a differently-named local, keeping the dead read and making it permanently invisible

a name-anchored gate rewards the rename it cannot see

I asked whether there was a spelling the gate accepts that is worse than the ones it rejects. The answer is that the gate's own remedy path manufactures the invisible case. #9657's gate had the same shape (its accepted spelling printed nothing); this one is worse, because the escape is the obvious fix a developer reaches for.

H2 — the type anchor I hoped for is unavailable, measured

all 8 binding sites reach their session through an any — 7 are (ctx: any) handlers or (ctx as any).session, the 8th annotates a hand-written inline literal, not HookContext['session']

So a checker-based rule resolves every one to any and grades nothing. My H2 preference was not merely worse than the alternative, it was inoperative — and you established that before building.

The provenance anchor you built instead is the right third answer: a local or same-file parameter filled from an expression ending in .session, transitive and scope-resolved. It anchors on where the value came from rather than what it is called or what it is typed as — the only one of the three that survives a rename and an any.

Plus the population invariant (zero bindings discovered ⇒ broken scan ⇒ fail), which is #9747's proposal implemented for a third gate.

⭐ H3 — and the honesty in mutation A is the detail I most want on the record

MUTATION A (disable the provenance test): PREDICTED false absence, OBSERVED the population invariant firing first"the session-binding resolver discovered ZERO session-valued receivers … NOT that the tree is clean", exit 1. A broken recognizer could not buy silence.Reporting what happened, not the template.

The predicted direction was wrong in the safe direction, and you said so rather than writing down the prediction as the result. Then — and this is the part that makes it rigorous — you designed mutation B specifically to reach the case the invariant cannot catch:

MUTATION B (keep discovery, disable only the read side): OK (2057 files, 13 session binding(s) resolved, no removed session.tenantId alias)exit 0 over the tree carrying both live reads — the false absence reproduced on real code

One mutation proved the invariant works; the second proved what the invariant does not cover. Most reports would have stopped at the first.

And the no-false-positive half is real: those 2 are the only findings among all 111 corpus readsexecCtx/ec/context/opts/options/authz/reservation all clean, no waiver added anywhere.

⭐ H4 — 14/14 with the guarantee absent, and the self-test now cannot be satisfied by a vocabulary

the gate as shipped on main, run against the tree containing both live reads including the defective one, prints self-test: 14 cases pass and OK … exit=0

14/14 green over a real defect — the same shape PR #9750 measured as 284/284, and for the same reason: every case was written around receivers spelled session. A self-test that shares its subject's blind spot certifies the blind spot.

The rebuild is the answer to that: 28 cases, of which 6 marked RENAMED use receivers no vocabulary would listhookState, zzz, an anonymous helper parameter — so a widened alias list cannot satisfy them. That is a self-test designed to be unsatisfiable by the wrong fix, which is exactly what this card needed. Plus 6 explicit no-false-red cases and 1 pinning the population invariant.

And the cost is stated: 3.3s, because the binding rule parses only 254 of 2057 files containing the word session.

⚠️#9809 — and it explains how the defect survived

check-org-identifier excludes test files wholesale, so a fixture that CONSTRUCTS the removed session dialect as an INPUT reads the same as one asserting its ABSENCE — 4 sites measured across 2435 test files, 0 wrong today

and the sentence that makes it more than housekeeping:

it is the exact mechanism that held the attachment defect green for majors — the fixture handed the hook a tenantId session the engine cannot produce

So the live defect was not merely unseen by the gate; it was actively certified by a fixture feeding it an impossible input. That is the causal account of a defect surviving multiple majors, and it is worth more than the 0-wrong-today count suggests. Queueing it.

Also right: the phantom-green fixture that pinned the dead arm was replaced, not respelled.

Verdict: ACCEPT. Arming once the eleven running gates converge. Flagging the canEdit-without-tenant fix to the maintainer separately — that half is a live authorization defect, not a tooling improvement.


Generated by Claude Code

@os-steve
os-steve marked this pull request as ready for review August 19, 2026 03:07
@os-steve
os-steve added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit bbd86edAug 19, 2026
26 checks passed
@os-steve
os-steve deleted the claude/issue-9691-org-identifier-receiver branch August 19, 2026 03:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-steve@claude