Skip to content

Commit b1a85b9

Browse files
committed
fix: echo "$(...)" command substitution bypass
_is_in_echo_string now only suppresses patterns inside single-quoted strings (literal in bash). Double-quoted strings with $(...) or backticks are NOT suppressed since the command substitution actually executes. Both copies (_scanner.py and _rules.py) fixed.
1 parent 896e877 commit b1a85b9

2 files changed

Lines changed: 45 additions & 33 deletions

File tree

trpc_agent_sdk/tools/safety/_rules.py

Lines changed: 22 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -852,45 +852,46 @@ def __call__(
852852

853853

854854
def _is_in_echo_string(line: str, pattern: str) -> bool:
855-
"""Return True if *pattern* matches are ALL inside echo/printf string literals.
855+
"""Return True if *pattern* matches are ALL inside harmless echo/printf string literals.
856856
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.
857+
In Bash, single-quoted strings are literal (``echo 'rm -rf /'`` is harmless).
858+
Double-quoted strings allow ``$(...)`` and backtick command substitution,
859+
so ``echo "$(rm -rf /)"`` actually executes — we must NOT suppress patterns
860+
inside double quotes when command substitution is present.
861861
"""
862862
stripped = line.strip()
863-
# Only applies to echo / printf commands
864863
if not (stripped.startswith("echo ") or stripped.startswith("echo\t") or stripped.startswith("printf ")
865864
or stripped.startswith("printf\t") or stripped.startswith("/bin/echo ")
866865
or stripped.startswith("/usr/bin/echo ")):
867866
return False
868-
# Check if the pattern matches inside any quoted string
869867
try:
870868
pat = re.compile(pattern, re.IGNORECASE)
871869
except re.error:
872870
return False
873871

874-
in_quotes = False
872+
# Single-quoted strings are always literal in bash → safe to suppress
873+
in_safe_quotes = False
875874
for m in re.finditer(r"'[^']*'", stripped):
876875
if pat.search(m.group(0)):
877-
in_quotes = True
876+
in_safe_quotes = True
878877
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.
878+
879+
# Double-quoted strings: only safe if they contain NO command substitution
880+
for m in re.finditer(r'"[^"]*"', stripped):
881+
if pat.search(m.group(0)):
882+
dq_content = m.group(0)
883+
if re.search(r'\$\(|`', dq_content):
884+
return False # $(...) or backticks execute — real danger
885+
in_safe_quotes = True
886+
break
887+
888+
if not in_safe_quotes:
889+
return False
890+
# Pattern inside safe quotes — check if appears outside too
889891
outside = re.sub(r"'[^']*'", " ", stripped)
890892
outside = re.sub(r'"[^"]*"', " ", outside)
891893
if pat.search(outside):
892-
return False # match outside quotes → real danger
893-
894+
return False
894895
return True
895896

896897

trpc_agent_sdk/tools/safety/_scanner.py

Lines changed: 23 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1014,10 +1014,12 @@ 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 are ALL inside echo/printf string literals.
1017+
"""Return True if *pattern* matches are ALL inside harmless echo/printf string literals.
10181018
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.
1019+
In Bash, single-quoted strings are literal (``echo 'rm -rf /'`` is harmless).
1020+
Double-quoted strings allow ``$(...)`` and backtick command substitution,
1021+
so ``echo "$(rm -rf /)"`` actually executes ``rm -rf /`` — we must NOT
1022+
suppress patterns inside double quotes when command substitution is present.
10211023
"""
10221024
stripped = line.strip()
10231025
if not (stripped.startswith("echo ") or stripped.startswith("echo\t") or stripped.startswith("printf ")
@@ -1028,19 +1030,28 @@ def _is_in_echo_string(line: str, pattern: str) -> bool:
10281030
pat = re.compile(pattern, re.IGNORECASE)
10291031
except re.error:
10301032
return False
1031-
in_quotes = False
1033+
1034+
# Check single-quoted strings (always literal in bash → safe to suppress)
1035+
in_safe_quotes = False
10321036
for m in re.finditer(r"'[^']*'", stripped):
10331037
if pat.search(m.group(0)):
1034-
in_quotes = True
1038+
in_safe_quotes = True
10351039
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:
1040+
1041+
# Check double-quoted strings — only safe if they contain NO command substitution
1042+
for m in re.finditer(r'"[^"]*"', stripped):
1043+
if pat.search(m.group(0)):
1044+
dq_content = m.group(0)
1045+
# If the double-quoted string contains $(...) or backticks,
1046+
# the pattern actually executes — do not suppress.
1047+
if re.search(r'\$\(|`', dq_content):
1048+
return False
1049+
in_safe_quotes = True
1050+
break
1051+
1052+
if not in_safe_quotes:
10421053
return False
1043-
# Pattern found inside quotes — also check if it appears outside.
1054+
# Pattern is inside safe quotes — also check if it appears outside.
10441055
outside = re.sub(r"'[^']*'", " ", stripped)
10451056
outside = re.sub(r'"[^"]*"', " ", outside)
10461057
if pat.search(outside):

0 commit comments

Comments
 (0)