Skip to content

fix: critical security vulnerabilities in IPC handlers and renderer#27

Open
navedr wants to merge 2 commits into
doctly:mainfrom
navedr:security/fix-critical-vulnerabilities
Open

fix: critical security vulnerabilities in IPC handlers and renderer#27
navedr wants to merge 2 commits into
doctly:mainfrom
navedr:security/fix-critical-vulnerabilities

Conversation

@navedr

@navedr navedr commented Apr 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Security audit identified several critical and high-severity vulnerabilities. This PR fixes the top 6:

  • Arbitrary file read/write via IPCread-file-for-panel, save-file-for-panel, watch-file, read-memory, and save-memory accepted any file path from the renderer with no validation. Added path allowlisting (~/.claude/ + active session project dirs).
  • XSS via unsanitized markdownmarked.parse() output was assigned to innerHTML without sanitization. Added DOMPurify to sanitize all markdown rendering.
  • Command injection via session optionspermissionMode, worktreeName, and addDirs were interpolated into shell command strings without validation. Added shell metacharacter rejection.
  • No Content Security Policy — Added CSP header (default-src 'self') via session.defaultSession.webRequest.onHeadersReceived.

What's NOT in this PR

  • preLaunchCmd is intentionally a shell command (by design), so it's left as-is. A future refactor could move PTY spawning to array-based args to eliminate the entire shell interpretation surface.
  • macOS entitlements (allow-dyld-environment-variables, etc.) are required by native modules and can't be tightened without breaking node-pty/better-sqlite3.

Files changed

File Change
main.js Path validation helper, shell arg validation, CSP header, secured 5 IPC handlers
package.json Added dompurify dependency
public/index.html Load DOMPurify script
public/viewer-toolbar.js Sanitize marked output
public/viewer-panel.js Sanitize marked output

Test plan

  • npm test passes
  • npm run bundle:codemirror builds successfully
  • Verify markdown preview still renders correctly in plans/memory viewers
  • Verify file panel opens/saves files within project directories
  • Verify file panel rejects paths outside project directories
  • Verify new session launch with worktree name / additional dirs still works
  • Verify CSP doesn't break any existing functionality (xterm, codemirror, etc.)

🤖 Generated with Claude Code

- add DOMPurify to sanitize marked markdown output (XSS via malicious .md files)
- add Content Security Policy header (default-src 'self')
- add path allowlisting to read-file-for-panel, save-file-for-panel, watch-file,
  read-memory, and save-memory IPC handlers (arbitrary file read/write)
- add shell metacharacter validation for permissionMode, worktreeName, and addDirs
  fields passed to PTY spawn (command injection)

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@abasiri

abasiri commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

Great stuff. Thank you

@abasiri

abasiri commented Apr 10, 2026

Copy link
Copy Markdown
Contributor

@navedr the OSC8 hyperlinks in the terminal can point to any file. So in Claude when you are modifying a file, it becomes clickable and clicking it opens it in a side panel. So this restriction would break that:

Read-file-for-panel, save-file-for-panel, watch-file.

Address review feedback from @abasiri: the file panel intentionally opens
arbitrary files via OSC8 hyperlinks from terminal output, so a strict
allowlist breaks that functionality.

Changed approach:
- file panel IPC (read/save/watch): denylist of sensitive paths (.ssh,
  .gnupg, .aws/credentials, .env, .netrc, .docker/config, .kube/config)
- memory IPC (read/save): strict allowlist (unchanged, ~/.claude/ + project dirs)

The primary XSS→file-access chain is mitigated by CSP + DOMPurify;
the sensitive path denylist is defense-in-depth.

Co-Authored-By: Claude Opus 4.6 (1M context) <[email protected]>
@navedr

navedr commented Apr 10, 2026

Copy link
Copy Markdown
Contributor Author

Good point — updated in dc2e2a4.

Changed the approach for file panel IPC (read-file-for-panel, save-file-for-panel, watch-file):

  • Before: strict allowlist (only ~/.claude/ + active project dirs) — would break OSC8 hyperlinks
  • After: denylist of known-sensitive paths (.ssh/, .gnupg/, .aws/credentials, .env, .netrc, .docker/config.json, .kube/config)

Memory IPC (read-memory, save-memory) keeps the strict allowlist since those only need ~/.claude/ and project dirs.

The primary defense against the XSS→file-access chain is CSP + DOMPurify — the sensitive path denylist is defense-in-depth.

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.

3 participants