fix: do not exit code 1 with invalid glibc #6342

j-luong · 2025-12-02T14:26:30Z

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)
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?

This PR fixes an issue where an unsupported glibc version will result in a generic error catalog error and an exit code 1 for some Snyk commands.

We will not exit code 2 and provide a more useful RequirementsNotMet error catalog error.

Low risk as it fixes an already existing exit condition by changing exit codes and messaging.

Where should the reviewer start?

How should this be manually tested?

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

Fixed an issue where the CLI would exit with the wrong exit code when certain runtime requirements are not met. The CLI will now exit correctly with an appropriate error message.

PeterSchafer · 2025-12-04T11:32:50Z

cliv2/internal/utils/checkrequirements.go

+// ValidateGlibcVersion checks if the glibc version is supported and returns an Error Catalog error if it is not.
+// This check only applies to glibc-based Linux systems (amd64, arm64).
+// Optionally accepts a custom GlibcVersion, mainly for testing.
+func ValidateGlibcVersion(opt ...GlibcVersion) error {


Suggestion: Move the glibc validation logic and constants close to the legacycli workflow, since the requirement comes from there. The logic to return the glibc version can be here.

PeterSchafer · 2025-12-04T11:38:51Z

cliv2/internal/utils/checkrequirements.go

+}
+
+// detectGlibcVersion attempts to detect the glibc version on Linux systems
+func detectGlibcVersion() (string, error) {


Suggestion: I think we do have plenty of examples on the structure of the ldd output, maybe we can improve the implementation by using this knowledge and adding tests for the different cases? Adding tests for the extraction logic would require it to be callable with a string though. But this would actually implement IOSP. So probably worth it.

PeterSchafer · 2025-12-10T16:30:35Z

cliv2/pkg/basic_workflows/legacycli.go

+	}
+
+	version, err := versionFn()
+	if err != nil {


Question: Do we really want to fail the validation, if there was an error in the version function? I would think this could be to strict and fail more often then we want to. IMO it would be better to not fail the validation if any intermediate step fails.

PeterSchafer · 2025-12-10T16:30:56Z

cliv2/pkg/basic_workflows/legacycli.go

+	}
+
+	res, err := utils.SemverCompare(version, minVersion)
+	if err != nil {


same question about failing as above

PeterSchafer · 2025-12-10T16:33:00Z

cliv2/internal/utils/helpers.go

+//	parseSemver("2.27") // returns []int{2, 27}
+//	parseSemver("2.31") // returns []int{2, 31}
+//	parseSemver("2.35") // returns []int{2, 35}
+func parseSemver(v string) ([]int, error) {


Suggestion: since semver is actually a bit more advanced than just 1.2.3 should we maybe use a library to do proper parsing and comparison?

depending on the decision, if we keep our own implementation, let's add some tests for parseSemver() and SemverCompare().

Will use golang.org/x/mod/semver package instead, which performs the same functionality and should be more robust

PeterSchafer · 2025-12-10T16:39:15Z

cliv2/internal/utils/glibcversion.go

+	"sync"
+)
+
+const (


Suggestion: Move this to legacycli, as it is defined by the nodejs runtime which is part of the legacycli workflow.

PeterSchafer · 2025-12-10T16:47:13Z

cliv2/internal/utils/helpers.go

+//	SemverCompare("2.27", "2.31") // returns -1
+//	SemverCompare("2.31", "2.31") // returns 0
+//	SemverCompare("2.35", "2.31") // returns 1
+func SemverCompare(v1 string, v2 string) (int, error) {


Question: It took me a while to fully understand the behaviour and the usage, so I'm wondering if a simpler function like SemverGreaterEquals() bool or SemverLessThan() bool is easier to understand.

Will use golang.org/x/mod/semver package for semver comparison instead

PeterSchafer · 2025-12-17T15:53:17Z

cliv2/pkg/basic_workflows/legacycli.go

+	}
+
+	var minVersion string
+	switch runtime.GOARCH {


Suggestion: If the architecture becomes a parameter to the function, tests can actually be written without depending on the platform architecture they are executed on.

PeterSchafer · 2025-12-17T16:09:18Z

cliv2/pkg/basic_workflows/legacycli.go

+// ValidateGlibcVersion checks if the glibc version is supported and returns an Error Catalog error if it is not.
+// This check only applies to glibc-based Linux systems (amd64, arm64).
+// Optionally accepts a custom GlibcVersion, mainly for testing.
+func ValidateGlibcVersion(debugLogger *zerolog.Logger, opt ...utils.GlibcVersion) error {


Suggestion: just use regular parameter for utils.GlibcVersion, debuglogger, runtime, goos, goarch and skip the optional parameter.

PeterSchafer · 2025-12-17T18:30:55Z

cliv2/pkg/basic_workflows/legacycli_test.go

+		expectedSnykErrCode string
+	}
+
+	tests := []test{


Suggestion: Let's also add some edge cases (like windows ...) to ensure they are correctly covered.

PeterSchafer · 2025-12-18T16:10:16Z

cliv2/internal/utils/helpers.go

+		v2 = "v" + v2
+	}
+
+	return semver.Compare(v1, v2)


Suggestion: Let's be a bit paranoid and check if both versions are actually valid using isValid()

just because the doc of Compare() says the following

An invalid semantic version string is considered less than a valid one. All invalid semantic version strings compare equal to each other.

github-actions · 2025-12-23T10:04:54Z

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

Generated by 🚫 dangerJS against ca11db1

j-luong force-pushed the fix/cli-1230_glibcChecker branch 2 times, most recently from 4027467 to 7546c5a Compare December 3, 2025 16:23

PeterSchafer reviewed Dec 4, 2025

View reviewed changes

j-luong force-pushed the fix/cli-1230_glibcChecker branch 6 times, most recently from 0b9dd34 to 20b3987 Compare December 9, 2025 19:59

j-luong marked this pull request as ready for review December 10, 2025 09:05

j-luong requested review from a team as code owners December 10, 2025 09:05

PeterSchafer reviewed Dec 10, 2025

View reviewed changes

j-luong force-pushed the fix/cli-1230_glibcChecker branch 3 times, most recently from d135904 to 8be1f1b Compare December 17, 2025 12:08

PeterSchafer reviewed Dec 17, 2025

View reviewed changes

j-luong force-pushed the fix/cli-1230_glibcChecker branch from 8be1f1b to 342a4ec Compare December 17, 2025 17:14

PeterSchafer reviewed Dec 17, 2025

View reviewed changes

j-luong force-pushed the fix/cli-1230_glibcChecker branch 3 times, most recently from 20f379e to f721656 Compare December 18, 2025 16:05

j-luong enabled auto-merge December 18, 2025 16:07

PeterSchafer reviewed Dec 18, 2025

View reviewed changes

j-luong disabled auto-merge December 18, 2025 16:10

j-luong force-pushed the fix/cli-1230_glibcChecker branch from f721656 to d9eab3d Compare December 18, 2025 16:13

PeterSchafer approved these changes Dec 18, 2025

View reviewed changes

fix: do not exit code 1 with invalid glibc

66fbb50

j-luong force-pushed the fix/cli-1230_glibcChecker branch from d9eab3d to 66fbb50 Compare December 18, 2025 16:19

j-luong enabled auto-merge December 18, 2025 16:21

fix: move test case that should fail validation

ca11db1

+              	"sync"
+              )
+              const (

fix: do not exit code 1 with invalid glibc #6342

Are you sure you want to change the base?

fix: do not exit code 1 with invalid glibc #6342

Uh oh!

Conversation

j-luong commented Dec 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Submission Checklist

What does this PR do?

Where should the reviewer start?

How should this be manually tested?

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

j-luong Dec 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Dec 23, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

j-luong commented Dec 2, 2025 •

edited

Loading

j-luong Dec 12, 2025 •

edited

Loading