Skip to content
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

x/vuln/cmd/govulncheck: v1.1.0: Fatal error when used in a workspace #66863

Closed
nicholascapo opened this issue Apr 16, 2024 · 9 comments
Closed
Assignees
Labels
vulncheck or vulndb Issues for the x/vuln or x/vulndb repo WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.

Comments

@nicholascapo
Copy link

nicholascapo commented Apr 16, 2024

govulncheck version

Go: go1.22.2
Scanner: govulncheck@v1.1.0
DB: https://vuln.go.dev
DB updated: 2024-04-16 21:40:19 +0000 UTC

Does this issue reproduce at the latest version of golang.org/x/vuln?

Yes

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/home/nicholas/.cache/go-build'
GOENV='/home/nicholas/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/home/nicholas/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/home/nicholas/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/lib/go-1.22'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/lib/go-1.22/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='go1.22.2'
GCCGO='gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='g++'
CGO_ENABLED='1'
GOMOD='/dev/null'
GOWORK='/home/nicholas/src/demo/example/go.work'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build2849136563=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Construct a workspace like this:

$ tree
.
├── go.work
└── pkg
    ├── go.mod
    └── main.go

go.work

go 1.22.2

use ./pkg

pkg/go.mod

module example

go 1.22.2

pkg/main.go

package main

import "fmt"

func main() {
	fmt.Println("main")
}

Run govulncheck

$ govulncheck ./pkg/...

What did you see happen?

govulncheck: no go.mod file

govulncheck only works with Go modules. Try navigating to your module directory.
Otherwise, run go mod init to make your project a module.

See https://go.dev/doc/modules/managing-dependencies for more information.

What did you expect to see?

Scanning your code and 45 packages across 1 dependent module for known vulnerabilities...

No vulnerabilities found.
@nicholascapo nicholascapo added the vulncheck or vulndb Issues for the x/vuln or x/vulndb repo label Apr 16, 2024
@gopherbot gopherbot modified the milestones: Unreleased, vuln/unplanned Apr 16, 2024
@nicholascapo
Copy link
Author

nicholascapo commented Apr 16, 2024

Running git bisect between v1.0.4 (good) and v1.1.0 (bad):

$ git bisect start v1.1.0 v1.0.4
Bisecting: 21 revisions left to test after this (roughly 5 steps)
[d26ab060e8520ed8415aeee168d57f86257524f5] internal/vulncheck: delete only synthetic nodes not related to generics
$ git bisect run go run ./cmd/govulncheck -C ~/src/demo/example ./pkg/...
running 'go' 'run' './cmd/govulncheck' '-C' '/home/nicholas/src/demo/example' './pkg/...'
Scanning your code and 0 packages across 1 dependent module for known vulnerabilities...

No vulnerabilities found.
Bisecting: 10 revisions left to test after this (roughly 4 steps)
[df69562cba6dec55b47a7c5f05b1e5007ea1fda4] cmd/govulncheck: make fixup part of a test case
running 'go' 'run' './cmd/govulncheck' '-C' '/home/nicholas/src/demo/example' './pkg/...'
govulncheck: no go.mod file

govulncheck only works with Go modules. Try navigating to your module directory.
Otherwise, run go mod init to make your project a module.

See https://go.dev/doc/modules/managing-dependencies for more information.
exit status 1
Bisecting: 5 revisions left to test after this (roughly 3 steps)
[3c9f0482c172a445a80545c10165229fb3eed4b4] internal/sarif: add stacks
running 'go' 'run' './cmd/govulncheck' '-C' '/home/nicholas/src/demo/example' './pkg/...'
Scanning your code and 0 packages across 1 dependent module for known vulnerabilities...

No vulnerabilities found.
Bisecting: 2 revisions left to test after this (roughly 2 steps)
[086dd732aa1ad2b6579f98f6719a1a4bf6ad07b5] cmd/govulncheck: add nogomod test case
running 'go' 'run' './cmd/govulncheck' '-C' '/home/nicholas/src/demo/example' './pkg/...'
govulncheck: no go.mod file

govulncheck only works with Go modules. Try navigating to your module directory.
Otherwise, run go mod init to make your project a module.

See https://go.dev/doc/modules/managing-dependencies for more information.
exit status 1
Bisecting: 0 revisions left to test after this (roughly 1 step)
[483612e84764b2ba8cfe88f8e0f390fe4ae9f3be] cmd/govulncheck: restructure testdata tests
running 'go' 'run' './cmd/govulncheck' '-C' '/home/nicholas/src/demo/example' './pkg/...'
Scanning your code and 0 packages across 1 dependent module for known vulnerabilities...

No vulnerabilities found.
086dd732aa1ad2b6579f98f6719a1a4bf6ad07b5 is the first bad commit
commit 086dd732aa1ad2b6579f98f6719a1a4bf6ad07b5
Author: Zvonimir Pavlinovic <zpavlinovic@google.com>
Date:   Tue Mar 12 20:24:42 2024 +0000

    cmd/govulncheck: add nogomod test case
    
    Change-Id: Ie5c9afb08d30568f02a96468ad857e55ee362a4e
    Reviewed-on: https://go-review.googlesource.com/c/vuln/+/571116
    Reviewed-by: Maceo Thompson <maceothompson@google.com>
    Run-TryBot: Zvonimir Pavlinovic <zpavlinovic@google.com>
    LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
    TryBot-Result: Gopher Robot <gobot@golang.org>

 cmd/govulncheck/main_test.go                       |  49 +++++++--------
 cmd/govulncheck/test_utils.go                      |  66 +++++++++++++++++++++
 .../testdata/common/modules/nogomod/vuln.go        |  12 ----
 .../common/testfiles/failures/source_fail.ct       |  10 ----
 .../testdata/nogomod/modules/nogomod/vuln.go       |  12 ++++
 .../nogomod/testfiles/failures/source_fail.ct      |   9 +++
 .../testdata/nogomod/vulndb-v1/index/db.json       |   0
 .../testdata/nogomod/vulndb-v1/index/db.json.gz    | Bin 0 -> 28 bytes
 .../testdata/nogomod/vulndb-v1/index/modules.json  |   0
 .../nogomod/vulndb-v1/index/modules.json.gz        | Bin 0 -> 33 bytes
 .../testdata/nogomod/vulndb-v1/index/vulns.json    |   0
 .../testdata/nogomod/vulndb-v1/index/vulns.json.gz | Bin 0 -> 31 bytes
 internal/scan/util.go                              |   3 +-
 13 files changed, 110 insertions(+), 51 deletions(-)
 create mode 100644 cmd/govulncheck/test_utils.go
 delete mode 100644 cmd/govulncheck/testdata/common/modules/nogomod/vuln.go
 create mode 100644 cmd/govulncheck/testdata/nogomod/modules/nogomod/vuln.go
 create mode 100644 cmd/govulncheck/testdata/nogomod/testfiles/failures/source_fail.ct
 create mode 100644 cmd/govulncheck/testdata/nogomod/vulndb-v1/index/db.json
 create mode 100644 cmd/govulncheck/testdata/nogomod/vulndb-v1/index/db.json.gz
 create mode 100644 cmd/govulncheck/testdata/nogomod/vulndb-v1/index/modules.json
 create mode 100644 cmd/govulncheck/testdata/nogomod/vulndb-v1/index/modules.json.gz
 create mode 100644 cmd/govulncheck/testdata/nogomod/vulndb-v1/index/vulns.json
 create mode 100644 cmd/govulncheck/testdata/nogomod/vulndb-v1/index/vulns.json.gz
bisect found first bad commit

086dd732aa1ad2b6579f98f6719a1a4bf6ad07b5

@nicholascapo
Copy link
Author

Happy to provide more details if needed.

We have a workspace at the root of the (private) repo and several packages and services in sub-directories, so use govulncheck ./path/to/packages/... ./path/to/service/... ./path/to/service/... to scan everything at once.
We have been using this form of the command since 2022-09-16, and it's been working fine (except when it was broken in v1.0.2).

@nicholascapo nicholascapo changed the title x/vuln/cmd/govulncheck: v1.1.0 error in workspace x/vuln/cmd/govulncheck: v1.1.0 Fatal error when used in a workspace Apr 17, 2024
@nicholascapo nicholascapo changed the title x/vuln/cmd/govulncheck: v1.1.0 Fatal error when used in a workspace x/vuln/cmd/govulncheck@v1.1.0: Fatal error when used in a workspace Apr 17, 2024
@zpavlinovic
Copy link
Contributor

The current behavior is in fact the intended one. The previous version had a slight bug addressed by the commit you referenced.

I understand this introduces an inconvenience, but you could accomplish the same simply with

govulncheck -C ./vuln ./...

Would this solution work for you?

@zpavlinovic zpavlinovic self-assigned this Apr 17, 2024
@nicholascapo
Copy link
Author

Ah, maybe my example should have included more modules.

We actually have five modules listed in our private go.work with some imports across modules.
If starting in a module (instead of a workspace) is the expected use, I would need to run govulncheck five times, correct?

@zpavlinovic
Copy link
Contributor

zpavlinovic commented Apr 17, 2024

Yes, correct. But you'd have to do that with earlier versions of govulncheck too, I believe.

I've tested the behavior of govulncheck@v1.0.4 at the root of a workspace with several modules and got this:

govulncheck ./...

There are errors with the provided package patterns:

-: pattern ./...: directory prefix . does not contain modules listed in go.work or their selected dependencies

For details on package patterns, see https://pkg.go.dev/cmd/go#hdr-Package_lists_and_patterns.

So I guess you had to do something like

govulncheck ./pk1/...
govulncheck ./pkg2/...
...

Note that calling govulncheck separately can actually be beneficial. It can produce more accurate results. (The static analysis under the hood is inherently over-approximate and, if merging information from different modules together, could produce more inaccurate results)

@seankhliao seankhliao changed the title x/vuln/cmd/govulncheck@v1.1.0: Fatal error when used in a workspace x/vuln/cmd/govulncheck: v1.1.0: Fatal error when used in a workspace Apr 18, 2024
@nicholascapo
Copy link
Author

My point is that we have been using it like this since 2022-09-16 and we didn't expect a breaking change in a post-v1 minor release.
Previous to v1.1.0 govulncheck ./pkg1/... ./pkg2/... worked fine (note that using ./... doesn't work).

@zpavlinovic
Copy link
Contributor

I see. We'll address the issue. However, we highly recommend that you invoke govulncheck multiple times for more accurate results. We'll also add this recommendation to the docs.

@zpavlinovic
Copy link
Contributor

zpavlinovic commented May 8, 2024

Going back to this, how big of an issue is if we disallow multi patterns?

I understand that this feature was enabled (by accident) in previous versions, but it really does not benefit anyone. Not the user, not the maintainers. With multi-patterns, users will get less precise results and the maintainers more complex code base.

@zpavlinovic zpavlinovic added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jun 3, 2024
@gopherbot
Copy link
Contributor

Timed out in state WaitingForInfo. Closing.

(I am just a bot, though. Please speak up if this is a mistake or you have the requested information.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vulncheck or vulndb Issues for the x/vuln or x/vulndb repo WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

3 participants