fix(config): layer project .cu.yml correctly and stop Save leaking it into global config - #43
Open
timimsms wants to merge 1 commit into
Open
fix(config): layer project .cu.yml correctly and stop Save leaking it into global config#43timimsms wants to merge 1 commit into
timimsms wants to merge 1 commit into
Conversation
Project config was merged with viper.Set, which writes viper's override slot — outranking flags and environment variables and inverting the documented precedence. It now merges via MergeConfigMap into the config layer, so the chain is flags > env > project .cu.yml > global config > defaults. Save serialized the whole merged viper state into the global file, so `cu config set` inside any project baked that project's values into ~/.config/cu/config.yaml. Save now starts from the file on disk and applies only values written through Set. Together these were worse than a precedence bug: with the released binary, a project .cu.yml containing api_token overwrote the real token in the global config. Credential keys from a project file are now dropped with a warning, per the context-layer design's credential blocklist. Config loading also moves entirely into config.Init. It previously ran in both cobra.OnInitialize and PersistentPreRunE, in that order, so any re-read could clobber the merged layer. Fixes #37 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014aqbmccWm1tqttmBUCR5rv ClickUp: 86dxbeqyt
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
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.
Summary
Fixes the two halves of #37: project
.cu.ymlwas loaded at the wrong precedence, andSavewrote far more than it should.Precedence. Project config merged with
viper.Set()— viper's override slot, which outranks everything. The documented chain isflags > env > project > global; the actual behaviour wasproject > flags > env > global. It now merges viaMergeConfigMapinto the config layer, so project values override the global file while still losing to env and flags.Save.
Save()serialized the entire merged viper state into the global file, socu config setinside any project baked that project's values into~/.config/cu/config.yaml. It now starts from the file on disk and applies only values written throughSet.Why this is more than a precedence bug. Because those two combine, a project
.cu.ymlcontainingapi_tokenwas written into the global config, replacing the real token. Reproduced against v0.1.0:Any cloned repository could substitute the credential used for API calls. Credential keys from a project file are now dropped with a warning, implementing the credential blocklist from the accepted context-layer design (§2.4).
Same fixture on this branch:
Also: config loading moves entirely into
config.Init. It previously ran in bothcobra.OnInitializeandPersistentPreRunE, in that order, so a re-read could clobber the merged layer.Tests
New
TestProjectConfigPrecedence(project > global, env > project, flag > project, credentials ignored) andTestSaveDoesNotLeakProjectConfig. All four precedence cases also verified end-to-end with a built binary against a sandboxedHOME.Checklist
./scripts/ci.shpasses locally — excepterrcheck, which reports the same 27 pre-existing findings onmain, none in files this PR touches