Skip to content

Harden serve defaults and close XSS/path injection sinks - #79

Open
batmnnn wants to merge 1 commit into
adrida:mainfrom
batmnnn:fix/security-harden-serve-reports-watch
Open

batmnnn wants to merge 1 commit into
adrida:mainfrom
batmnnn:fix/security-harden-serve-reports-watch

Conversation

@batmnnn

@batmnnn batmnnn commented Aug 12, 2026 •

Copy link
Copy Markdown

Summary

  • Make tracer serve loopback-by-default (127.0.0.1), with opt-in CORS, request body / batch / embedding-element caps, socket timeouts, and Connection: close on error responses — so a local demo server is not an unauthenticated LAN/WAN endpoint by accident.
  • Close stored XSS sinks in fit HTML reports and scan viz (escape/coerce manifest fields, sanitize Sankey labels, \u003c-safe viz JSON) and path traversal in watch LocalFileSink via an allowlisted filename (Python + @tracer-llm/watch).
  • Document the operator trust model in SECURITY.md (serve has no auth; .tracer / pipeline.joblib is trusted code / pickle). This PR does not eliminate pickle RCE — it makes the trust boundary explicit.

Why this shape

One reviewable theme: secure-by-default local surface + injection sink closure. Follow-ups intentionally deferred: Actions SHA pinning, cloud login/session/put_url hardening, artifact signing.

Compatibility notes

  • Breaking-ish default: tracer serve / serve() now bind 127.0.0.1 (was 0.0.0.0). Use --host 0.0.0.0 only behind a reverse proxy with auth/TLS.
  • CORS: no Access-Control-Allow-Origin unless --cors-origin is set (related enhancement #71 / #70 should keep opt-in defaults, not restore *).
  • Watcher names must match [A-Za-z0-9][A-Za-z0-9._-]{0,127}.

Test plan

  • pytest tests/test_serve.py tests/test_watch.py tests/test_html_report_escape.py -v (all passed locally)
  • Confirm tracer serve --help shows host default 127.0.0.1 and documents --cors-origin / body caps
  • Open a report with poisoned trace text / embedding_dim — no raw <script> / <img onerror>
  • LocalFileSink("../evil") raises; safe names still write under the watch dir
  • With --host 0.0.0.0, warning is printed; without auth this remains operator responsibility

Made with Cursor

Make tracer serve loopback-only with opt-in CORS and request/batch caps, escape untrusted report/scan content (including sankey/viz), and allowlist watch sink filenames so local telemetry cannot traverse the filesystem.

Co-authored-by: Cursor <cursoragent@cursor.com>
Copilot AI lite review requested due to automatic review settings August 12, 2026 20:38
@batmnnn
batmnnn requested a review from adrida as a code owner August 12, 2026 20:38

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens TRACER’s local HTTP serving defaults and closes multiple injection sinks by making tracer serve loopback-by-default with opt-in CORS and request caps, tightening file naming to prevent watch path traversal, and escaping/sanitizing untrusted fields in generated HTML (fit reports and scan viz). It also documents the operator trust model (notably: serve has no auth and .tracer/pipeline.joblib is trusted pickle code).

Changes:

  • Harden tracer serve: bind 127.0.0.1 by default, opt-in CORS, body/batch/element caps, and close connections on error responses.
  • Prevent path traversal in watch sinks via allowlisted watcher names (Python + JS) and root-escape checks.
  • Close stored-XSS/script-breakout vectors in HTML outputs (fit report fields, sankey labels, scan viz embedded JSON) and add/extend security documentation and tests.

Reviewed changes

Copilot reviewed 16 out of 17 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/test_watch.py Adds regression tests for rejecting unsafe watcher names and allowing safe names.
tests/test_serve.py Updates serve tests to use fit/FitConfig directly and adds tests for loopback default, body/batch caps, CORS default-off, and Connection: close on errors.
tests/test_html_report_escape.py New tests covering HTML report escaping, sankey label sanitization, and scan viz JSON escaping.
src/tracer/watch.py Adds allowlisted watcher-name validation and prevents file path escape from the watch directory.
src/tracer/scanner.py Escapes embedded viz JSON to prevent </script> breakout; hardens HTML escaping helper.
src/tracer/runtime/serve.py Implements loopback-by-default serving, opt-in CORS/preflight, request caps, and error-response connection closing.
src/tracer/policy/artifacts.py Adds explicit warning docstring about pickle/joblib trust boundary when loading pipelines.
src/tracer/cli/main.py Wires new serve CLI flags (host default, cors origin, body cap, batch cap) into runtime serve.
src/tracer/analysis/sankey.py Sanitizes node labels to prevent markup/script injection in Plotly HTML.
src/tracer/analysis/html_report.py Escapes/coerces dynamic manifest/report fields; pins Plotly CDN version and injects only when needed.
SECURITY.md Documents operator trust model: .tracer artifacts are trusted code; serve has no auth; watch can exfiltrate.
js/test/watch.test.ts Adds JS tests for rejecting unsafe watch names.
js/src/index.ts Adds JS allowlist validator for watch filenames and prevents directory escape via resolved path checks.
docs/javascript.md Updates docs to reflect tracer serve now binds to 127.0.0.1 by default and recommends reverse proxy for 0.0.0.0.
docs/cli.md Documents new/default serve flags and hardened defaults.
AGENTS.md Updates agent-facing docs to reflect hardened serve defaults and trust boundaries.
.gitignore Adds patterns to prevent committing secrets/credentials and local tooling artifacts.
Suppressed comments (2)

src/tracer/runtime/serve.py:106

  • /predict returns HTTP 500 for client-caused validation failures (e.g., bad embedding shape/dtype or dimension mismatch). These should be 400-series responses so callers can distinguish invalid input from server faults.
        except Exception as e:
            self._json_response(500, {"error": str(e)})

src/tracer/analysis/html_report.py:437

  • int(s["count"]) in the length-bucket table can raise if count is missing or not an integer, causing the HTML report generation to fail. Coerce defensively (as with manifest fields) rather than crashing on malformed artifacts.
            for s in length_slices:
                html += (
                    f'<tr><td>{_esc(s["slice_name"])}</td>'
                    f'<td><b style="color:#f0f6fc">{s["handled_rate"]:.1%}</b></td>'
                    f'<td>{_bar_html(s["handled_rate"])}</td>'
                    f'<td style="color:#8b949e">{int(s["count"])}</td></tr>\n'
                )

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines 138 to 139
except Exception as e:
self._json_response(500, {"error": str(e)})
Comment on lines +422 to +423
f'<td style="color:#8b949e">{int(s["count"])}</td>'
f'<td style="color:#8b949e">{_esc(ta_str)}</td></tr>\n'
Repository owner deleted a comment from tarun101 Aug 17, 2026
@batmnnn

batmnnn commented Sep 6, 2026

Copy link
Copy Markdown
Author

@adrida hey is tracer open to contribute ?

This branch has not been deployed

No deployments
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.

2 participants