From 614ca99ffc1dec921aa3bd4380e72e10d3bfe068 Mon Sep 17 00:00:00 2001 From: Dmitri Shuralyov Date: Mon, 24 Feb 2020 22:34:51 -0500 Subject: [PATCH] internal/code: reject pseduo-versions from non-master branches The intention of the current ModuleHandler is to serve only v0.0.0 pseudo-versions from commits that are on the master branch. This requires checking the commit hash component of the incoming module request, and verifying that the corresponding commit is a part of the master branch. This wasn't done previously, which means an explicit request to a pseudo-version based on a commit on non-master branch would be served by ModuleHandler as if such a version existed on the master branch. No non-master branches were ever pushed, and pseudo-versions based on commits on non-master branches were never advertised by the list endpoint. As a result, unintentional versions were not indexed by the Go module mirror. Updates https://github.com/golang/go/issues/24031. --- internal/code/module.go | 11 +++++++++++ internal/code/module_test.go | 5 +++++ .../a0/40d5e5144e7d8b243238c40a3cbbdb5331d4bd | Bin 0 -> 301 bytes .../c4/66b17874c06ad8ed4f162206d3d94dac08cdca | Bin 0 -> 86 bytes .../c6/1324d16db7fa26c252edfb6305dfc2a22e06f5 | 1 + .../kebabcase/refs/heads/branch2 | 1 + 6 files changed, 18 insertions(+) create mode 100644 internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/a0/40d5e5144e7d8b243238c40a3cbbdb5331d4bd create mode 100644 internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c4/66b17874c06ad8ed4f162206d3d94dac08cdca create mode 100644 internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c6/1324d16db7fa26c252edfb6305dfc2a22e06f5 create mode 100644 internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/refs/heads/branch2 diff --git a/internal/code/module.go b/internal/code/module.go index cfc9881..2e49eec 100644 --- a/internal/code/module.go +++ b/internal/code/module.go @@ -107,6 +107,8 @@ func (h ModuleHandler) ServeModule(w http.ResponseWriter, req *http.Request) err commit, err := repo.GetCommit(commitID) if err != nil || commit.Committer == nil || !versionTime.Equal(time.Unix(commit.Committer.Date.Seconds, 0).UTC()) { return os.ErrNotExist + } else if !isCommitOnMaster(req.Context(), gitDir, commit) { + return os.ErrNotExist } // Handle one of "/@v/." requests. @@ -223,6 +225,15 @@ func WriteModuleZip(w io.Writer, m module.Version, r vcs.Repository, id vcs.Comm return err } +// isCommitOnMaster reports whether commit c is a part of master branch +// of git repo at gitDir, and no errors occurred while determining that. +func isCommitOnMaster(ctx context.Context, gitDir string, c *vcs.Commit) bool { + cmd := exec.CommandContext(ctx, "git", "merge-base", "--is-ancestor", "--", string(c.ID), "master") + cmd.Dir = gitDir + err := cmd.Run() + return err == nil +} + // listMasterCommits returns a list of commits in git repo on master branch. // If master branch doesn't exist, an empty list is returned. func listMasterCommits(ctx context.Context, gitDir string) ([]mod.RevInfo, error) { diff --git a/internal/code/module_test.go b/internal/code/module_test.go index 14ee61d..fd4b155 100644 --- a/internal/code/module_test.go +++ b/internal/code/module_test.go @@ -168,6 +168,11 @@ v0.0.0-20180326031431-f628922a6885 url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v1.2.4-0.20170912031248-a1d95f8919b5.info", wantNotExist: true, }, + { + name: "commit on non-master branch", + url: "/api/module/dmitri.shuralyov.com/kebabcase/@v/v0.0.0-20200225024836-c61324d16db7.info", + wantNotExist: true, + }, } { t.Run(tt.name, func(t *testing.T) { req := httptest.NewRequest(http.MethodGet, tt.url, nil) diff --git a/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/a0/40d5e5144e7d8b243238c40a3cbbdb5331d4bd b/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/a0/40d5e5144e7d8b243238c40a3cbbdb5331d4bd new file mode 100644 index 0000000000000000000000000000000000000000..5c136df112b88a214a383941cece80c7c779445a GIT binary patch literal 301 zcmV+|0n+|>0bNkbPQx$|^qb2EW^IXR@=d0+&BXB)Z&RbjX3Lsa6+5#ZPsgZnAjFj? literal 0 HcmV?d00001 diff --git a/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c4/66b17874c06ad8ed4f162206d3d94dac08cdca b/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c4/66b17874c06ad8ed4f162206d3d94dac08cdca new file mode 100644 index 0000000000000000000000000000000000000000..0a0360e9ba52c8b6ce10dfe89a6d8f7a06061f93 GIT binary patch literal 86 zcmV-c0IC0Y0V^p=O;xZkWiT`_Ff%bx$WBd4OiE5HPSs1#XIS8H^{I$oZMTY%#St!> s-M51cuk1xv7hjTETmn+8R#!E*n$^YcQ@Z|Yuk@}}pFJ~T0Nl7DQR@&UOaK4? literal 0 HcmV?d00001 diff --git a/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c6/1324d16db7fa26c252edfb6305dfc2a22e06f5 b/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c6/1324d16db7fa26c252edfb6305dfc2a22e06f5 new file mode 100644 index 0000000..645ccb7 --- /dev/null +++ b/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/objects/c6/1324d16db7fa26c252edfb6305dfc2a22e06f5 @@ -0,0 +1 @@ +x¥ŽÁN!D=óý»†a˜dc<øz3šî^‡d Úø÷¢f¿À¤N¯*UE5çÔa2ú¡7 ç}4KXi„ÝÙxkµç‰WÇH:ª+6)âpµ8ËjÂ9ÊÊ4gPÛ‰Ãbl%*¼õ­6xk-ÁËvkxùªŸpâ_ò´ßÉ‘j~3;¯a5zÖZ :‚]þQ¡ÞbÃB›}Ú°|¤|­­ ]°aOµìJ½ni‡!¼§2²@-”Z÷ŸUGõ P§gY \ No newline at end of file diff --git a/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/refs/heads/branch2 b/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/refs/heads/branch2 new file mode 100644 index 0000000..71c8e66 --- /dev/null +++ b/internal/code/testdata/repositories/dmitri.shuralyov.com/kebabcase/refs/heads/branch2 @@ -0,0 +1 @@ +c61324d16db7fa26c252edfb6305dfc2a22e06f5