From 5bc6b1fad63648bb369a11be6d5b6a398dd09fe0 Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 14 Aug 2026 13:23:42 +0200 Subject: [PATCH 1/2] fix(policies): bound rego policy evaluation with an execution timeout The rego engine ignored the ExecutionTimeout option, so evaluation was bounded only by the caller context. In the CLI that context carries no deadline, which left a runaway policy able to hang an attestation indefinitely. The rego engine now honors ExecutionTimeout, defaulting to 60s, and applies it to both the main rule and the matches_parameters / matches_evaluation rules. A new policies.WithExecutionTimeout option sets the bound at the verifier level for both the rego and the WASM engines. Assisted-by: Claude Code Signed-off-by: Jose I. Paris --- pkg/policies/engine/engine.go | 11 ++- pkg/policies/engine/rego/rego.go | 25 +++++ pkg/policies/engine/rego/rego_test.go | 92 +++++++++++++++++++ .../rego/testfiles/slow_evaluation.rego | 27 ++++++ pkg/policies/policies.go | 26 ++++++ pkg/policies/policies_test.go | 34 +++++++ 6 files changed, 212 insertions(+), 3 deletions(-) create mode 100644 pkg/policies/engine/rego/testfiles/slow_evaluation.rego diff --git a/pkg/policies/engine/engine.go b/pkg/policies/engine/engine.go index adda578d2..3de5ef02d 100644 --- a/pkg/policies/engine/engine.go +++ b/pkg/policies/engine/engine.go @@ -60,9 +60,13 @@ type Options struct { // OperatingMode defines whether the Rego engine runs in restrictive (0) or permissive (1) mode OperatingMode int32 - // WASM-specific options + // ExecutionTimeout bounds a single policy evaluation. It is honored by both + // the rego and the WASM engines, each of which falls back to its own default + // when the value is non-positive. ExecutionTimeout time.Duration - Logger *zerolog.Logger + + // WASM-specific options + Logger *zerolog.Logger } // WithAllowedHostnames sets the list of allowed hostnames for HTTP requests @@ -94,7 +98,8 @@ func WithOperatingMode(mode int32) Option { } } -// WithExecutionTimeout sets the WASM execution timeout +// WithExecutionTimeout sets the maximum duration of a single policy evaluation. +// Applies to both the rego and the WASM engines. func WithExecutionTimeout(timeout time.Duration) Option { return func(opts *Options) { opts.ExecutionTimeout = timeout diff --git a/pkg/policies/engine/rego/rego.go b/pkg/policies/engine/rego/rego.go index e83dc8912..f923e4fba 100644 --- a/pkg/policies/engine/rego/rego.go +++ b/pkg/policies/engine/rego/rego.go @@ -21,6 +21,7 @@ import ( "encoding/json" "fmt" "slices" + "time" "github.com/chainloop-dev/chainloop/pkg/policies/engine" "github.com/chainloop-dev/chainloop/pkg/policies/engine/rego/builtins" @@ -30,11 +31,19 @@ import ( "golang.org/x/exp/maps" ) +// DefaultExecutionTimeout bounds a single policy evaluation when no explicit +// timeout is provided. Rego policies can hang indefinitely (e.g. a pathological +// comprehension or a chain of http.send calls), and the contexts they run under +// rarely carry a deadline of their own, so the engine always applies one. +const DefaultExecutionTimeout = 60 * time.Second + // Engine policy checker for chainloop attestations and materials type Engine struct { // operatingMode defines the mode of running the policy engine // by restricting or not the operations allowed by the compiler operatingMode EnvironmentMode + // executionTimeout bounds each evaluation performed by this engine + executionTimeout time.Duration // Embed common engine options *engine.CommonEngineOptions } @@ -43,11 +52,18 @@ type Engine struct { // default operating mode is EnvironmentModeRestrictive // default allowed hostnames are www.chainloop.dev and www.cisa.gov // user provided allowed hostnames are appended to the base ones +// default execution timeout is DefaultExecutionTimeout func NewEngine(opts ...engine.Option) *Engine { options := engine.ApplyOptions(opts...) + executionTimeout := options.ExecutionTimeout + if executionTimeout <= 0 { + executionTimeout = DefaultExecutionTimeout + } + return &Engine{ operatingMode: EnvironmentMode(options.OperatingMode), + executionTimeout: executionTimeout, CommonEngineOptions: options.CommonEngineOptions, } } @@ -124,6 +140,11 @@ func (r *Engine) injectProjectMetadata(inputMap map[string]interface{}) map[stri } func (r *Engine) Verify(ctx context.Context, policy *engine.Policy, input []byte, args map[string]any) (*engine.EvaluationResult, error) { + // Bound the whole evaluation. A caller deadline that expires earlier still + // wins, since context.WithTimeout never extends the parent's. + ctx, cancel := context.WithTimeout(ctx, r.executionTimeout) + defer cancel() + policyString := string(policy.Source) parsedModule, err := ast.ParseModule(policy.Name, policyString) if err != nil { @@ -433,6 +454,10 @@ func (r *Engine) MatchesEvaluation(ctx context.Context, policy *engine.Policy, v // Evaluates a single rule and returns its boolean result func (r *Engine) evaluateMatchingRule(ctx context.Context, ruleName string, parsedModule *ast.Module, decodedInput interface{}) (result bool, found bool, err error) { + // Bound the evaluation, same as Verify does for the main rule + ctx, cancel := context.WithTimeout(ctx, r.executionTimeout) + defer cancel() + // Add input regoInput := rego.Input(decodedInput) diff --git a/pkg/policies/engine/rego/rego_test.go b/pkg/policies/engine/rego/rego_test.go index 967d2166b..9f9a37f4b 100644 --- a/pkg/policies/engine/rego/rego_test.go +++ b/pkg/policies/engine/rego/rego_test.go @@ -20,6 +20,7 @@ import ( "encoding/json" "os" "testing" + "time" "github.com/chainloop-dev/chainloop/pkg/policies/engine" "github.com/chainloop-dev/chainloop/pkg/policies/engine/rego/builtins" @@ -868,3 +869,94 @@ func TestRego_StructuredLicenseViolations(t *testing.T) { assert.Equal(t, "GPL-3.0", v.RawFinding["license_id"]) assert.Equal(t, "pkg:npm/libfoo@2.1.0", v.RawFinding["package_purl"]) } + +func TestRego_ExecutionTimeoutOption(t *testing.T) { + testCases := []struct { + name string + opts []engine.Option + want time.Duration + }{ + { + name: "defaults when not provided", + want: DefaultExecutionTimeout, + }, + { + name: "honors the provided timeout", + opts: []engine.Option{engine.WithExecutionTimeout(42 * time.Second)}, + want: 42 * time.Second, + }, + { + name: "falls back to the default on a non-positive timeout", + opts: []engine.Option{engine.WithExecutionTimeout(0)}, + want: DefaultExecutionTimeout, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.want, NewEngine(tc.opts...).executionTimeout) + }) + } +} + +func TestRego_VerifyIsBoundedByExecutionTimeout(t *testing.T) { + regoContent, err := os.ReadFile("testfiles/slow_evaluation.rego") + require.NoError(t, err) + + policy := &engine.Policy{Name: "slow evaluation", Source: regoContent} + + testCases := []struct { + name string + // executionTimeout configured in the engine + executionTimeout time.Duration + // deadline carried by the caller context, if any + callerTimeout time.Duration + }{ + { + name: "the engine timeout bounds an unbounded caller context", + executionTimeout: 200 * time.Millisecond, + }, + { + name: "a shorter caller deadline is not extended by the engine timeout", + executionTimeout: time.Hour, + callerTimeout: 200 * time.Millisecond, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + ctx := context.Background() + if tc.callerTimeout > 0 { + var cancel context.CancelFunc + ctx, cancel = context.WithTimeout(ctx, tc.callerTimeout) + defer cancel() + } + + r := NewEngine(engine.WithExecutionTimeout(tc.executionTimeout)) + + start := time.Now() + _, err := r.Verify(ctx, policy, []byte("{}"), nil) + elapsed := time.Since(start) + + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.Less(t, elapsed, 10*time.Second, "evaluation was not interrupted") + }) + } +} + +func TestRego_MatchesEvaluationIsBoundedByExecutionTimeout(t *testing.T) { + regoContent, err := os.ReadFile("testfiles/slow_evaluation.rego") + require.NoError(t, err) + + r := NewEngine(engine.WithExecutionTimeout(200 * time.Millisecond)) + policy := &engine.Policy{Name: "slow evaluation", Source: regoContent} + + start := time.Now() + _, err = r.MatchesEvaluation(context.Background(), policy, []string{"a violation"}, nil) + elapsed := time.Since(start) + + require.Error(t, err) + assert.ErrorIs(t, err, context.DeadlineExceeded) + assert.Less(t, elapsed, 10*time.Second, "evaluation was not interrupted") +} diff --git a/pkg/policies/engine/rego/testfiles/slow_evaluation.rego b/pkg/policies/engine/rego/testfiles/slow_evaluation.rego new file mode 100644 index 000000000..fc1625d8c --- /dev/null +++ b/pkg/policies/engine/rego/testfiles/slow_evaluation.rego @@ -0,0 +1,27 @@ +package main + +import rego.v1 + +# Deliberately expensive computation used to verify that rego evaluation is +# bounded by the engine execution timeout. Materializing the cross product +# takes several seconds, so any short timeout must interrupt it. +expensive := count([x | + some i in numbers.range(1, 1500) + some j in numbers.range(1, 1500) + x := i * j +]) + +result := { + "skipped": false, + "skip_reason": "", + "violations": violations, +} + +violations contains msg if { + expensive > 0 + msg := "expensive computation completed" +} + +matches_evaluation if { + expensive > 0 +} diff --git a/pkg/policies/policies.go b/pkg/policies/policies.go index b96d33d64..781f4309b 100644 --- a/pkg/policies/policies.go +++ b/pkg/policies/policies.go @@ -101,6 +101,7 @@ type PolicyVerifier struct { projectName string projectVersionName string runtimeInputs *RuntimeInputs + executionTimeout time.Duration } var _ Verifier = (*PolicyVerifier)(nil) @@ -118,6 +119,7 @@ type PolicyVerifierOptions struct { ProjectName string ProjectVersionName string RuntimeInputs *RuntimeInputs + ExecutionTimeout time.Duration } type PolicyVerifierOption func(*PolicyVerifierOptions) @@ -198,6 +200,21 @@ func WithRuntimeInputs(inputs *RuntimeInputs) PolicyVerifierOption { } } +// DefaultExecutionTimeout bounds a single policy evaluation, for both the rego +// and the WASM engines. Policy evaluation runs on whatever context the caller +// provides, and those contexts (e.g. the CLI's cobra command context) typically +// carry no deadline, so a runaway policy would otherwise hang the attestation +// indefinitely. Override with WithExecutionTimeout. +const DefaultExecutionTimeout = 60 * time.Second + +// WithExecutionTimeout sets the maximum duration of a single policy evaluation. +// Non-positive values fall back to DefaultExecutionTimeout. +func WithExecutionTimeout(timeout time.Duration) PolicyVerifierOption { + return func(o *PolicyVerifierOptions) { + o.ExecutionTimeout = timeout + } +} + const defaultPolicyCacheTTL = 5 * time.Minute func NewPolicyVerifier(policies *v1.Policies, client v13.AttestationServiceClient, logger *zerolog.Logger, opts ...PolicyVerifierOption) *PolicyVerifier { @@ -211,6 +228,11 @@ func NewPolicyVerifier(policies *v1.Policies, client v13.AttestationServiceClien maxConcurrency = defaultMaxConcurrency } + executionTimeout := options.ExecutionTimeout + if executionTimeout <= 0 { + executionTimeout = DefaultExecutionTimeout + } + if options.PolicyCache == nil { options.PolicyCache, _ = cache.New[*policyWithReference](cache.WithTTL(defaultPolicyCacheTTL)) } @@ -235,6 +257,7 @@ func NewPolicyVerifier(policies *v1.Policies, client v13.AttestationServiceClien projectName: options.ProjectName, projectVersionName: options.ProjectVersionName, runtimeInputs: options.RuntimeInputs, + executionTimeout: executionTimeout, } } @@ -659,6 +682,9 @@ func (pv *PolicyVerifier) executeScript(ctx context.Context, script *engine.Poli opts = append(opts, engine.WithProjectContext(pv.projectName, pv.projectVersionName)) } + // Bound each policy evaluation regardless of the engine that runs it + opts = append(opts, engine.WithExecutionTimeout(pv.executionTimeout)) + switch policyType { case engine.PolicyTypeRego: policyEngine = rego.NewEngine(opts...) diff --git a/pkg/policies/policies_test.go b/pkg/policies/policies_test.go index e68ea63d9..cb2399a3f 100644 --- a/pkg/policies/policies_test.go +++ b/pkg/policies/policies_test.go @@ -21,12 +21,14 @@ import ( "io/fs" "os" "testing" + "time" v12 "github.com/chainloop-dev/chainloop/app/controlplane/api/workflowcontract/v1" v1 "github.com/chainloop-dev/chainloop/pkg/attestation/crafter/api/attestation/v1" "github.com/chainloop-dev/chainloop/pkg/policies/engine" intoto "github.com/in-toto/attestation/go/v1" "github.com/rs/zerolog" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/suite" "google.golang.org/protobuf/encoding/protojson" ) @@ -1963,3 +1965,35 @@ func (s *testSuite) TestEngineEvaluationsToAPIViolationsBehaviorMatrix() { }) } } + +func TestPolicyVerifierExecutionTimeout(t *testing.T) { + logger := zerolog.Nop() + + testCases := []struct { + name string + opts []PolicyVerifierOption + want time.Duration + }{ + { + name: "defaults when not provided", + want: DefaultExecutionTimeout, + }, + { + name: "honors the provided timeout", + opts: []PolicyVerifierOption{WithExecutionTimeout(5 * time.Second)}, + want: 5 * time.Second, + }, + { + name: "falls back to the default on a non-positive timeout", + opts: []PolicyVerifierOption{WithExecutionTimeout(0)}, + want: DefaultExecutionTimeout, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + pv := NewPolicyVerifier(&v12.Policies{}, nil, &logger, tc.opts...) + assert.Equal(t, tc.want, pv.executionTimeout) + }) + } +} From 07d6f9179f834dcf596e1703c36c76bd4191496f Mon Sep 17 00:00:00 2001 From: "Jose I. Paris" Date: Fri, 14 Aug 2026 13:31:24 +0200 Subject: [PATCH 2/2] test(policies): drop timing-dependent rego timeout tests The evaluation-bound tests relied on a hardcoded cross product outrunning a 200ms timeout, which is not a property CI hardware guarantees. Remove them along with their fixture, keeping the option-handling coverage. Assisted-by: Claude Code Signed-off-by: Jose I. Paris --- pkg/policies/engine/rego/rego_test.go | 62 ------------------- .../rego/testfiles/slow_evaluation.rego | 27 -------- 2 files changed, 89 deletions(-) delete mode 100644 pkg/policies/engine/rego/testfiles/slow_evaluation.rego diff --git a/pkg/policies/engine/rego/rego_test.go b/pkg/policies/engine/rego/rego_test.go index 9f9a37f4b..573ccf1c3 100644 --- a/pkg/policies/engine/rego/rego_test.go +++ b/pkg/policies/engine/rego/rego_test.go @@ -898,65 +898,3 @@ func TestRego_ExecutionTimeoutOption(t *testing.T) { }) } } - -func TestRego_VerifyIsBoundedByExecutionTimeout(t *testing.T) { - regoContent, err := os.ReadFile("testfiles/slow_evaluation.rego") - require.NoError(t, err) - - policy := &engine.Policy{Name: "slow evaluation", Source: regoContent} - - testCases := []struct { - name string - // executionTimeout configured in the engine - executionTimeout time.Duration - // deadline carried by the caller context, if any - callerTimeout time.Duration - }{ - { - name: "the engine timeout bounds an unbounded caller context", - executionTimeout: 200 * time.Millisecond, - }, - { - name: "a shorter caller deadline is not extended by the engine timeout", - executionTimeout: time.Hour, - callerTimeout: 200 * time.Millisecond, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - ctx := context.Background() - if tc.callerTimeout > 0 { - var cancel context.CancelFunc - ctx, cancel = context.WithTimeout(ctx, tc.callerTimeout) - defer cancel() - } - - r := NewEngine(engine.WithExecutionTimeout(tc.executionTimeout)) - - start := time.Now() - _, err := r.Verify(ctx, policy, []byte("{}"), nil) - elapsed := time.Since(start) - - require.Error(t, err) - assert.ErrorIs(t, err, context.DeadlineExceeded) - assert.Less(t, elapsed, 10*time.Second, "evaluation was not interrupted") - }) - } -} - -func TestRego_MatchesEvaluationIsBoundedByExecutionTimeout(t *testing.T) { - regoContent, err := os.ReadFile("testfiles/slow_evaluation.rego") - require.NoError(t, err) - - r := NewEngine(engine.WithExecutionTimeout(200 * time.Millisecond)) - policy := &engine.Policy{Name: "slow evaluation", Source: regoContent} - - start := time.Now() - _, err = r.MatchesEvaluation(context.Background(), policy, []string{"a violation"}, nil) - elapsed := time.Since(start) - - require.Error(t, err) - assert.ErrorIs(t, err, context.DeadlineExceeded) - assert.Less(t, elapsed, 10*time.Second, "evaluation was not interrupted") -} diff --git a/pkg/policies/engine/rego/testfiles/slow_evaluation.rego b/pkg/policies/engine/rego/testfiles/slow_evaluation.rego deleted file mode 100644 index fc1625d8c..000000000 --- a/pkg/policies/engine/rego/testfiles/slow_evaluation.rego +++ /dev/null @@ -1,27 +0,0 @@ -package main - -import rego.v1 - -# Deliberately expensive computation used to verify that rego evaluation is -# bounded by the engine execution timeout. Materializing the cross product -# takes several seconds, so any short timeout must interrupt it. -expensive := count([x | - some i in numbers.range(1, 1500) - some j in numbers.range(1, 1500) - x := i * j -]) - -result := { - "skipped": false, - "skip_reason": "", - "violations": violations, -} - -violations contains msg if { - expensive > 0 - msg := "expensive computation completed" -} - -matches_evaluation if { - expensive > 0 -}