A grant secret belongs to whoever provisions, and the sweep skips what it will not address

Issue 225. The mesh seals one credential per consumer beside the provider's
contributions file, and wrote it root-owned. That was right while a module's
own code ran in a container as root; ADR 0198 moved that code under the node's
runtime, as the node's account, and the secret stayed root's. On the control
machine two consumers went unprovisioned for three hours and the only sign
was a line reading 'secret not readable yet', 4330 times.

The same sentence is already written for a module's own secrets a few hundred
lines above — 'a root-owned 0600 file is one that process cannot read'. This
is that rule reaching the other kind of secret the mesh writes for a module.

Issue 226. The sweep met a reference recorded with the store's old address,
read 'I will not address this' as 'the store refuses everything', and
collected none of the 1681 it had found. Two changes: references from build
records are read through Recorded, where the provenance is known — not in
LetGo, which cannot tell one registry host from another and must stay strict
— and a reference the sweep will not address is now ErrNotOurs, skipped,
never a reason to stop. Only the store refusing ends a sweep.

make check: the two failures both fail on main as well — the resolver test
(hq 202/203) and the service-manager test, which reads this machine's own
shell environment.

cmd/mesh-controller/collect.go

@@ -60,7 +60,7 @@ func collect(ctx context.Context, inv *inventory.Inventory) {
 	store := artifacts.Store{Address: address}
 
 	var done []string
-	var left int
+	var left, skipped int
 	for i, reference := range references {
 		if i >= mostPerSweep || within.Err() != nil {
 			left = len(references) - i
@@ -73,10 +73,22 @@ func collect(ctx context.Context, inv *inventory.Inventory) {
 			done = append(done, reference)
 			continue
 		}
-		// **Stopped at the first refusal, not pushed through.** A store that refuses one refuses
-		// all of them — deletion disabled, the store down, the network gone — so going on would
-		// be a hundred identical failures and a hundred identical log lines in front of whoever
-		// was building something.
+		if errors.Is(err, artifacts.ErrNotOurs) {
+			// **A fact about this record, so this record is skipped** (novox/hq issue 226). Not
+			// marked collected — the mesh did not remove it and should not claim to — and not a
+			// reason to stop, because the store was never asked. One of these at the front of
+			// the oldest-first order ended every sweep until this.
+			skipped++
+			if skipped == 1 {
+				fmt.Fprintf(os.Stderr,
+					"the sweep will not address %s and went on: %v\n", reference, err)
+			}
+			continue
+		}
+		// **Stopped at the first refusal by the STORE, not pushed through.** A store that refuses
+		// one refuses all of them — deletion disabled, the store down, the network gone — so
+		// going on would be a hundred identical failures and a hundred identical log lines in
+		// front of whoever was building something.
 		fmt.Fprintf(os.Stderr, "the artifact store kept %s, so nothing more was asked of it: %v\n",
 			reference, err)
 		left = len(references) - i
@@ -97,6 +109,9 @@ func collect(ctx context.Context, inv *inventory.Inventory) {
 	if left > 0 {
 		fmt.Fprintf(os.Stderr, "%d more to collect; the next build asks again\n", left)
 	}
+	if skipped > 0 {
+		fmt.Fprintf(os.Stderr, "%d artifact(s) the sweep will not address were skipped\n", skipped)
+	}
 }
 
 // mostPerSweep is how many artifacts one sweep will ask about. Enough that a mesh building

internal/artifacts/store.go

@@ -8,6 +8,7 @@ package artifacts
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"strings"
@@ -26,7 +27,15 @@ type Store struct {
 }
 
 // Gone is the answer when the store does not hold it: the outcome wanted, already true.
-var Gone = fmt.Errorf("the store does not hold it")
+var Gone = errors.New("the store does not hold it")
+
+// ErrNotOurs is a reference this sweep will not address: not the mesh's own, or naming nothing
+// the store holds by digest.
+//
+// **A fact about the record, not about the store** (novox/hq issue 226). The two deserve opposite
+// responses — skip one and go on, abandon the sweep for the other — and collapsing them into "an
+// error" is how a cautious loop became one that did nothing while reporting the right number.
+var ErrNotOurs = errors.New("not a reference into the mesh's artifact store")
 
 // LetGo asks the store to drop one artifact the mesh recorded making.
 //
@@ -38,12 +47,17 @@ var Gone = fmt.Errorf("the store does not hold it")
 // wants the artifact absent, and it is. It is distinguished from success only so a caller can say
 // which of the two happened.
 func (s Store) LetGo(ctx context.Context, reference string) error {
+	// **Strict, and deliberately** (novox/hq issue 226). Only a reference the mesh keeps in its
+	// own vocabulary is addressed here. `Recorded` would read `docker.io/library/registry@sha256:…`
+	// as the mesh's too — it cannot tell one registry host from another — so normalising belongs
+	// where the provenance is known, which is the sweep reading its own build records, not here
+	// where the only job is to refuse anything that is not plainly ours.
 	path, kept := catalogue.InArtifactStore(reference)
 	if !kept {
 		// Nothing the mesh put in its own store. Refused rather than attempted: composing a
 		// delete for a reference of unknown shape is how a sweep reaches something that is not
-		// the mesh's.
-		return fmt.Errorf("%s is not a reference into the mesh's artifact store", reference)
+		// the mesh's. Distinguished from a store that refuses, so a sweep skips this and goes on.
+		return fmt.Errorf("%w: %s", ErrNotOurs, reference)
 	}
 	if s.Address == "" {
 		return fmt.Errorf("this mesh has no artifact store on its network to ask about %s", reference)
@@ -95,5 +109,5 @@ func split(path string) (repository, kind, digest string, err error) {
 	if before, after, ok := strings.Cut(path, "/blobs/sha256:"); ok {
 		return before, "blobs", "sha256:" + after, nil
 	}
-	return "", "", "", fmt.Errorf("%q names nothing the store holds by digest", path)
+	return "", "", "", fmt.Errorf("%w: %q names nothing the store holds by digest", ErrNotOurs, path)
 }

internal/artifacts/store_test.go

@@ -92,3 +92,26 @@ func TestAReferenceThatIsNotTheMeshsOwnIsNeverAsked(t *testing.T) {
 		t.Fatalf("the store was asked about %v", *asked)
 	}
 }
+
+// A reference this sweep will not address says so as ErrNotOurs, which is a fact about the
+// record and not about the store (novox/hq issue 226).
+//
+// The sweep skips one and abandons itself for the other, so they cannot be the same error. The
+// first live run met a reference recorded with the store's old address, read the refusal as "the
+// store refuses everything", and collected none of the 1681 it had found.
+func TestAReferenceThisSweepWillNotAddressIsToldApartFromAStoreRefusing(t *testing.T) {
+	store, asked := fakeStore(t, http.StatusAccepted)
+	for _, reference := range []string{
+		"docker.io/library/registry@sha256:abc123",
+		"127.0.0.1:5100/mesh-tools/build@sha256:abc123",
+		"1.4.2",
+	} {
+		err := store.LetGo(context.Background(), reference)
+		if !errors.Is(err, ErrNotOurs) {
+			t.Errorf("%s answered %v; a sweep must be able to skip it and go on", reference, err)
+		}
+	}
+	if len(*asked) != 0 {
+		t.Fatalf("the store was asked about %v", *asked)
+	}
+}

internal/catalogue/declaration.go

@@ -610,14 +610,14 @@ func (r Resolution) compose(with Rendering, owner map[string]string,
 					// and nothing would say so.
 					continue
 				}
-				first = append(first, map[string]any{
+				first = append(first, ownedBy(r.provisionsAs(m), map[string]any{
 					// One file per holder — the consumer's module with its local name after it
 					// where it keeps several (ADR 0094); the lab found two files with one id.
 					"id":     GrantID(to, g.Consumer+"."+holderAs(g.From, g.Local)),
 					"type":   "file",
 					"path":   grantPath(m.Grants[to], g.Consumer, holderAs(g.From, g.Local)),
 					"sealed": g.Sealed,
-				})
+				}))
 			}
 		}
 		for _, to := range sortedKeys(m.Binds) {
@@ -1240,6 +1240,28 @@ type Contribution struct {
 	Derived map[string]any `json:"derived,omitempty"`
 }
 
+// provisionsAs is the account that reads what the mesh writes for this provider: the one secret
+// per consumer it must open to set that consumer's password (novox/hq issue 225).
+//
+// **A root-owned 0600 file is one that process cannot read**, which is the same sentence already
+// written above for a module's own secrets — and the grant secret is the other kind of secret
+// the mesh writes for a module, so it is the same rule.
+//
+// Which account depends on where the module's code runs. A module whose code is a bundle is run
+// by the node's tool runtime, as the node's account ([ADR 0198](0198)); one still in a container
+// is whatever it declares as its secrets owner. Nothing names these paths, so the rule that
+// claims a bundle's other files by the words that name them (givenTo) cannot reach them: the
+// harness composes a grant secret's path from the contributions file, not from a word.
+//
+// Empty is root, which is what it was and what a module with no bundle and no declared owner
+// still wants.
+func (r Resolution) provisionsAs(m Manifest) string {
+	if len(m.Bundles) > 0 && r.Account != "" {
+		return r.Account
+	}
+	return m.SecretsOwner
+}
+
 // grantPath is where one consumer's sealed credential lands on the providing machine.
 //
 // Suffixed, so the directory can also hold whatever the module writing it keeps there and so a

internal/catalogue/grant_secret_owner_test.go

@@ -0,0 +1,103 @@
+package catalogue
+
+import (
+	"strings"
+	"testing"
+)
+
+// A grant secret is read by whatever provisions, and that stopped being root (novox/hq issue 225).
+//
+// The mesh seals one credential per consumer beside the provider's contributions file. The
+// provider's harness reads both: the file to learn who asked, the secret to set their password.
+// While a module's own code ran in a container as root, a root-owned 0600 file was readable by
+// the thing that needed it. ADR 0198 moved that code under the node's runtime, which runs as the
+// operator's account — and the secret stayed root's.
+//
+// **The cost was silence.** The harness says `secret not readable yet`, which is true and
+// ordinary on the first pass, so four thousand refusals in three hours read as patience. No user
+// was ever created, and two consumers crash-looped against a database that had never heard of
+// them.
+//
+// The same reasoning is already written for a module's *own* secrets, three hundred lines above:
+// "a root-owned 0600 file is one that process cannot read". This is that rule reaching the other
+// kind of secret the mesh writes for a module.
+
+// aProviderWithABundle is a provider whose code is a bundle the node's runtime runs — the shape
+// every TypeScript provisioner has since ADR 0198.
+func aProviderWithABundle() Manifest {
+	return Manifest{
+		Module: "mongodb", Version: "1",
+		Provides: FromAnywhere("mongodb-database"),
+		Receives: map[string]string{"mongodb-database": "/var/lib/mongodb/grants/mesh.json"},
+		Grants:   map[string]string{"mongodb-database": "/var/lib/mongodb/grants"},
+		Bundles:  []Bundle{{Name: "code", Language: "typescript"}},
+		Resources: []map[string]any{{
+			"id": "server", "type": "container", "name": "mongodb-server",
+			"image": "mongo@sha256:" + strings.Repeat("a", 64),
+		}},
+	}
+}
+
+func TestAGrantSecretIsOwnedByTheAccountThatProvisions(t *testing.T) {
+	r, err := Resolve(shelf(aProviderWithABundle()), []string{"mongodb"}, reachable(), World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	r.Account = "operator"
+	out, err := r.Declaration(Rendering{Grants: []Grant{{
+		Provision: "mongodb-database", Consumer: "workstation", From: "photos", Slug: "photos",
+		Values: map[string]any{}, Sealed: "c2VhbGVk",
+	}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	var secret map[string]any
+	for _, res := range out {
+		if res["type"] == "file" && strings.HasSuffix(fmtPath(res), ".secret") {
+			secret = res
+		}
+	}
+	if secret == nil {
+		t.Fatalf("no grant secret was composed at all: %v", out)
+	}
+	if got := secret["owner"]; got != "operator" {
+		t.Fatalf("the grant secret at %v belongs to %v; the provisioner runs as %q and a "+
+			"root-owned 0600 file is one it cannot read — which is silent, because the harness "+
+			"calls it \"not readable yet\"", fmtPath(secret), got, "operator")
+	}
+}
+
+// And a provider whose code still runs in a container keeps the owner it declares, so this
+// changes nothing for the modules the runtime has not taken.
+func TestAContainerProvidersGrantSecretKeepsItsDeclaredOwner(t *testing.T) {
+	m := aProviderWithABundle()
+	m.Bundles = nil
+	m.SecretsOwner = "65534:65534"
+	r, err := Resolve(shelf(m), []string{"mongodb"}, reachable(), World{})
+	if err != nil {
+		t.Fatal(err)
+	}
+	r.Account = "operator"
+	out, err := r.Declaration(Rendering{Grants: []Grant{{
+		Provision: "mongodb-database", Consumer: "workstation", From: "photos", Slug: "photos",
+		Values: map[string]any{}, Sealed: "c2VhbGVk",
+	}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	for _, res := range out {
+		if res["type"] == "file" && strings.HasSuffix(fmtPath(res), ".secret") {
+			if got := res["owner"]; got != "65534:65534" {
+				t.Fatalf("a container provider's grant secret belongs to %v, not what it declares", got)
+			}
+			return
+		}
+	}
+	t.Fatal("no grant secret was composed")
+}
+
+func fmtPath(r map[string]any) string {
+	p, _ := r["path"].(string)
+	return p
+}

internal/inventory/collection.go

@@ -4,6 +4,8 @@ import (
 	"context"
 	"encoding/json"
 	"strings"
+
+	"github.com/novox/mesh-controller/internal/catalogue"
 )
 
 // What the artifact store keeps, and what it may let go (novox/hq ADR 0189, issue 108).
@@ -71,11 +73,12 @@ func (i *Inventory) ToCollect(ctx context.Context) ([]string, error) {
 			continue
 		}
 		for _, a := range made {
-			if a.Reference == "" || keep[a.Reference] || collected[a.Reference] || seen[a.Reference] {
+			reference := asRecorded(a.Reference)
+			if reference == "" || keep[reference] || collected[reference] || seen[reference] {
 				continue
 			}
-			seen[a.Reference] = true
-			out = append(out, a.Reference)
+			seen[reference] = true
+			out = append(out, reference)
 		}
 	}
 	return out, rows.Err()
@@ -127,8 +130,8 @@ func (i *Inventory) keptReferences(ctx context.Context) (map[string]bool, error)
 			continue
 		}
 		for _, a := range made {
-			if a.Reference != "" {
-				keep[a.Reference] = true
+			if reference := asRecorded(a.Reference); reference != "" {
+				keep[reference] = true
 			}
 		}
 	}
@@ -186,16 +189,35 @@ func (i *Inventory) everyReferenceMade(ctx context.Context) ([]string, error) {
 			continue
 		}
 		for _, a := range made {
-			if a.Reference == "" || seen[a.Reference] {
+			reference := asRecorded(a.Reference)
+			if reference == "" || seen[reference] {
 				continue
 			}
-			seen[a.Reference] = true
-			out = append(out, a.Reference)
+			seen[reference] = true
+			out = append(out, reference)
 		}
 	}
 	return out, rows.Err()
 }
 
+// asRecorded is an artifact reference in the one vocabulary the sweep speaks (novox/hq issue 226).
+//
+// **Every reference here came from a build record, so every one of them is the mesh's own.** That
+// is what makes it safe to normalise: references kept before the store's address stopped being
+// written are `<host>:<port>/<path>@sha256:…` (04-ISSUES/102), and `Recorded` reads those as the
+// `artifact-store://` references the rest of the mesh uses. Done here rather than when the store
+// is asked, because `Recorded` cannot tell one registry host from another — only the provenance
+// can, and the provenance is here.
+//
+// The oldest artifacts are exactly the ones recorded the old way, and exactly the ones a
+// sweep reaches first. Untranslated, the first of them ended every sweep.
+func asRecorded(reference string) string {
+	if reference == "" {
+		return ""
+	}
+	return catalogue.Recorded(reference)
+}
+
 // digestIn is the `sha256:<hex>` a reference names, empty when it names none.
 func digestIn(reference string) string {
 	for _, marker := range []string{"@sha256:", "/sha256:"} {

internal/inventory/collection_test.go

@@ -145,3 +145,48 @@ func TestAFailedBuildNamesNothingToCollectAndEachModuleIsCountedOnItsOwn(t *test
 		t.Fatalf("offered %v; want only web's oldest — db's three are all within its five", go_)
 	}
 }
+
+// An artifact recorded with the store's old address is offered for collection, in the vocabulary
+// the rest of the mesh speaks (novox/hq issue 226).
+//
+// Before references were kept without an address the mesh recorded
+// `<host>:<port>/<path>@sha256:…` (04-ISSUES/102). Those are the oldest artifacts, which makes
+// them exactly the ones an oldest-first sweep reaches first — and the first live run met one,
+// read "I will not address this" as "the store refuses everything", and collected none of 1681.
+func TestAnArtifactRecordedWithAnAddressIsOfferedAsTheMeshRecordsOne(t *testing.T) {
+	inv := fresh(t)
+	ctx := context.Background()
+
+	// The oldest build published the old way; five newer ones fill the module's five.
+	old := aBuild("a00", "tools", "")
+	old.Made = []Artifact{{Name: "build", Kind: "image",
+		Reference: "127.0.0.1:5100/tools/build@sha256:" + fmt.Sprintf("%064x", 1)}}
+	if err := inv.RecordBuild(ctx, old); err != nil {
+		t.Fatal(err)
+	}
+	for i := 2; i <= 6; i++ {
+		built(t, inv, fmt.Sprintf("a%02d", i), "tools", i)
+	}
+
+	go_, err := inv.ToCollect(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	want := ref("tools", "build", 1)
+	if len(go_) != 1 || go_[0] != want {
+		t.Fatalf("offered %v; want %q — the address is a route to the artifact, not part of its "+
+			"name, and the sweep speaks the name", go_, want)
+	}
+	// And marking it collected uses that same name, so the next sweep does not offer it again
+	// under a spelling it has not seen.
+	if err := inv.MarkCollected(ctx, go_); err != nil {
+		t.Fatal(err)
+	}
+	again, err := inv.ToCollect(ctx)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if len(again) != 0 {
+		t.Fatalf("offered %v again after collecting it", again)
+	}
+}