Skip to content

Commit

Permalink
sourcepolicy: add validations for nil values
Browse files Browse the repository at this point in the history
Signed-off-by: Tonis Tiigi <tonistiigi@gmail.com>
(cherry picked from commit 4e2569e796aae398648082689d70ca1d4f4f74a8)
(cherry picked from commit caea271063973c6903be08c1ebbc7c103f67805f)
  • Loading branch information
tonistiigi committed Jan 31, 2024
1 parent 96663dd commit e1924dc
Show file tree
Hide file tree
Showing 5 changed files with 137 additions and 0 deletions.
1 change: 1 addition & 0 deletions client/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,7 @@ func TestIntegration(t *testing.T) {
testValidateInvalidConfig,
testValidatePlatformsEmpty,
testValidatePlatformsInvalid,
testValidateSourcePolicy,
)
}

Expand Down
102 changes: 102 additions & 0 deletions client/validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/moby/buildkit/client/llb"
"github.com/moby/buildkit/exporter/containerimage/exptypes"
"github.com/moby/buildkit/frontend/gateway/client"
sppb "github.com/moby/buildkit/sourcepolicy/pb"
"github.com/moby/buildkit/util/testutil/integration"
ocispecs "github.com/opencontainers/image-spec/specs-go/v1"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -204,3 +205,104 @@ func testValidatePlatformsInvalid(t *testing.T, sb integration.Sandbox) {
})
}
}

func testValidateSourcePolicy(t *testing.T, sb integration.Sandbox) {
requiresLinux(t)

ctx := sb.Context()

c, err := New(ctx, sb.Address())
require.NoError(t, err)
defer c.Close()

tcases := []struct {
name string
value *sppb.Policy
exp string
}{
// this condition fails on marshaling atm
// {
// name: "nilrule",
// value: &sppb.Policy{
// Rules: []*sppb.Rule{nil},
// },
// exp: "",
// },
{
name: "nilselector",
value: &sppb.Policy{
Rules: []*sppb.Rule{
{
Action: sppb.PolicyAction_CONVERT,
},
},
},
exp: "invalid nil selector in policy",
},
{
name: "emptyaction",
value: &sppb.Policy{
Rules: []*sppb.Rule{
{
Action: sppb.PolicyAction(9000),
Selector: &sppb.Selector{
Identifier: "docker-image://docker.io/library/alpine:latest",
},
},
},
},
exp: "unknown type",
},
{
name: "nilupdates",
value: &sppb.Policy{
Rules: []*sppb.Rule{
{
Action: sppb.PolicyAction_CONVERT,
Selector: &sppb.Selector{
Identifier: "docker-image://docker.io/library/alpine:latest",
},
},
},
},
exp: "missing destination for convert rule",
},
}

for _, tc := range tcases {
t.Run(tc.name, func(t *testing.T) {

var viaFrontend bool

b := func(ctx context.Context, c client.Client) (*client.Result, error) {
def, err := llb.Image("alpine").Marshal(ctx)
if err != nil {
return nil, err
}

req := client.SolveRequest{
Evaluate: true,
Definition: def.ToPB(),
}
if viaFrontend {
req.SourcePolicies = []*sppb.Policy{
tc.value,
}
}
return c.Solve(ctx, req)
}

_, err = c.Build(ctx, SolveOpt{
SourcePolicy: tc.value,
}, "", b, nil)
require.Error(t, err)
require.Contains(t, err.Error(), tc.exp)

viaFrontend = true
_, err = c.Build(ctx, SolveOpt{}, "", b, nil)
require.Error(t, err)
require.Contains(t, err.Error(), tc.exp)

})
}
}
8 changes: 8 additions & 0 deletions solver/llbsolver/bridge.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ func (b *llbBridge) loadResult(ctx context.Context, def *pb.Definition, cacheImp
}
var polEngine SourcePolicyEvaluator
if srcPol != nil || len(pol) > 0 {
for _, p := range pol {
if p == nil {
return nil, errors.Errorf("invalid nil policy")
}
if err := validateSourcePolicy(*p); err != nil {
return nil, err
}
}
if srcPol != nil {
pol = append([]*spb.Policy{srcPol}, pol...)
}
Expand Down
23 changes: 23 additions & 0 deletions solver/llbsolver/solver.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,9 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
j.SetValue(keyEntitlements, set)

if srcPol != nil {
if err := validateSourcePolicy(*srcPol); err != nil {
return nil, err
}
j.SetValue(keySourcePolicy, *srcPol)
}

Expand Down Expand Up @@ -595,6 +598,23 @@ func (s *Solver) Solve(ctx context.Context, id string, sessionID string, req fro
}, nil
}

func validateSourcePolicy(pol spb.Policy) error {
for _, r := range pol.Rules {
if r == nil {
return errors.New("invalid nil rule in policy")
}
if r.Selector == nil {
return errors.New("invalid nil selector in policy")
}
for _, c := range r.Selector.Constraints {
if c == nil {
return errors.New("invalid nil constraint in policy")
}
}
}
return nil
}

func runCacheExporters(ctx context.Context, exporters []RemoteCacheExporter, j *solver.Job, cached *result.Result[solver.CachedResult], inp *result.Result[cache.ImmutableRef]) (map[string]string, error) {
eg, ctx := errgroup.WithContext(ctx)
g := session.NewGroup(j.SessionID)
Expand Down Expand Up @@ -991,6 +1011,9 @@ func loadSourcePolicy(b solver.Builder) (*spb.Policy, error) {
return errors.Errorf("invalid source policy %T", v)
}
for _, f := range x.Rules {
if f == nil {
return errors.Errorf("invalid nil policy rule")
}
r := *f
srcPol.Rules = append(srcPol.Rules, &r)
}
Expand Down
3 changes: 3 additions & 0 deletions sourcepolicy/matcher.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ import (

func match(ctx context.Context, src *selectorCache, ref string, attrs map[string]string) (bool, error) {
for _, c := range src.Constraints {
if c == nil {
return false, errors.Errorf("invalid nil constraint for %v", src)
}
switch c.Condition {
case spb.AttrMatch_EQUAL:
if attrs[c.Key] != c.Value {
Expand Down

0 comments on commit e1924dc

Please sign in to comment.