fix(cli): classify non-failure exit codes in tearDown to prevent duplicate output [CLI-1765]

[PeterSchafer]

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) — none
• Links to automated tests covering new functionality
• Includes manual testing instructions (if necessary)
• Updates relevant GitBook documentation (PR link: ___) — n/a, no user-facing docs change
• Includes product update to be announced in the next stable release notes

What does this PR do?

CLI Ticket: https://snyksec.atlassian.net/browse/CLI-1765

Problem

When a CLI command completes successfully but with a non-zero exit code (e.g. exit 1 = vulnerabilities found), and an auxiliary network error was handled during the run (e.g. a provenance fetch failure in snyk-docker-plugin 9.20.0), tearDown feeds both errors into processError. Inside processError, FindMostRelevantError joins them via errors.Join, producing a new error whose concrete type is neither *exec.ExitError nor *ErrorWithExitCode. The existing guard in displayError uses direct type assertions, so it fails to recognise the joined error as a command result and prints it after the command's own JSON output — making the output unparseable.

Root cause

The condition if err != nil in tearDown (line 519) treats every non-nil error the same. But Go's error interface is used for two distinct purposes here:

1. Command results — *exec.ExitError (exit 1, 3) or *ErrorWithExitCode that carry a known non-failure exit code. The command already produced its output.
2. Actual failures — errors that need decoration, prioritisation, and display.

Sending command results through processError → FindMostRelevantError → errors.Join destroys the type information that the rest of the pipeline depends on.

Fix

Replace if err != nil with if cli_errors.IsFailure(err). IsFailure checks both the error type and the specific exit code:

• *exec.ExitError or *ErrorWithExitCode with exit code 1 (vulnerabilities found) or 3 (unsupported projects) → not a failure, skip processError, preserve the original type
• *exec.ExitError with exit code 44 (TS CLI terminated) → is a failure, goes through processError where the existing terminate filter handles it
• Everything else (exit code 2, plain errors, catalog errors) → is a failure, full processing as before

This also pins snyk-docker-plugin to 9.20.0, which is the version that triggers the auxiliary network errors that exposed this bug.

Where should the reviewer start?

cliv2/internal/errors/errors.go — IsFailure and isNonFailureExitCode, then the one-line change in cliv2/pkg/core/main.go tearDown.

Then cliv2/internal/errors/errors_test.go — TestIsFailure covers nil, exit codes 1/2/3/44, ErrorWithExitCode, plain errors, and joined errors.

How should this be manually tested?

Against an image whose provenance fetch fails (snyk-docker-plugin 9.20.0+):

snyk container test <image> --json > out.json
jq . out.json

With the fix, out.json should be valid JSON with no trailing error text. Without the fix, an extra error line is appended after the JSON.

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

n/a — this fixes an internal error-handling issue; no user-facing behavior change beyond the bug fix.

Risk assessment (Low | Medium | High)?

Low — the change gates a single if condition in tearDown with a well-defined predicate. Command results with exit codes 1 and 3 skip processError (which was a no-op for them in the single-error case anyway). All other error paths are unchanged. Exit code 44 (TS CLI terminated) intentionally remains classified as a failure so the existing filter in processError continues to handle it.

Any background context you want to provide?

Alternative to #7130, which patches the same bug inside processError (short-circuit when command owns output) and adds a shouldSuppressDisplay guard before displayError. This PR instead fixes it at the call site — command results never enter the error processing pipeline, so errors.Join never destroys the type, and displayError's existing guard works unchanged.

What are the relevant tickets?

• https://snyksec.atlassian.net/browse/CLI-1765
• Original PR: fix(cli): prevent handled errors from producing duplicate output [CLI-1765] #7130
• Discussion: https://snyksec.atlassian.net/servicedesk/customer/portal/64/CLIA-1576

[snyk-io]

✅ 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.

[github-actions]

	Warnings
⚠️	There are multiple commits on your branch, please squash them locally before merging!
⚠️	"[fix(cli): classify non-failure exit codes in tearDown to prevent duplicate output CLI-1765](#7191)" is too long. Keep the first line of your commit message under 72 characters.

Generated by 🚫 dangerJS against 836ee0d

[snyk-pr-review-bot]

PR Reviewer Guide 🔍

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Brittle Type Assertion 🟠 [major] The IsFailure function uses direct type assertions (err.(*exec.ExitError) and err.(*ErrorWithExitCode)) instead of errors.As. This will fail to correctly identify non-failure exit codes if the error has been wrapped (e.g., via fmt.Errorf("%w", ...)). Given that cliv2/pkg/core/exitcode.go uses errors.Join to combine exit codes with other errors, IsFailure will return true for these wrapped errors, leading to the same duplicate output issue this PR intends to fix. if exitErr, ok := err.(*exec.ExitError); ok { return !isNonFailureExitCode(exitErr.ExitCode()) } if exitCodeErr, ok := err.(*ErrorWithExitCode); ok { return !isNonFailureExitCode(exitCodeErr.ExitCode) } Lost Auxiliary Errors 🟠 [major] By guarding the call to processError with IsFailure(err), the logic now ignores the errorList (containing auxiliary errors like background network failures) whenever the main command returns a non-failure exit code (1 or 3). processError is responsible for merging err and errorList into a prioritized outputError. If skipped, outputError remains just the exit code, and any critical errors in errorList will neither influence the exit code nor be displayed to the user. if cli_errors.IsFailure(err) { allErrors, outputError = processError(err, errorList) }

📚 Repository Context Analyzed This review considered 11 relevant code sections from 9 files (average relevance: 0.88) cliv2/pkg/core/exitcode.go (lines 1-197) cliv2/pkg/core/errorhandling.go (lines 1-110) cliv2/pkg/core/logheaderfooter.go (lines 1-251) package.json (lines 1-236) src/cli/main.ts (lines 1-236) cliv2/cmd/cliv2/behavior/maperrortoexitcode.go (lines 1-31) cliv2/pkg/core/main.go (3 segments, lines 394-557) ... and 2 more file(s)

🤖 Repository instructions applied (from AGENTS.md)

[PeterSchafer]

PR Reviewer Guide 🔍

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

Brittle Type Assertion 🟠 [major]
The IsFailure function uses direct type assertions (err.(*exec.ExitError) and err.(*ErrorWithExitCode)) instead of errors.As. This will fail to correctly identify non-failure exit codes if the error has been wrapped (e.g., via fmt.Errorf("%w", ...)). Given that cliv2/pkg/core/exitcode.go uses errors.Join to combine exit codes with other errors, IsFailure will return true for these wrapped errors, leading to the same duplicate output issue this PR intends to fix.

if exitErr, ok := err.(*exec.ExitError); ok {
	return !isNonFailureExitCode(exitErr.ExitCode())
}
if exitCodeErr, ok := err.(*ErrorWithExitCode); ok {
	return !isNonFailureExitCode(exitCodeErr.ExitCode)
}

Lost Auxiliary Errors 🟠 [major]
By guarding the call to processError with IsFailure(err), the logic now ignores the errorList (containing auxiliary errors like background network failures) whenever the main command returns a non-failure exit code (1 or 3). processError is responsible for merging err and errorList into a prioritized outputError. If skipped, outputError remains just the exit code, and any critical errors in errorList will neither influence the exit code nor be displayed to the user.

if cli_errors.IsFailure(err) {
	allErrors, outputError = processError(err, errorList)
}

📚 Repository Context Analyzed

This review considered 11 relevant code sections from 9 files (average relevance: 0.88)

• cliv2/pkg/core/exitcode.go (lines 1-197)
• cliv2/pkg/core/errorhandling.go (lines 1-110)
• cliv2/pkg/core/logheaderfooter.go (lines 1-251)
• package.json (lines 1-236)
• src/cli/main.ts (lines 1-236)
• cliv2/cmd/cliv2/behavior/maperrortoexitcode.go (lines 1-31)
• cliv2/pkg/core/main.go (3 segments, lines 394-557)
  ... and 2 more file(s)

🤖 Repository instructions applied (from AGENTS.md)

both findings are intentional changes to fix the bug where non failures caused an error message to be printed.

[danskmt]

Here you are testing for ExitError and ErrorWithExitCode types, how do we guarantee we are addressing all possible types in this function? (e.g. suppose tomorrow we have ErrorWithExitCode2...)

I would suggest having a separate function for extracting this ExitCode based on the type, if possible

[PeterSchafer]

not sure if it is worth yet to do so. For now It probably is small enough.

[danskmt]

Suggestion: change the name of this function to reflect the returned value itself, e.g. isSuccessfulCode, sounds much better. (although I know to call SNYK_EXIT_CODE_UNSUPPORTED_PROJECTS as successful is debatable - wondering about when have no vulns found, is there any separate code?)
Reading a "not" / "non" in a function name is counter-intuitive

[danskmt]

here you are bumping snyk-docker-plugin, is there anything to fill in the PR description for the release automation about this change in "What's the product update that needs to be communicated to CLI users?"?

[PeterSchafer]

I will add this in a follow based on the teams feedback.

[danskmt]

Left some suggestions, mostly about naming. LGTM