fix(baseline): fingerprint every line of a finding and store digests only (#223, #224)

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
 

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:

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,15 +107,15 @@ 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(
                 {
                     (
                         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"
         )

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:

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:

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,)