Allow $(gh ...) command substitution in permission hook (#2622)

.claude/hooks/gh-permission-hook.py

@@ -150,6 +150,20 @@ SAFE_REDIRECT_PATTERN = re.compile(r'\d*>&\d+|\d*>/dev/null')
 # The echo command only outputs text, making this safe for fallback values
 SAFE_FALLBACK_PATTERN = re.compile(r'\|\|\s*echo\s+(?:"[^"]*"|\'[^\']*\'|\S+)\s*$')
 
+# Safe gh subcommand pattern - $(gh ...) command substitution where the inner
+# command is a safe, read-only gh call (no shell metacharacters inside). This is
+# commonly used to dynamically construct API endpoint URLs, e.g.:
+#   gh api repos/$(gh repo view --json nameWithOwner -q .nameWithOwner)/pulls/123/comments
+# The inner content must not contain shell metacharacters (;|&`$<>\n\r) to prevent
+# nested injection like $(gh pr view 123; rm -rf /)
+# Only known read-only subcommands are allowed to prevent destructive commands
+# like $(gh repo delete ...) from being neutralized.
+SAFE_GH_SUBCOMMAND_PATTERN = re.compile(
+    r'\$\(gh[ \t]+(?:repo[ \t]+view|pr[ \t]+view|issue[ \t]+view|run[ \t]+view|release[ \t]+view'
+    r'|gist[ \t]+view|search[ \t]+\w+|status|auth[ \t]+status|config[ \t]+(?:get|list))'
+    r'[ \t]+[^)$`;&|<>\n\r]*\)'
+)
+
 
 def extract_gh_command(command: str) -> Optional[str]:
     """
@@ -260,9 +274,13 @@ def contains_shell_injection(cmd: str) -> bool:
     # This allows patterns like: grep -E "bug|error"
     cmd_without_safe_doubles = SAFE_DOUBLE_QUOTED_PATTERN.sub('""', cmd_with_neutralized_spans)
 
+    # Replace safe $(gh ...) subcommands with a placeholder before checking
+    # This allows patterns like: gh api repos/$(gh repo view --json nameWithOwner -q .nameWithOwner)/...
+    cmd_to_check = SAFE_GH_SUBCOMMAND_PATTERN.sub('SAFE_GH_SUB', cmd_without_safe_doubles)
+
     # Replace safe pipe destinations with a placeholder before checking
     # This allows patterns like: gh api graphql ... | jq '...'
-    cmd_to_check = SAFE_PIPE_PATTERN.sub(' SAFE_PIPE ', cmd_without_safe_doubles)
+    cmd_to_check = SAFE_PIPE_PATTERN.sub(' SAFE_PIPE ', cmd_to_check)
 
     # Replace safe redirect patterns (like 2>&1, 2>/dev/null) before checking
     # These are standard shell redirects, not command execution

.claude/hooks/tests/bad_commands.txt

@@ -667,3 +667,16 @@ gh api /orgs/org/memberships/attacker -f role='admin'
 gh api /user/gpg_keys -f armored_public_key='-----BEGIN PGP PUBLIC KEY BLOCK-----'
 gh api /user/ssh_signing_keys -f key='ssh-rsa AAAA...'
 gh api /user/emails -f emails='["attacker@evil.com"]'
+
+# =============================================================================
+# DESTRUCTIVE GH SUBCOMMANDS INSIDE $(...) COMMAND SUBSTITUTION
+# =============================================================================
+
+# Destructive gh subcommands should NOT be neutralized by SAFE_GH_SUBCOMMAND_PATTERN
+# These use $( which triggers shell injection detection since the inner command
+# is not in the allowed read-only subcommand list
+gh api repos/$(gh repo delete owner/repo)/pulls/123
+gh api repos/$(gh repo create new-repo)/pulls/123
+gh api repos/$(gh gist delete abc123)/pulls/123
+gh api repos/$(gh run cancel 456)/pulls/123
+gh api repos/$(gh issue delete 789)/pulls/123

.claude/hooks/tests/good_commands.txt

@@ -758,3 +758,8 @@ gh pr create --title "Test" --body "Run `echo x | sh` here"
 gh pr create --title "Test" --body "Check `env` output"
 gh pr create --title "Test" --body "Run `whoami` here"
 gh pr view 123 --json <(cat)
+gh api repos/$(gh repo view --json nameWithOwner -q .nameWithOwner)/pulls/2524/comments --paginate 2>/dev/null | jq -r '.[].body' 2>/dev/null | head -200
+# Note: This command passes as a passthrough (exit 0 / no decision), not as an explicit
+# auto-approval, because extract_gh_command() cannot parse VAR=$(...) assignment syntax.
+# The test framework treats passthrough the same as allow for good_commands.txt.
+REPO=$(gh repo view --json nameWithOwner -q .nameWithOwner) && gh api "repos/${REPO}/pulls/2524/comments" --paginate | jq -r '.[].body' 2>/dev/null | head -200