diff --git a/.changeset/report-caller-envelope-forwarding.md b/.changeset/report-caller-envelope-forwarding.md new file mode 100644 index 0000000000..7680b6e9e0 --- /dev/null +++ b/.changeset/report-caller-envelope-forwarding.md @@ -0,0 +1,30 @@ +--- +'@objectstack/plugin-reports': patch +--- + +Reports read with the caller's whole execution envelope, so a `group`-posture report no longer under-reports + +`executeReport` rebuilt a five-field projection of the caller's `ExecutionContext` +(`userId` / `tenantId` / `positions` / `permissions` / `isSystem`) before handing it to +the engine read that produces the report — while the method's own comment promised +"reports execute with the caller's identity". + +**Before.** `accessible_org_ids` was not in that projection, and the engine reads it by +name (`buildDriverOptions`, ADR-0105 D2 / #3623) to widen the driver's native tenant +scope to the caller's whole membership set under the `group` tenancy posture. Absent, the +drivers fall back to active-org equality — "fail toward isolation". So the identical query +returned the membership union in an interactive list view and collapsed to the active org +inside a **saved or scheduled** report: silently short rows, no error, nothing in the +output saying so. Measured end-to-end on a real kernel + SQL driver: three rows across two +member orgs came back as three interactively and two in the report, and a scheduled CSV +digest emailed the owner the same two. `timezone` went the same way, so a read-time +formula field resolved its calendar day in UTC instead of the caller's business timezone; +`posture`, `org_user_ids`, `systemPermissions` and `onBehalfOf` were dropped too. + +**After.** The read receives the caller's envelope whole (the #6206 ruling — enforcement +adjudicates on the whole `resolveAuthzContext` envelope, never a per-site subset), minus +the `__`-prefixed keys plugin-security stamps for the operation in flight, and as a fresh +object so a callee's stamp cannot write back into the caller's request context. The same +shape `plugin-audit` (#7141) and `service-storage` (#7145) landed. Direction is unchanged +outside `group`: the `isolated` posture, a deployment with no posture provider, and a +`group` caller with an empty accessible set all still read at active-org equality. diff --git a/packages/plugins/plugin-reports/package.json b/packages/plugins/plugin-reports/package.json index bff958763d..574cd6dfeb 100644 --- a/packages/plugins/plugin-reports/package.json +++ b/packages/plugins/plugin-reports/package.json @@ -2,7 +2,7 @@ "name": "@objectstack/plugin-reports", "version": "17.0.0-rc.5", "license": "Apache-2.0", - "description": "Saved reports + scheduled email digests for ObjectStack — sys_saved_report + sys_report_schedule + IReportService.", + "description": "Saved reports + scheduled email digests for ObjectStack \u2014 sys_saved_report + sys_report_schedule + IReportService.", "main": "dist/index.js", "types": "dist/index.d.ts", "exports": { @@ -24,6 +24,8 @@ "croner": "^10.0.1" }, "devDependencies": { + "@objectstack/driver-sql": "workspace:*", + "@objectstack/objectql": "workspace:*", "@types/node": "^26.1.2", "typescript": "^6.0.3", "vitest": "^4.1.10" diff --git a/packages/plugins/plugin-reports/src/report-group-posture-scope.integration.test.ts b/packages/plugins/plugin-reports/src/report-group-posture-scope.integration.test.ts new file mode 100644 index 0000000000..7566fe2c1d --- /dev/null +++ b/packages/plugins/plugin-reports/src/report-group-posture-scope.integration.test.ts @@ -0,0 +1,250 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * #7204 — a `group`-posture report returns the caller's membership union, the + * same row set the caller sees interactively. + * + * `executeReport` used to rebuild a five-field projection of the caller's + * execution envelope (`userId` / `tenantId` / `positions` / `permissions` / + * `isSystem`) before handing it to the engine read that produces the report. + * `accessible_org_ids` was not in that projection, and `buildDriverOptions` + * reads it BY NAME (`engine.ts`, ADR-0105 D2 / #3623) to widen the driver's + * native tenant scope to the caller's whole membership set under the `group` + * posture. Absent, drivers "fall back to equality: fail toward isolation" — so + * the report silently returned FEWER rows than the identical interactive query, + * with no error and nothing in the output saying so. + * + * WHY THIS FILE REFUSES TO STUB THE ENGINE. The defect is invisible one layer + * up: a fake engine that records the context it was handed can only assert that + * a key is present, and "the key is on the object" is exactly what the previous + * shape of this bug looked like from inside the service. The consumer is the + * REAL `buildDriverOptions` → real `@objectstack/driver-sql` native scope, so + * the assertions below land on ROW SETS: the union of the rows in both orgs, or + * the equality subset. Backend is better-sqlite3 `:memory:`, the canonical + * in-repo integration stack (PR #5715). + * + * The posture matrix is load-bearing, not decoration. `group` is the only + * posture the widening applies to; `isolated`, "no provider wired" and "group + * with an empty accessible set" must all still collapse to active-org equality + * after the fix, or the change traded under-reporting for exposure. + */ + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { ObjectKernel } from '@objectstack/core'; +import { ObjectQLPlugin } from '@objectstack/objectql'; +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SysSavedReport, SysReportSchedule } from '@objectstack/platform-objects/audit'; +import { ReportService, type ReportEngine, type ReportEmail } from './report-service.js'; + +/** + * A tenant-scoped business object. `organization_id` is what makes the driver's + * native tenant scope engage at all (`isTenancyDisabled` / the SQL driver's + * tenant column), and `day` is a READ-TIME formula field: `applyFormulaPlan` + * evaluates it with `execCtx.timezone`, which the same projection also dropped. + */ +const account = { + name: 'account', + label: 'Account', + fields: { + organization_id: { name: 'organization_id', label: 'Org', type: 'text' }, + name: { name: 'name', label: 'Name', type: 'text' }, + day: { + name: 'day', + label: 'Calendar day', + type: 'formula', + expression: { dialect: 'cel', source: 'today()' }, + }, + }, +}; + +/** The caller: active org `org_a`, membership union `org_a` + `org_b`. */ +const GROUP_CTX = { + userId: 'u1', + tenantId: 'org_a', + positions: [], + permissions: [], + posture: 'MEMBER', + accessible_org_ids: ['org_a', 'org_b'], +} as any; + +const ids = (rows: any[]): string[] => rows.map((r) => String(r.id)).sort(); + +/** + * The engine surface this harness drives. `getService('objectql')` is declared + * as the data-plane contract `IDataEngine`; the boot-time knobs below + * (`registerDriver`, `registry`, `syncSchemas`) and the posture provider + * plugin-security wires in production sit outside it, so they are named here + * rather than erased with `any`. + */ +interface TestEngine extends IDataEngine { + registerDriver(driver: unknown, isDefault?: boolean): void; + registry: { registerObject(def: unknown, packageId: string, namespace: string): void }; + syncSchemas(): Promise; + setTenancyPostureProvider(provider: () => string | undefined): void; + destroy(): Promise; +} + +describe('#7204 a group-posture report reads the caller\'s whole membership union', () => { + let objectql: TestEngine | undefined; + let svc: ReportService; + let email: ReportEmail & { _sent: any[] }; + + afterEach(async () => { + vi.useRealTimers(); + try { await objectql?.destroy(); } catch { /* noop */ } + }); + + beforeEach(async () => { + const kernel = new ObjectKernel({ logger: { level: 'error' } }); + await kernel.use(new ObjectQLPlugin()); + await kernel.bootstrap(); + objectql = kernel.getService('objectql') as TestEngine; + + // The engine's own `init()` ran during bootstrap, before this driver + // existed, so the connect the engine would have done is done here. + const driver = new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }); + await driver.connect(); + engine().registerDriver(driver, true); + for (const def of [account, SysSavedReport, SysReportSchedule]) { + engine().registry.registerObject(def, 'reports-test', 'reports-test'); + } + await engine().syncSchemas(); + + // Two rows in the active org, one in the other org the caller belongs to. + for (const row of [ + { id: 'a1', organization_id: 'org_a', name: 'A1' }, + { id: 'a2', organization_id: 'org_a', name: 'A2' }, + { id: 'b1', organization_id: 'org_b', name: 'B1' }, + ]) { + await engine().insert('account', row, { context: { isSystem: true } }); + } + + const sent: any[] = []; + email = { + _sent: sent, + async send(input) { sent.push(input); return { status: 'sent' as const }; }, + }; + svc = new ReportService({ + engine: engine() as unknown as ReportEngine, + email, + // A scheduled run executes as the report's OWNER (#2849 / #2980) — the + // resolver hands back a real RLS-bearing envelope, membership set and all. + resolveOwnerContext: async (ownerId: string) => + (ownerId === 'u1' ? ({ ...GROUP_CTX } as any) : null), + }); + }); + + /** What plugin-security's wiring reports in a deployment of this posture. */ + const posture = (p: string | undefined) => engine().setTenancyPostureProvider(() => p); + + /** The booted engine — narrowed once so every call site stays typed. */ + const engine = (): TestEngine => objectql as TestEngine; + + const saveAccountReport = async (format = 'csv') => + svc.saveReport( + { name: 'Accounts', object: 'account', query: {}, format } as any, + GROUP_CTX, + ); + + describe('group posture', () => { + beforeEach(() => posture('group')); + + it('a SAVED report returns the same rows as the identical interactive query', async () => { + const interactive = await engine().find('account', {}, { context: GROUP_CTX }); + expect(ids(interactive), 'the interactive baseline is the union').toEqual(['a1', 'a2', 'b1']); + + const report = await svc.run((await saveAccountReport()).id, GROUP_CTX); + + // The card's exact claim: the saved-report path used to return FEWER rows + // (['a1','a2'] — active-org equality) than the interactive query above. + expect(ids(report.rows)).toEqual(ids(interactive)); + expect(report.rowCount).toBe(3); + }); + + it('an AD-HOC report returns the union too', async () => { + const report = await svc.runAdHoc( + { name: 'Ad hoc', object: 'account', query: {} } as any, + GROUP_CTX, + ); + expect(ids(report.rows)).toEqual(['a1', 'a2', 'b1']); + }); + + it('a SCHEDULED run emails the owner the union, not the active org', async () => { + const report = await saveAccountReport(); + await svc.scheduleReport( + { reportId: report.id, recipients: ['ops@example.com'], format: 'csv', intervalMinutes: 60 }, + GROUP_CTX, + ); + + const result = await svc.dispatchDue(new Date(Date.now() + 3 * 60 * 60 * 1000)); + expect(result, 'the sweep must actually fire').toMatchObject({ fired: 1, failed: 0 }); + + const csv: string = email._sent[email._sent.length - 1]?.attachments?.[0]?.content ?? ''; + const dataRows = csv.split('\r\n').slice(1).filter(Boolean); + expect(dataRows).toHaveLength(3); + expect(csv).toContain('B1'); + }); + + it('the report also sees rows the ACTIVE org has none of', async () => { + // Nothing in org_a at all: under equality the report is empty, under the + // union it is the one org_b row. Isolates the widening from "the active + // org happened to hold most of the rows". + const report = await svc.runAdHoc( + { name: 'B only', object: 'account', query: { filter: { name: 'B1' } } } as any, + GROUP_CTX, + ); + expect(ids(report.rows)).toEqual(['b1']); + }); + + it('an EMPTY accessible set still collapses to active-org equality (fail toward isolation)', async () => { + const ctx = { ...GROUP_CTX, accessible_org_ids: [] }; + const interactive = await engine().find('account', {}, { context: ctx }); + const report = await svc.runAdHoc({ name: 'r', object: 'account', query: {} } as any, ctx); + expect(ids(report.rows)).toEqual(['a1', 'a2']); + expect(ids(report.rows)).toEqual(ids(interactive)); + }); + + it('forwards the business timezone, so a read-time formula field resolves the caller\'s calendar day', async () => { + // 20:00Z is the day BEFORE in UTC and the day AFTER in UTC+14, so the two + // timezones disagree deterministically at this instant. `today()` is a + // read-time formula evaluated by `applyFormulaPlan` with `execCtx.timezone` + // — dropped by the same projection, and observable on the row's VALUE. + vi.useFakeTimers({ shouldAdvanceTime: true }); + vi.setSystemTime(new Date('2026-08-10T20:00:00Z')); + const ctx = { ...GROUP_CTX, timezone: 'Pacific/Kiritimati' }; + + const interactive = await engine().find('account', { where: { id: 'a1' } }, { context: ctx }); + const report = await svc.runAdHoc( + { name: 'r', object: 'account', query: { filter: { id: 'a1' } } } as any, + ctx, + ); + + const dayOf = (v: unknown): string => + String(v instanceof Date ? v.toISOString() : v).slice(0, 10); + expect(dayOf(interactive[0]?.day), 'UTC+14 is already on the 11th').toBe('2026-08-11'); + expect(dayOf((report.rows[0] as any)?.day)).toBe(dayOf(interactive[0]?.day)); + }); + }); + + describe('every other posture is unchanged — the widening is group-only', () => { + it('isolated posture: the report stays at active-org equality', async () => { + posture('isolated'); + const interactive = await engine().find('account', {}, { context: GROUP_CTX }); + const report = await svc.runAdHoc({ name: 'r', object: 'account', query: {} } as any, GROUP_CTX); + expect(ids(report.rows)).toEqual(['a1', 'a2']); + expect(ids(report.rows)).toEqual(ids(interactive)); + }); + + it('no posture provider (no enforcement layer): equality, never widened', async () => { + const interactive = await engine().find('account', {}, { context: GROUP_CTX }); + const report = await svc.runAdHoc({ name: 'r', object: 'account', query: {} } as any, GROUP_CTX); + expect(ids(report.rows)).toEqual(['a1', 'a2']); + expect(ids(report.rows)).toEqual(ids(interactive)); + }); + }); +}); diff --git a/packages/plugins/plugin-reports/src/report-service.test.ts b/packages/plugins/plugin-reports/src/report-service.test.ts index ccb8bc4a3f..c4458ea77f 100644 --- a/packages/plugins/plugin-reports/src/report-service.test.ts +++ b/packages/plugins/plugin-reports/src/report-service.test.ts @@ -500,4 +500,124 @@ describe('ReportService', () => { expect(seen).toEqual(['u1']); // resolved the owner, not a system context }); }); + + // ─── #7204 — the caller envelope the report read receives ───────── + // + // The row-level consequence (a `group`-posture report under-reporting) is + // pinned end-to-end against a real engine + real SQL driver in + // `report-group-posture-scope.integration.test.ts` — that is where the claim + // "the report returns fewer rows than the same query interactively" is + // measured, because a key sitting on a context object is precisely what this + // defect looked like from in here. What is left for a fake engine is the + // structural half: WHICH keys cross, and that the crossing cannot write back. + describe('executeReport forwards the caller envelope, not a rebuilt subset (#7204)', () => { + const lastFindContext = () => + (engine._tables as any) && (engine as any)._lastLeadFindContext; + + beforeEach(() => { + const inner = engine.find.bind(engine); + (engine as any).find = async (object: string, options?: any) => { + if (object === 'lead') (engine as any)._lastLeadFindContext = options?.context; + return inner(object, options); + }; + }); + + it('forwards the principal fields the projection used to drop', async () => { + await svc.runAdHoc({ name: 'A', object: 'lead', query: {} }, { + userId: 'u1', + tenantId: 'org_a', + positions: ['p1'], + permissions: ['perm'], + // Every one of these was dropped by the five-field projection. + accessible_org_ids: ['org_a', 'org_b'], + posture: 'MEMBER', + org_user_ids: ['u1', 'u2'], + systemPermissions: ['viewAllData'], + timezone: 'Asia/Shanghai', + principalKind: 'agent', + onBehalfOf: { userId: 'u9' }, + } as any); + + expect(lastFindContext()).toMatchObject({ + userId: 'u1', + tenantId: 'org_a', + positions: ['p1'], + permissions: ['perm'], + accessible_org_ids: ['org_a', 'org_b'], + posture: 'MEMBER', + org_user_ids: ['u1', 'u2'], + systemPermissions: ['viewAllData'], + timezone: 'Asia/Shanghai', + principalKind: 'agent', + onBehalfOf: { userId: 'u9' }, + }); + }); + + it('does NOT forward the middleware-private `__` keys — another object\'s access depth stays behind', async () => { + // A REST request that touched some other object before reaching + // /reports/:id/run arrives with that object's depth already stamped on it, + // and plugin-security only OVERWRITES the stamp when it resolves + // permission sets for the new object. Carried across, it would widen the + // report's owner-match on a question it was never resolved for. + await svc.runAdHoc({ name: 'A', object: 'lead', query: {} }, { + userId: 'u1', + __readScope: 'all', + __delegatorReadScope: 'all', + __expandRead: true, + __referentialFieldClear: true, + } as any); + + const seen = lastFindContext(); + expect(seen.userId).toBe('u1'); + expect(Object.keys(seen).filter((k) => k.startsWith('__'))).toEqual([]); + }); + + it('hands the engine a FRESH object — a callee stamping onto it cannot write back into the caller\'s envelope', async () => { + // plugin-security stamps `sc.__readScope = …` in place on whatever it is + // handed. Forwarding the caller's own object by reference would leave the + // report's depth on the REQUEST context the route goes on using. + const caller: any = { userId: 'u1', accessible_org_ids: ['org_a'] }; + (engine as any).find = async (object: string, options?: any) => { + if (object === 'lead') { + (engine as any)._lastLeadFindContext = options?.context; + options.context.__readScope = 'all'; + } + return engine._tables[object] ?? []; + }; + + await svc.runAdHoc({ name: 'A', object: 'lead', query: {} }, caller); + + expect(lastFindContext()).not.toBe(caller); + expect(lastFindContext().__readScope).toBe('all'); // the callee did stamp + expect(caller.__readScope, 'the caller envelope must be untouched').toBeUndefined(); + }); + + it('keeps the projection\'s defaults for an envelope that omits them', async () => { + // Byte-for-byte what the five-field projection produced: this change adds + // fields, it does not change the ones that were already there. + await svc.runAdHoc({ name: 'A', object: 'lead', query: {} }, { userId: 'u1' } as any); + expect(lastFindContext()).toMatchObject({ + userId: 'u1', positions: [], permissions: [], isSystem: false, + }); + }); + + it('an explicit isSystem:true still crosses (the scheduler / tooling path)', async () => { + await svc.runAdHoc({ name: 'A', object: 'lead', query: {} }, { isSystem: true } as any); + expect(lastFindContext().isSystem).toBe(true); + }); + + it('assertExportAllowed still sees the UN-projected caller context', async () => { + // The export axis runs before any row is read, against the caller's own + // envelope — the forwarding change must not have re-routed its input. + const seen: any[] = []; + const exportSvc = new ReportService({ + engine: engine as any, email, clock: { now: () => now }, + canExport: async (_object: string, ctx: unknown) => { seen.push(ctx); return true; }, + }); + const caller: any = { userId: 'u1', accessible_org_ids: ['org_a', 'org_b'] }; + await exportSvc.runAdHoc({ name: 'A', object: 'lead', query: {}, format: 'csv' }, caller); + expect(seen).toHaveLength(1); + expect(seen[0]).toBe(caller); + }); + }); }); diff --git a/packages/plugins/plugin-reports/src/report-service.ts b/packages/plugins/plugin-reports/src/report-service.ts index e733e7a9f7..effb28986e 100644 --- a/packages/plugins/plugin-reports/src/report-service.ts +++ b/packages/plugins/plugin-reports/src/report-service.ts @@ -174,6 +174,51 @@ function renderSubject(template: string | undefined, vars: Record vars[String(k)] ?? ''); } +// ─── Caller envelope ────────────────────────────────────────────── + +/** + * Keys plugin-security's middleware STAMPS onto the operation context, resolved + * for the object of the operation in flight. + * + * They are middleware-private vocabulary, not fields of `ExecutionContext`, and + * every one of them is read as a WIDENING input: the ADR-0057 D1 access DEPTH + * the sharing owner-match expands to (`__readScope` / `__writeScope`, plus the + * ADR-0090 D10 delegator halves, stamped in place by `security-plugin.ts` — + * `sc.__readScope = …`), and the engine's internal privilege markers on the + * same channel (`__expandRead` waives the object-level CRUD check for a lookup + * expansion, `__referentialFieldClear` the referential-clear write). + * + * A report read asks about `report.object_name`, which is not necessarily the + * object the caller's envelope last carried a depth for — a REST request that + * touched another object before reaching `/reports/:id/run` hands over an + * envelope the middleware has already written into, and plugin-security only + * OVERWRITES `__readScope` when it resolves permission sets for the new object + * (`if (permissionSets.length > 0)`). A stale depth therefore survives into a + * question it was never resolved for. Dropped by PREFIX rather than by a name + * list: the `__` convention is what marks a key as belonging to the operation + * in flight, and a list would go stale the day the middleware stamps another + * one. Same shape as `plugin-audit`'s and `service-storage`'s kits (#7141 / + * #7145). + */ +const OPERATION_PRIVATE_KEY_PREFIX = '__'; + +/** + * The caller's execution envelope, minus the operation-private keys above. + * + * A FRESH object every time, in both directions: the engine's middleware + * stamps `__readScope` for `report.object_name` onto whatever it is handed, so + * forwarding the caller's own envelope by reference would write that depth + * back into the REQUEST context the route goes on using. + */ +function withoutOperationPrivateKeys(exec: Record): Record { + const out: Record = {}; + for (const [key, value] of Object.entries(exec)) { + if (key.startsWith(OPERATION_PRIVATE_KEY_PREFIX)) continue; + out[key] = value; + } + return out; +} + // ─── Service ────────────────────────────────────────────────────── /** @@ -458,9 +503,35 @@ export class ReportService implements IReportService { // Reports execute with the caller's identity so sharing rules // (if installed) apply. Falls back to system bypass only when // the report definition was created by a system writer. + // + // [#7204] The WHOLE envelope, not a rebuilt subset of it — the #6206 + // ruling (#6523): a read that adjudicates on the caller's identity + // adjudicates on the whole `resolveAuthzContext` envelope. The + // five-field projection this replaced (`userId` / `tenantId` / + // `positions` / `permissions` / `isSystem`) was doing two jobs, and + // only one of them was correct: + // + // - dropping the middleware-private keys — CORRECT, and preserved + // above by {@link withoutOperationPrivateKeys}; + // - dropping the PRINCIPAL fields — the defect. `accessible_org_ids` + // (ADR-0105 D2) is the one that changes rows: `buildDriverOptions` + // reads it BY NAME to widen the driver's native tenant scope to the + // caller's membership union under the `group` posture, and an absent + // set makes drivers "fall back to equality: fail toward isolation". + // So the same query returned the union in an interactive list view + // and collapsed to active-org equality inside a saved or scheduled + // report — silently short rows, no error. `timezone` is read two + // lines up in the same engine method (`hasTz`) and again by + // `applyFormulaPlan` for read-time formula fields, and `posture`, + // `org_user_ids`, `systemPermissions` and `onBehalfOf` went the same + // way; they are forwarded now for the same reason — the envelope is + // the contract's unit. + // + // The three defaults below are byte-for-byte what the projection + // produced for an envelope that omits them, and are kept so this change + // adds fields without changing any that were already there. context: { - userId: context.userId, - tenantId: context.tenantId, + ...withoutOperationPrivateKeys(context as unknown as Record), positions: context.positions ?? [], permissions: context.permissions ?? [], isSystem: context.isSystem ?? false, diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index a57d2b127f..7e88375c1c 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -1646,6 +1646,12 @@ importers: specifier: ^10.0.1 version: 10.0.1 devDependencies: + '@objectstack/driver-sql': + specifier: workspace:* + version: link:../../drivers/driver-sql + '@objectstack/objectql': + specifier: workspace:* + version: link:../../objectql '@types/node': specifier: ^26.1.2 version: 26.1.2