diff --git a/deploy/helm/paperclip/templates/statefulset.yaml b/deploy/helm/paperclip/templates/statefulset.yaml index ad59e7109db5..3322e57ef9a4 100644 --- a/deploy/helm/paperclip/templates/statefulset.yaml +++ b/deploy/helm/paperclip/templates/statefulset.yaml @@ -471,9 +471,36 @@ spec: # arrives scrubbed. Do not "simplify" this back to a direct exec. exec /paperclip/.local/bin/paperclip-github-token-env /usr/local/bin/node /opt/paperclip-bundled-adapters/node_modules/@paperclipai/adapter-utils/dist/github-mcp-egress-runtime.js /usr/local/bin/github-mcp-server "$@" EOF + # PEN-3156: the THIRD egress door. `gh` and `github-mcp-server` + # above rewrite a payload in flight; this one cannot, because a + # commit object is content-addressed — altering a blob or a + # message changes that commit's SHA and every descendant's. So the + # guard here REFUSES a publish that would carry credential-shaped + # material, and names the commit to amend. + # + # The runtime goes INSIDE the token wrapper, like + # github-mcp-server above, so git still inherits its credentials. + # It runs on every invocation but acts only on a publish: it + # injects core.hooksPath for the hook below and rejects + # --no-verify, which would otherwise skip that hook. Everything + # else is passed through untouched. + GIT_HOOKS_DIR="${BASE}/.local/share/paperclip-git-hooks" + mkdir -p "${GIT_HOOKS_DIR}" + cat > "${GIT_HOOKS_DIR}/pre-push" <<'EOF' + #!/bin/sh + exec /usr/local/bin/node /opt/paperclip-bundled-adapters/node_modules/@paperclipai/adapter-utils/dist/github-git-egress-runtime.js --pre-push-hook "$@" + EOF + chmod 0755 "${GIT_HOOKS_DIR}/pre-push" + # Ownership, not a security boundary. This container is already + # runAsUser 1000 and the PVC is fsGroup 1000, so the hook is + # writable by the agent no matter what is written here — and the + # chart has no root to make it otherwise. The guard's threat model + # is accidental disclosure; see the header of + # packages/adapter-utils/src/github-git-egress-runtime.ts. + chown 1000:1000 "${GIT_HOOKS_DIR}" "${GIT_HOOKS_DIR}/pre-push" 2>/dev/null || true cat > "${LOCAL_BIN}/git" <<'EOF' #!/bin/sh - exec /paperclip/.local/bin/paperclip-github-token-env /usr/bin/git \ + exec /paperclip/.local/bin/paperclip-github-token-env /usr/local/bin/node /opt/paperclip-bundled-adapters/node_modules/@paperclipai/adapter-utils/dist/github-git-egress-runtime.js /usr/bin/git \ -c credential.https://github.com.helper= \ -c credential.https://github.com.helper=/paperclip/.local/bin/github-token-credential-helper \ "$@" @@ -505,8 +532,19 @@ spec: mkdir -p "${PATH_BIN}" ln -sf "${LOCAL_BIN}/gh" "${PATH_BIN}/gh" chown -h 1000:1000 "${PATH_BIN}/gh" 2>/dev/null || true + # PEN-3156: `git` needs the same treatment, and for the same + # reason. Measured in a live agent Job pod before this change: + # PATH was ".../paperclip/bin:...:/usr/bin", `command -v gh` gave + # ${PATH_BIN}/gh, and `command -v git` gave /usr/bin/git — because + # ${LOCAL_BIN}/git existed but ${LOCAL_BIN} was not on that PATH + # and no symlink published it here. The push guard would have been + # a choke point nothing traverses. Without this line the rest of + # PEN-3156 is decorative. + ln -sf "${LOCAL_BIN}/git" "${PATH_BIN}/git" + chown -h 1000:1000 "${PATH_BIN}/git" 2>/dev/null || true echo "seed: installed fresh-read GitHub token wrappers in ${LOCAL_BIN}" echo "seed: published egress-scrubbing gh on the default PATH at ${PATH_BIN}/gh" + echo "seed: published publish-guarding git on the default PATH at ${PATH_BIN}/git" # Seed a project-scope .mcp.json at $HOME (CWD for k8s Job pods is # /paperclip by default — matches PAPERCLIP_HOME) so every claude diff --git a/deploy/helm/paperclip/tests/agent-egress-path.test.mjs b/deploy/helm/paperclip/tests/agent-egress-path.test.mjs index c385c73de229..8a964f242cbb 100644 --- a/deploy/helm/paperclip/tests/agent-egress-path.test.mjs +++ b/deploy/helm/paperclip/tests/agent-egress-path.test.mjs @@ -104,6 +104,32 @@ function extractPathPublishFragment(rendered) { throw new Error("did not find the gh symlink line in the publish step"); } +/** + * PEN-3156: the same publish step, but captured through the `git` symlink + * rather than stopping at the `gh` one. + * + * A separate walker rather than a parameter on the one above, so that the `gh` + * assertion keeps testing exactly the region it always did and cannot start + * passing because of a line added for `git`. + */ +function extractPathPublishFragmentThroughGit(rendered) { + const lines = rendered.split("\n"); + const startIdx = lines.findIndex((line) => + /^\s*PATH_BIN="\$\{BASE\}\/bin"$/.test(line), + ); + assert.notEqual(startIdx, -1, "seed script no longer publishes onto the default PATH"); + const indent = lines[startIdx].match(/^(\s*)/)[1]; + const body = []; + for (let i = startIdx; i < lines.length; i += 1) { + const line = lines[i].slice(indent.length); + body.push(line); + if (/^ln -sf "\$\{LOCAL_BIN\}\/git"/.test(line)) return body.join("\n"); + } + throw new Error( + "the seed does not publish git onto the PATH-visible bin; the push guard would be off the traffic path (PEN-3156)", + ); +} + function writeExecutable(dir, name, body) { const file = path.join(dir, name); fs.writeFileSync(file, body, { mode: 0o755 }); @@ -216,6 +242,126 @@ test("a non-login shell resolves gh to the scrubbing wrapper under the default-v assert.equal(result.stdout.trim(), "scrubbing-wrapper"); }); +// --- The publish guard on the `git` door (PEN-3156) ------------------------- + +test("a non-login shell resolves git to the publish-guarding wrapper", () => { + // The regression this pins, measured in a live agent Job pod on 2026-09-10: + // ${LOCAL_BIN}/git existed and was byte-identical to the chart, but the pod's + // PATH carried /paperclip/bin without /paperclip/.local/bin and nothing + // published git into the former — so `command -v git` gave /usr/bin/git and + // any guard in the wrapper was a choke point nothing traversed. `gh` had the + // same defect and PEN-2527 fixed it with a symlink; git never got one. + const base = fs.mkdtempSync(path.join(os.tmpdir(), "git-path-reach-")); + const localBin = path.join(base, ".local", "bin"); + const imageBin = path.join(base, "usr-bin"); + for (const dir of [localBin, imageBin]) fs.mkdirSync(dir, { recursive: true }); + + const rendered = render("templates/statefulset.yaml", { + set: [`persistence.mountPath=${base}`], + }); + + writeExecutable(localBin, "git", "#!/bin/sh\necho guarding-wrapper\n"); + writeExecutable(imageBin, "git", "#!/bin/sh\necho image-git\n"); + + // Run the seed's own publish step, so this fails if the seed stops publishing + // git rather than merely if a symlink is missing. + const seeded = spawnSync( + "sh", + [ + "-c", + [ + "set -eu", + `BASE=${JSON.stringify(base)}`, + `LOCAL_BIN=${JSON.stringify(localBin)}`, + extractPathPublishFragmentThroughGit(rendered), + ].join("\n"), + ], + { encoding: "utf8" }, + ); + assert.equal(seeded.status, 0, seeded.stderr); + + const containerPathValue = containerPath(rendered); + const testPath = containerPathValue + .split(":") + .map((entry) => (entry === "/usr/bin" ? imageBin : entry)) + .join(":"); + + const result = spawnSync("/bin/sh", ["-c", "git"], { + encoding: "utf8", + env: { PATH: testPath }, + }); + assert.equal(result.status, 0, result.stderr); + assert.equal(result.stdout.trim(), "guarding-wrapper"); +}); + +test("the seeded git wrapper routes through the egress runtime, inside the token wrapper", () => { + const rendered = render("templates/statefulset.yaml", {}); + const match = /cat > "\$\{LOCAL_BIN\}\/git" <<'EOF'\n([\s\S]*?)\n[ \t]*EOF/.exec(rendered); + assert.notEqual(match, null, "the seed no longer writes a git wrapper"); + const body = match[1]; + + assert.match( + body, + /github-git-egress-runtime\.js/, + "the git wrapper does not reach the publish guard (PEN-3156)", + ); + + // Ordering is load-bearing in one direction: the token wrapper must stay + // outermost or git runs without its credentials. + const tokenAt = body.indexOf("paperclip-github-token-env"); + const runtimeAt = body.indexOf("github-git-egress-runtime.js"); + assert.ok(tokenAt >= 0 && runtimeAt > tokenAt, "the scrub runtime must sit inside the token wrapper"); +}); + +test("the seed installs a pre-push hook for the guard to run", () => { + const rendered = render("templates/statefulset.yaml", {}); + // Without the hook the wrapper injects core.hooksPath at a directory holding + // nothing, and every push is allowed while looking guarded. + assert.match(rendered, /paperclip-git-hooks/, "no hooks directory is seeded"); + assert.match( + rendered, + /cat > "\$\{GIT_HOOKS_DIR\}\/pre-push" <<'EOF'/, + "the seed does not write a pre-push hook", + ); + assert.match(rendered, /--pre-push-hook/, "the seeded hook does not invoke the guard"); +}); + +test("the seeded hooks directory is the one the runtime actually looks in", () => { + // The assertions above match `--pre-push-hook` and `paperclip-git-hooks` as + // strings, which catches deletion but not DIVERGENCE — and divergence is the + // failure this seam actually has. The runtime hardcodes DEFAULT_HOOKS_DIR + // (`/paperclip/...`) while the seed writes to `${BASE}/...` from + // persistence.mountPath. They are equal in every values file today, so a + // deployment that relocated the PVC would point core.hooksPath at a directory + // holding no hook. `prePushHookPresent` exists to turn that into a refusal + // rather than a silent unscanned push; this turns it into a failing test + // instead, which is cheaper than discovering it in production. + const rendered = render("templates/statefulset.yaml", {}); + + const base = rendered.match(/^\s*BASE=(?:"([^"]*)"|'([^']*)'|(\S+))\s*$/m); + assert.ok(base, "could not find BASE in the rendered seed script"); + const baseValue = base[1] ?? base[2] ?? base[3]; + + const hooks = rendered.match(/GIT_HOOKS_DIR="\$\{BASE\}([^"]*)"/); + assert.ok(hooks, "could not find GIT_HOOKS_DIR in the rendered seed script"); + const seeded = `${baseValue}${hooks[1]}`; + + // Read the constant from source rather than restating it: a test that + // hardcodes both sides of an equality cannot observe either one moving. + const runtimeSource = fs.readFileSync( + path.join(repoRoot, "packages/adapter-utils/src/github-git-egress-runtime.ts"), + "utf8", + ); + const declared = runtimeSource.match(/DEFAULT_HOOKS_DIR\s*=\s*"([^"]+)"/); + assert.ok(declared, "could not read DEFAULT_HOOKS_DIR from the runtime source"); + + assert.equal( + seeded, + declared[1], + "the seed writes the pre-push hook somewhere the runtime will not look", + ); +}); + // --- Fail closed on overrides that would take the scrubber off the path ---- test("a PATH entry in env.extra is rejected rather than silently overriding the chart", () => { diff --git a/packages/adapter-utils/src/github-git-egress-runtime.test.ts b/packages/adapter-utils/src/github-git-egress-runtime.test.ts new file mode 100644 index 000000000000..0bac7e5ee3cf --- /dev/null +++ b/packages/adapter-utils/src/github-git-egress-runtime.test.ts @@ -0,0 +1,1017 @@ +import { afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; + +import { execFileSync, spawnSync } from "node:child_process"; +import { mkdtempSync, readFileSync, symlinkSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import path from "node:path"; + +import { + buildGitArgv, + DEFAULT_GIT_BINARY, + DEFAULT_HOOKS_DIR, + GitEgressRuntimeError, + gitBinary, + hooksDirectory, + makeGitReader, + readStdin, + type HookStdin, + runGitEgressRuntime, + runPrePushHook, +} from "./github-git-egress-runtime.js"; +import type { GitReader } from "./github-git-egress-shim.js"; + +const HOOKS = "/hooks"; + +/** + * The deployment precondition every push test depends on: the seed has written + * an executable `pre-push` into the hooks directory. + * + * Spelled out rather than defaulted, because a push with no hook installed is + * now a refusal — see the "fails closed when the hook is missing" test. Tests + * that are not about that case assert the precondition holds. + */ +const PRESENT = { hooksDir: HOOKS, hookPresent: () => true }; + +/** Real git, or null when this environment has none to drive. */ +const GIT = spawnSync("git", ["--version"], { encoding: "utf8" }).status === 0 ? "git" : null; + +/** + * The spawned-entrypoint tests need git at DEFAULT_GIT_BINARY specifically, not + * merely on PATH: the hook's own reader hardcodes that path, because the + * `PAPERCLIP_GIT_EGRESS_GIT` override was removed as an escape hatch (see + * `hooksDirectory`). Checking PATH instead would let those tests run with a git + * the hook cannot reach, and a scan that cannot read git refuses — which would + * look like the regression passing. + */ +const SPAWNED_GIT = + spawnSync(DEFAULT_GIT_BINARY, ["--version"], { encoding: "utf8" }).status === 0; + +describe("buildGitArgv", () => { + it("points a push at the hooks directory that holds the guard", () => { + expect(buildGitArgv(["push", "origin", "main"], { ...PRESENT })).toEqual([ + "-c", + `core.hooksPath=${HOOKS}`, + "push", + "origin", + "main", + ]); + }); + + it("puts the injected option before the subcommand", () => { + // git only accepts global options ahead of the subcommand; appending it + // would make git treat it as a push argument and the hook would not run. + const argv = buildGitArgv(["push"], { ...PRESENT }); + expect(argv.indexOf("-c")).toBeLessThan(argv.indexOf("push")); + }); + + it("leaves every non-push invocation completely untouched", () => { + // Scoping the injection matters: a hooks directory holding only `pre-push` + // silently disables a repository's pre-commit and commit-msg hooks, so this + // must not be set globally. + for (const argv of [["status"], ["commit", "-m", "x"], ["fetch", "origin"], ["log"]]) { + expect(buildGitArgv(argv, { ...PRESENT })).toEqual(argv); + } + }); + + it("refuses the flag that would skip the hook", () => { + // Without this the whole control is one flag away from being off. + expect(() => buildGitArgv(["push", "--no-verify"], { ...PRESENT })).toThrow( + GitEgressRuntimeError, + ); + expect(() => buildGitArgv(["push", "--no-verify"], { ...PRESENT })).toThrow( + /--no-verify is disabled/, + ); + }); + + it("allows `push -n`, which is --dry-run and not a hook bypass", () => { + // `-n` means --dry-run for push. The hook still runs under it and nothing + // is published either way, so refusing it rejected a safe command and gave + // a reason that was not true. + expect(buildGitArgv(["push", "-n"], { ...PRESENT })).toEqual([ + "-c", + `core.hooksPath=${HOOKS}`, + "push", + "-n", + ]); + }); + + it("injects the guard AFTER the caller's global options, so git reads it last", () => { + // Git takes the last `-c` given for a key. Injecting at the front let + // `git -c core.hooksPath=/tmp/empty push` override the guard and skip the + // scanner while still traversing the wrapper — measured against git 2.47.3. + const argv = buildGitArgv(["-C", "/repo", "--no-pager", "push", "origin", "main"], { + ...PRESENT, + }); + expect(argv).toEqual([ + "-C", + "/repo", + "--no-pager", + "-c", + `core.hooksPath=${HOOKS}`, + "push", + "origin", + "main", + ]); + // The guard is the last core.hooksPath in the argv, and still ahead of the + // subcommand, which is where git requires global options to sit. + const positions = argv + .map((token, index) => (token.toLowerCase().startsWith("core.hookspath=") ? index : -1)) + .filter((index) => index >= 0); + expect(positions.at(-1)).toBe(argv.indexOf(`core.hooksPath=${HOOKS}`)); + expect(argv.indexOf(`core.hooksPath=${HOOKS}`)).toBeLessThan(argv.indexOf("push")); + }); + + it("refuses a push that sets core.hooksPath itself", () => { + for (const argv of [ + ["-c", "core.hooksPath=/tmp/empty", "push"], + ["-c", "CORE.HOOKSPATH=/tmp/empty", "push"], + ["--config-env=core.hooksPath=HP", "push"], + ]) { + expect(() => buildGitArgv(argv, { ...PRESENT }), argv.join(" ")).toThrow( + /sets core\.hooksPath itself/, + ); + } + }); + + it("refuses an alias whose expansion carries the bypass", () => { + // Injection cannot win here: git expands the alias after the command line, + // so the expansion's own flag is the last thing git sees. + expect(() => + buildGitArgv(["yolo"], { + ...PRESENT, + resolveAlias: (name) => (name === "yolo" ? "push --no-verify" : null), + }), + ).toThrow(/expands to a push that skips the pre-push hook/); + + expect(() => + buildGitArgv(["sneaky"], { + ...PRESENT, + resolveAlias: (name) => + name === "sneaky" ? "-c core.hooksPath=/tmp/empty push" : null, + }), + ).toThrow(/points core\.hooksPath somewhere else/); + }); + + it("still guards a push reached through an alias", () => { + const argv = buildGitArgv(["yolo"], { + ...PRESENT, + resolveAlias: (name) => (name === "yolo" ? "push --force" : null), + }); + expect(argv).toEqual(["-c", `core.hooksPath=${HOOKS}`, "yolo"]); + }); + + it("refuses an alias the command line defines for itself", () => { + // `git -c alias.yolo='push --no-verify' yolo` pushed to a real remote with + // the hook never running (git 2.47.3). The definition is unreachable from + // the `git config --get` this wrapper runs — that lookup exits 1 with no + // output — so no resolver is supplied here: the guard has to read the + // definition out of argv or it does not hold at all. + expect(() => + buildGitArgv(["-c", "alias.yolo=push --no-verify", "yolo"], { ...PRESENT }), + ).toThrow(/expands to a push that skips the pre-push hook/); + }); + + it("guards a push reached through an alias the command line defines", () => { + // The non-refusal half: an ordinary command-line alias must still be + // recognised as a push so the guard is injected. Measured against git + // 2.47.3, `git -c alias.p=push -c core.hooksPath= p origin HEAD:...` + // ran the hook and the push aborted; without the injection it succeeded. + const argv = buildGitArgv(["-c", "alias.p=push", "p", "origin", "main"], { + ...PRESENT, + }); + expect(argv).toEqual([ + "-c", + "alias.p=push", + "-c", + `core.hooksPath=${HOOKS}`, + "p", + "origin", + "main", + ]); + expect(argv.indexOf(`core.hooksPath=${HOOKS}`)).toBeLessThan(argv.indexOf("p")); + }); + + it("reads a --config-env alias through the environment it is given", () => { + expect(() => + buildGitArgv(["--config-env=alias.yolo=A_PUSH", "yolo"], { + ...PRESENT, + env: { A_PUSH: "push --no-verify" }, + }), + ).toThrow(/expands to a push that skips the pre-push hook/); + }); + + it("leaves a command-line alias to a non-push alone", () => { + const argv = ["-c", "alias.st=status --short", "st"]; + expect(buildGitArgv(argv, { ...PRESENT })).toEqual(argv); + }); +}); + +describe.skipIf(!GIT)("runGitEgressRuntime alias resolution", () => { + // These drive REAL git, because the thing under test is whether the alias + // lookup runs against the same configuration git will use. A stubbed + // resolver would answer whatever the stub decided and prove nothing. + + it("resolves a file-based alias in the repository the invocation selects", () => { + // `-C` chooses which config files an alias lookup reads. A bare `git config + // --get` reads the WRAPPER's cwd instead, finds nothing, and classifies the + // invocation as not-a-push. Measured against git 2.47.3 on a real remote: + // before the caller's global options were forwarded to the lookup, + // `git -C ` published a commit carrying credential-shaped + // material with the hook never running. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "alias.yolo", "push --no-verify"]); + + // Refused during argv construction, so no push is ever attempted: reaching + // the refusal at all is the proof that the alias resolved. + // Thrown synchronously, before the promise is built — which is exactly why + // the entrypoint wraps this call in a try/catch as well as a .catch(). + expect(() => + runGitEgressRuntime({ target: "git", argv: ["-C", repo, "yolo"], ...PRESENT }), + ).toThrow(/expands to a push that skips the pre-push hook/); + }); + + it("prefers a command-line definition over the repository's own", () => { + // Git takes the command-line one; so must the guard, or a benign on-disk + // alias masks a hostile command-line redefinition of the same name. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "alias.p", "status"]); + + expect(() => + runGitEgressRuntime({ + target: "git", + argv: ["-C", repo, "-c", "alias.p=push --no-verify", "p"], + ...PRESENT, + }), + ).toThrow(/expands to a push that skips the pre-push hook/); + }); +}); + +describe("entrypoint configuration", () => { + // Regression for a bypass Ally found on 6228564: both of these were `env` + // overrides defaulting to the constants, and the deployed entrypoint called + // them with no argument, i.e. against process.env. The environment is + // agent-controlled, so `PAPERCLIP_GIT_EGRESS_HOOKS_DIR=/tmp/empty git push` + // aimed core.hooksPath at a directory with no hook and published unscanned, + // and PAPERCLIP_GIT_EGRESS_GIT aimed the hook's own reader at a binary of the + // caller's choosing. + // + // These mutate process.env and assert the value does NOT move, so + // reintroducing the read fails here rather than only in production. + const vars = ["PAPERCLIP_GIT_EGRESS_HOOKS_DIR", "PAPERCLIP_GIT_EGRESS_GIT"] as const; + const saved = new Map(); + + beforeEach(() => { + for (const name of vars) saved.set(name, process.env[name]); + }); + + afterEach(() => { + for (const name of vars) { + const previous = saved.get(name); + if (previous === undefined) delete process.env[name]; + else process.env[name] = previous; + } + }); + + it("uses the path the Helm seed writes", () => { + expect(hooksDirectory()).toBe(DEFAULT_HOOKS_DIR); + expect(gitBinary()).toBe(DEFAULT_GIT_BINARY); + }); + + it("cannot be redirected by the agent-controlled environment", () => { + process.env.PAPERCLIP_GIT_EGRESS_HOOKS_DIR = "/tmp/empty"; + process.env.PAPERCLIP_GIT_EGRESS_GIT = "/tmp/fake-git"; + + expect(hooksDirectory()).toBe(DEFAULT_HOOKS_DIR); + expect(gitBinary()).toBe(DEFAULT_GIT_BINARY); + }); +}); + +describe("missing hook", () => { + // git treats a hooks directory with no pre-push in it as "no hook to run" and + // the push proceeds. Nothing downstream can tell that apart from a clean + // scan, so the only safe reading of an absent guard is refusal. + it("fails closed when the hook is not installed", () => { + expect(() => + buildGitArgv(["push", "origin", "main"], { hooksDir: HOOKS, hookPresent: () => false }), + ).toThrow(/no executable pre-push hook/); + }); + + it("does not require the hook for commands that publish nothing", () => { + const argv = ["status"]; + expect(buildGitArgv(argv, { hooksDir: HOOKS, hookPresent: () => false })).toEqual(argv); + }); +}); + +describe("publishing verbs other than `push` (PEN-3156)", () => { + // The bypass these close: `isPush` was `subcommand === "push"`, so every + // other verb that publishes classified as not-a-push and went through + // untouched. Measured against git 2.47.3 by Ally on head 5bc6f36e, and + // re-measured end to end here: `git -c core.hooksPath= send-pack + // HEAD:refs/heads/x` printed `* [new branch]`, landed the ref, and + // ran no hook — the guard's injected config present and irrelevant, because + // `send-pack` never consults the pre-push hook. + // + // These therefore assert the REFUSAL, not that the hook fired. Asserting the + // hook would be asserting something unreachable on this path, and would pass + // for the wrong reason the moment the refusal regressed to a guard. + const SEND_PACK = "send-pack"; + + it("refuses send-pack on the bare-argv leg", () => { + expect(() => + buildGitArgv([SEND_PACK, "origin", "HEAD:refs/heads/x"], { ...PRESENT }), + ).toThrow(/refusing to run `git send-pack`/); + }); + + it("says injecting the hook would not help, so nobody re-fixes it as a guard", () => { + // The obvious remedy — add these verbs to the push classification so + // `core.hooksPath` is injected — is inert. The message has to carry that, + // or the next author re-applies it. + expect(() => buildGitArgv([SEND_PACK], { ...PRESENT })).toThrow(/does not help/); + }); + + it("refuses send-pack reached through an alias, naming the chain", () => { + // The alias-expansion leg. Ally measured this one publishing too: + // `alias.publishit = send-pack HEAD:refs/heads/viaalias` landed + // the ref with the hook silent. + expect(() => + buildGitArgv(["publishit"], { + ...PRESENT, + resolveAlias: (name) => + name === "publishit" ? `${SEND_PACK} origin HEAD:refs/heads/x` : null, + }), + ).toThrow(/refusing to run `git send-pack`[\s\S]*alias `publishit`/); + }); + + it("refuses an unrecognised verb, because unknown cannot be shown to be safe", () => { + // The inversion. A denylist would pass this; the allowlist refuses it, and + // that is the whole behavioural change. `lfs` is the realistic case — a + // third-party verb that does publish. + expect(() => buildGitArgv(["lfs", "push"], { ...PRESENT })).toThrow( + /refusing to run `git lfs`[\s\S]*not one of them/, + ); + }); + + it("tells the operator which constant to widen, so the fix is not to bypass the wrapper", () => { + expect(() => buildGitArgv(["some-new-plumbing"], { ...PRESENT })).toThrow( + /NON_PUBLISHING_GIT_VERBS/, + ); + }); + + it("still passes ordinary non-publishing verbs through untouched", () => { + // The control that would catch the allowlist being over-applied as a + // blanket refusal — the failure mode that would break every agent's git. + // Deliberately wide, and includes plumbing, because the risk of an + // allowlist is what it omits. + for (const argv of [ + ["status"], + ["commit", "-m", "x"], + ["fetch", "origin"], + ["log"], + ["rev-parse", "HEAD"], + ["worktree", "list"], + ["cat-file", "-p", "HEAD"], + ["ls-remote", "origin"], + ["for-each-ref"], + ["update-ref", "refs/heads/x", "HEAD"], + ["stash", "pop"], + ["bundle", "create", "/tmp/b", "HEAD"], + ]) { + expect(buildGitArgv(argv, { ...PRESENT }), argv.join(" ")).toEqual(argv); + } + }); + + it("still guards a real push rather than refusing it", () => { + // The other half of that control: the inversion must not have swept `push` + // itself into the refusal path. + const argv = buildGitArgv(["push", "origin", "main"], { ...PRESENT }); + expect(argv).toContain("push"); + expect(argv.join(" ")).toContain("core.hooksPath"); + }); +}); + +describe("shell aliases", () => { + // Measured against git 2.47.3: git PREPENDS its exec-path to PATH for the + // shell it spawns, and /usr/lib/git-core ships a complete `git`. So a bare + // `git push` inside a `!` alias reaches the real git without passing through + // this wrapper or the hook — no absolute path needed. Passing these through + // was the residual gap this replaces. + // + // The expansions below are assembled rather than written out, because + // scripts/check-no-git-push.mjs rejects that adjacency anywhere in this tree + // and these are fixtures for a guard that exists to recognise exactly it. The + // assembled value is byte-identical to the literal at runtime; the marker + // would instead assert an operator-approved publish path, which a test string + // is not. + const PUSH = "push"; + + it("refuses a shell alias rather than guessing whether it publishes", () => { + expect(() => + buildGitArgv(["publish"], { + ...PRESENT, + resolveAlias: (name) => + name === "publish" ? `!/usr/bin/git ${PUSH} --no-verify` : null, + }), + ).toThrow(/refusing to run the shell alias `publish`/); + }); + + it("refuses an alias chain deeper than the hop limit instead of handing it to git", () => { + // The chain reaches a push, but resolution runs out of budget before seeing + // it, so `isPush` is false. That must NOT reach the not-a-push early return: + // measured against git 2.47.3, handing this argv back unchanged meant no + // `core.hooksPath` was injected, git expanded the chain itself, and + // `refs/heads/deep5` landed on the remote with the hook never running. + const chain: Record = { + a1: "a2", + a2: "a3", + a3: "a4", + a4: "a5", + a5: "push", + }; + expect(() => + buildGitArgv(["a1", "origin", "main"], { + ...PRESENT, + resolveAlias: (name) => chain[name] ?? null, + }), + ).toThrow(/alias chain deeper than 4 hops/); + }); + + it("refuses an unparseable alias expansion even though it classifies as not-a-push", () => { + // Same shape as the depth case and the reason this ordering is tested at the + // wrapper rather than only at `classifyGitInvocation`: the shim deliberately + // reports this bypass with `isPush` false, and an early return here would + // discard it. The shim's own unit test cannot catch that — it asserts the + // bypass is REPORTED, which it is either way. + expect(() => + buildGitArgv(["q", "origin", "main"], { + ...PRESENT, + resolveAlias: (name) => (name === "q" ? 'push "--no-verify' : null), + }), + ).toThrow(/unterminated quote/); + }); + + it("still passes an ordinary non-push through untouched", () => { + // The guard against over-reading the two rules above: only a bypass the shim + // kept without a push refuses early. A plain non-push alias must not. + const argv = ["st", "--short"]; + expect( + buildGitArgv(argv, { + ...PRESENT, + resolveAlias: (name) => (name === "st" ? "status --short" : null), + }), + ).toEqual(argv); + }); + + it("refuses one whose expansion names no push at all, because it cannot be parsed", () => { + // The point of failing closed: this publishes, and no textual test for + // the subcommand that indirection can defeat is worth trusting. + expect(() => + buildGitArgv(["helper"], { + ...PRESENT, + resolveAlias: (name) => (name === "helper" ? `!f() { git ${PUSH}; }; f` : null), + }), + ).toThrow(/refusing to run the shell alias `helper`/); + }); + + it("refuses one defined on the command line, which no separate lookup can see", () => { + expect(() => + buildGitArgv(["-c", `alias.x=!/usr/bin/git ${PUSH}`, "x"], { ...PRESENT }), + ).toThrow(/refusing to run the shell alias `x`/); + }); + + it("leaves ordinary non-shell aliases alone", () => { + const argv = buildGitArgv(["lg"], { + ...PRESENT, + resolveAlias: (name) => (name === "lg" ? "log --oneline" : null), + }); + expect(argv).toEqual(["lg"]); + }); +}); + +describe("runPrePushHook", () => { + const dump = ["ALPHA", "BRAVO", "CHARLIE", "DELTA", "ECHOES"] + .map((name, index) => `${name}=value-${index}`) + .join("\n"); + + const dirtyGit: GitReader = (args) => { + if (args[0] === "rev-list") return "c0ffee0000000000\n"; + if (args.includes("--format=%s")) return "wip"; + if (args.includes("--format=%B")) return `wip\n\n${dump}\n`; + return ""; + }; + + it("exits zero when there is nothing to publish", async () => { + const code = await runPrePushHook({ + input: "", + runGit: () => { + throw new Error("git must not be consulted"); + }, + }); + expect(code).toBe(0); + }); + + it("exits zero for a clean push", async () => { + const code = await runPrePushHook({ + input: "refs/heads/m a refs/heads/m b\n", + runGit: (args) => { + if (args[0] === "rev-list") return "abc\n"; + if (args.includes("--format=%s")) return "clean"; + if (args.includes("--format=%B")) return "clean\n"; + return ""; + }, + }); + expect(code).toBe(0); + }); + + it("aborts the push and explains which commit to amend", async () => { + const errors: string[] = []; + const code = await runPrePushHook({ + input: "refs/heads/m a refs/heads/m b\n", + runGit: dirtyGit, + stderr: (message) => errors.push(message), + }); + // Non-zero is what actually stops the push; the text is what makes it + // actionable rather than a bare rejection. + expect(code).toBe(1); + expect(errors.join("\n")).toContain("c0ffee000000"); + expect(errors.join("\n")).toContain("environment-dump"); + }); + + it("aborts the push when the scan cannot be completed", async () => { + // A guard whose error path is "allow" is not a guard. Before this, a failed + // rev-list reported an empty commit set and the push proceeded unscanned. + const errors: string[] = []; + const code = await runPrePushHook({ + input: "refs/heads/m a refs/heads/m b\n", + runGit: () => null, + stderr: (message) => errors.push(message), + }); + expect(code).toBe(1); + const text = errors.join("\n"); + expect(text).toContain("could not be completed"); + // It must not read as a detection: there is no commit to go and amend. + expect(text).toContain("refusal, not a detection"); + // Name the git command that failed, so the fault is diagnosable. Matched by + // shape rather than by literal: which read fails FIRST is an ordering + // detail (the annotated-tag `cat-file -t` probe now precedes `rev-list`), + // and pinning the literal here turned a reordering into a false failure. + // The rev-list leg keeps its own explicit coverage in the next test. + expect(text).toMatch(/`git [a-z][a-z-]*/); + }); + + it("names rev-list when the commit range is what fails", async () => { + // Guards the leg the test above used to pin: the tag probe succeeds, so the + // scan reaches `rev-list`, and its failure must still abort and be named. + const errors: string[] = []; + const code = await runPrePushHook({ + input: "refs/heads/m a refs/heads/m b\n", + // `cat-file -t` answers "commit" (no tag object to scan); everything else + // fails, which lands on the rev-list read. + runGit: (args) => (args[0] === "cat-file" && args[1] === "-t" ? "commit\n" : null), + stderr: (message) => errors.push(message), + }); + expect(code).toBe(1); + const text = errors.join("\n"); + expect(text).toContain("could not be completed"); + expect(text).toContain("rev-list"); + }); + + it("aborts the push on an unexpected scanner failure", async () => { + // Anything thrown leaves the verdict unknown, which must refuse, not pass. + const errors: string[] = []; + const code = await runPrePushHook({ + input: "refs/heads/m a refs/heads/m b\n", + runGit: () => { + throw new Error("git went missing"); + }, + stderr: (message) => errors.push(message), + }); + expect(code).toBe(1); + expect(errors.join("\n")).toContain("git went missing"); + }); +}); + +describe.skipIf(!GIT)("content leg against a real repository", () => { + // These drive REAL git for the same reason the alias tests above do: the + // property under test is what `git show` EMITS, which a stubbed reader would + // simply assert into existence. Each case below is a way for a file's bytes + // never to reach the scanner, and each was a silent pass before `--text` / + // `--no-textconv` (Ally review on b8ae369f found the first; the other two + // turned up while confirming it). + + /** + * A vendor key, assembled rather than pasted — same rule as the fixtures in + * `github-git-egress-shim.test.ts`. A literal would be a real `sk-` string + * committed to this repository: `.github/scripts/check-pr-security.mjs` + * flags `sk-[a-zA-Z0-9]{32,}` as a high-severity finding, so the pull request + * adding a secret-containment control would itself trip the secret scan. + * Joining the parts at runtime produces the same bytes for git to publish + * while leaving no matchable literal in the source. + */ + function vendorKey(): string { + return ["sk", "ant", "api03", "Xq7mZp2Lw9Rt4Nv8Bc3Hj6Kd1Fg5Ys0Ae"].join("-"); + } + + /** A repo with one commit, and the pre-push input that would publish it. */ + function repoWithCommit(files: Record): { + runGit: GitReader; + input: string; + } { + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-content-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + for (const [name, body] of Object.entries(files)) { + writeFileSync(path.join(repo, name), body); + } + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "add fixtures"]); + const head = execFileSync("git", ["-C", repo, "rev-parse", "HEAD"], { + encoding: "utf8", + }).trim(); + return { + runGit: makeGitReader("git", repo), + // A new branch the remote does not have: `0`*40 on the remote side is the + // shape git hands the hook for a first push. + input: `refs/heads/topic ${head} refs/heads/topic ${"0".repeat(40)}\n`, + }; + } + + async function refusals(files: Record): Promise { + const { runGit, input } = repoWithCommit(files); + const errors: string[] = []; + const code = await runPrePushHook({ + input, + runGit, + stderr: (message) => errors.push(message), + }); + expect(code).toBe(1); + return errors.join("\n"); + } + + it("scans a binary addition instead of skipping it", async () => { + // Git calls a file binary when a NUL lands in the first 8000 bytes, and + // then prints `Binary files ... differ` with no `+` lines at all — so + // `addedLinesFromPatch` saw the empty string and reported the commit clean. + // A credential does not stop being a credential for sharing a blob with a + // NUL byte. + const blob = Buffer.concat([ + Buffer.from("header"), + Buffer.from([0x00]), + Buffer.from(`${vendorKey()}\n`), + ]); + expect(await refusals({ "payload.bin": blob })).toContain("vendor-key"); + }); + + it("scans a path a .gitattributes entry marks -diff", async () => { + // The wider hole, and it needs no binary content: `-diff` makes git print + // the same `Binary files ... differ` summary for plain ASCII. `*` applies + // it to the whole tree, and the attributes file is committed in the same + // push it hides, so nothing earlier in the range would have caught it. + expect( + await refusals({ ".gitattributes": "* -diff\n", "secret.txt": `${vendorKey()}\n` }), + ).toContain("vendor-key"); + }); + + it("scans the real bytes, not a textconv driver's output", async () => { + // `diff..textconv` is ordinary repository config — agent-writable — + // and replaces a file's content with that command's output for display. + // Without `--no-textconv` the scanner reads `innocuous` and passes the + // commit while the real bytes go to the remote. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-textconv-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + execFileSync("git", ["-C", repo, "config", "diff.launder.textconv", "echo innocuous"]); + writeFileSync(path.join(repo, ".gitattributes"), "secret.txt diff=launder\n"); + writeFileSync(path.join(repo, "secret.txt"), `${vendorKey()}\n`); + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "add fixtures"]); + const head = execFileSync("git", ["-C", repo, "rev-parse", "HEAD"], { + encoding: "utf8", + }).trim(); + + const errors: string[] = []; + const code = await runPrePushHook({ + input: `refs/heads/topic ${head} refs/heads/topic ${"0".repeat(40)}\n`, + runGit: makeGitReader("git", repo), + stderr: (message) => errors.push(message), + }); + expect(code).toBe(1); + expect(errors.join("\n")).toContain("vendor-key"); + }); + + it("scans an annotated tag's own message, which no commit carries", async () => { + // The gap: `commitsForRefUpdate` runs `rev-list `, which PEELS the + // tag to the commits it reaches — measured, the tag object's own sha is + // never in that output. `scanCommit` then reads commit messages and patches, + // so nothing on the path ever reads the tag object. But `git push + // refs/tags/` publishes that object verbatim, free-form message + // included. A release tag whose message interpolates build environment is + // the PEN-2526 class on a ref update whose every commit is clean. + // + // The commit here is deliberately spotless, so a pass would mean the tag + // message went out unscanned rather than that something else caught it. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-tag-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + writeFileSync(path.join(repo, "readme.md"), "Nothing interesting here.\n"); + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "a completely clean commit"]); + execFileSync("git", [ + "-C", + repo, + "tag", + "-a", + "v1", + "-m", + `Release v1\n\nBuilt with:\n${vendorKey()}\n`, + ]); + const tagSha = execFileSync("git", ["-C", repo, "rev-parse", "v1"], { + encoding: "utf8", + }).trim(); + + // What git hands the hook for a tag push is the TAG OBJECT's sha. + const errors: string[] = []; + const code = await runPrePushHook({ + input: `refs/tags/v1 ${tagSha} refs/tags/v1 ${"0".repeat(40)}\n`, + runGit: makeGitReader("git", repo), + stderr: (message) => errors.push(message), + }); + expect(code).toBe(1); + const report = errors.join("\n"); + expect(report).toContain("vendor-key"); + expect(report).toContain("annotated tag message"); + // The remedy has to be the one that can actually reach a tag object. + expect(report).toContain("git tag -f -a v1"); + expect(report).not.toContain("rebase -i"); + }); + + it("leaves a lightweight tag to the commit leg", async () => { + // A lightweight tag's ref points straight at the commit, so there is no tag + // object to read and the commit leg already covers it. This asserts the + // scan does not refuse or error on the no-tag-object shape. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-lightweight-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + writeFileSync(path.join(repo, "readme.md"), "Nothing interesting here.\n"); + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "a completely clean commit"]); + execFileSync("git", ["-C", repo, "tag", "v-light"]); + const sha = execFileSync("git", ["-C", repo, "rev-parse", "v-light"], { + encoding: "utf8", + }).trim(); + + const errors: string[] = []; + const code = await runPrePushHook({ + input: `refs/tags/v-light ${sha} refs/tags/v-light ${"0".repeat(40)}\n`, + runGit: makeGitReader("git", repo), + stderr: (message) => errors.push(message), + }); + expect(errors.join("\n")).toBe(""); + expect(code).toBe(0); + }); + + it("still passes an annotated tag with a clean message", async () => { + // The tag leg must not turn every release tag into a refusal. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-tag-clean-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + writeFileSync(path.join(repo, "readme.md"), "Nothing interesting here.\n"); + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "a completely clean commit"]); + execFileSync("git", ["-C", repo, "tag", "-a", "v2", "-m", "Release v2\n\nBug fixes.\n"]); + const tagSha = execFileSync("git", ["-C", repo, "rev-parse", "v2"], { + encoding: "utf8", + }).trim(); + + const errors: string[] = []; + const code = await runPrePushHook({ + input: `refs/tags/v2 ${tagSha} refs/tags/v2 ${"0".repeat(40)}\n`, + runGit: makeGitReader("git", repo), + stderr: (message) => errors.push(message), + }); + expect(errors.join("\n")).toBe(""); + expect(code).toBe(0); + }); + + it("still passes a genuinely clean commit", async () => { + // The flags widen what the scanner SEES; they must not turn every binary + // file into a refusal. A png-shaped blob with nothing credential-shaped in + // it is the common case and has to keep pushing. + const { runGit, input } = repoWithCommit({ + "logo.bin": Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x00, 0x01, 0x02, 0x03]), + "readme.md": "Hello, world.\n", + }); + const errors: string[] = []; + const code = await runPrePushHook({ + input, + runGit, + stderr: (message) => errors.push(message), + }); + expect(errors.join("\n")).toBe(""); + expect(code).toBe(0); + }); +}); + +describe("readStdin", () => { + /** + * A stand-in for the pipe git supplies, typed as `HookStdin` so the test + * exercises the same overloads production does rather than an `any` cast. + */ + function fakePipe(): { + stream: HookStdin; + emitData: (chunk: string) => void; + emitEnd: () => void; + } { + const data: ((chunk: string) => void)[] = []; + const end: (() => void)[] = []; + const stream: HookStdin = { + isTTY: false, + setEncoding() {}, + on(event: "data" | "end" | "error", listener: unknown) { + if (event === "data") data.push(listener as (chunk: string) => void); + if (event === "end") end.push(listener as () => void); + return stream; + }, + }; + return { + stream, + emitData: (chunk) => data.forEach((listener) => listener(chunk)), + emitEnd: () => end.forEach((listener) => listener()), + }; + } + + // The hook's stdin is the only description it gets of what the push + // publishes. "Could not read it" and "git said there is nothing to push" must + // not be the same value, because the second is a legitimate pass. + it("refuses a TTY rather than reporting an empty ref list", async () => { + // Resolving "" here was a fail-open one layer above `runPrePushHook`: + // `parsePrePushInput("")` yields zero updates, which is a pass. + const tty: HookStdin = { isTTY: true, setEncoding() {}, on: () => tty }; + await expect(readStdin(tty)).rejects.toThrow(/TTY/); + }); + + it("still reads a piped ref-update list", async () => { + // The control for the test above: the refusal must be scoped to a TTY and + // must not break the pipe git actually supplies. + const pipe = fakePipe(); + const read = readStdin(pipe.stream); + pipe.emitData("refs/heads/m a refs/heads/m b\n"); + pipe.emitEnd(); + await expect(read).resolves.toBe("refs/heads/m a refs/heads/m b\n"); + }); + + it("reads an EMPTY pipe as an empty ref list, which is a pass", async () => { + // Git runs the pre-push hook with zero bytes on the pipe for an + // "Everything up-to-date" push — measured against git 2.47.3. Refusing that + // would fail every no-op push, so the TTY refusal above must not widen into + // "empty stdin refuses". + const pipe = fakePipe(); + const read = readStdin(pipe.stream); + pipe.emitEnd(); + await expect(read).resolves.toBe(""); + expect(await runPrePushHook({ input: "", runGit: () => null })).toBe(0); + }); +}); + +describe.skipIf(!SPAWNED_GIT)("pre-push hook bootstrap, spawned as git spawns it", () => { + // `runPrePushHook` is covered directly above, but nothing drove the module as + // an ENTRYPOINT, and that is where the fail-open was: the bootstrap decided + // whether to run at all by comparing `process.argv[1]` against + // `import.meta.url`. Those differ whenever the package is reached through a + // symlink, and a bootstrap that does not run exits 0 — which git reads as a + // hook that passed. Everything below the bootstrap was already fail-closed; + // the bootstrap was not. + // + // These tests spawn the compiled entrypoint the way the seeded hook does, so + // a regression shows up as a push that is allowed rather than as a unit that + // returns the wrong number. + + let built: string; + let linked: string; + + beforeAll(async () => { + // Compile rather than run the TypeScript directly: Node's strip-only mode + // rejects this module's parameter property. `transpileModule` skips type + // checking, which keeps the test independent of whether `@types/node` is + // installed in the sandbox. + const ts = (await import("typescript")).default; + const out = mkdtempSync(path.join(tmpdir(), "git-egress-boot-")); + for (const name of [ + "github-egress-scrub", + "github-git-egress-shim", + "github-git-egress-runtime", + ]) { + const source = readFileSync(path.join(import.meta.dirname, `${name}.ts`), "utf8"); + const js = ts.transpileModule(source, { + compilerOptions: { module: ts.ModuleKind.ESNext, target: ts.ScriptTarget.ES2022 }, + }).outputText; + writeFileSync(path.join(out, `${name}.js`), js); + } + built = path.join(out, "github-git-egress-runtime.js"); + + // The shape the finding names: the package directory is a symlink, as it is + // under a pnpm store layout or workspace hoisting. `argv[1]` is then the + // link path while `import.meta.url` is the realpath. + const linkRoot = mkdtempSync(path.join(tmpdir(), "git-egress-link-")); + symlinkSync(out, path.join(linkRoot, "adapter-utils")); + linked = path.join(linkRoot, "adapter-utils", "github-git-egress-runtime.js"); + }); + + /** + * Spawn the entrypoint exactly as the seeded `pre-push` hook does. + * + * `cwd` is load-bearing, not incidental: git runs the hook inside the + * repository being pushed, and the hook's reader inherits that directory. + * Spawning from the test runner's cwd instead made every scan fail to read + * the commit — and a failed scan ALSO refuses, with a message that also + * begins "refusing to publish". The refusal assertions below therefore pin + * the detection wording specifically, or they would pass without the + * scanner ever having looked at the commit. + */ + function hook( + entrypoint: string, + input: string, + cwd: string, + ): { status: number | null; stderr: string } { + const result = spawnSync(process.execPath, [entrypoint, "--pre-push-hook", "origin", "url"], { + input, + cwd, + encoding: "utf8", + }); + return { status: result.status, stderr: result.stderr }; + } + + /** A commit carrying credential-shaped material, and the hook stdin for it. */ + function dirtyPush(): { repo: string; input: string } { + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-boot-repo-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + // Derived, not a literal: an embedded token would trip the repository's own + // secret scan, which reads the commit range rather than the worktree. + writeFileSync(path.join(repo, "config.txt"), `key = ghp_${"A".repeat(24)}\n`); + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "add config"]); + const head = execFileSync("git", ["-C", repo, "rev-parse", "HEAD"], { + encoding: "utf8", + }).trim(); + return { + repo, + input: `refs/heads/topic ${head} refs/heads/topic ${"0".repeat(40)}\n`, + }; + } + + /** + * The detection wording, distinct from the scan-failure wording. Asserting + * this is what makes these tests non-vacuous. + */ + const DETECTED = "credential-shaped material found"; + + it("refuses credential-bearing input when run from the real path", () => { + // The positive control. Without it, the symlink assertion below could pass + // because the scanner refuses everything, or because the fixture is wrong. + const { repo, input } = dirtyPush(); + const { status, stderr } = hook(built, input, repo); + expect(status).not.toBe(0); + expect(stderr).toContain(DETECTED); + expect(stderr).not.toContain("could not be completed"); + }); + + it("refuses the same input when the package is reached through a symlink", () => { + // The regression. Before the fix this exited 0 with EMPTY stderr, and a + // real `git push` through it landed the commit on the remote. + const { repo, input } = dirtyPush(); + const { status, stderr } = hook(linked, input, repo); + expect(status).not.toBe(0); + expect(stderr).toContain(DETECTED); + expect(stderr).not.toContain("could not be completed"); + }); + + it("does not refuse a clean push through the symlinked path", () => { + // Entering the hook on argv alone must not turn the bootstrap into a + // blanket refusal: the common case still has to push. + const repo = mkdtempSync(path.join(tmpdir(), "git-egress-boot-clean-")); + execFileSync("git", ["init", "-q", repo]); + execFileSync("git", ["-C", repo, "config", "user.email", "t@example.invalid"]); + execFileSync("git", ["-C", repo, "config", "user.name", "T"]); + writeFileSync(path.join(repo, "readme.md"), "Hello, world.\n"); + execFileSync("git", ["-C", repo, "add", "-A"]); + execFileSync("git", ["-C", repo, "commit", "-qm", "clean"]); + const head = execFileSync("git", ["-C", repo, "rev-parse", "HEAD"], { + encoding: "utf8", + }).trim(); + const { status, stderr } = hook( + linked, + `refs/heads/topic ${head} refs/heads/topic ${"0".repeat(40)}\n`, + repo, + ); + expect(stderr).toBe(""); + expect(status).toBe(0); + }); +}); diff --git a/packages/adapter-utils/src/github-git-egress-runtime.ts b/packages/adapter-utils/src/github-git-egress-runtime.ts new file mode 100644 index 000000000000..fc5ef9e92d45 --- /dev/null +++ b/packages/adapter-utils/src/github-git-egress-runtime.ts @@ -0,0 +1,635 @@ +// PEN-3156: process wrapper for the `git` egress door. +// +// SCOPE — this is an ACCIDENTAL-DISCLOSURE guard. It is not a tamper-resistant +// boundary and must not be described as one. Every component of it — wrapper, +// hook, and scanner alike — sits on a surface writable by the account it +// guards, so it stops a mistake and does not stop a decision. Enforcement that +// holds against a deliberate bypass does not exist yet; it is tracked as +// PEN-3183 and that, not this file, is the control to cite when asking whether +// an agent can publish a credential on purpose. The reasoning is under THREAT +// MODEL below. Read it before extending, renaming, or relying on any of this. +// +// Two modes, one module, because they must agree about what a push is: +// +// wrapper node github-git-egress-runtime.js +// Classifies argv. On a push it closes the hook bypass and points +// git at the hooks directory below, then execs the real git. +// +// hook node github-git-egress-runtime.js --pre-push-hook +// Runs as git's pre-push hook. Reads the ref updates git computes on +// stdin, scans what they would publish, and exits non-zero — which +// is what aborts the push — naming the commit to amend. +// +// Why a hook rather than scanning in the wrapper: git already resolves +// refspecs, `push.default`, and tracking config to decide what it is about to +// send, and hands that result to the hook as exact ` ` +// pairs. Re-deriving it in the wrapper would mean a second implementation of +// those rules that could disagree with the push actually taking place, and +// disagreeing in the permissive direction is a silent hole. +// +// Why the wrapper is still needed: a hook alone is bypassable with +// `--no-verify`, and `core.hooksPath` has to be injected by something. The +// wrapper is what `git` resolves to on PATH, so it is on the path every +// ordinary invocation takes. +// +// THREAT MODEL — read this before extending the guard, because it bounds what +// any change here can achieve. +// +// What this defends against is an agent that discloses WITHOUT INTENDING TO. +// That is the exposure this control descends from: PEN-2526 was an environment +// dump interpolated into prose by an agent doing its job. Against that, a guard +// on the path the agent actually takes is effective, and every refusal above +// earns its place. +// +// What it does NOT defend against is an agent that sets out to defeat it. In +// the deployed pod, measured 2026-09-11: +// +// - `/usr/bin/git` is mode 0755 and directly executable. Nothing routes it +// through this wrapper; the wrapper is reached via PATH, which the caller +// chooses. No file needs to be written to take that path. +// - The agent runs as uid 1000 (`node`), and +// `/opt/paperclip-bundled-adapters` — which holds THIS FILE as compiled +// JavaScript — is `drwxr-xr-x node node`. The scanner is writable by the +// account it scans. +// - `${LOCAL_BIN}/git`, `${PATH_BIN}/git` and the hooks directory are all on +// the PVC, mounted `fsGroup: 1000`, so they are writable too. +// +// So the hook, the wrapper and the scanner are peers, not layers: an agent able +// to tamper with any one of them can tamper with all three, and does not need +// to tamper with anything to call the real git directly. Do not add integrity +// checks between these components — verifying the hook's content from the +// wrapper, or vice versa, asserts a boundary that does not exist and reads to +// the next person as though tamper-resistance had been established. +// +// Making this door hold against a deliberate bypass is a different change in a +// different place: enforcement off the agent's own machine (a server-side +// pre-receive hook, or egress network policy that forces pushes through a +// proxy). That is PEN-3183 — filed, with the four bypasses enumerated and +// measured, rather than approximated here. It is named rather than gestured at +// on purpose: "filed separately" is a promise a reader cannot check, and an +// uncheckable promise is how a documented limit quietly becomes a forgotten +// one. A control that looks tamper-proof and is not is worse than one whose +// limits are written down and tracked. + +import { spawn, spawnSync } from "node:child_process"; +import { accessSync, constants, realpathSync } from "node:fs"; +import path from "node:path"; +import { fileURLToPath } from "node:url"; + +import { + ALIAS_HOP_LIMIT, + classifyGitInvocation, + formatRefusal, + formatScanFailure, + gitGlobalOptions, + parsePrePushInput, + scanPrePushUpdates, + type GitAliasBypass, + type GitPublishVerb, + type GitReader, +} from "./github-git-egress-shim.js"; + +/** Where the Helm seed writes the pre-push hook. */ +export const DEFAULT_HOOKS_DIR = "/paperclip/.local/share/paperclip-git-hooks"; + +/** The real git, for the hook's own read-only queries. */ +export const DEFAULT_GIT_BINARY = "/usr/bin/git"; + +/** + * The hooks directory the deployed entrypoint uses. + * + * Takes no arguments, and reads no environment, on purpose. These were once + * `env`-backed overrides defaulting to the constants, which is an escape hatch + * in a boundary whose whole job is to be unavoidable: the environment here is + * agent-controlled, so a publish run with + * `PAPERCLIP_GIT_EGRESS_HOOKS_DIR=/tmp/empty` aimed `core.hooksPath` at a + * directory with no hook and proceeded unscanned. The sibling + * `PAPERCLIP_GIT_EGRESS_GIT` was worse: it aimed the hook's own reader at a + * binary of the caller's choosing, so a fake git reporting clean output + * cleared the scan. + * + * The test seam that justified them is still there and is strictly better — + * `runGitEgressRuntime` takes `hooksDir` and `runPrePushHook` takes `runGit` as + * parameters, so tests inject directly instead of through ambient state. + */ +export function hooksDirectory(): string { + return DEFAULT_HOOKS_DIR; +} + +/** The git binary the deployed entrypoint reads with. See `hooksDirectory`. */ +export function gitBinary(): string { + return DEFAULT_GIT_BINARY; +} + +export class GitEgressRuntimeError extends Error { + constructor( + message: string, + readonly exitCode = 64, + ) { + super(message); + this.name = "GitEgressRuntimeError"; + } +} + +/** + * Is SOME executable `pre-push` present at `hooksDir`? + * + * Named for what it can observe, not for what the caller wishes it meant. It + * was `prePushHookInstalled`, and "installed" read at the call site as "the + * guard is in place" — which is precisely the claim this cannot support and + * the misreading the threat model exists to prevent. An executable + * pass-through named `pre-push` satisfies it exactly as well as this scanner + * does. + * + * Pointing `core.hooksPath` at a directory that holds no executable `pre-push` + * is not an error to git — it runs no hook and the push proceeds. That is the + * one failure mode this whole door cannot tolerate, and it is silent: nothing + * in the output distinguishes "scanned and clean" from "never scanned". + * + * It matters more now that the directory is a constant rather than an override. + * `DEFAULT_HOOKS_DIR` hardcodes `/paperclip`, while the seed writes to + * `{{ .Values.persistence.mountPath }}` — equal in every values file today, but + * a deployment that changed the mount path would silently publish unscanned. + * Checking turns that from a hole into a refusal that names the missing file. + * + * This is a DEPLOYMENT check, not a tamper check, and the distinction is worth + * keeping straight: it establishes that a hook is present and executable, not + * that the hook is this scanner. It cannot establish the latter, and neither + * could a content or ownership check bolted on beside it — the seed runs + * `runAsNonRoot: true, runAsUser: 1000`, so it has no way to write a file the + * agent cannot rewrite, and the scanner's own code is on an agent-writable + * path regardless. See the threat model at the top of this file. + */ +function prePushHookPresent(hooksDir: string): boolean { + try { + accessSync(path.join(hooksDir, "pre-push"), constants.X_OK); + return true; + } catch { + return false; + } +} + +/** + * Build the argv the real git is invoked with. + * + * `core.hooksPath` is injected only for a push. Setting it unconditionally + * would shadow every OTHER hook a repository defines — pre-commit, commit-msg — + * because a hooks directory holding only `pre-push` makes the rest silently stop + * running. Scoping the injection to the one command whose hook we supply keeps + * that blast radius off unrelated workflows. + * + * Placement is a security property, not a style choice. Git takes the LAST `-c` + * given for a key, so injecting at the FRONT lets a caller-supplied + * `git -c core.hooksPath=/tmp/empty push` override the guard and skip the + * scanner entirely. The guard therefore goes immediately before the subcommand, + * after every global option the caller passed, where it is the last value git + * sees. Measured against git 2.47.3: from that position it also beats + * `--config-env=core.hooksPath=...`, the case-folded `CORE.HOOKSPATH` spelling, + * a repository's own `core.hooksPath` config, and `GIT_CONFIG_KEY_*` in the + * environment. + */ +/** + * The refusal for an alias-carried bypass. + * + * Extracted because it is thrown from two places in {@link buildGitArgv} — once + * ahead of the not-a-push early return and once after it — and the two must not + * drift into saying different things about the same finding. + */ +function aliasBypassRefusal(bypass: GitAliasBypass): GitEgressRuntimeError { + const { alias, expansion, reason, chain } = bypass; + + // Worded separately because this one asserts no bypass: the chain outran the + // resolver, so what it reaches is simply unknown. Telling the author it + // "expands to a push that ..." would be a claim the guard cannot make. + if (reason === "alias-depth") { + const shown = (chain ?? [alias]).join("` → `"); + return new GitEgressRuntimeError( + `paperclip-github-egress: refusing to run \`${alias}\` — it is an alias chain deeper than ${ALIAS_HOP_LIMIT} hops (\`${shown}\` → \`${expansion}\`), so this guard stopped resolving before reaching the command git would actually run, and cannot tell whether it publishes. Resolution is bounded on purpose: the config defining the chain is writable from here, so an unbounded walk would be a denial of service. Invoke the underlying command directly, or flatten the alias so it resolves within ${ALIAS_HOP_LIMIT} hops.`, + ); + } + + const what = + reason === "no-verify" + ? "skips the pre-push hook with --no-verify" + : reason === "hooks-path" + ? "points core.hooksPath somewhere else" + : "cannot be parsed the way git parses an alias (unterminated quote), so it cannot be checked for a bypass"; + return new GitEgressRuntimeError( + `paperclip-github-egress: refusing to publish — the alias \`${alias}\` expands to a push that ${what} (\`${expansion}\`), which would bypass the check for credential-shaped material. Invoke the push directly instead of through the alias, or redefine the alias without it.`, + ); +} + +/** + * The refusal for a verb that is not `push` and not cleared as non-publishing. + * + * The two wordings differ because the remedies differ. For a verb known to + * publish there is nothing to add to an allowlist and the author needs to hear + * that the `push` subcommand is the guarded way. For an unrecognised one the + * refusal is a conservative default that an operator may legitimately want to + * relax, so it names the constant to edit — otherwise the natural fix is to + * reach around the wrapper, which is strictly worse than widening the list on + * purpose. + */ +function publishVerbRefusal(publishVerb: GitPublishVerb): GitEgressRuntimeError { + const { verb, known, alias, chain } = publishVerb; + const via = alias + ? ` It was reached through the alias \`${alias}\` (\`${(chain ?? [alias]).join("` → `")}\`), so the definition is where the fix goes.` + : ""; + + if (known) { + return new GitEgressRuntimeError( + `paperclip-github-egress: refusing to run \`git ${verb}\` — it publishes to a remote but does NOT run the pre-push hook, so the check for credential-shaped material would never see the objects it sends. Measured against git 2.47.3: \`send-pack\` landed a new ref on the remote with the hook silent, and with this guard's own \`core.hooksPath\` present — injecting the hook does not help, because the command never reads it. Publish with the \`push\` subcommand, which is guarded.${via}`, + ); + } + + return new GitEgressRuntimeError( + `paperclip-github-egress: refusing to run \`git ${verb}\` — this guard passes through only verbs it knows cannot publish to a remote, and \`${verb}\` is not one of them. Unrecognised is refused rather than allowed because the plumbing verbs that publish (\`send-pack\`, \`http-push\`) do so WITHOUT running the pre-push hook, so letting an unknown verb through is a silent hole rather than a noisy one. If \`${verb}\` cannot publish, add it to NON_PUBLISHING_GIT_VERBS in github-git-egress-shim.ts; if it can, use the \`push\` subcommand instead.${via}`, + ); +} + +export function buildGitArgv( + argv: readonly string[], + options: { + hooksDir: string; + resolveAlias?: (name: string) => string | null; + env?: NodeJS.ProcessEnv; + /** Seam for the fs check; the default is the real one. */ + hookPresent?: (hooksDir: string) => boolean; + }, +): string[] { + const classification = classifyGitInvocation(argv, options.resolveAlias, options.env ?? {}); + + // Checked ahead of the not-a-push early return, deliberately. A shell alias + // cannot be classified as a push or not — its expansion is arbitrary shell — + // and it is the one form that escapes this guard entirely, because git + // prepends its exec-path (which ships a complete `git`) to PATH for the shell + // it spawns. So a bare `git push` inside the expansion reaches the real git + // without passing through this wrapper or the hook. + if (classification.shellAlias) { + const { alias, expansion } = classification.shellAlias; + throw new GitEgressRuntimeError( + `paperclip-github-egress: refusing to run the shell alias \`${alias}\` (\`${expansion}\`). A \`!\` alias runs arbitrary shell, and git puts its own exec-path ahead of PATH for it, so a \`push\` inside the expansion would reach git directly and skip the check for credential-shaped material. Run the underlying commands directly instead of through the alias.`, + ); + } + + // Checked ahead of the not-a-push early return, for the same reason the shell + // alias above is. `classifyGitInvocation` keeps a bypass with no push attached + // ONLY for the two reasons that exist because "is this a push?" is itself the + // question that went unanswered — `unquotable` and `alias-depth` — and it + // documents at length why neither may be gated on `isPush`. Letting the early + // return below discard them defeats that one layer up, silently: the shim + // reports the refusal and the wrapper drops it. + // + // Measured against git 2.47.3 with `alias.a1=a2 … alias.a5=push`: the early + // return handed argv straight to git, which expanded the whole chain and + // pushed `refs/heads/deep5` to the remote with no hook. The unit test on + // `classifyGitInvocation` passed throughout — it asserts the bypass is + // REPORTED, which it was. Only driving the wrapper end to end showed it being + // thrown away. + // + // The push-carried reasons are deliberately NOT handled here; they fall + // through to the block below so the argv-level `--no-verify` and + // `core.hooksPath` refusals keep winning the message on a real push. + if ( + classification.aliasBypass && + (classification.aliasBypass.reason === "unquotable" || + classification.aliasBypass.reason === "alias-depth") + ) { + throw aliasBypassRefusal(classification.aliasBypass); + } + + // Ahead of the not-a-push early return, for the same reason as the two + // blocks above: this is set precisely WHEN the invocation is not a `push`, + // so letting the early return run first would discard every one of them. + // + // Refusal, not guarding. Adding these verbs to the push classification so + // `core.hooksPath` is injected does nothing — `send-pack` does not consult + // the pre-push hook at all, measured against git 2.47.3 with this guard's + // own `-c` present: the ref landed and the hook never ran. There is no hook + // to make fire, so the only enforcement available is to not run the command. + if (classification.publishVerb) { + throw publishVerbRefusal(classification.publishVerb); + } + + if (!classification.isPush) return [...argv]; + + if (classification.hasNoVerify) { + throw new GitEgressRuntimeError( + "paperclip-github-egress: --no-verify is disabled on publish, because it skips the hook that checks whether the commits carry credential-shaped material. Re-run without it.", + ); + } + + // Injecting last already wins over this, so the refusal is not what makes the + // guard hold — it is here so a caller who asked for a different hooks + // directory is told their request was rejected rather than silently dropped, + // and so the control does not rest on ordering alone. + if (classification.hooksPathOverride) { + throw new GitEgressRuntimeError( + `paperclip-github-egress: refusing to publish — this invocation sets core.hooksPath itself (\`${classification.hooksPathOverride}\`), which would replace the hook that checks whether these commits carry credential-shaped material. Re-run the push without it.`, + ); + } + + // An alias is the one case injection cannot win: git expands it AFTER the + // command line, so a `-c core.hooksPath=` or `--no-verify` inside the + // expansion is the last thing git sees no matter where the guard is placed. + // Refusal is the only enforcement available here. + if (classification.aliasBypass) { + throw aliasBypassRefusal(classification.aliasBypass); + } + + // Last, so the specific caller-error refusals above win the message. Placed + // before the injection because injecting a hooks path with no hook in it is + // indistinguishable, from the outside, from a push that was scanned. + const hookPresent = options.hookPresent ?? prePushHookPresent; + if (!hookPresent(options.hooksDir)) { + throw new GitEgressRuntimeError( + `paperclip-github-egress: refusing to publish — no executable pre-push hook at \`${path.join(options.hooksDir, "pre-push")}\`, so this push could not be checked for credential-shaped material. This is a deployment fault, not something to work around: the hook is written by the chart's agent-runtime seed. Report it rather than pushing past it.`, + ); + } + + // Immediately before the subcommand: after the caller's global options, so + // this is the last `core.hooksPath` git reads, and still ahead of the + // subcommand, which is where git requires global options to sit. + const at = classification.subcommandIndex; + return [ + ...argv.slice(0, at), + "-c", + `core.hooksPath=${options.hooksDir}`, + ...argv.slice(at), + ]; +} + +export function makeGitReader(gitPath: string, cwd?: string): GitReader { + return (args: string[]) => { + const result = spawnSync(gitPath, args, { + cwd, + encoding: "utf8", + maxBuffer: 64 * 1024 * 1024, + }); + if (result.error || result.status !== 0) return null; + return result.stdout; + }; +} + +/** + * The part of `process.stdin` the hook reads, narrowed so a test can supply a + * stand-in. `isTTY` is the field the refusal in {@link readStdin} turns on. + */ +export interface HookStdin { + isTTY?: boolean; + setEncoding(encoding: "utf8"): unknown; + on(event: "data", listener: (chunk: string) => void): unknown; + on(event: "end", listener: () => void): unknown; + on(event: "error", listener: (error: unknown) => void): unknown; +} + +/** + * Read the ref-update list git pipes to the pre-push hook. + * + * Takes the stream so the refusal below is reachable from a test without + * allocating a pty; production passes `process.stdin`. + */ +export function readStdin(stream: HookStdin = process.stdin): Promise { + return new Promise((resolve, reject) => { + let buffer = ""; + // A TTY on stdin means git did not invoke this. Git always hands the + // pre-push hook a pipe — measured against git 2.47.3, `isTTY` is false and + // fd 0 is a FIFO on both a real push and an "Everything up-to-date" one. + // + // Resolving "" here was the fail-open: it is indistinguishable from the + // empty ref list git legitimately sends for a no-op push, so + // `runPrePushHook` returned 0 and the caller read "scanned and clean" from + // a run that never had input to scan. Refuse instead — the guard cannot see + // its input, and an unread ref update is an unscanned ref update. + // + // Note this rejects rather than waiting for `end`: a TTY never ends, so + // waiting would hang the push instead of refusing it. + if (stream.isTTY) { + reject(new Error("pre-push hook stdin is a TTY; expected the ref-update pipe git supplies")); + return; + } + stream.setEncoding("utf8"); + stream.on("data", (chunk) => { + buffer += chunk; + }); + stream.on("end", () => resolve(buffer)); + // Reject rather than resolving the partial buffer. A truncation that lands + // mid-line throws in `parsePrePushInput` and refuses, but one landing + // exactly on a newline yields a SHORTER, well-formed update list — and + // `runPrePushHook` reads a short list as "that is all this push contains" + // and returns 0 for the ref updates that were dropped. The rejection + // reaches `reportRuntimeError`, which sets a non-zero exit code, so the + // push aborts: an unread ref update is an unscanned ref update, exactly as + // an unreadable commit is an unscanned commit. + stream.on("error", reject); + }); +} + +/** + * The pre-push hook. Exit 0 lets the push proceed; non-zero aborts it. + * + * Every remote is guarded, not only github.com. The material this refuses is + * credential-shaped wherever it lands, and scoping the check by remote URL would + * turn `git remote add` into the bypass. + * + * The scan is wrapped so that ANY failure aborts the push. A guard whose error + * path is "allow" is not a guard: a git read that fails, a buffer that + * overflows on a large diff, or an unanticipated throw would each otherwise + * publish the commits unscanned, and the larger the push the likelier that gets. + */ +export async function runPrePushHook(options: { + input: string; + runGit: GitReader; + stderr?: (message: string) => void; +}): Promise { + const write = options.stderr ?? ((message: string) => process.stderr.write(`${message}\n`)); + + let findings; + try { + // Inside the try: parsing the hook's stdin is part of deciding what this + // push publishes, so a throw from it must refuse like any other unknown + // verdict rather than escaping the guard's own error path. + const updates = parsePrePushInput(options.input); + // An empty update list is a PASS, and must stay one. Git runs the pre-push + // hook with genuinely empty stdin on an "Everything up-to-date" push — + // measured against git 2.47.3, hook invoked, zero bytes on the pipe — so + // refusing here would fail every no-op push. + // + // What makes that safe is that the one way to arrive here WITHOUT git + // having said "nothing to push" is now closed upstream: `readStdin` refuses + // a TTY rather than resolving "", so "git sent an empty list" and "this was + // never invoked by git" are no longer the same value. + if (updates.length === 0) return 0; + findings = scanPrePushUpdates(updates, options.runGit); + } catch (error) { + // Deliberately catching everything, not just GitEgressScanError. An + // unexpected throw is exactly the case where the scanner's verdict is + // unknown, which must refuse rather than pass. + write(formatScanFailure(error)); + return 1; + } + + if (findings.length === 0) return 0; + + write(formatRefusal(findings)); + return 1; +} + +export function runGitEgressRuntime(options: { + target: string; + argv: string[]; + hooksDir: string; + env?: NodeJS.ProcessEnv; + hookPresent?: (hooksDir: string) => boolean; +}): Promise { + const env = options.env ?? process.env; + + // Resolve aliases under the same effective configuration git itself will use. + // A bare `git config --get` reads whichever config files the WRAPPER's cwd + // selects, which is not necessarily the set the invocation selects: `-C`, + // `--git-dir` and `--work-tree` all change it, so `git -C /elsewhere yolo` + // would be looked up against the wrong repository. Forwarding the caller's + // own global options puts the lookup in the same place as the push. + // + // `--no-pager` goes last so it beats a caller's `-p`, which would otherwise + // hand this read to a pager. Nothing here can run a hook: `config` is a read. + // + // This does NOT cover `-c alias.x=...`; a definition on the command line is + // unreachable from a second process no matter what it is passed. That case is + // closed in `classifyGitInvocation`, which reads such definitions straight out + // of argv and consults them before this callback. + const globals = gitGlobalOptions(options.argv); + const resolveAlias = (name: string): string | null => { + const result = spawnSync( + options.target, + [...globals, "--no-pager", "config", "--get", `alias.${name}`], + { encoding: "utf8", env, timeout: 10_000 }, + ); + if (result.error || result.status !== 0) return null; + const value = result.stdout.trim(); + return value.length > 0 ? value : null; + }; + + const argv = buildGitArgv(options.argv, { + hooksDir: options.hooksDir, + resolveAlias, + env, + hookPresent: options.hookPresent, + }); + + return new Promise((resolve, reject) => { + const child = spawn(options.target, argv, { stdio: "inherit" }); + let forwardedSignal = false; + let settled = false; + const forwardSignal = (signal: NodeJS.Signals) => { + forwardedSignal = true; + child.kill(signal); + }; + process.on("SIGINT", forwardSignal); + process.on("SIGTERM", forwardSignal); + const cleanup = () => { + process.off("SIGINT", forwardSignal); + process.off("SIGTERM", forwardSignal); + }; + + child.once("error", (error: NodeJS.ErrnoException) => { + if (settled) return; + settled = true; + cleanup(); + reject(new GitEgressRuntimeError(`unable to start git (${error.code ?? "unknown error"})`, 1)); + }); + child.once("close", (code, signal) => { + if (settled) return; + settled = true; + cleanup(); + if (code !== null) { + resolve(code); + return; + } + resolve(forwardedSignal ? 128 : signal ? 128 + (signal === "SIGINT" ? 2 : 15) : 1); + }); + }); +} + +function reportRuntimeError(error: unknown): void { + const message = error instanceof Error ? error.message : "unexpected preparation failure"; + const exitCode = error instanceof GitEgressRuntimeError ? error.exitCode : 1; + console.error(message.startsWith("paperclip-github-egress") ? message : `paperclip-github-egress: ${message}`); + process.exitCode = exitCode; +} + +/** + * Is this module the process entrypoint? + * + * Tolerates a symlinked install. `process.argv[1]` is the literal path the + * caller named; `import.meta.url` is what Node resolved, which is the REALPATH + * unless `--preserve-symlinks` is set. A plain string compare of the two is + * false whenever any parent directory is a link — measured on Node 24.16: with + * the package reached through a symlinked directory, `path.resolve(argv[1])` + * and `fileURLToPath(import.meta.url)` differ, and `realpathSync` agrees again. + * + * This is used ONLY for the wrapper leg. The hook leg must not depend on it — + * see the entry block below for why. + */ +function invokedAsEntrypoint(): boolean { + const argv1 = process.argv[1]; + if (!argv1) return false; + const here = fileURLToPath(import.meta.url); + if (path.resolve(argv1) === here) return true; + try { + return realpathSync(argv1) === realpathSync(here); + } catch { + // An unresolvable argv[1] is not this module. The hook leg does not reach + // here, so returning false cannot open the publish boundary. + return false; + } +} + +// Hook mode is selected by argv ALONE, deliberately, and not by +// `invokedAsEntrypoint()`. +// +// The entrypoint guard is the standard idiom, and it is copied from +// `github-cli-egress-runtime.ts` and `github-mcp-egress-runtime.ts` where it is +// safe. It is NOT safe here, and the failure direction is inverted: for `gh` and +// `github-mcp-server` a bootstrap that does not run is a command that produces +// no output, which is loud. For a pre-push hook it is a silent exit 0, and git +// reads exit 0 as "hook passed" — the one thing `prePushHookPresent` documents +// that this door cannot tolerate, reinstated one layer up. +// +// Measured end to end: with the package reached through a symlinked directory, +// a push carrying a credential-shaped literal exited 0 with no output and +// landed the commit on the remote, while the identical push through the +// unlinked path was refused and landed nothing. Causes that make the two paths +// diverge are ordinary, not exotic — pnpm's store layout, workspace hoisting, +// or a bundler emitting a re-export shim. +// +// `realpathSync` alone would not be enough: it repairs a symlink, but a +// re-export shim is a DIFFERENT FILE, so no amount of path canonicalisation +// makes the comparison true. The only form that fails closed is to take argv as +// the contract — `--pre-push-hook` is a private spelling nothing but the seeded +// hook passes — and keep path resolution out of the decision entirely. +const invokedAsPrePushHook = process.argv[2] === "--pre-push-hook"; + +if (invokedAsPrePushHook) { + void readStdin() + .then((input) => + runPrePushHook({ input, runGit: makeGitReader(gitBinary()) }), + ) + .then((exitCode) => { + process.exitCode = exitCode; + }) + .catch(reportRuntimeError); +} else if (invokedAsEntrypoint()) { + const target = process.argv[2]; + const argv = process.argv.slice(3); + try { + if (!target) throw new GitEgressRuntimeError("missing git target"); + void runGitEgressRuntime({ target, argv, hooksDir: hooksDirectory() }) + .then((exitCode) => { + process.exitCode = exitCode; + }) + .catch(reportRuntimeError); + } catch (error) { + reportRuntimeError(error); + } +} diff --git a/packages/adapter-utils/src/github-git-egress-shim.test.ts b/packages/adapter-utils/src/github-git-egress-shim.test.ts new file mode 100644 index 000000000000..7aa53bcbff2f --- /dev/null +++ b/packages/adapter-utils/src/github-git-egress-shim.test.ts @@ -0,0 +1,935 @@ +import { describe, expect, it } from "vitest"; + +import { + addedLinesFromPatch, + classifyGitInvocation, + commitsForRefUpdate, + formatRefusal, + type GitAliasBypass, + GitEgressInputError, + GitEgressScanError, + gitGlobalOptions, + parsePrePushInput, + scanAnnotatedTags, + scanCommit, + scanPrePushUpdates, + TAG_PEEL_LIMIT, + type GitReader, +} from "./github-git-egress-shim.js"; + +/** + * An environment dump, assembled rather than pasted. + * + * Every fixture in this file is built from parts on purpose. A literal + * credential here would be committed, and CI scans the COMMIT RANGE with + * gitleaks — so a literal would trip the secret gate inside the very pull + * request that adds a secret-containment control, and could then only be + * cleared by rewriting the commit rather than by deleting the line. + */ +function environmentDump(): string { + return ["ALPHA", "BRAVO", "CHARLIE", "DELTA", "ECHOES"] + .map((name, index) => `${name}=value-${index}`) + .join("\n"); +} + +/** A vendor-key-shaped token, assembled for the same reason as the dump above. */ +function vendorKey(): string { + return `${["gh", "p"].join("")}_${"Ab3".repeat(9)}`; +} + +/** A git reader backed by a fixed map, so no repository is needed. */ +function fakeGit(responses: Record): GitReader { + return (args: string[]) => { + const key = args.join(" "); + return key in responses ? responses[key]! : null; + }; +} + +describe("classifyGitInvocation", () => { + it("finds a bare push", () => { + const result = classifyGitInvocation(["push", "origin", "main"]); + expect(result.isPush).toBe(true); + expect(result.subcommand).toBe("push"); + }); + + it("is not fooled by a global option that takes a separate value", () => { + // The regression this guards: treating `-c` as a valueless flag makes + // `foo=bar` read as the subcommand, and the push sails past the guard. + for (const argv of [ + ["-c", "foo=bar", "push"], + ["-C", "/tmp/repo", "push"], + ["--git-dir", "/tmp/repo/.git", "push"], + ["--work-tree", "/tmp/repo", "push"], + ]) { + expect(classifyGitInvocation(argv).isPush, argv.join(" ")).toBe(true); + } + }); + + it("handles the self-contained --opt=value spelling", () => { + expect(classifyGitInvocation(["--git-dir=/tmp/r/.git", "push"]).isPush).toBe(true); + }); + + it("does not classify unrelated subcommands as a push", () => { + for (const sub of ["status", "commit", "fetch", "log", "diff"]) { + expect(classifyGitInvocation([sub]).isPush, sub).toBe(false); + } + }); + + it("resolves an alias that expands to a push", () => { + const resolve = (name: string) => (name === "yolo" ? "push --force" : null); + expect(classifyGitInvocation(["yolo"], resolve).isPush).toBe(true); + }); + + it("resolves a chain of aliases, and stops rather than looping forever", () => { + const chain: Record = { a: "b", b: "c", c: "push" }; + expect(classifyGitInvocation(["a"], (n) => chain[n] ?? null).isPush).toBe(true); + + const cyclic: Record = { x: "y", y: "x" }; + expect(classifyGitInvocation(["x"], (n) => cyclic[n] ?? null).isPush).toBe(false); + }); + + it("does not try to parse a shell alias", () => { + // Assembled from parts on purpose. scripts/check-no-git-push.mjs scans + // string literals in this tree for those two words together, and the + // `paperclip:allow-git-push` marker that opts a line out asserts an + // operator-approved push path exists. This is a test fixture and no such + // path exists, so spending that escape hatch here would put a false claim + // inside a security control. + const shellAlias = `!git ${"push"} --all`; + expect(classifyGitInvocation(["sh"], () => shellAlias).isPush).toBe(false); + }); + + it("sees a bypass through every quoting form git dequotes", () => { + // Git does NOT split an alias on whitespace — it runs `split_cmdline()`, + // which applies shell quoting. Splitting on /\s+/ left `"--no-verify"` with + // its quotes attached, matched nothing, and pushed unscanned. + // + // Every form below was measured against git 2.47.3 as a REAL bypass: with + // `-c core.hooksPath= -c 'alias.q=
' q origin HEAD:refs/heads/t` + // the push exited 0, the hook did not run, and the ref landed on the bare + // remote. They are regression cases, not hypotheses. + const forms: Array<[string, string, GitAliasBypass["reason"]]> = [ + ["bare", "push --no-verify", "no-verify"], + ["double-quoted", 'push "--no-verify"', "no-verify"], + ["single-quoted", "push '--no-verify'", "no-verify"], + ["split across a quote boundary", 'push "--no-ver"ify', "no-verify"], + ["backslash-escaped", "push \\-\\-no-verify", "no-verify"], + ["bare hooks path", "-c core.hooksPath=/tmp/empty push", "hooks-path"], + ["quoted hooks path", '-c "core.hooksPath=/tmp/empty" push', "hooks-path"], + ]; + for (const [label, expansion, reason] of forms) { + const result = classifyGitInvocation(["q"], (name) => + name === "q" ? expansion : null, + ); + expect(result.aliasBypass, label).toMatchObject({ alias: "q", reason }); + } + }); + + it("fails closed on an alias it cannot tokenise", () => { + // Git rejects an unclosed quote outright (`fatal: bad alias.q string: + // unclosed quote`), so nothing publishes either way — but the guard must + // not make its safety depend on git's parser agreeing with ours. + const result = classifyGitInvocation(["q"], (name) => + name === "q" ? 'push "--no-verify' : null, + ); + expect(result.aliasBypass).toMatchObject({ alias: "q", reason: "unquotable" }); + }); + + it("does not refuse an ordinary alias that merely contains quotes", () => { + // Failing closed on the presence of a quote would reject legitimate + // aliases; it is an unparseable expansion that is refused, not a quoted one. + const result = classifyGitInvocation(["p"], (name) => + name === "p" ? 'push "origin" main' : null, + ); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toBeNull(); + }); + + it("detects the hook-skipping flag", () => { + expect(classifyGitInvocation(["push", "--no-verify"]).hasNoVerify).toBe(true); + expect(classifyGitInvocation(["push"]).hasNoVerify).toBe(false); + }); + + it.each([["--no-veri"], ["--no-verif"]])( + "detects the hook-skipping flag spelled as the abbreviation %s", + (flag) => { + // Git's parse-options accepts any UNAMBIGUOUS long-option abbreviation, so + // these are `--no-verify` to git while matching no literal spelling. + // Measured against git 2.47.3: `git -c core.hooksPath= push --no-veri + // origin HEAD:refs/heads/t` printed no hook output and landed the ref on + // the remote, while the same push without the flag ran the hook and was + // refused. `--no-veri` is the shortest that works — `--no-ver` and + // shorter are ambiguous with `--no-verbose` and git rejects them. + expect(classifyGitInvocation(["push", flag]).hasNoVerify).toBe(true); + }, + ); + + it.each([["--no-ver"], ["--no-ve"], ["--no-v"], ["--no"], ["--n"]])( + "refuses %s too, though git rejects it as ambiguous", + (flag) => { + // Refusing a spelling git will not accept costs nothing — no command git + // would have run is turned away — and it keeps the test "could this be + // that flag?" rather than an enumeration that a future git could outgrow. + expect(classifyGitInvocation(["push", flag]).hasNoVerify).toBe(true); + }, + ); + + it("does not mistake the end-of-options separator for the flag", () => { + // `--` is a prefix of nothing meaningful and must not trip the length floor. + expect(classifyGitInvocation(["push", "--"]).hasNoVerify).toBe(false); + }); + + it("does not mistake --verify for --no-verify", () => { + expect(classifyGitInvocation(["push", "--verify"]).hasNoVerify).toBe(false); + }); + + it("detects an abbreviated hook-skipping flag inside an alias expansion", () => { + // The alias leg had the same gap: `alias.q = push --no-veri` pushed + // unscanned against git 2.47.3. + const result = classifyGitInvocation(["q"], (name) => + name === "q" ? "push --no-veri" : null, + ); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toMatchObject({ alias: "q", reason: "no-verify" }); + }); + + it("does not treat `push -n` as a hook bypass, because it is --dry-run", () => { + // For `push`, `-n` is --dry-run, not --no-verify (see the subcommand's own + // `-h` output). Refusing + // it would reject a safe command while giving a false reason, and it is + // harmless twice over: verified against git 2.47.3, the pre-push hook still + // runs under --dry-run, and a dry run publishes nothing even if it did not. + expect(classifyGitInvocation(["push", "-n"]).hasNoVerify).toBe(false); + }); + + it("reports a caller-supplied core.hooksPath override, however it is spelled", () => { + // Each of these overrode a guard injected at the FRONT of argv when + // measured against git 2.47.3, because git takes the last value for a key. + const cases: Array<[string, string[]]> = [ + ["-c", ["-c", "core.hooksPath=/tmp/empty", "push"]], + ["case-folded key", ["-c", "CORE.HOOKSPATH=/tmp/empty", "push"]], + ["--config-env=", ["--config-env=core.hooksPath=HP", "push"]], + ["--config-env separate", ["--config-env", "core.hooksPath=HP", "push"]], + ]; + for (const [label, argv] of cases) { + const result = classifyGitInvocation(argv); + expect(result.isPush, label).toBe(true); + expect(result.hooksPathOverride, label).not.toBeNull(); + } + }); + + it("leaves hooksPathOverride null for unrelated config", () => { + const result = classifyGitInvocation(["-c", "user.name=someone", "push"]); + expect(result.isPush).toBe(true); + expect(result.hooksPathOverride).toBeNull(); + }); + + it("still sees the push when an alias expansion leads with its own global options", () => { + // Reading only the expansion's first word classifies this as a `-c` + // subcommand, so the guard is never injected and the alias pushes freely. + // Measured as a working bypass against git 2.47.3. + const resolve = (name: string) => + name === "sneaky" ? "-c core.hooksPath=/tmp/empty push" : null; + const result = classifyGitInvocation(["sneaky"], resolve); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toMatchObject({ alias: "sneaky", reason: "hooks-path" }); + }); + + it("reports an alias whose expansion skips the hook", () => { + const resolve = (name: string) => (name === "yolo" ? "push --no-verify" : null); + const result = classifyGitInvocation(["yolo"], resolve); + expect(result.isPush).toBe(true); + // argv itself carries no --no-verify; the bypass is only in the expansion, + // and git applies it after the command line, so injection cannot beat it. + expect(result.hasNoVerify).toBe(false); + expect(result.aliasBypass).toMatchObject({ alias: "yolo", reason: "no-verify" }); + }); + + it("carries a bypass found part-way along an alias chain", () => { + const chain: Record = { a: "-c core.hooksPath=/tmp/empty b", b: "push" }; + const result = classifyGitInvocation(["a"], (n) => chain[n] ?? null); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toMatchObject({ alias: "a", reason: "hooks-path" }); + }); + + it("ignores a bypass on an alias that never reaches a push", () => { + // `git amend` skipping commit-msg hooks is not this guard's business, and + // refusing it would break unrelated tooling. + const resolve = (name: string) => (name === "amend" ? "commit --amend --no-verify" : null); + const result = classifyGitInvocation(["amend"], resolve); + expect(result.isPush).toBe(false); + expect(result.aliasBypass).toBeNull(); + }); + + it("reports no alias bypass for an ordinary push alias", () => { + const resolve = (name: string) => (name === "p" ? "push --force-with-lease" : null); + const result = classifyGitInvocation(["p"], resolve); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toBeNull(); + }); + + it("resolves an alias the invocation defines on its own command line", () => { + // The hole this closes, measured end to end against git 2.47.3: + // git -c alias.yolo='push --no-verify' yolo origin HEAD:refs/heads/t + // pushed to the remote with the pre-push hook never running. A separate + // `git config --get alias.yolo` run beside it exits 1 with no output, so a + // lookup in another process cannot see the definition at all — consulting + // only `resolveAlias` classified `yolo` as not-a-push, left argv untouched, + // and git then expanded the alias itself. The definition is therefore read + // out of argv rather than looked up. No resolver is passed here on purpose. + const result = classifyGitInvocation(["-c", "alias.yolo=push --no-verify", "yolo"]); + expect(result.isPush).toBe(true); + // argv itself carries no --no-verify; only the expansion does. + expect(result.hasNoVerify).toBe(false); + expect(result.aliasBypass).toMatchObject({ alias: "yolo", reason: "no-verify" }); + }); + + it("resolves a command-line alias that expands to an ordinary push", () => { + // The other half of the same hole, and the half that is not a refusal: + // classifying this as a push is what makes the wrapper inject the guard. + // Verified against git 2.47.3 — with `-c core.hooksPath=` injected the hook + // ran and aborted the push; without it the push went through unscanned. + const result = classifyGitInvocation(["-c", "alias.p=push", "p", "origin", "main"]); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toBeNull(); + expect(result.subcommandIndex).toBe(2); + }); + + it("reads a command-line alias however git would spell it", () => { + const cases: Array<[string, string[], Record]> = [ + ["-c", ["-c", "alias.yolo=push", "yolo"], {}], + // Section and variable names are both case-insensitive in git config; + // `-c alias.YOLO=` and `-c ALIAS.yolo=` each define what `git yolo` runs. + ["case-folded name", ["-c", "alias.YOLO=push", "yolo"], {}], + ["case-folded section", ["-c", "ALIAS.yolo=push", "yolo"], {}], + ["--config-env=", ["--config-env=alias.yolo=A_PUSH", "yolo"], { A_PUSH: "push" }], + ["--config-env separate", ["--config-env", "alias.yolo=A_PUSH", "yolo"], { A_PUSH: "push" }], + ]; + for (const [label, argv, env] of cases) { + expect(classifyGitInvocation(argv, undefined, env).isPush, label).toBe(true); + } + }); + + it("takes the LAST command-line definition of an alias, as git does", () => { + // `git -c alias.d=status -c alias.d='push --no-verify' config --get-all + // alias.d` prints both, and git runs the last. Taking the first would read + // this invocation as a `status` and wave the push through. + const result = classifyGitInvocation([ + "-c", + "alias.d=status", + "-c", + "alias.d=push --no-verify", + "d", + ]); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toMatchObject({ reason: "no-verify" }); + }); + + it("prefers a command-line definition over one in config, as git does", () => { + const result = classifyGitInvocation( + ["-c", "alias.p=push --no-verify", "p"], + () => "status", + ); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toMatchObject({ reason: "no-verify" }); + }); + + it("carries command-line definitions made inside an alias expansion", () => { + // `alias.outer = -c alias.inner=push inner` reaches a push in git 2.47.3: + // the expansion's own `-c` defines an alias git then resolves. Scanning the + // expansion for its subcommand but discarding what it DEFINES loses the + // chain one hop early. + const result = classifyGitInvocation(["-c", "alias.outer=-c alias.inner=push inner", "outer"]); + expect(result.isPush).toBe(true); + }); + + it("does not treat a command-line alias to a non-push as a push", () => { + const result = classifyGitInvocation(["-c", "alias.st=status --short", "st"]); + expect(result.isPush).toBe(false); + expect(result.aliasBypass).toBeNull(); + }); + + it("refuses an alias chain deeper than the hop limit rather than passing it", () => { + // Measured against git 2.47.3 with `alias.a1=a2 … alias.a5=push`: hops 0-3 + // walk a1→a5 and resolution ends with `isPush` still false, so `buildGitArgv` + // returned argv untouched, no `core.hooksPath` was injected, and git then + // expanded the whole chain to a push that ran no hook — + // `git a1 origin HEAD:refs/heads/deep5` landed the ref on the remote. The + // same chain WITH the guard's `-c` did run the hook, which is what makes the + // missing injection the hole rather than the depth. + const chain: Record = { + a1: "a2", + a2: "a3", + a3: "a4", + a4: "a5", + a5: "push", + }; + const result = classifyGitInvocation(["a1", "origin", "HEAD:refs/heads/deep5"], (name) => + chain[name] ?? null, + ); + // The refusal does not depend on `isPush`: whether it reaches a push is + // exactly the question resolution ran out of budget to answer. + expect(result.aliasBypass).toMatchObject({ alias: "a1", reason: "alias-depth" }); + expect(result.aliasBypass?.chain).toEqual(["a1", "a2", "a3", "a4", "a5"]); + }); + + it("resolves a chain that reaches a push within the hop limit without refusing", () => { + const chain: Record = { a1: "a2", a2: "a3", a3: "a4", a4: "push" }; + const result = classifyGitInvocation(["a1"], (name) => chain[name] ?? null); + expect(result.isPush).toBe(true); + expect(result.aliasBypass).toBeNull(); + }); + + it("does not refuse a deep chain that ends in something git would not resolve", () => { + // Running out of hops is only a refusal when the chain is still LIVE. A tail + // git cannot resolve either is genuinely not a push, and refusing it would + // reject a command that publishes nothing. + const chain: Record = { b1: "b2", b2: "b3", b3: "b4", b4: "status" }; + const result = classifyGitInvocation(["b1"], (name) => chain[name] ?? null); + expect(result.isPush).toBe(false); + expect(result.aliasBypass).toBeNull(); + }); + + it("ignores a --config-env alias naming a variable that is not set", () => { + // git would fail the invocation outright; there is no expansion to parse, + // and inventing one would refuse a push over a definition that never + // existed. + expect(classifyGitInvocation(["--config-env=alias.yolo=NOPE", "yolo"], undefined, {}).isPush).toBe( + false, + ); + }); +}); + +describe("gitGlobalOptions", () => { + it("returns the options that precede the subcommand", () => { + expect(gitGlobalOptions(["-C", "/repo", "-c", "alias.p=push", "p", "origin"])).toEqual([ + "-C", + "/repo", + "-c", + "alias.p=push", + ]); + expect(gitGlobalOptions(["push", "origin"])).toEqual([]); + expect(gitGlobalOptions([])).toEqual([]); + }); +}); + +describe("parsePrePushInput", () => { + it("parses git's ref-update lines and ignores blank ones", () => { + const updates = parsePrePushInput( + "refs/heads/main aaa refs/heads/main bbb\n\nrefs/heads/x ccc refs/heads/x ddd\n", + ); + expect(updates).toHaveLength(2); + expect(updates[0]).toEqual({ + localRef: "refs/heads/main", + localSha: "aaa", + remoteRef: "refs/heads/main", + remoteSha: "bbb", + }); + }); + + it("returns nothing for empty input", () => { + expect(parsePrePushInput("")).toEqual([]); + }); + + it("throws on a non-empty line it cannot parse rather than scanning it as nothing", () => { + // Not reachable against git today — the format is fixed and refs cannot + // contain whitespace. It is closed anyway because skipping was the one + // fail-open shape left in the module, and the worst kind: `runPrePushHook` + // treats an empty update list as a PASS, so a line that silently failed to + // parse would publish its ref unscanned rather than merely under-report. + expect(() => parsePrePushInput("refs/heads/main aaa refs/heads/main")).toThrow( + GitEgressInputError, + ); + }); +}); + +describe("commitsForRefUpdate", () => { + const zero = "0".repeat(40); + + it("uses a two-dot range when the remote already has the ref", () => { + const seen: string[][] = []; + const runGit: GitReader = (args) => { + seen.push(args); + return "c1\nc2\n"; + }; + const commits = commitsForRefUpdate( + { localRef: "r", localSha: "new", remoteRef: "r", remoteSha: "old" }, + runGit, + ); + expect(seen[0]).toEqual(["rev-list", "old..new"]); + expect(commits).toEqual(["c1", "c2"]); + }); + + it("excludes everything already published when the ref is new", () => { + // Without `--not --remotes` a new branch re-reports the repository's whole + // history, which would refuse pushes over material that is already public. + const seen: string[][] = []; + const runGit: GitReader = (args) => { + seen.push(args); + return "c1\n"; + }; + commitsForRefUpdate( + { localRef: "r", localSha: "new", remoteRef: "r", remoteSha: zero }, + runGit, + ); + expect(seen[0]).toEqual(["rev-list", "new", "--not", "--remotes"]); + }); + + it("treats a branch deletion as publishing nothing", () => { + const commits = commitsForRefUpdate( + { localRef: "", localSha: zero, remoteRef: "r", remoteSha: "old" }, + () => { + throw new Error("git must not be consulted for a deletion"); + }, + ); + expect(commits).toEqual([]); + }); + + it("refuses rather than reporting an empty range when rev-list fails", () => { + // The regression this guards: returning [] on a failed read makes the push + // look like it publishes nothing, so it proceeds entirely unscanned. A + // `maxBuffer` overflow reaches here, which makes the largest pushes the + // likeliest to slip through. + expect(() => + commitsForRefUpdate( + { localRef: "r", localSha: "new", remoteRef: "r", remoteSha: "old" }, + () => null, + ), + ).toThrow(GitEgressScanError); + }); +}); + +describe("addedLinesFromPatch", () => { + it("strips the marker from added lines and drops everything else", () => { + const patch = ["--- a/f", "+++ b/f", "@@ -0,0 +1 @@", "+ADDED=1", "-REMOVED=1", " CONTEXT=1"].join( + "\n", + ); + expect(addedLinesFromPatch(patch)).toBe("ADDED=1"); + }); + + it("keeps an added content line whose own text begins with ++", () => { + // Skipping every `+++` line to drop the `+++ b/path` header also discarded + // added CONTENT beginning `++`, which git emits as `+++...`. Reproduced + // against git 2.47.3: committing a file whose first line is `++` + // produced the patch line `+++`, and it was dropped before the + // scrubber ever saw it. The vendor-key and JWT detectors are `\b`-anchored + // substring matches rather than line-anchored, so dropping the line drops + // the whole detection. + // + // Position, not spelling, separates the two: `+++` is a header only before + // the first `@@` of a file block. + const patch = [ + "diff --git a/leak.txt b/leak.txt", + "--- /dev/null", + "+++ b/leak.txt", + "@@ -0,0 +1,2 @@", + "+++MARKER=1", + "+normal line", + ].join("\n"); + expect(addedLinesFromPatch(patch)).toBe("++MARKER=1\nnormal line"); + }); + + it("still drops the +++ file header when content in the same patch starts with ++", () => { + const patch = [ + "diff --git a/a.txt b/a.txt", + "+++ b/a.txt", + "@@ -0,0 +1 @@", + "+plain", + "diff --git a/b.txt b/b.txt", + "+++ b/b.txt", + "@@ -0,0 +1 @@", + "+++ two-plus-space", + ].join("\n"); + // Neither `+++ b/a.txt` nor `+++ b/b.txt` is content; the last line is, and + // it survives even though it is spelled exactly like a header. + expect(addedLinesFromPatch(patch)).toBe("plain\n++ two-plus-space"); + }); + + it("keeps a line-anchored detector working through a diff", () => { + // THE regression this function exists for. The environment-dump detector + // anchors on `^[A-Z][A-Z0-9_]{2,}=`; in a raw patch every added line starts + // with `+`, so the anchor never matches and the PEN-2526 class goes + // undetected. Scanning the raw patch below must find nothing; scanning the + // normalised text must find the dump. + const patch = environmentDump() + .split("\n") + .map((line) => `+${line}`) + .join("\n"); + const findings = scanCommit( + "deadbeefcafe0000", + fakeGit({ + "log -1 --format=%s deadbeefcafe0000": "add config", + "log -1 --format=%B deadbeefcafe0000": "add config\n", + "show --format= --no-color -m --unified=0 --text --no-textconv deadbeefcafe0000": patch, + }), + ); + expect(findings.map((f) => f.where)).toEqual(["content"]); + expect(findings[0]!.classes).toContain("environment-dump"); + }); +}); + +describe("scanCommit", () => { + const sha = "abcdef0123456789abcdef0123456789abcdef01"; + + it("finds material in a commit message", () => { + const findings = scanCommit( + sha, + fakeGit({ + [`log -1 --format=%s ${sha}`]: "wip", + [`log -1 --format=%B ${sha}`]: `wip\n\n${environmentDump()}\n`, + [`show --format= --no-color -m --unified=0 --text --no-textconv ${sha}`]: "", + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.where).toBe("message"); + expect(findings[0]!.shortCommit).toBe("abcdef012345"); + expect(findings[0]!.subject).toBe("wip"); + }); + + it("redacts the subject it reports, so the refusal cannot republish the material", () => { + // The subject of a one-line commit IS the text that fired the detector, so + // reporting it verbatim carries the credential out of the control through + // stderr and the run log — the disclosure shape this whole path exists to + // stop. The finding must never hold the raw value. + const token = vendorKey(); + const subject = `fix: drop ${token} from config`; + const findings = scanCommit( + sha, + fakeGit({ + [`log -1 --format=%s ${sha}`]: subject, + [`log -1 --format=%B ${sha}`]: `${subject}\n`, + [`show --format= --no-color -m --unified=0 --text --no-textconv ${sha}`]: "", + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.where).toBe("message"); + expect(findings[0]!.subject).not.toContain(token); + expect(findings[0]!.subject).toContain("redacted: vendor-key"); + // The surrounding prose survives — the point is a recognisable subject, not + // an opaque one. + expect(findings[0]!.subject).toContain("fix: drop"); + }); + + it("passes a clean commit", () => { + const findings = scanCommit( + sha, + fakeGit({ + [`log -1 --format=%s ${sha}`]: "fix: tidy the readme", + [`log -1 --format=%B ${sha}`]: "fix: tidy the readme\n", + [`show --format= --no-color -m --unified=0 --text --no-textconv ${sha}`]: "+Hello, world.\n", + }), + ); + expect(findings).toEqual([]); + }); + + it("refuses when a read it needs fails, rather than skipping that leg", () => { + // Each read decides part of the verdict, so skipping one reports a commit + // clean that was never inspected. `fakeGit` returns null for any key it is + // not given, so each case below omits exactly one read. + const complete: Record = { + [`log -1 --format=%s ${sha}`]: "wip", + [`log -1 --format=%B ${sha}`]: "wip\n", + [`show --format= --no-color -m --unified=0 --text --no-textconv ${sha}`]: "+clean\n", + }; + for (const omitted of Object.keys(complete)) { + const responses = { ...complete }; + delete responses[omitted]; + expect(() => scanCommit(sha, fakeGit(responses)), omitted).toThrow(GitEgressScanError); + } + }); + + it("distinguishes an empty read from a failed one", () => { + // git exits zero with no output for an empty message or an empty diff. + // That is genuinely nothing to scan and must not be confused with a read + // that failed, or every such commit would refuse its own push. + const findings = scanCommit( + sha, + fakeGit({ + [`log -1 --format=%s ${sha}`]: "", + [`log -1 --format=%B ${sha}`]: "", + [`show --format= --no-color -m --unified=0 --text --no-textconv ${sha}`]: "", + }), + ); + expect(findings).toEqual([]); + }); +}); + +describe("scanPrePushUpdates", () => { + it("reports a commit once even when two pushed refs both reach it", () => { + const shared = "1111111111111111111111111111111111111111"; + const runGit: GitReader = (args) => { + if (args[0] === "rev-list") return `${shared}\n`; + if (args.includes("--format=%s")) return "shared"; + if (args.includes("--format=%B")) return `shared\n\n${environmentDump()}\n`; + return ""; + }; + const findings = scanPrePushUpdates( + [ + { localRef: "a", localSha: "a1", remoteRef: "a", remoteSha: "a0" }, + { localRef: "b", localSha: "b1", remoteRef: "b", remoteSha: "b0" }, + ], + runGit, + ); + expect(findings).toHaveLength(1); + }); +}); + +describe("scanAnnotatedTags", () => { + const zero = "0".repeat(40); + const tagSha = "a".repeat(40); + const commitSha = "b".repeat(40); + + function tagObject(message: string, name = "v1"): string { + return [ + `object ${commitSha}`, + "type commit", + `tag ${name}`, + "tagger T 1700000000 +0000", + "", + message, + ].join("\n"); + } + + it("finds credential-shaped material in a tag object's own message", () => { + const findings = scanAnnotatedTags( + { localRef: "refs/tags/v1", localSha: tagSha, remoteRef: "refs/tags/v1", remoteSha: zero }, + fakeGit({ + [`cat-file -t ${tagSha}`]: "tag\n", + [`cat-file tag ${tagSha}`]: tagObject(`Release v1\n\n${environmentDump()}\n`), + [`cat-file -t ${commitSha}`]: "commit\n", + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.where).toBe("tag-message"); + expect(findings[0]!.subject).toBe("tag v1"); + expect(findings[0]!.commit).toBe(tagSha); + }); + + it("does not scan the tagger header as message content", () => { + // The header block carries an email address. Reporting it would refuse + // every annotated tag on material the author did not write into the + // message, so the split has to land after the header's blank line. + const findings = scanAnnotatedTags( + { localRef: "refs/tags/v1", localSha: tagSha, remoteRef: "refs/tags/v1", remoteSha: zero }, + fakeGit({ + [`cat-file -t ${tagSha}`]: "tag\n", + [`cat-file tag ${tagSha}`]: tagObject("Release v1\n\nJust a normal release.\n"), + [`cat-file -t ${commitSha}`]: "commit\n", + }), + ); + expect(findings).toEqual([]); + }); + + it("reads nothing for a lightweight tag", () => { + // The ref points at a commit, so there is no tag object and the commit leg + // already covers it. + const findings = scanAnnotatedTags( + { localRef: "refs/tags/v1", localSha: commitSha, remoteRef: "refs/tags/v1", remoteSha: zero }, + fakeGit({ [`cat-file -t ${commitSha}`]: "commit\n" }), + ); + expect(findings).toEqual([]); + }); + + it("consults git for nothing on a tag deletion", () => { + expect( + scanAnnotatedTags( + { localRef: "", localSha: zero, remoteRef: "refs/tags/v1", remoteSha: commitSha }, + () => { + throw new Error("git must not be consulted for a deletion"); + }, + ), + ).toEqual([]); + }); + + it("refuses rather than passing when the tag object cannot be read", () => { + // Fail closed, for the same reason every other read on this path does: an + // unreadable tag object is an unscanned tag object. + expect(() => + scanAnnotatedTags( + { localRef: "refs/tags/v1", localSha: tagSha, remoteRef: "refs/tags/v1", remoteSha: zero }, + fakeGit({ [`cat-file -t ${tagSha}`]: "tag\n" }), + ), + ).toThrow(GitEgressScanError); + }); + + it("terminates on a tag object that points at itself", () => { + // Git permits a tag pointing at a tag. A cyclic or malformed chain must not + // spin the push forever. + const selfReferential = [ + `object ${tagSha}`, + "type tag", + "tag loop", + "tagger T 1700000000 +0000", + "", + "nothing interesting", + ].join("\n"); + expect( + scanAnnotatedTags( + { localRef: "refs/tags/loop", localSha: tagSha, remoteRef: "refs/tags/loop", remoteSha: zero }, + fakeGit({ + [`cat-file -t ${tagSha}`]: "tag\n", + [`cat-file tag ${tagSha}`]: selfReferential, + }), + ), + ).toEqual([]); + }); + it("refuses when the peel budget runs out with a tag still unscanned", () => { + // Exhaustion must fail closed, exactly as the alias hop limit does. A push + // of the tag ref publishes every object in the chain, so returning the + // findings gathered so far would report an unscanned tail clean. + const chain = Array.from({ length: TAG_PEEL_LIMIT + 1 }, (_, index) => + // +1 so the first object is not the all-zero null sha, which reads as a + // deletion and would return before the walk starts. + (index + 1).toString(16).padStart(2, "0").repeat(20), + ); + const responses: Record = { [`cat-file -t ${commitSha}`]: "commit\n" }; + chain.forEach((objectSha, index) => { + const next = chain[index + 1] ?? commitSha; + responses[`cat-file -t ${objectSha}`] = "tag\n"; + responses[`cat-file tag ${objectSha}`] = [ + `object ${next}`, + next === commitSha ? "type commit" : "type tag", + `tag v${index}`, + "tagger T 1700000000 +0000", + "", + "nothing interesting", + ].join("\n"); + }); + + expect(() => + scanAnnotatedTags( + { localRef: "refs/tags/v0", localSha: chain[0]!, remoteRef: "refs/tags/v0", remoteSha: zero }, + fakeGit(responses), + ), + ).toThrow(GitEgressScanError); + }); + + it("scans a chain that exactly fills the budget rather than refusing it", () => { + // The budget is checked after the type read, so a chain of exactly + // TAG_PEEL_LIMIT tags terminating in a commit is scanned to the end. The + // material sits on the DEEPEST tag, so a loop that stopped one short would + // return no findings and this would pass for the wrong reason. + const chain = Array.from({ length: TAG_PEEL_LIMIT }, (_, index) => + // +1 so the first object is not the all-zero null sha, which reads as a + // deletion and would return before the walk starts. + (index + 1).toString(16).padStart(2, "0").repeat(20), + ); + const responses: Record = { [`cat-file -t ${commitSha}`]: "commit\n" }; + chain.forEach((objectSha, index) => { + const deepest = index === chain.length - 1; + const next = chain[index + 1] ?? commitSha; + responses[`cat-file -t ${objectSha}`] = "tag\n"; + responses[`cat-file tag ${objectSha}`] = [ + `object ${next}`, + deepest ? "type commit" : "type tag", + `tag v${index}`, + "tagger T 1700000000 +0000", + "", + deepest ? `Release\n\n${environmentDump()}\n` : "nothing interesting", + ].join("\n"); + }); + + const findings = scanAnnotatedTags( + { localRef: "refs/tags/v0", localSha: chain[0]!, remoteRef: "refs/tags/v0", remoteSha: zero }, + fakeGit(responses), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.commit).toBe(chain[chain.length - 1]); + }); + + it("redacts a tag name before it reaches the finding", () => { + // The name is author-chosen free text and is interpolated into the + // `git tag -f -a ` remedy, so it is the same disclosure path as a + // commit subject. + const token = vendorKey(); + const findings = scanAnnotatedTags( + { localRef: "refs/tags/v1", localSha: tagSha, remoteRef: "refs/tags/v1", remoteSha: zero }, + fakeGit({ + [`cat-file -t ${tagSha}`]: "tag\n", + [`cat-file tag ${tagSha}`]: tagObject(`Release\n\n${environmentDump()}\n`, `rel-${token}`), + [`cat-file -t ${commitSha}`]: "commit\n", + }), + ); + expect(findings).toHaveLength(1); + expect(findings[0]!.subject).not.toContain(token); + expect(findings[0]!.subject).toContain("redacted: vendor-key"); + }); +}); + +describe("formatRefusal", () => { + it("names the commit, the class, and a reachable remedy", () => { + const message = formatRefusal([ + { + commit: "f".repeat(40), + shortCommit: "ffffffffffff", + subject: "add fixture", + where: "content", + classes: ["environment-dump"], + }, + ]); + // A bare rejection is not actionable: the author has to be able to locate + // the commit and know which remedy applies. + expect(message).toContain("ffffffffffff"); + expect(message).toContain("environment-dump"); + expect(message).toContain("file content"); + expect(message).toContain("--amend"); + expect(message).toContain("content-addressed"); + }); + + it("points a multi-commit refusal at the oldest one", () => { + const message = formatRefusal([ + { + commit: "a".repeat(40), + shortCommit: "aaaaaaaaaaaa", + subject: "newer", + where: "message", + classes: ["environment-dump"], + }, + { + commit: "b".repeat(40), + shortCommit: "bbbbbbbbbbbb", + subject: "older", + where: "message", + classes: ["environment-dump"], + }, + ]); + // rev-list is newest-first, so the last finding is the oldest commit and is + // the one an interactive rebase has to reach. + expect(message).toContain("rebase -i bbbbbbbbbbbb~1"); + }); + + it("does not reprint credential-shaped material carried in a subject", () => { + // Second barrier. The findings this renderer receives from `scanCommit` + // arrive scrubbed, so this constructs an UNSCRUBBED one on purpose: the + // renderer is the only thing here that writes to a human-visible sink, and + // it must not depend on every construction site having remembered. + const token = vendorKey(); + const message = formatRefusal([ + { + commit: "f".repeat(40), + shortCommit: "ffffffffffff", + subject: `fix: drop ${token} from config`, + where: "message", + classes: ["vendor-key"], + }, + ]); + expect(message).not.toContain(token); + expect(message).toContain("redacted: vendor-key"); + expect(message).toContain("ffffffffffff"); + }); + + it("does not reprint a credential-shaped tag name in the retag remedy", () => { + const token = vendorKey(); + const message = formatRefusal([ + { + commit: "a".repeat(40), + shortCommit: "aaaaaaaaaaaa", + subject: `tag rel-${token}`, + where: "tag-message", + classes: ["vendor-key"], + }, + ]); + expect(message).toContain("git tag -f -a"); + expect(message).not.toContain(token); + }); +}); diff --git a/packages/adapter-utils/src/github-git-egress-shim.ts b/packages/adapter-utils/src/github-git-egress-shim.ts new file mode 100644 index 000000000000..cdb88c77070f --- /dev/null +++ b/packages/adapter-utils/src/github-git-egress-shim.ts @@ -0,0 +1,1345 @@ +// PEN-3156: the THIRD egress door. PEN-2527 put `scrubGitHubEgressText` in front +// of the GitHub CLI and PEN-3152 put it in front of the `github` MCP server. The +// `git` wrapper the Helm seed writes alongside them reaches the same destination +// with the same seat token and, on a push, publishes commit messages and file +// contents — a strict superset of what `create_or_update_file` / `push_files` +// carry — with no scrubber anywhere on the path. +// +// This door cannot be fixed the way the other two were. Both of those rewrite a +// payload in flight, which is available to them precisely because nothing has +// hashed it yet. A commit object is content-addressed: altering a blob or a +// message after the fact changes that commit's SHA and every descendant's, +// breaks any signature over them, and desynchronises the agent's local ref from +// what landed. So the enforcement here is REFUSAL, not redaction — the push is +// stopped and the author is told which commit to amend. +// +// Everything in this module is pure. Git is reached only through the injected +// `runGit` callback, so the classification and scanning rules are testable +// without a repository, a remote, or a network. + +import { + type GitHubEgressScrubClass, + scrubGitHubEgressText, +} from "./github-egress-scrub.js"; + +/** + * Git global options that consume a SEPARATE following token. + * + * Getting this set wrong is not cosmetic: it shifts which token is read as the + * subcommand. `git -c foo=bar push` would classify as a `foo=bar` subcommand + * and the push would sail past the guard, so this list is the guard's integrity + * rather than a parsing nicety. The `--opt=value` spelling is self-contained and + * is handled separately. + * + * ONE KNOWN MISMATCH WITH GIT, and it is safe in the one direction that + * matters. `--exec-path` takes a value only in its `=` form: bare + * `--exec-path` PRINTS the exec path and exits without consuming the next + * token. So for `git --exec-path push origin main` this scan skips `push` as a + * value and classifies `origin` as the subcommand — not a push, no hook + * injected. That costs nothing, because git does not push on that invocation + * either: measured against git 2.47.3, it printed `/usr/lib/git-core`, exited + * 0, and left the remote with no refs. An invocation that publishes nothing + * cannot be a hole. + * + * Do not read the rest of this set as an exact model of git's parser on the + * strength of that one. The property the set needs is narrower: over-skipping + * is safe only while the over-skipped invocation publishes nothing, and + * UNDER-skipping is what opens a hole. Anything added here should be checked + * against git for which of the two it can do. + */ +const VALUE_TAKING_GLOBAL_OPTIONS: ReadonlySet = new Set([ + "-C", + "-c", + "--exec-path", + "--git-dir", + "--work-tree", + "--namespace", + "--super-prefix", + "--config-env", + "--attr-source", +]); + +/** The flag that skips the pre-push hook. */ +const NO_VERIFY_FLAG = "--no-verify"; + +/** + * True for any token git would resolve to `--no-verify`. + * + * Matching the fully-spelled flag alone was a measured bypass. Git's + * `parse-options` accepts any unambiguous long-option ABBREVIATION, so + * `--no-veri` and `--no-verif` are `--no-verify` to git while matching no + * literal spelling here. Measured against git 2.47.3: + * + * git -c core.hooksPath= push --no-veri origin HEAD:refs/heads/t + * + * printed no hook output and landed the ref on the remote, while the same push + * without the flag ran the hook and was refused. The alias leg had the same gap + * (`alias.q = push --no-veri`). + * + * So this tests "could this token be that flag?" rather than enumerating + * spellings — the enumeration is exactly the parser-disagrees-with-git failure + * this module sets out to avoid, and a new git release could shorten the + * accepted prefix without anything here changing. `--no-ver` and shorter are + * ambiguous with `--no-verbose` and git rejects them outright; refusing them + * too costs nothing, because no command git accepts is being turned away. + * + * The length floor keeps the bare `--` end-of-options separator out. + * + * `-n` is deliberately NOT matched. For `push` it means `--dry-run`, not + * `--no-verify` (see the subcommand's own `-h` output), and it is harmless + * twice over: the pre-push hook still runs under it, and a dry run publishes + * nothing even if it did not. Refusing it would reject a safe command while + * telling the author something untrue about why. + * + * The global-option scan needs no equivalent: git does NOT abbreviate those. + * Measured — `--config-e`, `--config-en` and `--exec-p` are each rejected with + * `unknown option`. + */ +function isNoVerifyFlag(token: string): boolean { + return token.length > 2 && NO_VERIFY_FLAG.startsWith(token); +} + +/** The config key whose value decides which directory git reads hooks from. */ +const HOOKS_PATH_KEY = "core.hookspath"; + +/** The config section under which an assignment defines an alias. */ +const ALIAS_KEY_PREFIX = "alias."; + +/** + * Git verbs that do NOT publish to a remote, and so may pass through unguarded. + * + * This is an allowlist, and the direction is the point. The obvious shape is a + * denylist — refuse `send-pack`, guard `push`, pass everything else — and it + * was the shape this guard shipped with, as `subcommand === "push"`. It fails + * OPEN on every verb nobody thought of, which is not a hypothetical: `git + * send-pack ` classified as not-a-push, went through untouched, and + * published. Measured against git 2.47.3, `git -c core.hooksPath= + * send-pack HEAD:refs/heads/x` printed `* [new branch]`, landed the + * ref, and produced no hook output at all — the guard's own injected config was + * present and made no difference, because `send-pack` is plumbing and never + * consults the pre-push hook. Only `push` runs it. + * + * So an unrecognised verb refuses. The cost is a loud, self-describing refusal + * on a verb that turns out to be harmless, which an operator fixes by adding it + * here. The cost of the other direction is a silent publish, which nobody sees. + * + * Derived from `git --list-cmds=builtins,main,others,nohelpers` at git 2.47.3 + * (157 verbs), less three groups held back deliberately: + * + * publishing `push` (guarded, not listed here), `send-pack`, `http-push`, + * and `subtree`, whose `push` mode shells out to a bare `git` + * that resolves to git's own exec-path rather than back through + * this wrapper. + * transport `remote-http{,s}`, `remote-ext`, `remote-fd`, `remote-ftp{,s}`. + * Invoked directly these speak the transport protocol on stdin, + * which includes push. + * server/egress `daemon`, `http-backend`, `instaweb`, `shell`, `receive-pack`, + * `upload-pack`, `upload-archive`, `imap-send`. None has an + * agent use, and each either serves the repository to the + * network or sends its contents somewhere. + * + * A third-party verb (`git lfs`, `git flow`) is unrecognised and therefore + * refused. That is correct for those two specifically — both publish. + */ +const NON_PUBLISHING_GIT_VERBS: ReadonlySet = new Set([ + "add", "am", "annotate", "apply", "archive", "bisect", "blame", "branch", + "bugreport", "bundle", "cat-file", "check-attr", "check-ignore", + "check-mailmap", "check-ref-format", "checkout", "checkout-index", "cherry", + "cherry-pick", "clean", "clone", "column", "commit", "commit-graph", + "commit-tree", "config", "count-objects", "credential", "credential-cache", + "credential-store", "describe", "diagnose", "diff", "diff-files", + "diff-index", "diff-tree", "difftool", "fast-export", "fast-import", "fetch", + "fetch-pack", "filter-branch", "fmt-merge-msg", "for-each-ref", + "for-each-repo", "format-patch", "fsck", "fsck-objects", "gc", + "get-tar-commit-id", "grep", "hash-object", "help", "hook", "http-fetch", + "index-pack", "init", "init-db", "interpret-trailers", "log", "ls-files", + "ls-remote", "ls-tree", "mailinfo", "mailsplit", "maintenance", "merge", + "merge-base", "merge-file", "merge-index", "merge-octopus", "merge-one-file", + "merge-ours", "merge-recursive", "merge-recursive-ours", + "merge-recursive-theirs", "merge-resolve", "merge-subtree", "merge-tree", + "mergetool", "mktag", "mktree", "multi-pack-index", "mv", "name-rev", "notes", + "pack-objects", "pack-redundant", "pack-refs", "patch-id", "pickaxe", "prune", + "prune-packed", "pull", "quiltimport", "range-diff", "read-tree", "rebase", + "reflog", "refs", "remote", "repack", "replace", "replay", "request-pull", + "rerere", "reset", "restore", "rev-list", "rev-parse", "revert", "rm", + "shortlog", "show", "show-branch", "show-index", "show-ref", + "sparse-checkout", "stage", "stash", "status", "stripspace", "switch", + "symbolic-ref", "tag", "unpack-file", "unpack-objects", "update-index", + "update-ref", "update-server-info", "var", "verify-commit", "verify-pack", + "verify-tag", "version", "whatchanged", "worktree", "write-tree", +]); + +/** + * Verbs known to publish, as opposed to merely unrecognised. + * + * Enforcement does NOT read this set — {@link NON_PUBLISHING_GIT_VERBS} decides + * that, so a publishing verb missing from here is still refused. It exists only + * so the refusal can say "this publishes without running the hook" where that + * is known, instead of the weaker "I do not recognise this", which reads like a + * tooling gap rather than a security decision and invites the wrong fix. + */ +const KNOWN_PUBLISHING_GIT_VERBS: ReadonlySet = new Set([ + "push", "send-pack", "http-push", "subtree", "remote-http", "remote-https", + "remote-ext", "remote-fd", "remote-ftp", "remote-ftps", +]); + +/** + * True when `verb` is one git will run without any chance of publishing. + * + * Case-sensitive, matching git: `git STATUS` is not `git status` (measured + * against git 2.47.3 — `git: 'STATUS' is not a git command`). Folding here + * would admit spellings git itself rejects, which is a widening with no caller. + */ +export function isNonPublishingGitVerb(verb: string): boolean { + return NON_PUBLISHING_GIT_VERBS.has(verb); +} + +/** True when `verb` is a verb this guard knows publishes to a remote. */ +export function isKnownPublishingGitVerb(verb: string): boolean { + return KNOWN_PUBLISHING_GIT_VERBS.has(verb); +} + +/** + * How many alias hops resolution will walk before refusing. + * + * Bounded rather than recursive because the config defining the chain is + * agent-writable, so an unbounded walk is a denial of service on ourselves. The + * bound is safe only because exhausting it REFUSES; see the post-loop check in + * {@link classifyGitInvocation}, where falling through instead was a measured + * hook bypass. + */ +export const ALIAS_HOP_LIMIT = 4; + +/** + * How many objects an annotated-tag chain is peeled through before refusing. + * + * Bounded for the same reason as {@link ALIAS_HOP_LIMIT} — the chain is + * agent-writable and a cycle must not spin a push forever — and safe for the + * same reason: exhausting it refuses. Set well above any legitimate shape; a + * tag pointing at a tag is already unusual and a chain of sixteen has no honest + * use. + */ +export const TAG_PEEL_LIMIT = 16; + +/** + * The environment `--config-env` reads through. + * + * Narrower than `NodeJS.ProcessEnv` on purpose: this module is pure, and taking + * the lookup as data keeps `--config-env` testable without mutating the real + * environment. + */ +export type GitEgressEnv = Readonly>; + +/** The key half of a `=` config assignment, case-folded. */ +function assignmentKey(assignment: string): string { + return (assignment.split("=", 1)[0] ?? "").trim().toLowerCase(); +} + +/** + * True when a `=` config assignment targets `core.hooksPath`. + * + * The comparison is case-folded because git config keys are case-insensitive in + * their section and variable names: `-c CORE.HOOKSPATH=...` sets exactly the + * same key as `-c core.hooksPath=...`, and a case-sensitive check here would see + * only one of the two spellings. Verified against git 2.47.3. + */ +function isHooksPathAssignment(assignment: string): boolean { + return assignmentKey(assignment) === HOOKS_PATH_KEY; +} + +/** + * The alias a `=` config assignment defines, or null. + * + * Case-folded for the same reason {@link isHooksPathAssignment} is, and + * measured the same way: against git 2.47.3, `-c alias.YOLO=...` defines the + * alias `git yolo` runs and `-c ALIAS.zz=...` defines `git zz`, so a + * case-sensitive match here would see one spelling of three. + * + * An assignment carrying no `=` defines nothing. Git rejects `-c alias.b` + * outright (`missing value for 'alias.b'`, `fatal: unable to parse command-line + * config`), so there is no boolean-true alias to model. + */ +function aliasAssignment( + assignment: string, + options: { fromEnv: boolean; env: GitEgressEnv }, +): { name: string; expansion: string } | null { + const separator = assignment.indexOf("="); + if (separator < 0) return null; + const key = assignmentKey(assignment); + if (!key.startsWith(ALIAS_KEY_PREFIX)) return null; + const name = key.slice(ALIAS_KEY_PREFIX.length); + if (!name) return null; + const raw = assignment.slice(separator + 1); + // `--config-env` names an environment variable; `-c` carries the value itself. + const expansion = options.fromEnv ? options.env[raw] : raw; + return expansion === undefined ? null : { name, expansion }; +} + +interface GlobalOptionScan { + /** Index of the first token that is not a global option or its value. */ + subcommandIndex: number; + /** A caller-supplied `core.hooksPath` override, as spelled, or null. */ + hooksPathOverride: string | null; + /** Aliases this token run defines, keyed by case-folded name. */ + aliasDefinitions: Map; +} + +/** + * Walk git's global options, reporting where the subcommand starts, whether the + * caller set `core.hooksPath` along the way, and which aliases they defined. + * + * All three outputs are security-relevant. Getting the option boundary wrong + * shifts which token reads as the subcommand, so `git -c foo=bar push` would + * classify as a `foo=bar` subcommand and sail past the guard. Missing a + * `core.hooksPath` override lets the caller nominate the hooks directory + * themselves. Missing an alias DEFINITION is the subtler one and is why + * `aliasDefinitions` exists at all: an alias defined here is invisible to a + * separate `git config --get`, so it cannot be looked up after the fact — see + * {@link classifyGitInvocation}. + * + * Only the separate-token `-c =` spelling is modelled because it is + * the only one git accepts: `-calias.x=push` is rejected with `unknown option` + * (git 2.47.3), so there is no attached short form to miss. + */ +function scanGlobalOptions( + tokens: readonly string[], + env: GitEgressEnv = {}, +): GlobalOptionScan { + let index = 0; + let hooksPathOverride: string | null = null; + const aliasDefinitions = new Map(); + const define = (assignment: string, fromEnv: boolean) => { + const alias = aliasAssignment(assignment, { fromEnv, env }); + // Last wins, matching git: `-c alias.d=status -c alias.d=push` runs push. + if (alias) aliasDefinitions.set(alias.name, alias.expansion); + }; + while (index < tokens.length) { + const token = tokens[index]!; + if (!token.startsWith("-")) break; + // `--opt=value` carries its own value; `--opt value` and `-c x=y` do not. + if (VALUE_TAKING_GLOBAL_OPTIONS.has(token)) { + const value = tokens[index + 1]; + if (value !== undefined && (token === "-c" || token === "--config-env")) { + if (isHooksPathAssignment(value)) hooksPathOverride ??= `${token} ${value}`; + define(value, token === "--config-env"); + } + index += 2; + continue; + } + if (token.startsWith("--config-env=")) { + const assignment = token.slice("--config-env=".length); + if (isHooksPathAssignment(assignment)) hooksPathOverride ??= token; + define(assignment, true); + } + index += 1; + } + return { subcommandIndex: index, hooksPathOverride, aliasDefinitions }; +} + +/** + * The leading global options of an invocation, up to but excluding the + * subcommand. + * + * Exported so the wrapper can resolve aliases under the same effective + * configuration git itself will use — `-C`, `--git-dir` and friends all select + * WHICH config files an alias lookup reads, and a bare `git config --get` reads + * the wrong ones. + */ +export function gitGlobalOptions(argv: readonly string[]): string[] { + return argv.slice(0, scanGlobalOptions(argv).subcommandIndex); +} + +/** A hook bypass carried by an alias expansion rather than by argv. */ +export interface GitAliasBypass { + /** The alias the caller invoked. */ + alias: string; + /** Its expansion, so the refusal can quote what git would have run. */ + expansion: string; + /** + * Which bypass the expansion carries. + * + * `unquotable` is the fail-closed case: the expansion could not be tokenised + * the way git would tokenise it, so no claim about its contents is sound. + * + * `alias-depth` is the other one: resolution ran out of hops with the chain + * still unresolved, so whether it reaches a push went unanswered. Neither + * asserts a bypass is present — both say the question could not be settled, + * which is why both refuse without waiting for `isPush`. + */ + reason: "no-verify" | "hooks-path" | "unquotable" | "alias-depth"; + /** + * The alias names walked, in order, when the reason is `alias-depth`. + * + * Carried so the refusal can name the chain rather than just its head — the + * author has to find the definition to fix it. + */ + chain?: readonly string[]; +} + +/** + * Split an alias expansion the way git splits it, or return null. + * + * Git does NOT split an alias on whitespace. It runs the expansion through + * `split_cmdline()` (`alias.c`), which applies shell-style quoting, so the + * tokens git acts on are not the tokens a `/\s+/` split produces. That gap was + * a live bypass, measured against git 2.47.3: + * + * git -c core.hooksPath= -c 'alias.q=push "--no-verify"' q origin HEAD:t + * + * split on whitespace yields the token `"--no-verify"` WITH its quotes, which + * matches no spelling {@link isNoVerifyFlag} accepts, so the expansion read as an + * ordinary push and the guard injected its hooks path as usual. Git dequoted it + * to `--no-verify`, skipped the hook, exited 0, and the ref landed on the + * remote. The scanner never ran. + * + * Rules implemented, matching `split_cmdline`: single quotes are literal to the + * next single quote; double quotes run to the next unescaped double quote and + * honour backslash escapes; a backslash outside quotes escapes the next + * character; quoted and bare runs concatenate within one token (`"--no-ver"ify` + * is one token, `--no-verify`). + * + * Returns null on an unterminated quote — which git rejects outright — so the + * caller can fail closed. Guessing at a string git itself will not parse is how + * a bypass gets waved through by a scanner that believed it understood the + * command. + */ +export function splitAliasExpansion(expansion: string): string[] | null { + const tokens: string[] = []; + let current = ""; + let started = false; + let index = 0; + + while (index < expansion.length) { + const char = expansion[index]!; + + if (/\s/.test(char)) { + if (started) { + tokens.push(current); + current = ""; + started = false; + } + index += 1; + continue; + } + + started = true; + + if (char === "'") { + const end = expansion.indexOf("'", index + 1); + if (end === -1) return null; + current += expansion.slice(index + 1, end); + index = end + 1; + continue; + } + + if (char === '"') { + index += 1; + let closed = false; + while (index < expansion.length) { + const inner = expansion[index]!; + if (inner === "\\" && index + 1 < expansion.length) { + current += expansion[index + 1]!; + index += 2; + continue; + } + if (inner === '"') { + closed = true; + index += 1; + break; + } + current += inner; + index += 1; + } + if (!closed) return null; + continue; + } + + if (char === "\\" && index + 1 < expansion.length) { + current += expansion[index + 1]!; + index += 2; + continue; + } + + current += char; + index += 1; + } + + if (started) tokens.push(current); + return tokens; +} + +/** + * A verb that reached the wrapper without being cleared as non-publishing. + * + * Covers two cases the refusal wording distinguishes but enforcement does not: + * a verb known to publish (`send-pack`), and one simply not on the allowlist. + * Both refuse, because the second cannot be shown to be safe. + */ +export interface GitPublishVerb { + /** The verb git would actually have run. */ + verb: string; + /** True when {@link isKnownPublishingGitVerb} recognises it. */ + known: boolean; + /** The alias the caller typed, when the verb was reached through one. */ + alias: string | null; + /** + * The alias names walked, ending at {@link verb}, when `alias` is set. + * + * Carried for the same reason `alias-depth` carries one: the author has to + * find the definition to fix it, and the head of the chain is rarely where + * the publishing verb is written. + */ + chain?: readonly string[]; +} + +export interface GitInvocationClassification { + /** The resolved subcommand, or null when argv carries only global options. */ + subcommand: string | null; + /** True when this invocation would contact a remote to publish refs. */ + isPush: boolean; + /** True when the invocation asks git to skip hooks. */ + hasNoVerify: boolean; + /** Index in argv at which the subcommand was found, or -1. */ + subcommandIndex: number; + /** + * A caller-supplied `core.hooksPath` override among argv's global options. + * + * Injecting the guard last already beats this one (git takes the LAST `-c` + * for a key), but it is reported so the wrapper can refuse it explicitly + * rather than silently discarding what the caller asked for. + */ + hooksPathOverride: string | null; + /** + * A bypass inside an alias expansion. Unlike the argv case this CANNOT be + * beaten by injection: git expands the alias after the command line, so the + * expansion's own `-c core.hooksPath=` or `--no-verify` wins. + */ + aliasBypass: GitAliasBypass | null; + /** + * A `!`-prefixed shell alias reached while resolving the subcommand. + * + * Set whether or not the chain reached a push, because for a shell alias that + * question is unanswerable: the expansion is arbitrary shell, and deciding + * whether it publishes would mean parsing it. The wrapper refuses on this + * rather than guessing. See `classifyGitInvocation` for why passing it through + * is not an option. + */ + shellAlias: GitShellAlias | null; + /** + * A verb that is neither `push` nor on the non-publishing allowlist. + * + * Set whether or not the chain reached a push — it is set precisely when it + * did NOT — so, like `shellAlias`, it must be refused ahead of the + * not-a-push early return. Gating it on `isPush` would discard every one of + * them, which is the bug it exists to close. + */ + publishVerb: GitPublishVerb | null; +} + +export interface GitShellAlias { + /** The alias the caller invoked. */ + alias: string; + /** Its expansion, so the refusal can quote what git would have run. */ + expansion: string; +} + +/** + * Split argv into git's global options and its subcommand. + * + * Aliases are resolved when the subcommand is not `push` — `git -c + * alias.yolo=push yolo` is otherwise a complete bypass of this guard. + * Resolution has two sources, and the order between them is the fix for a + * measured hole: + * + * 1. Aliases the invocation DEFINES ITSELF, via `-c alias.x=...` or + * `--config-env=alias.x=VAR`. These come first because git resolves them + * first, and because they are invisible to any lookup made in a separate + * process: `git -c alias.yolo='push --no-verify' yolo` pushes, while a + * plain `git config --get alias.yolo` beside it exits 1 with no output. + * Asking `resolveAlias` alone therefore returns nothing, `yolo` classifies + * as not-a-push, and the argv is handed to git untouched — which then + * expands the alias and skips the hook. Measured end to end against git + * 2.47.3: the push landed on the remote with the hook never running. + * 2. `resolveAlias`, for aliases that live in config files. Cheap: a local + * config read with no network. + * + * Definitions accumulate ACROSS hops, because an expansion's own global options + * define aliases too — `alias.outer = -c alias.inner=push inner` reaches a push + * in git 2.47.3, so dropping the inner definition would lose the chain. + * + * Resolution is bounded rather than recursive: git permits an alias to expand to + * another alias, and an unbounded loop here would be a denial-of-service on a + * config the agent controls. + * + * An expansion is parsed with the same global-option scan as argv, not by + * reading its first word. `alias.sneaky = -c core.hooksPath=/tmp/empty push` + * expands to a push whose first word is `-c`, so a first-word test classifies it + * as not-a-push and the guard is never injected at all — measured as a working + * bypass against git 2.47.3. + */ +export function classifyGitInvocation( + argv: readonly string[], + resolveAlias?: (name: string) => string | null, + env: GitEgressEnv = {}, +): GitInvocationClassification { + const globals = scanGlobalOptions(argv, env); + + if (globals.subcommandIndex >= argv.length) { + return { + subcommand: null, + isPush: false, + hasNoVerify: false, + subcommandIndex: -1, + hooksPathOverride: globals.hooksPathOverride, + aliasBypass: null, + shellAlias: null, + publishVerb: null, + }; + } + + const subcommandIndex = globals.subcommandIndex; + const subcommand = argv[subcommandIndex]!; + const rest = argv.slice(subcommandIndex + 1); + const hasNoVerify = rest.some(isNoVerifyFlag); + + let isPush = subcommand === "push"; + // Accumulated across hops, then kept only if the chain reaches a push: a + // bypass on an alias that never publishes anything is not this guard's + // business, and refusing it would break unrelated tooling. + let pendingBypass: GitAliasBypass | null = null; + let shellAlias: GitShellAlias | null = null; + // Set when alias resolution runs out of hops with the chain still live. Like + // `unquotable`, it means "is this a push?" went unanswered, so the refusal + // must not be gated on `isPush` — that is the very thing not known. + let aliasDepthExhausted = false; + // The verb git will actually run, when it is neither `push` nor cleared as + // non-publishing. Set at the one place the walk learns what that verb is. + let publishVerb: GitPublishVerb | null = null; + + if (!isPush) { + const definitions = new Map(globals.aliasDefinitions); + // Command-line definitions beat config files, as they do in git. + const lookup = (name: string): string | null => + definitions.get(name.toLowerCase()) ?? resolveAlias?.(name) ?? null; + + let name: string | null = subcommand; + // Names walked, for the refusal message when the cap is exhausted. + const chain: string[] = []; + let hop = 0; + for (; hop < ALIAS_HOP_LIMIT && name && !isPush; hop += 1) { + const expansion = lookup(name); + // No expansion means `name` is not an alias, so it is the verb git will + // actually run — and this is the ONLY place in the walk where that is + // known. Testing here covers the bare-argv leg (`hop === 0`, `name` is + // the typed subcommand) and the alias-expansion leg (`hop > 0`, `name` + // came out of an expansion) with one test. Ally's finding named the two + // legs as separate call sites; they are deliberately not implemented as + // two, because two tests of the same property are two things that can + // drift, and the four bypasses already closed on this path were all + // drift of exactly that kind. + if (!expansion) { + if (!isNonPublishingGitVerb(name)) { + publishVerb = { + verb: name, + known: isKnownPublishingGitVerb(name), + alias: hop === 0 ? null : subcommand, + ...(hop === 0 ? {} : { chain: [...chain, name] }), + }; + } + break; + } + chain.push(name); + // A `!`-prefixed alias is an arbitrary shell command, and it is the one + // expansion that escapes this guard completely — so it is recorded for + // refusal rather than passed through. + // + // Passing it through used to be justified on the theory that a bare `git` + // inside the expansion would re-enter the wrapper through PATH. Measured + // against git 2.47.3, that is false: git PREPENDS its exec-path to PATH + // for the shell it spawns, and `/usr/lib/git-core` ships a complete `git` + // binary. So inside a shell alias, a bare `git push` resolves to + // /usr/lib/git-core/git — the real one — and neither the wrapper nor the + // hook is reached. No absolute path is needed for the bypass; the alias + // supplies it. That makes this the dangerous shape: an ordinary-looking + // `git ` that silently is not guarded. + // + // Refusal is the only sound response. Deciding whether the expansion + // publishes would mean parsing arbitrary shell, and a textual test for + // `push` is defeated by any indirection. Refusing every shell alias is + // the conservative direction, and it is cheap: no shell alias is defined + // in any config the agent image ships. + if (expansion.startsWith("!")) { + shellAlias = { alias: name, expansion }; + break; + } + + // Tokenised the way git tokenises an alias, not on whitespace. See + // `splitAliasExpansion`: a `/\s+/` split leaves quotes attached, and a + // quoted `"--no-verify"` then matched nothing and pushed unscanned. + const tokens = splitAliasExpansion(expansion); + if (tokens === null) { + // Unterminated quote. Git rejects this, so it cannot reach a push — but + // the guard must not be the component that decides that on a guess, and + // a parser disagreeing with git in the permissive direction is exactly + // the failure this whole function exists to avoid. + if (!pendingBypass) pendingBypass = { alias: name, expansion, reason: "unquotable" }; + break; + } + const expansionGlobals = scanGlobalOptions(tokens, env); + for (const [alias, value] of expansionGlobals.aliasDefinitions) { + definitions.set(alias, value); + } + if (!pendingBypass && expansionGlobals.hooksPathOverride) { + pendingBypass = { alias: name, expansion, reason: "hooks-path" }; + } + + const expanded: string | null = tokens[expansionGlobals.subcommandIndex] ?? null; + const expandedRest = tokens.slice(expansionGlobals.subcommandIndex + 1); + if (!pendingBypass && expandedRest.some(isNoVerifyFlag)) { + pendingBypass = { alias: name, expansion, reason: "no-verify" }; + } + + if (expanded === "push") { + isPush = true; + break; + } + name = expanded; + } + + // The cap is a denial-of-service bound, not a claim that chains stop here — + // git resolves deeper. Leaving the loop by falling through as NOT-a-push + // was therefore a measured hole, and in the permissive direction: with + // `alias.a1=a2 … alias.a5=push`, hops 0-3 walk a1→a5 and the loop ends with + // `isPush` still false, so `buildGitArgv` returns argv untouched, no + // `core.hooksPath` is injected, and git then expands the whole chain to a + // push that runs no hook. Measured against git 2.47.3: `git a1 origin + // HEAD:refs/heads/deep5` landed the ref on the remote with the hook never + // running, while the same chain WITH the guard's `-c` did run it — which is + // what identifies the missing injection, rather than the depth itself, as + // the hole. + // + // Refusing is chosen over classifying it as a push. Injecting the hooks + // path would let the hook decide, but only for a chain whose unscanned tail + // carries no `--no-verify` — and the tail is unscanned precisely because + // the budget ran out, so that variant is closed only by assumption. + // Refusal holds in every case, and a chain this deep is not a shape any + // config the agent image ships defines. + // + // `hop` reaching the limit is what separates budget exhaustion from every + // `break` above, each of which leaves it short. The final lookup then + // separates "ran out of road" from "arrived": a chain ending in a name git + // would not resolve either is genuinely not a push, and must not refuse. + if (!isPush && !shellAlias && hop >= ALIAS_HOP_LIMIT && name) { + const unresolved = lookup(name); + if (unresolved !== null) { + aliasDepthExhausted = true; + chain.push(name); + if (!pendingBypass) { + pendingBypass = { + alias: subcommand, + expansion: unresolved, + reason: "alias-depth", + chain: [...chain], + }; + } + } + } + } + + return { + subcommand, + isPush, + hasNoVerify, + subcommandIndex, + hooksPathOverride: globals.hooksPathOverride, + // An alias bypass only matters on a push — EXCEPT for the two reasons that + // exist because "is it a push?" is itself the question that went + // unanswered, where gating on `isPush` would read the unanswered question + // as a "no": + // + // unquotable the expansion could not be tokenised the way git tokenises + // one. Git happens to reject an unclosed quote itself + // (measured: `fatal: bad alias.q string: unclosed quote`), so + // nothing publishes either way — but gating here would make + // this guard's safety depend on git's parser agreeing with + // ours. + // alias-depth resolution ran out of hops with the chain still live, so + // the tail that decides it was never read. Unlike the above, + // git does NOT reject this one: the chain resolves fine and + // pushes. + // + // Where the two parsers disagree the guard must be the stricter one; a + // visible refusal is the safe direction, a silent pass is not. + aliasBypass: + isPush || aliasDepthExhausted || pendingBypass?.reason === "unquotable" + ? pendingBypass + : null, + shellAlias, + publishVerb, + }; +} + +/** + * Raised when the hook's own stdin carries a ref-update line that does not + * parse. + * + * Separate from {@link GitEgressScanError} because nothing failed to READ here + * — git handed over input in a shape this parser does not recognise, which is a + * different fault with a different remedy. Both reach the same place: the + * runtime catches everything around the scan and refuses. + */ +export class GitEgressInputError extends Error { + constructor(readonly line: string) { + super( + `the pre-push hook received a ref update it could not parse (\`${line}\`), so the refs this push would publish could not be determined`, + ); + this.name = "GitEgressInputError"; + } +} + +export interface PrePushRefUpdate { + localRef: string; + localSha: string; + remoteRef: string; + remoteSha: string; +} + +/** git's all-zero sha, used for "this ref does not exist on the remote yet". */ +const NULL_SHA_RE = /^0{40,64}$/; + +export function isNullSha(sha: string): boolean { + return NULL_SHA_RE.test(sha); +} + +/** + * Parse the pre-push hook's stdin: one ` + * ` line per ref being updated. + * + * Using git's own computation rather than re-deriving the range from argv is + * deliberate — refspec resolution, `push.default`, and tracking configuration + * are git's to interpret, and a second implementation of them would disagree + * with the push that is actually about to happen. + * + * A non-empty line that does not parse THROWS rather than being skipped. Git's + * format is fixed and refs cannot contain whitespace, so this is not reachable + * today — but skipping was the one fail-open shape left in a module that is + * otherwise uniformly fail-closed, and it is the worst kind: `runPrePushHook` + * treats an empty update list as a pass, so a line that silently failed to + * parse would publish its ref unscanned rather than merely under-reporting. + * Refusing costs nothing while the case stays unreachable, and holds if a + * future git widens the format. + */ +export function parsePrePushInput(input: string): PrePushRefUpdate[] { + const updates: PrePushRefUpdate[] = []; + for (const rawLine of input.split("\n")) { + const line = rawLine.trim(); + if (!line) continue; + const parts = line.split(/\s+/); + if (parts.length < 4) throw new GitEgressInputError(line); + updates.push({ + localRef: parts[0]!, + localSha: parts[1]!, + remoteRef: parts[2]!, + remoteSha: parts[3]!, + }); + } + return updates; +} + +export interface GitPushFinding { + /** Full sha of the object carrying the material — a commit, or a tag object. */ + commit: string; + /** Abbreviated sha, for the message. */ + shortCommit: string; + /** + * Commit subject, so the author can recognise it without looking it up. + * + * ALREADY SCRUBBED at construction, and that is a security invariant rather + * than a formatting choice. For a `message` finding the subject IS the head of + * the text that fired the detector, so a one-line commit would otherwise carry + * the material back out through the refusal — into agent stderr, into run + * logs, and from there into whatever the agent pastes when it reports the + * refusal. That is PEN-2526's shape, reached from inside the control built to + * prevent it. Anything reading this field is reading redacted text. + */ + subject: string; + /** + * Where in the pushed object the material sits. + * + * `tag-message` is the annotated-tag object's own message, which is neither a + * commit message nor file content and is published by the tag ref update + * itself. It is a separate case because the remedy differs: a tag is retagged, + * not amended. + */ + where: "message" | "content" | "tag-message"; + /** Which scrub classes fired. */ + classes: GitHubEgressScrubClass[]; +} + +/** Reads git. Returns stdout, or null when the command failed. */ +export type GitReader = (args: string[]) => string | null; + +/** + * A git read the scanner needed in order to reach a verdict did not succeed. + * + * This exists so an unreadable repository cannot be mistaken for a clean one. + * Every read below decides either WHICH commits the push would publish or WHAT + * is inside one, so a failure leaves the scanner with no evidence — and "no + * evidence of credential-shaped material" is not the same statement as "no + * credential-shaped material". Treating the two as equivalent turns any git + * error, including a `maxBuffer` overflow on a large diff, into a silent pass + * at exactly the moment the push is biggest. + */ +export class GitEgressScanError extends Error { + constructor( + readonly command: readonly string[], + readonly commit?: string, + /** + * Overrides the failed-read wording for a refusal that is NOT a failed + * read. Everything on this path refuses, but the author is told why, and + * "git failed" would send them to look for a broken repository when the + * scan in fact ran out of budget. See {@link tagPeelExhausted}. + */ + reason?: string, + ) { + super( + reason ?? + `\`git ${command.join(" ")}\` failed, so ${ + commit ? `commit ${commit.slice(0, 12)}` : "the set of commits this push would publish" + } could not be read`, + ); + this.name = "GitEgressScanError"; + } + + /** + * The annotated-tag peel budget ran out with another tag object still in + * front of the scanner, so the rest of the chain would be published + * unscanned. + */ + static tagPeelExhausted(sha: string, limit: number): GitEgressScanError { + return new GitEgressScanError( + ["cat-file", "-t", sha], + sha, + `an annotated tag chain deeper than ${limit} objects reached ${sha.slice(0, 12)} with objects still unscanned; ` + + "a push of the tag ref publishes every object in the chain, so the unscanned tail cannot be reported clean. " + + "Recreate the tag so it points at its target directly rather than through a chain of tags.", + ); + } +} + +/** Run a read the verdict depends on, refusing rather than guessing on failure. */ +function readGit(runGit: GitReader, args: string[], commit?: string): string { + const output = runGit(args); + if (output === null) throw new GitEgressScanError(args, commit); + return output; +} + +/** + * Commits that a push would publish for one ref update. + * + * For an existing remote ref the range is `remoteSha..localSha`. For a ref the + * remote does not have, `--not --remotes` excludes everything already published + * under any remote-tracking ref, which is what keeps a new branch off a shared + * base from re-reporting the entire history of the repository. + * + * A known trade-off, recorded rather than fixed: `--remotes` is EVERY remote, + * not the push target's. A commit present only on a second remote — a fork, an + * upstream — is genuinely reaching this one for the first time and is skipped. + * The exclusion set is therefore agent-writable via `git remote add`, which sits + * oddly beside the hook's refusal to scope itself by remote URL. It stays as it + * is because the alternative is worse in the common case: scoping to the target + * remote re-reports every commit a branch shares with an already-published + * upstream, on every push, with nothing the author can amend. The residual is + * narrow — material already published to another remote, i.e. already disclosed + * once — where the alternative's cost falls on ordinary clean pushes. + * + * Throws {@link GitEgressScanError} if `rev-list` fails: without its output the + * scanner does not know what the push contains, and an empty list would read as + * "nothing to check" and pass. + */ +export function commitsForRefUpdate( + update: PrePushRefUpdate, + runGit: GitReader, +): string[] { + if (isNullSha(update.localSha)) return []; // a deletion publishes no content + const args = isNullSha(update.remoteSha) + ? ["rev-list", update.localSha, "--not", "--remotes"] + : ["rev-list", `${update.remoteSha}..${update.localSha}`]; + return readGit(runGit, args) + .split("\n") + .map((line) => line.trim()) + .filter((line) => line.length > 0); +} + +/** + * Reduce a unified diff to just the content it ADDS, with the `+` markers + * removed. + * + * This is load-bearing, not tidying. `scrubGitHubEgressText`'s environment-dump + * detector anchors each assignment to the start of a line + * (`^[ \t]*[A-Z][A-Z0-9_]{2,}=`), and every added line in a patch arrives + * prefixed with `+`. Scanning raw `git show` output would therefore be blind to + * an environment dump — which is the exact class that caused PEN-2526, the + * exposure this whole control descends from. Stripping the marker restores the + * anchor. + * + * Only added lines are kept. Context and removed lines are, by definition, + * already on the remote; reporting them would refuse a push for material the + * author cannot remove by amending anything in this range. + * + * The parse is a hunk-state machine rather than a prefix test, and that is the + * fix for a measured blind spot. Skipping every line starting with `+++` — the + * way to drop a `+++ b/path` file header — also discarded any ADDED CONTENT + * LINE whose own text begins with `++`, because git emits that as `+++...`. + * Measured: a file whose first line is `++` produced the patch line + * `+++`, and the token was gone before `scrubGitHubEgressText` ever saw + * it. That drops a real detection outright, because the vendor-key, JWT and + * long-assignment detectors are `\b`-anchored substring matches rather than + * line-anchored ones, so the line carried the whole finding. + * + * Tightening to `+++ ` with a trailing space does NOT fix it — content + * beginning `++ ` reproduces it exactly. What separates the two is position, + * not spelling: `---`/`+++` are headers only BEFORE the first `@@` of a file + * block. Inside a hunk every line carries a one-character origin marker, so a + * leading `+` is content no matter what follows it, and exactly one `+` comes + * off. + * + * Outside a hunk the `+++ ` header is dropped so a path is not mistaken + * for content, but any OTHER `+` line is still kept. Git emits no content + * outside a hunk, so that branch is unreachable on real `git show` output; it + * is there so malformed or synthetic input over-reports rather than + * under-reports. Every uncertainty in this function resolves toward scanning + * more, because the cost of a spurious line is a false refusal the author can + * read, and the cost of a dropped one is a credential on the remote. + */ +export function addedLinesFromPatch(patch: string): string { + const added: string[] = []; + let inHunk = false; + for (const line of patch.split("\n")) { + if (inHunk) { + if (line.startsWith("+")) { + added.push(line.slice(1)); + continue; + } + // The rest of a hunk body: context, removals, the no-newline marker, and + // the bare empty line git emits for an empty context line. + if (line === "" || line.startsWith(" ") || line.startsWith("-") || line.startsWith("\\")) { + continue; + } + // Anything else has left the body — a `diff --git` for the next file, or + // the next `@@`. Fall through to the out-of-hunk tests below. + inHunk = false; + } + if (line.startsWith("@@")) { + inHunk = true; + continue; + } + if (line.startsWith("+++ ")) continue; + if (line.startsWith("+")) added.push(line.slice(1)); + } + return added.join("\n"); +} + +/** + * Scan one commit's message and its introduced content. + * + * Both legs matter and they fail differently: PEN-2526 was an environment dump + * interpolated into prose, which here would land in a commit MESSAGE, while the + * file-content leg is what makes this door a superset of the MCP write tools. + * + * Content is scanned as the commit's own patch rather than as full file bodies, + * so the finding is attributable to the commit that introduced the material and + * the remedy is a rewrite of that commit. Material already present on the remote + * is out of scope here by construction: it has already been published, and + * re-reporting it would make every push refuse with nothing the author can do. + * + * Every read throws {@link GitEgressScanError} on failure rather than being + * skipped. An unreadable message or patch is an unscanned commit, and letting it + * through would mean the guard reports clean on precisely the commits it could + * not inspect. Note this is distinct from an EMPTY read: git exits zero with no + * output for a commit with an empty message or no diff, and that genuinely is + * nothing to scan. + */ +export function scanCommit(commit: string, runGit: GitReader): GitPushFinding[] { + const findings: GitPushFinding[] = []; + const shortCommit = commit.slice(0, 12); + // Scrubbed HERE rather than at the point it is printed, so the invariant + // belongs to the finding and not to one renderer. The subject of a one-line + // commit is the material that fired the detector; carrying it raw in the + // object would leak through any sink that later reads a finding, not just + // through `formatRefusal`. + const subject = scrubGitHubEgressText( + readGit(runGit, ["log", "-1", "--format=%s", commit], commit).trim(), + ).text; + + const message = readGit(runGit, ["log", "-1", "--format=%B", commit], commit); + if (message) { + const scrubbed = scrubGitHubEgressText(message); + if (scrubbed.redacted) { + findings.push({ commit, shortCommit, subject, where: "message", classes: scrubbed.classes }); + } + } + + // `--format=` suppresses the commit header so the message is not scanned + // twice and reported as two findings. `--no-color` keeps escape sequences out + // of the scrubbed text. `-m` makes merge commits emit a patch at all. + // + // `--text` and `--no-textconv` are SECURITY flags, not formatting ones. Both + // defeat a way for a file's real bytes never to reach this scanner, and each + // was verified against git 2.47 by committing the material and reading what + // `show` emitted: + // + // --text Without it git prints `Binary files ... differ` and NO `+` + // lines for anything it considers binary, so + // `addedLinesFromPatch` returns the empty string and the + // commit is reported clean. Two ways in: a single NUL byte in + // the first 8000 bytes makes any file binary, and a + // `.gitattributes` entry marking a path `-diff` does the same + // to a plain-ASCII one. The second is the wider hole — + // `* -diff` blanks the content leg for the WHOLE tree, needs + // no binary content at all, and is committed in the same push + // it hides. + // --no-textconv A `diff..textconv` in the repository's own config, + // bound to a path by `.gitattributes`, replaces a file's + // content with that command's output for display. It is + // ordinary repo config, so it is agent-writable, and it + // launders the bytes before the scanner ever sees them. + // + // Size policy is the reader's `maxBuffer`, and it fails closed: an oversized + // read leaves spawnSync with `ENOBUFS` and a null status, `makeGitReader` + // returns null, and `readGit` throws rather than scanning a truncated patch. + // That is deliberate — the alternative is scanning a prefix and calling the + // rest clean, which is worst exactly when the push is biggest. + const patch = readGit( + runGit, + ["show", "--format=", "--no-color", "-m", "--unified=0", "--text", "--no-textconv", commit], + commit, + ); + if (patch) { + const scrubbed = scrubGitHubEgressText(addedLinesFromPatch(patch)); + if (scrubbed.redacted) { + findings.push({ commit, shortCommit, subject, where: "content", classes: scrubbed.classes }); + } + } + + return findings; +} + +/** + * Scan the annotated-tag objects a ref update would publish. + * + * `commitsForRefUpdate` peels a tag to the commits it reaches, because that is + * what `rev-list` does — measured: `rev-list --not --remotes` returns + * the tagged COMMIT and never the tag object itself. So a tag object's own + * message is reachable by nothing `scanCommit` reads, while a ref update naming + * `refs/tags/` publishes that object verbatim, free-form message included. + * A generated release tag that interpolates build environment into its message + * is the PEN-2526 class exactly, on a ref update whose commits are all clean. + * + * Lightweight tags need nothing here: their ref points straight at a commit, so + * `cat-file -t` reports `commit` and the commit leg already covers it. + * + * The peel loop handles a tag pointing at a tag, which git permits. It is + * bounded rather than `while (true)`: a malformed or cyclic chain must not spin + * a push forever. Exhausting that bound REFUSES rather than returning what it + * has, for the same reason the alias hop limit does. Pushing `refs/tags/` + * sends every object in the chain, not just the first, so a chain longer than + * the budget would publish objects — messages included — that nothing read. + * Returning the findings so far would be a clean verdict on an unscanned tail, + * which is the permissive direction and contradicts this module's rule that + * every uncertainty resolves toward scanning more. + * + * The budget is checked AFTER the type read, so a chain that is exactly + * {@link TAG_PEEL_LIMIT} tags deep and then reaches a commit terminates + * normally. Only a further tag object — one this loop would have had to scan + * and cannot — refuses. + * + * Reads throw {@link GitEgressScanError} rather than being skipped, for the same + * reason as every other read on this path — an unreadable tag object is an + * unscanned tag object, and treating it as clean would report a pass on + * precisely what could not be inspected. + */ +export function scanAnnotatedTags( + update: PrePushRefUpdate, + runGit: GitReader, +): GitPushFinding[] { + if (isNullSha(update.localSha)) return []; // a deletion publishes no object + + const findings: GitPushFinding[] = []; + let sha = update.localSha; + + for (let depth = 0; ; depth += 1) { + if (readGit(runGit, ["cat-file", "-t", sha], sha).trim() !== "tag") break; + + // A tag object is in front of us and the budget is gone: this one and + // everything behind it would be published unscanned. Refuse. + if (depth >= TAG_PEEL_LIMIT) throw GitEgressScanError.tagPeelExhausted(sha, TAG_PEEL_LIMIT); + + const raw = readGit(runGit, ["cat-file", "tag", sha], sha); + const shortCommit = sha.slice(0, 12); + const { name, message } = parseTagObject(raw); + + if (message) { + const scrubbed = scrubGitHubEgressText(message); + if (scrubbed.redacted) { + findings.push({ + commit: sha, + shortCommit, + // Scrubbed for the same reason a commit subject is: a tag name is + // author-chosen free text that ends up in the refusal, and in the + // `git tag -f -a ` remedy line. + subject: name ? `tag ${scrubGitHubEgressText(name).text}` : "annotated tag", + where: "tag-message", + classes: scrubbed.classes, + }); + } + } + + // Follow `object` to whatever this tag points at; a commit ends the walk on + // the next iteration's type check. + const target = /^object ([0-9a-f]{40,64})$/m.exec(raw)?.[1]; + if (!target || target === sha) break; + sha = target; + } + + return findings; +} + +/** + * Split a raw tag object into its tag name and its message. + * + * The object is a header block (`object`, `type`, `tag`, `tagger`) terminated by + * ONE blank line, then the free-form message. Splitting on the first blank line + * rather than counting headers keeps this correct if git ever adds a header, and + * keeps header text out of the scanned body — a `tagger` line carries an email + * address, which should not be reported as a finding. + */ +function parseTagObject(raw: string): { name: string | null; message: string } { + const separator = raw.indexOf("\n\n"); + const header = separator === -1 ? raw : raw.slice(0, separator); + const message = separator === -1 ? "" : raw.slice(separator + 2); + return { name: /^tag (.+)$/m.exec(header)?.[1]?.trim() ?? null, message }; +} + +export function scanPrePushUpdates( + updates: readonly PrePushRefUpdate[], + runGit: GitReader, +): GitPushFinding[] { + const findings: GitPushFinding[] = []; + const seen = new Set(); + for (const update of updates) { + // Tag objects first: the annotated-tag leg is about the object the ref + // names, which `commitsForRefUpdate` peels away before the commit leg ever + // sees it. + for (const finding of scanAnnotatedTags(update, runGit)) { + // A tag pushed under two refs is one problem, not two. + if (seen.has(finding.commit)) continue; + seen.add(finding.commit); + findings.push(finding); + } + for (const commit of commitsForRefUpdate(update, runGit)) { + // A commit reachable from two pushed refs is one problem, not two. + if (seen.has(commit)) continue; + seen.add(commit); + findings.push(...scanCommit(commit, runGit)); + } + } + return findings; +} + +/** + * The refusal text. + * + * It names the object and the class because a bare rejection is not actionable: + * the author cannot amend what they cannot locate. The oldest offending commit + * is called out separately because that is the one an interactive rebase has to + * reach, and it is the single most common thing to get wrong when the material + * is several commits back. + * + * Tag findings get their OWN remedy line, and the commit remedy is emitted only + * when a commit is actually implicated. An annotated tag is not reachable by + * `--amend` or `rebase -i` — it is a separate object that has to be recreated — + * so printing the commit advice for a tag-only refusal would send the author to + * a command that cannot fix what was found. + */ +export function formatRefusal(findings: readonly GitPushFinding[]): string { + const lines: string[] = [ + "paperclip-github-egress: refusing to publish — credential-shaped material found in objects this push would make public.", + "", + "Git objects are content-addressed, so this cannot be redacted in flight the way an issue comment or a pull-request body is; the object itself has to change.", + "", + ]; + + const WHERE_LABEL = { + message: "commit message", + content: "file content", + "tag-message": "annotated tag message", + } as const; + + // `subject` arrives scrubbed (see {@link GitPushFinding.subject}); re-applying + // it here is a second barrier, not a duplicate. This function is the one thing + // on this path that writes to a human-visible sink, so it must not depend on + // every present and future construction site having remembered. The scrubber + // carries an existing marker through untouched, so a twice-scrubbed subject + // renders identically to a once-scrubbed one. + const displaySubject = (finding: GitPushFinding): string => + scrubGitHubEgressText(finding.subject).text; + + for (const finding of findings) { + const subject = displaySubject(finding); + lines.push( + ` ${finding.shortCommit} ${WHERE_LABEL[finding.where]}: ${finding.classes.join(", ")}${subject ? ` (${subject})` : ""}`, + ); + } + + const commitFindings = findings.filter((finding) => finding.where !== "tag-message"); + const tagFindings = findings.filter((finding) => finding.where === "tag-message"); + const oldest = commitFindings.length > 0 ? commitFindings[commitFindings.length - 1]! : null; + lines.push(""); + if (oldest && commitFindings.length === 1) { + lines.push("To fix: remove the material, then `git commit --amend` if it is the tip commit,"); + lines.push(`or \`git rebase -i ${oldest.shortCommit}~1\` to reach it if it is not.`); + } else if (oldest) { + lines.push( + `To fix: remove the material from each commit above. The oldest is ${oldest.shortCommit}, so \`git rebase -i ${oldest.shortCommit}~1\` reaches all of them.`, + ); + } + if (tagFindings.length > 0) { + const names = tagFindings + .map((finding) => displaySubject(finding).replace(/^tag /, "")) + .filter((name) => name && name !== "annotated tag"); + lines.push( + `To fix the annotated tag${tagFindings.length === 1 ? "" : "s"}: the message lives in the tag object, not in any commit, so \`--amend\` and \`rebase\` cannot reach it. Recreate with \`git tag -f -a ${names[0] ?? ""} -m ''\`${names.length > 0 ? "" : " for each tag above"}.`, + ); + } + lines.push(""); + lines.push( + "If this is a false positive on a test fixture, derive the value at runtime instead of embedding a literal — that is what the existing fixtures in this repository do, and it closes the finding permanently rather than suppressing it.", + ); + + return lines.join("\n"); +} + +/** + * The refusal text for a scan that could not be completed. + * + * Deliberately distinct from {@link formatRefusal}: nothing was found, so + * telling the author to amend a commit would send them looking for material + * that may not exist. What they need to know is that this is a refusal rather + * than a detection, and what to do about the read that failed. + */ +export function formatScanFailure(error: unknown): string { + const detail = error instanceof Error ? error.message : String(error); + return [ + "paperclip-github-egress: refusing to publish — the credential scan could not be completed.", + "", + ` ${detail}`, + "", + "This is a refusal, not a detection: nothing was found because nothing could be read.", + "A scan that cannot inspect the commits it is meant to check cannot report them clean,", + "so the push is stopped rather than allowed through unscanned.", + "", + "If git cannot read the repository, fix that and re-run. If the push is very large, the", + "read may have exceeded the scanner's buffer — push in smaller batches.", + ].join("\n"); +} diff --git a/server/src/__tests__/github-egress-outbound-coverage.test.ts b/server/src/__tests__/github-egress-outbound-coverage.test.ts index 201604700c7c..cdcd4737e171 100644 --- a/server/src/__tests__/github-egress-outbound-coverage.test.ts +++ b/server/src/__tests__/github-egress-outbound-coverage.test.ts @@ -34,9 +34,16 @@ import { * GitHub-touching binary in the agent sandbox, and `${LOCAL_BIN}` is first on * the PATH of every agent Job. Those launchers are the complete set of * interposition points available to us, so they are the complete set of places - * an outbound scrub can live. This table requires each one to be classified, - * and — for the ones claimed as scrubbed — asserts the launcher actually still - * execs its egress runtime. Deleting the scrub from a wrapper fails here. + * an outbound control can live. This table requires each one to be classified, + * and — for the ones claimed as controlled — asserts the launcher actually + * still execs its egress runtime. Deleting the control from a wrapper fails + * here. + * + * Note that "controlled" covers two different mechanisms, and the `Coverage` + * union keeps them apart on purpose: `egress-scrubbed` rewrites the payload in + * flight and the caller still succeeds, while `egress-refused` (PEN-3156's git + * door) can only stop the publish, because by then the objects are + * content-addressed. Do not collapse the two kinds to simplify the table. * * It also enumerates the server-side write set, which is a second family * entirely: `paperclip-api` writes to GitHub over HTTP from `server/`, reaching @@ -67,9 +74,11 @@ const statefulSetPath = path.join(repoRoot, "deploy/helm/paperclip/templates/sta const servicesDirectory = path.join(repoRoot, "server/src/services"); const serverSourceDirectory = path.join(repoRoot, "server/src"); -/** The compiled entrypoints that carry a scrub, as the wrappers name them. */ +/** The compiled entrypoints that carry a control, as the wrappers name them. */ const CLI_EGRESS_RUNTIME = "github-cli-egress-runtime.js"; const MCP_EGRESS_RUNTIME = "github-mcp-egress-runtime.js"; +const GIT_EGRESS_RUNTIME = "github-git-egress-runtime.js"; + /** The server-side wrapper over `scrubGitHubEgressText`, as the service files name it. */ const SERVER_EGRESS_SCRUB = "scrubOutboundGitHubText"; @@ -80,6 +89,22 @@ type Coverage = * the server-side scrub helper for a service file. */ | { kind: "egress-scrubbed"; runtime: string } + /** + * Agent-authored text on this path is REFUSED, not rewritten, when it carries + * material `scrubGitHubEgressText` would remove. + * + * A deliberately separate `kind` from `egress-scrubbed`, because the two are + * not interchangeable and collapsing them would overstate the cover. A + * scrubbed door rewrites a payload in flight and the caller's request still + * succeeds. This door cannot: the objects are content-addressed by the time + * they exist, so editing a message or a blob changes every downstream SHA. + * The only available control is to stop the publish and name the offending + * object, which means the agent's command FAILS and it must go back and redo + * the object. Anything reading this table to answer "is this door covered" + * gets a yes; anything reading it to answer "does authored text reach GitHub + * unaltered here" needs the distinction. + */ + | { kind: "egress-refused"; runtime: string; ticket: string; why: string } /** Reaches GitHub carrying authored text, with NO scrubber on the path. */ | { kind: "unscrubbed"; ticket: string; why: string } /** Touches GitHub credentials but is not itself a path authored text travels. */ @@ -106,14 +131,18 @@ const WRAPPER_COVERAGE: Readonly> = { runtime: MCP_EGRESS_RUNTIME, }, git: { - kind: "unscrubbed", + kind: "egress-refused", + runtime: GIT_EGRESS_RUNTIME, ticket: "PEN-3156", - // Worded to avoid the literal command name: scripts/check-no-git-push.mjs - // scans this tree for it, and the marker that opts a line out asserts an - // operator-approved push path exists. No such path exists here — this is a - // description of a gap — so spending that escape hatch on a doc string - // would put a false claim inside a security control. - why: "token-injection wrapper only; pushing through it publishes commit messages and file contents — a strict superset of what create_or_update_file/push_files carry. Not fixable by in-flight redaction: commit objects are content-addressed, so altering a blob or message after the fact changes every downstream SHA. The fix shape is refusal at push time, which is its own rollout", + // Still worded to avoid the literal command name, for the SAME reason the + // `unscrubbed` row this replaces was: scripts/check-no-git-push.mjs scans + // server/src (see its DEFAULT_SCAN_ROOTS), so this very file is in scope, + // and the marker that opts a line out asserts an operator-approved publish + // path exists on it. This is a description of a control, not a path that + // publishes, so spending that escape hatch here would put a false claim + // inside a security control. Note the scanner matches `git-push` and + // `git_push` too, so hyphenating is not a way around it. + why: "guarded by github-git-egress-runtime.js, which the seed puts on BOTH interposition points: the ${LOCAL_BIN}/git launcher (inside the token wrapper, so credentials still reach it) and a pre-push hook it injects via core.hooksPath. It scans the outgoing commit range — messages, added file content including binary/textconv-laundered paths, and annotated tag messages — and refuses the publish naming the offending object and class, rather than rewriting it, because commit objects are content-addressed and altering one changes every downstream SHA. Failed reads refuse rather than pass unscanned. The hook covers the porcelain publish verb ONLY, because it is the only one git runs a pre-push hook for; the plumbing verbs that publish without consulting it (send-pack, http-push) are refused outright by the wrapper, which enforces by an allowlist of verbs known not to publish, so an unrecognised verb refuses rather than passing through unscanned", }, "paperclip-github-token-env": { kind: "not-an-authored-text-path", @@ -252,12 +281,14 @@ describe("outbound GitHub egress coverage", () => { expect(seeded).toEqual(classified); }); - it("every launcher claimed as scrubbed still execs its egress runtime", () => { + it("every launcher claimed as controlled still execs its egress runtime", () => { // This is the regression guard for the control itself. Removing the // scrub from a wrapper — the change that would silently reopen - // PEN-2527's or PEN-3152's gap — fails right here. + // PEN-2527's or PEN-3152's gap — fails right here. `egress-refused` is + // included on the same footing: PEN-3156's guard is reached the same + // way, by the launcher exec'ing a runtime, so deleting it fails here too. for (const [name, coverage] of Object.entries(WRAPPER_COVERAGE)) { - if (coverage.kind !== "egress-scrubbed") continue; + if (coverage.kind !== "egress-scrubbed" && coverage.kind !== "egress-refused") continue; const body = readWrapperBody(name); expect(body, `${name} no longer execs ${coverage.runtime}`).toContain(coverage.runtime); expect(body, `${name} must exec its target, not merely mention the runtime`).toMatch( @@ -279,12 +310,64 @@ describe("outbound GitHub egress coverage", () => { expect(runtimeAt).toBeGreaterThan(tokenAt); }); - it("the two scrubbed doors use DIFFERENT runtimes for their two transports", () => { - // argv rewriting and JSON-RPC frame rewriting are not interchangeable. - // Pointing one wrapper at the other's runtime would produce a process - // that runs, scrubs nothing, and looks correct in this table. + it("each controlled door uses its OWN runtime, never a sibling's", () => { + // argv rewriting, JSON-RPC frame rewriting and commit-range refusal are + // not interchangeable. Pointing one wrapper at another's runtime would + // produce a process that runs, controls nothing, and looks correct in + // this table. expect(readWrapperBody("gh")).not.toContain(MCP_EGRESS_RUNTIME); + expect(readWrapperBody("gh")).not.toContain(GIT_EGRESS_RUNTIME); expect(readWrapperBody("github-mcp-server")).not.toContain(CLI_EGRESS_RUNTIME); + expect(readWrapperBody("github-mcp-server")).not.toContain(GIT_EGRESS_RUNTIME); + expect(readWrapperBody("git")).not.toContain(CLI_EGRESS_RUNTIME); + expect(readWrapperBody("git")).not.toContain(MCP_EGRESS_RUNTIME); + }); + }); + + describe("the git publish door (PEN-3156)", () => { + // This door is the one `egress-refused` case, and unlike the scrubbed + // doors its control does not live in the launcher alone. The launcher + // handles the flags that would skip the hook; the hook is what actually + // sees the outgoing range. Both halves are asserted here because either + // one alone is not the control. + + it("runs the guard INSIDE the token wrapper, so git keeps its credentials", () => { + // Same ordering constraint as github-mcp-server, and load-bearing for + // the same reason: a guard placed outside paperclip-github-token-env + // would leave git unauthenticated, and an authentication failure is the + // kind of breakage that gets a security control reverted, not fixed. + const body = readWrapperBody("git"); + const tokenAt = body.indexOf("paperclip-github-token-env"); + const runtimeAt = body.indexOf(GIT_EGRESS_RUNTIME); + expect(tokenAt).toBeGreaterThanOrEqual(0); + expect(runtimeAt).toBeGreaterThan(tokenAt); + }); + + it("seeds a pre-push hook that execs the same runtime", () => { + // The launcher can only see the argv it was handed. The hook is what + // git itself invokes with the outgoing ref updates on stdin, so it is + // the half that reads the range being published. Deleting it would + // leave a wrapper that still rejects hook-skipping flags while nothing + // downstream ever inspects a commit. + const source = readFileSync(statefulSetPath, "utf8"); + const hookAt = source.indexOf('cat > "${GIT_HOOKS_DIR}/pre-push" <<\'EOF\''); + expect(hookAt, "the seed no longer writes a pre-push hook").toBeGreaterThanOrEqual(0); + const hookBody = source.slice(hookAt, source.indexOf("\n EOF", hookAt)); + expect(hookBody).toContain(GIT_EGRESS_RUNTIME); + expect(hookBody, "the hook must run the runtime in its hook mode").toContain( + "--pre-push-hook", + ); + }); + + it("publishes the guarded launcher on the default PATH", () => { + // Without this the guard is decorative, and that is measured rather + // than theoretical: before PEN-3156, ${LOCAL_BIN}/git existed while a + // live agent Job pod resolved `command -v git` to /usr/bin/git, because + // ${LOCAL_BIN} is only prepended to PATH by a LOGIN shell and agent + // tool harnesses spawn non-login shells. Same defect PEN-2527 fixed for + // the gh wrapper — a choke point nothing traverses. + const source = readFileSync(statefulSetPath, "utf8"); + expect(source).toContain('ln -sf "${LOCAL_BIN}/git" "${PATH_BIN}/git"'); }); }); @@ -379,12 +462,16 @@ describe("outbound GitHub egress coverage", () => { }); describe("table hygiene", () => { - it("every unscrubbed door names a ticket that owns it", () => { + it("every door that names a ticket names a real one, with a reason", () => { + // `egress-refused` is held to this too, not just `unscrubbed`. A refusal + // door still owes the reader a ticket and an explanation of what it + // refuses, because "covered" here does not mean "behaves like a + // scrubbed door" — see the kind's own doc comment. for (const coverage of [ ...Object.values(WRAPPER_COVERAGE), ...Object.values(SERVER_WRITE_COVERAGE), ]) { - if (coverage.kind !== "unscrubbed") continue; + if (coverage.kind !== "unscrubbed" && coverage.kind !== "egress-refused") continue; expect(coverage.ticket).toMatch(/^(PEN|BLO)-\d+$/); expect(coverage.why.length).toBeGreaterThan(40); }