Fix host-protection hooks failing closed and report jq dependency - #28
Merged
Merged
Conversation
The PreToolUse guard hook and SessionStart skill-loader hook relied on jq under `set -euo pipefail`. When jq was missing or simply not on the hook's minimal PATH (e.g. Homebrew's /opt/homebrew/bin), the hook crashed with a non-zero exit, which Copilot/Claude treat as a deny -- blocking ALL bash commands everywhere, even in non-devcontainer repos. Changes: - Prepend common bin dirs (/opt/homebrew/bin, /usr/local/bin, /usr/bin, /bin) to PATH so jq is found under the hooks' minimal PATH. - Move all decision logic into a subshell (decide()/build_context()) run under set -euo pipefail. The parent captures stdout via `if VAR=$(decide ...)`. A deny is signaled ONLY by stdout content; any failure (missing jq, malformed payload, etc.) makes the subshell exit non-zero and the parent falls open (allow / no context). This replaces a fragile ERR-trap approach that does not reliably terminate on macOS bash 3.2 during command-substitution assignments. - Report jq in install.sh/install.ps1 using the same checkmark style as the other dependencies, warning that hooks are inactive without it. Copilot-Session: 1061ab5c-0b78-4baa-87e2-2ba93359413f
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The host-protection hooks could fail closed — crashing in a way that Copilot/Claude interpret as a deny, which blocks all bash commands everywhere, even in repos without a devcontainer.
Root cause was two-fold:
jqnot found under the hook's minimal PATH. The hooks run with a minimal PATH that omits Homebrew's/opt/homebrew/bin, so even an installedjqwasn't found.set -euo pipefail, a missing/failingjqmade the hook exit non-zero → treated as a deny.A deny is meant to be signaled by stdout JSON content, with the script exiting
0in all intended cases. So any non-zero exit is really a crash and should fail open, not block.Changes
/opt/homebrew/bin:/usr/local/bin:/usr/bin:/bin) in both hooks sojqresolves under the minimal PATH.devcontainer-guard.shruns all decision logic in adecide()subshell (set -euo pipefail) that prints deny JSON on stdout or nothing for allow. The parent usesif DECISION=$(decide "$INPUT"); any failure → allow.devcontainer-skill-loader.shuses the same pattern viabuild_context(); any failure → no context injected.ERR-trap approach. macOS ships bash 3.2, whereexitinside anERRtrap fired by avar=$(cmd)assignment does not reliably terminate the script — the subshell-capture pattern doesn't depend on trap-exit at all.jqininstall.sh/install.ps1using the same✓ / ✗ — <url>style as the other dependencies (devpod, gh, devcontainer), warning that the host-protection hooks are inactive untiljqis installed.Testing
Verified 11 cases against both hooks:
git && npm→ denyUSER_CONFIRMED_HOST_OPERATION=1bypassjqgenuinely absent → single fail-open, exit 0, no spurious deny