go icon indicating copy to clipboard operation
go copied to clipboard

cmd/go: using `--trimpath` in `go build` removes `-ldflags` from `go version -m` output

Open wallyqs opened this issue 2 years ago • 18 comments

What version of Go are you using (go version)?

$ go version 1.21.2

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env

GO111MODULE='' GOARCH='arm64' GOBIN='' GOCACHE='/Users/example/Library/Caches/go-build' GOENV='/Users/example/Library/Application Support/go/env' GOEXE='' GOEXPERIMENT='' GOFLAGS='' GOHOSTARCH='arm64' GOHOSTOS='darwin' GOINSECURE='' GOMODCACHE='/Users/example/repos/dev/pkg/mod' GONOPROXY='' GONOSUMDB='' GOOS='darwin' GOPATH='/Users/example/repos/dev' GOPRIVATE='' GOPROXY='https://proxy.golang.org,direct' GOROOT='/opt/homebrew/Cellar/go/1.21.2/libexec' GOSUMDB='sum.golang.org' GOTMPDIR='' GOTOOLCHAIN='auto' GOTOOLDIR='/opt/homebrew/Cellar/go/1.21.2/libexec/pkg/tool/darwin_arm64' GOVCS='' GOVERSION='go1.21.2' GCCGO='gccgo' AR='ar' CC='cc' CXX='c++' CGO_ENABLED='1' GOMOD='/Users/example/repos/dev/src/github.com/nats-io/nats-server/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 -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/kr/7z1nxlm160ng389wq51vpktw0000gn/T/go-build4075377866=/tmp/go-build -gno-record-gcc-switches -fno-common'

What did you do?

git clone https://github.com/nats-io/nats-server
go build -ldflags="-X main.version='2.10.2'"  --trimpath .
go version -m ./nats-server | grep ldflags

What did you expect to see?

go version -m ./nats-server | grep ldflags
	build	-ldflags="-X main.version='2.10.2'"
echo $?
0

What did you see instead?

go version -m ./nats-server | grep ldflags
echo $?
1

wallyqs avatar Oct 07 '23 04:10 wallyqs

See: https://github.com/golang/go/issues/52372

cuonglm avatar Oct 07 '23 04:10 cuonglm

Thanks ok I see now there are a couple of TODOs so this is a known issue: https://github.com/golang/go/blob/d252fdd630a51db206194b3c41c3d036fdd6f48a/src/cmd/go/internal/load/pkg.go#L2336

Maybe just whitelisting -X main.version which is what sbom tooling like syft uses to detect a go build version might be sufficient?

wallyqs avatar Oct 07 '23 05:10 wallyqs

CC @bcmills, @matloob.

dmitshur avatar Oct 09 '23 22:10 dmitshur

@wallyqs if you'd like to make progress on that TODO, you're welcome to send a fix! 🙃 (https://go.dev/doc/contribute)

I don't think we should add a special case for -X main.version, though. This is a general problem and ought to have a general solution.

bcmills avatar Oct 12 '23 18:10 bcmills

Change https://go.dev/cl/536795 mentions this issue: cmd/go: add -ldflags to build metadata when CGO is disabled

gopherbot avatar Oct 21 '23 21:10 gopherbot

What about removing -ldflags only if it contains a path separator (/, \)?

dolmen avatar Nov 10 '23 09:11 dolmen

What about having special info in buildinfo to mention that -ldflags was used for build but is omitted there?

dolmen avatar Nov 10 '23 09:11 dolmen

@dolmen, I think the right solution here is probably to parse the ldflags and replace absolute references in flags with with environment variables when possible (such as $GOROOT or $GOPATH or $GOMODCACHE).

On the other hand, if we can't identify a specific variable that refers to the path passed in -ldflags then I'm not sure what we should do about it. 🤔

bcmills avatar Nov 10 '23 15:11 bcmills

Is go build protected against shell injection via -ldflags?

I'm thinking about the case where a tool that rebuilds binaries would take the raw ldflags value from a binary and pass it to go build...

dolmen avatar Nov 13 '23 23:11 dolmen

go build disallows many flags in #cgo directives (search for CGO_CFLAGS_ALLOW in https://pkg.go.dev/cmd/cgo). I don't know whether the -ldflags flag itself has those protections.

bcmills avatar Nov 14 '23 16:11 bcmills

Change https://go.dev/cl/582055 mentions this issue: cmd/go: report trimpath erasing ldflags, and allow override

gopherbot avatar Apr 26 '24 18:04 gopherbot

Hi, I am proposing to add a flag to allow override, and emit ldflags despite trimpath active.

Another option is to a heuristic to allow ldflags..... as long as it doesn't have any slashes.

Separately, other commands allow substituting prefixes, so maybe an option to regexp substitute things in ldflags would work too?

@matloob requested to discuss this further, so happy to discuss more.

Note, that my proposal to override ldflags should not result in any detrimental behavour w.r.t. reproducible builds by default.

xnox avatar Apr 30 '24 14:04 xnox

@matloob which one of the above solutions are desired? the currently proposed PR addresses the outstanding TODO.

xnox avatar May 02 '24 17:05 xnox

@matloob @michaelmatloob any comments?

xnox avatar May 22 '24 00:05 xnox

I think the best option is to do what the TODO suggests and what Bryan suggested in https://github.com/golang/go/issues/63432#issuecomment-1805916772 and parse the ldflags to see if it contains known paths.

matloob avatar May 22 '24 20:05 matloob

Agree with @matloob and @bcmills because removing the ldflags when there are slashes would remove them when injecting data into modules like here:

https://www.digitalocean.com/community/tutorials/using-ldflags-to-set-version-information-for-go-applications

LaurentGoderre avatar Jul 17 '24 15:07 LaurentGoderre

Was -trimpath ldflags stripping got added to get to reproducible builds of go toolchain itself? in that case the goal is to "preserve as many ldflags as possible" but "without breaking reproducibility of the go toolchain builds itself" +/- any other bugs?

Cause absolute and relative paths are ok, as long as they are stable. It's when they are dynamic the binaries become non reproducible.

xnox avatar Jul 17 '24 16:07 xnox

Change https://go.dev/cl/608818 mentions this issue: cmd/go/internal/load: trim paths from ldflags instead of removing them

gopherbot avatar Aug 27 '24 22:08 gopherbot