Skip to content

Issue: Danger handling of environment variables [with missing values] #118

@rodkevich

Description

@rodkevich

Description:

When processing environment variables using os.Expand (specifically when expanding defaultValue), if the environment variable is not found, the current implementation replaces the placeholder with an empty string (""). As a result, the $ symbol and the variable name are removed from the string, leading to unexpected behavior.

Potential Risk:

This issue can cause subtle bugs where a missing environment variable results in the loss of 2 chars.
The team may end up searching for the root cause for hours =)

Modified test from examples:

func Example_defaults() {
	// This example demonstrates how to set default values for fields. Fields will
	// use their default value if no value is provided for that key in the
	// environment.

	type MyStruct struct {
		Port     int    `env:"PORT, default=8080"`
		Username string `env:"USERNAME, default=$OTHER_ENV"`
		Secret   string `env:"SECRET, default=expand^makes$no&problems"`
	}

	var s MyStruct
	if err := envconfig.Process(ctx, &s); err != nil {
		panic(err) // TODO: handle error
	}

	fmt.Printf("port: %d\n", s.Port)
	fmt.Printf("username: %s\n", s.Username)
	fmt.Printf("secret: %s\n", s.Secret)

	// Output:
	// port: 8080
	// username:
	// secret: expand^makes$no&problems
}

--- FAIL: Example_defaults (0.00s)
got:
port: 8080
username: 
secret: expand^makes&problems  

want:
port: 8080
username:
secret: expand^makes$no&problems

FAIL

A possible fix would be to return the extracted part with the dollar sign $ included, like this:

		if defaultValue != "" {
			// Expand the default value. This allows for a default value that maps to
			// a different environment variable.
			val = os.Expand(defaultValue, func(i string) string {
				lookuper := l
				if v, ok := lookuper.(unwrappableLookuper); ok {
					lookuper = v.Unwrap()
				}

				s, ok := lookuper.Lookup(i)
				if ok {
					return s
				}
				--  return ""
				++ return "$" + i
			})

			return val, false, true, nil
		}

However, this approach would alter the original author's assumption that missing environment variables should return an empty string.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions