diff --git a/build/build.go b/build/build.go index 738a82190..dab40e7d8 100644 --- a/build/build.go +++ b/build/build.go @@ -272,18 +272,18 @@ func warnOnNoOutput(ctx context.Context, nodes []builder.Node, opts map[string]O logrus.Warnf("%s. Build result will only remain in the build cache. To push result image into registry use --push or to load image into docker use --load", warnNoOutputBuf.String()) } -func newBuildRequests(ctx context.Context, docker *dockerutil.Client, cfg *confutil.Config, drivers map[string][]*noderesolver.ResolvedNode, w progress.Writer, opts map[string]Options) (_ map[string][]*reqForNode, _ func(), retErr error) { +func newBuildRequests(ctx context.Context, docker *dockerutil.Client, cfg *confutil.Config, drivers map[string][]*noderesolver.ResolvedNode, w progress.Writer, opts map[string]Options) (_ map[string][]*reqForNode, _ func(error), retErr error) { reqForNodes := make(map[string][]*reqForNode) - var releasers []func() - releaseAll := func() { + var releasers []func(error) + releaseAll := func(inErr error) { for _, fn := range releasers { - fn() + fn(inErr) } } defer func() { if retErr != nil { - releaseAll() + releaseAll(retErr) } }() @@ -432,7 +432,9 @@ func BuildWithResultHandler(ctx context.Context, nodes []builder.Node, opts map[ if err != nil { return nil, err } - defer release() + defer func() { + release(err) + }() // validate that all links between targets use same drivers if err := validateTargetLinks(reqForNodes, drivers, opts); err != nil { diff --git a/build/opt.go b/build/opt.go index d0758406a..44e5d4e1f 100644 --- a/build/opt.go +++ b/build/opt.go @@ -213,18 +213,27 @@ func (l *policyProgressLogger) sendVertexComplete(started time.Time, err error) l.ch <- &client.SolveStatus{Vertexes: []*client.Vertex{&vtx}} } -func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt *Options, bopts gateway.BuildOpts, cfg *confutil.Config, pw progress.Writer, docker *dockerutil.Client) (_ *client.SolveOpt, release func(), err error) { +func isPolicyEvaluationError(policies []*policy.Policy, err error) bool { + for _, p := range policies { + if p != nil && p.IsPolicyError(err) { + return true + } + } + return false +} + +func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt *Options, bopts gateway.BuildOpts, cfg *confutil.Config, pw progress.Writer, docker *dockerutil.Client) (_ *client.SolveOpt, release func(error), err error) { nodeDriver := node.Driver - defers := make([]func(), 0, 2) - releaseF := func() { + defers := make([]func(error), 0, 2) + releaseF := func(inErr error) { for _, f := range defers { - f() + f(inErr) } } defer func() { if err != nil { - releaseF() + releaseF(err) } }() @@ -432,7 +441,9 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt *O if err != nil { return nil, nil, err } - defers = append(defers, cancel) + defers = append(defers, func(error) { + cancel() + }) opt.Exports[i].Output = func(_ map[string]string) (io.WriteCloser, error) { return w, nil } @@ -478,7 +489,9 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt *O if err != nil { return nil, nil, err } - defers = append(defers, releaseLoad) + defers = append(defers, func(error) { + releaseLoad() + }) if opt.Inputs.policy == nil { if len(opt.Policy) > 0 { @@ -512,8 +525,13 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt *O if len(policyFiles) > 0 { policyLogger = newPolicyProgressLogger(pw, fmt.Sprintf("loading policies %s", strings.Join(policyFiles, ", "))) } + var policies []*policy.Policy if policyLogger != nil { - defers = append(defers, func() { + defers = append(defers, func(inErr error) { + if len(policysession.DenyMessages(inErr)) > 0 || isPolicyEvaluationError(policies, inErr) { + policyLogger.Close(inErr) + return + } policyLogger.Close(nil) }) } @@ -537,6 +555,7 @@ func toSolveOpt(ctx context.Context, node builder.Node, multiDriver bool, opt *O VerifierProvider: policy.SignatureVerifier(cfg), DefaultPlatform: defaultPlatform(bopts), }) + policies = append(policies, p) cbs = append(cbs, p.CheckPolicy) if popt.Strict { if bopts.LLBCaps.Supports(pb.CapSourcePolicySession) != nil { diff --git a/policy/policy_error_test.go b/policy/policy_error_test.go new file mode 100644 index 000000000..1f433ba10 --- /dev/null +++ b/policy/policy_error_test.go @@ -0,0 +1,51 @@ +package policy + +import ( + "errors" + "testing" + + gwpb "github.com/moby/buildkit/frontend/gateway/pb" + solverpb "github.com/moby/buildkit/solver/pb" + "github.com/moby/buildkit/sourcepolicy/policysession" + "github.com/stretchr/testify/require" +) + +func TestPolicyIsPolicyError(t *testing.T) { + tests := []struct { + name string + err error + want bool + }{ + { + name: "matches-recorded-source", + err: errors.New("failed to solve: error evaluating the source policy: source \"docker-image://busybox:latest\" not allowed by policy: action DENY"), + want: true, + }, + { + name: "does-not-match-without-buildkit-pattern", + err: errors.New("failed to parse dockerfile for docker-image://busybox:latest"), + want: false, + }, + { + name: "does-not-match-unrelated-error", + err: errors.New("failed to solve: error evaluating the source policy: source \"docker-image://alpine:latest\" not allowed by policy: action DENY"), + want: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + p := NewPolicy(Opt{}) + req := &policysession.CheckPolicyRequest{ + Source: &gwpb.ResolveSourceMetaResponse{ + Source: &solverpb.SourceOp{ + Identifier: "docker-image://busybox:latest", + }, + }, + } + p.recordDenyIdentifier(req) + + require.Equal(t, tt.want, p.IsPolicyError(tt.err)) + }) + } +} diff --git a/policy/validate.go b/policy/validate.go index 91fd56112..ef0ae71a3 100644 --- a/policy/validate.go +++ b/policy/validate.go @@ -10,6 +10,7 @@ import ( "path" "slices" "strings" + "sync" "time" "github.com/containerd/platforms" @@ -32,6 +33,9 @@ import ( type Policy struct { opt Opt funcs []fun + + denyMu sync.Mutex + denyIdentifiers map[string]struct{} } type state struct { @@ -84,6 +88,43 @@ func (p *Policy) log(level logrus.Level, format string, v ...any) { p.opt.Log(level, fmt.Sprintf(format, v...)) } +func (p *Policy) recordDenyIdentifier(req *policysession.CheckPolicyRequest) { + if p == nil || req == nil || req.Source == nil || req.Source.Source == nil { + return + } + + p.denyMu.Lock() + defer p.denyMu.Unlock() + + if p.denyIdentifiers == nil { + p.denyIdentifiers = make(map[string]struct{}) + } + id := strings.TrimSpace(req.Source.Source.Identifier) + if id == "" { + return + } + p.denyIdentifiers[id] = struct{}{} +} + +func (p *Policy) IsPolicyError(err error) bool { + if p == nil || err == nil { + return false + } + errText := err.Error() + // TODO: replace this string matching with a typed BuildKit error that is + // always attached for policy DENY decisions. + p.denyMu.Lock() + defer p.denyMu.Unlock() + for id := range p.denyIdentifiers { + pattern := fmt.Sprintf("source %q not allowed by policy: action %s", id, moby_buildkit_v1_sourcepolicy.PolicyAction_DENY.String()) + if strings.Contains(errText, pattern) { + return true + } + } + + return false +} + func (p *Policy) CheckPolicy(ctx context.Context, req *policysession.CheckPolicyRequest) (*policysession.DecisionResponse, *gwpb.ResolveSourceMetaRequest, error) { if req.Source == nil || req.Source.Source == nil { return nil, nil, errors.Errorf("no source info in request") @@ -307,6 +348,9 @@ func (p *Policy) CheckPolicy(ctx context.Context, req *policysession.CheckPolicy for _, dm := range resp.DenyMessages { p.log(logrus.InfoLevel, " - %s", dm.Message) } + if resp.Action == moby_buildkit_v1_sourcepolicy.PolicyAction_DENY { + p.recordDenyIdentifier(req) + } return resp, nil, nil } diff --git a/tests/policy_build.go b/tests/policy_build.go index fe8180545..c04bd1a98 100644 --- a/tests/policy_build.go +++ b/tests/policy_build.go @@ -1,11 +1,13 @@ package tests import ( + "encoding/json" "errors" "fmt" "net/url" "os" "path/filepath" + "strings" "testing" "github.com/containerd/continuity/fs/fstest" @@ -13,6 +15,7 @@ import ( "github.com/distribution/reference" "github.com/docker/buildx/util/gitutil" "github.com/docker/buildx/util/gitutil/gittestutil" + "github.com/moby/buildkit/client" "github.com/moby/buildkit/identity" "github.com/moby/buildkit/util/contentutil" "github.com/moby/buildkit/util/testutil" @@ -25,6 +28,7 @@ import ( var policyBuildTests = []func(t *testing.T, sb integration.Sandbox){ testBuildPolicyAllow, testBuildPolicyDeny, + testBuildPolicyDenyProgressStream, testBuildPolicyImageName, testBuildPolicyEnv, testBuildPolicyHTTP, @@ -103,6 +107,80 @@ decision := {"allow": allow, "deny_msg": deny_msg} require.Contains(t, string(out), "DENY") } +func testBuildPolicyDenyProgressStream(t *testing.T, sb integration.Sandbox) { + skipNoCompatBuildKit(t, sb, ">= 0.26.0-0", "policy input requires BuildKit v0.26.0+") + dockerfile := []byte(` +FROM busybox:latest +RUN echo policy-nope +`) + policyFile := []byte(` +package docker + +default allow = false + +allow if not input.image + +decision := {"allow": allow} +`) + dir := tmpdir( + t, + fstest.CreateFile("Dockerfile", dockerfile, 0600), + fstest.CreateFile("policy.rego", policyFile, 0600), + ) + policyPath := filepath.Join(dir, "policy.rego") + + cmd := buildxCmd(sb, withDir(dir), withArgs( + "build", + "--progress=rawjson", + "--policy", "filename="+policyPath, + "--output=type=cacheonly", + dir, + )) + out, err := cmd.CombinedOutput() + outStr := string(out) + require.Error(t, err, outStr) + + policyVertexName := "loading policies " + policyPath + sawPolicyVertex := false + sawPolicyCompleted := false + policyVertexDigest := "" + policyVertexError := "" + + for line := range strings.SplitSeq(outStr, "\n") { + line = strings.TrimSpace(line) + if !strings.HasPrefix(line, "{") { + continue + } + var st client.SolveStatus + require.NoError(t, json.Unmarshal([]byte(line), &st), line) + for _, v := range st.Vertexes { + if v == nil { + continue + } + if v.Name == policyVertexName { + sawPolicyVertex = true + if v.Digest != "" { + policyVertexDigest = string(v.Digest) + } + } + if policyVertexDigest == "" || string(v.Digest) != policyVertexDigest { + continue + } + if v.Completed != nil { + sawPolicyCompleted = true + } + if v.Error != "" { + policyVertexError = v.Error + } + } + } + + require.True(t, sawPolicyVertex, outStr) + require.True(t, sawPolicyCompleted, outStr) + require.Contains(t, policyVertexError, "not allowed by policy", outStr) + require.Contains(t, outStr, "not allowed by policy") +} + func testBuildPolicyImageName(t *testing.T, sb integration.Sandbox) { skipNoCompatBuildKit(t, sb, ">= 0.26.0-0", "policy input requires BuildKit v0.26.0+") registry, err := sb.NewRegistry()