Skip to content

feat: add proxy command for sandbox port forwarding with review fixes - #15

Merged
wswcfan merged 10 commits into
TencentCloudAgentRuntime:mainfrom
keithfx999:main
Mar 29, 2026
Merged

feat: add proxy command for sandbox port forwarding with review fixes#15
wswcfan merged 10 commits into
TencentCloudAgentRuntime:mainfrom
keithfx999:main

Conversation

@keithfx999

@keithfx999 keithfx999 commented Mar 19, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Summary

Adds ags proxy, a new subcommand that forwards a remote sandbox port to localhost — similar to kubectl port-forward. Both HTTP and WebSocket protocols are fully supported, making it suitable for web dev servers (Vite, webpack-dev-server), API servers, and any HTTP-based service running inside a sandbox.

# Forward sandbox port 8080 to localhost:8080
ags proxy sandbox-xxx 8080

# Forward sandbox port 8080 to a different local port
ags proxy sandbox-xxx 3000:8080

# Bind to all interfaces (with a security warning)
ags proxy sandbox-xxx 8080 --address 0.0.0.0

How it works

  • Starts a local HTTP server that acts as a reverse proxy to https://<port>-<sandbox-id>.<region-domain>
  • Automatically injects the sandbox access token (X-Access-Token) into every proxied request
  • WebSocket upgrade requests are detected per RFC 6455 and tunnelled via a bidirectional gorilla/websocket bridge
  • The access token is acquired once at startup; its lifetime is bound to the sandbox instance lifecycle so no refresh or 401-retry logic is needed

Changes

File Change
cmd/proxy.go CLI entry point: argument parsing, address validation, signal handling
cmd/proxy_test.go Unit tests for parsePortSpec (18 cases incl. edge cases)
internal/proxy/proxy.go Core reverse proxy: HTTP forwarding, WebSocket bridge, graceful shutdown
internal/proxy/proxy_test.go Integration tests: HTTP/WS forwarding, concurrency, Stop-with-active-WS, verbose logging
internal/repl/repl.go REPL autocomplete and help text
docs/ags-proxy{,-zh}.md New bilingual reference page
README{,-zh}.md, docs/ags{,-zh}.md Feature list and command table updated

Security

  • Binds to 127.0.0.1 by default; prints an explicit warning when --address is set to a non-loopback address
  • In non-verbose mode, proxy errors only return "Bad Gateway" to the client — internal host names and TLS details are never exposed
  • Token is injected via header only, never in URLs or logs

Known Limitations

  • WebSocket Ping/Pong control frames are handled internally by the proxy (gorilla/websocket behaviour) and are not forwarded end-to-end
  • The access token is fetched once at startup; if the sandbox is destroyed and recreated while the proxy is running, restart ags proxy to pick up the new token

Test Plan

  • go test ./internal/proxy/... ./cmd/... — all pass
  • golangci-lint run ./... — 0 issues
  • gofmt -d . — no diff
  • Manual: ags proxy <sandbox-id> 8080 against a live sandbox, verified HTTP and WebSocket traffic forwarded correctly

kofj
kofj reviewed Mar 19, 2026
View reviewed changes
Comment thread cmd/proxy.go
@keithfx999
keithfx999 requested a review from vaaandark March 23, 2026 08:09
keithfx999 and others added 3 commits March 25, 2026 16:22
@keithfx999 @claude
fix: simplify token handling and improve proxy code quality
0087d62 
- Replace per-request TokenProvider with a single upfront Token string,
  removing the tokenAwareTransport/tokenErrorKey machinery (~40 lines).
  Token and sandbox instance share the same lifecycle so one-time
  acquisition is sufficient.
- Remove redundant outer 5 s shutdown timeout in cmd/proxy.go; p.Stop()
  already blocks with its own 5 s graceful-shutdown window.
- Drop time.Sleep(100ms) in TestProxyStartStop; p.Stop() is synchronous.
- Translate two Chinese comments to English in internal/proxy/proxy.go.
- Make REPL completer position-aware: show <sandbox-id> hint at arg 1,
  port-format hints at arg 2, and flags only after both args are present.
- Add --verbose to REPL help text.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999 @claude
docs: add proxy command documentation
ac491c7 
- Add docs/ags-proxy.md and docs/ags-proxy-zh.md with full reference
  for the new proxy command (synopsis, options, port syntax, examples,
  workflow, and see-also links)
- Update docs/ags.md and docs/ags-zh.md to include proxy in the
  command table and see-also section
- Update README.md and README-zh.md: add Port Forwarding feature bullet,
  add a Port Forwarding section with quick examples, and add proxy row
  to the command reference table

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999 @claude
fix(proxy): clarify port config instructions, remove redundant auth h…
c5b543b 
…eader, document token lifecycle

- Problem 1: Replace vague "configure as accessible" note with concrete
  steps across all docs (cmd/proxy.go, README.md, README-zh.md,
  docs/ags-proxy.md, docs/ags-proxy-zh.md): navigate to sandbox
  instance → Network → Open Port and add the port to the allowlist.

- Problem 2: Remove redundant Authorization: Bearer header from both
  HTTP Director and WebSocket upstream dialer; the sandbox gateway
  authenticates via X-Access-Token only.

- Problem 3: Add explicit comments explaining that the access token
  lifetime is bound to the sandbox instance lifecycle, so no token
  refresh or 401-retry logic is needed.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999
keithfx999 marked this pull request as draft March 26, 2026 06:43
keithfx999 and others added 4 commits March 26, 2026 15:54
@keithfx999 @claude
fix(proxy): fix WebSocket shutdown, response body leak, missing timeo…
404e5ae 
…uts, and network warning

- WebSocket Stop() race: start a watchdog goroutine that calls
  SetReadDeadline(time.Now()) on both connections when p.ctx is
  cancelled, so bridgeWebSocket goroutines unblock immediately
  instead of hanging until the remote peer closes.

- upstreamResp.Body leak: always defer-close the HTTP response body
  returned by websocket.Dialer.DialContext, which is non-nil when
  the upstream rejects the upgrade with a non-101 response.

- Missing ResponseHeaderTimeout (30s) on the reverse-proxy Transport
  to prevent HTTP requests from hanging indefinitely when the remote
  sandbox is unresponsive.

- Missing ReadHeaderTimeout (10s) on http.Server to mitigate Slowloris
  attacks when --address is set to a non-loopback interface.

- Print a warning to stderr when --address is not 127.0.0.1/localhost,
  informing the user that the proxy (and the sandbox access token)
  will be reachable from the network.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999 @claude
fix(proxy): address all remaining review issues
cf2751e 
- Add ::1 (IPv6 loopback) to the non-warning allowlist alongside
  127.0.0.1 and localhost.

- Validate --address format upfront with net.ParseIP; non-loopback
  hostnames now get a clear error instead of a confusing net.Listen
  failure.

- Restrict error detail in the reverse-proxy ErrorHandler: full error
  text is only sent to the client in verbose mode to avoid leaking
  internal hostnames or network topology to network-accessible clients.

- Add TestProxyStopWithActiveWebSocket: verifies that p.Stop() returns
  within 10 s even when a WebSocket bridge goroutine is actively
  blocked in ReadMessage, exercising the SetReadDeadline shutdown path.

- Add TestProxyVerboseLogging: verifies that verbose mode writes the
  HTTP method and path to the configured logger.

- Add parsePortSpec edge-case tests: empty string and multi-colon
  input (e.g. "8080:80:8080").

- Document two known limitations in ags-proxy.md / ags-proxy-zh.md:
  (1) WebSocket Ping/Pong control frames are not forwarded;
  (2) the access token is acquired once at startup — restart the proxy
  if the sandbox is destroyed and recreated.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999 @claude
fix(proxy): align WS error response and connection logs with verbose …
2e1323e 
…flag

- WebSocket upstream dial failure now only exposes error details to the
  client when verbose mode is enabled, matching the behaviour of the
  HTTP reverse-proxy ErrorHandler and avoiding leaking internal host
  names or TLS details to network-accessible clients.

- WebSocket connection-established and connection-closed log lines are
  now guarded by p.options.Verbose, consistent with the HTTP request
  logging path.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999 @claude
fix(proxy): fix IPv6 listen address, buffer sizes, and minor cleanups
a163ec3 
- Fix IPv6 listen address bug: use net.JoinHostPort instead of
  fmt.Sprintf so IPv6 addresses are correctly bracketed
  (e.g. "[::1]:8080" instead of the invalid "::1:8080").

- Set explicit ReadBufferSize/WriteBufferSize (65536) on both the
  WebSocket Upgrader and Dialer so large frames are handled
  efficiently without fragmentation.

- Add explanatory comment for why ReadTimeout is intentionally omitted
  from http.Server (would also cap long-lived WebSocket connections).

- Move "Proxy stopped." log line inside the `if p.server != nil` guard
  so it is not printed when Stop() is called before Start().

- Remove internal implementation detail "SandPortal" from the public
  package doc comment; use "AGS gateway" instead.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@keithfx999
keithfx999 marked this pull request as ready for review March 26, 2026 08:58
@wswcfan
wswcfan merged commit f85120d into TencentCloudAgentRuntime:main Mar 29, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vaaandark vaaandark Awaiting requested review from vaaandark
1 more reviewer
@kofj kofj kofj left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@keithfx999 @kofj @wswcfan