diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b50977..cefe927 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,43 @@ Versioning: [Semantic Versioning](https://semver.org/spec/v2.0.0.html) --- +## [0.14.4] - 2026-07-27 + +ToolParameters Approval Rules wire contract (Tier 2 / Разрыв 2 follow-up). The backend already accepted `BusinessImpact::ToolCall(ToolCallParams)` on the `/execute` wire (backend commit `1e501cd6`); 0.14.4 lands the SDK-side path so users get ToolParameters rules by default on every bare `@sensitive` function, with no decorator change. Also fixes a silent regression in the auto-attach path that dropped an explicit `impact=tool_params({...})` map, and pins the cross-language `ToolCall` action digest against the Rust backend's golden hex. No on-wire breaking change for money callers; the only behavioural change is that bare `@sensitive` now ships `kind=tool_call` on the wire where it previously shipped nothing. + +### Added + +- **`BusinessImpact.tool_call(tool_name, params)`** factory — `business_impact.py:323` new factory builds a `BusinessImpact(kind='tool_call', tool_name=..., params=...)` envelope by analogy with the legacy `BusinessImpact` money constructor. Mirrors the backend `BusinessImpact::ToolCall(ToolCallParams)` variant (`backend/src/proxy/gate/business_impact.rs:62-307`). Used internally by `ToolParamsExtractor`; exposed publicly so users can hand-build impacts without importing the dataclass. +- **`ToolCallParams` dataclass** — `business_impact.py:143` mirrors the backend struct (`tool_name` ≤ 128 bytes, `param_name` ≤ 64, JSON-roundtrippable values only). `BusinessImpact.kind` now discriminates `Money` | `ToolCall`; existing money callers continue to discriminate on the same field via the `extractor_*` metadata. +- **`ToolParamsExtractor` + `tool_params(...)` factory** — `extractor.py:815` (class) and the matching factory. Three modes: explicit `{rule_param: arg_name}` map, `include_all=True` (default — every kwarg captured), or `include_all=False` with no map (empty). PII-masked sentinels (`"***"`) and JSON-unsafe values (`float`, custom objects) are filtered before the wire. The factory is the analogue of `MoneyImpactExtractor + money_outflow(...)`. +- **Bare `@sensitive` now ships ToolParameters on the wire** — `decorators.py:1096` (`_do_sensitive_register`) auto-attaches a default `ToolParamsExtractor(include_all=True)` on a bare `@sensitive` decorator. The stamp goes through `_stamp_extractor_on_innermost` so the bare function (the one `@protect` captures as `fn`) carries the attribute, not just the `@protect` wrapper. An explicit `@sensitive(impact=money_outflow(...))` or `@sensitive(impact=tool_params({...}))` wins — the auto-attach only fires when no extractor is present. +- **`@sensitive(impact=tool_params({...}))` decorator form** — `decorators.py:1065` new docstring + `decorators.py:711` dispatch branch. Operators writing ToolParameters Approval Rules on the backend can now declare the per-rule param map directly at the decorator site instead of relying on the auto-attach default. + +### Fixed + +- **Auto-attach chain walk preserves an explicit `impact=tool_params({...})` map** — `decorators.py:43` new helper `_find_extractor_in_chain` walks `__wrapped__` (bounded at 32 hops) so the auto-attach check sees the explicit extractor stamped on the bare function instead of falling through to the default. **Before this fix**, `@sensitive(impact=tool_params({"delete_force": "force"})) @protect def delete_user(force, user_id): ...` silently shipped `{force: , user_id: }` (the auto-attach default) instead of the explicit `{delete_force: }` map. **After this fix**, the renamed key reaches the wire. Regression tests in `TestAutoAttachChainWalk` (4 cases): bare auto-attach, explicit tool_params map preserved, explicit money_outflow preserved, circular-`__wrapped__` defensive bounded walk. +- **`_enforce_sensitive_tool` dispatch handles both extractor types** — `decorators.py:677` (success path) and `decorators.py:711` (error path) now branch by extractor type. NR-B003 error hint text branches too — operators writing ToolParameters rules see "did you mean `impact=tool_params(...)`?" while money operators see the money remediation advice. +- **Bare `@sensitive` regression in the existing `tests/test_sensitive_extractor.py`** — the 5 existing tests still pass because they register the tool manually via `rt.add_sensitive_tool(name)`, which bypasses the decorator auto-attach path. Documented as a deliberate carve-out: only `@sensitive` (the decorator form) auto-attaches. + +### Tests + +- `tests/test_tool_params_extractor.py` — **23 new tests** across 5 classes (`TestToolParamsFactory`, `TestToolParamsExtraction`, `TestAutoAttachOnBareSensitive`, `TestToolCallParamsShape`, `TestAutoAttachChainWalk`). Covers factory shape (3), three extraction modes (4), PII sentinel + float filtering (3), action digest byte-identity with the backend's canonical JSON (1), the auto-attach wiring (2), dataclass validator (7), kind dispatch (1), and the chain-walk regression (4). Verified: 23/23 pass. +- `tests/test_business_impact.py::TestToolCallActionDigestPins` — **5 new tests** cross-language parity for the `ToolCall` impact, pinned to the same hex literal the Rust backend pins in `backend/src/proxy/gate/business_impact.rs::tests::tool_call_digest_golden_value_stripe_charge_500`. A drift on either side trips the test on the other side next time the suite runs. Fixture payload: `tool_call("stripe.charge", {"region": "EU", "amount": 500})` → `9975a8b75a436fb78b9d141b9e0c0a90838c1243d78119b304ae6ed0526966a6`. +- `tests/test_sensitive_extractor.py` — 5/5 pass (regression check, the auto-attach wiring is additive on top of 0.14.1). +- `tests/test_business_impact.py` — full class passes (28/28 including the 5 new parity pins). +- `tests/test_extractors.py` — 35/35 pass. +- `tests/test_protect.py + test_protect_branches.py + test_execute_approval_flow.py + test_approval_money_flow.py + test_gate_real_path.py + test_handle.py` — 99/99 pass. +- `tests/test_runtime.py + test_runtime_branches.py + test_init_contract.py` — 70/70 pass (1 skipped, pre-existing). + +### Compatibility + +- **Default SDK behaviour for bare `@sensitive` CHANGED** — was `no business_impact on wire`, now `kind=tool_call on wire`. Operators who relied on the Phase 0 path (approval_id-only grant consume) must either pass `@sensitive(impact=tool_params(include_all=False))` explicitly, or accept the new ToolParameters wire shape. The change is additive on the SDK side; legacy backends ignore `kind=tool_call` and fall through to a no-op. +- **Existing `@sensitive(impact=money_outflow(...))` callers are unaffected** — the explicit extractor wins over the auto-attach (verified by `test_explicit_money_outflow_chain_walk_preserved`). +- **Legacy "no impact extractor" call sites (registered via `rt.add_sensitive_tool(name)` directly) are unaffected** — the auto-attach is only wired through `_do_sensitive_register`, which only the `@sensitive` decorator calls. +- **No SDK_MIN_VERSION bump.** ToolParameters is an opt-in backend feature; SDK 0.14.4 talking to a backend that has the `BusinessImpact::ToolCall` variant (commit `1e501cd6` and later) is the supported path. SDK 0.14.4 talking to an older backend works but the `kind=tool_call` envelope is ignored — same effective behaviour as 0.14.3 minus the wire bytes. + +--- + ## [0.14.2] - 2026-07-24 Three hotfixes that fell out of the 0.14.1 demo run. Each one is independently small but each one would have surfaced as a runtime crash on a real customer call, so they ship together as a patch. No on-wire breaking change. No SDK_MIN_VERSION bump. Backends on `1.0.0` keep working unchanged. diff --git a/pyproject.toml b/pyproject.toml index d00abf1..8c49ea5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -140,7 +140,7 @@ name = "nullrun" # by ``@protect`` were missing ``tokens``/``execution_id`` so # the backend's SdkTrackRequest rejected them. See CHANGELOG.md # for the full per-commit description. -version = "0.14.2" +version = "0.14.4" # Kept under the 200-char preview threshold so the full line is visible # without an "expand" click. Keywords are matched against likely search # queries ("AI agent cost control", "LLM circuit breaker", etc.). diff --git a/src/nullrun/__version__.py b/src/nullrun/__version__.py index 3ed40e9..c270269 100644 --- a/src/nullrun/__version__.py +++ b/src/nullrun/__version__.py @@ -1,6 +1,7 @@ """NullRun Platform SDK. -v3.29 / 0.14.1 (2026-07-24) — Decimal JSON serialization patch. +v3.30 / 0.14.4 (2026-07-27) — ToolParameters Approval Rules +wire contract (Tier 2 / Разрыв 2 follow-up). Pre-fix 0.14.0, a ``track_tool`` event payload containing a ``Decimal`` (e.g. ``refund_amount`` from a @@ -1017,5 +1018,5 @@ """ -__version__ = "0.13.11" +__version__ = "0.14.4" __platform_version__ = "1.0.0" diff --git a/src/nullrun/business_impact.py b/src/nullrun/business_impact.py index 125fac9..e96625c 100644 --- a/src/nullrun/business_impact.py +++ b/src/nullrun/business_impact.py @@ -49,10 +49,20 @@ EQ = "eq" -# MVP: only `money` kind is supported; the discriminated union is -# shaped forward-compat for record_count / resource_quantity etc. -# when they land in MVPs 1.1+. +# MVP 1.0: `money` kind for per-call flat amounts. +# MVP 1.1 (ToolParameters / Phase 1 / Tier 2): `tool_call` kind +# for free-form tool-call argument bags matched against +# ToolParameters Approval Rules on the backend. KIND_MONEY = "money" +KIND_TOOL_CALL = "tool_call" + + +# Mirrors the backend constant at +# ``backend/src/proxy/gate/business_impact.rs`` (the same value +# caps both the SDK-side mirror's ``tool_name`` and per-key name +# length). Kept in sync manually; a backend-side bump is a one-line +# edit here. +TOOL_PARAMETERS_MAX_PARAM_NAME = 64 @dataclass @@ -85,21 +95,12 @@ def validate(self) -> None: backend's `MoneyImpact::validate()` mirrors these checks. """ if self.direction not in (OUTFLOW, INFLOW): - raise ValueError( - f"direction must be {OUTFLOW!r} or {INFLOW!r}, " - f"got {self.direction!r}" - ) - if not isinstance(self.amount_minor, int) or isinstance( - self.amount_minor, bool - ): + raise ValueError(f"direction must be {OUTFLOW!r} or {INFLOW!r}, got {self.direction!r}") + if not isinstance(self.amount_minor, int) or isinstance(self.amount_minor, bool): # bool is a subclass of int in Python — explicit exclude. - raise ValueError( - f"amount_minor must be int, got {type(self.amount_minor).__name__}" - ) + raise ValueError(f"amount_minor must be int, got {type(self.amount_minor).__name__}") if self.amount_minor < 0: - raise ValueError( - f"amount_minor must be non-negative, got {self.amount_minor}" - ) + raise ValueError(f"amount_minor must be non-negative, got {self.amount_minor}") if ( not isinstance(self.currency, str) or len(self.currency) != 3 @@ -107,8 +108,7 @@ def validate(self) -> None: or not self.currency.isupper() ): raise ValueError( - f"currency must be a 3-letter uppercase ISO-4217 code, " - f"got {self.currency!r}" + f"currency must be a 3-letter uppercase ISO-4217 code, got {self.currency!r}" ) def to_wire_dict(self) -> dict[str, Any]: @@ -129,6 +129,125 @@ def to_wire_dict(self) -> dict[str, Any]: } +@dataclass +class ToolCallParams: + """Free-form tool-call argument bag (Phase 1 / Tier 2 wire shape). + + Mirrors the backend ``BusinessImpact::ToolCall(ToolCallParams)`` + variant at ``backend/src/proxy/gate/business_impact.rs:62-307``. + The backend matches ``params`` against ToolParameters Approval + Rules (``ValueMatcher``: Equals / OneOf / NumericRange / Regex / + Exists; ``TriggerLogic``: Any / All / DNF groups). + + Why this exists as a separate dataclass (rather than reusing the + raw ``dict[str, Any]`` that the runtime already passes around): + - the validator enforces ``tool_name`` shape and the + canonical-JSON digest layer needs a stable, sortable struct + to produce a byte-identical digest with the backend + ``canonical_json()`` implementation + - the ``extractor_*`` fields mirror the ``MoneyImpact`` + provenance pattern: self-reported by the SDK, treated as + advisory metadata. The trust boundary is the digest + round-trip — the SDK and backend both canonicalise the + same payload to the same bytes, and a mismatch on /execute + re-check is a 403 DIGEST_MISMATCH + + Attributes: + tool_name: canonical name of the tool the SDK is about to + call. Must be non-empty and <= 128 bytes. + params: free-form argument bag the operator wrote the rule + against. Keyed by the rule's ``param_name`` field. + extractor_id: self-reported SDK extractor id (e.g. + "nullrun.tool_call.path"). + extractor_version: self-reported version. + """ + + tool_name: str + params: dict[str, Any] = field(default_factory=dict) + extractor_id: str = "nullrun.tool_call.path" + extractor_version: str = "1" + + def validate(self) -> None: + """Reject malformed impacts at extraction time (fail-fast). + + Mirrors ``ToolCallParams::validate()`` in the backend so a + tool with bad extractor args fails locally before the wire + round-trip (one error class, one user_action message). + """ + if not isinstance(self.tool_name, str) or not self.tool_name: + raise ValueError("tool_name must be a non-empty string") + if len(self.tool_name) > 128: + raise ValueError(f"tool_name length {len(self.tool_name)} exceeds max 128") + if not self.tool_name.isascii(): + raise ValueError("tool_name must be printable ASCII") + for k in self.params: + if not isinstance(k, str): + raise ValueError(f"params key {k!r} must be a string") + if len(k) > TOOL_PARAMETERS_MAX_PARAM_NAME: + raise ValueError( + f"params['{k}'] key length {len(k)} exceeds " + f"max {TOOL_PARAMETERS_MAX_PARAM_NAME}" + ) + _validate_param_value(self.params[k], path=f"params['{k}']") + + def to_wire_dict(self) -> dict[str, Any]: + """Serialize to the JSON shape the backend expects. + + Key order is NOT significant — the backend's + ``canonical_json()`` re-sorts keys before hashing. + """ + return { + "kind": KIND_TOOL_CALL, + "tool_name": self.tool_name, + "params": dict(self.params), + "extractor_id": self.extractor_id, + "extractor_version": self.extractor_version, + } + + +def _validate_param_value(value: Any, path: str) -> None: + """Reject values that the digest layer cannot round-trip. + + Backend mirror at ``business_impact.rs:310-318``: the canonical + JSON layer accepts the four JSON kinds (null/bool/number/string/ + object/array) but rejects f64 and non-finite numbers because + ``serde_json::Number`` cannot losslessly represent them. We do + the same here so the SDK fails at extraction time rather than + producing a digest that the backend will reject. + """ + if value is None or isinstance(value, bool): + return + if isinstance(value, int): + # int round-trips through JSON losslessly. NOTE: bool is a + # subclass of int in Python; we explicitly handle it above. + return + if isinstance(value, str): + return + if isinstance(value, (list, tuple)): + for i, item in enumerate(value): + _validate_param_value(item, path=f"{path}[{i}]") + return + if isinstance(value, dict): + for k, v in value.items(): + _validate_param_value(v, path=f"{path}['{k}']") + return + if isinstance(value, float): + # Reject explicitly -- we DO NOT round to int because the + # operator might be relying on sub-cent precision (this is + # the same rationale as MoneyImpactExtractor rejecting + # ``float`` for money amounts). + raise ValueError( + f"{path}: float values are not supported on the wire " + f"(JSON round-trip is not lossless for IEEE-754); pass " + f"an int (minor units) or a str (operator-defined format)" + ) + raise ValueError( + f"{path}: value of type {type(value).__name__!r} is not " + f"supported on the wire; pass int / str / bool / None / " + f"list / dict" + ) + + def business_impact_to_dict(impact: BusinessImpact) -> dict[str, Any]: """Top-level wire dict for `GateRequest.business_impact`. @@ -146,18 +265,23 @@ def business_impact_to_dict(impact: BusinessImpact) -> dict[str, Any]: class BusinessImpact: """Top-level BusinessImpact union. - For MVP 1.0 the only supported variant is `Money`. Future - kinds land by adding new subclasses and a `kind` value. + MVP 1.0: `Money` only. + MVP 1.1 (Phase 1 / Tier 2): adds `ToolCall` for free-form + tool-call argument bags matched against ToolParameters + Approval Rules on the backend. + The SDK validates the variant at construction time so the backend never sees malformed output. """ - impact: Any # MoneyImpact in MVP. + impact: Any # MoneyImpact | ToolCallParams @property def kind(self) -> str: if isinstance(self.impact, MoneyImpact): return KIND_MONEY + if isinstance(self.impact, ToolCallParams): + return KIND_TOOL_CALL raise TypeError(f"unknown impact type: {type(self.impact)}") def validate(self) -> None: @@ -181,6 +305,31 @@ def money( m.validate() return cls(impact=m) + @classmethod + def tool_call( + cls, + tool_name: str, + params: dict[str, Any] | None = None, + extractor_id: str = "nullrun.tool_call.path", + extractor_version: str = "1", + ) -> BusinessImpact: + """Construct a ``kind="tool_call"`` BusinessImpact. + + Used by the ToolParamsExtractor; callers building impacts + by hand should use this factory rather than constructing + ``ToolCallParams`` and wrapping themselves -- the factory + validates before returning so a misuse fails locally + instead of after a wire round-trip. + """ + p = ToolCallParams( + tool_name=tool_name, + params=params or {}, + extractor_id=extractor_id, + extractor_version=extractor_version, + ) + p.validate() + return cls(impact=p) + def _canonicalize_json(value: Any) -> Any: """Sort object keys recursively before serialization. diff --git a/src/nullrun/decorators.py b/src/nullrun/decorators.py index 8957201..7330dca 100644 --- a/src/nullrun/decorators.py +++ b/src/nullrun/decorators.py @@ -674,12 +674,22 @@ def _enforce_sensitive_tool( if extractor is not None: try: from nullrun.business_impact import compute_action_digest - from nullrun.extractor import MoneyImpactExtractor + from nullrun.extractor import MoneyImpactExtractor, ToolParamsExtractor if isinstance(extractor, MoneyImpactExtractor): impact = extractor.impact_for(fn, args, kwargs) business_impact_dict = impact.to_wire_dict() action_digest_hex = compute_action_digest(impact) + elif isinstance(extractor, ToolParamsExtractor): + # Phase 1 / MVP 1.1 (Tier 2): free-form tool-call + # argument bag, matched against ToolParameters + # Approval Rules on the backend. Same wire envelope + # (BusinessImpact) and same digest contract as the + # Money variant -- only the discriminator and the + # ``params`` field differ. + impact = extractor.impact_for(fn, args, kwargs) + business_impact_dict = impact.to_wire_dict() + action_digest_hex = compute_action_digest(impact) except Exception as exc: # noqa: BLE001 from nullrun.breaker.exceptions import ( NullRunBlockedException, @@ -688,6 +698,39 @@ def _enforce_sensitive_tool( ) workflow_id = get_workflow_id() or UNKNOWN_WORKFLOW_ID + # The user-facing hint depends on which extractor fired. + # Money extractor wants the bound arg name; ToolParams + # extractor wants the rule-param -> arg-name mapping + # (or the include_all flag if no map was supplied). + if isinstance(extractor, MoneyImpactExtractor): + hint = ( + "could not extract a MoneyImpact from the live " + "arguments. Check that the function declares the " + "argument named in `impact=money_outflow(...)`." + ) + elif isinstance(extractor, ToolParamsExtractor): + if extractor.param_extractors is not None: + hint = ( + "could not extract a ToolCall impact from the " + "live arguments. Check that the function " + "declares every arg named in " + "`impact=tool_params(...)`." + ) + else: + hint = ( + "could not extract a ToolCall impact from the " + "live arguments. The @sensitive tool's kwargs " + "could not be validated for wire emission " + "(unsupported types or invalid param keys)." + ) + else: + # Defensive fallback for a future extractor type + # that doesn't update this hint. + hint = ( + "could not extract business_impact from the live " + "arguments. Check the @sensitive decorator's " + "`impact=...` argument." + ) err = NullRunBlockedException( workflow_id=workflow_id, reason=( @@ -695,12 +738,7 @@ def _enforce_sensitive_tool( ), tool_name=fn.__name__, error_code="NR-B003", - user_action=( - f"The @sensitive decorator on {fn.__name__!r} " - f"could not extract a MoneyImpact from the live " - f"arguments. Check that the function declares the " - f"argument named in `impact=money_outflow(...)`." - ), + user_action=(f"The @sensitive decorator on {fn.__name__!r} {hint}"), ) runtime._emit_sdk_error( err, @@ -1017,7 +1055,84 @@ def _stamp_extractor_on_innermost(fn: F, impact: Any) -> None: setattr(fn, "_nullrun_extractor", impact) # noqa: B010 +def _find_extractor_in_chain(fn: Any) -> Any: + """Walk ``fn.__wrapped__`` looking for a stamped extractor. + + Used by ``_do_sensitive_register`` to detect an explicit + ``impact=tool_params({...})`` (or ``impact=money_outflow(...)``) + that was already stamped on the bare function by the + ``@sensitive`` factory form. Without the chain walk the + auto-attach path would see ``None`` on the @protect wrapper + and silently stamp its default ToolParamsExtractor on top, + breaking the user's explicit map. + + Returns the extractor object (the actual ``_nullrun_extractor`` + value) or ``None`` if no extractor is found on the chain. + The chain walk is bounded to ``len(repr(callable))`` hops to + defend against pathological ``__wrapped__`` cycles; in practice + the chain is at most 3 deep (@sensitive factory + @protect + + functools.wraps chain from @protect). + """ + seen: set[int] = set() + current: Any = fn + # Bound the walk: a decorator chain longer than this is almost + # certainly a cycle. The cap is generous (the real chain is + # typically 2-3 deep). + for _ in range(32): + if current is None or id(current) in seen: + return None + seen.add(id(current)) + ext = getattr(current, "_nullrun_extractor", None) + if ext is not None: + return ext + current = getattr(current, "__wrapped__", None) + return None + + def _do_sensitive_register(fn: F) -> F: + # Phase 1 / MVP 1.1 (Tier 2 -- ToolParameters): if @sensitive + # was applied bare (no impact=...), auto-attach a default + # ``ToolParamsExtractor(include_all=True)`` so the tool is + # immediately eligible for ToolParameters Approval Rules + # without requiring every user to write + # ``@sensitive(impact=tool_params())`` explicitly. + # + # The existing money extractor (set via + # ``@sensitive(impact=money_outflow(...))``) wins because the + # ``@sensitive`` decorator stamps the explicit extractor + # BEFORE calling this function; we only auto-attach when no + # extractor is present. See ``sensitive()`` factory form + # (lines ~979) where ``_attach_decorator`` runs first and may + # have already set ``_nullrun_extractor``. + # + # The auto-attach uses ``_stamp_extractor_on_innermost`` so the + # attribute lands on the bare user function -- the @protect + # wrapper captures the bare function as ``fn`` and the + # ``_enforce_sensitive_tool`` guard finds the extractor via a + # single ``getattr`` lookup. See the 2026-07-24 root-cause + # fix (line 1024 onward) for the rationale. + try: + from nullrun.extractor import ToolParamsExtractor, tool_params + + # Walk the __wrapped__ chain in case the explicit extractor + # was stamped on the bare function (by + # ``_stamp_extractor_on_innermost``) and we received the + # @protect-wrapped outer function as ``fn``. Without the + # chain walk, the auto-attach would silently overwrite + # the explicit extractor and break the user's + # ``impact=tool_params({...})`` map. + if _find_extractor_in_chain(fn) is None: + _stamp_extractor_on_innermost(fn, tool_params(include_all=True)) + except ImportError: + # The extractor module is loaded above us on every path + # we care about; this ImportError guard is defensive in + # case the SDK is shrunk (e.g. for a hypothetical + # tool-only build). Falling back to legacy Phase 0 path + # is the safe default -- the wire payload drops the + # business_impact field and the backend uses approval_id- + # only grant consume. + pass + try: # Use the same slot the @protect wrapper uses so the # registration lands on the same runtime instance the diff --git a/src/nullrun/extractor.py b/src/nullrun/extractor.py index 851f372..e8ad150 100644 --- a/src/nullrun/extractor.py +++ b/src/nullrun/extractor.py @@ -167,6 +167,7 @@ def refund(amount: Decimal) -> ... # 50 = $50.00 (5000 cents) OUTFLOW, BusinessImpact, MoneyImpact, + ToolCallParams, compute_action_digest, ) @@ -439,9 +440,7 @@ def business_cap_minor(currency: str) -> int: return _BUSINESS_CAP_MINOR[normalize_currency(currency)] -def _decimal_has_more_fractional_digits( - value: Decimal, allowed: int -) -> bool: +def _decimal_has_more_fractional_digits(value: Decimal, allowed: int) -> bool: """Return True iff ``value`` has more fractional digits than ``allowed``. @@ -492,9 +491,7 @@ def _check_overflow(amount_minor: int, currency: str) -> None: ) -def _check_business_cap( - amount_minor: int, currency: str, enforce: bool -) -> None: +def _check_business_cap(amount_minor: int, currency: str, enforce: bool) -> None: """Raise ``InvalidMoneyAmountError`` if ``amount_minor`` exceeds the per-currency business cap. @@ -521,7 +518,9 @@ def _check_business_cap( def _to_minor_units( - value: int | Decimal, units: str, currency: str, + value: int | Decimal, + units: str, + currency: str, enforce_business_cap: bool = True, ) -> int: """Convert a Decimal-or-int value to integer minor units. @@ -600,9 +599,7 @@ def _to_minor_units( _check_business_cap(converted, currency, enforce_business_cap) return converted - raise ValueError( - f"unknown units={units!r}; expected one of: {UNITS}" - ) + raise ValueError(f"unknown units={units!r}; expected one of: {UNITS}") class MoneyImpactExtractor: @@ -644,14 +641,9 @@ def __init__( enforce_business_cap: bool = True, ) -> None: if direction not in (OUTFLOW, INFLOW): - raise ValueError( - f"direction must be {OUTFLOW!r} or {INFLOW!r}, " - f"got {direction!r}" - ) + raise ValueError(f"direction must be {OUTFLOW!r} or {INFLOW!r}, got {direction!r}") if units not in UNITS: - raise ValueError( - f"units must be one of {UNITS}, got {units!r}" - ) + raise ValueError(f"units must be one of {UNITS}, got {units!r}") # ``normalize_currency`` raises ``InvalidCurrencyError`` # if the input is not a 3-letter uppercase ISO-4217 # code in the whitelist. The constructor fails-CLOSED: @@ -768,6 +760,274 @@ def money_outflow( ) +# ============================================================================ +# Phase 1 / MVP 1.1 -- ToolParameters (Разрыв 2 / Tier 2) +# ============================================================================ +# +# The ToolParamsExtractor is the SDK-side mirror of the backend's +# ``BusinessImpact::ToolCall(ToolCallParams)`` variant. It captures a +# free-form argument bag from the live call and ships it as +# ``kind: "tool_call"`` on the /execute wire so the backend can match +# the params against ToolParameters Approval Rules (ValueMatcher: +# Equals / OneOf / NumericRange / Regex / Exists; TriggerLogic: Any / +# All / DNF groups). +# +# Why this exists alongside MoneyImpactExtractor: +# - The Money variant answers one question ("how much money?") and +# is matched against MoneyAmount predicates. +# - The ToolCall variant answers a different question ("which args?") +# and is matched against ToolParameters predicates. +# - Same wire envelope (``BusinessImpact``), same digest contract, same +# fail-CLOSED semantics at extraction time -- only the +# ``impact_for(...)`` body differs. +# +# Why ``include_all=True`` is the default (and not opt-in): +# - Operators adopting ToolParameters Approval Rules need their +# tools to ship args without rewriting every decorator site. +# - Bare ``@sensitive`` (no impact=...) auto-attaches this extractor +# in ``_do_sensitive_register`` (see decorators.py) so the +# behavior is "every @sensitive tool ships its args by default". +# - Users with sensitive args (e.g. raw PANs, secrets) who want to +# opt out pass ``include_all=False`` AND set ``param_extractors`` +# to a whitelist of safe-to-share keys. +# +# Why ``param_extractors`` is an explicit map (not a glob): +# - The operator-facing rule references param names +# ("user_id == 42") not arg names ("uid", "userId" -- which the +# SDK may rename mid-refactor). An explicit map decouples the +# rule name from the function signature. +# - Without it, a Python refactor of ``uid`` -> ``user_id`` would +# silently break every ToolParameters rule without warning. + +_TOOL_CALL_EXTRACTOR_ID = "nullrun.tool_call.path" +_TOOL_CALL_EXTRACTOR_VERSION = "1" + + +class ToolParamsExtractor: + """Phase 1 / MVP 1.1 tool-params impact extractor. + + Captures the live call's kwargs into a free-form JSON object + that the backend matches against ToolParameters Approval + Rules. The wire shape mirrors ``ToolCallParams`` in + ``nullrun.business_impact`` and the backend + ``BusinessImpact::ToolCall`` variant in + ``backend/src/proxy/gate/business_impact.rs:62-307``. + + Three extraction modes (mutually exclusive in priority order): + + 1. ``param_extractors`` set: explicit {rule_param: arg_name} + map. Only the listed args are extracted; everything else + is dropped. Use this when the rule name diverges from the + function arg name (``{"user_id": "uid"}``). + 2. ``include_all=True`` (default): capture every kwarg as-is. + Use this when rule names match arg names. + 3. ``include_all=False`` and no map: empty ``params``. Rare; + for tools that take no args but should still be eligible + for ToolCall-kind Approval Rules. + + Args dropped from ``params``: + - positional args (only kwargs survive; positional binding + would require the operator to know Python's arg-order + semantics, which is fragile across refactors) + - values that fail ``_validate_param_value`` (e.g. ``float`` + which can't round-trip through JSON losslessly) + - values that survived ``_safe_kwargs`` masking (i.e. + ``***`` sentinels for PAN, password, etc.). Sending a + masked sentinel to the operator would never match a + real rule -- the rule predicate sees ``"***"`` and never + the real value. So masking happens BEFORE this extractor + runs and the masked value is filtered out here. + """ + + __slots__ = ( + "param_extractors", + "include_all", + "extractor_id", + "extractor_version", + ) + + def __init__( + self, + param_extractors: dict[str, str] | None = None, + *, + include_all: bool = True, + extractor_id: str = _TOOL_CALL_EXTRACTOR_ID, + extractor_version: str = _TOOL_CALL_EXTRACTOR_VERSION, + ) -> None: + if param_extractors is not None and include_all is False: + # The two modes are mutually exclusive. Setting both is + # almost certainly a typo -- fail-CLOSED at decorator- + # application time rather than silently dropping rules. + raise ValueError( + "ToolParamsExtractor: param_extractors and include_all=False are mutually exclusive" + ) + self.param_extractors = param_extractors + self.include_all = include_all + self.extractor_id = extractor_id + self.extractor_version = extractor_version + + def impact_for( + self, + fn: Callable[..., Any], + args: tuple[Any, ...], + kwargs: dict[str, Any], + ) -> BusinessImpact: + """Extract a ToolCall BusinessImpact from the live call. + + Args: + fn: the decorated function (used only for ``fn.__name__`` + on the wire; positional arg shape is not consulted). + args: positional args (dropped; only kwargs are extracted). + kwargs: keyword args from the live call. May have been + PII-masked by ``_safe_kwargs`` before reaching here; + masked values (any ``str`` that equals the + ``MASK_SENTINEL`` value used by the masking layer) + are filtered out before the wire. + + Returns: + ``BusinessImpact(impact=ToolCallParams(...))`` ready to + serialise to wire via ``to_wire_dict()``. + + Raises: + ValueError: if the resulting ``ToolCallParams`` fails + validation (bad ``tool_name``, ``params`` key too + long, f64 value, unsupported type). + """ + # Filter masked values BEFORE building ToolCallParams. The + # masking layer uses a fixed sentinel string (e.g. "***"); + # sending that to the backend would never match a real + # rule, so we silently drop it. This matches the + # pre-existing rationale that masked audit-log entries + # exist for compliance, not for runtime matching. + # Note: the decorator wrapper above us already masked the + # positional args (via ``_safe_args``) and the kwargs (via + # ``_safe_kwargs``); we just filter kwargs against the + # same sentinel value. + params = self._extract_params(kwargs) + + params_obj = ToolCallParams( + tool_name=fn.__name__, + params=params, + extractor_id=self.extractor_id, + extractor_version=self.extractor_version, + ) + params_obj.validate() + return BusinessImpact(impact=params_obj) + + def _extract_params(self, kwargs: dict[str, Any]) -> dict[str, Any]: + """Pull the right kwargs into the wire-shape ``params`` dict. + + Three branches, in priority order: + 1. ``param_extractors`` set: explicit {rule_param: arg_name} + 2. ``include_all=True``: every kwarg + 3. neither: empty dict + + Each value is filtered through ``_safe_for_wire``: only + JSON-roundtrippable types survive, and PII-masked + sentinels are dropped. + """ + result: dict[str, Any] = {} + if self.param_extractors is not None: + for rule_param, arg_name in self.param_extractors.items(): + if arg_name not in kwargs: + continue + value = kwargs[arg_name] + if not _safe_for_wire(value): + continue + result[rule_param] = value + elif self.include_all: + for k, v in kwargs.items(): + if not _safe_for_wire(v): + continue + result[k] = v + return result + + +def _safe_for_wire(value: Any) -> bool: + """Return True if ``value`` can land on the wire unmodified. + + Two reasons to drop a value: + 1. It's a PII-masked sentinel (``"***"`` or similar) -- the + operator would see a placeholder, never the real value, + so the rule predicate can't match. + 2. It's an unsupported type that the canonical-JSON layer + can't round-trip (``float``, ``set``, custom objects). + The extractor validates each surviving value through + ``_validate_param_value`` which catches f64 explicitly. + + This is the wire-safety mirror of the backend's + ``ToolCallParams::validate()`` and ``_check_value_kind``. + Doing the check here keeps the SDK's "what to ship" logic + in one place (this module), so the wire shape is + documented and testable without crossing the network. + """ + if value is None: + return True + if isinstance(value, bool): + return True + if isinstance(value, int): + return True + if isinstance(value, str): + # Drop PII-masked sentinels. The masking layer uses the + # literal "***" string; a rule that matches "***" would + # be a confused rule. Real values that happen to equal + # "***" are vanishingly rare; if the operator runs into + # it, they can rename the value. + if value == "***": + return False + return True + if isinstance(value, (list, tuple)): + return True + if isinstance(value, dict): + return True + # float, set, custom objects -- rejected at this layer so we + # never build a ToolCallParams that the backend would reject. + return False + + +def tool_params( + param_extractors: dict[str, str] | None = None, + *, + include_all: bool = True, +) -> ToolParamsExtractor: + """Shorthand constructor used by ``@sensitive(impact=tool_params(...))``. + + Phase 1 / MVP 1.1 (Tier 2): every bare ``@sensitive`` tool + auto-attaches a ``ToolParamsExtractor(include_all=True)`` + (see ``_do_sensitive_register`` in decorators.py), so most + users never need to call this function explicitly. The + factory below is for two opt-in cases: + + 1. Explicit ``{rule_param: arg_name}`` mapping when the + rule name diverges from the function arg name:: + + @sensitive(impact=tool_params({"user_id": "uid"})) + def delete_user(uid: int): ... + + 2. Strict opt-out from auto-capture (rare; for tools whose + every kwarg is a secret the operator must never see):: + + @sensitive(impact=tool_params(include_all=False)) + def handle_secret(token: str): ... + + Args: + param_extractors: explicit ``{rule_param: arg_name}`` map. + When set, only those args are captured under + ``rule_param`` keys. ``include_all`` is ignored. + include_all: when True (default), capture every kwarg. + Ignored when ``param_extractors`` is set. + + Returns: + ``ToolParamsExtractor`` ready to be passed to + ``@sensitive(impact=...)`` or stamped on a function by the + auto-attach path. + """ + return ToolParamsExtractor( + param_extractors=param_extractors, + include_all=include_all, + ) + + def compute_impact_digest(impact: BusinessImpact) -> str: """Thin alias re-exported for call-site readability.""" return compute_action_digest(impact) @@ -775,4 +1035,5 @@ def compute_impact_digest(impact: BusinessImpact) -> str: def gc_get_objects() -> list[Any]: import gc - return gc.get_objects() \ No newline at end of file + + return gc.get_objects() diff --git a/tests/test_business_impact.py b/tests/test_business_impact.py index 33c7def..a3728f9 100644 --- a/tests/test_business_impact.py +++ b/tests/test_business_impact.py @@ -44,6 +44,7 @@ OUTFLOW, BusinessImpact, MoneyImpact, + ToolCallParams, business_impact_to_dict, compute_action_digest, ) @@ -56,6 +57,22 @@ "dfc96387ca539b7130caebe705e042f2e34e52ab44352ae5e527bcef64f0df27" ) +# Cross-language parity pin for the Tier 2 / Разрыв 2 +# ``ToolCall`` impact (Разрыв 2 / 2026-07-27). The Rust backend +# asserts the same hex literal in +# ``backend/src/proxy/gate/business_impact.rs::tests:: +# tool_call_digest_golden_value_stripe_charge_500``. Any drift +# between SDK and backend trips BOTH pins (here on the SDK +# side, in ``cargo test`` on the backend side). The fixture +# payload is ``BusinessImpact::ToolCall(tool_call("stripe.charge"))`` +# (backend helper at ``business_impact.rs:1473``): tool name +# ``stripe.charge``, params ``{"region": "EU", "amount": 500}``. +# The protocol prefix and canonical-JSON algorithm must remain +# identical across both languages. +GOLDEN_HEX_TOOL_CALL_STRIPE_CHARGE_500 = ( + "9975a8b75a436fb78b9d141b9e0c0a90838c1243d78119b304ae6ed0526966a6" +) + # --------------------------------------------------------------------------- # 1. Round-trip / canonical-JSON pin @@ -163,9 +180,7 @@ def test_wire_dict_round_trips_through_json(self) -> None: # --------------------------------------------------------------------------- -def _refund_customer_func( - amount_cents: int, customer_id: str = "c-1" -) -> dict: +def _refund_customer_func(amount_cents: int, customer_id: str = "c-1") -> dict: """Stand-in for a user-decorated tool. Mirrors the shape of ``refund_customer(amount_cents=..., customer_id=...)`` that ``test_approval_money_flow.py::TestExtractor`` exercises.""" @@ -266,4 +281,124 @@ def test_bool_amount_rejected_even_though_bool_is_int_in_python(self) -> None: # ``amount_minor=1`` to forge a tiny refund. ext = money_outflow(argument="amount_cents") with pytest.raises(TypeError): - ext.impact_for(_refund_customer_func, (True,), {"customer_id": "c-1"}) \ No newline at end of file + ext.impact_for(_refund_customer_func, (True,), {"customer_id": "c-1"}) + + +# --------------------------------------------------------------------------- +# 1b. Tier 2 / Разрыв 2 — ToolCall impact cross-language parity +# --------------------------------------------------------------------------- +# +# Pins the SDK digest to the SAME hex literal the Rust backend +# pins in +# ``backend/src/proxy/gate/business_impact.rs::tests:: +# tool_call_digest_golden_value_stripe_charge_500``. A drift on +# either side trips the test on the OTHER side the next time +# the suite runs. +# +# Fixture payload: tool name ``stripe.charge``, params +# ``{"region": "EU", "amount": 500}`` (mirror of the backend +# ``tool_call("stripe.charge")`` helper at +# ``business_impact.rs:1473``). + + +class TestToolCallActionDigestPins: + """Cross-language parity for the ``ToolCall`` impact.""" + + def test_tool_call_stripe_charge_500_matches_golden_hex(self) -> None: + impact = BusinessImpact.tool_call( + "stripe.charge", + {"region": "EU", "amount": 500}, + ) + assert compute_action_digest(impact) == GOLDEN_HEX_TOOL_CALL_STRIPE_CHARGE_500, ( + "ToolCall digest drifted from the Rust golden pin; SDK " + "and backend disagree on the same payload. See " + "docs/runbooks/action-digest-contract.md BEFORE bumping " + "the hex — a real cross-language drift is a P0 security " + "regression (the operator's approval would silently " + "mismatch the SDK's replay on /execute)." + ) + + def test_tool_call_two_calls_produce_identical_hex(self) -> None: + # Determinism for the tamper-evident re-check on + # /execute (same input -> same output, byte-for-byte). + a = compute_action_digest( + BusinessImpact.tool_call( + "stripe.charge", + {"region": "EU", "amount": 500}, + ) + ) + b = compute_action_digest( + BusinessImpact.tool_call( + "stripe.charge", + {"region": "EU", "amount": 500}, + ) + ) + assert a == b == GOLDEN_HEX_TOOL_CALL_STRIPE_CHARGE_500 + + def test_tool_call_param_change_produces_different_hex(self) -> None: + # Any parameter change flips the digest, so the + # operator's approved-arg-bag snapshot either matches + # the SDK's replay exactly or refuses with 403 + # DIGEST_MISMATCH. This test asserts the positive + # half: change the param value, get a different digest. + a = compute_action_digest( + BusinessImpact.tool_call( + "stripe.charge", + {"region": "EU", "amount": 500}, + ) + ) + b = compute_action_digest( + BusinessImpact.tool_call( + "stripe.charge", + {"region": "US", "amount": 500}, # region: EU -> US + ) + ) + assert a != b + + def test_tool_call_wire_dict_shape(self) -> None: + # The ``kind`` discriminator on the wire is what the + # backend's ``serde(tag = "kind", rename_all = + # "snake_case")`` uses to route to + # ``BusinessImpact::ToolCall(...)``. A typo here + # (e.g. "toolcall", "tool-call", "toolCall") would + # silently route to Money on the backend side and + # either match a money rule by accident + # (false-positive approval) or fail to match a tool + # rule (false-negative block). + d = business_impact_to_dict( + BusinessImpact.tool_call( + "stripe.charge", + {"region": "EU", "amount": 500}, + ) + ) + assert d["kind"] == "tool_call" + assert d["tool_name"] == "stripe.charge" + assert d["params"] == {"region": "EU", "amount": 500} + + def test_tool_call_extractor_metadata_advisory(self) -> None: + # The ``extractor_*`` fields are advisory provenance + # metadata — they're serialised to the wire but the + # backend doesn't enforce a specific value. The contract + # is that they're present, strings, and default to the + # ``nullrun.tool_call.path`` extractor. A change to the + # default here is a wire-shape change (additive, but the + # backend's audit-event consumer may start writing new + # rows keyed on the new value). + impact = BusinessImpact.tool_call( + "stripe.charge", + {"region": "EU", "amount": 500}, + ) + d = business_impact_to_dict(impact) + assert d["extractor_id"] == "nullrun.tool_call.path" + assert d["extractor_version"] == "1" + + # NOTE: ``ToolCallParams.validate()`` is NOT auto-invoked by the + # dataclass __init__; the SDK relies on ``BusinessImpact.tool_call(...)`` + # factory (which calls ``validate()`` itself) to enforce the + # rejection paths. Direct ``ToolCallParams(tool_name="", ...)`` + # construction succeeds without raising. This is a documented + # design choice — the dataclass is a wire-shape carrier, not an + # enforcing validator. Validation tests are deferred until the SDK + # decides whether to enforce ``__post_init__`` (the matching + # backend Rust struct uses ``ToolCallParams::new(...).validate()`` + # only at the construction site, mirroring the Python factory). diff --git a/tests/test_tool_params_extractor.py b/tests/test_tool_params_extractor.py new file mode 100644 index 0000000..4bbb682 --- /dev/null +++ b/tests/test_tool_params_extractor.py @@ -0,0 +1,677 @@ +"""Phase 1 / MVP 1.1 -- SDK e2e for the ToolParameters path. + +These tests pin the wire shape produced by ``@sensitive`` when +paired with ``ToolParamsExtractor`` (the Tier 2 / Razryv 2 +follow-up to ``MoneyImpactExtractor``). Mirrors the structure of +``test_sensitive_extractor.py`` so a reader who knows one file +knows the other. + +What this file covers: +- ``tool_params()`` factory: include_all default, explicit map, + mutual-exclusion guard +- ``ToolParamsExtractor.impact_for``: three extraction modes + (explicit map, include_all, empty) +- Auto-attach on bare ``@sensitive`` (no impact=...) ships + ``kind: "tool_call"`` with all kwargs as ``params`` +- Auto-attach does NOT overwrite an explicit + ``@sensitive(impact=money_outflow(...))`` +- PII-masked sentinels (``"***"``) are filtered out +- Unsupported types (float) cause fail-CLOSED at extraction time +- Wire payload contains ``business_impact`` (ToolCall shape) + + ``action_digest`` (byte-identical to the SDK's own computation) + +The tests use ``_enforce_sensitive_tool`` directly rather than +``@sensitive`` decoration when checking the extraction layer in +isolation, and ``@sensitive`` decoration when checking the +auto-attach wiring. Both paths are exercised; the second is +the production-critical one. +""" + +from __future__ import annotations + +import hashlib +import json +from typing import Any + +import pytest + +from nullrun._registry import get_registry +from nullrun.business_impact import ( + KIND_TOOL_CALL, + TOOL_PARAMETERS_MAX_PARAM_NAME, + BusinessImpact, + ToolCallParams, + compute_action_digest, +) +from nullrun.decorators import ( + _do_sensitive_register, + _enforce_sensitive_tool, + _find_extractor_in_chain, + _stamp_extractor_on_innermost, +) +from nullrun.extractor import ( + MoneyImpactExtractor, + ToolParamsExtractor, + money_outflow, + tool_params, +) +from nullrun.runtime import NullRunRuntime + +# --------------------------------------------------------------------------- +# Wire payload capture (same pattern as test_sensitive_extractor.py) +# --------------------------------------------------------------------------- + + +class _PayloadCapture: + """Trampoline that records the most recent kwargs to + ``runtime._transport.execute`` and returns a synthetic "allow" + decision. + """ + + def __init__(self) -> None: + self.last_kwargs: dict[str, Any] | None = None + + def __call__(self, *args: Any, **kwargs: Any) -> dict[str, Any]: + del args + self.last_kwargs = kwargs + return { + "decision": "allow", + "decision_source": "test_capture", + "policy_version": 0, + "allow_execution": True, + } + + +@pytest.fixture +def captured_runtime(monkeypatch): + """Build a test-mode runtime, register it as the active + singleton, and rebind ``_transport.execute`` to a recorder. + """ + NullRunRuntime.reset_instance() + rt = NullRunRuntime(api_key="nr_test_tier2", _test_mode=True) + cap = _PayloadCapture() + monkeypatch.setattr(rt._transport, "execute", cap) + get_registry().set(rt) + yield rt + get_registry().clear() + NullRunRuntime.reset_instance() + + +@pytest.fixture +def captured_payload(captured_runtime) -> _PayloadCapture: + return captured_runtime._transport.execute # type: ignore[attr-defined,return-value] + + +def _register_tool(rt: NullRunRuntime, fn: Any) -> Any: + """Manual registration helper -- mirrors what @sensitive does + at decoration time but without paying the runtime-singleton + init cost on every test. Used for the extraction-layer tests. + """ + rt.add_sensitive_tool(fn.__name__) + return fn + + +# --------------------------------------------------------------------------- +# 1. Factory tests (no runtime, no extraction -- just constructor shape) +# --------------------------------------------------------------------------- + + +class TestToolParamsFactory: + def test_default_is_include_all_true(self) -> None: + """``tool_params()`` with no args must capture every kwarg. + + Bare ``@sensitive`` auto-attaches this default; operators + adopting ToolParameters Approval Rules need the tool to + ship its args without rewriting every decorator site. + """ + e = tool_params() + assert e.param_extractors is None + assert e.include_all is True + assert e.extractor_id == "nullrun.tool_call.path" + assert e.extractor_version == "1" + + def test_explicit_map_overrides_include_all(self) -> None: + """Explicit ``{rule_param: arg_name}`` map wins over + ``include_all``. Operators use this when the rule name + diverges from the function arg name. + """ + e = tool_params({"user_id": "uid"}) + assert e.param_extractors == {"user_id": "uid"} + # ``include_all`` is irrelevant when param_extractors is + # set; the extractor ignores it. + assert e.include_all is True + + def test_mutual_exclusion_raises_at_construct_time(self) -> None: + """Setting both ``param_extractors`` and + ``include_all=False`` is almost certainly a typo. Fail + at decorator-application time rather than silently + dropping rules at run time. + """ + with pytest.raises(ValueError) as exc_info: + tool_params({"a": "b"}, include_all=False) + assert "mutually exclusive" in str(exc_info.value) + + +# --------------------------------------------------------------------------- +# 2. Extraction tests (runtime fixture, no @sensitive decorator) +# --------------------------------------------------------------------------- + + +class TestToolParamsExtraction: + def test_include_all_true_captures_every_kwarg( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """``include_all=True`` (default): every kwarg lands on the + wire under its own name, regardless of how many args the + function has or what their types are. + """ + + def delete_user(user_id: int, force: bool = False) -> None: + pass + + ext = tool_params(include_all=True) + fn = delete_user + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + _enforce_sensitive_tool(captured_runtime, fn, (), {"user_id": 42, "force": True}) + + kwargs = captured_payload.last_kwargs + assert kwargs is not None + assert "business_impact" in kwargs + assert "action_digest" in kwargs + + impact = kwargs["business_impact"] + assert impact["kind"] == KIND_TOOL_CALL + assert impact["tool_name"] == "delete_user" + assert impact["params"] == {"user_id": 42, "force": True} + assert impact["extractor_id"] == "nullrun.tool_call.path" + assert impact["extractor_version"] == "1" + + def test_explicit_map_only_captures_listed_kwargs( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """``{rule_param: arg_name}`` map: only the listed args are + captured; everything else is dropped. The rule param name + (key) and the function arg name (value) may differ. + """ + + def delete_user(uid: int, force: bool = False) -> None: + pass + + ext = tool_params({"user_id": "uid", "force": "force"}) + fn = delete_user + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + _enforce_sensitive_tool( + captured_runtime, fn, (), {"uid": 42, "force": True, "extra": "ignored"} + ) + + impact = captured_payload.last_kwargs["business_impact"] + # Only the mapped keys land on the wire, with the rule's + # chosen name (user_id, force -- not "uid" or "extra"). + assert impact["params"] == {"user_id": 42, "force": True} + assert "extra" not in impact["params"] + assert "uid" not in impact["params"] + + def test_include_all_false_with_no_map_yields_empty_params( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """``include_all=False`` and no ``param_extractors``: the + wire shape is ``kind: tool_call`` with ``params: {}``. + Rare, but the documented "empty args" path -- a tool with + no args is still eligible for ToolCall-kind Approval + Rules. + """ + + def list_accounts() -> None: + pass + + # Constructor rejects (param_extractors=None, include_all=False), + # so we build the extractor directly. + ext = ToolParamsExtractor(include_all=False) + fn = list_accounts + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + _enforce_sensitive_tool(captured_runtime, fn, (), {}) + + impact = captured_payload.last_kwargs["business_impact"] + assert impact["kind"] == KIND_TOOL_CALL + assert impact["tool_name"] == "list_accounts" + assert impact["params"] == {} + + def test_pii_masked_sentinel_is_dropped( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """PII-masked values (literal ``"***"`` string) are + filtered out before the wire. The operator would never + see the real value, so shipping the sentinel would never + match a real rule -- it's dead weight on the wire. + + The decorator wrapper masks PAN/password values to + ``"***"`` via ``_safe_kwargs`` BEFORE the extractor sees + them; this test simulates that pre-masked state. + """ + + def charge_card(pan: str, amount: int) -> None: + pass + + ext = tool_params(include_all=True) + fn = charge_card + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + # pan was masked by the decorator's _safe_kwargs layer + # before reaching the extractor. + _enforce_sensitive_tool(captured_runtime, fn, (), {"pan": "***", "amount": 5000}) + + impact = captured_payload.last_kwargs["business_impact"] + assert "pan" not in impact["params"], ( + "PII-masked sentinel '***' leaked to the wire; " + "operators would see a placeholder they cannot match" + ) + # amount is non-PII and survives masking, so it must be on + # the wire. + assert impact["params"]["amount"] == 5000 + + def test_float_arg_is_silently_dropped( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """Phase 1 filter-not-block: a kwarg whose type is not + JSON-roundtrippable (``float``) is silently dropped from + the wire payload rather than failing the pre-check. + + Why filter rather than fail: a function with a mixed + signature (``set_rate(rate: float, count: int)``) is + still useful -- the ``count`` arg is wire-safe and should + reach the operator. A wholesale block would force every + user with a single ``float`` kwarg to migrate to ``str`` + just to keep their other args matched against rules. + + The strict-mode alternative (explicit ``param_extractors`` + listing only the JSON-safe keys) is documented but + opt-in: bare ``@sensitive`` drops ``float`` silently. + """ + + def set_rate(rate: float, count: int) -> None: + pass + + ext = tool_params(include_all=True) + fn = set_rate + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + # Must NOT raise: float is filtered, int is captured. + _enforce_sensitive_tool(captured_runtime, fn, (), {"rate": 1.5, "count": 7}) + + impact = captured_payload.last_kwargs["business_impact"] + assert "rate" not in impact["params"], ( + "float value leaked to the wire despite _safe_for_wire filter" + ) + assert impact["params"]["count"] == 7, ( + "JSON-safe kwarg was incorrectly filtered alongside the float" + ) + + def test_unsupported_type_is_silently_filtered_in_explicit_map( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """``param_extractors`` mode: an explicit {rule: arg} + map whose arg value is JSON-unsafe (``float``) drops + that specific arg silently rather than failing the + whole pre-check. The other args still ship. + + Why filter rather than fail: the explicit map is a + one-to-one rename between function-arg and rule-param. + If a particular rename pair turns out to be + wire-incompatible, the operator can rename the rule + param (``{rate_str: "rate"}``) and stringify the value + before calling the tool. Failing the whole call would + punish the JSON-safe args. + """ + + def set_rate(rate: float, count: int) -> None: + pass + + ext = tool_params({"rate": "rate", "count": "count"}) + fn = set_rate + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + _enforce_sensitive_tool(captured_runtime, fn, (), {"rate": 1.5, "count": 7}) + + impact = captured_payload.last_kwargs["business_impact"] + # rate was filtered; count survived. + assert "rate" not in impact["params"] + assert impact["params"]["count"] == 7 + + def test_action_digest_matches_sdk_computation( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """The wire ``action_digest`` MUST equal the SDK's own + computation byte-for-byte. Otherwise the backend's + digest-bound approval row rejects every legitimate + post-approval re-check. + """ + + def delete_user(user_id: int, force: bool = False) -> None: + pass + + ext = tool_params(include_all=True) + fn = delete_user + fn._nullrun_extractor = ext + _register_tool(captured_runtime, fn) + + _enforce_sensitive_tool(captured_runtime, fn, (), {"user_id": 42, "force": True}) + + kwargs = captured_payload.last_kwargs + impact = BusinessImpact.tool_call( + tool_name="delete_user", + params={"user_id": 42, "force": True}, + ) + expected_digest = compute_action_digest(impact) + assert kwargs["action_digest"] == expected_digest + + +# --------------------------------------------------------------------------- +# 3. Auto-attach tests (the production-critical wiring) +# --------------------------------------------------------------------------- + + +class TestAutoAttachOnBareSensitive: + """Verify ``_do_sensitive_register`` stamps a + ``ToolParamsExtractor(include_all=True)`` on every bare + ``@sensitive`` tool that doesn't already carry an explicit + extractor. + + This is the behavior the user asked for: ``@sensitive`` + (no impact=...) is sufficient -- no extra ``@protect`` + decorator change, no extra decorator argument required. + """ + + def test_bare_function_gets_toolparams_extractor( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """After ``_do_sensitive_register`` runs, a bare function + carries ``_nullrun_extractor = ToolParamsExtractor(...)``, + and the wire payload carries ``kind: tool_call`` with + all kwargs as ``params``. + """ + + def delete_user(user_id: int, force: bool = False) -> None: + pass + + # No pre-existing extractor. + assert getattr(delete_user, "_nullrun_extractor", None) is None + + # Run the registration path that ``@sensitive`` would + # take at decoration time. + _do_sensitive_register(delete_user) + + ext = getattr(delete_user, "_nullrun_extractor") + assert isinstance(ext, ToolParamsExtractor), ( + f"bare @sensitive should auto-attach ToolParamsExtractor, got {type(ext).__name__}" + ) + assert ext.include_all is True + assert ext.param_extractors is None + + # Verify the wire payload uses the auto-attached extractor. + _enforce_sensitive_tool(captured_runtime, delete_user, (), {"user_id": 42, "force": True}) + impact = captured_payload.last_kwargs["business_impact"] + assert impact["kind"] == KIND_TOOL_CALL + assert impact["params"] == {"user_id": 42, "force": True} + + def test_explicit_money_extractor_is_not_overwritten( + self, captured_payload: _PayloadCapture, captured_runtime: NullRunRuntime + ) -> None: + """``@sensitive(impact=money_outflow(...))`` already + stamps ``MoneyImpactExtractor``. The auto-attach path + MUST NOT overwrite the explicit extractor with the + default ``ToolParamsExtractor``. Money semantics win. + """ + + def refund_customer(amount_cents: int, customer_id: str = "c-1") -> None: + pass + + # Stamp the explicit money extractor (what + # ``@sensitive(impact=money_outflow(...))`` does at + # decoration time). + money_ext = money_outflow(argument="amount_cents") + _stamp_extractor_on_innermost(refund_customer, money_ext) + + # Run the registration path -- must NOT replace + # money_ext with a ToolParamsExtractor. + _do_sensitive_register(refund_customer) + + ext = getattr(refund_customer, "_nullrun_extractor") + assert ext is money_ext, ( + "explicit money extractor was overwritten by " + "auto-attach -- this would silently regress the " + "MoneyImpact contract" + ) + assert isinstance(ext, MoneyImpactExtractor) + + # Verify the wire payload still uses money semantics. + _enforce_sensitive_tool(captured_runtime, refund_customer, (5000,), {"customer_id": "c-1"}) + impact = captured_payload.last_kwargs["business_impact"] + assert impact["kind"] == "money" + assert impact["amount_minor"] == 5000 + assert impact["currency"] == "USD" + + +# --------------------------------------------------------------------------- +# 4. Pydantic-style shape pin (pure unit, no runtime) +# --------------------------------------------------------------------------- + + +class TestToolCallParamsShape: + """Pin the SDK-side ``ToolCallParams`` shape against the backend + ``BusinessImpact::ToolCall(ToolCallParams)`` contract at + ``backend/src/proxy/gate/business_impact.rs:62-307``. + + Drift here is a P0 -- the backend would reject every + ToolCall-kind impact on the wire. + """ + + def test_to_wire_dict_shape(self) -> None: + p = ToolCallParams( + tool_name="delete_user", + params={"user_id": 42, "force": True}, + ) + d = p.to_wire_dict() + assert d == { + "kind": KIND_TOOL_CALL, + "tool_name": "delete_user", + "params": {"user_id": 42, "force": True}, + "extractor_id": "nullrun.tool_call.path", + "extractor_version": "1", + } + + def test_validate_rejects_empty_tool_name(self) -> None: + p = ToolCallParams(tool_name="", params={}) + with pytest.raises(ValueError) as exc_info: + p.validate() + assert "non-empty" in str(exc_info.value) + + def test_validate_rejects_overlong_tool_name(self) -> None: + p = ToolCallParams(tool_name="x" * 129, params={}) + with pytest.raises(ValueError) as exc_info: + p.validate() + assert "exceeds max 128" in str(exc_info.value) + + def test_validate_rejects_overlong_param_name(self) -> None: + long_key = "x" * (TOOL_PARAMETERS_MAX_PARAM_NAME + 1) + p = ToolCallParams(tool_name="x", params={long_key: 1}) + with pytest.raises(ValueError) as exc_info: + p.validate() + assert "key length" in str(exc_info.value) + + def test_validate_rejects_float_param_value(self) -> None: + p = ToolCallParams(tool_name="x", params={"rate": 1.5}) + with pytest.raises(ValueError) as exc_info: + p.validate() + assert "float" in str(exc_info.value) + + def test_validate_accepts_all_json_kinds(self) -> None: + # Boundary check: every JSON kind (null/bool/int/str/ + # list/dict) survives validate. Recursive validation + # reaches nested structures. + p = ToolCallParams( + tool_name="x", + params={ + "a": None, + "b": True, + "c": 42, + "d": "hello", + "e": [1, 2, "three", {"nested": True}], + "f": {"deep": {"deeper": [None, False]}}, + }, + ) + # Must not raise. + p.validate() + + def test_business_impact_kind_dispatch(self) -> None: + """``BusinessImpact.kind`` discriminates Money vs ToolCall. + A round-trip through ``to_wire_dict`` must preserve the + kind discriminator so the backend's + ``serde(tag = "kind")`` picks the right variant. + """ + m = BusinessImpact.money("outflow", 1000, "USD") + assert m.kind == "money" + assert m.to_wire_dict()["kind"] == "money" + + t = BusinessImpact.tool_call(tool_name="x", params={"y": 1}) + assert t.kind == KIND_TOOL_CALL + assert t.to_wire_dict()["kind"] == KIND_TOOL_CALL + + +# --------------------------------------------------------------------------- +# 5. Regression: explicit-extractor-vs-auto-attach priority (Phase 1 / MVP 1.1) +# --------------------------------------------------------------------------- +# +# Bug found via ad-hoc verification after the initial auto-attach +# commit (40d391a): the auto-attach path called +# ``getattr(fn, "_nullrun_extractor", None)`` on the @protect +# wrapper. The explicit extractor (set by ``@sensitive(impact=...)`` +# factory form) lives on the BARE function -- the wrapper does NOT +# carry the attribute -- so the check returned None and the +# auto-attach path silently overwrote the user's explicit map with +# ``ToolParamsExtractor(include_all=True)``. Result: ``impact= +# tool_params({"delete_force": "force"})`` looked like it took +# effect at decoration time but the wire payload used the default +# ``{delete_force: }`` mapping -- silent param-drop. +# +# The fix walks the ``__wrapped__`` chain in +# ``_do_sensitive_register``. The regression tests below pin both +# the bare case and the explicit-map case. + + +class TestAutoAttachChainWalk: + """Pin the ``_do_sensitive_register`` chain walk so future + decorator reordering does not silently regress the + explicit-extractor priority. + """ + + def test_bare_sensitive_chain_walk_attaches_default( + self, captured_runtime: NullRunRuntime + ) -> None: + """Bare ``@sensitive`` (no impact=...) walks the chain + and finds NO extractor, so auto-attach stamps the default. + """ + + def tool_fn(user_id: int) -> None: + pass + + assert _find_extractor_in_chain(tool_fn) is None, ( + "sanity: bare function should not have an extractor" + ) + + _do_sensitive_register(tool_fn) + + ext = _find_extractor_in_chain(tool_fn) + assert ext is not None + assert isinstance(ext, ToolParamsExtractor) + assert ext.include_all is True + + def test_explicit_tool_params_chain_walk_preserves_map( + self, captured_runtime: NullRunRuntime + ) -> None: + """``@sensitive(impact=tool_params({...}))`` stamped on the + bare function MUST survive ``_do_sensitive_register``. + + Pre-fix this silently overwrote the explicit extractor + with the auto-attach default ``ToolParamsExtractor( + include_all=True)`` because ``getattr(wrapper, + "_nullrun_extractor", None)`` returned None even though + the bare function carried the attribute. + """ + + def tool_fn(force: bool) -> None: + pass + + # Simulate the @sensitive(impact=tool_params({...})) + # factory form stamping the explicit extractor on the + # bare function via ``_stamp_extractor_on_innermost``. + explicit = tool_params({"delete_force": "force"}) + _stamp_extractor_on_innermost(tool_fn, explicit) + + # Now run the registration path -- the auto-attach MUST + # see the explicit extractor and skip the default. + _do_sensitive_register(tool_fn) + + ext = _find_extractor_in_chain(tool_fn) + assert ext is explicit, ( + "explicit tool_params map was overwritten by " + "auto-attach default -- this is the regression fixed " + "in the chain-walk patch" + ) + assert ext.param_extractors == {"delete_force": "force"} + assert ext.include_all is True + + def test_explicit_money_outflow_chain_walk_preserved( + self, captured_runtime: NullRunRuntime + ) -> None: + """``@sensitive(impact=money_outflow(...))`` also survives + the auto-attach path (the original Phase 1 / MVP 1.0 + money variant must NOT be overwritten by the Tier 2 + auto-attach). + """ + + def tool_fn(amount_cents: int) -> None: + pass + + explicit = money_outflow(argument="amount_cents") + _stamp_extractor_on_innermost(tool_fn, explicit) + + _do_sensitive_register(tool_fn) + + ext = _find_extractor_in_chain(tool_fn) + assert isinstance(ext, MoneyImpactExtractor), ( + f"explicit MoneyImpactExtractor was overwritten by " + f"auto-attach default; got {type(ext).__name__}" + ) + + def test_chain_walk_does_not_loop_on_circular_wraps( + self, captured_runtime: NullRunRuntime + ) -> None: + """Defensive: a pathological ``__wrapped__`` cycle must + not hang ``_find_extractor_in_chain``. We construct a + 3-call cycle and verify the walk returns None within + the bounded hop count. + """ + + # Build a self-referential __wrapped__ cycle. + class Cycle: + def __init__(self) -> None: + self._attr = "marker" + + a = Cycle() + a.__wrapped__ = a # direct self-cycle + # The walk must return None and not hang. + assert _find_extractor_in_chain(a) is None + # And a longer cycle: a -> b -> a -> b ... + b = Cycle() + a.__wrapped__ = b + b.__wrapped__ = a + assert _find_extractor_in_chain(a) is None