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

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
Labels
GoCommand cmd/go NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Milestone
Backlog

Comments

@oiooj
Copy link
Member

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 commented Mar 16, 2020

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

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

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 the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 16, 2020
@jayconrod jayconrod added this to the Backlog milestone Mar 16, 2020
@bcmills
Copy link
Contributor

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
Contributor

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 cmd/go: GOPROXY credentials exposed in case of errors #30610.)

@gopherbot
Copy link

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

@odeke-em odeke-em mentioned this issue Mar 21, 2020
Closed
gopherbot pushed a commit that referenced this issue Apr 27, 2020
@oiooj
cmd/go/internal/web: use url.Redacted
20ed142 
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 pushed a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
@oiooj @xujianhai666
cmd/go/internal/web: use url.Redacted
2be3ad7 
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
Assignees
No one assigned
Labels
GoCommand cmd/go NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made.
Projects
None yet
Milestone
Backlog
Development

No branches or pull requests
4 participants
@jayconrod @oiooj @bcmills @gopherbot