From 8c5d3e9d5478b93cd595ff80b1dc5997a0c63e95 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:43:56 -0600 Subject: [PATCH 1/3] Reject owner/repo in --repo and deprecate --pending-head --repo was documented as owner/repo, but the value is sent as the repository slug, which the API validates against [a-zA-Z0-9._-]+. Following the help text failed every scan, at repository creation or full-scan creation. Describe the slug in the help text and docs, and exit 1 up front when --repo contains "/", naming the slug to use. --pending-head was parsed and never read: set_as_pending_head follows is_default_branch. Keep accepting it so existing pipelines still parse, but hide it from --help and log a deprecation warning pointing at --default-branch. Also fix the docs examples that taught owner/repo: the GitLab CI example ($CI_PROJECT_PATH -> $CI_PROJECT_NAME) and the Slack repos filter, which is matched against the bare slug. Bump to 2.11.12. --- CHANGELOG.md | 19 +++++++++++++++++++ docs/cli-reference.md | 19 +++++++++---------- pyproject.toml | 2 +- socketsecurity/__init__.py | 2 +- socketsecurity/config.py | 26 +++++++++++++++++++++++--- tests/unit/test_config.py | 28 ++++++++++++++++++++++++++++ tests/unit/test_fossa_compat.py | 14 +++++++------- uv.lock | 2 +- 8 files changed, 89 insertions(+), 23 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bfe78cc5..556d155a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,24 @@ # Changelog +## 2.11.12 + +### Fixed: `--repo` takes a repository slug, not `owner/repo` + +- The help text and docs said `--repo` takes `owner/repo`, but the API only + accepts a repository slug, which can't contain `/`. Following the docs made + every scan fail. The help text and docs now describe the slug, and the CLI + exits `1` up front with the slug to use when `--repo` contains `/`. Use + `--workspace` for the owner. +- The GitLab CI example now passes `$CI_PROJECT_NAME` instead of + `$CI_PROJECT_PATH`, and the Slack `repos` filter example uses slugs. + +### Deprecated: `--pending-head` + +- `--pending-head` never had any effect: a scan on the default branch becomes + its head scan whether or not the flag is set. It is still accepted so + existing pipelines keep working, but it is hidden from `--help` and logs a + deprecation warning. Use `--default-branch`. + ## 2.11.11 ### Changed: use lowercase Python CLI User-Agent diff --git a/docs/cli-reference.md b/docs/cli-reference.md index eeb2e3df..f412ccc1 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -154,7 +154,7 @@ These are two full-scan uploads, two server-side graphs, and two repository head **Generate GitLab Security Dashboard report:** ```bash socketcli --enable-gitlab-security \ - --repo owner/repo \ + --repo my-repo \ --target-path . ``` @@ -167,7 +167,7 @@ This will: **Save SARIF report to file (e.g. for GitHub Code Scanning, SonarQube, or VS Code):** ```bash socketcli --sarif-file results.sarif \ - --repo owner/repo \ + --repo my-repo \ --target-path . ``` @@ -176,7 +176,7 @@ socketcli --sarif-file results.sarif \ socketcli --enable-json \ --sarif-file results.sarif \ --enable-gitlab-security \ - --repo owner/repo + --repo my-repo ``` This will simultaneously generate: @@ -212,7 +212,7 @@ socketcli [-h] [--api-token API_TOKEN] [--repo REPO] [--workspace WORKSPACE] [-- [--base-scan-id BASE_SCAN_ID | --base-commit-sha BASE_COMMIT_SHA] [--target-path TARGET_PATH] [--sbom-file SBOM_FILE] [--license-file-name LICENSE_FILE_NAME] [--save-submitted-files-list SAVE_SUBMITTED_FILES_LIST] [--save-manifest-tar SAVE_MANIFEST_TAR] [--files FILES] [--sub-path SUB_PATH] [--workspace-name WORKSPACE_NAME] - [--excluded-ecosystems EXCLUDED_ECOSYSTEMS] [--exclude-paths EXCLUDE_PATHS] [--include-dirs INCLUDE_DIRS] [--default-branch] [--pending-head] [--generate-license] [--enable-debug] + [--excluded-ecosystems EXCLUDED_ECOSYSTEMS] [--exclude-paths EXCLUDE_PATHS] [--include-dirs INCLUDE_DIRS] [--default-branch] [--generate-license] [--enable-debug] [--enable-json] [--enable-sarif] [--sarif-file ] [--sarif-scope {diff,full}] [--sarif-grouping {instance,alert}] [--sarif-reachability {all,reachable,potentially,reachable-or-potentially}] [--enable-gitlab-security] [--gitlab-security-file ] [--disable-overview] [--exclude-license-details] [--allow-unverified] [--disable-security-issue] [--ignore-commit-files] [--disable-blocking] [--disable-ignore] [--enable-diff] [--scm SCM] [--timeout TIMEOUT] [--include-module-folders] @@ -235,7 +235,7 @@ If you don't want to provide the Socket API Token every time then you can use th #### Repository | Parameter | Required | Default | Description | |:-----------------|:---------|:--------|:------------------------------------------------------------------------------------------------------------------| -| `--repo` | False | *auto* | Repository name in owner/repo format (auto-detected from git remote) | +| `--repo` | False | *auto* | Repository slug in Socket, without the owner: `my-repo`, not `my-org/my-repo`. Auto-detected from the git remote. A value containing `/` is rejected; use `--workspace` for the owner. | | `--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). When omitted, `--scm github` or `--scm gitlab` implies the matching integration. | @@ -308,7 +308,6 @@ If you don't want to provide the Socket API Token every time then you can use th | Parameter | Required | Default | Description | |:-------------------------|:---------|:--------|:------------------------------------------------------------------------------------------------------| | `--default-branch` | False | *auto* | Make this branch the default branch (auto-detected from git and CI environment when not specified) | -| `--pending-head` | False | *auto* | If true, the new scan will be set as the branch's head scan (automatically synced with default-branch) | | `--include-module-folders` | False | False | If enabled, re-includes the JS/TS module folders (`node_modules`, `bower_components`, `jspm_packages`) in manifest discovery. For other excluded directories, use `--include-dirs`. | #### Output Configuration @@ -551,7 +550,7 @@ export SOCKET_SLACK_BOT_TOKEN="xoxb-your-bot-token-here" Webhook mode (`url_configs`): - `reachability_alerts_only` (boolean, default: false): When `--reach` is enabled, only send reachable vulnerabilities from the selected diff alert set (uses reachability facts when available; otherwise falls back to blocking-status behavior) -- `repos` (array): Only send alerts for specific repositories (e.g., `["owner/repo1", "owner/repo2"]`) +- `repos` (array): Only send alerts for specific repositories, matched against the `--repo` slug without the owner (e.g., `["my-repo1", "my-repo2"]`) - `alert_types` (array): Only send specific alert types (e.g., `["malware", "typosquat"]`) - `severities` (array): Only send alerts with specific severities (e.g., `["high", "critical"]`) @@ -683,7 +682,7 @@ The CLI uses intelligent default branch detection with the following priority: 3. **Git repository analysis**: Compares current branch with repository's default branch 4. **Fallback**: Defaults to `false` if none of the above methods succeed -Both `--default-branch` and `--pending-head` parameters are automatically synchronized to ensure consistent behavior. +A scan on the default branch also becomes that branch's head scan. `--pending-head` is deprecated and has no effect: it is still accepted so existing pipelines keep working, but it logs a warning. Use `--default-branch` instead. ## Who can ignore an alert @@ -763,7 +762,7 @@ Socket CLI can generate reports compatible with GitLab's Security Dashboard, all To generate a GitLab-compatible security report: ```bash -socketcli --enable-gitlab-security --repo owner/repo +socketcli --enable-gitlab-security --repo my-repo ``` This creates a `gl-dependency-scanning-report.json` file following GitLab's Dependency Scanning report schema. @@ -782,7 +781,7 @@ socket_security_scan: script: - socketcli --api-token $SOCKET_API_TOKEN - --repo $CI_PROJECT_PATH + --repo $CI_PROJECT_NAME --branch $CI_COMMIT_REF_NAME --commit-sha $CI_COMMIT_SHA --enable-gitlab-security diff --git a/pyproject.toml b/pyproject.toml index 966fee16..183008bf 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.11.11" +version = "2.11.12" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index 4a07834f..590de206 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,4 +1,4 @@ __author__ = 'socket.dev' -__version__ = '2.11.11' +__version__ = '2.11.12' USER_AGENT_NAME = 'socket-python-cli' USER_AGENT = f'{USER_AGENT_NAME}/{__version__}' diff --git a/socketsecurity/config.py b/socketsecurity/config.py index f5e24316..629f6647 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -448,6 +448,22 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig': logging.error("--workspace-name requires --sub-path to be specified") exit(1) + # The API validates the repo as a slug, which can't contain "/", so an + # owner/repo value would otherwise fail later at repository creation. + if args.repo and "/" in args.repo: + suggested = args.repo.rstrip("/").rsplit("/", 1)[-1] + logging.error( + f"--repo takes the repository slug without its owner, but got '{args.repo}'. " + f"Use --repo {suggested}, and --workspace for the owner if your repositories use one." + ) + exit(1) + + if args.pending_head: + logging.warning( + "--pending-head is deprecated and has no effect. A scan becomes the head scan " + "when it runs on the default branch; use --default-branch to mark it as such." + ) + # argparse only enforces the mutually exclusive group for real CLI args; # this also catches both values arriving via a --config file. if args.base_scan_id and args.base_commit_sha: @@ -527,8 +543,11 @@ def create_argument_parser() -> argparse.ArgumentParser: repo_group = parser.add_argument_group('Repository') repo_group.add_argument( "--repo", - metavar="", - help="Repository name in owner/repo format", + metavar="", + help=( + "Repository slug in Socket, without the owner (e.g. my-repo, not my-org/my-repo). " + "Defaults to the git remote's repository name. Use --workspace for the owner." + ), required=False ) repo_group.add_argument( @@ -743,11 +762,12 @@ def create_argument_parser() -> argparse.ArgumentParser: action="store_true", help=argparse.SUPPRESS ) + # Deprecated no-op, still accepted so existing pipelines don't fail to parse. config_group.add_argument( "--pending-head", dest="pending_head", action="store_true", - help="If true, the new scan will be set as the branch's head scan" + help=argparse.SUPPRESS ) config_group.add_argument( "--ignore-authorization", diff --git a/tests/unit/test_config.py b/tests/unit/test_config.py index 47bb3d88..50047caa 100644 --- a/tests/unit/test_config.py +++ b/tests/unit/test_config.py @@ -168,6 +168,34 @@ def test_config_file_json_sets_defaults(self, tmp_path): assert config.sarif_reachability == "reachable" +class TestRepoAndPendingHeadFlags: + """--repo takes a bare slug, and --pending-head is a deprecated no-op""" + + @pytest.mark.parametrize("repo", ["my-org/my-repo", "group/sub/my-repo", "my-org/my-repo/"]) + def test_repo_with_owner_exits_with_suggestion(self, repo, caplog): + with pytest.raises(SystemExit) as exc_info: + CliConfig.from_args(["--api-token", "test-token", "--repo", repo]) + assert exc_info.value.code == 1 + assert "Use --repo my-repo" in caplog.text + + def test_repo_slug_is_accepted(self): + config = CliConfig.from_args(["--api-token", "test-token", "--repo", "my-repo"]) + assert config.repo == "my-repo" + + @pytest.mark.parametrize("flag", ["--pending-head", "--pending_head"]) + def test_pending_head_is_accepted_with_a_deprecation_warning(self, flag, caplog): + config = CliConfig.from_args(["--api-token", "test-token", flag]) + assert config.pending_head is True + assert "--pending-head is deprecated" in caplog.text + + def test_help_hides_pending_head_and_shows_repo_slug(self, capsys): + with pytest.raises(SystemExit): + CliConfig.from_args(["--help"]) + help_text = capsys.readouterr().out + assert "--pending-head" not in help_text + assert "--repo " in help_text + + class TestBaseScanFlags: """Tests for the --base-scan-id / --base-commit-sha diff baseline overrides.""" diff --git a/tests/unit/test_fossa_compat.py b/tests/unit/test_fossa_compat.py index fadaf034..1a3daf1a 100644 --- a/tests/unit/test_fossa_compat.py +++ b/tests/unit/test_fossa_compat.py @@ -66,7 +66,7 @@ def test_fossa_report_payload_vulnerability_shape_is_stable(): config = CliConfig.from_args([ "--api-token", "test", "--legal-format", "fossa", - "--repo", "owner/repo", + "--repo", "widgets", "--branch", "refs/heads/main", ]) diff = Diff(id="scan-123", report_url="https://socket.dev/report/123") @@ -147,14 +147,14 @@ def test_fossa_report_payload_vulnerability_shape_is_stable(): def test_project_metadata_uses_dollar_revision_separator(): """The composed FOSSA `project.id` is `$`.""" from socketsecurity.fossa_compat import _build_project_metadata - config = CliConfig.from_args(["--api-token", "test", "--legal-format", "fossa", "--repo", "acme/widgets", "--branch", "refs/heads/main"]) + config = CliConfig.from_args(["--api-token", "test", "--legal-format", "fossa", "--repo", "widgets", "--branch", "refs/heads/main"]) diff = Diff(id="scan-abc123", report_url="https://socket.dev/x") project = _build_project_metadata(diff, config) assert project == { "branch": "refs/heads/main", - "id": "acme/widgets$scan-abc123", - "project": "acme/widgets", - "projectId": "acme/widgets", + "id": "widgets$scan-abc123", + "project": "widgets", + "projectId": "widgets", "revision": "scan-abc123", "url": "https://socket.dev/x", } @@ -311,10 +311,10 @@ def test_attribution_payload_top_level_is_5_keys(): def test_attribution_project_has_only_name_and_revision(): """SBOM `project` is the 2-key subset, not the 6-key analyze project.""" - config = CliConfig.from_args(["--api-token", "test", "--legal-format", "fossa", "--repo", "acme/widgets"]) + config = CliConfig.from_args(["--api-token", "test", "--legal-format", "fossa", "--repo", "widgets"]) diff = Diff(id="rev-x") payload = build_fossa_attribution_payload(diff, config) - assert payload["project"] == {"name": "acme/widgets", "revision": "rev-x"} + assert payload["project"] == {"name": "widgets", "revision": "rev-x"} def test_attribution_empty_diff_yields_empty_collections(): diff --git a/uv.lock b/uv.lock index 0a14aad8..f9e7a033 100644 --- a/uv.lock +++ b/uv.lock @@ -1332,7 +1332,7 @@ wheels = [ [[package]] name = "socketsecurity" -version = "2.11.11" +version = "2.11.12" source = { editable = "." } dependencies = [ { name = "beautifulsoup4" }, From 8f38c02501bbc404a8a1f64d0d88dc5dc791f4cb Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Wed, 4 Nov 2026 15:56:24 -0600 Subject: [PATCH 2/3] Shorten the 2.11.12 changelog to what users see The changelog is user-facing, so it states the new behavior and leaves the background to the PR description. --- CHANGELOG.md | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 556d155a..9d33d5d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,22 +2,16 @@ ## 2.11.12 -### Fixed: `--repo` takes a repository slug, not `owner/repo` +### Changed: `--repo` takes a repository slug -- The help text and docs said `--repo` takes `owner/repo`, but the API only - accepts a repository slug, which can't contain `/`. Following the docs made - every scan fail. The help text and docs now describe the slug, and the CLI - exits `1` up front with the slug to use when `--repo` contains `/`. Use - `--workspace` for the owner. -- The GitLab CI example now passes `$CI_PROJECT_NAME` instead of - `$CI_PROJECT_PATH`, and the Slack `repos` filter example uses slugs. +- `--repo` takes the repository slug without its owner (`my-repo`, not + `my-org/my-repo`). A value containing `/` now exits `1` and names the slug to + use. Pass the owner with `--workspace`. ### Deprecated: `--pending-head` -- `--pending-head` never had any effect: a scan on the default branch becomes - its head scan whether or not the flag is set. It is still accepted so - existing pipelines keep working, but it is hidden from `--help` and logs a - deprecation warning. Use `--default-branch`. +- `--pending-head` has no effect and logs a deprecation warning. Use + `--default-branch`. ## 2.11.11 From 800fde7adc7444f5010095b1869729dd08d93f61 Mon Sep 17 00:00:00 2001 From: lelia <2418071+lelia@users.noreply.github.com> Date: Thu, 5 Nov 2026 13:52:47 -0600 Subject: [PATCH 3/3] Build PR comment manifest links from the full repository URL Manifest links in GitHub, GitLab and Bitbucket comments were built as {server}/{config.repo}/blob/..., but config.repo is the Socket repository slug, which has no owner, so every link pointed at a path that doesn't exist. The ignore telemetry's owner field split config.repo on "/" and was always empty for the same reason. Resolve the repository's web URL once, from the CI environment and then the checkout's origin remote, reusing the lookups the pull request URL already does. Manifest links and the telemetry owner use it, and links fall back to the Socket report when it can't be resolved. --- CHANGELOG.md | 6 ++ socketsecurity/config.py | 2 + socketsecurity/core/git_interface.py | 2 + socketsecurity/core/messages.py | 31 ++--------- socketsecurity/core/pull_request.py | 74 +++++++++++++++++++++++-- socketsecurity/socketcli.py | 12 +++- tests/unit/test_messages.py | 23 ++++++++ tests/unit/test_pr_comment_rendering.py | 8 +-- tests/unit/test_pull_request_context.py | 43 ++++++++++++++ 9 files changed, 165 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d33d5d1..4f727e42 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ `my-org/my-repo`). A value containing `/` now exits `1` and names the slug to use. Pass the owner with `--workspace`. +### Fixed: manifest links in pull request comments + +- Manifest links in GitHub, GitLab and Bitbucket comments now include the + repository owner. When the CLI can't determine the owner, they link to the + Socket report instead. + ### Deprecated: `--pending-head` - `--pending-head` has no effect and logs a deprecation warning. Use diff --git a/socketsecurity/config.py b/socketsecurity/config.py index 629f6647..9e8f1580 100644 --- a/socketsecurity/config.py +++ b/socketsecurity/config.py @@ -197,6 +197,8 @@ class CliConfig: sub_paths: List[str] = field(default_factory=list) workspace_name: Optional[str] = None workspace: Optional[str] = None + # Web URL of the scanned repository, resolved at runtime; see resolve_repository_url. + repository_url: Optional[str] = None # Reachability Flags reach: bool = False reach_version: Optional[str] = None diff --git a/socketsecurity/core/git_interface.py b/socketsecurity/core/git_interface.py index 563b41ac..98dc4121 100644 --- a/socketsecurity/core/git_interface.py +++ b/socketsecurity/core/git_interface.py @@ -74,8 +74,10 @@ def __init__(self, path: str, base_commit_sha: str | None = None): log.debug(f"Commit committer: {self.commit.committer.name} <{self.commit.committer.email}>") # Extract repository name from git remote, with fallback to default + self.remote_url: str | None = None try: remote_url = self.repo.remotes.origin.url + self.remote_url = remote_url self.repo_name = remote_url.split('.git')[0].split('/')[-1] log.debug(f"Repository name detected from git remote: {self.repo_name}") except Exception as error: diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 4017a9f8..b2229f65 100644 --- a/socketsecurity/core/messages.py +++ b/socketsecurity/core/messages.py @@ -1,6 +1,5 @@ import json import logging -import os import re import uuid from datetime import datetime, timezone @@ -94,30 +93,12 @@ def get_manifest_file_url(diff: Diff, manifest_path: str, config=None) -> str: # Generate URL based on SCM type using config information # NEVER use diff.diff_url for SCM URLs - those are Socket URLs for "View report" links - if scm_type == "github": - if config and hasattr(config, 'repo') and config.repo: - # Get branch from config, default to main - branch = getattr(config, 'branch', 'main') if hasattr(config, 'branch') and config.branch else 'main' - # Construct GitHub URL from repo info (could be github.com or GitHub Enterprise) - github_server = os.getenv('GITHUB_SERVER_URL', 'https://github.com') - return f"{github_server}/{config.repo}/blob/{branch}/{clean_path}" - - elif scm_type == "gitlab": - if config and hasattr(config, 'repo') and config.repo: - # Get branch from config, default to main - branch = getattr(config, 'branch', 'main') if hasattr(config, 'branch') and config.branch else 'main' - # Construct GitLab URL from repo info (could be gitlab.com or self-hosted GitLab) - gitlab_server = os.getenv('CI_SERVER_URL', 'https://gitlab.com') - return f"{gitlab_server}/{config.repo}/-/blob/{branch}/{clean_path}" - - elif scm_type == "bitbucket": - if config and hasattr(config, 'repo') and config.repo: - # Get branch from config, default to main - branch = getattr(config, 'branch', 'main') if hasattr(config, 'branch') and config.branch else 'main' - # Construct Bitbucket URL from repo info (could be bitbucket.org or Bitbucket Server) - bitbucket_server = os.getenv('BITBUCKET_SERVER_URL', 'https://bitbucket.org') - return f"{bitbucket_server}/{config.repo}/src/{branch}/{clean_path}" - + repository_url = getattr(config, 'repository_url', None) if config else None + if repository_url and scm_type in ("github", "gitlab", "bitbucket"): + branch = getattr(config, 'branch', None) or 'main' + blob_path = {"github": "blob", "gitlab": "-/blob", "bitbucket": "src"}[scm_type] + return f"{repository_url}/{blob_path}/{branch}/{clean_path}" + # Fallback to Socket file view for API or unknown repository types if hasattr(diff, 'report_url') and diff.report_url: # Strip leading slash and URL encode for Socket dashboard diff --git a/socketsecurity/core/pull_request.py b/socketsecurity/core/pull_request.py index 2dcf77f1..1804dd6d 100644 --- a/socketsecurity/core/pull_request.py +++ b/socketsecurity/core/pull_request.py @@ -59,8 +59,10 @@ def _github_number(env: Mapping[str, str]) -> int: return parse_pull_request_number(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")) +def _github_repository_url( + repo: Optional[str], env: Mapping[str, str], remote_url: Optional[str] = None +) -> Optional[str]: + remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO") or remote_url) # 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 @@ -71,13 +73,15 @@ def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Opt or (_http_url(f"https://{remote_host}") if remote_host else None) or "https://github.com" ) - return f"{server}/{repository.strip('/')}/pull/{number}" + return f"{server}/{repository.strip('/')}" -def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]: +def _gitlab_project_url( + repo: Optional[str], env: Mapping[str, str], remote_url: Optional[str] = None +) -> 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")) + remote_host, remote_path = parse_git_remote(env.get("BUILDKITE_REPO") or remote_url) project_path = env.get("CI_PROJECT_PATH") or remote_path or repo server = ( _http_url(env.get("CI_SERVER_URL")) @@ -85,6 +89,66 @@ def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Opt ) if server and project_path and "/" in project_path: project_url = f"{server}/{project_path.strip('/')}" + return project_url + + +def _bitbucket_repository_url( + repo: Optional[str], env: Mapping[str, str], remote_url: Optional[str] = None +) -> Optional[str]: + remote_host, remote_path = parse_git_remote(remote_url) + repository = env.get("BITBUCKET_REPO_FULL_NAME") or remote_path or repo + if not repository or "/" not in repository: + return None + server = ( + _http_url(env.get("BITBUCKET_SERVER_URL")) + or (_http_url(f"https://{remote_host}") if remote_host else None) + or "https://bitbucket.org" + ) + return f"{server}/{repository.strip('/')}" + + +def resolve_repository_url( + scm: Optional[str], + repo: Optional[str] = None, + env: Optional[Mapping[str, str]] = None, + remote_url: Optional[str] = None, +) -> Optional[str]: + """Return the web URL of the scanned repository, such as ``https://github.com/owner/repo``. + + The CI environment wins, then the checkout's ``origin`` remote. ``None`` means the + owner could not be determined: ``config.repo`` is the Socket repository slug, + which has no owner, so a link built from it alone would point nowhere. + """ + environment = env or {} + provider = str(scm or "").lower() + if provider == "github": + return _github_repository_url(repo, environment, remote_url) + if provider == "gitlab": + return _gitlab_project_url(repo, environment, remote_url) + if provider == "bitbucket": + return _bitbucket_repository_url(repo, environment, remote_url) + return None + + +def repository_owner(repository_url: Optional[str]) -> str: + """Return everything before the repository name in ``repository_url``'s path. + + That is the owner on GitHub and Bitbucket, and the full group path on GitLab, + where projects can be nested under subgroups. + """ + if not repository_url: + return "" + path = urlparse(repository_url).path.strip("/") + return path.rsplit("/", 1)[0] if "/" in path else "" + + +def _github_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]: + repository_url = _github_repository_url(repo, env) + return f"{repository_url}/pull/{number}" if repository_url else None + + +def _gitlab_url(number: int, repo: Optional[str], env: Mapping[str, str]) -> Optional[str]: + project_url = _gitlab_project_url(repo, env) return f"{project_url}/-/merge_requests/{number}" if project_url else None diff --git a/socketsecurity/socketcli.py b/socketsecurity/socketcli.py index 2af75074..066fe2df 100644 --- a/socketsecurity/socketcli.py +++ b/socketsecurity/socketcli.py @@ -22,7 +22,9 @@ from socketsecurity.core.messages import Messages from socketsecurity.core.pull_request import ( parse_pull_request_number, + repository_owner, resolve_pull_request_context, + resolve_repository_url, ) from socketsecurity.core.scm_comments import Comments from socketsecurity.core.socket_config import SocketConfig, module_folder_dirs @@ -601,6 +603,12 @@ def main_code(): from socketsecurity.core.scm.gitlab import Gitlab, GitlabConfig gitlab_config = GitlabConfig.from_env() scm = Gitlab(client=client, config=gitlab_config, ignore_authorization=config.ignore_authorization) + config.repository_url = resolve_repository_url( + config.scm, + config.repo, + os.environ, + remote_url=getattr(git_repo, "remote_url", None) if is_repo else None, + ) # Don't override config.default_branch if it was explicitly set via --default-branch flag # Only use SCM detection if --default-branch wasn't provided if scm is not None and not config.default_branch: @@ -806,7 +814,7 @@ def _is_unprocessed(c): "alert_action": "error", "event_sender_created_at": now, "vcs_provider": integration_type, - "owner": config.repo.split("/")[0] if "/" in config.repo else "", + "owner": repository_owner(config.repository_url), "repo": config.repo, "pr_number": pr_number, "ignore_all": ignore_all, @@ -881,7 +889,7 @@ def _is_unprocessed(c): "client_action": "ignore", "event_sender_created_at": now, "vcs_provider": integration_type, - "owner": config.repo.split("/")[0] if "/" in config.repo else "", + "owner": repository_owner(config.repository_url), "repo": config.repo, "pr_number": pr_number, "ignore_all": c_ignore_all, diff --git a/tests/unit/test_messages.py b/tests/unit/test_messages.py index ce6b696e..ddc5ec62 100644 --- a/tests/unit/test_messages.py +++ b/tests/unit/test_messages.py @@ -79,3 +79,26 @@ def test_security_comment_omits_missing_patched_version(): comment = Messages.security_comment_template(diff) assert "Patched version:" not in comment + + +class _LinkConfig: + scm = "github" + branch = "main" + repository_url = "https://github.com/acme/widgets" + + +def test_manifest_link_uses_the_full_repository_url(): + url = Messages.get_manifest_file_url(Diff(), "app/package.json", _LinkConfig()) + + assert url == "https://github.com/acme/widgets/blob/main/app/package.json" + + +def test_manifest_link_falls_back_to_the_report_without_a_repository_url(): + """A bare slug has no owner, so the link goes to the Socket report instead.""" + config = _LinkConfig() + config.repository_url = None + diff = Diff(report_url="https://socket.dev/report/123") + + url = Messages.get_manifest_file_url(diff, "app/package.json", config) + + assert url == "https://socket.dev/report/123?tab=files&file=app%2Fpackage.json" diff --git a/tests/unit/test_pr_comment_rendering.py b/tests/unit/test_pr_comment_rendering.py index 717fa68f..78c2a567 100644 --- a/tests/unit/test_pr_comment_rendering.py +++ b/tests/unit/test_pr_comment_rendering.py @@ -376,11 +376,11 @@ def test_returns_empty_when_absent(self): class _RepoConfig(_FakeConfig): """A config that reaches the branch which embeds the path verbatim. - Without repo/branch, get_manifest_file_url returns "" or a percent-encoded - Socket link, and the path never lands in the comment -- so a test using the - bare config asserts nothing. + Without repository_url/branch, get_manifest_file_url returns "" or a + percent-encoded Socket link, and the path never lands in the comment -- so a + test using the bare config asserts nothing. """ - repo: str = "acme/widgets" + repository_url: str = "https://github.com/acme/widgets" branch: str = "main" diff --git a/tests/unit/test_pull_request_context.py b/tests/unit/test_pull_request_context.py index 0a59e4d4..b1a6dd80 100644 --- a/tests/unit/test_pull_request_context.py +++ b/tests/unit/test_pull_request_context.py @@ -2,7 +2,9 @@ from socketsecurity.core.pull_request import ( parse_pull_request_number, + repository_owner, resolve_pull_request_context, + resolve_repository_url, ) @@ -291,3 +293,44 @@ def test_bare_repository_name_alone_yields_no_url(): assert context.number == 42 assert context.url is None + + +@pytest.mark.parametrize( + ("scm", "env", "remote_url", "expected"), + [ + ("github", {"GITHUB_REPOSITORY": "acme/widgets"}, None, "https://github.com/acme/widgets"), + ( + "github", + {"GITHUB_REPOSITORY": "acme/widgets", "GITHUB_SERVER_URL": "https://ghe.example.com"}, + None, + "https://ghe.example.com/acme/widgets", + ), + ( + "gitlab", + {"CI_PROJECT_URL": "https://gitlab.example.com/group/sub/widgets"}, + None, + "https://gitlab.example.com/group/sub/widgets", + ), + ("bitbucket", {"BITBUCKET_REPO_FULL_NAME": "acme/widgets"}, None, "https://bitbucket.org/acme/widgets"), + # Outside CI, the origin remote supplies the host and path; credentials are dropped. + ("github", {}, "https://x-token:secret@github.com/acme/widgets.git", "https://github.com/acme/widgets"), + ("gitlab", {}, "git@gitlab.com:group/widgets.git", "https://gitlab.com/group/widgets"), + # A bare slug has no owner, so there is no repository URL to build. + ("github", {}, None, None), + ("api", {"GITHUB_REPOSITORY": "acme/widgets"}, None, None), + ], +) +def test_resolve_repository_url(scm, env, remote_url, expected): + assert resolve_repository_url(scm, "widgets", env, remote_url=remote_url) == expected + + +@pytest.mark.parametrize( + ("url", "owner"), + [ + ("https://github.com/acme/widgets", "acme"), + ("https://gitlab.com/group/sub/widgets", "group/sub"), + (None, ""), + ], +) +def test_repository_owner(url, owner): + assert repository_owner(url) == owner