T09 · Insecure Skill Coding Practices
Error
- Location
- SKILL.md:173
- Finding
- Shell Command Injection Through Unquoted Git Branch Names<![CDATA[ ## Vulnerability Details **File Location**: `SKILL.md`, lines 173–179 **Vulnerability Type**: Shell command injection in generated cleanup commands **Risk Level**: High ### Vulnerable Code ```bash # Delete merged local branches git branch -d feat/add-login feat/fix-typo chore/update-deps # Delete merged remote branches git push origin --delete feat/add-login feat/fix-typo chore/update-deps ``` The skill instructs the agent to replace the example branch names with branch names discovered from the repository and present the result as a copy-paste shell command. These repository-controlled names are inserted directly into shell command text without validation or shell-safe quoting. ### Technical Analysis Git reference names cannot contain certain characters, but they can contain several characters that have special meaning when included literally in a shell command. A maliciously constructed branch name containing a command separator or command-substitution syntax can alter the generated command's structure. For example, if a repository contains a branch whose name incorporates a semicolon followed by a command, inserting that name verbatim into the documented command causes the shell to treat the remainder as a separate command. This differs from ordinary shell variable expansion: the branch name becomes literal source text in a newly generated command before the user's shell parses it. The risk applies to both local deletion commands and remote deletion commands. Similar unsafe command-generation patterns are repeated in the report template, including the cleanup examples around lines 216–220 and 240–241. The use of `git branch -d` limits accidental deletion of unmerged branches, but it does not prevent the shell from interpreting metacharacters before Git processes its arguments. The optional PR status check also does not sanitize branch names for subsequent shell output. ### Attack Path 1. An attacker creates a branch with a valid Git refer ...[truncated 1486 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions 1. **Do not concatenate raw branch names into shell source text.** Prefer executing Git through an API that accepts an argument array, so branch names never pass through shell parsing. 2. **Validate every discovered reference before use.** Apply `git check-ref-format --branch` and reject any branch that does not satisfy the expected branch-name rules. Validation alone should not replace safe argument handling. 3. **Apply robust shell escaping when copy-paste commands are required.** Quote every branch name with a proven implementation such as Python's `shlex.quote`: ```python import shlex command = "git branch -d -- " + " ".join( shlex.quote(branch) for branch in branches ) ``` 4. **Use option delimiters where supported.** Place `--` before branch-name arguments to prevent a branch name from being interpreted as a command-line option: ```bash git branch -d -- 'branch-name' ``` 5. **Treat remote deletion separately.** Construct fully qualified deletion refspecs through an argument array rather than interpolating names into a shell command. Validate that each destination begins with the expected `refs/heads/` namespace. 6. **Refuse executable output for suspicious names.** If a branch contains shell metacharacters or cannot be represented safely, display it only as escaped diagnostic data and require manual handling. 7. **Require explicit confirmation before destructive operations.** Clearly separate audit output from executable deletion commands, and never characterize generated commands as automatically safe solely because branches appear merged. 8. **Add adversarial tests.** Test command rendering with branch names containing semicolons, dollar signs, parentheses, quotes, newlines where accepted, leading option-like characters, Unicode, and other unusual but valid reference-name characters. ]]>