Skip to content

Fix GitLab report links, identifiers and locations, and make diff baselines resilient - #337

Draft
lelia wants to merge 15 commits into
mainfrom
lelia/gitlab-integration-stability
Draft

Fix GitLab report links, identifiers and locations, and make diff baselines resilient#337
lelia wants to merge 15 commits into
mainfrom
lelia/gitlab-integration-stability

Conversation

@lelia

@lelia lelia commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

Package links

  • Normalize package identities and links when the SDK returns enum-backed ecosystem types and namespaced packages
  • Separate namespace from name, so Maven links no longer fuse groupId and artifactId into one unresolvable path segment
  • Warn when a namespaced ecosystem yields a package with no namespace, instead of emitting a broken link silently

Report contents

  • Emit CVE and GHSA identifiers from the current API payload fields, and from the older spellings that still appear
  • Omit an absent identifier url instead of sending null, which fails schema validation for the whole finding
  • Record the manifest a finding came from when the introducing chain is unavailable, instead of reporting the location as unknown
  • Report whether a dependency is direct from the package record, rather than inferring it from a dependency-path string no producer emits

Diff baselines

  • --base-commit-sha uses the exact matching scan when available and otherwise falls back to the nearest scanned first-parent ancestor, logging which commit was used and how far back it is
  • Paginate matching scan history so reruns and other branches cannot hide a reachable scanned ancestor beyond the first 100 recent scans
  • Resolve implicit baselines from the same workspace, scan type, repository, and default branch, including when no workspace is supplied
  • Treat failed head and exact-commit lookups as API errors rather than resolving to an empty baseline or falling back to an older ancestor
  • Skip temporary scans when selecting a baseline across all three lookups
  • Update CLI help and CI/CD documentation to describe the ancestor fallback, local-history limit, failure behavior, and optional exact-baseline backfill

Version bumped to 2.8.2.

Why these belong together

These all sit at the boundary between Socket API data and GitLab-facing output, and they share the same two failure modes: a malformed value reaching the report, or a missing baseline silently becoming an empty one. They also ship in one release, so splitting them would add review cycles without changing what lands.

Testing

  • pytest -q tests/unit tests/core — 568 passed, 2 skipped. A bare pytest honors testpaths = tests/unit and skips tests/core; CI runs both directories explicitly.
  • ruff check socketsecurity tests — clean across the full tree.
  • git diff --check — clean.
  • Report output validated against GitLab's published dependency-scanning-report-format.json (v15.2.5): zero errors, including the previously invalid null-url case.
  • Baseline coverage includes scan-type scoping without a workspace, paginated ancestor selection, exact-lookup API failures, temporary scans, missing local history, and missing ancestors.

Notes for review

  • The package-link separator is a slash for every ecosystem, matching socket.dev's own package URLs. Purl strings keep the slash form the purl spec defines.
  • location.file is required with minLength: 1 in the GitLab schema, so unknown stays as a last-resort placeholder rather than being omitted.
  • Directness previously read as true for every finding, transitive included, because the inference looked for a " > " separator get_source_data never produces. Two tests encoded that inference and now use the package record.
  • Ancestor fallback is only entered after a successful exact-commit lookup returns no persisted scan. API failures remain infrastructure errors, because selecting an older or empty baseline would misreport what the PR introduced.

Fixes: CE-435, CE-438, CE-453
Refs: CE-441

@lelia
lelia requested a review from a team as a code owner September 3, 2026 19:31
@lelia
lelia deployed to socket-firewall September 3, 2026 19:34 — with GitHub Actions Active
lelia and others added 3 commits September 4, 2026 12:07
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>
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>
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>
@lelia
lelia deployed to socket-firewall September 4, 2026 16:07 — with GitHub Actions Active
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>
@lelia
lelia marked this pull request as draft September 8, 2026 21:22
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>
@lelia lelia changed the title Stabilize GitLab report serialization and workspace baselines Fix GitLab report package links and identifiers, and workspace diff baselines Sep 11, 2026
@lelia

lelia commented Sep 11, 2026

Copy link
Copy Markdown
Contributor Author

bugbot run

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Bugbot reviewed your changes and found no new issues!

Comment @cursor review or bugbot run to trigger another review on this PR

Reviewed by Cursor Bugbot for commit c5fcbc8. Configure here.

lelia and others added 4 commits September 12, 2026 00:03
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>
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>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
main added structured baseline-selection logging to resolve_base_full_scan_id
while this branch added workspace and scan-type scoping plus explicit handling
for a failed lookup. Both are kept: the log line now reports the scan chosen by
the workspace-scoped lookup, and a lookup that errors still exits rather than
resolving to an empty baseline.

The CHANGELOG section was renamed to 2.8.2 before merging, since main has since
shipped 2.8.1 and a matching header is dropped silently.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@lelia
lelia deployed to socket-firewall September 12, 2026 04:32 — with GitHub Actions Active
…ations

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>
@lelia
lelia deployed to socket-firewall September 12, 2026 04:53 — with GitHub Actions Active
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>
@lelia
lelia deployed to socket-firewall September 12, 2026 05:00 — with GitHub Actions Active
…mmit-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>
@lelia lelia changed the title Fix GitLab report package links and identifiers, and workspace diff baselines Fix GitLab report links, identifiers and locations, and make diff baselines resilient Sep 12, 2026
@lelia
lelia deployed to socket-firewall September 12, 2026 05:03 — with GitHub Actions Active
@lelia
lelia deployed to socket-firewall September 12, 2026 06:02 — with GitHub Actions Active
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant