From 8ca86b5e249581002969522a191e3f25dcce3e10 Mon Sep 17 00:00:00 2001 From: Otavio Salvador Date: Mon, 4 May 2026 18:51:37 -0300 Subject: [PATCH] fix: use #!/usr/bin/env bash in community .sh hooks for distro portability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The three opt-in bash hooks (gsd-phase-boundary.sh, gsd-session-state.sh, gsd-validate-commit.sh) shipped with #!/bin/bash, which fails on distros that don't ship bash at /bin/bash (NixOS, minimal Alpine images, some container runtimes). POSIX guarantees /bin/sh but not /bin/bash. This is latent in the default install path because Claude Code wires the hooks as `bash ` from settings.json (PATH-resolved — the script's own shebang is read as a comment by bash). The fix matters when scripts are run directly: tests, future installer changes, or manual debugging. Changes: - hooks/gsd-{phase-boundary,session-state,validate-commit}.sh: shebang switched to #!/usr/bin/env bash, matching the convention already used in scripts/*.sh. - tests/bug-2136-sh-hook-version.test.cjs: assertion updated to expect the new shebang; comment updated to spell out the rationale. - tests/bug-2979-hook-absolute-node.test.cjs: doc-comment updated — the prior wording cited "POSIX std PATH always has /bin" as the reason bare `bash` is OK. The actual reason is that bare `bash` is PATH-resolved, which is portable across distros that don't ship /bin/bash. POSIX std PATH guarantees /bin/sh, not /bin/bash. - bin/install.js::buildHookCommand: comment block clarifying the same. No behavior change in this file — bare `bash` was already correct. - .changeset/portable-bash-shebang-hooks.md: changeset entry. Verified locally on NixOS: - npm run build:hooks: hooks/dist/*.sh shebangs propagate correctly. - node --test tests/bug-2136-*.cjs tests/bug-2979-*.cjs tests/bug-1817-*.cjs tests/bug-1834-*.cjs tests/bug-1906-*.cjs tests/bug-2557-*.cjs tests/bug-3017-*.cjs tests/security-scan.test.cjs tests/hooks-doc-parity.test.cjs: 126/126 pass. - node scripts/run-tests.cjs (full suite): 6944 pass / 0 fail / 5 skip. --- .changeset/portable-bash-shebang-hooks.md | 5 +++++ bin/install.js | 14 ++++++++++---- hooks/gsd-phase-boundary.sh | 2 +- hooks/gsd-session-state.sh | 2 +- hooks/gsd-validate-commit.sh | 2 +- tests/bug-2136-sh-hook-version.test.cjs | 13 ++++++++++--- tests/bug-2979-hook-absolute-node.test.cjs | 3 ++- 7 files changed, 30 insertions(+), 11 deletions(-) create mode 100644 .changeset/portable-bash-shebang-hooks.md diff --git a/.changeset/portable-bash-shebang-hooks.md b/.changeset/portable-bash-shebang-hooks.md new file mode 100644 index 000000000..60e0f5ce9 --- /dev/null +++ b/.changeset/portable-bash-shebang-hooks.md @@ -0,0 +1,5 @@ +--- +type: Fixed +pr: 3105 +--- +**Community .sh hooks now use `#!/usr/bin/env bash` for cross-distro portability.** The three opt-in bash hooks (`gsd-phase-boundary.sh`, `gsd-session-state.sh`, `gsd-validate-commit.sh`) shipped with `#!/bin/bash`, which fails on distros that don't ship bash at `/bin/bash` (NixOS, minimal Alpine images, some container runtimes). POSIX guarantees `/bin/sh` but not `/bin/bash`. The fix matches the convention already used in `scripts/*.sh`. Latent in the default install path because Claude Code wires hooks as `bash ` from `settings.json` (PATH-resolved — the script's own shebang is read as a comment), but the bug surfaces immediately if a hook is run directly (tests, future installer changes, manual debugging). Comment in `bin/install.js::buildHookCommand` updated to clarify that the runner is PATH-resolved bare `bash`, not `/bin/bash` — POSIX std PATH guarantee was the wrong rationale. diff --git a/bin/install.js b/bin/install.js index 42c19c450..24677a299 100755 --- a/bin/install.js +++ b/bin/install.js @@ -709,10 +709,16 @@ function rewriteLegacyCodexHookBlock(content, absoluteRunner) { */ function buildHookCommand(configDir, hookName, opts) { if (!opts) opts = {}; - // .sh hooks run under /bin/bash (POSIX std PATH always includes /bin), - // so bare `bash` is fine. .js hooks need the absolute node path because - // GUI-launched runtimes start with a minimal PATH that does not include - // nvm/Homebrew/Volta-installed node binaries (#2979). + // .sh hooks run under bare `bash` (PATH-resolved). POSIX guarantees + // /bin/sh but not /bin/bash, and distros like NixOS do not ship + // /bin/bash by default — so PATH-resolved `bash` is more portable than + // an absolute /bin/bash. The wrapping `bash ` invocation also + // means the script's own shebang (#!/usr/bin/env bash) is read as a + // comment in this code path; it only matters when the script is run + // directly (e.g. tests or future installer changes). .js hooks still + // need the absolute node path because GUI-launched runtimes start with + // a minimal PATH that does not include nvm/Homebrew/Volta-installed + // node binaries (#2979). const nodeRunner = resolveNodeRunner(); const runner = hookName.endsWith('.sh') ? 'bash' : nodeRunner; // resolveNodeRunner returns null when process.execPath is unavailable. diff --git a/hooks/gsd-phase-boundary.sh b/hooks/gsd-phase-boundary.sh index b1a35233e..fcdb482c5 100755 --- a/hooks/gsd-phase-boundary.sh +++ b/hooks/gsd-phase-boundary.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # gsd-hook-version: {{GSD_VERSION}} # gsd-phase-boundary.sh — PostToolUse hook: detect .planning/ file writes # Outputs a reminder when planning files are modified outside normal workflow. diff --git a/hooks/gsd-session-state.sh b/hooks/gsd-session-state.sh index 9eb0c56dc..9b9a05df0 100755 --- a/hooks/gsd-session-state.sh +++ b/hooks/gsd-session-state.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # gsd-hook-version: {{GSD_VERSION}} # gsd-session-state.sh — SessionStart hook: inject project state reminder # Outputs STATE.md head on every session start for orientation. diff --git a/hooks/gsd-validate-commit.sh b/hooks/gsd-validate-commit.sh index f8a655e9b..7bd755241 100755 --- a/hooks/gsd-validate-commit.sh +++ b/hooks/gsd-validate-commit.sh @@ -1,4 +1,4 @@ -#!/bin/bash +#!/usr/bin/env bash # gsd-hook-version: {{GSD_VERSION}} # gsd-validate-commit.sh — PreToolUse hook: enforce Conventional Commits format # Blocks git commit commands with non-conforming messages (exit 2). diff --git a/tests/bug-2136-sh-hook-version.test.cjs b/tests/bug-2136-sh-hook-version.test.cjs index 8c8fbdfde..5b9c693d5 100644 --- a/tests/bug-2136-sh-hook-version.test.cjs +++ b/tests/bug-2136-sh-hook-version.test.cjs @@ -100,11 +100,18 @@ describe('bug #2136 part 1: bash hook sources carry gsd-hook-version placeholder } test('version header is on line 2 (immediately after shebang)', () => { - // Placing the header immediately after #!/bin/bash ensures it is always - // found regardless of how much of the file is read. + // Placing the header immediately after the shebang ensures it is always + // found regardless of how much of the file is read. The shebang itself + // must use `#!/usr/bin/env bash` (PATH-resolved) rather than `#!/bin/bash` + // — POSIX guarantees /bin/sh but not /bin/bash, and distros like NixOS + // do not ship /bin/bash by default. for (const sh of SH_HOOKS) { const lines = fs.readFileSync(path.join(HOOKS_DIR, sh), 'utf8').split('\n'); - assert.strictEqual(lines[0], '#!/bin/bash', `${sh} line 1 must be #!/bin/bash`); + assert.strictEqual( + lines[0], + '#!/usr/bin/env bash', + `${sh} line 1 must be "#!/usr/bin/env bash" for cross-distro portability` + ); assert.ok( lines[1].startsWith('# gsd-hook-version:'), `${sh} line 2 must be the gsd-hook-version header (got: "${lines[1]}")` diff --git a/tests/bug-2979-hook-absolute-node.test.cjs b/tests/bug-2979-hook-absolute-node.test.cjs index 780c5c71f..da81f6d63 100644 --- a/tests/bug-2979-hook-absolute-node.test.cjs +++ b/tests/bug-2979-hook-absolute-node.test.cjs @@ -20,7 +20,8 @@ process.env.GSD_TEST_MODE = '1'; * resolveNodeRunner helper, asserting on structured records: * - the runner field is an absolute path (not bare 'node') * - it ends with /node or \\node (or .exe on Windows simulation) - * - .sh hooks still use bare 'bash' (POSIX std PATH always has /bin) + * - .sh hooks still use bare 'bash' (PATH-resolved; portable across + * distros that don't ship /bin/bash, like NixOS) * * No source-grep on install.js content — assertions go against the * value returned by the exported function and the parsed structure of