From 7087e8d6d20128ada24c3cee90f56f2b8985c1cc Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 11 Jul 2026 05:09:04 +0000 Subject: [PATCH] fix(cli): validate --timeout value instead of silently coercing to NaN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An invalid or missing --timeout value (e.g. `--timeout abc`, or `--timeout` misplaced before the URL) parsed to NaN and was passed straight through to fetchHeaders. setTimeout(..., NaN) fires in ~1ms in Node, so the fetch aborted almost immediately with a cryptic "This operation was aborted" instead of a clear usage error — misleading for a tool whose main use case is a CI gate. Extract argument parsing into a pure parseArgs() in src/cli-args.ts so it's unit-testable without invoking main()'s network/process.exit side effects, and reject non-positive/non-numeric --timeout values with an actionable error message. --- src/cli-args.ts | 33 +++++++++++++++++++++++++ src/cli.ts | 22 ++++++++++------- test/cli-args.test.ts | 57 +++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 103 insertions(+), 9 deletions(-) create mode 100644 src/cli-args.ts create mode 100644 test/cli-args.test.ts diff --git a/src/cli-args.ts b/src/cli-args.ts new file mode 100644 index 0000000..cf9b982 --- /dev/null +++ b/src/cli-args.ts @@ -0,0 +1,33 @@ +export interface ParsedArgs { + help: boolean; + version: boolean; + json: boolean; + timeoutMs?: number; + url?: string; + error?: string; +} + +export function parseArgs(args: string[]): ParsedArgs { + const help = args.includes('--help') || args.includes('-h'); + const version = args.includes('--version') || args.includes('-v'); + const json = args.includes('--json'); + + const timeoutIndex = args.indexOf('--timeout'); + let timeoutMs: number | undefined; + if (timeoutIndex !== -1) { + const raw = args[timeoutIndex + 1]; + const parsed = raw !== undefined ? Number(raw) : NaN; + if (!Number.isFinite(parsed) || parsed <= 0) { + const got = raw === undefined ? '' : JSON.stringify(raw); + return { help, version, json, error: `--timeout requires a positive number of milliseconds (got ${got})` }; + } + timeoutMs = parsed; + } + + // Exclude the value consumed by --timeout by position, not by string equality + // against the parsed number — matching by value broke whenever a URL happened + // to coincide with the (possibly NaN-stringified) timeout argument. + const url = args.find((a, i) => !a.startsWith('--') && !(timeoutIndex !== -1 && i === timeoutIndex + 1)); + + return { help, version, json, timeoutMs, url }; +} diff --git a/src/cli.ts b/src/cli.ts index a6d2e8b..23c6edc 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -1,6 +1,7 @@ #!/usr/bin/env node import { createRequire } from 'node:module'; import { analyze } from './index.js'; +import { parseArgs } from './cli-args.js'; import type { SecurityHeaderReport } from './types.js'; const require = createRequire(import.meta.url); @@ -68,29 +69,32 @@ function printReport(r: SecurityHeaderReport) { async function main() { const args = process.argv.slice(2); + const parsed = parseArgs(args); - if (args.includes('--help') || args.includes('-h')) { + if (parsed.help) { printHelp(); process.exit(0); } - if (args.includes('--version') || args.includes('-v')) { + if (parsed.version) { console.log(getVersion()); process.exit(0); } - const jsonMode = args.includes('--json'); - const timeoutArg = args.find((a, i) => a === '--timeout' && args[i + 1]); - const timeoutMs = timeoutArg ? parseInt(args[args.indexOf('--timeout') + 1], 10) : undefined; - const url = args.find(a => !a.startsWith('--') && a !== String(timeoutMs)); - if (!url) { + if (parsed.error) { + console.error(`Error: ${parsed.error}`); + console.error('Run with --help for full usage information.'); + process.exit(1); + } + + if (!parsed.url) { console.error('Usage: security-headers [--json] [--timeout ms] [--help] [--version]'); console.error('Run with --help for full usage information.'); process.exit(1); } try { - const report = await analyze(url, timeoutMs !== undefined ? { timeoutMs } : undefined); - if (jsonMode) { console.log(JSON.stringify(report, null, 2)); } + const report = await analyze(parsed.url, parsed.timeoutMs !== undefined ? { timeoutMs: parsed.timeoutMs } : undefined); + if (parsed.json) { console.log(JSON.stringify(report, null, 2)); } else { printReport(report); } if (report.grade === 'D' || report.grade === 'F') process.exit(1); } catch (err) { diff --git a/test/cli-args.test.ts b/test/cli-args.test.ts new file mode 100644 index 0000000..4e9e9fd --- /dev/null +++ b/test/cli-args.test.ts @@ -0,0 +1,57 @@ +import { describe, it, expect } from 'vitest'; +import { parseArgs } from '../src/cli-args.js'; + +describe('parseArgs', () => { + it('parses a plain URL with no flags', () => { + const r = parseArgs(['https://example.com']); + expect(r.url).toBe('https://example.com'); + expect(r.timeoutMs).toBeUndefined(); + expect(r.error).toBeUndefined(); + }); + + it('parses --json and --timeout alongside the URL', () => { + const r = parseArgs(['https://example.com', '--json', '--timeout', '3000']); + expect(r.url).toBe('https://example.com'); + expect(r.json).toBe(true); + expect(r.timeoutMs).toBe(3000); + }); + + it('parses the URL when it precedes --timeout', () => { + const r = parseArgs(['https://example.com', '--timeout', '3000']); + expect(r.url).toBe('https://example.com'); + expect(r.timeoutMs).toBe(3000); + }); + + it('rejects a non-numeric --timeout value instead of silently using NaN', () => { + const r = parseArgs(['https://example.com', '--timeout', 'abc']); + expect(r.error).toMatch(/--timeout requires a positive number/); + expect(r.timeoutMs).toBeUndefined(); + }); + + it('rejects --timeout with no value (e.g. it precedes the URL by mistake)', () => { + const r = parseArgs(['--timeout', 'https://example.com']); + expect(r.error).toMatch(/--timeout requires a positive number/); + }); + + it('rejects a zero or negative --timeout', () => { + expect(parseArgs(['https://example.com', '--timeout', '0']).error).toBeDefined(); + expect(parseArgs(['https://example.com', '--timeout', '-500']).error).toBeDefined(); + }); + + it('rejects --timeout as the trailing argument with nothing after it', () => { + const r = parseArgs(['https://example.com', '--timeout']); + expect(r.error).toMatch(//); + }); + + it('leaves url undefined when only flags are given', () => { + const r = parseArgs(['--json']); + expect(r.url).toBeUndefined(); + }); + + it('recognizes --help and --version', () => { + expect(parseArgs(['--help']).help).toBe(true); + expect(parseArgs(['-h']).help).toBe(true); + expect(parseArgs(['--version']).version).toBe(true); + expect(parseArgs(['-v']).version).toBe(true); + }); +});