fix(security): refuse credential-shaped material at the git publish boundary (PEN-3156)

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

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", () => {