diff --git a/README.md b/README.md index 11f728e2..4840651e 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,7 @@ This guide walks you through both installation and usage. 4. [Commit History Scan](#commit-history-scan) 1. [Commit Range Option (Diff Scanning)](#commit-range-option-diff-scanning) 5. [Pre-Commit Scan](#pre-commit-scan) + 1. [Local Diff Scanning (IDE Integrations)](#local-diff-scanning-ide-integrations) 6. [Pre-Push Scan](#pre-push-scan) 2. [Scan Results](#scan-results) 1. [Show/Hide Secrets](#showhide-secrets) @@ -798,12 +799,12 @@ The Cycode CLI application offers several types of scans so that you can choose | `--maven-settings-file` | For Maven only, allows using a custom [settings.xml](https://maven.apache.org/settings.html) file when scanning for dependencies | | `--help` | Show options for given command. | -| Command | Description | -|----------------------------------------|-----------------------------------------------------------------------| -| [commit-history](#commit-history-scan) | Scan commit history or perform diff scanning between specific commits | -| [path](#path-scan) | Scan the files in the path supplied in the command | -| [pre-commit](#pre-commit-scan) | Use this command to scan the content that was not committed yet | -| [repository](#repository-scan) | Scan git repository including its history | +| Command | Description | +|-----------------------------------------|----------------------------------------------------------------------------------------------------| +| [commit-history](#commit-history-scan) | Scan commit history or perform diff scanning between specific commits | +| [path](#path-scan) | Scan the files in the path supplied in the command | +| [pre-commit](#pre-commit-scan) | Scan content that was not committed yet; also supports local diff scanning via flags (IDE-friendly) | +| [repository](#repository-scan) | Scan git repository including its history | ### Options @@ -1085,6 +1086,36 @@ After installing the pre-commit hook, you may occasionally wish to skip scanning SKIP=cycode git commit -m ` ``` +The following options are available for use with this command: + +| Option | Description | +|-----------------------|---------------------------------------------------------------------------------------------------------------------| +| `-b, --base-ref TEXT` | Git ref (commit, branch, or tag) to diff against; defaults to `HEAD`, matching the pre-commit hook behavior | +| `--include-unstaged` | Also scan unstaged changes to tracked files, not just what is staged; off by default | +| `--path PATH` | Optional path(s) to scope the diff scan to; repeatable; defaults to the entire working directory | + +#### Local Diff Scanning (IDE Integrations) + +`--base-ref` and `--include-unstaged` turn `pre-commit` into a general-purpose **local diff scan**: comparing any commit (default `HEAD`) against your current working directory — including edits that aren't staged yet. This is intended for IDE plugins and other tools that need continuous, real-time feedback as you work, rather than the git hook flow. Combined with `--include-unstaged`, `--path` lets an IDE scope the scan to just the file currently open in the editor. + +> [!NOTE] +> Local diff scanning (via these flags) is not available for IaC scans. + +**Scan everything currently changed (staged + unstaged) against the last commit:** +```bash +cycode scan pre-commit --include-unstaged +``` + +**Scan changes against a specific commit or branch:** +```bash +cycode scan pre-commit --include-unstaged --base-ref main +``` + +**Scan only a specific file (e.g., the file currently open in your IDE):** +```bash +cycode scan pre-commit --include-unstaged --path src/app.py +``` + ### Pre-Push Scan A pre-push scan automatically identifies any issues before you push changes to the remote repository. This hook runs on the client side and scans only the commits that are about to be pushed, making it efficient for catching issues before they reach the remote repository. diff --git a/cycode/cli/apps/scan/__init__.py b/cycode/cli/apps/scan/__init__.py index 629c3b8f..424fcaa5 100644 --- a/cycode/cli/apps/scan/__init__.py +++ b/cycode/cli/apps/scan/__init__.py @@ -28,7 +28,8 @@ ) app.command( name='pre-commit', - short_help='Use this command in pre-commit hook to scan any content that was not committed yet.', + short_help='Use this command in pre-commit hook to scan any content that was not committed yet. ' + 'Also supports IDE-style local diff scanning via --base-ref/--include-unstaged/--path.', rich_help_panel=_AUTOMATION_COMMANDS_RICH_HELP_PANEL, )(pre_commit_command) app.command( diff --git a/cycode/cli/apps/scan/commit_range_scanner.py b/cycode/cli/apps/scan/commit_range_scanner.py index b1a58202..f1f52318 100644 --- a/cycode/cli/apps/scan/commit_range_scanner.py +++ b/cycode/cli/apps/scan/commit_range_scanner.py @@ -24,10 +24,7 @@ from cycode.cli.files_collector.commit_range_documents import ( collect_commit_range_diff_documents, get_commit_range_modified_documents, - get_diff_file_content, - get_diff_file_path, get_pre_commit_modified_documents, - get_staged_diff_index, parse_commit_range, ) from cycode.cli.files_collector.documents_walk_ignore import filter_documents_with_cycodeignore @@ -35,12 +32,10 @@ from cycode.cli.files_collector.models.in_memory_zip import InMemoryZip from cycode.cli.files_collector.sca.sca_file_collector import ( perform_sca_pre_commit_range_scan_actions, - perform_sca_pre_hook_range_scan_actions, + perform_sca_pre_commit_scan_actions, ) from cycode.cli.files_collector.zip_documents import zip_documents from cycode.cli.models import Document -from cycode.cli.utils.git_proxy import git_proxy -from cycode.cli.utils.path_utils import get_path_by_os from cycode.cli.utils.progress_bar import ScanProgressBarSection from cycode.cli.utils.scan_utils import ( generate_unique_scan_id, @@ -329,84 +324,103 @@ def scan_commit_range(ctx: typer.Context, repo_path: str, commit_range: str, **k _SCAN_TYPE_TO_COMMIT_RANGE_HANDLER[scan_type](ctx, repo_path, commit_range, **kwargs) -def _scan_sca_pre_commit(ctx: typer.Context, repo_path: str) -> None: - scan_parameters = get_scan_parameters(ctx) +def _scan_sca_pre_commit( + ctx: typer.Context, + repo_path: str, + base_ref: str = consts.GIT_HEAD_COMMIT_REV, + include_unstaged: bool = False, + paths: Optional[list[str]] = None, +) -> None: + scan_parameters = get_scan_parameters(ctx, (repo_path,)) - git_head_documents, pre_committed_documents, _ = get_pre_commit_modified_documents( + from_ref_documents, working_copy_documents, _diff_documents = get_pre_commit_modified_documents( progress_bar=ctx.obj['progress_bar'], progress_bar_section=ScanProgressBarSection.PREPARE_LOCAL_FILES, repo_path=repo_path, + base_ref=base_ref, + include_unstaged=include_unstaged, + paths=paths, ) - git_head_documents = excluder.exclude_irrelevant_documents_to_scan(consts.SCA_SCAN_TYPE, git_head_documents) - pre_committed_documents = excluder.exclude_irrelevant_documents_to_scan( - consts.SCA_SCAN_TYPE, pre_committed_documents - ) + from_ref_documents = excluder.exclude_irrelevant_documents_to_scan(consts.SCA_SCAN_TYPE, from_ref_documents) + working_copy_documents = excluder.exclude_irrelevant_documents_to_scan(consts.SCA_SCAN_TYPE, working_copy_documents) is_cycodeignore_allowed = is_cycodeignore_allowed_by_scan_config(ctx) - git_head_documents = filter_documents_with_cycodeignore(git_head_documents, repo_path, is_cycodeignore_allowed) - pre_committed_documents = filter_documents_with_cycodeignore( - pre_committed_documents, repo_path, is_cycodeignore_allowed + from_ref_documents = filter_documents_with_cycodeignore(from_ref_documents, repo_path, is_cycodeignore_allowed) + working_copy_documents = filter_documents_with_cycodeignore( + working_copy_documents, repo_path, is_cycodeignore_allowed ) - perform_sca_pre_hook_range_scan_actions(repo_path, git_head_documents, pre_committed_documents) + perform_sca_pre_commit_scan_actions(repo_path, from_ref_documents, base_ref, working_copy_documents) _scan_commit_range_documents( ctx, - git_head_documents, - pre_committed_documents, + from_ref_documents, + working_copy_documents, scan_parameters, configuration_manager.get_sca_pre_commit_timeout_in_seconds(), ) -def _scan_secret_pre_commit(ctx: typer.Context, repo_path: str) -> None: - progress_bar = ctx.obj['progress_bar'] - repo = git_proxy.get_repo(repo_path) - _, diff_index = get_staged_diff_index(repo) - - progress_bar.set_section_length(ScanProgressBarSection.PREPARE_LOCAL_FILES, len(diff_index)) - - documents_to_scan = [] - for diff in diff_index: - progress_bar.update(ScanProgressBarSection.PREPARE_LOCAL_FILES) - documents_to_scan.append( - Document( - get_path_by_os(get_diff_file_path(diff, repo=repo)), - get_diff_file_content(diff), - is_git_diff_format=True, - ) - ) +def _scan_secret_pre_commit( + ctx: typer.Context, + repo_path: str, + base_ref: str = consts.GIT_HEAD_COMMIT_REV, + include_unstaged: bool = False, + paths: Optional[list[str]] = None, +) -> None: + # collect_file_contents=False: the secret scan only ever uses diff_documents below, so skip + # building from_ref_documents/working_copy_documents (a disk read per changed file it would + # otherwise discard immediately). + _from_ref_documents, _working_copy_documents, diff_documents = get_pre_commit_modified_documents( + progress_bar=ctx.obj['progress_bar'], + progress_bar_section=ScanProgressBarSection.PREPARE_LOCAL_FILES, + repo_path=repo_path, + base_ref=base_ref, + include_unstaged=include_unstaged, + paths=paths, + collect_file_contents=False, + ) - documents_to_scan = excluder.exclude_irrelevant_documents_to_scan(consts.SECRET_SCAN_TYPE, documents_to_scan) + diff_documents = excluder.exclude_irrelevant_documents_to_scan(consts.SECRET_SCAN_TYPE, diff_documents) is_cycodeignore_allowed = is_cycodeignore_allowed_by_scan_config(ctx) - documents_to_scan = filter_documents_with_cycodeignore(documents_to_scan, repo_path, is_cycodeignore_allowed) + diff_documents = filter_documents_with_cycodeignore(diff_documents, repo_path, is_cycodeignore_allowed) - scan_documents(ctx, documents_to_scan, get_scan_parameters(ctx), is_git_diff=True) + scan_documents(ctx, diff_documents, get_scan_parameters(ctx, (repo_path,)), is_git_diff=True) -def _scan_sast_pre_commit(ctx: typer.Context, repo_path: str, **_) -> None: +def _scan_sast_pre_commit( + ctx: typer.Context, + repo_path: str, + base_ref: str = consts.GIT_HEAD_COMMIT_REV, + include_unstaged: bool = False, + paths: Optional[list[str]] = None, + **_, +) -> None: scan_parameters = get_scan_parameters(ctx, (repo_path,)) - _, pre_committed_documents, diff_documents = get_pre_commit_modified_documents( + _from_ref_documents, working_copy_documents, diff_documents = get_pre_commit_modified_documents( progress_bar=ctx.obj['progress_bar'], progress_bar_section=ScanProgressBarSection.PREPARE_LOCAL_FILES, repo_path=repo_path, + base_ref=base_ref, + include_unstaged=include_unstaged, + paths=paths, ) - pre_committed_documents = excluder.exclude_irrelevant_documents_to_scan( - consts.SAST_SCAN_TYPE, pre_committed_documents + working_copy_documents = excluder.exclude_irrelevant_documents_to_scan( + consts.SAST_SCAN_TYPE, working_copy_documents ) diff_documents = excluder.exclude_irrelevant_documents_to_scan(consts.SAST_SCAN_TYPE, diff_documents) is_cycodeignore_allowed = is_cycodeignore_allowed_by_scan_config(ctx) - pre_committed_documents = filter_documents_with_cycodeignore( - pre_committed_documents, repo_path, is_cycodeignore_allowed + working_copy_documents = filter_documents_with_cycodeignore( + working_copy_documents, repo_path, is_cycodeignore_allowed ) diff_documents = filter_documents_with_cycodeignore(diff_documents, repo_path, is_cycodeignore_allowed) - _scan_commit_range_documents(ctx, pre_committed_documents, diff_documents, scan_parameters=scan_parameters) + _scan_commit_range_documents(ctx, working_copy_documents, diff_documents, scan_parameters=scan_parameters) _SCAN_TYPE_TO_PRE_COMMIT_HANDLER = { @@ -416,10 +430,18 @@ def _scan_sast_pre_commit(ctx: typer.Context, repo_path: str, **_) -> None: } -def scan_pre_commit(ctx: typer.Context, repo_path: str) -> None: +def scan_pre_commit( + ctx: typer.Context, + repo_path: str, + base_ref: str = consts.GIT_HEAD_COMMIT_REV, + include_unstaged: bool = False, + paths: Optional[list[str]] = None, +) -> None: scan_type = ctx.obj['scan_type'] if scan_type not in _SCAN_TYPE_TO_PRE_COMMIT_HANDLER: raise click.ClickException(f'Pre-commit scanning for {scan_type.upper()} is not supported') - _SCAN_TYPE_TO_PRE_COMMIT_HANDLER[scan_type](ctx, repo_path) + _SCAN_TYPE_TO_PRE_COMMIT_HANDLER[scan_type]( + ctx, repo_path, base_ref=base_ref, include_unstaged=include_unstaged, paths=paths + ) logger.debug('Pre-commit scan completed successfully') diff --git a/cycode/cli/apps/scan/pre_commit/pre_commit_command.py b/cycode/cli/apps/scan/pre_commit/pre_commit_command.py index e0cbc7a8..beacf734 100644 --- a/cycode/cli/apps/scan/pre_commit/pre_commit_command.py +++ b/cycode/cli/apps/scan/pre_commit/pre_commit_command.py @@ -1,18 +1,100 @@ import os +from pathlib import Path from typing import Annotated, Optional import typer +from cycode.cli import consts from cycode.cli.apps.scan.commit_range_scanner import scan_pre_commit +from cycode.cli.exceptions.custom_exceptions import ScanPathOutsideRepositoryError, UnresolvedGitRefError +from cycode.cli.exceptions.handle_scan_errors import handle_scan_exception +from cycode.cli.logger import logger +from cycode.cli.utils.git_proxy import git_proxy + + +def _resolve_repo_root(cwd: str) -> str: + """Resolve the repository root from `cwd`, which may be any subdirectory of the repo. + + `git_proxy.get_repo()` requires an exact match (the root or a `.git` dir) unless told to + search parent directories, so without this, running the command from anywhere but the repo + root raises a misleading "not a git repository" error. The git pre-commit hook framework + always invokes from the repo root, so this is a no-op there; it matters for IDE-style + invocations (--include-unstaged etc.), which may run from any subdirectory. + """ + repo = git_proxy.get_repo(cwd, search_parent_directories=True) + return repo.working_tree_dir or cwd + + +def _validate_base_ref(repo_path: str, base_ref: str) -> None: + """Raise a clear, user-facing error for an unresolvable `--base-ref`. + + A repository with no commits at all is a valid state (the diff falls back to comparing + against the empty tree), so validation is skipped in that case. + """ + repo = git_proxy.get_repo(repo_path) + + try: + repo.rev_parse(consts.GIT_HEAD_COMMIT_REV) + except Exception as e: + logger.debug('Repository has no commits yet; skipping --base-ref validation', exc_info=e) + return + + try: + repo.commit(base_ref) + except Exception as e: + raise UnresolvedGitRefError(base_ref) from e def pre_commit_command( ctx: typer.Context, _: Annotated[Optional[list[str]], typer.Argument(help='Ignored arguments', hidden=True)] = None, + base_ref: Annotated[ + str, + typer.Option( + '--base-ref', + '-b', + help='Git ref (commit, branch, or tag) to diff against. Defaults to HEAD, matching the ' + 'pre-commit hook behavior. Combine with --include-unstaged for IDE-style local diff scanning.', + ), + ] = consts.GIT_HEAD_COMMIT_REV, + include_unstaged: Annotated[ + bool, + typer.Option( + '--include-unstaged', + help='Also scan unstaged changes to tracked files, not just what is staged. ' + 'Off by default, so the pre-commit hook flow is unaffected.', + ), + ] = False, + paths: Annotated[ + Optional[list[Path]], + typer.Option( + '--path', + help='Optional paths to scope the diff scan to (e.g. the file currently open in an IDE). ' + 'Defaults to the entire working directory. Repeatable.', + show_default=False, + resolve_path=True, + ), + ] = None, ) -> None: - repo_path = os.getcwd() # change locally for easy testing + try: + repo_path = _resolve_repo_root(os.getcwd()) + _validate_base_ref(repo_path, base_ref) + str_paths = [str(path) for path in paths] if paths else None + # realpath both sides: repo_path (from GitPython) and str_path (from Click's + # resolve_path=True) can disagree on 8.3 short-name vs long-name form on Windows, + # which would otherwise make an in-repo path look like it's outside the repo. + repo_path_real = os.path.realpath(repo_path) + for str_path in str_paths or []: + if os.path.commonpath([repo_path_real, os.path.realpath(str_path)]) != repo_path_real: + raise ScanPathOutsideRepositoryError(str_path, repo_path) + except Exception as e: + handle_scan_exception(ctx, e) + return progress_bar = ctx.obj['progress_bar'] progress_bar.start() - scan_pre_commit(ctx, repo_path) + try: + scan_pre_commit(ctx, repo_path, base_ref=base_ref, include_unstaged=include_unstaged, paths=str_paths) + except Exception as e: + handle_scan_exception(ctx, e) diff --git a/cycode/cli/exceptions/custom_exceptions.py b/cycode/cli/exceptions/custom_exceptions.py index f4dd787d..af66e89b 100644 --- a/cycode/cli/exceptions/custom_exceptions.py +++ b/cycode/cli/exceptions/custom_exceptions.py @@ -103,6 +103,25 @@ def __str__(self) -> str: return f'Error occurred while parsing terraform plan file. Path: {self.file_path}' +class ScanPathOutsideRepositoryError(CycodeError): + def __init__(self, path: str, repo_path: str) -> None: + self.path = path + self.repo_path = repo_path + super().__init__() + + def __str__(self) -> str: + return f'The path {self.path!r} is outside the repository {self.repo_path!r}' + + +class UnresolvedGitRefError(CycodeError): + def __init__(self, ref: str) -> None: + self.ref = ref + super().__init__() + + def __str__(self) -> str: + return f'Could not resolve git ref: {self.ref!r}' + + _SSL_ERROR_CA_BUNDLE_HINT = ( 'set the REQUESTS_CA_BUNDLE (or CURL_CA_BUNDLE) environment variable to the path of a valid .pem or similar' ) diff --git a/cycode/cli/exceptions/handle_scan_errors.py b/cycode/cli/exceptions/handle_scan_errors.py index 51957d63..5e2d9776 100644 --- a/cycode/cli/exceptions/handle_scan_errors.py +++ b/cycode/cli/exceptions/handle_scan_errors.py @@ -60,6 +60,16 @@ def handle_scan_exception(ctx: typer.Context, err: Exception, *, return_exceptio message='The path you supplied does not correlate to a Git repository. ' 'If you still wish to scan this path, use: `cycode scan path `', ), + custom_exceptions.ScanPathOutsideRepositoryError: CliError( + soft_fail=False, + code='invalid_scan_path_error', + message=f'\n{err!s}\n--path must point to a location inside the scanned repository', + ), + custom_exceptions.UnresolvedGitRefError: CliError( + soft_fail=False, + code='invalid_git_ref_error', + message=f'\n{err!s}\nPass a commit, branch, or tag that exists in this repository', + ), } return handle_errors(ctx, err, errors, return_exception=return_exception) diff --git a/cycode/cli/files_collector/commit_range_documents.py b/cycode/cli/files_collector/commit_range_documents.py index bb406541..d5fdcc2f 100644 --- a/cycode/cli/files_collector/commit_range_documents.py +++ b/cycode/cli/files_collector/commit_range_documents.py @@ -15,7 +15,7 @@ from cycode.logger import get_logger if TYPE_CHECKING: - from git import Diff, DiffIndex, Repo + from git import Blob, Diff, DiffIndex, Repo from cycode.cli.utils.progress_bar import BaseProgressBar, ProgressBarSection @@ -47,7 +47,7 @@ def get_safe_head_reference_for_diff(repo: 'Repo') -> str: return consts.GIT_EMPTY_TREE_OBJECT -def get_staged_diff_index(repo: 'Repo') -> tuple[str, 'DiffIndex']: +def get_staged_diff_index(repo: 'Repo', paths: Optional[list[str]] = None) -> tuple[str, 'DiffIndex']: """Diff the index against HEAD, or against the empty tree in repositories with no commits. GitPython only inverts the `R` flag for HEAD, so `R` must be off for the empty tree to keep @@ -55,13 +55,14 @@ def get_staged_diff_index(repo: 'Repo') -> tuple[str, 'DiffIndex']: Args: repo: Git repository object + paths: Optional pathspec to scope the diff to Returns: The reference that was diffed against, and the resulting diff index """ head_reference = get_safe_head_reference_for_diff(repo) reverse = head_reference == consts.GIT_HEAD_COMMIT_REV - return head_reference, repo.index.diff(head_reference, create_patch=True, R=reverse) + return head_reference, repo.index.diff(head_reference, create_patch=True, R=reverse, paths=paths or None) def _does_reach_to_max_commits_to_scan_limit(commit_ids: list[str], max_commits_count: Optional[int]) -> bool: @@ -185,6 +186,20 @@ def _get_file_content_from_commit_diff(repo: 'Repo', commit: str, diff: 'Diff') return get_file_content_from_commit_path(repo, commit, file_path) +def _get_blob_content(blob: Optional['Blob']) -> Optional[str]: + """Read a diff-side blob's content in-process, with no subprocess spawn. + + `diff.a_blob`/`diff.b_blob` already hold whatever git loaded to compute the patch, so this is + the same content `git show :` would return -- for every `Diffable.diff()` variant + (`index.diff(HEAD, R=True)`, `commit.diff()`, `commit.diff(None)`), `a_blob` is exactly the + "before" side, and it's `None` precisely when the file didn't exist there (e.g. diffed against + the empty tree for a brand-new repo/file) -- verified against a scratch repo for each variant. + """ + if blob is None: + return None + return blob.data_stream.read().decode('UTF-8', errors='replace') + + def get_commit_range_modified_documents( progress_bar: 'BaseProgressBar', progress_bar_section: 'ProgressBarSection', @@ -453,13 +468,65 @@ def get_pre_commit_modified_documents( progress_bar: 'BaseProgressBar', progress_bar_section: 'ProgressBarSection', repo_path: str, + base_ref: str = consts.GIT_HEAD_COMMIT_REV, + include_unstaged: bool = False, + paths: Optional[list[str]] = None, + collect_file_contents: bool = True, ) -> tuple[list[Document], list[Document], list[Document]]: - git_head_documents = [] - pre_committed_documents = [] + """Diffs `base_ref` against the staged index (default) or the full working tree. + + Args: + base_ref: Git ref to diff against. Defaults to HEAD, matching the pre-commit hook's + historical behavior exactly. + include_unstaged: If False (default), diffs the staged index only -- this is the exact + pre-commit hook flow. If True, diffs the full working tree (staged + unstaged). + paths: Optional pathspec to scope the diff to. + collect_file_contents: If False, skip building `from_ref_documents`/`working_copy_documents` + entirely -- only `diff_documents` (the patch itself) is produced. The secret scan path + only ever uses `diff_documents`, so this avoids a disk read per changed file (and, + before the `a_blob` fix below, a subprocess spawn per file) that it would otherwise + discard immediately. Defaults to True so existing callers (SCA/SAST) are unaffected. + + Returns: + (from_ref_documents, working_copy_documents, diff_documents). `from_ref_documents` holds + each changed file's content at `base_ref` (skipped if it didn't exist there, or was empty); + `working_copy_documents` holds each changed file's current on-disk content (skipped if + the file no longer exists, or is empty); `diff_documents` holds the unified diff per + changed file. + """ + from_ref_documents = [] + working_copy_documents = [] diff_documents = [] repo = git_proxy.get_repo(repo_path) - head_reference, diff_index = get_staged_diff_index(repo) + + if base_ref == consts.GIT_HEAD_COMMIT_REV and not include_unstaged: + # The exact pre-commit hook flow, byte-for-byte unchanged: diff the staged index against + # HEAD (or the empty tree in a brand-new repo), via get_staged_diff_index()'s proven + # R-flag handling. Keeping this as its own branch (rather than folding it into the + # `diff_target.diff(...)` calls below) means this default, most-used path can't regress. + _, diff_index = get_staged_diff_index(repo, paths=paths) + else: + try: + diff_target = repo.commit(base_ref) + except Exception as e: + # Repository has no commits yet, or base_ref doesn't resolve; diff against the empty + # tree instead. (git_proxy.get_null_tree() is only a sentinel usable as the `other` + # side of a diff, so we resolve the well-known empty tree object itself to diff from.) + logger.debug( + 'Could not resolve base_ref, falling back to the empty tree, %s', {'base_ref': base_ref}, exc_info=e + ) + diff_target = repo.tree(consts.GIT_EMPTY_TREE_OBJECT) + + if include_unstaged: + diff_index = diff_target.diff(None, create_patch=True, paths=paths or None) + else: + # `Diffable.diff()` defaults `other` to the staged index, so omitting it here diffs + # `base_ref` against the index only -- the same "staged changes against an arbitrary + # ref" semantics as `git diff --cached `, verified to produce identical, + # correctly-directed patches. + diff_index = diff_target.diff(create_patch=True, paths=paths or None) + progress_bar.set_section_length(progress_bar_section, len(diff_index)) for diff in diff_index: progress_bar.update(progress_bar_section) @@ -474,18 +541,25 @@ def get_pre_commit_modified_documents( ) ) - # Only get file content from HEAD if HEAD exists (not the empty tree hash) - if head_reference == consts.GIT_HEAD_COMMIT_REV: - file_content = _get_file_content_from_commit_diff(repo, head_reference, diff) - if file_content: - git_head_documents.append(Document(file_path, file_content)) + if not collect_file_contents: + continue + + # `a_blob` is exactly the content at `base_ref` -- see `_get_blob_content`'s docstring. + # Deliberately `if file_content:` (skip empty string) rather than `is not None`: this + # matches the working_copy branch immediately below, treating an empty file the same as + # a nonexistent one. `get_commit_range_modified_documents` uses `is not None` instead, but + # for a different reason (there, `None` only ever means "no such commit"); don't "fix" + # this one to match it. + file_content = _get_blob_content(diff.a_blob) + if file_content: + from_ref_documents.append(Document(file_path, file_content)) if os.path.exists(file_path): file_content = get_file_content(file_path) if file_content: - pre_committed_documents.append(Document(file_path, file_content)) + working_copy_documents.append(Document(file_path, file_content)) - return git_head_documents, pre_committed_documents, diff_documents + return from_ref_documents, working_copy_documents, diff_documents def parse_commit_range(commit_range: str, path: str) -> tuple[Optional[str], Optional[str], Optional[str]]: diff --git a/cycode/cli/files_collector/sca/sca_file_collector.py b/cycode/cli/files_collector/sca/sca_file_collector.py index 4db5cd04..3ac2d3e9 100644 --- a/cycode/cli/files_collector/sca/sca_file_collector.py +++ b/cycode/cli/files_collector/sca/sca_file_collector.py @@ -64,12 +64,19 @@ def perform_sca_pre_commit_range_scan_actions( _add_ecosystem_related_files_if_exists(to_commit_documents, repo, to_commit_rev) -def perform_sca_pre_hook_range_scan_actions( - repo_path: str, git_head_documents: list[Document], pre_committed_documents: list[Document] +def perform_sca_pre_commit_scan_actions( + repo_path: str, from_ref_documents: list[Document], base_ref: str, working_copy_documents: list[Document] ) -> None: + """Add ecosystem-related project files for a pre-commit-style scan (staged-index-or-working-tree vs `base_ref`). + + `base_ref` can be any ref, not just HEAD -- covers both the default pre-commit hook flow and + the opt-in `--base-ref`/`--include-unstaged` flags. + """ repo = git_proxy.get_repo(repo_path) - _add_ecosystem_related_files_if_exists(git_head_documents, repo, consts.GIT_HEAD_COMMIT_REV) - _add_ecosystem_related_files_if_exists(pre_committed_documents) + _add_ecosystem_related_files_if_exists(from_ref_documents, repo, base_ref) + # working copy documents reflect the live filesystem (staged, or staged+unstaged), so their + # related project files must also be read from disk, not from a commit tree. + _add_ecosystem_related_files_if_exists(working_copy_documents) def _get_doc_ecosystem_related_project_files( diff --git a/tests/cli/commands/scan/test_commit_range_scanner.py b/tests/cli/commands/scan/test_commit_range_scanner.py index a4a6c58b..24743e55 100644 --- a/tests/cli/commands/scan/test_commit_range_scanner.py +++ b/tests/cli/commands/scan/test_commit_range_scanner.py @@ -47,3 +47,60 @@ def test_commit_range_scan_falls_back_to_api_when_presigned_upload_raises_wrappe mock_v4_async.assert_called_once() mock_async.assert_called_once() mock_handle_exception.assert_not_called() + + +class TestScanPreCommit: + """Test the scan_pre_commit dispatcher, including the folded-in local-diff-style flags.""" + + def _make_ctx(self, scan_type: str) -> MagicMock: + mock_ctx = MagicMock() + mock_ctx.obj = {'scan_type': scan_type, 'progress_bar': MagicMock()} + return mock_ctx + + def test_unsupported_scan_type_raises(self) -> None: + import click + import pytest + + from cycode.cli.apps.scan.commit_range_scanner import scan_pre_commit + + mock_ctx = self._make_ctx(consts.IAC_SCAN_TYPE) + + with pytest.raises(click.ClickException, match='IAC'): + scan_pre_commit(mock_ctx, repo_path='/repo') + + def test_dispatches_secret_scan_type_with_default_args(self) -> None: + from cycode.cli.apps.scan import commit_range_scanner + from cycode.cli.apps.scan.commit_range_scanner import scan_pre_commit + + mock_handler = Mock() + with patch.dict(commit_range_scanner._SCAN_TYPE_TO_PRE_COMMIT_HANDLER, {consts.SECRET_SCAN_TYPE: mock_handler}): + mock_ctx = self._make_ctx(consts.SECRET_SCAN_TYPE) + scan_pre_commit(mock_ctx, repo_path='/repo') + + mock_handler.assert_called_once_with(mock_ctx, '/repo', base_ref='HEAD', include_unstaged=False, paths=None) + + def test_dispatches_sca_scan_type_with_folded_in_flags(self) -> None: + from cycode.cli.apps.scan import commit_range_scanner + from cycode.cli.apps.scan.commit_range_scanner import scan_pre_commit + + mock_handler = Mock() + with patch.dict(commit_range_scanner._SCAN_TYPE_TO_PRE_COMMIT_HANDLER, {consts.SCA_SCAN_TYPE: mock_handler}): + mock_ctx = self._make_ctx(consts.SCA_SCAN_TYPE) + scan_pre_commit( + mock_ctx, repo_path='/repo', base_ref='abc123', include_unstaged=True, paths=['/repo/file.py'] + ) + + mock_handler.assert_called_once_with( + mock_ctx, '/repo', base_ref='abc123', include_unstaged=True, paths=['/repo/file.py'] + ) + + def test_dispatches_sast_scan_type(self) -> None: + from cycode.cli.apps.scan import commit_range_scanner + from cycode.cli.apps.scan.commit_range_scanner import scan_pre_commit + + mock_handler = Mock() + with patch.dict(commit_range_scanner._SCAN_TYPE_TO_PRE_COMMIT_HANDLER, {consts.SAST_SCAN_TYPE: mock_handler}): + mock_ctx = self._make_ctx(consts.SAST_SCAN_TYPE) + scan_pre_commit(mock_ctx, repo_path='/repo') + + mock_handler.assert_called_once_with(mock_ctx, '/repo', base_ref='HEAD', include_unstaged=False, paths=None) diff --git a/tests/cli/commands/scan/test_pre_commit_command.py b/tests/cli/commands/scan/test_pre_commit_command.py new file mode 100644 index 00000000..d62159a1 --- /dev/null +++ b/tests/cli/commands/scan/test_pre_commit_command.py @@ -0,0 +1,258 @@ +import os +import tempfile +from collections.abc import Generator +from contextlib import contextmanager +from unittest.mock import MagicMock, patch + +import pytest +import typer +from git import Repo +from typer.testing import CliRunner + +from cycode.cli.apps.scan.pre_commit.pre_commit_command import ( + _resolve_repo_root, + _validate_base_ref, + pre_commit_command, +) +from cycode.cli.exceptions.custom_exceptions import UnresolvedGitRefError + + +@contextmanager +def temporary_git_repository() -> Generator[tuple[str, Repo], None, None]: + with tempfile.TemporaryDirectory() as temp_dir: + repo = Repo.init(temp_dir, b='main') + try: + yield temp_dir, repo + finally: + repo.close() + + +class TestValidateBaseRef: + def test_valid_base_ref_does_not_raise(self) -> None: + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'file.txt') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['file.txt']) + repo.index.commit('initial') + + _validate_base_ref(temp_dir, 'HEAD') + + def test_invalid_base_ref_raises_unresolved_git_ref_error(self) -> None: + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'file.txt') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['file.txt']) + repo.index.commit('initial') + + with pytest.raises(UnresolvedGitRefError): + _validate_base_ref(temp_dir, 'not-a-real-ref') + + def test_empty_repository_with_default_head_does_not_raise(self) -> None: + with temporary_git_repository() as (temp_dir, _repo): + _validate_base_ref(temp_dir, 'HEAD') + + +class TestResolveRepoRoot: + """Running from any subdirectory of the repo must resolve to the actual repo root. + + Regression test: git_proxy.get_repo() requires an exact match (root or .git dir) unless + told to search parent directories, so a naive `os.getcwd()` breaks the moment the command + is invoked from a subdirectory -- exactly how an IDE plugin scoped to the open file's + folder, or a workspace subdirectory in a monorepo, would invoke it. The git pre-commit hook + framework always invokes from the repo root, so this is a no-op there. + """ + + def test_resolves_root_from_subdirectory(self) -> None: + with temporary_git_repository() as (temp_dir, repo): + os.makedirs(os.path.join(temp_dir, 'sub')) + file_path = os.path.join(temp_dir, 'sub', 'app.py') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['sub/app.py']) + repo.index.commit('initial') + + resolved_root = _resolve_repo_root(os.path.join(temp_dir, 'sub')) + + assert os.path.realpath(resolved_root) == os.path.realpath(temp_dir) + + def test_resolves_root_when_already_at_root(self) -> None: + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'file.txt') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['file.txt']) + repo.index.commit('initial') + + resolved_root = _resolve_repo_root(temp_dir) + + assert os.path.realpath(resolved_root) == os.path.realpath(temp_dir) + + +class TestPreCommitCommandPathResolution: + """A relative --path value must reach scan_pre_commit already resolved to absolute. + + Regression test: `paths` is handed to git as a pathspec, and git resolves relative + pathspecs against the repo root -- not the process cwd. Invoking from a subdirectory + (`--path app.py` from inside `sub/`) would therefore target `/app.py` instead + of `/sub/app.py`, silently scanning the wrong file or nothing at all. + `resolve_path=True` pins the path to the caller's cwd before git ever sees it. + """ + + def test_relative_path_option_is_resolved_to_absolute(self, monkeypatch: pytest.MonkeyPatch) -> None: + original_cwd = os.getcwd() + with temporary_git_repository() as (temp_dir, repo): + os.makedirs(os.path.join(temp_dir, 'sub')) + file_path = os.path.join(temp_dir, 'sub', 'app.py') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['sub/app.py']) + repo.index.commit('initial') + + app = typer.Typer() + app.command()(pre_commit_command) + + # Actually chdir rather than patching os.getcwd(): on Windows/Python 3.9, Click's + # path resolution does not consistently go through the patched os.getcwd symbol, + # so the mock silently has no effect there and the test passes for the wrong reason + # (or, as happened in CI, resolves against the real process cwd instead). + monkeypatch.chdir(temp_dir) + with patch('cycode.cli.apps.scan.pre_commit.pre_commit_command.scan_pre_commit') as mock_scan: + result = CliRunner().invoke(app, ['--path', 'sub/app.py'], obj=MagicMock()) + + # monkeypatch only restores the cwd at fixture teardown, which runs after this test + # function returns -- but temporary_git_repository()'s cleanup (below, at the end of + # this `with` block) runs now, inside the test. Windows can't delete a directory that + # is still the process's cwd, so we must chdir back out before that cleanup happens. + monkeypatch.chdir(original_cwd) + + assert result.exit_code == 0, result.output + mock_scan.assert_called_once() + _, kwargs = mock_scan.call_args + assert [os.path.realpath(p) for p in kwargs['paths']] == [ + os.path.realpath(os.path.join(temp_dir, 'sub', 'app.py')) + ] + + +class TestPreCommitCommandPathContainment: + """--path must be validated against the repo root using normalized (realpath) forms. + + Regression test: CI on windows-latest surfaced a false-positive rejection here. GitPython's + `working_tree_dir` can come back in Windows' 8.3 short-name form (e.g. `RUNNE~1`), while + Click's `resolve_path=True` expands `--path` to the long form -- an in-repo path then looks + like it's outside the repo under a naive string comparison. Reproduced here in a way that + doesn't depend on Windows-only short names: repo_path is returned with an unresolved `..` + segment, which only a realpath-normalized comparison sees through. + """ + + def test_unresolved_repo_root_form_does_not_reject_an_in_repo_path(self, monkeypatch: pytest.MonkeyPatch) -> None: + original_cwd = os.getcwd() + with temporary_git_repository() as (temp_dir, repo): + os.makedirs(os.path.join(temp_dir, 'sub')) + file_path = os.path.join(temp_dir, 'sub', 'app.py') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['sub/app.py']) + repo.index.commit('initial') + + app = typer.Typer() + app.command()(pre_commit_command) + + monkeypatch.chdir(temp_dir) + unresolved_repo_path = os.path.join(temp_dir, 'sub', '..') + with ( + patch( + 'cycode.cli.apps.scan.pre_commit.pre_commit_command._resolve_repo_root', + return_value=unresolved_repo_path, + ), + patch('cycode.cli.apps.scan.pre_commit.pre_commit_command.scan_pre_commit') as mock_scan, + ): + result = CliRunner().invoke(app, ['--path', 'sub/app.py'], obj=MagicMock()) + monkeypatch.chdir(original_cwd) + + assert result.exit_code == 0, result.output + mock_scan.assert_called_once() + + def test_path_outside_repo_raises_scan_path_outside_repository_error(self, monkeypatch: pytest.MonkeyPatch) -> None: + original_cwd = os.getcwd() + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'file.txt') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['file.txt']) + repo.index.commit('initial') + + with tempfile.TemporaryDirectory() as outside_dir: + app = typer.Typer() + app.command()(pre_commit_command) + + monkeypatch.chdir(temp_dir) + with patch('cycode.cli.apps.scan.pre_commit.pre_commit_command.scan_pre_commit') as mock_scan: + result = CliRunner().invoke(app, ['--path', os.path.join(outside_dir, 'file.txt')], obj=MagicMock()) + monkeypatch.chdir(original_cwd) + + assert result.exit_code == 0, result.output + mock_scan.assert_not_called() + + +class TestPreCommitCommandFromSubdirectory: + """End-to-end: invoking the command from a repo subdirectory must not fail.""" + + def test_scan_pre_commit_called_with_repo_root_not_subdirectory(self, monkeypatch: pytest.MonkeyPatch) -> None: + original_cwd = os.getcwd() + with temporary_git_repository() as (temp_dir, repo): + os.makedirs(os.path.join(temp_dir, 'sub')) + file_path = os.path.join(temp_dir, 'sub', 'app.py') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['sub/app.py']) + repo.index.commit('initial') + + app = typer.Typer() + app.command()(pre_commit_command) + + # Actually chdir rather than patching os.getcwd() -- see the comment in + # TestPreCommitCommandPathResolution for why the mock is unreliable on Windows. + monkeypatch.chdir(os.path.join(temp_dir, 'sub')) + with patch('cycode.cli.apps.scan.pre_commit.pre_commit_command.scan_pre_commit') as mock_scan: + result = CliRunner().invoke(app, [], obj=MagicMock()) + + # See the matching comment in TestPreCommitCommandPathResolution: must chdir back out + # before temporary_git_repository()'s cleanup runs, or Windows can't delete the dir. + monkeypatch.chdir(original_cwd) + + assert result.exit_code == 0, result.output + mock_scan.assert_called_once() + args, _kwargs = mock_scan.call_args + assert os.path.realpath(args[1]) == os.path.realpath(temp_dir) + + +class TestPreCommitCommandDefaults: + """The default invocation (no new flags) must match the pre-existing hook behavior exactly.""" + + def test_default_invocation_uses_head_and_staged_only(self, monkeypatch: pytest.MonkeyPatch) -> None: + original_cwd = os.getcwd() + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'file.txt') + with open(file_path, 'w') as f: + f.write('content') + repo.index.add(['file.txt']) + repo.index.commit('initial') + + app = typer.Typer() + app.command()(pre_commit_command) + + monkeypatch.chdir(temp_dir) + # Simulate the pre-commit framework's own auto-passed staged filenames -- must be + # silently swallowed, not treated as a --path scope, per the ignored hidden argument. + with patch('cycode.cli.apps.scan.pre_commit.pre_commit_command.scan_pre_commit') as mock_scan: + result = CliRunner().invoke(app, ['file.txt'], obj=MagicMock()) + monkeypatch.chdir(original_cwd) + + assert result.exit_code == 0, result.output + mock_scan.assert_called_once() + _, kwargs = mock_scan.call_args + assert kwargs['base_ref'] == 'HEAD' + assert kwargs['include_unstaged'] is False + assert kwargs['paths'] is None diff --git a/tests/cli/files_collector/test_commit_range_documents.py b/tests/cli/files_collector/test_commit_range_documents.py index 4cde5ec4..ccbba7e4 100644 --- a/tests/cli/files_collector/test_commit_range_documents.py +++ b/tests/cli/files_collector/test_commit_range_documents.py @@ -16,6 +16,7 @@ collect_commit_range_diff_documents, get_diff_file_path, get_pre_commit_framework_push_range, + get_pre_commit_modified_documents, get_safe_head_reference_for_diff, get_staged_diff_index, parse_commit_range, @@ -1232,3 +1233,253 @@ def test_collect_reports_added_lines_for_child_commit(self) -> None: documents = collect_commit_range_diff_documents(mock_ctx, temp_dir, f'{root_commit.hexsha}..HEAD') assert len(documents) == 1 assert '+secret' in documents[0].content + + +class TestGetPreCommitModifiedDocuments: + """Test get_pre_commit_modified_documents across its (base_ref, include_unstaged) combinations.""" + + @staticmethod + def _mock_progress_bar() -> Mock: + mock_progress_bar = Mock() + mock_progress_bar.set_section_length = Mock() + mock_progress_bar.update = Mock() + return mock_progress_bar + + def test_default_call_matches_default_arguments(self) -> None: + """Regression-critical: calling with no base_ref/include_unstaged/paths at all (the exact + pre-commit hook invocation) must behave identically to passing the documented defaults. + """ + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'tracked.txt') + with open(file_path, 'w') as f: + f.write('line1\n') + repo.index.add(['tracked.txt']) + repo.index.commit('initial') + + with open(file_path, 'a') as f: + f.write('staged\n') + repo.index.add(['tracked.txt']) + + no_args_result = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + ) + explicit_defaults_result = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + base_ref='HEAD', + include_unstaged=False, + paths=None, + ) + + no_args_contents = [(d.path, d.content, d.is_git_diff_format) for docs in no_args_result for d in docs] + explicit_contents = [ + (d.path, d.content, d.is_git_diff_format) for docs in explicit_defaults_result for d in docs + ] + assert no_args_contents == explicit_contents + + def test_default_staged_only_ignores_unstaged_changes(self) -> None: + """The default (include_unstaged=False) diff/patch channel must only ever reflect staged + content, matching today's pre-commit hook behavior exactly -- an unstaged edit on top must + not leak into `diff_docs`. `work_docs` always reflects the live on-disk file regardless of + staging state (true both before and after this change -- it's a plain file read, with no + awareness of the index), so it legitimately includes the unstaged line too. + + Files are written with newline='' so the on-disk (and therefore committed) content is + exactly the LF bytes given, regardless of platform -- otherwise Python's default text-mode + write translates '\\n' to the OS line ending, and on Windows the resulting CRLF blob makes + the `from_docs[0].content == 'line1\\n'` assertion below fail with a trailing '\\r'. + """ + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'tracked.txt') + with open(file_path, 'w', newline='') as f: + f.write('line1\n') + repo.index.add(['tracked.txt']) + repo.index.commit('initial') + + with open(file_path, 'a', newline='') as f: + f.write('staged\n') + repo.index.add(['tracked.txt']) + + with open(file_path, 'a', newline='') as f: + f.write('unstaged\n') + + from_docs, work_docs, diff_docs = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + ) + + assert len(from_docs) == 1 + assert from_docs[0].content == 'line1\n' + + assert len(work_docs) == 1 + assert work_docs[0].content == 'line1\nstaged\nunstaged\n' + + assert len(diff_docs) == 1 + assert diff_docs[0].is_git_diff_format is True + assert '+staged' in diff_docs[0].content + assert '+unstaged' not in diff_docs[0].content + + def test_include_unstaged_combines_staged_and_unstaged_changes(self) -> None: + """A tracked file with both a staged and an unstaged edit should appear as a single diff/document + once include_unstaged=True. Files use newline='' -- see the comment in + test_default_staged_only_ignores_unstaged_changes for why. + """ + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'tracked.txt') + with open(file_path, 'w', newline='') as f: + f.write('line1\n') + repo.index.add(['tracked.txt']) + repo.index.commit('initial') + + with open(file_path, 'a', newline='') as f: + f.write('staged\n') + repo.index.add(['tracked.txt']) + + with open(file_path, 'a', newline='') as f: + f.write('unstaged\n') + + from_docs, work_docs, diff_docs = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + include_unstaged=True, + ) + + assert len(from_docs) == 1 + assert from_docs[0].content == 'line1\n' + + assert len(work_docs) == 1 + assert work_docs[0].content == 'line1\nstaged\nunstaged\n' + + assert len(diff_docs) == 1 + assert diff_docs[0].is_git_diff_format is True + assert '+staged' in diff_docs[0].content + assert '+unstaged' in diff_docs[0].content + + def test_scopes_to_requested_paths(self) -> None: + """Only files under the requested paths should be collected.""" + with temporary_git_repository() as (temp_dir, repo): + os.makedirs(os.path.join(temp_dir, 'sub')) + + included_path = os.path.join(temp_dir, 'sub', 'app.py') + excluded_path = os.path.join(temp_dir, 'excluded.py') + with open(included_path, 'w') as f: + f.write("print('old')\n") + with open(excluded_path, 'w') as f: + f.write("print('old')\n") + repo.index.add(['sub/app.py', 'excluded.py']) + repo.index.commit('initial') + + with open(included_path, 'a') as f: + f.write("print('new')\n") + with open(excluded_path, 'a') as f: + f.write("print('new')\n") + repo.index.add(['sub/app.py', 'excluded.py']) + + _from_docs, work_docs, diff_docs = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + paths=[os.path.join(temp_dir, 'sub')], + ) + + diff_paths = {doc.path for doc in diff_docs} + work_paths = {doc.path for doc in work_docs} + + assert diff_paths == {get_path_by_os(included_path)} + assert work_paths == {get_path_by_os(included_path)} + + def test_include_unstaged_uses_explicit_base_ref_not_just_head(self) -> None: + """Diffing against an older ref should show changes made since that ref, not just since HEAD.""" + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'tracked.txt') + with open(file_path, 'w') as f: + f.write('A\n') + repo.index.add(['tracked.txt']) + first_commit = repo.index.commit('A') + + with open(file_path, 'a') as f: + f.write('B\n') + repo.index.add(['tracked.txt']) + repo.index.commit('B') + + with open(file_path, 'a') as f: + f.write('unstaged-C\n') + + _from_docs, _work_docs, diff_docs = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + base_ref=first_commit.hexsha, + include_unstaged=True, + ) + + assert len(diff_docs) == 1 + assert '+B' in diff_docs[0].content + assert '+unstaged-C' in diff_docs[0].content + + def test_staged_only_against_non_head_base_ref_shows_correct_diff_direction(self) -> None: + """The new combination: staged-only (include_unstaged=False) diffed against an older ref. + + Verifies GitPython's Diffable.diff() defaulting `other` to the index produces the same, + correctly-directed patch as `git diff --cached ` -- this is the one code path + with no prior coverage (see plan: verified manually against raw git before implementing). + """ + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'tracked.txt') + with open(file_path, 'w') as f: + f.write('A\n') + repo.index.add(['tracked.txt']) + first_commit = repo.index.commit('A') + + with open(file_path, 'a') as f: + f.write('B\n') + repo.index.add(['tracked.txt']) + repo.index.commit('B') + + with open(file_path, 'a') as f: + f.write('staged-C\n') + repo.index.add(['tracked.txt']) + + with open(file_path, 'a') as f: + f.write('unstaged-D\n') + + _from_docs, _work_docs, diff_docs = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + base_ref=first_commit.hexsha, + include_unstaged=False, + ) + + assert len(diff_docs) == 1 + assert diff_docs[0].content.count('+B') == 1 + assert diff_docs[0].content.count('+staged-C') == 1 + assert 'unstaged-D' not in diff_docs[0].content + # every changed line must be an addition, never a removal, for this pure-append history + assert '-A' not in diff_docs[0].content + assert '-B' not in diff_docs[0].content + + def test_empty_repository_falls_back_to_empty_tree(self) -> None: + """A repository with zero commits should treat all staged content as new, not error out.""" + with temporary_git_repository() as (temp_dir, repo): + file_path = os.path.join(temp_dir, 'new_file.txt') + with open(file_path, 'w') as f: + f.write('brand-new-content\n') + repo.index.add(['new_file.txt']) + + from_docs, work_docs, diff_docs = get_pre_commit_modified_documents( + progress_bar=self._mock_progress_bar(), + progress_bar_section='prepare', + repo_path=temp_dir, + ) + + assert from_docs == [] + assert len(work_docs) == 1 + assert work_docs[0].content == 'brand-new-content\n' + assert len(diff_docs) == 1 + assert '+brand-new-content' in diff_docs[0].content