diff --git a/agents/services/docker_recipe.go b/agents/services/docker_recipe.go index cef49488..a2e1d659 100644 --- a/agents/services/docker_recipe.go +++ b/agents/services/docker_recipe.go @@ -4,16 +4,22 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "io" "os" "path/filepath" "sort" + "strings" builderv0 "github.com/codefly-dev/core/generated/go/codefly/services/builder/v0" ) // DockerBuildRecipeContractVersion identifies the recipe contract a caller -// validates before building from an emitted plan. -const DockerBuildRecipeContractVersion = "codefly.dev/docker-build-recipe/v1" +// validates before building from an emitted plan. The version is bumped whenever +// 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" // ValidateBuildRequestOutputDirectory enforces the BuildRequest.output_directory // contract: when set, the destination must be an absolute path the caller owns. @@ -41,7 +47,9 @@ func ValidateBuildRequestOutputDirectory(req *builderv0.BuildRequest) error { // 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 -// deterministic function of that inventory. The caller (the CLI) verifies the +// deterministic function of both the recipes and that inventory. Every recipe is +// validated to reference real, contained tree entries before the plan is +// 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. func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRecipe) (*builderv0.DockerBuildPlan, error) { @@ -49,17 +57,23 @@ func BuildDockerBuildPlan(destination string, recipes []*builderv0.DockerBuildRe if err != nil { return nil, fmt.Errorf("inventory recipe tree: %w", err) } + if err := validateRecipes(destination, recipes, files); err != nil { + return nil, err + } return &builderv0.DockerBuildPlan{ Recipes: recipes, Files: files, - Digest: aggregateRecipeDigest(files), + Digest: aggregateRecipeDigest(recipes, files), ContractVersion: DockerBuildRecipeContractVersion, }, nil } // VerifyDockerBuildPlan re-inventories the recipe tree at destination and checks // it 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. +// 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. func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) error { if plan == nil { return fmt.Errorf("build plan is nil") @@ -71,12 +85,92 @@ func VerifyDockerBuildPlan(destination string, plan *builderv0.DockerBuildPlan) if err != nil { return fmt.Errorf("inventory recipe tree: %w", err) } - if digest := aggregateRecipeDigest(files); digest != plan.GetDigest() { + if err := validateRecipes(destination, plan.GetRecipes(), files); err != nil { + return err + } + if digest := aggregateRecipeDigest(plan.GetRecipes(), files); digest != plan.GetDigest() { return fmt.Errorf("recipe tree digest %s does not match plan digest %s", digest, plan.GetDigest()) } return nil } +// 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. +func validateRecipes(destination string, recipes []*builderv0.DockerBuildRecipe, files []*builderv0.RecipeFile) error { + inventory := make(map[string]struct{}, len(files)) + for _, file := range files { + inventory[file.GetPath()] = struct{}{} + } + seenNames := make(map[string]struct{}, len(recipes)) + for _, recipe := range recipes { + name := recipe.GetName() + if name == "" { + return fmt.Errorf("recipe has an empty name") + } + if _, ok := seenNames[name]; ok { + return fmt.Errorf("recipe name %q is not unique within the service", name) + } + seenNames[name] = struct{}{} + dockerfile, err := recipeRelPath(destination, recipe.GetDockerfile()) + if err != nil { + return fmt.Errorf("recipe %q dockerfile: %w", recipe.GetName(), err) + } + 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 { + 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 { + return fmt.Errorf("recipe %q dockerignore: %w", recipe.GetName(), err) + } + if _, ok := inventory[dockerignore]; !ok { + return fmt.Errorf("recipe %q dockerignore %q is not present in the recipe tree", recipe.GetName(), recipe.GetDockerignore()) + } + } + } + return nil +} + +// recipeRelPath validates that p is a non-empty relative path that stays inside +// destination, and returns it in the slash-separated form used by the inventory. +// Absolute paths and paths that escape destination (via "..") are rejected so a +// recipe can never point buildx at a Dockerfile or context outside the +// caller-owned output directory. +func recipeRelPath(destination, p string) (string, error) { + if p == "" { + return "", fmt.Errorf("path is empty") + } + if filepath.IsAbs(p) { + return "", fmt.Errorf("path %q must be relative", p) + } + clean := filepath.Clean(filepath.FromSlash(p)) + full := filepath.Join(destination, clean) + rel, err := filepath.Rel(destination, full) + if err != nil { + return "", err + } + if rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + return "", fmt.Errorf("path %q escapes the output directory", p) + } + return filepath.ToSlash(rel), nil +} + func inventoryRecipeFiles(destination string) ([]*builderv0.RecipeFile, error) { var files []*builderv0.RecipeFile err := filepath.WalkDir(destination, func(entryPath string, entry os.DirEntry, walkErr error) error { @@ -86,18 +180,31 @@ func inventoryRecipeFiles(destination string) ([]*builderv0.RecipeFile, error) { if entry.IsDir() { return nil } - content, readErr := os.ReadFile(entryPath) - if readErr != nil { - return readErr - } + // Reject symlinks outright rather than following them. A symlink is the + // escape the lexical recipeRelPath check cannot see: fileDigest would + // otherwise hash the symlink's out-of-tree target, and buildx would + // follow it out of the caller-owned output directory. Rejecting here + // contains the tree by construction, and replaces the cryptic + // "is a directory" error a directory symlink used to produce. relative, relErr := filepath.Rel(destination, entryPath) if relErr != nil { return relErr } - sum := sha256.Sum256(content) + if entry.Type()&os.ModeSymlink != 0 { + return fmt.Errorf("recipe tree entry %q is a symlink; symlinks are not permitted in the recipe tree", filepath.ToSlash(relative)) + } + info, infoErr := entry.Info() + if infoErr != nil { + return infoErr + } + digest, digestErr := fileDigest(entryPath) + if digestErr != nil { + return digestErr + } files = append(files, &builderv0.RecipeFile{ Path: filepath.ToSlash(relative), - Digest: "sha256:" + hex.EncodeToString(sum[:]), + Digest: digest, + Mode: uint32(info.Mode().Perm()), }) return nil }) @@ -108,10 +215,74 @@ func inventoryRecipeFiles(destination string) ([]*builderv0.RecipeFile, error) { return files, nil } -func aggregateRecipeDigest(files []*builderv0.RecipeFile) string { +// fileDigest streams the file through sha256 rather than buffering it whole, so +// inventorying a large build context does not read every file into memory. +func fileDigest(path string) (string, error) { + f, err := os.Open(path) + if err != nil { + return "", err + } + defer f.Close() hasher := sha256.New() + if _, err := io.Copy(hasher, f); err != nil { + return "", err + } + return "sha256:" + hex.EncodeToString(hasher.Sum(nil)), nil +} + +// aggregateRecipeDigest is a deterministic digest over both the recipes and the +// file inventory. Every string is written length-prefixed (netstring form) so no +// field value — including a path that contains a separator or newline — can be +// confused with a field boundary, and map keys are sorted so map iteration order +// does not perturb the result. Because the recipes are covered, a plan whose +// image reference, build args, target, or paths were altered no longer matches +// its digest even when the on-disk files are byte-identical. Each file's Unix +// 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 { + hasher := sha256.New() + hashField(hasher, "recipes") + hashCount(hasher, len(recipes)) + for _, recipe := range recipes { + hashField(hasher, recipe.GetName()) + hashField(hasher, recipe.GetDockerfile()) + hashField(hasher, recipe.GetContext()) + hashField(hasher, recipe.GetDockerignore()) + hashField(hasher, recipe.GetImage()) + hashField(hasher, recipe.GetTarget()) + platforms := recipe.GetPlatforms() + hashCount(hasher, len(platforms)) + for _, platform := range platforms { + hashField(hasher, platform) + } + args := recipe.GetBuildArgs() + keys := make([]string, 0, len(args)) + for key := range args { + keys = append(keys, key) + } + sort.Strings(keys) + hashCount(hasher, len(keys)) + for _, key := range keys { + hashField(hasher, key) + hashField(hasher, args[key]) + } + } + hashField(hasher, "files") + hashCount(hasher, len(files)) for _, file := range files { - fmt.Fprintf(hasher, "%s\x00%s\n", file.GetPath(), file.GetDigest()) + hashField(hasher, file.GetPath()) + hashField(hasher, file.GetDigest()) + hashCount(hasher, int(file.GetMode())) } return "sha256:" + hex.EncodeToString(hasher.Sum(nil)) } + +func hashField(hasher io.Writer, value string) { + fmt.Fprintf(hasher, "%d:", len(value)) + io.WriteString(hasher, value) +} + +func hashCount(hasher io.Writer, n int) { + fmt.Fprintf(hasher, "#%d:", n) +} diff --git a/agents/services/docker_recipe_test.go b/agents/services/docker_recipe_test.go index 5d7d2de4..2fd04197 100644 --- a/agents/services/docker_recipe_test.go +++ b/agents/services/docker_recipe_test.go @@ -123,3 +123,137 @@ func TestBuildDockerBuildPlanDigestChangesWithContent(t *testing.T) { require.NotEqual(t, first.GetDigest(), second.GetDigest()) } + +// A plan that passes must be buildable: a recipe whose Dockerfile is not in the +// emitted tree is rejected rather than deferred to a confusing buildx failure. +func TestBuildDockerBuildPlanRejectsRecipeReferencingMissingFile(t *testing.T) { + destination := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(destination, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + + _, err := BuildDockerBuildPlan(destination, []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "app/Dockerfile", Context: ".", Image: "repo/app:v1"}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "not present in the recipe tree") +} + +// Recipe paths must stay inside the caller-owned output directory: neither a +// traversing relative path nor an absolute path may point buildx elsewhere. +func TestBuildDockerBuildPlanRejectsRecipePathEscapingOutputDirectory(t *testing.T) { + destination := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(destination, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + + _, err := BuildDockerBuildPlan(destination, []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "../Dockerfile", Context: ".", Image: "repo/app:v1"}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "escapes the output directory") + + _, err = BuildDockerBuildPlan(destination, []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "Dockerfile", Context: "/etc", Image: "repo/app:v1"}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "must be relative") +} + +// Recipe names are the logical identity within a service and must be unique; +// two recipes sharing a name is rejected rather than silently building both +// under the same identity. +func TestBuildDockerBuildPlanRejectsDuplicateRecipeName(t *testing.T) { + destination := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(destination, "one"), 0o755)) + require.NoError(t, os.MkdirAll(filepath.Join(destination, "two"), 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(destination, "one", "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + require.NoError(t, os.WriteFile(filepath.Join(destination, "two", "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + + _, err := BuildDockerBuildPlan(destination, []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "one/Dockerfile", Context: ".", Image: "repo/app:v1"}, + {Name: "app", Dockerfile: "two/Dockerfile", Context: ".", Image: "repo/app:v2"}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "not unique") +} + +// The aggregate digest covers the recipes, not only the file inventory, so +// tampering with a recipe's image reference while leaving the on-disk files +// byte-identical is caught by verification. +func TestVerifyDockerBuildPlanRejectsTamperedRecipe(t *testing.T) { + destination := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(destination, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + recipes := []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1"}, + } + + plan, err := BuildDockerBuildPlan(destination, recipes) + require.NoError(t, err) + require.NoError(t, VerifyDockerBuildPlan(destination, plan)) + + plan.Recipes[0].Image = "attacker/app:v1" + err = VerifyDockerBuildPlan(destination, plan) + require.Error(t, err) + require.Contains(t, err.Error(), "does not match plan digest") +} + +// The aggregate digest covers each file's permission bits, so flipping a recipe +// file's executable bit after the plan is emitted — content byte-identical, a +// change buildx carries into the image — is caught by verification. +func TestVerifyDockerBuildPlanRejectsModeTamper(t *testing.T) { + destination := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(destination, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + entrypoint := filepath.Join(destination, "entrypoint.sh") + require.NoError(t, os.WriteFile(entrypoint, []byte("#!/bin/sh\necho hi\n"), 0o644)) + recipes := []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1"}, + } + + plan, err := BuildDockerBuildPlan(destination, recipes) + require.NoError(t, err) + require.NoError(t, VerifyDockerBuildPlan(destination, plan)) + + // Flip the executable bit without touching content. + require.NoError(t, os.Chmod(entrypoint, 0o755)) + err = VerifyDockerBuildPlan(destination, plan) + require.Error(t, err) + require.Contains(t, err.Error(), "does not match plan digest") +} + +// A symlink in the recipe tree is the escape the lexical path check cannot see: +// it is rejected outright rather than followed to an out-of-tree target. +func TestBuildDockerBuildPlanRejectsSymlinkInTree(t *testing.T) { + outside := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(outside, "evil"), []byte("FROM scratch\n"), 0o644)) + + destination := t.TempDir() + require.NoError(t, os.MkdirAll(filepath.Join(destination, "context"), 0o755)) + // Dockerfile is a symlink pointing at a file outside the output directory. + if err := os.Symlink(filepath.Join(outside, "evil"), filepath.Join(destination, "Dockerfile")); err != nil { + t.Skipf("symlinks unsupported on this platform: %v", err) + } + + _, err := BuildDockerBuildPlan(destination, []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "Dockerfile", Context: "context", Image: "repo/app:v1"}, + }) + require.Error(t, err) + require.Contains(t, err.Error(), "symlinks are not permitted") +} + +// A plan emitted by an older core carries the previous contract version. It is +// rejected as a contract mismatch — a clear, actionable error — rather than +// slipping into the digest comparison where a version-driven algorithm change +// would surface as an indistinguishable "tamper" failure. +func TestVerifyDockerBuildPlanRejectsStaleContractVersion(t *testing.T) { + destination := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(destination, "Dockerfile"), []byte("FROM alpine\n"), 0o644)) + recipes := []*builderv0.DockerBuildRecipe{ + {Name: "app", Dockerfile: "Dockerfile", Context: ".", Image: "repo/app:v1"}, + } + + plan, err := BuildDockerBuildPlan(destination, recipes) + require.NoError(t, err) + plan.ContractVersion = "codefly.dev/docker-build-recipe/v1" + + err = VerifyDockerBuildPlan(destination, plan) + require.Error(t, err) + require.Contains(t, err.Error(), "contract") + require.NotContains(t, err.Error(), "does not match plan digest") +} diff --git a/generated/go/codefly/services/builder/v0/docker.pb.go b/generated/go/codefly/services/builder/v0/docker.pb.go index 7c0138a6..4fcd2fa4 100644 --- a/generated/go/codefly/services/builder/v0/docker.pb.go +++ b/generated/go/codefly/services/builder/v0/docker.pb.go @@ -129,7 +129,12 @@ type RecipeFile struct { // path is the output_directory-relative POSIX path of the recipe file. Path string `protobuf:"bytes,1,opt,name=path,proto3" json:"path,omitempty"` // digest is the content digest of the file, formatted as "sha256:". - Digest string `protobuf:"bytes,2,opt,name=digest,proto3" json:"digest,omitempty"` + Digest string `protobuf:"bytes,2,opt,name=digest,proto3" json:"digest,omitempty"` + // mode is the file's Unix permission bits (the low 9 bits of the file mode). + // It is part of the plan's integrity boundary: buildx preserves the executable + // bit into the image, so a mode change with identical content is a real change + // the aggregate digest must detect. + Mode uint32 `protobuf:"varint,3,opt,name=mode,proto3" json:"mode,omitempty"` unknownFields protoimpl.UnknownFields sizeCache protoimpl.SizeCache } @@ -178,6 +183,13 @@ func (x *RecipeFile) GetDigest() string { return "" } +func (x *RecipeFile) GetMode() uint32 { + if x != nil { + return x.Mode + } + return 0 +} + // DockerBuildRecipe is a single reproducible Docker image build: a Dockerfile, // its build context, and the target image reference. A service may emit many // recipes (for example an application image and a migration image). @@ -377,11 +389,12 @@ const file_codefly_services_builder_v0_docker_proto_rawDesc = "" + "\x11docker_repository\x18\x01 \x01(\tR\x10dockerRepository\x12!\n" + "\fimage_digest\x18\x02 \x01(\tR\vimageDigest\"+\n" + "\x11DockerBuildResult\x12\x16\n" + - "\x06images\x18\x01 \x03(\tR\x06images\"8\n" + + "\x06images\x18\x01 \x03(\tR\x06images\"L\n" + "\n" + "RecipeFile\x12\x12\n" + "\x04path\x18\x01 \x01(\tR\x04path\x12\x16\n" + - "\x06digest\x18\x02 \x01(\tR\x06digest\"\xed\x02\n" + + "\x06digest\x18\x02 \x01(\tR\x06digest\x12\x12\n" + + "\x04mode\x18\x03 \x01(\rR\x04mode\"\xed\x02\n" + "\x11DockerBuildRecipe\x12\x12\n" + "\x04name\x18\x01 \x01(\tR\x04name\x12\x1e\n" + "\n" + diff --git a/proto/codefly/services/builder/v0/docker.proto b/proto/codefly/services/builder/v0/docker.proto index 35466aad..a3a8cf1c 100644 --- a/proto/codefly/services/builder/v0/docker.proto +++ b/proto/codefly/services/builder/v0/docker.proto @@ -22,6 +22,11 @@ message RecipeFile { string path = 1; // digest is the content digest of the file, formatted as "sha256:". string digest = 2; + // mode is the file's Unix permission bits (the low 9 bits of the file mode). + // It is part of the plan's integrity boundary: buildx preserves the executable + // bit into the image, so a mode change with identical content is a real change + // the aggregate digest must detect. + uint32 mode = 3; } // DockerBuildRecipe is a single reproducible Docker image build: a Dockerfile,