Repository navigation
fix(go): preserve Echo versions in go.mod - #11438
LaizaAngrest wants to merge 3 commits into
Conversation
ab7a13d to
f4afbc2
Compare
| // parseModFile preserves the existing parser behavior for non-Echo input. | ||
| // | ||
| // A non-nil VersionFixer is also applied to retract directives, where x/mod | ||
| // deliberately permits version queries that are not semantic versions. Try the | ||
| // strict Echo-aware parse first so valid files take the normal single-pass path. | ||
| // If it fails, use the default parser to distinguish a dependency error from a | ||
| // tolerated retract query, then retry with a fixer that leaves such queries | ||
| // unchanged. | ||
| func parseModFile(data []byte) (*modfile.File, error) { | ||
| parsed, err := modfile.Parse("go.mod", data, canonicalizeVersion) | ||
| if err == nil { | ||
| return parsed, nil | ||
| } | ||
|
|
||
| baseline, baselineErr := modfile.Parse("go.mod", data, nil) | ||
| if baselineErr != nil { | ||
| return nil, baselineErr | ||
| } | ||
|
|
||
| // x/mod can't apply any VersionFixer to retractions without a module path. | ||
| // Keep the previous result for this already non-standard file shape. | ||
| if baseline.Module == nil && len(baseline.Retract) != 0 { | ||
| return baseline, nil | ||
| } | ||
|
|
||
| return modfile.Parse("go.mod", data, canonicalizeVersionOrKeep) | ||
| } | ||
|
|
||
| // canonicalizeVersion preserves valid Echo versions and applies the standard Go | ||
| // module canonicalization to every other version. | ||
| func canonicalizeVersion(path, version string) (string, error) { | ||
| if echoVersionSuffixRe.MatchString(version) && semver.IsValid(version) { | ||
| return version, nil | ||
| } | ||
|
|
||
| canonical := module.CanonicalVersion(version) | ||
| if canonical == "" { | ||
| return "", module.VersionError(module.Version{Path: path, Version: version}, &module.InvalidVersionError{ | ||
| Version: version, | ||
| Err: errors.New("must be of the form v1.2.3"), | ||
| }) | ||
| } | ||
| return canonical, nil | ||
| } | ||
|
|
||
| // canonicalizeVersionOrKeep is used only after the default parser has validated | ||
| // all dependency versions. An unrecognized version can therefore be retained as | ||
| // a retract query, matching the behavior of modfile.Parse with a nil fixer. | ||
| func canonicalizeVersionOrKeep(path, version string) (string, error) { | ||
| canonical, err := canonicalizeVersion(path, version) | ||
| if err != nil { | ||
| return version, nil | ||
| } | ||
| return canonical, nil | ||
| } |
There was a problem hiding this comment.
I think the logic can be simplified quite a bit.
Right now the fixer is passed for every go.mod, which makes x/mod run fixRetract on every file, and that's why the retries, canonicalizeVersionOrKeep and the special case for files without module are needed.
How about this:
- always do the first parse with
nil, as today: all validation and errors stay the same, and files without+echo.stop there; - for files with
+echo., blank outretractdirectives with spaces (Trivy doesn't use them, and x/mod gives their positions in the original bytes, so offsets don't shift) and parse again with the fixer.
This keeps version queries in retract (master, [master, dev]), the block form retract (...), and files without module working.
All tests from this PR pass with it.
| // parseModFile preserves the existing parser behavior for non-Echo input. | |
| // | |
| // A non-nil VersionFixer is also applied to retract directives, where x/mod | |
| // deliberately permits version queries that are not semantic versions. Try the | |
| // strict Echo-aware parse first so valid files take the normal single-pass path. | |
| // If it fails, use the default parser to distinguish a dependency error from a | |
| // tolerated retract query, then retry with a fixer that leaves such queries | |
| // unchanged. | |
| func parseModFile(data []byte) (*modfile.File, error) { | |
| parsed, err := modfile.Parse("go.mod", data, canonicalizeVersion) | |
| if err == nil { | |
| return parsed, nil | |
| } | |
| baseline, baselineErr := modfile.Parse("go.mod", data, nil) | |
| if baselineErr != nil { | |
| return nil, baselineErr | |
| } | |
| // x/mod can't apply any VersionFixer to retractions without a module path. | |
| // Keep the previous result for this already non-standard file shape. | |
| if baseline.Module == nil && len(baseline.Retract) != 0 { | |
| return baseline, nil | |
| } | |
| return modfile.Parse("go.mod", data, canonicalizeVersionOrKeep) | |
| } | |
| // canonicalizeVersion preserves valid Echo versions and applies the standard Go | |
| // module canonicalization to every other version. | |
| func canonicalizeVersion(path, version string) (string, error) { | |
| if echoVersionSuffixRe.MatchString(version) && semver.IsValid(version) { | |
| return version, nil | |
| } | |
| canonical := module.CanonicalVersion(version) | |
| if canonical == "" { | |
| return "", module.VersionError(module.Version{Path: path, Version: version}, &module.InvalidVersionError{ | |
| Version: version, | |
| Err: errors.New("must be of the form v1.2.3"), | |
| }) | |
| } | |
| return canonical, nil | |
| } | |
| // canonicalizeVersionOrKeep is used only after the default parser has validated | |
| // all dependency versions. An unrecognized version can therefore be retained as | |
| // a retract query, matching the behavior of modfile.Parse with a nil fixer. | |
| func canonicalizeVersionOrKeep(path, version string) (string, error) { | |
| canonical, err := canonicalizeVersion(path, version) | |
| if err != nil { | |
| return version, nil | |
| } | |
| return canonical, nil | |
| } | |
| // parseModFile parses go.mod and keeps Echo versions (e.g. v0.55.0+echo.1), | |
| // which modfile.Parse otherwise strips as build metadata. | |
| func parseModFile(data []byte) (*modfile.File, error) { | |
| f, err := modfile.Parse("go.mod", data, nil) | |
| // The default parse validates every version, so its error is returned as is. | |
| // Only files with Echo versions need the second parse below. | |
| if err != nil || !bytes.Contains(data, []byte("+echo.")) { | |
| return f, err | |
| } | |
| // A VersionFixer also applies to retract directives, which Trivy doesn't use | |
| // and which may contain version queries, so blank them out before re-parsing. | |
| data = bytes.Clone(data) | |
| for _, r := range f.Retract { | |
| for i := r.Syntax.Start.Byte; i < r.Syntax.End.Byte; i++ { | |
| data[i] = ' ' | |
| } | |
| } | |
| return modfile.Parse("go.mod", data, fixEchoVersion) | |
| } | |
| // fixEchoVersion keeps Echo versions and canonicalizes all others, | |
| // matching what modfile.Parse does with a nil VersionFixer. | |
| func fixEchoVersion(path, version string) (string, error) { | |
| if echoVersionSuffixRe.MatchString(version) && semver.IsValid(version) { | |
| return version, nil | |
| } | |
| if canonical := module.CanonicalVersion(version); canonical != "" { | |
| return canonical, nil | |
| } | |
| return "", xerrors.Errorf("invalid version %q of %s: must be of the form v1.2.3", version, path) | |
| } |
The imports need bytes added and errors removed.
About the tests: we don't need separate test functions, let's try to fit everything into TestParse cases.
For example, add testdata/echo/go.mod with Echo versions in require and on both sides of replace, a replace version mismatch, ordinary build metadata, a v2 module, and a retract block with a version query, and put the expected packages and dependencies in parse_testcase.go like the other cases.
With this approach, TestParsePreservesModulelessRetract and TestParseRejectsInvalidDependencyVersions only cover existing behavior (the file takes the same nil path as before), so they can be removed.
The table test for fixEchoVersion can stay.
Regards, Dmitriy
There was a problem hiding this comment.
Thanks, Dmitriy. I applied the suggested two-pass approach: the default parser now runs first for every file, and only files containing +echo. are reparsed after retract syntax ranges are blanked. I also moved the Echo coverage into the existing TestParse fixture/table and kept the focused fixEchoVersion table test. The fixture covers require, both sides of replace, a replacement-version mismatch, ordinary build metadata, a v2 module, and block-form retract version queries. I rebased onto current main and reran the parser/analyzer tests, race tests, vet, golangci-lint, go mod tidy, and a local Trivy filesystem scan. All passed, and the scan retained the exact +echo.N versions and PURLs. Could you please take another look?
Keep valid versions ending in +echo.N intact during modfile parsing while preserving standard canonicalization and invalid-version errors for every other version. Signed-off-by: Cursor <cursoragent@cursor.com> Signed-off-by: Laiza Angrest <laiza.angrest@echo.ai> Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
23197f6 to
2df95eb
Compare
Description
Preserve valid Go module versions carrying a canonical Echo build suffix while parsing
go.modfiles.nilVersionFixerfirst, preserving existing validation and errors+echo., after blankingretractdirectives that Trivy does not consume+echo.0or+echo.[1-9][0-9]*+echo.01, matching the Echo toolchainmodule.CanonicalVersionfor every other dependency versionrequire, both sides ofreplace, replacement-version mismatches, retract version queries, v2 module paths, and ordinary build metadataNon-Echo files stay on the existing single-pass parser path. Their canonicalization, validation, and errors remain unchanged.
Scope
This is the independent parser part of Echo Go module support. It intentionally does not enable Go in the Echo supplier or add a temporary dependency replacement. The comparison API landed in aquasecurity/go-version#19 and will be wired into Trivy separately after a tagged go-version release is available.
Related
WithBuildMetadata) go-version#18Verification
go test -count=1 ./pkg/dependency/parser/golang/... ./pkg/fanal/analyzer/language/golang/mod/...go test -race -count=1 ./pkg/dependency/parser/golang/mod/...go vet ./pkg/dependency/parser/golang/mod/... ./pkg/fanal/analyzer/language/golang/mod/...golangci-lint v2.14.0 run ./pkg/dependency/parser/golang/mod/... ./pkg/fanal/analyzer/language/golang/mod/...go mod tidyleaves the module files unchanged+echo.Nversions and encoded PURLs through the full analyzer pathChecklist
replacedirective is included.