mesh-build-go: a catalogue module's non-Go files enter its packages' digests; markers where tests read other modules (issue 1935)

modules/mesh-build-go/README.md

@@ -102,7 +102,11 @@ integration level, the postgres major and the nats-server version; and, when the
 workspace (`beside`), each one's repository and commit, which the answer echoes as `beside` (the checker keeps a verdict only
 for the siblings it was judged with). The text's form is `DigestVersion` 3. The Go version is not in it: the toolchain enters a
 fingerprint once, as the version the build nodes agree on (ADR 0477 §3), and the standard library's packages are that
-version's. A package with no tests at the level has no digest and is named in `without_tests`. A directory that vendors
+version's. A package with no tests at the level has no digest and is named in `without_tests`. A catalogue module's packages also hash the module's non-Go files (the nearest ancestor directory holding `module.json`: its manifest, README, `files/` and the rest, so `../../module.json` is covered; digest form 5). A package whose directory holds a file named `reads-outside` (any content; say what the tests read) is never kept (issue 1935): its tests read
+files outside it, which a digest from `go list` cannot name, so its digest is a fresh random one on every call, the ask finds no kept
+verdict for it and runs it, and the answer names it in `reads_outside` and the module's log says why. The checker still writes a
+verdict under that digest, which nothing ever reads again. The marker is the plain way to say it until each verdict keeps the files
+its tests opened (Go's test log). A directory that vendors
 is listed and tested from its vendor directory (`-mod=vendor`).
 
 ## A change to this module is judged by its candidate

modules/mesh-build-go/cmd/mesh-build-go/beside_digest_test.go

@@ -116,7 +116,7 @@ func TestTheIntegrationLevelDigestsUnderBothTags(t *testing.T) {
 			t.Errorf("the integration digest listed %q", s.command)
 		}
 	}
-	if DigestVersion != "4" {
+	if DigestVersion != "5" {
 		t.Errorf("the digest's form changed in kind and is still version %s", DigestVersion)
 	}
 }

modules/mesh-build-go/cmd/mesh-build-go/digest.go

@@ -7,6 +7,7 @@ package main
 import (
 	"bytes"
 	"context"
+	"crypto/rand"
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
@@ -24,12 +25,20 @@ import (
 // 2: no Go version in the text; the toolchain enters a fingerprint as the agreed version (ADR 0477 §3). 3: the siblings'
 // commits (the lead's decision L5) and the integration level's tags integration,isolation (the checker's agreement).
 // 4: the siblings' commits enter only the digest of a package whose tests reach a file naming MESH_CHECK_BESIDE (ADR 0536 §1).
-const DigestVersion = "4"
+// 5: a catalogue module's non-Go files enter the digest of its packages (issue 1935).
+const DigestVersion = "5"
 
 // besideVariable is the name a file bears when it reads where the siblings lie; a package whose tests reach such a file
 // depends on the siblings' commits.
 const besideVariable = "MESH_CHECK_BESIDE"
 
+// ReadsOutsideMarker is the name of a file in a package's directory that says its tests read files outside it (a
+// repository's documents, a sibling directory): such a package is never kept (issue 1935). The digest hashes only what
+// the Go toolchain lists, so it cannot name what a test opens elsewhere; a marker file, beside the Go files where a
+// reader of the package sees it, is the plain way to say so until the checker keeps the files a test opened (the Go
+// test log) with each verdict.
+const ReadsOutsideMarker = "reads-outside"
+
 // integrationTags are the build tags of the integration level, its run and its digest alike: integration, and isolation
 // for the bubblewrap proofs (mesh-sdk's and the build modules'), which a build node can run.
 var integrationTags = []string{"integration", "isolation"}
@@ -89,6 +98,9 @@ type DigestResult struct {
 	// only when they are the siblings it gave the fetch.
 	Beside       []string `json:"beside,omitempty"`
 	WithoutTests []string `json:"without_tests,omitempty"`
+	// ReadsOutside names the packages that carry the marker: each has a digest that no kept verdict can have (a fresh
+	// random one), so the check runs it every time.
+	ReadsOutside []string `json:"reads_outside,omitempty"`
 	// JudgedBy is the program that ran the digest (ADR 0535 §5): "installed", or "candidate <merge commit>" under the
 	// runner workspace.
 	JudgedBy string `json:"judged_by,omitempty"`
@@ -199,6 +211,12 @@ func (m *Module) digestHere(ctx context.Context, workspace, level string) (Diges
 		}
 	}
 	sort.Strings(res.WithoutTests)
+	sort.Strings(res.ReadsOutside)
+	for _, p := range res.ReadsOutside {
+		if m.Say != nil {
+			m.Say("the %s level of %s always runs: its directory carries %s, so its tests read files outside it that no digest names", level, p, ReadsOutsideMarker)
+		}
+	}
 	return res, nil
 }
 
@@ -256,6 +274,17 @@ func digestModule(res *DigestResult, pkgs map[string]*goPackage, tree string, he
 		if testMain == nil {
 			return fmt.Errorf("go list gave no test binary for %s", path)
 		}
+		if _, err := os.Stat(filepath.Join(root.Dir, ReadsOutsideMarker)); err == nil {
+			// The digest of a package that reads outside itself is one nothing can have kept: the key is a fresh random
+			// hex, so the ask finds no kept verdict and runs it, and what the checker writes under it is never read again.
+			var nonce [32]byte
+			if _, err := rand.Read(nonce[:]); err != nil {
+				return err
+			}
+			res.Packages[path] = hex.EncodeToString(nonce[:])
+			res.ReadsOutside = append(res.ReadsOutside, path)
+			continue
+		}
 		d, err := packageDigest(root, testMain, pkgs, tree, header, beside)
 		if err != nil {
 			return fmt.Errorf("%s: %w", path, err)
@@ -303,6 +332,15 @@ func packageDigest(root, testMain *goPackage, pkgs map[string]*goPackage, tree s
 			}
 		}
 	}
+	// A catalogue module's tests read its manifest, its README and its files from the package's directory (../../module.json):
+	// every non-Go file of the module's directory is an input (issue 1935). The module is the nearest ancestor with a module.json.
+	if modDir := enclosingModule(tree, root.Dir); modDir != "" {
+		more, err := moduleFileLines(tree, root, modDir)
+		if err != nil {
+			return "", err
+		}
+		lines = append(lines, more...)
+	}
 	data := filepath.Join(root.Dir, "testdata")
 	if _, err := os.Stat(data); err == nil {
 		err := filepath.WalkDir(data, func(path string, e fs.DirEntry, err error) error {
@@ -328,6 +366,41 @@ func packageDigest(root, testMain *goPackage, pkgs map[string]*goPackage, tree s
 	return hex.EncodeToString(h.Sum(nil)), nil
 }
 
+// enclosingModule is the nearest ancestor of dir, inside tree, that holds a module.json, or "" when there is none.
+func enclosingModule(tree, dir string) string {
+	tree = filepath.Clean(tree)
+	for d := filepath.Clean(dir); d != tree && strings.HasPrefix(d, tree+string(filepath.Separator)); d = filepath.Dir(d) {
+		if st, err := os.Stat(filepath.Join(d, "module.json")); err == nil && !st.IsDir() {
+			return d
+		}
+	}
+	return ""
+}
+
+// moduleFileLines are the content lines of every file of a module's directory that is not Go source (Go source is the
+// packages' own, listed by go list), skipping .git and node_modules.
+func moduleFileLines(tree string, p *goPackage, modDir string) ([]string, error) {
+	var lines []string
+	err := filepath.WalkDir(modDir, func(path string, e fs.DirEntry, err error) error {
+		if err != nil {
+			return err
+		}
+		if e.IsDir() {
+			if n := e.Name(); n == ".git" || n == "node_modules" {
+				return filepath.SkipDir
+			}
+			return nil
+		}
+		if strings.HasSuffix(path, ".go") {
+			return nil
+		}
+		l, err := contentLine(tree, p, path)
+		lines = append(lines, l)
+		return err
+	})
+	return lines, err
+}
+
 func fileLine(tree string, p *goPackage, name string) (string, error) {
 	return contentLine(tree, p, filepath.Join(p.Dir, name))
 }

modules/mesh-build-go/cmd/mesh-build-go/digest_test.go

@@ -189,3 +189,71 @@ func TestAModuleOutsideTheTreeCountsByVersionAndChecksum(t *testing.T) {
 		t.Errorf("a new version left the digest as it was")
 	}
 }
+
+// Issue 1935: a package whose directory carries the reads-outside marker is never kept: its digest is one no kept verdict
+// can have (it differs on every call, though nothing changed), and the answer names it. A package without the marker
+// keeps its digest.
+func TestAPackageWithTheReadsOutsideMarkerIsNeverKept(t *testing.T) {
+	t.Parallel()
+	m, tree := digestFixture(t)
+	plain := digests(t, m, "unit")
+	if len(plain.ReadsOutside) != 0 {
+		t.Fatalf("a package carries the marker in the fixture: %v", plain.ReadsOutside)
+	}
+	edit(t, tree, "c/"+ReadsOutsideMarker, "the tests read ../docs\n")
+	first, second := digests(t, m, "unit"), digests(t, m, "unit")
+	c := "fixture/digest/c"
+	if len(first.Packages[c]) != 64 || first.Packages[c] == second.Packages[c] || first.Packages[c] == plain.Packages[c] {
+		t.Errorf("c carries the marker and its digest is stable: %s, %s, before %s", first.Packages[c], second.Packages[c], plain.Packages[c])
+	}
+	if strings.Join(first.ReadsOutside, ",") != c {
+		t.Errorf("the answer does not name the package that reads outside: %v", first.ReadsOutside)
+	}
+	a := "fixture/digest/a"
+	if first.Packages[a] != plain.Packages[a] || first.Packages[a] != second.Packages[a] {
+		t.Errorf("a has no marker and its digest moved: %s, %s, before %s", first.Packages[a], second.Packages[a], plain.Packages[a])
+	}
+}
+
+// Issue 1935: a catalogue module's packages read ../../module.json and the module's files: a change to them changes the
+// digest of the module's package, and a change to another module's files does not.
+func TestACatalogueModulesFilesChangeTheDigestOfItsPackagesOnly(t *testing.T) {
+	t.Parallel()
+	m, tree := digestFixture(t)
+	for _, mod := range []string{"mod1", "mod2"} {
+		dir := filepath.Join(tree, mod, "cmd", "p")
+		if err := os.MkdirAll(filepath.Join(tree, mod, "files"), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.MkdirAll(dir, 0o755); err != nil {
+			t.Fatal(err)
+		}
+		edit(t, tree, mod+"/module.json", `{"module":"`+mod+`"}`)
+		edit(t, tree, mod+"/README.md", "# "+mod+"\n")
+		edit(t, tree, mod+"/files/f.env", "A=1\n")
+		edit(t, tree, mod+"/cmd/p/p.go", "package p\n")
+		edit(t, tree, mod+"/cmd/p/p_test.go", "package p\n\nimport (\n\t\"os\"\n\t\"testing\"\n)\n\nfunc TestReads(t *testing.T) {\n\tif _, err := os.ReadFile(\"../../module.json\"); err != nil {\n\t\tt.Fatal(err)\n\t}\n}\n")
+	}
+	p1, p2 := "fixture/digest/mod1/cmd/p", "fixture/digest/mod2/cmd/p"
+	before := digests(t, m, "unit")
+	if len(before.Packages[p1]) != 64 || len(before.Packages[p2]) != 64 {
+		t.Fatalf("no digest for the module packages: %+v", before.Packages)
+	}
+	if before.Packages[p1] == before.Packages[p2] {
+		t.Errorf("two modules, one digest")
+	}
+	for _, file := range []string{"mod1/module.json", "mod1/README.md", "mod1/files/f.env"} {
+		edit(t, tree, file, "changed "+file+"\n")
+		after := digests(t, m, "unit")
+		if after.Packages[p1] == before.Packages[p1] {
+			t.Errorf("a change to %s did not change the module's digest", file)
+		}
+		if after.Packages[p2] != before.Packages[p2] {
+			t.Errorf("a change to %s changed the other module's digest", file)
+		}
+		before = after
+	}
+	if got := digests(t, m, "unit"); got.Packages["fixture/digest/c"] == "" {
+		t.Errorf("a package outside any module lost its digest")
+	}
+}

modules/mesh-build-go/cmd/mesh-build-go/reads-outside

@@ -0,0 +1 @@
+These tests read other catalogue modules' files, outside this module (issue 1935): never kept.

modules/mesh-build-image/cmd/mesh-build-image/reads-outside

@@ -0,0 +1 @@
+These tests read other catalogue modules' files, outside this module (issue 1935): never kept.

modules/mesh-build-node/cmd/mesh-build-node/reads-outside

@@ -0,0 +1 @@
+These tests read other catalogue modules' files, outside this module (issue 1935): never kept.

modules/mesh-provisioner-client/cmd/mesh-provisioner-client/reads-outside

@@ -0,0 +1 @@
+These tests read other catalogue modules' files, outside this module (issue 1935): never kept.

modules/minio/cmd/object-store/reads-outside

@@ -0,0 +1 @@
+These tests read other catalogue modules' files, outside this module (issue 1935): never kept.

modules/postgres/cmd/postgres-provider/reads-outside

@@ -0,0 +1 @@
+These tests read other catalogue modules' files, outside this module (issue 1935): never kept.