Skip to content

feat: allow incomplete SBOM#6666

Open
neema-beglou-snyk wants to merge 5 commits intomainfrom
feat/CSENG-173_allow_incomplete_sbom
Open

feat: allow incomplete SBOM#6666
neema-beglou-snyk wants to merge 5 commits intomainfrom
feat/CSENG-173_allow_incomplete_sbom

Conversation

@neema-beglou-snyk
Copy link
Copy Markdown

Pull Request Submission Checklist

  • Follows CONTRIBUTING guidelines
  • Commit messages
    are release-note ready, emphasizing
    what was changed, not how.
  • Includes detailed description of changes
  • Contains risk assessment (Low | Medium | High)
  • Highlights breaking API changes (if applicable)
  • Links to automated tests covering new functionality
  • Includes manual testing instructions (if necessary)
  • Updates relevant GitBook documentation (PR link: ___)
  • Includes product update to be announced in the next stable release notes

What does this PR do?

When --print-effective-graph-with-errors is set, the legacy CLI can fail on individual projects during a multi-project scan. These changes catch those project failures and return them as failedResults instead of throwing, so downstream consumers can include them to annotate sboms. Changes made to both the single and multi plugin route and is robust against full and partial failure of manifest file/'s.

Where should the reviewer start?

get-deps-from-plugin.ts --> Wrap the plugin around a try/catch to ensure we always return something.
get-multi-plugin-result.ts --> We make sure to return failedResults to later parse into scanError.

What's the product update that needs to be communicated to CLI users?

Any background context you want to provide?

This is for the JPMC "fail-fast initative" --> the idea is to annotate sbom's with failures rather than print to stdout.

@neema-beglou-snyk neema-beglou-snyk requested review from a team as code owners March 19, 2026 22:56
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Mar 19, 2026

Warnings
⚠️ There are multiple commits on your branch, please squash them locally before merging!
⚠️

"[test: add unit tests for allow-incomplete-sbom error handling CSENG-173](https://api.github.com/repos/snyk/cli/git/commits/bbc90a981ef52c6504034d5f64a1216bbbbe90af)" is too long. Keep the first line of your commit message under 72 characters.

Generated by 🚫 dangerJS against 3c24ca8

@snyk-pr-review-bot

This comment has been minimized.

@snyk-io
Copy link
Copy Markdown

snyk-io bot commented Mar 19, 2026

Snyk checks have passed. No issues have been found so far.

Status Scan Engine Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@mihaibuzgau mihaibuzgau changed the title Feat/cseng 173 allow incomplete SBOM feat: allow incomplete SBOM Mar 20, 2026
@snyk-pr-review-bot

This comment has been minimized.

@neema-beglou-snyk neema-beglou-snyk force-pushed the feat/CSENG-173_allow_incomplete_sbom branch from 6ec1575 to 03a480b Compare March 20, 2026 11:48
@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot
Copy link
Copy Markdown

snyk-pr-review-bot bot commented Mar 20, 2026

Reviewed

I've reviewed this and there is no data loss. In an event where there is a partial result, allResults.length is truthy, so the return block between lines 213-219 still gives us results for the extension-sbom to consume.

PR Reviewer Guide 🔍

🧪 No relevant tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Partial Failure Data Loss 🟠 [major]

The failedResults containing project scan errors are only returned if the scan fails for all projects (where allResults.length is zero). In an 'incomplete SBOM' scenario where some projects succeed and others fail, the if (!allResults.length) block is bypassed. The TODO on line 171 confirms that these failures are not currently propagated back in the main return path. This means downstream consumers will not see failure annotations unless the entire scan fails.

if (!allResults.length) {
  // When allow-incomplete-sbom is active, return instead of throwing
  // so the caller can print per-project JSONL error entries
  if (options['print-effective-graph-with-errors']) {
    return {
      plugin: {
        name: 'custom-auto-detect',
      },
      scannedProjects: allResults,
      failedResults,
    };
  }
📚 Repository Context Analyzed

This review considered 6 relevant code sections from 6 files (average relevance: 1.05)

@snyk-pr-review-bot

This comment has been minimized.

@snyk-pr-review-bot
Copy link
Copy Markdown

PR Reviewer Guide 🔍

🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Potential Silent Success 🟠 [major]

When print-effective-graph-with-errors is enabled and a single plugin scan fails, getDepsFromPlugin returns a MultiProjectResultCustom with an empty scannedProjects list. In src/lib/snyk-test/run-test.ts, the logic for generating payloads iterates over deps.scannedProjects. If this list is empty, no payloads are created, and assembleLocalPayloads returns an empty array. This can lead to the CLI exiting with a success code (0) even if the only project failed to scan, which contradicts expected behavior for a 'fail' scenario unless explicitly handled at the entry point.

scannedProjects: [],
failedResults: [
  {
    targetFile: options.file,
    error,
    errMessage,
  },
],
Bypassed Error Propagation 🟠 [major]

In getMultiPluginResult, if allResults.length is 0 and the error flag is set, the function returns early. This bypasses the FailedToRunTestError throw and the errorMessageWithRetry logic. If downstream consumers do not specifically check failedResults and instead only check for the presence of scannedProjects, the system treats a total failure as an empty success. This may break CI/CD pipelines that rely on the CLI to fail when no valid manifests are found.

if (options['print-effective-graph-with-errors']) {
  return {
    plugin: {
      name: 'custom-auto-detect',
    },
    scannedProjects: allResults,
    failedResults,
  };
}
📚 Repository Context Analyzed

This review considered 12 relevant code sections from 4 files (average relevance: 0.83)

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.

2 participants