Merge remote-tracking branch 'origin/release/v3.8.51' into fix/data-dir-private-perms

# Conflicts:
#	stryker.conf.json

bin/cli/privateDataDir.mjs

@@ -0,0 +1,62 @@
+// Private-by-default helpers for the OmniRoute data directory (GHSA-2pg2-xm9r-8544).
+//
+// DATA_DIR holds `.env` (STORAGE_ENCRYPTION_KEY — the key to every credential in
+// storage.sqlite) and the database itself. Creating them without an explicit mode lets the
+// process umask decide; under the common umask 002 that is 0775 / 0664, i.e. readable by
+// every local user. These helpers make the private mode explicit, the same convention the
+// CLI already follows elsewhere (contexts.mjs, model-preferences.mjs, setup-qwen.mjs).
+//
+// chmod is best-effort on purpose: on Windows it is a no-op, and a DATA_DIR owned by another
+// user (a bind mount, a shared volume) must not stop the server from starting.
+
+import { chmodSync, existsSync, mkdirSync, statSync, writeFileSync } from "node:fs";
+import { join } from "node:path";
+
+const SECRET_FILES = [".env", "server.env"];
+
+/** Create the data directory owner-only (0700). No-op when it already exists. */
+export function ensurePrivateDataDir(dataDir) {
+  if (existsSync(dataDir)) return;
+  mkdirSync(dataDir, { recursive: true, mode: 0o700 });
+  // `mode` is still filtered by the umask on some platforms; pin it explicitly.
+  try {
+    chmodSync(dataDir, 0o700);
+  } catch {
+    /* best-effort — see header */
+  }
+}
+
+/** Write a secrets file owner-only (0600), tightening it if it already existed. */
+export function writePrivateFile(filePath, content) {
+  writeFileSync(filePath, content, { encoding: "utf-8", mode: 0o600 });
+  // `mode` only applies when the file is created — an existing 0664 file keeps its bits.
+  try {
+    chmodSync(filePath, 0o600);
+  } catch {
+    /* best-effort — see header */
+  }
+}
+
+/**
+ * Repair an existing install on startup: drop "other" access from the data dir and make the
+ * secrets files 0600. Group bits on the directory are left alone — user-private groups are
+ * the norm, and a deliberate group share must not break on upgrade; world access is what
+ * leaked. Never throws.
+ */
+export function tightenDataDirSecrets(dataDir) {
+  try {
+    if (!existsSync(dataDir)) return;
+    const current = statSync(dataDir).mode & 0o777;
+    if (current & 0o007) chmodSync(dataDir, current & ~0o007);
+  } catch {
+    /* best-effort — see header */
+  }
+  for (const name of SECRET_FILES) {
+    try {
+      const filePath = join(dataDir, name);
+      if (existsSync(filePath)) chmodSync(filePath, 0o600);
+    } catch {
+      /* best-effort — see header */
+    }
+  }
+}

bin/omniroute.mjs

@@ -14,7 +14,7 @@
  * All other commands are routed through Commander (bin/cli/program.mjs).
  */
 
-import { existsSync, readFileSync, writeFileSync } from "node:fs";
+import { existsSync, readFileSync } from "node:fs";
 import { join, dirname } from "node:path";
 import { fileURLToPath, pathToFileURL } from "node:url";
 let updateNotifier = null;
@@ -30,6 +30,11 @@ import { shouldProvisionStorageKey } from "./cli/utils/storageKeyProvision.mjs";
 import { isVersionFastPath } from "./cli/utils/versionFastPath.mjs";
 import { parseEnvValue } from "./cli/utils/parseEnvValue.mjs";
 import { describeVolatileEnvWarning } from "./cli/utils/volatileEnvPath.mjs";
+import {
+  ensurePrivateDataDir,
+  tightenDataDirSecrets,
+  writePrivateFile,
+} from "./cli/privateDataDir.mjs";
 
 const __filename = fileURLToPath(import.meta.url);
 const __dirname = dirname(__filename);
@@ -119,7 +124,7 @@ function migrateElectronServerEnv(dataDir) {
     const envPath = join(dataDir, ".env");
     const serverEnvPath = join(dataDir, "server.env");
     if (existsSync(envPath) || !existsSync(serverEnvPath)) return;
-    writeFileSync(envPath, readFileSync(serverEnvPath, "utf-8"), "utf-8");
+    writePrivateFile(envPath, readFileSync(serverEnvPath, "utf-8"));
     console.log(`  \x1b[2m♻ Migrated Electron secrets from ${serverEnvPath} to ${envPath}\x1b[0m`);
   } catch {
     // Ignore errors migrating server.env — fall back to normal env loading below.
@@ -229,10 +234,15 @@ loadEnvFile();
 // mutate the data dir.
 if (shouldProvisionStorageKey(process.argv)) {
   const { randomBytes } = await import("node:crypto");
-  const { existsSync, mkdirSync, readFileSync, writeFileSync } = await import("node:fs");
+  const { existsSync, readFileSync } = await import("node:fs");
   const { join } = await import("node:path");
   const { homedir } = await import("node:os");
 
+  // GHSA-2pg2-xm9r-8544: installs created before the fix have a world-readable .env and a
+  // world-traversable data dir. Repair them on every run that touches encrypted storage
+  // (best-effort; group bits are kept). Informational commands never reach this block.
+  tightenDataDirSecrets(process.env.DATA_DIR || join(homedir(), ".omniroute"));
+
   if (!process.env.STORAGE_ENCRYPTION_KEY) {
     // Persist the key into DATA_DIR when set — that's the directory mounted as a volume in
     // Docker (where storage.sqlite lives), so the key survives `docker down` / `docker pull`.
@@ -256,9 +266,8 @@ if (shouldProvisionStorageKey(process.argv)) {
       );
     } else {
       // First run (no database yet) — generate and persist a fresh key.
-      if (!existsSync(dataDir)) {
-        mkdirSync(dataDir, { recursive: true });
-      }
+      // GHSA-2pg2-xm9r-8544: owner-only — .env holds the key to every stored credential.
+      ensurePrivateDataDir(dataDir);
 
       const key = randomBytes(32).toString("hex");
 
@@ -272,7 +281,7 @@ if (shouldProvisionStorageKey(process.argv)) {
       if (!content.includes("STORAGE_ENCRYPTION_KEY=")) {
         const separator = content.trim() ? "\n" : "";
         const newContent = content.trimEnd() + separator + `STORAGE_ENCRYPTION_KEY=${key}`;
-        writeFileSync(envPath, newContent + "\n", "utf-8");
+        writePrivateFile(envPath, newContent + "\n");
         console.log(`  \x1b[2m✨ Generated STORAGE_ENCRYPTION_KEY in ${envPath}\x1b[0m`);
       }
 

changelog.d/fixes/data-dir-private-permissions.md

@@ -0,0 +1 @@
+- **Security — data directory permissions (GHSA-2pg2-xm9r-8544):** the CLI created `~/.omniroute` (or `DATA_DIR`) and wrote `.env` — which holds `STORAGE_ENCRYPTION_KEY`, the key to every stored credential — without an explicit mode, so under the common `umask 002` they landed as `0775`/`0664` and any local user could read the key and the database. First runs now create the directory `0700` and write `.env` `0600` (also when migrating Electron's `server.env`), and every run that touches encrypted storage repairs existing installs: world access is removed from the directory and `.env`/`server.env` become `0600`. Group bits on the directory are kept so deliberate group shares keep working; informational commands (`--version`, `--help`) never touch the directory.

stryker.conf.json

@@ -508,7 +508,8 @@
       "tests/unit/combo-ref-capabilities-14232.test.ts",
       "tests/unit/intelligent-routing-weight-edit.test.ts",
       "tests/unit/noauth-model-lockout-retryable.test.ts",
-      "tests/unit/noauth-refresh-guard.test.ts"
+      "tests/unit/noauth-refresh-guard.test.ts",
+      "tests/unit/data-dir-private-perms.test.ts"
     ],
     "nodeArgs": [
       "--import",

tests/unit/data-dir-private-perms.test.ts

@@ -0,0 +1,77 @@
+// GHSA-2pg2-xm9r-8544: on first run the CLI created ~/.omniroute and wrote .env (which holds
+// STORAGE_ENCRYPTION_KEY, the key to every credential in storage.sqlite) without an explicit
+// mode, so under the common umask 002 the directory was 0775 and .env 0664 — any other local
+// user could read the key and the database. These cases pin the private-by-default contract
+// of the helpers bin/omniroute.mjs now uses. POSIX-only: Windows has no mode bits to check.
+
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+
+const { ensurePrivateDataDir, writePrivateFile, tightenDataDirSecrets } =
+  await import("../../bin/cli/privateDataDir.mjs");
+
+const posix = process.platform !== "win32";
+const mode = (p: string) => fs.statSync(p).mode & 0o777;
+
+let root: string;
+let previousUmask: number;
+
+test.beforeEach(() => {
+  root = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-perms-"));
+  previousUmask = process.umask(0o002); // the umask from the report
+});
+
+test.afterEach(() => {
+  process.umask(previousUmask);
+  fs.rmSync(root, { recursive: true, force: true });
+});
+
+test("a fresh data dir is created 0700 even under umask 002", { skip: !posix }, () => {
+  const dataDir = path.join(root, "fresh", ".omniroute");
+  ensurePrivateDataDir(dataDir);
+  assert.equal(mode(dataDir), 0o700);
+});
+
+test("the secrets file is written 0600 even under umask 002", { skip: !posix }, () => {
+  const envPath = path.join(root, ".env");
+  writePrivateFile(envPath, "STORAGE_ENCRYPTION_KEY=abc\n");
+  assert.equal(mode(envPath), 0o600);
+  assert.equal(fs.readFileSync(envPath, "utf8"), "STORAGE_ENCRYPTION_KEY=abc\n");
+});
+
+test("rewriting an existing world-readable secrets file also tightens it", { skip: !posix }, () => {
+  const envPath = path.join(root, ".env");
+  fs.writeFileSync(envPath, "OLD=1\n", { mode: 0o664 });
+  fs.chmodSync(envPath, 0o664);
+  writePrivateFile(envPath, "OLD=1\nSTORAGE_ENCRYPTION_KEY=abc\n");
+  assert.equal(mode(envPath), 0o600);
+});
+
+test(
+  "an existing install is repaired on startup: world bits off the dir, .env/server.env 0600",
+  { skip: !posix },
+  () => {
+    const dataDir = path.join(root, ".omniroute");
+    fs.mkdirSync(dataDir, { mode: 0o775 });
+    fs.chmodSync(dataDir, 0o775);
+    for (const name of [".env", "server.env"]) {
+      fs.writeFileSync(path.join(dataDir, name), "K=v\n");
+      fs.chmodSync(path.join(dataDir, name), 0o664);
+    }
+
+    tightenDataDirSecrets(dataDir);
+
+    // Group bits are left alone on purpose: user-private groups are the norm and a deliberate
+    // group share (e.g. a docker group) must not break on upgrade. "Other" is what leaked.
+    assert.equal(mode(dataDir) & 0o007, 0, "no world access to the data dir");
+    assert.equal(mode(path.join(dataDir, ".env")), 0o600);
+    assert.equal(mode(path.join(dataDir, "server.env")), 0o600);
+  }
+);
+
+test("repair is best-effort: a missing dir or file never throws", () => {
+  assert.doesNotThrow(() => tightenDataDirSecrets(path.join(root, "does-not-exist")));
+});