From 3368376ebf423e2d09f21d679cd39c33284a6c81 Mon Sep 17 00:00:00 2001 From: Hugo Date: Tue, 13 Oct 2026 08:28:48 +0200 Subject: [PATCH] feat(findings): mark findings in vendored code with their context Closes #243 --- docs/usage.md | 12 ++- src/coretrace_python/engine.py | 10 +- src/coretrace_python/findings/context.py | 20 ++-- src/coretrace_python/reporters/sarif.py | 2 +- tests/test_vendored_context.py | 115 +++++++++++++++++++++++ 5 files changed, 141 insertions(+), 18 deletions(-) create mode 100644 tests/test_vendored_context.py diff --git a/docs/usage.md b/docs/usage.md index da24f22..1eeff8c 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -360,10 +360,14 @@ A finding in test code carries `context: test` in its metadata, in the JSON repo a property of its SARIF result. Test code is a file under a `test`, `tests`, `testing`, `fixtures` or `__tests__` directory of the project, or a file named `test_*.py`, `*_test.py` or `conftest.py`, judged on its path from the root, not on where the -project sits. The context changes no verdict: it is there for baselines, `--fail-on` -wrappers, readers and refiner plugins, which see it. A rule may still give its own -context, as `hardcoded-credential` does with `example` for template files, and the -SARIF result carries that context too. +project sits. A finding in vendored code, under a `third_party`, `third-party`, `vendor` +or `vendored` directory, carries `context: vendored` instead, the test code of a vendored +project included: whose code it is comes first. Vendored code is still analysed and its +findings reported, so that a flow from the project through a vendored module is not +lost. The context changes no verdict: it is there for baselines, `--fail-on` wrappers, +readers and refiner plugins, which see it. A rule may still give its own context, as +`hardcoded-credential` does with `example` for template files, and the SARIF result +carries that context too, unless the code is vendored. `root` is the directory the paths are relative to. A file's `status` is `analysed`, `ambiguous` (analysed, but another file has its module name, see above), `syntax-error` diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index d11eb13..5e93025 100644 --- a/src/coretrace_python/engine.py +++ b/src/coretrace_python/engine.py @@ -72,7 +72,7 @@ Finding, Severity, ) -from coretrace_python.findings.context import code_context +from coretrace_python.findings.context import VENDORED, code_context from coretrace_python.findings.refutation import RefutationAnalysis from coretrace_python.findings.suppressions import partition from coretrace_python.frontend import HIRBuildError, ParseError, build_hir @@ -692,17 +692,15 @@ def _located(path: Path, root: Path) -> str: def _situated(finding: Finding, root: Path) -> Finding: """``finding`` with the context of its code in its metadata (``code_context``), judged - on its path in the project at ``root``. A context the rule gave the finding stays: it - is the rule's (#244).""" + on its path in the project at ``root``. A context the rule gave the finding stays + (#244), unless the code is vendored: whose code it is comes first (#243).""" - if "context" in finding.metadata: - return finding try: path = Path(str(finding.span.source_id)).resolve().relative_to(root) except ValueError: return finding situation = code_context(path.as_posix()) - if situation is None: + if situation is None or ("context" in finding.metadata and situation != VENDORED): return finding return replace(finding, metadata={**finding.metadata, "context": situation}) diff --git a/src/coretrace_python/findings/context.py b/src/coretrace_python/findings/context.py index 864a1e8..6e9c7eb 100644 --- a/src/coretrace_python/findings/context.py +++ b/src/coretrace_python/findings/context.py @@ -1,7 +1,7 @@ -"""Where the code of a finding lives, by its path in the project (#244): test code, which -a project runs to check itself and does not ship. The context is a fact about the file, -shown in the finding's metadata; what it changes, if anything, is the rule's decision, or -a refiner plugin's.""" +"""Where the code of a finding lives, by its path in the project: test code, which a +project runs to check itself and does not ship (#244), and vendored code, someone else's +kept in the project (#243). The context is a fact about the file, shown in the finding's +metadata; what it changes, if anything, is the rule's decision, or a refiner plugin's.""" from __future__ import annotations @@ -9,6 +9,9 @@ from pathlib import PurePosixPath TEST = "test" +VENDORED = "vendored" + +_VENDORED_DIRECTORIES = frozenset({"third_party", "third-party", "vendor", "vendored"}) _TEST_DIRECTORIES = frozenset({"test", "tests", "testing", "fixtures", "__tests__"}) _TEST_FILE = re.compile(r"^(test_.*|.*_test|conftest)\.py$") @@ -25,8 +28,11 @@ def is_test_code(path: str) -> bool: def code_context(path: str) -> str | None: - """The context a finding at ``path`` carries: ``"test"`` for test code, ``None`` for - anything else. ``path`` is relative to the project root, so where the project sits - does not count.""" + """The context a finding at ``path`` carries: ``"vendored"`` under a vendored + directory, the test code of a vendored project included, ``"test"`` for test code, + ``None`` for anything else. ``path`` is relative to the project root, so where the + project sits does not count.""" + if any(part in _VENDORED_DIRECTORIES for part in PurePosixPath(path).parts[:-1]): + return VENDORED return TEST if is_test_code(path) else None diff --git a/src/coretrace_python/reporters/sarif.py b/src/coretrace_python/reporters/sarif.py index 17f2a97..eb21b98 100644 --- a/src/coretrace_python/reporters/sarif.py +++ b/src/coretrace_python/reporters/sarif.py @@ -23,7 +23,7 @@ # a result exports: other metadata, such as evidence, may quote what the analysed code # holds. _INJECTIONS = frozenset({"option", "command"}) -_CONTEXTS = frozenset({"test", "example"}) +_CONTEXTS = frozenset({"test", "example", "vendored"}) def _artifact(report: Report, path: str) -> dict[str, str]: diff --git a/tests/test_vendored_context.py b/tests/test_vendored_context.py new file mode 100644 index 0000000..0465030 --- /dev/null +++ b/tests/test_vendored_context.py @@ -0,0 +1,115 @@ +"""Acceptance tests for issue #243: vendored code is still analysed, and its findings say +where they are. + +Code a project keeps under a ``third_party``, ``third-party``, ``vendor`` or ``vendored`` +directory is someone else's: its findings carry ``context: vendored`` (the context of +#244), in the JSON report and in SARIF, over any other context, test code of the vendored +project included. Nothing is excluded: the findings and the flows through vendored +modules stay as they were. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.findings import Finding +from coretrace_python.findings.context import code_context +from coretrace_python.reporters.sarif import render_sarif + +XML_TEST = """\ +from xml.dom import minidom + +PASSWORD = "Zx81kQpLw0RtY7vB" +EXPECTED = "" + + +def expected_document(): + return minidom.parseString(EXPECTED) +""" + +RUNNER = "import os\n\n\ndef run(cmd):\n os.system(cmd)\n" +VIEWS = ( + "from flask import request\n\nfrom third_party.tools.runner import run\n\n\n" + "def view():\n run(request.args['c'])\n" +) + + +@pytest.mark.parametrize( + ("path", "context"), + [ + ("third_party/gtest/test/xml_output_test.py", "vendored"), + ("third-party/lib/parse.py", "vendored"), + ("vendor/requests/api.py", "vendored"), + ("src/vendored/six.py", "vendored"), + ("tests/vendor/test_api.py", "vendored"), + ("app/vendor_api.py", None), + ("external/client.py", None), + ("tests/test_app.py", "test"), + ], +) +def test_vendored_code_is_recognised_by_its_path_in_the_project(path: str, context: str | None) -> None: + assert code_context(path) == context + + +def write(root: Path, files: dict[str, str]) -> Path: + for name, text in files.items(): + (root / name).parent.mkdir(parents=True, exist_ok=True) + (root / name).write_text(text, encoding="utf-8") + return root + + +def located(findings: tuple[Finding, ...], name: str) -> list[Finding]: + return [f for f in findings if Path(str(f.span.source_id)).as_posix().endswith(name)] + + +def test_vendored_findings_are_kept_and_carry_the_context(tmp_path: Path) -> None: + root = write( + tmp_path / "project", + {"third_party/gtest/test/xml_output_test.py": XML_TEST, "app/xml_output.py": XML_TEST}, + ) + + findings = engine.analyze_project(root, [engine.BUNDLED_PLUGINS]).findings + vendored = {f.rule_id: f for f in located(findings, "third_party/gtest/test/xml_output_test.py")} + own = {f.rule_id: f for f in located(findings, "app/xml_output.py")} + + assert set(vendored) == set(own) == {"hardcoded-credential", "unsafe-xml"} + assert {rule: f.metadata.get("context") for rule, f in vendored.items()} == dict.fromkeys(vendored, "vendored") + assert vendored["unsafe-xml"].confidence == own["unsafe-xml"].confidence + # The rule still judges the credential as test code, at low confidence; the vendored + # context is the one shown. + assert vendored["hardcoded-credential"].confidence.value == "low" + + +def test_a_flow_through_a_vendored_module_is_still_reported(tmp_path: Path) -> None: + root = write( + tmp_path / "project", + { + "third_party/__init__.py": "", + "third_party/tools/__init__.py": "", + "third_party/tools/runner.py": RUNNER, + "app/__init__.py": "", + "app/views.py": VIEWS, + }, + ) + + findings = engine.analyze_project(root, [engine.BUNDLED_PLUGINS]).findings + flows = [f for f in findings if f.rule_id == "command-injection"] + + assert [(Path(str(f.span.source_id)).name, f.span.start_line) for f in flows] == [("views.py", 7)] + assert "context" not in flows[0].metadata + + +def test_the_sarif_result_carries_the_vendored_context(tmp_path: Path) -> None: + root = write(tmp_path / "project", {"vendor/lib/xml_output.py": XML_TEST}) + analysis = engine.analyze_project(root, [engine.BUNDLED_PLUGINS]) + + results = json.loads(render_sarif(engine.report(analysis.findings, root=root)))["runs"][0]["results"] + + assert {r["ruleId"]: r["properties"]["context"] for r in results} == { + "hardcoded-credential": "vendored", + "unsafe-xml": "vendored", + }