Skip to content

fix: stop expand from recursing forever on a cyclic reference #447

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
abo3losh1 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: stop expand from recursing forever on a cyclic reference #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1a9c839
fix: stop expand from recursing forever on a cyclic reference
abo3losh1 Oct 6, 2026
d80eca7
test: cover cyclic references with expand
abo3losh1 Oct 6, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 19 additions & 7 deletions env.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -176,12 +176,24 @@ type Options struct {
rawEnvVars map[string]string
}

func (opts *Options) getRawEnv(s string) string {
val := opts.rawEnvVars[s]
if val == "" {
val = opts.Environment[s]
}
return os.Expand(val, opts.getRawEnv)
// expand replaces ${var} or $var in val, expanding the values it refers to
// as well. visiting holds the variables being expanded, so a reference back
// to one of them (e.g. A=${A}) expands to an empty string instead of
// recursing until the stack overflows.
func (opts *Options) expand(val string, visiting map[string]bool) string {
return os.Expand(val, func(s string) string {
if visiting[s] {
return ""
}
visiting[s] = true
defer delete(visiting, s)

raw := opts.rawEnvVars[s]
if raw == "" {
raw = opts.Environment[s]
}
return opts.expand(raw, visiting)
})
}

func defaultOptions() Options {
Expand Down Expand Up @@ -601,7 +613,7 @@ func get(fieldParams FieldParams, opts Options) (val string, err error) {
)

if fieldParams.Expand {
val = os.Expand(val, opts.getRawEnv)
val = opts.expand(val, map[string]bool{fieldParams.Key: true})

@caarlos0 caarlos0 Oct 7, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Preserve valid references to earlier cached values. Marking the current key as visited before reading the cache rejects references that are not cyclic. For example:

type config struct {
    RawPort string `env:"PORT" envDefault:"3000"`
    Port    int    `env:"PORT,expand" envDefault:"${PORT}"`
}

var cfg config
err := ParseWithOptions(&cfg, Options{
    Environment: map[string]string{},
})

The first field caches "3000" under PORT. The second field should read that value, but the cycle guard returns an empty string before checking the cache. Parsing returns no error and leaves Port at 0. I confirmed that the same test passes on the base with Port == 3000 and fails on this head with Port == 0.

Do not mark the current key as cyclic when its reference resolves to a nonempty value cached by an earlier field. Keep the recursive cycle guard. Add this case as a regression test with an explicitly empty environment map, asserting no error and both field values.

}

opts.rawEnvVars[fieldParams.Key] = val
Expand Down
23 changes: 23 additions & 0 deletions env_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1051,6 +1051,29 @@ func TestParseExpandWithDefaultOption(t *testing.T) {
isEqual(t, "3000:5000", cfg.NoDefault)
}

func TestParseExpandCyclicReference(t *testing.T) {
type config struct {
Self string `env:"SELF,expand"`
Indirect string `env:"INDIRECT,expand"`
Twice string `env:"TWICE,expand"`
}

t.Setenv("SELF", "a-${SELF}")
t.Setenv("INDIRECT", "b-${LOOP_A}")
t.Setenv("LOOP_A", "${LOOP_B}")
t.Setenv("LOOP_B", "${LOOP_A}")
t.Setenv("TWICE", "${ONE}${ONE}")
t.Setenv("ONE", "1")

cfg := config{}
err := Parse(&cfg)

isNoErr(t, err)
isEqual(t, "a-", cfg.Self)
isEqual(t, "b-", cfg.Indirect)
isEqual(t, "11", cfg.Twice)
}

func TestParseUnsetRequireOptions(t *testing.T) {
type config struct {
Password string `env:"PASSWORD,unset,required"`
Expand Down
Loading