Skip to content

🎯 Fix StringSliceFlag Precedence: CLI Arguments Should Override EnvVars Instead of Appending #1

Description

@Tylerx3udv

📝 Description

There is a precedence violation in how StringSliceFlag handles values when both an environment variable (via EnvVars) and command-line arguments are provided.

Currently, if a StringSliceFlag is configured with an environment variable source and the user also explicitly provides the flag via the command line, the CLI values are appended to the values retrieved from the environment variable.

According to standard CLI configuration precedence rules, explicit command-line arguments must completely override environment variables. The current behavior leads to duplicated or mixed configuration states, causing unexpected behavior in downstream applications that rely on these flags for clean lists (e.g., webhook URLs, allowed origins, or include/exclude lists).

🎯 Acceptance Criteria

  • When a StringSliceFlag is populated via an environment variable and explicitly passed via command-line arguments, the command-line arguments must completely replace/override the environment variable values.
  • If only the environment variable is present, the flag must resolve to the environment variable values.
  • If only the command-line arguments are present, the flag must resolve to the command-line values.
  • The fix must not break existing default value behavior (i.e., environment variables should still override hardcoded defaults, and CLI arguments should override both).
  • This correct precedence behavior should apply consistently to all slice-type flags (e.g., IntSliceFlag, Float64SliceFlag) if they share the same underlying parsing logic.

🛠️ Technical Specifications & Context

In the Tylerx3udv/cli codebase (which inherits from the urfave/cli architecture):

  • Flags are typically defined in files like flag_string_slice.go, flag_int_slice.go, etc.
  • The underlying value type (e.g., StringSlice in string_slice.go) implements the flag.Value interface, where the Set(string) error method is called during parsing.
  • Currently, when environment variables are processed, they call Set() on the flag's value. When the command-line arguments are subsequently parsed, they also call Set(), which appends the new values to the existing slice.
  • Proposed Solution:
    • Introduce a tracking mechanism (e.g., a boolean flag like hasBeenSetByCLI or tracking the source of the set operation) within the slice value types or the context parser.
    • Alternatively, clear the slice the first time Set() is called by a command-line argument parser if it was previously populated only by EnvVars or Default values.
    • Check flag.go and the parsing loop where Apply or Parse is executed to ensure the transition from environment variable application to command-line argument parsing properly handles resetting or overriding slice values.

🧪 Verification & Testing

Automated Tests

Add a new test case in flag_string_slice_test.go (and equivalent slice flag test files):

func TestStringSliceFlag_EnvVarAndCLIPresedence(t *testing.T) {
    t.Setenv("MY_APP_ALLOWED_ORIGINS", "http://env1.com,http://env2.com")

    app := &App{
        Flags: []Flag{
            &StringSliceFlag{
                Name:    "origin",
                EnvVars: []string{"MY_APP_ALLOWED_ORIGINS"},
            },
        },
        Action: func(ctx *Context) error {
            origins := ctx.StringSlice("origin")
            expected := []string{"http://cli-only.com"}
            if !reflect.DeepEqual(origins, expected) {
                t.Errorf("expected %v, got %v", expected, origins)
            }
            return nil
        },
    }

    // Run with explicit CLI flag overriding the env var
    err := app.Run([]string{"app", "--origin", "http://cli-only.com"})
    if err != nil {
        t.Fatal(err)
    }
}

Manual Verification

  1. Define a command with a StringSliceFlag named --target and env var TARGETS.
  2. Run: TARGETS="a,b" go run main.go --target c --target d
  3. Verify the resolved slice is ["c", "d"] and NOT ["a", "b", "c", "d"].

Opire Bounty


This repo is using Opire - what does it mean? 👇
💵 Everyone can add rewards for this issue commenting /reward 100 (replace 100 with the amount).
🕵️‍♂️ If someone starts working on this issue to earn the rewards, they can comment /try to let everyone know!
🙌 And when they open the PR, they can comment /claim #1 either in the PR description or in a PR's comment.

🪙 Also, everyone can tip any user commenting /tip 20 @Tylerx3udv (replace 20 with the amount, and @Tylerx3udv with the user to tip).

📖 If you want to learn more, check out our documentation.

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions