diff --git a/app/serve-studio.mjs b/app/serve-studio.mjs index d0ea9eb..917607e 100644 --- a/app/serve-studio.mjs +++ b/app/serve-studio.mjs @@ -20,7 +20,7 @@ import http from "node:http"; import fs from "node:fs"; import path from "node:path"; import { fileURLToPath } from "node:url"; -import { RUNTIME_OWNED_VERBS, SUBSTITUTABLE_VERBS, openVocabMap, lookup, verbMatches, resolveApi } from "./verbs.mjs"; +import { RUNTIME_OWNED_VERBS, SUBSTITUTABLE_VERBS, openVocabMap, lookup, verbMatches, matchApi } from "./verbs.mjs"; const HERE = path.dirname(fileURLToPath(import.meta.url)); const ROOT = path.resolve(HERE, ".."); @@ -1042,9 +1042,9 @@ function filesSearch(q) { // row matched, bounded by the row that registered it, and handlers receive // { searchParams, verb, rest } — never a pathname. // -// ROUTING lives in resolveApi (app/verbs.mjs), the one place a pathname meets -// this table. handleRequest hands over the raw URL and receives an execute -// closure; the pathname is OUT OF SCOPE downstream by construction, so "a +// ROUTING lives in matchApi (app/verbs.mjs), the one place a pathname meets +// this table. handleRequest hands over the raw URL and receives a table +// index; the pathname is OUT OF SCOPE downstream by construction, so "a // route cannot serve without a row" is a fact about the data flow. See // docs/design/routing-data-boundary.md for the load-bearing invariant. // @@ -1217,20 +1217,27 @@ const API_ROUTES = [ // <<< API ROUTING SURFACE const handleRequest = async (req, res) => { - // THE API FLOW HOLDS NO PATHNAME. Routing happens inside resolveApi (the + // THE API FLOW HOLDS NO PATHNAME. Routing happens inside matchApi (the // grammar module): this function hands over the raw request URL and receives - // either null (not the api namespace) or an execute closure that already - // carries its resolved row. Neither `u` nor the namespace prefix exists as a + // either null (not the api namespace) or a table INDEX plus request data — + // never a closure, never a status. Neither `u` nor the namespace prefix exists as a // binding anywhere in this flow — a pathname special-case here is a // ReferenceError, not a code-review question. THE ORDERING BELOW IS // LOAD-BEARING: the static-flow URL is deliberately constructed AFTER this // branch returns, so no code that could see an api request ever holds a // pathname. See docs/design/routing-data-boundary.md before "simplifying" // either fact away. - const api = resolveApi(req.url, `http://127.0.0.1:${PORT}`, API_ROUTES, sendJson); - if (api) { - try { await api.execute(req, res); } - catch (e) { return sendJson(res, 500, { ok: false, error: String(e.message || e) }); } + const m = matchApi(req.url, `http://127.0.0.1:${PORT}`, API_ROUTES); + if (m) { + try { + // TABLE MEMBERSHIP IS THE ONLY DOOR TO A HANDLER, and 404 is the only + // status the miss branch can have: the resolver returns an index and a + // body object — data — never a closure and never a status. See the + // resolver-boundary discriminator in test/reserved-verbs.test.mjs. + const row = API_ROUTES[m.index]; + if (row) await row.handler({ searchParams: m.searchParams, verb: row.verb, rest: m.rest }, req, res); + else sendJson(res, 404, m.notFound); + } catch (e) { return sendJson(res, 500, { ok: false, error: String(e.message || e) }); } return; } const u = new URL(req.url, `http://127.0.0.1:${PORT}`); diff --git a/app/verbs.mjs b/app/verbs.mjs index d195842..ee8bfe7 100644 --- a/app/verbs.mjs +++ b/app/verbs.mjs @@ -81,31 +81,38 @@ export const isRuntimeOwned = (pathname) => // // THE DATA BOUNDARY (PM's ratified criterion, round 5): after the table lookup // resolves a row, the raw pathname and the namespace prefix are OUT OF SCOPE -// for everything downstream. The runtime's api flow never constructs a URL at -// all — it hands this function the raw request URL and receives back either -// null (not this namespace) or an `execute` closure. The row branch closes over -// a request object carrying ONLY { searchParams, verb, rest }; the miss branch -// closes over the pathname internally for its 404 message. So a pathname -// special-case cannot be WRITTEN downstream: there is no pathname there to -// branch on. That is a fact about the data flow, not a claim a guard checks — -// the guards that remain (the literal fence, the unwritability control) police -// the residue, which is this function itself: the definition of routing, where -// any special case must now be written in plain sight and where the -// routes=[] property test sees it. +// for everything downstream — the runtime's api flow never constructs a URL at +// all. AND (round 6, sdk-qa): the path-bearing side holds NO RESPONSE +// AUTHORITY. The previous shape returned an `execute` closure and took a +// respond callback, so a resolver-internal special case could mint a 200 +// before the table lookup — the trusted region had moved in here. Now this +// function returns ONLY DATA: +// +// null — not the api namespace +// { index, rest, searchParams } — a TABLE INDEX, never a handler +// { index: -1, notFound } — a 404 BODY object, never a status +// +// The executable is fetched downstream as `routes[m.index]` — table membership +// is the only door to a handler — and the miss status is HARDCODED 404 at the +// single dispatch site. A special case written in here cannot mint a closure +// that runs (unknown fields are inert downstream), cannot mint a success +// status (there is no status channel), and cannot reach a handler that is not +// a row. What it CAN still do is the function's charter and its stated +// residue: return a different DECLARED row's index — a wrong routing decision +// among enumerated rows, written into the definition of routing where the +// resolver-boundary discriminator and review see it. export const API_PREFIX = "/api/"; -export function resolveApi(rawUrl, base, routes, respond) { +export function matchApi(rawUrl, base, routes) { const u = new URL(rawUrl, base); if (!u.pathname.startsWith(API_PREFIX)) return null; - const row = routes.find((r) => verbMatches(r.verb, u.pathname)); - if (!row) { - const pathname = u.pathname; - return { execute: (req, res) => respond(res, 404, { ok: false, - error: `no such contract route: ${pathname} — see GET /api/contract for capabilities and contract/runtime-api.md for the verb set` }) }; + const index = routes.findIndex((r) => verbMatches(r.verb, u.pathname)); + if (index === -1) { + return { index: -1, notFound: { ok: false, + error: `no such contract route: ${u.pathname} — see GET /api/contract for capabilities and contract/runtime-api.md for the verb set` } }; } - const request = { searchParams: u.searchParams, verb: row.verb, rest: u.pathname.slice(row.verb.length) }; - return { execute: (req, res) => row.handler(request, req, res) }; + return { index, rest: u.pathname.slice(routes[index].verb.length), searchParams: u.searchParams }; } // ---- the routing surface tripwire ------------------------------------------ diff --git a/docs/design/routing-data-boundary.md b/docs/design/routing-data-boundary.md index 35c1d2d..145f4a2 100644 --- a/docs/design/routing-data-boundary.md +++ b/docs/design/routing-data-boundary.md @@ -11,14 +11,22 @@ prefix are out of scope for everything downstream.** In this codebase that is implemented more strongly: the runtime's API flow in `app/serve-studio.mjs` never holds a pathname at all. -- Routing happens in exactly one place: `resolveApi()` in `app/verbs.mjs`. It +- Routing happens in exactly one place: `matchApi()` in `app/verbs.mjs`. It receives the raw request URL, constructs the URL object, matches the table, - and returns an `execute` closure. + and returns **data only**: a table index plus `{ rest, searchParams }`, or + `{ index: -1, notFound }` where `notFound` is a 404 *body object*. +- **The path-bearing side holds no response authority** (round 6, sdk-qa). An + earlier shape returned an `execute` closure and took a respond callback — so + a resolver-internal special case could mint a 200 before the table lookup. + Now the resolver cannot mint a closure that runs (unknown fields are inert + downstream), cannot set a status (the miss branch is hardcoded 404 at the + dispatch site), and cannot reach a handler that is not a row (the executable + is fetched as `API_ROUTES[m.index]` — table membership is the only door). - Handlers receive `{ searchParams, verb, rest }` — never a pathname, never a URL object. A handler needing path data uses `rest`, which is already scoped to its own matched verb. -- `handleRequest` calls `resolveApi` and executes the result. Neither `u` nor - `API_PREFIX` is a binding anywhere in that flow. +- `handleRequest` calls `matchApi` and dispatches the indexed row. Neither `u` + nor `API_PREFIX` is a binding anywhere in that flow. - The static-file flow's `const u = new URL(...)` is constructed **after** the API branch has returned. That ordering is load-bearing: it is what guarantees no code that can see an API request ever holds a pathname. @@ -60,17 +68,32 @@ to remove it: round-4 literal arm and round-5 derived-prefix arm at the top of the API flow and asserts each detonates as a `ReferenceError` naming the absent binding — and above all, never serves. -- **Property tests on `resolveApi`**: with `routes=[]` every API path misses; - with one route, exactly its matches resolve; the resolved request carries no - pathname field. +- **Resolver-boundary discriminator** (`test/reserved-verbs.test.mjs`): plants + special cases *inside* `matchApi` and asserts every response channel is dead — + a minted `{ execute }` is inert and never runs, a minted status loses to the + hardcoded 404, and nothing a resolver special case does can be enumerated. +- **Property tests on `matchApi`**: with `routes=[]` every API path misses; + with one route, exactly its matches resolve; the match result carries no + pathname field and no function-valued field. - **The literal fence** (`routingSurfaceViolations`): every textual `/api` mention in `serve-studio.mjs` lives inside the marked data-only surface. - **Bypass control**: raw arms planted in the static flow demonstrably cannot serve API requests (404), in all the spellings sdk-qa used across rounds. +## The stated residue + +The resolver's remaining freedom is exactly its charter: choosing which +declared row a path maps to. A special case written into `matchApi` can +mis-route a path to a *different declared row's* index — the discriminator pins +this blast radius (the served response is a declared row's; nothing new is +enumerated) — but it can never produce a response that is not a declared +row's. A wrong routing decision must be written into the eight-line definition +of routing, where review and the `routes=[]` property test see it. + ## If you need to change this -If a future feature genuinely needs path data downstream, extend the resolved -`request` (e.g., a richer `rest`) inside `resolveApi` — do not reintroduce the -pathname. If you believe the boundary itself must move, that is a PM-level -design change: say so in the commit and update this note in the same change. +If a future feature genuinely needs path data downstream, extend the match +result (e.g., a richer `rest`) inside `matchApi` — do not reintroduce the +pathname, and never return a function or a status. If you believe the boundary +itself must move, that is a PM-level design change: say so in the commit and +update this note in the same change. diff --git a/test/reserved-verbs.test.mjs b/test/reserved-verbs.test.mjs index f57529f..e782915 100644 --- a/test/reserved-verbs.test.mjs +++ b/test/reserved-verbs.test.mjs @@ -29,7 +29,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { spawn } from "node:child_process"; import { RUNTIME_OWNED_VERBS, SUBSTITUTABLE_VERBS, openVocabMap, lookup, - routingSurfaceViolations, verbMatches, isRuntimeOwned, resolveApi } from "../app/verbs.mjs"; + routingSurfaceViolations, verbMatches, isRuntimeOwned, matchApi } from "../app/verbs.mjs"; const REPO = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const read = (...p) => fs.readFileSync(path.join(REPO, ...p), "utf8"); @@ -134,11 +134,11 @@ test("the safe primitive actually resists prototype keys", () => { // Boot a mutated copy of the runtime and probe it. Shared by the two controls // below, which differ only in WHAT they plant. -async function bootMutated(plantedSrc, probes) { +async function bootMutated(plantedSrc, probes, file = "serve-studio.mjs") { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "routes-ctl-")); fs.cpSync(path.join(REPO, "app"), path.join(dir, "app"), { recursive: true }); fs.cpSync(path.join(REPO, "fixtures"), path.join(dir, "fixtures"), { recursive: true }); - fs.writeFileSync(path.join(dir, "app", "serve-studio.mjs"), plantedSrc); + fs.writeFileSync(path.join(dir, "app", file), plantedSrc); const port = 9960 + Math.floor(Math.random() * 30); const proc = spawn(process.execPath, [path.join(dir, "app", "serve-studio.mjs"), "--port", String(port)], { cwd: dir, stdio: ["ignore", "pipe", "pipe"] }); @@ -147,10 +147,14 @@ async function bootMutated(plantedSrc, probes) { try { await fetch(`http://127.0.0.1:${port}/api/contract`, { cache: "no-store" }); break; } catch { await new Promise((r) => setTimeout(r, 50)); } } - const statuses = {}; - for (const p of probes) statuses[p] = (await fetch(`http://127.0.0.1:${port}${p}`, { cache: "no-store" })).status; + const statuses = {}, bodies = {}; + for (const p of probes) { + const r = await fetch(`http://127.0.0.1:${port}${p}`, { cache: "no-store" }); + statuses[p] = r.status; + bodies[p] = await r.text(); + } const c = await (await fetch(`http://127.0.0.1:${port}/api/contract`, { cache: "no-store" })).json(); - return { statuses, routes: c.runtime.routes }; + return { statuses, bodies, routes: c.runtime.routes }; } finally { proc.kill(); fs.rmSync(dir, { recursive: true, force: true }); } } @@ -241,7 +245,7 @@ test("UNWRITABILITY CONTROL: sdk-qa's derived-prefix arm at the ordinary place i // naming the missing binding. If either ever boots again, a pathname has // been reintroduced into the api flow and the data boundary is gone. const src = read("app", "serve-studio.mjs"); - const ordinary = /const api = resolveApi\(req\.url[^\n]*\n/; + const ordinary = /const m = matchApi\(req\.url[^\n]*\n/; assert.match(src, ordinary, "the api flow entry moved — re-anchor this control before trusting it"); const plants = [ @@ -289,35 +293,102 @@ test("UNWRITABILITY CONTROL: sdk-qa's derived-prefix arm at the ordinary place i } }); -test("routing is exactly the table: routes=[] misses everything, routes=[X] resolves exactly X", async () => { +test("routing is exactly the table: routes=[] misses everything, routes=[X] resolves exactly X", () => { // The property test the pure function enables — routing asserted against its // DEFINITION rather than against source text. A special case smuggled into - // resolveApi that does not consult the table would let some path resolve - // with an EMPTY table, which is the first assertion here. + // matchApi that does not consult the table would let some path resolve with + // an EMPTY table, which is the first assertion here. const BASE = "http://127.0.0.1:9999"; - const sent = []; - const respond = (res, code, obj) => sent.push({ code, obj }); for (const p of ["/api/focus", "/api/contract", "/api/anything/else", "/api/qa-derived"]) { - const decision = resolveApi(p, BASE, [], respond); - assert.ok(decision, `routes=[]: '${p}' left the api namespace unclaimed`); - decision.execute(null, null); - assert.equal(sent.at(-1).code, 404, `routes=[]: '${p}' resolved despite an empty table`); + const m = matchApi(p, BASE, []); + assert.ok(m, `routes=[]: '${p}' left the api namespace unclaimed`); + assert.equal(m.index, -1, `routes=[]: '${p}' resolved despite an empty table`); + assert.match(m.notFound.error, /no such contract route/); } - assert.equal(resolveApi("/not-api", BASE, [], respond), null, "a non-api path was claimed"); + assert.equal(matchApi("/not-api", BASE, []), null, "a non-api path was claimed"); - let got = null; - const X = [{ verb: "/api/export-status/", handler: (request) => { got = request; } }]; - const hit = resolveApi("/api/export-status/42?a=b", BASE, X, respond); - await hit.execute(null, null); - assert.equal(got.verb, "/api/export-status/"); - assert.equal(got.rest, "42", "the prefix remainder is not handed to the handler as rest"); - assert.equal(got.searchParams.get("a"), "b"); - assert.equal(got.pathname, undefined, - "the resolved request carries a pathname — the data boundary leaks the thing it exists to remove"); - const miss = resolveApi("/api/export-status", BASE, X, respond); - miss.execute(null, null); - assert.equal(sent.at(-1).code, 404, "an exact path matched a prefix verb without the trailing slash"); + const X = [{ verb: "/api/export-status/", handler: () => {} }]; + const hit = matchApi("/api/export-status/42?a=b", BASE, X); + assert.equal(hit.index, 0); + assert.equal(hit.rest, "42", "the prefix remainder is not returned as rest"); + assert.equal(hit.searchParams.get("a"), "b"); + assert.equal(hit.pathname, undefined, + "the match result carries a pathname — the data boundary leaks the thing it exists to remove"); + assert.equal(matchApi("/api/export-status", BASE, X).index, -1, + "an exact path matched a prefix verb without the trailing slash"); + + // AND THE RESULT IS DATA, NOT AUTHORITY (round 6): no closure to run, no + // status to set. A resolver special case has neither channel. + for (const m of [matchApi("/api/focus", BASE, X), matchApi("/api/export-status/1", BASE, X)]) { + for (const v of Object.values(m)) { + assert.notEqual(typeof v, "function", + "matchApi returned a function — the path-bearing side has response authority again"); + } + } +}); + +test("RESOLVER-BOUNDARY DISCRIMINATOR: a special case inside matchApi cannot mint a public response", async () => { + // sdk-qa round 6: the trusted region had moved into the resolver — it held + // the pathname AND a respond callback AND returned executable closures, so a + // pre-lookup special case could mint a 200 that appeared in no enumeration + // (reproduced on 6fc145b: status 200, minted:true, absent from routes). + // Now the resolver returns DATA and this control plants special cases INSIDE + // it — at the precise boundary — and asserts each channel is dead: + // closure-mint: an { execute } return is INERT — downstream never calls it; + // status-mint: there is no status channel — the miss branch is hardcoded + // 404 at the dispatch site, whatever the resolver returns. + // The genuine registered routes stay the positive control (every other test + // in this file). THE STATED RESIDUE, asserted for exactly what it is: the + // resolver can still return a DIFFERENT DECLARED row's index — a wrong + // routing decision confined to enumerated rows, which is the routing + // definition's charter; what it can never do again is produce a response + // that is not a declared row's. + const src = read("app", "verbs.mjs"); + const anchor = " const index = routes.findIndex((r) => verbMatches(r.verb, u.pathname));"; + assert.ok(src.includes(anchor), "matchApi's lookup moved — re-anchor this control before trusting it"); + + // closure-mint: the round-6 plant, verbatim shape. Must be inert: not 200, + // the canary body never served, the path absent from routes. + { + const planted = src.replace(anchor, + ' if (u.pathname === API_PREFIX + "qa-resolver-mint") return { execute: (req, res) => { throw new Error("canary"); }, minted: true };\n' + anchor); + assert.notEqual(planted, src, "closure-mint plant did not apply"); + const { statuses, bodies, routes } = await bootMutated(planted, ["/api/qa-resolver-mint"], "verbs.mjs"); + assert.notEqual(statuses["/api/qa-resolver-mint"], 200, + "a resolver-minted closure SERVED — the path-bearing side has response authority again"); + assert.ok(!bodies["/api/qa-resolver-mint"].includes("canary"), "the minted closure ran"); + assert.ok(!routes.some((v) => v.includes("qa-resolver-mint")), "the minted route was enumerated"); + } + + // status-mint: a crafted miss with a success-shaped body and a code field. + // The dispatch site's hardcoded 404 must win. + { + const planted = src.replace(anchor, + ' if (u.pathname === API_PREFIX + "qa-status-mint") return { index: -1, notFound: { ok: true, minted: true }, code: 200 };\n' + anchor); + assert.notEqual(planted, src, "status-mint plant did not apply"); + const { statuses } = await bootMutated(planted, ["/api/qa-status-mint"], "verbs.mjs"); + assert.equal(statuses["/api/qa-status-mint"], 404, + "the resolver set a response status — the miss branch is no longer pinned to 404"); + } + + // misbind: the stated residue, pinned to its exact blast radius — the + // response served is a DECLARED row's (the contract handler's), reported + // routes are unchanged, and the misbound path itself is not enumerated. + { + const src2 = read("app", "serve-studio.mjs"); + const rows = [...src2.matchAll(/\{ verb: (?:"[^"]+"|CONTRACT_VERB)/g)]; + const contractIndex = rows.findIndex((m2) => m2[0].includes("CONTRACT_VERB")); + assert.ok(contractIndex >= 0, "could not locate the contract row's index in the table"); + const planted = src.replace(anchor, + ` if (u.pathname === API_PREFIX + "qa-misbind") return { index: ${contractIndex}, rest: "", searchParams: u.searchParams };\n` + anchor); + assert.notEqual(planted, src, "misbind plant did not apply"); + const { statuses, bodies, routes } = await bootMutated(planted, ["/api/qa-misbind"], "verbs.mjs"); + assert.equal(statuses["/api/qa-misbind"], 200, "the misbind residue changed shape — re-derive this control"); + assert.ok(bodies["/api/qa-misbind"].includes("contractVersion"), + "misbind served something that is NOT a declared row's response — a new channel exists"); + assert.ok(!routes.some((v) => v.includes("qa-misbind")), "the misbound path was enumerated"); + } }); test("the runtime source keeps every /api mention inside the routing surface", () => {