Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ require (
github.com/spf13/cobra v1.10.2
github.com/spf13/pflag v1.0.10
github.com/tilt-dev/fsnotify v1.4.8-0.20220602155310-fff9c274a375
github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0
go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp v0.71.0
go.opentelemetry.io/otel v1.46.0
go.opentelemetry.io/otel/exporters/otlp/otlptrace v1.46.0
Expand Down Expand Up @@ -108,7 +109,6 @@ require (
github.com/secure-systems-lab/go-securesystemslib v0.11.0 // indirect
github.com/shibumi/go-pathspec v1.3.0 // indirect
github.com/tonistiigi/fsutil v0.0.0-20260819142231-83cac42c1c52 // indirect
github.com/tonistiigi/go-csvvalue v0.0.0-20240814133006-030d3b2625d0 // indirect
github.com/tonistiigi/units v0.0.0-20180711220420-6950e57a87ea // indirect
github.com/tonistiigi/vt100 v0.0.0-20240514184818-90bafcd6abab // indirect
github.com/xhit/go-str2duration/v2 v2.1.0 // indirect
Expand Down
161 changes: 130 additions & 31 deletions pkg/compose/build_bake.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,20 @@ import (
"github.com/compose-spec/compose-go/v2/types"
"github.com/containerd/console"
"github.com/containerd/errdefs"
"github.com/distribution/reference"
"github.com/docker/cli/cli-plugins/manager"
"github.com/docker/cli/cli/command"
"github.com/docker/cli/cli/command/image/build"
"github.com/docker/cli/cli/streams"
"github.com/google/uuid"
"github.com/moby/buildkit/client"
gitutil "github.com/moby/buildkit/frontend/dockerfile/dfgitutil"
buildkitgit "github.com/moby/buildkit/util/gitutil"
"github.com/moby/buildkit/util/progress/progressui"
"github.com/moby/moby/client/pkg/versions"
"github.com/sirupsen/logrus"
"github.com/spf13/cobra"
"github.com/tonistiigi/go-csvvalue"
"golang.org/x/sync/errgroup"

"github.com/docker/compose/v5/pkg/api"
Expand Down Expand Up @@ -121,12 +124,13 @@ type buildStatus struct {
// drive `buildx bake`: the bake file definition, plus the settings that travel
// as command arguments or environment variables rather than in the file.
type bakeBuild struct {
cfg bakeConfig
targetNames map[string]string // service name -> bake target name
expectedImages map[string]string // service name -> expected image
localPaths []string // local build contexts bake needs `--allow fs.read` for
privileged bool
secretsEnv []string
cfg bakeConfig
targetNames map[string]string // service name -> bake target name
expectedImages map[string]string // service name -> expected image
localPaths []string // local build inputs bake needs `--allow fs.read` for
localWritePaths []string // local cache destinations bake needs `--allow fs.write` for
entitlements []string
secretsEnv []string
}

func (s *composeService) doBuildBake(ctx context.Context, project *types.Project, serviceToBeBuild types.Services, options api.BuildOptions) (map[string]string, error) {
Expand Down Expand Up @@ -250,9 +254,26 @@ func (s *composeService) prepareBakeBuild(project *types.Project, serviceToBeBui
args[k] = strings.ReplaceAll(v, "${", "$${")
}

entitlements, privileged := bakeEntitlements(buildConfig)
bake.privileged = bake.privileged || privileged
bake.localPaths = append(bake.localPaths, localBuildPaths(buildConfig)...)
entitlements := bakeEntitlements(buildConfig)
bake.entitlements = append(bake.entitlements, entitlements...)
bake.localPaths = append(bake.localPaths, localBuildPaths(project, buildConfig)...)
bake.localPaths = append(bake.localPaths, localCachePaths(buildConfig.CacheFrom, "src")...)
bake.localWritePaths = append(bake.localWritePaths, localCachePaths(buildConfig.CacheTo, "dest")...)

ssh := slices.Concat(buildConfig.SSH, options.SSHs)
for _, key := range ssh {
if key.Path == "" {
bake.entitlements = append(bake.entitlements, "ssh")
} else {
bake.localPaths = append(bake.localPaths, strings.Split(key.Path, ",")...)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] SSH key.Path comma-split may produce empty paths in fs.read grants

strings.Split(key.Path, ",") is used to support comma-separated socket paths. If key.Path has a trailing comma or contains consecutive commas (e.g. a malformed or programmatically constructed config), an empty string is appended to bake.localPaths. The slices.Compact(slices.Sorted(...)) deduplication in bakeArgs sorts paths ascending — the empty string sorts to the front and, since it appears only once, survives Compact — and this would emit --allow fs.read= to Bake, which may be rejected or behave unexpectedly.

Consider filtering out empty segments:

Suggested change
bake.localPaths = append(bake.localPaths, strings.Split(key.Path, ",")...)
bake.localPaths = append(bake.localPaths, slices.DeleteFunc(strings.Split(key.Path, ","), func(s string) bool { return s == "" })...)
Confidence Score
🟡 moderate 67/100

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bake's SSH parser also uses strings.Split and preserves empty segments. Filtering them only from the grants would make the grants differ from the SSH configuration passed to Bake, without fixing the malformed input. I'd leave validation to Bake and BuildKit rather than silently filter those segments here.

}
}
// Bake implicitly forwards the default agent for SSH Git contexts.
if len(ssh) == 0 {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[medium] SSH agent entitlement not granted when path-based SSH keys coexist with an SSH git context

When a service has explicit path-based SSH socket keys (key.Path != ""), those paths are added to fs.read grants but the "ssh" entitlement is not appended. The guard at line 272 only adds the "ssh" entitlement for SSH-protocol git contexts when len(ssh) == 0 — meaning no SSH config at all. If a user configures explicit SSH socket files and uses an SSH git build context (e.g. context: git+ssh://...), neither branch appends "ssh" to bake.entitlements:

for _, key := range ssh {
    if key.Path == "" {
        bake.entitlements = append(bake.entitlements, "ssh")  // default agent → ✓
    } else {
        bake.localPaths = append(bake.localPaths, strings.Split(key.Path, ",")...)  // socket path → fs.read only, no "ssh" entitlement
    }
}
// Only fires when ssh is completely empty:
if len(ssh) == 0 {
    if u, err := buildkitgit.ParseURL(buildConfig.Context); err == nil && u.Scheme == buildkitgit.SSHProtocol {
        bake.entitlements = append(bake.entitlements, "ssh")  // ← missed when len(ssh) > 0
    }
}

Bake implicitly forwards the SSH agent for SSH git cloning regardless of whether explicit --ssh socket paths are also configured. The missing entitlement would cause a permission denied / missing entitlement error when Bake tries to clone the SSH git context. Consider changing the guard to check for an SSH git context independently of whether ssh is non-empty:

Suggested change
if len(ssh) == 0 {
if u, err := buildkitgit.ParseURL(buildConfig.Context); err == nil && u.Scheme == buildkitgit.SSHProtocol {
bake.entitlements = append(bake.entitlements, "ssh")
}
Confidence Score
🟡 moderate 67/100

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bake only injects the default SSH agent when no explicit SSH configuration is provided for the build context. Our len(ssh) == 0 guard matches that behavior. With explicit key or socket paths, Bake checks filesystem access to those paths, which we already grant. Removing the guard would grant agent access that this configuration doesn't require.

if u, err := buildkitgit.ParseURL(buildConfig.Context); err == nil && u.Scheme == buildkitgit.SSHProtocol {
bake.entitlements = append(bake.entitlements, "ssh")
}
}

image := api.GetImageNameOrDefault(service, project.Name)
if _, ok := serviceToBeBuild[serviceName]; ok {
Expand Down Expand Up @@ -280,7 +301,7 @@ func (s *composeService) prepareBakeBuild(project *types.Project, serviceToBeBui
Platforms: buildConfig.Platforms,
Target: buildConfig.Target,
Secrets: secrets,
SSH: toBakeSSH(append(buildConfig.SSH, options.SSHs...)),
SSH: toBakeSSH(ssh),
Pull: buildConfig.Pull || options.Pull,
NoCache: buildConfig.NoCache || options.NoCache,
ShmSize: buildConfig.ShmSize,
Expand Down Expand Up @@ -325,16 +346,16 @@ func bakeTargetNames(project *types.Project) map[string]string {
return targets
}

// bakeEntitlements returns the entitlements for a build target, and whether
// bake must be granted `security.insecure`.
func bakeEntitlements(buildConfig types.BuildConfig) ([]string, bool) {
entitlements := buildConfig.Entitlements
privileged := slices.Contains(buildConfig.Entitlements, "security.insecure")
if buildConfig.Privileged {
// bakeEntitlements returns the entitlements requested by a build target.
func bakeEntitlements(buildConfig types.BuildConfig) []string {
entitlements := slices.Clone(buildConfig.Entitlements)
if buildConfig.Privileged && !slices.Contains(entitlements, "security.insecure") {
entitlements = append(entitlements, "security.insecure")
privileged = true
}
return entitlements, privileged
if buildConfig.Network == "host" && !slices.Contains(entitlements, "network.host") {
entitlements = append(entitlements, "network.host")
}
return entitlements
}

// bakeOutputs selects the bake output type — or the lint call for `--check` —
Expand All @@ -353,22 +374,91 @@ func bakeOutputs(service types.ServiceConfig, options api.BuildOptions) (outputs
}
}

// localBuildPaths returns the build context paths that live on the local
// filesystem — remote (git or URL) contexts need no fs.read entitlement.
func localBuildPaths(buildConfig types.BuildConfig) []string {
// localBuildPaths returns local build inputs that need an fs.read grant.
func localBuildPaths(project *types.Project, buildConfig types.BuildConfig) []string {
paths := []string{buildConfig.Context}
for _, path := range buildConfig.AdditionalContexts {
paths = append(paths, path)
}
var local []string
for _, path := range paths {
if _, _, err := gitutil.ParseGitRef(path); !strings.Contains(path, "://") && err != nil {
if cwdPath, ok := strings.CutPrefix(path, "cwd://"); ok {
local = append(local, cwdPath)
} else if layout, ok := strings.CutPrefix(path, "oci-layout://"); ok {
local = append(local, ociLayoutPath(layout))
} else if strings.HasPrefix(path, types.ServicePrefix) || strings.HasPrefix(path, "target:") || strings.HasPrefix(path, "docker-image:") {
continue
} else if _, _, err := gitutil.ParseGitRef(path); !strings.Contains(path, "://") && err != nil {
local = append(local, path)
}
}
contextPath := strings.TrimPrefix(buildConfig.Context, "cwd://")
if buildConfig.Dockerfile != "" && buildConfig.DockerfileInline == "" &&

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] Dockerfile directory grant may not deduplicate against a relative context path

When the Dockerfile lives inside a relative context (e.g. context: ./app, dockerfile: Dockerfile), dockerFilePath resolves and EvalSymlinks the path to an absolute directory like /abs/app, but buildConfig.Context was already appended as the raw relative string ./app. Because slices.Compact(slices.Sorted(...)) deduplicates by string equality, both --allow fs.read=./app and --allow fs.read=/abs/app are emitted. Duplicate/overlapping grants are not harmful — buildx accepts them — but the stated intent of the deduplication is not fully achieved for this common case. The existing test uses contextDir = filepath.Join(root, "app") (already absolute), so the relative-path variant is not exercised.

Consider normalising buildConfig.Context with filepath.Abs before appending it to paths, or normalising all collected paths at the end of localBuildPaths before returning them.

Confidence Score
🟡 moderate 57/100

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Compose loader resolves local build contexts to absolute paths before this function runs, so context: ./app won't normally reach this code as ./app. Also, filepath.EvalSymlinks doesn't inherently turn relative paths into absolute ones. The deduplication here removes identical grants; symlink aliases can still produce overlapping grants, but those are harmless. I don't think we need another normalization step here.

(!isRemoteContext(contextPath) || filepath.IsAbs(buildConfig.Dockerfile)) {
local = append(local, dockerFilePath(contextPath, buildConfig.Dockerfile))
}
for _, ref := range buildConfig.Secrets {
secret := project.Secrets[ref.Source]
if secret.Environment == "" && secret.File != "" {
local = append(local, secret.File)
}
}
return local
}

// ociLayoutPath extracts the filesystem path using Buildx's OCI layout syntax.
// The caller removes the scheme; Bake validates the original reference.
// Path extraction adapted from https://github.com/docker/buildx/blob/c7748d0a0bdbdad73fb939cb62fcd4e634e3d7e6/util/ocilayout/parse.go
func ociLayoutPath(path string) string {
if i := strings.LastIndex(path, "@"); i >= 0 && reference.DigestRegexp.MatchString(path[i+1:]) {
path = path[:i]
}
if i := strings.LastIndex(path, ":"); i >= 0 && !isWindowsDrivePath(path, i) {
if reference.TagRegexp.MatchString(path[i+1:]) {
path = path[:i]
}
}
return path
}

// isWindowsDrivePath reports whether colon marks an absolute Windows drive prefix.
// Adapted from https://github.com/docker/buildx/blob/c7748d0a0bdbdad73fb939cb62fcd4e634e3d7e6/util/ocilayout/parse.go
func isWindowsDrivePath(path string, colon int) bool {
if colon != 1 || len(path) < 3 {
return false
}
if path[2] != '/' && path[2] != '\\' {
return false
}
c := path[0]
return ('a' <= c && c <= 'z') || ('A' <= c && c <= 'Z')
}

// localCachePaths extracts local cache sources or destinations. Bake reports
// invalid cache definitions; they must not produce grants here.
func localCachePaths(entries []string, key string) []string {
var paths []string
for _, entry := range entries {
fields, err := csvvalue.Fields(entry, nil)
if err != nil {
continue
}
attrs := make(map[string]string, len(fields))
for _, field := range fields {
name, value, ok := strings.Cut(field, "=")
if !ok {
attrs = nil
break
}
attrs[strings.ToLower(name)] = value
}
if attrs["type"] == "local" && attrs[key] != "" {
paths = append(paths, attrs[key])
}
}
return paths
}

// bakeMetadataPath picks a fresh temporary path for bake's --metadata-file.
// We don't use os.CreateTemp here as we need a temporary file name, but don't
// want it actually created, as bake relies on atomicwriter and this creates
Expand All @@ -392,11 +482,14 @@ func bakeMetadataPath() (string, error) {
func bakeArgs(bake *bakeBuild, metadataFile string, options api.BuildOptions) []string {
args := []string{"bake", "--file", "-", "--progress", "rawjson", "--metadata-file", metadataFile}
// FIXME we should prompt user about this, but this is a breaking change in UX
for _, path := range bake.localPaths {
for _, path := range slices.Compact(slices.Sorted(slices.Values(bake.localPaths))) {
args = append(args, "--allow", "fs.read="+path)
}
if bake.privileged {
args = append(args, "--allow", "security.insecure")
for _, path := range slices.Compact(slices.Sorted(slices.Values(bake.localWritePaths))) {
args = append(args, "--allow", "fs.write="+path)
}
for _, entitlement := range slices.Compact(slices.Sorted(slices.Values(bake.entitlements))) {
args = append(args, "--allow", entitlement)
}
if options.SBOM != "" {
args = append(args, "--sbom="+options.SBOM)
Expand Down Expand Up @@ -573,7 +666,11 @@ func toBakeUlimits(ulimits map[string]*types.UlimitsConfig) []string {
func toBakeSSH(ssh types.SSHConfig) []string {
var s []string
for _, key := range ssh {
s = append(s, fmt.Sprintf("%s=%s", key.ID, key.Path))
if key.Path == "" {
s = append(s, key.ID)
} else {
s = append(s, fmt.Sprintf("%s=%s", key.ID, key.Path))
}
}
return s
}
Expand Down Expand Up @@ -618,15 +715,17 @@ func appendAttest(attests []string, attestType, value string) []string {
return attests
}

// isRemoteContext reports whether the Dockerfile is resolved within a remote source.
func isRemoteContext(ctxName string) bool {
contextType, _ := build.DetectContextType(ctxName)
return contextType == build.ContextTypeGit || contextType == build.ContextTypeRemote || strings.Contains(ctxName, "://")

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[low] isRemoteContext classifies cwd:// as remote, potentially skipping the Dockerfile directory grant

isRemoteContext (introduced in this PR) returns true whenever the context string contains ://:

func isRemoteContext(ctxName string) bool {
    contextType, _ := build.DetectContextType(ctxName)
    return contextType == build.ContextTypeGit || contextType == build.ContextTypeRemote || strings.Contains(ctxName, "://")
}

This matches cwd://-prefixed contexts, which are actually local filesystem paths. The Dockerfile directory grant in localBuildPaths is gated on this function:

if buildConfig.Dockerfile != "" && buildConfig.DockerfileInline == "" &&
    (!isRemoteContext(buildConfig.Context) || filepath.IsAbs(buildConfig.Dockerfile)) {
    local = append(local, filepath.Dir(dockerFilePath(buildConfig.Context, buildConfig.Dockerfile)))
}

When the main context is cwd://some/path and the Dockerfile is a relative path, both conditions are false (isRemoteContext → true, filepath.IsAbs → false), so the Dockerfile directory is never added to the read-grant list — even though cwd:// resolves to a local path that Bake needs to access.

The context path itself is added to local correctly (the strings.CutPrefix("cwd://", ...) branch in the first loop), so only the Dockerfile's parent directory (for .dockerignore and related files) would be missing from the grant.

If cwd:// can appear as buildConfig.Context with a relative dockerfile: path, the build would fail with an entitlement error. Consider excluding cwd:// from the :// check:

return contextType == build.ContextTypeGit || contextType == build.ContextTypeRemote ||
    (strings.Contains(ctxName, "://") && !strings.HasPrefix(ctxName, "cwd://"))
Confidence Score
🟡 moderate 65/100

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed by stripping cwd:// before resolving the Dockerfile path for the read grant. The Bake target keeps its original context and Dockerfile values so Bake handles their resolution.

}

func dockerFilePath(ctxName string, dockerfile string) string {
if dockerfile == "" {
return ""
}
contextType, _ := build.DetectContextType(ctxName)
if contextType == build.ContextTypeGit || contextType == build.ContextTypeRemote {
return dockerfile
}
if strings.Contains(ctxName, "://") {
if isRemoteContext(ctxName) {
return dockerfile
}
if !filepath.IsAbs(dockerfile) {
Expand Down
Loading
Loading