From 3bfd525cc449dbef3d3c572876692f76f12e45b7 Mon Sep 17 00:00:00 2001 From: Mads Lorentzen Date: Wed, 19 Aug 2026 20:44:26 +0200 Subject: [PATCH] fix(portals)!: reject unknown flags in all six CLIs Silently discarded flags produced silently wrong results: jobdanmark with --query (its real flag is --text) returned all 13,862 jobs as if they matched, exit 0, empty stderr - indistinguishable from a real result set. The four bunli CLIs get an argv preflight built from each command's own options object; linkedin and freehire validate parsed flags against per-command known sets. help/version still pass, and add-portal.md's existing bogus-flag-exits-1 contract now holds for the reference implementations contributors copy. One linkedin pin updated: "--jobage-minutes -5" now fails as UNKNOWN_FLAG (the stray -5 token) rather than BAD_ARG - same loud-failure invariant, earlier gate. Review finding F13 (2026-08-19), decision approved by Mads. Co-Authored-By: Claude Opus 5 (1M context) --- .agents/skills/freehire-search/cli/src/cli.ts | 30 +++++++++++++++ .../cli/tests/cli-flag-validation.test.ts | 17 +++++++++ .agents/skills/jobbank-search/cli/src/cli.ts | 35 ++++++++++++++++- .../cli/tests/cli-flag-validation.test.ts | 23 +++++++++++ .../skills/jobdanmark-search/cli/src/cli.ts | 38 ++++++++++++++++--- .../cli/tests/cli-flag-validation.test.ts | 23 +++++++++++ .agents/skills/jobindex-search/cli/src/cli.ts | 35 ++++++++++++++++- .../cli/tests/cli-flag-validation.test.ts | 17 +++++++++ .agents/skills/jobnet-search/cli/src/cli.ts | 37 ++++++++++++++++-- .../cli/tests/cli-flag-validation.test.ts | 23 +++++++++++ .agents/skills/linkedin-search/cli/src/cli.ts | 29 ++++++++++++++ .../cli/tests/cli-flag-validation.test.ts | 26 +++++++++++-- CHANGELOG.md | 8 ++++ 13 files changed, 324 insertions(+), 17 deletions(-) diff --git a/.agents/skills/freehire-search/cli/src/cli.ts b/.agents/skills/freehire-search/cli/src/cli.ts index 4bb8baf..1d1c091 100644 --- a/.agents/skills/freehire-search/cli/src/cli.ts +++ b/.agents/skills/freehire-search/cli/src/cli.ts @@ -118,6 +118,17 @@ function parseIntFlag(name: string, raw: string | boolean | string[]): number | return val } +// Long-form flag names each command accepts (parseFlags resolves the short +// aliases q/n to these before validation). "help"/"h" pass so `search --help` +// still prints usage. +const KNOWN_FLAGS: Record> = { + search: new Set([ + "query", "category", "city", "company", "country", "facet", "format", "jobage", "limit", + "page", "region", "remote", "seniority", "skill", "description-format", "help", "h", + ]), + detail: new Set(["format", "description-format", "help", "h"]), +} + async function main(): Promise { const argv = process.argv.slice(2) const flags = parseFlags(argv) @@ -128,6 +139,25 @@ async function main(): Promise { return cmd ? 0 : 1 } + // Reject unknown flags instead of silently discarding them: a discarded + // filter changes what the search returns with no error (a wrong flag name + // once returned an entire portal's database as if it matched the query). + // add-portal.md's contract requires a bogus flag to exit 1 with a JSON + // error on stderr. + const knownFlags = KNOWN_FLAGS[cmd] + if (knownFlags) { + for (const key of Object.keys(flags)) { + if (key === "_" || knownFlags.has(key)) continue + process.stderr.write( + JSON.stringify({ + error: `unknown flag --${key} for '${cmd}' - flags are never silently ignored, because a discarded filter changes what the search returns; see --help for the supported flags`, + code: "UNKNOWN_FLAG", + }) + "\n", + ) + return 1 + } + } + if (cmd === "search") { const fmt = (flags.format as string) || "json" diff --git a/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts index 4842916..f29637b 100644 --- a/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts +++ b/.agents/skills/freehire-search/cli/tests/cli-flag-validation.test.ts @@ -77,3 +77,20 @@ describe("freehire CLI flag validation", () => { }); }); }); + + +describe("unknown flag rejection", () => { + // add-portal.md's contract: "a bogus flag or missing required arg exits 1 + // with a JSON error on stderr". A silently discarded flag is worse than an + // error: on jobdanmark a wrong flag name returned the entire database + // (13,862 results) as if it matched the query (review finding F13, + // 2026-08-19). Rejection happens before dispatch, so these are network-free. + test("a bogus --flag exits 1 with a JSON error instead of being silently discarded", async () => { + const result = await runCLI(["search", "--query", "test", "--bogus-flag", "xyz"]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + const error = JSON.parse(result.stderr); + expect(error.code).toBe("UNKNOWN_FLAG"); + expect(error.error).toContain("--bogus-flag"); + }); +}); diff --git a/.agents/skills/jobbank-search/cli/src/cli.ts b/.agents/skills/jobbank-search/cli/src/cli.ts index 936b0f2..da7ab5d 100644 --- a/.agents/skills/jobbank-search/cli/src/cli.ts +++ b/.agents/skills/jobbank-search/cli/src/cli.ts @@ -1,4 +1,5 @@ import { createCLI } from "@bunli/core" +import { writeError } from "./helpers.js" import { search } from "./commands/search.js" import { detail } from "./commands/detail.js" @@ -8,7 +9,37 @@ const cli = await createCLI({ description: "CLI for Akademikernes Jobbank (jobbank.dk) — job search for highly educated candidates", }) -cli.command(search) -cli.command(detail) +const commands = [search, detail] +for (const command of commands) { + cli.command(command) +} + +// Reject unknown --flags before dispatch. bunli silently discards them, and a +// silently discarded filter changes what the search returns without any error +// (a wrong flag name once returned an entire portal's database as if it +// matched the query). add-portal.md's contract requires a bogus flag to exit 1 +// with a JSON error on stderr; this enforces it for the reference CLIs too. +const argv = process.argv.slice(2) +const invoked = commands.find((c) => (c as { name?: string }).name === argv[0]) +if (invoked) { + const known = new Set([ + ...Object.keys((invoked as { options?: Record }).options ?? {}), + "help", + "version", + ]) + for (const token of argv.slice(1)) { + if (token === "--") break + if (token.startsWith("--")) { + const flag = token.slice(2).split("=")[0] + if (!known.has(flag)) { + writeError( + `unknown flag --${flag} for '${argv[0]}' - flags are never silently ignored, because a discarded filter changes what the search returns; see --help for the supported flags`, + "UNKNOWN_FLAG", + ) + process.exit(1) + } + } + } +} await cli.run() diff --git a/.agents/skills/jobbank-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/jobbank-search/cli/tests/cli-flag-validation.test.ts index 2dadadd..2bc133f 100644 --- a/.agents/skills/jobbank-search/cli/tests/cli-flag-validation.test.ts +++ b/.agents/skills/jobbank-search/cli/tests/cli-flag-validation.test.ts @@ -57,3 +57,26 @@ describe("Jobbank CLI flag validation", () => { }); }); }); + + +describe("unknown flag rejection", () => { + // add-portal.md's contract: "a bogus flag or missing required arg exits 1 + // with a JSON error on stderr". A silently discarded flag is worse than an + // error: on jobdanmark a wrong flag name returned the entire database + // (13,862 results) as if it matched the query (review finding F13, + // 2026-08-19). Rejection happens before dispatch, so these are network-free. + test("a bogus --flag exits 1 with a JSON error instead of being silently discarded", async () => { + const result = await runCLI(["search", "--key", "test", "--bogus-flag", "xyz"]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + const error = JSON.parse(result.stderr); + expect(error.code).toBe("UNKNOWN_FLAG"); + expect(error.error).toContain("--bogus-flag"); + }); + + test("--query (another portal's free-text flag) is rejected, not treated as no filter", async () => { + const result = await runCLI(["search", "--query", "test"]); + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stderr).code).toBe("UNKNOWN_FLAG"); + }); +}); diff --git a/.agents/skills/jobdanmark-search/cli/src/cli.ts b/.agents/skills/jobdanmark-search/cli/src/cli.ts index 0111a9a..be6b536 100644 --- a/.agents/skills/jobdanmark-search/cli/src/cli.ts +++ b/.agents/skills/jobdanmark-search/cli/src/cli.ts @@ -1,4 +1,5 @@ import { createCLI } from "@bunli/core" +import { writeError } from "./helpers.js" import { search } from "./commands/search.js" import { detail } from "./commands/detail.js" import { categories } from "./commands/categories.js" @@ -11,10 +12,37 @@ const cli = await createCLI({ description: "CLI for the Jobdanmark.dk public job search API", }) -cli.command(search) -cli.command(detail) -cli.command(categories) -cli.command(autocomplete) -cli.command(locations) +const commands = [search, detail, categories, autocomplete, locations] +for (const command of commands) { + cli.command(command) +} + +// Reject unknown --flags before dispatch. bunli silently discards them, and a +// silently discarded filter changes what the search returns without any error +// (a wrong flag name once returned an entire portal's database as if it +// matched the query). add-portal.md's contract requires a bogus flag to exit 1 +// with a JSON error on stderr; this enforces it for the reference CLIs too. +const argv = process.argv.slice(2) +const invoked = commands.find((c) => (c as { name?: string }).name === argv[0]) +if (invoked) { + const known = new Set([ + ...Object.keys((invoked as { options?: Record }).options ?? {}), + "help", + "version", + ]) + for (const token of argv.slice(1)) { + if (token === "--") break + if (token.startsWith("--")) { + const flag = token.slice(2).split("=")[0] + if (!known.has(flag)) { + writeError( + `unknown flag --${flag} for '${argv[0]}' - flags are never silently ignored, because a discarded filter changes what the search returns; see --help for the supported flags`, + "UNKNOWN_FLAG", + ) + process.exit(1) + } + } + } +} await cli.run() diff --git a/.agents/skills/jobdanmark-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/jobdanmark-search/cli/tests/cli-flag-validation.test.ts index 454c44b..380d9e6 100644 --- a/.agents/skills/jobdanmark-search/cli/tests/cli-flag-validation.test.ts +++ b/.agents/skills/jobdanmark-search/cli/tests/cli-flag-validation.test.ts @@ -72,3 +72,26 @@ describe("Jobdanmark CLI flag validation", () => { }); }); }); + + +describe("unknown flag rejection", () => { + // add-portal.md's contract: "a bogus flag or missing required arg exits 1 + // with a JSON error on stderr". A silently discarded flag is worse than an + // error: on jobdanmark a wrong flag name returned the entire database + // (13,862 results) as if it matched the query (review finding F13, + // 2026-08-19). Rejection happens before dispatch, so these are network-free. + test("a bogus --flag exits 1 with a JSON error instead of being silently discarded", async () => { + const result = await runCLI(["search", "--text", "test", "--bogus-flag", "xyz"]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + const error = JSON.parse(result.stderr); + expect(error.code).toBe("UNKNOWN_FLAG"); + expect(error.error).toContain("--bogus-flag"); + }); + + test("--query (another portal's free-text flag) is rejected, not treated as no filter", async () => { + const result = await runCLI(["search", "--query", "test"]); + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stderr).code).toBe("UNKNOWN_FLAG"); + }); +}); diff --git a/.agents/skills/jobindex-search/cli/src/cli.ts b/.agents/skills/jobindex-search/cli/src/cli.ts index d89abf7..e816916 100644 --- a/.agents/skills/jobindex-search/cli/src/cli.ts +++ b/.agents/skills/jobindex-search/cli/src/cli.ts @@ -1,4 +1,5 @@ import { createCLI } from "@bunli/core" +import { writeError } from "./helpers.js" import { search } from "./commands/search.js" import { detail } from "./commands/detail.js" @@ -8,7 +9,37 @@ const cli = await createCLI({ description: "CLI for searching jobs on Jobindex.dk", }) -cli.command(search) -cli.command(detail) +const commands = [search, detail] +for (const command of commands) { + cli.command(command) +} + +// Reject unknown --flags before dispatch. bunli silently discards them, and a +// silently discarded filter changes what the search returns without any error +// (a wrong flag name once returned an entire portal's database as if it +// matched the query). add-portal.md's contract requires a bogus flag to exit 1 +// with a JSON error on stderr; this enforces it for the reference CLIs too. +const argv = process.argv.slice(2) +const invoked = commands.find((c) => (c as { name?: string }).name === argv[0]) +if (invoked) { + const known = new Set([ + ...Object.keys((invoked as { options?: Record }).options ?? {}), + "help", + "version", + ]) + for (const token of argv.slice(1)) { + if (token === "--") break + if (token.startsWith("--")) { + const flag = token.slice(2).split("=")[0] + if (!known.has(flag)) { + writeError( + `unknown flag --${flag} for '${argv[0]}' - flags are never silently ignored, because a discarded filter changes what the search returns; see --help for the supported flags`, + "UNKNOWN_FLAG", + ) + process.exit(1) + } + } + } +} await cli.run() diff --git a/.agents/skills/jobindex-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/jobindex-search/cli/tests/cli-flag-validation.test.ts index 3cf10e3..bedb368 100644 --- a/.agents/skills/jobindex-search/cli/tests/cli-flag-validation.test.ts +++ b/.agents/skills/jobindex-search/cli/tests/cli-flag-validation.test.ts @@ -61,3 +61,20 @@ describe("Jobindex CLI flag validation", () => { }); }); }); + + +describe("unknown flag rejection", () => { + // add-portal.md's contract: "a bogus flag or missing required arg exits 1 + // with a JSON error on stderr". A silently discarded flag is worse than an + // error: on jobdanmark a wrong flag name returned the entire database + // (13,862 results) as if it matched the query (review finding F13, + // 2026-08-19). Rejection happens before dispatch, so these are network-free. + test("a bogus --flag exits 1 with a JSON error instead of being silently discarded", async () => { + const result = await runCLI(["search", "--query", "test", "--bogus-flag", "xyz"]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + const error = JSON.parse(result.stderr); + expect(error.code).toBe("UNKNOWN_FLAG"); + expect(error.error).toContain("--bogus-flag"); + }); +}); diff --git a/.agents/skills/jobnet-search/cli/src/cli.ts b/.agents/skills/jobnet-search/cli/src/cli.ts index 644ee9e..4436d81 100644 --- a/.agents/skills/jobnet-search/cli/src/cli.ts +++ b/.agents/skills/jobnet-search/cli/src/cli.ts @@ -1,4 +1,5 @@ import { createCLI } from "@bunli/core" +import { writeError } from "./helpers.js" import { search } from "./commands/search.js" import { detail } from "./commands/detail.js" import { occupations } from "./commands/occupations.js" @@ -10,9 +11,37 @@ const cli = await createCLI({ description: "CLI for the Jobnet.dk Danish government job portal API", }) -cli.command(search) -cli.command(detail) -cli.command(occupations) -cli.command(suggestions) +const commands = [search, detail, occupations, suggestions] +for (const command of commands) { + cli.command(command) +} + +// Reject unknown --flags before dispatch. bunli silently discards them, and a +// silently discarded filter changes what the search returns without any error +// (a wrong flag name once returned an entire portal's database as if it +// matched the query). add-portal.md's contract requires a bogus flag to exit 1 +// with a JSON error on stderr; this enforces it for the reference CLIs too. +const argv = process.argv.slice(2) +const invoked = commands.find((c) => (c as { name?: string }).name === argv[0]) +if (invoked) { + const known = new Set([ + ...Object.keys((invoked as { options?: Record }).options ?? {}), + "help", + "version", + ]) + for (const token of argv.slice(1)) { + if (token === "--") break + if (token.startsWith("--")) { + const flag = token.slice(2).split("=")[0] + if (!known.has(flag)) { + writeError( + `unknown flag --${flag} for '${argv[0]}' - flags are never silently ignored, because a discarded filter changes what the search returns; see --help for the supported flags`, + "UNKNOWN_FLAG", + ) + process.exit(1) + } + } + } +} await cli.run() diff --git a/.agents/skills/jobnet-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/jobnet-search/cli/tests/cli-flag-validation.test.ts index 40f5c74..76bc1b3 100644 --- a/.agents/skills/jobnet-search/cli/tests/cli-flag-validation.test.ts +++ b/.agents/skills/jobnet-search/cli/tests/cli-flag-validation.test.ts @@ -72,3 +72,26 @@ describe("Jobnet CLI flag validation", () => { }); }); }); + + +describe("unknown flag rejection", () => { + // add-portal.md's contract: "a bogus flag or missing required arg exits 1 + // with a JSON error on stderr". A silently discarded flag is worse than an + // error: on jobdanmark a wrong flag name returned the entire database + // (13,862 results) as if it matched the query (review finding F13, + // 2026-08-19). Rejection happens before dispatch, so these are network-free. + test("a bogus --flag exits 1 with a JSON error instead of being silently discarded", async () => { + const result = await runCLI(["search", "--search-string", "test", "--bogus-flag", "xyz"]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + const error = JSON.parse(result.stderr); + expect(error.code).toBe("UNKNOWN_FLAG"); + expect(error.error).toContain("--bogus-flag"); + }); + + test("--query (another portal's free-text flag) is rejected, not treated as no filter", async () => { + const result = await runCLI(["search", "--query", "test"]); + expect(result.exitCode).toBe(1); + expect(JSON.parse(result.stderr).code).toBe("UNKNOWN_FLAG"); + }); +}); diff --git a/.agents/skills/linkedin-search/cli/src/cli.ts b/.agents/skills/linkedin-search/cli/src/cli.ts index 52ada1a..f697398 100644 --- a/.agents/skills/linkedin-search/cli/src/cli.ts +++ b/.agents/skills/linkedin-search/cli/src/cli.ts @@ -63,6 +63,16 @@ EXAMPLES Personal use only — uses LinkedIn's public pages; keep volume low (LinkedIn ToS). ` +// Long-form flag names each command accepts (parseFlags resolves the short +// aliases q/l/n to these before validation). "help"/"h" pass so `search --help` +// still prints usage. +const KNOWN_FLAGS: Record> = { + search: new Set([ + "location", "query", "jobage", "jobage-minutes", "remote", "page", "limit", "format", "help", "h", + ]), + detail: new Set(["format", "help", "h"]), +} + async function main(): Promise { const argv = process.argv.slice(2) const flags = parseFlags(argv) @@ -73,6 +83,25 @@ async function main(): Promise { return cmd ? 0 : 1 } + // Reject unknown flags instead of silently discarding them: a discarded + // filter changes what the search returns with no error (a wrong flag name + // once returned an entire portal's database as if it matched the query). + // add-portal.md's contract requires a bogus flag to exit 1 with a JSON + // error on stderr. + const knownFlags = KNOWN_FLAGS[cmd] + if (knownFlags) { + for (const key of Object.keys(flags)) { + if (key === "_" || knownFlags.has(key)) continue + process.stderr.write( + JSON.stringify({ + error: `unknown flag --${key} for '${cmd}' - flags are never silently ignored, because a discarded filter changes what the search returns; see --help for the supported flags`, + code: "UNKNOWN_FLAG", + }) + "\n", + ) + return 1 + } + } + if (cmd === "search") { const location = typeof flags.location === "string" ? flags.location : undefined if (!location) { diff --git a/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts b/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts index 87c1123..1fa7747 100644 --- a/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts +++ b/.agents/skills/linkedin-search/cli/tests/cli-flag-validation.test.ts @@ -68,13 +68,14 @@ describe("LinkedIn CLI flag validation", () => { // parseFlags in cli.ts treats a next-token starting with "-" as absent // (`next.startsWith("-")` → flag becomes boolean `true`), and there is no // `--flag=value` syntax. So "-5" never reaches --jobage-minutes as a value; - // parseInt("true") is NaN, and BAD_ARG comes from the NaN branch, not the - // `v <= 0` guard. Negatives are unreachable through the CLI as currently parsed. + // it parses as a stray flag named "5", which the unknown-flag guard now + // rejects before the NaN branch can. Either way the invariant holds: a + // negative value fails loudly with exit 1 and a JSON error, never a + // silent unfiltered search. const result = await runCLI(["search", "-l", LOCATION, "--jobage-minutes", "-5"]); expect(result.exitCode).not.toBe(0); const err = parsedStderr(result.stderr); - expect(err.code).toBe("BAD_ARG"); - expect(err.error).toMatch(/jobage-minutes/); + expect(err.code).toBe("UNKNOWN_FLAG"); }); }); @@ -126,3 +127,20 @@ describe("LinkedIn CLI flag validation", () => { }); }); }); + + +describe("unknown flag rejection", () => { + // add-portal.md's contract: "a bogus flag or missing required arg exits 1 + // with a JSON error on stderr". A silently discarded flag is worse than an + // error: on jobdanmark a wrong flag name returned the entire database + // (13,862 results) as if it matched the query (review finding F13, + // 2026-08-19). Rejection happens before dispatch, so these are network-free. + test("a bogus --flag exits 1 with a JSON error instead of being silently discarded", async () => { + const result = await runCLI(["search", "-l", "Denmark", "-q", "test", "--bogus-flag", "xyz"]); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + const error = JSON.parse(result.stderr); + expect(error.code).toBe("UNKNOWN_FLAG"); + expect(error.error).toContain("--bogus-flag"); + }); +}); diff --git a/CHANGELOG.md b/CHANGELOG.md index 15a4e6b..7f957ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -65,6 +65,14 @@ per-file diff commands. ### Changed +- **BREAKING (scripts passing stray flags): all six portal CLIs reject unknown flags** + with exit 1 and `{"error", "code": "UNKNOWN_FLAG"}` on stderr, instead of silently + discarding them. A discarded filter changes what a search returns with no error - a + wrong flag name on jobdanmark returned the entire database (13,862 results, none + matching) as if it matched the query, and the six portals use four different names for + the free-text flag, so cross-portal guessing is likely. `add-portal.md` already + required contributed portals to exit 1 on a bogus flag; the reference CLIs now meet + their own bar. Pinned by nine new cases across the six `cli-flag-validation` suites. - **`/rank` persists its location verdict as `location_verdict`** - the bare `location` key meant two incompatible things in `seen_jobs.json`: a place (scraper search output, driving the commute filter) and a PASS/FAIL/FLAG verdict (`/rank` Step 4), so ranking