Skip to content

Commit 88408dc

Browse files
leliaclaude
andauthored
Fix GitLab report links, identifiers and locations, and make diff baselines resilient (#337)
* fix(gitlab): stabilize report data * chore: bump version to 2.8.1 * fix(gitlab): harden implicit diff baseline resolution The workspace-scoped head scan lookup treated any failed request as "no baseline". The SDK logs and returns {} for every non-200, so a transient API error resolved to None, and create_new_diff answers None by creating an empty baseline scan -- reporting every dependency in the repository as newly added. An absent "results" key now raises APIFailure, and resolve_base_full_scan_id surfaces it the same way a missing --base-commit-sha baseline is surfaced. Selecting the newest scan on the default branch also reintroduced temporary scans, which the repository head pointer had excluded. The empty baseline scan that create_new_diff creates inherits the branch and commit of the run that created it, so a default-branch run whose real scan fails leaves that empty scan as the newest one. Both baseline lookups now skip tmp scans. Also unwrap scan_type before it is URL encoded. FullScanParams types it as a ScanType enum, and urlencode renders a (str, Enum) member as its repr-style name, which would filter on a scan type that does not exist. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gitlab): match snake_case vulnerability ids in report identifiers Issue.props reaches the GitLab formatter from several sources, and core.alert_selection already matches both ghsaId/ghsa_id and cveId/cve_id when deciding reachability. The identifier extractor only read the camelCase spellings, so an alert carrying ghsa_id was selected for the report but emitted with only its socket_alert identifier -- the CVE and GHSA values GitLab dedupes and links on were dropped. Values that are neither a string nor a sequence are now skipped rather than iterated, so a malformed prop cannot raise out of the whole report. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * test: use a generic package name in the namespace normalization fixture The fixture named a real organization. Public test data should not, so use the reserved com.example namespace instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gitlab): use colon-separated Maven coordinates in package links Socket addresses Maven package pages as groupId:artifactId. The CLI emitted the slash-separated form, so every Maven package link 404'd -- the dashboard's Maven handler rejects the slash form outright with "Maven package must have a colon". Removing the enum leak from these URLs fixed how they looked without fixing where they pointed. The separator now follows the ecosystem, via Package.socket_url, which both the full-scan and diff construction paths call. Previously each built its URL inline and they disagreed on namespace handling, so the same package could produce different links depending on which path ran. Purl strings are deliberately left on the slash form for every ecosystem: that is what the purl spec defines and what Socket's purl API consumes. Only the dashboard URL is ecosystem-dependent. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs: tighten comments added by this branch The Maven separator rule was explained in three places and the enum-unwrapping rule in two. Each now has one home: the separator at URL_NAMESPACE_SEPARATORS where it is defined, the enum behavior at each helper that depends on it, stated once rather than narrated. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gitlab): warn when a Maven coordinate has no namespace An ecosystem with its own URL separator cannot be addressed without both halves of the coordinate. A Maven artifact that arrives with no groupId still gets a link so the finding reports, but that link cannot resolve, and previously it was emitted silently. It now logs a warning naming the package. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gitlab): separate namespace and name with a slash, not a colon Reverts the separator introduced two commits ago. It rested on a report that the slash form does not resolve, which has since failed to reproduce: every affected link in that report loads, and the report's own screenshots show a working slash-form link. The defect those links actually exhibit is a namespace and name fused with no separator at all, which yields one path segment that cannot be split back into two. A slash fixes that and matches what the other package construction path has always emitted. The missing-namespace warning is kept and re-aimed: an absent namespace is what produces the unsplittable single segment, so that is the case worth surfacing. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * chore: bump version to 2.8.2 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gitlab): report a real manifest and real directness in report locations Two defects in the same location block. The manifest path fell back to "unknown" whenever a package had no introducing chain. That happens routinely for a transitive package whose top-level ancestors are absent from the scan's package set, which a diff-scoped run causes by construction. The package records its own manifest files regardless, so those are now used before giving up. Directness was inferred by looking for " > " in the introducing entry, but no producer emits that separator -- get_source_data yields either ("direct", files) or (ancestor_purl, files). Every finding was therefore reported as direct, including transitive ones. It now comes from the package record. The dependency chain was also parsed into a local that was never read, and the docstring advertised a dependency_path key the function never returned. Both are removed rather than wired up, since the GitLab schema expects dependency references rather than a name path. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix(gitlab): omit an absent identifier url instead of sending null The GitLab dependency-scanning schema types an identifier's url as a string matching ^(https?|ftp)://, so a null fails validation. The socket_alert identifier emitted null whenever an alert carried no url, which invalidates that finding for every consumer that validates the report. Verified against the published schema: a report containing an alert with no url now produces zero validation errors. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * feat(gitlab): fall back to the nearest scanned ancestor for --base-commit-sha A merge base can have no full scan even when default-branch scanning is configured and running: squash merges and rebases rewrite commits, and a multi-commit push produces one scan for the tip while leaving the commits in between unscanned. Any of those turned every open merge request into a failed pipeline, because a missing baseline was a hard stop with no degraded mode. The requested commit is still preferred. When it has no scan, one listing of recent scans is matched against local first-parent history and the nearest scanned ancestor is used instead, logged at warning with the commit chosen and its distance. Only an unreachable ancestor now fails the run. Both bounds are fixed and neither costs an extra request: the listing is fetched once, and the walk stops at a set depth. Following first parents keeps a merge commit from contributing everything merged into it, and a shallow checkout simply narrows the search rather than breaking it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * fix: harden diff baseline resolution --------- Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 2dd3001 commit 88408dc

14 files changed

Lines changed: 980 additions & 134 deletions

CHANGELOG.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,32 @@
11
# Changelog
22

3+
## 2.8.3
4+
5+
### Fixed: GitLab report serialization and workspace baselines
6+
7+
- Full-scan package identities and Socket links now preserve namespaced packages
8+
when the SDK returns enum-backed ecosystem values.
9+
- Namespaced package links separate the namespace from the name instead of
10+
concatenating them, so Maven links no longer fuse groupId and artifactId into a
11+
single unresolvable path segment. A namespaced package whose namespace is
12+
missing now logs a warning rather than emitting a broken link silently.
13+
- GitLab dependency-scanning reports emit CVE and GHSA identifiers from current
14+
API fields while remaining compatible with legacy CVE data.
15+
- GitLab report findings record the manifest they came from when the package's
16+
introducing chain is unavailable, instead of reporting the location as
17+
`unknown`, and report whether a dependency is direct from the package record
18+
rather than inferring it from a dependency-path string that is never produced.
19+
- `--base-commit-sha` degrades to the nearest scanned ancestor of the requested
20+
commit instead of failing the run, and logs which commit was used and how far
21+
back it is. Squash merges, rebases, and multi-commit pushes all leave a merge
22+
base unscanned even when default-branch scanning is configured correctly. The
23+
lookup follows paginated scan history and the run still fails when no scanned
24+
ancestor is reachable or the exact-commit lookup itself fails.
25+
- Implicit diff baselines are selected from the same workspace, scan type,
26+
repository, and default branch, including when no workspace is supplied. A
27+
baseline lookup that fails is reported as an API error instead of resolving to
28+
an empty baseline, and temporary scans are skipped when selecting one.
29+
330
## 2.8.2
431

532
### Changed: bump pinned @coana-tech/cli to 15.10.41

README.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -44,21 +44,22 @@ socketcli --enable-gitlab-security --gitlab-security-file gl-dependency-scanning
4444

4545
### PR scan diffed against the merge base
4646

47-
By default, PR scans are diffed against the repository's latest head scan. To diff against
48-
the exact commit your PR branched from instead, pass the merge base as the baseline:
47+
By default, PR scans are diffed against the repository's latest matching head scan. To
48+
prefer the commit your PR branched from as the baseline, pass the merge base:
4949

5050
```bash
5151
BASE_SHA=$(git merge-base origin/main HEAD)
5252
socketcli --pr-number 123 --base-commit-sha "$BASE_SHA"
5353
```
5454

55-
> **Requirement:** `--base-commit-sha` only works if Socket already has a full scan for that
56-
> exact commit. In practice this means your CI must run `socketcli` on **every commit that
57-
> lands on your default branch** — not just some of them. If merges can land without a scan
58-
> (skipped/canceled builds, `[skip ci]`, path-filtered pipelines), the PR scan will fail with
59-
> exit code 3 rather than silently diff against the wrong baseline. See
55+
> The CLI uses the exact commit's newest matching full scan when one exists. Otherwise, it
56+
> searches up to 100 first-parent commits in the local checkout and uses the nearest scanned
57+
> ancestor, with a warning that the diff is wider than the merge base. Run `socketcli`
58+
> regularly on your default branch and ensure PR checkouts contain enough history for that
59+
> walk. The run fails with the configured API-error exit code only when no scanned ancestor
60+
> is reachable (or when the scan lookup itself fails). See
6061
> [`docs/cli-reference.md`](https://github.com/SocketDev/socket-python-cli/blob/main/docs/cli-reference.md)
61-
> for the full requirements and a backfill pattern that makes PR jobs self-sufficient.
62+
> for the full behavior and an optional exact-baseline backfill pattern.
6263
6364
A specific full scan ID also works: `--base-scan-id <id>`.
6465

docs/ci-cd.md

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -347,11 +347,14 @@ generator rather than a static YAML file:
347347
BASE_SHA=$(git merge-base "origin/${TARGET}" HEAD)
348348
```
349349

350-
- **Emit the backfill step conditionally from the generator.** The generator is the
351-
natural place for the "does a baseline scan exist?" check
350+
- **If an exact baseline is required, emit a backfill step conditionally from the
351+
generator.** The generator is the natural place for the "does an exact baseline
352+
scan exist?" check
352353
(`GET /orgs/{org}/full-scans?repo=<repo>&commit_hash=$BASE_SHA&per_page=1`): only
353354
emit the baseline-scan step when it returns nothing. The emitted pipeline then shows
354-
in the UI whether a backfill will run.
355+
in the UI whether a backfill will run. Without a backfill, the CLI automatically
356+
uses the nearest scanned first-parent ancestor within 100 commits and warns that
357+
the diff is wider.
355358

356359
- **Keep the backfill inside one command step.** The checkout-base → scan →
357360
checkout-PR sequence must not be split across steps — steps can land on different
@@ -360,17 +363,18 @@ generator rather than a static YAML file:
360363
checkout: `git worktree add /tmp/socket-base "$BASE_SHA"` then
361364
`socketcli --target-path /tmp/socket-base --branch "$TARGET" --disable-blocking`.
362365

363-
- **Soft-fail infra errors, not findings.** A missing baseline (or any API error)
364-
exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1.
366+
- **Soft-fail infra errors, not findings.** No reachable scanned ancestor (or any API
367+
error) exits with code 3 (`--exit-code-on-api-error` to change it); real findings exit 1.
365368
[`soft_fail: [{exit_status: 3}]`](https://buildkite.com/docs/pipelines/configure/step-types/command-step)
366369
on the PR scan step keeps infra errors from blocking merges while security findings
367370
still do.
368371

369372
- **["Cancel intermediate builds"](https://buildkite.com/docs/pipelines/configure/canceling-builds#cancel-running-intermediate-builds)
370-
on the default branch is the main source of baseline gaps.** Canceled builds never
371-
scan their commit, so merge-base lookups for PRs based on those commits fail. The
372-
conditional backfill step above is the remedy; there is no per-step exemption from
373-
build cancellation in Buildkite. If you need strict scan-once semantics for
373+
on the default branch is a common source of exact-baseline gaps.** Canceled builds
374+
never scan their commit, so these PRs fall back to an older scanned ancestor. Use
375+
the conditional backfill step above when an exact merge-base comparison is required;
376+
there is no per-step exemption from build cancellation in Buildkite. If you need
377+
strict scan-once semantics for
374378
concurrent backfills of the same merge base, serialize the backfill step with a
375379
[concurrency group](https://buildkite.com/docs/pipelines/configure/workflows/controlling-concurrency)
376380
keyed on the merge-base SHA.

docs/cli-reference.md

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -256,24 +256,22 @@ If you don't want to provide the Socket API Token every time then you can use th
256256
| `--commit-message` | False | *auto* | Commit message (auto-detected from git) |
257257
| `--commit-sha` | False | *auto* | Commit SHA (auto-detected from git) |
258258
| `--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` |
259-
| `--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` |
259+
| `--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` |
260260

261-
> **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:
261+
> **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:
262262
>
263263
> ```shell
264264
> BASE_SHA=$(git merge-base origin/main HEAD)
265265
> socketcli --pr-number 123 --base-commit-sha "$BASE_SHA"
266266
> ```
267267
>
268-
> **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:
268+
> `--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.
269269
>
270-
> - 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))
271-
> - `[skip ci]` commits, path-filtered pipelines, or failed/canceled scan steps
272-
> - merge-base commits that predate your Socket rollout
270+
> 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.
273271
>
274-
> 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.
272+
> 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.
275273
>
276-
> **Backfill pattern** — if your default-branch coverage has gaps, the PR job can create the missing baseline itself before scanning:
274+
> **Optional exact-baseline backfill** — if the wider ancestor fallback is not acceptable, the PR job can create the missing exact baseline before scanning:
277275
>
278276
> ```shell
279277
> BASE_SHA=$(git merge-base origin/main HEAD)
@@ -285,7 +283,7 @@ If you don't want to provide the Socket API Token every time then you can use th
285283
> socketcli --pr-number 123 --base-commit-sha "$BASE_SHA"
286284
> ```
287285
>
288-
> 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.
286+
> 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.
289287
>
290288
> 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.
291289

pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ build-backend = "hatchling.build"
66

77
[project]
88
name = "socketsecurity"
9-
version = "2.8.2"
9+
version = "2.8.3"
1010
requires-python = ">= 3.11"
1111
license = {"file" = "LICENSE"}
1212
dependencies = [

socketsecurity/__init__.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
__author__ = 'socket.dev'
2-
__version__ = '2.8.2'
2+
__version__ = '2.8.3'
33
USER_AGENT = f'SocketPythonCLI/{__version__}'

socketsecurity/config.py

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -587,9 +587,9 @@ def create_argument_parser() -> argparse.ArgumentParser:
587587
metavar="<sha>",
588588
default=None,
589589
help="Commit SHA to diff the new scan against, overriding the repository's head "
590-
"scan as the baseline. The most recent full scan matching this commit (e.g. "
591-
"the merge base from 'git merge-base origin/main HEAD') is used; the CLI "
592-
"errors if no scan exists for it. Mutually exclusive with --base-scan-id."
590+
"scan as the baseline. The CLI uses the most recent matching full scan, or "
591+
"the nearest scanned first-parent ancestor within 100 local commits when "
592+
"the commit itself was not scanned. Mutually exclusive with --base-scan-id."
593593
)
594594

595595
# Path and File options

0 commit comments

Comments
 (0)