diff --git a/.github/workflows/pr-preview.yml b/.github/workflows/pr-preview.yml index 3a2ab1a9..7fba7ca3 100644 --- a/.github/workflows/pr-preview.yml +++ b/.github/workflows/pr-preview.yml @@ -261,6 +261,10 @@ jobs: - name: Build and push Docker preview uses: docker/build-push-action@53b7df96c91f9c12dcc8a07bcb9ccacbed38856a # v7.3.0 with: + # Build from the checked-out workspace, not the default Git context. The + # wheel is only present here as a downloaded artifact, and this also keeps + # Dockerfile.preview on the default-branch checkout rather than the PR ref. + context: . file: Dockerfile.preview push: true pull: true diff --git a/CHANGELOG.md b/CHANGELOG.md index c87837da..7e4bc6c4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,27 @@ # Changelog +## 2.6.5 + +### Changed: faster local scan setup for large repositories + +- Replaced repeated per-pattern recursive manifest globs with one streaming + filesystem walk per scan root. Excluded directories and `.git` are pruned + before descent, reducing filesystem metadata work without building a + repository-sized in-memory file index. +- Removed the unconditional `git fetch --all` from CLI initialization. Pull + request scans now use local refs first and fetch only a required base or head + ref when the checkout does not contain enough history. +- Added native Buildkite commit, branch, pull-request range, and GitHub SCM + configuration fallbacks so Buildkite jobs no longer need GitHub + Actions-shaped environment-variable shims. +- Added INFO-level timings for CLI run registration, organization setup, Git + initialization and fetches, changed-file detection, supported-pattern lookup, + and manifest discovery. +- Supported manifest patterns are cached for each CLI invocation once the API + returns them, so a transient lookup failure no longer keeps the run on the + smaller local fallback pattern set. Manifest results from `--sub-path` routing + are reused during scan creation. + ## 2.6.4 ### Changed: bump pinned @coana-tech/cli to 15.10.13 diff --git a/benchmarks/manifest_discovery.py b/benchmarks/manifest_discovery.py new file mode 100644 index 00000000..abc0e8bb --- /dev/null +++ b/benchmarks/manifest_discovery.py @@ -0,0 +1,101 @@ +#!/usr/bin/env python3 +"""Compare legacy per-pattern rglob discovery with the single-pass walker. + +This is an opt-in developer benchmark, not a timing assertion in the test +suite. It creates a synthetic monorepo so filesystem or CI-agent changes do not +make regular tests flaky. +""" + +import argparse +import tempfile +import time +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock + +from socketsecurity.core import Core +from socketsecurity.core.socket_config import SocketConfig +from socketsecurity.core.utils import socket_globs + + +def seed_tree(root: Path, directories: int, files_per_directory: int) -> None: + for directory_index in range(directories): + directory = root / "packages" / f"package-{directory_index:05d}" + directory.mkdir(parents=True) + (directory / "package.json").write_text("{}\n", encoding="utf-8") + for file_index in range(files_per_directory): + (directory / f"source-{file_index:03d}.txt").write_text( + "not a manifest\n", + encoding="utf-8", + ) + + # These trees model the expensive directories that the new walker prunes + # before descent rather than visiting once for every manifest pattern. + for excluded in (".git/objects", "node_modules/example", ".venv/site-packages"): + directory = root / excluded + directory.mkdir(parents=True) + for index in range(files_per_directory * 10): + (directory / f"object-{index:05d}").write_text("x", encoding="utf-8") + + +def legacy_discover(root: Path) -> set[str]: + results = set() + excluded_dirs = SocketConfig(api_key="benchmark").excluded_dirs + for ecosystem_patterns in socket_globs.values(): + for details in ecosystem_patterns.values(): + for pattern in Core.expand_brace_pattern(details["pattern"]): + insensitive = Core.to_case_insensitive_regex(pattern) + for candidate in root.rglob(insensitive): + if candidate.is_file() and not Core.is_excluded( + str(candidate), + excluded_dirs, + ): + results.add(candidate.as_posix()) + return results + + +def new_core() -> Core: + core = Core.__new__(Core) + core.config = SocketConfig(api_key="benchmark") + core.cli_config = SimpleNamespace(exclude_paths=None) + core.sdk = MagicMock() + core._supported_patterns = socket_globs + return core + + +def timed(function, root: Path) -> tuple[set[str], float]: + start = time.perf_counter() + results = set(function(root)) + return results, time.perf_counter() - start + + +def main() -> None: + parser = argparse.ArgumentParser() + parser.add_argument("--directories", type=int, default=500) + parser.add_argument("--files-per-directory", type=int, default=20) + args = parser.parse_args() + + with tempfile.TemporaryDirectory(prefix="socket-manifest-benchmark-") as temp: + root = Path(temp) + seed_tree(root, args.directories, args.files_per_directory) + legacy_results, legacy_seconds = timed(legacy_discover, root) + new_results, new_seconds = timed( + lambda path: new_core().find_files(str(path)), + root, + ) + + if legacy_results != new_results: + raise SystemExit( + "Manifest result mismatch: " + f"legacy={len(legacy_results)}, single_pass={len(new_results)}" + ) + + speedup = legacy_seconds / new_seconds if new_seconds else float("inf") + print(f"Manifests: {len(new_results)}") + print(f"Legacy per-pattern rglob: {legacy_seconds:.3f}s") + print(f"Single-pass walk: {new_seconds:.3f}s") + print(f"Speedup: {speedup:.1f}x") + + +if __name__ == "__main__": + main() diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 66193f38..061d18ea 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -81,6 +81,19 @@ steps: SOCKET_SECURITY_API_TOKEN: "${SOCKET_SECURITY_API_TOKEN}" ``` +The CLI reads Buildkite's native `BUILDKITE_COMMIT`, `BUILDKITE_BRANCH`, +`BUILDKITE_PULL_REQUEST`, and `BUILDKITE_PULL_REQUEST_BASE_BRANCH` variables. +For pull-request builds, ensure the checkout contains the base branch and the +checked-out head commit. The CLI uses those local refs first and performs a +targeted fetch only when a required ref or its comparison history is missing; +it does not fetch every remote ref and tag during startup. + +When `--scm github` is used from Buildkite, the CLI also derives GitHub comment +context from `BUILDKITE_REPO`, `BUILDKITE_BUILD_CHECKOUT_PATH`, and the variables +above. Set `GH_API_TOKEN` to a GitHub token with the required repository access. +GitHub Enterprise users should also set `GITHUB_API_URL`; GitHub.com defaults to +`https://api.github.com`. + #### Merge-base baselines in Buildkite (dynamic pipelines) Notes for using `--base-commit-sha` (see the diff --git a/pyproject.toml b/pyproject.toml index 313a8fae..48ae5907 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.6.4" +version = "2.6.5" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 9d9963d7..5875d7cb 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.6.4' +__version__ = '2.6.5' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index 7be24858..447f9ee2 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -1,3 +1,7 @@ +import copy +import fnmatch +import importlib +import json import logging import os import random @@ -6,10 +10,9 @@ import tarfile import tempfile import time -import json from dataclasses import asdict -from pathlib import Path, PurePath -from typing import Dict, List, Tuple, Set, TYPE_CHECKING, Optional +from pathlib import PurePath +from typing import TYPE_CHECKING, Dict, List, NamedTuple, Optional, Set, Tuple if TYPE_CHECKING: from socketsecurity.config import CliConfig @@ -18,21 +21,15 @@ from socketdev.fullscans import DiffArtifacts, FullScanParams, SocketArtifact from socketdev.org import Organization from socketdev.repos import RepositoryInfo -import copy -from socketsecurity import __version__, USER_AGENT -from socketsecurity.core.classes import ( - Alert, - Diff, - FullScan, - Issue, - Package, - Purl -) + +from socketsecurity import USER_AGENT, __version__ +from socketsecurity.core.classes import Alert, Diff, FullScan, Issue, Package, Purl from socketsecurity.core.exceptions import APIResourceNotFound + +from .resource_utils import check_file_count_against_ulimit from .socket_config import SocketConfig from .utils import socket_globs -from .resource_utils import check_file_count_against_ulimit -import importlib + logging_std = importlib.import_module("logging") @@ -125,6 +122,30 @@ def _humanize_alert_type(alert_type: str) -> str: return " ".join(part[:1].upper() + part[1:] for part in parts if part) +class ManifestPatterns(NamedTuple): + """Manifest patterns prepared once per scan root, case-folded for matching. + + The first three fields are the authoritative matchers used by + Core._matches_manifest_pattern. The candidate_* fields are a prefilter over + basenames alone: a manifest-discovery walk visits every file in the repository + but only a few hundred are manifests, so rejecting a name up front avoids + building a relative path and running a path match for the rest. The prefilter's + globs are pre-compiled into one alternation so the cost per rejected file stays + flat as the API's pattern list grows. + """ + + literal_basenames: Set[str] + basename_globs: List[str] + path_globs: List[str] + candidate_basenames: Set[str] + candidate_basename_regex: Optional["re.Pattern"] + + @property + def is_empty(self) -> bool: + """True when every ecosystem was filtered out, so the walk can be skipped.""" + return not (self.literal_basenames or self.basename_globs or self.path_globs) + + class Core: """Main class for interacting with Socket Security API and processing scan results.""" @@ -146,7 +167,13 @@ def __init__(self, config: SocketConfig, sdk: socketdev, cli_config: Optional['C self.config = config self.sdk = sdk self.cli_config = cli_config + self._supported_patterns: Optional[Dict] = None + org_start_time = time.perf_counter() self.set_org_vars() + log.info( + "Organization initialization completed in " + f"{time.perf_counter() - org_start_time:.2f}s" + ) def set_org_vars(self) -> None: """Sets the main shared configuration variables for organization access.""" @@ -421,6 +448,112 @@ def format_bytes(bytes_value): except Exception as e: log.error(f"Failed to save manifest tar.gz to {output_path}: {e}") + @staticmethod + def _prepare_manifest_patterns( + patterns: Dict, + ecosystems: Optional[List[str]], + excluded_ecosystems: List[str] + ) -> "ManifestPatterns": + """Prepare case-folded manifest patterns for a single filesystem walk. + + Literal basenames are kept in a set for the common fast path. Basename + globs and path-shaped globs are kept separately so the latter retain + pathlib's path-segment-aware matching behavior. The candidate basename + collections are derived here so the walker can reject a file on its name + alone; see ManifestPatterns. + """ + included_ecosystems = set(ecosystems) if ecosystems is not None else None + excluded = set(excluded_ecosystems) + literal_basenames: Set[str] = set() + basename_globs: Set[str] = set() + path_globs: Set[str] = set() + + for ecosystem, ecosystem_patterns in patterns.items(): + if included_ecosystems is not None and ecosystem not in included_ecosystems: + continue + if ecosystem in excluded: + continue + log.debug(f"Scanning ecosystem: {ecosystem}") + for details in ecosystem_patterns.values(): + original_pattern = details["pattern"] + for expanded in Core.expand_brace_pattern(original_pattern): + normalized = expanded.replace("\\", "/").casefold() + if "/" in normalized: + path_globs.add(normalized) + elif any(character in normalized for character in "*?["): + basename_globs.add(normalized) + else: + literal_basenames.add(normalized) + + # PurePath.match compares pattern segments right to left, so a path-shaped glob + # can only match a file whose basename matches the glob's final segment. Folding + # those final segments into the basename prefilter lets the walk skip the path + # match for everything else. An empty final segment (a trailing "/") constrains + # nothing, so it becomes "*" and the prefilter admits every name. + candidate_basenames = set(literal_basenames) + candidate_basename_globs = set(basename_globs) + for pattern in path_globs: + final_segment = pattern.rstrip("/").rsplit("/", 1)[-1] or "*" + if any(character in final_segment for character in "*?["): + candidate_basename_globs.add(final_segment) + else: + candidate_basenames.add(final_segment) + + return ManifestPatterns( + literal_basenames=literal_basenames, + basename_globs=sorted(basename_globs), + path_globs=sorted(path_globs), + candidate_basenames=candidate_basenames, + candidate_basename_regex=Core._compile_basename_globs(candidate_basename_globs), + ) + + @staticmethod + def _compile_basename_globs(globs: Set[str]) -> Optional["re.Pattern"]: + """Compile basename globs into a single alternation, or None if there are none. + + fnmatch.translate anchors the tail with ``\\Z`` and re.match anchors the head, + so each alternative matches exactly what fnmatch.fnmatchcase would. + """ + if not globs: + return None + return re.compile( + "|".join(f"(?:{fnmatch.translate(glob)})" for glob in sorted(globs)) + ) + + @staticmethod + def _basename_could_match(normalized_name: str, patterns: "ManifestPatterns") -> bool: + """Cheap prefilter: could a file with this basename match any manifest pattern? + + False is authoritative; True still has to be confirmed by + _matches_manifest_pattern against the scan-root-relative path. + """ + if normalized_name in patterns.candidate_basenames: + return True + return ( + patterns.candidate_basename_regex is not None + and patterns.candidate_basename_regex.match(normalized_name) is not None + ) + + @staticmethod + def _matches_manifest_pattern(relative_path: str, patterns: "ManifestPatterns") -> bool: + """Return whether a scan-root-relative path matches a manifest pattern.""" + normalized_path = relative_path.replace("\\", "/").casefold() + basename = normalized_path.rsplit("/", 1)[-1] + if basename in patterns.literal_basenames: + return True + if any(fnmatch.fnmatchcase(basename, pattern) for pattern in patterns.basename_globs): + return True + if not patterns.path_globs: + return False + + candidate = PurePath(normalized_path) + return any(candidate.match(pattern) for pattern in patterns.path_globs) + + @staticmethod + def _matches_excluded_directory(directory_name: str, excluded_dirs: Set[str]) -> bool: + """Match configured directory exclusions, including entries such as ``*.egg-info``.""" + return any(fnmatch.fnmatchcase(directory_name, pattern) for pattern in excluded_dirs) + def find_files(self, path: str, ecosystems: Optional[List[str]] = None) -> List[str]: """ Finds supported manifest files in the given path. @@ -432,8 +565,8 @@ def find_files(self, path: str, ecosystems: Optional[List[str]] = None) -> List[ Returns: List of found manifest file paths. """ - log.debug("Starting Find Files") - start_time = time.time() + log.debug("Starting manifest discovery") + start_time = time.perf_counter() files: Set[str] = set() # Unified --exclude-paths: filter discovered manifests by the same paths/globs that are @@ -447,50 +580,92 @@ def find_files(self, path: str, ecosystems: Optional[List[str]] = None) -> List[ exclude_paths = getattr(self.cli_config, "exclude_paths", None) if self.cli_config else None exclude_regexes = Core.compile_exclude_paths(exclude_paths) if exclude_paths else [] - # Get supported patterns from the API patterns = self.get_supported_patterns() + manifest_patterns = self._prepare_manifest_patterns( + patterns, + ecosystems, + self.config.excluded_ecosystems, + ) - for ecosystem in patterns: - # If ecosystems filter is provided, only include specified ecosystems - if ecosystems is not None and ecosystem not in ecosystems: - continue - if ecosystem in self.config.excluded_ecosystems: - continue - log.debug(f'Scanning ecosystem: {ecosystem}') - ecosystem_patterns = patterns[ecosystem] - for file_name in ecosystem_patterns: - original_pattern = ecosystem_patterns[file_name]["pattern"] - - # Expand brace patterns - expanded_patterns = Core.expand_brace_pattern(original_pattern) - - for pattern in expanded_patterns: - case_insensitive_pattern = Core.to_case_insensitive_regex(pattern) - - log.debug(f"Searching for pattern: {case_insensitive_pattern}") - glob_start = time.time() - - # Use pathlib.Path.rglob() instead of glob.glob() to properly match dotfiles/dotdirs - base_path = Path(path) - glob_files = base_path.rglob(case_insensitive_pattern) - - for glob_file in glob_files: - glob_file_str = str(glob_file) - if not os.path.isfile(glob_file_str): - continue - if Core.is_excluded(glob_file_str, self.config.excluded_dirs): - continue - if exclude_regexes: - rel = os.path.relpath(glob_file_str, path) - if Core.path_matches_exclude_regexes(rel, exclude_regexes): - continue - files.add(glob_file_str.replace("\\", "/")) - - glob_end = time.time() - log.debug(f"Globbing took {glob_end - glob_start:.4f} seconds") + if manifest_patterns.is_empty: + elapsed = time.perf_counter() - start_time + log.info( + "Manifest discovery completed in " + f"{elapsed:.2f}s: root={os.path.abspath(path)}, " + "directories_visited=0, directories_pruned=0, " + "files_visited=0, manifests_found=0" + ) + log.info("Total files found: 0") + return [] + + directories_visited = 0 + directories_pruned = 0 + files_visited = 0 + excluded_dirs = set(self.config.excluded_dirs) + + def handle_walk_error(error: OSError) -> None: + log.debug(f"Unable to inspect path during manifest discovery: {error}") + + for current_root, directory_names, file_names in os.walk( + path, + topdown=True, + followlinks=False, + onerror=handle_walk_error, + ): + directories_visited += 1 + + kept_directories = [] + for directory_name in directory_names: + if directory_name == ".git" or Core._matches_excluded_directory( + directory_name, + excluded_dirs, + ): + directories_pruned += 1 + continue + # Only --exclude-paths needs a scan-root-relative path, so build one + # lazily rather than for every directory in the repository. + if exclude_regexes: + relative_directory = os.path.relpath( + os.path.join(current_root, directory_name), + path, + ) + if Core.path_matches_exclude_regexes(relative_directory, exclude_regexes): + directories_pruned += 1 + continue + kept_directories.append(directory_name) + directory_names[:] = kept_directories + + files_visited += len(file_names) + for file_name in file_names: + # Reject on the basename first: os.walk already hands us the name, so + # non-manifests cost one set lookup instead of a relative path plus a + # path match. Exclusions are then only evaluated for real candidates. + if not Core._basename_could_match(file_name.casefold(), manifest_patterns): + continue + file_path = os.path.join(current_root, file_name) + relative_path = os.path.relpath(file_path, path) + if not Core._matches_manifest_pattern(relative_path, manifest_patterns): + continue + if exclude_regexes and Core.path_matches_exclude_regexes( + relative_path, + exclude_regexes, + ): + continue + if os.path.isfile(file_path): + files.add(file_path.replace("\\", "/")) file_list = sorted(files) file_count = len(file_list) + elapsed = time.perf_counter() - start_time + log.info( + "Manifest discovery completed in " + f"{elapsed:.2f}s: root={os.path.abspath(path)}, " + f"directories_visited={directories_visited}, " + f"directories_pruned={directories_pruned}, " + f"files_visited={files_visited}, manifests_found={file_count}" + ) + # Retain the established count-only message for log consumers while they + # transition to the stage-level timing above. log.info(f"Total files found: {file_count}") # Check if the number of manifest files might exceed ulimit -n @@ -532,19 +707,36 @@ def get_supported_patterns(self) -> Dict: Returns: Dictionary of supported file patterns with 'general' key removed """ + cached_patterns = getattr(self, "_supported_patterns", None) + if cached_patterns is not None: + log.debug("Using cached supported manifest patterns") + return cached_patterns + + start_time = time.perf_counter() response = self.sdk.report.supported() + source = "api" if not response: log.error("Failed to get supported patterns from API") - # Import the old patterns as fallback - from .utils import socket_globs - return socket_globs - - # Remove the 'general' key if it exists - if 'general' in response: - response.pop('general') - - # The response is already in the format we need - return response + response = socket_globs + source = "local-fallback" + + # Do not mutate the SDK response, which may be shared by its own cache. + patterns = { + ecosystem: ecosystem_patterns + for ecosystem, ecosystem_patterns in response.items() + if ecosystem != "general" + } + # Only cache a successful lookup. The local fallback covers far fewer ecosystems + # than the API, so one transient failure must not pin the rest of the run to it — + # has_manifest_files() runs before find_files() and would poison the cache. + if source == "api": + self._supported_patterns = patterns + elapsed = time.perf_counter() - start_time + log.info( + "Supported manifest patterns loaded in " + f"{elapsed:.2f}s: source={source}, ecosystems={len(patterns)}" + ) + return patterns def has_manifest_files(self, files: list) -> bool: """ @@ -627,7 +819,7 @@ def empty_head_scan_file() -> List[str]: temp_path = os.path.join(temp_dir, '.socket.facts.json') # Create the empty file - with open(temp_path, 'w') as f: + with open(temp_path, 'w'): pass # Creates an empty file log.debug(f"Created temporary empty file for baseline scan: {temp_path}") diff --git a/socketsecurity/core/git_interface.py b/socketsecurity/core/git_interface.py index da614063..b3c53bdc 100644 --- a/socketsecurity/core/git_interface.py +++ b/socketsecurity/core/git_interface.py @@ -1,6 +1,7 @@ +import os import re +import time import urllib.parse -import os from git import Repo @@ -12,34 +13,33 @@ class Git: path: str def __init__(self, path: str): + initialization_start = time.perf_counter() self.path = path + self._fetched_ref_commits = {} self.ensure_safe_directory(path) self.repo = Repo(path) assert self.repo self.head = self.repo.head - - # Always fetch all remote refs to ensure branches exist for diffing - try: - self.repo.git.fetch('--all') - log.debug("Fetched all remote refs for diffing.") - except Exception as fetch_error: - log.debug(f"Failed to fetch all remote refs: {fetch_error}") # Use CI environment SHA if available, otherwise fall back to current HEAD commit github_sha = os.getenv('GITHUB_SHA') gitlab_sha = os.getenv('CI_COMMIT_SHA') bitbucket_sha = os.getenv('BITBUCKET_COMMIT') - ci_sha = github_sha or gitlab_sha or bitbucket_sha + buildkite_sha = os.getenv('BUILDKITE_COMMIT') + ci_commits = ( + ("BUILDKITE_COMMIT", buildkite_sha), + ("GITHUB_SHA", github_sha), + ("CI_COMMIT_SHA", gitlab_sha), + ("BITBUCKET_COMMIT", bitbucket_sha), + ) + env_source, ci_sha = next( + ((source, sha) for source, sha in ci_commits if sha), + (None, None), + ) if ci_sha: try: self.commit = self.repo.commit(ci_sha) - if github_sha: - env_source = "GITHUB_SHA" - elif gitlab_sha: - env_source = "CI_COMMIT_SHA" - else: - env_source = "BITBUCKET_COMMIT" log.debug(f"Using commit from {env_source}: {ci_sha}") except Exception as error: log.debug(f"Failed to get commit from CI environment: {error}") @@ -82,13 +82,19 @@ def __init__(self, path: str): # Bitbucket Pipelines variables bitbucket_branch = os.getenv('BITBUCKET_BRANCH') + + # Buildkite branch (the source branch for pull-request builds) + buildkite_branch = os.getenv('BUILDKITE_BRANCH') - # Select CI branch with priority: GitLab -> GitHub -> Bitbucket - ci_branch = gitlab_branch or github_branch or bitbucket_branch + # Prefer the native environment when Buildkite is driving the job. This + # also avoids requiring Buildkite users to emulate GitHub Actions vars. + ci_branch = buildkite_branch or gitlab_branch or github_branch or bitbucket_branch if ci_branch: self.branch = ci_branch - if gitlab_branch: + if buildkite_branch: + env_source = "Buildkite" + elif gitlab_branch: env_source = "GitLab CI" elif github_branch: env_source = "GitHub Actions" @@ -141,40 +147,39 @@ def __init__(self, path: str): self.commit_sha = self.commit.binsha self.commit_message = self.commit.message self.committer = self.commit.committer - # Detect changed files in PR/MR context for GitHub, GitLab, Bitbucket; fallback to git show + + # Detect changed files in PR/MR context, using local refs first and + # fetching only a required ref when the checkout does not contain it. + changed_files_start = time.perf_counter() self.show_files = [] detected = False - # GitHub Actions PR context + detection_source = "single-commit" + github_base_ref = os.getenv('GITHUB_BASE_REF') github_head_ref = os.getenv('GITHUB_HEAD_REF') github_event_name = os.getenv('GITHUB_EVENT_NAME') github_before_sha = os.getenv('GITHUB_EVENT_BEFORE') # previous commit for push github_sha = os.getenv('GITHUB_SHA') # current commit - if github_event_name == 'pull_request' and github_base_ref and github_head_ref: - try: - # Fetch both branches individually - self.repo.git.fetch('origin', github_base_ref) - self.repo.git.fetch('origin', github_head_ref) - # Try remote diff first - diff_range = f"origin/{github_base_ref}...origin/{github_head_ref}" - try: - diff_files = self.repo.git.diff('--name-only', diff_range) - self.show_files = diff_files.splitlines() - log.debug(f"Changed files detected via git diff (GitHub PR remote): {self.show_files}") - detected = True - except Exception as remote_error: - log.debug(f"Remote diff failed: {remote_error}") - # Try local branch diff - local_diff_range = f"{github_base_ref}...{github_head_ref}" - try: - diff_files = self.repo.git.diff('--name-only', local_diff_range) - self.show_files = diff_files.splitlines() - log.debug(f"Changed files detected via git diff (GitHub PR local): {self.show_files}") - detected = True - except Exception as local_error: - log.debug(f"Local diff failed: {local_error}") - except Exception as error: - log.debug(f"Failed to fetch branches or diff for GitHub PR: {error}") + + buildkite_pr = os.getenv('BUILDKITE_PULL_REQUEST') + buildkite_base_ref = os.getenv('BUILDKITE_PULL_REQUEST_BASE_BRANCH') + buildkite_head_ref = os.getenv('BUILDKITE_BRANCH') + if self._is_buildkite_pull_request(buildkite_pr) and buildkite_base_ref: + detected = self._detect_pull_request_changes( + provider="Buildkite", + base_ref=buildkite_base_ref, + head_ref=buildkite_head_ref, + ) + if detected: + detection_source = "buildkite-pr" + elif github_event_name == 'pull_request' and github_base_ref: + detected = self._detect_pull_request_changes( + provider="GitHub", + base_ref=github_base_ref, + head_ref=github_head_ref, + ) + if detected: + detection_source = "github-pr" # Commits to default branch (push events) elif github_event_name == 'push' and github_before_sha and github_sha: try: @@ -182,6 +187,7 @@ def __init__(self, path: str): self.show_files = diff_files.splitlines() log.debug(f"Changed files detected via git diff (GitHub push): {self.show_files}") detected = True + detection_source = "github-push" except Exception as error: log.debug(f"Failed to get changed files via git diff (GitHub push): {error}") elif github_event_name == 'push': @@ -189,6 +195,7 @@ def __init__(self, path: str): self.show_files = self.repo.git.show(self.commit, name_only=True, format="%n").splitlines() log.debug(f"Changed files detected via git show (GitHub push fallback): {self.show_files}") detected = True + detection_source = "github-push-fallback" except Exception as error: log.debug(f"Failed to get changed files via git show (GitHub push fallback): {error}") # GitLab CI Merge Request context @@ -196,15 +203,13 @@ def __init__(self, path: str): gitlab_target = os.getenv('CI_MERGE_REQUEST_TARGET_BRANCH_NAME') gitlab_source = os.getenv('CI_MERGE_REQUEST_SOURCE_BRANCH_NAME') if gitlab_target and gitlab_source: - try: - self.repo.git.fetch('origin', gitlab_target, gitlab_source) - diff_range = f"origin/{gitlab_target}...origin/{gitlab_source}" - diff_files = self.repo.git.diff('--name-only', diff_range) - self.show_files = diff_files.splitlines() - log.debug(f"Changed files detected via git diff (GitLab): {self.show_files}") - detected = True - except Exception as error: - log.debug(f"Failed to get changed files via git diff (GitLab): {error}") + detected = self._detect_pull_request_changes( + provider="GitLab", + base_ref=gitlab_target, + head_ref=gitlab_source, + ) + if detected: + detection_source = "gitlab-mr" # Bitbucket Pipelines PR context if not detected: bitbucket_pr_id = os.getenv('BITBUCKET_PR_ID') @@ -212,15 +217,13 @@ def __init__(self, path: str): bitbucket_dest = os.getenv('BITBUCKET_PR_DESTINATION_BRANCH') # BITBUCKET_BRANCH is the source branch in PR builds if bitbucket_pr_id and bitbucket_source and bitbucket_dest: - try: - self.repo.git.fetch('origin', bitbucket_dest, bitbucket_source) - diff_range = f"origin/{bitbucket_dest}...origin/{bitbucket_source}" - diff_files = self.repo.git.diff('--name-only', diff_range) - self.show_files = diff_files.splitlines() - log.debug(f"Changed files detected via git diff (Bitbucket): {self.show_files}") - detected = True - except Exception as error: - log.debug(f"Failed to get changed files via git diff (Bitbucket): {error}") + detected = self._detect_pull_request_changes( + provider="Bitbucket", + base_ref=bitbucket_dest, + head_ref=bitbucket_source, + ) + if detected: + detection_source = "bitbucket-pr" # Fallback to git show for single commit if not detected: # Check if this is a merge commit first @@ -233,20 +236,132 @@ def __init__(self, path: str): self.show_files = self.repo.git.show(self.commit, name_only=True, format="%n").splitlines() log.debug(f"Changed files detected via git show (merge commit fallback): {self.show_files}") detected = True + detection_source = "merge-commit-fallback" + if detected and detection_source == "single-commit": + detection_source = "merge-diff" else: # Regular single commit self.show_files = self.repo.git.show(self.commit, name_only=True, format="%n").splitlines() log.debug(f"Changed files detected via git show: {self.show_files}") detected = True + detection_source = "single-commit" self.changed_files = [] for item in self.show_files: if item != "": # Use relative path for glob matching self.changed_files.append(item) + + log.info( + "Changed-file detection completed in " + f"{time.perf_counter() - changed_files_start:.2f}s: " + f"source={detection_source}, files={len(self.changed_files)}" + ) # Determine if this commit is on the default branch # This considers both GitHub Actions detached HEAD and regular branch situations self.is_default_branch = self._is_commit_and_branch_default() + log.info( + "Git initialization completed in " + f"{time.perf_counter() - initialization_start:.2f}s" + ) + + @staticmethod + def _is_buildkite_pull_request(pull_request: str | None) -> bool: + return bool(pull_request and pull_request.casefold() != "false") + + def _resolve_ref(self, ref: str | None) -> str | None: + """Resolve a branch, tag, or SHA without accessing the network.""" + if not ref: + return None + if ref in self._fetched_ref_commits: + return self._fetched_ref_commits[ref] + + candidates = [ref] + if not ref.startswith("refs/"): + candidates = [f"origin/{ref}", ref] + for candidate in candidates: + try: + return self.repo.commit(candidate).hexsha + except Exception: + continue + return None + + def _fetch_ref(self, ref: str, reason: str) -> str | None: + """Fetch one required ref and return its commit without broadening scope.""" + if ref in self._fetched_ref_commits: + return self._fetched_ref_commits[ref] + + fetch_start = time.perf_counter() + try: + self.repo.git.fetch("origin", ref) + commit_sha = self.repo.commit("FETCH_HEAD").hexsha + self._fetched_ref_commits[ref] = commit_sha + log.info( + "Git fetch completed in " + f"{time.perf_counter() - fetch_start:.2f}s: " + f"remote=origin, ref={ref}, reason={reason}" + ) + return commit_sha + except Exception as error: + log.info( + "Git fetch failed in " + f"{time.perf_counter() - fetch_start:.2f}s: " + f"remote=origin, ref={ref}, reason={reason}" + ) + log.debug(f"Targeted fetch failed for {ref}: {error}") + return None + + def _detect_pull_request_changes( + self, + provider: str, + base_ref: str, + head_ref: str | None, + ) -> bool: + """Detect a full PR range locally, fetching only refs needed to complete it.""" + base_commit = self._resolve_ref(base_ref) + if base_commit is None: + base_commit = self._fetch_ref(base_ref, f"{provider} pull-request base ref missing") + if base_commit is None: + log.debug(f"Unable to resolve {provider} pull-request base ref: {base_ref}") + return False + + head_commit = self.commit.hexsha + diff_range = f"{base_commit}...{head_commit}" + try: + diff_files = self.repo.git.diff("--name-only", diff_range) + self.show_files = diff_files.splitlines() + log.debug( + f"Changed files detected via local git diff ({provider}): {self.show_files}" + ) + return True + except Exception as local_error: + log.debug(f"Local {provider} pull-request diff failed: {local_error}") + + # A shallow checkout can contain both tips but not their merge base. In + # that case refresh only the two relevant branch histories and retry. + base_commit = self._fetch_ref( + base_ref, + f"{provider} pull-request history incomplete", + ) or base_commit + if head_ref: + self._fetch_ref( + head_ref, + f"{provider} pull-request history incomplete", + ) + + try: + diff_files = self.repo.git.diff( + "--name-only", + f"{base_commit}...{head_commit}", + ) + self.show_files = diff_files.splitlines() + log.debug( + f"Changed files detected after targeted fetch ({provider}): {self.show_files}" + ) + return True + except Exception as retry_error: + log.debug(f"Targeted {provider} pull-request diff failed: {retry_error}") + return False def _is_commit_and_branch_default(self) -> bool: """ @@ -268,9 +383,29 @@ def _is_commit_and_branch_default(self) -> bool: gitlab_mr_branch = os.getenv('CI_MERGE_REQUEST_SOURCE_BRANCH_NAME') gitlab_default_branch = os.getenv('CI_DEFAULT_BRANCH', '') bitbucket_branch = os.getenv('BITBUCKET_BRANCH') + buildkite_branch = os.getenv('BUILDKITE_BRANCH') + buildkite_pr = os.getenv('BUILDKITE_PULL_REQUEST') + buildkite_default_branch = os.getenv('BUILDKITE_PIPELINE_DEFAULT_BRANCH') + # Handle Buildkite before GitHub because some Buildkite pipelines + # intentionally provide GitHub-compatible environment variables. + if buildkite_branch: + if self._is_buildkite_pull_request(buildkite_pr): + log.debug( + f"Processing Buildkite pull request from branch: {buildkite_branch}, " + "not default branch" + ) + return False + default_branch_name = buildkite_default_branch or self.get_default_branch_name() + is_default = buildkite_branch == default_branch_name + log.debug( + f"Buildkite branch: {buildkite_branch}, Default: {default_branch_name}, " + f"Is default: {is_default}" + ) + return is_default + # Handle GitHub Actions - if github_ref: + elif github_ref: log.debug(f"GitHub ref: {github_ref}") # Handle pull requests - they're not on the default branch @@ -483,7 +618,7 @@ def get_default_branch_name(self) -> str: if f'origin/{branch_name}' in [str(ref) for ref in self.repo.remotes.origin.refs]: log.debug(f"Using fallback default branch: {branch_name}") return branch_name - except: + except Exception: continue # Last fallback: assume 'main' @@ -505,12 +640,12 @@ def is_commit_on_default_branch(self) -> bool: # Try remote branch first default_branch_ref = self.repo.remotes.origin.refs[default_branch] default_branch_commit = default_branch_ref.commit - except: + except Exception: # Fallback to local branch try: default_branch_ref = self.repo.heads[default_branch] default_branch_commit = default_branch_ref.commit - except: + except Exception: log.debug(f"Could not find default branch '{default_branch}' locally or remotely") return False @@ -572,4 +707,4 @@ def ensure_safe_directory(path: str) -> None: else: log.debug(f"{abs_path} already present in git safe.directory config.") except Exception as safe_error: - log.debug(f"Failed to set safe.directory for git: {safe_error}") \ No newline at end of file + log.debug(f"Failed to set safe.directory for git: {safe_error}") diff --git a/socketsecurity/core/scm/github.py b/socketsecurity/core/scm/github.py index 7d5905d2..7504a46c 100644 --- a/socketsecurity/core/scm/github.py +++ b/socketsecurity/core/scm/github.py @@ -1,6 +1,7 @@ import json import os import sys +import urllib.parse from dataclasses import dataclass from git import Optional @@ -34,6 +35,31 @@ class GithubConfig: event_action: Optional[str] headers: dict + @staticmethod + def _repository_from_buildkite() -> tuple[str, str]: + """Return ``(owner, repository)`` from Buildkite's Git repository URL.""" + repository_url = ( + # Comments and statuses belong to the pipeline/base repository, + # not a contributor's fork from BUILDKITE_PULL_REQUEST_REPO. + os.getenv("BUILDKITE_REPO") + or os.getenv("BUILDKITE_PULL_REQUEST_REPO") + or "" + ).strip() + if not repository_url: + return "", "" + + if "://" in repository_url: + repository_path = urllib.parse.urlparse(repository_url).path + elif ":" in repository_url: + # SCP-style SSH URL: git@github.com:owner/repository.git + repository_path = repository_url.split(":", 1)[1] + else: + repository_path = repository_url + parts = repository_path.strip("/").removesuffix(".git").split("/") + if len(parts) < 2: + return "", "" + return parts[-2], parts[-1] + @classmethod def from_env(cls, pr_number: Optional[str] = None) -> 'GithubConfig': """Create config from environment variables with optional overrides""" @@ -42,12 +68,24 @@ def from_env(cls, pr_number: Optional[str] = None) -> 'GithubConfig': log.error("Unable to get Github API Token from GH_API_TOKEN") sys.exit(2) - # Use provided PR number if available, otherwise fall back to env var + is_buildkite = os.getenv("BUILDKITE") == "true" + buildkite_pr = os.getenv("BUILDKITE_PULL_REQUEST") + is_buildkite_pr = bool( + is_buildkite + and buildkite_pr + and buildkite_pr.casefold() != "false" + ) + + # Use explicit/GitHub-compatible values first, then native Buildkite PR context. pr_number = pr_number or os.getenv('PR_NUMBER') + if not pr_number and is_buildkite_pr: + pr_number = buildkite_pr # Add debug logging - sha = os.getenv('GITHUB_SHA', '') - log.debug(f"Loading SHA from GITHUB_SHA: {sha}") + sha = os.getenv('GITHUB_SHA') or ( + os.getenv("BUILDKITE_COMMIT", "") if is_buildkite else "" + ) + log.debug(f"Loading GitHub integration SHA: {sha}") event_action = os.getenv('EVENT_ACTION', None) if not event_action: event_path = os.getenv('GITHUB_EVENT_PATH') @@ -55,29 +93,66 @@ def from_env(cls, pr_number: Optional[str] = None) -> 'GithubConfig': with open(event_path, 'r') as f: event = json.load(f) event_action = event.get('action') + if not event_action and is_buildkite_pr: + # Buildkite provides the current PR state, not the originating + # GitHub webhook action. A running PR build is equivalent to the + # supported synchronize path for comment updates. + event_action = "synchronize" repository = os.getenv('GITHUB_REPOSITORY', '') owner = os.getenv('GITHUB_REPOSITORY_OWNER', '') if '/' in repository: owner = repository.split('/')[0] repository = repository.split('/')[1] + elif is_buildkite: + buildkite_owner, buildkite_repository = cls._repository_from_buildkite() + owner = owner or buildkite_owner + repository = repository or buildkite_repository default_branch_env = os.getenv('DEFAULT_BRANCH') # Consider the variable truthy if it exists and isn't explicitly 'false' - is_default = default_branch_env is not None and default_branch_env.lower() != 'false' + if default_branch_env is not None: + is_default = default_branch_env.lower() != 'false' + elif is_buildkite: + # Require a branch name: comparing two unset variables would otherwise report + # every build as the default branch and overwrite the repository's baseline. + buildkite_branch = os.getenv("BUILDKITE_BRANCH") + is_default = bool( + not is_buildkite_pr + and buildkite_branch + and buildkite_branch == os.getenv("BUILDKITE_PIPELINE_DEFAULT_BRANCH") + ) + else: + is_default = False + + event_name = os.getenv('GITHUB_EVENT_NAME', '') + if not event_name and is_buildkite: + event_name = "pull_request" if is_buildkite_pr else "push" return cls( - sha=os.getenv('GITHUB_SHA', ''), - api_url=os.getenv('GITHUB_API_URL', ''), - ref_type=os.getenv('GITHUB_REF_TYPE', ''), - event_name=os.getenv('GITHUB_EVENT_NAME', ''), - workspace=os.getenv('GITHUB_WORKSPACE', ''), + sha=sha, + api_url=os.getenv('GITHUB_API_URL') or ( + "https://api.github.com" if is_buildkite else "" + ), + ref_type=os.getenv('GITHUB_REF_TYPE') or ( + "branch" if is_buildkite else "" + ), + event_name=event_name, + workspace=os.getenv('GITHUB_WORKSPACE') or ( + os.getenv("BUILDKITE_BUILD_CHECKOUT_PATH", "") if is_buildkite else "" + ), repository=repository, - ref_name=os.getenv('GITHUB_REF_NAME', ''), + ref_name=os.getenv('GITHUB_REF_NAME') or ( + os.getenv("BUILDKITE_BRANCH", "") if is_buildkite else "" + ), default_branch=is_default, is_default_branch=is_default, pr_number=pr_number, pr_name=os.getenv('PR_NAME'), - commit_message=os.getenv('COMMIT_MESSAGE'), - actor=os.getenv('GITHUB_ACTOR', ''), + commit_message=os.getenv('COMMIT_MESSAGE') or ( + os.getenv("BUILDKITE_MESSAGE") if is_buildkite else None + ), + actor=os.getenv('GITHUB_ACTOR') or ( + os.getenv("BUILDKITE_BUILD_CREATOR", "") if is_buildkite else "" + ), env=os.getenv('GITHUB_ENV', ''), token=token, owner=owner, diff --git a/socketsecurity/core/streaming.py b/socketsecurity/core/streaming.py index 20b45eac..e6910b44 100644 --- a/socketsecurity/core/streaming.py +++ b/socketsecurity/core/streaming.py @@ -13,6 +13,7 @@ """ import logging +import time from typing import Optional from .cli_client import CliClient @@ -49,12 +50,17 @@ def set_report_run_id(self, report_run_id: Optional[str]) -> None: self._report_run_id = report_run_id def __enter__(self) -> "StreamingLogs": + registration_start = time.perf_counter() self._run_id = register_cli_run( self._client, client_version=self._client_version, upload_logs=self._upload_logs, ) cli_logger = self._loggers[0] + cli_logger.info( + "CLI run registration completed in " + f"{time.perf_counter() - registration_start:.2f}s" + ) if not self._run_id: cli_logger.debug("server log streaming not active for this run") return self diff --git a/socketsecurity/socketcli.py b/socketsecurity/socketcli.py index 0d8bcccb..24e8e966 100644 --- a/socketsecurity/socketcli.py +++ b/socketsecurity/socketcli.py @@ -207,6 +207,8 @@ def main_code(): if dirs_to_include: core.config.excluded_dirs = set(core.config.excluded_dirs) - dirs_to_include log.debug(f"Re-including normally-excluded directories in scan: {sorted(dirs_to_include)}") + if config.excluded_ecosystems: + core.config.excluded_ecosystems = list(config.excluded_ecosystems) # Check for required dependencies if reachability analysis is enabled if config.reach: @@ -292,6 +294,9 @@ def main_code(): facts_file_to_submit = None # Variable to track SBOM files to submit when using --reach-use-only-pregenerated-sboms sbom_files_to_submit = None + # Manifest results retained from the --sub-path routing pre-check. Reusing + # these avoids walking every selected sub-path again during scan creation. + discovered_scan_files = None # Git setup is_repo = False @@ -534,14 +539,18 @@ def main_code(): # Override file checking to look in the scan paths instead # Get manifest files from all scan paths try: - all_scan_files = [] + discovered_scan_files = [] for scan_path in scan_paths: scan_files = core.find_files(scan_path) - all_scan_files.extend(scan_files) - has_supported_files = len(all_scan_files) > 0 - log.debug(f"Found {len(all_scan_files)} manifest files across {len(scan_paths)} scan paths") + discovered_scan_files.extend(scan_files) + has_supported_files = len(discovered_scan_files) > 0 + log.debug( + f"Found {len(discovered_scan_files)} manifest files across " + f"{len(scan_paths)} scan paths" + ) except Exception as e: log.debug(f"Error finding files in scan paths: {e}") + discovered_scan_files = None has_supported_files = False # Case 3: If no supported files or files are empty, force API mode (no PR comments) @@ -564,8 +573,6 @@ def main_code(): org_slug = core.config.org_slug if config.repo_is_public: core.config.repo_visibility = "public" - if config.excluded_ecosystems and len(config.excluded_ecosystems) > 0: - core.config.excluded_ecosystems = config.excluded_ecosystems integration_type = config.integration_type integration_org_slug = config.integration_org_slug or org_slug try: @@ -613,6 +620,12 @@ def main_code(): diff.diff_url = "" diff.report_url = "" + scan_explicit_files = ( + sbom_files_to_submit + if sbom_files_to_submit is not None + else discovered_scan_files + ) + # Handle SCM-specific flows log.debug(f"Flow decision: scm={scm is not None}, force_diff_mode={force_diff_mode}, force_api_mode={force_api_mode}, enable_diff={config.enable_diff}") @@ -684,7 +697,7 @@ def _is_unprocessed(c): log.info("Push initiated flow") if scm.check_event_type() == "diff": log.info("Starting comment logic for PR/MR event") - diff = core.create_new_diff(scan_paths, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, explicit_files=sbom_files_to_submit) + diff = core.create_new_diff(scan_paths, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, explicit_files=scan_explicit_files) comments = scm.get_comments_for_pr() # FIXME: this overwrites diff.new_alerts, which was previously populated by Core.create_issue_alerts @@ -807,14 +820,14 @@ def _is_unprocessed(c): ) else: log.info("Starting non-PR/MR flow") - diff = core.create_new_diff(scan_paths, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, explicit_files=sbom_files_to_submit) + diff = core.create_new_diff(scan_paths, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, explicit_files=scan_explicit_files) output_handler.handle_output(diff) elif (config.enable_diff or force_diff_mode) and not force_api_mode: # New logic: --enable-diff or force_diff_mode (from --ignore-commit-files in git repos) forces diff mode log.info("Diff mode enabled without SCM integration") - diff = core.create_new_diff(scan_paths, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, explicit_files=sbom_files_to_submit) + diff = core.create_new_diff(scan_paths, params, no_change=should_skip_scan, save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, explicit_files=scan_explicit_files) output_handler.handle_output(diff) elif (config.enable_diff or force_diff_mode) and force_api_mode: @@ -834,7 +847,7 @@ def _is_unprocessed(c): save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, - explicit_files=sbom_files_to_submit + explicit_files=scan_explicit_files ) log.info(f"Full scan created with ID: {diff.id}") log.info(f"Full scan report URL: {diff.report_url}") @@ -842,7 +855,10 @@ def _is_unprocessed(c): else: if force_api_mode: - log.info("No Manifest files changed, creating Socket Report") + log.info( + "No supported manifest detected in the changed-file set; " + "creating a full Socket report" + ) serializable_params = { key: value if isinstance(value, (int, float, str, list, dict, bool, type(None))) else str(value) for key, value in params.__dict__.items() @@ -855,7 +871,7 @@ def _is_unprocessed(c): save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, - explicit_files=sbom_files_to_submit + explicit_files=scan_explicit_files ) log.info(f"Full scan created with ID: {diff.id}") log.info(f"Full scan report URL: {diff.report_url}") @@ -868,7 +884,7 @@ def _is_unprocessed(c): save_files_list_path=config.save_submitted_files_list, save_manifest_tar_path=config.save_manifest_tar, base_paths=base_paths, - explicit_files=sbom_files_to_submit + explicit_files=scan_explicit_files ) output_handler.handle_output(diff) diff --git a/tests/unit/test_git_interface.py b/tests/unit/test_git_interface.py new file mode 100644 index 00000000..a22cf634 --- /dev/null +++ b/tests/unit/test_git_interface.py @@ -0,0 +1,251 @@ +import logging +import subprocess +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from socketsecurity.core.git_interface import Git + +CI_ENVIRONMENT_VARIABLES = ( + "BUILDKITE", + "BUILDKITE_BRANCH", + "BUILDKITE_COMMIT", + "BUILDKITE_PIPELINE_DEFAULT_BRANCH", + "BUILDKITE_PULL_REQUEST", + "BUILDKITE_PULL_REQUEST_BASE_BRANCH", + "GITHUB_BASE_REF", + "GITHUB_EVENT_BEFORE", + "GITHUB_EVENT_NAME", + "GITHUB_HEAD_REF", + "GITHUB_REF", + "GITHUB_SHA", + "CI_COMMIT_BRANCH", + "CI_COMMIT_SHA", + "CI_DEFAULT_BRANCH", + "CI_MERGE_REQUEST_SOURCE_BRANCH_NAME", + "CI_MERGE_REQUEST_TARGET_BRANCH_NAME", + "BITBUCKET_BRANCH", + "BITBUCKET_COMMIT", + "BITBUCKET_PR_DESTINATION_BRANCH", + "BITBUCKET_PR_ID", +) + + +@pytest.fixture(autouse=True) +def clear_ci_environment(monkeypatch): + for variable in CI_ENVIRONMENT_VARIABLES: + monkeypatch.delenv(variable, raising=False) + + +def _git(path, *args): + return subprocess.run( + ["git", *args], + cwd=path, + check=True, + capture_output=True, + text=True, + ).stdout.strip() + + +@pytest.fixture +def pull_request_repo(tmp_path): + path = tmp_path / "repo" + path.mkdir() + _git(path, "init", "-b", "main") + _git(path, "config", "user.name", "Socket Test") + _git(path, "config", "user.email", "socket@example.com") + (path / "README.md").write_text("base\n", encoding="utf-8") + _git(path, "add", "README.md") + _git(path, "commit", "-m", "base") + _git(path, "checkout", "-b", "feature") + (path / "package.json").write_text("{}\n", encoding="utf-8") + _git(path, "add", "package.json") + _git(path, "commit", "-m", "add manifest") + return path + + +@pytest.mark.parametrize( + ("environment", "expected_branch", "expected_source"), + [ + ( + { + "BUILDKITE": "true", + "BUILDKITE_BRANCH": "feature", + "BUILDKITE_PULL_REQUEST": "123", + "BUILDKITE_PULL_REQUEST_BASE_BRANCH": "main", + }, + "feature", + "buildkite-pr", + ), + ( + { + "GITHUB_EVENT_NAME": "pull_request", + "GITHUB_BASE_REF": "main", + "GITHUB_HEAD_REF": "feature", + "GITHUB_REF": "refs/pull/123/merge", + }, + "feature", + "github-pr", + ), + ( + { + "CI_MERGE_REQUEST_SOURCE_BRANCH_NAME": "feature", + "CI_MERGE_REQUEST_TARGET_BRANCH_NAME": "main", + }, + "feature", + "gitlab-mr", + ), + ( + { + "BITBUCKET_BRANCH": "feature", + "BITBUCKET_PR_DESTINATION_BRANCH": "main", + "BITBUCKET_PR_ID": "123", + }, + "feature", + "bitbucket-pr", + ), + ], +) +def test_pull_request_context_uses_local_refs_without_fetch( + pull_request_repo, monkeypatch, mocker, caplog, + environment, expected_branch, expected_source, +): + head_sha = _git(pull_request_repo, "rev-parse", "HEAD") + sha_variable = { + "buildkite-pr": "BUILDKITE_COMMIT", + "github-pr": "GITHUB_SHA", + "gitlab-mr": "CI_COMMIT_SHA", + "bitbucket-pr": "BITBUCKET_COMMIT", + }[expected_source] + environment[sha_variable] = head_sha + for name, value in environment.items(): + monkeypatch.setenv(name, value) + + fetch = mocker.patch.object( + Git, + "_fetch_ref", + side_effect=AssertionError("unexpected fetch"), + ) + mocker.patch.object(Git, "ensure_safe_directory") + + with caplog.at_level(logging.INFO, logger="socketdev"): + repository = Git(str(pull_request_repo)) + + assert repository.branch == expected_branch + assert repository.changed_files == ["package.json"] + assert repository.is_default_branch is False + fetch.assert_not_called() + assert any( + f"source={expected_source}" in record.message + for record in caplog.records + ) + assert any( + "Git initialization completed" in record.message + for record in caplog.records + ) + + +def test_buildkite_native_context_wins_over_github_compatibility_shims( + pull_request_repo, monkeypatch, mocker +): + head_sha = _git(pull_request_repo, "rev-parse", "HEAD") + monkeypatch.setenv("BUILDKITE", "true") + monkeypatch.setenv("BUILDKITE_BRANCH", "feature") + monkeypatch.setenv("BUILDKITE_COMMIT", head_sha) + monkeypatch.setenv("BUILDKITE_PULL_REQUEST", "123") + monkeypatch.setenv("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "main") + monkeypatch.setenv("GITHUB_EVENT_NAME", "pull_request") + monkeypatch.setenv("GITHUB_BASE_REF", "wrong-base") + monkeypatch.setenv("GITHUB_HEAD_REF", "wrong-head") + mocker.patch.object( + Git, + "_fetch_ref", + side_effect=AssertionError("unexpected fetch"), + ) + mocker.patch.object(Git, "ensure_safe_directory") + + repository = Git(str(pull_request_repo)) + + assert repository.changed_files == ["package.json"] + + +def test_regular_initialization_never_fetches_all(pull_request_repo, mocker): + fetch = mocker.patch.object( + Git, + "_fetch_ref", + side_effect=AssertionError("unexpected fetch"), + ) + mocker.patch.object(Git, "ensure_safe_directory") + + repository = Git(str(pull_request_repo)) + + assert repository.commit_str == _git(pull_request_repo, "rev-parse", "HEAD") + assert repository.changed_files == ["package.json"] + fetch.assert_not_called() + + +def test_detached_head_uses_buildkite_branch_and_commit(pull_request_repo, monkeypatch, mocker): + head_sha = _git(pull_request_repo, "rev-parse", "HEAD") + _git(pull_request_repo, "checkout", "--detach", head_sha) + monkeypatch.setenv("BUILDKITE", "true") + monkeypatch.setenv("BUILDKITE_BRANCH", "feature") + monkeypatch.setenv("BUILDKITE_COMMIT", head_sha) + monkeypatch.setenv("BUILDKITE_PULL_REQUEST", "123") + monkeypatch.setenv("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "main") + mocker.patch.object( + Git, + "_fetch_ref", + side_effect=AssertionError("unexpected fetch"), + ) + mocker.patch.object(Git, "ensure_safe_directory") + + repository = Git(str(pull_request_repo)) + + assert repository.commit_str == head_sha + assert repository.branch == "feature" + assert repository.changed_files == ["package.json"] + + +def test_missing_base_ref_fetches_only_that_ref( + pull_request_repo, monkeypatch, mocker, caplog +): + head_sha = _git(pull_request_repo, "rev-parse", "HEAD") + monkeypatch.setenv("BUILDKITE_BRANCH", "feature") + monkeypatch.setenv("BUILDKITE_COMMIT", head_sha) + monkeypatch.setenv("BUILDKITE_PULL_REQUEST", "123") + monkeypatch.setenv("BUILDKITE_PULL_REQUEST_BASE_BRANCH", "remote-main") + mocker.patch.object(Git, "ensure_safe_directory") + base_sha = _git(pull_request_repo, "rev-parse", "main") + fetch = mocker.patch.object(Git, "_fetch_ref", return_value=base_sha) + + with caplog.at_level(logging.INFO, logger="socketdev"): + repository = Git(str(pull_request_repo)) + + fetch.assert_called_once_with( + "remote-main", + "Buildkite pull-request base ref missing", + ) + assert repository.changed_files == ["package.json"] + + +def test_targeted_fetch_never_uses_all(): + repository = Git.__new__(Git) + repository.repo = MagicMock() + repository._fetched_ref_commits = {} + main_sha = "a" * 40 + repository.repo.commit.return_value = SimpleNamespace(hexsha=main_sha) + + result = repository._fetch_ref("main", "test") + + repository.repo.git.fetch.assert_called_once_with("origin", "main") + assert "--all" not in repository.repo.git.fetch.call_args.args + assert result == main_sha + + +@pytest.mark.parametrize( + ("value", "expected"), + [(None, False), ("", False), ("false", False), ("False", False), ("0", True), ("123", True)], +) +def test_buildkite_pull_request_detection(value, expected): + assert Git._is_buildkite_pull_request(value) is expected diff --git a/tests/unit/test_github_buildkite_config.py b/tests/unit/test_github_buildkite_config.py new file mode 100644 index 00000000..23f41507 --- /dev/null +++ b/tests/unit/test_github_buildkite_config.py @@ -0,0 +1,173 @@ +import pytest + +from socketsecurity.core.scm.github import Github, GithubConfig + +CONTEXT_VARIABLES = ( + "BUILDKITE", + "BUILDKITE_BRANCH", + "BUILDKITE_BUILD_CHECKOUT_PATH", + "BUILDKITE_BUILD_CREATOR", + "BUILDKITE_COMMIT", + "BUILDKITE_MESSAGE", + "BUILDKITE_PIPELINE_DEFAULT_BRANCH", + "BUILDKITE_PULL_REQUEST", + "BUILDKITE_PULL_REQUEST_REPO", + "BUILDKITE_REPO", + "DEFAULT_BRANCH", + "EVENT_ACTION", + "GH_API_TOKEN", + "GITHUB_ACTOR", + "GITHUB_API_URL", + "GITHUB_EVENT_NAME", + "GITHUB_EVENT_PATH", + "GITHUB_REF_NAME", + "GITHUB_REF_TYPE", + "GITHUB_REPOSITORY", + "GITHUB_REPOSITORY_OWNER", + "GITHUB_SHA", + "GITHUB_WORKSPACE", + "PR_NUMBER", +) + + +@pytest.fixture(autouse=True) +def clear_context(monkeypatch): + for variable in CONTEXT_VARIABLES: + monkeypatch.delenv(variable, raising=False) + monkeypatch.setenv("GH_API_TOKEN", "test-token") + + +def test_github_config_uses_native_buildkite_pull_request_context(monkeypatch): + values = { + "BUILDKITE": "true", + "BUILDKITE_BRANCH": "feature/socket", + "BUILDKITE_BUILD_CHECKOUT_PATH": "/workspace/repo", + "BUILDKITE_BUILD_CREATOR": "octocat", + "BUILDKITE_COMMIT": "a" * 40, + "BUILDKITE_MESSAGE": "Update dependencies", + "BUILDKITE_PIPELINE_DEFAULT_BRANCH": "main", + "BUILDKITE_PULL_REQUEST": "123", + "BUILDKITE_PULL_REQUEST_REPO": "git@github.com:acme/widgets.git", + "BUILDKITE_REPO": "git@github.com:acme/widgets.git", + } + for name, value in values.items(): + monkeypatch.setenv(name, value) + + config = GithubConfig.from_env() + + assert config.sha == "a" * 40 + assert config.api_url == "https://api.github.com" + assert config.ref_type == "branch" + assert config.event_name == "pull_request" + assert config.event_action == "synchronize" + assert config.workspace == "/workspace/repo" + assert config.owner == "acme" + assert config.repository == "widgets" + assert config.ref_name == "feature/socket" + assert config.pr_number == "123" + assert config.commit_message == "Update dependencies" + assert config.actor == "octocat" + assert config.is_default_branch is False + assert Github(client=object(), config=config).check_event_type() == "diff" + + +def test_buildkite_non_pr_build_uses_push_and_default_branch(monkeypatch): + values = { + "BUILDKITE": "true", + "BUILDKITE_BRANCH": "main", + "BUILDKITE_COMMIT": "b" * 40, + "BUILDKITE_PIPELINE_DEFAULT_BRANCH": "main", + "BUILDKITE_PULL_REQUEST": "false", + "BUILDKITE_REPO": "https://github.com/acme/widgets.git", + } + for name, value in values.items(): + monkeypatch.setenv(name, value) + + config = GithubConfig.from_env() + + assert config.event_name == "push" + assert config.pr_number is None + assert config.owner == "acme" + assert config.repository == "widgets" + assert config.is_default_branch is True + assert Github(client=object(), config=config).check_event_type() == "main" + + +@pytest.mark.parametrize( + "branch_variables", + [ + {}, + {"BUILDKITE_BRANCH": "feature/socket"}, + {"BUILDKITE_PIPELINE_DEFAULT_BRANCH": "main"}, + ], +) +def test_buildkite_default_branch_requires_a_matching_branch_name( + monkeypatch, branch_variables +): + """Absent branch context must not be read as 'this build is the default branch'.""" + monkeypatch.setenv("BUILDKITE", "true") + for name, value in branch_variables.items(): + monkeypatch.setenv(name, value) + + config = GithubConfig.from_env() + + assert config.is_default_branch is False + assert config.default_branch is False + + +def test_explicit_github_values_take_priority_in_buildkite(monkeypatch): + values = { + "BUILDKITE": "true", + "BUILDKITE_BRANCH": "buildkite-branch", + "BUILDKITE_COMMIT": "b" * 40, + "BUILDKITE_PULL_REQUEST": "123", + "BUILDKITE_REPO": "git@github.com:buildkite/repository.git", + "EVENT_ACTION": "opened", + "GITHUB_API_URL": "https://github.example/api/v3", + "GITHUB_EVENT_NAME": "pull_request", + "GITHUB_REF_NAME": "github-branch", + "GITHUB_REF_TYPE": "branch", + "GITHUB_REPOSITORY": "github/repository", + "GITHUB_SHA": "c" * 40, + "GITHUB_WORKSPACE": "/github/workspace", + "PR_NUMBER": "456", + } + for name, value in values.items(): + monkeypatch.setenv(name, value) + + config = GithubConfig.from_env() + + assert config.sha == "c" * 40 + assert config.api_url == "https://github.example/api/v3" + assert config.workspace == "/github/workspace" + assert config.owner == "github" + assert config.repository == "repository" + assert config.ref_name == "github-branch" + assert config.pr_number == "456" + assert config.event_action == "opened" + + +@pytest.mark.parametrize( + ("repository_url", "expected"), + [ + ("git@github.com:acme/widgets.git", ("acme", "widgets")), + ("https://github.com/acme/widgets.git", ("acme", "widgets")), + ("ssh://git@github.com/acme/widgets.git", ("acme", "widgets")), + ("", ("", "")), + ("not-a-repository", ("", "")), + ], +) +def test_buildkite_repository_url_parsing(monkeypatch, repository_url, expected): + monkeypatch.setenv("BUILDKITE_REPO", repository_url) + + assert GithubConfig._repository_from_buildkite() == expected + + +def test_buildkite_pipeline_repository_wins_over_pull_request_fork(monkeypatch): + monkeypatch.setenv("BUILDKITE_REPO", "git@github.com:acme/widgets.git") + monkeypatch.setenv( + "BUILDKITE_PULL_REQUEST_REPO", + "git@github.com:contributor/widgets.git", + ) + + assert GithubConfig._repository_from_buildkite() == ("acme", "widgets") diff --git a/tests/unit/test_manifest_discovery.py b/tests/unit/test_manifest_discovery.py new file mode 100644 index 00000000..a74f2474 --- /dev/null +++ b/tests/unit/test_manifest_discovery.py @@ -0,0 +1,305 @@ +import logging +import os +from pathlib import Path +from types import SimpleNamespace +from unittest.mock import MagicMock + +import pytest + +from socketsecurity.core import Core +from socketsecurity.core.socket_config import SocketConfig, default_exclude_dirs +from socketsecurity.core.utils import socket_globs + + +def _make_core(*, patterns=socket_globs, excluded_dirs=None, exclude_paths=None): + core = Core.__new__(Core) + core.config = SocketConfig( + api_key="test-key", + excluded_dirs=set(default_exclude_dirs if excluded_dirs is None else excluded_dirs), + ) + core.cli_config = SimpleNamespace(exclude_paths=exclude_paths) + core.sdk = MagicMock() + core._supported_patterns = patterns + return core + + +def _write_files(root: Path, relative_paths): + for relative_path in relative_paths: + target = root / relative_path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text("test\n", encoding="utf-8") + + +def _relative_results(root: Path, results): + return {Path(result).relative_to(root).as_posix() for result in results} + + +ALL_PATTERN_EXAMPLES = { + "app.spdx.json", + "bom.json", + "nested/app-cdx.json", + "nested/app-cyclonedx.xml", + "package.json", + "nested/package-lock.json", + "npm-shrinkwrap.json", + "yarn.lock", + "pnpm-lock.yaml", + "pnpm-lock.yml", + "pnpm-workspace.yaml", + "pnpm-workspace.yml", + "bun.lock", + "bun.lockb", + "vlt-lock.json", + "PIPFILE", + "pyproject.toml", + "poetry.lock", + "requirements.txt", + "dev-requirements.txt", + "requirements-dev.txt", + "requirements_test.txt", + "requirements.frozen", + "requirements/base.txt", + "nested/requirements/constraints.txt", + "setup.py", + "go.mod", + "go.sum", + "pom.xml", + "src/Project.CSPROJ", + "Directory.Build.Props", + "build.targets", + "project.nuspec", + "nuget.CONFIG", + "packages.config", + "packages.lock.json", +} + + +def test_all_builtin_manifest_patterns_match_in_one_walk(tmp_path, mocker): + _write_files( + tmp_path, + ALL_PATTERN_EXAMPLES + | { + "README.md", + "requirements/deep/not-a-direct-child.txt", + "src/package.json.backup", + }, + ) + original_walk = os.walk + walk = mocker.patch("socketsecurity.core.os.walk", wraps=original_walk) + + found = _relative_results(tmp_path, _make_core().find_files(str(tmp_path))) + + assert found == ALL_PATTERN_EXAMPLES + walk.assert_called_once() + + +def test_single_walk_matches_legacy_rglob_results_for_builtin_patterns(tmp_path): + _write_files( + tmp_path, + ALL_PATTERN_EXAMPLES + | { + ".hidden/package.json", + "nested/Requirements.TXT", + "src/not-a-manifest.json", + }, + ) + core = _make_core(excluded_dirs=set()) + + legacy_results = set() + for ecosystem_patterns in socket_globs.values(): + for details in ecosystem_patterns.values(): + for expanded in Core.expand_brace_pattern(details["pattern"]): + case_insensitive = Core.to_case_insensitive_regex(expanded) + for result in tmp_path.rglob(case_insensitive): + if result.is_file(): + legacy_results.add(result.as_posix()) + + assert set(core.find_files(str(tmp_path))) == legacy_results + + +def test_prunes_git_default_globs_and_exclude_paths_before_descent( + tmp_path, mocker, caplog +): + _write_files( + tmp_path, + { + "package.json", + ".git/objects/package.json", + "node_modules/pkg/package.json", + "generated.egg-info/package.json", + "legacy/nested/package.json", + ".hidden/package.json", + }, + ) + scanned_directories = [] + original_scandir = os.scandir + + def tracking_scandir(path): + scanned_directories.append(Path(path).relative_to(tmp_path).as_posix()) + return original_scandir(path) + + mocker.patch("socketsecurity.core.os.scandir", side_effect=tracking_scandir) + core = _make_core(exclude_paths=["legacy"]) + + with caplog.at_level(logging.INFO, logger="socketdev"): + found = _relative_results(tmp_path, core.find_files(str(tmp_path))) + + assert found == {"package.json", ".hidden/package.json"} + assert ".git" not in scanned_directories + assert "node_modules" not in scanned_directories + assert "generated.egg-info" not in scanned_directories + assert "legacy" not in scanned_directories + assert any( + "directories_pruned=4" in record.message + and "manifests_found=2" in record.message + for record in caplog.records + ) + + +def test_include_dirs_and_excluded_ecosystems_are_preserved(tmp_path): + _write_files( + tmp_path, + { + "build/package.json", + "build/requirements.txt", + "dist/package.json", + }, + ) + core = _make_core(excluded_dirs=set(default_exclude_dirs) - {"build"}) + core.config.excluded_ecosystems = ["npm"] + + found = _relative_results(tmp_path, core.find_files(str(tmp_path))) + + assert found == {"build/requirements.txt"} + + +def test_excluding_every_ecosystem_skips_the_filesystem_walk(tmp_path, mocker): + core = _make_core() + core.config.excluded_ecosystems = list(socket_globs) + walk = mocker.patch( + "socketsecurity.core.os.walk", + side_effect=AssertionError("unexpected walk"), + ) + + assert core.find_files(str(tmp_path)) == [] + walk.assert_not_called() + + +def test_symlinked_file_is_included_but_symlinked_directory_is_not_followed(tmp_path): + if not hasattr(os, "symlink"): + pytest.skip("symlinks are not supported") + + source_file = tmp_path / "source.txt" + source_file.write_text("{}", encoding="utf-8") + source_directory = tmp_path / "external" + _write_files(source_directory, {"package.json"}) + try: + (tmp_path / "package.json").symlink_to(source_file) + (tmp_path / "linked-directory").symlink_to(source_directory, target_is_directory=True) + except OSError as error: + pytest.skip(f"symlinks are unavailable: {error}") + + found = _relative_results(tmp_path, _make_core().find_files(str(tmp_path))) + + assert "package.json" in found + assert "linked-directory/package.json" not in found + assert "external/package.json" in found + + +def test_supported_patterns_are_cached_without_mutating_sdk_response(): + response = { + "general": {"ignored": {"pattern": "ignored"}}, + "npm": {"package.json": {"pattern": "package.json"}}, + } + core = _make_core(patterns=None) + core.sdk.report.supported.return_value = response + + first = core.get_supported_patterns() + second = core.get_supported_patterns() + + assert first is second + assert first == {"npm": {"package.json": {"pattern": "package.json"}}} + assert "general" in response + core.sdk.report.supported.assert_called_once_with() + + +def test_failed_pattern_lookup_is_not_cached(): + """A transient API failure must not pin the run to the smaller local fallback.""" + api_response = {"npm": {"package.json": {"pattern": "package.json"}}} + core = _make_core(patterns=None) + core.sdk.report.supported.side_effect = [None, api_response] + + fallback = core.get_supported_patterns() + assert set(fallback) == set(socket_globs) + + recovered = core.get_supported_patterns() + assert set(recovered) == {"npm"} + # The successful lookup is still cached, so the API is not re-queried again. + assert core.get_supported_patterns() is recovered + assert core.sdk.report.supported.call_count == 2 + + +def test_basename_prefilter_admits_every_supported_manifest(): + """The cheap prefilter must never reject a path the authoritative matcher accepts.""" + patterns = Core._prepare_manifest_patterns(socket_globs, None, []) + + for relative_path in ALL_PATTERN_EXAMPLES: + basename = relative_path.rsplit("/", 1)[-1].casefold() + assert Core._matches_manifest_pattern(relative_path, patterns), relative_path + assert Core._basename_could_match(basename, patterns), relative_path + + +def test_results_are_sorted_and_deduplicated_across_overlapping_patterns(tmp_path): + _write_files(tmp_path, {"z/package.json", "a/package.json"}) + overlapping_patterns = { + "npm": { + "literal": {"pattern": "package.json"}, + "wildcard": {"pattern": "package*.json"}, + } + } + + found = _make_core(patterns=overlapping_patterns).find_files(str(tmp_path)) + + assert found == sorted(found) + assert len(found) == 2 + + +def test_explicit_discovery_results_prevent_a_second_walk(tmp_path): + manifest = tmp_path / "package.json" + manifest.write_text("{}", encoding="utf-8") + core = _make_core() + core.config.org_slug = "example" + core.cli_config = None + core.find_files = MagicMock(side_effect=AssertionError("unexpected second walk")) + core.create_full_scan = MagicMock(return_value=SimpleNamespace(id="scan-123")) + params = MagicMock() + + diff = core.create_full_scan_with_report_url( + [str(tmp_path)], + params, + explicit_files=[manifest.as_posix()], + ) + + core.find_files.assert_not_called() + core.create_full_scan.assert_called_once_with( + [manifest.as_posix()], + params, + base_paths=None, + ) + assert diff.id == "scan-123" + + +def test_core_initialization_logs_organization_timing(caplog): + sdk = MagicMock() + sdk.org.get.return_value = { + "organizations": {"org-id": {"slug": "example"}}, + } + + with caplog.at_level(logging.INFO, logger="socketdev"): + core = Core(SocketConfig(api_key="test-key"), sdk) + + assert core.config.org_slug == "example" + assert any( + "Organization initialization completed" in record.message + for record in caplog.records + ) diff --git a/tests/unit/test_streaming.py b/tests/unit/test_streaming.py index 999b44a8..3b9cbd9b 100644 --- a/tests/unit/test_streaming.py +++ b/tests/unit/test_streaming.py @@ -18,14 +18,19 @@ def _make(**overrides): return StreamingLogs(**kwargs) -def test_setup_streaming_is_noop_when_register_fails(): +def test_setup_streaming_is_noop_when_register_fails(caplog): finalize_calls = [] - with patch("socketsecurity.core.streaming.register_cli_run", return_value=None), \ - patch("socketsecurity.core.streaming.finalize_cli_run", side_effect=lambda *a, **k: finalize_calls.append(k)): - with _make(cli_name="t-fail-cli", sdk_name="t-fail-sdk") as streaming: - assert isinstance(streaming, StreamingLogs) + with caplog.at_level(logging.INFO, logger="t-fail-cli"): + with patch("socketsecurity.core.streaming.register_cli_run", return_value=None), \ + patch("socketsecurity.core.streaming.finalize_cli_run", side_effect=lambda *a, **k: finalize_calls.append(k)): + with _make(cli_name="t-fail-cli", sdk_name="t-fail-sdk") as streaming: + assert isinstance(streaming, StreamingLogs) # No run was registered → finalize must not be called. assert finalize_calls == [] + assert any( + "CLI run registration completed" in record.message + for record in caplog.records + ) def test_clean_exit_reports_success(): diff --git a/uv.lock b/uv.lock index 8fd80913..156b9936 100644 --- a/uv.lock +++ b/uv.lock @@ -1282,7 +1282,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.6.4" +version = "2.6.5" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" },