From 4ffad8eb1685a7de93d9310549387bdc315bb7ed Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 20:36:08 +0000 Subject: [PATCH 1/2] refactor(plugin-sharing,plugin-audit): annotate enforcement on the full ExecutionContext (#7136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Consumer half of #6523. The contracts converged onto the complete resolveAuthzContext envelope (#6206 ruling: no per-site subset contracts); the implementations behind them still named the six-field SharingExecutionContext, so nothing they could read had widened. - 27 enforcement parameters across SharingService, SharingRuleService and plugin-audit's comment-access gates now declare ExecutionContext, plus the two return types that produce the contexts feeding them. - exec-context-seam.testkit.ts returns the resolved envelope with no cast, deleting the `as unknown as` double cast on the value tests are meant to trust. - SharingRuleService's SYSTEM_CTX is typed as the envelope and passed as itself, retiring `SYSTEM_CTX as any` at all 11 call sites. - `(context as any).userId` / `.tenantId` in SharingService now read declared fields. Kept and documented in place: __readScope / __writeScope (middleware-private keys, not envelope fields) and organizationId (#5858 / check:org-identifier, held out of #7070 on purpose). No runtime behaviour changes — the values were always complete, the damage was type-side. Parameters only widen what they accept. Because a re-narrowed annotation would compile, ship and pass every test, exec-context-annotation.pin.ts pins the convergence at compile time via excess-property checks on envelope-only fields, with negatives so a parameter erased to `any` cannot pass either. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- .../sharing-audit-exec-context-annotations.md | 48 ++++++++++ .../plugin-audit/src/comment-access-hooks.ts | 24 ++++- .../src/exec-context-annotation.pin.ts | 90 +++++++++++++++++++ .../src/exec-context-seam.testkit.ts | 14 ++- .../src/sharing-rule-service.ts | 64 ++++++++----- .../plugin-sharing/src/sharing-service.ts | 64 ++++++++----- 6 files changed, 252 insertions(+), 52 deletions(-) create mode 100644 .changeset/sharing-audit-exec-context-annotations.md create mode 100644 packages/plugins/plugin-sharing/src/exec-context-annotation.pin.ts diff --git a/.changeset/sharing-audit-exec-context-annotations.md b/.changeset/sharing-audit-exec-context-annotations.md new file mode 100644 index 0000000000..c3ae8914b2 --- /dev/null +++ b/.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 11 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. diff --git a/packages/plugins/plugin-audit/src/comment-access-hooks.ts b/packages/plugins/plugin-audit/src/comment-access-hooks.ts index 45844c5503..5543bb3c2c 100644 --- a/packages/plugins/plugin-audit/src/comment-access-hooks.ts +++ b/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 | 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 { diff --git a/packages/plugins/plugin-sharing/src/exec-context-annotation.pin.ts b/packages/plugins/plugin-sharing/src/exec-context-annotation.pin.ts new file mode 100644 index 0000000000..905c099f84 --- /dev/null +++ b/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[1]; +type WriteGateContext = Parameters[2]; +type GrantContext = Parameters[1]; +type DefineRuleContext = Parameters[1]; + +/** What the seam hands a test — the resolved envelope, not a projection of it. */ +type SeamContext = Awaited>; + +/** + * 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' }); +} diff --git a/packages/plugins/plugin-sharing/src/exec-context-seam.testkit.ts b/packages/plugins/plugin-sharing/src/exec-context-seam.testkit.ts index 8e24c36f98..03d920680d 100644 --- a/packages/plugins/plugin-sharing/src/exec-context-seam.testkit.ts +++ b/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) { * `isSystem: false`) — this helper never names a tenancy field, so neither does * the test that calls it. */ -export async function bootRequestContext(principal: SeamPrincipal): Promise { +export async function bootRequestContext(principal: SeamPrincipal): Promise { 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 { + async defineRule(input: DefineSharingRuleInput, context: ExecutionContext): Promise { this.assertCanManageRules(context); if (!input.name) throw new Error('VALIDATION_FAILED: name is required'); if (!input.label) throw new Error('VALIDATION_FAILED: label is required'); @@ -124,7 +136,13 @@ export class SharingRuleService implements ISharingRuleService { throw new Error(`VALIDATION_FAILED: ${MATCH_ALL_CRITERIA_MESSAGE}`); } - const orgId = (context as any)?.organizationId ?? (context as any)?.tenantId ?? null; + // [#7136] Only the `tenantId` half of this read lost its cast: `tenantId` + // is a declared field of the envelope, `organizationId` is not a field of + // it at ALL. That spelling has its own history (#5858 / + // `check:org-identifier`) and was explicitly held out of this change + // (#7070) — so it stays cast, and the asymmetry above is now the visible + // marker of which of the two names the contract actually knows. + const orgId = (context as any)?.organizationId ?? context?.tenantId ?? null; const now = new Date().toISOString(); // Authoring path — `full` normalises to `edit`, anything unrecognised is a // loud VALIDATION_FAILED alongside the required-field checks above (#3865). @@ -205,13 +223,14 @@ export class SharingRuleService implements ISharingRuleService { async listRules( filter: { object?: string; activeOnly?: boolean }, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { this.assertCanManageRules(context); // [ADR-0111 D6] const where: any = {}; if (filter.object) where.object_name = filter.object; if (filter.activeOnly) where.active = true; - const orgId = (context as any)?.organizationId ?? (context as any)?.tenantId; + // `organizationId` is not on the envelope — see defineRule(). + const orgId = (context as any)?.organizationId ?? context?.tenantId; if (orgId) where.organization_id = orgId; const rows = await this.engine.find('sys_sharing_rule', { where, @@ -222,10 +241,11 @@ export class SharingRuleService implements ISharingRuleService { return Array.isArray(rows) ? rows.map(rowFromRule) : []; } - async getRule(idOrName: string, context: SharingExecutionContext): Promise { + async getRule(idOrName: string, context: ExecutionContext): Promise { this.assertCanManageRules(context); // [ADR-0111 D6] if (!idOrName) return null; - const orgId = (context as any)?.organizationId ?? (context as any)?.tenantId; + // `organizationId` is not on the envelope — see defineRule(). + const orgId = (context as any)?.organizationId ?? context?.tenantId; const byId = await this.engine.find('sys_sharing_rule', { where: { id: idOrName }, limit: 1, @@ -241,7 +261,7 @@ export class SharingRuleService implements ISharingRuleService { return null; } - async deleteRule(idOrName: string, context: SharingExecutionContext): Promise { + async deleteRule(idOrName: string, context: ExecutionContext): Promise { this.assertCanManageRules(context); // [ADR-0111 D6] const row = await this.getRule(idOrName, context); if (!row) return; @@ -271,7 +291,7 @@ export class SharingRuleService implements ISharingRuleService { } as any); } - async evaluateRule(idOrName: string, context: SharingExecutionContext): Promise { + async evaluateRule(idOrName: string, context: ExecutionContext): Promise { this.assertCanManageRules(context); // [ADR-0111 D6] const rule = await this.getRule(idOrName, context); if (!rule) throw new Error('RULE_NOT_FOUND'); @@ -335,7 +355,7 @@ export class SharingRuleService implements ISharingRuleService { // A `source: 'rule'` row with no `source_id` names no rule that could // ever re-grant it — equally unreachable, equally void. if (sourceId != null && live.has(String(sourceId))) continue; - await this.sharing.revoke(String((g as any).id), SYSTEM_CTX as any); + await this.sharing.revoke(String((g as any).id), SYSTEM_CTX); revoked += 1; } if (revoked > 0) { @@ -364,7 +384,7 @@ export class SharingRuleService implements ISharingRuleService { async evaluateAllForRecord( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const rules = await this.listRules({ object }, context); if (rules.length === 0) return []; @@ -397,11 +417,11 @@ export class SharingRuleService implements ISharingRuleService { */ async evaluateAllRulesForObject(object: string): Promise { if (!object) return 0; - const rules = await this.listRules({ object }, SYSTEM_CTX as any); + const rules = await this.listRules({ object }, SYSTEM_CTX); let reconciled = 0; for (const rule of rules) { try { - await this.evaluateRule(rule.id, SYSTEM_CTX as any); + await this.evaluateRule(rule.id, SYSTEM_CTX); reconciled += 1; } catch (err: any) { this.logger?.warn?.('[sharing-rule] object reconcile failed for rule', { @@ -610,7 +630,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX as any, + SYSTEM_CTX, ); updated += 1; } @@ -627,14 +647,14 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX as any, + SYSTEM_CTX, ); created += 1; } } // Revoke stale. for (const [, stale] of existingMap.entries()) { - await this.sharing.revoke(stale.id, SYSTEM_CTX as any); + await this.sharing.revoke(stale.id, SYSTEM_CTX); revoked += 1; } @@ -683,7 +703,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX as any, + SYSTEM_CTX, ); updated += 1; } @@ -700,7 +720,7 @@ export class SharingRuleService implements ISharingRuleService { sourceId: rule.id, reason: `rule:${rule.name}`, } as any, - SYSTEM_CTX as any, + SYSTEM_CTX, ); created += 1; } @@ -709,7 +729,7 @@ export class SharingRuleService implements ISharingRuleService { // Anything still in existingMap is stale (either match=false or // user no longer in expanded set). for (const [, stale] of existingMap.entries()) { - await this.sharing.revoke(stale.id, SYSTEM_CTX as any); + await this.sharing.revoke(stale.id, SYSTEM_CTX); revoked += 1; } @@ -732,7 +752,7 @@ export class SharingRuleService implements ISharingRuleService { }); let revoked = 0; for (const row of (existing ?? [])) { - await this.sharing.revoke((row as any).id, SYSTEM_CTX as any); + await this.sharing.revoke((row as any).id, SYSTEM_CTX); revoked += 1; } return revoked; diff --git a/packages/plugins/plugin-sharing/src/sharing-service.ts b/packages/plugins/plugin-sharing/src/sharing-service.ts index fa71077c52..1b4b7874c9 100644 --- a/packages/plugins/plugin-sharing/src/sharing-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-service.ts @@ -7,7 +7,6 @@ import type { IHierarchyScopeResolver, RecordShare, GrantShareInput, - SharingExecutionContext, ShareAccessLevel, SharingWriteVerdict, } from '@objectstack/spec/contracts'; @@ -16,6 +15,12 @@ import { postureEnforcesWall, type TenancyPosture, } from '@objectstack/spec/security'; +// [#7136] Every enforcement method below takes the FULL `resolveAuthzContext` +// envelope — the same type `ISharingService` declares for these parameters +// since #6523 (the #6206 ruling: no per-site subset contracts). Annotating the +// implementations with a narrower shape is what forced this file to cast its +// way out of its own contract to read fields the caller had already supplied. +import type { ExecutionContext } from '@objectstack/spec/kernel'; import { WRITE_ACCESS_LEVELS, normalizeAccessLevel } from './access-level.js'; import { deleteRowsForDeletedRecords, @@ -128,8 +133,8 @@ export function effectiveSharingModel(schema: any): 'private' | 'read' | 'public * organization": the contract types the field as `string | null`, and `null` is * the value a resolver's fail-closed obligation is written against. */ -function activeOrganizationId(context: SharingExecutionContext): string | null { - const org = (context as any)?.tenantId; +function activeOrganizationId(context: ExecutionContext): string | null { + const org = context?.tenantId; return typeof org === 'string' && org.trim() !== '' ? org : null; } @@ -276,7 +281,7 @@ export class SharingService implements ISharingService { */ async buildReadFilter( object: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { if (this.shouldBypass(object, context)) return null; @@ -294,6 +299,15 @@ export class SharingService implements ISharingService { // [ADR-0057 D1] Access DEPTH widens the owner-match for this grant: // own → [me], unit → my BU members, unit_and_below → my BU subtree, org → // no owner filter. Sharing grants are still OR-ed in on top (additive). + // + // [#7136] This cast SURVIVES the annotation widening, on purpose. Unlike + // `userId` / `tenantId`, `__readScope` and `__writeScope` are not fields of + // `ExecutionContext` at all: they are private keys plugin-security's + // middleware stamps onto the context it forwards + // (`security-plugin.ts` — `sc.__readScope = …`). So the cast is not + // residue of the narrow contract and is NOT deletable here. ⛔ Nor is the + // fix to declare them on the envelope — that would publish a middleware + // seam as authorable, client-supplied vocabulary. const readScope = (context as any).__readScope as ('own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined); if (readScope === 'org') return null; const ownerIds = await this.resolveOwnerScopeIds(context, readScope); @@ -343,7 +357,7 @@ export class SharingService implements ISharingService { */ async buildWriteFilter( object: string, - context: SharingExecutionContext, + context: ExecutionContext, verb: 'update' | 'delete' = 'update', ): Promise { if (this.shouldBypass(object, context)) return null; @@ -358,6 +372,7 @@ export class SharingService implements ISharingService { return { id: '__deny_all__' }; } + // Middleware-stamped, not a field of the envelope — see buildReadFilter(). const writeScope = (context as any).__writeScope as ('own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined); if (writeScope === 'org') return null; const ownerIds = await this.resolveOwnerScopeIds(context, writeScope); @@ -399,7 +414,7 @@ export class SharingService implements ISharingService { private async matchesOwnerScope( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const own = await this.engine.find(object, { where: { id: recordId }, @@ -409,6 +424,7 @@ export class SharingService implements ISharingService { }); const owner = Array.isArray(own) && own[0] ? (own[0] as any)[OWNER_FIELD] : undefined; if (owner == null) return false; + // Middleware-stamped, not a field of the envelope — see buildReadFilter(). const writeScope = (context as any).__writeScope as ('own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined); if (writeScope === 'org') return true; const owners = await this.resolveOwnerScopeIds(context, writeScope); @@ -434,7 +450,7 @@ export class SharingService implements ISharingService { */ private async hasModifyAllBypass( object: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const probe = this.securityService?.(); if (!probe || typeof probe.hasWriteBypass !== 'function') return false; @@ -462,7 +478,7 @@ export class SharingService implements ISharingService { */ private bypassVerdict( object: string, - context: SharingExecutionContext, + context: ExecutionContext, ): SharingWriteVerdict | null { if (context?.isSystem) return 'allow'; if (this.bypassObjects.has(object)) return 'abstain'; @@ -483,7 +499,7 @@ export class SharingService implements ISharingService { verb: 'update' | 'delete', object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, err: unknown, ): SharingWriteVerdict { this.logger?.error?.( @@ -520,7 +536,7 @@ export class SharingService implements ISharingService { async checkEdit( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const bypass = this.bypassVerdict(object, context); if (bypass) return bypass; @@ -577,7 +593,7 @@ export class SharingService implements ISharingService { async canEdit( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { return (await this.checkEdit(object, recordId, context)) !== 'deny'; } @@ -603,7 +619,7 @@ export class SharingService implements ISharingService { async checkDelete( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const bypass = this.bypassVerdict(object, context); if (bypass) return bypass; @@ -641,7 +657,7 @@ export class SharingService implements ISharingService { async canDelete( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { return (await this.checkDelete(object, recordId, context)) !== 'deny'; } @@ -683,7 +699,7 @@ export class SharingService implements ISharingService { object: string, recordId: string, operation: AuthoredRowWriteOperation, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { try { if (!context?.userId) return 'abstain'; @@ -728,7 +744,7 @@ export class SharingService implements ISharingService { async canManageShares( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { if (context?.isSystem) return true; if (!object || !recordId || !context?.userId) return false; @@ -792,7 +808,7 @@ export class SharingService implements ISharingService { private async isRecordVisible( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { try { const rows = await this.engine.find(object, { @@ -815,7 +831,7 @@ export class SharingService implements ISharingService { private async assertCanManageShares( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { if (context?.isSystem) return; if (!(await this.isRecordVisible(object, recordId, context))) { @@ -874,7 +890,7 @@ export class SharingService implements ISharingService { */ async grant( input: GrantShareInput, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { if (!input.object) throw new Error('VALIDATION_FAILED: object is required'); if (!input.recordId) throw new Error('VALIDATION_FAILED: recordId is required'); @@ -975,7 +991,7 @@ export class SharingService implements ISharingService { */ async revoke( shareId: string, - context: SharingExecutionContext, + context: ExecutionContext, scope?: { object: string; recordId: string }, ): Promise { if (!shareId) throw new Error('VALIDATION_FAILED: shareId is required'); @@ -1029,7 +1045,7 @@ export class SharingService implements ISharingService { async listShares( object: string, recordId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { if (!context?.isSystem) { await this.assertCanManageShares(object, recordId, context); @@ -1164,10 +1180,10 @@ export class SharingService implements ISharingService { * posture-scoped rather than unconditional. */ private async resolveOwnerScopeIds( - context: SharingExecutionContext, + context: ExecutionContext, scope: 'own' | 'own_and_reports' | 'unit' | 'unit_and_below' | 'org' | undefined, ): Promise { - const me = String((context as any).userId); + const me = String(context.userId); if (!scope || scope === 'own' || scope === 'org') return [me]; const resolver = this.hierarchyResolver?.(); if (!resolver) return [me]; @@ -1203,7 +1219,7 @@ export class SharingService implements ISharingService { // The @deprecated compatibility alias, carried through unchanged for // resolvers that still read it. It is NOT the authority — a resolver // reading it alone is the shape #5858 ruled out. - tenantId: (context as any).tenantId ?? null, + tenantId: context.tenantId ?? null, }, scope, ); @@ -1289,7 +1305,7 @@ export class SharingService implements ISharingService { return 'isolated'; } - private shouldBypass(object: string, context: SharingExecutionContext): boolean { + private shouldBypass(object: string, context: ExecutionContext): boolean { if (context?.isSystem) return true; if (this.bypassObjects.has(object)) return true; return false; From 526f4037ae7b4c023c9581fd7e86a2156bd69e55 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 9 Aug 2026 20:43:38 +0000 Subject: [PATCH 2/2] fix(plugin-sharing): SYSTEM_CTX cast count is 10, not 11 (#7136) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The measured count was taken after the new doc comment — which itself quotes the string `SYSTEM_CTX as any` — had been inserted, so the doc comment counted itself. On origin/main the erasure appears at exactly 10 call sites. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01BM1tNf5U3nEbHKR4fo5qVQ --- .changeset/sharing-audit-exec-context-annotations.md | 2 +- packages/plugins/plugin-sharing/src/sharing-rule-service.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.changeset/sharing-audit-exec-context-annotations.md b/.changeset/sharing-audit-exec-context-annotations.md index c3ae8914b2..4a0da368ca 100644 --- a/.changeset/sharing-audit-exec-context-annotations.md +++ b/.changeset/sharing-audit-exec-context-annotations.md @@ -23,7 +23,7 @@ them — and the casts the narrow annotation forced are gone: `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 11 of its call sites — an erasure + 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 diff --git a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts index 624e64df9f..3a3e2d19a8 100644 --- a/packages/plugins/plugin-sharing/src/sharing-rule-service.ts +++ b/packages/plugins/plugin-sharing/src/sharing-rule-service.ts @@ -24,7 +24,7 @@ import { BusinessUnitGraphService } from './business-unit-graph.js'; * System-elevated context for the rule evaluator's own reconcile writes. * * [#7136] Typed as the full envelope so it is passed AS ITSELF. It used to be - * declared `as const` and forced through an `as any` at all 11 of its context + * declared `as const` and forced through an `as any` at all 10 of its context * call sites — an erasure on an enforcement input, which switches checking off * for the whole argument, not just for the readonly-array mismatch that * provoked it.