From d2375ea9c13aa8fb419644eb413a17c314871d80 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 21 Aug 2026 13:32:46 +0000 Subject: [PATCH] =?UTF-8?q?fix(core,hono):=20retire=20/graphql=20residue?= =?UTF-8?q?=20=E2=80=94=20stale=20surface=20list=20and=20inert=20test=20do?= =?UTF-8?q?ubles?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two halves of #10835, deliberately different in kind. anonymous-deny.ts named three Phase-1 surfaces, two of them long gone: /graphql (removed with the GraphQL surface, #2462 follow-on) and the raw-hono /data routes (deleted as a duplicate surface in v17, #4073). Replaced the inline route list with the two corrections and a pointer to the authz-conformance ratchet, which enumerates the live entry points from source. Comment-only; no code touched. The three handleGraphQL test doubles were inert, not deliberate: the hono adapter calls exactly getDiscoveryInfo, handleAuth and dispatch, so a fourth key on the mock is unreachable from index.ts. Removed, and replaced with a request-level test that /api/graphql reaches the catch-all — the same shape the retired /storage mount is pinned with, which needs no handleStorage double either. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --- .../adapters/hono/src/__mocks__/runtime.ts | 11 +++++- .../src/hono-wildcard-fallthrough.test.ts | 1 - packages/adapters/hono/src/hono.test.ts | 32 ++++++++++++++- packages/core/src/security/anonymous-deny.ts | 39 ++++++++++++++++--- 4 files changed, 75 insertions(+), 8 deletions(-) diff --git a/packages/adapters/hono/src/__mocks__/runtime.ts b/packages/adapters/hono/src/__mocks__/runtime.ts index b7d188ef1a..b87987aa76 100644 --- a/packages/adapters/hono/src/__mocks__/runtime.ts +++ b/packages/adapters/hono/src/__mocks__/runtime.ts @@ -1,9 +1,18 @@ // Stub for @objectstack/runtime - resolved via vitest alias +// +// [#10835] Every method here must exist on the REAL `HttpDispatcher` +// (`packages/runtime/src/http-dispatcher.ts`). A stub method the subject does +// not have can never diverge from it — it is green by construction — and it +// reads to the next author grepping the name as evidence the runtime still has +// the method. `handleGraphQL` sat here for exactly that reason after `/graphql` +// was removed (#2462 follow-on) and was the last non-CHANGELOG hit for the name +// in `packages/**`. Declaring MORE than this adapter calls is fine (it calls +// only `getDiscoveryInfo`, `handleAuth` and `dispatch`); declaring something the +// dispatcher does not implement is not. import { vi } from 'vitest'; export class HttpDispatcher { getDiscoveryInfo = vi.fn().mockReturnValue({ version: '1.0', routes: {} }); - handleGraphQL = vi.fn().mockResolvedValue({ data: {} }); handleAuth = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: { ok: true } } }); handleMetadata = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: { objects: [] } } }); handleData = vi.fn().mockResolvedValue({ handled: true, response: { status: 200, body: { records: [] } } }); diff --git a/packages/adapters/hono/src/hono-wildcard-fallthrough.test.ts b/packages/adapters/hono/src/hono-wildcard-fallthrough.test.ts index 5dc23745c8..bc62a0f0ed 100644 --- a/packages/adapters/hono/src/hono-wildcard-fallthrough.test.ts +++ b/packages/adapters/hono/src/hono-wildcard-fallthrough.test.ts @@ -34,7 +34,6 @@ import type { Hono } from 'hono'; const mockDispatcher = { getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', routes: {} }), handleAuth: vi.fn(), - handleGraphQL: vi.fn(), dispatch: vi.fn(), }; diff --git a/packages/adapters/hono/src/hono.test.ts b/packages/adapters/hono/src/hono.test.ts index b67e8424d0..063236de3f 100644 --- a/packages/adapters/hono/src/hono.test.ts +++ b/packages/adapters/hono/src/hono.test.ts @@ -7,7 +7,6 @@ import { Hono } from 'hono'; const mockDispatcher = { getDiscoveryInfo: vi.fn().mockReturnValue({ version: '1.0', routes: {} }), handleAuth: vi.fn().mockResolvedValue({ handled: true, response: { body: { ok: true }, status: 200 } }), - handleGraphQL: vi.fn().mockResolvedValue({ data: {} }), dispatch: vi.fn().mockResolvedValue({ handled: true, response: { body: { success: true }, status: 200 } }), }; @@ -512,6 +511,37 @@ describe('createHonoApp', () => { '/api', ); }); + + // [#10835] `/graphql` is NOT a routed domain: it was removed along with the + // GraphQL surface itself (`runtime/http-dispatcher.ts` — "/graphql removed + // — GraphQL is not in the product plan", #2462 follow-on), so it is ordinary + // catch-all traffic, exactly like the retired `/storage` mount above. + // + // This REPLACES a `handleGraphQL` double that used to sit on + // `mockDispatcher` and proved nothing. The adapter calls exactly three + // dispatcher methods — `getDiscoveryInfo`, `handleAuth`, `dispatch` — so an + // extra key on the mock was unreachable from `index.ts`: it could never + // diverge from the real dispatcher, stayed green by construction, and read + // to anyone grepping the name as evidence the runtime still had the method. + // A double for an unrouted method is not how fall-through is demonstrated; + // a request that lands on `dispatch()` is, which is why the `/storage` test + // above needs no `handleStorage` double either. + it('POST /api/graphql reaches the catch-all — GraphQL is not a routed domain', async () => { + const res = await app.request('/api/graphql', { + method: 'POST', + body: JSON.stringify({ query: '{ __typename }' }), + headers: { 'Content-Type': 'application/json' }, + }); + expect(res.status).toBe(200); + expect(mockDispatcher.dispatch).toHaveBeenCalledWith( + 'POST', + '/graphql', + { query: '{ __typename }' }, + expect.any(Object), + expect.objectContaining({ request: expect.anything() }), + '/api', + ); + }); }); describe('Error Handling', () => { diff --git a/packages/core/src/security/anonymous-deny.ts b/packages/core/src/security/anonymous-deny.ts index 7e2fa1b6e0..b08b9da206 100644 --- a/packages/core/src/security/anonymous-deny.ts +++ b/packages/core/src/security/anonymous-deny.ts @@ -4,11 +4,40 @@ * #2567 — the single anonymous-deny decision, shared by every HTTP seam. * * ADR-0056 D2 made the platform deny anonymous callers by default. Phase 1 gated - * each surface (REST `/data`, dispatcher `/graphql` + `/meta`, raw-hono `/data`) - * but every seam hand-rolled the same `!userId && !isSystem → 401` check. This - * centralises that DECISION into one pure, tested function — the exact pattern - * {@link ./auth-gate.ts} established for the ADR-0069 auth-policy gate: keeping - * the decision in one function means the seams can never drift on who is denied. + * each surface separately, with every seam hand-rolling the same + * `!userId && !isSystem → 401` check. This centralises that DECISION into one + * pure, tested function — the exact pattern {@link ./auth-gate.ts} established + * for the ADR-0069 auth-policy gate: keeping the decision in one function means + * the seams can never drift on who is denied. + * + * ## Do NOT keep a route list here (#10835) + * + * This paragraph used to name the Phase-1 surfaces inline — "REST `/data`, + * dispatcher `/graphql` + `/meta`, raw-hono `/data`" — and TWO of those three + * outlived the routes they named: + * + * - **`/graphql` is gone.** Removed with the GraphQL surface itself + * (`packages/runtime/src/http-dispatcher.ts`: "/graphql removed — GraphQL is + * not in the product plan", #2462 follow-on). No `handleGraphQL` survives + * anywhere in `packages/runtime`, and `/graphql` is now ordinary catch-all + * traffic. + * - **raw-hono `/data` is gone.** Those routes were deleted as a duplicate + * surface in v17 (#4073); `packages/adapters/hono` mounts only `/discovery`, + * `/auth/*` and the terminal `${prefix}/*` catch-all. + * + * A stale list here is worse than no list, because the seams it names are the + * thing a reader opens this file to learn: a dead route reads as a live surface. + * That is not hypothetical — the `handleGraphQL` test doubles removed alongside + * this edit had become the only non-CHANGELOG hits in `packages/**`, and so read + * to the next author grepping the name as evidence the runtime still had the + * method. + * + * The live enumeration is mechanical, so defer to it rather than restating it: + * `packages/qa/dogfood/test/authz-conformance.matrix.ts` and its companion + * `authz-conformance.test.ts` ratchet a CURATED table of HTTP/transport entry + * points out of source — deleting a `shouldDenyAnonymous` call makes its row + * STALE, a new ungated route in one of those files is UNCLASSIFIED, and either + * breaks CI. Read the ratchet, or read the call sites. * * ## The `requireAuth` opt-out is gone (#3963) *