Compose named lock keys one segment per coordinate field - #2091
Merged
Merged
Conversation
The filesystem-friendly GAV/GAECV name mappers now pass every coordinate field through PathUtils.stringToPathSegment, and BasedirNameMapper rejects lock names that do not resolve under the locks base directory, so the resolved lock path is always contained in it.
slachiewicz
added
bug
slachiewicz
changed the title
2.0.22 has already been released without these methods.
gnodet
approved these changes
Aug 31, 2026
gnodet
left a comment
gnodet
left a comment
Contributor
There was a problem hiding this comment.
Well-crafted security hardening with a sound two-layer defense:
fieldToSegmentsanitization —PathUtils.stringToPathSegmentreplaces all illegal path characters, preventing coordinate fields from introducing path separators that could escape the locks directory.BasedirNameMapper.resolveContainedcontainment check — standardnormalize()+startsWith()pattern, correctly markedprivate staticto prevent subclass override. Provides defense-in-depth against any delegate that emits traversal sequences.
The HashingNameMapper path is inherently safe since it hashes the entire name into a fixed hex string before resolution, eliminating attacker-controlled content.
Minor observations (not blocking):
- Lock names will change for coordinates that previously contained path separator characters (rare/malicious) — worth a release notes mention.
- No
GAECVNameMapperTestexists, though the mechanism is inherited fromGAVNameMapperand tested there.
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
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.
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.
The named lock mappers build a lock name by concatenating artifact coordinate fields. The fields are
inserted as-is, so a field containing a separator produces a name with more segments than intended.
BasedirNameMapperthen resolves that name against the locks base directory, and the file lockfactory creates and deletes a lock file at the resolved path.
Two changes:
GAVNameMapperandGAECVNameMappercompose each coordinate field throughfieldToSegment, so afield always contributes exactly one segment.
BasedirNameMapperresolves the delegate-provided name against the base directory and requires theresult to stay under it, since that is where lock files are created and removed.
Marked
@since 2.0.23. No public API changes.