From e11e1374bd37df6035d14e3c4a79cc83eff430bd Mon Sep 17 00:00:00 2001 From: jochen Date: Fri, 25 Sep 2026 14:20:06 +0200 Subject: [PATCH] route-proxy: a priority is an ordering, not a port MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found reviewing my own change before merging it, and it was load-bearing rather than cosmetic. Priority was read with asPort, which caps at 65535. A rule declared above that silently became priority 0 and stopped shadowing the route it exists to shadow. The one real rule this has to reproduce is declared at 100000 — so path scoping and refusal would both have shipped looking complete, passing their tests, and doing nothing on the only case that motivated them. A priority is an ordering and has no range. asWhole takes any whole number the mesh wrote and rejects a non-integral one, which was not meant as a priority. Also: a host may now be routed on some paths and not others, which made the 404 dishonest — it said "no route for this name" while listing that very name as served, a contradiction an operator has to disbelieve the proxy to get past. An uncovered path now says so, and a name that is genuinely not served still lists what is. Two regression tests, both through the proxy rather than against the parser, because the parser was where the bug looked fine. --- examples/route-proxy/main.go | 31 +++++++++++++++++++++- examples/route-proxy/policy_test.go | 40 +++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 1 deletion(-) diff --git a/examples/route-proxy/main.go b/examples/route-proxy/main.go index 10d7217..9b59643 100644 --- a/examples/route-proxy/main.go +++ b/examples/route-proxy/main.go @@ -416,8 +416,17 @@ func handler(held *table) http.Handler { // **Named, not a bare 404.** A route that was withdrawn and a name that never existed // are different things, and a proxy that says only "not found" makes an operator go // and read the mesh to tell them apart. What it is serving is the answer to both. + // + // And since a host may now be routed only on some paths, those are a third thing: + // saying "no route for this name" while listing that very name as served is a + // contradiction an operator would have to disbelieve the proxy to get past. w.Header().Set("Content-Type", "text/plain; charset=utf-8") w.WriteHeader(http.StatusNotFound) + if held.routed(r.Host) { + fmt.Fprintf(w, "%s is served here, but no route covers %q.\n", + bareHost(r.Host), r.URL.Path) + return + } fmt.Fprintf(w, "no route for %q in this mesh.\nserving: %s\n", r.Host, strings.Join(held.names(), ", ")) return @@ -524,7 +533,7 @@ func routesFrom(path string) (map[string][]rule, error) { host := strings.ToLower(name) made := rule{path: asPath(c.Values["path"])} - if p, ok := asPort(c.Values["priority"]); ok { + if p, ok := asWhole(c.Values["priority"]); ok { made.priority = p } made.policy.deny, _ = c.Values["deny"].(bool) @@ -574,6 +583,26 @@ func routesFrom(path string) (map[string][]rule, error) { return out, nil } +// asWhole is any whole number the mesh wrote, whatever its magnitude. +// +// **Not asPort.** Priority was read with the port reader first, which caps at 65535 — so a rule +// declared at a priority above that silently became priority 0 and stopped shadowing the route it +// exists to shadow. The one real rule this has to reproduce is declared at 100000, so the bug was +// exactly load-bearing. A priority is an ordering, not a port: it has no range. +func asWhole(v any) (int, bool) { + switch n := v.(type) { + case float64: + // JSON makes a float of every number, so a non-integral one was not meant as a priority. + if n != float64(int(n)) { + return 0, false + } + return int(n), true + case int: + return n, true + } + return 0, false +} + // asPath is the path prefix a rule is scoped to, or "" for every path. func asPath(v any) string { p, _ := v.(string) diff --git a/examples/route-proxy/policy_test.go b/examples/route-proxy/policy_test.go index 7f98ec4..db1dc0b 100644 --- a/examples/route-proxy/policy_test.go +++ b/examples/route-proxy/policy_test.go @@ -231,3 +231,43 @@ func TestPriorityOutranksPathLength(t *testing.T) { t.Fatalf("a higher priority did not win: %+v", rules[0]) } } + +// A priority above a port number survives, because a priority is an ordering and not a port. +// +// **Found by review, and it was load-bearing.** Priority was first read with the port reader, which +// caps at 65535 — so a rule declared above that silently became priority 0 and stopped shadowing the +// route it exists to shadow. The one real rule this has to reproduce is declared at 100000, so the +// capability would have shipped looking complete and doing nothing. +func TestAPriorityAboveAPortNumberSurvives(t *testing.T) { + at, port := served(t, "the workload") + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","at":"`+at+`","values":{"name":"forge.example","port":`+strconv.Itoa(port)+`}}, + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","priority":100000,"deny":true}} + ]}`) + + if got := ask(t, proxy, "forge.example", "/api/internal/hook", [2]string{}).StatusCode; got != http.StatusForbidden { + t.Fatalf("a rule declared at priority 100000 answered %d instead of refusing", got) + } +} + +// A host routed only on some paths says so, rather than claiming the name is not served here. +// +// Saying "no route for this name" while listing that very name as served is a contradiction an +// operator has to disbelieve the proxy to get past — and path scoping makes it reachable, because a +// host can now have rules that none of this request's paths match. +func TestAHostRoutedOnlyOnSomePathsSaysSo(t *testing.T) { + proxy := proxyFor(t, `{"given":[ + {"from":"forge","node":"anchor","values":{"name":"forge.example","path":"/api/internal","deny":true}} + ]}`) + + answer := ask(t, proxy, "forge.example", "/elsewhere", [2]string{}) + if answer.StatusCode != http.StatusNotFound { + t.Fatalf("an uncovered path answered %d", answer.StatusCode) + } + body := make([]byte, 256) + n, _ := answer.Body.Read(body) + said := string(body[:n]) + if !strings.Contains(said, "is served here") || !strings.Contains(said, "/elsewhere") { + t.Fatalf("the refusal does not distinguish an uncovered path from an unserved name: %q", said) + } +}