-
Notifications
You must be signed in to change notification settings - Fork 2.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(go): use toolchain
as stdlib
version for go.mod
files
#7163
base: main
Are you sure you want to change the base?
feat(go): use toolchain
as stdlib
version for go.mod
files
#7163
Conversation
toolchain
as stdlib
version for go.mod
filestoolchain
as stdlib
version for go.mod
files
There is one discussion. Technically,
However, if the local version is older, the toolchain version is used for forward compatibility.
They can be configured through
@DmitriyLewen Do you think we should work in the same way? |
Hmm... I didn't look at that. This seems to be the same case as for the SDK in
Let's start from default logic. |
This is how govulncheck works.
But I'm still not sure this is a good approach. If the environment in which Trivy is run is different from the environment in which the project is built, the vulnerability may be overlooked. Also, people can be confused as a vulnerability may be detected or not detected even though they scan the same project. |
I thought that we don't want to run other apps in Trivy, even if it's
in this case, users will need to check (or just be mindful) of many things when scanning |
OK, it is better to use the toolchain version at the moment for reproducibility. Since it's technically different from how the Go version is selected for compiling a binary, we should document it. |
Yeah. I will update docs. |
@knqyf263 i updated docs. |
@@ -83,6 83,19 @@ func (p *Parser) Parse(r xio.ReadSeekerAt) ([]ftypes.Package, []ftypes.Dependenc | |||
skipIndirect = lessThan117(modFileParsed.Go.Version) | |||
} | |||
|
|||
// Stdlib | |||
if toolchain := modFileParsed.Toolchain; toolchain != nil { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since Go uses go
line if toolchain
is omitted, we probably need to check the go
line as well.
If the toolchain line is omitted, the module or workspace is considered to have an implicit toolchain goV line, where V is the Go version from the go line.
But we need to consider how to treat a go line omitting a patch version, like go 1.22
. I think we can skip stdlib in this case.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can skip stdlib in this case.
If module uses version without patch (and child modules don't use patch and toolchain
) - go
doesn't add patch/toolchain:
➜ cat ../greetings/go.mod
module github.com/greetings
go 1.22
➜ cat go.mod
module example.com/hello
go 1.22
replace example.com/greetings => ../greetings
require example.com/greetings v0.0.0-00010101000000-000000000000
➜ go version
go version go1.22.0 darwin/arm64
since we say we use minimum required version
for stdlib
- we can say that v1.x.0
(v1.21.0
for this example) is the minimum required version, no?
I think I'm missing something, but I can't figure out what 😄
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
or do you mean that if go
version doesn't contain patch - that means it is not a situation where toolchan
is omitted?
and we don't need to check for cases where toolchain
is not used (or omitted).
But it doesn't work for v1.19
or early: The standard Go toolchains are named goV where V is a Go version denoting a beta release, release candidate, or release. For example, go1.21rc1 and go1.21.0 are toolchain names; go1.21 and go1.22 are not (the initial releases are go1.21.0 and go1.22.0), but go1.20 and go1.19 are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can say that v1.x.0 (v1.21.0 for this example) is the minimum required version, no?
I found answer - 1.21
!= 1.21.0
:
For example, 1.21 < 1.21rc1 < 1.21rc2 < 1.21.0 < 1.21.1 < 1.21.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@knqyf263 I updated this PR:
- if toolchain is omitted - check
go
line- check go version (take only >= 1.21)
- check patch
- check
rc
releases
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm... I like this solution.
This flag will force users to carefully read docs and investigate result of Trivy.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detection using the minimum version may be better enabled when this flag is used. For example, django>=3.0.0
in requirements.txt, we can take 3.0.0
as the version even if the project may use newer than 3.0.0. The toolchain version is the same. From toolchain go1.21.4
in go.mod, we consider it Go 1.21.4 even if the project may actually use Go 1.21.5.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Detection using the minimum version may be better enabled when this flag is used.
Agree with you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I've converted this PR to draft. Let's finalize this proposal first and come back here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
django>=3.0.0 in requirements.txt, we can take 3.0.0 as the version even if the project may use newer than 3.0.0
this is good idea 👍
Description
Use
toolchain
asstdlib
version.➜ cat ./go.mod module github.com/aquasecurity/trivy go 1.22.0 toolchain go1.22.4 ➜ trivy -q fs ./go.mod go.mod (gomod) Total: 1 (UNKNOWN: 0, LOW: 0, MEDIUM: 1, HIGH: 0, CRITICAL: 0) ┌─────────┬────────────────┬──────────┬────────┬───────────────────┬─────────────────┬──────────────────────────────────────────────────────────┐ │ Library │ Vulnerability │ Severity │ Status │ Installed Version │ Fixed Version │ Title │ ├─────────┼────────────────┼──────────┼────────┼───────────────────┼─────────────────┼──────────────────────────────────────────────────────────┤ │ stdlib │ CVE-2024-24791 │ MEDIUM │ fixed │ 1.22.4 │ 1.21.12, 1.22.5 │ net/http: Denial of service due to improper 100-continue │ │ │ │ │ │ │ │ handling in net/http │ │ │ │ │ │ │ │ https://avd.aquasec.com/nvd/cve-2024-24791 │ └─────────┴────────────────┴──────────┴────────┴───────────────────┴─────────────────┴──────────────────────────────────────────────────────────┘
Related issues
Checklist