amadan

carlos / platform Public

provision: close Backing's siblings and pass-ownership gaps in shapeMatches

Review fix (2026-08-10, Task 4 re-review). Amendment 2 named only Backing
as the box-scoped column a record can never state; Backing's own doc
comment names Tunnel and Caps as its siblings, and both were live gaps:

- Tunnel outranks the Socket a record derives (internal/edge picks the
  tunnel proxy before ever looking at Socket), so a tunnel-backed route
  could match every compared column and get adopted with its traffic still
  going down a channel the record can't see or reclaim.
- Caps is the operator capability-grant column; reg.Delete's purge of it is
  already treated as human-gated/last-resort elsewhere in this file, and
  adoption would have handed that purge to an unattended record deletion.

Also closes two Important findings: Platform/Canary rows (domains.Reconcile's
own removal-authority flags, exploitable via mintedRoute's empty
Slug/Pool/Region/DBPath) are now excluded from adoption too, and the
refusal log — the one line an operator backfilling the fleet actually sees
on a refusal — now names the first column that actually differed instead
of three columns that are identical on every non-obvious refusal.

shapeMatches is now shapeMismatch(...) == "", so the match predicate and
the diagnostic message can never drift apart. Added refusal tests for
Tunnel, Caps, Platform, Canary, and a DBPath-only mismatch (the latter
closing a gap where deleting the DBPath comparison would have left the
suite green).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Paul Campbell e6a1a3abad44833991f5b0f8facd2dc62f351978 parent f764983
2 files changed, +236 −22
  • internal/provision/reconcile.go +77 −22
  • internal/provision/reconcile_test.go +159 −0
diff --git a/internal/provision/reconcile.go b/internal/provision/reconcile.go
index 66e93c12b..3c564fd16 100644
--- a/internal/provision/reconcile.go
+++ b/internal/provision/reconcile.go
@@ -79,30 +79,86 @@ func routeFor(rec Record, dataDir, seed string) registry.Route {
// wrong channel is the whole reason to adopt one. Requiring them to agree
// would make adoption impossible in exactly the case it exists for.
//
-// Backing is deliberately required to be empty on the existing row. A
-// record has no field for Backing — it is a box-scoped operator column,
-// alongside capability grants and tunnels (registry.Route.Backing's own
-// doc comment) — so a record can never describe a carlos-app@<host> unit
-// tenant at all. A row this pass cannot state the Backing of is a row this
-// pass must not claim to describe, no matter how well everything else
-// lines up.
+// Backing, Tunnel and Caps are deliberately required to be empty (unset) on
+// the existing row — registry.Route's own doc comments name these three as
+// the box-scoped operator columns, and a record has a field for none of
+// them, so a record can never fully describe a row that carries one:
+// - Backing "unit" names a carlos-app@<host> systemd tenant; adopting
+// across it would claim a process shape the record says nothing about.
+// - Tunnel is routing-decisive and OUTRANKS the Socket/Addr a record can
+// state: internal/edge picks the tunnel proxy before it ever looks at
+// Socket, so a tunnel-backed route can agree with a record on every
+// other compared column while every byte it actually serves goes down
+// a channel the record cannot see or reclaim.
+// - Caps is the operator-granted, box-local capability grant column
+// (port:25, host-role, ...). registry.Delete's purge of a route's
+// grants is already treated as a human-gated, last-resort action
+// elsewhere in this file (the shape-drift log below); adoption must not
+// hand that purge to an unattended bucket-record deletion.
+//
+// A row this pass cannot state the shape of is a row this pass must not
+// claim to describe, no matter how well everything else lines up.
+//
+// Platform and Canary are likewise required false: those flags mark a row
+// domains.Reconcile minted and re-mints/removes on its own authority
+// (mintedRoute leaves Slug/Pool/Region/DBPath at their zero values, so a
+// platform apex on an unconstrained-region box can otherwise agree with a
+// record on every other compared column). Adoption must not hand this
+// pass's removal authority to a row another pass already owns and
+// periodically rebuilds — nor let it write Channel/Environment onto a row
+// the CLI itself refuses (platformCanaryOwnedRow, cli.go) for the same
+// reason.
//
// Everything else that identifies the route is compared, including the
// derived Socket and DBPath — two rows agreeing on kind and app but not on
// where the database lives are not the same route, and adopting across that
// difference would point a running process at a second copy of its own data.
func shapeMatches(existing, want registry.Route) bool {
- return existing.Backing == "" &&
- existing.App == want.App &&
- existing.Account == want.Account &&
- existing.Kind == want.Kind &&
- existing.Addr == want.Addr &&
- existing.Unit == want.Unit &&
- existing.Slug == want.Slug &&
- existing.Pool == want.Pool &&
- existing.Region == want.Region &&
- existing.Socket == want.Socket &&
- existing.DBPath == want.DBPath
+ return shapeMismatch(existing, want) == ""
+}
+
+// shapeMismatch names the first column on which existing disagrees with
+// want — including the box-scoped and pass-ownership guards shapeMatches
+// documents — or "" when every compared column agrees. It backs both
+// shapeMatches and the adoption refusal log: printing only a couple of
+// columns there (the original message did app/kind/addr) leaves every
+// refusal over any OTHER column looking identical and undiagnosable to the
+// operator the refusal message exists for.
+func shapeMismatch(existing, want registry.Route) string {
+ switch {
+ case existing.Backing != "":
+ return fmt.Sprintf("backing %q (box-scoped, not expressible in a record)", existing.Backing)
+ case existing.Tunnel != "":
+ return fmt.Sprintf("tunnel %q (box-scoped, not expressible in a record)", existing.Tunnel)
+ case existing.Caps != "":
+ return fmt.Sprintf("caps %q (box-scoped, not expressible in a record)", existing.Caps)
+ case existing.Platform:
+ return "platform (owned by the domains reconcile pass)"
+ case existing.Canary:
+ return "canary (owned by the domains reconcile pass)"
+ case existing.App != want.App:
+ return fmt.Sprintf("app %q/%q", existing.App, want.App)
+ case existing.Account != want.Account:
+ return fmt.Sprintf("account %q/%q", existing.Account, want.Account)
+ case existing.Kind != want.Kind:
+ return fmt.Sprintf("kind %q/%q", existing.Kind, want.Kind)
+ case existing.Addr != want.Addr:
+ return fmt.Sprintf("addr %q/%q", existing.Addr, want.Addr)
+ case existing.Unit != want.Unit:
+ return fmt.Sprintf("unit %q/%q", existing.Unit, want.Unit)
+ case existing.Slug != want.Slug:
+ return fmt.Sprintf("slug %q/%q", existing.Slug, want.Slug)
+ case existing.Pool != want.Pool:
+ return fmt.Sprintf("pool %q/%q", existing.Pool, want.Pool)
+ case existing.Region != want.Region:
+ return fmt.Sprintf("region %q/%q", existing.Region, want.Region)
+ case existing.Socket != want.Socket:
+ return fmt.Sprintf("socket %q/%q", existing.Socket, want.Socket)
+ case existing.DBPath != want.DBPath:
+ return fmt.Sprintf("db %q/%q", existing.DBPath, want.DBPath)
+ default:
+ return ""
+ }
}
// convergeOwned applies the record's Channel and Environment onto a route it
@@ -382,10 +438,9 @@ func Reconcile(ctx context.Context, s release.Store, reg *registry.Registry, cfg
// over the row, so the match test is the only thing standing
// between a backfill and a retired production route.
want := routeFor(rec, cfg.DataDir, seed)
- if !shapeMatches(existing, want) {
- log.Printf("provision: %s exists but is not the route this record describes "+
- "(app %q/%q kind %q/%q addr %q/%q) — leaving it alone",
- rec.Host, existing.App, want.App, existing.Kind, want.Kind, existing.Addr, want.Addr)
+ if reason := shapeMismatch(existing, want); reason != "" {
+ log.Printf("provision: %s exists but is not the route this record describes (%s) — leaving it alone",
+ rec.Host, reason)
continue
}
if err := reg.SetProvisioned(rec.Host, true); err != nil {
diff --git a/internal/provision/reconcile_test.go b/internal/provision/reconcile_test.go
index ef6f30117..9af1a31c7 100644
--- a/internal/provision/reconcile_test.go
+++ b/internal/provision/reconcile_test.go
@@ -1039,6 +1039,165 @@ func TestReconcileRefusesToAdoptBackedRoute(t *testing.T) {
}
}
+// Tunnel is routing-decisive and OUTRANKS the Socket/Addr a record can
+// state: internal/edge picks the tunnel proxy before it ever looks at
+// Socket (edge.go's route-selection switch), so a tunnel-backed route can
+// otherwise agree with a record on every other compared column while every
+// byte it actually serves goes down a channel the record cannot see or
+// reclaim (2026-08-10 review, Critical).
+func TestReconcileRefusesToAdoptTunnelBoundRoute(t *testing.T) {
+ s, sqid := setup(t)
+ if err := CreateRecord(ctx, s, acmeRecord(sqid)); err != nil {
+ t.Fatal(err)
+ }
+ handAdded := registry.Route{
+ Host: "acme.go.tito.io", Slug: "acme", Kind: "instance",
+ Socket: "/run/carlos/acme.go.tito.io.sock", DBPath: "/data/acme.go.tito.io.db",
+ App: "tito", Account: sqid, Pool: "tito", Region: "ie",
+ Channel: "stable", Hibernate: true, Status: "ready",
+ Tunnel: "pi-fleet",
+ }
+ reg := regWith(t, handAdded)
+ before := *mustGet(t, reg, "acme.go.tito.io")
+ cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: "/data"}
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ after := *mustGet(t, reg, "acme.go.tito.io")
+ if after != before {
+ t.Errorf("a tunnel-bound route was touched: before %+v after %+v", before, after)
+ }
+ if after.Provisioned {
+ t.Error("a tunnel-bound route was adopted — a record cannot state or reclaim its Tunnel binding")
+ }
+}
+
+// Caps is the operator-granted, box-local capability-grant column, and
+// registry.Delete's purge of a route's grants is already a human-gated,
+// last-resort action elsewhere in this file (the shape-drift log). Adoption
+// must not hand that purge to an unattended bucket-record deletion
+// (2026-08-10 review, Critical).
+func TestReconcileRefusesToAdoptCapsBearingRoute(t *testing.T) {
+ s, sqid := setup(t)
+ if err := CreateRecord(ctx, s, acmeRecord(sqid)); err != nil {
+ t.Fatal(err)
+ }
+ handAdded := registry.Route{
+ Host: "acme.go.tito.io", Slug: "acme", Kind: "instance",
+ Socket: "/run/carlos/acme.go.tito.io.sock", DBPath: "/data/acme.go.tito.io.db",
+ App: "tito", Account: sqid, Pool: "tito", Region: "ie",
+ Channel: "stable", Hibernate: true, Status: "ready",
+ Caps: "port:25",
+ }
+ reg := regWith(t, handAdded)
+ before := *mustGet(t, reg, "acme.go.tito.io")
+ cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: "/data"}
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ after := *mustGet(t, reg, "acme.go.tito.io")
+ if after != before {
+ t.Errorf("a caps-bearing route was touched: before %+v after %+v", before, after)
+ }
+ if after.Provisioned {
+ t.Error("a caps-bearing route was adopted — its grants would now be one record deletion from purged")
+ }
+}
+
+// domains.mintedRoute builds platform routes with empty Slug/Pool/Region/
+// DBPath, copying Kind/Socket/Addr from the source route — so a record
+// whose Slug/Pool/Region are also empty (an unconstrained-region/pool box)
+// can otherwise agree with a platform-minted row on every other compared
+// column. Adoption must not hand this pass's removal authority to a row
+// domains.Reconcile already owns and periodically re-Upserts/removes on its
+// own authority (2026-08-10 review, Important 1).
+func TestReconcileRefusesToAdoptPlatformRoute(t *testing.T) {
+ s, sqid := setup(t)
+ rec := Record{Schema: 1, Host: "acme.go.tito.io", App: "tito", Account: sqid,
+ Channel: "stable", CreatedAt: time.Now()}
+ if err := CreateRecord(ctx, s, rec); err != nil {
+ t.Fatal(err)
+ }
+ handAdded := registry.Route{
+ Host: "acme.go.tito.io", Kind: "instance",
+ Socket: "/run/carlos/acme.go.tito.io.sock", DBPath: "/data/acme.go.tito.io.db",
+ App: "tito", Account: sqid, Status: "ready", Platform: true,
+ }
+ reg := regWith(t, handAdded)
+ before := *mustGet(t, reg, "acme.go.tito.io")
+ cfg := ReconcileConfig{DataDir: "/data"} // unconstrained region, default pool only
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ after := *mustGet(t, reg, "acme.go.tito.io")
+ if after != before {
+ t.Errorf("a platform-owned route was touched: before %+v after %+v", before, after)
+ }
+ if after.Provisioned {
+ t.Error("a platform-owned route was adopted — removal authority granted to a second pass")
+ }
+}
+
+// Same guard, the other flag domains.Reconcile also uses for its own
+// removal authority (2026-08-10 review, Important 1).
+func TestReconcileRefusesToAdoptCanaryRoute(t *testing.T) {
+ s, sqid := setup(t)
+ rec := Record{Schema: 1, Host: "acme.go.tito.io", App: "tito", Account: sqid,
+ Channel: "stable", CreatedAt: time.Now()}
+ if err := CreateRecord(ctx, s, rec); err != nil {
+ t.Fatal(err)
+ }
+ handAdded := registry.Route{
+ Host: "acme.go.tito.io", Kind: "instance",
+ Socket: "/run/carlos/acme.go.tito.io.sock", DBPath: "/data/acme.go.tito.io.db",
+ App: "tito", Account: sqid, Status: "ready", Canary: true,
+ }
+ reg := regWith(t, handAdded)
+ before := *mustGet(t, reg, "acme.go.tito.io")
+ cfg := ReconcileConfig{DataDir: "/data"}
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ after := *mustGet(t, reg, "acme.go.tito.io")
+ if after != before {
+ t.Errorf("a canary-owned route was touched: before %+v after %+v", before, after)
+ }
+ if after.Provisioned {
+ t.Error("a canary-owned route was adopted — removal authority granted to a second pass")
+ }
+}
+
+// Nothing currently locked the derived Socket/DBPath comparison specifically
+// — the only prior mismatch test differed on App, so a regression that
+// dropped the DBPath comparison from shapeMismatch would leave the suite
+// green (2026-08-10 review, test gap called out alongside the Important
+// findings).
+func TestReconcileRefusesToAdoptOnDBPathMismatch(t *testing.T) {
+ s, sqid := setup(t)
+ if err := CreateRecord(ctx, s, acmeRecord(sqid)); err != nil {
+ t.Fatal(err)
+ }
+ handAdded := registry.Route{
+ Host: "acme.go.tito.io", Slug: "acme", Kind: "instance",
+ Socket: "/run/carlos/acme.go.tito.io.sock", DBPath: "/data/other.db",
+ App: "tito", Account: sqid, Pool: "tito", Region: "ie",
+ Channel: "stable", Hibernate: true, Status: "ready",
+ }
+ reg := regWith(t, handAdded)
+ before := *mustGet(t, reg, "acme.go.tito.io")
+ cfg := ReconcileConfig{Region: "ie", Pools: []string{"tito"}, DataDir: "/data"}
+ if err := Reconcile(ctx, s, reg, cfg); err != nil {
+ t.Fatal(err)
+ }
+ after := *mustGet(t, reg, "acme.go.tito.io")
+ if after != before {
+ t.Errorf("a DBPath-mismatched route was touched: before %+v after %+v", before, after)
+ }
+ if after.Provisioned {
+ t.Error("a route was adopted despite a DBPath mismatch")
+ }
+}
+
// An adopted route is removable by the pass that adopted it, on the same
// terms as one it minted: the record disappearing is the bucket's own
// decision to retire the instance.