diff --git a/.changeset/approvals-reports-exec-context-annotations.md b/.changeset/approvals-reports-exec-context-annotations.md new file mode 100644 index 0000000000..bc2fd91f61 --- /dev/null +++ b/.changeset/approvals-reports-exec-context-annotations.md @@ -0,0 +1,57 @@ +--- +"@objectstack/plugin-approvals": patch +"@objectstack/plugin-reports": patch +--- + +refactor(plugin-approvals,plugin-reports): enforcement implementations annotate the full `ExecutionContext` (#7135) + +The services half of #7070, mirroring what PR #7140 did for +`plugin-sharing` / `plugin-audit`. #6523 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 the six-field shape the contracts used to name, so nothing they could +*read* had widened. + +`ApprovalService`, the approval flow-node provider and `ReportService` now +declare `ExecutionContext` on all 43 of those positions, and the casts the +narrow annotation forced are gone: + +- `isOverrideActor()` read the derived `posture` (ADR-0095) through an + unchecked `(context as any)`. That gate decides whether a platform or tenant + admin may release a STUCK approval — one routed to an unstaffed position, the + only in-product recovery from a permanently locked record — so an erasure sat + directly on an enforcement input: a mistyped rung would have compiled and + silently denied every override. It is a declared read now. +- Both services' `SYSTEM_CTX` is typed as the envelope and passed as itself, + retiring the `SYSTEM_CTX as unknown as …` double casts at the three sites + that hand it to a contract method. +- The `(context as any).userId` / `.tenantId` reads in `ApprovalService` now + read declared fields. +- `OwnerContextResolver` returns the envelope, which is what a scheduled report + actually resolves for its owner (#2849 / #2980). + +**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, and no public export changes shape. + +Casts deliberately kept, and now documented where they sit: `organizationId` +is not a field of the envelope at all — that spelling has its own history +(#5858 / `check:org-identifier`) and was held out of this change by #7070. In +`approval-node.ts` the single remaining assertion exists only because the +literal names that key; it was reduced from `as unknown as …` to a single +`as ExecutionContext`, which still requires the literal to be comparable to +the envelope. + +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 per +package, `exec-context-annotation.pin.ts`: it hands each 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. + +The exported `SharingExecutionContext` type itself is NOT removed here: it is +defined in `packages/spec`, which is single-owner, so its retirement is a +separate follow-up. diff --git a/packages/plugins/plugin-approvals/src/approval-node.ts b/packages/plugins/plugin-approvals/src/approval-node.ts index a6be011a25..c26f8e20db 100644 --- a/packages/plugins/plugin-approvals/src/approval-node.ts +++ b/packages/plugins/plugin-approvals/src/approval-node.ts @@ -24,7 +24,10 @@ import { APPROVAL_NODE_TYPE, type ApprovalNodeConfig, } from '@objectstack/spec/automation'; -import type { SharingExecutionContext } from '@objectstack/spec/contracts'; +// [#7135] The full `resolveAuthzContext` envelope — what +// `IApprovalService.openNodeRequest` declares for its context parameter since +// #6523 (the #6206 ruling: no per-site subset contracts). +import type { ExecutionContext } from '@objectstack/spec/kernel'; import type { ApprovalService } from './approval-service.js'; import { registerApprovalReviseNode } from './approval-revise-node.js'; @@ -55,7 +58,7 @@ interface MinimalLogger { warn?: (msg: any, ...rest: any[]) => void; } -const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; +const SYSTEM_CTX: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; /** * Rebuild the nested object the engine's CEL conditions see from the flow's @@ -169,9 +172,20 @@ export function registerApprovalNode( }, { ...SYSTEM_CTX, userId: context?.userId, + // [#7135] ⚠️ This assertion SURVIVES the annotation widening, and it + // is `as ExecutionContext` rather than the `as unknown as + // SharingExecutionContext` double cast it replaces. The sole reason + // a cast is still needed is `organizationId`, which is not a field + // of the envelope at ALL — that spelling has its own history (#5858 + // / `check:org-identifier`) and was explicitly held out of this + // change (#7070), so removing the key here would be a RUNTIME change + // belonging to that card. Dropping the second hop matters: `as + // unknown as` erases the value entirely, while a single assertion + // still requires the literal to be comparable to the envelope, so a + // `userId: 42` here is once again a compile error. organizationId: context?.organizationId, tenantId: context?.tenantId, - } as unknown as SharingExecutionContext); + } as ExecutionContext); // #3447 P2: empty slate + onEmptyApprovers: 'auto_approve' — nobody to // ask, no request row. Complete (don't suspend) straight down the diff --git a/packages/plugins/plugin-approvals/src/approval-service.ts b/packages/plugins/plugin-approvals/src/approval-service.ts index ef32ae1f89..a0fc0db91b 100644 --- a/packages/plugins/plugin-approvals/src/approval-service.ts +++ b/packages/plugins/plugin-approvals/src/approval-service.ts @@ -32,8 +32,14 @@ import type { ApprovalResubmitInput, ApprovalResubmitResult, ApprovalStatus, - SharingExecutionContext, } from '@objectstack/spec/contracts'; +// [#7135] The full `resolveAuthzContext` envelope — what `IApprovalService` +// declares for every one of these context parameters since #6523 (the #6206 +// ruling: enforcement adjudicates on the whole envelope, never a per-site +// subset). Annotating the implementation with the retired six-field 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 { RESUME_AUTHORITY_SERVICE } from '@objectstack/spec/contracts'; import { isFileIdToken } from '@objectstack/spec/data'; import { isGrantActive } from '@objectstack/core'; @@ -213,7 +219,17 @@ export type ActionTokenOutcome = | { ok: true; action: 'approve' | 'reject'; request: ApprovalRequestRow; approverId: string } | { ok: false; reason: 'invalid' | 'expired' | 'consumed' | 'not_pending' | 'not_approver'; request?: ApprovalRequestRow }; -const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; +/** + * System-elevated context for this service's own metadata reads and writes. + * + * [#7135] Typed as the full envelope so it is passed AS ITSELF. It used to be + * declared `as const` and forced through `as unknown as + * SharingExecutionContext` at the three sites that handed it to a CONTRACT + * method — a double cast on an enforcement input, which switches checking off + * for the whole argument rather than for the readonly-array mismatch that + * provoked it. + */ +const SYSTEM_CTX: ExecutionContext = { isSystem: true, positions: [], permissions: [] }; /** * Who is acting, for the purpose of a data write made on their behalf (#3783). @@ -237,8 +253,8 @@ const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] } as const; * `null` for a machine caller (the SLA sweep passes {@link SYSTEM_CTX}), so a * reserved sentinel like {@link SLA_ACTOR_ID} can never surface as a `userId`. */ -function actingUserId(context: SharingExecutionContext | undefined): string | null { - const userId = (context as { userId?: unknown } | undefined)?.userId; +function actingUserId(context: ExecutionContext | undefined): string | null { + const userId = context?.userId; return typeof userId === 'string' && userId ? userId : null; } @@ -653,12 +669,19 @@ export class ApprovalService implements IApprovalService { * the derived `posture`, ADR-0095) so any transport that resolves through the * shared authz resolver lights this up without extra wiring. */ - private isOverrideActor(context: SharingExecutionContext, requestOrg?: string | null): boolean { + private isOverrideActor(context: ExecutionContext, requestOrg?: string | null): boolean { if (!context) return false; if (context.isSystem) return true; const perms = Array.isArray(context.permissions) ? context.permissions : []; const positions = Array.isArray(context.positions) ? context.positions : []; - const posture = (context as any).posture; + // [#7135] A DECLARED read. `posture` (ADR-0095 D2) is resolved by + // `resolveAuthzContext` and is a field of the envelope the contract has + // named here since #6523 — the doc block above already says it is the + // intended signal. Until this parameter widened, reading it meant an + // unchecked `as any` on an enforcement input: a typo (`postures`, + // `'PLATFORM-ADMIN'`) would have compiled and silently denied every + // override, leaving a stuck approval with no in-product recovery. + const posture = context.posture; const isPlatformAdmin = posture === 'PLATFORM_ADMIN' || perms.includes(ADMIN_FULL_ACCESS) || positions.includes(BUILTIN_IDENTITY_PLATFORM_ADMIN); @@ -670,7 +693,13 @@ export class ApprovalService implements IApprovalService { if (!isTenantAdmin) return false; // A tenant admin's authority stops at their own org; a null-org request is // global and any admin may release it. - const actorTenant = (context as any).tenantId ?? (context as any).organizationId ?? null; + // 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 is now the visible marker of which of the two names + // the contract actually knows. + const actorTenant = context.tenantId ?? (context as any).organizationId ?? null; return requestOrg == null || (actorTenant != null && String(requestOrg) === String(actorTenant)); } @@ -707,7 +736,7 @@ export class ApprovalService implements IApprovalService { */ private async resolveActor( actorId: string | undefined, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { // The machine callers — their actor is server-minted, not caller-supplied. if (context?.isSystem) { @@ -1447,7 +1476,7 @@ export class ApprovalService implements IApprovalService { */ variables?: Record | null; }, - 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'); @@ -1462,7 +1491,8 @@ export class ApprovalService implements IApprovalService { throw new Error(`DUPLICATE_REQUEST: a pending approval already exists for ${input.object}/${input.recordId}`); } - const ctxOrg = (context as any)?.organizationId ?? (context as any)?.tenantId ?? input.organizationId ?? null; + // `organizationId` is not on the envelope — see isOverrideActor(). + const ctxOrg = (context as any)?.organizationId ?? context?.tenantId ?? input.organizationId ?? null; const nowDate = this.clock.now(); // OOO auto-skip (#1322 M1): reroute individually-routed approvers who are // out of office. Collected hops drive the audit + notification below (M4). @@ -1681,7 +1711,7 @@ export class ApprovalService implements IApprovalService { async decideNode( requestId: string, input: { decision: 'approve' | 'reject'; actorId: string; comment?: string; attachments?: string[]; outputs?: Record }, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise<{ request: ApprovalRequestRow; runId: string | null; nodeId: string | null; finalized: boolean; decision: 'approve' | 'reject'; outputs?: Record }> { if (!requestId) throw new Error('VALIDATION_FAILED: requestId is required'); const actorId = await this.resolveActor(input?.actorId, context); @@ -2084,7 +2114,7 @@ export class ApprovalService implements IApprovalService { async decide( requestId: string, input: ApprovalDecisionInput, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const result = await this.decideNode(requestId, input, context); @@ -2140,7 +2170,7 @@ export class ApprovalService implements IApprovalService { async recall( requestId: string, input: ApprovalRecallInput, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { if (!requestId) throw new Error('VALIDATION_FAILED: requestId is required'); const actorId = await this.resolveActor(input?.actorId, context); @@ -2249,7 +2279,7 @@ export class ApprovalService implements IApprovalService { async sendBack( requestId: string, input: ApprovalSendBackInput, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const actorId = await this.resolveActor(input?.actorId, context); const raw = await this.loadPendingRow(requestId); @@ -2389,7 +2419,7 @@ export class ApprovalService implements IApprovalService { async resubmit( requestId: string, input: ApprovalResubmitInput, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const actorId = await this.resolveActor(input?.actorId, context); const rawRows = await this.engine.find('sys_approval_request', { @@ -2545,7 +2575,7 @@ export class ApprovalService implements IApprovalService { async reassign( requestId: string, input: { actorId: string; to: string; from?: string; comment?: string }, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise<{ request: ApprovalRequestRow }> { const actorId = await this.resolveActor(input?.actorId, context); const to = String(input?.to ?? '').trim(); @@ -2634,7 +2664,7 @@ export class ApprovalService implements IApprovalService { async remind( requestId: string, input: { actorId: string; comment?: string }, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise<{ request: ApprovalRequestRow; notified: number }> { const actorId = await this.resolveActor(input?.actorId, context); const raw = await this.loadPendingRow(requestId); @@ -2770,7 +2800,7 @@ export class ApprovalService implements IApprovalService { if (Date.parse(token.expires_at) < this.clock.now().getTime()) { return { ok: false, reason: 'expired' }; } - const request = await this.getRequest(token.request_id, SYSTEM_CTX as unknown as SharingExecutionContext); + const request = await this.getRequest(token.request_id, SYSTEM_CTX); if (!request || request.status !== 'pending') { return { ok: false, reason: 'not_pending', request: request ?? undefined }; } @@ -2810,7 +2840,7 @@ export class ApprovalService implements IApprovalService { // context, so the status mirror and every flow it cascades into are // attributed exactly like a decision made through the UI. Elevation is // unchanged: `isSystem` still stands in for the missing session. - }, { ...SYSTEM_CTX, userId: res.token.approver_id } as unknown as SharingExecutionContext); + }, { ...SYSTEM_CTX, userId: res.token.approver_id }); return { ok: true, action: res.token.action, request: out.request, approverId: res.token.approver_id }; } @@ -2821,7 +2851,7 @@ export class ApprovalService implements IApprovalService { async requestInfo( requestId: string, input: { actorId: string; comment: string }, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise<{ request: ApprovalRequestRow }> { const actorId = await this.resolveActor(input?.actorId, context); if (!input?.comment?.trim()) throw new Error('VALIDATION_FAILED: comment is required'); @@ -2860,7 +2890,7 @@ export class ApprovalService implements IApprovalService { async comment( requestId: string, input: { actorId: string; comment: string; attachments?: string[] }, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise<{ request: ApprovalRequestRow }> { const actorId = await this.resolveActor(input?.actorId, context); if (!input?.comment?.trim()) throw new Error('VALIDATION_FAILED: comment is required'); @@ -3256,7 +3286,7 @@ export class ApprovalService implements IApprovalService { decision: action === 'auto_approve' ? 'approve' : 'reject', actorId: SLA_ACTOR_ID, comment: 'SLA escalation', - }, SYSTEM_CTX as unknown as SharingExecutionContext); + }, SYSTEM_CTX); } else { // 'notify' (and the reassign-without-target fallback) await this.notify({ @@ -3634,7 +3664,7 @@ export class ApprovalService implements IApprovalService { submitterId?: string; q?: string; } | undefined, - context: SharingExecutionContext, + context: ExecutionContext, ): { where: any; tenantOrg: string | null } { const f: any = {}; if (filter?.object) f.object_name = filter.object; @@ -3646,7 +3676,8 @@ export class ApprovalService implements IApprovalService { // from leaking other-tenant rows since we deliberately query with // SYSTEM_CTX to bypass RLS on the engine (the approver-visibility rule // spans three identity forms, which RLS can't model cleanly). - const tenantOrg = (context as any)?.organizationId ?? (context as any)?.tenantId ?? null; + // `organizationId` is not on the envelope — see isOverrideActor(). + const tenantOrg = (context as any)?.organizationId ?? context?.tenantId ?? null; if (tenantOrg) f.organization_id = tenantOrg; // Free-text search, pushed down: `payload_json` carries the record // snapshot, so record titles match without any join. `$contains` is the @@ -3730,11 +3761,11 @@ export class ApprovalService implements IApprovalService { * request from someone who could actually act on it. */ private async visibleRequestIds( - context: SharingExecutionContext, + context: ExecutionContext, tenantOrg: string | null, ): Promise | null> { if (this.isOverrideActor(context, tenantOrg)) return null; - const uid = (context as any)?.userId != null ? String((context as any).userId) : ''; + const uid = context?.userId != null ? String(context.userId) : ''; // A tokenless/anonymous caller participates in nothing. Fail closed. if (!uid) return new Set(); @@ -3809,7 +3840,7 @@ export class ApprovalService implements IApprovalService { limit?: number; offset?: number; } | undefined, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const { where, tenantOrg } = this.buildRequestWhere(filter, context); const approverTargets = (Array.isArray(filter?.approverId) ? filter!.approverId : filter?.approverId ? [filter.approverId] : []) @@ -3852,7 +3883,7 @@ export class ApprovalService implements IApprovalService { async countRequests( filter: Parameters[0], - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const { where, tenantOrg } = this.buildRequestWhere(filter, context); const approverTargets = (Array.isArray(filter?.approverId) ? filter!.approverId : filter?.approverId ? [filter.approverId] : []) @@ -3897,23 +3928,24 @@ export class ApprovalService implements IApprovalService { */ private async readBackRequest( requestId: string, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { return this.loadRequest(requestId, context, false); } - async getRequest(requestId: string, context: SharingExecutionContext): Promise { + async getRequest(requestId: string, context: ExecutionContext): Promise { return this.loadRequest(requestId, context, true); } private async loadRequest( requestId: string, - context: SharingExecutionContext, + context: ExecutionContext, enforceVisibility: boolean, ): Promise { if (!requestId) return null; const where: any = { id: requestId }; - const tenantOrg = (context as any)?.organizationId ?? (context as any)?.tenantId; + // `organizationId` is not on the envelope — see isOverrideActor(). + const tenantOrg = (context as any)?.organizationId ?? context?.tenantId; if (tenantOrg) where.organization_id = tenantOrg; const rows = await this.engine.find('sys_approval_request', { where, limit: 1, context: SYSTEM_CTX, @@ -4009,8 +4041,8 @@ export class ApprovalService implements IApprovalService { * `can_act`/`is_submitter` block (system gets `can_override` too — it may act * on anything). Cheap + synchronous — safe on list reads. */ - private attachViewers(rows: ApprovalRequestRow[], context: SharingExecutionContext): void { - const uid = (context as any)?.userId != null ? String((context as any).userId) : null; + private attachViewers(rows: ApprovalRequestRow[], context: ExecutionContext): void { + const uid = context?.userId != null ? String(context.userId) : null; for (const row of rows) { const pending = row.pending_approvers ?? []; (row as any).viewer = { @@ -4062,7 +4094,7 @@ export class ApprovalService implements IApprovalService { } catch { /* display-only — never fail the read */ } } - async listActions(requestId: string, context: SharingExecutionContext): Promise { + async listActions(requestId: string, context: ExecutionContext): Promise { if (!requestId) return []; // Tenant gate: ensure the caller can see the parent request before // returning its action history. Skipping this would leak history rows @@ -4110,7 +4142,7 @@ export class ApprovalService implements IApprovalService { * exactly, rather than inventing a second, looser rule for the bytes. Fails * closed on any error. */ - async authorizeFileRead(actionId: string, context: SharingExecutionContext): Promise { + async authorizeFileRead(actionId: string, context: ExecutionContext): Promise { if (!actionId) return false; try { const rows = await this.engine.find('sys_approval_action', { diff --git a/packages/plugins/plugin-approvals/src/exec-context-annotation.pin.ts b/packages/plugins/plugin-approvals/src/exec-context-annotation.pin.ts new file mode 100644 index 0000000000..5d017941f0 --- /dev/null +++ b/packages/plugins/plugin-approvals/src/exec-context-annotation.pin.ts @@ -0,0 +1,87 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7135 — 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). #7135 is the services half of the #7070 + * consumer split — 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 the retired type's own doc block records. + * + * WHY A `.pin.ts` AND NOT A `*.test.ts`: `packages/plugins/plugin-approvals/ + * tsconfig.json` excludes `**\/*.test.ts` (measured on this card, and the same + * exclusion `plugin-sharing` carries — see #7136 / PR #7140), 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`). This file IS in that program. It + * is imported by nothing, so tsup (entry `src/index.ts`) never bundles it into + * `dist`. + */ + +import type { ApprovalService } from './approval-service.js'; + +type GetRequestContext = Parameters[1]; +type DecideContext = Parameters[2]; +type ListActionsContext = Parameters[1]; +type AuthorizeFileReadContext = Parameters[1]; +type OpenNodeRequestContext = Parameters[1]; +type ListRequestsContext = Parameters[1]; + +/** + * 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 __pinApprovalsTakesTheFullEnvelope( + getRequest: (requestId: string, context: GetRequestContext) => unknown, + decide: (requestId: string, input: never, context: DecideContext) => unknown, + listActions: (requestId: string, context: ListActionsContext) => unknown, + authorizeFileRead: (actionId: string, context: AuthorizeFileReadContext) => unknown, + openNodeRequest: (input: never, context: OpenNodeRequestContext) => unknown, + listRequests: (filter: undefined, context: ListRequestsContext) => unknown, +): void { + // ── POSITIVE: fields that exist ONLY on the full envelope, no cast. ─────── + // + // `posture` is the load-bearing one for THIS package: `isOverrideActor()` + // reads it to decide whether a platform/tenant admin may release a stuck + // approval, and until #7135 that read was an unchecked `(context as any)`. + getRequest('req_1', { userId: 'u1', posture: 'PLATFORM_ADMIN' }); + decide('req_1', undefined as never, { userId: 'u1', posture: 'TENANT_ADMIN', org_user_ids: ['u1', 'u2'] }); + listActions('req_1', { userId: 'u1', accessible_org_ids: ['org_a'] }); + authorizeFileRead('act_1', { userId: 'u1', posture: 'MEMBER' }); + openNodeRequest(undefined as never, { userId: 'u1', posture: 'MEMBER', accessible_org_ids: ['org_a'] }); + listRequests(undefined, { userId: 'u1', org_user_ids: ['u1'] }); + + // ── 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 + getRequest('req_1', { userId: 'u1', posture: 'SUPERUSER' }); + // @ts-expect-error `userId` is a string on the envelope, not a number + listActions('req_1', { userId: 42 }); + // @ts-expect-error `accessible_org_ids` is a string[], not a bare string + authorizeFileRead('act_1', { 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 #7135 on purpose. The reads of it left in `approval-service.ts` are + // still cast, and this line is why they have to be. + decide('req_1', undefined as never, { organizationId: 'org_a' }); +} diff --git a/packages/plugins/plugin-reports/src/exec-context-annotation.pin.ts b/packages/plugins/plugin-reports/src/exec-context-annotation.pin.ts new file mode 100644 index 0000000000..5bf8237ae3 --- /dev/null +++ b/packages/plugins/plugin-reports/src/exec-context-annotation.pin.ts @@ -0,0 +1,95 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7135 — compile-time pin for the CONTEXT type this plugin's report methods + * accept, and for what `OwnerContextResolver` is required to return. + * + * #6523 converged 36 contract signatures onto the full `ExecutionContext` (the + * #6206 ruling: enforcement adjudicates on the whole `resolveAuthzContext` + * envelope, never a per-site subset). #7135 is the services half of the #7070 + * consumer split — 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 the retired type's own doc block records. + * + * WHY A `.pin.ts` AND NOT A `*.test.ts`: unlike its sibling packages, + * `packages/plugins/plugin-reports/tsconfig.json` does NOT exclude + * `**\/*.test.ts` (measured on this card — `plugin-approvals` and + * `plugin-sharing` both do), so a pin in a test file here would in fact be + * read by `tsc --noEmit`. The `.pin.ts` convention is kept anyway: it does not + * depend on that exclusion staying absent, it survives a vitest-only run that + * never type-checks, and it keeps both halves of this card's pin identical in + * shape. It is imported by nothing, so tsup (entry `src/index.ts`) never + * bundles it into `dist`. + */ + +import type { ReportService, OwnerContextResolver } from './report-service.js'; + +type SaveReportContext = Parameters[1]; +type RunContext = Parameters[1]; +type GetReportContext = Parameters[1]; +type ScheduleReportContext = Parameters[1]; +type ListSchedulesContext = Parameters[1]; + +/** + * What a scheduled run executes AS. `resolveOwnerContext` resolves a saved + * report's owner into a real, RLS-bearing context so the digest sees the rows + * the owner would see interactively (#2849 / #2980); typing its result as the + * envelope is what lets `executeReport` read the whole thing it was handed. + */ +type ResolvedOwnerContext = NonNullable>>; + +/** + * 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 __pinReportsTakesTheFullEnvelope( + saveReport: (input: never, context: SaveReportContext) => unknown, + run: (reportId: string, context: RunContext) => unknown, + getReport: (reportId: string, context: GetReportContext) => unknown, + scheduleReport: (input: never, context: ScheduleReportContext) => unknown, + listSchedules: (filter: undefined, context: ListSchedulesContext) => unknown, + ownerContext: ResolvedOwnerContext, +): void { + // ── POSITIVE: fields that exist ONLY on the full envelope, no cast. ─────── + saveReport(undefined as never, { userId: 'u1', posture: 'MEMBER' }); + run('rep_1', { userId: 'u1', posture: 'TENANT_ADMIN', accessible_org_ids: ['org_a'] }); + getReport('rep_1', { userId: 'u1', org_user_ids: ['u1', 'u2'] }); + scheduleReport(undefined as never, { userId: 'u1', accessible_org_ids: ['org_a'] }); + listSchedules(undefined, { userId: 'u1', posture: 'PLATFORM_ADMIN' }); + + // The resolver hands back what it RESOLVED. Reading a field the six-field + // shape never had is what pins that: a scheduled run adjudicates on the + // whole envelope or it is not running as the owner at all. + const posture: ResolvedOwnerContext['posture'] = ownerContext.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 + run('rep_1', { userId: 'u1', posture: 'SUPERUSER' }); + // @ts-expect-error `userId` is a string on the envelope, not a number + getReport('rep_1', { userId: 42 }); + // @ts-expect-error `accessible_org_ids` is a string[], not a bare string + listSchedules(undefined, { 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 #7135 on purpose. + saveReport(undefined as never, { organizationId: 'org_a' }); +} diff --git a/packages/plugins/plugin-reports/src/report-service.ts b/packages/plugins/plugin-reports/src/report-service.ts index 00d49641b5..e733e7a9f7 100644 --- a/packages/plugins/plugin-reports/src/report-service.ts +++ b/packages/plugins/plugin-reports/src/report-service.ts @@ -9,8 +9,14 @@ import type { ReportFormat, SaveReportInput, ScheduleReportInput, - SharingExecutionContext, } from '@objectstack/spec/contracts'; +// [#7135] The full `resolveAuthzContext` envelope — what `IReportService` +// declares for every one of these context parameters since #6523 (the #6206 +// ruling: enforcement adjudicates on the whole envelope, never a per-site +// subset). A scheduled run resolves a REAL owner context through +// `OwnerContextResolver`; naming the retired six-field shape here made this +// file's own type say it could not see what that resolver returns. +import type { ExecutionContext } from '@objectstack/spec/kernel'; import { Cron } from 'croner'; /** @@ -182,7 +188,7 @@ function renderSubject(template: string | undefined, vars: Record Promise; +) => Promise; export interface ReportServiceOptions { engine: ReportEngine; @@ -267,7 +273,7 @@ export class ReportService implements IReportService { private async assertExportAllowed( object: string, format: string, - context: SharingExecutionContext | undefined, + context: ExecutionContext | undefined, ): Promise { if (!BULK_EXPORT_FORMATS.has(format)) return; if (context?.isSystem) return; @@ -297,7 +303,7 @@ export class ReportService implements IReportService { * report by id (#2980). An explicit elevated context (`isSystem`) — the * scheduler / server tooling — sees everything. */ - private canAccessReport(row: { owner_id?: unknown } | null | undefined, context: SharingExecutionContext | undefined): boolean { + private canAccessReport(row: { owner_id?: unknown } | null | undefined, context: ExecutionContext | undefined): boolean { if (!row) return false; if (context?.isSystem) return true; const userId = context?.userId; @@ -322,7 +328,7 @@ export class ReportService implements IReportService { // ── Report CRUD ──────────────────────────────────────────────── - async saveReport(input: SaveReportInput, context: SharingExecutionContext): Promise { + async saveReport(input: SaveReportInput, context: ExecutionContext): Promise { if (!input.name) throw new Error('VALIDATION_FAILED: name is required'); if (!input.object) throw new Error('VALIDATION_FAILED: object is required'); if (!input.query) throw new Error('VALIDATION_FAILED: query is required'); @@ -366,7 +372,7 @@ export class ReportService implements IReportService { async listReports( filter: { object?: string; ownerId?: string } | undefined, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { const f: any = {}; if (filter?.object) f.object_name = filter.object; @@ -387,14 +393,14 @@ export class ReportService implements IReportService { return Array.isArray(rows) ? rows.map(rowFromSaved) : []; } - async getReport(reportId: string, context: SharingExecutionContext): Promise { + async getReport(reportId: string, context: ExecutionContext): Promise { const row = await this.loadReportRow(reportId); // Unauthorized reads are indistinguishable from a genuine miss (#2980). if (!this.canAccessReport(row, context)) return null; return rowFromSaved(row); } - async deleteReport(reportId: string, context: SharingExecutionContext): Promise { + async deleteReport(reportId: string, context: ExecutionContext): Promise { if (!reportId) throw new Error('VALIDATION_FAILED: reportId is required'); const row = await this.loadReportRow(reportId); if (!row) return; // idempotent — nothing to drop @@ -415,13 +421,13 @@ export class ReportService implements IReportService { // ── Execution ─────────────────────────────────────────────────── - async run(reportId: string, context: SharingExecutionContext): Promise { + async run(reportId: string, context: ExecutionContext): Promise { const report = await this.getReport(reportId, context); if (!report) throw new Error(`REPORT_NOT_FOUND: ${reportId}`); return this.executeReport(report, context); } - async runAdHoc(input: SaveReportInput, context: SharingExecutionContext): Promise { + async runAdHoc(input: SaveReportInput, context: ExecutionContext): Promise { if (!input.object) throw new Error('VALIDATION_FAILED: object is required'); if (!input.query) throw new Error('VALIDATION_FAILED: query is required'); const adhoc: SavedReport = { @@ -436,7 +442,7 @@ export class ReportService implements IReportService { private async executeReport( report: SavedReport, - context: SharingExecutionContext, + context: ExecutionContext, stamp = true, ): Promise { // [#3544 / #3710] The export axis, BEFORE any row is read — a refusal must @@ -489,7 +495,7 @@ export class ReportService implements IReportService { // ── Schedules ────────────────────────────────────────────────── - async scheduleReport(input: ScheduleReportInput, context: SharingExecutionContext): Promise { + async scheduleReport(input: ScheduleReportInput, context: ExecutionContext): Promise { if (!input.reportId) throw new Error('VALIDATION_FAILED: reportId is required'); if (!input.recipients || input.recipients.length === 0) { throw new Error('VALIDATION_FAILED: recipients must be a non-empty array'); @@ -540,7 +546,7 @@ export class ReportService implements IReportService { return rowFromSchedule(row); } - async unscheduleReport(scheduleId: string, context: SharingExecutionContext): Promise { + async unscheduleReport(scheduleId: string, context: ExecutionContext): Promise { if (!scheduleId) throw new Error('VALIDATION_FAILED: scheduleId is required'); const schedule = await this.loadScheduleRow(scheduleId); if (!schedule) return; // idempotent — nothing to drop (mirrors deleteReport) @@ -557,7 +563,7 @@ export class ReportService implements IReportService { async listSchedules( filter: { reportId?: string } | undefined, - context: SharingExecutionContext, + context: ExecutionContext, ): Promise { // Schedules are owned through their report (#2980): a non-system caller may // only list the schedules of a report they can access. The route always