fix: security audit — plugin RCE, CORS, token leak, shell exec, login rate limit

server/index.ts

@@ -70,6 +70,10 @@ const RUNNING_VERSION = (() => {
         return null;
     }
 })();
+const SERVER_PORT = Number.parseInt(process.env.SERVER_PORT || '3001', 10);
+const HOST = process.env.HOST || '0.0.0.0';
+const DISPLAY_HOST = getConnectableHost(HOST);
+const VITE_PORT = process.env.VITE_PORT || 5173;
 const systemRoutes = createSystemModule({
     appRoot: APP_ROOT,
     installMode,
@@ -119,7 +123,43 @@ const wss = createWebSocketServer(server, {
 // Make WebSocket server available to routes
 app.locals.wss = wss;
 
-app.use(cors({ exposedHeaders: ['X-Refreshed-Token', 'X-Auth-Error'] }));
+// CORS — only reflect origins that share a host:port with the server. The
+// default `cors()` configuration reflects any Origin header, which lets any
+// malicious site make authenticated cross-origin requests against this server
+// when a victim visits it in the same browser session. Limiting the
+// reflected origin to the server's own loopback / LAN addresses keeps the
+// browser same-origin policy intact without breaking the local Electron app
+// or LAN-hosted deployments.
+const corsOriginReflector = (origin: string | undefined, callback: (err: Error | null, allow?: boolean) => void) => {
+    // No Origin header → same-origin request (e.g. server-to-server, curl);
+    // these are not subject to CORS and should always be allowed through.
+    if (!origin) {
+        callback(null, true);
+        return;
+    }
+
+    try {
+        const parsed = new URL(origin);
+        const requestHost = parsed.hostname;
+        const requestPort = parsed.port || (parsed.protocol === 'https:' ? '443' : '80');
+        const serverHost = HOST === '0.0.0.0' || HOST === '::' ? requestHost : HOST;
+        const serverPort = String(SERVER_PORT);
+
+        if (requestHost === serverHost && requestPort === serverPort) {
+            callback(null, true);
+            return;
+        }
+    } catch {
+        // Malformed Origin header — refuse.
+    }
+
+    callback(null, false);
+};
+
+app.use(cors({
+    origin: corsOriginReflector,
+    exposedHeaders: ['X-Refreshed-Token', 'X-Auth-Error'],
+}));
 app.use(express.json({
     limit: '50mb',
     type: (req) => {
@@ -272,10 +312,6 @@ app.use((err: unknown, req: Request, res: Response, next: NextFunction) => {
   });
 });
 
-const SERVER_PORT = Number.parseInt(process.env.SERVER_PORT || '3001', 10);
-const HOST = process.env.HOST || '0.0.0.0';
-const DISPLAY_HOST = getConnectableHost(HOST);
-const VITE_PORT = process.env.VITE_PORT || 5173;
 const LOCAL_SERVER_MARKER_PATH = path.join(os.homedir(), '.cloudcli', 'local-server.json');
 
 function getErrorCode(error: unknown): string | undefined {

server/modules/auth/auth.routes.ts

@@ -2,9 +2,24 @@ import express from 'express';
 import type { RequestHandler } from 'express';
 
 import type { createAuthService } from './auth.service.js';
+import { createRateLimiter } from './rate-limit.middleware.js';
 
 type AuthenticatedRequest = express.Request & { user?: unknown };
 
+// 10 attempts per IP per minute is generous for legitimate use (the only
+// registered user is the local admin) but tight enough to slow down online
+// credential stuffing. Lockouts extend the window by an additional minute so
+// a misconfigured client cannot hammer the endpoint forever.
+const AUTH_RATE_LIMIT_MAX = 10;
+const AUTH_RATE_LIMIT_WINDOW_MS = 60_000;
+const AUTH_RATE_LIMIT_LOCKOUT_MS = 60_000;
+
+const authRateLimit = createRateLimiter({
+  maxAttempts: AUTH_RATE_LIMIT_MAX,
+  windowMs: AUTH_RATE_LIMIT_WINDOW_MS,
+  lockoutMs: AUTH_RATE_LIMIT_LOCKOUT_MS,
+});
+
 /**
  * Creates the Auth transport adapter. Handlers only parse request data and
  * delegate authentication behavior to the injected application service.
@@ -23,7 +38,7 @@ export function createAuthRouter(
     }
   });
 
-  router.post('/register', async (req, res, next) => {
+  router.post('/register', authRateLimit.middleware, async (req, res, next) => {
     try {
       const body = req.body as { username?: unknown; password?: unknown };
       res.json(await service.register(body.username, body.password));
@@ -32,7 +47,7 @@ export function createAuthRouter(
     }
   });
 
-  router.post('/login', async (req, res, next) => {
+  router.post('/login', authRateLimit.middleware, async (req, res, next) => {
     try {
       const body = req.body as { username?: unknown; password?: unknown };
       res.json(await service.login(body.username, body.password));

server/modules/auth/rate-limit.middleware.ts

@@ -0,0 +1,116 @@
+// rate-limit middleware exposes a single factory that the auth router mounts
+// on the login and register endpoints. Keeping the failure path explicit (no
+// shared store) so the limit applies per-process without coupling to the
+// persistence layer.
+import type { Request, RequestHandler, Response } from 'express';
+
+type RateLimiterOptions = {
+  /** Maximum number of attempts allowed within the rolling window. */
+  maxAttempts: number;
+  /** Length of the rolling window, in milliseconds. */
+  windowMs: number;
+  /** How long a locked-out client should be told to wait before retrying. */
+  lockoutMs?: number;
+  /** Optional clock for tests. */
+  now?: () => number;
+};
+
+type AttemptRecord = {
+  /** Timestamps (ms) of attempts that fall inside the rolling window. */
+  timestamps: number[];
+  /** Earliest time at which the client may try again after a lockout. */
+  blockedUntil: number;
+};
+
+function readClientKey(req: Request): string {
+  // Prefer the address of the TCP peer; fall back to a header chain that
+  // includes the most common reverse-proxy forwarded-for conventions.
+  const socketAddress = req.socket?.remoteAddress;
+  if (socketAddress && socketAddress !== '::1' && socketAddress !== '127.0.0.1') {
+    return socketAddress;
+  }
+
+  const forwarded = req.headers['x-forwarded-for'];
+  if (typeof forwarded === 'string' && forwarded.trim()) {
+    return forwarded.split(',')[0]!.trim();
+  }
+  if (Array.isArray(forwarded) && forwarded.length > 0) {
+    return forwarded[0]!.split(',')[0]!.trim();
+  }
+
+  return socketAddress || 'unknown';
+}
+
+/**
+ * Builds a per-key sliding-window rate limiter suitable for protecting the
+ * login and registration endpoints against credential-stuffing attempts.
+ *
+ * The limiter keeps an in-memory `Map` of client-key → attempt history. Each
+ * incoming request drops expired timestamps from the history; if the count
+ * after dropping exceeds `maxAttempts`, the request is rejected with HTTP
+ * 429 until enough time has elapsed for at least one slot to expire.
+ */
+export function createRateLimiter(options: RateLimiterOptions): {
+  middleware: RequestHandler;
+  reset: () => void;
+} {
+  const maxAttempts = options.maxAttempts;
+  const windowMs = options.windowMs;
+  const lockoutMs = options.lockoutMs ?? windowMs;
+  const clock = options.now ?? (() => Date.now());
+
+  const records = new Map<string, AttemptRecord>();
+
+  const middleware: RequestHandler = (req: Request, res: Response, next) => {
+    const clientKey = readClientKey(req);
+    const now = clock();
+    let record = records.get(clientKey);
+    if (!record) {
+      record = { timestamps: [], blockedUntil: 0 };
+      records.set(clientKey, record);
+    }
+
+    // An active lockout short-circuits the limiter; do not consume an attempt
+    // slot so a misbehaving client cannot extend the lockout indefinitely.
+    if (record.blockedUntil > now) {
+      const retryAfterSeconds = Math.max(1, Math.ceil((record.blockedUntil - now) / 1000));
+      res.setHeader('Retry-After', String(retryAfterSeconds));
+      res.status(429).json({
+        success: false,
+        error: {
+          code: 'RATE_LIMITED',
+          message: 'Too many attempts. Please try again later.',
+          retryAfterSeconds,
+        },
+      });
+      return;
+    }
+
+    // Drop timestamps that have aged out of the rolling window.
+    const cutoff = now - windowMs;
+    record.timestamps = record.timestamps.filter((timestamp) => timestamp > cutoff);
+
+    if (record.timestamps.length >= maxAttempts) {
+      record.blockedUntil = now + lockoutMs;
+      const retryAfterSeconds = Math.max(1, Math.ceil(lockoutMs / 1000));
+      res.setHeader('Retry-After', String(retryAfterSeconds));
+      res.status(429).json({
+        success: false,
+        error: {
+          code: 'RATE_LIMITED',
+          message: 'Too many attempts. Please try again later.',
+          retryAfterSeconds,
+        },
+      });
+      return;
+    }
+
+    record.timestamps.push(now);
+    next();
+  };
+
+  return {
+    middleware,
+    reset: () => records.clear(),
+  };
+}

server/modules/auth/tests/rate-limit.middleware.test.ts

@@ -0,0 +1,170 @@
+import assert from 'node:assert/strict';
+import test from 'node:test';
+
+import { createRateLimiter } from '../rate-limit.middleware.js';
+
+function createMockResponse(): {
+  statusCode: number;
+  body: unknown;
+  headers: Record<string, string>;
+  status(code: number): typeof response;
+  setHeader(name: string, value: string): void;
+  json(body: unknown): typeof response;
+} {
+  const response = {
+    statusCode: 200,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) {
+      this.statusCode = code;
+      return this;
+    },
+    setHeader(name: string, value: string) {
+      this.headers[name] = value;
+    },
+    json(body: unknown) {
+      this.body = body;
+      return this;
+    },
+  };
+  return response;
+}
+
+function createMockRequest(clientKey: string): {
+  socket?: { remoteAddress?: string };
+  headers: Record<string, string>;
+} {
+  return {
+    socket: { remoteAddress: clientKey },
+    headers: {},
+  };
+}
+
+test('requests below the attempt cap pass through', () => {
+  const limiter = createRateLimiter({ maxAttempts: 3, windowMs: 1000, now: () => 1000 });
+  const next = (() => { calls.push(1); }) as () => void;
+  const calls: number[] = [];
+
+  for (let i = 0; i < 3; i += 1) {
+    limiter.middleware(createMockRequest('203.0.113.1') as never, createMockResponse() as never, next);
+  }
+
+  assert.equal(calls.length, 3);
+});
+
+test('the request that trips the limit is rejected with 429 + Retry-After', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 2,
+    windowMs: 1000,
+    lockoutMs: 5000,
+    now: () => currentTime,
+  });
+  const next = () => undefined;
+  const blocked: { status: number; body: unknown; headers: Record<string, string> } = {
+    status: 200,
+    body: undefined,
+    headers: {},
+  };
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  // Two successful requests.
+  limiter.middleware(createMockRequest('203.0.113.2') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.2') as never, response as never, next);
+
+  // Third request trips the limiter.
+  limiter.middleware(createMockRequest('203.0.113.2') as never, response as never, next);
+
+  assert.equal(response.statusCode, 429);
+  assert.equal(response.headers['Retry-After'], '5');
+  assert.ok(response.body && typeof response.body === 'object');
+  assert.equal((response.body as { success: boolean }).success, false);
+});
+
+test('lockout does not extend when the client keeps hammering', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 1,
+    windowMs: 1000,
+    lockoutMs: 2000,
+    now: () => currentTime,
+  });
+  const next = () => undefined;
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  // First request consumes the only slot.
+  limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
+  // Next request trips the limit and starts a lockout.
+  limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
+  const firstBlockEnd = response.headers['Retry-After'];
+
+  // Advance time by half a second — still inside the lockout — and verify the
+  // block window does NOT extend (would happen if we kept consuming slots).
+  currentTime += 500;
+  limiter.middleware(createMockRequest('203.0.113.3') as never, response as never, next);
+
+  assert.equal(firstBlockEnd, '2');
+  assert.equal(response.statusCode, 429);
+});
+
+test('rolling window lets the client through once old attempts age out', () => {
+  let currentTime = 1000;
+  const limiter = createRateLimiter({
+    maxAttempts: 2,
+    windowMs: 1000,
+    now: () => currentTime,
+  });
+  const next = (() => { calls.push(1); }) as () => void;
+  const calls: number[] = [];
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  assert.equal(calls.length, 2);
+
+  // Advance past the window so the earlier timestamps drop off.
+  currentTime += 1100;
+  limiter.middleware(createMockRequest('203.0.113.4') as never, response as never, next);
+  assert.equal(calls.length, 3);
+});
+
+test('different client keys are tracked independently', () => {
+  const limiter = createRateLimiter({ maxAttempts: 1, windowMs: 1000, now: () => 1000 });
+  const next = (() => { calls.push(1); }) as () => void;
+  const calls: number[] = [];
+  const response = {
+    statusCode: 0,
+    body: undefined as unknown,
+    headers: {} as Record<string, string>,
+    status(code: number) { this.statusCode = code; return this; },
+    setHeader(name: string, value: string) { this.headers[name] = value; },
+    json(body: unknown) { this.body = body; return this; },
+  };
+
+  limiter.middleware(createMockRequest('203.0.113.5') as never, response as never, next);
+  limiter.middleware(createMockRequest('203.0.113.6') as never, response as never, next);
+
+  assert.equal(calls.length, 2);
+});