From c5a2fc91053cbb55f3429eccc0f27b9e29b22a7c Mon Sep 17 00:00:00 2001 From: Zakaria Date: Fri, 23 Oct 2026 10:43:41 -0400 Subject: [PATCH] fix(api): block open redirects, add health check and body limits, run as non-root - safeReturnTo resolves the target against a placeholder origin and rejects backslashes and control characters, so `/\evil.example` and similar payloads can no longer redirect off-site after sign-in. - Add a real unauthenticated /api/health and answer unknown /api/* paths with a JSON 404 instead of the SPA shell. - Limit request bodies: 16 KB for /api/auth/* (read before a session exists), 128 MB for resource create/update and uploads, 10 MB elsewhere. - Run the production image as an unprivileged user and add a Docker HEALTHCHECK against /api/health. --- docker/Dockerfile | 7 +++- packages/api/src/auth.test.ts | 21 +++++++++++ packages/api/src/auth.ts | 14 ++++++- packages/api/src/index.hardening.test.ts | 48 ++++++++++++++++++++++++ packages/api/src/index.ts | 22 +++++++++++ 5 files changed, 110 insertions(+), 2 deletions(-) create mode 100644 packages/api/src/index.hardening.test.ts diff --git a/docker/Dockerfile b/docker/Dockerfile index 011e304..7f8d80c 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -23,7 +23,9 @@ RUN bun build --compile --minify src/index.ts --outfile server # Stage 3 — minimal runtime (no bun, no node_modules) FROM alpine:3 -RUN apk add --no-cache ca-certificates libstdc++ +RUN apk add --no-cache ca-certificates libstdc++ \ + && addgroup -S -g 10001 thecloud \ + && adduser -S -D -H -u 10001 -G thecloud thecloud WORKDIR /app COPY --from=api-build /app/server ./server COPY --from=frontend-build /app/packages/frontend/dist ./public @@ -33,4 +35,7 @@ COPY svgAsset/authentik.svg ./public/authentik.svg COPY aws-icons-svg ./public/aws-icons ENV PORT=4500 EXPOSE 4500 +USER thecloud +HEALTHCHECK --interval=30s --timeout=5s --start-period=10s --retries=3 \ + CMD wget -q -O /dev/null "http://127.0.0.1:${PORT:-4500}/api/health" || exit 1 CMD ["./server"] diff --git a/packages/api/src/auth.test.ts b/packages/api/src/auth.test.ts index 84a4fec..2f6f566 100644 --- a/packages/api/src/auth.test.ts +++ b/packages/api/src/auth.test.ts @@ -106,6 +106,27 @@ describe('login throttling', () => { }) }) +describe('post-login redirect', () => { + async function redirectAfterLogin(returnTo: string) { + const response = await request(app(localConfig()), '/api/auth/login', '198.51.100.40', { + method: 'POST', + headers: {cookie: 'floci_ui_csrf=csrf-token', 'content-type': 'application/x-www-form-urlencoded', accept: 'application/json'}, + body: new URLSearchParams({username: 'admin', password: 'correct-horse-battery', csrf: 'csrf-token', returnTo}), + }) + return (await response.json() as {redirectTo: string}).redirectTo + } + + test('keeps same-origin paths with their query and fragment', async () => { + expect(await redirectAfterLogin('/cloud-explorer/aws/storage?search=logs#top')).toBe('/cloud-explorer/aws/storage?search=logs#top') + }) + + test('refuses destinations that a browser would resolve to another origin', async () => { + for (const returnTo of ['//evil.example', '/\\evil.example', '/\\/evil.example', '/\t/evil.example', 'https://evil.example', '/%0d%0a/evil.example'.replace('%0d%0a', '\r\n')]) { + expect(await redirectAfterLogin(returnTo)).toBe('/console/aws') + } + }) +}) + describe('OIDC authorization', () => { let privateKey: CryptoKey let jwk: JWK diff --git a/packages/api/src/auth.ts b/packages/api/src/auth.ts index c28c4fe..c53204e 100644 --- a/packages/api/src/auth.ts +++ b/packages/api/src/auth.ts @@ -290,7 +290,19 @@ function loginErrorMessage(code: string | undefined): string | null { function isPublic(path: string): boolean { return path === '/login' || path === '/login.js' || path === '/TheCloud.png' || path === '/authentik.svg' || path.startsWith('/aws-icons/') || path === '/api/auth/login' || path === '/api/auth/oidc/login' || path === '/api/auth/oidc/callback' || path === '/api/auth/session' || path === '/api/health' } function unauthenticated(c: Context): Response { return c.req.path.startsWith('/api/') ? c.json({error: 'Authentication required'}, 401) : c.redirect(`/login?returnTo=${encodeURIComponent(safeReturnTo(c.req.path + new URL(c.req.url).search))}`, 302) } -function safeReturnTo(value?: string): string { return value && value.startsWith('/') && !value.startsWith('//') ? value : '/console/aws' } +// Browsers treat `\` like `/` and drop tabs/newlines in URLs, so `/\evil.example` would +// otherwise pass a plain prefix check and land on another origin. +function safeReturnTo(value?: string): string { + const fallback = '/console/aws' + if (!value || !value.startsWith('/') || /[\\\u0000-\u001f\u007f]/.test(value)) return fallback + const base = 'http://return-to.invalid' + try { + const url = new URL(value, base) + return url.origin === base ? `${url.pathname}${url.search}${url.hash}` : fallback + } catch { + return fallback + } +} function escapeHtml(value: string): string { return value.replace(/[&<>'"]/g, (character) => ({'&': '&', '<': '<', '>': '>', "'": ''', '"': '"'}[character] ?? character)) } function acceptsHtml(c: Context): boolean { return c.req.header('accept')?.includes('text/html') ?? false } function stateChanging(method: string): boolean { return !['GET', 'HEAD', 'OPTIONS'].includes(method) } diff --git a/packages/api/src/index.hardening.test.ts b/packages/api/src/index.hardening.test.ts new file mode 100644 index 0000000..98c4dd3 --- /dev/null +++ b/packages/api/src/index.hardening.test.ts @@ -0,0 +1,48 @@ +import {describe, expect, test} from 'bun:test' +import server from './index' + +describe('API surface hardening', () => { + test('serves an unauthenticated JSON health check', async () => { + const res = await server.fetch(new Request('http://localhost/api/health')) + + expect(res.status).toBe(200) + expect(await res.json()).toEqual({status: 'ok'}) + }) + + test('answers unknown API paths with JSON instead of the SPA shell', async () => { + const res = await server.fetch(new Request('http://localhost/api/does-not-exist')) + + expect(res.headers.get('content-type')).toContain('application/json') + }) + + test('rejects an oversized sign-in body before parsing it', async () => { + const res = await server.fetch(new Request('http://localhost/api/auth/login', { + method: 'POST', + headers: {'content-type': 'application/x-www-form-urlencoded', accept: 'application/json'}, + body: `username=admin&password=${'x'.repeat(20 * 1024)}`, + })) + + expect(res.status).toBe(413) + expect(await res.json()).toEqual({error: 'Request body is too large'}) + }) + + test('keeps the upload ceiling for object uploads', async () => { + const res = await server.fetch(new Request('http://localhost/api/clouds/aws/services/storage/resources/bucket-1/object?key=big.bin', { + method: 'PUT', + headers: {'content-type': 'application/octet-stream'}, + body: new Uint8Array(11 * 1024 * 1024), + })) + + expect(res.status).toBe(401) + }) + + test('applies the default limit to other API routes', async () => { + const res = await server.fetch(new Request('http://localhost/api/clouds/aws/services/messaging/resources/queue-1/messages', { + method: 'POST', + headers: {'content-type': 'application/json'}, + body: new Uint8Array(11 * 1024 * 1024), + })) + + expect(res.status).toBe(413) + }) +}) diff --git a/packages/api/src/index.ts b/packages/api/src/index.ts index 0abb055..5839c88 100644 --- a/packages/api/src/index.ts +++ b/packages/api/src/index.ts @@ -1,6 +1,7 @@ import "dotenv/config"; import { Hono } from "hono"; import { serveStatic } from "hono/bun"; +import { bodyLimit } from "hono/body-limit"; import { cors } from "hono/cors"; import { logger } from "hono/logger"; import {authConfig, authMiddleware, authRoutes} from './auth'; @@ -44,9 +45,19 @@ app.use( }), ); app.use("*", logger()); +// Sign-in endpoints read the body before a session exists, so they get a small +// limit; uploads (S3 objects, Lambda ZIPs sent as base64, Lightsail bundles) keep +// Bun's default 128 MB ceiling. +app.use("/api/*", (c, next) => + bodyLimit({ + maxSize: requestBodyLimit(c.req.path), + onError: (c) => c.json({ error: "Request body is too large" }, 413), + })(c, next), +); app.use("*", authMiddleware(authentication)); app.use("*", authRoutes(authentication)); +app.get("/api/health", (c) => c.json({ status: "ok" })); app.route("/api/eks", eks); app.route("/api/ec2", ec2); app.route("/api/secretsmanager", secretsmanager); @@ -55,6 +66,7 @@ app.route("/api/msk", msk); app.route("/api/msk-connect", mskConnect); app.route("/api/ses", ses); app.route("/api/elasticache", elasticache); +app.all("/api/*", (c) => c.json({ error: "Not found" }, 404)); // Serve static frontend files when public/ directory is present (production) app.use("*", serveStatic({ root: "./public" })); @@ -63,6 +75,16 @@ app.get("*", serveStatic({ path: "./public/index.html" })); const port = Number(process.env.PORT ?? 4501); export default { port, fetch: app.fetch }; +function requestBodyLimit(path: string): number { + if (path.startsWith("/api/auth/")) return 16 * 1024; + if ( + /^\/api\/clouds\/[^/]+\/services\/[^/]+\/resources(?:\/[^/]+(?:\/object|\/static-files|\/node-application)?)?$/.test(path) + ) { + return 128 * 1024 * 1024; + } + return 10 * 1024 * 1024; +} + function isSensitiveCredentialRoute(path: string): boolean { return ( path.startsWith("/api/secretsmanager") ||