fix(redact-prepush): preserve the trailing newline handed to chained pre-push.local

The chaining wrapper captured stdin with $(cat), which strips the trailing
newline — a chained shell hook built on `while read` then never entered its
loop for the final (usually only) ref line and exited 0, failing OPEN. Use
the printf-x sentinel so the byte-exact input reaches the chained hook, with
tests covering both the pass-through and the short-circuit paths.

Contributed by @francis-eye (PR #2358).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This commit is contained in:
Garry Tan 2026-08-14 15:49:19 -07:00
parent d77a2c8e58
commit 57a211e4b0
No known key found for this signature in database
GPG Key ID: C1F69E85C74EFE1D
2 changed files with 54 additions and 1 deletions

View File

@ -73,10 +73,15 @@ function installPrepushHook(): void {
}
// stdin is single-consume: capture it once, feed both the chained hook and ours.
// The `printf x` sentinel preserves the trailing newline that `$(cat)` strips.
// Without it, a chained shell pre-push.local built on `while read` silently
// drops the final (often only) ref line and exits 0 — the guard reports
// success having scanned nothing, i.e. it fails OPEN.
const wrapper = `#!/usr/bin/env bash
${MANAGED_MARKER}
set -euo pipefail
_input="$(cat)"
_input="$(cat; printf x)"
_input="\${_input%x}"
_local="$(git rev-parse --git-path hooks/pre-push.local)"
if [ -x "$_local" ]; then
printf '%s' "$_input" | "$_local" "$@" || exit $?

View File

@ -216,6 +216,54 @@ describe("install / chaining", () => {
expect(fs.readFileSync(path.join(hookDir, "pre-push.local"), "utf8")).toContain("echo mine");
});
// Regression: `_input="$(cat)"` strips the trailing newline, so a chained
// shell hook using `while read` never entered its loop body for the final
// (usually only) ref line — it saw zero refs and exited 0, failing OPEN.
test("chained pre-push.local receives the final ref line (trailing newline preserved)", () => {
const hookDir = path.join(repo, ".git", "hooks");
fs.mkdirSync(hookDir, { recursive: true });
spawnSync("bun", [REDACT, "install-prepush-hook"], { cwd: repo });
const seen = path.join(repo, "seen.txt");
fs.writeFileSync(
path.join(hookDir, "pre-push.local"),
`#!/usr/bin/env bash\nwhile read -r a b c d; do echo "$a $b $c $d" >> ${JSON.stringify(seen)}; done\nexit 0\n`,
{ mode: 0o755 },
);
const sha = "a".repeat(40);
const line = `refs/heads/main ${sha} refs/heads/main ${ZERO}\n`;
const r = spawnSync("bash", [path.join(hookDir, "pre-push")], {
cwd: repo,
input: Buffer.from(line),
encoding: "utf8",
env: { ...process.env, GSTACK_REDACT_PREPUSH: "skip" },
});
expect(r.status).toBe(0);
expect(fs.existsSync(seen)).toBe(true);
expect(fs.readFileSync(seen, "utf8").trim()).toBe(
`refs/heads/main ${sha} refs/heads/main ${ZERO}`,
);
});
test("a blocking pre-push.local still short-circuits the push", () => {
const hookDir = path.join(repo, ".git", "hooks");
fs.mkdirSync(hookDir, { recursive: true });
spawnSync("bun", [REDACT, "install-prepush-hook"], { cwd: repo });
fs.writeFileSync(
path.join(hookDir, "pre-push.local"),
"#!/usr/bin/env bash\nwhile read -r _a _b _c _d || [ -n \"${_a:-}\" ]; do exit 1; done\nexit 0\n",
{ mode: 0o755 },
);
const r = spawnSync("bash", [path.join(hookDir, "pre-push")], {
cwd: repo,
input: Buffer.from(`refs/heads/main ${"b".repeat(40)} refs/heads/main ${ZERO}\n`),
encoding: "utf8",
env: { ...process.env, GSTACK_REDACT_PREPUSH: "skip" },
});
expect(r.status).toBe(1);
});
test("uninstall restores the chained original", () => {
const hookDir = path.join(repo, ".git", "hooks");
fs.mkdirSync(hookDir, { recursive: true });