Skip to content

Commit 3f4731f

Browse files
committed
fix: echo bypass, VAR=val extraction, multi-cmd analysis
1 parent 88e78cf commit 3f4731f

4 files changed

Lines changed: 153 additions & 121 deletions

File tree

trpc_agent_sdk/tools/safety/_bash_scanner.py

Lines changed: 105 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -237,115 +237,118 @@ def _scan_lines(self) -> None:
237237
# Check pipes and background
238238
self._check_operators(line_no, raw_line, tokens)
239239

240-
# Command analysis
241-
cmd = tokens[0]
242-
# Skip variable assignments (FOO=bar cmd)
243-
if "=" in cmd and "=" == cmd.split(None, 1)[0][-1:]:
244-
# VAR=val — skip to next token
245-
if len(tokens) > 1:
246-
cmd = tokens[1]
247-
tokens = tokens[1:]
248-
else:
249-
continue
240+
# Command analysis — process each sub-command in the token stream
241+
# (splits on ; && || so that "echo x; rm -rf /" is fully analysed)
242+
self._dispatch_commands(line_no, raw_line, tokens)
250243

251-
cmd_lower = cmd.lower()
252-
args = tokens[1:]
253-
args_str = " ".join(args)
244+
# ------------------------------------------------------------------
245+
# Sub-command dispatcher (handles ; && || on a line)
246+
# ------------------------------------------------------------------
254247

255-
# --- Dispatch ---
256-
if cmd_lower == "rm":
257-
self._check_rm(line_no, raw_line, tokens, args)
258-
elif cmd_lower in _NETWORK_COMMANDS:
259-
self._findings.append(
260-
BashScanFinding(
261-
kind=cmd_lower,
262-
command=cmd_lower,
263-
args=args_str,
264-
line_number=line_no,
265-
evidence=raw_line.strip()[:300],
266-
))
267-
elif cmd_lower in _PRIVILEGE_COMMANDS:
268-
self._findings.append(
269-
BashScanFinding(
270-
kind="sudo" if cmd_lower != "chroot" else "sudo",
271-
command=cmd_lower,
272-
args=args_str,
273-
line_number=line_no,
274-
evidence=raw_line.strip()[:300],
275-
extra={"privilege_command": cmd_lower},
276-
))
277-
elif cmd_lower in _INSTALL_COMMANDS:
278-
sub = args[0].lower() if args else ""
279-
is_install = sub in _SUBCOMMAND_MAP.get(cmd_lower, set())
280-
if is_install or cmd_lower in ("pip", "pip3", "pipx"):
281-
self._findings.append(
282-
BashScanFinding(
283-
kind="install",
284-
command=cmd_lower,
285-
args=args_str,
286-
line_number=line_no,
287-
evidence=raw_line.strip()[:300],
288-
extra={
289-
"package_manager": cmd_lower,
290-
"subcommand": sub if is_install else "",
291-
},
292-
))
293-
elif cmd_lower in _DESTRUCTIVE_COMMANDS:
294-
self._findings.append(
295-
BashScanFinding(
296-
kind="command",
297-
command=cmd_lower,
298-
args=args_str,
299-
line_number=line_no,
300-
evidence=raw_line.strip()[:300],
301-
extra={"risk": "destructive"},
302-
))
303-
elif cmd_lower in _DYNAMIC_COMMANDS:
304-
self._findings.append(
305-
BashScanFinding(
306-
kind="eval",
307-
command=cmd_lower,
308-
args=args_str,
309-
line_number=line_no,
310-
evidence=raw_line.strip()[:300],
311-
))
312-
elif cmd_lower in _FILE_READ_COMMANDS:
313-
# Check if reads a sensitive path
314-
path_args = [a for a in args if not a.startswith("-")]
315-
for pa in path_args:
316-
if _is_sensitive_path(pa):
317-
self._findings.append(
318-
BashScanFinding(
319-
kind="command",
248+
def _dispatch_commands(self, line_no: int, raw_line: str, tokens: List[str]) -> None:
249+
"""Analyse each sub-command in *tokens*, splitting on ``;`` ``&&`` ``||``.
250+
251+
This ensures that ``echo x; rm -rf /`` and ``FOO=bar rm -rf /`` are
252+
both fully analysed, not just the first token.
253+
"""
254+
seg_start = 0
255+
for i, t in enumerate(tokens):
256+
if t in (";", "&&", "||"):
257+
self._analyse_one_command(line_no, raw_line, tokens[seg_start:i])
258+
seg_start = i + 1
259+
# Tail segment (or the whole line if no separators)
260+
if seg_start < len(tokens):
261+
self._analyse_one_command(line_no, raw_line, tokens[seg_start:])
262+
263+
def _analyse_one_command(self, line_no: int, raw_line: str, cmd_tokens: List[str]) -> None:
264+
"""Dispatch a single command segment to the appropriate checker."""
265+
if not cmd_tokens:
266+
return
267+
# Skip variable assignments (FOO=bar cmd ...)
268+
idx = 0
269+
while idx < len(cmd_tokens) and re.match(r"[A-Za-z_]\w*=", cmd_tokens[idx]):
270+
idx += 1
271+
if idx >= len(cmd_tokens):
272+
return # pure assignment, no command
273+
cmd = cmd_tokens[idx]
274+
args = cmd_tokens[idx + 1:]
275+
cmd_lower = cmd.lower()
276+
args_str = " ".join(args)
277+
evidence = " ".join(cmd_tokens)[:300]
278+
279+
if cmd_lower == "rm":
280+
self._check_rm(line_no, raw_line, cmd_tokens[idx:], args)
281+
elif cmd_lower in _NETWORK_COMMANDS:
282+
self._findings.append(
283+
BashScanFinding(kind=cmd_lower,
320284
command=cmd_lower,
321285
args=args_str,
322286
line_number=line_no,
323-
evidence=raw_line.strip()[:300],
324-
extra={
325-
"risk": "sensitive_file_read",
326-
"path": pa
327-
},
328-
))
329-
break
330-
elif cmd_lower == "dd":
331-
self._check_dd(line_no, raw_line, args)
332-
elif cmd_lower == "tee":
333-
# tee writes to files; check for sensitive paths in args
334-
for pa in args:
335-
if pa and not pa.startswith("-") and (_is_sensitive_path(pa) or pa.startswith("/dev/sd")):
336-
self._findings.append(
337-
BashScanFinding(
338-
kind="command",
339-
command="tee",
287+
evidence=evidence))
288+
elif cmd_lower in _PRIVILEGE_COMMANDS:
289+
self._findings.append(
290+
BashScanFinding(kind="sudo",
291+
command=cmd_lower,
292+
args=args_str,
293+
line_number=line_no,
294+
evidence=evidence,
295+
extra={"privilege_command": cmd_lower}))
296+
elif cmd_lower in _INSTALL_COMMANDS:
297+
sub = args[0].lower() if args else ""
298+
is_install = sub in _SUBCOMMAND_MAP.get(cmd_lower, set())
299+
if is_install or cmd_lower in ("pip", "pip3", "pipx"):
300+
self._findings.append(
301+
BashScanFinding(kind="install",
302+
command=cmd_lower,
303+
args=args_str,
304+
line_number=line_no,
305+
evidence=evidence,
306+
extra={
307+
"package_manager": cmd_lower,
308+
"subcommand": sub if is_install else ""
309+
}))
310+
elif cmd_lower in _DESTRUCTIVE_COMMANDS:
311+
self._findings.append(
312+
BashScanFinding(kind="command",
313+
command=cmd_lower,
340314
args=args_str,
341315
line_number=line_no,
342-
evidence=raw_line.strip()[:300],
343-
extra={
344-
"risk": "sensitive_file_write",
345-
"path": pa
346-
},
347-
))
348-
break
316+
evidence=evidence,
317+
extra={"risk": "destructive"}))
318+
elif cmd_lower in _DYNAMIC_COMMANDS:
319+
self._findings.append(
320+
BashScanFinding(kind="eval", command=cmd_lower, args=args_str, line_number=line_no, evidence=evidence))
321+
elif cmd_lower in _FILE_READ_COMMANDS:
322+
path_args = [a for a in args if not a.startswith("-")]
323+
for pa in path_args:
324+
if _is_sensitive_path(pa):
325+
self._findings.append(
326+
BashScanFinding(kind="command",
327+
command=cmd_lower,
328+
args=args_str,
329+
line_number=line_no,
330+
evidence=evidence,
331+
extra={
332+
"risk": "sensitive_file_read",
333+
"path": pa
334+
}))
335+
break
336+
elif cmd_lower == "dd":
337+
self._check_dd(line_no, raw_line, args)
338+
elif cmd_lower == "tee":
339+
for pa in args:
340+
if pa and not pa.startswith("-") and (_is_sensitive_path(pa) or pa.startswith("/dev/sd")):
341+
self._findings.append(
342+
BashScanFinding(kind="command",
343+
command="tee",
344+
args=args_str,
345+
line_number=line_no,
346+
evidence=evidence,
347+
extra={
348+
"risk": "sensitive_file_write",
349+
"path": pa
350+
}))
351+
break
349352

350353
# ------------------------------------------------------------------
351354
# Specific checks

trpc_agent_sdk/tools/safety/_rules.py

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -852,29 +852,46 @@ def __call__(
852852

853853

854854
def _is_in_echo_string(line: str, pattern: str) -> bool:
855-
"""Return True if *pattern* match is inside an echo/printf string literal.
855+
"""Return True if *pattern* matches are ALL inside echo/printf string literals.
856856
857-
In Bash, ``echo 'rm -rf /'`` is harmless — the dangerous command is just
858-
printed, not executed. This helper avoids flagging such lines.
857+
In Bash, ``echo 'rm -rf /'`` is harmless. But ``echo "rm -rf /"; rm -rf /``
858+
is dangerous — the first ``rm`` is harmless but the second is real. This
859+
helper only suppresses a finding when the pattern appears **nowhere** outside
860+
echo/printf quoted strings on the line.
859861
"""
860862
stripped = line.strip()
861863
# Only applies to echo / printf commands
862864
if not (stripped.startswith("echo ") or stripped.startswith("echo\t") or stripped.startswith("printf ")
863865
or stripped.startswith("printf\t") or stripped.startswith("/bin/echo ")
864866
or stripped.startswith("/usr/bin/echo ")):
865867
return False
866-
# Check if the pattern *matches* inside single or double quotes
868+
# Check if the pattern matches inside any quoted string
867869
try:
868870
pat = re.compile(pattern, re.IGNORECASE)
869871
except re.error:
870872
return False
873+
874+
in_quotes = False
871875
for m in re.finditer(r"'[^']*'", stripped):
872876
if pat.search(m.group(0)):
873-
return True
874-
for m in re.finditer(r'"[^"]*"', stripped):
875-
if pat.search(m.group(0)):
876-
return True
877-
return False
877+
in_quotes = True
878+
break
879+
if not in_quotes:
880+
for m in re.finditer(r'"[^"]*"', stripped):
881+
if pat.search(m.group(0)):
882+
in_quotes = True
883+
break
884+
if not in_quotes:
885+
return False # pattern not in any quoted string — normal danger report
886+
887+
# Pattern found inside quotes. Now check whether it ALSO appears
888+
# outside quotes (e.g. after ; / &&). If so, it is a real danger.
889+
outside = re.sub(r"'[^']*'", " ", stripped)
890+
outside = re.sub(r'"[^"]*"', " ", outside)
891+
if pat.search(outside):
892+
return False # match outside quotes → real danger
893+
894+
return True
878895

879896

880897
def _all_commands_whitelisted(line: str, policy: SafetyPolicy) -> bool:

trpc_agent_sdk/tools/safety/_safety_filter.py

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -127,6 +127,9 @@ async def _before(self, ctx: AgentContext, req: Any, rsp: FilterResult) -> None:
127127
# Always set OTel attributes (no-op if OTel not installed)
128128
set_safety_span_attributes(report)
129129

130+
# Always expose the report for downstream inspection
131+
setattr(rsp, "safety_report", report)
132+
130133
if report.decision == Decision.DENY:
131134
logger.warning(
132135
"ToolSafetyFilter BLOCKED tool '%s': %s",
@@ -143,14 +146,12 @@ async def _before(self, ctx: AgentContext, req: Any, rsp: FilterResult) -> None:
143146
tool_name,
144147
report.summary,
145148
)
146-
setattr(rsp, "safety_report", report)
147149
if self._block_on_review:
148150
rsp.error = ToolSafetyDeniedError(report)
149151
rsp.is_continue = False
150152

151153
else:
152154
logger.debug("ToolSafetyFilter allowed tool '%s'.", tool_name)
153-
setattr(rsp, "safety_report", report)
154155

155156

156157
class ToolSafetyDeniedError(RuntimeError):

trpc_agent_sdk/tools/safety/_scanner.py

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,10 +1014,10 @@ def _strip_python_comment_line(line: str) -> str:
10141014

10151015

10161016
def _is_in_echo_string(line: str, pattern: str) -> bool:
1017-
"""Return True if *pattern* matches inside an echo/printf string literal.
1017+
"""Return True if *pattern* matches are ALL inside echo/printf string literals.
10181018
1019-
In Bash, ``echo 'rm -rf /'`` is harmless — the dangerous command is just
1020-
printed, not executed.
1019+
In Bash, ``echo 'rm -rf /'`` is harmless. But ``echo "rm -rf /"; rm -rf /``
1020+
is dangerous — the first match is harmless but the second is real.
10211021
"""
10221022
stripped = line.strip()
10231023
if not (stripped.startswith("echo ") or stripped.startswith("echo\t") or stripped.startswith("printf ")
@@ -1028,10 +1028,21 @@ def _is_in_echo_string(line: str, pattern: str) -> bool:
10281028
pat = re.compile(pattern, re.IGNORECASE)
10291029
except re.error:
10301030
return False
1031+
in_quotes = False
10311032
for m in re.finditer(r"'[^']*'", stripped):
10321033
if pat.search(m.group(0)):
1033-
return True
1034-
for m in re.finditer(r'"[^"]*"', stripped):
1035-
if pat.search(m.group(0)):
1036-
return True
1037-
return False
1034+
in_quotes = True
1035+
break
1036+
if not in_quotes:
1037+
for m in re.finditer(r'"[^"]*"', stripped):
1038+
if pat.search(m.group(0)):
1039+
in_quotes = True
1040+
break
1041+
if not in_quotes:
1042+
return False
1043+
# Pattern found inside quotes — also check if it appears outside.
1044+
outside = re.sub(r"'[^']*'", " ", stripped)
1045+
outside = re.sub(r'"[^"]*"', " ", outside)
1046+
if pat.search(outside):
1047+
return False
1048+
return True

0 commit comments

Comments
 (0)