fix: security and reliability fixes from audit

Dockerfile

@@ -37,6 +37,10 @@ RUN pnpm prune --prod --ignore-scripts
 FROM node:20-slim AS runtime
 ENV NODE_ENV="production"
 ENV PORT="5173"
+# Bind all interfaces so published ports work; the bind guard in
+# instrument.server.mjs then requires BOLT_AUTH_TOKEN to actually start,
+# because the runtime API grants shell execution and file access.
+ENV HOST="0.0.0.0"
 WORKDIR /app
 
 # git: needed by api.git-info.ts (execSync('git ...'))
@@ -53,11 +57,14 @@ COPY --from=build /app/build ./build
 # Copy package.json (needed by @react-router/serve)
 COPY --from=build /app/package.json ./
 
+# Copy the server instrumentation / bind-policy guard loaded via --import
+COPY --from=build /app/instrument.server.mjs ./
+
 # Non-root user for security
 RUN groupadd --system --gid 1001 appgroup && \
     useradd --system --uid 1001 --gid appgroup --create-home appuser && \
     chown -R appuser:appgroup /app
 USER appuser
 
 EXPOSE 5173
-CMD ["node", "node_modules/@react-router/serve/dist/cli.js", "./build/server/index.js"]
+CMD ["node", "--import", "./instrument.server.mjs", "node_modules/@react-router/serve/dist/cli.js", "./build/server/index.js"]

app/lib/api/cookies.ts

@@ -20,26 +20,26 @@ export function setDecryptor(fn: DecryptFn): void {
  * Attempt to decrypt a cookie value. Values prefixed with "enc:" are treated
  * as encrypted; all others pass through unchanged (plaintext migration).
  *
- * On decryption failure (key rotation, corruption), falls back to returning
- * the raw ciphertext without the prefix — never throws.
+ * On decryption failure (missing decryptor, key rotation, corruption) returns
+ * `null` so the caller can drop the value. Returning the raw ciphertext would
+ * hand the encrypted blob downstream as if it were a real API key, producing
+ * confusing auth failures against providers. Never throws.
  */
-function decryptCookieValue(value: string): string {
+function decryptCookieValue(value: string): string | null {
   if (!value.startsWith(ENC_PREFIX)) {
     return value;
   }
 
-  const ciphertext = value.slice(ENC_PREFIX.length);
-
   if (!_decryptor) {
-    logger.warn('Encrypted cookie value found but no decryptor registered, returning raw ciphertext');
-    return ciphertext;
+    logger.warn('Encrypted cookie value found but no decryptor registered; dropping value');
+    return null;
   }
 
   try {
-    return _decryptor(ciphertext);
+    return _decryptor(value.slice(ENC_PREFIX.length));
   } catch (error) {
-    logger.warn('Failed to decrypt cookie value, falling back to raw ciphertext:', error);
-    return ciphertext;
+    logger.warn('Failed to decrypt cookie value; dropping value:', error);
+    return null;
   }
 }
 
@@ -84,7 +84,14 @@ export function getApiKeysFromCookie(cookieHeader: string | null): Record<string
 
     for (const [provider, value] of Object.entries(keys)) {
       if (typeof value === 'string' && value.startsWith(ENC_PREFIX)) {
-        keys[provider] = decryptCookieValue(value);
+        const decrypted = decryptCookieValue(value);
+
+        // Drop keys we can't decrypt rather than leaking ciphertext downstream.
+        if (decrypted === null) {
+          delete keys[provider];
+        } else {
+          keys[provider] = decrypted;
+        }
       }
     }
 

app/lib/security.ts

@@ -78,15 +78,23 @@ export function checkRateLimit(request: Request, endpoint: string): { allowed: b
 }
 
 /**
- * Get client IP address from request
+ * Get client IP address from request.
+ *
+ * Forwarding headers (`x-forwarded-for`, `x-real-ip`, `cf-connecting-ip`) are
+ * client-controlled and trivially spoofed, so an attacker could bypass rate
+ * limiting by rotating them. Only trust them when the deployment sits behind a
+ * known proxy and opts in via `TRUST_PROXY_HEADERS=true`. Otherwise fall back to
+ * a single shared bucket, which cannot be spoofed apart.
  */
 function getClientIP(request: Request): string {
-  // Try various headers that might contain the real IP
-  const forwardedFor = request.headers.get('x-forwarded-for');
-  const realIP = request.headers.get('x-real-ip');
+  if (process.env.TRUST_PROXY_HEADERS !== 'true') {
+    return 'local';
+  }
+
   const cfConnectingIP = request.headers.get('cf-connecting-ip');
+  const realIP = request.headers.get('x-real-ip');
+  const forwardedFor = request.headers.get('x-forwarded-for');
 
-  // Return the first available IP or a fallback
   return cfConnectingIP || realIP || forwardedFor?.split(',')[0]?.trim() || 'unknown';
 }
 

app/routes/api.runtime.terminal.ts

@@ -76,6 +76,11 @@
         start(controller) {
           const encoder = new TextEncoder();
 
+          // Track listeners registered on sessions so we can remove them when
+          // the client disconnects — otherwise every reconnect leaks a listener
+          // that keeps enqueuing on a closed controller.
+          const disposers: Array<() => void> = [];
+
           // Search all runtimes for the session
           for (const projectId of manager.listProjects()) {
             // We need to use an async IIFE to manage the Promise-based getRuntime
@@ -89,14 +94,35 @@
                 }
 
                 // Register data listener for this session
-                session.dataListeners.push((data: string) => {
+                const onData = (data: string) => {
                   try {
                     const payload = JSON.stringify({ type: 'data', data });
                     controller.enqueue(encoder.encode(`data: ${payload}\n\n`));
                   } catch {
                     // Stream may have been closed
                   }
-                });
+                };
+                session.dataListeners.push(onData);
+
+                const dispose = () => {
+                  const idx = session.dataListeners.indexOf(onData);
+
+                  if (idx !== -1) {
+                    session.dataListeners.splice(idx, 1);
+                  }
+                };
+
+                /*
+                 * If the client already disconnected while we were resolving the
+                 * runtime, remove the listener immediately — the abort handler
+                 * ran before this disposer was registered.
+                 */
+                if (request.signal.aborted) {
+                  dispose();
+                  return;
+                }
+
+                disposers.push(dispose);
 
                 // Listen for process exit
                 session.exitPromise
@@ -147,6 +173,10 @@
           request.signal.addEventListener('abort', () => {
             clearInterval(heartbeat);
 
+            for (const dispose of disposers) {
+              dispose();
+            }
+
             try {
               controller.close();
             } catch {

instrument.server.mjs

@@ -1,3 +1,25 @@
+/*
+ * Enforce a safe bind policy before the server starts. The app exposes a
+ * runtime API that executes arbitrary shell commands and reads/writes files, so
+ * it must never listen on a non-loopback interface without authentication.
+ * Default to loopback; require BOLT_AUTH_TOKEN to bind anything else. This runs
+ * for every entrypoint that loads this file via `node --import`.
+ */
+{
+  const host = process.env.HOST;
+  const isLoopback = !host || host === '127.0.0.1' || host === '::1' || host === 'localhost';
+
+  if (!host) {
+    process.env.HOST = '127.0.0.1';
+  } else if (!isLoopback && !process.env.BOLT_AUTH_TOKEN) {
+    console.error(
+      `[Bolt] Refusing to bind ${host} without authentication: the runtime API grants shell execution ` +
+        `and file access. Set BOLT_AUTH_TOKEN to expose the server beyond localhost, or unset HOST to bind 127.0.0.1.`,
+    );
+    process.exit(1);
+  }
+}
+
 import * as Sentry from '@sentry/node';
 
 const dsn = process.env.SENTRY_DSN;

server.ts

@@ -18,6 +18,19 @@ import { handleUpgrade } from './app/lib/.server/ws/ws-server';
 
 const PORT = Number(process.env.PORT) || 5173;
 
+/*
+ * Bind to loopback by default. The app exposes a runtime API that executes
+ * arbitrary shell commands and reads/writes files, so a non-loopback bind
+ * (e.g. HOST=0.0.0.0 for Docker port publishing) is only allowed when
+ * BOLT_AUTH_TOKEN is configured — otherwise anyone who can reach the port gets
+ * host command execution. Set HOST + BOLT_AUTH_TOKEN to expose it deliberately.
+ */
+const HOST = process.env.HOST || '127.0.0.1';
+
+function isLoopbackHost(host: string): boolean {
+  return host === '127.0.0.1' || host === '::1' || host === 'localhost';
+}
+
 function log(level: string, ...args: unknown[]) {
   const timestamp = new Date().toISOString();
 
@@ -159,9 +172,18 @@ server.on('upgrade', (req, socket, head) => {
   socket.destroy();
 });
 
-server.listen(PORT, () => {
-  log('info', `Bolt server listening on http://localhost:${PORT}`);
-  log('info', `WebSocket endpoint: ws://localhost:${PORT}/ws`);
+if (!isLoopbackHost(HOST) && !process.env.BOLT_AUTH_TOKEN) {
+  log(
+    'error',
+    `Refusing to bind ${HOST} without authentication: the runtime API grants shell execution and file access. ` +
+      `Set BOLT_AUTH_TOKEN to expose the server beyond localhost, or unset HOST to bind 127.0.0.1.`,
+  );
+  process.exit(1);
+}
+
+server.listen(PORT, HOST, () => {
+  log('info', `Bolt server listening on http://${HOST}:${PORT}`);
+  log('info', `WebSocket endpoint: ws://${HOST}:${PORT}/ws`);
 });
 
 /*