cmd/go: revert "remove base.ShortPathConservative"

This reverts commit 2e07ff3543 (CL 630276).

Reason for revert: broke longtest builders.

Change-Id: I86b313cc6c1be061b408502e96a09916a6ac2c4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/630317
Auto-Submit: Russ Cox <rsc@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/cmd/go/internal/base/path.go

@@ -6,12 +6,13 @@ package base
 
 import (
 	"errors"
-	"io/fs"
 	"os"
 	"path/filepath"
 	"runtime"
 	"strings"
 	"sync"
+
+	"cmd/go/internal/str"
 )
 
 // UncachedCwd returns the current working directory.
@@ -34,32 +35,44 @@ func Cwd() string {
 }
 
 // ShortPath returns an absolute or relative name for path, whatever is shorter.
-// ShortPath should only be used when formatting paths for error messages.
+// There are rare cases where the path produced by ShortPath could be incorrect
+// so it should only be used when formatting paths for error messages, not to read
+// a file.
 func ShortPath(path string) string {
-	if rel, err := filepath.Rel(Cwd(), path); err == nil && len(rel) < len(path) && sameFile(rel, path) {
+	if rel, err := filepath.Rel(Cwd(), path); err == nil && len(rel) < len(path) {
 		return rel
 	}
 	return path
 }
 
-func sameFile(path1, path2 string) bool {
-	fi1, err1 := os.Stat(path1)
-	fi2, err2 := os.Stat(path2)
-	if err1 != nil || err2 != nil {
-		// If there were errors statting the files return false,
-		// unless both of the files don't exist.
-		return os.IsNotExist(err1) && os.IsNotExist(err2)
+// ShortPathConservative is similar to ShortPath, but returns the input if the result of ShortPath
+// would meet conditions that could make it invalid. If the short path would reach into a
+// parent directory and the base path contains a symlink, a ".." component can
+// cross a symlink boundary. That could be a problem because the symlinks could be evaluated,
+// changing the relative location of the boundary, before the ".." terms are applied to
+// go to parents. The check here is a little more conservative: it checks
+// whether the path starts with a ../ or ..\ component, and if any of the parent directories
+// of the working directory are symlinks.
+// See #68383 for a case where this could happen.
+func ShortPathConservative(path string) string {
+	if rel, err := relConservative(Cwd(), path); err == nil && len(rel) < len(path) {
+		return rel
 	}
-	return os.SameFile(fi1, fi2)
+	return path
 }
 
-// ShortPathError rewrites the path in err using base.ShortPath, if err is a wrapped PathError.
-func ShortPathError(err error) error {
-	var pe *fs.PathError
-	if errors.As(err, &pe) {
-		pe.Path = ShortPath(pe.Path)
+func relConservative(basepath, targpath string) (string, error) {
+	relpath, err := filepath.Rel(basepath, targpath)
+	if err != nil {
+		return "", err
 	}
-	return err
+	if strings.HasPrefix(relpath, str.WithFilePathSeparator("..")) {
+		expanded, err := filepath.EvalSymlinks(basepath)
+		if err != nil || expanded != basepath { // The basepath contains a symlink. Be conservative and reject it.
+			return "", errors.New("conservatively rejecting relative path that may be invalid")
+		}
+	}
+	return relpath, nil
 }
 
 // RelPaths returns a copy of paths with absolute paths
@@ -67,7 +80,11 @@ func ShortPathError(err error) error {
 func RelPaths(paths []string) []string {
 	out := make([]string, 0, len(paths))
 	for _, p := range paths {
-		out = append(out, ShortPath(p))
+		rel, err := relConservative(Cwd(), p)
+		if err == nil && len(rel) < len(p) {
+			p = rel
+		}
+		out = append(out, p)
 	}
 	return out
 }

src/cmd/go/internal/modload/init.go

@@ -944,7 +944,7 @@ func loadModFile(ctx context.Context, opts *PackageOpts) (*Requirements, error)
 					err = errWorkTooOld(gomod, workFile, tooNew.GoVersion)
 				} else {
 					err = fmt.Errorf("cannot load module %s listed in go.work file: %w",
-						base.ShortPath(filepath.Dir(gomod)), base.ShortPathError(err))
+						base.ShortPath(filepath.Dir(gomod)), err)
 				}
 			}
 			errs = append(errs, err)

src/cmd/go/internal/modload/load.go

@@ -670,7 +670,7 @@ func resolveLocalPackage(ctx context.Context, dir string, rs *Requirements) (str
 		}
 		if inWorkspaceMode() {
 			if mr := findModuleRoot(absDir); mr != "" {
-				return "", fmt.Errorf("%s is contained in a module that is not one of the workspace modules listed in go.work. You can add the module to the workspace using:\n\tgo work use %s", dirstr, base.ShortPath(mr))
+				return "", fmt.Errorf("%s is contained in a module that is not one of the workspace modules listed in go.work. You can add the module to the workspace using:\n\tgo work use %s", dirstr, base.ShortPathConservative(mr))
 			}
 			return "", fmt.Errorf("%s outside modules listed in go.work or their selected dependencies", dirstr)
 		}

src/cmd/go/internal/modload/modfile.go

@@ -30,6 +30,10 @@ import (
 // ReadModFile reads and parses the mod file at gomod. ReadModFile properly applies the
 // overlay, locks the file while reading, and applies fix, if applicable.
 func ReadModFile(gomod string, fix modfile.VersionFixer) (data []byte, f *modfile.File, err error) {
+	// The path used to open the file shows up in errors. Use ShortPathConservative
+	// so a more convenient path is displayed in the errors. ShortPath isn't used
+	// because it's meant only to be used in errors, not to open files.
+	gomod = base.ShortPathConservative(gomod)
 	if fsys.Replaced(gomod) {
 		// Don't lock go.mod if it's part of the overlay.
 		// On Plan 9, locking requires chmod, and we don't want to modify any file
@@ -45,18 +49,18 @@ func ReadModFile(gomod string, fix modfile.VersionFixer) (data []byte, f *modfil
 	f, err = modfile.Parse(gomod, data, fix)
 	if err != nil {
 		// Errors returned by modfile.Parse begin with file:line.
-		return nil, nil, fmt.Errorf("errors parsing %s:\n%w", base.ShortPath(gomod), err)
+		return nil, nil, fmt.Errorf("errors parsing %s:\n%w", gomod, err)
 	}
 	if f.Go != nil && gover.Compare(f.Go.Version, gover.Local()) > 0 {
 		toolchain := ""
 		if f.Toolchain != nil {
 			toolchain = f.Toolchain.Name
 		}
-		return nil, nil, &gover.TooNewError{What: base.ShortPath(gomod), GoVersion: f.Go.Version, Toolchain: toolchain}
+		return nil, nil, &gover.TooNewError{What: gomod, GoVersion: f.Go.Version, Toolchain: toolchain}
 	}
 	if f.Module == nil {
 		// No module declaration. Must add module path.
-		return nil, nil, fmt.Errorf("error reading %s: missing module declaration. To specify the module path:\n\tgo mod edit -module=example.com/mod", base.ShortPath(gomod))
+		return nil, nil, fmt.Errorf("error reading %s: missing module declaration. To specify the module path:\n\tgo mod edit -module=example.com/mod", gomod)
 	}
 
 	return data, f, err

src/cmd/go/internal/workcmd/use.go

@@ -102,7 +102,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st
 	lookDir := func(dir string) {
 		absDir, dir := pathRel(workDir, dir)
 
-		file := filepath.Join(absDir, "go.mod")
+		file := base.ShortPathConservative(filepath.Join(absDir, "go.mod"))
 		fi, err := fsys.Stat(file)
 		if err != nil {
 			if os.IsNotExist(err) {
@@ -114,7 +114,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st
 		}
 
 		if !fi.Mode().IsRegular() {
-			sw.Error(fmt.Errorf("%v is not a regular file", base.ShortPath(file)))
+			sw.Error(fmt.Errorf("%v is not a regular file", file))
 			return
 		}
 
@@ -126,17 +126,18 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st
 
 	for _, useDir := range args {
 		absArg, _ := pathRel(workDir, useDir)
+		useDirShort := base.ShortPathConservative(absArg) // relative to the working directory rather than the workspace
 
-		info, err := fsys.Stat(absArg)
+		info, err := fsys.Stat(useDirShort)
 		if err != nil {
 			// Errors raised from os.Stat are formatted to be more user-friendly.
 			if os.IsNotExist(err) {
-				err = fmt.Errorf("directory %v does not exist", base.ShortPath(absArg))
+				err = fmt.Errorf("directory %v does not exist", useDirShort)
 			}
 			sw.Error(err)
 			continue
 		} else if !info.IsDir() {
-			sw.Error(fmt.Errorf("%s is not a directory", base.ShortPath(absArg)))
+			sw.Error(fmt.Errorf("%s is not a directory", useDirShort))
 			continue
 		}
 
@@ -157,7 +158,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st
 			if !d.IsDir() {
 				if d.Type()&fs.ModeSymlink != 0 {
 					if target, err := fsys.Stat(path); err == nil && target.IsDir() {
-						fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", base.ShortPath(path))
+						fmt.Fprintf(os.Stderr, "warning: ignoring symlink %s\n", base.ShortPathConservative(path))
 					}
 				}
 				return nil
@@ -209,7 +210,7 @@ func workUse(ctx context.Context, gowork string, wf *modfile.WorkFile, args []st
 		} else {
 			abs = filepath.Join(workDir, use.Path)
 		}
-		_, mf, err := modload.ReadModFile(filepath.Join(abs, "go.mod"), nil)
+		_, mf, err := modload.ReadModFile(base.ShortPathConservative(filepath.Join(abs, "go.mod")), nil)
 		if err != nil {
 			sw.Error(err)
 			continue