fix(session): refuse option-like VCS ranges in session reloads

.changeset/refuse-reload-flag-injection.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Reject session reload inputs whose VCS `range` or `ref` values look like command options. A caller reaching the session broker could otherwise inject `git` flags such as `--output=<path>` through a `/session-api` reload request and make Hunk write diff output to an arbitrary path.

src/app/session/reloadBounds.test.ts

@@ -61,6 +61,61 @@ describe("session reload filesystem bounds", () => {
     }
   });
 
+  test("rejects option-like VCS ranges and refs in daemon reloads", () => {
+    const repo = mkdtempSync(join(tmpdir(), "hunk-reload-bounds-options-"));
+
+    try {
+      const bounds = createSessionReloadBounds(
+        bootstrapFor({ kind: "vcs", staged: false, options: {} }, repo),
+        { cwd: repo },
+      );
+
+      expect(() =>
+        validateSessionReloadWithinBounds(bounds, {
+          kind: "vcs",
+          range: "--output=/tmp/hunk-poc",
+          staged: false,
+          options: {},
+        }),
+      ).toThrow("diff range that looks like a VCS option");
+      expect(() =>
+        validateSessionReloadWithinBounds(bounds, {
+          kind: "show",
+          ref: "--output=/tmp/hunk-poc",
+          options: {},
+        }),
+      ).toThrow("show ref that looks like a VCS option");
+      expect(() =>
+        validateSessionReloadWithinBounds(bounds, {
+          kind: "stash-show",
+          ref: "-R",
+          options: {},
+        }),
+      ).toThrow("stash-show ref that looks like a VCS option");
+
+      // Plain revisions and ranges still reload, including literal-dash pathspecs that
+      // bundled backends append after `--` so the VCS treats them as paths.
+      expect(() =>
+        validateSessionReloadWithinBounds(bounds, {
+          kind: "vcs",
+          range: "main..feature",
+          staged: false,
+          pathspecs: ["--help"],
+          options: {},
+        }),
+      ).not.toThrow();
+      expect(() =>
+        validateSessionReloadWithinBounds(bounds, {
+          kind: "stash-show",
+          ref: "stash@{1}",
+          options: {},
+        }),
+      ).not.toThrow();
+    } finally {
+      rmSync(repo, { force: true, recursive: true });
+    }
+  });
+
   test("rejects daemon reload source paths outside the initial repo root", () => {
     const repo = mkdtempSync(join(tmpdir(), "hunk-reload-bounds-repo-"));
     const outside = mkdtempSync(join(tmpdir(), "hunk-reload-bounds-outside-"));

src/app/session/reloadBounds.ts

@@ -22,6 +22,11 @@ import type { VcsCatalog } from "../../core/vcs/types";
  *
  * All candidate paths are realpath-normalized through existing ancestors so symlinks cannot escape
  * the roots, including paths whose final file does not exist yet.
+ *
+ * Revision-shaped fields (`vcs` `range`, `show`/`stash-show` `ref`) must name revisions, not
+ * command options: a value beginning with `-` would let a daemon caller inject flags such as
+ * `--output=<path>` into the VCS backend's spawned command. Pathspecs are exempt because every
+ * bundled backend appends them after `--`, where the VCS treats them as paths.
  */
 
 export interface SessionReloadBounds {
@@ -148,6 +153,13 @@ function assertReloadSourceWithinBounds(bounds: SessionReloadBounds, cwd: string
   return candidate;
 }
 
+/** Reject daemon-supplied revision fields that a VCS backend would parse as command options. */
+function assertReloadRevisionNotOptionLike(description: string, value: string) {
+  if (value.startsWith("-")) {
+    throw new Error(`Session reload refused ${description} that looks like a VCS option: ${value}`);
+  }
+}
+
 /** Validate common reload options that may cause filesystem reads. */
 function validateCommonReloadOptions(
   bounds: SessionReloadBounds,
@@ -199,8 +211,15 @@ export function validateSessionReloadWithinBounds(
       }
       break;
     case "vcs":
+      if (nextInput.range) {
+        assertReloadRevisionNotOptionLike("diff range", nextInput.range);
+      }
+      break;
     case "show":
     case "stash-show":
+      if (nextInput.ref) {
+        assertReloadRevisionNotOptionLike(`${nextInput.kind} ref`, nextInput.ref);
+      }
       break;
   }
 

src/extensions/default/vcs/git/commands.test.ts

@@ -4,10 +4,13 @@ import { tmpdir } from "node:os";
 import { join, resolve } from "node:path";
 import {
   buildGitDiffArgs,
+  buildGitDiffNumstatArgs,
   buildGitIgnoredDirectoryArgs,
+  buildGitShowArgs,
   buildGitStashShowArgs,
   buildGitStatusArgs,
   listGitIgnoredDirectoryRoots,
+  listGitUntrackedFiles,
   parseGitIgnoredDirectoryRoots,
   resolveGitDiffEndpoints,
   parseGitNumstat,
@@ -101,6 +104,27 @@ describe("git command helpers", () => {
     expect(buildGitDiffArgs(makeGitInput())).toContain("core.quotePath=true");
   });
 
+  test("refuses option-like revision values before spawning Git", () => {
+    expect(() => buildGitDiffArgs(makeGitInput({ range: "--output=/tmp/hunk-poc" }))).toThrow(
+      "looks like a Git option",
+    );
+    expect(() => buildGitDiffNumstatArgs(makeGitInput({ range: "-R" }))).toThrow(
+      "looks like a Git option",
+    );
+    expect(() =>
+      buildGitShowArgs({ kind: "show", ref: "--output=/tmp/hunk-poc", options: {} }),
+    ).toThrow("looks like a Git option");
+    expect(() =>
+      buildGitStashShowArgs({ kind: "stash-show", ref: "--output=/tmp/hunk-poc", options: {} }),
+    ).toThrow("looks like a Git option");
+    // Guarded revision helpers must also refuse ranges before any rev-parse spawn.
+    expect(() => listGitUntrackedFiles(makeGitInput({ range: "--output=/tmp/hunk-poc" }))).toThrow(
+      "looks like a Git option",
+    );
+
+    expect(buildGitDiffArgs(makeGitInput({ range: "main..feature" }))).toContain("main..feature");
+  });
+
   test("disables external diff tools for stash patches", () => {
     const args = buildGitStashShowArgs({
       kind: "stash-show",

src/extensions/default/vcs/git/commands.ts

@@ -56,6 +56,23 @@ export function appendGitPathspecs(args: string[], pathspecs?: string[]) {
   args.push("--", ...pathspecs);
 }
 
+/**
+ * Return one caller-supplied revision or range argument, refusing option-like values.
+ * Revisions and ranges never start with `-`, so a leading dash is injected content (for
+ * example `--output=<path>`) that Git would parse as a flag; it fails closed here instead
+ * of reaching the spawned command.
+ */
+export function requireGitRevisionArg(input: GitBackedInput, value: string) {
+  if (value.startsWith("-")) {
+    throw new HunkExtensionUserError(
+      `\`${formatGitCommandLabel(input)}\` refused revision \`${value}\` because it looks like a Git option.`,
+      { suggestions: ["Pass a plain revision or range, such as `HEAD` or `main..feature`."] },
+    );
+  }
+
+  return value;
+}
+
 // @pierre/diffs currently assumes git-style a/ and b/ prefixes when parsing patch headers.
 // Force byte-safe path quoting and canonical prefixes so user/repo git config cannot replace raw
 // non-UTF-8 bytes during stdout decoding or break the shared parser's expected side prefixes.
@@ -130,7 +147,7 @@ export function buildGitDiffArgs(
   }
 
   if (input.range) {
-    args.push(input.range);
+    args.push(requireGitRevisionArg(input, input.range));
   }
 
   if (excludedPathspecs.length > 0) {
@@ -155,7 +172,7 @@ export function buildGitDiffNumstatArgs(input: ExtensionVcsDiffInput) {
   }
 
   if (input.range) {
-    args.push(input.range);
+    args.push(requireGitRevisionArg(input, input.range));
   }
 
   appendGitPathspecs(args, input.pathspecs);
@@ -237,7 +254,7 @@ export function buildGitShowArgs(
   ];
 
   if (input.ref) {
-    args.push(input.ref);
+    args.push(requireGitRevisionArg(input, input.ref));
   }
 
   appendGitPathspecs(args, input.pathspecs);
@@ -259,7 +276,7 @@ export function buildGitStashShowArgs(
   ];
 
   if (input.ref) {
-    args.push(input.ref);
+    args.push(requireGitRevisionArg(input, input.ref));
   }
 
   return withNormalizedDiffPrefixes(withGitMovedLineColorConfig(args, colorMoved));
@@ -564,7 +581,7 @@ function isWorkingTreeGitDiffInput(
 
   const revs = runGitText({
     input,
-    args: ["rev-parse", "--revs-only", input.range],
+    args: ["rev-parse", "--revs-only", requireGitRevisionArg(input, input.range)],
     cwd,
     gitExecutable,
     preventOptionalLocks,
@@ -825,7 +842,7 @@ function resolveRangeRevisions(
 ): { positives: string[]; negatives: string[] } {
   const revs = runGitText({
     input,
-    args: ["rev-parse", "--revs-only", range],
+    args: ["rev-parse", "--revs-only", requireGitRevisionArg(input, range)],
     cwd: repoRoot ?? cwd,
     gitExecutable,
   })

test/session/cli.test.ts

@@ -1,5 +1,5 @@
 import { afterAll, afterEach, describe, expect, test } from "bun:test";
-import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
+import { existsSync, mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs";
 import { createServer } from "node:net";
 import { tmpdir } from "node:os";
 import { join } from "node:path";
@@ -491,6 +491,56 @@ sessionDescribe("session CLI integration", () => {
     }
   }, 20_000);
 
+  test("reload refuses option-like VCS ranges sent directly to the session API", async () => {
+    const port = await reserveLoopbackPort();
+    const fixture = createFixtureFiles(
+      "reload-injection",
+      ["export const visible = 1;"],
+      ["export const visible = 2;"],
+    );
+    mkdirSync(join(fixture.dir, ".git"));
+    const session = spawnHunkSession(fixture, port);
+
+    try {
+      const listed = await waitForRegisteredSessions(port);
+
+      const sessionId = listed[0]!.sessionId;
+      // Bypass the CLI parser on purpose: the raw daemon surface is the attacker-controlled
+      // path, so reproduce the injected flag exactly as a hostile /session-api caller would.
+      const sentinel = join(fixture.dir, "hunk-poc");
+      const response = await fetch(`http://127.0.0.1:${port}/session-api`, {
+        method: "POST",
+        headers: { "content-type": "application/json" },
+        body: JSON.stringify({
+          action: "reload",
+          selector: { sessionId },
+          nextInput: {
+            kind: "vcs",
+            range: `--output=${sentinel}`,
+            staged: false,
+            options: {},
+          },
+        }),
+      });
+
+      expect(response.status).toBe(400);
+      await expect(response.json()).resolves.toMatchObject({
+        error: expect.stringContaining("looks like a VCS option"),
+      });
+      expect(existsSync(sentinel)).toBe(false);
+
+      const get = runSessionCli(["get", sessionId, "--json"], port);
+      expect(get.proc.exitCode).toBe(0);
+      expect(JSON.parse(get.stdout)).toMatchObject({
+        session: {
+          files: [{ path: fixture.afterName }],
+        },
+      });
+    } finally {
+      await cleanupHunkSession(session, fixture, port);
+    }
+  }, 20_000);
+
   test("navigate works, and comment add only focuses the session when --focus is passed", async () => {
     const port = await reserveLoopbackPort();
     const fixture = createFixtureFiles(