fix(sandbox-ssh-fix): use PATH-based nc wrapper instead of replacing GIT_SSH_COMMAND#60
Conversation
|
Warning Review limit reached
More reviews will be available in 44 minutes and 42 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe sandbox SSH plugin now prepends its ChangesSandbox SSH proxy flow
Sequence Diagram(s)sequenceDiagram
participant Hook as fix-git-ssh.sh
participant Env as CLAUDE_ENV_FILE
participant SSH as ssh
participant Wrapper as bin/nc
participant Ncat as ncat
participant RealNc as /usr/bin/nc
Hook->>Env: append PATH=$CLAUDE_PLUGIN_ROOT/bin
SSH->>Wrapper: resolve nc from PATH
alt ALL_PROXY has credentials and ncat exists
Wrapper->>Ncat: run with SOCKS5 proxy auth from ALL_PROXY
else fallback
Wrapper->>RealNc: exec original arguments
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@sandbox-ssh-fix/bin/nc`:
- Around line 2-7: The current nc wrapper in the bin/nc script is too broad: it
reroutes every ALL_PROXY request with credentials through ncat and drops most of
the original argv, which breaks the required /usr/bin/nc fallback. Update the
wrapper logic so only SOCKS5 proxy invocations are intercepted and forwarded to
ncat, preserving the expected arguments for that case, while all other nc calls
continue to exec /usr/bin/nc unchanged.
In `@sandbox-ssh-fix/CLAUDE.md`:
- Around line 5-24: The README walkthrough is out of sync with the current
sandbox-SSH flow and still mentions the removed GIT_SSH_COMMAND override and old
activation checks. Update README.md to match the behavior described in CLAUDE.md
by documenting the SessionStart PATH prepending via CLAUDE_ENV_FILE, the
sandbox-provided GIT_SSH_COMMAND invoking nc -X 5, the bin/nc wrapper
intercepting SOCKS5 calls, parsing ALL_PROXY credentials, delegating to ncat
with proxy auth, and the 127.0.0.1 localhost workaround.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 5f0b84ff-3845-44fb-b855-af2d3385f893
📒 Files selected for processing (4)
sandbox-ssh-fix/.claude-plugin/plugin.jsonsandbox-ssh-fix/CLAUDE.mdsandbox-ssh-fix/bin/ncsandbox-ssh-fix/scripts/fix-git-ssh.sh
There was a problem hiding this comment.
Pull request overview
This PR reworks the sandbox-ssh-fix plugin, which works around broken git-over-SSH in the Claude Code sandbox on macOS (anthropics/claude-code#70684). The previous approach rewrote GIT_SSH_COMMAND from the SessionStart hook, but the PR explains that sandbox env vars (SANDBOX_RUNTIME, GIT_SSH_COMMAND, ALL_PROXY) aren't available to the hook, so that approach couldn't work. The new approach ships an nc wrapper and only manipulates PATH at startup, deferring the actual proxy fix to runtime when ALL_PROXY is available.
Changes:
- Adds a
bin/ncwrapper that delegates SOCKS5 proxy calls toncatwith credentials parsed fromALL_PROXY, falling back to/usr/bin/nc. - Simplifies the SessionStart hook to prepend the plugin's
bin/toPATHviaCLAUDE_ENV_FILE, removing the priorSANDBOX_RUNTIME/GIT_SSH_COMMAND/ALL_PROXYguards. - Updates
CLAUDE.mdand bumpsplugin.jsonto1.0.1with a revised description.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
sandbox-ssh-fix/bin/nc |
New PATH wrapper that rewrites nc SOCKS5 calls to ncat with auth; falls back to /usr/bin/nc. |
sandbox-ssh-fix/scripts/fix-git-ssh.sh |
Hook now only prepends bin/ to PATH instead of rewriting GIT_SSH_COMMAND. |
sandbox-ssh-fix/CLAUDE.md |
Rewrites docs to describe the wrapper-based approach. |
sandbox-ssh-fix/.claude-plugin/plugin.json |
Version bump to 1.0.1 and updated description. |
Note: sandbox-ssh-fix/README.md is not part of this PR but still documents the old GIT_SSH_COMMAND-replacement approach and a direct-SSH fallback that no longer exists; it is now inconsistent with the implementation and should be updated in a follow-up.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…GIT_SSH_COMMAND Sandbox env vars (SANDBOX_RUNTIME, GIT_SSH_COMMAND, ALL_PROXY) are not available to SessionStart hooks. Instead of trying to replace GIT_SSH_COMMAND, ship an nc wrapper that intercepts SOCKS5 proxy calls at runtime and delegates to ncat with auth credentials from ALL_PROXY. The SessionStart hook now just prepends the plugin's bin/ to PATH via CLAUDE_ENV_FILE. When SSH runs the ProxyCommand, the wrapper is found first and handles the auth delegation transparently. Assisted-by: Claude:claude-opus-4-6
cblecker
force-pushed
the
fix/sandbox-ssh-fix-nc-wrapper
branch
from
1b59ba0 to
9ee2281
Compare
Summary
SANDBOX_RUNTIME,GIT_SSH_COMMAND,ALL_PROXY) are not available to SessionStart hooks, so the original approach of replacingGIT_SSH_COMMANDat startup couldn't workncwrapper (bin/nc) that intercepts SOCKS5 proxy calls at runtime and delegates toncatwith auth credentials parsed fromALL_PROXYbin/toPATHviaCLAUDE_ENV_FILETest plan
claude --plugin-dir ./sandbox-ssh-fixshowssandbox-ssh-fix: added nc wrapper to PATHon startupwhich ncresolves to the plugin's wrapper, not/usr/bin/ncgit ls-remote --heads git@github.com:cblecker/claude-plugins.gitsucceeds in sandboxSummary by CodeRabbit
New Features
nc.Documentation
Chores
1.0.1.