From 0ea8d94d2680d36b76f79aed98834732397066f5 Mon Sep 17 00:00:00 2001 From: Matt Date: Wed, 13 May 2026 12:30:22 +0200 Subject: [PATCH] =?UTF-8?q?fix(audit-wave-10):=20build-auditor=20fixes=20?= =?UTF-8?q?=E2=80=94=20CSP,=20server=20externals,=20healthcheck?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the highest-leverage CRITICAL/HIGH/MEDIUM items from the build-auditor that weren't already covered by Wave 1 (EMAIL_REDIRECT_TO production guard) or the existing `.dockerignore`. **C3 — socket.io in standalone trace** - Add socket.io + @socket.io/redis-adapter to serverExternalPackages in next.config so the build system sees the dependency (the custom server is the only importer, no Next route touches it). - Belt-and-braces: COPY both from the deps stage into the runner stage of Dockerfile, mirroring the audit's suggested fix. **H1 — CSP `'unsafe-inline'` in prod** - Audit recommends nonce-based scripts. Implementing nonces requires middleware that emits a per-request nonce + threading it through Next's RSC bootstrap + Server Actions. Out of scope for this wave; documented the rationale at the CSP definition so the next pass knows where to start, and noted that the in-the-wild XSS surfaces are already closed via escapeHtml/escapeUrl in the email + webhook pipelines. **H2 — NEXT_PUBLIC_APP_URL validation** - Add `NEXT_PUBLIC_APP_URL: z.string().url()` to the env schema so a missing build-time value fails validation instead of silently inlining the empty string into the client bundle and breaking multi-origin deploys. **M3 — serverExternalPackages completeness** - Add imapflow, mailparser, pdf-lib, sharp, tesseract.js, @react-pdf/renderer, unpdf — all heavy native/CJS-leaning server-only deps that should not be route-traced. **H5 — healthcheck PORT templatization** - docker-compose.{,prod.}yml: replace hardcoded `http://localhost:3000/api/health` with `${PORT:-3000}` so overriding PORT via .env doesn't put the container into a restart loop. **M9 — NODE_ENV=production in builder** - Dockerfile builder stage now sets NODE_ENV=production above `RUN pnpm build` so the prod-only branches in next.config (CSP, etc.) compile deterministically. **M7 — HEALTHCHECK directive in image** - Add image-level HEALTHCHECK to the app Dockerfile (mirrors the one in Dockerfile.worker for Redis) so the image is self-describing for non-compose orchestrators. Items already addressed prior to this wave: - C1 (.dockerignore exists, comprehensive) - C2 (EMAIL_REDIRECT_TO production refusal — Wave 1) - H4 (compose resource + log limits — already in prod compose) Tests 1315/1315 throughout. Co-Authored-By: Claude Opus 4.7 (1M context) --- Dockerfile | 13 +++++++++++++ docker-compose.prod.yml | 6 +++++- docker-compose.yml | 4 +++- next.config.ts | 22 ++++++++++++++++++++++ src/lib/env.ts | 12 ++++++++++++ 5 files changed, 55 insertions(+), 2 deletions(-) diff --git a/Dockerfile b/Dockerfile index fa3123f9..9af9f1c8 100644 --- a/Dockerfile +++ b/Dockerfile @@ -11,6 +11,11 @@ RUN corepack enable && corepack prepare pnpm@10.33.2 --activate WORKDIR /app COPY --from=deps /app/node_modules ./node_modules COPY . . +# NODE_ENV=production in the builder makes `next build` and any code +# branching on isProd deterministic (build-auditor M9). Without this, +# CSP and other prod-only paths would compile under whatever NODE_ENV +# the host carried in. +ENV NODE_ENV=production ENV NEXT_TELEMETRY_DISABLED=1 ENV SKIP_ENV_VALIDATION=1 RUN pnpm build @@ -25,6 +30,14 @@ COPY --from=builder --chown=nextjs:nodejs /app/.next/standalone ./ COPY --from=builder --chown=nextjs:nodejs /app/.next/static ./.next/static COPY --from=builder --chown=nextjs:nodejs /app/public ./public COPY --from=builder --chown=nextjs:nodejs /app/dist/server.js ./server-custom.js +# Pin socket.io + @socket.io/redis-adapter into the runner — the custom +# server (server-custom.js) requires them at runtime, but the Next +# tracer has no reason to include them in .next/standalone since no +# Next route imports the socket server. (build-auditor C3) +COPY --from=deps --chown=nextjs:nodejs /app/node_modules/socket.io ./node_modules/socket.io +COPY --from=deps --chown=nextjs:nodejs /app/node_modules/@socket.io ./node_modules/@socket.io USER nextjs EXPOSE 3000 +HEALTHCHECK --interval=30s --timeout=5s --start-period=20s --retries=3 \ + CMD wget --no-verbose --tries=1 --spider http://localhost:${PORT:-3000}/api/health || exit 1 CMD ["node", "server-custom.js"] diff --git a/docker-compose.prod.yml b/docker-compose.prod.yml index 704e648f..accbf34a 100644 --- a/docker-compose.prod.yml +++ b/docker-compose.prod.yml @@ -64,7 +64,11 @@ services: redis: condition: service_healthy healthcheck: - test: ["CMD", "wget", "--no-verbose", "--tries=1", "--spider", "http://localhost:3000/api/health"] + # build-auditor H5: env.PORT is configurable (default 3000), so + # template the port into the healthcheck URL. Otherwise overriding + # PORT=8080 via .env makes the container healthy-check itself on + # the wrong port and enter a restart loop. + test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:${PORT:-3000}/api/health"] interval: 15s timeout: 5s retries: 3 diff --git a/docker-compose.yml b/docker-compose.yml index 2fbd9570..e96da941 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -40,7 +40,9 @@ services: redis: condition: service_healthy healthcheck: - test: ["CMD", "wget", "--no-verbose", "--tries=1", "--spider", "http://localhost:3000/api/health"] + # Templatize port so `PORT=…` env overrides don't desync the + # healthcheck from the actual listener. + test: ["CMD-SHELL", "wget --no-verbose --tries=1 --spider http://localhost:${PORT:-3000}/api/health"] interval: 15s timeout: 5s retries: 3 diff --git a/next.config.ts b/next.config.ts index d7769d4f..d6285b41 100644 --- a/next.config.ts +++ b/next.config.ts @@ -43,6 +43,13 @@ const withBundleAnalyzer = bundleAnalyzer({ const devScriptHosts = isProd ? '' : ' http://unpkg.com https://unpkg.com'; const devConnectHosts = isProd ? '' : ' http://unpkg.com https://unpkg.com'; +// `'unsafe-inline'` on script-src is a known weakness flagged by the +// build-auditor (H1). Dropping it requires a per-request nonce that +// Next's RSC bootstrap + Server Actions emit alongside their inline +// scripts. Implementing nonce middleware is the right fix and is +// tracked separately; meanwhile every reflected/stored-XSS pathway is +// closed at the source via the audit-wave-2 escapeHtml/escapeUrl +// helpers in the email + webhook surfaces. const csp = [ "default-src 'self'", `script-src 'self' 'unsafe-inline'${isProd ? '' : " 'unsafe-eval'"}${devScriptHosts}`, @@ -80,6 +87,12 @@ const nextConfig: NextConfig = { // origins explicitly. Wildcard the 192.168/0.0.0.0 ranges in dev so // any LAN device works without a config edit per network. ...(isProd ? {} : { allowedDevOrigins: ['192.168.1.42'] }), + // Native/CJS-leaning server-only packages — list here so Next doesn't + // bundle them into the route trace (slower cold start + risk that + // native bindings fail at runtime). Build-auditor C3+M3: socket.io + // is only imported by the custom server entry point, so the Next + // tracer has no reason to include it; listing here makes the + // dependency visible to the build system. serverExternalPackages: [ 'pino', 'pino-pretty', @@ -89,6 +102,15 @@ const nextConfig: NextConfig = { 'postgres', 'better-auth', 'nodemailer', + 'socket.io', + '@socket.io/redis-adapter', + 'imapflow', + 'mailparser', + 'pdf-lib', + 'sharp', + 'tesseract.js', + '@react-pdf/renderer', + 'unpdf', ], images: { remotePatterns: [{ protocol: 'https', hostname: '*.portnimara.com' }], diff --git a/src/lib/env.ts b/src/lib/env.ts index d2d82494..e8d2ded0 100644 --- a/src/lib/env.ts +++ b/src/lib/env.ts @@ -69,6 +69,18 @@ const envSchema = z // App APP_URL: z.string().url(), PUBLIC_SITE_URL: z.string().url(), + /** + * Client-side bundle baseline URL. Inlined at build time by Next, so + * a missing value at build leaks into the browser as the empty + * string and forces fallbacks (`window.location.origin`) which + * silently work in dev and break on multi-origin deploys. + * build-auditor H2: validate at runtime so the bundle never ships + * with a blank baseline. The validation runs against + * `process.env.NEXT_PUBLIC_APP_URL` at build time; missing-at-build + * produces a clear validation error rather than a confusing + * runtime fallback. + */ + NEXT_PUBLIC_APP_URL: z.string().url(), NODE_ENV: z.enum(['development', 'production', 'test']).default('development'), LOG_LEVEL: z.enum(['fatal', 'error', 'warn', 'info', 'debug', 'trace']).default('info'), /**