diff --git a/CHANGELOG.md b/CHANGELOG.md index 8366cb7..0c150cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ All notable changes to diffly are documented here. The format follows [Keep a Ch ### Fixed - Fast keystroke bursts in the interactive review menu register every press: escape sequences stop at their terminator instead of swallowing neighboring keys. +- Diff content lines beginning with `++` or `--` are counted, scanned for credentials, and analyzed instead of being mistaken for the file's header lines. - `diffly setup` no longer runs the update check twice in a row (once in `main()` and again inside the wizard it launches). - Pressing an arrow key in the interactive review menu no longer crashes with `NameError: name '_read_escape_sequence' is not defined`; the escape-sequence reader is defined again and a lone Escape still exits without blocking. diff --git a/src/diffly_cli/diffparse.py b/src/diffly_cli/diffparse.py index a7b91aa..9d93e3d 100644 --- a/src/diffly_cli/diffparse.py +++ b/src/diffly_cli/diffparse.py @@ -33,6 +33,28 @@ def parse_hunks(patch: str) -> list[Hunk]: return hunks +def hunk_body_lines(patch: str) -> list[str]: + """Return the raw lines inside hunk bodies, excluding the file-header prelude. + + Content lines may themselves begin with ``---`` or ``+++`` (removed CLI + flags, added Markdown headings), so header detection must stop at the + first ``@@`` marker instead of pattern-matching every line. Patches that + carry no ``@@`` marker at all are treated as body-only (bare hunks from + the GitHub files endpoint); a full ``diff --git`` block without hunks has + its prelude stripped. + """ + lines = patch.splitlines() + for index, line in enumerate(lines): + if line.startswith("@@"): + return lines[index + 1 :] + if lines and lines[0].startswith("diff --git"): + for index, line in enumerate(lines): + if line.startswith("+++ "): + return lines[index + 1 :] + return [] + return lines + + def files_from_unified_diff(diff: str) -> list[ChangedFile]: """Build ChangedFile objects from a raw GitHub unified diff.""" blocks = re.split(r"(?=^diff --git a/)", diff, flags=re.MULTILINE) @@ -50,8 +72,9 @@ def files_from_unified_diff(diff: str) -> list[ChangedFile]: status = "removed" elif re.search(r"^rename from ", block, flags=re.MULTILINE): status = "renamed" - additions = sum(1 for line in block.splitlines() if line.startswith("+") and not line.startswith("+++") ) - deletions = sum(1 for line in block.splitlines() if line.startswith("-") and not line.startswith("---") ) + body = hunk_body_lines(block) + additions = sum(1 for line in body if line.startswith("+")) + deletions = sum(1 for line in body if line.startswith("-")) file = ChangedFile(path=path, status=status, additions=additions, deletions=deletions, changes=additions + deletions, patch=block) file.hunks = parse_hunks(block) files.append(file) @@ -89,12 +112,8 @@ def language_for_path(path: str) -> str | None: def added_source(patch: str) -> str: lines = [] - for line in patch.splitlines(): - if line.startswith("+++") or line.startswith("---"): - continue - if line.startswith("+"): - lines.append(line[1:]) - elif line.startswith(" "): + for line in hunk_body_lines(patch): + if line.startswith("+") or line.startswith(" "): lines.append(line[1:]) return "\n".join(lines) diff --git a/src/diffly_cli/triage.py b/src/diffly_cli/triage.py index fb29600..99ea20e 100644 --- a/src/diffly_cli/triage.py +++ b/src/diffly_cli/triage.py @@ -5,6 +5,7 @@ from collections import defaultdict from typing import Any +from .diffparse import hunk_body_lines from .models import ChangedFile, PRMetadata, RiskFlag from .redact import redact_secrets @@ -37,8 +38,8 @@ def _added_dependency_names(file: ChangedFile) -> list[str]: if file.path.rsplit("/", 1)[-1] not in DEPENDENCY_FILES: return [] values: list[str] = [] - for line in file.patch.splitlines(): - if line.startswith("+") and not line.startswith("+++"): + for line in hunk_body_lines(file.patch): + if line.startswith("+"): match = re.search(r"[\"']([@A-Za-z0-9_./-]+)[\"']\s*[:=]", line) if match: values.append(match.group(1)) @@ -63,7 +64,7 @@ def _covered_by_test(file: ChangedFile, test_paths: list[str]) -> list[str]: def _has_exposed_secret(file: ChangedFile) -> bool: """Detect a credential-like value added in the changed hunk, not just its path.""" - added_lines = "\n".join(line[1:] for line in file.patch.splitlines() if line.startswith("+") and not line.startswith("+++")) + added_lines = "\n".join(line[1:] for line in hunk_body_lines(file.patch) if line.startswith("+")) # A generic ``token = os.getenv(...)`` assignment is common application # code, not evidence of an exposed credential. Block only on high-confidence # credential formats (keys, bearer tokens, private keys, connection URLs). diff --git a/tests/test_regressions.py b/tests/test_regressions.py index 1cf6c85..19082b3 100644 --- a/tests/test_regressions.py +++ b/tests/test_regressions.py @@ -415,3 +415,96 @@ def test_setup_delegates_the_update_check_to_the_wizard(monkeypatch): monkeypatch.setattr(cli, "run_pr", lambda args: 0) assert cli.main(["pr", "acme/demo", "1"]) == 0 assert calls == ["check"] + + +def test_content_lines_looking_like_headers_are_still_counted(): + """Regression: added/deleted lines whose content begins with ++/-- were + mistaken for the file's ---/+++ header lines and dropped from counts and + scans.""" + from diffly_cli.diffparse import files_from_unified_diff + + diff = ( + "diff --git a/notes.md b/notes.md\n" + "index 0000001..0000002 100644\n" + "--- a/notes.md\n" + "+++ b/notes.md\n" + "@@ -1,3 +1,3 @@\n" + " intro\n" + "--- old banner\n" + "+++ new banner\n" + " tail\n" + ) + file = files_from_unified_diff(diff)[0] + assert file.additions == 1 + assert file.deletions == 1 + + +def test_github_json_patch_without_prelude_is_counted(): + """Bare hunk bodies (GitHub files-endpoint patches) carry no @@ prelude and + must scan unchanged.""" + from diffly_cli.diffparse import hunk_body_lines + + body = hunk_body_lines("@@ -1,2 +1,2 @@\n-context\n+replacement\n") + assert sum(1 for line in body if line.startswith("+")) == 1 + assert sum(1 for line in body if line.startswith("-")) == 1 + + bare = hunk_body_lines("+API_KEY = 'x'\n-context\n") + assert bare[0] == "+API_KEY = 'x'" + assert "-context" in bare + + +def test_truncated_diff_block_with_prelude_but_no_hunks_counts_nothing(): + from diffly_cli.diffparse import files_from_unified_diff + + diff = ( + "diff --git a/big.bin b/big.bin\n" + "index 0000001..0000002 100644\n" + "Binary files a/big.bin and b/big.bin differ\n" + "--- a/big.bin\n" + "+++ b/big.bin\n" + ) + file = files_from_unified_diff(diff)[0] + assert file.additions == 0 + assert file.deletions == 0 + + +def test_secret_on_plus_prefixed_content_line_blocks_the_pr(): + """Regression: content beginning with ++ made the whole diff line look like + the file's +++ header, hiding it from counting and the secret scan.""" + from diffly_cli.models import ChangedFile + from diffly_cli.triage import compute_flags, verdict_for + + patch = ( + "diff --git a/docs/deploy.md b/docs/deploy.md\n" + "--- a/docs/deploy.md\n" + "+++ b/docs/deploy.md\n" + "@@ -1,2 +1,3 @@\n" + " intro\n" + "+++ postgresql://admin:hunter2@db.internal.example/prod\n" + ) + files = [ChangedFile(path="docs/deploy.md", status="modified", additions=1, deletions=0, changes=1, patch=patch)] + checks = {"state": "success", "count": 1, "repository_tree_complete": True} + flags = compute_flags(metadata(), files, checks, ["docs/deploy.md"]) + codes = {flag.code for flag in flags} + assert "EXPOSED_SECRET" in codes + assert files[0].additions == 1 + assert verdict_for(flags, checks)[0] == "BLOCK" + + +def test_dependency_detection_survives_plus_prefixed_lines(): + from diffly_cli.models import ChangedFile + from diffly_cli.triage import _added_dependency_names + + patch = ( + "diff --git a/package.json b/package.json\n" + "--- a/package.json\n" + "+++ b/package.json\n" + "@@ -1,3 +1,4 @@\n" + " {\n" + '+ "left-pad": "^1.3.0",\n' + "+ // +++ see docs\n" + ' "name": "x"\n' + " }\n" + ) + names = _added_dependency_names(ChangedFile(path="package.json", status="modified", additions=2, deletions=0, changes=2, patch=patch)) + assert names == ["left-pad"]