From b48572bdfc73b0dac85b55d895e800a7f5958d0b Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 23 Sep 2026 23:35:29 +0200 Subject: [PATCH] A null body limit is refused, not read as no limit; the gate on the wrong machine is refused MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review of the registry hand-over. The proxy's bodyLimit treated an absent key and a JSON null alike, so a `max-request-body: null` was served unlimited here while the adapter skipped it and the catalogue refused it — one provider carrying what the others refuse. Presence is now checked before the value is read. The catalogue-backed test asserted the gate pulling the store in beside it as the feature. It was the fault: a node-scoped requirement with one candidate installs that candidate, so a gate assigned to a machine without the store raised a second, empty one there behind the real credentials and the public name. The store's seat is one per mesh now (mesh-catalog), and the test asserts the refusal by name. Delete is asserted only behind the lock. hq ADR 0082/0104, the registry hand-over. --- examples/route-proxy/main.go | 27 ++++++++------- examples/route-proxy/routes_test.go | 7 ++-- internal/catalogue/registry_gate_test.go | 44 ++++++++++++++++++------ 3 files changed, 53 insertions(+), 25 deletions(-) diff --git a/examples/route-proxy/main.go b/examples/route-proxy/main.go index e580aac..afbe3eb 100644 --- a/examples/route-proxy/main.go +++ b/examples/route-proxy/main.go @@ -387,12 +387,17 @@ func routesFrom(path string) (map[string]route, error) { // A limit it cannot honour is a route it does not serve — skipped and named, like a // port that is not one. Serving the route with no limit instead would carry exactly what // the module said not to carry, and report success. - limit, ok := bodyLimit(c.Values["max-request-body"]) - if !ok { - log.Printf("%s on %s asked for route %q with a max-request-body of %v, which is not "+ - "a whole positive number of bytes; skipped", c.From, c.Node, name, - c.Values["max-request-body"]) - continue + // Absent is no limit; present is a limit or a refusal — a `null` written where a number + // was meant is the second, not the first, and the adapter and the catalogue read it the + // same way. + var limit int64 + if raw, said := c.Values["max-request-body"]; said { + var ok bool + if limit, ok = bodyLimit(raw); !ok { + log.Printf("%s on %s asked for route %q with a max-request-body of %v, which is "+ + "not a whole positive number of bytes; skipped", c.From, c.Node, name, raw) + continue + } } // Where the mesh says that machine is. Empty means it is this one — a workload beside the // proxy is ordinary, and reaching it over loopback is both correct and the only thing @@ -409,13 +414,11 @@ func routesFrom(path string) (map[string]route, error) { return out, nil } -// bodyLimit reads a contribution's `max-request-body`: absent is no limit, and anything present -// must be a whole positive number of bytes — the same rule the control plane's catalogue applies -// when it parses the manifest, so a limit that reaches here has already passed it once. +// bodyLimit reads a contribution's `max-request-body`, which must be a whole positive number of +// bytes — the same rule the control plane's catalogue applies when it parses the manifest, so a +// limit that reaches here has already passed it once. Absence is the caller's to notice; a `null` +// arriving here is refused like any other non-number. func bodyLimit(v any) (int64, bool) { - if v == nil { - return 0, true - } var n float64 switch x := v.(type) { case float64: diff --git a/examples/route-proxy/routes_test.go b/examples/route-proxy/routes_test.go index b5b8163..25f3721 100644 --- a/examples/route-proxy/routes_test.go +++ b/examples/route-proxy/routes_test.go @@ -208,7 +208,8 @@ func TestABodyLimitIsReadFromTheContributionOrTheRouteIsSkipped(t *testing.T) { {"from":"gate","node":"anchor","values":{"name":"registry-api.example","port":5001,"max-request-body":21474836480}}, {"from":"app","node":"anchor","values":{"name":"app.example","port":8080}}, {"from":"odd","node":"anchor","values":{"name":"odd.example","port":8081,"max-request-body":"20g"}}, - {"from":"none","node":"anchor","values":{"name":"none.example","port":8082,"max-request-body":0}} + {"from":"none","node":"anchor","values":{"name":"none.example","port":8082,"max-request-body":0}}, + {"from":"nul","node":"anchor","values":{"name":"nul.example","port":8083,"max-request-body":null}} ]}`)) if err != nil { t.Fatal(err) @@ -219,7 +220,9 @@ func TestABodyLimitIsReadFromTheContributionOrTheRouteIsSkipped(t *testing.T) { if got := routes["app.example"].MaxRequestBody; got != 0 { t.Errorf("a route that asked for no limit was given one: %d", got) } - for _, skipped := range []string{"odd.example", "none.example"} { + // A `null` is not absence: the adapter skips it and the catalogue refuses it, and a proxy + // that read it as "no limit" would be the one provider carrying what the others refuse. + for _, skipped := range []string{"odd.example", "none.example", "nul.example"} { if _, served := routes[skipped]; served { t.Errorf("%s asked for a limit that is not a number of bytes and was served anyway", skipped) } diff --git a/internal/catalogue/registry_gate_test.go b/internal/catalogue/registry_gate_test.go index 3268c7d..3ed1d83 100644 --- a/internal/catalogue/registry_gate_test.go +++ b/internal/catalogue/registry_gate_test.go @@ -16,6 +16,9 @@ import ( // `distribution-gate` is a second registry process on the same volume, behind the registry's own // basic auth, with the public name. The mesh's own pulls never pass through it, which is provable // from the two manifests: the store's container carries no auth and mounts no htpasswd. +// +// And the gate can only ever stand beside THE store: the store's seat is one per mesh, so a gate +// assigned to a machine without it does not quietly raise a second, empty store there. // aStubProxy provides `route` so a declaration can be made without a built artifact: the real // providers are the adapter (whose container is a mesh-built artifact) and the proxy. @@ -39,22 +42,35 @@ func TestTheStoreNeedsNoRouteAndTheGateBringsTheStoreBesideIt(t *testing.T) { t.Fatalf("the store alone resolved to %v", got) } - // The gate wants the store's storage, which is a node-scoped provision: assigning the gate - // brings the store in beside it — the two share a volume, and a gate on a machine without the - // store would serve an empty one. And `route` resolves from the adapter exactly as from the - // proxy (ADR 0104). + // The gate wants the store's storage, which is a node-scoped provision: assigned beside the + // store it resolves, and `route` resolves from the adapter exactly as from the proxy (ADR 0104). together, err := Resolve(shelf(store, gate, adapter), - []string{"distribution-gate", "route-adapter"}, withDomain("example.test"), World{}) + []string{"distribution", "distribution-gate", "route-adapter"}, withDomain("example.test"), World{}) if err != nil { - t.Fatalf("the gate does not resolve beside the adapter: %v", err) + t.Fatalf("the gate does not resolve beside the store and the adapter: %v", err) } for _, want := range []string{"distribution", "distribution-gate", "route-adapter"} { if !among(names(together), want) { t.Errorf("%s is missing from %v", want, names(together)) } } - if because := together.Because["distribution"]; !strings.Contains(because, "distribution-gate") { - t.Errorf("the store was not pulled in by the gate: %q", because) + + // **Assigned to the wrong machine, it is refused rather than served.** A node-scoped + // requirement with one candidate installs that candidate on the node — which for the gate + // would be a second, empty store behind the real credentials and the public name, and a + // second `artifact-store` offered to the mesh so every consumer elsewhere refuses. The store's + // claim is mesh-scoped for exactly this: a second store anywhere is refused by name. + elsewhere := World{Held: []Held{{Claim: "the-artifact-store", Scope: ScopeMesh, + Node: "anchor", Module: "distribution"}}} + other := withDomain("example.test") + other.Name = "laptop" + _, err = Resolve(shelf(store, gate, adapter), []string{"distribution-gate", "route-adapter"}, other, elsewhere) + if err == nil { + t.Fatal("the gate on a machine without the store was accepted, and would have raised an " + + "empty second store behind the public name") + } + if !strings.Contains(err.Error(), "the-artifact-store") || !strings.Contains(err.Error(), "one per mesh") { + t.Fatalf("refused without naming the store's seat: %v", err) } } @@ -163,9 +179,15 @@ func TestTheGateContributesTheRegistrysPublicNameAndAGivenPortFollowsIntoIt(t *t if strings.Contains(cfg["content"].(string), "blobdescriptor") { t.Errorf("%v keeps a per-process blob cache over a store two processes write: %s", cfg["path"], cfg["content"]) } - if !strings.Contains(cfg["content"].(string), "delete:\n enabled: true") { - t.Errorf("%v lost the predecessor's delete setting, which tag retention depends on", cfg["path"]) - } + } + // Delete is the predecessor's setting and tag retention depends on it — but only behind the + // lock. The store's door is reached by the whole private network with no account (ADR 0082), + // and a delete anything on the overlay may send is not a setting to carry there. + if !strings.Contains(gateConfig["content"].(string), "delete:\n enabled: true") { + t.Errorf("the gate lost the predecessor's delete setting, which tag retention depends on") + } + if strings.Contains(storeConfig["content"].(string), "delete:\n enabled: true") { + t.Errorf("the account-free store accepts DELETE from anything on the overlay: %s", storeConfig["content"]) } // And the machine published the gate where the node said. if ports, _ := gateContainer["ports"].([]any); len(ports) != 1 || ports[0] != "5101:5001" {