gomod: Update transitive dependencies (with security vulnerabilities)

[eriksw]

What would you like Renovate to be able to do?

Renovate should update dependencies with security vulnerabilities even if they have the // indirect comment in go.mod.

The current behavior of totally ignoring dependencies that have the // indirect comment means that in this reproduction:

https://github.com/eriksw/renovate-gomod-insecure-transitive

Renovate should have opened a PR to update github.com/tidwall/gjson per CVE-2021-42836.

This is followup to #9578 (comment)

If you have any ideas on how this should be implemented, please tell us here.

Consider reverting #4650 in favor of something like what was proposed in #4586 (comment)

Is this a feature you are interested in implementing yourself?

Maybe

[eriksw]

Important note: Unless the go version directive in go.mod is 1.17 or later, it is incorrect to assume that transitive dependencies will be present in go.mod. See the alt branch of the repro at https://github.com/eriksw/renovate-gomod-insecure-transitive/tree/alt

If you run go mod tidy while the go version directive is present with anything older than 1.17 it won't populate the transitive dependencies ingo.mod. You can work around this with the following:

• If there is a go version directive in go.mod and it is 1.17 or higher, run go mod tidy
• If there is no go version directive or it is anything older than 1.17, run go mod tidy -go=1.17

After running whichever command is applicable, you can assume that parsing go.mod will tell you about all transitive dependencies, and that the // indirect comments will be accurately present/absent as appropriate. (You will of course want to git reset these changes away after parsing the resulting go.mod, this is all just to get go to do the work of listing and labeling indirect dependencies for you.)

(This matters because there are many projects out there that maintain support for old go versions, so their version directive will be old if not absent entirely!)

[rarkins]

I would prefer if we could (a) support updating the directive in go.mod, and (b) write this feature to assume 1.17 or later, after which it sounds like we can assume all transitive dependencies are in the go.mod?

[eriksw]

I don't think it's appropriate for Renovate to raise the go version directive, especially not by default, because that would cause problems for the many projects that maintain support for older versions of go.

• 84K go.mod with 'go 1.13'
• 32K go.mod with 'go 1.14'
• 124K go.mod with 'go 1.15'
• 81K go.mod with 'go 1.16'

Bumping the go version directive would be a disruptive change for any of those that are libraries with downstream users, so it's not safe to do by default.

However, ignoring security issues in transitive dependencies is also a surprising foot-gun. I believe updating insecure transitive dependencies should be in the default/base config.

Thus, my suggestion to do a throwaway upgrade of the directive for the purpose of producing a go.mod that you can trust, then reverting it away after parsing it to learn the full set of dependencies.

---

Scratch that, just noticed a better way! (Still posting it because I think there should be some recorded notes about why bumping the go version directive is not a good thing.)

Don't parse go.mod, parse the output of go mod graph instead!

➜  renovate-gomod-insecure-transitive git:(alt) cat go.mod
module github.com/eriksw/renovate-gomod-insecure-transitive

go 1.13

require github.com/fifsky/goconf v1.0.1
➜  renovate-gomod-insecure-transitive git:(alt) go mod graph
github.com/eriksw/renovate-gomod-insecure-transitive github.com/fifsky/goconf@v1.0.1
github.com/fifsky/goconf@v1.0.1 github.com/ilibs/json5@v1.0.0
github.com/fifsky/goconf@v1.0.1 github.com/kylelemons/godebug@v0.0.0-20170820004349-d65d576e9348
github.com/fifsky/goconf@v1.0.1 github.com/pkg/errors@v0.8.0
github.com/fifsky/goconf@v1.0.1 github.com/robertkrimen/otto@v0.0.0-20180617131154-15f95af6e78d
github.com/fifsky/goconf@v1.0.1 github.com/tidwall/gjson@v1.1.3
github.com/fifsky/goconf@v1.0.1 github.com/tidwall/match@v0.0.0-20171002075945-1731857f09b1
github.com/fifsky/goconf@v1.0.1 gopkg.in/sourcemap.v1@v1.0.5
➜  renovate-gomod-insecure-transitive git:(alt)

Caution: If you run go mod graph while go.sum is missing or if it's missing any entries, it will create/modify go.sum.

➜  renovate-gomod-insecure-transitive git:(alt) git checkout alt2
Switched to branch 'alt2'
Your branch is up to date with 'origin/alt2'.
➜  renovate-gomod-insecure-transitive git:(alt2) go mod graph > /dev/null
➜  renovate-gomod-insecure-transitive git:(alt2) ✗ git status
On branch alt2
Your branch is up to date with 'origin/alt2'.

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	go.sum

nothing added to commit but untracked files present (use "git add" to track)
➜  renovate-gomod-insecure-transitive git:(alt2) ✗

[rarkins]

I disagree that Renovate should not offer to update the go.mod directive. Proposing upgrades is what Renovate does, especially when versions are end of life.

Our goal would be to support semantics like: "Keep my libraries bumped to support non-EOL go versions, and nothing more".

Don't forget that users of Renovate have a choice about whether to accept an update, or to suppress it by default to be listed in the Dependency Dashboard for creation on demand.

We are also not going to do this:

Don't parse go.mod, parse the output of go mod graph instead!

Our parsing is now JS/TS-based, and "shelling out" to CLI tools like go mod graph for parsing in the past caused problems every time and we have learned from that lesson.

[eriksw]

Our parsing is now JS/TS-based, and "shelling out" to CLI tools like go mod graph for parsing in the past caused problems every time and we have learned from that lesson.

In light of that constraint, having Renovate bump to 1.17 and then presuming the go.mod to be complete (per behavior of 1.17) seems fine.

I think you would have to be manually modifying go.mod (eschewing use of the >=1.17 go tool to do dependency management) to wind up with an incomplete go.mod after the update to 1.17 is done.

I disagree that Renovate should not offer to update the go.mod directive. Proposing upgrades is what Renovate does, especially when versions are end of life.

Our goal would be to support semantics like: "Keep my libraries bumped to support non-EOL go versions, and nothing more".

This means supporting pre-1.17 until go 1.19 is out.

Don't forget that users of Renovate have a choice about whether to accept an update, or to suppress it by default to be listed in the Dependency Dashboard for creation on demand.

Good point!

I do think that in light of the library situation, some caution is probably warranted with regards to automerge. People may not be expecting adding the :automergePr preset to cause their go version directive to be auto-merged.

I think it would be important that the PR for the update to go1.17 include clear messaging about why the bump is necessary in order to allow Renovate to monitor indirect dependencies. Using go mod tidy -go=... to do the update.

Previewing what subsequent indirect dependency updates would be opened (akin to the preview in the onboarding PR) would be nice.

[eriksw]

What are the next steps to get this moved beyond "status: Requirements" to something that might get implemented?

[rarkins]

I think this discussion ballooned into something that's difficult to mark as ready. Perhaps better to narrow requirements - possibly in a separate feature request - that Renovate update indirect dependencies, as this would be the starting point.

[eriksw]

Consider reverting #4650 in favor of something like what was proposed in #4586 (comment)

Is this not actionable?

[rarkins]

No.

[blitz]

@rarkins Maybe we can continue the discussion in #15753? If this is noise and just duplicates the discussion, feel free to close it. :)