fix(core): prevent OS command injection via CI branch names (GHSA-4x45-gxvp-6283)

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", () => {
@@ -10,16 +14,62 @@
 // These tests are just examples to run locally
 // Hard to make it work reliably in CI
 
 describe.skip("#branch", () => {
   it("returns the current branch", () => {
     expect(branch()).toBe("main");
   });
 });
 
 describe.skip("#getRepositoryURL", () => {
   it("returns the repository URL", () => {
     expect(getRepositoryURL()).toBe(
       "git@github.com:argos-ci/argos-javascript.git",
     );
   });
 });
+
+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);
+  });
+});

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,16 +156,20 @@ 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;
 }
 
 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 [];