fix: resolve default/profile client credential conflict from env vars#44
Open
piotrek-janus wants to merge 2 commits into
Open
fix: resolve default/profile client credential conflict from env vars#44piotrek-janus wants to merge 2 commits into
piotrek-janus wants to merge 2 commits into
Conversation
When both the default client and a profile client are configured through environment variables, the profile silently uses the default client's credentials instead of its own. Two coupled defects cause this: - profile client env vars (PROFILES_<NAME>_CLIENT_*) are never read, so the profile has no client of its own - a profile without its own client falls back to the default client by sharing the same pointer, so it inherits the default's credentials and is aliased to it Add two failing tests (and a fixture) that assert the correct behavior to document the bug.
Two coupled defects caused a profile to silently use the default client's credentials when both were configured via environment variables: - profile client env vars (PROFILES_<NAME>_CLIENT_*) were never read, so a profile had no client of its own - a profile without its own client fell back to the default client by sharing the same pointer, aliasing the two configs Fixes: - bind each default config key per profile and promote any set profile env var into viper's explicit override layer, which is deep-merged on Unmarshal (AutomaticEnv/BindEnv values otherwise do not merge into nested maps that originate from a config file) - deep-copy the default sub-configs on fallback in SetImplicitValues so profiles never share or mutate the default's objects - use an isolated viper instance instead of the global singleton so repeated InitConfig calls do not leak explicit overrides into each other The previously failing tests now pass.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
When both the default client and a profile client were configured through environment variables, the profile silently used the default client's credentials instead of its own. Reported in practice when
CLIENT_*(default) andPROFILES_<NAME>_CLIENT_*(profile) were all set via env.Root cause
Two coupled defects in
internal/cac/config/config.go:AutomaticEnv/BindEnvvalues do not merge into nested maps (profiles.<name>.client.*) that originate from a config file duringUnmarshal, soPROFILES_<NAME>_CLIENT_*was ignored — the profile had no client of its own.*Configurationpointer inSetImplicitValues, so it inherited the default's env-set credentials and was mutation-aliased to it.Fix
v.Set), which is deep-merged onUnmarshal.SetImplicitValuesinstead of sharing pointers.viper.New()instance instead of the globalviper.GetViper()singleton, so repeatedInitConfigcalls don't leak explicit overrides into each other. Nothing else in the repo uses the global instance.Tests
Added two tests (and a fixture
testdata/profile_env_conflict.yaml) that previously failed and now pass:profile client from env does not collide with default client from env— both clients set via env; profile must use its own credentials.profile without client is not aliased to the default client— fallback must produce an independent copy, not a shared pointer.go build ./...,go vet, and the fullgo test ./...suite pass.