Skip to content

Commit 27397ea

Browse files
committed
fix: P0 security/stability hardening bundle
Closes the P0/P1/P2/P3 issues from the security review (plan §10/§11.4). Security / PCI-DSS / GDPR - P0-1: Mask positional PII in `_enforce_sensitive_tool` by introspecting the wrapped function's signature and applying `SENSITIVE_ARG_KEYS` to positional params. Pre-fix, `charge("4111-…-1111", 50)` forwarded the PAN into `/execute` and the audit log. - P0-6 / P3-3: `_safe_repr` now redacts BEFORE truncating. The pre-fix order truncated first, so `details={…}` past position 50 leaked verbatim. `_safe_repr` is now the single source of truth for the redact-then-truncate flow. Cost-audit / reliability - P0-3: Bounded chunked reads on the sync + async httpx transports (`MAX_RESPONSE_BYTES`, default 16 MiB, `NULLRUN_MAX_RESPONSE_BYTES` env override). Above the cap, tracking is skipped and `_coverage_streaming_skipped` is incremented. Replaces the `response.read()` / `await response.aread()` unbounded buffer that held entire LLM streaming bodies in memory. - P0-4: `_do_flush_locked` re-queue on CB OPEN now drops the NEWEST non-critical events instead of the oldest. The oldest events (incident start, billing-period start) are exactly what a billing investigator needs; losing them silently broke monthly rollups. Control-plane events (`state_change`, `kill_received`, `policy_invalidated`, `key_rotated`) are preserved unconditionally so the dashboard KILL switch lands even under sustained backend outage. Identity - S-8 / P2-4: `agent()` now emits `str(uuid.uuid4())` (with dashes). Pre-fix the format was `f"agent-{uuid.uuid4().hex}"` — 32 hex chars, no dashes — and backend UUID-typed columns dropped these to NULL on insert. User-supplied names are still preserved verbatim. - §7.2 #16: `workflow()` context manager now resets `span_id` (not only `workflow_id` / `trace_id`) so nested `with span()` blocks don't leave the inner span_id visible inside the workflow scope. Resource leaks - S-9: `_active_runs` on `NullRunCallback` is now an `OrderedDict` capped at 4096 with FIFO eviction. Pre-fix the dict grew unbounded when `on_chain_end` did not fire (some LangChain versions short-circuit the end hook on chain-body errors). - S-10: WebSocket reconnect loop is now capped at 10 consecutive failures, then falls back to HTTP-poll. Pre-fix the loop ran forever when the backend was permanently down, leaking the WS thread. Transport - §7.2 #6: Separate `hmac_verify_expired_total` counter so SRE can distinguish clock-skew (NTP drift) from forged packets. Mirrored in both the HTTP and WebSocket verify paths. - §7.2 #35: `CircuitBreaker.call` now dispatches the OPEN→HALF_OPEN jitter through `_maybe_apply_open_jitter_sync` / `_maybe_apply_open_jitter_async`. Pre-fix the jitter used `time.sleep` before dispatching to async, which blocked the caller's event loop on every transition. - P2-1: `_coverage_seen` now bumps in the httpx path (sync + async). Pre-fix the counter was only bumped by the `requests` transport, so the dashboard's coverage view was empty for the dominant OpenAI / Anthropic / Gemini / Mistral / Cohere traffic. - P2-3: `is_sensitive_tool` match is case-insensitive. Pre-fix `"stripe.charge"` did not match `"Stripe.Charge"`, bypassing the sensitive gate. Concurrency - §7.2 #39: New `_tools_lock` guards every mutation of `_strict_mode_tools` / `_sensitive_tools`. Same lock guards the coverage-counter bump+prune sequence (§7.2 #33) so two threads can't both observe the dict at length 4095 and both grow it to 4097 before either prune lands. - §7.2 #47: New `_langchain_lock` / `_langgraph_lock` guard the patch sequences end-to-end. Pre-fix two threads racing through `auto_instrument` could both pass the early `_x_patched` check and double-wrap `BaseCallbackManager` / `Pregel`. - §7.2 #33: `_COVERAGE_CAP` (4096) bounds the per-host coverage dicts. Webhook delivery - P3-2: Exponential backoff (0.5s, 1s, 2s, 4s, 8s, 16s, 30s cap) replaces the previous linear schedule. Linear didn't back off fast enough under sustained outage — each KILL/PAUSE spawned its own delivery thread, producing 1000+ spinning threads hammering the dead endpoint. WAL crash-recovery - P1-5b: Atomic WAL writes (tmp + `fsync` + `os.replace`), 64 MiB rotation with `os.replace(wal, wal.1)`, replay drains both `wal.1` and `wal`. New `NULLRUN_WAL_PATH` / `NULLRUN_WAL_MAX_BYTES` env overrides for containers with `readOnlyRootFilesystem: true`. Tests 8 new regression test files (57 tests total): test_agent_id_uuid.py, test_args_pii_masked.py, test_streaming_oom_cap.py, test_lru_active_runs.py, test_reconnect_cap.py, test_coverage_seen_httpx.py, test_webhook_backoff.py, test_redact.py `test_buffer_invariants.py` extended with drop-newest + critical-event preservation cases. `test_release_polish.py` updated to pin the 5s cap on both the sync and async jitter helpers (post §7.2 #35 split). Full incident write-ups in CHANGELOG.md under the same P0/S/P tags.
1 parent ac6db5d commit 27397ea

21 files changed

Lines changed: 2076 additions & 181 deletions

CHANGELOG.md

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,81 @@ surface is unchanged. Aligns the SDK with the contracts in
265265

266266
### Fixed
267267

268+
- **P0-1 (PCI-DSS / GDPR): positional PII masking.** Sensitive tools
269+
called positionally (e.g. ``charge("4111-1111-1111-1111", 50)``) now
270+
mask positional args the same way kwargs already do, by introspecting
271+
the function signature with ``inspect.signature(fn)`` and applying
272+
``SENSITIVE_ARG_KEYS`` to the matching parameter name. Pre-fix the
273+
PAN at position 0 was forwarded as-is into ``/execute`` and landed
274+
in the audit log.
275+
- **P0-3 (OOM): streaming response memory cap.** Sync and async
276+
httpx transports now use bounded chunked reads capped at
277+
``MAX_RESPONSE_BYTES`` (16 MiB by default; ``NULLRUN_MAX_RESPONSE_BYTES``
278+
env var to override). When the cap is exceeded, tracking is skipped
279+
and ``_coverage_streaming_skipped`` is incremented so the dashboard
280+
sees which hosts are producing oversized responses. Pre-fix
281+
``response.read()`` / ``await response.aread()`` buffered the entire
282+
response body in memory — a 16+ MB allocation per streaming LLM
283+
call under load.
284+
- **P0-4 (cost-audit): drop-newest on buffer overflow.** The CB-OPEN
285+
re-queue path in ``Transport._do_flush_locked`` now drops the
286+
NEWEST non-critical events instead of the oldest. The oldest
287+
events (start-of-incident, start-of-billing-period) are exactly
288+
what a billing investigator needs to reconstruct — losing them
289+
silently broke monthly rollups. Control-plane events
290+
(``state_change`` / ``kill_received`` / ``policy_invalidated`` /
291+
``key_rotated``) are preserved regardless of position so the
292+
dashboard's KILL switch continues to land even under sustained
293+
backend outage.
294+
- **P0-6 + P3-3 (security): redact-before-truncate.** ``_safe_repr``
295+
now runs ``_strip_details_balanced`` on the FULL repr before
296+
truncating to ``max_len=50``. Pre-fix the truncate ran first, and
297+
if ``details={...}`` lived past position 50 in the original repr
298+
(common for httpx.HTTPError with a long URL), the redact pass
299+
saw nothing on the truncated slice and the raw payload leaked
300+
into ``span_end`` audit events.
301+
- **S-8 / P2-4: ``agent_id`` is now a real UUID with dashes.**
302+
``agent()`` context manager emits ``str(uuid.uuid4())`` (e.g.
303+
``95ca7c0b-8334-478a-af23-2788803ef3b8``) for auto-generated ids.
304+
Pre-fix the format was ``f"agent-{uuid.uuid4().hex}"`` — 32 hex
305+
chars with no dashes; backend UUID-typed columns silently
306+
dropped these to NULL on insert. User-supplied names are still
307+
preserved verbatim.
308+
- **S-9: LRU cap on ``NullRunCallback._active_runs``** (4096 entries,
309+
FIFO eviction with WARN log). Pre-fix this dict grew unbounded
310+
when ``on_chain_end`` did not fire (errors in the chain body
311+
short-circuited the end hook for some LangChain versions),
312+
leaking memory in long-running services.
313+
- **S-10: WebSocket reconnect max-attempts cap** (10 consecutive
314+
failures). Pre-fix the loop was unbounded (``while not
315+
self._closed:``) and leaked the WS thread forever when the
316+
backend was permanently down. After the cap the SDK falls back
317+
to HTTP-poll for control-plane state delivery.
318+
- **P2-1: ``_coverage_seen`` now bumps in the httpx path.**
319+
Pre-fix the counter was only incremented in the ``requests``
320+
path (``auto_requests.py:185``), so the dashboard's coverage
321+
view was empty for the dominant httpx traffic (every OpenAI /
322+
Anthropic / Gemini / Mistral / Cohere call). Now both sync and
323+
async httpx ``_emit`` bump the counter.
324+
- **P3-2: webhook delivery uses exponential backoff** (cap 30s).
325+
Pre-fix the schedule was linear (``0.5 * (attempt + 1)``); under
326+
sustained outage this produced a tight retry storm on the dead
327+
endpoint — each KILL/PAUSE spawned its own delivery thread.
328+
Post-fix the schedule is ``0.5 * 2**attempt`` capped at 30s:
329+
0.5s, 1.0s, 2.0s, 4.0s, 8.0s, 16.0s, 30.0s.
330+
331+
### Tests
332+
333+
Added regression tests for every item above (57 new tests across 9
334+
new test files: ``test_agent_id_uuid.py``, ``test_args_pii_masked.py``,
335+
``test_streaming_oom_cap.py``, ``test_lru_active_runs.py``,
336+
``test_reconnect_cap.py``, ``test_coverage_seen_httpx.py``,
337+
``test_webhook_backoff.py``, ``test_redact.py``; existing
338+
``test_buffer_invariants.py`` extended with drop-newest + critical-event
339+
preservation cases).
340+
341+
### Legacy
342+
268343
- **SDK silent runtime fallback removed** (FIX-4): `_get_or_create_runtime`
269344
in `nullrun.decorators` no longer wraps `NullRunRuntime.get_instance()`
270345
in a `try/except Exception` that rebuilds a no-arg `NullRunRuntime()`.

src/nullrun/actions.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,20 @@ def _deliver_webhook(self, webhook: WebhookConfig, payload: dict[str, Any]) -> N
372372
logger.warning("httpx not installed, cannot send webhook")
373373
return
374374

375+
# P3-2 (plan §10): exponential backoff between attempts with a
376+
# 30s cap. Pre-fix the schedule was linear (``0.5 * (attempt+1)``
377+
# → 0.5s, 1.0s, 1.5s, ...). Linear doesn't back off fast enough
378+
# when the destination is down — a transient outage produced
379+
# 100+ retries in seconds, and each KILL/PAUSE from the server
380+
# spawns its own delivery thread, so 1000 events/min generated
381+
# 1000 spinning daemon threads hammering the dead endpoint.
382+
#
383+
# Schedule: 0.5s, 1.0s, 2.0s, 4.0s, 8.0s, 16.0s, 30.0s (capped).
384+
# Total worst-case wait over 7 retries is ~62s — long enough to
385+
# ride out a brief blip, short enough that one stuck thread
386+
# doesn't block forever.
387+
_BACKOFF_BASE = 0.5
388+
_BACKOFF_CAP = 30.0
375389
for attempt in range(webhook.retries):
376390
try:
377391
response = httpx.post(
@@ -386,7 +400,8 @@ def _deliver_webhook(self, webhook: WebhookConfig, payload: dict[str, Any]) -> N
386400
except Exception as e:
387401
logger.warning(f"Webhook attempt {attempt + 1} failed: {e}")
388402
if attempt < webhook.retries - 1:
389-
time.sleep(0.5 * (attempt + 1))
403+
delay = min(_BACKOFF_BASE * (2 ** attempt), _BACKOFF_CAP)
404+
time.sleep(delay)
390405

391406
def stop_webhooks(self) -> None:
392407
"""Stop webhook delivery thread."""

src/nullrun/breaker/circuit_breaker.py

Lines changed: 60 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -251,50 +251,76 @@ def state(self) -> CBState:
251251
return self._state
252252

253253
def call(self, func: Callable[..., Any], *args, **kwargs) -> Any:
254-
"""Execute func through circuit breaker. Supports both sync and async functions."""
255-
254+
"""Execute func through circuit breaker. Supports both sync and async functions.
255+
256+
§7.2 #35: the pre-fix code did the OPEN→HALF_OPEN jitter
257+
via ``time.sleep`` here, BEFORE dispatching to
258+
``_call_sync`` / ``_call_async``. That meant an async
259+
caller invoking ``breaker.call(async_func, ...)`` from
260+
inside an event loop would block that loop on a sync
261+
sleep — turning every HALF_OPEN probe into a 0–5 second
262+
stall of the entire coroutine scheduler. The fix decides
263+
here whether jitter is needed and lets the dispatch path
264+
use ``time.sleep`` for sync callers and ``asyncio.sleep``
265+
for async ones.
266+
"""
256267
# Check global Redis state first - reject if another instance has it open
257268
if not self._global_state_allows_call():
258269
raise BreakerTransportError(
259270
f"Circuit breaker OPEN (global) -- service unavailable. "
260271
f"Retry in {self._recovery_timeout:.0f}s"
261272
)
262273

263-
# Add jitter before transitioning from OPEN to HALF_OPEN to prevent thundering herd
274+
# Decide whether jitter is needed; the actual sleep happens
275+
# in the dispatch path so it can be ``time.sleep`` for sync
276+
# callers and ``asyncio.sleep`` for async ones.
277+
needs_open_jitter = (
278+
self._state == CBState.OPEN
279+
and self._opened_at is not None
280+
and (time.monotonic() - self._opened_at) >= self._recovery_timeout
281+
)
282+
283+
# Check if func is a coroutine function (async) before
284+
# grabbing any locks — async callers need an awaitable.
285+
import inspect
286+
if inspect.iscoroutinefunction(func):
287+
return self._call_async(func, needs_open_jitter, *args, **kwargs)
288+
return self._call_sync(func, needs_open_jitter, *args, **kwargs)
289+
290+
def _maybe_apply_open_jitter_sync(self) -> None:
291+
"""Sync version of the OPEN→HALF_OPEN jitter. See §7.2 #35."""
264292
if self._state == CBState.OPEN and self._opened_at is not None:
265293
time_in_open = time.monotonic() - self._opened_at
266294
if time_in_open >= self._recovery_timeout:
267-
# Add random jitter (0-30 seconds) to prevent thundering herd
268-
# Phase 8: cap at 5s (was 30s). The previous value
269-
# blocked the caller's thread for up to 30s on
270-
# every OPEN->HALF_OPEN transition. 5s is plenty
271-
# to spread reconnects across workers.
295+
# Phase 8: cap at 5s (was 30s). 5s is plenty to
296+
# spread reconnects across workers.
272297
jitter = random.uniform(0, 5.0)
273298
time.sleep(jitter)
274299

275-
state = self.state
300+
async def _maybe_apply_open_jitter_async(self) -> None:
301+
"""Async version of the OPEN→HALF_OPEN jitter. Awaits
302+
instead of blocking the event loop. See §7.2 #35."""
303+
if self._state == CBState.OPEN and self._opened_at is not None:
304+
time_in_open = time.monotonic() - self._opened_at
305+
if time_in_open >= self._recovery_timeout:
306+
jitter = random.uniform(0, 5.0)
307+
await asyncio.sleep(jitter)
276308

309+
def _call_sync(self, func: Callable[..., Any], needs_open_jitter: bool, *args, **kwargs) -> Any:
310+
"""Execute sync func through circuit breaker."""
311+
if needs_open_jitter:
312+
self._maybe_apply_open_jitter_sync()
313+
state = self.state
277314
if state == CBState.OPEN:
278315
raise BreakerTransportError(
279316
f"Circuit breaker OPEN -- service unavailable. "
280317
f"Retry in {self._recovery_timeout:.0f}s"
281318
)
282-
283319
if state == CBState.HALF_OPEN:
284320
with self._lock:
285321
if self._half_open_calls >= self._half_open_max_calls:
286322
raise BreakerTransportError("Circuit breaker HALF_OPEN -- waiting")
287323
self._half_open_calls += 1
288-
289-
# Check if func is a coroutine function (async)
290-
import inspect
291-
if inspect.iscoroutinefunction(func):
292-
return self._call_async(func, *args, **kwargs)
293-
else:
294-
return self._call_sync(func, *args, **kwargs)
295-
296-
def _call_sync(self, func: Callable[..., Any], *args, **kwargs) -> Any:
297-
"""Execute sync func through circuit breaker."""
298324
try:
299325
result = func(*args, **kwargs)
300326
self._on_success()
@@ -303,8 +329,21 @@ def _call_sync(self, func: Callable[..., Any], *args, **kwargs) -> Any:
303329
self._on_failure()
304330
raise
305331

306-
async def _call_async(self, func: Callable[..., Any], *args, **kwargs) -> Any:
332+
async def _call_async(self, func: Callable[..., Any], needs_open_jitter: bool, *args, **kwargs) -> Any:
307333
"""Execute async func through circuit breaker."""
334+
if needs_open_jitter:
335+
await self._maybe_apply_open_jitter_async()
336+
state = self.state
337+
if state == CBState.OPEN:
338+
raise BreakerTransportError(
339+
f"Circuit breaker OPEN -- service unavailable. "
340+
f"Retry in {self._recovery_timeout:.0f}s"
341+
)
342+
if state == CBState.HALF_OPEN:
343+
with self._lock:
344+
if self._half_open_calls >= self._half_open_max_calls:
345+
raise BreakerTransportError("Circuit breaker HALF_OPEN -- waiting")
346+
self._half_open_calls += 1
308347
try:
309348
result = await func(*args, **kwargs)
310349
await self._on_success_async()

src/nullrun/context.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,17 +111,29 @@ def workflow(name: str | None = None) -> Generator[str, None, None]:
111111
# was inconsistent with the rest of the SDK's id generation.
112112
workflow_id = name or str(uuid.uuid4())
113113
trace_id = generate_trace_id()
114+
# §7.2 #16: a new workflow gets a fresh span_id too. The
115+
# pre-fix code only reset workflow_id and trace_id, so a
116+
# ``with span("inner"); with workflow("outer")`` block would
117+
# leave the inner span_id visible inside the workflow scope —
118+
# the span emitted by the workflow would carry the wrong
119+
# parent. We set a new span_id here so the audit log can
120+
# correctly nest the workflow's own span_start under the
121+
# workflow_id (rather than under some earlier span that
122+
# happened to be on the contextvar stack).
123+
span_id = generate_span_id()
114124

115125
# Save current values
116126
wf_token = _workflow_id_var.set(workflow_id)
117127
trace_token = _trace_id_var.set(trace_id)
128+
span_token = _span_id_var.set(span_id)
118129

119130
try:
120131
yield workflow_id
121132
finally:
122133
# Restore previous values
123134
_workflow_id_var.reset(wf_token)
124135
_trace_id_var.reset(trace_token)
136+
_span_id_var.reset(span_token)
125137

126138

127139
@contextmanager
@@ -168,7 +180,15 @@ def agent(name: str | None = None) -> Generator[str, None, None]:
168180
Yields:
169181
The agent_id string
170182
"""
171-
agent_id = name or f"agent-{uuid.uuid4().hex}"
183+
# P2-4 / S-8: emit a real UUID4 with dashes (matching
184+
# ``generate_trace_id`` / ``generate_span_id``). The previous
185+
# ``f"agent-{uuid.uuid4().hex}"`` format was 32 hex chars
186+
# without dashes; backend UUID-typed columns (cost_events.
187+
# agent_id, audit_log) silently dropped these to NULL on insert
188+
# (``Uuid::parse_str(...).ok()`` returned None). User-supplied
189+
# ``name`` is preserved verbatim so existing dashboards continue
190+
# to work for already-allocated agent ids.
191+
agent_id = name or str(uuid.uuid4())
172192
token = _agent_id_var.set(agent_id)
173193

174194
try:

src/nullrun/decorators.py

Lines changed: 74 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,8 +88,38 @@ def researcher(q):
8888

8989

9090
def _safe_repr(value: object, max_len: int = 50) -> str:
91-
"""Safe representation of an argument for logging."""
91+
"""Safe representation of an argument for logging.
92+
93+
P0-6 (plan §10): redaction happens BEFORE truncation, not after.
94+
Pre-fix the order was truncate-then-redact: ``_safe_repr`` cut the
95+
repr to 50 chars first, and ``_strip_details_balanced`` then tried
96+
to find ``details={...}`` in that 50-char slice. If ``details=``
97+
lived past position 50 (a common case — repr() of an HTTPError
98+
with a long URL places the dict payload well into the string), the
99+
substring was gone, the redact pass saw nothing, and the raw
100+
``details={...}`` payload leaked into the audit log.
101+
102+
Post-fix the order is redact-then-truncate: call
103+
``_strip_details_balanced`` first (which works on the full repr),
104+
then truncate. The cost is a single string scan over ``len(repr)``
105+
instead of ``len(repr[:50])`` — irrelevant for the 200-byte
106+
strings we actually pass through this code path.
107+
108+
P3-3 (plan §10): also consolidates the two-pass flow that
109+
previously lived as separate ``_safe_repr`` + ``_strip_details_balanced``
110+
calls — there are now two callers that compose them, and the
111+
invariant ``redact BEFORE truncate`` was being maintained by
112+
convention only. ``_safe_repr`` is now the single source of truth.
113+
"""
92114
r = repr(value)
115+
# Phase 1: redact ``details={...}`` substrings on the FULL repr.
116+
# Cheap (single linear scan over the string), and ensures the
117+
# ``details=`` substring is replaced before we potentially
118+
# truncate it away.
119+
r = _strip_details_balanced(r)
120+
# Phase 2: truncate to ``max_len`` so a giant repr doesn't bloat
121+
# span events. We append ``...<truncated>`` so consumers can
122+
# see the cut happened.
93123
if len(r) > max_len:
94124
return r[:max_len] + "...<truncated>"
95125
return r
@@ -103,6 +133,43 @@ def _safe_kwargs(kwargs: dict[str, Any]) -> dict[str, Any]:
103133
}
104134

105135

136+
def _safe_args(fn: Callable[..., Any], args: tuple[Any, ...]) -> list[Any]:
137+
"""Mask sensitive positional args (P0-1, plan §10).
138+
139+
Pre-fix only kwargs were masked via SENSITIVE_ARG_KEYS. A
140+
``def charge(card_number, amount)`` with positional call
141+
``charge("4111-1111-1111-1111", 50)`` would leak the PAN into the
142+
audit log. We now introspect ``fn``'s signature, bind the positional
143+
args to parameter names, and apply the same ``SENSITIVE_ARG_KEYS``
144+
mask that kwargs already use.
145+
146+
Extra positional args (``*args``) have no parameter name to key on —
147+
we still redact them with ``_safe_repr`` so we don't ship a full
148+
repr of an arbitrary object to the audit log, but we cannot tell
149+
them apart from benign primitives. This is the same posture as the
150+
kwargs branch (apply mask by name; otherwise best-effort repr).
151+
"""
152+
try:
153+
sig = inspect.signature(fn)
154+
except (TypeError, ValueError):
155+
# C-extension / built-in without a signature — fall back to
156+
# safe repr for every arg so we still don't leak raw
157+
# repr(value) of an arbitrary object.
158+
return [_safe_repr(a) for a in args]
159+
160+
bound_params = list(sig.parameters.items())[: len(args)]
161+
masked: list[Any] = []
162+
for (pname, _param), value in zip(bound_params, args):
163+
if pname.lower() in SENSITIVE_ARG_KEYS:
164+
masked.append("***")
165+
else:
166+
masked.append(_safe_repr(value))
167+
# Trailing *args have no name — best-effort safe repr.
168+
for value in args[len(bound_params):]:
169+
masked.append(_safe_repr(value))
170+
return masked
171+
172+
106173
# SEC-29: strip the `details={...}` payload from an exception's
107174
# string form before it lands in the span_end audit event.
108175
# Phase 3 replaced the previous one-level regex with a
@@ -496,6 +563,11 @@ def _enforce_sensitive_tool(
496563
if not runtime.is_sensitive_tool(fn.__name__):
497564
return
498565
masked = _safe_kwargs(kwargs)
566+
# P0-1: positional args are masked the same way as kwargs. Without
567+
# this, a sensitive tool called positionally (e.g.
568+
# ``charge("4111-1111-1111-1111", 50)``) would leak the PAN into
569+
# the /execute payload that lands in the audit log.
570+
masked_args = _safe_args(fn, args)
499571

500572
# ADR-008: prefer `on_transport_error` (raise classified
501573
# NullRunTransportError); fall back to legacy `fallback_mode` for
@@ -518,7 +590,7 @@ def _enforce_sensitive_tool(
518590
# uniformly.
519591
result = runtime.execute(
520592
fn.__name__,
521-
{"args": list(args), "kwargs": masked},
593+
{"args": masked_args, "kwargs": masked},
522594
on_transport_error="raise",
523595
)
524596
except NullRunBlockedException:

0 commit comments

Comments
 (0)