Back to skill

Security audit

AI Code Review

Security checks for vulnerabilities and agentic risk

Overview

The skill is mostly purpose-aligned, but it overstates its security posture and includes under-disclosed high-impact behavior and file/network handling weaknesses.

Review this carefully before installing or running. Only use it in a controlled environment, restrict allowed diff sources, avoid sensitive voice-note directories, protect webhook/API secrets, and do not run the healthcheck with broad system privileges unless you explicitly want it to restart those services.

Vulnerability Patterns
  • Unauthorized Access and Privilege EscalationObtains permissions beyond the task's legitimate needs
  • 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
Findings (4)

T09 · Insecure Skill Coding Practices

Error
Location
references/code_review_service.py:49
Finding

Incomplete SSRF Protection for Pull Request Diff Retrieval

Content
View full analysis
None: """Validate that a diff URL is safe to fetch (prevents SSRF).""" parsed = urlparse(url) if parsed.scheme not in ALLOWED_DIFF_SCHEMES: raise ValueError(f"Unsupported URL scheme '{parsed.scheme}' for diff URL. Allowed: {ALLOWED_DIFF_SCHEMES}") hostname = parsed.hostname or "" # Block internal/private network addresses if hostname in ("localhost", "127.0.0.1", "0.0.0.0", "::1") or hostname.startswith("10.") or hostname.startswith("192.168.") or hostname.startswith("172."): if not os.getenv("ALLOW_INTERNAL_DIFF_URLS"): raise ValueError(f"Diff URL points to internal address '{hostname}', which is blocked for security") if not hostname: raise ValueError("Diff URL has no hostname") ``` The validated URL is subsequently fetched as follows: ```python if diff_url: _validate_diff_url(diff_url) try: resp = requests.get(diff_url, timeout=REQUEST_TIMEOUT) resp.raise_for_status() diff_content = resp.text ``` ### Technical Analysis The validation checks only the textual hostname supplied in the URL. It does not resolve the hostname and inspect all resulting IP addresses. Consequently, it does not reliably reject destinations in loopback, private, link-local, reserved, unspecified, multicast, or other non-public address ranges. Specific bypass conditions include: - Link-local addresses such as `169.254.169.254`, commonly used by cloud metadata services. - Other loopback representations not explicitly listed. - Hostnames that resolve to private or loopback addresses. - DNS rebinding, where the hostname resolves differently bet ...[truncated 1658 chars]
Remediation
View remediation

T09 · Insecure Skill Coding Practices

Error
Location
references/code_review_service.py:65
Finding

Voice-Note Sandbox Bypass Can Upload Files Outside the Intended Directory

Content
View full analysis
Path: """Validate and resolve voice note path to prevent path traversal.""" resolved = Path(file_path).resolve() if not str(resolved).startswith(str(VOICE_NOTE_BASE_DIR)): raise ValueError(f"Voice note path '{file_path}' is outside the allowed directory '{VOICE_NOTE_BASE_DIR}'") if not resolved.is_file(): raise FileNotFoundError(f"Voice note file not found: {resolved}") return resolved ``` The accepted file is transmitted to OpenAI: ```python def transcribe_voice_note(audio_file_path: str) -> str: """Transcribe voice note from code review meetings using Whisper API.""" client = _get_openai_client() resolved_path = _validate_voice_note_path(audio_file_path) with open(resolved_path, "rb") as audio_file: transcription = client.audio.transcriptions.create( model="whisper-1", file=audio_file, ) ``` ### Technical Analysis Directory containment is tested using a string-prefix comparison: ```python str(resolved).startswith(str(VOICE_NOTE_BASE_DIR)) ``` A textual prefix does not establish a filesystem ancestry relationship. With the default base directory `/tmp/voice_notes`, paths such as `/tmp/voice_notes_backup/secret` pass the check even though they are outside `/tmp/voice_notes`. `Path.resolve()` normalizes traversal components and resolves symlinks, but it does not make the subsequent string-prefix check safe. No file extension, MIME type, maximum size, ownership, or sensitivity validation is performed before the file is opened and sent to an external service. ### Attack Path 1. An attacker identifies or creates a re ...[truncated 1220 chars]
Remediation
View remediation

T05 · Unauthorized Access and Privilege Escalation

Warning
Location
scripts/healthcheck.sh:43
Finding

Health Check Automatically Performs Privileged Service Restarts

Content
View full analysis
/dev/null 2>&1; then if systemctl is-active --quiet "$service"; then echo -e "${GREEN}✅ $service restarted successfully${NC}" log "$service restarted successfully" send_alert "Service $service was down and successfully restarted" else echo -e "${RED}❌ Failed to restart $service${NC}" log "Failed to restart $service" send_alert "CRITICAL: Service $service is down and failed to restart - manual intervention required" fi else echo -e "${RED}❌ Failed to restart $service${NC}" log "Failed to restart $service" send_alert "CRITICAL: Service $service is down and failed to restart - manual intervention required" fi fi done ``` ### Technical Analysis A passive health check only needs permission to inspect service state and disk usage. This script additionally invokes `systemctl restart` for nginx, Docker, the code-review service, a ...[truncated 2020 chars]
Remediation
View remediation

T09 · Insecure Skill Coding Practices

Note
Location
scripts/healthcheck.sh:20
Finding

Discord Health Alert Request Has No Network Timeout

Content
View full analysis
/dev/null 2>&1 } ``` ### Technical Analysis The `curl` command has neither a connection timeout nor a maximum execution time. A webhook server can accept a connection and then delay or never complete the response, leaving the health-check process blocked indefinitely. This contradicts the statement in `SKILL.md` that all HTTP requests enforce a 30-second timeout. Because alerts occur while iterating through service checks and disk checks, one blocked request can prevent the remaining checks and final status from completing. The webhook URL is configuration-controlled rather than directly supplied by an unauthenticated caller, which limits exploitability. Nevertheless, endpoint compromise, DNS manipulation, network failure, or configuration error can trigger the condition. ### Attack Path 1. `DISCORD_WEBHOOK_URL` is configured to use an endpoint that is unavailable, compromised, or intentionally slow. 2. A service failure or disk threshold invokes `send_alert`. 3. `curl` connects to the endpoint or waits indefinitely during connection establishment. 4. Because no timeout is configured, the health-check process does not progress. 5. Subsequent service checks, logging, and the final exit status are delayed indefinitely. ### Impact Assessment The issue affects availability and monitoring reliability rather than confidentiality or code execution. Potential consequences include: - Hung scheduled health-check processes. - Accumulation of overlapping monitoring jo ...[truncated 206 chars]
Remediation
View remediation
Vulnerability Patterns
  • Data ExfiltrationExternal Transmission, Env Variable Harvesting, File System Enumeration
  • Taint TrackingDirect Taint Flow, Variable-Mediated Taint Flow, Credential Exfiltration Chain
  • MCP Tool PoisoningHidden Instructions, Unicode Deception, Parameter Description Injection
  • Prompt InjectionInstruction Override, Hidden Instructions, Exfiltration Commands
  • Privilege EscalationExcessive Permissions, Sudo/Root Execution, Credential Access
Findings (7)

Tainted flow: 'DISCORD_WEBHOOK_URL' from os.getenv (line 27, credential/environment) → requests.post (network output)

Critical
Category
Data Flow
Confidence
90% confidence
Finding

Credentials or environment variables flow to a network sink. This is a high-confidence indicator of credential exfiltration.

Content

Scanner excerpt · references/code_review_service.py (reported line 108)May include surrounding context.

python
payload["embeds"] = [safe_embed]

    try:
        response = requests.post(
            DISCORD_WEBHOOK_URL,
            headers={"Content-Type": "application/json"},
            data=json.dumps(payload),

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
95% confidence
Finding

The module docstring presents this service as providing 'AI Code Review Service' with 'Code review analysis' integration, which implies real automated review behavior. However, analyze_code_changes later returns a hardcoded placeholder result and explicitly states the actual LLM-based review logic is not yet implemented, so the documentation overstates what the code currently does.

Content

No source excerpt is available for this finding.

External Transmission

Medium
Category
Data Exfiltration
Confidence
60% confidence
Finding

Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

Content

Scanner excerpt · references/code_review_service.py (reported line 28)May include surrounding context.

python
# Configuration from environment variables
OPENAI_API_KEY = os.getenv("OPENAI_API_KEY")
DISCORD_WEBHOOK_URL = os.getenv("DISCORD_WEBHOOK_URL")
CLAWHUB_API_URL = os.getenv("CLAWHUB_API_URL", "https://api.clawhub.com/v1")

# Allowed diff URL schemes to prevent SSRF
ALLOWED_DIFF_SCHEMES = {"https", "http"}

Intent-Code Divergence

Medium
Category
Not specified by scanner
Confidence
98% confidence
Finding

The function docstring says 'Publish a skill to ClawHub,' which describes a concrete side-effecting operation. In reality, the function only validates inputs, logs that publishing is not yet implemented, and returns True, creating a direct contradiction between the documented intent and actual behavior.

Content

No source excerpt is available for this finding.

External Transmission

Medium
Category
Data Exfiltration
Confidence
70% confidence
Finding

Data is being sent to an external URL. This could be legitimate telemetry or data exfiltration. Manual review is recommended.

Content

Scanner excerpt · scripts/healthcheck.sh (reported line 28)May include surrounding context.

sh
return
    fi
    
    curl -s -H "Content-Type: application/json" \
         -X POST \
         -d "{\"content\": \"🚨 **HEALTH ALERT**: $message\"}" \
         "$DISCORD_WEBHOOK_URL" > /dev/null 2>&1

Missing User Warnings

Medium
Category
Not specified by scanner
Confidence
88% confidence
Finding

The script automatically restarts services when they are found unhealthy, which is a safety-critical operation that can affect system availability and active workloads. Although the action is echoed at runtime, the file provides no upfront warning in comments or documentation that this health check will modify system state rather than only observe it.

Content

No source excerpt is available for this finding.

Missing User Warnings

Low
Category
Not specified by scanner
Confidence
85% confidence
Finding

The script sends service-status and disk-usage alerts to a Discord webhook, which is a network transmission of system operational data. While comments mention alerts to Discord, there is no explicit warning that health information will be sent to an external endpoint when the webhook variable is configured.

Content

No source excerpt is available for this finding.

Static analysis

No suspicious patterns detected.