[Michael McQuade (giraffesyo)]

Summary

A condition fails when it compares a double that isn't finite (NaN or an infinity), which dividing by zero or double("NaN") makes. Before that rule, a deny's failed / total >= 0.5 lifted where both were 0.0. The check reached the operands and the values a list or map holds, but these paths still let such a double decide, so a deny written that way lifted and a grant held.

• A map keyed by a double that isn't finite decides comparisons.
  • The problem: CEL finds no key equal to NaN. So {failed / total: 1} == {0.5: 1} was false where it should fail, != was true, 0.5 in {double(score): 1} was false, and a loop's {r: 1} == {0.5: 1} over [failed / total] was false too. A map the host passes in an undeclared kind can be keyed by NaN too: request.attributes.m == {0.5: 1} was false. Indexing a host map by an infinite key a condition reads, m[request.attributes.y], found it.
  • The fix:
    • A key a condition computes fails when it is a double that isn't finite (finiteKey). The decorators that already charge every computed key, as a call (keyedCall) or an attribute (keyedAttribute), refuse one. That covers map literals, lookups and loop variables.
    • An index by an attribute (qualifier) refuses one. CEL indexes by a double only for float32, float64 and types.Double, so only those are checked, which keeps a JSON attribute from being parsed a second time.
    • Comparing maps of as many entries fails where either holds such a key (nonFiniteKey). The walk runs after the size check, within what the comparison is charged. Maps of different sizes still compare unequal, as CEL says.
    • in a map the host passes keyed by one stays false for a key it doesn't hold. Finding whether it holds one would read every key, which in isn't charged for. Condition's and Attributes' docs say so.
• Converting a double that isn't finite to a string decides.
  • The problem: string(failed / total) is "NaN", so a deny's string(failed / total) == "0.5" lifted. int() and uint() of one already fail.
  • The fix: a stringing decorator runs string() through CEL's own bindings for the overload the checker resolved, or by name where it resolved several, the same lookup CEL's planner does, after refusing a double that isn't finite. Every other value converts exactly as CEL converts it, including a value CEL checks as another type than it holds. The bindings are read only when a condition calls string(), and plan refuses a conversion it finds no binding for. That can't happen with this version of CEL, and a stored condition refused so fails closed.
• An optional the host passes holding a double that isn't finite compares unequal, and comparing one is charged as one value.
  • The problem: CEL compares optionals by its own equality, so an optional holding NaN compared unequal rather than failing. valueSize charged any optional as 1, so comparing one holding a long list walked it all for that charge.
  • The fix: equal compares what two optionals hold, and valueSize counts it, so the charge only grows.
• A key folded into a double that isn't finite would be read unchecked.
  • The problem: this version of CEL folds no constants, but another could fold 0.0 / 0.0 into a constant key that finiteKey never sees.
  • The fix: planning refuses a constant key that isn't finite, as it refuses a key planned where the package can't charge it.

Testing

• TestADoubleAConditionMakesThatIsntFiniteFailsWhatComparesIt has five more grant and deny pairs: map literals keyed by failed / total compared with ==, with !=, inside a list, from the other side, and searched with in; a map keyed by double(score) searched with in; the same comparisons inside all and exists loops; and string() of the ratio. Each runs with NaN, +Inf and −Inf. Without the fixes the new cases fail: grants hold and denies lift. With the call path fixed but not the attribute path, the loop cases still fail.
• TestAMapTheHostPassesKeyedByADoubleThatIsntFiniteFailsWhatComparesOrIndexesIt: host maps keyed by NaN and ±Inf, compared from either side and indexed by an infinite key. Each case fails without its check.
• TestAnOptionalTheHostPassesComparesAsWhatItHolds: optionals holding NaN, nested, and holding a long list. These fail without the equality change, and without the size change.
• TestFiniteValuesDecideAsCELDoes: a differential test against plain CEL. It covers string() of every type, plain, through dyn() and checked as each other type, optionals, and maps with mixed int, uint and double keys. It fails with a conversion chosen by Go type.
• TestAKeyFoldedIntoADoubleThatIsntFiniteIsNoCondition and TestAConversionToAStringBoundOtherwiseIsNoCondition: each refusal fails its test without it.
• Cost: 30,000 random trials mixing nested optionals, lists, maps with int, uint, double, NaN, ±Inf and string keys, and string() each cost no more than their bound. The limit and actual cost of 110 string() conditions are identical to canary's.
• Three reviews, each at xhigh.
• make check: runs before it lands.

Checklist

• make check passes
• No exported name, stored value or table was renamed (they are contract once released)