proxy/config: skip env macros in YAML comment lines (#496)

Fix a bug where ${env.macro_not_exist} in comments would trigger a non-substituted macro error. 

fixes #495
This commit is contained in:
Benson Wong
2026-01-30 20:10:29 -08:00
committed by GitHub
parent 5de387dbf9
commit cdea7d16bd
2 changed files with 89 additions and 4 deletions
+25 -4
View File
@@ -667,11 +667,32 @@ func substituteMacroInValue(value any, macroName string, macroValue any) (any, e
}
}
// substituteEnvMacros replaces ${env.VAR_NAME} with environment variable values
// Returns error if any env var is not set or contains invalid characters
// substituteEnvMacros replaces ${env.VAR_NAME} with environment variable values.
// Returns error if any referenced env var is not set or contains invalid characters.
// Env macros inside YAML comments are ignored by unmarshalling the YAML first
// (which strips comments) and only checking the comment-free version for macros.
func substituteEnvMacros(s string) (string, error) {
result := s
matches := envMacroRegex.FindAllStringSubmatch(s, -1)
// Unmarshal and remarshal to strip YAML comments
var raw any
if err := yaml.Unmarshal([]byte(s), &raw); err != nil {
// If YAML is invalid, fall back to scanning the original string
// so the user gets the env var error rather than a confusing YAML parse error
return substituteEnvMacrosInString(s, s)
}
clean, err := yaml.Marshal(raw)
if err != nil {
return substituteEnvMacrosInString(s, s)
}
return substituteEnvMacrosInString(s, string(clean))
}
// substituteEnvMacrosInString finds ${env.VAR} macros in scanStr and substitutes
// them in target. This separation allows scanning comment-free YAML while
// substituting in the original string.
func substituteEnvMacrosInString(target, scanStr string) (string, error) {
result := target
matches := envMacroRegex.FindAllStringSubmatch(scanStr, -1)
for _, match := range matches {
fullMatch := match[0] // ${env.VAR_NAME}
varName := match[1] // VAR_NAME
+64
View File
@@ -1308,4 +1308,68 @@ peers:
assert.Contains(t, err.Error(), "peers.openrouter.filters.setParams")
assert.Contains(t, err.Error(), "unknown macro")
})
t.Run("env macros in comments are ignored", func(t *testing.T) {
content := `
# apiKeys:
# - "${env.COMMENTED_OUT_KEY_1}"
# - "${env.COMMENTED_OUT_KEY_2}"
models:
test:
cmd: "server"
proxy: "http://localhost:8080"
`
// These env vars are NOT set, but should not cause an error
// because they only appear in comment lines
config, err := LoadConfigFromReader(strings.NewReader(content))
assert.NoError(t, err)
assert.Empty(t, config.RequiredAPIKeys)
})
t.Run("env macros in comments ignored while active ones resolve", func(t *testing.T) {
t.Setenv("TEST_ACTIVE_KEY", "active-key-value")
content := `
# apiKeys: ["${env.COMMENTED_OUT_KEY}"]
apiKeys: ["${env.TEST_ACTIVE_KEY}"]
models:
test:
cmd: "server"
proxy: "http://localhost:8080"
`
config, err := LoadConfigFromReader(strings.NewReader(content))
assert.NoError(t, err)
assert.Equal(t, []string{"active-key-value"}, config.RequiredAPIKeys)
})
t.Run("env macros in indented comments are ignored", func(t *testing.T) {
content := `
models:
test:
cmd: |
server
--port 8080
proxy: "http://localhost:8080"
# metadata:
# api_key: "${env.SOME_UNSET_KEY}"
`
_, err := LoadConfigFromReader(strings.NewReader(content))
assert.NoError(t, err)
})
t.Run("env macros in inline comments are ignored", func(t *testing.T) {
t.Setenv("TEST_INLINE_KEY", "real-value")
content := `
apiKeys: ["${env.TEST_INLINE_KEY}"] # TODO: add ${env.FUTURE_KEY} later
models:
test:
cmd: "server"
proxy: "http://localhost:8080"
`
config, err := LoadConfigFromReader(strings.NewReader(content))
assert.NoError(t, err)
assert.Equal(t, []string{"real-value"}, config.RequiredAPIKeys)
})
}