fix(ssh): wait for reverse-forward listener before running setup-gpg - #834
Merged
Conversation
setupGPGAgent started the gpg-agent socket's reverse forward in a background goroutine and immediately ran the remote setup-gpg command, which symlinks the container's gnupg socket to that forward's path. Nothing synchronized the two: setup-gpg could finish symlinking before the SSH global request binding the remote listener had actually been acknowledged, leaving the symlink pointing at a path nothing was listening on yet. Split ReversePortForward into ReverseListen (blocking bind) and RunReverseForward (the accept loop), and add startReverseForwardsAndWait, which binds every configured forward before returning so setup-gpg only runs once the listener is confirmed live. Note: this closes one real race, but is not sufficient on its own to fix "gpg: no gpg-agent running in this session" end to end — the container's ~/.gnupg/private-keys-v1.d never receives the secret-key shadow stubs GnuPG needs to recognize an agent-forwarded key, which is a separate, larger gap in the forwarding setup.
✅ Deploy Preview for devsydev canceled.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change separates reverse listener creation from forwarding, binds workspace listeners before asynchronous startup, and validates GPG-agent forwarding synchronously. ChangesReverse forwarding lifecycle
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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 |
✅ Deploy Preview for images-devsy-sh canceled.
|
skevetter
marked this pull request as ready for review
July 31, 2026 04:37
skevetter
marked this pull request as draft
July 31, 2026 04:38
skevetter
marked this pull request as ready for review
July 31, 2026 05:20
![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 current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@cmd/workspace/ssh.go`:
- Around line 516-537: Update bindReverseForwards to close all previously
created listeners in bound before returning from either a port.ParsePortSpec or
devssh.ReverseListen failure. Reuse the listener references stored in bound,
preserve the existing wrapped errors, and ensure cleanup occurs only for
partial-bind failure paths.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 621024f6-2426-436c-bf81-55c17ca5ba96
📒 Files selected for processing (2)
cmd/workspace/ssh.gopkg/ssh/forward.go
skevetter
marked this pull request as draft
July 31, 2026 05:26
…s failure If a later port mapping failed to parse or bind, earlier listeners in bound stayed open on the remote until the SSH transport closed, leaking the bind and breaking a retry against the same address.
Contributor
Author
|
Fixed in f794851 — |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
setupGPGAgent(cmd/workspace/ssh.go) started the gpg-agent socket's reverse forward in a background goroutine and immediately ran the remotesetup-gpgcommand, which ends by symlinking the container's gnupg socket to that forward's path (SetupRemoteSocketLink). Nothing synchronized the two —setup-gpgcould finish symlinking before the SSH global request that binds the remote listener had actually been acknowledged (client.Listen(...)inpkg/ssh/forward.go), leaving the symlink pointing at a path with no live listener yet.ReversePortForwardintoReverseListen(the blocking bind) andRunReverseForward(the accept loop), mirroring the existingPortForwardWithListener/ForwardOptspattern already used for the forward-port TOCTOU fix.startReverseForwardsAndWait, which binds every configured reverse forward before returning, then runs each one's accept loop in the background.setupGPGAgentnow calls this and waits for it to return before runningsetup-gpg, instead of firing the forward off in an unsynchronized goroutine.Caveat
Reproduced the reported
gpg: no gpg-agent running in this sessionagainst a realws3-sshworkspace before and after this change — this fix closes a real race (confirmed via debug logs: the forwarded socket now reliably has a live listener by the timesetup-gpgruns, and a real connection does land on it), but it is not sufficient on its own to resolve the reported symptom end-to-end. Further investigation found the container's~/.gnupg/private-keys-v1.dnever receives the secret-key shadow stub files GnuPG needs to recognize an agent-forwarded key as available (ImportGpgKey/configureGPGAgentonly ever import the public key + owner-trust). That's a separate, larger gap tracked for follow-up — this PR only fixes the listener race.Summary by CodeRabbit