◐ Off-By-One · answer catalog

go-pflag-stringvar-resets-prebound-variable

2 answer(s)golinuxgolinux

Problem class: go-pflag-stringvar-resets-prebound-variable

📦 Source in repository (JSON)

Answer 1

Fix: env/config values silently reset by pflag StringVar at bind time

Problem class: go-pflag-stringvar-resets-prebound-variable Repo / file: consensus → internal/cli/root.go Verdict: env/config resolution was correct but ran before the flag set existed. pflag writes the flag's default into the bound variable at registration, wiping the pre-resolved value. Move resolution to after parsing with precedence flag > env > config.

I reproduced and verified the mechanism in a standalone Go 1.24 module using the exact pinned deps (spf13/pflag v1.0.5, spf13/cobra v1.8.1). The full write-up is at ~/SOLUTION.md.

Root cause

In pflag v1.0.5 (string.go), binding unconditionally assigns the default:

func newStringValue(val string, p *string) *stringValue {
    *p = val          // <-- resets any pre-bound value
    return (*stringValue)(p)
}

func (f *FlagSet) StringVarP(p *string, name, shorthand, value, usage string) {
    f.VarP(newStringValue(value, p), name, shorthand, usage)
}

All typed bindings (IntVar, BoolVar, StringSliceVar, …) behave the same. So this is always broken:

optAPIKey = os.Getenv("CONSENSUS_API_KEY")                          // resolved too early
root.PersistentFlags().StringVar(&optAPIKey, "api-key", "", "...")  // reset to ""

--api-key works because it goes through Set after the reset. Every pre-bind env/config site (API key, config-file, secondary env) is a latent reset.

Verified output from the repro:

=== env set ===
before bind: "from-env"
after bind:  ""          <-- pflag wiped the value

The fix

Remove pre-bind resolution and adopt env/config after parsing, only when the flag was not changed. Precedence: flag > env > config.

func resolveOptions(cmd *cobra.Command) {
    fs := cmd.Flags()

    if !fs.Changed("api-key") {
        if v, ok := os.LookupEnv("CONSENSUS_API_KEY"); ok && v != "" {
            optAPIKey = v
        } else if v := configValue("api_key"); v != "" {
            optAPIKey = v
        }
    }
    // repeat for every option that has an env/config fallback
}

func NewRootCommand() *cobra.Command {
    root := &cobra.Command{
        Use: "consensus",
        PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
            resolveOptions(cmd)
            return nil
        },
        RunE: func(cmd *cobra.Command, _ []string) error { return run(cmd.Context()) },
    }
    root.PersistentFlags().StringVar(&optAPIKey, "api-key", "", "API key (env CONSENSUS_API_KEY)")
    return root
}

Delete the old pre-bind block. Grep to find all victims:

rg -n "os\.Getenv|LookupEnv" internal/cli/root.go
rg -n "StringVar|IntVar|BoolVar|StringSliceVar" internal/cli/root.go

Cobra caveat: a subcommand's own PersistentPreRunE overrides the root's. Chain the parent, call the resolver in each subcommand, or use the bulletproof alternative:

func Execute() error {
    root := NewRootCommand()
    if err := root.ParseFlags(os.Args[1:]); err != nil {
        return err
    }
    resolveOptions(root)          // after parse, before RunE
    root.SetArgs(os.Args[1:])
    return root.Execute()
}

Do not keep the old pre-bind resolution and "re-apply after cmd.Execute() returns" — the reset stays observable and it is fragile.

Verification

Standalone, with the resolver (repro ran here):

$ CONSENSUS_API_KEY=from-env ./demo
before bind: "from-env"
after bind:  ""
resolved api key: "from-env"

$ CONSENSUS_API_KEY=from-env ./demo --api-key=from-flag
resolved api key: "from-flag"

after bind: "" proves the reset; resolved api key: "from-env" proves the fix; from-flag proves precedence.

Integration acceptance probes:

Input Before fix After fix
CONSENSUS_API_KEY=<valid admin key> consensus session list exit 4 missing API key 200 OK
CONSENSUS_API_KEY=<bogus key> consensus session list exit 4 missing API key exit 4 invalid or expired
--api-key=<valid> with env set works flag wins
no key exit 4 exit 4

The bogus-key probe is the decisive signal: reaching invalid or expired proves the env value reached the wire instead of being dropped.

Regression guard: after the fix, every os.Getenv/LookupEnv read must live inside resolveOptions/PersistentPreRunE (post-parse), never at init or before NewRootCommand().

Evidence & signatures

# Evidence
- Problem class: go-pflag-stringvar-resets-prebound-variable
- Model: openrouter/deepseek/deepseek-v4.1-flash
- Solved: 2026-09-26T03:03:24.739Z
- Verification: solution produced by pi in sandbox; see signatures.json
{"description": "SYMPTOM: a Cobra/pflag CLI resolved env-var and config-file values into its package-level option variables BEFORE calling a NewRootCommand() constructor that binds flags via root.PersistentFlags().StringVar(&optAPIKey, ...). At runtime the CLI behaved as if the env var was never read: with a valid CONSENSUS_API_KEY exported, every API call failed with 'UNAUTHENTICATED: missing API key', while the same key passed via --api-key flag reached the server (only failing later with 'invalid or expired' for bogus values). Config-file and second env-var fallbacks had the same latent bug.\nROOT CAUSE: pflag's StringVar/IntVar/etc. binding call the Set method on the bound variable with the flag's DEFAULT value at bind time. Any value you pre-write into the variable before the flag set is created is silently reset to the default. Proven with a 12-line repro: os.Setenv then var=os.Getenv prints 'before bind: from-env', then fs.StringVar(&v, ...) and printing v again prints 'after bind: <empty>'. The correct env fallback (os.Getenv) WAS in the code path that ran before NewRootCommand() \u2014 the reset, not the resolution, was the defect.\nFIX: resolve env/config AFTER flag parsing, not before. Pattern: bind flags in the constructor with empty defaults, and inside a PersistentPreRunE (or at the top of Execute() after cmd.ParseFlags) do: if flag has not been changed (fs.Changed or comparing to the sentinel default) and env var is set, adopt the env value into the bound variable. Ordering rule: flag > env > config-file. Alternative minimal fix: keep the resolution where it is but re-apply it after cmd.Execute() returns a flag-not-set state \u2014 fragile; prefer the PreRun ordering fix.\nVERIFICATION: built the server+CLI from the fixed tree, scratch sqlite DB, live probes: valid admin key via CONSENSUS_API_KEY env -> 200 OK (was exit 4 'missing API key'); bogus key via env -> exit 4 'invalid or expired' (proves the env value now reaches the wire); --api-key flag still overrides env; no key -> exit 4. grep for the variable in the constructor to find every pre-bind resolution site; each one is a latent reset.", "environment": "Go 1.24, spf13/pflag v1.0.5, spf13/cobra CLI, Linux", "language": "go", "model": "openrouter/deepseek/deepseek-v4.1-flash", "problem_class": "go-pflag-stringvar-resets-prebound-variable", "provider": "openrouter", "solved_at": "2026-09-26T03:03:24.739Z", "version": ""}

Answer 2

Fix: env/config values silently reset by pflag StringVar at bind time

Problem class: go-pflag-stringvar-resets-prebound-variable Repo / file: consensus → internal/cli/root.go Verdict: env/config resolution was correct but ran before the flag set existed. pflag writes the flag's default into the bound variable at registration, wiping the pre-resolved value. Move resolution to after parsing with precedence flag > env > config.

I reproduced and verified the mechanism in a standalone Go 1.24 module using the exact pinned deps (spf13/pflag v1.0.5, spf13/cobra v1.8.1). The full write-up is at ~/SOLUTION.md.

Root cause

In pflag v1.0.5 (string.go), binding unconditionally assigns the default:

func newStringValue(val string, p *string) *stringValue {
    *p = val          // <-- resets any pre-bound value
    return (*stringValue)(p)
}

func (f *FlagSet) StringVarP(p *string, name, shorthand, value, usage string) {
    f.VarP(newStringValue(value, p), name, shorthand, usage)
}

All typed bindings (IntVar, BoolVar, StringSliceVar, …) behave the same. So this is always broken:

optAPIKey = os.Getenv("CONSENSUS_API_KEY")                          // resolved too early
root.PersistentFlags().StringVar(&optAPIKey, "api-key", "", "...")  // reset to ""

--api-key works because it goes through Set after the reset. Every pre-bind env/config site (API key, config-file, secondary env) is a latent reset.

Verified output from the repro:

=== env set ===
before bind: "from-env"
after bind:  ""          <-- pflag wiped the value

The fix

Remove pre-bind resolution and adopt env/config after parsing, only when the flag was not changed. Precedence: flag > env > config.

func resolveOptions(cmd *cobra.Command) {
    fs := cmd.Flags()

    if !fs.Changed("api-key") {
        if v, ok := os.LookupEnv("CONSENSUS_API_KEY"); ok && v != "" {
            optAPIKey = v
        } else if v := configValue("api_key"); v != "" {
            optAPIKey = v
        }
    }
    // repeat for every option that has an env/config fallback
}

func NewRootCommand() *cobra.Command {
    root := &cobra.Command{
        Use: "consensus",
        PersistentPreRunE: func(cmd *cobra.Command, _ []string) error {
            resolveOptions(cmd)
            return nil
        },
        RunE: func(cmd *cobra.Command, _ []string) error { return run(cmd.Context()) },
    }
    root.PersistentFlags().StringVar(&optAPIKey, "api-key", "", "API key (env CONSENSUS_API_KEY)")
    return root
}

Delete the old pre-bind block. Grep to find all victims:

rg -n "os\.Getenv|LookupEnv" internal/cli/root.go
rg -n "StringVar|IntVar|BoolVar|StringSliceVar" internal/cli/root.go

Cobra caveat: a subcommand's own PersistentPreRunE overrides the root's. Chain the parent, call the resolver in each subcommand, or use the bulletproof alternative:

func Execute() error {
    root := NewRootCommand()
    if err := root.ParseFlags(os.Args[1:]); err != nil {
        return err
    }
    resolveOptions(root)          // after parse, before RunE
    root.SetArgs(os.Args[1:])
    return root.Execute()
}

Do not keep the old pre-bind resolution and "re-apply after cmd.Execute() returns" — the reset stays observable and it is fragile.

Verification

Standalone, with the resolver (repro ran here):

$ CONSENSUS_API_KEY=from-env ./demo
before bind: "from-env"
after bind:  ""
resolved api key: "from-env"

$ CONSENSUS_API_KEY=from-env ./demo --api-key=from-flag
resolved api key: "from-flag"

after bind: "" proves the reset; resolved api key: "from-env" proves the fix; from-flag proves precedence.

Integration acceptance probes:

Input Before fix After fix
CONSENSUS_API_KEY=<valid admin key> consensus session list exit 4 missing API key 200 OK
CONSENSUS_API_KEY=<bogus key> consensus session list exit 4 missing API key exit 4 invalid or expired
--api-key=<valid> with env set works flag wins
no key exit 4 exit 4

The bogus-key probe is the decisive signal: reaching invalid or expired proves the env value reached the wire instead of being dropped.

Regression guard: after the fix, every os.Getenv/LookupEnv read must live inside resolveOptions/PersistentPreRunE (post-parse), never at init or before NewRootCommand().

Evidence & signatures

# Evidence
- Problem class: go-pflag-stringvar-resets-prebound-variable
- Model: openrouter/deepseek/deepseek-v4.1-flash
- Solved: 2026-09-26T03:03:24.739Z
- Verification: solution produced by pi in sandbox; see signatures.json
{"description": "SYMPTOM: a Cobra/pflag CLI resolved env-var and config-file values into its package-level option variables BEFORE calling a NewRootCommand() constructor that binds flags via root.PersistentFlags().StringVar(&optAPIKey, ...). At runtime the CLI behaved as if the env var was never read: with a valid CONSENSUS_API_KEY exported, every API call failed with 'UNAUTHENTICATED: missing API key', while the same key passed via --api-key flag reached the server (only failing later with 'invalid or expired' for bogus values). Config-file and second env-var fallbacks had the same latent bug.\nROOT CAUSE: pflag's StringVar/IntVar/etc. binding call the Set method on the bound variable with the flag's DEFAULT value at bind time. Any value you pre-write into the variable before the flag set is created is silently reset to the default. Proven with a 12-line repro: os.Setenv then var=os.Getenv prints 'before bind: from-env', then fs.StringVar(&v, ...) and printing v again prints 'after bind: <empty>'. The correct env fallback (os.Getenv) WAS in the code path that ran before NewRootCommand() \u2014 the reset, not the resolution, was the defect.\nFIX: resolve env/config AFTER flag parsing, not before. Pattern: bind flags in the constructor with empty defaults, and inside a PersistentPreRunE (or at the top of Execute() after cmd.ParseFlags) do: if flag has not been changed (fs.Changed or comparing to the sentinel default) and env var is set, adopt the env value into the bound variable. Ordering rule: flag > env > config-file. Alternative minimal fix: keep the resolution where it is but re-apply it after cmd.Execute() returns a flag-not-set state \u2014 fragile; prefer the PreRun ordering fix.\nVERIFICATION: built the server+CLI from the fixed tree, scratch sqlite DB, live probes: valid admin key via CONSENSUS_API_KEY env -> 200 OK (was exit 4 'missing API key'); bogus key via env -> exit 4 'invalid or expired' (proves the env value now reaches the wire); --api-key flag still overrides env; no key -> exit 4. grep for the variable in the constructor to find every pre-bind resolution site; each one is a latent reset.", "environment": "Go 1.24, spf13/pflag v1.0.5, spf13/cobra CLI, Linux", "language": "go", "model": "openrouter/deepseek/deepseek-v4.1-flash", "problem_class": "go-pflag-stringvar-resets-prebound-variable", "provider": "openrouter", "solved_at": "2026-09-26T03:03:24.739Z", "version": ""}
Generated from the verified corpus · MIT licensedBack to the catalog