Skip to content

Commit

Permalink
Fix: Have the policy-tester library check policy result.
Browse files Browse the repository at this point in the history
🐛 The policy tester library blindly checked the errors, but with multiple authorities we might have errors, but still successfully evaluated the policy.

Here is the comment from the ~equivalent call to `ValidatePolicy(` from `validator.go` for comparison:

```go
			switch {
			// Return AuthorityMatches before errors, since even if there
			// are errors, if there are 0 or more authorities that match,
			// it will pass the Policy. Of course, a CIP level policy can
			// override this behaviour, but that has been checked above and
			// if it failed, it will nil out the policyResult.
			case result.policyResult != nil:
				policyResults[result.name] = result.policyResult
			case len(result.errors) > 0:
				ret[result.name] = append(ret[result.name], result.errors...)
			default:
				ret[result.name] = append(ret[result.name], fmt.Errorf("failed to process policy: %s", result.name))
			}

```

... with this I was able to successfully run the policy tester using a policy that had a separate authority for SLSA `v0.2` and `v1` predicate types without it failing.

/kind bug

Signed-off-by: Matt Moore <mattmoor@chainguard.dev>
  • Loading branch information
mattmoor committed May 30, 2023
1 parent 1b402ff commit b4622d1
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions pkg/policy/verifier.go
Original file line number Diff line number Diff line change
Expand Up @@ -157,18 +157,23 @@ func (i *impl) Verify(ctx context.Context, ref name.Reference, kc authn.Keychain
opts = append(opts, ociremote.WithRemoteOptions(remote.WithAuthFromKeychain(kc)))

for _, p := range matches {
_, errs := webhook.ValidatePolicy(ctx, "" /* namespace */, ref, p, kc, opts...)
for _, err := range errs {
var fe *apis.FieldError
if errors.As(err, &fe) {
if warnFE := fe.Filter(apis.WarningLevel); warnFE != nil {
i.ww("%v", warnFE)
res, errs := webhook.ValidatePolicy(ctx, "" /* namespace */, ref, p, kc, opts...)
if res != nil {
// Ignore the errors for other authorities if we got a policy result.
} else {
// If we didn't get a policy result, then surface any errors.
for _, err := range errs {
var fe *apis.FieldError
if errors.As(err, &fe) {
if warnFE := fe.Filter(apis.WarningLevel); warnFE != nil {
i.ww("%v", warnFE)
}
if errorFE := fe.Filter(apis.ErrorLevel); errorFE != nil {
return errorFE
}
} else {
return err

Check warning on line 175 in pkg/policy/verifier.go

View check run for this annotation

Codecov / codecov/patch

pkg/policy/verifier.go#L174-L175

Added lines #L174 - L175 were not covered by tests
}
if errorFE := fe.Filter(apis.ErrorLevel); errorFE != nil {
return errorFE
}
} else {
return err
}
}
}
Expand Down

0 comments on commit b4622d1

Please sign in to comment.