client: Extract proxy handshakes into a new proxy_handshake crate - #61427
Merged
Conversation
cla-bot
Bot
added
the
cla-signed
zed-community-bot
Bot
added
the
staff
swannysec
approved these changes
Jul 23, 2026
swannysec
left a comment
swannysec
left a comment
Contributor
There was a problem hiding this comment.
Looks good on the security side 🔒
The CONNECT and SOCKS4/4a/5/5h client handshakes move from client/src/proxy into a new sans-IO crate. The protocol logic is a pure state machine (feed bytes in, get bytes to send out), with thin drivers behind futures-io and tokio feature flags; client uses the tokio driver. This drops the tokio-socks dependency and makes the protocol logic testable synchronously, including a proptest that the outcome is invariant under how proxy bytes are chunked across reads. Two behavior changes, both converging on spec/curl semantics: - Proxy credentials in URLs are now percent-decoded before use (RFC 3986 s3.2.1); previously they were sent encoded. - NO_PROXY is now honored for the collaboration connection, matching the exclusions reqwest already applies to HTTP traffic.
- Rename percent_decode to decode_userinfo and document that it exists because RFC 3986 s3.2.1 defines userinfo as percent-encoded. - Document that the SOCKS5 refusal messages are verbatim from RFC 1928 s6. - Expand ProxySpec's docs to explain its relationship to url::Url. - Free the Tunneled leftover buffer once it has been fully replayed.
Address two non-blocking security review findings: - Enforce the HTTP CONNECT response-head cap on complete heads too, not just partial ones, so the cap holds regardless of how large the driver's read chunks are. - Reject ASCII control bytes in Target::Domain at Handshake::new (they could forge protocol structure in the CONNECT request line and NUL-terminated SOCKS4a fields), and reject NUL in the SOCKS4 user id (the field is NUL-terminated on the wire). Enforcement lives in Handshake::new rather than ProxySpec::parse because ProxySpec's fields are public and parse can be bypassed. HTTP Basic and SOCKS5 credentials are deliberately not filtered: base64 encoding and length prefixes already neutralize control bytes there, and RFC 1929 allows arbitrary octets in SOCKS5 passwords.
eholk
force-pushed
the
proxy-handshake-crate
branch
from
July 23, 2026 22:35
55755f9 to
3140693
Compare
eholk
enabled auto-merge
July 23, 2026 22:40
mdz-axo
added a commit
to mdz-axo/zed-kask
that referenced
this pull request
Jul 25, 2026
Upstream changes (zed-industries/zed main, 27 commits): - agent: Add agent.compaction_model setting for context compaction (zed-industries#60012) - agent: Show effort selector for anthropic compatible providers (zed-industries#61579) - acp: Update agent-client-protocol SDK to 2.0.0 (zed-industries#61570) - client: Extract proxy handshakes into new proxy_handshake crate (zed-industries#61427) - collab: Fix multiworkspace location out of sync bugs (zed-industries#61598) - editor: Fix sticky header drag cancels autoscroll (zed-industries#53592) - editor: Fix crash when copying and pasting using multiple cursors (zed-industries#61545) - editor: Skip untitled buffers when saving a multi-buffer (zed-industries#61380) - gpui: Fix images not being drawn with rounded corners with ObjectFit::Cover (zed-industries#61383) - gpui: Fix deadlock in performance profiler and reenable it (zed-industries#61584) - git_ui: Prevent Git panel bindings in repository selector (zed-industries#61282) - language_model: Add explicit OpenAI conversation compaction and fix Anthropic compaction (zed-industries#61370) - markdown: Fix squashed Mermaid diagrams in markdown preview (zed-industries#61260) - Opus 5 BYOK Support (zed-industries#61596) - repl: Show add-cell controls in empty notebooks (zed-industries#61329) - search: Escape seeded buffer search query in regex mode (zed-industries#57748) - settings: Fix VS Code import appending duplicate file associations (zed-industries#61355) - settings: Split VSCode and Zed keymap files (zed-industries#61532) - Treat blank spawn_agent session IDs as absent (zed-industries#60893) - worktree: Reload git state when a watcher rescan covers a repository (zed-industries#61541) - Plus 7 more minor fixes. Merge fixes: - crates/agent/src/thread.rs: replay_tool_call used 'message_ix' (undefined) after auto-merge; renamed to 'owning_message_ix' (the parameter name). - Cargo.toml: Removed stale workspace members hkask-wallet and hkask-git-cas (both directories deleted in prior commits but workspace entries remained). - kask/crates/hkask-regulation/src/wallet_manager.rs: Stubbed consume() and settle_rjoules() on WalletBudgetPort — these were API-key encumbrance operations from the deleted hkask-wallet crate; regulation tracks per-agent gas balances, not per-key encumbrances. - kask/crates/hkask-regulation/src/wallet_gas_calibrator.rs: Fixed test to use crate::agent_wallet_store::WalletStore instead of hkask_storage::WalletStore. - kask/crates/hkask-regulation/Cargo.toml: Added tokio macros feature to dev-dependencies for #[tokio::test]. - kask/crates/kask_bridge/Cargo.toml: Added futures dependency (needed by context_injector.rs for futures::executor::block_on). - kask/crates/kask_bridge/src/context_injector.rs: Fixed futures_util::executor to futures::executor (futures-util doesn't include executor module). Release Notes: - N/A
jolutz
pushed a commit
to jolutz/zed
that referenced
this pull request
Aug 8, 2026
…d-industries#61427) This consolidates the proxy code in `client/src/proxy` into a new `proxy_handshake` crate that implements the client side of HTTP CONNECT and SOCKS4/4a/5/5h handshakes sans-IO: the protocol logic is a pure state machine that never touches a socket, with thin drivers behind `futures-io` and `tokio` feature flags. `client` now drives the websocket's proxy tunnel through the tokio driver, and the `tokio-socks` dependency goes away. The main win is testability: the handshakes are now covered by synchronous byte-level tests on both sides of each exchange, plus a property test that the outcome is invariant under how the proxy's bytes are chunked across reads — the class of partial-read bug that's hard to hit with hand-written tests but easy to hit in production behind a buffering proxy. The crate is written as if it were standalone (no zed-internal dependencies, structured errors instead of `anyhow`), which is also why it grows both driver flavors rather than just the one `client` needs today. Two deliberate behavior changes, both converging on spec/curl semantics: - Proxy credentials embedded in URLs are now percent-decoded before use, per RFC 3986 §3.2.1. Previously they were sent in their encoded form, which only worked for credentials with no reserved characters. A password that literally contains something like `%40` and relied on the old behavior now needs the RFC-correct `%2540` spelling. - `NO_PROXY` is now honored for the collaboration connection, matching the exclusions reqwest already applies to the rest of Zed's HTTP traffic. TLS to the proxy itself (for `https://` proxy URLs) and DNS policy stay in `client`, since those are I/O concerns the sans-IO crate deliberately doesn't own. Release Notes: - Improved proxy support: the collaboration connection now respects `NO_PROXY` and percent-encoded proxy credentials.
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
2 participants
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.
This consolidates the proxy code in
client/src/proxyinto a newproxy_handshakecrate that implements the client side of HTTP CONNECT and SOCKS4/4a/5/5h handshakes sans-IO: the protocol logic is a pure state machine that never touches a socket, with thin drivers behindfutures-ioandtokiofeature flags.clientnow drives the websocket's proxy tunnel through the tokio driver, and thetokio-socksdependency goes away.The main win is testability: the handshakes are now covered by synchronous byte-level tests on both sides of each exchange, plus a property test that the outcome is invariant under how the proxy's bytes are chunked across reads — the class of partial-read bug that's hard to hit with hand-written tests but easy to hit in production behind a buffering proxy. The crate is written as if it were standalone (no zed-internal dependencies, structured errors instead of
anyhow), which is also why it grows both driver flavors rather than just the oneclientneeds today.Two deliberate behavior changes, both converging on spec/curl semantics:
%40and relied on the old behavior now needs the RFC-correct%2540spelling.NO_PROXYis now honored for the collaboration connection, matching the exclusions reqwest already applies to the rest of Zed's HTTP traffic.TLS to the proxy itself (for
https://proxy URLs) and DNS policy stay inclient, since those are I/O concerns the sans-IO crate deliberately doesn't own.Release Notes:
NO_PROXYand percent-encoded proxy credentials.