Caleb Madara commented on issue #8258 at GitLab.org / cli

Caleb Madara
2026-04-16T04:51:49Z

Hi! @Aayush-Saini I'd love to work on this issue 🙌

After looking at the failing job logs and digging into the codebase, I believe I've found the root cause of the race condition.

Root Cause

The race originates in internal/cmdutils/flag.go at lines 10–11:

func GroupOverride(cmd *cobra.Command) (string, error) {
viper.SetEnvPrefix("GITLAB") // line 10
viper.BindEnv("group") // line 11
...
}

viper is a popular Go configuration library. When you call top-level functions like viper.BindEnv() and viper.SetEnvPrefix(), they write to a single global *Viper object shared across the entire process. You can see this in viper's own source:

var v *Viper // one global instance, lives for the whole process

func init() { v = New() }

func BindEnv(input ...string) error { return v.BindEnv(input...) } // writes to v.env (a map)
func SetEnvPrefix(in string) { v.SetEnvPrefix(in) } // writes to v.envPrefix

v.env is a plain Go map and maps in Go are not goroutine-safe. Writing to the same map from two goroutines simultaneously without synchronization is a data race.

Why Tests Trigger It But Production Does Not

In production: every glab command invocation is a separate OS process. Currently SetEnvPrefix and BindEnv run inline inside GroupOverride() on every execution but since each process only ever runs one command, there is never any parallelism. No race.

In tests: all tests run inside one single process. The parallel subtests in internal/commands/token/rotate/rotate_test.go all call GroupOverride() at the same time, which means multiple goroutines are simultaneously writing to the same global viper map triggering the race detector.

This is why the bug is transient: it only appears when the goroutines happen to overlap in timing.

Reproducing It

You can reproduce the race locally by running:

go test -race -count=5 ./internal/commands/token/rotate/...

You should see output like:

WARNING: DATA RACE
Write at ... by goroutine 21:
github.com/spf13/viper.(*Viper).BindEnv()
.../viper.go:1121
gitlab.com/gitlab-org/cli/internal/cmdutils.GroupOverride()
internal/cmdutils/flag.go:11

Previous write at ... by goroutine 29:
github.com/spf13/viper.(*Viper).BindEnv()
.../viper.go:1121
gitlab.com/gitlab-org/cli/internal/cmdutils.GroupOverride()
internal/cmdutils/flag.go:11

Proposed Fix

viper.SetEnvPrefix() and viper.BindEnv() are one-time setup calls they register configuration, they don't need to run on every command execution. Moving them to an init() function ensures they run exactly once per process, before any goroutine starts:

func init() {
viper.SetEnvPrefix("GITLAB")
viper.BindEnv("group") //nolint:errcheck
}

func GroupOverride(cmd *cobra.Command) (string, error) {
// No more setup here viper already configured
groupFromEnv := viper.GetString("group")
...
}

This works correctly in both scenarios:

Production: init() runs once when the process starts, same as before.

Tests: init() runs once before all parallel subtests, no concurrent writes to viper, race eliminated.

Happy to open an MR with this fix if the approach looks good.