mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-18 10:52:24 +02:00
fix(redact): install-prepush-hook refreshes a stale managed hook
The marker check returned before the only writer, so once a repo had the hook, no later change to the wrapper could ever reach it. The `printf x` fail-open fix (v1.64.0.0) has still not landed in any repo that received the hook before it, and a wrapper naming a gstack that has since moved stays pointed at a dead path for the same reason. Compare the body against what this version generates: rewrite on drift, stay a no-op when identical. The chained pre-push.local is untouched on both paths. The existing trailing-newline regression test cannot catch this — it installs into a repo with no prior managed hook, the one case that was never broken.
This commit is contained in:
+33
-13
@@ -62,19 +62,6 @@ function installPrepushHook(): void {
|
|||||||
const hookPath = path.join(dir, "pre-push");
|
const hookPath = path.join(dir, "pre-push");
|
||||||
const prepushBin = path.join(import.meta.dir, "gstack-redact-prepush");
|
const prepushBin = path.join(import.meta.dir, "gstack-redact-prepush");
|
||||||
|
|
||||||
// If a non-managed hook exists, preserve it as pre-push.local and chain it.
|
|
||||||
if (fs.existsSync(hookPath)) {
|
|
||||||
const existing = fs.readFileSync(hookPath, "utf8");
|
|
||||||
if (existing.includes(MANAGED_MARKER)) {
|
|
||||||
process.stdout.write("gstack-redact: pre-push hook already installed.\n");
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
const localPath = path.join(dir, "pre-push.local");
|
|
||||||
fs.renameSync(hookPath, localPath);
|
|
||||||
fs.chmodSync(localPath, 0o755);
|
|
||||||
process.stdout.write("gstack-redact: preserved existing hook as pre-push.local (chained).\n");
|
|
||||||
}
|
|
||||||
|
|
||||||
// stdin is single-consume: capture it once, feed both the chained hook and ours.
|
// 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.
|
// The `printf x` sentinel preserves the trailing newline that `$(cat)` strips.
|
||||||
// Without it, a chained shell pre-push.local built on `while read` silently
|
// Without it, a chained shell pre-push.local built on `while read` silently
|
||||||
@@ -91,6 +78,39 @@ if [ -x "$_local" ]; then
|
|||||||
fi
|
fi
|
||||||
printf '%s' "$_input" | bun "${prepushBin}" "$@"
|
printf '%s' "$_input" | bun "${prepushBin}" "$@"
|
||||||
`;
|
`;
|
||||||
|
|
||||||
|
// If a non-managed hook exists, preserve it as pre-push.local and chain it.
|
||||||
|
if (fs.existsSync(hookPath)) {
|
||||||
|
const existing = fs.readFileSync(hookPath, "utf8");
|
||||||
|
if (existing.includes(MANAGED_MARKER)) {
|
||||||
|
// A hook we already own. Returning here unconditionally froze every
|
||||||
|
// existing install on whatever wrapper it first received: the `printf x`
|
||||||
|
// fail-open fix landed in v1.64.0.0 and still had not reached a single
|
||||||
|
// repo that got the hook before it, because the only writer is gated on
|
||||||
|
// this branch. A wrapper naming a gstack that has since been moved or
|
||||||
|
// removed stays pointed at that dead path for the same reason.
|
||||||
|
//
|
||||||
|
// Rewrite when the body has drifted from what this version generates;
|
||||||
|
// stay a no-op when it has not, so the command is still idempotent. The
|
||||||
|
// chained pre-push.local is never touched on either path — it is the
|
||||||
|
// user's, not ours.
|
||||||
|
if (existing === wrapper) {
|
||||||
|
process.stdout.write("gstack-redact: pre-push hook already installed.\n");
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
fs.writeFileSync(hookPath, wrapper, { mode: 0o755 });
|
||||||
|
fs.chmodSync(hookPath, 0o755);
|
||||||
|
process.stdout.write(
|
||||||
|
`gstack-redact: refreshed stale managed pre-push hook at ${hookPath}\n`,
|
||||||
|
);
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
const localPath = path.join(dir, "pre-push.local");
|
||||||
|
fs.renameSync(hookPath, localPath);
|
||||||
|
fs.chmodSync(localPath, 0o755);
|
||||||
|
process.stdout.write("gstack-redact: preserved existing hook as pre-push.local (chained).\n");
|
||||||
|
}
|
||||||
|
|
||||||
fs.writeFileSync(hookPath, wrapper, { mode: 0o755 });
|
fs.writeFileSync(hookPath, wrapper, { mode: 0o755 });
|
||||||
fs.chmodSync(hookPath, 0o755);
|
fs.chmodSync(hookPath, 0o755);
|
||||||
process.stdout.write(`gstack-redact: installed pre-push hook at ${hookPath}\n`);
|
process.stdout.write(`gstack-redact: installed pre-push hook at ${hookPath}\n`);
|
||||||
|
|||||||
@@ -381,6 +381,87 @@ describe("install / chaining", () => {
|
|||||||
expect(r.status).toBe(1);
|
expect(r.status).toBe(1);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression: install returned early on ANY hook carrying the managed marker,
|
||||||
|
// so the only writer was unreachable once a hook existed. Every fix to the
|
||||||
|
// wrapper — including the `printf x` fail-open fix of v1.64.0.0 — stopped at
|
||||||
|
// repos that had never had the hook. The pre-existing newline test above
|
||||||
|
// cannot catch this: it installs into a repo with no prior managed hook, which
|
||||||
|
// is the one case that was never broken.
|
||||||
|
test("a stale managed hook is rewritten, and the refresh delivers the newline fix", () => {
|
||||||
|
const hookDir = path.join(repo, ".git", "hooks");
|
||||||
|
fs.mkdirSync(hookDir, { recursive: true });
|
||||||
|
const hook = path.join(hookDir, "pre-push");
|
||||||
|
|
||||||
|
// The v1.63-era wrapper, verbatim: same marker, `$(cat)` with no sentinel.
|
||||||
|
fs.writeFileSync(
|
||||||
|
hook,
|
||||||
|
[
|
||||||
|
"#!/usr/bin/env bash",
|
||||||
|
"# gstack-redact pre-push (managed)",
|
||||||
|
"set -euo pipefail",
|
||||||
|
'_input="$(cat)"',
|
||||||
|
'_local="$(git rev-parse --git-path hooks/pre-push.local)"',
|
||||||
|
'if [ -x "$_local" ]; then',
|
||||||
|
` printf '%s' "$_input" | "$_local" "$@" || exit $?`,
|
||||||
|
"fi",
|
||||||
|
`printf '%s' "$_input" | bun ${JSON.stringify(PREPUSH)} "$@"`,
|
||||||
|
"",
|
||||||
|
].join("\n"),
|
||||||
|
{ mode: 0o755 },
|
||||||
|
);
|
||||||
|
|
||||||
|
// A chained local hook of the shape the old wrapper starved: a bare
|
||||||
|
// `while read`, which never enters its body without a trailing newline.
|
||||||
|
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" >> ${JSON.stringify(seen)}; done\nexit 0\n`,
|
||||||
|
{ mode: 0o755 },
|
||||||
|
);
|
||||||
|
|
||||||
|
const r = spawnSync("bun", [REDACT, "install-prepush-hook"], {
|
||||||
|
cwd: repo,
|
||||||
|
encoding: "utf8",
|
||||||
|
});
|
||||||
|
expect(r.status).toBe(0);
|
||||||
|
expect(r.stdout).toContain("refreshed stale managed pre-push hook");
|
||||||
|
expect(fs.readFileSync(hook, "utf8")).toContain("printf x");
|
||||||
|
|
||||||
|
// The chained hook is the user's; a refresh must not rename or rewrite it.
|
||||||
|
expect(fs.readFileSync(path.join(hookDir, "pre-push.local"), "utf8")).toContain("while read");
|
||||||
|
|
||||||
|
// Behavioural half: the refreshed wrapper actually feeds the final ref line.
|
||||||
|
const sha = "c".repeat(40);
|
||||||
|
const run = spawnSync("bash", [hook], {
|
||||||
|
cwd: repo,
|
||||||
|
input: Buffer.from(`refs/heads/main ${sha} refs/heads/main ${ZERO}\n`),
|
||||||
|
encoding: "utf8",
|
||||||
|
env: { ...process.env, GSTACK_REDACT_PREPUSH: "skip" },
|
||||||
|
});
|
||||||
|
expect(run.status).toBe(0);
|
||||||
|
expect(fs.readFileSync(seen, "utf8").trim()).toBe("refs/heads/main");
|
||||||
|
});
|
||||||
|
|
||||||
|
test("install stays idempotent: an up-to-date managed hook is not rewritten", () => {
|
||||||
|
const hookDir = path.join(repo, ".git", "hooks");
|
||||||
|
fs.mkdirSync(hookDir, { recursive: true });
|
||||||
|
const hook = path.join(hookDir, "pre-push");
|
||||||
|
|
||||||
|
spawnSync("bun", [REDACT, "install-prepush-hook"], { cwd: repo });
|
||||||
|
const first = fs.readFileSync(hook, "utf8");
|
||||||
|
const stamp = fs.statSync(hook).mtimeMs;
|
||||||
|
|
||||||
|
const again = spawnSync("bun", [REDACT, "install-prepush-hook"], {
|
||||||
|
cwd: repo,
|
||||||
|
encoding: "utf8",
|
||||||
|
});
|
||||||
|
expect(again.status).toBe(0);
|
||||||
|
expect(again.stdout).toContain("already installed");
|
||||||
|
expect(again.stdout).not.toContain("refreshed");
|
||||||
|
expect(fs.readFileSync(hook, "utf8")).toBe(first);
|
||||||
|
expect(fs.statSync(hook).mtimeMs).toBe(stamp);
|
||||||
|
});
|
||||||
|
|
||||||
test("uninstall restores the chained original", () => {
|
test("uninstall restores the chained original", () => {
|
||||||
const hookDir = path.join(repo, ".git", "hooks");
|
const hookDir = path.join(repo, ".git", "hooks");
|
||||||
fs.mkdirSync(hookDir, { recursive: true });
|
fs.mkdirSync(hookDir, { recursive: true });
|
||||||
|
|||||||
Reference in New Issue
Block a user