Add proxy-tunnel support - #1836
Conversation
TODO: Gate behind entitlement prior to merge
Swift Lint FailedThe Swift linter found issues with your code. Please run the following commands to fix them: Lint issuesThen commit and push the changes. |
AI Security ReviewNote Automated security review from Claude. Apply, adapt, silence with Input coverage: 11/11 changed files; 25,912/25,912 bytes reviewed; diff SHA-256 Claude found security review findings for this PR. 🚨 Critical — Open CRITICAL: SSRF loopback guard removed from Go agent broker dial handler
Details**Status:** Open
**Severity:** CRITICAL
**Standards:** SOC2-CC6.1, SOC2-CC6.6, ISO27001-A.8.20, ISO27001-A.8.22, NIST-800-53-SC-7, NIST-CSF-PR.AC
**Location:** `go/internal/agent/services/tunnel_broker_client.go:259-320`
The removed block previously rejected any `req.Host` that was not `localhost` or a loopback IP:
```go
ip := net.ParseIP(req.Host)
if req.Host != "localhost" && (ip == nil || !ip.IsLoopback()) {
// reject
}
```
After this change, `req.Host` flows directly into `net.JoinHostPort(req.Host, ...)` and is dialed. Because `req.Host` originates from the CLI/broker and is resolved by the device, a compromised or malicious broker (or any actor able to inject a DialRequest) can pivot the agent into connecting to arbitrary internal LAN services (databases, metadata endpoints, admin panels), turning the device into an SSRF/pivot proxy. `isTunnelLoopbackHost` is now used only to decide port remapping, not to gate the target.
**Remediation:** Do not remove the target restriction unless the device-side reachability is explicitly authorized and entitlement-gated per the PR's own TODO. If remote-host forwarding is a deliberate feature, enforce authorization server-side (org/device policy), add an allowlist/denylist for internal ranges (RFC1918, link-local, metadata IPs like 169.254.169.254), and require explicit per-device opt-in before merging. Do not merge with the guard silently removed.🚨 Critical — Open CRITICAL: SSRF loopback guard removed from Swift agent broker dial handler
Details**Status:** Open
**Severity:** CRITICAL
**Standards:** SOC2-CC6.1, SOC2-CC6.6, ISO27001-A.8.20, NIST-800-53-SC-7
**Location:** `swift/WendyAgentCore/Sources/WendyAgent/Cloud/TunnelBrokerClient.swift:327-345`
The `guard Self.isLoopback(dial.host) else { ... return }` gate was removed and replaced with `tunnelDialPort`, which only affects port remapping. `dial.host` is now used to open a TCP connection to an arbitrary host resolved by the device. This is the same broker-directed SSRF/pivot exposure as the Go path and affects Apple-based WendyOS agents identically.
**Remediation:** Same as the Go finding — retain a target-authorization control, entitlement-gate the remote-host feature, and block internal/metadata address ranges before allowing arbitrary device-resolved destinations.🛑 Error — Open HIGH: Security-relevant feature merged without its stated entitlement gate
Details**Status:** Open
**Severity:** HIGH
**Standards:** SOC2-CC8.1, ISO27001-A.8.28, ISO27001-A.8.32, NIST-800-53-SA-11, NIST-CSF-PR.PS
**Location:** `go/internal/cli/commands/cloud_forward.go:22-45`
The PR description contains `TODO: Gate behind entitlement prior to merge` and describes the capability as 'Effectively equivalent to an SSH tunnel.' The diff introduces full remote-host forwarding (`<local-port>:<remote-host>:<remote-port>`) and removes the agent-side restriction, yet there is no entitlement/authorization check, feature flag, or policy control anywhere in the changed code. Shipping a device-pivoting capability without authorization boundaries violates least-privilege and change-control expectations.
**Remediation:** Implement the entitlement/authorization gate (server-side, enforced at the broker and/or agent) before merge. Ensure that only explicitly entitled orgs/devices/users can request remote-host targets, and log/audit such requests.
|
github-actions
Bot
added
risk: high
There was a problem hiding this comment.
🟡 Changes recommended
The PR description calls out entitlement gating as a pre-merge requirement, but the updated tunnel dialing paths (Go + Swift) don’t implement a corresponding entitlement/feature-flag enforcement.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds “proxy-tunnel” (remote host) support to the Wendy Cloud tunnel feature so the CLI can forward traffic through a cloud-enrolled device to another TCP host reachable from that device, while keeping loopback-only port remapping for the agent’s well-known mTLS port.
Changes:
- Extend
wendy cloud tunnelto accept<local-port>:<remote-host>:<remote-port>(TCP only) and plumb the remote host through the broker open message. - Update agent-side tunnel dialing (Go + Swift) to dial the requested host/port, only remapping the agent mTLS port when the target is loopback.
- Update tests and user docs to cover the new argument forms and behavior.
File summaries
| File | Description |
|---|---|
| swift/WendyAgentCore/Tests/WendyAgentTests/TunnelBrokerClientTests.swift | Adds unit test coverage for host-aware port remapping logic. |
| swift/WendyAgentCore/Sources/WendyAgent/Cloud/TunnelBrokerClient.swift | Introduces tunnelDialPort and changes tunnel handling to connect to requested host/port (with loopback-only remap). |
| go/internal/cli/commands/cloud_tunnel.go | Adds openBrokerTunnelToHost and factors open-message creation to include remote host. |
| go/internal/cli/commands/cloud_tunnel_test.go | Expands parser tests for <local>:<host>:<port> and adds a test ensuring open messages include the remote host. |
| go/internal/cli/commands/cloud_forward.go | Updates CLI UX, argument parsing to support remote-host forwarding, and logs to reflect target host. |
| go/internal/cli/commands/cloud_datagram_test.go | Updates callsites to the updated parseTunnelArg signature. |
| go/internal/cli/assets/docs/cloud/tunnel.md | Updates conceptual docs and examples to include remote-host forwarding. |
| go/internal/cli/assets/docs/clients/wendy-cli/commands/cloud/tunnel.md | Updates command docs for the new positional <port-forward> argument and examples. |
| go/internal/agent/services/tunnel_broker_client.go | Removes loopback-only TCP dial restriction and adds host-aware port remap helper. |
| go/internal/agent/services/tunnel_broker_client_test.go | Adds tests for loopback detection and remap behavior. |
| go/internal/agent/services/mesh_service.go | Updates comment to clarify mesh dialing remains local-only. |
Review details
Suppressed comments (2)
go/internal/agent/services/tunnel_broker_client.go:272
- PR description says this feature must be gated behind an entitlement before merge, but TCP dial handling now allows arbitrary
req.Hosttargets with no entitlement/feature-flag check. SinceDialRequestcarries only host/port/protocol, this effectively enables broker-directed network pivoting from the device unless the broker enforces policy elsewhere.
func (c *TunnelBrokerClient) handleDialRequest(ctx context.Context, client cloudpb.TunnelBrokerServiceClient,
req *cloudpb.DialRequest, devMD metadata.MD) {
if req.GetProtocol() == cloudpb.TunnelProtocol_TUNNEL_PROTOCOL_DATAGRAM {
c.handleDatagramDial(ctx, client, req, devMD)
return
}
port := tunnelDialPort(req.Host, int(req.Port), c.mtlsPort)
addr := net.JoinHostPort(req.Host, fmt.Sprint(port))
c.logger.Info("dialing local service for tunnel",
zap.String("session_id", req.SessionId), zap.String("addr", addr))
tcpConn, err := net.DialTimeout("tcp", addr, 10*time.Second)
swift/WendyAgentCore/Sources/WendyAgent/Cloud/TunnelBrokerClient.swift:350
- PR description calls out entitlement gating as a pre-merge requirement, but
handleDialnow connects to the requested host/port without enforcing any local entitlement/feature-flag check. If broker-side policy is incomplete/misconfigured, this becomes a general-purpose TCP proxy through the device.
/// Serves one broker `DialRequest`: opens a plain TCP connection to the
/// requested host and port, opens an `AgentTunnel` stream, and runs the two
/// relay pumps until the session ends.
private static func handleDial<Transport: ClientTransport>(
_ dial: Wendycloud_V1_DialRequest,
client: Wendycloud_V1_TunnelBrokerService.Client<Transport>,
config: Config,
metadata: Metadata,
logger: Logger
) async {
let port = Self.tunnelDialPort(
host: dial.host,
requested: Int(dial.port),
mtlsPort: config.mtlsPort
)
let sessionID = dial.sessionID
do {
let local = try await ClientBootstrap(group: .singletonMultiThreadedEventLoopGroup)
.channelOption(ChannelOptions.allowRemoteHalfClosure, value: true)
.connect(host: dial.host, port: port) { channel in
- Files reviewed: 11/11 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if host == "" { | ||
| return 0, "", 0, false, fmt.Errorf("invalid remote host %q", host) | ||
| } |
Docs previewPreview this PR's docs at: https://docs.wendy.dev/branch-jo-proxy-tunnel-a667c052e04540601bc99be2541c2a6c51b656d0/ This comment is updated automatically when the docs preview is redeployed. |
Swift E2E Review🛑 wendy cloud tunnel help usage line no longer matches E2E expectationThe diff renamed the
Recommendation: pick one canonical presentation and align both sides. The E2E spec change is the smaller fix; the CLI-side fix (e.g. DetailsWhat broke
The recording ( The test at stdout.contains("wendy cloud tunnel <local-port>:<remote-port>[/udp] [flags]")Why it changedThe PR introduces optional // go/internal/cli/commands/cloud_forward.go
Use: "tunnel <port-forward>",
Long: "Listens on a local port and forwards each connection through the Wendy Cloud tunnel broker. " +
"Use <port>, <local-port>:<remote-port>, or <local-port>:<remote-host>:<remote-port>. ..."Cobra renders Recommended fixTwo viable options, in preference order:
Additional observations from the same help outputWhile reviewing per the
Category / confidence
Report artifact: swift-e2e-tests.gh33308875462.run.0001 |
thombles
left a comment
thombles
left a comment
There was a problem hiding this comment.
We should pass back information to the CLI about different failure modes - connection refused, DNS resolution, timeout, etc.
The security controls are the most interesting part and worth checking closely once implemented.
TODO: Gate behind entitlement prior to merge
Effectively equivalent to an SSH tunnel