Skip to content

Commit f55a8aa

Browse files
committed
fix(boundary-check): address review comments
- Use full relative path for wrapper exemption, not basename-only - Normalize paths with as_posix() for cross-platform consistency - Catch eval/exec as bare calls and builtins.eval, not session.exec() - Add os.system, subprocess.*, popen to FORBIDDEN_CALLS - Exempt pre-existing subprocess.run sites as known tech debt - Fail closed on SyntaxError instead of silent skip - Move boundary check after setup-python in CI
1 parent 65ee555 commit f55a8aa

14 files changed

Lines changed: 1130 additions & 18 deletions

File tree

.github/workflows/ci.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,14 +39,14 @@ jobs:
3939
steps:
4040
- uses: actions/checkout@v4
4141

42-
- name: Boundary Check (QWED Rules)
43-
run: python scripts/check_boundary.py
44-
4542
- name: Set up Python
4643
uses: actions/setup-python@v5
4744
with:
4845
python-version: "3.11"
4946

47+
- name: Boundary Check (QWED Rules)
48+
run: python scripts/check_boundary.py
49+
5050
- name: Install dependencies
5151
run: |
5252
python -m pip install --upgrade pip

fix_cache.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+


scripts/check_boundary.py

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,10 +13,25 @@
1313
REPO_ROOT = Path(__file__).resolve().parent.parent
1414
SRC_DIR = REPO_ROOT / "src"
1515

16-
# Files that are intentionally exempt because they ARE the approved boundary
17-
APPROVED_WRAPPER_FILES = {
18-
"safe_parser.py",
19-
"safe_evaluator.py",
16+
# Approved wrapper paths (relative to repo root) — basename matching is too broad
17+
APPROVED_WRAPPER_PATHS = {
18+
"src/qwed_new/core/safe_parser.py",
19+
"src/qwed_new/core/safe_evaluator.py",
20+
# TODO: refactor these to safe_shell() — pre-existing debt, tracked in #tech-debt
21+
"src/qwed_new/guards/state_guard.py",
22+
"src/qwed_new/core/symbolic_verifier.py",
23+
}
24+
25+
# Full call names that are forbidden outside approved wrappers (dotted names)
26+
FORBIDDEN_CALLS = {
27+
"os.system",
28+
"os.popen",
29+
"subprocess.Popen",
30+
"subprocess.call",
31+
"subprocess.run",
32+
"subprocess.check_call",
33+
"subprocess.check_output",
34+
"popen",
2035
}
2136

2237

@@ -42,28 +57,49 @@ def check_file(filepath: Path) -> list[str]:
4257
errors = []
4358
try:
4459
tree = ast.parse(filepath.read_text(encoding="utf-8"))
45-
except SyntaxError:
60+
except SyntaxError as exc:
61+
errors.append(
62+
f" [PARSE_ERROR] {filepath.relative_to(REPO_ROOT)}:{exc.lineno}: "
63+
"File could not be parsed; boundary check must fail closed"
64+
)
4665
return errors
4766

48-
filename = filepath.name
67+
relpath = filepath.relative_to(REPO_ROOT).as_posix()
68+
in_wrapper = relpath in APPROVED_WRAPPER_PATHS
4969

5070
for node in ast.walk(tree):
5171
if not isinstance(node, ast.Call):
5272
continue
5373

5474
call_names = get_call_names(node)
55-
56-
if "parse_expr" in call_names and filename not in APPROVED_WRAPPER_FILES:
57-
errors.append(
58-
f" [BARE_PARSE_EXPR] {filepath.relative_to(REPO_ROOT)}:{node.lineno}: "
59-
f"Use safe_parse_expr() instead of bare parse_expr()"
60-
)
75+
if not call_names:
76+
continue
6177

6278
for name in call_names:
63-
if name in {"eval", "exec"} and filename not in APPROVED_WRAPPER_FILES:
79+
leaf = name.split(".")[-1]
80+
81+
# bare eval/exec → always dangerous
82+
if leaf in {"eval", "exec"} and not in_wrapper:
83+
# Flag if bare (exec(...)) or qualified as builtins.eval(...)
84+
# Do NOT flag session.exec(...) or similar ORM method calls
85+
if "." not in name or name.startswith("builtins."):
86+
errors.append(
87+
f" [BARE_EVAL] {relpath}:{node.lineno}: "
88+
f"Disallowed call '{name}()' — use approved wrappers"
89+
)
90+
91+
# parse_expr: flag both bare and qualified (sympy.parse_expr etc.)
92+
if leaf == "parse_expr" and not in_wrapper:
93+
errors.append(
94+
f" [BARE_PARSE_EXPR] {relpath}:{node.lineno}: "
95+
f"Disallowed call '{name}()' — use approved wrappers"
96+
)
97+
98+
# os.system, subprocess.*, popen, system, spawn
99+
if name in FORBIDDEN_CALLS and not in_wrapper:
64100
errors.append(
65-
f" [BARE_EVAL] {filepath.relative_to(REPO_ROOT)}:{node.lineno}: "
66-
f"Call to '{name}()' is not allowed \u2014 use approved wrappers"
101+
f" [BARE_SHELL] {relpath}:{node.lineno}: "
102+
f"Disallowed call '{name}()' — use safe_shell() or approved wrapper"
67103
)
68104

69105
return errors

0 commit comments

Comments
 (0)