From 9b86582709c2a6d21919c448b4650dc9c4ac6511 Mon Sep 17 00:00:00 2001 From: Manuel de Brito Fontes Date: Fri, 13 Nov 2026 02:47:58 -0300 Subject: [PATCH] testidentity: no test identity reaches a commit, and no test's git the checkout Twice a test in spin wrote its author into the checkout's .git/config and every worktree committed as it, to GitHub. Two halves, the same as spin#1196's: - internal/gittest is how a test here runs git: under the temporary directory, with git's search stopped at the checkout and the temporary directory, no inherited GIT_ variable, no global or system configuration, and an author at go-tools.invalid. A package whose own code runs git fences its tests in TestMain. internal/gitexec's test fails on a test that runs git another way, and on such a package without the fence. mutate's TestTheLinesAChange... ran `git config user.email` in a fresh temp repository - safe until a hook's GIT_DIR or a lost init sends it to the checkout - and goes through gittest like the rest. - cmd/testidentity, released with the other commands so every repository runs it under `go tool task`: a commit since the base whose author or committer is at a reserved domain, has no dot, or is spelled in the tree's tests and fixtures, fails, with what to do. GitHub's noreply and bot addresses are real by rule; -allow is a ratchet with reasons. task lint runs it on this repository. Co-Authored-By: Claude Opus 5.5 --- README.md | 1 + Taskfile.yml | 6 +- cmd/mutate/example_test.go | 8 +- cmd/mutate/fence_test.go | 15 + cmd/proven/fence_test.go | 15 + cmd/proven/main_test.go | 6 +- cmd/testidentity/main.go | 40 +++ internal/exposure/exposure_test.go | 8 +- internal/exposure/fence_test.go | 15 + internal/gitexec/gitexec.go | 195 ++++++++++++ internal/gitexec/gitexec_test.go | 134 +++++++++ internal/gittest/gittest.go | 143 +++++++++ internal/gittest/gittest_test.go | 71 +++++ internal/gittest/refused_internal_test.go | 30 ++ internal/mutate/fence_test.go | 15 + internal/mutate/mutate_test.go | 8 +- internal/mutate/tree_internal_test.go | 9 +- internal/proven/fence_test.go | 15 + internal/proven/github_test.go | 6 +- internal/refs/fence_test.go | 15 + internal/refs/refs_test.go | 7 +- .../release/cmd/releasename/fence_test.go | 15 + internal/sfw/fence_test.go | 15 + internal/sfw/sfw_test.go | 6 +- internal/taskgate/example_test.go | 8 +- internal/taskgate/fence_test.go | 15 + internal/taskgate/taskfile_test.go | 12 +- internal/taskgate/taskgate_test.go | 9 +- internal/testidentity/run.go | 104 +++++++ internal/testidentity/testidentity.go | 184 ++++++++++++ internal/testidentity/testidentity_test.go | 282 ++++++++++++++++++ site/src/content/docs/index.mdx | 5 + site/src/content/docs/testidentity.md | 76 +++++ site/src/lib/nav.mjs | 1 + versions/gate/fence_test.go | 15 + versions/gate/gate_test.go | 6 +- versions/upstream/check_test.go | 9 +- versions/upstream/upstream_test.go | 9 +- 38 files changed, 1461 insertions(+), 72 deletions(-) create mode 100644 cmd/mutate/fence_test.go create mode 100644 cmd/proven/fence_test.go create mode 100644 cmd/testidentity/main.go create mode 100644 internal/exposure/fence_test.go create mode 100644 internal/gitexec/gitexec.go create mode 100644 internal/gitexec/gitexec_test.go create mode 100644 internal/gittest/gittest.go create mode 100644 internal/gittest/gittest_test.go create mode 100644 internal/gittest/refused_internal_test.go create mode 100644 internal/mutate/fence_test.go create mode 100644 internal/proven/fence_test.go create mode 100644 internal/refs/fence_test.go create mode 100644 internal/release/cmd/releasename/fence_test.go create mode 100644 internal/sfw/fence_test.go create mode 100644 internal/taskgate/fence_test.go create mode 100644 internal/testidentity/run.go create mode 100644 internal/testidentity/testidentity.go create mode 100644 internal/testidentity/testidentity_test.go create mode 100644 site/src/content/docs/testidentity.md create mode 100644 versions/gate/fence_test.go diff --git a/README.md b/README.md index db17af2..8f4e432 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ their `go.mod`; each fails CI with a specific finding and says how to resolve it | [`versions`](https://spin-stack.github.io/go-tools/versions/) | pins written outside `versions.yaml`, and pins behind their upstream | | [`sfw`](https://spin-stack.github.io/go-tools/sfw/) | npm, npx or yarn, and a pnpm install that does not go through Socket Firewall | | [`taskgate`](https://spin-stack.github.io/go-tools/taskgate/) | a workflow step that is not one `task`, or names one the Taskfile does not have, a task that holds a program rather than a few commands, and a script a task runs that decides anything - so CI and a developer run the same thing, and what decides is Go with a test | +| [`testidentity`](https://spin-stack.github.io/go-tools/testidentity/) | a commit of the branch whose author or committer is a test's - an address at a domain reserved for testing, or one the tree's tests spell - which a test that wrote git's author into the checkout leaves on every commit after it | | [`deadgate`](https://spin-stack.github.io/go-tools/deadgate/) | a function the binaries link and never call, or a package none imports, that neither the allowlist nor the pending list explains - and an entry in either that explains nothing | | [`linkweight`](https://spin-stack.github.io/go-tools/linkweight/) | what each binary links and why: the bytes every module puts in it, the function that stops the linker dropping unused methods, the import whose cut drops the most, and a capability - exec, listen, mount, setuid - no line of the allowlist explains | | [`proven`](https://spin-stack.github.io/go-tools/proven/) | what CI already proved about a commit, by its tree: the run that answers for it, what a run on main is of, whether it may be released, each test's time, which failed tests are flaky | diff --git a/Taskfile.yml b/Taskfile.yml index 848cb2d..3d9ccf8 100644 --- a/Taskfile.yml +++ b/Taskfile.yml @@ -2,7 +2,8 @@ # # task test the Go tests, under the race detector, held to a coverage floor # task lint golangci-lint with spin's rules, at spin's version, this repository's own -# gates run on itself: ctxlife, testquality, refs, sfw, taskgate, and actionlint +# gates run on itself: ctxlife, testquality, refs, sfw, taskgate, testidentity, +# and actionlint # task mutate break what the change against BASE (origin/main) touched, and ask the tests # task coverage coverage.out's numbers, to the terminal or the workflow run's summary # task trace where a workflow run's time went (ghatrace), to its summary and _output/ghatrace @@ -62,6 +63,9 @@ tasks: - go run ./cmd/mutate -stale-exempts - go run ./cmd/sfw check - go run ./cmd/taskgate -allow .github/taskgate-allow.txt -task {{.TASK_EXE}} + # A commit since main whose author is a test's: what a test's git wrote into the checkout's + # config. On main, the range is empty. + - go run ./cmd/testidentity -base origin/main # A vulnerability the code calls and that has a fix fails, the standard library's too: the # answer is the upgrade. It asks vuln.go.dev, so lint needs the network. - go run ./cmd/vulngate ./... diff --git a/cmd/mutate/example_test.go b/cmd/mutate/example_test.go index 48c3aa0..f90a975 100644 --- a/cmd/mutate/example_test.go +++ b/cmd/mutate/example_test.go @@ -11,6 +11,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/spin-stack/go-tools/internal/gittest" ) // The example the site shows (site/src/content/docs/mutate.mdx) is this command run over @@ -31,12 +33,8 @@ func TestTheExampleTheSiteShows(t *testing.T) { repo := t.TempDir() git := func(args ...string) { - cmd := exec.Command("git", append([]string{"-c", "user.name=example", "-c", "user.email=example@example.com", "-c", "commit.gpgsign=false"}, args...)...) - cmd.Dir = repo // The machine's own git configuration has no say in what the example prints. - cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL="+os.DevNull, "GIT_CONFIG_SYSTEM="+os.DevNull) - out, err := cmd.CombinedOutput() - require.NoError(t, err, "git %v: %s", args, out) + gittest.Run(t, repo, args...) } put := func(from, to string) { raw, err := os.ReadFile(filepath.Join(example, from)) diff --git a/cmd/mutate/fence_test.go b/cmd/mutate/fence_test.go new file mode 100644 index 0000000..583998a --- /dev/null +++ b/cmd/mutate/fence_test.go @@ -0,0 +1,15 @@ +package main + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The command runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/cmd/proven/fence_test.go b/cmd/proven/fence_test.go new file mode 100644 index 0000000..583998a --- /dev/null +++ b/cmd/proven/fence_test.go @@ -0,0 +1,15 @@ +package main + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The command runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/cmd/proven/main_test.go b/cmd/proven/main_test.go index eede35a..33c5998 100644 --- a/cmd/proven/main_test.go +++ b/cmd/proven/main_test.go @@ -8,7 +8,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -16,6 +15,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/proven" ) @@ -62,9 +62,7 @@ func setup(t *testing.T) *fixture { root := t.TempDir() git := func(args ...string) string { t.Helper() - out, err := exec.Command("git", append([]string{"-C", root, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false"}, args...)...).CombinedOutput() - require.NoError(t, err, "git %v: %s", args, out) - return strings.TrimSpace(string(out)) + return gittest.Run(t, root, args...) } git("init", "-q", "-b", "main") for _, name := range []string{"one", "two", "three"} { diff --git a/cmd/testidentity/main.go b/cmd/testidentity/main.go new file mode 100644 index 0000000..2dbfa8a --- /dev/null +++ b/cmd/testidentity/main.go @@ -0,0 +1,40 @@ +// Command testidentity fails on a commit of the branch - every one in BASE..HEAD - whose author +// or committer is one of the identities a test uses: an address at a domain reserved for testing, +// one with no dot, or one the tree's tests and fixtures spell. A test that wrote git's author into +// a checkout's config makes every commit from it, every worktree's, that test's; this is what +// stops those commits before they are pushed. +// +// go tool testidentity [-base origin/main] [-allow file] [dir] +// +// -allow names an address that reads as a test's and is somebody's real author, one +// `
# why` a line; a line whose address is not a test identity any more fails too. +// GitHub's own addresses - its Apps' and users' noreply, and noreply@github.com - need no line. +package main + +import ( + "context" + "errors" + "flag" + "fmt" + "os" + + "github.com/spin-stack/go-tools/internal/testidentity" +) + +func main() { + base := flag.String("base", "origin/main", "the commit the branch's own commits start after") + allowPath := flag.String("allow", "", "addresses that are somebody's and read as a test's, `
# why` a line (none by default)") + flag.Parse() + root := "." + if flag.NArg() > 0 { + root = flag.Arg(0) + } + err := testidentity.Run(context.Background(), root, *base, *allowPath, os.Stdout) + switch { + case errors.Is(err, testidentity.ErrFound): + os.Exit(1) + case err != nil: + fmt.Fprintf(os.Stderr, "testidentity: %v\n", err) //nolint:errcheck // the exit status is the answer + os.Exit(2) + } +} diff --git a/internal/exposure/exposure_test.go b/internal/exposure/exposure_test.go index c2c7332..9572f08 100644 --- a/internal/exposure/exposure_test.go +++ b/internal/exposure/exposure_test.go @@ -5,7 +5,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -14,6 +13,7 @@ import ( "github.com/stretchr/testify/require" "github.com/spin-stack/go-tools/internal/exposure" + "github.com/spin-stack/go-tools/internal/gittest" ) // Each kind finds what is someone's and passes over what is no one's: AWS's documented example @@ -67,9 +67,9 @@ func TestARepositorysOwnKindsAreRead(t *testing.T) { func TestEveryRefIsReadAndAPullRequestsHeadToo(t *testing.T) { dir := t.TempDir() git := func(args ...string) string { - cmd := exec.Command("git", append([]string{"-c", "user.name=Ana", "-c", "user.email=ana@acme.io", "-c", "commit.gpgsign=false", "-c", "init.defaultBranch=main"}, args...)...) - cmd.Dir = dir - cmd.Env = append(os.Environ(), "GIT_CONFIG_GLOBAL="+os.DevNull, "GIT_CONFIG_SYSTEM="+os.DevNull) + cmd := gittest.Command(t, dir, append([]string{"-c", "init.defaultBranch=main"}, args...)...) + // The author whose address the scan reads, over gittest's own. + cmd.Env = append(cmd.Env, "GIT_AUTHOR_NAME=Ana", "GIT_AUTHOR_EMAIL=ana@acme.io", "GIT_COMMITTER_NAME=Ana", "GIT_COMMITTER_EMAIL=ana@acme.io") out, err := cmd.CombinedOutput() require.NoError(t, err, "git %v: %s", args, out) return strings.TrimSpace(string(out)) diff --git a/internal/exposure/fence_test.go b/internal/exposure/fence_test.go new file mode 100644 index 0000000..ad14af7 --- /dev/null +++ b/internal/exposure/fence_test.go @@ -0,0 +1,15 @@ +package exposure_test + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/gitexec/gitexec.go b/internal/gitexec/gitexec.go new file mode 100644 index 0000000..ecbe548 --- /dev/null +++ b/internal/gitexec/gitexec.go @@ -0,0 +1,195 @@ +// Package gitexec holds this repository's tests to running git through internal/gittest. +// +// git looks for its repository upward from where it runs, and a test runs inside the checkout, +// so a git a test meant for a repository it made can land on the checkout's own config - which +// every worktree and every agent commits from. In spin that put a test's author on commits +// pushed to GitHub, twice. internal/gittest is the fence; this is what makes it the only way, +// decided from the syntax: +// +// - a test file that execs git itself, rather than through gittest.Command; +// - a package whose own code execs git, with tests and no gittest.Fence() in them: its tests +// run that git, and one that loses its -C - a mutant mutate makes - finds the checkout. +// +// What it sees is os/exec's Command or CommandContext with the program spelled "git". A wrapper +// that takes the program as an argument is not seen. +package gitexec + +import ( + "fmt" + "go/ast" + "go/parser" + "go/token" + "io/fs" + "path/filepath" + "strconv" + "strings" +) + +// Finding is one git a test can run unfenced, as a line to go to. Key is what an allow line +// names: the test file, or the package's directory. +type Finding struct { + File string + Line int + Key string + Why string +} + +func (f Finding) String() string { return fmt.Sprintf("%s:%d: %s", f.File, f.Line, f.Why) } + +// fence is the package whose Command and Fence are the way a test runs git. +const fence = "internal/gittest" + +type pkg struct { + runs []Finding // the package's own code's execs of git + tested []Finding // its test files' own execs of git + tests bool + fenced bool +} + +// Check walks root as go walks a module - no directory whose name starts with . or _, no +// testdata - and reports every git a test can run unfenced, in the order of the tree. +func Check(root string) ([]Finding, error) { + pkgs := &tree{byDir: map[string]*pkg{}} + err := filepath.WalkDir(root, func(path string, d fs.DirEntry, err error) error { + if err != nil { + return err + } + name := d.Name() + if d.IsDir() { + if path != root && (strings.HasPrefix(name, ".") || strings.HasPrefix(name, "_") || name == "testdata" || name == "node_modules") { + return filepath.SkipDir + } + return nil + } + if !strings.HasSuffix(name, ".go") { + return nil + } + return read(root, path, pkgs) + }) + if err != nil { + return nil, err + } + var found []Finding + for _, p := range pkgs.inOrder { + found = append(found, p.tested...) + if p.tests && !p.fenced { + found = append(found, p.runs...) + } + } + return found, nil +} + +// tree is the packages read, by directory and in the order the walk met them - which is the +// order their findings are said in, the same on every run. +type tree struct { + byDir map[string]*pkg + inOrder []*pkg +} + +func (t *tree) of(dir string) *pkg { + p := t.byDir[dir] + if p == nil { + p = &pkg{} + t.byDir[dir] = p + t.inOrder = append(t.inOrder, p) + } + return p +} + +// read adds the Go file at path to its package in pkgs: its execs of git, and whether it is a +// test and calls gittest.Fence. +func read(root, path string, pkgs *tree) error { + rel, err := filepath.Rel(root, path) + if err != nil { + return err + } + dir := filepath.ToSlash(filepath.Dir(rel)) + if dir == fence { + return nil + } + fset := token.NewFileSet() + file, err := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) + if err != nil { + return err + } + p := pkgs.of(dir) + test := strings.HasSuffix(path, "_test.go") + for _, line := range gitExecs(fset, file) { + f := Finding{File: filepath.ToSlash(rel), Line: line} + if test { + f.Key, f.Why = f.File, "a test runs git itself: run it through gittest.Command (internal/gittest), "+ + "which keeps git off any repository the test did not make" + p.tested = append(p.tested, f) + } else { + f.Key, f.Why = dir, "this package's code runs git and its tests do not fence it: call gittest.Fence() "+ + "first in its TestMain (internal/gittest), or a git of it that misses its -C writes the checkout's config" + p.runs = append(p.runs, f) + } + } + if test { + p.tests = true + p.fenced = p.fenced || callsFence(file) + } + return nil +} + +// programArg is which argument of os/exec's constructors names the program. +var programArg = map[string]int{"Command": 0, "CommandContext": 1} + +// gitExecs is the line of every os/exec Command or CommandContext in file whose program is "git". +func gitExecs(fset *token.FileSet, file *ast.File) []int { + name := "" + for _, imp := range file.Imports { + if imp.Path.Value == `"os/exec"` { + name = "exec" + if imp.Name != nil { + name = imp.Name.Name + } + } + } + if name == "" { + return nil + } + var lines []int + ast.Inspect(file, func(n ast.Node) bool { + call, ok := n.(*ast.CallExpr) + if !ok { + return true + } + sel, ok := call.Fun.(*ast.SelectorExpr) + if !ok { + return true + } + if x, ok := sel.X.(*ast.Ident); !ok || x.Name != name { + return true + } + // Both take the program as a parameter of their own, so code that builds has the argument. + program, ok := programArg[sel.Sel.Name] + if !ok { + return true + } + if lit, ok := call.Args[program].(*ast.BasicLit); ok && lit.Kind == token.STRING { + if s, err := strconv.Unquote(lit.Value); err == nil && s == "git" { + lines = append(lines, fset.Position(call.Pos()).Line) + } + } + return true + }) + return lines +} + +// callsFence is whether file calls gittest.Fence. +func callsFence(file *ast.File) bool { + found := false + ast.Inspect(file, func(n ast.Node) bool { + if call, ok := n.(*ast.CallExpr); ok { + if sel, ok := call.Fun.(*ast.SelectorExpr); ok && sel.Sel.Name == "Fence" { + if x, ok := sel.X.(*ast.Ident); ok && x.Name == "gittest" { + found = true + } + } + } + return !found + }) + return found +} diff --git a/internal/gitexec/gitexec_test.go b/internal/gitexec/gitexec_test.go new file mode 100644 index 0000000..4a375bc --- /dev/null +++ b/internal/gitexec/gitexec_test.go @@ -0,0 +1,134 @@ +package gitexec_test + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/spin-stack/go-tools/internal/gitexec" +) + +const ( + runsGit = "package p\n\nimport \"os/exec\"\n\nfunc run() error { return exec.Command(\"git\", \"config\", \"user.name\", \"Ana\").Run() }\n" + fenced = "package p\n\nimport (\n\t\"testing\"\n\n\t\"github.com/spin-stack/go-tools/internal/gittest\"\n)\n\nfunc TestMain(m *testing.M) { gittest.Fence(); m.Run() }\n" + plain = "package p\n\nimport \"testing\"\n\nfunc TestRun(t *testing.T) { _ = run() }\n" +) + +// This repository's own tests: every git one runs goes through internal/gittest, or runs in a +// package whose TestMain fences it. A finding here is the test to change, not a line to allow. +func TestNoTestHereRunsGitUnfenced(t *testing.T) { + found, err := gitexec.Check(filepath.Join("..", "..")) + require.NoError(t, err) + for _, f := range found { + t.Error(f) + } +} + +// Which trees hold a git a test can run unfenced: each case is a module of its own, and the +// findings are named by the test file or the package that has to change. +func TestAGitATestCanRunUnfencedIsFound(t *testing.T) { + for _, tc := range []struct { + name string + files map[string]string + want []string + }{ + { + name: "a test's own exec of git", + files: map[string]string{"p/a_test.go": "package p\n\nimport (\n\t\"os/exec\"\n\t\"testing\"\n)\n\nfunc TestA(t *testing.T) { _ = exec.CommandContext(t.Context(), \"git\", \"init\").Run() }\n"}, + want: []string{"p/a_test.go"}, + }, + { + name: "under another name for os/exec", + files: map[string]string{"p/a_test.go": "package p\n\nimport (\n\tx \"os/exec\"\n\t\"testing\"\n)\n\nfunc TestA(t *testing.T) { _ = x.Command(\"git\", \"init\").Run() }\n"}, + want: []string{"p/a_test.go"}, + }, + { + name: "each, in the order of the tree", + files: map[string]string{ + "b/x_test.go": "package b\n\nimport \"os/exec\"\n\nvar _ = exec.Command(\"git\")\n", + "a/x_test.go": "package a\n\nimport \"os/exec\"\n\nvar _ = exec.Command(\"git\")\n", + }, + want: []string{"a/x_test.go", "b/x_test.go"}, + }, + { + name: "a lookup of git, which runs nothing", + files: map[string]string{"p/a_test.go": "package p\n\nimport \"os/exec\"\n\nvar _, _ = exec.LookPath(\"git\")\n"}, + }, + { + name: "a program named by a variable, which this cannot read", + files: map[string]string{"p/a_test.go": "package p\n\nimport \"os/exec\"\n\nfunc run(name string) { _ = exec.Command(name).Run() }\n"}, + }, + { + name: "a test's exec of another program", + files: map[string]string{"p/a_test.go": "package p\n\nimport (\n\t\"os/exec\"\n\t\"testing\"\n)\n\nfunc TestA(t *testing.T) { _ = exec.Command(\"ssh-keygen\", \"git\").Run() }\n"}, + }, + { + name: "a call of another package's Command", + files: map[string]string{"p/a_test.go": "package p\n\nimport (\n\t\"os/exec\"\n\t\"testing\"\n)\n\nvar _ = exec.ErrNotFound\n\nfunc TestA(t *testing.T) { _ = other.Command(\"git\") }\n"}, + }, + { + name: "a package's git its tests run unfenced", + files: map[string]string{"p/run.go": runsGit, "p/run_test.go": plain}, + want: []string{"p"}, + }, + { + name: "a package's git its TestMain fences", + files: map[string]string{"p/run.go": runsGit, "p/run_test.go": plain, "p/main_test.go": fenced}, + }, + { + name: "a package's git its TestMain fences with another package's Fence", + files: map[string]string{ + "p/run.go": runsGit, + "p/run_test.go": plain, + "p/main_test.go": "package p\n\nimport \"testing\"\n\nfunc TestMain(m *testing.M) { other.Fence(); m.Run() }\n", + }, + want: []string{"p"}, + }, + { + name: "a package's git no test runs", + files: map[string]string{"p/run.go": runsGit}, + }, + { + name: "what go does not build: testdata, a worktree, a _ directory, gittest itself", + files: map[string]string{ + "p/testdata/a_test.go": "package p\n\nimport \"os/exec\"\n\nvar _ = exec.Command(\"git\")\n", + ".wt/x/p/a_test.go": "package p\n\nimport \"os/exec\"\n\nvar _ = exec.Command(\"git\")\n", + "_output/p/a_test.go": "package p\n\nimport \"os/exec\"\n\nvar _ = exec.Command(\"git\")\n", + "node_modules/p/a_test.go": "package p\n\nimport \"os/exec\"\n\nvar _ = exec.Command(\"git\")\n", + "internal/gittest/gittest.go": runsGit, + "internal/gittest/gittest_test.go": plain, + "notes.txt": "exec.Command(\"git\")\n", + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + root := t.TempDir() + for name, body := range tc.files { + require.NoError(t, os.MkdirAll(filepath.Join(root, filepath.Dir(name)), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(root, name), []byte(body), 0o600)) + } + found, err := gitexec.Check(root) + require.NoError(t, err) + var keys []string + for _, f := range found { + keys = append(keys, f.Key) + assert.NotEmpty(t, f.Why) + assert.Positive(t, f.Line) + } + assert.Equal(t, tc.want, keys) + }) + } +} + +// A finding reads as a line to go to, and a file go cannot parse is an error, not a pass. +func TestAFindingSaysWhereAndAnUnparsableFileFails(t *testing.T) { + assert.Equal(t, "p/a_test.go:7: why", gitexec.Finding{File: "p/a_test.go", Line: 7, Why: "why"}.String()) + + root := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(root, "broken.go"), []byte("package"), 0o600)) + _, err := gitexec.Check(root) + require.Error(t, err) +} diff --git a/internal/gittest/gittest.go b/internal/gittest/gittest.go new file mode 100644 index 0000000..35c2aab --- /dev/null +++ b/internal/gittest/gittest.go @@ -0,0 +1,143 @@ +// Package gittest runs git for a test where it cannot reach a repository the test did not make. +// +// A test runs inside a checkout - go test's working directory is the package's - and git looks for +// its repository upward from where it runs. So a git the test meant for a repository it made, run +// one level off, is a git on the checkout every worktree and every agent commits from: twice a +// test in spin set its author there (`ana@corp.example`, 2026-10 and 2026-11-12), and commits +// carried it to GitHub. Once it was a mutant of the code under test that lost its -C; once a +// TMPDIR inside the checkout made the "not a repository" the test asked for the checkout's own +// config. A hook's GIT_DIR, inherited by a test a hook runs, points every git at the checkout too. +// +// Command is the way a test here runs git (internal/gitexec refuses another), and Fence is what a +// package whose own code runs git calls in its TestMain: both stop git's search at the checkout +// and at the temporary directory, drop every GIT_ variable the test inherited, and leave out every +// configuration the machine has. +package gittest + +import ( + "os" + "os/exec" + "path/filepath" + "slices" + "strings" + "testing" +) + +// Identity is the author and committer of what a test commits through Command: an address at a +// reserved domain, which cmd/testidentity refuses on a commit of any repository. +const ( + IdentityName = "gittest" + IdentityEmail = "gittest@go-tools.invalid" +) + +// Command is git with args in dir, which must be under the temporary directory: the repository +// is the one there, or none, never one above it. An option that names another repository (-C, +// --git-dir, --work-tree) is refused - it is the escape this exists to close. Env holds the +// fence and Identity; a caller appends to it to set more, and the last value of a key wins. +func Command(t *testing.T, dir string, args ...string) *exec.Cmd { + t.Helper() + if why := refused(dir, args); why != "" { + t.Fatal("gittest: " + why) + } + cmd := exec.CommandContext(t.Context(), "git", args...) //nolint:gosec // a test's own arguments + cmd.Dir = dir + cmd.Env = append(append(withoutGit(os.Environ()), fence()...), + "GIT_AUTHOR_NAME="+IdentityName, "GIT_AUTHOR_EMAIL="+IdentityEmail, + "GIT_COMMITTER_NAME="+IdentityName, "GIT_COMMITTER_EMAIL="+IdentityEmail) + return cmd +} + +// refused says why git with args in dir could reach a repository the test did not make, or "" +// when it cannot. +func refused(dir string, args []string) string { + tmp := resolved(os.TempDir()) + if d := resolved(dir); d != tmp && !strings.HasPrefix(d, tmp+string(filepath.Separator)) { + return "git runs under the temporary directory " + tmp + ", not in " + dir + } + for _, a := range args { + if a == "-C" || strings.HasPrefix(a, "--git-dir") || strings.HasPrefix(a, "--work-tree") { + return a + " names a repository other than " + dir + "'s; run the command in that directory" + } + } + return "" +} + +// Run is Command run, failing the test with git's words when git fails; it answers git's +// output, trimmed. +func Run(t *testing.T, dir string, args ...string) string { + t.Helper() + out, err := Command(t, dir, args...).CombinedOutput() + if err != nil { + t.Fatalf("git %v in %s: %v: %s", args, dir, err, out) + } + return strings.TrimSpace(string(out)) +} + +// Fence puts the fence in this process's environment, for every git the code under test starts: +// called first in the TestMain of a package whose own code runs git (internal/gitexec holds +// it). A git there that lost its -C then finds no repository, rather than the checkout's. +func Fence() { + for _, kv := range os.Environ() { + if k, _, _ := strings.Cut(kv, "="); strings.HasPrefix(k, "GIT_") { + _ = os.Unsetenv(k) //nolint:errcheck // fails only on an empty or malformed name, which Environ does not give + } + } + for _, kv := range fence() { + k, v, _ := strings.Cut(kv, "=") + _ = os.Setenv(k, v) //nolint:errcheck // fails only on an empty or malformed name, and these are constants + } +} + +// withoutGit is env with no GIT_ variable: a hook's GIT_DIR or GIT_INDEX_FILE points every git +// at the checkout. +func withoutGit(env []string) []string { + return slices.DeleteFunc(env, func(kv string) bool { + k, _, _ := strings.Cut(kv, "=") + return strings.HasPrefix(k, "GIT_") + }) +} + +// fence is git's search stopped short of the checkout and of the temporary directory, with no +// global or system configuration read. +func fence() []string { + ceilings := []string{resolved(os.TempDir())} + if top := checkout(); top != "" { + ceilings = append(ceilings, top) + } + return []string{ + "GIT_CEILING_DIRECTORIES=" + strings.Join(ceilings, string(filepath.ListSeparator)), + "GIT_CONFIG_GLOBAL=" + os.DevNull, + "GIT_CONFIG_NOSYSTEM=1", + } +} + +// checkout is the nearest directory at or above the working directory that holds a .git - a +// directory, or a worktree's file - or "" when there is none. +func checkout() string { + dir, err := os.Getwd() + if err != nil { + return "" + } + for dir = resolved(dir); ; { + if _, err := os.Lstat(filepath.Join(dir, ".git")); err == nil { + return dir + } + parent := filepath.Dir(dir) + if parent == dir { + return "" + } + dir = parent + } +} + +// resolved is path absolute and with its links followed, as git compares ceilings: a TMPDIR +// behind a link is otherwise a ceiling git never meets. +func resolved(path string) string { + if abs, err := filepath.Abs(path); err == nil { + path = abs + } + if real, err := filepath.EvalSymlinks(path); err == nil { + path = real + } + return path +} diff --git a/internal/gittest/gittest_test.go b/internal/gittest/gittest_test.go new file mode 100644 index 0000000..d63242b --- /dev/null +++ b/internal/gittest/gittest_test.go @@ -0,0 +1,71 @@ +package gittest_test + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// outer is a repository standing for the checkout a test runs in, with a directory inside it. +func outer(t *testing.T) (repo, inside string) { + t.Helper() + repo = t.TempDir() + gittest.Run(t, repo, "init", "-q") + inside = filepath.Join(repo, "tmp", "x") + require.NoError(t, os.MkdirAll(inside, 0o700)) + return repo, inside +} + +// The ways a test's author reaches the checkout, each refused: a test's own git in a directory +// that is not a repository, under a TMPDIR inside the checkout; the code under test's git run +// without its -C in a package of the checkout; and a GIT_DIR a hook left in the environment. +func TestAGitOffItsRepositoryWritesNoOtherRepositorysConfig(t *testing.T) { + for _, tc := range []struct { + name string + run func(t *testing.T, repo, inside string) ([]byte, error) + }{ + {"a test's git under a TMPDIR inside the checkout", func(t *testing.T, repo, inside string) ([]byte, error) { + t.Setenv("TMPDIR", filepath.Join(repo, "tmp")) + return gittest.Command(t, inside, "config", "user.name", "Ana Lima").CombinedOutput() + }}, + {"a test's git under a hook's GIT_DIR", func(t *testing.T, repo, inside string) ([]byte, error) { + t.Setenv("TMPDIR", filepath.Join(repo, "tmp")) + t.Setenv("GIT_DIR", filepath.Join(repo, ".git")) + return gittest.Command(t, inside, "config", "user.name", "Ana Lima").CombinedOutput() + }}, + {"the code under test's git in the checkout, fenced", func(t *testing.T, repo, inside string) ([]byte, error) { + for _, k := range []string{"GIT_CEILING_DIRECTORIES", "GIT_CONFIG_GLOBAL", "GIT_CONFIG_NOSYSTEM"} { + t.Setenv(k, "") + } + t.Setenv("GIT_DIR", filepath.Join(repo, ".git")) + t.Chdir(inside) + gittest.Fence() + return exec.CommandContext(t.Context(), "git", "config", "user.name", "Ana Lima").CombinedOutput() + }}, + } { + t.Run(tc.name, func(t *testing.T) { + repo, inside := outer(t) + out, err := tc.run(t, repo, inside) + require.Error(t, err, "git wrote: %s", out) + assert.Contains(t, string(out), "not in a git directory") + config, err := os.ReadFile(filepath.Join(repo, ".git", "config")) + require.NoError(t, err) + assert.NotContains(t, string(config), "Ana Lima") + }) + } +} + +// What a test commits says gittest's identity - an address the authors gate refuses - and not +// whoever the machine's configuration names. +func TestATestsCommitIsGittests(t *testing.T) { + repo := t.TempDir() + gittest.Run(t, repo, "init", "-q") + gittest.Run(t, repo, "commit", "-q", "--allow-empty", "-m", "one") + assert.Equal(t, gittest.IdentityEmail+" "+gittest.IdentityEmail, gittest.Run(t, repo, "log", "-1", "--format=%ae %ce")) +} diff --git a/internal/gittest/refused_internal_test.go b/internal/gittest/refused_internal_test.go new file mode 100644 index 0000000..c76fd9d --- /dev/null +++ b/internal/gittest/refused_internal_test.go @@ -0,0 +1,30 @@ +package gittest + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +// A git outside the temporary directory, or one naming another repository, is refused before it +// runs; one in the temporary directory or under it, naming none, is not. +func TestAGitNamingAnotherRepositoryIsRefused(t *testing.T) { + tmp := t.TempDir() + for _, tc := range []struct { + dir string + args []string + refused bool + }{ + {os.TempDir(), []string{"status"}, false}, + {filepath.Join(tmp, "x"), []string{"-c", "user.name=x", "commit"}, false}, + {"/", []string{"status"}, true}, + {os.TempDir() + "-sibling", []string{"status"}, true}, + {tmp, []string{"-C", "/", "status"}, true}, + {tmp, []string{"--git-dir=/x/.git", "status"}, true}, + {tmp, []string{"--work-tree", "/", "status"}, true}, + } { + assert.Equal(t, tc.refused, refused(tc.dir, tc.args) != "", "git %v in %s", tc.args, tc.dir) + } +} diff --git a/internal/mutate/fence_test.go b/internal/mutate/fence_test.go new file mode 100644 index 0000000..aac7271 --- /dev/null +++ b/internal/mutate/fence_test.go @@ -0,0 +1,15 @@ +package mutate_test + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/mutate/mutate_test.go b/internal/mutate/mutate_test.go index 88f619b..c14ddb8 100644 --- a/internal/mutate/mutate_test.go +++ b/internal/mutate/mutate_test.go @@ -3,7 +3,6 @@ package mutate_test import ( "encoding/json" "os" - "os/exec" "path/filepath" "strings" "testing" @@ -11,6 +10,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/mutate" ) @@ -317,13 +317,9 @@ func TestTheLinesAChangeTouchesAreTheOnesItChanged(t *testing.T) { t.Chdir(dir) run := func(args ...string) { t.Helper() - cmd := exec.Command("git", args...) - out, err := cmd.CombinedOutput() - require.NoError(t, err, "git %s: %s", strings.Join(args, " "), out) + gittest.Run(t, dir, args...) } run("init", "-q", "-b", "main") - run("config", "user.email", "test@example.com") - run("config", "user.name", "test") require.NoError(t, os.WriteFile("sample.go", source, 0o600)) require.NoError(t, os.WriteFile("sample_test.go", []byte("package sample\n"), 0o600)) diff --git a/internal/mutate/tree_internal_test.go b/internal/mutate/tree_internal_test.go index 74fb2a4..0b947bc 100644 --- a/internal/mutate/tree_internal_test.go +++ b/internal/mutate/tree_internal_test.go @@ -4,10 +4,11 @@ import ( "io/fs" "net" "os" - "os/exec" "path/filepath" "slices" "testing" + + "github.com/spin-stack/go-tools/internal/gittest" ) // A tree copies the module as git sees it - what it tracks and what it does not ignore - links a @@ -202,11 +203,7 @@ func gitModule(t *testing.T) string { t.Fatal(err) } for _, args := range [][]string{{"init", "-q"}, {"add", "go.mod", ".gitignore", "doc.txt", "p/tracked.go", "p/link", "p/testdata"}} { - git := exec.Command("git", args...) - git.Dir = root - if out, err := git.CombinedOutput(); err != nil { - t.Fatalf("git %v: %v: %s", args, err, out) - } + gittest.Run(t, root, args...) } return root } diff --git a/internal/proven/fence_test.go b/internal/proven/fence_test.go new file mode 100644 index 0000000..3f202f1 --- /dev/null +++ b/internal/proven/fence_test.go @@ -0,0 +1,15 @@ +package proven_test + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/proven/github_test.go b/internal/proven/github_test.go index cbcec24..b96006e 100644 --- a/internal/proven/github_test.go +++ b/internal/proven/github_test.go @@ -6,13 +6,13 @@ import ( "net/http/httptest" "net/url" "os" - "os/exec" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/proven" ) @@ -114,9 +114,7 @@ func clone(t *testing.T) (root string, main []string, pr string) { root = t.TempDir() git := func(args ...string) string { t.Helper() - out, err := exec.Command("git", append([]string{"-C", root, "-c", "user.name=t", "-c", "user.email=t@t", "-c", "commit.gpgsign=false"}, args...)...).CombinedOutput() - require.NoError(t, err, "git %v: %s", args, out) - return string(out) + return gittest.Run(t, root, args...) } git("init", "-q", "-b", "main") for _, name := range []string{"one", "two", "three"} { diff --git a/internal/refs/fence_test.go b/internal/refs/fence_test.go new file mode 100644 index 0000000..9419922 --- /dev/null +++ b/internal/refs/fence_test.go @@ -0,0 +1,15 @@ +package refs_test + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/refs/refs_test.go b/internal/refs/refs_test.go index 601b7fa..2161ddc 100644 --- a/internal/refs/refs_test.go +++ b/internal/refs/refs_test.go @@ -9,6 +9,7 @@ import ( "testing" "github.com/spin-stack/go-tools/internal/allow" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/refs" ) @@ -234,11 +235,7 @@ func TestABuildOutputIsNotInTheTreeWhetherOrNotItWasBuilt(t *testing.T) { root := tree(t, "The build copies it into `cmd/cli/web`, which is also named `cmd/cli/main.go`.\n") git := func(args ...string) { t.Helper() - cmd := exec.Command("git", args...) - cmd.Dir = root - if out, err := cmd.CombinedOutput(); err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } + gittest.Run(t, root, args...) } git("init", "-q") if err := os.WriteFile(filepath.Join(root, ".gitignore"), []byte("cmd/cli/web/\n"), 0o600); err != nil { diff --git a/internal/release/cmd/releasename/fence_test.go b/internal/release/cmd/releasename/fence_test.go new file mode 100644 index 0000000..583998a --- /dev/null +++ b/internal/release/cmd/releasename/fence_test.go @@ -0,0 +1,15 @@ +package main + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The command runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/sfw/fence_test.go b/internal/sfw/fence_test.go new file mode 100644 index 0000000..47845bd --- /dev/null +++ b/internal/sfw/fence_test.go @@ -0,0 +1,15 @@ +package sfw_test + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/sfw/sfw_test.go b/internal/sfw/sfw_test.go index 57b78aa..a53d6ce 100644 --- a/internal/sfw/sfw_test.go +++ b/internal/sfw/sfw_test.go @@ -7,7 +7,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "sync/atomic" "testing" @@ -15,6 +14,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/sfw" "github.com/spin-stack/go-tools/versions" ) @@ -84,9 +84,7 @@ tasks: write("internal/x/testdata/package-lock.json", "{}") write(".gitignore", "ignored/\n") write("ignored/install.sh", "npm install\n") - git := exec.Command("git", "init", "-q", repo) - out, err := git.CombinedOutput() - require.NoError(t, err, "%s", out) + gittest.Run(t, repo, "init", "-q") findings, err := sfw.Check(repo) require.NoError(t, err) diff --git a/internal/taskgate/example_test.go b/internal/taskgate/example_test.go index 0125872..725df6e 100644 --- a/internal/taskgate/example_test.go +++ b/internal/taskgate/example_test.go @@ -1,19 +1,25 @@ package taskgate_test import ( + "os" "path/filepath" "testing" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/taskgate" ) // The site's CI page shows site/examples/github as a repository would have it, so it is held to // what taskgate holds such a repository to, at the limits the command's flags default to: an // example that a gate here would fail is one nobody can copy. +// It is copied into a repository of its own, as a repository would have it: git run in this +// checkout is fenced off it (internal/gittest). func TestTheSitesCIExamplePasses(t *testing.T) { - root := filepath.Join("..", "..", "site", "examples", "github") + root := t.TempDir() + require.NoError(t, os.CopyFS(root, os.DirFS(filepath.Join("..", "..", "site", "examples", "github")))) + gittest.Run(t, root, "init", "-q") known, err := taskgate.Tasks("task", root) require.NoError(t, err) require.Contains(t, known, "mutate", "the example's Taskfile was not the one listed") diff --git a/internal/taskgate/fence_test.go b/internal/taskgate/fence_test.go new file mode 100644 index 0000000..486be9f --- /dev/null +++ b/internal/taskgate/fence_test.go @@ -0,0 +1,15 @@ +package taskgate_test + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/internal/taskgate/taskfile_test.go b/internal/taskgate/taskfile_test.go index 358bcc7..ee0ebef 100644 --- a/internal/taskgate/taskfile_test.go +++ b/internal/taskgate/taskfile_test.go @@ -2,13 +2,13 @@ package taskgate_test import ( "os" - "os/exec" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/taskgate" ) @@ -20,8 +20,7 @@ func repo(t *testing.T, files map[string]string) string { require.NoError(t, os.MkdirAll(filepath.Dir(filepath.Join(root, rel)), 0o750)) require.NoError(t, os.WriteFile(filepath.Join(root, rel), []byte(body), 0o600)) } - out, err := exec.Command("git", "init", "-q", root).CombinedOutput() - require.NoError(t, err, "%s", out) + gittest.Run(t, root, "init", "-q") return root } @@ -187,8 +186,7 @@ func TestAScriptATaskRunsIsNotAProgram(t *testing.T) { "hack/jq.sh": "#!/bin/sh\ncurl x | jq .a\n", }) // Staged and then deleted: git lists it, and there is nothing to read. - out, err := exec.Command("git", "-C", root, "add", "hack/staged.sh").CombinedOutput() - require.NoError(t, err, "%s", out) + gittest.Run(t, root, "add", "hack/staged.sh") require.NoError(t, os.Remove(filepath.Join(root, "hack/staged.sh"))) findings, err := taskgate.Check(root, taskgate.Limits{Script: 2}, nil) require.NoError(t, err) @@ -240,9 +238,7 @@ func TestASubmoduleATaskNamesIsNoScript(t *testing.T) { "Taskfile.yml": "tasks:\n oracle:\n dir: reference\n cmds:\n - pnpm --dir reference install\n", }) require.NoError(t, os.MkdirAll(filepath.Join(root, "reference"), 0o750)) - out, err := exec.Command("git", "-C", root, "update-index", "--add", "--cacheinfo", - "160000,5dc7533314b5ba7ec3875c30143dfe1aec644870,reference").CombinedOutput() - require.NoError(t, err, "%s", out) + gittest.Run(t, root, "update-index", "--add", "--cacheinfo", "160000,5dc7533314b5ba7ec3875c30143dfe1aec644870,reference") findings, err := taskgate.Check(root, taskgate.Limits{Block: 12, Script: 40}, nil) require.NoError(t, err) assert.Empty(t, findings) diff --git a/internal/taskgate/taskgate_test.go b/internal/taskgate/taskgate_test.go index d0b7532..f521caf 100644 --- a/internal/taskgate/taskgate_test.go +++ b/internal/taskgate/taskgate_test.go @@ -2,13 +2,13 @@ package taskgate_test import ( "os" - "os/exec" "path/filepath" "testing" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/internal/taskgate" ) @@ -62,13 +62,10 @@ jobs: write(".github/workflows/README.md", "jobs:\n x:\n steps:\n - run: make\n") write(".github/workflows/old/ci.yml", "jobs:\n x:\n steps:\n - run: make\n") write(".github/ci.yml", "jobs:\n x:\n steps:\n - run: make\n") - git := exec.Command("git", "init", "-q", repo) - out, err := git.CombinedOutput() - require.NoError(t, err, "%s", out) + gittest.Run(t, repo, "init", "-q") // Staged and then deleted: git lists it, and there is nothing to read. write(".github/workflows/staged.yml", "jobs:\n x:\n steps:\n - run: make\n") - out, err = exec.Command("git", "-C", repo, "add", ".github/workflows/staged.yml").CombinedOutput() - require.NoError(t, err, "%s", out) + gittest.Run(t, repo, "add", ".github/workflows/staged.yml") require.NoError(t, os.Remove(filepath.Join(repo, ".github/workflows/staged.yml"))) findings, err := taskgate.Check(repo, taskgate.Limits{}, nil) diff --git a/internal/testidentity/run.go b/internal/testidentity/run.go new file mode 100644 index 0000000..ccbdd75 --- /dev/null +++ b/internal/testidentity/run.go @@ -0,0 +1,104 @@ +package testidentity + +import ( + "context" + "errors" + "fmt" + "io" + "os/exec" + "strings" + + "github.com/spin-stack/go-tools/internal/allow" +) + +// ErrFound is a run that found a test identity on a commit, or an allow line for nothing. +var ErrFound = errors.New("a test identity, or a stale exemption") + +// Run checks the commits of the repository at root since base - every one in base..HEAD - against +// the test identities of its tree, less what the allow file at allowPath exempts (none when it is +// ""), and writes what it found to out. It answers ErrFound when the gate fails, and another +// error when it could not ask. +func Run(ctx context.Context, root, base, allowPath string, out io.Writer) error { + allowed, err := allow.Read(allowPath) + if err != nil { + return err + } + if _, err := git(ctx, root, "rev-parse", "--verify", "--quiet", base+"^{commit}"); err != nil { + return fmt.Errorf("%s is not a commit here: the branch's commits are the ones after it, so fetch it "+ + "(git fetch origin, or fetch-depth: 0 in a workflow's checkout)", base) + } + tracked, err := git(ctx, root, "ls-files", "-z") + if err != nil { + return err + } + fixtures, err := Fixtures(root, strings.FieldsFunc(tracked, func(r rune) bool { return r == 0 })) + if err != nil { + return err + } + // %ae and %ce, not %aE: what the commit says, before a .mailmap maps it to somebody. + log, err := git(ctx, root, "log", "-z", "--format=%H%x1f%s%x1f%ae%x1f%ce", base+"..HEAD") + if err != nil { + return err + } + commits, err := parseLog(log) + if err != nil { + return err + } + + found := Check(commits, fixtures, allowed) + stale := Stale(allowed, fixtures) + for _, f := range found { + _, _ = fmt.Fprintln(out, f) //nolint:errcheck // the status is the gate's answer; out failing changes no status + } + for _, email := range stale { + _, _ = fmt.Fprintf(out, "%s: %s is not a test identity any more: take the line out\n", allowPath, email) //nolint:errcheck // as above + } + if len(found) > 0 { + _, _ = fmt.Fprintf(out, remedy, base) //nolint:errcheck // as above + } + if len(found) > 0 || len(stale) > 0 { + return ErrFound + } + _, err = fmt.Fprintf(out, "OK: %d commit(s) since %s, none by a test identity (%d addresses in the tree's tests and fixtures).\n", + len(commits), base, len(fixtures)) + return err +} + +// remedy is what to do about a finding, in the order that stops the next one first. +const remedy = ` +A test identity reached a commit: git took it from a configuration a test wrote, most likely +the checkout's own. Before anything else, ask git where its author comes from, in the checkout +or worktree the commit was made in: + + git config --show-origin --get-all user.email + git config --show-origin --get-all user.name + +and take out what a test put there (git config --local --unset user.email, and user.name), +so the next commit is not the same. Then give the branch's commits their real author again: + + git rebase --exec 'git commit --amend --no-edit --reset-author' %s + +and push with --force-with-lease. A commit already on the default branch stays; .mailmap maps +its address. +` + +// parseLog is the commits of git log's -z output in the format Run asks for. +func parseLog(log string) ([]Commit, error) { + var commits []Commit + for _, rec := range strings.FieldsFunc(log, func(r rune) bool { return r == 0 }) { + f := strings.Split(strings.TrimPrefix(rec, "\n"), "\x1f") + if len(f) != 4 { + return nil, fmt.Errorf("git log answered %q", rec) + } + commits = append(commits, Commit{SHA: f[0], Subject: f[1], AuthorEmail: f[2], CommitterEmail: f[3]}) + } + return commits, nil +} + +func git(ctx context.Context, root string, args ...string) (string, error) { + out, err := exec.CommandContext(ctx, "git", append([]string{"-C", root}, args...)...).Output() //nolint:gosec // git, with the base the caller names + if err != nil { + return "", fmt.Errorf("git %s: %w", strings.Join(args, " "), err) + } + return string(out), nil +} diff --git a/internal/testidentity/testidentity.go b/internal/testidentity/testidentity.go new file mode 100644 index 0000000..ec4a4a3 --- /dev/null +++ b/internal/testidentity/testidentity.go @@ -0,0 +1,184 @@ +// Package testidentity tells a commit's author from the identities a tree's tests use. +// +// Twice a test in spin set git's author on the checkout it ran in (`ana@corp.example`, 2026-10 +// and 2026-11-12), and every worktree of it - agents' too - committed as that author, one commit +// onto main. A fence around a test's git keeps it off the checkout; this is the other side, the +// same in every repository: a branch whose commits say a test identity is not pushed as if +// somebody wrote them. +// +// A test identity is not a list kept by hand. It is an address at a domain reserved for +// documentation and testing - RFC 2606 and 6761 (.test, .example, .invalid, .localhost and +// example.com/.net/.org), 6762's .local, 8375's home.arpa, ICANN's .internal - or one with no +// dot at all (`t@t`); or an address spelled in one of the tree's tests or fixtures, read from +// the tree every run, so a fixture added tomorrow is refused tomorrow. +// +// GitHub's own addresses - an App's `+[bot]@users.noreply.github.com` (Dependabot, a +// release App), a person's noreply, and the noreply@github.com that commits a merge made on the +// site - are real by rule, before the tests are asked: GitHub mints them and writes them on what +// its Apps commit, and a test may spell one without making the App's own commits a test's. +package testidentity + +import ( + "fmt" + "os" + "path" + "path/filepath" + "regexp" + "slices" + "strings" +) + +// Commit is one commit's addresses, as git wrote them, before any .mailmap. +type Commit struct { + SHA, Subject string + AuthorEmail, CommitterEmail string +} + +// Finding is one identity of one commit that is a test's. +type Finding struct { + Commit Commit + Role string // author or committer + Email string + Why string +} + +func (f Finding) String() string { + return fmt.Sprintf("%.12s %s: %s %s is a test identity (%s)", f.Commit.SHA, f.Commit.Subject, f.Role, f.Email, f.Why) +} + +// reservedTLDs are the top-level names no registry delegates: RFC 2606 and 6761, 6762's .local, +// and .internal (ICANN, 2024). +var reservedTLDs = []string{"test", "example", "invalid", "localhost", "local", "internal"} + +// reservedDomains are the names under a delegated top level that are reserved all the same: +// RFC 2606's three and RFC 8375's home.arpa. +var reservedDomains = []string{"example.com", "example.net", "example.org", "home.arpa"} + +// Reserved says why domain is one no person's mail is at, or "" when it may be. +func Reserved(domain string) string { + domain = strings.TrimSuffix(strings.ToLower(domain), ".") + if !strings.Contains(domain, ".") { + return "a domain with no dot" + } + if tld := domain[strings.LastIndex(domain, ".")+1:]; slices.Contains(reservedTLDs, tld) { + return "." + tld + " is reserved for testing (RFC 2606, 6761, 6762)" + } + for _, d := range reservedDomains { + if domain == d || strings.HasSuffix(domain, "."+d) { + return d + " is reserved (RFC 2606, 8375)" + } + } + return "" +} + +// github is whether email is an address GitHub mints: a user's or an App's noreply, or the +// committer of what is merged on the site. +func github(email string) bool { + return email == "noreply@github.com" || strings.HasSuffix(email, "@users.noreply.github.com") +} + +// Classify says why email is a test identity, or "" when it is not. fixtures is every address +// the tree's tests and fixtures spell, and the file that spells it. +func Classify(email string, fixtures map[string]string) string { + email = strings.ToLower(strings.TrimSpace(email)) + if github(email) { + return "" + } + if file, ok := fixtures[email]; ok { + return "spelled in " + file + } + _, domain, ok := strings.Cut(email, "@") + if !ok { + return "not an address" + } + return Reserved(domain) +} + +// Check is every author and committer of commits that is a test identity, less what allow +// exempts (an address, any case). +func Check(commits []Commit, fixtures map[string]string, allow map[string]bool) []Finding { + allowed := lower(allow) + var found []Finding + for _, c := range commits { + for _, id := range []struct{ role, email string }{{"author", c.AuthorEmail}, {"committer", c.CommitterEmail}} { + if allowed[strings.ToLower(id.email)] { + continue + } + if why := Classify(id.email, fixtures); why != "" { + found = append(found, Finding{Commit: c, Role: id.role, Email: id.email, Why: why}) + } + } + } + return found +} + +// Stale is every allowed address that is not a test identity any more: an exemption for nothing, +// which a ratchet does not keep. Whether a commit of the range uses it does not matter - the +// range is a branch's, and the address is somebody's on every branch. +func Stale(allow map[string]bool, fixtures map[string]string) []string { + var stale []string + for email := range allow { + if Classify(email, fixtures) == "" { + stale = append(stale, email) + } + } + slices.Sort(stale) + return stale +} + +func lower(allow map[string]bool) map[string]bool { + out := make(map[string]bool, len(allow)) + for k := range allow { + out[strings.ToLower(k)] = true + } + return out +} + +// address is an e-mail address as a test or fixture spells one. +var address = regexp.MustCompile(`[A-Za-z0-9._%+\[\]-]+@[A-Za-z0-9-]+(\.[A-Za-z0-9-]+)*`) + +// fixtureDirs are the directories whose every file is a test's or a fixture: Go's testdata, and +// what the repositories here name theirs (test/unit, test/e2e, modules/x/tests, test/mocks). +var fixtureDirs = []string{"testdata", "fixtures", "testutil", "e2e", "test", "tests", "__tests__", "mocks"} + +// Fixture is whether p, slash-separated and relative to the tree, is a test or a fixture: a Go +// test, a JavaScript or TypeScript one (x.test.ts, x.spec.mjs, x.e2e.test.ts), a Terraform test, +// an e2e configuration, or anything under a directory fixtureDirs names. +func Fixture(p string) bool { + base := path.Base(p) + if strings.HasSuffix(base, "_test.go") || strings.HasSuffix(base, ".tftest.hcl") || strings.HasSuffix(base, ".e2e.yaml") { + return true + } + if strings.Contains(base, ".test.") || strings.Contains(base, ".spec.") { + return true + } + return slices.ContainsFunc(strings.Split(path.Dir(p), "/"), func(dir string) bool { + return slices.Contains(fixtureDirs, dir) + }) +} + +// Fixtures is every address spelled in files under root that Fixture names, lower-case, with +// the first file that spells it. A path listed that is no file on disk - deleted and not yet +// staged, or a submodule's directory - is nothing to read. +func Fixtures(root string, files []string) (map[string]string, error) { + found := map[string]string{} + for _, f := range files { + if !Fixture(f) { + continue + } + p := filepath.Join(root, filepath.FromSlash(f)) + if info, err := os.Stat(p); err != nil || !info.Mode().IsRegular() { + continue + } + b, err := os.ReadFile(p) //nolint:gosec // a tracked file of the tree + if err != nil { + return nil, err + } + for _, m := range address.FindAll(b, -1) { // mutate-exempt: any negative count is every match + if a := strings.ToLower(string(m)); found[a] == "" { + found[a] = f + } + } + } + return found, nil +} diff --git a/internal/testidentity/testidentity_test.go b/internal/testidentity/testidentity_test.go new file mode 100644 index 0000000..24d7e5d --- /dev/null +++ b/internal/testidentity/testidentity_test.go @@ -0,0 +1,282 @@ +package testidentity_test + +import ( + "bytes" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/spin-stack/go-tools/internal/gittest" + "github.com/spin-stack/go-tools/internal/testidentity" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} + +// Which addresses are a test's, and the reason said for each: the reserved domains, an address +// the tree's tests spell, and nothing else - and never GitHub's own, even spelled in a test. The +// person here is made up: an author's real address spelled in this file would be a test's from +// then on. +func TestATestIdentityIsAReservedDomainOrAnAddressATestSpells(t *testing.T) { + fixtures := map[string]string{ + "agent@spinbox.dev": "cmd/supervisor/sign_test.go", + "335013441+spin-stack-release[bot]@users.noreply.github.com": "hack/machine/main_test.go", + "noreply@github.com": "hack/machine/main_test.go", + } + for _, tc := range []struct { + email string + why string // "" is not a test's + }{ + {"ana@corp.example", ".example is reserved for testing"}, + {"Ana@CORP.EXAMPLE", ".example is reserved for testing"}, + {" ana@corp.example ", ".example is reserved for testing"}, + {"t@t", "a domain with no dot"}, + {"dev@localhost", "a domain with no dot"}, + {"no-address", "not an address"}, + {"t@example.com", "example.com is reserved"}, + {"t@example.com.", "example.com is reserved"}, + {"x@app.example.com", "example.com is reserved"}, + {"x@example.org", "example.org is reserved"}, + {"x@example.net", "example.net is reserved"}, + {"x@spin.test", ".test is reserved for testing"}, + {"gittest@go-tools.invalid", ".invalid is reserved for testing"}, + {"x@host.localhost", ".localhost is reserved for testing"}, + {"x@printer.local", ".local is reserved for testing"}, + {"x@router.home.arpa", "home.arpa is reserved"}, + {"x@home.arpa", "home.arpa is reserved"}, + {"x@host.internal", ".internal is reserved for testing"}, + {"agent@spinbox.dev", "spelled in cmd/supervisor/sign_test.go"}, + {"Agent@Spinbox.dev", "spelled in cmd/supervisor/sign_test.go"}, + {"person@gmail.com", ""}, + {"someone@spinbox.dev", ""}, + {"x@notexample.com", ""}, + {"x@example.community", ""}, + {"x@myhome.arpa", ""}, + {"x@testing.dev", ""}, + {"49699333+dependabot[bot]@users.noreply.github.com", ""}, + {"335013441+spin-stack-release[bot]@users.noreply.github.com", ""}, + {"noreply@github.com", ""}, + } { + t.Run(tc.email, func(t *testing.T) { + why := testidentity.Classify(tc.email, fixtures) + if tc.why == "" { + assert.Empty(t, why) + return + } + assert.Contains(t, why, tc.why) + }) + } +} + +// A commit is held by its author and by its committer, and an allow line takes one address out, +// in any case; a line whose address is not a test's is itself reported. +func TestACommitByATestIdentityIsFoundUnlessAllowed(t *testing.T) { + commits := []testidentity.Commit{ + {SHA: "746cd0df4", Subject: "the leak", AuthorEmail: "ana@corp.example", CommitterEmail: "person@gmail.com"}, + {SHA: "aaaa", Subject: "rebased by a test", AuthorEmail: "person@gmail.com", CommitterEmail: "t@t"}, + {SHA: "bbbb", Subject: "dependabot", AuthorEmail: "49699333+dependabot[bot]@users.noreply.github.com", CommitterEmail: "noreply@github.com"}, + {SHA: "cccc", Subject: "allowed", AuthorEmail: "bot@ci.example", CommitterEmail: "person@gmail.com"}, + } + allow := map[string]bool{"Bot@CI.example": true, "person@gmail.com": true} + var got []string + for _, f := range testidentity.Check(commits, nil, allow) { + got = append(got, f.Commit.SHA+" "+f.Role+" "+f.Email) + } + assert.Equal(t, []string{"746cd0df4 author ana@corp.example", "aaaa committer t@t"}, got) + assert.Equal(t, []string{"person@gmail.com"}, testidentity.Stale(allow, nil)) + + f := testidentity.Finding{Commit: commits[0], Role: "author", Email: "ana@corp.example", Why: "why"} + assert.Equal(t, "746cd0df4 the leak: author ana@corp.example is a test identity (why)", f.String()) +} + +// The fixtures are read from the tree's tests and fixtures only - Go's, JavaScript's, +// Terraform's, and what a tests directory holds: an address in a document or the code is +// somebody's, not a test's. +func TestTheFixturesAreTheAddressesTheTestsSpell(t *testing.T) { + root := t.TempDir() + files := map[string]string{ + "cmd/supervisor/sign_test.go": `"GIT_AUTHOR_EMAIL=agent@spinbox.dev"`, + "configs/fixtures/e2e.yaml": "email: user@e2e.test\n", + "dashboard/e2e/admin.spec.ts": "const a = 'Admin@Spin.Test'\n", + "internal/x/testdata/fixture.txt": "who: fixture@spinbox.dev\n", + "internal/e2e/stack.go": "const owner = \"owner@e2e.dev\"\n", + "src/auth.test.ts": "const u = 'unit@vscode.dev'\n", + "test/e2e/cli.e2e.test.ts": "const u = 'e2e@vscode.dev'\n", + "test/mocks/vscode.ts": "const u = 'mock@vscode.dev'\n", + "modules/cp/tests/posture.tftest.hcl": "owner = \"tf@spinbox.dev\"\n", + "lanes/stack.e2e.yaml": "owner: lane@spinbox.dev\n", + "x/testutil/who.go": "// util@spinbox.dev\n", + "web/__tests__/who.js": "// jest@spinbox.dev\n", + "internal/x/testdata/again.txt": "who: agent@spinbox.dev\n", + "README.md": "write to person@gmail.com\n", + "internal/x/x.go": "// owner@spinbox.dev\n", + "src/testing.ts": "// code@spinbox.dev\n", + "internal/attestation/attestation.go": "// attest@spinbox.dev\n", + "modules/cp/tests.tf": "# terraform@spinbox.dev\n", + "internal/x/testdata/deleted-not-staged": "", + } + var names []string + for name, body := range files { + require.NoError(t, os.MkdirAll(filepath.Join(root, filepath.Dir(name)), 0o700)) + require.NoError(t, os.WriteFile(filepath.Join(root, name), []byte(body), 0o600)) + names = append(names, name) + } + require.NoError(t, os.Remove(filepath.Join(root, "internal/x/testdata/deleted-not-staged"))) + require.NoError(t, os.MkdirAll(filepath.Join(root, "test", "submodule"), 0o700)) + // Sorted, so which file is first to spell an address is the same on every run. + names = append(names, "test/submodule") + slices.Sort(names) + + fixtures, err := testidentity.Fixtures(root, names) + require.NoError(t, err) + assert.Equal(t, map[string]string{ + "agent@spinbox.dev": "cmd/supervisor/sign_test.go", + "user@e2e.test": "configs/fixtures/e2e.yaml", + "admin@spin.test": "dashboard/e2e/admin.spec.ts", + "fixture@spinbox.dev": "internal/x/testdata/fixture.txt", + "owner@e2e.dev": "internal/e2e/stack.go", + "unit@vscode.dev": "src/auth.test.ts", + "e2e@vscode.dev": "test/e2e/cli.e2e.test.ts", + "mock@vscode.dev": "test/mocks/vscode.ts", + "tf@spinbox.dev": "modules/cp/tests/posture.tftest.hcl", + "lane@spinbox.dev": "lanes/stack.e2e.yaml", + "util@spinbox.dev": "x/testutil/who.go", + "jest@spinbox.dev": "web/__tests__/who.js", + }, fixtures) +} + +// The run, over a repository: its branch's commits since the base, the addresses its own tests +// spell, and an allow file; what it says for each answer, and its error for each. +func TestTheBranchsCommitsAreCheckedSinceTheBase(t *testing.T) { + // A commit by name and email, committed by the same unless committer is set. + type commit struct{ name, email, committer string } + for _, tc := range []struct { + name string + commits []commit + allow string + base string + want error + says []string + }{ + { + name: "a person's commit", + commits: []commit{{"Person", "person@gmail.com", ""}}, + says: []string{"OK: 1 commit(s) since main, none by a test identity (1 addresses"}, + }, + { + name: "a commit by the checkout's test author", + commits: []commit{{"Person", "person@gmail.com", ""}, {"Ana Lima", "ana@corp.example", ""}}, + want: testidentity.ErrFound, + says: []string{ + "the leak: author ana@corp.example is a test identity (.example is reserved", + "the leak: committer ana@corp.example", + "git config --show-origin --get-all user.email", + "git rebase --exec 'git commit --amend --no-edit --reset-author' main", + }, + }, + { + name: "a commit authored by an address the tree's test spells", + commits: []commit{{"Fixture", "fixture@spinbox.dev", "person@gmail.com"}}, + want: testidentity.ErrFound, + says: []string{ + "the leak: author fixture@spinbox.dev is a test identity (spelled in x_test.go)", + "git rebase --exec", + }, + }, + { + name: "a commit committed by a test", + commits: []commit{{"Person", "person@gmail.com", "t@t"}}, + want: testidentity.ErrFound, + says: []string{"the leak: committer t@t is a test identity (a domain with no dot)"}, + }, + { + name: "an allowed address", + commits: []commit{{"Bot", "bot@ci.example", ""}}, + allow: "bot@ci.example # the CI's own bot, which is real\n", + says: []string{"OK: 1 commit(s)"}, + }, + { + name: "an allow line for a real address", + commits: []commit{{"Person", "person@gmail.com", ""}}, + allow: "person@gmail.com # not a test's\n", + want: testidentity.ErrFound, + says: []string{"person@gmail.com is not a test identity any more: take the line out"}, + }, + { + name: "an allow line with no reason", + allow: "bot@ci.example\n", + want: errAny, + }, + { + name: "a base that is not here", + base: "origin/main", + want: errAny, + says: nil, + }, + } { + t.Run(tc.name, func(t *testing.T) { + repo := t.TempDir() + gittest.Run(t, repo, "init", "-q", "-b", "main") + require.NoError(t, os.WriteFile(filepath.Join(repo, "x_test.go"), []byte("// fixture@spinbox.dev\n"), 0o600)) + gittest.Run(t, repo, "add", ".") + gittest.Run(t, repo, "commit", "-q", "-m", "base") + gittest.Run(t, repo, "checkout", "-q", "-b", "branch") + for i, c := range tc.commits { + subject := "one" + if i == len(tc.commits)-1 { + subject = "the leak" + } + committer := c.email + if c.committer != "" { + committer = c.committer + } + cmd := gittest.Command(t, repo, "commit", "-q", "--allow-empty", "-m", subject) + cmd.Env = append(cmd.Env, "GIT_AUTHOR_NAME="+c.name, "GIT_AUTHOR_EMAIL="+c.email, + "GIT_COMMITTER_NAME="+c.name, "GIT_COMMITTER_EMAIL="+committer) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "%s", out) + } + // A finding names its commit, by the first twelve of its name. + head := gittest.Run(t, repo, "rev-parse", "HEAD") + allowPath := "" + if tc.allow != "" { + allowPath = filepath.Join(t.TempDir(), "allow.txt") + require.NoError(t, os.WriteFile(allowPath, []byte(tc.allow), 0o600)) + } + base := "main" + if tc.base != "" { + base = tc.base + } + + var out bytes.Buffer + err := testidentity.Run(t.Context(), repo, base, allowPath, &out) + switch tc.want { + case nil: + require.NoError(t, err, "%s", out.String()) + case errAny: + require.Error(t, err) + require.NotErrorIs(t, err, testidentity.ErrFound) + default: + require.ErrorIs(t, err, tc.want) + } + for _, s := range tc.says { + if strings.HasPrefix(s, "the leak:") { + s = head[:12] + " " + s + } + assert.Contains(t, out.String(), s) + } + }) + } +} + +// errAny is a run that could not ask: anything but ErrFound. +var errAny = os.ErrInvalid diff --git a/site/src/content/docs/index.mdx b/site/src/content/docs/index.mdx index 4eeae1e..dd27e19 100644 --- a/site/src/content/docs/index.mdx +++ b/site/src/content/docs/index.mdx @@ -52,6 +52,11 @@ if it were wrong. href="taskgate/" description="Fails on a workflow step that is not one task. What CI does is written once, in the Taskfile, and a developer runs exactly the same thing." /> + ## What a build takes in, and what it ships diff --git a/site/src/content/docs/testidentity.md b/site/src/content/docs/testidentity.md new file mode 100644 index 0000000..ee1d448 --- /dev/null +++ b/site/src/content/docs/testidentity.md @@ -0,0 +1,76 @@ +--- +title: testidentity +description: Fails on a commit of the branch whose author or committer is a test's - an address at a domain reserved for testing, or one the tree's tests spell - which is what a test that wrote git's author into the checkout leaves on every commit made from it. +--- + +A test that runs git looks for its repository upward from where it runs, and a test runs inside +the checkout. One that misses the repository it made - a mutant that lost its `-C`, a `TMPDIR` +inside the checkout, a hook's `GIT_DIR` - runs `git config user.email` on the checkout itself. +From then on every commit made there, from every worktree of it, is that test's: in spin it put +`Ana Lima ` on commits pushed to GitHub, twice, one of them onto main. + +`testidentity` reads who each commit of the branch says wrote it and fails on one that is a +test's, before it is pushed and merged: + +```sh +go tool testidentity [-base origin/main] [-allow file] [dir] +``` + +It checks every commit in `BASE..HEAD`, its author and its committer, as git wrote them - before a +`.mailmap` maps them to somebody. + +## What a test identity is + +Not a list kept by hand. An address is a test's when it is + +- at a domain no registry delegates: `.test`, `.example`, `.invalid`, `.localhost` (RFC 2606 and + 6761), `.local` (RFC 6762), `.internal`, and `example.com`, `example.net`, `example.org`, + `home.arpa` and their subdomains; +- at a domain with no dot at all: `t@t`, `dev@localhost`; +- spelled in one of the tree's tests or fixtures: a Go test (`_test.go`), a JavaScript or + TypeScript one (`x.test.ts`, `x.spec.mjs`), a Terraform test (`.tftest.hcl`), an `.e2e.yaml`, + or any file under a `testdata`, `fixtures`, `testutil`, `e2e`, `test`, `tests`, `__tests__` or + `mocks` directory. The tree is read on every run, from `git ls-files`, so a fixture added + tomorrow is refused tomorrow. + +GitHub's own addresses are real by rule, before the tests are asked: an App's +`+[bot]@users.noreply.github.com` (Dependabot, a release App), a person's noreply, and +the `noreply@github.com` that commits what is merged on the site. + +## What it says + +Each finding is a line: the commit, its subject, which of author or committer, the address, and +why the address is a test's. After them, what to do, in the order that stops the next one first: +ask git where the author comes from (`git config --show-origin --get-all user.email`) in the +checkout the commit was made in and take out what the test wrote there; then give the branch's +commits their real author again, `git rebase --exec 'git commit --amend --no-edit +--reset-author' `, and push with `--force-with-lease`. A commit already on the default +branch stays, and `.mailmap` maps its address. + +The exit status is 1 on a finding, 2 when it could not ask - the base is not in the clone, which a +workflow's checkout needs `fetch-depth: 0` for. + +## In a Taskfile + +```yaml title="Taskfile.yml" +lint: + cmds: + - testidentity -base origin/main +``` + +Under `go tool task` the command is on Task's `PATH`; a repository with no `go.mod` runs +`go run github.com/spin-stack/go-tools/cmd/testidentity@`. On main the range is empty and +it passes. + +## Exemptions + +`-allow` names an address that reads as a test's and is somebody's real author, one a line, +each with its reason: + +```text title="hack/testidentity-allow.txt" +ci@build.internal # the build host's own commits of the generated tree, which are real +``` + +A line whose address is not a test identity any more fails too, so the list only shrinks. An +address a test spells is a test's from then on: a test that needs a person's address spells a +made-up one, never an author's. diff --git a/site/src/lib/nav.mjs b/site/src/lib/nav.mjs index 03bda9f..02919a9 100644 --- a/site/src/lib/nav.mjs +++ b/site/src/lib/nav.mjs @@ -16,6 +16,7 @@ export const sidebar = [ { label: 'refs', slug: 'refs' }, { label: 'vulngate', slug: 'vulngate' }, { label: 'taskgate', slug: 'taskgate' }, + { label: 'testidentity', slug: 'testidentity' }, { label: 'deadgate', slug: 'deadgate' }, { label: 'linkweight', slug: 'linkweight' }, { label: 'proven', slug: 'proven' }, diff --git a/versions/gate/fence_test.go b/versions/gate/fence_test.go new file mode 100644 index 0000000..47b3b61 --- /dev/null +++ b/versions/gate/fence_test.go @@ -0,0 +1,15 @@ +package gate + +import ( + "os" + "testing" + + "github.com/spin-stack/go-tools/internal/gittest" +) + +// The package runs git, and a git of it that misses its repository - a mutant - finds the +// checkout's: fenced first, it finds none (internal/gittest). +func TestMain(m *testing.M) { + gittest.Fence() + os.Exit(m.Run()) +} diff --git a/versions/gate/gate_test.go b/versions/gate/gate_test.go index 83cbf92..3cb144f 100644 --- a/versions/gate/gate_test.go +++ b/versions/gate/gate_test.go @@ -2,12 +2,12 @@ package gate import ( "os" - "os/exec" "path/filepath" "regexp" "strings" "testing" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/versions" ) @@ -25,9 +25,7 @@ func repo(t *testing.T, files map[string]string) string { } } 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\n%s", args, err, out) - } + gittest.Run(t, root, args...) } return root } diff --git a/versions/upstream/check_test.go b/versions/upstream/check_test.go index 9eef18d..89431d4 100644 --- a/versions/upstream/check_test.go +++ b/versions/upstream/check_test.go @@ -7,7 +7,6 @@ import ( "log" "net/http" "net/http/httptest" - "os/exec" "path/filepath" "strings" "testing" @@ -17,6 +16,7 @@ import ( "github.com/google/go-containerregistry/pkg/v1/random" "github.com/google/go-containerregistry/pkg/v1/remote" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/versions" ) @@ -31,12 +31,7 @@ func newGitRepo(t *testing.T) gitRepo { r := gitRepo{dir: t.TempDir()} git := func(args ...string) string { t.Helper() - cmd := exec.Command("git", append([]string{"-C", r.dir, "-c", "user.name=t", "-c", "user.email=t@t"}, args...)...) - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } - return strings.TrimSpace(string(out)) + return gittest.Run(t, r.dir, args...) } git("init", "-q", "-b", "master") git("commit", "-q", "--allow-empty", "-m", "one") diff --git a/versions/upstream/upstream_test.go b/versions/upstream/upstream_test.go index fdb2115..a9ca4c7 100644 --- a/versions/upstream/upstream_test.go +++ b/versions/upstream/upstream_test.go @@ -9,7 +9,6 @@ import ( "net/http" "net/http/httptest" "os" - "os/exec" "path/filepath" "runtime" "slices" @@ -21,6 +20,7 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "github.com/spin-stack/go-tools/internal/gittest" "github.com/spin-stack/go-tools/versions" ) @@ -133,12 +133,7 @@ func TestACommitTrackedFileIsBehindOnlyWhenItChanged(t *testing.T) { repo := t.TempDir() git := func(args ...string) string { t.Helper() - cmd := exec.Command("git", append([]string{"-C", repo, "-c", "user.name=t", "-c", "user.email=t@t"}, args...)...) - out, err := cmd.CombinedOutput() - if err != nil { - t.Fatalf("git %v: %v\n%s", args, err, out) - } - return strings.TrimSpace(string(out)) + return gittest.Run(t, repo, args...) } git("init", "-q", "-b", "master") git("commit", "-q", "--allow-empty", "-m", "one")