Skip to content

Commit cdec4e1

Browse files
committed
fix(cli): restore unknown-option errors swallowed by positional args
Cliffy 1.2 parses an unrecognized `--flag` into the next free positional slot instead of raising an unknown-option error. Since the dependency bump, `deno deploy --prd` was silently treated as `deno deploy <root-path=--prd>`, so a typo'd flag ran the deploy action (and reported an auth error) instead of exiting 2 with a VALIDATION_ERROR envelope. Two CLI-contract tests have been failing on main because of this. Reject flag-like positionals in `actionHandler`, except for slots that forward their value verbatim (a shell command, an env var value, a SQL query), which are identified by their declared argument name. Also gives the hidden `tunnel-login` command the one-line description every other command has.
1 parent 81c0e91 commit cdec4e1

3 files changed

Lines changed: 63 additions & 0 deletions

File tree

config.ts

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -167,6 +167,42 @@ export interface ConfigContext {
167167
noCreate(): void;
168168
}
169169

170+
/**
171+
* Positional slots that legitimately receive flag-like values: they are
172+
* forwarded verbatim (a shell command, an env var value, a SQL query) rather
173+
* than interpreted by this CLI.
174+
*/
175+
const PASSTHROUGH_ARGUMENTS = new Set(["command", "value", "query"]);
176+
177+
interface ArgumentDef {
178+
name: string;
179+
variadic?: boolean;
180+
}
181+
182+
/**
183+
* Cliffy >= 1.2 fills the next positional slot with an unrecognized `--flag`
184+
* instead of raising an unknown-option error, so `deno deploy --prd` would
185+
* silently deploy a directory literally named `--prd`. Restore the usage error
186+
* for every slot that isn't a passthrough.
187+
*/
188+
function assertNoFlagLikeArgs(command: unknown, args: unknown[]): void {
189+
const defs: ArgumentDef[] =
190+
(command as { getArguments?(): ArgumentDef[] } | undefined)
191+
?.getArguments?.() ?? [];
192+
const last = defs.at(-1);
193+
194+
for (let i = 0; i < args.length; i++) {
195+
const arg = args[i];
196+
if (typeof arg !== "string" || arg.length < 2 || !arg.startsWith("-")) {
197+
continue;
198+
}
199+
// A trailing variadic slot absorbs every remaining argument.
200+
const def = defs[i] ?? (last?.variadic ? last : undefined);
201+
if (def && PASSTHROUGH_ARGUMENTS.has(def.name)) continue;
202+
throw new ValidationError(`Unknown option "${arg}".`);
203+
}
204+
}
205+
170206
export function actionHandler<
171207
O extends GlobalContext,
172208
A extends unknown[] = unknown[],
@@ -181,6 +217,7 @@ export function actionHandler<
181217
rootPath?: (...args: A) => string | undefined,
182218
): (options: O, ...args: A) => Promise<void> {
183219
return async function (this: unknown, context: O, ...args: A) {
220+
assertNoFlagLikeArgs(this, args);
184221
try {
185222
const root = rootPath?.(...args) ?? Deno.cwd();
186223
// Discover the config file only (cheap). The deploy file manifest is a

deploy/mod.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,7 @@ const setupGCPCommand = new Command<GlobalContext>()
107107
}));
108108

109109
const tunnelLoginCommand = new Command<GlobalContext>()
110+
.description("Resolve org, app and token for `deno tunnel` (internal)")
110111
.option("--really-no-config", "really no config")
111112
.option("--out <file:string>", "out file")
112113
.hidden()

tests/cli_contract.test.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,31 @@ Deno.test("combined short flag -jy is detected as JSON mode for the error envelo
4848
assertEquals(envelope.error.code, "VALIDATION_ERROR");
4949
});
5050

51+
Deno.test("an unknown flag is not swallowed by the root's [root-path] argument", async () => {
52+
const res = await runCli(["--json", "--prd"]);
53+
assertEquals(res.code, 2, `stderr: ${res.stderr}`);
54+
const envelope = JSON.parse(res.stderr.trim().split("\n").pop()!);
55+
assertEquals(envelope.error.code, "VALIDATION_ERROR");
56+
assert(
57+
envelope.error.message.includes("--prd"),
58+
`message should name the flag: ${envelope.error.message}`,
59+
);
60+
});
61+
62+
Deno.test("flag-like positional values are still passed through", async () => {
63+
// `env add FOO -bar` must reach the action (which then requires auth) rather
64+
// than being rejected as an unknown option.
65+
const res = await runCli(
66+
["env", "add", "FOO", "-bar", "--json", "--non-interactive"],
67+
{ DENO_DEPLOY_TOKEN: "" }, // never reach the backend, whatever the runner has
68+
);
69+
const envelope = JSON.parse(res.stderr.trim().split("\n").pop()!);
70+
assert(
71+
envelope.error.code !== "VALIDATION_ERROR",
72+
`unexpected usage error: ${res.stderr}`,
73+
);
74+
});
75+
5176
Deno.test("--help exits 0", async () => {
5277
const res = await runCli(["--help"]);
5378
assertEquals(res.code, 0, `stderr: ${res.stderr}`);

0 commit comments

Comments
 (0)