T09 · Insecure Skill Coding Practices
Error
- Location
- scripts/review-pr.sh:69
- Finding
- Arbitrary Shell Command Injection Through Pull Request Metadata and Review Prompts<![CDATA[ ## Vulnerability Details **File Location**: `scripts/review-pr.sh:69-82, 113-150, 156-164` **Vulnerability Type**: Shell command injection through `eval` **Risk Level**: Critical ### Vulnerable Code ```bash PR_TITLE=$(echo "$PR_JSON" | python3 -c "import json,sys;print(json.load(sys.stdin).get('title',''))") PR_BRANCH=$(echo "$PR_JSON" | python3 -c "import json,sys;print(json.load(sys.stdin).get('headRefName',''))") PR_BASE=$(echo "$PR_JSON" | python3 -c "import json,sys;print(json.load(sys.stdin).get('baseRefName','main'))") PR_BODY=$(echo "$PR_JSON" | python3 -c "import json,sys;d=json.load(sys.stdin);print(d.get('body','')[:500])") if [ -n "$CUSTOM_PROMPT" ]; then REVIEW_PROMPT="$CUSTOM_PROMPT" else REVIEW_PROMPT="Review this pull request thoroughly. PR #${PR_NUM}: ${PR_TITLE} Branch: ${PR_BRANCH} → ${PR_BASE} Changes: +${PR_ADDITIONS} -${PR_DELETIONS} ${PR_BODY:+Description: ${PR_BODY}} Review guidelines: 1. Check for bugs, logic errors, and edge cases 2. Review code style and consistency 3. Look for security vulnerabilities 4. Check test coverage 5. Evaluate naming and documentation 6. Note any performance concerns Read the changed files, understand the context, and provide a structured review with: - Summary of changes - Issues found (critical, major, minor) - Suggestions for improvement - Overall assessment (approve, request changes, or comment) Write your final review to /tmp/pr-review-${PR_NUM}.md When completely finished, run: openclaw system event --text 'Done: PR #${PR_NUM} review complete' --mode now" fi case "$AGENT" in claude*) AGENT_CMD="claude -p --dangerously-skip-permissions --output-format stream-json --verbose '${REVIEW_PROMPT}'" ;; codex*) AGENT_CMD="codex exec --json --full-auto '${REVIEW_PROMPT}'" ;; *) AGENT_CMD="${AGENT} '${REVIEW_PROMPT}'" ;; esac RELAY_FLAGS="-w $WORKDIR -t $TIMEOUT -n '${AGENT} Review'" [ "$THREAD_MODE" = true ] && RELAY_FLAGS="$RELAY_FLAGS --thread" [ "$SKIP_REA ...[truncated 1898 chars]
- Remediation
- <![CDATA[ ## Remediation Suggestions - Remove `eval` entirely. - Build every command as a Bash array, with each prompt and option stored as a distinct array element. - Pass PR metadata only as data, never as shell source. - Restrict `AGENT` to an explicit allowlist such as `claude` or `codex`. - Validate numeric options before use. - Avoid global permission-bypass flags. A safe construction should follow this pattern: ```bash case "$AGENT" in claude) AGENT_CMD=( claude -p --output-format stream-json --verbose "$REVIEW_PROMPT" ) ;; codex) AGENT_CMD=(codex exec --json "$REVIEW_PROMPT") ;; *) echo "Unsupported agent" >&2 exit 1 ;; esac RELAY_ARGS=(-w "$WORKDIR" -t "$TIMEOUT" -n "$AGENT Review") [ "$THREAD_MODE" = true ] && RELAY_ARGS+=(--thread) [ "$SKIP_READS" = true ] && RELAY_ARGS+=(--skip-reads) [ -n "$RATE_LIMIT" ] && RELAY_ARGS+=(-r "$RATE_LIMIT") bash "$SCRIPT_DIR/dev-relay.sh" "${RELAY_ARGS[@]}" -- "${AGENT_CMD[@]}" ``` ]]>
