diff --git a/docs/usage.md b/docs/usage.md index 4ca5464..6aca604 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -228,21 +228,30 @@ coretrace-python-analyzer --check src/ --baseline coretrace-baseline.json The first run writes the file with every current finding and passes. Later runs set the recorded findings apart and fail only on new ones. A finding is recognised by its file, -its rule, its function and the text of its line, not by its line number, so code inserted -above it does not make it new; a change to the line itself does. Commit the file and -shrink it as findings are fixed: an entry without a matching finding is simply unused. +its rule, its function and the text of every line it covers, not by its line number, so +code inserted above it does not make it new; a change to its text does, on any of its +lines, such as the body of a key written over several lines. Commit the file and shrink +it as findings are fixed: an entry without a matching finding is simply unused. Baselined findings are counted in the text report, listed under `baselined` in the JSON report and marked `baselineState: unchanged` in the SARIF log, where new results are marked `new`. A finding with a file location is recognised by the JSON pointer of its -value and a digest of the value (never the value itself) instead of the text of a line, -so a new value at the same place is a new finding. - -Baselines are written in schema 2. A schema 1 file, written by an earlier version, is -still read and its entries match exactly as they did; but it recorded a finding in a -JSON, TOML or lock file with the text of its key's first line, which was its own only -when the key was not repeated. A finding such an entry cannot identify with certainty is -reported as new, and the check then warns on standard error: review the new findings, -delete the baseline and run the check again to record it in schema 2. +value and its value instead of the text of a line, so a new value at the same place is a +new finding. Entries hold a digest of that text, value and pointer, never the text +itself, so no accepted secret is stored in plain text. That does not make the file +confidential: a digest does not hide a weak secret from whoever guesses it, since a +guessed password, hashed with its line, can be checked against the file. Treat the +baseline as a possibly sensitive file. + +Baselines are written in schema 3. A file written by an earlier version, in schema 1 or +2, is still read and never rewritten, and the check warns on standard error, once per +run, that it may contain secrets in plain text: its entries hold the text of each +finding's first line. They match exactly as they did a finding on a single line; a +finding over several lines, whose other lines they do not hold, is reported as new, and +so is a finding in a JSON, TOML or lock file that a schema 1 entry, recorded with the +text of its key's first line, cannot identify with certainty. The exit status depends +on the findings only. Review such a baseline and its new findings, then migrate it +explicitly to schema 3: delete it and run the check again. The old file's secrets +remain in the history of a repository that committed it. ## Reports diff --git a/src/coretrace_python/cli.py b/src/coretrace_python/cli.py index 2a0bccb..1408c72 100644 --- a/src/coretrace_python/cli.py +++ b/src/coretrace_python/cli.py @@ -276,7 +276,7 @@ def main(argv: list[str] | None = None) -> int: if args.baseline.is_file(): baseline = Baseline.load(args.baseline) findings, baselined = baseline.partition(findings, root) - notice = baseline.transition_notice(findings) + notice = baseline.transition_notice() if notice is not None: print(f"coretrace: {args.baseline}: {notice}", file=sys.stderr) else: diff --git a/src/coretrace_python/findings/baseline.py b/src/coretrace_python/findings/baseline.py index 11dae3a..1fa0ae9 100644 --- a/src/coretrace_python/findings/baseline.py +++ b/src/coretrace_python/findings/baseline.py @@ -1,38 +1,44 @@ """The baseline: findings accepted at a point in time, so only new ones fail a check. -A finding is recognised by its file relative to the root, its rule, its function and the -text of its line, never by its line number: code inserted above it does not make it -new, a change to the line itself does. A finding with a file location, whose line is not -established, is recognised by the JSON pointer of its value and a digest of the value -instead, never the value itself: a new value there is a new finding. Entries are -counted, so two identical findings on identical lines need two entries. - -Baselines are written in schema 2, whose entries carry the pointer. Schema 1 files are -still read, and their entries match exactly as they did. There, a finding in a JSON, TOML -or lock file was recorded with the text of its key's first line, its own only when the -key was not repeated: such an entry still identifies that finding, and no other. A -finding it cannot identify with certainty is new, and ``transition_notice`` asks for the -baseline to be recorded again. +A finding is recognised by its file relative to the root, its rule, its function and a +digest of the text of every line its span covers, never by its line number: code +inserted above it does not make it new, a change to its text does, on any of its lines, +as the body of a secret written over several lines (#223). A finding with a file +location, whose line is not established, is recognised by the JSON pointer of its value +and the value instead. An entry holds digests of them only, never the text, the value or +the pointer, whose keys may be secrets too, so the file, which is meant to be committed, +holds no accepted secret (#224). Entries are counted, so two identical findings on +identical lines need two entries. + +Baselines are written in schema 3. Schema 1 and 2 files are still read, and never +rewritten; their entries hold the text of a finding's first line, which identifies a +finding on a single line exactly as it did, and no finding over several lines, whose +other lines it does not hold: such a finding is new. Schema 1 also recorded a finding +in a JSON, TOML or lock file with the text of its key's first line, its own only when +the key was not repeated: such an entry still identifies that finding, and no other. +Reading an old baseline, the check warns, with ``transition_notice``, that it may hold +secrets in plain text and is to be reviewed and recorded again. """ from __future__ import annotations import json from collections import Counter -from collections.abc import Iterable +from collections.abc import Callable, Iterable from dataclasses import dataclass, field from pathlib import Path from coretrace_python.findings.model import Finding from coretrace_python.source import SourceSpan, decode_text, lines_of +from coretrace_python.source.positions import text_digest -BASELINE_SCHEMA = 2 -_LEGACY_SCHEMA = 1 -# Files whose findings schema 1 placed at their key's first line. -_STRUCTURED = (".json", ".toml", ".lock") +BASELINE_SCHEMA = 3 +# Schemas whose entries hold the text of a finding's first line. +_LEGACY_SCHEMAS = (1, 2) -# File, rule, function, text of the line (for a file location, the digest of its -# value), pointer (empty for a line). +# File, rule, function, digest of the text of the span's lines (of the value, for a +# file location), digest of the pointer (empty for a span). In schemas 1 and 2, the text +# of the span's first line and the pointer itself. Fingerprint = tuple[str, str, str, str, str] @@ -41,35 +47,56 @@ class BaselineError(Exception): def fingerprint(finding: Finding, root: Path) -> Fingerprint: + path, head = _head(finding, root) + span = finding.span + if isinstance(span, SourceSpan): + return (*head, text_digest(_span_text(path, span)), "") + # A key above the value may itself be a secret. + return (*head, span.digest, text_digest(span.pointer)) + + +def legacy_fingerprint(finding: Finding, root: Path) -> Fingerprint | None: + """The fingerprint of ``finding`` as schemas 1 and 2 recorded it, or None for a + finding over several lines, which an entry of those schemas cannot identify.""" + + path, head = _head(finding, root) + span = finding.span + if not isinstance(span, SourceSpan): + return (*head, span.digest, span.pointer) + if span.end_line not in (None, span.start_line): + return None + return (*head, _span_text(path, span), "") + + +def _head(finding: Finding, root: Path) -> tuple[Path, tuple[str, str, str]]: path = Path(str(finding.span.source_id)) try: relative = path.relative_to(root).as_posix() except ValueError: relative = path.as_posix() - head = (relative, finding.rule_id, finding.function or "") - if isinstance(finding.span, SourceSpan): - return (*head, _line_text(path, finding.span.start_line), "") - return (*head, finding.span.digest, finding.span.pointer) + return path, (relative, finding.rule_id, finding.function or "") -def _line_text(path: Path, line: int) -> str: - """The text of ``line``, lines counted as every line number of the engine counts - them (``lines_of``).""" +def _span_text(path: Path, span: SourceSpan) -> str: + """The text of the lines ``span`` covers, each stripped, lines counted as every line + number of the engine counts them (``lines_of``); the line number when the file + cannot be read or does not reach it.""" try: lines = lines_of(decode_text(path.read_bytes())) except (OSError, UnicodeDecodeError): - return str(line) - if not 1 <= line <= len(lines): - return str(line) - return lines[line - 1].strip() + return str(span.start_line) + if span.start_line > len(lines): + return str(span.start_line) + covered = lines[span.start_line - 1 : span.end_line or span.start_line] + return "\n".join(line.strip() for line in covered) @dataclass(frozen=True) class Baseline: entries: Counter[Fingerprint] = field(default_factory=Counter) - # Read from a schema 1 file, whose entries on structured files may no longer match. - legacy: bool = False + # The schema of the file read; its entries are compared by that schema's fingerprint. + schema: int = BASELINE_SCHEMA @classmethod def of(cls, findings: Iterable[Finding], root: Path) -> Baseline: @@ -80,7 +107,7 @@ def load(cls, path: Path) -> Baseline: try: document = json.loads(path.read_text(encoding="utf-8")) schema = document.get("schema") - if schema not in (_LEGACY_SCHEMA, BASELINE_SCHEMA): + if schema not in (*_LEGACY_SCHEMAS, BASELINE_SCHEMA): raise BaselineError(f"{path}: unsupported baseline schema {schema!r}") entries = Counter( { @@ -88,7 +115,7 @@ def load(cls, path: Path) -> Baseline: str(e["path"]), str(e["rule"]), str(e["function"]), - str(e.get("line", "")), + str(e["digest"] if schema == BASELINE_SCHEMA else e.get("line", "")), str(e.get("pointer", "")), ): int(e["count"]) for e in document["findings"] @@ -98,14 +125,14 @@ def load(cls, path: Path) -> Baseline: raise except (OSError, ValueError, KeyError, TypeError, AttributeError) as error: raise BaselineError(f"{path}: {error}") from error - return cls(entries, legacy=schema == _LEGACY_SCHEMA) + return cls(entries, schema) def save(self, path: Path) -> None: document = { "schema": BASELINE_SCHEMA, "findings": [ - {"path": p, "rule": r, "function": f, "line": line, "pointer": pointer, "count": count} - for (p, r, f, line, pointer), count in sorted(self.entries.items()) + {"path": p, "rule": r, "function": f, "digest": digest, "pointer": pointer, "count": count} + for (p, r, f, digest, pointer), count in sorted(self.entries.items()) ], } path.write_text(json.dumps(document, indent=2) + "\n", encoding="utf-8") @@ -115,28 +142,33 @@ def partition( ) -> tuple[tuple[Finding, ...], tuple[Finding, ...]]: """The findings not in the baseline, and those it accounts for.""" + key_of: Callable[[Finding, Path], Fingerprint | None] = ( + fingerprint if self.schema == BASELINE_SCHEMA else legacy_fingerprint + ) remaining = Counter(self.entries) new: list[Finding] = [] baselined: list[Finding] = [] for finding in findings: - key = fingerprint(finding, root) - if remaining[key] > 0: + key = key_of(finding, root) + if key is not None and remaining[key] > 0: remaining[key] -= 1 baselined.append(finding) else: new.append(finding) return tuple(new), tuple(baselined) - def transition_notice(self, new: Iterable[Finding]) -> str | None: - """What to tell when a schema 1 baseline leaves findings of JSON, TOML or lock - files new: their old entries may name another line, so the baseline is to be - recorded again; None otherwise.""" + def transition_notice(self) -> str | None: + """What to tell when the baseline was read from schema 1 or 2, whose entries hold + the text of lines and identify only findings on a single line: it is to be + reviewed and recorded again; None for a schema 3 baseline.""" - if not self.legacy or not any(str(finding.span.source_id).endswith(_STRUCTURED) for finding in new): + if self.schema == BASELINE_SCHEMA: return None return ( - "schema 1 baseline: findings in JSON, TOML and lock files are now located at their own " - "line or by their pointer, and an old entry that cannot identify one with certainty no " - "longer accounts for it; review the new findings, then delete the baseline and run the " - "check again to record it in schema 2" + f"schema {self.schema} baseline: this baseline may contain secrets in plain text. Its " + "entries identify only findings on a single line, so a finding over several lines, or " + "in a JSON, TOML or lock file, may be reported new only because no old entry can " + "identify it with certainty. Review the baseline and the new findings, then migrate it " + f"explicitly to schema {BASELINE_SCHEMA}: delete it and run the check again, which " + "records digests only" ) diff --git a/src/coretrace_python/plugins/secrets.py b/src/coretrace_python/plugins/secrets.py index 21eb72c..f4cd1e1 100644 --- a/src/coretrace_python/plugins/secrets.py +++ b/src/coretrace_python/plugins/secrets.py @@ -17,6 +17,7 @@ import math import re import tomllib +from bisect import bisect_right from collections import Counter from collections.abc import Iterator, Sequence from dataclasses import dataclass @@ -347,7 +348,10 @@ def walk(node: object, path: Pointer, key: str | None, named: bool = True) -> It def _pairs(source: SourceId, text: str) -> Iterator[Literal]: - for number, line in enumerate(lines_of(text), start=1): + lines = lines_of(text) + # The numbers of the lines holding each quote, to find where an open quote closes. + quoted = {quote: [n for n, line in enumerate(lines, start=1) if quote in line] for quote in "'\""} + for number, line in enumerate(lines, start=1): stripped = line.strip() if not stripped or stripped[0] in "#;[": continue @@ -357,10 +361,45 @@ def _pairs(source: SourceId, text: str) -> Iterator[Literal]: key, value = match.group(1), match.group(2) if value.startswith("#"): continue + last = _value_end(lines, quoted, number, match.start(1), value) if len(value) >= 2 and value[0] == value[-1] and value[0] in "'\"": value = value[1:-1] if value: - yield value, key, SourceSpan(source, number, 1), None + span = ( + SourceSpan(source, number, 1) + if last == number + else SourceSpan(source, number, 1, last, len(lines[last - 1]) + 1) + ) + yield value, key, span, None + + +def _value_end(lines: list[str], quoted: dict[str, list[int]], number: int, indent: int, value: str) -> int: + """The last line of the value written on line ``number`` after a key at column + ``indent``: a value may continue on the lines that close a quote it leaves open + (``.env``, YAML), after a trailing backslash (properties), or on lines indented + deeper than its key (YAML, INI). A later line is counted in when it may continue + the value, so a change to it is a change to the value's text.""" + + last = number + quote = value[:1] + if quote in ("'", '"') and quote not in value[1:]: + closing = quoted[quote] + index = bisect_right(closing, number) + last = closing[index] if index < len(closing) else len(lines) + while last < len(lines): + if lines[last - 1].rstrip().endswith("\\"): + last += 1 + continue + following = last + 1 + while following <= len(lines) and not lines[following - 1].strip(): + following += 1 + if following > len(lines): + break + line = lines[following - 1] + if len(line) - len(line.lstrip()) <= indent: + break + last = following + return last def redacted(value: str) -> str: diff --git a/src/coretrace_python/source/positions.py b/src/coretrace_python/source/positions.py index ef620fe..f5f8dc9 100644 --- a/src/coretrace_python/source/positions.py +++ b/src/coretrace_python/source/positions.py @@ -39,8 +39,13 @@ def file_location(source_id: SourceId, path: Pointer, value: str) -> FileLocatio pointer, and a digest of the value, so that a baseline tells a changed value apart without recording it.""" - digest = hashlib.sha256(value.encode("utf-8", "surrogatepass")).hexdigest()[:16] - return FileLocation(source_id, pointer_text(path), f"sha256:{digest}") + return FileLocation(source_id, pointer_text(path), text_digest(value)) + + +def text_digest(text: str) -> str: + """A short digest of ``text``, which tells a changed text apart without holding it.""" + + return "sha256:" + hashlib.sha256(text.encode("utf-8", "surrogatepass")).hexdigest()[:16] def pointer_text(path: Pointer) -> str: diff --git a/tests/test_baseline.py b/tests/test_baseline.py index 90d37d9..1270ecb 100644 --- a/tests/test_baseline.py +++ b/tests/test_baseline.py @@ -20,6 +20,7 @@ from coretrace_python.cli import main from coretrace_python.findings import Confidence, Finding, Severity from coretrace_python.source import SourceId, SourceSpan +from coretrace_python.source.positions import text_digest try: from coretrace_python.findings.baseline import Baseline, BaselineError, fingerprint @@ -56,13 +57,13 @@ def test_fingerprint_ignores_the_line_number_but_not_the_line_text(tmp_path: Pat source.write_text(EVAL.replace("eval(code)", "eval(code.strip())"), encoding="utf-8") changed = fingerprint(finding(str(source), 4), tmp_path) - assert before == after == ("app.py", "dangerous-eval", "run", "eval(code)", "") + assert before == after == ("app.py", "dangerous-eval", "run", text_digest("eval(code)"), "") assert changed != before assert fingerprint(finding(str(source), 4, "weak-crypto"), tmp_path) != before def test_fingerprint_falls_back_to_the_line_number_without_the_file(tmp_path: Path) -> None: - assert fingerprint(finding(str(tmp_path / "gone.py"), 7), tmp_path) == ("gone.py", "dangerous-eval", "run", "7", "") + assert fingerprint(finding(str(tmp_path / "gone.py"), 7), tmp_path) == ("gone.py", "dangerous-eval", "run", text_digest("7"), "") def test_baseline_round_trips_and_partitions_with_counts(tmp_path: Path) -> None: @@ -75,7 +76,7 @@ def test_baseline_round_trips_and_partitions_with_counts(tmp_path: Path) -> None loaded = Baseline.load(path) new, baselined = loaded.partition((first, second), tmp_path) - assert json.loads(path.read_text(encoding="utf-8"))["schema"] == 2 + assert json.loads(path.read_text(encoding="utf-8"))["schema"] == 3 assert baselined == (first,) and new == (second,) diff --git a/tests/test_baseline_digests.py b/tests/test_baseline_digests.py new file mode 100644 index 0000000..6952fd6 --- /dev/null +++ b/tests/test_baseline_digests.py @@ -0,0 +1,155 @@ +"""Acceptance tests for issues #223 and #224: what a baseline entry records. + +An entry recognises a finding by a digest of the text of every line its span covers, +never by that text: a secret written over several lines, whose first line does not hold +it, is new once its body changes (#223), and the baseline file, which is meant to be +committed, holds no accepted secret in plain text (#224). Baselines are written in +schema 3. Schema 1 and 2 files are still read and never rewritten; their entries, which +hold the text of a finding's first line, identify only a finding on a single line, and +any other finding is new. +""" + +from __future__ import annotations + +import json +from pathlib import Path + +import pytest + +from coretrace_python.cli import main + +TOKEN = "ghp_" + "a1B2" * 9 +PASSWORD = "Zx81kQpLw0RtY7vBn3MsD9cF2hJ6gK4a" +BODY = "MIIEowIBAAKCAQEA1b2c3d4e5f6g7h8i9j0k1l2m3n4o5p6q7r8s9t0u1v2w3x4" +KEY = f'KEY = """\n-----BEGIN RSA PRIVATE KEY-----\n{BODY}\n-----END RSA PRIVATE KEY-----\n"""\n' +FILES = { + "keys.py": KEY, + "app.py": f"def run(code):\n eval(code)\n\nTOKEN = '{TOKEN}'\n", + ".env": f"DB_PASSWORD={PASSWORD}\n", + "config.yaml": f"service:\n api_token: {PASSWORD[::-1]}\n", + "settings.json": f'{{\n "db": {{"password": "{PASSWORD.upper()}"}}\n}}\n', +} +SECRETS = (BODY, TOKEN, PASSWORD, PASSWORD[::-1], PASSWORD.upper()) +DONE = "no findings, 6 baselined\ncoverage: 2/2 files, 3/3 functions\n" + + +@pytest.fixture +def root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + root = tmp_path / "proj" + root.mkdir() + for name, text in FILES.items(): + (root / name).write_text(text, encoding="utf-8") + monkeypatch.chdir(tmp_path) + return root + + +def check(root: Path, baseline: Path, capsys: pytest.CaptureFixture[str]) -> tuple[int, str, str]: + status = main(["--check", root.name, "--baseline", str(baseline)]) + captured = capsys.readouterr() + return status, captured.out, captured.err + + +def test_a_changed_secret_in_a_multi_line_string_is_new( + root: Path, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + baseline = tmp_path / "baseline.json" + check(root, baseline, capsys) + (root / "keys.py").write_text(KEY.replace(BODY, BODY[::-1]), encoding="utf-8") + + status, out, _ = check(root, baseline, capsys) + + assert status == 1 + assert out.startswith("keys.py:1:7: high hardcoded-secret") + assert "1 finding, 5 baselined\n" in out + + +def test_an_unchanged_multi_line_secret_stays_baselined_when_code_moves( + root: Path, tmp_path: Path, capsys: pytest.CaptureFixture[str] +) -> None: + baseline = tmp_path / "baseline.json" + check(root, baseline, capsys) + (root / "keys.py").write_text("import os\n\n\n" + KEY, encoding="utf-8") + + assert check(root, baseline, capsys)[:2] == (0, DONE) + + +def test_the_baseline_holds_no_accepted_secret_and_is_not_reported_itself( + root: Path, capsys: pytest.CaptureFixture[str] +) -> None: + """The baseline is recorded inside the project, as when it is committed with it.""" + + baseline = root / "baseline.json" + check(root, baseline, capsys) + text = baseline.read_text(encoding="utf-8") + document = json.loads(text) + + assert document["schema"] == 3 + assert not [secret for secret in SECRETS if secret in text or secret[:12] in text] + assert {"path", "rule", "function", "digest", "pointer", "count"} == set( + document["findings"][0] + ) + assert check(root, baseline, capsys)[:2] == (0, DONE) + + +def legacy(path: Path, schema: int, *entries: tuple[str, str, str, str]) -> Path: + findings = [ + {"path": file, "rule": rule, "function": function, "line": line, "pointer": "", "count": 1} + for file, rule, function, line in entries + ] + path.write_text(json.dumps({"schema": schema, "findings": findings}), encoding="utf-8") + return path + + +@pytest.mark.parametrize("schema", [1, 2]) +def test_an_old_entry_never_accounts_for_a_finding_over_several_lines( + root: Path, tmp_path: Path, capsys: pytest.CaptureFixture[str], schema: int +) -> None: + """The old entry of the key holds the text of its first line, which a changed key + keeps: it cannot identify the finding, which is new; entries of findings on a single + line still match exactly, and the file is left as it was.""" + + for name in (".env", "config.yaml", "settings.json"): + (root / name).unlink() + (root / "keys.py").write_text(KEY.replace(BODY, BODY[::-1]), encoding="utf-8") + baseline = legacy( + tmp_path / "baseline.json", + schema, + ("keys.py", "hardcoded-secret", "", 'KEY = """'), + ("app.py", "dangerous-eval", "run", "eval(code)"), + ("app.py", "hardcoded-secret", "", f"TOKEN = '{TOKEN}'"), + ) + before = baseline.read_bytes() + + status, out, err = check(root, baseline, capsys) + + assert status == 1 + assert out.startswith("keys.py:1:7: high hardcoded-secret") + assert "1 finding, 2 baselined\n" in out + assert ( + err.count(f"schema {schema} baseline: this baseline may contain secrets in plain text") == 1 + ) + assert baseline.read_bytes() == before + + +def test_a_secret_written_as_a_key_is_not_kept_in_the_pointer( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture[str] +) -> None: + """A value at a file location is recognised by its pointer, which names the keys + above it; a key may itself be a secret, so the entry holds a digest of the pointer.""" + + root = tmp_path / "proj" + root.mkdir() + (root / "app.py").write_text("x = 1\n", encoding="utf-8") + (root / "settings.toml").write_text( + f'[tokens]\n"{TOKEN}" = {{ owner = "ci", password = "{PASSWORD}" }}\n', encoding="utf-8" + ) + monkeypatch.chdir(tmp_path) + baseline = root / "baseline.json" + check(root, baseline, capsys) + text = baseline.read_text(encoding="utf-8") + + assert TOKEN not in text and PASSWORD not in text + assert check(root, baseline, capsys)[:2] == ( + 0, + "no findings, 1 baselined\ncoverage: 1/1 files, 1/1 functions\n", + ) diff --git a/tests/test_baseline_schema_1.py b/tests/test_baseline_schema_1.py index b11ce2f..8820837 100644 --- a/tests/test_baseline_schema_1.py +++ b/tests/test_baseline_schema_1.py @@ -97,4 +97,4 @@ def test_a_schema_1_baseline_of_python_findings_matches_as_before( captured = capsys.readouterr() assert captured.out.startswith("no findings, 1 baselined\n") - assert NOTICE not in captured.err + assert NOTICE in captured.err diff --git a/tests/test_config_value_spans.py b/tests/test_config_value_spans.py new file mode 100644 index 0000000..4c3c669 --- /dev/null +++ b/tests/test_config_value_spans.py @@ -0,0 +1,67 @@ +"""Acceptance tests for #223 in configuration files: a value written over several lines. + +A ``.env``, YAML, INI or properties value may continue past the line of its key: a quote +left open (``.env``, YAML), lines indented under the key (YAML, INI), a trailing +backslash (properties). Its span covers every line it continues on, so the baseline, +which digests the text of those lines, tells a changed body apart; the next key, at the +key's indentation or less, is not part of it. +""" + +from __future__ import annotations + +from pathlib import Path + +import pytest + +from coretrace_python.cli import main + +BODY = "MIIEowIBAAKCAQEA9z8y7x6w5v4u3t2s1r0q9p8o7n6m5l4k3j2i1h0g9f8e7d6" +BEGIN, END = "-----BEGIN RSA PRIVATE KEY-----", "-----END RSA PRIVATE KEY-----" + + +@pytest.fixture +def root(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + root = tmp_path / "proj" + root.mkdir() + (root / "app.py").write_text("x = 1\n", encoding="utf-8") + monkeypatch.chdir(tmp_path) + return root + + +def changed_is_new(root: Path, name: str, before: str, after: str) -> bool: + (root / name).write_text(before, encoding="utf-8") + baseline = root.parent / "baseline.json" + assert main(["--check", "proj", "--baseline", str(baseline)]) == 0 + (root / name).write_text(after, encoding="utf-8") + return main(["--check", "proj", "--baseline", str(baseline)]) == 1 + + +@pytest.mark.parametrize( + ("name", "text"), + [ + (".env", f'PRIVATE_KEY="{BEGIN}\n{BODY}\n{END}"\nOTHER=1\n'), + ("config.yaml", f'private_key: "{BEGIN}\n {BODY}\n {END}"\nother: 1\n'), + ("config.yaml", f"private_key: {BEGIN}\n {BODY}\n {END}\nother: 1\n"), + ("app.ini", f"[db]\npassword = Zx81kQpLw0RtY7vB\n {BODY}\nuser = app\n"), + ("app.properties", f"db.password=Zx81kQpLw0RtY7vB\\\n {BODY}\ndb.user=app\n"), + ], +) +def test_a_changed_body_of_a_value_over_several_lines_is_new( + root: Path, name: str, text: str, capsys: pytest.CaptureFixture[str] +) -> None: + assert changed_is_new(root, name, text, text.replace(BODY, BODY[::-1])) + + +@pytest.mark.parametrize( + ("name", "text", "after"), + [ + (".env", f'PRIVATE_KEY="{BEGIN}"\nOTHER=1\n', "OTHER=2"), + ("config.yaml", "db:\n password: Zx81kQpLw0RtY7vB\n user: app\n", " user: web"), + ("app.ini", "[db]\npassword = Zx81kQpLw0RtY7vB\nuser = app\n", "user = web"), + ], +) +def test_the_next_key_is_not_part_of_the_value( + root: Path, name: str, text: str, after: str, capsys: pytest.CaptureFixture[str] +) -> None: + following = text.splitlines()[-1] + assert not changed_is_new(root, name, text, text.replace(following, after)) diff --git a/tests/test_file_locations.py b/tests/test_file_locations.py index 05e1106..92af42c 100644 --- a/tests/test_file_locations.py +++ b/tests/test_file_locations.py @@ -19,7 +19,7 @@ from coretrace_python.cli import main from coretrace_python.findings import Finding from coretrace_python.source import FileLocation, SourceSpan -from coretrace_python.source.positions import file_location +from coretrace_python.source.positions import file_location, text_digest PLUGINS = Path(__file__).resolve().parent.parent / "src" / "coretrace_python" / "bundled" TOKEN = "Zx81kQpLw0RtY7vBn3MsD9cF2hJ6gK4a" @@ -135,12 +135,12 @@ def test_the_baseline_recognises_a_file_location_by_its_pointer( document = json.loads(baseline.read_text(encoding="utf-8")) capsys.readouterr() - assert document["schema"] == 2 + assert document["schema"] == 3 assert { "path": "settings.toml", "rule": "hardcoded-credential", "function": "", - "pointer": "/a/api_token", + "pointer": text_digest("/a/api_token"), }.items() <= next( e for e in document["findings"] if e["rule"] == "hardcoded-credential" ).items()