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.

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

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

Open
oiooj opened this issue Mar 16, 2020 · 5 comments
Open

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

oiooj opened this issue Mar 16, 2020 · 5 comments

Comments

@oiooj
Copy link
Member

@oiooj oiooj commented Mar 16, 2020

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
Copy link
Member Author

@oiooj oiooj commented Mar 16, 2020

@oiooj oiooj added the GoCommand label Mar 16, 2020
@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Mar 16, 2020

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.

@jayconrod jayconrod added this to the Backlog milestone Mar 16, 2020
@bcmills
Copy link
Member

@bcmills bcmills commented Mar 16, 2020

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

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 16, 2020

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
Copy link

@gopherbot gopherbot commented Mar 17, 2020

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

gopherbot pushed a commit that referenced this issue Apr 27, 2020
Updates #37873

Change-Id: I2228f31fc7bd7daef086cd05d365fa7c68e60a83
Reviewed-on: https://go-review.googlesource.com/c/go/+/223757
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
Updates golang#37873

Change-Id: I2228f31fc7bd7daef086cd05d365fa7c68e60a83
Reviewed-on: https://go-review.googlesource.com/c/go/+/223757
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.