fix(cli): don't write STORAGE_ENCRYPTION_KEY to .env on informational commands (#3129)

Running any CLI command — even `omniroute --version` or `--help` — generated a
32-byte STORAGE_ENCRYPTION_KEY and created `~/.omniroute/.env` (or DATA_DIR/.env).
A read-only command should never mutate the data dir. Gate the provisioning
behind shouldProvisionStorageKey(): skip for --version/--help/help/completion and
bare invocations; still provision for real commands (serve, keys, …) so the
encryption key persists before storage is accessed (#1622).
This commit is contained in:
Diego Rodrigues de Sa e Souza
2026-06-03 18:53:22 -03:00
committed by GitHub
parent a718558d68
commit 80c9ca7096
3 changed files with 71 additions and 1 deletions

View File

@@ -0,0 +1,28 @@
/**
* Decide whether a CLI invocation should provision (generate + persist) the
* STORAGE_ENCRYPTION_KEY into DATA_DIR/.env.
*
* Purely informational invocations must NOT create `~/.omniroute/.env` or write
* a key — they never touch encrypted storage. Generating a 32-byte key and a
* `.env` file just to print `omniroute --version` (or `--help`) is a surprising
* side effect: running a read-only command should not mutate the data dir.
*
* Returns false for: no command at all (bare `omniroute` → Commander prints
* help), `--version`/`-V` or `--help`/`-h` anywhere in the args, and the
* `help`/`completion` subcommands. Returns true for every real command
* (`serve`, `keys`, `providers`, …) so the encryption key is still provisioned
* before encrypted storage is accessed (preserves the #1622 persistence fix).
*
* @param {string[]} argv - process.argv (node + script + args).
* @returns {boolean}
*/
const INFO_FLAGS = new Set(["-h", "--help", "-V", "--version"]);
const INFO_COMMANDS = new Set(["help", "completion"]);
export function shouldProvisionStorageKey(argv) {
const args = Array.isArray(argv) ? argv.slice(2) : [];
if (args.length === 0) return false;
if (args.some((a) => INFO_FLAGS.has(a))) return false;
if (INFO_COMMANDS.has(args[0])) return false;
return true;
}

View File

@@ -17,6 +17,7 @@ import { homedir, platform } from "node:os";
import updateNotifier from "update-notifier";
import { isNativeBinaryCompatible } from "../scripts/build/native-binary-compat.mjs";
import { getNodeRuntimeSupport, getNodeRuntimeWarning } from "./nodeRuntimeSupport.mjs";
import { shouldProvisionStorageKey } from "./cli/utils/storageKeyProvision.mjs";
// Register tsx so dynamic imports of .ts source files (referenced as .js per
// TypeScript conventions) resolve correctly. The build never emits .js for
@@ -92,7 +93,12 @@ loadEnvFile();
// Generate STORAGE_ENCRYPTION_KEY if not set (persisted to ~/.omniroute/.env)
// This ensures the key survives across upgrades and is not regenerated on each install.
// See: https://github.com/diegosouzapw/OmniRoute/issues/1622
{
//
// Only provision for commands that actually touch encrypted storage. Purely
// informational invocations (`--version`, `--help`, `help`) must not create a
// key or write ~/.omniroute/.env — running a read-only command should never
// mutate the data dir.
if (shouldProvisionStorageKey(process.argv)) {
const { randomBytes } = await import("node:crypto");
const { existsSync, mkdirSync, readFileSync, writeFileSync } = await import("node:fs");
const { join } = await import("node:path");

View File

@@ -0,0 +1,36 @@
import test from "node:test";
import assert from "node:assert/strict";
import { shouldProvisionStorageKey } from "../../bin/cli/utils/storageKeyProvision.mjs";
// argv shape is [node, script, ...args]
const argv = (...args: string[]) => ["node", "omniroute", ...args];
test("storage key: informational commands do NOT provision a key", () => {
assert.equal(shouldProvisionStorageKey(argv()), false); // bare omniroute → help
assert.equal(shouldProvisionStorageKey(argv("--version")), false);
assert.equal(shouldProvisionStorageKey(argv("-V")), false);
assert.equal(shouldProvisionStorageKey(argv("--help")), false);
assert.equal(shouldProvisionStorageKey(argv("-h")), false);
assert.equal(shouldProvisionStorageKey(argv("help")), false);
assert.equal(shouldProvisionStorageKey(argv("completion")), false);
});
test("storage key: --help/--version anywhere in the args still skips", () => {
assert.equal(shouldProvisionStorageKey(argv("serve", "--help")), false);
assert.equal(shouldProvisionStorageKey(argv("keys", "list", "-h")), false);
assert.equal(shouldProvisionStorageKey(argv("--lang", "en", "--version")), false);
});
test("storage key: real commands DO provision (preserves #1622 persistence)", () => {
assert.equal(shouldProvisionStorageKey(argv("serve")), true);
assert.equal(shouldProvisionStorageKey(argv("keys", "list")), true);
assert.equal(shouldProvisionStorageKey(argv("providers")), true);
assert.equal(shouldProvisionStorageKey(argv("serve", "--port", "20128")), true);
// global --lang option before a real command must not suppress provisioning
assert.equal(shouldProvisionStorageKey(argv("--lang", "en", "serve")), true);
});
test("storage key: defensive on non-array input", () => {
// @ts-expect-error intentional bad input
assert.equal(shouldProvisionStorageKey(undefined), false);
});