From 19e78bda2ade3bd99e59a507694cbb3505fab3e2 Mon Sep 17 00:00:00 2001
From: Roger Peppe <rogpeppe@gmail.com>
Date: Wed, 13 Mar 2024 17:45:46 +0000
Subject: [PATCH] internal/mod/modresolve: fix stripPrefix for exact match

When a module path exactly matches the prefix that's specified for it,
the logic was not stripping the prefix as it should, resulting in
unexpected access to the wrong repository.

This CL fixes that, and also causes the config file parsing to fail when
`stripPrefix` is set and there's no repository specified in the
registry, because that would cause an exact match to use the empty
repository, which isn't allowed in the OCI spec. Technically it would be
possible to allow all modules that have the prefix but don't _exactly_
match the prefix, but that use case seems relatively limited (in
practice almost everyone seems to use a repository), and we could always
relax that restriction later if we choose.

I thought about adding a more end-to-end test in `cmd/cue` but that
turns out quite hard to do without duplicating a lot of logic which is
unit tested. Instead I manually verified that this scenario does indeed
work correctly.

Thanks to Erik Mogensen for bringing this issue to attention.

Signed-off-by: Roger Peppe <rogpeppe@gmail.com>
Change-Id: I9e2aeae599cca0b4a161b40c763262338e7007f4
Dispatch-Trailer: {"type":"trybot","CL":1178365,"patchset":3,"ref":"refs/changes/65/1178365/3","targetBranch":"master"}
---
 internal/mod/modresolve/resolve.go      | 13 +++++++++++--
 internal/mod/modresolve/resolve_test.go | 17 +++++++++++++++++
 2 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/internal/mod/modresolve/resolve.go b/internal/mod/modresolve/resolve.go
index 1faec6a82..bf37fbccc 100644
--- a/internal/mod/modresolve/resolve.go
+++ b/internal/mod/modresolve/resolve.go
@@ -155,8 +155,16 @@ func (r *registryConfig) init() error {
 		// Shouldn't happen because default should apply.
 		return fmt.Errorf("empty pathEncoding")
 	}
-	if r.StripPrefix && r.PathEncoding != encPath {
-		return fmt.Errorf("cannot strip prefix unless using path encoding")
+	if r.StripPrefix {
+		if r.PathEncoding != encPath {
+			// TODO we could relax this to allow storing of naked tags
+			// when the module path matches exactly and hash tags
+			// otherwise.
+			return fmt.Errorf("cannot strip prefix unless using path encoding")
+		}
+		if r.repository == "" {
+			return fmt.Errorf("use of stripPrefix requires a non-empty repository within the registry")
+		}
 	}
 	return nil
 }
@@ -388,6 +396,7 @@ func (r *resolver) ResolveToLocation(mpath, vers string) (Location, bool) {
 	bestMatchReg := r.cfg.DefaultRegistry
 	for pat, reg := range r.cfg.ModuleRegistries {
 		if pat == mpath {
+			bestMatch = pat
 			bestMatchReg = reg
 			break
 		}
diff --git a/internal/mod/modresolve/resolve_test.go b/internal/mod/modresolve/resolve_test.go
index 5bd15c226..70329be8f 100644
--- a/internal/mod/modresolve/resolve_test.go
+++ b/internal/mod/modresolve/resolve_test.go
@@ -381,6 +381,11 @@ moduleRegistries: {
 				Repository: "repo/one/two/three",
 				Tag:        "v0.0.1",
 			},
+			"stripped.org/bar v0.0.1": {
+				Host:       "r3.example",
+				Repository: "repo",
+				Tag:        "v0.0.1",
+			},
 		},
 	}, {
 		testName: "InvalidModulePath",
@@ -428,6 +433,18 @@ moduleRegistries: {
 }
 `,
 		err: `registry host "ok.com" is specified both as secure and insecure`,
+	}, {
+		testName:        "StripPrefixWithNoRepo",
+		catchAllDefault: "c.example",
+		in: `
+moduleRegistries: {
+	"a.example/foo": {
+		registry: "foo.example"
+		stripPrefix: true
+	}
+}
+`,
+		err: `invalid registry configuration in "a.example/foo": use of stripPrefix requires a non-empty repository within the registry`,
 	}}
 
 	for _, tc := range testCases {