Skip to content

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

Description

@beardthelion

matches in server/src/computer/policy.ts reads an expression result as evaluate(...) === true. An expression that parses and evaluates but answers with something other than a boolean is therefore neither a match nor an error: it falls out of the deny loop, and the allow: ["true"] that ships as the default then permits the action.

deny: ["Submit order"] is the way in. It is what somebody writes who reads the deny list as a list of labels rather than expressions, and it is a valid CEL string literal.

Verified against main at 93ff1b1 by calling evaluateActionPolicy with allow: ["true"], on a click of a button named "Submit order":

deny rule result
contains(element.name, "submit") refused, source deny
contains(element.name, "submit") && nosuchfield (throws) refused, source deny
"Submit order" allowed, source allow
element.name allowed, source allow
contains(element.name, "submit") ? element.name : false allowed, source allow
repeat.count allowed, source allow

Three things make this worse than an ordinary typo:

The whole matrix above emitted two log lines, both from the throwing rule. The computer-policy-expression-error log lives in the catch, so this path is silent, though the comment on matches says a broken expression is logged loudly.

Nothing rejects it on the way in. parseActionPolicy returns ok: true for deny: ['"Submit order"'], and its docstring justifies not validating on the grounds that the engine fails closed, which holds for throwing expressions only. The Boundaries page has no check of its own, and the API answers 200 with the policy echoed back, so an operator sees the rule saved and listed as in force.

The direction is asymmetric. The same expressions in allow correctly fail closed and refuse. Only the deny list weakens, and that is the list where failing open grants an action.

Fix is in matches: treat any non-boolean answer as a broken rule and give it the same onError path as a throw, which also makes the existing log line fire. False stays a real answer, since a deny list that read every false as a denial would refuse everything. PR to follow.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions