diff --git a/proxy/config/config.go b/proxy/config/config.go index c4387f4..945a9d4 100644 --- a/proxy/config/config.go +++ b/proxy/config/config.go @@ -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 diff --git a/proxy/config/config_test.go b/proxy/config/config_test.go index a19cbb5..49bbdc9 100644 --- a/proxy/config/config_test.go +++ b/proxy/config/config_test.go @@ -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) + }) + }