fix(daemon): add token-based authentication for local connections#397
Open
sorlen008 wants to merge 3 commits intojackwener:mainfrom
Open
fix(daemon): add token-based authentication for local connections#397sorlen008 wants to merge 3 commits intojackwener:mainfrom
sorlen008 wants to merge 3 commits intojackwener:mainfrom
Conversation
The existing CSRF protections (Origin check, X-OpenCLI header) block browser-based attacks but don't protect against local process impersonation. Any process on the machine that knows the port and static header value can connect to the daemon and access all browser sessions. This adds a random shared secret (stored at ~/.opencli/token) that the daemon requires on all HTTP and WebSocket connections: - Token is generated on first daemon start (32 random bytes, hex-encoded) - File is created with mode 0o600 (owner-only read/write) - HTTP requests must include x-opencli-token header - WebSocket connections pass the token via query parameter or Sec-WebSocket-Protocol header - CLI and discover code updated to read and send the token This closes the local privilege escalation gap identified in jackwener#395. Closes jackwener#395
…ion, Windows ACLs Addresses review feedback on the token authentication implementation: - Use crypto.timingSafeEqual() instead of === for token comparison, preventing timing side-channel attacks on the shared secret - Use O_EXCL flag for atomic token file creation, preventing race conditions when multiple daemon processes start simultaneously - Add icacls enforcement on Windows where Unix mode 0o600 is ignored, restricting token file to current user only - Validate token format with regex (exactly 64 hex chars) to detect corrupted files instead of accepting any 32+ char string - Add rotateToken() for token invalidation if the secret leaks - Add verifyToken() export for constant-time comparison - Export authHeaders() from daemon-client and reuse in discover.ts to eliminate duplicate auth header construction - Improve error messages when token directory/file can't be created
…eate When the token file exists but contains invalid data (corrupted), the O_EXCL flag on the new file creation fails with EEXIST since the corrupt file is still on disk. Fix: unlink the corrupt file before attempting to create a new one. Found during UAT on Windows.
Contributor
Author
UAT Results (Windows 11)
All 7 tests pass after the corruption fix. Windows ACLs verified via |
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
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.
What this does
Adds a random shared secret (
~/.opencli/token) that the daemon requires on all HTTP and WebSocket connections. This closes the local privilege escalation gap where any process on the machine could connect to the daemon by just knowing the port and the staticX-OpenCLI: 1header.How it works
~/.opencli/token(mode 0600, owner-only)x-opencli-tokenheader with the correct token, or it gets a 401?token=...) orSec-WebSocket-ProtocolheaderreadToken()— no config needed from the userFiles changed
src/token.ts(new) — token generation, reading, and constantssrc/daemon.ts— token validation on HTTP + WebSocketverifyClientsrc/browser/daemon-client.ts— sends token on all HTTP requestssrc/browser/discover.ts— sends token on status checksWhat stays the same
Testing
npx tsc --noEmit— zero type errorsnpm test— 257 passed, 4 failed (same failures onmain, not introduced by this PR)Closes #395