From 1bdca045c2daa1d74440d3e20bb4dcbbc83f1799 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:04:43 +0100 Subject: [PATCH 01/11] F-catalog-B: distribution removes a tagged version's tag alone when another tag names its digest Deleting the manifest by its digest removed every tag on it, and reproducible builds share a digest between a branch's tagged version and the trunk's numbered one, so a branch's end deleted the trunk's image (security review F1). The fake catalog now deletes by digest as the store does; a store that cannot delete a tag alone removes nothing and the removal is refused. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../internal/holder/holder_test.go | 32 +++++++++++++++ modules/distribution/internal/holder/oci.go | 15 ++++++- modules/distribution/internal/holder/verbs.go | 41 ++++++++++++++++++- 3 files changed, 85 insertions(+), 3 deletions(-) diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index 5e15abfa..c4324bcd 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -89,6 +89,7 @@ type fakeCatalog struct { tags map[string]string // name:version -> digest removed []string mirrorErr error + tagErr error indexes map[string][]string // name@digest -> the platform manifests it names } @@ -104,8 +105,23 @@ func (c *fakeCatalog) Versions(name string) ([]string, error) { func (c *fakeCatalog) Digest(name, version string) (string, error) { return c.tags[name+":"+version], nil } + +// Remove deletes by the digest, as the store does: every tag naming the digest goes with it. func (c *fakeCatalog) Remove(name, version string) error { c.removed = append(c.removed, name+":"+version) + d := c.tags[name+":"+version] + for k, v := range c.tags { + if n, _, _ := strings.Cut(k, ":"); n == name && v == d { + delete(c.tags, k) + } + } + return nil +} +func (c *fakeCatalog) RemoveTag(name, version string) error { + if c.tagErr != nil { + return c.tagErr + } + c.removed = append(c.removed, name+":"+version+" (tag)") delete(c.tags, name+":"+version) return nil } @@ -368,6 +384,22 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.5-feat-x.1", Reason: "branch merged"}); err != nil || b.cat.tags["m/p:0.1.5-feat-x.1"] != "" { t.Errorf("the builder's removal of a tagged version: %v", err) } + // Security review F1: a reproducible build gives a branch's tagged version and the trunk's numbered one one + // digest; the tagged one's removal takes its tag alone, and the numbered one stays pullable. + b.cat.tags["m/p:0.1.6-feat-y.1"], b.cat.tags["m/p:0.1.6"] = part.Digest, part.Digest + if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.6-feat-y.1", Reason: "branch merged"}); err != nil { + t.Fatalf("the removal of a tagged version sharing its digest: %v", err) + } + if b.cat.tags["m/p:0.1.6-feat-y.1"] != "" || b.cat.tags["m/p:0.1.6"] != part.Digest { + t.Errorf("tags after the tagged version's removal: %v; want the tag gone and 0.1.6 still at %s", b.cat.tags, part.Digest) + } + // A store that cannot delete a tag alone removes nothing, and the removal is refused naming the shared digest. + b.cat.tags["m/p:0.1.7-feat-z.1"], b.cat.tags["m/p:0.1.7"] = part.Digest, part.Digest + b.cat.tagErr = errors.New("UNSUPPORTED") + if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.7-feat-z.1", Reason: "branch merged"}); err == nil || !strings.Contains(err.Error(), "shares its digest") || b.cat.tags["m/p:0.1.7"] != part.Digest { + t.Errorf("a store that cannot delete a tag alone: %v, tags %v; want refused and 0.1.7 kept", err, b.cat.tags) + } + b.cat.tagErr = nil }) // CAT-AS1: what the sweep did over the store's door is the holder's: a removal for retention of an index removes // its platform manifests first, but those a tagged or held index of the repository names; a manifest such an index diff --git a/modules/distribution/internal/holder/oci.go b/modules/distribution/internal/holder/oci.go index c722d03b..c2bd00ef 100644 --- a/modules/distribution/internal/holder/oci.go +++ b/modules/distribution/internal/holder/oci.go @@ -253,7 +253,20 @@ func (o *OCI) Digest(name, version string) (string, error) { return resp.Header.Get("Docker-Content-Digest"), nil } -// Remove deletes the manifest name holds at version. +// RemoveTag deletes the tag version of name alone (the OCI distribution protocol's DELETE of a manifest by its tag); +// the manifest stays under every other tag. A store that cannot delete a tag answers an error. +func (o *OCI) RemoveTag(name, version string) error { + resp, b, err := o.do(http.MethodDelete, "/v2/"+name+"/manifests/"+version, "", nil) + if err != nil { + return err + } + if resp.StatusCode != http.StatusAccepted && resp.StatusCode != http.StatusNotFound { + return fmt.Errorf("the store did not delete the tag %s:%s alone (%d): %s", name, version, resp.StatusCode, b) + } + return nil +} + +// Remove deletes the manifest name holds at version, by its digest: every tag naming it goes with it. func (o *OCI) Remove(name, version string) error { d, err := o.Digest(name, version) if err != nil || d == "" { diff --git a/modules/distribution/internal/holder/verbs.go b/modules/distribution/internal/holder/verbs.go index cc90a063..31af720b 100644 --- a/modules/distribution/internal/holder/verbs.go +++ b/modules/distribution/internal/holder/verbs.go @@ -18,8 +18,12 @@ type Catalog interface { Versions(name string) ([]string, error) // Digest is the digest name holds at version, empty when none. Digest(name, version string) (string, error) - // Remove deletes name at version (its manifest); the store's collector frees its blobs. + // Remove deletes name at version by its manifest's digest, every tag naming that digest with it; the store's + // collector frees its blobs. Remove(name, version string) error + // RemoveTag deletes the tag version of name alone, the manifest staying under every other tag naming it; a store + // that cannot delete a tag alone answers an error and removes nothing. + RemoveTag(name, version string) error // RemoveDigest deletes an untagged manifest of name by its digest (retention). RemoveDigest(name, digest string) error // Platforms are the manifests an index of name names; none for a manifest that is not an index or is not held. @@ -149,7 +153,18 @@ func (v *Verbs) Remove(seat, caller string, r RemoveRequest) error { return fmt.Errorf("remove: the store cannot read %s@%s: %w", r.Name, r.Version, err) } if d != "" { - if err := v.Catalog.Remove(r.Name, r.Version); err != nil { + // Reproducible builds share a digest between a branch's tagged version and the trunk's numbered one: a + // manifest deleted by its digest would take the trunk's version with it (security review F1). When another + // tag names the digest, the tag alone goes; when the store cannot do that, nothing goes, said. + others, err := v.otherTags(r.Name, r.Version, d) + if err != nil { + return fmt.Errorf("remove: the store cannot say which other tags of %s name %s, so nothing is removed: %w", r.Name, d, err) + } + if len(others) > 0 { + if err := v.Catalog.RemoveTag(r.Name, r.Version); err != nil { + return fmt.Errorf("remove: refused: %s@%s shares its digest %s with %v, and the store did not remove the tag alone, so nothing is removed: %w", r.Name, r.Version, d, others, err) + } + } else if err := v.Catalog.Remove(r.Name, r.Version); err != nil { return fmt.Errorf("remove: the store did not remove %s@%s: %w", r.Name, r.Version, err) } } @@ -158,6 +173,28 @@ func (v *Verbs) Remove(seat, caller string, r RemoveRequest) error { return v.Say(seat, EventRemoved, ev) } +// otherTags are the tags of name but version that name digest. +func (v *Verbs) otherTags(name, version, digest string) ([]string, error) { + tags, err := v.Catalog.Versions(name) + if err != nil { + return nil, err + } + var out []string + for _, t := range tags { + if t == version { + continue + } + d, err := v.Catalog.Digest(name, t) + if err != nil { + return nil, err + } + if d == digest { + out = append(out, t) + } + } + return out, nil +} + // HoldRequest is `hold` and `release`. type HoldRequest struct { Name string `json:"name"` From 1b485eb2aae7db3e187fa199dce92056d02c7445 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:05:24 +0100 Subject: [PATCH 02/11] F-catalog-B: distribution's front takes no bootstrap credential The lead's ENG-5 ruling: on a fresh mesh the bootstrap pushes the holders' own images to the store on its loopback with no credential, so the front's bootstrap token, its file, the bootstrap-push window and the program's removal of the file go. Fewer secrets; nothing to refuse once a window closes. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/distribution-tools/main.go | 32 +------------ modules/distribution/cmd/distribution/main.go | 47 +------------------ .../cmd/distribution/main_test.go | 35 -------------- modules/distribution/internal/holder/front.go | 27 ++--------- .../internal/holder/holder_test.go | 26 +++------- .../distribution/internal/holder/window.go | 4 -- 6 files changed, 12 insertions(+), 159 deletions(-) delete mode 100644 modules/distribution/cmd/distribution/main_test.go diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index 2385b3e9..dd32a367 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -80,27 +80,6 @@ func finding(kind, subject, words string, raised bool) { } } -// closeBootstrap asks the program to discard the bootstrap's one-off credential once a publish is answered (design 51 -// *On a fresh mesh*): the window bootstrap-push closed in the state windows, which the program follows and then removes -// the file. The tools bundle runs in the tool runner's account, which cannot remove a file in distribution's state -// directory. Once per process; the window's record says it to the next. -var closeBootstrap func() - -func bootstrapCloser(windows kept[holder.Window]) func() { - return sync.OnceFunc(func() { - cur, _, err := windows.Get(holder.BootstrapWindow) - if err == nil && !cur.Closed.IsZero() { - return - } - w := holder.Window{Name: holder.BootstrapWindow, Closed: time.Now().UTC()} - if err := windows.Put(w); err != nil { - logf("the bootstrap's one-off credential could not be asked away: %v", err) - return - } - logf("the bootstrap's one-off credential is asked away: the holders serve publish") - }) -} - // learned are the proved callers whose build this process recorded as publishing through the seats. var learned sync.Map @@ -268,9 +247,6 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. } _, _ = v.DropIdle() a, err := v.Publish(seat, stdio.CallerOf(args), publishOf(in)) - if err == nil && closeBootstrap != nil { - closeBootstrap() - } if err == nil { learnPublishing(windows, stdio.CallerOf(args)) } @@ -465,12 +441,7 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. return nil, err } } - b, _, err := windows.Get(holder.BootstrapWindow) - if err != nil { - return nil, err - } - // The program removes the file when this window closes and writes it closed again with its removal's time. - return map[string]any{"window": cur, "bootstrap_credential_discarded": !b.Closed.IsZero(), "bootstrap": b}, nil + return map[string]any{"window": cur}, nil }}) } return out @@ -484,7 +455,6 @@ func main() { v := &holder.Verbs{Holder: h, Catalog: store, Holds: kept[holder.Hold]{state: "holds", key: holder.Hold.Key}, Mirrors: mirrors{}} go followMirrors(v) windows := kept[holder.Window]{state: "windows", key: func(w holder.Window) string { return w.Name }} - closeBootstrap = bootstrapCloser(windows) if err := stdio.Serve("", tools(v, windows, holder.ConfiguredCallers)); err != nil { logf("%v", err) os.Exit(1) diff --git a/modules/distribution/cmd/distribution/main.go b/modules/distribution/cmd/distribution/main.go index 242cc0b4..0de9470b 100644 --- a/modules/distribution/cmd/distribution/main.go +++ b/modules/distribution/cmd/distribution/main.go @@ -1,7 +1,7 @@ // Command distribution is distribution's program (novox/hq design 51): it serves the artifact store's port in front of // the registry container, which listens on its node's loopback only. Reads (GET, HEAD) go through to every node's // container runtime; a write is refused but while the direct-write window is open, never over a tag the seat holds -// published, or with the bootstrap's one-off credential (holder.Front). The windows are distribution's state windows: +// published (holder.Front). The windows are distribution's state windows: // the program moves them (windows.go) and follows the direct-write window's record on each change; it holds no timer. package main @@ -10,7 +10,6 @@ import ( "encoding/json" "errors" "fmt" - "io/fs" "log" "net/http" "net/url" @@ -58,7 +57,6 @@ func run(ctx context.Context, getenv func(string) string) error { windows := process.OptionalBucket(ctx, js, "distribution_windows", "the direct-write window is read as open until the program starts again", logger.Printf) if windows != nil { go follow(ctx, windows, &open, logger) - go followBootstrap(ctx, windows, logger) calls, err := process.Connect(cred) if err != nil { return err @@ -72,13 +70,6 @@ func run(ctx context.Context, getenv func(string) string) error { Published: func(name, tag string) (bool, error) { d, err := oci.Digest(name, tag) return d != "", err - }, - Bootstrap: func() (string, bool) { - b, err := os.ReadFile(holder.BootstrapFile) - if err != nil { - return "", false - } - return strings.TrimSpace(string(b)), true }} srv := &http.Server{Addr: or(getenv("MESH_REGISTRY_LISTEN"), ":5000"), Handler: front, ReadHeaderTimeout: 30 * time.Second} errs := make(chan error, 1) @@ -119,42 +110,6 @@ func follow(ctx context.Context, kv jetstream.KeyValue, open *atomic.Bool, logge } } -// followBootstrap removes the bootstrap's one-off credential (holder.BootstrapFile) when the tools bundle closes the -// window bootstrap-push at its first answered publish (design 51 *On a fresh mesh*): the program runs as distribution's -// account, which owns the file. A file already gone is no fault; one that cannot be removed is said each time the -// window is written. -func followBootstrap(ctx context.Context, kv jetstream.KeyValue, logger *log.Logger) { - w, err := kv.Watch(ctx, holder.BootstrapWindow) - if err != nil { - logger.Printf("the bootstrap's credential window cannot be watched (%v): its file stays until the program starts again", err) - return - } - for e := range w.Updates() { - if e == nil || e.Operation() != jetstream.KeyValuePut { - continue - } - var win holder.Window - if json.Unmarshal(e.Value(), &win) != nil || win.Closed.IsZero() { - continue - } - if why := removeBootstrap(holder.BootstrapFile); why != "" { - logger.Printf("%s", why) - } - } -} - -// removeBootstrap removes the credential's file; it answers what went wrong, or "" when the file is gone. -func removeBootstrap(path string) string { - switch err := os.Remove(path); { - case err == nil: - return "the bootstrap's one-off credential is discarded: the holders serve publish" - case errors.Is(err, fs.ErrNotExist): - return "" - default: - return fmt.Sprintf("the bootstrap's one-off credential could not be discarded, so it is still accepted: %v", err) - } -} - func or(v, d string) string { if strings.TrimSpace(v) == "" { return d diff --git a/modules/distribution/cmd/distribution/main_test.go b/modules/distribution/cmd/distribution/main_test.go deleted file mode 100644 index f9ef3acf..00000000 --- a/modules/distribution/cmd/distribution/main_test.go +++ /dev/null @@ -1,35 +0,0 @@ -package main - -import ( - "os" - "path/filepath" - "strings" - "testing" -) - -// The program, which runs as distribution's account, removes the bootstrap's one-off credential when its window -// closes (design 51 *On a fresh mesh*; a3-publish-contract): removed once, a file already gone is no fault, and one -// that cannot be removed is said, since the front still accepts it. -func TestTheProgramDiscardsTheBootstrapCredential(t *testing.T) { - dir := t.TempDir() - file := filepath.Join(dir, "bootstrap-push") - if err := os.WriteFile(file, []byte("token\n"), 0o600); err != nil { - t.Fatal(err) - } - if why := removeBootstrap(file); !strings.Contains(why, "discarded") { - t.Fatalf("first removal = %q, want it said discarded", why) - } - if _, err := os.Stat(file); !os.IsNotExist(err) { - t.Fatalf("the file is still there: %v", err) - } - if why := removeBootstrap(file); why != "" { - t.Fatalf("a file already gone = %q, want nothing said", why) - } - stuck := filepath.Join(dir, "stuck") - if err := os.MkdirAll(filepath.Join(stuck, "child"), 0o700); err != nil { - t.Fatal(err) - } - if why := removeBootstrap(stuck); !strings.Contains(why, "still accepted") { - t.Fatalf("an unremovable file = %q, want it said still accepted", why) - } -} diff --git a/modules/distribution/internal/holder/front.go b/modules/distribution/internal/holder/front.go index eb42ae13..44c4988a 100644 --- a/modules/distribution/internal/holder/front.go +++ b/modules/distribution/internal/holder/front.go @@ -1,7 +1,6 @@ package holder import ( - "crypto/subtle" "fmt" "net/http" "net/http/httputil" @@ -15,7 +14,8 @@ import ( // protocol's GET and HEAD) stay open to every node's container runtime; a write reaches the store only from // distribution's own process, which writes to the loopback directly and never through the front. While the direct-write // window is open, a build module's old build still pushes through the front as today, but never to a tag the seat holds -// published. +// published. No credential opens it: on a fresh mesh the bootstrap pushes the holders' own images to the store on its +// loopback, never through the front (design 51 *On a fresh mesh*). type Front struct { // Store is the registry container's loopback address. Store *url.URL @@ -23,9 +23,6 @@ type Front struct { Window *atomic.Bool // Published says whether name holds a tag published through the seat. Published func(name, tag string) (bool, error) - // Bootstrap is the bootstrap's one-off credential for the holders' first images (design 51 *On a fresh mesh*): - // read on each write, never kept, so the file's removal (BootstrapFile) refuses the next write that carries it. - Bootstrap func() (token string, ok bool) Logf func(string, ...any) proxy *httputil.ReverseProxy } @@ -40,14 +37,7 @@ func (f *Front) ServeHTTP(w http.ResponseWriter, r *http.Request) { f.proxy.ServeHTTP(w, r) return } - bootstrap := false - if f.Bootstrap != nil { - if token, ok := f.Bootstrap(); ok && token != "" { - got, has := strings.CutPrefix(r.Header.Get("Authorization"), "Bearer ") - bootstrap = has && subtle.ConstantTimeCompare([]byte(got), []byte(token)) == 1 - } - } - if !bootstrap && (f.Window == nil || !f.Window.Load()) { + if f.Window == nil || !f.Window.Load() { refuse(w, http.StatusForbidden, "DENIED", "the store takes writes from distribution's own process alone: publish through the seat's publish") return } @@ -62,21 +52,12 @@ func (f *Front) ServeHTTP(w http.ResponseWriter, r *http.Request) { return } } - if bootstrap { - if f.Logf != nil { - f.Logf("a write %s %s from %s with the bootstrap's one-off credential", r.Method, r.URL.Path, r.RemoteAddr) - } - } else if f.Logf != nil { + if f.Logf != nil { f.Logf("a direct write %s %s from %s, accepted while the direct-write window is open", r.Method, r.URL.Path, r.RemoteAddr) } f.proxy.ServeHTTP(w, r) } -// BootstrapFile is where the node-engine's bootstrap gives distribution its one-off credential, in distribution's -// state directory (0600, distribution's account): one line, the token. The tools bundle removes it when it answers its -// first publish, so the holders serve publish and the credential is gone (RemoveBootstrap). -const BootstrapFile = "/var/lib/mesh/registry/bootstrap-push" - // manifestPut is a PUT of /v2//manifests/, a tag and not a digest. func manifestPut(r *http.Request) (name, tag string, ok bool) { if r.Method != http.MethodPut { diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index c4324bcd..19ad1e33 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -469,9 +469,7 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { defer store.Close() u, _ := url.Parse(store.URL) var open atomic.Bool - token, kept := "s3cret", true - f := &Front{Store: u, Window: &open, Published: func(name, tag string) (bool, error) { return name == "m/i" && tag == "0.1.3", nil }, - Bootstrap: func() (string, bool) { return token, kept }} + f := &Front{Store: u, Window: &open, Published: func(name, tag string) (bool, error) { return name == "m/i" && tag == "0.1.3", nil }} auth := "" do := func(method, path string) int { rec := httptest.NewRecorder() @@ -500,24 +498,12 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { t.Errorf("window open %v: %s %s = %d, want %d", c.open, c.method, c.path, got, c.want) } } - // The bootstrap's one-off credential writes with the window closed, never over a published tag, and not once - // its file is gone; a wrong token writes nothing. + // No credential opens the front (the lead's ENG-5 ruling): a write carrying a token, with the window closed, is + // refused as any other. open.Store(false) - for _, c := range []struct { - auth string - kept bool - path string - want int - }{ - {"s3cret", true, "/v2/gitea/program/manifests/0.1.0", http.StatusCreated}, - {"s3cret", true, "/v2/m/i/manifests/0.1.3", http.StatusForbidden}, - {"wrong", true, "/v2/gitea/program/manifests/0.1.0", http.StatusForbidden}, - {"s3cret", false, "/v2/gitea/program/manifests/0.1.0", http.StatusForbidden}, - } { - auth, kept = c.auth, c.kept - if got := do(http.MethodPut, c.path); got != c.want { - t.Errorf("bootstrap %q kept %v: PUT %s = %d, want %d", c.auth, c.kept, c.path, got, c.want) - } + auth = "s3cret" + if got := do(http.MethodPut, "/v2/gitea/program/manifests/0.1.0"); got != http.StatusForbidden { + t.Errorf("a write with a bearer token and the window closed = %d, want %d", got, http.StatusForbidden) } }) t.Run("mirror copies an outside image by its pinned digest, every platform, only the parts the store lacks", func(t *testing.T) { diff --git a/modules/distribution/internal/holder/window.go b/modules/distribution/internal/holder/window.go index 3336d74c..8b626823 100644 --- a/modules/distribution/internal/holder/window.go +++ b/modules/distribution/internal/holder/window.go @@ -16,10 +16,6 @@ import ( const ( OldNameWindow = "old-name" DirectWritesWindow = "direct-writes" - // BootstrapWindow is the bootstrap's one-off credential (BootstrapFile): open while the file is there. The tools - // bundle closes it at its first answered publish; the program, which runs as distribution's account and alone may - // remove the file in distribution's state directory, removes it on that close. - BootstrapWindow = "bootstrap-push" // WindowFindingAfter is how long a node holds a window open before it is distribution's finding naming the node. WindowFindingAfter = 7 * 24 * time.Hour // The events a window's close says on distribution's own family distribution.events (7 days, 16 MiB, drop From 6f89e622df0d7e5de39a72808a3621486dc28ec6 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:07:10 +0100 Subject: [PATCH 03/11] F-catalog-B: distribution's windows step by mesh-sdk's delivery.StepWindow and say why a node holds one A read naming no node (an empty node_haves) closed both windows for good (security review F3, second of a kind): the window embeds delivery.WindowHeld and steps by the shared rule, named being every node the read named, and keeps its own guard that a window once closed stays closed. A node-engine whose manifest-reads did not answer (a timeout included) counts as stating nothing as before, and the window's record now says so per node (Why), so registry_old_name tells a timeout from an old node-engine. A read that fails otherwise is an error the wake judges nothing on, no longer taken as not served. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../distribution/cmd/distribution/windows.go | 7 +- .../distribution/internal/holder/driver.go | 33 +++++-- .../internal/holder/holder_test.go | 27 ++++-- .../internal/holder/mirror_fakes_test.go | 5 +- .../distribution/internal/holder/window.go | 86 ++++++++++++------- 5 files changed, 111 insertions(+), 47 deletions(-) diff --git a/modules/distribution/cmd/distribution/windows.go b/modules/distribution/cmd/distribution/windows.go index 03734c82..e2356fbd 100644 --- a/modules/distribution/cmd/distribution/windows.go +++ b/modules/distribution/cmd/distribution/windows.go @@ -91,9 +91,12 @@ func (d busDelivery) Reads(node string) (bool, []string, error) { raw, err := d.c.CallOn(node, readsAsk, map[string]any{}) if err != nil { if notServed(err) { - return false, nil, nil // a node-engine not serving manifest-reads states nothing (design 51) + // A node-engine not serving manifest-reads states nothing (design 51); what the call answered (an away node, + // an older node-engine, a timeout) goes with it, so the window's tool says why the node holds it. + return false, nil, err } - return false, nil, err + // Any other failure is a read that did not happen: served true with its error, so the wake judges nothing. + return true, nil, err } var a struct { Reads []string `json:"manifest-reads"` diff --git a/modules/distribution/internal/holder/driver.go b/modules/distribution/internal/holder/driver.go index d682923d..49cab5bf 100644 --- a/modules/distribution/internal/holder/driver.go +++ b/modules/distribution/internal/holder/driver.go @@ -3,6 +3,7 @@ package holder import ( "errors" "fmt" + "maps" "slices" "sort" "strings" @@ -44,7 +45,8 @@ type NodeHave struct { // Delivery is what the windows' driver asks of the delivery parts and the node-engine seat, through their tools and // verbs on the bus (ADR 0596): every node's haves, away ones included, and one node-engine's stated reader -// capabilities. Reads answers served false when the node-engine does not serve `manifest-reads` (it states nothing). +// capabilities. Reads answers served false when the node-engine does not serve `manifest-reads` (it states nothing), +// with the call's error, if any, saying why (an away node, an older node-engine, a timeout). type Delivery interface { Haves() ([]NodeHave, error) Reads(node string) (served bool, stated []string, err error) @@ -118,6 +120,9 @@ func (d *Driver) Read() ([]NodeRead, error) { return nil, fmt.Errorf("%s's manifest-reads cannot be read, so no window moves: %w", node, err) } n.Serves, n.Stated = served, stated + if !served && err != nil { + n.NotServed = err.Error() + } out = append(out, *n) } return out, nil @@ -131,13 +136,20 @@ func (d *Driver) Wake(why string) ([]string, error) { return nil, err } now := d.now() + named := make([]string, 0, len(nodes)) + byNode := map[string]NodeRead{} + for _, n := range nodes { + named = append(named, n.Node) + byNode[n.Node] = n + } var said, errs []string for _, c := range []struct { name, event, seat string holding func([]NodeRead, []DroppedNode) []string + why func(NodeRead) string }{ - {OldNameWindow, OldNameClosedEvent, OldSeat, HoldingOldName}, - {DirectWritesWindow, DirectWritesClosedEvent, "", HoldingDirectWrites}, + {OldNameWindow, OldNameClosedEvent, OldSeat, HoldingOldName, WhyOldName}, + {DirectWritesWindow, DirectWritesClosedEvent, "", HoldingDirectWrites, WhyDirectWrites}, } { cur, err := d.State.Window(c.name) if err != nil { @@ -145,9 +157,16 @@ func (d *Driver) Wake(why string) ([]string, error) { continue } if cur.Name == "" { - cur = Window{Name: c.name, Open: true} + cur = Window{Name: c.name} + cur.Open = true + } + next, closed, overdue := cur.Step(named, c.holding(nodes, cur.Dropped), now) + if next.Open && len(named) > 0 { + next.Why = map[string]string{} + for _, n := range next.Holding { + next.Why[n] = c.why(byNode[n]) + } } - next, closed, overdue := cur.Step(c.holding(nodes, cur.Dropped), now) if !windowsEqual(cur, next) { if err := d.State.PutWindow(next); err != nil { errs = append(errs, fmt.Sprintf("the %s window is not kept: %v", c.name, err)) @@ -172,6 +191,8 @@ func (d *Driver) Wake(why string) ([]string, error) { } } said = append(said, fmt.Sprintf("the %s window is closed (%s); it was held last by %v", c.name, why, closed.LastHolding)) + } else if len(named) == 0 { + said = append(said, fmt.Sprintf("the %s window is not judged (%s): the read named no node", c.name, why)) } else if next.Open { said = append(said, fmt.Sprintf("the %s window stays open (%s): held by %v", c.name, why, next.Holding)) } @@ -191,5 +212,5 @@ func (d *Driver) now() time.Time { func windowsEqual(a, b Window) bool { return a.Open == b.Open && slices.Equal(a.Holding, b.Holding) && a.Closed.Equal(b.Closed) && - len(a.Since) == len(b.Since) && len(a.Dropped) == len(b.Dropped) + len(a.Since) == len(b.Since) && len(a.Dropped) == len(b.Dropped) && maps.Equal(a.Why, b.Why) } diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index 19ad1e33..c5efea76 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -630,20 +630,26 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { t.Errorf("direct writes held by %v, want g14", got) } t0 := time.Unix(0, 0) - w, closed, _ := Window{Name: OldNameWindow}.Step([]string{"g14"}, t0) + all := []string{"ace", "g14", "novox"} + w, closed, _ := Window{Name: OldNameWindow}.Step(all, []string{"g14"}, t0) if closed != nil || !w.Open { t.Fatalf("Step = %+v, %+v", w, closed) } - if _, _, overdue := w.Step([]string{"g14"}, t0.Add(WindowFindingAfter)); !reflect.DeepEqual(overdue, []string{"g14"}) { + if _, _, overdue := w.Step(all, []string{"g14"}, t0.Add(WindowFindingAfter)); !reflect.DeepEqual(overdue, []string{"g14"}) { t.Errorf("seven days: overdue %v", overdue) } + // Security review F3: a read naming no node (an empty node_haves) judges nothing; the window stays open. + if same, shut, _ := w.Step(nil, nil, t0.Add(time.Minute)); shut != nil || !same.Open || !reflect.DeepEqual(same.Holding, []string{"g14"}) { + t.Errorf("a read naming no node: %+v, closed %+v; want the window open as it was", same, shut) + } w, _ = w.Drop("g14", "person.jochen", "retired", t0) - w, closed, _ = w.Step(HoldingOldName(nodes, w.Dropped), t0.Add(time.Hour)) + w, closed, _ = w.Step(all, HoldingOldName(nodes, w.Dropped), t0.Add(time.Hour)) if closed == nil || closed.Window != OldNameWindow || !reflect.DeepEqual(closed.LastHolding, []string{"g14"}) || len(closed.Dropped) != 1 { t.Errorf("closing = %+v", closed) } - if _, again, _ := w.Step([]string{"g14"}, t0.Add(2*time.Hour)); again != nil { - t.Error("a closed window closed again") + // Once closed, it stays closed: a node holding it later opens nothing. + if again, shut, _ := w.Step(all, []string{"g14"}, t0.Add(2*time.Hour)); shut != nil || again.Open { + t.Errorf("a closed window stepped on a holder: %+v, closed %+v; want it closed, said once", again, shut) } }) t.Run("the windows' driver: every node read on each wake, closed once, a paused build agent's same build counted", func(t *testing.T) { @@ -665,6 +671,17 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if !reflect.DeepEqual(st.w[OldNameWindow].Holding, []string{"g14"}) || !reflect.DeepEqual(st.w[DirectWritesWindow].Holding, []string{"g14"}) { t.Errorf("windows = %+v, want g14 holding both (away: states nothing; runs the old build)", st.w) } + // The window's record says why g14 holds it: its manifest-reads timed out, counted as stating nothing. + if why := st.w[OldNameWindow].Why["g14"]; !strings.Contains(why, "did not answer") || !strings.Contains(why, "timeout") { + t.Errorf("why g14 holds the old name = %q, want the timeout said", why) + } + // Security review F3: a wake whose node_haves names no node closes neither window. + haves := del.haves + del.haves = nil + if _, err := d.Wake("delivered"); err != nil || !st.w[OldNameWindow].Open || !st.w[DirectWritesWindow].Open || len(closed) != 0 { + t.Fatalf("a wake on an empty node_haves: %v, windows %+v, closed %v; want both open, none said", err, st.w, closed) + } + del.haves = haves now = now.Add(WindowFindingAfter) _, _ = d.Wake("delivered") if !found[FindingWindowHeld+" "+OldNameWindow+".g14"] { diff --git a/modules/distribution/internal/holder/mirror_fakes_test.go b/modules/distribution/internal/holder/mirror_fakes_test.go index 6e51ebd0..2ee12d86 100644 --- a/modules/distribution/internal/holder/mirror_fakes_test.go +++ b/modules/distribution/internal/holder/mirror_fakes_test.go @@ -98,7 +98,10 @@ type fakeDelivery struct { func (f *fakeDelivery) Haves() ([]NodeHave, error) { return f.haves, nil } func (f *fakeDelivery) Reads(node string) (bool, []string, error) { s, ok := f.stated[node] - return ok, s, nil + if !ok { + return false, nil, errors.New("nats: timeout") + } + return true, s, nil } type memWindows struct { diff --git a/modules/distribution/internal/holder/window.go b/modules/distribution/internal/holder/window.go index 8b626823..8ebae12a 100644 --- a/modules/distribution/internal/holder/window.go +++ b/modules/distribution/internal/holder/window.go @@ -2,6 +2,8 @@ package holder import ( "fmt" + + "git.novox.be/novox/mesh-sdk/go/delivery" "slices" "sort" "time" @@ -27,16 +29,16 @@ const ( ) // Window is one window's record, kept in distribution's state windows: what registry_old_name and -// registry_direct_writes answer. +// registry_direct_writes answer. Its open, holding, since, closed and last holding are mesh-sdk's delivery.WindowHeld, +// the part every window keeper shares and steps by one rule (delivery.StepWindow). type Window struct { - Name string `json:"name"` - Open bool `json:"open"` - Holding []string `json:"holding,omitempty"` - Since map[string]time.Time `json:"since,omitempty"` - Dropped []DroppedNode `json:"dropped,omitempty"` - Closed time.Time `json:"closed,omitzero"` - // LastHolding are the nodes that held it last, said when it closes. - LastHolding []string `json:"last_holding,omitempty"` + Name string `json:"name"` + delivery.WindowHeld + Dropped []DroppedNode `json:"dropped,omitempty"` + // Why says, of each node holding the window, why it holds it: what its read answered, a node-engine whose + // manifest-reads did not answer (an away node, an older node-engine, a timeout) included, so a timeout taken as + // stating nothing is told apart from a node-engine that states the old names. + Why map[string]string `json:"why,omitempty"` } // DroppedNode is one retired node dropped from a window by hand: a hand act. @@ -57,16 +59,42 @@ type Closed struct { } // NodeRead is what distribution read of one node: the reader capabilities its node-engine states through -// `manifest-reads` (Serves false while it does not serve the verb: it states nothing), whether its declaration names an -// artifact, and its build modules' haves: whether each runs a build that publishes through the seats. +// `manifest-reads` (Serves false while it does not serve the verb: it states nothing; NotServed then says what the +// call answered), whether its declaration names an artifact, and its build modules' haves: whether each runs a build +// that publishes through the seats. type NodeRead struct { Node string `json:"node"` Serves bool `json:"serves"` + NotServed string `json:"not_served,omitempty"` Stated []string `json:"stated,omitempty"` NamesArtifact bool `json:"names_artifact"` BuildModules map[string]bool `json:"build_modules,omitempty"` } +// WhyOldName says why a node holds the old name open. +func WhyOldName(n NodeRead) string { + if !n.Serves { + if n.NotServed != "" { + return "its node-engine's manifest-reads did not answer, so it counts as stating nothing: " + n.NotServed + } + return "its node-engine does not serve manifest-reads, so it states nothing" + } + return "its node-engine does not state " + NewNamesCapability +} + +// WhyDirectWrites says why a node holds the direct writes open: its build modules running a build not heard +// publishing through the seats. +func WhyDirectWrites(n NodeRead) string { + var old []string + for m, publishes := range n.BuildModules { + if !publishes { + old = append(old, m) + } + } + sort.Strings(old) + return fmt.Sprintf("its %v run a build not heard publishing through the seats", old) +} + // HoldingOldName are the nodes holding the old name open: a node whose declaration names an artifact and whose // node-engine does not state the new names (one not serving manifest-reads states nothing). func HoldingOldName(nodes []NodeRead, dropped []DroppedNode) []string { @@ -106,33 +134,25 @@ func isDropped(dropped []DroppedNode, node string) bool { return slices.ContainsFunc(dropped, func(d DroppedNode) bool { return d.Node == node }) } -// Step moves the window to the nodes holding it at now. It answers the next record, the closing event when it closed in -// this step (said once), and the nodes holding it past WindowFindingAfter (the finding; cleared when it closes). A -// window once closed stays closed: a node that holds it later is no reason to serve the old name again. -func (w Window) Step(holding []string, now time.Time) (next Window, closed *Closed, overdue []string) { - next = Window{Name: w.Name, Dropped: w.Dropped, Closed: w.Closed, LastHolding: w.LastHolding} +// Step moves the window by mesh-sdk's one rule (delivery.StepWindow): named is every node the read named, dropped ones +// included, holding those holding the window at now. A read naming no node judges nothing. It answers the next record, +// the closing event when it closed in this step (said once), and the nodes holding it past WindowFindingAfter (the +// finding; cleared when it closes). A window once closed stays closed: a node that holds it later is no reason to serve +// the old name again or to take direct writes again. +func (w Window) Step(named, holding []string, now time.Time) (next Window, closed *Closed, overdue []string) { + next = w if !w.Open && !w.Closed.IsZero() { return next, nil, nil } - if len(holding) == 0 { - next.Closed, next.LastHolding = now, w.Holding - if next.LastHolding == nil { - next.LastHolding = []string{} - } - return next, &Closed{Window: w.Name, At: now, LastHolding: next.LastHolding, Dropped: w.Dropped}, nil + held, shut, overdue := delivery.StepWindow(w.WindowHeld, named, holding, now, WindowFindingAfter) + next.WindowHeld = held + if !held.Open { + next.Why = nil } - next.Open, next.Holding, next.Since = true, holding, map[string]time.Time{} - for _, n := range holding { - since, had := w.Since[n] - if !had { - since = now - } - next.Since[n] = since - if now.Sub(since) >= WindowFindingAfter { - overdue = append(overdue, n) - } + if !shut { + return next, nil, overdue } - return next, nil, overdue + return next, &Closed{Window: w.Name, At: held.Closed, LastHolding: held.LastHolding, Dropped: w.Dropped}, nil } // Drop drops a retired node from the window by hand, a hand act with who and why. From dce7d41671c01eb86b36073cfc9266d0467ed9c9 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:09:17 +0100 Subject: [PATCH 04/11] F-catalog-B: distribution counts incoming against its binding's cap and drops an idle transfer by its expiring key The holder counted against 20 GiB while the bus caps incoming at 8 GiB, so the bus refused writes before the holder said refused naming the cap, and the 80 % finding came too late (running review F2). The cap is read from incoming's binding (the object store's fill); IncomingCap, 8 GiB, is said only where none is read, and the manifest's row holds it equal to max-mib. An abandoned transfer was dropped only on the next publish, and the transfers state's own lifetime dropped its key while its parts stayed: transfers keeps for good, and each open transfer has a key in the new state idle with its own time to live, whose end drops it (re-armed for the rest when it took a part since). registry_incoming reports the bound cap. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/distribution-tools/main.go | 55 +++++++++++++++- .../internal/holder/holder_test.go | 66 +++++++++++++++++-- .../distribution/internal/holder/transfer.go | 64 +++++++++++++++--- modules/distribution/module.json | 7 +- 4 files changed, 174 insertions(+), 18 deletions(-) diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index dd32a367..17b9ec2c 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -15,10 +15,12 @@ import ( "os" "strings" "sync" + "sync/atomic" "time" reg "git.novox.be/novox/mesh-contracts/go/seats/containerimageregistry" stdio "git.novox.be/novox/mesh-sdk/go" + "git.novox.be/novox/mesh-sdk/go/busstate" "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/health" "git.novox.be/novox/mesh-sdk/go/parts" @@ -164,6 +166,53 @@ func learnInto(state casState, module, fp string, attempts int) (bool, error) { return false, fmt.Errorf("%s was written by another writer first %d times: %w", holder.PublishingKey, attempts, stdio.ErrChanged) } +// incomingCap reads incoming's cap from its binding (the bus's bound of the object store, design 51 *Publishing*), once +// it answers one: the binding's cap does not change while the bundle runs. +func incomingCap() func() (uint64, error) { + var known atomic.Uint64 + return func() (uint64, error) { + if c := known.Load(); c > 0 { + return c, nil + } + f, err := busstate.ObjectsOf(holder.IncomingObjects).Fill() + if err != nil { + return 0, fmt.Errorf("incoming's cap cannot be read from its binding: %w", err) + } + if f.MaxBytes == 0 { + return 0, errors.New("incoming's binding carries no cap") + } + known.Store(f.MaxBytes) + return f.MaxBytes, nil + } +} + +// armIdle writes a transfer's idle key with its own time to live; one already there is the same bound, kept. +func armIdle(id string, ttl time.Duration) error { + if _, err := stdio.State(holder.IdleState).CreateTTL(id, map[string]any{"transfer": id}, ttl); err != nil && !errors.Is(err, stdio.ErrChanged) { + return err + } + return nil +} + +// followIdle drops a transfer when its idle key ends (holder.Expired): the bus's marker is the event, no timer runs. +func followIdle(h *holder.Holder) { + <-stdio.Served() + stdio.State(holder.IdleState).Follow("", func(ch stdio.StateChange) error { + if ch.Op != stdio.OpExpire { + return nil + } + dropped, err := h.Expired(ch.Key) + if err != nil { + logf("%v", err) + return err + } + if dropped { + logf("transfer %s had no part for %s: dropped with its parts", ch.Key, holder.TransferIdle) + } + return nil + }) +} + // mirrors is the state mirrors: an ask is created once; one already kept is the same copy, taken on already. type mirrors struct{} @@ -245,7 +294,6 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. if err != nil { return nil, err } - _, _ = v.DropIdle() a, err := v.Publish(seat, stdio.CallerOf(args), publishOf(in)) if err == nil { learnPublishing(windows, stdio.CallerOf(args)) @@ -397,7 +445,7 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. } all, err := v.Transfers.List() used, _ := v.Objects.Used() - return map[string]any{"transfers": all, "used": used, "cap": holder.IncomingCap, "dropped_idle": idle}, err + return map[string]any{"transfers": all, "used": used, "cap": v.CapNow(), "dropped_idle": idle}, err }}, stdio.Tool{Name: "registry_holds", Description: "What is held, since when and for whom; with release, lets one go through the seat's release as the operator's session (a hand act).", Input: obj(map[string]any{"seat": s("the seat"), "release": s("the name to release"), "digest": s("its digest"), "why": s("why")}), @@ -451,9 +499,10 @@ func main() { base, _ := url.Parse("http://127.0.0.1:5001") store := &holder.OCI{Base: base, Upstream: &holder.Upstream{}} h := &holder.Holder{Objects: parts.BusObjects(holder.IncomingObjects), Builder: delivery.BuilderVersions{Ask: stdio.Ask}, Store: store, Say: say, Finding: finding, - Transfers: parts.BusTransfers[holder.Opening](holder.TransfersState)} + Transfers: parts.BusTransfers[holder.Opening](holder.TransfersState), Cap: incomingCap(), Idle: armIdle} v := &holder.Verbs{Holder: h, Catalog: store, Holds: kept[holder.Hold]{state: "holds", key: holder.Hold.Key}, Mirrors: mirrors{}} go followMirrors(v) + go followIdle(h) windows := kept[holder.Window]{state: "windows", key: func(w holder.Window) string { return w.Name }} if err := stdio.Serve("", tools(v, windows, holder.ConfiguredCallers)); err != nil { logf("%v", err) diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index c5efea76..0b384a03 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -194,6 +194,9 @@ func newBed(builderOK bool) *bed { return b } +// capOf is a binding's cap of n bytes. +func capOf(n uint64) func() (uint64, error) { return func() (uint64, error) { return n, nil } } + func keys[V any](m map[string]V) []string { var out []string for k := range m { @@ -267,7 +270,11 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { _, err := b.h.PublishPart(ArtifactSeat, builder, PartRequest{Transfer: "t", Digest: part.Digest, Data: make([]byte, MaxPart+1)}) return err }, "over the"}, - {"a new transfer at the cap", func(b *bed) error { b.h.Cap = 100; _, err := b.h.Publish(ArtifactSeat, builder, req("t")); return err }, "at their cap"}, + {"a new transfer at the cap", func(b *bed) error { + b.h.Cap = capOf(100) + _, err := b.h.Publish(ArtifactSeat, builder, req("t")) + return err + }, "at their cap"}, {"a version the builder did not number", func(b *bed) error { b.h.Builder = fakeBuilder{ok: false} b.store.has[part.Digest] = true // held already: the publish ends at once @@ -332,10 +339,38 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if dropped, _ := b.h.DropIdle(); !reflect.DeepEqual(dropped, []string{"t2"}) { t.Errorf("DropIdle = %v, want t2 after 24 hours with no part", dropped) } + // Running review F2, ADR 0611 §3: the drop follows each transfer's idle key. The open arms it for 24 hours; its + // end re-arms a transfer that took a part since for the rest, and drops one idle 24 hours, its chunks with it. + b = newBed(true) + armed := map[string]time.Duration{} + b.h.Idle = func(id string, ttl time.Duration) error { armed[id] = ttl; return nil } + if _, err := b.h.Publish(ArtifactSeat, builder, req("t4")); err != nil || armed["t4"] != TransferIdle { + t.Fatalf("publish = %v, armed %v; want t4's idle key for %s", err, armed, TransferIdle) + } + b.now = b.now.Add(time.Hour) + _, _ = b.h.PublishPart(ArtifactSeat, builder, PartRequest{Transfer: "t4", Digest: part.Digest, Data: blob[:100]}) + b.now = b.now.Add(TransferIdle - time.Hour) + if dropped, err := b.h.Expired("t4"); err != nil || dropped || armed["t4"] != time.Hour { + t.Errorf("the key's end an hour after a part: dropped %v, %v, armed %v; want t4 kept and armed for 1h", dropped, err, armed) + } + b.now = b.now.Add(time.Hour) + if dropped, err := b.h.Expired("t4"); err != nil || !dropped { + t.Errorf("the key's end 24 hours after the last part: dropped %v, %v; want t4 dropped", dropped, err) + } + if _, found, _ := b.h.Transfers.Get("t4"); found || len(b.h.Objects.(memObjects)) != 0 { + t.Errorf("t4 after its drop: kept %v, objects %v; want both gone", found, b.h.Objects) + } + if dropped, err := b.h.Expired("t4"); err != nil || dropped { + t.Errorf("the end of a key whose transfer is gone: %v, %v; want nothing done", dropped, err) + } + b.h.Idle = func(string, time.Duration) error { return errors.New("bus down") } + if _, err := b.h.Publish(ArtifactSeat, builder, req("t5")); err == nil || !strings.Contains(err.Error(), "idle bound") { + t.Errorf("a publish whose idle key is not written = %v; want it failed loudly", err) + } }) t.Run("incoming's finding at 80 %, cleared below", func(t *testing.T) { b := newBed(true) - b.h.Cap = 3500 + b.h.Cap = capOf(3500) _, _ = b.h.Publish(ArtifactSeat, builder, req("t3")) _, _ = b.h.PublishPart(ArtifactSeat, builder, PartRequest{Transfer: "t3", Digest: part.Digest, Data: blob[:2800]}) if !b.findings["incoming-full incoming"] { @@ -346,12 +381,12 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { t.Error("the finding stayed after the transfer was dropped") } // A refusal for want of room stays loud on incoming until the holder takes a publish again. - b.h.Cap = 100 + b.h.Cap = capOf(100) _, _ = b.h.Publish(ArtifactSeat, builder, req("t4")) if !b.findings["publish-refused incoming"] { t.Errorf("a refusal at the cap raised %v, want publish-refused on incoming", b.findings) } - b.h.Cap = 1 << 20 + b.h.Cap = capOf(1 << 20) b.store.has[part.Digest] = true if _, err := b.h.Publish(ArtifactSeat, builder, req("t5")); err != nil || b.findings["publish-refused incoming"] { t.Errorf("the next publish taken = %v, findings %v: want the refusal at the cap cleared", err, b.findings) @@ -764,5 +799,28 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if store < 0 || program < 0 || store > program { t.Errorf("store at %d, program at %d: want the store listed first", store, program) } + // Running review F2: incoming asks the bus 8 GiB, the most the objects template allows, equal to IncomingCap + // (said only where no binding is read); transfers keeps for good, the holder dropping an idle one by its key + // in idle, a state of keys with their own time to live. + var req struct { + Requires []struct { + Name string `json:"name"` + Options map[string]any `json:"options"` + } `json:"requires"` + } + _ = json.Unmarshal(raw, &req) + opts := map[string]map[string]any{} + for _, r := range req.Requires { + opts[r.Name] = r.Options + } + if mib, _ := opts[IncomingObjects]["max-mib"].(float64); uint64(mib)<<20 != IncomingCap { + t.Errorf("incoming asks %v MiB, IncomingCap is %d bytes: want them equal", opts[IncomingObjects]["max-mib"], uint64(IncomingCap)) + } + if _, ok := opts[TransfersState]["ttl-seconds"]; ok { + t.Errorf("transfers has a lifetime %v: the bus would drop a transfer's key and leave its parts", opts[TransfersState]) + } + if opts[IdleState]["key-ttl"] != true { + t.Errorf("idle's options %v: want key-ttl, each transfer's key ending on its own", opts[IdleState]) + } }) } diff --git a/modules/distribution/internal/holder/transfer.go b/modules/distribution/internal/holder/transfer.go index 6df69f08..8882cf01 100644 --- a/modules/distribution/internal/holder/transfer.go +++ b/modules/distribution/internal/holder/transfer.go @@ -12,16 +12,22 @@ import ( "git.novox.be/novox/mesh-sdk/go/parts" ) -// The bounds of incoming (design 51 *Publishing*, ADR 0611): a part is at most 1 MiB, the store 20 GiB, a transfer -// with no part for 24 hours is dropped, and past 80 % of the cap the holder writes a finding. +// The bounds of incoming (design 51 *Publishing*, ADR 0611): a part is at most 1 MiB, a transfer with no part for 24 +// hours is dropped, and past 80 % of the cap the holder writes a finding. The cap is the one incoming's binding carries +// (the bus's own bound of the object store, read through the runtime); IncomingCap is the manifest's 8 GiB, the most +// the objects template allows, said only where no binding is read (a test), and the manifest's test holds the two equal. const ( MaxPart = 1 << 20 - IncomingCap = 20 << 30 + IncomingCap = 8 << 30 TransferIdle = 24 * time.Hour IncomingFullPct = 80 // IncomingObjects and TransfersState are the object store and the state distribution asks as bus state. IncomingObjects = "incoming" TransfersState = "transfers" + // IdleState is the state of one key per open transfer with its own time to live (a state declared key-ttl): the + // bus's marker of its end is the event that drops a transfer left with no part for TransferIdle (ADR 0611 §3), never + // a timer. + IdleState = "idle" ) // The kinds a seat holds. @@ -156,8 +162,10 @@ type Holder struct { Say Say Finding Finding Now func() time.Time - // Cap is incoming's cap; IncomingCap when zero. - Cap uint64 + // Cap reads incoming's cap from its binding; IncomingCap when nil or when the binding names none. + Cap func() (uint64, error) + // Idle writes a transfer's idle key with its own time to live, once; one already there is no fault. Nil arms nothing. + Idle func(id string, ttl time.Duration) error } func (h *Holder) now() time.Time { @@ -167,13 +175,18 @@ func (h *Holder) now() time.Time { return time.Now().UTC() } -func (h *Holder) cap() uint64 { - if h.Cap > 0 { - return h.Cap +// CapNow is incoming's cap as its binding carries it. +func (h *Holder) CapNow() uint64 { + if h.Cap != nil { + if c, err := h.Cap(); err == nil && c > 0 { + return c + } } return IncomingCap } +func (h *Holder) cap() uint64 { return h.CapNow() } + // receiver binds mesh-sdk's receiving side to distribution's incoming, its transfers state, the builder's check, its // store and its seats' events. func (h *Holder) receiver() *parts.Receiver[Opening] { @@ -255,6 +268,13 @@ func (h *Holder) Publish(seat, caller string, r PublishRequest) (PublishAnswer, if err != nil { return answerOf(got), fmt.Errorf("publish: %w", err) } + // An open transfer is bounded by its idle key; a key that cannot be written fails the publish loudly, and the + // build module's next publish of the same transfer answers the parts held and arms it again. + if !got.Ended && h.Idle != nil { + if err := h.Idle(r.Transfer, TransferIdle); err != nil { + return answerOf(got), fmt.Errorf("publish: transfer %s is open but its idle bound is not kept, so it would never be dropped: %w", r.Transfer, err) + } + } return answerOf(got), nil } @@ -283,10 +303,34 @@ func (h *Holder) eventOf(seat string, r PublishRequest, ref, reason string) Even // Drop drops a transfer by hand (registry_incoming) or because it was idle for TransferIdle: its chunks and its key. func (h *Holder) Drop(id string) (Transfer, error) { return h.receiver().Drop(id) } -// DropIdle drops every transfer with no part for TransferIdle, answering them; it runs on each publish's open and each -// registry_incoming call, never on a timer (the object store's own lifetime of 24 hours drops the bytes too). +// DropIdle drops every transfer with no part for TransferIdle, answering them: registry_incoming's look, by hand. The +// drop itself follows each transfer's idle key (Expired); the objects template keeps for good, so nothing else does. func (h *Holder) DropIdle() ([]string, error) { return h.receiver().DropIdle() } +// Expired is told that a transfer's idle key ended (the bus's marker): a transfer with no part for TransferIdle is +// dropped, its chunks with it; one that took a part since is armed again for the rest of its bound; one already gone is +// nothing to do. It answers whether it dropped the transfer. +func (h *Holder) Expired(id string) (bool, error) { + t, found, err := h.Transfers.Get(id) + if err != nil { + return false, fmt.Errorf("transfer %s's idle bound ended, but it cannot be read: %w", id, err) + } + if !found { + return false, nil + } + idle := h.now().Sub(t.Last) + if idle >= TransferIdle { + if _, err := h.Drop(id); err != nil { + return false, fmt.Errorf("transfer %s had no part for %s and is not dropped: %w", id, TransferIdle, err) + } + return true, nil + } + if h.Idle == nil { + return false, nil + } + return false, h.Idle(id, max(TransferIdle-idle, time.Second)) +} + func checkPublish(seat string, r PublishRequest) error { switch { case !slices.Contains(Seats, seat): diff --git a/modules/distribution/module.json b/modules/distribution/module.json index f3e98583..8f7e20dc 100644 --- a/modules/distribution/module.json +++ b/modules/distribution/module.json @@ -83,9 +83,14 @@ { "name": "transfers", "storage": "key-value", + "template": "state" + }, + { + "name": "idle", + "storage": "key-value", "template": "state", "options": { - "ttl-seconds": 86400 + "key-ttl": true } }, { From 8f5611a2d1905971dfd567cae29c9947a6f364a7 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:10:09 +0100 Subject: [PATCH 05/11] F-catalog-B: registry_mirrors reads distribution's mirrors, asks one again and drops one by hand The state mirrors had no read tool and no ask-again (seams review note 1): a copy a follower holds and does not end could only wait for a restart. registry_mirrors lists each kept copy and its claim, asks one again with its claim let go, and drops one, saying refused under the ask's id so its waiter ends. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/distribution-tools/main.go | 48 +++++++++++++++++ .../internal/holder/holder_test.go | 26 +++++++++ modules/distribution/internal/holder/verbs.go | 53 +++++++++++++++++++ modules/distribution/module.json | 1 + 4 files changed, 128 insertions(+) diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index 17b9ec2c..98603529 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -230,6 +230,28 @@ func (mirrors) Claim(key string, r holder.MirrorRequest, revision uint64) (bool, return err == nil, err } func (mirrors) Delete(key string) error { return stdio.State(holder.MirrorsState).Delete(key) } +func (mirrors) List() (map[string]holder.MirrorRequest, error) { + keys, err := stdio.State(holder.MirrorsState).Keys() + if err != nil { + return nil, err + } + out := map[string]holder.MirrorRequest{} + for _, k := range keys { + e, err := stdio.State(holder.MirrorsState).Get(k) + if err != nil { + return nil, err + } + var r holder.MirrorRequest + if e != nil && json.Unmarshal(e.Value, &r) == nil { + out[k] = r + } + } + return out, nil +} +func (mirrors) Put(key string, r holder.MirrorRequest) error { + _, err := stdio.State(holder.MirrorsState).Put(key, r) + return err +} // followMirrors runs every copy `mirror` took on, on the state's watch: the asks kept when the bundle starts (a copy a // restart cut) and each new one, one at a time. Each is claimed first (Verbs.Take), so a second follower copies none @@ -447,6 +469,32 @@ func tools(v *holder.Verbs, windows kept[holder.Window], callers func() (holder. used, _ := v.Objects.Used() return map[string]any{"transfers": all, "used": used, "cap": v.CapNow(), "dropped_idle": idle}, err }}, + stdio.Tool{Name: "registry_mirrors", Description: "The copies of outside images mirror took on and has not ended (the state mirrors): each by its key, its source, digest, who asked and the follower that claimed it (none: it waits). With again, asks one again by hand, its claim let go so a follower takes it on; with drop, drops one by hand, saying refused on the image seat's events so its waiter ends (each a hand act, why required).", + Input: obj(map[string]any{"again": s("a kept copy's key to ask again"), "drop": s("a kept copy's key to drop"), "why": s("why (a hand act)")}), + Run: func(args map[string]any) (any, error) { + by, why := stdio.CallerOf(args), str(args, "why") + switch { + case str(args, "again") != "": + if why == "" { + return nil, errors.New("asking a copy again is a hand act: say why") + } + r, err := v.MirrorAgain(str(args, "again")) + if err != nil { + return nil, err + } + logf("the copy %s asked again by hand by %s: %s", str(args, "again"), by, why) + return map[string]any{"asked_again": str(args, "again"), "copy": r}, nil + case str(args, "drop") != "": + r, err := v.MirrorDrop(str(args, "drop"), by, why) + if err != nil { + return nil, err + } + logf("the copy %s dropped by hand by %s: %s", str(args, "drop"), by, why) + return map[string]any{"dropped": str(args, "drop"), "copy": r}, nil + } + all, err := v.MirrorsNow() + return map[string]any{"mirrors": all}, err + }}, stdio.Tool{Name: "registry_holds", Description: "What is held, since when and for whom; with release, lets one go through the seat's release as the operator's session (a hand act).", Input: obj(map[string]any{"seat": s("the seat"), "release": s("the name to release"), "digest": s("its digest"), "why": s("why")}), Run: func(args map[string]any) (any, error) { diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index 0b384a03..f3f11868 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -159,6 +159,14 @@ func (m memMirrors) Claim(k string, r MirrorRequest, rev uint64) (bool, error) { return true, nil } func (m memMirrors) Delete(k string) error { delete(m, k); return nil } +func (m memMirrors) List() (map[string]MirrorRequest, error) { + out := map[string]MirrorRequest{} + for k, r := range m { + out[k] = r + } + return out, nil +} +func (m memMirrors) Put(k string, r MirrorRequest) error { m[k] = r; return nil } // mirrorRev is a kept ask's revision in memMirrors: 1 unclaimed, 2 once claimed. func mirrorRev(r MirrorRequest) uint64 { @@ -646,6 +654,24 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { t.Fatalf("two followers on one ask said %+v (ask id %q), asks %v: want one copy, said once under the ask's id", b.said[copies:], ask.ID, asked) } b.said = b.said[:copies] + // registry_mirrors (seams review note 1): a kept copy is read, asked again with its claim let go, and dropped + // by hand, which says refused under the ask's id with who and why. + _, _ = b.v.Mirror(ImageSeat, "novox.mesh-build-image", MirrorRequest{Source: "nats", Digest: d}) + asked[key] = MirrorRequest{Source: asked[key].Source, Digest: d, ID: asked[key].ID, Claim: "cut-follower"} + if now, err := b.v.MirrorsNow(); err != nil || now[key].Claim != "cut-follower" { + t.Fatalf("MirrorsNow = %v, %v; want the claimed ask", now, err) + } + if r, err := b.v.MirrorAgain(key); err != nil || r.Claim != "" || asked[key].Claim != "" { + t.Errorf("MirrorAgain = %+v, %v (kept %+v); want the claim let go", r, err, asked[key]) + } + if _, err := b.v.MirrorDrop(key, "person.jochen", ""); err == nil { + t.Error("a drop with no why is taken") + } + if _, err := b.v.MirrorDrop(key, "person.jochen", "upstream gone"); err != nil || len(asked) != 0 || + b.said[copies].event != EventRefused || b.said[copies].body.ID == "" || !strings.Contains(b.said[copies].body.Reason, "dropped by hand by person.jochen: upstream gone") { + t.Errorf("MirrorDrop = %v, said %+v, asks %v; want refused said under the ask's id and the ask gone", err, b.said[copies:], asked) + } + b.said = b.said[:copies] b.cat.mirrorErr = errors.New("the upstream said 404") _, _ = b.v.Mirror(ImageSeat, "novox.mesh-build-image", MirrorRequest{Source: "nats", Digest: d}) if err := b.v.Mirrored(asked[key]); err != nil || b.said[1].event != EventRefused || !strings.Contains(b.said[1].body.Reason, "404") || len(asked) != 0 { diff --git a/modules/distribution/internal/holder/verbs.go b/modules/distribution/internal/holder/verbs.go index 31af720b..095da21d 100644 --- a/modules/distribution/internal/holder/verbs.go +++ b/modules/distribution/internal/holder/verbs.go @@ -46,6 +46,10 @@ type Mirrors interface { // writer wrote the key first. Claim(key string, r MirrorRequest, revision uint64) (bool, error) Delete(key string) error + // List is every ask kept, by its key: what registry_mirrors reads. + List() (map[string]MirrorRequest, error) + // Put writes r under key whatever it held: an ask asked again by hand, its claim let go. + Put(key string, r MirrorRequest) error } // HoldState is distribution's own state holds (bus state, template state; ADR 0617 §2). @@ -353,6 +357,55 @@ func (v *Verbs) Take(key string, r MirrorRequest, revision uint64, current bool, return v.Mirrored(r) } +// MirrorsNow lists every copy `mirror` took on and has not ended, by its key, for registry_mirrors: what waits (no +// claim) and what a follower took on (its claim). +func (v *Verbs) MirrorsNow() (map[string]MirrorRequest, error) { return v.Mirrors.List() } + +// MirrorAgain asks a kept copy again by hand (registry_mirrors, a hand act): its claim is let go, so the state's watch +// hands it to a follower, which takes it on as a new ask. A copy whose follower was cut, or that a follower holds and +// does not end, goes on this way without a restart. +func (v *Verbs) MirrorAgain(key string) (MirrorRequest, error) { + all, err := v.Mirrors.List() + if err != nil { + return MirrorRequest{}, fmt.Errorf("mirror: the asks cannot be read: %w", err) + } + r, ok := all[key] + if !ok { + return MirrorRequest{}, fmt.Errorf("mirror: no copy is kept under %s", key) + } + r.Claim = "" + if err := v.Mirrors.Put(key, r); err != nil { + return MirrorRequest{}, fmt.Errorf("mirror: %s is not asked again: %w", key, err) + } + return r, nil +} + +// MirrorDrop drops a kept copy by hand (registry_mirrors, a hand act with who and why): it says refused on the image +// seat's events with that reason, under the ask's id, so a waiter on the copy ends, and the ask leaves the state. +func (v *Verbs) MirrorDrop(key, by, why string) (MirrorRequest, error) { + if strings.TrimSpace(why) == "" { + return MirrorRequest{}, errors.New("mirror: a drop is a hand act: say why") + } + all, err := v.Mirrors.List() + if err != nil { + return MirrorRequest{}, fmt.Errorf("mirror: the asks cannot be read: %w", err) + } + r, ok := all[key] + if !ok { + return MirrorRequest{}, fmt.Errorf("mirror: no copy is kept under %s", key) + } + src, err := ParseSource(r.Source) + if err != nil { + return MirrorRequest{}, err + } + e := Event{Name: src.Name(), Kind: KindUpstream, Digest: r.Digest, Holder: Self, By: by, ID: r.ID, + Reason: fmt.Sprintf("%s@%s was not copied: dropped by hand by %s: %s", r.Source, r.Digest, by, why)} + if err := v.Say(ImageSeat, EventRefused, e); err != nil { + return MirrorRequest{}, fmt.Errorf("mirror: the drop of %s cannot be said, so the ask stays: %w", key, err) + } + return r, v.Mirrors.Delete(key) +} + // HeldNow lists the holds, for registry_holds. func (v *Verbs) HeldNow() ([]Hold, error) { all, err := v.Holds.List() diff --git a/modules/distribution/module.json b/modules/distribution/module.json index 8f7e20dc..bf8c4f4b 100644 --- a/modules/distribution/module.json +++ b/modules/distribution/module.json @@ -48,6 +48,7 @@ "registry_health", "registry_holds", "registry_incoming", + "registry_mirrors", "registry_old_name" ], "invokes": [ From bccaf75556ceddbd8a099219449040a6ce2cfd96 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:14:04 +0100 Subject: [PATCH 06/11] F-catalog-B: npm_incoming and go_incoming read and drop the package seats' transfers; an idle one drops by its key The package seats kept incoming, transfers and findings with no read or drop tool (tools review F4). npm_incoming and go_incoming list a seat's open transfers, incoming's fill and the cap its binding carries, and drop one by hand. The same fault as distribution's (running review F2, its second instance): the transfers state's own lifetime dropped a transfer's key and left its chunks, and the drop otherwise waited for the next publish. transfers keeps for good; each open transfer has a key in the new state idle with its own time to live, whose end drops it. The holder counts against its binding's cap. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- modules/gitea/README.md | 2 + .../gitea/cmd/package-registry/incoming.go | 105 ++++++++++++++++++ modules/gitea/cmd/package-registry/main.go | 11 +- .../cmd/package-registry/retention_test.go | 12 +- modules/gitea/cmd/package-registry/seats.go | 6 +- .../whole_integration_test.go | 21 +++- modules/gitea/internal/packageseats/holder.go | 91 +++++++++++++-- .../internal/packageseats/holder_test.go | 42 +++++++ modules/gitea/module.json | 7 +- 9 files changed, 276 insertions(+), 21 deletions(-) create mode 100644 modules/gitea/cmd/package-registry/incoming.go diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 5b0f8934..75221eb7 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -208,6 +208,8 @@ deletes the versions nothing keeps. | `npm_retention` | a | the versions retention would delete, and the bytes. **A dry run unless `dry_run` is false**; a real run needs `why` | | `go_packages` | r | every Go module of the registry's owner (the Go seat's), each version newest first with its publish time, its size and why it is kept: a `go.mod` on a default branch requires it, or it is among the newest `keep` | | `go_retention` | a | the Go module versions retention would delete, and the bytes; as `npm_retention`, and a `go.mod` that could not be read stops a real run. Each deletion says `removed` on the Go seat | +| `npm_incoming` | a | the npm seat's open transfers in the module's object store `incoming`, its fill and the cap its binding carries; with `drop` and `why`, drops one by hand. A transfer with no chunk for 24 hours is dropped on its own, when its key in the state `idle` ends | +| `go_incoming` | a | the same for the Go seat | **A version is kept when:** diff --git a/modules/gitea/cmd/package-registry/incoming.go b/modules/gitea/cmd/package-registry/incoming.go new file mode 100644 index 00000000..eda80b2a --- /dev/null +++ b/modules/gitea/cmd/package-registry/incoming.go @@ -0,0 +1,105 @@ +package main + +// The package seats' incoming, read and steered (design 51 *Publishing*; tools review F4): npm_incoming and +// go_incoming list a seat's open transfers, incoming's fill and the cap its binding carries, and drop one by hand. +// The holder counts against that cap, never a number of its own, and drops a transfer left with no chunk for 24 hours +// when its idle key ends (the bus's marker), never on a timer. + +import ( + "errors" + "fmt" + "os" + "strings" + "sync/atomic" + "time" + + stdio "git.novox.be/novox/mesh-sdk/go" + "git.novox.be/novox/mesh-sdk/go/busstate" + + "gitea/internal/packageseats" +) + +// incomingTool is a seat's _incoming. +func incomingTool(typ, seat string, holder func() (*packageseats.Holder, error)) stdio.Tool { + str := func(args map[string]any, k string) string { s, _ := args[k].(string); return strings.TrimSpace(s) } + return stdio.Tool{ + Name: typ + "_incoming", + Description: "The " + seat + " seat's open transfers in the forge module's object store incoming (each with its " + + "opener, its parts and when it last took a chunk), incoming's fill and the cap its binding carries; a " + + "transfer with no chunk for 24 hours is dropped on its own. With drop, drops one by hand, its chunks with " + + "it (a hand act, why required). (a)", + Input: map[string]any{"type": "object", "properties": map[string]any{ + "drop": map[string]any{"type": "string", "description": "a transfer id (the caller's own name) to drop"}, + "why": map[string]any{"type": "string", "description": "why it is dropped"}, + }}, + Run: func(args map[string]any) (any, error) { + h, err := holder() + if err != nil { + return nil, err + } + if id := str(args, "drop"); id != "" { + if str(args, "why") == "" { + return nil, errors.New("a drop is a hand act: say why") + } + t, err := h.Drop(seat, id) + if err != nil { + return nil, err + } + fmt.Fprintf(os.Stderr, "transfer %s of %s dropped by hand by %s: %s\n", id, seat, stdio.CallerOf(args), str(args, "why")) + return map[string]any{"dropped": t.ID}, nil + } + return h.IncomingOf(seat) + }, + } +} + +// incomingCap reads incoming's cap from its binding (the bus's bound of the object store), once it answers one. +func incomingCap() func() (uint64, error) { + var known atomic.Uint64 + return func() (uint64, error) { + if c := known.Load(); c > 0 { + return c, nil + } + f, err := busstate.ObjectsOf(packageseats.IncomingObjects).Fill() + if err != nil { + return 0, fmt.Errorf("incoming's cap cannot be read from its binding: %w", err) + } + if f.MaxBytes == 0 { + return 0, errors.New("incoming's binding carries no cap") + } + known.Store(f.MaxBytes) + return f.MaxBytes, nil + } +} + +// armIdle writes a transfer's idle key with its own time to live; one already there is the same bound, kept. +func armIdle(key string, ttl time.Duration) error { + if _, err := stdio.State(packageseats.IdleState).CreateTTL(key, map[string]any{"transfer": key}, ttl); err != nil && !errors.Is(err, stdio.ErrChanged) { + return err + } + return nil +} + +// followIdle drops a transfer when its idle key ends (packageseats.Holder.Expired): the bus's marker is the event. +func followIdle(holder func() (*packageseats.Holder, error)) { + <-stdio.Served() + stdio.State(packageseats.IdleState).Follow("", func(ch stdio.StateChange) error { + if ch.Op != stdio.OpExpire { + return nil + } + h, err := holder() + if err != nil { + fmt.Fprintf(os.Stderr, "transfer %s's idle bound ended, but the holder cannot be made: %v\n", ch.Key, err) + return err + } + dropped, err := h.Expired(ch.Key) + if err != nil { + fmt.Fprintln(os.Stderr, err) + return err + } + if dropped { + fmt.Fprintf(os.Stderr, "transfer %s had no chunk for %s: dropped with its chunks\n", ch.Key, packageseats.TransferIdle) + } + return nil + }) +} diff --git a/modules/gitea/cmd/package-registry/main.go b/modules/gitea/cmd/package-registry/main.go index 9f1ca0fc..0869cfca 100644 --- a/modules/gitea/cmd/package-registry/main.go +++ b/modules/gitea/cmd/package-registry/main.go @@ -21,11 +21,13 @@ import ( // ToolNames are the tools this bundle serves. The module's manifest lists no `tools`: its TypeScript // bundle's tools are served without one, and a claim without `serves` would offer a `tools` list to both // of the module's seats as their verbs. The README names these two, and a test holds the two together. -var ToolNames = []string{"go_packages", "go_retention", "npm_packages", "npm_retention"} +var ToolNames = []string{"go_packages", "go_retention", "go_incoming", "npm_packages", "npm_retention", "npm_incoming"} func main() { forge := func() (*Forge, error) { return ForgeFromEnv() } - tools := append(Tools(forge), SeatTools(func() (*packageseats.Holder, error) { return busHolder(forge) })...) + holder := func() (*packageseats.Holder, error) { return busHolder(forge) } + go followIdle(holder) + tools := append(Tools(forge, holder), SeatTools(holder)...) if err := stdio.Serve("", tools); err != nil { fmt.Fprintln(os.Stderr, err) os.Exit(1) @@ -34,8 +36,9 @@ func main() { // Tools are the bundle's tools over the forge the function gives; asked per call, so a manifest that // does not set the environment is a refusal naming what is missing, not a bundle that will not start. -func Tools(forge func() (*Forge, error)) []stdio.Tool { - return append(typeTools(TypeGo, forge), typeTools(TypeNpm, forge)...) +func Tools(forge func() (*Forge, error), holder func() (*packageseats.Holder, error)) []stdio.Tool { + out := append(typeTools(TypeGo, forge), incomingTool(TypeGo, packageseats.GoSeat, holder)) + return append(append(out, typeTools(TypeNpm, forge)...), incomingTool(TypeNpm, packageseats.NpmSeat, holder)) } // typeTools are one package type's two tools, _packages and _retention, over the forge reading that type. diff --git a/modules/gitea/cmd/package-registry/retention_test.go b/modules/gitea/cmd/package-registry/retention_test.go index 7bbdbbd1..092d3dc1 100644 --- a/modules/gitea/cmd/package-registry/retention_test.go +++ b/modules/gitea/cmd/package-registry/retention_test.go @@ -402,12 +402,12 @@ func TestAWrongPasswordIsAFailureThatDoesNotSayThePassword(t *testing.T) { } func TestToolsAreTheOnesTheReadmeNamesAndSayWhatTheyDo(t *testing.T) { - tools := Tools(func() (*Forge, error) { return nil, fmt.Errorf("unset") }) + tools := Tools(func() (*Forge, error) { return nil, fmt.Errorf("unset") }, func() (*packageseats.Holder, error) { return nil, fmt.Errorf("unset") }) var served []string for _, tl := range tools { served = append(served, tl.Name) suffix := "(r)" - if strings.HasSuffix(tl.Name, "_retention") { + if strings.HasSuffix(tl.Name, "_retention") || strings.HasSuffix(tl.Name, "_incoming") { suffix = "(a)" } if !strings.HasSuffix(tl.Description, suffix) { @@ -468,11 +468,15 @@ func TestToolsAreTheOnesTheReadmeNamesAndSayWhatTheyDo(t *testing.T) { t.Errorf("the seat tools are %v", names) } req, _ := json.Marshal(m.Requires) - for _, want := range []string{`"name":"incoming"`, `"name":"transfers"`, `"name":"findings"`} { + for _, want := range []string{`"name":"incoming"`, `"name":"transfers"`, `"name":"findings"`, `"name":"idle","options":{"key-ttl":true}`} { if !strings.Contains(string(req), want) { - t.Errorf("the manifest does not require %s", want) + t.Errorf("the manifest does not require %s (%s)", want, req) } } + // transfers keeps for good: a lifetime of its own would drop a transfer's key and leave its chunks in incoming. + if strings.Contains(string(req), `"name":"transfers","options"`) { + t.Errorf("transfers has options (a lifetime): %s", req) + } // The retention asks the builder's versions; the forge's other bundles ask the indexer's modules and record a re-read. if strings.Join(m.Invokes, ",") != "mesh-delivery-indexer.indexer_read_modules,seat:mesh-hand-acts.hand-act,mesh-delivery-builder.builder_versions" { t.Errorf("the manifest invokes %v", m.Invokes) diff --git a/modules/gitea/cmd/package-registry/seats.go b/modules/gitea/cmd/package-registry/seats.go index 759bdfad..6447791f 100644 --- a/modules/gitea/cmd/package-registry/seats.go +++ b/modules/gitea/cmd/package-registry/seats.go @@ -263,6 +263,9 @@ func finding(kind, subject, words string, raised bool) { } } +// boundCap is incoming's cap from its binding, read once per bundle. +var boundCap = incomingCap() + // busHolder is the holder over the runtime's bus state, the builder's numbering and the forge. func busHolder(forge func() (*Forge, error)) (*packageseats.Holder, error) { callers, err := packageseats.ConfiguredCallers() @@ -271,7 +274,8 @@ func busHolder(forge func() (*Forge, error)) (*packageseats.Holder, error) { } return &packageseats.Holder{Callers: callers, Objects: parts.BusObjects(packageseats.IncomingObjects), Transfers: parts.BusTransfers[packageseats.Opening](packageseats.TransfersState), - Builder: delivery.BuilderVersions{Ask: stdio.Ask}, Store: seatStore{forge}, Say: say, Finding: finding}, nil + Builder: delivery.BuilderVersions{Ask: stdio.Ask}, Store: seatStore{forge}, Say: say, Finding: finding, + Cap: boundCap, Idle: armIdle}, nil } // SeatTools are both seats' verbs as tools .. diff --git a/modules/gitea/cmd/package-registry/whole_integration_test.go b/modules/gitea/cmd/package-registry/whole_integration_test.go index 9085bd12..401adb78 100644 --- a/modules/gitea/cmd/package-registry/whole_integration_test.go +++ b/modules/gitea/cmd/package-registry/whole_integration_test.go @@ -114,6 +114,7 @@ func (p *player) handle(m wire) { Subject string `json:"subject"` Body json.RawMessage `json:"body"` Kind string `json:"kind"` + TTL int `json:"ttl_seconds"` } _ = json.Unmarshal(m.Params, &a) switch { @@ -131,7 +132,7 @@ func (p *player) handle(m wire) { case strings.HasPrefix(m.Method, "mesh/objects."): p.objects(m, a.Objects, a.Name, a.Data) case strings.HasPrefix(m.Method, "mesh/state."): - p.state(m, a.State, a.Key, a.Value) + p.state(m, a.State, a.Key, a.Value, a.TTL) default: p.answer(m.ID, nil, errors.New("not played here: "+m.Method)) } @@ -172,7 +173,7 @@ func (p *player) objects(m wire, local, name string, data []byte) { } } -func (p *player) state(m wire, state, key string, value json.RawMessage) { +func (p *player) state(m wire, state, key string, value json.RawMessage, ttl int) { kv, err := p.js.KeyValue(p.ctx, "gitea_"+state) if err != nil { p.answer(m.ID, nil, err) @@ -193,6 +194,13 @@ func (p *player) state(m wire, state, key string, value json.RawMessage) { case "mesh/state.put": rev, err := kv.Put(p.ctx, key, value) p.answer(m.ID, map[string]any{"revision": rev}, err) + case "mesh/state.create": + var opts []jetstream.KVCreateOpt + if ttl > 0 { + opts = append(opts, jetstream.KeyTTL(time.Duration(ttl)*time.Second)) + } + rev, err := kv.Create(p.ctx, key, value, opts...) + p.answer(m.ID, map[string]any{"revision": rev}, err) case "mesh/state.delete": p.answer(m.ID, map[string]any{}, kv.Delete(p.ctx, key)) case "mesh/state.keys": @@ -268,6 +276,10 @@ func TestThePackageSeatsWhole(t *testing.T) { t.Fatal(err) } } + // idle takes keys with their own time to live (key-ttl), as the bus module makes it. + if _, err := js.CreateKeyValue(ctx, jetstream.KeyValueConfig{Bucket: "gitea_" + packageseats.IdleState, LimitMarkerTTL: time.Minute}); err != nil { + t.Fatal(err) + } if _, err := js.CreateObjectStore(ctx, jetstream.ObjectStoreConfig{Bucket: "gitea_" + packageseats.IncomingObjects}); err != nil { t.Fatal(err) } @@ -364,6 +376,11 @@ func TestThePackageSeatsWhole(t *testing.T) { if transfer == "" || len(lacks) != 1 || lacks[0] != digest { t.Fatalf("publish answered transfer %q lacks %v", transfer, lacks) } + // The open transfer is bounded by its idle key, written through the runtime with its own time to live. + idle, _ := js.KeyValue(ctx, "gitea_"+packageseats.IdleState) + if _, err := idle.Get(ctx, packageseats.GoSeat+"_"+transfer); err != nil { + t.Errorf("the open transfer %s has no idle key: %v", transfer, err) + } if err := send(transfer, digest, zipped); err != nil { t.Fatal(err) } diff --git a/modules/gitea/internal/packageseats/holder.go b/modules/gitea/internal/packageseats/holder.go index 7a3706a3..80ae121e 100644 --- a/modules/gitea/internal/packageseats/holder.go +++ b/modules/gitea/internal/packageseats/holder.go @@ -44,8 +44,8 @@ const ( ) // The bounds of incoming (design 51 *Publishing*, ADR 0611): a chunk at most 1 MiB, a transfer with no chunk for 24 -// hours dropped, a finding past 80 % of the cap. A package is one file, far smaller than an image, so the cap is the -// manifest's 2 GiB. +// hours dropped, a finding past 80 % of the cap. The cap is the one incoming's binding carries; IncomingCap is the +// manifest's 2 GiB (a package is one file, far smaller than an image), said only where no binding is read. const ( MaxChunk = 1 << 20 IncomingCap = 2 << 30 @@ -53,6 +53,9 @@ const ( IncomingFullPct = 80 IncomingObjects = "incoming" TransfersState = "transfers" + // IdleState is the state of one key per open transfer with its own time to live (a state declared key-ttl): its + // end is the event that drops a transfer left with no chunk for TransferIdle, never a timer. + IdleState = "idle" // KindPackage is the one kind a package seat holds. KindPackage = "package" // Self is the forge module, the holder every outcome names. @@ -151,7 +154,20 @@ type Holder struct { // Finding writes (raised) or clears the module's finding of a kind for a subject. Finding func(kind, subject, words string, raised bool) Now func() time.Time - Cap uint64 + // Cap reads incoming's cap from its binding; IncomingCap when nil or when the binding names none. + Cap func() (uint64, error) + // Idle writes a transfer's idle key with its own time to live, once; one already there is no fault. Nil arms nothing. + Idle func(key string, ttl time.Duration) error +} + +// CapNow is incoming's cap as its binding carries it. +func (h *Holder) CapNow() uint64 { + if h.Cap != nil { + if c, err := h.Cap(); err == nil && c > 0 { + return c + } + } + return IncomingCap } func (h *Holder) now() time.Time { @@ -168,10 +184,7 @@ func Reference(name, version string) string { return name + "@" + version } func key(seat, transfer string) string { return seat + "_" + transfer } func (h *Holder) receiver() *parts.Receiver[Opening] { - cap := h.Cap - if cap == 0 { - cap = IncomingCap - } + cap := h.CapNow() return &parts.Receiver[Opening]{Objects: h.Objects, Transfers: h.Transfers, Cap: cap, MaxChunk: MaxChunk, Idle: TransferIdle, Full: IncomingFullPct, Now: h.now, // The builder's numbering is asked before anything is uploaded (ADR 0605 §4). @@ -230,7 +243,6 @@ func (h *Holder) Publish(seat, caller string, p Publish) (Answer, error) { if err := checkPublish(seat, p); err != nil { return Answer{}, err } - _, _ = h.receiver().DropIdle() // A rebuild of a published version answers it held and uploads nothing; the same version with other content is // refused, as the registry would refuse it. if d, ok, err := h.Store.Held(seat, p.Name, p.Version); err != nil { @@ -253,6 +265,13 @@ func (h *Holder) Publish(seat, caller string, p Publish) (Answer, error) { if err != nil { return h.answer(p.Transfer, got), fmt.Errorf("publish: %w", err) } + // An open transfer is bounded by its idle key; one that cannot be written fails the publish loudly, and the build + // module's next publish of the same transfer answers the part held and arms it again. + if !got.Ended && h.Idle != nil { + if err := h.Idle(key(seat, p.Transfer), TransferIdle); err != nil { + return h.answer(p.Transfer, got), fmt.Errorf("publish: transfer %s is open but its idle bound is not kept, so it would never be dropped: %w", p.Transfer, err) + } + } return h.answer(p.Transfer, got), nil } @@ -324,11 +343,65 @@ func (h *Holder) Retained(seat, name, version, digest string) error { return h.Say(seat, EventRemoved, Outcome{Name: name, Version: version, Kind: KindPackage, Digest: digest, Reason: ReasonRetention}) } -// Drop drops a transfer by hand; DropIdle every transfer idle for TransferIdle. +// Drop drops a transfer by hand. func (h *Holder) Drop(seat, transfer string) (Transfer, error) { return h.receiver().Drop(key(seat, transfer)) } +// Incoming is what npm_incoming and go_incoming answer of a seat: its open transfers, incoming's fill (both seats +// share the store) and the cap its binding carries. +type Incoming struct { + Seat string `json:"seat"` + Transfers []Transfer `json:"transfers"` + Used uint64 `json:"used"` + Cap uint64 `json:"cap"` +} + +// IncomingOf reads a seat's open transfers and incoming's fill. +func (h *Holder) IncomingOf(seat string) (Incoming, error) { + if _, ok := Seats[seat]; !ok { + return Incoming{}, fmt.Errorf("the forge holds no seat %s", seat) + } + all, err := h.Transfers.List() + if err != nil { + return Incoming{}, fmt.Errorf("the transfers cannot be read: %w", err) + } + out := Incoming{Seat: seat, Transfers: []Transfer{}, Cap: h.CapNow()} + for _, t := range all { + if t.Shape.Seat == seat { + out.Transfers = append(out.Transfers, t) + } + } + if out.Used, err = h.Objects.Used(); err != nil { + return out, fmt.Errorf("incoming's fill cannot be read: %w", err) + } + return out, nil +} + +// Expired is told that a transfer's idle key (its key in the state transfers) ended: a transfer with no chunk for +// TransferIdle is dropped with its chunks; one that took a chunk since is armed again for the rest; one gone is +// nothing to do. It answers whether it dropped it. +func (h *Holder) Expired(id string) (bool, error) { + t, found, err := h.Transfers.Get(id) + if err != nil { + return false, fmt.Errorf("transfer %s's idle bound ended, but it cannot be read: %w", id, err) + } + if !found { + return false, nil + } + idle := h.now().Sub(t.Last) + if idle >= TransferIdle { + if _, err := h.receiver().Drop(id); err != nil { + return false, fmt.Errorf("transfer %s had no chunk for %s and is not dropped: %w", id, TransferIdle, err) + } + return true, nil + } + if h.Idle == nil { + return false, nil + } + return false, h.Idle(id, max(TransferIdle-idle, time.Second)) +} + func checkPublish(seat string, p Publish) error { switch { case Seats[seat] == "": diff --git a/modules/gitea/internal/packageseats/holder_test.go b/modules/gitea/internal/packageseats/holder_test.go index 41aeaae9..700001c0 100644 --- a/modules/gitea/internal/packageseats/holder_test.go +++ b/modules/gitea/internal/packageseats/holder_test.go @@ -149,6 +149,48 @@ func TestTheForgeHoldsBothPackageSeats(t *testing.T) { } } }) + // Tools review F4 and running review F2's kind: a seat's incoming is read per seat with the cap its binding + // carries; an open transfer is armed with its idle key, re-armed for the rest when it took a chunk since, and + // dropped with its chunks when it had none for 24 hours; a cap from the binding refuses a transfer past it. + t.Run("incoming: read per seat, the binding's cap, an idle transfer dropped when its key ends", func(t *testing.T) { + b := newBed(true) + now := time.Unix(1e9, 0) + b.h.Now = func() time.Time { return now } + armed := map[string]time.Duration{} + b.h.Idle = func(k string, ttl time.Duration) error { armed[k] = ttl; return nil } + b.h.Cap = func() (uint64, error) { return 1000, nil } + big := []byte(strings.Repeat("x", 600)) + if _, err := b.h.Publish(NpmSeat, nodeBuild, pub("a", "0.1.0", big)); err != nil { + t.Fatal(err) + } + if _, err := b.h.Publish(GoSeat, goBuild, pub("b", "0.1.0", big)); err != nil || armed["go-package-registry_t-b"] != TransferIdle { + t.Fatalf("publish = %v, armed %v; want the Go transfer's idle key for %s", err, armed, TransferIdle) + } + _, _ = b.h.PublishPart(NpmSeat, nodeBuild, "t-a", digest(big), 0, false, big[:500]) + in, err := b.h.IncomingOf(NpmSeat) + if err != nil || len(in.Transfers) != 1 || in.Transfers[0].Shape.Name != "a" || in.Cap != 1000 || in.Used != 500 { + t.Fatalf("IncomingOf(npm) = %+v, %v; want a's transfer alone, 500 used of the binding's 1000", in, err) + } + if _, err := b.h.Publish(NpmSeat, nodeBuild, pub("c", "0.1.0", big)); err == nil || !strings.Contains(err.Error(), "cap") { + t.Errorf("a transfer past the binding's cap = %v; want refused at the cap", err) + } + now = now.Add(time.Hour) + _, _ = b.h.PublishPart(GoSeat, goBuild, "t-b", digest(big), 0, false, big[:10]) + now = now.Add(TransferIdle - time.Hour) + if dropped, err := b.h.Expired("go-package-registry_t-b"); err != nil || dropped || armed["go-package-registry_t-b"] != time.Hour { + t.Errorf("b's key ends an hour after a chunk: %v, %v, armed %v; want kept, armed 1h", dropped, err, armed) + } + if dropped, err := b.h.Expired("npm-package-registry_t-a"); err != nil || !dropped { + t.Errorf("a's key ends 24 hours after its last chunk: %v, %v; want dropped", dropped, err) + } + if in, _ := b.h.IncomingOf(NpmSeat); len(in.Transfers) != 0 || in.Used != 10 { + t.Errorf("after a's drop: %+v; want no npm transfer and only b's 10 bytes used", in) + } + b.h.Idle = func(string, time.Duration) error { return errors.New("bus down") } + if _, err := b.h.Publish(NpmSeat, nodeBuild, pub("d", "0.1.0", file)); err == nil || !strings.Contains(err.Error(), "idle bound") { + t.Errorf("a publish whose idle key is not written = %v; want it failed loudly", err) + } + }) t.Run("refusals", func(t *testing.T) { for _, c := range []struct { name string diff --git a/modules/gitea/module.json b/modules/gitea/module.json index 0caba4c2..0c0e1095 100644 --- a/modules/gitea/module.json +++ b/modules/gitea/module.json @@ -17,9 +17,14 @@ { "name": "transfers", "storage": "key-value", + "template": "state" + }, + { + "name": "idle", + "storage": "key-value", "template": "state", "options": { - "ttl-seconds": 86400 + "key-ttl": true } }, { From 64c893e2c2edccead1b13a68a36470feed30c452 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:15:41 +0100 Subject: [PATCH 07/11] F-catalog-B: the gitea module removes the npm pair's teams packages and readers and their accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ADR 0605 §5 revokes the npm write token, and the 14:04 deferral is withdrawn (security review F5): the accounts the removed pair provisioner made (mesh__) still logged in with write on the organisation's packages. cmd/agent-teams, run at each start of the module and change of its teams value, removes both teams and each account of that form whose only teams they are, never purging: one the forge refuses to delete because it owns something keeps it and has its login prohibited. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- modules/gitea/README.md | 9 +- modules/gitea/cmd/agent-teams/main.go | 4 +- modules/gitea/cmd/agent-teams/retire.go | 107 ++++++++++++++++++++ modules/gitea/cmd/agent-teams/teams.go | 4 + modules/gitea/cmd/agent-teams/teams_test.go | 64 ++++++++++++ 5 files changed, 184 insertions(+), 4 deletions(-) create mode 100644 modules/gitea/cmd/agent-teams/retire.go diff --git a/modules/gitea/README.md b/modules/gitea/README.md index 75221eb7..de98b90e 100644 --- a/modules/gitea/README.md +++ b/modules/gitea/README.md @@ -13,8 +13,10 @@ trusts that root. On a machine with no public domain the key is left out, and th **Both package provisions carry no credential** (`credential.form` none, design 51 *Publishing*): reads of the organisation's packages are anonymous, and every write goes through the seats' verbs, so no consumer is given an -account of the forge. The registry accounts the TypeScript pair provisioner made before stay on the forge until -removed by hand; nothing hands their passwords out any more. +account of the forge. The registry accounts the TypeScript pair provisioner made before (`mesh__`) +and its two teams `packages` and `readers` are removed by `cmd/agent-teams` at each of its runs (ADR 0605 §5 revokes the +npm write token): both teams go, and each account of that form whose only teams they are; one the forge refuses to +delete because it owns something keeps its data and has its login prohibited. ## Announcing pull requests to the merge check (novox/hq to-be 45 §9, issue 347) @@ -142,7 +144,8 @@ teams exist and who is in them is the operator's, and it is one trusted config v whenever that file changes. For each team it makes `agent-read-` in the organisation of the first repository's owner, with permission read and no other right, its repositories exactly the listed ones and its members exactly the listed accounts (an account that does not exist yet is skipped and said). A team named `agent-read-*` that has left -the value is emptied, never deleted. A team without that prefix is never touched. There are no verbs: a change is a +the value is emptied, never deleted. A team without that prefix is never touched, but the two the module made once +and declares no more, `packages` and `readers` (above). There are no verbs: a change is a change of the value. It says each change in one line on stdout, and never a secret. `go test ./cmd/agent-teams` runs it against a fake forge. diff --git a/modules/gitea/cmd/agent-teams/main.go b/modules/gitea/cmd/agent-teams/main.go index eb92207c..a42d5c8a 100644 --- a/modules/gitea/cmd/agent-teams/main.go +++ b/modules/gitea/cmd/agent-teams/main.go @@ -2,7 +2,9 @@ // runtime runs once when the module starts and again whenever the module's config value `agent-read-teams` changes: // it reads the file the mesh renders from that value, and makes the forge's teams named agent-read-* match it. Each // team reads only; its repositories and its members (agent accounts, agent-) are exactly the value's. It -// touches nothing outside the value: no team without the prefix, no repository or account of another kind. +// touches nothing else of the value's kind: no team without the prefix, no repository or account of another kind. Beside +// the value it removes what the module made once and declares no more (retire.go): the npm pair's teams packages and +// readers and the accounts in them alone. // // It says each change on stdout in one line, and never a secret. package main diff --git a/modules/gitea/cmd/agent-teams/retire.go b/modules/gitea/cmd/agent-teams/retire.go new file mode 100644 index 00000000..8da05e44 --- /dev/null +++ b/modules/gitea/cmd/agent-teams/retire.go @@ -0,0 +1,107 @@ +package main + +import ( + "context" + "errors" + "fmt" + "net/http" + "net/url" + "regexp" + "slices" + "sort" + "strings" + + "gitea/internal/forgehttp" +) + +// The forge accounts and teams the module made once and declares no more (novox/hq ADR 0605 §5: no build module holds +// a registry's write credential; the npm write token is revoked). The npm pair provisioner, removed when the package +// seats came, made each consumer of npm-package-registry an account under the mesh's derived login +// (mesh__) and put it in the organisation's team packages (read and write) or readers (read only). +// Reads of the packages are anonymous and every write goes through the seats, so both teams go, and each account of +// that form in no team but them goes with them. Nothing else is touched: a team of another name, an account of +// another form or one still in a team the module does not retire stays as it is. +var RetiredTeams = []string{"packages", "readers"} + +// retiredAccount is the login form the npm pair provisioner made its accounts under. +var retiredAccount = regexp.MustCompile(`^mesh_[a-z0-9-]+_[a-z0-9_]+$`) + +// Retire removes the retired teams in every organisation, and each account of the pair's form whose only teams they +// are. An account the forge refuses to delete (it owns a repository or an organisation, so a delete would take what +// it owns) has its login prohibited instead, said, so its password no longer reaches anything. Each removal is said +// in one line; a run with nothing left to retire says nothing. +func (s *Syncer) Retire(ctx context.Context, orgs []string) error { + teamsOf := map[string][]string{} // login -> every team it is in, as org/team + type retired struct { + org string + team forgeTeam + } + var gone []retired + for _, org := range orgs { + teams, err := s.teams(ctx, org) + if err != nil { + return err + } + for _, t := range teams { + var members []forgeMember + if err := s.list(ctx, fmt.Sprintf("/api/v1/teams/%d/members", t.ID), &members); err != nil { + return fmt.Errorf("the members of %s in %s could not be read, so nothing is retired: %w", t.Name, org, err) + } + for _, m := range members { + teamsOf[m.Login] = append(teamsOf[m.Login], org+"/"+t.Name) + } + if slices.Contains(RetiredTeams, t.Name) { + gone = append(gone, retired{org, t}) + } + } + } + isRetired := func(orgTeam string) bool { + _, team, _ := strings.Cut(orgTeam, "/") + return slices.Contains(RetiredTeams, team) + } + var accounts []string + for login, in := range teamsOf { + if retiredAccount.MatchString(login) && slices.ContainsFunc(in, isRetired) && !slices.ContainsFunc(in, func(t string) bool { return !isRetired(t) }) { + accounts = append(accounts, login) + } + } + sort.Strings(accounts) + var errs []string + for _, a := range accounts { + if err := s.removeAccount(ctx, a); err != nil { + errs = append(errs, err.Error()) + } + } + for _, r := range gone { + if err := s.Forge.SendJSON(ctx, http.MethodDelete, fmt.Sprintf("/api/v1/teams/%d", r.team.ID), nil, nil); err != nil && !errors.Is(err, forgehttp.ErrNotFound) { + errs = append(errs, fmt.Sprintf("the retired team %s in %s could not be removed: %v", r.team.Name, r.org, err)) + continue + } + s.say("removed the retired team %s in %s (the npm pair's, ADR 0605 §5)", r.team.Name, r.org) + } + if len(errs) > 0 { + return errors.New(strings.Join(errs, "; ")) + } + return nil +} + +// removeAccount deletes a retired account, never purging what it owns; one the forge refuses to delete has its login +// prohibited instead. +func (s *Syncer) removeAccount(ctx context.Context, login string) error { + err := s.Forge.SendJSON(ctx, http.MethodDelete, "/api/v1/admin/users/"+url.PathEscape(login), nil, nil) + if err == nil || errors.Is(err, forgehttp.ErrNotFound) { + s.say("removed the retired account %s (the npm pair's, ADR 0605 §5)", login) + return nil + } + var se *forgehttp.StatusError + if !errors.As(err, &se) || se.Status != http.StatusUnprocessableEntity { + return fmt.Errorf("the retired account %s could not be removed: %w", login, err) + } + if perr := s.Forge.SendJSON(ctx, http.MethodPatch, "/api/v1/admin/users/"+url.PathEscape(login), map[string]any{ + "login_name": login, "source_id": 0, "prohibit_login": true, + }, nil); perr != nil { + return fmt.Errorf("the retired account %s could not be removed (%v) nor its login prohibited: %w", login, err, perr) + } + s.say("the retired account %s owns something, so the forge keeps it (%v): its login is prohibited instead", login, err) + return nil +} diff --git a/modules/gitea/cmd/agent-teams/teams.go b/modules/gitea/cmd/agent-teams/teams.go index 5f2a0b6e..2e7685c2 100644 --- a/modules/gitea/cmd/agent-teams/teams.go +++ b/modules/gitea/cmd/agent-teams/teams.go @@ -135,6 +135,10 @@ func (s *Syncer) Sync(ctx context.Context, v Value) error { } } } + if err := s.Retire(ctx, orgs); err != nil { + s.say("not retired: %v", err) + failed = append(failed, "the retired teams and accounts") + } if len(failed) > 0 { return fmt.Errorf("not applied: %s", strings.Join(failed, ", ")) } diff --git a/modules/gitea/cmd/agent-teams/teams_test.go b/modules/gitea/cmd/agent-teams/teams_test.go index 0dfe617c..a6800757 100644 --- a/modules/gitea/cmd/agent-teams/teams_test.go +++ b/modules/gitea/cmd/agent-teams/teams_test.go @@ -9,6 +9,7 @@ import ( "os" "path/filepath" "regexp" + "slices" "sort" "strconv" "strings" @@ -34,6 +35,9 @@ type fakeForge struct { users map[string]bool teams []*fTeam next int64 + // owners are accounts that own a repository: the forge refuses to delete them (422); prohibited, whose login + // was prohibited. + owners, prohibited map[string]bool } func newFake(orgs []string, users ...string) *fakeForge { @@ -122,6 +126,26 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { out = nil } reply(200, out) + case strings.HasPrefix(p, "/api/v1/admin/users/"): + u := strings.TrimPrefix(p, "/api/v1/admin/users/") + switch { + case !f.users[u]: + reply(404, nil) + case r.Method == http.MethodDelete && f.owners[u]: + reply(422, map[string]string{"message": "user still owns one or more repositories"}) + case r.Method == http.MethodDelete: + delete(f.users, u) + for _, t := range f.teams { + delete(t.Members, u) + } + reply(204, nil) + case r.Method == http.MethodPatch && in["prohibit_login"] == true: + if f.prohibited == nil { + f.prohibited = map[string]bool{} + } + f.prohibited[u] = true + reply(200, map[string]string{}) + } case strings.HasPrefix(p, "/api/v1/users/"): if !f.users[strings.TrimPrefix(p, "/api/v1/users/")] { reply(404, nil) @@ -155,6 +179,10 @@ func (f *fakeForge) ServeHTTP(w http.ResponseWriter, r *http.Request) { } } reply(200, out) + case teamPath.MatchString(p) && r.Method == http.MethodDelete: + t := f.find(teamPath.FindStringSubmatch(p)[1]) + f.teams = slices.DeleteFunc(f.teams, func(x *fTeam) bool { return x == t }) + reply(204, nil) case teamPath.MatchString(p): t := f.find(teamPath.FindStringSubmatch(p)[1]) t.Perm = in["permission"].(string) @@ -331,3 +359,39 @@ func TestReadValue(t *testing.T) { t.Fatal("an unknown field was accepted") } } + +// Security review F5 (ADR 0605 §5): the npm pair's teams packages and readers go, and each account of the pair's form +// whose only teams they are; one that owns a repository is kept with its login prohibited; an account of the pair's +// form still in another team, an account of another form and every other team stay; a second run changes nothing. +func TestTheNpmPairsTeamsAndAccountsAreRetired(t *testing.T) { + f := newFake([]string{"novox"}, "mesh_ace_mesh_build_node", "mesh_shanks_lab", "mesh_g14_agent", "mesh_novox_owner", + "mesh-packages", "agent-ace", "jschoubben") + f.owners = map[string]bool{"mesh_novox_owner": true} + f.add("novox", "packages", "none", nil, []string{"mesh_ace_mesh_build_node", "mesh_g14_agent", "mesh_novox_owner", "jschoubben"}) + f.add("novox", "readers", "none", nil, []string{"mesh_ace_mesh_build_node", "mesh_shanks_lab"}) + f.add("novox", "agents", "none", nil, []string{"mesh_g14_agent", "agent-ace"}) + f.add("novox", "mesh-packages", "none", nil, []string{"mesh-packages"}) + s, said := syncer(t, f) + if err := s.Sync(context.Background(), Value{}); err != nil { + t.Fatal(err) + } + if f.byName("novox", "packages") != nil || f.byName("novox", "readers") != nil { + t.Errorf("the retired teams stay: %+v", f.teams) + } + if f.byName("novox", "agents") == nil || f.byName("novox", "mesh-packages") == nil { + t.Errorf("a team the module did not retire was removed: %+v", f.teams) + } + for u, want := range map[string]bool{"mesh_ace_mesh_build_node": false, "mesh_shanks_lab": false, "mesh_g14_agent": true, + "mesh_novox_owner": true, "mesh-packages": true, "agent-ace": true, "jschoubben": true} { + if f.users[u] != want { + t.Errorf("account %s kept %v, want %v", u, f.users[u], want) + } + } + if !f.prohibited["mesh_novox_owner"] || len(f.prohibited) != 1 { + t.Errorf("prohibited %v, want mesh_novox_owner alone", f.prohibited) + } + before := len(*said) + if err := s.Sync(context.Background(), Value{}); err != nil || len(*said) != before { + t.Errorf("a second run: %v, said %v; want nothing", err, (*said)[before:]) + } +} From d8e43b48c467614dfdc5d557b5b3116d645b9410 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:17:54 +0100 Subject: [PATCH 08/11] F-catalog-B: no ignore pattern hides a module's source; nfs-server's and restic's patterns anchored A bare pattern meant for a binary (mesh-build-node) matched the command directory of the same name, so git ignored every new file there and publish.go was never committed (catalogue review F1). The kind: every bare pattern of a module's .gitignore is anchored to the binary (nfs-server's hid all of cmd/nfs-server), and checks/gitignored refuses an ignore pattern, the module's or the catalogue's, that hides a directory under a module's cmd/ or internal/, a Go file added there, or a source file there, in a git checkout and a plain folder alike. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- checks/gitignored/gitignored.go | 124 +++++++++++++++++++ checks/gitignored/gitignored_test.go | 80 ++++++++++++ checks/gitignored/go.mod | 3 + modules/nfs-server/.gitignore | 4 +- modules/restic/cmd/restic-backups/.gitignore | 3 +- 5 files changed, 212 insertions(+), 2 deletions(-) create mode 100644 checks/gitignored/gitignored.go create mode 100644 checks/gitignored/gitignored_test.go create mode 100644 checks/gitignored/go.mod diff --git a/checks/gitignored/gitignored.go b/checks/gitignored/gitignored.go new file mode 100644 index 00000000..e33f86af --- /dev/null +++ b/checks/gitignored/gitignored.go @@ -0,0 +1,124 @@ +// Package gitignored refuses an ignore pattern that hides a module's source (catalogue review F1, and its kind): the +// bare pattern `mesh-build-node` in a module's .gitignore, meant for the binary `go build` leaves, also matched the +// directory cmd/mesh-build-node, so git ignored every new file there and publish.go was never committed. A path under +// a module's cmd/ or internal/ is source: no .gitignore, the module's or the catalogue's, may ignore one, tracked or +// new. A binary is ignored by an anchored pattern (`/mesh-build-node`, `/cmd/mesh-build-node/mesh-build-node`). +package gitignored + +import ( + "bytes" + "fmt" + "io/fs" + "os" + "os/exec" + "path/filepath" + "sort" + "strings" +) + +// Probe is the file name asked of each source directory: a Go file a change might add there. +const Probe = "added_by_a_change.go" + +// Hidden is one source path an ignore pattern hides, and the pattern (its file, line and text, as git names it). +type Hidden struct { + Path, Pattern string +} + +// Sources are the source directories of root's modules, relative to root: every directory under modules/*/cmd and +// modules/*/internal (node_modules left out). +func Sources(root string) ([]string, error) { + mods, err := filepath.Glob(filepath.Join(root, "modules", "*")) + if err != nil { + return nil, err + } + var out []string + for _, m := range mods { + for _, top := range []string{"cmd", "internal"} { + base := filepath.Join(m, top) + if st, err := os.Stat(base); err != nil || !st.IsDir() { + continue + } + err := filepath.WalkDir(base, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if !d.IsDir() { + return nil + } + if d.Name() == "node_modules" { + return filepath.SkipDir + } + rel, _ := filepath.Rel(root, p) + out = append(out, filepath.ToSlash(rel)) + return nil + }) + if err != nil { + return nil, err + } + } + } + sort.Strings(out) + return out, nil +} + +// isSource is whether a file is source a module's build reads: Go and TypeScript files and a Go module's own files. +// What a build or a test script leaves beside them (a binary, compiled JavaScript) may be ignored. +func isSource(name string) bool { + switch filepath.Ext(name) { + case ".go", ".ts", ".tsx": + return true + } + return name == "go.mod" || name == "go.sum" +} + +// Find asks git which of the source paths root's ignore files hide: each source directory, a Go file added in it, and +// every source file there (isSource). It reads the .gitignore files of root's tree through a repository of its own, so it judges a git +// checkout and a plain folder alike, and no one's global ignores count. +func Find(root string, dirs []string) ([]Hidden, error) { + gitDir, err := os.MkdirTemp("", "gitignored-") + if err != nil { + return nil, err + } + defer os.RemoveAll(gitDir) + if out, err := exec.Command("git", "init", "-q", "--bare", gitDir).CombinedOutput(); err != nil { + return nil, fmt.Errorf("git init: %v: %s", err, out) + } + var ask bytes.Buffer + for _, d := range dirs { + ask.WriteString(d + "/\x00" + d + "/" + Probe + "\x00") + entries, err := os.ReadDir(filepath.Join(root, d)) + if err != nil { + return nil, err + } + for _, e := range entries { + if !e.IsDir() && isSource(e.Name()) { + ask.WriteString(d + "/" + e.Name() + "\x00") + } + } + } + abs, err := filepath.Abs(root) + if err != nil { + return nil, err + } + cmd := exec.Command("git", "--git-dir="+gitDir, "--work-tree="+abs, "-c", "core.excludesFile=/dev/null", + "check-ignore", "--no-index", "-v", "-z", "--stdin") + cmd.Dir = abs + cmd.Stdin = &ask + out, err := cmd.Output() + if ee, ok := err.(*exec.ExitError); ok && ee.ExitCode() == 1 { + return nil, nil // nothing is ignored + } + if err != nil { + return nil, fmt.Errorf("git check-ignore: %w", err) + } + // -v -z answers source, line, pattern and path, each ended by NUL. + f := strings.Split(strings.TrimRight(string(out), "\x00"), "\x00") + var hidden []Hidden + for i := 0; i+3 < len(f); i += 4 { + if f[i+2] == "" || strings.HasPrefix(f[i+2], "!") { + continue // a negation keeps the path + } + hidden = append(hidden, Hidden{Path: f[i+3], Pattern: fmt.Sprintf("%s:%s: %s", f[i], f[i+1], f[i+2])}) + } + return hidden, nil +} diff --git a/checks/gitignored/gitignored_test.go b/checks/gitignored/gitignored_test.go new file mode 100644 index 00000000..304e444f --- /dev/null +++ b/checks/gitignored/gitignored_test.go @@ -0,0 +1,80 @@ +package gitignored + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// The kind of catalogue review F1: a bare pattern hides the command directory it shares a name with; an anchored one +// hides the binary alone; a glob that ignores source is refused too; a negation keeps a path. +func TestFindNamesEachPatternThatHidesSource(t *testing.T) { + for name, c := range map[string]struct { + ignore string + hidden []string + }{ + "a bare name hides cmd/": {"nfs-server\n", []string{"modules/m/cmd/nfs-server/", "modules/m/cmd/nfs-server/" + Probe, "modules/m/cmd/nfs-server/main.go"}}, + "anchored to the binary": {"/nfs-server\n/cmd/nfs-server/nfs-server\n", nil}, + "a glob ignoring Go source": {"*.go\n!main.go\n", []string{"modules/m/cmd/" + Probe, "modules/m/cmd/nfs-server/" + Probe, + "modules/m/internal/" + Probe, "modules/m/internal/x/" + Probe, "modules/m/internal/x/x.go"}}, + "a pattern over internal/": {"internal/\n", []string{"modules/m/internal/", "modules/m/internal/" + Probe, "modules/m/internal/x/", + "modules/m/internal/x/" + Probe, "modules/m/internal/x/x.go"}}, + "nothing but generated JavaScript": {"*.js\n", nil}, + } { + root := t.TempDir() + for f, b := range map[string]string{ + "modules/m/.gitignore": c.ignore, + "modules/m/cmd/nfs-server/main.go": "package main\n", + "modules/m/cmd/nfs-server/run.js": "", + "modules/m/internal/x/x.go": "package x\n", + "modules/m/node_modules/y/index.go": "", + "modules/other/cmd/other/other.go": "package main\n", + } { + p := filepath.Join(root, f) + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte(b), 0o644); err != nil { + t.Fatal(err) + } + } + dirs, err := Sources(root) + if err != nil { + t.Fatal(err) + } + got, err := Find(root, dirs) + if err != nil { + t.Fatal(err) + } + var paths []string + for _, h := range got { + paths = append(paths, h.Path) + if !strings.HasPrefix(h.Pattern, "modules/m/.gitignore:") { + t.Errorf("%s: %s is named hidden by %q, want the module's .gitignore", name, h.Path, h.Pattern) + } + } + if strings.Join(paths, ",") != strings.Join(c.hidden, ",") { + t.Errorf("%s: hidden %v, want %v", name, paths, c.hidden) + } + } +} + +// The catalogue's own tree: no ignore pattern hides a path under a module's cmd/ or internal/. +func TestNoIgnorePatternHidesAModulesSource(t *testing.T) { + root := filepath.Join("..", "..") + dirs, err := Sources(root) + if err != nil { + t.Fatal(err) + } + if len(dirs) == 0 { + t.Fatal("no module source directory found: the check reads nothing") + } + hidden, err := Find(root, dirs) + if err != nil { + t.Fatal(err) + } + for _, h := range hidden { + t.Errorf("%s is hidden by %s: a module's source is never ignored; anchor the pattern to the binary (/, /cmd//)", h.Path, h.Pattern) + } +} diff --git a/checks/gitignored/go.mod b/checks/gitignored/go.mod new file mode 100644 index 00000000..e016e7f9 --- /dev/null +++ b/checks/gitignored/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/gitignored + +go 1.22 diff --git a/modules/nfs-server/.gitignore b/modules/nfs-server/.gitignore index 3cbd8705..d52d678a 100644 --- a/modules/nfs-server/.gitignore +++ b/modules/nfs-server/.gitignore @@ -1 +1,3 @@ -nfs-server +# the binary, built at the module or in its command directory; never the command directory itself +/nfs-server +/cmd/nfs-server/nfs-server diff --git a/modules/restic/cmd/restic-backups/.gitignore b/modules/restic/cmd/restic-backups/.gitignore index 066e2897..5c650c18 100644 --- a/modules/restic/cmd/restic-backups/.gitignore +++ b/modules/restic/cmd/restic-backups/.gitignore @@ -1 +1,2 @@ -restic-backups +# the binary, built in this command directory +/restic-backups From d320a19a8c9d43d50d8e4bca8d56add577d359dd Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:18:45 +0100 Subject: [PATCH 09/11] F-catalog-B: the catalogue's Go is as gofmt writes it, and checks/gofmt holds it so buildstep's generated.go and generated_test.go and claude-code's config_test.go reached the join unformatted: the second of a kind, which no check caught since each module's tests hold only themselves. All three are formatted, and checks/gofmt refuses a Go file of the catalogue gofmt would change. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- checks/gofmt/go.mod | 3 + checks/gofmt/gofmt.go | 58 +++++++++++++++++++ checks/gofmt/gofmt_test.go | 53 +++++++++++++++++ .../cmd/claude-code/config_test.go | 2 +- modules/mesh-build-go/buildstep/generated.go | 2 +- .../mesh-build-go/buildstep/generated_test.go | 2 +- 6 files changed, 117 insertions(+), 3 deletions(-) create mode 100644 checks/gofmt/go.mod create mode 100644 checks/gofmt/gofmt.go create mode 100644 checks/gofmt/gofmt_test.go diff --git a/checks/gofmt/go.mod b/checks/gofmt/go.mod new file mode 100644 index 00000000..5d9098aa --- /dev/null +++ b/checks/gofmt/go.mod @@ -0,0 +1,3 @@ +module git.novox.be/novox/mesh-catalog/checks/gofmt + +go 1.22 diff --git a/checks/gofmt/gofmt.go b/checks/gofmt/gofmt.go new file mode 100644 index 00000000..275adb24 --- /dev/null +++ b/checks/gofmt/gofmt.go @@ -0,0 +1,58 @@ +// Package gofmt refuses a Go file of the catalogue that gofmt would change (the second of a kind in one milestone: +// buildstep's generated.go and claude-code's config_test.go reached a join unformatted). A check here holds every +// module to it, where a module's own tests hold only themselves. +package gofmt + +import ( + "bytes" + "go/format" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" +) + +// Files are the Go files under root, relative to it: every .go file but those in .git, node_modules and a testdata +// directory (a test's fixture may be anything). +func Files(root string) ([]string, error) { + var out []string + err := filepath.WalkDir(root, func(p string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + if d.IsDir() { + switch d.Name() { + case ".git", "node_modules", "testdata": + return filepath.SkipDir + } + return nil + } + if strings.HasSuffix(d.Name(), ".go") && d.Type().IsRegular() { + rel, _ := filepath.Rel(root, p) + out = append(out, filepath.ToSlash(rel)) + } + return nil + }) + sort.Strings(out) + return out, err +} + +// Unformatted are the files gofmt would change, and why for one that does not parse. +func Unformatted(root string, files []string) (map[string]string, error) { + out := map[string]string{} + for _, f := range files { + src, err := os.ReadFile(filepath.Join(root, f)) + if err != nil { + return nil, err + } + got, err := format.Source(src) + switch { + case err != nil: + out[f] = "it does not parse: " + err.Error() + case !bytes.Equal(got, src): + out[f] = "gofmt changes it: run gofmt -w " + f + } + } + return out, nil +} diff --git a/checks/gofmt/gofmt_test.go b/checks/gofmt/gofmt_test.go new file mode 100644 index 00000000..89470ab7 --- /dev/null +++ b/checks/gofmt/gofmt_test.go @@ -0,0 +1,53 @@ +package gofmt + +import ( + "os" + "path/filepath" + "testing" +) + +func TestUnformattedNamesWhatGofmtChanges(t *testing.T) { + root := t.TempDir() + for f, src := range map[string]string{ + "modules/a/a.go": "package a\n", + "modules/b/b.go": "package b\nimport (\n\t\"os\"\n\t\"fmt\"\n)\nvar _, _ = fmt.Sprint, os.Exit\n", + "modules/c/testdata/c.go": "package c", + "modules/d/node_modules/d.go": "package d", + } { + p := filepath.Join(root, f) + _ = os.MkdirAll(filepath.Dir(p), 0o755) + if err := os.WriteFile(p, []byte(src), 0o644); err != nil { + t.Fatal(err) + } + } + files, err := Files(root) + if err != nil { + t.Fatal(err) + } + got, err := Unformatted(root, files) + if err != nil { + t.Fatal(err) + } + if len(files) != 2 || len(got) != 1 || got["modules/b/b.go"] == "" { + t.Errorf("files %v, unformatted %v; want a.go and b.go read, b.go named", files, got) + } +} + +// The catalogue's own Go files are as gofmt writes them. +func TestTheCataloguesGoIsFormatted(t *testing.T) { + root := filepath.Join("..", "..") + files, err := Files(root) + if err != nil { + t.Fatal(err) + } + if len(files) == 0 { + t.Fatal("no Go file found: the check reads nothing") + } + got, err := Unformatted(root, files) + if err != nil { + t.Fatal(err) + } + for f, why := range got { + t.Errorf("%s: %s", f, why) + } +} diff --git a/modules/claude-code/cmd/claude-code/config_test.go b/modules/claude-code/cmd/claude-code/config_test.go index f443507e..3c219d04 100644 --- a/modules/claude-code/cmd/claude-code/config_test.go +++ b/modules/claude-code/cmd/claude-code/config_test.go @@ -269,7 +269,7 @@ func TestTheManifestListsEveryToolServed(t *testing.T) { t.Fatal(err) } var m struct { - Tools []string `json:"tools"` + Tools []string `json:"tools"` Invokes []string `json:"invokes"` Reads []string `json:"reads"` Consumes []string `json:"consumes"` diff --git a/modules/mesh-build-go/buildstep/generated.go b/modules/mesh-build-go/buildstep/generated.go index 488dc1ec..c164951d 100644 --- a/modules/mesh-build-go/buildstep/generated.go +++ b/modules/mesh-build-go/buildstep/generated.go @@ -20,8 +20,8 @@ import ( "sort" "strings" - "git.novox.be/novox/mesh-sdk/go/catalogue" "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep/seatgen" + "git.novox.be/novox/mesh-sdk/go/catalogue" ) // GeneratedRootGo and GeneratedRootTS are the generated roots of a contracts module, one per build module. diff --git a/modules/mesh-build-go/buildstep/generated_test.go b/modules/mesh-build-go/buildstep/generated_test.go index 369466e3..bce50b66 100644 --- a/modules/mesh-build-go/buildstep/generated_test.go +++ b/modules/mesh-build-go/buildstep/generated_test.go @@ -10,8 +10,8 @@ import ( "strings" "testing" - "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-catalog/modules/mesh-build-go/buildstep/seatgen" + "git.novox.be/novox/mesh-sdk/go/delivery" ) // contracts writes a contracts module of one seat, its generated files as the generator writes them, and answers its From f28d3bf2866b6fa3e79bf1aa4c11787134709730 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:26:11 +0100 Subject: [PATCH 10/11] F-parts: distribution and gitea bind mesh-sdk's idle key and binding cap instead of their own copies Both holders carried the same idle bound and the same read of incoming's cap; the second instance of the kind moved into mesh-sdk's Receiver (Arm, Expired, Bound, with BusArm, BusCap and FollowIdle). Each holder now hands its idle state and its objects to those, and keeps its rows. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../cmd/distribution-tools/main.go | 59 +++---------------- .../distribution/internal/holder/transfer.go | 54 ++++------------- .../gitea/cmd/package-registry/incoming.go | 50 +++------------- modules/gitea/cmd/package-registry/seats.go | 4 +- modules/gitea/internal/packageseats/holder.go | 53 ++++------------- 5 files changed, 37 insertions(+), 183 deletions(-) diff --git a/modules/distribution/cmd/distribution-tools/main.go b/modules/distribution/cmd/distribution-tools/main.go index 98603529..de40893b 100644 --- a/modules/distribution/cmd/distribution-tools/main.go +++ b/modules/distribution/cmd/distribution-tools/main.go @@ -15,12 +15,10 @@ import ( "os" "strings" "sync" - "sync/atomic" "time" reg "git.novox.be/novox/mesh-contracts/go/seats/containerimageregistry" stdio "git.novox.be/novox/mesh-sdk/go" - "git.novox.be/novox/mesh-sdk/go/busstate" "git.novox.be/novox/mesh-sdk/go/delivery" "git.novox.be/novox/mesh-sdk/go/health" "git.novox.be/novox/mesh-sdk/go/parts" @@ -166,53 +164,6 @@ func learnInto(state casState, module, fp string, attempts int) (bool, error) { return false, fmt.Errorf("%s was written by another writer first %d times: %w", holder.PublishingKey, attempts, stdio.ErrChanged) } -// incomingCap reads incoming's cap from its binding (the bus's bound of the object store, design 51 *Publishing*), once -// it answers one: the binding's cap does not change while the bundle runs. -func incomingCap() func() (uint64, error) { - var known atomic.Uint64 - return func() (uint64, error) { - if c := known.Load(); c > 0 { - return c, nil - } - f, err := busstate.ObjectsOf(holder.IncomingObjects).Fill() - if err != nil { - return 0, fmt.Errorf("incoming's cap cannot be read from its binding: %w", err) - } - if f.MaxBytes == 0 { - return 0, errors.New("incoming's binding carries no cap") - } - known.Store(f.MaxBytes) - return f.MaxBytes, nil - } -} - -// armIdle writes a transfer's idle key with its own time to live; one already there is the same bound, kept. -func armIdle(id string, ttl time.Duration) error { - if _, err := stdio.State(holder.IdleState).CreateTTL(id, map[string]any{"transfer": id}, ttl); err != nil && !errors.Is(err, stdio.ErrChanged) { - return err - } - return nil -} - -// followIdle drops a transfer when its idle key ends (holder.Expired): the bus's marker is the event, no timer runs. -func followIdle(h *holder.Holder) { - <-stdio.Served() - stdio.State(holder.IdleState).Follow("", func(ch stdio.StateChange) error { - if ch.Op != stdio.OpExpire { - return nil - } - dropped, err := h.Expired(ch.Key) - if err != nil { - logf("%v", err) - return err - } - if dropped { - logf("transfer %s had no part for %s: dropped with its parts", ch.Key, holder.TransferIdle) - } - return nil - }) -} - // mirrors is the state mirrors: an ask is created once; one already kept is the same copy, taken on already. type mirrors struct{} @@ -547,10 +498,16 @@ func main() { base, _ := url.Parse("http://127.0.0.1:5001") store := &holder.OCI{Base: base, Upstream: &holder.Upstream{}} h := &holder.Holder{Objects: parts.BusObjects(holder.IncomingObjects), Builder: delivery.BuilderVersions{Ask: stdio.Ask}, Store: store, Say: say, Finding: finding, - Transfers: parts.BusTransfers[holder.Opening](holder.TransfersState), Cap: incomingCap(), Idle: armIdle} + Transfers: parts.BusTransfers[holder.Opening](holder.TransfersState), Cap: parts.BusCap(holder.IncomingObjects), Idle: parts.BusArm(holder.IdleState)} v := &holder.Verbs{Holder: h, Catalog: store, Holds: kept[holder.Hold]{state: "holds", key: holder.Hold.Key}, Mirrors: mirrors{}} go followMirrors(v) - go followIdle(h) + go parts.FollowIdle(holder.IdleState, h.Expired, func(id string, dropped bool, err error) { + if err != nil { + logf("%v", err) + return + } + logf("transfer %s had no part for %s: dropped with its parts", id, holder.TransferIdle) + }) windows := kept[holder.Window]{state: "windows", key: func(w holder.Window) string { return w.Name }} if err := stdio.Serve("", tools(v, windows, holder.ConfiguredCallers)); err != nil { logf("%v", err) diff --git a/modules/distribution/internal/holder/transfer.go b/modules/distribution/internal/holder/transfer.go index 8882cf01..599b105f 100644 --- a/modules/distribution/internal/holder/transfer.go +++ b/modules/distribution/internal/holder/transfer.go @@ -162,9 +162,10 @@ type Holder struct { Say Say Finding Finding Now func() time.Time - // Cap reads incoming's cap from its binding; IncomingCap when nil or when the binding names none. + // Cap reads incoming's cap from its binding (parts.BusCap); IncomingCap when nil or when the binding names none. Cap func() (uint64, error) - // Idle writes a transfer's idle key with its own time to live, once; one already there is no fault. Nil arms nothing. + // Idle writes a transfer's idle key with its own time to live, once (parts.BusArm over IdleState); the receiver + // arms it on each open that leaves a transfer open. Nil arms nothing. Idle func(id string, ttl time.Duration) error } @@ -176,22 +177,13 @@ func (h *Holder) now() time.Time { } // CapNow is incoming's cap as its binding carries it. -func (h *Holder) CapNow() uint64 { - if h.Cap != nil { - if c, err := h.Cap(); err == nil && c > 0 { - return c - } - } - return IncomingCap -} - -func (h *Holder) cap() uint64 { return h.CapNow() } +func (h *Holder) CapNow() uint64 { return h.receiver().CapNow() } // receiver binds mesh-sdk's receiving side to distribution's incoming, its transfers state, the builder's check, its // store and its seats' events. func (h *Holder) receiver() *parts.Receiver[Opening] { - return &parts.Receiver[Opening]{Objects: h.Objects, Transfers: h.Transfers, Cap: h.cap(), MaxChunk: MaxPart, - Idle: TransferIdle, Full: IncomingFullPct, Now: h.now, + return &parts.Receiver[Opening]{Objects: h.Objects, Transfers: h.Transfers, Cap: IncomingCap, Bound: h.Cap, + MaxChunk: MaxPart, Idle: TransferIdle, Arm: h.Idle, Full: IncomingFullPct, Now: h.now, // Only the missing layers are sent: a part the store already holds under the name counts as held. Holds: func(t Transfer, digest string) bool { ok, err := h.Store.Has(t.Shape.Name, digest) @@ -268,13 +260,6 @@ func (h *Holder) Publish(seat, caller string, r PublishRequest) (PublishAnswer, if err != nil { return answerOf(got), fmt.Errorf("publish: %w", err) } - // An open transfer is bounded by its idle key; a key that cannot be written fails the publish loudly, and the - // build module's next publish of the same transfer answers the parts held and arms it again. - if !got.Ended && h.Idle != nil { - if err := h.Idle(r.Transfer, TransferIdle); err != nil { - return answerOf(got), fmt.Errorf("publish: transfer %s is open but its idle bound is not kept, so it would never be dropped: %w", r.Transfer, err) - } - } return answerOf(got), nil } @@ -307,29 +292,10 @@ func (h *Holder) Drop(id string) (Transfer, error) { return h.receiver().Drop(id // drop itself follows each transfer's idle key (Expired); the objects template keeps for good, so nothing else does. func (h *Holder) DropIdle() ([]string, error) { return h.receiver().DropIdle() } -// Expired is told that a transfer's idle key ended (the bus's marker): a transfer with no part for TransferIdle is -// dropped, its chunks with it; one that took a part since is armed again for the rest of its bound; one already gone is -// nothing to do. It answers whether it dropped the transfer. -func (h *Holder) Expired(id string) (bool, error) { - t, found, err := h.Transfers.Get(id) - if err != nil { - return false, fmt.Errorf("transfer %s's idle bound ended, but it cannot be read: %w", id, err) - } - if !found { - return false, nil - } - idle := h.now().Sub(t.Last) - if idle >= TransferIdle { - if _, err := h.Drop(id); err != nil { - return false, fmt.Errorf("transfer %s had no part for %s and is not dropped: %w", id, TransferIdle, err) - } - return true, nil - } - if h.Idle == nil { - return false, nil - } - return false, h.Idle(id, max(TransferIdle-idle, time.Second)) -} +// Expired is told that a transfer's idle key ended (the bus's marker, parts.FollowIdle): the receiver drops a transfer +// with no part for TransferIdle, its chunks with it, arms one that took a part since again for the rest, and leaves one +// already gone. It answers whether it dropped the transfer. +func (h *Holder) Expired(id string) (bool, error) { return h.receiver().Expired(id) } func checkPublish(seat string, r PublishRequest) error { switch { diff --git a/modules/gitea/cmd/package-registry/incoming.go b/modules/gitea/cmd/package-registry/incoming.go index eda80b2a..9ad5c3c7 100644 --- a/modules/gitea/cmd/package-registry/incoming.go +++ b/modules/gitea/cmd/package-registry/incoming.go @@ -10,11 +10,9 @@ import ( "fmt" "os" "strings" - "sync/atomic" - "time" stdio "git.novox.be/novox/mesh-sdk/go" - "git.novox.be/novox/mesh-sdk/go/busstate" + "git.novox.be/novox/mesh-sdk/go/parts" "gitea/internal/packageseats" ) @@ -53,53 +51,19 @@ func incomingTool(typ, seat string, holder func() (*packageseats.Holder, error)) } } -// incomingCap reads incoming's cap from its binding (the bus's bound of the object store), once it answers one. -func incomingCap() func() (uint64, error) { - var known atomic.Uint64 - return func() (uint64, error) { - if c := known.Load(); c > 0 { - return c, nil - } - f, err := busstate.ObjectsOf(packageseats.IncomingObjects).Fill() - if err != nil { - return 0, fmt.Errorf("incoming's cap cannot be read from its binding: %w", err) - } - if f.MaxBytes == 0 { - return 0, errors.New("incoming's binding carries no cap") - } - known.Store(f.MaxBytes) - return f.MaxBytes, nil - } -} - -// armIdle writes a transfer's idle key with its own time to live; one already there is the same bound, kept. -func armIdle(key string, ttl time.Duration) error { - if _, err := stdio.State(packageseats.IdleState).CreateTTL(key, map[string]any{"transfer": key}, ttl); err != nil && !errors.Is(err, stdio.ErrChanged) { - return err - } - return nil -} - // followIdle drops a transfer when its idle key ends (packageseats.Holder.Expired): the bus's marker is the event. func followIdle(holder func() (*packageseats.Holder, error)) { - <-stdio.Served() - stdio.State(packageseats.IdleState).Follow("", func(ch stdio.StateChange) error { - if ch.Op != stdio.OpExpire { - return nil - } + parts.FollowIdle(packageseats.IdleState, func(id string) (bool, error) { h, err := holder() if err != nil { - fmt.Fprintf(os.Stderr, "transfer %s's idle bound ended, but the holder cannot be made: %v\n", ch.Key, err) - return err + return false, fmt.Errorf("transfer %s's idle bound ended, but the holder cannot be made: %w", id, err) } - dropped, err := h.Expired(ch.Key) + return h.Expired(id) + }, func(id string, dropped bool, err error) { if err != nil { fmt.Fprintln(os.Stderr, err) - return err + return } - if dropped { - fmt.Fprintf(os.Stderr, "transfer %s had no chunk for %s: dropped with its chunks\n", ch.Key, packageseats.TransferIdle) - } - return nil + fmt.Fprintf(os.Stderr, "transfer %s had no chunk for %s: dropped with its chunks\n", id, packageseats.TransferIdle) }) } diff --git a/modules/gitea/cmd/package-registry/seats.go b/modules/gitea/cmd/package-registry/seats.go index 6447791f..36a0ff90 100644 --- a/modules/gitea/cmd/package-registry/seats.go +++ b/modules/gitea/cmd/package-registry/seats.go @@ -264,7 +264,7 @@ func finding(kind, subject, words string, raised bool) { } // boundCap is incoming's cap from its binding, read once per bundle. -var boundCap = incomingCap() +var boundCap = parts.BusCap(packageseats.IncomingObjects) // busHolder is the holder over the runtime's bus state, the builder's numbering and the forge. func busHolder(forge func() (*Forge, error)) (*packageseats.Holder, error) { @@ -275,7 +275,7 @@ func busHolder(forge func() (*Forge, error)) (*packageseats.Holder, error) { return &packageseats.Holder{Callers: callers, Objects: parts.BusObjects(packageseats.IncomingObjects), Transfers: parts.BusTransfers[packageseats.Opening](packageseats.TransfersState), Builder: delivery.BuilderVersions{Ask: stdio.Ask}, Store: seatStore{forge}, Say: say, Finding: finding, - Cap: boundCap, Idle: armIdle}, nil + Cap: boundCap, Idle: parts.BusArm(packageseats.IdleState)}, nil } // SeatTools are both seats' verbs as tools .. diff --git a/modules/gitea/internal/packageseats/holder.go b/modules/gitea/internal/packageseats/holder.go index 80ae121e..557fa181 100644 --- a/modules/gitea/internal/packageseats/holder.go +++ b/modules/gitea/internal/packageseats/holder.go @@ -154,21 +154,15 @@ type Holder struct { // Finding writes (raised) or clears the module's finding of a kind for a subject. Finding func(kind, subject, words string, raised bool) Now func() time.Time - // Cap reads incoming's cap from its binding; IncomingCap when nil or when the binding names none. + // Cap reads incoming's cap from its binding (parts.BusCap); IncomingCap when nil or when the binding names none. Cap func() (uint64, error) - // Idle writes a transfer's idle key with its own time to live, once; one already there is no fault. Nil arms nothing. + // Idle writes a transfer's idle key with its own time to live, once (parts.BusArm over IdleState); the receiver + // arms it on each open that leaves a transfer open. Nil arms nothing. Idle func(key string, ttl time.Duration) error } // CapNow is incoming's cap as its binding carries it. -func (h *Holder) CapNow() uint64 { - if h.Cap != nil { - if c, err := h.Cap(); err == nil && c > 0 { - return c - } - } - return IncomingCap -} +func (h *Holder) CapNow() uint64 { return h.receiver().CapNow() } func (h *Holder) now() time.Time { if h.Now != nil { @@ -184,9 +178,8 @@ func Reference(name, version string) string { return name + "@" + version } func key(seat, transfer string) string { return seat + "_" + transfer } func (h *Holder) receiver() *parts.Receiver[Opening] { - cap := h.CapNow() - return &parts.Receiver[Opening]{Objects: h.Objects, Transfers: h.Transfers, Cap: cap, MaxChunk: MaxChunk, - Idle: TransferIdle, Full: IncomingFullPct, Now: h.now, + return &parts.Receiver[Opening]{Objects: h.Objects, Transfers: h.Transfers, Cap: IncomingCap, Bound: h.Cap, + MaxChunk: MaxChunk, Idle: TransferIdle, Arm: h.Idle, Full: IncomingFullPct, Now: h.now, // The builder's numbering is asked before anything is uploaded (ADR 0605 §4). Check: func(t Transfer) error { ok, why, err := h.Builder.Recorded(t.Shape.Name, t.Shape.Version, t.Shape.Digest) @@ -265,13 +258,6 @@ func (h *Holder) Publish(seat, caller string, p Publish) (Answer, error) { if err != nil { return h.answer(p.Transfer, got), fmt.Errorf("publish: %w", err) } - // An open transfer is bounded by its idle key; one that cannot be written fails the publish loudly, and the build - // module's next publish of the same transfer answers the part held and arms it again. - if !got.Ended && h.Idle != nil { - if err := h.Idle(key(seat, p.Transfer), TransferIdle); err != nil { - return h.answer(p.Transfer, got), fmt.Errorf("publish: transfer %s is open but its idle bound is not kept, so it would never be dropped: %w", p.Transfer, err) - } - } return h.answer(p.Transfer, got), nil } @@ -378,29 +364,10 @@ func (h *Holder) IncomingOf(seat string) (Incoming, error) { return out, nil } -// Expired is told that a transfer's idle key (its key in the state transfers) ended: a transfer with no chunk for -// TransferIdle is dropped with its chunks; one that took a chunk since is armed again for the rest; one gone is -// nothing to do. It answers whether it dropped it. -func (h *Holder) Expired(id string) (bool, error) { - t, found, err := h.Transfers.Get(id) - if err != nil { - return false, fmt.Errorf("transfer %s's idle bound ended, but it cannot be read: %w", id, err) - } - if !found { - return false, nil - } - idle := h.now().Sub(t.Last) - if idle >= TransferIdle { - if _, err := h.receiver().Drop(id); err != nil { - return false, fmt.Errorf("transfer %s had no chunk for %s and is not dropped: %w", id, TransferIdle, err) - } - return true, nil - } - if h.Idle == nil { - return false, nil - } - return false, h.Idle(id, max(TransferIdle-idle, time.Second)) -} +// Expired is told that a transfer's idle key (its key in the state transfers) ended (the bus's marker, +// parts.FollowIdle): the receiver drops a transfer with no chunk for TransferIdle with its chunks, arms one that took a +// chunk since again for the rest, and leaves one gone. It answers whether it dropped it. +func (h *Holder) Expired(id string) (bool, error) { return h.receiver().Expired(id) } func checkPublish(seat string, p Publish) error { switch { From 00d6fe81bf1e93ea1d149cb1ce2fdf55448cfef8 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 4 Nov 2026 17:30:50 +0100 Subject: [PATCH 11/11] F-parts: on registry 2.8.3 a shared-digest tagged version's remove keeps the tag and says removed The store's pinned image is registry 2.8.3, whose DeleteManifest cannot delete a tag alone: by a tag it deletes the empty digest and answers 400 DIGEST_INVALID. The fakes accepted a DELETE by tag, a fixture agreeing with the mistake, so every merge of a reproducible build would have been a refused remove and a retry finding live. RemoveTag now answers ErrNoTagAlone for that answer; remove then keeps the tag, whose bytes stay under the other tag anyway, and says removed with that reason, so the builder lets it go. Any other store failure is still refused; a digest shared with nothing is still deleted by digest. The fakes answer as 2.8.3 does. Claude-Session: https://claude.ai/code/session_01JG73KJNrH5EgFXMKKYSrig --- .../internal/holder/holder_test.go | 46 ++++++++++++++----- .../internal/holder/mirror_fakes_test.go | 26 ++++++++++- modules/distribution/internal/holder/oci.go | 12 ++++- modules/distribution/internal/holder/verbs.go | 12 +++-- 4 files changed, 79 insertions(+), 17 deletions(-) diff --git a/modules/distribution/internal/holder/holder_test.go b/modules/distribution/internal/holder/holder_test.go index f3f11868..36ae96ae 100644 --- a/modules/distribution/internal/holder/holder_test.go +++ b/modules/distribution/internal/holder/holder_test.go @@ -5,6 +5,7 @@ import ( "encoding/hex" "encoding/json" "errors" + "fmt" "net/http" "net/http/httptest" "net/url" @@ -117,13 +118,14 @@ func (c *fakeCatalog) Remove(name, version string) error { } return nil } + +// RemoveTag answers as OCI.RemoveTag over registry 2.8.3 does (fakeRegistry's DELETE by tag): the store cannot +// remove a tag alone, and nothing goes. tagErr is any other failure of the store. func (c *fakeCatalog) RemoveTag(name, version string) error { if c.tagErr != nil { return c.tagErr } - c.removed = append(c.removed, name+":"+version+" (tag)") - delete(c.tags, name+":"+version) - return nil + return fmt.Errorf("%w: DELETE of %s:%s by its tag answered 400 DIGEST_INVALID", ErrNoTagAlone, name, version) } func (c *fakeCatalog) RemoveDigest(name, digest string) error { c.removed = append(c.removed, name+"@"+digest) @@ -427,22 +429,42 @@ func TestTheHolderOfTheTwoImageSeats(t *testing.T) { if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.5-feat-x.1", Reason: "branch merged"}); err != nil || b.cat.tags["m/p:0.1.5-feat-x.1"] != "" { t.Errorf("the builder's removal of a tagged version: %v", err) } - // Security review F1: a reproducible build gives a branch's tagged version and the trunk's numbered one one - // digest; the tagged one's removal takes its tag alone, and the numbered one stays pullable. + // Security review F1 on registry 2.8.3: a reproducible build gives a branch's tagged version and the trunk's + // numbered one one digest; the store cannot remove the tag alone, so nothing goes, the numbered one stays + // pullable, and the removal is said removed with that reason: no refusal, so the builder raises no retry. b.cat.tags["m/p:0.1.6-feat-y.1"], b.cat.tags["m/p:0.1.6"] = part.Digest, part.Digest if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.6-feat-y.1", Reason: "branch merged"}); err != nil { - t.Fatalf("the removal of a tagged version sharing its digest: %v", err) + t.Fatalf("the removal of a tagged version sharing its digest: %v; want no error", err) } - if b.cat.tags["m/p:0.1.6-feat-y.1"] != "" || b.cat.tags["m/p:0.1.6"] != part.Digest { - t.Errorf("tags after the tagged version's removal: %v; want the tag gone and 0.1.6 still at %s", b.cat.tags, part.Digest) + if b.cat.tags["m/p:0.1.6-feat-y.1"] != part.Digest || b.cat.tags["m/p:0.1.6"] != part.Digest { + t.Errorf("tags after the tagged version's removal: %v; want both kept at %s", b.cat.tags, part.Digest) } - // A store that cannot delete a tag alone removes nothing, and the removal is refused naming the shared digest. + if last := b.said[len(b.said)-1]; last.event != EventRemoved || last.body.Reason != "branch merged; the store cannot remove a tag alone, so the tag 0.1.6-feat-y.1 is kept and its bytes stay under 0.1.6" { + t.Errorf("said %s %q; want removed, saying the tag is kept under 0.1.6", last.event, last.body.Reason) + } + // Any other failure of the store removes nothing and is refused naming the shared digest. b.cat.tags["m/p:0.1.7-feat-z.1"], b.cat.tags["m/p:0.1.7"] = part.Digest, part.Digest - b.cat.tagErr = errors.New("UNSUPPORTED") - if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.7-feat-z.1", Reason: "branch merged"}); err == nil || !strings.Contains(err.Error(), "shares its digest") || b.cat.tags["m/p:0.1.7"] != part.Digest { - t.Errorf("a store that cannot delete a tag alone: %v, tags %v; want refused and 0.1.7 kept", err, b.cat.tags) + b.cat.tagErr = errors.New("the store answered 500") + said := len(b.said) + if err := b.v.Remove(ArtifactSeat, "novox.mesh-delivery-builder", RemoveRequest{Name: "m/p", Version: "0.1.7-feat-z.1", Reason: "branch merged"}); err == nil || !strings.Contains(err.Error(), "shares its digest") || b.cat.tags["m/p:0.1.7"] != part.Digest || len(b.said) != said { + t.Errorf("a store failing otherwise: %v, tags %v; want refused, 0.1.7 kept, nothing said", err, b.cat.tags) } b.cat.tagErr = nil + // The store's own answers, as registry 2.8.3 gives them: DELETE by tag removes nothing (ErrNoTagAlone); by + // digest the manifest goes with every tag naming it. + r283 := newFakeRegistry() + sts := httptest.NewServer(r283) + defer sts.Close() + u, _ := url.Parse(sts.URL) + o := &OCI{Base: u} + manifest := []byte(`{"schemaVersion":2}`) + r283.manifests["m/p:0.1.8-feat-w.1"], r283.manifests["m/p:0.1.8"] = manifest, manifest + if err := o.RemoveTag("m/p", "0.1.8-feat-w.1"); !errors.Is(err, ErrNoTagAlone) || r283.manifests["m/p:0.1.8-feat-w.1"] == nil { + t.Errorf("RemoveTag on 2.8.3 = %v; want ErrNoTagAlone and the tag kept", err) + } + if err := o.Remove("m/p", "0.1.8"); err != nil || len(r283.manifests) != 0 { + t.Errorf("Remove by digest on 2.8.3 = %v, %d manifests left; want both tags gone", err, len(r283.manifests)) + } }) // CAT-AS1: what the sweep did over the store's door is the holder's: a removal for retention of an index removes // its platform manifests first, but those a tagged or held index of the repository names; a manifest such an index diff --git a/modules/distribution/internal/holder/mirror_fakes_test.go b/modules/distribution/internal/holder/mirror_fakes_test.go index 2ee12d86..d65f18d2 100644 --- a/modules/distribution/internal/holder/mirror_fakes_test.go +++ b/modules/distribution/internal/holder/mirror_fakes_test.go @@ -10,7 +10,10 @@ import ( ) // fakeRegistry is an OCI registry in memory, enough for a mirror's copy: blobs, uploads and manifests by -// reference. With token set it answers 401 with a Bearer challenge until a request carries it. +// reference. With token set it answers 401 with a Bearer challenge until a request carries it. A DELETE of a manifest +// answers as registry 2.8.3, the store's pinned image, does (distribution v2.8.3 registry/handlers/manifests.go, +// DeleteManifest): by digest it deletes the manifest and every tag naming it; by a tag it deletes nothing and answers +// 400 DIGEST_INVALID, the check of the empty digest it deletes instead failing. type fakeRegistry struct { mu sync.Mutex blobs map[string][]byte // name@digest @@ -69,6 +72,27 @@ func (f *fakeRegistry) ServeHTTP(w http.ResponseWriter, r *http.Request) { case strings.Contains(p, "/manifests/"): i := strings.Index(p, "/manifests/") key := p[:i] + ":" + p[i+11:] + if r.Method == http.MethodDelete { + ref := p[i+11:] + if !strings.HasPrefix(ref, "sha256:") { + w.WriteHeader(http.StatusBadRequest) + _, _ = io.WriteString(w, `{"errors":[{"code":"DIGEST_INVALID","message":"provided digest did not match uploaded content"}]}`) + return + } + found := false + for k, b := range f.manifests { + if strings.HasPrefix(k, p[:i]+":") && digestOf(b) == ref { + delete(f.manifests, k) + found = true + } + } + if !found { + w.WriteHeader(http.StatusNotFound) + return + } + w.WriteHeader(http.StatusAccepted) + return + } if r.Method == http.MethodPut { b, _ := io.ReadAll(r.Body) f.manifests[key] = b diff --git a/modules/distribution/internal/holder/oci.go b/modules/distribution/internal/holder/oci.go index c2bd00ef..7b41d9fe 100644 --- a/modules/distribution/internal/holder/oci.go +++ b/modules/distribution/internal/holder/oci.go @@ -3,6 +3,7 @@ package holder import ( "bytes" "encoding/json" + "errors" "fmt" "io" "net/http" @@ -253,13 +254,22 @@ func (o *OCI) Digest(name, version string) (string, error) { return resp.Header.Get("Docker-Content-Digest"), nil } +// ErrNoTagAlone is a store that cannot delete a tag alone. registry 2.8.3, the store's pinned image, is one: its +// DeleteManifest (distribution v2.8.3 registry/handlers/manifests.go) ignores a tag reference and deletes the empty +// digest, whose check fails, answered 400 DIGEST_INVALID; registry 3 untags alone. +var ErrNoTagAlone = errors.New("the store cannot remove a tag alone") + // RemoveTag deletes the tag version of name alone (the OCI distribution protocol's DELETE of a manifest by its tag); -// the manifest stays under every other tag. A store that cannot delete a tag answers an error. +// the manifest stays under every other tag. A store that cannot delete a tag answers ErrNoTagAlone, any other failure +// an error of its own; either removes nothing. func (o *OCI) RemoveTag(name, version string) error { resp, b, err := o.do(http.MethodDelete, "/v2/"+name+"/manifests/"+version, "", nil) if err != nil { return err } + if resp.StatusCode == http.StatusBadRequest && bytes.Contains(b, []byte(`"DIGEST_INVALID"`)) { + return fmt.Errorf("%w: DELETE of %s:%s by its tag answered 400 DIGEST_INVALID", ErrNoTagAlone, name, version) + } if resp.StatusCode != http.StatusAccepted && resp.StatusCode != http.StatusNotFound { return fmt.Errorf("the store did not delete the tag %s:%s alone (%d): %s", name, version, resp.StatusCode, b) } diff --git a/modules/distribution/internal/holder/verbs.go b/modules/distribution/internal/holder/verbs.go index 095da21d..dd184de7 100644 --- a/modules/distribution/internal/holder/verbs.go +++ b/modules/distribution/internal/holder/verbs.go @@ -6,6 +6,7 @@ import ( "errors" "fmt" "slices" + "sort" "strings" "time" @@ -22,7 +23,7 @@ type Catalog interface { // collector frees its blobs. Remove(name, version string) error // RemoveTag deletes the tag version of name alone, the manifest staying under every other tag naming it; a store - // that cannot delete a tag alone answers an error and removes nothing. + // that cannot delete a tag alone answers ErrNoTagAlone and removes nothing. RemoveTag(name, version string) error // RemoveDigest deletes an untagged manifest of name by its digest (retention). RemoveDigest(name, digest string) error @@ -159,13 +160,18 @@ func (v *Verbs) Remove(seat, caller string, r RemoveRequest) error { if d != "" { // Reproducible builds share a digest between a branch's tagged version and the trunk's numbered one: a // manifest deleted by its digest would take the trunk's version with it (security review F1). When another - // tag names the digest, the tag alone goes; when the store cannot do that, nothing goes, said. + // tag names the digest, the tag alone goes. A store that cannot remove a tag alone (registry 2.8.3) keeps the + // tag: its bytes are kept under the other tags anyway, so the removal is said removed with that reason and + // the builder lets the version go from its keeping, no refusal and no retry. Any other failure is refused. others, err := v.otherTags(r.Name, r.Version, d) if err != nil { return fmt.Errorf("remove: the store cannot say which other tags of %s name %s, so nothing is removed: %w", r.Name, d, err) } if len(others) > 0 { - if err := v.Catalog.RemoveTag(r.Name, r.Version); err != nil { + if err := v.Catalog.RemoveTag(r.Name, r.Version); errors.Is(err, ErrNoTagAlone) { + sort.Strings(others) + ev.Reason = fmt.Sprintf("%s; the store cannot remove a tag alone, so the tag %s is kept and its bytes stay under %s", r.Reason, r.Version, strings.Join(others, ", ")) + } else if err != nil { return fmt.Errorf("remove: refused: %s@%s shares its digest %s with %v, and the store did not remove the tag alone, so nothing is removed: %w", r.Name, r.Version, d, others, err) } } else if err := v.Catalog.Remove(r.Name, r.Version); err != nil {