diff --git a/CHANGELOG.md b/CHANGELOG.md index 1007a3b9..2f212f05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,41 @@ # Changelog +## 2.8.0 + +### Added: patched versions in human-readable security output + +- The native console alert table now includes a `Patched Version` column, + populated from `props.firstPatchedVersionIdentifier` when the API provides it. +- GitHub pull request and GitLab merge request security comments now show the + patched version in each applicable alert's details. + +### Fixed: CLI scans retain pull request context in the Socket Dashboard + +- Pull request numbers are detected from standard GitHub Actions, GitLab CI, + and Azure Pipelines environments when `--pr-number` is not supplied. An + explicitly supplied value, including `0`, remains authoritative. +- The Buildkite workflow and CI/CD guide now forward `BUILDKITE_PULL_REQUEST` + explicitly and document provider selection for Dashboard PR association. With + `--integration github` or `--integration gitlab`, the repository slug and host + for the link are read from `BUILDKITE_REPO`, covering self-hosted installations. +- `--scm github` and `--scm gitlab` now imply the matching scan integration + unless `--integration` is explicitly supplied. +- Diff scans include the detected pull request or merge request URL as their + external link, allowing Dashboard reports to retain their CI change context. + Re-running a comparison over an already-compared scan pair now applies the + link to the existing diff scan instead of leaving that report unassociated. +- GitHub and GitLab branch pipelines now create full scans by default. Explicit + diff flags continue to opt non-PR runs into comparison mode. + +### Fixed: pull request and merge request comment accuracy + +- Per-alert ignore instructions now use ecosystem-qualified package names and + accept scoped packages while remaining compatible with older bare-name replies. +- Dependency overviews preserve added, updated, removed, and replaced package + classifications instead of presenting updates as new dependencies. +- Shared security comment copy no longer describes GitLab merge request output + as Socket for GitHub. + ## 2.7.1 ### Changed: bump pinned @coana-tech/cli to 15.10.36 diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 061d18ea..045c10a7 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -2,6 +2,10 @@ Use this guide for pipeline-focused CLI usage across platforms. +The shell commands in the recommended patterns are CI-provider neutral. Buildkite +pipeline equivalents and provider-specific considerations are called out alongside +the relevant guidance below. + ## Recommended patterns ### Dashboard-style reachable SARIF @@ -27,6 +31,27 @@ socketcli \ --strict-blocking ``` +### Buildkite: retain SARIF as a build artifact + +Either recommended pattern can run directly in a Buildkite command step. When the +scan writes SARIF, add +[`artifact_paths`](https://buildkite.com/docs/pipelines/configure/artifacts#upload-artifacts-with-a-command-step) +so developers can download the report from the build after the command finishes: + +```yaml +steps: + - label: ":socket: Socket reachable diff" + command: | + socketcli \ + --reach \ + --sarif-file results.sarif \ + --sarif-scope diff \ + --sarif-reachability reachable \ + --strict-blocking + artifact_paths: + - "results.sarif" +``` + ## Config file usage in CI Use `--config .socketcli.toml` or `--config .socketcli.json` to keep pipeline commands small. @@ -60,6 +85,9 @@ Equivalent JSON: } ``` +The Buildkite examples below use the same checked-in `.socketcli.toml` file; no +Buildkite-specific config-file format is required. + ## Platform examples ### GitHub Actions @@ -73,14 +101,33 @@ Equivalent JSON: ### Buildkite +This example assumes a GitHub-hosted repository. Change +`SOCKET_SCM_INTEGRATION` to `gitlab` for a GitLab-hosted repository, or `api` +when provider association is not wanted. The doubled dollar signs defer +Buildkite variable expansion until the command runs on an agent. + ```yaml +env: + SOCKET_SCM_INTEGRATION: "github" + steps: - label: "Socket scan" - command: "socketcli --config .socketcli.toml --target-path ." - env: - SOCKET_SECURITY_API_TOKEN: "${SOCKET_SECURITY_API_TOKEN}" + command: | + socketcli \ + --config .socketcli.toml \ + --target-path . \ + --integration "$${SOCKET_SCM_INTEGRATION:-api}" \ + --pr-number "$${BUILDKITE_PULL_REQUEST:-0}" + secrets: + - SOCKET_SECURITY_API_TOKEN ``` +The `secrets` block expects a +[Buildkite secret](https://buildkite.com/docs/pipelines/security/secrets/buildkite-secrets) +named `SOCKET_SECURITY_API_TOKEN`. If your organization uses an external secrets +plugin or an agent hook instead, remove that block and inject the same environment +variable through your existing mechanism. Do not store the token in pipeline YAML. + 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 @@ -88,11 +135,12 @@ 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`. +When `--scm github` is used from Buildkite, the CLI also posts GitHub PR comments. +It identifies the repository from `BUILDKITE_REPO` and takes the rest of the build +context from `BUILDKITE_BUILD_CHECKOUT_PATH` and the variables above — see +[Buildkite PR context](#buildkite-pr-context). 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) @@ -152,6 +200,18 @@ socket_scan: SOCKET_SECURITY_API_TOKEN: $SOCKET_SECURITY_API_TOKEN ``` +### Azure Pipelines + +```yaml +- script: | + socketcli \ + --integration azure \ + --enable-diff \ + --target-path "$(Build.SourcesDirectory)" + env: + SOCKET_SECURITY_API_TOKEN: $(SOCKET_SECURITY_API_TOKEN) +``` + ### Bitbucket Pipelines ```yaml @@ -162,6 +222,45 @@ pipelines: - socketcli --config .socketcli.toml --target-path . ``` +## Pull request and Dashboard association + +The CLI sends the resolved pull request number with each full scan and attaches +the pull request URL to diff scans so the Socket Dashboard can associate the +report with its originating change. If `--pr-number` is supplied, it wins; +passing `--pr-number 0` explicitly disables automatic association. + +Without an explicit value, the CLI recognizes: + +- GitHub Actions: `PR_NUMBER`, then the PR number in `GITHUB_REF`. +- GitLab CI: `CI_MERGE_REQUEST_IID`. +- Azure Pipelines: `SYSTEM_PULLREQUEST_PULLREQUESTNUMBER` for GitHub-hosted + repositories, otherwise `SYSTEM_PULLREQUEST_PULLREQUESTID` for Azure Repos. + +### Buildkite PR context + +Buildkite is SCM-provider neutral, so the CLI does not infer a provider or consume +its PR variable automatically. Pass Buildkite's +[`BUILDKITE_PULL_REQUEST`](https://buildkite.com/docs/pipelines/configure/environment-variables#BUILDKITE_PULL_REQUEST) +value to +`--pr-number` and identify the repository host with `--integration`, as shown in +the Buildkite platform example above. Buildkite sets `BUILDKITE_PULL_REQUEST` to +`false` outside PR builds; the CLI treats that value as no PR. + +Use `--integration github` for GitHub-hosted repositories and `--integration gitlab` +for GitLab-hosted ones. The CLI identifies the repository from +[`BUILDKITE_REPO`](https://buildkite.com/docs/pipelines/configure/environment-variables#BUILDKITE_REPO), +taking both the slug and the host from it, so github.com, GitLab.com, and self-hosted +installations all build a correct pull request or merge request link without extra +configuration. That same value identifies the repository for GitHub PR comments when +`--scm github` is set. `CI_PROJECT_URL` still overrides the derived GitLab project URL. +Keep `--scm api` unless you also intend to configure an existing GitHub or GitLab +comment adapter and its provider token. + +`--scm github` and `--scm gitlab` also imply the matching scan integration for +Dashboard metadata unless `--integration` was explicitly supplied. PR comments +remain limited to the existing GitHub and GitLab SCM adapters; Azure receives +console output and Dashboard association but does not post a PR comment. + ## Workflow templates Prebuilt examples in this repo: @@ -178,3 +277,11 @@ Prebuilt examples in this repo: - `--sarif-grouping alert` currently applies to `--sarif-scope full`. - Diff-based SARIF can validly be empty when there are no matching net-new alerts. - Keep API tokens in secret stores (`SOCKET_SECURITY_API_TOKEN`), not in config files. +- In Buildkite pipeline YAML, follow its + [runtime interpolation](https://buildkite.com/docs/pipelines/configure/environment-variables#runtime-variable-interpolation) + guidance and use `$$` for variables that must expand when the command runs rather + than when the pipeline is uploaded. +- Security findings with `props.firstPatchedVersionIdentifier` show that value in + the console table, including native Buildkite job logs, and in GitHub/GitLab + security comments when that SCM adapter is configured. Findings without a known + patched release leave the console cell blank and omit the comment field. diff --git a/docs/cli-reference.md b/docs/cli-reference.md index 0d4d4833..7b7d5345 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -175,7 +175,7 @@ If you don't want to provide the Socket API Token every time then you can use th | `--repo` | False | *auto* | Repository name in owner/repo format (auto-detected from git remote) | | `--workspace` | False | | The Socket workspace to associate the scan with (e.g. `my-org` in `my-org/my-repo`). See note below. | | `--repo-is-public` | False | False | If set, flags a new repository creation as public. Defaults to false. | -| `--integration` | False | api | Integration type (api, github, gitlab, azure, bitbucket) | +| `--integration` | False | api | Integration type (api, github, gitlab, azure, bitbucket). When omitted, `--scm github` or `--scm gitlab` implies the matching integration. | | `--owner` | False | | Name of the integration owner, defaults to the socket organization slug | | `--branch` | False | *auto* | Branch name (auto-detected from git) | | `--committers` | False | *auto* | Committer(s) to filter by (auto-detected from git commit) | @@ -189,7 +189,7 @@ If you don't want to provide the Socket API Token every time then you can use th #### Pull Request and Commit | Parameter | Required | Default | Description | |:-----------------|:---------|:--------|:-----------------------------------------------| -| `--pr-number` | False | "0" | Pull request number | +| `--pr-number` | False | *auto* | Pull request number. Auto-detected in GitHub Actions, GitLab CI, and Azure Pipelines; explicitly passing `0` disables detection. | | `--commit-message` | False | *auto* | Commit message (auto-detected from git) | | `--commit-sha` | False | *auto* | Commit SHA (auto-detected from git) | | `--base-scan-id` | False | | Full scan ID to diff against, overriding the repository's head scan as the baseline. Mutually exclusive with `--base-commit-sha` | diff --git a/pyproject.toml b/pyproject.toml index 3dcd5718..0cafbaf1 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.7.1" +version = "2.8.0" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 78220a1f..a97b4d18 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.7.1' +__version__ = '2.8.0' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/config.py b/socketsecurity/config.py index 0a6a5be4..645e5145 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -1,12 +1,14 @@ import argparse +import json import logging import os +import tomllib from dataclasses import asdict, dataclass, field from typing import List, Optional -from socketsecurity import __version__ + from socketdev import INTEGRATION_TYPES, IntegrationType -import json -import tomllib + +from socketsecurity import __version__ def get_plugin_config_from_env(prefix: str) -> dict: @@ -113,6 +115,7 @@ class CliConfig: branch: str = "" committers: Optional[List[str]] = None pr_number: str = "0" + pr_number_explicit: bool = False commit_message: Optional[str] = None default_branch: bool = False target_path: str = "./" @@ -206,10 +209,10 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig': pre_parser.add_argument("--config", dest="config_file", default=None) pre_args, _ = pre_parser.parse_known_args(args_list) + normalized_defaults = {} if pre_args.config_file: config_defaults = load_cli_config_file(pre_args.config_file) valid_dests = {action.dest for action in parser._actions if action.dest != "help"} - normalized_defaults = {} for key, value in config_defaults.items(): dest = str(key).replace("-", "_") if dest in valid_dests: @@ -217,6 +220,17 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig': parser.set_defaults(**normalized_defaults) args = parser.parse_args(args_list) + integration_explicit = hasattr(args, "integration") + pr_number_explicit = hasattr(args, "pr_number") + + integration_type = getattr(args, "integration", "api") + pr_number = getattr(args, "pr_number", "0") + if ( + not integration_explicit and + integration_type == "api" and + args.scm in ("github", "gitlab") + ): + integration_type = args.scm if args.reach_exclude_paths: logging.warning( @@ -260,7 +274,8 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig': 'repo': args.repo, 'branch': args.branch, 'committers': args.committers, - 'pr_number': args.pr_number, + 'pr_number': pr_number, + 'pr_number_explicit': pr_number_explicit, 'commit_message': commit_message, 'default_branch': args.default_branch, 'target_path': os.path.expanduser(args.target_path), @@ -292,7 +307,7 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig': 'disable_ignore': args.disable_ignore, 'upload_logs': args.upload_logs, 'strict_blocking': args.strict_blocking, - 'integration_type': args.integration, + 'integration_type': integration_type, 'pending_head': args.pending_head, 'timeout': args.timeout, 'exit_code_on_api_error': args.exit_code_on_api_error, @@ -517,8 +532,12 @@ def create_argument_parser() -> argparse.ArgumentParser: "--integration", choices=INTEGRATION_TYPES, metavar="", - help="Integration type of api, github, gitlab, azure, or bitbucket. Defaults to api", - default="api" + help=( + "Integration type of api, github, gitlab, azure, or bitbucket. " + "Defaults to api; --scm github/gitlab implies the matching integration " + "when this option is omitted" + ), + default=argparse.SUPPRESS ) integration_group.add_argument( "--owner", @@ -533,13 +552,17 @@ def create_argument_parser() -> argparse.ArgumentParser: "--pr-number", dest="pr_number", metavar="", - help="Pull request number", - default="0" + help=( + "Pull request number. Auto-detected in supported CI environments when omitted; " + "pass 0 explicitly to disable detection" + ), + default=argparse.SUPPRESS ) pr_group.add_argument( "--pr_number", dest="pr_number", - help=argparse.SUPPRESS + help=argparse.SUPPRESS, + default=argparse.SUPPRESS ) pr_group.add_argument( "--commit-message", diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index a5305bee..af5aa53e 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -1596,7 +1596,8 @@ def get_license_text_via_purl(self, packages: dict[str, Package], batch_size: in def get_diff_scan_artifacts( self, head_full_scan_id: str, - new_full_scan_id: str + new_full_scan_id: str, + external_href: Optional[str] = None ) -> DiffArtifacts: """Compare two full scans via the diff-scans endpoints, polling for the result. @@ -1619,6 +1620,8 @@ def get_diff_scan_artifacts( Args: head_full_scan_id: The before/base full scan ID new_full_scan_id: The after/head full scan ID + external_href: Optional pull request or merge request URL to associate + with the diff scan in the Socket Dashboard Returns: DiffArtifacts with the added/removed/unchanged/replaced/updated lists @@ -1628,6 +1631,16 @@ def get_diff_scan_artifacts( "after": new_full_scan_id, "description": f"Socket Security CLI v{__version__} scan comparison", } + if external_href: + create_params["external_href"] = external_href + # external_href is only honored while a diff scan is being created, + # so re-running a comparison over an already-compared scan pair + # would otherwise leave the Dashboard report with no link back to + # the pull request. on_duplicate=update applies the link to the + # existing resource and answers 200 with the same {"diff_scan": ...} + # envelope as a create. Notably it is not on_duplicate=redirect, + # whose 302 the SDK follows into a GET without cached=true. + create_params["on_duplicate"] = "update" try: result = self.sdk.diffscans.create_from_ids(self.config.org_slug, create_params) diff_scan = result.get("diff_scan") or {} @@ -1636,11 +1649,14 @@ def get_diff_scan_artifacts( if error.status_code != 409: raise - # Do not use on_duplicate=redirect here. The SDK follows that 302 - # automatically with a GET that lacks cached=true, which can leave - # the connection idle while an existing diff scan is still computing. - # Resolve the duplicate resource explicitly so every result fetch - # continues through the bounded cached polling path below. + # Reached without on_duplicate=update (no pull request context to + # attach) and against deployments that predate it and still answer + # 409 regardless. Do not switch this to on_duplicate=redirect: the + # SDK follows that 302 automatically with a GET that lacks + # cached=true, which can leave the connection idle while an existing + # diff scan is still computing. Resolve the duplicate resource + # explicitly so every result fetch continues through the bounded + # cached polling path below. existing = self.sdk.diffscans.list( self.config.org_slug, params={ @@ -1776,7 +1792,8 @@ def get_added_and_removed_packages( self, head_full_scan_id: str, new_full_scan_id: str, - include_license_details: bool = False + include_license_details: bool = False, + external_href: Optional[str] = None ) -> Tuple[Dict[str, Package], Dict[str, Package], Dict[str, Package]]: """ Get packages that were added and removed between scans. @@ -1809,6 +1826,8 @@ def get_added_and_removed_packages( is retained as an explicit override seam, not wired to the ``--exclude-license-details`` user flag (which still governs the human-facing dashboard report URL). + external_href: Optional pull request or merge request URL to associate + with the primary diff-scan resource Returns: Tuple of (added_packages, removed_packages) dictionaries @@ -1820,7 +1839,8 @@ def get_added_and_removed_packages( try: diff_artifacts = self.get_diff_scan_artifacts( head_full_scan_id, - new_full_scan_id + new_full_scan_id, + external_href=external_href, ) except Exception as error: # SDK error messages can span many lines (path + response headers); the @@ -1933,7 +1953,8 @@ def create_new_diff( save_files_list_path: Optional[str] = None, save_manifest_tar_path: Optional[str] = None, base_paths: Optional[List[str]] = None, - explicit_files: Optional[List[str]] = None + explicit_files: Optional[List[str]] = None, + external_href: Optional[str] = None ) -> Diff: """Create a new diff using the Socket SDK. @@ -1945,6 +1966,8 @@ def create_new_diff( save_manifest_tar_path: Optional path to save manifest files tar.gz archive base_paths: List of base paths for the scan (optional) explicit_files: Optional list of explicit files to use instead of discovering files + external_href: Optional pull request or merge request URL to associate + with the diff scan """ log.debug(f"starting create_new_diff with no_change: {no_change}") if no_change: @@ -2069,7 +2092,8 @@ def create_new_diff( ) = self.get_added_and_removed_packages( head_full_scan_id, new_full_scan.id, - include_license_details=False + include_license_details=False, + external_href=external_href, ) # Separate unchanged packages from added/removed for --strict-blocking support @@ -2133,16 +2157,22 @@ def create_diff_report( alerts_in_removed_packages: Dict[str, List[Issue]] = {} alerts_in_unchanged_packages: Dict[str, List[Issue]] = {} - seen_new_packages = set() - seen_removed_packages = set() + seen_packages = { + "added": set(), + "updated": set(), + "removed": set(), + "replaced": set(), + } for package_id, package in added_packages.items(): purl = self.create_purl(package_id, added_packages) base_purl = f"{purl.ecosystem}/{purl.name}@{purl.version}" - if (not direct_only or package.direct) and base_purl not in seen_new_packages: - diff.new_packages.append(purl) - seen_new_packages.add(base_purl) + change_type = "updated" if package.diffType == "updated" else "added" + target = diff.updated_packages if change_type == "updated" else diff.new_packages + if (not direct_only or package.direct) and base_purl not in seen_packages[change_type]: + target.append(purl) + seen_packages[change_type].add(base_purl) self.add_package_alerts_to_collection( package=package, @@ -2154,9 +2184,11 @@ def create_diff_report( purl = self.create_purl(package_id, removed_packages) base_purl = f"{purl.ecosystem}/{purl.name}@{purl.version}" - if (not direct_only or package.direct) and base_purl not in seen_removed_packages: - diff.removed_packages.append(purl) - seen_removed_packages.add(base_purl) + change_type = "replaced" if package.diffType == "replaced" else "removed" + target = diff.replaced_packages if change_type == "replaced" else diff.removed_packages + if (not direct_only or package.direct) and base_purl not in seen_packages[change_type]: + target.append(purl) + seen_packages[change_type].add(base_purl) self.add_package_alerts_to_collection( package=package, @@ -2286,18 +2318,16 @@ def add_purl_capabilities(diff: Diff) -> None: Args: diff: Diff object to update with capability information """ - new_packages = [] - for purl in diff.new_packages: - if purl.id in diff.new_capabilities: - new_purl = Purl( - **{**purl.__dict__, - "capabilities": diff.new_capabilities[purl.id]} - ) - new_packages.append(new_purl) - else: - new_packages.append(purl) - - diff.new_packages = new_packages + for attribute in ("new_packages", "updated_packages"): + packages = [] + for purl in getattr(diff, attribute): + if purl.id in diff.new_capabilities: + purl = Purl( + **{**purl.__dict__, + "capabilities": diff.new_capabilities[purl.id]} + ) + packages.append(purl) + setattr(diff, attribute, packages) def add_package_alerts_to_collection(self, package: Package, alerts_collection: dict, packages: dict) -> dict: """ diff --git a/socketsecurity/core/alert_selection.py b/socketsecurity/core/alert_selection.py index ae5b4772..132be294 100644 --- a/socketsecurity/core/alert_selection.py +++ b/socketsecurity/core/alert_selection.py @@ -31,7 +31,9 @@ def clone_diff_with_selected_alerts(diff: Diff, selected_alerts: List[Issue]) -> removed_alerts=[], diff_url=getattr(diff, "diff_url", ""), new_packages=getattr(diff, "new_packages", []), + updated_packages=getattr(diff, "updated_packages", []), removed_packages=getattr(diff, "removed_packages", []), + replaced_packages=getattr(diff, "replaced_packages", []), packages=getattr(diff, "packages", {}), ) selected_diff.id = getattr(diff, "id", "") diff --git a/socketsecurity/core/classes.py b/socketsecurity/core/classes.py index db145221..d821d872 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -507,7 +507,9 @@ class Diff: """ new_packages: list[Purl] + updated_packages: list[Purl] removed_packages: list[Purl] + replaced_packages: list[Purl] packages: dict[str, Package] new_capabilities: Dict[str, List[str]] new_alerts: list[Issue] @@ -525,8 +527,12 @@ def __init__(self, **kwargs): setattr(self, key, value) if not hasattr(self, "new_packages"): self.new_packages = [] + if not hasattr(self, "updated_packages"): + self.updated_packages = [] if not hasattr(self, "removed_packages"): self.removed_packages = [] + if not hasattr(self, "replaced_packages"): + self.replaced_packages = [] if not hasattr(self, "new_alerts"): self.new_alerts = [] if not hasattr(self, "unchanged_alerts"): @@ -548,8 +554,10 @@ def to_dict(self) -> dict: """ return { "new_packages": [p.to_dict() for p in self.new_packages], + "updated_packages": [p.to_dict() for p in self.updated_packages], "new_capabilities": self.new_capabilities, "removed_packages": [p.to_dict() for p in self.removed_packages], + "replaced_packages": [p.to_dict() for p in self.replaced_packages], "new_alerts": [alert.__dict__ for alert in self.new_alerts], "unchanged_alerts": [alert.__dict__ for alert in self.unchanged_alerts] if hasattr(self, "unchanged_alerts") else [], "removed_alerts": [alert.__dict__ for alert in self.removed_alerts] if hasattr(self, "removed_alerts") else [], diff --git a/socketsecurity/core/git_remote.py b/socketsecurity/core/git_remote.py new file mode 100644 index 00000000..9ca5dc57 --- /dev/null +++ b/socketsecurity/core/git_remote.py @@ -0,0 +1,43 @@ +"""Parsing for git remote URLs. + +CI systems that are not tied to a single SCM expose the checkout URL rather than +an ``owner/repo`` slug (Buildkite's ``BUILDKITE_REPO``, for example). Both the +GitHub comment adapter and pull request context resolution need to recover the +slug from it, so the parsing lives here rather than in either caller. +""" +import re +from typing import Optional, Tuple +from urllib.parse import urlparse + +# git@host:owner/repo - the scp-like syntax urlparse cannot handle. The negative +# lookahead keeps scheme-prefixed URLs (https://, ssh://) out of this branch. +_SCP_LIKE_REMOTE = re.compile(r"^(?:[^@/]+@)?([^:/]+):(?!//)(.+)$") + + +def parse_git_remote(value: Optional[str]) -> Tuple[Optional[str], Optional[str]]: + """Split a git remote URL into its host and its repository path. + + Returns ``(host, path)``, or ``(None, None)`` when the value is not a usable + remote. The path is returned whole rather than as ``owner``/``repo`` because + GitLab projects can be nested under subgroups; callers that only want the + last two segments can split it themselves. ``host`` is ``None`` for a bare + ``owner/repo`` path, which carries no host to report. + """ + if not value: + return None, None + url = value.strip().rstrip("/") + if url.endswith(".git"): + url = url[:-4] + + match = _SCP_LIKE_REMOTE.match(url) + if match: + return match.group(1), match.group(2).strip("/") + + parsed = urlparse(url) + if parsed.scheme in ("http", "https", "ssh", "git") and parsed.hostname: + return parsed.hostname, parsed.path.strip("/") + + # A bare owner/repo path, with no scheme and nothing to infer a host from. + if "/" in url: + return None, url.strip("/") + return None, None diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 673dde5c..76047689 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -4,7 +4,9 @@ import re import uuid from datetime import datetime, timezone +from html import escape from pathlib import Path + from mdutils import MdUtils from prettytable import PrettyTable @@ -14,6 +16,13 @@ class Messages: + @staticmethod + def get_patched_version(alert: Issue) -> str: + """Return the first patched version exposed by an alert, if any.""" + props = getattr(alert, "props", {}) or {} + value = props.get("firstPatchedVersionIdentifier") + return str(value) if value not in (None, "") else "" + @staticmethod def map_severity_to_sarif(severity: str) -> str: """ @@ -927,7 +936,7 @@ def security_comment_template(diff: Diff, config=None) -> str: > **Review the following alerts detected in dependencies.** > > According to your organization's policies, you **must** resolve all **"Block"** alerts before proceeding. It's recommended to resolve **"Warn"** alerts too. -> Learn more about [Socket for GitHub](https://socket.dev?utm_medium=gh). +> Learn more about [Socket](https://socket.dev). @@ -948,12 +957,18 @@ def security_comment_template(diff: Diff, config=None) -> str: severity_icon = Messages.get_severity_icon(alert.severity) action = "Block" if alert.error else "Warn" details_open = "" + patched_version = Messages.get_patched_version(alert) + patched_version_html = ( + "

Patched version: " + f"{escape(Messages.inline_html_text(patched_version))}

" + if patched_version else "" + ) # Generate proper manifest URL manifest_url = Messages.get_manifest_file_url(diff, alert.manifests, config) # Generate a table row for each alert ignore_html = ( f"

Mark as acceptable risk: To ignore this alert only in this pull request, reply with:
" - f"@SocketSecurity ignore {alert.pkg_name}@{alert.pkg_version}
" + f"@SocketSecurity ignore {alert.pkg_type}/{alert.pkg_name}@{alert.pkg_version}
" f"Or ignore all future alerts with:
" f"@SocketSecurity ignore-all

" ) if show_ignore else "" @@ -968,6 +983,7 @@ def security_comment_template(diff: Diff, config=None) -> str:
{alert.pkg_name}@{alert.pkg_version} - {Messages.inline_html_text(alert.title)}

Note: {Messages.inline_html_text(alert.description)}

+ {patched_version_html}

Source: Manifest File

ℹ️ Read more on: This package | @@ -1016,7 +1032,7 @@ def security_comment_template(diff: Diff, config=None) -> str: license_ignore_html = ( f"

Mark the package as acceptable risk: To ignore this alert only in this pull request, reply with the comment " - f"@SocketSecurity ignore {first_alert.pkg_name}@{first_alert.pkg_version}. " + f"@SocketSecurity ignore {first_alert.pkg_type}/{first_alert.pkg_name}@{first_alert.pkg_version}. " f"You can also ignore all packages with @SocketSecurity ignore-all. " f"To ignore an alert for all future pull requests, use Socket's Dashboard to change the triage state of this alert.

" ) if show_ignore else "" @@ -1252,51 +1268,58 @@ def create_added_table(diff: Diff, md: MdUtils) -> MdUtils: num_of_overview_columns = len(overview_table) count = 0 - for added in diff.new_packages: - added: Purl # Ensure `added` has scores and relevant attributes. - - package_url = f"[{added.purl}]({added.url})" - diff_badge = f"[![+](https://github-app-statics.socket.dev/diff-added.svg)]({added.url})" - - # Scores dynamically converted to badge URLs and linked - def score_to_badge(score): - score_percent = int(score * 100) # Convert to integer percentage - return f"[![{score_percent}](https://github-app-statics.socket.dev/score-{score_percent}.svg)]({added.url})" - - def get_score_for_badge(score_name: str) -> float: - scores = getattr(added, "scores", None) - if isinstance(scores, dict): - raw_score = scores.get(score_name) - else: - raw_score = getattr(scores, score_name, None) if scores is not None else None - - if raw_score is None: - return 1.0 - - score = float(raw_score) - if score > 1: - score = score / 100 - return max(0.0, min(score, 1.0)) - - # Generate badges for each score type - supply_chain_risk_badge = score_to_badge(get_score_for_badge("supplyChain")) - vulnerability_badge = score_to_badge(get_score_for_badge("vulnerability")) - quality_badge = score_to_badge(get_score_for_badge("quality")) - maintenance_badge = score_to_badge(get_score_for_badge("maintenance")) - license_badge = score_to_badge(get_score_for_badge("license")) - - # Add the row for this package - row = [ - diff_badge, - package_url, - supply_chain_risk_badge, - vulnerability_badge, - quality_badge, - maintenance_badge, - license_badge - ] - overview_table.extend(row) - count += 1 # Count total packages + changes = ( + ("Added", diff.new_packages), + ("Updated", diff.updated_packages), + ("Removed", diff.removed_packages), + ("Replaced", diff.replaced_packages), + ) + for change, packages in changes: + for package in packages: + package: Purl + + package_url = f"[{package.purl}]({package.url})" + diff_badge = f"**{change}**" + + # Scores dynamically converted to badge URLs and linked + def score_to_badge(score): + score_percent = int(score * 100) # Convert to integer percentage + return f"[![{score_percent}](https://github-app-statics.socket.dev/score-{score_percent}.svg)]({package.url})" + + def get_score_for_badge(score_name: str) -> float: + scores = getattr(package, "scores", None) + if isinstance(scores, dict): + raw_score = scores.get(score_name) + else: + raw_score = getattr(scores, score_name, None) if scores is not None else None + + if raw_score is None: + return 1.0 + + score = float(raw_score) + if score > 1: + score = score / 100 + return max(0.0, min(score, 1.0)) + + # Generate badges for each score type + supply_chain_risk_badge = score_to_badge(get_score_for_badge("supplyChain")) + vulnerability_badge = score_to_badge(get_score_for_badge("vulnerability")) + quality_badge = score_to_badge(get_score_for_badge("quality")) + maintenance_badge = score_to_badge(get_score_for_badge("maintenance")) + license_badge = score_to_badge(get_score_for_badge("license")) + + # Add the row for this package + row = [ + diff_badge, + package_url, + supply_chain_risk_badge, + vulnerability_badge, + quality_badge, + maintenance_badge, + license_badge + ] + overview_table.extend(row) + count += 1 # Calculate total rows for table num_of_overview_rows = count + 1 # Include header row @@ -1331,6 +1354,7 @@ def create_console_security_alert_table(diff: Diff) -> PrettyTable: [ "Alert", "Package", + "Patched Version", "url", "Introduced by", "Manifest File", @@ -1351,6 +1375,7 @@ def create_console_security_alert_table(diff: Diff) -> PrettyTable: row = [ alert.title, alert.purl, + Messages.get_patched_version(alert), alert.url, source_str, manifest_str, diff --git a/socketsecurity/core/pull_request.py b/socketsecurity/core/pull_request.py new file mode 100644 index 00000000..60ad3965 --- /dev/null +++ b/socketsecurity/core/pull_request.py @@ -0,0 +1,118 @@ +import re +from dataclasses import dataclass +from typing import Mapping, Optional +from urllib.parse import urlparse + +from socketsecurity.core.git_remote import parse_git_remote + + +@dataclass(frozen=True) +class PullRequestContext: + number: int = 0 + url: Optional[str] = None + + +def _positive_int(value) -> int: + try: + parsed = int(value) + except (TypeError, ValueError): + return 0 + return parsed if parsed > 0 else 0 + + +def _repository_url(value: Optional[str]) -> Optional[str]: + if not value: + return None + url = value.strip().rstrip("/") + if url.endswith(".git"): + url = url[:-4] + parsed = urlparse(url) + return url if parsed.scheme in ("http", "https") and parsed.netloc else None + + +def _github_number(env: Mapping[str, str]) -> int: + number = _positive_int(env.get("PR_NUMBER")) + if number: + return number + match = re.match(r"^refs/pull/(\d+)/", env.get("GITHUB_REF", "")) + return _positive_int(match.group(1)) if match else 0 + + +def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]: + remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO")) + # config.repo is only ever a bare repository name, so it cannot produce a + # slug on its own; it is kept last for callers that pass a full owner/repo. + repository = env.get("GITHUB_REPOSITORY") or remote_path or repo + if not repository or "/" not in repository: + return None + server = env.get("GITHUB_SERVER_URL") or (f"https://{remote_host}" if remote_host else "") + server = (server or "https://github.com").rstrip("/") + return f"{server}/{repository.strip('/')}/pull/{number}" + + +def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]: + project_url = _repository_url(env.get("CI_PROJECT_URL")) + if not project_url: + remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO")) + project_path = env.get("CI_PROJECT_PATH") or remote_path or repo + server = env.get("CI_SERVER_URL") or (f"https://{remote_host}" if remote_host else "") + server = server.rstrip("/") + if server and project_path and "/" in project_path: + project_url = f"{server}/{project_path.strip('/')}" + return f"{project_url}/-/merge_requests/{number}" if project_url else None + + +def _azure_url(number: int, env: Mapping[str, str], github_pr: bool) -> Optional[str]: + repository_url = _repository_url( + env.get("BUILD_REPOSITORY_URI") or + env.get("SYSTEM_PULLREQUEST_SOURCEREPOSITORYURI") + ) + if not repository_url: + return None + github_pr = github_pr or "github" in urlparse(repository_url).netloc.lower() + path = "pull" if github_pr else "pullrequest" + return f"{repository_url}/{path}/{number}" + + +def resolve_pull_request_context( + integration_type: str, + configured_number, + repo: Optional[str], + *, + configured_explicit: bool = False, + env: Optional[Mapping[str, str]] = None, +) -> PullRequestContext: + """Resolve PR metadata without making provider API calls. + + Explicit CLI/config values win, including an explicit zero used to disable + association. Otherwise the provider's standard CI environment is used. + """ + environment = env or {} + provider = str(integration_type or "api").lower() + number = _positive_int(configured_number) + + if not configured_explicit and not number: + if provider == "github": + number = _github_number(environment) + elif provider == "gitlab": + number = _positive_int(environment.get("CI_MERGE_REQUEST_IID")) + elif provider == "azure": + number = ( + _positive_int(environment.get("SYSTEM_PULLREQUEST_PULLREQUESTNUMBER")) or + _positive_int(environment.get("SYSTEM_PULLREQUEST_PULLREQUESTID")) + ) + + if not number: + return PullRequestContext() + + if provider == "github": + url = _github_url(number, repo, environment) + elif provider == "gitlab": + url = _gitlab_url(number, repo, environment) + elif provider == "azure": + github_pr = bool(environment.get("SYSTEM_PULLREQUEST_PULLREQUESTNUMBER")) + url = _azure_url(number, environment, github_pr) + else: + url = None + + return PullRequestContext(number=number, url=url) diff --git a/socketsecurity/core/scm/github.py b/socketsecurity/core/scm/github.py index 7504a46c..9ec1e4ca 100644 --- a/socketsecurity/core/scm/github.py +++ b/socketsecurity/core/scm/github.py @@ -1,7 +1,6 @@ import json import os import sys -import urllib.parse from dataclasses import dataclass from git import Optional @@ -9,6 +8,7 @@ from socketsecurity import USER_AGENT from socketsecurity.core import log from socketsecurity.core.classes import Comment +from socketsecurity.core.git_remote import parse_git_remote from socketsecurity.core.scm_comments import Comments from socketsecurity.socketcli import CliClient @@ -38,24 +38,12 @@ class GithubConfig: @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("/") + # Comments and statuses belong to the pipeline/base repository, not a + # contributor's fork from BUILDKITE_PULL_REQUEST_REPO. + _, repository_path = parse_git_remote( + os.getenv("BUILDKITE_REPO") or os.getenv("BUILDKITE_PULL_REQUEST_REPO") + ) + parts = repository_path.split("/") if repository_path else [] if len(parts) < 2: return "", "" return parts[-2], parts[-1] diff --git a/socketsecurity/core/scm_comments.py b/socketsecurity/core/scm_comments.py index 7c479b72..ea758ca1 100644 --- a/socketsecurity/core/scm_comments.py +++ b/socketsecurity/core/scm_comments.py @@ -37,10 +37,10 @@ def remove_alerts(comments: dict, new_alerts: list) -> list: if ignore_all: break else: - full_name = f"{alert.pkg_type}/{alert.pkg_name}" - purl = (full_name, alert.pkg_version) - purl_star = (full_name, "*") - if purl in ignore_commands or purl_star in ignore_commands: + if any( + Comments.is_ignore(alert.pkg_name, alert.pkg_version, name, version, alert.pkg_type) + for name, version in ignore_commands + ): log.info(f"Alerts for {alert.pkg_name}@{alert.pkg_version} ignored") else: log.info(f"Adding alert {alert.type} for {alert.pkg_name}@{alert.pkg_version}") @@ -66,8 +66,10 @@ def get_ignore_options(comments: dict) -> [bool, list]: ignore_all = True else: command = command.lstrip("ignore").strip() - name, version = command.split("@") - data = (name, version) + name, separator, version = command.rpartition("@") + if not separator or not name or not version: + raise ValueError("Expected package@version") + data = (name.strip(), version.strip()) ignore_commands.append(data) except Exception as error: log.error(f"Unable to process ignore command for {comment}") @@ -75,11 +77,17 @@ def get_ignore_options(comments: dict) -> [bool, list]: return ignore_all, ignore_commands @staticmethod - def is_ignore(pkg_name: str, pkg_version: str, name: str, version: str) -> bool: - result = False - if pkg_name == name and (pkg_version == version or version == "*"): - result = True - return result + def is_ignore( + pkg_name: str, pkg_version: str, name: str, version: str, + pkg_type: str = "" + ) -> bool: + package_names = {pkg_name} + if pkg_type: + package_names.add(f"{pkg_type}/{pkg_name}") + target_names = {name} + if not pkg_type and "/" in name: + target_names.add(name.split("/", 1)[1]) + return bool(package_names & target_names) and (pkg_version == version or version == "*") @staticmethod def is_heading_line(line) -> bool: @@ -187,7 +195,7 @@ def process_updated_security_comment( # Extract package name and version from the comment try: start_marker = stripped[len("" in body assert "" in body + def test_copy_is_provider_neutral(self): + body = Messages.security_comment_template( + _make_diff([_make_alert()]), _FakeConfig(scm="gitlab") + ) + assert "Socket for GitHub" not in body + assert "Learn more about [Socket]" in body + class TestSecurityCommentTemplateWithNoAlerts: def test_no_alerts_omits_the_empty_table(self): @@ -232,6 +239,23 @@ def test_ignoring_every_alert_individually_collapses_too(self): assert "No dependency alerts to report" in new_body + def test_qualified_scoped_package_ignore_matches_comment_marker(self): + security = _security_comment_with([ + _make_alert( + pkg_name="@socketsecurity/example", + purl="pkg:npm/@socketsecurity/example@4.17.21", + ) + ]) + comments = { + "security": security, + "ignore": [_make_comment( + "SocketSecurity ignore npm/@socketsecurity/example@4.17.21", + comment_id=2, + )], + } + + assert "No dependency alerts to report" in Comments.process_security_comment(security, comments) + def test_no_ignore_commands_leaves_alerts_in_place(self): security = self._two_alert_comment() comments = {"security": security, "ignore": []} diff --git a/tests/unit/test_pull_request_context.py b/tests/unit/test_pull_request_context.py new file mode 100644 index 00000000..5ad12903 --- /dev/null +++ b/tests/unit/test_pull_request_context.py @@ -0,0 +1,228 @@ +from socketsecurity.core.pull_request import resolve_pull_request_context + + +def test_explicit_pr_number_wins_over_detected_context(): + context = resolve_pull_request_context( + "github", + "42", + "acme/widgets", + configured_explicit=True, + env={ + "GITHUB_REF": "refs/pull/99/merge", + "GITHUB_REPOSITORY": "acme/widgets", + }, + ) + + assert context.number == 42 + assert context.url == "https://github.com/acme/widgets/pull/42" + + +def test_explicit_zero_disables_pr_auto_detection(): + context = resolve_pull_request_context( + "github", + "0", + "acme/widgets", + configured_explicit=True, + env={"GITHUB_REF": "refs/pull/99/merge"}, + ) + + assert context.number == 0 + assert context.url is None + + +def test_buildkite_non_pr_sentinel_is_treated_as_no_pull_request(): + context = resolve_pull_request_context( + "github", + "false", + "acme/widgets", + configured_explicit=True, + env={}, + ) + + assert context.number == 0 + assert context.url is None + + +def test_github_context_is_detected_from_actions_environment(): + context = resolve_pull_request_context( + "github", + "0", + None, + env={ + "GITHUB_REF": "refs/pull/123/merge", + "GITHUB_REPOSITORY": "acme/widgets", + "GITHUB_SERVER_URL": "https://github.example.com", + }, + ) + + assert context.number == 123 + assert context.url == "https://github.example.com/acme/widgets/pull/123" + + +def test_gitlab_context_is_detected_from_merge_request_environment(): + context = resolve_pull_request_context( + "gitlab", + "0", + None, + env={ + "CI_MERGE_REQUEST_IID": "81", + "CI_PROJECT_URL": "https://gitlab.example.com/acme/widgets", + }, + ) + + assert context.number == 81 + assert context.url == "https://gitlab.example.com/acme/widgets/-/merge_requests/81" + + +def test_azure_repos_context_uses_pull_request_id(): + context = resolve_pull_request_context( + "azure", + "0", + None, + env={ + "SYSTEM_PULLREQUEST_PULLREQUESTID": "17", + "BUILD_REPOSITORY_URI": "https://dev.azure.com/acme/platform/_git/widgets", + }, + ) + + assert context.number == 17 + assert context.url == "https://dev.azure.com/acme/platform/_git/widgets/pullrequest/17" + + +def test_azure_fork_context_uses_target_repository_url(): + context = resolve_pull_request_context( + "azure", + "0", + None, + env={ + "SYSTEM_PULLREQUEST_PULLREQUESTID": "17", + "BUILD_REPOSITORY_URI": "https://dev.azure.com/acme/platform/_git/widgets", + "SYSTEM_PULLREQUEST_SOURCEREPOSITORYURI": ( + "https://dev.azure.com/contributor/forks/_git/widgets" + ), + }, + ) + + assert context.number == 17 + assert context.url == "https://dev.azure.com/acme/platform/_git/widgets/pullrequest/17" + + +def test_azure_pipeline_with_github_repo_uses_pull_request_number(): + context = resolve_pull_request_context( + "azure", + "0", + None, + env={ + "SYSTEM_PULLREQUEST_PULLREQUESTNUMBER": "23", + "SYSTEM_PULLREQUEST_PULLREQUESTID": "98765", + "BUILD_REPOSITORY_URI": "https://github.com/acme/widgets.git", + }, + ) + + assert context.number == 23 + assert context.url == "https://github.com/acme/widgets/pull/23" + + +def test_explicit_azure_github_pr_number_still_uses_github_url_shape(): + context = resolve_pull_request_context( + "azure", + "23", + None, + configured_explicit=True, + env={"BUILD_REPOSITORY_URI": "https://github.com/acme/widgets.git"}, + ) + + assert context.number == 23 + assert context.url == "https://github.com/acme/widgets/pull/23" + + +def test_non_pr_run_has_no_context(): + context = resolve_pull_request_context("azure", "0", "acme/widgets", env={}) + + assert context.number == 0 + assert context.url is None + + +# --------------------------------------------------------------------------- +# Provider-neutral CI (Buildkite). The provider comes from --integration and the +# PR number from --pr-number; only the repository slug has to be recovered from +# the checkout URL, because config.repo is a bare repository name with no owner. +# --------------------------------------------------------------------------- + + +def test_buildkite_github_repo_url_is_derived_from_the_checkout_remote(): + context = resolve_pull_request_context( + "github", + "42", + "widgets", + configured_explicit=True, + env={"BUILDKITE_REPO": "git@github.com:acme/widgets.git"}, + ) + + assert context.number == 42 + assert context.url == "https://github.com/acme/widgets/pull/42" + + +def test_buildkite_github_enterprise_host_is_taken_from_the_remote(): + context = resolve_pull_request_context( + "github", + "42", + "widgets", + configured_explicit=True, + env={"BUILDKITE_REPO": "https://github.example.com/acme/widgets.git"}, + ) + + assert context.url == "https://github.example.com/acme/widgets/pull/42" + + +def test_github_actions_environment_wins_over_the_checkout_remote(): + context = resolve_pull_request_context( + "github", + "42", + "widgets", + configured_explicit=True, + env={ + "GITHUB_REPOSITORY": "acme/widgets", + "GITHUB_SERVER_URL": "https://github.example.com", + "BUILDKITE_REPO": "git@github.com:stale/mirror.git", + }, + ) + + assert context.url == "https://github.example.com/acme/widgets/pull/42" + + +def test_buildkite_gitlab_repo_url_keeps_nested_subgroups(): + context = resolve_pull_request_context( + "gitlab", + "81", + "widgets", + configured_explicit=True, + env={"BUILDKITE_REPO": "ssh://git@gitlab.example.com/acme/platform/widgets.git"}, + ) + + assert context.url == "https://gitlab.example.com/acme/platform/widgets/-/merge_requests/81" + + +def test_gitlab_ci_project_url_wins_over_the_checkout_remote(): + context = resolve_pull_request_context( + "gitlab", + "81", + "widgets", + configured_explicit=True, + env={ + "CI_PROJECT_URL": "https://gitlab.example.com/acme/widgets", + "BUILDKITE_REPO": "git@gitlab.example.com:stale/mirror.git", + }, + ) + + assert context.url == "https://gitlab.example.com/acme/widgets/-/merge_requests/81" + + +def test_bare_repository_name_alone_yields_no_url(): + """config.repo has no owner segment, so it cannot stand in for a slug.""" + context = resolve_pull_request_context( + "github", "42", "widgets", configured_explicit=True, env={} + ) + + assert context.number == 42 + assert context.url is None diff --git a/tests/unit/test_socketcli.py b/tests/unit/test_socketcli.py index 8cae52ba..a008416e 100644 --- a/tests/unit/test_socketcli.py +++ b/tests/unit/test_socketcli.py @@ -2,10 +2,12 @@ import pytest -from socketsecurity.core.classes import Diff, Package from socketsecurity import socketcli -from socketsecurity.socketcli import build_license_artifact_payload, should_write_comment - +from socketsecurity.core.classes import Diff, Package +from socketsecurity.socketcli import ( + build_license_artifact_payload, + should_write_comment, +) # --------------------------------------------------------------------------- # Exit-code-on-api-error (flag-only, non-breaking for 2.3.x). @@ -63,6 +65,23 @@ def test_keyboard_interrupt_still_exits_2(monkeypatch): assert code == 2 +@pytest.mark.parametrize("scm", ["github", "gitlab"]) +def test_pr_context_provider_prefers_active_scm_adapter(scm): + assert socketcli._select_pull_request_provider("api", scm) == scm + + +def test_pr_context_provider_uses_integration_without_comment_adapter(): + assert socketcli._select_pull_request_provider("azure", "api") == "azure" + + +def test_scm_merge_request_event_creates_diff(): + assert socketcli._should_create_scm_diff("diff") is True + + +def test_scm_branch_event_always_uses_full_scan(): + assert socketcli._should_create_scm_diff("main") is False + + # --------------------------------------------------------------------------- # Buildkite-aware infrastructure error formatting. # --------------------------------------------------------------------------- diff --git a/uv.lock b/uv.lock index d0338009..542a8620 100644 --- a/uv.lock +++ b/uv.lock @@ -1282,7 +1282,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.7.1" +version = "2.8.0" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" }, diff --git a/workflows/buildkite.yml b/workflows/buildkite.yml index a2f8e452..3657f283 100644 --- a/workflows/buildkite.yml +++ b/workflows/buildkite.yml @@ -1,13 +1,22 @@ # Socket Security Buildkite pipeline example -# Runs Socket CLI in a Buildkite step using repository-level environment variables. +# Runs Socket CLI in a Buildkite step. Set SOCKET_SCM_INTEGRATION below to github +# or gitlab for Dashboard PR association, or leave it as api when provider +# association is not wanted. The repository slug and host are read from +# BUILDKITE_REPO, so no further configuration is needed for either provider. + +env: + SOCKET_SCM_INTEGRATION: "api" steps: - label: "Socket Security Scan" command: | socketcli \ --target-path . \ - --scm api \ - --pr-number 0 - env: - # Configure this in Buildkite pipeline/repo settings. - SOCKET_SECURITY_API_TOKEN: "${SOCKET_SECURITY_API_TOKEN}" + --integration "$${SOCKET_SCM_INTEGRATION:-api}" \ + --pr-number "$${BUILDKITE_PULL_REQUEST:-0}" + secrets: + - SOCKET_SECURITY_API_TOKEN + + # This uses a Buildkite secret named SOCKET_SECURITY_API_TOKEN. If your + # organization uses an external secrets plugin or agent hook, remove the + # secrets block and inject that environment variable through your mechanism.