fix(security): post-review hardening batch — command injection, CSP, auth gaps, error sanitization, resilience (#2435)

Integrated into release/v3.8.1

bin/cli/commands/plugin.mjs

@@ -1,10 +1,21 @@
-import { execSync } from "node:child_process";
+import { spawnSync } from "node:child_process";
 import { existsSync, mkdirSync, writeFileSync } from "node:fs";
 import { join } from "node:path";
 import { t } from "../i18n.mjs";
 import { emit } from "../output.mjs";
 import { discoverPlugins } from "../plugins.mjs";
 
+// Run npm with an explicit argument array and no shell. Passing args this way
+// (instead of string-interpolating into `execSync`) prevents a malicious plugin
+// name like `foo; rm -rf ~` or `` foo`id` `` from being interpreted by the shell.
+function runNpm(args) {
+  const res = spawnSync("npm", args, { stdio: "inherit", shell: false });
+  if (res.error) throw res.error;
+  if (typeof res.status === "number" && res.status !== 0) {
+    throw new Error(`npm exited with code ${res.status}`);
+  }
+}
+
 const TEMPLATE_INDEX = `export const meta = {
   name: "PLUGIN_NAME",
   version: "0.1.0",
@@ -69,7 +80,7 @@ export function registerPlugin(program) {
       }
 
       try {
-        execSync(`npm install -g ${pkgName}`, { stdio: "inherit" });
+        runNpm(["install", "-g", pkgName]);
         process.stdout.write(`\n✓ Installed: ${pkgName}\n`);
       } catch {
         process.stderr.write(`✗ Failed to install ${pkgName}\n`);
@@ -91,7 +102,7 @@ export function registerPlugin(program) {
         }
       }
       try {
-        execSync(`npm uninstall -g ${pkgName}`, { stdio: "inherit" });
+        runNpm(["uninstall", "-g", pkgName]);
         process.stdout.write(`✓ Removed: ${pkgName}\n`);
       } catch {
         process.stderr.write(`✗ Failed to remove ${pkgName}\n`);
@@ -144,10 +155,26 @@ export function registerPlugin(program) {
     .command("update [name]")
     .description(t("plugin.update") || "Update installed plugin(s)")
     .action(async (name) => {
-      const pkg = name ? `omniroute-cmd-${name}` : "omniroute-cmd-*";
       try {
-        execSync(`npm update -g ${pkg}`, { stdio: "inherit" });
-        process.stdout.write(`✓ Updated: ${pkg}\n`);
+        if (name) {
+          const pkg = `omniroute-cmd-${name}`;
+          runNpm(["update", "-g", pkg]);
+          process.stdout.write(`✓ Updated: ${pkg}\n`);
+          return;
+        }
+        // No name → update every installed plugin. Enumerate them explicitly
+        // instead of relying on a shell glob (`omniroute-cmd-*`), which never
+        // expands without a shell and would otherwise update nothing.
+        const plugins = await discoverPlugins();
+        const names = plugins
+          .map((p) => p.name)
+          .filter((n) => typeof n === "string" && n.startsWith("omniroute-cmd-"));
+        if (names.length === 0) {
+          process.stdout.write("No plugins installed to update.\n");
+          return;
+        }
+        runNpm(["update", "-g", ...names]);
+        process.stdout.write(`✓ Updated: ${names.join(", ")}\n`);
       } catch {
         process.stderr.write(`✗ Update failed\n`);
         process.exit(1);

electron/main.js

@@ -280,12 +280,12 @@ function installUpdate() {
 // ── Content Security Policy (#15) ──────────────────────────
 function setupContentSecurityPolicy() {
   session.defaultSession.webRequest.onHeadersReceived((details, callback) => {
-    // Determine if the request is for the Next.js development server
-    const isDevUrl =
-      details.url.includes("localhost:20128") || details.url.includes("127.0.0.1:20128");
-
-    // React/Next.js requires unsafe-eval for source maps and Hot Module Replacement (HMR) during development.
-    const scriptSrc = isDevUrl
+    // React/Next.js needs 'unsafe-eval' only for source maps + HMR in development.
+    // Gate it on the real dev flag (isDev = NODE_ENV==="development" || !app.isPackaged),
+    // NOT on the request URL: a packaged production build still talks to its embedded
+    // server on localhost:20128, so a URL-substring check would silently grant
+    // 'unsafe-eval' in production and open a code-injection vector via XSS.
+    const scriptSrc = isDev
       ? "script-src 'self' 'unsafe-inline' 'unsafe-eval' blob:"
       : "script-src 'self' 'unsafe-inline' blob:";
 
@@ -297,14 +297,15 @@ function setupContentSecurityPolicy() {
       "frame-src 'none'",
       "child-src 'none'",
       "form-action 'self'",
-      `connect-src 'self' http://localhost:* ws://localhost:* wss://localhost:* https://*.omniroute.online https://*.omniroute.dev`,
+      // Single connect-src: a duplicate directive is ignored by the browser (first wins),
+      // which previously dropped the 127.0.0.1 origins. Keep both loopback forms here.
+      `connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:* wss://localhost:* wss://127.0.0.1:* https://*.omniroute.online https://*.omniroute.dev`,
       scriptSrc,
       "script-src-attr 'none'",
       "style-src 'self' 'unsafe-inline' https://fonts.googleapis.com",
       "font-src 'self' https://fonts.gstatic.com data:",
       "img-src 'self' data: blob: https:",
       "media-src 'self' data: blob:",
-      `connect-src 'self' http://localhost:* http://127.0.0.1:* ws://localhost:* ws://127.0.0.1:* https://*.omniroute.online https://*.omniroute.dev`,
       "worker-src 'self' blob:",
       "manifest-src 'self'",
     ].join("; ");

open-sse/executors/claude-web.ts

@@ -26,6 +26,7 @@ import { createAutoRefreshMiddleware, refreshCookie } from "../services/claudeWe
 import { getCfClearanceToken, getCacheStatus } from "../services/claudeTurnstileSolver.ts";
 import { normalizeSessionCookieHeader } from "@/lib/providers/webCookieAuth";
 import { randomUUID } from "crypto";
+import { sanitizeErrorMessage } from "../utils/error.ts";
 
 // ─── Constants ──────────────────────────────────────────────────────────────
 const CLAUDE_WEB_API_BASE = "https://claude.ai/api";
@@ -735,7 +736,7 @@ export class ClaudeWebExecutor extends BaseExecutor {
       const errorResp = new Response(
         JSON.stringify({
           error: {
-            message: `Claude Web connection failed: ${errorMessage}`,
+            message: `Claude Web connection failed: ${sanitizeErrorMessage(errorMessage)}`,
             type: "api_connection_error",
           },
         }),

open-sse/executors/codex.ts

@@ -1202,7 +1202,13 @@ export class CodexExecutor extends BaseExecutor {
         "TOKEN_REFRESH",
         `Codex: token refresh failed (${result.error}) — re-authentication required`
       );
-      return result;
+      // Return null (not the error-only object): base.ts spreads any truthy
+      // result onto activeCredentials and persists it via onCredentialsRefreshed.
+      // Spreading `{ error }` would keep the stale/expired accessToken in place
+      // and write garbage to the connection. Returning null leaves the original
+      // credentials untouched so the upstream 401/403 drives the proper
+      // re-auth / mark-expired path instead.
+      return null;
     }
     return result;
   }

open-sse/executors/copilot-web.ts

@@ -17,6 +17,7 @@
 import { BaseExecutor, type ExecuteInput } from "./base.ts";
 import { FETCH_TIMEOUT_MS } from "../config/constants.ts";
 import { createHash, randomBytes } from "node:crypto";
+import { sanitizeErrorMessage } from "../utils/error.ts";
 
 // ─── Constants ──────────────────────────────────────────────────────────────
 
@@ -595,7 +596,9 @@ export class CopilotWebExecutor extends BaseExecutor {
       conversationId = session.conversationId;
       sessionCookies = session.cookies;
     } catch (err) {
-      const msg = err instanceof Error ? err.message : "Failed to start Copilot conversation";
+      const msg = sanitizeErrorMessage(
+        err instanceof Error ? err.message : "Failed to start Copilot conversation"
+      );
       return {
         response: new Response(JSON.stringify({ error: { message: msg } }), {
           status: 502,

open-sse/executors/gemini-web.ts

@@ -14,6 +14,7 @@
  */
 
 import { BaseExecutor, type ExecuteInput } from "./base.ts";
+import { sanitizeErrorMessage } from "../utils/error.ts";
 
 // ─── Constants ──────────────────────────────────────────────────────────────
 
@@ -273,7 +274,9 @@ export class GeminiWebExecutor extends BaseExecutor {
     } catch (error) {
       return {
         response: new Response(
-          JSON.stringify({ error: error instanceof Error ? error.message : "Unknown error" }),
+          JSON.stringify({
+            error: sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error"),
+          }),
           { status: 500, headers: { "Content-Type": "application/json" } }
         ),
         url: GEMINI_URL,

open-sse/services/combo.ts

@@ -1994,12 +1994,12 @@ export async function handleComboChat({
     return comboModelNotFoundResponse("Combo has no executable targets");
   }
 
-  // #1731: Per-request in-memory set of providers whose quota is fully exhausted.
-  // When a target returns a quota-exhausted 429, remaining same-provider targets are skipped.
-  const exhaustedProviders = new Set<string>();
   let globalAttempts = 0;
 
   for (let setTry = 0; setTry <= maxSetRetries; setTry++) {
+    // #1731: Per-set-iteration set of providers whose quota is fully exhausted.
+    // Reset each retry so providers excluded in a previous attempt get another chance.
+    const exhaustedProviders = new Set<string>();
     if (setTry > 0) {
       log.info("COMBO", `All targets failed — retrying set (${setTry}/${maxSetRetries})`);
       await new Promise((resolve) => {

open-sse/services/tokenRefresh.ts

@@ -194,7 +194,10 @@ export async function refreshWindsurfToken(
       expiresIn,
     };
   } catch (error) {
-    log?.error?.("TOKEN_REFRESH", `Network error refreshing Windsurf token: ${error.message}`);
+    log?.error?.(
+      "TOKEN_REFRESH",
+      `Network error refreshing Windsurf token: ${error instanceof Error ? error.message : String(error)}`
+    );
     return null;
   }
 }

src/app/api/oauth/[provider]/[action]/route.ts

@@ -26,6 +26,7 @@ import {
 } from "@/shared/validation/schemas";
 import { isValidationFailure, validateBody } from "@/shared/validation/helpers";
 import { isAuthRequired, isAuthenticated } from "@/shared/utils/apiAuth";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 // Use globalThis to persist callback server state across Next.js HMR reloads
 if (!globalThis.__codexCallbackState) {
@@ -668,7 +669,10 @@ export async function POST(
           },
         });
       } catch (importErr: any) {
-        return NextResponse.json({ success: false, error: importErr.message }, { status: 500 });
+        return NextResponse.json(
+          { success: false, error: sanitizeErrorMessage(importErr.message) || "Import failed" },
+          { status: 500 }
+        );
       }
     }
 
@@ -738,7 +742,10 @@ export async function POST(
           },
         });
       } catch (importErr: any) {
-        return NextResponse.json({ success: false, error: importErr.message }, { status: 500 });
+        return NextResponse.json(
+          { success: false, error: sanitizeErrorMessage(importErr.message) || "Import failed" },
+          { status: 500 }
+        );
       }
     }
 

src/app/api/resilience/reset/route.ts

@@ -1,9 +1,15 @@
 import { NextResponse } from "next/server";
+import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
 
 /**
- * POST /api/resilience/reset — Reset all provider circuit breakers and model lockouts
+ * POST /api/resilience/reset — Reset all provider circuit breakers and model lockouts.
+ *
+ * Requires management auth: flushing every breaker + model lockout disrupts
+ * routing for all traffic, so it must not be reachable unauthenticated.
  */
-export async function POST() {
+export async function POST(request: Request) {
+  const authError = await requireManagementAuth(request);
+  if (authError) return authError;
   try {
     const { getAllCircuitBreakerStatuses, getCircuitBreaker } =
       await import("@/shared/utils/circuitBreaker");
@@ -28,8 +34,7 @@ export async function POST() {
       message: `Reset ${resetCount} circuit breaker(s) and model lockouts`,
     });
   } catch (err: unknown) {
-    const message = err instanceof Error ? err.message : "Failed to reset resilience state";
     console.error("[API] POST /api/resilience/reset error:", err);
-    return NextResponse.json({ error: message }, { status: 500 });
+    return NextResponse.json({ error: "Failed to reset resilience state" }, { status: 500 });
   }
 }

src/app/api/usage/budget/bulk/route.ts

@@ -1,6 +1,7 @@
 import { NextResponse } from "next/server";
 import { getCostSummary, checkBudget } from "@/domain/costRules";
 import { getApiKeys } from "@/lib/db/apiKeys";
+import { requireManagementAuth } from "@/lib/api/requireManagementAuth";
 
 /**
  * GET /api/usage/budget/bulk — Bulk budget summary for every API key.
@@ -8,8 +9,13 @@ import { getApiKeys } from "@/lib/db/apiKeys";
  * Avoids N+1 in dashboard views that need a per-key snapshot of spend and
  * limits in one round-trip. Returns a record keyed by apiKeyId so callers
  * can lookup by id without scanning.
+ *
+ * Requires management auth: this exposes spend + budget limits for ALL keys,
+ * so it must not be reachable without the dashboard/management token.
  */
-export async function GET() {
+export async function GET(request: Request) {
+  const authError = await requireManagementAuth(request);
+  if (authError) return authError;
   try {
     const keys = await getApiKeys();
     const budgets: Record<

src/app/api/v1/agents/tasks/[id]/route.ts

@@ -14,6 +14,7 @@ import {
 } from "@/lib/cloudAgent/api";
 import { z } from "zod";
 import pino from "pino";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const logger = pino({ name: "cloud-agents-api" });
 
@@ -104,7 +105,11 @@ export async function GET(request: NextRequest, { params }: { params: Promise<{
     );
   } catch (error) {
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : "Unknown error" },
+      {
+        error:
+          sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error") ||
+          "Internal server error",
+      },
       { status: 500, headers: getCloudAgentCorsHeaders(request) }
     );
   }
@@ -180,7 +185,11 @@ export async function POST(request: NextRequest, { params }: { params: Promise<{
   } catch (error) {
     logger.error({ err: error }, "Failed to process task action");
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : "Unknown error" },
+      {
+        error:
+          sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error") ||
+          "Internal server error",
+      },
       { status: 500, headers: getCloudAgentCorsHeaders(request) }
     );
   }
@@ -208,7 +217,11 @@ export async function DELETE(
     return NextResponse.json({ success: true }, { headers: getCloudAgentCorsHeaders(request) });
   } catch (error) {
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : "Unknown error" },
+      {
+        error:
+          sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error") ||
+          "Internal server error",
+      },
       { status: 500, headers: getCloudAgentCorsHeaders(request) }
     );
   }

src/app/api/v1/agents/tasks/route.ts

@@ -17,6 +17,7 @@ import {
 } from "@/lib/cloudAgent/api";
 import { CreateCloudAgentTaskSchema } from "@/lib/cloudAgent/types";
 import pino from "pino";
+import { sanitizeErrorMessage } from "@omniroute/open-sse/utils/error";
 
 const logger = pino({ name: "cloud-agents-api" });
 
@@ -59,7 +60,11 @@ export async function GET(request: NextRequest) {
     );
   } catch (error) {
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : "Unknown error" },
+      {
+        error:
+          sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error") ||
+          "Internal server error",
+      },
       { status: 500, headers: getCloudAgentCorsHeaders(request) }
     );
   }
@@ -143,7 +148,11 @@ export async function POST(request: NextRequest) {
   } catch (error) {
     logger.error({ err: error }, "Failed to create cloud agent task");
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : "Unknown error" },
+      {
+        error:
+          sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error") ||
+          "Internal server error",
+      },
       { status: 500, headers: getCloudAgentCorsHeaders(request) }
     );
   }
@@ -171,7 +180,11 @@ export async function DELETE(request: NextRequest) {
     return NextResponse.json({ success: true }, { headers: getCloudAgentCorsHeaders(request) });
   } catch (error) {
     return NextResponse.json(
-      { error: error instanceof Error ? error.message : "Unknown error" },
+      {
+        error:
+          sanitizeErrorMessage(error instanceof Error ? error.message : "Unknown error") ||
+          "Internal server error",
+      },
       { status: 500, headers: getCloudAgentCorsHeaders(request) }
     );
   }

src/shared/utils/circuitBreaker.ts

@@ -110,6 +110,14 @@ export class CircuitBreaker {
         }
         this.failureCount = saved.failureCount;
         this.lastFailureTime = saved.lastFailureTime;
+        const savedKind = saved.options?.lastFailureKind;
+        if (
+          savedKind === "rate_limit" ||
+          savedKind === "quota_exhausted" ||
+          savedKind === "transient"
+        ) {
+          this.lastFailureKind = savedKind;
+        }
         if (this.state === STATE.HALF_OPEN) {
           this.halfOpenAllowed = this.halfOpenRequests;
         }
@@ -133,6 +141,7 @@ export class CircuitBreaker {
           failureThreshold: this.failureThreshold,
           resetTimeout: this.resetTimeout,
           halfOpenRequests: this.halfOpenRequests,
+          lastFailureKind: this.lastFailureKind,
         },
       });
     } catch {