fix(cli): close shell injection via crafted staged filenames (security, #87)

CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to `@fusengine/harness`. Format: [Keep a Changelog](https://
 
 ## [Unreleased]
 
+## [0.1.89] - 2026-08-03
+
+### Security
+
+- **Arbitrary command execution via a crafted staged filename in `harness check`** (`src/cli/run.ts`) — `stagedContent()` built a shell command string via `execSync(\`git show ":${path}"\`)`, with `path` taken directly from the literal name of a staged file, fully controlled by whoever authored the commit. Git's `core.quotepath` escapes an embedded `"` or `\`, but not backticks or `$(...)`, so a filename like `` x`touch pwned.txt`.ts `` ran as shell syntax. Anyone running `harness check` as a pre-commit hook (the README's own documented usage) against an untrusted staged file list — e.g. right after `git add .` on a checked-out contributor branch — got arbitrary command execution. Both `stagedContent()` and `stagedFiles()` now use `execFileSync` with an argv array; the filename reaches `git show` as a single literal argument and is never parsed by a shell. Reported in [#87](https://github.com/fusengine/harness/issues/87) by @VikramNehreTR, with reproduction, root-cause analysis, and the exact fix adopted here. A non-regression test exercises the real `stagedContent()` against an isolated repo with a backtick-laden filename, proven discriminating against the vulnerable form. All 9 execution sites in `src/` were audited for the same shape; this was the only exploitable one.
+
 ## [0.1.88] - 2026-08-03
 
 ### Added

package.json

@@ -1,6 +1,6 @@
 {
 	"name": "@fusengine/harness",
-	"version": "0.1.88",
+	"version": "0.1.89",
 	"description": "Harness-agnostic toolkit for AI coding agents: runtime harness detection (Claude Code, Codex, Cursor, Cline, Gemini, Aider...), pure policy core (env config, project/framework detection, SOLID/file-size limits, APEX freshness, guard patterns, portable prompts), cache, project memory, ref routing, state/locks, statusline, per-harness adapters (Claude/Cursor/Cline/Gemini) and a cli-mode harness-check binary. Bun-native, with a built dist for Node + bundlers.",
 	"type": "module",
 	"module": "src/index.ts",

src/cli/run.ts

@@ -1,17 +1,24 @@
-import { execSync } from "node:child_process";
+import { execFileSync } from "node:child_process";
 import { evaluate } from "../policy/evaluate";
 import { formatPrompt } from "../prompt/types";
 import { isCodeFile } from "../util/project-root";
 
 /** Staged files (Added/Copied/Modified/Renamed). Uses `node:child_process` (Bun shell can hang on `git show`). */
 export function stagedFiles(): string[] {
-  const out = execSync("git diff --cached --name-only --diff-filter=ACMR", { encoding: "utf8" });
+  const out = execFileSync("git", ["diff", "--cached", "--name-only", "--diff-filter=ACMR"], {
+    encoding: "utf8",
+  });
   return out.trim().split("\n").filter(Boolean);
 }
 
-/** Read a file's staged (index) content — not the working-tree version. */
+/**
+ * Read a file's staged (index) content — not the working-tree version.
+ * Uses `execFileSync` with an argv array (never a shell), so a staged
+ * filename containing shell metacharacters (backticks, `$(...)`, quotes)
+ * reaches `git show` as a single literal argument — see issue #87.
+ */
 export function stagedContent(path: string): string {
-  return execSync(`git show ":${path}"`, { encoding: "utf8" });
+  return execFileSync("git", ["show", `:${path}`], { encoding: "utf8" });
 }
 
 /**

test/staged-content-shell-injection.test.ts

@@ -0,0 +1,51 @@
+import { test, expect } from "bun:test";
+import { execFileSync } from "node:child_process";
+import { mkdtempSync, rmSync, writeFileSync, existsSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { stagedContent } from "../src/cli/run";
+
+/**
+ * Regression test for the shell-injection reported in issue #87: a staged
+ * filename containing a command substitution (`` `...` `` / `$(...)`) must
+ * NOT be interpreted by a shell when `stagedContent` reads its staged blob
+ * via `git show`. Exercises the real entry point (`stagedContent`) against
+ * an isolated temporary git repo — never the real repo — and cleans up in
+ * `finally` regardless of outcome.
+ */
+test("stagedContent: a filename with shell metacharacters is never executed by a shell", () => {
+  const repo = mkdtempSync(join(tmpdir(), "harness-issue-87-"));
+  const marker = join(repo, "pwned.txt");
+  try {
+    execFileSync("git", ["init", "-q"], { cwd: repo });
+    execFileSync("git", ["config", "user.email", "test@example.com"], { cwd: repo });
+    execFileSync("git", ["config", "user.name", "test"], { cwd: repo });
+
+    // Filename carries a command substitution that would create `marker` if
+    // ever handed to a shell (`sh -c "git show \":${path}\""`). Kept relative
+    // (no `/`) since a filename component cannot embed a path separator —
+    // `stagedContent` runs with `cwd` pinned to `repo` (see below).
+    const evilName = "x`touch pwned.txt`.ts";
+    writeFileSync(join(repo, evilName), "content\n");
+    execFileSync("git", ["add", "--", evilName], { cwd: repo });
+
+    const content = stagedContentAt(repo, evilName);
+
+    expect(content).toBe("content\n");
+    expect(existsSync(marker)).toBe(false);
+  } finally {
+    rmSync(repo, { recursive: true, force: true });
+    rmSync(marker, { force: true });
+  }
+});
+
+/** Runs the real `stagedContent` implementation with `cwd` pinned to `repo`. */
+function stagedContentAt(repo: string, path: string): string {
+  const cwd = process.cwd();
+  process.chdir(repo);
+  try {
+    return stagedContent(path);
+  } finally {
+    process.chdir(cwd);
+  }
+}