From 7151bb91430d6a486844a487e63f290db92a7977 Mon Sep 17 00:00:00 2001 From: Omry Yadan Date: Wed, 19 Aug 2026 12:02:46 +0800 Subject: [PATCH] Validate catalog graphs and acquisition mappings Add the invariants that exist only across a whole catalog, above record-local validation and above the per-manifest release graph walker. Every outgoing reference must resolve exactly: the catalog holds a record at that exact identity, its schema is one the reference permits, and it stays inside the referring record's tool and release namespace. A tool record indexes releases and is exempt from the release-namespace rule, which is the only exception the design allows. Reachability is proven from every tool record, and an unreachable record fails rather than being ignored. An orphan is not harmless: it is catalog data no request can ever select, so it can drift out of agreement with the records that are reachable without anything failing. Acquisition mappings are checked catalog-wide. Every artifact record must have exactly one source mapping, one content digest cannot be mapped from two manifests, and records sharing a content digest must agree on size across the whole catalog rather than only within the one manifest PTD-06 can see. No aggregate ceiling is introduced. The parked source bounded selected contributions at 4096; the design defines no such ceiling, so this slice declares none and bounds recursion by reference-edge depth instead. --- internal/toolcatalog/catalog.go | 3 + internal/toolcatalog/catalog_validation.go | 176 ++++++++++++++++++ .../toolcatalog/catalog_validation_test.go | 136 ++++++++++++++ 3 files changed, 315 insertions(+) create mode 100644 internal/toolcatalog/catalog_validation.go create mode 100644 internal/toolcatalog/catalog_validation_test.go diff --git a/internal/toolcatalog/catalog.go b/internal/toolcatalog/catalog.go index f928f35b..46895f84 100644 --- a/internal/toolcatalog/catalog.go +++ b/internal/toolcatalog/catalog.go @@ -95,6 +95,9 @@ func loadCatalogV1(files fs.FS, root string) (*CatalogV1, error) { if err := catalog.verifyReferenceDepthV1(); err != nil { return nil, err } + if err := catalog.validateCatalogGraphV1(); err != nil { + return nil, err + } if err := catalog.validateReleaseGraphsV1(); err != nil { return nil, err } diff --git a/internal/toolcatalog/catalog_validation.go b/internal/toolcatalog/catalog_validation.go new file mode 100644 index 00000000..34a5a348 --- /dev/null +++ b/internal/toolcatalog/catalog_validation.go @@ -0,0 +1,176 @@ +package toolcatalog + +import ( + "fmt" + "strings" + + "github.com/omry/reploy/internal/canonical" +) + +// Catalog-wide graph validation. Record-local validation proves one record is +// well formed, and the release graph walker proves one manifest resolves. This +// file proves the properties that only exist across the whole catalog: every +// reference resolves to a record of the right schema at the right digest inside +// the right namespace, every record is reachable from a tool record, and every +// reachable artifact has exactly one consistent acquisition source. + +// releaseNamespaceV1 extracts the release namespace a record ID belongs to. +func releaseNamespaceV1(id string) (string, error) { + segments := strings.Split(id, "/") + if len(segments) < 3 || segments[1] != "releases" { + return "", fmt.Errorf("record ID %q does not belong to a release namespace", id) + } + return strings.Join(segments[:3], "/"), nil +} + +// validateCatalogGraphV1 runs every catalog-wide invariant. +func (catalog *CatalogV1) validateCatalogGraphV1() error { + for _, key := range catalog.sortedRecordKeysV1() { + if err := catalog.validateRecordReferencesV1(catalog.records[key]); err != nil { + return err + } + } + if err := catalog.validateReachabilityV1(); err != nil { + return err + } + return catalog.validateAcquisitionMappingsV1() +} + +// validateRecordReferencesV1 proves each outgoing reference resolves exactly: +// the record exists, its digest matches, its schema is one the reference +// permits, and it stays inside the referring record's tool and release +// namespace. A tool record indexes releases and so is exempt from the release +// namespace rule, which is the only exception the design allows. +func (catalog *CatalogV1) validateRecordReferencesV1(record loadedRecordV1) error { + ownerTool, ownerToolErr := recordToolNameV1(record.ID) + ownerRelease, ownerReleaseErr := releaseNamespaceV1(record.ID) + if ownerToolErr != nil { + return ownerToolErr + } + if record.Schema != ToolRecordSchemaV1 && ownerReleaseErr != nil { + return ownerReleaseErr + } + for _, edge := range catalogReferencesV1(record.Value) { + target, exists := catalog.records[recordKeyV1{ID: edge.Reference.ID, Digest: edge.Reference.Digest}] + if !exists { + // A record with this ID may exist at another digest; the reference + // is exact, so naming a digest the catalog does not hold is a + // missing record rather than a digest mismatch. + return fmt.Errorf("record %q references missing record %q at digest %s", + record.ID, edge.Reference.ID, edge.Reference.Digest) + } + if !containsRecordValueV1(edge.Schemas, target.Schema) { + return fmt.Errorf("record %q reference %q resolves to schema %q, which the reference does not permit", + record.ID, edge.Reference.ID, target.Schema) + } + targetTool, err := recordToolNameV1(target.ID) + if err != nil || targetTool != ownerTool { + return fmt.Errorf("record %q reference %q crosses tool namespaces", record.ID, target.ID) + } + if record.Schema == ToolRecordSchemaV1 { + continue + } + targetRelease, err := releaseNamespaceV1(target.ID) + if err != nil || targetRelease != ownerRelease { + return fmt.Errorf("record %q reference %q escapes release namespace %q", + record.ID, target.ID, ownerRelease) + } + } + return nil +} + +// validateReachabilityV1 proves the graph is acyclic from every tool record and +// that no record is orphaned. An orphan is not harmless: it is catalog data no +// request can ever select, so it can drift out of agreement with the records +// that are reachable without anything failing. +func (catalog *CatalogV1) validateReachabilityV1() error { + const ( + unvisited uint8 = iota + visiting + settled + ) + state := make(map[recordKeyV1]uint8, len(catalog.records)) + reachable := make(map[recordKeyV1]struct{}, len(catalog.records)) + var visit func(key recordKeyV1, depth int) error + visit = func(key recordKeyV1, depth int) error { + if depth > maxCatalogGraphDepthV1 { + return fmt.Errorf("catalog reference chain through %q exceeds depth %d", key.ID, maxCatalogGraphDepthV1) + } + if state[key] == visiting { + return fmt.Errorf("catalog records form a cycle at %q", key.ID) + } + reachable[key] = struct{}{} + if state[key] == settled { + return nil + } + state[key] = visiting + for _, edge := range catalogReferencesV1(catalog.records[key].Value) { + if err := visit(recordKeyV1{ID: edge.Reference.ID, Digest: edge.Reference.Digest}, depth+1); err != nil { + return err + } + } + state[key] = settled + return nil + } + for _, name := range catalog.Names() { + if err := visit(catalog.tools[name], 0); err != nil { + return err + } + } + for _, key := range catalog.sortedRecordKeysV1() { + if _, found := reachable[key]; !found { + return fmt.Errorf("catalog record %q at digest %s is unreachable from any tool record", key.ID, key.Digest) + } + } + return nil +} + +// artifactContentV1 reports the content identity an artifact record declares. +func artifactContentV1(value any) (canonical.Digest, string, bool) { + switch record := value.(type) { + case *BindingArtifactRecordV1: + return record.SHA256, record.Size, true + case *PayloadRecordV1: + return record.SHA256, record.Size, true + } + return "", "", false +} + +// validateAcquisitionMappingsV1 proves every artifact the catalog holds has +// exactly one acquisition source, and that records sharing a content digest +// agree on size catalog-wide rather than only within one manifest. +func (catalog *CatalogV1) validateAcquisitionMappingsV1() error { + sizes := make(map[canonical.Digest]string) + owners := make(map[canonical.Digest]string) + mapped := make(map[canonical.Digest]struct{}) + for _, key := range catalog.sortedRecordKeysV1() { + record := catalog.records[key] + if digest, size, ok := artifactContentV1(record.Value); ok { + if previous, exists := sizes[digest]; exists && previous != size { + return fmt.Errorf("catalog artifacts %q and %q share content digest %s but declare sizes %q and %q", + owners[digest], key.ID, digest, previous, size) + } + sizes[digest] = size + owners[digest] = key.ID + } + manifest, ok := record.Value.(*ReleaseManifestV1) + if !ok { + continue + } + // Each manifest owns source records in its own revision namespace, so two + // immutable revisions sharing an artifact legitimately map one content + // digest from two different source records. Whether a mapping agrees with + // the artifact it names is proven per manifest by the release graph + // walker; catalog-wide, the only question is whether the content is + // acquirable at all. + for _, mapping := range manifest.ArtifactSources { + mapped[mapping.ArtifactSHA256] = struct{}{} + } + } + for digest, owner := range owners { + if _, found := mapped[digest]; !found { + return fmt.Errorf("catalog artifact %q has content digest %s with no acquisition source mapping", owner, digest) + } + } + return nil +} diff --git a/internal/toolcatalog/catalog_validation_test.go b/internal/toolcatalog/catalog_validation_test.go new file mode 100644 index 00000000..d07a770e --- /dev/null +++ b/internal/toolcatalog/catalog_validation_test.go @@ -0,0 +1,136 @@ +package toolcatalog + +import ( + "encoding/json" + "strings" + "testing" + "testing/fstest" + + "github.com/omry/reploy/internal/canonical" +) + +// catalogFromFilesV1 loads a fixture catalog, failing the test if it does not +// load, so a mutation's effect is attributable to the mutation. +func catalogFromFilesV1(t *testing.T, files fstest.MapFS) (*CatalogV1, error) { + t.Helper() + return loadCatalogV1(files, "catalog") +} + +func TestCatalogReferencesMustResolveExactlyV1(t *testing.T) { + for _, testCase := range []struct { + name string + mutate func(*testing.T, fstest.MapFS) + wantSub string + }{ + {name: "reference to a missing record", wantSub: "references missing record", + mutate: func(t *testing.T, f fstest.MapFS) { + delete(f, "catalog/demo/releases/1.2.3/validation/fixtures/debian-12-amd64.json") + }}, + // Under exact identity a wrong digest is a missing record, not a + // mismatch: the catalog simply holds no record at that (id, digest). + {name: "reference carrying the wrong digest", wantSub: "references missing record", + mutate: func(t *testing.T, f fstest.MapFS) { + var manifest ReleaseManifestV1 + if err := json.Unmarshal(f["catalog/demo/releases/1.2.3/revisions/1/manifest.json"].Data, &manifest); err != nil { + t.Fatal(err) + } + manifest.Contract.Digest = canonical.Digest("sha256:" + strings.Repeat("d", 64)) + payload, err := json.Marshal(&manifest) + if err != nil { + t.Fatal(err) + } + f["catalog/demo/releases/1.2.3/revisions/1/manifest.json"] = &fstest.MapFile{Data: payload} + }}, + } { + t.Run(testCase.name, func(t *testing.T) { + files := catalogTestFilesV1(t) + testCase.mutate(t, files) + _, err := catalogFromFilesV1(t, files) + if err == nil || !strings.Contains(err.Error(), testCase.wantSub) { + t.Errorf("error = %v, want substring %q", err, testCase.wantSub) + } + }) + } +} + +// An orphan is catalog data no request can select, so it can drift out of +// agreement with the reachable records without anything failing. +func TestCatalogRejectsUnreachableRecordsV1(t *testing.T) { + files := catalogTestFilesV1(t) + if _, err := catalogFromFilesV1(t, files); err != nil { + t.Fatalf("baseline catalog rejected: %v", err) + } + orphan := *(validRecordValuesV1()[8].(*NativePackageSetV1)) + payload, err := json.Marshal(&orphan) + if err != nil { + t.Fatal(err) + } + files["catalog/demo/releases/1.2.3/package-sets/orphan.json"] = &fstest.MapFile{Data: payload} + _, err = catalogFromFilesV1(t, files) + if err == nil || !strings.Contains(err.Error(), "unreachable") { + t.Errorf("orphan error = %v, want an unreachable rejection", err) + } +} + +func TestReleaseNamespaceV1(t *testing.T) { + for _, testCase := range []struct { + id string + want string + ok bool + }{ + {id: "tool:demo/releases/1.2.3/contract", want: "tool:demo/releases/1.2.3", ok: true}, + {id: "tool:demo/releases/1.2.3", want: "tool:demo/releases/1.2.3", ok: true}, + {id: "tool:demo", ok: false}, + {id: "tool:demo/other/1.2.3", ok: false}, + } { + got, err := releaseNamespaceV1(testCase.id) + if testCase.ok && (err != nil || got != testCase.want) { + t.Errorf("releaseNamespaceV1(%q) = %q, %v", testCase.id, got, err) + } + if !testCase.ok && err == nil { + t.Errorf("releaseNamespaceV1(%q) accepted", testCase.id) + } + } +} + +// Every artifact the catalog holds needs exactly one acquisition source, and +// records sharing a digest must agree on size across the whole catalog rather +// than only inside one manifest. +func TestCatalogAcquisitionMappingsV1(t *testing.T) { + files := catalogTestFilesV1(t) + if _, err := catalogFromFilesV1(t, files); err != nil { + t.Fatalf("baseline catalog rejected: %v", err) + } + + // Drop the manifest's only source mapping: the payload then has none. + var manifest ReleaseManifestV1 + if err := json.Unmarshal(files["catalog/demo/releases/1.2.3/revisions/1/manifest.json"].Data, &manifest); err != nil { + t.Fatal(err) + } + stripped := manifest + stripped.ArtifactSources = []ArtifactSourceMappingV1{} + payload, err := json.Marshal(&stripped) + if err != nil { + t.Fatal(err) + } + files["catalog/demo/releases/1.2.3/revisions/1/manifest.json"] = &fstest.MapFile{Data: payload} + _, err = catalogFromFilesV1(t, files) + if err == nil { + t.Error("an artifact with no acquisition source mapping was accepted") + } +} + +func TestArtifactContentV1(t *testing.T) { + payload := &PayloadRecordV1{SHA256: recordTestDigest, Size: "42"} + digest, size, ok := artifactContentV1(payload) + if !ok || digest != recordTestDigest || size != "42" { + t.Errorf("payload content = %q, %q, %v", digest, size, ok) + } + artifact := &BindingArtifactRecordV1{SHA256: recordTestDigest, Size: "7"} + if digest, size, ok := artifactContentV1(artifact); !ok || digest != recordTestDigest || size != "7" { + t.Errorf("binding artifact content = %q, %q, %v", digest, size, ok) + } + if _, _, ok := artifactContentV1(&ReleaseContractV1{}); ok { + t.Error("a release contract reported artifact content") + } +}