fix(web): server-side sessionId and filePath validation - #216
Merged
Merged
Conversation
The web server process is spawned without a HOST env var, so adapter-node defaults to binding 0.0.0.0 (all interfaces) instead of localhost. Explicitly setting HOST=127.0.0.1 removes the LAN/external network exposure of the unauthenticated local API. Signed-off-by: DrumRobot <drumrobot43@gmail.com>
filePath was interpolated directly into a shell command string via execAsync, letting a crafted path inject and execute arbitrary shell commands. Switch to execFile with argv-separated arguments so the input is never parsed by a shell, and split a multi-token CLAUDE_SESSIONS_EDITOR into its binary plus leading args before appending filePath. Adds regression coverage: normal path, an injection-payload path (asserts it reaches execFile as a single literal argument), and the multi-token editor split. Signed-off-by: DrumRobot <drumrobot43@gmail.com>
Reject any request whose Host header doesn't name this loopback server (localhost / 127.0.0.0/8 / [::1], case/trailing-dot normalized, IPv6 bracket-aware). GET requests carry no Origin header, so Host is the only anchor available against DNS rebinding or same-LAN access. Live-verified: evil.com and a LAN-victim-style Host both return 403; 127.0.0.1:<port> and localhost:<port> pass through to 200. Signed-off-by: DrumRobot <drumrobot43@gmail.com>
…b server (P1-2)
Layer 3 hardening: hooks.server.ts now requires a valid session cookie
or a one-time CLAUDE_SESSIONS_AUTH_TOKEN exchange for every route
except /api/version (health-check). Fixes the 3 flaws the plan doc's
Q2 review found in the original sketch:
- token wasn't actually single-use -> synchronous check-and-set before
any await, so concurrent requests with the correct token can't both pass
- cookie value was a guessable constant ("valid") -> random per-process
secret, timing-safe compared
- token leaked indefinitely via the URL -> stripped via 302 redirect
immediately after a successful exchange
extension.ts generates the token per spawn (env, never argv) and
appends it to every URL it opens (openSession, openWebUI); auth is a
no-op when CLAUDE_SESSIONS_AUTH_TOKEN is unset (standalone `npx
@claude-sessions/web` keeps working without it, matching how HOST is
also extension-only). restartWebServer now returns the handle so
tests/tooling can obtain the token without importing extension
internals.
Verified against a real VS Code extension host (not just curl): the
existing webview.test.ts HTTP-200 checks now perform the token->cookie
exchange before asserting page content, and both they and the
click-through "Session click opens webview" flow pass end-to-end.
Signed-off-by: DrumRobot <drumrobot43@gmail.com>
…rdening (P2-1) /api/version now reports a hardened: true capability flag — older @claude-sessions/web builds without hooks.server.ts won't have this field at all, so its absence is the mismatch signal. Using a capability flag rather than a semver threshold avoids hardcoding a "minimum safe version" we can't know yet (this fix hasn't been released). extension.ts checks the flag on every health check after it has sent CLAUDE_SESSIONS_AUTH_TOKEN to the server (a fresh spawn, or an already-running server from earlier in the same activation) and warns once per activation if a pinned webServerPath/packageTag points at an unhardened build — graceful degrade (server keeps working) + surfaced to the user, per the plan doc's P2-1 guidance. Live-verified /api/version returns hardened: true; full vscode-extension integration suite (67 tests) and web suite (125 tests) still pass. Signed-off-by: DrumRobot <drumrobot43@gmail.com>
…+ Host allow-list (P2-2) Two of the plan doc's P2-2 checklist items didn't have an automated equivalent yet (they were only manually curl/lsof-verified during the P0-1/P1-1 sessions): - loopback-only bind: connect via this machine's own LAN-facing IPv4 address (os.networkInterfaces()) instead of localhost — must fail. Cross-platform, unlike parsing lsof/netstat output. Skips gracefully when no LAN interface exists. - Host allow-list: a real request against the running server with an explicit Host: evil.com header must get 403 (verified Node's http.get honors an explicit Host header override before writing this). The remaining P2-2 items (token-missing 401, token-replay 401, open-file injection blocked, session-loading regression) were already covered by hooks.server.test.ts (P1-1/P1-2 commits) and the existing open-file/server.test.ts — this rounds out the checklist with the two that specifically need a live server rather than a unit test. Signed-off-by: DrumRobot <drumrobot43@gmail.com>
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
Cache: Disabled due to Reviews > Disable Cache setting Knowledge base: Disabled due to 📝 WalkthroughWalkthroughThe web server now uses loopback binding, Host validation, one-time authentication tokens, and session cookies. The VS Code extension passes tokens through server handles and authenticated URLs. The open-file endpoint now uses shell-safe editor execution. ChangesWeb server hardening
Safe file opening
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant VSCodeExtension
participant WebServer
participant hooksServer as hooks.server
participant Browser
VSCodeExtension->>WebServer: Start on 127.0.0.1 with auth token
Browser->>WebServer: Open URL with auth token
WebServer->>hooksServer: Process request
hooksServer->>hooksServer: Validate Host and consume token
hooksServer-->>Browser: Set session cookie and redirect
Browser->>WebServer: Request protected page with cookie
WebServer-->>Browser: Return authenticated page
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Member
Author
|
🎉 This PR is included in version 0.5.6-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Member
Author
|
🎉 This PR is included in version 0.4.13-beta.1 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
4 tasks
Member
Author
|
🎉 This PR is included in version 0.5.6 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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
sessionIdin/api/session/resumebefore it reaches the shell-interpolated resume pathexecAsyncwith argv-safeexecFilein/api/open-fileTest plan
🤖 Generated with Claude Code
Summary by CodeRabbit
Security Enhancements
Improvements