Reject invalid CLI arguments before side effects

src/argv.ts

@@ -1,143 +1,47 @@
+import { BOOLEAN_FLAGS, COMMAND_SPECS, GLOBAL_FLAGS } from "./command-spec.js";
+import { usageError } from "./problems.js";
+
 export interface ParsedArgs {
   command: string[];
   flags: Record<string, string | boolean>;
   positionals: string[];
 }
 
-function takeValue(
-  argv: string[],
-  index: number,
-  current: string,
-): { value: string; nextIndex: number } {
-  const eq = current.indexOf("=");
-  if (eq >= 0) {
-    return { value: current.slice(eq + 1), nextIndex: index };
-  }
-  const next = argv[index + 1];
-  if (next === undefined || next.startsWith("-")) {
-    return { value: "", nextIndex: index };
-  }
-  return { value: next, nextIndex: index + 1 };
-}
-
-const VALUE_FLAGS = new Set([
-  "api-url",
-  "beta-key",
-  "email",
-  "token",
-  "config",
-  "idempotency-key",
-  "request-id",
-  "timeout",
-  "output",
-  "only",
-  "target-width",
-  "target-height",
-  "ink-padding",
-  "cursor",
-  "after",
-  "limit",
-  "if-match",
-  "revision",
-  "id",
-  "key",
-  "value",
-  "json-value",
-  "file",
-  "value-base64",
-  "application-id",
-  "playlist-id",
-  "timezone",
-  "screen-id",
-  "media-id",
-  "name",
-  "type",
-  "severity",
-  "operation-id",
-  "poll-ms",
-  "label",
-  "content-type",
-  "code",
-  "codec",
-  "preset",
-  "max-fps",
-  "max-edge",
-  "webp-quality",
-  "body",
-  "body-file",
-  "command",
-  "kind",
-  "primitive",
-  "level",
-  "text",
-  "duration-ms",
-  "tag",
-  "day",
-  "state",
-  "page",
-  "update",
-  "concurrency",
-  "frame-ms",
-  "prompt",
-  "aspect-ratio",
-  "quality",
-]);
-
 export function parseArgv(argv: string[]): ParsedArgs {
-  const flags: Record<string, string | boolean> = {};
+  const flags: Record<string, string | boolean> = Object.create(null) as Record<string, string | boolean>;
   const positionals: string[] = [];
+  const known = new Set<string>([...GLOBAL_FLAGS, ...COMMAND_SPECS.flatMap((spec) => spec.flags)]);
   for (let i = 0; i < argv.length; i += 1) {
-    const arg = argv[i];
-    if (arg === undefined) {
-      continue;
-    }
+    const arg = argv[i]!;
     if (arg === "--") {
       positionals.push(...argv.slice(i + 1));
       break;
     }
-    if (arg === "--json") {
-      flags.json = true;
+    if (!arg.startsWith("-") || arg === "-") {
+      positionals.push(arg);
       continue;
     }
-    if (arg === "--no-wait") {
-      flags["no-wait"] = true;
-      continue;
-    }
-    if (arg === "--repair-config") {
-      flags["repair-config"] = true;
-      continue;
-    }
-    if (arg === "--help" || arg === "-h") {
-      flags.help = true;
-      continue;
-    }
-    if (arg === "--version" || arg === "-V") {
-      flags.version = true;
-      continue;
-    }
-    if (arg.startsWith("--")) {
-      const raw = arg.slice(2);
-      const name = raw.split("=")[0] ?? raw;
-      if (VALUE_FLAGS.has(name) || raw.includes("=")) {
-        const taken = takeValue(argv, i, raw);
-        if (taken.value === "") {
-          flags[name] = true;
-        } else {
-          flags[name] = taken.value;
-          i = taken.nextIndex;
-        }
-        continue;
-      }
+    const normalized = arg === "-h" ? "--help" : arg === "-V" ? "--version" : arg;
+    if (!normalized.startsWith("--")) throw usageError("Unsupported short option. Use --help to discover supported options.");
+    const eq = normalized.indexOf("=");
+    const name = normalized.slice(2, eq < 0 ? undefined : eq);
+    // Do not echo unknown names or values: either can contain credentials.
+    if (!known.has(name)) throw usageError("Unsupported option. Use --help to discover supported options.");
+    if (Object.hasOwn(flags, name)) throw usageError(`--${name} may be supplied only once.`);
+    if (BOOLEAN_FLAGS.has(name)) {
+      if (eq >= 0) throw usageError(`--${name} takes no value.`);
       flags[name] = true;
       continue;
     }
-    positionals.push(arg);
+    const value = eq >= 0 ? normalized.slice(eq + 1) : argv[i + 1];
+    if (value === undefined || (eq < 0 && value.startsWith("-") && !/^-\d/.test(value))) {
+      throw usageError(`--${name} requires a value.`);
+    }
+    if (value.length === 0 && name !== "value-base64") throw usageError(`--${name} requires a value.`);
+    flags[name] = value;
+    if (eq < 0) i += 1;
   }
-  return {
-    command: positionals.slice(0, 2),
-    flags,
-    positionals,
-  };
+  return { command: positionals.slice(0, 2), flags, positionals };
 }
 
 export function flagString(flags: Record<string, string | boolean>, name: string): string | undefined {

src/cli.test.ts

@@ -4659,11 +4659,10 @@ test("screen update cannot send observation", async () => {
     transport,
     { fs: fsLike },
   );
-  assert.equal(result.code, ExitCode.Success, result.stdout);
+  assert.equal(result.code, ExitCode.Usage, result.stdout);
   const patch = transport.calls.find((call) => call.method === "PATCH");
-  assert.equal(patch?.path, "/api/v1/screens/scr_PAIRINGAAAAAAAAAAAAAAAA");
-  assert.deepEqual(patch?.body, { name: "Lobby" });
-  assert.equal(JSON.stringify(patch?.body).includes("observation"), false);
+  assert.equal(patch, undefined);
+  assert.equal(transport.calls.length, 0, "unsupported flags must fail before any request");
   assert.equal(transport.calls.some((call) => call.path === "/runtime/v1/observation"), false);
   await rm(configDir, { recursive: true, force: true });
 });
@@ -4804,14 +4803,10 @@ test("screen update cannot send online, last_online_at, or last_ip", async () =>
     transport,
     { fs: fsLike },
   );
-  assert.equal(result.code, ExitCode.Success, result.stdout);
+  assert.equal(result.code, ExitCode.Usage, result.stdout);
   const patch = transport.calls.find((call) => call.method === "PATCH");
-  assert.equal(patch?.path, "/api/v1/screens/scr_PAIRINGAAAAAAAAAAAAAAAA");
-  assert.deepEqual(patch?.body, { name: "Lobby" });
-  const patchJson = JSON.stringify(patch?.body);
-  assert.equal(patchJson.includes("online"), false);
-  assert.equal(patchJson.includes("last_online_at"), false);
-  assert.equal(patchJson.includes("last_ip"), false);
+  assert.equal(patch, undefined);
+  assert.equal(transport.calls.length, 0, "unsupported flags must fail before any request");
   await rm(configDir, { recursive: true, force: true });
 });
 

src/command-spec.ts

@@ -0,0 +1,86 @@
+/** Supported invocation shapes shared by argument validation and help discovery. */
+export interface CommandSpec {
+  readonly path: readonly string[];
+  readonly minArgs: number;
+  readonly maxArgs: number;
+  readonly aliases?: readonly (readonly string[])[];
+  readonly flags: readonly string[];
+}
+
+export const GLOBAL_FLAGS = ["json", "help", "version", "api-url", "config", "request-id", "idempotency-key", "timeout", "beta-key", "token"] as const;
+
+export const BOOLEAN_FLAGS = new Set([
+  "json", "help", "version", "open-dashboard", "print-url", "yes", "allow-lockout",
+  "no-wait", "no-transcode", "no-progress", "no-audio", "clear-tag", "safe-area",
+  "lint-only", "combined", "open", "contact-sheet", "no-context", "repair-config",
+]);
+
+export const COMMAND_SPECS: readonly CommandSpec[] = [
+  { path: ["account", "show"], minArgs: 0, maxArgs: 0, flags: [] },
+  { path: ["agent", "enroll"], minArgs: 0, maxArgs: 0, flags: ["email", "name", "open-dashboard"] },
+  { path: ["agent", "connect"], minArgs: 0, maxArgs: 0, flags: ["name", "print-url"] },
+  { path: ["agent", "status"], minArgs: 0, maxArgs: 0, flags: [] },
+  { path: ["agent", "disconnect"], minArgs: 0, maxArgs: 0, flags: ["yes", "allow-lockout"] },
+  { path: ["dashboard"], minArgs: 0, maxArgs: 0, flags: ["print-url"] },
+  { path: ["app", "pack"], minArgs: 1, maxArgs: 1, flags: ["output"] },
+  { path: ["app", "upload"], minArgs: 1, maxArgs: 1, flags: ["name", "no-wait", "poll-ms"] },
+  { path: ["app", "update"], minArgs: 2, maxArgs: 2, flags: ["if-match", "no-wait", "poll-ms"] },
+  { path: ["app", "list"], minArgs: 0, maxArgs: 0, flags: [] },
+  { path: ["app", "show"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["media", "generate"], minArgs: 0, maxArgs: 0, flags: ["prompt", "aspect-ratio", "quality", "tag"] },
+  { path: ["media", "upload"], minArgs: 1, maxArgs: 1, flags: ["content-type", "tag", "no-wait", "poll-ms", "no-transcode", "codec", "max-fps", "max-edge", "webp-quality", "no-progress", "preset", "no-audio"] },
+  { path: ["media", "upload-batch"], minArgs: 1, maxArgs: 1, flags: ["state", "concurrency", "no-transcode", "tag", "no-progress", "poll-ms", "codec", "max-fps", "max-edge", "webp-quality", "preset", "no-audio"] },
+  { path: ["media", "show"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["media", "download"], minArgs: 1, maxArgs: 1, flags: ["output"] },
+  { path: ["media", "list"], minArgs: 0, maxArgs: 0, flags: ["tag", "primitive"] },
+  { path: ["media", "update"], minArgs: 1, maxArgs: 1, flags: ["tag", "clear-tag", "if-match"] },
+  { path: ["media", "delete"], minArgs: 1, maxArgs: 1, flags: ["if-match"] },
+  { path: ["compose", "catalog"], minArgs: 0, maxArgs: 0, flags: [] },
+  { path: ["compose", "batch"], minArgs: 1, maxArgs: 1, flags: ["output", "only", "target-width", "target-height", "safe-area", "lint-only"] },
+  { path: ["compose", "render"], minArgs: 1, maxArgs: 1, flags: ["output", "combined", "target-width", "target-height", "safe-area", "open", "lint-only"] },
+  { path: ["playlist", "validate"], minArgs: 1, maxArgs: 1, flags: ["lint-only"] },
+  { path: ["playlist", "preview"], minArgs: 1, maxArgs: 1, flags: ["output", "frame-ms", "contact-sheet", "lint-only"] },
+  { path: ["playlist", "templates"], minArgs: 0, maxArgs: 0, flags: [] },
+  { path: ["playlist", "create"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["playlist", "update"], minArgs: 2, maxArgs: 2, flags: ["if-match"] },
+  { path: ["playlist", "export"], minArgs: 1, maxArgs: 1, flags: ["output"] },
+  { path: ["playlist", "import"], minArgs: 1, maxArgs: 1, flags: ["name", "update", "if-match", "poll-ms"] },
+  { path: ["playlist", "show"], aliases: [["playlist", "get"]], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["playlist", "list"], minArgs: 0, maxArgs: 0, flags: [] },
+  { path: ["playlist", "delete"], minArgs: 1, maxArgs: 1, flags: ["if-match"] },
+  { path: ["screen", "pair"], minArgs: 1, maxArgs: 1, flags: ["label"] },
+  { path: ["screen", "provision"], minArgs: 0, maxArgs: 0, flags: ["open", "print-url", "label"] },
+  { path: ["browser", "setup"], minArgs: 0, maxArgs: 0, flags: ["code", "open"] },
+  { path: ["screen", "update"], minArgs: 1, maxArgs: 1, flags: ["name", "playlist-id", "timezone", "if-match"] },
+  { path: ["screen", "list"], minArgs: 0, maxArgs: 0, flags: ["state"] },
+  { path: ["screen", "show"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["screen", "assign"], minArgs: 1, maxArgs: 1, flags: ["playlist-id", "if-match"] },
+  { path: ["screen", "set-timezone"], minArgs: 1, maxArgs: 1, flags: ["timezone", "if-match"] },
+  { path: ["screen", "archive"], minArgs: 1, maxArgs: 1, flags: ["if-match"] },
+  { path: ["screen", "unarchive"], minArgs: 1, maxArgs: 1, flags: ["if-match"] },
+  { path: ["screen", "delete"], minArgs: 1, maxArgs: 1, flags: ["if-match"] },
+  { path: ["screen", "rotate-public-id"], minArgs: 1, maxArgs: 1, flags: ["if-match"] },
+  { path: ["screen", "toast"], minArgs: 1, maxArgs: 1, flags: ["text", "level", "duration-ms"] },
+  { path: ["screen", "screenshot"], minArgs: 1, maxArgs: 1, flags: ["output", "poll-ms"] },
+  { path: ["kv", "get"], minArgs: 1, maxArgs: 1, flags: ["application-id"] },
+  { path: ["kv", "set"], minArgs: 1, maxArgs: 1, flags: ["application-id", "json-value", "file", "value-base64", "content-type", "if-match"] },
+  { path: ["kv", "delete"], minArgs: 1, maxArgs: 1, flags: ["application-id", "if-match"] },
+  { path: ["kv", "list"], minArgs: 0, maxArgs: 0, flags: ["application-id"] },
+  { path: ["comment", "show", "screen"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["comment", "show", "playlist"], minArgs: 1, maxArgs: 1, flags: ["page"] },
+  { path: ["comment", "set", "screen"], minArgs: 1, maxArgs: 1, flags: ["json-value", "file"] },
+  { path: ["comment", "set", "playlist"], minArgs: 1, maxArgs: 1, flags: ["page", "json-value", "file"] },
+  { path: ["comment", "delete", "screen"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["comment", "delete", "playlist"], minArgs: 1, maxArgs: 1, flags: ["page"] },
+  { path: ["operations", "get"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["operations", "wait"], minArgs: 1, maxArgs: 1, flags: ["poll-ms"] },
+  { path: ["operations", "cancel"], minArgs: 1, maxArgs: 1, flags: [] },
+  { path: ["events", "list"], minArgs: 0, maxArgs: 0, flags: ["after", "cursor", "limit"] },
+  { path: ["events", "follow"], minArgs: 0, maxArgs: 0, flags: ["after", "cursor"] },
+  { path: ["playback", "list"], minArgs: 0, maxArgs: 0, flags: ["screen-id", "media-id", "day"] },
+  { path: ["feedback", "bug"], minArgs: 1, maxArgs: 1, flags: ["body", "body-file", "command", "no-context"] },
+  { path: ["feedback", "feature"], minArgs: 1, maxArgs: 1, flags: ["body", "body-file", "command", "no-context"] },
+  { path: ["feedback", "list"], minArgs: 0, maxArgs: 0, flags: ["kind"] },
+  { path: ["doctor"], minArgs: 0, maxArgs: 0, flags: ["repair-config"] },
+  { path: ["version"], minArgs: 0, maxArgs: 0, flags: [] },
+];

src/commands.ts

@@ -138,6 +138,7 @@ import {
   type AgentConnectionConfig,
 } from "./agent-identity.js";
 import { CLI_VERSION } from "./version.js";
+import { validateInvocation } from "./validate-invocation.js";
 
 export { CLI_VERSION };
 
@@ -474,6 +475,7 @@ function humanLines(title: string, fields: Array<[string, string | undefined]>):
 }
 
 export async function dispatch(args: ParsedArgs, runtime: CliRuntime): Promise<CommandResult> {
+  validateInvocation(args);
   const group = args.positionals[0];
   const action = args.positionals[1];
   if (!group || flagBool(args.flags, "help") || group === "help") {

src/log/logger.test.ts

@@ -123,9 +123,8 @@ async function listenUnix(socketPath: string): Promise<{ events: LogEvent[]; wai
 test("USAGE and argv do not define a log-socket flag", () => {
   assert.doesNotMatch(USAGE, /\[--log-socket/);
   assert.match(USAGE, /log_socket/);
-  const parsed = parseArgv(["--log-socket", "/tmp/screenrig.sock", "version"]);
-  assert.equal(parsed.flags["log-socket"], true);
-  assert.equal(parsed.positionals[0], "/tmp/screenrig.sock");
+  assert.throws(() => parseArgv(["--log-socket", "/tmp/screenrig.sock", "version"]),
+    (error: unknown) => error instanceof CliError && error.problem.code === "usage_error");
 });
 
 test("HTTP request and response share correlation_id with distinct event_id", async () => {

src/validate-invocation.test.ts

@@ -0,0 +1,59 @@
+import assert from "node:assert/strict";
+import { test } from "node:test";
+import { parseArgv } from "./argv.js";
+import { dispatch } from "./commands.js";
+import { CliError } from "./problems.js";
+import type { CliRuntime } from "./runtime.js";
+import { validateInvocation } from "./validate-invocation.js";
+
+const invalid = [
+  ["screen", "update", "scr_TEST", "--name", "Lobby", "--if-match", "1", "--dry-run"],
+  ["screen", "list", "--name", "Lobby"],
+  ["screen", "show", "scr_TEST", "extra"],
+  ["screen", "list", "--timeout", "oops"],
+  ["screen", "list", "--timeout"],
+  ["screen", "list", "--timeout="],
+  ["screen", "list", "--timeout", "-1"],
+  ["screen", "list", "--timeout", "Infinity"],
+  ["screen", "list", "--timeout", "1.5"],
+  ["screen", "list", "--json=false"],
+  ["screen", "list", "--json", "--json"],
+  ["screen", "list", "--state", "archived", "--state", "archived"],
+  ["screen", "list", "-x"],
+  ["screen", "show", "scr_TEST", "--name=secret"],
+  ["screen", "archive", "scr_TEST", "--if-match", "oops"],
+  ["media", "upload", "poster.png", "--no-transcode", "--codec", "h264"],
+  ["media", "update", "med_TEST", "--tag", "x", "--clear-tag", "--if-match", "1"],
+  ["feedback", "bug", "Title", "--body", "x", "--body-file", "x.md"],
+  ["kv", "set", "key", "--application-id", "app_TEST", "--json-value", "{}", "--file", "x"],
+  ["compose", "render", "page.json", "--target-width", "1920"],
+  ["events", "list", "--after", "a", "--cursor", "b"],
+  ["playlist", "import", "bundle", "--update", "pl_TEST"],
+];
+
+for (const [index, argv] of invalid.entries()) {
+  test(`invalid invocation ${index + 1} is rejected before accessing runtime`, async () => {
+    const runtime = new Proxy({} as CliRuntime, { get() { assert.fail("invalid invocation accessed runtime"); } });
+    await assert.rejects(async () => dispatch(parseArgv(argv), runtime), (error: unknown) => error instanceof CliError && error.problem.code === "usage_error");
+  });
+}
+
+test("supported values, aliases, empty byte payload, and help paths remain valid", () => {
+  for (const argv of [
+    ["screen", "update", "scr_TEST", "--name=Lobby=North", "--if-match", '"1"'],
+    ["screen", "screenshot", "scr_TEST", "--timeout", "0"],
+    ["events", "follow", "--timeout", "0"],
+    ["playlist", "get", "pl_TEST"],
+    ["app", "pack", "--", "--directory"],
+    ["kv", "set", "key", "--application-id", "app_TEST", "--value-base64=", "--content-type", "text/plain"],
+    ["--help"], ["--version"], ["screen"], ["comment", "show"],
+    ["screen", "assign", "--help"], ["help", "comment", "show", "screen"], ["help", "help"],
+  ]) assert.doesNotThrow(() => validateInvocation(parseArgv(argv)), JSON.stringify(argv));
+  assert.equal(parseArgv(["app", "pack", "--", "--directory"]).positionals[2], "--directory");
+});
+
+test("unknown option names and invalid values never appear in usage errors", () => {
+  for (const argv of [["screen", "list", "--private-secret=value"], ["screen", "list", "--timeout=private-secret"]]) {
+    assert.throws(() => validateInvocation(parseArgv(argv)), (error: unknown) => error instanceof CliError && !JSON.stringify(error.problem).includes("private-secret"));
+  }
+});