Docs: socks5 autodiscovery, exit security, rust socks5 page#6928
Docs: socks5 autodiscovery, exit security, rust socks5 page#6928mfahampshire wants to merge 3 commits into
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
📝 WalkthroughWalkthroughChangesThe PR adds a Rust SDK SOCKS5 module documentation page, updates Rust SDK navigation and cross-references, generalizes Exit Gateway security guidance to IPR and NR, and revises service consumer listings. Rust SDK SOCKS5 documentation
Supporting documentation alignment
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant RustApplication
participant Socks5MixnetClient
participant Mixnet
participant NetworkRequester
participant Destination
RustApplication->>Socks5MixnetClient: Connect through socks5h://
Socks5MixnetClient->>Mixnet: Send segmented TCP stream
Mixnet->>NetworkRequester: Deliver SOCKS request
NetworkRequester->>Destination: Request public destination
Destination-->>NetworkRequester: Return response
NetworkRequester-->>Socks5MixnetClient: Reassemble response
Socks5MixnetClient-->>RustApplication: Return response
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)documentation/docs/pages/developers/rust/_meta.jsonTraceback (most recent call last): 🔧 ESLint
ESLint install timed out. The project may have too many dependencies for the sandbox. 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 |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@documentation/docs/pages/developers/concepts/exit-security.mdx`:
- Around line 70-71: The SOCKS5 documentation bullet uses stale API names, so
update the referenced callsites in this section to match the current symbols.
Replace the outdated `Socks5MixnetClient::connect_new(...)` and
`discover().countries([...])` wording with the correct
`Socks5MixnetClient::connect_with(...)` and
`NetworkRequester::in_countries([...])` references so the docs align with the
actual API.
In `@documentation/docs/pages/developers/rust.mdx`:
- Line 54: The SOCKS proxy description in the Rust developer docs overstates
supported protocols and should be narrowed to match the actual
`Socks5MixnetClient` contract. Update the `SOCKS5 module` blurb in the Rust docs
to describe only SOCKS5 support unless there is explicit implementation evidence
elsewhere, and keep the wording aligned with the `./rust/socks5` module name and
related docs.
In `@documentation/docs/pages/developers/rust/socks5.mdx`:
- Line 129: The SOCKS5 example link currently points to the mutable develop
branch, which can drift from the documented Rust API. Update the link in the
socks5.mdx page to use a version-stable target, such as a tagged permalink or
another immutable reference, so the example stays aligned with the docs. Use the
existing “Example: SOCKS5 proxy” link text as the anchor and replace only the
branch-based GitHub target.
🪄 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: bff02bde-c53b-46c3-ae18-26034e4d8633
📒 Files selected for processing (8)
documentation/docs/pages/developers/_meta.jsondocumentation/docs/pages/developers/clients/socks5.mdxdocumentation/docs/pages/developers/concepts/exit-security.mdxdocumentation/docs/pages/developers/index.mdxdocumentation/docs/pages/developers/rust.mdxdocumentation/docs/pages/developers/rust/_meta.jsondocumentation/docs/pages/developers/rust/socks5.mdxdocumentation/docs/pages/network/infrastructure/exit-services.mdx
|
|
||
| - [**`smolmix`**](/developers/smolmix): `TcpStream` and `UdpSocket` over the Mixnet via a userspace IP stack. Compatible with `tokio-rustls`, `hyper`, `tokio-tungstenite`, and the rest of the async Rust ecosystem. | ||
| - [**SOCKS Client**](./rust/mixnet): SOCKS4/4a/5 proxy via the Exit Gateway's Network Requester. Works with any SOCKS-capable application without code changes. | ||
| - [**SOCKS5 module**](./rust/socks5): SOCKS4/4a/5 proxy via the Exit Gateway's network requester. Works with any SOCKS-capable application without code changes. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Don't overstate protocol support here.
The supplied upstream contract describes Socks5MixnetClient as a SOCKS5 proxy interface, so calling the SDK module a “SOCKS4/4a/5 proxy” looks broader than the implementation/docs evidence in this PR. Please narrow this to SOCKS5 unless there’s matching implementation evidence elsewhere.
🤖 Prompt for 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.
In `@documentation/docs/pages/developers/rust.mdx` at line 54, The SOCKS proxy
description in the Rust developer docs overstates supported protocols and should
be narrowed to match the actual `Socks5MixnetClient` contract. Update the
`SOCKS5 module` blurb in the Rust docs to describe only SOCKS5 support unless
there is explicit implementation evidence elsewhere, and keep the wording
aligned with the `./rust/socks5` module name and related docs.
| ## Further reading | ||
|
|
||
| - [API reference on docs.rs](https://docs.rs/nym-sdk/latest/nym_sdk/mixnet/struct.Socks5MixnetClient.html): all methods, configuration, and types | ||
| - [Example: SOCKS5 proxy](https://github.com/nymtech/nym/blob/develop/sdk/rust/nym-sdk/examples/socks5.rs): select a requester (auto-discover, pin by country, or a known address) and proxy a request |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid linking versioned docs to develop.
This page is being merged independently of the SDK rollout, so a develop-branch example can drift from the API documented here and confuse readers later. Prefer a tagged permalink or another version-stable target.
🤖 Prompt for 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.
In `@documentation/docs/pages/developers/rust/socks5.mdx` at line 129, The SOCKS5
example link currently points to the mutable develop branch, which can drift
from the documented Rust API. Update the link in the socks5.mdx page to use a
version-stable target, such as a tagged permalink or another immutable
reference, so the example stays aligned with the docs. Use the existing
“Example: SOCKS5 proxy” link text as the anchor and replace only the
branch-based GitHub target.
Split out from max/socks5-to-socks-plus-autodiscovery so docs can be reviewed and merged (autodeploys) independently of the nym-sdk code, which still needs QA.
The SDK type is NetworkRequesterSelector, not NetworkRequester, and the exit-security callout referenced a discovery builder that no longer exists. Addresses PR 6928 review comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix the country-selection claim to match the self-reported location semantics, note reqwest's socks feature requirement, reword the builder snippet to what it demonstrates, deduplicate the exit-trust prose, capitalise Mixnet and Network Requester per house style, correct both ASCII diagrams, retarget the stale Mixnet-page front-matter, and revert _meta.json churn. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
650798f to
d3ea0f4
Compare
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 `@documentation/docs/pages/developers/concepts/exit-security.mdx`:
- Line 21: Update the proxy-mode explanation in
documentation/docs/pages/developers/concepts/exit-security.mdx at lines 21-21
and 48-48: describe the Network Requester as seeing the SOCKS destination, and
qualify hostname visibility as applying only to socks5h requests while
IP-address requests expose only the address. Preserve the existing distinction
at line 65.
- Around line 89-96: Update the “Exit node sees traffic?” row in the Nym column
to scope its visibility claim to proxy mode, using wording such as “Proxy mode:
Yes.” Keep the existing Tor and VPN entries unchanged and ensure the table no
longer implies that end-to-end Nym traffic has an Exit Gateway.
🪄 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 Plus
Run ID: bfb5a060-dbac-4686-bc05-02adddc02615
📒 Files selected for processing (8)
documentation/docs/pages/developers/clients/socks5.mdxdocumentation/docs/pages/developers/concepts/exit-security.mdxdocumentation/docs/pages/developers/index.mdxdocumentation/docs/pages/developers/rust.mdxdocumentation/docs/pages/developers/rust/_meta.jsondocumentation/docs/pages/developers/rust/mixnet.mdxdocumentation/docs/pages/developers/rust/socks5.mdxdocumentation/docs/pages/network/infrastructure/exit-services.mdx
🚧 Files skipped from review as they are similar to previous changes (4)
- documentation/docs/pages/developers/index.mdx
- documentation/docs/pages/developers/rust/_meta.json
- documentation/docs/pages/network/infrastructure/exit-services.mdx
- documentation/docs/pages/developers/rust/socks5.mdx
| ## Proxy mode or end-to-end? | ||
|
|
||
| This page is about **proxy mode**: your traffic leaves the mixnet at an IPR exit and continues to a third-party server over clearnet, where the security trade-offs apply. | ||
| Most of this page is about **proxy mode**: your traffic leaves the Mixnet at an Exit Gateway, via either the IP Packet Router (raw IP packets) or the SOCKS-based Network Requester (which sees the destination hostname), and continues to a third-party server over clearnet, where the security trade-offs apply. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Qualify Network Requester destination visibility by SOCKS address type.
A SOCKS request can carry an IP address as well as a domain name; this page already correctly limits hostname exposure to socks5h at line 65.
documentation/docs/pages/developers/concepts/exit-security.mdx#L21-L21: describe the NR as seeing the SOCKS destination, with hostname visibility conditional onsocks5h.documentation/docs/pages/developers/concepts/exit-security.mdx#L48-L48: replace the unconditional “NR sees the destination hostname” with address-type-specific wording.
📍 Affects 1 file
documentation/docs/pages/developers/concepts/exit-security.mdx#L21-L21(this comment)documentation/docs/pages/developers/concepts/exit-security.mdx#L48-L48
🤖 Prompt for 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.
In `@documentation/docs/pages/developers/concepts/exit-security.mdx` at line 21,
Update the proxy-mode explanation in
documentation/docs/pages/developers/concepts/exit-security.mdx at lines 21-21
and 48-48: describe the Network Requester as seeing the SOCKS destination, and
qualify hostname visibility as applying only to socks5h requests while
IP-address requests expose only the address. Preserve the existing distinction
at line 65.
| | | Nym (Mixnet) | Tor | VPN | | ||
| |---|---|---|---| | ||
| | **Exit node sees traffic?** | Yes (encrypt it) | Yes (encrypt it) | Yes (encrypt it) | | ||
| | **Exit node knows sender?** | No (mixnet hides identity) | No (onion routing) | Yes (VPN provider knows) | | ||
| | **Exit node knows sender?** | No (Mixnet hides identity) | No (onion routing) | Yes (VPN provider knows) | | ||
| | **Timing analysis resistance** | Strong with defaults (mixing, cover traffic) | Weak (low-latency) | None | | ||
| | **UDP support** | Yes | No (TCP only) | Yes | | ||
|
|
||
| The timing-analysis rating assumes the defaults. Cover traffic and Poisson timing can be turned off to trade that resistance for latency and bandwidth, moving Nym's row toward Tor's. The named switches for this (`disableCoverTraffic` / `disablePoissonTraffic`) are specific to the browser/wasm packages ([`mix-tunnel`](/developers/mix-tunnel/guides#configuration) and the feature packages built on it); the native `smolmix` crate does not expose them by those names. The UDP row reflects a design difference, not a ranking: Tor is TCP-only by design, while the Nym IPR routes raw IP. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Scope the Nym exit-visibility row to proxy mode.
The page explicitly says end-to-end traffic has no Exit Gateway, but the table currently says an exit sees all Nym traffic. Label this “Proxy mode: yes” (or equivalent).
Proposed fix
-| **Exit node sees traffic?** | Yes (encrypt it) | Yes (encrypt it) | Yes (encrypt it) |
+| **Exit node sees traffic?** | Proxy mode: yes (encrypt it) | Yes (encrypt it) | Yes (encrypt it) |📝 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.
| | | Nym (Mixnet) | Tor | VPN | | |
| |---|---|---|---| | |
| | **Exit node sees traffic?** | Yes (encrypt it) | Yes (encrypt it) | Yes (encrypt it) | | |
| | **Exit node knows sender?** | No (mixnet hides identity) | No (onion routing) | Yes (VPN provider knows) | | |
| | **Exit node knows sender?** | No (Mixnet hides identity) | No (onion routing) | Yes (VPN provider knows) | | |
| | **Timing analysis resistance** | Strong with defaults (mixing, cover traffic) | Weak (low-latency) | None | | |
| | **UDP support** | Yes | No (TCP only) | Yes | | |
| The timing-analysis rating assumes the defaults. Cover traffic and Poisson timing can be turned off to trade that resistance for latency and bandwidth, moving Nym's row toward Tor's. The named switches for this (`disableCoverTraffic` / `disablePoissonTraffic`) are specific to the browser/wasm packages ([`mix-tunnel`](/developers/mix-tunnel/guides#configuration) and the feature packages built on it); the native `smolmix` crate does not expose them by those names. The UDP row reflects a design difference, not a ranking: Tor is TCP-only by design, while the Nym IPR routes raw IP. | |
| | | Nym (Mixnet) | Tor | VPN | | |
| |---|---|---|---| | |
| | **Exit node sees traffic?** | Proxy mode: yes (encrypt it) | Yes (encrypt it) | Yes (encrypt it) | | |
| | **Exit node knows sender?** | No (Mixnet hides identity) | No (onion routing) | Yes (VPN provider knows) | | |
| | **Timing analysis resistance** | Strong with defaults (mixing, cover traffic) | Weak (low-latency) | None | | |
| | **UDP support** | Yes | No (TCP only) | Yes | | |
| The timing-analysis rating assumes the defaults. Cover traffic and Poisson timing can be turned off to trade that resistance for latency and bandwidth, moving Nym's row toward Tor's. The named switches for this (`disableCoverTraffic` / `disablePoissonTraffic`) are specific to the browser/wasm packages ([`mix-tunnel`](/developers/mix-tunnel/guides#configuration) and the feature packages built on it); the native `smolmix` crate does not expose them by those names. The UDP row reflects a design difference, not a ranking: Tor is TCP-only by design, while the Nym IPR routes raw IP. |
🤖 Prompt for 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.
In `@documentation/docs/pages/developers/concepts/exit-security.mdx` around lines
89 - 96, Update the “Exit node sees traffic?” row in the Nym column to scope its
visibility claim to proxy mode, using wording such as “Proxy mode: Yes.” Keep
the existing Tor and VPN entries unchanged and ensure the table no longer
implies that end-to-end Nym traffic has an Exit Gateway.
Split out from max/socks5-to-socks-plus-autodiscovery so docs can be reviewed and merged (autodeploys) independently of the nym-sdk code, which still needs QA.
This change is
Summary by CodeRabbit