• Will Chen's avatar
    Allow $(gh ...) command substitution in permission hook (#2622) · af50a6c4
    Will Chen 提交于
    ## Summary
    - Add safe pattern for `$(gh ...)` subcommands in the shell injection
    checker, allowing common patterns like dynamically constructing API
    endpoint URLs using `gh repo view` output
    - The inner content must not contain shell metacharacters to prevent
    nested injection
    - Add two new test cases to `good_commands.txt` covering `$(gh repo view
    ...)` and `REPO=$(gh repo view ...) &&` patterns
    
    ## Test plan
    - [x] All 923 hook permission tests pass (516 good + 407 bad)
    - [x] Unit tests pass (31 test files)
    - [x] Lint/format/type checks pass
    
    🤖 Generated with [Claude Code](https://claude.com/claude-code)
    <!-- devin-review-badge-begin -->
    
    ---
    
    <a href="https://app.devin.ai/review/dyad-sh/dyad/pull/2622"
    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 -->
    
    <!-- CURSOR_SUMMARY -->
    ---
    
    > [!NOTE]
    > **Medium Risk**
    > Touches shell-injection detection logic in a security gate; a
    too-permissive regex could allow malicious command substitution, though
    it is constrained to read-only subcommands and covered by new tests.
    > 
    > **Overview**
    > **Permission hook now permits a constrained form of `$(gh ...)`
    substitution.** `gh-permission-hook.py` adds
    `SAFE_GH_SUBCOMMAND_PATTERN` and neutralizes matching read-only `$(gh
    …)` subcommands before running shell-metacharacter checks, enabling
    dynamic `gh api` paths built from `gh repo view`/similar.
    > 
    > **Tests updated to validate both sides.** `good_commands.txt` adds
    allowed examples using `$(gh repo view …)` (including a var-assignment
    pattern that remains passthrough), and `bad_commands.txt` adds cases
    ensuring destructive subcommands inside `$(...)` are still blocked.
    `package-lock.json` also changes peer metadata for multiple dependencies
    (lockfile-only churn).
    > 
    > <sup>Written by [Cursor
    Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
    8bcb142513e4dfab4689e0160d581f44f7b24e95. This will update automatically
    on new commits. Configure
    [here](https://cursor.com/dashboard?tab=bugbot).</sup>
    <!-- /CURSOR_SUMMARY -->
    
    <!-- This is an auto-generated description by cubic. -->
    ---
    ## Summary by cubic
    Enabled safe $(gh ...) command substitution in the permission hook for
    dynamic GitHub API paths, limited to read-only gh subcommands. Tightened
    validation to block nested injection and newline/CR bypass.
    
    - **New Features**
    - Treat $(gh ...) as safe when the inner command has no shell
    metacharacters and uses an allowed read-only subcommand
    (repo/pr/issue/run/release/gist view, search, status, auth status,
    config get/list), and neutralize it before checks.
    - Added two good-command tests for gh api with $(gh repo view ...) and
    REPO=$(gh repo view ...) && patterns.
    
    - **Bug Fixes**
    - Fixed regex to prevent newline/CR bypass by using [ \t] and excluding
    \n\r in the inner content.
    - Added five bad-command tests to ensure destructive subcommands inside
    $(...) are blocked.
    
    <sup>Written for commit 8bcb142513e4dfab4689e0160d581f44f7b24e95.
    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>
    Co-authored-by: 's avatarclaude[bot] <41898282+claude[bot]@users.noreply.github.com>
    af50a6c4
名称
最后提交
最后更新
..
commands 正在载入提交数据...
hooks 正在载入提交数据...
skills 正在载入提交数据...
README.md 正在载入提交数据...
run-e2e-update.sh 正在载入提交数据...
settings.json 正在载入提交数据...