Skip to content

check-org-identifier anchors on the literal receiver name session, so a sess.tenantId read — the spelling the removed alias actually had in shipped code — scores zero #9691

Description

@os-project-manager

Found while implementing #9516 (the dead sess.tenantId fallback arm in plugin-audit). Reported, not fixed: #9516's scope is plugin-audit only, and this is a gate defect in scripts/.

Related but distinct: #9444 (closed by #9496) fixed this same gate's comment-masking hole. This is a different hole in the same gate, still live on main after #9496.

The mechanism

scripts/check-org-identifier.mjs is a hard-fail guard whose header states:

This is a hard-fail guard, not a ratchet: the scanned surfaces carry ZERO occurrences today, so any match is a NEW one and fails.

and it scopes itself to examples/, apps/, and packages/. Its detector is:

constPATTERN=/\bsession\s*\??\.\s*tenantId\b/;

The pattern anchors on the literal word session as the receiver — deliberately, so the driver-layer execCtx.tenantId / opts.tenantId axis is never matched. But the receiver of a hook session is routinely bound to a local variable, and the shipped code that carried the removed alias spelled it sess:

constsess: any=(ctxasany).session??{};
...
consttenantId: string|undefined=recordOrgId??sess.tenantId;

sess is not session, so the pattern does not match, and the gate reports clean.

Measured, on main (c07d6e8), against the real pre-#9516 file

Running the gate's own PATTERN over its own maskComments projection of the unfixed packages/plugins/plugin-audit/src/audit-writers.ts:

gate hits on the UNFIXED audit-writers.ts: 0
actual dead-alias reads present: 2 (lines 1340, 1629)

And per-line:

MISSED "const tenantId = recordOrgId ?? sess.tenantId;" <- the real pre-fix line
MATCH "const t = ctx.session.tenantId;" <- the spelling the gate was written for
MISSED "const s = sess; return s.tenantId;" <- one-hop rebind

So pnpm check:org-identifier printed OK (2053 author-facing source file(s), no removed session.tenantId alias) for the entire period those two reads sat in packages/, on every PR. The header's "the scanned surfaces carry ZERO occurrences today" was not an observation about the tree; it was an artifact of the receiver spelling.

Why it matters

The failure direction is silent under-reporting — the class AGENTS.md names as worse than no verifier, because it reports success. Concretely it cost the defect in #9516 its detector: a guard against NULL-tenant audit rows could not fire for several majors, and the one mechanism positioned to notice scored zero.

Reachability beyond the #9516 site

Not yet measured. Whoever picks this up should sweep packages/, apps/ and examples/ for .tenantId reads whose receiver is a local binding of ctx.session under any name, and report the census — sess is one alias, and the gate is blind to every one of them.

Sketch of a fix

Two directions, and they trade off differently:

  1. Widen the receiver set — match a small vocabulary (session, sess, s, hookSession, …). Cheap, but it is a guess at names and the next alias is invisible again; s.tenantId would also collide with unrelated receivers.
  2. Resolve the binding — flag X.tenantId where X is a local initialised from something ending in .session (a one-hop, per-file scan of const X = ....session). Narrower false-positive surface than (1) and it catches the real shape, at the cost of a small amount of scanning state.

Either way, add --self-test cases pinning the sess.tenantId shape in both directions, since the existing self-test's 14 cases all pass today while the gate is blind. A gate that measured its own corpus and found the two live reads would have failed on main, which is the behaviour to restore.

Backlinks: #9516 (the defect this gate did not catch), #3290 (the alias removal), #9444 / #9496 (the previous hole in this gate).

Metadata

Metadata

Assignees

Type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions