fix(security): v3.8.15 hardening follow-ups (Seg2/Seg3/Seg4/Bug3) (#5512)

Security v3.8.15 hardening follow-ups: Seg2 (CHANGEME boot warn), Seg3 (auth_token cookie maxAge 30d), Seg4 (VS Code path-token once-per-process warning), Bug3 (real global install path resolution), Bug1 (segment-match node_modules in auto-update detection). All 5 carry TDD regression guards.

CHANGELOG.md

@@ -10,6 +10,8 @@
 
 ### 🔧 Bug Fixes
 
+- **Security hardening follow-ups (v3.8.15):** the `auth_token` cookie now sets an explicit 30-day `maxAge` so sessions persist as intended (Seg3); the management bootstrap warns at boot when `INITIAL_PASSWORD` is left at the insecure `CHANGEME` default (Seg2); VS Code path-token endpoints (`/api/v1/vscode/raw/[token]`) emit a once-per-process security warning since the API key travels in the URL and can leak via logs/proxies (Seg4); the system version route resolves the real global install path via `npm root -g` instead of a hardcoded `/app` (Bug3); and auto-update mode detection segment-matches `node_modules` instead of substring-matching, eliminating false "global install" positives (Bug1).
+
 ### 📝 Maintenance
 
 ---

src/app/api/auth/login/route.ts

@@ -142,6 +142,9 @@ export async function POST(request) {
         secure: useSecureCookie,
         sameSite: "lax",
         path: "/",
+        // 30 days — bound the cookie lifetime to the JWT's 30d expiry so the browser
+        // drops it on the same schedule the token stops being valid (Seg3 hardening).
+        maxAge: 60 * 60 * 24 * 30,
       });
 
       logAuditEvent({

src/app/api/system/version/route.ts

@@ -18,6 +18,7 @@ import {
 } from "@/lib/system/autoUpdate";
 import { NEWS_JSON_URL, parseActiveNewsPayload } from "@/shared/utils/releaseNotes";
 import { isNewer, resolveLatestVersion } from "@/lib/system/versionCheck";
+import { resolveGlobalOmniroutePath } from "@/lib/system/globalPackagePath";
 
 const execFileAsync = promisify(execFile);
 
@@ -302,10 +303,7 @@ export async function POST(req: NextRequest) {
           status: "running",
           message: "Rebuilding native modules (better-sqlite3)...",
         });
-          const globalRoot = (
-            await execFileAsync("npm", ["root", "-g"], { timeout: 10000, cwd: PROJECT_ROOT })
-          ).stdout.trim();
-        const omniPath = `${globalRoot}/omniroute/app`;
+        const omniPath = await resolveGlobalOmniroutePath();
         await execFileAsync(
           "npm",
           ["rebuild", "better-sqlite3"],

src/app/api/v1/vscode/raw/[token]/tokenizedRequest.ts

@@ -1 +1,5 @@
-export { withPathTokenApiKey, withSanitizedPathTokenApiKey } from "@/lib/vscode/tokenizedRequest";
+export {
+  withPathTokenApiKey,
+  withSanitizedPathTokenApiKey,
+  __vscodeRawInternals,
+} from "@/lib/vscode/tokenizedRequest";

src/lib/auth/managementPassword.ts

@@ -4,13 +4,17 @@ import { getSettings, updateSettings } from "@/lib/db/settings";
 const BCRYPT_HASH_PATTERN = /^\$2[aby]\$\d{2}\$[./A-Za-z0-9]{53}$/;
 const MANAGEMENT_PASSWORD_SALT_ROUNDS = 12;
 
+// Well-known placeholder shipped in `.env.example` (INITIAL_PASSWORD=CHANGEME). Bootstrapping
+// with it leaves the dashboard open to anyone, so we warn loudly on boot (Seg2 hardening).
+const INSECURE_DEFAULT_PASSWORDS = new Set(["CHANGEME"]);
+
 type JsonRecord = Record<string, unknown>;
 
 type MigrationSource = "stored_hash" | "stored_plaintext" | "env" | "missing";
 
 interface EnsureManagementPasswordOptions {
   initialPassword?: string | null;
-  logger?: Pick<Console, "log">;
+  logger?: Pick<Console, "log"> & Partial<Pick<Console, "warn">>;
   settings?: JsonRecord;
   source?: string;
 }
@@ -69,6 +73,15 @@ export async function ensurePersistentManagementPasswordHash(
     storedPassword ||
     getInitialPasswordValue(options.initialPassword ?? process.env.INITIAL_PASSWORD);
 
+  if (bootstrapPassword && INSECURE_DEFAULT_PASSWORDS.has(bootstrapPassword)) {
+    const warn = options.logger?.warn?.bind(options.logger) ?? console.warn;
+    warn(
+      '[AUTH][SECURITY] Management password is set to the well-known default "CHANGEME" ' +
+        "(INITIAL_PASSWORD in .env.example). Anyone can sign in to the dashboard with it — " +
+        "change it immediately via the dashboard or a strong INITIAL_PASSWORD."
+    );
+  }
+
   if (!bootstrapPassword) {
     return {
       hash: null,

src/lib/system/autoUpdate.ts

@@ -64,6 +64,41 @@ function normalizeMode(raw: string | undefined): AutoUpdateMode {
   return "npm";
 }
 
+/**
+ * Match `node_modules` as a real path **segment**, not a substring — a folder literally named
+ * "my-node_modules-backup" must NOT count as an installed-package location. Operates on a local
+ * filesystem path (`__dirname`), never untrusted input; the pattern is linear (ReDoS-safe).
+ *
+ * @internal — exported for testability.
+ */
+export function isUnderNodeModules(dir: string): boolean {
+  return /(^|[/\\])node_modules([/\\]|$)/.test(dir);
+}
+
+/**
+ * Decide the effective auto-update channel when the operator left it at the default ("npm").
+ *
+ * - A source checkout (`.git` present) always self-updates via git, even if it also lives under a
+ *   `node_modules` path.
+ * - Otherwise "npm" mode only makes sense when the running module sits under a real
+ *   `node_modules/` path segment (a global or local package install). Anything else — a downloaded
+ *   build/zip with no `.git` — is treated as source.
+ *
+ * Behavior matches the previous inline heuristic for every realistic path; it only tightens the
+ * pathological case where "node_modules" appeared as a substring but not a path segment (Bug 1,
+ * security-report v3.8.15). Pure + injectable so the branch logic is unit-testable.
+ *
+ * @internal — exported for testability.
+ */
+export function resolveAutoUpdateMode(
+  rawMode: AutoUpdateMode,
+  detection: { isGitRepo: boolean; currentDir: string }
+): AutoUpdateMode {
+  if (rawMode !== "npm") return rawMode;
+  if (detection.isGitRepo) return "source";
+  return isUnderNodeModules(detection.currentDir) ? "npm" : "source";
+}
+
 async function pathExists(targetPath: string): Promise<boolean> {
   try {
     await access(targetPath);
@@ -92,13 +127,7 @@ export function getAutoUpdateConfig(env: NodeJS.ProcessEnv = process.env): AutoU
   if (mode === "npm") {
     const isGitRepo = existsSync(path.join(PROJECT_ROOT, ".git"));
     const currentDir = typeof __dirname !== "undefined" ? __dirname : PROJECT_ROOT;
-    const isGlobalNodeModules = currentDir.includes("node_modules");
-
-    // If we are not in a global node_modules directory, we are likely a local source install/build.
-    // Even if .git is missing (downloaded zip), we should treat it as source.
-    if (isGitRepo || !isGlobalNodeModules) {
-      mode = "source";
-    }
+    mode = resolveAutoUpdateMode(mode, { isGitRepo, currentDir });
   }
 
   return {

src/lib/system/globalPackagePath.ts

@@ -0,0 +1,49 @@
+import { execFile } from "child_process";
+import { promisify } from "util";
+import { existsSync } from "fs";
+import path from "path";
+import { PROJECT_ROOT } from "./autoUpdate";
+
+const execFileAsync = promisify(execFile);
+
+type ExecFileLike = (
+  file: string,
+  args: string[],
+  options: { timeout?: number; cwd?: string }
+) => Promise<{ stdout: string | Buffer }>;
+
+type ExistsLike = (target: string) => boolean;
+
+/**
+ * Resolve the real install directory of the globally-installed `omniroute` package — the directory
+ * that owns `node_modules/better-sqlite3` and so is the correct cwd for `npm rebuild`.
+ *
+ * Replaces the hardcoded `${globalRoot}/omniroute/app` assumption (Bug 3, security-report v3.8.15):
+ * the global package root is `${npm root -g}/omniroute`, not `/omniroute/app`. We probe the real
+ * layout (current root first, then the legacy `app/` sub-dir) and fall back to the package root.
+ *
+ * `execImpl`/`fsExists` are injectable so the resolution logic is unit-testable without a real
+ * global install.
+ */
+export async function resolveGlobalOmniroutePath(
+  execImpl: ExecFileLike = execFileAsync,
+  fsExists: ExistsLike = existsSync
+): Promise<string> {
+  const result = await execImpl("npm", ["root", "-g"], {
+    timeout: 10000,
+    cwd: PROJECT_ROOT,
+  });
+  const globalRoot = String(result.stdout).trim();
+
+  const packageRoot = path.join(globalRoot, "omniroute");
+  // [current layout, legacy layout] — first whose package.json exists wins.
+  const candidates = [packageRoot, path.join(packageRoot, "app")];
+
+  for (const candidate of candidates) {
+    if (fsExists(path.join(candidate, "package.json"))) {
+      return candidate;
+    }
+  }
+
+  return packageRoot;
+}

src/lib/vscode/tokenizedRequest.ts

@@ -1,5 +1,26 @@
 import { sanitizeVscodeRequest } from "@/app/api/v1/vscode/contextSanitizer";
 
+// Path-token endpoints carry the API key in the URL (by design, for Ollama/VS Code clients that
+// cannot send an Authorization header). URLs leak via access logs, proxies, and browser history,
+// so we alert the operator once per process when such an endpoint is exercised (Seg4 hardening).
+let hasWarnedTokenInUrl = false;
+
+export const __vscodeRawInternals = {
+  resetTokenInUrlWarning() {
+    hasWarnedTokenInUrl = false;
+  },
+};
+
+function warnTokenInUrlOnce() {
+  if (hasWarnedTokenInUrl) return;
+  hasWarnedTokenInUrl = true;
+  console.warn(
+    "[VSCODE][SECURITY] A path-token endpoint (/api/v1/vscode/raw/[token] or /api/v1/vscode/[token]) " +
+      "was used. The API key travels in the request URL and can leak via access logs, proxies, and " +
+      "browser history. Prefer the Authorization header where the client supports it."
+  );
+}
+
 function inferTokenFromVscodePath(request: Request) {
   try {
     const url = new URL(request.url, "http://localhost");
@@ -27,6 +48,8 @@ export function withPathTokenApiKey(request: Request, token?: string) {
   const resolvedToken = token || inferTokenFromVscodePath(request);
   if (!resolvedToken) return request;
 
+  warnTokenInUrlOnce();
+
   const headers = new Headers(request.headers);
 
   if (!headers.has("x-api-key")) {

tests/unit/auth-login-route.test.ts

@@ -107,3 +107,29 @@ test("auth login route lazily migrates INITIAL_PASSWORD to a persisted hash befo
     true
   );
 });
+
+test("auth login route sets a bounded maxAge on the auth_token cookie (Seg3)", async () => {
+  process.env.INITIAL_PASSWORD = "bootstrap-secret";
+  const setCalls: unknown[][] = [];
+  loginRoute.authRouteInternals.getCookieStore = async () => ({
+    set: (...args: unknown[]) => setCalls.push(args),
+  });
+
+  const response = await loginRoute.POST(
+    new Request("http://localhost/api/auth/login", {
+      method: "POST",
+      headers: { "content-type": "application/json" },
+      body: JSON.stringify({ password: "bootstrap-secret" }),
+    })
+  );
+
+  assert.equal(response.status, 200);
+  assert.equal(setCalls.length, 1);
+  const [cookieName, , options] = setCalls[0] as [string, string, Record<string, unknown>];
+  assert.equal(cookieName, "auth_token");
+  // 30 days in seconds — must match the JWT 30d expiry so the cookie is not an open-ended
+  // session cookie outliving its token.
+  assert.equal(options.maxAge, 60 * 60 * 24 * 30);
+  assert.equal(options.httpOnly, true);
+  assert.equal(options.path, "/");
+});

tests/unit/auto-update-mode-detection.test.ts

@@ -0,0 +1,71 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const { resolveAutoUpdateMode, isUnderNodeModules } = await import(
+  "../../src/lib/system/autoUpdate.ts"
+);
+
+test("non-npm modes pass through untouched (operator choice wins)", () => {
+  assert.equal(
+    resolveAutoUpdateMode("source", { isGitRepo: false, currentDir: "/x" }),
+    "source"
+  );
+  assert.equal(
+    resolveAutoUpdateMode("docker-compose", {
+      isGitRepo: true,
+      currentDir: "/x/node_modules/y",
+    }),
+    "docker-compose"
+  );
+});
+
+test("npm + git repo → source (a source checkout self-updates via git)", () => {
+  assert.equal(
+    resolveAutoUpdateMode("npm", {
+      isGitRepo: true,
+      currentDir: "/home/me/omniroute/dist/lib/system",
+    }),
+    "source"
+  );
+});
+
+test("npm + global install under node_modules → npm", () => {
+  assert.equal(
+    resolveAutoUpdateMode("npm", {
+      isGitRepo: false,
+      currentDir: "/usr/lib/node_modules/omniroute/dist/lib/system",
+    }),
+    "npm"
+  );
+});
+
+test("npm + no git + not under node_modules → source (downloaded build/zip)", () => {
+  assert.equal(
+    resolveAutoUpdateMode("npm", {
+      isGitRepo: false,
+      currentDir: "/opt/omniroute/dist/lib/system",
+    }),
+    "source"
+  );
+});
+
+test("Bug1: a substring-only node_modules path is not treated as an install", () => {
+  // The old heuristic (`currentDir.includes("node_modules")`) returned "npm" for this path,
+  // misclassifying it as a global install. The segment match treats it as source.
+  assert.equal(isUnderNodeModules("/opt/my-node_modules-backup/dist"), false);
+  assert.equal(
+    resolveAutoUpdateMode("npm", {
+      isGitRepo: false,
+      currentDir: "/opt/my-node_modules-backup/dist",
+    }),
+    "source"
+  );
+});
+
+test("isUnderNodeModules matches real segments on both path separators", () => {
+  assert.equal(isUnderNodeModules("/usr/lib/node_modules/omniroute"), true);
+  assert.equal(isUnderNodeModules("C:\\Users\\me\\node_modules\\omniroute"), true);
+  assert.equal(isUnderNodeModules("/usr/lib/node_modules"), true); // trailing segment
+  assert.equal(isUnderNodeModules("/opt/app/dist"), false);
+  assert.equal(isUnderNodeModules("/opt/mynode_modulesbar/dist"), false);
+});

tests/unit/global-package-path.test.ts

@@ -0,0 +1,37 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import path from "node:path";
+
+const mod = await import("../../src/lib/system/globalPackagePath.ts");
+
+const GLOBAL_ROOT = "/usr/lib/node_modules";
+const PACKAGE_ROOT = path.join(GLOBAL_ROOT, "omniroute");
+const LEGACY_ROOT = path.join(PACKAGE_ROOT, "app");
+
+function execStub(stdout: string) {
+  return async () => ({ stdout });
+}
+
+test("resolves the package root when its package.json exists (Bug3)", async () => {
+  const exists = (target: string) => target === path.join(PACKAGE_ROOT, "package.json");
+  const result = await mod.resolveGlobalOmniroutePath(execStub(`${GLOBAL_ROOT}\n`), exists);
+  assert.equal(result, PACKAGE_ROOT);
+});
+
+test("falls back to the legacy app/ layout when only it has a package.json", async () => {
+  const exists = (target: string) => target === path.join(LEGACY_ROOT, "package.json");
+  const result = await mod.resolveGlobalOmniroutePath(execStub(GLOBAL_ROOT), exists);
+  assert.equal(result, LEGACY_ROOT);
+});
+
+test("defaults to the package root when neither layout is present", async () => {
+  const exists = () => false;
+  const result = await mod.resolveGlobalOmniroutePath(execStub(GLOBAL_ROOT), exists);
+  assert.equal(result, PACKAGE_ROOT);
+});
+
+test("trims whitespace from the npm root -g output", async () => {
+  const exists = (target: string) => target === path.join(PACKAGE_ROOT, "package.json");
+  const result = await mod.resolveGlobalOmniroutePath(execStub(`  ${GLOBAL_ROOT}  \n`), exists);
+  assert.equal(result, PACKAGE_ROOT);
+});

tests/unit/management-password-insecure-default.test.ts

@@ -0,0 +1,64 @@
+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 TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-mgmt-pwd-insecure-"));
+process.env.DATA_DIR = TEST_DATA_DIR;
+
+const core = await import("../../src/lib/db/core.ts");
+const managementPassword = await import("../../src/lib/auth/managementPassword.ts");
+
+function makeLogger() {
+  const warnings: string[] = [];
+  return {
+    warnings,
+    log() {},
+    warn: (...args: unknown[]) => {
+      warnings.push(args.map((a) => String(a)).join(" "));
+    },
+  };
+}
+
+test.afterEach(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+  fs.mkdirSync(TEST_DATA_DIR, { recursive: true });
+});
+
+test.after(() => {
+  core.resetDbInstance();
+  fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true });
+});
+
+test("warns when bootstrapping the management password with the CHANGEME default (Seg2)", async () => {
+  const logger = makeLogger();
+
+  const result = await managementPassword.ensurePersistentManagementPasswordHash({
+    settings: {},
+    initialPassword: "CHANGEME",
+    logger,
+  });
+
+  // It still bootstraps (does not hard-reject — would break local dev), but it must warn loudly.
+  assert.equal(managementPassword.isBcryptHash(result.hash), true);
+  assert.equal(
+    logger.warnings.some((line) => line.includes("CHANGEME")),
+    true,
+    "expected a security warning mentioning the CHANGEME default"
+  );
+});
+
+test("does not warn when bootstrapping with a strong password", async () => {
+  const logger = makeLogger();
+
+  const result = await managementPassword.ensurePersistentManagementPasswordHash({
+    settings: {},
+    initialPassword: "a-strong-unique-password",
+    logger,
+  });
+
+  assert.equal(managementPassword.isBcryptHash(result.hash), true);
+  assert.equal(logger.warnings.length, 0, "did not expect any security warning for a strong password");
+});

tests/unit/vscode-token-in-url-warning.test.ts

@@ -0,0 +1,57 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+
+const tokenizedRequest = await import(
+  "../../src/app/api/v1/vscode/raw/[token]/tokenizedRequest.ts"
+);
+
+function captureWarn<T>(fn: () => T): { result: T; warnings: string[] } {
+  const warnings: string[] = [];
+  const original = console.warn;
+  console.warn = (...args: unknown[]) => {
+    warnings.push(args.map((a) => String(a)).join(" "));
+  };
+  try {
+    return { result: fn(), warnings };
+  } finally {
+    console.warn = original;
+  }
+}
+
+test.beforeEach(() => {
+  tokenizedRequest.__vscodeRawInternals.resetTokenInUrlWarning();
+});
+
+test("warns once when a path token is used, then stays quiet (Seg4)", () => {
+  const makeReq = () =>
+    new Request("http://localhost/api/v1/vscode/raw/sk-secret-token/models");
+
+  const first = captureWarn(() => tokenizedRequest.withPathTokenApiKey(makeReq()));
+  assert.equal(
+    first.warnings.some((line) => line.includes("[VSCODE][SECURITY]")),
+    true,
+    "expected a security warning on the first path-token request"
+  );
+
+  const second = captureWarn(() => tokenizedRequest.withPathTokenApiKey(makeReq()));
+  assert.equal(second.warnings.length, 0, "must not warn again within the same process");
+});
+
+test("propagates the path token into x-api-key / authorization headers", () => {
+  const request = new Request("http://localhost/api/v1/vscode/raw/sk-secret-token/models");
+  const { result } = captureWarn(() => tokenizedRequest.withPathTokenApiKey(request));
+
+  assert.equal(result.headers.get("x-api-key"), "sk-secret-token");
+  assert.equal(result.headers.get("authorization"), "Bearer sk-secret-token");
+});
+
+test("does not warn when there is no resolvable token", () => {
+  // No /vscode segment → inferTokenFromVscodePath returns null and the request passes through.
+  const request = new Request("http://localhost/api/v1/models");
+  const { result, warnings } = captureWarn(() =>
+    tokenizedRequest.withPathTokenApiKey(request)
+  );
+
+  assert.equal(warnings.length, 0);
+  assert.equal(result.headers.get("x-api-key"), null);
+});