fix(cli): align local paths and SQLite selection validation (#9882)

fix(cli): batch of small fixes for #9714, #9718, #9759, #9856 and #9809

#9714: replace bare z.string().min(1) local paths in ADB CLI commands with localPathStringSchema so NUL bytes are rejected at the CLI boundary.
#9718: decode-android-resources path uses localPathStringSchema, matching apktoolInputSchemas.
#9759: inspect-sqlite-database CLI options now reject an empty column array and reject rowLimit/rowOffset/column/schemaScope=selected_table without --table via superRefine, mirroring inspectSqliteDatabaseInputSchema.
#9856 and #9809: shared snapshotSelectionOption is now localPathStringSchema.optional(), so every analyze/inspect/decompile/xrefs/trace/function/instructions/search --snapshot rejects NUL bytes.

Tests: tests/boundary/cli/overnightBatch1CliSchemas.test.ts (15 tests) proves empty/NUL input returns VALIDATION_ERROR before any provider runs and valid input is admitted.

Co-authored-by: morluto <76467478+morluto@users.noreply.github.com>

src/cli/adbCommands.ts

@@ -11,6 +11,7 @@ import {
   MAX_ADB_ACTIVITY_EXTRAS,
   type AdbOperation,
 } from "../domain/adb/adbDeviceAnalysis.js";
+import { localPathStringSchema } from "../domain/localPath.js";
 import type { Logger } from "pino";
 import { cliDecimalField } from "./options.js";
 import type { CliInstance } from "./types.js";
@@ -88,12 +89,9 @@ export const registerAdbCommands = (
       package: packageName.describe("Installed package name"),
     }),
     options: z.object({
-      "output-directory": z
-        .string()
-        .min(1)
-        .describe(
-          "Local directory for the pulled APK set; a package-named subdirectory is created inside",
-        ),
+      "output-directory": localPathStringSchema.describe(
+        "Local directory for the pulled APK set; a package-named subdirectory is created inside",
+      ),
     }),
     run: ({ args, options }) =>
       execute(CLI_COMMANDS.pullAdbPackage, "pull_adb_package", {
@@ -142,10 +140,9 @@ export const registerAdbCommands = (
       "device-path": adbInputSchemas.pull_adb_file.shape.device_path,
     }),
     options: z.object({
-      "output-directory": z
-        .string()
-        .min(1)
-        .describe("Local directory for the pulled file"),
+      "output-directory": localPathStringSchema.describe(
+        "Local directory for the pulled file",
+      ),
     }),
     run: ({ args, options }) =>
       execute(CLI_COMMANDS.pullAdbFile, "pull_adb_file", {
@@ -159,7 +156,7 @@ export const registerAdbCommands = (
       "Push one local file to the device; refuses existing targets without --overwrite",
     args: z.object({
       serial,
-      "local-path": z.string().min(1).describe("Existing local file"),
+      "local-path": localPathStringSchema.describe("Existing local file"),
       "device-path": adbInputSchemas.push_adb_file.shape.device_path,
     }),
     options: z.object({
@@ -180,10 +177,9 @@ export const registerAdbCommands = (
     description: "Capture one screen frame as a digested PNG",
     args: z.object({ serial }),
     options: z.object({
-      "output-directory": z
-        .string()
-        .min(1)
-        .describe("Local directory for screen.png"),
+      "output-directory": localPathStringSchema.describe(
+        "Local directory for screen.png",
+      ),
     }),
     run: ({ args, options }) =>
       execute(CLI_COMMANDS.captureAdbScreen, "capture_adb_screen", {
@@ -251,10 +247,9 @@ export const registerAdbCommands = (
     description: "Collect and digest one device-generated bugreport archive",
     args: z.object({ serial }),
     options: z.object({
-      "output-directory": z
-        .string()
-        .min(1)
-        .describe("Local directory for the bugreport zip"),
+      "output-directory": localPathStringSchema.describe(
+        "Local directory for the bugreport zip",
+      ),
     }),
     run: ({ args, options }) =>
       execute(CLI_COMMANDS.collectAdbBugreport, "collect_adb_bugreport", {
@@ -273,7 +268,7 @@ export const registerAdbCommands = (
       "Install one local APK; --replace requests reinstall mode to keep existing app data",
     args: z.object({
       serial,
-      "apk-path": z.string().min(1).describe("Local APK file to install"),
+      "apk-path": localPathStringSchema.describe("Local APK file to install"),
     }),
     options: z.object({
       replace: z

src/cli/apktoolCommands.ts

@@ -9,6 +9,7 @@ import {
   apktoolInputSchemas,
   type ApktoolOperation,
 } from "../domain/apktool/apktoolResourceAnalysis.js";
+import { localPathStringSchema } from "../domain/localPath.js";
 import type { Logger } from "pino";
 import { cliDecimalField } from "./options.js";
 import type { CliInstance } from "./types.js";
@@ -59,7 +60,7 @@ export const registerApktoolCommands = (
     description:
       "Decode one APK's manifest, metadata, and string resources with apktool",
     args: z.object({
-      path: z.string().min(1).describe("Local APK file to decode"),
+      path: localPathStringSchema.describe("Local APK file to decode"),
     }),
     // The locale grammar and the string-projection default belong to the
     // apktool input owner. Read them from that schema rather than restating a

src/cli/decompileDirectoryCommand.ts

@@ -9,6 +9,7 @@ import {
   cliDecimalField,
   directAnalysisOptionsFromCli,
   nativeOpenOptionFields,
+  snapshotSelectionOption,
 } from "./options.js";
 import { decompileDirectoryInputSchema } from "../contracts/decompileDirectory.js";
 import type { CliInstance } from "./types.js";
@@ -72,13 +73,9 @@ export const registerDecompileDirectoryCommand = (
       ).describe(
         "Maximum functions to attempt after skipping verified files and recorded external functions",
       ),
-      snapshot: z
-        .string()
-        .min(1)
-        .optional()
-        .describe(
-          "Load a local analysis snapshot. decompile-all does not update it.",
-        ),
+      snapshot: snapshotSelectionOption.describe(
+        "Load a local analysis snapshot. decompile-all does not update it.",
+      ),
       ...nativeOpenOptionFields,
     }),
     run: ({ args, options }) =>

src/cli/sqliteCommands.ts

@@ -10,7 +10,19 @@ import { localPathStringSchema } from "../domain/localPath.js";
 import { absoluteCallerPath } from "../filesystem/callerPath.js";
 import { finishCliEvidenceCommand } from "./cliEvidenceCallerPath.js";
 import { cliDecimalField } from "./options.js";
-import { inspectSqliteDatabaseInputSchema } from "../domain/sqlite/sqliteDatabase.js";
+import {
+  inspectSqliteDatabaseInputSchema,
+  sqliteTableSelectionSchema,
+  SQLITE_TABLE_REQUIRED_SELECTION_FIELDS,
+} from "../domain/sqlite/sqliteDatabase.js";
+
+const selectionAliases = {
+  table: "table",
+  columns: "column",
+  schema_scope: "schemaScope",
+  row_limit: "rowLimit",
+  row_offset: "rowOffset",
+} as const;
 
 /** Resolve an operator-selected SQLite snapshot through the shared inspection workflow. */
 export const registerSqliteCommands = (
@@ -25,35 +37,62 @@ export const registerSqliteCommands = (
     args: z.object({
       path: localPathStringSchema.describe("Local SQLite database snapshot"),
     }),
-    options: z.object({
-      workerTimeoutMs: cliDecimalField(
-        inspectSqliteDatabaseInputSchema.shape.worker_timeout_ms,
-      ),
-      table: z
-        .string()
-        .optional()
-        .describe("Exact ordinary table name to read"),
-      column: z
-        .array(z.string())
-        .optional()
-        .describe(
+    options: z
+      .object({
+        workerTimeoutMs: cliDecimalField(
+          inspectSqliteDatabaseInputSchema.shape.worker_timeout_ms,
+        ),
+        table: sqliteTableSelectionSchema.shape.table.describe(
+          "Exact ordinary table name to read",
+        ),
+        column: sqliteTableSelectionSchema.shape.columns.describe(
           "Repeat to select exact row columns in the requested order; requires table",
         ),
-      schemaScope: z
-        .enum(["inventory", "selected_table", "database"])
-        .optional()
-        .describe(
+        schemaScope: sqliteTableSelectionSchema.shape.schema_scope.describe(
           "Compact inventory by default, or selected-table details when table is supplied; database requests full schema",
         ),
-      rowLimit: cliDecimalField(
-        inspectSqliteDatabaseInputSchema.shape.row_limit,
-      ).describe("Positive safe integer row count; default 20; requires table"),
-      rowOffset: cliDecimalField(
-        inspectSqliteDatabaseInputSchema.shape.row_offset,
-      ).describe(
-        "Non-negative safe integer rows to skip before the window; default 0; requires table",
-      ),
-    }),
+        rowLimit: cliDecimalField(
+          inspectSqliteDatabaseInputSchema.shape.row_limit,
+        ).describe(
+          "Positive safe integer row count; default 20; requires table",
+        ),
+        rowOffset: cliDecimalField(
+          inspectSqliteDatabaseInputSchema.shape.row_offset,
+        ).describe(
+          "Non-negative safe integer rows to skip before the window; default 0; requires table",
+        ),
+      })
+      .superRefine((options, context) => {
+        const selection = sqliteTableSelectionSchema.safeParse({
+          table: options.table,
+          columns: options.column,
+          schema_scope: options.schemaScope,
+          row_limit: options.rowLimit,
+          row_offset: options.rowOffset,
+        });
+        if (selection.success) return;
+        for (const issue of selection.error.issues) {
+          const path = issue.path.map((field) =>
+            typeof field === "string" && field in selectionAliases
+              ? selectionAliases[field as keyof typeof selectionAliases]
+              : field,
+          );
+          context.addIssue({ ...issue, path });
+        }
+      })
+      .meta({
+        dependentRequired: Object.fromEntries(
+          SQLITE_TABLE_REQUIRED_SELECTION_FIELDS.map((field) => [
+            selectionAliases[field],
+            ["table"],
+          ]),
+        ),
+        if: {
+          properties: { schemaScope: { const: "selected_table" } },
+          required: ["schemaScope"],
+        },
+        then: { required: ["table"] },
+      }),
     run: ({ args, options }) =>
       withCommandCancellation((signal) =>
         logCliCommand(logger, CLI_COMMANDS.inspectSqliteDatabase, () =>

src/domain/sqlite/sqliteDatabase.ts

@@ -26,17 +26,16 @@ const artifactSchema = z.strictObject({
   bytes: nonnegative,
 });
 
-/** Inspect one frozen database and its sibling WAL, with optional ordinary-table rows. */
-export const inspectSqliteDatabaseInputSchema = z
+/** Fields that require the caller to select one ordinary table. */
+export const SQLITE_TABLE_REQUIRED_SELECTION_FIELDS = [
+  "row_limit",
+  "columns",
+  "row_offset",
+] as const;
+
+/** Shared row/schema selection constraints, independent of path transport. */
+export const sqliteTableSelectionSchema = z
   .strictObject({
-    path: absoluteLocalPathStringSchema.describe(
-      "Absolute path to an offline SQLite database snapshot",
-    ),
-    worker_timeout_ms: workerTimeout
-      .optional()
-      .describe(
-        `Deadline in milliseconds for worker startup, queries and reply writing (default ${String(DEFAULT_SQLITE_WORKER_TIMEOUT_MS)}), not snapshot acquisition or the whole call. Allow client headroom for acquisition, reply reads, cleanup and delivery. Fixed byte/heap limits do not change.`,
-      ),
     table: z
       .string()
       .optional()
@@ -71,7 +70,6 @@ export const inspectSqliteDatabaseInputSchema = z
         "Rows to skip before the retained window; default 0; requires table",
       ),
   })
-  .describe("When schema_scope is selected_table, also provide table.")
   .superRefine((input, context) => {
     if (input.row_limit !== undefined && input.table === undefined)
       context.addIssue({
@@ -99,11 +97,9 @@ export const inspectSqliteDatabaseInputSchema = z
       });
   })
   .meta({
-    dependentRequired: {
-      row_limit: ["table"],
-      columns: ["table"],
-      row_offset: ["table"],
-    },
+    dependentRequired: Object.fromEntries(
+      SQLITE_TABLE_REQUIRED_SELECTION_FIELDS.map((field) => [field, ["table"]]),
+    ),
     if: {
       properties: { schema_scope: { const: "selected_table" } },
       required: ["schema_scope"],
@@ -111,6 +107,21 @@ export const inspectSqliteDatabaseInputSchema = z
     then: { required: ["table"] },
   });
 
+/** Inspect one frozen database and its sibling WAL, with optional ordinary-table rows. */
+export const inspectSqliteDatabaseInputSchema = sqliteTableSelectionSchema
+  .safeExtend({
+    path: absoluteLocalPathStringSchema.describe(
+      "Absolute path to an offline SQLite database snapshot",
+    ),
+    worker_timeout_ms: workerTimeout
+      .optional()
+      .describe(
+        `Deadline in milliseconds for worker startup, queries and reply writing (default ${String(DEFAULT_SQLITE_WORKER_TIMEOUT_MS)}), not snapshot acquisition or the whole call. Allow client headroom for acquisition, reply reads, cleanup and delivery. Fixed byte/heap limits do not change.`,
+      ),
+  })
+  .describe("When schema_scope is selected_table, also provide table.")
+  .meta(sqliteTableSelectionSchema.meta() ?? {});
+
 /** JSON-safe SQLite cells retain integers, binary content and special REAL values. */
 export const sqliteValueSchema = z.discriminatedUnion("type", [
   z.strictObject({ type: z.literal("null") }),

tests/boundary/cli/overnightBatch1CliSchemas.test.ts

@@ -0,0 +1,298 @@
+import { join } from "node:path";
+import { Ajv2020 } from "ajv/dist/2020.js";
+import { createTestTempDirectory } from "../../fixtures/temporaryDirectory.js";
+import { Cli } from "incur";
+import { expect, it, vi } from "vitest";
+import { z } from "incur";
+import { createAnalysisExecution } from "../../../src/application/AnalysisProvider.js";
+import type { AdbDeviceAnalysisPort } from "../../../src/application/adb/AdbDeviceAnalysisPort.js";
+import type { ApktoolResourceAnalysisPort } from "../../../src/application/apktool/ApktoolResourceAnalysisPort.js";
+import { ok } from "../../../src/domain/result.js";
+import { registerAdbCommands } from "../../../src/cli/adbCommands.js";
+import { registerApktoolCommands } from "../../../src/cli/apktoolCommands.js";
+import { registerDecompileDirectoryCommand } from "../../../src/cli/decompileDirectoryCommand.js";
+import { registerCoreBinaryCommands } from "../../../src/cli/coreBinaryCommands.js";
+import { registerSqliteCommands } from "../../../src/cli/sqliteCommands.js";
+import { visitRegisteredCommands } from "../../../src/cli/visitRegisteredCommands.js";
+import { silentLogger } from "../../../src/logger.js";
+
+const registry = () => {
+  const failure = {
+    ok: false as const,
+    error: { code: "invalid_request", message: "Admitted target control" },
+  };
+  const identity = { id: "stub", name: "Stub provider", version: null };
+  const adbExecute = vi.fn(async () =>
+    ok(createAnalysisExecution({}, identity)),
+  );
+  const apktoolExecute = vi.fn(async () =>
+    ok(createAnalysisExecution({}, identity)),
+  );
+  const runDirectAnalysis = vi.fn(async () => failure);
+  const adbProvider: AdbDeviceAnalysisPort = {
+    inspectAvailability: async () => ({
+      status: "available",
+      code: null,
+      reason: null,
+      diagnostics: {},
+    }),
+    close: async () => {},
+    execute: adbExecute,
+  };
+  const apktoolProvider: ApktoolResourceAnalysisPort = {
+    inspectAvailability: async () => ({
+      status: "available",
+      code: null,
+      reason: null,
+      diagnostics: {},
+    }),
+    close: async () => {},
+    execute: apktoolExecute,
+  };
+  const cli = Cli.create("overnight-batch1-cli-schemas", { sync: false });
+  registerAdbCommands(cli, silentLogger, {}, adbProvider);
+  registerApktoolCommands(cli, silentLogger, {}, apktoolProvider);
+  registerCoreBinaryCommands(cli, silentLogger, runDirectAnalysis, {});
+  registerSqliteCommands(cli, silentLogger, {});
+  registerDecompileDirectoryCommand(cli, silentLogger, runDirectAnalysis);
+  const serve = async (argv: readonly string[]): Promise<unknown> => {
+    let stdout = "";
+    await cli.serve([...argv, "--json"], {
+      env: {},
+      stdout: (text) => {
+        stdout += text;
+      },
+      exit: () => undefined,
+    });
+    return JSON.parse(stdout);
+  };
+  return { cli, serve, adbExecute, apktoolExecute, runDirectAnalysis };
+};
+
+/**
+ * ADB local-path arguments (#9714). `before` holds the argv up to the
+ * value under test; `after` holds the argv that follows it.
+ */
+const adbLocalPathCases: readonly {
+  readonly label: string;
+  readonly field: string;
+  readonly before: readonly string[];
+  readonly after: readonly string[];
+}[] = [
+  {
+    label: "pull-adb-package --output-directory",
+    field: "output-directory",
+    before: [
+      "pull-adb-package",
+      "emulator-5554",
+      "com.example",
+      "--output-directory",
+    ],
+    after: [],
+  },
+  {
+    label: "pull-adb-file --output-directory",
+    field: "output-directory",
+    before: [
+      "pull-adb-file",
+      "emulator-5554",
+      "/data/local/tmp/drop",
+      "--output-directory",
+    ],
+    after: [],
+  },
+  {
+    label: "push-adb-file --local-path",
+    field: "local-path",
+    before: ["push-adb-file", "emulator-5554"],
+    after: ["/data/local/tmp/drop"],
+  },
+  {
+    label: "capture-adb-screen --output-directory",
+    field: "output-directory",
+    before: ["capture-adb-screen", "emulator-5554", "--output-directory"],
+    after: [],
+  },
+  {
+    label: "collect-adb-bugreport --output-directory",
+    field: "output-directory",
+    before: ["collect-adb-bugreport", "emulator-5554", "--output-directory"],
+    after: [],
+  },
+  {
+    label: "install-adb-package --apk-path",
+    field: "apk-path",
+    before: ["install-adb-package", "emulator-5554"],
+    after: [],
+  },
+];
+
+it.each(adbLocalPathCases)(
+  "rejects an empty or NUL $label before contacting adb and admits a plain path",
+  async ({ field, before, after }) => {
+    const { serve, adbExecute } = registry();
+    expect(await serve([...before, "", ...after])).toMatchObject({
+      code: "VALIDATION_ERROR",
+      fieldErrors: [
+        expect.objectContaining({ path: field, code: "too_small" }),
+      ],
+    });
+    expect(await serve([...before, "out\x00dir", ...after])).toMatchObject({
+      code: "VALIDATION_ERROR",
+      fieldErrors: [
+        expect.objectContaining({ path: field, code: "invalid_format" }),
+      ],
+    });
+    expect(adbExecute).not.toHaveBeenCalled();
+    await serve([...before, "out", ...after]);
+    expect(adbExecute).toHaveBeenCalledTimes(1);
+  },
+);
+
+it("rejects an empty or NUL decode-android-resources path before launching apktool and admits a plain path", async () => {
+  const { serve, apktoolExecute } = registry();
+  expect(await serve(["decode-android-resources", ""])).toMatchObject({
+    code: "VALIDATION_ERROR",
+    fieldErrors: [expect.objectContaining({ path: "path", code: "too_small" })],
+  });
+  expect(
+    await serve(["decode-android-resources", "app\x00.apk"]),
+  ).toMatchObject({
+    code: "VALIDATION_ERROR",
+    fieldErrors: [
+      expect.objectContaining({ path: "path", code: "invalid_format" }),
+    ],
+  });
+  expect(apktoolExecute).not.toHaveBeenCalled();
+  await serve(["decode-android-resources", "app.apk"]);
+  expect(apktoolExecute).toHaveBeenCalledTimes(1);
+});
+
+it.each([["analyze"], ["inspect"], ["decompile-all", "--output", "out"]])(
+  "rejects an empty or NUL --snapshot on %s before analysis and admits a plain path",
+  async (command, ...required) => {
+    const { serve, runDirectAnalysis } = registry();
+    expect(
+      await serve([command, ".", ...required, "--snapshot", ""]),
+    ).toMatchObject({
+      code: "VALIDATION_ERROR",
+      fieldErrors: [
+        expect.objectContaining({ path: "snapshot", code: "too_small" }),
+      ],
+    });
+    expect(
+      await serve([command, ".", ...required, "--snapshot", "snap\x00shot"]),
+    ).toMatchObject({
+      code: "VALIDATION_ERROR",
+      fieldErrors: [
+        expect.objectContaining({ path: "snapshot", code: "invalid_format" }),
+      ],
+    });
+    expect(runDirectAnalysis).not.toHaveBeenCalled();
+    await serve([command, ".", ...required, "--snapshot", "snapshot.db"]);
+    expect(runDirectAnalysis).toHaveBeenCalled();
+  },
+);
+
+it.each([
+  { argv: ["--row-limit", "5"], field: "rowLimit" },
+  { argv: ["--row-offset", "1"], field: "rowOffset" },
+  { argv: ["--column", "c"], field: "column" },
+  { argv: ["--schema-scope", "selected_table"], field: "schemaScope" },
+])(
+  "rejects inspect-sqlite-database $field without --table",
+  async ({ argv, field }) => {
+    const { serve } = registry();
+    expect(
+      await serve(["inspect-sqlite-database", "db.sqlite", ...argv]),
+    ).toMatchObject({
+      code: "VALIDATION_ERROR",
+      fieldErrors: [expect.objectContaining({ path: field, code: "custom" })],
+    });
+  },
+);
+
+it("rejects an empty column array in the inspect-sqlite-database options schema", () => {
+  const { cli } = registry();
+  let options: unknown;
+  visitRegisteredCommands(cli, (command, path) => {
+    if (path === "inspect-sqlite-database") options = command.options;
+  });
+  expect(options).toBeInstanceOf(z.ZodObject);
+  if (!(options instanceof z.ZodObject)) throw new Error("unreachable");
+  const parsed = options.safeParse({ column: [] });
+  expect(parsed.success).toBe(false);
+  if (parsed.success) return;
+  expect(parsed.error.issues).toEqual(
+    expect.arrayContaining([
+      expect.objectContaining({ path: ["column"], code: "too_small" }),
+    ]),
+  );
+});
+
+it("admits --table with --column and --row-limit to runtime validation", async () => {
+  const { serve } = registry();
+  const missing = join(
+    await createTestTempDirectory("rea-sqlite-admission-"),
+    "missing.db",
+  );
+  // The SQLite worker only runs from the built CLI, so this boundary test
+  // proves admission with a missing snapshot: CLI validation passes and the
+  // runtime refuses the absent file instead of the option combination. The
+  // end-to-end read is covered by tests/acceptance/sqliteDatabaseCli.test.ts.
+  expect(
+    await serve([
+      "inspect-sqlite-database",
+      missing,
+      "--table",
+      "t",
+      "--column",
+      "c",
+      "--row-limit",
+      "5",
+    ]),
+  ).toMatchObject({
+    code: "invalid_request",
+    details: {
+      operation: "inspect_sqlite_database",
+      issues: [
+        expect.objectContaining({
+          path: ["path"],
+          message: expect.stringContaining(
+            "Selected SQLite database does not exist",
+          ),
+        }),
+      ],
+    },
+  });
+});
+
+it("advertises SQLite selection dependencies in CLI discovery", async () => {
+  const { serve } = registry();
+  const schema = await serve(["inspect-sqlite-database", "--schema"]);
+  const { options } = z
+    .object({ options: z.record(z.string(), z.unknown()) })
+    .parse(schema);
+  const validate = new Ajv2020({ strict: false }).compile(options);
+  expect(validate({ column: ["id"] })).toBe(false);
+  expect(validate({ schemaScope: "selected_table" })).toBe(false);
+  expect(validate({ table: "items", column: [] })).toBe(false);
+  expect(
+    validate({ table: "items", column: ["id"], schemaScope: "selected_table" }),
+  ).toBe(true);
+  expect(schema).toMatchObject({
+    options: {
+      dependentRequired: {
+        rowLimit: ["table"],
+        rowOffset: ["table"],
+        column: ["table"],
+      },
+      if: {
+        properties: { schemaScope: { const: "selected_table" } },
+        required: ["schemaScope"],
+      },
+      then: { required: ["table"] },
+      properties: { column: { minItems: 1 } },
+    },
+  });
+});