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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 8 additions & 6 deletions build/build.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}()

Expand Down Expand Up @@ -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 {
Expand Down
35 changes: 27 additions & 8 deletions build/opt.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}()

Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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)
})
}
Expand All @@ -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 {
Expand Down
51 changes: 51 additions & 0 deletions policy/policy_error_test.go
Original file line number Diff line number Diff line change
@@ -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))
})
}
}
44 changes: 44 additions & 0 deletions policy/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"path"
"slices"
"strings"
"sync"
"time"

"github.com/containerd/platforms"
Expand All @@ -32,6 +33,9 @@ import (
type Policy struct {
opt Opt
funcs []fun

denyMu sync.Mutex
denyIdentifiers map[string]struct{}
}

type state struct {
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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
}
Expand Down
78 changes: 78 additions & 0 deletions tests/policy_build.go
Original file line number Diff line number Diff line change
@@ -1,18 +1,21 @@
package tests

import (
"encoding/json"
"errors"
"fmt"
"net/url"
"os"
"path/filepath"
"strings"
"testing"

"github.com/containerd/continuity/fs/fstest"
"github.com/containerd/platforms"
"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"
Expand All @@ -25,6 +28,7 @@ import (
var policyBuildTests = []func(t *testing.T, sb integration.Sandbox){
testBuildPolicyAllow,
testBuildPolicyDeny,
testBuildPolicyDenyProgressStream,
testBuildPolicyImageName,
testBuildPolicyEnv,
testBuildPolicyHTTP,
Expand Down Expand Up @@ -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()
Expand Down
Loading