From 3fb51316eda949b5c333d5ff270ba0e82a125569 Mon Sep 17 00:00:00 2001 From: Alex Powell Date: Thu, 22 Oct 2026 13:46:33 +1030 Subject: [PATCH] fix(cli): reject a value flag whose value is missing or another flag --offline-db, --cache-dir, --osv-url, --audit-log and --rule read the next argv token unconditionally, so "--audit-log --json" wrote the audit log to a file named --json and dropped JSON output. Require a non-empty value that does not start with - for the spaced and = forms. Fixes #1381 --- src/cli/args.ts | 31 ++++++++----- tests/cli/value-flag-args.test.ts | 74 +++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 12 deletions(-) create mode 100644 tests/cli/value-flag-args.test.ts diff --git a/src/cli/args.ts b/src/cli/args.ts index 07c50aa0..baeff795 100644 --- a/src/cli/args.ts +++ b/src/cli/args.ts @@ -58,6 +58,13 @@ export const SBOM_FORMAT_ALIASES: Record = { "spdx2.3": "spdx", }; +function requireFlagValue(flag: string, value: string | undefined, noun: string): string { + if (value === undefined || value === "" || value.startsWith("-")) { + throw new Error(`${flag} requires a ${noun} argument`); + } + return value; +} + function parseSbomFormat(value: string | undefined): SbomFormat { const valid = Object.keys(SBOM_FORMAT_ALIASES).join(", "); if (value === undefined || value.startsWith("-")) { @@ -229,19 +236,19 @@ export function parseArgs(argv: string[]): { continue; } if (arg === FLAG_AUDIT_LOG) { - options.auditLog = argv[++i]; + options.auditLog = requireFlagValue(FLAG_AUDIT_LOG, argv[++i], "path"); continue; } if (arg.startsWith(`${FLAG_AUDIT_LOG}=`)) { - options.auditLog = arg.slice(FLAG_AUDIT_LOG.length + 1); + options.auditLog = requireFlagValue(FLAG_AUDIT_LOG, arg.slice(FLAG_AUDIT_LOG.length + 1), "path"); continue; } if (arg === FLAG_RULE) { - options.rule = argv[++i]; + options.rule = requireFlagValue(FLAG_RULE, argv[++i], "rule id"); continue; } if (arg.startsWith(`${FLAG_RULE}=`)) { - options.rule = arg.slice(FLAG_RULE.length + 1); + options.rule = requireFlagValue(FLAG_RULE, arg.slice(FLAG_RULE.length + 1), "rule id"); continue; } if (arg === FLAG_FAIL_ON) { @@ -324,11 +331,11 @@ export function parseArgs(argv: string[]): { continue; } if (arg === FLAG_OFFLINE_DB) { - options.offlineDb = argv[++i]; + options.offlineDb = requireFlagValue(FLAG_OFFLINE_DB, argv[++i], "path"); continue; } if (arg.startsWith(`${FLAG_OFFLINE_DB}=`)) { - options.offlineDb = arg.slice(FLAG_OFFLINE_DB.length + 1); + options.offlineDb = requireFlagValue(FLAG_OFFLINE_DB, arg.slice(FLAG_OFFLINE_DB.length + 1), "path"); continue; } if (arg === FLAG_ALL) { @@ -352,19 +359,19 @@ export function parseArgs(argv: string[]): { continue; } if (arg === FLAG_CACHE_DIR) { - options.cacheDir = argv[++i]; + options.cacheDir = requireFlagValue(FLAG_CACHE_DIR, argv[++i], "path"); continue; } if (arg.startsWith(`${FLAG_CACHE_DIR}=`)) { - options.cacheDir = arg.slice(FLAG_CACHE_DIR.length + 1); + options.cacheDir = requireFlagValue(FLAG_CACHE_DIR, arg.slice(FLAG_CACHE_DIR.length + 1), "path"); continue; } if (arg === FLAG_OSV_URL) { - options.osvUrl = argv[++i]; + options.osvUrl = requireFlagValue(FLAG_OSV_URL, argv[++i], "url"); continue; } if (arg.startsWith(`${FLAG_OSV_URL}=`)) { - options.osvUrl = arg.slice(FLAG_OSV_URL.length + 1); + options.osvUrl = requireFlagValue(FLAG_OSV_URL, arg.slice(FLAG_OSV_URL.length + 1), "url"); continue; } if (arg === FLAG_ALLOW_PRIVATE_OSV_URL) { @@ -469,11 +476,11 @@ export function parseArgs(argv: string[]): { continue; } if (arg === FLAG_AUDIT_LOG) { - options.auditLog = argv[++i]; + options.auditLog = requireFlagValue(FLAG_AUDIT_LOG, argv[++i], "path"); continue; } if (arg.startsWith(`${FLAG_AUDIT_LOG}=`)) { - options.auditLog = arg.slice(FLAG_AUDIT_LOG.length + 1); + options.auditLog = requireFlagValue(FLAG_AUDIT_LOG, arg.slice(FLAG_AUDIT_LOG.length + 1), "path"); continue; } if (arg === FLAG_CHECK_OVERRIDES) { diff --git a/tests/cli/value-flag-args.test.ts b/tests/cli/value-flag-args.test.ts new file mode 100644 index 00000000..e0168bf7 --- /dev/null +++ b/tests/cli/value-flag-args.test.ts @@ -0,0 +1,74 @@ +import { parseArgs } from "../../src/cli/args.js"; + +const SCAN_VALUE_FLAGS: Array<{ + flag: string; + noun: string; + option: "offlineDb" | "cacheDir" | "osvUrl" | "auditLog"; + value: string; +}> = [ + { flag: "--offline-db", noun: "path", option: "offlineDb", value: "/tmp/advisories.db" }, + { flag: "--cache-dir", noun: "path", option: "cacheDir", value: "/tmp/cache" }, + { flag: "--osv-url", noun: "url", option: "osvUrl", value: "https://example.com/osv" }, + { flag: "--audit-log", noun: "path", option: "auditLog", value: "/tmp/audit.ndjson" }, +]; + +const OVERRIDES_VALUE_FLAGS: Array<{ + flag: string; + noun: string; + option: "auditLog" | "rule"; + value: string; +}> = [ + { flag: "--audit-log", noun: "path", option: "auditLog", value: "/tmp/audit.ndjson" }, + { flag: "--rule", noun: "rule id", option: "rule", value: "OA001" }, +]; + +describe("parseArgs - required value flags (scan)", () => { + it.each(SCAN_VALUE_FLAGS)( + "rejects $flag when the next token is another flag", + ({ flag, noun }) => { + expect(() => parseArgs([".", flag, "--json"])).toThrow(`${flag} requires a ${noun} argument`); + }, + ); + + it.each(SCAN_VALUE_FLAGS)("rejects $flag when it is missing at the end", ({ flag, noun }) => { + expect(() => parseArgs([".", flag])).toThrow(`${flag} requires a ${noun} argument`); + }); + + it.each(SCAN_VALUE_FLAGS)("rejects $flag with an empty = value", ({ flag, noun }) => { + expect(() => parseArgs([".", `${flag}=`])).toThrow(`${flag} requires a ${noun} argument`); + }); + + it.each(SCAN_VALUE_FLAGS)("parses $flag with a value in both forms", ({ flag, option, value }) => { + expect(parseArgs([".", flag, value]).options[option]).toBe(value); + expect(parseArgs([".", `${flag}=${value}`]).options[option]).toBe(value); + }); +}); + +describe("parseArgs - required value flags (overrides)", () => { + it.each(OVERRIDES_VALUE_FLAGS)( + "rejects $flag when the next token is another flag", + ({ flag, noun }) => { + expect(() => parseArgs(["overrides", ".", flag, "--json"])).toThrow( + `${flag} requires a ${noun} argument`, + ); + }, + ); + + it.each(OVERRIDES_VALUE_FLAGS)("rejects $flag when it is missing at the end", ({ flag, noun }) => { + expect(() => parseArgs(["overrides", ".", flag])).toThrow(`${flag} requires a ${noun} argument`); + }); + + it.each(OVERRIDES_VALUE_FLAGS)("rejects $flag with an empty = value", ({ flag, noun }) => { + expect(() => parseArgs(["overrides", ".", `${flag}=`])).toThrow( + `${flag} requires a ${noun} argument`, + ); + }); + + it.each(OVERRIDES_VALUE_FLAGS)( + "parses $flag with a value in both forms", + ({ flag, option, value }) => { + expect(parseArgs(["overrides", ".", flag, value]).options[option]).toBe(value); + expect(parseArgs(["overrides", ".", `${flag}=${value}`]).options[option]).toBe(value); + }, + ); +});