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

Import go-header from new repo location #2549

Closed

Conversation

aelse
Copy link

@aelse aelse commented Feb 3, 2022

It seems that github.com/denis-tingajkin/go-header has moved to github.com/denis-tingaikin/go-header. The denis-tingajkin account no longer exists and a redirect leads you to the new location when importing the package.

This change updates the package import path and version to avoid a possible account takeover vulnerability on the no-longer existing account name. Someone has registered and deleted the old account to trigger github's limited time protection where no one can takeover that account and in this time dependencies need to be updated in packages importing from github.com/denis-tingajkin

The most recent tagged version in the new location still has the old package path, which go get objects to. I've opened an issue on the repository to publish a new version. denis-tingaikin/go-header#21

In the mean time this pulls in the version HEAD and does a rewrite for the tools package which has a transitive dependency via an older published version of golangci-lint itself.

In the future a newly published version of the dependency and subsequent upgrade in golangci-lint's go.mod and update to the tools package to use a newer golangci-lint will fully resolve this.

@boring-cyborg
Copy link

boring-cyborg bot commented Feb 3, 2022

Hey, thank you for opening your first Pull Request !

@CLAassistant
Copy link

CLAassistant commented Feb 3, 2022

CLA assistant check
All committers have signed the CLA.

@ldez
Copy link
Member

ldez commented Feb 4, 2022

Hello,

The module name doesn't have to follow the repo name.
The module name is github.com/denis-tingajkin/go-header so it's right from the point of view of Go.
The module is right and doesn't have to be changed but the import comments can be added to enforce the difference between the repo and the module.
ex: package goheader // import "github.com/denis-tingajkin/go-header"

The go proxies are here to handle this kind of problem (GOPROXY="https://proxy.golang.org,direct" )

We try to follow real tags/versions.

I think that @denis-tingaikin (he is a golangci-lint member), when he will create a new tag/release will update himself the linter.

I will decline your PR because of my previous explanations.

@ldez ldez closed this Feb 4, 2022
@ldez ldez added the declined label Feb 4, 2022
@aelse
Copy link
Author

aelse commented Feb 4, 2022

Hi @ldez Thanks for responding.

I accept that a module name does not have to match the import url, though that's fighting against the go tooling

go get: github.com/denis-tingaikin/go-header@v0.4.2: parsing go.mod:
	module declares its path as: github.com/denis-tingajkin/go-header
	        but was required as: github.com/denis-tingaikin/go-header

My concern here is that there's an open security vulnerability where someone can register a github account named denis-tingajkin and publish modified code. This is an open supply chain vulnerability that someone can take advantage of, exposing golangci-lint users (who either build from source themselves or rely on a build published by the golangci-lint project in the future) to arbitrary code execution.

I understand you wish to stick to published versions. In that case would you accept a patch doing the rewrite (the change of tools/go.mod) in the top level module? This closes the hole and the rewrite can be removed any time after Denis publishes a new package version.

@ldez
Copy link
Member

ldez commented Feb 4, 2022

My concern here is that there's an open security vulnerability where someone can register a github account named denis-tingajkin and publish modified code.

We use tagged versions, and Go proxies (via Go checksum database) are here to keep the integrity: a tag cannot be overridden, it's impossible to do that.

Go modules with Go proxies are the best secure dependency system out there.

We cannot protect users that don't follow the security rules.

As we have to follow the configuration of a linter, when we update a linter, we have to review the code of the linter itself.

Otherwise, your patch will not change the current or previous versions, it will just apply to the future version of golangci-lint. So there is no need to do it quickly.

@aelse
Copy link
Author

aelse commented Feb 4, 2022

Go proxies protect only against published tags it already knows. If a new version is published (eg v0.4.3) the proxy checksums won't help.

@ldez
Copy link
Member

ldez commented Feb 4, 2022

golangci-lint is not a library, it's a binary (even if you are using the "tools" pattern the principle is the same).
And we use tagged versions, so those versions are already known by the checksum databases.

Even if you are using golangci-lint as a library, you are using a tagged version of golangci-lint, you don't have to update a dependency by hand, if you do that you have to take the responsibility to audit the update in any case.

The Go modules (via MVS) guarantee that a version superior (even a patch) to the real need will not be used except for an intentional action by a human to force the version upgrade.

Go modules are not npm packages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants