Skip to content

fix: [AI-194] pre-release security and resource cleanup fixes for tracing#197

Merged
anandgupta42 merged 4 commits intomainfrom
fix/pre-release-security-fixes
Mar 16, 2026
Merged

fix: [AI-194] pre-release security and resource cleanup fixes for tracing#197
anandgupta42 merged 4 commits intomainfrom
fix/pre-release-security-fixes

Conversation

@anandgupta42
Copy link
Contributor

What does this PR do?

Three targeted fixes found during pre-release review (v0.4.0 → v0.5.0):

  1. XSS fix in trace viewer (viewer.ts:200): Escape t.summary.status with the existing e() HTML escape function — it was the only unescaped user-controllable field in the header tags block
  2. Graceful shutdown for trace viewer server (trace.ts): Add SIGINT/SIGTERM handlers to explicitly stop the Bun server on Ctrl+C instead of relying on process termination
  3. Diagnostic logging for snapshot failures (tracing.ts:631): Log failed trace snapshot writes with console.debug instead of silently swallowing errors

Type of change

  • Bug fix (non-breaking change which fixes an issue)

Issue for this PR

Closes #194

How did you verify your code works?

  • All 380 tracing tests pass (13 files, 1248 assertions)
  • All 25 engine bridge tests pass
  • All 45 feedback submit tests pass
  • Typecheck passes (4/4 packages)
  • 6-model code review (Claude, GPT 5.2 Codex, Gemini 3.1 Pro, Kimi K2.5, MiniMax M2.5, GLM-5): unanimous APPROVE

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • New and existing unit tests pass locally with my changes
  • I have verified the changes match the diff (not just memory)

…cing

- Escape `t.summary.status` with `e()` in trace viewer HTML to prevent XSS
- Add SIGINT/SIGTERM handlers to gracefully stop trace viewer server on Ctrl+C
- Log snapshot write failures with `console.debug` instead of silently swallowing

Closes #194

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
anandgupta42 and others added 3 commits March 16, 2026 16:00
Systematically escape all user-controllable fields in `viewer.ts`:

- Escape `span.kind` and `span.status` in detail panel, waterfall, tree, and log views
- Escape `span.spanId` in `data-sid` attributes
- Coerce all numeric fields with `Number()` to prevent string injection via `.toLocaleString()`
- Add single-quote escaping (`&#x27;`) to the `e()` function for defense-in-depth

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
`Bun.Server.stop()` returns a `Promise<void>` — calling it without
`await` exits the process before in-flight requests are drained.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Prevents unhandled promise rejection if the server fails to stop
cleanly on SIGINT/SIGTERM.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@anandgupta42 anandgupta42 merged commit 4880171 into main Mar 16, 2026
7 checks passed
anandgupta42 added a commit that referenced this pull request Mar 17, 2026
…cing (#197)

* fix: [AI-194] pre-release security and resource cleanup fixes for tracing

- Escape `t.summary.status` with `e()` in trace viewer HTML to prevent XSS
- Add SIGINT/SIGTERM handlers to gracefully stop trace viewer server on Ctrl+C
- Log snapshot write failures with `console.debug` instead of silently swallowing

Closes #194

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: comprehensive XSS hardening for trace viewer HTML

Systematically escape all user-controllable fields in `viewer.ts`:

- Escape `span.kind` and `span.status` in detail panel, waterfall, tree, and log views
- Escape `span.spanId` in `data-sid` attributes
- Coerce all numeric fields with `Number()` to prevent string injection via `.toLocaleString()`
- Add single-quote escaping (`&#x27;`) to the `e()` function for defense-in-depth

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: await `server.stop()` for graceful shutdown in trace viewer

`Bun.Server.stop()` returns a `Promise<void>` — calling it without
`await` exits the process before in-flight requests are drained.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

* fix: catch `server.stop()` rejection in shutdown handler

Prevents unhandled promise rejection if the server fails to stop
cleanly on SIGINT/SIGTERM.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@anandgupta42 anandgupta42 deleted the fix/pre-release-security-fixes branch March 17, 2026 00:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fix: pre-release security and resource cleanup fixes for tracing

1 participant