From 2c599c2f5a799afae9eb7c658299f29ba338abcb Mon Sep 17 00:00:00 2001 From: Hugo Date: Wed, 14 Oct 2026 19:51:31 +0200 Subject: [PATCH] fix(secrets): judge test code on a file's path in the project Closes #253 --- docs/plugins.md | 4 + docs/usage.md | 2 +- .../secrets/config_secrets/config_secrets.py | 3 +- src/coretrace_python/engine.py | 44 ++++----- src/coretrace_python/findings/context.py | 15 ++- src/coretrace_python/plugins/api.py | 19 +++- src/coretrace_python/plugins/secrets.py | 16 +++- tests/test_credential_project_path.py | 91 +++++++++++++++++++ 8 files changed, 159 insertions(+), 35 deletions(-) create mode 100644 tests/test_credential_project_path.py diff --git a/docs/plugins.md b/docs/plugins.md index 16872fe..7e64e04 100644 --- a/docs/plugins.md +++ b/docs/plugins.md @@ -324,6 +324,10 @@ any analysis result: `SSAAnalysis` (`coretrace_python.ir.ssa`) for the code, `TaintAnalysis` (`coretrace_python.taint`) for the flows, `RefutationAnalysis` (`coretrace_python.findings.refutation`) for their verdicts, `DependencyAnalysis` (`coretrace_python.dependency`) for the requirements. List what you read in `requires`. +`ctx.path(source_id)` is the path of a file from the root of the analysis, the one +`coretrace_python.findings.context` classifies (`is_test_code`, `code_context`, +`code_origin`): judge test code on it, never on the absolute path, which depends on where +the project sits. It is `None` for a file outside the root. ## Findings diff --git a/docs/usage.md b/docs/usage.md index fb118ba..9e8a37f 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -221,7 +221,7 @@ sources and in configuration files. | Rule | Confidence | Trigger | |---|---|---| | `hardcoded-secret` | high | A provider-specific format: AWS, GitHub, Slack, Stripe, Google, private keys, JWTs, SendGrid, Twilio. | -| `hardcoded-credential` | medium | A credential-like name (`password`, `token`, `api_key`, `app.config['SECRET_KEY']`, …) bound to a real value. Placeholders are excluded. Low confidence in test code, fixtures and template files such as `.env.example`, with the context in the metadata. | +| `hardcoded-credential` | medium | A credential-like name (`password`, `token`, `api_key`, `app.config['SECRET_KEY']`, …) bound to a real value. Placeholders are excluded. Low confidence in test code, fixtures and template files such as `.env.example`, with the context in the metadata, judged on the file's path in the project, not on where the project sits. | | `high-entropy-string` | low | An opaque high-entropy token. Hex digests (MD5, SHA-1, SHA-256, SHA-512 lengths, or a name such as `hash`, `checksum`, `commit`), subresource-integrity hashes (`sha512-…`) and character-set constants are not secrets unless a credential name says so. | ### Dependencies diff --git a/src/coretrace_python/bundled/secrets/config_secrets/config_secrets.py b/src/coretrace_python/bundled/secrets/config_secrets/config_secrets.py index 1b8374e..aaea3d3 100644 --- a/src/coretrace_python/bundled/secrets/config_secrets/config_secrets.py +++ b/src/coretrace_python/bundled/secrets/config_secrets/config_secrets.py @@ -9,6 +9,7 @@ from typing import ClassVar from coretrace_python.findings import Finding +from coretrace_python.findings.context import path_in_project from coretrace_python.plugins import PluginContext, ProjectContext, ProjectPlugin from coretrace_python.plugins.secrets import ( DEFAULT_CREDENTIAL_NAMES, @@ -32,7 +33,7 @@ def analyze_project(self, ctx: ProjectContext) -> Sequence[Finding]: return () findings: list[Finding] = [] for value, name, span, function in config_literals(ctx.root): - finding = self.judge(value, name, span, function) + finding = self.judge(value, name, span, function, path=path_in_project(span.source_id, ctx.root)) if finding is not None: findings.append(finding) return findings diff --git a/src/coretrace_python/engine.py b/src/coretrace_python/engine.py index 82b3f11..c7f1648 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, code_origin +from coretrace_python.findings.context import code_context, code_origin, path_in_project from coretrace_python.findings.refutation import RefutationAnalysis from coretrace_python.findings.suppressions import partition from coretrace_python.frontend import HIRBuildError, ParseError, build_hir @@ -342,11 +342,9 @@ def analyze_file(source: SourceFile, plugin_roots: Sequence[Path], root: Path | manager.provide(ClearingAnalysis, table.clearing()) manager.provide(ProjectSummaries, SummaryIndex()) manager.provide(RegisteredRoutes, _routes_of(manager)) - findings, supported = _check_module(manager, tuple(loaded.plugin for loaded in registry)) - if source.path is None: - path: str | None = Path(str(source.source_id)).as_posix() - else: - path = _path_from(source.path, source.path.resolve().parent if root is None else root) + reference = None if source.path is None else root or source.path.resolve().parent + findings, supported = _check_module(manager, tuple(loaded.plugin for loaded in registry), reference) + path = path_in_project(source.source_id, reference) findings = tuple(_situated(f, path if f.span.source_id == source.source_id else None) for f in findings) functions = len(analyzable_functions(manager.module)) coverage = Coverage((FileCoverage(str(source.source_id), "analysed", functions, len(supported)),)) @@ -551,7 +549,9 @@ def analyze_project( for component in pending: batch = {name: analysable[name] for name in sorted(component)} computed.update( - _analyse_managers(batch, _seed(results, graph, component), module_plugins, affected, suffixes) + _analyse_managers( + batch, _seed(results, graph, component), module_plugins, affected, suffixes, root + ) ) for manager in batch.values(): manager.run(ResultsEvicted) @@ -613,7 +613,7 @@ def analyze_project( if isinstance(plugin, ProjectPlugin): findings.extend(plugin.analyze_project(context)) # The context comes first, so that a refiner can read it. - findings = [_situated(finding, _path_from(finding.span.source_id, root)) for finding in findings] + findings = [_situated(finding, path_in_project(finding.span.source_id, root)) for finding in findings] for plugin in all_plugins: if isinstance(plugin, ProjectPlugin): findings = list(apply_refinement(plugin, findings, plugin.refine(context, tuple(findings)))) @@ -698,16 +698,6 @@ def _located(path: Path, root: Path) -> str: return str(path) -def _path_from(source_id: object, root: Path) -> str | None: - """The path of the file ``source_id`` names from ``root``, the root of the report; - ``None`` for a file outside it.""" - - try: - return Path(str(source_id)).resolve().relative_to(root.resolve()).as_posix() - except ValueError: - return None - - def _situated(finding: Finding, path: str | None) -> Finding: """``finding`` with the context (``code_context``) and the origin (``code_origin``) of its code in its metadata, judged on ``path``, its file's path from the root of the @@ -832,6 +822,7 @@ def _analyse_managers( plugins: tuple[Plugin, ...], affected: Affected, suffixes: SuffixIndex, + root: Path, ) -> dict[str, CachedModule]: """Analyse one component: iterate its summaries to a fixpoint over ``seed`` (§21), then extract what the rest of the run needs from each module. Each member registers @@ -860,7 +851,7 @@ def _analyse_managers( for manager in managers.values(): manager.run(ProjectSummariesUpdated) manager.provide(ProjectSummaries, index) - return {name: _analyse_module(manager, plugins, affected, suffixes) for name, manager in managers.items()} + return {name: _analyse_module(manager, plugins, affected, suffixes, root) for name, manager in managers.items()} @dataclass(frozen=True) @@ -903,6 +894,7 @@ class _Worker: affected: Affected suffixes: SuffixIndex spool: ProjectCache + root: Path read: dict[str, CachedModule] = field(default_factory=dict) def entries(self, imported: Mapping[str, tuple[str, str]]) -> dict[str, CachedModule]: @@ -964,6 +956,7 @@ def managed(manager: AnalysisManager) -> None: affected, suffix_index(affected, project.modules), ProjectCache(project.spool), + project.root, ) @@ -975,7 +968,7 @@ def _analyse_batch(batch: _Batch) -> dict[str, dict[str, Any]]: for manager in managers.values(): worker.managed(manager) seed = _index(worker.entries(batch.imported), {name: import_name for name, (import_name, _) in batch.imported.items()}) - results = _analyse_managers(managers, seed, worker.plugins, worker.affected, worker.suffixes) + results = _analyse_managers(managers, seed, worker.plugins, worker.affected, worker.suffixes, worker.root) return {name: encode(entry) for name, entry in results.items()} @@ -1025,12 +1018,12 @@ def _configuration_key( def _analyse_module( - manager: AnalysisManager, plugins: tuple[Plugin, ...], affected: Affected, suffixes: SuffixIndex + manager: AnalysisManager, plugins: tuple[Plugin, ...], affected: Affected, suffixes: SuffixIndex, root: Path ) -> CachedModule: """One module's findings, summaries, call sites and symbol reads: what the cache keeps (§11).""" - findings, supported = _check_module(manager, plugins) + findings, supported = _check_module(manager, plugins, root) graph = manager.get(CallGraphAnalysis) correlated: list[Finding] = [] if affected: @@ -1067,10 +1060,11 @@ def _summaries_of(manager: AnalysisManager) -> dict[str, FunctionSummary]: def _check_module( - manager: AnalysisManager, plugins: tuple[Plugin, ...] + manager: AnalysisManager, plugins: tuple[Plugin, ...], root: Path | None ) -> tuple[tuple[Finding, ...], tuple[nodes.Function, ...]]: """Findings of the module plugins plus notes for unsupported functions, and the - functions that could be analysed.""" + functions that could be analysed; ``root`` is the root of the analysis the plugins + read a file's path in the project from (``PluginContext.path``).""" supported: list[nodes.Function] = [] notes: list[Finding] = [] @@ -1090,7 +1084,7 @@ def _check_module( ) else: supported.append(function) - findings = run_plugins(manager, plugins, tuple(supported)) + findings = run_plugins(manager, plugins, tuple(supported), root) return (*findings, *notes), tuple(supported) diff --git a/src/coretrace_python/findings/context.py b/src/coretrace_python/findings/context.py index 772b6c5..54cee9e 100644 --- a/src/coretrace_python/findings/context.py +++ b/src/coretrace_python/findings/context.py @@ -7,7 +7,7 @@ from __future__ import annotations import re -from pathlib import PurePosixPath +from pathlib import Path, PurePosixPath TEST = "test" VENDORED = "vendored" @@ -42,3 +42,16 @@ def code_origin(path: str) -> str | None: project's own code.""" return VENDORED if any(part in _VENDORED_DIRECTORIES for part in PurePosixPath(path).parts[:-1]) else None + + +def path_in_project(source_id: object, root: Path | None) -> str | None: + """The path of the file ``source_id`` names from ``root``, the root of the analysis, + as the classifications above read it; ``None`` for a file outside ``root``. Without a + root, the source is taken as named: a source that is no file has no other path.""" + + if root is None: + return Path(str(source_id)).as_posix() + try: + return Path(str(source_id)).resolve().relative_to(root.resolve()).as_posix() + except ValueError: + return None diff --git a/src/coretrace_python/plugins/api.py b/src/coretrace_python/plugins/api.py index 5b5ff8e..ca6370c 100644 --- a/src/coretrace_python/plugins/api.py +++ b/src/coretrace_python/plugins/api.py @@ -24,6 +24,7 @@ from coretrace_python.analysis.provider import R from coretrace_python.dependency import Advisory, DependencyGraph, Policy from coretrace_python.findings import Finding +from coretrace_python.findings.context import path_in_project from coretrace_python.hir import nodes from coretrace_python.interprocedural import ( CallGraph, @@ -171,17 +172,27 @@ def apply_refinement(plugin: Plugin, original: Sequence[Finding], refined: Seque class PluginContext: - """What one plugin may see while analysing one module.""" + """What one plugin may see while analysing one module. ``root`` is the root of the + analysis, when the engine knows one: a file's path in the project is read from it.""" def __init__( self, manager: AnalysisManager, plugin: Plugin, functions: tuple[nodes.Function, ...] | None = None, + root: Path | None = None, ) -> None: self._manager = manager self._plugin = plugin self._functions = analyzable_functions(manager.module) if functions is None else functions + self._root = root + + def path(self, source_id: object) -> str | None: + """The path of the file ``source_id`` names from the root of the analysis, what + tells test code apart (``findings.context``), not where the project sits; ``None`` + for a file outside it (#253).""" + + return path_in_project(source_id, self._root) @property def module(self) -> nodes.Module: @@ -215,10 +226,12 @@ def run_plugins( manager: AnalysisManager, plugins: Iterable[Plugin], functions: tuple[nodes.Function, ...] | None = None, + root: Path | None = None, ) -> tuple[Finding, ...]: - """Run every plugin against one shared manager and concatenate their findings.""" + """Run every plugin against one shared manager and concatenate their findings; + ``root`` is the root of the analysis (``PluginContext.path``).""" findings: list[Finding] = [] for plugin in plugins: - findings.extend(plugin.analyze(PluginContext(manager, plugin, functions))) + findings.extend(plugin.analyze(PluginContext(manager, plugin, functions, root))) return tuple(findings) diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 5896d2d..16af8ab 100644 --- a/src/coretrace_python/plugins/secrets.py +++ b/src/coretrace_python/plugins/secrets.py @@ -503,9 +503,12 @@ class SecretDetector(Plugin): def analyze(self, ctx: PluginContext) -> Sequence[Finding]: findings: list[Finding] = [] + path = ctx.path(ctx.module.span.source_id) for literal, pieces in literal_groups(ctx.module): - found = self.judge(*literal) - parts = [finding for finding in (self.judge(*piece) for piece in pieces) if finding is not None] + found = self.judge(*literal, path=path) + parts = [ + finding for finding in (self.judge(*piece, path=path) for piece in pieces) if finding is not None + ] if found is None: findings.extend(parts) else: @@ -516,7 +519,12 @@ def analyze(self, ctx: PluginContext) -> Sequence[Finding]: findings.append(strongest if strongest is found else replace(strongest, span=found.span)) return findings - def judge(self, value: str, name: str | None, span: Location, function: str | None) -> Finding | None: + def judge( + self, value: str, name: str | None, span: Location, function: str | None, path: str | None = None + ) -> Finding | None: + """The finding for one literal, if any. ``path`` is its file's path in the project, + which tells test code and template files apart; ``None`` tells neither (#253).""" + where = f"in {name}" if name is not None else "in a string literal" for pattern in self.patterns: if pattern.matches(value): @@ -537,7 +545,7 @@ def judge(self, value: str, name: str | None, span: Location, function: str | No ): # A password in a test fixture or a template file is rarely a leak; a name # is a hint, so the finding stays, at low confidence. - context = credential_context(str(span.source_id)) + context = None if path is None else credential_context(path) metadata = {"name": name, "length": str(len(value))} if context is not None: metadata["context"] = context diff --git a/tests/test_credential_project_path.py b/tests/test_credential_project_path.py new file mode 100644 index 0000000..58ac372 --- /dev/null +++ b/tests/test_credential_project_path.py @@ -0,0 +1,91 @@ +"""Acceptance tests for issue #253: test code is judged on a file's path in the project, +not on where the project sits. + +``hardcoded-credential`` and ``config-secrets`` lower the confidence of a credential in +test code or in a template file. They used to judge the absolute path, so a project +under a directory named ``tests`` had every credential at low confidence, as test code. +They now judge the path from the root of the analysis, as the context of #244 does, in +every way the engine runs them: one process, worker processes, a file checked alone. +""" + +from __future__ import annotations + +import contextlib +import io +import json +from pathlib import Path + +import pytest + +from coretrace_python import engine +from coretrace_python.cli import main +from coretrace_python.findings import Confidence, Finding +from coretrace_python.source import SourceManager + +PASSWORD = 'PASSWORD = "Zx81kQpLw0RtY7vB"\n' +YAML = "database:\n password: Zx81kQpLw0RtY7vB\n" + + +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 credentials(findings: tuple[Finding, ...]) -> dict[str, tuple[Confidence, str | None]]: + return { + Path(str(f.span.source_id)).name: (f.confidence, f.metadata.get("context")) + for f in findings + if f.rule_id == "hardcoded-credential" + } + + +@pytest.mark.parametrize("jobs", [1, 2]) +def test_a_project_under_a_tests_directory_keeps_its_credentials_confidence(tmp_path: Path, jobs: int) -> None: + files = { + "app/settings.py": PASSWORD, + "app/other.py": "VALUE = 1\n", + "config/settings.yaml": YAML, + "tests/test_settings.py": PASSWORD, + "docker/.env.example": "DB_PASSWORD=Zx81kQpLw0RtY7vB\n", + } + root = write(tmp_path / "tests" / "project", files) + + found = credentials(engine.analyze_project(root, [engine.BUNDLED_PLUGINS], jobs=jobs).findings) + + assert found == { + "settings.py": (Confidence.MEDIUM, None), + "settings.yaml": (Confidence.MEDIUM, None), + # The project's own test code and template files keep the lower confidence. + "test_settings.py": (Confidence.LOW, "test"), + ".env.example": (Confidence.LOW, "example"), + } + + +def report(target: Path) -> dict[str, tuple[str, str | None]]: + out = io.StringIO() + with contextlib.redirect_stdout(out): + main(["--check", str(target), "--format", "json"]) + return { + Path(f["location"]["path"]).name: (f["confidence"], f["metadata"].get("context")) + for f in json.loads(out.getvalue())["findings"] + if f["rule_id"] == "hardcoded-credential" + } + + +def test_a_file_checked_alone_is_judged_from_its_directory(tmp_path: Path) -> None: + root = write(tmp_path / "tests" / "project", {"app/settings.py": PASSWORD, "app/test_settings.py": PASSWORD}) + + assert report(root / "app" / "settings.py") == {"settings.py": ("medium", None)} + assert report(root / "app" / "test_settings.py") == {"test_settings.py": ("low", "test")} + + +@pytest.mark.parametrize( + ("name", "expected"), + [("tests/settings.py", (Confidence.LOW, "test")), ("app/settings.py", (Confidence.MEDIUM, None))], +) +def test_a_source_that_is_no_file_is_judged_on_its_name(name: str, expected: tuple[Confidence, str | None]) -> None: + findings = engine.check(SourceManager().add_source(name, PASSWORD), [engine.BUNDLED_PLUGINS]) + + assert credentials(findings) == {"settings.py": expected}