From 9654fd5122a060b148b950662f56e9af50f03d70 Mon Sep 17 00:00:00 2001 From: Chris Alfano Date: Mon, 1 Jun 2026 09:21:23 -0400 Subject: [PATCH] fix(api): gate /api/_test/* routes to non-production (closes #116) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three test-harness routes in health.ts: POST /api/_test/validation-error POST /api/_test/internal-error POST /api/_test/idempotency exist purely to exercise the error-mapping + idempotency code paths from CI. They were always reachable in production — meaning any caller could hit /api/_test/internal-error and force a 500. Defense in depth: wrap registration in `if (fastify.config.NODE_ENV !== 'production')`. CI tests that depend on these routes already run in NODE_ENV=test, so nothing breaks. New api-skeleton test asserts all three return 404 when the app is built with NODE_ENV=production. Co-Authored-By: Claude Opus 4.7 (1M context) --- apps/api/src/routes/health.ts | 96 ++++++++++++++++------------- apps/api/tests/api-skeleton.test.ts | 37 +++++++++++ 2 files changed, 90 insertions(+), 43 deletions(-) diff --git a/apps/api/src/routes/health.ts b/apps/api/src/routes/health.ts index d74082e..bb2dee0 100644 --- a/apps/api/src/routes/health.ts +++ b/apps/api/src/routes/health.ts @@ -118,53 +118,63 @@ export async function healthRoutes(fastify: FastifyInstance): Promise { }, ); - // Stub route for testing validation errors - fastify.post( - '/api/_test/validation-error', - { - schema: { - hide: true, - body: { - type: 'object', - properties: { - trigger: { type: 'string' }, + // --------------------------------------------------------------------------- + // Test-harness routes — registered only when NODE_ENV !== 'production'. + // + // These exist purely to exercise the error-mapping + idempotency code + // paths from CI tests. Gating them off in production is defense in + // depth: there's no reason production callers should be able to hit + // /api/_test/internal-error and force a 500. See issue #116. + // --------------------------------------------------------------------------- + if (fastify.config.NODE_ENV !== 'production') { + // Stub route for testing validation errors + fastify.post( + '/api/_test/validation-error', + { + schema: { + hide: true, + body: { + type: 'object', + properties: { + trigger: { type: 'string' }, + }, }, }, }, - }, - async () => { - const { ApiValidationError } = await import('../lib/errors.js'); - throw new ApiValidationError('Test validation failed', { field: 'required' }); - }, - ); + async () => { + const { ApiValidationError } = await import('../lib/errors.js'); + throw new ApiValidationError('Test validation failed', { field: 'required' }); + }, + ); - // Stub route for testing unknown/500 errors - fastify.post( - '/api/_test/internal-error', - { schema: { hide: true } }, - async () => { - throw new Error('Deliberate internal error — should not leak to client'); - }, - ); + // Stub route for testing unknown/500 errors + fastify.post( + '/api/_test/internal-error', + { schema: { hide: true } }, + async () => { + throw new Error('Deliberate internal error — should not leak to client'); + }, + ); - // Stub route for testing idempotency - fastify.post( - '/api/_test/idempotency', - { schema: { hide: true } }, - async (request, reply) => { - const idempotencyKey = request.headers['idempotency-key']; - if (typeof idempotencyKey === 'string' && idempotencyKey.length > 0) { - const personId = 'test-person'; - const cached = request.server.idempotency.check(personId, idempotencyKey); - if (cached) { - return reply.code(cached.status).send(cached.body); - } + // Stub route for testing idempotency + fastify.post( + '/api/_test/idempotency', + { schema: { hide: true } }, + async (request, reply) => { + const idempotencyKey = request.headers['idempotency-key']; + if (typeof idempotencyKey === 'string' && idempotencyKey.length > 0) { + const personId = 'test-person'; + const cached = request.server.idempotency.check(personId, idempotencyKey); + if (cached) { + return reply.code(cached.status).send(cached.body); + } - const body = ok({ echoed: idempotencyKey, at: new Date().toISOString() }); - request.server.idempotency.store(personId, idempotencyKey, { status: 200, body }); - return reply.code(200).send(body); - } - return ok({ echoed: null }); - }, - ); + const body = ok({ echoed: idempotencyKey, at: new Date().toISOString() }); + request.server.idempotency.store(personId, idempotencyKey, { status: 200, body }); + return reply.code(200).send(body); + } + return ok({ echoed: null }); + }, + ); + } } diff --git a/apps/api/tests/api-skeleton.test.ts b/apps/api/tests/api-skeleton.test.ts index 0562f8d..142c2b9 100644 --- a/apps/api/tests/api-skeleton.test.ts +++ b/apps/api/tests/api-skeleton.test.ts @@ -127,6 +127,43 @@ describe('error mapper', () => { }); }); +// --------------------------------------------------------------------------- +// /api/_test/* gating (issue #116) +// +// The test-harness routes exist only to exercise the error-mapping + +// idempotency code paths from CI. In NODE_ENV=production they must NOT +// be registered — no reason a prod caller should be able to hit +// /api/_test/internal-error and force a 500. +// --------------------------------------------------------------------------- + +describe('/api/_test/* route gating', () => { + it('returns 404 for all three test-harness routes when NODE_ENV=production', async () => { + // Close the default (NODE_ENV=test) app so the prod-mode app gets + // a clean fixture. The base afterEach takes care of the rest. + if (app) { + await app.close(); + app = undefined; + } + const prodApp = await buildTestApp({ NODE_ENV: 'production' }); + try { + const paths = [ + '/api/_test/validation-error', + '/api/_test/internal-error', + '/api/_test/idempotency', + ]; + for (const url of paths) { + const res = await prodApp.inject({ method: 'POST', url }); + expect(res.statusCode, `expected ${url} to 404 in production`).toBe(404); + } + // Sanity: real routes still respond. + const health = await prodApp.inject({ method: 'GET', url: '/api/health' }); + expect(health.statusCode).toBe(200); + } finally { + await prodApp.close(); + } + }); +}); + // --------------------------------------------------------------------------- // Rate limiting // ---------------------------------------------------------------------------