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.

Sign up for GitHub

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

Jump to bottom

cmd/go/internal/modload: MainModuleSet.DirImportPath does not check module boundaries #66119

Open
bcmills opened this issue Mar 5, 2024 · 1 comment
Open

cmd/go/internal/modload: MainModuleSet.DirImportPath does not check module boundaries #66119

bcmills opened this issue Mar 5, 2024 · 1 comment
Assignees
@samthanawalla
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. GoCommand cmd/go modules
Milestone
Backlog

Comments

@bcmills
Copy link
Contributor

bcmills commented Mar 5, 2024

Go version

5dcc04a

Output of go env in your module/workspace:

GO111MODULE=''
GOARCH='amd64'
GOBIN=''
GOCACHE='/usr/local/google/home/bcmills/.cache/go-build'
GOENV='/usr/local/google/home/bcmills/.config/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFLAGS=''
GOHOSTARCH='amd64'
GOHOSTOS='linux'
GOINSECURE=''
GOMODCACHE='/tmp/tmp.JAmBbk4Nn9/.gopath/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='linux'
GOPATH='/tmp/tmp.JAmBbk4Nn9/.gopath'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/google/home/bcmills/sdk/gotip'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='local'
GOTOOLDIR='/usr/local/google/home/bcmills/sdk/gotip/pkg/tool/linux_amd64'
GOVCS=''
GOVERSION='devel go1.23-5dcc04ae Mon Mar 4 21:01:23 2024 +0000'
GODEBUG=''
GCCGO='/usr/bin/gccgo'
GOAMD64='v1'
AR='ar'
CC='gcc'
CXX='c++'
CGO_ENABLED='1'
GOMOD='/tmp/tmp.JAmBbk4Nn9/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -m64 -pthread -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=/tmp/go-build4217272553=/tmp/go-build -gno-record-gcc-switches'

What did you do?

Reviewed test failures on pending https://go.dev/cl/567435.

Noticed an existing bug in modload.MainModuleSet.DirImportPath introduced in https://go.dev/cl/129798 (for #27022).

Reproduced the bug as follows:

go build testdata/q/q.go

-- go.mod --
module example

go 1.23

require (
	testdata/q v0.1.0
)

replace (
	testdata/q => ./testdata/q
)
-- internal/p/p.go --
package p
-- testdata/q/go.mod --
module testdata/q

go 1.23
-- testdata/q/q.go --
package q

import _ "example/internal/p"

What did you see happen?

$ go build ./testdata/q/q.go

(no error)

What did you expect to see?

An error, because (per the behavior agreed upon in #23970) a package in the module testdata/q should not be allowed to import an internal module from a module that does not share its import path.

Per https://go.dev/cl/129798:

In GOPATH mode the rule has always been that 'go run x.go' can
import whatever the package in x.go's directory would be able to
import. Apply the same rule here.

go build testdata/q confirms that the package in q.go's directory is not allowed to import that package:

$ go build testdata/q
package testdata/q
        testdata/q/q.go:3:8: use of internal package example/internal/p not allowed

The underlying problem is that DirImportPath is just checking for a file path prefix, not using modload.dirInModule to properly check that the directory is contained in the module without intervening go.mod files carving out module boundaries.

(CC @matloob @samthanawalla)
@bcmills bcmills added NeedsFix The path to resolution is known, but the work has not been done. GoCommand cmd/go modules labels Mar 5, 2024
@bcmills bcmills added this to the Backlog milestone Mar 5, 2024
@bcmills bcmills changed the title cmd/go/internal/modload: MainModuleSet.DirImportPath cmd/go/internal/modload: MainModuleSet.DirImportPath does not check module boundaries Mar 5, 2024
@samthanawalla samthanawalla self-assigned this Mar 5, 2024
@samthanawalla samthanawalla added FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsFix The path to resolution is known, but the work has not been done. labels Mar 5, 2024
@gopherbot
Copy link

Change https://go.dev/cl/569277 mentions this issue: cmd/go: verify path is in module for DirImportPath

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@samthanawalla samthanawalla

Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. GoCommand cmd/go modules
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
3 participants
@bcmills @gopherbot @samthanawalla