From 8f54c17a03b3e9cb0e748d3aa38f84a5ac49b390 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Greg=20Berg=C3=A9?= <berge.greg@gmail.com>
Date: Sun, 21 Jun 2026 10:20:07 +0200
Subject: [PATCH] fix(core): prevent OS command injection via CI branch names
 (GHSA-4x45-gxvp-6283)

Attacker-controlled CI branch/ref strings were interpolated into execSync()
template literals in the git CI-environment helpers. Since execSync runs the
command through /bin/sh -c, shell metacharacters such as $() were evaluated
before git ran, letting anyone who could influence a branch name (e.g. via a
PR) execute arbitrary commands on the CI runner.

Replace the shell-interpolating execSync calls with execFileSync using
argument arrays, which spawn git directly and never invoke a shell. Covers
all four injectable sinks: gitFetch, gitMergeBase, listShas and
listParentCommits.

Adds a regression test that feeds a $() payload through
getMergeBaseCommitSha and asserts the injected command does not run.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---
 packages/core/src/ci-environment/git.test.ts | 54 +++++++++++++++++++-
 packages/core/src/ci-environment/git.ts      | 26 +++++++---
 2 files changed, 70 insertions(+), 10 deletions(-)

diff --git a/packages/core/src/ci-environment/git.test.ts b/packages/core/src/ci-environment/git.test.ts
index e3ec85c6..3ce8fc3e 100644
--- a/packages/core/src/ci-environment/git.test.ts
+++ b/packages/core/src/ci-environment/git.test.ts
@@ -1,5 +1,9 @@
-import { head, branch, getRepositoryURL } from "./git";
-import { describe, it, expect } from "vitest";
+import { execFileSync } from "node:child_process";
+import { existsSync, mkdtempSync, rmSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+import { branch, getMergeBaseCommitSha, getRepositoryURL, head } from "./git";
 
 describe("#head", () => {
   it("returns the current commit", () => {
@@ -23,3 +27,49 @@ describe.skip("#getRepositoryURL", () => {
     );
   });
 });
+
+describe("#getMergeBaseCommitSha (command injection)", () => {
+  let cwd: string;
+  let repoDir: string;
+  let markerFile: string;
+
+  beforeEach(() => {
+    cwd = process.cwd();
+    const root = mkdtempSync(join(tmpdir(), "argos-git-test-"));
+    repoDir = join(root, "repo");
+    markerFile = join(root, "pwned");
+
+    // A bare repo acts as the "origin" remote so that git fetch has a
+    // reachable target.
+    const bareDir = join(root, "origin.git");
+    execFileSync("git", ["init", "--bare", bareDir]);
+    execFileSync("git", ["init", repoDir]);
+    const git = (...args: string[]) =>
+      execFileSync("git", ["-C", repoDir, ...args]);
+    git("remote", "add", "origin", bareDir);
+
+    process.chdir(repoDir);
+  });
+
+  afterEach(() => {
+    process.chdir(cwd);
+    rmSync(join(repoDir, ".."), { recursive: true, force: true });
+  });
+
+  it("does not execute shell metacharacters in the branch name", () => {
+    // Mirrors the GHSA-4x45-gxvp-6283 PoC: a ref containing $() command
+    // substitution must be passed to git as a literal argument, never
+    // evaluated by a shell.
+    const malicious = `main$(touch ${markerFile})`;
+
+    // git will fail because the ref does not exist; that's expected. What
+    // matters is that the injected `touch` never runs.
+    try {
+      getMergeBaseCommitSha({ base: malicious, head: malicious });
+    } catch {
+      // Ignore the git failure.
+    }
+
+    expect(existsSync(markerFile)).toBe(false);
+  });
+});
diff --git a/packages/core/src/ci-environment/git.ts b/packages/core/src/ci-environment/git.ts
index 0a1da05c..70be7d67 100644
--- a/packages/core/src/ci-environment/git.ts
+++ b/packages/core/src/ci-environment/git.ts
@@ -1,4 +1,4 @@
-import { execSync } from "node:child_process";
+import { execFileSync, execSync } from "node:child_process";
 import { debug, isDebugEnabled } from "../debug";
 
 /**
@@ -64,7 +64,7 @@ export function getRepositoryURL() {
  */
 function gitMergeBase(input: { base: string; head: string }) {
   try {
-    return execSync(`git merge-base ${input.head} ${input.base}`)
+    return execFileSync("git", ["merge-base", input.head, input.base])
       .toString()
       .trim();
   } catch (error) {
@@ -85,9 +85,15 @@ function gitMergeBase(input: { base: string; head: string }) {
  * Run git fetch with a specific ref and depth.
  */
 function gitFetch(input: { ref: string; depth: number; target: string }) {
-  execSync(
-    `git fetch --force --update-head-ok --depth ${input.depth} origin ${input.ref}:${input.target}`,
-  );
+  execFileSync("git", [
+    "fetch",
+    "--force",
+    "--update-head-ok",
+    "--depth",
+    String(input.depth),
+    "origin",
+    `${input.ref}:${input.target}`,
+  ]);
 }
 
 /**
@@ -150,8 +156,12 @@ export function getMergeBaseCommitSha(input: {
 }
 
 function listShas(path: string, maxCount?: number): string[] {
-  const maxCountArg = maxCount ? `--max-count=${maxCount}` : "";
-  const raw = execSync(`git log --format="%H" ${maxCountArg} ${path}`.trim());
+  const args = ["log", "--format=%H"];
+  if (maxCount) {
+    args.push(`--max-count=${maxCount}`);
+  }
+  args.push(path);
+  const raw = execFileSync("git", args);
   const shas = raw.toString().trim().split("\n");
   return shas;
 }
@@ -159,7 +169,7 @@ function listShas(path: string, maxCount?: number): string[] {
 export function listParentCommits(input: { sha: string }): string[] | null {
   const limit = 200;
   try {
-    execSync(`git fetch --depth=${limit} origin ${input.sha}`);
+    execFileSync("git", ["fetch", `--depth=${limit}`, "origin", input.sha]);
   } catch (error) {
     if (error instanceof Error && error.message.includes("not our ref")) {
       return [];