fix(security): write server.env 0600 in a 0700 data dir and repair older installs (GHSA-mh4f-3xj9-4gc4)

changelog.d/fixes/env-writers-private-modes.md

@@ -0,0 +1 @@
+- **Security (GHSA-mh4f-3xj9-4gc4):** `server.env` — which holds the generated `JWT_SECRET`, `STORAGE_ENCRYPTION_KEY` and `API_KEY_SECRET` — was written by `scripts/build/bootstrap-env.mjs` (dev/start, Docker) and by the desktop app without an explicit mode, so under the usual umask it came out 0644 in a 0755 directory. Both writers now create the data dir 0700 and `server.env` 0600, and repair a file an earlier version left world-readable on every start (the file is only rewritten when a secret is missing). Follow-up to GHSA-2pg2-xm9r-8544; the package-root `.env` no longer receives generated secrets since #11436. Reported by @peterbussch.

electron/main.js

@@ -191,6 +191,29 @@ function resolveServerNodePath(env = process.env, extraDirs = []) {
   return entries.join(path.delimiter);
 }
 
+// Private modes for the generated secrets (GHSA-mh4f-3xj9-4gc4) — same contract as
+// scripts/build/bootstrap-env.mjs and bin/cli/privateDataDir.mjs. Best-effort: a no-op on
+// Windows, and a data dir owned by someone else must not stop the app.
+function chmodQuietly(target, mode) {
+  try {
+    fs.chmodSync(target, mode);
+  } catch {
+    /* best-effort */
+  }
+}
+
+function tightenServerEnv(dataDir, serverEnvPath) {
+  try {
+    if (fs.existsSync(dataDir)) {
+      const current = fs.statSync(dataDir).mode & 0o777;
+      if (current & 0o007) chmodQuietly(dataDir, current & ~0o007);
+    }
+    if (fs.existsSync(serverEnvPath)) chmodQuietly(serverEnvPath, 0o600);
+  } catch {
+    /* best-effort */
+  }
+}
+
 function resolveDataDir(overridePath, env = process.env) {
   if (overridePath && overridePath.trim()) return path.resolve(overridePath);
 
@@ -747,6 +770,11 @@ function startNextServer() {
   const preferredEnv = preferredEnvPath ? parseEnvFile(preferredEnvPath) : {};
   const dataDir = resolveDataDir(null, { ...preferredEnv, ...process.env });
   const serverEnvPath = path.join(dataDir, "server.env");
+  // GHSA-mh4f-3xj9-4gc4: server.env holds JWT_SECRET / STORAGE_ENCRYPTION_KEY /
+  // API_KEY_SECRET. Repair a file an earlier version wrote world-readable (and the
+  // data dir's "other" bits) on every start — the write below only runs when a secret
+  // is missing. Best-effort: never block the app on a chmod failure.
+  tightenServerEnv(dataDir, serverEnvPath);
   const persisted = parseEnvFile(serverEnvPath);
   const serverEnv = { ...persisted, ...preferredEnv, ...process.env };
   let changed = false;
@@ -782,14 +810,18 @@ function startNextServer() {
   if (changed) {
     serverEnv.OMNIROUTE_BOOTSTRAPPED = "true";
     try {
-      fs.mkdirSync(dataDir, { recursive: true });
+      if (!fs.existsSync(dataDir)) {
+        fs.mkdirSync(dataDir, { recursive: true, mode: 0o700 });
+        chmodQuietly(dataDir, 0o700);
+      }
       const lines = [
         "# Auto-generated by OmniRoute bootstrap",
         "",
         ...Object.entries(persisted).map(([k, v]) => `${k}=${v}`),
         "",
       ];
-      fs.writeFileSync(serverEnvPath, lines.join("\n"), "utf8");
+      fs.writeFileSync(serverEnvPath, lines.join("\n"), { encoding: "utf8", mode: 0o600 });
+      chmodQuietly(serverEnvPath, 0o600);
       console.log("[Electron] 📁 Secrets persisted to:", serverEnvPath);
     } catch (e) {
       console.warn("[Electron] Could not persist secrets:", e.message);

scripts/build/bootstrap-env.mjs

@@ -19,7 +19,7 @@
  */
 
 import { randomBytes, createDecipheriv, scryptSync, createHash } from "node:crypto";
-import { existsSync, mkdirSync, readFileSync, writeFileSync } from "node:fs";
+import { chmodSync, existsSync, mkdirSync, readFileSync, statSync, writeFileSync } from "node:fs";
 import { createRequire } from "node:module";
 import { homedir } from "node:os";
 import { join, resolve } from "node:path";
@@ -183,7 +183,42 @@ function writeEnvFile(filePath, env) {
     ...Object.entries(env).map(([k, v]) => `${k}=${v}`),
     "",
   ];
-  writeFileSync(filePath, lines.join("\n"), "utf8");
+  writeFileSync(filePath, lines.join("\n"), { encoding: "utf8", mode: 0o600 });
+  // `mode` only applies when the file is created; an existing 0644 file keeps its bits.
+  chmodQuietly(filePath, 0o600);
+}
+
+// ── Private modes for the secrets (GHSA-mh4f-3xj9-4gc4) ───────────────────────
+// server.env holds JWT_SECRET, STORAGE_ENCRYPTION_KEY and API_KEY_SECRET. Without an
+// explicit mode the umask decides (0644 / 0755 under the usual 022), which leaves them
+// readable by other local accounts. Same contract as bin/cli/privateDataDir.mjs
+// (GHSA-2pg2-xm9r-8544). chmod is best-effort: a no-op on Windows, and a DATA_DIR owned
+// by someone else (a bind mount) must not stop the server from starting.
+function chmodQuietly(path, fileMode) {
+  try {
+    chmodSync(path, fileMode);
+  } catch {
+    /* best-effort — see above */
+  }
+}
+
+function ensurePrivateDataDir(dataDir) {
+  if (existsSync(dataDir)) return;
+  mkdirSync(dataDir, { recursive: true, mode: 0o700 });
+  chmodQuietly(dataDir, 0o700);
+}
+
+/** Repair an install an earlier version left world-readable. Never throws. */
+function tightenServerEnv(dataDir, serverEnvPath) {
+  try {
+    if (existsSync(dataDir)) {
+      const current = statSync(dataDir).mode & 0o777;
+      if (current & 0o007) chmodQuietly(dataDir, current & ~0o007);
+    }
+    if (existsSync(serverEnvPath)) chmodQuietly(serverEnvPath, 0o600);
+  } catch {
+    /* best-effort — see above */
+  }
 }
 
 // ── Main bootstrap function ──────────────────────────────────────────────────
@@ -200,6 +235,7 @@ export function bootstrapEnv({ dataDirOverride, quiet = false } = {}) {
   const serverEnvPath = join(dataDir, "server.env");
 
   // ── Layer 1: Load persisted server.env ────────────────────────────────────
+  tightenServerEnv(dataDir, serverEnvPath);
   let persisted = parseEnvFile(serverEnvPath);
 
   // ── Layer 2: Load the same preferred .env that the CLI wrapper uses ───────
@@ -262,7 +298,7 @@ export function bootstrapEnv({ dataDirOverride, quiet = false } = {}) {
   // ── Persist new secrets ────────────────────────────────────────────────────
   if (needsPersist) {
     try {
-      mkdirSync(dataDir, { recursive: true });
+      ensurePrivateDataDir(dataDir);
       // Only persist keys that we auto-generated (not .env or process.env vals)
       writeEnvFile(serverEnvPath, persisted);
       log(`📁 Secrets persisted to: ${serverEnvPath}`);

stryker.conf.json

@@ -523,7 +523,9 @@
       "tests/unit/data-dir-private-perms.test.ts",
       "tests/unit/translation-failure-skips-account-cooldown-14815.test.ts",
       "tests/unit/sudo-password-never-reaches-command-stdin.test.ts",
-      "tests/unit/middleware-hook-realm-isolation.test.ts"
+      "tests/unit/middleware-hook-realm-isolation.test.ts",
+      "tests/unit/bootstrap-env-private-modes.test.ts",
+      "tests/unit/electron-server-env-private-modes.test.ts"
     ],
     "nodeArgs": [
       "--import",

tests/unit/bootstrap-env-private-modes.test.ts

@@ -0,0 +1,96 @@
+// GHSA-mh4f-3xj9-4gc4 (follow-up to GHSA-2pg2-xm9r-8544): bootstrapEnv() writes the
+// generated JWT_SECRET / STORAGE_ENCRYPTION_KEY / API_KEY_SECRET into DATA_DIR/server.env.
+// It created the file and the directory without an explicit mode, so under the usual
+// umask 022 they came out 0644 / 0755 — readable by other local accounts (macOS `staff`,
+// Debian 0755 homes, the Docker ./data bind mount). These cases pin private modes for a
+// fresh write and the repair of a file an earlier version left world-readable.
+// 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";
+
+import { bootstrapEnv } from "../../scripts/build/bootstrap-env.mjs";
+
+const posix = process.platform !== "win32";
+const mode = (p: string) => fs.statSync(p).mode & 0o777;
+const SECRET_KEYS = ["JWT_SECRET", "STORAGE_ENCRYPTION_KEY", "API_KEY_SECRET"];
+
+function withIsolatedEnv(fn: (dataDir: string) => void) {
+  const originalCwd = process.cwd();
+  const originalEnv = { ...process.env };
+  const previousUmask = process.umask(0o022);
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-envmode-"));
+  const home = path.join(root, "home");
+  const cwd = path.join(root, "cwd");
+  fs.mkdirSync(home, { recursive: true });
+  fs.mkdirSync(cwd, { recursive: true });
+  for (const key of [...SECRET_KEYS, "STORAGE_ENCRYPTION_KEY_VERSION", "DATA_DIR"]) {
+    delete process.env[key];
+  }
+  delete process.env.XDG_CONFIG_HOME;
+  process.env.HOME = home;
+  process.chdir(cwd);
+  try {
+    fn(path.join(home, ".omniroute"));
+  } finally {
+    process.chdir(originalCwd);
+    process.umask(previousUmask);
+    for (const key of Object.keys(process.env)) if (!(key in originalEnv)) delete process.env[key];
+    Object.assign(process.env, originalEnv);
+    fs.rmSync(root, { recursive: true, force: true });
+  }
+}
+
+test(
+  "a first run creates the data dir 0700 and server.env 0600 under umask 022",
+  { skip: !posix },
+  () => {
+    withIsolatedEnv((dataDir) => {
+      bootstrapEnv({ quiet: true });
+      const serverEnv = path.join(dataDir, "server.env");
+      assert.equal(mode(dataDir), 0o700);
+      assert.equal(mode(serverEnv), 0o600);
+      const written = fs.readFileSync(serverEnv, "utf8");
+      for (const key of SECRET_KEYS) assert.match(written, new RegExp(`^${key}=\\S`, "m"));
+    });
+  }
+);
+
+test(
+  "a server.env an earlier version left 0644 is tightened on the next start",
+  { skip: !posix },
+  () => {
+    withIsolatedEnv((dataDir) => {
+      bootstrapEnv({ quiet: true });
+      const serverEnv = path.join(dataDir, "server.env");
+      fs.chmodSync(serverEnv, 0o644);
+      fs.chmodSync(dataDir, 0o755);
+
+      // Nothing is missing, so nothing is rewritten — the repair must still happen.
+      bootstrapEnv({ quiet: true });
+      assert.equal(mode(serverEnv), 0o600);
+      assert.equal(mode(dataDir) & 0o007, 0, "no world access to the data dir");
+    });
+  }
+);
+
+test(
+  "rewriting an existing 0644 server.env to add a missing secret keeps it 0600",
+  { skip: !posix },
+  () => {
+    withIsolatedEnv((dataDir) => {
+      bootstrapEnv({ quiet: true });
+      const serverEnv = path.join(dataDir, "server.env");
+      fs.writeFileSync(
+        serverEnv,
+        fs.readFileSync(serverEnv, "utf8").replace(/^API_KEY_SECRET=.*\n/m, "")
+      );
+      fs.chmodSync(serverEnv, 0o644);
+      bootstrapEnv({ quiet: true });
+      assert.equal(mode(serverEnv), 0o600);
+      assert.match(fs.readFileSync(serverEnv, "utf8"), /^API_KEY_SECRET=\S/m);
+    });
+  }
+);

tests/unit/electron-server-env-private-modes.test.ts

@@ -0,0 +1,35 @@
+// GHSA-mh4f-3xj9-4gc4: the desktop app writes the same generated secrets as
+// scripts/build/bootstrap-env.mjs into <dataDir>/server.env. electron/main.js exports
+// nothing, so this pins the contract at the source: private modes on write, and a repair
+// of an earlier world-readable file before it is read. The behaviour itself is covered for
+// the shared logic in bootstrap-env-private-modes.test.ts.
+import test from "node:test";
+import assert from "node:assert/strict";
+import fs from "node:fs";
+import path from "node:path";
+
+const src = fs.readFileSync(path.join(process.cwd(), "electron/main.js"), "utf8");
+
+test("electron main still resolves the data dir and persists server.env", () => {
+  assert.match(src, /function resolveDataDir\(overridePath, env = process\.env\) \{/);
+  assert.match(src, /const serverEnvPath = path\.join\(dataDir, "server\.env"\);/);
+});
+
+test("server.env is written 0600 and chmod-ed after the write", () => {
+  assert.match(
+    src,
+    /fs\.writeFileSync\(serverEnvPath, lines\.join\("\\n"\), \{ encoding: "utf8", mode: 0o600 \}\);\s*chmodQuietly\(serverEnvPath, 0o600\);/
+  );
+});
+
+test("a new data dir is created 0700", () => {
+  assert.match(src, /fs\.mkdirSync\(dataDir, \{ recursive: true, mode: 0o700 \}\);/);
+});
+
+test("an existing server.env is repaired before it is read", () => {
+  assert.match(
+    src,
+    /tightenServerEnv\(dataDir, serverEnvPath\);\s*const persisted = parseEnvFile\(serverEnvPath\);/
+  );
+  assert.match(src, /if \(fs\.existsSync\(serverEnvPath\)\) chmodQuietly\(serverEnvPath, 0o600\);/);
+});