Skip to content

Refuse on a deny rule that never answers the question - #28

Open
beardthelion wants to merge 1 commit into
CopilotKit:mainfrom
beardthelion:fix/deny-rule-non-boolean
Open

Refuse on a deny rule that never answers the question#28
beardthelion wants to merge 1 commit into
CopilotKit:mainfrom
beardthelion:fix/deny-rule-non-boolean

Conversation

@beardthelion

Copy link
Copy Markdown

Closes #26.

matches read an expression result 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 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 allow list 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.example example 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 server suite has the same 71 failures before and after this branch, all integration tests wanting a Postgres this machine does not have. bun run typecheck and bunx biome check are clean.

`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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A deny rule that answers with a non-boolean silently does not deny

1 participant