Skip to content

security: validate tool_use_id path component; drop --reload in network mode#50

Open
nithiink wants to merge 1 commit into
mainfrom
security-fixes-2026-06-22
Open

security: validate tool_use_id path component; drop --reload in network mode#50
nithiink wants to merge 1 commit into
mainfrom
security-fixes-2026-06-22

Conversation

@nithiink

Copy link
Copy Markdown
Owner

Fixes the auto-fixable findings from the daily security review (#49).

Fixes

L1 — tool_use_id validated before use as a filesystem path component

tool_use_id is interpolated into decisions/<id>.json and reaches open(), os.remove(), and os.replace(), but — unlike session_id/handle (validated via _SESSION_ID_RE) — it had no validation, so a traversal value could escape the decisions/ dir.

  • backend/tmux_hooks/_common.py — add safe_id() (same charset/length bound as validate_session_id) and use it in decision_path().
  • backend/tmux_runner.py — apply the matching guard on the runner's write side (os.replace path).

L2 — --reload removed from the network-exposed run script

  • backend/run-network.sh — the 0.0.0.0-bound mode no longer enables uvicorn --reload; the autoreloader's file-watcher/child-process supervisor is needless attack surface on a LAN-reachable service (and turns any writable-code scenario into reload-triggered RCE). The localhost run.sh is unchanged.

Left for manual attention (NOT in this PR)

M1 (Medium) — DNS-rebinding can reach the command-executing API in the default tokenless mode

There is no Host-header validation anywhere, so a DNS-rebinding attack (and any co-resident localhost web app) can satisfy both the proxy's same-origin check and the backend's loopback trust. The fix — a Host allowlist via Starlette TrustedHostMiddleware and/or a check in blockCrossSite() — needs design care so legitimate LAN access in network mode (192.168.x.x / hostname) keeps working (suggest a VC_ALLOWED_HOSTS env knob). Deliberately left out so it can be reviewed deliberately.

Verification

  • python -m py_compile passes on both edited Python files.
  • bash -n passes on run-network.sh.
  • (pytest is not installed in the review environment, so the suite wasn't run here.)

Fixes #49 (L1, L2). M1 tracked in #49 for manual follow-up.


Generated by Claude Code

…network mode

Daily security review fixes (see issue #49):

- L1: tool_use_id is interpolated into decisions/<id>.json and reaches
  open()/os.remove()/os.replace(), but — unlike session_id/handle — it was
  never validated. Add safe_id() (same charset/length as _SESSION_ID_RE) in the
  hook helper and a matching guard on the runner's write side so a malformed id
  can't escape the decisions/ dir.

- L2: run-network.sh (0.0.0.0 bind) no longer enables uvicorn --reload; the
  autoreloader's file-watcher/child-process supervisor is needless attack
  surface on a LAN-reachable service.

The Medium DNS-rebinding finding (M1) is left for manual attention — a Host
allowlist (TrustedHostMiddleware) needs care to keep legitimate LAN access
working in network mode.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0186kFkxiLCUPEb8B7uSxNWs
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.

Daily security review — 2026-06-22

2 participants