Repository navigation
fix(policy): a rule without an explicit enabled field is off in both evaluators (#261) - #297
Merged
Merged
Conversation
…evaluators (solongate#261) `PolicyRule` carries `Enabled *bool`, and the two evaluators read an absent field oppositely: `enabledForRego` required an explicit `true`, while `enabledForFallback` treated `nil` as on. The same policy file therefore allowed or denied depending only on whether OPA was installed -- a DENY rule could block on a machine without OPA and be dead on one with it. One helper now answers the question for both: a rule with no `enabled` field is off, and turning a rule on takes an explicit `"enabled": true`. That is the reading the Rego compiler and its test already encoded (`rego_test.go`'s rule `d`), so the fix aligns the fallback with the documented behaviour instead of relaxing an assertion to match new code. Behaviour change, stated for the release notes: a DENY or ALLOW rule with no `enabled` field no longer applies on machines that fall back to the pure Go evaluator. The Rego path already ignored it. Tests: a table over the three states (absent / true / false) asserts the two evaluators agree and that the rule is live exactly when it is explicitly enabled; a second test pins the fallback alone. Both fail on the previous head with "compiled into Rego=false, blocked by the fallback=true". The four tool-pattern tests that relied on the old nil default now set `enabled` explicitly, so they keep testing the pattern rather than the default. Co-authored-by: Hermes Agent (DeepSeek V4) <noreply@nousresearch.com> Signed-off-by: Turgay Nalbant <182372382+tnalbant@users.noreply.github.com>
tnalbant
force-pushed
the
fix/261-enabled-default
branch
from
November 3, 2026 06:44
b7b98b6 to
a57be42
Compare
codeyevsky
approved these changes
Nov 3, 2026
codeyevsky
added a commit
that referenced
this pull request
Nov 3, 2026
fix(policy): a rule without an explicit enabled field is off in both evaluators (#261)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
PolicyRule.Enabledis a*bool, and the two evaluators answered "is this rule on?" differently:enabledForRegoinpackages/policy/rego.gorequired an explicittrue.enabledForFallbacktreated a nilEnabledas on.One policy file could therefore allow or deny depending only on whether OPA happened to be installed: a DENY rule written without
enabledblocked on a machine using the pure Go fallback and was dead on a machine where the rules were compiled to Rego.One helper now answers the question for both, and a missing
enabledmeans off. That is what the Rego compiler already did, and whatrego_test.gopins with its ruled, so the fallback is the side that moves — the existing assertion is left as it is.Behaviour change, so it belongs in the title: a DENY or ALLOW rule with no
enabledfield no longer applies where the fallback evaluator runs. Nothing changes on the Rego path, and a rule that sets"enabled": trueis unaffected either way.What I tested
cd packages/policy && go test -count=1 ./...— ok.TestBothEvaluatorsAgreeOnWhetherARuleIsEnabled/no_enabled_fieldreportscompiled into Rego=false, blocked by the fallback=true, andTestFallbackEvaluateSkipsARuleWithoutAnEnabledFieldsees the fallback blockrm -rf /tmp/x.toolpattern_test.gobuilt their rules withoutenabledand so relied on the old default; they set it explicitly now, and keep testing the tool pattern rather than the default.go vet ./...clean inshared,policyandguard.scripts/sg-unit-check.shstops at its gofmt leg on this machine: local Go is 1.27.3 while the modules declare 1.24–1.26, so it lists files this change never touched,packages/guardamong them. I ran each package'sgo vetandgo testdirectly instead.packages/sandboxandpackages/core/configcarry failing tests here (Darwin sandbox preview, file locks under macOS 27) that fail identically with these changes reverted, so they are not from this diff.The real OPA binary was not run here; the comparison is between the compiled Rego and the fallback evaluator, which is how the existing tests compare them too.
Written with an AI coding assistant.