Repository navigation
Commit 0092d11
authored
chore(release): 0.14.0 — hardening pass on the money contract (#76)
* fix(sdk): /execute handles require_approval + re-checks with approval_id
Until now runtime.execute() only handled decision=block. A backend that
returned require_approval was treated as 'allow' and the @sensitive
body ran, silently bypassing human approval. Phase 0 closes the gap:
1. On require_approval, runtime.execute parks on the existing
_wait_for_approval_resolution() (WS push + threaded.Event). Denied
or timed-out -> NullRunBlockedException. Approved -> re-checks.
2. Re-check forwards approval_id on the second /execute request.
The same operation_id is reused so the backend can bind both
requests to one logical action.
3. If the re-check still returns require_approval, the body is
blocked (defensive: the backend should not return require_approval
to a re-check, but a misbehaving server must not cause a silent
allow).
4. Server-authoritative approval_timeout_seconds from the first
response drives the event.wait() timeout (Разрыв 1c sync).
Transport.execute gains an optional approval_id kwarg; when present it
is forwarded on the wire as a top-level field, distinct from
operation_id. Legacy callers (no approval_id) get the previous
behaviour.
Tests (tests/test_execute_approval_flow.py):
- approved flow: re-check with matching tool/input/operation_id, then allow.
- denied flow: no re-check, NullRunBlockedException.
- require_approval without approval_id: fail-CLOSED.
Verification:
- 17/17 execute_approval_flow + approval_timeout_field + runtime.execute
+ sensitive-tool fail-closed tests pass.
- pytest -q full SDK suite: 1246 passed, 7 skipped.
* fix(sdk): clamp server approval_timeout to [1, 3600]s (Phase 0 review)
Phase 0 review (2026-07-23): the existing approval_timeout
validation only rejected non-positive values, so a misconfigured
backend (or a malicious proxy) advertising 0 (deadlock) or
1e9 (lock the thread for years) would be passed straight to
event.wait().
This commit:
1. Adds MIN_APPROVAL_TIMEOUT_SECONDS=1 and
MAX_APPROVAL_TIMEOUT_SECONDS=3600 module constants.
2. Extracts the validation into a _validate_approval_timeout()
helper that coerces to float, returns None on non-numeric,
and returns None on out-of-range with a WARN log.
3. Wires the helper into both call sites:
- check_workflow_budget (existing /gate path)
- runtime.execute (the new Phase 0 /execute path)
4. The pre-existing inline parse+reject logic in both sites is
replaced by the helper (no copy-paste).
5. Updates the docstring on _wait_for_approval_resolution
that mentioned a 'fall back to legacy /status poll path' on
timeout — the Разрыв 1c contract is fail-CLOSED on
timeout, and the legacy fallback was a stale comment that
would mislead future maintainers.
6. test_timeout_sentinel_returned_when_no_ws_push now uses a
1.5s timeout (the new minimum in-range) and asserts the
elapsed wait is between 1.0s and 5.0s — the old 0.1s
value would now fall back to the env default 300s.
Tests:
- 5 new test_validate_approval_timeout_* tests pin every branch:
in-range (incl. int coercion), below MIN, above MAX, non-numeric
(str, list, dict), and None.
- Pre-existing 8 test_approval_timeout_field tests still pass
(1s-300s range covered by 15s, 42s, 90s, 120s, 300s inputs).
- 17/17 test_execute_approval_flow + TestNullRunRuntimeExecute
+ TestEnforceSensitiveToolFailClosed still pass.
- 22 passed, 1 skipped in the approval timeout + execute flow
slice.
* feat(sdk): BusinessImpact + MoneyImpactExtractor + 5 DoD scenarios
Phase 1 / MVP 1.0 close on the SDK side. Three new modules
mirror the backend BusinessImpact discriminated union and wire
contract so the SDK can produce /gate + /execute requests that
pass the backend digest re-check byte-for-byte.
What landed
1. nullrun.business_impact -- Python mirror of the Rust
BusinessImpact enum + compute_action_digest() helper.
- Same canonical-JSON algorithm: sort object keys recursively,
then SHA-256 over the protocol prefix || canonical bytes.
- MoneyImpact dataclass with explicit validate() (negative
amount rejected, currency must be 3 ASCII uppercase chars,
direction must be outflow|inflow). Same checks as the
backend MoneyImpact::validate.
- The MVP supports only kind=money. The enum shape is
forward-compat: future record_count, resource_quantity,
permission_change, etc. land as new dataclass branches
without changing the wire discriminator.
2. nullrun.extractor -- declarative SDK extractor with a tiny
shorthand factory.
- money_outflow(argument="amount_cents") returns a
MoneyImpactExtractor that binds the call's arguments via
inspect.signature(...).bind(*args, **kwargs) and pulls the
named argument out, treating positional and keyword
invocations identically.
- Fails fast on missing argument / wrong type / bool.
- impact_for() returns a fully-validated BusinessImpact;
the caller is expected to send it on /gate and re-send the
same on /execute (the SDK will compute the digest
automatically).
3. tests/test_approval_money_flow.py -- the 5 DoD scenarios
requested on 2026-07-23:
1. Refund $40 -> Allow
2. Refund $1200 -> Require Approval -> Approve -> Execute
3. Refund $1200 -> Approve -> Modify amount to $1300
-> block on digest mismatch (Phase 1 headline security)
4. Approved -> Execute -> Second Execute -> block on replay
5. Approved -> wait expiry -> Execute -> block on expiry
Plus 13 supporting tests: digest deterministic / 1-cent change
flips digest / EUR vs USD / MoneyImpact validation / extractor
positional vs keyword vs mixed args / extractor rejects bad
types.
The ApprovalSimulator class is an in-process mirror of
gate_internal grant-consume + Phase 1 digest re-check. Verified
against db.rs::consume_approved SQL + the new Rust digest-block
code path.
Tests verified
- pytest -q tests/test_approval_money_flow.py -> 18/18 passed.
- pytest -q tests/test_execute_approval_flow.py -> 3/3 passed.
- pytest -q tests/test_approval_timeout_field.py -> 13/13 passed.
What this commit does NOT close
- Wiring into @sensitive: extractor.impact_for is a helper
callable from runtime.execute but not yet auto-wired. The
DoD tests call it manually via _refund_call(). A future PR
flips this to automatic via a function attribute set by
@sensitive.
- Backend HTTP integration test wiring axum::Router + mock
ApprovalRepository. The simulator mirrors the decision
logic byte-for-byte.
- Frontend regen of MoneyImpact in api.ts (SDK side does not
touch the frontend; tracked separately).
* feat(sdk): wire business_impact + action_digest on @sensitive(impact=...)
Phase 1 / MVP 1.0 closes the wire side of the action-bound
approval flow. The previous SDK commit (ccdf857) shipped the
MoneyImpactExtractor helper but did NOT wire it through
``runtime.execute()``; this commit does.
What changed
- ``runtime.execute()`` gains two new kwargs:
``business_impact: dict | None`` and
``action_digest: str | None``. When supplied, they ride the
payload to /execute. When absent, the field is omitted from
the wire and the backend falls back to the legacy Phase 0
approval_id-only grant consume path. Both fields default
to None so existing callers compile unchanged.
- ``_enforce_sensitive_tool`` reads the wrapped function's
``_nullrun_extractor`` attribute (set by the @sensitive
decorator's ``impact=...`` form), runs the extractor on the
live args, computes the action_digest from the resulting
BusinessImpact, and threads both onto the runtime.execute
call. If the extractor raises (bad arg name, wrong type,
negative amount, ...), the body MUST NOT run per ADR-008:
the error is wrapped as ``NullRunBlockedException`` with
error_code NR-B003 and the wrapper raises before the
sensitive tool executes.
- ``@sensitive`` becomes parameterised:
``@sensitive`` (bare form, unchanged) and
``@sensitive(impact=money_outflow(argument="..."))``
(factory form, new). Both register the tool as sensitive so
the pre-check fires; only the factory form attaches the
extractor. Implementation splits into ``sensitive(fn, *,
impact)`` for the public entry point and
``_do_sensitive_register(fn)`` for the shared registration
body.
Tests verified
- pytest -q tests/test_sensitive_extractor.py -> 5/5 passed.
The new file monkeypatches ``runtime._transport.execute`` on
a freshly-built NullRunRuntime singleton (registered via
``RuntimeRegistry.set``), invokes
``_enforce_sensitive_tool`` directly, and asserts the
payload reaching the wire contains:
- ``business_impact`` with the documented MoneyImpact
shape (kind, direction, amount_minor, currency,
extractor_id, extractor_version).
- ``action_digest`` matching the SDK's own
``compute_action_digest`` byte-for-byte (the cross-
language contract pin from 4445f95).
The legacy Phase 0 path (no ``_nullrun_extractor``) sends
neither field. Extractor errors (bad arg, negative amount)
raise ``NullRunBlockedException(NR-B003)``.
- pytest -q tests/test_approval_money_flow.py
tests/test_execute_approval_flow.py
tests/test_approval_timeout_field.py -> 32/32 passed
(regression check on Phase 0 SDK paths).
The wire shape produced here matches the manual-call pattern
in ``tests/test_approval_money_flow.py::TestDoDScenarios``
which has been green since ccdf857; both pin the cross-
language contract on the same hex literal.
* test(sdk): add dedicated BusinessImpact test module
Adds ``tests/test_business_impact.py`` -- the Python counterpart
of the backend's ``business_impact::tests`` module. The two
must stay in lockstep: any drift in canonicalisation, hex
shape, or validator behaviour breaks one or both of these
test suites before reaching a customer runtime.
What the new file covers
- ``TestComputeActionDigestPins`` -- 5 tests pinning the
canonical SHA-256 hex for ``BusinessImpact.money(OUTFLOW,
5_000, "USD")`` to the same golden literal
(``dfc96387ca539b7130caebe705e042f2e34e52ab44352ae5e527bcef64f0df27``)
that the Rust golden test asserts. Two-call determinism,
amount-change / currency-change / direction-change digests
differ.
- ``TestBusinessImpactWireDict`` -- 6 tests pinning the JSON
shape ``to_wire_dict()`` produces. Round-trips through
``json.dumps(sort_keys=True)`` so a non-deterministic dict
ordering in the canonical encoder would surface as a
digest mismatch.
- ``TestExtractorArgumentLookup`` -- 5 tests pinning
``inspect.signature(...).bind(...)`` resolves the declared
argument positionally, by keyword, and mixed. Sanity-check
on ``bound.apply_defaults()`` which is what the extractor
relies on. Explicit ``TypeError`` on missing argument (the
@Protect wrapper converts this into a
NullRunBlockedException).
- ``TestExtractorFailureModes`` -- 3 tests pinning the
fail-CLOSED behaviour: negative amount -> ValueError,
non-int -> TypeError, ``True`` -> TypeError. The ``True``
pin is the most important: ``True == 1`` would silently
round-trip through ``inspect.signature`` and reach the
canonical encoder as ``amount_minor=true``; the validator
must reject this so a hostile SDK caller can't smuggle a
tiny refund through ``amount_minor=True``.
Tests verified
- ``pytest -q tests/test_business_impact.py`` -> 19/19 passed
- ``pytest -q tests/test_approval_money_flow.py
tests/test_sensitive_extractor.py`` -> 23/23 passed
(no regression on the existing approval-flow or
sensitive-extractor tests).
The golden hex pin is the same literal the Rust test
``action_digest_golden_usd_outflow_5000_cents`` asserts. A
change to either algorithm trips a test on both sides before
a customer sees the regression.
* feat(sdk): explicit units discriminator + Decimal support
Phase 1.1 UX follow-up addressing the silent truncation bug
in the Phase 0 extractor (``int(50.99) == 50`` silently
dropped 99 cents) and the operator-friction where the unit
semantics were implicit from the value type.
The new contract
- ``money_outflow(argument="amount", units="major")``:
``Decimal`` argument. ``Decimal * 100`` with banker's
rounding (``ROUND_HALF_EVEN``) to integer minor units.
``int`` and ``bool`` are rejected outright because a
bare ``int`` in major units is the silent bug class the
explicit discriminator is designed to prevent.
- ``money_outflow(argument="amount_cents", units="minor")``
(the default for backward compatibility): ``int`` is the
canonical type. ``Decimal`` is accepted only if it is
already integer-valued; a fractional ``Decimal`` is a
unit-confusion bug and surfaces a ``TypeError`` pointing
the operator at the right alternative. ``float`` is
rejected outright at both paths.
The wire shape is unchanged: the SDK converts to integer
minor units before reaching the ``BusinessImpact`` struct,
so the backend still sees ``amount_minor`` in cents and the
cross-language golden hex pin
(``dfc96387ca539b7130caebe705e042f2e34e52ab44352ae5e527bcef64f0df27``)
matches whether the operator passed ``int(5000)``,
``Decimal("50")``, or ``Decimal("50.00")``.
Why the discriminator is explicit (not type-implicit)
A signature refactor (``int`` -> ``Decimal`` or vice versa)
does NOT silently flip the unit semantics. The operator
must pass ``units="major"`` to opt into Decimal conversion,
and that opt-in survives the refactor. This addresses the
review note: an ``int = minor, Decimal = major`` shortcut
is rejected because it makes the unit semantics implicit
and brittle.
Files changed
- ``nullrun-sdk-python/src/nullrun/extractor.py`` -- the
``_to_minor_units`` helper is the new conversion
primitive; ``MoneyImpactExtractor.__init__`` accepts
``units: str = UNIT_MINOR``; ``impact_for`` delegates to
the helper. ``bool`` is rejected explicitly (it is a
subclass of ``int`` in Python and would otherwise slip
through).
- ``nullrun-sdk-python/tests/test_units_discriminator.py`` --
29 unit tests covering the unit-discriminator matrix:
7 for ``units="major"`` (Decimal success + int/float/
bool rejection), 7 for ``units="minor"`` (int success +
Decimal-int acceptance + fractional-Decimal/float/str/bool
rejection), 2 for the cross-language golden hex pin
survival, 3 for the discriminator being explicit, 9 for
``_to_minor_units`` directly, 1 for the direction
independence.
- ``nullrun-sdk-python/tests/test_approval_money_flow.py``
-- the two existing tests ``test_extractor_rejects_wrong_type``
and ``test_extractor_rejects_bool_amount`` updated to match
the new error message ("requires int or Decimal"). The
tests still pass; the messages now name the unit
discriminator so the operator can fix the call site
without guessing.
Backend: no changes. The wire shape is in minor units
regardless of the SDK's units discriminator, so the
backend's ``action_predicate`` evaluator (which compares
``amount_minor`` values) is unaffected.
Tests verified
- ``pytest tests/test_units_discriminator.py`` -> 29/29 pass
- ``pytest tests/test_approval_money_flow.py
tests/test_sensitive_extractor.py
tests/test_business_impact.py`` -> 42/42 pass
(regression -- the existing tests were updated to match
the new error message; no behavioural change for
callers who used the legacy ``int`` path).
* feat(sdk+ui): reject sub-precision Decimals instead of silently rounding
Production-grade money contract. The previous commit (3a3ae6b)
introduced ``Decimal`` support with ``ROUND_HALF_EVEN`` (banker's
rounding) for ``units="major"``. Review rejected that on the
grounds that ``Decimal("50.005")`` for USD silently drops the
half-cent (``5000`` minor units) and surprises the operator.
Payment systems either truncate explicitly or refuse ambiguous
precision; banker's rounding at the input boundary is a
third-class behaviour that hides bugs.
This commit replaces banker's rounding with strict precision
validation against the ISO-4217 minor-unit exponent for the
currency.
Contract
- ``currency_minor_digits(currency)`` returns the ISO-4217
minor-unit exponent for the currency: ``2`` for USD/EUR/
GBP/CHF/CAD/AUD, ``0`` for JPY, ``3`` for KWD/BHD/OMR.
Unknown currencies fall back to ``2`` (a future addition
is one line in ``_CURRENCY_MINOR_DIGITS``).
- ``_decimal_has_more_fractional_digits(value, allowed)``
returns ``True`` iff the value has a non-zero fractional
part whose precision exceeds ``allowed``. The check uses
``value % 1`` so that ``Decimal("50.00")`` (which
``as_tuple()`` reports as having two fractional digits)
is correctly classified as an integer-valued decimal with
zero effective fractional digits.
- ``_to_minor_units`` for ``units="major"`` rejects any
``Decimal`` whose precision exceeds the currency's
minor-unit exponent with
``ValueError("{currency} supports at most {N} fractional digit(s); got {value} ({M}).")``.
The error message names the currency and the offending
precision, so the operator sees exactly what to fix.
Edge cases
- ``Decimal("50")`` (USD) -> ``5000`` minor OK (no fractional).
- ``Decimal("50.99")`` (USD) -> ``5099`` minor OK (2 digits).
- ``Decimal("50.00")`` (USD) -> ``5000`` minor OK
(zero effective fractional).
- ``Decimal("50.005")`` (USD) -> ``ValueError`` (3 digits, USD supports 2).
- ``Decimal("50.999")`` (USD) -> ``ValueError``.
- ``Decimal("0.005")`` (USD) -> ``ValueError``.
- ``Decimal("100.5")`` (JPY) -> ``ValueError`` (JPY supports 0).
- ``Decimal("1000")`` (JPY) -> ``1000`` minor OK.
- ``Decimal("1.234")`` (KWD) -> ``1234`` minor OK (KWD supports 3).
- ``Decimal("1.2345")`` (KWD) -> ``ValueError``.
The wire format is unchanged: ``amount_minor`` is still an
integer cents/fils/yen on the wire. The conversion is exact
because precision was validated before the multiplier ran,
so there is no rounding at any step.
UI
The rule editor (frontend/app/(platform)/control-center/policies/approval-rules/page.tsx)
extends ``CURRENCIES`` from a flat string array to a table
of ``{ code, digits }`` pairs and uses ``CURRENCY_BY_CODE``
to look up the allowed fractional digits per currency.
The regex in ``predicateToJson`` and the error message in
``validatePredicateForm`` both parameterise on the currency
so ``50.005`` for USD raises an inline error pointing at the
currency's minor-unit exponent, and ``100.5`` for JPY raises
an inline error saying "JPY supports at most 0 fractional
digit(s)". The hint text on the form is updated to reflect
the exact conversion (no rounding) and the new rejection
behaviour.
Backend
No changes. The backend ``action_predicate`` evaluator
already compares ``amount_minor`` (integer cents / fils / yen)
which is currency-agnostic at the wire level. The validator
in ``approval_rule_service.rs`` checks the JSON shape but
not the precision (the precision contract lives in the
SDK + form, the security boundary at the backend is the
SQL ``consume_approved`` atomic path).
Tests verified
- ``pytest tests/test_units_discriminator.py`` -> 36/36 pass
(was 29/29 before; replaced 2 banker's-rounding tests
with 9 precision-validation tests covering USD sub-cent,
JPY sub-yen, KWD sub-fil, integer-valued-with-trailing-zeros).
- ``pytest tests/test_business_impact.py
tests/test_approval_money_flow.py
tests/test_sensitive_extractor.py`` -> 44/44 pass
(regression; the legacy ``int`` path is unchanged).
- ``npm run type-check`` -> exit 0
- ``cargo test --lib proxy::service::approval_rule_service::tests::``
-> 6/6 pass (backend regression; no backend changes here).
Backward compat
``Decimal("50.00")`` and ``Decimal("50")`` still produce the
same ``amount_minor=5000`` as before; only values with
non-zero fractional parts exceeding the currency's minor-unit
exponent are now rejected. Existing call sites that passed
``Decimal("50.99")`` (the only precision the SDK could
silently round) continue to work; call sites that passed
``Decimal("50.005")`` were silently buggy and now surface
as ``ValueError`` instead of a subtle drift in the wire
shape.
* feat(sdk): hardening pass on the money contract
Closes the four review gaps from the Phase 1.1 / UX follow-up:
1. **Dedicated error types** -- ``InvalidMoneyPrecisionError``
and ``InvalidMoneyAmountError`` (both subclass ``ValueError``
for backward compat). The ``amount`` variant carries a
``reason`` discriminator (``"negative"`` / ``"overflow"`` /
``"non_finite"``) so a UI or test harness can branch on
type without parsing the message. The ``precision`` variant
carries ``currency`` / ``allowed`` / ``received`` /
``received_digits`` so the error message names the
offending currency and precision.
2. **Negative amount rejection** -- a negative ``amount_minor``
would silently fall through every ``op=gt`` predicate
(``negative < positive`` is always False), so the SDK
rejects ``Decimal("-50.00")`` / ``int(-5000)`` /
``Decimal("-5000")`` on both unit paths with
``InvalidMoneyAmountError(reason="negative", ...)``. ``0``
is accepted (legitimate $0.00 refund).
3. **Overflow guard** -- the converted ``amount_minor`` is
checked against ``2**63 - 1`` (the wire format is
``i64``). Values exceeding the limit raise
``InvalidMoneyAmountError(reason="overflow", ...)`` with
a message that names ``i64::MAX`` so the operator knows it
is a wire-format limit, not a currency arithmetic limit.
``Decimal("1e30")`` for USD is rejected (or
``OverflowError`` if ``int(...)`` raises before the
explicit check).
4. **Serialization stability** -- ``Decimal("50")``,
``Decimal("50.0")``, ``Decimal("50.00")``, ``Decimal("50.000")``
and ``Decimal("50.0000")`` all reduce to ``int(50)`` and
produce the same SHA-256 digest. The cross-language
golden hex pin (``dfc96387ca539b7130caebe705e042f2e34e52ab44352ae5e527bcef64f0df27``)
matches for every trailing-zero variant.
5. **Unsupported currency fallback** -- unknown ISO-4217 codes
fall back to 2 fractional digits (USD-style validation).
The fallback is conservative: a value that would be
valid in 3-digit KWD is rejected in an unknown code
because the fallback assumes 2 digits. The operator adds
the new code to ``_CURRENCY_MINOR_DIGITS`` to opt in.
Files changed
- ``nullrun-sdk-python/src/nullrun/extractor.py`` -- added
``InvalidMoneyPrecisionError`` and
``InvalidMoneyAmountError`` classes, the
``_check_overflow`` helper, sign validation in
``_to_minor_units``, normalised Decimal-to-int conversion
in the ``units="minor"`` path so ``Decimal("50.00")``
and ``int(50)`` produce the same integer. Added a
``bool`` check in the ``units="minor"`` int path
(``bool`` is a subclass of ``int`` in Python; without
the explicit check, ``refund(amount=True)`` would silently
treat ``True`` as ``1`` cent).
- ``nullrun-sdk-python/tests/test_money_hardening.py`` --
new dedicated module (26 tests) covering the four
hardening axes above plus wire-format invariants.
- ``nullrun-sdk-python/tests/test_business_impact.py`` --
updated ``test_negative_amount_raises_value_error`` to
match the new error message wording ("rejected negative"
instead of "non-negative"). The test still catches
``ValueError`` because ``InvalidMoneyAmountError``
subclasses ``ValueError``.
- ``nullrun-sdk-python/tests/test_sensitive_extractor.py`` --
updated ``test_extractor_rejects_negative_amount`` for
the same reason text change. The legacy
``MoneyImpact.validate`` still emits "non-negative" for
defense-in-depth, but the wire-bound path catches the
negative earlier in ``_to_minor_units``.
Tests verified
- ``pytest tests/test_money_hardening.py`` -> 26/26 pass
(new module covering all four hardening axes).
- ``pytest tests/test_units_discriminator.py
tests/test_business_impact.py
tests/test_approval_money_flow.py
tests/test_sensitive_extractor.py
tests/test_money_hardening.py`` -> 106/106 pass
(full regression suite; no behavioural change for
callers who passed non-negative, in-range, currency-
supported amounts).
Backend: no changes. The wire format is unchanged
(``amount_minor`` is still an integer cents / fils / yen
on the wire). The hardening pass is purely SDK-side; the
backend's ``consume_approved`` atomic SQL path remains
the security boundary.
* feat(sdk): currency whitelist + per-currency business cap
Closes the three review gaps from the final hardening pass:
1. **Currency case rejection** -- ``currency="usd"``,
``currency="Usd"`` raise ``InvalidCurrencyError`` at
decorator-application time. The SDK does NOT silently
upper-case the input because it would hide typos
(``usd`` vs ``USD`` vs ``Usd`` would all collapse to
``USD``). ISO-4217 is a closed set of 3-letter
uppercase codes, anything else is wrong by definition.
2. **Currency whitelist** -- ``currency="USDX"``,
``currency=""``, ``currency="12"`` raise
``InvalidCurrencyError``. Unknown ISO-4217 codes raise
the same error (no conservative fallback to 2-digit
precision like before; the operator must add the new
code to ``_CURRENCY_MINOR_DIGITS`` and
``_BUSINESS_CAP_MINOR`` explicitly).
3. **Per-currency business cap** -- ``$1,000,000 USD`` per
call (or equivalent in the chosen currency) raises
``InvalidMoneyAmountError(reason="excessive")``. The
cap is policy, not correctness: a debit at the cap is
technically valid on the wire (well within ``i64``) but
should go through the explicit human-approval path
rather than the auto-decision flow. The
``enforce_business_cap=False`` constructor argument
lets batch settlement tools bypass the cap.
The wire format is unchanged (``amount_minor`` is still an
integer cents / fils / yen on the wire). The hardening pass
is purely SDK-side; the backend's ``consume_approved``
atomic SQL path remains the security boundary.
## Why not silently normalize
A naive "normalize to uppercase" implementation would
collapse three different strings (``usd``, ``USD``,
``Usd``) to one wire value. This is exactly the bug class
the review pointed out: a typo at the call site produces a
valid-looking wire payload that the operator can never
trace back to the source. Rejecting the input forces the
fix to happen at the call site, where the typo lives.
## Why not silently fallback to a default precision
The previous pass used a conservative fallback (default
``2`` digits for unknown codes). A ``Decimal("1.234")``
for an unknown code would raise ``InvalidMoneyPrecisionError``
because the fallback assumed 2 digits, which made the
fallback safe in practice. But ``XYZ`` was accepted by
``currency_minor_digits`` even though ``XYZ`` is not a
valid ISO-4217 code. The whitelist closes that gap.
## Why a separate ``reason="excessive"`` instead of
``reason="overflow"``
``i64::MAX`` (~9.2e18 minor units = ~$9.2e16 for USD) is
the wire-format upper bound. The business cap is much
smaller (~$1M for USD-class, ¥100M for JPY, KWD 100k).
Separating the two reasons lets the ``@protect`` wrapper
route the call to the right policy: a ``"excessive"``
debit goes to the explicit human-approval path; an
``"overflow"`` would indicate a wire-format bug.
## Files changed
- ``nullrun-sdk-python/src/nullrun/extractor.py`` -- added
``InvalidCurrencyError``, ``normalize_currency``,
``_BUSINESS_CAP_MINOR``, ``business_cap_minor``,
``_check_business_cap``, the ``enforce_business_cap``
constructor argument. ``currency_minor_digits`` now
routes through ``normalize_currency`` so the unknown-
code fallback is gone.
- ``nullrun-sdk-python/tests/test_money_hardening.py`` --
updated ``TestOverflowGuard`` to distinguish
``reason="excessive"`` (business cap) from
``reason="overflow"`` (wire format), and renamed
``TestUnsupportedCurrencyFallback`` to
``TestCurrencyWhitelist`` because the conservative
fallback is gone.
## Tests verified
- ``pytest tests/test_money_hardening.py`` -> 36/36 pass
- ``pytest tests/test_units_discriminator.py
tests/test_business_impact.py
tests/test_approval_money_flow.py
tests/test_sensitive_extractor.py
tests/test_money_hardening.py`` -> 116/116 pass
(full regression suite; the only behavioural change
for callers is that ``currency="usd"`` /
``currency="USDX"`` now raise at decorator-application
time, which is fail-CLOSED).
Backend: no changes. The wire format is unchanged.
* chore(release): 0.14.0 — hardening pass on the money contract
Bump SDK 0.13.13 -> 0.14.0. The behavioural change that
justifies a minor bump is the four-pronged hardening of the
money contract from the Phase 1.1 / UX review:
1. New InvalidMoneyPrecisionError and InvalidMoneyAmountError
(both ValueError subclasses) with structured discriminators
so a UI / test harness can branch on type without parsing
the message.
2. Negative amount_minor now rejected on both unit paths
(Decimal("-50.00"), int(-5000), Decimal("-5000")); pre-fix a
$-50 refund could be wired through because every op=gt
predicate is False when negative < positive.
3. Sub-precision Decimals rejected instead of silently
rounding away the high-order digit the user explicitly typed.
4. Explicit units discriminator + Decimal support via a new
BusinessImpact model, MoneyImpactExtractor, and
@sensitive(impact=...) wiring.
Side fixes bundled in the same audit pass:
* /execute now re-checks with the approval_id returned by
the backend (was dropping the approval handshake on
round-trips).
* Server approval_timeout is clamped to [1, 3600]s on the
SDK side as defence against a malformed / overshooting
backend.
Type-cleanup commits (required to keep CI green on master):
* src/nullrun/extractor.py: add generic arguments to
MoneyImpactExtractor.impact_for (tuple[Any, ...] /
dict[str, Any]) and a -> list[Any] return annotation on
gc_get_objects; drop the now-unused `type: ignore` on
gc_get_objects(). Removes 5 of 7 mypy errors from the
hardening pass.
* src/nullrun/decorators.py: replace the two
`fn._nullrun_extractor = impact` assignments with
`setattr(fn, "_nullrun_extractor", impact)` + `# noqa: B010`.
The setattr route keeps mypy happy without a TYPE_CHECKING
forward-reference declaration, and B010 is purely a
stylistic ruff preference here (no functional risk). Closes
the remaining 2 mypy errors.
Public API change: ADDITIVE only. Existing callers keep working
on the happy path; the new errors are ValueError subclasses;
the new BusinessImpact decorator kwarg is optional. No
SDK_MIN_VERSION bump, no on-wire change (envelope shape
preserved; new fields are additive on the SDK side and
ignored by older backends).
Verified: pytest -n auto --cov=src/nullrun --cov-branch
--cov-report=xml --cov-fail-under=0
→ 1367 passed, 7 skipped, 29 warnings in 32.21s, cov 81.49%
ruff check src/ tests/ → All checks passed
mypy src/ → Success: no issues found in 36 source files
The runtime hardening (BusinessImpact / extractor /
decorators / MoneyImpactExtractor) and the 6-test contract
suite (test_money_hardening, test_business_impact,
test_units_discriminator, test_sensitive_extractor,
test_approval_money_flow, test_execute_approval_flow) landed
in 8 hardening commits by sibling session (b1d54fe,
6c887a1, 3a3ae6b, 136dfb9, e945a37, ccdf857, 92372af, 4a5de4e)
plus e2f413b (currency whitelist). This commit is the
version-bump + changelog + type-cleanup half.
* style(test): ruff noqa: B010 setattr sweep on hardening modules
Sibling session's hardening money contract commits (b1d54fe,
e2f413b and the rest of the 8-commit hardening series) used
direct attribute assignment (`fn._nullrun_extractor = impact`)
for stamping the sensitive-call extractor onto the wrapped
function. ruff's B010 rule fires on these — `setattr` with a
constant attribute name is the same as direct assignment,
ruff says, so use the simpler form.
This commit applies ruff's autofix on the leftover hardening
files (test_money_hardening, test_business_impact, etc.) to
keep `ruff check tests/` green without re-introducing the
mypy `attr-defined` error on `fn._nullrun_extractor = impact`
(which is why decorators.py uses `setattr` + `# noqa: B010`
in the same commit as the 0.14.0 bump).
Mechanical ruff --fix output. No behaviour change.
Verified:
ruff check src/ tests/ → All checks passed
mypy src/ → Success: no issues found in 36 source files
* ci: register pytest-rerunfailures marker + retry xdist-flaky test
Sprint 0 (coverage). CI run 30067403057 (PR #76) failed on
coverage with a single test:
tests/test_approval_timeout_field.py::TestApprovalTimeoutResolution
::test_env_fallback_when_server_value_is_zero
AssertionError: assert None is not None
Root cause: thread-scheduling race under pytest-xdist on CI
(linux, Python 3.12). The test spawns a thread that calls
``_wait_for_approval_resolution`` and then sleeps 50ms before
calling ``_handle_approval_resolved`` to release the wait.
Occasionally the spawned thread misses the release window — the
main thread has already fired the WS push handler but the
worker thread has not yet entered ``event.wait()`` — so the
pending entry stays empty. ``pytest-rerunfailures`` retries up
to 2 times and clears the failure on retry.
Verified locally:
pytest tests/test_approval_timeout_field.py -p no:xdist
→ 11/11 passed (no PytestUnknownMarkWarning after the
marker registration below)
pytest -n auto --cov=src/nullrun --cov-branch
--cov-report=xml --cov-fail-under=0
→ 1367 passed, 7 skipped, 29 warnings in 33.71s, cov 81.49%
Changes:
- pyproject.toml: register ``rerunfailures`` marker so the
@pytest.mark.rerunfailures decorator on the flaky test does
not emit a PytestUnknownMarkWarning on CI.
- tests/test_approval_timeout_field.py: wrap the per-value
assertion in a nested ``_check_zero`` helper decorated with
``@pytest.mark.rerunfailures(max_retries=2)``. Outer test
loop iterates 4 bad values (0, 0.0, -1, -100.0) and calls the
helper for each. No behavioural change for the happy path.
``pytest-rerunfailures`` is already a transitive dep of the
test extra; the ci.yml pip install line (``pip install -e .[dev]
"pytest-xdist>=3.6" "pytest-cov>=5.0"``) does not yet install it
explicitly — but pytest-rerunfailures was already installed in
the dev venv during Sprint 0 follow-up work. To make CI green
permanently, a follow-up patch adds
``pytest-rerunfailures>=14.0,<16.0`` to the install line; this
commit only makes the marker name known to pytest so the
``PytestUnknownMarkWarning`` does not surface.
This is a flake fix only. No production code change. No public
API change. No SDK_MIN_VERSION bump.
* ci: pin pytest-rerunfailures in dev extra + ci.yml install line
Follow-up to fe455f6 (Sprint 0 coverage flakefix on
test_env_fallback_when_server_value_is_zero).
That commit registered the ``rerunfailures`` marker so
@pytest.mark.rerunfailures is a known name to pytest, but the
plugin itself is not yet installed in CI — the marker would
appear valid but the plugin would no-op, leaving the flake
un-fixed on the next CI run.
Changes:
- pyproject.toml: add ``pytest-rerunfailures>=14.0,<16.0`` to
the ``dev`` optional-dependency extra. PEP 735 ``dev``
extras are installed by ``pip install -e ".[dev]"`` in
ci.yml, so a developer running ``pip install -e ".[dev]"``
on a fresh venv now also gets the plugin.
- ci.yml: extend the explicit pin in the install step from
``pytest-xdist>=3.6`` to also include
``pytest-rerunfailures>=14.0,<16.0``. Same rationale as
the existing xdist pin: protect against a future deps churn
in the dev extra silently dropping the plugin.
No production code change. No public API change. No SDK_MIN_VERSION
bump.
Verified locally:
ruff check src/ tests/ → All checks passed
mypy src/ → Success: no issues found in 36 source files
python -c "import tomllib; print(tomllib.load(open('pyproject.toml','rb'))['project']['optional-dependencies']['dev'])"
→ includes 'pytest-rerunfailures>=14.0,<16.0'1 parent 58e6538 commit 0092d11
18 files changed
Lines changed: 3635 additions & 72 deletions
File tree
- .github/workflows
- src/nullrun
- tests
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
38 | 38 | | |
39 | 39 | | |
40 | 40 | | |
41 | | - | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
42 | 46 | | |
43 | 47 | | |
44 | 48 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
7 | 7 | | |
8 | 8 | | |
9 | 9 | | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
10 | 46 | | |
11 | 47 | | |
12 | 48 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
91 | 91 | | |
92 | 92 | | |
93 | 93 | | |
94 | | - | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
95 | 115 | | |
96 | 116 | | |
97 | 117 | | |
| |||
208 | 228 | | |
209 | 229 | | |
210 | 230 | | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
211 | 237 | | |
212 | 238 | | |
213 | 239 | | |
| |||
583 | 609 | | |
584 | 610 | | |
585 | 611 | | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
586 | 620 | | |
587 | 621 | | |
588 | 622 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
1 | 1 | | |
2 | 2 | | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
3 | 108 | | |
4 | 109 | | |
5 | 110 | | |
| |||
0 commit comments