Merge pull request 'A check never sees the forge credential, and what it publishes is redacted (issue 462)' (#226) from fix/462-a-check-never-sees-the-forge-credential into main

internal/builder/builder.go

@@ -7,8 +7,10 @@ import (
 	"crypto/sha256"
 	"encoding/hex"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+	"io/fs"
 	"os"
 	"os/exec"
 	"path"
@@ -159,17 +161,18 @@ func build(ctx context.Context, run Runner, publish Publisher,
 	}
 	// The credential is a file git reads, never an argument: a URL carrying a password in argv
 	// would be readable by anything that can list processes for as long as a clone runs.
-	credentials := ""
+	credentials, forget, err := storeCredential(workspace, forge)
-	if forge.URL != "" {
+	if err != nil {
-		credentials = filepath.Join(workspace, "git-credentials")
+		return Result{}, err
-		if err := os.WriteFile(credentials, []byte(forge.URL+"\n"), 0o600); err != nil {
-			return Result{}, err
-		}
 	}
+	defer forget()
 	tree := filepath.Join(workspace, "source")
 	if err := os.RemoveAll(tree); err != nil {
 		return Result{}, err
 	}
+	// Removed when the build ends, whatever happens: the tree holds the .npmrc a build may be handed, in the
+	// workspace a later check's container mounts as its HOME (novox/hq issue 462).
+	defer os.RemoveAll(tree)
 	// A fresh clone every time rather than a fetch into a tree that is already there. A build
 	// that reuses a working tree can succeed because of something a previous build left behind,
 	// and that is a build nobody can reproduce.
@@ -177,6 +180,9 @@ func build(ctx context.Context, run Runner, publish Publisher,
 		say("clone", "FAILED: %v", err)
 		return Result{}, fmt.Errorf("cannot clone %s: %w", repository, err)
 	}
+	if err := recordedWithoutUserinfo(ctx, run, tree, repository); err != nil {
+		return Result{}, err
+	}
 	say("clone", "done")
 	if ref != "" {
 		if _, err := run(ctx, tree, "git", "checkout", "--quiet", ref); err != nil {
@@ -248,6 +254,8 @@ func build(ctx context.Context, run Runner, publish Publisher,
 		if err := os.WriteFile(npmrcPath, []byte(content), 0o600); err != nil {
 			return Result{}, fmt.Errorf("cannot write the package-registry credential for the build: %w", err)
 		}
+		// Only for as long as the build: a later check's container mounts this workspace (novox/hq issue 462).
+		defer os.Remove(npmrcPath)
 		say("packages", "resolving %s from the mesh's package registry", npmrc.Scope)
 		src.notPinned("it resolves packages from the mesh's registry at build time")
 	}
@@ -441,6 +449,9 @@ func contextFrom(ctx context.Context, run Runner, workspace, artifact, credentia
 	if _, err := run(ctx, workspace, "git", cloneWith(credentials, "clone", "--quiet", url, dir)...); err != nil {
 		return "", fmt.Errorf("cannot clone %s: %w", url, err)
 	}
+	if err := recordedWithoutUserinfo(ctx, run, dir, url); err != nil {
+		return "", err
+	}
 	if from.Ref != "" {
 		if _, err := run(ctx, dir, "git", "checkout", "--quiet", from.Ref); err != nil {
 			return "", fmt.Errorf("%s has no %s: %w", from.Repository, from.Ref, err)
@@ -467,6 +478,96 @@ func contextURL(from catalogue.ArtifactContext, seats map[string]string) (string
 	return strings.TrimRight(base, "/") + "/" + strings.TrimSuffix(strings.Trim(from.Repository, "/"), ".git") + ".git", nil
 }
 
+// storeCredential writes the forge credential where git's credential store reads it, and the function that
+// removes it again; "" and nothing to remove when the builder holds none.
+//
+// **Never inside the workspace** (novox/hq issue 462): a merge check's toolchain container mounts the
+// workspace as its HOME, so a credential kept there — even one a build left behind — is readable by any pull
+// request's merge-check.sh, and what it prints is kept on the bus. So it lives in a directory of its own
+// outside the workspace, made private, and a credential an older builder left in the workspace is removed.
+func storeCredential(workspace string, forge GitCredential) (string, func(), error) {
+	if err := os.Remove(filepath.Join(workspace, "git-credentials")); err != nil && !errors.Is(err, os.ErrNotExist) {
+		return "", nil, fmt.Errorf("a credential left in the workspace cannot be removed: %w", err)
+	}
+	if forge.URL == "" {
+		return "", func() {}, nil
+	}
+	dir, err := os.MkdirTemp("", "mesh-forge-credential-")
+	if err != nil {
+		return "", nil, err
+	}
+	forget := func() { os.RemoveAll(dir) }
+	if withinDir(workspace, dir) {
+		forget()
+		return "", nil, fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's "+
+			"container mounts: the forge credential is not written where it could read it", dir, workspace)
+	}
+	path := filepath.Join(dir, "git-credentials")
+	if err := os.WriteFile(path, []byte(forge.URL+"\n"), 0o600); err != nil {
+		forget()
+		return "", nil, err
+	}
+	return path, forget, nil
+}
+
+// recordedWithoutUserinfo makes a clone record the URL it came from without userinfo: git keeps it as given in
+// the clone's .git/config, inside the workspace a check's container mounts (novox/hq issue 462).
+func recordedWithoutUserinfo(ctx context.Context, run Runner, clone, repository string) error {
+	bare, carried := withoutUserinfo(repository)
+	if !carried {
+		return nil
+	}
+	if _, err := run(ctx, clone, "git", "remote", "set-url", "origin", bare); err != nil {
+		return fmt.Errorf("the clone of %s keeps its credential: %w", bare, err)
+	}
+	return nil
+}
+
+// forgetLeftCredentials removes what an earlier build or an older builder left in the workspace that holds
+// a credential: the forge's git-credentials, and every .npmrc a build wrote into a tree it cloned. Run
+// before a check, whose container mounts the workspace as its HOME (novox/hq issue 462). The Go caches are
+// not walked: no build writes a credential there, and they hold more files than everything else.
+func forgetLeftCredentials(workspace string) error {
+	if err := os.Remove(filepath.Join(workspace, "git-credentials")); err != nil && !errors.Is(err, os.ErrNotExist) {
+		return err
+	}
+	return filepath.WalkDir(workspace, func(p string, d fs.DirEntry, err error) error {
+		if err != nil {
+			if errors.Is(err, os.ErrNotExist) {
+				return nil
+			}
+			return err
+		}
+		if d.IsDir() && p != workspace && (d.Name() == "go-cache" || d.Name() == "go-modules") &&
+			filepath.Dir(p) == workspace {
+			return filepath.SkipDir
+		}
+		if !d.IsDir() && d.Name() == ".npmrc" {
+			if err := os.Remove(p); err != nil && !errors.Is(err, os.ErrNotExist) {
+				return fmt.Errorf("a package-registry credential left at %s cannot be removed: %w", p, err)
+			}
+		}
+		return nil
+	})
+}
+
+// withinDir is whether path is dir or under it, both made absolute and resolved.
+func withinDir(dir, path string) bool {
+	d, err1 := filepath.Abs(dir)
+	p, err2 := filepath.Abs(path)
+	if err1 != nil || err2 != nil {
+		return true
+	}
+	if r, err := filepath.EvalSymlinks(d); err == nil {
+		d = r
+	}
+	if r, err := filepath.EvalSymlinks(p); err == nil {
+		p = r
+	}
+	rel, err := filepath.Rel(d, p)
+	return err != nil || rel == "." || filepath.IsLocal(rel)
+}
+
 // cloneWith is a git invocation that may offer a stored credential.
 //
 // The first `-c credential.helper=` clears every helper the environment might carry, so exactly

internal/builder/builder_test.go

@@ -438,30 +438,34 @@ func TestABuildOffersTheForgesCredentialThroughGitsOwnStore(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	stored := filepath.Join(workspace, "git-credentials")
 	clone := r.ran[0]
-	if !strings.Contains(clone, "credential.helper=store --file="+stored) {
+	stored := storeNamedIn(t, clone)
-		t.Fatalf("the clone does not name the credential store: %s", clone)
-	}
 	for _, line := range r.ran {
 		if strings.Contains(line, "sw0rdfi5h") {
 			t.Fatalf("the secret is in a command line, readable by anything that can list processes: %s", line)
 		}
 	}
-	raw, err := os.ReadFile(stored)
+	// Outside the workspace a check's container mounts, and gone once the build is (novox/hq issue 462); what
-	if err != nil {
+	// it held while git read it is checked with the check's clones (TestACheckContainerSeesNoForgeCredential).
-		t.Fatal(err)
+	if withinDir(workspace, stored) {
+		t.Fatalf("the credential store %s is inside the workspace %s", stored, workspace)
 	}
-	if strings.TrimSpace(string(raw)) != "http://mesh_novox_builder:sw0rdfi5h@forge.invalid:20000" {
+	if _, err := os.Stat(stored); !os.IsNotExist(err) {
-		t.Fatalf("the store does not hold the credential as given: %q", raw)
+		t.Fatalf("the credential store outlives the build: %v", err)
 	}
-	info, err := os.Stat(stored)
+	if _, err := os.Stat(filepath.Join(workspace, "git-credentials")); !os.IsNotExist(err) {
-	if err != nil {
+		t.Fatal("a credential file is left in the workspace")
-		t.Fatal(err)
 	}
-	if info.Mode().Perm() != 0o600 {
+}
-		t.Fatalf("the credential file is readable beyond its owner: %v", info.Mode())
+
+// storeNamedIn is the credential store a git command line offers.
+func storeNamedIn(t *testing.T, line string) string {
+	t.Helper()
+	_, after, ok := strings.Cut(line, "credential.helper=store --file=")
+	if !ok {
+		t.Fatalf("the clone does not name the credential store: %s", line)
 	}
+	return strings.Fields(after)[0]
 }
 
 // Without a credential, a clone is exactly the invocation it always was, and no credential file
@@ -506,7 +510,7 @@ func TestAContextCloneCarriesTheSameCredentialStore(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
-	stored := filepath.Join(workspace, "git-credentials")
+	stored := storeNamedIn(t, r.ran[0])
 	var contextClone string
 	for _, line := range r.ran {
 		if strings.Contains(line, "clone") && strings.Contains(line, "source.git") {

internal/builder/check.go

@@ -229,7 +229,13 @@ func ScriptToolchain(script []byte) string {
 // Check runs one merge check. An error is that it could not run; the verdict is then "error".
 func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry string, forge GitCredential,
 	log Log) (CheckVerdict, error) {
-	say := logging(log)
+	// Everything a check says goes to the build's log, which the bus keeps: said redacted (novox/hq issue 462).
+	redact := redactorFor(forge)
+	say := logging(func(step, message string) {
+		if log != nil {
+			log(step, redact.redact(message))
+		}
+	})
 	began := time.Now()
 	ctx, stop := context.WithTimeout(ctx, CheckTimeout)
 	defer stop()
@@ -242,20 +248,26 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 		return CheckVerdict{}, err
 	}
 	defer os.RemoveAll(root)
-	credentials := ""
+	// The forge credential lives outside the workspace the check's containers mount, and only while the
-	if forge.URL != "" {
+	// check clones (novox/hq issue 462).
-		credentials = filepath.Join(workspace, "git-credentials")
+	credentials, forget, err := storeCredential(workspace, forge)
-		if err := os.WriteFile(credentials, []byte(forge.URL+"\n"), 0o600); err != nil {
+	if err != nil {
-			return CheckVerdict{}, err
+		return CheckVerdict{}, err
-		}
+	}
+	defer forget()
+	if err := forgetLeftCredentials(workspace); err != nil {
+		return CheckVerdict{}, err
 	}
 	clone := func(repository, ref, dir string) error {
 		if _, err := run(ctx, root, "git", cloneWith(credentials, "clone", "--quiet", repository, dir)...); err != nil {
-			return fmt.Errorf("cannot clone %s: %w", repository, err)
+			return fmt.Errorf("cannot clone %s: %s", redact.redact(repository), redact.redact(err.Error()))
+		}
+		if err := recordedWithoutUserinfo(ctx, run, filepath.Join(root, dir), repository); err != nil {
+			return err
 		}
 		if ref != "" {
 			if _, err := run(ctx, filepath.Join(root, dir), "git", "checkout", "--quiet", ref); err != nil {
-				return fmt.Errorf("%s has no %s: %w", repository, ref, err)
+				return fmt.Errorf("%s has no %s: %w", redact.redact(repository), ref, err)
 			}
 		}
 		return nil
@@ -323,6 +335,9 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 		}
 	}
 
+	// Every clone is made: the credential is gone before anything of the check runs (novox/hq issue 462).
+	forget()
+
 	// The facts, and the versions they say the mesh runs.
 	if registry == "" {
 		return CheckVerdict{}, errors.New("no artifact store to read the facts snapshot from")
@@ -479,8 +494,11 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
 		}
 	}
 
+	// What the check printed travels in the verdict to the forge and the controller: redacted as the log is.
+	v.Gate.Summary, v.Repo.Summary, v.Repo.Failed = redact.redact(v.Gate.Summary), redact.redact(v.Repo.Summary),
+		redact.redact(v.Repo.Failed)
 	v.Verdict, v.Summary = v.Gate.Verdict, v.Gate.Summary
-	v.Report, v.Took = withWhatFailed(out.String(), v.Repo), time.Since(began)
+	v.Report, v.Took = redact.redact(withWhatFailed(out.String(), v.Repo)), time.Since(began)
 	say("check", "gate %s — %s; repository %s — %s (%s)", strings.ToUpper(v.Gate.Verdict), v.Gate.Summary,
 		strings.ToUpper(v.Repo.Verdict), v.Repo.Summary, v.Took.Round(time.Second))
 	return v, nil
@@ -606,7 +624,9 @@ func (l *toTheLog) line(line string) {
 		line = line[:logLineBytes] + fmt.Sprintf(" … (%d bytes more)", len(line)-logLineBytes)
 	}
 	l.said++
-	l.say("output", "%s", line)
+	// Redacted by its shape before the bus keeps it (novox/hq issue 462); Check's own say adds the secrets
+	// the builder knows.
+	l.say("output", "%s", redactor{}.redact(line))
 }
 
 // close says the last line, and, when lines were left out, how many and what failed.
@@ -624,7 +644,7 @@ func (l *toTheLog) close(failed string) {
 	}
 	l.say("output", "--- what failed, picked from the whole of its output")
 	for _, line := range strings.Split(failed, "\n") {
-		l.say("output", "%s", line)
+		l.say("output", "%s", redactor{}.redact(line))
 	}
 }
 

internal/builder/check_credential_test.go

@@ -0,0 +1,251 @@
+package builder
+
+import (
+	"context"
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/json"
+	"errors"
+	"io/fs"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+	"time"
+
+	"github.com/novox/mesh-controller/internal/facts"
+)
+
+// **A check's container never sees the forge credential** (novox/hq issue 462): the toolchain container
+// mounts the workspace as HOME, so a credential kept there — or a clone's .git/config carrying one — is
+// readable by any pull request's merge-check.sh, and printed, kept on the bus for days.
+
+const (
+	forgeSecret  = "sw0rdfi5h-forge"
+	forgeURL     = "http://mesh_novox_builder:" + forgeSecret + "@forge.invalid:20000"
+	besideSecret = "b3side-t0ken"
+	npmSecret    = "npm-s3cret-t0ken"
+)
+
+// aFactsRegistry is an artifact store holding the facts snapshot, and nothing else.
+func aFactsRegistry(t *testing.T) string {
+	t.Helper()
+	body, err := json.Marshal(facts.Facts{Format: facts.Format, Taken: time.Now().UTC(),
+		Versions: facts.Versions{Bus: "2.11.17", Store: "17.11"}, Machines: []facts.Machine{{Name: "abcdef", Length: 6}}})
+	if err != nil {
+		t.Fatal(err)
+	}
+	sum := sha256.Sum256(body)
+	digest := "sha256:" + hex.EncodeToString(sum[:])
+	manifest, _ := json.Marshal(map[string]any{"schemaVersion": 2, "layers": []map[string]any{
+		{"mediaType": facts.MediaType, "digest": digest, "size": len(body)}}})
+	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case strings.Contains(r.URL.Path, "/manifests/"):
+			w.Write(manifest)
+		case strings.HasSuffix(r.URL.Path, "/blobs/"+digest):
+			w.Write(body)
+		default:
+			http.NotFound(w, r)
+		}
+	}))
+	t.Cleanup(srv.Close)
+	return strings.TrimPrefix(srv.URL, "http://")
+}
+
+// bareURL is a URL with its userinfo left out.
+func bareURL(t *testing.T, raw string) string {
+	u, err := url.Parse(raw)
+	if err != nil {
+		t.Fatal(err)
+	}
+	u.User = nil
+	return u.String()
+}
+
+func TestACheckContainerSeesNoForgeCredential(t *testing.T) {
+	repo, head := aCheckedRepository(t, map[string]string{CheckScript: "echo checked\n"})
+	besideRepo, besideHead := aCheckedRepository(t, map[string]string{"README": "beside"})
+	// A clone source that carries userinfo, as a forge's clone URL may: git records it as given in the
+	// clone's .git/config, which the container reads.
+	besideURL := "file://beside-user:" + besideSecret + "@" + besideRepo
+	workspace := t.TempDir()
+
+	var stores []string
+	reached := false
+	var leaks []string
+	run := func(ctx context.Context, dir, name string, args ...string) (string, error) {
+		switch name {
+		case "git":
+			for _, a := range args {
+				if f, ok := strings.CutPrefix(a, "credential.helper=store --file="); ok {
+					stores = append(stores, f)
+					raw, err := os.ReadFile(f)
+					if err != nil || strings.TrimSpace(string(raw)) != forgeURL {
+						t.Errorf("git is offered a store that does not hold the credential as given: %q, %v", raw, err)
+					}
+					if info, err := os.Stat(f); err == nil && info.Mode().Perm() != 0o600 {
+						t.Errorf("the credential store is readable beyond its owner: %v", info.Mode())
+					}
+				}
+			}
+			if len(args) >= 2 && args[len(args)-3] == "--quiet" && hasString(args, "clone") {
+				source := args[len(args)-2]
+				if u, err := url.Parse(source); err == nil && u.User != nil {
+					// Git cannot reach a file:// URL with userinfo; clone it without, then record it as git
+					// would have: as given.
+					clone := append(append([]string{}, args[:len(args)-2]...), bareURL(t, source), args[len(args)-1])
+					if out, err := Command(ctx, dir, "git", clone...); err != nil {
+						return out, err
+					}
+					return Command(ctx, filepath.Join(dir, args[len(args)-1]), "git", "remote", "set-url", "origin", source)
+				}
+			}
+			return Command(ctx, dir, name, args...)
+		case "docker":
+			if !reached {
+				reached = true
+				// The first container: everything the workspace holds is what the toolchain container sees.
+				filepath.WalkDir(workspace, func(path string, d fs.DirEntry, err error) error {
+					if err != nil || d.IsDir() {
+						return nil
+					}
+					raw, _ := os.ReadFile(path)
+					s := string(raw)
+					if strings.Contains(s, forgeSecret) || strings.Contains(s, besideSecret) ||
+						strings.Contains(s, npmSecret) || d.Name() == "git-credentials" || d.Name() == ".npmrc" ||
+						strings.Contains(s, "credential.helper") {
+						leaks = append(leaks, path)
+					}
+					return nil
+				})
+				for _, f := range stores {
+					if _, err := os.Stat(f); !errors.Is(err, os.ErrNotExist) {
+						leaks = append(leaks, f+" (still there when the first container runs)")
+					}
+					if rel, err := filepath.Rel(workspace, f); err == nil && !strings.HasPrefix(rel, "..") {
+						leaks = append(leaks, f+" (inside the workspace the container mounts)")
+					}
+				}
+			}
+			if len(args) > 0 && args[0] == "ps" {
+				return "", nil
+			}
+			return "", errors.New("no container runtime in this test")
+		}
+		return "", errors.New("unexpected command " + name)
+	}
+	// A credential an older builder left in the workspace is removed too: the forge's, and the .npmrc a
+	// build wrote into the tree it cloned.
+	if err := os.WriteFile(filepath.Join(workspace, "git-credentials"), []byte(forgeURL+"\n"), 0o600); err != nil {
+		t.Fatal(err)
+	}
+	for _, dir := range []string{"source/x", "context-server"} {
+		if err := os.MkdirAll(filepath.Join(workspace, dir), 0o755); err != nil {
+			t.Fatal(err)
+		}
+		if err := os.WriteFile(filepath.Join(workspace, dir, ".npmrc"),
+			[]byte("//forge.invalid/api/packages/novox/npm/:_authToken="+npmSecret+"\n"), 0o600); err != nil {
+			t.Fatal(err)
+		}
+	}
+	_, err := Check(t.Context(), run, CheckSpec{ID: "check-462", Repository: repo, Ref: head, Owner: "novox",
+		Repo: "mesh-controller", Number: 1, Toolchain: "golang", Beside: map[string]Beside{
+			"mesh-catalog": {Repository: besideURL, Ref: besideHead}}}, workspace, aFactsRegistry(t),
+		GitCredential{URL: forgeURL}, nil)
+	if err == nil {
+		t.Fatal("the check ran past its first container in a test with none")
+	}
+	if !reached {
+		t.Fatalf("the check never reached its first container: %v", err)
+	}
+	if len(stores) == 0 {
+		t.Fatal("no clone was offered the forge credential")
+	}
+	if len(leaks) > 0 {
+		t.Fatalf("the check's container sees the credential:\n%s", strings.Join(leaks, "\n"))
+	}
+}
+
+// Every line a repository's own check prints is published to the build's log redacted.
+func TestACheckLinePublishedToTheLogIsRedacted(t *testing.T) {
+	var said []string
+	say := func(step, format string, args ...any) {
+		if step == "output" && len(args) > 0 {
+			said = append(said, args[0].(string))
+		}
+	}
+	var out tail
+	layer := ownCheck(t.Context(), CheckSpec{Toolchain: "golang"}, []ScriptPart{{Toolchain: "go", Script: CheckScript}},
+		t.TempDir(), &out, func() bool { return false }, func(string, string) *exec.Cmd {
+			return exec.CommandContext(t.Context(), "sh", "-c", "echo cloning http://mesh_builder:t0ps3cret-forge@forge.invalid/novox/x.git; "+
+				"echo token ghp_abcdefghijklmnopqrstuvwxyz0123456789")
+		}, say)
+	if layer == nil || layer.Verdict != "pass" {
+		t.Fatalf("the check answered %+v\n%s", layer, out.String())
+	}
+	joined := strings.Join(said, "\n")
+	if strings.Contains(joined, "t0ps3cret-forge") || strings.Contains(joined, "ghp_abcdef") {
+		t.Fatalf("a secret the check printed is published to the build's log:\n%s", joined)
+	}
+	if !strings.Contains(joined, "http://mesh_builder:[redacted: a password in a URI]@forge.invalid/novox/x.git") {
+		t.Fatalf("the line is not said with what was there named:\n%s", joined)
+	}
+}
+
+func TestTheRedactorHidesTheForgeCredentialAndShapes(t *testing.T) {
+	r := redactorFor(GitCredential{URL: forgeURL})
+	for in, want := range map[string]string{
+		"the secret alone: " + forgeSecret:    "the secret alone: [redacted: the forge credential]",
+		"go test ./... ok":                    "go test ./... ok",
+		"--password hunter22 and done":        "--password [redacted: the word after --password] and done",
+		"commit 3b6b54a0c1d2e3f4a5b6c7d8e9f0": "commit 3b6b54a0c1d2e3f4a5b6c7d8e9f0",
+	} {
+		if got := r.redact(in); got != want {
+			t.Errorf("%q redacted as %q, want %q", in, got, want)
+		}
+	}
+}
+
+// A build's clone of a URL carrying userinfo records it without, so no build leaves a credential in
+// workspace/source/.git/config while it runs either (novox/hq issue 462); the tree itself is gone when the
+// build ends.
+func TestABuildsCloneRecordsNoUserinfo(t *testing.T) {
+	repo, _ := aCheckedRepository(t, map[string]string{ManifestName: `{"module":"plain","version":"1"}`})
+	source := "file://build-user:" + besideSecret + "@" + repo
+	workspace := t.TempDir()
+	tree := filepath.Join(workspace, "source")
+	var configs []string
+	run := func(ctx context.Context, dir, name string, args ...string) (string, error) {
+		if name == "git" && hasString(args, "clone") && args[len(args)-2] == source {
+			clone := append(append([]string{}, args[:len(args)-2]...), bareURL(t, source), args[len(args)-1])
+			if out, err := Command(ctx, dir, "git", clone...); err != nil {
+				return out, err
+			}
+			return Command(ctx, args[len(args)-1], "git", "remote", "set-url", "origin", source)
+		}
+		if name == "git" && len(args) > 0 && args[0] == "rev-parse" {
+			raw, _ := os.ReadFile(filepath.Join(tree, ".git", "config"))
+			configs = append(configs, string(raw))
+		}
+		return Command(ctx, dir, name, args...)
+	}
+	if _, err := Build(t.Context(), run, &recorded{}, source, "", "", workspace, nil, Npmrc{}, GitCredential{}, nil); err != nil {
+		t.Fatal(err)
+	}
+	if len(configs) == 0 {
+		t.Fatal("the build never read its clone")
+	}
+	for _, c := range configs {
+		if strings.Contains(c, besideSecret) || strings.Contains(c, "build-user") {
+			t.Fatalf("the build's clone records the credential it was cloned with:\n%s", c)
+		}
+	}
+	if _, err := os.Stat(tree); !os.IsNotExist(err) {
+		t.Fatalf("the build's tree outlives the build: %v", err)
+	}
+}

internal/builder/packages_test.go

@@ -70,7 +70,16 @@ func TestNpmrcDisabledUntilThereIsARegistry(t *testing.T) {
 func TestAnImageBuildGetsTheCredentialInTheContextAndHostNetwork(t *testing.T) {
 	r, workspace := aRepository(t, withBoth, map[string]string{"Dockerfile": "FROM scratch\nCOPY .npmrc ./", "files/x": "y"})
 	n := Npmrc{Scope: "@novox", Registry: "https://forge.invalid/api/packages/novox/npm/", Token: "t"}
-	if _, err := Build(context.Background(), r.run, r,
+	npmrc := filepath.Join(workspace, "source", ".npmrc")
+	inContext := false
+	run := func(ctx context.Context, dir, name string, args ...string) (string, error) {
+		if name == "docker" && len(args) > 0 && args[0] == "build" {
+			_, err := os.Stat(npmrc)
+			inContext = err == nil
+		}
+		return r.run(ctx, dir, name, args...)
+	}
+	if _, err := Build(context.Background(), run, r,
 		"https://forge.invalid/meshboard.git", "", "", workspace, nil, n, GitCredential{}, nil); err != nil {
 		t.Fatalf("the build failed: %v", err)
 	}
@@ -90,11 +99,14 @@ func TestAnImageBuildGetsTheCredentialInTheContextAndHostNetwork(t *testing.T) {
 	if !strings.Contains(build, "--network host") {
 		t.Fatalf("the build was not given the host network to reach the registry: %s", build)
 	}
-	// The .npmrc is written into the build context (the source tree), where a Dockerfile COPYs it.
+	// The .npmrc is written into the build context (the source tree), where a Dockerfile COPYs it, and
-	tree := filepath.Join(workspace, "source")
+	// removed with the tree when the build ends: a later check's container mounts the workspace (novox/hq
-	npmrc := filepath.Join(tree, ".npmrc")
+	// issue 462).
-	if _, err := os.Stat(npmrc); err != nil {
+	if !inContext {
-		t.Fatalf("the credential was not written into the build context: %v", err)
+		t.Fatal("the credential was not in the build context when the image was built")
+	}
+	if _, err := os.Stat(npmrc); !os.IsNotExist(err) {
+		t.Fatalf("the credential outlives the build in the workspace: %v", err)
 	}
 }
 

internal/builder/redact.go

@@ -0,0 +1,191 @@
+package builder
+
+// **Every line of a check's output is redacted before it is kept** (novox/hq issue 462).
+//
+// A repository's own check prints into the build's log, which the bus keeps for days and anyone who may read
+// its events reads, and into the verdict, which the forge shows on the pull request. Software prints what it
+// was given — a URL carrying a password, a token in a flag — and a pull request may print on purpose.
+// So a line is said only after every secret the builder knows (the forge credential's password) and every
+// value whose shape says it is one is replaced by a mark naming what was there, as the journal verb does.
+//
+// Copied from the journal tool's redactor (mesh-catalog, modules/systemd/cmd/systemd-tools/secrets.go,
+// itself a copy of the docker module's), narrowed to a line's shapes, with the token shapes a check's output
+// may carry added. A third copy: sharing them through mesh-sdk is novox/hq issue 471.
+
+import (
+	"net/url"
+	"regexp"
+	"strings"
+)
+
+// secretName is a variable name that says its value is a secret.
+var secretName = regexp.MustCompile(`(?i)(pass(word|wd|phrase)?|secret|token|api_?key|private_?key|access_?key|credential|auth)`)
+
+// notAValue is a name that says its value is where a secret is, not the secret: a file or a path.
+var notAValue = regexp.MustCompile(`(?i)(_FILE|FILE|_PATH|_DIR)$`)
+
+// uriPassword is a URI carrying a password in its userinfo: scheme://user:password@.
+var uriPassword = regexp.MustCompile(`[A-Za-z][A-Za-z0-9+.-]*://[^\s/:@'"]*:([^\s/@'"]+)@`)
+
+// tokenShaped are tokens recognised by their own prefix, whatever surrounds them: a forge's or a host's
+// access token, a JSON web token, a NATS seed.
+var tokenShaped = []struct {
+	name string
+	re   *regexp.Regexp
+}{
+	{"an access token", regexp.MustCompile(`\b(gh[pousr]_[A-Za-z0-9]{20,}|github_pat_[A-Za-z0-9_]{20,}|glpat-[A-Za-z0-9_-]{20,}|xox[abpr]-[A-Za-z0-9-]{10,}|sk-ant-[A-Za-z0-9_-]{20,})`)},
+	{"a JSON web token", regexp.MustCompile(`\beyJ[A-Za-z0-9_-]{8,}\.eyJ[A-Za-z0-9_-]{8,}\.[A-Za-z0-9_-]+`)},
+	{"a NATS seed", regexp.MustCompile(`\bS[ACNOU][A-Z2-7]{56}\b`)},
+}
+
+// masked is a password a program already hid: ***, xxx, <redacted>, [REDACTED].
+var masked = regexp.MustCompile(`^(\*+|x+|X+|<[^>]*>|\[[^\]]*\]|%2A+)$`)
+
+// ordinary is a value under a secret's name that is not one: a path, an address, a number, a switch.
+var ordinary = regexp.MustCompile(`^(/.*|[A-Za-z][A-Za-z0-9+.-]*://.*|[0-9.]+[a-z]?|(?i:true|false|yes|no|on|off|none|null))$`)
+
+// leastSecret is the shortest value compared as a secret: a shorter one matches ordinary words.
+const leastSecret = 6
+
+// passwordFlags take a secret as their next word, or after `=`, whatever the program.
+var passwordFlags = map[string]bool{
+	"-P": true, "--password": true, "--pass": true, "--passwd": true, "--secret": true, "--secret-key": true,
+	"--token": true, "--api-key": true, "--apikey": true, "--auth": true,
+}
+
+// knownSecret is one value the builder holds, by the name it is said under.
+type knownSecret struct {
+	Name  string
+	Value string
+}
+
+// redactor hides the secrets it knows and those a line's shapes say are secrets.
+type redactor struct{ known []knownSecret }
+
+// redactorFor knows the forge credential's password, and its user's name with it, in every form git or a
+// program may print them.
+func redactorFor(forge GitCredential) redactor {
+	var r redactor
+	if forge.URL == "" {
+		return r
+	}
+	for _, m := range uriPassword.FindAllStringSubmatch(forge.URL, -1) {
+		r.add("the forge credential", m[1])
+		if dec, err := url.PathUnescape(m[1]); err == nil && dec != m[1] {
+			r.add("the forge credential", dec)
+		}
+	}
+	return r
+}
+
+func (r *redactor) add(name, value string) {
+	if len(value) < leastSecret || masked.MatchString(value) {
+		return
+	}
+	for _, k := range r.known {
+		if k.Value == value {
+			return
+		}
+	}
+	r.known = append(r.known, knownSecret{name, value})
+}
+
+// redact is a text with every known secret, every value its shape says is one, and every password inside a
+// URI replaced by a mark naming what was there. Line by line: a shape is judged within its line.
+func (r redactor) redact(text string) string {
+	if !strings.ContainsAny(text, "\n") {
+		return r.line(text)
+	}
+	lines := strings.Split(text, "\n")
+	for i, l := range lines {
+		lines[i] = r.line(l)
+	}
+	return strings.Join(lines, "\n")
+}
+
+func (r redactor) line(line string) string {
+	replace := func(s knownSecret) {
+		for _, f := range forms(s.Value) {
+			line = strings.ReplaceAll(line, f, "[redacted: "+s.Name+"]")
+		}
+	}
+	for _, s := range r.known {
+		replace(s)
+	}
+	for _, s := range shaped(line) {
+		replace(s)
+	}
+	line = uriPassword.ReplaceAllStringFunc(line, func(m string) string {
+		sub := uriPassword.FindStringSubmatch(m)
+		if masked.MatchString(sub[1]) || strings.HasPrefix(sub[1], "[redacted") {
+			return m
+		}
+		return strings.TrimSuffix(m, sub[1]+"@") + "[redacted: a password in a URI]@"
+	})
+	for _, t := range tokenShaped {
+		line = t.re.ReplaceAllString(line, "[redacted: "+t.name+"]")
+	}
+	return line
+}
+
+// forms are the ways a value may appear printed: as given, and URL-encoded.
+func forms(value string) []string {
+	out := []string{value}
+	for _, f := range []string{url.QueryEscape(value), url.PathEscape(value)} {
+		if f != value && !hasString(out, f) {
+			out = append(out, f)
+		}
+	}
+	return out
+}
+
+func hasString(list []string, s string) bool {
+	for _, x := range list {
+		if x == s {
+			return true
+		}
+	}
+	return false
+}
+
+// shaped are the values a line carries by their shape: the word after a password flag, or the value of one
+// given with `=`, and a NAME=value whose name says secret.
+func shaped(line string) []knownSecret {
+	var out []knownSecret
+	add := func(name, value string) {
+		value = strings.Trim(value, `"',;`)
+		if len(value) < leastSecret || masked.MatchString(value) || ordinary.MatchString(value) ||
+			strings.HasPrefix(value, "[redacted") {
+			return
+		}
+		out = append(out, knownSecret{name, value})
+	}
+	words := strings.Fields(line)
+	for i, w := range words {
+		if flag, value, ok := strings.Cut(w, "="); ok && strings.HasPrefix(flag, "-") {
+			if passwordFlags[flag] {
+				add("the value of "+flag, value)
+			}
+			continue
+		}
+		if name, value, ok := strings.Cut(w, "="); ok && name != "" && secretName.MatchString(name) &&
+			!notAValue.MatchString(name) && !strings.ContainsAny(name, "/:") {
+			add("the value of "+name, value)
+			continue
+		}
+		if i+1 < len(words) && passwordFlags[w] {
+			add("the word after "+w, words[i+1])
+		}
+	}
+	return out
+}
+
+// withoutUserinfo is a URL with its userinfo left out, and whether it carried any.
+func withoutUserinfo(raw string) (string, bool) {
+	u, err := url.Parse(raw)
+	if err != nil || u.User == nil {
+		return raw, false
+	}
+	u.User = nil
+	return u.String(), true
+}