Repository navigation
Take each upstream's allow-list whole from the source that wins it [patch] - #123
Open
matt-edmondson wants to merge 1 commit into
Open
matt-edmondson wants to merge 1 commit into
matt-edmondson wants to merge 1 commit into
Conversation
…atch] Configuration merges arrays by index, so a --config file naming one repository over an appsettings.json in the working directory naming two bound the file's repository plus the second from appsettings.json, allowing a repository the operator never named. ResolveAllowLists now takes each upstream's Repositories from the last source that sets any entry of it, with --allow still beating every source, and ReplaceAllowLists assigns that list after binding. Fixes #120 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW4iaJ7gETqBaPbNzq1
|
5 tasks
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.
Fixes #120
What was wrong
RunAsyncadds the--configfile after the builder has loadedappsettings.jsonfrom the working directory, so that the explicitly named file wins. For scalar settings it does. ForUpstreams:{name}:Repositoriesit doesn't, because .NET configuration merges arrays by index. Suppose the--configfile namesstudio/game.gitand a strayappsettings.jsonnamesstudio/sandbox.gitandstudio/secret.git. The bound list was then[studio/game.git, studio/secret.git], which allows a repository the operator never named.What changed
Program.ResolveAllowLists(IConfigurationRoot, flagAllowLists). For each configured upstream, it takes the wholeRepositorieslist from the last configuration provider that sets any entry of it, ordered by numeric index.--allowlists still beat every source.RunAsyncpasses the resolved lists to the existingReplaceAllowListspost-configure step, which assigns each list after binding and so replaces it rather than patching it. This is the approach--allowcombined with--configoverwrites the config file's first allow-list entries instead of appending, silently dropping repositories #46 took for--allow.Testing
ConfigFileAllowListTestslayers anappsettings.json-style source under a--configsource with a shorter list (GitHubcased differently on purpose). It checks:--configlist wins whole (studio/game.gitonly).--configfile doesn't name keeps its list.BaseUrlfrom the lower file still binds.--allowbeats both files.ConfigFile_ShorterList_ReplacesTheAppSettingsListfails with the issue's symptom. With it enabled, the test passes. The other four are guards that pass either way.Generated by Claude Code