diff --git a/.github/actions/setup-docker/action.yml b/.github/actions/setup-docker/action.yml index 58239ff6..c5ce0fa3 100644 --- a/.github/actions/setup-docker/action.yml +++ b/.github/actions/setup-docker/action.yml @@ -19,9 +19,9 @@ inputs: runs: using: "composite" steps: - - uses: docker/setup-qemu-action@96fe6ef7f33517b61c61be40b68a1882f3264fb8 # v4.2.0 + - uses: docker/setup-qemu-action@1f40c72289eff860ee54a304f1438e3cff362e0a # v4.3.0 if: inputs.enable-qemu == 'true' - - uses: docker/setup-buildx-action@bb05f3f5519dd87d3ba754cc423b652a5edd6d2c # v4.2.0 + - uses: docker/setup-buildx-action@37fe631027851001ddb9b187196cc803df7f5f0e # v4.3.0 - uses: docker/login-action@dbcb813823bdd20940b903addbd779551569679f # v4.6.0 with: username: ${{ inputs.dockerhub-username }} diff --git a/.github/workflows/python-tests.yml b/.github/workflows/python-tests.yml index 34717226..f05b27c9 100644 --- a/.github/workflows/python-tests.yml +++ b/.github/workflows/python-tests.yml @@ -67,6 +67,26 @@ jobs: uv export --no-hashes --no-emit-project --format requirements-txt > /tmp/req-audit.txt uvx pip-audit --strict --progress-spinner off --disable-pip --no-deps -r /tmp/req-audit.txt + ruff: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + persist-credentials: false + - name: ๐Ÿ setup python + uses: actions/setup-python@5fda3b95a4ea91299a34e894583c3862153e4b97 # v7.0.0 + with: + python-version: ${{ env.PYTHON_VERSION }} + - name: ๐Ÿ› ๏ธ install deps + run: | + python -m pip install --upgrade pip + pip install uv + uv sync --extra dev + - name: ๐Ÿงน run ruff + run: uv run ruff check + unsupported-python-install: runs-on: ubuntu-latest timeout-minutes: 10 diff --git a/.github/workflows/version-check.yml b/.github/workflows/version-check.yml index d2ff77ad..292e2980 100644 --- a/.github/workflows/version-check.yml +++ b/.github/workflows/version-check.yml @@ -6,6 +6,8 @@ on: - 'socketsecurity/**' - 'pyproject.toml' - 'uv.lock' + # Included so a change to the check itself is exercised by its own PR. + - '.github/workflows/version-check.yml' permissions: contents: read @@ -42,16 +44,37 @@ jobs: export PR_VERSION export MAIN_VERSION - # Compare against both main and latest published PyPI release. + # Compare against the latest published PyPI release. python3 <<'PY' import json import os + import tomllib import urllib.request from packaging import version pr_ver = version.parse(os.environ["PR_VERSION"]) main_ver = version.parse(os.environ["MAIN_VERSION"]) + with open("pyproject.toml", "rb") as fh: + pyproject_ver = version.parse(tomllib.load(fh)["project"]["version"]) + + # The version is two hand-maintained literals with nothing deriving one + # from the other: pyproject.toml is what actually gets published, and + # socketsecurity/__init__.py is what the CLI reports as its User-Agent. + # Every comparison below reads only __init__.py, so bumping that alone + # would pass this job and then publish under the old number -- caught + # late, by twine rejecting an existing file, after the merge. Require + # the two to agree before comparing anything. (uv.lock carries a third + # copy, but uv derives it and `uv lock --locked` in python-tests + # already fails when it drifts.) + if pr_ver != pyproject_ver: + print( + f"โŒ Version mismatch inside the PR: pyproject.toml is " + f"{pyproject_ver}, socketsecurity/__init__.py is {pr_ver}. " + f"Bump both." + ) + raise SystemExit(1) + with urllib.request.urlopen("https://pypi.org/pypi/socketsecurity/json") as response: pypi_data = json.load(response) @@ -62,19 +85,37 @@ jobs: published_versions.append(parsed) pypi_ver = max(published_versions) if published_versions else version.parse("0.0.0") - required_floor = max(main_ver, pypi_ver) - if pr_ver <= required_floor: + # The only hard requirement is that the version is ahead of what is + # actually released. Treating main's version as a second floor breaks + # the legitimate case where several PRs share one unreleased release: + # the first bumps main to the new version and the rest ride it without + # bumping again, which is what keeps them under a single changelog + # header. Main is therefore only a floor when this PR moves the + # version -- a change to it must go forwards, never backwards. + if pr_ver <= pypi_ver: print( - f"โŒ Version must be greater than main and PyPI! " - f"Main: {main_ver}, PyPI: {pypi_ver}, PR: {pr_ver}" + f"โŒ Version {pr_ver} is already published on PyPI " + f"(latest release: {pypi_ver}). Bump it." + ) + raise SystemExit(1) + + if pr_ver < main_ver: + print( + f"โŒ Version moves backwards: main is {main_ver}, PR is {pr_ver}." ) raise SystemExit(1) - print( - f"โœ… Version properly incremented. " - f"Main: {main_ver}, PyPI: {pypi_ver}, PR: {pr_ver}" - ) + if pr_ver == main_ver: + print( + f"โœ… Riding main's unreleased {pr_ver} " + f"(latest PyPI release: {pypi_ver})." + ) + else: + print( + f"โœ… Version properly incremented. " + f"Main: {main_ver}, PyPI: {pypi_ver}, PR: {pr_ver}" + ) PY - name: Require uv.lock update when pyproject changes diff --git a/CHANGELOG.md b/CHANGELOG.md index 21150734..5f5d78a4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,247 @@ # Changelog +## 2.9.3 + +### Changed: bump pinned @coana-tech/cli to 15.10.45 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.44` to + `15.10.45`. See the + [reachability analysis changelog](https://docs.socket.dev/docs/reachability-analysis-changelog) + for engine changes. + +## 2.9.2 + +### Changed: bump pinned @coana-tech/cli to 15.10.44 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.43` to + `15.10.44`. See the + [reachability analysis changelog](https://docs.socket.dev/docs/reachability-analysis-changelog) + for engine changes. + +## 2.9.1 + +### Changed: bump pinned @coana-tech/cli to 15.10.43 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.41` to + `15.10.43`. See the + [reachability analysis changelog](https://docs.socket.dev/docs/reachability-analysis-changelog) + for engine changes. + +## 2.9.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. +- A `--pr-number` value that is not a positive integer is now normalized to `0` + before the GitHub adapter reads it, so Buildkite's `false` on a branch build no + longer makes that build look like a pull request event. + +### Changed: GitHub and GitLab branch pipelines create full scans + +- With `--scm github` or `--scm gitlab`, only pull request and merge request + events create diff scans. Every other pipeline, including default-branch + pushes, creates a full scan. The detected event type is authoritative: + `--enable-diff` and `--ignore-commit-files` no longer opt an SCM branch run + into comparison mode. +- Those runs no longer set a blocking exit code. A full scan has no baseline, so + it cannot distinguish newly introduced alerts from pre-existing ones; the CLI + now behaves as if `--disable-blocking` was supplied, matching how it already + treats a run with no supported manifest files. Pull request and merge request + pipelines are unaffected and still block. +- `--generate-license` and `--legal-format fossa` fetch the package list on this + path, so attribution files generated from a branch pipeline are complete rather + than empty. +- Console-only full scans link to the Socket report and state that findings were + not fetched for console output instead of presenting an empty local alert list + as "No issues found." +- License enrichment keeps the package namespace in PURL requests and response + matching, so scoped npm packages and namespaced Maven packages receive their + license details. + +### Changed: `@SocketSecurity ignore` requires write access + +- An ignore command suppresses a security alert, but the CLI honored one from any + commenter, including a drive-by comment from someone with no access to the + repository. Commands are now accepted only from an author with write access. +- On GitHub this is read from the effective repository permission and cached per + commenter for the run. Write, maintain, or admin access is required; relationship + labels such as `MEMBER` and `COLLABORATOR` are not treated as permissions. +- A 404 from GitHub's collaborator-permission endpoint is treated as a definitive + denial rather than an unreadable permission, so the default `enforce` policy does + not honor ignore commands from users outside the repository. +- GitLab notes carry no equivalent field, so project membership is read once per + run (only when an ignore command is present) and Developer or above is required. + If that lookup cannot be answered โ€” a `CI_JOB_TOKEN` generally cannot read the + members API โ€” the command is still honored and a warning names the author, so + enabling this does not silently break pipelines that relied on ignore commands. + Use a `GITLAB_TOKEN` with API read access to get enforcement. +- A rejected command is logged and is also absent from the ignore telemetry, which + records what was acted on. No acknowledgement reaction is added to a comment that + was not honored. +- `--ignore-authorization` selects the policy: `enforce` (default) requires write + access and honors the command with a warning where the provider cannot report it, + `strict` rejects it in that case instead, and `off` performs no check. + +### Fixed: GitLab authentication fallback never ran + +- When a GitLab token's type cannot be inferred from its shape, the CLI guesses + between Bearer and PRIVATE-TOKEN and retries once under the other scheme on a + 401. That retry never happened: the retry caught `requests.exceptions.HTTPError`, + but the HTTP client translates every request error into `APIFailure` first, so a + misclassified token failed the run instead of falling back. +- API failures raised by the CLI's HTTP client now carry their HTTP status code. + Without it a 401 was indistinguishable from any other failure, and + `is_transient_error` could not classify one either. +- The CLI's `APIFailure` now subclasses the SDK exception of the same name. They + were independent types, so an `except APIFailure` importing the SDK's โ€” which is + what every handler in `socketsecurity.core` does โ€” did not catch a failure raised + by the HTTP client. + +### 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. + A leading npm scope is no longer mistaken for an ecosystem, so + `ignore @types/node@*` no longer also ignores the package named `node`. +- Ignore telemetry uses the same package matcher as alert suppression, so legacy + bare-name commands generate an event for the alert they suppress. +- Dependency overviews preserve added, updated, removed, and replaced package + classifications instead of presenting updates as new dependencies. Added and + updated rows keep their diff badge; removed and replaced, which have no + published badge, use a text label. +- Shared security comment copy no longer describes GitLab merge request output + as Socket for GitHub. +- Updating a security comment in the legacy table format no longer raises on a + malformed row. Each row was unpacked through four consecutive splits with no + bounds checks, so a cell carrying an extra `|`, a package cell that is not a + markdown link, or a name with no version ended the run before it reported + status โ€” and a scoped package name in Socket's own table was enough to trigger + it. Rows are now parsed defensively, and a row that cannot be read keeps its + alert reported. Ignore commands for a scoped package are accepted there in both + the ecosystem-qualified and bare forms. +- Server URLs read from `GITHUB_SERVER_URL` and `CI_SERVER_URL` are validated as + http(s) URLs before being composed into a diff scan's external link, matching + the check already applied to the other repository URLs read from CI. +- Repository-derived values are escaped before they are rendered into a pull + request or merge request comment. Manifest paths and sources are file paths from + the scanned repository, and alert text comes from the API; neither is markup the + CLI authored, so both are now escaped at the point they are interpolated. The + alert markers can no longer be terminated early by a package name. Slack, Jira + and console output are unchanged, since none of them render HTML. +## 2.8.3 + +### Fixed: GitLab report serialization and workspace baselines + +- Full-scan package identities and Socket links now preserve namespaced packages + when the SDK returns enum-backed ecosystem values. +- Namespaced package links separate the namespace from the name instead of + concatenating them, so Maven links no longer fuse groupId and artifactId into a + single unresolvable path segment. A namespaced package whose namespace is + missing now logs a warning rather than emitting a broken link silently. +- GitLab dependency-scanning reports emit CVE and GHSA identifiers from current + API fields while remaining compatible with legacy CVE data. +- GitLab report findings record the manifest they came from when the package's + introducing chain is unavailable, instead of reporting the location as + `unknown`, and report whether a dependency is direct from the package record + rather than inferring it from a dependency-path string that is never produced. +- `--base-commit-sha` degrades to the nearest scanned ancestor of the requested + commit instead of failing the run, and logs which commit was used and how far + back it is. Squash merges, rebases, and multi-commit pushes all leave a merge + base unscanned even when default-branch scanning is configured correctly. The + lookup follows paginated scan history and the run still fails when no scanned + ancestor is reachable or the exact-commit lookup itself fails. +- Implicit diff baselines are selected from the same workspace, scan type, + repository, and default branch, including when no workspace is supplied. A + baseline lookup that fails is reported as an API error instead of resolving to + an empty baseline, and temporary scans are skipped when selecting one. +## 2.8.2 + +### Changed: bump pinned @coana-tech/cli to 15.10.41 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.40` to + `15.10.41`. See the + [reachability analysis changelog](https://docs.socket.dev/docs/reachability-analysis-changelog) + for engine changes. + +## 2.8.1 + +### Changed: bump pinned @coana-tech/cli to 15.10.40 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.39` to + `15.10.40`. See the + [reachability analysis changelog](https://docs.socket.dev/docs/reachability-analysis-changelog) + for engine changes. + +## 2.8.0 + +### Changed: improve monorepo scan diagnostics and guidance + +- Added aggregate scan configuration, manifest-count, baseline-selection, and + fallback diagnostics without listing submitted manifest paths. +- Clarified monorepo scan scoping, workspace flags, CI path filters, and timeout + behavior, with a changed-workspace GitHub Actions example. + +### Changed: bump socketdev to 3.6.0 + +- Bumped the pinned SDK (`socketdev`) from `3.5.0` to `3.6.0`. Its package-type + enum gained ten members โ€” `alpm`, `chrome`, `clawhub`, `edge-extension`, + `firefox-extension`, `qpkg`, `socket`, `swid`, `vscode` and + `vscode-extension` โ€” so artifacts of those types are now reported under their + own type instead of falling back to `unknown`. + +### Fixed: apply configured exit codes to API failures + +- Full-scan and streamed-diff API failures now use the configured infrastructure + error exit code instead of the security-finding exit code. + +### Fixed: mid-severity findings were dropped from the Slack summary + +- The Slack reachability formatter keyed every severity lookup on `medium`, + but the API sends `middle`. A mid-severity finding therefore missed all of + them at once: it was not counted, so the summary always read `Medium: 0`; it + was excluded from `total_findings`, which can drive the "and N more" count + negative; and it sorted at the default order of 4, below `low`, so it was the + first thing truncated when the Slack block limit was reached. +- Severity is now normalized to one spelling when an alert is read, matching + how the GitLab and PR-comment paths already handle both forms. The findings + themselves were always listed; only the counts, ordering and truncation were + wrong. + +## 2.7.2 + +### Changed: bump pinned @coana-tech/cli to 15.10.39 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.36` to + `15.10.39`. See the [Coana changelogs](https://docs.coana.tech/changelogs) for + engine changes. + +## 2.7.1 + +### Changed: bump pinned @coana-tech/cli to 15.10.36 + +- Bumped the pinned reachability engine (`@coana-tech/cli`) from `15.10.32` to + `15.10.36`. See the [Coana changelogs](https://docs.coana.tech/changelogs) for + engine changes. + ## 2.7.0 ### Fixed: unreadable reachability facts no longer report a blocking package diff --git a/README.md b/README.md index cda9a407..5221daf1 100644 --- a/README.md +++ b/README.md @@ -44,21 +44,22 @@ socketcli --enable-gitlab-security --gitlab-security-file gl-dependency-scanning ### PR scan diffed against the merge base -By default, PR scans are diffed against the repository's latest head scan. To diff against -the exact commit your PR branched from instead, pass the merge base as the baseline: +By default, PR scans are diffed against the repository's latest matching head scan. To +prefer the commit your PR branched from as the baseline, pass the merge base: ```bash BASE_SHA=$(git merge-base origin/main HEAD) socketcli --pr-number 123 --base-commit-sha "$BASE_SHA" ``` -> **Requirement:** `--base-commit-sha` only works if Socket already has a full scan for that -> exact commit. In practice this means your CI must run `socketcli` on **every commit that -> lands on your default branch** โ€” not just some of them. If merges can land without a scan -> (skipped/canceled builds, `[skip ci]`, path-filtered pipelines), the PR scan will fail with -> exit code 3 rather than silently diff against the wrong baseline. See +> The CLI uses the exact commit's newest matching full scan when one exists. Otherwise, it +> searches up to 100 first-parent commits in the local checkout and uses the nearest scanned +> ancestor, with a warning that the diff is wider than the merge base. Run `socketcli` +> regularly on your default branch and ensure PR checkouts contain enough history for that +> walk. The run fails with the configured API-error exit code only when no scanned ancestor +> is reachable (or when the scan lookup itself fails). See > [`docs/cli-reference.md`](https://github.com/SocketDev/socket-python-cli/blob/main/docs/cli-reference.md) -> for the full requirements and a backfill pattern that makes PR jobs self-sufficient. +> for the full behavior and an optional exact-baseline backfill pattern. A specific full scan ID also works: `--base-scan-id `. @@ -229,6 +230,10 @@ value โ€” e.g. a Buildkite code, or `0` to swallow infra errors. Exit `3` is a Socket convention, not an industry standard. +This mapping applies to errors the CLI receives and handles. An external process +supervisor (for example GNU `timeout`) can terminate the CLI before it handles an +error, so the supervisor's exit status (commonly 124 or 137) takes precedence. + ### How these options interact The two flags that affect exit codes can cancel each other out, so the order of diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 061d18ea..253c27ca 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 @@ -71,16 +99,268 @@ Equivalent JSON: SOCKET_SECURITY_API_TOKEN: ${{ secrets.SOCKET_SECURITY_API_TOKEN }} ``` +#### GitHub Actions: scan changed monorepo workspaces independently + +GitHub Actions `paths` filters only decide whether a workflow starts. They do not +change `socketcli` discovery or upload scope. For a merge gate, it is usually safer +to start a small selector job on every PR update, then create one scan job per +affected logical workspace. This also avoids a required check remaining pending +when GitHub skips the entire workflow because of a top-level path filter. + +This pattern produces one dashboard entry per logical workspace, which is what +gives each component its own alerts, baseline, and policy. It is also the layout +that grows the dashboard's repository list. See +[Choosing a scan layout](cli-reference.md#choosing-a-scan-layout) for when that +trade-off is worth making. + +Define a repository variable named `SOCKET_MONOREPO_WORKSPACES_JSON`. Its value is +an array with one stable workspace name, one or more scan roots, and the path globs +that should select that workspace. Fill these placeholders with the repository's +real layout. A workspace definition selects directory roots; shared root manifests, +lockfiles, and cross-directory path dependencies outside those roots are not included +automatically. + +```json +[ + { + "name": "", + "sub_paths": [""], + "watch_globs": [""] + } +] +``` + +Each `sub_paths` value must be a directory, not an individual manifest or lockfile. +Using `.` includes the entire target path. Do not use this changed-workspace pattern +until the directory boundaries preserve every shared input needed to resolve each +logical graph. If root workspace metadata governs most or all of the repository, a +smaller coverage-preserving split may not be representable with `--sub-path` alone. + +Also define `SOCKETCLI_VERSION` as the exact package version validated for the +workflow. The workflow below logs that version, uses full Git history for reliable +base/head selection, creates one matrix job (and therefore one graph and baseline) +per selected workspace, and fails closed on CLI/API/timeout failures. It uses API +SCM mode plus `--enable-diff` because parallel `--scm github` jobs can race while +updating the same PR comments; the matrix checks and report links are the gate. + +```yaml +name: Socket Security + +on: + pull_request: + types: [opened, synchronize, reopened] + push: + branches: [main] + +permissions: + contents: read + +jobs: + select-workspaces: + runs-on: ubuntu-latest + outputs: + count: ${{ steps.select.outputs.count }} + matrix: ${{ steps.select.outputs.matrix }} + steps: + - uses: actions/checkout@v5 + with: + fetch-depth: 0 + persist-credentials: false + + - id: select + name: Select changed workspaces + env: + WORKSPACES_JSON: ${{ vars.SOCKET_MONOREPO_WORKSPACES_JSON }} + BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.before }} + HEAD_SHA: ${{ github.event.pull_request.head.sha || github.sha }} + shell: bash + run: | + python - <<'PY' + import fnmatch + import json + import os + import re + import subprocess + + workspaces = json.loads(os.environ["WORKSPACES_JSON"]) + if not isinstance(workspaces, list): + raise SystemExit("SOCKET_MONOREPO_WORKSPACES_JSON must be a JSON array") + + base = os.environ["BASE_SHA"] + head = os.environ["HEAD_SHA"] + if not base or set(base) == {"0"}: + base = subprocess.check_output( + ["git", "rev-parse", f"{head}^"], text=True + ).strip() + changed_output = subprocess.check_output( + ["git", "diff", "--name-only", "-z", base, head] + ) + changed = [ + item.decode("utf-8", "surrogateescape") + for item in changed_output.split(b"\0") + if item + ] + + selected = [] + for workspace in workspaces: + name = workspace.get("name", "") + sub_paths = workspace.get("sub_paths") or [] + watch_globs = workspace.get("watch_globs") or [] + if not re.fullmatch(r"[A-Za-z0-9._-]+", name): + raise SystemExit(f"Invalid workspace name: {name!r}") + if not sub_paths or any( + not isinstance(path, str) + or path.startswith("/") + or ".." in path.split("/") + for path in sub_paths + ): + raise SystemExit(f"Invalid sub_paths for workspace {name!r}") + if not watch_globs: + watch_globs = [ + pattern + for path in sub_paths + for pattern in ( + ["*"] + if path.strip("/") in ("", ".") + else [path.rstrip("/"), f"{path.rstrip('/')}/*"] + ) + ] + if any( + fnmatch.fnmatchcase(path, pattern) + for path in changed + for pattern in watch_globs + ): + selected.append({"name": name, "sub_paths": sub_paths}) + + matrix = json.dumps({"include": selected}, separators=(",", ":")) + with open(os.environ["GITHUB_OUTPUT"], "a", encoding="utf-8") as output: + output.write(f"count={len(selected)}\n") + output.write(f"matrix={matrix}\n") + PY + + scan-workspace: + needs: select-workspaces + if: needs.select-workspaces.outputs.count != '0' + timeout-minutes: 20 + strategy: + fail-fast: false + matrix: ${{ fromJSON(needs.select-workspaces.outputs.matrix) }} + name: Socket scan (${{ matrix.name }}) + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v5 + with: + fetch-depth: 0 + persist-credentials: false + + - uses: actions/setup-python@v6 + with: + python-version: '3.12' + + - name: Install pinned Socket CLI + env: + SOCKETCLI_VERSION: ${{ vars.SOCKETCLI_VERSION }} + run: | + python -m pip install "socketsecurity==$SOCKETCLI_VERSION" + socketcli --version + + - name: Scan workspace + env: + SOCKET_SECURITY_API_KEY: ${{ secrets.SOCKET_SECURITY_API_KEY }} + PR_NUMBER: ${{ github.event.pull_request.number || 0 }} + WORKSPACE_NAME: ${{ matrix.name }} + SUB_PATHS_JSON: ${{ toJSON(matrix.sub_paths) }} + shell: bash + run: | + set +e + args=( + --target-path "$GITHUB_WORKSPACE" + --workspace-name "$WORKSPACE_NAME" + --enable-diff + --pr-number "$PR_NUMBER" + --exit-code-on-api-error 3 + --report-link-file socket-report-link.txt + --summary-file socket-summary.txt + ) + while IFS= read -r sub_path; do + args+=(--sub-path "$sub_path") + done < <(jq -r '.[]' <<<"$SUB_PATHS_JSON") + + socketcli "${args[@]}" 2>&1 | tee socket-output.log + code=${PIPESTATUS[0]} + + { + echo "## Socket scan: $WORKSPACE_NAME" + if [ -s socket-report-link.txt ]; then + echo "[View the report]($(cat socket-report-link.txt))" + fi + if [ -s socket-summary.txt ]; then + echo '```' + cat socket-summary.txt + echo '```' + fi + } >> "$GITHUB_STEP_SUMMARY" + + exit "$code" + + socket-security: + if: always() + needs: [select-workspaces, scan-workspace] + runs-on: ubuntu-latest + steps: + - name: Enforce matrix result + env: + SELECT_RESULT: ${{ needs.select-workspaces.result }} + SCAN_RESULT: ${{ needs.scan-workspace.result }} + run: | + test "$SELECT_RESULT" = success + [[ "$SCAN_RESULT" = success || "$SCAN_RESULT" = skipped ]] +``` + +Each configuration object may intentionally contain several `sub_paths` when +those directories are one logical dependency graph. To split backend resolution, +use separate objects with different `name` values. Add `--workspace ` only +when the Socket organization requires API workspace association; it is not a scan +scope control. Use `--save-submitted-files-list` in a non-required canary to verify +the exact manifests selected before adopting workspace-level scans as a merge gate. + +The job has an explicit 20-minute total budget. Tune that value from observed +workspace-level latency after the split; a five-minute cap can still be too close +to a slow request plus local startup. The CLI's `--timeout` is different: it +defaults to 1,200 seconds **per API request**. If an operator adds GNU `timeout`, +that process supervisor can terminate the CLI before it maps an error through +`--exit-code-on-api-error`; without `--preserve-status`, GNU reports 124 after its +initial timeout signal or 137 if `SIGKILL` is involved. + ### 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 +368,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) @@ -114,11 +395,14 @@ generator rather than a static YAML file: BASE_SHA=$(git merge-base "origin/${TARGET}" HEAD) ``` -- **Emit the backfill step conditionally from the generator.** The generator is the - natural place for the "does a baseline scan exist?" check +- **If an exact baseline is required, emit a backfill step conditionally from the + generator.** The generator is the natural place for the "does an exact baseline + scan exist?" check (`GET /orgs/{org}/full-scans?repo=&commit_hash=$BASE_SHA&per_page=1`): only emit the baseline-scan step when it returns nothing. The emitted pipeline then shows - in the UI whether a backfill will run. + in the UI whether a backfill will run. Without a backfill, the CLI automatically + uses the nearest scanned first-parent ancestor within 100 commits and warns that + the diff is wider. - **Keep the backfill inside one command step.** The checkout-base โ†’ scan โ†’ checkout-PR sequence must not be split across steps โ€” steps can land on different @@ -127,17 +411,18 @@ generator rather than a static YAML file: checkout: `git worktree add /tmp/socket-base "$BASE_SHA"` then `socketcli --target-path /tmp/socket-base --branch "$TARGET" --disable-blocking`. -- **Soft-fail infra errors, not findings.** A missing baseline (or any API error) - exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1. +- **Soft-fail infra errors, not findings.** No reachable scanned ancestor (or any API + error) exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1. [`soft_fail: [{exit_status: 3}]`](https://buildkite.com/docs/pipelines/configure/step-types/command-step) on the PR scan step keeps infra errors from blocking merges while security findings still do. - **["Cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds) - on the default branch is the main source of baseline gaps.** Canceled builds never - scan their commit, so merge-base lookups for PRs based on those commits fail. The - conditional backfill step above is the remedy; there is no per-step exemption from - build cancellation in Buildkite. If you need strict scan-once semantics for + on the default branch is a common source of exact-baseline gaps.** Canceled builds + never scan their commit, so these PRs fall back to an older scanned ancestor. Use + the conditional backfill step above when an exact merge-base comparison is required; + there is no per-step exemption from build cancellation in Buildkite. If you need + strict scan-once semantics for concurrent backfills of the same merge base, serialize the backfill step with a [concurrency group](https://buildkite.com/docs/pipelines/configure/workflows/controlling-concurrency) keyed on the merge-base SHA. @@ -152,6 +437,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 +459,69 @@ pipelines: - socketcli --config .socketcli.toml --target-path . ``` +## Scan type by pipeline + +With `--scm github` or `--scm gitlab`, the detected event decides the scan type: + +| Event | Scan | Blocks the build | +|:------|:-----|:-----------------| +| Pull request / merge request | Diff scan against the repository's baseline | Yes, on newly introduced alerts | +| Any other pipeline, including default-branch pushes | Full scan | No | + +A full scan has no baseline, so it cannot tell a newly introduced alert from one +that was already there. Rather than block on a number that would mean something +different depending on which output format was enabled, those runs behave as if +`--disable-blocking` was supplied and report through the Dashboard instead. This +matches how the CLI already treats a run with no supported manifest files. + +The event type is authoritative once `--scm` is set: `--enable-diff` and +`--ignore-commit-files` do not turn a branch pipeline into a comparison. To diff +a branch build, drop `--scm` and use `--enable-diff` with `--integration`, which +runs the comparison without the PR comment adapter. + +`--generate-license` and `--legal-format fossa` work on both paths; a full scan +fetches the package list for them. + +## 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. Any value that +is not a positive integer, including Buildkite's `false`, means no pull request. + +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 +538,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 6c94daf3..c5d4f637 100644 --- a/docs/cli-reference.md +++ b/docs/cli-reference.md @@ -53,18 +53,67 @@ Pre-configured workflow files are in [`../workflows/`](../workflows/). > **Note:** If you're looking to associate a scan with a named Socket workspace (e.g. because your repo is identified as `org/repo`), see the [`--workspace` flag](#repository) instead. The `--workspace-name` flag described in this section is an unrelated monorepo feature. -The Socket CLI supports scanning specific workspaces within monorepo structures while preserving git context from the repository root. This is useful for organizations that maintain multiple applications or services in a single repository. +The Socket CLI supports scanning selected directories within a monorepo while preserving git context from the repository root. Scan scope is controlled by `--target-path` and `--sub-path`; CI workflow path filters and the CLI's changed-file detection do not narrow the manifests uploaded after a scan starts. ### Key Features -- **Multiple Sub-paths**: Specify multiple `--sub-path` options to scan different directories within your monorepo -- **Combined Workspace**: All sub-paths are scanned together as a single workspace in Socket +- **Target path**: Supplies repository/Git context and is the discovery root when no `--sub-path` is present +- **Multiple Sub-paths**: Restrict discovery to those directories, but combine every repeated `--sub-path` into one upload and one server-side dependency graph - **Git Context Preserved**: Repository metadata (commits, branches, etc.) comes from the main target-path -- **Workspace Naming**: Use `--workspace-name` to differentiate scans from different parts of your monorepo +- **Workspace Naming**: Use a stable, unique `--workspace-name` for each independently scanned logical workspace; it suffixes the repository slug and therefore gives that workspace its own repository head/baseline + +`--workspace` is different: it sends Socket organization workspace context with the full-scan API request. It does not narrow client-side filesystem discovery, split the upload into independent scans, or change the repository suffix. Backend policy/routing for that workspace remains server-owned. + +> **Performance consequence:** If the goal is smaller independently resolvable graphs, run one CLI invocation per logical workspace, with a distinct `--workspace-name`. Adding several unrelated directories to one command with repeated `--sub-path` flags still asks the backend to resolve one combined graph. + +Normal scan logs include the effective repository and Socket workspace context, +repository-relative discovery roots, aggregate manifest count, and selected baseline. +Individual manifest paths remain opt-in through `--save-submitted-files-list`. + +### Choosing a scan layout + +`--sub-path` and `--workspace-name` support two layouts, and picking between them +is a trade-off rather than a preference. There is no third option today. + +**One combined scan** โ€” a single invocation, no `--workspace-name`, with +`--target-path` at the repository root or several repeated `--sub-path` values +sharing one workspace name: + +- One dashboard entry for the repository, named after the repository +- One server-side dependency graph covering everything that was uploaded +- Alerts are **not** broken out by component, so a finding does not tell you which + part of the monorepo introduced it +- Transitive findings can surface without a clear owning component, because the + combined graph has no component boundaries to attribute them to + +**One scan per component** โ€” a separate invocation per component, each with its +own `--sub-path` and a distinct `--workspace-name`: + +- Per-component alerts, baselines, and policy +- Each component gets its own dependency graph, which is also the faster option + (see the performance note above) +- But `--workspace-name` suffixes the repository slug, so *N* components produce + *N* separate entries in the dashboard's repository list + +The second point is what makes this a real choice: a monorepo with a dozen or more +independently scanned components produces a dozen or more repository entries, which +gets hard to navigate as the list grows. A single consolidated entry that still +preserves per-component attribution is a known request and is not available today. + +Rules of thumb: + +- **Few components, or components that share a release cycle** โ€” use one combined + scan and accept coarser attribution. +- **Many components, or components with different owners or policies** โ€” use + per-component scans and accept the extra dashboard entries. Per-component policy + is only possible in this layout. +- **Components that are genuinely one application** โ€” group them under a single + `--workspace-name`, as in the first example below. Grouping is per logical + application, not per directory. ### Usage Examples -**Scan multiple frontend and backend workspaces:** +**Scan several directories that belong to one logical application:** ```bash socketcli --target-path /path/to/monorepo \ --sub-path frontend \ @@ -89,6 +138,19 @@ This will: - Create a repository in Socket named like `my-repo-mobile-web` - Preserve git context (commits, branch info) from the repository root +**Create independent frontend and backend scans:** +```bash +socketcli --target-path /path/to/monorepo \ + --sub-path frontend \ + --workspace-name frontend + +socketcli --target-path /path/to/monorepo \ + --sub-path backend \ + --workspace-name backend +``` + +These are two full-scan uploads, two server-side graphs, and two repository head/baseline sequences. In CI they can run as separate matrix jobs. See [GitHub Actions: scan changed monorepo workspaces independently](ci-cd.md#github-actions-scan-changed-monorepo-workspaces-independently). + **Generate GitLab Security Dashboard report:** ```bash socketcli --enable-gitlab-security \ @@ -138,6 +200,7 @@ This will simultaneously generate: - Both `--sub-path` and `--workspace-name` must be specified together - `--sub-path` can be used multiple times to include multiple directories +- Repeated `--sub-path` values are combined into one scan; they do not create independent workspace scans - All specified sub-paths must exist within the target-path ## Usage @@ -156,7 +219,7 @@ socketcli [-h] [--api-token API_TOKEN] [--repo REPO] [--workspace WORKSPACE] [-- [--reach] [--reach-version REACH_VERSION] [--reach-analysis-timeout REACH_ANALYSIS_TIMEOUT] [--reach-analysis-memory-limit REACH_ANALYSIS_MEMORY_LIMIT] [--reach-concurrency REACH_CONCURRENCY] [--reach-ecosystems REACH_ECOSYSTEMS] [--reach-min-severity ] [--reach-skip-cache] [--reach-disable-analytics] [--reach-enable-analysis-splitting] [--reach-detailed-analysis-log-file] - [--reach-lazy-mode] [--reach-use-only-pregenerated-sboms] [--reach-debug] [--reach-disable-external-tool-checks] + [--reach-use-only-pregenerated-sboms] [--reach-debug] [--reach-disable-external-tool-checks] [--reach-output-file REACH_OUTPUT_FILE] [--only-facts-file] [--version] ```` @@ -175,7 +238,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,28 +252,26 @@ 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` | -| `--base-commit-sha`| False | | Commit SHA to diff against, overriding the repository's head scan as the baseline. The most recent full scan for that commit is used; the CLI errors (exit code 3, or `--exit-code-on-api-error`) if no scan exists for it. Mutually exclusive with `--base-scan-id` | +| `--base-commit-sha`| False | | Commit SHA to prefer as the diff baseline, overriding the repository's head scan. The CLI uses its most recent matching full scan or the nearest scanned first-parent ancestor within 100 local commits. It errors (exit code 3, or `--exit-code-on-api-error`) if no scanned ancestor is reachable. Mutually exclusive with `--base-scan-id` | -> **Diffing against the merge base** โ€” by default, PR scans are diffed against the repository's *latest* head scan, which may include newer default-branch commits than your PR branched from. To diff against the exact commit your PR is based on, compute the merge base and pass it as the baseline: +> **Diffing against the merge base** โ€” by default, PR scans are diffed against the repository's latest matching head scan, which may include newer default-branch commits than your PR branched from. To prefer the commit your PR is based on, compute the merge base and pass it as the baseline: > > ```shell > BASE_SHA=$(git merge-base origin/main HEAD) > socketcli --pr-number 123 --base-commit-sha "$BASE_SHA" > ``` > -> **Requirement: a full scan must already exist for the merge-base commit.** `--base-commit-sha` does not create a scan of that commit; it looks up an existing one. That lookup only succeeds if your CI runs `socketcli` on **every commit that lands on your default branch** โ€” every merge and direct push, not just periodic or latest-only scans. Common ways commits slip through without a scan: +> `--base-commit-sha` does not create a scan of that commit. The CLI first looks for the newest non-temporary scan matching the repository, workspace, scan type, and exact commit. If the exact commit was not scanned, it walks up to 100 first-parent commits from that SHA in the local checkout and uses the nearest matching scanned ancestor. It logs a warning with the selected commit and distance because this produces a wider diff than the merge base. > -> - CI settings that cancel or skip intermediate builds when newer commits land (e.g. Buildkite's ["cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds)) -> - `[skip ci]` commits, path-filtered pipelines, or failed/canceled scan steps -> - merge-base commits that predate your Socket rollout +> Run `socketcli` regularly on the default branch so recent ancestors have scans. PR checkouts must also retain the merge base and enough first-parent history; shallow clones can shorten the search. Gaps are expected when CI cancels intermediate builds, commits use `[skip ci]`, pipelines are path-filtered, or the merge base predates your Socket rollout. > -> If no scan exists for the commit, the CLI **fails** (exit code 3, or your `--exit-code-on-api-error` value; exit 0 with `--disable-blocking`) instead of silently falling back to the head scan โ€” a wrong baseline would misreport which alerts the PR introduces. Don't adopt this flag without default-branch scan coverage in place; you'll fail PR builds on lookup misses. +> If no scanned ancestor is reachable within the local 100-commit walk, the CLI **fails** (exit code 3, or your `--exit-code-on-api-error` value; exit 0 with `--disable-blocking`) instead of silently falling back to the repository head. API or permission failures also fail rather than being treated as a missing exact scan. > -> **Backfill pattern** โ€” if your default-branch coverage has gaps, the PR job can create the missing baseline itself before scanning: +> **Optional exact-baseline backfill** โ€” if the wider ancestor fallback is not acceptable, the PR job can create the missing exact baseline before scanning: > > ```shell > BASE_SHA=$(git merge-base origin/main HEAD) @@ -222,7 +283,7 @@ If you don't want to provide the Socket API Token every time then you can use th > socketcli --pr-number 123 --base-commit-sha "$BASE_SHA" > ``` > -> Run the baseline step with `--disable-blocking` (findings on the default branch must not fail the PR job) and an explicit `--branch`, since branch auto-detection is unreliable at a detached HEAD. +> Run the baseline step with `--disable-blocking` (findings on the default branch must not fail the PR job) and an explicit `--branch`, since branch auto-detection is unreliable at a detached HEAD. Without this step, the CLI automatically uses the nearest scanned ancestor. > > Buildkite users with dynamically generated pipelines: see [Merge-base baselines in Buildkite](ci-cd.md#merge-base-baselines-in-buildkite-dynamic-pipelines) for generation-time vs. step-time guidance. @@ -275,7 +336,7 @@ If you don't want to provide the Socket API Token every time then you can use th | Parameter | Required | Default | Description | |:---------------------------------|:---------|:--------|:---------------------------------------------------------------------------------------------------------------------------| | `--reach` | False | False | Enable reachability analysis to identify which vulnerable functions are actually called by your code. Creates a full application reachability scan (`scan_type=socket_tier1`). | -| `--reach-version` | False | 15.10.32 | Version of @coana-tech/cli to use. Defaults to the pinned version that ships with this CLI release, so the engine only changes when you upgrade the Socket CLI. Pass `latest` to always use the newest published version (opt-in auto-update), or an explicit version (e.g. `1.2.3`) to pin it. | +| `--reach-version` | False | 15.10.45 | Version of @coana-tech/cli to use. Defaults to the pinned version that ships with this CLI release, so the engine only changes when you upgrade the Socket CLI. Pass `latest` to always use the newest published version (opt-in auto-update), or an explicit version (e.g. `1.2.3`) to pin it. | | `--reach-analysis-timeout` | False | 10m | Timeout for each reachability analysis run, e.g. `90s`, `10m` or `1h`. Omitted by default, so coana applies its own default (`10m`). Alias: `--reach-timeout` | | `--reach-analysis-memory-limit` | False | 8GB | Memory limit for each reachability analysis run, e.g. `512MB` or `8GB`. Omitted by default, so coana applies its own default (`8GB`). Alias: `--reach-memory-limit` | | `--reach-concurrency` | False | 1 | Control parallel analysis execution (must be >= 1). Omitted by default, so coana applies its own default. | @@ -286,7 +347,6 @@ If you don't want to provide the Socket API Token every time then you can use th | `--reach-disable-analytics` | False | False | Disable analytics collection during reachability analysis | | `--reach-enable-analysis-splitting` | False | False | Enable analysis splitting/bucketing (a legacy performance feature). Splitting is disabled by default. | | `--reach-detailed-analysis-log-file` | False | False | Write a detailed analysis log file; its path is printed to stdout | -| `--reach-lazy-mode` | False | False | Enable lazy mode (experimental performance feature) | | `--reach-use-only-pregenerated-sboms` | False | False | Build the scan only from pre-generated CycloneDX (CDX) and SPDX files in your project (requires --reach) | | `--reach-debug` | False | False | Enable coana debug output (`--debug`) for the analysis, independent of the global `--enable-debug` | | `--reach-disable-external-tool-checks` | False | False | Disable coana's external tool availability checks (passes `--disable-external-tool-checks`) | @@ -369,11 +429,12 @@ The launcher can be tuned via the `SOCKET_CLI_COANA_LAUNCHER` environment variab |:-------------------------|:---------|:--------|:----------------------------------------------------------------------| | `--ignore-commit-files` | False | False | Ignore commit files | | `--disable-blocking` | False | False | Non-blocking CI mode: the CLI always exits **0**, even when blocking alerts are present (including with `--strict-blocking`). Also exits 0 on uncaught runtime errors and Socket API failures, so the job is treated as successful while findings and errors are still logged. Takes precedence over `--strict-blocking`. | -| `--disable-ignore` | False | False | Disable support for `@SocketSecurity ignore` commands in PR comments. When set, alerts cannot be suppressed via comments and ignore instructions are hidden from comment output. | +| `--disable-ignore` | False | False | Disable support for `@SocketSecurity ignore` commands in PR comments. When set, alerts cannot be suppressed via comments and ignore instructions are hidden from comment output. See [Who can ignore an alert](#who-can-ignore-an-alert). | +| `--ignore-authorization` | False | enforce | Who may suppress alerts with `@SocketSecurity ignore`. `enforce` requires write access and honors the command with a warning when the provider cannot report it; `strict` rejects it in that case; `off` honors any commenter. See [Who can ignore an alert](#who-can-ignore-an-alert). | | `--strict-blocking` | False | False | Fail on ANY security policy violations (blocking severity), not just new ones. Only works in diff mode. See [Strict Blocking Mode](#strict-blocking-mode) for details. | | `--enable-diff` | False | False | Enable diff mode even when using `--integration api` (forces diff mode without SCM integration) | | `--scm` | False | api | Source control management type | -| `--timeout` | False | | Timeout in seconds for API requests | +| `--timeout` | False | 1200 | Timeout in seconds for each API request. This is not a total CLI runtime limit and does not limit local discovery, Git, or reachability analysis. | #### Plugins @@ -628,6 +689,37 @@ The CLI uses intelligent default branch detection with the following priority: Both `--default-branch` and `--pending-head` parameters are automatically synchronized to ensure consistent behavior. +## Who can ignore an alert + +`@SocketSecurity ignore /@` and +`@SocketSecurity ignore-all` suppress security findings, so the CLI honors them +only from a commenter with write access to the repository. A command from anyone +else is skipped, logged with the author's name, and the alerts it named stay +reported. `--disable-ignore` turns the feature off entirely. + +| Provider | How access is determined | If it cannot be determined | +|:---------|:-------------------------|:---------------------------| +| GitHub | Effective repository permission, read once per commenter per run. Write, maintain, or admin access is honored. | The command is honored and a warning is logged. | +| GitLab | Project membership, read once per run when an ignore command is present. Developer (30) or above is honored. | The command is honored and a warning is logged. | + +The GitHub check needs a token that can read repository metadata. GitLab notes +carry no permission field, so that check needs a `GITLAB_TOKEN` that can read +`GET /projects/:id/members/all`. A `CI_JOB_TOKEN` generally cannot. + +`--ignore-authorization` decides what happens when access cannot be determined: + +| Value | Verified write access | Access cannot be determined | +|:------|:----------------------|:----------------------------| +| `enforce` (default) | Honored | Honored, with a warning naming the author | +| `strict` | Honored | Rejected | +| `off` | Honored | Honored, no check performed | + +`enforce` closes the hole wherever the provider can answer, without breaking a +pipeline whose token cannot read membership. `strict` closes it everywhere, at the +cost of failing those pipelines. `off` restores the prior behavior and should be +paired with `--disable-ignore` unless you specifically need comment-driven ignores +from unverified authors. + ## GitLab Token Configuration GitLab token/auth behavior and CI examples are documented in [`ci-cd.md`](ci-cd.md). diff --git a/pyproject.toml b/pyproject.toml index 293bbd13..82d3d566 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -6,7 +6,7 @@ build-backend = "hatchling.build" [project] name = "socketsecurity" -version = "2.7.0" +version = "2.9.3" requires-python = ">= 3.11" license = {"file" = "LICENSE"} dependencies = [ @@ -16,11 +16,11 @@ dependencies = [ "GitPython==3.1.59", "packaging==26.3", "python-dotenv==1.2.3", - "socketdev==3.5.0", + "socketdev==3.6.0", "beautifulsoup4==4.15.0", "markdown==3.10.3", "brotli==1.2.0; platform_python_implementation == 'CPython'", - "brotlicffi==1.2.0.1; platform_python_implementation != 'CPython'", + "brotlicffi==1.2.0.2; platform_python_implementation != 'CPython'", ] readme = "README.md" description = "Socket Security CLI for CI/CD" @@ -47,9 +47,9 @@ test = [ "pytest-watch==4.2.0" ] dev = [ - "ruff==0.16.4", + "ruff==0.16.6", "twine==7.0.0", # for building - "uv==0.12.5", # for dependency management + "uv==0.12.9", # for dependency management "pre-commit==4.6.2", "hatch==1.18.0" ] diff --git a/socketsecurity/__init__.py b/socketsecurity/__init__.py index d72ecc6e..7e432574 100644 --- a/socketsecurity/__init__.py +++ b/socketsecurity/__init__.py @@ -1,3 +1,3 @@ __author__ = 'socket.dev' -__version__ = '2.7.0' +__version__ = '2.9.3' USER_AGENT = f'SocketPythonCLI/{__version__}' diff --git a/socketsecurity/config.py b/socketsecurity/config.py index 26542447..bb2ad009 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 = "./" @@ -141,6 +144,7 @@ class CliConfig: ignore_commit_files: bool = False disable_blocking: bool = False disable_ignore: bool = False + ignore_authorization: str = "enforce" # Tri-state log-upload preference: True = --upload-logs, False = --no-upload-logs, # None = neither (server-side override decides). upload_logs: Optional[bool] = None @@ -176,7 +180,7 @@ class CliConfig: reach_disable_analysis_splitting: bool = False # Deprecated, kept for backwards compatibility reach_enable_analysis_splitting: bool = False reach_detailed_analysis_log_file: bool = False - reach_lazy_mode: bool = False + reach_lazy_mode: bool = False # Deprecated, kept for backwards compatibility reach_ecosystems: Optional[List[str]] = None reach_exclude_paths: Optional[List[str]] = None reach_skip_cache: bool = False @@ -217,6 +221,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 +275,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), @@ -290,9 +306,10 @@ def from_args(cls, args_list: Optional[List[str]] = None) -> 'CliConfig': 'ignore_commit_files': args.ignore_commit_files, 'disable_blocking': args.disable_blocking, 'disable_ignore': args.disable_ignore, + 'ignore_authorization': args.ignore_authorization, '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 +534,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 +554,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", @@ -585,9 +610,9 @@ def create_argument_parser() -> argparse.ArgumentParser: metavar="", default=None, help="Commit SHA to diff the new scan against, overriding the repository's head " - "scan as the baseline. The most recent full scan matching this commit (e.g. " - "the merge base from 'git merge-base origin/main HEAD') is used; the CLI " - "errors if no scan exists for it. Mutually exclusive with --base-scan-id." + "scan as the baseline. The CLI uses the most recent matching full scan, or " + "the nearest scanned first-parent ancestor within 100 local commits when " + "the commit itself was not scanned. Mutually exclusive with --base-scan-id." ) # Path and File options @@ -701,6 +726,19 @@ def create_argument_parser() -> argparse.ArgumentParser: action="store_true", help="If true, the new scan will be set as the branch's head scan" ) + config_group.add_argument( + "--ignore-authorization", + dest="ignore_authorization", + choices=["enforce", "strict", "off"], + default="enforce", + help=( + "Who may suppress alerts with @SocketSecurity ignore comments. " + "'enforce' (default) requires write access, and honors the command with " + "a warning when the provider cannot report the commenter's access. " + "'strict' rejects the command in that case instead. " + "'off' honors a command from any commenter." + ) + ) config_group.add_argument( "--pending_head", dest="pending_head", @@ -1091,7 +1129,7 @@ def create_argument_parser() -> argparse.ArgumentParser: "--reach-lazy-mode", dest="reach_lazy_mode", action="store_true", - help="Enable lazy mode for reachability analysis. This is an experimental feature for improving performance" + help=argparse.SUPPRESS # Deprecated, kept for backwards compatibility (no-op) ) reachability_group.add_argument( "--reach-output-file", diff --git a/socketsecurity/core/__init__.py b/socketsecurity/core/__init__.py index a5305bee..fe49bceb 100644 --- a/socketsecurity/core/__init__.py +++ b/socketsecurity/core/__init__.py @@ -14,10 +14,11 @@ import time from dataclasses import asdict from pathlib import PurePath -from typing import TYPE_CHECKING, Dict, List, NamedTuple, Optional, Set, Tuple +from typing import TYPE_CHECKING, Dict, Iterator, List, NamedTuple, Optional, Set, Tuple if TYPE_CHECKING: from socketsecurity.config import CliConfig +from git import Repo from socketdev import socketdev from socketdev.exceptions import APIFailure from socketdev.fullscans import DiffArtifacts, FullScanParams, SocketArtifact @@ -51,6 +52,17 @@ _HUMANIZE_BOUNDARY = re.compile(r"(?<=[a-z0-9])(?=[A-Z])|(?<=[A-Z])(?=[A-Z][a-z])") +# How many full scans to request when resolving a diff baseline. The newest scan is +# usually the one we want, but temporary scans have to be skipped (see +# Core.newest_persisted_scan_id), so a single result is not enough. +SCAN_LOOKUP_PAGE_SIZE = 10 + +# Bounds on the search for a scanned ancestor when the requested baseline commit has +# no full scan of its own. Full-scan listing pages are matched against bounded local +# history, so the local walk cannot grow without limit. +ANCESTOR_SCAN_LOOKUP_LIMIT = 100 +ANCESTOR_WALK_MAX_DEPTH = 100 + # Reachability facts-file upload compression. # # The Socket full-scan endpoint transparently brotli-decompresses any multipart part @@ -1154,6 +1166,30 @@ def create_full_scan(self, files: List[str], params: FullScanParams, base_paths: return full_scan + @staticmethod + def _log_scan_configuration( + paths: List[str], + params: FullScanParams, + files: List[str], + manifest_source: str, + base_paths: Optional[List[str]] = None, + ) -> None: + """Log aggregate scan inputs without exposing submitted manifest paths.""" + base_path = base_paths[0] if base_paths else (paths[0] if paths else ".") + absolute_base_path = os.path.abspath(base_path) + relative_roots = [ + os.path.relpath(os.path.abspath(path), absolute_base_path).replace("\\", "/") + for path in paths + ] or ["."] + log.info( + "Scan configuration: " + f"repo={json.dumps(getattr(params, 'repo', None), default=str)} " + f"workspace={json.dumps(getattr(params, 'workspace', None), default=str)} " + f"scan_type={json.dumps(getattr(params, 'scan_type', None), default=str)} " + f"roots={json.dumps(relative_roots, separators=(',', ':'))} " + f"manifests={len(files)} manifest_source={manifest_source}" + ) + def create_full_scan_with_report_url( self, paths: List[str], @@ -1196,6 +1232,14 @@ def create_full_scan_with_report_url( for path in paths: files = self.find_files(path) all_files.extend(files) + + self._log_scan_configuration( + paths, + params, + all_files, + manifest_source="provided" if explicit_files is not None else "discovered", + base_paths=base_paths, + ) # Save submitted files list if requested if save_files_list_path and all_files: @@ -1247,6 +1291,7 @@ def create_full_scan_with_report_url( diff.report_url = f"{base_socket}/{self.config.org_slug}/sbom/{new_full_scan.id}" diff.diff_url = diff.report_url diff.id = new_full_scan.id + diff.is_full_scan = True needs_alerts = ( self.cli_config is not None @@ -1256,30 +1301,43 @@ def create_full_scan_with_report_url( or self.cli_config.enable_sarif ) ) + # --generate-license (and --legal-format fossa, which it gates) enumerates + # diff.packages rather than the alert list, so a full scan has to carry the + # package map even when no alert-bearing output format is enabled. Without + # this, an SCM branch pipeline writes an attribution file with zero packages. + # Keep in sync with _requires_unchanged_artifacts, which lists the same + # consumers for the comparison path. + needs_license_artifacts = ( + self.cli_config is not None and self.cli_config.generate_license + ) - if needs_alerts: - log.info("Output format requires alerts, fetching SBOM data for full scan") + if needs_alerts or needs_license_artifacts: + log.info("Output format requires SBOM data, fetching it for the full scan") sbom_start = time.time() sbom_artifacts_dict = self.get_sbom_data(new_full_scan.id) sbom_artifacts = self.get_sbom_data_list(sbom_artifacts_dict) packages = self._create_packages_dict_without_license_text(sbom_artifacts) + if needs_license_artifacts: + packages = self._add_license_details(packages) diff.packages = packages - all_alerts_collection: Dict[str, List[Issue]] = {} - for package_id, package in packages.items(): - self.add_package_alerts_to_collection( - package=package, - alerts_collection=all_alerts_collection, - packages=packages - ) + if needs_alerts: + all_alerts_collection: Dict[str, List[Issue]] = {} + for package_id, package in packages.items(): + self.add_package_alerts_to_collection( + package=package, + alerts_collection=all_alerts_collection, + packages=packages + ) - consolidated: Set[str] = set() - for alert_key, alerts in all_alerts_collection.items(): - for alert in alerts: - alert_str = f"{alert.purl},{alert.type}" - if (alert.error or alert.warn) and alert_str not in consolidated: - diff.new_alerts.append(alert) - consolidated.add(alert_str) + consolidated: Set[str] = set() + for alert_key, alerts in all_alerts_collection.items(): + for alert in alerts: + alert_str = f"{alert.purl},{alert.type}" + if (alert.error or alert.warn) and alert_str not in consolidated: + diff.new_alerts.append(alert) + consolidated.add(alert_str) + diff.alerts_fetched = True sbom_end = time.time() log.info( @@ -1291,6 +1349,29 @@ def create_full_scan_with_report_url( return diff + def _add_license_details(self, packages: dict[str, Package]) -> dict[str, Package]: + """Populate licenseAttrib/licenseDetails on a full scan's package map. + + get_license_text_via_purl keys off ``ecosystem/name@version`` because that is + what the PURL endpoint echoes back, while a full scan's package map is keyed + by artifact id. Build a purl-keyed view over the same Package objects so the + enrichment lands on the map the caller keeps. + """ + batch_size = self.cli_config.max_purl_batch_size if self.cli_config else 5000 + packages_by_purl = {} + for package in packages.values(): + qualified_name = package.name + if package.namespace: + qualified_name = f"{package.namespace.strip('/')}/{qualified_name}" + packages_by_purl[ + f"{package.type}/{qualified_name}@{package.version}" + ] = package + self.get_license_text_via_purl( + packages_by_purl, + batch_size=batch_size, + ) + return packages + def get_full_scan(self, full_scan_id: str) -> FullScan: """ Get a FullScan object for an existing full scan including sbom_artifacts and packages. @@ -1431,19 +1512,229 @@ def get_repo_info(self, repo_slug: str, default_branch: str = "socket-default-br return response.data - def get_head_scan_for_repo(self, repo_slug: str) -> str: + def get_head_scan_for_repo( + self, + repo_slug: str, + workspace: Optional[str] = None, + scan_type: Optional[str] = None, + ) -> Optional[str]: """ Gets the head scan ID for a repository. + Without a workspace or scan type this is the repository's head scan pointer. + That pointer tracks a single scan for the whole repository rather than one per + workspace or scan type, so scoped runs instead take the newest matching scan + on the default branch. + Args: repo_slug: Repository slug to get head scan for + workspace: Socket workspace the scan belongs to, if any + scan_type: Socket scan type to match, if any Returns: Head scan ID if it exists, None otherwise + + Raises: + APIFailure: If the scoped scan lookup fails. A failed lookup must not + be reported as "no baseline": the caller answers that by creating an + empty baseline scan, which reports every dependency in the repository + as newly added. """ repo_info = self.get_repo_info(repo_slug) + if workspace or scan_type: + query_params = { + "repo": repo_slug, + "branch": repo_info.default_branch, + "sort": "created_at", + "direction": "desc", + "per_page": SCAN_LOOKUP_PAGE_SIZE, + } + if workspace: + query_params["workspace"] = workspace + if scan_type: + query_params["scan_type"] = Core.query_param_value(scan_type) + for results in self._full_scan_result_pages( + query_params, + f"Failed to list matching full scans for repo {repo_slug}", + ): + scan_id = Core.newest_persisted_scan_id(results) + if scan_id: + return scan_id + return None return repo_info.head_full_scan_id if repo_info.head_full_scan_id else None + @staticmethod + def query_param_value(value): + """ + Unwraps an enum member so it survives URL encoding. + + The SDK types several query params as str-backed enums (ScanType, + IntegrationType). urlencode calls str(), which renders a (str, Enum) member + as "ScanType.SOCKET_TIER1" -- a filter value the API does not recognize. + """ + return getattr(value, "value", value) + + @staticmethod + def newest_persisted_scan_id(results: List[dict]) -> Optional[str]: + """ + Returns the newest scan ID from a full scan listing, skipping temporary scans. + + create_new_diff creates an empty ``tmp`` scan when a repository has no baseline + yet, and that scan inherits the branch and commit of the run that created it. + If the real scan then fails, the empty scan is left behind as the newest scan + for that branch/commit; selecting it as a baseline would report every + dependency as newly added. + + Args: + results: Full scan listing results, newest first + + Returns: + Newest non-temporary scan ID, or None if the listing has none + """ + for result in results or []: + if not isinstance(result, dict) or result.get("tmp"): + continue + scan_id = result.get("id") + if scan_id: + return scan_id + return None + + def _full_scan_result_pages( + self, + query_params: dict, + failure_message: str, + ) -> Iterator[List[dict]]: + """Yields successful full-scan listing pages and rejects failed lookups.""" + request_params = dict(query_params) + seen_pages = {str(request_params.get("page", 1))} + per_page = int(request_params.get("per_page", 30)) + + while True: + response = self.sdk.fullscans.get(self.config.org_slug, request_params) + results = response.get("results") if isinstance(response, dict) else None + if results is None: + # The SDK logs and returns {} for any non-200, so a present results + # key is the only signal that the request itself succeeded. + raise APIFailure(failure_message) + yield results + + next_page = response.get("nextPage") + # The API has historically returned nextPage=1 for a short final page. + if len(results) < per_page or next_page in (None, 0, "0", False, ""): + return + + page_key = str(next_page) + if page_key in seen_pages: + raise APIFailure( + f"{failure_message}: full-scan pagination repeated page {next_page}" + ) + seen_pages.add(page_key) + request_params = {**query_params, "page": next_page} + + def first_parent_commits(self, start_commit_sha: str, max_count: int) -> List[str]: + """ + Lists a commit and its first-parent ancestors, newest first. + + Follows only first parents so a merge commit contributes the branch's own + history rather than everything merged into it. A shallow checkout simply + yields fewer commits, which narrows the search rather than failing it. + + Args: + start_commit_sha: Commit to walk back from, included in the result + max_count: Maximum number of commits to return + + Returns: + Commit SHAs, newest first. Empty when the repository or commit is + unavailable locally. + """ + target_path = self.cli_config.target_path if self.cli_config else None + if not target_path: + return [] + try: + repo = Repo(target_path) + output = repo.git.rev_list( + "--first-parent", + f"--max-count={max_count}", + start_commit_sha, + ) + except Exception as error: + log.debug(f"Unable to walk history back from {start_commit_sha}: {error}") + return [] + return [line.strip() for line in output.splitlines() if line.strip()] + + def find_baseline_scan_for_ancestor( + self, + repo_slug: str, + commit_sha: str, + workspace: Optional[str] = None, + scan_type: Optional[str] = None, + ) -> Tuple[Optional[str], Optional[str], int]: + """ + Finds the nearest ancestor of a commit that does have a full scan. + + Used when --base-commit-sha names a commit that was never scanned. Squash + merges and rebases rewrite commits, and a multi-commit push produces one scan + for the tip, so a merge base can be unscanned even when default-branch + scanning is configured correctly. Diffing against a slightly older ancestor + is a wider diff; failing outright is no diff at all. + + Scan listing pages are matched against local first-parent history. All pages + are considered because scans from other branches and reruns can fill newer + pages without covering the nearest candidate ancestors. + + Args: + repo_slug: Repository slug the scan belongs to + commit_sha: Commit that has no full scan of its own + workspace: Socket workspace the scan belongs to, if any + scan_type: Socket scan type to match, if any + + Returns: + (scan_id, ancestor_commit_sha, commits_back), or (None, None, 0) when no + scanned ancestor is reachable. + """ + ancestors = self.first_parent_commits(commit_sha, ANCESTOR_WALK_MAX_DEPTH) + if not ancestors: + return None, None, 0 + + query_params = { + "repo": repo_slug, + "sort": "created_at", + "direction": "desc", + "per_page": ANCESTOR_SCAN_LOOKUP_LIMIT, + } + if workspace: + query_params["workspace"] = workspace + if scan_type: + query_params["scan_type"] = Core.query_param_value(scan_type) + + scans_by_commit = {} + ancestor_set = set(ancestors) + for results in self._full_scan_result_pages( + query_params, + f"Failed to list ancestor full scans for repo {repo_slug}", + ): + for result in results: + if not isinstance(result, dict) or result.get("tmp"): + continue + result_commit = result.get("commit_hash") + scan_id = result.get("id") + # Newest first, so the first scan seen for a commit is the one to keep. + if ( + result_commit in ancestor_set + and scan_id + and result_commit not in scans_by_commit + ): + scans_by_commit[result_commit] = scan_id + + if not scans_by_commit: + return None, None, 0 + + for distance, ancestor in enumerate(ancestors): + scan_id = scans_by_commit.get(ancestor) + if scan_id: + return scan_id, ancestor, distance + return None, None, 0 + def get_full_scan_id_by_commit( self, repo_slug: str, @@ -1472,38 +1763,42 @@ def get_full_scan_id_by_commit( "commit_hash": commit_sha, "sort": "created_at", "direction": "desc", - "per_page": 1, + "per_page": SCAN_LOOKUP_PAGE_SIZE, } if workspace: query_params["workspace"] = workspace if scan_type: - query_params["scan_type"] = scan_type + query_params["scan_type"] = Core.query_param_value(scan_type) - response = self.sdk.fullscans.get( - self.config.org_slug, - query_params, - ) - results = response.get("results") if isinstance(response, dict) else None - if not results: - return None - return results[0].get("id") + for results in self._full_scan_result_pages( + query_params, + f"Failed to list full scans for commit {commit_sha} in repo {repo_slug}", + ): + scan_id = Core.newest_persisted_scan_id(results) + if scan_id: + return scan_id + return None def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: """ Resolves the baseline full scan ID to diff a new scan against. Priority: --base-scan-id (used verbatim), then --base-commit-sha (newest - full scan for that commit), then the repository's current head scan. A - --base-commit-sha with no matching full scan is a hard error rather than a - silent fallback to the head scan, because diffing against the wrong - baseline silently misreports which alerts a PR introduces. + full scan for that commit, or its nearest scanned first-parent ancestor), + then the repository's current matching head scan. A --base-commit-sha with + no reachable scanned ancestor is a hard error rather than a silent fallback + to the head scan, because diffing against the wrong baseline silently + misreports which alerts a PR introduces. Returns: Full scan ID to use as the diff baseline, or None when the repository has no head scan yet (caller creates an empty baseline scan). """ if self.cli_config and self.cli_config.base_scan_id: - log.info(f"Using full scan {self.cli_config.base_scan_id} as diff baseline (--base-scan-id)") + log.info( + "Baseline selected: source=explicit-scan " + f"scan_id={json.dumps(self.cli_config.base_scan_id)}" + ) return self.cli_config.base_scan_id if self.cli_config and self.cli_config.base_commit_sha: @@ -1514,32 +1809,75 @@ def resolve_base_full_scan_id(self, params: FullScanParams) -> Optional[str]: workspace=params.workspace, scan_type=params.scan_type, ) + baseline_source = "explicit-commit" + baseline_commit = commit_sha if scan_id is None: - log.error( - f"No full scan found for commit {commit_sha} in repo {params.repo} " - "(--base-commit-sha). Ensure a scan was created for that commit " - "(e.g. the CLI runs on default-branch pushes), or pass " - "--base-scan-id instead." + scan_id, ancestor_sha, commits_back = self.find_baseline_scan_for_ancestor( + params.repo, + commit_sha, + workspace=params.workspace, + scan_type=params.scan_type, ) - if self.cli_config.disable_blocking: - sys.exit(0) - sys.exit(self.cli_config.exit_code_on_api_error) - log.info(f"Using full scan {scan_id} (commit {commit_sha}) as diff baseline (--base-commit-sha)") + if scan_id: + baseline_source = "explicit-commit-ancestor" + baseline_commit = ancestor_sha + log.warning( + f"No full scan for commit {commit_sha} (--base-commit-sha). " + f"Diffing against its nearest scanned ancestor {ancestor_sha}, " + f"{commits_back} commit(s) earlier, so the diff is wider than " + "the merge base." + ) + else: + log.error( + f"No full scan found for commit {commit_sha} in repo {params.repo} " + "(--base-commit-sha), and no scanned ancestor within " + f"{ANCESTOR_WALK_MAX_DEPTH} commits of it. Ensure a scan was " + "created for that commit (e.g. the CLI runs on default-branch " + "pushes), or pass --base-scan-id instead." + ) + if self.cli_config.disable_blocking: + sys.exit(0) + sys.exit(self.cli_config.exit_code_on_api_error) + log.info( + f"Baseline selected: source={baseline_source} " + f"scan_id={json.dumps(scan_id)} commit={json.dumps(baseline_commit)}" + ) return scan_id try: - return self.get_head_scan_for_repo(params.repo) + scan_id = self.get_head_scan_for_repo( + params.repo, + workspace=params.workspace, + scan_type=params.scan_type, + ) except APIResourceNotFound: return None + except APIFailure as error: + # Returning None would make the caller create an empty baseline scan, + # reporting every dependency as newly added, so fail loudly like the + # --base-commit-sha path above. + log.error( + f"Failed to resolve the matching head scan for repo {params.repo}: {error}" + ) + if self.cli_config is None: + raise + if self.cli_config.disable_blocking: + sys.exit(0) + sys.exit(self.cli_config.exit_code_on_api_error) + if scan_id: + log.info( + "Baseline selected: source=repository-head " + f"scan_id={json.dumps(scan_id)}" + ) + return scan_id @staticmethod def update_package_values(pkg: Package) -> Package: + pkg.type = Package.normalize_type(pkg.type) pkg.purl = f"{pkg.name}@{pkg.version}" - pkg.url = f"https://socket.dev/{pkg.type}/package" if pkg.namespace: pkg.purl = f"{pkg.namespace}/{pkg.purl}" - pkg.url += f"/{pkg.namespace}" - pkg.url += f"/{pkg.name}/overview/{pkg.version}" + pkg.url = Package.socket_url(pkg.type, pkg.namespace, pkg.name, pkg.version) return pkg def get_license_text_via_purl(self, packages: dict[str, Package], batch_size: int = 5000) -> dict: @@ -1583,6 +1921,9 @@ def get_license_text_via_purl(self, packages: dict[str, Package], batch_size: in for result in results: ecosystem = result["type"] name = result["name"] + namespace = (result.get("namespace") or "").strip("/") + if namespace and not name.startswith(f"{namespace}/"): + name = f"{namespace}/{name}" package_version = result["version"] licenseDetails = result.get("licenseDetails") licenseAttrib = result.get("licenseAttrib") @@ -1596,7 +1937,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 +1961,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 +1972,13 @@ 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 a re-run over an already-compared scan pair needs + # on_duplicate=update to apply the link to the existing resource. It + # answers 200 with the same {"diff_scan": ...} envelope as a create. + 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 +1987,13 @@ 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 when there is no pull request context to attach, and on + # deployments that 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 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 +2129,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 +2163,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 +2176,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 @@ -1830,6 +2187,10 @@ def get_added_and_removed_packages( f"Diff scan comparison failed with {type(error).__name__}({error_summary}), " "falling back to the streaming scan comparison" ) + log.info( + "Diff comparison mode: requested=diff-scan effective=streaming " + f"reason={type(error).__name__}" + ) if diff_artifacts is None: try: @@ -1844,9 +2205,8 @@ def get_added_and_removed_packages( ) except APIFailure as e: log.error(f"API Error: {e}") - if self.cli_config and self.cli_config.disable_blocking: - sys.exit(0) - sys.exit(1) + # API failures are mapped to the configured infrastructure exit code by cli(). + raise except Exception as e: import traceback log.error(f"Error getting diff report: {str(e)}") @@ -1933,7 +2293,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 +2306,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: @@ -1959,6 +2322,14 @@ def create_new_diff( for path in paths: files = self.find_files(path) all_files.extend(files) + + self._log_scan_configuration( + paths, + params, + all_files, + manifest_source="provided" if explicit_files is not None else "discovered", + base_paths=base_paths, + ) # Save submitted files list if requested if save_files_list_path and all_files: @@ -1994,7 +2365,10 @@ def create_new_diff( try: head_full_scan = self.create_full_scan(empty_files, tmp_params, base_paths=base_paths) head_full_scan_id = head_full_scan.id - log.debug(f"Created empty baseline scan: {head_full_scan_id}") + log.info( + "Baseline selected: source=empty " + f"scan_id={json.dumps(head_full_scan_id)}" + ) # Clean up the temporary empty file for temp_file in empty_files: @@ -2030,9 +2404,8 @@ def create_new_diff( os.unlink(temp_file) except OSError: pass - if self.cli_config and self.cli_config.disable_blocking: - sys.exit(0) - sys.exit(1) + # API failures are mapped to the configured infrastructure exit code by cli(). + raise except Exception as e: import traceback log.error(f"Error creating new full scan: {str(e)}") @@ -2069,7 +2442,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 +2507,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 +2534,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, @@ -2281,23 +2663,24 @@ def get_source_data(package: Package, packages: dict) -> list: @staticmethod def add_purl_capabilities(diff: Diff) -> None: """ - Adds capability information to each package in the diff's new_packages list. + Adds capability information to the diff's added and updated packages. + + Both lists are walked because an updated package is still newly present at + its new version, so its capabilities are as relevant as an added one's. 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: """ @@ -2350,6 +2733,8 @@ def add_package_alerts_to_collection(self, package: Package, alerts_collection: suggestion=props.suggestion, next_step_title=props.nextStepTitle, introduced_by=introduced_by, + manifest_files=package.manifestFiles or [], + direct=bool(package.direct), purl=package.purl, url=package.url ) 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..fd8fabcd 100644 --- a/socketsecurity/core/classes.py +++ b/socketsecurity/core/classes.py @@ -1,4 +1,5 @@ import json +import logging from dataclasses import dataclass, field from typing import Dict, List, Optional, TypedDict @@ -11,6 +12,13 @@ SocketScore, ) +log = logging.getLogger("socketdev") + +# Ecosystems whose package pages cannot be addressed by name alone. A Maven +# coordinate is a groupId plus an artifactId; with no namespace the URL collapses to +# one path segment that cannot be split back into two, and the page does not resolve. +NAMESPACE_REQUIRED_TYPES = frozenset({"maven"}) + __all__ = [ "Report", "Score", @@ -142,6 +150,43 @@ class Package(): licenseAttrib: Optional[List] = None + @staticmethod + def normalize_type(package_type) -> str: + """ + Unwraps the SDK's str-backed SocketPURL_Type enum to its value. + + str(SocketPURL_Type.MAVEN) is "SocketPURL_Type.MAVEN", not "maven", so any + enum member reaching an f-string leaks the class name into user-facing output. + """ + return getattr(package_type, "value", package_type) + + @staticmethod + def socket_url(package_type, namespace: Optional[str], name: str, version: str) -> str: + """ + Builds the socket.dev package overview URL for a package. + + Namespace and name are separate path segments, the same form purl strings use. + + Args: + package_type: Ecosystem, as a string or SocketPURL_Type member + namespace: Package namespace (Maven groupId, npm scope), if any + name: Package name + version: Package version + + Returns: + Package overview URL on socket.dev + """ + package_type = Package.normalize_type(package_type) + namespace = (namespace or "").strip("/") + if not namespace and package_type in NAMESPACE_REQUIRED_TYPES: + # The link is still emitted so the finding reports, but it cannot resolve. + log.warning( + f"{package_type} package {name}@{version} has no namespace, so its " + "Socket link collapses to a single path segment and will not resolve" + ) + package_path = "/".join(part for part in (namespace, name) if part) + return f"https://socket.dev/{package_type}/package/{package_path}/overview/{version}" + @classmethod def from_socket_artifact(cls, data: dict) -> "Package": """ @@ -153,18 +198,16 @@ def from_socket_artifact(cls, data: dict) -> "Package": Returns: New Package instance """ - purl = f"{data['type']}/" - namespace = data.get("namespace") - if namespace: - purl += f"{namespace}@" - purl += f"{data['name']}@{data['version']}" - base_url = "https://socket.dev" - url = f"{base_url}/{data['type']}/package/{namespace or ''}{data['name']}/overview/{data['version']}" + package_type = Package.normalize_type(data["type"]) + namespace = (data.get("namespace") or "").strip("/") + package_path = "/".join(part for part in (namespace, data["name"]) if part) + purl = f"{package_type}/{package_path}@{data['version']}" + url = Package.socket_url(package_type, namespace, data["name"], data["version"]) return cls( id=data["id"], name=data["name"], version=data["version"], - type=data["type"], + type=package_type, release=data.get("release"), diffType=data.get("diffType"), score=data["score"], @@ -179,7 +222,7 @@ def from_socket_artifact(cls, data: dict) -> "Package": artifact=data.get("artifact"), purl=purl, url=url, - namespace=namespace + namespace=namespace or None ) @classmethod @@ -274,6 +317,11 @@ class Issue: manifests: str url: str purl: str + # The package's own manifest files, independent of how it was introduced. A + # transitive package whose ancestors are absent from the scan has no + # introduced_by chain, but its manifest is still known. + manifest_files: list + direct: bool def __init__(self, **kwargs): if kwargs: @@ -282,6 +330,10 @@ def __init__(self, **kwargs): if hasattr(self, "created_at"): self.created_at = self.created_at.strip(" (Coordinated Universal Time)") + if not hasattr(self, "manifest_files"): + self.manifest_files = [] + if not hasattr(self, "direct"): + self.direct = False if not hasattr(self, "manifests"): self.manifests = "" if not hasattr(self, "suggestion"): @@ -507,7 +559,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] @@ -518,6 +572,8 @@ class Diff: report_url: str diff_url: str new_scan_id: str + is_full_scan: bool + alerts_fetched: bool def __init__(self, **kwargs): if kwargs: @@ -525,8 +581,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"): @@ -535,6 +595,10 @@ def __init__(self, **kwargs): self.removed_alerts = [] if not hasattr(self, "new_capabilities"): self.new_capabilities = {} + if not hasattr(self, "is_full_scan"): + self.is_full_scan = False + if not hasattr(self, "alerts_fetched"): + self.alerts_fetched = False def __str__(self): return json.dumps(self.__dict__) @@ -548,8 +612,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/cli_client.py b/socketsecurity/core/cli_client.py index bfad0d14..405a7443 100644 --- a/socketsecurity/core/cli_client.py +++ b/socketsecurity/core/cli_client.py @@ -6,6 +6,7 @@ import requests from socketsecurity import USER_AGENT + from .exceptions import APIFailure from .socket_config import SocketConfig @@ -55,7 +56,12 @@ def request( except requests.exceptions.RequestException as e: logger.error(f"API request failed: {str(e)}") - raise APIFailure(f"Request failed: {str(e)}") + # Carry the status forward. Callers that need to react to a specific + # code -- the GitLab auth fallback to the other token scheme, and + # APIFailure.is_transient_error -- have no other way to recover it + # once the requests exception has been translated. + status_code = e.response.status_code if e.response is not None else None + raise APIFailure(f"Request failed: {str(e)}", status_code=status_code) from e def post_telemetry_events(self, org_slug: str, events: List[Dict]) -> None: """Post telemetry events one at a time to the v0 telemetry API. Fire-and-forget โ€” logs errors but never raises.""" diff --git a/socketsecurity/core/exceptions.py b/socketsecurity/core/exceptions.py index 03e69b87..b2112ee0 100644 --- a/socketsecurity/core/exceptions.py +++ b/socketsecurity/core/exceptions.py @@ -1,3 +1,5 @@ +from socketdev.exceptions import APIFailure as SdkAPIFailure + __all__ = [ "APIFailure", "APIKeyMissing", @@ -18,8 +20,14 @@ class APIKeyMissing(Exception): pass -class APIFailure(Exception): - """Raised when there is an error using the API""" +class APIFailure(SdkAPIFailure): + """Raised when there is an error using the API. + + Subclasses the SDK's exception of the same name so a handler written against + either one catches both. A separate Exception subclass would bypass an + ``except APIFailure`` importing the SDK's -- which every handler in + socketsecurity.core does -- and would not carry the SDK class's status code. + """ pass @@ -39,4 +47,4 @@ class APIResourceNotFound(Exception): class RequestTimeoutExceeded(Exception): """Raised when access is denied to the API""" - pass \ No newline at end of file + pass diff --git a/socketsecurity/core/git_remote.py b/socketsecurity/core/git_remote.py new file mode 100644 index 00000000..eb5b9c02 --- /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 case. +_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/helper/__init__.py b/socketsecurity/core/helper/__init__.py index ab7d06f7..224f3cc7 100644 --- a/socketsecurity/core/helper/__init__.py +++ b/socketsecurity/core/helper/__init__.py @@ -1,7 +1,8 @@ +import string + import markdown from bs4 import BeautifulSoup, Tag from bs4.element import NavigableString -import string class Helper: diff --git a/socketsecurity/core/helper/socket_facts_loader.py b/socketsecurity/core/helper/socket_facts_loader.py index fd93b9dd..26c1ae25 100644 --- a/socketsecurity/core/helper/socket_facts_loader.py +++ b/socketsecurity/core/helper/socket_facts_loader.py @@ -2,9 +2,9 @@ import json import logging -from pathlib import Path -from typing import Dict, Any, Optional, List from copy import deepcopy +from pathlib import Path +from typing import Any, Dict, List, Optional logger = logging.getLogger(__name__) @@ -65,7 +65,7 @@ def load_socket_facts(file_path: str = ".socket.facts.json") -> Optional[Dict[st return None if 'components' not in data: - logger.warning(f"Socket facts file missing 'components' key") + logger.warning("Socket facts file missing 'components' key") return data diff --git a/socketsecurity/core/lazy_file_loader.py b/socketsecurity/core/lazy_file_loader.py index 9127652b..a5bfa15a 100644 --- a/socketsecurity/core/lazy_file_loader.py +++ b/socketsecurity/core/lazy_file_loader.py @@ -2,9 +2,7 @@ Lazy file loading utilities for efficient manifest file processing. """ import logging -from typing import List, Tuple, Union, BinaryIO -from io import BytesIO -import os +from typing import List, Tuple log = logging.getLogger("socketdev") diff --git a/socketsecurity/core/messages.py b/socketsecurity/core/messages.py index 673dde5c..4017a9f8 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: """ @@ -647,32 +656,48 @@ def extract_identifiers_gitlab(alert: Issue) -> list: """ identifiers = [] - # Primary identifier: Socket alert type - identifiers.append({ + # Primary identifier: Socket alert type. The GitLab schema types identifier + # url as a string matching ^(https?|ftp)://, so an absent url is omitted + # rather than sent as null, which fails validation for the whole finding. + socket_identifier = { "type": "socket_alert", "name": f"Socket {alert.type}", "value": alert.type, - "url": alert.url if hasattr(alert, 'url') and alert.url else None - }) - - # Extract CVE identifiers from props - if hasattr(alert, 'props') and alert.props: - if 'cve' in alert.props: - cves = alert.props['cve'] - if isinstance(cves, list): - for cve in cves: - identifiers.append({ - "type": "cve", - "name": cve, - "value": cve, - "url": f"https://cve.mitre.org/cgi-bin/cvename.cgi?name={cve}" - }) - elif isinstance(cves, str): + } + alert_url = getattr(alert, "url", None) + if alert_url: + socket_identifier["url"] = alert_url + identifiers.append(socket_identifier) + + props = getattr(alert, "props", None) or {} + # Alerts reach Issue.props from several sources, so both spellings of each + # field are in play; core.alert_selection matches on the same pair. "cve" is + # the older spelling and still appears in some payloads. + identifier_fields = ( + (("cveId", "cve_id", "cve"), "cve", "https://nvd.nist.gov/vuln/detail/"), + (("ghsaId", "ghsa_id"), "ghsa", "https://github.com/advisories/"), + ) + seen = set() + for fields, identifier_type, url_prefix in identifier_fields: + for field in fields: + values = props.get(field) + if isinstance(values, str): + values = [values] + elif not isinstance(values, (list, tuple)): + continue + for value in values: + if not isinstance(value, str) or not value.strip(): + continue + value = value.strip() + identifier_key = (identifier_type, value.upper()) + if identifier_key in seen: + continue + seen.add(identifier_key) identifiers.append({ - "type": "cve", - "name": cves, - "value": cves, - "url": f"https://cve.mitre.org/cgi-bin/cvename.cgi?name={cves}" + "type": identifier_type, + "name": value, + "value": value, + "url": f"{url_prefix}{value}" }) return identifiers @@ -685,37 +710,35 @@ def extract_location_gitlab(alert: Issue) -> dict: GitLab location requires: - file: path to manifest file - dependency: package name and version - - dependency_path (optional): dependency chain """ - # Get manifest file from introduced_by or manifests attribute - manifest_file = "unknown" - dependency_path = [] - is_direct = True - - if hasattr(alert, 'introduced_by') and alert.introduced_by: - if isinstance(alert.introduced_by, list) and len(alert.introduced_by) > 0: - first_entry = alert.introduced_by[0] - if isinstance(first_entry, (list, tuple)) and len(first_entry) >= 2: - dependency_path_str = first_entry[0] - manifest_file = first_entry[1].split(';')[0] if ';' in first_entry[1] else first_entry[1] - - # Parse dependency path - if ' > ' in dependency_path_str: - dependency_path = dependency_path_str.split(' > ') - # If there's a chain, it's transitive (not direct) - is_direct = len(dependency_path) <= 1 - - elif hasattr(alert, 'manifests') and alert.manifests: - manifest_file = alert.manifests.split(';')[0] + manifest_file = "" + + introduced_by = getattr(alert, "introduced_by", None) + if isinstance(introduced_by, list) and introduced_by: + first_entry = introduced_by[0] + if isinstance(first_entry, (list, tuple)) and len(first_entry) >= 2: + manifest_file = (first_entry[1] or "").split(";")[0] + + if not manifest_file: + manifest_file = (getattr(alert, "manifests", "") or "").split(";")[0] + + if not manifest_file: + # A transitive package whose ancestors are not in this scan has no + # introduced_by chain, but the package still records its own manifest. + for entry in getattr(alert, "manifest_files", None) or []: + candidate = entry.get("file") if isinstance(entry, dict) else None + if candidate: + manifest_file = candidate + break location = { - "file": manifest_file, + "file": manifest_file or "unknown", "dependency": { "package": { "name": alert.pkg_name }, "version": alert.pkg_version, - "direct": is_direct + "direct": bool(getattr(alert, "direct", False)) } } @@ -828,6 +851,37 @@ def inline_html_text(value) -> str: return "" return " ".join(str(value).split()) + @staticmethod + def html_text(value) -> str: + """Flatten a value onto one line and escape it for an HTML text node. + + Manifest paths and sources come from the customer's repository, so any PR + author controls them: a directory named ``![x](https://host/p.png)`` or + carrying a raw tag would otherwise render as that markup inside a comment + posted by a trusted integration. Alert text comes from the API and is + escaped for the same reason, since neither is markup the CLI authored. + """ + return escape(Messages.inline_html_text(value)) + + @staticmethod + def html_attr(value) -> str: + """Escape a value for an HTML attribute, quotes included. + + Used for href and src, where an unescaped quote closes the attribute and + everything after it is read as more attributes. + """ + return escape(Messages.inline_html_text(value), quote=True) + + @staticmethod + def comment_marker_text(value) -> str: + """Neutralize an HTML comment terminator inside a marker value. + + The alert markers carry the package name so the comment can be rewritten + later, and the parser reads them back verbatim -- so this cannot escape the + value, only stop it ending the comment early. + """ + return str(value or "").replace("-->", "-->").replace(" @@ -948,39 +1002,48 @@ 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"{Messages.html_text(patched_version)}

" + if patched_version else "" + ) # Generate proper manifest URL manifest_url = Messages.get_manifest_file_url(diff, alert.manifests, config) + pkg_label = Messages.html_text(f"{alert.pkg_name}@{alert.pkg_version}") + pkg_marker = Messages.comment_marker_text(f"{alert.pkg_name}@{alert.pkg_version}") # 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 {Messages.html_text(alert.pkg_type)}/{pkg_label}
" f"Or ignore all future alerts with:
" f"@SocketSecurity ignore-all

" ) if show_ignore else "" comment += f""" - + - + """ # Add license policy violation entries grouped by PURL @@ -991,24 +1054,31 @@ def security_comment_template(diff: Diff, config=None) -> str: # Use orange diamond for license policy violations license_icon = "๐Ÿ”ถ" + license_label = Messages.html_text( + f"{first_alert.pkg_name}@{first_alert.pkg_version}" + ) + license_marker = Messages.comment_marker_text( + f"{first_alert.pkg_name}@{first_alert.pkg_version}" + ) + # Build license findings list license_findings = [] for alert in alerts: license_findings.append(alert.title) comment += f""" - + - + """ # Close table @@ -1231,6 +1301,22 @@ def create_remove_line(diff: Diff, md: MdUtils) -> MdUtils: md.new_line(removed_line) return md + # Change types the shared badge host publishes an image for. Removed and + # replaced have no artwork, so they fall back to a bold text label rather than + # rendering a broken image; added and updated render the available badges. + DIFF_BADGES = { + "Added": "diff-added.svg", + "Updated": "diff-updated.svg", + } + + @staticmethod + def get_diff_badge(change: str, package_url: str) -> str: + """Return the Dependency Overview cell marking how a package changed.""" + badge = Messages.DIFF_BADGES.get(change) + if not badge: + return f"**{change}**" + return f"[![{change}](https://github-app-statics.socket.dev/{badge})]({package_url})" + @staticmethod def create_added_table(diff: Diff, md: MdUtils) -> MdUtils: """ @@ -1252,51 +1338,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 = Messages.get_diff_badge(change, package.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)]({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 +1424,7 @@ def create_console_security_alert_table(diff: Diff) -> PrettyTable: [ "Alert", "Package", + "Patched Version", "url", "Introduced by", "Manifest File", @@ -1351,6 +1445,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, @@ -1366,8 +1461,11 @@ def create_sources(alert: Issue, style="md") -> tuple[str, str]: for source, manifest in alert.introduced_by: if style == "md": - add_str = f"
  • {manifest}
  • " - source_str = f"
  • {source}
  • " + # These land in rendered Markdown, where an unescaped path is read + # as markup. plain and raw are consumed by Slack, Jira and the + # console, which do not render HTML, so they stay verbatim. + add_str = f"
  • {Messages.html_text(manifest)}
  • " + source_str = f"
  • {Messages.html_text(source)}
  • " elif style == "plain": add_str = f"โ€ข {manifest}" source_str = f"โ€ข {source}" diff --git a/socketsecurity/core/pull_request.py b/socketsecurity/core/pull_request.py new file mode 100644 index 00000000..2dcf77f1 --- /dev/null +++ b/socketsecurity/core/pull_request.py @@ -0,0 +1,144 @@ +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 parse_pull_request_number(value) -> int: + """Coerce a configured or CI-supplied pull request number to a positive int. + + Anything that is not a positive integer means "no pull request", including the + literal ``false`` that Buildkite puts in ``BUILDKITE_PULL_REQUEST`` on non-PR + builds. Callers that hand the value on to a comment adapter should store this + result rather than the raw string, which is truthy. + """ + try: + parsed = int(value) + except (TypeError, ValueError): + return 0 + return parsed if parsed > 0 else 0 + + +def _http_url(value: Optional[str]) -> Optional[str]: + """Return ``value`` if it is an http(s) URL with a host, else ``None``. + + Every URL fragment read out of the CI environment goes through here before it + is composed into a link, because the result is sent to the API as a diff scan's + ``external_href``. Standard runners set these variables themselves, so this is + defense in depth rather than a live hole. + """ + if not value: + return None + url = value.strip().rstrip("/") + parsed = urlparse(url) + return url if parsed.scheme in ("http", "https") and parsed.netloc else None + + +def _repository_url(value: Optional[str]) -> Optional[str]: + if not value: + return None + url = value.strip().rstrip("/") + if url.endswith(".git"): + url = url[:-4] + return _http_url(url) + + +def _github_number(env: Mapping[str, str]) -> int: + number = parse_pull_request_number(env.get("PR_NUMBER")) + if number: + return number + match = re.match(r"^refs/pull/(\d+)/", env.get("GITHUB_REF", "")) + 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")) + # 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 = ( + _http_url(env.get("GITHUB_SERVER_URL")) + or (_http_url(f"https://{remote_host}") if remote_host else None) + or "https://github.com" + ) + 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 = ( + _http_url(env.get("CI_SERVER_URL")) + or (_http_url(f"https://{remote_host}") if remote_host else None) + ) + 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 = parse_pull_request_number(configured_number) + + if not configured_explicit and not number: + if provider == "github": + number = _github_number(environment) + elif provider == "gitlab": + number = parse_pull_request_number(environment.get("CI_MERGE_REQUEST_IID")) + elif provider == "azure": + number = ( + parse_pull_request_number(environment.get("SYSTEM_PULLREQUEST_PULLREQUESTNUMBER")) or + parse_pull_request_number(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/resource_utils.py b/socketsecurity/core/resource_utils.py index b49cc2e9..dc78c1b8 100644 --- a/socketsecurity/core/resource_utils.py +++ b/socketsecurity/core/resource_utils.py @@ -2,7 +2,6 @@ System resource utilities for the Socket Security CLI. """ import logging -import sys # The resource module is only available on Unix-like systems resource_available = False diff --git a/socketsecurity/core/scm/client.py b/socketsecurity/core/scm/client.py index 05757117..08769f3f 100644 --- a/socketsecurity/core/scm/client.py +++ b/socketsecurity/core/scm/client.py @@ -2,6 +2,7 @@ from typing import Dict from socketsecurity import USER_AGENT + from ..cli_client import CliClient diff --git a/socketsecurity/core/scm/github.py b/socketsecurity/core/scm/github.py index 7504a46c..dcfa5607 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,8 @@ from socketsecurity import USER_AGENT from socketsecurity.core import log from socketsecurity.core.classes import Comment +from socketsecurity.core.exceptions import APIFailure +from socketsecurity.core.git_remote import parse_git_remote from socketsecurity.core.scm_comments import Comments from socketsecurity.socketcli import CliClient @@ -38,24 +39,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] @@ -166,9 +155,21 @@ def from_env(cls, pr_number: Optional[str] = None) -> 'GithubConfig': class Github: - def __init__(self, client: CliClient, config: Optional[GithubConfig] = None): + WRITE_PERMISSIONS = frozenset({"write", "maintain", "admin"}) + + def __init__( + self, + client: CliClient, + config: Optional[GithubConfig] = None, + ignore_authorization: str = "enforce", + ): self.config = config or GithubConfig.from_env() self.client = client + self.ignore_authorization = ignore_authorization + # Permission is stable for the duration of one CLI run. Cache both + # positive and negative answers so several ignore comments by the same + # author do not each make an API request. + self._ignore_permission_cache: dict[str, Optional[bool]] = {} if not self.config.token: log.error("Unable to get Github API Token") @@ -236,7 +237,76 @@ def get_comments_for_pr(self) -> dict: else: log.error(raw_comments) - return Comments.check_for_socket_comments(comments) + gate = None if self.ignore_authorization == "off" else self.is_ignore_authorized + return Comments.check_for_socket_comments(comments, gate) + + def is_ignore_authorized(self, comment: Comment) -> bool: + """Whether a commenter may suppress alerts with @SocketSecurity ignore. + + ``author_association`` describes a social relationship to the repository, + not the author's role: an organization member or outside collaborator can + still have read-only access. Ask GitHub for the effective repository + permission instead, and cache the answer for subsequent comments. + """ + author = Comments.comment_author_name(comment) + if author == "an unknown user": + permission = None + elif author in self._ignore_permission_cache: + permission = self._ignore_permission_cache[author] + else: + path = ( + f"repos/{self.config.owner}/{self.config.repository}/" + f"collaborators/{author}/permission" + ) + try: + response = self.client.request( + path=path, + headers=self.config.headers, + base_url=self.config.api_url, + ) + result = response.json() + if not isinstance(result, dict) or not isinstance( + result.get("permission"), str + ): + log.warning("Unexpected GitHub repository permission response") + permission = None + else: + permission = ( + result["permission"].casefold() in self.WRITE_PERMISSIONS + ) + except APIFailure as error: + if getattr(error, "status_code", None) == 404: + # The repository was readable when its comments were listed, + # so a missing collaborator permission is a definitive denial. + permission = False + else: + log.warning( + "Could not read GitHub repository permission for " + f"{author}: {error}" + ) + permission = None + except Exception as error: + log.warning( + f"Could not read GitHub repository permission for {author}: {error}" + ) + permission = None + self._ignore_permission_cache[author] = permission + + if permission is not None: + return permission + if self.ignore_authorization == "strict": + log.warning( + f"Rejecting @SocketSecurity ignore from {author}: GitHub repository " + "permission could not be read and --ignore-authorization is strict." + ) + return False + log.warning( + f"Honoring @SocketSecurity ignore from {author} without verifying write " + "access: GitHub repository permission could not be read. Use a token " + "with repository metadata access, or --ignore-authorization strict to " + "reject instead." + ) + return True def add_socket_comments( self, diff --git a/socketsecurity/core/scm/gitlab.py b/socketsecurity/core/scm/gitlab.py index b3b3492f..d1fd1119 100644 --- a/socketsecurity/core/scm/gitlab.py +++ b/socketsecurity/core/scm/gitlab.py @@ -2,9 +2,11 @@ import os import sys from dataclasses import dataclass -from typing import Optional, Union +from typing import Optional import requests +from socketdev.exceptions import APIFailure + from socketsecurity import USER_AGENT from socketsecurity.core import log from socketsecurity.core.classes import Comment @@ -125,37 +127,54 @@ def _get_auth_headers(token: str) -> dict: } class Gitlab: - def __init__(self, client: CliClient, config: Optional[GitlabConfig] = None): + # GitLab access levels: 30 Developer, 40 Maintainer, 50 Owner. Reporter (20) + # and Guest (10) cannot push, so they cannot suppress an alert either. + MIN_IGNORE_ACCESS_LEVEL = 30 + # Bounded so a project with a very large membership cannot stall a scan. Past + # the cap the answer is "undetermined", handled the same as a failed lookup. + MEMBER_PAGE_SIZE = 100 + MEMBER_PAGE_LIMIT = 10 + + def __init__( + self, + client: CliClient, + config: Optional[GitlabConfig] = None, + ignore_authorization: str = "enforce", + ): self.config = config or GitlabConfig.from_env() self.client = client + self.ignore_authorization = ignore_authorization + # None until the first ignore comment forces a lookup; stays None when the + # members API cannot be read, which is the "undetermined" state. + self._member_access: Optional[dict] = None + self._member_lookup_attempted = False def _request_with_fallback(self, **kwargs): - """ - Make a request with automatic fallback between Bearer and PRIVATE-TOKEN authentication. - This provides robustness when the initial token type detection is incorrect. + """Request with one retry under the other GitLab auth scheme on a 401. + + _get_auth_headers guesses between Bearer and PRIVATE-TOKEN from the shape of + the token, and the guess can be wrong for tokens that do not match a known + pattern. Rather than fail the run, try the other scheme once. + + Catches APIFailure, not requests.exceptions.HTTPError: CliClient translates + every requests error into APIFailure, which does not inherit from HTTPError, + so catching the latter here never fired and the fallback never ran. """ try: - # Try the initial request with the configured headers return self.client.request(**kwargs) - except requests.exceptions.HTTPError as e: - # Check if this is an authentication error (401) - if e.response and e.response.status_code == 401: - log.debug(f"Authentication failed with initial headers, trying fallback method") - - # Determine the fallback headers - original_headers = kwargs.get('headers', self.config.headers) - fallback_headers = self._get_fallback_headers(original_headers) - - if fallback_headers and fallback_headers != original_headers: - log.debug("Retrying request with fallback authentication method") - kwargs['headers'] = fallback_headers - return self.client.request(**kwargs) - - # Re-raise the original exception if it's not an auth error or fallback failed - raise - except Exception as e: - # Handle other types of exceptions that don't have response attribute - raise + except APIFailure as error: + if error.status_code != 401: + raise + + log.debug("Authentication failed with initial headers, trying fallback method") + original_headers = kwargs.get('headers', self.config.headers) + fallback_headers = self._get_fallback_headers(original_headers) + if not fallback_headers or fallback_headers == original_headers: + raise + + log.debug("Retrying request with fallback authentication method") + kwargs['headers'] = fallback_headers + return self.client.request(**kwargs) def _get_fallback_headers(self, original_headers: dict) -> dict: """ @@ -255,7 +274,88 @@ def get_comments_for_pr(self) -> dict: comment.body_list = comment.body.split("\n") else: log.error(raw_comments) - return Comments.check_for_socket_comments(comments) + gate = None if self.ignore_authorization == "off" else self.is_ignore_authorized + return Comments.check_for_socket_comments(comments, gate) + + def _load_member_access(self) -> Optional[dict]: + """Map project member user id -> access level, or None if unreadable. + + ``members/all`` is used rather than a per-user lookup because it answers + non-membership with a 200 and an absent id. CliClient collapses every HTTP + error into APIFailure without a status code, so a per-user 404 -- exactly + the outsider case this guards against -- would be indistinguishable from a + token that cannot read the endpoint, and would have to fail open. + """ + if self._member_lookup_attempted: + return self._member_access + self._member_lookup_attempted = True + if not self.config.mr_project_id: + return None + + access: dict = {} + for page in range(1, Gitlab.MEMBER_PAGE_LIMIT + 1): + path = ( + f"projects/{self.config.mr_project_id}/members/all" + f"?per_page={Gitlab.MEMBER_PAGE_SIZE}&page={page}" + ) + try: + response = self._request_with_fallback( + path=path, + headers=self.config.headers, + base_url=self.config.api_url + ) + members = response.json() + except Exception as error: + log.warning(f"Could not read GitLab project members: {error}") + return None + if not isinstance(members, list): + log.warning("Unexpected GitLab project members response") + return None + for member in members: + if isinstance(member, dict) and member.get("id") is not None: + access[member["id"]] = member.get("access_level") or 0 + if len(members) < Gitlab.MEMBER_PAGE_SIZE: + self._member_access = access + return access + + log.warning( + f"GitLab project has more than {Gitlab.MEMBER_PAGE_SIZE * Gitlab.MEMBER_PAGE_LIMIT} " + "members; cannot confirm ignore-command authorization" + ) + return None + + def is_ignore_authorized(self, comment: Comment) -> bool: + """Whether a commenter may suppress alerts with @SocketSecurity ignore. + + GitLab notes carry no permission field, so this costs one members lookup + per run (cached, and only when an ignore command is actually present). + + When membership can be read the answer is definitive. When it cannot -- a + CI_JOB_TOKEN generally cannot read the members API -- the command is + honored and a warning is logged, preserving compatibility for pipelines + that rely on ignore commands. Set a token with API read access to get + enforcement. + """ + access = self._load_member_access() + if access is None: + author = Comments.comment_author_name(comment) + if self.ignore_authorization == "strict": + log.warning( + f"Rejecting @SocketSecurity ignore from {author}: GitLab project " + "membership could not be read and --ignore-authorization is strict." + ) + return False + log.warning( + f"Honoring @SocketSecurity ignore from {author} without verifying " + "write access: GitLab project membership could not be read. Use a " + "token with API read access, or --ignore-authorization strict to " + "reject instead." + ) + return True + + author = getattr(comment, "author", None) or {} + user_id = author.get("id") + return access.get(user_id, 0) >= Gitlab.MIN_IGNORE_ACCESS_LEVEL def add_socket_comments( self, diff --git a/socketsecurity/core/scm_comments.py b/socketsecurity/core/scm_comments.py index 7c479b72..3ef0e3a6 100644 --- a/socketsecurity/core/scm_comments.py +++ b/socketsecurity/core/scm_comments.py @@ -1,5 +1,6 @@ import json import re +from typing import Callable, Optional from requests import Response @@ -11,6 +12,12 @@ class Comments: VIEW_REPORT_PATTERN = re.compile(r"\[View full report\]\(([^)\s]+)\)") + @staticmethod + def comment_author_name(comment: Comment) -> str: + """Best-effort display name for a comment author, across providers.""" + user = getattr(comment, "user", None) or getattr(comment, "author", None) or {} + return user.get("login") or user.get("username") or "an unknown user" + @staticmethod def process_response(response: Response) -> dict: output = {} @@ -37,10 +44,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 +73,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 +84,30 @@ 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: + """Match an alert's package against one parsed ignore command. + + Generated commands are ecosystem-qualified (``npm/lodash@4.17.21``) but + replies typed by hand, and commands written by older CLI versions, use the + bare package name, so both have to match. + + Callers that parse the package out of a ``start-socket-alert`` marker have no + pkg_type to compare against and instead strip the ecosystem off the command. + An npm scope looks the same as an ecosystem prefix there, so only strip when + the leading segment cannot be one: without the guard, + ``ignore @types/node@*`` would also silently ignore alerts for a package + literally named ``node``. + """ + 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 and not name.startswith("@"): + 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: @@ -112,6 +140,33 @@ def process_security_comment(comment: Comment, comments) -> str: return new_body + @staticmethod + def parse_alert_table_row(line: str) -> Optional[tuple[str, str, str]]: + """Pull ``(ecosystem, package, version)`` out of a legacy alert table row. + + Returns None for any row that does not have the expected shape rather than + raising. The row comes back from the provider's API, so its contents are + outside this process's control. Malformed cells must not interrupt status + reporting. A row that cannot be read is a row whose alert stays reported. + """ + cells = line.strip().lstrip("|").rstrip("|").split("|") + if len(cells) != 5: + return None + package = cells[1] + if "](" not in package: + return None + details = package.split("](", 1)[0].lstrip("[") + if "/" not in details: + return None + ecosystem, remainder = details.split("/", 1) + if "@" not in remainder: + return None + # Split from the right: a scoped name carries its own "@". + pkg_name, pkg_version = remainder.rsplit("@", 1) + if not pkg_name or not pkg_version: + return None + return ecosystem, pkg_name, pkg_version + @staticmethod def process_original_security_comment( comment: Comment, @@ -127,19 +182,21 @@ def process_original_security_comment( start = True lines.append(line) elif start and "end-socket-alerts-table" not in line and not Comments.is_heading_line(line) and line != '': - title, package, introduced_by, manifest, ci = line.lstrip("|").rstrip("|").split("|") - details, _ = package.split("](") - ecosystem, details = details.split("/", 1) - ecosystem = ecosystem.lstrip("[") - pkg_name, pkg_version = details.split("@") - pkg_name = f"{ecosystem}/{pkg_name}" + parsed = Comments.parse_alert_table_row(line) # ignore_all has to be checked outside the loop: an ignore-all # comment produces no ignore_commands, so a loop-internal check # never runs and every row was kept. - ignore = ignore_all or any( - Comments.is_ignore(pkg_name, pkg_version, name, version) - for name, version in ignore_commands - ) + if parsed is None: + # An unparseable row cannot be evaluated against the ignore + # commands, so keep it: leaving an alert reported is the safe + # direction, and the comment body is not ours to discard. + ignore = ignore_all + else: + ecosystem, pkg_name, pkg_version = parsed + ignore = ignore_all or any( + Comments.is_ignore(pkg_name, pkg_version, name, version, ecosystem) + for name, version in ignore_commands + ) if not ignore: kept_alert = True lines.append(line) @@ -187,7 +244,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 +241,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": []} @@ -265,6 +291,17 @@ def test_collapsed_body_is_stable_when_reprocessed(self): [View full report](https://socket.dev/report/legacy?action=error%2Cwarn) """ +SCOPED_LEGACY_COMMENT = """ + + +|Alert|Package|Introduced by|Manifest File|CI| +|:---|:---|:---|:---|:---| +|Known Malware|[npm/@socketsecurity/example@1.0.0](https://socket.dev/z)|example|package.json|:no_entry_sign:| + + +[View full report](https://socket.dev/report/legacy?action=error%2Cwarn) +""" + class TestProcessOriginalSecurityComment: def test_partial_ignore_keeps_remaining_row(self): @@ -292,6 +329,27 @@ def test_ignore_all_collapses_to_the_no_alerts_body(self): assert "No dependency alerts to report" in new_body assert "[View full report](https://socket.dev/report/legacy)" in new_body + def test_scoped_package_row_does_not_raise(self): + """A scoped name carries its own "@", so the split must come from the right.""" + security = _make_comment(SCOPED_LEGACY_COMMENT) + comments = {"security": security, "ignore": []} + + new_body = Comments.process_security_comment(security, comments) + + assert "npm/@socketsecurity/example@1.0.0" in new_body + + def test_scoped_package_row_is_ignorable_both_ways(self): + for command in ( + "SocketSecurity ignore npm/@socketsecurity/example@1.0.0", + "SocketSecurity ignore @socketsecurity/example@1.0.0", + ): + security = _make_comment(SCOPED_LEGACY_COMMENT) + comments = {"security": security, "ignore": [_make_comment(command, comment_id=2)]} + + new_body = Comments.process_security_comment(security, comments) + + assert "No dependency alerts to report" in new_body, command + class TestExtractReportUrl: def test_strips_the_action_filter(self): @@ -302,3 +360,146 @@ def test_strips_the_action_filter(self): def test_returns_empty_when_absent(self): assert Comments.extract_report_url("no link here") == "" + + +# --- Escaping repo-derived values --------------------------------------------- +# +# Manifest paths and sources are file paths inside the customer's repository, so +# anyone who can open a pull request controls them: a directory named +# `![x](https://host/p.png)` holding a manifest puts that markup into a comment +# posted by a trusted integration. GitHub and GitLab sanitize comment HTML, so the +# exposure is external resource loading, phishing links and content spoofing +# rather than script execution. + + +@dataclass +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. + """ + repo: str = "acme/widgets" + branch: str = "main" + + +HOSTILE_PATHS = { + "image": "![x](https://evil.example/p.png)/package.json", + "link": "[click me](https://evil.example)/package.json", + "raw_tag": "/package.json", + "backtick": "`code`/package.json", + "pipe": "a|b/package.json", + "quote": 'a" onmouseover="x/package.json', + "comment_close": "x-->y/package.json", +} + + +def _rendered_with_path(path: str) -> str: + return Messages.security_comment_template( + _make_diff([_make_alert(manifests=path)]), _RepoConfig() + ) + + +def test_the_hostile_path_actually_reaches_the_comment(): + """Guards the fixture itself: if the path stops being rendered, the escaping + tests below would pass while asserting nothing.""" + body = _rendered_with_path("sentinel-path/package.json") + + assert "sentinel-path" in body + + +@pytest.mark.parametrize("name,path", sorted(HOSTILE_PATHS.items())) +def test_hostile_manifest_path_cannot_introduce_markup(name, path): + """In the rendered comment the path only ever lands inside an href, where + Markdown is inert. The property that matters there is that the value cannot + open a tag or close the attribute -- see create_sources for the context where + Markdown itself is live.""" + body = _rendered_with_path(path) + + rendered = [ln for ln in body.split("\n") if "Manifest File" in ln][0] + value = rendered.split('href="', 1)[1].split('"', 1)[0] + + for char in ("<", ">", '"'): + assert char not in value, f"{char!r} survived into the href: {value!r}" + assert_html_block_intact(body) + + +def test_quote_in_a_path_cannot_escape_the_href(): + body = _rendered_with_path('a" onmouseover="x/package.json') + + assert """ in body + assert 'href="https://github.com/acme/widgets/blob/main/a" ' not in body + + +def test_hostile_package_name_cannot_close_the_alert_marker(): + body = Messages.security_comment_template( + _make_diff([_make_alert(pkg_name="evil-->x")]), _FakeConfig() + ) + + # Exactly the terminator the CLI wrote, and no stray one inside the value. + for line in body.split("\n"): + if "socket-alert-" in line: + assert line.count("-->") == 1, line + + +def test_alert_text_from_the_api_is_escaped(): + body = Messages.security_comment_template( + _make_diff([_make_alert(description="")]), _FakeConfig() + ) + + assert "
    {action} - {alert.severity} + {Messages.html_attr(alert.severity)}
    - {alert.pkg_name}@{alert.pkg_version} - {Messages.inline_html_text(alert.title)} -

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

    -

    Source: Manifest File

    + {pkg_label} - {Messages.html_text(alert.title)} +

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

    + {patched_version_html} +

    Source: Manifest File

    โ„น๏ธ Read more on: - This package | - This alert | + This package | + This alert | What is known malware?

    -

    Suggestion: {Messages.inline_html_text(alert.suggestion)}

    +

    Suggestion: {Messages.html_text(alert.suggestion)}

    {ignore_html}
    {action} {license_icon}
    - {first_alert.pkg_name}@{first_alert.pkg_version} has a License Policy Violation. + {license_label} has a License Policy Violation.

    License findings:

      """ for finding in license_findings: - comment += f"
    • {Messages.inline_html_text(finding)}
    • \n" + comment += f"
    • {Messages.html_text(finding)}
    • \n" # Generate proper manifest URL for license violations @@ -1016,13 +1086,13 @@ 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 {Messages.html_text(first_alert.pkg_type)}/{license_label}. " 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 "" comment += f"""
    -

    From: Manifest File

    -

    โ„น๏ธ Read more on: This package | What is a license policy violation?

    +

    From: Manifest File

    +

    โ„น๏ธ Read more on: This package | What is a license policy violation?

    Next steps: Take a moment to review the security alert above. Review the linked package source code to understand the potential risk. Ensure the package is not malicious before proceeding. If you're unsure how to proceed, reach out to your security team or ask the Socket team for help at support@socket.dev.

    Suggestion: Find a package that does not violate your license policy or adjust your policy to allow this package's license.

    @@ -1031,7 +1101,7 @@ def security_comment_template(diff: Diff, config=None) -> str: