Build every judge where no pull request can reach, and never read mesh-lab at a release
mesh/merge-gate out of date: judged against trunk c975dbf7b053, the trunk is now 76304d9db817; judged again on a recheck or when it is merged
mesh/repo-check out of date: judged against trunk c975dbf7b053, the trunk is now 76304d9db817; judged again on a recheck or when it is merged
mesh/delivery superseded: a newer head of the same pull request
mesh/merge-gate out of date: judged against trunk c975dbf7b053, the trunk is now 76304d9db817; judged again on a recheck or when it is merged
mesh/repo-check out of date: judged against trunk c975dbf7b053, the trunk is now 76304d9db817; judged again on a recheck or when it is merged
mesh/delivery superseded: a newer head of the same pull request
Review of #329: a judge built with HOME in the workspace runs git under a configuration a pull request's script can plant, so a hook could rewrite the judge's sources before they compile and the result be kept under an honest key (issue 482, issue 704). Every judge now builds with its own HOME, configuration and Go cache outside the workspace, no git configuration and no VCS stamping. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
+54
-22
@@ -429,10 +429,17 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
if dir == name || !safeName.MatchString(dir) {
|
||||
continue
|
||||
}
|
||||
if dir == "mesh-lab" && b.Else != "" {
|
||||
// **mesh-lab is read at its trunk, always** (novox/hq ADR 0358): its replays run with the container
|
||||
// runtime's socket, so a branch of a release's name — which anyone may create — is never what runs there.
|
||||
b.Ref, b.Else = b.Else, ""
|
||||
if dir == "mesh-lab" {
|
||||
// **mesh-lab is never read at a release branch** (novox/hq ADR 0358): its replays run with the container
|
||||
// runtime's socket, so a branch of a release's name — which anyone may create — is never what runs there,
|
||||
// whether it is asked with a fallback or without.
|
||||
if b.Else != "" || (spec.Release != "" && b.Ref == spec.Release) {
|
||||
b.Ref = b.Else
|
||||
if b.Ref == "" {
|
||||
b.Ref = "main"
|
||||
}
|
||||
}
|
||||
b.Else = ""
|
||||
}
|
||||
if err := cloneAt(b.Repository, b.Ref, b.Else, dir); err != nil {
|
||||
return CheckVerdict{}, fmt.Errorf("beside it, %w", err)
|
||||
@@ -565,21 +572,47 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
user = fmt.Sprintf("%d:%d", os.Getuid(), os.Getgid())
|
||||
}
|
||||
in := func(image, dir string, env []string, command ...string) []string {
|
||||
home := workspace
|
||||
for _, e := range env {
|
||||
if strings.HasPrefix(e, "HOME=") {
|
||||
home = strings.TrimPrefix(e, "HOME=")
|
||||
}
|
||||
}
|
||||
// As the builder itself: what a check writes into the workspace is the builder's to remove.
|
||||
args := []string{"run", "--rm", "--network", "host", "--volume", workspace + ":" + workspace, "--workdir", dir,
|
||||
"--user", user, "--env", "HOME=" + workspace,
|
||||
"--user", user, "--env", "HOME=" + home,
|
||||
// The check's own checkouts, whoever cloned them: git in a container of another user than the
|
||||
// one that cloned refuses a repository it does not own ("dubious ownership"), and Go's build
|
||||
// stamps the version from git — a judge that would not build for want of it.
|
||||
"--env", "GIT_CONFIG_COUNT=1", "--env", "GIT_CONFIG_KEY_0=safe.directory", "--env", "GIT_CONFIG_VALUE_0=*"}
|
||||
for _, e := range env {
|
||||
args = append(args, "--env", e)
|
||||
if !strings.HasPrefix(e, "HOME=") {
|
||||
args = append(args, "--env", e)
|
||||
}
|
||||
}
|
||||
return append(append(args, image), command...)
|
||||
}
|
||||
inToolchain := func(dir string, env []string, command ...string) []string {
|
||||
return in(spec.Toolchain, dir, env, command...)
|
||||
}
|
||||
// **Every judge is built where no pull request can reach** (novox/hq issue 482, ADR 0358 review): the workspace's
|
||||
// Go cache, HOME and git configuration are written by pull requests' own scripts, and git runs during a Go build
|
||||
// to stamp its version — a planted hook would rewrite the judge's sources before they compile. So a judge is built
|
||||
// with a HOME, configuration and Go cache of its own, outside the workspace and made fresh for this check, no
|
||||
// global or system git configuration, no Go environment file, and no version stamping from git.
|
||||
judgeHome, err := os.MkdirTemp("", "mesh-check-judge-")
|
||||
if err != nil {
|
||||
return CheckVerdict{}, err
|
||||
}
|
||||
defer os.RemoveAll(judgeHome)
|
||||
if withinDir(workspace, judgeHome) {
|
||||
return CheckVerdict{}, fmt.Errorf("the temporary directory %s is inside the workspace %s, which a check's "+
|
||||
"containers mount: no judge is built where a pull request could reach it", judgeHome, workspace)
|
||||
}
|
||||
inJudgeBuild := func(dir string, _ []string, command ...string) []string {
|
||||
args := in(spec.Toolchain, dir, JudgeBuildEnv(judgeHome), command...)
|
||||
return append([]string{args[0], "--volume", judgeHome + ":" + judgeHome}, args[1:]...)
|
||||
}
|
||||
var out tail
|
||||
// running runs one container of the check, its output into the report, in a process group of its own.
|
||||
running := func(args []string) error {
|
||||
@@ -591,7 +624,7 @@ func Check(ctx context.Context, run Runner, spec CheckSpec, workspace, registry
|
||||
|
||||
// The judge. Its failing to build is the change's fault when the change is the judge or its validator,
|
||||
// and the check's when it is the controller the mesh runs.
|
||||
gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, inToolchain, say)
|
||||
gate, judgeFault, err := judgeFor(ctx, labelled, run, spec, root, tree, judged.BesideAt, inJudgeBuild, say)
|
||||
if err != nil {
|
||||
return CheckVerdict{}, err
|
||||
}
|
||||
@@ -1046,8 +1079,7 @@ func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, t
|
||||
switch spec.Judge {
|
||||
case "self":
|
||||
bin := filepath.Join(root, "bin", "judge-of-itself")
|
||||
if _, err := labelled(ctx, root, "docker", inToolchain(tree,
|
||||
[]string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")},
|
||||
if _, err := labelled(ctx, root, "docker", inToolchain(tree, nil,
|
||||
"go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil {
|
||||
return "", "the change's controller, which judges itself, does not build: " + firstLine(err.Error()), nil
|
||||
}
|
||||
@@ -1080,8 +1112,7 @@ func judgeFor(ctx context.Context, labelled, run Runner, spec CheckSpec, root, t
|
||||
}
|
||||
}
|
||||
withValidator := filepath.Join(root, "bin", "judge-with-this-validator")
|
||||
if _, err := labelled(ctx, root, "docker", inToolchain(filepath.Join(root, judgeTree),
|
||||
[]string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")},
|
||||
if _, err := labelled(ctx, root, "docker", inToolchain(filepath.Join(root, judgeTree), nil,
|
||||
"go", "build", "-o", withValidator, "./cmd/mesh-controller")...); err != nil {
|
||||
if spec.Release != "" {
|
||||
return "", spec.Release + "'s controller does not build with this change's validator: " +
|
||||
@@ -1135,12 +1166,8 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s
|
||||
}
|
||||
}
|
||||
}
|
||||
// **Built with a cache of its own, never the workspace's** (novox/hq issue 482): the workspace's Go cache and HOME are
|
||||
// written by every pull request's own script, so a poisoned cache entry or Go environment file would otherwise be
|
||||
// built into the judge kept for this head. A fresh cache under this check's root, made before anything of the
|
||||
// change runs, and no Go environment file.
|
||||
if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir),
|
||||
[]string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOENV=off", "GOCACHE=" + judgeGoCache(root)},
|
||||
// Built where no pull request reaches (JudgeBuildEnv): a judge kept for this head is the head's, nothing else.
|
||||
if _, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), nil,
|
||||
"go", "build", "-o", bin, "./cmd/mesh-controller")...); err != nil {
|
||||
return "", "", fmt.Errorf("the controller of %s does not build%s: %w", spec.Release, with, err)
|
||||
}
|
||||
@@ -1150,9 +1177,15 @@ func releaseJudge(ctx context.Context, run, plain Runner, spec CheckSpec, root s
|
||||
return bin, dir, nil
|
||||
}
|
||||
|
||||
// judgeGoCache is the Go cache a release branch's judge is built with: fresh for each check, under its root, which is
|
||||
// emptied before the check clones anything — never the workspace's shared cache.
|
||||
func judgeGoCache(root string) string { return filepath.Join(root, "judge-go-cache") }
|
||||
// JudgeBuildEnv is the environment every judge is built in (novox/hq issue 482): home, a directory outside the
|
||||
// workspace made fresh for the check, as HOME, configuration and Go cache; no global or system git configuration; no
|
||||
// Go environment file; and no version stamped from git, so no git runs during the build at all.
|
||||
func JudgeBuildEnv(home string) []string {
|
||||
return []string{"HOME=" + home, "XDG_CONFIG_HOME=" + filepath.Join(home, ".config"),
|
||||
"GIT_CONFIG_GLOBAL=/dev/null", "GIT_CONFIG_NOSYSTEM=1", "GOENV=off",
|
||||
"GOCACHE=" + filepath.Join(home, "go-cache"), "GOPATH=" + filepath.Join(home, "go"),
|
||||
"CGO_ENABLED=0", "GOFLAGS=-mod=vendor -buildvcs=false", "GOPROXY=off"}
|
||||
}
|
||||
|
||||
// judgesKept is how many release-branch judges a judge cache keeps: a head moved past is not built from again.
|
||||
const judgesKept = 4
|
||||
@@ -1514,8 +1547,7 @@ func judge(ctx context.Context, run, plain Runner, root string, inToolchain func
|
||||
say func(step, format string, args ...any)) (string, string, error) {
|
||||
bin := filepath.Join(root, "bin", "mesh-controller")
|
||||
build := func(dir string) error {
|
||||
_, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir),
|
||||
[]string{"CGO_ENABLED=0", "GOFLAGS=-mod=vendor", "GOPROXY=off", "GOCACHE=" + filepath.Join(filepath.Dir(root), "go-cache")},
|
||||
_, err := run(ctx, root, "docker", inToolchain(filepath.Join(root, dir), nil,
|
||||
"go", "build", "-o", bin, "./cmd/mesh-controller")...)
|
||||
return err
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user