diff --git a/.changeset/25484-build-probe-caller-context.md b/.changeset/25484-build-probe-caller-context.md new file mode 100644 index 00000000000..b1dbe9fadfc --- /dev/null +++ b/.changeset/25484-build-probe-caller-context.md @@ -0,0 +1,16 @@ +--- +'@objectstack/metadata-protocol': patch +'@objectstack/runtime': patch +--- + +fix: `POST /api/v1/packages/:id/publish-drafts` probes dashboard widgets as the publishing caller, so a correct dataset over a permission-gated object is no longer reported as an error-tier `widget_query_failed` + +Clause-②: no +Metadata-protocol: none + +The build probes that run after a package publish called the analytics service's `queryDataset` with no execution context. Read admission refuses a read with no context, so every dashboard widget bound to a dataset over a permission-gated object came back as `widget_query_failed` ("Access denied: reading … is not permitted for this user"), with a fix telling the author to change a dataset that renders correctly. + +- **`@objectstack/runtime`:** the publish-drafts route now passes the caller's execution context to `publishPackageDrafts` on every request. Before, it passed one only when the request named a `backfill`. +- **`@objectstack/metadata-protocol`:** `publishPackageDrafts` passes that context to `runBuildProbes`, which takes a new optional `context`. The widget probe runs `queryDataset` as that caller, the same path the dashboard renderer takes. "Empty although the object has rows" is now judged over the rows that caller can see. + +The probe never falls back to a system context. A publisher who cannot read the dataset still gets the error-tier `widget_query_failed`, because that widget will not render for them either. diff --git a/packages/metadata-protocol/src/build-probes-caller-context.test.ts b/packages/metadata-protocol/src/build-probes-caller-context.test.ts new file mode 100644 index 00000000000..66a95e7e172 --- /dev/null +++ b/packages/metadata-protocol/src/build-probes-caller-context.test.ts @@ -0,0 +1,69 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#25484] The dashboard-widget plane reads as the caller it is handed. + * + * `queryDataset` receives the caller's context (the renderer's path), and the + * "empty although the object has rows" comparison counts the rows THAT caller + * can see. A system count beside a caller-scoped query would call a row-scoped + * empty answer broken. With no context the plane is unchanged: a context-less + * query, and a system count. + * + * The door-to-admission leg is pinned in `@objectstack/runtime` + * (`packages-publish-drafts-probe-caller-context.test.ts`). + */ + +import { describe, expect, it, vi } from 'vitest'; +import { runBuildProbes, type ProbeEngine } from './build-probes.js'; + +const ITEMS: Record = { + 'dashboard board': { name: 'board', widgets: [{ id: 'w1', dataset: 'ds', values: ['total'] }] }, + 'dataset ds': { name: 'ds', object: 'task_item', measures: [{ name: 'total', aggregate: 'count' }] }, +}; +const getItem = async (type: string, name: string) => ITEMS[`${type} ${name}`]; + +const CALLER = { userId: 'u_row_scoped' }; + +/** Rows exist for the system; the row-scoped caller sees none. Honours `limit`. */ +function engineVisibleToSystemOnly(): ProbeEngine & { contexts: unknown[] } { + const contexts: unknown[] = []; + return { + contexts, + find: async (_object: string, query: unknown) => { + const q = query as { limit?: unknown; context?: { isSystem?: boolean } } | undefined; + contexts.push(q?.context); + const rows = q?.context?.isSystem ? [{ id: 'r1' }, { id: 'r2' }] : []; + return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows; + }, + }; +} + +describe('runBuildProbes — the widget plane reads as the caller (#25484)', () => { + it('hands queryDataset the caller context and counts the rows that caller sees', async () => { + const engine = engineVisibleToSystemOnly(); + const analytics = { queryDataset: vi.fn(async (..._args: unknown[]) => ({ rows: [] })) }; + const report = await runBuildProbes({ + engine, getItem, analytics, context: CALLER, + published: [{ type: 'dashboard', name: 'board' }], + }); + + expect(analytics.queryDataset).toHaveBeenCalledOnce(); + expect(analytics.queryDataset.mock.calls[0]![2]).toBe(CALLER); + expect(engine.contexts).toEqual([CALLER]); + expect(report.checked.widgets).toBe(1); + expect(report.issues).toEqual([]); + }); + + it('with no context: a context-less query and a system count, as before', async () => { + const engine = engineVisibleToSystemOnly(); + const analytics = { queryDataset: vi.fn(async (..._args: unknown[]) => ({ rows: [] })) }; + const report = await runBuildProbes({ + engine, getItem, analytics, + published: [{ type: 'dashboard', name: 'board' }], + }); + + expect(analytics.queryDataset.mock.calls[0]![2]).toBeUndefined(); + expect(engine.contexts).toEqual([{ isSystem: true }]); + expect(report.issues.map((i) => [i.code, i.severity])).toEqual([['empty_query', 'error']]); + }); +}); diff --git a/packages/metadata-protocol/src/build-probes.ts b/packages/metadata-protocol/src/build-probes.ts index 287ab83e987..d23a96693a7 100644 --- a/packages/metadata-protocol/src/build-probes.ts +++ b/packages/metadata-protocol/src/build-probes.ts @@ -81,6 +81,19 @@ export interface RunBuildProbesOptions { analytics?: ProbeAnalytics; /** Threaded into engine/analytics reads (tenant scoping). */ organizationId?: string | null; + /** + * The PUBLISHING caller's execution context. Widget probes run + * `queryDataset` as this caller, because that is what the renderer does: + * a widget is healthy when the person who sees it can read it, so the + * probe asks read admission the question the dashboard will ask. + * + * ⛔ Never substitute a system context when it is absent. A probe that + * reads as the system reports a widget healthy that its own publisher + * cannot open, and that is the false clean this plane exists to prevent. + * Absent, the read goes out context-less and read admission answers for + * it — a refusal, reported as such. + */ + context?: unknown; } type Rec = Record; @@ -103,17 +116,22 @@ function resultRows(result: unknown): unknown[] { return []; } -/** True when the object has at least one row (probe read, isSystem). */ +/** + * True when the object has at least one row. A system read by default; the + * widget plane passes the caller's context so "the object has rows" is judged + * over the rows that caller can see — the same rows its dataset query saw. + */ async function hasRows( engine: ProbeEngine, objectName: string, organizationId?: string | null, + readContext: unknown = { isSystem: true }, ): Promise { const rows = await engine.find(objectName, { fields: ['id'], limit: 1, ...(organizationId ? { where: { organization_id: organizationId } } : {}), - context: { isSystem: true }, + context: readContext, }); return Array.isArray(rows) && rows.length > 0; } @@ -139,7 +157,7 @@ async function hasRows( export async function runBuildProbes(opts: RunBuildProbesOptions): Promise { const issues: RuntimeBuildIssue[] = []; const checked = { seeds: 0, views: 0, widgets: 0, objects: 0 }; - const { engine, getItem, published, analytics, organizationId } = opts; + const { engine, getItem, published, analytics, organizationId, context } = opts; // Memoized active-item reads (a dashboard and its widgets share datasets). const itemCache = new Map(); @@ -340,9 +358,21 @@ export async function runBuildProbes(opts: RunBuildProbesOptions): Promise = { + 'dashboard task_board': { + name: 'task_board', + widgets: [{ id: 'w_tasks', dataset: 'task_metrics', values: ['task_count'] }], + }, + 'dataset task_metrics': DATASET, +}; + +/** Both callers may publish; only the reader may read `task_item`. */ +const caller = (userId: string) => ({ + request: {}, + executionContext: { + userId, + systemPermissions: ['manage_metadata', 'studio.access', 'setup.access'], + }, +}) as any; +const READER = 'u_reader'; +const NON_READER = 'u_publisher_without_read'; + +function makeDoor() { + const admissionContexts: Array = []; + const analytics = new AnalyticsService({ + queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }), + admitObjectRead: (object, context) => { + admissionContexts.push(context); + return object !== GATED || context?.userId === READER; + }, + executeAggregate: async () => [{ task_count: 7 }], + }); + + const protocol = new ObjectStackProtocolImplementation({} as never); + const p = protocol as any; + p.ensureOverlayIndex = async () => {}; + p.getOverlayRepo = () => ({ + listDrafts: async () => [ + { type: 'dataset', name: 'task_metrics' }, + { type: 'dashboard', name: 'task_board' }, + ], + get: async (ref: any, opts: any) => + opts?.state === 'draft' ? { body: ITEMS[`${ref.type} ${ref.name}`], hash: 'h' } : null, + }); + vi.spyOn(p, 'promoteDraftForPublish').mockImplementation(async (req: any) => ({ + singularType: req.type, + orgId: null, + advisories: [], + result: { version: 'h', seq: 1, item: { body: ITEMS[`${req.type} ${req.name}`] }, packageId: null }, + })); + vi.spyOn(p, 'runPublishSideEffects').mockResolvedValue({}); + p.getMetaItem = async ({ type, name }: any) => ({ item: ITEMS[`${type} ${name}`] }); + p.getMetaItems = async () => ({ items: [] }); + const engineFinds: Array<{ object: string; context: unknown }> = []; + p.engine = { + find: async (object: string, query: any) => { + engineFinds.push({ object, context: query?.context }); + const rows = [{ id: 'r1' }, { id: 'r2' }]; + return typeof query?.limit === 'number' ? rows.slice(0, query.limit) : rows; + }, + // The ADR-0067 pre-publish capture: never published, so no active row. + findOne: async (object: string, query: unknown) => { + assertEngineFindOnePredicate(object, query as never); + return null; + }, + // The ADR-0067 commit row the batch writes; not under test here. + insert: async (_object: string, row: unknown) => row, + }; + p.getServicesRegistry = () => new Map([['analytics', analytics]]); + + const kernel: any = { + getService: (name: string) => { + if (name === 'protocol') return Promise.resolve(protocol); + if (name === 'objectql') { + return Promise.resolve({ + insert: vi.fn(), find: vi.fn(), update: vi.fn(), + registry: { getAllPackages: vi.fn().mockReturnValue([]) }, + }); + } + if (name === 'metadata') return Promise.resolve({ getObject: vi.fn() }); + return null; + }, + context: { getService: () => null, trigger: vi.fn() }, + }; + return { dispatcher: new HttpDispatcher(kernel), admissionContexts, engineFinds }; +} + +async function publishAs(userId: string) { + const door = makeDoor(); + const result = await door.dispatcher.handlePackages( + '/com.tasks/publish-drafts', 'POST', {}, {}, caller(userId), + ); + expect(result.response?.status).toBe(200); + const data: any = (result.response as any)?.body?.data; + expect(data?.outcome).toBe('published'); + return { ...door, probes: data?.probes }; +} + +afterEach(() => vi.restoreAllMocks()); + +describe('publish-drafts widget probes run as the publishing caller (#25484)', () => { + it('a correct dataset over a gated object the caller CAN read yields no error-tier issue', async () => { + const { probes, admissionContexts, engineFinds } = await publishAs(READER); + + expect(probes?.checked?.widgets).toBe(1); + expect(probes.issues.filter((i: any) => i.severity === 'error')).toEqual([]); + // Admission was asked about the caller, never about a system principal. + expect(admissionContexts.length).toBeGreaterThan(0); + for (const ctx of admissionContexts) { + expect(ctx?.userId).toBe(READER); + expect((ctx as any)?.isSystem).toBeUndefined(); + } + // No system read was used to judge this widget either. + expect(engineFinds.filter((f) => (f.context as any)?.isSystem)).toEqual([]); + }); + + it('a caller who CANNOT read the dataset still gets the error-tier widget_query_failed', async () => { + const { probes, admissionContexts } = await publishAs(NON_READER); + + expect(probes?.checked?.widgets).toBe(1); + const failed = probes.issues.filter((i: any) => i.code === 'widget_query_failed'); // adr0112-ok: D6c build-probe diagnostics code + expect(failed).toHaveLength(1); + expect(failed[0]).toMatchObject({ + layer: 'runtime', + severity: 'error', + artifact: { type: 'dashboard', name: 'task_board' }, + ref: { type: 'dataset', name: 'task_metrics', member: 'w_tasks' }, + }); + expect(admissionContexts.every((ctx) => ctx?.userId === NON_READER)).toBe(true); + }); +}); diff --git a/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts b/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts index 49f135eab6c..c02c2c8bff4 100644 --- a/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts +++ b/packages/runtime/src/domains/packages-publish-drafts-response-conformance.test.ts @@ -312,12 +312,13 @@ describe('publish-drafts backfill crosses the door (#24253)', () => { expect(PublishPackageDraftsResponseSchema.parse(data).backfillApplied).toEqual(stopped); }); - it('without the key, the protocol is sent neither a backfill nor a context', async () => { + it('without the key, the protocol is sent no backfill but still THIS caller\'s context (#25484: the build probes read as it)', async () => { const { publishPackageDrafts } = await publishDrafts({ apps: [] }); const sent = publishPackageDrafts.mock.calls[0]![0]; expect('backfill' in sent).toBe(false); - expect('context' in sent).toBe(false); + expect(sent.context).toEqual(PKG_ADMIN().executionContext); + expect(sent.context?.isSystem).toBeUndefined(); }); it('the protocol\'s request refusal answers 400 VALIDATION_ERROR with its issues, nothing flipped or announced', async () => { diff --git a/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts b/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts index bdd4241b2ab..c7ae6e9c546 100644 --- a/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts +++ b/packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts @@ -390,8 +390,12 @@ describe('#15068 · 0 · the publish-then-read path really runs', () => { const { publishRequest } = await publishThenRead({ activeOrganizationId: ORG }); // The door threads no organization into `publishPackageDrafts` (nor - // into `applyPublishedSeeds`), even with one active. - expect(publishRequest()).toEqual({ packageId: PKG }); + // into `applyPublishedSeeds`), even with one active. [#25484] The + // caller's execution context does ride along, verbatim: the build + // probes read a widget's dataset AS that caller (the renderer's path), + // and nothing in the publish derives a write scope from it. + const caller = PKG_ADMIN(ORG).executionContext; + expect(publishRequest()).toEqual({ packageId: PKG, context: caller }); expect(publishRequest()).not.toHaveProperty('organizationId'); }); }); diff --git a/packages/runtime/src/domains/packages.ts b/packages/runtime/src/domains/packages.ts index b9d582c1f5a..5234f93e94c 100644 --- a/packages/runtime/src/domains/packages.ts +++ b/packages/runtime/src/domains/packages.ts @@ -1654,11 +1654,15 @@ export async function handlePackagesRequest(deps: DomainHandlerDeps, path: strin ...(body?.actor ? { actor: body.actor } : {}), // [#24253] Passed through as sent: the protocol judges // its shape and what it names, and refuses the whole - // request before anything is published. The writes run - // as THIS caller, so the request's own execution - // context rides with it, never a system one. - ...(body?.backfill !== undefined - ? { backfill: body.backfill, context: _context?.executionContext } + // request before anything is published. + ...(body?.backfill !== undefined ? { backfill: body.backfill } : {}), + // THIS caller's execution context, never a system one: + // the backfill writes run as it, and [#25484] so do the + // build probes' dashboard-widget reads — the renderer's + // path, so a widget the caller can read is not reported + // broken and one the caller cannot read still is. + ...(_context?.executionContext !== undefined + ? { context: _context.executionContext } : {}), }); // Publishing a `seed` draft is what actually loads its diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index d589a3930b3..718f05270d8 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -4241,6 +4241,11 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/runtime/src/domains/packages-publish-drafts-probe-caller-context.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/runtime/src/domains/packages-seed-apply-org-scope.test.ts", "verb": "delete",