Skip to content

Security hardening: escape generated HTML (XSS), validate manifest path, pin/lock deps - #11

Open
jmitchellol wants to merge 1 commit into
ryannadel:mainfrom
jmitchellol:security-hardening
Open

Security hardening: escape generated HTML (XSS), validate manifest path, pin/lock deps#11
jmitchellol wants to merge 1 commit into
ryannadel:mainfrom
jmitchellol:security-hardening

Conversation

@jmitchellol

@jmitchellol jmitchellol commented Jul 8, 2026

Copy link
Copy Markdown

Summary

Hardens the skill's generated artifacts and the MCP server against issues found in a security review.

1. Prevent XSS in generated HTML (highest impact)

SKILL.md tells the agent to embed real source snippets — and, in the dashboard, manifest name/description fields and imported manifests — into self-contained HTML that Phase 6 auto-opens in the browser. Nothing required escaping that content, so a source file (or imported manifest) containing markup like </pre><script>...</script> or <img src=x onerror=...> would execute as JavaScript when the guide/dashboard opens. This matters precisely because the skill is pointed at unfamiliar/inherited code.

  • Adds a mandatory "Escape All Interpolated Content" section to Phase 3 (escape & < > " '; never innerHTML with untrusted strings; prefer textContent).
  • Requires a restrictive Content-Security-Policy meta tag in every generated file — compatible with the inline-script design and blocks network exfiltration as defense in depth.
  • Mirrors the rules in Phase 5 (dashboard) and adds items to the quality checklists.

2. Validate the --manifest path (mcp/server.py)

The --manifest argument flowed unsanitized into the atomic write in save_manifest (Snyk CWE-23, Low). It is trusted operator config, but as defense in depth configure_manifest now requires it to resolve to a .json file.

3. Reproducible dependencies (mcp/)

uv.lock was git-ignored and mcp was pinned open-ended (>=1.2.0), so installs weren't reproducible and there was no lockfile to audit. Stops ignoring uv.lock and adds an upper bound (mcp>=1.2.0,<2). Run uv lock and commit the result to finish this.

Test plan

  • python3 -m py_compile mcp/server.py
  • Smoke test: module loads; coverage_summary / find_by_file work on the example manifest; a non-.json --manifest path is rejected
  • mcp/test_server.py passes (4/4). pytest isn't installable in this environment, so the exact test functions were run via a stdlib driver supplying the tmp_path fixture; all use .json manifest paths, so the new validation doesn't affect them.
  • Regenerated all four artifacts from the updated skill and verified in a headless browser: injected markup in source/manifest content renders as inert text (no <script>/<img> execution, no network callback), while an unescaped control does fire — confirming escaping is the effective defense. The CSP meta tag is present in every generated artifact as defense in depth.

Found via a security audit (Snyk SAST + package health + manual review).

Made with Cursor

…n risk

- SKILL.md: require HTML-escaping of all interpolated content and add a
  restrictive Content-Security-Policy to the generated guide and dashboard.
  Analyzed source (or an imported manifest) can contain markup, and the
  artifacts are auto-opened in a browser, so unescaped content was an XSS
  vector.
- server.py: validate that the --manifest path resolves to a .json file as
  defense in depth for the atomic manifest write (Snyk CWE-23).
- mcp: stop git-ignoring uv.lock and add an upper bound to the mcp dependency
  so installs are reproducible and auditable.

Co-authored-by: Cursor <cursoragent@cursor.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant