diff --git a/.changeset/24228-actions-every-verb-anonymous-floor.md b/.changeset/24228-actions-every-verb-anonymous-floor.md new file mode 100644 index 00000000000..dd2302352ec --- /dev/null +++ b/.changeset/24228-actions-every-verb-anonymous-floor.md @@ -0,0 +1,14 @@ +--- +'@objectstack/runtime': patch +--- + +fix(runtime): on the standalone server, every verb on `/api/v1/actions/...` reaches the `actions` domain, so an anonymous caller is answered `401`; and an `Allow` header no longer lists verbs a route mounts only to refuse + +Clause-②: no + +On the standalone server (`os dev`, `os serve`: `HonoServerPlugin` with the dispatcher plugin), a request that matched no route under its verb was answered by the HTTP adapter: `405 METHOD_NOT_ALLOWED` with an `Allow` header built from the verbs registered on the path. Two answers were wrong. + +- **`/api/v1/actions/...` (all three path shapes, unscoped and `/environments/:environmentId`-scoped).** Only `POST` is mounted, so an anonymous `GET`, `HEAD`, `PUT`, `PATCH`, `DELETE` or `TRACE` was answered `405` + `Allow: POST` instead of the published `401 UNAUTHENTICATED`. Every such request now reaches the `actions` domain, as it does on a `createHonoApp` host: an anonymous caller gets `401 UNAUTHENTICATED` with no `Allow`, and a signed-in caller gets the domain's `405` + `Allow: POST`. An anonymous `OPTIONS` (only reachable with CORS off) gets the domain's `405` + `Allow: POST` on both hosts, because the shared anonymous decision lets a CORS preflight through. +- **`Allow` on `/api/v1/keys`, the three `/api/v1/notifications` faces and `/api/v1/mcp/skill`.** These paths mount verbs that exist only so the domain can refuse them. A verb that cannot be mounted (`OPTIONS` with CORS off, `TRACE`, an extension method), and `PUT`/`PATCH` on `/mcp/skill`, got the adapter's `405` with those refusal-only verbs listed as supported (`/keys`: `Allow: DELETE, GET, HEAD, PATCH, POST, PUT`). They now reach the domain too: `/keys` answers `401` anonymous and `405` + `Allow: POST` signed in, `/notifications` answers `401` anonymous and `404 ROUTE_NOT_FOUND` signed in, and `/mcp/skill` answers `405` + `Allow: GET`. + +Every request that was served is served as before, and no route is added: the dispatcher's existing fallback seam hands these requests to the domain. A transport that implements `setFallbackHandler` but not `resolveMountedRoute` keeps the adapter's answer, and the dispatcher says so once at boot, at `warn`. diff --git a/packages/runtime/src/actions-every-verb.hono.integration.test.ts b/packages/runtime/src/actions-every-verb.hono.integration.test.ts new file mode 100644 index 00000000000..c0ae9251afe --- /dev/null +++ b/packages/runtime/src/actions-every-verb.hono.integration.test.ts @@ -0,0 +1,214 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { request as httpRequest } from 'node:http'; +import { LiteKernel, Plugin, PluginContext } from '@objectstack/core'; +import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; +import type { IHttpServer } from '@objectstack/spec/contracts'; + +import { createDispatcherPlugin } from './dispatcher-plugin.js'; + +/** + * Every verb on `/api/v1/actions/...` reaches the `actions` domain on the + * standalone server, so its anonymous baseline answers first (#24228) — over + * a real Hono socket. + * + * `handleActionsRequest` decides an anonymous caller as its FIRST statement, + * ahead of its `405` (ADR-0138 D2: "Everything else answers 401 + * (UNAUTHENTICATED). The decision is taken once per domain, as the domain's + * first statement"). The standalone dispatcher mounts the three `/actions` + * paths for `POST` only, so every other verb matched no route and the + * transport answered it itself: `405` + `Allow: POST`, to an anonymous caller + * too. A `createHonoApp` host answers the same request `401`, because its + * catch-all hands the domain every verb. + * + * The dispatcher now declares these paths as ones whose domain decides every + * verb, and its fallback seam hands the domain each request the router could + * not route there. Deliberately NOT refusal-only `GET`/`PUT`/`PATCH`/`DELETE` + * mounts (the `/keys` shape): the transport builds `Allow` from the verbs + * REGISTERED on a path, so each such mount would have been listed as supported + * in the `405` the transport still answers for the verbs left over (`OPTIONS` + * with CORS off, `TRACE`, …). The signed-in rows therefore pin `Allow` too. + * + * Which DOOR answered is read off the body, as in + * `keys-every-verb.hono.integration.test.ts`: the dispatcher's envelope fills + * `error.httpStatus` and carries no `error.details`; the transport's fills + * `error.details.{method,path,allowed}` and no `httpStatus`. The signed-in + * rows answered `405` + `Allow: POST` before the fix as well, so the door is + * what tells them apart. + * + * Both prefixes: `enableProjectScoping` mounts the three paths under + * `/api/v1` and under `/api/v1/environments/:environmentId`, through the same + * `registerActionRoutes`, and the scoped request reaches `dispatch()` with its + * environment read off the path rather than off route params (an unrouted + * request has none) — so the scoped rows are not a copy of the unscoped ones. + * + * Raw `node:http`, not `fetch`: `fetch` refuses `TRACE`, and `TRACE` is the + * verb the transport answers even with CORS on (CORS answers `OPTIONS`). + * `LiteKernel`, no `objectql`: every answer under test comes before the domain + * touches storage, so the signed-in `POST` (a real invocation) is not driven. + */ + +const SESSION_HEADER = 'x-test-session'; +const PREFIXES = ['/api/v1', '/api/v1/environments/env_actions_verb'] as const; +const SHAPES = ['/actions//close_all', '/actions/task/close', '/actions/task/close/rec_1'] as const; +/** Verbs that carry a JSON body on the wire; the rest are sent bodiless. */ +const BODY_VERBS = new Set(['PUT', 'PATCH']); +/** Every verb but `POST` and the two whose answer differs in shape (below). */ +const REFUSED_VERBS = ['GET', 'PUT', 'PATCH', 'DELETE', 'TRACE'] as const; + +function fakeAuthPlugin(): Plugin { + return { + name: 'com.objectstack.test.fake-auth-actions-every-verb', + version: '1.0.0', + init: async (ctx: PluginContext) => { + ctx.registerService('auth', { + api: { + getSession: async ({ headers }: any) => + (headers?.get?.(SESSION_HEADER) ? { user: { id: 'u_actions_verb' } } : undefined), + }, + }); + }, + }; +} + +interface Answer { + status: number; + allow: string | null; + /** Parsed JSON body; `undefined` for a bodiless answer (`HEAD`). */ + body: Record | undefined; +} + +describe('every /api/v1/actions verb reaches the actions domain (integration, #24228)', () => { + let kernel: LiteKernel; + let port: number; + + beforeAll(async () => { + kernel = new LiteKernel(); + kernel.use(fakeAuthPlugin()); + // port 0 → an OS-assigned free port, read back through getPort(). + // CORS off: with it on, CORS answers every `OPTIONS` before routing. + kernel.use(new HonoServerPlugin({ port: 0, cors: false })); + kernel.use(createDispatcherPlugin({ + prefix: '/api/v1', + securityHeaders: false, + scoping: { enableProjectScoping: true, projectResolution: 'auto' }, + })); + await kernel.bootstrap(); + port = kernel.getService('http.server').getPort!(); + }, 30_000); + + afterAll(async () => { + if (kernel) { + await Promise.race([ + kernel.shutdown(), + new Promise((resolve) => setTimeout(resolve, 10_000)), + ]); + } + }, 30_000); + + function call(method: string, path: string, signedIn: boolean): Promise { + return new Promise((resolve, reject) => { + const payload = BODY_VERBS.has(method) ? JSON.stringify({ params: {} }) : undefined; + const req = httpRequest({ + host: '127.0.0.1', + port, + method, + path, + headers: { + ...(payload ? { 'content-type': 'application/json' } : {}), + ...(signedIn ? { [SESSION_HEADER]: '1' } : {}), + }, + }, (res) => { + let text = ''; + res.setEncoding('utf8'); + res.on('data', (chunk) => { text += chunk; }); + res.on('end', () => { + const allow = res.headers.allow; + resolve({ + status: res.statusCode ?? 0, + allow: typeof allow === 'string' ? allow : null, + body: text.length > 0 ? JSON.parse(text) as Record : undefined, + }); + }); + }); + req.on('error', reject); + req.end(payload); + }); + } + + for (const prefix of PREFIXES) { + for (const shape of SHAPES) { + const path = `${prefix}${shape}`; + + // ── the defect: an anonymous non-POST verb is refused 401, by the domain ── + for (const method of REFUSED_VERBS) { + it(`anonymous ${method} ${path} → 401 UNAUTHENTICATED, no Allow, in the dispatcher's envelope`, async () => { + const { status, allow, body } = await call(method, path, false); + + expect(status).toBe(401); + expect(allow).toBeNull(); + expect(body?.success).toBe(false); + expect(body?.error?.code).toBe('UNAUTHENTICATED'); + expect(body?.error?.httpStatus).toBe(401); + expect(body?.error).not.toHaveProperty('details'); + }); + } + + // ── signed in: the DOMAIN's 405, naming only the verb it serves ── + for (const method of REFUSED_VERBS) { + it(`signed-in ${method} ${path} → the actions domain's 405 with \`Allow: POST\`, not the transport's`, async () => { + const { status, allow, body } = await call(method, path, true); + + expect(status).toBe(405); + expect(allow).toBe('POST'); + expect(body?.error?.code).toBe('METHOD_NOT_ALLOWED'); + // The door: the domain fills `httpStatus`; the transport fills `details`. + expect(body?.error?.httpStatus).toBe(405); + expect(body?.error).not.toHaveProperty('details'); + }); + } + + // HEAD has no body to read the door from; status and header say it. + it(`HEAD ${path} → 401 with no Allow anonymously, 405 with \`Allow: POST\` signed in`, async () => { + const anonymous = await call('HEAD', path, false); + expect(anonymous.status).toBe(401); + expect(anonymous.allow).toBeNull(); + + const signedIn = await call('HEAD', path, true); + expect(signedIn.status).toBe(405); + expect(signedIn.allow).toBe('POST'); + }); + + // `shouldDenyAnonymous` lets every `OPTIONS` through (a CORS preflight + // carries no credentials), so the domain's 405 answers it — on a + // `createHonoApp` host with CORS off as well. Pinned as the boundary: + // the domain answers it, and `Allow` names `POST` alone. + it(`anonymous OPTIONS ${path} (CORS off) → the domain's 405 with \`Allow: POST\` — the preflight exemption`, async () => { + const { status, allow, body } = await call('OPTIONS', path, false); + + expect(status).toBe(405); + expect(allow).toBe('POST'); + expect(body?.error?.httpStatus).toBe(405); + expect(body?.error).not.toHaveProperty('details'); + }); + + // ── control: the verb the route always served is answered as before ── + it(`anonymous POST ${path} → 401 UNAUTHENTICATED (unchanged)`, async () => { + const { status, body } = await call('POST', path, false); + + expect(status).toBe(401); + expect(body?.error?.code).toBe('UNAUTHENTICATED'); + expect(body?.error?.httpStatus).toBe(401); + }); + } + } + + // ── the claim is narrow: a path no `/actions` route serves stays the transport's 404 ── + it('a four-segment /actions path no route serves → the transport\'s 404, not the domain', async () => { + const { status, body } = await call('GET', '/api/v1/actions/task/close/rec_1/extra', false); + + expect(status).toBe(404); + expect(body?.error?.code).toBe('ENDPOINT_NOT_FOUND'); + }); +}); diff --git a/packages/runtime/src/dispatcher-plugin.fallback-absence-warn.test.ts b/packages/runtime/src/dispatcher-plugin.fallback-absence-warn.test.ts index 8d84da97a2e..888bd59553d 100644 --- a/packages/runtime/src/dispatcher-plugin.fallback-absence-warn.test.ts +++ b/packages/runtime/src/dispatcher-plugin.fallback-absence-warn.test.ts @@ -166,3 +166,37 @@ describe('dispatcher declarative-endpoint seam — absence is loud (#5400)', () .toBe(true); }, 60_000); }); + +/** + * [#24228] The fallback seam also hands a domain that decides every verb + * (`/actions`, `/keys`, `/notifications`, `/mcp/skill`) the requests its + * router could not route on that domain's paths. Which paths those are is the + * router's verdict, asked through `IHttpServer.resolveMountedRoute` — OPTIONAL + * on the contract like the seam itself. A transport that has the seam but not + * the resolver answers those requests with its own 405, so its absence is + * announced the same way, once, at `warn`: a functional gap (the anonymous + * floor and the domain's `Allow` are skipped), nothing persisted. + */ +describe('dispatcher domain-decided verbs — a missing route resolver is loud (#24228)', () => { + const resolverLines = (logs: LogLine[]) => logs.filter((l) => l.message.includes('resolveMountedRoute')); + + it('announces a seam without `resolveMountedRoute` once, at `warn`, naming the consequence and the remedy', async () => { + const logs = await bootWith(makeSeamedServer()); + + const lines = resolverLines(logs); + expect(lines).toHaveLength(1); + expect(lines[0].level).toBe('warn'); + expect(lines[0].message).toMatch(/\/actions/); + expect(lines[0].message).toMatch(/instead of 401/); + expect(lines[0].message).toMatch(/`Allow`/); + expect(lines[0].message).toMatch(/@objectstack\/plugin-hono-server/); + }, 60_000); + + it('stays SILENT when the adapter exposes both the seam and the resolver', async () => { + const server = makeSeamedServer(); + server.resolveMountedRoute = () => undefined; + const logs = await bootWith(server); + + expect(resolverLines(logs)).toEqual([]); + }, 60_000); +}); diff --git a/packages/runtime/src/dispatcher-plugin.ts b/packages/runtime/src/dispatcher-plugin.ts index 0136bea52b1..24648a251fc 100644 --- a/packages/runtime/src/dispatcher-plugin.ts +++ b/packages/runtime/src/dispatcher-plugin.ts @@ -1084,6 +1084,40 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu }, }) as IHttpServer; + // ── Paths whose domain decides EVERY verb (#24228) ────────── + // `IHttpServer` can register five verbs. A request under any other + // verb — `OPTIONS` when the transport's CORS is off, `TRACE`, an + // extension method — and under any of the five a path does not + // mount, matches no route, and the transport answers it itself: + // `405`, with an `Allow` built from the verbs REGISTERED on the + // path. That answer is wrong twice for a path whose domain decides + // every verb (ADR-0138 D2: the anonymous decision is the domain's + // first statement, taken once per domain; RFC 9110 §15.5.6: `Allow` + // names the SUPPORTED methods): + // 1. an anonymous caller is told the route's shape instead of 401, + // because the domain never ran (`/actions` mounts `POST` only); + // 2. a verb mounted only so the domain could refuse it is listed as + // supported (`/keys` answered `OPTIONS` with + // `Allow: DELETE, GET, HEAD, PATCH, POST, PUT`; it serves `POST`). + // So such a path is declared here, by the route that serves it, and + // the fallback seam below hands every request the router could not + // route on it to `dispatch()` — the domain answers, exactly as it + // does on a `createHonoApp` host, whose catch-all hands it every + // verb. Refusal-only MOUNTS are not the answer for `/actions`: each + // one would be one more verb the transport lists in `Allow`. + // + // A declaration is the serving route's verb and its pattern, spelled + // as it was registered. The fallback asks the transport's own router + // which registered route a path resolves to (`resolveMountedRoute`), + // so whether a path belongs to a declared route is the router's + // verdict, never a second matcher written here. + const domainDecidedRoutes = new Map<'GET' | 'POST', Set>(); + const domainDecidesEveryVerb = (method: 'GET' | 'POST', pattern: string): void => { + const patterns = domainDecidedRoutes.get(method) ?? new Set(); + patterns.add(pattern); + domainDecidedRoutes.set(method, patterns); + }; + // ── Discovery (.well-known) ───────────────────────────────── server.get('/.well-known/objectstack', async (_req: any, res: any) => { if (securityHeaders) { @@ -1334,6 +1368,10 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu mountMcpSkill('GET'); mountMcpSkill('POST'); mountMcpSkill('DELETE'); + // [#24228] `PUT`, `PATCH` and every verb the transport cannot route + // reach the same branch through the fallback seam, so the only + // `Allow` this path answers is the domain's `GET`. + domainDecidesEveryVerb('GET', `${prefix}/mcp/skill`); // [#23909] `/keys` is mounted for EVERY verb, though `POST` (mint) // is the only one it serves. The published v18 text is "`/keys`: @@ -1378,6 +1416,11 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu mountKeys('PUT'); mountKeys('PATCH'); mountKeys('DELETE'); + // [#24228] And every verb `IHttpServer` cannot register (`OPTIONS` + // with CORS off, `TRACE`, …) reaches the domain through the fallback + // seam, rather than the transport's `405` listing the four + // refusal-only mounts above as supported. + domainDecidesEveryVerb('POST', `${prefix}/keys`); // ── In-app notifications (ADR-0030) ───────────────────────── // The inbox list + receipt mark-read surface backed by the @@ -1464,6 +1507,11 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu mountNotificationsVerb(subPath, method); } } + // [#24228] Every verb `IHttpServer` cannot register reaches the + // domain through the fallback seam, as on `/keys` above. + domainDecidesEveryVerb('GET', `${prefix}/notifications`); + domainDecidesEveryVerb('POST', `${prefix}/notifications/read`); + domainDecidesEveryVerb('POST', `${prefix}/notifications/read/all`); // ── Packages ──────────────────────────────────────────────── // Single pipeline (ADR-0076 D11 / OQ#9): every package route flows @@ -1917,6 +1965,18 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu errorResponse(err, res); } }); + // [#24228] `POST` is the one verb these paths serve, and the only + // one mounted. Every other verb reaches the domain through the + // fallback seam, so its anonymous baseline — the FIRST statement + // of `handleActionsRequest` (ADR-0138 D2) — answers `401`, and a + // signed-in caller gets the domain's own `405` + `Allow: POST`. + // Before this, the transport answered both `405` itself. + // ⛔ Not refusal-only `GET`/`PUT`/`PATCH`/`DELETE` mounts (the + // `/keys` shape): the transport would list each one in the + // `Allow` it builds for the verbs left over. + domainDecidesEveryVerb('POST', `${base}/actions//:action`); + domainDecidesEveryVerb('POST', `${base}/actions/:object/:action`); + domainDecidesEveryVerb('POST', `${base}/actions/:object/:action/:recordId`); }; // ── Security catalog activation (ADR-0126 §3 regime C, ADR-0131 D3/D6) ── @@ -2045,9 +2105,63 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu logger: ctx.logger, }); + // ── [#24228] Does an unrouted request belong to a domain that + // decides every verb? The router's own verdict: the path belongs + // to a declared route when the transport resolves it, under that + // route's verb, to that route's pattern — one router match per + // declared verb. Read per request, never cached: the declarations + // are made during this same `start()`. Optional on `IHttpServer`, + // so feature-detected; without it the transport's own answer + // stands, and the boot says so below. + const resolveMountedRoute = typeof rawServer.resolveMountedRoute === 'function' + ? rawServer.resolveMountedRoute.bind(rawServer) + : undefined; + const isDomainDecidedPath = (path: string): boolean => { + if (!resolveMountedRoute || !path.startsWith(prefix)) return false; + for (const [verb, patterns] of domainDecidedRoutes) { + const route = resolveMountedRoute(verb, path); + if (route && route.method === verb && patterns.has(route.pattern)) return true; + } + return false; + }; + if (!resolveMountedRoute) { + ctx.logger.warn( + '[dispatcher] http.server exposes no `resolveMountedRoute`: on /actions, /keys, ' + + '/notifications and /mcp/skill a verb the transport cannot route is answered by the ' + + 'transport\'s own 405, not by the domain — an anonymous caller is told the route\'s ' + + 'shape instead of 401, and `Allow` names verbs mounted only to be refused. ' + + 'Fix: compose an HTTP adapter that implements `resolveMountedRoute` ' + + '(e.g. `@objectstack/plugin-hono-server`).', + { domainDecidedRoutes: [...domainDecidedRoutes].map(([verb, p]) => `${verb}: ${[...p].join(', ')}`) }, + ); + } + rawServer.setFallbackHandler(async (req: any, res: any) => { try { const path: string = req.path ?? ''; + // ── [#24228] A path whose domain decides every verb ───── + // The router found no route for this verb, but the path is + // one a declared route serves: the domain answers, so its + // anonymous baseline runs first and its `405` carries its + // own `Allow` (see `domainDecidesEveryVerb`). Handed on as + // `createHonoApp`'s catch-all hands it: the whole sub-path + // after the prefix, `/environments/:id` included — an + // unrouted request has no route params, so `dispatch()` + // reads the environment off the path itself. No body: no + // domain reads one for a verb it refuses, and reading + // `req.body` would let the transport's `400` for a body it + // cannot parse answer ahead of the floor's `401`. + if (isDomainDecidedPath(path)) { + const result = await dispatcher.dispatch( + req.method, + path.substring(prefix.length), + undefined, + req.query, + { request: req }, + ); + sendResult(result, res); + return; + } // ── Scoping test FIRST, before any resolution ──────── // Everything below costs something (a kernel resolve, a // session lookup), and an unmatched request that is not @@ -2248,7 +2362,9 @@ export function createDispatcherPlugin(config: DispatcherPluginConfig = {}): Plu // remedy — compose an adapter that implements the seam. ctx.logger.warn( '[dispatcher] http.server exposes no `setFallbackHandler`: every metadata-declared ' - + '`apis:` endpoint is UNREACHABLE on this transport and will answer a bare 404. ' + + '`apis:` endpoint is UNREACHABLE on this transport and will answer a bare 404, and on ' + + '/actions, /keys, /notifications and /mcp/skill a verb the transport cannot route is ' + + 'answered by the transport, not by the domain. ' + 'Fix: compose an HTTP adapter that implements `setFallbackHandler` ' + '(e.g. `@objectstack/plugin-hono-server`).', { mount: appEndpointMountPrefix(prefix), declarativeEndpoints: 'unreachable' }, diff --git a/packages/runtime/src/domains/keys-actions-method-not-allowed-allow.test.ts b/packages/runtime/src/domains/keys-actions-method-not-allowed-allow.test.ts index 9bcbfb7d1aa..81b0cda2232 100644 --- a/packages/runtime/src/domains/keys-actions-method-not-allowed-allow.test.ts +++ b/packages/runtime/src/domains/keys-actions-method-not-allowed-allow.test.ts @@ -25,10 +25,11 @@ * standalone `os dev` / `os serve` composition), read through `fetch`. * `/api/v1/keys` is mounted for every verb, so a signed-in `GET` reaches * the domain and its 405 is the one on the wire. `/actions` is mounted for - * `POST` alone there, so a `GET` never reaches the domain: the adapter's - * own unmatched-method 405 answers it, and that one already names `POST`. - * The row is kept so "both hosts" is a statement about the wire for both - * domains, not about one handler. + * `POST` alone there; since #24228 the dispatcher's fallback seam hands the + * domain every other verb, so its 405 is the one on the wire too (before + * that, the adapter's own unmatched-method 405 answered, also naming + * `POST`). The row is kept so "both hosts" is a statement about the wire + * for both domains, not about one handler. * * ## The anonymous baseline stays first (ADR-0138 D2) * @@ -176,7 +177,7 @@ describe('the keys and actions 405s carry `Allow` on the wire of the standalone expect(res.headers.get('allow')).toBeNull(); }); - it(`signed-in GET /api/v1/actions/task/close → 405 with \`Allow: ${ALLOW}\` (the adapter's: GET is not mounted)`, async () => { + it(`signed-in GET /api/v1/actions/task/close → 405 with \`Allow: ${ALLOW}\` (the domain's, through the fallback seam since #24228)`, async () => { const { res, body } = await call('GET', '/actions/task/close', true); expect(res.status).toBe(405); diff --git a/packages/runtime/src/keys-every-verb.hono.integration.test.ts b/packages/runtime/src/keys-every-verb.hono.integration.test.ts index f52483e5736..74f578272f7 100644 --- a/packages/runtime/src/keys-every-verb.hono.integration.test.ts +++ b/packages/runtime/src/keys-every-verb.hono.integration.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { request as httpRequest } from 'node:http'; import { LiteKernel, Plugin, PluginContext } from '@objectstack/core'; import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; import type { IHttpServer } from '@objectstack/spec/contracts'; @@ -40,6 +41,12 @@ import { createDispatcherPlugin } from './dispatcher-plugin.js'; const KEYS_PATH = '/api/v1/keys'; const SESSION_HEADER = 'x-test-session'; const NON_POST_VERBS = ['GET', 'PUT', 'PATCH', 'DELETE'] as const; +/** + * [#24228] Verbs `IHttpServer` cannot register, so no `/keys` mount takes them: + * `OPTIONS` (CORS is off in this boot; with it on, CORS answers it) and `TRACE` + * (the transport routes it whatever CORS says). + */ +const UNREGISTRABLE_VERBS = ['OPTIONS', 'TRACE'] as const; function fakeAuthPlugin(): Plugin { return { @@ -125,6 +132,63 @@ describe('every /api/v1/keys verb reaches the keys domain (integration, #23909)' }); } + // ── [#24228] a verb no mount takes reaches the domain too ── + // The transport used to answer these itself: `405` with an `Allow` built + // from the verbs REGISTERED on the path, `DELETE, GET, HEAD, PATCH, POST, + // PUT`, listing the four refusal-only mounts above as supported. The + // dispatcher's fallback seam now hands them to the domain, whose `Allow` + // names the one verb it serves. Raw `node:http`: `fetch` refuses `TRACE`. + function rawCall(method: string, signedIn: boolean): Promise<{ status: number; allow: string | null; body: Record }> { + const { hostname, port } = new URL(baseUrl); + return new Promise((resolve, reject) => { + const req = httpRequest({ + host: hostname, + port, + method, + path: KEYS_PATH, + headers: signedIn ? { [SESSION_HEADER]: '1' } : {}, + }, (res) => { + let text = ''; + res.setEncoding('utf8'); + res.on('data', (chunk) => { text += chunk; }); + res.on('end', () => { + const allow = res.headers.allow; + resolve({ + status: res.statusCode ?? 0, + allow: typeof allow === 'string' ? allow : null, + body: JSON.parse(text) as Record, + }); + }); + }); + req.on('error', reject); + req.end(); + }); + } + + for (const method of UNREGISTRABLE_VERBS) { + it(`anonymous ${method} → 401 UNAUTHENTICATED, no Allow — not the transport's 405 (#24228)`, async () => { + const { status, allow, body } = await rawCall(method, false); + + expect(status).toBe(401); + expect(allow).toBeNull(); + expect(body.error.code).toBe('UNAUTHENTICATED'); + expect(body.error.httpStatus).toBe(401); + expect(body.error).not.toHaveProperty('details'); + }); + + it(`signed-in ${method} → the domain's 405, whose \`Allow\` names POST alone (#24228)`, async () => { + const { status, allow, body } = await rawCall(method, true); + + expect(status).toBe(405); + // Not `DELETE, GET, HEAD, PATCH, POST, PUT`: the four mounts above + // exist only to refuse, and `Allow` names supported methods. + expect(allow).toBe('POST'); + expect(body.error.code).toBe('METHOD_NOT_ALLOWED'); + expect(body.error.httpStatus).toBe(405); + expect(body.error).not.toHaveProperty('details'); + }); + } + // ── control: the verb the route always served is answered as before ── it('anonymous POST → 401 UNAUTHENTICATED (unchanged)', async () => { const { res, body } = await call('POST', false); diff --git a/packages/runtime/src/mcp-skill-method-not-allowed.hono.integration.test.ts b/packages/runtime/src/mcp-skill-method-not-allowed.hono.integration.test.ts index e10c6c6d327..f3ba1feb40d 100644 --- a/packages/runtime/src/mcp-skill-method-not-allowed.hono.integration.test.ts +++ b/packages/runtime/src/mcp-skill-method-not-allowed.hono.integration.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { request as httpRequest } from 'node:http'; import { LiteKernel, Plugin, PluginContext } from '@objectstack/core'; import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; import type { IHttpServer } from '@objectstack/spec/contracts'; @@ -213,28 +214,50 @@ describe('POST /api/v1/mcp/skill answers the standard 405 envelope (integration, expect(text).toContain(`${baseUrl}/api/v1/mcp`); }); - // A verb with no mount at all still falls to the adapter, and should: - // `unmatchedResponse()` is the correct owner of a route that does not - // exist under that verb. This pins the BOUNDARY of the fix rather than - // claiming the adapter answer is wrong everywhere. - it('PUT — unmounted — still falls through to the adapter (boundary, not a regression)', async () => { - const { res, body } = await call('PUT'); - expect(res.status).toBe(405); + // [#24228] A verb with no mount reaches the domain branch too. This case + // used to pin the BOUNDARY the other way: `PUT` fell to the adapter, whose + // `405` listed `DELETE, GET, HEAD, POST` in `Allow` — the verbs REGISTERED + // on the path, two of which (`POST`, `DELETE`) are mounted only so the + // domain can refuse them. RFC 9110 §15.5.6: `Allow` names the SUPPORTED + // methods. The dispatcher declares this path as one whose domain decides + // every verb, and its fallback seam hands the domain each request the + // router cannot route here: `PUT`/`PATCH` (unmounted), `OPTIONS` (CORS is + // off in this boot) and `TRACE` (any verb `IHttpServer` cannot register). + // Raw `node:http`: `fetch` refuses `TRACE`. + function rawCall(method: string): Promise<{ status: number; allow: string | null; body: Record }> { + const { hostname, port } = new URL(baseUrl); + return new Promise((resolve, reject) => { + const req = httpRequest({ host: hostname, port, method, path: SKILL_PATH }, (res) => { + let text = ''; + res.setEncoding('utf8'); + res.on('data', (chunk) => { text += chunk; }); + res.on('end', () => { + const allow = res.headers.allow; + resolve({ + status: res.statusCode ?? 0, + allow: typeof allow === 'string' ? allow : null, + body: JSON.parse(text) as Record, + }); + }); + }); + req.on('error', reject); + req.end(); + }); + } - // The adapter's own 405. Since #9364 it speaks the SAME declared - // envelope as the domain branch, so `success`/`error.code` no longer - // tell the two apart — `error.details.allowed` is what does, and it is - // the direct successor of the top-level `allowed` this case used to - // read. Asserted by VALUE, not by presence — and the value is the - // whole four-verb set because #7649's fix mounts GET/POST/DELETE, with - // Hono registering HEAD implicitly alongside GET. That is precisely - // why PUT is the boundary probe: it is the one verb left unmounted. - expect(body.error.code).toBe('METHOD_NOT_ALLOWED'); - expect(body.error.details.allowed).toEqual(['DELETE', 'GET', 'HEAD', 'POST']); - expect(body.error.details.method).toBe('PUT'); - expect(body.error.details.path).toBe(SKILL_PATH); - // …and it is NOT the domain branch's answer, which fills `httpStatus` - // and no `details`. - expect(body.error).not.toHaveProperty('httpStatus'); - }); + for (const method of ['PUT', 'PATCH', 'OPTIONS', 'TRACE'] as const) { + it(`${method} — no mount — gets the domain branch's 405 with \`Allow: GET\`, not the adapter's four-verb list (#24228)`, async () => { + const { status, allow, body } = await rawCall(method); + + expect(status).toBe(405); + expect(allow).toBe('GET'); + expect(body.success).toBe(false); + expect(body.error.code).toBe('METHOD_NOT_ALLOWED'); + expect(body.error.message).toBe(EXPECTED_MESSAGE); + // The domain branch fills `httpStatus`; the adapter's 405 fills + // `details.{method,path,allowed}`. + expect(body.error.httpStatus).toBe(405); + expect(body.error).not.toHaveProperty('details'); + }); + } }); diff --git a/packages/runtime/src/notifications-every-verb.hono.integration.test.ts b/packages/runtime/src/notifications-every-verb.hono.integration.test.ts index bfe497ea8c6..56ab158af0d 100644 --- a/packages/runtime/src/notifications-every-verb.hono.integration.test.ts +++ b/packages/runtime/src/notifications-every-verb.hono.integration.test.ts @@ -1,6 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { request as httpRequest } from 'node:http'; import { LiteKernel, Plugin, PluginContext } from '@objectstack/core'; import { HonoServerPlugin } from '@objectstack/plugin-hono-server'; import type { IHttpServer } from '@objectstack/spec/contracts'; @@ -49,6 +50,14 @@ const VERBS = ['GET', 'POST', 'PUT', 'PATCH', 'DELETE'] as const; const UNSERVED: Array<[string, string]> = FACES.flatMap((face) => VERBS.filter((verb) => verb !== SERVED[face]).map((verb): [string, string] => [verb, face]), ); +/** + * [#24228] Verbs `IHttpServer` cannot register, so no face's mount takes them: + * `OPTIONS` (CORS is off in this boot; with it on, CORS answers it) and `TRACE` + * (the transport routes it whatever CORS says). + */ +const UNREGISTRABLE: Array<[string, string]> = FACES.flatMap((face) => + (['OPTIONS', 'TRACE'] as const).map((verb): [string, string] => [verb, face]), +); function fakeAuthAndInboxPlugin(): Plugin { return { @@ -154,6 +163,61 @@ describe('every /api/v1/notifications verb reaches the notifications domain (int }); } + // ── [#24228] a verb no mount takes reaches the domain too ── + // The transport used to answer these itself: `405` with an `Allow` built + // from the verbs REGISTERED on the face, `DELETE, GET, HEAD, PATCH, POST, + // PUT`, listing the refusal-only mounts as supported. The dispatcher's + // fallback seam now hands them to the domain, which answers them as it + // answers every verb no face serves. Raw `node:http`: `fetch` refuses `TRACE`. + function rawCall(method: string, face: string, signedIn: boolean): Promise<{ status: number; allow: string | null; body: Record }> { + const { hostname, port } = new URL(baseUrl); + return new Promise((resolve, reject) => { + const req = httpRequest({ + host: hostname, + port, + method, + path: `/api/v1${face}`, + headers: signedIn ? { [SESSION_HEADER]: '1' } : {}, + }, (res) => { + let text = ''; + res.setEncoding('utf8'); + res.on('data', (chunk) => { text += chunk; }); + res.on('end', () => { + const allow = res.headers.allow; + resolve({ + status: res.statusCode ?? 0, + allow: typeof allow === 'string' ? allow : null, + body: JSON.parse(text) as Record, + }); + }); + }); + req.on('error', reject); + req.end(); + }); + } + + for (const [method, face] of UNREGISTRABLE) { + it(`anonymous ${method} ${face} → 401 UNAUTHENTICATED, no Allow — not the transport's 405 (#24228)`, async () => { + const { status, allow, body } = await rawCall(method, face, false); + + expect(status).toBe(401); + expect(allow).toBeNull(); + expect(body.error.code).toBe('UNAUTHENTICATED'); + expect(body.error.httpStatus).toBe(401); + expect(body.error).not.toHaveProperty('details'); + }); + + it(`signed-in ${method} ${face} → the dispatcher's 404, no Allow listing refusal-only mounts (#24228)`, async () => { + const { status, allow, body } = await rawCall(method, face, true); + + expect(status).toBe(404); + expect(allow).toBeNull(); + expect(body.error.code).toBe('ROUTE_NOT_FOUND'); + expect(body.error.httpStatus).toBe(404); + expect(body.error).not.toHaveProperty('details'); + }); + } + // ── control: the verb each face always served is answered as before ── for (const face of FACES) { it(`anonymous ${SERVED[face]} ${face} → 401 UNAUTHENTICATED (unchanged)`, async () => {