mirror of
https://github.com/garrytan/gstack.git
synced 2026-09-09 14:38:59 +02:00
fix(redact): tolerate EEXIST from recursive mkdir in install-prepush-hook on bun/Windows (#2635)
fs.mkdirSync(dir, { recursive: true }) is a no-op on an existing directory
in Node, but bun on Windows throws EEXIST - crashing hook install on any
repo whose .git/hooks already existed, leaving the repo unprotected.
Add lib/fs-utils.ts mkdirpSync: swallow EEXIST only when statSync confirms
the path is an existing directory; a regular file occupying the path, a
stat failure, or any other errno still rethrows. Use it in
installPrepushHook().
The regression test emulates the Windows bun fs semantics via a
bun --preload fixture, so the exact crash path runs (and fails on the old
code) on any platform, including CI Linux.
Absorbed from PR #2641 with authorship preserved.
Fixes #2635
This commit is contained in:
+4
-1
@@ -37,6 +37,7 @@ import {
|
||||
type ScanOptions,
|
||||
type Finding,
|
||||
} from "../lib/redact-engine";
|
||||
import { mkdirpSync } from "../lib/fs-utils";
|
||||
|
||||
const MAX_STDIN_BYTES = 16 * 1024 * 1024; // hard ceiling before the engine cap
|
||||
|
||||
@@ -55,7 +56,9 @@ function hooksPath(): string {
|
||||
|
||||
function installPrepushHook(): void {
|
||||
const dir = hooksPath();
|
||||
fs.mkdirSync(dir, { recursive: true });
|
||||
// mkdirpSync, not bare mkdirSync: bun on Windows throws EEXIST from a
|
||||
// recursive mkdir when .git/hooks already exists (#2635).
|
||||
mkdirpSync(dir);
|
||||
const hookPath = path.join(dir, "pre-push");
|
||||
const prepushBin = path.join(import.meta.dir, "gstack-redact-prepush");
|
||||
|
||||
|
||||
@@ -0,0 +1,27 @@
|
||||
import { mkdirSync, statSync } from "fs";
|
||||
|
||||
/**
|
||||
* mkdir -p that tolerates the target directory already existing.
|
||||
*
|
||||
* Node's mkdirSync(dir, { recursive: true }) is a no-op when dir already
|
||||
* exists, but bun on Windows throws EEXIST in the same situation (#2635),
|
||||
* which crashed `gstack-redact install-prepush-hook` on any repo whose
|
||||
* .git/hooks already existed. Swallow EEXIST only when statSync confirms the
|
||||
* path is an existing directory; anything else - a regular file occupying the
|
||||
* path, a stat failure, a different errno - rethrows the original error, so a
|
||||
* real collision still fails loudly.
|
||||
*/
|
||||
export function mkdirpSync(dir: string): void {
|
||||
try {
|
||||
mkdirSync(dir, { recursive: true });
|
||||
} catch (e) {
|
||||
if ((e as NodeJS.ErrnoException | null)?.code === "EEXIST") {
|
||||
try {
|
||||
if (statSync(dir).isDirectory()) return;
|
||||
} catch {
|
||||
// stat failed - fall through and rethrow the original mkdir error
|
||||
}
|
||||
}
|
||||
throw e;
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,90 @@
|
||||
/**
|
||||
* mkdirpSync + install-prepush-hook under bun-on-Windows EEXIST semantics
|
||||
* (#2635).
|
||||
*
|
||||
* bun on Windows throws EEXIST from fs.mkdirSync(dir, { recursive: true })
|
||||
* when dir already exists - Node treats it as a no-op - which crashed
|
||||
* `gstack-redact install-prepush-hook` on any repo whose .git/hooks already
|
||||
* existed. The CLI regression test below emulates those Windows semantics via
|
||||
* a `bun --preload` fixture (test/helpers/emulate-bun-windows-eexist.ts), so
|
||||
* the crash path runs on any platform, including CI Linux.
|
||||
*/
|
||||
import { describe, test, expect } from "bun:test";
|
||||
import * as fs from "fs";
|
||||
import * as os from "os";
|
||||
import * as path from "path";
|
||||
import { spawnSync } from "child_process";
|
||||
import { mkdirpSync } from "../lib/fs-utils";
|
||||
|
||||
const REDACT = path.resolve(import.meta.dir, "..", "bin", "gstack-redact");
|
||||
const EEXIST_PRELOAD = path.resolve(
|
||||
import.meta.dir,
|
||||
"helpers",
|
||||
"emulate-bun-windows-eexist.ts",
|
||||
);
|
||||
|
||||
function tmpdir(): string {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), "fs-utils-"));
|
||||
}
|
||||
|
||||
describe("mkdirpSync", () => {
|
||||
test("creates missing nested directories", () => {
|
||||
const base = tmpdir();
|
||||
try {
|
||||
const dir = path.join(base, "a", "b", "c");
|
||||
mkdirpSync(dir);
|
||||
expect(fs.statSync(dir).isDirectory()).toBe(true);
|
||||
} finally {
|
||||
fs.rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("tolerates the directory already existing", () => {
|
||||
const base = tmpdir();
|
||||
try {
|
||||
mkdirpSync(base); // exists -> must be a no-op, not EEXIST
|
||||
mkdirpSync(base); // and idempotent on repeat calls
|
||||
} finally {
|
||||
fs.rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
test("still throws EEXIST when a regular file occupies the path", () => {
|
||||
const base = tmpdir();
|
||||
try {
|
||||
const file = path.join(base, "occupied");
|
||||
fs.writeFileSync(file, "x");
|
||||
expect(() => mkdirpSync(file)).toThrow(/EEXIST/);
|
||||
} finally {
|
||||
fs.rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("install-prepush-hook under bun-on-Windows EEXIST semantics (#2635)", () => {
|
||||
test("install succeeds when .git/hooks already exists, existing hook preserved", () => {
|
||||
const base = tmpdir();
|
||||
try {
|
||||
const repo = path.join(base, "repo");
|
||||
spawnSync("git", ["init", "-q", repo]);
|
||||
const hookDir = path.join(repo, ".git", "hooks");
|
||||
fs.mkdirSync(hookDir, { recursive: true });
|
||||
const hookPath = path.join(hookDir, "pre-push");
|
||||
fs.writeFileSync(hookPath, "#!/usr/bin/env bash\necho mine\n", { mode: 0o755 });
|
||||
|
||||
// Under the emulated bun-on-Windows fs, the bare
|
||||
// fs.mkdirSync(dir, { recursive: true }) in installPrepushHook() throws
|
||||
// EEXIST (the #2635 crash). With mkdirpSync it must install cleanly.
|
||||
const r = spawnSync("bun", ["--preload", EEXIST_PRELOAD, REDACT, "install-prepush-hook"], {
|
||||
cwd: repo,
|
||||
encoding: "utf8",
|
||||
});
|
||||
expect(r.status).toBe(0);
|
||||
expect(r.stderr ?? "").not.toContain("EEXIST");
|
||||
expect(fs.readFileSync(hookPath, "utf8")).toContain("gstack-redact pre-push (managed)");
|
||||
expect(fs.readFileSync(path.join(hookDir, "pre-push.local"), "utf8")).toContain("echo mine");
|
||||
} finally {
|
||||
fs.rmSync(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,20 @@
|
||||
/**
|
||||
* Bun --preload fixture that emulates bun-on-Windows fs.mkdirSync semantics
|
||||
* (see #2635): a recursive mkdir on an already-existing directory throws
|
||||
* EEXIST, where Node (and bun on Linux/macOS) treat it as a no-op success.
|
||||
*
|
||||
* Loaded into a child process with `bun --preload <this file> <script>`, it
|
||||
* lets the #2635 regression test exercise the exact Windows crash path on any
|
||||
* platform. The patch is deliberately transparent - it changes nothing except
|
||||
* throwing EEXIST where Windows bun would.
|
||||
*/
|
||||
const fs = require("fs");
|
||||
const orig = fs.mkdirSync;
|
||||
fs.mkdirSync = (p: string, opts: any) => {
|
||||
if (opts?.recursive && fs.existsSync(p) && fs.statSync(p).isDirectory()) {
|
||||
const e = new Error(`EEXIST: file already exists, mkdir '${p}'`);
|
||||
(e as any).code = "EEXIST";
|
||||
throw e;
|
||||
}
|
||||
return orig(p, opts);
|
||||
};
|
||||
Reference in New Issue
Block a user