Describe the bug
There is a critical architectural vulnerability in the core engine's execution loop (internal/engine/executor.go).
When an entity (like a repository or pull request) is received and processed, the EvalEntityEvent function correctly iterates over all assigned profiles and evaluates their rules. However, if e.evaluateRule() returns an initialization or configuration error, the engine immediately aborts the entire evaluation loop:
for _, rule := range profile.Rules {
if err := e.evaluateRule(ctx, inf, provider,
&profile, &rule, ruleEngineCache, profileEvalStatus);
err != nil {
return fmt.Errorf("error evaluating entity event: %w", err)
// <--- SILENT EVAL ABORT
}
}
The underlying issue is that evaluateRule will return a hard error if GetRuleEngine fails (e.g., if a custom rule type has malformed Rego syntax) or if the ActionConfig fails initialization.Because the engine throws the error back to the event handler and drops the entity locker:
- Denial of Service & Security Bypass: If a user defines one single malformed rule inside one single profile, evaluating that rule will throw an error and abort the loop. Because the loop terminates, every other rule and profile running subsequently on that entity is silently skipped. This means critical security checks (like automated branch protections) can be bypassed entirely due to a minor configuration error in an unrelated profile.
- Missing Observability: Because it returns before
createOrUpdateEvalStatus is called, the database never records that the rule threw an evaluation error. The entity essentially ghost-skips its checks, leaving the system blind to the failure.
(Note: This is conceptually identical to the profile execution loop bug fixed in #6330, but located at the absolute heart of the security evaluation pipeline)
Expected behavior
If evaluateRule() encounters a configuration or initialization failure for a specific rule, it should:
- Construct an
EvalResult recording the internal system's error state.
- Write that "Error" status to the database via
createOrUpdateEvalStatus so administrators know the rule config is broken.
- Catch the initialization error and continue executing the rest of the rules and profiles so subsequent security checks are not bypassed.
Proposed Fix
In executor.go inside evaluateRule: When createEvalStatusParams, GetRuleEngine, or NewRuleActions fail, we should not return the error directly. Instead, we should populate evalParams.SetEvalErr(err) and call e.createOrUpdateEvalStatus(ctx, evalParams), log the issue, and finally return nil to gracefully allow the upstream profile loop to continue executing the subsequent policies.
Describe the bug
There is a critical architectural vulnerability in the core engine's execution loop (internal/engine/executor.go).
When an entity (like a repository or pull request) is received and processed, the EvalEntityEvent function correctly iterates over all assigned profiles and evaluates their rules. However, if
e.evaluateRule()returns an initialization or configuration error, the engine immediately aborts the entire evaluation loop:The underlying issue is that
evaluateRulewill return a hard error ifGetRuleEnginefails (e.g., if a custom rule type has malformed Rego syntax) or if theActionConfigfails initialization.Because the engine throws the error back to the event handler and drops the entity locker:createOrUpdateEvalStatusis called, the database never records that the rule threw an evaluation error. The entity essentially ghost-skips its checks, leaving the system blind to the failure.(Note: This is conceptually identical to the profile execution loop bug fixed in #6330, but located at the absolute heart of the security evaluation pipeline)
Expected behavior
If
evaluateRule()encounters a configuration or initialization failure for a specific rule, it should:EvalResultrecording the internal system's error state.createOrUpdateEvalStatusso administrators know the rule config is broken.Proposed Fix
In
executor.goinsideevaluateRule: WhencreateEvalStatusParams,GetRuleEngine, orNewRuleActionsfail, we should not return the error directly. Instead, we should populateevalParams.SetEvalErr(err)and calle.createOrUpdateEvalStatus(ctx, evalParams), log the issue, and finally return nil to gracefully allow the upstream profile loop to continue executing the subsequent policies.