Skip to content

Commit c63028e

Browse files
fix(metadata-protocol)!: the cross-object search asks the exposure decision for search on each object (#22640) (#22704)
Fixes #22640 Clause-②: no The cross-object search now asks the spec's one exposure decision for the `search` operation on each object. Before, `ObjectStackProtocolImplementation.searchAll` (`packages/metadata-protocol/src/protocol.ts`) used its own skip predicate, which read `enable.apiEnabled === false` and `enable.searchable === false` and never read `apiMethods`. So a whitelist that withholds `list`, and with it `search`, kept nothing out of the sweep. ## What changed - **`searchAll`'s per-object skip** asks `canServeApiOperation(obj.enable, 'search')` from `@objectstack/spec/data`. This is the decision the REST data routes, the dispatcher, MCP and the analytics door already judge. It runs first in the loop, so a refused object is never queried, named or counted. An explicit `objects=` naming it answers like one naming no object at all. - **`searchable: false` keeps its own skip** beside the decision. The decision does not cover it everywhere, as measured below. - No other product code changed. `getQueryableFields` and the data routes in `protocol.ts` are untouched. ## The premise, checked on `origin/main` `cb3bb933` - **The predicate.** Found inside `searchAll`, as the card describes. The line numbers had moved since the card was filed. - **What the decision covers for `search`.** Measured against the built `@objectstack/spec/data`: | `enable` | `apiExposureDenialReason(enable, 'search')` | |:--|:--| | absent, or `{}` | `null` (served) | | `{ searchable: false }` (no whitelist) | `null` (served) | | `{ searchable: false, apiMethods: ['get', 'list'] }` | `method-not-allowed` | | `{ apiMethods: ['get'] }` | `method-not-allowed` | | `{ apiMethods: [] }` | `method-not-allowed` | | `{ apiEnabled: false }`, with or without a whitelist | `api-disabled` | | `{ apiMethods: ['list'] }` or `['get', 'list']` | `null` (served) | The decision covers `apiEnabled: false` and the whitelist. It covers `searchable: false` only under a whitelist: for an object with none (`unrestricted`) it answers without reading the flag. That is why the `searchable` line stays. The PM's mechanism assumption that the decision covers both flags is therefore only half right: it holds for `apiEnabled`, and for `searchable` only under a whitelist. - **Dormant today, confirmed.** Every object in this repository whose `apiMethods` withholds `list` also declares `searchable: false` or `apiEnabled: false`. That is three `['get']` objects (`sys_verification`, `sys_device_code`, `sys_two_factor`), each with `searchable: false`, and nine deny-all `[]` objects, each with `apiEnabled: false` and all but `sys_approval_token` also with `searchable: false`. The control: the same scan reads all 70 inline whitelist declarations in the repository, and a second scan for any `apiMethods:` not written inline finds only the registry's rewrite and the Zod schema, so no object hides a multi-line whitelist. No example app declares `apiMethods`. So no shipped object's search results change. Deployed and cloud-held definitions were NOT MEASURED. ## Pins 1. **`packages/metadata-protocol/src/protocol.search-api-exposure.test.ts`** (13 cases) holds the card's three pins: - An object whose whitelist withholds `list`, without `searchable: false`, is not swept. This is checked unscoped, with `objects=`, as deny-all `[]`, and context-less. - The existing skips still hold: `apiEnabled: false`, and `searchable: false` both with and without a whitelist. - An ordinary object, and one whose whitelist grants `list`, is swept as before. 2. **The enumeration pin, homed outside `metadata-protocol`: `packages/core/src/security/row-serving-door-exposure.pin.test.ts`.** This is triage's escalation: every row-serving door that is not a data route must ask the same decision. - **Discriminator.** A door that sweeps or joins objects it chose itself has to decide read admission per object, so it calls the security service's `canReadObject` itself. A data route gets that from the engine middleware instead. Every non-test `.ts` source under `packages/` that calls it must be classified in the pin's table. - **Current classification.** On this tree that is two doors and one provider: - search: `metadata-protocol` `searchAll`, operation `search`; - analytics: `service-analytics`, operation `aggregate`, decision asked in `api-exposure-door.ts`; - the provider: `plugin-security`, which is not a door. - **What it asserts.** Each door's decision site calls the decision, and the door's behaviour pin exists. A new unclassified caller, or a stale classified file, turns it red. - **Why it lives in `core`.** `core` already declares the `packages/**/*.ts` cross-package radius, so the pin needs no new declaration in `scripts/cross-package-test-inputs.mjs` or `turbo.json`. Like its siblings, it is listed in `packages/core/vitest.repo-tests.json`, so it runs in core's `repo` project, whose turbo task hashes that radius. Its scan surface and prefilter copy `operation-private-keys.pin.test.ts` in the same directory. - **Blind spot, stated in its header.** It cannot see a door that reads rows without asking `canReadObject` at all. Two such doors are named there and not held: the share-link door, and knowledge retrieval. ## Ablations (all from committed state, via `scripts/ablation-replace.mjs`, restore proven by blob == HEAD and an empty `git diff HEAD`) - **The fix.** The decision line was replaced with the old `apiEnabled` check (anchor 1 to 0). The exposure pin went red on exactly the 4 negative cases and stayed green on the 9 others. The first failure read `expected [ 'plain', 'get_only' ] to deeply equal [ 'plain' ]`. - **Census A, the same mutation.** The census pin's "each door asks the decision" case went red with "the cross-object search ... does not ask the exposure decision in packages/metadata-protocol/src/protocol.ts". The other two cases stayed green. This also shows that the docblock mention of the call is not counted as a call. - **Census B, a planted caller.** A `.canReadObject(` call was appended in a non-door core source. The "every caller is classified" case went red, naming that file, and the other two stayed green. The first attempt was a no-op, refused by the tool: the replacement contained the anchor, so the anchor count did not drop. It was redone with a different anchor spelling. ## Tests and gates All on HEAD `7e7d0eefe2`, which is this branch after merging `origin/main` `d8830c2805`, unless a line names another commit. - `pnpm --filter @objectstack/metadata-protocol exec vitest run --maxWorkers=2 src/protocol.search-api-exposure.test.ts src/protocol.search-skip-unreadable.test.ts`: 2 files, 25 tests passed. - `pnpm --filter @objectstack/core exec vitest run --project repo --maxWorkers=2`: 4 files, 51 tests passed, the census included. - `pnpm --filter @objectstack/plugin-approvals exec vitest run src/sys-approval-token-generic-door.integration.test.ts`: 6 tests passed. This suite runs on a real engine and resolves `metadata-protocol` through its built `dist/`, which carries the fix (`canServeApiOperation(obj.enable, "search")` is present in `dist/index.js`). - Typecheck: `pnpm --filter @objectstack/metadata-protocol run typecheck` exited 0. `pnpm --filter @objectstack/core run typecheck` also exited 0, with the test layer OK. - Full suites on `f3b4714a`, before the merge and the last test-only commit; the `metadata-protocol` source is unchanged since: - `metadata-protocol`: 222 files passed and 3 skipped; 28060 tests passed. - `core --project local`: 89 files, 2296 tests passed. - **Gates.** `dispatch-gates --commands` on this HEAD derives 71 families, the 51 named in the dispatch plus 20 more. All 71 were run on this HEAD and every one exited 0. `dispatch-gates --ran` reports: 71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN, a derived zero. - A first battery on `f3b4714a` had 7 non-zero exits: - `check:engine-double-contract` flagged a new `findOne` double. The double was unneeded and was dropped, which leaves the ledger untouched. - `check:cross-package-test-inputs` asked for the census in core's `repo` project, and for one quoted path mention to be reworded. - Four were prerequisites, not findings: the teardown-shape self-test needed its pinned fixture commit fetched (the clone is shallow), and `check:dual-build-cjs-loads`, `check:i18n` and `check:lean-entry-closure` needed built dists. - `check:type-check-debt` was stopped by the runner's own 900-second cap during its full re-measure. - All 7 are green in the second battery, after a full `./packages/**` build. - **Lint, a declared narrowing.** `eslint --no-inline-config --format json` over the 4 changed `.ts` files: 4 files, 0 errors, 0 warnings. The checked population was read from the config: `--print-config` matches every one, and the `.md` and `.json` files are outside the config's `files`. The invariance holds because `eslint.config.mjs` enables no type-aware linting (no `parserOptions.project` and no `projectService` in code), so this diff cannot move a verdict on an untouched file. The repository-wide `pnpm lint` is CI's. ## Also in this diff - `packages/plugins/plugin-approvals/src/sys-approval-token-generic-door.integration.test.ts`: one docblock sentence said `searchAll` "never consults `apiMethods` at all". This change made that false, so the sentence was rewritten. It is a comment-only edit in a test file, outside the claim's file surface. - `packages/core/vitest.repo-tests.json`: one entry, the census, which `check:cross-package-test-inputs` requires. It is a test-infrastructure list, outside the claim's file surface. - `.changeset/22640-search-api-exposure.md`: `minor`, BREAKING (an accept-set narrowing), FROM to TO, and ADR-0087 `not-required (no-migration-prescription)`. ## Acceptance notes - **The Clause-② line.** The `Clause-②: no` line is copied verbatim from the claim. The claim's own text calls this change a narrowing, but the line carries no `(narrowing)` arm. The changeset declares breaking through its `**BREAKING**` banner and its `!` summary, and `check:adr-0087-registration` reads it as `[BREAKING+bang]`. - **Doors the enumeration cannot see** (observation only, not filed): - The share-link door (`plugin-sharing` `share-link-routes.ts`) serves a record as the system behind its token. Its gate is its own opt-in public-sharing policy, not the exposure decision. - Knowledge retrieval (`service-knowledge` `KnowledgeService.search` / `reindexSource`) serves object-source content without asking the decision. It has no HTTP door in this repository. - Neither was measured through a public door. - **A comment now incomplete.** `service-analytics` `api-exposure-door.ts` says the cross-object search "reads the same off switch". It now asks the whole decision. The sentence is still true but incomplete, and it was not edited because it is product code outside this claim. - **No checklist item yet.** The search area of the platform test checklist (`docs/qa/platform-checklist/areas/search.json`) has no item for this exposure skip. --- _Generated by [Claude Code](https://claude.ai/code/session_01JfJfBUC3cQ6hhgm9MQK76T)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9f15939 commit c63028e

6 files changed

Lines changed: 423 additions & 8 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
---
2+
'@objectstack/metadata-protocol': minor
3+
---
4+
5+
fix(metadata-protocol)!: the cross-object search asks the spec's exposure decision for `search` on each object, so an `apiMethods` whitelist that withholds `list` keeps the object out of the sweep (#22640)
6+
7+
Clause-②: no
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export or stored shape is removed, renamed or re-shaped, so there is nothing for `objectstack migrate meta` to rewrite. What narrows is a runtime read door: the search stops serving objects whose existing `enable` declaration already refuses `search` on every other generic exit, and the remedy is the declaration the author already wrote. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no exported declaration is removed or narrowed (not runtime-interface-only or type-surface-only). -->
10+
11+
**BREAKING** (an accept-set narrowing), shipped as `minor` under the launch-window convention for breaking changes.
12+
13+
The REST data routes, the runtime dispatcher, the MCP data tools and the analytics door judge an object's `enable` block through the spec's one exposure decision (`apiExposureDenialReason` / `canServeApiOperation`). In the derivation table, `search` derives from `list`. The cross-object search (`GET /search`, `searchAll`) spelled its own skip instead, and that skip read only `apiEnabled: false` and `searchable: false`.
14+
15+
**FROM.** The search swept an object whose `apiMethods` whitelist does not grant `list` (for example `['get']`, or the deny-all `[]`), unless the object also declared `searchable: false` or `apiEnabled: false`. It served that object's rows while every data route refused them.
16+
17+
**TO.** Before an object is queried, the search asks `canServeApiOperation(enable, 'search')`, and an object it refuses is skipped. A skipped object is never queried, named or counted. An explicit `objects=` naming it answers like one naming no object at all.
18+
19+
- **Every caller.** The decision takes no user, so administrators and context-less in-process calls skip the same objects.
20+
- **`searchable: false` keeps its own skip.** The decision answers an object with no whitelist without reading that flag.
21+
- **Unchanged.** An object with no whitelist, or with one that grants `list`, is swept exactly as before.
22+
23+
**Measured producers.** Read on `origin/main` `cb3bb933`: every object in this repository whose whitelist withholds `list` also declares `searchable: false` or `apiEnabled: false`, and no example app declares `apiMethods`. So no shipped object's search results change. Deployed and cloud-held object definitions were NOT MEASURED.
Lines changed: 197 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,197 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
/**
4+
* [#22640] Every door that serves object rows OUTSIDE the data routes asks the
5+
* spec's one exposure decision: `apiExposureDenialReason` or its boolean face
6+
* `canServeApiOperation` (`@objectstack/spec/data`, ADR-0049).
7+
*
8+
* The data routes (REST, the runtime dispatcher, MCP) ask it per request. Two
9+
* other doors were each found reading an object's exposure by hand, or not at
10+
* all, after the data routes were settled: the analytics door (#22634) and
11+
* the cross-object search (#22640). Each was fixed in its own package and
12+
* pinned there by behaviour. This file is the ENUMERATION: it names the doors
13+
* of that family, holds each one to the decision, and goes red when a new
14+
* door of the same shape appears without being classified here.
15+
*
16+
* ## How a door of this family is found, mechanically
17+
*
18+
* A data route gets its read admission from the engine middleware, one object
19+
* per request. A door that sweeps or joins objects it chose itself (search,
20+
* analytics) has to decide per object which ones the caller may read, so it
21+
* asks the `security` service's `canReadObject` itself. That call is the
22+
* discriminator: every non-test `.ts` source under `packages/` that CALLS
23+
* `canReadObject` must be classified in {@link CALLERS} below, either as a
24+
* door (with the file where it asks the exposure decision, and the behaviour
25+
* pin that holds it) or as something that is not a door (with the reason).
26+
* An unclassified caller, or a classified file that no longer calls it, turns
27+
* this red.
28+
*
29+
* ⛔ What the discriminator does NOT see, stated rather than discovered later:
30+
* a door that reads rows without asking `canReadObject` at all, such as one
31+
* that relies on the engine middleware by passing the caller's context to
32+
* `find`, or one that reads as the system behind its own token. Read on
33+
* `origin/main` at cb3bb933, the doors of that kind are the share-link door
34+
* (`plugin-sharing`, gated by its own opt-in public-sharing policy) and
35+
* knowledge retrieval (`service-knowledge`, an admin-declared source with no
36+
* HTTP door in this repository). They are named here, not held.
37+
*
38+
* The scan surface and its prefilter follow
39+
* `operation-private-keys.pin.test.ts` in this directory: git's authored-file
40+
* list, never a directory crawl, and `.ts` only, the radius this package
41+
* declares in the cross-package test-inputs table. That is also why this file
42+
* is listed in this package's `repo` vitest project.
43+
*/
44+
45+
import { execFileSync } from 'node:child_process';
46+
import { existsSync, readFileSync } from 'node:fs';
47+
import { dirname, join, resolve } from 'node:path';
48+
import { fileURLToPath } from 'node:url';
49+
50+
import { describe, it, expect } from 'vitest';
51+
52+
const HERE = dirname(fileURLToPath(import.meta.url));
53+
/** …/packages/core/src/security → repo root */
54+
const REPO_ROOT = resolve(HERE, '../../../..');
55+
56+
/** The read-admission method, as data. */
57+
const ADMISSION = 'canReadObject';
58+
59+
/** A CALL of the admission on some receiver: `svc.canReadObject(`, `svc.canReadObject!(`. */
60+
const admissionCall = (): RegExp => new RegExp(`\\.${ADMISSION}!?\\s*\\(`);
61+
62+
/** A CALL of the spec's exposure decision, either face. */
63+
const decisionCall = (): RegExp => /\b(?:apiExposureDenialReason|canServeApiOperation)\s*\(/;
64+
65+
type Classification =
66+
| {
67+
kind: 'door';
68+
/** The door, in words, with the operation it is judged as. */
69+
door: string;
70+
operation: 'search' | 'aggregate';
71+
/** Where the door asks the exposure decision. */
72+
decisionSite: string;
73+
/** The behaviour pin that holds the door to it. */
74+
pin: string;
75+
}
76+
| { kind: 'not-a-door'; reason: string };
77+
78+
/** Every non-test caller of the read admission, classified. */
79+
const CALLERS: Record<string, Classification> = {
80+
'packages/metadata-protocol/src/protocol.ts': {
81+
kind: 'door',
82+
door: 'the cross-object search (`GET /search`, `searchAll`)',
83+
operation: 'search',
84+
decisionSite: 'packages/metadata-protocol/src/protocol.ts',
85+
pin: 'packages/metadata-protocol/src/protocol.search-api-exposure.test.ts',
86+
},
87+
'packages/services/service-analytics/src/plugin.ts': {
88+
kind: 'door',
89+
door: 'the analytics door (`AnalyticsService` query and registration)',
90+
operation: 'aggregate',
91+
decisionSite: 'packages/services/service-analytics/src/api-exposure-door.ts',
92+
pin: 'packages/services/service-analytics/src/__tests__/api-exposure-door.test.ts',
93+
},
94+
'packages/plugins/plugin-security/src/security-plugin.ts': {
95+
kind: 'not-a-door',
96+
reason: 'the provider: it implements the admission and binds it into the security service it registers; it serves no rows',
97+
},
98+
};
99+
100+
const SCAN_TIMEOUT_MS = 60_000;
101+
102+
function git(args: string[]): string[] {
103+
let stdout: string;
104+
try {
105+
stdout = execFileSync('git', args, { cwd: REPO_ROOT, encoding: 'utf8', maxBuffer: 1 << 28 });
106+
} catch (error) {
107+
const failure = error as { status?: number; stderr?: string };
108+
// `git grep` exits 1 for "found nothing", which is data. Anything else is a
109+
// broken scan and must not read as "no callers".
110+
if (failure.status === 1) return [];
111+
throw new Error(`git ${args.join(' ')} failed with status ${String(failure.status)}: ${failure.stderr ?? ''}`);
112+
}
113+
return stdout.split('\0').filter((line) => line.length > 0);
114+
}
115+
116+
const isScannedSource = (path: string) =>
117+
path.endsWith('.ts') && !path.endsWith('.d.ts') && !/\.(test|spec)\.ts$/.test(path);
118+
119+
/** Code lines only: a docblock or line comment that names a call is not one. */
120+
const codeLines = (file: string) =>
121+
readFileSync(join(REPO_ROOT, file), 'utf8')
122+
.split('\n')
123+
.filter((line) => !/^\s*(\*|\/\/|\/\*)/.test(line));
124+
125+
const calls = (file: string, matcher: () => RegExp) => codeLines(file).some((line) => matcher().test(line));
126+
127+
/** Fixed-string prefilter over tracked plus untracked sources; the regex decides. */
128+
function filesMentioningTheAdmission(): string[] {
129+
return git(['grep', '--files-with-matches', '-z', '--untracked', '--fixed-strings', '-e', ADMISSION, '--', 'packages'])
130+
.filter(isScannedSource);
131+
}
132+
133+
function admissionCallers(): string[] {
134+
return filesMentioningTheAdmission().filter((file) => calls(file, admissionCall)).sort();
135+
}
136+
137+
describe('[#22640] every row-serving door outside the data routes asks the exposure decision', () => {
138+
it(
139+
'every caller of the read admission is classified, and every classified file still calls it',
140+
() => {
141+
const found = admissionCallers();
142+
const unclassified = found.filter((file) => !(file in CALLERS));
143+
const stale = Object.keys(CALLERS).filter((file) => !found.includes(file));
144+
145+
expect(
146+
unclassified,
147+
[
148+
'These sources ask the read admission themselves, which is how a door that serves rows outside',
149+
'the data routes looks, and this pin does not know them:',
150+
...unclassified.map((f) => ` - ${f}`),
151+
'',
152+
'Classify each in CALLERS. A door must ask `canServeApiOperation` / `apiExposureDenialReason`',
153+
'(`@objectstack/spec/data`) for the operation it serves, before any row is read, and carry a',
154+
'behaviour pin in its own package.',
155+
].join('\n'),
156+
).toEqual([]);
157+
expect(stale, 'These classified files no longer call the read admission; re-classify or remove them.').toEqual([]);
158+
},
159+
SCAN_TIMEOUT_MS,
160+
);
161+
162+
it(
163+
'each door asks the decision at its decision site, and its behaviour pin exists',
164+
() => {
165+
const doors = Object.values(CALLERS).filter(
166+
(c): c is Extract<Classification, { kind: 'door' }> => c.kind === 'door',
167+
);
168+
expect(doors.map((d) => d.operation).sort()).toEqual(['aggregate', 'search']);
169+
170+
for (const door of doors) {
171+
expect(calls(door.decisionSite, decisionCall), `${door.door} does not ask the exposure decision in ${door.decisionSite}`).toBe(true);
172+
expect(
173+
readFileSync(join(REPO_ROOT, door.decisionSite), 'utf8'),
174+
`${door.decisionSite} does not import the decision from the spec`,
175+
).toMatch(/from '@objectstack\/spec\/data'/);
176+
expect(existsSync(join(REPO_ROOT, door.pin)), `the behaviour pin of ${door.door} is missing: ${door.pin}`).toBe(true);
177+
}
178+
},
179+
SCAN_TIMEOUT_MS,
180+
);
181+
182+
it(
183+
'the scan cannot pass vacuously: the prefilter reaches every classified file and the matcher fires on each',
184+
() => {
185+
const mentioning = filesMentioningTheAdmission();
186+
for (const file of Object.keys(CALLERS)) {
187+
expect(mentioning, `the prefilter did not reach ${file}`).toContain(file);
188+
expect(calls(file, admissionCall), `the call matcher does not fire on ${file}`).toBe(true);
189+
}
190+
// The contract DECLARES the method and names it in prose; neither is a call.
191+
const contract = 'packages/spec/src/contracts/security-service.ts';
192+
expect(mentioning).toContain(contract);
193+
expect(calls(contract, admissionCall)).toBe(false);
194+
},
195+
SCAN_TIMEOUT_MS,
196+
);
197+
});
Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
[
22
"src/security/authz-store-unavailable.test.ts",
33
"src/security/granted-permissions-not-enforced.pin.test.ts",
4-
"src/security/operation-private-keys.pin.test.ts"
4+
"src/security/operation-private-keys.pin.test.ts",
5+
"src/security/row-serving-door-exposure.pin.test.ts"
56
]

0 commit comments

Comments
 (0)