From 36351d22a6f6bf282142b2d782c5855e7ee97211 Mon Sep 17 00:00:00 2001 From: Saurabh Maheshwari <242053479+saurabhmaheshwari-obin@users.noreply.github.com> Date: Wed, 21 Oct 2026 16:58:34 -0400 Subject: [PATCH 1/6] feat(build): fetch declared git sources on the host and hand the build a mirror context A recipe can now declare the locked git commits it fetches while it builds (DockerBuildRecipe.git_source_downloads, recipe contract v6). The CLI fetches them the way it fetches declared Go modules: in the plan phase, after the plan is verified and image reuse is decided, with the host's own git and environment, so the host's insteadOf rewrites, credential helper or ssh agent apply and no credential reaches the build. Each repository gets one bare mirror at GitSourceMirrorPath(repository) in a per-flow directory, holding each declared commit under GitSourceMirrorRef(commit): fetched by id at depth 1, or through the repository's branches and tags when the host refuses an unadvertised commit, then verified with cat-file. The fetch sets GIT_TERMINAL_PROMPT=0 and the Go prefetch's maintenance.auto=false / gc.auto=0, and no GIT_ALLOW_PROTOCOL (git checks it after the host's rewrites). Errors are credential-redacted; a missing git is a clear error. The mirror directory is passed as --build-context =, normalized out of the image identity like a Go module proxy (the commits are in the recipe tree digest), a declaring recipe without its mirrors is refused, and the mirrors are removed when the flow stops. The core pin moves to the change that declares the field. Co-Authored-By: Claude Opus 5.5 --- .github/workflows/go.yml | 8 + pkg/orchestration/build_git_sources.go | 179 ++++++++++++++++++ .../build_git_sources_integration_test.go | 157 +++++++++++++++ pkg/orchestration/build_git_sources_test.go | 175 +++++++++++++++++ pkg/orchestration/build_go_modules.go | 20 +- pkg/orchestration/build_plan.go | 21 +- pkg/orchestration/build_private_modules.go | 4 +- pkg/orchestration/builder.go | 31 ++- pkg/orchestration/flow.go | 14 ++ pkg/orchestration/image_cache.go | 5 +- pkg/orchestration/image_cache_test.go | 30 +++ 11 files changed, 624 insertions(+), 20 deletions(-) create mode 100644 pkg/orchestration/build_git_sources.go create mode 100644 pkg/orchestration/build_git_sources_integration_test.go create mode 100644 pkg/orchestration/build_git_sources_test.go diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index a227a68f1..f40fb8332 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -190,6 +190,14 @@ jobs: if: matrix.gate == 'control-integration' run: go test -tags=integration -run '^TestTwoServicesPinningOneRepositoryFetchWithoutGitMaintainingTheCache$' ./pkg/orchestration -v + # A locked git commit only the host's git configuration reaches is + # mirrored on the host, by id and through a host that refuses unadvertised + # commits, and an image build with no network fetches it from the mirror + # context and fails without it. Needs git and Docker; no provider toolchain. + - name: Prove a git source build needs no credential with its prefetched mirror + if: matrix.gate == 'control-integration' + run: go test -tags=integration -run '^(TestALockedGitSourceIsMirroredWithTheHostsGitConfiguration|TestAGitSourceBuildNeedsNoCredentialWithThePrefetchedMirror)$' ./pkg/orchestration -v + # Keep the tag in step with qualificationImage in # cmd/generate/container_recovery_docker_test.go. - name: Pull the interrupted-generate proof's image diff --git a/pkg/orchestration/build_git_sources.go b/pkg/orchestration/build_git_sources.go new file mode 100644 index 000000000..eb26aa997 --- /dev/null +++ b/pkg/orchestration/build_git_sources.go @@ -0,0 +1,179 @@ +package orchestration + +import ( + "bytes" + "context" + "errors" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "sync" + + coreservices "github.com/codefly-dev/core/agents/services" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + + "github.com/codefly-dev/cli/pkg/redact" +) + +// gitSourcePrefetch fetches the locked git commits a flow's recipes declare +// (DockerBuildRecipe.git_source_downloads) once, on the host, before any image +// is built, and hands each build the result as bare mirrors in a named build +// context. It is goModulePrefetch for a dependency a lockfile pins to a git +// commit rather than to a module version. +// +// The fetch runs with the host's own git and environment, so it authenticates +// exactly as the developer's or the CI job's git does (insteadOf rewrites, a +// credential helper, an ssh agent); nothing secret reaches the build. Each +// repository has one bare mirror at GitSourceMirrorPath(repository), holding +// each declared commit under GitSourceMirrorRef(commit), which is the ref a +// fetch by commit id writes, so a build fetching that commit from the mirror +// finds it. Every recipe of the flow is handed the one directory holding all the +// mirrors, as every Go recipe is handed one module proxy: a build fetches only +// the commits its own lockfile names, so another service's mirrors are inert. +type gitSourcePrefetch struct { + mu sync.Mutex + root string + fetched map[[2]string]bool + // run executes git; a test substitutes it. + run func(ctx context.Context, env []string, args ...string) error +} + +func newGitSourcePrefetch() *gitSourcePrefetch { + return &gitSourcePrefetch{fetched: map[[2]string]bool{}, run: runGitCommand} +} + +// fetch places commit of repository in its mirror and returns the directory +// holding every mirror. A commit fetched earlier in the flow is not fetched +// again. +func (p *gitSourcePrefetch) fetch(ctx context.Context, repository, commit string) (string, error) { + p.mu.Lock() + defer p.mu.Unlock() + if p.fetched[[2]string{repository, commit}] { + return p.root, nil + } + if p.root == "" { + root, err := os.MkdirTemp("", "codefly-git-sources-*") + if err != nil { + return "", fmt.Errorf("cannot stage the git source prefetch: %w", err) + } + p.root = root + } + mirror := filepath.Join(p.root, coreservices.GitSourceMirrorPath(repository)) + env := gitSourcePrefetchEnv(os.Environ()) + if _, err := os.Stat(mirror); err != nil { + if err := p.run(ctx, env, "init", "--quiet", "--bare", mirror); err != nil { + return "", fmt.Errorf("cannot create the git source mirror: %w", err) + } + } + git := func(args ...string) error { + return p.run(ctx, env, append([]string{"--git-dir=" + mirror}, args...)...) + } + ref := coreservices.GitSourceMirrorRef(commit) + // A host that serves a commit by id gives just that commit. One that refuses + // an unadvertised commit (protocol v0 without allowAnySHA1InWant) gives it + // with its branches and tags, which is how any git client then reaches it. + if err := git("fetch", "--depth=1", "--end-of-options", repository, "+"+commit+":"+ref); err != nil { + full := []string{"fetch"} + if _, shallow := os.Stat(filepath.Join(mirror, "shallow")); shallow == nil { + full = append(full, "--unshallow") + } + full = append(full, "--end-of-options", repository, "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*") + if fullErr := git(full...); fullErr != nil { + return "", fmt.Errorf("cannot fetch commit %s of a declared git source: %w", commit, errors.Join(err, fullErr)) + } + if err := git("update-ref", ref, commit); err != nil { + return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) + } + } + if err := git("cat-file", "-e", commit+"^{commit}"); err != nil { + return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) + } + p.fetched[[2]string{repository, commit}] = true + return p.root, nil +} + +// mirrorsFor fetches every git source a recipe declares and returns the +// directory to supply under each declared mirror context. +func (p *gitSourcePrefetch) mirrorsFor(ctx context.Context, recipe *builderv0.DockerBuildRecipe) (map[string]string, error) { + downloads := recipe.GetGitSourceDownloads() + if len(downloads) == 0 { + return nil, nil + } + mirrors := make(map[string]string, len(downloads)) + for index, download := range downloads { + root, err := p.fetch(ctx, download.GetRepository(), download.GetCommit()) + if err != nil { + return nil, fmt.Errorf("recipe %s git source %d: %w", recipe.GetName(), index, err) + } + mirrors[download.GetMirrorContext()] = root + } + return mirrors, nil +} + +// recipeMissesGitSourceMirrors reports whether a recipe declares a git source +// that has no fetched mirror under its build-context name. +func recipeMissesGitSourceMirrors(recipe *builderv0.DockerBuildRecipe, contexts map[string]string) bool { + for _, download := range recipe.GetGitSourceDownloads() { + if _, ok := contexts[download.GetMirrorContext()]; !ok { + return true + } + } + return false +} + +// Close removes every mirror the flow fetched. +func (p *gitSourcePrefetch) Close() error { + if p == nil { + return nil + } + p.mu.Lock() + defer p.mu.Unlock() + if p.root == "" { + return nil + } + err := os.RemoveAll(p.root) + p.root = "" + p.fetched = map[[2]string]bool{} + return err +} + +// gitSourcePrefetchEnv is the host environment, so the host's git configuration +// decides how a repository is reached, as goPrefetchEnv leaves it to decide how +// a module is. It imposes three things: no prompt for a credential the host +// lacks (the fetch fails instead of hanging), gitPrefetchConfig, and no +// variable that would point git at a repository other than the mirror. It sets +// no GIT_ALLOW_PROTOCOL: git checks that after the host's insteadOf rewrites, so +// it would refuse a host rewrite to a local path; the declared URL itself is +// https or ssh, which the recipe's validation enforces. +func gitSourcePrefetchEnv(base []string) []string { + env := make([]string, 0, len(base)+1) + for _, entry := range base { + switch key, _, _ := strings.Cut(entry, "="); key { + case "GIT_TERMINAL_PROMPT", "GIT_DIR", "GIT_WORK_TREE", "GIT_COMMON_DIR", "GIT_INDEX_FILE", + "GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES", "GIT_NAMESPACE": + continue + } + env = append(env, entry) + } + return withGitPrefetchConfig(append(env, "GIT_TERMINAL_PROMPT=0")) +} + +var errGitMissing = errors.New("a recipe declares git sources, which the CLI fetches with the host's git, and `git` is not on PATH") + +func runGitCommand(ctx context.Context, env []string, args ...string) error { + gitBinary, err := exec.LookPath("git") + if err != nil { + return errGitMissing + } + command := exec.CommandContext(ctx, gitBinary, args...) + command.Env = env + var output bytes.Buffer + command.Stdout = &output + command.Stderr = &output + if err := command.Run(); err != nil { + return fmt.Errorf("git %s: %w\n%s", redact.URLCredentials(strings.Join(args, " ")), err, redact.URLCredentials(lastLines(output.String(), 40))) + } + return nil +} diff --git a/pkg/orchestration/build_git_sources_integration_test.go b/pkg/orchestration/build_git_sources_integration_test.go new file mode 100644 index 000000000..f9671127a --- /dev/null +++ b/pkg/orchestration/build_git_sources_integration_test.go @@ -0,0 +1,157 @@ +//go:build integration + +package orchestration + +import ( + "context" + "fmt" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + coreservices "github.com/codefly-dev/core/agents/services" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// lockedGitSource is a repository URL nothing resolves: only the host's git +// configuration reaches it, standing in for a private repository whose +// credential the host holds and the image build does not. +const lockedGitSource = "https://git.invalid/team/lib.git" + +// lockedGitRepository writes a bare repository with two commits and returns it +// with the first, which is not a branch tip, so fetching it by id is a request +// for an unadvertised commit. +func lockedGitRepository(t *testing.T) (string, string) { + t.Helper() + work := t.TempDir() + git := func(dir string, args ...string) string { + t.Helper() + command := exec.Command("git", args...) + command.Dir = dir + command.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null", + "GIT_AUTHOR_NAME=codefly", "GIT_AUTHOR_EMAIL=codefly@example.com", + "GIT_COMMITTER_NAME=codefly", "GIT_COMMITTER_EMAIL=codefly@example.com") + out, err := command.CombinedOutput() + require.NoError(t, err, "git %s: %s", strings.Join(args, " "), out) + return strings.TrimSpace(string(out)) + } + git(work, "init", "-q", "-b", "main", ".") + git(work, "commit", "-q", "--allow-empty", "-m", "the locked commit") + locked := git(work, "rev-parse", "HEAD") + git(work, "commit", "-q", "--allow-empty", "-m", "a later commit") + repository := filepath.Join(t.TempDir(), "lib.git") + git(work, "clone", "-q", "--bare", work, repository) + return repository, locked +} + +// reachLockedSourceThroughGit points the host's git at the repository on disk, +// as a developer's or CI job's insteadOf points it at a private repository. +// extra is further host configuration. +func reachLockedSourceThroughGit(t *testing.T, repository, extra string) { + t.Helper() + configuration := filepath.Join(t.TempDir(), "gitconfig") + require.NoError(t, os.WriteFile(configuration, []byte(fmt.Sprintf( + "[url \"file://%s\"]\n\tinsteadOf = %s\n%s", repository, lockedGitSource, extra)), 0o644)) + t.Setenv("GIT_CONFIG_GLOBAL", configuration) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") +} + +// TestALockedGitSourceIsMirroredWithTheHostsGitConfiguration fetches a commit +// only the host's git configuration reaches, by id and, from a host that +// refuses an unadvertised commit (protocol v0), through the fallback, and +// checks the mirror holds it where a build looks for it and is handed to the +// build as the declared context. +func TestALockedGitSourceIsMirroredWithTheHostsGitConfiguration(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("the prefetch runs the host git") + } + for name, extra := range map[string]string{ + "by commit id": "", + "through the branches and tags": "[protocol]\n\tversion = 0\n", + } { + t.Run(name, func(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) + defer cancel() + repository, locked := lockedGitRepository(t) + reachLockedSourceThroughGit(t, repository, extra) + + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { require.NoError(t, prefetch.Close()) }) + mirrors, err := prefetch.mirrorsFor(ctx, &builderv0.DockerBuildRecipe{Name: "app", GitSourceDownloads: []*builderv0.GitSourceDownload{ + {Repository: lockedGitSource, Commit: locked, MirrorContext: "gitsources"}, + }}) + require.NoError(t, err) + + mirror := filepath.Join(mirrors["gitsources"], coreservices.GitSourceMirrorPath(lockedGitSource)) + out, err := exec.Command("git", "--git-dir="+mirror, "rev-parse", "--verify", coreservices.GitSourceMirrorRef(locked)).CombinedOutput() + require.NoError(t, err, string(out)) + require.Equal(t, locked, strings.TrimSpace(string(out))) + + // Only the fallback brings the branches along. + branch := exec.Command("git", "--git-dir="+mirror, "rev-parse", "--verify", "-q", "refs/heads/main").Run() + require.Equal(t, extra != "", branch == nil, "the fetch path taken") + + args := privateModuleBuild{}.withProxies(mirrors).buildxArgs(nil) + require.Equal(t, []string{"--build-context", "gitsources=" + mirrors["gitsources"]}, args) + }) + } +} + +// TestAGitSourceBuildNeedsNoCredentialWithThePrefetchedMirror is the end-to-end +// claim: with no network, the image build fetches the locked commit from the +// mirror context the way a package manager does (the locked URL rewritten to +// the mirror), and without the context the same build fails. +func TestAGitSourceBuildNeedsNoCredentialWithThePrefetchedMirror(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("the prefetch runs the host git") + } + if err := exec.Command("docker", "info").Run(); err != nil { + t.Skip("docker is not available") + } + ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) + defer cancel() + repository, locked := lockedGitRepository(t) + reachLockedSourceThroughGit(t, repository, "") + + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { require.NoError(t, prefetch.Close()) }) + recipe := &builderv0.DockerBuildRecipe{ + Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "codefly-git-source-prefetch-test:latest", + Platforms: []string{"linux/" + goArch(t)}, + GitSourceDownloads: []*builderv0.GitSourceDownload{{Repository: lockedGitSource, Commit: locked, MirrorContext: "gitsources"}}, + } + mirrors, err := prefetch.mirrorsFor(ctx, recipe) + require.NoError(t, err) + + contextDir := t.TempDir() + dockerfile := fmt.Sprintf(`FROM scratch AS gitsources + +FROM alpine:3.22 +RUN apk add --no-cache git +RUN --network=none --mount=type=bind,from=gitsources,target=/gitsources \ + git init -q /src && \ + git -C /src -c safe.directory='*' -c url.file:///gitsources/%s.insteadOf=%s \ + fetch -q %s +%s:refs/commit/%s +`, coreservices.GitSourceMirrorPath(lockedGitSource), lockedGitSource, lockedGitSource, locked, locked) + require.NoError(t, os.WriteFile(filepath.Join(contextDir, "Dockerfile"), []byte(dockerfile), 0o644)) + + build := func(private privateModuleBuild) (string, error) { + args, err := cachedBuildxArgs(recipe, filepath.Join(contextDir, "Dockerfile"), contextDir, false, false, "", "", nil, private) + require.NoError(t, err) + require.NotContains(t, args, "--secret", "no credential reaches the build") + out, err := exec.CommandContext(ctx, "docker", args...).CombinedOutput() + return string(out), err + } + t.Cleanup(func() { _ = exec.Command("docker", "image", "rm", "-f", recipe.GetImage()).Run() }) + + out, err := build(privateModuleBuild{}.withProxies(mirrors)) + require.NoError(t, err, "the build must fetch the locked commit from the mirror:\n%s", out) + + out, err = build(privateModuleBuild{}) + require.Error(t, err, "without the mirror the build has no route to the source:\n%s", out) + require.Contains(t, out, coreservices.GitSourceMirrorPath(lockedGitSource), "it failed at the fetch from the empty mirror context") +} diff --git a/pkg/orchestration/build_git_sources_test.go b/pkg/orchestration/build_git_sources_test.go new file mode 100644 index 000000000..3b3126453 --- /dev/null +++ b/pkg/orchestration/build_git_sources_test.go @@ -0,0 +1,175 @@ +package orchestration + +import ( + "context" + "errors" + "os" + "path/filepath" + "slices" + "strings" + "testing" + + coreservices "github.com/codefly-dev/core/agents/services" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +const ( + gitSourceURL = "https://git.example.com/team/lib.git" + commitOne = "1111111111111111111111111111111111111111" + commitTwo = "2222222222222222222222222222222222222222" +) + +// recordingGit stands in for git: it records each call, creates the mirror on +// init, and fails every call whose arguments contain failOn. +func recordingGit(calls *[][]string, failOn string) func(context.Context, []string, ...string) error { + return func(_ context.Context, _ []string, args ...string) error { + *calls = append(*calls, args) + if failOn != "" && slices.Contains(args, failOn) { + return errors.New("exit status 128") + } + if args[0] == "init" { + return os.MkdirAll(args[len(args)-1], 0o755) + } + return nil + } +} + +func gitSourceRecipe(downloads ...*builderv0.GitSourceDownload) *builderv0.DockerBuildRecipe { + return &builderv0.DockerBuildRecipe{Name: "app", GitSourceDownloads: downloads} +} + +// TestGitSourcePrefetchFetchesEachCommitOnceIntoOneMirror pins what a build +// receives: one bare mirror per repository at GitSourceMirrorPath, each declared +// commit fetched by id under GitSourceMirrorRef and verified, once per commit +// however many recipes declare it, all under one directory per flow. +func TestGitSourcePrefetchFetchesEachCommitOnceIntoOneMirror(t *testing.T) { + var calls [][]string + prefetch := newGitSourcePrefetch() + prefetch.run = recordingGit(&calls, "") + t.Cleanup(func() { require.NoError(t, prefetch.Close()) }) + + recipe := gitSourceRecipe( + &builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"}, + &builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitTwo, MirrorContext: "gitsources"}, + ) + mirrors, err := prefetch.mirrorsFor(context.Background(), recipe) + require.NoError(t, err) + root := mirrors["gitsources"] + require.Equal(t, map[string]string{"gitsources": root}, mirrors) + mirror := filepath.Join(root, coreservices.GitSourceMirrorPath(gitSourceURL)) + gitDir := "--git-dir=" + mirror + require.Equal(t, [][]string{ + {"init", "--quiet", "--bare", mirror}, + {gitDir, "fetch", "--depth=1", "--end-of-options", gitSourceURL, "+" + commitOne + ":" + coreservices.GitSourceMirrorRef(commitOne)}, + {gitDir, "cat-file", "-e", commitOne + "^{commit}"}, + {gitDir, "fetch", "--depth=1", "--end-of-options", gitSourceURL, "+" + commitTwo + ":" + coreservices.GitSourceMirrorRef(commitTwo)}, + {gitDir, "cat-file", "-e", commitTwo + "^{commit}"}, + }, calls) + + calls = nil + again, err := prefetch.mirrorsFor(context.Background(), recipe) + require.NoError(t, err) + require.Equal(t, mirrors, again) + require.Empty(t, calls, "a commit fetched earlier in the flow is not fetched again") + + none, err := prefetch.mirrorsFor(context.Background(), gitSourceRecipe()) + require.NoError(t, err) + require.Nil(t, none) + + require.NoError(t, prefetch.Close()) + require.NoDirExists(t, root, "the mirrors are removed with the flow") +} + +// TestGitSourcePrefetchFallsBackToItsBranchesAndTags covers a host that refuses +// a commit it does not advertise: the commit is reached through the +// repository's branches and tags, then pinned under its mirror ref. +func TestGitSourcePrefetchFallsBackToItsBranchesAndTags(t *testing.T) { + var calls [][]string + prefetch := newGitSourcePrefetch() + prefetch.run = recordingGit(&calls, "--depth=1") + t.Cleanup(func() { _ = prefetch.Close() }) + + root, err := prefetch.fetch(context.Background(), gitSourceURL, commitOne) + require.NoError(t, err) + gitDir := "--git-dir=" + filepath.Join(root, coreservices.GitSourceMirrorPath(gitSourceURL)) + require.Equal(t, [][]string{ + {gitDir, "fetch", "--end-of-options", gitSourceURL, "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*"}, + {gitDir, "update-ref", coreservices.GitSourceMirrorRef(commitOne), commitOne}, + {gitDir, "cat-file", "-e", commitOne + "^{commit}"}, + }, calls[2:]) +} + +func TestGitSourcePrefetchReportsACommitTheSourceLacks(t *testing.T) { + var calls [][]string + prefetch := newGitSourcePrefetch() + prefetch.run = recordingGit(&calls, "cat-file") + t.Cleanup(func() { _ = prefetch.Close() }) + _, err := prefetch.mirrorsFor(context.Background(), gitSourceRecipe(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"})) + require.ErrorContains(t, err, "recipe app git source 0") + require.ErrorContains(t, err, "does not hold commit "+commitOne) +} + +// TestGitSourcePrefetchEnvKeepsHowTheHostReachesRepositories pins the boundary: +// the host's git configuration (an insteadOf carrying a credential) reaches the +// fetch, no prompt can hang it, no maintenance runs in the mirror, nothing +// points git at another repository, and no protocol allow-list overrides the +// host's own rewrites. +func TestGitSourcePrefetchEnvKeepsHowTheHostReachesRepositories(t *testing.T) { + env := gitSourcePrefetchEnv([]string{ + "HOME=/home/dev", "GIT_DIR=/elsewhere/.git", "GIT_TERMINAL_PROMPT=1", + "GIT_CONFIG_COUNT=1", + "GIT_CONFIG_KEY_0=url.https://token@git.example.com/.insteadOf", + "GIT_CONFIG_VALUE_0=https://git.example.com/", + }) + value, _ := envValue(env, "GIT_TERMINAL_PROMPT") + require.Equal(t, "0", value) + _, set := envValue(env, "GIT_DIR") + require.False(t, set) + _, set = envValue(env, "GIT_ALLOW_PROTOCOL") + require.False(t, set) + value, _ = envValue(env, "GIT_CONFIG_KEY_0") + require.True(t, strings.HasPrefix(value, "url.https://token@"), "the host's own entry is kept first") + require.Equal(t, map[string]string{ + "url.https://token@git.example.com/.insteadOf": "https://git.example.com/", + "maintenance.auto": "false", "gc.auto": "0", + }, gitConfigSettings(env)) +} + +func TestGitCommandReportsAMissingGit(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + require.ErrorIs(t, runGitCommand(context.Background(), nil, "version"), errGitMissing) +} + +// TestGitCommandOutputHasNoURLCredentials guards the error the prefetch returns, +// which is logged verbatim: a rewritten remote carrying a token must not reach it. +func TestGitCommandOutputHasNoURLCredentials(t *testing.T) { + err := runGitCommand(context.Background(), os.Environ(), "--git-dir="+t.TempDir(), "fetch", "https://x-access-token:not-a-real-token@git.invalid/team/lib.git") + require.Error(t, err) + require.NotContains(t, err.Error(), "not-a-real-token") +} + +// TestAGitSourceRecipeIsNotBuiltWithoutItsMirrors pins the refusal that keeps a +// declared source from silently falling back to a fetch inside the build. +func TestAGitSourceRecipeIsNotBuiltWithoutItsMirrors(t *testing.T) { + recipe := gitSourceRecipe(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"}) + require.True(t, recipeMissesGitSourceMirrors(recipe, nil)) + require.True(t, recipeMissesGitSourceMirrors(recipe, map[string]string{"gomodproxy": "/p"})) + require.False(t, recipeMissesGitSourceMirrors(recipe, map[string]string{"gitsources": "/m"})) + require.False(t, recipeMissesGitSourceMirrors(gitSourceRecipe(), nil)) +} + +// TestGitSourceMirrorsNormalizeLikeProxies pins the identity: a mirror path, +// fetched or not yet, reads as the placeholder, so the reuse decision can +// precede the fetch, and the mirror is passed as a named build context. +func TestGitSourceMirrorsNormalizeLikeProxies(t *testing.T) { + recipe := gitSourceRecipe(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"}) + none := resolvePrivateModuleBuild(func(string) (string, bool) { return "", false }) + normalized := func(contexts map[string]string) []string { + args := append([]string{"buildx", "build"}, none.withProxies(contexts).buildxArgs(nil)...) + return normalizedBuildxArgs(append(args, "/ws/api"), "/ws/api", "/ws/api/Dockerfile", contexts) + } + fetched := map[string]string{"gitsources": "/tmp/codefly-git-sources-1"} + require.Contains(t, none.withProxies(fetched).buildxArgs(nil), "gitsources=/tmp/codefly-git-sources-1") + require.Equal(t, normalized(fetched), normalized(placeholderProxies(recipe))) +} diff --git a/pkg/orchestration/build_go_modules.go b/pkg/orchestration/build_go_modules.go index 5f86aa829..46c00940b 100644 --- a/pkg/orchestration/build_go_modules.go +++ b/pkg/orchestration/build_go_modules.go @@ -227,13 +227,26 @@ func (p *goModulePrefetch) Close() error { // repaired (a downloaded toolchain ignores -modcacherw; removeTree covers it). // The one thing it does impose on git is gitPrefetchConfig. func goPrefetchEnv(base []string, modCache string) []string { + env := make([]string, 0, len(base)+3) + for _, entry := range base { + switch key, _, _ := strings.Cut(entry, "="); key { + case "GOMODCACHE", "GOFLAGS", "GOWORK": + continue + } + env = append(env, entry) + } + env = append(env, "GOMODCACHE="+modCache, "GOFLAGS=-mod=readonly -modcacherw", "GOWORK=off") + return withGitPrefetchConfig(env) +} + +// withGitPrefetchConfig is base with gitPrefetchConfig appended after the git +// configuration entries the host already passes in the environment. +func withGitPrefetchConfig(base []string) []string { configured := hostGitConfigCount(base) - env := make([]string, 0, len(base)+4+2*len(gitPrefetchConfig)) + env := make([]string, 0, len(base)+1+2*len(gitPrefetchConfig)) for _, entry := range base { key, _, _ := strings.Cut(entry, "=") switch { - case key == "GOMODCACHE", key == "GOFLAGS", key == "GOWORK": - continue case key == "GIT_CONFIG_COUNT": // Re-emitted below with the prefetch's own entries counted in. continue @@ -245,7 +258,6 @@ func goPrefetchEnv(base []string, modCache string) []string { } env = append(env, entry) } - env = append(env, "GOMODCACHE="+modCache, "GOFLAGS=-mod=readonly -modcacherw", "GOWORK=off") for _, setting := range gitPrefetchConfig { env = append(env, fmt.Sprintf("GIT_CONFIG_KEY_%d=%s", configured, setting[0]), diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go index 0b2c11fd9..805b9a27a 100644 --- a/pkg/orchestration/build_plan.go +++ b/pkg/orchestration/build_plan.go @@ -199,20 +199,23 @@ func (b *Builder) decideImageReuse( return decision, nil } -// placeholderProxies stands in for the Go module proxies a recipe declares -// before they are fetched. normalizedBuildxArgs rewrites every fetched proxy +// placeholderProxies stands in for the Go module proxies and git source mirrors +// a recipe declares before they are fetched. normalizedBuildxArgs rewrites every fetched proxy // path to this same placeholder, so an identity computed over these equals the // one computed over the fetched paths — which is what lets the reuse decision // precede the fetch. func placeholderProxies(recipe *builderv0.DockerBuildRecipe) map[string]string { downloads := recipe.GetGoModuleDownloads() - if len(downloads) == 0 { + if len(downloads) == 0 && len(recipe.GetGitSourceDownloads()) == 0 { return nil } - proxies := make(map[string]string, len(downloads)) + proxies := make(map[string]string, len(downloads)+len(recipe.GetGitSourceDownloads())) for _, download := range downloads { proxies[download.GetProxyContext()] = proxyPlaceholder } + for _, download := range recipe.GetGitSourceDownloads() { + proxies[download.GetMirrorContext()] = proxyPlaceholder + } return proxies } @@ -296,7 +299,7 @@ func (b *Builder) buildRecipe( plan *builderv0.DockerBuildPlan, recipe *builderv0.DockerBuildRecipe, shouldPush bool, - goModuleProxies map[string]string, + prefetched map[string]string, reuse *imageReuse, ) error { recipe, err := b.recipeAsBuilt(recipe, shouldPush) @@ -332,9 +335,13 @@ func (b *Builder) buildRecipe( // A recipe declaring Go module downloads is only built with the proxies // its plan phase fetched; building it without them would fall back to // fetching inside the build, which is what declaring them removes. - if recipeMissesGoModuleProxies(recipe, goModuleProxies) { + if recipeMissesGoModuleProxies(recipe, prefetched) { return w.NewError("recipe %s of %s declares Go module downloads that were not fetched before the build", recipe.GetName(), b.instance.Unique()) } + // The same for a recipe declaring git sources and their mirrors. + if recipeMissesGitSourceMirrors(recipe, prefetched) { + return w.NewError("recipe %s of %s declares git sources that were not fetched before the build", recipe.GetName(), b.instance.Unique()) + } contextDir, err := recipeContext(contextRoot, recipe) if err != nil { @@ -355,7 +362,7 @@ func (b *Builder) buildRecipe( // A service that imports a private Go module reads it from the proxy the // plan phase fetched; GOPRIVATE, a list of module paths and not a // credential, still reaches recipes that declare it as an ARG. - private := resolvePrivateModuleBuild(os.LookupEnv).withProxies(goModuleProxies) + private := resolvePrivateModuleBuild(os.LookupEnv).withProxies(prefetched) // Where this environment reads a mirrored registry from. It reaches the // build through the builder's buildkitd configuration and the base-image diff --git a/pkg/orchestration/build_private_modules.go b/pkg/orchestration/build_private_modules.go index 57210a54c..629ac0529 100644 --- a/pkg/orchestration/build_private_modules.go +++ b/pkg/orchestration/build_private_modules.go @@ -20,8 +20,8 @@ type privateModuleBuild struct { // GoPrivate is the host's GOPRIVATE, empty when unset. It names module // paths, not credentials. GoPrivate string - // Proxies maps each Go module proxy build context the recipe declares to - // the prefetched directory supplied under that name. + // Proxies maps each Go module proxy and git source mirror build context the + // recipe declares to the prefetched directory supplied under that name. Proxies map[string]string } diff --git a/pkg/orchestration/builder.go b/pkg/orchestration/builder.go index d337c3379..65a944fd9 100644 --- a/pkg/orchestration/builder.go +++ b/pkg/orchestration/builder.go @@ -4,6 +4,7 @@ import ( "context" "encoding/json" "fmt" + "maps" "os" "os/exec" "regexp" @@ -280,13 +281,14 @@ func (b *Builder) buildRecipeRoot() (string, error) { } // plannedBuild is one service's emitted build: where its recipe lives, the -// agent's answer, and the Go module proxies prefetched for each recipe. +// agent's answer, and the Go module proxies and git source mirrors prefetched +// for each recipe. type plannedBuild struct { recipeRoot string outputDir string response *builderv0.BuildResponse - // proxies maps a recipe name to its prefetched Go module proxies, keyed by - // the build-context name the recipe declares. + // proxies maps a recipe name to its prefetched Go module proxies and git + // source mirrors, keyed by the build-context name the recipe declares. proxies map[string]map[string]string // reuse maps a recipe name to the reuse decision Plan took for it, for the // recipes it decided before fetching; Build adopts or builds from it. @@ -358,7 +360,7 @@ func (b *Builder) Plan(ctx context.Context) (*OutputProperty, error) { } contextRoot := recipeContextRoot(b.instance.Service.Dir(), outputDir, plan) for _, recipe := range plan.GetRecipes() { - if len(recipe.GetGoModuleDownloads()) == 0 { + if len(recipe.GetGoModuleDownloads()) == 0 && len(recipe.GetGitSourceDownloads()) == 0 { continue } // Decide reuse before fetching, on the recipe as it will be built: an @@ -377,7 +379,7 @@ func (b *Builder) Plan(ctx context.Context) (*OutputProperty, error) { } planned.reuse[recipe.GetName()] = reuse if reuse.entry != nil { - w.Info("no image input changed; the Go modules this recipe declares are not fetched", + w.Info("no image input changed; the Go modules and git sources this recipe declares are not fetched", wool.Field("recipe", recipe.GetName()), wool.Field("image", reuse.entry.Image)) continue } @@ -385,7 +387,9 @@ func (b *Builder) Plan(ctx context.Context) (*OutputProperty, error) { if err != nil { return nil, w.Wrapf(err, "cannot resolve build context for recipe %s of %s", recipe.GetName(), b.instance.Unique()) } - w.Info("fetching Go modules before any image build", wool.Field("recipe", recipe.GetName())) + if len(recipe.GetGoModuleDownloads()) > 0 { + w.Info("fetching Go modules before any image build", wool.Field("recipe", recipe.GetName())) + } proxies, err := b.world.goModules().proxiesFor(ctx, contextDir, recipe) if err != nil { // What names the cause of a failed fetch is the go tool's own @@ -398,6 +402,21 @@ func (b *Builder) Plan(ctx context.Context) (*OutputProperty, error) { wool.Field("recipe", recipe.GetName()), wool.ErrField(err)) return nil, w.Wrapf(err, "cannot fetch the Go modules of %s", b.instance.Unique()) } + // The same phase and route for a dependency locked to a git commit: + // the host's git fetches it, the build reads a mirror context. + if len(recipe.GetGitSourceDownloads()) > 0 { + w.Info("fetching git sources before any image build", wool.Field("recipe", recipe.GetName())) + mirrors, err := b.world.gitSources().mirrorsFor(ctx, recipe) + if err != nil { + w.Warn("cannot fetch the git sources this recipe declares", + wool.Field("recipe", recipe.GetName()), wool.ErrField(err)) + return nil, w.Wrapf(err, "cannot fetch the git sources of %s", b.instance.Unique()) + } + if proxies == nil { + proxies = make(map[string]string, len(mirrors)) + } + maps.Copy(proxies, mirrors) + } if planned.proxies == nil { planned.proxies = map[string]map[string]string{} } diff --git a/pkg/orchestration/flow.go b/pkg/orchestration/flow.go index ca03a3c79..7cab602ed 100644 --- a/pkg/orchestration/flow.go +++ b/pkg/orchestration/flow.go @@ -271,6 +271,9 @@ type World struct { // fetched once on the host before any image build (see goModulePrefetch). // It is removed when the flow stops. goModulePrefetch *goModulePrefetch + // gitSourcePrefetch holds the git source mirrors this flow's recipes + // declared, fetched the same way (see gitSourcePrefetch). + gitSourcePrefetch *gitSourcePrefetch // hostArchitecture reads the container engine's architecture, which a // local cluster's nodes run; nil selects `docker info`. @@ -1599,6 +1602,9 @@ func (flow *Flow) Stop() error { if err := flow.world.goModulePrefetch.Close(); err != nil { wool.Get(context.Background()).In("flow.Stop").Warn("cannot remove the Go module prefetch", wool.ErrField(err)) } + if err := flow.world.gitSourcePrefetch.Close(); err != nil { + wool.Get(context.Background()).In("flow.Stop").Warn("cannot remove the git source prefetch", wool.ErrField(err)) + } } // Builder-only flows have no Runtime runner to stop. Their service-scoped // agent connections are closed by the caller after this lifecycle hook. @@ -2544,6 +2550,14 @@ func (world *World) goModules() *goModulePrefetch { return world.goModulePrefetch } +// gitSources is the flow's git source prefetch, created on first use. +func (world *World) gitSources() *gitSourcePrefetch { + if world.gitSourcePrefetch == nil { + world.gitSourcePrefetch = newGitSourcePrefetch() + } + return world.gitSourcePrefetch +} + // imagePlatforms is the environment's image platforms, resolved once per flow. func (world *World) imagePlatforms() ([]string, error) { if world.platforms != nil { diff --git a/pkg/orchestration/image_cache.go b/pkg/orchestration/image_cache.go index 61d17cfa8..21f86fdf7 100644 --- a/pkg/orchestration/image_cache.go +++ b/pkg/orchestration/image_cache.go @@ -68,7 +68,10 @@ import ( // version missing from go.sum fail the fetch instead of being written in. A // filesystem `replace` resolves to a directory rather than the proxy, and the // build can only read that directory if it is in the context, where it is hashed -// like any other file. +// like any other file. A recipe declaring git sources is handed a mirror +// directory the same way, normalized out of the key the same way; the +// repositories and commits it holds are the declaration's, which the recipe +// tree digest covers. // // A base image is resolved, not taken from its name. A recipe that says // `FROM golang:1.25` is built from whatever manifest that tag points at when the diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index bd8075b08..096badc10 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/pkg/orchestration/image_cache_test.go @@ -114,6 +114,8 @@ type imageCachePeer struct { goModules bool // image is the recipe's image; empty means example.test/app:v1. image string + // gitSources makes the recipe declare a locked git commit for prefetching. + gitSources bool } func (*imageCachePeer) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { @@ -137,6 +139,9 @@ func (peer *imageCachePeer) Build(_ context.Context, request *builderv0.BuildReq if peer.goModules { options = append(options, coreservices.WithGoModuleDownloads(&builderv0.GoModuleDownload{ModuleRoot: "code", ProxyContext: "gomodproxy"})) } + if peer.gitSources { + options = append(options, coreservices.WithGitSourceDownloads(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"})) + } plan, err := coreservices.SingleImageBuildPlan(output, peer.imageName(), coreservices.RecipeBuildPlatforms(), []string{"Dockerfile", "dockerignore"}, options...) if err != nil { return nil, err @@ -159,6 +164,8 @@ type imageCacheHarness struct { rebuild bool // goCalls records every go command the prefetch of each build runs. goCalls []goCall + // gitCalls records every git command the git source prefetch runs. + gitCalls [][]string } func newImageCacheHarness(t *testing.T, push, registryServes bool) *imageCacheHarness { @@ -232,6 +239,10 @@ func (harness *imageCacheHarness) build(t *testing.T) string { prefetch.run = recordingGo(&harness.goCalls, nil) world.goModulePrefetch = prefetch t.Cleanup(func() { require.NoError(t, prefetch.Close()) }) + gitPrefetch := newGitSourcePrefetch() + gitPrefetch.run = recordingGit(&harness.gitCalls, "") + world.gitSourcePrefetch = gitPrefetch + t.Cleanup(func() { require.NoError(t, gitPrefetch.Close()) }) build, err := harness.run(ctx, instance, world) require.NoError(t, err) if harness.push { @@ -384,6 +395,25 @@ func TestAnUpToDateImageFetchesNoGoModules(t *testing.T) { require.Len(t, harness.goCalls, 4, "a changed input is fetched before it is built") } +// TestAnUpToDateImageFetchesNoGitSources is the same plan phase for a declared +// git source: fetched before the first build and handed to it as the mirror +// context, not fetched for an image already built from these inputs. +func TestAnUpToDateImageFetchesNoGitSources(t *testing.T) { + harness := newImageCacheHarness(t, true, true) + harness.peer.gitSources = true + require.NoError(t, os.WriteFile(filepath.Join(harness.docker.dir, "hook"), []byte(`printf '%s\n' "$@" > "$STATE/args"`+"\n"), 0o644)) + first := harness.build(t) + require.Equal(t, 1, harness.docker.calls(t, "build")) + require.Len(t, harness.gitCalls, 3, "the first build fetches the declared commit") + args, err := os.ReadFile(filepath.Join(harness.docker.dir, "args")) + require.NoError(t, err) + require.Contains(t, string(args), "--build-context\ngitsources=") + + require.Equal(t, first, harness.build(t)) + require.Equal(t, 1, harness.docker.calls(t, "build")) + require.Len(t, harness.gitCalls, 3, "an image built from these inputs needs no fetch") +} + // TestPlaceholderProxiesNormalizeLikeFetchedPaths pins what lets the reuse // decision precede the fetch: an identity reads a declared proxy as one // placeholder whether or not its path exists yet, so the key a build recorded From 230ac1060b994f2e74218365abf710f8a78b3e83 Mon Sep 17 00:00:00 2001 From: Saurabh Maheshwari <242053479+saurabhmaheshwari-obin@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:28:27 -0400 Subject: [PATCH 2/6] fix(build): pin each git source mirror to the exact locked commit and keep host state out of it - Create each mirror with the object format of its commit id (SHA-1 or SHA-256) and refuse a repository declared with both. - Verify through the mirror ref that it resolves to exactly the locked id, never to a ref spelled like one. - Create mirrors from an empty template and drop GIT_TEMPLATE_DIR, so a host template's hooks or configuration never reach the build context. - Drop GIT_TRACE*, GIT_CURL_VERBOSE and GIT_TRACE2* from the fetch: a trace prints a credential helper's command line into the reported error. - Set GCM_INTERACTIVE=never unless the host chose, as the go tool does. - Run git in its own process group, kill the group on cancel and bound the wait, so a credential helper cannot hold the fetch past its deadline. - The build test now runs a package manager's own fetch by commit id against the shallow mirror, offline, and reads the locked file, in a pinned git image; the plan-phase test covers a changed commit. - Name the per-recipe map and log field for what they carry: prefetched contexts, Go proxies and git mirrors. Co-Authored-By: Claude Opus 5.5 --- pkg/orchestration/build_git_sources.go | 123 +++++++--- .../build_git_sources_integration_test.go | 49 ++-- pkg/orchestration/build_git_sources_test.go | 225 ++++++++++++++++-- pkg/orchestration/build_plan.go | 6 +- pkg/orchestration/build_private_modules.go | 4 +- pkg/orchestration/builder.go | 12 +- pkg/orchestration/image_cache_test.go | 19 +- 7 files changed, 336 insertions(+), 102 deletions(-) diff --git a/pkg/orchestration/build_git_sources.go b/pkg/orchestration/build_git_sources.go index eb26aa997..ef11dd27c 100644 --- a/pkg/orchestration/build_git_sources.go +++ b/pkg/orchestration/build_git_sources.go @@ -10,6 +10,8 @@ import ( "path/filepath" "strings" "sync" + "syscall" + "time" coreservices "github.com/codefly-dev/core/agents/services" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -25,23 +27,31 @@ import ( // // The fetch runs with the host's own git and environment, so it authenticates // exactly as the developer's or the CI job's git does (insteadOf rewrites, a -// credential helper, an ssh agent); nothing secret reaches the build. Each -// repository has one bare mirror at GitSourceMirrorPath(repository), holding -// each declared commit under GitSourceMirrorRef(commit), which is the ref a -// fetch by commit id writes, so a build fetching that commit from the mirror -// finds it. Every recipe of the flow is handed the one directory holding all the -// mirrors, as every Go recipe is handed one module proxy: a build fetches only -// the commits its own lockfile names, so another service's mirrors are inert. +// credential helper, an ssh agent); nothing secret reaches the build. Like the +// go tool's fetches, which run in repositories under the module cache, it runs +// in a repository of its own, so the configuration of a checkout the CLI runs +// from is not consulted; global and environment configuration is. +// +// Each repository has one bare mirror at GitSourceMirrorPath(repository), +// holding each declared commit under GitSourceMirrorRef(commit). The mirror is +// shallow by design: it carries the locked commits and nothing else, which +// serves a consumer that fetches by commit id, as a package manager installing +// a locked git dependency does (`git fetch +:refs/commit/`). +// Every recipe of the flow is handed the one directory holding all the mirrors, +// as every Go recipe is handed one module proxy: a build fetches only the +// commits its own lockfile names, so another service's mirrors are inert. type gitSourcePrefetch struct { mu sync.Mutex root string fetched map[[2]string]bool - // run executes git; a test substitutes it. - run func(ctx context.Context, env []string, args ...string) error + // formats is the object format each repository's mirror was created with. + formats map[string]string + // run executes git and returns its standard output; a test substitutes it. + run func(ctx context.Context, env []string, args ...string) (string, error) } func newGitSourcePrefetch() *gitSourcePrefetch { - return &gitSourcePrefetch{fetched: map[[2]string]bool{}, run: runGitCommand} + return &gitSourcePrefetch{fetched: map[[2]string]bool{}, formats: map[string]string{}, run: runGitCommand} } // fetch places commit of repository in its mirror and returns the directory @@ -53,6 +63,14 @@ func (p *gitSourcePrefetch) fetch(ctx context.Context, repository, commit string if p.fetched[[2]string{repository, commit}] { return p.root, nil } + // A 40-digit id is SHA-1 and a 64-digit one SHA-256; a repository has one. + format := "sha1" + if len(commit) == 64 { + format = "sha256" + } + if known, ok := p.formats[repository]; ok && known != format { + return "", fmt.Errorf("a git source is declared with both %s and %s commit ids", known, format) + } if p.root == "" { root, err := os.MkdirTemp("", "codefly-git-sources-*") if err != nil { @@ -62,34 +80,43 @@ func (p *gitSourcePrefetch) fetch(ctx context.Context, repository, commit string } mirror := filepath.Join(p.root, coreservices.GitSourceMirrorPath(repository)) env := gitSourcePrefetchEnv(os.Environ()) - if _, err := os.Stat(mirror); err != nil { - if err := p.run(ctx, env, "init", "--quiet", "--bare", mirror); err != nil { + if _, known := p.formats[repository]; !known { + // An empty template: a host template's hooks or configuration would + // otherwise be copied into the mirror, and so into the build context. + if _, err := p.run(ctx, env, "init", "--quiet", "--bare", "--template=", "--object-format="+format, mirror); err != nil { return "", fmt.Errorf("cannot create the git source mirror: %w", err) } + p.formats[repository] = format } - git := func(args ...string) error { + git := func(args ...string) (string, error) { return p.run(ctx, env, append([]string{"--git-dir=" + mirror}, args...)...) } ref := coreservices.GitSourceMirrorRef(commit) // A host that serves a commit by id gives just that commit. One that refuses // an unadvertised commit (protocol v0 without allowAnySHA1InWant) gives it // with its branches and tags, which is how any git client then reaches it. - if err := git("fetch", "--depth=1", "--end-of-options", repository, "+"+commit+":"+ref); err != nil { + if _, err := git("fetch", "--depth=1", "--end-of-options", repository, "+"+commit+":"+ref); err != nil { full := []string{"fetch"} if _, shallow := os.Stat(filepath.Join(mirror, "shallow")); shallow == nil { full = append(full, "--unshallow") } full = append(full, "--end-of-options", repository, "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*") - if fullErr := git(full...); fullErr != nil { + if _, fullErr := git(full...); fullErr != nil { return "", fmt.Errorf("cannot fetch commit %s of a declared git source: %w", commit, errors.Join(err, fullErr)) } - if err := git("update-ref", ref, commit); err != nil { + if _, err := git("update-ref", ref, commit); err != nil { return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) } } - if err := git("cat-file", "-e", commit+"^{commit}"); err != nil { + // The mirror ref must name exactly the locked commit: a branch spelled like + // a commit id is never taken for it. + resolved, err := git("rev-parse", "--verify", "--end-of-options", ref+"^{commit}") + if err != nil { return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) } + if resolved != commit { + return "", fmt.Errorf("the declared git source resolves commit %s to %s", commit, resolved) + } p.fetched[[2]string{repository, commit}] = true return p.root, nil } @@ -136,44 +163,74 @@ func (p *gitSourcePrefetch) Close() error { err := os.RemoveAll(p.root) p.root = "" p.fetched = map[[2]string]bool{} + p.formats = map[string]string{} return err } // gitSourcePrefetchEnv is the host environment, so the host's git configuration // decides how a repository is reached, as goPrefetchEnv leaves it to decide how -// a module is. It imposes three things: no prompt for a credential the host -// lacks (the fetch fails instead of hanging), gitPrefetchConfig, and no -// variable that would point git at a repository other than the mirror. It sets -// no GIT_ALLOW_PROTOCOL: git checks that after the host's insteadOf rewrites, so -// it would refuse a host rewrite to a local path; the declared URL itself is -// https or ssh, which the recipe's validation enforces. +// a module is. What it imposes: +// - no prompt for a credential the host lacks (GIT_TERMINAL_PROMPT=0, and +// GCM_INTERACTIVE=never unless the host chose, as the go tool does), so the +// fetch fails instead of hanging; +// - gitPrefetchConfig; +// - no variable pointing git at another repository or template; +// - no tracing: a trace prints a credential helper's command line and +// protocol traffic into the output this reports. +// +// It sets no GIT_ALLOW_PROTOCOL: git checks that after the host's insteadOf +// rewrites, so it would refuse a host rewrite to a local path; the declared URL +// itself is https or ssh, which the recipe's validation enforces. func gitSourcePrefetchEnv(base []string) []string { - env := make([]string, 0, len(base)+1) + env := make([]string, 0, len(base)+2) + interactive := false for _, entry := range base { - switch key, _, _ := strings.Cut(entry, "="); key { + key, value, _ := strings.Cut(entry, "=") + switch key { case "GIT_TERMINAL_PROMPT", "GIT_DIR", "GIT_WORK_TREE", "GIT_COMMON_DIR", "GIT_INDEX_FILE", - "GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES", "GIT_NAMESPACE": + "GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES", "GIT_NAMESPACE", + "GIT_TEMPLATE_DIR", "GIT_CURL_VERBOSE": + continue + case "GCM_INTERACTIVE": + interactive = value != "" + } + if strings.HasPrefix(key, "GIT_TRACE") { continue } env = append(env, entry) } - return withGitPrefetchConfig(append(env, "GIT_TERMINAL_PROMPT=0")) + env = append(env, "GIT_TERMINAL_PROMPT=0") + if !interactive { + env = append(env, "GCM_INTERACTIVE=never") + } + return withGitPrefetchConfig(env) } var errGitMissing = errors.New("a recipe declares git sources, which the CLI fetches with the host's git, and `git` is not on PATH") -func runGitCommand(ctx context.Context, env []string, args ...string) error { +// gitWaitDelay bounds how long a cancelled git may hold its output open: a +// credential helper or askpass it started can outlive it. +const gitWaitDelay = 5 * time.Second + +// runGitCommand runs git in its own process group, so cancelling the context +// stops git and every helper it started, and returns its trimmed standard +// output. +func runGitCommand(ctx context.Context, env []string, args ...string) (string, error) { gitBinary, err := exec.LookPath("git") if err != nil { - return errGitMissing + return "", errGitMissing } command := exec.CommandContext(ctx, gitBinary, args...) command.Env = env - var output bytes.Buffer - command.Stdout = &output + command.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + command.Cancel = func() error { return syscall.Kill(-command.Process.Pid, syscall.SIGKILL) } + command.WaitDelay = gitWaitDelay + var stdout, output bytes.Buffer + command.Stdout = &stdout command.Stderr = &output if err := command.Run(); err != nil { - return fmt.Errorf("git %s: %w\n%s", redact.URLCredentials(strings.Join(args, " ")), err, redact.URLCredentials(lastLines(output.String(), 40))) + output.Write(stdout.Bytes()) + return "", fmt.Errorf("git %s: %w\n%s", redact.URLCredentials(strings.Join(args, " ")), err, redact.URLCredentials(lastLines(output.String(), 40))) } - return nil + return strings.TrimSpace(stdout.String()), nil } diff --git a/pkg/orchestration/build_git_sources_integration_test.go b/pkg/orchestration/build_git_sources_integration_test.go index f9671127a..41bcd4f90 100644 --- a/pkg/orchestration/build_git_sources_integration_test.go +++ b/pkg/orchestration/build_git_sources_integration_test.go @@ -22,31 +22,8 @@ import ( // credential the host holds and the image build does not. const lockedGitSource = "https://git.invalid/team/lib.git" -// lockedGitRepository writes a bare repository with two commits and returns it -// with the first, which is not a branch tip, so fetching it by id is a request -// for an unadvertised commit. -func lockedGitRepository(t *testing.T) (string, string) { - t.Helper() - work := t.TempDir() - git := func(dir string, args ...string) string { - t.Helper() - command := exec.Command("git", args...) - command.Dir = dir - command.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null", - "GIT_AUTHOR_NAME=codefly", "GIT_AUTHOR_EMAIL=codefly@example.com", - "GIT_COMMITTER_NAME=codefly", "GIT_COMMITTER_EMAIL=codefly@example.com") - out, err := command.CombinedOutput() - require.NoError(t, err, "git %s: %s", strings.Join(args, " "), out) - return strings.TrimSpace(string(out)) - } - git(work, "init", "-q", "-b", "main", ".") - git(work, "commit", "-q", "--allow-empty", "-m", "the locked commit") - locked := git(work, "rev-parse", "HEAD") - git(work, "commit", "-q", "--allow-empty", "-m", "a later commit") - repository := filepath.Join(t.TempDir(), "lib.git") - git(work, "clone", "-q", "--bare", work, repository) - return repository, locked -} +// gitSourceBuildImage is the image the build test fetches in: git, pinned. +const gitSourceBuildImage = "alpine/git:v2.52.0@sha256:4a0e72d49596a1f5d3701aeedafdadc5c0da4062be4657c7bdc4017387f591cc" // reachLockedSourceThroughGit points the host's git at the repository on disk, // as a developer's or CI job's insteadOf points it at a private repository. @@ -55,7 +32,7 @@ func reachLockedSourceThroughGit(t *testing.T, repository, extra string) { t.Helper() configuration := filepath.Join(t.TempDir(), "gitconfig") require.NoError(t, os.WriteFile(configuration, []byte(fmt.Sprintf( - "[url \"file://%s\"]\n\tinsteadOf = %s\n%s", repository, lockedGitSource, extra)), 0o644)) + "[url \"%s\"]\n\tinsteadOf = %s\n%s", repository, lockedGitSource, extra)), 0o644)) t.Setenv("GIT_CONFIG_GLOBAL", configuration) t.Setenv("GIT_CONFIG_NOSYSTEM", "1") } @@ -76,7 +53,7 @@ func TestALockedGitSourceIsMirroredWithTheHostsGitConfiguration(t *testing.T) { t.Run(name, func(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute) defer cancel() - repository, locked := lockedGitRepository(t) + repository, locked := gitSourceRepository(t, "sha1") reachLockedSourceThroughGit(t, repository, extra) prefetch := newGitSourcePrefetch() @@ -114,7 +91,7 @@ func TestAGitSourceBuildNeedsNoCredentialWithThePrefetchedMirror(t *testing.T) { } ctx, cancel := context.WithTimeout(context.Background(), 10*time.Minute) defer cancel() - repository, locked := lockedGitRepository(t) + repository, locked := gitSourceRepository(t, "sha1") reachLockedSourceThroughGit(t, repository, "") prefetch := newGitSourcePrefetch() @@ -128,15 +105,19 @@ func TestAGitSourceBuildNeedsNoCredentialWithThePrefetchedMirror(t *testing.T) { require.NoError(t, err) contextDir := t.TempDir() + // The consumer's own fetch, as a package manager installing a locked git + // dependency runs it (uv: `git fetch --force --update-head-ok + // +:refs/commit/`), against the shallow mirror through an + // insteadOf rewrite, with no network; then the locked file is read. dockerfile := fmt.Sprintf(`FROM scratch AS gitsources -FROM alpine:3.22 -RUN apk add --no-cache git +FROM %s RUN --network=none --mount=type=bind,from=gitsources,target=/gitsources \ - git init -q /src && \ - git -C /src -c safe.directory='*' -c url.file:///gitsources/%s.insteadOf=%s \ - fetch -q %s +%s:refs/commit/%s -`, coreservices.GitSourceMirrorPath(lockedGitSource), lockedGitSource, lockedGitSource, locked, locked) + git init -q /src && cd /src && \ + git -c safe.directory='*' -c url.file:///gitsources/%s.insteadOf=%s \ + fetch --force --update-head-ok %s '+%s:refs/commit/%s' && \ + git checkout -q %s && test "$(cat lib.txt)" = locked +`, gitSourceBuildImage, coreservices.GitSourceMirrorPath(lockedGitSource), lockedGitSource, lockedGitSource, locked, locked, locked) require.NoError(t, os.WriteFile(filepath.Join(contextDir, "Dockerfile"), []byte(dockerfile), 0o644)) build := func(private privateModuleBuild) (string, error) { diff --git a/pkg/orchestration/build_git_sources_test.go b/pkg/orchestration/build_git_sources_test.go index 3b3126453..8d01026c0 100644 --- a/pkg/orchestration/build_git_sources_test.go +++ b/pkg/orchestration/build_git_sources_test.go @@ -3,11 +3,16 @@ package orchestration import ( "context" "errors" + "fmt" + "net/http" + "net/http/httptest" "os" + "os/exec" "path/filepath" "slices" "strings" "testing" + "time" coreservices "github.com/codefly-dev/core/agents/services" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -21,17 +26,22 @@ const ( ) // recordingGit stands in for git: it records each call, creates the mirror on -// init, and fails every call whose arguments contain failOn. -func recordingGit(calls *[][]string, failOn string) func(context.Context, []string, ...string) error { - return func(_ context.Context, _ []string, args ...string) error { +// init, resolves a mirror ref to the commit it names, and fails every call whose +// arguments contain failOn. +func recordingGit(calls *[][]string, failOn string) func(context.Context, []string, ...string) (string, error) { + return func(_ context.Context, _ []string, args ...string) (string, error) { *calls = append(*calls, args) if failOn != "" && slices.Contains(args, failOn) { - return errors.New("exit status 128") + return "", errors.New("exit status 128") } - if args[0] == "init" { - return os.MkdirAll(args[len(args)-1], 0o755) + last := args[len(args)-1] + switch { + case args[0] == "init": + return "", os.MkdirAll(last, 0o755) + case slices.Contains(args, "rev-parse"): + return strings.TrimSuffix(strings.TrimPrefix(last, "refs/commit/"), "^{commit}"), nil } - return nil + return "", nil } } @@ -39,10 +49,15 @@ func gitSourceRecipe(downloads ...*builderv0.GitSourceDownload) *builderv0.Docke return &builderv0.DockerBuildRecipe{Name: "app", GitSourceDownloads: downloads} } +func verifyCall(gitDir, commit string) []string { + return []string{gitDir, "rev-parse", "--verify", "--end-of-options", coreservices.GitSourceMirrorRef(commit) + "^{commit}"} +} + // TestGitSourcePrefetchFetchesEachCommitOnceIntoOneMirror pins what a build -// receives: one bare mirror per repository at GitSourceMirrorPath, each declared -// commit fetched by id under GitSourceMirrorRef and verified, once per commit -// however many recipes declare it, all under one directory per flow. +// receives: one bare mirror per repository at GitSourceMirrorPath, created from +// no template, each declared commit fetched by id under GitSourceMirrorRef and +// verified, once per commit however many recipes declare it, all under one +// directory per flow. func TestGitSourcePrefetchFetchesEachCommitOnceIntoOneMirror(t *testing.T) { var calls [][]string prefetch := newGitSourcePrefetch() @@ -60,11 +75,11 @@ func TestGitSourcePrefetchFetchesEachCommitOnceIntoOneMirror(t *testing.T) { mirror := filepath.Join(root, coreservices.GitSourceMirrorPath(gitSourceURL)) gitDir := "--git-dir=" + mirror require.Equal(t, [][]string{ - {"init", "--quiet", "--bare", mirror}, + {"init", "--quiet", "--bare", "--template=", "--object-format=sha1", mirror}, {gitDir, "fetch", "--depth=1", "--end-of-options", gitSourceURL, "+" + commitOne + ":" + coreservices.GitSourceMirrorRef(commitOne)}, - {gitDir, "cat-file", "-e", commitOne + "^{commit}"}, + verifyCall(gitDir, commitOne), {gitDir, "fetch", "--depth=1", "--end-of-options", gitSourceURL, "+" + commitTwo + ":" + coreservices.GitSourceMirrorRef(commitTwo)}, - {gitDir, "cat-file", "-e", commitTwo + "^{commit}"}, + verifyCall(gitDir, commitTwo), }, calls) calls = nil @@ -96,20 +111,180 @@ func TestGitSourcePrefetchFallsBackToItsBranchesAndTags(t *testing.T) { require.Equal(t, [][]string{ {gitDir, "fetch", "--end-of-options", gitSourceURL, "+refs/heads/*:refs/heads/*", "+refs/tags/*:refs/tags/*"}, {gitDir, "update-ref", coreservices.GitSourceMirrorRef(commitOne), commitOne}, - {gitDir, "cat-file", "-e", commitOne + "^{commit}"}, + verifyCall(gitDir, commitOne), }, calls[2:]) } func TestGitSourcePrefetchReportsACommitTheSourceLacks(t *testing.T) { var calls [][]string prefetch := newGitSourcePrefetch() - prefetch.run = recordingGit(&calls, "cat-file") + prefetch.run = recordingGit(&calls, "rev-parse") t.Cleanup(func() { _ = prefetch.Close() }) _, err := prefetch.mirrorsFor(context.Background(), gitSourceRecipe(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"})) require.ErrorContains(t, err, "recipe app git source 0") require.ErrorContains(t, err, "does not hold commit "+commitOne) } +// TestGitSourcePrefetchRefusesAMirrorRefNamingAnotherCommit pins that the +// mirror ref must resolve to the locked id itself. +func TestGitSourcePrefetchRefusesAMirrorRefNamingAnotherCommit(t *testing.T) { + prefetch := newGitSourcePrefetch() + var calls [][]string + record := recordingGit(&calls, "") + prefetch.run = func(ctx context.Context, env []string, args ...string) (string, error) { + if slices.Contains(args, "rev-parse") { + return commitTwo, nil + } + return record(ctx, env, args...) + } + t.Cleanup(func() { _ = prefetch.Close() }) + _, err := prefetch.fetch(context.Background(), gitSourceURL, commitOne) + require.ErrorContains(t, err, "resolves commit "+commitOne+" to "+commitTwo) +} + +// TestGitSourcePrefetchMatchesTheObjectFormatToTheCommit pins that a 64-digit +// id gets a SHA-256 mirror and that one repository declared with both lengths +// is refused. +func TestGitSourcePrefetchMatchesTheObjectFormatToTheCommit(t *testing.T) { + var calls [][]string + prefetch := newGitSourcePrefetch() + prefetch.run = recordingGit(&calls, "") + t.Cleanup(func() { _ = prefetch.Close() }) + long := strings.Repeat("3", 64) + _, err := prefetch.fetch(context.Background(), gitSourceURL, long) + require.NoError(t, err) + require.Contains(t, calls[0], "--object-format=sha256") + _, err = prefetch.fetch(context.Background(), gitSourceURL, commitOne) + require.ErrorContains(t, err, "both sha256 and sha1") +} + +// gitSourceRepository writes a bare repository in the given object format with +// a file at two commits, and returns its file:// URL and the first commit, +// which is not a branch tip, so fetching it by id asks for an unadvertised +// commit. +func gitSourceRepository(t *testing.T, format string) (string, string) { + t.Helper() + work := t.TempDir() + git := func(args ...string) string { + t.Helper() + command := exec.Command("git", args...) + command.Dir = work + command.Env = append(os.Environ(), "GIT_CONFIG_NOSYSTEM=1", "GIT_CONFIG_GLOBAL=/dev/null", + "GIT_AUTHOR_NAME=codefly", "GIT_AUTHOR_EMAIL=codefly@example.com", + "GIT_COMMITTER_NAME=codefly", "GIT_COMMITTER_EMAIL=codefly@example.com") + out, err := command.CombinedOutput() + require.NoError(t, err, "git %s: %s", strings.Join(args, " "), out) + return strings.TrimSpace(string(out)) + } + git("init", "-q", "-b", "main", "--object-format="+format, ".") + require.NoError(t, os.WriteFile(filepath.Join(work, "lib.txt"), []byte("locked\n"), 0o644)) + git("add", "lib.txt") + git("commit", "-q", "-m", "the locked commit") + locked := git("rev-parse", "HEAD") + require.NoError(t, os.WriteFile(filepath.Join(work, "lib.txt"), []byte("later\n"), 0o644)) + git("commit", "-q", "-am", "a later commit") + repository := filepath.Join(t.TempDir(), "lib.git") + git("clone", "-q", "--bare", work, repository) + return "file://" + repository, locked +} + +// hostGitConfig makes content the host's global git configuration for the test. +func hostGitConfig(t *testing.T, content string) { + t.Helper() + configuration := filepath.Join(t.TempDir(), "gitconfig") + require.NoError(t, os.WriteFile(configuration, []byte(content), 0o644)) + t.Setenv("GIT_CONFIG_GLOBAL", configuration) + t.Setenv("GIT_CONFIG_NOSYSTEM", "1") +} + +func requireGit(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("the prefetch runs the host git") + } +} + +// TestGitSourcePrefetchMirrorsASHA256Repository fetches a SHA-256 commit into +// a mirror of the same format. +func TestGitSourcePrefetchMirrorsASHA256Repository(t *testing.T) { + requireGit(t) + hostGitConfig(t, "") + repository, locked := gitSourceRepository(t, "sha256") + require.Len(t, locked, 64) + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { require.NoError(t, prefetch.Close()) }) + _, err := prefetch.fetch(context.Background(), repository, locked) + require.NoError(t, err) +} + +// TestGitSourceMirrorTakesNoHostTemplate pins that a host template, which may +// carry hooks or a credential configuration, is not copied into the mirror the +// build receives. +func TestGitSourceMirrorTakesNoHostTemplate(t *testing.T) { + requireGit(t) + template := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(template, "config"), []byte("[credential]\n\thelper = leaked-from-template\n"), 0o644)) + require.NoError(t, os.MkdirAll(filepath.Join(template, "hooks"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(template, "hooks", "leaked-hook"), []byte("#!/bin/sh\n"), 0o755)) + hostGitConfig(t, fmt.Sprintf("[init]\n\ttemplateDir = %s\n", template)) + t.Setenv("GIT_TEMPLATE_DIR", template) + repository, locked := gitSourceRepository(t, "sha1") + + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { require.NoError(t, prefetch.Close()) }) + root, err := prefetch.fetch(context.Background(), repository, locked) + require.NoError(t, err) + mirror := filepath.Join(root, coreservices.GitSourceMirrorPath(repository)) + config, err := os.ReadFile(filepath.Join(mirror, "config")) + require.NoError(t, err) + require.NotContains(t, string(config), "leaked-from-template") + require.NoFileExists(t, filepath.Join(mirror, "hooks", "leaked-hook")) +} + +// unauthorizedGitHost answers every request with 401, so git asks the host's +// credential helper. +func unauthorizedGitHost(t *testing.T) string { + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) { + w.Header().Set("WWW-Authenticate", `Basic realm="git"`) + w.WriteHeader(http.StatusUnauthorized) + })) + t.Cleanup(server.Close) + return server.URL + "/team/lib.git" +} + +// TestGitSourcePrefetchTracesNoCredential pins that a host's git tracing, +// which prints a credential helper's command line, does not carry a secret into +// the error the prefetch reports and logs. +func TestGitSourcePrefetchTracesNoCredential(t *testing.T) { + requireGit(t) + const secret = "not-a-real-secret-0123" + hostGitConfig(t, "[credential]\n\thelper = \"!f() { echo username=ci; echo password="+secret+"; }; f\"\n") + for _, variable := range []string{"GIT_TRACE", "GIT_TRACE_CURL", "GIT_TRACE_PACKET", "GIT_TRACE2", "GIT_CURL_VERBOSE"} { + t.Setenv(variable, "1") + } + t.Setenv("GIT_TRACE_REDACT", "0") + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { _ = prefetch.Close() }) + _, err := prefetch.fetch(context.Background(), unauthorizedGitHost(t), commitOne) + require.ErrorContains(t, err, "Authentication failed", "the credential helper was consulted") + require.NotContains(t, err.Error(), secret) +} + +// TestGitSourcePrefetchStopsAHelperWithItsContext pins that a credential helper +// still running when the context ends is stopped with git, so the fetch +// returns at the deadline instead of when the helper finishes. +func TestGitSourcePrefetchStopsAHelperWithItsContext(t *testing.T) { + requireGit(t) + hostGitConfig(t, "[credential]\n\thelper = \"!f() { sleep 30; }; f\"\n") + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { _ = prefetch.Close() }) + ctx, cancel := context.WithTimeout(context.Background(), time.Second) + defer cancel() + start := time.Now() + _, err := prefetch.fetch(ctx, unauthorizedGitHost(t), commitOne) + require.Error(t, err) + require.Less(t, time.Since(start), gitWaitDelay, "the helper's process group is killed, not waited out") +} + // TestGitSourcePrefetchEnvKeepsHowTheHostReachesRepositories pins the boundary: // the host's git configuration (an insteadOf carrying a credential) reaches the // fetch, no prompt can hang it, no maintenance runs in the mirror, nothing @@ -117,17 +292,22 @@ func TestGitSourcePrefetchReportsACommitTheSourceLacks(t *testing.T) { // host's own rewrites. func TestGitSourcePrefetchEnvKeepsHowTheHostReachesRepositories(t *testing.T) { env := gitSourcePrefetchEnv([]string{ - "HOME=/home/dev", "GIT_DIR=/elsewhere/.git", "GIT_TERMINAL_PROMPT=1", + "HOME=/home/dev", "GIT_DIR=/elsewhere/.git", "GIT_TERMINAL_PROMPT=1", "GIT_TEMPLATE_DIR=/t", + "GIT_TRACE=1", "GIT_TRACE2_EVENT=/tmp/t", "GIT_CURL_VERBOSE=1", "GIT_CONFIG_COUNT=1", "GIT_CONFIG_KEY_0=url.https://token@git.example.com/.insteadOf", "GIT_CONFIG_VALUE_0=https://git.example.com/", }) value, _ := envValue(env, "GIT_TERMINAL_PROMPT") require.Equal(t, "0", value) - _, set := envValue(env, "GIT_DIR") - require.False(t, set) - _, set = envValue(env, "GIT_ALLOW_PROTOCOL") - require.False(t, set) + value, _ = envValue(env, "GCM_INTERACTIVE") + require.Equal(t, "never", value) + for _, dropped := range []string{"GIT_DIR", "GIT_ALLOW_PROTOCOL", "GIT_TEMPLATE_DIR", "GIT_TRACE", "GIT_TRACE2_EVENT", "GIT_CURL_VERBOSE"} { + _, set := envValue(env, dropped) + require.False(t, set, dropped) + } + value, _ = envValue(gitSourcePrefetchEnv([]string{"GCM_INTERACTIVE=always"}), "GCM_INTERACTIVE") + require.Equal(t, "always", value, "a host's own choice is kept") value, _ = envValue(env, "GIT_CONFIG_KEY_0") require.True(t, strings.HasPrefix(value, "url.https://token@"), "the host's own entry is kept first") require.Equal(t, map[string]string{ @@ -138,13 +318,14 @@ func TestGitSourcePrefetchEnvKeepsHowTheHostReachesRepositories(t *testing.T) { func TestGitCommandReportsAMissingGit(t *testing.T) { t.Setenv("PATH", t.TempDir()) - require.ErrorIs(t, runGitCommand(context.Background(), nil, "version"), errGitMissing) + _, err := runGitCommand(context.Background(), nil, "version") + require.ErrorIs(t, err, errGitMissing) } // TestGitCommandOutputHasNoURLCredentials guards the error the prefetch returns, // which is logged verbatim: a rewritten remote carrying a token must not reach it. func TestGitCommandOutputHasNoURLCredentials(t *testing.T) { - err := runGitCommand(context.Background(), os.Environ(), "--git-dir="+t.TempDir(), "fetch", "https://x-access-token:not-a-real-token@git.invalid/team/lib.git") + _, err := runGitCommand(context.Background(), os.Environ(), "--git-dir="+t.TempDir(), "fetch", "https://x-access-token:not-a-real-token@git.invalid/team/lib.git") require.Error(t, err) require.NotContains(t, err.Error(), "not-a-real-token") } diff --git a/pkg/orchestration/build_plan.go b/pkg/orchestration/build_plan.go index 805b9a27a..ef0f21367 100644 --- a/pkg/orchestration/build_plan.go +++ b/pkg/orchestration/build_plan.go @@ -49,7 +49,7 @@ const ( // recipe — multi-arch and pushed as a manifest list when pushing — so the build // recipe is a durable, first-class artifact and images are not tied to the // builder's host architecture. -func (b *Builder) buildFromPlan(ctx context.Context, outputDir string, plan *builderv0.DockerBuildPlan, proxies map[string]map[string]string, reuse map[string]imageReuse) error { +func (b *Builder) buildFromPlan(ctx context.Context, outputDir string, plan *builderv0.DockerBuildPlan, prefetched map[string]map[string]string, reuse map[string]imageReuse) error { w := wool.Get(ctx).In("Builder.buildFromPlan", wool.ThisField(b.instance)) if err := coreservices.VerifyDockerBuildPlan(outputDir, plan); err != nil { return w.Wrapf(err, "cannot verify build recipe for %s", b.instance.Unique()) @@ -74,7 +74,7 @@ func (b *Builder) buildFromPlan(ctx context.Context, outputDir string, plan *bui if decision, ok := reuse[recipe.GetName()]; ok { decided = &decision } - if err := b.buildRecipe(ctx, w, outputDir, contextRoot, plan, recipe, shouldPush, proxies[recipe.GetName()], decided); err != nil { + if err := b.buildRecipe(ctx, w, outputDir, contextRoot, plan, recipe, shouldPush, prefetched[recipe.GetName()], decided); err != nil { return err } } @@ -396,7 +396,7 @@ func (b *Builder) buildRecipe( } started := time.Now() w.Info("building image", wool.Field("image", recipe.GetImage()), wool.Field("push", shouldPush), - wool.Field("goprivate", private.GoPrivate), wool.Field("go_module_proxies", len(private.Proxies))) + wool.Field("goprivate", private.GoPrivate), wool.Field("prefetched_contexts", len(private.Proxies))) command := exec.CommandContext(ctx, "docker", args...) command.Stdout = os.Stderr command.Stderr = os.Stderr diff --git a/pkg/orchestration/build_private_modules.go b/pkg/orchestration/build_private_modules.go index 629ac0529..f100c19b0 100644 --- a/pkg/orchestration/build_private_modules.go +++ b/pkg/orchestration/build_private_modules.go @@ -21,7 +21,9 @@ type privateModuleBuild struct { // paths, not credentials. GoPrivate string // Proxies maps each Go module proxy and git source mirror build context the - // recipe declares to the prefetched directory supplied under that name. + // recipe declares to the prefetched directory supplied under that name. Git + // source mirrors (gitSourcePrefetch) share this route, the same named build + // context, so they share the field rather than a second one. Proxies map[string]string } diff --git a/pkg/orchestration/builder.go b/pkg/orchestration/builder.go index 65a944fd9..e6c8d2322 100644 --- a/pkg/orchestration/builder.go +++ b/pkg/orchestration/builder.go @@ -287,9 +287,9 @@ type plannedBuild struct { recipeRoot string outputDir string response *builderv0.BuildResponse - // proxies maps a recipe name to its prefetched Go module proxies and git + // prefetched maps a recipe name to its prefetched Go module proxies and git // source mirrors, keyed by the build-context name the recipe declares. - proxies map[string]map[string]string + prefetched map[string]map[string]string // reuse maps a recipe name to the reuse decision Plan took for it, for the // recipes it decided before fetching; Build adopts or builds from it. reuse map[string]imageReuse @@ -417,10 +417,10 @@ func (b *Builder) Plan(ctx context.Context) (*OutputProperty, error) { } maps.Copy(proxies, mirrors) } - if planned.proxies == nil { - planned.proxies = map[string]map[string]string{} + if planned.prefetched == nil { + planned.prefetched = map[string]map[string]string{} } - planned.proxies[recipe.GetName()] = proxies + planned.prefetched[recipe.GetName()] = proxies } } b.planned = planned @@ -457,7 +457,7 @@ func (b *Builder) Build(ctx context.Context) (*OutputProperty, error) { buildResult := dockerBuildResult(resp.Result) if plan != nil { - if err = b.buildFromPlan(ctx, outputDir, plan, planned.proxies, planned.reuse); err != nil { + if err = b.buildFromPlan(ctx, outputDir, plan, planned.prefetched, planned.reuse); err != nil { return nil, err } } else if buildResult != nil { diff --git a/pkg/orchestration/image_cache_test.go b/pkg/orchestration/image_cache_test.go index 096badc10..73184b395 100644 --- a/pkg/orchestration/image_cache_test.go +++ b/pkg/orchestration/image_cache_test.go @@ -114,8 +114,10 @@ type imageCachePeer struct { goModules bool // image is the recipe's image; empty means example.test/app:v1. image string - // gitSources makes the recipe declare a locked git commit for prefetching. + // gitSources makes the recipe declare a locked git commit for prefetching: + // commitOne, or gitCommit when set. gitSources bool + gitCommit string } func (*imageCachePeer) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { @@ -140,7 +142,11 @@ func (peer *imageCachePeer) Build(_ context.Context, request *builderv0.BuildReq options = append(options, coreservices.WithGoModuleDownloads(&builderv0.GoModuleDownload{ModuleRoot: "code", ProxyContext: "gomodproxy"})) } if peer.gitSources { - options = append(options, coreservices.WithGitSourceDownloads(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"})) + commit := peer.gitCommit + if commit == "" { + commit = commitOne + } + options = append(options, coreservices.WithGitSourceDownloads(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commit, MirrorContext: "gitsources"})) } plan, err := coreservices.SingleImageBuildPlan(output, peer.imageName(), coreservices.RecipeBuildPlatforms(), []string{"Dockerfile", "dockerignore"}, options...) if err != nil { @@ -397,7 +403,8 @@ func TestAnUpToDateImageFetchesNoGoModules(t *testing.T) { // TestAnUpToDateImageFetchesNoGitSources is the same plan phase for a declared // git source: fetched before the first build and handed to it as the mirror -// context, not fetched for an image already built from these inputs. +// context, not fetched for an image already built from these inputs, and a +// changed commit is a new identity, fetched and built. func TestAnUpToDateImageFetchesNoGitSources(t *testing.T) { harness := newImageCacheHarness(t, true, true) harness.peer.gitSources = true @@ -412,6 +419,12 @@ func TestAnUpToDateImageFetchesNoGitSources(t *testing.T) { require.Equal(t, first, harness.build(t)) require.Equal(t, 1, harness.docker.calls(t, "build")) require.Len(t, harness.gitCalls, 3, "an image built from these inputs needs no fetch") + + harness.peer.gitCommit = commitTwo + require.NotEqual(t, first, harness.build(t)) + require.Equal(t, 2, harness.docker.calls(t, "build")) + require.Len(t, harness.gitCalls, 6, "a changed commit is fetched before it is built") + require.Contains(t, harness.gitCalls[4], "+"+commitTwo+":"+coreservices.GitSourceMirrorRef(commitTwo)) } // TestPlaceholderProxiesNormalizeLikeFetchedPaths pins what lets the reuse From 5487344efb17941197af95c63ccb4af5cbd807fd Mon Sep 17 00:00:00 2001 From: Saurabh Maheshwari <242053479+saurabhmaheshwari-obin@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:42:13 -0400 Subject: [PATCH 3/6] fix(build): turn off trace2 targets a host enables in its git configuration Dropping the GIT_TRACE* variables left trace2.normalTarget, eventTarget and perfTarget from the host's git configuration active, and each wrote a credential helper's command line, secret included, into the reported error or a trace file. The fetch now sets GIT_TRACE2, GIT_TRACE2_EVENT and GIT_TRACE2_PERF to 0, which override the configuration. Co-Authored-By: Claude Opus 5.5 --- pkg/orchestration/build_git_sources.go | 9 ++-- pkg/orchestration/build_git_sources_test.go | 51 ++++++++++++++++----- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/pkg/orchestration/build_git_sources.go b/pkg/orchestration/build_git_sources.go index ef11dd27c..fe7ce4195 100644 --- a/pkg/orchestration/build_git_sources.go +++ b/pkg/orchestration/build_git_sources.go @@ -175,8 +175,9 @@ func (p *gitSourcePrefetch) Close() error { // fetch fails instead of hanging; // - gitPrefetchConfig; // - no variable pointing git at another repository or template; -// - no tracing: a trace prints a credential helper's command line and -// protocol traffic into the output this reports. +// - no tracing, whether the host enables it in the environment or in its +// git configuration: a trace prints a credential helper's command line and +// protocol traffic into the output this reports or a file. // // It sets no GIT_ALLOW_PROTOCOL: git checks that after the host's insteadOf // rewrites, so it would refuse a host rewrite to a local path; the declared URL @@ -199,7 +200,9 @@ func gitSourcePrefetchEnv(base []string) []string { } env = append(env, entry) } - env = append(env, "GIT_TERMINAL_PROMPT=0") + // Trace2 is also enabled from configuration (trace2.*Target), which the + // environment overrides; turning each target off here covers both. + env = append(env, "GIT_TERMINAL_PROMPT=0", "GIT_TRACE2=0", "GIT_TRACE2_EVENT=0", "GIT_TRACE2_PERF=0") if !interactive { env = append(env, "GCM_INTERACTIVE=never") } diff --git a/pkg/orchestration/build_git_sources_test.go b/pkg/orchestration/build_git_sources_test.go index 8d01026c0..f07a7f497 100644 --- a/pkg/orchestration/build_git_sources_test.go +++ b/pkg/orchestration/build_git_sources_test.go @@ -252,21 +252,44 @@ func unauthorizedGitHost(t *testing.T) string { } // TestGitSourcePrefetchTracesNoCredential pins that a host's git tracing, -// which prints a credential helper's command line, does not carry a secret into -// the error the prefetch reports and logs. +// which prints a credential helper's command line, carries no secret into the +// error the prefetch reports and logs, nor into a trace file, whether the host +// enables it in the environment or through a trace2 target in its git +// configuration. func TestGitSourcePrefetchTracesNoCredential(t *testing.T) { requireGit(t) const secret = "not-a-real-secret-0123" - hostGitConfig(t, "[credential]\n\thelper = \"!f() { echo username=ci; echo password="+secret+"; }; f\"\n") - for _, variable := range []string{"GIT_TRACE", "GIT_TRACE_CURL", "GIT_TRACE_PACKET", "GIT_TRACE2", "GIT_CURL_VERBOSE"} { - t.Setenv(variable, "1") + helper := "[credential]\n\thelper = \"!f() { echo username=ci; echo password=" + secret + "; }; f\"\n" + for name, target := range map[string]string{ + "environment": "", + "trace2.normalTarget": "1", + "trace2.eventTarget": "file", + "trace2.perfTarget": "file", + } { + t.Run(name, func(t *testing.T) { + traceFile := filepath.Join(t.TempDir(), "trace") + config := helper + switch target { + case "": + for _, variable := range []string{"GIT_TRACE", "GIT_TRACE_CURL", "GIT_TRACE_PACKET", "GIT_TRACE2", "GIT_CURL_VERBOSE"} { + t.Setenv(variable, "1") + } + case "file": + config += fmt.Sprintf("[trace2]\n\t%s = %s\n", strings.TrimPrefix(name, "trace2."), traceFile) + default: + config += fmt.Sprintf("[trace2]\n\t%s = %s\n", strings.TrimPrefix(name, "trace2."), target) + } + hostGitConfig(t, config) + t.Setenv("GIT_TRACE_REDACT", "0") + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { _ = prefetch.Close() }) + _, err := prefetch.fetch(context.Background(), unauthorizedGitHost(t), commitOne) + require.ErrorContains(t, err, "Authentication failed", "the credential helper was consulted") + require.NotContains(t, err.Error(), secret) + traced, _ := os.ReadFile(traceFile) + require.NotContains(t, string(traced), secret) + }) } - t.Setenv("GIT_TRACE_REDACT", "0") - prefetch := newGitSourcePrefetch() - t.Cleanup(func() { _ = prefetch.Close() }) - _, err := prefetch.fetch(context.Background(), unauthorizedGitHost(t), commitOne) - require.ErrorContains(t, err, "Authentication failed", "the credential helper was consulted") - require.NotContains(t, err.Error(), secret) } // TestGitSourcePrefetchStopsAHelperWithItsContext pins that a credential helper @@ -302,10 +325,14 @@ func TestGitSourcePrefetchEnvKeepsHowTheHostReachesRepositories(t *testing.T) { require.Equal(t, "0", value) value, _ = envValue(env, "GCM_INTERACTIVE") require.Equal(t, "never", value) - for _, dropped := range []string{"GIT_DIR", "GIT_ALLOW_PROTOCOL", "GIT_TEMPLATE_DIR", "GIT_TRACE", "GIT_TRACE2_EVENT", "GIT_CURL_VERBOSE"} { + for _, dropped := range []string{"GIT_DIR", "GIT_ALLOW_PROTOCOL", "GIT_TEMPLATE_DIR", "GIT_TRACE", "GIT_CURL_VERBOSE"} { _, set := envValue(env, dropped) require.False(t, set, dropped) } + for _, off := range []string{"GIT_TRACE2", "GIT_TRACE2_EVENT", "GIT_TRACE2_PERF"} { + value, _ = envValue(env, off) + require.Equal(t, "0", value, "%s is off, overriding a trace2 target in the host's configuration", off) + } value, _ = envValue(gitSourcePrefetchEnv([]string{"GCM_INTERACTIVE=always"}), "GCM_INTERACTIVE") require.Equal(t, "always", value, "a host's own choice is kept") value, _ = envValue(env, "GIT_CONFIG_KEY_0") From b6079c66de278e1b299cbec0723a27d258be4798 Mon Sep 17 00:00:00 2001 From: Saurabh Maheshwari <242053479+saurabhmaheshwari-obin@users.noreply.github.com> Date: Wed, 21 Oct 2026 17:52:41 -0400 Subject: [PATCH 4/6] fix(build): report a failed git source fetch by classification, never by git's output git's own error text can carry a secret no URL redaction recognises: it prints a credential helper's command line when the helper tells it to quit, a missing helper's path, and whatever a helper or askpass writes to stderr. That text reached the returned error and the plan phase's warning. A failure now names the git subcommand and its exit status, a reason from a fixed set (authentication failed, host unreachable, repository not found, commit not available, git failed) matched from the output without echoing it, and a hint to rerun the same git with the host's git to see its output. Co-Authored-By: Claude Opus 5.5 --- pkg/orchestration/build_git_sources.go | 58 +++++++++++++++++-- pkg/orchestration/build_git_sources_test.go | 63 ++++++++++++++++++++- 2 files changed, 114 insertions(+), 7 deletions(-) diff --git a/pkg/orchestration/build_git_sources.go b/pkg/orchestration/build_git_sources.go index fe7ce4195..5fd1ec6b8 100644 --- a/pkg/orchestration/build_git_sources.go +++ b/pkg/orchestration/build_git_sources.go @@ -15,8 +15,6 @@ import ( coreservices "github.com/codefly-dev/core/agents/services" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" - - "github.com/codefly-dev/cli/pkg/redact" ) // gitSourcePrefetch fetches the locked git commits a flow's recipes declare @@ -115,7 +113,7 @@ func (p *gitSourcePrefetch) fetch(ctx context.Context, repository, commit string return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) } if resolved != commit { - return "", fmt.Errorf("the declared git source resolves commit %s to %s", commit, resolved) + return "", fmt.Errorf("%s: the mirror ref of commit %s resolves to %s", gitCommitUnavailable, commit, resolved) } p.fetched[[2]string{repository, commit}] = true return p.root, nil @@ -218,6 +216,12 @@ const gitWaitDelay = 5 * time.Second // runGitCommand runs git in its own process group, so cancelling the context // stops git and every helper it started, and returns its trimmed standard // output. +// +// A failure never carries git's own output. git prints a credential helper's +// command line, a missing helper's path, and whatever a helper or askpass +// writes to stderr, and none of that is a URL a redaction can recognise. The +// error names the git subcommand, its exit status and a reason classified from +// the output, which is matched here and then dropped. func runGitCommand(ctx context.Context, env []string, args ...string) (string, error) { gitBinary, err := exec.LookPath("git") if err != nil { @@ -232,8 +236,52 @@ func runGitCommand(ctx context.Context, env []string, args ...string) (string, e command.Stdout = &stdout command.Stderr = &output if err := command.Run(); err != nil { - output.Write(stdout.Bytes()) - return "", fmt.Errorf("git %s: %w\n%s", redact.URLCredentials(strings.Join(args, " ")), err, redact.URLCredentials(lastLines(output.String(), 40))) + subcommand := gitSubcommand(args) + failure := fmt.Errorf("git %s exited with status %d: %s; rerun the same git %s with the host's git to see its own output", + subcommand, command.ProcessState.ExitCode(), classifyGitFailure(output.String()+stdout.String()), subcommand) + if ctx.Err() != nil { + return "", errors.Join(failure, ctx.Err()) + } + return "", failure } return strings.TrimSpace(stdout.String()), nil } + +// gitSubcommand is the first argument that is not an option. +func gitSubcommand(args []string) string { + for _, argument := range args { + if !strings.HasPrefix(argument, "-") { + return argument + } + } + return "git" +} + +// gitFailureReasons is the fixed set a git failure is reported as, each with +// the output that identifies it, checked in order. +var gitFailureReasons = []struct { + reason string + markers []string +}{ + {gitCommitUnavailable, []string{"not our ref", "couldn't find remote ref", "unadvertised object"}}, + {"authentication failed", []string{"authentication failed", "could not read username", "could not read password", + "terminal prompts disabled", "permission denied (publickey)", "told us to quit"}}, + {"host unreachable", []string{"could not resolve host", "connection refused", "connection timed out", "failed to connect"}}, + {"repository not found", []string{"repository not found", "does not appear to be a git repository", "not found"}}, +} + +const gitCommitUnavailable = "commit not available" + +// classifyGitFailure reduces git's output to one of gitFailureReasons, or +// "git failed". +func classifyGitFailure(output string) string { + output = strings.ToLower(output) + for _, candidate := range gitFailureReasons { + for _, marker := range candidate.markers { + if strings.Contains(output, marker) { + return candidate.reason + } + } + } + return "git failed" +} diff --git a/pkg/orchestration/build_git_sources_test.go b/pkg/orchestration/build_git_sources_test.go index f07a7f497..3c27ea4fc 100644 --- a/pkg/orchestration/build_git_sources_test.go +++ b/pkg/orchestration/build_git_sources_test.go @@ -139,7 +139,7 @@ func TestGitSourcePrefetchRefusesAMirrorRefNamingAnotherCommit(t *testing.T) { } t.Cleanup(func() { _ = prefetch.Close() }) _, err := prefetch.fetch(context.Background(), gitSourceURL, commitOne) - require.ErrorContains(t, err, "resolves commit "+commitOne+" to "+commitTwo) + require.ErrorContains(t, err, "commit not available: the mirror ref of commit "+commitOne+" resolves to "+commitTwo) } // TestGitSourcePrefetchMatchesTheObjectFormatToTheCommit pins that a 64-digit @@ -284,7 +284,7 @@ func TestGitSourcePrefetchTracesNoCredential(t *testing.T) { prefetch := newGitSourcePrefetch() t.Cleanup(func() { _ = prefetch.Close() }) _, err := prefetch.fetch(context.Background(), unauthorizedGitHost(t), commitOne) - require.ErrorContains(t, err, "Authentication failed", "the credential helper was consulted") + require.ErrorContains(t, err, "authentication failed", "the credential helper was consulted") require.NotContains(t, err.Error(), secret) traced, _ := os.ReadFile(traceFile) require.NotContains(t, string(traced), secret) @@ -292,6 +292,65 @@ func TestGitSourcePrefetchTracesNoCredential(t *testing.T) { } } +// TestGitSourceFailuresAreClassifiedNotEchoed pins that git's own output never +// reaches the error the prefetch returns, which the plan phase logs: git prints +// a credential helper's command line, a missing helper's path, and what a +// helper or askpass writes to stderr, and none of it is a URL redaction can +// recognise. The failure is classified instead. +func TestGitSourceFailuresAreClassifiedNotEchoed(t *testing.T) { + requireGit(t) + const marker = "secret-marker-4711" + askpass := filepath.Join(t.TempDir(), "askpass") + require.NoError(t, os.WriteFile(askpass, []byte("#!/bin/sh\necho "+marker+" >&2\nexit 1\n"), 0o755)) + for name, setup := range map[string]func(t *testing.T){ + "a helper's command": func(t *testing.T) { + hostGitConfig(t, "[credential]\n\thelper = \"!f() { token="+marker+"; echo quit=true; }; f\"\n") + }, + "a missing helper": func(t *testing.T) { + hostGitConfig(t, "[credential]\n\thelper = /nonexistent/"+marker+"\n") + }, + "a helper's stderr": func(t *testing.T) { + hostGitConfig(t, "[credential]\n\thelper = \"!f() { echo "+marker+" >&2; }; f\"\n") + }, + "askpass stderr": func(t *testing.T) { + hostGitConfig(t, "") + t.Setenv("GIT_ASKPASS", askpass) + }, + } { + t.Run(name, func(t *testing.T) { + setup(t) + prefetch := newGitSourcePrefetch() + t.Cleanup(func() { _ = prefetch.Close() }) + _, err := prefetch.mirrorsFor(context.Background(), gitSourceRecipe(&builderv0.GitSourceDownload{ + Repository: unauthorizedGitHost(t), Commit: commitOne, MirrorContext: "gitsources"})) + require.Error(t, err) + require.NotContains(t, err.Error(), marker) + require.ErrorContains(t, err, "recipe app git source 0") + require.ErrorContains(t, err, "git fetch exited with status 128: authentication failed") + require.ErrorContains(t, err, "rerun") + }) + } +} + +// TestGitFailureClassification pins the fixed set of reasons a failure is +// reported as. +func TestGitFailureClassification(t *testing.T) { + for output, reason := range map[string]string{ + "fatal: Authentication failed for 'https://h/r'": "authentication failed", + "fatal: could not read Username for 'https://h': terminal prompts disabled": "authentication failed", + "git@h: Permission denied (publickey).": "authentication failed", + "fatal: unable to access 'https://h/': Could not resolve host: h": "host unreachable", + "Failed to connect to h port 443: Connection refused": "host unreachable", + "remote: Repository not found.": "repository not found", + "fatal: '/x' does not appear to be a git repository": "repository not found", + "error: upload-pack: not our ref 1111": "commit not available", + "fatal: couldn't find remote ref 1111": "commit not available", + "fatal: something else": "git failed", + } { + require.Equal(t, reason, classifyGitFailure(output), output) + } +} + // TestGitSourcePrefetchStopsAHelperWithItsContext pins that a credential helper // still running when the context ends is stopped with git, so the fetch // returns at the deadline instead of when the helper finishes. From 7e27f84d1d85a8d7baede8901a5f154bd560a2ad Mon Sep 17 00:00:00 2001 From: Saurabh Maheshwari <242053479+saurabhmaheshwari-obin@users.noreply.github.com> Date: Wed, 21 Oct 2026 18:01:09 -0400 Subject: [PATCH 5/6] fix(build): a commit still missing after the fallback fetch reads as unavailable Co-Authored-By: Claude Opus 5.5 --- pkg/orchestration/build_git_sources.go | 6 ++++-- pkg/orchestration/build_git_sources_test.go | 23 ++++++++++++++++++++- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/pkg/orchestration/build_git_sources.go b/pkg/orchestration/build_git_sources.go index 5fd1ec6b8..1a03365a3 100644 --- a/pkg/orchestration/build_git_sources.go +++ b/pkg/orchestration/build_git_sources.go @@ -102,15 +102,17 @@ func (p *gitSourcePrefetch) fetch(ctx context.Context, repository, commit string if _, fullErr := git(full...); fullErr != nil { return "", fmt.Errorf("cannot fetch commit %s of a declared git source: %w", commit, errors.Join(err, fullErr)) } + // The branches and tags arrived but none reaches the commit: the + // source no longer holds it, whatever git's own wording was. if _, err := git("update-ref", ref, commit); err != nil { - return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) + return "", fmt.Errorf("%s: the declared git source does not hold commit %s", gitCommitUnavailable, commit) } } // The mirror ref must name exactly the locked commit: a branch spelled like // a commit id is never taken for it. resolved, err := git("rev-parse", "--verify", "--end-of-options", ref+"^{commit}") if err != nil { - return "", fmt.Errorf("the declared git source does not hold commit %s: %w", commit, err) + return "", fmt.Errorf("%s: the declared git source does not hold commit %s", gitCommitUnavailable, commit) } if resolved != commit { return "", fmt.Errorf("%s: the mirror ref of commit %s resolves to %s", gitCommitUnavailable, commit, resolved) diff --git a/pkg/orchestration/build_git_sources_test.go b/pkg/orchestration/build_git_sources_test.go index 3c27ea4fc..92c8574f6 100644 --- a/pkg/orchestration/build_git_sources_test.go +++ b/pkg/orchestration/build_git_sources_test.go @@ -122,7 +122,28 @@ func TestGitSourcePrefetchReportsACommitTheSourceLacks(t *testing.T) { t.Cleanup(func() { _ = prefetch.Close() }) _, err := prefetch.mirrorsFor(context.Background(), gitSourceRecipe(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"})) require.ErrorContains(t, err, "recipe app git source 0") - require.ErrorContains(t, err, "does not hold commit "+commitOne) + require.ErrorContains(t, err, "commit not available: the declared git source does not hold commit "+commitOne) +} + +// TestGitSourcePrefetchClassifiesACommitMissingAfterTheFallback pins that a +// commit still absent once the branches and tags arrived reads as unavailable, +// not as a generic git failure. +func TestGitSourcePrefetchClassifiesACommitMissingAfterTheFallback(t *testing.T) { + prefetch := newGitSourcePrefetch() + prefetch.run = func(_ context.Context, _ []string, args ...string) (string, error) { + if args[0] == "init" { + return "", os.MkdirAll(args[len(args)-1], 0o755) + } + if slices.Contains(args, "--depth=1") || slices.Contains(args, "update-ref") { + return "", errors.New("git update-ref exited with status 128: git failed") + } + return "", nil + } + t.Cleanup(func() { _ = prefetch.Close() }) + _, err := prefetch.mirrorsFor(context.Background(), gitSourceRecipe(&builderv0.GitSourceDownload{Repository: gitSourceURL, Commit: commitOne, MirrorContext: "gitsources"})) + require.ErrorContains(t, err, "recipe app git source 0") + require.ErrorContains(t, err, "commit not available: the declared git source does not hold commit "+commitOne) + require.NotContains(t, err.Error(), "git failed") } // TestGitSourcePrefetchRefusesAMirrorRefNamingAnotherCommit pins that the From 0d8232082d8d79eeb6a3232456d59aa4e1e82a59 Mon Sep 17 00:00:00 2001 From: Saurabh Maheshwari <242053479+saurabhmaheshwari-obin@users.noreply.github.com> Date: Fri, 23 Oct 2026 16:19:13 -0400 Subject: [PATCH 6/6] build(deps): pin core to v0.19.6 merged into the envelope commit main pins cli main pins core at 5522c4da (chore/envelope-document-plus-buildx-fix), which carries the module envelope API pkg/delivery/signing uses. core chore/envelope-on-0.19.6 (257e6b23) merges v0.19.6 into that commit, so this branch gets the git source declaration (#832) without dropping the envelope. Co-Authored-By: Claude Opus 5.5 --- docs/supported-matrix.md | 5 ++++- go.mod | 2 +- go.sum | 4 ++-- pkg/conformance/matrix.json | 2 +- 4 files changed, 8 insertions(+), 5 deletions(-) diff --git a/docs/supported-matrix.md b/docs/supported-matrix.md index 440397918..44e8a3434 100644 --- a/docs/supported-matrix.md +++ b/docs/supported-matrix.md @@ -27,7 +27,10 @@ When `go.mod` moves to a new core release line, update the `core` field in `go test ./pkg/conformance/...` before the full coverage and race gates. `TestMatrixCoreMatchesGoMod` refuses a stale declared version. -The current pin is a `v0.19.6` pseudo-version of core, ahead of any tag: +The current pin is a `v0.19.7` pseudo-version of core: `257e6b23`, branch +`chore/envelope-on-0.19.6`, which merges `v0.19.6` (with the git source +declaration, codefly-dev/core#832) into `5522c4da`, the commit carrying the +module envelope that `pkg/delivery/signing` uses. `v0.19.6` carries `b81d92e6`, the merge of codefly-dev/core#812 — the module manifest (`solutionhost.ModuleManifest`, one signed document per module, whose workload is `(service, container)` and which names no build) and diff --git a/go.mod b/go.mod index d4ab0c0c0..c2ccd745d 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,7 @@ require ( github.com/asottile/dockerfile v3.1.0+incompatible github.com/blang/semver v3.5.1+incompatible github.com/briandowns/spinner v1.23.2 - github.com/codefly-dev/core v0.19.6-0.20261022093222-5522c4dac804 + github.com/codefly-dev/core v0.19.7-0.20261023201433-257e6b234bd3 github.com/codefly-dev/golor v0.1.3 github.com/codefly-dev/llm v0.1.7 github.com/codefly-dev/sdk-go/workcontext v0.1.1-0.20261018052022-70dc7a546458 diff --git a/go.sum b/go.sum index 346c7db56..4c679558b 100644 --- a/go.sum +++ b/go.sum @@ -103,8 +103,8 @@ github.com/clipperhouse/uax29/v2 v2.7.0 h1:+gs4oBZ2gPfVrKPthwbMzWZDaAFPGYK72F0NJ github.com/clipperhouse/uax29/v2 v2.7.0/go.mod h1:EFJ2TJMRUaplDxHKj1qAEhCtQPW2tJSwu5BF98AuoVM= github.com/cloudflare/circl v1.6.5 h1:O64F26HEqNhznd/hrC5KZXVKYuKM2rx4deZDTc4ihQA= github.com/cloudflare/circl v1.6.5/go.mod h1:h5LNyxAc5nTue9DS5jT+48en2PSDYt3zdGnz5OstK6c= -github.com/codefly-dev/core v0.19.6-0.20261022093222-5522c4dac804 h1:JyW6eDXENq/BqNJbPy0De30MpFHoEDBW9Uwc0eR2Prs= -github.com/codefly-dev/core v0.19.6-0.20261022093222-5522c4dac804/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= +github.com/codefly-dev/core v0.19.7-0.20261023201433-257e6b234bd3 h1:Qi+DwoxKK7UU6OVmiwj+bLIqlvrLXk5ttnz96+2f97A= +github.com/codefly-dev/core v0.19.7-0.20261023201433-257e6b234bd3/go.mod h1:1ina3kP+YSAL4tlZvnEYd4P8bFGnJECfkP86ojeeF1A= github.com/codefly-dev/golor v0.1.3 h1:xmo+ceyJFRYZdvpWE2fNd0jeaadp/Ibm1BnganiGKOc= github.com/codefly-dev/golor v0.1.3/go.mod h1:sl/u/K1l7J0Pr3xyVZp8fOJYQItKKst1No9JqgzLLoY= github.com/codefly-dev/gortk v0.2.0 h1:7bOlS5valYz2zil+fZctQNcPCYBcPj86abcw9N8h1hQ= diff --git a/pkg/conformance/matrix.json b/pkg/conformance/matrix.json index 932e51f44..5dfa1818d 100644 --- a/pkg/conformance/matrix.json +++ b/pkg/conformance/matrix.json @@ -1,7 +1,7 @@ { "schema_version": 1, "cli": "source", - "core": "v0.19.6", + "core": "v0.19.7", "rows": [ { "id": "linux-amd64-source",