Refuse on a deny rule that never answers the question - #28
Open
beardthelion wants to merge 1 commit into
Open
Conversation
`matches` read the result of an expression as `evaluate(...) === true`, so a rule that parsed and evaluated but answered with something other than a boolean was neither a match nor an error. In the deny list that meant it did not deny, and the permissive `allow: ["true"]` that ships by default then let the action through. `deny: ["Submit order"]` is the way in. It is what somebody writes who reads the list as labels rather than expressions, and it is a valid CEL string, so it evaluates to "Submit order", falls out of the deny loop, and the Bot clicks the button. Nothing was logged, because only the throwing path logged, and the rule still sat on the Boundaries page looking as though it were in force. A bare field reference, a ternary returning a string and a bare number all land the same way. Treat any non-boolean answer as a broken rule and send it down the existing fail-closed path, which denies in the deny list, does not permit in the allow list, and logs either way. False stays a real answer: a deny list that read every false as a denial would refuse everything. Checked against every rule the product ships, the .env.example example and the four Boundaries presets, over contexts with and without an element, a key and a file. None of them changes verdict.
beardthelion
requested review from
MikeRyanDev,
davidmckayv,
guidovizoso and
tylerslaton
as code owners
August 20, 2026 02:57
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.
Closes #26.
matchesread an expression result asevaluate(...) === true, so a rule that parsed and evaluated but answered with something other than a boolean was neither a match nor an error. In the deny list that meant it did not deny, and the permissiveallow: ["true"]that ships by default let the action through.deny: ["Submit order"]is the way in: what somebody writes who reads the list as labels rather than expressions, and a valid CEL string. Nothing was logged, because only the throwing path logged, and the rule still sat on the Boundaries page looking as though it were in force.What it does
Treat any non-boolean answer as a broken rule and send it down the existing fail-closed path. It denies in the deny list, does not permit in the allow list, and logs either way, with the reason saying what came back instead.
False stays a real answer. A deny list that read every false as a denial would refuse everything.
Verification
Six cases added to
server/tests/computer-policy.test.ts. Four are the deny rules that should have denied and did not (a bare string, a bare field reference, a ternary returning a string, a number), failing before and passing after.Two are the directions that must not move, both passing before and after: a non-boolean in the
allowlist still does not permit, and a deny rule that answers false still permits, which is the check that this does not turn every policy into a refusal.Checked against every rule the product ships, the
.env.exampleexample and the four Boundaries presets, across contexts with and without an element, a key and a file. No verdict changes.Existing tests unchanged and passing. The
serversuite has the same 71 failures before and after this branch, all integration tests wanting a Postgres this machine does not have.bun run typecheckandbunx biome checkare clean.