fix(cli): validate --timeout value instead of silently coercing to NaN

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 ? '<nothing>' : 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 };
+}

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 <url> [--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) {

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(/<nothing>/);
+  });
+
+  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);
+  });
+});