[thaJeztah]

release notes: https://github.com/spf13/pflag/releases/tag/v1.1.0
ful diff: spf13/pflag@v1.0.10...v1.1.0

Summary

Release notes (optional)

A picture of a cute animal (not mandatory but encouraged)

[docker-agent]

Assessment: 🟡 NEEDS ATTENTION

[docker-agent]

[medium] GetStringToString now returns the flag's internal map by reference — callers that mutate the returned map will silently corrupt flag state

In v1.1.0, GetStringToString was rewritten to return *fv.value directly — a pointer dereference that yields the same map[string]string the flag stores internally. The old implementation produced a fresh copy via stringToStringConv. This means any caller that iterates-and-deletes, assigns new keys, or clears the returned map will silently mutate the flag's stored value without going through Set.

The new doc comment explicitly documents this behaviour ("The returned map shares memory with the internal flag value [Flag.Value]"), so it is an intentional API change — but callers who overlook the note (or who relied on the old copy-on-read semantics) will introduce silent data corruption that is very hard to track down.

Trigger: Any code path that calls GetStringToString and then modifies the returned map — e.g. a for range-and-delete loop, or passing the map to a function that appends or reassigns keys.

Impact: Subsequent calls to flag.Value.String(), Set(), or another GetStringToString will observe the mutated state; this can silently change the behaviour of flags even after they have been fully parsed.

Recommendation: Audit docker/cli call sites of GetStringToString. If none mutate the returned map today, the risk is low; otherwise copy the map before mutating:

m, _ := flags.GetStringToString("my-flag")
// safe defensive copy before any mutation:
copy := make(map[string]string, len(m))
for k, v := range m { copy[k] = v }

Confidence	Score
🟡 moderate	70/100

[docker-agent]

[low] ErrorOnUnknownFlag = iota (= 0) is indistinguishable from the zero value of UnknownFlagsHandling, making it impossible to use the new field to explicitly override a deprecated UnknownFlags = true

ErrorOnUnknownFlag is declared as iota (value 0), which is also the zero value of an uninitialized UnknownFlagsHandling field. The guard in getUnknownFlagsHandling:

if f.ParseErrorsAllowlist.UnknownFlagsHandling != ErrorOnUnknownFlag {
    return f.ParseErrorsAllowlist.UnknownFlagsHandling
}

…is false for both "field not set" and "field explicitly set to ErrorOnUnknownFlag". This means a caller who writes:

fs.ParseErrorsAllowlist.UnknownFlagsHandling = ErrorOnUnknownFlag  // "I want strict mode"
fs.ParseErrorsWhitelist.UnknownFlags = true                        // legacy: set by dependency

…will still get IgnoreUnknownFlag because the allowlist check falls through and the deprecated whitelist UnknownFlags = true takes effect. The explicit assignment to ErrorOnUnknownFlag has no observable effect in this scenario.

Impact: Callers migrating from the deprecated ParseErrorsWhitelist/UnknownFlags to the new ParseErrorsAllowlist/UnknownFlagsHandling field cannot use ErrorOnUnknownFlag to restore strict mode if the deprecated field is still set anywhere in the callchain. This is a subtle correctness issue in a newly-added API.

Note: The common case (no deprecated field set, wanting default strict mode) works correctly — the function falls through to return ErrorOnUnknownFlag at the end. Only the override scenario is broken.

Confidence	Score
🟡 moderate	70/100