Security audit

PR-Proof-of-Work

Security checks for vulnerabilities and agentic risk

Overview

The skill fits its stated E2E screenshot PR workflow, but its bundled screenshot publishing script can force-push to a GitHub remote and post screenshots with weak guardrails.

Review before installing. Use this only in repositories where you are comfortable granting the agent GitHub issue, PR, comment, commit, and push access. Do not run the bundled screenshot comment script on sensitive screenshots or shared repos unless you first add remote validation, remove or constrain force-push behavior, use safe temporary files, and sanitize screenshot path names.

A.I.G

Vulnerability Patterns

Insecure Skill Coding PracticesFinds exploitable flaws such as hardcoded secrets or command injection
Skill Instruction HijackingAlters the agent's session goals or safety constraints when the skill loads
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 (3)

T09 · Insecure Skill Coding Practices

Warning

Location: scripts/pr-comment-screenshots.sh:70
Finding: Predictable temporary file permits local symlink-based file overwrite
Content: View full analysis
/tmp/pr-comment-body.md # ── 3. Post comment ── gh pr comment "$PR_NUM" --repo "$REPO" --body-file /tmp/pr-comment-body.md echo "Comment posted to PR #$PR_NUM" # Cleanup rm -rf "$TMP" /tmp/pr-comment-body.md ``` ### Technical Analysis The script writes the generated pull-request comment to the fixed, globally predictable path `/tmp/pr-comment-body.md`. Shared temporary directories are normally writable by every local user. Although the directory commonly has the sticky bit set, that protection does not prevent another user from creating the filename before this script runs. Shell output redirection follows symbolic links. Consequently, if an attacker creates `/tmp/pr-comment-body.md` as a symbolic link to another file, the redirection can truncate and overwrite the linked file using the privileges of the account running the script. The script already creates a private temporary directory with `mktemp -d`, but it does not place the comment body inside that directory. The final `rm -rf` normally unlinks the symbolic link itself rather than deleting its target; therefore, the confirmed security consequence is target-file overwrite or truncation, not target-file deletion. ### Attack Path 1. An attacker with access to the same host anticipates that another user or automated process will execute the script. 2. The attacker creates a symbolic link at the predictable location: ```bash ln -s /path/to/victim-writable-file /tmp/pr-comment-body.md ``` 3. The victim executes `pr-comment-screenshots.sh`. 4. The shell follows the symbolic link while processing: ```bash echo -e "$BODY" > /tmp/pr-comment-body.md ``` 5. The linked file is truncated and replaced with the generated Markdown content. 6. The GitHub CLI may then rea ...[truncated 690 chars]
Remediation: View remediation
"$BODY_FILE" gh pr comment "$PR_NUM" \ --repo "$REPO" \ --body-file "$BODY_FILE" ``` Additional hardening steps: 1. Use `printf` instead of `echo -e` to avoid implementation-dependent option and escape processing. 2. Keep every temporary artifact under the private `$TMP` directory. 3. Use `trap` so cleanup also occurs on errors and signals. 4. Quote all temporary paths and place `--` before paths passed to destructive commands. 5. Run the script with only the repository and filesystem permissions required for its task. ]]>

T09 · Insecure Skill Coding Practices

Warning

Location: assets/screenshot-reporter.ts:25
Finding: Unvalidated screenshot names allow filesystem path traversal
Content: View full analysis
{ if (!fs.existsSync(dir)) { const opts = { recursive: true } fs.mkdirSync(dir, opts) } } const writeManifest = () => { ensureDir() const manifestPath = path.join(dir, "manifest.json") const data = JSON.stringify(manifest, null, 2) fs.writeFileSync(manifestPath, data) } return { async capture(label, options = {}) { ensureDir() const filepath = path.join(dir, `${label}-${Date.now()}.png`) const shotOpts = { path: filepath, fullPage: options.fullPage ?? true } await page.screenshot(shotOpts) return filepath }, async captureBefore(label, options = {}) { ensureDir() const filepath = path.join(dir, `BEFORE-${label}-${Date.now()}.png`) const shotOpts = { path: filepath, fullPage: options.fullPage ?? true } await page.screenshot(shotOpts) manifest.before = filepath writeManifest() return filepath }, async captureAfter(label, options = {}) { ensureDir() const filepath = path.join(dir, `AFTER-${label}-${Date.now()}.png`) const shotOpts = { path: filepath, fullPage: options.fullPage ?? true } await page.screenshot(shotOpts) manifest.after = filepath writeManifest() return filepath }, ``` ### Technical Analysis The caller-controlled `testName` and `label` values are incorporated into filesystem paths without validation or containment checks. A `testName` containing parent-directory components such as `../../outside` causes `dir` to resolve out ...[truncated 2439 chars]
Remediation: View remediation

T09 · Insecure Skill Coding Practices

Warning

Location: scripts/pr-comment-screenshots.sh:27
Finding: Predictable screenshot branch is overwritten with an unconditional force push
Content: View full analysis
/dev/null || true git checkout -b "$BRANCH" 2>/dev/null || git checkout "$BRANCH" 2>/dev/null mkdir -p e2e-screenshots cp -r "$TMP"/* e2e-screenshots/ 2>/dev/null || true git add e2e-screenshots/ git commit -m "chore: add E2E screenshots for PR #${PR_NUM}" 2>/dev/null || true REMOTE=$(git remote -v | grep -m1 "(push)" | awk '{print $1}') git push "$REMOTE" "$BRANCH" --force 2>&1 || true git checkout "$CURRENT" 2>/dev/null ``` ### Technical Analysis The script derives a predictable branch name solely from the supplied PR number. If that branch already exists locally, the script silently checks it out. It then performs an unconditional `git push --force`, which allows the local branch state to replace remote branch history without verifying the expected remote commit. This violates safe concurrent-update practices. It can destroy commits placed on the same branch by another user, a previous run, or another automation process. The use of `|| true` suppresses push, commit, and copy failures, so the script may continue to post a comment even when the screenshot branch was not correctly updated. The script also does not validate `PR_NUM` as a decimal PR identifier before using it in a Git reference and commit message. Quoting prevents ordinary shell command injection, but strict input validation is still needed to ensure a valid, expected branch identifier. ### Attack Path 1. A branch named `e2e-screenshots-` already exists on the remote and contains commits from a prior run or another contributor. 2. A local branch with the same name exists but points to a different history, or the ...[truncated 1378 chars]
Remediation: View remediation
&2 exit 1 ;; esac ``` 2. Use a unique branch for each invocation, for example: ```bash RUN_ID="$(date +%s)-$$" BRANCH="e2e-screenshots-pr-${PR_NUM}-${RUN_ID}" ``` 3. Avoid force pushing. If updating an intentionally reusable branch is required, fetch its remote state and use `--force-with-lease` with an explicitly verified expected commit instead of `--force`. 4. Do not suppress critical failures: ```bash git push --set-upstream "$REMOTE" "$BRANCH" ``` Let `set -e` stop execution if copying, committing, pushing, or restoring repository state fails. 5. Restore the original checkout through a trap. Record the original commit as well as the branch so detached-HEAD operation is handled safely: ```bash ORIGINAL_REF=$(git rev-parse --verify HEAD) trap 'git checkout --detach "$ORIGINAL_REF" >/dev/null 2>&1 || true' EXIT ``` 6. Create the artifact branch through an isolated worktree or temporary clone. This avoids mutating the caller's working tree and reduces concurrency hazards. 7. Delete remote artifact branches after the PR closes, or enforce a documented retention policy. 8. Verify that the pushed commit contains both expected screenshots before posting the GitHub comment. ]]>

SkillSpector

Vulnerability Patterns

Tool MisuseTool Parameter Abuse, Chaining Abuse, Unsafe Defaults
MCP Least PrivilegeUnderdeclared Capability, Wildcard Permission, Missing Permission Declaration
MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration

Findings (8)

Ae1

High

Category: analysis-evasion
Confidence: 100% confidence
Finding: Referenced artifact was not completely inspected
Content: Scanner excerpt · SKILL.md (reported line 45)May include surrounding context.
md
3. Copy [screenshot-reporter.ts](assets/screenshot-reporter.ts) into the worktree's e2e/ directory (not tracked on fix branches)

Ae1

High

Category: analysis-evasion
Confidence: 100% confidence
Finding: Referenced artifact was not completely inspected
Content: Scanner excerpt · SKILL.md (reported line 215)May include surrounding context.
md
3. Copy [screenshot-reporter.ts](assets/screenshot-reporter.ts) into the worktree's e2e/ directory (not tracked on fix branches)

Tool Parameter Abuse

High

Category: Tool Misuse
Confidence: 97% confidence
Finding: The push target is chosen from the first configured push remote and then force-pushed automatically. In an agent skill context, that is dangerous because a caller may not realize which remote will be modified, and the script can overwrite branch contents on an unintended destination while also uploading potentially sensitive screenshots.
Content: Scanner excerpt · scripts/pr-comment-screenshots.sh (reported line 39)May include surrounding context.
sh
git commit -m "chore: add E2E screenshots for PR #${PR_NUM}" 2>/dev/null || true REMOTE=$(git remote -v | grep -m1 "(push)" | awk '{print $1}') git push "$REMOTE" "$BRANCH" --force 2>&1 || true git checkout "$CURRENT" 2>/dev/null # ── 2. Build comment from manifests ──

Tool Parameter Abuse

High

Category: Tool Misuse
Confidence: 100% confidence
Finding: Tool parameters are crafted to achieve unintended or unsafe behavior. Parameter abuse can bypass intended safety checks (e.g. shell=True, --force, dangerous glob patterns).
Content: Scanner excerpt · scripts/pr-comment-screenshots.sh (reported line 80)May include surrounding context.
sh
echo "Comment posted to PR #$PR_NUM" # Cleanup rm -rf "$TMP" /tmp/pr-comment-body.md

Undeclared Tool Scope

Medium

Category: MCP Least Privilege
Confidence: 82% confidence
Finding: The skill instructs the agent to use environment-dependent behavior (export E2E_SCREENSHOT_DIR=...) and to interact with local git worktrees, but it declares no explicit tool scope or permissions metadata. In a system that relies on skill-declared scopes for sandboxing or review, this can cause the agent to access environment state or perform side effects without clear authorization boundaries, increasing the chance of unintended data exposure or unsafe execution.
Content: No source excerpt is available for this finding.

Intent-Code Divergence

Medium

Category: Not specified by scanner
Confidence: 92% confidence
Finding: The header says the script 'uploads images to a temporary branch' while the actual implementation creates commits and force-pushes to a git remote. This understatement is security-relevant because operators may run the script without understanding that repository state will be modified and data will be published to a remote branch.
Content: No source excerpt is available for this finding.

Description-Behavior Mismatch

Medium

Category: Not specified by scanner
Confidence: 98% confidence
Finding: The script does more than post a PR comment: it copies local screenshots into the repository, creates or reuses a branch, commits them, and force-pushes that branch to a remote. That behavior can publish potentially sensitive screenshots outside the immediate PR discussion and creates a data-exfiltration path if screenshots contain secrets, internal URLs, or customer data.
Content: No source excerpt is available for this finding.

Missing User Warnings

Medium

Category: Not specified by scanner
Confidence: 95% confidence
Finding: The script force-pushes a branch automatically with no warning, dry-run, or confirmation. Even though the branch name is derived from the PR number, this can overwrite existing branch history, surprise users, and publish data to a remote without an explicit acknowledgment.
Content: No source excerpt is available for this finding.

Static analysis

No suspicious patterns detected.