amadan

carlos / platform Public

edge: fix :80 tunnel-host regression, own wildcard-refusal + pin created_at

Final whole-branch review findings:
- httpFallbackHandler gated on lookupUpstream alone, which has no tunnel
  branch, so http://<tunnel-host>/ regressed to unknownHost instead of the
  https redirect Allowed's own tunnel special-case implies it should get.
- Allowed() now refuses any "*."-prefixed host locally instead of resting
  on autocert's own IDN rejection of "*" to keep the certable set bounded.
- New reconcile test pins wildcard_domains.CreatedAt stability across
  repeated passes over an unchanged claim set — the skip-upsert-when-
  unchanged guard is what keeps Fingerprint (which hashes created_at for
  this table, unlike route_aliases) from forcing an edge reload every poll.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Paul Campbell 6a1107d0a660d8483e3b27a46f9843944dab857e parent 2bab40f
3 files changed, +99 −1
  • internal/domains/aliases_test.go +49 −0
  • internal/edge/alias_test.go +35 −0
  • internal/edge/edge.go +15 −1
diff --git a/internal/domains/aliases_test.go b/internal/domains/aliases_test.go
index fc96b90f1..58f39f12d 100644
--- a/internal/domains/aliases_test.go
+++ b/internal/domains/aliases_test.go
@@ -266,6 +266,55 @@ func TestReconcileAliasesIdempotent(t *testing.T) {
}
}
+// TestReconcileWildcardCreatedAtStable is the final-review pin: unlike
+// route_aliases (UpsertAlias's ON CONFLICT deliberately leaves created_at out
+// of its SET list, so the DB itself preserves it), UpsertWildcardDomain DOES
+// overwrite created_at on conflict — wildcard_domains.created_at is only kept
+// stable by the reconcile pass's own skip-upsert-when-unchanged guard (an
+// unchanged host/account/app row is never re-upserted). That guard matters
+// because Fingerprint hashes wildcard_domains.created_at (unlike
+// route_aliases, which excludes it): a created_at that bumped on every pass
+// would move the fingerprint on every 2s poll and force a needless edge
+// reload forever, even with a completely unchanged claim set.
+func TestReconcileWildcardCreatedAtStable(t *testing.T) {
+ s := release.NewDirStore(t.TempDir())
+ reg := aliasRegWith(t)
+ addRoute(t, reg, "woodstar-relay.bab.oncarlos.com", "bab", "woodstar-relay")
+ putClaim(t, s, Claim{Host: "woodstar.app", Account: "bab", App: "woodstar-relay",
+ Route: "woodstar-relay.bab.oncarlos.com", CreatedAt: "2026-08-13T00:00:00Z",
+ Wildcard: true, Catchall: true})
+
+ if err := ReconcileAliases(context.Background(), s, reg, func() time.Time { return time.Unix(1000, 0) }); err != nil {
+ t.Fatal(err)
+ }
+ first, err := reg.ListWildcardDomains()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(first) != 1 || first[0].Host != "woodstar.app" {
+ t.Fatalf("wildcard_domains after first pass = %+v, want one row for woodstar.app", first)
+ }
+ firstCreatedAt := first[0].CreatedAt
+
+ // Second pass, later clock, same claim set: CreatedAt must be preserved,
+ // not bumped — a bump here would move Fingerprint's wildcard_domains
+ // section every poll and force a reload→refresh loop with nothing
+ // actually changed.
+ if err := ReconcileAliases(context.Background(), s, reg, func() time.Time { return time.Unix(1000+3600, 0) }); err != nil {
+ t.Fatal(err)
+ }
+ second, err := reg.ListWildcardDomains()
+ if err != nil {
+ t.Fatal(err)
+ }
+ if len(second) != 1 || second[0].Host != "woodstar.app" {
+ t.Fatalf("wildcard_domains after second pass = %+v, want one row for woodstar.app", second)
+ }
+ if second[0].CreatedAt != firstCreatedAt {
+ t.Fatalf("wildcard_domains CreatedAt changed on re-run: first=%d second=%d", firstCreatedAt, second[0].CreatedAt)
+ }
+}
+
func TestReconcileDerivesWildcardState(t *testing.T) {
ctx := context.Background()
s := release.NewDirStore(t.TempDir())
diff --git a/internal/edge/alias_test.go b/internal/edge/alias_test.go
index 1b342fe3c..638fd13a3 100644
--- a/internal/edge/alias_test.go
+++ b/internal/edge/alias_test.go
@@ -431,6 +431,12 @@ func TestHostPolicyExcludesWildcardFallback(t *testing.T) {
if e.Allowed("random.woodstar.app") {
t.Fatalf("Allowed(random.woodstar.app) = true, want false — still no exact row for it")
}
+ // The wildcard label itself must never be certable — even though it is
+ // the exact map key backing the catch-all fallback, refused locally
+ // rather than resting on autocert's own IDN rejection of "*".
+ if e.Allowed("*.woodstar.app") {
+ t.Fatalf("Allowed(*.woodstar.app) = true, want false — the wildcard label itself must never be certable")
+ }
}
// TestHTTPFallbackRedirectsWildcardAlias is the round-2 review fix: the :80
@@ -483,3 +489,32 @@ func TestHTTPFallbackRedirectsWildcardAlias(t *testing.T) {
t.Fatalf("nowhere.example.com: status = %d, want 404 (unknownHost, no fallback URL)", rec.Code)
}
}
+
+// TestHTTPFallbackRedirectsTunnelHost is the final-review fix: the old inline
+// :80 closure gated on Allowed(host), which special-cases the reserved tunnel
+// hostname (see Allowed's own e.tunnels check); the extracted handler gates
+// on lookupUpstream alone, which has no tunnel branch at all, so plain
+// http://<tunnel-host>/ regressed to unknownHost instead of the classic
+// redirect into the hub's own TLS handshake on 443.
+func TestHTTPFallbackRedirectsTunnelHost(t *testing.T) {
+ e, err := New(openReg(t), nil)
+ if err != nil {
+ t.Fatalf("New: %v", err)
+ }
+ e.EnableTunnels(NewTunnelHub("tunnel.test", map[string]string{"pi-box": "sekrit"}))
+ if err := e.Reload(); err != nil {
+ t.Fatalf("reload: %v", err)
+ }
+ h := e.httpFallbackHandler()
+
+ req := httptest.NewRequest(http.MethodGet, "http://tunnel.test/register", nil)
+ req.Host = "tunnel.test"
+ rec := httptest.NewRecorder()
+ h.ServeHTTP(rec, req)
+ if rec.Code != http.StatusFound {
+ t.Fatalf("tunnel.test: status = %d, want 302 — the reserved tunnel hostname answers on 443 and must redirect like any other routable host", rec.Code)
+ }
+ if loc := rec.Header().Get("Location"); loc != "https://tunnel.test/register" {
+ t.Fatalf("tunnel.test: Location = %q, want https://tunnel.test/register", loc)
+ }
+}
diff --git a/internal/edge/edge.go b/internal/edge/edge.go
index 45c369c7d..7d94d86d1 100644
--- a/internal/edge/edge.go
+++ b/internal/edge/edge.go
@@ -536,6 +536,11 @@ func (e *Edge) currentHosts() []string {
// Allowed reports whether host is configured — used as the autocert
// HostPolicy, so a certificate is only ever requested for a host we serve.
func (e *Edge) Allowed(host string) bool {
+ // Never certable directly: refuse the wildcard label itself rather than
+ // resting on autocert's own IDN rejection of "*" to keep the bounded set.
+ if strings.HasPrefix(host, "*.") {
+ return false
+ }
// The reserved tunnel hostname is not a registry route, but remote hosts
// dial it over TLS on this same 443 listener — so it must be Allowed, and
// autocert will mint for it. That is intended: it is a hostname the
@@ -1061,7 +1066,16 @@ func (e *Edge) httpFallbackHandler() http.Handler {
if h, _, err := net.SplitHostPort(host); err == nil {
host = h
}
- if _, ok := e.lookupUpstream(host); !ok {
+ // The reserved tunnel hostname is answered by the hub itself on 443
+ // (ServeHTTP, and Allowed's same check for issuance) but is not a
+ // registry route, so lookupUpstream alone never sees it — without this,
+ // plain http://<tunnel-host>/ fell through to unknownHost instead of
+ // redirecting like every other routable host.
+ routable := e.tunnels != nil && host == e.tunnels.Host()
+ if !routable {
+ _, routable = e.lookupUpstream(host)
+ }
+ if !routable {
e.unknownHost(w, r)
return
}