diff --git a/CHANGELOG.md b/CHANGELOG.md index 525d50d..3cf1f08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,37 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ## [Unreleased] +### Fixed +- `changed_files` diff-only mode always resolved to zero files in the pre-built + Docker GitHub Action: the container runs as root while the checkout is owned + by the runner user, so git's ownership check refused every diff lookup, the + scope silently resolved to nothing, and the scanners skipped with a green + run. Git subprocesses now mark the scan workspace as `safe.directory` via + command-scope `GIT_CONFIG_*` environment entries. No config files are + touched, and caller-provided `GIT_CONFIG_*` entries (including the previously + documented workaround) are preserved. The same mismatch broke git-based + repository/branch/commit and default-branch discovery in local Docker runs; + those lookups are covered by the same change. +- A failed `changed_files` diff resolution is no longer indistinguishable from + an empty diff. Git errors are captured and logged instead of discarded, and + when the scope cannot be resolved — unreadable repository, unresolvable base + ref, or `pr` mode with no base ref — Socket Basics now **fails with a + configuration error** rather than reporting a green run that scanned nothing. + Shallow checkouts get a more specific error naming `fetch-depth: 0`. A + genuinely empty diff (e.g. a delete-only PR) is a successful resolution and + still skips the scanners as before. + +### Added +- The resolved `changed_files` scope is now logged on every scoped run: file + count at INFO, the full file list at DEBUG — so an empty diff and a failed + lookup are visible and distinguishable in run logs. +- `scan_all` is now a declared action input and doubles as the fail-open escape + hatch for `changed_files`: when the scope cannot be resolved, widen to a + full-repo scan with a warning instead of failing. Every enabled scanner + widens consistently on that failure path. A successfully resolved scope + remains authoritative, including a genuinely empty diff, which still skips + scoped scanners. + ## [3.0.0] - 2026-08-06 Major release: Trivy-backed scanning returns, now built and published through diff --git a/action.yml b/action.yml index 67eeb35..f846dda 100644 --- a/action.yml +++ b/action.yml @@ -11,6 +11,7 @@ runs: INPUT_WORKSPACE: ${{ inputs.workspace }} # Scan scope INPUT_CHANGED_FILES: ${{ inputs.changed_files }} + INPUT_SCAN_ALL: ${{ inputs.scan_all }} INPUT_SCAN_FILES: ${{ inputs.scan_files }} # Input mappings for all parameters INPUT_ALL_LANGUAGES_ENABLED: ${{ inputs.all_languages_enabled }} @@ -118,9 +119,20 @@ inputs: GITHUB_BASE_REF), or 'current-commit'. For PR/'auto' modes, check out with actions/checkout fetch-depth: 0 so the base branch is available. When the diff resolves to no existing files (e.g. a delete-only PR) the scanners - are skipped rather than scanning the whole repo. + are skipped rather than scanning the whole repo. When the diff cannot be + resolved at all (unreadable repo, missing base ref) the run fails with a + configuration error instead of reporting a green scan of nothing; set + scan_all to widen to a full-repo scan in that case instead. required: false default: "" + scan_all: + description: >- + Fail-open escape hatch for changed_files: when a requested diff cannot be + resolved, scan the full workspace instead of failing. Every enabled + scanner widens consistently. A successfully resolved scope still wins, + including a genuinely empty diff, which skips scoped scanners. + required: false + default: "false" scan_files: description: >- Explicit comma-separated list of files to scan. Scopes SAST/OpenGrep, diff --git a/docs/github-action.md b/docs/github-action.md index c2e577f..67d0ae4 100644 --- a/docs/github-action.md +++ b/docs/github-action.md @@ -324,6 +324,41 @@ jobs: > nothing rather than falling back to the whole repo. To scan an explicit file > list regardless of git state, use the `scan_files` input instead. +> [!NOTE] +> **When the diff cannot be resolved** — the checkout is unreadable, or the base +> branch is missing (most commonly a shallow clone without `fetch-depth: 0`) — +> Socket Basics **fails with a configuration error** naming the underlying git +> error. It does not scan. +> +> This is deliberate. Diff-only scoping is an explicit instruction, and if it +> cannot be honored there is no honest result to report: +> +> - **Skipping the scanners** would exit green having scanned zero files. A +> passing check that inspected nothing is worse than a failing one, and a +> warning buried in a run log is not something anyone acts on. +> - **Silently scanning everything** would do the expensive thing on every PR — +> precisely what asking for a diff scope was avoiding. On a large repository +> that is a slow or OOM-prone check, and it reports **pre-existing** findings +> rather than the PR's own, so a checkout misconfiguration surfaces as large PR +> comments on every PR until corrected. +> +> If the error appears on **every** PR, the cause is almost always a missing +> `fetch-depth: 0` — fix the checkout rather than sizing up the runner. Shallow +> checkouts get a more specific error naming that fix directly, including the +> `no merge base` shape where the base tip was fetched without connecting +> history. +> +> **To scan anyway, set `scan_all: true`.** That widens an unresolvable scope to +> a full-repository scan with a warning instead of failing. Every enabled scanner +> widens consistently on that failure path. `scan_all` does not override a scope +> that resolved successfully: the changed files remain authoritative, and a +> genuinely empty diff still skips the scoped scanners. +> +> A genuinely *empty* diff (e.g. a delete-only PR) is a successful resolution and +> still skips the scanners — only a **failed** resolution errors. The resolved +> file count is logged on every scoped run, so an empty diff and a failed lookup +> are always distinguishable in the logs. + ## PR Comment Customization Socket Basics automatically posts enhanced PR comments with **smart defaults that work out of the box** — clickable file links, collapsible sections, syntax highlighting, CVE links, CVSS scores, and auto-labels are all enabled by default. diff --git a/docs/parameters.md b/docs/parameters.md index 22adfeb..ea03fa9 100644 --- a/docs/parameters.md +++ b/docs/parameters.md @@ -120,6 +120,24 @@ PR), the scanners are skipped rather than falling back to scanning the whole repository. For PR/`auto`/`pr` modes, check out with full history (e.g. `actions/checkout` with `fetch-depth: 0`) so the base branch is available. +If the diff **cannot be resolved** — unreadable repository, missing base ref, or +a shallow checkout with no base to diff against — the run **fails with a +configuration error** naming the underlying git error, and nothing is scanned. +Neither alternative is honest: skipping the scanners exits green having scanned +zero files, and widening to the whole repository does the expensive thing on +every PR, which is what requesting a diff scope was avoiding. Shallow checkouts +get a more specific error naming `fetch-depth: 0`, covering both the missing-ref +and disconnected-history (`no merge base`) shapes. + +Set **`scan_all`** to widen instead of failing: an unresolvable scope then falls +back to a full-workspace scan with a warning, consistently across every enabled +scanner. A scope that resolves successfully remains authoritative even when +`scan_all` is set; a genuinely empty diff still skips the scoped scanners. + +The resolved scope is logged on every run (file count at INFO, full file list +at DEBUG), so an empty diff and a failed lookup are distinguishable in run +logs. + **Example:** ```bash socket-basics --changed-files auto diff --git a/socket_basics/core/config.py b/socket_basics/core/config.py index f202d83..c705421 100644 --- a/socket_basics/core/config.py +++ b/socket_basics/core/config.py @@ -275,9 +275,12 @@ def get_scan_targets(self) -> List[str]: """Determine files to scan based on configuration. Precedence (highest to lowest): - 1. ``scan_all`` -> scan the entire workspace (explicit override). - 2. ``changed_files`` -> scope the scan to the PR/diff changed files - (diff-only mode; mirrors how Socket SCA Pull Request alerts behave). + 1. A successfully resolved ``changed_files`` request -> scope the + scan to the PR/diff changed files (diff-only mode; mirrors how + Socket SCA Pull Request alerts behave). + 2. ``scan_all`` -> scan the entire workspace. When supplied with a + changed-files request, this is the fail-open fallback used only + when that request could not be resolved. 3. ``scan_files`` -> explicit user-provided file list. 4. default -> scan the entire workspace. @@ -287,10 +290,6 @@ def get_scan_targets(self) -> List[str]: rather than falling back to scanning the whole workspace or their own working directory. """ - # Explicit "scan everything" override. - if self.get('scan_all', False): - return [str(self.workspace)] - # Diff-only mode: scope the scan to the files changed in the PR/commit. # Keep honoring the scope when git resolves to zero files, e.g. a # delete-only PR, so callers skip instead of scanning the workspace. @@ -298,6 +297,12 @@ def get_scan_targets(self) -> List[str]: if changed_files or self.get('changed_files_scope_requested', False): return self._resolve_file_targets(changed_files) + # Explicit fail-open fallback. A failed changed-files resolution clears + # changed_files_scope_requested before Config is constructed, so this + # branch is reached only when no successful changed-files scope exists. + if self.get('scan_all', False): + return [str(self.workspace)] + # Explicit list of files to scan. if self.scan_files: return self._resolve_file_targets(self.scan_files) @@ -1650,39 +1655,77 @@ def create_config_from_args(args) -> Config: if changed_files_arg: val = str(changed_files_arg).strip() config_dict['changed_files_scope_requested'] = True - # 'auto' resolves to the PR base-ref diff in CI, else staged changes. - if val.lower() == 'auto': + _scope_log = logging.getLogger(__name__) + + def _apply_scoped_changed_files(mode_label: str, **detect_kwargs) -> None: + """Resolve the diff scope, distinguishing failure from an empty diff. + + A failed resolution (None) is a configuration error and stops the + run. Diff-only scoping is an explicit instruction; if it cannot be + honored, Socket Basics cannot make any statement about the code, and + both alternatives are worse than stopping. Skipping the scanners + reports a green check having scanned nothing — false assurance, and a + warning in a run log is not a signal anyone acts on. Silently + widening to the whole repository does the expensive thing on every + PR, which is precisely what a caller asking for a diff scope was + avoiding. + + ``scan_all`` is the opt-in for that widening: when the requested + scope cannot be resolved, scan everything rather than failing. A + successfully resolved scope remains authoritative. + + A genuinely empty diff (e.g. a delete-only PR) is a successful + resolution — it keeps the empty scope and skips, as before. + """ + fail_open = bool(config_dict.get('scan_all', False)) try: - git_changed = _detect_git_changed_files(config_dict.get('workspace', os.getcwd()), mode='auto') - config_dict['changed_files'] = git_changed + resolved = _detect_git_changed_files( + config_dict.get('workspace', os.getcwd()), fail_open=fail_open, **detect_kwargs + ) except Exception as e: - logging.getLogger(__name__).warning("Warning: failed to detect git changed files (auto): %s", e) - config_dict['changed_files'] = [] + _scope_log.warning("Warning: failed to detect git changed files (%s): %s", mode_label, e) + resolved = None + if resolved is None: + if fail_open: + _scope_log.warning( + "changed_files scope could not be resolved (%s); scan_all is set, so " + "falling back to a full-repo scan. See the warnings above for the " + "underlying git error.", + mode_label, + ) + config_dict['changed_files'] = [] + config_dict['changed_files_scope_requested'] = False + return + raise SystemExit( + f"changed_files: the requested scope ({mode_label}) could not be resolved, so " + "the scan would either report a green run having scanned nothing or silently " + "widen to the whole repository. See the warnings above for the underlying git " + "error. Fix the git problem, or set scan_all to widen to a full-repo scan when " + "the scope cannot be resolved." + ) + _scope_log.info("changed_files scope resolved to %d file(s) (%s)", len(resolved), mode_label) + if resolved: + _scope_log.debug("changed_files scope: %s", ", ".join(resolved)) + else: + _scope_log.info( + "changed_files diff is genuinely empty (e.g. delete-only change); " + "scoped scanners will be skipped" + ) + config_dict['changed_files'] = resolved + + # 'auto' resolves to the PR base-ref diff in CI, else staged changes. + if val.lower() == 'auto': + _apply_scoped_changed_files('auto', mode='auto') elif val.lower() == 'pr': # Explicit PR diff against the base branch (GITHUB_BASE_REF). - try: - git_changed = _detect_git_changed_files(config_dict.get('workspace', os.getcwd()), mode='pr') - config_dict['changed_files'] = git_changed - except Exception as e: - logging.getLogger(__name__).warning("Warning: failed to detect git changed files (pr): %s", e) - config_dict['changed_files'] = [] + _apply_scoped_changed_files('pr', mode='pr') elif val.lower() in ('current-commit', 'current_commit'): - try: - git_changed = _detect_git_changed_files(config_dict.get('workspace', os.getcwd()), mode='current-commit') - config_dict['changed_files'] = git_changed - except Exception as e: - logging.getLogger(__name__).warning("Warning: failed to detect git changed files (current-commit): %s", e) - config_dict['changed_files'] = [] + _apply_scoped_changed_files('current-commit', mode='current-commit') else: # If value looks like a commit hash, list files in that commit import re if re.match(r'^[0-9a-fA-F]{7,40}$', val): - try: - git_changed = _detect_git_changed_files(config_dict.get('workspace', os.getcwd()), mode='commit', commit=val) - config_dict['changed_files'] = git_changed - except Exception as e: - logging.getLogger(__name__).warning("Warning: failed to detect git changed files (commit %s): %s", val, e) - config_dict['changed_files'] = [] + _apply_scoped_changed_files(f'commit {val}', mode='commit', commit=val) else: # parse comma-separated list of files provided manually config_dict['changed_files'] = [f.strip() for f in val.split(',') if f.strip()] @@ -1721,7 +1764,55 @@ def create_config_from_args(args) -> Config: return Config(config_dict) -def _detect_git_changed_files(workspace_path: str, mode: str = 'staged', commit: str | None = None, base_ref: str | None = None) -> List[str]: +def _git_env(workspace_path: str | Path | None = None) -> Dict[str, str]: + """Environment for git subprocesses that marks the scan workspace safe to read. + + The pre-built GitHub Action runs as root inside a Docker container while the + checkout at ``GITHUB_WORKSPACE`` is owned by the runner user, so git's + ownership check (git 2.35.2+) refuses the repository and every git lookup + here fails. ``changed_files`` diff-only mode then resolves to zero files and + the scanners silently skip. ``actions/checkout`` cannot help: its + ``safe.directory`` entry is written to the runner's global config, which is + not mounted into container actions. + + The workspace is an explicit scan target, not an incidentally discovered + repository, so mark it safe for these subprocesses only. Injecting via + ``GIT_CONFIG_*`` (command-scope config, honored for ``safe.directory`` since + git 2.38; the bundled image ships newer) touches no config files, and + appending after any caller-provided ``GIT_CONFIG_*`` entries preserves + workarounds users already deployed. The path is resolved to an absolute one + first because git ignores relative ``safe.directory`` values. + """ + env = dict(os.environ) + try: + count = max(0, int(env.get('GIT_CONFIG_COUNT', '0') or '0')) + except ValueError: + count = 0 + ws = workspace_path or os.environ.get('GITHUB_WORKSPACE') or os.getcwd() + env[f'GIT_CONFIG_KEY_{count}'] = 'safe.directory' + env[f'GIT_CONFIG_VALUE_{count}'] = str(Path(ws).resolve()) + env['GIT_CONFIG_COUNT'] = str(count + 1) + return env + + +class _GitScopeError(Exception): + """A git invocation needed for changed-files scoping failed outright. + + Carries git's first stderr line as the message. ``ref_miss`` is True when + the failure only means "this ref does not exist" (safe to try another + candidate) rather than "git could not read the repository at all." + ``merge_base_miss`` is True when the ref exists but shares no history with + HEAD (``A...HEAD: no merge base``) — the signature of a partial/shallow + fetch where the base tip was fetched without connecting history. + """ + + def __init__(self, message: str, ref_miss: bool = False, merge_base_miss: bool = False): + super().__init__(message) + self.ref_miss = ref_miss + self.merge_base_miss = merge_base_miss + + +def _detect_git_changed_files(workspace_path: str, mode: str = 'staged', commit: str | None = None, base_ref: str | None = None, fail_open: bool = False) -> Optional[List[str]]: """Detect changed files in a git repository. mode: @@ -1735,11 +1826,23 @@ def _detect_git_changed_files(workspace_path: str, mode: str = 'staged', commit: (``GITHUB_BASE_REF`` is set), otherwise staged changes. This is what ``--changed-files auto`` resolves to. - Returns a list of file paths relative to the workspace root. If not a git - repo or detection fails, returns []. + Returns a list of file paths relative to the workspace root. An empty list + means git resolved the diff and it is genuinely empty (e.g. a delete-only + change), or the workspace is not a git repo (nothing to diff). Returns + ``None`` when resolution *failed* — git could not read the repository, or a + requested base ref could not be resolved — so callers can distinguish "no + changed files" from "the lookup broke" instead of silently scanning + nothing. The specific git error is logged here at WARNING level. + + ``fail_open`` mirrors the caller's ``scan_all`` setting. When False (the + default) a deterministic misconfiguration raises SystemExit here with the + specific fix, rather than returning None for the caller to turn into a + generic error. When True the caller has opted into widening an unresolvable + scope to a full-repo scan, so those checks are skipped and the failure is + reported as None. """ + log = logging.getLogger(__name__) try: - from subprocess import check_output, CalledProcessError import subprocess # Prefer GITHUB_WORKSPACE if set (GitHub Actions environment) @@ -1757,70 +1860,159 @@ def _detect_git_changed_files(workspace_path: str, mode: str = 'staged', commit: if not git_dir.exists(): return [] + # Mark the workspace safe for the git subprocesses below; without this + # every command fails under the container-action ownership mismatch and + # the diff silently resolves to nothing. + git_env = _git_env(ws) + + # stderr markers that mean "this ref does not exist" — a soft miss the + # base-ref candidate loop may retry — as opposed to git being unable + # to read the repository at all (ownership, corruption, ...). + ref_miss_markers = ('unknown revision', 'bad revision', 'ambiguous argument') + + def _split(out: str) -> List[str]: + return [line.strip() for line in out.splitlines() if line.strip()] + + def _run_git(args: List[str]) -> List[str]: + """Run git, returning stdout lines; raise _GitScopeError on failure. + + stderr is captured rather than discarded so the failure reason — + e.g. git's self-diagnosing ``dubious ownership`` message — survives + into the logs instead of being indistinguishable from an empty diff. + """ + res = subprocess.run(args, text=True, capture_output=True, env=git_env) + if res.returncode != 0: + stderr = (res.stderr or '').strip() + first = stderr.splitlines()[0] if stderr else f'exit code {res.returncode}' + miss = any(m in stderr.lower() for m in ref_miss_markers) + mb_miss = 'no merge base' in stderr.lower() + raise _GitScopeError(first, ref_miss=miss, merge_base_miss=mb_miss) + return _split(res.stdout) + # Change to workspace directory before running git commands # This ensures git runs in the correct repository context original_cwd = os.getcwd() try: os.chdir(str(ws)) - def _split(out: str) -> List[str]: - return [line.strip() for line in out.splitlines() if line.strip()] + def _fail_fast_if_shallow(ref: str) -> None: + """Deterministic misconfiguration check for an unresolvable base. + + A shallow checkout (no ``fetch-depth: 0``) can *never* resolve + the base ref, so every PR on this checkout would fail the same + way. Name the one-line fix here instead of letting the caller + report a generic unresolvable-scope error. Follows the existing + SystemExit convention for unrecoverable configuration errors + (see repository/branch discovery below). + + Skipped under ``fail_open``: the caller set ``scan_all`` and has + opted into widening an unresolvable scope instead of failing. + """ + if fail_open: + return + try: + shallow = _run_git(['git', 'rev-parse', '--is-shallow-repository']) == ['true'] + except Exception: + return # probe failed; fall through to the generic fallback + if shallow: + raise SystemExit( + f"changed_files: cannot diff against base ref '{ref}' (missing ref or no " + "shared history) and this checkout is shallow. Set 'fetch-depth: 0' on " + "actions/checkout (or otherwise fetch the base branch with full history) " + "so the diff has a base to compare against." + ) + + def _resolve_base_diff(ref: str) -> Optional[List[str]]: + """Base diff with the shallow fail-fast applied to both failure shapes. + + A shallow misconfiguration shows up either as a missing base ref + (nothing fetched) or as ``no merge base`` (base tip fetched but + history disconnected). Both are deterministic — fail fast when + shallow; otherwise preserve the original failure semantics. + """ + if not ref: + return None + try: + files = _diff_against_base(ref) + except _GitScopeError as e: + if e.merge_base_miss: + _fail_fast_if_shallow(ref) + log.warning("changed_files scope: base ref %r shares no merge base with HEAD (%s)", ref, e) + return None + raise + if files is None: + _fail_fast_if_shallow(ref) + return files def _diff_against_base(ref: str) -> Optional[List[str]]: """Diff changed files (excluding deletions) against a base ref. Tries the remote-tracking ref (``origin/``) first, then the - bare ref. Returns None when neither ref can be resolved so the - caller can fall back to another detection strategy. The - ``--diff-filter=ACMR`` excludes deleted paths so they never - become scan targets. + bare ref. Returns None when neither candidate resolves (the ref + does not exist locally); raises _GitScopeError when git itself + cannot read the repository. The ``--diff-filter=ACMR`` excludes + deleted paths so they never become scan targets. """ if not ref: return None + last_miss = '' for candidate in (f'origin/{ref}', ref): try: - out = check_output( - ['git', 'diff', '--name-only', '--diff-filter=ACMR', f'{candidate}...HEAD'], - text=True, stderr=subprocess.DEVNULL, - ) - return _split(out) - except CalledProcessError: - continue + return _run_git(['git', 'diff', '--name-only', '--diff-filter=ACMR', f'{candidate}...HEAD']) + except _GitScopeError as e: + if e.ref_miss: + last_miss = str(e) + continue + raise + log.warning("changed_files scope: base ref %r could not be resolved (%s)", ref, last_miss or 'no candidates tried') return None if mode == 'auto': # Prefer the PR base-ref diff in CI; fall back to staged changes # for local/pre-commit use. base = base_ref or os.environ.get('GITHUB_BASE_REF', '') - pr_files = _diff_against_base(base) + pr_files = _resolve_base_diff(base) if pr_files is not None: return pr_files - out = check_output(['git', 'diff', '--name-only', '--cached'], text=True, stderr=subprocess.DEVNULL) - return _split(out) + if base: + # A base ref was provided (we are in a PR context) but could + # not be diffed against, and the shallow fail-fast did not + # apply (non-shallow checkout). Report failure so the caller + # falls back to a full scan rather than the staged diff, + # which is almost always empty in CI. + return None + return _run_git(['git', 'diff', '--name-only', '--cached']) elif mode == 'pr': base = base_ref or os.environ.get('GITHUB_BASE_REF', '') - return _diff_against_base(base) or [] + if not base: + log.warning("changed_files scope: mode 'pr' but no base ref available (GITHUB_BASE_REF unset)") + return None + return _resolve_base_diff(base) elif mode == 'staged': # staged but not yet committed - out = check_output(['git', 'diff', '--name-only', '--cached'], text=True, stderr=subprocess.DEVNULL) - return _split(out) + return _run_git(['git', 'diff', '--name-only', '--cached']) elif mode == 'current-commit': # files that are part of HEAD commit - out = check_output(['git', 'diff-tree', '--no-commit-id', '--name-only', '-r', 'HEAD'], text=True, stderr=subprocess.DEVNULL) - return _split(out) + return _run_git(['git', 'diff-tree', '--no-commit-id', '--name-only', '-r', 'HEAD']) elif mode == 'commit' and commit: - out = check_output(['git', 'diff-tree', '--no-commit-id', '--name-only', '-r', commit], text=True, stderr=subprocess.DEVNULL) - return _split(out) + return _run_git(['git', 'diff-tree', '--no-commit-id', '--name-only', '-r', commit]) else: return [] finally: # Always restore original working directory os.chdir(original_cwd) - except CalledProcessError: - return [] - except Exception: - return [] + except _GitScopeError as e: + msg = str(e) + hint = '' + if 'dubious ownership' in msg.lower(): + hint = (" — the checkout is owned by a different user; the workspace should be" + " marked safe.directory automatically as of this release, so please report this") + log.warning("changed_files scope: git failed: %s%s", msg, hint) + return None + except Exception as e: + log.warning("changed_files scope: unexpected error during git detection: %s", e) + return None def discover_all_files(workspace_path: str, respect_gitignore: bool = True) -> List[str]: @@ -2016,9 +2208,10 @@ def _discover_repository(cli_repo: str | None, github_repository: str = '', gith # 4. Git information try: url = subprocess.check_output( - ['git', 'config', '--get', 'remote.origin.url'], - text=True, - stderr=subprocess.DEVNULL + ['git', 'config', '--get', 'remote.origin.url'], + text=True, + stderr=subprocess.DEVNULL, + env=_git_env() ).strip() if url.endswith('.git'): @@ -2085,9 +2278,10 @@ def _discover_branch(cli_branch: str | None, github_head_ref: str = '', github_r # 4. Git information try: branch = subprocess.check_output( - ['git', 'rev-parse', '--abbrev-ref', 'HEAD'], + ['git', 'rev-parse', '--abbrev-ref', 'HEAD'], text=True, - stderr=subprocess.DEVNULL + stderr=subprocess.DEVNULL, + env=_git_env() ).strip() if branch and branch != 'HEAD': @@ -2122,9 +2316,10 @@ def _discover_commit_hash() -> str: # 2. Git information try: commit = subprocess.check_output( - ['git', 'rev-parse', '--short', 'HEAD'], + ['git', 'rev-parse', '--short', 'HEAD'], text=True, - stderr=subprocess.DEVNULL + stderr=subprocess.DEVNULL, + env=_git_env() ).strip() if commit: @@ -2166,7 +2361,8 @@ def _discover_is_default_branch(current_branch: str, workspace_path: str = '') - ['git', 'symbolic-ref', 'refs/remotes/origin/HEAD'], text=True, stderr=subprocess.DEVNULL, - cwd=cwd + cwd=cwd, + env=_git_env(workspace_path) ).strip() # Extract branch name from refs/remotes/origin/branch-name @@ -2187,7 +2383,8 @@ def _discover_is_default_branch(current_branch: str, workspace_path: str = '') - ['git', 'ls-remote', '--symref', 'origin', 'HEAD'], text=True, stderr=subprocess.DEVNULL, - cwd=cwd + cwd=cwd, + env=_git_env(workspace_path) ).strip() # Parse the output: "ref: refs/heads/main\tHEAD" diff --git a/socket_basics/core/connector/base.py b/socket_basics/core/connector/base.py index 9574544..564af21 100644 --- a/socket_basics/core/connector/base.py +++ b/socket_basics/core/connector/base.py @@ -205,7 +205,20 @@ def get_scan_targets(self) -> List[str]: List of file paths or directories to scan """ return self.config.get_scan_targets() if hasattr(self.config, 'get_scan_targets') else [] - + + def _changed_files_scope_requested(self) -> bool: + """Return whether the user requested a changed-files scope. + + Connectors that derive their own targets must not substitute staged + files or the workspace when a successful scope resolved to nothing. + The configuration layer clears this flag only when resolution failed + and ``scan_all`` explicitly opted into the full-scan fallback. + """ + try: + return bool(self.config.get('changed_files_scope_requested', False)) + except Exception: + return False + def get_name(self) -> str: """Get the connector name diff --git a/socket_basics/core/connector/trivy/trivy.py b/socket_basics/core/connector/trivy/trivy.py index c4f518b..be2ff47 100644 --- a/socket_basics/core/connector/trivy/trivy.py +++ b/socket_basics/core/connector/trivy/trivy.py @@ -139,18 +139,46 @@ def scan_dockerfiles(self) -> Dict[str, Any]: else: dockerfiles = [] - # Try to detect changed Dockerfiles even if none explicitly configured - changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] - if not changed_files: + # A configured changed-files scope is authoritative. Only use the + # connector's staged-file convenience fallback when neither a scope nor + # scan_all was requested. The configuration layer clears the scope flag + # when a failed resolution plus scan_all opts into a full scan; scan_all + # must not then be narrowed again by incidental staged files. + configured_changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] + scope_requested = bool(configured_changed_files) or self._changed_files_scope_requested() + changed_files = configured_changed_files + if ( + not changed_files + and not scope_requested + and not self.config.get('scan_all', False) + ): try: from socket_basics.core.config import _detect_git_changed_files - changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') + changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') or [] except Exception: changed_files = [] - # If explicit dockerfiles are not set, but changed Dockerfiles exist, use them - if not dockerfiles and changed_files: - # Filter changed files for Dockerfile candidates + # A successful changed-files scope always narrows this scanner. If it + # contains no existing Dockerfile, there is nothing for this scanner to + # do; configured Dockerfiles must not replace the requested scope. + if scope_requested: + changed_dockerfiles = [] + for cf in changed_files: + base = Path(cf).name + if base == 'Dockerfile' or 'dockerfile' in base.lower() or base.lower().endswith('.dockerfile'): + if (self.config.workspace / cf).exists(): + changed_dockerfiles.append(cf) + if not changed_dockerfiles: + logger.info( + "Trivy Dockerfile scan skipped: the changed-files scope contains no " + "existing Dockerfile" + ) + return {} + logger.info(f"Detected {len(changed_dockerfiles)} changed Dockerfile(s); restricting Trivy to them") + dockerfiles = changed_dockerfiles + elif not dockerfiles and changed_files: + # Preserve the connector's existing staged-file convenience when + # nobody requested a changed-files scope. possible = [] for cf in changed_files: base = Path(cf).name @@ -167,17 +195,10 @@ def scan_dockerfiles(self) -> Dict[str, Any]: logger.info("Running Trivy Dockerfile scanning") results = {} - # If changed_files is provided, prefer scanning only changed Dockerfiles - changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] - # Fallback: attempt to detect staged changed files if none present - if not changed_files: - try: - # import helper from config module - from socket_basics.core.config import _detect_git_changed_files - changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') - except Exception: - changed_files = [] - if changed_files: + # For an unrequested staged-file hint, retain the existing behavior of + # narrowing when it names Dockerfiles and otherwise leaving the + # configured list alone. + if changed_files and not scope_requested: # Filter changed files down to ones that are Dockerfiles or named 'Dockerfile' changed_dockerfiles = [] for cf in changed_files: @@ -322,10 +343,15 @@ def scan_vulnerabilities(self) -> Dict[str, Any]: # Check for changed files to restrict scanning changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] - if not changed_files: + scope_requested = bool(changed_files) or self._changed_files_scope_requested() + if ( + not changed_files + and not scope_requested + and not self.config.get('scan_all', False) + ): try: from socket_basics.core.config import _detect_git_changed_files - changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') + changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') or [] except Exception: changed_files = [] @@ -351,8 +377,17 @@ def scan_vulnerabilities(self) -> Dict[str, Any]: if scan_paths: logger.info(f"Restricting Trivy scan to {len(scan_paths)} changed directory(ies)") - # If no changed files or no valid paths, scan entire workspace + # A successful changed-files scope remains authoritative when it is + # empty or none of its paths survive. Only a failed resolution with the + # explicit scan_all fallback clears the scope flag and reaches the + # workspace fallback below. if not scan_paths: + if scope_requested: + logger.info( + "Trivy vulnerability scan skipped: the changed-files scope resolved to " + "no scannable paths" + ) + return results scan_paths = [workspace_path] for scan_path in scan_paths: @@ -1096,4 +1131,4 @@ def generate_notifications(self, components: List[Dict[str, Any]], item_name: st def get_name(self) -> str: """Return the display name for this connector""" - return "Trivy" \ No newline at end of file + return "Trivy" diff --git a/socket_basics/core/connector/trufflehog/__init__.py b/socket_basics/core/connector/trufflehog/__init__.py index 43eef14..eff74e0 100644 --- a/socket_basics/core/connector/trufflehog/__init__.py +++ b/socket_basics/core/connector/trufflehog/__init__.py @@ -223,12 +223,21 @@ def scan(self) -> Dict[str, Any]: exclude_file_path = None exclude_patterns: List[str] = [] try: - # Prefer explicit changed_files, fallback to git staged + # Prefer explicit changed_files, falling back to git staged only + # when neither a changed-files scope nor scan_all was requested. A + # successful scope that resolved empty must stay empty; after a + # failed resolution, scan_all must use the workspace target rather + # than an incidental staged-file scope. changed_files = self.config.get('changed_files', []) if hasattr(self.config, '_config') else [] - if not changed_files: + scope_requested = self._changed_files_scope_requested() + if ( + not changed_files + and not scope_requested + and not self.config.get('scan_all', False) + ): try: from socket_basics.core.config import _detect_git_changed_files - changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') + changed_files = _detect_git_changed_files(str(self.config.workspace), mode='staged') or [] except Exception: changed_files = [] @@ -251,7 +260,7 @@ def scan(self) -> Dict[str, Any]: # If changed_files are present, pass those individual files; # otherwise use the configured targets (including scan_files). - if changed_files: + if changed_files or scope_requested: target_candidates = [self.config.workspace / cf for cf in changed_files] excluded_target_message = "Skipping excluded changed file: %s" all_excluded_message = "All changed files were excluded from TruffleHog scanning" diff --git a/tests/test_changed_files_scope.py b/tests/test_changed_files_scope.py index 3eda06d..2a05f32 100644 --- a/tests/test_changed_files_scope.py +++ b/tests/test_changed_files_scope.py @@ -5,13 +5,22 @@ re-scanning the whole repository. """ +import json import os import subprocess from argparse import Namespace +from pathlib import Path +from types import SimpleNamespace import pytest -from socket_basics.core.config import Config, _detect_git_changed_files, create_config_from_args +from socket_basics.core.config import ( + Config, + _detect_git_changed_files, + _discover_repository, + _git_env, + create_config_from_args, +) def _make_config(workspace, **overrides): @@ -27,11 +36,22 @@ def test_default_scans_whole_workspace(self, tmp_path): (tmp_path / "a.py").write_text("x = 1") assert _make_config(tmp_path).get_scan_targets() == [str(tmp_path)] - def test_scan_all_returns_workspace(self, tmp_path): + def test_resolved_changed_files_outrank_scan_all_fallback(self, tmp_path): (tmp_path / "a.py").write_text("x = 1") cfg = _make_config(tmp_path, scan_all=True, changed_files=["a.py"]) - # scan_all is an explicit override and wins over changed_files - assert cfg.get_scan_targets() == [str(tmp_path)] + assert cfg.get_scan_targets() == [str(tmp_path / "a.py")] + + def test_empty_resolved_scope_outranks_scan_all_fallback(self, tmp_path): + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=[], + changed_files_scope_requested=True, + ) + assert cfg.get_scan_targets() == [] + + def test_scan_all_without_a_successful_scope_returns_workspace(self, tmp_path): + assert _make_config(tmp_path, scan_all=True).get_scan_targets() == [str(tmp_path)] def test_changed_files_scopes_to_existing_files(self, tmp_path): (tmp_path / "a.py").write_text("x = 1") @@ -170,3 +190,452 @@ def test_delete_only_pr_config_creation_keeps_empty_scope(self, tmp_path, monkey assert cfg.get("changed_files") == [] assert cfg.get_scan_targets() == [] + + +def _git_refuses_repo(repo): + """True when git, told to assume a different owner, refuses to read `repo`. + + ``GIT_TEST_ASSUME_DIFFERENT_OWNER`` is git's own test knob for the + ownership check behind ``safe.directory``; it makes every repository look + like it belongs to another user, which is exactly what a checkout looks + like from inside the container action. Used as a control so these tests + skip (instead of passing vacuously) on a git build without the knob. + """ + probe = subprocess.run( + ["git", "-C", str(repo), "rev-parse", "--show-toplevel"], + capture_output=True, + text=True, + env={**os.environ, "GIT_TEST_ASSUME_DIFFERENT_OWNER": "1"}, + ) + return probe.returncode != 0 + + +class TestDubiousOwnership: + """Git subprocesses must survive the container-action ownership mismatch. + + The pre-built Docker action runs as root while the checkout is owned by + the runner user; without ``safe.directory`` git refuses the repo, the diff + resolves to zero files, and the scanners silently skip. + """ + + def test_pr_diff_survives_dubious_ownership(self, pr_repo, monkeypatch): + for i in range(3): + monkeypatch.delenv(f"GIT_CONFIG_KEY_{i}", raising=False) + monkeypatch.delenv(f"GIT_CONFIG_VALUE_{i}", raising=False) + monkeypatch.delenv("GIT_CONFIG_COUNT", raising=False) + + if not _git_refuses_repo(pr_repo): + pytest.skip("this git build does not honor GIT_TEST_ASSUME_DIFFERENT_OWNER") + + monkeypatch.setenv("GIT_TEST_ASSUME_DIFFERENT_OWNER", "1") + files = _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="main") + assert sorted(files) == ["base.py", "feat.py"] + + def test_relative_workspace_survives_dubious_ownership(self, pr_repo, monkeypatch): + # git ignores relative safe.directory values, so the injected path must + # be absolutized even when --workspace is given as a relative path. + if not _git_refuses_repo(pr_repo): + pytest.skip("this git build does not honor GIT_TEST_ASSUME_DIFFERENT_OWNER") + + monkeypatch.chdir(pr_repo.parent) + monkeypatch.setenv("GIT_TEST_ASSUME_DIFFERENT_OWNER", "1") + files = _detect_git_changed_files(pr_repo.name, mode="pr", base_ref="main") + assert sorted(files) == ["base.py", "feat.py"] + + def test_repo_discovery_survives_dubious_ownership(self, pr_repo, monkeypatch): + _git(pr_repo, "remote", "add", "origin", "https://github.com/acme/demo.git") + + if not _git_refuses_repo(pr_repo): + pytest.skip("this git build does not honor GIT_TEST_ASSUME_DIFFERENT_OWNER") + + monkeypatch.chdir(pr_repo) + monkeypatch.setenv("GIT_TEST_ASSUME_DIFFERENT_OWNER", "1") + assert _discover_repository(None, "", "") == "acme/demo" + + +class TestGitEnv: + """_git_env injects safe.directory without clobbering caller config.""" + + def test_injects_safe_directory_for_workspace(self, monkeypatch): + monkeypatch.delenv("GIT_CONFIG_COUNT", raising=False) + env = _git_env("/scan/me") + assert env["GIT_CONFIG_COUNT"] == "1" + assert env["GIT_CONFIG_KEY_0"] == "safe.directory" + assert env["GIT_CONFIG_VALUE_0"] == "/scan/me" + + def test_appends_after_caller_provided_entries(self, monkeypatch): + # A user already deploying the documented env-var workaround must not + # have their entry clobbered. + monkeypatch.setenv("GIT_CONFIG_COUNT", "1") + monkeypatch.setenv("GIT_CONFIG_KEY_0", "user.name") + monkeypatch.setenv("GIT_CONFIG_VALUE_0", "runner") + env = _git_env("/scan/me") + assert env["GIT_CONFIG_COUNT"] == "2" + assert env["GIT_CONFIG_KEY_0"] == "user.name" + assert env["GIT_CONFIG_VALUE_0"] == "runner" + assert env["GIT_CONFIG_KEY_1"] == "safe.directory" + assert env["GIT_CONFIG_VALUE_1"] == "/scan/me" + + def test_garbage_count_treated_as_zero(self, monkeypatch): + monkeypatch.setenv("GIT_CONFIG_COUNT", "not-a-number") + env = _git_env("/scan/me") + assert env["GIT_CONFIG_COUNT"] == "1" + assert env["GIT_CONFIG_KEY_0"] == "safe.directory" + + def test_workspace_value_is_absolute(self, monkeypatch, tmp_path): + monkeypatch.delenv("GIT_CONFIG_COUNT", raising=False) + monkeypatch.chdir(tmp_path) + env = _git_env("some/relative/dir") + assert Path(env["GIT_CONFIG_VALUE_0"]).is_absolute() + + def test_defaults_to_github_workspace(self, monkeypatch): + monkeypatch.delenv("GIT_CONFIG_COUNT", raising=False) + monkeypatch.setenv("GITHUB_WORKSPACE", "/github/workspace") + env = _git_env() + assert env["GIT_CONFIG_VALUE_0"] == "/github/workspace" + + +class TestScopeResolutionFailure: + """Failed diff resolution must be distinguishable from an empty diff. + + A git failure (unreadable repo, unresolvable base ref) returns None from the + detection helper, and the config layer turns that into a configuration error + — never a green run that silently scanned nothing, and never a silent widen + to the whole repository. ``scan_all`` opts into the widen instead. A + genuinely empty diff still returns [] and keeps the skip behavior (see the + delete-only test above). + """ + + def test_unreadable_repo_returns_none(self, pr_repo): + # Corrupt HEAD so every git command fails hard (not a ref miss). + (pr_repo / ".git" / "HEAD").write_text("garbage") + result = _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="main") + assert result is None + + def test_unresolvable_base_ref_returns_none(self, pr_repo): + result = _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="no-such-branch") + assert result is None + + def test_auto_with_unresolvable_base_ref_returns_none(self, pr_repo, monkeypatch): + # In a PR context (base ref set) an unresolvable base must NOT quietly + # fall back to the (usually empty) staged diff. + monkeypatch.setenv("GITHUB_BASE_REF", "no-such-branch") + result = _detect_git_changed_files(str(pr_repo), mode="auto") + assert result is None + + def test_pr_mode_without_base_ref_returns_none(self, pr_repo, monkeypatch): + monkeypatch.delenv("GITHUB_BASE_REF", raising=False) + result = _detect_git_changed_files(str(pr_repo), mode="pr") + assert result is None + + def test_failure_logs_git_stderr(self, pr_repo, caplog): + (pr_repo / ".git" / "HEAD").write_text("garbage") + with caplog.at_level("WARNING"): + _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="main") + assert any("changed_files scope" in r.getMessage() for r in caplog.records) + + def test_config_creation_fails_closed_on_failure(self, pr_repo, monkeypatch): + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.delenv("INPUT_SCAN_ALL", raising=False) + monkeypatch.setenv("GITHUB_BASE_REF", "main") + (pr_repo / ".git" / "HEAD").write_text("garbage") + + # Scope resolution failed -> configuration error. Neither a green run + # that scanned nothing nor a silent full-repo scan. + with pytest.raises(SystemExit, match="could not be resolved"): + create_config_from_args(_config_args(pr_repo, "auto")) + + def test_config_error_names_scan_all_escape_hatch(self, pr_repo, monkeypatch): + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.delenv("INPUT_SCAN_ALL", raising=False) + monkeypatch.setenv("GITHUB_BASE_REF", "main") + (pr_repo / ".git" / "HEAD").write_text("garbage") + + with pytest.raises(SystemExit, match="scan_all"): + create_config_from_args(_config_args(pr_repo, "auto")) + + def test_scan_all_opts_into_full_scan_fallback(self, pr_repo, monkeypatch, caplog): + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.setenv("GITHUB_BASE_REF", "main") + monkeypatch.setenv("INPUT_SCAN_ALL", "true") + (pr_repo / ".git" / "HEAD").write_text("garbage") + + with caplog.at_level("WARNING"): + cfg = create_config_from_args(_config_args(pr_repo, "auto")) + + # scan_all is the documented fail-open opt-in: widen, do not fail. + assert cfg.get("changed_files") == [] + assert cfg.get("changed_files_scope_requested") is False + assert cfg.get_scan_targets() == [str(pr_repo)] + assert any("falling back to a full-repo scan" in r.getMessage() for r in caplog.records) + + def test_config_creation_logs_resolved_count(self, pr_repo, monkeypatch, caplog): + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.setenv("GITHUB_BASE_REF", "main") + + with caplog.at_level("INFO"): + cfg = create_config_from_args(_config_args(pr_repo, "auto")) + + assert sorted(cfg.get("changed_files")) == ["base.py", "feat.py"] + assert any("resolved to 2 file(s)" in r.getMessage() for r in caplog.records) + + +class TestShallowCheckoutFailFast: + """A shallow checkout that cannot resolve the base ref is a deterministic + misconfiguration (missing fetch-depth: 0), so the detection helper raises + with that one-line fix rather than returning a generic failure. Other + failures return None and let the config layer report the generic + unresolvable-scope error. Under ``fail_open`` (the caller set ``scan_all``) + the check is skipped so the widen can happen instead. + """ + + def test_shallow_missing_base_fails_fast_pr_mode(self, pr_repo): + (pr_repo / ".git" / "shallow").touch() + with pytest.raises(SystemExit, match="fetch-depth"): + _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="no-such-branch") + + def test_shallow_missing_base_fails_fast_auto_mode(self, pr_repo, monkeypatch): + (pr_repo / ".git" / "shallow").touch() + monkeypatch.setenv("GITHUB_BASE_REF", "no-such-branch") + with pytest.raises(SystemExit, match="fetch-depth"): + _detect_git_changed_files(str(pr_repo), mode="auto") + + def test_shallow_with_resolvable_base_still_diffs(self, pr_repo): + # Shallowness alone is fine — only shallow AND unresolvable-base fails. + (pr_repo / ".git" / "shallow").touch() + files = _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="main") + assert sorted(files) == ["base.py", "feat.py"] + + def test_non_shallow_missing_base_returns_none(self, pr_repo): + assert _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="no-such-branch") is None + + def test_fail_open_skips_shallow_fail_fast(self, pr_repo): + # scan_all was set, so the caller wants to widen rather than fail: the + # shallow check must not pre-empt that with a SystemExit. + (pr_repo / ".git" / "shallow").touch() + result = _detect_git_changed_files( + str(pr_repo), mode="pr", base_ref="no-such-branch", fail_open=True + ) + assert result is None + + def test_fail_open_skips_no_merge_base_fail_fast(self, pr_repo): + _git(pr_repo, "checkout", "--orphan", "disconnected") + _git(pr_repo, "add", "-A") + _git(pr_repo, "commit", "-m", "orphan") + (pr_repo / ".git" / "shallow").touch() + result = _detect_git_changed_files( + str(pr_repo), mode="pr", base_ref="main", fail_open=True + ) + assert result is None + + def test_fail_open_still_resolves_a_good_diff(self, pr_repo): + # fail_open only affects failure handling, never a successful diff. + files = _detect_git_changed_files( + str(pr_repo), mode="pr", base_ref="main", fail_open=True + ) + assert sorted(files) == ["base.py", "feat.py"] + + def test_config_creation_propagates_config_error(self, pr_repo, monkeypatch): + monkeypatch.delenv("GITHUB_WORKSPACE", raising=False) + monkeypatch.setenv("GITHUB_BASE_REF", "no-such-branch") + (pr_repo / ".git" / "shallow").touch() + with pytest.raises(SystemExit, match="fetch-depth"): + create_config_from_args(_config_args(pr_repo, "auto")) + + def test_shallow_no_merge_base_fails_fast(self, pr_repo, monkeypatch): + # Base tip exists but shares no history with HEAD (partial fetch shape): + # `A...HEAD: no merge base`. Shallow -> config error, same as missing ref. + _git(pr_repo, "checkout", "--orphan", "disconnected") + _git(pr_repo, "add", "-A") + _git(pr_repo, "commit", "-m", "orphan") + (pr_repo / ".git" / "shallow").touch() + with pytest.raises(SystemExit, match="fetch-depth"): + _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="main") + + def test_non_shallow_no_merge_base_returns_none(self, pr_repo): + _git(pr_repo, "checkout", "--orphan", "disconnected") + _git(pr_repo, "add", "-A") + _git(pr_repo, "commit", "-m", "orphan") + assert _detect_git_changed_files(str(pr_repo), mode="pr", base_ref="main") is None + + +class TestTruffleHogScopePolicy: + """TruffleHog must use the same successful-scope/failure distinction.""" + + @staticmethod + def _scanner(config): + from socket_basics.core.connector.trufflehog import TruffleHogScanner + + scanner = TruffleHogScanner(config) + scanner._process_results = lambda findings: {} + scanner.generate_notifications = lambda components: {} + return scanner + + def test_successful_empty_scope_skips_even_with_scan_all(self, tmp_path, monkeypatch): + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=[], + changed_files_scope_requested=True, + secret_scanning_enabled=True, + trufflehog_exclude_dir="", + trufflehog_show_unverified=False, + ) + staged_calls = [] + invocations = [] + monkeypatch.setattr( + "socket_basics.core.config._detect_git_changed_files", + lambda *args, **kwargs: staged_calls.append(kwargs.get("mode")) or ["staged.py"], + ) + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", + lambda command, **kwargs: invocations.append(command) + or SimpleNamespace(returncode=0, stdout="", stderr=""), + ) + + assert self._scanner(cfg).scan() == {} + assert staged_calls == [] + assert invocations == [] + + def test_scan_all_after_failed_resolution_scans_workspace(self, tmp_path, monkeypatch): + (tmp_path / "staged.py").write_text("token = 'abc'\n") + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=[], + changed_files_scope_requested=False, + secret_scanning_enabled=True, + trufflehog_exclude_dir="", + trufflehog_show_unverified=False, + ) + staged_calls = [] + invocations = [] + monkeypatch.setattr( + "socket_basics.core.config._detect_git_changed_files", + lambda *args, **kwargs: staged_calls.append(kwargs.get("mode")) or ["staged.py"], + ) + monkeypatch.setattr( + "socket_basics.core.connector.trufflehog.subprocess.run", + lambda command, **kwargs: invocations.append(command) + or SimpleNamespace(returncode=0, stdout="", stderr=""), + ) + + self._scanner(cfg).scan() + assert staged_calls == [] + assert invocations == [ + [ + "trufflehog", + "filesystem", + "--json", + "--no-verification", + str(tmp_path), + ] + ] + + +def _record_trivy_paths(monkeypatch): + scanned = [] + + def fake_run(command, **kwargs): + scanned.append(command[-1]) + output_path = Path(command[command.index("--output") + 1]) + output_path.write_text(json.dumps({"Results": []})) + return SimpleNamespace(returncode=0, stdout="", stderr="") + + monkeypatch.setattr( + "socket_basics.core.connector.trivy.trivy.subprocess.run", + fake_run, + ) + return scanned + + +class TestTrivyScopePolicy: + """Trivy's custom target builders must honor the same scope matrix.""" + + @staticmethod + def _scanner(config): + from socket_basics.core.connector.trivy.trivy import TrivyScanner + + return TrivyScanner(config) + + def test_successful_empty_scope_skips_vulnerability_scan_with_scan_all( + self, tmp_path, monkeypatch + ): + scanned = _record_trivy_paths(monkeypatch) + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=[], + changed_files_scope_requested=True, + trivy_vuln_enabled=True, + ) + + assert self._scanner(cfg).scan_vulnerabilities() == {} + assert scanned == [] + + def test_scan_all_after_failed_resolution_scans_workspace(self, tmp_path, monkeypatch): + (tmp_path / "staged").mkdir() + (tmp_path / "staged" / "requirements.txt").write_text("requests==2.0.0\n") + scanned = _record_trivy_paths(monkeypatch) + staged_calls = [] + monkeypatch.setattr( + "socket_basics.core.config._detect_git_changed_files", + lambda *args, **kwargs: staged_calls.append(kwargs.get("mode")) + or ["staged/requirements.txt"], + ) + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=[], + changed_files_scope_requested=False, + trivy_vuln_enabled=True, + ) + + self._scanner(cfg).scan_vulnerabilities() + assert staged_calls == [] + assert scanned == [str(tmp_path)] + + def test_successful_non_dockerfile_scope_skips_configured_dockerfile( + self, tmp_path, monkeypatch + ): + monkeypatch.chdir(tmp_path) + (tmp_path / "Dockerfile").write_text("FROM alpine\n") + (tmp_path / "app.py").write_text("x = 1\n") + scanned = _record_trivy_paths(monkeypatch) + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=["app.py"], + changed_files_scope_requested=True, + dockerfile_scanning_enabled=True, + dockerfiles="Dockerfile", + ) + + assert self._scanner(cfg).scan_dockerfiles() == {} + assert scanned == [] + + def test_scan_all_after_failed_resolution_scans_configured_dockerfile( + self, tmp_path, monkeypatch + ): + monkeypatch.chdir(tmp_path) + (tmp_path / "Dockerfile").write_text("FROM alpine\n") + (tmp_path / "staged.Dockerfile").write_text("FROM busybox\n") + scanned = _record_trivy_paths(monkeypatch) + staged_calls = [] + monkeypatch.setattr( + "socket_basics.core.config._detect_git_changed_files", + lambda *args, **kwargs: staged_calls.append(kwargs.get("mode")) + or ["staged.Dockerfile"], + ) + cfg = _make_config( + tmp_path, + scan_all=True, + changed_files=[], + changed_files_scope_requested=False, + dockerfile_scanning_enabled=True, + dockerfiles="Dockerfile", + ) + + self._scanner(cfg).scan_dockerfiles() + assert staged_calls == [] + assert scanned == ["Dockerfile"]