refactor(plugin-sharing,plugin-audit): annotate enforcement on the full ExecutionContext and delete the double-casts (#7136)

.changeset/sharing-audit-exec-context-annotations.md

@@ -0,0 +1,48 @@
+---
+"@objectstack/plugin-sharing": patch
+"@objectstack/plugin-audit": patch
+---
+
+refactor(plugin-sharing,plugin-audit): enforcement implementations annotate the full `ExecutionContext` (#7136)
+
+The consumer half of #6523. That change converged 36 contract signatures onto
+the complete `resolveAuthzContext` envelope, applying the #6206 ruling —
+enforcement adjudicates on the whole envelope, never a per-site subset. The
+implementations behind those contracts still annotated their own parameters
+with `SharingExecutionContext`, the six-field shape the contracts used to name,
+so nothing they could *read* had widened.
+
+`SharingService`, `SharingRuleService`, the sharing exec-context seam and
+plugin-audit's comment-access gates now declare `ExecutionContext` on all 27 of
+those parameters — plus the two return types that produce the contexts feeding
+them — and the casts the narrow annotation forced are gone:
+
+- `exec-context-seam.testkit.ts` resolved a REAL context and then had to force
+  it into the narrow type — `{ ...authz, isSystem: false } as unknown as
+  SharingExecutionContext`. It now returns what it resolved, so a drift in
+  `resolveAuthzContext`'s output reaches the tests that trust this seam instead
+  of being absorbed by a double cast.
+- `SharingRuleService`'s system context is typed as the envelope and passed as
+  itself, retiring `SYSTEM_CTX as any` at all 10 of its call sites — an erasure
+  on an enforcement input switches checking off for the whole argument, not
+  just for the readonly-array mismatch that provoked it.
+- The `(context as any).userId` / `.tenantId` reads in `SharingService` now read
+  declared fields.
+
+**No runtime behaviour changes.** The values were always complete — this
+family's damage was type-side — so every gate answers exactly what it answered
+before. Method parameters only WIDEN what they accept, so no caller is affected.
+
+Two casts are deliberately kept, and are now documented where they sit:
+`__readScope` / `__writeScope` are private keys plugin-security's middleware
+stamps onto the context it forwards and are not fields of the envelope, and
+`organizationId` is not on the envelope at all — that spelling has its own
+history (#5858 / `check:org-identifier`) and was held out of this change.
+
+Because a re-narrowed annotation would compile, ship and pass every test in
+these packages, the convergence is pinned by a new compile-time module,
+`exec-context-annotation.pin.ts`: it hands each enforcement parameter a fresh
+literal naming envelope-only fields (`posture`, `accessible_org_ids`,
+`org_user_ids`), which TypeScript's excess-property check rejects the moment a
+parameter narrows back, plus negative cases so a parameter erased to `any`
+cannot pass either.

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

@@ -55,7 +55,8 @@
  * load-bearing.
  */
 
-import type { ISharingService, SharingExecutionContext } from '@objectstack/spec/contracts';
+import type { ISharingService } from '@objectstack/spec/contracts';
+import type { ExecutionContext } from '@objectstack/spec/kernel';
 
 /** Minimal engine surface these installers need — duck-typed (like
  * service-storage's attachment seams) so tests can fake it and so plugin-audit
@@ -180,8 +181,25 @@ function asIdList(id: unknown): Array<string | number> | null {
 }
 
 /** The caller's ExecutionContext rides on the operation options — the session
- * snapshot lacks `permissions`, which sharing bypasses need. */
-function callerContext(ctx: any): SharingExecutionContext {
+ * snapshot lacks `permissions`, which sharing bypasses need.
+ *
+ * [#7136] Typed as the full envelope, which is what `ISharingService` declares
+ * for every parameter this value is handed to (#6523 / the #6206 ruling).
+ *
+ * ⚠️ The BODY still projects a five-field subset, which the same ruling tells
+ * callers not to do — and that half is deliberately NOT changed here, because
+ * it is not the inert half. Widening the annotation is type-side; forwarding
+ * `exec` whole is a RUNTIME change. plugin-security's middleware MUTATES the
+ * operation context in place (`sc.__readScope = …`, `security-plugin.ts`), so
+ * the context this hook receives carries the depth resolved for `sys_comment` —
+ * the object of the operation — while these gates ask the sharing service about
+ * the PARENT record's object. Forwarding it would hand one object's access
+ * depth to another object's owner-match, the exact stale-scope leak
+ * `resolveWriteScopeForSharing` was extracted to prevent ("a stale value can
+ * never leak in through a spread"). This projection is currently what stops
+ * that, so replacing it needs its own card and its own evidence — filed rather
+ * than folded in. */
+function callerContext(ctx: any): ExecutionContext {
   const exec = ctx?.input?.options?.context;
   if (exec && typeof exec === 'object') {
     return {

packages/plugins/plugin-sharing/src/exec-context-annotation.pin.ts

@@ -0,0 +1,90 @@
+// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
+
+/**
+ * #7136 — compile-time pin for the CONTEXT type this plugin's enforcement
+ * methods accept.
+ *
+ * #6523 converged 36 contract signatures onto the full `ExecutionContext` (the
+ * #6206 ruling: enforcement adjudicates on the whole `resolveAuthzContext`
+ * envelope, never a per-site subset). #7136 is the consumer half — the
+ * implementations here now annotate their own parameters with that same
+ * envelope instead of the six-field shape they used to name.
+ *
+ * WHY THIS FILE EXISTS AT ALL. That convergence has no runtime behaviour and no
+ * compiler pressure in either direction: the values were always complete, and
+ * the narrow annotation is STRUCTURALLY ASSIGNABLE to the wide one, so
+ * re-narrowing any of these parameters compiles, ships, and passes every test
+ * in this package. Nothing would notice. This module is the one thing that
+ * does — every declaration below is red exactly when a parameter narrows back.
+ *
+ * HOW IT BITES: TypeScript's excess-property check on a FRESH object literal.
+ * `posture` (ADR-0095 D2), `accessible_org_ids` (ADR-0105 D2) and
+ * `org_user_ids` are fields of the envelope that the retired six-field shape
+ * did not carry, so a literal naming them is rejected the moment the parameter
+ * is annotated with anything that lacks them. Note this is the ONLY direction
+ * that works: a `@ts-expect-error` asserting the reverse would be unsatisfied
+ * and fail the build, because a narrow context IS assignable to a wide
+ * parameter — the boundary `SharingExecutionContext`'s own doc block records.
+ *
+ * WHY A `.pin.ts` AND NOT A `*.test.ts`: `packages/plugins/plugin-sharing/
+ * tsconfig.json` excludes `**\/*.test.ts` (a measured TEST_DEBT of 3 in
+ * `scripts/check-type-check-coverage.mjs`), so no tsc program the `typecheck`
+ * script runs would ever read a pin written in a test file here — it would be
+ * a phantom check that stays green however this file is broken (AGENTS.md,
+ * #5286's `PINS_CHECKED`; #6212 measured the same hole on driver-mongodb).
+ * This file IS in that program. It is imported by nothing, so tsup (entry
+ * `src/index.ts`) never bundles it into `dist`, exactly like the sibling
+ * `.testkit.ts`.
+ */
+
+import type { SharingService } from './sharing-service.js';
+import type { SharingRuleService } from './sharing-rule-service.js';
+import type { bootRequestContext } from './exec-context-seam.testkit.js';
+
+type ReadFilterContext = Parameters<SharingService['buildReadFilter']>[1];
+type WriteGateContext = Parameters<SharingService['checkEdit']>[2];
+type GrantContext = Parameters<SharingService['grant']>[1];
+type DefineRuleContext = Parameters<SharingRuleService['defineRule']>[1];
+
+/** What the seam hands a test — the resolved envelope, not a projection of it. */
+type SeamContext = Awaited<ReturnType<typeof bootRequestContext>>;
+
+/**
+ * Never called — every line below is a type-level assertion evaluated by
+ * `tsc --noEmit`. The parameters are taken as arguments rather than read off a
+ * live service so the pin needs no instance and no import cycle.
+ */
+export function __pinEnforcementTakesTheFullEnvelope(
+  buildReadFilter: (object: string, context: ReadFilterContext) => unknown,
+  checkEdit: (object: string, recordId: string, context: WriteGateContext) => unknown,
+  grant: (input: never, context: GrantContext) => unknown,
+  defineRule: (input: never, context: DefineRuleContext) => unknown,
+  seamContext: SeamContext,
+): void {
+  // ── POSITIVE: fields that exist ONLY on the full envelope, no cast. ───────
+  buildReadFilter('account', { userId: 'u1', posture: 'MEMBER', accessible_org_ids: ['org_a'] });
+  checkEdit('account', 'a1', { userId: 'u1', posture: 'TENANT_ADMIN', org_user_ids: ['u1', 'u2'] });
+  grant(undefined as never, { userId: 'u1', posture: 'PLATFORM_ADMIN' });
+  defineRule(undefined as never, { userId: 'u1', accessible_org_ids: ['org_a'] });
+
+  // The seam resolves a REAL context and returns it AS RESOLVED. Reading a
+  // field the six-field shape never had is what pins that: the double cast
+  // this card deleted (`as unknown as`) would have hidden any drift here.
+  const posture: SeamContext['posture'] = seamContext.posture;
+  void posture;
+
+  // ── NEGATIVE: widening must not have degenerated into `any`. ─────────────
+  // A parameter erased to `any` would swallow every positive above just as
+  // happily, so the pin is only worth its weight if wrong input still fails.
+  // @ts-expect-error 'SUPERUSER' is not an ADR-0095 posture rung
+  buildReadFilter('account', { userId: 'u1', posture: 'SUPERUSER' });
+  // @ts-expect-error `userId` is a string on the envelope, not a number
+  checkEdit('account', 'a1', { userId: 42 });
+  // @ts-expect-error `accessible_org_ids` is a string[], not a bare string
+  defineRule(undefined as never, { accessible_org_ids: 'org_a' });
+  // @ts-expect-error `organizationId` is NOT a field of the envelope — that
+  // spelling has its own history (#5858 / `check:org-identifier`) and was held
+  // out of #7136 on purpose. The three reads of it left in `sharing-rule-
+  // service.ts` are still cast, and this line is why they have to be.
+  grant(undefined as never, { organizationId: 'org_a' });
+}

packages/plugins/plugin-sharing/src/exec-context-seam.testkit.ts

@@ -28,7 +28,7 @@
  */
 
 import { resolveAuthzContext } from '@objectstack/core';
-import type { SharingExecutionContext } from '@objectstack/spec/contracts';
+import type { ExecutionContext } from '@objectstack/spec/kernel';
 
 /** A `sys_member` row as the identity tables really store it. */
 export interface SeamMembership {
@@ -75,7 +75,7 @@ function makeSeamQl(tables: Record<string, any[]>) {
  * `isSystem: false`) — this helper never names a tenancy field, so neither does
  * the test that calls it.
  */
-export async function bootRequestContext(principal: SeamPrincipal): Promise<SharingExecutionContext> {
+export async function bootRequestContext(principal: SeamPrincipal): Promise<ExecutionContext> {
   const activeOrg = principal.activeOrganizationId ?? null;
   const memberships: SeamMembership[] =
     principal.memberships ?? (activeOrg ? [{ organization_id: activeOrg, role: 'member' }] : []);
@@ -103,5 +103,13 @@ export async function bootRequestContext(principal: SeamPrincipal): Promise<Shar
     }),
   });
 
-  return { ...authz, isSystem: false } as unknown as SharingExecutionContext;
+  // [#7136] Returned AS RESOLVED — no cast. This helper's whole promise is
+  // that a test receives what a real request receives, and until the sharing
+  // service's own parameters named the full envelope, keeping that promise
+  // required forcing the real thing through `as unknown as` into a six-field
+  // shape. A double cast on the value a test is meant to trust is the seam
+  // lying about itself: it would have absorbed a genuine drift in
+  // `resolveAuthzContext`'s output silently, which is the exact failure mode
+  // (#5852) this file exists to prevent.
+  return { ...authz, isSystem: false };
 }