T09 · Insecure Skill Coding Practices
Warning
- Location
- SKILL.md:220
- Finding
- Predictable temporary review file permits symlink overwrite and review-payload tampering## Vulnerability Details **File Location**: `SKILL.md:220-242` **Vulnerability Type**: Predictable unsafe temporary file **Risk Level**: Medium ### Vulnerable Code ```bash cat > /tmp/pr-review.json <<'EOF' { "event": "REQUEST_CHANGES", "body": "1 critical, 1 minor - details inline on the diff.", "comments": [ { "path": "src/main.rs", "line": 1653, "side": "RIGHT", "body": "[Correctness] `fmt::layer()` defaults to stdout, moving all tracing output onto the JSON-RPC channel.\n\n```suggestion\n .with(fmt::layer().with_writer(std::io::stderr))\n```" }, { "path": "src/main.rs", "start_line": 1651, "line": 1654, "side": "RIGHT", "body": "[Convention] The stderr choice deserves a WHY comment - it is the only thing keeping stdout clean for JSON-RPC." } ] } EOF gh api repos/{owner}/{repo}/pulls/<number>/reviews --input /tmp/pr-review.json ``` ### Technical Analysis The skill instructs the agent to create a sensitive API request payload at the fixed, globally predictable path `/tmp/pr-review.json`. Shell output redirection normally follows symbolic links and does not create the file atomically with exclusive ownership checks. A local attacker able to manipulate that path may exploit two distinct time-of-check/time-of-use opportunities: 1. Before redirection, the attacker can place a symbolic link at `/tmp/pr-review.json`. The shell may then truncate and overwrite the link target with the review JSON. 2. After the heredoc finishes but before `gh api` opens the file, the attacker can replace or alter the predictable file. The GitHub CLI would then transmit attacker-controlled review content using the victim's authenticated GitHub credentials. Common Linux protections such as `fs.protected_symlinks` and sticky-directory ownership restrictions can reduce cross-user exploitation, but they are platform-d ...[truncated 2693 chars]
- Remediation
- ## Remediation Suggestions Replace the fixed filename with a securely and atomically created temporary file, restrict its permissions, pass the exact generated path to `gh`, and guarantee cleanup: ```bash review_file="$(mktemp "${TMPDIR:-/tmp}/pr-review.XXXXXXXX.json")" || exit 1 chmod 600 "$review_file" trap 'rm -f -- "$review_file"' EXIT HUP INT TERM cat > "$review_file" <<'EOF' { "event": "REQUEST_CHANGES", "body": "1 critical, 1 minor - details inline on the diff.", "comments": [] } EOF gh api "repos/{owner}/{repo}/pulls/<number>/reviews" --input "$review_file" ``` Additional hardening should include: - Prefer piping the generated payload directly to `gh api --input -` if supported, eliminating the temporary file. - Generate JSON with a structured serializer such as `jq` rather than interpolating untrusted strings into a heredoc. - Keep the existing explicit user-confirmation requirement before transmission. - Validate the final repository, pull-request number, review event, and payload summary immediately before submission. - Avoid printing tokens or authenticated command environments in logs. - Ensure cleanup occurs on successful completion, errors, and interruption. - If a temporary file must be reopened, verify that it remains a regular file owned by the current user and has not been replaced before submission.