Security hardening: XSS escaping, loopback bind, WS origin check, path-traversal guard - #18
Open
bdouglas84 wants to merge 5 commits into
Open
Security hardening: XSS escaping, loopback bind, WS origin check, path-traversal guard#18bdouglas84 wants to merge 5 commits into
bdouglas84 wants to merge 5 commits into
Conversation
Message text blocks were run through the markdown regexes and written to innerHTML without HTML-escaping, so an <img onerror>/<svg onload> in any transcript (tool output, pasted content, or an imported session) executed in the browser. tool_use name/summary were interpolated raw as well. - new applyInlineMarkdown(): escapeHtml FIRST, then the markdown transforms - escape toolName / toolSummary in formatToolCall - escapeHtml now encodes quotes too, so it is safe in attribute context (folder-browser title=/data-path=) - add test/unit/text-content-xss.test.js Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSdbwKWK9zWe2VRkghmVGs
The native server called app.listen(port) with no host, so Node bound 0.0.0.0 and exposed the unauthenticated API (the full conversation history) to every host on the LAN. The README's "localhost-only" claim only held for the Docker mapping, not the native run. - default bind 127.0.0.1; opt into a wider bind via options.host / CHATS_HOST - set CHATS_HOST=0.0.0.0 in both compose files: Docker forwards published ports to the container's eth0, not loopback, so an in-container loopback bind would be unreachable. The host-side mapping stays 127.0.0.1-only. - loud warning when bound non-loopback, and on the Cloudflare tunnel path (the tunnel publishes the unauthenticated app to the public internet) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSdbwKWK9zWe2VRkghmVGs
WebSockets are not covered by the same-origin policy, so any website the user visited could open ws://localhost:9876/ws and receive pushed conversation activity/state. Add a verifyClient gate that requires the handshake Origin's host to equal the request Host header (same-origin, which also holds when served via the tunnel). Requests with no Origin (CLI clients, tests) are allowed since they are not a CSWSH vector. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSdbwKWK9zWe2VRkghmVGs
installSession() joined an untrusted conversation.id straight into the write path, so a malicious shared/imported session could write its JSONL outside ~/.claude/projects (e.g. conversation.id = "../../../../.bashrc"). The download side is otherwise well hardened; this was the one unchecked value that survived it. - require conversation.id to match [A-Za-z0-9._-] and contain no ".." - assert the resolved write path stays under projectDir (defense in depth, mirroring the MCP server's isUnderRoot check) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QSdbwKWK9zWe2VRkghmVGs
Search snippets and the claude-chat://conversation/{id} resource return
raw past-conversation text, including tool output (web pages, files,
command output) that can contain text crafted to look like instructions.
When an agent queries history it enters that agent's context. Document
treating it as data to reason about, not instructions to follow -- a
second-order prompt-injection vector the server cannot strip for you.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01QSdbwKWK9zWe2VRkghmVGs
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
I was setting this tool up for my own use and, while poking at it, ran the code through Claude to sanity-check it. Claude flagged a handful of things that looked like they could be security issues. I don't have the background to fully vet these myself, so please treat this as "here is what an AI review turned up" rather than an expert audit. I already emailed you a heads-up; this PR is just the fixes it suggested, in case any of them save you time. No worries at all if you'd rather adapt or redo them your own way.
All changes are small and self-contained. The existing test suite still passes (339/339) and I confirmed the app still boots and renders chats after the changes.
What's in here
CHATS_HOSTenv var defaulting to127.0.0.1, with a visible warning if it's ever bound to a non-loopback address or exposed via the tunnel. The Docker compose files setCHATS_HOST=0.0.0.0inside the container so port mapping still works, while the host side maps to127.0.0.1only.verifyClientgate that compares Origin host to the request Host (connections with no Origin are still allowed, so non-browser clients keep working)...check, and a resolved-path containment assert.Notes
test/unit/text-content-xss.test.jscovers the escaping (payloads neutralized, normal markdown preserved).CHATS_HOSTexplicitly. The Docker path is handled so nothing breaks there.