fix(cli): bare omniroute (default serve) must provision STORAGE_ENCRYPTION_KEY

My #3129 gate wrongly skipped provisioning for a bare `omniroute` invocation —
but `serve` is isDefault:true, so bare runs the server, which needs the key.
Only --version/--help/help/completion skip now. Realigns with #1622: its bootstrap
test invoked `--help` (now correctly skipped), so it's switched to `config list
--json` (a real, fast, offline command) to exercise the provisioning path.
This commit is contained in:
diegosouzapw 2026-06-03 21:08:01 -03:00
parent f1c42359a2
commit e1007acb7e
3 changed files with 24 additions and 11 deletions

View file

@ -5,13 +5,15 @@
* 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.
* side effect: 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).
* Returns FALSE only for: `--version`/`-V` or `--help`/`-h` anywhere in the args,
* and the `help`/`completion` subcommands.
*
* Returns TRUE for everything else, INCLUDING a bare `omniroute` (no args) the
* `serve` command is `isDefault: true`, so a bare invocation starts the server,
* which needs the encryption key. This preserves the #1622 persistence fix
* (key generated on first real run and reused across restarts).
*
* @param {string[]} argv - process.argv (node + script + args).
* @returns {boolean}
@ -21,7 +23,8 @@ 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;
// Bare `omniroute` runs the default `serve` command → must provision.
if (args.length === 0) return true;
if (args.some((a) => INFO_FLAGS.has(a))) return false;
if (INFO_COMMANDS.has(args[0])) return false;
return true;

View file

@ -23,7 +23,11 @@ function runCli(dataDir: string): { code: number | null; stderr: string } {
// the bootstrap skips writing DATA_DIR/.env (the behaviour the test exercises).
const isolatedHome = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-key-home-"));
try {
const res = spawnSync("node", [BIN, "--help"], {
// Use a real (non-informational) command so the STORAGE_ENCRYPTION_KEY
// bootstrap runs. `--version`/`--help` are intentionally skipped now (#3129),
// so the #1622 provisioning path must be exercised by an actual command.
// `config list --json` is fast and offline (no server, no network).
const res = spawnSync("node", [BIN, "config", "list", "--json"], {
cwd: dataDir,
env: {
...cleanEnv,

View file

@ -6,7 +6,6 @@ import { shouldProvisionStorageKey } from "../../bin/cli/utils/storageKeyProvisi
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);
@ -15,6 +14,11 @@ test("storage key: informational commands do NOT provision a key", () => {
assert.equal(shouldProvisionStorageKey(argv("completion")), false);
});
test("storage key: bare `omniroute` provisions (serve is the default command)", () => {
// No args → Commander runs the isDefault `serve` command, which needs the key.
assert.equal(shouldProvisionStorageKey(argv()), true);
});
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);
@ -30,7 +34,9 @@ test("storage key: real commands DO provision (preserves #1622 persistence)", ()
assert.equal(shouldProvisionStorageKey(argv("--lang", "en", "serve")), true);
});
test("storage key: defensive on non-array input", () => {
test("storage key: defensive on non-array input → fail-safe to provisioning", () => {
// Non-array argv collapses to [] → treated as a bare invocation (default serve),
// so it provisions. Fail-safe: better to have the key than to skip it.
// @ts-expect-error intentional bad input
assert.equal(shouldProvisionStorageKey(undefined), false);
assert.equal(shouldProvisionStorageKey(undefined), true);
});