Validate the repository key used as a local repository path segment - #2089
Merged
Merged
Conversation
stringToPathSegment maps '.' and '..' to explicit tokens (-DOT-, -DOTDOT-), matching the existing single-character replacements, so repository keys spliced into split local repository prefixes and origin-aware trusted checksums paths are always inert path segments. Add a defensive path-component validation of the repository key at the split prefix composer and the sparse-directory trusted checksums source.
slachiewicz
changed the title
gnodet
approved these changes
Aug 31, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Well-targeted security hardening that closes a real gap in stringToPathSegment (the . and .. cases pass through unchanged since they contain none of the characters in the replacement map) and adds defense-in-depth validation at the two critical call sites where repository keys are used as standalone directory segments.
The layered approach is sound: stringToPathSegment sanitizes at the source (inside RepositoryIdHelper.idToPathSegment), and validatePathComponent acts as a safety net at the point of use.
Minor observations (not blocking):
SummaryFileTrustedChecksumsSource.summaryFile()also uses the repository key in filename construction — could benefit from the same validation in a follow-up (pre-existing, not introduced by this PR).- Backward compatibility risk is negligible: only affects repository IDs that are exactly
.or...
This review was generated by an AI agent and may contain inaccuracies. Please verify all suggestions before applying.
Claude Code on behalf of gnodet
cstamas
approved these changes
Aug 31, 2026
4 tasks
This was referenced Aug 31, 2026
cstamas
pushed a commit
that referenced
this pull request
Aug 31, 2026
Follow-up to the review notes on #2089 and #2091: `SummaryFileTrustedChecksumsSource` splices the repository key into the summary file name without the point-of-use validation the sparse source received, so this adds the same `PathUtils.validatePathComponent` guard. The built-in key functions already sanitize; the guard matters for custom `RepositoryKeyFunction` implementations, and the new test wires a non-sanitizing key function to exercise it. Also adds the `GAECVNameMapperTest` that was noted as missing.
cstamas
pushed a commit
that referenced
this pull request
Aug 31, 2026
The 1.9.x line predates the repository key function layer, so the split local repository prefix (`LocalPathPrefixComposerFactorySupport`, four sites) and the trusted checksums sources (`SummaryFileTrustedChecksumsSource`, `SparseDirectoryTrustedChecksumsSource`) splice the raw repository id into paths. This routes those ids through `PathUtils.stringToPathSegment` and validates the result, matching the 2.x behaviour after #2089 and #2099. Also ports `GAECVNameMapperTest` from master; `GAECVNameMapper` itself needed no change since it inherits the earlier `fieldToSegment` handling from `GAVNameMapper`. Repository ids that are exactly `.`/`..` or contain path separators now compose to a neutralized segment (e.g. `-DOTDOT-`) instead of being spliced verbatim; well-formed ids are unaffected.
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
3 participants
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.
When the local repository is split, the repository key is spliced directly into the local repository
path prefix by
LocalPathPrefixComposerFactorySupport, and into the file name used bySparseDirectoryTrustedChecksumsSource. The key comes from a configurableRepositoryKeyFunction, so its shape is not guaranteed to be a usable single path segment.This validates the key with the existing
PathUtils.validatePathComponentbefore using it, so theprefix composes a valid relative path.
No public API changes.