golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
122.87k stars 17.52k forks source link

envcmd: Replace any non-empty password in GOPROXY #37873

Open oiooj opened 4 years ago

oiooj commented 4 years ago

When a user submits an issue, we also need the user to file the output of go env(many users don't use go bug command), so we should replace any non-empty password in GOPROXY environment by default. The string form replaces password in the original URL with "[redacted]". Like go get -x:

MBA:/tmp$ go get -x -v golang.org/x/tools
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/@v/list
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@v/list
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/@v/list
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@v/list: 200 OK (0.649s)
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@latest
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/@v/list: 404 Not Found (0.649s)
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/@v/list: 404 Not Found (0.649s)
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@latest: 200 OK (0.006s)
go: downloading golang.org/x/tools v0.0.0-20200313205530-4303120df7d8
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@v/v0.0.0-20200313205530-4303120df7d8.zip
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@v/v0.0.0-20200313205530-4303120df7d8.zip: 200 OK (0.196s)
go: golang.org/x/tools upgrade => v0.0.0-20200313205530-4303120df7d8
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@v/v0.0.0-20200313205530-4303120df7d8.mod
# get https://oiooj:%5Bredacted%5D@goproxy.io/golang.org/x/tools/@v/v0.0.0-20200313205530-4303120df7d8.mod: 200 OK (0.032s)
# get https://oiooj:%5Bredacted%5D@goproxy.io/github.com/yuin/goldmark/@v/v1.1.25.mod
# get https://oiooj:%5Bredacted%5D@goproxy.io/github.com/yuin/goldmark/@v/v1.1.25.mod: 200 OK (0.028s)
MBA:/tmp$ go env
GO111MODULE="on"
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/test/Library/Caches/go-build"
GOENV="/Users/test/Library/Application Support/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOINSECURE=""
GONOPROXY="*.test.com"
GONOSUMDB="*.test.com"
GOOS="darwin"
GOPATH="/Users/test/golang"
GOPRIVATE="*.test.com"
GOPROXY="https://username:%5Bredacted%5D@goproxy.io,direct"
GOROOT="/usr/local/go"
GOSUMDB="off"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_amd64"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/hg/ss37ccnn4_1fhhg_tdrzhggh0000gn/T/go-build962445838=/tmp/go-build -gno-record-gcc-switches -fno-common"

We should only change the go env and go bug command, we can get the original config from go env GOPROXY and go env -json at this time, since some tools today expect to parse go env GOPROXY and use -json option.

oiooj commented 4 years ago

/cc @rsc @jayconrod @bcmills @FiloSottile @katiehockman

jayconrod commented 4 years ago

I think this is the same issue as in CL 223098.

I'd lean toward redacting go bug but not go env.

Ideally, passwords and secrets would be in ~/.netrc, but I don't think that's adequately documented yet.

bcmills commented 4 years ago

Yikes, I didn't realize those brackets would be percent-escaped. I think we need to fix web.Redact... 😅

bcmills commented 4 years ago

So, thinking about this some more:

  1. On the one hand, tools may reasonably expect go env to report the actual environment in use.
  2. On the other hand, if folks are using .netrc as we recommend, they won't have credentials in go env in the first place, so it really doesn't matter.
  3. On the gripping hand, now that .netrc handling is fixed maybe we shouldn't allow credentials in GOPROXY at all. Then there would be no question about whether we would print those credentials. (See #30610.)
gopherbot commented 4 years ago

Change https://golang.org/cl/223757 mentions this issue: cmd/go/internal/web: Redacts any password with "xxxxx"