amadan
edge: fix scheme-less tunnel auth bypass, correct stale comments, document known limitations
- tunnel.go: strings.CutPrefix (requiring ok) instead of TrimPrefix on the Authorization header — a header with no "Bearer " prefix now 401s instead of comparing its raw value against the configured token. Test added. - tunnel.go: TunnelHub.State's doc comment falsely claimed fleet-status consumption; corrected to note it's test-consumed only today. - edge.go: Reload's tunnel-case comment only covered the hub-less degrade path; extended it to cover a hub-configured edge whose client set lacks the route's channel name (deliberate 502, honors the newest binding). - Documented two known limitations surfaced in final review: the second tunnel hop re-stamps X-Forwarded-For/-Proto (client identity dropped, never spoofed) and WebSockets cannot traverse a tunnel-backed route (h2 RoundTrip rejects Connection: Upgrade). Recorded in the design spec and infra/remote-host.md, with future-fix directions for each.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
5 files changed,
+75
−3
docs/superpowers/specs/2026-08-04-remote-host-pi-design.md+28 −0infra/remote-host.md+16 −0internal/edge/edge.go+8 −0internal/edge/tunnel.go+9 −3internal/edge/tunnel_test.go+14 −0
diff --git a/docs/superpowers/specs/2026-08-04-remote-host-pi-design.md b/docs/superpowers/specs/2026-08-04-remote-host-pi-design.md| index 5b06fb4f0..8bcf53651 100644 |
| --- a/docs/superpowers/specs/2026-08-04-remote-host-pi-design.md |
| +++ b/docs/superpowers/specs/2026-08-04-remote-host-pi-design.md |
| @@ -42,6 +42,34 @@ outbound 443, which survives venue Wi-Fi. |
| `applyPurgeHeader` outbound. **A tunnel-backed host is exactly as |
| untrusted as an addr-backed one.** |
| + **Known limitations (final review, 2026-08-04):** |
| + - **Second-hop header fidelity.** That inherited-discipline claim holds |
| + for the first hop (client → edge) but not the second: a tunnel-backed |
| + request also crosses the remote host's OWN `*edge.Edge` — the same |
| + `ReverseProxy` code, but a second, independent hop with its own |
| + `SetXForwarded` call. Stdlib `Rewrite`-mode proxying deletes any |
| + inbound `X-Forwarded-For`/`X-Forwarded-Proto` and re-stamps them from |
| + the immediate peer, so the app on the remote host sees the edge's own |
| + IP as client (not the real one) and `X-Forwarded-Proto: http` (h2 |
| + `ServeConn` over the hijacked tunnel conn has `r.TLS == nil`, even |
| + though the original client-to-edge hop was TLS). This fails in the |
| + safe direction — client identity is DROPPED, never spoofed; a |
| + tunnel-backed app cannot be tricked into trusting a forged |
| + `X-Forwarded-*` from outside. Future fix: the remote host's edge could |
| + special-case its own tunnel listener and trust/forward the inbound |
| + `X-Forwarded-*` it already received over that hijacked conn, since |
| + that hop terminates the same `TunnelHub` handshake this file |
| + implements — not general inbound trust. |
| + - **No WebSockets over a tunnel-backed route.** The edge's upgrade path |
| + hands `Connection: Upgrade` requests straight to the route's |
| + `ReverseProxy`; a tunnel's transport is an `http2.ClientConn` |
| + (`RoundTrip`), which rejects connection-specific headers, so every |
| + WebSocket handshake to a tunnel-backed host fails closed — a loud 502, |
| + never a silent hang or a downgraded connection. Future door: RFC 8441 |
| + extended CONNECT, which carries a bidirectional stream inside h2 and |
| + is the standards-track way to tunnel a WebSocket over an HTTP/2 |
| + connection. |
| + |
| ### Transport: reverse HTTP/2 over a host-dialed TLS connection |
| Decision (the in-repo judgment call #53 asks for, recorded): we port the |
diff --git a/infra/remote-host.md b/infra/remote-host.md| index 375e67083..9782cfdb4 100644 |
| --- a/infra/remote-host.md |
| +++ b/infra/remote-host.md |
| @@ -73,6 +73,22 @@ In `-dev` (HTTP) edge mode the registration bearer token travels in clear |
| text. Tunnel hosts must only ever dial a real-TLS edge in production — |
| never point `CARLOS_TUNNEL_URL` at an HTTP edge outside a dev sandbox. |
| +## Known limitations |
| + |
| +- **Apps see the edge's IP, not the real client's.** A tunnel-backed request |
| + crosses two reverse-proxy hops: client → edge (over the internet), then |
| + edge → app (over the tunnel, on the remote host's own local edge). The |
| + second hop re-stamps `X-Forwarded-For`/`X-Forwarded-Proto` from its own |
| + immediate peer, so an app on a tunnel-backed host sees the edge's IP as |
| + client, and `X-Forwarded-Proto: http` even though the original request was |
| + TLS. This fails safe — client identity is dropped, never spoofed — but any |
| + app on a tunnel-backed host that logs or gates on client IP will be wrong |
| + today. |
| +- **No WebSockets to a tunnel-backed host.** The tunnel transport can't carry |
| + a `Connection: Upgrade` handshake; every WebSocket attempt to an app on a |
| + tunnel-backed host fails with a 502 (loud, not a hang). Plain HTTP request/ |
| + response is unaffected. RFC 8441 extended CONNECT is the likely future fix. |
| + |
| ## Related |
| The hq PHYSICAL half (racking, power, network uplink for a region-hq box) |
diff --git a/internal/edge/edge.go b/internal/edge/edge.go| index d52f23491..d4835aabd 100644 |
| --- a/internal/edge/edge.go |
| +++ b/internal/edge/edge.go |
| @@ -261,6 +261,14 @@ func (e *Edge) Reload() error { |
| // previous backend, never take the host off the air); a tunnel-only |
| // route on such an edge matches nothing and falls to |
| // `default: continue`, un-served and un-certed, until the hub lands. |
| + // |
| + // That degrade path only covers a HUB-LESS edge (e.tunnels == nil). |
| + // A hub-configured edge whose client set doesn't (yet) include this |
| + // route's channel name still takes this branch — e.tunnels != nil is |
| + // all this case checks — and the built proxy 502s every request, even |
| + // though t.Addr may hold a perfectly working backend. Deliberate: once |
| + // a hub exists, a `-tunnel` binding is the newest operator intent and |
| + // wins outright rather than silently falling back to a stale Addr. |
| case t.Tunnel != "" && e.tunnels != nil: |
| p := tunnelProxy(t.Tunnel, e.tunnels) |
| p.ModifyResponse = e.purgeModifier(t) |
diff --git a/internal/edge/tunnel.go b/internal/edge/tunnel.go| index 8cd1659e2..afd6459d0 100644 |
| --- a/internal/edge/tunnel.go |
| +++ b/internal/edge/tunnel.go |
| @@ -91,7 +91,11 @@ func (h *TunnelHub) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| return |
| } |
| name := r.Header.Get("Carlos-Tunnel-Name") |
| - token := strings.TrimPrefix(r.Header.Get("Authorization"), "Bearer ") |
| + // CutPrefix's ok is load-bearing: TrimPrefix would return a scheme-less |
| + // header (e.g. "Authorization: sekrit") unchanged, letting a credential |
| + // that never claimed to be a bearer token compare equal to a configured |
| + // one. No "Bearer " prefix is an outright 401, not a token of "". |
| + token, hasBearer := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ") |
| want, ok := h.clients[name] |
| // Constant-time compare; the name lookup happening first is fine — the |
| // name is not the secret, the token is. A token is bound to exactly one |
| @@ -99,7 +103,7 @@ func (h *TunnelHub) ServeHTTP(w http.ResponseWriter, r *http.Request) { |
| // An empty configured token never authenticates anything: parseTunnelClients |
| // rejects one, and a hub built some other way must not turn a blank config |
| // value into a channel that answers to "Authorization: Bearer ". |
| - if name == "" || !ok || want == "" || subtle.ConstantTimeCompare([]byte(token), []byte(want)) != 1 { |
| + if name == "" || !ok || want == "" || !hasBearer || subtle.ConstantTimeCompare([]byte(token), []byte(want)) != 1 { |
| http.Error(w, "unauthorized", http.StatusUnauthorized) |
| return |
| } |
| @@ -193,7 +197,9 @@ func (h *TunnelHub) conn(name string) *http2.ClientConn { |
| return nil |
| } |
| -// State reports "connected" or "" — consumed by fleet status rendering. |
| +// State reports "connected" or "". The heartbeat's tunnel-state field comes |
| +// from the host-side dial loop, not from here; today this is test-consumed |
| +// only — no production caller reads it. |
| func (h *TunnelHub) State(name string) string { |
| if h.conn(name) != nil { |
| return "connected" |
diff --git a/internal/edge/tunnel_test.go b/internal/edge/tunnel_test.go| index 7efe2eb32..cc02d65c0 100644 |
| --- a/internal/edge/tunnel_test.go |
| +++ b/internal/edge/tunnel_test.go |
| @@ -47,6 +47,20 @@ func TestTunnelRegisterRejectsBadToken(t *testing.T) { |
| } |
| } |
| +// Scheme-less credential: "Authorization: sekrit" must not authenticate even |
| +// though "sekrit" is the correct configured token — TrimPrefix would have let |
| +// this through unchanged (nothing to strip), which is the bug this pins. |
| +func TestTunnelRegisterRejectsSchemelessAuthorization(t *testing.T) { |
| + hub := NewTunnelHub("tunnel.test", map[string]string{"pi-box": "sekrit"}) |
| + req := tunnelReq("pi-box", "") |
| + req.Header.Set("Authorization", "sekrit") // no "Bearer " prefix |
| + rec := httptest.NewRecorder() |
| + hub.ServeHTTP(rec, req) |
| + if rec.Code != http.StatusUnauthorized { |
| + t.Fatalf("scheme-less Authorization: got %d, want 401", rec.Code) |
| + } |
| +} |
| + |
| func TestTunnelRegisterAuthPrecedesUpgradeCheck(t *testing.T) { |
| hub := NewTunnelHub("tunnel.test", map[string]string{"pi-box": "sekrit"}) |
| req := httptest.NewRequest("GET", "/register", nil) // no upgrade headers at all |