From 33afcbf3648daaaa4131ec1a24f7557d682ec9d3 Mon Sep 17 00:00:00 2001 From: Meta Vens Date: Wed, 30 Sep 2026 15:30:06 +0200 Subject: [PATCH] HTTP error answers carry fixed text, never an error's message Soul pass 3 (R1): with no error handler, Fastify answered a 5xx with the error's message. A store failure under a misbound database URL reached an anonymous caller as `getaddrinfo ENOTFOUND ` or `database "" does not exist`, and a misbound URL puts part of the password in either. The same default quoted the request body in a JSON parse error (Node's JSON.parse message carries the input), the Content-Type header in a 415, and the path in a 404. buildApp now sets both handlers. A 5xx answers `{ error: "internal_error" }` and writes one stderr line naming the method, the route pattern and the error code. A schema failure answers the failed rules by schema path and Ajv message, which come from the route's schema. Any other 4xx answers `invalid_request` with the error code. An unknown route answers `not_found`. The app's own answers had two more echoes: `unknown_app` quoted the slug, and a failed provider callback put the caught error's text in the browser page and the backend handoff (the JSON answer was already fixed). Both are fixed text now; the full callback error stays in the audit event. Co-Authored-By: Claude Opus 5.5 --- src/app.ts | 49 +++++++++++--- tests/app.test.ts | 169 ++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 207 insertions(+), 11 deletions(-) diff --git a/src/app.ts b/src/app.ts index 6f85979..322c6e8 100644 --- a/src/app.ts +++ b/src/app.ts @@ -1,6 +1,6 @@ import { createHmac, randomUUID } from "node:crypto"; import cors from "@fastify/cors"; -import Fastify, { type FastifyInstance } from "fastify"; +import Fastify, { type FastifyError, type FastifyInstance } from "fastify"; import { type AppCaller, isAppCaller, resolveAppCaller } from "./app-caller.js"; import { deliverBackendHandoff, type BackendHandoffPayload } from "./backend-handoff.js"; import { @@ -532,7 +532,7 @@ export async function startOAuthFlow( const { config, keys } = ctx; const profile = getAppProfile(input.appSlug); if (!profile) { - return { statusCode: 404, body: { error: "unknown_app", detail: `No app '${input.appSlug}' is registered.` } }; + return { statusCode: 404, body: { error: "unknown_app", detail: "No app is registered under that slug." } }; } if (!supportsProvider(profile, provider)) { @@ -736,6 +736,31 @@ export async function buildApp(options: BuildAppOptions = {}): Promise { + const status = error.statusCode !== undefined && error.statusCode >= 400 && error.statusCode < 600 ? error.statusCode : 500; + const code = typeof error.code === "string" ? error.code : "error"; + reply.code(status); + if (status >= 500) { + console.error(`Heimdall ${request.method} ${request.routeOptions.url ?? "(no route)"} failed (${code}).`); + return { error: "internal_error" }; + } + if (error.validation) { + // Ajv's messages and schema paths come from the route's schema, not the request. + const failures = error.validation.map((failure) => `${failure.schemaPath} ${failure.message ?? failure.keyword}`); + return { error: "invalid_request", detail: `${error.validationContext ?? "request"}: ${failures.join("; ")}` }; + } + return { error: "invalid_request", code }; + }); + app.setNotFoundHandler((_request, reply) => { + reply.code(404); + return { error: "not_found" }; + }); app.decorate("heimdallContext", { config, keys, store, oauthRuntimes, tokenCustody }); app.addHook("onClose", async () => { await store.close(); @@ -788,7 +813,7 @@ export async function buildApp(options: BuildAppOptions = {}): Promise { expect(tokenSet.scope).toEqual(["user:read:email"]); }); }); + +// An answer never carries an input value. The canary stands in for whatever a +// failure could quote: part of a misbound database URL in a pg or resolver +// error, a request body in a JSON parse error, a header or a path. +describe("error answers never echo an input", () => { + const CANARY = "CANARY"; + + function failingStore(): InMemoryStore { + const store = new InMemoryStore(); + store.consumeAuthCompletion = async () => { + throw Object.assign(new Error(`getaddrinfo ENOTFOUND ${CANARY}host.invalid`), { + code: "ENOTFOUND", + hostname: `${CANARY}host.invalid`, + }); + }; + return store; + } + + it("answers a store failure with a fixed 500 body and logs only the route and code", async () => { + const app = await buildApp({ config: createTestConfig(), store: failingStore() }); + apps.push(app); + const logged: string[] = []; + const originalError = console.error; + console.error = (...args: unknown[]) => void logged.push(args.map(String).join(" ")); + try { + const response = await app.inject({ + method: "POST", + url: "/v1/apps/bifrost/auth-completions/redeem", + payload: { completionCode: "x" }, + }); + + expect(response.statusCode).toBe(500); + expect(response.json()).toEqual({ error: "internal_error" }); + } finally { + console.error = originalError; + } + expect(logged).toEqual(["Heimdall POST /v1/apps/:appSlug/auth-completions/redeem failed (ENOTFOUND)."]); + }); + + it("answers a body that is not JSON without quoting it", async () => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + + const response = await app.inject({ + method: "POST", + url: "/v1/apps/bifrost/auth-completions/redeem", + headers: { "content-type": "application/json" }, + payload: `${CANARY}notjson`, + }); + + expect(response.statusCode).toBe(400); + expect(response.json().error).toBe("invalid_request"); + expect(response.body).not.toContain(CANARY); + }); + + it("answers an unsupported media type without quoting the header", async () => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + + const response = await app.inject({ + method: "POST", + url: "/v1/apps/bifrost/auth-completions/redeem", + headers: { "content-type": `application/${CANARY}` }, + payload: "x", + }); + + expect(response.statusCode).toBe(415); + expect(response.body).not.toContain(CANARY); + }); + + it("names the failed schema rule without quoting the request", async () => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + + const response = await app.inject({ + method: "POST", + url: "/v1/apps/bifrost/auth-completions/redeem", + payload: { [`${CANARY}extra`]: `${CANARY}value` }, + }); + + expect(response.statusCode).toBe(400); + expect(response.json().error).toBe("invalid_request"); + expect(response.json().detail).toMatch(/^body: #\//); + expect(response.body).not.toContain(CANARY); + }); + + it("answers an unknown route or app without quoting the path", async () => { + const app = await buildApp({ config: createTestConfig() }); + apps.push(app); + + const route = await app.inject({ method: "GET", url: `/v1/${CANARY}route` }); + expect(route.statusCode).toBe(404); + expect(route.json()).toEqual({ error: "not_found" }); + + const unknownApp = await app.inject({ method: "GET", url: `/v1/apps/${CANARY}app` }); + expect(unknownApp.statusCode).toBe(404); + expect(unknownApp.json().error).toBe("unknown_app"); + expect(unknownApp.body).not.toContain(CANARY); + + const start = await app.inject({ + method: "POST", + url: "/v1/oauth/discord/start", + payload: { appSlug: `${CANARY}app`, mode: "sign_in", returnTo: "https://repixelizer.gamecult.org/app/" }, + }); + expect(start.statusCode).toBe(400); + expect(start.body).not.toContain(CANARY); + }); + + function failingDiscordRuntime(): OAuthProviderRuntime { + return { + ...createMockDiscordRuntime(), + async exchangeAuthorizationCode() { + throw new SyntaxError(`Unexpected token 'a', "access_token=${CANARY}token" is not valid JSON`); + }, + }; + } + + it("renders a failed provider callback page without the failure's text", async () => { + const app = await buildApp({ config: createTestConfig(), oauthRuntimes: { discord: failingDiscordRuntime() } }); + apps.push(app); + + const stateToken = await startDiscordSignIn(app); + const response = await app.inject({ + method: "GET", + url: `/v1/oauth/discord/callback?code=test-code&state=${encodeURIComponent(stateToken)}`, + headers: { accept: "text/html" }, + }); + + expect(response.headers["content-type"]).toContain("text/html"); + expect(response.body).toContain("oauth_callback_failed"); + expect(response.body).not.toContain(CANARY); + }); + + it("delivers a failed provider callback to the app backend without the failure's text", async () => { + const deliveries: string[] = []; + globalThis.fetch = async (_input, init) => { + deliveries.push(String(init?.body)); + return new Response(null, { status: 204 }); + }; + const app = await buildApp({ config: createTestConfig(), oauthRuntimes: { discord: failingDiscordRuntime() } }); + apps.push(app); + + const start = await app.inject({ + method: "POST", + url: "/v1/oauth/discord/start", + payload: { + appSlug: "repixelizer", + mode: "sign_in", + returnTo: "https://repixelizer.gamecult.org/app/", + handoff: { + kind: "backend_callback", + attemptId: "attempt-123", + callbackUrl: "https://repixelizer.gamecult.org/api/auth/heimdall/callback", + }, + }, + }); + expect(start.statusCode).toBe(201); + const response = await app.inject({ + method: "GET", + url: `/v1/oauth/discord/callback?code=test-code&state=${encodeURIComponent(start.json().stateToken as string)}`, + }); + + expect(response.statusCode).toBe(502); + expect(response.body).not.toContain(CANARY); + expect(deliveries).toHaveLength(1); + expect(JSON.parse(deliveries[0]!)).toMatchObject({ status: "error", error: "oauth_callback_failed" }); + expect(deliveries[0]).not.toContain(CANARY); + }); +});