Skip to content

Commit a834834

Browse files
committed
feat(safety): wire remaining rules, fix import, add detection eval
- Wire tool-fs-system-dir-write (open(...,'w') into system dirs) and tool-secret-private-key (embedded PEM literal anywhere in source) in the python scanner; these rule ids were previously defined but never fired - Add `from typing import Optional` to _shell_parse (it was referenced only inside a non-evaluated local annotation, but keep it correct/explicit) - Add manifest samples 15/16 covering the two newly wired rules - Add scripts/tool_safety_eval.py: detection / false-positive rate eval for issue #90 acceptance #2 (>=90% detection, <=10% FP) -> 100% / 0% on manifest All 73 safety tests pass.
1 parent cfc973e commit a834834

4 files changed

Lines changed: 133 additions & 11 deletions

File tree

scripts/tool_safety_eval.py

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
# Copyright (C) 2026 Tencent. All rights reserved.
2+
# tRPC-Agent-Python is licensed under Apache-2.0.
3+
"""Detection-rate / false-positive-rate eval against the sample manifest.
4+
5+
Satisfies issue #90 acceptance #2 (high-risk detection >= 90%, safe-sample
6+
false-positive <= 10%) with a runnable, reproducible measurement.
7+
8+
A "positive" sample is one whose manifest expected_decision != ALLOW (it should
9+
be flagged as risky); "detected" means the scan also returns != ALLOW. A
10+
"negative" sample expects ALLOW; a false positive is one the scan blocks.
11+
12+
Usage:
13+
python scripts/tool_safety_eval.py [--manifest m.yaml] [--policy p.yaml]
14+
"""
15+
from __future__ import annotations
16+
17+
import argparse
18+
from pathlib import Path
19+
from typing import NamedTuple
20+
21+
import yaml
22+
23+
from trpc_agent_sdk.tools.safety._policy import load_policy
24+
from trpc_agent_sdk.tools.safety._scanner import scan
25+
26+
_DEFAULT_MANIFEST = (
27+
Path(__file__).resolve().parent.parent
28+
/ "tests" / "tools" / "safety" / "samples" / "manifest.yaml"
29+
)
30+
31+
32+
class Outcome(NamedTuple):
33+
name: str
34+
expected: str
35+
actual: str
36+
correct: bool # scan agrees with manifest on allow-vs-block
37+
38+
39+
def _evaluate(manifest: Path, policy_path: str | None) -> list[Outcome]:
40+
data = yaml.safe_load(manifest.read_text(encoding="utf-8"))
41+
policy = load_policy(policy_path)
42+
out: list[Outcome] = []
43+
for s in data["samples"]:
44+
actual = scan(policy, s["script"], language=s["language"]).decision.name
45+
expected = s["expected_decision"]
46+
correct = (actual == "ALLOW") == (expected == "ALLOW")
47+
out.append(Outcome(s["name"], expected, actual, correct))
48+
return out
49+
50+
51+
def main() -> int:
52+
ap = argparse.ArgumentParser(description=__doc__)
53+
ap.add_argument("--manifest", default=str(_DEFAULT_MANIFEST))
54+
ap.add_argument("--policy", default=None)
55+
args = ap.parse_args()
56+
57+
outcomes = _evaluate(Path(args.manifest), args.policy)
58+
positives = [o for o in outcomes if o.expected != "ALLOW"]
59+
negatives = [o for o in outcomes if o.expected == "ALLOW"]
60+
detected = [o for o in positives if o.actual != "ALLOW"]
61+
false_pos = [o for o in negatives if o.actual != "ALLOW"]
62+
63+
det_rate = len(detected) / len(positives) if positives else 1.0
64+
fp_rate = len(false_pos) / len(negatives) if negatives else 0.0
65+
66+
for o in outcomes:
67+
flag = "OK " if o.correct else "ERR"
68+
print(f" [{flag}] {o.name:<24} expected={o.expected:<14} actual={o.actual}")
69+
print()
70+
print(f"positives={len(positives)} detected={len(detected)} "
71+
f"detection_rate={det_rate:.0%}")
72+
print(f"negatives={len(negatives)} false_pos={len(false_pos)} "
73+
f"false_positive_rate={fp_rate:.0%}")
74+
75+
ok = det_rate >= 0.90 and fp_rate <= 0.10
76+
print("RESULT:", "PASS" if ok else "FAIL",
77+
"(acceptance #2: detection>=90%, false_positive<=10%)")
78+
return 0 if ok else 1
79+
80+
81+
if __name__ == "__main__":
82+
raise SystemExit(main())

tests/tools/safety/samples/manifest.yaml

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -109,4 +109,20 @@ samples:
109109
from concurrent.futures import ThreadPoolExecutor
110110
ThreadPoolExecutor(max_workers=10000)
111111
expected_decision: NEEDS_REVIEW
112-
required_rule_ids: [tool-res-concurrent-flood]
112+
required_rule_ids: [tool-res-concurrent-flood]
113+
114+
- name: 15_system_dir_write
115+
language: python
116+
script: |
117+
with open("/etc/passwd", "w") as f:
118+
f.write("hacked")
119+
expected_decision: DENY
120+
required_rule_ids: [tool-fs-system-dir-write]
121+
122+
- name: 16_private_key
123+
language: python
124+
script: |
125+
key = "-----BEGIN RSA PRIVATE KEY-----\nMIIEoQIBAA\n-----END RSA PRIVATE KEY-----"
126+
print(key)
127+
expected_decision: DENY
128+
required_rule_ids: [tool-secret-private-key]

trpc_agent_sdk/tools/safety/_python_scanner.py

Lines changed: 33 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,15 @@
1515
from trpc_agent_sdk.tools.safety._rules import R_CODE_UNSAFE_EXEC
1616
from trpc_agent_sdk.tools.safety._rules import R_FS_RECURSIVE_DELETE
1717
from trpc_agent_sdk.tools.safety._rules import R_FS_READ_CREDENTIALS
18+
from trpc_agent_sdk.tools.safety._rules import R_FS_SYSTEM_DIR
1819
from trpc_agent_sdk.tools.safety._rules import R_NET_HTTP
1920
from trpc_agent_sdk.tools.safety._rules import R_NET_SOCKET
2021
from trpc_agent_sdk.tools.safety._rules import R_PROC_SUBPROCESS
2122
from trpc_agent_sdk.tools.safety._rules import R_RES_CONCURRENT_FLOOD
2223
from trpc_agent_sdk.tools.safety._rules import R_RES_INFINITE_LOOP
2324
from trpc_agent_sdk.tools.safety._rules import R_RES_LARGE_WRITE
2425
from trpc_agent_sdk.tools.safety._rules import R_SECRET_LOGGING
26+
from trpc_agent_sdk.tools.safety._rules import R_SECRET_PRIVATE_KEY
2527
from trpc_agent_sdk.tools.safety._types import Decision
2628
from trpc_agent_sdk.tools.safety._types import Finding
2729
from trpc_agent_sdk.tools.safety._types import RiskLevel
@@ -30,6 +32,9 @@
3032
_URL_RE = re.compile(r"https?://([^/\s'\"']+)", re.I)
3133
_SECRET_NAME_RE = re.compile(r"(api[_-]?key|secret|token|password|passwd|private[_-]?key)", re.I)
3234
_PRIVATE_KEY_RE = re.compile(r"-----BEGIN [A-Z ]*PRIVATE KEY-----")
35+
# System directories: writing here can brick the runtime (issue risk class #1).
36+
_SYSTEM_DIRS = ("/etc/", "/usr/", "/bin/", "/sbin/", "/boot/", "/sys/",
37+
"/proc/", "/lib/", "/lib64/", "/var/", "/dev/")
3338

3439
# attribute path -> rule fired when called/used
3540
_DANGEROUS_ATTR = {
@@ -125,16 +130,17 @@ def resolve_attr(node: ast.AST) -> str:
125130
# infinite loop
126131
if isinstance(node, (ast.While,)) and _is_truthy(node.test):
127132
add(R_RES_INFINITE_LOOP, "while True:", "infinite loop; review.")
128-
# secret logging: assignment of secret-named var + later print
133+
# secret logging: assignment to a secret-named variable.
129134
if isinstance(node, ast.Assign):
130135
for t in node.targets:
131136
if isinstance(t, ast.Name) and _SECRET_NAME_RE.search(t.id):
132-
if _PRIVATE_KEY_RE.search(_literal(node.value)):
133-
add(R_SECRET_LOGGING, f"private key in {t.id}",
134-
"private key literal detected.")
135-
else:
136-
add(R_SECRET_LOGGING, f"secret assigned to {t.id}",
137-
"secret-like variable; avoid logging.")
137+
add(R_SECRET_LOGGING, f"secret assigned to {t.id}",
138+
"secret-like variable; avoid logging.")
139+
# private-key literal embedded anywhere (independent of variable name).
140+
if isinstance(node, ast.Constant) and isinstance(node.value, str) \
141+
and _PRIVATE_KEY_RE.search(node.value):
142+
add(R_SECRET_PRIVATE_KEY, "private key literal",
143+
"embedded private key; refuse.")
138144

139145
return findings
140146

@@ -161,6 +167,11 @@ def _check_open_path(call: ast.Call, policy: Policy, add) -> None:
161167
path = arg.value
162168
elif isinstance(arg, ast.JoinedStr):
163169
path = "".join(v.value for v in arg.values if isinstance(v, ast.Constant))
170+
mode = _open_mode(call)
171+
if path and _is_write_mode(mode) and path.startswith(_SYSTEM_DIRS):
172+
add(R_FS_SYSTEM_DIR, f"open('{path}','{mode}')",
173+
f"writing to system directory {path}; refuse.")
174+
return
164175
if path and _CRED_PATH_RE.search(path):
165176
add(R_FS_READ_CREDENTIALS, f"open('{path}')",
166177
f"reading credential path {path}; review.")
@@ -218,12 +229,24 @@ def _is_whitelisted(url: str, policy: Policy) -> bool:
218229
return root in {d.lower() for d in policy.whitelisted_domains} or host in policy.whitelisted_domains
219230

220231

221-
def _literal(node: ast.AST) -> str:
222-
if isinstance(node, ast.Constant) and isinstance(node.value, str):
223-
return node.value
232+
def _open_mode(call: ast.Call) -> str:
233+
"""Return the literal mode string of an open() call, else ''."""
234+
if len(call.args) >= 2:
235+
m = call.args[1]
236+
if isinstance(m, ast.Constant) and isinstance(m.value, str):
237+
return m.value
238+
for kw in call.keywords:
239+
if kw.arg == "mode" and isinstance(kw.value, ast.Constant) \
240+
and isinstance(kw.value.value, str):
241+
return kw.value.value
224242
return ""
225243

226244

245+
def _is_write_mode(mode: str) -> bool:
246+
"""True if the mode can create/modify a file (w x a +)."""
247+
return bool(set(mode) & set("wxa+"))
248+
249+
227250
def _heuristic_fallback(policy: Policy, script: str) -> list[Finding]:
228251
"""Best-effort when the script is not valid Python AST."""
229252
findings: list[Finding] = []

trpc_agent_sdk/tools/safety/_shell_parse.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
from __future__ import annotations
1010

1111
import shlex
12+
from typing import Optional
1213

1314

1415
def split_tokens(cmd: str) -> list[str]:

0 commit comments

Comments
 (0)