Skip to content

Commit e579bc1

Browse files
committed
fix: critical AST bypass + fail-open hardening + bash prefix coverage
- _python_scanner: _resolve_canonical now checks _class_instances for Session().get() bypass (s=requests.Session(); s.get(url) now detected) - _python_scanner: _handle_assign propagates dangerous callable names (e=eval; e('x'), m=__import__('os'); m.system('id') now caught) - _scanner: rule exceptions now emit GLOBAL-003 sentinel finding instead of silent fail-open - _scanner: allow_patterns refuses upgrade when HIGH/CRITICAL findings exist - _scanner: reload_policy() also refreshes _rules - _bash_scanner: _PREFIX_CMDS extended with env/nohup/timeout/nice/xargs - _rules: _all_commands_whitelisted splits on ;/&&/||/& in addition to |
1 parent a3676f1 commit e579bc1

5 files changed

Lines changed: 66 additions & 10 deletions

File tree

tests/test_tool_safety.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1262,7 +1262,9 @@ def _bad_rule(script, scan_input, policy):
12621262
tool_name="rule_exc_test",
12631263
))
12641264
# Should still complete despite the broken rule
1265-
assert report.decision == Decision.ALLOW
1265+
# Now produces GLOBAL-003 sentinel → MEDIUM → NEEDS_HUMAN_REVIEW
1266+
assert report.decision in (Decision.ALLOW, Decision.NEEDS_HUMAN_REVIEW)
1267+
assert any(f.rule_id == "GLOBAL-003" for f in report.findings)
12661268
finally:
12671269
# Clean up: remove the bad rule from registry
12681270
from trpc_agent_sdk.tools.safety._rules import _EXTRA_RULES

trpc_agent_sdk/tools/safety/_bash_scanner.py

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -276,9 +276,22 @@ def _analyse_one_command(self, line_no: int, raw_line: str, cmd_tokens: List[str
276276
if not cmd_tokens:
277277
return
278278
# Skip prefixes that don't change the real command:
279-
# VAR=val, export VAR=val, declare/readonly/local/typeset, command/builtin
279+
# VAR=val, export, env, nohup, timeout, nice, xargs, find, sudo variants
280280
idx = 0
281-
_PREFIX_CMDS = frozenset({"export", "declare", "local", "readonly", "typeset", "command", "builtin"})
281+
_PREFIX_CMDS = frozenset({
282+
"export",
283+
"declare",
284+
"local",
285+
"readonly",
286+
"typeset",
287+
"command",
288+
"builtin",
289+
"env",
290+
"nohup",
291+
"timeout",
292+
"nice",
293+
"xargs",
294+
})
282295
while idx < len(cmd_tokens):
283296
t = cmd_tokens[idx]
284297
if re.match(r"[A-Za-z_]\w*=", t) or re.match(r"[A-Za-z_]\w*\[\w*\]=", t):

trpc_agent_sdk/tools/safety/_python_scanner.py

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,11 +561,26 @@ def _handle_assign(self, node: ast.Assign) -> None:
561561
if not var_name:
562562
continue
563563

564-
# Check if RHS is an import
564+
# Check if RHS is an import / class instantiation
565565
if isinstance(node.value, ast.Call):
566566
canonical = self._resolve_canonical(node.value.func)
567567
if canonical in _NETWORK_CALLS:
568568
self._class_instances[var_name] = canonical
569+
if canonical in _DYNAMIC_EXEC_CALLS or canonical in _PROCESS_CALLS:
570+
# e = eval; e("code") / s = os.system; s("id")
571+
self._aliases[var_name] = canonical
572+
573+
# Propagate bare-name assignments: e = eval; m = __import__
574+
if isinstance(node.value, ast.Name):
575+
src_canonical = self._resolve_canonical(node.value)
576+
if src_canonical in _DYNAMIC_EXEC_CALLS or src_canonical in _PROCESS_CALLS:
577+
self._aliases[var_name] = src_canonical
578+
579+
# Propagate __import__ / importlib result
580+
if isinstance(node.value, ast.Call):
581+
inner = self._resolve_canonical(node.value.func)
582+
if inner in ("__import__", "importlib.import_module"):
583+
self._aliases[var_name] = "__import__"
569584

570585
# Check if RHS is from os.environ / os.getenv → taint
571586
# Only taint when the env key looks sensitive (KEY/TOKEN/SECRET/...)
@@ -720,7 +735,12 @@ def _resolve_canonical(self, node: ast.expr) -> str:
720735
``getattr(x, 'y')`` → ``"getattr"``
721736
"""
722737
if isinstance(node, ast.Name):
723-
# Direct name lookup — consult alias table first
738+
# Direct name lookup — consult class_instances first (so that
739+
# s.get("http://evil.com") where s = requests.Session() resolves
740+
# to "requests.Session.get"), then alias table, then bare name.
741+
cls = self._class_instances.get(node.id)
742+
if cls:
743+
return cls
724744
return self._aliases.get(node.id, node.id)
725745

726746
if isinstance(node, ast.Attribute):

trpc_agent_sdk/tools/safety/_rules.py

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -897,10 +897,15 @@ def _is_in_echo_string(line: str, pattern: str) -> bool:
897897

898898

899899
def _all_commands_whitelisted(line: str, policy: SafetyPolicy) -> bool:
900-
"""Return True if every command in a piped bash line is whitelisted."""
900+
"""Return True if every command in a line is whitelisted.
901+
902+
Splits on ``|``, ``;``, ``&&``, ``||``, and ``&`` so that commands
903+
after non-pipe separators are also checked.
904+
"""
905+
import re as _re
901906
cmds = []
902-
for part in line.split("|"):
903-
part = part.strip()
907+
for part in _re.split(r"[|;&]", line):
908+
part = part.strip().lstrip("&|")
904909
if part:
905910
first_word = part.split()[0] if part.split() else ""
906911
if first_word and not first_word.startswith("-"):

trpc_agent_sdk/tools/safety/_scanner.py

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,17 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
198198
all_findings.extend(findings)
199199
except Exception: # pylint: disable=broad-except
200200
logger.error("Safety rule raised an exception; skipping: %s", str(getattr(rule, "__class__", rule)))
201+
all_findings.append(
202+
SafetyFinding(
203+
rule_id="GLOBAL-003",
204+
category=RiskCategory.RESOURCE_ABUSE,
205+
risk_level=RiskLevel.MEDIUM,
206+
evidence="A safety rule crashed during scanning.",
207+
message=f"Rule {getattr(rule, '__class__', rule)} failed — scan may be incomplete.",
208+
recommendation="Review the script manually; automated analysis was partial.",
209+
line_number=0,
210+
matched_pattern="",
211+
))
201212

202213
# Check environment variables against blocklist
203214
if scan_input.environment_variables:
@@ -270,8 +281,12 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
270281
# Also refuses to upgrade when any CRITICAL finding exists, regardless
271282
# of how the policy maps CRITICAL → decision.
272283
has_critical = any(f.risk_level == RiskLevel.CRITICAL for f in all_findings)
284+
has_high_or_critical = any(f.risk_level in (RiskLevel.HIGH, RiskLevel.CRITICAL) for f in all_findings)
273285
allow_upgraded = False
274-
if (decision == Decision.NEEDS_HUMAN_REVIEW and not has_critical and self._check_allow_patterns(script)):
286+
# Only upgrade MEDIUM or lower (NEEDS_HUMAN_REVIEW) — never upgrade HIGH/CRITICAL
287+
# even if the policy maps them to NEEDS_HUMAN_REVIEW.
288+
if (decision == Decision.NEEDS_HUMAN_REVIEW and not has_high_or_critical
289+
and self._check_allow_patterns(script)):
275290
logger.info("allow_patterns upgraded NEEDS_HUMAN_REVIEW → ALLOW for '%s'", scan_input.tool_name)
276291
decision = Decision.ALLOW
277292
allow_upgraded = True
@@ -321,8 +336,9 @@ def scan(self, scan_input: SafetyScanInput) -> SafetyScanReport:
321336
)
322337

323338
def reload_policy(self) -> None:
324-
"""Reload the policy from disk (useful for hot-reload)."""
339+
"""Reload the policy and rules from disk (useful for hot-reload)."""
325340
self._policy = reload_policy()
341+
self._rules = get_all_rules()
326342

327343
# ------------------------------------------------------------------
328344
# Layer 1: AST-based Python scanning

0 commit comments

Comments
 (0)