This repository was archived by the owner on Aug 27, 2026. It is now read-only.
Repository navigation
[release/10.0.1xx] Backport Maven hardening - #1495
Closed
jonathanpeppers wants to merge 1 commit into
Closed
jonathanpeppers wants to merge 1 commit into
jonathanpeppers wants to merge 1 commit into
Conversation
Backport cache path containment from #1480 and Maven artifact coordinate validation from #1479, including their targeted tests. Co-authored-by: Copilot App <[email protected]>
There was a problem hiding this comment.
Pull request overview
Backports hardening for Java.Interop.Tools.Maven onto release/10.0.1xx, tightening Maven coordinate validation (Artifact) and ensuring cached artifact paths cannot resolve outside the configured cache directory (CachedMavenRepository). This aligns downstream consumers (e.g., dotnet/android) with the hardened behaviors from main.
Changes:
- Add stricter validation for Maven artifact coordinates in
Artifact(reject malformed group/artifact/version inputs; keep ctor allowance for empty version). - Centralize cache-path computation in
CachedMavenRepository.GetArtifactFilePath()and assert resolved paths remain underCacheDirectory. - Add/extend unit tests for both coordinate validation and cache path hardening.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs | Adds coverage for cache-path resolution, including escaping/relative-path scenarios. |
| tests/Java.Interop.Tools.Maven-Tests/ArtifactTests.cs | Adds coverage for valid/invalid Maven coordinates and ctor behavior (incl. empty-version allowance). |
| src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs | Introduces GetArtifactFilePath() and routes caching through it with a cache-directory containment assertion. |
| src/Java.Interop.Tools.Maven/Models/Artifact.cs | Implements stricter coordinate/version validation and updates TryParse behavior/signature. |
Suppressed comments (2)
src/Java.Interop.Tools.Maven/Models/Artifact.cs:94
IsValidVersionalso permits "."/"..". Sinceversionis used as a directory name in the cache layout, a value of ".." will traverse upward when combined and normalized (even without any path separators present in the string). Consider rejecting these special segments to keep cache layout deterministic and avoid collisions.
static bool IsValidVersion (string value)
{
if (string.IsNullOrWhiteSpace (value))
return false;
foreach (var c in value) {
tests/Java.Interop.Tools.Maven-Tests/ArtifactTests.cs:111
- Similarly, the constructor-invalid coverage should include "."/".." for
groupId/artifactId/versionso the cache-path traversal edge case stays blocked for direct ctor calls too.
[TestCase ("com.example", "lib", "1.0/../")]
[TestCase ("com.example", "lib", "..\\1.0")]
[TestCase ("com/example", "lib", "1.0")]
public void Ctor_Invalid_Throws (string groupId, string artifactId, string version)
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+73
to
+77
| static bool IsValidCoordinate (string value) | ||
| { | ||
| if (string.IsNullOrWhiteSpace (value)) | ||
| return false; | ||
| foreach (var c in value) { |
Comment on lines
+52
to
+55
| [TestCase ("a/../b:c:1")] | ||
| [TestCase ("..\\a:b:1")] | ||
| [TestCase ("a:b:..\\1.0")] | ||
| public void TryParse_Invalid (string value) |
| } | ||
|
|
||
| public static bool TryParse (string value, [NotNullWhen (true)]out Artifact? artifact) | ||
| public static bool TryParse (string? value, [NotNullWhen (true)]out Artifact? artifact) |
Member
Author
|
Superseded by #1498, which contains the complete release-applicable hardening and approved-feed routing as one commit on the pinned release baseline. |
jonathanpeppers
added a commit
that referenced
this pull request
Aug 21, 2026
…1498) Supersedes #1495 with the complete release-applicable Maven backport as exactly one commit on top of the Java.Interop commit pinned by dotnet/android `release/10.0.1xx` (`33992194b9373e9322244612c94ed9941b9bc2fd`). Backports: - #1480: assert resolved Maven cache paths remain under `CacheDirectory` - #1479: validate Maven artifact coordinates - Route Maven integration-test resolvers through `dotnet-public-maven` - Remove java-source-utils' project-local `mavenCentral()` and use Android's shared approved repository settings when embedded, with an equivalent standalone Java.Interop fallback - Coupled Maven hardening tests The Kotlin Gradle fixture from Android main is intentionally excluded because it does not exist at this release pin. Validation: - Java.Interop.Tools.Maven tests: 106 passed, 0 failed - java-source-utils assembly: succeeded - java-source-utils standalone CI-mode settings evaluation: succeeded - Fresh-cache java-source-utils `compileJava`: succeeded entirely through `dotnet-public-maven` - java-source-utils legacy suite: 16 passed before two existing missing-resource NPEs (`JavaType.java`) unrelated to repository routing Feed prerequisite: the release JavaParser `3.18.0` dependency graph must be mirrored into `dotnet-public-maven` for anonymous CI resolution. The complete graph has been seeded and fresh-cache resolution is confirmed. Co-authored-by: Copilot App <[email protected]>
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Backports the complete Java.Interop.Tools.Maven hardening set from
mainonto the Java.Interop commit currently pinned by dotnet/android'srelease/10.0.1xxbranch (33992194b9373e9322244612c94ed9941b9bc2fd):fa9ccfb1b82ec28652fe8d2d244bc323597fdfb8/ [Java.Interop.Tools.Maven] Assert resolved cache paths stay under CacheDirectory #1480: assert resolved cache paths remain underCacheDirectory70493645c7d95648010a4cef948234a28744c03f/ [Java.Interop.Tools.Maven] Validate Artifact coordinates #1479: validate Maven artifact coordinatesArtifactTestsandCachedMavenRepositoryTestsThe two consecutive
mainchanges are combined into exactly one backport commit. No other Maven implementation, test, or documentation changes exist between the release baseline andmain.Validation:
dotnet test tests\Java.Interop.Tools.Maven-Tests\Java.Interop.Tools.Maven-Tests.csproj --configuration Release