Skip to content

Commit dea17d5

Browse files
claudeclaude
committed
Merge remote-tracking branch 'origin/main' into claude/project-thread-1qb5el
2 parents 9522958 + ce18e73 commit dea17d5

4 files changed

Lines changed: 207 additions & 10 deletions

File tree

‎.changeset/25544-widget-probe-access-refusal-fix.md‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
fix(metadata-protocol): a publish build probe refused by read admission tells the publisher to get read access, not to change a correct dataset
6+
7+
Clause-②: no
8+
Metadata-protocol: none
9+
10+
The dashboard-widget probe runs the widget's dataset query as the publishing caller. A publisher who may not read the dataset's object got a `widget_query_failed` issue whose `fix` said to fix the dataset definition so the query compiles. The dataset was correct; the cause was the publisher's read access.
11+
12+
- **An access refusal names the access.** When the analytics service refuses the query with `PERMISSION_DENIED` (an object or a field the caller may not read), the `fix` now says the dataset is not the cause, and that the publisher (or the role the dashboard is for) needs read access to the refused object, and the refused fields when the refusal names them. The refusal is told apart by the error's code, not its message.
13+
- **Every other failure keeps the dataset fix.** A selection the dataset does not declare, and a member the field-level gate cannot judge (also `PERMISSION_DENIED`, but its remedy is a dataset change), keep "Fix the dataset definition (or the widget's values/dimensions) so the query compiles, then republish."
14+
- **Nothing else changes.** The issue's `code` (`widget_query_failed`), `severity` (`error`) and `message` are the same, and so is every other probe plane.

‎packages/metadata-protocol/src/build-probes-widget-refusal-fix.test.ts‎

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* A `widget_query_failed` issue's `fix` names the remedy for the failure that
5+
* actually happened.
6+
*
7+
* The widget plane runs `queryDataset` as the publishing caller, so read
8+
* admission can refuse a dataset that is correct. That refusal is told apart
9+
* by the thrown error's ADR-0112 `code` (`PERMISSION_DENIED`), and its `fix`
10+
* names the caller's read access — never a dataset edit. Every other throw
11+
* (a selection the dataset does not declare, an unjudgeable member) keeps the
12+
* dataset fix. `code`, `severity` and `message` are the same on both arms.
13+
*
14+
* The errors below are the analytics service's envelopes field for field
15+
* (`read-admission.ts`, `field-read-admission.ts`, `dataset-refusal.ts`); the
16+
* same leg through a real `AnalyticsService` is pinned in `@objectstack/runtime`
17+
* (`packages-publish-drafts-probe-caller-context.test.ts`).
18+
*/
19+
20+
import { describe, expect, it } from 'vitest';
21+
import { runBuildProbes, type ProbeEngine } from './build-probes.js';
22+
23+
const ITEMS: Record<string, unknown> = {
24+
'dashboard board': { name: 'board', widgets: [{ id: 'w1', dataset: 'ds', values: ['total'] }] },
25+
'dataset ds': { name: 'ds', object: 'task_item', measures: [{ name: 'total', aggregate: 'count' }] },
26+
};
27+
const getItem = async (type: string, name: string) => ITEMS[`${type} ${name}`];
28+
const engine: ProbeEngine = { find: async () => [{ id: 'r1' }] };
29+
30+
const DATASET_FIX = `Fix the dataset definition (or the widget's values/dimensions) so the query compiles, then republish.`;
31+
32+
function envelope(message: string, fields: Record<string, unknown>): Error {
33+
return Object.assign(new Error(message), fields);
34+
}
35+
36+
async function probeThrowing(err: Error) {
37+
const report = await runBuildProbes({
38+
engine,
39+
getItem,
40+
analytics: { queryDataset: async () => { throw err; } },
41+
context: { userId: 'u_publisher' },
42+
published: [{ type: 'dashboard', name: 'board' }],
43+
});
44+
expect(report.checked.widgets).toBe(1);
45+
expect(report.issues).toHaveLength(1);
46+
const issue = report.issues[0]!;
47+
// Unchanged on every arm: the code, the tier and the message the error carries.
48+
expect(issue).toMatchObject({
49+
layer: 'runtime',
50+
severity: 'error',
51+
code: 'widget_query_failed', // adr0112-ok: D6c build-probe diagnostics code
52+
artifact: { type: 'dashboard', name: 'board' },
53+
ref: { type: 'dataset', name: 'ds', member: 'w1' },
54+
});
55+
expect(issue.message).toContain(err.message);
56+
return issue;
57+
}
58+
59+
describe('widget_query_failed — the fix follows the failure', () => {
60+
it('an object read-admission refusal names the caller\'s read access, not the dataset', async () => {
61+
const issue = await probeThrowing(envelope(
62+
'[Analytics] Access denied: reading "task_item" is not permitted for this user.',
63+
{ code: 'PERMISSION_DENIED', status: 403, object: 'task_item' },
64+
));
65+
expect(issue.fix).toContain('read access to object "task_item"');
66+
expect(issue.fix).toContain('Dataset "ds" is not the cause');
67+
expect(issue.fix).not.toContain(DATASET_FIX);
68+
});
69+
70+
it('names the object the refusal names — a joined object, not the dataset\'s base', async () => {
71+
const issue = await probeThrowing(envelope(
72+
'[Analytics] Access denied: reading "project" is not permitted for this user.',
73+
{ code: 'PERMISSION_DENIED', status: 403, object: 'project' },
74+
));
75+
expect(issue.fix).toContain('read access to object "project"');
76+
expect(issue.fix).not.toContain('"task_item"');
77+
});
78+
79+
it('a field-level read refusal names the refused fields', async () => {
80+
const issue = await probeThrowing(envelope(
81+
"[Security] Field read denied: not permitted to aggregate [salary] on 'task_item'",
82+
{ code: 'PERMISSION_DENIED', status: 403, object: 'task_item', fields: ['salary'] },
83+
));
84+
expect(issue.fix).toContain('read access to object "task_item" and its field(s) salary');
85+
});
86+
87+
it('a selection the dataset does not declare keeps the dataset fix', async () => {
88+
const issue = await probeThrowing(envelope(
89+
'Dataset "ds" does not declare measure "nope".',
90+
{ code: 'DATASET_INVALID', status: 400 },
91+
));
92+
expect(issue.fix).toBe(DATASET_FIX);
93+
});
94+
95+
it('an unjudgeable member — PERMISSION_DENIED whose remedy IS the dataset — keeps the dataset fix', async () => {
96+
const issue = await probeThrowing(envelope(
97+
"[Analytics] Access denied: member 'total' on 'task_item' is not a column reference",
98+
{ code: 'PERMISSION_DENIED', status: 403, object: 'task_item', member: 'total' },
99+
));
100+
expect(issue.fix).toBe(DATASET_FIX);
101+
});
102+
103+
it('a bare Error keeps the dataset fix', async () => {
104+
const issue = await probeThrowing(new Error('RAW_SQL_UNSUPPORTED'));
105+
expect(issue.fix).toBe(DATASET_FIX);
106+
});
107+
});

‎packages/metadata-protocol/src/build-probes.ts‎

Lines changed: 46 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@
1515
// chat surfaces, and eval harnesses consume one stream regardless of which
1616
// verification plane found the problem.
1717

18+
import type { StandardErrorCode } from '@objectstack/spec/api';
19+
1820
/** One runtime-verification finding (ADR-0038 BuildIssue, layer 'runtime'). */
1921
export interface RuntimeBuildIssue {
2022
layer: 'runtime';
@@ -106,6 +108,36 @@ function asArr(v: unknown): unknown[] {
106108
return Array.isArray(v) ? v : [];
107109
}
108110

111+
/**
112+
* `PERMISSION_DENIED`, pinned against the STANDARD catalog — the code the
113+
* analytics service's read admission throws (ADR-0112 envelope, 403).
114+
*/
115+
const PERMISSION_DENIED: StandardErrorCode = 'PERMISSION_DENIED';
116+
117+
/**
118+
* The read-admission refusal a widget query can be answered with, judged by
119+
* the thrown error's ADR-0112 `code` — never by its message text.
120+
*
121+
* The analytics service refuses a caller who may not read an object (or a
122+
* field the dataset reads) with `PERMISSION_DENIED` and names the refused
123+
* object in `object` (a joined object can be the one refused, so it is read
124+
* from the error before the dataset's own) and the refused fields in
125+
* `fields`. That is the caller's access, not the dataset's definition.
126+
*
127+
* One `PERMISSION_DENIED` is NOT an access question: a member that is no
128+
* column reference, refused because the gate cannot tell which fields it
129+
* reads, carries `member`, and its remedy IS a dataset change — so it keeps
130+
* the dataset fix.
131+
*/
132+
function readAdmissionRefusal(e: unknown): { object?: string; fields: string[] } | undefined {
133+
const err = e && typeof e === 'object' ? (e as Rec) : undefined;
134+
if (!err || err.code !== PERMISSION_DENIED || typeof err.member === 'string') return undefined;
135+
return {
136+
object: typeof err.object === 'string' && err.object ? err.object : undefined,
137+
fields: asArr(err.fields).filter((f): f is string => typeof f === 'string' && f.length > 0),
138+
};
139+
}
140+
109141
/** Result rows of a queryDataset call, tolerant of {rows}/{data}/array shapes. */
110142
function resultRows(result: unknown): unknown[] {
111143
if (Array.isArray(result)) return result;
@@ -384,14 +416,27 @@ export async function runBuildProbes(opts: RunBuildProbesOptions): Promise<Build
384416
});
385417
}
386418
} catch (e) {
419+
// The query runs as the publishing caller, so read admission
420+
// can refuse a dataset that is correct. That refusal names an
421+
// access remedy; telling the author to edit the dataset would
422+
// send them to change something that works.
423+
const refusal = readAdmissionRefusal(e);
424+
const refusedObject = refusal?.object ?? objectName;
425+
const fix = refusal
426+
? `Dataset "${dsName}" is not the cause and needs no change: read admission refused the publishing caller. ` +
427+
`The publisher (or the role this dashboard is for) needs read access to ` +
428+
`${refusedObject ? `object "${refusedObject}"` : 'the object this dataset reads'}` +
429+
`${refusal.fields.length ? ` and its field(s) ${refusal.fields.join(', ')}` : ''}; grant it, then republish. ` +
430+
`The finding is about this caller's access: a viewer who has that access can read the widget.`
431+
: `Fix the dataset definition (or the widget's values/dimensions) so the query compiles, then republish.`;
387432
issues.push({
388433
layer: 'runtime',
389434
severity: 'error',
390435
artifact: { type: 'dashboard', name: p.name },
391436
ref: { type: 'dataset', name: dsName, member: widgetId },
392437
code: 'widget_query_failed',
393438
message: `Dashboard "${p.name}" widget "${widgetId}" query against dataset "${dsName}" failed: ${String((e as Error)?.message ?? e)}`,
394-
fix: `Fix the dataset definition (or the widget's values/dimensions) so the query compiles, then republish.`,
439+
fix,
395440
});
396441
}
397442
}

‎packages/runtime/src/domains/packages-publish-drafts-probe-caller-context.test.ts‎

Lines changed: 40 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121
* The two legs are each other's control: the reader's publish must come back
2222
* clean, and the non-reader's must still carry the refusal, so neither "the
2323
* probe stopped running" nor "the probe now reads as the system" can pass.
24+
* The refusal's `fix` names the caller's read access; a third leg, a query that
25+
* cannot compile, keeps the dataset fix, so the fix follows the failure rather
26+
* than the plane.
2427
*/
2528
import { afterEach, describe, expect, it, vi } from 'vitest';
2629
import { DatasetSchema } from '@objectstack/spec/ui';
@@ -41,13 +44,27 @@ const DATASET = DatasetSchema.parse({
4144
measures: [{ name: 'task_count', label: 'Tasks', aggregate: 'count' }],
4245
});
4346

44-
const ITEMS: Record<string, unknown> = {
47+
/**
48+
* The same measure over a relationship path the dataset never declares in
49+
* `include`: a dataset that cannot compile (`DATASET_INVALID`), whoever reads it.
50+
*/
51+
const DATASET_UNDECLARED_JOIN = DatasetSchema.parse({
52+
...DATASET,
53+
measures: [{ name: 'task_count', label: 'Tasks', aggregate: 'sum', field: 'project.estimate' }],
54+
});
55+
56+
const itemsWithDataset = (dataset: unknown): Record<string, unknown> => ({
4557
'dashboard task_board': {
4658
name: 'task_board',
4759
widgets: [{ id: 'w_tasks', dataset: 'task_metrics', values: ['task_count'] }],
4860
},
49-
'dataset task_metrics': DATASET,
50-
};
61+
'dataset task_metrics': dataset,
62+
});
63+
const ITEMS = itemsWithDataset(DATASET);
64+
const ITEMS_UNDECLARED_JOIN = itemsWithDataset(DATASET_UNDECLARED_JOIN);
65+
66+
/** The fix every widget query failure that is NOT an access refusal keeps. */
67+
const DATASET_FIX = `Fix the dataset definition (or the widget's values/dimensions) so the query compiles, then republish.`;
5168

5269
/** Both callers may publish; only the reader may read `task_item`. */
5370
const caller = (userId: string) => ({
@@ -60,7 +77,7 @@ const caller = (userId: string) => ({
6077
const READER = 'u_reader';
6178
const NON_READER = 'u_publisher_without_read';
6279

63-
function makeDoor() {
80+
function makeDoor(items: Record<string, unknown> = ITEMS) {
6481
const admissionContexts: Array<ExecutionContext | undefined> = [];
6582
const analytics = new AnalyticsService({
6683
queryCapabilities: () => ({ nativeSql: false, objectqlAggregate: true, inMemory: false }),
@@ -80,16 +97,16 @@ function makeDoor() {
8097
{ type: 'dashboard', name: 'task_board' },
8198
],
8299
get: async (ref: any, opts: any) =>
83-
opts?.state === 'draft' ? { body: ITEMS[`${ref.type} ${ref.name}`], hash: 'h' } : null,
100+
opts?.state === 'draft' ? { body: items[`${ref.type} ${ref.name}`], hash: 'h' } : null,
84101
});
85102
vi.spyOn(p, 'promoteDraftForPublish').mockImplementation(async (req: any) => ({
86103
singularType: req.type,
87104
orgId: null,
88105
advisories: [],
89-
result: { version: 'h', seq: 1, item: { body: ITEMS[`${req.type} ${req.name}`] }, packageId: null },
106+
result: { version: 'h', seq: 1, item: { body: items[`${req.type} ${req.name}`] }, packageId: null },
90107
}));
91108
vi.spyOn(p, 'runPublishSideEffects').mockResolvedValue({});
92-
p.getMetaItem = async ({ type, name }: any) => ({ item: ITEMS[`${type} ${name}`] });
109+
p.getMetaItem = async ({ type, name }: any) => ({ item: items[`${type} ${name}`] });
93110
p.getMetaItems = async () => ({ items: [] });
94111
const engineFinds: Array<{ object: string; context: unknown }> = [];
95112
p.engine = {
@@ -125,8 +142,8 @@ function makeDoor() {
125142
return { dispatcher: new HttpDispatcher(kernel), admissionContexts, engineFinds };
126143
}
127144

128-
async function publishAs(userId: string) {
129-
const door = makeDoor();
145+
async function publishAs(userId: string, items: Record<string, unknown> = ITEMS) {
146+
const door = makeDoor(items);
130147
const result = await door.dispatcher.handlePackages(
131148
'/com.tasks/publish-drafts', 'POST', {}, {}, caller(userId),
132149
);
@@ -166,6 +183,20 @@ describe('publish-drafts widget probes run as the publishing caller (#25484)', (
166183
artifact: { type: 'dashboard', name: 'task_board' },
167184
ref: { type: 'dataset', name: 'task_metrics', member: 'w_tasks' },
168185
});
186+
expect(failed[0].message).toContain(`reading "${GATED}" is not permitted for this user`);
187+
// The dataset is correct: the fix names the caller's read access, never a dataset edit.
188+
expect(failed[0].fix).toContain(`read access to object "${GATED}"`);
189+
expect(failed[0].fix).not.toContain(DATASET_FIX);
169190
expect(admissionContexts.every((ctx) => ctx?.userId === NON_READER)).toBe(true);
170191
});
192+
193+
it('a query that cannot compile, for a caller who CAN read, keeps the dataset fix', async () => {
194+
const { probes } = await publishAs(READER, ITEMS_UNDECLARED_JOIN);
195+
196+
expect(probes?.checked?.widgets).toBe(1);
197+
const failed = probes.issues.filter((i: any) => i.code === 'widget_query_failed'); // adr0112-ok: D6c build-probe diagnostics code
198+
expect(failed).toHaveLength(1);
199+
expect(failed[0]).toMatchObject({ layer: 'runtime', severity: 'error', fix: DATASET_FIX });
200+
expect(failed[0].message).toContain('is not declared in the dataset');
201+
});
171202
});

0 commit comments

Comments
 (0)