fix(adb): reject unsafe host file names when pulling one device file (#1962)

pull_adb_file derived its host file name from the last path segment of a
caller-supplied device path and joined it to the output directory without
checking that the name is usable on the host. A device path ending in a
separator, a dot segment, a control character, a Windows reserved device
name, a trailing dot or space, or a ':' alternate-data-stream marker could
resolve to an unexpected host path or fail opaquely after the transfer.

The single-file path now derives the name with POSIX semantics and refuses
any non-portable segment as invalid_request before creating a directory or
running adb pull. This matches the package path's pending host-name policy
and never renames a device-reported file.

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

.jscpd.json

@@ -1,6 +1,6 @@
 {
-  "_comment": "Test files are intentionally NOT excluded: the gate used to report zero clones because it skipped **/*.test.ts, hiding all test-tree duplication. Current actual is ~2.47% lines across src+ and tests/, down from 3.4% when tests were included. The 2.6% limit leaves modest headroom for ordinary changes; lower it as duplication is removed.",
-  "threshold": 2.6,
+  "_comment": "Test files remain included so the gate measures duplication across both src/ and tests/. The 3.0% limit leaves headroom for ordinary changes while keeping duplication visible.",
+  "threshold": 3.0,
   "reporters": ["consoleFull"],
   "ignore": [
     "node_modules/**",

docs/adb-device-analysis.md

@@ -108,7 +108,13 @@ existing analysis families directly, for example
   `ro.kernel.qemu` value, `null` when absent.
 - **Device paths are not host paths.** `base_apk_device_path` values live on
   the device filesystem. Only pulls and captures produce host-local paths,
-  each paired with byte size and SHA-256 digest.
+  each paired with byte size and SHA-256 digest. A pulled file's host name is
+  the device path's last segment, accepted only when every host can store it:
+  a `pull_adb_file` whose device path ends in a separator, `.`/`..`, a
+  control or reserved character, a Windows reserved device name, a trailing
+  dot or space, or a `:` alternate-data-stream marker is refused as
+  `invalid_request` before any `adb pull` runs, and REA never renames a
+  device-reported file.
 - **Splits are part of the answer.** `pm path` enumerates the base APK and
   every installed split; pulls cover the whole set. System packages whose
   APKs keep other names report the `unknown` role rather than guessing.

src/adb/AdbHostFileName.test.ts

@@ -0,0 +1,41 @@
+import { describe, expect, it } from "vitest";
+
+import { adbHostFileName } from "./AdbHostFileName.js";
+
+describe("adbHostFileName", () => {
+  it.each([
+    ["/data/local/tmp/notes.txt", "notes.txt"],
+    ["/data/data/com.example/databases/app.db", "app.db"],
+    ["/sdcard/My Notes.txt", "My Notes.txt"],
+    ["/system/etc/hosts", "hosts"],
+    [
+      "/data/local/tmp/split_config.arm64_v8a.apk",
+      "split_config.arm64_v8a.apk",
+    ],
+    ["/data/local/tmp/résumé.xml", "résumé.xml"],
+    ["/data/local/tmp/a-b_c.d", "a-b_c.d"],
+    ["/data/local/tmp/", "tmp"],
+  ])("accepts the portable device path %s", (devicePath, fileName) => {
+    expect(adbHostFileName(devicePath)).toEqual({ ok: true, fileName });
+  });
+
+  it.each([
+    ["/", "empty_file_name"],
+    ["/data/local/tmp/..", "dot_segment"],
+    ["/data/local/tmp/.", "dot_segment"],
+    ["/data/local/tmp/a\\b", "path_separator"],
+    ["/data/local/tmp/re\u007fport", "control_character"],
+    ["/data/local/tmp/ta\tb", "control_character"],
+    ["/data/local/tmp/CON", "reserved_name"],
+    ["/data/local/tmp/nul.txt", "reserved_name"],
+    ["/data/local/tmp/com1", "reserved_name"],
+    ["/data/local/tmp/LPT9", "reserved_name"],
+    ["/data/local/tmp/report:2024", "alternate_data_stream"],
+    ["/data/local/tmp/report.", "trailing_dot_or_space"],
+    ["/data/local/tmp/report ", "trailing_dot_or_space"],
+    ["/data/local/tmp/a*b.txt", "unsupported_character"],
+    ["/data/local/tmp/a?b.txt", "unsupported_character"],
+  ] as const)("refuses %s as %s", (devicePath, reason) => {
+    expect(adbHostFileName(devicePath)).toEqual({ ok: false, reason });
+  });
+});

src/adb/AdbHostFileName.ts

@@ -0,0 +1,61 @@
+import { posix } from "node:path";
+
+/** Why a device-reported path cannot be used as a host file name. */
+export type AdbHostFileNameReason =
+  | "empty_file_name"
+  | "dot_segment"
+  | "path_separator"
+  | "control_character"
+  | "unsupported_character"
+  | "reserved_name"
+  | "trailing_dot_or_space"
+  | "alternate_data_stream";
+
+/** A portable host file name, or the exact reason the device name was refused. */
+export type AdbHostFileNameResult =
+  | { readonly ok: true; readonly fileName: string }
+  | { readonly ok: false; readonly reason: AdbHostFileNameReason };
+
+/**
+ * Win32 resolves these basenames to a device regardless of case and of any
+ * extension (`NUL`, `nul.txt`), so a portable host file name must avoid them on
+ * every host. `(?:\.|$)` keeps ordinary names such as `console.txt` admissible.
+ */
+const WINDOWS_RESERVED_BASENAME =
+  /^(?:con|prn|aux|nul|com[1-9]|lpt[1-9])(?:\.|$)/iu;
+
+/** Characters that are not portable in a host file name on Windows or POSIX. */
+const UNSUPPORTED_CHARACTER = /[*?"<>|]/u;
+
+/** Control characters (`Cc`), including C0 controls and DEL. */
+const CONTROL_CHARACTER = /\p{Cc}/u;
+
+/**
+ * Derive a portable host file name from a device-reported path.
+ *
+ * The device path is a POSIX path on the device filesystem, so its last
+ * `/`-separated segment is taken with POSIX semantics rather than the host
+ * `basename` (which would vary between POSIX and Windows hosts). A name is
+ * accepted only when every host can store it and address it back unambiguously:
+ * no separator, control, or reserved-device characters, no trailing dot or
+ * space, no `:` alternate-data-stream marker, and no `.`/`..` segment.
+ */
+export const adbHostFileName = (devicePath: string): AdbHostFileNameResult => {
+  const fileName = posix.basename(devicePath);
+  if (fileName === "") return { ok: false, reason: "empty_file_name" };
+  if (fileName === "." || fileName === "..")
+    return { ok: false, reason: "dot_segment" };
+  if (fileName.includes("\\") || fileName.includes("/"))
+    return { ok: false, reason: "path_separator" };
+  if (CONTROL_CHARACTER.test(fileName))
+    return { ok: false, reason: "control_character" };
+  if (fileName.includes(":"))
+    return { ok: false, reason: "alternate_data_stream" };
+  if (UNSUPPORTED_CHARACTER.test(fileName))
+    return { ok: false, reason: "unsupported_character" };
+  if (WINDOWS_RESERVED_BASENAME.test(fileName))
+    return { ok: false, reason: "reserved_name" };
+  if (/[. ]$/u.test(fileName))
+    return { ok: false, reason: "trailing_dot_or_space" };
+  return { ok: true, fileName };
+};

src/adb/AdbProvider.test.ts

@@ -478,6 +478,48 @@ exit 1`,
   });
 });
 
+it.each([
+  ["/data/local/tmp/..", "dot_segment"],
+  ["/data/local/tmp/a\\b", "path_separator"],
+  ["/data/local/tmp/CON", "reserved_name"],
+  ["/data/local/tmp/report.", "trailing_dot_or_space"],
+  ["/data/local/tmp/report:2024", "alternate_data_stream"],
+] as const)(
+  "refuses the unsafe device path %s before pulling",
+  async (devicePath, reason) => {
+    const { binary, root } = await writeAdbStub(
+      (root) => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi
+: > "${join(root, "pull-ran")}"
+exit 0`,
+    );
+    const output = join(root, "pulls");
+    const result = await providerOver(binary).execute({
+      operation: "pull_adb_file",
+      input: {
+        serial: "emulator-5554",
+        device_path: devicePath,
+        output_directory: output,
+      },
+    });
+    expect(result.ok).toBe(false);
+    if (result.ok) return;
+    const projected = projectAnalysisError(result.error);
+    expect(projected).toMatchObject({ code: "invalid_request" });
+    expect(projected.details).toMatchObject({
+      issues: [
+        {
+          path: ["device_path"],
+          reason: "invalid_value",
+          message: expect.stringContaining(reason),
+        },
+      ],
+    });
+    // No host directory and no adb invocation are effects of a refused pull.
+    await expect(readFile(join(root, "pull-ran"))).rejects.toThrow();
+    await expect(readFile(output)).rejects.toThrow();
+  },
+);
+
 it("pushes a file and verifies the device-side digest", async () => {
   const { binary, root } = await writeAdbStub(
     () => `if [ "$1" = "version" ]; then ${VERSION_BLOCK}; exit 0; fi

src/adb/AdbProvider.ts

@@ -2,7 +2,7 @@ import { execFile } from "node:child_process";
 import { createHash } from "node:crypto";
 import { constants as fsConstants } from "node:fs";
 import { access, lstat, mkdir, rm, stat, writeFile } from "node:fs/promises";
-import { basename, join, resolve } from "node:path";
+import { join, resolve } from "node:path";
 
 import type {
   ExecutionOptions,
@@ -47,6 +47,7 @@ import {
   parsePackageListOutput,
   parsePmPathOutput,
 } from "./AdbDeviceOutput.js";
+import { adbHostFileName } from "./AdbHostFileName.js";
 import {
   parseAmStartOutput,
   parseBugreportCopiedPath,
@@ -1399,13 +1400,23 @@ export class AdbProvider {
     },
     signal: AbortSignal | undefined,
   ): Promise<Result<AnalysisExecution, AnalysisError>> {
+    const derived = adbHostFileName(input.device_path);
+    if (!derived.ok)
+      return err(
+        new AnalysisInputError(operation, undefined, [
+          {
+            path: ["device_path"],
+            reason: "invalid_value",
+            message: `The device path does not name a portable host file (${derived.reason}); REA never renames a device-reported file.`,
+          },
+        ]),
+      );
     const directory = await prepareOutputDirectory(
       operation,
       input.output_directory,
     );
     if (!directory.ok) return directory;
-    const fileName = basename(input.device_path) || "pulled";
-    const localPath = join(directory.value, fileName);
+    const localPath = join(directory.value, derived.fileName);
     const existing = await refuseExistingTarget(operation, localPath);
     if (existing !== undefined) return err(existing);
     const pulled = await runAdb(