From 1e19944e3952144e5d9aa3878218c23a3cd9468a Mon Sep 17 00:00:00 2001 From: JSONbored <49853598+JSONbored@users.noreply.github.com> Date: Tue, 28 Jul 2026 22:48:39 -0700 Subject: [PATCH] fix(openapi): give every 401-declaring operation the credential it actually wants (#9707) applySecurityMetadata only fills a stanza in when requiresApiToken(path) is true, and that returns false for /v1/internal/* and for the anchor-attempt ingest -- not because either is open, but because each carries its OWN credential check. Those operations therefore ended up with the security key entirely ABSENT while still publishing a 401. `[]` and undefined are not the same claim: `[]` is OpenAPI's explicit "this operation needs no credential", undefined is "not stated". A generated client for the provider-credential rotation surface had no credential to send, and a reader could not tell those routes apart from a genuinely public one. The five named operations move to registerRouteSpec entries that declare their auth, so the stanza derives from the declaration rather than from a second path-prefix model of the same policy. The four internal ones emit LoopOverBearer; the anchor-attempt ingest emits OrbBearer, since LOOPOVER_LEDGER_ANCHOR_REPORT_TOKEN is an ingest bearer and not a LoopOver API token. Every response code survives the move, including the 503 and the 413/422 pair, and the provider segment keeps its claude-code | codex enum -- registerRouteSpec now accepts a declared `params` that overrides the derived string one, for the rare closed-set segment. The new assertion is that NO operation declares a 401 with an absent stanza, which turned up three more families the two existing parity tests are blind to (both filter on security being present, so an absent one satisfies neither). The GitHub webhook is genuinely gated and now says so; the device-flow entry points are how a caller OBTAINS a credential, so they declare `public` -- their 401 means "that code is not authorized", not "you forgot a token"; and three internal job routes were left silent only because they had no /run sibling to be tabled with. #9706 still owns that family's duplicate registrations and misreported statuses. Fix-what-you-find: OrbWebhookSignature published `x-loopover-signature`, a string that appears nowhere else in src/. Both webhook handlers read `x-hub-signature-256`, so a client generated from this document signed the right body and sent it under a header the server never looks at, earning a 401 it could not diagnose. --- apps/loopover-ui/public/openapi.json | 86 +++++--- src/openapi/define-route.ts | 19 +- .../internal-and-public-route-specs.ts | 204 +++++++++++++++++- src/openapi/spec.ts | 121 +---------- test/unit/openapi-security-parity.test.ts | 22 +- test/unit/openapi.test.ts | 4 +- 6 files changed, 310 insertions(+), 146 deletions(-) diff --git a/apps/loopover-ui/public/openapi.json b/apps/loopover-ui/public/openapi.json index 6ee7148fe..2a4121a79 100644 --- a/apps/loopover-ui/public/openapi.json +++ b/apps/loopover-ui/public/openapi.json @@ -16718,8 +16718,8 @@ "OrbWebhookSignature": { "type": "apiKey", "in": "header", - "name": "x-loopover-signature", - "description": "HMAC signature over the raw request body, verified against the instance's shared secret. The webhook carries no bearer token." + "name": "x-hub-signature-256", + "description": "GitHub-style HMAC-SHA256 signature over the raw request body, verified against the receiving app's own webhook secret. A webhook delivery carries no bearer token." } } }, @@ -20701,6 +20701,11 @@ "operationId": "postGithubWebhook", "tags": [ "Webhooks" + ], + "security": [ + { + "OrbWebhookSignature": [] + } ] } }, @@ -20977,7 +20982,8 @@ "operationId": "postAuthGithubDeviceStart", "tags": [ "Auth" - ] + ], + "security": [] } }, "/v1/auth/github/device/poll": { @@ -21003,7 +21009,8 @@ "operationId": "postAuthGithubDevicePoll", "tags": [ "Auth" - ] + ], + "security": [] } }, "/v1/auth/github/session": { @@ -21029,7 +21036,8 @@ "operationId": "postAuthGithubSession", "tags": [ "Auth" - ] + ], + "security": [] } }, "/v1/auth/logout": { @@ -21055,7 +21063,8 @@ "operationId": "postAuthLogout", "tags": [ "Auth" - ] + ], + "security": [] } }, "/v1/auth/session": { @@ -22206,7 +22215,7 @@ "summary": "Queue a contributor evidence build job", "responses": { "202": { - "description": "Internal job queued" + "description": "Job queued" }, "401": { "description": "Invalid internal token" @@ -22215,6 +22224,11 @@ "operationId": "postInternalJobsBuildContributorEvidence", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] } }, @@ -22246,7 +22260,7 @@ "summary": "Queue a burden forecast build job", "responses": { "202": { - "description": "Internal job queued" + "description": "Job queued" }, "401": { "description": "Invalid internal token" @@ -22255,6 +22269,11 @@ "operationId": "postInternalJobsBuildBurdenForecasts", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] } }, @@ -22309,7 +22328,7 @@ "summary": "Queue a data fidelity repair job", "responses": { "202": { - "description": "Internal job queued" + "description": "Job queued" }, "401": { "description": "Invalid internal token" @@ -22318,6 +22337,11 @@ "operationId": "postInternalJobsRepairDataFidelity", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] } }, @@ -22335,6 +22359,11 @@ "operationId": "postInternalBountiesImport", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] } }, @@ -23549,6 +23578,11 @@ "operationId": "getInternalProviderCredentialsByProvider", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] }, "post": { @@ -23567,23 +23601,6 @@ "in": "path" } ], - "requestBody": { - "content": { - "application/json": { - "schema": { - "type": "object", - "properties": { - "credential": { - "type": "string" - } - }, - "required": [ - "credential" - ] - } - } - } - }, "responses": { "200": { "description": "Credential stored. Returns the secret-free status.", @@ -23611,6 +23628,11 @@ "operationId": "postInternalProviderCredentialsByProvider", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] }, "delete": { @@ -23661,6 +23683,11 @@ "operationId": "deleteInternalProviderCredentialsByProvider", "tags": [ "Internal" + ], + "security": [ + { + "LoopOverBearer": [] + } ] } }, @@ -26297,7 +26324,12 @@ "422": { "description": "Authenticated but unverifiable: unknown_key, bad_signature, row_not_found, or row_hash_mismatch — an `ok` report must verify against a published key AND match the live chain row" } - } + }, + "security": [ + { + "OrbBearer": [] + } + ] } } }, diff --git a/src/openapi/define-route.ts b/src/openapi/define-route.ts index 8c8e0b350..b1e4e8e6d 100644 --- a/src/openapi/define-route.ts +++ b/src/openapi/define-route.ts @@ -149,7 +149,21 @@ export function defineRoute< /** Spec-side view of a route definition: the same fields, with the request schemas widened, since * emitting the document never needs the parsed types the handler does. */ export type RouteSpecOptions = Omit, "request"> & { - request?: { body?: z.ZodTypeAny | undefined; query?: z.ZodObject | undefined } | undefined; + request?: + | { + body?: z.ZodTypeAny | undefined; + query?: z.ZodObject | undefined; + /** + * Narrower path parameters than the derived `z.string()` ones (#9707). + * + * Spec-only, and rare on purpose: the whole point of deriving parameters from the path is that + * nobody has to declare them twice. This exists for the handful of routes whose segment is a + * closed set -- `/v1/internal/provider-credentials/:provider` is `claude-code | codex` -- where + * publishing a bare string would tell a generated client less than the route actually enforces. + */ + params?: z.ZodObject | undefined; + } + | undefined; }; /** The spec half, exported separately so a route that cannot yet move its handler through the seam @@ -167,7 +181,8 @@ export function registerRouteSpec(registry: OpenAPIRegistry, options: RouteSpecO method: options.method, path: toSpecPath(options.path), request: { - ...(pathParameters(options.path) ?? {}), + // A declared `params` wins over the derived one -- same parameters, a narrower schema. + ...(options.request?.params ? { params: options.request.params } : (pathParameters(options.path) ?? {})), ...(options.request?.body ? { body: { content: { "application/json": { schema: options.request.body } } } } : {}), ...(options.request?.query ? { query: options.request.query } : {}), }, diff --git a/src/openapi/internal-and-public-route-specs.ts b/src/openapi/internal-and-public-route-specs.ts index 1b025f782..b35d50f0f 100644 --- a/src/openapi/internal-and-public-route-specs.ts +++ b/src/openapi/internal-and-public-route-specs.ts @@ -10,6 +10,7 @@ // the two answer with different status codes. Registering them from one table keeps that pairing // visible instead of leaving nineteen near-identical stanzas to drift apart by hand. import type { OpenAPIRegistry } from "@asteasolutions/zod-to-openapi"; +import { z } from "zod"; import { registerRouteSpec, type RouteAuth, type RouteMethod } from "./define-route"; type SpecEntry = { @@ -19,7 +20,11 @@ type SpecEntry = { tags: [string, ...string[]]; summary: string; auth: RouteAuth; - responses: Record; + /** Narrower path parameters than the derived string ones; only for a closed-set segment (#9707). */ + request?: { params?: z.ZodObject }; + /** `schema` is optional: most entries here describe a status and nothing more, but an operation that + * already published a response body must not lose it on the way through the seam (#9707). */ + responses: Record; }; const INTERNAL_AUTH = { 401: { description: "Invalid internal token" } }; @@ -375,6 +380,201 @@ const MISC_ROUTES: SpecEntry[] = [ }, ]; +/** + * The five operations that declared a 401 while publishing no security scheme at all (#9707). + * + * They were legacy `registerPath` calls, and `applySecurityMetadata` only fills a stanza in when + * `requiresApiToken(path)` is true. That returns false for `/v1/internal/*` and for the anchor-attempt + * ingest -- not because either is open, but because each carries its OWN credential check. The result + * was the worst of both: a published 401 with no scheme that could produce it, which a generated client + * cannot act on and a reader cannot tell apart from a genuinely public route. + * + * Declaring `auth` here is the fix, because the stanza then DERIVES from the declaration rather than + * from a second path-prefix model of the same policy. + */ +const PROVIDER = { params: z.object({ provider: z.enum(["claude-code", "codex"]) }) }; +const CREDENTIAL_STATUS = z.union([ + z.object({ configured: z.literal(false) }), + z.object({ configured: z.literal(true), provider: z.string(), last4: z.string(), updatedBy: z.string().nullable(), updatedAt: z.string() }), +]); + +const CREDENTIAL_GATED: SpecEntry[] = [ + { + method: "get", + path: "/v1/internal/provider-credentials/:provider", + operationId: "getInternalProviderCredentialsByProvider", + tags: ["Internal"], + summary: "Read the secret-free status of a stored instance subscription credential", + auth: "internal", + request: PROVIDER, + responses: { + 200: { description: "Credential status. Never includes the credential itself.", schema: CREDENTIAL_STATUS }, + 400: { description: "Unknown provider" }, + ...INTERNAL_AUTH, + }, + }, + { + method: "post", + path: "/v1/internal/provider-credentials/:provider", + operationId: "postInternalProviderCredentialsByProvider", + tags: ["Internal"], + summary: "Store or replace an instance subscription credential, encrypted at rest", + auth: "internal", + request: PROVIDER, + responses: { + 200: { description: "Credential stored. Returns the secret-free status.", schema: z.record(z.string(), z.unknown()) }, + 400: { description: "Unknown provider, or a credential that is empty, padded, or not a single line" }, + ...INTERNAL_AUTH, + 503: { description: "TOKEN_ENCRYPTION_SECRET is not configured, so the credential cannot be stored encrypted" }, + }, + }, + { + method: "delete", + path: "/v1/internal/provider-credentials/:provider", + operationId: "deleteInternalProviderCredentialsByProvider", + tags: ["Internal"], + summary: "Clear a stored instance subscription credential, falling back to the secret file or boot env", + auth: "internal", + request: PROVIDER, + responses: { + 200: { description: "Credential cleared", schema: z.object({ configured: z.literal(false) }) }, + 400: { description: "Unknown provider" }, + ...INTERNAL_AUTH, + }, + }, + { + method: "post", + path: "/v1/internal/bounties/import", + operationId: "postInternalBountiesImport", + tags: ["Internal"], + summary: "Import a bounty snapshot", + auth: "internal", + responses: { 200: { description: "Bounty snapshot imported" }, ...INTERNAL_AUTH }, + }, + { + method: "post", + path: "/v1/decision-ledger/anchor-attempts", + operationId: "reportDecisionLedgerAnchorAttempt", + tags: ["Public"], + summary: "Report one off-Worker anchoring attempt (success or failure) into the public attempt log", + // `orb`, not `token`: the gate is LOOPOVER_LEDGER_ANCHOR_REPORT_TOKEN, an ingest bearer that is not a + // LoopOver API token -- the same posture the `orb` level already exists for. + auth: "orb", + responses: { + 200: { description: "{ recorded: true, status: 'ok' | 'failed' }" }, + 400: { description: "Unparseable body, or a report whose named field failed validation" }, + 401: { description: "Missing or wrong bearer token; also returned when no report token is configured (fails closed)" }, + 413: { description: "Body exceeded the ingest ceiling" }, + 422: { + description: + "Authenticated but unverifiable: unknown_key, bad_signature, row_not_found, or row_hash_mismatch — an `ok` report must verify against a published key AND match the live chain row", + }, + }, + }, +]; + +/** + * The remaining operations that published a 401 with no scheme (#9707). + * + * The five above were the credential-gated ones. These are the rest of what the "no operation declares a + * 401 while stating no credential" assertion turns up, and each needed a different answer: + * + * - The GitHub webhook IS gated, by an HMAC over the raw body, so it declares the signature scheme. + * - The three device-flow entry points are how a caller OBTAINS a credential; their 401 is "that code + * is not authorized", not "you forgot a token". `public` says that out loud -- an empty security array + * is OpenAPI's explicit "needs no credential", which is exactly true and is not the same as silence. + * - Logout acts on the caller's own session, so it declares one. + * - The three job routes are gated by the /v1/internal/* middleware like every one of their siblings; + * they were left behind only because they had no `/run` form to be tabled with. #9706 owns the wider + * cleanup of that family (the duplicate registrations and the statuses they misreport); this moves + * just the auth declaration, because the assertion above cannot pass while they stay silent. + */ +const AUTH_FLOW_RESPONSES = { + 200: { description: "Auth request completed" }, + 201: { description: "Auth session created" }, + 400: { description: "Invalid auth request" }, + 401: { description: "Unauthorized" }, + 429: { description: "Rate limited" }, +}; + +const SILENT_401: SpecEntry[] = [ + { + method: "post", + path: "/v1/github/webhook", + operationId: "postGithubWebhook", + tags: ["Webhooks"], + summary: "Receive a GitHub webhook delivery", + auth: "webhook", + responses: { 202: { description: "Webhook queued" }, 401: { description: "Invalid webhook signature" } }, + }, + { + method: "post", + path: "/v1/auth/github/device/start", + operationId: "postAuthGithubDeviceStart", + tags: ["Auth"], + summary: "Start GitHub device-flow authentication", + auth: "public", + responses: AUTH_FLOW_RESPONSES, + }, + { + method: "post", + path: "/v1/auth/github/device/poll", + operationId: "postAuthGithubDevicePoll", + tags: ["Auth"], + summary: "Poll a pending GitHub device-flow authorization", + auth: "public", + responses: AUTH_FLOW_RESPONSES, + }, + { + method: "post", + path: "/v1/auth/github/session", + operationId: "postAuthGithubSession", + tags: ["Auth"], + summary: "Exchange a GitHub token for a LoopOver session", + auth: "public", + responses: AUTH_FLOW_RESPONSES, + }, + { + method: "post", + path: "/v1/auth/logout", + operationId: "postAuthLogout", + tags: ["Auth"], + summary: "End the current session", + // `public`, not `session`: the handler revokes whatever identity the request carries and answers 200 + // either way -- it never rejects an anonymous caller, and requiresApiToken exempts the family. Saying + // `session` would advertise a credential no gate demands. + auth: "public", + responses: AUTH_FLOW_RESPONSES, + }, + { + method: "post", + path: "/v1/internal/jobs/build-contributor-evidence", + operationId: "postInternalJobsBuildContributorEvidence", + tags: ["Internal"], + summary: "Queue a contributor evidence build job", + auth: "internal", + responses: QUEUED, + }, + { + method: "post", + path: "/v1/internal/jobs/build-burden-forecasts", + operationId: "postInternalJobsBuildBurdenForecasts", + tags: ["Internal"], + summary: "Queue a burden forecast build job", + auth: "internal", + responses: QUEUED, + }, + { + method: "post", + path: "/v1/internal/jobs/repair-data-fidelity", + operationId: "postInternalJobsRepairDataFidelity", + tags: ["Internal"], + summary: "Queue a data fidelity repair job", + auth: "internal", + responses: QUEUED, + }, +]; + export function registerInternalAndPublicRouteSpecs(registry: OpenAPIRegistry): void { for (const entry of INTERNAL_AND_PUBLIC_ROUTE_SPECS) registerRouteSpec(registry, entry); } @@ -386,4 +586,6 @@ export const INTERNAL_AND_PUBLIC_ROUTE_SPECS: readonly SpecEntry[] = [ ...INTERNAL_OTHER, ...PUBLIC_ROUTES, ...MISC_ROUTES, + ...CREDENTIAL_GATED, + ...SILENT_401, ]; diff --git a/src/openapi/spec.ts b/src/openapi/spec.ts index 6fa238296..a790dc65f 100644 --- a/src/openapi/spec.ts +++ b/src/openapi/spec.ts @@ -1853,17 +1853,6 @@ export function buildOpenApiSpec() { 404: { description: "Bounty not found" }, }, }); - registry.registerPath({ - method: "post", - path: "/v1/github/webhook", - operationId: "postGithubWebhook", - tags: ["Webhooks"], - summary: "Receive a GitHub webhook delivery", - responses: { - 202: { description: "Webhook queued" }, - 401: { description: "Invalid webhook signature" }, - }, - }); registry.registerPath({ method: "get", path: "/v1/public/decision-ledger/verify", @@ -1920,20 +1909,6 @@ export function buildOpenApiSpec() { 404: { description: "Anchor signing is not configured, or the ledger is empty — nothing is claimed to be anchorable yet" }, }, }); - registry.registerPath({ - method: "post", - path: "/v1/decision-ledger/anchor-attempts", - operationId: "reportDecisionLedgerAnchorAttempt", - tags: ["Public"], - summary: "Report one off-Worker anchoring attempt (success or failure) into the public attempt log", - responses: { - 200: { description: "{ recorded: true, status: 'ok' | 'failed' }" }, - 400: { description: "Unparseable body, or a report whose named field failed validation" }, - 401: { description: "Missing or wrong bearer token; also returned when no report token is configured (fails closed)" }, - 413: { description: "Body exceeded the ingest ceiling" }, - 422: { description: "Authenticated but unverifiable: unknown_key, bad_signature, row_not_found, or row_hash_mismatch — an `ok` report must verify against a published key AND match the live chain row" }, - }, - }); registry.registerPath({ method: "get", path: "/v1/public/decision-records/{owner}/{repo}/{pull}", @@ -1980,26 +1955,6 @@ export function buildOpenApiSpec() { 302: { description: "Completes GitHub web OAuth and redirects to the app" }, }, }); - for (const [path, summary] of [ - ["/v1/auth/github/device/start", "Start GitHub device-flow authentication"], - ["/v1/auth/github/device/poll", "Poll a pending GitHub device-flow authorization"], - ["/v1/auth/github/session", "Exchange a GitHub token for a LoopOver session"], - ["/v1/auth/logout", "End the current session"], - ] as const) { - registry.registerPath({ - method: "post", - path, - ...loopOperationMeta("post", path, "Auth"), - summary, - responses: { - 200: { description: "Auth request completed" }, - 201: { description: "Auth session created" }, - 400: { description: "Invalid auth request" }, - 401: { description: "Unauthorized" }, - 429: { description: "Rate limited" }, - }, - }); - } registry.registerPath({ method: "get", path: "/v1/auth/session", @@ -2254,59 +2209,6 @@ export function buildOpenApiSpec() { } // Instance subscription-CLI credentials (#9543). The response NEVER carries the credential itself -- // only configured/last4/timestamps -- so the whole surface is safe to describe publicly. - registry.registerPath({ - method: "get", - path: "/v1/internal/provider-credentials/{provider}", - operationId: "getInternalProviderCredentialsByProvider", - tags: ["Internal"], - summary: "Read the secret-free status of a stored instance subscription credential", - request: { params: z.object({ provider: z.enum(["claude-code", "codex"]) }) }, - responses: { - 200: { - description: "Credential status. Never includes the credential itself.", - content: { - "application/json": { - schema: z.union([ - z.object({ configured: z.literal(false) }), - z.object({ configured: z.literal(true), provider: z.string(), last4: z.string(), updatedBy: z.string().nullable(), updatedAt: z.string() }), - ]), - }, - }, - }, - 400: { description: "Unknown provider" }, - 401: { description: "Invalid internal token" }, - }, - }); - registry.registerPath({ - method: "post", - path: "/v1/internal/provider-credentials/{provider}", - operationId: "postInternalProviderCredentialsByProvider", - tags: ["Internal"], - summary: "Store or replace an instance subscription credential, encrypted at rest", - request: { - params: z.object({ provider: z.enum(["claude-code", "codex"]) }), - body: { content: { "application/json": { schema: z.object({ credential: z.string() }) } } }, - }, - responses: { - 200: { description: "Credential stored. Returns the secret-free status.", content: { "application/json": { schema: z.record(z.string(), z.unknown()) } } }, - 400: { description: "Unknown provider, or a credential that is empty, padded, or not a single line" }, - 401: { description: "Invalid internal token" }, - 503: { description: "TOKEN_ENCRYPTION_SECRET is not configured, so the credential cannot be stored encrypted" }, - }, - }); - registry.registerPath({ - method: "delete", - path: "/v1/internal/provider-credentials/{provider}", - operationId: "deleteInternalProviderCredentialsByProvider", - tags: ["Internal"], - summary: "Clear a stored instance subscription credential, falling back to the secret file or boot env", - request: { params: z.object({ provider: z.enum(["claude-code", "codex"]) }) }, - responses: { - 200: { description: "Credential cleared", content: { "application/json": { schema: z.object({ configured: z.literal(false) }) } } }, - 400: { description: "Unknown provider" }, - 401: { description: "Invalid internal token" }, - }, - }); registry.registerPath({ method: "post", path: "/v1/internal/jobs/refresh-registry", @@ -2369,12 +2271,9 @@ export function buildOpenApiSpec() { ["/v1/internal/jobs/refresh-scoring-model", "Queue a scoring model refresh job"], ["/v1/internal/jobs/refresh-upstream-drift", "Queue an upstream drift refresh job"], ["/v1/internal/jobs/file-upstream-drift-issues", "Queue a job that files upstream drift issues"], - ["/v1/internal/jobs/build-contributor-evidence", "Queue a contributor evidence build job"], ["/v1/internal/jobs/build-contributor-decision-packs", "Queue a contributor decision pack build job"], - ["/v1/internal/jobs/build-burden-forecasts", "Queue a burden forecast build job"], ["/v1/internal/jobs/generate-signal-snapshots", "Queue a signal snapshot generation job"], ["/v1/internal/jobs/generate-weekly-value-report", "Queue a weekly value report job"], - ["/v1/internal/jobs/repair-data-fidelity", "Queue a data fidelity repair job"], ] as const) { registry.registerPath({ method: "post", @@ -2387,17 +2286,6 @@ export function buildOpenApiSpec() { }, }); } - registry.registerPath({ - method: "post", - path: "/v1/internal/bounties/import", - operationId: "postInternalBountiesImport", - tags: ["Internal"], - summary: "Import a bounty snapshot", - responses: { - 200: { description: "Bounty snapshot imported" }, - 401: { description: "Invalid internal token" }, - }, - }); // #9531: the ORB ingress, the control-panel app surface, and the per-repo key/settings routes. // Registered from their own module rather than inline here because each entry declares an auth @@ -2457,11 +2345,16 @@ function applySecurityMetadata(document: GeneratedOpenApiDocument): GeneratedOpe scheme: "bearer", description: "ORB-issued instance token, minted by POST /v1/orb/token and presented by a self-hosted ORB instance on the relay endpoints. Not a LoopOver API token.", }, + // #9707: the header NAME was wrong. This published `x-loopover-signature`, a string that appears + // nowhere else in src/ -- both webhook handlers read `x-hub-signature-256` (src/orb/webhook.ts:25, + // src/github/webhook.ts:101), so a client generated from this document signed the right body and + // sent it under a header the server never looks at, earning a 401 it could not diagnose. OrbWebhookSignature: { type: "apiKey", in: "header", - name: "x-loopover-signature", - description: "HMAC signature over the raw request body, verified against the instance's shared secret. The webhook carries no bearer token.", + name: "x-hub-signature-256", + description: + "GitHub-style HMAC-SHA256 signature over the raw request body, verified against the receiving app's own webhook secret. A webhook delivery carries no bearer token.", }, }, }; diff --git a/test/unit/openapi-security-parity.test.ts b/test/unit/openapi-security-parity.test.ts index 75b16ff37..f74c123d0 100644 --- a/test/unit/openapi-security-parity.test.ts +++ b/test/unit/openapi-security-parity.test.ts @@ -12,7 +12,7 @@ import { describe, expect, it } from "vitest"; import { buildOpenApiSpec } from "../../src/openapi/spec"; import { requiresApiToken } from "../../src/auth/route-auth"; -type Operation = { security?: Array> }; +type Operation = { security?: Array>; responses?: Record }; function operations(): Array<{ path: string; method: string; operation: Operation }> { const spec = buildOpenApiSpec(); @@ -63,6 +63,26 @@ describe("OpenAPI security parity with the real gate (#9531)", () => { expect(silent).toEqual([]); }); + it("never declares a 401 while stating no credential at all (#9707)", () => { + // The class both assertions above are blind to: they filter on `operation.security` being PRESENT, so + // an ABSENT stanza satisfies neither. `[]` and undefined are not the same claim -- `[]` is OpenAPI's + // explicit "this operation needs no credential", undefined is "not stated" -- and an operation that + // publishes a 401 while saying nothing leaves a generated client with no credential to send and a + // reader unable to tell it apart from a genuinely open route. + const silent = operations() + .filter(({ operation }) => operation.responses?.["401"] !== undefined && operation.security === undefined) + .map(({ path, method }) => `${method.toUpperCase()} ${path}`); + expect(silent).toEqual([]); + }); + + it("names the header the webhook handlers actually read (#9707)", () => { + // This published `x-loopover-signature`, a string that appears nowhere else in src/. Both handlers + // read `x-hub-signature-256`, so a client generated from the document signed the right body and sent + // it under a header the server never looks at. + const document = buildOpenApiSpec() as { components?: { securitySchemes?: Record } }; + expect(document.components?.securitySchemes?.OrbWebhookSignature?.name).toBe("x-hub-signature-256"); + }); + it("distinguishes the auth levels rather than collapsing them to one stanza", () => { // The defect this replaced: every operation emitted the same LoopOverBearer+SessionCookie pair, // so the document could not tell a caller which routes need nothing, which need an internal diff --git a/test/unit/openapi.test.ts b/test/unit/openapi.test.ts index 721cf0b3f..b7b79b8d5 100644 --- a/test/unit/openapi.test.ts +++ b/test/unit/openapi.test.ts @@ -184,7 +184,9 @@ describe("OpenAPI contract", () => { expect(spec.paths["/v1/repos"]?.get?.security).toEqual([{ LoopOverBearer: [] }, { LoopOverSessionCookie: [] }]); expect(spec.paths["/v1/app/overview"]?.get?.security).toEqual([{ LoopOverBearer: [] }, { LoopOverSessionCookie: [] }]); expect(spec.paths["/v1/auth/session"]?.get?.security).toBeUndefined(); - expect(spec.paths["/v1/auth/logout"]?.post?.security).toBeUndefined(); + // #9707: `[]`, not undefined. Logout publishes a 401, and an operation declaring one while stating no + // credential is unreadable — `[]` says "needs none" out loud, which is what this route means. + expect(spec.paths["/v1/auth/logout"]?.post?.security).toEqual([]); // #9531: session-only, not the generic pair. This handler returns 403 to a bearer-only caller // (`identity.kind !== "session"`), so advertising LoopOverBearer as sufficient was a published // lie -- one neither of the two former security models got right.