From 060f2dbe78f1a4f70fbc208183ad7c0b0e2b79cd Mon Sep 17 00:00:00 2001 From: Rudy Mizrahi Celekli <47457359+rudycelekli@users.noreply.github.com> Date: Sun, 18 Oct 2026 01:02:41 -0400 Subject: [PATCH] fix(portal-clis): reject missing Danish option values before dispatch (#573) * fix(portal-clis): reject missing Danish option values before dispatch The Bunli parser discards known bare options before schema validation, silently dropping filters or applying defaults. Extend the existing pre-dispatch guards across the four Danish portals while preserving built-in switches, valued and equals-form arguments, and existing unknown-flag errors. Actual CLI subprocess regressions deny and record every fetch to demonstrate that missing filters reach request construction on master. Valued-option and help controls keep accepted command forms covered without contacting any portal. * docs(changelog): record fix for #573 --- .agents/skills/jobbank-search/cli/src/cli.ts | 20 ++++++ .../cli/tests/missing-option-values.test.ts | 62 +++++++++++++++++++ .../skills/jobdanmark-search/cli/src/cli.ts | 20 ++++++ .../cli/tests/missing-option-values.test.ts | 62 +++++++++++++++++++ .agents/skills/jobindex-search/cli/src/cli.ts | 20 ++++++ .../cli/tests/missing-option-values.test.ts | 62 +++++++++++++++++++ .agents/skills/jobnet-search/cli/src/cli.ts | 20 ++++++ .../cli/tests/missing-option-values.test.ts | 62 +++++++++++++++++++ CHANGELOG.md | 2 + 9 files changed, 330 insertions(+) create mode 100644 .agents/skills/jobbank-search/cli/tests/missing-option-values.test.ts create mode 100644 .agents/skills/jobdanmark-search/cli/tests/missing-option-values.test.ts create mode 100644 .agents/skills/jobindex-search/cli/tests/missing-option-values.test.ts create mode 100644 .agents/skills/jobnet-search/cli/tests/missing-option-values.test.ts diff --git a/.agents/skills/jobbank-search/cli/src/cli.ts b/.agents/skills/jobbank-search/cli/src/cli.ts index 4bc771bd..c3f1458a 100644 --- a/.agents/skills/jobbank-search/cli/src/cli.ts +++ b/.agents/skills/jobbank-search/cli/src/cli.ts @@ -59,6 +59,26 @@ if (invoked) { if (!knownShorts.has(flag)) rejectFlag(`-${flag}`) } } + // Every declared portal option takes a value; only the built-in help and + // version flags are switches. Bunli drops a bare known option before schema + // validation, so an optional filter disappears or a default is used instead. + // Check after unknown flags so negative tokens keep their existing error. + for (let i = 1; i < argv.length; i++) { + const token = argv[i] + if (token === "--") break + if (!token.startsWith("-") || token.includes("=")) continue + const name = token.replace(/^-+/, "") + const isOption = token.startsWith("--") + ? options[name] !== undefined + : Object.values(options).some((o) => o?.short === name) + if (!isOption) continue + const next = argv[i + 1] + if (next === undefined || next.startsWith("-")) { + writeError(`option ${token} requires a value`, "BAD_ARG") + process.exit(1) + } + } + } await cli.run() diff --git a/.agents/skills/jobbank-search/cli/tests/missing-option-values.test.ts b/.agents/skills/jobbank-search/cli/tests/missing-option-values.test.ts new file mode 100644 index 00000000..79427a6a --- /dev/null +++ b/.agents/skills/jobbank-search/cli/tests/missing-option-values.test.ts @@ -0,0 +1,62 @@ +import { afterAll, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const dir = mkdtempSync(join(tmpdir(), "portal-missing-values-")); +const preload = join(dir, "deny-fetch.ts"); +writeFileSync(preload, `globalThis.fetch = async (input, init) => { + throw new Error("FIXTURE_FETCH_ATTEMPT " + JSON.stringify({ url: String(input), body: init?.body })); +};`); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +async function cli(args: string[]) { + const proc = Bun.spawn([ + process.execPath, "run", "--preload", preload, + join(import.meta.dir, "../src/cli.ts"), ...args, + ], { stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), new Response(proc.stderr).text(), proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +// These are the documented flags sent to the actual CLI, rather than values +// fed directly to its schema. The preload records and denies every fetch so a +// silently dropped filter is observable without contacting the portal. +const missingValueCases: string[][] = [ + ["search", "--key", "python", "--remote"], + ["search", "--key", "python", "--type"], + ["search", "--key", "python", "--company"], + ["search", "--key", "python", "--since", "--limit", "5"], +]; +for (const args of missingValueCases) { + test(`missing option value: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + expect(result.stderr).not.toContain("FIXTURE_FETCH_ATTEMPT"); + expect(JSON.parse(result.stderr).code).toBe("BAD_ARG"); + expect(JSON.parse(result.stderr).error).toContain("requires a value"); + }); +} + +const valuedCases: Array<[string[], string]> = [ + [["search", "--key", "python", "--remote", "helt"], "fjernarbejde=helt"], + [["search", "--key=python", "--type=3,6"], "cvtype=3&cvtype=6"], +]; +for (const [args, value] of valuedCases) { + test(`valued options still reach the request: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("FIXTURE_FETCH_ATTEMPT"); + expect(result.stderr).toContain(value); + expect(JSON.parse(result.stderr).code).toBe("API_ERROR"); + }); +} + +test("built-in short help remains a switch", async () => { + const result = await cli(["search", "-h"]); + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); +}); diff --git a/.agents/skills/jobdanmark-search/cli/src/cli.ts b/.agents/skills/jobdanmark-search/cli/src/cli.ts index 47910e98..cbb6e358 100644 --- a/.agents/skills/jobdanmark-search/cli/src/cli.ts +++ b/.agents/skills/jobdanmark-search/cli/src/cli.ts @@ -62,6 +62,26 @@ if (invoked) { if (!knownShorts.has(flag)) rejectFlag(`-${flag}`) } } + // Every declared portal option takes a value; only the built-in help and + // version flags are switches. Bunli drops a bare known option before schema + // validation, so an optional filter disappears or a default is used instead. + // Check after unknown flags so negative tokens keep their existing error. + for (let i = 1; i < argv.length; i++) { + const token = argv[i] + if (token === "--") break + if (!token.startsWith("-") || token.includes("=")) continue + const name = token.replace(/^-+/, "") + const isOption = token.startsWith("--") + ? options[name] !== undefined + : Object.values(options).some((o) => o?.short === name) + if (!isOption) continue + const next = argv[i + 1] + if (next === undefined || next.startsWith("-")) { + writeError(`option ${token} requires a value`, "BAD_ARG") + process.exit(1) + } + } + } await cli.run() diff --git a/.agents/skills/jobdanmark-search/cli/tests/missing-option-values.test.ts b/.agents/skills/jobdanmark-search/cli/tests/missing-option-values.test.ts new file mode 100644 index 00000000..3de35ec9 --- /dev/null +++ b/.agents/skills/jobdanmark-search/cli/tests/missing-option-values.test.ts @@ -0,0 +1,62 @@ +import { afterAll, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const dir = mkdtempSync(join(tmpdir(), "portal-missing-values-")); +const preload = join(dir, "deny-fetch.ts"); +writeFileSync(preload, `globalThis.fetch = async (input, init) => { + throw new Error("FIXTURE_FETCH_ATTEMPT " + JSON.stringify({ url: String(input), body: init?.body })); +};`); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +async function cli(args: string[]) { + const proc = Bun.spawn([ + process.execPath, "run", "--preload", preload, + join(import.meta.dir, "../src/cli.ts"), ...args, + ], { stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), new Response(proc.stderr).text(), proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +// These are the documented flags sent to the actual CLI, rather than values +// fed directly to its schema. The preload records and denies every fetch so a +// silently dropped filter is observable without contacting the portal. +const missingValueCases: string[][] = [ + ["search", "--category"], + ["search", "--municipality"], + ["search", "--jobtitle-id"], + ["search", "--region", "--limit", "5"], +]; +for (const args of missingValueCases) { + test(`missing option value: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + expect(result.stderr).not.toContain("FIXTURE_FETCH_ATTEMPT"); + expect(JSON.parse(result.stderr).code).toBe("BAD_ARG"); + expect(JSON.parse(result.stderr).error).toContain("requires a value"); + }); +} + +const valuedCases: Array<[string[], string]> = [ + [["search", "--municipality", "Odense"], "Odense"], + [["search", "--category=3"], "category"], +]; +for (const [args, value] of valuedCases) { + test(`valued options still reach the request: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("FIXTURE_FETCH_ATTEMPT"); + expect(result.stderr).toContain(value); + expect(JSON.parse(result.stderr).code).toBe("API_ERROR"); + }); +} + +test("built-in short help remains a switch", async () => { + const result = await cli(["search", "-h"]); + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); +}); diff --git a/.agents/skills/jobindex-search/cli/src/cli.ts b/.agents/skills/jobindex-search/cli/src/cli.ts index e496c078..9b7779be 100644 --- a/.agents/skills/jobindex-search/cli/src/cli.ts +++ b/.agents/skills/jobindex-search/cli/src/cli.ts @@ -59,6 +59,26 @@ if (invoked) { if (!knownShorts.has(flag)) rejectFlag(`-${flag}`) } } + // Every declared portal option takes a value; only the built-in help and + // version flags are switches. Bunli drops a bare known option before schema + // validation, so an optional filter disappears or a default is used instead. + // Check after unknown flags so negative tokens keep their existing error. + for (let i = 1; i < argv.length; i++) { + const token = argv[i] + if (token === "--") break + if (!token.startsWith("-") || token.includes("=")) continue + const name = token.replace(/^-+/, "") + const isOption = token.startsWith("--") + ? options[name] !== undefined + : Object.values(options).some((o) => o?.short === name) + if (!isOption) continue + const next = argv[i + 1] + if (next === undefined || next.startsWith("-")) { + writeError(`option ${token} requires a value`, "BAD_ARG") + process.exit(1) + } + } + } await cli.run() diff --git a/.agents/skills/jobindex-search/cli/tests/missing-option-values.test.ts b/.agents/skills/jobindex-search/cli/tests/missing-option-values.test.ts new file mode 100644 index 00000000..33b322a9 --- /dev/null +++ b/.agents/skills/jobindex-search/cli/tests/missing-option-values.test.ts @@ -0,0 +1,62 @@ +import { afterAll, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const dir = mkdtempSync(join(tmpdir(), "portal-missing-values-")); +const preload = join(dir, "deny-fetch.ts"); +writeFileSync(preload, `globalThis.fetch = async (input, init) => { + throw new Error("FIXTURE_FETCH_ATTEMPT " + JSON.stringify({ url: String(input), body: init?.body })); +};`); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +async function cli(args: string[]) { + const proc = Bun.spawn([ + process.execPath, "run", "--preload", preload, + join(import.meta.dir, "../src/cli.ts"), ...args, + ], { stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), new Response(proc.stderr).text(), proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +// These are the documented flags sent to the actual CLI, rather than values +// fed directly to its schema. The preload records and denies every fetch so a +// silently dropped filter is observable without contacting the portal. +const missingValueCases: string[][] = [ + ["search", "--query", "python", "--sort"], + ["search", "--query", "python", "--page"], + ["search", "--query", "python", "--jobage", "--limit", "5"], + ["search", "-q"], +]; +for (const args of missingValueCases) { + test(`missing option value: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + expect(result.stderr).not.toContain("FIXTURE_FETCH_ATTEMPT"); + expect(JSON.parse(result.stderr).code).toBe("BAD_ARG"); + expect(JSON.parse(result.stderr).error).toContain("requires a value"); + }); +} + +const valuedCases: Array<[string[], string]> = [ + [["search", "-q", "python", "--sort", "date"], "sort=date"], + [["search", "--query=-python", "--page=2"], "q=-python"], +]; +for (const [args, value] of valuedCases) { + test(`valued options still reach the request: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("FIXTURE_FETCH_ATTEMPT"); + expect(result.stderr).toContain(value); + expect(JSON.parse(result.stderr).code).toBe("API_ERROR"); + }); +} + +test("built-in short help remains a switch", async () => { + const result = await cli(["search", "-h"]); + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); +}); diff --git a/.agents/skills/jobnet-search/cli/src/cli.ts b/.agents/skills/jobnet-search/cli/src/cli.ts index 3f69b3ae..2abd40c2 100644 --- a/.agents/skills/jobnet-search/cli/src/cli.ts +++ b/.agents/skills/jobnet-search/cli/src/cli.ts @@ -61,6 +61,26 @@ if (invoked) { if (!knownShorts.has(flag)) rejectFlag(`-${flag}`) } } + // Every declared portal option takes a value; only the built-in help and + // version flags are switches. Bunli drops a bare known option before schema + // validation, so an optional filter disappears or a default is used instead. + // Check after unknown flags so negative tokens keep their existing error. + for (let i = 1; i < argv.length; i++) { + const token = argv[i] + if (token === "--") break + if (!token.startsWith("-") || token.includes("=")) continue + const name = token.replace(/^-+/, "") + const isOption = token.startsWith("--") + ? options[name] !== undefined + : Object.values(options).some((o) => o?.short === name) + if (!isOption) continue + const next = argv[i + 1] + if (next === undefined || next.startsWith("-")) { + writeError(`option ${token} requires a value`, "BAD_ARG") + process.exit(1) + } + } + } await cli.run() diff --git a/.agents/skills/jobnet-search/cli/tests/missing-option-values.test.ts b/.agents/skills/jobnet-search/cli/tests/missing-option-values.test.ts new file mode 100644 index 00000000..cbb003b6 --- /dev/null +++ b/.agents/skills/jobnet-search/cli/tests/missing-option-values.test.ts @@ -0,0 +1,62 @@ +import { afterAll, expect, test } from "bun:test"; +import { mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; + +const dir = mkdtempSync(join(tmpdir(), "portal-missing-values-")); +const preload = join(dir, "deny-fetch.ts"); +writeFileSync(preload, `globalThis.fetch = async (input, init) => { + throw new Error("FIXTURE_FETCH_ATTEMPT " + JSON.stringify({ url: String(input), body: init?.body })); +};`); +afterAll(() => rmSync(dir, { recursive: true, force: true })); + +async function cli(args: string[]) { + const proc = Bun.spawn([ + process.execPath, "run", "--preload", preload, + join(import.meta.dir, "../src/cli.ts"), ...args, + ], { stdout: "pipe", stderr: "pipe" }); + const [stdout, stderr, exitCode] = await Promise.all([ + new Response(proc.stdout).text(), new Response(proc.stderr).text(), proc.exited, + ]); + return { stdout, stderr, exitCode }; +} + +// These are the documented flags sent to the actual CLI, rather than values +// fed directly to its schema. The preload records and denies every fetch so a +// silently dropped filter is observable without contacting the portal. +const missingValueCases: string[][] = [ + ["search", "--per-page"], + ["search", "--postal-code", "5000", "--radius"], + ["search", "--region"], + ["search", "--work-hours", "--limit", "5"], +]; +for (const args of missingValueCases) { + test(`missing option value: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stdout).toBe(""); + expect(result.stderr).not.toContain("FIXTURE_FETCH_ATTEMPT"); + expect(JSON.parse(result.stderr).code).toBe("BAD_ARG"); + expect(JSON.parse(result.stderr).error).toContain("requires a value"); + }); +} + +const valuedCases: Array<[string[], string]> = [ + [["search", "--region", "Nordjylland"], "regions=Nordjylland"], + [["search", "--postal-code=5000", "--radius=25"], "kmRadius=25"], +]; +for (const [args, value] of valuedCases) { + test(`valued options still reach the request: ${args.join(" ")}`, async () => { + const result = await cli(args); + expect(result.exitCode).toBe(1); + expect(result.stderr).toContain("FIXTURE_FETCH_ATTEMPT"); + expect(result.stderr).toContain(value); + expect(JSON.parse(result.stderr).code).toBe("API_ERROR"); + }); +} + +test("built-in short help remains a switch", async () => { + const result = await cli(["search", "-h"]); + expect(result.exitCode).toBe(0); + expect(result.stderr).toBe(""); +}); diff --git a/CHANGELOG.md b/CHANGELOG.md index 78f0e01e..7614ffe1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -285,6 +285,8 @@ per-file diff commands. the fixed set it is; parsing `apply.md`'s example list was deliberately not done, since a parser keyed on prose wording is the next thing to go stale. Reported by Holo-Eter. +- **Danish portal CLIs reject missing option values** (#573) - value-taking flags followed by another option now fail before search dispatch instead of treating the next flag as the value. + - **`/expand` and `/setup` now append behavioral findings to sections `02-behavioral-profile.md` actually has** (`.claude/commands/expand.md` Step 5, `.claude/commands/setup.md` Step A7, `tests/test_expand_command.py`, `tests/test_setup_command.py`) - both told the model to add