T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/create_pr.py:10
- Finding
- Shell Command Injection and Guardrail Bypass in Pull Request Creation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/create_pr.py:10-12, 58-71` **Vulnerability Type**: Shell command injection and missing authorization enforcement **Risk Level**: Critical ### Vulnerable Code ```python def run_cmd(cmd, check=True): """Run shell command and return output.""" result = subprocess.run(cmd, shell=True, capture_output=True, text=True) if check and result.returncode != 0: print(f"Command failed: {cmd}", file=sys.stderr) print(f"Error: {result.stderr}", file=sys.stderr) sys.exit(1) return result.stdout.strip() ``` ```python # Create branch if not exists current_branch = run_cmd("git branch --show-current") if current_branch != branch_name: run_cmd(f"git checkout -b {branch_name}") # Push branch print(f"Pushing branch {branch_name}...", file=sys.stderr) run_cmd(f"git push -u origin {branch_name}") # Create PR print("Creating Pull Request...", file=sys.stderr) cmd = f'gh pr create --title "{title}" --body-file "{body_file}"' try: pr_url = run_cmd(cmd) ``` ### Technical Analysis The script constructs shell command strings using the CLI-controlled `branch_name`, `title`, and `body_file` values and executes them through `subprocess.run(..., shell=True)`. These values are not escaped, validated, or passed as individual argument-array elements. An attacker can place shell syntax in one of these arguments. For example, a malicious title can terminate the quoted `--title` value, insert another shell command, and comment out or otherwise neutralize the remaining command text. Branch and body-file arguments provide additional injection surfaces. The script also directly performs branch creation, push, and pull-request creation without calling `Guardrails`, `Sandbox`, or an approval-verification mechanism. This contradicts the Skill's documented requirements that pushing and pull-request creation require explicit user approval. The command also omits `--draft`, despite the confi ...[truncated 1434 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. Remove `shell=True` and pass commands as argument arrays: ```python subprocess.run( ["git", "checkout", "-b", branch_name], shell=False, capture_output=True, text=True, check=True, ) ``` 2. Invoke GitHub CLI in the same manner: ```python subprocess.run( ["gh", "pr", "create", "--draft", "--title", title, "--body-file", body_file], shell=False, capture_output=True, text=True, check=True, ) ``` 3. Validate `branch_name` through `Guardrails.check_branch()` and reject names that do not use the configured working-branch prefix. 4. Resolve and validate `body_file` against the intended repository root. Reject absolute paths, traversal, symlinks escaping the repository, and denied sensitive paths. 5. Require a cryptographically unpredictable, short-lived approval artifact bound to: - The exact repository. - The exact branch and commit. - The exact push operation. - The pull-request title and draft status. 6. Perform push and pull-request creation only through the hardened sandbox implementation. 7. Add tests covering quote characters, semicolons, command substitution, newlines, option injection, path traversal, and malicious branch names. 8. Enforce `--draft` programmatically rather than relying on documentation or configuration alone. ]]>