Skip to content

fix: multi artifact - #4440

Merged
ulemons merged 2 commits into
mainfrom
fix/multi-artifcat-bug
Aug 5, 2026
Merged

fix: multi artifact#4440
ulemons merged 2 commits into
mainfrom
fix/multi-artifcat-bug

Conversation

@ulemons

@ulemons ulemons commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes a silent-fallback bug in the blast-radius intel stage: when an advisory affects
multiple artifacts (multi-module Go, multi-artifact Maven, multi-package npm) and either no
package was requested or the requested package wasn't found in the advisory, the code
picked entries[0] as a fallback. This meant an advisory-wide analysis on a multi-artifact
advisory silently analyzed only the first affected package, reporting the job as completed
successfully while actually covering just one of several affected artifacts.

Changes

  • Added selectAdvisoryEntry (blast-radius/stages/selectAdvisoryEntry.ts), a shared helper
    used by npm/Go/Maven's intel stages that replaces the find(...) || entries[0] fallback
    pattern:
    • a requested package not found in the advisory now throws, instead of silently falling
      back to the first entry
    • an omitted request against a multi-artifact advisory now throws too (advisory-wide
      analysis is only valid for single-artifact advisories) — the failure is loud
      (status: 'failed') rather than a misleadingly "successful" partial analysis
    • an omitted request against a single-artifact advisory still analyzes that one entry, same
      as before
  • Updated intelGo.ts, intelMaven.ts, intelNpm.ts to use selectAdvisoryEntry instead of
    their own inline fallback logic.
  • Updated the public API's OpenAPI spec (single job, bulk job, and the package field
    description) to document that advisory-wide analysis is only supported for single-package
    advisories, and that multi-package advisories require an explicit package.
  • Added selectAdvisoryEntry.test.ts covering: single-entry no-request pass-through,
    matching-request selection, non-matching-request rejection, and omitted-request rejection
    against a multi-artifact advisory.

Type of change

  • Bug fix
  • New feature
  • Refactor / cleanup
  • Performance improvement
  • Chore / dependency update
  • Documentation

JIRA ticket

Copilot AI balanced review requested due to automatic review settings August 4, 2026 15:11
@ulemons ulemons self-assigned this Aug 4, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Prevents blast-radius jobs from silently analyzing the wrong artifact in multi-package OSV advisories.

Changes:

  • Adds shared advisory-entry selection and tests.
  • Applies selection to npm, Maven, and Go stages.
  • Documents explicit-package requirements.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
selectAdvisoryEntry.ts Adds strict artifact selection.
selectAdvisoryEntry.test.ts Tests selection and rejection paths.
npm/intelNpm.ts Uses shared selection for npm.
maven/intelMaven.ts Uses shared selection for Maven.
go/intelGo.ts Uses shared selection for Go.
openapi.yaml Documents multi-package behavior.
Suppressed comments (1)

services/apps/packages_worker/src/blast-radius/stages/selectAdvisoryEntry.ts:22

  • This counts affected records rather than distinct packages. OSV legitimately repeats one package across multiple affected[] entries for disjoint ranges, so an omitted package would fail even though the advisory affects only one package, contradicting the new API contract. Check unique normalized package identities and aggregate duplicate records before deciding this is multi-artifact.
  if (entries.length > 1) {

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ulemons ulemons added the Bug Created by Linear-GitHub Sync label Aug 5, 2026
Copilot AI review requested due to automatic review settings August 5, 2026 07:59
ulemons added 2 commits August 5, 2026 10:03
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
Signed-off-by: Umberto Sgueglia <usgueglia@contractor.linuxfoundation.org>
@ulemons
ulemons force-pushed the fix/multi-artifcat-bug branch from f444e4e to 5903630 Compare August 5, 2026 08:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

Suppressed comments (1)

services/apps/packages_worker/src/blast-radius/clients/osvClient.ts:79

  • When duplicate entries mix representation styles (for example, one has ranges and another has only versions), this merge retains both fields but both semverRangeEvents and mavenRangeEvents consume versions only when no range events exist. The explicit versions from the second entry are therefore silently excluded from the vulnerable-version set. Please combine explicit versions with range-derived events (and add a mixed ranges/versions regression case) so all duplicate-entry data is analyzed.
    if (entry.versions) {
      existing.versions = [...new Set([...(existing.versions ?? []), ...entry.versions])]

Copilot AI review requested due to automatic review settings August 5, 2026 08:03

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.

@ulemons
ulemons marked this pull request as ready for review August 5, 2026 08:19
@cursor

cursor Bot commented Aug 5, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes blast-radius job outcomes for multi-artifact or mismatched package requests (previously misleading success); behavior is stricter but correct for security analysis accuracy.

Overview
Fixes a silent wrong-package bug in blast-radius intel: npm, Go, and Maven stages no longer use find(...) || entries[0] when resolving which OSV affected entry to analyze.

A new shared selectAdvisoryEntry helper fails the job (non-retryable Temporal ApplicationFailure) when the requested package is missing from the advisory, when package is omitted on a multi-artifact advisory, or when an empty string is treated as an explicit request. Single-package advisories with no package still analyze that one entry; explicit matches still return relatedAffectedPackages for the other affected names.

affectedEntriesForEcosystem now merges duplicate OSV rows for the same ecosystem+name (aggregating ranges/versions, deduping identical ranges) so multi-entry same-package advisories are handled correctly before counting artifacts.

The Akrites blast-radius OpenAPI text is updated to match: advisory-wide submit works only for single-package advisories; multi-package jobs need package or they fail with status: 'failed'.

Reviewed by Cursor Bugbot for commit 5903630. Bugbot is set up for automated code reviews on this repo. Configure here.

@ulemons
ulemons merged commit 68e687c into main Aug 5, 2026
16 checks passed
@ulemons
ulemons deleted the fix/multi-artifcat-bug branch August 5, 2026 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Bug Created by Linear-GitHub Sync

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants