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

.changeset/org-identifier-session-provenance.md

@@ -0,0 +1,24 @@
+---
+'@objectstack/service-storage': patch
+'@objectstack/plugin-audit': patch
+---
+
+Attachment access hooks: read the caller's org under the blessed `organizationId` name
+
+`callerContext()` in the `sys_attachment` access kit built its fallback
+execution envelope from `session.tenantId` — an alias removed from the
+hook/action session surface in v11 (#3290). `HookContextSchema` strips a
+`tenantId` key and the engine's `buildSession` only ever emits
+`organizationId`, so on every call that reached the session fallback (no
+execution context riding along) the envelope handed to
+`ISharingService.canEdit` carried **no organization at all**. Parent-record
+access for attachments was therefore evaluated without the caller's active
+org on that path. It now reads `session.organizationId`, matching the
+`sys_comment` kit, which already did.
+
+The `sys_comment` kit's own `callerContext()` had the same read as a dead
+first arm (`s.tenantId ?? s.organizationId`); the arm is removed. That half
+is behaviour-neutral — the fallback already carried the value.
+
+Both kits gain coverage of the session-fallback path in both directions: the
+blessed name is read, and a stray removed-alias key does not become the org.

packages/plugins/plugin-audit/src/audit-writers.test.ts

@@ -1657,8 +1657,13 @@ describe('audit writers — the writer reads the session key the engine emits (#
  // A session in the REMOVED dialect. The engine cannot produce one, so the
  // only way this shape reaches the writer is a caller that is itself broken
  // — and honouring it here would hide that. This pin goes red the day
- // `sess.tenantId` is reintroduced as a fallback arm; `pnpm check:org-identifier`
- // cannot see that reintroduction when the receiver is spelled `sess`.
+ // `sess.tenantId` is reintroduced as a fallback arm. It used to be the ONLY
+ // thing that would: `pnpm check:org-identifier` was anchored on the literal
+ // receiver name `session` and scored zero on `sess`. Since #9691 that gate
+ // resolves the receiver's PROVENANCE instead — a local filled from a
+ // `.session` expression is a session whatever it is called — so the
+ // reintroduction is caught in both places now. Keep this pin anyway: the
+ // gate cannot see a wrong VALUE, only a removed-alias read.
  await fire('afterInsert', {
  object: 'crm_lead',
  input: { id: 'lead-1' },

packages/plugins/plugin-audit/src/comment-access-hooks.test.ts

@@ -495,4 +495,40 @@ describe('#7141 — caller envelope forwarded to the sharing gate', () => {
  ).rejects.toMatchObject({ code: 'RECORD_NOT_ACCESSIBLE', status: 403 });
  expect((canEdit.mock.calls[0]![2] as any).__writeScope).toBeUndefined();
  });
+
+ // ── The session fallback, and the org name it reads (#9691) ───────────
+ //
+ // The kit had no coverage of the no-execution-context path at all, so the
+ // dead `s.tenantId ?? s.organizationId` first arm was invisible in both
+ // directions: nothing proved the blessed name was read, and nothing would
+ // have noticed if the fallback had been dropped. Both directions are pinned
+ // here, on the session shape `ObjectQLEngine.buildSession` actually emits.
+ it('falls back to the session snapshot and reads the caller org under the BLESSED name (#9691)', async () => {
+ const canEdit = vi.fn(async (_o: string, _r: string, _c: any) => true);
+ const { beforeDelete } = install({ comments: [row], sharing: { canEdit } });
+ await beforeDelete({
+ object: 'sys_comment',
+ event: 'beforeDelete',
+ input: { id: 'c1' },
+ session: { userId: 'u1', organizationId: 'org_1', positions: ['p1'] },
+ api: apiFor(['crm_opportunity/opp1']),
+ });
+ // `tenantId` on the way OUT is `ExecutionContext`'s driver-layer name for
+ // the same value — the separate axis #3290 deliberately left alone.
+ expect(canEdit.mock.calls[0]![2]).toEqual({ userId: 'u1', tenantId: 'org_1', positions: ['p1'] });
+ });
+
+ it('does not resurrect the removed `session.tenantId` alias if one ever reaches a hook (#9691)', async () => {
+ const canEdit = vi.fn(async (_o: string, _r: string, _c: any) => true);
+ const { beforeDelete } = install({ comments: [row], sharing: { canEdit } });
+ await beforeDelete({
+ object: 'sys_comment',
+ event: 'beforeDelete',
+ input: { id: 'c1' },
+ // A key `HookContextSchema` strips (#3290). It is not the caller's org.
+ session: { userId: 'u1', tenantId: 'stale_org', positions: ['p1'] } as any,
+ api: apiFor(['crm_opportunity/opp1']),
+ });
+ expect((canEdit.mock.calls[0]![2] as any).tenantId).toBeUndefined();
+ });
 });

packages/plugins/plugin-audit/src/comment-access-hooks.ts

@@ -252,7 +252,13 @@ function callerContext(ctx: any): ExecutionContext {
  return withoutOperationPrivateKeys(exec as Record<string, unknown>);
  }
  const s = ctx?.session ?? {};
- return { userId: s.userId, tenantId: s.tenantId ?? s.organizationId, positions: s.positions };
+ // [#9691] The `s.tenantId` arm was DEAD, not a fallback: `HookContextSchema`
+ // strips a `tenantId` key from the session (#3290) and the engine's
+ // `buildSession` only ever emits `organizationId`, so the first arm answered
+ // `undefined` on every call and the second one carried the value. Dropping it
+ // is byte-for-byte the same envelope; it is removed because a dead read of a
+ // removed alias is what an author copies out of a reference body.
+ return { userId: s.userId, tenantId: s.organizationId, positions: s.positions };
 }
 
 /** Can the CALLER read `(object, recordId)`? A caller-scoped `findOne` through

packages/services/service-storage/src/attachment-access-hooks.test.ts

@@ -546,17 +546,45 @@ describe('#7145 — caller envelope forwarded to the sharing gate', () => {
  expect((canEdit.mock.calls[0]![2] as any).__writeScope).toBeUndefined();
  });
 
- // ── The session fallback is unchanged ─────────────────────────────────
- it('still falls back to the session snapshot when no execution context rides along', async () => {
+ // ── The session fallback, and the org name it reads (#9691) ───────────
+ //
+ // ⚠️ This case used to hand the hook a session spelling `tenantId: 'org_1'`
+ // and assert the same key came back out. That is a session the engine cannot
+ // produce: `HookContextSchema` STRIPS a `tenantId` key (#3290, pinned in
+ // `packages/spec/src/data/hook.test.ts`) and `buildSession` only ever emits
+ // `organizationId`. So the fixture pinned the removed-alias arm itself — it
+ // 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.
+ // Replaced rather than respelled: the fixture below is the envelope a real
+ // transport builds.
+ it('falls back to the session snapshot and reads the caller org under the BLESSED name (#9691)', async () => {
  const canEdit = vi.fn(async (_o: string, _r: string, _c: any) => true);
  const { beforeDelete } = install({ attachments: [attRow], sharing: { canEdit } });
  await beforeDelete({
  object: 'sys_attachment',
  event: 'beforeDelete',
  input: { id: 'a1' },
- session: { userId: 'u1', tenantId: 'org_1', positions: ['p1'] },
+ // Exactly what `ObjectQLEngine.buildSession` emits.
+ session: { userId: 'u1', organizationId: 'org_1', positions: ['p1'] },
  api: apiFor([]),
  });
+ // `tenantId` on the way OUT is `ExecutionContext`'s driver-layer name for
+ // the same value — the separate axis #3290 deliberately left alone.
  expect(canEdit.mock.calls[0]![2]).toEqual({ userId: 'u1', tenantId: 'org_1', positions: ['p1'] });
  });
+
+ it('does not resurrect the removed `session.tenantId` alias if one ever reaches a hook (#9691)', async () => {
+ const canEdit = vi.fn(async (_o: string, _r: string, _c: any) => true);
+ const { beforeDelete } = install({ attachments: [attRow], sharing: { canEdit } });
+ await beforeDelete({
+ object: 'sys_attachment',
+ event: 'beforeDelete',
+ input: { id: 'a1' },
+ // A key the schema strips. Reaching for it is how this seam handed the
+ // sharing service an envelope with no org at all for several majors.
+ session: { userId: 'u1', tenantId: 'stale_org', positions: ['p1'] } as any,
+ api: apiFor([]),
+ });
+ expect((canEdit.mock.calls[0]![2] as any).tenantId).toBeUndefined();
+ });
 });

packages/services/service-storage/src/attachment-access-hooks.ts

@@ -131,7 +131,17 @@ function callerContext(ctx: any): ExecutionContext {
  return withoutOperationPrivateKeys(exec as Record<string, unknown>);
  }
  const s = ctx?.session ?? {};
- return { userId: s.userId, tenantId: s.tenantId, positions: s.positions };
+ // [#9691] `s.organizationId`, NOT `s.tenantId`. The hook session's org key is
+ // `organizationId` (engine `buildSession`; `HookContextSchema` STRIPS a
+ // `tenantId` key outright, pinned in `packages/spec/src/data/hook.test.ts`),
+ // so the removed alias read here answered `undefined` on every call and this
+ // fallback handed `ISharingService.canEdit` an envelope with no org at all.
+ // The target field keeps its `tenantId` spelling: that is `ExecutionContext`'s
+ // driver-layer name for the same value, a separate axis #3290 deliberately
+ // left alone. The comment kit's `callerContext` already read the blessed name
+ // (`s.tenantId ?? s.organizationId`), so this is the #7145 parity that kit's
+ // card asked for, completed.
+ return { userId: s.userId, tenantId: s.organizationId, positions: s.positions };
 }
 
 export function installAttachmentAccessHooks(