Skip to content

Commit 99cffb9

Browse files
committed
fix: enforce max_script_bytes, fix decorator silent skip, rm flag parsing, tee dead branch
1. Enforce max_script_bytes: single-line giant payloads with no newlines are now caught by a byte-size check alongside the existing line-count check. Both trigger DENY with a blocklist pre-scan. 2. Decorator silent skip: @safety_wrapper now logs a warning when the script_arg_name is not found in kwargs / positional dict args, so callers can detect missing protection. 3. _check_blocklist_override now receives script_type — Python string-literal stripping is skipped for Bash to avoid false negatives on cat '/etc/shadow' etc. 4. rm flag detection: parse short flags character-by-character instead of substring matching (rm -i no longer falsely triggers rm -rf). 5. tee dead branch: tee writes to sensitive paths (tee /etc/shadow, tee ~/.aws/credentials) are now detected. 6. Evidence truncation: <truncated:N> now reports len-300 consistently. 7. Redundant ternary: Decision.DENY if x else Decision.DENY → Decision.DENY.
1 parent 0305e68 commit 99cffb9

3 files changed

Lines changed: 91 additions & 31 deletions

File tree

trpc_agent_sdk/tools/safety/_bash_scanner.py

Lines changed: 38 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -328,8 +328,25 @@ def _scan_lines(self) -> None:
328328
},
329329
))
330330
break
331-
elif cmd_lower in _FILE_WRITE_COMMANDS and cmd_lower == "dd":
331+
elif cmd_lower == "dd":
332332
self._check_dd(line_no, raw_line, args)
333+
elif cmd_lower == "tee":
334+
# tee writes to files; check for sensitive paths in args
335+
for pa in args:
336+
if pa and not pa.startswith("-") and (_is_sensitive_path(pa) or pa.startswith("/dev/sd")):
337+
self._findings.append(
338+
BashScanFinding(
339+
kind="command",
340+
command="tee",
341+
args=args_str,
342+
line_number=line_no,
343+
evidence=raw_line.strip()[:300],
344+
extra={
345+
"risk": "sensitive_file_write",
346+
"path": pa
347+
},
348+
))
349+
break
333350

334351
# ------------------------------------------------------------------
335352
# Specific checks
@@ -338,17 +355,25 @@ def _scan_lines(self) -> None:
338355
def _check_rm(self, line_no: int, raw_line: str, tokens: List[str], args: List[str]) -> None:
339356
"""Check for recursive-delete. Flags both ``rm -rf`` and ``rm -r -f``."""
340357
all_tokens = set(tokens)
341-
# Handles combined flags: -rf, -fr, -r, -R, --recursive
342-
has_r = any(("r" in t.replace("-", "").lower() and not t.startswith("--")) or t in ("-r", "-R", "--recursive")
343-
for t in tokens if t.startswith("-"))
344-
# Handles combined flags: -rf, -fr, -f, --force
345-
has_f = any(("f" in t.replace("-", "").lower() and not t.startswith("--")) or t in ("-f", "--force")
346-
for t in tokens if t.startswith("-"))
347-
348-
# Check recursive via long option
349-
has_recursive = "--recursive" in all_tokens
350-
351-
if (has_r or has_recursive) and has_f:
358+
# Parse short flags character-by-character (avoids substring false
359+
# positives: "-i" has no "r", "-v" has no "f", etc.).
360+
short_flags: set[str] = set()
361+
has_long_recursive = False
362+
has_long_force = False
363+
for t in tokens:
364+
if t.startswith("--"):
365+
if t == "--recursive":
366+
has_long_recursive = True
367+
if t == "--force":
368+
has_long_force = True
369+
elif t.startswith("-") and not t.startswith("--"):
370+
for ch in t.lstrip("-"):
371+
short_flags.add(ch.lower())
372+
373+
has_r = "r" in short_flags or has_long_recursive
374+
has_f = "f" in short_flags or has_long_force
375+
376+
if has_r and has_f:
352377
target = args[-1] if args else "?"
353378
self._findings.append(
354379
BashScanFinding(
@@ -363,7 +388,7 @@ def _check_rm(self, line_no: int, raw_line: str, tokens: List[str], args: List[s
363388
"force": True
364389
},
365390
))
366-
elif has_r or has_recursive:
391+
elif has_r:
367392
target = args[-1] if args else "?"
368393
if target and (target.startswith("/") or _is_sensitive_path(target)):
369394
self._findings.append(

trpc_agent_sdk/tools/safety/_safety_wrapper.py

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,15 @@ async def my_tool_run(tool_context, args):
3131
from __future__ import annotations
3232

3333
import functools
34+
import logging
3435
from contextlib import asynccontextmanager
3536
from typing import Any
3637
from typing import AsyncIterator
3738
from typing import Callable
3839
from typing import Optional
3940

41+
_logger = logging.getLogger("trpc_agent_sdk.tools.safety.wrapper")
42+
4043
from ._audit import AuditLogger
4144
from ._policy import SafetyPolicy
4245
from ._policy import get_policy
@@ -217,7 +220,7 @@ def decorator(func: Callable) -> Callable:
217220

218221
@functools.wraps(func)
219222
async def async_wrapper(*args: Any, **kwargs: Any) -> Any:
220-
script = kwargs.get(script_arg_name)
223+
script: Optional[str] = kwargs.get(script_arg_name)
221224
if script is None:
222225
# Try to find it in positional args (e.g. tool_context, args)
223226
for arg in args:
@@ -226,18 +229,32 @@ async def async_wrapper(*args: Any, **kwargs: Any) -> Any:
226229
break
227230
if script and isinstance(script, str):
228231
wrapper_inst.check(script)
232+
elif script is not None and not isinstance(script, str):
233+
_logger.warning("safety_wrapper: '%s' value is not a string (type=%s) — scan skipped.", script_arg_name,
234+
type(script).__name__)
235+
else:
236+
_logger.warning(
237+
"safety_wrapper: could not find '%s' in kwargs or positional dict args "
238+
"for %s — scan skipped.", script_arg_name, func.__name__)
229239
return await func(*args, **kwargs)
230240

231241
@functools.wraps(func)
232242
def sync_wrapper(*args: Any, **kwargs: Any) -> Any:
233-
script = kwargs.get(script_arg_name)
243+
script: Optional[str] = kwargs.get(script_arg_name)
234244
if script is None:
235245
for arg in args:
236246
if isinstance(arg, dict) and script_arg_name in arg:
237247
script = arg[script_arg_name]
238248
break
239249
if script and isinstance(script, str):
240250
wrapper_inst.check(script)
251+
elif script is not None and not isinstance(script, str):
252+
_logger.warning("safety_wrapper: '%s' value is not a string (type=%s) — scan skipped.", script_arg_name,
253+
type(script).__name__)
254+
else:
255+
_logger.warning(
256+
"safety_wrapper: could not find '%s' in kwargs or positional dict args "
257+
"for %s — scan skipped.", script_arg_name, func.__name__)
241258
return func(*args, **kwargs)
242259

243260
import asyncio

trpc_agent_sdk/tools/safety/_scanner.py

Lines changed: 34 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -103,14 +103,23 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
103103
script = script + "\n" + args_text
104104

105105
script_lines = script.count("\n") + (1 if script else 0)
106+
script_bytes = len(script.encode("utf-8"))
106107

107108
# ══════════════════════════════════════════════════════════════
108-
# EARLY RETURN: script too large → skip expensive scanning
109-
# BUT still run the lightweight blocklist-pattern check first.
110-
# Without this an attacker can pad a malicious script with empty
111-
# lines / comments past max_script_lines and bypass all detection.
109+
# EARLY RETURN: script too large (lines OR bytes) → skip
110+
# expensive scanning. Runs a lightweight blocklist-pattern
111+
# pre-check first so that an attacker cannot pad a malicious
112+
# script past the limit and bypass all detection.
112113
# ══════════════════════════════════════════════════════════════
114+
oversized = script_lines > self._policy.max_script_lines
115+
oversized_reason = ""
113116
if script_lines > self._policy.max_script_lines:
117+
oversized_reason = (f"Script is {script_lines} lines (max {self._policy.max_script_lines})")
118+
elif script_bytes > self._policy.max_script_bytes:
119+
oversized = True
120+
oversized_reason = (f"Script is {script_bytes} bytes (max {self._policy.max_script_bytes})")
121+
122+
if oversized:
114123
# Fast pre-check: scan blocklist patterns against the full text
115124
blocklist_hit = None
116125
for pattern in self._policy.blocklist_patterns:
@@ -126,9 +135,9 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
126135
rule_id="GLOBAL-001",
127136
category=RiskCategory.RESOURCE_ABUSE,
128137
risk_level=RiskLevel.MEDIUM,
129-
evidence=f"Script is {script_lines} lines (max {self._policy.max_script_lines})",
130-
message="Script exceeds maximum line count.",
131-
recommendation="Split the script or increase max_script_lines in policy.",
138+
evidence=oversized_reason,
139+
message="Script exceeds maximum size limit.",
140+
recommendation="Split the script or increase the limit in policy.",
132141
line_number=0,
133142
matched_pattern="",
134143
)
@@ -152,10 +161,10 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
152161
tool_name=scan_input.tool_name,
153162
script_type=scan_input.script_type,
154163
script_size_lines=script_lines,
155-
decision=Decision.DENY if blocklist_hit else Decision.DENY,
164+
decision=Decision.DENY,
156165
risk_level=RiskLevel.CRITICAL if blocklist_hit else RiskLevel.HIGH,
157166
findings=oversized_findings,
158-
summary=f"Script is too large: {script_lines} lines (max {self._policy.max_script_lines})." +
167+
summary=f"{oversized_reason}." +
159168
(" Blocklist pattern matched — denied." if blocklist_hit else " Denied for safety."),
160169
scan_duration_ms=round(duration_ms, 2),
161170
policy_version=self._policy.content_hash,
@@ -214,7 +223,7 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
214223

215224
# Apply blocklist override — blocklist patterns always → deny
216225
if decision != Decision.DENY:
217-
decision = self._check_blocklist_override(script, decision)
226+
decision = self._check_blocklist_override(script, decision, scan_input.script_type)
218227

219228
# Apply allow-pattern override — allow patterns → allow
220229
# Only upgrades NEEDS_HUMAN_REVIEW; never overrides DENY (blocklist wins).
@@ -745,12 +754,17 @@ def _detect_type(script: str) -> ScriptType:
745754
return ScriptType.BASH
746755
return ScriptType.UNKNOWN
747756

748-
def _check_blocklist_override(self, script: str, current_decision: Decision) -> Decision:
757+
def _check_blocklist_override(self,
758+
script: str,
759+
current_decision: Decision,
760+
script_type: ScriptType = ScriptType.UNKNOWN) -> Decision:
749761
"""If any blocklist pattern matches, escalate to DENY.
750762
751763
Per-line matching is used so that patterns appearing inside
752764
``echo`` / ``printf`` string literals do not trigger false
753-
positives.
765+
positives. Python string-literal stripping is only applied for
766+
Python scripts — applying it to Bash would turn ``cat
767+
'/etc/shadow'`` into a false negative.
754768
"""
755769
for pattern in self._policy.blocklist_patterns:
756770
try:
@@ -762,9 +776,13 @@ def _check_blocklist_override(self, script: str, current_decision: Decision) ->
762776
stripped = line.lstrip()
763777
if stripped.startswith("#"):
764778
continue
765-
# Strip Python string-literal content so that patterns
766-
# like ``rm -rf /`` inside ``r'...'`` don't match.
767-
search_line = _strip_python_comment_line(line)
779+
# Strip Python string-literal content only for Python
780+
# scripts so that Bash single-quoted paths like
781+
# cat '/etc/shadow' are not masked.
782+
if script_type in (ScriptType.PYTHON, ScriptType.UNKNOWN):
783+
search_line = _strip_python_comment_line(line)
784+
else:
785+
search_line = line
768786
if pat.search(search_line):
769787
# Skip if the match is inside an echo/printf string literal
770788
if _is_in_echo_string(line, pattern):
@@ -824,7 +842,7 @@ def _redact_evidence(self, findings: list[SafetyFinding]) -> list[SafetyFinding]
824842
f.evidence = re.sub(pat, repl, f.evidence, flags=re.IGNORECASE)
825843
# Truncate very long evidence to prevent data leakage
826844
if len(f.evidence) > 320:
827-
f.evidence = f.evidence[:300] + f"...<truncated:{len(f.evidence) - 320}>"
845+
f.evidence = f.evidence[:300] + f"...<truncated:{len(f.evidence) - 300}>"
828846
return findings
829847

830848

0 commit comments

Comments
 (0)