From e1244c1f0ac888e609fcb5a4f9430676c68c0226 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 12 Jun 2026 03:04:09 +0000 Subject: [PATCH] fix(cli): correct --format parsing so default text output works The CLI crashed on the documented default invocation (sbom-diff old.json new.json) with 'Unsupported format: old.json'. When no --format flag was present, args.indexOf('--format') returned -1, so args[indexOf+1] resolved to args[0] (the first file path), which was then passed as the report format. A space-separated '--format json' placed before the positional arguments also broke, because positional filtering only checked startsWith('--') and so treated the value 'json' as a file path. Rewrite argument parsing into a single, order-independent pass that: - defaults to 'text' when no flag is given - supports both '--format=value' and '--format value' - consumes the flag value so it is never mistaken for a path - validates the format and reports a clean error (no stack trace) - rejects unknown options Extract the parser as exported parseArgs() and guard main() so it only runs when the module is invoked directly, enabling unit tests. Add cli.test.ts covering the regressions and edge cases. --- package-lock.json | 33 ++------------------ src/__tests__/cli.test.ts | 40 ++++++++++++++++++++++++ src/cli.ts | 66 +++++++++++++++++++++++++++++++++------ 3 files changed, 99 insertions(+), 40 deletions(-) create mode 100644 src/__tests__/cli.test.ts diff --git a/package-lock.json b/package-lock.json index 0934876..72185ed 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,6 +19,9 @@ "typescript": "^5.9.3", "typescript-eslint": "^8.59.3", "vitest": "^4.1.6" + }, + "engines": { + "node": ">=18.0.0" } }, "node_modules/@babel/helper-string-parser": { @@ -492,9 +495,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -512,9 +512,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -532,9 +529,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -552,9 +546,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -572,9 +563,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -592,9 +580,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2187,9 +2172,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2211,9 +2193,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2235,9 +2214,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -2259,9 +2235,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/src/__tests__/cli.test.ts b/src/__tests__/cli.test.ts new file mode 100644 index 0000000..3532b91 --- /dev/null +++ b/src/__tests__/cli.test.ts @@ -0,0 +1,40 @@ +import { describe, it, expect } from 'vitest'; +import { parseArgs } from '../cli.js'; + +describe('parseArgs', () => { + it('defaults to text format when no flag is given', () => { + const { positional, format } = parseArgs(['old.json', 'new.json']); + expect(positional).toEqual(['old.json', 'new.json']); + expect(format).toBe('text'); + }); + + it('parses --format=json', () => { + const { positional, format } = parseArgs(['old.json', 'new.json', '--format=json']); + expect(positional).toEqual(['old.json', 'new.json']); + expect(format).toBe('json'); + }); + + it('parses space-separated --format markdown', () => { + const { positional, format } = parseArgs(['old.json', 'new.json', '--format', 'markdown']); + expect(positional).toEqual(['old.json', 'new.json']); + expect(format).toBe('markdown'); + }); + + it('handles the flag appearing before positional paths', () => { + const { positional, format } = parseArgs(['--format', 'json', 'old.json', 'new.json']); + expect(positional).toEqual(['old.json', 'new.json']); + expect(format).toBe('json'); + }); + + it('throws on an unsupported format value', () => { + expect(() => parseArgs(['old.json', 'new.json', '--format=yaml'])).toThrow(/Invalid --format/); + }); + + it('throws when --format is given without a value', () => { + expect(() => parseArgs(['old.json', 'new.json', '--format'])).toThrow(/Invalid --format/); + }); + + it('throws on an unknown option', () => { + expect(() => parseArgs(['old.json', 'new.json', '--bogus'])).toThrow(/Unknown option/); + }); +}); diff --git a/src/cli.ts b/src/cli.ts index 986eb1a..969f741 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -7,24 +7,66 @@ */ import { readFile } from 'node:fs/promises'; +import { pathToFileURL } from 'node:url'; import { parse } from './parser.js'; import { diff } from './diff.js'; import { renderReport } from './reporter.js'; import type { ReportFormat } from './types.js'; +const USAGE = 'Usage: sbom-diff [--format text|json|markdown]'; +const VALID_FORMATS: ReportFormat[] = ['text', 'json', 'markdown']; + +export interface ParsedArgs { + positional: string[]; + format: ReportFormat; +} + +/** + * Parse CLI arguments into positional paths and the requested output format. + * + * Supports `--format text`, `--format=text`, and flags appearing in any + * position relative to the positional file paths. Defaults to `text`. + * + * @throws if an unknown flag or unsupported format value is supplied. + */ +export function parseArgs(argv: string[]): ParsedArgs { + const positional: string[] = []; + let format: ReportFormat = 'text'; + + for (let i = 0; i < argv.length; i++) { + const arg = argv[i]; + if (arg === '--format') { + format = assertFormat(argv[++i]); + } else if (arg.startsWith('--format=')) { + format = assertFormat(arg.slice('--format='.length)); + } else if (arg.startsWith('-')) { + throw new Error(`Unknown option: ${arg}\n${USAGE}`); + } else { + positional.push(arg); + } + } + + return { positional, format }; +} + +function assertFormat(value: string | undefined): ReportFormat { + if (value !== undefined && (VALID_FORMATS as string[]).includes(value)) { + return value as ReportFormat; + } + throw new Error( + `Invalid --format value: ${value ?? '(none)'}. Expected one of: ${VALID_FORMATS.join(', ')}`, + ); +} + async function main(): Promise { - const args = process.argv.slice(2); + const { positional, format } = parseArgs(process.argv.slice(2)); - const positional = args.filter(a => !a.startsWith('--')); if (positional.length < 2) { - console.error('Usage: sbom-diff [--format text|json|markdown]'); + console.error(USAGE); process.exit(1); } const [oldPath, newPath] = positional; - const formatArg = args.find(a => a.startsWith('--format='))?.split('=')[1] - ?? args[args.indexOf('--format') + 1]; - const format: ReportFormat = (formatArg as ReportFormat) ?? 'text'; const [oldRaw, newRaw] = await Promise.all([ readFile(oldPath, 'utf-8'), @@ -38,7 +80,11 @@ async function main(): Promise { console.log(renderReport(report, format)); } -main().catch(err => { - console.error(err); - process.exit(1); -}); +// Only run when invoked directly (not when imported by tests). +const invokedPath = process.argv[1]; +if (invokedPath && import.meta.url === pathToFileURL(invokedPath).href) { + main().catch(err => { + console.error(err instanceof Error ? err.message : err); + process.exit(1); + }); +}