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/pkgsite: license ignored when not next to go.mod #43817

Closed
zx2c4 opened this issue Jan 20, 2021 · 10 comments
Closed

x/pkgsite: license ignored when not next to go.mod #43817

zx2c4 opened this issue Jan 20, 2021 · 10 comments

Comments

@zx2c4
Copy link
Contributor

@zx2c4 zx2c4 commented Jan 20, 2021

https://git.zx2c4.com/wireguard-go/ is a MIT licensed repo. It contains two go modules. One is in the root -- /go.mod - and one is a bit deeper nested - /tun/netstack/go.mod. The repo also contains a license - /COPYING. All is well.

You can explore the APIs exposed by the repo's root module here: https://pkg.go.dev/golang.zx2c4.com/wireguard/tun

But if you try to do the same for the nested one, it complains that it can't find the license file: https://pkg.go.dev/golang.zx2c4.com/wireguard/tun/netstack

The problem is that it's looking for the COPYING file next to the non-root go.mod, rather than in the true root of the git repository.

@dmitshur
Copy link
Member

@dmitshur dmitshur commented Jan 21, 2021

Perhaps somewhat relevant, the go command has a special behavior where if a nested module root doesn't have a file named "LICENSE", the LICENSE file is copied from the repository root if one exists there (see coderepo.go#L1027-L1032). Notably, it applies only to files named "LICENSE", not "COPYING".

CC @julieqiu, @jayconrod.

@dmitshur dmitshur changed the title pkg.go.dev: license ignored when not next to go.mod x/pkgsite: license ignored when not next to go.mod Jan 21, 2021
@gopherbot gopherbot added the pkgsite label Jan 21, 2021
@gopherbot gopherbot added this to the Unreleased milestone Jan 21, 2021
@julieqiu
Copy link
Contributor

@julieqiu julieqiu commented Jan 21, 2021

/cc @jba

@julieqiu julieqiu modified the milestones: Unreleased, pkgsite/licenses Jan 21, 2021
@jba
Copy link
Contributor

@jba jba commented Jan 21, 2021

pkg.go.dev works on modules, not repos. We fetch module zips from proxy.golang.org and process them. The zip for your nested module does not include a license file:

> curl -L -o /tmp/netstack.zip https://proxy.golang.org/golang.zx2c4.com/wireguard/tun/netstack/@v/v0.0.0-20210120232502-fcc8ad05df75.zip
> unzip -l /tmp/netstack.zip 
Archive:  /tmp/netstack.zip
  Length      Date    Time    Name
---------  ---------- -----   ----
     1047  1980-00-00 00:00   golang.zx2c4.com/wireguard/tun/netstack@v0.0.0-20210120232502-fcc8ad05df75/examples/http_client.go
     1223  1980-00-00 00:00   golang.zx2c4.com/wireguard/tun/netstack@v0.0.0-20210120232502-fcc8ad05df75/examples/http_server.go
      218  1980-00-00 00:00   golang.zx2c4.com/wireguard/tun/netstack@v0.0.0-20210120232502-fcc8ad05df75/go.mod
    40172  1980-00-00 00:00   golang.zx2c4.com/wireguard/tun/netstack@v0.0.0-20210120232502-fcc8ad05df75/go.sum
    20835  1980-00-00 00:00   golang.zx2c4.com/wireguard/tun/netstack@v0.0.0-20210120232502-fcc8ad05df75/tun.go
---------                     -------
    63495                     5 files

You should have a license file in each module rather than each repo, or you can take advantage of the special behavior that @dmitshur pointed out above.

@jba jba closed this Jan 21, 2021
@zx2c4
Copy link
Contributor Author

@zx2c4 zx2c4 commented Jan 21, 2021

You should have a license file in each module rather than each repo,

Thanks, but no thanks. The licensing concerns of my repository are per-repository, and I have no intention of changing this, or entering less-charted legal territory of a repository with differing licenses in different places. This isn't going to happen.

While I see the technical reason why this is challenging -- the proxy pre-makes zips for you -- I still believe this to be a bug in some part of the Go stack. Perhaps that proxy zipper should copy the license files in? Perhaps x/pkgsite should pursue a different way of license discovery.

But I can't imagine an acceptable solution being to change license-per-repo, which has lots of legal precedent, with license-per-go-module, whatever that might be in the mind of a judge.

@zx2c4
Copy link
Contributor Author

@zx2c4 zx2c4 commented Jan 21, 2021

Perhaps somewhat relevant, the go command has a special behavior where if a nested module root doesn't have a file named "LICENSE", the LICENSE file is copied from the repository root if one exists there (see coderepo.go#L1027-L1032). Notably, it applies only to files named "LICENSE", not "COPYING".

CC @julieqiu, @jayconrod.

Thanks. This seems like good behavior. I'll send a CL to extend that to COPYING.

@gopherbot
Copy link

@gopherbot gopherbot commented Jan 21, 2021

Change https://golang.org/cl/285352 mentions this issue: modfetch: copy COPYING in addition to LICENSE

@zx2c4
Copy link
Contributor Author

@zx2c4 zx2c4 commented Jan 21, 2021

@dmitshur wrote on that CL:

A change like this may cause content of existing modules to change, invalidating their hashes. I don't know whether how viable it is to roll it out. Maybe it's possible by gating behind the Go language version stated in go.mod. All this is likely better discussed in an issue.

So, responding here.

This is a good point. I can update the CL to gate it on go.mod version. (Or, if you've got some way to dump the proxy server, you could just test that hypothesis on all known modules.) But I wonder if that's happening if there's anything else we should consider too. Should this actually take the full list of license file names that x/pkgsite recognizes?

Alternatively, if you feel that I should just rename COPYING to LICENSE, that would be perhaps an easier solution and just fine, but in that case, would it make sense to discuss changing x/pkgsite to only look at LICENSE and not the others, in order to keep the tooling semantics consistent across the ecosystem?

@zx2c4
Copy link
Contributor Author

@zx2c4 zx2c4 commented Jan 21, 2021

Alternatively, if you feel that I should just rename COPYING to LICENSE, that would be perhaps an easier solution and just fine, but in that case, would it make sense to discuss changing x/pkgsite to only look at LICENSE and not the others, in order to keep the tooling semantics consistent across the ecosystem?

I sent a CL in for this strategy. Feel free to -2 it if you hate that idea, but I figured I'd prepare it to give you the option. https://go-review.googlesource.com/c/pkgsite/+/285273

@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Jan 21, 2021

This issue is closely related to #39417. Copying my comments from there:

I won't say it's impossible, but it's very difficult to make any change to the set of files that get included in a module .zip file.

If different versions of Go choose different sets of files, they won't generate the same hashes, so when a module is downloaded by an old version of the go command it will look like someone has tampered with the module after it was released. That could be somewhat mitigated by gating the change on a new go version in go.mod. We'd have to choose a version far enough in the future (at least a year, probably more) that by the time it was current, versions of Go that don't support this are no longer supported.

It's also hard to decide what files to actually include: LICENSE, LICENSE.txt, sure. How about COPYING.html? copyright-license.pdf? pkg.go.dev has a list of names they use, and we'd probably use the same list. Not sure what they do if there are multiple matching files.

...

Just to close the loop on this: @bcmills, @matloob, and I discussed this last week, and we decided not to change anything here. Some mitigations are possible, but fundamentally, this would cause different versions of the go command to produce different sums for the same module, which would cause security errors. The cost of that outweighs the benefit.

@zx2c4
Copy link
Contributor Author

@zx2c4 zx2c4 commented Jan 21, 2021

Okay. I suppose I'll drop this, and rename COPYING->LICENSE.

zx2c4-bot pushed a commit to WireGuard/wireguard-go that referenced this issue Mar 6, 2021
Otherwise the netstack module doesn't show up on the package site.

golang/go#43817 (comment)

Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants