fix(gateway): preserve external Tailscale Funnel routes in serve mode#73755
fix(gateway): preserve external Tailscale Funnel routes in serve mode#73755RenzoMXD wants to merge 2 commits intoopenclaw:mainfrom
Conversation
Greptile SummaryThis PR introduces an opt-in The three issues flagged in the prior review round — the missing empty-stdout guard before Confidence Score: 5/5Safe to merge — all previously flagged issues are addressed and the new code is well-guarded and tested. No P0 or P1 issues remain. Previously reported bugs (empty-stdout crash path, silent resetOnExit no-op, always-false proxy comparison) are all fixed. The implementation is backward-compatible, falls back gracefully on any error, and is covered by unit tests for the key edge cases. No files require special attention. Reviews (3): Last reviewed commit: "Merge branch 'main' into fix/openclaw-57..." | Re-trigger Greptile |
|
Codex review: needs maintainer review before merge. Summary Reproducibility: yes. Current main statically reproduces the overwrite path through Next step before merge Security Review detailsBest possible solution: Land a scoped preserveFunnel fix that preserves externally owned Funnel routes without changing OpenClaw-managed Funnel auth policy, after required exact-head CI finishes green. Do we have a high-confidence way to reproduce the issue? Yes. Current main statically reproduces the overwrite path through Is this the best way to solve the issue? Yes for the current PR shape. The opt-in status precheck is a narrow maintainable fix because it only preserves an already external Funnel route and leaves OpenClaw-managed Funnel password-only behavior unchanged. What I checked:
Likely related people:
Remaining risk / open question:
Codex review notes: model gpt-5.5, reasoning high; reviewed against 695960975a60. |
|
Hi, @greptileai |
|
@greptileai Please review my PR again. I think current CI test failures are not from my change. |
Adds the missing public-docs reference flagged in clawsweeper's review on PR openclaw#73755. Two sibling bullets: - docs/gateway/tailscale.md (Notes section, alongside resetOnExit) - docs/gateway/configuration-reference.md (alongside tailscale.mode) No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
b86ed6c to
1013e44
Compare
1013e44 to
29d4a0a
Compare
Summary
gateway.tailscale.mode = "serve", every gateway restart unconditionally runstailscale serve --bg --yes <port>, which overwrites any externally configuredtailscale funnelroute for that port. Public Funnel reverts to tailnet-only on every restart.mode = "funnel"is intentionally password-only (kept per fix(gateway): block mode=none auth with tailscale serve remote exposure #51339).gateway.tailscale.preserveFunnel: boolean. Whentrueandmode = "serve", checktailscale funnel status --jsonfirst; skip re-applying Serve if a Funnel route already covers the gateway port.resetOnExitsemantics unchanged. No changes toserver-runtime-config.ts(fix: Tailscale serve + auth.mode=none exposes gateway to full... #50631's territory).Change Type
Scope
Linked Issue/PR
Root Cause
enableTailscaleServe(port)runstailscale serve --bg --yes <port>, which by Tailscale CLI semantics replaces whatever Serve/Funnel configuration controls that port. No precondition check, no opt-out flag — externally configured Funnel routes are clobbered on every gateway restart.Regression Test Plan
src/gateway/server-tailscale.test.ts(new)enableTailscaleServe; (b) serve + preserveFunnel + Funnel route present → skips; (c) serve + preserveFunnel + no route → falls back; (d) funnel mode → never consults the helper.User-visible Changes
New optional field
gateway.tailscale.preserveFunnel(defaultfalse). Existing deployments behave identically. Whentrue, an externally managed Funnel route survives gateway restarts.Diagram
Security Impact
tailscale funnel status --jsonquery.Repro + Verification
Environment
Steps
gateway.tailscale.mode: "serve".tailscale funnel --bg --set-path / <gateway-port>.tailscale funnel statusshows the route.Expected (with fix +
preserveFunnel: true)Funnel route still public after restart; log records
serve skipped: preserving externally configured Tailscale Funnel for port <port>.Actual (on
maintoday)Funnel route gone after restart — only gateway's Serve binding remains.
Evidence
server-tailscale.test.tsfail onmain, pass with this PR.Human Verification
pnpm tsgo:core,tsgo:core:test,lint:core,check:import-cyclesall clean;pnpm config:schema:gen+config:docs:genregenerated cleanly withgateway.tailscale.preserveFunnelpresent.tailscalebinary, malformed JSON, Funnel enabled but for a different port,mode="funnel"withpreserveFunnel=true— all confirmed via tests.Review Conversations
Compatibility / Migration
false.Risks and Mitigations
funnel status --jsonshape isn't a pinned contract.Mitigation: helper returns
falseon any parse error / non-zero exit; falls back to today's Serve behavior. Zero regression for opt-out users.Mitigation: exact port match — only a handler whose
Proxyresolves to127.0.0.1:<gatewayPort>triggers the skip.