diff --git a/.changeset/21836-search-skip-unreadable.md b/.changeset/21836-search-skip-unreadable.md new file mode 100644 index 00000000000..2710331ce75 --- /dev/null +++ b/.changeset/21836-search-skip-unreadable.md @@ -0,0 +1,12 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +Global search (`GET /api/v1/search`) skips the objects a caller cannot read instead of failing the whole request with `403 PERMISSION_DENIED` (#21836). + +Clause-②: no + +- **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. +- **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. +- **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. +- **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. diff --git a/packages/metadata-protocol/src/protocol.search-skip-unreadable.test.ts b/packages/metadata-protocol/src/protocol.search-skip-unreadable.test.ts new file mode 100644 index 00000000000..0dc3375465f --- /dev/null +++ b/packages/metadata-protocol/src/protocol.search-skip-unreadable.test.ts @@ -0,0 +1,325 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `searchAll` skips an object the caller may not READ, instead of failing the + * whole search. + * + * The REST search door checks authentication only. Each swept object reached + * `engine.find`, whose security middleware refuses an object the caller holds + * no read grant on — and that refusal propagated out of the sweep, so a member + * whose scope included ANY unreadable object got `403 PERMISSION_DENIED` for + * the whole request, whatever the query (the console palette then showed + * "No results found."). + * + * The sweep now asks the `security` service's `canReadObject` — the + * middleware's own read gate — before it queries an object, and skips one it + * refuses. What these pins hold: + * + * - a mixed scope answers the readable objects' hits, and the unreadable + * object is never queried, never named, never counted; + * - an explicit `objects=` naming an unreadable object answers exactly as + * one naming an object that does not exist; + * - nothing about the skipped object's rows can shape the answer — it is + * identical whether or not its rows would have matched; + * - an all-access caller's answer is unchanged; + * - every other failure still fails the request: a read error on a readable + * object, and an admission check that itself throws; + * - a readable object is searched only on the fields the caller may QUERY + * (`getQueryableFields`), and one left with none is skipped — the engine's + * predicate guard refuses a search over a hidden field with the same 403; + * - no context means no pre-filter (the reads pose no principal). + * + * The engine double here stands in for the middleware: it THROWS the typed + * denial for an object the fake security service refuses, so a sweep that + * stopped consulting `canReadObject` turns these pins red with that 403. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectStackProtocolImplementation } from './protocol.js'; +import { assertEngineFindOnePredicate, type EngineFindOneQueryInput } from '@objectstack/metadata-core'; + +interface FixtureObject { + name: string; + label: string; + fields: Record; + searchableFields?: string[]; +} + +const objectFixture = (name: string): FixtureObject => ({ + name, + label: name, + fields: { name: { name: 'name', label: 'Name', type: 'text' } }, +}); + +function fixtureRegistry(objects: FixtureObject[]) { + return { + getObject: (n: string) => objects.find((o) => o.name === n), + getAllObjects: () => objects, + getItem: () => undefined, + listItems: () => [], + applyNavContributions: (x: unknown) => x, + isPackageDisabled: () => false, + getObjectOwner: () => undefined, + getPackage: () => undefined, + }; +} + +/** The engine middleware's typed object-level denial, as `PermissionDeniedError` carries it. */ +const objectReadDenied = () => + Object.assign(new Error('You do not have permission to perform this action.'), { + name: 'PermissionDeniedError', + code: 'PERMISSION_DENIED', + status: 403, + statusCode: 403, + }); + +const acct = objectFixture('acct'); +const lead = objectFixture('lead'); +const secret = objectFixture('secret_ledger'); + +const ROWS: Record>> = { + acct: [{ id: 'a1', name: 'Acme' }], + lead: [{ id: 'l1', name: 'Acme Lead' }], + secret_ledger: [{ id: 's1', name: 'Acme Secret' }], +}; + +const MEMBER = { userId: 'usr_member', tenantId: 'org_1' }; + +/** + * An engine whose `find` enforces `readable` the way the middleware does + * (typed throw for anything else), and a security service answering the same + * set. `rows` lets a case vary what the unreadable object WOULD hold. + */ +function harness(opts: { + readable: Set | 'all'; + withSecurity?: boolean; + rows?: Record>>; + canReadObject?: (object: string, context?: unknown) => Promise; + getQueryableFields?: (object: string, context?: unknown) => Promise; + objects?: FixtureObject[]; + failRead?: { object: string; error: unknown }; +}) { + const rows = opts.rows ?? ROWS; + const readCalls: string[] = []; + const findOptions: Array<[string, Record]> = []; + const admits = (o: string) => opts.readable === 'all' || opts.readable.has(o); + const engine = { + registry: fixtureRegistry(opts.objects ?? [acct, lead, secret]), + find: vi.fn(async (object: string, options: Record) => { + if (object === 'sys_metadata') return []; + readCalls.push(object); + findOptions.push([object, options]); + if (!admits(object)) throw objectReadDenied(); + if (opts.failRead && opts.failRead.object === object) throw opts.failRead.error; + return rows[object] ?? []; + }), + findOne: vi.fn(async (object: string, query?: EngineFindOneQueryInput) => { + assertEngineFindOnePredicate(object, query); + return null; + }), + }; + const canReadObject = vi.fn(opts.canReadObject ?? (async (o: string) => admits(o))); + const services = new Map(); + const security: Record = { canReadObject }; + if (opts.getQueryableFields) security.getQueryableFields = vi.fn(opts.getQueryableFields); + if (opts.withSecurity !== false) services.set('security', security); + const protocol = new ObjectStackProtocolImplementation(engine as never, () => services as Map); + return { protocol, readCalls, findOptions, canReadObject }; +} + +async function rejection(run: () => Promise): Promise> { + let caught: unknown; + let didResolve = false; + try { + await run(); + didResolve = true; + } catch (e) { + caught = e; + } + expect(didResolve, 'expected a rejection, but the call resolved').toBe(false); + return caught as Record; +} + +describe('searchAll — an object the caller may not read is skipped, not fatal', () => { + it('control: without the pre-filter the middleware denial fails the whole search with 403', async () => { + // The defect, reproduced on this harness: no security service to ask, + // so the sweep reaches `find` on `lead` and the typed denial escapes. + const { protocol } = harness({ readable: new Set(['acct']), withSecurity: false }); + const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER })); + expect(caught.code).toBe('PERMISSION_DENIED'); + expect(caught.status).toBe(403); + }); + + it('a mixed scope answers the readable objects and never queries, names or counts the rest', async () => { + const { protocol, readCalls, canReadObject } = harness({ readable: new Set(['acct']) }); + + const result = await protocol.searchAll({ q: 'Acme', context: MEMBER }); + + expect(result.hits.map((h) => [h.object, h.id])).toEqual([['acct', 'a1']]); + expect(result.totalObjects).toBe(1); + expect(result.totalHits).toBe(1); + expect(result.truncated).toBe(false); + // Never queried — so nothing about its rows can reach the answer. + expect(readCalls).toEqual(['acct']); + // Asked with the caller's own context, for every swept object. + expect(canReadObject.mock.calls).toEqual([ + ['acct', MEMBER], + ['lead', MEMBER], + ['secret_ledger', MEMBER], + ]); + // Never named, anywhere in the response. + const wire = JSON.stringify(result); + expect(wire).not.toContain('lead'); + expect(wire).not.toContain('secret_ledger'); + expect(Object.keys(result).sort()).toEqual( + ['hits', 'pages', 'query', 'totalHits', 'totalObjects', 'truncated'], + ); + }); + + it('the answer does not depend on what a skipped object holds', async () => { + const matching = harness({ readable: new Set(['acct']) }); + const empty = harness({ + readable: new Set(['acct']), + rows: { acct: ROWS.acct, lead: [], secret_ledger: [] }, + }); + + const a = await matching.protocol.searchAll({ q: 'Acme', context: MEMBER }); + const b = await empty.protocol.searchAll({ q: 'Acme', context: MEMBER }); + + expect(a).toEqual(b); + }); + + it('an explicit objects= naming an unreadable object answers like one naming no such object', async () => { + const { protocol, readCalls } = harness({ readable: new Set(['acct']) }); + + const mixed = await protocol.searchAll({ q: 'Acme', objects: ['acct', 'secret_ledger'], context: MEMBER }); + expect(mixed.hits.map((h) => h.object)).toEqual(['acct']); + expect(mixed.totalObjects).toBe(1); + expect(JSON.stringify(mixed)).not.toContain('secret_ledger'); + + const unreadable = await protocol.searchAll({ q: 'Acme', objects: ['secret_ledger'], context: MEMBER }); + const nonexistent = await protocol.searchAll({ q: 'Acme', objects: ['no_such_object'], context: MEMBER }); + expect(unreadable).toEqual(nonexistent); + expect(unreadable).toEqual({ + query: 'Acme', hits: [], pages: [], totalObjects: 0, totalHits: 0, truncated: false, + }); + expect(readCalls).toEqual(['acct']); + }); + + it('an all-access caller gets exactly what the sweep answered without a security service', async () => { + const admin = harness({ readable: 'all' }); + const bare = harness({ readable: 'all', withSecurity: false }); + + const withService = await admin.protocol.searchAll({ q: 'Acme', context: { userId: 'usr_admin' } }); + const without = await bare.protocol.searchAll({ q: 'Acme', context: { userId: 'usr_admin' } }); + + expect(withService).toEqual(without); + expect(withService.hits.map((h) => h.object)).toEqual(['acct', 'lead', 'secret_ledger']); + expect(withService.totalObjects).toBe(3); + expect(admin.readCalls).toEqual(['acct', 'lead', 'secret_ledger']); + }); + + it('a read failure on a READABLE object still fails the search, envelope intact', async () => { + const injected = Object.assign(new Error('connection terminated unexpectedly'), { code: 'ECONNRESET' }); + const { protocol } = harness({ + readable: new Set(['acct', 'lead']), + failRead: { object: 'lead', error: injected }, + }); + + const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER })); + expect(caught).toBe(injected); + }); + + it('an admission check that THROWS fails the search instead of shrinking it', async () => { + const injected = Object.assign(new Error('permission store unreachable'), { code: 'ECONNREFUSED' }); + const { protocol, readCalls } = harness({ + readable: new Set(['acct']), + canReadObject: async (o) => { + if (o === 'lead') throw injected; + return o === 'acct'; + }, + }); + + const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER })); + expect(caught).toBe(injected); + expect(readCalls).not.toContain('lead'); + }); + + it('a context-less call is not pre-filtered: its reads carry no principal to ask about', async () => { + const { protocol, canReadObject } = harness({ readable: 'all' }); + + const result = await protocol.searchAll({ q: 'Acme' }); + + expect(canReadObject).not.toHaveBeenCalled(); + expect(result.totalObjects).toBe(3); + }); +}); + +describe('searchAll — a readable object is searched only on the fields the caller may query', () => { + const memo: FixtureObject = { + name: 'memo', + label: 'memo', + fields: { + name: { name: 'name', label: 'Name', type: 'text' }, + hidden_note: { name: 'hidden_note', label: 'Hidden note', type: 'text' }, + }, + searchableFields: ['name', 'hidden_note'], + }; + const rows = { memo: [{ id: 'm1', name: 'Acme memo' }] }; + + it('a partial queryable set is handed to the engine as searchFields', async () => { + const { protocol, findOptions } = harness({ + readable: 'all', objects: [memo], rows, + getQueryableFields: async () => ['id', 'name'], + }); + + const result = await protocol.searchAll({ q: 'Acme', context: MEMBER }); + + expect(findOptions).toHaveLength(1); + expect(findOptions[0][1].searchFields).toEqual(['name']); + expect(findOptions[0][1].search).toBe('Acme'); + expect(result.hits.map((h) => [h.object, h.id])).toEqual([['memo', 'm1']]); + expect(result.totalObjects).toBe(1); + }); + + it('an object with NO queryable search field is skipped, unqueried and uncounted', async () => { + const { protocol, readCalls } = harness({ + readable: 'all', objects: [acct, memo], rows: { ...ROWS, ...rows }, + getQueryableFields: async (o) => (o === 'memo' ? ['id'] : ['id', 'name']), + }); + + const result = await protocol.searchAll({ q: 'Acme', context: MEMBER }); + + expect(readCalls).toEqual(['acct']); + expect(result.totalObjects).toBe(1); + expect(JSON.stringify(result)).not.toContain('memo'); + }); + + it('a full queryable set, or no answer, leaves the request exactly as before (no searchFields)', async () => { + const full = harness({ + readable: 'all', objects: [memo], rows, + getQueryableFields: async () => ['id', 'name', 'hidden_note'], + }); + await full.protocol.searchAll({ q: 'Acme', context: MEMBER }); + expect('searchFields' in full.findOptions[0][1]).toBe(false); + + const noAnswer = harness({ + readable: 'all', objects: [memo], rows, + getQueryableFields: async () => undefined, + }); + await noAnswer.protocol.searchAll({ q: 'Acme', context: MEMBER }); + expect('searchFields' in noAnswer.findOptions[0][1]).toBe(false); + }); + + it('a queryable-fields check that THROWS fails the search', async () => { + const injected = new Error('field resolution unavailable'); + const { protocol, readCalls } = harness({ + readable: 'all', objects: [memo], rows, + getQueryableFields: async () => { throw injected; }, + }); + + const caught = await rejection(() => protocol.searchAll({ q: 'Acme', context: MEMBER })); + expect(caught).toBe(injected); + expect(readCalls).toEqual([]); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 55a83aae073..0b35ebb2763 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -145,7 +145,7 @@ import { PLURAL_TO_SINGULAR, SINGULAR_TO_PLURAL, canonicalMetaUrlType, metaUrlSp // [#13331] The cluster fan-out transport type only — the protocol never // depends on `@objectstack/service-cluster`; a bridge plugin there hands the // live transport in through `attachMetadataMutationPubSub`. -import type { IObjectQLEngine, IPubSub } from '@objectstack/spec/contracts'; +import type { IObjectQLEngine, IPubSub, ISecurityService } from '@objectstack/spec/contracts'; import { applyConversionsToStoredItem, type ConversionNotice, type ConversionTodoNotice } from '@objectstack/spec'; import { type FormView, type I18nLabel, isAggregatedViewContainer, expandViewContainer, resolveI18nLabel } from '@objectstack/spec/ui'; // [commit ece4dad31] Emitted-specifier pin. This module's inferred public declarations @@ -13816,6 +13816,34 @@ export class ObjectStackProtocolImplementation implements * RBAC/RLS is enforced by forwarding the caller's `context` to * `engine.find` so users only see records they are entitled to read. * + * ## An object the caller may not READ is outside the sweep + * + * The REST door checks authentication only, so the object-level read + * admission is decided per object, here: before an object is queried, the + * `security` service's `canReadObject` — the engine middleware's own read + * gate, arm for arm (`ISecurityService.canReadObject`) — is asked with the + * caller's context, and an object it refuses is skipped. Without this, the + * middleware's denial on the first unreadable object in scope failed the + * WHOLE search with 403, so a member who could not read every object got + * no results at all. The field-level half is the same question one axis + * down: the engine's predicate guard refuses a search that would match on + * a field the caller may not query, so each object's search fields are + * narrowed to the security service's `getQueryableFields` answer, and an + * object left with none is skipped too. + * + * A skipped object leaves nothing behind: it is never queried, never + * named, and never counted in `totalObjects`, and the decision is made + * before any row is read — so no hit, count or timing depends on what the + * object holds. An explicit `objects=` naming an unreadable object answers + * exactly as one naming an object that does not exist. A `false` is a + * narrowing only; the engine still enforces everything on the objects + * that ARE queried, row scope included. If `canReadObject` itself throws, + * the search fails rather than answering without that object. Note that + * plugin-security's `canReadObject` catches a permission-resolution + * failure itself and answers `false` (its documented fail-closed + * contract), so during a permission outage the sweep SKIPS objects rather + * than failing: nothing is disclosed, but `totalObjects` shrinks. + * * ## [#8896] A swept object that could not be READ fails the search * * `totalObjects` / `totalHits` / `truncated` describe a COMPLETE sweep of @@ -13916,6 +13944,27 @@ export class ObjectStackProtocolImplementation implements const hits: Array<{ object: string; id: string; title: string; snippet?: string; record: any }> = []; let objectsScanned = 0; + // The caller's read admission, asked of the same authority the engine + // middleware enforces (see the method doc): `canReadObject` for the + // object, `getQueryableFields` for the fields the search may match on. + // Only a caller that carries a context is asked about: a context-less + // in-process call reaches `find` without one, so asking on its behalf + // would answer a question its reads never pose. No security service, + // or one without a method, means no pre-filter on that axis — `find` + // still enforces both, so the absence can only make the answer + // stricter (a denial propagates, as before), never wider. That is also + // why an `undefined` ("no answer") from `getQueryableFields` narrows + // nothing here: this sweep never bypasses the middleware's guards. + const securityService = request.context !== undefined + ? this.getServicesRegistry?.().get('security') as Partial | undefined + : undefined; + const canReadObject = typeof securityService?.canReadObject === 'function' + ? (object: string) => securityService.canReadObject!(object, request.context) + : undefined; + const getQueryableFields = typeof securityService?.getQueryableFields === 'function' + ? (object: string) => securityService.getQueryableFields!(object, request.context) + : undefined; + for (const obj of allObjects) { if (hits.length >= overallLimit) break; if (!obj?.name) continue; @@ -14014,7 +14063,7 @@ export class ObjectStackProtocolImplementation implements // of one helper, which is the stronger form of the same fix. const fieldMetaByName: Record = {}; for (const f of fields) if (f?.name) fieldMetaByName[f.name] = f; - const { allowed: searchableFields } = resolveSearchFieldResolution({ + const { allowed: declaredSearchFields } = resolveSearchFieldResolution({ fields: fieldMetaByName, searchableFields: obj.searchableFields, // [ADR-0079] `nameField` is the canonical primary-title pointer; @@ -14023,7 +14072,39 @@ export class ObjectStackProtocolImplementation implements // a third spelling here would re-split what this card merged. displayField: obj.nameField ?? obj.displayNameField, }); - if (searchableFields.length === 0) continue; + if (declaredSearchFields.length === 0) continue; + + // Skip an object the caller may not read — before any query, so + // nothing about its rows can shape the answer, and before the + // count, so `totalObjects` covers only what was swept. A throw + // propagates: an admission that could not be decided fails the + // search rather than quietly shrinking it. + if (canReadObject && !(await canReadObject(obj.name))) continue; + + // …and match only on the fields the caller may QUERY. The engine's + // predicate guard refuses a search whose resolved fields include + // one hidden from the caller (the filter oracle: row presence would + // disclose the hidden value), and that 403 failed the whole sweep — + // `sys_user`'s searchable fields include admin-only columns, so + // every member's unscoped search hit it. `searchFields` only ever + // NARROWS the server-resolved set (ADR-0061), so handing the engine + // the queryable subset searches what the caller could have filtered + // on themselves through `searchFields`. An object left with no + // queryable search field is skipped like an unreadable one. + // An `undefined` answer narrows nothing here: unlike the contract's + // fallback for consumers without an answer (treat every field with a + // `maskingRule` as not queryable), this sweep leaves that judgement to + // the engine's predicate guard on `find`, which can only refuse. + let searchableFields = declaredSearchFields; + if (getQueryableFields) { + const queryable = await getQueryableFields(obj.name); + if (queryable !== undefined) { + const allowed = new Set(queryable); + searchableFields = declaredSearchFields.filter((f) => allowed.has(f)); + if (searchableFields.length === 0) continue; + } + } + const narrowed = searchableFields.length < declaredSearchFields.length; objectsScanned++; @@ -14040,10 +14121,12 @@ export class ObjectStackProtocolImplementation implements // and `search` is a declared `find` option // (`EngineQueryOptionsSchema`, `ENGINE_FIND_OPTION_KEYS`) — // so this is the engine's published door, not a private one. - // No `searchFields`: that key only ever NARROWS the resolved - // set (ADR-0061), and the palette wants the object's full - // default reach. + // `searchFields` only when the caller's queryable set is + // narrower than the declared one (see above): that key only + // ever NARROWS the resolved set (ADR-0061), and otherwise + // the palette wants the object's full default reach. search: q, + ...(narrowed ? { searchFields: searchableFields } : {}), limit: perObject, orderBy: [{ field: 'updated_at', order: 'desc' }], }; @@ -14099,15 +14182,19 @@ export class ObjectStackProtocolImplementation implements // query error or a refused datasource all mean the object's rows // may well match and simply were not seen. // - // The comment this replaces named "RBAC denial" as a benign - // reason. Measured on this tree, that is not a failure mode of - // this seam: object-level authorization is enforced at the REST - // door (`enforceAuth`) BEFORE `searchAll` is reached, and - // row-level security narrows `find`'s result set rather than - // throwing. Nothing in-repo registers a `beforeFind` hook that - // denies by throwing. Were one added, the ruling for this family - // still applies: a read that could not run must not be answered - // "there are no matches here". + // A permission denial is NOT swallowed here either. The REST + // door (`enforceAuth`) checks authentication only; the read + // admission is asked BEFORE this `try`, via the security + // service's `canReadObject` and `getQueryableFields` (see + // above), so an object the caller may not read never reaches + // `find`, and one it may read is searched only on fields it may + // query. A denial that still arrives here is a verdict those + // answers did not foresee — a permission subsystem that could + // not resolve, a delegator that does not exist, a security + // service without those methods — and the ruling for this + // family applies to it: a read that could not run must not be + // answered "there are no matches here". Row-level security + // narrows `find`'s result set rather than throwing. // // No new response field and no new error code — the caller // receives the read's own failure, envelope intact. diff --git a/packages/qa/dogfood/test/search-skip-unreadable.dogfood.test.ts b/packages/qa/dogfood/test/search-skip-unreadable.dogfood.test.ts new file mode 100644 index 00000000000..21154bd60b0 --- /dev/null +++ b/packages/qa/dogfood/test/search-skip-unreadable.dogfood.test.ts @@ -0,0 +1,172 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// END-TO-END: a member whose global-search scope includes objects they cannot +// read gets hits from the objects they CAN read — not a 403 for the whole +// request. +// +// ## The defect +// +// `GET /api/v1/search` checks authentication only; each object in scope was +// then queried through `engine.find`, whose security middleware refuses an +// object the caller holds no read grant on. `searchAll` let that refusal +// propagate, so a member who could not read EVERY object in scope was answered +// `403 PERMISSION_DENIED` whatever the query. An unscoped search sweeps every +// registered object — the platform's own `sys_*` objects included — so for a +// plain member that was every unscoped search. The console palette sends such +// a scope and rendered "No results found." with no error. +// +// ## What this boot pins +// +// A real kernel with the real `SecurityPlugin`, over HTTP, two app objects +// holding a row that matches the same term: the member's fallback set grants +// read on `skipsearch_open` and nothing on `skipsearch_walled`. +// +// - the member's UNSCOPED search answers 200 with the readable row, and the +// walled object is neither hit nor named; +// - an explicit `objects=` naming the walled object answers 200 with the +// readable object's hits only — and naming it alone answers exactly as a +// name that matches no object; +// - the administrator still gets both rows (the negative control: a fix that +// skipped everything would turn this red); +// - the walled object stays refused at its own door (`GET /data/...` 403), +// so the search is not a way around it. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +const TERM = 'zephyrquill'; + +const SkipSearchOpen = ObjectSchema.create({ + name: 'skipsearch_open', + sharingModel: 'public_read_write', + label: 'Skip Search Open', + pluralLabel: 'Skip Search Open', + fields: { name: Field.text({ label: 'Name', required: true, searchable: true }) }, +}); + +const SkipSearchWalled = ObjectSchema.create({ + name: 'skipsearch_walled', + sharingModel: 'public_read_write', + label: 'Skip Search Walled', + pluralLabel: 'Skip Search Walled', + fields: { name: Field.text({ label: 'Name', required: true, searchable: true }) }, +}); + +const stackDef = defineStack({ + manifest: { + id: 'com.dogfood.search-skip-unreadable', + namespace: 'skipsearch', + version: '0.0.0', + type: 'app', + name: 'Search Skip Unreadable Fixture', + description: 'Two searchable objects; the member may read one of them.', + }, + objects: [SkipSearchOpen, SkipSearchWalled], +}); + +const memberSet: PermissionSet = PermissionSetSchema.parse({ + name: 'skipsearch_member', + label: 'Skip Search Member — read on skipsearch_open only', + objects: { + skipsearch_open: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, +}); + +interface SearchBody { + hits: Array<{ object: string; id: string; title: string }>; + totalObjects: number; + totalHits: number; +} + +let stack: VerifyStack; +let adminToken: string; +let memberToken: string; +let openId: string; +let walledId: string; + +function createdId(body: unknown): string { + const b = body as { id?: unknown; record?: { id?: unknown }; data?: { id?: unknown } }; + const id = b.id ?? b.record?.id ?? b.data?.id; + expect(typeof id).toBe('string'); + return String(id); +} + +async function search(token: string, query: string): Promise<{ status: number; text: string; body: SearchBody }> { + const res = await stack.apiAs(token, 'GET', `/search?${query}`); + const text = await res.text(); + return { status: res.status, text, body: JSON.parse(text) as SearchBody }; +} + +describe('dogfood: global search skips the objects a member cannot read', () => { + beforeAll(async () => { + stack = await bootStack(stackDef as never, { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, memberSet], + fallbackPermissionSet: memberSet.name, + }), + }); + adminToken = await stack.signIn(); + memberToken = await stack.signUp('skipsearch-member@verify.test'); + + const open = await stack.apiAs(adminToken, 'POST', '/data/skipsearch_open', { name: `${TERM} open` }); + expect(open.status).toBeLessThan(300); + openId = createdId(await open.json()); + const walled = await stack.apiAs(adminToken, 'POST', '/data/skipsearch_walled', { name: `${TERM} walled` }); + expect(walled.status).toBeLessThan(300); + walledId = createdId(await walled.json()); + }, 120_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('control: the walled object is refused to the member at its own door', async () => { + const res = await stack.apiAs(memberToken, 'GET', '/data/skipsearch_walled'); + expect(res.status).toBe(403); + const readable = await stack.apiAs(memberToken, 'GET', '/data/skipsearch_open'); + expect(readable.status).toBe(200); + }); + + it("the member's UNSCOPED search answers the readable object's hit and never names the walled one", async () => { + const { status, text, body } = await search(memberToken, `q=${TERM}`); + + expect(status, text).toBe(200); + expect(body.hits.filter((h) => h.object === 'skipsearch_open').map((h) => h.id)).toEqual([openId]); + expect(body.hits.some((h) => h.object === 'skipsearch_walled')).toBe(false); + expect(text).not.toContain('skipsearch_walled'); + expect(text).not.toContain(walledId); + }); + + it('an explicit objects= naming the walled object answers the readable hits only', async () => { + const mixed = await search(memberToken, `q=${TERM}&objects=skipsearch_open,skipsearch_walled`); + expect(mixed.status).toBe(200); + expect(mixed.body.hits.map((h) => [h.object, h.id])).toEqual([['skipsearch_open', openId]]); + expect(mixed.body.totalObjects).toBe(1); + expect(mixed.text).not.toContain('skipsearch_walled'); + + // Naming ONLY the walled object answers exactly as naming no object at all. + const walledOnly = await search(memberToken, `q=${TERM}&objects=skipsearch_walled`); + const nonexistent = await search(memberToken, `q=${TERM}&objects=skipsearch_no_such_object`); + expect(walledOnly.status).toBe(200); + expect(walledOnly.body).toEqual(nonexistent.body); + expect(walledOnly.body.hits).toEqual([]); + expect(walledOnly.body.totalObjects).toBe(0); + }); + + it('negative control: the administrator still gets the hits from both objects', async () => { + const { status, text, body } = await search(adminToken, `q=${TERM}&objects=skipsearch_open,skipsearch_walled`); + expect(status, text).toBe(200); + expect(body.hits.map((h) => [h.object, h.id]).sort()).toEqual( + [['skipsearch_open', openId], ['skipsearch_walled', walledId]].sort(), + ); + expect(body.totalObjects).toBe(2); + + const unscoped = await search(adminToken, `q=${TERM}`); + expect(unscoped.status).toBe(200); + expect(unscoped.body.hits.filter((h) => h.object.startsWith('skipsearch_')).length).toBe(2); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index b7afd946262..4521905cf84 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -1391,6 +1391,11 @@ "verb": "findOne", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.search-skip-unreadable.test.ts", + "verb": "findOne", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.served-content-hash.test.ts", "verb": "delete",