policy: mark policy progress vertex on DENY build failures

Track denied source identifiers during policy evaluation and flag the policy progress vertex as failed when BuildKit returns a matching DENY error pattern. This improves the progress output of policy error and shows last policy logs with the build error. Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
2026-02-18 17:19:54 -08:00
parent a0103f4278
commit 048ebea483
4 changed files with 135 additions and 14 deletions
@@ -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 {
@@ -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 {
@@ -0,0 +1,56 @@
+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 TestPolicyIsPolicyErrorMatchesRecordedSource(t *testing.T) {
+	p := NewPolicy(Opt{})
+	req := &policysession.CheckPolicyRequest{
+		Source: &gwpb.ResolveSourceMetaResponse{
+			Source: &solverpb.SourceOp{
+				Identifier: "docker-image://busybox:latest",
+			},
+		},
+	}
+	p.recordDenyIdentifier(req)
+
+	err := errors.New("failed to solve: error evaluating the source policy: source \"docker-image://busybox:latest\" not allowed by policy: action DENY")
+	require.True(t, p.IsPolicyError(err))
+}
+
+func TestPolicyIsPolicyErrorDoesNotMatchWithoutBuildkitPattern(t *testing.T) {
+	p := NewPolicy(Opt{})
+	req := &policysession.CheckPolicyRequest{
+		Source: &gwpb.ResolveSourceMetaResponse{
+			Source: &solverpb.SourceOp{
+				Identifier: "docker-image://busybox:latest",
+			},
+		},
+	}
+	p.recordDenyIdentifier(req)
+
+	err := errors.New("failed to parse dockerfile for docker-image://busybox:latest")
+	require.False(t, p.IsPolicyError(err))
+}
+
+func TestPolicyIsPolicyErrorDoesNotMatchUnrelatedError(t *testing.T) {
+	p := NewPolicy(Opt{})
+	req := &policysession.CheckPolicyRequest{
+		Source: &gwpb.ResolveSourceMetaResponse{
+			Source: &solverpb.SourceOp{
+				Identifier: "docker-image://busybox:latest",
+			},
+		},
+	}
+	p.recordDenyIdentifier(req)
+
+	err := errors.New("failed to solve: error evaluating the source policy: source \"docker-image://alpine:latest\" not allowed by policy: action DENY")
+	require.False(t, p.IsPolicyError(err))
+}
@@ -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
 }