Skip to content

Commit 0f9ce97

Browse files
committed
Merge remote-tracking branch 'origin/main' into claude/issue-21884-fls-mask-object-override
2 parents af06da7 + 87712ab commit 0f9ce97

5 files changed

Lines changed: 616 additions & 15 deletions

File tree

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
---
2+
'@objectstack/metadata-protocol': patch
3+
---
4+
5+
Global search (`GET /api/v1/search`) skips the objects a caller cannot read instead of failing the whole request with `403 PERMISSION_DENIED` (#21836).
6+
7+
Clause-②: no
8+
9+
- **Object level.** Before an object is queried, `searchAll` asks the `security` service's `canReadObject` with the caller's context, the same read gate the engine middleware enforces. An object it refuses is skipped. A member whose scope included any object they hold no read grant on used to get 403 for every query. The console palette then showed "No results found." with no error.
10+
- **Field level.** Each object is searched only on the fields the caller may query (`getQueryableFields`). The engine refuses a search that would match on a hidden field, and `sys_user`'s searchable fields include admin-only columns, so a member's unscoped search hit that refusal as well. An object left with no queryable search field is skipped.
11+
- **Nothing about a skipped object reaches the response.** It is not queried, named or counted in `totalObjects`, and the decision is made before any row is read. An explicit `objects=` naming an unreadable object gets the same answer as a name that matches no object.
12+
- **Other failures still fail the search.** A read error on a readable object, or an admission check that throws, propagates as before. Callers that can read every object, and calls without a context, get the same answer as before.
Lines changed: 325 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,325 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* `searchAll` skips an object the caller may not READ, instead of failing the
5+
* whole search.
6+
*
7+
* The REST search door checks authentication only. Each swept object reached
8+
* `engine.find`, whose security middleware refuses an object the caller holds
9+
* no read grant on — and that refusal propagated out of the sweep, so a member
10+
* whose scope included ANY unreadable object got `403 PERMISSION_DENIED` for
11+
* the whole request, whatever the query (the console palette then showed
12+
* "No results found.").
13+
*
14+
* The sweep now asks the `security` service's `canReadObject` — the
15+
* middleware's own read gate — before it queries an object, and skips one it
16+
* refuses. What these pins hold:
17+
*
18+
* - a mixed scope answers the readable objects' hits, and the unreadable
19+
* object is never queried, never named, never counted;
20+
* - an explicit `objects=` naming an unreadable object answers exactly as
21+
* one naming an object that does not exist;
22+
* - nothing about the skipped object's rows can shape the answer — it is
23+
* identical whether or not its rows would have matched;
24+
* - an all-access caller's answer is unchanged;
25+
* - every other failure still fails the request: a read error on a readable
26+
* object, and an admission check that itself throws;
27+
* - a readable object is searched only on the fields the caller may QUERY
28+
* (`getQueryableFields`), and one left with none is skipped — the engine's
29+
* predicate guard refuses a search over a hidden field with the same 403;
30+
* - no context means no pre-filter (the reads pose no principal).
31+
*
32+
* The engine double here stands in for the middleware: it THROWS the typed
33+
* denial for an object the fake security service refuses, so a sweep that
34+
* stopped consulting `canReadObject` turns these pins red with that 403.
35+
*/
36+
37+
import { describe, it, expect, vi } from 'vitest';
38+
import { ObjectStackProtocolImplementation } from './protocol.js';
39+
import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core';
40+
41+
interface FixtureObject {
42+
name: string;
43+
label: string;
44+
fields: Record<string, { name: string; label: string; type: string }>;
45+
searchableFields?: string[];
46+
}
47+
48+
const objectFixture = (name: string): FixtureObject => ({
49+
name,
50+
label: name,
51+
fields: { name: { name: 'name', label: 'Name', type: 'text' } },
52+
});
53+
54+
function fixtureRegistry(objects: FixtureObject[]) {
55+
return {
56+
getObject: (n: string) => objects.find((o) => o.name === n),
57+
getAllObjects: () => objects,
58+
getItem: () => undefined,
59+
listItems: () => [],
60+
applyNavContributions: (x: unknown) => x,
61+
isPackageDisabled: () => false,
62+
getObjectOwner: () => undefined,
63+
getPackage: () => undefined,
64+
};
65+
}
66+
67+
/** The engine middleware's typed object-level denial, as `PermissionDeniedError` carries it. */
68+
const objectReadDenied = () =>
69+
Object.assign(new Error('You do not have permission to perform this action.'), {
70+
name: 'PermissionDeniedError',
71+
code: 'PERMISSION_DENIED',
72+
status: 403,
73+
statusCode: 403,
74+
});
75+
76+
const acct = objectFixture('acct');
77+
const lead = objectFixture('lead');
78+
const secret = objectFixture('secret_ledger');
79+
80+
const ROWS: Record<string, Array<Record<string, unknown>>> = {
81+
acct: [{ id: 'a1', name: 'Acme' }],
82+
lead: [{ id: 'l1', name: 'Acme Lead' }],
83+
secret_ledger: [{ id: 's1', name: 'Acme Secret' }],
84+
};
85+
86+
const MEMBER = { userId: 'usr_member', tenantId: 'org_1' };
87+
88+
/**
89+
* An engine whose `find` enforces `readable` the way the middleware does
90+
* (typed throw for anything else), and a security service answering the same
91+
* set. `rows` lets a case vary what the unreadable object WOULD hold.
92+
*/
93+
function harness(opts: {
94+
readable: Set<string> | 'all';
95+
withSecurity?: boolean;
96+
rows?: Record<string, Array<Record<string, unknown>>>;
97+
canReadObject?: (object: string, context?: unknown) => Promise<boolean>;
98+
getQueryableFields?: (object: string, context?: unknown) => Promise<string[] | undefined>;
99+
objects?: FixtureObject[];
100+
failRead?: { object: string; error: unknown };
101+
}) {
102+
const rows = opts.rows ?? ROWS;
103+
const readCalls: string[] = [];
104+
const findOptions: Array<[string, Record<string, unknown>]> = [];
105+
const admits = (o: string) => opts.readable === 'all' || opts.readable.has(o);
106+
const engine = {
107+
registry: fixtureRegistry(opts.objects ?? [acct, lead, secret]),
108+
find: vi.fn(async (object: string, options: Record<string, unknown>) => {
109+
if (object === 'sys_metadata') return [];
110+
readCalls.push(object);
111+
findOptions.push([object, options]);
112+
if (!admits(object)) throw objectReadDenied();
113+
if (opts.failRead && opts.failRead.object === object) throw opts.failRead.error;
114+
return rows[object] ?? [];
115+
}),
116+
findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => {
117+
assertEngineFindOnePredicate(object, query);
118+
return null;
119+
}),
120+
};
121+
const canReadObject = vi.fn(opts.canReadObject ?? (async (o: string) => admits(o)));
122+
const services = new Map<string, unknown>();
123+
const security: Record<string, unknown> = { canReadObject };
124+
if (opts.getQueryableFields) security.getQueryableFields = vi.fn(opts.getQueryableFields);
125+
if (opts.withSecurity !== false) services.set('security', security);
126+
const protocol = new ObjectStackProtocolImplementation(engine as never, () => services as Map<string, any>);
127+
return { protocol, readCalls, findOptions, canReadObject };
128+
}
129+
130+
async function rejection(run: () => Promise<unknown>): Promise<Record<string, unknown>> {
131+
let caught: unknown;
132+
let didResolve = false;
133+
try {
134+
await run();
135+
didResolve = true;
136+
} catch (e) {
137+
caught = e;
138+
}
139+
expect(didResolve, 'expected a rejection, but the call resolved').toBe(false);
140+
return caught as Record<string, unknown>;
141+
}
142+
143+
describe('searchAll — an object the caller may not read is skipped, not fatal', () => {
144+
it('control: without the pre-filter the middleware denial fails the whole search with 403', async () => {
145+
// The defect, reproduced on this harness: no security service to ask,
146+
// so the sweep reaches `find` on `lead` and the typed denial escapes.
147+
const { protocol } = harness({ readable: new Set(['acct']), withSecurity: false });
148+
const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER }));
149+
expect(caught.code).toBe('PERMISSION_DENIED');
150+
expect(caught.status).toBe(403);
151+
});
152+
153+
it('a mixed scope answers the readable objects and never queries, names or counts the rest', async () => {
154+
const { protocol, readCalls, canReadObject } = harness({ readable: new Set(['acct']) });
155+
156+
const result = await protocol.searchAll({ q: 'Acme', context: MEMBER });
157+
158+
expect(result.hits.map((h) => [h.object, h.id])).toEqual([['acct', 'a1']]);
159+
expect(result.totalObjects).toBe(1);
160+
expect(result.totalHits).toBe(1);
161+
expect(result.truncated).toBe(false);
162+
// Never queried — so nothing about its rows can reach the answer.
163+
expect(readCalls).toEqual(['acct']);
164+
// Asked with the caller's own context, for every swept object.
165+
expect(canReadObject.mock.calls).toEqual([
166+
['acct', MEMBER],
167+
['lead', MEMBER],
168+
['secret_ledger', MEMBER],
169+
]);
170+
// Never named, anywhere in the response.
171+
const wire = JSON.stringify(result);
172+
expect(wire).not.toContain('lead');
173+
expect(wire).not.toContain('secret_ledger');
174+
expect(Object.keys(result).sort()).toEqual(
175+
['hits', 'pages', 'query', 'totalHits', 'totalObjects', 'truncated'],
176+
);
177+
});
178+
179+
it('the answer does not depend on what a skipped object holds', async () => {
180+
const matching = harness({ readable: new Set(['acct']) });
181+
const empty = harness({
182+
readable: new Set(['acct']),
183+
rows: { acct: ROWS.acct, lead: [], secret_ledger: [] },
184+
});
185+
186+
const a = await matching.protocol.searchAll({ q: 'Acme', context: MEMBER });
187+
const b = await empty.protocol.searchAll({ q: 'Acme', context: MEMBER });
188+
189+
expect(a).toEqual(b);
190+
});
191+
192+
it('an explicit objects= naming an unreadable object answers like one naming no such object', async () => {
193+
const { protocol, readCalls } = harness({ readable: new Set(['acct']) });
194+
195+
const mixed = await protocol.searchAll({ q: 'Acme', objects: ['acct', 'secret_ledger'], context: MEMBER });
196+
expect(mixed.hits.map((h) => h.object)).toEqual(['acct']);
197+
expect(mixed.totalObjects).toBe(1);
198+
expect(JSON.stringify(mixed)).not.toContain('secret_ledger');
199+
200+
const unreadable = await protocol.searchAll({ q: 'Acme', objects: ['secret_ledger'], context: MEMBER });
201+
const nonexistent = await protocol.searchAll({ q: 'Acme', objects: ['no_such_object'], context: MEMBER });
202+
expect(unreadable).toEqual(nonexistent);
203+
expect(unreadable).toEqual({
204+
query: 'Acme', hits: [], pages: [], totalObjects: 0, totalHits: 0, truncated: false,
205+
});
206+
expect(readCalls).toEqual(['acct']);
207+
});
208+
209+
it('an all-access caller gets exactly what the sweep answered without a security service', async () => {
210+
const admin = harness({ readable: 'all' });
211+
const bare = harness({ readable: 'all', withSecurity: false });
212+
213+
const withService = await admin.protocol.searchAll({ q: 'Acme', context: { userId: 'usr_admin' } });
214+
const without = await bare.protocol.searchAll({ q: 'Acme', context: { userId: 'usr_admin' } });
215+
216+
expect(withService).toEqual(without);
217+
expect(withService.hits.map((h) => h.object)).toEqual(['acct', 'lead', 'secret_ledger']);
218+
expect(withService.totalObjects).toBe(3);
219+
expect(admin.readCalls).toEqual(['acct', 'lead', 'secret_ledger']);
220+
});
221+
222+
it('a read failure on a READABLE object still fails the search, envelope intact', async () => {
223+
const injected = Object.assign(new Error('connection terminated unexpectedly'), { code: 'ECONNRESET' });
224+
const { protocol } = harness({
225+
readable: new Set(['acct', 'lead']),
226+
failRead: { object: 'lead', error: injected },
227+
});
228+
229+
const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER }));
230+
expect(caught).toBe(injected);
231+
});
232+
233+
it('an admission check that THROWS fails the search instead of shrinking it', async () => {
234+
const injected = Object.assign(new Error('permission store unreachable'), { code: 'ECONNREFUSED' });
235+
const { protocol, readCalls } = harness({
236+
readable: new Set(['acct']),
237+
canReadObject: async (o) => {
238+
if (o === 'lead') throw injected;
239+
return o === 'acct';
240+
},
241+
});
242+
243+
const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER }));
244+
expect(caught).toBe(injected);
245+
expect(readCalls).not.toContain('lead');
246+
});
247+
248+
it('a context-less call is not pre-filtered: its reads carry no principal to ask about', async () => {
249+
const { protocol, canReadObject } = harness({ readable: 'all' });
250+
251+
const result = await protocol.searchAll({ q: 'Acme' });
252+
253+
expect(canReadObject).not.toHaveBeenCalled();
254+
expect(result.totalObjects).toBe(3);
255+
});
256+
});
257+
258+
describe('searchAll — a readable object is searched only on the fields the caller may query', () => {
259+
const memo: FixtureObject = {
260+
name: 'memo',
261+
label: 'memo',
262+
fields: {
263+
name: { name: 'name', label: 'Name', type: 'text' },
264+
hidden_note: { name: 'hidden_note', label: 'Hidden note', type: 'text' },
265+
},
266+
searchableFields: ['name', 'hidden_note'],
267+
};
268+
const rows = { memo: [{ id: 'm1', name: 'Acme memo' }] };
269+
270+
it('a partial queryable set is handed to the engine as searchFields', async () => {
271+
const { protocol, findOptions } = harness({
272+
readable: 'all', objects: [memo], rows,
273+
getQueryableFields: async () => ['id', 'name'],
274+
});
275+
276+
const result = await protocol.searchAll({ q: 'Acme', context: MEMBER });
277+
278+
expect(findOptions).toHaveLength(1);
279+
expect(findOptions[0][1].searchFields).toEqual(['name']);
280+
expect(findOptions[0][1].search).toBe('Acme');
281+
expect(result.hits.map((h) => [h.object, h.id])).toEqual([['memo', 'm1']]);
282+
expect(result.totalObjects).toBe(1);
283+
});
284+
285+
it('an object with NO queryable search field is skipped, unqueried and uncounted', async () => {
286+
const { protocol, readCalls } = harness({
287+
readable: 'all', objects: [acct, memo], rows: { ...ROWS, ...rows },
288+
getQueryableFields: async (o) => (o === 'memo' ? ['id'] : ['id', 'name']),
289+
});
290+
291+
const result = await protocol.searchAll({ q: 'Acme', context: MEMBER });
292+
293+
expect(readCalls).toEqual(['acct']);
294+
expect(result.totalObjects).toBe(1);
295+
expect(JSON.stringify(result)).not.toContain('memo');
296+
});
297+
298+
it('a full queryable set, or no answer, leaves the request exactly as before (no searchFields)', async () => {
299+
const full = harness({
300+
readable: 'all', objects: [memo], rows,
301+
getQueryableFields: async () => ['id', 'name', 'hidden_note'],
302+
});
303+
await full.protocol.searchAll({ q: 'Acme', context: MEMBER });
304+
expect('searchFields' in full.findOptions[0][1]).toBe(false);
305+
306+
const noAnswer = harness({
307+
readable: 'all', objects: [memo], rows,
308+
getQueryableFields: async () => undefined,
309+
});
310+
await noAnswer.protocol.searchAll({ q: 'Acme', context: MEMBER });
311+
expect('searchFields' in noAnswer.findOptions[0][1]).toBe(false);
312+
});
313+
314+
it('a queryable-fields check that THROWS fails the search', async () => {
315+
const injected = new Error('field resolution unavailable');
316+
const { protocol, readCalls } = harness({
317+
readable: 'all', objects: [memo], rows,
318+
getQueryableFields: async () => { throw injected; },
319+
});
320+
321+
const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER }));
322+
expect(caught).toBe(injected);
323+
expect(readCalls).toEqual([]);
324+
});
325+
});

0 commit comments

Comments
 (0)