Merge pull request #3194 from gsd-build/fix/3104-portable-bash-shebang
fix(hooks): use #!/usr/bin/env bash in community .sh hooks for portability (#3104)
This commit is contained in:
5
.changeset/portable-bash-shebang-hooks.md
Normal file
5
.changeset/portable-bash-shebang-hooks.md
Normal file
@@ -0,0 +1,5 @@
|
||||
---
|
||||
type: Fixed
|
||||
pr: 3194
|
||||
---
|
||||
**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 <path>` 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.
|
||||
@@ -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 <path>` 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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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.
|
||||
|
||||
@@ -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).
|
||||
|
||||
@@ -3,6 +3,13 @@
|
||||
// "Prohibited: Raw Text Matching on Test Outputs". Per-file review may
|
||||
// reclassify some entries as source-text-is-the-product during migration.
|
||||
|
||||
// allow-test-rule: structural-regression-guard
|
||||
// The shebang line must be `#!/usr/bin/env bash` (PATH-resolved) rather than
|
||||
// `#!/bin/bash` for cross-distro portability (NixOS, minimal Alpine do not
|
||||
// ship /bin/bash). This is an architectural constraint that cannot be verified
|
||||
// by executing the hooks — they run fine with either shebang on distros that
|
||||
// have /bin/bash, so only a source assertion catches a future regression.
|
||||
|
||||
/**
|
||||
* Regression tests for bug #2136 / #2206
|
||||
*
|
||||
@@ -100,11 +107,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]}")`
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user