Skip to content

Commit 896e877

Browse files
committed
fix: safety_wrapper fail-closed by default (require_script=True)
1 parent 5c79d2f commit 896e877

2 files changed

Lines changed: 41 additions & 7 deletions

File tree

tests/test_tool_safety.py

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1873,16 +1873,27 @@ def sync_tool(*args, **kwargs):
18731873

18741874

18751875
def test_safety_wrapper_decorator_sync_no_script():
1876-
"""@safety_wrapper sync must skip scan when script is None."""
1876+
"""@safety_wrapper sync with require_script=False must skip scan and execute."""
18771877

1878-
@safety_wrapper(tool_name="sync_noscript_test", script_arg_name="code")
1878+
@safety_wrapper(tool_name="sync_noscript_test", script_arg_name="code", require_script=False)
18791879
def sync_tool(*args, **kwargs):
18801880
return "executed"
18811881

18821882
result = sync_tool(args={})
18831883
assert result == "executed"
18841884

18851885

1886+
def test_safety_wrapper_decorator_sync_fail_closed():
1887+
"""@safety_wrapper sync must raise RuntimeError when script arg is missing (fail-closed)."""
1888+
1889+
@safety_wrapper(tool_name="sync_failclosed_test", script_arg_name="code")
1890+
def sync_tool(*args, **kwargs):
1891+
return "executed"
1892+
1893+
with pytest.raises(RuntimeError, match="not found"):
1894+
sync_tool(args={})
1895+
1896+
18861897
# ==========================================================================
18871898
# Coverage gap tests — _telemetry.py
18881899
# ==========================================================================
@@ -1975,16 +1986,27 @@ def test_dependency_install_python():
19751986

19761987

19771988
async def test_safety_wrapper_decorator_async_no_script():
1978-
"""@safety_wrapper async must skip scan when script is None."""
1989+
"""@safety_wrapper async with require_script=False must skip scan and execute."""
19791990

1980-
@safety_wrapper(tool_name="async_noscript_test", script_arg_name="code")
1991+
@safety_wrapper(tool_name="async_noscript_test", script_arg_name="code", require_script=False)
19811992
async def async_tool(*args, **kwargs):
19821993
return "executed"
19831994

19841995
result = await async_tool(args={})
19851996
assert result == "executed"
19861997

19871998

1999+
async def test_safety_wrapper_decorator_async_fail_closed():
2000+
"""@safety_wrapper async must raise RuntimeError when script arg is missing (fail-closed)."""
2001+
2002+
@safety_wrapper(tool_name="async_failclosed_test", script_arg_name="code")
2003+
async def async_tool(*args, **kwargs):
2004+
return "executed"
2005+
2006+
with pytest.raises(RuntimeError, match="not found"):
2007+
await async_tool(args={})
2008+
2009+
19882010
def test_scanner_empty_script():
19892011
"""Scanner must handle empty script content without crashing."""
19902012
scanner = SafetyScanner()

trpc_agent_sdk/tools/safety/_safety_wrapper.py

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ def safety_wrapper(
189189
policy: Optional[SafetyPolicy] = None,
190190
audit_log_path: Optional[str] = None,
191191
raise_on_deny: bool = True,
192+
require_script: bool = True,
192193
):
193194
"""Decorator that applies safety checks before a function executes.
194195
@@ -201,6 +202,10 @@ def safety_wrapper(
201202
policy: Optional policy override.
202203
audit_log_path: Path to JSONL audit file.
203204
raise_on_deny: Raise ``SafetyDeniedError`` on DENY.
205+
require_script: If True (default), raise ``RuntimeError`` when
206+
*script_arg_name* is not found — fail-closed so that a
207+
misconfigured decorator does not silently allow execution
208+
without scanning.
204209
205210
Example::
206211
@@ -222,14 +227,17 @@ def decorator(func: Callable) -> Callable:
222227
async def async_wrapper(*args: Any, **kwargs: Any) -> Any:
223228
script: Optional[str] = kwargs.get(script_arg_name)
224229
if script is None:
225-
# Try to find it in positional args (e.g. tool_context, args)
226230
for arg in args:
227231
if isinstance(arg, dict) and script_arg_name in arg:
228232
script = arg[script_arg_name]
229233
break
230234
if script and isinstance(script, str):
231235
wrapper_inst.check(script)
232-
elif script is not None and not isinstance(script, str):
236+
elif require_script:
237+
raise RuntimeError(f"safety_wrapper: '{script_arg_name}' not found in kwargs or positional "
238+
f"dict args for {func.__name__}. Check the decorator configuration "
239+
f"or set require_script=False to skip scanning.")
240+
elif script is not None:
233241
_logger.warning("safety_wrapper: '%s' value is not a string (type=%s) — scan skipped.", script_arg_name,
234242
type(script).__name__)
235243
else:
@@ -248,7 +256,11 @@ def sync_wrapper(*args: Any, **kwargs: Any) -> Any:
248256
break
249257
if script and isinstance(script, str):
250258
wrapper_inst.check(script)
251-
elif script is not None and not isinstance(script, str):
259+
elif require_script:
260+
raise RuntimeError(f"safety_wrapper: '{script_arg_name}' not found in kwargs or positional "
261+
f"dict args for {func.__name__}. Check the decorator configuration "
262+
f"or set require_script=False to skip scanning.")
263+
elif script is not None:
252264
_logger.warning("safety_wrapper: '%s' value is not a string (type=%s) — scan skipped.", script_arg_name,
253265
type(script).__name__)
254266
else:

0 commit comments

Comments
 (0)