Refuse on a deny rule that never answers the question

server/src/computer/policy.ts

@@ -206,20 +206,38 @@ const POLICY_FUNCTIONS: Record<string, (...args: never[]) => unknown> = {
  * `onError` decides what a broken expression means, because the safe answer differs by list: a broken
  * `allow` must not permit, and a broken `deny` must not stop denying. Both are logged loudly, because
  * a policy that silently misbehaves is worse than one that visibly refuses.
+ *
+ * A rule can be broken two ways and only one of them throws. `"Submit order"` is valid CEL: it parses,
+ * it evaluates, and it answers with a string, which is not an answer to "does this rule apply". That
+ * is what somebody writes who reads the deny list as a list of labels rather than expressions, and
+ * reading it as "no match" would let the action through under the permissive allow rule that ships by
+ * default, with nothing logged and the rule still listed on the Boundaries page as though it were in
+ * force. So anything other than a boolean is a broken rule, and takes the same fail-closed path as a
+ * throw. False is a real answer and stays one; a deny list that read every false as a denial would
+ * refuse everything.
  */
 function matches(
   expression: string,
   context: PolicyContext,
   onError: boolean,
 ): boolean {
   try {
-    return (
-      evaluate(
+    const result = evaluate(
+      expression,
+      context as unknown as Record<string, unknown>,
+      POLICY_FUNCTIONS as Record<string, CallableFunction>,
+    );
+    if (typeof result === "boolean") return result;
+
+    console.error(
+      JSON.stringify({
+        type: "computer-policy-expression-error",
         expression,
-        context as unknown as Record<string, unknown>,
-        POLICY_FUNCTIONS as Record<string, CallableFunction>,
-      ) === true
+        error: `expected a true or false answer, got ${result === null ? "null" : typeof result}`,
+        treatedAs: onError,
+      }),
     );
+    return onError;
   } catch (error) {
     console.error(
       JSON.stringify({

server/tests/computer-policy.test.ts

@@ -91,6 +91,49 @@ describe("evaluateActionPolicy", () => {
     expect(decision.source).toBe("deny");
   });
 
+  // The other way a rule is broken. These parse and evaluate, so nothing throws; they simply do not
+  // answer the question that was asked, and the only safe reading of a deny rule that did not answer
+  // is that it denied. `"Submit order"` is what somebody writes who thinks the list takes labels
+  // rather than expressions, and it is a valid CEL string.
+  test.each([
+    ['"Submit order"', "a bare string, i.e. the list read as labels"],
+    ["element.name", "a bare field reference"],
+    ['contains(element.name, "submit") ? element.name : false', "a ternary"],
+    ["repeat.count", "a number"],
+  ])(
+    "a deny expression that is not a question (%s: %s) still denies",
+    (rule) => {
+      const decision = evaluateActionPolicy(
+        { ...permissive, deny: [rule] },
+        context(),
+      );
+      expect(decision.allowed).toBe(false);
+      expect(decision.source).toBe("deny");
+    },
+  );
+
+  // The mirror. A rule that does not answer must not permit either, which is what this already did by
+  // reading anything other than true as no match.
+  test("an allow expression that is not a question does not permit", () => {
+    const decision = evaluateActionPolicy(
+      { mode: "enforce", deny: [], allow: ['"Submit order"'] },
+      context(),
+    );
+    expect(decision.allowed).toBe(false);
+    expect(decision.source).toBe("default");
+  });
+
+  // A rule that answers "no" is not broken, and must not be read as one: a deny list where every
+  // false reading became a denial would refuse everything.
+  test("a deny expression that answers false permits", () => {
+    const decision = evaluateActionPolicy(
+      { ...permissive, deny: ['contains(element.name, "cancel")'] },
+      context(),
+    );
+    expect(decision.allowed).toBe(true);
+    expect(decision.source).toBe("allow");
+  });
+
   test("a broken allow expression does not permit", () => {
     const decision = evaluateActionPolicy(
       { mode: "enforce", deny: [], allow: ["also not ( valid"] },