Security audit

Address Review Comments

Security checks for vulnerabilities and agentic risk

Overview

The skill is transparent about its goal, but it allows PR comments to drive automatic commits, pushes, GitHub replies, and future watch-triggered rounds using the user's credentials.

Install only if you are comfortable with this skill using your GitHub credentials to modify a PR branch, push commits, and post comments automatically. Use it only on repositories where PR commenters are tightly trusted, verify the proposed diff before relying on the result, and confirm how any background watch can be listed and stopped before enabling the loop behavior.

A.I.G

Vulnerability Patterns

Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
Agent Memory PoisoningWrites attacker-controlled rules into memory that affect later sessions
Remote Payload Retrieval and ExecutionFetches external code whose behavior can change after review
Embedded Malicious CodeShips malicious scripts inside the skill and executes them locally

Findings (2)

T01 · Skill Instruction Hijacking

Error

Location: SKILL.md:18
Finding: Untrusted PR Comments Can Direct Repository Modifications
Content: View full analysis
Vulnerability Details

File Location: SKILL.md, lines 18–22, 194–201, and 221–232
Vulnerability Type: Untrusted instructions controlling privileged repository operations
Risk Level: High

Vulnerable Code Snippets

markdown
It decides which round to act on by matching PR comment CONTENT against a marker pattern (see Step 2), not by authenticating who posted it beyond GitHub's own login attribution — anyone who can comment on the PR (including, on some repos, the PR author replying to their own automated review) can shape which comment this skill treats as "the latest reviewer batch."

markdown
Identify the latest REVIEWER comment and the latest comment authored by us (the reply posted last round). Then branch: **Do not distinguish them by author.** The automated reviewer may post under the SAME GitHub account as our replies, so `.user.login` cannot tell them apart. Distinguish by content: reviewer batches carry a `` metadata block; our replies carry `codex-addressed:<batch_id>`.

markdown
3. **New findings**: the latest reviewer comment is newer than our last reply and contains findings. Proceed to Step 3.

markdown
## Step 3 — Apply the fixes (per-edit disk writes, asserted matches) Enumerate every finding in the latest reviewer comment. For each one, locate the exact current text (`grep -n` first — never edit from memory of the file), then apply the edit.

Technical Analysis

The skill treats a marker embedded in PR comment content as the trust signal for a reviewer batch. The marker and all associated findings are attacker-controlled text. The instructions explicitly decline to authenticate the batch by its author and acknowledge that anyone able to comment may shape the selected batch.

After selecting such a batch, the skill directs the agent to implement every finding, commit the resulting modifications, and push them using the invoking user's GitHub cr ...[truncated 1608 chars]
Remediation: View remediation
Remediation Suggestions

Authenticate reviewer batches against an explicit allowlist of immutable GitHub actor IDs, not display names, login text, or comment-body markers.

Fetch actor identity and repository role through the GitHub API and reject comments from unauthorized, suspended, or insufficiently privileged accounts.

Treat comment bodies as untrusted data. Ignore embedded instructions that attempt to alter the skill workflow, invoke tools, access secrets, or expand the requested scope.

Define an edit policy that restricts changes to files and findings directly relevant to the PR review.

Generate and display the proposed diff before any commit or push.

Require explicit human confirmation before privileged write operations, especially changes to CI workflows, executable scripts, dependencies, release configuration, and security controls.

Cryptographically or server-side bind each batch identifier to the authorized reviewer and PR rather than trusting a copyable HTML marker.

Record the validated actor ID, comment ID, and commit SHA in the reply for auditability.

Preserve the existing prohibition on merging, but do not rely on it as the primary control because pushed branch changes can still execute in CI or influence later merges.

T09 · Insecure Skill Coding Practices

Warning

Location: SKILL.md:44
Finding: Predictable Shared Temporary Paths Permit Local File Manipulation
Content: View full analysis
Vulnerability Details

File Location: SKILL.md, lines 44–48, 126–145, and 335–340
Vulnerability Type: Predictable temporary directory and unsafe file handling
Risk Level: Medium

Vulnerable Code Snippets

bash
SCRATCH="${TMPDIR:-/tmp}/pr${PR}-round"; mkdir -p "$SCRATCH"

bash
SCRATCH="${TMPDIR:-/tmp}/pr${PR}-round"; mkdir -p "$SCRATCH" # Brace the variable: `pr$PR_all_comments.txt` expands to `pr.txt` — one undefined name, # silently the wrong file. OUT="$SCRATCH/pr${PR}_all_comments.txt" # Write to a temp and move only on success. A failed page must not leave a SHORT history # file behind, because a short file is exactly what this block exists to prevent and # nothing downstream can tell one from a quiet PR. { for ep in "issues/$PR/comments:comment" "pulls/$PR/reviews:review" "pulls/$PR/comments:reply"; do gh api --paginate --slurp "repos/$REPO/${ep%%:*}" \ | jq --arg k "${ep##*:}" '[ .[][] | {kind:$k, who:.user.login, at:.created_at, body:(.body // "")} ]' done } | jq -s -r '[.[][]] | sort_by(.at) | .[] | "=== \(.kind) by \(.who) @ \(.at) ===\n\(if .body == "" then "(no body — inline comments only)" else .body end)\n"' \ > "$OUT.part" mv "$OUT.part" "$OUT"

bash
set -euo pipefail PR="$ARGUMENTS" case "$PR" in ''|*[!0-9]*) echo "PR must be numeric, got '$PR'" >&2; exit 1 ;; esac SCRATCH="${TMPDIR:-/tmp}/pr${PR}-round" gh pr comment "$PR" --body-file "$SCRATCH/reply.md"

Technical Analysis

The scratch directory name is derived solely from a predictable PR number and defaults to shared /tmp. The skill creates or reuses this directory with mkdir -p without validating its owner, permissions, or file type. It also does not protect individual paths against symbolic links.

The history file is written through a predictable *.part path and then moved. The reply is later read from a predictable reply.md path. A local attacker with access to the same temporary namespace can pre-create the direct ...[truncated 1484 chars]
Remediation: View remediation
Remediation Suggestions

Create a unique private directory using mktemp -d, and enforce permissions with umask 077 and mode 0700.

Persist the generated directory path through a protected host state mechanism instead of deriving it from a public PR number.

Verify that the scratch directory is owned by the current user, is not a symbolic link, and has no group or world permissions before every reuse.

Create files with exclusive creation and no-follow semantics where supported.

Validate that reply.md and history files are regular files owned by the current user immediately before use.

Avoid predictable .part names; use a unique file created inside the private directory and atomically rename it after successful generation.

Reject untrusted TMPDIR values unless the selected directory is validated as secure.

Remove the private scratch directory when the workflow reaches a terminal state.

Where cross-process continuity is required, pass a securely generated identifier or path explicitly rather than reconstructing a globally predictable path.

SkillSpector