|
|
@@ -109,9 +109,26 @@ def _rhs_is_attacker_shape(node: ast.AST) -> bool:
|
|
|
return isinstance(node, ast.Name)
|
|
|
|
|
|
|
|
|
+_CONTINUATION_TOKENS = (")", "]", "}", ",")
|
|
|
+
|
|
|
+
|
|
|
def _line_has_marker(source_lines: list[str], lineno: int, end_lineno: int | None) -> bool:
|
|
|
- # Walk every line spanned by the BinOp — the marker can sit anywhere in the
|
|
|
- # expression (start, end, or any continuation line of a multi-line join).
|
|
|
+ """Check whether a ``# SEC-PATH-OK:`` marker covers this join.
|
|
|
+
|
|
|
+ Looks at every line spanned by the BinOp itself, plus one line past
|
|
|
+ ``end_lineno`` IF that line begins with a continuation token (``)``,
|
|
|
+ ``]``, ``}``, ``,``). The peek captures the project's convention of
|
|
|
+ wrapping a BinOp in parens and placing the marker on the closing line:
|
|
|
+
|
|
|
+ file_path = (
|
|
|
+ library_dir / filename
|
|
|
+ ) # SEC-PATH-OK: filename validated above
|
|
|
+
|
|
|
+ The BinOp itself ends on the inner line, but the marker reads more
|
|
|
+ naturally on the closing line. Restricting the peek to continuation
|
|
|
+ lines prevents giving a free pass to a marker that happens to sit on
|
|
|
+ a wholly unrelated follow-on statement.
|
|
|
+ """
|
|
|
start = max(1, lineno)
|
|
|
end = max(start, end_lineno or lineno)
|
|
|
for i in range(start, end + 1):
|
|
|
@@ -119,6 +136,13 @@ def _line_has_marker(source_lines: list[str], lineno: int, end_lineno: int | Non
|
|
|
continue
|
|
|
if _MARKER in source_lines[i - 1]:
|
|
|
return True
|
|
|
+ # Project convention: marker often sits on the closing-paren line, one
|
|
|
+ # line past the BinOp's `end_lineno`. Only accept it when the line is a
|
|
|
+ # continuation of the wrapping expression.
|
|
|
+ if end < len(source_lines):
|
|
|
+ trailing = source_lines[end].lstrip()
|
|
|
+ if trailing.startswith(_CONTINUATION_TOKENS) and _MARKER in source_lines[end]:
|
|
|
+ return True
|
|
|
return False
|
|
|
|
|
|
|
|
|
@@ -174,6 +198,83 @@ def _scan_file(py_file: Path) -> list[str]:
|
|
|
return findings
|
|
|
|
|
|
|
|
|
+def _scan_source(source: str, tmp_path: Path) -> list[str]:
|
|
|
+ """Drop ``source`` into a temp .py file and run ``_scan_file`` against it.
|
|
|
+ Returns the findings list."""
|
|
|
+ f = tmp_path / "candidate.py"
|
|
|
+ f.write_text(source)
|
|
|
+ return _scan_file(f)
|
|
|
+
|
|
|
+
|
|
|
+class TestMarkerDetection:
|
|
|
+ """Pins the contract for where a ``# SEC-PATH-OK:`` marker is accepted.
|
|
|
+
|
|
|
+ Markers must sit either (a) somewhere within the BinOp's own source
|
|
|
+ lines, or (b) on the immediately-following line when that line is a
|
|
|
+ continuation token (closing paren / bracket / brace / comma). The
|
|
|
+ second case is the project's convention of wrapping the BinOp in
|
|
|
+ parens and placing the marker on the closing-paren line.
|
|
|
+ """
|
|
|
+
|
|
|
+ def test_marker_on_binop_line_recognised(self, tmp_path):
|
|
|
+ source = (
|
|
|
+ "from pathlib import Path\nbase_dir = Path('.')\ndef f(x): return base_dir / x # SEC-PATH-OK: trusted x\n"
|
|
|
+ )
|
|
|
+ assert _scan_source(source, tmp_path) == []
|
|
|
+
|
|
|
+ def test_marker_on_closing_paren_line_recognised(self, tmp_path):
|
|
|
+ # The exact convention used across api/routes/ and services/: the
|
|
|
+ # BinOp lives inside a parenthesised expression and the marker sits
|
|
|
+ # on the closing-paren line, one past the BinOp's `end_lineno`.
|
|
|
+ source = (
|
|
|
+ "from pathlib import Path\n"
|
|
|
+ "base_dir = Path('.')\n"
|
|
|
+ "def f(x):\n"
|
|
|
+ " return (\n"
|
|
|
+ " base_dir / x\n"
|
|
|
+ " ) # SEC-PATH-OK: trusted x\n"
|
|
|
+ )
|
|
|
+ assert _scan_source(source, tmp_path) == []
|
|
|
+
|
|
|
+ def test_unrelated_marker_below_does_not_silence(self, tmp_path):
|
|
|
+ # A second join below carries a marker; the first does not. Only
|
|
|
+ # the first must be flagged — markers don't cross statement
|
|
|
+ # boundaries.
|
|
|
+ source = (
|
|
|
+ "from pathlib import Path\n"
|
|
|
+ "base_dir = Path('.')\n"
|
|
|
+ "def f(x, y):\n"
|
|
|
+ " p = base_dir / x\n"
|
|
|
+ " q = base_dir / y # SEC-PATH-OK: trusted y\n"
|
|
|
+ )
|
|
|
+ findings = _scan_source(source, tmp_path)
|
|
|
+ assert len(findings) == 1
|
|
|
+ assert "base_dir / x" in findings[0]
|
|
|
+
|
|
|
+ def test_marker_on_non_continuation_line_does_not_silence(self, tmp_path):
|
|
|
+ # A SEC-PATH-OK comment on the line right after the BinOp counts
|
|
|
+ # only when that line is a continuation of the wrapping expression.
|
|
|
+ # An unrelated next statement's marker must not free-pass the join.
|
|
|
+ source = (
|
|
|
+ "from pathlib import Path\n"
|
|
|
+ "base_dir = Path('.')\n"
|
|
|
+ "def f(x):\n"
|
|
|
+ " a = base_dir / x\n"
|
|
|
+ " b = 1 # SEC-PATH-OK: unrelated, for a different statement\n"
|
|
|
+ )
|
|
|
+ findings = _scan_source(source, tmp_path)
|
|
|
+ assert len(findings) == 1
|
|
|
+ assert "base_dir / x" in findings[0]
|
|
|
+
|
|
|
+ def test_no_marker_anywhere_is_flagged(self, tmp_path):
|
|
|
+ # Pin the negative path so a future refactor can't accidentally turn
|
|
|
+ # marker detection into "always returns True".
|
|
|
+ source = "from pathlib import Path\nbase_dir = Path('.')\ndef f(x): return base_dir / x\n"
|
|
|
+ findings = _scan_source(source, tmp_path)
|
|
|
+ assert len(findings) == 1
|
|
|
+ assert "base_dir / x" in findings[0]
|
|
|
+
|
|
|
+
|
|
|
def test_route_path_arithmetic_is_safe_joined_or_marked():
|
|
|
"""Every ``<dir-like> / <non-constant>`` join in a route handler must
|
|
|
either route through ``safe_join_under(...)`` or carry a
|