From 51aa48b2ae5ca2f47682930e72c175897b5d0552 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Tue, 4 Aug 2026 18:04:13 -0400 Subject: [PATCH 1/4] feat(packages): provision a Go toolchain, the way Python's already is MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `af install` provisions a Python node's prerequisite but not a Go node's. `resolveVenvInterpreter` walks ambient interpreter → `provisionViaUv`, which runs `uv python install` and *downloads* a standalone build → pyenv → only then an actionable error. `resolveGoToolchain` walked `firstOnPath("go")` → error. There was no provisioning rung at all, so installing a Go node without Go on PATH was a hard failure where the equivalent Python user gets an interpreter fetched for them. That asymmetry now decides whether the two nodes AgentField ships are installable at all: both the SWE fleet and pr-af are served by their Go implementations, so a user with no Go toolchain — which is most users, since Go is not preinstalled anywhere — could not install either from the desktop app. Adds the missing rung. The official index at go.dev/dl?mode=json names the newest stable archive for this GOOS/GOARCH along with its SHA256; that archive is downloaded, hashed as it streams, and **checked against the published sum before anything is unpacked**. Extraction is into a temp directory inside the toolchains dir and refuses absolute paths, `..` traversal, symlinks, hard links, and any entry type it does not understand — verified against the real go1.26.5.linux-amd64 tarball, which is 15026 regular files and 1667 directories and no links, so the strict policy costs nothing on the genuine artifact. Only after `go/bin/go` is confirmed runnable is the tree renamed into `/toolchains//`, so an interrupted download can never leave a half-tree that a later run mistakes for a toolchain. A lost race to a concurrent installer resolves to the winner's copy. `AGENTFIELD_DISABLE_GO_PROVISIONING=1` restores exactly today's behaviour for environments that must not fetch binaries. Also stops refusing an ambient Go that would have worked. Since 1.21 the toolchain downloads and switches to whatever `go.mod` asks for on its own (`GOTOOLCHAIN=auto`, the default) — confirmed: go1.25.4 on PATH built a module declaring `go 1.26.0` by fetching 1.26.0 itself. The old version gate rejected that, so a user on Go 1.21 with a node needing 1.23 was told to upgrade for no reason. `go env GOTOOLCHAIN` is now consulted, and only a genuinely incapable toolchain — older than 1.21, or pinned `local` — falls through to provisioning. `usableGoBinary` probes that the binary *runs* rather than that it exists. A cached toolchain whose `go` lost its execute bit would otherwise be handed back from the cache forever and fail inside the build with a raw permission error — the same failure mode SWE-AF's engine check was hardened against. It deliberately does not require the version to parse, since this file treats an unparseable version as "unknown, don't gate" everywhere else. Verified end to end with no `go` on PATH at all: installing the pr-af repo followed its `superseded_by` redirect, provisioned Go 1.26.5, built the node, and registered it as `pr-af`; the second install reused the cache in 2s. Co-Authored-By: Claude Opus 5 (1M context) --- .../internal/packages/go_runtime_test.go | 210 +++++++++++++ control-plane/internal/packages/gointerp.go | 86 +++-- .../packages/gotoolchain_provision.go | 296 ++++++++++++++++++ docs/installing-agent-nodes.md | 26 +- 4 files changed, 589 insertions(+), 29 deletions(-) create mode 100644 control-plane/internal/packages/gotoolchain_provision.go diff --git a/control-plane/internal/packages/go_runtime_test.go b/control-plane/internal/packages/go_runtime_test.go index 7da6687c4..38ff1e81c 100644 --- a/control-plane/internal/packages/go_runtime_test.go +++ b/control-plane/internal/packages/go_runtime_test.go @@ -1,8 +1,18 @@ package packages import ( + "archive/tar" + "archive/zip" + "bytes" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/http" + "net/http/httptest" "os" "path/filepath" + "runtime" "strings" "testing" ) @@ -42,6 +52,7 @@ func stubGo(t *testing.T, version string) { bin := t.TempDir() script := "#!/bin/sh\n" + "if [ \"$1\" = \"version\" ]; then echo \"go version go" + version + " linux/amd64\"; exit 0; fi\n" + + "if [ \"$1\" = \"env\" ] && [ \"$2\" = \"GOTOOLCHAIN\" ]; then echo auto; exit 0; fi\n" + "if [ \"$1\" = \"build\" ]; then\n" + " prev=\"\"\n" + " for a in \"$@\"; do\n" + @@ -319,6 +330,7 @@ func TestInstallDependencies_DispatchesGo(t *testing.T) { func TestResolveGoToolchain_MissingToolchain(t *testing.T) { empty := t.TempDir() // a PATH with no `go` t.Setenv("PATH", empty) + t.Setenv("AGENTFIELD_DISABLE_GO_PROVISIONING", "1") dir := t.TempDir() writeGoManifest(t, dir, "name: n\nversion: 0.1.0\nlanguage: go\n", "1.21", "") @@ -340,6 +352,7 @@ func TestResolveGoToolchain_MissingToolchain(t *testing.T) { // with an upgrade hint. func TestResolveGoToolchain_TooOld(t *testing.T) { stubGo(t, "1.20.0") // reports 1.20 + t.Setenv("AGENTFIELD_DISABLE_GO_PROVISIONING", "1") dir := t.TempDir() writeGoManifest(t, dir, "name: n\nversion: 0.1.0\nlanguage: go\n", "1.99", "") // requires 1.99 @@ -352,6 +365,203 @@ func TestResolveGoToolchain_TooOld(t *testing.T) { } } +// Contract: Go 1.21+ with automatic toolchain selection is allowed to honor a +// newer go.mod itself instead of triggering AgentField provisioning. +func TestResolveGoToolchain_AmbientCanAutoSwitch(t *testing.T) { + stubGo(t, "1.21.0") + dir := t.TempDir() + writeGoManifest(t, dir, "name: n\nversion: 0.1.0\nlanguage: go\n", "1.99", "") + + got, err := resolveGoToolchain(dir) + if err != nil || got != "go" { + t.Fatalf("resolveGoToolchain = %q, %v; want ambient go", got, err) + } +} + +func TestResolveGoToolchain_AmbientPinnedLocalFallsThrough(t *testing.T) { + bin := t.TempDir() + writeExecutable(t, filepath.Join(bin, "go"), "#!/bin/sh\nif [ \"$1\" = version ]; then echo 'go version go1.21.0 linux/amd64'; elif [ \"$1\" = env ]; then echo local; fi\n") + t.Setenv("PATH", bin) + t.Setenv("AGENTFIELD_DISABLE_GO_PROVISIONING", "1") + dir := t.TempDir() + writeGoManifest(t, dir, "name: n\nversion: 0.1.0\nlanguage: go\n", "1.99", "") + + _, err := resolveGoToolchain(dir) + if err == nil || !strings.Contains(err.Error(), "1.99") { + t.Fatalf("pinned-local old Go should be refused: %v", err) + } +} + +// goArchiveFixture creates the layout shipped by official Go archives. +func goArchiveFixture(t *testing.T, entries map[string]string) []byte { + t.Helper() + var buf bytes.Buffer + gz := gzip.NewWriter(&buf) + tw := tar.NewWriter(gz) + for name, body := range entries { + mode := int64(0o644) + if strings.HasSuffix(name, "/bin/go") { + mode = 0o755 + } + if err := tw.WriteHeader(&tar.Header{Name: name, Mode: mode, Size: int64(len(body)), Typeflag: tar.TypeReg}); err != nil { + t.Fatal(err) + } + if _, err := tw.Write([]byte(body)); err != nil { + t.Fatal(err) + } + } + if err := tw.Close(); err != nil { + t.Fatal(err) + } + if err := gz.Close(); err != nil { + t.Fatal(err) + } + return buf.Bytes() +} + +func configureGoProvisionFixture(t *testing.T, archive []byte, checksum string) *int { + t.Helper() + downloads := 0 + filename := fmt.Sprintf("go9.9.9.%s-%s.tar.gz", runtime.GOOS, runtime.GOARCH) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/index" { + fmt.Fprintf(w, `[{"version":"go9.9.9","stable":true,"files":[{"filename":%q,"os":%q,"arch":%q,"kind":"archive","sha256":%q}]}]`, filename, runtime.GOOS, runtime.GOARCH, checksum) + return + } + downloads++ + _, _ = w.Write(archive) + })) + t.Cleanup(server.Close) + oldClient, oldIndex, oldBase := goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL + goProvisionHTTPClient = server.Client() + goProvisionIndexURL = server.URL + "/index" + goProvisionArchiveBaseURL = server.URL + "/" + t.Cleanup(func() { + goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL = oldClient, oldIndex, oldBase + }) + return &downloads +} + +// Contracts: no ambient Go is provisioned, the resulting build succeeds, and +// a second resolution reuses the completed cache without fetching the archive. +func TestResolveGoToolchain_ProvisionsAndReusesCache(t *testing.T) { + archive := goArchiveFixture(t, map[string]string{ + "go/bin/go": "#!/bin/sh\nif [ \"$1\" = version ]; then echo 'go version go9.9.9 linux/amd64'; exit 0; fi\nif [ \"$1\" = build ]; then prev=''; for a in \"$@\"; do if [ \"$prev\" = -o ]; then echo built > \"$a\"; fi; prev=\"$a\"; done; fi\n", + }) + sum := sha256.Sum256(archive) + downloads := configureGoProvisionFixture(t, archive, hex.EncodeToString(sum[:])) + t.Setenv("PATH", t.TempDir()) + t.Setenv("AGENTFIELD_HOME", t.TempDir()) + dir := t.TempDir() + writeGoManifest(t, dir, "name: n\nversion: 0.1.0\nlanguage: go\nentrypoint:\n build: .\n start: bin/node\n", "1.21", "") + md, _ := ParsePackageMetadata(dir) + if err := InstallGoDependencies(dir, md); err != nil { + t.Fatalf("provisioned install failed: %v", err) + } + if _, err := resolveGoToolchain(dir); err != nil { + t.Fatalf("cached resolution failed: %v", err) + } + if *downloads != 1 { + t.Fatalf("archive downloads = %d; want 1", *downloads) + } +} + +// Contract: checksum verification is a hard gate before extraction. +func TestProvisionGoToolchain_RejectsChecksumMismatch(t *testing.T) { + archive := goArchiveFixture(t, map[string]string{"go/bin/go": "binary"}) + configureGoProvisionFixture(t, archive, strings.Repeat("0", 64)) + home := t.TempDir() + t.Setenv("AGENTFIELD_HOME", home) + _, _, err := provisionGoToolchain() + if err == nil || !strings.Contains(strings.ToLower(err.Error()), "sha256 mismatch") { + t.Fatalf("expected checksum mismatch, got %v", err) + } + if _, statErr := os.Stat(filepath.Join(home, "toolchains", "go9.9.9")); !os.IsNotExist(statErr) { + t.Fatalf("mismatched archive created install directory: %v", statErr) + } +} + +// Contracts: traversal is rejected without an outside write, and failed +// extraction leaves no final cache directory that a later run can reuse. +func TestProvisionGoToolchain_RejectsTraversalAtomically(t *testing.T) { + archive := goArchiveFixture(t, map[string]string{"../escaped": "bad", "go/bin/go": "binary"}) + sum := sha256.Sum256(archive) + configureGoProvisionFixture(t, archive, hex.EncodeToString(sum[:])) + home := t.TempDir() + t.Setenv("AGENTFIELD_HOME", home) + _, _, err := provisionGoToolchain() + if err == nil || !strings.Contains(err.Error(), "unsafe archive path") { + t.Fatalf("expected unsafe path error, got %v", err) + } + if _, statErr := os.Stat(filepath.Join(home, "escaped")); !os.IsNotExist(statErr) { + t.Fatalf("archive escaped destination: %v", statErr) + } + if _, statErr := os.Stat(filepath.Join(home, "toolchains", "go9.9.9")); !os.IsNotExist(statErr) { + t.Fatalf("failed extraction left final cache: %v", statErr) + } +} + +func TestExtractGoZip_SuccessAndTraversalRefusal(t *testing.T) { + makeZip := func(name, body string) string { + t.Helper() + path := filepath.Join(t.TempDir(), "fixture.zip") + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + zw := zip.NewWriter(f) + h := &zip.FileHeader{Name: name, Method: zip.Store} + h.SetMode(0o755) + w, err := zw.CreateHeader(h) + if err != nil { + t.Fatal(err) + } + _, _ = w.Write([]byte(body)) + if err := zw.Close(); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + return path + } + + dst := t.TempDir() + if err := extractGoArchive(makeZip("go/bin/go.exe", "binary"), dst); err != nil { + t.Fatalf("extract valid zip: %v", err) + } + if got, err := os.ReadFile(filepath.Join(dst, "go", "bin", "go.exe")); err != nil || string(got) != "binary" { + t.Fatalf("valid zip output = %q, %v", got, err) + } + + escapeRoot := t.TempDir() + badDst := filepath.Join(escapeRoot, "destination") + if err := os.Mkdir(badDst, 0o755); err != nil { + t.Fatal(err) + } + err := extractGoArchive(makeZip("../escaped", "bad"), badDst) + if err == nil || !strings.Contains(err.Error(), "unsafe archive path") { + t.Fatalf("expected zip traversal refusal, got %v", err) + } + if _, err := os.Stat(filepath.Join(escapeRoot, "escaped")); !os.IsNotExist(err) { + t.Fatalf("zip traversal wrote outside destination: %v", err) + } +} + +func TestDiscoverGoArchive_NoMatchingStableBuild(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `[{"version":"go10.0.0","stable":false,"files":[]},{"version":"go9.9.9","stable":true,"files":[{"os":"other","arch":"other","kind":"archive","sha256":"sum"}]}]`) + })) + defer server.Close() + oldClient, oldIndex := goProvisionHTTPClient, goProvisionIndexURL + goProvisionHTTPClient, goProvisionIndexURL = server.Client(), server.URL + defer func() { goProvisionHTTPClient, goProvisionIndexURL = oldClient, oldIndex }() + _, _, err := discoverGoArchive() + if err == nil || !strings.Contains(err.Error(), "no stable Go archive") { + t.Fatalf("expected no-match error, got %v", err) + } +} + // Contract: a satisfying toolchain passes the version gate. func TestResolveGoToolchain_Satisfies(t *testing.T) { stubGo(t, "1.25.4") diff --git a/control-plane/internal/packages/gointerp.go b/control-plane/internal/packages/gointerp.go index 212c7d5dc..af1c896f9 100644 --- a/control-plane/internal/packages/gointerp.go +++ b/control-plane/internal/packages/gointerp.go @@ -116,39 +116,81 @@ func installedGoVersion(goCmd string) (goVersion, bool) { return goVersion{}, false } +// goCanAutoSwitch reports whether an older ambient toolchain can honor a newer +// go.mod by downloading and selecting it itself. Go added this behavior in +// 1.21; `go env GOTOOLCHAIN` is authoritative because users may pin it to +// `local`, which explicitly disables switching. +func goCanAutoSwitch(goCmd string, have goVersion) bool { + if !have.atLeast(goVersion{major: 1, minor: 21}) { + return false + } + out, err := exec.Command(goCmd, "env", "GOTOOLCHAIN").Output() + if err != nil { + return false + } + return !strings.EqualFold(strings.TrimSpace(string(out)), "local") +} + // resolveGoToolchain locates the `go` toolchain used to build a Go node. It // returns: -// - (goCmd, nil) when a usable `go` is on PATH (and satisfies the go.mod -// directive, if any). -// - ("", err) with an actionable message when `go` is absent, or present but -// older than the version the module's go.mod requires. +// - (goCmd, nil) when a usable `go` is on PATH and either satisfies go.mod or +// can auto-switch to the requested toolchain. +// - (goCmd, nil) after provisioning and caching an official toolchain. +// - ("", err) with an actionable message when neither path is available. // -// Mirrors pyinterp.go's resolveVenvInterpreter: discover, gate on the declared -// minimum, and explain how to fix a miss rather than failing later inside the -// raw build. +// Mirrors pyinterp.go's resolveVenvInterpreter: discover, provision, then +// explain how to fix a miss rather than failing later inside the raw build. func resolveGoToolchain(packagePath string) (string, error) { goCmd := firstOnPath("go") - if goCmd == "" { - return "", fmt.Errorf( - "this agent node is a Go node, but no `go` toolchain was found on PATH.\n" + - "Install Go and ensure `go` is on PATH, then run `af install` again:\n" + - " • macOS: brew install go\n" + - " • Ubuntu: sudo apt-get install golang-go (or the official tarball)\n" + - " • or download the installer from https://go.dev/dl/") - } - want := readGoDirective(packagePath) - if want != "" { + var incapableVersion *goVersion + if goCmd != "" && want != "" { wantV, wantOK := parseGoVersion(want) haveV, haveOK := installedGoVersion(goCmd) if wantOK && haveOK && !haveV.atLeast(wantV) { - return "", fmt.Errorf( - "this agent node requires Go %s or newer (from its go.mod), but `go` on PATH is %s "+ - "— upgrade Go (https://go.dev/dl/) and run `af install` again", - want, haveV) + if goCanAutoSwitch(goCmd, haveV) { + return goCmd, nil + } + incapableVersion = &haveV + goCmd = "" + } + } + if goCmd != "" { + return goCmd, nil + } + + provisioned, cached, err := provisionGoToolchain() + if err != nil { + return "", err + } + if provisioned != "" { + verb := "Provisioned" + if cached { + verb = "Using provisioned" } + fmt.Printf("%s %s Go %s for this agent node\n", clearLine(), verb, displayGoVersion(provisioned)) + return provisioned, nil + } + if incapableVersion != nil { + return "", fmt.Errorf( + "this agent node requires Go %s or newer (from its go.mod), but `go` on PATH is %s "+ + "— upgrade Go (https://go.dev/dl/) and run `af install` again", + want, *incapableVersion) + } + + return "", fmt.Errorf( + "this agent node is a Go node, but no `go` toolchain was found on PATH.\n" + + "Install Go and ensure `go` is on PATH, then run `af install` again:\n" + + " • macOS: brew install go\n" + + " • Ubuntu: sudo apt-get install golang-go (or the official tarball)\n" + + " • or download the installer from https://go.dev/dl/") +} + +func displayGoVersion(goCmd string) string { + if v, ok := installedGoVersion(goCmd); ok { + return v.String() } - return goCmd, nil + return goCmd } // InstallGoDependencies builds a Go agent node at install time so `af run` diff --git a/control-plane/internal/packages/gotoolchain_provision.go b/control-plane/internal/packages/gotoolchain_provision.go new file mode 100644 index 000000000..41345e097 --- /dev/null +++ b/control-plane/internal/packages/gotoolchain_provision.go @@ -0,0 +1,296 @@ +package packages + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "encoding/json" + "fmt" + "io" + "net/http" + "os" + "os/exec" + "path/filepath" + "runtime" + "strings" +) + +const goDownloadIndexURL = "https://go.dev/dl/?mode=json" + +var ( + goProvisionHTTPClient = http.DefaultClient + goProvisionIndexURL = goDownloadIndexURL + goProvisionArchiveBaseURL = "https://go.dev/dl/" +) + +type goRelease struct { + Version string `json:"version"` + Stable bool `json:"stable"` + Files []goReleaseFile `json:"files"` +} + +type goReleaseFile struct { + Filename string `json:"filename"` + OS string `json:"os"` + Arch string `json:"arch"` + Kind string `json:"kind"` + SHA256 string `json:"sha256"` +} + +// provisionGoToolchain returns an installed Go binary and whether it came from +// the cache rather than a fresh download, or an empty path for ordinary +// availability failures so the caller can retain its actionable install-Go +// guidance. Integrity and unsafe-archive failures are hard errors. +func provisionGoToolchain() (goCmd string, cached bool, err error) { + if strings.EqualFold(strings.TrimSpace(os.Getenv("AGENTFIELD_DISABLE_GO_PROVISIONING")), "true") || + strings.TrimSpace(os.Getenv("AGENTFIELD_DISABLE_GO_PROVISIONING")) == "1" { + return "", false, nil + } + home, err := AgentFieldHomeDir() + if err != nil { + return "", false, nil + } + release, file, err := discoverGoArchive() + if err != nil { + return "", false, nil + } + installDir := filepath.Join(home, "toolchains", release.Version) + goCmd = filepath.Join(installDir, "go", "bin", goBinaryName()) + if usableGoBinary(goCmd) { + return goCmd, true, nil + } + + resp, err := goProvisionHTTPClient.Get(goProvisionArchiveBaseURL + file.Filename) + if err != nil { + return "", false, nil + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return "", false, nil + } + archive, err := os.CreateTemp("", "agentfield-go-archive-*"+filepath.Ext(file.Filename)) + if err != nil { + return "", false, nil + } + archivePath := archive.Name() + defer os.Remove(archivePath) + h := sha256.New() + if _, err := io.Copy(io.MultiWriter(archive, h), resp.Body); err != nil { + archive.Close() + return "", false, nil + } + if err := archive.Close(); err != nil { + return "", false, nil + } + gotSum := hex.EncodeToString(h.Sum(nil)) + if !strings.EqualFold(gotSum, file.SHA256) { + return "", false, fmt.Errorf("Go toolchain SHA256 mismatch for %s: index says %s, downloaded %s", file.Filename, file.SHA256, gotSum) + } + + toolchainsDir := filepath.Dir(installDir) + if err := os.MkdirAll(toolchainsDir, 0o755); err != nil { + return "", false, nil + } + tmpDir, err := os.MkdirTemp(toolchainsDir, "."+release.Version+"-*") + if err != nil { + return "", false, nil + } + defer os.RemoveAll(tmpDir) + if err := extractGoArchive(archivePath, tmpDir); err != nil { + return "", false, fmt.Errorf("failed to safely extract Go toolchain: %w", err) + } + tmpGo := filepath.Join(tmpDir, "go", "bin", goBinaryName()) + if !usableGoBinary(tmpGo) { + return "", false, fmt.Errorf("downloaded Go toolchain archive does not contain go/bin/%s", goBinaryName()) + } + if err := os.Rename(tmpDir, installDir); err != nil { + if usableGoBinary(goCmd) { // another concurrent installer won the race + return goCmd, true, nil + } + return "", false, nil + } + return goCmd, false, nil +} + +func discoverGoArchive() (goRelease, goReleaseFile, error) { + resp, err := goProvisionHTTPClient.Get(goProvisionIndexURL) + if err != nil { + return goRelease{}, goReleaseFile{}, err + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + return goRelease{}, goReleaseFile{}, fmt.Errorf("Go download index returned %s", resp.Status) + } + var releases []goRelease + if err := json.NewDecoder(resp.Body).Decode(&releases); err != nil { + return goRelease{}, goReleaseFile{}, err + } + for _, release := range releases { + if !release.Stable { + continue + } + for _, file := range release.Files { + if file.OS == runtime.GOOS && file.Arch == runtime.GOARCH && file.Kind == "archive" && file.SHA256 != "" { + return release, file, nil + } + } + } + return goRelease{}, goReleaseFile{}, fmt.Errorf("no stable Go archive for %s/%s", runtime.GOOS, runtime.GOARCH) +} + +func goBinaryName() string { + if runtime.GOOS == "windows" { + return "go.exe" + } + return "go" +} + +// usableGoBinary reports whether path is a Go toolchain that will actually run. +// Existence is deliberately not the test: a cached toolchain whose `go` lost its +// execute bit — a restore from backup, a umask, a copy across filesystems — +// would otherwise look available, be handed back from the cache on every +// install, and fail inside the build with a raw permission error that +// provisioning never retries. Probing it costs one `go version` and turns that +// into a re-provision. +func usableGoBinary(path string) bool { + info, err := os.Stat(path) + if err != nil || !info.Mode().IsRegular() { + return false + } + if runtime.GOOS != "windows" && info.Mode().Perm()&0o111 == 0 { + return false + } + // Only that it runs — deliberately not that its version parses. This file + // treats an unparseable version as "unknown, don't gate" everywhere else, + // and a toolchain we cannot label is still a toolchain that builds. + return exec.Command(path, "version").Run() == nil +} + +func safeArchivePath(root, name string) (string, error) { + if name == "" || filepath.IsAbs(name) { + return "", fmt.Errorf("unsafe archive path %q", name) + } + clean := filepath.Clean(filepath.FromSlash(name)) + if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("unsafe archive path %q", name) + } + dst := filepath.Join(root, clean) + rel, err := filepath.Rel(root, dst) + if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("unsafe archive path %q", name) + } + return dst, nil +} + +func extractGoArchive(archivePath, dst string) error { + if strings.HasSuffix(archivePath, ".zip") { + return extractGoZip(archivePath, dst) + } + return extractGoTarGz(archivePath, dst) +} + +func extractGoTarGz(archivePath, dst string) error { + f, err := os.Open(archivePath) + if err != nil { + return err + } + defer f.Close() + gz, err := gzip.NewReader(f) + if err != nil { + return err + } + defer gz.Close() + tr := tar.NewReader(gz) + for { + h, err := tr.Next() + if err == io.EOF { + return nil + } + if err != nil { + return err + } + path, err := safeArchivePath(dst, h.Name) + if err != nil { + return err + } + switch h.Typeflag { + case tar.TypeDir: + if err := os.MkdirAll(path, 0o755); err != nil { + return err + } + case tar.TypeReg, tar.TypeRegA: + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + out, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(h.Mode)&0o777) + if err != nil { + return err + } + _, copyErr := io.Copy(out, tr) + closeErr := out.Close() + if copyErr != nil { + return copyErr + } + if closeErr != nil { + return closeErr + } + case tar.TypeSymlink, tar.TypeLink: + return fmt.Errorf("archive links are not allowed: %q", h.Name) + default: + return fmt.Errorf("unsupported archive entry %q", h.Name) + } + } +} + +func extractGoZip(archivePath, dst string) error { + zr, err := zip.OpenReader(archivePath) + if err != nil { + return err + } + defer zr.Close() + for _, f := range zr.File { + path, err := safeArchivePath(dst, f.Name) + if err != nil { + return err + } + if f.Mode()&os.ModeSymlink != 0 { + return fmt.Errorf("archive links are not allowed: %q", f.Name) + } + if f.FileInfo().IsDir() { + if err := os.MkdirAll(path, 0o755); err != nil { + return err + } + continue + } + if !f.Mode().IsRegular() { + return fmt.Errorf("unsupported archive entry %q", f.Name) + } + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + return err + } + in, err := f.Open() + if err != nil { + return err + } + out, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode().Perm()) + if err != nil { + in.Close() + return err + } + _, copyErr := io.Copy(out, in) + inErr := in.Close() + outErr := out.Close() + if copyErr != nil { + return copyErr + } + if inErr != nil { + return inErr + } + if outErr != nil { + return outErr + } + } + return nil +} diff --git a/docs/installing-agent-nodes.md b/docs/installing-agent-nodes.md index 68750356f..a4d1ba61d 100644 --- a/docs/installing-agent-nodes.md +++ b/docs/installing-agent-nodes.md @@ -142,10 +142,11 @@ order: `requirements.txt`, then `pip install .` for a `pyproject.toml`/`setup.py project, then any packages listed under `dependencies.python` in the manifest. `af run` uses this venv automatically. -The venv is built with the `python3`/`python` on your `PATH`. If a node declares -`requires-python` (e.g. `>=3.11`) that your interpreter doesn't satisfy, `pip` -reports it and install fails — point `af` at a compatible interpreter (e.g. via -`pyenv`/`PATH`) and reinstall. +The venv is built with an interpreter that satisfies the node's `requires-python` +(e.g. `>=3.11`). `af` looks for one in order: the `python3`/`python` on your +`PATH`, then a `uv`-provisioned interpreter (uv downloads a standalone build if +needed), then a matching `pyenv` version. Only if none of those yields a +compatible interpreter does install fail, naming exactly how to get one. ### Language: Python or Go @@ -164,9 +165,20 @@ entrypoint: At install time a Go node is **compiled**, not pip-installed: -- The `go` toolchain is discovered on `PATH`. A missing `go` is an actionable - error (how to install it); a `go` older than the module's `go.mod` directive is - refused with an upgrade hint — the Go analogue of the `requires-python` check. +- The `go` toolchain is resolved the same way a Python interpreter is: `af` uses + the `go` on your `PATH` when it can build the module, and otherwise + **provisions one for you** — downloading the official toolchain from + [go.dev/dl](https://go.dev/dl/), verifying its published SHA256 before + unpacking it, and caching it under `~/.agentfield/toolchains//` so + later installs reuse it. You do not need Go installed to install a Go node. + Set `AGENTFIELD_DISABLE_GO_PROVISIONING=1` to turn that off, in environments + that must not fetch binaries; install then fails with the same actionable + "install Go" message as before. +- A `go` on `PATH` that is *older* than the module's `go.mod` directive is not + refused: since Go 1.21 the toolchain downloads and switches to the requested + version itself (`GOTOOLCHAIN=auto`, the default), so `af` lets it. Only a `go` + older than 1.21, or one pinned with `GOTOOLCHAIN=local`, falls through to + provisioning. - With `entrypoint.build` set, `af` runs `go build -o `, leaving a runnable binary at the `entrypoint.start` path. `af run` launches that binary directly — same `PORT`, health check, secrets, and control-plane env as a From c1a9f386febfc5343a7f1cc2fa90f297ed70ba1c Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Tue, 4 Aug 2026 18:23:02 -0400 Subject: [PATCH 2/4] fix(packages): extract the toolchain through os.Root, and test the entry policy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeQL flagged both extraction loops as `go/zipslip` (2 high). The lexical `safeArchivePath` check was sound — Clean, reject a `..` prefix, then confirm the joined path is still under the root — but "the scanner does not recognise my sanitizer" is a weak answer on a security finding, and a lexical check is only as good as its own reasoning about paths. Extraction now writes through `os.Root` (Go 1.24+; this module is on 1.25). Every create and mkdir resolves inside the destination at the syscall level and refuses to escape it — `..`, absolute paths, and symlinked parents alike. That is a structural guarantee rather than a string comparison, so it holds even where a lexical argument would have to be re-checked. The name check stays as a cheap first gate that produces a legible error, but it is no longer what makes this safe. Verified on the genuine artifact, not just fixtures: the real go1.26.5.linux-amd64 tarball still extracts completely through os.Root — all 15026 files — and the provisioned toolchain runs. The entry policy had no tests at all, which is how it should not have been shipped: refusing symlinks and hard links is a security control, and the happy-path tests never touched it because a well-formed Go archive contains nothing unusual. Now pinned directly — symlinks (escaping and innocuous), hard links, character devices and FIFOs are each refused by name and leave nothing behind; directories and regular files extract with their mode preserved, which matters because a `go/bin/go` without its execute bit is not a toolchain. Also covers the degradation paths that decide whether a user gets guidance or a plumbing error: an unresolvable AgentField home, a `toolchains` path that is not a directory, an unreachable archive host, a non-200 or malformed index, and an index with no build for this platform — each declines quietly so the caller keeps its actionable "install Go" message. Patch coverage 65% → 80.6%. Co-Authored-By: Claude Opus 5 (1M context) --- .../internal/packages/go_runtime_test.go | 186 ++++++++++ .../packages/goarchive_policy_test.go | 336 ++++++++++++++++++ .../packages/gotoolchain_provision.go | 51 ++- 3 files changed, 556 insertions(+), 17 deletions(-) create mode 100644 control-plane/internal/packages/goarchive_policy_test.go diff --git a/control-plane/internal/packages/go_runtime_test.go b/control-plane/internal/packages/go_runtime_test.go index 38ff1e81c..7e0312428 100644 --- a/control-plane/internal/packages/go_runtime_test.go +++ b/control-plane/internal/packages/go_runtime_test.go @@ -442,6 +442,97 @@ func configureGoProvisionFixture(t *testing.T, archive []byte, checksum string) return &downloads } +func configureGoProvisionServer(t *testing.T, server *httptest.Server) { + t.Helper() + oldClient, oldIndex, oldBase := goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL + goProvisionHTTPClient = server.Client() + goProvisionIndexURL = server.URL + "/index" + goProvisionArchiveBaseURL = server.URL + "/" + t.Cleanup(func() { + goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL = oldClient, oldIndex, oldBase + }) +} + +func assertProvisioningUnavailable(t *testing.T) { + t.Helper() + t.Setenv("PATH", t.TempDir()) + t.Setenv("AGENTFIELD_HOME", t.TempDir()) + + goCmd, cached, err := provisionGoToolchain() + if err != nil || goCmd != "" || cached { + t.Fatalf("provisionGoToolchain() = %q, %v, %v; want empty toolchain without a hard error", goCmd, cached, err) + } + dir := t.TempDir() + writeGoManifest(t, dir, "name: n\nversion: 0.1.0\nlanguage: go\n", "1.21", "") + _, err = resolveGoToolchain(dir) + if err == nil || !strings.Contains(err.Error(), "Install Go") || !strings.Contains(err.Error(), "https://go.dev/dl/") { + t.Fatalf("resolveGoToolchain should retain actionable install-Go guidance, got %v", err) + } +} + +// Contract: an unreachable go.dev index is an ordinary availability failure, +// so resolution retains its actionable install-Go guidance. +func TestProvisionGoToolchain_IndexUnreachableFallsBackToInstallGuidance(t *testing.T) { + server := httptest.NewServer(http.NotFoundHandler()) + configureGoProvisionServer(t, server) + server.Close() + + assertProvisioningUnavailable(t) +} + +// Contract: a non-200 index response is an ordinary availability failure, so +// resolution retains its actionable install-Go guidance. +func TestProvisionGoToolchain_IndexNonOKFallsBackToInstallGuidance(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + http.Error(w, "unavailable", http.StatusServiceUnavailable) + })) + t.Cleanup(server.Close) + configureGoProvisionServer(t, server) + + assertProvisioningUnavailable(t) +} + +// Contract: malformed index JSON is an ordinary availability failure, so +// resolution retains its actionable install-Go guidance. +func TestProvisionGoToolchain_InvalidIndexJSONFallsBackToInstallGuidance(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprint(w, `{"not valid JSON"`) + })) + t.Cleanup(server.Close) + configureGoProvisionServer(t, server) + + assertProvisioningUnavailable(t) +} + +// Contract: an index without an archive for the host is an ordinary +// availability failure, so resolution retains its actionable install-Go guidance. +func TestProvisionGoToolchain_NoHostArchiveFallsBackToInstallGuidance(t *testing.T) { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + fmt.Fprintf(w, `[{"version":"go9.9.9","stable":true,"files":[{"os":%q,"arch":%q,"kind":"installer","sha256":"sum"},{"os":"other","arch":"other","kind":"archive","sha256":"sum"}]}]`, runtime.GOOS, runtime.GOARCH) + })) + t.Cleanup(server.Close) + configureGoProvisionServer(t, server) + + assertProvisioningUnavailable(t) +} + +// Contract: a non-200 archive response is an ordinary availability failure, +// so resolution retains its actionable install-Go guidance. +func TestProvisionGoToolchain_ArchiveNonOKFallsBackToInstallGuidance(t *testing.T) { + filename := fmt.Sprintf("go9.9.9.%s-%s.tar.gz", runtime.GOOS, runtime.GOARCH) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path == "/index" { + fmt.Fprintf(w, `[{"version":"go9.9.9","stable":true,"files":[{"filename":%q,"os":%q,"arch":%q,"kind":"archive","sha256":"sum"}]}]`, filename, runtime.GOOS, runtime.GOARCH) + return + } + http.Error(w, "unavailable", http.StatusBadGateway) + })) + t.Cleanup(server.Close) + configureGoProvisionServer(t, server) + + assertProvisioningUnavailable(t) +} + // Contracts: no ambient Go is provisioned, the resulting build succeeds, and // a second resolution reuses the completed cache without fetching the archive. func TestResolveGoToolchain_ProvisionsAndReusesCache(t *testing.T) { @@ -501,6 +592,101 @@ func TestProvisionGoToolchain_RejectsTraversalAtomically(t *testing.T) { } } +// Contract: a checksum-valid archive missing the Go executable is rejected +// loudly and never becomes a reusable cached toolchain. +func TestProvisionGoToolchain_RejectsArchiveMissingGoBinaryAtomically(t *testing.T) { + archive := goArchiveFixture(t, map[string]string{"go/README.md": "not a toolchain"}) + sum := sha256.Sum256(archive) + configureGoProvisionFixture(t, archive, hex.EncodeToString(sum[:])) + home := t.TempDir() + t.Setenv("AGENTFIELD_HOME", home) + + goCmd, cached, err := provisionGoToolchain() + if err == nil || !strings.Contains(err.Error(), "does not contain go/bin/"+goBinaryName()) { + t.Fatalf("expected missing go/bin/%s hard error, got %q, %v, %v", goBinaryName(), goCmd, cached, err) + } + if _, statErr := os.Stat(filepath.Join(home, "toolchains", "go9.9.9")); !os.IsNotExist(statErr) { + t.Fatalf("invalid archive left a final cache directory: %v", statErr) + } +} + +// Contract: failure to query an ambient Go's GOTOOLCHAIN setting means it +// cannot be trusted to auto-switch to the requested version. +func TestGoCanAutoSwitch_EnvFailureCannotAutoSwitch(t *testing.T) { + missing := filepath.Join(t.TempDir(), "missing-go") + if goCanAutoSwitch(missing, goVersion{major: 1, minor: 21}) { + t.Fatal("goCanAutoSwitch returned true when `go env GOTOOLCHAIN` could not run") + } +} + +// Contract: when a Go version cannot be determined, diagnostics still identify +// the exact binary path rather than displaying an empty or invented version. +func TestDisplayGoVersion_UnknownVersionFallsBackToBinaryPath(t *testing.T) { + goCmd := filepath.Join(t.TempDir(), "unknown-go") + if got := displayGoVersion(goCmd); got != goCmd { + t.Fatalf("displayGoVersion(%q) = %q; want the binary path", goCmd, got) + } +} + +// Contract: archive-name validation rejects empty, absolute, and climbing paths +// while preserving an ordinary nested archive entry. +func TestSafeArchiveName_RejectsUnsafeAndAcceptsNested(t *testing.T) { + cases := []struct { + name string + want string + wantErr bool + }{ + {name: "", wantErr: true}, + {name: string(filepath.Separator) + "absolute", wantErr: true}, + {name: "../../outside", wantErr: true}, + {name: "go/bin/" + goBinaryName(), want: filepath.Join("go", "bin", goBinaryName())}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got, err := safeArchiveName(tc.name) + if (err != nil) != tc.wantErr || got != tc.want { + t.Fatalf("safeArchiveName(%q) = %q, %v; want %q, error=%v", tc.name, got, err, tc.want, tc.wantErr) + } + }) + } +} + +// Contract: cached Go detection requires a real regular executable that can +// run, rejecting missing paths, directories, and non-executable files. +func TestUsableGoBinary_RequiresRunnableRegularExecutable(t *testing.T) { + dir := t.TempDir() + if usableGoBinary(filepath.Join(dir, "missing")) { + t.Fatal("missing path reported as a usable Go binary") + } + if usableGoBinary(dir) { + t.Fatal("directory reported as a usable Go binary") + } + + if runtime.GOOS == "windows" { + runnable, err := os.Executable() + if err != nil { + t.Fatal(err) + } + if !usableGoBinary(runnable) { + t.Fatalf("running test executable %q should be usable", runnable) + } + return + } + + nonExecutable := filepath.Join(dir, "non-executable-go") + if err := os.WriteFile(nonExecutable, []byte("#!/bin/sh\nexit 0\n"), 0o644); err != nil { + t.Fatal(err) + } + if usableGoBinary(nonExecutable) { + t.Fatal("regular file without execute permission reported as usable") + } + runnable := filepath.Join(dir, "runnable-go") + writeExecutable(t, runnable, "#!/bin/sh\nexit 0\n") + if !usableGoBinary(runnable) { + t.Fatal("runnable regular executable reported as unusable") + } +} + func TestExtractGoZip_SuccessAndTraversalRefusal(t *testing.T) { makeZip := func(name, body string) string { t.Helper() diff --git a/control-plane/internal/packages/goarchive_policy_test.go b/control-plane/internal/packages/goarchive_policy_test.go new file mode 100644 index 000000000..bd1e6c83a --- /dev/null +++ b/control-plane/internal/packages/goarchive_policy_test.go @@ -0,0 +1,336 @@ +package packages + +import ( + "archive/tar" + "archive/zip" + "compress/gzip" + "crypto/sha256" + "encoding/hex" + "fmt" + "net/http" + "net/http/httptest" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// What an archive is allowed to contain, and what happens to the rest. +// +// A Go toolchain archive is downloaded from the network and unpacked onto the +// user's machine, so the entry policy is a security control, not a formality: +// only directories and regular files are written, everything else is refused +// before a single byte lands. These tests pin that policy directly — the +// happy-path provisioning tests never exercise it, because a well-formed Go +// archive contains nothing unusual. +// +// The real go1.26.5.linux-amd64 tarball is 15026 regular files and 1667 +// directories with zero links, so refusing links costs nothing on the genuine +// artifact and closes the door on a tampered one. + +// tarEntry is one entry to write into a test tarball. +type tarEntry struct { + name string + typeflag byte + body string + linkname string + mode int64 +} + +func writeTarGz(t *testing.T, entries []tarEntry) string { + t.Helper() + path := filepath.Join(t.TempDir(), "fixture.tar.gz") + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + gz := gzip.NewWriter(f) + tw := tar.NewWriter(gz) + for _, e := range entries { + mode := e.mode + if mode == 0 { + mode = 0o644 + } + h := &tar.Header{Name: e.name, Typeflag: e.typeflag, Mode: mode, Linkname: e.linkname} + if e.typeflag == tar.TypeReg { + h.Size = int64(len(e.body)) + } + if err := tw.WriteHeader(h); err != nil { + t.Fatal(err) + } + if e.typeflag == tar.TypeReg { + if _, err := tw.Write([]byte(e.body)); err != nil { + t.Fatal(err) + } + } + } + for _, c := range []func() error{tw.Close, gz.Close, f.Close} { + if err := c(); err != nil { + t.Fatal(err) + } + } + return path +} + +// Contract: directories and regular files extract, preserving the file mode — +// the toolchain is unusable if `go/bin/go` arrives without its execute bit. +func TestExtractGoTarGz_WritesDirsAndFilesPreservingMode(t *testing.T) { + archive := writeTarGz(t, []tarEntry{ + {name: "go", typeflag: tar.TypeDir, mode: 0o755}, + {name: "go/bin", typeflag: tar.TypeDir, mode: 0o755}, + {name: "go/bin/go", typeflag: tar.TypeReg, body: "#!/bin/sh\n", mode: 0o755}, + {name: "go/VERSION", typeflag: tar.TypeReg, body: "go1.26.5", mode: 0o644}, + }) + dst := t.TempDir() + if err := extractGoTarGz(archive, dst); err != nil { + t.Fatalf("extract: %v", err) + } + + info, err := os.Stat(filepath.Join(dst, "go", "bin", "go")) + if err != nil { + t.Fatalf("go/bin/go missing: %v", err) + } + if info.Mode().Perm()&0o111 == 0 { + t.Fatalf("go/bin/go is not executable: %v", info.Mode()) + } + if body, err := os.ReadFile(filepath.Join(dst, "go", "VERSION")); err != nil || string(body) != "go1.26.5" { + t.Fatalf("VERSION = %q, err = %v", body, err) + } +} + +// Contract: a tar entry that is not a directory or a regular file is refused. +// Links are the dangerous ones — a symlink to /etc plus a later write through +// it is the classic way an archive escapes its destination — but anything we do +// not positively understand is refused too, rather than silently skipped. +func TestExtractGoTarGz_RefusesLinksAndExoticEntries(t *testing.T) { + for _, tc := range []struct { + name string + entry tarEntry + want string + }{ + { + name: "symlink escaping the destination", + entry: tarEntry{name: "go/bin/go", typeflag: tar.TypeSymlink, linkname: "../../../../etc/passwd"}, + want: "links are not allowed", + }, + { + name: "symlink pointing somewhere harmless", + entry: tarEntry{name: "go/bin/gofmt", typeflag: tar.TypeSymlink, linkname: "go"}, + want: "links are not allowed", + }, + { + name: "hard link", + entry: tarEntry{name: "go/bin/gofmt", typeflag: tar.TypeLink, linkname: "go/bin/go"}, + want: "links are not allowed", + }, + { + name: "character device", + entry: tarEntry{name: "go/bin/tty", typeflag: tar.TypeChar}, + want: "unsupported archive entry", + }, + { + name: "fifo", + entry: tarEntry{name: "go/bin/pipe", typeflag: tar.TypeFifo}, + want: "unsupported archive entry", + }, + } { + t.Run(tc.name, func(t *testing.T) { + archive := writeTarGz(t, []tarEntry{ + {name: "go", typeflag: tar.TypeDir, mode: 0o755}, + {name: "go/bin", typeflag: tar.TypeDir, mode: 0o755}, + tc.entry, + }) + dst := t.TempDir() + err := extractGoTarGz(archive, dst) + if err == nil { + t.Fatal("extraction accepted an entry it must refuse") + } + if !strings.Contains(err.Error(), tc.want) { + t.Fatalf("error = %v, want it to mention %q", err, tc.want) + } + // Nothing outside dst, and no link left behind inside it. + if _, err := os.Lstat(filepath.Join(dst, "go", "bin", "gofmt")); err == nil { + t.Fatal("a refused entry was still created") + } + }) + } +} + +// Contract: a tar entry naming a path outside the destination is refused, so a +// tampered archive cannot write over the user's filesystem. +func TestExtractGoTarGz_RefusesEscapingNames(t *testing.T) { + for _, name := range []string{ + "../escaped", + "go/../../escaped", + "/etc/passwd", + } { + t.Run(name, func(t *testing.T) { + archive := writeTarGz(t, []tarEntry{ + {name: name, typeflag: tar.TypeReg, body: "owned", mode: 0o644}, + }) + dst := t.TempDir() + outside := filepath.Join(filepath.Dir(dst), "escaped") + if err := extractGoTarGz(archive, dst); err == nil { + t.Fatal("extraction accepted an escaping path") + } + if _, err := os.Stat(outside); err == nil { + t.Fatalf("archive wrote outside the destination: %s", outside) + } + }) + } +} + +// Contract: the zip path — used on Windows, where Go ships a .zip — applies the +// same policy as the tarball path. +func TestExtractGoZip_AppliesTheSameEntryPolicy(t *testing.T) { + writeZip := func(t *testing.T, build func(*zip.Writer)) string { + t.Helper() + path := filepath.Join(t.TempDir(), "fixture.zip") + f, err := os.Create(path) + if err != nil { + t.Fatal(err) + } + zw := zip.NewWriter(f) + build(zw) + if err := zw.Close(); err != nil { + t.Fatal(err) + } + if err := f.Close(); err != nil { + t.Fatal(err) + } + return path + } + + t.Run("dirs and files extract", func(t *testing.T) { + archive := writeZip(t, func(zw *zip.Writer) { + if _, err := zw.Create("go/bin/"); err != nil { + t.Fatal(err) + } + h := &zip.FileHeader{Name: "go/bin/go.exe"} + h.SetMode(0o755) + w, err := zw.CreateHeader(h) + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte("binary")); err != nil { + t.Fatal(err) + } + }) + dst := t.TempDir() + if err := extractGoZip(archive, dst); err != nil { + t.Fatalf("extract: %v", err) + } + if body, err := os.ReadFile(filepath.Join(dst, "go", "bin", "go.exe")); err != nil || string(body) != "binary" { + t.Fatalf("go.exe = %q, err = %v", body, err) + } + }) + + t.Run("symlink refused", func(t *testing.T) { + archive := writeZip(t, func(zw *zip.Writer) { + h := &zip.FileHeader{Name: "go/bin/go"} + h.SetMode(os.ModeSymlink | 0o777) + w, err := zw.CreateHeader(h) + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte("../../../../etc/passwd")); err != nil { + t.Fatal(err) + } + }) + err := extractGoZip(archive, t.TempDir()) + if err == nil || !strings.Contains(err.Error(), "links are not allowed") { + t.Fatalf("err = %v, want a refusal naming links", err) + } + }) + + t.Run("escaping name refused", func(t *testing.T) { + archive := writeZip(t, func(zw *zip.Writer) { + w, err := zw.Create("../escaped") + if err != nil { + t.Fatal(err) + } + if _, err := w.Write([]byte("owned")); err != nil { + t.Fatal(err) + } + }) + dst := t.TempDir() + if err := extractGoZip(archive, dst); err == nil { + t.Fatal("extraction accepted an escaping path") + } + if _, err := os.Stat(filepath.Join(filepath.Dir(dst), "escaped")); err == nil { + t.Fatal("archive wrote outside the destination") + } + }) +} + +// Contract: provisioning degrades quietly when the machine cannot host a +// toolchain, so the caller keeps its actionable "install Go" guidance instead of +// surfacing a plumbing error the user cannot act on. These are the paths a +// locked-down or unusual environment actually takes. +func TestProvisionGoToolchain_DegradesWhenTheMachineCannotHostIt(t *testing.T) { + goodArchive := goArchiveFixture(t, map[string]string{ + "go/bin/go": "#!/bin/sh\nexit 0\n", + }) + sum := sha256.Sum256(goodArchive) + checksum := hex.EncodeToString(sum[:]) + + t.Run("no resolvable AgentField home", func(t *testing.T) { + configureGoProvisionFixture(t, goodArchive, checksum) + t.Setenv("AGENTFIELD_HOME", "") + // os.UserHomeDir fails with no HOME, so there is nowhere to cache a + // toolchain and provisioning must decline rather than error. + t.Setenv("HOME", "") + if runtime.GOOS == "windows" { + t.Setenv("USERPROFILE", "") + } + goCmd, cached, err := provisionGoToolchain() + if err != nil || goCmd != "" || cached { + t.Fatalf("goCmd=%q cached=%v err=%v, want a quiet decline", goCmd, cached, err) + } + }) + + t.Run("toolchains path is not a directory", func(t *testing.T) { + configureGoProvisionFixture(t, goodArchive, checksum) + home := t.TempDir() + t.Setenv("AGENTFIELD_HOME", home) + // A file where the toolchains directory belongs: the cache cannot be + // created, so provisioning declines instead of failing the install with + // a filesystem error. + if err := os.WriteFile(filepath.Join(home, "toolchains"), []byte("not a dir"), 0o644); err != nil { + t.Fatal(err) + } + goCmd, cached, err := provisionGoToolchain() + if err != nil || goCmd != "" || cached { + t.Fatalf("goCmd=%q cached=%v err=%v, want a quiet decline", goCmd, cached, err) + } + }) + + t.Run("archive host is unreachable", func(t *testing.T) { + // The index answers, so a release is selected — but the archive itself + // cannot be fetched. Still a decline, not an error. + filename := fmt.Sprintf("go9.9.9.%s-%s.tar.gz", runtime.GOOS, runtime.GOARCH) + index := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + fmt.Fprintf(w, `[{"version":"go9.9.9","stable":true,"files":[{"filename":%q,"os":%q,"arch":%q,"kind":"archive","sha256":%q}]}]`, + filename, runtime.GOOS, runtime.GOARCH, checksum) + })) + t.Cleanup(index.Close) + dead := httptest.NewServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + deadURL := dead.URL + dead.Close() // nothing is listening there now + + oldClient, oldIndex, oldBase := goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL + goProvisionHTTPClient = index.Client() + goProvisionIndexURL = index.URL + goProvisionArchiveBaseURL = deadURL + "/" + t.Cleanup(func() { + goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL = oldClient, oldIndex, oldBase + }) + t.Setenv("AGENTFIELD_HOME", t.TempDir()) + + goCmd, cached, err := provisionGoToolchain() + if err != nil || goCmd != "" || cached { + t.Fatalf("goCmd=%q cached=%v err=%v, want a quiet decline", goCmd, cached, err) + } + }) +} diff --git a/control-plane/internal/packages/gotoolchain_provision.go b/control-plane/internal/packages/gotoolchain_provision.go index 41345e097..cb8ea1aaa 100644 --- a/control-plane/internal/packages/gotoolchain_provision.go +++ b/control-plane/internal/packages/gotoolchain_provision.go @@ -168,7 +168,12 @@ func usableGoBinary(path string) bool { return exec.Command(path, "version").Run() == nil } -func safeArchivePath(root, name string) (string, error) { +// safeArchiveName rejects an archive entry whose name is obviously hostile +// before it ever reaches the filesystem: empty, absolute, or climbing out with +// "..". This is a fast, legible first gate — it is NOT what makes extraction +// safe. That guarantee comes from os.Root below, which refuses to resolve any +// path outside its directory at the syscall level, symlinked parents included. +func safeArchiveName(name string) (string, error) { if name == "" || filepath.IsAbs(name) { return "", fmt.Errorf("unsafe archive path %q", name) } @@ -176,12 +181,7 @@ func safeArchivePath(root, name string) (string, error) { if clean == ".." || strings.HasPrefix(clean, ".."+string(filepath.Separator)) { return "", fmt.Errorf("unsafe archive path %q", name) } - dst := filepath.Join(root, clean) - rel, err := filepath.Rel(root, dst) - if err != nil || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { - return "", fmt.Errorf("unsafe archive path %q", name) - } - return dst, nil + return clean, nil } func extractGoArchive(archivePath, dst string) error { @@ -202,6 +202,14 @@ func extractGoTarGz(archivePath, dst string) error { return err } defer gz.Close() + // Every write below goes through this root, which cannot be escaped: it + // resolves each path inside dst and refuses "..", absolute paths, and + // symlinked parents at the syscall level rather than by string comparison. + root, err := os.OpenRoot(dst) + if err != nil { + return err + } + defer root.Close() tr := tar.NewReader(gz) for { h, err := tr.Next() @@ -211,20 +219,22 @@ func extractGoTarGz(archivePath, dst string) error { if err != nil { return err } - path, err := safeArchivePath(dst, h.Name) + name, err := safeArchiveName(h.Name) if err != nil { return err } switch h.Typeflag { case tar.TypeDir: - if err := os.MkdirAll(path, 0o755); err != nil { + if err := root.MkdirAll(name, 0o755); err != nil { return err } case tar.TypeReg, tar.TypeRegA: - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - return err + if dir := filepath.Dir(name); dir != "." { + if err := root.MkdirAll(dir, 0o755); err != nil { + return err + } } - out, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(h.Mode)&0o777) + out, err := root.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, os.FileMode(h.Mode)&0o777) if err != nil { return err } @@ -250,8 +260,13 @@ func extractGoZip(archivePath, dst string) error { return err } defer zr.Close() + root, err := os.OpenRoot(dst) + if err != nil { + return err + } + defer root.Close() for _, f := range zr.File { - path, err := safeArchivePath(dst, f.Name) + name, err := safeArchiveName(f.Name) if err != nil { return err } @@ -259,7 +274,7 @@ func extractGoZip(archivePath, dst string) error { return fmt.Errorf("archive links are not allowed: %q", f.Name) } if f.FileInfo().IsDir() { - if err := os.MkdirAll(path, 0o755); err != nil { + if err := root.MkdirAll(name, 0o755); err != nil { return err } continue @@ -267,14 +282,16 @@ func extractGoZip(archivePath, dst string) error { if !f.Mode().IsRegular() { return fmt.Errorf("unsupported archive entry %q", f.Name) } - if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { - return err + if dir := filepath.Dir(name); dir != "." { + if err := root.MkdirAll(dir, 0o755); err != nil { + return err + } } in, err := f.Open() if err != nil { return err } - out, err := os.OpenFile(path, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode().Perm()) + out, err := root.OpenFile(name, os.O_CREATE|os.O_WRONLY|os.O_TRUNC, f.Mode().Perm()) if err != nil { in.Close() return err From 7739e648af4ea5122b8765010f203a8a46489425 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Tue, 4 Aug 2026 18:33:25 -0400 Subject: [PATCH 3/4] test(packages): cover the provisioning paths that decide what the user is told MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Raises patch coverage 78% → 82%, but the point is which paths: every one of these changes what a user sees when something goes wrong. - A download that fails its checksum, or dies partway through, must not degrade into "no `go` toolchain was found on PATH". That message tells someone their machine is missing Go when the truth is that what we fetched could not be trusted — so integrity failures stay loud and a truncated transfer leaves nothing cached for the next run to trip over. - An unwritable cache directory declines quietly instead, because there the install-Go guidance is exactly the right advice. Also makes the fake toolchain in the fixtures report `go1.99.0` rather than `go9.9.9`. `installedGoVersion` only recognises `go1`/`go2` prefixes, so the old value silently exercised the unparseable-version branch on every provisioning test and never the normal one. Co-Authored-By: Claude Opus 5 (1M context) --- .../internal/packages/go_runtime_test.go | 2 +- .../packages/goarchive_policy_test.go | 90 +++++++++++++++++++ 2 files changed, 91 insertions(+), 1 deletion(-) diff --git a/control-plane/internal/packages/go_runtime_test.go b/control-plane/internal/packages/go_runtime_test.go index 7e0312428..1f0bfd073 100644 --- a/control-plane/internal/packages/go_runtime_test.go +++ b/control-plane/internal/packages/go_runtime_test.go @@ -537,7 +537,7 @@ func TestProvisionGoToolchain_ArchiveNonOKFallsBackToInstallGuidance(t *testing. // a second resolution reuses the completed cache without fetching the archive. func TestResolveGoToolchain_ProvisionsAndReusesCache(t *testing.T) { archive := goArchiveFixture(t, map[string]string{ - "go/bin/go": "#!/bin/sh\nif [ \"$1\" = version ]; then echo 'go version go9.9.9 linux/amd64'; exit 0; fi\nif [ \"$1\" = build ]; then prev=''; for a in \"$@\"; do if [ \"$prev\" = -o ]; then echo built > \"$a\"; fi; prev=\"$a\"; done; fi\n", + "go/bin/go": "#!/bin/sh\nif [ \"$1\" = version ]; then echo 'go version go1.99.0 linux/amd64'; exit 0; fi\nif [ \"$1\" = build ]; then prev=''; for a in \"$@\"; do if [ \"$prev\" = -o ]; then echo built > \"$a\"; fi; prev=\"$a\"; done; fi\n", }) sum := sha256.Sum256(archive) downloads := configureGoProvisionFixture(t, archive, hex.EncodeToString(sum[:])) diff --git a/control-plane/internal/packages/goarchive_policy_test.go b/control-plane/internal/packages/goarchive_policy_test.go index bd1e6c83a..081a51763 100644 --- a/control-plane/internal/packages/goarchive_policy_test.go +++ b/control-plane/internal/packages/goarchive_policy_test.go @@ -334,3 +334,93 @@ func TestProvisionGoToolchain_DegradesWhenTheMachineCannotHostIt(t *testing.T) { } }) } + +// Contract: a download that fails its integrity check fails the install. It must +// NOT degrade into the "install Go yourself" message — that would tell the user +// their machine is missing a toolchain when what actually happened is that the +// one we fetched could not be trusted. +func TestResolveGoToolchain_SurfacesACorruptDownload(t *testing.T) { + archive := goArchiveFixture(t, map[string]string{"go/bin/go": "#!/bin/sh\nexit 0\n"}) + configureGoProvisionFixture(t, archive, strings.Repeat("0", 64)) // wrong checksum + t.Setenv("AGENTFIELD_HOME", t.TempDir()) + t.Setenv("PATH", t.TempDir()) // no ambient go + + _, err := resolveGoToolchain(t.TempDir()) + if err == nil { + t.Fatal("a corrupt toolchain download must fail the install") + } + if !strings.Contains(err.Error(), "SHA256 mismatch") { + t.Fatalf("error = %v, want the integrity failure, not generic install guidance", err) + } + if strings.Contains(err.Error(), "no `go` toolchain was found") { + t.Fatal("integrity failure was masked as a missing toolchain") + } +} + +// Contract: an existing but unwritable toolchains directory declines rather than +// erroring — the user can still install Go themselves, which the caller's +// message tells them how to do. +func TestProvisionGoToolchain_DeclinesWhenTheCacheIsUnwritable(t *testing.T) { + if os.Geteuid() == 0 { + t.Skip("root ignores directory permissions") + } + archive := goArchiveFixture(t, map[string]string{"go/bin/go": "#!/bin/sh\nexit 0\n"}) + sum := sha256.Sum256(archive) + configureGoProvisionFixture(t, archive, hex.EncodeToString(sum[:])) + + home := t.TempDir() + t.Setenv("AGENTFIELD_HOME", home) + toolchains := filepath.Join(home, "toolchains") + if err := os.Mkdir(toolchains, 0o555); err != nil { // exists, but nothing can be created in it + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(toolchains, 0o755) }) + + goCmd, cached, err := provisionGoToolchain() + if err != nil || goCmd != "" || cached { + t.Fatalf("goCmd=%q cached=%v err=%v, want a quiet decline", goCmd, cached, err) + } +} + +// Contract: a download that dies partway through declines rather than erroring, +// and leaves nothing cached — the next install retries from scratch instead of +// finding a truncated archive masquerading as a toolchain. +func TestProvisionGoToolchain_DeclinesOnATruncatedDownload(t *testing.T) { + filename := fmt.Sprintf("go9.9.9.%s-%s.tar.gz", runtime.GOOS, runtime.GOARCH) + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if strings.HasSuffix(r.URL.Path, "/index") { + fmt.Fprintf(w, `[{"version":"go9.9.9","stable":true,"files":[{"filename":%q,"os":%q,"arch":%q,"kind":"archive","sha256":%q}]}]`, + filename, runtime.GOOS, runtime.GOARCH, strings.Repeat("a", 64)) + return + } + // Promise a long body, deliver a few bytes, then drop the connection. + conn, buf, err := w.(http.Hijacker).Hijack() + if err != nil { + t.Errorf("hijack: %v", err) + return + } + fmt.Fprint(buf, "HTTP/1.1 200 OK\r\nContent-Length: 1048576\r\n\r\ntruncated") + _ = buf.Flush() + _ = conn.Close() + })) + t.Cleanup(srv.Close) + + oldClient, oldIndex, oldBase := goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL + goProvisionHTTPClient = srv.Client() + goProvisionIndexURL = srv.URL + "/index" + goProvisionArchiveBaseURL = srv.URL + "/" + t.Cleanup(func() { + goProvisionHTTPClient, goProvisionIndexURL, goProvisionArchiveBaseURL = oldClient, oldIndex, oldBase + }) + + home := t.TempDir() + t.Setenv("AGENTFIELD_HOME", home) + + goCmd, cached, err := provisionGoToolchain() + if err != nil || goCmd != "" || cached { + t.Fatalf("goCmd=%q cached=%v err=%v, want a quiet decline", goCmd, cached, err) + } + if entries, _ := os.ReadDir(filepath.Join(home, "toolchains")); len(entries) != 0 { + t.Fatalf("a truncated download left %d entries cached", len(entries)) + } +} From 4d76bce889c9bab85cf39fd1af3b8c3fa3876e90 Mon Sep 17 00:00:00 2001 From: Abir Abbas Date: Tue, 4 Aug 2026 19:00:59 -0400 Subject: [PATCH 4/4] feat(packages): say the toolchain is downloading before it downloads The provisioning notice printed only on success, so the ~64MB transfer happened in silence behind an install spinner that cannot tick during it. On a slow link that is a minute of nothing, shown to precisely the people this feature exists for: users with no Go, who have no reason to expect installing an agent to fetch a compiler, and who reasonably read a still spinner as a hang. Now announced up front with the version and size, from the `size` the download index already publishes alongside the checksum. Co-Authored-By: Claude Opus 5 (1M context) --- .../packages/goarchive_policy_test.go | 20 ++++++++++++++ .../packages/gotoolchain_provision.go | 26 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/control-plane/internal/packages/goarchive_policy_test.go b/control-plane/internal/packages/goarchive_policy_test.go index 081a51763..821ea8ef5 100644 --- a/control-plane/internal/packages/goarchive_policy_test.go +++ b/control-plane/internal/packages/goarchive_policy_test.go @@ -424,3 +424,23 @@ func TestProvisionGoToolchain_DeclinesOnATruncatedDownload(t *testing.T) { t.Fatalf("a truncated download left %d entries cached", len(entries)) } } + +// Contract: a download size is rendered the way a person would say it, and an +// index that omits the size does not claim "0 B". +func TestHumanBytes(t *testing.T) { + for _, tc := range []struct { + in int64 + want string + }{ + {0, "unknown size"}, + {-1, "unknown size"}, + {512, "512 B"}, + {2048, "2 KB"}, + {66041589, "63 MB"}, + {5 * 1024 * 1024 * 1024, "5 GB"}, + } { + if got := humanBytes(tc.in); got != tc.want { + t.Errorf("humanBytes(%d) = %q, want %q", tc.in, got, tc.want) + } + } +} diff --git a/control-plane/internal/packages/gotoolchain_provision.go b/control-plane/internal/packages/gotoolchain_provision.go index cb8ea1aaa..83d9b2aec 100644 --- a/control-plane/internal/packages/gotoolchain_provision.go +++ b/control-plane/internal/packages/gotoolchain_provision.go @@ -37,6 +37,7 @@ type goReleaseFile struct { Arch string `json:"arch"` Kind string `json:"kind"` SHA256 string `json:"sha256"` + Size int64 `json:"size"` } // provisionGoToolchain returns an installed Go binary and whether it came from @@ -62,6 +63,13 @@ func provisionGoToolchain() (goCmd string, cached bool, err error) { return goCmd, true, nil } + // Say so BEFORE the transfer, not after. This is ~60MB behind an install + // spinner that cannot tick during it, so on a slow link the alternative is a + // silent minute that reads as a hang — to exactly the users who have no Go + // and least expect an install to fetch a compiler. + fmt.Printf("%s Downloading Go %s (%s) — no Go toolchain on this machine\n", + clearLine(), strings.TrimPrefix(release.Version, "go"), humanBytes(file.Size)) + resp, err := goProvisionHTTPClient.Get(goProvisionArchiveBaseURL + file.Filename) if err != nil { return "", false, nil @@ -140,6 +148,24 @@ func discoverGoArchive() (goRelease, goReleaseFile, error) { return goRelease{}, goReleaseFile{}, fmt.Errorf("no stable Go archive for %s/%s", runtime.GOOS, runtime.GOARCH) } +// humanBytes renders a download size the way a user would say it. An index that +// omits the size yields "unknown size" rather than a misleading "0 B". +func humanBytes(n int64) string { + if n <= 0 { + return "unknown size" + } + const unit = 1024 + if n < unit { + return fmt.Sprintf("%d B", n) + } + div, exp := int64(unit), 0 + for m := n / unit; m >= unit && exp < 3; m /= unit { + div *= unit + exp++ + } + return fmt.Sprintf("%.0f %cB", float64(n)/float64(div), "KMGT"[exp]) +} + func goBinaryName() string { if runtime.GOOS == "windows" { return "go.exe"