From a76e59bcaac4151d315af0c7fc75a901012000da Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Fri, 11 Sep 2026 21:02:01 -0400 Subject: [PATCH 1/6] fix: make shared image runners recipe-only (#461) --- .github/workflows/build-cache.yml | 6 +- agents/services/base_builder.go | 10 +- agents/services/docker_recipe.go | 10 +- agents/services/docker_recipe_test.go | 6 +- docs/build-cache.md | 50 ++--- .../codefly/services/builder/v0/builder.pb.go | 4 +- .../codefly/services/builder/v0/builder.proto | 4 +- runners/golang/agent_builder.go | 91 ++------- runners/golang/agent_builder_cache_test.go | 187 +++++++++--------- runners/golang/agent_builder_test.go | 43 ---- runners/rust/agent_builder.go | 41 +--- 11 files changed, 142 insertions(+), 310 deletions(-) delete mode 100644 runners/golang/agent_builder_test.go diff --git a/.github/workflows/build-cache.yml b/.github/workflows/build-cache.yml index 4304549e2..66c4dfc18 100644 --- a/.github/workflows/build-cache.yml +++ b/.github/workflows/build-cache.yml @@ -29,11 +29,9 @@ jobs: - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version: "1.27.0" - - name: Verify in-agent Buildx selection and cache export + - name: Verify recipe-only runners without Docker on PATH if: matrix.language == 'go' - env: - CODEFLY_TEST_REGISTRY_CACHE: "1" - run: go test ./runners/golang -run '^TestCustomContextBuildUsesRequestedBuildxAndExportsCache$' -count=1 -v -timeout=5m + run: go test ./runners/golang -run '^TestImageRecipe' -count=1 -v -timeout=5m - name: Reject hidden inputs in exported cache layers env: CODEFLY_TEST_REGISTRY_CACHE: "1" diff --git a/agents/services/base_builder.go b/agents/services/base_builder.go index d576f0674..252abf592 100644 --- a/agents/services/base_builder.go +++ b/agents/services/base_builder.go @@ -259,14 +259,8 @@ func (s *BuilderWrapper) WithBuildPlan(plan *builderv0.DockerBuildPlan) { } } -// SingleImageBuildResponse renders the conventional single-image recipe over the -// caller's output_directory, records it as the build result, and returns the build -// response. A language runner calls this — instead of building the image in-process -// — when BuildPlanRequested(req) is true. The conventional layout it emits -// (a Dockerfile in output_directory, context = the service directory, an optional -// dockerignore) is language-agnostic, so Go, Rust, Python, Node, and every -// other agent built on the shared builder move to CLI-owned, multi-arch builds -// through this one path — just by re-pinning core. +// SingleImageBuildResponse records a single-image recipe over the caller's +// output_directory and returns the build response. The CLI executes the plan. func (s *BuilderWrapper) SingleImageBuildResponse(req *builderv0.BuildRequest, image string) (*builderv0.BuildResponse, error) { plan, err := SingleImageBuildPlan(req.GetOutputDirectory(), image, RecipeBuildPlatforms()) if err != nil { diff --git a/agents/services/docker_recipe.go b/agents/services/docker_recipe.go index 50c0fe640..fa3fcac3a 100644 --- a/agents/services/docker_recipe.go +++ b/agents/services/docker_recipe.go @@ -23,7 +23,7 @@ const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v2" // ValidateBuildRequestOutputDirectory enforces the BuildRequest.output_directory // contract: when set, the destination must be an absolute path the caller owns. -// Empty is valid and selects the legacy in-agent build. A relative path is +// Empty is valid for requests that do not build an image. A relative path is // rejected with a clear error rather than silently resolved against whatever // working directory the agent happens to run in — the agent and the caller would // otherwise resolve it against different directories and the recipe handshake @@ -82,12 +82,8 @@ func RecipeBuildPlatforms() []string { return []string{"linux/amd64", "linux/arm64"} } -// BuildPlanRequested reports whether the caller (the CLI) owns the build for this -// request — a non-empty BuildRequest.output_directory means the runner should emit -// a recipe into that directory instead of building the image in-process. This is -// the single negotiation point every language runner checks, so the recipe path is -// language-agnostic: Go, Rust, Python, Node — any agent built on the shared builder -// switches to CLI-owned builds by consulting this and nothing else. +// BuildPlanRequested reports whether a request supplies a recipe destination. +// Image-building runners must reject requests without one before preparation. func BuildPlanRequested(req *builderv0.BuildRequest) bool { return req.GetOutputDirectory() != "" } diff --git a/agents/services/docker_recipe_test.go b/agents/services/docker_recipe_test.go index 2fd041974..d9b1a5792 100644 --- a/agents/services/docker_recipe_test.go +++ b/agents/services/docker_recipe_test.go @@ -64,10 +64,10 @@ func TestBuildDockerBuildPlanInventoriesRecipeTree(t *testing.T) { } func TestValidateBuildRequestOutputDirectory(t *testing.T) { - // A nil request carries no directory and selects the legacy in-agent build. + // A nil request carries no directory and requires no recipe destination. require.NoError(t, ValidateBuildRequestOutputDirectory(nil)) - // Empty selects the legacy in-agent build and is valid. + // Empty requires no recipe destination and is valid. require.NoError(t, ValidateBuildRequestOutputDirectory(&builderv0.BuildRequest{})) // An absolute destination honors the contract. @@ -104,7 +104,7 @@ func TestDockerBuildRequestEnforcesAbsoluteOutputDirectory(t *testing.T) { require.NoError(t, err) require.NotNil(t, got) - // An empty output_directory (legacy in-agent build) passes through. + // No-build requests may omit output_directory. got, err = wrapper.DockerBuildRequest(context.Background(), &builderv0.BuildRequest{BuildContext: dockerContext}) require.NoError(t, err) require.NotNil(t, got) diff --git a/docs/build-cache.md b/docs/build-cache.md index 5dba3b2f2..faa1d00d9 100644 --- a/docs/build-cache.md +++ b/docs/build-cache.md @@ -2,8 +2,7 @@ Core supports the `registry-v1` cache contract through `DockerBuildContext.cache`. This is optional layer reuse: every build still -executes BuildKit, verifies the requested architecture on the in-agent path, -and produces its own image. Cache references never substitute for artifact +executes BuildKit in the CLI and produces its own image. Cache references never substitute for artifact manifest digests, recipe verification, tests, or release authorization. A caller can supply this protobuf JSON fragment in its Docker build context: @@ -49,13 +48,9 @@ build instead of claiming publication succeeded. ## Integration through Codefly -The Go and Rust shared in-agent builders forward cache policy to Buildx. -`SingleImageBuildResponse` returns only recipes. Recipe agents need not claim -cache execution: the CLI validates their plan and applies its own policy. -`BuilderAgent.Build` requires `registry-v1` acknowledgement for successful -in-agent image results when cache was requested, rejecting legacy agents that -silently ignore it. Generated gRPC clients used directly must enforce the same -result-kind distinction. +The Go and Rust shared runners return only recipes through +`SingleImageBuildResponse`. The CLI validates their plan and applies its own +cache policy when executing Buildx. CLI recipe executors should verify the recipe tree as usual, then append `docker.CacheArguments(callerCache, recipe.Platforms)` to their existing Buildx @@ -135,27 +130,19 @@ edits, rebuilt it for lockfile/base changes, and produced the expected changed outputs. These are conformance measurements, not a speedup claim; startup and host load affect wall time. -### In-agent executor selection +### CLI executor selection -`DockerBuildContext.buildx_builder` carries the caller's Buildx builder name -through Go and Rust in-agent builds, including Go services with a custom context -root. It is passed as `--builder` on that build only; the selected global builder -and process environment are unchanged. The caller provisions its builder before -issuing Build. Registry exports require a container-driver builder or a Docker -driver configured with the containerd image store. +The CLI alone builds and publishes application images. Shared Go and Rust +runners require an absolute `output_directory` and return a DockerBuildPlan. +Missing destinations fail before template preparation. Go custom/workspace-root +contexts are rejected before preparation because the single-image recipe cannot +represent them. Managed images and runtime-only agents remain no-build. -Before dispatching a Build with an explicit selection, `BuilderAgent.Build` -calls the read-only `BuildCapabilities` RPC and requires `buildx_selection=true`. -Older agents, unsupported agents, and failed probes are rejected before Build -can transfer source, change image tags, or export cache. This also applies when -`output_directory` requests a recipe: an older agent can fall back to executing -the build. Requests without explicit selection do not require the probe. - -Agents must explicitly implement the capability RPC after verifying that every -in-agent path honors selection (for example, through the updated `BuildGoDocker` -or `BuildRustDocker` helper). Recipe-only agents may opt in as well. The default -server leaves the RPC unimplemented so a custom Build cannot accidentally claim -support just by updating Core: +`DockerBuildContext.buildx_builder` identifies the builder the CLI selected, +including selection for registry cache exports. The agent does not invoke it. +Before dispatching an explicit selection, `BuilderAgent.Build` requires the +read-only `BuildCapabilities` RPC to report `buildx_selection=true`. Recipe-only +agents should implement this capability after verifying their Build path: ```go func (*Builder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { @@ -163,7 +150,6 @@ func (*Builder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesR } ``` -Successful in-agent responses additionally acknowledge the selection in -`BuildResponse.buildx_builder`. `BuilderAgent.Build` rejects an absent or different -acknowledgement when a builder was requested. Recipe responses need no executor -acknowledgement because the caller executes the plan itself. +Recipe responses need no executor acknowledgement because the CLI executes the +plan itself. The Docker execution and registry-cache helpers remain available +for the CLI executor; service agents must not call them to build images. diff --git a/generated/go/codefly/services/builder/v0/builder.pb.go b/generated/go/codefly/services/builder/v0/builder.pb.go index 506bbe8e9..51137376a 100644 --- a/generated/go/codefly/services/builder/v0/builder.pb.go +++ b/generated/go/codefly/services/builder/v0/builder.pb.go @@ -1884,8 +1884,8 @@ type BuildRequest struct { // build_context is the container build context supplied to the builder. BuildContext *BuildContext `protobuf:"bytes,1,opt,name=build_context,json=buildContext,proto3" json:"build_context,omitempty"` // output_directory is an absolute destination owned by the caller where the - // builder writes reproducible build recipes. Empty means the builder does not - // emit recipes and performs the legacy in-agent build. + // builder writes reproducible build recipes. Required for application image + // builds; managed-image and runtime-only agents may omit it. OutputDirectory string `protobuf:"bytes,2,opt,name=output_directory,json=outputDirectory,proto3" json:"output_directory,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache diff --git a/proto/codefly/services/builder/v0/builder.proto b/proto/codefly/services/builder/v0/builder.proto index 6e3042fff..8dc8db9d4 100644 --- a/proto/codefly/services/builder/v0/builder.proto +++ b/proto/codefly/services/builder/v0/builder.proto @@ -254,8 +254,8 @@ message BuildRequest { // build_context is the container build context supplied to the builder. BuildContext build_context = 1; // output_directory is an absolute destination owned by the caller where the - // builder writes reproducible build recipes. Empty means the builder does not - // emit recipes and performs the legacy in-agent build. + // builder writes reproducible build recipes. Required for application image + // builds; managed-image and runtime-only agents may omit it. string output_directory = 2; } diff --git a/runners/golang/agent_builder.go b/runners/golang/agent_builder.go index 0c15b1dd9..4143f842e 100644 --- a/runners/golang/agent_builder.go +++ b/runners/golang/agent_builder.go @@ -4,8 +4,6 @@ import ( "context" "embed" "fmt" - "io" - "path/filepath" "strings" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" @@ -31,7 +29,7 @@ type DockerTemplating struct { SourceDir string // e.g. "code/cmd/server" — the Go main package location ModuleRoot string // e.g. "code" — where go.mod lives BuildTarget string // e.g. "./cmd/server" — package to build (relative to ModuleRoot) - ContextRoot string // optional exact Docker context root; defaults to the service directory + ContextRoot string // non-empty custom contexts are rejected before preparation Workspace bool // template hint for workspace-aware source copying } @@ -41,7 +39,7 @@ type DockerEnv struct { Value string } -// BuildGoDocker generates templates and builds a Docker image for a Go service. +// BuildGoDocker emits a Docker build recipe for a Go service. func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, req *builderv0.BuildRequest, location string, requirements *builders.Dependencies, builderFS embed.FS, @@ -49,13 +47,17 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, w := wool.Get(ctx).In("golang.BuildGoDocker") + if !services.BuildPlanRequested(req) { + return builder.BuildError(fmt.Errorf("BuildRequest.output_directory is required for image recipes")) + } + dockerRequest, err := builder.DockerBuildRequest(ctx, req) if err != nil { return nil, w.Wrapf(err, "docker build request") } image := builder.DockerImage(dockerRequest) - w.Debug("building docker image", wool.Field("image", image.FullName())) + w.Debug("preparing docker image recipe", wool.Field("image", image.FullName())) if !dockerhelpers.IsValidDockerImageName(image.Name) { return builder.BuildError(fmt.Errorf("invalid docker image name: %s", image.Name)) @@ -70,87 +72,18 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } - _ = shared.DeleteFile(ctx, location+"/builder/Dockerfile") - - err = builder.Templates(ctx, docker, services.WithBuilder(builderFS)) - if err != nil { - return builder.BuildError(err) + if docker.ContextRoot != "" { + return builder.BuildError(fmt.Errorf("custom Docker context root %q is not supported by image recipes", docker.ContextRoot)) } - // When the caller owns the build (output_directory set), emit the recipe and - // let the caller run docker buildx instead of building the image in-process. - // A custom ContextRoot builds from a directory other than the service dir, so - // the "context is the service directory" recipe model does not hold — fall - // through to the in-process build, and the caller uses its legacy push path. - if services.BuildPlanRequested(req) { - if docker.ContextRoot == "" { - return builder.SingleImageBuildResponse(req, image.FullName()) - } - // The caller asked for a recipe but a custom ContextRoot forces the legacy - // in-process build; surface it so a caller expecting a plan isn't left - // wondering why it got a DockerBuildResult instead. - w.Warn("recipe emission requested but skipped: service uses a custom Docker context root; building in-process", - wool.Field("context_root", docker.ContextRoot)) - } + _ = shared.DeleteFile(ctx, location+"/builder/Dockerfile") - configuration, err := goDockerBuilderConfiguration(location, image, w, docker) - if err != nil { - return builder.BuildError(err) - } - configuration.Cache = dockerRequest.GetCache() - configuration.BuildxBuilder = dockerRequest.GetBuildxBuilder() - b, err := dockerhelpers.NewBuilder(configuration) - if err != nil { - return builder.BuildError(err) - } - _, err = b.Build(ctx) + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS)) if err != nil { return builder.BuildError(err) } - builder.WithDockerImages(image) - resp, err := builder.BuildResponse() - if resp != nil { - resp.BuildxBuilder = dockerRequest.GetBuildxBuilder() - } - if resp != nil && dockerRequest.GetCache() != nil { - resp.CacheContractVersion = dockerhelpers.CacheContractVersion - } - return resp, err -} -func goDockerBuilderConfiguration( - location string, - image *resources.DockerImage, - output io.Writer, - docker DockerTemplating, -) (dockerhelpers.BuilderConfiguration, error) { - contextRoot := docker.ContextRoot - if contextRoot == "" { - contextRoot = location - } - resolvedRoot, err := filepath.EvalSymlinks(contextRoot) - if err != nil { - return dockerhelpers.BuilderConfiguration{}, fmt.Errorf("resolve Docker context root: %w", err) - } - resolvedLocation, err := filepath.EvalSymlinks(location) - if err != nil { - return dockerhelpers.BuilderConfiguration{}, fmt.Errorf("resolve service directory: %w", err) - } - relative, err := filepath.Rel(resolvedRoot, resolvedLocation) - if err != nil || relative == ".." || strings.HasPrefix(relative, ".."+string(filepath.Separator)) { - return dockerhelpers.BuilderConfiguration{}, fmt.Errorf( - "service directory %q is outside Docker context root %q", - resolvedLocation, - resolvedRoot, - ) - } - return dockerhelpers.BuilderConfiguration{ - Root: resolvedRoot, - Dockerfile: filepath.ToSlash(filepath.Join(relative, "builder", "Dockerfile")), - Ignorefile: filepath.ToSlash(filepath.Join(relative, "builder", "dockerignore")), - Destination: image, - Output: output, - }, nil + return builder.SingleImageBuildResponse(req, image.FullName()) } // DeployGoKubernetes deploys a Go service to Kubernetes. diff --git a/runners/golang/agent_builder_cache_test.go b/runners/golang/agent_builder_cache_test.go index 71771088d..48dd50803 100644 --- a/runners/golang/agent_builder_cache_test.go +++ b/runners/golang/agent_builder_cache_test.go @@ -1,17 +1,12 @@ package golang import ( - "bytes" "context" "embed" - "encoding/json" - "fmt" "net" - "net/http" "os" "os/exec" "path/filepath" - "strings" "testing" "time" @@ -20,6 +15,7 @@ import ( basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" "github.com/codefly-dev/core/resources" + "github.com/codefly-dev/core/runners/rust" "github.com/stretchr/testify/require" "google.golang.org/grpc" "google.golang.org/grpc/credentials/insecure" @@ -28,106 +24,105 @@ import ( //go:embed templates/builder var cacheBuilderTemplates embed.FS -type legacyGoCacheBuilder struct { +type recipeBuilder struct { *services.DefaultBuilder - wrapper *services.BuilderWrapper - root string - location string + wrapper *services.BuilderWrapper + location string + language string + contextRoot string } -func (builder *legacyGoCacheBuilder) Build(ctx context.Context, request *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { - return BuildGoDocker(ctx, builder.wrapper, request, builder.location, &builders.Dependencies{}, cacheBuilderTemplates, "", "", func(options *DockerTemplating) { options.ContextRoot = builder.root }) +func (b *recipeBuilder) Build(ctx context.Context, req *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { + if b.language == "rust" { + return rust.BuildRustDocker(ctx, b.wrapper, req, b.location, &builders.Dependencies{}, cacheBuilderTemplates, "", "") + } + return BuildGoDocker(ctx, b.wrapper, req, b.location, &builders.Dependencies{}, cacheBuilderTemplates, "", "", func(options *DockerTemplating) { options.ContextRoot = b.contextRoot }) } -func (*legacyGoCacheBuilder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { +func (*recipeBuilder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { return &builderv0.BuildCapabilitiesResponse{BuildxSelection: true}, nil } -func TestCustomContextBuildUsesRequestedBuildxAndExportsCache(t *testing.T) { - if os.Getenv("CODEFLY_TEST_REGISTRY_CACHE") != "1" { - t.Skip("set CODEFLY_TEST_REGISTRY_CACHE=1 to build against a disposable registry") - } - ctx, cancel := context.WithTimeout(t.Context(), 4*time.Minute) - defer cancel() - run := func(args ...string) string { - t.Helper() - cmd := exec.CommandContext(ctx, "docker", args...) - var stderr bytes.Buffer - cmd.Stderr = &stderr - output, err := cmd.Output() - require.NoError(t, err, "%v: %s", args, stderr.String()) - return strings.TrimSpace(string(output)) +func TestImageRecipeOverGRPC(t *testing.T) { + t.Setenv("PATH", t.TempDir()) + for _, executable := range []string{"docker", "buildx"} { + _, err := exec.LookPath(executable) + require.Error(t, err) } - cleanup := func(args ...string) { - t.Helper() - cleanupCtx, stop := context.WithTimeout(context.Background(), time.Minute) - defer stop() - output, err := exec.CommandContext(cleanupCtx, "docker", args...).CombinedOutput() - require.NoError(t, err, "%s", output) - } - name := fmt.Sprintf("codefly-builder-%d", time.Now().UnixNano()) - run("network", "create", name) - t.Cleanup(func() { cleanup("network", "rm", name) }) - registry := name + ".local" - run("run", "-d", "--name", registry, "--network", name, "-p", "127.0.0.1::5000", "registry:2") - t.Cleanup(func() { cleanup("rm", "-f", registry) }) - address := run("port", registry, "5000/tcp") - client := &http.Client{Timeout: time.Second} - require.Eventually(t, func() bool { - response, err := client.Get("http://" + address + "/v2/") - if err != nil { - return false + for _, language := range []string{"go", "rust"} { + for _, selection := range []string{"explicit", "cache"} { + for _, scenario := range []string{"recipe", "missing-output", "relative-output", "custom-context"} { + if language == "rust" && scenario == "custom-context" { + continue + } + t.Run(language+"/"+selection+"/"+scenario, func(t *testing.T) { + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + defer cancel() + root := t.TempDir() + location := filepath.Join(root, "service") + output := filepath.Join(location, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + dockerfile := filepath.Join(output, "Dockerfile") + require.NoError(t, os.WriteFile(dockerfile, []byte("original recipe"), 0o644)) + base := &services.Base{} + require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) + wrapper := &services.BuilderWrapper{Base: base} + base.Builder = wrapper + base.SetDockerImage(&resources.DockerImage{Name: "app", Tag: "test"}) + agent := &recipeBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper, location: location, language: language} + dockerContext := &builderv0.DockerBuildContext{BuildxBuilder: "selected"} + if selection == "cache" { + dockerContext.BuildxBuilder = "cache-selected" + dockerContext.Cache = &builderv0.BuildCacheOptions{Backend: "registry", Scope: "test/app", Exports: []string{"registry.example.com/cache"}} + } + request := &builderv0.BuildRequest{OutputDirectory: output, BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: dockerContext}}} + expectedError := "" + switch scenario { + case "missing-output": + request.OutputDirectory = "" + expectedError = "output_directory is required" + case "relative-output": + request.OutputDirectory = "builder" + expectedError = "must be absolute" + case "custom-context": + agent.contextRoot = root + expectedError = "custom Docker context root" + } + server := grpc.NewServer() + builderv0.RegisterBuilderServer(server, agent) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, connection.Close()) }) + response, err := services.NewBuilderAgentClient(connection).Build(ctx, request) + if expectedError != "" { + if err != nil { + require.ErrorContains(t, err, expectedError) + } else { + require.Equal(t, builderv0.BuildStatus_ERROR, response.GetState().GetState()) + require.Contains(t, response.GetState().GetMessage(), expectedError) + } + contents, readErr := os.ReadFile(dockerfile) + require.NoError(t, readErr) + require.Equal(t, "original recipe", string(contents)) + entries, readErr := os.ReadDir(output) + require.NoError(t, readErr) + require.Len(t, entries, 1) + return + } + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + plan := response.GetResult().GetDockerBuildPlan() + require.NotNil(t, plan) + require.NoError(t, services.VerifyDockerBuildPlan(output, plan)) + require.Len(t, plan.GetRecipes(), 1) + require.Equal(t, "app:test", plan.GetRecipes()[0].GetImage()) + require.Equal(t, services.RecipeBuildPlatforms(), plan.GetRecipes()[0].GetPlatforms()) + }) + } } - closeErr := response.Body.Close() - return response.StatusCode == http.StatusOK && closeErr == nil - }, 20*time.Second, 100*time.Millisecond) - config := filepath.Join(t.TempDir(), "buildkitd.toml") - require.NoError(t, os.WriteFile(config, []byte(fmt.Sprintf("[registry.\"%s:5000\"]\n http = true\n", registry)), 0o600)) - run("buildx", "create", "--name", name, "--driver", "docker-container", "--driver-opt", "network="+name, "--buildkitd-config", config, "--bootstrap") - t.Cleanup(func() { cleanup("buildx", "rm", name) }) - t.Setenv("BUILDX_BUILDER", "must-not-use-ambient-builder") - root := t.TempDir() - location := filepath.Join(root, "service") - require.NoError(t, os.Mkdir(location, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(root, "app"), []byte("legacy cached image"), 0o644)) - base := &services.Base{} - require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) - wrapper := &services.BuilderWrapper{Base: base} - base.Builder = wrapper - image := &resources.DockerImage{Name: name, Tag: "test"} - base.SetDockerImage(image) - request := &builderv0.BuildRequest{ - OutputDirectory: filepath.Join(location, "builder"), - BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: &builderv0.DockerBuildContext{ - BuildxBuilder: name, - Cache: &builderv0.BuildCacheOptions{Backend: "registry", Scope: "test/app", Exports: []string{registry + ":5000/cache"}}, - }}}, - } - server := grpc.NewServer() - builderv0.RegisterBuilderServer(server, &legacyGoCacheBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper, root: root, location: location}) - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - go func() { _ = server.Serve(listener) }() - t.Cleanup(server.Stop) - connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, connection.Close()) }) - response, err := services.NewBuilderAgentClient(connection).Build(ctx, request) - require.NoError(t, err) - require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState()) - t.Cleanup(func() { cleanup("image", "rm", image.FullName()) }) - require.Nil(t, response.GetResult().GetDockerBuildPlan()) - require.Equal(t, name, response.GetBuildxBuilder()) - require.Equal(t, "registry-v1", response.GetCacheContractVersion()) - require.Equal(t, "amd64", run("image", "inspect", "--format", "{{.Architecture}}", image.FullName())) - tags, err := client.Get("http://" + address + "/v2/cache/tags/list") - require.NoError(t, err) - defer func() { require.NoError(t, tags.Body.Close()) }() - require.Equal(t, http.StatusOK, tags.StatusCode) - var manifest struct { - Tags []string `json:"tags"` } - require.NoError(t, json.NewDecoder(tags.Body).Decode(&manifest)) - require.Len(t, manifest.Tags, 1) - require.True(t, strings.HasPrefix(manifest.Tags[0], "codefly-")) } diff --git a/runners/golang/agent_builder_test.go b/runners/golang/agent_builder_test.go deleted file mode 100644 index a3f207e77..000000000 --- a/runners/golang/agent_builder_test.go +++ /dev/null @@ -1,43 +0,0 @@ -package golang - -import ( - "io" - "os" - "path/filepath" - "testing" - - "github.com/codefly-dev/core/resources" - "github.com/stretchr/testify/require" -) - -func TestGoDockerBuilderConfigurationUsesExactWorkspaceContext(t *testing.T) { - workspace := t.TempDir() - service := filepath.Join(workspace, "modules", "users", "services", "forge-edge") - require.NoError(t, os.MkdirAll(service, 0o755)) - - configuration, err := goDockerBuilderConfiguration( - service, - &resources.DockerImage{Name: "registry.example.com/forge-edge", Tag: "test"}, - io.Discard, - DockerTemplating{ContextRoot: workspace, Workspace: true}, - ) - require.NoError(t, err) - resolvedWorkspace, err := filepath.EvalSymlinks(workspace) - require.NoError(t, err) - require.Equal(t, resolvedWorkspace, configuration.Root) - require.Equal(t, "modules/users/services/forge-edge/builder/Dockerfile", configuration.Dockerfile) - require.Equal(t, "modules/users/services/forge-edge/builder/dockerignore", configuration.Ignorefile) -} - -func TestGoDockerBuilderConfigurationRejectsServiceOutsideContext(t *testing.T) { - workspace := t.TempDir() - service := t.TempDir() - - _, err := goDockerBuilderConfiguration( - service, - &resources.DockerImage{Name: "registry.example.com/forge-edge", Tag: "test"}, - io.Discard, - DockerTemplating{ContextRoot: workspace, Workspace: true}, - ) - require.ErrorContains(t, err, "outside Docker context root") -} diff --git a/runners/rust/agent_builder.go b/runners/rust/agent_builder.go index 055f94020..4c098088d 100644 --- a/runners/rust/agent_builder.go +++ b/runners/rust/agent_builder.go @@ -34,8 +34,7 @@ type DockerEnv struct { Value string } -// BuildRustDocker generates templates and builds a Docker image for a Rust -// service. Mirrors golang.BuildGoDocker. +// BuildRustDocker emits a Docker build recipe for a Rust service. func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, req *builderv0.BuildRequest, location string, requirements *builders.Dependencies, builderFS embed.FS, @@ -43,13 +42,17 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, w := wool.Get(ctx).In("rust.BuildRustDocker") + if !services.BuildPlanRequested(req) { + return builder.BuildError(fmt.Errorf("BuildRequest.output_directory is required for image recipes")) + } + dockerRequest, err := builder.DockerBuildRequest(ctx, req) if err != nil { return nil, w.Wrapf(err, "docker build request") } image := builder.DockerImage(dockerRequest) - w.Debug("building docker image", wool.Field("image", image.FullName())) + w.Debug("preparing docker image recipe", wool.Field("image", image.FullName())) if !dockerhelpers.IsValidDockerImageName(image.Name) { return builder.BuildError(fmt.Errorf("invalid docker image name: %s", image.Name)) @@ -71,37 +74,7 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, return builder.BuildError(err) } - // When the caller owns the build (output_directory set), emit the recipe and - // let the caller run docker buildx instead of building the image in-process. - if services.BuildPlanRequested(req) { - return builder.SingleImageBuildResponse(req, image.FullName()) - } - - b, err := dockerhelpers.NewBuilder(dockerhelpers.BuilderConfiguration{ - Root: location, - Cache: dockerRequest.GetCache(), - BuildxBuilder: dockerRequest.GetBuildxBuilder(), - Dockerfile: "builder/Dockerfile", - Ignorefile: "builder/dockerignore", - Destination: image, - Output: w, - }) - if err != nil { - return builder.BuildError(err) - } - _, err = b.Build(ctx) - if err != nil { - return builder.BuildError(err) - } - builder.WithDockerImages(image) - resp, err := builder.BuildResponse() - if resp != nil { - resp.BuildxBuilder = dockerRequest.GetBuildxBuilder() - } - if resp != nil && dockerRequest.GetCache() != nil { - resp.CacheContractVersion = dockerhelpers.CacheContractVersion - } - return resp, err + return builder.SingleImageBuildResponse(req, image.FullName()) } // DeployRustKubernetes deploys a Rust service to Kubernetes. Identical in From e95db35265b09554016b1fee407efd2f4be72bb0 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sat, 12 Sep 2026 08:10:43 -0400 Subject: [PATCH 2/6] fix: render recipes into the directory that the plan verifies Render directly into output_directory instead of overwriting the service builder directory and certifying potentially stale files elsewhere. Cover fresh and stale alternative destinations over gRPC for both runners. --- runners/golang/agent_builder.go | 7 ++----- runners/golang/agent_builder_cache_test.go | 18 ++++++++++++++++-- runners/rust/agent_builder.go | 7 ++----- 3 files changed, 20 insertions(+), 12 deletions(-) diff --git a/runners/golang/agent_builder.go b/runners/golang/agent_builder.go index 4143f842e..a97df5dcb 100644 --- a/runners/golang/agent_builder.go +++ b/runners/golang/agent_builder.go @@ -10,7 +10,6 @@ import ( "github.com/codefly-dev/core/agents/services" "github.com/codefly-dev/core/builders" "github.com/codefly-dev/core/resources" - "github.com/codefly-dev/core/shared" "github.com/codefly-dev/core/wool" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -41,7 +40,7 @@ type DockerEnv struct { // BuildGoDocker emits a Docker build recipe for a Go service. func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, - req *builderv0.BuildRequest, location string, + req *builderv0.BuildRequest, _ string, requirements *builders.Dependencies, builderFS embed.FS, goVersion, alpineVersion string, opts ...func(*DockerTemplating)) (*builderv0.BuildResponse, error) { @@ -76,9 +75,7 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, return builder.BuildError(fmt.Errorf("custom Docker context root %q is not supported by image recipes", docker.ContextRoot)) } - _ = shared.DeleteFile(ctx, location+"/builder/Dockerfile") - - err = builder.Templates(ctx, docker, services.WithBuilder(builderFS)) + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS).WithDestination("%s", req.GetOutputDirectory())) if err != nil { return builder.BuildError(err) } diff --git a/runners/golang/agent_builder_cache_test.go b/runners/golang/agent_builder_cache_test.go index 48dd50803..be15269d9 100644 --- a/runners/golang/agent_builder_cache_test.go +++ b/runners/golang/agent_builder_cache_test.go @@ -51,7 +51,7 @@ func TestImageRecipeOverGRPC(t *testing.T) { } for _, language := range []string{"go", "rust"} { for _, selection := range []string{"explicit", "cache"} { - for _, scenario := range []string{"recipe", "missing-output", "relative-output", "custom-context"} { + for _, scenario := range []string{"recipe", "empty-destination", "stale-destination", "missing-output", "relative-output", "custom-context"} { if language == "rust" && scenario == "custom-context" { continue } @@ -78,6 +78,12 @@ func TestImageRecipeOverGRPC(t *testing.T) { request := &builderv0.BuildRequest{OutputDirectory: output, BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: dockerContext}}} expectedError := "" switch scenario { + case "empty-destination", "stale-destination": + request.OutputDirectory = filepath.Join(t.TempDir(), "recipes") + if scenario == "stale-destination" { + require.NoError(t, os.MkdirAll(request.OutputDirectory, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(request.OutputDirectory, "Dockerfile"), []byte("FROM stale-image\n"), 0o644)) + } case "missing-output": request.OutputDirectory = "" expectedError = "output_directory is required" @@ -117,7 +123,15 @@ func TestImageRecipeOverGRPC(t *testing.T) { require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) plan := response.GetResult().GetDockerBuildPlan() require.NotNil(t, plan) - require.NoError(t, services.VerifyDockerBuildPlan(output, plan)) + require.NoError(t, services.VerifyDockerBuildPlan(request.OutputDirectory, plan)) + fresh, readErr := os.ReadFile(filepath.Join(request.OutputDirectory, "Dockerfile")) + require.NoError(t, readErr) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) + if request.OutputDirectory != output { + original, readErr := os.ReadFile(dockerfile) + require.NoError(t, readErr) + require.Equal(t, "original recipe", string(original)) + } require.Len(t, plan.GetRecipes(), 1) require.Equal(t, "app:test", plan.GetRecipes()[0].GetImage()) require.Equal(t, services.RecipeBuildPlatforms(), plan.GetRecipes()[0].GetPlatforms()) diff --git a/runners/rust/agent_builder.go b/runners/rust/agent_builder.go index 4c098088d..7ad810be5 100644 --- a/runners/rust/agent_builder.go +++ b/runners/rust/agent_builder.go @@ -10,7 +10,6 @@ import ( "github.com/codefly-dev/core/agents/services" "github.com/codefly-dev/core/builders" "github.com/codefly-dev/core/resources" - "github.com/codefly-dev/core/shared" "github.com/codefly-dev/core/wool" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" @@ -36,7 +35,7 @@ type DockerEnv struct { // BuildRustDocker emits a Docker build recipe for a Rust service. func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, - req *builderv0.BuildRequest, location string, + req *builderv0.BuildRequest, _ string, requirements *builders.Dependencies, builderFS embed.FS, rustVersion, alpineVersion string, opts ...func(*DockerTemplating)) (*builderv0.BuildResponse, error) { @@ -67,9 +66,7 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } - _ = shared.DeleteFile(ctx, location+"/builder/Dockerfile") - - err = builder.Templates(ctx, docker, services.WithBuilder(builderFS)) + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS).WithDestination("%s", req.GetOutputDirectory())) if err != nil { return builder.BuildError(err) } From 316f204a5d739dfe6cf862b8dd7a7feebfb403f8 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sat, 12 Sep 2026 08:15:32 -0400 Subject: [PATCH 3/6] chore: prepare Core 0.3.28 for recipe-only agent builds --- version/info.codefly.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/version/info.codefly.yaml b/version/info.codefly.yaml index 63d3b5b3b..3a0891ffb 100644 --- a/version/info.codefly.yaml +++ b/version/info.codefly.yaml @@ -1 +1 @@ -version: 0.3.27 +version: 0.3.28 From 177cb87e85eef92b3f019d1f4afb650a5c219cb9 Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sat, 12 Sep 2026 08:26:21 -0400 Subject: [PATCH 4/6] fix: replace recipe Dockerfiles without following stale symlinks Preserve the old unlink-before-render behavior in the actual output directory and propagate removal errors. The destination change must not turn an existing Dockerfile symlink into an out-of-tree overwrite. --- runners/golang/agent_builder.go | 6 ++++++ runners/golang/agent_builder_cache_test.go | 8 ++++++-- runners/rust/agent_builder.go | 6 ++++++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/runners/golang/agent_builder.go b/runners/golang/agent_builder.go index a97df5dcb..32fc50768 100644 --- a/runners/golang/agent_builder.go +++ b/runners/golang/agent_builder.go @@ -4,6 +4,8 @@ import ( "context" "embed" "fmt" + "os" + "path/filepath" "strings" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" @@ -75,6 +77,10 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, return builder.BuildError(fmt.Errorf("custom Docker context root %q is not supported by image recipes", docker.ContextRoot)) } + if err = os.Remove(filepath.Join(req.GetOutputDirectory(), "Dockerfile")); err != nil && !os.IsNotExist(err) { + return builder.BuildError(err) + } + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS).WithDestination("%s", req.GetOutputDirectory())) if err != nil { return builder.BuildError(err) diff --git a/runners/golang/agent_builder_cache_test.go b/runners/golang/agent_builder_cache_test.go index be15269d9..082cbc4bb 100644 --- a/runners/golang/agent_builder_cache_test.go +++ b/runners/golang/agent_builder_cache_test.go @@ -51,7 +51,7 @@ func TestImageRecipeOverGRPC(t *testing.T) { } for _, language := range []string{"go", "rust"} { for _, selection := range []string{"explicit", "cache"} { - for _, scenario := range []string{"recipe", "empty-destination", "stale-destination", "missing-output", "relative-output", "custom-context"} { + for _, scenario := range []string{"recipe", "empty-destination", "stale-destination", "symlink-destination", "missing-output", "relative-output", "custom-context"} { if language == "rust" && scenario == "custom-context" { continue } @@ -78,12 +78,16 @@ func TestImageRecipeOverGRPC(t *testing.T) { request := &builderv0.BuildRequest{OutputDirectory: output, BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: dockerContext}}} expectedError := "" switch scenario { - case "empty-destination", "stale-destination": + case "empty-destination", "stale-destination", "symlink-destination": request.OutputDirectory = filepath.Join(t.TempDir(), "recipes") if scenario == "stale-destination" { require.NoError(t, os.MkdirAll(request.OutputDirectory, 0o755)) require.NoError(t, os.WriteFile(filepath.Join(request.OutputDirectory, "Dockerfile"), []byte("FROM stale-image\n"), 0o644)) } + if scenario == "symlink-destination" { + require.NoError(t, os.MkdirAll(request.OutputDirectory, 0o755)) + require.NoError(t, os.Symlink(dockerfile, filepath.Join(request.OutputDirectory, "Dockerfile"))) + } case "missing-output": request.OutputDirectory = "" expectedError = "output_directory is required" diff --git a/runners/rust/agent_builder.go b/runners/rust/agent_builder.go index 7ad810be5..eef07980c 100644 --- a/runners/rust/agent_builder.go +++ b/runners/rust/agent_builder.go @@ -4,6 +4,8 @@ import ( "context" "embed" "fmt" + "os" + "path/filepath" "strings" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" @@ -66,6 +68,10 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } + if err = os.Remove(filepath.Join(req.GetOutputDirectory(), "Dockerfile")); err != nil && !os.IsNotExist(err) { + return builder.BuildError(err) + } + err = builder.Templates(ctx, docker, services.WithBuilder(builderFS).WithDestination("%s", req.GetOutputDirectory())) if err != nil { return builder.BuildError(err) From aeae945d9521d553412f20c7e4fcc3f7b9177efd Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sun, 13 Sep 2026 14:47:10 -0400 Subject: [PATCH 5/6] fix: contain recipe emission to the files a build actually writes (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The recipe-only runners guarded one hardcoded filename, inventoried an entire committed directory, and resolved the build context against the wrong root. Each of those is fixed at its cause. Symlink write-escape. The renderer opens every destination with O_CREATE|O_TRUNC and tests existence with os.Stat, both of which follow symlinks, so rendering over a symlinked destination truncates its target anywhere on disk. The guard removed only "Dockerfile", leaving every other file in the template set exposed; the inventory's symlink rejection runs after the write and so detects the escape instead of preventing it. output_directory is the service's committed builder/ directory, so a symlink there is ordinary repository content. PrepareRecipeDestination now unlinks every path the template set will render, before rendering. Inventory scope. A plan inventoried whatever sat in output_directory, so a .DS_Store changed the digest between macOS and CI, and an unrelated symlink aborted the build naming a file with nothing to do with the image. Plans now inventory the files the build emitted, and VerifyDockerBuildPlan re-hashes exactly the paths the plan declares. Drift in a declared file — content, mode, replacement by a symlink, removal — is still rejected. This changes the digest algorithm, so the contract version moves to v3, which reports an older plan as an actionable mismatch rather than as tampering. BuildDockerBuildPlan keeps its whole-tree contract for callers that assemble a destination themselves. Context base. dockerfile and dockerignore are output_directory-relative but context is resolved by the executor against the service directory, which is why "." means "build the service". validateRecipes statted it against output_directory: that proved nothing for "." and rejected a valid context such as "code" whenever the service had no builder/code. The check is now lexical only, and the base is stated in the proto. Stale dockerignore. The recipe referenced any dockerignore found on disk. The executor copies the referenced ignore to the path buildx discovers and applies it, so one left by an older template set silently kept files out of the image. The recipe now references an ignore only when this build emitted it. Silent break for adopters. BuildGoDocker/BuildRustDocker kept an ignored location parameter, so agents kept compiling across a core bump while an empty output_directory flipped from building an image to erroring and the Dockerfile destination moved. The parameter is removed, so every call site fails to compile and is revisited. Workspace rejection. ContextRoot and Workspace carry the same requirement and agents set them together. Rejecting only ContextRoot let Workspace through and moved the failure into the executor as an opaque COPY error; both are rejected before preparation. Buildx selection coverage. Deleting TestCustomContextBuildUsesRequestedBuildx- AndExportsCache removed the only assertion that --builder reaches docker, for a helper the CLI still uses. The registry-cache integration run now points BUILDX_BUILDER at a nonexistent builder and passes the real one explicitly, and unit tests assert the argv directly. build-cache.yml carries that instead of a recipe test go.yml already runs through ./... Tests: rust recipe coverage moves into runners/rust with its own two-file template set; the explicit/cache matrix that asserted nothing is replaced by one test that the recipe is independent of caller transport policy. Co-Authored-By: Claude Opus 5 (1M context) --- .github/workflows/build-cache.yml | 4 +- .../helpers/docker/buildx_selection_test.go | 107 ++++++++ .../helpers/docker/cache_integration_test.go | 9 +- agents/services/base_builder.go | 7 +- agents/services/builder_cache_test.go | 4 +- agents/services/docker_recipe.go | 211 ++++++++++++--- agents/services/docker_recipe_prepare_test.go | 80 ++++++ agents/services/docker_recipe_scope_test.go | 123 +++++++++ .../docker_recipe_singleimage_test.go | 26 +- .../templates/builder/Dockerfile.tmpl | 2 + .../templates/builder/dockerignore | 1 + docs/build-cache.md | 26 +- .../codefly/services/builder/v0/docker.pb.go | 12 +- .../codefly/services/builder/v0/docker.proto | 12 +- runners/golang/agent_builder.go | 22 +- runners/golang/agent_builder_cache_test.go | 146 ----------- runners/golang/agent_builder_recipe_test.go | 246 ++++++++++++++++++ runners/rust/agent_builder.go | 9 +- runners/rust/agent_builder_recipe_test.go | 169 ++++++++++++ .../rust/templates/builder/Dockerfile.tmpl | 2 + runners/rust/templates/builder/dockerignore | 1 + 21 files changed, 1007 insertions(+), 212 deletions(-) create mode 100644 agents/helpers/docker/buildx_selection_test.go create mode 100644 agents/services/docker_recipe_prepare_test.go create mode 100644 agents/services/docker_recipe_scope_test.go create mode 100644 agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl create mode 100644 agents/services/testdata/recipetemplates/templates/builder/dockerignore delete mode 100644 runners/golang/agent_builder_cache_test.go create mode 100644 runners/golang/agent_builder_recipe_test.go create mode 100644 runners/rust/agent_builder_recipe_test.go create mode 100644 runners/rust/templates/builder/Dockerfile.tmpl create mode 100644 runners/rust/templates/builder/dockerignore diff --git a/.github/workflows/build-cache.yml b/.github/workflows/build-cache.yml index 66c4dfc18..f14c98feb 100644 --- a/.github/workflows/build-cache.yml +++ b/.github/workflows/build-cache.yml @@ -29,9 +29,9 @@ jobs: - uses: actions/setup-go@924ae3a1cded613372ab5595356fb5720e22ba16 # v6.5.0 with: go-version: "1.27.0" - - name: Verify recipe-only runners without Docker on PATH + - name: Verify Buildx selection and cache arguments reach the docker CLI if: matrix.language == 'go' - run: go test ./runners/golang -run '^TestImageRecipe' -count=1 -v -timeout=5m + run: go test ./agents/helpers/docker -run '^TestBuildx|^TestBuildOmits|^TestBuildCarries' -count=1 -v -timeout=5m - name: Reject hidden inputs in exported cache layers env: CODEFLY_TEST_REGISTRY_CACHE: "1" diff --git a/agents/helpers/docker/buildx_selection_test.go b/agents/helpers/docker/buildx_selection_test.go new file mode 100644 index 000000000..4dd51b82a --- /dev/null +++ b/agents/helpers/docker/buildx_selection_test.go @@ -0,0 +1,107 @@ +package docker + +import ( + "bytes" + "os" + "path/filepath" + "strings" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// recordedDocker puts a fake `docker` on PATH that appends each invocation's +// arguments to a log and succeeds, so the argv this package builds can be +// asserted without a daemon, a registry, or a real buildx builder. +func recordedDocker(t *testing.T) func() []string { + t.Helper() + bin := t.TempDir() + log := filepath.Join(t.TempDir(), "argv") + script := "#!/bin/sh\nprintf '%s\\n' \"$*\" >> " + log + "\nexit 0\n" + require.NoError(t, os.WriteFile(filepath.Join(bin, "docker"), []byte(script), 0o755)) + t.Setenv("PATH", bin) + return func() []string { + contents, err := os.ReadFile(log) + require.NoError(t, err) + var invocations []string + for _, line := range strings.Split(strings.TrimSpace(string(contents)), "\n") { + if line != "" { + invocations = append(invocations, line) + } + } + return invocations + } +} + +func buildInvocation(t *testing.T, invocations []string) string { + t.Helper() + for _, invocation := range invocations { + if strings.HasPrefix(invocation, "buildx build") { + return invocation + } + } + require.FailNow(t, "no `docker buildx build` invocation was recorded", "recorded: %v", invocations) + return "" +} + +func backendRequest() backendBuildRequest { + return backendBuildRequest{ + Platform: "linux/amd64", + Dockerfile: "/tmp/svc/builder/Dockerfile", + Tag: "repo/app:test", + Context: "/tmp/svc", + Output: &bytes.Buffer{}, + } +} + +// The caller provisions its own Buildx builder and names it in the request; the +// build must run on that builder and not on whatever ambient builder the host +// has selected. Nothing else in this repository asserts that --builder reaches +// the docker CLI, so a regression here would otherwise surface only as a build +// running on the wrong builder — silently succeeding against the wrong registry +// network, or failing to export cache at all. +func TestBuildxBuilderSelectionReachesTheDockerCLI(t *testing.T) { + recorded := recordedDocker(t) + + request := backendRequest() + request.BuildxBuilder = "codefly-provisioned" + require.NoError(t, dockerCLIBackend{}.Build(t.Context(), request)) + + invocation := buildInvocation(t, recorded()) + require.Contains(t, invocation, "--builder codefly-provisioned") + require.Contains(t, invocation, "--platform linux/amd64") + require.Contains(t, invocation, "-t repo/app:test") +} + +// Without an explicit selection the flag must be absent entirely: passing +// `--builder ""` would fail, and passing a default would silently override the +// operator's own buildx selection. +func TestBuildOmitsBuilderFlagWithoutSelection(t *testing.T) { + recorded := recordedDocker(t) + + require.NoError(t, dockerCLIBackend{}.Build(t.Context(), backendRequest())) + + require.NotContains(t, buildInvocation(t, recorded()), "--builder") +} + +// Cache policy and builder selection are independent: selecting a builder must +// not drop the cache arguments, which is the combination the registry-cache +// conformance job exercises against a real registry. +func TestBuildCarriesCacheArgumentsAlongsideBuilderSelection(t *testing.T) { + recorded := recordedDocker(t) + + request := backendRequest() + request.BuildxBuilder = "codefly-provisioned" + request.Cache = &builderv0.BuildCacheOptions{ + Backend: "registry", Scope: "workspace/service/app", + Imports: []string{"registry.example.com/cache"}, + Exports: []string{"registry.example.com/cache"}, + } + require.NoError(t, dockerCLIBackend{}.Build(t.Context(), request)) + + invocation := buildInvocation(t, recorded()) + require.Contains(t, invocation, "--builder codefly-provisioned") + require.Contains(t, invocation, "--cache-from") + require.Contains(t, invocation, "--cache-to") +} diff --git a/agents/helpers/docker/cache_integration_test.go b/agents/helpers/docker/cache_integration_test.go index 2c187ad9f..ff01a4d5e 100644 --- a/agents/helpers/docker/cache_integration_test.go +++ b/agents/helpers/docker/cache_integration_test.go @@ -129,7 +129,12 @@ func TestLanguageRegistryCacheAcrossCleanBuilders(t *testing.T) { for i := first; i <= last; i++ { builderName := fmt.Sprintf("%s-%d", name, i) run("buildx", "create", "--name", builderName, "--driver", "docker-container", "--driver-opt", "network="+name, "--buildkitd-config", config) - t.Setenv("BUILDX_BUILDER", builderName) + // Point the ambient selection at a builder that does not exist and pass + // the real one explicitly, so this run proves BuilderConfiguration. + // BuildxBuilder actually reaches docker buildx against a live registry. + // Falling back to the ambient builder would fail outright rather than + // quietly exporting cache from the wrong one. + t.Setenv("BUILDX_BUILDER", "must-not-use-ambient-builder") t.Cleanup(func() { out, err := exec.Command("docker", "buildx", "rm", builderName).CombinedOutput() require.NoError(t, err, "%s", out) @@ -165,7 +170,7 @@ func TestLanguageRegistryCacheAcrossCleanBuilders(t *testing.T) { out, err := exec.Command("docker", "image", "rm", tag).CombinedOutput() require.NoError(t, err, "%s", out) }) - builder, err := NewBuilder(BuilderConfiguration{Root: root, Dockerfile: "Dockerfile", Destination: resources.NewDockerImage(tag), Output: &output, Platform: "linux/amd64", Cache: policy}) + builder, err := NewBuilder(BuilderConfiguration{Root: root, Dockerfile: "Dockerfile", Destination: resources.NewDockerImage(tag), Output: &output, Platform: "linux/amd64", Cache: policy, BuildxBuilder: builderName}) require.NoError(t, err) result, err := builder.Build(ctx) require.NoError(t, err, "%s", output.String()) diff --git a/agents/services/base_builder.go b/agents/services/base_builder.go index 252abf592..77d8ba310 100644 --- a/agents/services/base_builder.go +++ b/agents/services/base_builder.go @@ -261,8 +261,11 @@ func (s *BuilderWrapper) WithBuildPlan(plan *builderv0.DockerBuildPlan) { // SingleImageBuildResponse records a single-image recipe over the caller's // output_directory and returns the build response. The CLI executes the plan. -func (s *BuilderWrapper) SingleImageBuildResponse(req *builderv0.BuildRequest, image string) (*builderv0.BuildResponse, error) { - plan, err := SingleImageBuildPlan(req.GetOutputDirectory(), image, RecipeBuildPlatforms()) +// emitted is the path set the caller just rendered, from PrepareRecipeDestination: +// it scopes the plan's inventory to the files this build wrote and decides whether +// the recipe references a dockerignore at all. +func (s *BuilderWrapper) SingleImageBuildResponse(req *builderv0.BuildRequest, image string, emitted []string) (*builderv0.BuildResponse, error) { + plan, err := SingleImageBuildPlan(req.GetOutputDirectory(), image, RecipeBuildPlatforms(), emitted) if err != nil { return s.BuildError(err) } diff --git a/agents/services/builder_cache_test.go b/agents/services/builder_cache_test.go index 1a684c93c..f5c5f3d23 100644 --- a/agents/services/builder_cache_test.go +++ b/agents/services/builder_cache_test.go @@ -51,8 +51,8 @@ func TestBuildCacheRequiresAgentAcknowledgement(t *testing.T) { } func TestRecipeAgentsDoNotAcknowledgeCallerOwnedCacheExecution(t *testing.T) { - directory := writeRecipeTree(t, true) - plan, err := SingleImageBuildPlan(directory, "repo/app:v1", RecipeBuildPlatforms()) + directory, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(directory, "repo/app:v1", RecipeBuildPlatforms(), emitted) require.NoError(t, err) client := &cacheBuilderClient{plan: plan} agent := &BuilderAgent{BuilderClient: client} diff --git a/agents/services/docker_recipe.go b/agents/services/docker_recipe.go index fa3fcac3a..1c1c382ee 100644 --- a/agents/services/docker_recipe.go +++ b/agents/services/docker_recipe.go @@ -5,11 +5,15 @@ import ( "encoding/hex" "fmt" "io" + "io/fs" "os" + "path" "path/filepath" "sort" "strings" + "github.com/codefly-dev/core/templates" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" ) @@ -18,8 +22,11 @@ import ( // the aggregate-digest algorithm changes, so a digest produced by an older core // is reported as a contract mismatch (a clear, actionable error) rather than as // a digest mismatch (indistinguishable from tampering). v2 covers the recipes -// and per-file mode in the digest; v1 covered only file paths and content. -const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v2" +// and per-file mode in the digest; v1 covered only file paths and content. v3 +// scopes the inventory to the files the plan declares rather than to whatever +// happens to sit in the destination directory, so unrelated content in a +// committed recipe directory can neither perturb the digest nor fail the build. +const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v3" // ValidateBuildRequestOutputDirectory enforces the BuildRequest.output_directory // contract: when set, the destination must be an absolute path the caller owns. @@ -44,6 +51,83 @@ func ValidateBuildRequestOutputDirectory(req *builderv0.BuildRequest) error { return nil } +// BuilderTemplateRoot is the subtree of an agent's embedded template filesystem +// that WithBuilder renders. PrepareRecipeDestination and WithBuilder must agree +// on it, or the prepared path set would not match the rendered one. +const BuilderTemplateRoot = "templates/builder" + +// PrepareRecipeDestination unlinks every path the builder template set is about +// to render into outputDirectory and returns that path set, destination-relative +// and sorted. +// +// It must run before the templates are rendered. The template writer opens each +// destination with os.OpenFile(O_WRONLY|O_CREATE|O_TRUNC) and tests existence +// with os.Stat, and both follow symlinks: rendering over a symlinked destination +// truncates the symlink's target, which can sit anywhere on disk. Because +// output_directory is the service's committed builder/ directory, a symlink +// there is ordinary repository content, not an attack precondition. Unlinking +// first (os.Remove never follows the final component) makes every render land +// on a fresh regular file inside the caller-owned directory. +// +// Inventorying the returned set rather than walking the destination is what +// keeps a plan's digest a function of the recipe: unrelated files in a committed +// directory — editor backups, .DS_Store, a symlink to shared config — are +// neither hashed nor rejected, because none of them is a file this build wrote. +func PrepareRecipeDestination(builderFS fs.FS, outputDirectory string) ([]string, error) { + var emitted []string + namer := templates.CutTemplateSuffix{} + err := fs.WalkDir(builderFS, BuilderTemplateRoot, func(entry string, info fs.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if info.IsDir() { + return nil + } + // fs.WalkDir always yields slash-separated paths, so the template-relative + // name is derived with path, not filepath, and converted once on the way to + // the destination below. + relative, found := strings.CutPrefix(entry, BuilderTemplateRoot+"/") + if !found { + return fmt.Errorf("template entry %q is outside %q", entry, BuilderTemplateRoot) + } + name := namer.NewName(path.Clean(relative)) + emitted = append(emitted, name) + if removeErr := os.Remove(filepath.Join(outputDirectory, filepath.FromSlash(name))); removeErr != nil && !os.IsNotExist(removeErr) { + return fmt.Errorf("cannot replace recipe file %q: %w", name, removeErr) + } + return nil + }) + if err != nil { + return nil, fmt.Errorf("prepare recipe destination: %w", err) + } + if len(emitted) == 0 { + return nil, fmt.Errorf("builder template set %q is empty; nothing to emit", BuilderTemplateRoot) + } + sort.Strings(emitted) + return emitted, nil +} + +// BuildEmittedDockerBuildPlan builds a plan whose inventory is exactly the files +// this build emitted — the set PrepareRecipeDestination returned — instead of +// whatever the destination directory currently holds. Use it from any runner +// that renders a known template set; BuildDockerBuildPlan remains for callers +// that assemble a destination tree by other means and want all of it covered. +func BuildEmittedDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRecipe, emitted []string) (*builderv0.DockerBuildPlan, error) { + files, err := inventoryRecipePaths(destination, emitted) + if err != nil { + return nil, fmt.Errorf("inventory recipe files: %w", err) + } + if err := validateRecipes(destination, recipes, files); err != nil { + return nil, err + } + return &builderv0.DockerBuildPlan{ + Recipes: recipes, + Files: files, + Digest: aggregateRecipeDigest(recipes, files), + ContractVersion: DockerBuildRecipeContractVersion, + }, nil +} + // BuildDockerBuildPlan inventories the recipe tree an agent wrote to destination // and returns a build plan: the ordered recipes plus the canonical sorted file // inventory with per-file sha256 digests and an aggregate digest that is a @@ -52,6 +136,13 @@ func ValidateBuildRequestOutputDirectory(req *builderv0.BuildRequest) error { // returned, so a plan that passes is buildable. The caller (the CLI) verifies the // on-disk tree against the plan before running docker buildx, so the recipe is a // durable, first-class artifact rather than an image built inside the agent. +// +// This inventories everything under destination, including entries the caller did +// not write, and rejects any symlink it finds. That is the right contract for a +// caller that assembles the whole destination itself and is claiming all of it — +// there, an unexpected entry is drift. A caller that renders a known template set +// into a directory it shares with the repository should use +// BuildEmittedDockerBuildPlan instead, which claims only the files it wrote. func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRecipe) (*builderv0.DockerBuildPlan, error) { files, err := inventoryRecipeFiles(destination) if err != nil { @@ -98,14 +189,20 @@ func BuildPlanRequested(req *builderv0.BuildRequest) bool { // caller requested recipe emission (a non-empty BuildRequest.output_directory) // instead of building the image in-process, so the build recipe becomes a durable // artifact the caller builds. -func SingleImageBuildPlan(outputDirectory, image string, platforms []string) (*builderv0.DockerBuildPlan, error) { +// +// emitted is the path set this build rendered, from PrepareRecipeDestination. The +// recipe references a dockerignore only when that set contains one. Probing the +// destination for a dockerignore instead would adopt a stale file: the executor +// copies the referenced ignore to the path buildx discovers and applies it, so an +// ignore left behind by an older template set would silently keep files out of +// the image with no error and no diff. +func SingleImageBuildPlan(outputDirectory, image string, platforms []string, emitted []string) (*builderv0.DockerBuildPlan, error) { dockerignore := "" - // Lstat, not Stat: inventoryRecipeFiles rejects symlinks outright, so a - // symlinked dockerignore that Stat would follow-and-accept must not enter the - // recipe — it would hard-fail the inventory with an error unrelated to the - // dockerignore reference. - if info, err := os.Lstat(filepath.Join(outputDirectory, "dockerignore")); err == nil && info.Mode().IsRegular() { - dockerignore = "dockerignore" + for _, name := range emitted { + if name == "dockerignore" { + dockerignore = "dockerignore" + break + } } recipe := &builderv0.DockerBuildRecipe{ Name: "app", @@ -115,15 +212,67 @@ func SingleImageBuildPlan(outputDirectory, image string, platforms []string) (*b Image: image, Platforms: platforms, } - return BuildDockerBuildPlan(outputDirectory, []*builderv0.DockerBuildRecipe{recipe}) + return BuildEmittedDockerBuildPlan(outputDirectory, []*builderv0.DockerBuildRecipe{recipe}, emitted) } -// VerifyDockerBuildPlan re-inventories the recipe tree at destination and checks -// it against plan. The caller (the CLI) runs this before docker buildx so it +// inventoryRecipePaths inventories exactly the destination-relative paths given, +// in canonical sorted order. Each path must be relative, stay inside destination, +// and resolve to a regular file. Symlinks are rejected by Lstat rather than +// followed: fileDigest would otherwise hash an out-of-tree target and buildx +// would read it, so containment is enforced on the real file type, not only on +// the lexical path. +func inventoryRecipePaths(destination string, paths []string) ([]*builderv0.RecipeFile, error) { + files := make([]*builderv0.RecipeFile, 0, len(paths)) + seen := make(map[string]struct{}, len(paths)) + for _, raw := range paths { + relative, err := recipeRelPath(destination, raw) + if err != nil { + return nil, fmt.Errorf("recipe file %q: %w", raw, err) + } + if _, duplicate := seen[relative]; duplicate { + return nil, fmt.Errorf("recipe file %q is listed more than once", relative) + } + seen[relative] = struct{}{} + full := filepath.Join(destination, filepath.FromSlash(relative)) + info, err := os.Lstat(full) + if err != nil { + return nil, err + } + if info.Mode()&os.ModeSymlink != 0 { + return nil, fmt.Errorf("recipe file %q is a symlink; symlinks are not permitted in the recipe tree", relative) + } + if !info.Mode().IsRegular() { + return nil, fmt.Errorf("recipe file %q is not a regular file", relative) + } + digest, err := fileDigest(full) + if err != nil { + return nil, err + } + files = append(files, &builderv0.RecipeFile{ + Path: relative, + Digest: digest, + Mode: uint32(info.Mode().Perm()), + }) + } + sort.Slice(files, func(i, j int) bool { return files[i].GetPath() < files[j].GetPath() }) + return files, nil +} + +// VerifyDockerBuildPlan re-inventories the files the plan declares and checks +// them against plan. The caller (the CLI) runs this before docker buildx so it // never builds from a tree that drifted from the inventory the agent validated, // and never builds recipes whose metadata (image, args, paths) was tampered with // after the plan was emitted: the digest covers the recipes as well as the files, // and every recipe is re-validated to reference real, contained tree entries. +// +// It re-hashes exactly plan.files rather than re-walking destination. Walking +// would make verification depend on content the plan never claimed: because +// output_directory is the service's committed builder/ directory, an unrelated +// file added there after emission would change the aggregate digest, and an +// unrelated symlink would abort the build with an error naming a file that has +// nothing to do with the image. A path the plan declares but that is now missing, +// modified, re-moded, or replaced by a symlink is still caught, which is the +// drift this guard exists to detect. func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) error { if plan == nil { return fmt.Errorf("build plan is nil") @@ -131,9 +280,13 @@ func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) if plan.GetContractVersion() != DockerBuildRecipeContractVersion { return fmt.Errorf("build plan contract %q, expected %q", plan.GetContractVersion(), DockerBuildRecipeContractVersion) } - files, err := inventoryRecipeFiles(destination) + declared := make([]string, 0, len(plan.GetFiles())) + for _, file := range plan.GetFiles() { + declared = append(declared, file.GetPath()) + } + files, err := inventoryRecipePaths(destination, declared) if err != nil { - return fmt.Errorf("inventory recipe tree: %w", err) + return fmt.Errorf("inventory recipe files: %w", err) } if err := validateRecipes(destination, plan.GetRecipes(), files); err != nil { return err @@ -145,12 +298,20 @@ func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) } // validateRecipes checks that every recipe carries a non-empty name that is -// unique within the service, and references paths that are relative, contained -// within destination, and present in the inventoried tree: the Dockerfile and -// (optional) dockerignore must be files in the inventory, and the context must -// be an existing directory. A plan that passes this is buildable by docker -// buildx and cannot point the build context or Dockerfile outside the -// caller-owned output directory. +// unique within the service, and references paths that are relative and stay +// inside the root they are resolved against: the Dockerfile and (optional) +// dockerignore must be files in the inventory, so they cannot point buildx -f at +// anything outside the caller-owned output directory. +// +// The context is checked lexically only. dockerfile and dockerignore are +// output_directory-relative, but context is resolved by the executor against the +// SERVICE directory — that is what makes "." mean "build the service" while the +// Dockerfile lives in the service's builder/ subdirectory. destination is +// therefore the wrong root to resolve the context against, and statting it here +// proved nothing: it accepted "." because output_directory trivially exists, and +// it rejected a perfectly valid context such as "code" whenever the service had +// no builder/code directory. Existence and containment of the context belong to +// the executor, which is the only party that knows the service directory. func validateRecipes(destination string, recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile) error { inventory := make(map[string]struct{}, len(files)) for _, file := range files { @@ -173,17 +334,9 @@ func validateRecipes(destination string, recipes []*builderv0.DockerBuildRecipe, if _, ok := inventory[dockerfile]; !ok { return fmt.Errorf("recipe %q dockerfile %q is not present in the recipe tree", recipe.GetName(), recipe.GetDockerfile()) } - context, err := recipeRelPath(destination, recipe.GetContext()) - if err != nil { + if _, err := recipeRelPath(destination, recipe.GetContext()); err != nil { return fmt.Errorf("recipe %q context: %w", recipe.GetName(), err) } - info, statErr := os.Stat(filepath.Join(destination, filepath.FromSlash(context))) - if statErr != nil { - return fmt.Errorf("recipe %q context %q: %w", recipe.GetName(), recipe.GetContext(), statErr) - } - if !info.IsDir() { - return fmt.Errorf("recipe %q context %q is not a directory", recipe.GetName(), recipe.GetContext()) - } if ignore := recipe.GetDockerignore(); ignore != "" { dockerignore, err := recipeRelPath(destination, ignore) if err != nil { diff --git a/agents/services/docker_recipe_prepare_test.go b/agents/services/docker_recipe_prepare_test.go new file mode 100644 index 000000000..6b00bf7ef --- /dev/null +++ b/agents/services/docker_recipe_prepare_test.go @@ -0,0 +1,80 @@ +package services + +import ( + "embed" + "io/fs" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +// A two-file template set: the guard has to cover every path the renderer writes, +// not just the Dockerfile. +// +//go:embed testdata/recipetemplates +var recipeTemplates embed.FS + +func builderTemplateSet(t *testing.T) fs.FS { + t.Helper() + sub, err := fs.Sub(recipeTemplates, "testdata/recipetemplates") + require.NoError(t, err) + return sub +} + +func TestPrepareRecipeDestinationReportsEveryEmittedPath(t *testing.T) { + emitted, err := PrepareRecipeDestination(builderTemplateSet(t), t.TempDir()) + require.NoError(t, err) + require.Equal(t, []string{"Dockerfile", "dockerignore"}, emitted) +} + +// The template renderer opens each destination with O_CREATE|O_TRUNC and tests +// existence with os.Stat, both of which follow symlinks — so a symlinked +// destination is written *through*, truncating a file anywhere on disk. Because +// output_directory is the service's committed builder/ directory, such a symlink +// is ordinary repository content. Unlinking every emitted path first is what +// keeps the write inside the caller-owned directory; guarding only "Dockerfile" +// left every other file in the template set escaping. +func TestPrepareRecipeDestinationUnlinksSymlinksWithoutFollowingThem(t *testing.T) { + root := t.TempDir() + output := filepath.Join(root, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + + outside := map[string]string{"Dockerfile": "keep-dockerfile-target", "dockerignore": "keep-ignore-target"} + for name, content := range outside { + target := filepath.Join(root, name+".target") + require.NoError(t, os.WriteFile(target, []byte(content), 0o644)) + require.NoError(t, os.Symlink(target, filepath.Join(output, name))) + } + + emitted, err := PrepareRecipeDestination(builderTemplateSet(t), output) + require.NoError(t, err) + require.Equal(t, []string{"Dockerfile", "dockerignore"}, emitted) + + for name, content := range outside { + require.NoFileExists(t, filepath.Join(output, name), "the symlink itself must be gone") + got, readErr := os.ReadFile(filepath.Join(root, name+".target")) + require.NoError(t, readErr) + require.Equal(t, content, string(got), "%s target was written through", name) + } +} + +// Content the template set does not own is left alone: the guard unlinks what it +// is about to write, never the directory's other contents. +func TestPrepareRecipeDestinationLeavesUnrelatedContentAlone(t *testing.T) { + output := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(output, "notes.md"), []byte("hand written"), 0o644)) + + _, err := PrepareRecipeDestination(builderTemplateSet(t), output) + require.NoError(t, err) + + got, err := os.ReadFile(filepath.Join(output, "notes.md")) + require.NoError(t, err) + require.Equal(t, "hand written", string(got)) +} + +func TestPrepareRecipeDestinationRejectsAnEmptyTemplateSet(t *testing.T) { + _, err := PrepareRecipeDestination(os.DirFS(t.TempDir()), t.TempDir()) + require.Error(t, err) +} diff --git a/agents/services/docker_recipe_scope_test.go b/agents/services/docker_recipe_scope_test.go new file mode 100644 index 000000000..8c364a458 --- /dev/null +++ b/agents/services/docker_recipe_scope_test.go @@ -0,0 +1,123 @@ +package services + +import ( + "os" + "path/filepath" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// output_directory is the service's committed builder/ directory, so unrelated +// content lands there routinely: an OS sidecar file, an editor backup, a symlink +// to shared configuration. None of it is a build input — the build context is the +// service directory, not this one — so none of it may perturb the plan or fail +// the build. Scoping the inventory to the emitted set is what guarantees that. +func TestPlanIgnoresContentThisBuildDidNotEmit(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + clean, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + + require.NoError(t, os.WriteFile(filepath.Join(dir, ".DS_Store"), []byte("mac"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile~"), []byte("backup"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(filepath.Dir(dir), "shared.conf"), []byte("x"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join(filepath.Dir(dir), "shared.conf"), filepath.Join(dir, "shared.conf"))) + + dirty, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err, "an unrelated symlink in the committed recipe directory must not fail the build") + require.Equal(t, clean.GetDigest(), dirty.GetDigest(), "unrelated files must not perturb the recipe digest") + + // Verification must tolerate the same content for the same reason. + require.NoError(t, VerifyDockerBuildPlan(dir, dirty)) +} + +// The drift the plan exists to detect must still be caught, on every axis the +// digest covers: content, permission bits, and a declared file replaced by a +// symlink or removed outright. +func TestVerifyStillCatchesDriftInDeclaredFiles(t *testing.T) { + for name, mutate := range map[string]func(t *testing.T, dir string){ + "content": func(t *testing.T, dir string) { + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM evil\n"), 0o644)) + }, + "mode": func(t *testing.T, dir string) { + require.NoError(t, os.Chmod(filepath.Join(dir, "Dockerfile"), 0o755)) + }, + "symlinked": func(t *testing.T, dir string) { + outside := filepath.Join(filepath.Dir(dir), "outside") + require.NoError(t, os.WriteFile(outside, []byte("FROM alpine\nCOPY . .\n"), 0o644)) + require.NoError(t, os.Remove(filepath.Join(dir, "Dockerfile"))) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "Dockerfile"))) + }, + "removed": func(t *testing.T, dir string) { + require.NoError(t, os.Remove(filepath.Join(dir, "dockerignore"))) + }, + } { + t.Run(name, func(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) + mutate(t, dir) + require.Error(t, VerifyDockerBuildPlan(dir, plan)) + }) + } +} + +// A dockerignore this build did not emit must not be adopted into the recipe: the +// executor copies the referenced ignore to the path buildx discovers and applies +// it, so a stale file left by an older template set would silently keep files out +// of the image with no error and no diff. +func TestSingleImageBuildPlanIgnoresStaleDockerignore(t *testing.T) { + dir, _ := writeRecipeTree(t, true) + + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), []string{"Dockerfile"}) + require.NoError(t, err) + require.Empty(t, plan.GetRecipes()[0].GetDockerignore()) + require.Len(t, plan.GetFiles(), 1) + require.Equal(t, "Dockerfile", plan.GetFiles()[0].GetPath()) +} + +// The executor resolves context against the SERVICE directory, not against +// output_directory. Core must not reject a context that is valid there — the old +// os.Stat resolved "code" to /code and refused every service +// whose context was a subdirectory. +func TestRecipeContextIsNotResolvedAgainstTheOutputDirectory(t *testing.T) { + dir, emitted := writeRecipeTree(t, false) + recipe := &builderv0.DockerBuildRecipe{ + Name: "app", Dockerfile: "Dockerfile", Context: "code", + Image: "repo/app:v1", Platforms: RecipeBuildPlatforms(), + } + plan, err := BuildEmittedDockerBuildPlan(dir, []*builderv0.DockerBuildRecipe{recipe}, emitted) + require.NoError(t, err) + require.Equal(t, "code", plan.GetRecipes()[0].GetContext()) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) +} + +// Lexical containment still holds: a context or Dockerfile that escapes is +// rejected, so a plan can never point buildx outside the roots it is given. +func TestRecipePathsStillRejectEscapes(t *testing.T) { + dir, emitted := writeRecipeTree(t, false) + for name, recipe := range map[string]*builderv0.DockerBuildRecipe{ + "context escapes": {Name: "app", Dockerfile: "Dockerfile", Context: "../..", Image: "i"}, + "context absolute": {Name: "app", Dockerfile: "Dockerfile", Context: "/etc", Image: "i"}, + "dockerfile escapes": {Name: "app", Dockerfile: "../Dockerfile", Context: ".", Image: "i"}, + "empty name": {Dockerfile: "Dockerfile", Context: ".", Image: "i"}, + } { + t.Run(name, func(t *testing.T) { + _, err := BuildEmittedDockerBuildPlan(dir, []*builderv0.DockerBuildRecipe{recipe}, emitted) + require.Error(t, err) + }) + } +} + +// A plan emitted by an older core carries an older contract version and must be +// reported as a contract mismatch, not silently verified or read as tampering. +func TestVerifyRejectsSupersededContractVersion(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + require.Equal(t, "codefly.dev/docker-build-recipe/v3", plan.GetContractVersion()) + plan.ContractVersion = "codefly.dev/docker-build-recipe/v2" + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "contract") +} diff --git a/agents/services/docker_recipe_singleimage_test.go b/agents/services/docker_recipe_singleimage_test.go index 73a195bff..7a1c5b959 100644 --- a/agents/services/docker_recipe_singleimage_test.go +++ b/agents/services/docker_recipe_singleimage_test.go @@ -9,23 +9,27 @@ import ( "github.com/stretchr/testify/require" ) -func writeRecipeTree(t *testing.T, withIgnore bool) string { +// writeRecipeTree returns the output_directory and the emitted path set a runner +// would have gotten back from PrepareRecipeDestination for this template set. +func writeRecipeTree(t *testing.T, withIgnore bool) (string, []string) { t.Helper() // dir is the output_directory the caller (the CLI) passes: the service's // committed builder/ recipe directory, with the Dockerfile — and the optional // dockerignore — directly inside it, exactly as the runner renders them there. dir := t.TempDir() require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\nCOPY . .\n"), 0o644)) + emitted := []string{"Dockerfile"} if withIgnore { require.NoError(t, os.WriteFile(filepath.Join(dir, "dockerignore"), []byte("code/node_modules\n"), 0o644)) + emitted = append(emitted, "dockerignore") } - return dir + return dir, emitted } func TestSingleImageBuildPlanConventionalLayout(t *testing.T) { - dir := writeRecipeTree(t, true) + dir, emitted := writeRecipeTree(t, true) - plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms()) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) require.NoError(t, err) require.Len(t, plan.GetRecipes(), 1) @@ -42,9 +46,9 @@ func TestSingleImageBuildPlanConventionalLayout(t *testing.T) { } func TestSingleImageBuildPlanOmitsAbsentDockerignore(t *testing.T) { - dir := writeRecipeTree(t, false) + dir, emitted := writeRecipeTree(t, false) - plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms()) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) require.NoError(t, err) require.Equal(t, "", plan.GetRecipes()[0].GetDockerignore()) require.NoError(t, VerifyDockerBuildPlan(dir, plan)) @@ -64,12 +68,12 @@ func TestBuildPlanRequested(t *testing.T) { // language runner returns SingleImageBuildResponse and gets the same DockerBuildPlan // result, verifiable against the caller's tree. func TestSingleImageBuildResponseEmitsPlan(t *testing.T) { - dir := writeRecipeTree(t, true) + dir, emitted := writeRecipeTree(t, true) base := &Base{loaded: true} wrapper := &BuilderWrapper{Base: base} base.Builder = wrapper // WithBuildPlan records onto s.Builder, as production wires it. - resp, err := wrapper.SingleImageBuildResponse(&builderv0.BuildRequest{OutputDirectory: dir}, "repo/app:v1") + resp, err := wrapper.SingleImageBuildResponse(&builderv0.BuildRequest{OutputDirectory: dir}, "repo/app:v1", emitted) require.NoError(t, err) require.Equal(t, builderv0.BuildStatus_SUCCESS, resp.GetState().GetState()) @@ -81,16 +85,16 @@ func TestSingleImageBuildResponseEmitsPlan(t *testing.T) { } func TestSingleImageBuildResponseLeavesCacheExecutionWithCaller(t *testing.T) { - dir := writeRecipeTree(t, true) + dir, emitted := writeRecipeTree(t, true) base := &Base{loaded: true} wrapper := &BuilderWrapper{Base: base} base.Builder = wrapper req := &builderv0.BuildRequest{OutputDirectory: dir} - cold, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1") + cold, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1", emitted) require.NoError(t, err) cache := &builderv0.BuildCacheOptions{Backend: "registry", Scope: "workspace/service/app", Imports: []string{"ghcr.io/org/cache"}} req.BuildContext = &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: &builderv0.DockerBuildContext{Cache: cache}}} - warm, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1") + warm, err := wrapper.SingleImageBuildResponse(req, "repo/app:v1", emitted) require.NoError(t, err) require.Empty(t, warm.CacheContractVersion) require.Equal(t, cold.GetResult().GetDockerBuildPlan().Digest, warm.GetResult().GetDockerBuildPlan().Digest) diff --git a/agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl b/agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl new file mode 100644 index 000000000..12c97b01d --- /dev/null +++ b/agents/services/testdata/recipetemplates/templates/builder/Dockerfile.tmpl @@ -0,0 +1,2 @@ +FROM scratch +COPY app /app diff --git a/agents/services/testdata/recipetemplates/templates/builder/dockerignore b/agents/services/testdata/recipetemplates/templates/builder/dockerignore new file mode 100644 index 000000000..3c3629e64 --- /dev/null +++ b/agents/services/testdata/recipetemplates/templates/builder/dockerignore @@ -0,0 +1 @@ +node_modules diff --git a/docs/build-cache.md b/docs/build-cache.md index faa1d00d9..94919d5b1 100644 --- a/docs/build-cache.md +++ b/docs/build-cache.md @@ -134,9 +134,29 @@ host load affect wall time. The CLI alone builds and publishes application images. Shared Go and Rust runners require an absolute `output_directory` and return a DockerBuildPlan. -Missing destinations fail before template preparation. Go custom/workspace-root -contexts are rejected before preparation because the single-image recipe cannot -represent them. Managed images and runtime-only agents remain no-build. +Missing destinations fail before template preparation. Go custom context roots +and workspace-root contexts are both rejected before preparation because the +single-image recipe cannot represent them — `Workspace` carries the same +requirement as `ContextRoot`, so rejecting only one would defer the failure to +an opaque `COPY` error inside the executor. Managed images and runtime-only +agents remain no-build. + +`output_directory` is the service's committed `builder/` directory, so it holds +whatever the repository and the developer's OS put there. A plan therefore +inventories only the files the emitting build wrote — the set +`PrepareRecipeDestination` returns — and `VerifyDockerBuildPlan` re-hashes +exactly those paths. Unrelated content cannot perturb the digest or fail the +build, while drift in a declared file (content, mode, replacement by a symlink, +removal) is still rejected. Because that directory is committed, every emitted +path is unlinked before rendering: the template writer opens destinations with +`O_CREATE|O_TRUNC` and would otherwise write *through* a symlink to a file +outside the caller-owned directory. + +Recipe path bases differ and the distinction is load-bearing: `dockerfile` and +`dockerignore` are relative to `output_directory`, while `context` is relative +to the **service** directory, which is why `"."` means "build the service". Core +checks the context lexically only; the executor resolves it and enforces +containment, because only the executor knows the service directory. `DockerBuildContext.buildx_builder` identifies the builder the CLI selected, including selection for registry cache exports. The agent does not invoke it. diff --git a/generated/go/codefly/services/builder/v0/docker.pb.go b/generated/go/codefly/services/builder/v0/docker.pb.go index 07c429585..63dc5d601 100644 --- a/generated/go/codefly/services/builder/v0/docker.pb.go +++ b/generated/go/codefly/services/builder/v0/docker.pb.go @@ -218,7 +218,12 @@ type DockerBuildRecipe struct { Name string `protobuf:"bytes,1,opt,name=name,proto3" json:"name,omitempty"` // dockerfile is the output_directory-relative path to the Dockerfile. Dockerfile string `protobuf:"bytes,2,opt,name=dockerfile,proto3" json:"dockerfile,omitempty"` - // context is the build-context directory the Dockerfile is evaluated against. + // context is the build-context directory the Dockerfile is evaluated against, + // relative to the SERVICE directory — not to output_directory, which the + // dockerfile and dockerignore paths are relative to. "." therefore means "build + // the service" while the Dockerfile itself lives in the service's builder/ + // subdirectory. The executor resolves it and is the party that enforces it + // stays inside the service. Context string `protobuf:"bytes,3,opt,name=context,proto3" json:"context,omitempty"` // dockerignore is the optional output_directory-relative ignore file. Dockerignore string `protobuf:"bytes,4,opt,name=dockerignore,proto3" json:"dockerignore,omitempty"` @@ -329,7 +334,10 @@ type DockerBuildPlan struct { state protoimpl.MessageState `protogen:"open.v1"` // recipes is the ordered set of image build recipes for the service. Recipes []*DockerBuildRecipe `protobuf:"bytes,1,rep,name=recipes,proto3" json:"recipes,omitempty"` - // files is the canonical, sorted inventory of recipe files with digests. + // files is the canonical, sorted inventory of the files the emitting build + // wrote, with digests. It is not an inventory of everything in + // output_directory: that directory is the service's committed builder/, so + // unrelated content there must neither perturb the digest nor fail the build. Files []*RecipeFile `protobuf:"bytes,2,rep,name=files,proto3" json:"files,omitempty"` // digest is the aggregate content digest over files, formatted as // "sha256:". Identical recipe trees yield an identical digest. diff --git a/proto/codefly/services/builder/v0/docker.proto b/proto/codefly/services/builder/v0/docker.proto index bb4ba225c..01317804f 100644 --- a/proto/codefly/services/builder/v0/docker.proto +++ b/proto/codefly/services/builder/v0/docker.proto @@ -42,7 +42,12 @@ message DockerBuildRecipe { string name = 1; // dockerfile is the output_directory-relative path to the Dockerfile. string dockerfile = 2; - // context is the build-context directory the Dockerfile is evaluated against. + // context is the build-context directory the Dockerfile is evaluated against, + // relative to the SERVICE directory — not to output_directory, which the + // dockerfile and dockerignore paths are relative to. "." therefore means "build + // the service" while the Dockerfile itself lives in the service's builder/ + // subdirectory. The executor resolves it and is the party that enforces it + // stays inside the service. string context = 3; // dockerignore is the optional output_directory-relative ignore file. string dockerignore = 4; @@ -64,7 +69,10 @@ message DockerBuildRecipe { message DockerBuildPlan { // recipes is the ordered set of image build recipes for the service. repeated DockerBuildRecipe recipes = 1; - // files is the canonical, sorted inventory of recipe files with digests. + // files is the canonical, sorted inventory of the files the emitting build + // wrote, with digests. It is not an inventory of everything in + // output_directory: that directory is the service's committed builder/, so + // unrelated content there must neither perturb the digest nor fail the build. repeated RecipeFile files = 2; // digest is the aggregate content digest over files, formatted as // "sha256:". Identical recipe trees yield an identical digest. diff --git a/runners/golang/agent_builder.go b/runners/golang/agent_builder.go index 32fc50768..e0c19aa36 100644 --- a/runners/golang/agent_builder.go +++ b/runners/golang/agent_builder.go @@ -4,8 +4,6 @@ import ( "context" "embed" "fmt" - "os" - "path/filepath" "strings" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" @@ -31,7 +29,7 @@ type DockerTemplating struct { ModuleRoot string // e.g. "code" — where go.mod lives BuildTarget string // e.g. "./cmd/server" — package to build (relative to ModuleRoot) ContextRoot string // non-empty custom contexts are rejected before preparation - Workspace bool // template hint for workspace-aware source copying + Workspace bool // workspace-root source copying; rejected before preparation } // DockerEnv is a key-value pair for Docker environment variables. @@ -42,7 +40,7 @@ type DockerEnv struct { // BuildGoDocker emits a Docker build recipe for a Go service. func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, - req *builderv0.BuildRequest, _ string, + req *builderv0.BuildRequest, requirements *builders.Dependencies, builderFS embed.FS, goVersion, alpineVersion string, opts ...func(*DockerTemplating)) (*builderv0.BuildResponse, error) { @@ -73,11 +71,23 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } + // ContextRoot and Workspace express one requirement — build from a directory + // above the service — and agents set them together. Rejecting only ContextRoot + // would let Workspace through, and a workspace template renders COPY paths + // relative to the workspace root while the recipe pins the context to the + // service directory: the build would then fail inside the CLI's buildx run + // with an opaque "COPY failed: file not found" instead of here, where the + // error can name the unsupported setting. if docker.ContextRoot != "" { return builder.BuildError(fmt.Errorf("custom Docker context root %q is not supported by image recipes", docker.ContextRoot)) } - if err = os.Remove(filepath.Join(req.GetOutputDirectory(), "Dockerfile")); err != nil && !os.IsNotExist(err) { + if docker.Workspace { + return builder.BuildError(fmt.Errorf("workspace-root Docker contexts are not supported by image recipes")) + } + + emitted, err := services.PrepareRecipeDestination(builderFS, req.GetOutputDirectory()) + if err != nil { return builder.BuildError(err) } @@ -86,7 +96,7 @@ func BuildGoDocker(ctx context.Context, builder *services.BuilderWrapper, return builder.BuildError(err) } - return builder.SingleImageBuildResponse(req, image.FullName()) + return builder.SingleImageBuildResponse(req, image.FullName(), emitted) } // DeployGoKubernetes deploys a Go service to Kubernetes. diff --git a/runners/golang/agent_builder_cache_test.go b/runners/golang/agent_builder_cache_test.go deleted file mode 100644 index 082cbc4bb..000000000 --- a/runners/golang/agent_builder_cache_test.go +++ /dev/null @@ -1,146 +0,0 @@ -package golang - -import ( - "context" - "embed" - "net" - "os" - "os/exec" - "path/filepath" - "testing" - "time" - - "github.com/codefly-dev/core/agents/services" - "github.com/codefly-dev/core/builders" - basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" - builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" - "github.com/codefly-dev/core/resources" - "github.com/codefly-dev/core/runners/rust" - "github.com/stretchr/testify/require" - "google.golang.org/grpc" - "google.golang.org/grpc/credentials/insecure" -) - -//go:embed templates/builder -var cacheBuilderTemplates embed.FS - -type recipeBuilder struct { - *services.DefaultBuilder - wrapper *services.BuilderWrapper - location string - language string - contextRoot string -} - -func (b *recipeBuilder) Build(ctx context.Context, req *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { - if b.language == "rust" { - return rust.BuildRustDocker(ctx, b.wrapper, req, b.location, &builders.Dependencies{}, cacheBuilderTemplates, "", "") - } - return BuildGoDocker(ctx, b.wrapper, req, b.location, &builders.Dependencies{}, cacheBuilderTemplates, "", "", func(options *DockerTemplating) { options.ContextRoot = b.contextRoot }) -} - -func (*recipeBuilder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { - return &builderv0.BuildCapabilitiesResponse{BuildxSelection: true}, nil -} - -func TestImageRecipeOverGRPC(t *testing.T) { - t.Setenv("PATH", t.TempDir()) - for _, executable := range []string{"docker", "buildx"} { - _, err := exec.LookPath(executable) - require.Error(t, err) - } - for _, language := range []string{"go", "rust"} { - for _, selection := range []string{"explicit", "cache"} { - for _, scenario := range []string{"recipe", "empty-destination", "stale-destination", "symlink-destination", "missing-output", "relative-output", "custom-context"} { - if language == "rust" && scenario == "custom-context" { - continue - } - t.Run(language+"/"+selection+"/"+scenario, func(t *testing.T) { - ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) - defer cancel() - root := t.TempDir() - location := filepath.Join(root, "service") - output := filepath.Join(location, "builder") - require.NoError(t, os.MkdirAll(output, 0o755)) - dockerfile := filepath.Join(output, "Dockerfile") - require.NoError(t, os.WriteFile(dockerfile, []byte("original recipe"), 0o644)) - base := &services.Base{} - require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) - wrapper := &services.BuilderWrapper{Base: base} - base.Builder = wrapper - base.SetDockerImage(&resources.DockerImage{Name: "app", Tag: "test"}) - agent := &recipeBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper, location: location, language: language} - dockerContext := &builderv0.DockerBuildContext{BuildxBuilder: "selected"} - if selection == "cache" { - dockerContext.BuildxBuilder = "cache-selected" - dockerContext.Cache = &builderv0.BuildCacheOptions{Backend: "registry", Scope: "test/app", Exports: []string{"registry.example.com/cache"}} - } - request := &builderv0.BuildRequest{OutputDirectory: output, BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: dockerContext}}} - expectedError := "" - switch scenario { - case "empty-destination", "stale-destination", "symlink-destination": - request.OutputDirectory = filepath.Join(t.TempDir(), "recipes") - if scenario == "stale-destination" { - require.NoError(t, os.MkdirAll(request.OutputDirectory, 0o755)) - require.NoError(t, os.WriteFile(filepath.Join(request.OutputDirectory, "Dockerfile"), []byte("FROM stale-image\n"), 0o644)) - } - if scenario == "symlink-destination" { - require.NoError(t, os.MkdirAll(request.OutputDirectory, 0o755)) - require.NoError(t, os.Symlink(dockerfile, filepath.Join(request.OutputDirectory, "Dockerfile"))) - } - case "missing-output": - request.OutputDirectory = "" - expectedError = "output_directory is required" - case "relative-output": - request.OutputDirectory = "builder" - expectedError = "must be absolute" - case "custom-context": - agent.contextRoot = root - expectedError = "custom Docker context root" - } - server := grpc.NewServer() - builderv0.RegisterBuilderServer(server, agent) - listener, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - go func() { _ = server.Serve(listener) }() - t.Cleanup(server.Stop) - connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) - require.NoError(t, err) - t.Cleanup(func() { require.NoError(t, connection.Close()) }) - response, err := services.NewBuilderAgentClient(connection).Build(ctx, request) - if expectedError != "" { - if err != nil { - require.ErrorContains(t, err, expectedError) - } else { - require.Equal(t, builderv0.BuildStatus_ERROR, response.GetState().GetState()) - require.Contains(t, response.GetState().GetMessage(), expectedError) - } - contents, readErr := os.ReadFile(dockerfile) - require.NoError(t, readErr) - require.Equal(t, "original recipe", string(contents)) - entries, readErr := os.ReadDir(output) - require.NoError(t, readErr) - require.Len(t, entries, 1) - return - } - require.NoError(t, err) - require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) - plan := response.GetResult().GetDockerBuildPlan() - require.NotNil(t, plan) - require.NoError(t, services.VerifyDockerBuildPlan(request.OutputDirectory, plan)) - fresh, readErr := os.ReadFile(filepath.Join(request.OutputDirectory, "Dockerfile")) - require.NoError(t, readErr) - require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) - if request.OutputDirectory != output { - original, readErr := os.ReadFile(dockerfile) - require.NoError(t, readErr) - require.Equal(t, "original recipe", string(original)) - } - require.Len(t, plan.GetRecipes(), 1) - require.Equal(t, "app:test", plan.GetRecipes()[0].GetImage()) - require.Equal(t, services.RecipeBuildPlatforms(), plan.GetRecipes()[0].GetPlatforms()) - }) - } - } - } -} diff --git a/runners/golang/agent_builder_recipe_test.go b/runners/golang/agent_builder_recipe_test.go new file mode 100644 index 000000000..975ea9651 --- /dev/null +++ b/runners/golang/agent_builder_recipe_test.go @@ -0,0 +1,246 @@ +package golang + +import ( + "context" + "embed" + "net" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/codefly-dev/core/agents/services" + "github.com/codefly-dev/core/builders" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +//go:embed templates/builder +var recipeBuilderTemplates embed.FS + +type recipeBuilder struct { + *services.DefaultBuilder + wrapper *services.BuilderWrapper + contextRoot string + workspace bool +} + +func (b *recipeBuilder) Build(ctx context.Context, req *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { + return BuildGoDocker(ctx, b.wrapper, req, &builders.Dependencies{}, recipeBuilderTemplates, "", "", + func(options *DockerTemplating) { + options.ContextRoot = b.contextRoot + options.Workspace = b.workspace + }) +} + +func (*recipeBuilder) BuildCapabilities(context.Context, *builderv0.BuildCapabilitiesRequest) (*builderv0.BuildCapabilitiesResponse, error) { + return &builderv0.BuildCapabilitiesResponse{BuildxSelection: true}, nil +} + +// recipeAgent stands up the real gRPC Builder surface with Docker and Buildx +// absent from PATH, so nothing here can accidentally execute a build. +type recipeAgent struct { + client builderv0.BuilderClient + service string + output string + agent *recipeBuilder +} + +func newRecipeAgent(t *testing.T, ctx context.Context) *recipeAgent { + t.Helper() + t.Setenv("PATH", t.TempDir()) + for _, executable := range []string{"docker", "buildx"} { + _, err := exec.LookPath(executable) + require.Error(t, err) + } + + root := t.TempDir() + location := filepath.Join(root, "service") + output := filepath.Join(location, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + + base := &services.Base{} + require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{ + Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) + wrapper := &services.BuilderWrapper{Base: base} + base.Builder = wrapper + base.SetDockerImage(&resources.DockerImage{Name: "app", Tag: "test"}) + + agent := &recipeBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper} + server := grpc.NewServer() + builderv0.RegisterBuilderServer(server, agent) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, connection.Close()) }) + + return &recipeAgent{client: builderv0.NewBuilderClient(connection), service: location, output: output, agent: agent} +} + +func (a *recipeAgent) request(dockerContext *builderv0.DockerBuildContext) *builderv0.BuildRequest { + return &builderv0.BuildRequest{ + OutputDirectory: a.output, + BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{DockerBuildContext: dockerContext}}, + } +} + +func selectedContext() *builderv0.DockerBuildContext { + return &builderv0.DockerBuildContext{BuildxBuilder: "selected"} +} + +func testContext(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + t.Cleanup(cancel) + return ctx +} + +func TestImageRecipeEmitsAVerifiablePlan(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + response, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + + plan := response.GetResult().GetDockerBuildPlan() + require.NotNil(t, plan) + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, plan)) + require.Len(t, plan.GetRecipes(), 1) + require.Equal(t, "app:test", plan.GetRecipes()[0].GetImage()) + require.Equal(t, services.RecipeBuildPlatforms(), plan.GetRecipes()[0].GetPlatforms()) + + fresh, err := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, err) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) + + // This template set emits no dockerignore, so the recipe must not claim one. + require.Empty(t, plan.GetRecipes()[0].GetDockerignore()) +} + +// A stale recipe from an earlier build is replaced, not left in place. +func TestImageRecipeReplacesStaleDockerfile(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, "Dockerfile"), []byte("FROM stale-image\n"), 0o644)) + + response, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + + fresh, err := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, err) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) +} + +// The recipe directory is committed, so it carries whatever the repository and +// the developer's OS put there. None of it is a build input — the build context +// is the service directory — so it must neither fail the build nor move the +// digest. A symlink here used to abort the build naming a file unrelated to the +// image; a stray sidecar file used to change the digest between macOS and CI. +func TestImageRecipeIsUnaffectedByUnrelatedContentInTheRecipeDirectory(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + clean, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, clean.GetState().GetState(), clean.GetState().GetMessage()) + + require.NoError(t, os.WriteFile(filepath.Join(agent.service, "shared.conf"), []byte("x"), 0o644)) + require.NoError(t, os.Symlink(filepath.Join(agent.service, "shared.conf"), filepath.Join(agent.output, "shared.conf"))) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, ".DS_Store"), []byte("mac"), 0o644)) + + dirty, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, dirty.GetState().GetState(), dirty.GetState().GetMessage()) + require.Equal(t, + clean.GetResult().GetDockerBuildPlan().GetDigest(), + dirty.GetResult().GetDockerBuildPlan().GetDigest()) + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, dirty.GetResult().GetDockerBuildPlan())) +} + +// Caller transport policy (which Buildx builder, which cache) is the executor's +// concern. It must not reach the recipe, or the same source would emit different +// durable artifacts depending on how the caller happened to invoke the build. +func TestImageRecipeIsIndependentOfCallerTransportPolicy(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + plain, err := agent.client.Build(ctx, agent.request(selectedContext())) + require.NoError(t, err) + + cached := selectedContext() + cached.BuildxBuilder = "cache-selected" + cached.Cache = &builderv0.BuildCacheOptions{Backend: "registry", Scope: "test/app", Exports: []string{"registry.example.com/cache"}} + selected, err := agent.client.Build(ctx, agent.request(cached)) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, selected.GetState().GetState(), selected.GetState().GetMessage()) + + require.Equal(t, + plain.GetResult().GetDockerBuildPlan().GetDigest(), + selected.GetResult().GetDockerBuildPlan().GetDigest()) + // The agent executes nothing, so it acknowledges no executor selection. + require.Empty(t, selected.GetBuildxBuilder()) + require.Empty(t, selected.GetCacheContractVersion()) +} + +func TestImageRecipeRejectsUnsupportedRequests(t *testing.T) { + for name, scenario := range map[string]struct { + mutate func(*recipeAgent, *builderv0.BuildRequest) + expected string + }{ + "missing output_directory": { + func(_ *recipeAgent, r *builderv0.BuildRequest) { r.OutputDirectory = "" }, + "output_directory is required", + }, + "relative output_directory": { + func(_ *recipeAgent, r *builderv0.BuildRequest) { r.OutputDirectory = "builder" }, + "must be absolute", + }, + "custom context root": { + func(a *recipeAgent, _ *builderv0.BuildRequest) { a.agent.contextRoot = a.service }, + "custom Docker context root", + }, + // Workspace carries the same requirement as ContextRoot and agents set the + // two together. Letting it through would emit a recipe whose context is the + // service directory while the template copies workspace-relative paths — + // failing later, inside the executor, with an opaque COPY error. + "workspace context": { + func(a *recipeAgent, _ *builderv0.BuildRequest) { a.agent.workspace = true }, + "workspace-root Docker contexts are not supported", + }, + } { + t.Run(name, func(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, "Dockerfile"), []byte("original recipe"), 0o644)) + + request := agent.request(selectedContext()) + scenario.mutate(agent, request) + + response, err := agent.client.Build(ctx, request) + if err != nil { + require.ErrorContains(t, err, scenario.expected) + } else { + require.Equal(t, builderv0.BuildStatus_ERROR, response.GetState().GetState()) + require.Contains(t, response.GetState().GetMessage(), scenario.expected) + } + + // A rejected request must not have touched the committed recipe. + contents, readErr := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, readErr) + require.Equal(t, "original recipe", string(contents)) + entries, readErr := os.ReadDir(agent.output) + require.NoError(t, readErr) + require.Len(t, entries, 1) + }) + } +} diff --git a/runners/rust/agent_builder.go b/runners/rust/agent_builder.go index eef07980c..3efc0d052 100644 --- a/runners/rust/agent_builder.go +++ b/runners/rust/agent_builder.go @@ -4,8 +4,6 @@ import ( "context" "embed" "fmt" - "os" - "path/filepath" "strings" dockerhelpers "github.com/codefly-dev/core/agents/helpers/docker" @@ -37,7 +35,7 @@ type DockerEnv struct { // BuildRustDocker emits a Docker build recipe for a Rust service. func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, - req *builderv0.BuildRequest, _ string, + req *builderv0.BuildRequest, requirements *builders.Dependencies, builderFS embed.FS, rustVersion, alpineVersion string, opts ...func(*DockerTemplating)) (*builderv0.BuildResponse, error) { @@ -68,7 +66,8 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, opt(&docker) } - if err = os.Remove(filepath.Join(req.GetOutputDirectory(), "Dockerfile")); err != nil && !os.IsNotExist(err) { + emitted, err := services.PrepareRecipeDestination(builderFS, req.GetOutputDirectory()) + if err != nil { return builder.BuildError(err) } @@ -77,7 +76,7 @@ func BuildRustDocker(ctx context.Context, builder *services.BuilderWrapper, return builder.BuildError(err) } - return builder.SingleImageBuildResponse(req, image.FullName()) + return builder.SingleImageBuildResponse(req, image.FullName(), emitted) } // DeployRustKubernetes deploys a Rust service to Kubernetes. Identical in diff --git a/runners/rust/agent_builder_recipe_test.go b/runners/rust/agent_builder_recipe_test.go new file mode 100644 index 000000000..386d3c5f3 --- /dev/null +++ b/runners/rust/agent_builder_recipe_test.go @@ -0,0 +1,169 @@ +package rust + +import ( + "context" + "embed" + "net" + "os" + "os/exec" + "path/filepath" + "testing" + "time" + + "github.com/codefly-dev/core/agents/services" + "github.com/codefly-dev/core/builders" + basev0 "github.com/codefly-dev/core/generated/go/codefly/base/v0" + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/codefly-dev/core/resources" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" +) + +// A two-file template set — Dockerfile and dockerignore — so the runner is +// exercised with a recipe that emits more than one file, which is where a guard +// scoped to a single filename leaks. +// +//go:embed templates/builder +var recipeBuilderTemplates embed.FS + +type recipeBuilder struct { + *services.DefaultBuilder + wrapper *services.BuilderWrapper +} + +func (b *recipeBuilder) Build(ctx context.Context, req *builderv0.BuildRequest) (*builderv0.BuildResponse, error) { + return BuildRustDocker(ctx, b.wrapper, req, &builders.Dependencies{}, recipeBuilderTemplates, "", "") +} + +type recipeAgent struct { + client builderv0.BuilderClient + service string + output string +} + +func newRecipeAgent(t *testing.T, ctx context.Context) *recipeAgent { + t.Helper() + t.Setenv("PATH", t.TempDir()) + for _, executable := range []string{"docker", "buildx"} { + _, err := exec.LookPath(executable) + require.Error(t, err) + } + + root := t.TempDir() + location := filepath.Join(root, "service") + output := filepath.Join(location, "builder") + require.NoError(t, os.MkdirAll(output, 0o755)) + + base := &services.Base{} + require.NoError(t, base.HeadlessLoad(ctx, &basev0.ServiceIdentity{ + Name: "app", Module: "test", WorkspacePath: root, RelativeToWorkspace: "service"})) + wrapper := &services.BuilderWrapper{Base: base} + base.Builder = wrapper + base.SetDockerImage(&resources.DockerImage{Name: "app", Tag: "test"}) + + server := grpc.NewServer() + builderv0.RegisterBuilderServer(server, &recipeBuilder{DefaultBuilder: services.NewDefaultBuilder(wrapper), wrapper: wrapper}) + listener, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + go func() { _ = server.Serve(listener) }() + t.Cleanup(server.Stop) + connection, err := grpc.NewClient(listener.Addr().String(), grpc.WithTransportCredentials(insecure.NewCredentials())) + require.NoError(t, err) + t.Cleanup(func() { require.NoError(t, connection.Close()) }) + + return &recipeAgent{client: builderv0.NewBuilderClient(connection), service: location, output: output} +} + +func (a *recipeAgent) request() *builderv0.BuildRequest { + return &builderv0.BuildRequest{ + OutputDirectory: a.output, + BuildContext: &builderv0.BuildContext{Kind: &builderv0.BuildContext_DockerBuildContext{ + DockerBuildContext: &builderv0.DockerBuildContext{BuildxBuilder: "selected"}}}, + } +} + +func testContext(t *testing.T) context.Context { + t.Helper() + ctx, cancel := context.WithTimeout(t.Context(), 10*time.Second) + t.Cleanup(cancel) + return ctx +} + +func TestImageRecipeEmitsEveryTemplateFile(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + response, err := agent.client.Build(ctx, agent.request()) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + + plan := response.GetResult().GetDockerBuildPlan() + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, plan)) + require.Equal(t, "dockerignore", plan.GetRecipes()[0].GetDockerignore()) + + inventory := make([]string, 0, len(plan.GetFiles())) + for _, file := range plan.GetFiles() { + inventory = append(inventory, file.GetPath()) + } + require.Equal(t, []string{"Dockerfile", "dockerignore"}, inventory) +} + +// The renderer follows symlinks on write, so any emitted path that is a symlink +// gets written *through*, truncating a file outside the caller-owned directory. +// A guard scoped to "Dockerfile" left every other file in the template set +// exposed; the recipe directory is committed, so a symlinked dockerignore is +// ordinary repository content rather than an attack precondition. +func TestImageRecipeNeverWritesThroughASymlinkedDestination(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + + targets := map[string]string{"Dockerfile": "dockerfile target", "dockerignore": "ignore target"} + for name, content := range targets { + outside := filepath.Join(agent.service, name+".outside") + require.NoError(t, os.WriteFile(outside, []byte(content), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(agent.output, name))) + } + + response, err := agent.client.Build(ctx, agent.request()) + require.NoError(t, err) + require.Equal(t, builderv0.BuildStatus_SUCCESS, response.GetState().GetState(), response.GetState().GetMessage()) + require.NoError(t, services.VerifyDockerBuildPlan(agent.output, response.GetResult().GetDockerBuildPlan())) + + for name, content := range targets { + got, readErr := os.ReadFile(filepath.Join(agent.service, name+".outside")) + require.NoError(t, readErr) + require.Equal(t, content, string(got), "%s target was written through the symlink", name) + } + + fresh, err := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, err) + require.Equal(t, "FROM scratch\nCOPY app /app\n", string(fresh)) +} + +func TestImageRecipeRequiresAnAbsoluteDestination(t *testing.T) { + for name, scenario := range map[string]struct{ directory, expected string }{ + "missing": {"", "output_directory is required"}, + "relative": {"builder", "must be absolute"}, + } { + t.Run(name, func(t *testing.T) { + ctx := testContext(t) + agent := newRecipeAgent(t, ctx) + require.NoError(t, os.WriteFile(filepath.Join(agent.output, "Dockerfile"), []byte("original recipe"), 0o644)) + + request := agent.request() + request.OutputDirectory = scenario.directory + response, err := agent.client.Build(ctx, request) + if err != nil { + require.ErrorContains(t, err, scenario.expected) + } else { + require.Equal(t, builderv0.BuildStatus_ERROR, response.GetState().GetState()) + require.Contains(t, response.GetState().GetMessage(), scenario.expected) + } + + contents, readErr := os.ReadFile(filepath.Join(agent.output, "Dockerfile")) + require.NoError(t, readErr) + require.Equal(t, "original recipe", string(contents)) + }) + } +} diff --git a/runners/rust/templates/builder/Dockerfile.tmpl b/runners/rust/templates/builder/Dockerfile.tmpl new file mode 100644 index 000000000..12c97b01d --- /dev/null +++ b/runners/rust/templates/builder/Dockerfile.tmpl @@ -0,0 +1,2 @@ +FROM scratch +COPY app /app diff --git a/runners/rust/templates/builder/dockerignore b/runners/rust/templates/builder/dockerignore new file mode 100644 index 000000000..eb5a316cb --- /dev/null +++ b/runners/rust/templates/builder/dockerignore @@ -0,0 +1 @@ +target From ecd357127d582b207026bf1e95afa4bb7341ccda Mon Sep 17 00:00:00 2001 From: Antoine Toussaint Date: Sun, 13 Sep 2026 15:12:24 -0400 Subject: [PATCH 6/6] fix: make a recipe plan declare what its file inventory covers (#461) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit aeae945d scoped plan inventories to the files a build emitted, so unrelated content in the service's committed builder/ directory could no longer perturb a digest or abort a build. It applied that reading to verification too, for every plan — and that was wrong for the other kind of emitter. Agents that assemble the whole destination themselves copy their build context into it and build ".": service-go's buildRecipe renders builder/Dockerfile and then copies the entire Go source tree to /code. For those plans every file under the destination is a real build input. Re-hashing only the declared paths stopped detecting a file ADDED to that tree after emission — an injected source file or script that buildx would copy straight into the image. Walking, which is what v2 did, caught it. The two readings are both right, for different emitters, and neither can be inferred from the destination. So the plan now states which one it is. DockerBuildPlan carries a RecipeInventoryScope: BuildEmittedDockerBuildPlan declares EMITTED and is verified by re-hashing its declared paths; BuildDockerBuildPlan declares TREE and is verified by re-walking the destination and comparing the whole inventory. A plan that declares no scope is rejected rather than verified under a default, because defaulting is exactly the silent guess this replaces. The scope is covered by the aggregate digest. Without that, rewriting a TREE plan's scope to EMITTED would downgrade it to the tolerant check while leaving the digest intact — turning the declaration into a way to disable the guard it exists to select. Tests cover both directions, since a fix for one reading is a regression in the other: files and symlinks added to a whole-tree build context are rejected, unclaimed content in an emitted-scope directory is tolerated, a downgraded scope breaks the digest, identical files under different scopes do not share a digest, and an unscoped plan is refused. Contract stays at v3, which this branch already introduced unreleased. Co-Authored-By: Claude Opus 5 (1M context) --- agents/services/docker_recipe.go | 79 +++++++---- .../docker_recipe_inventory_scope_test.go | 95 +++++++++++++ docs/build-cache.md | 22 ++- .../codefly/services/builder/v0/docker.pb.go | 128 +++++++++++++++--- .../codefly/services/builder/v0/docker.proto | 25 ++++ 5 files changed, 296 insertions(+), 53 deletions(-) create mode 100644 agents/services/docker_recipe_inventory_scope_test.go diff --git a/agents/services/docker_recipe.go b/agents/services/docker_recipe.go index 1c1c382ee..ccdb45821 100644 --- a/agents/services/docker_recipe.go +++ b/agents/services/docker_recipe.go @@ -23,9 +23,10 @@ import ( // is reported as a contract mismatch (a clear, actionable error) rather than as // a digest mismatch (indistinguishable from tampering). v2 covers the recipes // and per-file mode in the digest; v1 covered only file paths and content. v3 -// scopes the inventory to the files the plan declares rather than to whatever -// happens to sit in the destination directory, so unrelated content in a -// committed recipe directory can neither perturb the digest nor fail the build. +// makes each plan declare what its inventory covers (RecipeInventoryScope) and +// digests that declaration, so a plan rendered into a directory shared with the +// repository is not failed by unrelated content while a plan covering a whole +// assembled build context still rejects files added to it. const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v3" // ValidateBuildRequestOutputDirectory enforces the BuildRequest.output_directory @@ -123,8 +124,9 @@ func BuildEmittedDockerBuildPlan(destination string, recipes []*builderv0.Docker return &builderv0.DockerBuildPlan{ Recipes: recipes, Files: files, - Digest: aggregateRecipeDigest(recipes, files), + Digest: aggregateRecipeDigest(recipes, files, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED), ContractVersion: DockerBuildRecipeContractVersion, + Scope: builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED, }, nil } @@ -154,8 +156,9 @@ func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRe return &builderv0.DockerBuildPlan{ Recipes: recipes, Files: files, - Digest: aggregateRecipeDigest(recipes, files), + Digest: aggregateRecipeDigest(recipes, files, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE), ContractVersion: DockerBuildRecipeContractVersion, + Scope: builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE, }, nil } @@ -258,21 +261,31 @@ func inventoryRecipePaths(destination string, paths []string) ([]*builderv0.Reci return files, nil } -// VerifyDockerBuildPlan re-inventories the files the plan declares and checks -// them against plan. The caller (the CLI) runs this before docker buildx so it -// never builds from a tree that drifted from the inventory the agent validated, -// and never builds recipes whose metadata (image, args, paths) was tampered with -// after the plan was emitted: the digest covers the recipes as well as the files, -// and every recipe is re-validated to reference real, contained tree entries. +// VerifyDockerBuildPlan re-inventories a plan's files and checks them against +// plan. The caller (the CLI) runs this before docker buildx so it never builds +// from a tree that drifted from the inventory the agent validated, and never +// builds recipes whose metadata (image, args, paths) was tampered with after the +// plan was emitted: the digest covers the recipes as well as the files, and every +// recipe is re-validated to reference real, contained tree entries. // -// It re-hashes exactly plan.files rather than re-walking destination. Walking -// would make verification depend on content the plan never claimed: because -// output_directory is the service's committed builder/ directory, an unrelated -// file added there after emission would change the aggregate digest, and an -// unrelated symlink would abort the build with an error naming a file that has -// nothing to do with the image. A path the plan declares but that is now missing, -// modified, re-moded, or replaced by a symlink is still caught, which is the -// drift this guard exists to detect. +// How much of destination counts as "the plan's files" is the plan's own +// declaration, not a guess here, because the two possible answers are not +// interchangeable: +// +// - TREE: re-walk destination and compare the whole inventory. The emitter +// assembled the entire destination — service agents copy the build context +// there and build "." — so a file ADDED after emission is a build input that +// would be baked into the image, and must fail verification. +// - EMITTED: re-hash exactly the declared paths. The emitter rendered a known +// template set into the service's committed builder/ directory, which it +// shares with the repository, so unrelated content there is outside the claim +// and must neither move the digest nor abort the build. +// +// Verifying a TREE plan as if it were EMITTED silently stops detecting injected +// build-context files; verifying an EMITTED plan as if it were TREE fails builds +// over an editor backup. A plan that declares no scope is rejected rather than +// verified under a default, and the scope is covered by the digest so it cannot +// be rewritten to weaken the check. func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) error { if plan == nil { return fmt.Errorf("build plan is nil") @@ -280,18 +293,29 @@ func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) if plan.GetContractVersion() != DockerBuildRecipeContractVersion { return fmt.Errorf("build plan contract %q, expected %q", plan.GetContractVersion(), DockerBuildRecipeContractVersion) } - declared := make([]string, 0, len(plan.GetFiles())) - for _, file := range plan.GetFiles() { - declared = append(declared, file.GetPath()) + + var files []*builderv0.RecipeFile + var err error + switch scope := plan.GetScope(); scope { + case builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE: + files, err = inventoryRecipeFiles(destination) + case builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED: + declared := make([]string, 0, len(plan.GetFiles())) + for _, file := range plan.GetFiles() { + declared = append(declared, file.GetPath()) + } + files, err = inventoryRecipePaths(destination, declared) + default: + return fmt.Errorf("build plan declares no inventory scope (%s); it cannot be verified", scope) } - files, err := inventoryRecipePaths(destination, declared) if err != nil { return fmt.Errorf("inventory recipe files: %w", err) } + if err := validateRecipes(destination, plan.GetRecipes(), files); err != nil { return err } - if digest := aggregateRecipeDigest(plan.GetRecipes(), files); digest != plan.GetDigest() { + if digest := aggregateRecipeDigest(plan.GetRecipes(), files, plan.GetScope()); digest != plan.GetDigest() { return fmt.Errorf("recipe tree digest %s does not match plan digest %s", digest, plan.GetDigest()) } return nil @@ -443,8 +467,13 @@ func fileDigest(path string) (string, error) { // permission bits are covered too, so flipping a file's executable bit — which // buildx carries into the image — is detected even when its content is // unchanged. -func aggregateRecipeDigest(recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile) string { +func aggregateRecipeDigest(recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile, scope builderv0.RecipeInventoryScope) string { hasher := sha256.New() + // The scope decides how strictly the inventory is verified, so it is covered + // here: rewriting a TREE plan's scope to EMITTED would otherwise silently stop + // added files from being detected without disturbing the digest. + hashField(hasher, "scope") + hashField(hasher, scope.String()) hashField(hasher, "recipes") hashCount(hasher, len(recipes)) for _, recipe := range recipes { diff --git a/agents/services/docker_recipe_inventory_scope_test.go b/agents/services/docker_recipe_inventory_scope_test.go new file mode 100644 index 000000000..dd397592f --- /dev/null +++ b/agents/services/docker_recipe_inventory_scope_test.go @@ -0,0 +1,95 @@ +package services + +import ( + "os" + "path/filepath" + "testing" + + builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" + "github.com/stretchr/testify/require" +) + +// wholeTreePlan mirrors what a service agent emits when it copies its build +// context into the destination and builds "." — service-go's buildRecipe does +// exactly this, so every file under destination is a real build input. +func wholeTreePlan(t *testing.T) (string, *builderv0.DockerBuildPlan) { + t.Helper() + dir := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(dir, "code"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\nCOPY . .\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "code", "main.go"), []byte("package main\n"), 0o644)) + plan, err := BuildDockerBuildPlan(dir, []*builderv0.DockerBuildRecipe{{ + Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1", + Platforms: RecipeBuildPlatforms(), + }}) + require.NoError(t, err) + require.Equal(t, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE, plan.GetScope()) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) + return dir, plan +} + +// A whole-tree emitter claims the entire destination, so a file added after +// emission is an injected build input that buildx would copy into the image. +// Verifying such a plan by re-hashing only its declared paths would not see it. +func TestVerifyRejectsFilesAddedToAWholeTreeBuildContext(t *testing.T) { + dir, plan := wholeTreePlan(t) + + require.NoError(t, os.WriteFile(filepath.Join(dir, "code", "injected.go"), []byte("package main\n"), 0o644)) + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "digest") +} + +func TestVerifyRejectsSymlinksAddedToAWholeTreeBuildContext(t *testing.T) { + dir, plan := wholeTreePlan(t) + + outside := filepath.Join(t.TempDir(), "secret") + require.NoError(t, os.WriteFile(outside, []byte("secret"), 0o644)) + require.NoError(t, os.Symlink(outside, filepath.Join(dir, "code", "secret"))) + require.Error(t, VerifyDockerBuildPlan(dir, plan)) +} + +// The emitted-scope claim is the opposite case and must stay tolerant: the +// destination is the service's committed builder/ directory, shared with the +// repository, so content the build did not write is outside the claim. +func TestVerifyToleratesUnclaimedContentInAnEmittedScopePlan(t *testing.T) { + dir, emitted := writeRecipeTree(t, true) + plan, err := SingleImageBuildPlan(dir, "repo/app:v1", RecipeBuildPlatforms(), emitted) + require.NoError(t, err) + require.Equal(t, builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED, plan.GetScope()) + + require.NoError(t, os.WriteFile(filepath.Join(dir, ".DS_Store"), []byte("mac"), 0o644)) + require.NoError(t, VerifyDockerBuildPlan(dir, plan)) +} + +// Downgrading the scope would silently turn the strict check into the tolerant +// one, so the scope is covered by the aggregate digest. +func TestScopeCannotBeDowngradedWithoutBreakingTheDigest(t *testing.T) { + dir, plan := wholeTreePlan(t) + + plan.Scope = builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "digest") +} + +// Identical recipes and files under different scopes are different claims, so +// they must not share a digest. +func TestScopeParticipatesInTheAggregateDigest(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + recipes := []*builderv0.DockerBuildRecipe{{Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1"}} + + tree, err := BuildDockerBuildPlan(dir, recipes) + require.NoError(t, err) + emitted, err := BuildEmittedDockerBuildPlan(dir, recipes, []string{"Dockerfile"}) + require.NoError(t, err) + + require.Equal(t, tree.GetFiles()[0].GetDigest(), emitted.GetFiles()[0].GetDigest()) + require.NotEqual(t, tree.GetDigest(), emitted.GetDigest()) +} + +// A plan carrying no scope predates the declaration and cannot be verified under +// either reading; defaulting would pick one silently. +func TestVerifyRejectsAPlanWithNoDeclaredScope(t *testing.T) { + dir, plan := wholeTreePlan(t) + + plan.Scope = builderv0.RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_UNSPECIFIED + require.ErrorContains(t, VerifyDockerBuildPlan(dir, plan), "no inventory scope") +} diff --git a/docs/build-cache.md b/docs/build-cache.md index 94919d5b1..9e9ba9d00 100644 --- a/docs/build-cache.md +++ b/docs/build-cache.md @@ -142,12 +142,22 @@ an opaque `COPY` error inside the executor. Managed images and runtime-only agents remain no-build. `output_directory` is the service's committed `builder/` directory, so it holds -whatever the repository and the developer's OS put there. A plan therefore -inventories only the files the emitting build wrote — the set -`PrepareRecipeDestination` returns — and `VerifyDockerBuildPlan` re-hashes -exactly those paths. Unrelated content cannot perturb the digest or fail the -build, while drift in a declared file (content, mode, replacement by a symlink, -removal) is still rejected. Because that directory is committed, every emitted +whatever the repository and the developer's OS put there. A plan from the shared +runners therefore inventories only the files the emitting build wrote — the set +`PrepareRecipeDestination` returns — and declares +`RECIPE_INVENTORY_SCOPE_EMITTED`. `VerifyDockerBuildPlan` re-hashes exactly those +paths: unrelated content cannot perturb the digest or fail the build, while drift +in a declared file (content, mode, replacement by a symlink, removal) is still +rejected. + +An agent that assembles the whole destination itself — copying its build context +there and building `"."`, as the Go agent does — calls `BuildDockerBuildPlan` and +declares `RECIPE_INVENTORY_SCOPE_TREE` instead. There the inventory covers every +file, and verification re-walks the destination, because a file *added* after +emission is an injected build input that buildx would copy into the image. The +scope is part of the aggregate digest, so a plan cannot be rewritten to the +weaker check, and a plan declaring no scope is rejected rather than verified +under a default. Because that directory is committed, every emitted path is unlinked before rendering: the template writer opens destinations with `O_CREATE|O_TRUNC` and would otherwise write *through* a symlink to a file outside the caller-owned directory. diff --git a/generated/go/codefly/services/builder/v0/docker.pb.go b/generated/go/codefly/services/builder/v0/docker.pb.go index 63dc5d601..1d1a3c524 100644 --- a/generated/go/codefly/services/builder/v0/docker.pb.go +++ b/generated/go/codefly/services/builder/v0/docker.pb.go @@ -22,6 +22,70 @@ const ( _ = protoimpl.EnforceVersion(protoimpl.MaxVersion - 20) ) +// RecipeInventoryScope states what a plan's file inventory claims to cover, so +// the verifier knows what "complete" means for that plan. Without it the +// verifier has to guess, and the two answers are not interchangeable: guessing +// EMITTED for a TREE plan stops detecting files ADDED to a build context after +// emission, and guessing TREE for an EMITTED plan fails a build over unrelated +// content in the service's committed builder/ directory. +type RecipeInventoryScope int32 + +const ( + // RECIPE_INVENTORY_SCOPE_UNSPECIFIED does not authorize verification; a plan + // that omits the scope is rejected rather than verified under a default. + RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_UNSPECIFIED RecipeInventoryScope = 0 + // RECIPE_INVENTORY_SCOPE_EMITTED claims only the files the emitting build + // wrote. Other entries under output_directory are outside the claim and are + // neither digested nor rejected. + RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_EMITTED RecipeInventoryScope = 1 + // RECIPE_INVENTORY_SCOPE_TREE claims every file under output_directory. The + // emitter assembled the whole destination — typically because it copied the + // build context there and a recipe builds "." — so an entry the inventory does + // not list is drift in the emitter's own output and fails verification. + RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_TREE RecipeInventoryScope = 2 +) + +// Enum value maps for RecipeInventoryScope. +var ( + RecipeInventoryScope_name = map[int32]string{ + 0: "RECIPE_INVENTORY_SCOPE_UNSPECIFIED", + 1: "RECIPE_INVENTORY_SCOPE_EMITTED", + 2: "RECIPE_INVENTORY_SCOPE_TREE", + } + RecipeInventoryScope_value = map[string]int32{ + "RECIPE_INVENTORY_SCOPE_UNSPECIFIED": 0, + "RECIPE_INVENTORY_SCOPE_EMITTED": 1, + "RECIPE_INVENTORY_SCOPE_TREE": 2, + } +) + +func (x RecipeInventoryScope) Enum() *RecipeInventoryScope { + p := new(RecipeInventoryScope) + *p = x + return p +} + +func (x RecipeInventoryScope) String() string { + return protoimpl.X.EnumStringOf(x.Descriptor(), protoreflect.EnumNumber(x)) +} + +func (RecipeInventoryScope) Descriptor() protoreflect.EnumDescriptor { + return file_codefly_services_builder_v0_docker_proto_enumTypes[0].Descriptor() +} + +func (RecipeInventoryScope) Type() protoreflect.EnumType { + return &file_codefly_services_builder_v0_docker_proto_enumTypes[0] +} + +func (x RecipeInventoryScope) Number() protoreflect.EnumNumber { + return protoreflect.EnumNumber(x) +} + +// Deprecated: Use RecipeInventoryScope.Descriptor instead. +func (RecipeInventoryScope) EnumDescriptor() ([]byte, []int) { + return file_codefly_services_builder_v0_docker_proto_rawDescGZIP(), []int{0} +} + // DockerBuildContext contains Docker-specific image build inputs. type DockerBuildContext struct { state protoimpl.MessageState `protogen:"open.v1"` @@ -344,8 +408,12 @@ type DockerBuildPlan struct { Digest string `protobuf:"bytes,3,opt,name=digest,proto3" json:"digest,omitempty"` // contract_version identifies the recipe contract the caller validates. ContractVersion string `protobuf:"bytes,4,opt,name=contract_version,json=contractVersion,proto3" json:"contract_version,omitempty"` - unknownFields protoimpl.UnknownFields - sizeCache protoimpl.SizeCache + // scope states what the files inventory covers. It is part of the aggregate + // digest, so a plan cannot be downgraded to a weaker verification by + // rewriting this field. + Scope RecipeInventoryScope `protobuf:"varint,5,opt,name=scope,proto3,enum=codefly.services.builder.v0.RecipeInventoryScope" json:"scope,omitempty"` + unknownFields protoimpl.UnknownFields + sizeCache protoimpl.SizeCache } func (x *DockerBuildPlan) Reset() { @@ -406,6 +474,13 @@ func (x *DockerBuildPlan) GetContractVersion() string { return "" } +func (x *DockerBuildPlan) GetScope() RecipeInventoryScope { + if x != nil { + return x.Scope + } + return RecipeInventoryScope_RECIPE_INVENTORY_SCOPE_UNSPECIFIED +} + // BuildCacheOptions is caller-owned transport policy. Registry credentials are // resolved by Docker, never serialized into a request or recipe. type BuildCacheOptions struct { @@ -521,18 +596,23 @@ const file_codefly_services_builder_v0_docker_proto_rawDesc = "" + "\x06target\x18\b \x01(\tR\x06target\x1a<\n" + "\x0eBuildArgsEntry\x12\x10\n" + "\x03key\x18\x01 \x01(\tR\x03key\x12\x14\n" + - "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xdd\x01\n" + + "\x05value\x18\x02 \x01(\tR\x05value:\x028\x01\"\xa6\x02\n" + "\x0fDockerBuildPlan\x12H\n" + "\arecipes\x18\x01 \x03(\v2..codefly.services.builder.v0.DockerBuildRecipeR\arecipes\x12=\n" + "\x05files\x18\x02 \x03(\v2'.codefly.services.builder.v0.RecipeFileR\x05files\x12\x16\n" + "\x06digest\x18\x03 \x01(\tR\x06digest\x12)\n" + - "\x10contract_version\x18\x04 \x01(\tR\x0fcontractVersion\"\x8b\x01\n" + + "\x10contract_version\x18\x04 \x01(\tR\x0fcontractVersion\x12G\n" + + "\x05scope\x18\x05 \x01(\x0e21.codefly.services.builder.v0.RecipeInventoryScopeR\x05scope\"\x8b\x01\n" + "\x11BuildCacheOptions\x12\x18\n" + "\aimports\x18\x01 \x03(\tR\aimports\x12\x18\n" + "\aexports\x18\x02 \x03(\tR\aexports\x12\x14\n" + "\x05scope\x18\x03 \x01(\tR\x05scope\x12\x12\n" + "\x04mode\x18\x04 \x01(\tR\x04mode\x12\x18\n" + - "\abackend\x18\x05 \x01(\tR\abackendB\x84\x02\n" + + "\abackend\x18\x05 \x01(\tR\abackend*\x83\x01\n" + + "\x14RecipeInventoryScope\x12&\n" + + "\"RECIPE_INVENTORY_SCOPE_UNSPECIFIED\x10\x00\x12\"\n" + + "\x1eRECIPE_INVENTORY_SCOPE_EMITTED\x10\x01\x12\x1f\n" + + "\x1bRECIPE_INVENTORY_SCOPE_TREE\x10\x02B\x84\x02\n" + "\x1fcom.codefly.services.builder.v0B\vDockerProtoP\x01ZDgithub.com/codefly-dev/core/generated/go/codefly/services/builder/v0\xa2\x02\x04CSBV\xaa\x02\x1bCodefly.Services.Builder.V0\xca\x02\x1bCodefly\\Services\\Builder\\V0\xe2\x02'Codefly\\Services\\Builder\\V0\\GPBMetadata\xea\x02\x1eCodefly::Services::Builder::V0b\x06proto3" var ( @@ -547,26 +627,29 @@ func file_codefly_services_builder_v0_docker_proto_rawDescGZIP() []byte { return file_codefly_services_builder_v0_docker_proto_rawDescData } +var file_codefly_services_builder_v0_docker_proto_enumTypes = make([]protoimpl.EnumInfo, 1) var file_codefly_services_builder_v0_docker_proto_msgTypes = make([]protoimpl.MessageInfo, 7) var file_codefly_services_builder_v0_docker_proto_goTypes = []any{ - (*DockerBuildContext)(nil), // 0: codefly.services.builder.v0.DockerBuildContext - (*DockerBuildResult)(nil), // 1: codefly.services.builder.v0.DockerBuildResult - (*RecipeFile)(nil), // 2: codefly.services.builder.v0.RecipeFile - (*DockerBuildRecipe)(nil), // 3: codefly.services.builder.v0.DockerBuildRecipe - (*DockerBuildPlan)(nil), // 4: codefly.services.builder.v0.DockerBuildPlan - (*BuildCacheOptions)(nil), // 5: codefly.services.builder.v0.BuildCacheOptions - nil, // 6: codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry + (RecipeInventoryScope)(0), // 0: codefly.services.builder.v0.RecipeInventoryScope + (*DockerBuildContext)(nil), // 1: codefly.services.builder.v0.DockerBuildContext + (*DockerBuildResult)(nil), // 2: codefly.services.builder.v0.DockerBuildResult + (*RecipeFile)(nil), // 3: codefly.services.builder.v0.RecipeFile + (*DockerBuildRecipe)(nil), // 4: codefly.services.builder.v0.DockerBuildRecipe + (*DockerBuildPlan)(nil), // 5: codefly.services.builder.v0.DockerBuildPlan + (*BuildCacheOptions)(nil), // 6: codefly.services.builder.v0.BuildCacheOptions + nil, // 7: codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry } var file_codefly_services_builder_v0_docker_proto_depIdxs = []int32{ - 5, // 0: codefly.services.builder.v0.DockerBuildContext.cache:type_name -> codefly.services.builder.v0.BuildCacheOptions - 6, // 1: codefly.services.builder.v0.DockerBuildRecipe.build_args:type_name -> codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry - 3, // 2: codefly.services.builder.v0.DockerBuildPlan.recipes:type_name -> codefly.services.builder.v0.DockerBuildRecipe - 2, // 3: codefly.services.builder.v0.DockerBuildPlan.files:type_name -> codefly.services.builder.v0.RecipeFile - 4, // [4:4] is the sub-list for method output_type - 4, // [4:4] is the sub-list for method input_type - 4, // [4:4] is the sub-list for extension type_name - 4, // [4:4] is the sub-list for extension extendee - 0, // [0:4] is the sub-list for field type_name + 6, // 0: codefly.services.builder.v0.DockerBuildContext.cache:type_name -> codefly.services.builder.v0.BuildCacheOptions + 7, // 1: codefly.services.builder.v0.DockerBuildRecipe.build_args:type_name -> codefly.services.builder.v0.DockerBuildRecipe.BuildArgsEntry + 4, // 2: codefly.services.builder.v0.DockerBuildPlan.recipes:type_name -> codefly.services.builder.v0.DockerBuildRecipe + 3, // 3: codefly.services.builder.v0.DockerBuildPlan.files:type_name -> codefly.services.builder.v0.RecipeFile + 0, // 4: codefly.services.builder.v0.DockerBuildPlan.scope:type_name -> codefly.services.builder.v0.RecipeInventoryScope + 5, // [5:5] is the sub-list for method output_type + 5, // [5:5] is the sub-list for method input_type + 5, // [5:5] is the sub-list for extension type_name + 5, // [5:5] is the sub-list for extension extendee + 0, // [0:5] is the sub-list for field type_name } func init() { file_codefly_services_builder_v0_docker_proto_init() } @@ -579,13 +662,14 @@ func file_codefly_services_builder_v0_docker_proto_init() { File: protoimpl.DescBuilder{ GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: unsafe.Slice(unsafe.StringData(file_codefly_services_builder_v0_docker_proto_rawDesc), len(file_codefly_services_builder_v0_docker_proto_rawDesc)), - NumEnums: 0, + NumEnums: 1, NumMessages: 7, NumExtensions: 0, NumServices: 0, }, GoTypes: file_codefly_services_builder_v0_docker_proto_goTypes, DependencyIndexes: file_codefly_services_builder_v0_docker_proto_depIdxs, + EnumInfos: file_codefly_services_builder_v0_docker_proto_enumTypes, MessageInfos: file_codefly_services_builder_v0_docker_proto_msgTypes, }.Build() File_codefly_services_builder_v0_docker_proto = out.File diff --git a/proto/codefly/services/builder/v0/docker.proto b/proto/codefly/services/builder/v0/docker.proto index 01317804f..859bb5f4c 100644 --- a/proto/codefly/services/builder/v0/docker.proto +++ b/proto/codefly/services/builder/v0/docker.proto @@ -62,6 +62,27 @@ message DockerBuildRecipe { string target = 8; } +// RecipeInventoryScope states what a plan's file inventory claims to cover, so +// the verifier knows what "complete" means for that plan. Without it the +// verifier has to guess, and the two answers are not interchangeable: guessing +// EMITTED for a TREE plan stops detecting files ADDED to a build context after +// emission, and guessing TREE for an EMITTED plan fails a build over unrelated +// content in the service's committed builder/ directory. +enum RecipeInventoryScope { + // RECIPE_INVENTORY_SCOPE_UNSPECIFIED does not authorize verification; a plan + // that omits the scope is rejected rather than verified under a default. + RECIPE_INVENTORY_SCOPE_UNSPECIFIED = 0; + // RECIPE_INVENTORY_SCOPE_EMITTED claims only the files the emitting build + // wrote. Other entries under output_directory are outside the claim and are + // neither digested nor rejected. + RECIPE_INVENTORY_SCOPE_EMITTED = 1; + // RECIPE_INVENTORY_SCOPE_TREE claims every file under output_directory. The + // emitter assembled the whole destination — typically because it copied the + // build context there and a recipe builds "." — so an entry the inventory does + // not list is drift in the emitter's own output and fails verification. + RECIPE_INVENTORY_SCOPE_TREE = 2; +} + // DockerBuildPlan is the reproducible recipe set a builder emits to the caller's // output_directory. The caller owns running docker buildx from the recipes, so // the recipes are a durable, first-class artifact rather than an image built @@ -79,6 +100,10 @@ message DockerBuildPlan { string digest = 3; // contract_version identifies the recipe contract the caller validates. string contract_version = 4; + // scope states what the files inventory covers. It is part of the aggregate + // digest, so a plan cannot be downgraded to a weaker verification by + // rewriting this field. + RecipeInventoryScope scope = 5; } // BuildCacheOptions is caller-owned transport policy. Registry credentials are