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

cmd/go: install with explicit version should allow overriding versions of dependencies #44892

Closed
jayconrod opened this issue Mar 9, 2021 · 8 comments
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone

Comments

@jayconrod
Copy link
Contributor

This issue is based on a discussion with @rsc around https://blog.golang.org/path-security.

Let's say I want to install a command with go install example.com/cmd@latest. I know that the latest version of that command depends on a version of golang.org/x/tools that has a security vulnerability, so I actually want to build example.com/cmd with a different version of golang.org/x/tools where the vulnerability is fixed.

go install example.com/cmd@latest golang.org/x/tools@v0.1.0

In Go 1.16, this is an error. go install cmd@version requires that all arguments refer to main packages in the same module, at the same version. We could relax this a bit: if an argument refers to non-main packages (perhaps just a module path that's not a package, as above), that argument is used in version selection.

In other words, the above command would do the same thing as:

cd $HOME  # or any directory without go.mod
go get example.com/cmd@latest golang.org/x/tools@v0.1.0

The rules here should also apply to go run example.com/cmd@version and other commands if/when we support them.

cc @bcmills @matloob

@jayconrod jayconrod added NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. GoCommand cmd/go modules labels Mar 9, 2021
@jayconrod jayconrod added this to the Go1.17 milestone Mar 9, 2021
@mvdan
Copy link
Member

mvdan commented Mar 9, 2021

if an argument refers to non-main packages (perhaps just a module path that's not a package, as above), that argument is used in version selection.

What if an extra argument results in no changes in version selection for the main package to be built? For example, go install example.com/cmd@latest github.com/user/totally-unrelated-module@v0.2.0 should error instead of silently doing the same as go install example.com/cmd@latest, I imagine.

@jayconrod
Copy link
Contributor Author

What if an extra argument results in no changes in version selection for the main package to be built?

It seems well-formed, so I think that should be allowed, even if it isn't necessarily useful. It might make it easier to install a bunch of commands from different modules:

cmds=(example.com/m1/cmd example.com/m2/cmd)
for cmd in ${cmds[@]}; do
  go install $cmd@latest golang.org/x/tools@v0.1.0
done

The extra module dependency would still influence the go version -m output, though that's probably not important here.

@mvdan
Copy link
Member

mvdan commented Mar 9, 2021

My wording of "results in no changes in version selection" was also poor. What if $cmd@latest already uses an x/tools version newer than v0.1.0? We don't want the command to start erroring simply because the x/tools argument results in no dependency version upgrades.

I think what I meant is: reject arguments which can't possibly affect version selection for the main module, such as modules which are not in the module graph of the main module.

That said, I think you're right that it seems too restrictive. It could also cause problems if $cmd@latest has x/tools in the module graph today, but a new version comes out today which no longer has x/tools in the module graph. We don't want go install to start failing even though the build would still work.

@jayconrod
Copy link
Contributor Author

Some comments from the golang-tools meeting earlier today:

  • The story about upgrading past a security vulnerability when building a command doesn't resonate that well. Command authors should be responsible for upgrading quickly; that responsibility shouldn't be on their users.
  • A command line like go install example.com/cmd@latest golang.org/x/tools@v0.1.0 is a bit confusing, since golang.org/x/tools isn't actually being installed or even built.

@bcmills
Copy link
Contributor

bcmills commented Mar 10, 2021

I also worry about package/module ambiguity. The arguments to go get and go install are packages, but go get also allows arguments that are module paths.

Users often try to upgrade dependencies by passing module paths to go get instead of package paths, and that actually mostly works as long as they run something afterward (such as go mod tidy) to populate checksums for transitive dependenices.

But if someone tries to do that with go install, and the module they're trying to upgrade happens to contain another binary at the module root, then the command could inadvertently run afoul of the restriction that main packages come from only one module, and I worry that the resulting error messages would be too confusing.

@dmitshur
Copy link
Contributor

dmitshur commented Mar 10, 2021

The story about upgrading past a security vulnerability when building a command doesn't resonate that well. Command authors should be responsible for upgrading quickly; that responsibility shouldn't be on their users.

Consider also that it'd make it possible for users to make a command vulnerable (accidentally or by being tricked).

Suppose that example.com/lib@v1.2.0 and example.com/lib@v1.3.0 have a vulnerability which is fixed in v1.2.1 and v1.3.1 patch releases. If example.org/cmd@v1.0.1 uses lib@v1.2.1, then go install example.org/cmd@v1.0.1 example.com/lib@v1.3.0 is problematic despite looking innocuous.

The same problem can happen when authors perform version selection, but this is to support that the responsibility of selecting versions is better suited for authors (fewer, goes through code review, vulnerability databases, etc.) than users (many, can be copy-pasted from anywhere).

@rsc
Copy link
Contributor

rsc commented Mar 23, 2021

I was pushing for this but based on the discussion it sounds like we should probably not do it.

@jayconrod
Copy link
Contributor Author

Withdrawing this issue, reflecting discussion above.

@golang golang locked and limited conversation to collaborators Mar 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Development

No branches or pull requests

6 participants