diff --git a/.changeset/client-url-conformance-capstone.md b/.changeset/client-url-conformance-capstone.md new file mode 100644 index 0000000000..719cce5626 --- /dev/null +++ b/.changeset/client-url-conformance-capstone.md @@ -0,0 +1,41 @@ +--- +"@objectstack/client": patch +--- + +test(client): close the route audit's reverse direction — every SDK URL must match a route some surface mounts (#3642) + +The capstone of the #3563 route audit. The dispatcher (#3563), REST (#3587) and +service-mount (#3636) ledgers all run server → client: enumerate what a surface +mounts, demand a reviewed disposition, and for `sdk` rows demand the named +client method exists. None of them asked the reverse question — does the URL +the client *builds* match anything a server *mounts*? — so a method could name +a real function, carry a green ledger row, and 404 everywhere. + +That shipped four times, found one at a time by hand: `analytics.explain` and +`analytics.meta` (#3584), `meta.getView` (#3611), and `i18n.getTranslations` / +`getFieldLabels` (#3636) — the last pair having carried green `sdk` rows since +tranche 1. + +`client-url-conformance.test.ts` drives every method on a real client with a +recording `fetch` and matches each captured URL against the union of all four +ledgers. A real drive rather than a hand-written "method X targets route Y" +table, because such a table is an assertion *about* the code that the code can +drift away from — the exact failure being fixed. Mutation-checked: re-injecting +the #3636 dialect bug fails the suite. + +The sweep's own completeness is asserted, since that is what rots silently — a +new method must be driven or declared `NON_HTTP` with a reason; a driven method +emitting zero requests fails (stale placeholder args are how a sweep quietly +stops covering anything); a URL containing `undefined` fails; and the +`__api-endpoint` `(unmatched)` catch-all is excluded from the pattern set so it +cannot match everything and make the suite vacuous. + +196 of ~219 methods matched. Two bounds are reported rather than papered over: +`/api/v1/cloud/*` (23 `projects.*` methods) belongs to the sibling `cloud` repo +and is exempt by prefix, bounded so no other namespace can use it (#3655); and +60 of ~196 matched calls rest only on a `**` prefix claim rather than a +resolvable route — 54 of those on `* /auth/**` — a count the guard ratchets so +it can only shrink (#3656). + +No runtime change: this is a guard plus the ledger-header and audit-doc notes +recording what it does and does not cover. diff --git a/docs/audits/2026-07-dispatcher-client-route-coverage.md b/docs/audits/2026-07-dispatcher-client-route-coverage.md index 87cd537c4c..eb88ba63c6 100644 --- a/docs/audits/2026-07-dispatcher-client-route-coverage.md +++ b/docs/audits/2026-07-dispatcher-client-route-coverage.md @@ -195,10 +195,47 @@ decided by which plugin mounted the route. Also filed, not fixed: `GET {base}/_local/file/:key` is built by three call sites and mounted by none (#3641). -**The gap all three ledgers still share** is the reverse direction — no guard -compares the URL a client method *builds* against the patterns any surface -*mounts*. Four instances of that class have now been found one at a time -(#3584 ×2, #3611, #3636 ×2). Mechanizing it is the capstone, #3642. +**The gap all three ledgers shared** was the reverse direction — no guard +compared the URL a client method *builds* against the patterns any surface +*mounts*. Four instances of that class were found one at a time +(#3584 ×2, #3611, #3636 ×2). Mechanized in #3642, below. + +## 10. The reverse direction, mechanized (#3642) + +`packages/client/src/client-url-conformance.test.ts` drives **every** method on +a real `ObjectStackClient` with a recording `fetch` and matches each captured +URL against the **union** of all four ledgers (a union, not an intersection — +a route mounted by one surface is still reachable). A real drive, not a +declaration table: "method X targets route Y" written by hand is an assertion +*about* the code that the code can drift away from, which is the very failure +being fixed. + +Result at landing: 196 of ~219 methods matched; the only unmatched family was +`projects.*`, which targets the control plane (below). Mutation-checked — the +#3636 dialect bug, re-injected, fails the suite. + +**The sweep's own completeness is asserted**, because that is the part that +rots silently: + +| Assertion | What it stops | +|---|---| +| every method is driven or declared `NON_HTTP` **with a reason** | a new SDK method escaping coverage | +| a driven method emitting **zero** requests fails | placeholder args going stale, so the method throws before fetching and the guard passes while covering nothing | +| a URL containing `undefined` / `[object Object]` fails | a placeholder that is accepted but wrong masquerading as coverage | +| `(unmatched)` is excluded from the pattern set | the `__api-endpoint` catch-all matching everything and making the suite vacuous | + +**Two bounds, both explicit rather than papered over:** + +- **The control plane.** `/api/v1/cloud/*` (23 `projects.*` methods) is served + by the sibling `cloud` repo — this repo's dispatcher explicitly refuses those + paths — so no in-repo ledger can vouch for them. Exempt by prefix and bounded + from both ends: a non-`projects` method reaching `/cloud/` fails. Tracked as + #3655. +- **Dynamic families.** A `**` row claims a prefix, not a resolvable route. + **60 of ~196 matched calls (~31%) rest on nothing stronger** — 54 of them on + `* /auth/**`, where the routes come from a third-party dependency on its own + release cadence. The guard counts and ratchets this, so it can only shrink. + Tracked as #3656. ## Follow-up slicing (proposed) @@ -211,7 +248,9 @@ compares the URL a client method *builds* against the patterns any surface 7. **Deprecate `DEFAULT_DISPATCHER_ROUTES`**; point at the ledger. 8. **REST-surface tranche** (§8) with the same ledger+guard treatment — done in #3587. 9. **Autonomous service mounts** (§9) — done in #3636. -10. **Cross-surface URL conformance** (§9, the reverse direction) — #3642. +10. **Cross-surface URL conformance** (§10, the reverse direction) — done in #3642. +11. **Control-plane surface** (§10) — #3655, needs a ledger in the `cloud` repo. +12. **Enumerate `/auth/**`** (§10) — #3656, lowers the wildcard ratchet. Each gap closed must flip its ledger row to `sdk` and lower the ratchet bound in the conformance test — the guard enforces both directions from PR-1 onward. diff --git a/packages/client/src/client-url-conformance.test.ts b/packages/client/src/client-url-conformance.test.ts new file mode 100644 index 0000000000..8e617433f6 --- /dev/null +++ b/packages/client/src/client-url-conformance.test.ts @@ -0,0 +1,373 @@ +// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * Client-URL conformance — the capstone guard (#3642). + * + * WHAT THE OTHER FOUR GUARDS DO NOT ASK. The dispatcher (#3563), REST (#3587) + * and service-mount (#3636) ledgers all run server → client: enumerate what a + * surface mounts, demand a reviewed disposition, and for `sdk` rows demand the + * named client method exists. *The method exists* is not *the method can be + * called.* No guard compared the URL the client BUILDS against the patterns + * any server MOUNTS, so a client method could name a real function, carry a + * green ledger row, and still 404 everywhere. + * + * That gap shipped four times, found one at a time by hand: + * - `analytics.explain` called `/explain`; nothing served it (#3584) + * - `analytics.meta` called `/meta/:cube`; no server mounted it (#3584) + * - `meta.getView` sent `?type=`; REST mounts `/ui/view/:object/:type` (#3611) + * - `i18n.getTranslations` / `getFieldLabels` sent `?locale=` against + * path-param-only mounts (#3636) — both had carried a green `sdk` row + * since tranche 1. + * + * This suite closes the direction. It drives every method on a real client + * with a recording `fetch`, then matches each captured URL against the UNION + * of all four ledgers. A union, not an intersection: a route mounted by only + * one surface is still legitimately reachable. + * + * WHY A REAL DRIVE, NOT A DECLARATION. Asserting "method X targets route Y" in + * a table would be an assertion *about* the code that the code can drift away + * from — the same failure the audit keeps finding. Running the method and + * catching what it actually puts on the wire cannot drift. + * + * ANTI-ROT. The sweep's own completeness is the part that must not be skipped, + * so it is itself asserted: + * 1. Every method reachable on the client is either driven or carries an + * explicit skip reason (`NON_HTTP`) — a new method is a failure until + * someone classifies it. + * 2. A driven method that emits ZERO requests fails. That is how a sweep + * silently rots: the placeholder args stop satisfying the method, it + * throws before fetching, and the guard keeps passing while covering + * nothing. + * 3. A URL containing `undefined`/`[object Object]`/`NaN` fails, so a + * placeholder that is accepted but wrong cannot masquerade as coverage. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectStackClient } from './index'; +import { ROUTE_LEDGER } from '../../runtime/src/route-ledger'; +import { REST_ROUTE_LEDGER } from '../../rest/src/rest-route-ledger'; +import { STORAGE_ROUTE_LEDGER } from '../../services/service-storage/src/storage-route-ledger'; +import { I18N_ROUTE_LEDGER } from '../../services/service-i18n/src/i18n-route-ledger'; + +const BASE = 'http://localhost:9'; + +// --------------------------------------------------------------------------- +// 1. The ledger union → matchable patterns +// --------------------------------------------------------------------------- + +interface Pattern { verb: string; source: string; route: string; re: RegExp } + +/** + * `(unmatched)` is the `__api-endpoint` catch-all: metadata-declared custom + * endpoints, whose route set exists only at runtime. Treating it as a pattern + * would match every URL and make this whole suite vacuous, so it is excluded + * — the one ledger row this guard deliberately cannot use. + */ +const UNUSABLE_ROWS = new Set(['* (unmatched)']); + +function compile(route: string, prefix: string, source: string): Pattern[] { + const sp = route.indexOf(' '); + const verb = route.slice(0, sp); + const path = route.slice(sp + 1); + + const body = (prefix + path) + .split('/') + .map((seg) => { + if (seg === '**') return '.*'; + if (seg.startsWith(':') && seg.endsWith('?')) return null; // optional — handled below + if (seg.startsWith(':')) return '[^/]+'; + return seg.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); + }); + + // A trailing `:param?` makes the whole segment (with its slash) optional. + const optionalTail = body[body.length - 1] === null; + const head = (optionalTail ? body.slice(0, -1) : body).join('/'); + const src = `^${head}${optionalTail ? '(?:/[^/]+)?' : ''}$`; + + const pats: Pattern[] = [{ verb, source, route, re: new RegExp(src) }]; + + // Every surface additionally mirrors its routes under the environment scope + // when project scoping is on (rest-server registerRoutes, the dispatcher's + // scoped automation mounts) — that mirror is a mechanical duplication and is + // deliberately not re-ledgered, so it is generated here instead. + if (src.startsWith('^/api/v1')) { + pats.push({ + verb, + source: `${source} (env-scoped mirror)`, + route: route.replace('/api/v1', '/api/v1/environments/:environmentId'), + re: new RegExp(src.replace('^/api/v1', '^/api/v1/environments/[^/]+')), + }); + } + return pats; +} + +const PATTERNS: Pattern[] = [ + ...ROUTE_LEDGER.map((r) => r.route).filter((r) => !UNUSABLE_ROWS.has(r)).flatMap((r) => compile(r, '/api/v1', 'dispatcher')), + ...REST_ROUTE_LEDGER.map((r) => r.route).flatMap((r) => compile(r, '', 'rest')), + ...STORAGE_ROUTE_LEDGER.map((r) => r.route).flatMap((r) => compile(r, '', 'storage')), + ...I18N_ROUTE_LEDGER.map((r) => r.route).flatMap((r) => compile(r, '', 'i18n')), +]; + +function matches(verb: string, path: string): Pattern | undefined { + return PATTERNS.find((p) => (p.verb === '*' || p.verb === verb) && p.re.test(path)); +} + +/** + * The control plane. `/api/v1/cloud/*` is served by the sibling `cloud` repo, + * not by anything in this one — this repo's dispatcher explicitly REFUSES those + * paths (`http-dispatcher.ts`: "Guard against matching control-plane routes + * like /cloud/environments"). No in-repo ledger can vouch for them, so they are + * exempt here and tracked separately (#3655). + * + * Exempt by PREFIX, and bounded from both ends: the assertions below pin which + * methods are allowed to use it, so the hole cannot quietly widen into a place + * to park an unmatched URL. + */ +const CONTROL_PLANE = '/api/v1/cloud/'; +const CONTROL_PLANE_NAMESPACE = 'projects.'; + +// --------------------------------------------------------------------------- +// 2. The recorder +// --------------------------------------------------------------------------- + +interface Recorded { verb: string; url: string } + +/** + * Response permissive enough that a method reaches its LAST request rather + * than throwing at its first. Methods needing a sharper body carry an + * `expect`-shaped override in DRIVE below. + */ +function makeResponse(body: unknown): Response { + return { + ok: true, + status: 200, + statusText: 'OK', + headers: new Headers({ 'content-type': 'application/json' }), + json: async () => body, + text: async () => JSON.stringify(body), + blob: async () => new Blob([]), + arrayBuffer: async () => new ArrayBuffer(0), + clone() { return this as Response; }, + } as unknown as Response; +} + +function createRecordingClient(body: unknown) { + const calls: Recorded[] = []; + const client = new ObjectStackClient({ + baseUrl: BASE, + fetch: async (input: RequestInfo | URL, init?: RequestInit) => { + calls.push({ verb: (init?.method ?? 'GET').toUpperCase(), url: String(input) }); + return makeResponse(body); + }, + }); + return { client, calls }; +} + +// --------------------------------------------------------------------------- +// 3. The surface sweep +// --------------------------------------------------------------------------- + +/** Dotted paths of every callable reachable on a client instance. */ +function enumerateMethods(client: object): string[] { + const found: string[] = []; + const walk = (obj: Record, prefix: string, depth: number) => { + if (depth > 3) return; + for (const key of Object.keys(obj)) { + if (key.startsWith('_')) continue; + let value: unknown; + try { value = obj[key]; } catch { continue; } + const path = prefix ? `${prefix}.${key}` : key; + if (typeof value === 'function') found.push(path); + else if (value && typeof value === 'object' && !Array.isArray(value)) { + walk(value as Record, path, depth + 1); + } + } + }; + walk(client as Record, '', 0); + for (const key of Object.getOwnPropertyNames(Object.getPrototypeOf(client))) { + if (key === 'constructor' || key.startsWith('_')) continue; + const d = Object.getOwnPropertyDescriptor(Object.getPrototypeOf(client), key); + if (d && typeof d.value === 'function') found.push(key); + } + return found.sort(); +} + +/** + * Methods that legitimately put nothing on the wire. Every entry is a REASON, + * not a mute: a method parked here is claiming it makes no HTTP request, and + * assertion 2 below is what stops that claim from being used to hide a broken + * call — a `NON_HTTP` method that DOES fetch is itself a failure. + */ +const NON_HTTP: Record = { + 'fetch': 'the transport itself', + 'getRoute': 'pure route-table lookup', + 'unwrapResponse': 'pure envelope unwrap', + 'isFilterAST': 'pure type predicate', + 'project': 'constructs a ScopedProjectClient; its methods are swept separately', + 'setProjectId': 'local state', + 'getProjectId': 'local state', + 'setLocale': 'local state', + 'getLocale': 'local state', + 'fetchImpl': 'the injected transport', +}; + +/** + * Overrides for methods a bare placeholder cannot drive: those building the + * URL out of a FIELD of an object argument, and the composite flows whose + * later requests are addressed by the earlier responses. + * + * Kept deliberately small. Every entry is a place the generic driver stopped + * working, and the malformed/silent assertions below are what force one to be + * written rather than letting the method fall out of coverage. + */ +interface Drive { args?: unknown[]; body?: unknown; browser?: boolean } +const DRIVE: Record = { + 'auth.verifyEmail': { args: [{ token: 'tok' }] }, + // Guards on `window` and throws in node BEFORE fetching. It is a real HTTP + // method, so it gets a browser stub rather than a NON_HTTP exemption — + // parking it there would drop a live call out of coverage, which is the + // exact rot this suite exists to prevent. + 'auth.signInWithProvider': { args: ['github'], browser: true }, + 'storage.completeChunkedUpload': { args: [{ uploadId: 'up1', parts: [] }] }, + // The composite upload: presign → PUT the returned uploadUrl → commit. Hop 2 + // is addressed by hop 1's response, so the body has to carry a real target. + // Pointed at the local driver's loopback (a ledgered route) rather than an + // S3 URL, so all three hops stay in scope for the match below. + 'storage.upload': { + args: [{ name: 'a.png', type: 'image/png', size: 1 }, 'user'], + body: { + success: true, + data: { uploadUrl: `${BASE}/api/v1/storage/_local/raw/tok`, method: 'PUT', headers: {}, fileId: 'f1', expiresIn: 60 }, + }, + }, + // Resume reads progress, re-PUTs the outstanding chunks, then completes. + 'storage.resumeUpload': { + args: ['up1', new ArrayBuffer(8), 8, 'rtok'], + body: { success: true, data: { totalChunks: 1, uploadedChunks: 0, eTag: 'e1' } }, + }, +}; + +/** Placeholder positional args — enough for the id-in-path majority. */ +function autoArgs(fn: (...a: unknown[]) => unknown): unknown[] { + return Array.from({ length: fn.length }, (_, i) => `p${i + 1}`); +} + +/** + * Resolve a dotted path to its function AND its owner. Namespace methods are + * arrow functions that closed over `this`, but the top-level ones live on the + * prototype and lose `this` when plucked off by name — calling those unbound + * makes them throw before fetching, which would quietly read as "emits no + * request" and drop them from the sweep. + */ +function resolve(client: object, path: string): { fn: (...a: unknown[]) => unknown; owner: object } { + const keys = path.split('.'); + let owner: object = client; + for (const k of keys.slice(0, -1)) owner = (owner as Record)[k]; + return { fn: (owner as Record unknown>)[keys[keys.length - 1]], owner }; +} + +/** Run `body` with a minimal `window` in place, for browser-guarded methods. */ +async function withBrowser(enabled: boolean, body: () => Promise): Promise { + if (!enabled) return body(); + const g = globalThis as Record; + const had = 'window' in g; + const prev = g.window; + g.window = { location: { href: `${BASE}/app`, origin: BASE } }; + try { return await body(); } finally { if (had) g.window = prev; else delete g.window; } +} + +// --------------------------------------------------------------------------- + +describe('client URL conformance ↔ the union of all four route ledgers (#3642)', () => { + const probe = createRecordingClient({ success: true, data: {} }); + const METHODS = enumerateMethods(probe.client).filter((m) => !(m in NON_HTTP)); + + it('the ledger union compiles to a usable pattern set', () => { + expect(PATTERNS.length).toBeGreaterThan(100); + // Guard the guard: if `(unmatched)` ever slipped in, every URL would match + // and this suite would pass while asserting nothing. + expect(PATTERNS.some((p) => p.route.includes('(unmatched)'))).toBe(false); + }); + + it('every client method is classified — driven or explicitly non-HTTP', () => { + const all = enumerateMethods(probe.client); + const unclassified = all.filter((m) => !(m in NON_HTTP) && !METHODS.includes(m)); + expect( + unclassified, + `client methods neither driven nor declared NON_HTTP: ${unclassified.join(', ')}`, + ).toEqual([]); + expect(METHODS.length, 'the sweep should cover the whole SDK surface').toBeGreaterThan(150); + }); + + it('every URL the SDK builds matches a route some surface mounts', async () => { + const unmatched: string[] = []; + const silent: string[] = []; + const malformed: string[] = []; + const controlPlane: string[] = []; + const wildcardOnly: string[] = []; + + for (const name of METHODS) { + const drive = DRIVE[name] ?? {}; + const { client, calls } = createRecordingClient(drive.body ?? { success: true, data: {} }); + const { fn, owner } = resolve(client, name); + const args = drive.args ?? autoArgs(fn); + try { + await withBrowser(drive.browser === true, async () => fn.apply(owner, args)); + } catch { + // A throw after the request still counts — the URL is already recorded. + } + if (calls.length === 0) { silent.push(name); continue; } + + for (const call of calls) { + if (/undefined|\[object Object\]|NaN/.test(call.url)) { + malformed.push(`${name} → ${call.url}`); + continue; + } + const path = new URL(call.url, BASE).pathname; + if (path.startsWith(CONTROL_PLANE)) { controlPlane.push(`${name} → ${call.verb} ${path}`); continue; } + const hit = matches(call.verb, path); + if (!hit) { unmatched.push(`${name} → ${call.verb} ${path}`); continue; } + if (hit.route.includes('**')) wildcardOnly.push(`${name} → ${call.verb} ${path} (via ${hit.route})`); + } + } + + expect( + malformed, + 'placeholder args produced a malformed URL — give these an ARGS override so the sweep really covers them:\n' + + malformed.join('\n'), + ).toEqual([]); + + expect( + silent, + 'these methods emitted NO request, so the sweep does not cover them — fix the args ' + + 'via ARGS or declare them NON_HTTP with a reason:\n' + silent.join('\n'), + ).toEqual([]); + + expect( + unmatched, + 'SDK methods whose URL matches no route on ANY surface — these are wire-level 404s ' + + 'of the #3584 / #3611 / #3636 class:\n' + unmatched.join('\n'), + ).toEqual([]); + + // The control-plane hole, bounded from the other end: only `projects.*` may + // use it. Anything else reaching /api/v1/cloud/ is a method that has wandered + // off the data plane, and must not inherit this exemption. + const trespassers = controlPlane.filter((e) => !e.startsWith(CONTROL_PLANE_NAMESPACE)); + expect( + trespassers, + `non-projects methods targeting the control plane, which no in-repo ledger can vouch for:\n${trespassers.join('\n')}`, + ).toEqual([]); + expect(controlPlane.length, 'the projects namespace should still be reaching the control plane').toBeGreaterThan(0); + + // HOW STRONG IS THIS GUARD, HONESTLY. A `**` row asserts only that a prefix + // family is claimed, not that the specific URL resolves — `/auth/**` alone + // covers 26 SDK methods. Those matches are real but weak, so the count is + // ratcheted: enumerating a dynamic family (or dropping one) may lower it, + // and nothing may raise it without a deliberate decision. + expect( + wildcardOnly.length, + 'methods matched only by a wildcard `**` family — weaker evidence than an exact ' + + `route. Enumerate a dynamic family to lower this bound; do not raise it:\n${wildcardOnly.join('\n')}`, + ).toBeLessThanOrEqual(60); + }); +}); diff --git a/packages/runtime/src/route-ledger.ts b/packages/runtime/src/route-ledger.ts index 427d36361c..ae10cc07ef 100644 --- a/packages/runtime/src/route-ledger.ts +++ b/packages/runtime/src/route-ledger.ts @@ -27,8 +27,14 @@ * path pattern with a client method that exists; it does not check that the * client builds a URL any server actually mounts. The three `/i18n` rows below * looked healthy for exactly that reason while two of the three SDK methods - * spoke a `?locale=` dialect nothing routed (#3636). Cross-surface URL - * conformance is the capstone guard tracked in #3642. + * spoke a `?locale=` dialect nothing routed (#3636). That direction is covered + * since #3642 by `packages/client/src/client-url-conformance.test.ts`, which + * drives every SDK method and matches the URL it builds against the UNION of + * all four ledgers — so the `route` strings below are now load-bearing for the + * client half too, not just for dispatcher enumeration. Note what that guard + * can and cannot do with a `dynamic` row: `* /ai/**` and `* /auth/**` claim a + * prefix family, not a resolvable route, and 60 SDK methods match on nothing + * stronger than that (ratcheted in the guard; #3656 tracks enumerating them). * * This module is runtime-internal (not exported from the package index): it is * the guard's data, not public API. Promotion to `@objectstack/spec` is a