fix(cli): register reset-password subcommand + non-TTY stdin path (#6261, #6258)

CHANGELOG.md

@@ -21,6 +21,8 @@
 
 ### 🐛 Bug Fixes
 
+- **fix(cli):** `omniroute reset-password` now works as a real subcommand, and password resets over piped (non-TTY) stdin actually apply ([#6261](https://github.com/diegosouzapw/OmniRoute/issues/6261), [#6258](https://github.com/diegosouzapw/OmniRoute/issues/6258)). Two coupled defects: (1) **#6261** — `bin/omniroute.mjs` routed everything through Commander with only two pre-Commander bypasses (`--mcp`, `reset-encrypted-columns`), so `omniroute reset-password` was rejected as an unknown command; only the separate `omniroute-reset-password` bin worked, while the docs falsely advertised the subcommand (incl. a bogus "legacy alias still works"). A pre-Commander bypass mirroring `reset-encrypted-columns` now dynamically imports `bin/reset-password.mjs` (which self-executes) before Commander parses; the three doc lines were corrected. (2) **#6258** — `bin/reset-password.mjs` issued two sequential `rl.question` prompts; under piped stdin the second read never settled at EOF, so `main()` never reached `resetManagementPassword` and the reset was a silent no-op (both prompts printed, no success, password unchanged). The CLI now detects non-TTY stdin and reads it once (first line = password, second line = confirm if present, else reused), adds a `--password-stdin` flag (entire stdin is the password, no confirmation), and exits `0` explicitly so the success line always flushes; interactive TTY behavior is unchanged. Regression guard: `tests/unit/reset-password-cli-6261-6258.test.ts` (3). (thanks @chirag127)
+
 - **fix(db):** the mass-migration **safety abort** now tells the operator how to bypass it and stops flooding the log ([#6260](https://github.com/diegosouzapw/OmniRoute/issues/6260)) — after restoring a backup that wiped the migration tracking table, `runMigrations()` threw the abort on every downstream `ensureDbInitialized()`, re-logging the full banner 11+ times, and the message never mentioned the existing `OMNIROUTE_MAX_PENDING_MIGRATIONS` escape hatch. The abort text now appends a bypass hint (set `OMNIROUTE_MAX_PENDING_MIGRATIONS=0` in `server.env` / `DATA_DIR/.env`), and a new `MigrationSafetyAbortError` is memoized so repeated calls in the same process throw the same instance and emit a single concise line instead of the full cascade. Regression guard: `tests/unit/migration-safety-abort-6260.test.ts`. (thanks @chirag127)
 
 - **fix(auth):** importing a **distinct** Codex/ChatGPT OAuth `auth.json` is no longer falsely rejected as "already exists" when it belongs to a different user in the same workspace ([#6301](https://github.com/diegosouzapw/OmniRoute/issues/6301)). `findExistingCodexConnection` (in `src/lib/oauth/utils/codexAuthImport.ts`) deduped **only** on `providerSpecificData.workspaceId === accountId`, where `accountId` is the shared `chatgpt_account_id`/`tokens.account_id` — so two members of the same ChatGPT Team collapsed onto a single connection (409 `duplicate_account`). The id_token's `https://api.openai.com/auth` claim carries a per-user `chatgpt_user_id` alongside the workspace id (the device-flow path already persisted it as `chatgptUserId`, but the import path did not). Now `parseAndValidateCodexAuth` extracts `userId` (`chatgpt_user_id` → `user_id` → JWT `sub`) into `ParsedCodexAuth`, the create/update paths persist `chatgptUserId` in `providerSpecificData` (mirroring `codex.ts`), and dedup keys on `workspaceId` **AND** `chatgptUserId` — with a backward-compat fallback to legacy accountId-only matching when no stored connection for that workspace records a `chatgptUserId`, so genuinely-same accounts still dedup. Regression guard: `tests/unit/codex-auth-import-userid-dedup-6301.test.ts` (4). (thanks @anungma)

bin/omniroute.mjs

@@ -6,6 +6,7 @@
  * Special bypasses (handled before Commander):
  *   --mcp                     Start MCP server over stdio
  *   reset-encrypted-columns   Recovery tool for broken encrypted credentials
+ *   reset-password            Reset the admin/management password
  *
  * All other commands are routed through Commander (bin/cli/program.mjs).
  */
@@ -210,6 +211,15 @@ if (process.argv.includes("reset-encrypted-columns")) {
   process.exit(exitCode ?? 0);
 }
 
+if (process.argv.includes("reset-password")) {
+  // bin/reset-password.mjs self-executes its `main()` on import and calls
+  // process.exit() on completion/error. Await a never-resolving promise so
+  // control never falls through to Commander (which would then reject
+  // `reset-password` as an unknown command). See #6261.
+  await import(pathToFileURL(join(ROOT, "bin", "reset-password.mjs")).href);
+  await new Promise(() => {});
+}
+
 try {
   const { createProgram } = await import(
     pathToFileURL(join(ROOT, "bin", "cli", "program.mjs")).href

bin/reset-password.mjs

@@ -5,10 +5,15 @@
  *
  * Usage:
  *   node bin/reset-password.mjs
- *   npx omniroute reset-password
+ *   omniroute reset-password
+ *
+ * Non-interactive / scripted usage (piped stdin, e.g. CI or Docker):
+ *   printf 'NewPass123\nNewPass123\n' | omniroute reset-password
+ *   printf 'NewPass123' | omniroute reset-password --password-stdin
  *
  * Resets the admin password for OmniRoute.
- * Prompts for a new password and updates the database directly.
+ * Prompts for a new password (interactive TTY) or reads it from stdin
+ * (non-TTY) and updates the database directly.
  *
  * @module bin/reset-password
  */
@@ -21,19 +26,61 @@ import { readManagementPasswordState, resetManagementPassword } from "./cli/sqli
 const DATA_DIR = resolveDataDir();
 const DB_PATH = resolveStoragePath(DATA_DIR);
 
-const rl = createInterface({
-  input: process.stdin,
-  output: process.stdout,
-});
+const MIN_PASSWORD_LENGTH = 8;
 
-function ask(question) {
-  return new Promise((resolve) => rl.question(question, resolve));
+/** Read the entire stdin stream as a UTF-8 string (used for non-TTY input). */
+function readAllStdin() {
+  return new Promise((resolve) => {
+    let data = "";
+    process.stdin.setEncoding("utf8");
+    process.stdin.on("data", (chunk) => {
+      data += chunk;
+    });
+    process.stdin.on("end", () => resolve(data));
+    process.stdin.on("error", () => resolve(data));
+    // Resuming is implied by attaching a 'data' listener, but be explicit so a
+    // paused stream (some spawn setups) still flows to EOF.
+    process.stdin.resume();
+  });
 }
 
-function exitWithError(message) {
-  console.error(message);
-  rl.close();
-  process.exit(1);
+/**
+ * Obtain the new password (and its confirmation).
+ *
+ * - `--password-stdin`: the ENTIRE stdin is the password, no confirmation.
+ * - non-TTY stdin (piped): read all of stdin once; first line is the password,
+ *   second line — when present — is the confirmation, else the first line is
+ *   reused (a single-line pipe means "no separate confirmation").
+ * - interactive TTY: two sequential prompts (unchanged behavior).
+ *
+ * The non-TTY path exists because two sequential `rl.question` promises never
+ * settle under a piped EOF — the second read blocks forever, so the reset was
+ * silently never applied (#6258).
+ */
+async function collectPassword() {
+  if (process.argv.includes("--password-stdin")) {
+    const raw = await readAllStdin();
+    const password = raw.replace(/[\r\n]+$/, "");
+    return { password, confirm: password };
+  }
+
+  if (!process.stdin.isTTY) {
+    const raw = await readAllStdin();
+    const lines = raw.split(/\r?\n/);
+    const password = lines[0] ?? "";
+    const confirm = lines[1] ? lines[1] : password;
+    return { password, confirm };
+  }
+
+  const rl = createInterface({ input: process.stdin, output: process.stdout });
+  try {
+    const ask = (question) => new Promise((resolve) => rl.question(question, resolve));
+    const password = await ask("Enter new password (min 8 chars): ");
+    const confirm = await ask("Confirm new password: ");
+    return { password, confirm };
+  } finally {
+    rl.close();
+  }
 }
 
 console.log("\n🔑 OmniRoute — Password Reset\n");
@@ -54,27 +101,34 @@ async function main() {
     console.log("ℹ️  No password is currently set.");
   }
 
-  const password = await ask("Enter new password (min 8 chars): ");
+  const { password, confirm } = await collectPassword();
 
-  if (!password || password.length < 8) {
-    exitWithError("\n❌ Password must be at least 8 characters.\n");
+  if (!password || password.length < MIN_PASSWORD_LENGTH) {
+    console.error(`\n❌ Password must be at least ${MIN_PASSWORD_LENGTH} characters.\n`);
+    process.exit(1);
   }
 
-  const confirm = await ask("Confirm new password: ");
-
   if (password !== confirm) {
-    exitWithError("\n❌ Passwords do not match.\n");
+    console.error("\n❌ Passwords do not match.\n");
+    process.exit(1);
   }
 
   await resetManagementPassword(password, DB_PATH);
-  rl.close();
 
   console.log("\n✅ Password reset successfully!");
   console.log("   Restart OmniRoute for changes to take effect.\n");
 }
 
-main().catch((err) => {
-  console.error(`\n❌ Error: ${err.message}\n`);
-  rl.close();
-  process.exit(1);
-});
+main()
+  .then(() => {
+    // Explicit exit(0) so a caller that imports this module (bin/omniroute.mjs
+    // routes `omniroute reset-password` here) terminates cleanly instead of
+    // hanging / exiting with code 13 on an unsettled wrapper await. On POSIX,
+    // console.log to a pipe is synchronous, so the success line is already
+    // flushed by the time we exit.
+    process.exit(0);
+  })
+  .catch((err) => {
+    console.error(`\n❌ Error: ${err.message}\n`);
+    process.exit(1);
+  });

docs/reference/CLI-TOOLS.md

@@ -648,7 +648,7 @@ omniroute providers validate                        # Local-only structural vali
 ### Recovery & Reset
 
 ```bash
-omniroute reset-password                # Reset the admin password (legacy alias still works)
+omniroute reset-password                # Reset the admin password (also: omniroute-reset-password)
 omniroute reset-encrypted-columns       # Show warning + dry-run for encrypted credential reset
 omniroute reset-encrypted-columns --force  # Actually null out encrypted credentials in SQLite
 ```

tests/unit/reset-password-cli-6261-6258.test.ts

@@ -0,0 +1,168 @@
+import test from "node:test";
+import assert from "node:assert/strict";
+import { spawnSync } from "node:child_process";
+import fs from "node:fs";
+import os from "node:os";
+import path from "node:path";
+import { fileURLToPath } from "node:url";
+import bcrypt from "bcryptjs";
+import Database from "better-sqlite3";
+
+const ROOT = path.join(path.dirname(fileURLToPath(import.meta.url)), "..", "..");
+const OMNIROUTE_BIN = path.join(ROOT, "bin", "omniroute.mjs");
+const RESET_BIN = path.join(ROOT, "bin", "reset-password.mjs");
+
+// Isolate every spawn from the development repo's .env and the machine's real
+// ~/.omniroute so DATA_DIR is the only data directory in play.
+function baseEnv(dataDir: string, isolatedHome: string): NodeJS.ProcessEnv {
+  const env = { ...process.env };
+  return {
+    ...env,
+    DATA_DIR: dataDir,
+    HOME: isolatedHome,
+    // Give the CLI a key so bin/omniroute.mjs never warns/provisions.
+    STORAGE_ENCRYPTION_KEY: "0".repeat(64),
+    CI: "1",
+    NO_UPDATE_NOTIFIER: "1",
+    OMNIROUTE_NO_UPDATE_NOTIFIER: "1",
+    OMNIROUTE_CLI_SKIP_REPO_ENV: "1",
+  };
+}
+
+// Seed a storage.sqlite that already exists with the settings schema so the
+// reset CLI passes its "database exists" precondition.
+function seedDb(dataDir: string): string {
+  fs.mkdirSync(dataDir, { recursive: true });
+  const dbPath = path.join(dataDir, "storage.sqlite");
+  const db = new Database(dbPath);
+  db.pragma("journal_mode = WAL");
+  db.prepare(
+    `CREATE TABLE IF NOT EXISTS key_value (
+      namespace TEXT NOT NULL,
+      key TEXT NOT NULL,
+      value TEXT NOT NULL,
+      PRIMARY KEY (namespace, key)
+    )`
+  ).run();
+  db.close();
+  return dbPath;
+}
+
+// Read the stored management password (JSON-encoded bcrypt hash) from the DB.
+function readStoredPassword(dbPath: string): string | null {
+  const db = new Database(dbPath, { readonly: true });
+  try {
+    const row = db
+      .prepare("SELECT value FROM key_value WHERE namespace = 'settings' AND key = 'password'")
+      .get() as { value?: string } | undefined;
+    if (!row?.value) return null;
+    try {
+      return JSON.parse(row.value);
+    } catch {
+      return row.value;
+    }
+  } finally {
+    db.close();
+  }
+}
+
+function mkHome(): string {
+  return fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-reset-home-"));
+}
+function mkDataDir(): string {
+  return fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-reset-data-"));
+}
+
+// #6261: `omniroute reset-password` must be a real subcommand (not "unknown
+// command"), routing into bin/reset-password.mjs — and #6258: under piped
+// (non-TTY) stdin it must actually apply the reset and print the success line.
+test("omniroute reset-password subcommand applies the reset over piped stdin (#6261, #6258)", async () => {
+  const dataDir = mkDataDir();
+  const home = mkHome();
+  try {
+    const dbPath = seedDb(dataDir);
+    const res = spawnSync("node", [OMNIROUTE_BIN, "reset-password"], {
+      env: baseEnv(dataDir, home),
+      input: "ChangeMe\nChangeMe\n",
+      timeout: 60_000,
+      encoding: "utf-8",
+    });
+    const out = `${res.stdout ?? ""}\n${res.stderr ?? ""}`;
+    assert.equal(res.status, 0, `expected exit 0, got ${res.status}. Output:\n${out}`);
+    assert.doesNotMatch(
+      out,
+      /unknown command|unknown option|error: unknown/i,
+      `must not be treated as an unknown command:\n${out}`
+    );
+    assert.match(out, /Password Reset/i, `must enter the reset flow:\n${out}`);
+    assert.match(out, /reset successfully/i, `must print the success line:\n${out}`);
+
+    const stored = readStoredPassword(dbPath);
+    assert.ok(stored, "a password must be persisted to the DB");
+    assert.ok(
+      await bcrypt.compare("ChangeMe", stored as string),
+      "the stored password must verify against the piped value"
+    );
+  } finally {
+    fs.rmSync(dataDir, { recursive: true, force: true });
+    fs.rmSync(home, { recursive: true, force: true });
+  }
+});
+
+// #6258: the standalone bin under piped (non-TTY) two-line stdin must not hang;
+// it reads both lines, applies the reset, and flushes the success line.
+test("omniroute-reset-password applies the reset over piped two-line stdin (#6258)", async () => {
+  const dataDir = mkDataDir();
+  const home = mkHome();
+  try {
+    const dbPath = seedDb(dataDir);
+    const res = spawnSync("node", [RESET_BIN], {
+      env: baseEnv(dataDir, home),
+      input: "ChangeMe\nChangeMe\n",
+      timeout: 60_000,
+      encoding: "utf-8",
+    });
+    const out = `${res.stdout ?? ""}\n${res.stderr ?? ""}`;
+    assert.equal(res.status, 0, `expected exit 0, got ${res.status}. Output:\n${out}`);
+    assert.match(out, /reset successfully/i, `must print the success line:\n${out}`);
+
+    const stored = readStoredPassword(dbPath);
+    assert.ok(stored, "a password must be persisted to the DB");
+    assert.ok(
+      await bcrypt.compare("ChangeMe", stored as string),
+      "the stored password must verify against the piped value"
+    );
+  } finally {
+    fs.rmSync(dataDir, { recursive: true, force: true });
+    fs.rmSync(home, { recursive: true, force: true });
+  }
+});
+
+// #6258: the --password-stdin flag reads the entire stdin as the password (no
+// confirmation prompt) — for scripted / automated resets.
+test("omniroute-reset-password --password-stdin reads the whole stdin as the password (#6258)", async () => {
+  const dataDir = mkDataDir();
+  const home = mkHome();
+  try {
+    const dbPath = seedDb(dataDir);
+    const res = spawnSync("node", [RESET_BIN, "--password-stdin"], {
+      env: baseEnv(dataDir, home),
+      input: "ChangeMe\n",
+      timeout: 60_000,
+      encoding: "utf-8",
+    });
+    const out = `${res.stdout ?? ""}\n${res.stderr ?? ""}`;
+    assert.equal(res.status, 0, `expected exit 0, got ${res.status}. Output:\n${out}`);
+    assert.match(out, /reset successfully/i, `must print the success line:\n${out}`);
+
+    const stored = readStoredPassword(dbPath);
+    assert.ok(stored, "a password must be persisted to the DB");
+    assert.ok(
+      await bcrypt.compare("ChangeMe", stored as string),
+      "the stored password must verify against the --password-stdin value"
+    );
+  } finally {
+    fs.rmSync(dataDir, { recursive: true, force: true });
+    fs.rmSync(home, { recursive: true, force: true });
+  }
+});