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 dfe6d1a..f5ed23d 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); @@ -69,33 +70,36 @@ 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 allowPrivateNetworks = args.includes('--allow-private'); - 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] [--allow-private] [--help] [--version]'); console.error('Run with --help for full usage information.'); process.exit(1); } + const allowPrivateNetworks = args.includes('--allow-private'); try { - const report = await analyze(url, { - ...(timeoutMs !== undefined ? { timeoutMs } : {}), + const report = await analyze(parsed.url, { + ...(parsed.timeoutMs !== undefined ? { timeoutMs: parsed.timeoutMs } : {}), ...(allowPrivateNetworks ? { allowPrivateNetworks } : {}), }); - if (jsonMode) { console.log(JSON.stringify(report, null, 2)); } + 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/src/fetch.ts b/src/fetch.ts index 9aec643..1ff439f 100644 --- a/src/fetch.ts +++ b/src/fetch.ts @@ -70,7 +70,11 @@ async function assertPublicUrl(url: URL, allowPrivateNetworks: boolean | undefin } export async function fetchHeaders(url: string, options?: FetchOptions): Promise> { - const timeoutMs = options?.timeoutMs ?? 10000; + // Guards direct library callers (not just the CLI, which validates its own + // --timeout flag): a NaN/Infinity/non-positive timeoutMs would otherwise + // reach setTimeout and fire near-instantly, aborting the request immediately. + const rawTimeout = options?.timeoutMs; + const timeoutMs = Number.isFinite(rawTimeout) && (rawTimeout as number) > 0 ? (rawTimeout as number) : 10000; const controller = new AbortController(); const timer = setTimeout(() => controller.abort(), timeoutMs); try { 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); + }); +});