diff --git a/apps/api/src/common/http-response.interceptor.ts b/apps/api/src/common/http-response.interceptor.ts index b105403..c06e82e 100644 --- a/apps/api/src/common/http-response.interceptor.ts +++ b/apps/api/src/common/http-response.interceptor.ts @@ -9,11 +9,21 @@ import type { Observable } from "rxjs"; import { map } from "rxjs/operators"; import type { RequestWithId } from "./request-id.middleware.js"; -const isEnvelope = (value: unknown): value is SuccessResponse => +const SAFE_REQUEST_ID = /^[A-Za-z0-9._:-]{1,128}$/; + +export const isSuccessEnvelope = ( + value: unknown, +): value is SuccessResponse => typeof value === "object" && value !== null && "code" in value && - "requestId" in value; + value.code === "OK" && + "message" in value && + value.message === "success" && + "data" in value && + "requestId" in value && + typeof value.requestId === "string" && + SAFE_REQUEST_ID.test(value.requestId); @Injectable() export class HttpResponseInterceptor implements NestInterceptor< @@ -28,7 +38,9 @@ export class HttpResponseInterceptor implements NestInterceptor< return next .handle() .pipe( - map((data) => (isEnvelope(data) ? data : ok(data, request.requestId))), + map((data) => + isSuccessEnvelope(data) ? data : ok(data, request.requestId), + ), ); } } diff --git a/apps/api/src/common/request-id.middleware.ts b/apps/api/src/common/request-id.middleware.ts index 486252c..45a9392 100644 --- a/apps/api/src/common/request-id.middleware.ts +++ b/apps/api/src/common/request-id.middleware.ts @@ -5,6 +5,9 @@ import type { NextFunction, Request, Response } from "express"; export const REQUEST_ID_HEADER = "x-request-id"; const SAFE_REQUEST_ID = /^[A-Za-z0-9._:-]{1,128}$/; +export const sanitizeRequestId = (supplied: string | undefined): string => + supplied && SAFE_REQUEST_ID.test(supplied) ? supplied : randomUUID(); + export interface RequestWithId extends Request { requestId: string; } @@ -13,8 +16,7 @@ export interface RequestWithId extends Request { export class RequestIdMiddleware implements NestMiddleware { use(request: Request, response: Response, next: NextFunction): void { const supplied = request.header(REQUEST_ID_HEADER); - const requestId = - supplied && SAFE_REQUEST_ID.test(supplied) ? supplied : randomUUID(); + const requestId = sanitizeRequestId(supplied); (request as RequestWithId).requestId = requestId; response.setHeader(REQUEST_ID_HEADER, requestId); next(); diff --git a/apps/api/src/health/health.e2e-spec.ts b/apps/api/src/health/health.e2e-spec.ts index b3a7a80..656be1e 100644 --- a/apps/api/src/health/health.e2e-spec.ts +++ b/apps/api/src/health/health.e2e-spec.ts @@ -1,22 +1,37 @@ import "reflect-metadata"; /* eslint-disable @typescript-eslint/no-unsafe-argument, @typescript-eslint/no-unsafe-member-access */ -import { Body, Controller, Get, HttpStatus, Post } from "@nestjs/common"; +import { + Body, + Controller, + Get, + HttpStatus, + Post, + type INestApplication, +} from "@nestjs/common"; import { Test } from "@nestjs/testing"; -import { IsString } from "class-validator"; +import { ErrorCode, ok } from "@drift/contracts"; +import { Type } from "class-transformer"; +import { IsInt, IsString } from "class-validator"; import request from "supertest"; -import { afterAll, beforeAll, describe, expect, it } from "vitest"; -import type { INestApplication } from "@nestjs/common"; -import { ErrorCode } from "@drift/contracts"; -import { AppModule } from "../app.module"; -import { DomainException } from "../common/domain.exception"; - -import { configureApp } from "../main"; +import { afterAll, beforeAll, describe, expect, it, vi } from "vitest"; +import { AppModule } from "../app.module.js"; +import { DomainException } from "../common/domain.exception.js"; +import { sanitizeRequestId } from "../common/request-id.middleware.js"; +import { PrismaService } from "../database/prisma.service.js"; +import { bootstrap, configureApp } from "../main.js"; +import { HealthService } from "./health.service.js"; class ProbeDto { @IsString() name!: string; } +class TransformProbeDto { + @Type(() => Number) + @IsInt() + count!: number; +} + @Controller("probe") class ProbeController { @Post() @@ -24,6 +39,25 @@ class ProbeController { return dto; } + @Post("transform") + transform(@Body() dto: TransformProbeDto): { value: number; type: string } { + return { value: dto.count, type: typeof dto.count }; + } + + @Get("envelope") + envelope() { + return ok({ already: "wrapped" }, "controller-request-id"); + } + + @Get("envelope-lookalike") + envelopeLookalike() { + return { + code: "BUSINESS_CODE", + requestId: "business-request-id", + value: 1, + }; + } + @Get("domain-error") domainError(): never { throw new DomainException( @@ -45,6 +79,12 @@ Reflect.defineMetadata( ProbeController.prototype, "echo", ); +Reflect.defineMetadata( + "design:paramtypes", + [TransformProbeDto], + ProbeController.prototype, + "transform", +); describe("API infrastructure", () => { let app: INestApplication; @@ -97,6 +137,16 @@ describe("API infrastructure", () => { }, ); + it.each(["line\rbreak", "line\nbreak", "line\r\nbreak"])( + "sanitizes CR/LF in request ids without relying on an HTTP client accepting it", + (unsafeId) => { + const sanitized = sanitizeRequestId(unsafeId); + expect(sanitized).toMatch(/^[0-9a-f-]{36}$/); + expect(sanitized).not.toContain("\r"); + expect(sanitized).not.toContain("\n"); + }, + ); + it("returns a uniform 404 envelope", async () => { const response = await request(app.getHttpServer()) .get("/api/v1/missing") @@ -144,6 +194,39 @@ describe("API infrastructure", () => { }); }); + it("transforms DTO values before invoking the handler", async () => { + const response = await request(app.getHttpServer()) + .post("/api/v1/probe/transform") + .send({ count: "42" }) + .expect(201); + expect(response.body.data).toEqual({ value: 42, type: "number" }); + }); + + it("does not double-wrap a valid SuccessResponse returned by a controller", async () => { + const response = await request(app.getHttpServer()) + .get("/api/v1/probe/envelope") + .expect(200); + expect(response.body).toEqual({ + code: "OK", + message: "success", + data: { already: "wrapped" }, + requestId: "controller-request-id", + }); + }); + + it("wraps a business object that merely resembles an envelope", async () => { + const response = await request(app.getHttpServer()) + .get("/api/v1/probe/envelope-lookalike") + .expect(200); + expect(response.body.code).toBe("OK"); + expect(response.body.message).toBe("success"); + expect(response.body.data).toEqual({ + code: "BUSINESS_CODE", + requestId: "business-request-id", + value: 1, + }); + }); + it("applies helmet and same-origin credentialed CORS", async () => { const allowedOrigin = process.env.WEB_ORIGIN ?? "http://localhost:3000"; const allowed = await request(app.getHttpServer()) @@ -153,6 +236,8 @@ describe("API infrastructure", () => { expect(allowed.headers["access-control-allow-origin"]).toBe(allowedOrigin); expect(allowed.headers["access-control-allow-credentials"]).toBe("true"); expect(allowed.headers["x-content-type-options"]).toBe("nosniff"); + + // Standard CORS rejects browser access by omitting ACAO, not by changing HTTP status. const denied = await request(app.getHttpServer()) .get("/api/v1/health/live") .set("Origin", "https://evil.example") @@ -160,7 +245,15 @@ describe("API infrastructure", () => { expect(denied.headers).not.toHaveProperty("access-control-allow-origin"); }); - it("returns a sanitized 503 when a real dependency is unavailable", async () => { + it("allows non-browser requests without an Origin header", async () => { + const response = await request(app.getHttpServer()) + .get("/api/v1/health/live") + .expect(200); + expect(response.body.data).toEqual({ status: "ok" }); + expect(response.headers).not.toHaveProperty("access-control-allow-origin"); + }); + + it("returns a sanitized 503 specifically when Redis is unavailable", async () => { const originalRedisUrl = process.env.REDIS_URL; process.env.REDIS_URL = "redis://127.0.0.1:1"; const module = await Test.createTestingModule({ @@ -184,4 +277,73 @@ describe("API infrastructure", () => { process.env.REDIS_URL = originalRedisUrl; } }); + + it("returns a sanitized 503 specifically when PostgreSQL is unavailable", async () => { + const module = await Test.createTestingModule({ imports: [AppModule] }) + .overrideProvider(PrismaService) + .useValue({ + $queryRaw: vi + .fn() + .mockRejectedValue(new Error("secret postgres failure")), + }) + .compile(); + const unavailableApp = module.createNestApplication(); + configureApp(unavailableApp); + await unavailableApp.init(); + try { + const response = await request(unavailableApp.getHttpServer()) + .get("/api/v1/health") + .expect(503); + expect(response.body).toMatchObject({ + code: "SERVICE_UNAVAILABLE", + message: "Dependencies unavailable", + data: null, + }); + expect(JSON.stringify(response.body)).not.toContain( + "secret postgres failure", + ); + } finally { + await unavailableApp.close(); + } + }); + + it("closes the real Redis client during module destruction", async () => { + const module = await Test.createTestingModule({ + imports: [AppModule], + }).compile(); + const lifecycleApp = module.createNestApplication(); + configureApp(lifecycleApp); + await lifecycleApp.init(); + const health = module.get(HealthService); + await health.check(); + const redis = health.redisClient; + expect(redis.isOpen).toBe(true); + await lifecycleApp.close(); + expect(redis.isOpen).toBe(false); + await expect(redis.ping()).rejects.toThrow(); + }); + + it("bootstraps with numeric PORT, public host, and shutdown hooks", async () => { + const previousPort = process.env.PORT; + process.env.PORT = "4312"; + const appDouble = { + setGlobalPrefix: vi.fn(), + use: vi.fn(), + useGlobalPipes: vi.fn(), + useGlobalInterceptors: vi.fn(), + useGlobalFilters: vi.fn(), + enableCors: vi.fn(), + enableShutdownHooks: vi.fn(), + listen: vi.fn().mockResolvedValue(undefined), + }; + try { + await bootstrap(() => + Promise.resolve(appDouble as unknown as INestApplication), + ); + expect(appDouble.enableShutdownHooks).toHaveBeenCalledOnce(); + expect(appDouble.listen).toHaveBeenCalledWith(4312, "0.0.0.0"); + } finally { + process.env.PORT = previousPort; + } + }); }); diff --git a/apps/api/src/health/health.service.ts b/apps/api/src/health/health.service.ts index 694c84a..07f41a8 100644 --- a/apps/api/src/health/health.service.ts +++ b/apps/api/src/health/health.service.ts @@ -9,6 +9,10 @@ import { HttpStatus } from "@nestjs/common"; export class HealthService implements OnModuleDestroy { private readonly redis: RedisClientType; + get redisClient(): RedisClientType { + return this.redis; + } + constructor(@Inject(PrismaService) private readonly prisma: PrismaService) { this.redis = createClient({ url: process.env.REDIS_URL ?? "redis://127.0.0.1:6379", diff --git a/apps/api/src/main.ts b/apps/api/src/main.ts index 6e1dbf7..f71c81f 100644 --- a/apps/api/src/main.ts +++ b/apps/api/src/main.ts @@ -30,8 +30,12 @@ export function configureApp(app: INestApplication): void { }); } -async function bootstrap(): Promise { - const app = await NestFactory.create(AppModule); +type AppFactory = () => Promise; + +export async function bootstrap( + appFactory: AppFactory = () => NestFactory.create(AppModule), +): Promise { + const app = await appFactory(); configureApp(app); app.enableShutdownHooks(); await app.listen(Number(process.env.PORT ?? 3001), "0.0.0.0");