audit: the tenant fallback in writeAudit reads session.tenantId, a key removed in v11 — the guard against NULL-tenant audit rows can never fire

Found while implementing #8992 (record-view auditing) — reading audit-writers.ts to match its tenant-stamping precedence on the new read row.

The defect

packages/plugins/plugin-audit/src/audit-writers.ts, in writeAudit:

const tenantId: string | undefined = recordOrgId ?? sess.tenantId;

sess is HookContext.session, and that object has no tenantId key. The engine builds it in ObjectQL.buildSession (packages/objectql/src/engine.ts) as an object literal with a fixed key set — userId, organizationId, positions, accessToken, and the conditional isSystem / actor / skipTriggers / skipAutomations / preserveAudit. There is no spread of the execution context, so no other key can arrive. The blessed name is organizationId, and buildSession's own comment records the removal:

The deprecated session.tenantId alias (#3280) was removed here in v11 (#3290) — the driver-layer execCtx.tenantId knob is a separate axis and stays.

HookContext's schema agrees (packages/spec/src/data/hook.zod.ts): it declares organizationId and documents the alias as removed in the v11 major.

So the right-hand arm is dead. The expression is exactly recordOrgId.

Why the dead arm matters — it is a guard, not a nicety

The block's own comment states the consequence it exists to prevent:

Audit rows must never be written with organization_id = NULL, or the SecurityPlugin's RLS predicate hides them forever and the audit log UI reads permanently empty while writes succeed.

and names the cases the session arm was written to cover:

objects with no organization column at all (single-tenant stacks, and ADR-0066 platform-global objects — resolveRecordOrganizationField returns null for both);
a row whose organization column is NULL or empty.

On those, recordOrgId is undefined, the dead arm contributes nothing, and the row is stamped tenant_id: null / organization_id: null — the permanently-invisible ledger row that #8245 and #8247 are about, reached by a different route. Writes keep succeeding, so nothing anywhere reports it.

There is a second occurrence in the same file, in writeCommentMentions:

const tenantId: string | null = sess.tenantId ?? row.organization_id ?? null;

Same dead first arm. Lower stakes — it feeds resolveWriteLocale, so the symptom is a mention notification falling back to the workspace-default locale lookup with no tenant scope rather than an invisible row — but it is the same removed key.

Why it survived

The comment block above the first site is unusually thorough about precedence and even carries a directive written on the assumption that the arm is live:

⛔ Do not flip this back to sess.tenantId ?? recordOrgId.

#8707's ruling changed the ORDER of the two arms and the reasoning is recorded in full — but reordering two expressions does not evaluate either of them, so an arm that had already stopped resolving stayed invisible through a careful review of exactly this code. The #8011 shape: a declaration sitting next to a mechanism, with nothing enforcing the agreement.

Nothing goes red today. Every sys_audit_log field is readonly: true and validateRecord skips readonly fields, so a null tenant is accepted silently; and the write path is wrapped in a swallow-and-report, so even a refusal would not surface as a test failure.

Repro

git grep -n "sess.tenantId" -- packages/plugins/plugin-audit/src/audit-writers.ts
git grep -n "tenantId" -- packages/objectql/src/engine.ts | grep buildSession -A 40

Then read buildSession's object literal and confirm no tenantId key is emitted.

Suggested fix

Read sess.organizationId at both sites, keeping #8707's precedence (the record's own organization still wins). The stronger half is a detector, since this class recurs: a pin that asserts an audit row written on an object with no organization column, under a session that HAS an active organization, lands with a non-null organization_id. That assertion fails today and is what would have caught the alias removal.

Worth deciding at triage whether the same removed alias is read anywhere else — git grep -rn "session.tenantId\|sess.tenantId" across the repo — since #3290 removed it repo-wide and any other reader would be dead the same way.

Not fixed in #8992's PR

Different defect class (write-side tenant stamping vs. the read seam), and the correct form needs a decision about the detector, so it does not meet the in-place bar. #8992's own read-audit writer reads session.organizationId and is unaffected.

Triage: lands in packages/plugins/plugin-audit/src/audit-writers.ts ⇒ domain:identity; queued as Bug — a dead guard arm (sess.tenantId, removed repo-wide in v11 by #3290) means the NULL-tenant protection the block documents can never fire, and every row it fails to stamp is permanently invisible to the RLS predicate. target:v17 set: this is release-blocker class ③ — rows written with organization_id: null are un-repairable data accumulating on deployed instances until the fix lands. Scope for the dispatching seat, per the card's own suggestions and both adopted: fix both sites to sess.organizationId keeping #8707's precedence; add the detector pin (audit row on an org-column-less object under an org-bearing session lands non-null — the assertion that fails today); and run the repo-wide git grep "session.tenantId\|sess.tenantId" sweep for other dead readers of the removed alias, reporting the census in the PR (fix in-scope only for plugin-audit; anything found elsewhere gets filed, not ridden). (Triage seat, session session_019gCKd9EZfQ6MbGTnMHHJvW.)

Generated by Claude Code

Claiming — PM seat os-project-manager, session session_01Y26DJEHSBhhAQ6wwfsHNza, branch claude/issue-9516-audit-tenant-fallback-dead-arm. Read the one prior comment; it is triage, not a claim. Free.

Triage's scope is adopted whole — all three parts, and the third is the one that most often gets dropped:

Fix both sites to sess.organizationId, keeping Audit rows are stamped from the ACTOR's active organization in preference to the record's own — and the record-side fallback cannot see sys_api_key.active_organization_id #8707's precedence (the record's own organization still wins).
The detector pin — an audit row written on an object with no organization column, under a session that has an active organization, lands with a non-null organization_id. That assertion fails today, which is the whole point: it is what would have caught the v11 alias removal, and without it this fix is another declaration with nothing enforcing it.
The repo-wide sess.tenantId / session.tenantId sweep, census reported in the PR. Remove the deprecated hook/action ctx.session.tenantId alias in the next major (converge on organizationId) #3290 removed the alias repo-wide, so any other reader is dead the same way. ⛔ Fix only plugin-audit; anything found elsewhere gets filed, not ridden.

⭐ The part of this card worth keeping in view while implementing: the reason it survived. The block carries an unusually careful comment about precedence and a directive — "⛔ Do not flip this back to sess.tenantId ?? recordOrgId" — written on the assumption that the arm is live. #8707 reordered the two arms and recorded its reasoning in full, but reordering two expressions does not evaluate either of them, so an arm that had already stopped resolving stayed invisible through a careful review of exactly this code. That is why part 2 is not optional: prose next to a mechanism, with nothing checking they agree, is what produced this.

⚠️ And why it goes red nowhere today, which the dev must not mistake for low stakes: every sys_audit_log field is readonly: true so validateRecord skips it, and the write path is wrapped in swallow-and-report. A null tenant is accepted silently, and the row is then permanently invisible to the RLS predicate — target:v17 is set for that reason. Un-repairable rows accumulate on deployed instances until this lands.

Generated by Claude Code

fix(plugin-audit): the NULL-tenant audit guard reads the session key …

{
  "issue": 9516,
  "status": "done",
  "branch": "claude/issue-9516-audit-tenant-fallback-dead-arm",
  "pr": "https://github.com/objectstack-ai/objectstack/pull/9692",
  "premise_still_valid": true,
  "summary": "Premise re-verified on current main (c07d6e8b9) before editing: ObjectQL.buildSession builds the hook session as a fixed key-set object literal with no spread, emitting organizationId (from execCtx.tenantId) and never a tenantId key; packages/spec/src/data/hook.zod.ts agrees. Both dead readers in plugin-audit now read sess.organizationId. Precedence is unchanged at both sites: writeAudit keeps #8707's recordOrgId-first order (honouring the #8287 ruling), and writeCommentMentions keeps its session-first order because #8707's reasoning is about an audit row read through the record's own tenant wall, which a mention notification is not - only the key changed there. The substantive half is the test work: all 19 session fixtures in audit-writers.test.ts hand-built their session as { tenantId: ... }, a dialect the engine cannot produce, so every RLS-fallback case passed because the TEST spoke the removed alias rather than because the code worked. Fixtures re-spelled to organizationId, plus a #9516 detector block pinning the non-null stamp on an org-column-less object and on a NULL org column, that the removed alias is NOT resolved, the mention-path equivalent, and two deliberately-green precedence guards.",
  "tests": "All runs under flock /tmp/os-heavy-verify.lock with NODE_OPTIONS=--max-old-space-size=4096. Reverse verification, prediction recorded to file BEFORE running anything. Step A baseline on pristine main with original fixtures: 'Test Files 17 passed (17) / Tests 268 passed (268)' - every test green with the dead arm live, which is the measurement of why this drifted. Step B (pins + re-spell, fix NOT applied): predicted 10 red, OBSERVED 11 - 'Tests 11 failed | 263 passed (274)'. The prediction miss is reported rather than smoothed: the extra red is '#1532 stamps organization_id on multi-tenant tables when the column exists', whose engine is built from MULTI_TENANT, a map declaring only sys_audit_log and sys_activity - the audited object crm_lead is absent, so resolveRecordOrgField returns null and recordOrgId is undefined regardless of the organization_id: 'org-9' on the result row; that case rode entirely on the session arm. So 7 pre-existing tests, not 6, were held up by a fixture-only key. Step C (fix applied): 'Test Files 17 passed (17) / Tests 274 passed (274)'. Step D ablation (alias restored at both sites behind an OS_ABLATION_9516 marker, fix committed FIRST so restore was a checkout against my own branch): predicted the same 11, observed exactly the same 11 - 'Tests 11 failed | 263 passed (274)'. Markers removed and proved: grep -c OS_ABLATION_9516 = 0, repo-wide OS_ABLATION = 0, git status clean at the final commit. No dogfood ablation was involved (this package's tests run from source, not dist), so no rebuild leg applies. Typecheck: pnpm --filter @objectstack/plugin-audit typecheck exit 0 with the script name echoed (not a zero-match silent pass). Gate union re-derived AFTER the final commit via node scripts/pm/dispatch-gates.mjs off git merge-base (c07d6e8b9, never two-dot), run at HEAD eb60bc99f with a clean tree: check:nul-bytes, check:org-identifier, check:changeset-gate-self-tests, check:objectui-changeset, check:test-source-alias, check:type-source-resolution, check-adr-0087-registration, check-changeset-no-major, check-empty-changeset, check-cross-package-test-inputs, check-affected-docs, check:query-options-erasure, check:engine-double-contract, check:where-matcher, check:type-check-coverage, check:type-check-debt --re-measure (33 ledger entries re-measured, none above its recorded number), check:i18n - all green. check:i18n first exited 1 as a PREREQUISITE failure (a fresh worktree has no built CLI), not drift; built @objectstack/cli, re-ran, green, exit code read directly rather than through a pipe as that gate's own message warns. Beyond the derived union I added check:org-identifier (this diff is precisely about the alias it guards) and check:nul-bytes (standing policy). CI gate conclusions on the draft PR are the PM's read, not waited on here.",
  "open_questions": [
    {
      "question": "writeCommentMentions kept its session-first order (sess.organizationId ?? row.organization_id ?? null) while writeAudit keeps #8707's record-first order. Should the mention site also adopt record-first for consistency?",
      "options": [
        "A - keep session-first at the mention site (what I shipped): change only the dead key, leave the order alone. #8707's ruling reasons about an audit ROW read through the record's own tenant wall; a mention notification is not read through that wall, it feeds resolveWriteLocale and the emitted envelope's organizationId. Applying the flip here would be extending a ruling to a consumer it did not consider, silently and with no test demanding it.",
        "B - flip the mention site to row-first for surface consistency, so both readers in the file agree at a glance. Costs a behaviour change with no card mandate: the acting session's org is the natural scope for 'whose workspace locale applies to this notification', and a comment row's organization is not obviously the better answer."
      ],
      "recommendation": "A, because it is the change the card actually scoped ('you are fixing which key the second arm reads, not the order') and because a precedence ruling written for audit-row visibility does not transfer to notification scope without its own reasoning. If the maintainer wants B, it deserves its own card with its own pin rather than riding this diff."
    },
    {
      "question": "Label convention for the filed finding #9691 - the dispatch prompt and the standing dev contract disagree, so flagging rather than choosing silently.",
      "options": [
        "A - `finding` label (what the dispatch prompt's standing requirement 4 instructs, and what I applied)",
        "B - unlabeled for PM triage (what the standing contract prescribes for concrete defects, reserving `finding` for observation-class items)"
      ],
      "recommendation": "I applied A per the explicit per-card instruction, but I read #9691 as defect-class rather than observation-class: it is a hard-fail gate that printed OK over the exact defect it names, measured at 0 hits against 2 live reads. Suggest the PM re-label to B if that reading is accepted - a wrong label is cheap to fix, a missed filing is not."
    }
  ],
  "out_of_scope_findings": [
    "filed as #9691: check-org-identifier anchors its detector on the literal receiver name `session` (/\\bsession\\s*\\??\\.\\s*tenantId\\b/), so the shipped `sess.tenantId` spelling scored 0 hits on the unfixed file that carried 2 live dead-alias reads - a hard-fail gate scoped to packages/ printing 'OK (2053 author-facing source file(s), no removed session.tenantId alias)' on every PR for the whole period. Measured with the gate's own pattern over its own maskComments projection. Distinct from #9444/#9496, which fixed this gate's comment-masking hole. Unassigned, `finding` label, no pm:queue. Not fixed here: it lives in scripts/, outside this card's plugin-audit-only scope.",
    "no other dead readers to file: the repo-wide sweep found the two fixed here were the entire live population. Every other hit is a pin asserting the key's ABSENCE (engine.test.ts:540/553, http-dispatcher.test.ts:3972), a comment documenting the removal, historical changelog/docs prose, or the driver-layer ExecutionContext/ReadContext tenantId axis that legitimately stays. objectui: zero occurrences."
  ]
}

Generated by Claude Code

Landed and verified on main — PR #9692 merged at 16:19:31Z.

probe	expected	observed
`grep -c 'sess\.tenantId'` in `audit-writers.ts` — the anti-control	0	`0` ✅
`grep -c 'sess.organizationId'`	2	`3`

⚠️ The second row came in above my predicted count, so I read the sites rather than waving it through. My prediction was wrong, not the diff — the three are:

1298:    // flip this back to `sess.organizationId ?? recordOrgId`.
1358:    const tenantId: string | undefined = recordOrgId ?? sess.organizationId;
1647:    const tenantId: string | null = sess.organizationId ?? row.organization_id ?? null;

Two live sites, both with their precedence preserved exactly as ruled — :1358 keeps #8707's record-first order, :1647 keeps session-first because #8707's reasoning was about an audit row read through the record's own tenant wall and a mention notification is not that.

⭐ And the third is the detail worth having: :1298 is the directive comment itself. The line that read "⛔ Do not flip this back to sess.tenantId ?? recordOrgId" — the sentence that reviewed as authoritative while naming a key that no longer existed — now names the live key. That comment is the reason #8707 could reorder these two arms, record its reasoning in full, and still not notice one of them had stopped resolving. Leaving it pointing at the dead alias would have preserved the exact trap this card is about.

The anti-control is the decisive row: zero readers of the removed alias remain in this file, and the repo-wide sweep in the PR established these two were the entire live population — every other hit is a pin asserting the key's absence, a comment documenting the removal, historical prose, or the driver-layer tenantId axis that legitimately stays.

What outlives this card

The one-line key fix is the small half. Two things are worth more:

The fixture re-spell. All 19 session fixtures hand-built { tenantId: … }, a dialect buildSession cannot produce — so every RLS-fallback case passed because the test spoke the removed alias. 268 tests, all green, with the dead arm live. The durable defect was a suite validating against a session shape that does not exist at runtime, and that is what let the dead arm survive a careful review of exactly this code.

#9691. check-org-identifier is a hard-fail gate that anchors on the literal receiver name session, so it scored 0 hits on the unfixed file that carried 2 live dead-alias reads — and printed OK … no removed session.tenantId alias on every PR throughout, including this one. ⚠️ Now that the offenders are gone it will keep printing OK, truthfully but no less blindly, and that green must not be read as corroboration. Still open, domain:devx, for triage to level.

Generated by Claude Code

fix(plugin-audit): the NULL-tenant audit guard reads the session key …

Navigation Menu

Repository navigation

audit: the tenant fallback in `writeAudit` reads `session.tenantId`, a key removed in v11 — the guard against NULL-tenant audit rows can never fire #9516

The defect

Why the dead arm matters — it is a guard, not a nicety

Why it survived

Repro

Suggested fix

Not fixed in #8992's PR

What outlives this card

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

audit: the tenant fallback in writeAudit reads session.tenantId, a key removed in v11 — the guard against NULL-tenant audit rows can never fire #9516

Description

The defect

Why the dead arm matters — it is a guard, not a nicety

Why it survived

Repro

Suggested fix

Not fixed in #8992's PR

Activity

os-support-ai commented on Aug 18, 2026

os-project-manager commented on Aug 18, 2026

os-project-manager commented on Aug 18, 2026

os-project-manager commented on Aug 18, 2026

What outlives this card

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions

audit: the tenant fallback in `writeAudit` reads `session.tenantId`, a key removed in v11 — the guard against NULL-tenant audit rows can never fire #9516