fix: close remaining shell-injection vectors in github-issues and git-diff#39
Open
kovtcharov wants to merge 1 commit intomainfrom
Open
fix: close remaining shell-injection vectors in github-issues and git-diff#39kovtcharov wants to merge 1 commit intomainfrom
kovtcharov wants to merge 1 commit intomainfrom
Conversation
…-diff PR #38 migrated most exec() → execFile() but left four sites with user-controlled input interpolated into shell strings: - /api/workspaces/git-diff — filePath was wrapped in quotes only; shell metacharacters in the query parameter were interpreted. - /api/workspaces/github-issues (GET) — state and assignee from the query string flowed unvalidated into the gh CLI invocation. - /api/workspaces/github-issues (POST) — title and body only had " escaped; backticks, \$(), and backslashes could still inject. - /api/workspaces/github-issues/:num (PATCH) — subcommand path migrated as defense in depth. state is now allowlisted (open|closed|all) and assignee is restricted to @me or a valid GitHub username pattern before reaching gh. Also add .catch() handlers to the post-spawn learnings and git-state promises in task-spawner so a rejection no longer surfaces as an unhandledRejection. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5 tasks
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.
Summary
Follow-up to #38. That PR migrated most
exec()→execFile()but left four endpoints where user-controlled input was still interpolated into shell strings:GET /api/workspaces/git-diff—filePathwrapped in quotes only. Backticks /$()/\in the query would execute.GET /api/workspaces/github-issues—stateandassigneefrom the query string flowed unvalidated intogh issue list.POST /api/workspaces/github-issues—title/bodyonly had"escaped; backticks,$(), and backslashes could still break out.PATCH /api/workspaces/github-issues/:num— migrated as defense-in-depth (owner/repo came from a remote-URL regex).All four now use
execFilewith an args array. Added:stateallowlist (open|closed|all)assigneepattern (@meor a valid GitHub username).catch()on the post-spawn learnings / git-state promises intask-spawner.tsso a rejection no longer becomes anunhandledRejection.Test plan
tsc --noEmit)git-diff?file=\whoami`.txt` → filename stays literal, no subshellgithub-issues?state=hacker;rm -rf /→ rejected with 400gh issue createwith title/body containing backticks and$()— should create issue with literal text (smoke test when gh-authed)🤖 Generated with Claude Code