From b8ba9dd9669cbe747326dcf671095c605897a234 Mon Sep 17 00:00:00 2001 From: jochen Date: Wed, 28 Oct 2026 11:28:29 +0100 Subject: [PATCH] Widen the no-binaries check to every tracked file: a program under checks/ is refused too --- checks/nobinaries/nobinaries.go | 18 ++++++++----- checks/nobinaries/nobinaries_test.go | 39 ++++++++++++++++++++++++++-- checks/nobinaries/reads-outside | 6 +---- 3 files changed, 50 insertions(+), 13 deletions(-) diff --git a/checks/nobinaries/nobinaries.go b/checks/nobinaries/nobinaries.go index 55395bfa..6df00793 100644 --- a/checks/nobinaries/nobinaries.go +++ b/checks/nobinaries/nobinaries.go @@ -1,4 +1,4 @@ -// Package nobinaries refuses a compiled program under modules/ (issue 1577): it is build output, made by the module's build step, +// Package nobinaries refuses a compiled program anywhere in the repository (issue 1577; first a checks/ binary slipped past a modules/-only rule): it is build output, made by the module's build step, // and `go build` in a module's folder writes one beside go.mod where `git add -A` takes it. package nobinaries @@ -27,10 +27,10 @@ func Kind(head []byte) string { return "" } -// Files are the files of the catalogue's root that the change holds under modules/: the tracked ones when the root is a git -// checkout (an ignored binary a local build left is not the change's), else every file there. +// Files are the files of the catalogue's root that the change holds, anywhere: the tracked ones when the root is a git +// checkout (an ignored binary a local build left is not the change's), else every file there but .git's. func Files(root string) ([]string, error) { - cmd := exec.Command("git", "-C", root, "ls-files", "-z", "--", "modules") + cmd := exec.Command("git", "-C", root, "ls-files", "-z") if out, err := cmd.Output(); err == nil && len(out) > 0 { var files []string for _, f := range strings.Split(strings.TrimRight(string(out), "\x00"), "\x00") { @@ -41,10 +41,16 @@ func Files(root string) ([]string, error) { return files, nil } var files []string - err := filepath.WalkDir(filepath.Join(root, "modules"), func(p string, d os.DirEntry, err error) error { - if err != nil || d.IsDir() || !d.Type().IsRegular() { + err := filepath.WalkDir(root, func(p string, d os.DirEntry, err error) error { + if err != nil { return err } + if d.IsDir() && d.Name() == ".git" { + return filepath.SkipDir + } + if d.IsDir() || !d.Type().IsRegular() { + return nil + } rel, _ := filepath.Rel(root, p) files = append(files, rel) return nil diff --git a/checks/nobinaries/nobinaries_test.go b/checks/nobinaries/nobinaries_test.go index d26aeecd..4d874e99 100644 --- a/checks/nobinaries/nobinaries_test.go +++ b/checks/nobinaries/nobinaries_test.go @@ -2,6 +2,7 @@ package nobinaries import ( "os" + "os/exec" "path/filepath" "testing" ) @@ -16,6 +17,7 @@ func TestFindNamesEachCompiledProgramAndNothingElse(t *testing.T) { "modules/e/short": []byte("\x7f"), "modules/f/empty": {}, "modules/g/Foo.class": {0xca, 0xfe, 0xba, 0xbe, 0, 0, 0, 52}, // Java, not Mach-O + "checks/words/words": []byte("\x7fELF\x02\x01\x01\x00rest"), // outside modules/ too } var names []string for f, b := range files { @@ -32,8 +34,8 @@ func TestFindNamesEachCompiledProgramAndNothingElse(t *testing.T) { if err != nil { t.Fatal(err) } - if len(got) != 3 { - t.Fatalf("found %v; wanted modules/a/a, modules/b/cmd/b/b.exe and modules/c/c only", got) + if len(got) != 4 { + t.Fatalf("found %v; wanted modules/a/a, modules/b/cmd/b/b.exe and modules/c/c and checks/words/words only", got) } } @@ -52,3 +54,36 @@ func TestTheCatalogueHoldsNoCompiledProgram(t *testing.T) { t.Errorf("%s: a compiled program is build output: remove it and add it to .gitignore (issue 1577)", f) } } + +// A compiled program outside modules/ is refused too (a binary under checks/ slipped past the modules/-only rule): Files holds +// every tracked file of a git checkout and every file of a plain folder, and Find names the binary in both. +func TestAProgramUnderChecksIsRefused(t *testing.T) { + for _, git := range []bool{true, false} { + root := t.TempDir() + p := filepath.Join(root, "checks", "words", "words") + if err := os.MkdirAll(filepath.Dir(p), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(p, []byte("\x7fELF\x02\x01\x01\x00rest"), 0o644); err != nil { + t.Fatal(err) + } + if git { + for _, args := range [][]string{{"init", "-q"}, {"add", "-A"}} { + if out, err := exec.Command("git", append([]string{"-C", root}, args...)...).CombinedOutput(); err != nil { + t.Fatalf("git %v: %v: %s", args, err, out) + } + } + } + files, err := Files(root) + if err != nil { + t.Fatal(err) + } + found, err := Find(root, files) + if err != nil { + t.Fatal(err) + } + if len(found) != 1 { + t.Errorf("git=%v: found %v; wanted checks/words/words (ELF)", git, found) + } + } +} diff --git a/checks/nobinaries/reads-outside b/checks/nobinaries/reads-outside index 8c970936..2608ca02 100644 --- a/checks/nobinaries/reads-outside +++ b/checks/nobinaries/reads-outside @@ -1,5 +1 @@ -# What these tests read outside their package, as paths from the repository's root (a glob, or a directory read whole; -# ? before a path whose absence is read). mesh-build-go hashes them into the package's digest, so its verdict is kept -# until one of them changes (issue 1936). An empty list, or a line matching nothing, keeps nothing. -# every file under modules/ (git ls-files and a walk). -modules +# every tracked file of the repository (git ls-files, no path limit): the whole tree, so this package's verdict is never kept.