fix(portal-clis): reject undefined single-dash flags in the unknown-flag guard (#428)

The guard in the four bunli-based CLIs inspected only tokens starting
with `--`, so an undefined short flag bypassed it: bunli discarded it,
the search ran unfiltered, and the CLI exited 0. Live against jobnet,
`search -q "sygeplejerske"` returned all 18,179 ads as a successful
search against 667 for the real `--search-string` query - the same shape
as review finding F13 that motivated the guard.

Both dash forms are now checked. Declared shorts (jobindex's -q) and
bunli's built-in -h/-v stay valid. A negative number is rejected too:
bunli discards a `-`-prefixed token rather than consuming it as the
previous flag's value, so `--radius -5` silently fell back to the
default instead of failing its own min(1) schema; a value that must
begin with a dash uses the `--flag=value` form.

Fixes #426.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
OluwaJomiloju
2026-09-03 19:40:55 +02:00
committed by GitHub
co-authored by Claude Opus 5
parent c844359ed9
commit fa8db56a96
9 changed files with 272 additions and 52 deletions
+32 -13
View File
@@ -14,30 +14,49 @@ for (const command of commands) {
cli.command(command)
}
// Reject unknown --flags before dispatch. bunli silently discards them, and a
// 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.
//
// Both dash forms are checked. This loop inspected only `--long` tokens until
// #426, so an undefined short flag was discarded in silence: `-q "..."` on a
// portal whose keyword flag is `--search-string` returned the whole database
// as a successful, unfiltered search. Declared shorts and bunli's built-in
// -h/-v stay valid; every other single-dash token is rejected, including a
// negative number. bunli does not consume a `-`-prefixed token as the previous
// flag's value - it discards it - so `--radius -5` silently fell back to the
// default radius rather than failing its own `min(1)` schema. Erroring on it
// is the same trade linkedin-search already makes. A value that must begin
// with a dash uses the `--flag=value` form, which is checked as a long flag.
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<string, unknown> }).options ?? {}),
"help",
"version",
])
const options =
(invoked as { options?: Record<string, { short?: string } | undefined> }).options ?? {}
const known = new Set([...Object.keys(options), "help", "version"])
const knownShorts = new Set(
Object.values(options)
.map((o) => o?.short)
.filter((s): s is string => typeof s === "string")
.concat("h", "v"),
)
const rejectFlag = (rendered: string): never => {
writeError(
`unknown flag ${rendered} 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)
}
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)
}
if (!known.has(flag)) rejectFlag(`--${flag}`)
} else if (token.startsWith("-") && token !== "-") {
const flag = token.slice(1).split("=")[0]
if (!knownShorts.has(flag)) rejectFlag(`-${flag}`)
}
}
}
@@ -77,4 +77,40 @@ describe("unknown flag rejection", () => {
expect(error.code).toBe("UNKNOWN_FLAG");
expect(error.error).toContain("--bogus-flag");
});
// #426: the guard inspected only `--long` tokens, so a single-dash flag was
// discarded in silence. This CLI is the one portal that declares a short
// (`-q` for --query), so the fix has to reject undeclared shorts without
// breaking the declared one.
test("an undeclared short flag exits 1 with a JSON error", async () => {
const result = await runCLI(["search", "-z", "bogus"]);
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("-z");
});
// Network-free proof that the declared short survives the guard: -q is
// scanned before --bogus-flag, so naming --bogus-flag in the error means -q
// passed. Asserting -q is accepted directly would require a live search.
test("the declared short -q passes the guard", async () => {
const result = await runCLI(["search", "-q", "test", "--bogus-flag", "xyz"]);
expect(result.exitCode).toBe(1);
const error = JSON.parse(result.stderr);
expect(error.error).toContain("--bogus-flag");
expect(error.error).not.toContain("-q ");
});
test("a negative number is rejected instead of silently falling back to the default", async () => {
const result = await runCLI(["search", "--query", "test", "--limit", "-5"]);
expect(result.exitCode).toBe(1);
expect(JSON.parse(result.stderr).code).toBe("UNKNOWN_FLAG");
});
test("-h still prints help rather than being rejected as unknown", async () => {
const result = await runCLI(["search", "-h"]);
expect(result.exitCode).toBe(0);
expect(result.stderr).toBe("");
});
});