Skip to content

Commit 3455295

Browse files
authored
fix(governance): reject non-finite or negative amount in cbf barrier
1 parent bdee33d commit 3455295

2 files changed

Lines changed: 158 additions & 6 deletions

File tree

src/gateway/governance/cbf.py

Lines changed: 45 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@
6060
import asyncio
6161
import json
6262
import logging
63+
import math
6364

6465
# ---------------------------------------------------------------------------
6566
# Canonical gateway-internal imports (Phase 1.1)
@@ -325,6 +326,30 @@ def get_h(self, cash_balance: float) -> float:
325326
"""Safety function h(x). Safe when h(x) >= 0."""
326327
return cash_balance - self.min_cash_balance
327328

329+
@staticmethod
330+
def _resolve_trade_cost(action_name: str, payload: dict[str, Any]) -> float:
331+
"""Return the validated cash cost for *action_name*.
332+
333+
Only ``execute_trade`` carries a cash cost; every other action is 0.
334+
A non-finite (NaN/inf) or negative ``amount`` is rejected here so it can
335+
never reach the barrier certificate or the Redis cash-state write. A
336+
negative cost makes ``next_cash = current - cost`` larger than the
337+
current balance, so the ``h_next >= (1-gamma)*h_t`` envelope check passes
338+
and the atomic commit inflates ``safety:current_cash``; a NaN cost makes
339+
every comparison false, so the barrier also passes and the balance is
340+
poisoned. This mirrors the finiteness/positive guard that
341+
``FiscalLimitGuard.reserve`` already applies to reservations.
342+
"""
343+
if action_name != "execute_trade":
344+
return 0.0
345+
cost = float(payload.get("amount", 0.0))
346+
if not math.isfinite(cost) or cost < 0:
347+
raise ValueError(
348+
f"invalid trade amount {payload.get('amount')!r} — "
349+
"must be a finite, non-negative number"
350+
)
351+
return cost
352+
328353
# ------------------------------------------------------------------
329354
# verify_action
330355
# ------------------------------------------------------------------
@@ -381,9 +406,20 @@ async def _do_verify_action(
381406
)
382407
span.set_attribute("governance.scope", _mrm_meta["scope"])
383408

384-
cost = 0.0
385-
if action_name == "execute_trade":
386-
cost = float(payload.get("amount", 0.0))
409+
try:
410+
cost = self._resolve_trade_cost(action_name, payload)
411+
except (TypeError, ValueError) as exc:
412+
_mrm_meta = ControlRegistry().get_mapping(
413+
GovernanceControl.TRADITIONAL_MRM_VALIDATION
414+
)
415+
result = (
416+
f"[{GovernanceControl.TRADITIONAL_MRM_VALIDATION.value}] "
417+
f"{_mrm_meta['primary_framework']} Violation: {exc}"
418+
)
419+
logger.warning("⛔ CBF check rejected trade: %s", exc)
420+
if span:
421+
span.set_attribute("safety.result", result)
422+
return result
387423

388424
# Reviewer note H53: local intra-window debits subtracted from snapshot to prevent double-spend within TTL window.
389425
effective_balance = current_cash - self._local_debits
@@ -631,9 +667,12 @@ async def atomic_verify_and_commit(
631667
if redis_client is None:
632668
raise RuntimeError("Redis client unavailable — cannot run atomic CBF.")
633669

634-
cost = 0.0
635-
if action_name == "execute_trade":
636-
cost = float(payload.get("amount", 0.0))
670+
try:
671+
cost = self._resolve_trade_cost(action_name, payload)
672+
except (TypeError, ValueError) as exc:
673+
reason = f"UNSAFE: {exc}"
674+
logger.warning("⛔ CBF atomic check rejected trade: %s", exc)
675+
return (False, reason)
637676

638677
keys = ["safety:current_cash", "audit:state_ledger"]
639678
argv = [
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# Copyright 2026 Google LLC
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# https://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""
16+
Regression tests: the CBF barrier must reject a non-finite or negative trade
17+
``amount`` instead of treating it as SAFE.
18+
19+
A negative ``amount`` makes ``next_cash = current - cost`` larger than the
20+
current balance, so the ``h_next >= (1-gamma)*h_t`` envelope check passes and
21+
``atomic_verify_and_commit`` writes the inflated balance back to Redis; a NaN
22+
``amount`` makes every comparison false, so the barrier also passes. Both let
23+
attacker-supplied trade parameters defeat the cash barrier. FiscalLimitGuard.reserve
24+
already applies the same finiteness/positive guard to reservations.
25+
"""
26+
27+
from __future__ import annotations
28+
29+
from unittest.mock import MagicMock
30+
31+
import pytest
32+
33+
pytest.importorskip("fakeredis", reason="fakeredis required for CBF state tests")
34+
35+
import fakeredis.aioredis # type: ignore[import]
36+
37+
from src.gateway.governance.cbf import ControlBarrierFunction
38+
39+
_INVALID_AMOUNTS = [-1_000_000.0, float("nan"), float("inf"), float("-inf")]
40+
41+
42+
@pytest.mark.local
43+
@pytest.mark.asyncio
44+
async def test_do_verify_action_rejects_invalid_amount() -> None:
45+
"""The read-only barrier returns a non-SAFE result for negative / NaN / inf
46+
amounts, and still returns SAFE for a valid positive trade."""
47+
cbf = ControlBarrierFunction()
48+
49+
for amount in _INVALID_AMOUNTS:
50+
result = await cbf._do_verify_action(
51+
"execute_trade",
52+
{"amount": amount},
53+
current_cash=100_000.0,
54+
balance_source="self_reported",
55+
span=None,
56+
)
57+
assert result != "SAFE", (
58+
f"amount={amount!r} must be rejected by the barrier; got SAFE "
59+
"(negative/NaN amount bypasses the CBF envelope check)"
60+
)
61+
62+
ok = await cbf._do_verify_action(
63+
"execute_trade",
64+
{"amount": 1_000.0},
65+
current_cash=100_000.0,
66+
balance_source="self_reported",
67+
span=None,
68+
)
69+
assert ok == "SAFE", f"a valid positive trade must still pass; got {ok!r}"
70+
71+
72+
@pytest.mark.local
73+
@pytest.mark.asyncio
74+
async def test_atomic_verify_rejects_invalid_amount_without_mutating_balance() -> None:
75+
"""The atomic check+commit rejects an invalid amount and leaves the persisted
76+
cash balance untouched — a negative amount must not inflate it."""
77+
fake_redis = fakeredis.aioredis.FakeRedis(decode_responses=False)
78+
await fake_redis.set("safety:current_cash", "100000.0")
79+
80+
cbf = ControlBarrierFunction()
81+
cbf.tracer = None
82+
83+
mock_redis_module = MagicMock()
84+
mock_redis_module.get_raw_client.return_value = fake_redis
85+
86+
with pytest.MonkeyPatch().context() as mp:
87+
mp.setattr("src.gateway.governance.cbf.redis_client", mock_redis_module)
88+
89+
for amount in _INVALID_AMOUNTS:
90+
committed, reason = await cbf.atomic_verify_and_commit(
91+
action_name="execute_trade",
92+
payload={"amount": amount},
93+
)
94+
assert committed is False, (
95+
f"amount={amount!r} must not commit; got committed=True"
96+
)
97+
assert "UNSAFE" in reason, f"reason must flag UNSAFE; got {reason!r}"
98+
balance = (await fake_redis.get("safety:current_cash")).decode()
99+
assert float(balance) == 100_000.0, (
100+
f"balance must stay 100000.0 after rejecting amount={amount!r}; "
101+
f"got {balance} (negative amount inflated the persisted balance)"
102+
)
103+
104+
# A valid trade still commits and debits the balance.
105+
committed, reason = await cbf.atomic_verify_and_commit(
106+
action_name="execute_trade",
107+
payload={"amount": 1_000.0},
108+
)
109+
assert committed is True, f"valid trade must commit; got reason={reason!r}"
110+
balance = (await fake_redis.get("safety:current_cash")).decode()
111+
assert float(balance) == 99_000.0, (
112+
f"balance must debit to 99000.0; got {balance}"
113+
)

0 commit comments

Comments
 (0)