datasource: stop applying settings.xml auth to added Maven registries#1999
Open
1seal wants to merge 1 commit intogoogle:mainfrom
Open
datasource: stop applying settings.xml auth to added Maven registries#19991seal wants to merge 1 commit intogoogle:mainfrom
1seal wants to merge 1 commit intogoogle:mainfrom
Conversation
0763083 to
82fd540
Compare
Collaborator
|
Thank you! I'm not sure this is a behavior we want though, we are trying to match the ecosystem tooling behavior, so users shouldn't be running this extractor on untrusted input. Adding this feature might just be giving users false confidence on running on untrusted pom.xml files. On the other hand we might want something like this for a hardened scalibr scanning mode while still enabling these features. @cuixq is probably best to review this as they have the most context, though they are currently OOO for a few weeks, so the review will be a bit delayed. |
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
settings.xmlauthentication for the default or user-selected Maven registrypom.xmlcontentWhy
settings.xmlcredentials are keyed only by Maven server ID. When a scannedpom.xmladds a registry with an attacker-controlledidandurl, the current client can pair that ID with local credentials and forward them after a401challenge.Additional registries discovered from scanned project content are untrusted input, so they should not inherit
settings.xmlauthentication by default.Refs google/osv-scanner#2672
Testing
go test ./clients/datasource -run 'Test(DefaultRegistryUsesSettingsAuth|TestAddedRegistryDoesNotUseSettingsAuth)'go test ./clients/resolution/... ./common/... ./guidedremediation/...