-
Notifications
You must be signed in to change notification settings - Fork 18.5k
Description
@myitcv and I just spent a solid thirty minutes figuring out why our CI jobs were still hitting sum.golang.org for a module when it should be set up with GOPRIVATE. This is a GitHub Actions CI job running a YAML workflow step, which then calls a script, which then calls docker build, which then runs a Go build script inside a Dockerfile; so there are multiple layers involved.
And there was a bug in one of those layers; a Bash script double-quoted a GOPROXY value by accident, meaning that we effectively ended up running:
GOPRIVATE='"cuelang.org/go"' go install -trimpath cuelang.org/go/cmd/cue@v0.16.0-0.dev.0.20251203133616-9461bd296cae
As you can imagine, the fact that the GOPRIVATE includes double quotes means that it doesn't actually do what we expect; it still hits proxy.golang.org and sum.golang.org:
$ GOMODCACHE=$(mktemp -d) GOPRIVATE='"cuelang.org/go"' go install -x -trimpath cuelang.org/go/cmd/cue@v0.16.0-0.dev.0.20251203133616-9461bd296cae | grep cuelang\.org
# get https://proxy.golang.org/cuelang.org/go/cmd/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.info
# get https://proxy.golang.org/cuelang.org/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.info
# get https://proxy.golang.org/cuelang.org/go/cmd/cue/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.info
# get https://proxy.golang.org/cuelang.org/go/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.info
# get https://proxy.golang.org/cuelang.org/go/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.info: 200 OK (0.087s)
go: downloading cuelang.org/go v0.16.0-0.dev.0.20251203133616-9461bd296cae
# get https://proxy.golang.org/cuelang.org/go/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.zip
# get https://proxy.golang.org/cuelang.org/go/@v/v0.16.0-0.dev.0.20251203133616-9461bd296cae.zip: 200 OK (0.065s)
[...]
Just to double check, below you can see that removing the double quotes does make it skip the proxy:
$ GOMODCACHE=$(mktemp -d) GOPRIVATE='cuelang.org/go' go install -x -trimpath cuelang.org/go/cmd/cue@v0.16.0-0.dev.0.20251203133616-9461bd296cae | grep cuelang\.org
# get https://cuelang.org/go/cmd/cue?go-get=1
# get https://cuelang.org/go?go-get=1
# get https://cuelang.org/go/cmd?go-get=1
# get https://cuelang.org/go/cmd?go-get=1: 200 OK (0.136s)
# get https://cuelang.org/go/cmd/cue?go-get=1: 200 OK (0.136s)
# get https://cuelang.org/go?go-get=1: 200 OK (0.136s)
# get https://cuelang.org/go?go-get=1
mkdir -p /tmp/tmp.yu2mSC9VSM/cache/vcs # git3 https://cue.gerrithub.io/cue-lang/cue
[...]
The issue is obvious enough when you stare right at it:
$ GOPRIVATE='"cuelang.org/go"' go env GOPRIVATE
"cuelang.org/go"
However, I argue that cmd/go should be more user friendly here; any special shell characters (double quotes, single quotes, semicolons, or possibly even whitespace) should be rejected from certain env var values (GOPRIVATE, GONOPROXY, GONOSUMDB, GOPROXY, ...) as early as possible, in the hopes of catching these bugs before they silently lead to incorrect behavior, such as a GOPRIVATE that never matches anything.
In the case of env vars like GOPRIVATE it's easy; a module path can never contain double quotes, nor are double quotes used in the comma-separated pattern matching syntax.