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.

[VirrageS]

@rarkins

Can I ask why

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

is not actionable?

If someone would like to allow update only vulnerable packages both direct and indirect I would assume that:

{
  "$schema": "https://docs.renovatebot.com/renovate-schema.json",
  "extends": ["config:base"],
  "packageRules": [{
    "enabled": false,
    "managers": ["gomod"],
    "depTypeList": ["indirect"]
  }],
  "vulnerabilityAlerts": {
    "labels": ["dependencies"],
    "automerge": false
  },
  "osvVulnerabilityAlerts": true
}

would work. But it doesn't. I was unpleasantly surprised that there is such a gap in the renovate. What would be the problem with also considering indirect dependencies for vulnerabilities?

[rarkins]

First of all, your "unpleasantly surprised" phrase comes across as rude, so please take more care if it was unintentional, or be aware that it was noticed and not appreciated if it was intentional.

The proposal was not actionable because it's a single sentence and not enough to mark the feature as ready to start work on. This issue was weighed down both by lack of detail in suggestions as well as stretching into adjacent topics such as Go version upgrading, so I've now converted it into a GitHub Discussion. Your expectation that you can do only vulnerability updating of transitive is another example of straying from the original request. While everyone wants to tack on their pet topic, it can go nowhere.

Additionally, I'm not sure what gaps even remain. Since the issue was created, we saw #19431 implement indirect dependency updating and another PR add osv updating, which you have seen. I would like to hear how Renovate now goes on a reproduction repo when both transitive dependency updating as well as OSV alerts are enabled.

[VirrageS]

First of all, your "unpleasantly surprised" phrase comes across as rude, so please take more care if it was unintentional, or be aware that it was noticed and not appreciated if it was intentional.

It wasn't personal. Simply expected something to wok, but it didn't. So unpleasant surprise 😅. I really appreciate all the work that is being done in this project. I will be more careful with the wording.

I would like to hear how Renovate now goes on a reproduction repo when both transitive dependency updating as well as OSV alerts are enabled.

With the config I have posted it still doesn't work. Maybe that's because I only want to update vulnerable dependencies thus "enabled": false. I came across #15490 (reply in thread) which is similar case but without transitive/indirect dependencies. That's why I thought my problem might be relevant to the comment #4586 (comment) and that's something that is needed.

If you feel like this is separate issue I'm happy to create one.

[rarkins]

Using the original reproduction repository and the right config, this seems to now work: renovate-reproductions/22487#4

What it doesn't support is only vulnerability fixes for transitive. In other words you need to turn on all transitive updates.

[phisco]

Hi!
I'm trying to do exactly the same thing for crossplane.

So, there is no way currently to have direct dependencies updated if and only if either them or any of their dependencies is affected by a vulnerability, right?

[rarkins]

For now that's right. Please start a new discussion describing/clarifying the use case. This one went too long already

[phisco]

Done 🙏 #22505

[maxbrunet]

The security:gomodIndirectSecurityUpdates preset can now be used to opt into security updates of indirect dependencies (PR #43222)