Merge pull request 'fix(api): block open redirects, add health check and body limits, run as non-root' (#4) from fix/api-hardening into main

Reviewed-on: #4

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"]

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

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) => ({'&': '&amp;', '<': '&lt;', '>': '&gt;', "'": '&#39;', '"': '&quot;'}[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) }

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)
+    })
+})

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") ||