fix(settings): trust-split save so /settings changes survive the P0-1 allowlist#105
Merged
Merged
Conversation
… allowlist P0-1 (#103) made the loader drop non-allowlisted keys from the project settings.json, but save() still dumped every field there: each /settings save recreated the untrusted_project_setting_ignored warning storm, and a change to a user-scoped key (e.g. stateful_executor_backend) was silently reverted on the next start. save() now mirrors the load-side trust model: - project file gets ONLY allowlisted keys, fully rewritten (heals stale pre-P0-1 kitchen-sink files) - user ~/.{app}/settings.json gets user-scoped keys that differ from the settings class default, via read-merge-write that preserves unmanaged keys (hand-stored secrets, domain keys); reverting a key to its default removes it so the revert sticks and code-default changes keep applying - explicit save(path=...) keeps the legacy single-file full dump PROJECT_SETTABLE_KEYS moves to settings_persistence (config.py imports it; the reverse would be circular) so writer and reader share one allowlist. save() returns SettingsSaveResult; /settings reports both paths. Claude-Session: https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv
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
P0-1 (#103) added a deny-by-default allowlist on the load side:
_AllowlistFilterSourcedrops non-allowlisted keys from the project./.{app}/settings.json. ButSettingsPersistence.save()(used by/settings) still dumped every field to the project file, so writer and reader disagreed:/settingssave recreated theuntrusted_project_setting_ignoredwarning storm on the next launch (observed in research_demo: 23 warnings from a stale kitchen-sink file);/settingschange to a user-scoped key (e.g.stateful_executor_backend,raw_llm_logging) was written to the project file and then silently reverted on the next start.Fix
save()now mirrors the load-side trust model:PROJECT_SETTABLE_KEYS, fully rewritten — which also heals stale pre-P0-1 kitchen-sink files.~/.{app}/settings.jsonreceives user-scoped keys that differ from the settings class default, via read-merge-write: unmanaged keys (hand-stored API keys, domain keys) are preserved verbatim; a malformed user file raises instead of being clobbered; reverting a key to its default removes it, so reverts stick and future code-default changes keep applying to untouched fields.save(path=...)keeps the legacy single-file full dump.PROJECT_SETTABLE_KEYSmoves tosettings_persistence.py(config.py imports it; the reverse import would be circular), so the filter and the split cannot drift apart.save()returns aSettingsSaveResultand/settingsreports both destinations.Tests
TDD: 8 new tests in
TestTrustSplitSavewritten first and watched fail — including the round-trip regression (stateful_executor_backendchange must survive save + fresh load with zero untrusted-key warnings), healing of a stale project file, merge preservation of hand-stored secrets, and revert removal. Full offline suite: 1977 passed (-m 'not llm and not docker').https://claude.ai/code/session_01SwkKXQpy3JLEqrPkQA8QRv