Skip to content

🎯 Fix: Environment Variable Values Ignored for Slice Flags When Multiple Sources Are Present #1

Description

@thaohuynh14zc

📝 Description

There is an issue in the flag parsing lifecycle where environment variables (defined via EnvVars) for slice flags (such as StringSliceFlag, IntSliceFlag, etc.) are ignored or incorrectly overridden when multiple configuration sources (e.g., Default values, command-line arguments, or parent context settings) are present.

Specifically, when a slice flag has a defined Value (default value) and an environment variable is set, the environment variable's value is either completely ignored or fails to override the default value. According to the standard precedence rules of urfave/cli, environment variables should take precedence over default values, but command-line arguments should take precedence over environment variables. Currently, the slice flag parsing logic does not correctly merge or prioritize these sources, leading to unexpected default values being used instead of the environment variable values.

🎯 Acceptance Criteria

  • Environment variables specified in EnvVars for all slice flags (StringSliceFlag, IntSliceFlag, Float64SliceFlag, etc.) must be correctly parsed and applied.
  • Precedence Order: The resolution order for slice flags must strictly adhere to:
    1. Command-line arguments (highest priority)
    2. Environment variables (takes precedence over defaults)
    3. Default values / Value field (lowest priority)
  • Parsing Behavior: If an environment variable is used for a slice flag, it should support splitting comma-separated values (e.g., MY_FLAG=val1,val2 should result in []string{"val1", "val2"}).
  • If multiple environment variables are defined in the EnvVars slice, the first one that is set in the environment must be used, and subsequent ones ignored.
  • The fix must not introduce regressions for non-slice flags or standard CLI argument parsing.

🛠️ Technical Specifications & Context

The issue likely resides in the flag application and parsing lifecycle, specifically within the Apply methods of the slice flag implementations or the helper functions that resolve environment variables.

Key Files to Investigate:

  • flag_string_slice.go (specifically StringSliceFlag.Apply)
  • flag_int_slice.go (specifically IntSliceFlag.Apply)
  • Other slice flag files (e.g., flag_int64_slice.go, flag_float64_slice.go)
  • flag.go (where general flag application and environment variable helper functions reside)

Implementation Details:

  1. In urfave/cli, flags implement the Apply(*flag.FlagSet) error interface.
  2. During Apply, the flag checks if environment variables are set using helper functions (e.g., flag.go environment helpers).
  3. For slice flags, if an environment variable is found, its value needs to be split (usually by comma) and set on the underlying slice value.
  4. Ensure that if the environment variable is present, it overrides the default Value initialization. Currently, the default value initialization might be overwriting the environment variable value, or the environment variable value is parsed but then cleared/overwritten when the flag set initializes defaults.
  5. Check if the flag is marked as "seen" or "set" correctly so that subsequent parsing stages know the source of the value.

🧪 Verification & Testing

Automated Tests

Add unit tests in flag_string_slice_test.go and flag_int_slice_test.go to verify the precedence:

  1. Test Env Var Overrides Default: Set a default value on a StringSliceFlag, set the corresponding environment variable, run the app without CLI arguments, and assert that the slice contains only the environment variable values.
  2. Test CLI Overrides Env Var: Set a default value, set the environment variable, pass CLI arguments, and assert that the slice contains only the CLI argument values.
  3. Test Comma Separation: Set an environment variable to foo,bar,baz and assert that the parsed slice is []string{"foo", "bar", "baz"}.

Minimal Reproducible Example for Manual Verification

Create a test harness or run the following Go code:

package main

import (
	"fmt"
	"os"

	"github.com/urfave/cli/v2" // Adjust import path based on repository structure
)

func main() {
	app := &cli.App{
		Flags: []cli.Flag{
			&cli.StringSliceFlag{
				Name:    "test-slice",
				EnvVars: []string{"TEST_SLICE"},
				Value:   cli.NewStringSlice("default1", "default2"),
			},
		},
		Action: func(c *cli.Context) error {
			fmt.Printf("Slice values: %v\n", c.StringSlice("test-slice"))
			return nil
		},
	}

	// Scenario: Env Var is set, no CLI args passed.
	// Expected Output: Slice values: [env1 env2]
	// Actual Output (Bug): Slice values: [default1 default2]
	os.Setenv("TEST_SLICE", "env1,env2")
	
	if err := app.Run([]string{"app"}); err != nil {
		fmt.Println(err)
	}
}

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 @thaohuynh14zc (replace 20 with the amount, and @thaohuynh14zc 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