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

[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.

[sethvargo]

Hmm that's definitely a fun bug. I think the right approach would be to allow escaping the expansion (like \$). What do you think?

[rodkevich]

Hmm that's definitely a fun bug. I think the right approach would be to allow escaping the expansion (like \$). What do you think?

Escaping the $ with \$ could solve the issue, but, i believe, it might introduce breaking changes for many users.
May be, a custom placeholder for regex at the end, like os_expand=false could be used to prevent the expansion of certain variables, like MY_VAR, default=some$value,envconfig_literal_value. This would give users more control over how variables are expanded, without altering the current default behavior.

[rodkevich]

@sethvargo please review. Can be done this way

type MyStruct struct { Token string env:"TOKEN, default.raw=this^will$be&used|as-is" }
Or better raw,default=value for example idk
The problem here -if we remove 'default' from patten, it becomes less clear that the provided value will be used as a fallback if the environment variable is not found.
Another problem is that it would be better by default to lookup without os.Expand and expand should be an option

[sethvargo]

Hi @rodkevich - this isn't a use case I'd like to support. I've intentionally kept the API as succinct as possible. I think the best path forward is to add something a new option like "noexpand" that disables $ expansion.

[rodkevich]

Hi @rodkevich - this isn't a use case I'd like to support. I've intentionally kept the API as succinct as possible. I think the best path forward is to add something a new option like "noexpand" that disables $ expansion.

This is not just an issue with os.Expand, but also with other features specific to [default] behavior.
For example, commas can trim values, and may be other special symbols can modifiy string unexpectedly.

The main concern is how to disable these automatic transformations and use the environment variable’s literal value as-is, without any further processing

The issue is that the person writing the code and the one setting the environment variables are not always the same. They might not even realize what’s happening — everything seems correct in the manifests, and the app worked in one environment, but in another, it doesn’t work. =)

From my perspective, the simplest solution would be to introduce an option that completely disables all modifications — something like raw=, literal=, or strict=, depending on preference. This would allow developers to explicitly opt out of processing, while keeping the default behavior for cases where they are 100% confident no issues will arise.

It would be helpful if you could provide an example of how you envision this working with JSON tags