Repository navigation
fix(bake): grant permissions for configured build inputs #14325
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -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" | ||||||||||||||
|
|
@@ -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) { | ||||||||||||||
|
|
@@ -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, ",")...) | ||||||||||||||
| } | ||||||||||||||
| } | ||||||||||||||
| // Bake implicitly forwards the default agent for SSH Git contexts. | ||||||||||||||
| if len(ssh) == 0 { | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ( 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
Suggested change
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||
| 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 { | ||||||||||||||
|
|
@@ -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, | ||||||||||||||
|
|
@@ -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` — | ||||||||||||||
|
|
@@ -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 == "" && | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Consider normalising
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||||||||||||||
| (!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 | ||||||||||||||
|
|
@@ -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) | ||||||||||||||
|
|
@@ -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 | ||||||||||||||
| } | ||||||||||||||
|
|
@@ -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, "://") | ||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [low]
func isRemoteContext(ctxName string) bool {
contextType, _ := build.DetectContextType(ctxName)
return contextType == build.ContextTypeGit || contextType == build.ContextTypeRemote || strings.Contains(ctxName, "://")
}This matches 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 The context path itself is added to If return contextType == build.ContextTypeGit || contextType == build.ContextTypeRemote ||
(strings.Contains(ctxName, "://") && !strings.HasPrefix(ctxName, "cwd://"))
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed by stripping |
||||||||||||||
| } | ||||||||||||||
|
|
||||||||||||||
| 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) { | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
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. Ifkey.Pathhas a trailing comma or contains consecutive commas (e.g. a malformed or programmatically constructed config), an empty string is appended tobake.localPaths. Theslices.Compact(slices.Sorted(...))deduplication inbakeArgssorts paths ascending — the empty string sorts to the front and, since it appears only once, survivesCompact— and this would emit--allow fs.read=to Bake, which may be rejected or behave unexpectedly.Consider filtering out empty segments:
There was a problem hiding this comment.
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.Splitand 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.