T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/create_pr.js:39
- Finding
- Shell Command Injection in Pull Request Creation<![CDATA[ ## Vulnerability Details **File Location**: `scripts/create_pr.js:39-52` **Vulnerability Type**: OS command injection through unsafe shell command construction **Risk Level**: High ### Vulnerable Code ```javascript // Build gh pr create command let cmd = `gh pr create --title "${title}" --body "${prBody}"`; if (branch) { cmd += ` --head ${branch}`; } if (draft) { cmd += ' --draft'; } if (labels) { cmd += ` --label "${labels}"`; } console.log(`\n🚀 Creating PR: ${title}`); console.log(`Branch: ${branch || 'current'}\n`); try { const output = execSync(cmd, { encoding: 'utf8' }); ``` ### Technical Analysis The `title`, `body`, `branch`, and `labels` values originate from command-line arguments and are interpolated into a command string executed by `child_process.execSync`. The PR body may also contain template file contents. `execSync` executes string commands through a system shell. Adding double quotes around selected values does not safely neutralize shell metacharacters. An attacker can include closing quotes, command separators, command substitution expressions, redirections, or other shell syntax in a supplied value. The `branch` value is especially exposed because it is inserted without any quoting. Consequently, the shell may interpret attacker-controlled text as an additional local command instead of as a literal GitHub CLI argument. ### Attack Path 1. An attacker convinces a user or automation process to invoke `create_pr.js` with a crafted `--title`, `--body`, `--branch`, or `--labels` value. 2. The script places the malicious value directly into the `cmd` string. 3. `execSync(cmd)` passes the resulting string to the operating-system shell. 4. The shell parses the injected operators and executes the attacker's command. 5. The injected command runs with the same operating-system privileges, environment variables, filesystem access, and authenticated tooling available to the user running the Skill. ### Impact Assessment Su ...[truncated 760 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions Avoid invoking the GitHub CLI through a shell command string. Use `execFileSync` or `spawnSync` with each argument supplied as a distinct array element: ```javascript const { execFileSync } = require('child_process'); const ghArgs = [ 'pr', 'create', '--title', title, '--body', prBody ]; if (branch) { ghArgs.push('--head', branch); } if (draft) { ghArgs.push('--draft'); } if (labels) { ghArgs.push('--label', labels); } const output = execFileSync('gh', ghArgs, { encoding: 'utf8', shell: false }); ``` Apply defense-in-depth controls as well: 1. Validate branch names against the expected Git reference syntax or resolve them through Git before use. 2. Validate labels using an explicit character and length policy. 3. Limit title and body lengths to reasonable values. 4. Validate template identifiers against a fixed allowlist such as `feature` and `bugfix`. 5. Do not attempt to implement custom shell escaping; argument-array execution is safer and less error-prone. 6. Run the Skill with the least-privileged GitHub token and operating-system account needed for PR creation. 7. Add automated tests containing quotes, semicolons, command substitutions, newlines, and shell redirection characters to verify that values remain literal arguments. ]]>