fix(hooks): verify tmux socket exists before trusting TMUX env var#329
fix(hooks): verify tmux socket exists before trusting TMUX env var#329xiaolai wants to merge 1 commit intorohitg00:mainfrom
Conversation
block-dev-server.js allowed dev server commands when process.env.TMUX was set. However, TMUX is just an environment variable — any parent process can set it without actually running inside tmux, which would bypass the guard. TMUX is set by tmux to the socket path (format: socket,pid,session). We now verify the socket file exists with fs.existsSync() on the first comma-delimited segment, confirming a real tmux server is running rather than trusting the variable alone. Co-Authored-By: Claude Code <noreply@anthropic.com>
📝 WalkthroughWalkthroughThe dev server blocking script now imports Node's Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@hooks/scripts/block-dev-server.js`:
- Around line 29-31: The inTmux and inScreen checks are vulnerable to env
spoofing; update the TMUX validation to parse tmuxVal.split(",")[0] into
socketPath and then verify fs.existsSync(socketPath) AND that
fs.statSync(socketPath).isSocket() (catch and treat stat errors as non-tmux),
and disable the inScreen detection by setting inScreen = false (remove relying
on process.env.STY) until a proper Unix-socket validation is implemented; refer
to the tmuxVal, inTmux and inScreen symbols when making these changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2d4fcb14-3e4c-41a7-a0ec-c51e625de006
📒 Files selected for processing (1)
hooks/scripts/block-dev-server.js
| const tmuxVal = process.env.TMUX || ""; | ||
| const inTmux = tmuxVal !== "" && fs.existsSync(tmuxVal.split(",")[0]); | ||
| const inScreen = !!process.env.STY; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect existing project docs/tests for expected tmux/screen support and socket assumptions.
rg -n -C3 '\b(TMUX|STY|screen|tmux|block-dev-server)\b'Repository: rohitg00/awesome-claude-code-toolkit
Length of output: 32265
🏁 Script executed:
# Look for tests or documentation related to block-dev-server.js
find . -type f \( -name "*.test.*" -o -name "*.spec.*" \) | xargs grep -l "block-dev-server\|TMUX\|STY" 2>/dev/null || echo "No test files found"
# Check for any documentation about multiplexer expectations
find . -type f \( -name "*.md" -o -name "*.txt" -o -name "*.json" \) | xargs grep -l "screen.*socket\|tmux.*socket\|STY" 2>/dev/null || echo "No socket docs found"Repository: rohitg00/awesome-claude-code-toolkit
Length of output: 120
🏁 Script executed:
# Check for references to STY usage anywhere in the codebase
rg -i '\bSTY\b' --type-list | head -20
rg -i '\bSTY\b' 2>/dev/null || echo "STY not referenced elsewhere"
# Check if there are comments explaining the STY/screen usage in the file
sed -n '20,45p' hooks/scripts/block-dev-server.jsRepository: rohitg00/awesome-claude-code-toolkit
Length of output: 1263
🏁 Script executed:
# Look for socket validation patterns in similar Node.js files
rg 'isSocket|statSync' --type-list | head -5
rg 'isSocket|statSync' 2>/dev/null || echo "No socket validation patterns found"Repository: rohitg00/awesome-claude-code-toolkit
Length of output: 560
🏁 Script executed:
# Check the full context of block-dev-server.js to understand purpose and any comments
cat -n hooks/scripts/block-dev-server.jsRepository: rohitg00/awesome-claude-code-toolkit
Length of output: 1453
🏁 Script executed:
# Search for any usage of tmux or screen in shell scripts or config
rg 'tmux|screen' hooks/ --type-list 2>/dev/null || rg 'tmux|screen' hooks/ 2>/dev/null | head -20Repository: rohitg00/awesome-claude-code-toolkit
Length of output: 691
🏁 Script executed:
# Check if there are any tests at all for hooks
find . -path "*/hooks/*test*" -o -path "*/test*hooks*" 2>/dev/null | head -10
ls -la hooks/ 2>/dev/nullRepository: rohitg00/awesome-claude-code-toolkit
Length of output: 340
Harden beyond path existence to prevent environment variable spoofing.
Lines 30–31 are vulnerable:
TMUX=/tmp,1,1passesfs.existsSync()even though/tmpis not a tmux socket; any existing path bypasses the check.STY=fakebypasses the guard entirely with zero validation.
To close both spoofing vectors, validate that paths are actually Unix sockets:
Recommended fix
const tmuxVal = process.env.TMUX || "";
-const inTmux = tmuxVal !== "" && fs.existsSync(tmuxVal.split(",")[0]);
-const inScreen = !!process.env.STY;
+const isSocket = (socketPath) => {
+ if (!socketPath) return false;
+ try {
+ return fs.statSync(socketPath).isSocket();
+ } catch {
+ return false;
+ }
+};
+
+const inTmux = isSocket(tmuxVal.split(",")[0]);
+const inScreen = false; // screen socket validation not yet implementedFor tmux, the socket validation above is sufficient. For screen, no socket path assumptions are documented in the project, so inScreen is disabled until proper socket validation is added.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const tmuxVal = process.env.TMUX || ""; | |
| const inTmux = tmuxVal !== "" && fs.existsSync(tmuxVal.split(",")[0]); | |
| const inScreen = !!process.env.STY; | |
| const tmuxVal = process.env.TMUX || ""; | |
| const isSocket = (socketPath) => { | |
| if (!socketPath) return false; | |
| try { | |
| return fs.statSync(socketPath).isSocket(); | |
| } catch { | |
| return false; | |
| } | |
| }; | |
| const inTmux = isSocket(tmuxVal.split(",")[0]); | |
| const inScreen = false; // screen socket validation not yet implemented |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/scripts/block-dev-server.js` around lines 29 - 31, The inTmux and
inScreen checks are vulnerable to env spoofing; update the TMUX validation to
parse tmuxVal.split(",")[0] into socketPath and then verify
fs.existsSync(socketPath) AND that fs.statSync(socketPath).isSocket() (catch and
treat stat errors as non-tmux), and disable the inScreen detection by setting
inScreen = false (remove relying on process.env.STY) until a proper Unix-socket
validation is implemented; refer to the tmuxVal, inTmux and inScreen symbols
when making these changes.
1 participant
Security Finding (Low)
hooks/scripts/block-dev-server.jsguards dev-server commands (e.g.npm run dev,vite) by checking whetherprocess.env.TMUXorprocess.env.STYis set. The intent is to require users to be inside a terminal multiplexer so the dev server won't become an orphaned process.The issue:
TMUXis just an environment variable. Any process — or a user typingexport TMUX=fake— can set it without actually being inside tmux, bypassing the guard entirely.Fix
TMUX, when set by tmux itself, contains the socket path followed by server PID and session ID, comma-separated (e.g./tmp/tmux-1000/default,23456,0). The socket is a real file on disk. We verify it exists:If the env var is set but no socket file exists at that path,
inTmuxisfalse— the guard holds. Legitimate tmux sessions are unaffected.Files changed:
hooks/scripts/block-dev-server.js(+3 lines, -1 line)Summary by CodeRabbit
Release Notes