Skip to content

[Java.Interop.Tools.Maven] Assert resolved cache paths stay under CacheDirectory#1480

Open
jonathanpeppers wants to merge 1 commit into
mainfrom
jonathanpeppers-harden-cache-path
Open

[Java.Interop.Tools.Maven] Assert resolved cache paths stay under CacheDirectory#1480
jonathanpeppers wants to merge 1 commit into
mainfrom
jonathanpeppers-harden-cache-path

Conversation

@jonathanpeppers

Copy link
Copy Markdown
Member

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.

…heDirectory

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>
Copilot AI review requested due to automatic review settings June 22, 2026 17:50

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens Java.Interop.Tools.Maven’s caching layer by centralizing cache path resolution in CachedMavenRepository.GetArtifactFilePath (Artifact, string) and asserting that resolved artifact paths remain under CacheDirectory (throwing InvalidOperationException on escape attempts). This supports defense-in-depth and helps external callers avoid re-implementing cache layout logic.

Changes:

  • Added CachedMavenRepository.GetArtifactFilePath (Artifact, string) and routed existing file/path retrieval APIs through it.
  • Added guard logic to detect resolved cache paths escaping CacheDirectory.
  • Added a new NUnit test suite validating expected cache layout and escape-path failure behavior across multiple entry points.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
tests/Java.Interop.Tools.Maven-Tests/CachedMavenRepositoryTests.cs New tests covering cache layout and escape-path exceptions (including ensuring inner repo isn’t consulted on invalid paths).
src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs Introduces centralized cache path resolver with “must stay under CacheDirectory” assertion; updates existing APIs to use it.

Comment thread src/Java.Interop.Tools.Maven/Repositories/CachedMavenRepository.cs
@jonathanpeppers jonathanpeppers added the ready-to-review This PR is ready to review/merge, thanks! label Jun 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-to-review This PR is ready to review/merge, thanks!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants