From 686ad3612efeaafc922d432c6b63cc7999990be8 Mon Sep 17 00:00:00 2001 From: Jonathan Peppers Date: Mon, 22 Jun 2026 12:21:42 -0500 Subject: [PATCH] [Java.Interop.Tools.Maven] Assert resolved cache paths stay under CacheDirectory Adds a path-staying-under-CacheDirectory assertion to `CachedMavenRepository`. Exposes a new public API, `GetArtifactFilePath (Artifact, string)`, that returns the on-disk path where an artifact + filename would be cached and throws `InvalidOperationException` if the resolved path would not be under `CacheDirectory`. `TryGetFile`, `TryGetFilePath`, and `GetFilePathAsync` all route through this single method so there is exactly one place that knows the cache layout. The new public API lets callers (such as dotnet/android's `MavenExtensions.DownloadPayload`) stop reconstructing the cache path themselves with their own `Path.Combine` logic and get the assertion for free. Defense-in-depth, hardening. Not security enforcement. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Repositories/CachedMavenRepository.cs | 30 ++- .../CachedMavenRepositoryTests.cs | 185 ++++++++++++++++++ 2 files changed, 209 insertions(+), 6 deletions(-) create mode 100644 tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs diff --git a/src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs b/src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs index 65552af47..3d1f00bfb 100644 --- a/src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs +++ b/src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs @@ -1,3 +1,4 @@ +using System; using System.Diagnostics.CodeAnalysis; using System.IO; using System.Threading; @@ -39,8 +40,7 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr { path = null; - var directory = GetArtifactDirectory (artifact); - var file = Path.Combine (directory, filename); + var file = GetArtifactFilePath (artifact, filename); if (File.Exists (file)) { path = file; @@ -48,7 +48,7 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr } if (repository.TryGetFile (artifact, filename, out var repo_stream)) { - Directory.CreateDirectory (directory); + Directory.CreateDirectory (GetArtifactDirectory (artifact)); using (var sw = File.Create (file)) using (repo_stream) @@ -63,14 +63,13 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr public async Task GetFilePathAsync (Artifact artifact, string filename, CancellationToken cancellationToken) { - var directory = GetArtifactDirectory (artifact); - var file = Path.Combine (directory, filename); + var file = GetArtifactFilePath (artifact, filename); if (File.Exists (file)) return file; if (repository.TryGetFile (artifact, filename, out var repo_stream)) { - Directory.CreateDirectory (directory); + Directory.CreateDirectory (GetArtifactDirectory (artifact)); using (var sw = File.Create (file)) using (repo_stream) @@ -83,6 +82,25 @@ public bool TryGetFilePath (Artifact artifact, string filename, [NotNullWhen (tr return null; } + /// + /// Returns the on-disk path where the given + + /// would be cached under . Does not download or check for existence. + /// Throws if the resolved path would not be under + /// . + /// + public string GetArtifactFilePath (Artifact artifact, string filename) + { + var directory = GetArtifactDirectory (artifact); + var file = Path.Combine (directory, filename); + var full_file = Path.GetFullPath (file); + var full_cache = Path.GetFullPath (CacheDirectory); + if (!full_cache.EndsWith (Path.DirectorySeparatorChar.ToString ()) && !full_cache.EndsWith (Path.AltDirectorySeparatorChar.ToString ())) + full_cache += Path.DirectorySeparatorChar; + if (!full_file.StartsWith (full_cache, StringComparison.Ordinal)) + throw new InvalidOperationException ($"Resolved Maven cache path '{full_file}' escapes cache directory '{full_cache}'."); + return full_file; + } + string GetArtifactDirectory (Artifact artifact) { var version = artifact.Version; diff --git a/tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs b/tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs new file mode 100644 index 000000000..277466fad --- /dev/null +++ b/tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs @@ -0,0 +1,185 @@ +using System; +using System.Diagnostics.CodeAnalysis; +using System.IO; +using System.Threading; +using System.Threading.Tasks; +using Java.Interop.Tools.Maven.Models; +using Java.Interop.Tools.Maven.Repositories; + +namespace Java.Interop.Tools.Maven_Tests; + +public class CachedMavenRepositoryTests +{ + string cache_dir = ""; + + [SetUp] + public void SetUp () + { + cache_dir = Path.Combine (Path.GetTempPath (), "Java.Interop.Tools.Maven-Tests", Path.GetRandomFileName ()); + Directory.CreateDirectory (cache_dir); + } + + [TearDown] + public void TearDown () + { + if (Directory.Exists (cache_dir)) + Directory.Delete (cache_dir, recursive: true); + } + + [Test] + public void GetArtifactFilePath_HappyPath_ReturnsExpectedLayout () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var inner = new StubRepository ("central", artifact, "lib-1.0.0.jar", new byte [] { 1, 2, 3 }); + var cache = new CachedMavenRepository (cache_dir, inner); + + var expected = Path.GetFullPath (Path.Combine (cache_dir, "central", "com.example", "lib", "1.0.0", "lib-1.0.0.jar")); + var actual = cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar"); + + Assert.AreEqual (expected, actual); + Assert.IsFalse (File.Exists (actual), "GetArtifactFilePath must not create/download the file."); + } + + [Test] + public void TryGetFilePath_HappyPath_DownloadsAndReturnsExpectedPath () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var content = new byte [] { 1, 2, 3 }; + var inner = new StubRepository ("central", artifact, "lib-1.0.0.jar", content); + var cache = new CachedMavenRepository (cache_dir, inner); + + var expected = Path.GetFullPath (Path.Combine (cache_dir, "central", "com.example", "lib", "1.0.0", "lib-1.0.0.jar")); + + Assert.IsTrue (cache.TryGetFilePath (artifact, "lib-1.0.0.jar", out var path)); + Assert.AreEqual (expected, path); + Assert.IsTrue (File.Exists (path)); + CollectionAssert.AreEqual (content, File.ReadAllBytes (path!)); + } + + [Test] + public void GetArtifactFilePath_RelativeFilename_Throws () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var inner = new ThrowingRepository ("central"); + var cache = new CachedMavenRepository (cache_dir, inner); + + var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!; + var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar"); + var malicious = Path.GetRelativePath (artifact_dir, outside); + + Assert.Throws (() => cache.GetArtifactFilePath (artifact, malicious)); + } + + [Test] + public void TryGetFilePath_RelativeFilename_Throws () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var inner = new ThrowingRepository ("central"); + var cache = new CachedMavenRepository (cache_dir, inner); + + var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!; + var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar"); + var malicious = Path.GetRelativePath (artifact_dir, outside); + + Assert.Throws (() => cache.TryGetFilePath (artifact, malicious, out _)); + Assert.AreEqual (0, inner.CallCount, "Inner repository must not be consulted for an escaping path."); + } + + [Test] + public void TryGetFile_RelativeFilename_Throws () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var inner = new ThrowingRepository ("central"); + var cache = new CachedMavenRepository (cache_dir, inner); + + var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!; + var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar"); + var malicious = Path.GetRelativePath (artifact_dir, outside); + + Assert.Throws (() => cache.TryGetFile (artifact, malicious, out _)); + Assert.AreEqual (0, inner.CallCount, "Inner repository must not be consulted for an escaping path."); + } + + [Test] + public void GetFilePathAsync_RelativeFilename_Throws () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var inner = new ThrowingRepository ("central"); + var cache = new CachedMavenRepository (cache_dir, inner); + + var artifact_dir = Path.GetDirectoryName (cache.GetArtifactFilePath (artifact, "anchor.jar"))!; + var outside = Path.Combine (Path.GetDirectoryName (cache_dir)!, Path.GetFileName (cache_dir) + "-sibling", "relative.jar"); + var malicious = Path.GetRelativePath (artifact_dir, outside); + + Assert.ThrowsAsync (async () => + await cache.GetFilePathAsync (artifact, malicious, CancellationToken.None)); + Assert.AreEqual (0, inner.CallCount, "Inner repository must not be consulted for an escaping path."); + } + + [Test] + public void GetArtifactFilePath_RelativeRepositoryName_Throws () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var inner = new ThrowingRepository (Path.Combine ("..", Path.GetFileName (cache_dir) + "-sibling")); + var cache = new CachedMavenRepository (cache_dir, inner); + + Assert.Throws (() => cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar")); + } + + [Test] + public void GetArtifactFilePath_SiblingPrefixCacheDirectory_Throws () + { + var artifact = new Artifact ("com.example", "lib", "1.0.0"); + var sibling = cache_dir + "-sibling"; + var repo_name = Path.GetRelativePath (cache_dir, sibling); + var inner = new ThrowingRepository (repo_name); + var cache = new CachedMavenRepository (cache_dir, inner); + + Assert.Throws (() => cache.GetArtifactFilePath (artifact, "lib-1.0.0.jar")); + } + + sealed class StubRepository : IMavenRepository + { + readonly Artifact expected; + readonly string expected_filename; + readonly byte [] content; + + public StubRepository (string name, Artifact expected, string filename, byte [] content) + { + Name = name; + this.expected = expected; + this.expected_filename = filename; + this.content = content; + } + + public string Name { get; } + + public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream) + { + if (artifact.GroupId == expected.GroupId && artifact.Id == expected.Id && artifact.Version == expected.Version && filename == expected_filename) { + stream = new MemoryStream (content); + return true; + } + stream = null; + return false; + } + } + + sealed class ThrowingRepository : IMavenRepository + { + public ThrowingRepository (string name) + { + Name = name; + } + + public string Name { get; } + + public int CallCount { get; private set; } + + public bool TryGetFile (Artifact artifact, string filename, [NotNullWhen (true)] out Stream? stream) + { + CallCount++; + throw new InvalidOperationException ("Inner repository should not be consulted when the resolved path escapes the cache directory."); + } + } +}