Skip to content

Security: XSS via unescaped JSON in <script> tags and missing CSRF protection in skill-creator #13

@blackjlc

Description

@blackjlc

Security: XSS via unescaped JSON in <script> tags and missing CSRF protection in skill-creator

During a security review of locally installed developer tools, I found several vulnerabilities in the skill-creator skill's eval viewer. Line numbers refer to the installed version at time of review; paths are relative to the repo root.

1. XSS via unescaped JSON in <script> tags — MEDIUM

Location A: skills/skill-creator/eval-viewer/generate_review.py:279-281

data_json = json.dumps(embedded)
return template.replace("/*__EMBEDDED_DATA__*/", f"const EMBEDDED_DATA = {data_json};")

Python's json.dumps() does not escape < characters. The embedded data includes:

  • run.prompt — eval prompts (user-provided text)
  • Output file contents — text files read from eval run outputs (could contain arbitrary text)
  • Grading expectations and evidence — generated by the grader agent, may quote from outputs

If any of these contain the string </script>, it will break out of the <script> tag in viewer.html and allow injection of arbitrary HTML/JavaScript when the eval results are viewed in a browser. The viewer is served via a local HTTP server (generate_review.py:443) or written as a static HTML file, and automatically opened via webbrowser.open().

Exploitation scenario: A test prompt like "How do I escape </script> tags in HTML?" or an output file containing </script><img src=x onerror=alert(1)> would execute JavaScript in the browser context.

Location B: skills/skill-creator/assets/eval_review.html:63

const EVAL_DATA = __EVAL_DATA_PLACEHOLDER__;

The SKILL.md (lines 366-367) instructs the agent to replace __EVAL_DATA_PLACEHOLDER__ with "the JSON array of eval items (no quotes around it — it's a JS variable assignment)". If the agent uses standard JSON.stringify() (JavaScript) or json.dumps() (Python) to serialize eval queries, < characters are not escaped. Eval queries containing </script> would break the script tag and allow XSS when the HTML file is opened via open /tmp/eval_review_<skill-name>.html.

Note: quick_validate.py rejects < and > in skill descriptions (line 80-81) and enforces kebab-case names (line 65), which mitigates XSS via __SKILL_NAME_PLACEHOLDER__ and __SKILL_DESCRIPTION_PLACEHOLDER__. But eval queries are not sanitized.

Fix for both locations: Escape < to \u003c (and optionally > to \u003e, & to \u0026) in the JSON output before embedding:

data_json = json.dumps(embedded).replace("<", "\\u003c").replace(">", "\\u003e").replace("&", "\\u0026")

And update the SKILL.md instruction for __EVAL_DATA_PLACEHOLDER__ to require the same escaping.

2. Missing CSRF protection on feedback endpoint — LOW

File: skills/skill-creator/eval-viewer/generate_review.py:361-378

The HTTP server's do_POST handler for /api/feedback does not check the Origin or Content-Type headers. Any website the user visits while the eval viewer server is running (bound to 127.0.0.1:3117) could send a POST request via JavaScript to overwrite feedback.json.

Impact is limited to corrupting feedback data (displayed via textContent, so no XSS), and the server only runs during active eval review sessions.

Fix: Check the Origin header and reject requests from non-localhost origins:

origin = self.headers.get("Origin", "")
if origin and "127.0.0.1" not in origin and "localhost" not in origin:
    # reject with 403

3. Process killing on port 3117 — LOW

File: skills/skill-creator/eval-viewer/generate_review.py:288-306

The _kill_port() function sends SIGTERM to any process listening on the target port (default 3117). If another local process happens to use that port, it would be terminated.

The port is configurable via --port and the function uses safe subprocess calls (list arguments, no shell interpolation), so this is low severity. Consider checking if the process is a child of the current process before killing, or using a more specific port range.

Notes

Other scripts in skill-creator use safe patterns:

  • improve_description.py and run_eval.py use subprocess.run()/Popen() with list arguments (not shell=True) — no command injection.
  • quick_validate.py correctly uses yaml.safe_load().
  • generate_report.py correctly uses html.escape() for all untrusted data in HTML output.
  • viewer.html uses textContent for text outputs and escapeHtml() for grading data.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions