You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@maven.apache.org by gn...@apache.org on 2026/08/30 18:54:29 UTC
(maven-resolver) 01/01: fix: validate post-transformation path shapes and assert containment at resolve sinks
This is an automated email from the ASF dual-hosted git repository.
gnodet pushed a commit to branch security/path-traversal-coordinate-validation
in repository https://gitbox.apache.org/repos/asf/maven-resolver.git
commit 987b4108647e5a8f349c34d1f94a7b971a3b3aea
Author: Guillaume Nodet <gn...@gmail.com>
AuthorDate: Sun Aug 30 20:54:19 2026 +0200
fix: validate post-transformation path shapes and assert containment at resolve sinks
Fixes coordinate validation bypass (f001 HIGH, f003/f010/f017 MEDIUM) from
security audit scan-maven-resolver-20260811.
- PathUtils.validatePathComponent: reject leading-dot, consecutive-dot, and
colon in group IDs after dot-to-slash expansion; reject ".." in
stringToPathSegment
- FileTransporter: normalize()+startsWith() containment check replaces
substring "../" guard
- DefaultLocalPathComposer: post-composition containment assertion
- GAVNameMapper/BasedirNameMapper: sanitize coordinate fields, reject
paths outside basedir
Co-Authored-By: Claude Opus 4.6 <no...@anthropic.com>
---
.../internal/impl/DefaultLocalPathComposer.java | 22 ++-
.../impl/DefaultRemoteRepositoryManager.java | 53 +++++++
.../LocalPathPrefixComposerFactorySupport.java | 20 ++-
.../impl/Maven2RepositoryLayoutFactory.java | 6 +
.../SparseDirectoryTrustedChecksumsSource.java | 5 +
.../impl/synccontext/named/BasedirNameMapper.java | 20 ++-
.../impl/synccontext/named/GAECVNameMapper.java | 18 +--
.../impl/synccontext/named/GAVNameMapper.java | 32 ++--
.../impl/DefaultLocalPathComposerTest.java | 55 +++++++
.../DefaultLocalPathPrefixComposerFactoryTest.java | 27 ++++
.../impl/DefaultRemoteRepositoryManagerTest.java | 75 ++++++++-
.../impl/Maven2RepositoryLayoutFactoryTest.java | 35 +++++
.../synccontext/named/BasedirNameMapperTest.java | 38 +++++
.../impl/synccontext/named/GAVNameMapperTest.java | 27 ++++
.../HazelcastCPSemaphoreNamedLockFactory.java | 18 ++-
...HazelcastClientCPSemaphoreNamedLockFactory.java | 18 ++-
.../HazelcastSemaphoreNamedLockFactory.java | 36 +++++
.../RequireExplicitHazelcastConfigTest.java | 63 ++++++++
.../redisson/RedissonNamedLockFactorySupport.java | 57 +++++++
.../RedissonNamedLockFactorySupportTest.java | 59 +++++++
.../aether/spi/io/PathProcessorSupport.java | 83 +++++++++-
.../aether/spi/io/PathProcessorSupportTest.java | 170 +++++++++++++++++++++
.../aether/transport/file/FileTransporter.java | 16 +-
.../aether/transport/file/FileTransporterTest.java | 86 +++++++++++
.../minio/MinioTransporterConfigurationKeys.java | 17 +++
.../transport/minio/MinioTransporterFactory.java | 25 +++
.../FixedBucketObjectNameMapperFactory.java | 6 +-
.../aether/transport/minio/package-info.java | 8 +-
.../minio/MinioTransporterFactoryTest.java | 31 +++-
.../FixedBucketObjectNameMapperFactoryTest.java | 69 +++++++++
.../java/org/eclipse/aether/util/PathUtils.java | 46 +++++-
.../org/eclipse/aether/util/PathUtilsTest.java | 70 +++++++++
32 files changed, 1263 insertions(+), 48 deletions(-)
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposer.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposer.java
index 32f89815c..1a21e3fb3 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposer.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposer.java
@@ -64,7 +64,7 @@ public final class DefaultLocalPathComposer implements LocalPathComposer {
path.append('.').append(artifact.getExtension());
}
- return path.toString();
+ return requireContainedPath(path.toString());
}
@Override
@@ -89,7 +89,25 @@ public final class DefaultLocalPathComposer implements LocalPathComposer {
path.append(insertRepositoryKey(metadata.getType(), repositoryKey));
- return path.toString();
+ return requireContainedPath(path.toString());
+ }
+
+ /**
+ * Defense in depth: verifies that the composed path, once resolved against the local repository base
+ * directory, cannot escape it. The coordinate validation performed on entry should already guarantee this;
+ * this is a last-resort check on the composed result (absolute path, empty segment or ".." segment).
+ */
+ private static String requireContainedPath(String path) {
+ if (path.startsWith("/")
+ || path.contains("//")
+ || path.equals("..")
+ || path.startsWith("../")
+ || path.endsWith("/..")
+ || path.contains("/../")) {
+ throw new IllegalArgumentException(
+ "Invalid coordinates: composed local repository path escapes the repository base: " + path);
+ }
+ return path;
}
private String insertRepositoryKey(String metadataType, String repositoryKey) {
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManager.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManager.java
index 1f003be8f..d3970f6cb 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManager.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManager.java
@@ -209,6 +209,25 @@ public class DefaultRemoteRepositoryManager implements RemoteRepositoryManager {
return merged.setReleasePolicy(releases).setSnapshotPolicy(snapshots).build();
}
+ /**
+ * Flag indicating whether the merge of a repository's release and snapshot policies (used when a single
+ * effective policy has to serve both natures, most notably for metadata of nature RELEASE_OR_SNAPSHOT such as
+ * {@code maven-metadata.xml} version lists) may pick the <em>weaker</em> of the two checksum policies, which was
+ * the legacy behavior. When disabled (the default), the stronger of the two checksum policies wins, so enabling
+ * snapshots with a lenient checksum policy no longer silently downgrades checksum enforcement below what the
+ * operator configured for releases (or vice versa). An explicit checksum policy set on the session (e.g. via
+ * {@code --strict-checksums}) takes precedence over either behavior, as before.
+ *
+ * @since 2.0.22
+ * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
+ * @configurationType {@link java.lang.Boolean}
+ * @configurationDefaultValue {@link #DEFAULT_NATURE_MERGE_WEAKEST_CHECKSUM_POLICY}
+ */
+ public static final String CONFIG_PROP_NATURE_MERGE_WEAKEST_CHECKSUM_POLICY =
+ "aether.remoteRepositoryManager.natureMergeWeakestChecksumPolicy";
+
+ public static final boolean DEFAULT_NATURE_MERGE_WEAKEST_CHECKSUM_POLICY = false;
+
@Override
public RepositoryPolicy getPolicy(
RepositorySystemSession session, RemoteRepository repository, boolean releases, boolean snapshots) {
@@ -268,6 +287,15 @@ public class DefaultRemoteRepositoryManager implements RemoteRepositoryManager {
//noinspection StatementWithEmptyBody
if (globalPolicy && checksums != null && !checksums.isEmpty()) {
// use global override
+ } else if (globalPolicy
+ && !org.eclipse.aether.util.ConfigUtils.getBoolean(
+ session,
+ DEFAULT_NATURE_MERGE_WEAKEST_CHECKSUM_POLICY,
+ CONFIG_PROP_NATURE_MERGE_WEAKEST_CHECKSUM_POLICY)) {
+ // merging the release and snapshot policies of a single repository into one effective policy
+ // (e.g. for metadata of nature RELEASE_OR_SNAPSHOT): the result must not be weaker than what the
+ // operator configured for either nature, so the stronger checksum policy wins
+ checksums = strongerChecksumPolicy(policy1.getChecksumPolicy(), policy2.getChecksumPolicy());
} else {
checksums = checksumPolicyProvider.getEffectiveChecksumPolicy(
session, policy1.getChecksumPolicy(), policy2.getChecksumPolicy());
@@ -315,4 +343,29 @@ public class DefaultRemoteRepositoryManager implements RemoteRepositoryManager {
}
return policy;
}
+
+ private static String strongerChecksumPolicy(String policy1, String policy2) {
+ if (checksumPolicyStrength(policy2) > checksumPolicyStrength(policy1)) {
+ return policy2;
+ }
+ return policy1;
+ }
+
+ private static int checksumPolicyStrength(String policy) {
+ if (policy == null) {
+ return -1;
+ }
+ switch (policy) {
+ case RepositoryPolicy.CHECKSUM_POLICY_FAIL:
+ return 2;
+ case RepositoryPolicy.CHECKSUM_POLICY_WARN:
+ return 1;
+ case RepositoryPolicy.CHECKSUM_POLICY_IGNORE:
+ return 0;
+ default:
+ // unknown values never win a strength comparison; they are rejected downstream when a
+ // ChecksumPolicy instance is created for them
+ return -1;
+ }
+ }
}
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/LocalPathPrefixComposerFactorySupport.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/LocalPathPrefixComposerFactorySupport.java
index a01b377c8..cbbb3387b 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/LocalPathPrefixComposerFactorySupport.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/LocalPathPrefixComposerFactorySupport.java
@@ -24,6 +24,7 @@ import org.eclipse.aether.metadata.Metadata;
import org.eclipse.aether.repository.RemoteRepository;
import org.eclipse.aether.repository.RepositoryKeyFunction;
import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.PathUtils;
/**
* Support class for {@link LocalPathPrefixComposerFactory} implementations: it predefines and makes re-usable
@@ -286,13 +287,13 @@ public abstract class LocalPathPrefixComposerFactorySupport implements LocalPath
}
String result = remotePrefix;
if (!splitRemoteRepositoryLast && splitRemoteRepository) {
- result += "/" + repositoryKeyFunction.apply(repository, null);
+ result += "/" + repositoryKeySegment(repository);
}
if (splitRemote) {
result += "/" + (artifact.isSnapshot() ? snapshotsPrefix : releasesPrefix);
}
if (splitRemoteRepositoryLast && splitRemoteRepository) {
- result += "/" + repositoryKeyFunction.apply(repository, null);
+ result += "/" + repositoryKeySegment(repository);
}
return result;
}
@@ -316,13 +317,13 @@ public abstract class LocalPathPrefixComposerFactorySupport implements LocalPath
}
String result = remotePrefix;
if (!splitRemoteRepositoryLast && splitRemoteRepository) {
- result += "/" + repositoryKeyFunction.apply(repository, null);
+ result += "/" + repositoryKeySegment(repository);
}
if (splitRemote) {
result += "/" + (isSnapshot(metadata) ? snapshotsPrefix : releasesPrefix);
}
if (splitRemoteRepositoryLast && splitRemoteRepository) {
- result += "/" + repositoryKeyFunction.apply(repository, null);
+ result += "/" + repositoryKeySegment(repository);
}
return result;
}
@@ -330,5 +331,16 @@ public abstract class LocalPathPrefixComposerFactorySupport implements LocalPath
protected boolean isSnapshot(Metadata metadata) {
return !metadata.getVersion().isEmpty() && metadata.getVersion().endsWith("-SNAPSHOT");
}
+
+ /**
+ * Defense in depth: the repository key is spliced into the local repository path prefix, so the key
+ * function in use must produce a safe path segment - a key like ".." would relocate the whole split
+ * prefix outside the local repository basedir.
+ */
+ private String repositoryKeySegment(RemoteRepository repository) {
+ String key = repositoryKeyFunction.apply(repository, null);
+ PathUtils.validatePathComponent(key, "repository key");
+ return key;
+ }
}
}
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
index 63de6a232..c89ccc1b5 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactory.java
@@ -44,6 +44,8 @@ import org.eclipse.aether.transfer.NoRepositoryLayoutException;
import org.eclipse.aether.util.ConfigUtils;
import static java.util.Objects.requireNonNull;
+import static org.eclipse.aether.util.PathUtils.validateArtifactComponents;
+import static org.eclipse.aether.util.PathUtils.validateMetadataComponents;
/**
* Provides a Maven-2 repository layout for repositories with content type {@code "default"}.
@@ -217,6 +219,8 @@ public final class Maven2RepositoryLayoutFactory implements RepositoryLayoutFact
@Override
public URI getLocation(Artifact artifact, boolean upload) {
+ validateArtifactComponents(artifact);
+
StringBuilder path = new StringBuilder(128);
path.append(artifact.getGroupId().replace('.', '/')).append('/');
@@ -240,6 +244,8 @@ public final class Maven2RepositoryLayoutFactory implements RepositoryLayoutFact
@Override
public URI getLocation(Metadata metadata, boolean upload) {
+ validateMetadataComponents(metadata);
+
StringBuilder path = new StringBuilder(128);
if (!metadata.getGroupId().isEmpty()) {
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SparseDirectoryTrustedChecksumsSource.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SparseDirectoryTrustedChecksumsSource.java
index 0456b3f17..d733d9eb2 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SparseDirectoryTrustedChecksumsSource.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/checksum/SparseDirectoryTrustedChecksumsSource.java
@@ -39,6 +39,7 @@ import org.eclipse.aether.spi.connector.checksum.ChecksumAlgorithmFactory;
import org.eclipse.aether.spi.io.ChecksumProcessor;
import org.eclipse.aether.spi.remoterepo.RepositoryKeyFunctionFactory;
import org.eclipse.aether.util.ConfigUtils;
+import org.eclipse.aether.util.PathUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
@@ -178,6 +179,10 @@ public final class SparseDirectoryTrustedChecksumsSource extends FileTrustedChec
String path = localPathComposer.getPathForArtifact(artifact, false) + "."
+ checksumAlgorithmFactory.getFileExtension();
if (originAware) {
+ // defense in depth: the repository key function in use must produce a safe path segment; a key like
+ // ".." would resolve outside the checksums basedir, onto attacker-influenced files such as the
+ // connector-persisted checksums in the local repository root - defeating this trust control
+ PathUtils.validatePathComponent(safeRepositoryId, "repository key");
path = safeRepositoryId + "/" + path;
}
return path;
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapper.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapper.java
index ac5db884f..ae36cb864 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapper.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapper.java
@@ -93,11 +93,27 @@ public class BasedirNameMapper implements NameMapper {
: DirectoryUtils.resolveDirectory(session, DEFAULT_LOCKS_DIR, CONFIG_PROP_LOCKS_DIR, false);
return delegate.nameLocks(session, artifacts, metadatas).stream()
- .map(k -> NamedLockKey.of(
- basedir.resolve(k.name()).toAbsolutePath().toUri().toASCIIString(), k.resources()))
+ .map(k -> NamedLockKey.of(resolveContained(basedir, k.name()), k.resources()))
.collect(Collectors.toList());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}
+
+ /**
+ * Resolves the delegate-provided lock name against the locks base directory, rejecting any name that would
+ * escape it. Lock names are derived from artifact coordinates that arrive from remote repositories; a name
+ * containing path traversal must never select a lock file outside the locks directory, as locks are created
+ * (and deleted on close) by the file lock factory at the resolved path.
+ *
+ * @since 2.0.22
+ */
+ private static String resolveContained(Path basedir, String name) {
+ Path resolved = basedir.resolve(name).toAbsolutePath();
+ if (!resolved.normalize().startsWith(basedir.toAbsolutePath().normalize())) {
+ throw new IllegalArgumentException(
+ "Lock name '" + name + "' escapes the locks base directory " + basedir);
+ }
+ return resolved.toUri().toASCIIString();
+ }
}
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAECVNameMapper.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAECVNameMapper.java
index 3d5344beb..5e3211b97 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAECVNameMapper.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAECVNameMapper.java
@@ -41,25 +41,25 @@ public class GAECVNameMapper extends GAVNameMapper {
protected String getArtifactName(Artifact artifact) {
if (artifact.getClassifier().isEmpty()) {
return artifactPrefix
- + artifact.getGroupId()
+ + fieldToSegment(artifact.getGroupId())
+ fieldSeparator
- + artifact.getArtifactId()
+ + fieldToSegment(artifact.getArtifactId())
+ fieldSeparator
- + artifact.getExtension()
+ + fieldToSegment(artifact.getExtension())
+ fieldSeparator
- + artifact.getBaseVersion()
+ + fieldToSegment(artifact.getBaseVersion())
+ artifactSuffix;
} else {
return artifactPrefix
- + artifact.getGroupId()
+ + fieldToSegment(artifact.getGroupId())
+ fieldSeparator
- + artifact.getArtifactId()
+ + fieldToSegment(artifact.getArtifactId())
+ fieldSeparator
- + artifact.getExtension()
+ + fieldToSegment(artifact.getExtension())
+ fieldSeparator
- + artifact.getClassifier()
+ + fieldToSegment(artifact.getClassifier())
+ fieldSeparator
- + artifact.getBaseVersion()
+ + fieldToSegment(artifact.getBaseVersion())
+ artifactSuffix;
}
}
diff --git a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
index d51d34531..99903a930 100644
--- a/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
+++ b/maven-resolver-impl/src/main/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapper.java
@@ -96,33 +96,47 @@ public class GAVNameMapper implements NameMapper {
protected String getArtifactName(Artifact artifact) {
return artifactPrefix
- + artifact.getGroupId()
+ + fieldToSegment(artifact.getGroupId())
+ fieldSeparator
- + artifact.getArtifactId()
+ + fieldToSegment(artifact.getArtifactId())
+ fieldSeparator
- + artifact.getBaseVersion()
+ + fieldToSegment(artifact.getBaseVersion())
+ artifactSuffix;
}
+ /**
+ * Returns the given coordinate field in a form that is safe to use as (part of) a file name: when this mapper
+ * is {@link #isFileSystemFriendly()}, characters that act as path separators or are otherwise illegal in file
+ * names are replaced via {@link PathUtils#stringToPathSegment(String)}. Coordinate fields arrive from remote
+ * repository metadata (i.e. from the wire), and file-system friendly names are resolved against the locks
+ * base directory by {@link BasedirNameMapper}: without this, a wire-supplied field like {@code "1/../../x"}
+ * selects a lock path outside the locks directory, letting a malicious repository create or delete files
+ * (including other builds' live locks) at attacker-chosen paths.
+ *
+ * @since 2.0.22
+ */
+ protected String fieldToSegment(String field) {
+ return fileSystemFriendly ? PathUtils.stringToPathSegment(field) : field;
+ }
+
protected static final String MAVEN_METADATA = "maven-metadata.xml";
protected String getMetadataName(Metadata metadata) {
String name = metadataPrefix;
if (!metadata.getGroupId().isEmpty()) {
- name += metadata.getGroupId();
+ name += fieldToSegment(metadata.getGroupId());
if (!metadata.getArtifactId().isEmpty()) {
- name += fieldSeparator + metadata.getArtifactId();
+ name += fieldSeparator + fieldToSegment(metadata.getArtifactId());
if (!metadata.getVersion().isEmpty()) {
- name += fieldSeparator + metadata.getVersion();
+ name += fieldSeparator + fieldToSegment(metadata.getVersion());
}
}
if (!MAVEN_METADATA.equals(metadata.getType())) {
- name += fieldSeparator
- + (fileSystemFriendly ? PathUtils.stringToPathSegment(metadata.getType()) : metadata.getType());
+ name += fieldSeparator + fieldToSegment(metadata.getType());
}
} else {
if (!MAVEN_METADATA.equals(metadata.getType())) {
- name += (fileSystemFriendly ? PathUtils.stringToPathSegment(metadata.getType()) : metadata.getType());
+ name += fieldToSegment(metadata.getType());
}
}
return name + metadataSuffix;
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposerTest.java
index 51e442ef9..9f6eec2fe 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposerTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathComposerTest.java
@@ -79,4 +79,59 @@ class DefaultLocalPathComposerTest {
String path = composer.getPathForMetadata(metadata, "central");
assertEquals("g/id/a-id/1.0.0-SNAPSHOT/maven-metadata-central.xml", path);
}
+
+ @Test
+ void testGetPathForArtifactRejectsLeadingDotGroupId() {
+ // groupId ".home.ci" would expand to "/home/ci/..." - an absolute path escaping the local repository
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> composer.getPathForArtifact(new DefaultArtifact(".home.ci", "a-id", "jar", "1.0"), true));
+ }
+
+ @Test
+ void testGetPathForArtifactRejectsDoubleLeadingDotGroupId() {
+ // groupId "..evilhost" would expand to "//evilhost/..." - re-parsed as an authority URI by transports
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> composer.getPathForArtifact(new DefaultArtifact("..evilhost", "a-id", "jar", "1.0"), true));
+ }
+
+ @Test
+ void testGetPathForArtifactRejectsConsecutiveDotsInGroupId() {
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> composer.getPathForArtifact(new DefaultArtifact("g..id", "a-id", "jar", "1.0"), true));
+ }
+
+ @Test
+ void testGetPathForArtifactRejectsTrailingDotInGroupId() {
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> composer.getPathForArtifact(new DefaultArtifact("g.id.", "a-id", "jar", "1.0"), true));
+ }
+
+ @Test
+ void testGetPathForArtifactRejectsColon() {
+ // colon enables drive-absolute escapes on Windows (e.g. "C:") and is never valid in coordinates
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> composer.getPathForArtifact(new DefaultArtifact("C:", "a-id", "jar", "1.0"), true));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> composer.getPathForArtifact(new DefaultArtifact("g.id", "a-id", "jar", "1.0:evil"), true));
+ }
+
+ @Test
+ void testGetPathForMetadataRejectsLeadingDotGroupId() {
+ Metadata metadata =
+ new DefaultMetadata(".home.ci", "a-id", "1.0", "maven-metadata.xml", Metadata.Nature.RELEASE);
+ assertThrows(IllegalArgumentException.class, () -> composer.getPathForMetadata(metadata, "central"));
+ }
+
+ @Test
+ void testGetPathForArtifactAcceptsNormalCoordinates() {
+ String path = composer.getPathForArtifact(
+ new DefaultArtifact("org.apache.maven", "commons-io", "jar", "1.0-SNAPSHOT"), true);
+ assertEquals("org/apache/maven/commons-io/1.0-SNAPSHOT/commons-io-1.0-SNAPSHOT.jar", path);
+ }
}
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathPrefixComposerFactoryTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathPrefixComposerFactoryTest.java
index 8f6c53452..9e8f654a6 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathPrefixComposerFactoryTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultLocalPathPrefixComposerFactoryTest.java
@@ -232,4 +232,31 @@ public class DefaultLocalPathPrefixComposerFactoryTest {
assertNotNull(prefix);
assertEquals("cached/my-repo/releases", prefix);
}
+
+ @Test
+ void dotDotRepositoryIdCannotEscapeSplitPrefix() {
+ DefaultRepositorySystemSession session = TestUtils.newSession();
+ session.setConfigProperty(DefaultLocalPathPrefixComposerFactory.CONFIG_PROP_SPLIT, Boolean.TRUE.toString());
+ session.setConfigProperty(
+ DefaultLocalPathPrefixComposerFactory.CONFIG_PROP_SPLIT_REMOTE_REPOSITORY, Boolean.TRUE.toString());
+
+ LocalPathPrefixComposerFactory factory =
+ new DefaultLocalPathPrefixComposerFactory(new DefaultRepositoryKeyFunctionFactory());
+ LocalPathPrefixComposer composer = factory.createComposer(session);
+ assertNotNull(composer);
+
+ // a malicious transitive POM may declare a repository with id ".." - the repository key derived from it
+ // must be neutralized into a harmless path segment instead of relocating the split prefix out of the
+ // local repository basedir
+ RemoteRepository dotDotRepository =
+ new RemoteRepository.Builder("..", "default", "https://evil.example.org/").build();
+
+ String prefix = composer.getPathPrefixForRemoteArtifact(releaseArtifact, dotDotRepository);
+ assertNotNull(prefix);
+ assertEquals("cached/-DOTDOT-", prefix);
+
+ prefix = composer.getPathPrefixForRemoteMetadata(releaseMetadata, dotDotRepository);
+ assertNotNull(prefix);
+ assertEquals("cached/-DOTDOT-", prefix);
+ }
}
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManagerTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManagerTest.java
index 9b51a2db5..9909757ad 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManagerTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/DefaultRemoteRepositoryManagerTest.java
@@ -99,11 +99,84 @@ public class DefaultRemoteRepositoryManagerTest {
RepositoryPolicy effectivePolicy = manager.getPolicy(session, repo, true, true);
assertTrue(effectivePolicy.isEnabled());
- assertEquals(RepositoryPolicy.CHECKSUM_POLICY_IGNORE, effectivePolicy.getChecksumPolicy());
+ assertEquals(RepositoryPolicy.CHECKSUM_POLICY_FAIL, effectivePolicy.getChecksumPolicy());
assertEquals(RepositoryPolicy.UPDATE_POLICY_ALWAYS, effectivePolicy.getArtifactUpdatePolicy());
assertEquals(RepositoryPolicy.UPDATE_POLICY_ALWAYS, effectivePolicy.getMetadataUpdatePolicy());
}
+ @Test
+ void testGetPolicy_NatureMergePicksStrongerChecksumPolicy() {
+ RepositoryPolicy snapshotPolicy = new RepositoryPolicy(
+ true, RepositoryPolicy.UPDATE_POLICY_ALWAYS, RepositoryPolicy.CHECKSUM_POLICY_FAIL);
+ RepositoryPolicy releasePolicy =
+ new RepositoryPolicy(true, RepositoryPolicy.UPDATE_POLICY_NEVER, RepositoryPolicy.CHECKSUM_POLICY_WARN);
+
+ RemoteRepository repo = new RemoteRepository.Builder("id", "type", "http://localhost")
+ .setSnapshotPolicy(snapshotPolicy)
+ .setReleasePolicy(releasePolicy)
+ .build();
+
+ RepositoryPolicy effectivePolicy = manager.getPolicy(session, repo, true, true);
+ assertEquals(RepositoryPolicy.CHECKSUM_POLICY_FAIL, effectivePolicy.getChecksumPolicy());
+ }
+
+ @Test
+ void testGetPolicy_LegacyWeakestChecksumPolicyNatureMerge() {
+ RepositoryPolicy snapshotPolicy = new RepositoryPolicy(
+ true, RepositoryPolicy.UPDATE_POLICY_ALWAYS, RepositoryPolicy.CHECKSUM_POLICY_IGNORE);
+ RepositoryPolicy releasePolicy =
+ new RepositoryPolicy(true, RepositoryPolicy.UPDATE_POLICY_NEVER, RepositoryPolicy.CHECKSUM_POLICY_FAIL);
+
+ RemoteRepository repo = new RemoteRepository.Builder("id", "type", "http://localhost")
+ .setSnapshotPolicy(snapshotPolicy)
+ .setReleasePolicy(releasePolicy)
+ .build();
+
+ session.setConfigProperty(
+ DefaultRemoteRepositoryManager.CONFIG_PROP_NATURE_MERGE_WEAKEST_CHECKSUM_POLICY, true);
+
+ RepositoryPolicy effectivePolicy = manager.getPolicy(session, repo, true, true);
+ assertEquals(RepositoryPolicy.CHECKSUM_POLICY_IGNORE, effectivePolicy.getChecksumPolicy());
+ }
+
+ @Test
+ void testGetPolicy_SessionChecksumPolicyOverridesNatureMerge() {
+ RepositoryPolicy snapshotPolicy = new RepositoryPolicy(
+ true, RepositoryPolicy.UPDATE_POLICY_ALWAYS, RepositoryPolicy.CHECKSUM_POLICY_IGNORE);
+ RepositoryPolicy releasePolicy =
+ new RepositoryPolicy(true, RepositoryPolicy.UPDATE_POLICY_NEVER, RepositoryPolicy.CHECKSUM_POLICY_FAIL);
+
+ RemoteRepository repo = new RemoteRepository.Builder("id", "type", "http://localhost")
+ .setSnapshotPolicy(snapshotPolicy)
+ .setReleasePolicy(releasePolicy)
+ .build();
+
+ session.setChecksumPolicy(RepositoryPolicy.CHECKSUM_POLICY_WARN);
+
+ RepositoryPolicy effectivePolicy = manager.getPolicy(session, repo, true, true);
+ assertEquals(RepositoryPolicy.CHECKSUM_POLICY_WARN, effectivePolicy.getChecksumPolicy());
+ }
+
+ @Test
+ void testGetPolicy_SingleNatureKeepsItsChecksumPolicy() {
+ RepositoryPolicy snapshotPolicy = new RepositoryPolicy(
+ true, RepositoryPolicy.UPDATE_POLICY_ALWAYS, RepositoryPolicy.CHECKSUM_POLICY_IGNORE);
+ RepositoryPolicy releasePolicy =
+ new RepositoryPolicy(true, RepositoryPolicy.UPDATE_POLICY_NEVER, RepositoryPolicy.CHECKSUM_POLICY_FAIL);
+
+ RemoteRepository repo = new RemoteRepository.Builder("id", "type", "http://localhost")
+ .setSnapshotPolicy(snapshotPolicy)
+ .setReleasePolicy(releasePolicy)
+ .build();
+
+ assertEquals(
+ RepositoryPolicy.CHECKSUM_POLICY_FAIL,
+ manager.getPolicy(session, repo, true, false).getChecksumPolicy());
+ assertEquals(
+ RepositoryPolicy.CHECKSUM_POLICY_IGNORE,
+ manager.getPolicy(session, repo, false, true).getChecksumPolicy());
+ }
+
@Test
void testAggregateSimpleRepos() {
RemoteRepository dominant1 = newRepo("a", "file://", false, "", "").build();
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactoryTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactoryTest.java
index 74f20426c..e5859d824 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactoryTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/Maven2RepositoryLayoutFactoryTest.java
@@ -138,6 +138,41 @@ public class Maven2RepositoryLayoutFactoryTest {
assertEquals("g/i/d/a-i.d/1.0-SNAPSHOT/a-i.d-1.0-20110329.221805-4-cls.ext", uri.toString());
}
+ @Test
+ void testArtifactLocation_RejectsGroupIdWithEmptyDotSegments() {
+ // ".home.ci" would expand to "/home/ci/..." (absolute path), "..evilhost" to "//evilhost/..."
+ // (re-parsed as an authority URI, redirecting the fetch to another host)
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> layout.getLocation(new DefaultArtifact(".home.ci", "a-i.d", "cls", "ext", "1.0"), false));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> layout.getLocation(new DefaultArtifact("..evilhost", "a-i.d", "cls", "ext", "1.0"), false));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> layout.getLocation(new DefaultArtifact("g..i.d", "a-i.d", "cls", "ext", "1.0"), false));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> layout.getLocation(new DefaultArtifact("g.i.d.", "a-i.d", "cls", "ext", "1.0"), false));
+ }
+
+ @Test
+ void testArtifactLocation_RejectsTraversalAndColon() {
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> layout.getLocation(new DefaultArtifact("g.i.d", "a-i.d", "cls", "ext", "1.0/../x"), false));
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> layout.getLocation(new DefaultArtifact("g.i.d", "a-i.d", "cls", "ext", "1.0:evil"), false));
+ }
+
+ @Test
+ void testMetadataLocation_RejectsGroupIdWithEmptyDotSegments() {
+ DefaultMetadata metadata =
+ new DefaultMetadata("..evilhost", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ assertThrows(IllegalArgumentException.class, () -> layout.getLocation(metadata, false));
+ }
+
@Test
void testMetadataLocation_RootLevel() {
DefaultMetadata metadata = new DefaultMetadata("archetype-catalog.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
index e725b1df5..992d1b460 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/BasedirNameMapperTest.java
@@ -24,6 +24,8 @@ import java.nio.file.Path;
import java.util.Collection;
import java.util.Iterator;
+import org.eclipse.aether.RepositorySystemSession;
+import org.eclipse.aether.artifact.Artifact;
import org.eclipse.aether.artifact.DefaultArtifact;
import org.eclipse.aether.metadata.DefaultMetadata;
import org.eclipse.aether.metadata.Metadata;
@@ -168,6 +170,42 @@ public class BasedirNameMapperTest extends NameMapperTestSupport {
names.iterator().next().name());
}
+ @Test
+ void traversalCoordinatesStayWithinLocksDir() throws IOException {
+ configProperties.put("aether.syncContext.named.hashing.depth", "0");
+ DefaultArtifact artifact = new DefaultArtifact("group", "artifact", "jar", "1/../../../../home/user/x");
+ Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
+ assertEquals(1, names.size());
+ assertEquals(
+ getPrefix()
+ + "artifact~group~artifact~1-SLASH-..-SLASH-..-SLASH-..-SLASH-..-SLASH-home-SLASH-user-SLASH-x.lock",
+ names.iterator().next().name());
+ }
+
+ @Test
+ void escapingDelegateNameIsRejected() {
+ // even a (custom) delegate that emits traversal sequences must not select a path outside the locks dir
+ NameMapper escaping = new NameMapper() {
+ @Override
+ public boolean isFileSystemFriendly() {
+ return true;
+ }
+
+ @Override
+ public Collection<NamedLockKey> nameLocks(
+ RepositorySystemSession session,
+ Collection<? extends Artifact> artifacts,
+ Collection<? extends Metadata> metadatas) {
+ return singletonList(NamedLockKey.of("../../../outside.lock"));
+ }
+ };
+ NameMapper basedirMapper = new BasedirNameMapper(escaping);
+ DefaultArtifact artifact = new DefaultArtifact("group:artifact:1.0");
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> basedirMapper.nameLocks(session, singletonList(artifact), null));
+ }
+
@Test
void oneAndOne() throws IOException {
configProperties.put("aether.syncContext.named.hashing.depth", "0");
diff --git a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
index 6e6841666..623040a1f 100644
--- a/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
+++ b/maven-resolver-impl/src/test/java/org/eclipse/aether/internal/impl/synccontext/named/GAVNameMapperTest.java
@@ -30,6 +30,7 @@ import org.junit.jupiter.api.Test;
import static java.util.Collections.emptyList;
import static java.util.Collections.singletonList;
import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertTrue;
public class GAVNameMapperTest extends NameMapperTestSupport {
@@ -99,6 +100,32 @@ public class GAVNameMapperTest extends NameMapperTestSupport {
names.iterator().next().name());
}
+ @Test
+ void pathTraversalInArtifactCoordinatesIsNeutralized() {
+ // wire-supplied coordinate fields must not be able to select a lock path outside the locks directory
+ DefaultArtifact artifact = new DefaultArtifact("group", "artifact", "jar", "1/../../../../home/user/x");
+ Collection<NamedLockKey> names = mapper.nameLocks(session, singletonList(artifact), null);
+ assertEquals(1, names.size());
+ String name = names.iterator().next().name();
+ assertEquals(
+ "artifact~group~artifact~1-SLASH-..-SLASH-..-SLASH-..-SLASH-..-SLASH-home-SLASH-user-SLASH-x.lock",
+ name);
+ assertFalse(name.contains("/"));
+ assertFalse(name.contains("\\"));
+ }
+
+ @Test
+ void pathTraversalInMetadataCoordinatesIsNeutralized() {
+ DefaultMetadata metadata = new DefaultMetadata(
+ "group", "artifact", "1\\..\\..\\x", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT);
+ Collection<NamedLockKey> names = mapper.nameLocks(session, null, singletonList(metadata));
+ assertEquals(1, names.size());
+ String name = names.iterator().next().name();
+ assertEquals("metadata~group~artifact~1-BACKSLASH-..-BACKSLASH-..-BACKSLASH-x.lock", name);
+ assertFalse(name.contains("/"));
+ assertFalse(name.contains("\\"));
+ }
+
@Test
void oneAndOne() {
DefaultArtifact artifact = new DefaultArtifact("agroup:artifact:1.0");
diff --git a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastCPSemaphoreNamedLockFactory.java b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastCPSemaphoreNamedLockFactory.java
index 46d4e78aa..61b2fa4aa 100644
--- a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastCPSemaphoreNamedLockFactory.java
+++ b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastCPSemaphoreNamedLockFactory.java
@@ -37,11 +37,25 @@ public class HazelcastCPSemaphoreNamedLockFactory extends HazelcastSemaphoreName
public static final String NAME = "semaphore-hazelcast";
/**
- * The default constructor: creates own instance of Hazelcast using standard Hazelcast configuration discovery.
+ * The default constructor: creates own instance of Hazelcast using standard Hazelcast configuration discovery,
+ * but refuses to run on Hazelcast built-in defaults: an explicit operator-provided configuration (the
+ * {@code hazelcast.config} system property, or a {@code hazelcast.xml}/{@code hazelcast.yaml}/{@code
+ * hazelcast.yml} file on the classpath or in the working directory) is required, as the built-in defaults form
+ * an unauthenticated cluster any network peer may join and manipulate lock state through.
*/
@Inject
public HazelcastCPSemaphoreNamedLockFactory() {
- this(Hazelcast.newHazelcastInstance(), true);
+ this(createConfiguredHazelcastInstance(), true);
+ }
+
+ private static HazelcastInstance createConfiguredHazelcastInstance() {
+ requireExplicitHazelcastConfig(
+ "hazelcast.config",
+ HazelcastCPSemaphoreNamedLockFactory.class.getClassLoader(),
+ "hazelcast.xml",
+ "hazelcast.yaml",
+ "hazelcast.yml");
+ return Hazelcast.newHazelcastInstance();
}
/**
diff --git a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastClientCPSemaphoreNamedLockFactory.java b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastClientCPSemaphoreNamedLockFactory.java
index 25b08950a..41b7af2ac 100644
--- a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastClientCPSemaphoreNamedLockFactory.java
+++ b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastClientCPSemaphoreNamedLockFactory.java
@@ -38,11 +38,25 @@ public class HazelcastClientCPSemaphoreNamedLockFactory extends HazelcastSemapho
public static final String NAME = "semaphore-hazelcast-client";
/**
- * The default constructor: creates own instance of Hazelcast using standard Hazelcast configuration discovery.
+ * The default constructor: creates own instance of Hazelcast client using standard Hazelcast configuration
+ * discovery, but refuses to run on Hazelcast built-in defaults: an explicit operator-provided configuration
+ * (the {@code hazelcast.client.config} system property, or a {@code hazelcast-client.xml}/{@code
+ * hazelcast-client.yaml}/{@code hazelcast-client.yml} file on the classpath or in the working directory) is
+ * required, as the built-in defaults connect to an unauthenticated default-named cluster.
*/
@Inject
public HazelcastClientCPSemaphoreNamedLockFactory() {
- this(HazelcastClient.newHazelcastClient(), true);
+ this(createConfiguredHazelcastClient(), true);
+ }
+
+ private static HazelcastInstance createConfiguredHazelcastClient() {
+ requireExplicitHazelcastConfig(
+ "hazelcast.client.config",
+ HazelcastClientCPSemaphoreNamedLockFactory.class.getClassLoader(),
+ "hazelcast-client.xml",
+ "hazelcast-client.yaml",
+ "hazelcast-client.yml");
+ return HazelcastClient.newHazelcastClient();
}
/**
diff --git a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java
index 0fe51998d..c0f741aea 100644
--- a/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java
+++ b/maven-resolver-named-locks-hazelcast/src/main/java/org/eclipse/aether/named/hazelcast/HazelcastSemaphoreNamedLockFactory.java
@@ -18,6 +18,8 @@
*/
package org.eclipse.aether.named.hazelcast;
+import java.nio.file.Files;
+import java.nio.file.Paths;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.ConcurrentMap;
import java.util.concurrent.TimeUnit;
@@ -59,6 +61,40 @@ public class HazelcastSemaphoreNamedLockFactory extends NamedLockFactorySupport
this.semaphores = new ConcurrentHashMap<>();
}
+ /**
+ * Guard used by the default constructors of subclasses before they create a Hazelcast member or client via
+ * standard Hazelcast configuration discovery: refuses to proceed when no explicit operator-provided
+ * configuration is discoverable. Without one, Hazelcast falls back to its built-in defaults (default cluster
+ * name, auto-discovery join, no authentication or TLS in open-source Hazelcast), which lets any peer able to
+ * reach the network segment join the cluster and release the semaphore permits that guard local repository
+ * writes.
+ *
+ * @param configSystemProperty the Hazelcast system property naming an explicit configuration file
+ * @param classLoader the class loader to probe for configuration resources
+ * @param configResourceNames configuration file names probed on the classpath and in the working directory
+ * @throws IllegalStateException if no explicit configuration is discoverable
+ */
+ static void requireExplicitHazelcastConfig(
+ final String configSystemProperty, final ClassLoader classLoader, final String... configResourceNames) {
+ if (System.getProperty(configSystemProperty) != null) {
+ return;
+ }
+ for (String configResourceName : configResourceNames) {
+ if (classLoader.getResource(configResourceName) != null
+ || Files.isRegularFile(Paths.get(configResourceName))) {
+ return;
+ }
+ }
+ throw new IllegalStateException(
+ "Refusing to create a Hazelcast instance from built-in defaults (default cluster name,"
+ + " auto-discovery join, no authentication): distributed lock state guarding local"
+ + " repository writes would be modifiable by unauthenticated network peers. Provide an"
+ + " explicit Hazelcast configuration that secures or isolates the cluster, via the '"
+ + configSystemProperty + "' system property or one of "
+ + String.join(", ", configResourceNames)
+ + " on the classpath or in the working directory.");
+ }
+
@Override
protected AdaptedSemaphoreNamedLock createLock(final NamedLockKey key) {
ISemaphore semaphore = semaphores.computeIfAbsent(
diff --git a/maven-resolver-named-locks-hazelcast/src/test/java/org/eclipse/aether/named/hazelcast/RequireExplicitHazelcastConfigTest.java b/maven-resolver-named-locks-hazelcast/src/test/java/org/eclipse/aether/named/hazelcast/RequireExplicitHazelcastConfigTest.java
new file mode 100644
index 000000000..a85de7ab0
--- /dev/null
+++ b/maven-resolver-named-locks-hazelcast/src/test/java/org/eclipse/aether/named/hazelcast/RequireExplicitHazelcastConfigTest.java
@@ -0,0 +1,63 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.eclipse.aether.named.hazelcast;
+
+import java.net.URL;
+import java.net.URLClassLoader;
+import java.util.UUID;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertThrows;
+
+/**
+ * UT for the explicit-configuration guard used by the default constructors: creating a Hazelcast member or client
+ * from built-in defaults would expose the lock state to unauthenticated network peers, so the guard must refuse
+ * when no explicit configuration is discoverable.
+ */
+class RequireExplicitHazelcastConfigTest {
+ @Test
+ void refusesWithoutExplicitConfiguration() {
+ ClassLoader emptyClassLoader = new URLClassLoader(new URL[0], null);
+ assertThrows(
+ IllegalStateException.class,
+ () -> HazelcastSemaphoreNamedLockFactory.requireExplicitHazelcastConfig(
+ "no.such.system.property." + UUID.randomUUID(), emptyClassLoader, "no-such-config.xml"));
+ }
+
+ @Test
+ void acceptsSystemPropertyConfiguration() {
+ String property = "test.hazelcast.config." + UUID.randomUUID();
+ System.setProperty(property, "somewhere.xml");
+ try {
+ ClassLoader emptyClassLoader = new URLClassLoader(new URL[0], null);
+ HazelcastSemaphoreNamedLockFactory.requireExplicitHazelcastConfig(
+ property, emptyClassLoader, "no-such-config.xml");
+ } finally {
+ System.clearProperty(property);
+ }
+ }
+
+ @Test
+ void acceptsClasspathConfiguration() {
+ // the test classpath ships hazelcast.xml (used by the ITs)
+ HazelcastSemaphoreNamedLockFactory.requireExplicitHazelcastConfig(
+ "no.such.system.property." + UUID.randomUUID(), getClass().getClassLoader(), "hazelcast.xml");
+ }
+}
diff --git a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java
index 7684be567..522f3033e 100644
--- a/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java
+++ b/maven-resolver-named-locks-redisson/src/main/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupport.java
@@ -20,9 +20,11 @@ package org.eclipse.aether.named.redisson;
import java.io.IOException;
import java.io.InputStream;
+import java.net.URI;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
+import java.util.Locale;
import org.eclipse.aether.named.support.NamedLockFactorySupport;
import org.redisson.Redisson;
@@ -60,6 +62,21 @@ public abstract class RedissonNamedLockFactorySupport extends NamedLockFactorySu
public static final String DEFAULT_REDIS_ADDRESS = "redis://localhost:6379";
+ /**
+ * Whether a plaintext {@code redis://} address pointing at a non-loopback host is allowed. Such a connection is
+ * unencrypted and unauthenticated at the transport, so a tampered, spoofed, or on-path-modified Redis can grant
+ * conflicting locks and corrupt the shared local repository the locks protect. Disabled by default; prefer a
+ * {@code rediss://} (TLS) address, or a Redisson configuration file with authentication, for anything
+ * cross-host.
+ *
+ * @since 2.0.22
+ * @configurationSource {@link System#getProperty(String, String)}
+ * @configurationType {@link java.lang.Boolean}
+ * @configurationDefaultValue false
+ */
+ public static final String SYSTEM_PROP_ALLOW_INSECURE_ADDRESS =
+ "aether.syncContext.named.redisson.allowInsecureAddress";
+
protected final RedissonClient redissonClient;
public RedissonNamedLockFactorySupport() {
@@ -106,6 +123,22 @@ public abstract class RedissonNamedLockFactorySupport extends NamedLockFactorySu
} else {
config = new Config();
String defaultRedisAddress = System.getProperty(SYSTEM_PROP_REDIS_ADDRESS, DEFAULT_REDIS_ADDRESS);
+ if (isInsecureRemoteAddress(defaultRedisAddress)) {
+ if (Boolean.getBoolean(SYSTEM_PROP_ALLOW_INSECURE_ADDRESS)) {
+ logger.warn(
+ "Using plaintext Redis address '{}' for lock state guarding local repository writes;"
+ + " the connection is unencrypted and unauthenticated at the transport, so the"
+ + " endpoint and the network path to it must be trusted and isolated",
+ defaultRedisAddress);
+ } else {
+ throw new IllegalStateException("Refusing plaintext non-loopback Redis address '"
+ + defaultRedisAddress + "': lock answers from a tampered or spoofed Redis can void"
+ + " mutual exclusion and corrupt the shared local repository. Use a 'rediss://' (TLS)"
+ + " address, or a Redisson configuration file ('" + SYSTEM_PROP_CONFIG_FILE
+ + "') with authentication, or explicitly opt in with -D"
+ + SYSTEM_PROP_ALLOW_INSECURE_ADDRESS + "=true");
+ }
+ }
config.useSingleServer().setAddress(defaultRedisAddress).setClientName(DEFAULT_CLIENT_NAME);
}
@@ -114,4 +147,28 @@ public abstract class RedissonNamedLockFactorySupport extends NamedLockFactorySu
return redissonClient;
}
+
+ /**
+ * Returns {@code true} if the given address is a plaintext {@code redis://} address pointing at a non-loopback
+ * host. TLS ({@code rediss://}) addresses and loopback addresses are acceptable defaults; anything else is
+ * insecure. Unparseable addresses are treated as insecure (fail closed).
+ */
+ static boolean isInsecureRemoteAddress(String address) {
+ if (address.toLowerCase(Locale.ROOT).startsWith("rediss://")) {
+ return false; // TLS protects the channel
+ }
+ String host;
+ try {
+ host = URI.create(address).getHost();
+ } catch (IllegalArgumentException e) {
+ return true;
+ }
+ if (host == null) {
+ return true;
+ }
+ return !("localhost".equalsIgnoreCase(host)
+ || host.startsWith("127.")
+ || "::1".equals(host)
+ || "[::1]".equals(host));
+ }
}
diff --git a/maven-resolver-named-locks-redisson/src/test/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupportTest.java b/maven-resolver-named-locks-redisson/src/test/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupportTest.java
new file mode 100644
index 000000000..206d3aeb9
--- /dev/null
+++ b/maven-resolver-named-locks-redisson/src/test/java/org/eclipse/aether/named/redisson/RedissonNamedLockFactorySupportTest.java
@@ -0,0 +1,59 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.eclipse.aether.named.redisson;
+
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * UT for the insecure-address guard: lock answers gate writes to the shared local repository, so a plaintext
+ * non-loopback Redis address must be refused unless explicitly opted into.
+ */
+class RedissonNamedLockFactorySupportTest {
+ @Test
+ void tlsAndLoopbackAddressesAreAcceptable() {
+ assertFalse(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("redis://localhost:6379"));
+ assertFalse(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("redis://127.0.0.1:6379"));
+ assertFalse(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("redis://[::1]:6379"));
+ assertFalse(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("rediss://redis.example.com:6379"));
+ }
+
+ @Test
+ void plaintextRemoteAddressesAreInsecure() {
+ assertTrue(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("redis://redis.ci.internal:6379"));
+ assertTrue(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("redis://10.0.0.5:6379"));
+ // unparseable addresses fail closed
+ assertTrue(RedissonNamedLockFactorySupport.isInsecureRemoteAddress("not a valid uri"));
+ }
+
+ @Test
+ void plaintextRemoteAddressRefusedAtFactoryCreation() {
+ System.setProperty(
+ RedissonNamedLockFactorySupport.SYSTEM_PROP_REDIS_ADDRESS, "redis://redis.ci.internal:6379");
+ try {
+ // refused before any connection attempt is made
+ assertThrows(IllegalStateException.class, RedissonSemaphoreNamedLockFactory::new);
+ } finally {
+ System.clearProperty(RedissonNamedLockFactorySupport.SYSTEM_PROP_REDIS_ADDRESS);
+ }
+ }
+}
diff --git a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java
index b484ba9bc..703508c74 100644
--- a/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java
+++ b/maven-resolver-spi/src/main/java/org/eclipse/aether/spi/io/PathProcessorSupport.java
@@ -22,10 +22,12 @@ import java.io.BufferedInputStream;
import java.io.BufferedOutputStream;
import java.io.IOException;
import java.io.InputStream;
+import java.io.InterruptedIOException;
import java.io.OutputStream;
import java.nio.ByteBuffer;
import java.nio.charset.StandardCharsets;
import java.nio.file.AccessDeniedException;
+import java.nio.file.AtomicMoveNotSupportedException;
import java.nio.file.FileSystemException;
import java.nio.file.Files;
import java.nio.file.Path;
@@ -45,7 +47,7 @@ import static java.util.Objects.requireNonNull;
public class PathProcessorSupport implements PathProcessor {
/**
* Logic borrowed from Commons-Lang3: we really need only this, to decide do we NIO2 file ops or not.
- * For some reason non-NIO2 works better on Windows.
+ * On Windows the final move needs retry/staging logic, see {@link #retryingMove(Path, Path, StandardCopyOption[])}.
*/
protected static final boolean IS_WINDOWS =
System.getProperty("os.name", "unknown").startsWith("Windows");
@@ -56,6 +58,19 @@ public class PathProcessorSupport implements PathProcessor {
protected static final boolean ATOMIC_MOVE =
Boolean.parseBoolean(System.getProperty(PathProcessor.class.getName() + "ATOMIC_MOVE", "true"));
+ /**
+ * The number of attempts for the final move on Windows, where the move target may be transiently locked by a
+ * virus scanner, an indexer or a concurrent reader.
+ */
+ protected static final int WINDOWS_MOVE_ATTEMPTS =
+ Math.max(1, Integer.getInteger(PathProcessor.class.getName() + "WINDOWS_MOVE_ATTEMPTS", 5));
+
+ /**
+ * The delay in milliseconds applied between the move attempts on Windows.
+ */
+ protected static final long WINDOWS_MOVE_RETRY_DELAY =
+ Long.getLong(PathProcessor.class.getName() + "WINDOWS_MOVE_RETRY_DELAY", 50L);
+
@Override
public boolean setLastModified(Path path, long value) throws IOException {
try {
@@ -172,7 +187,7 @@ public class PathProcessorSupport implements PathProcessor {
}
: new StandardCopyOption[] {StandardCopyOption.REPLACE_EXISTING, StandardCopyOption.COPY_ATTRIBUTES};
if (IS_WINDOWS) {
- classicCopy(source, target);
+ retryingMove(source, target, copyOption);
} else {
Files.move(source, target, copyOption);
}
@@ -223,7 +238,7 @@ public class PathProcessorSupport implements PathProcessor {
public void close() throws IOException {
if (wantsMove.get()) {
if (IS_WINDOWS) {
- classicCopy(tempFile, file);
+ retryingMove(tempFile, file, copyOption);
} else {
Files.move(tempFile, file, copyOption);
}
@@ -234,7 +249,67 @@ public class PathProcessorSupport implements PathProcessor {
}
/**
- * On Windows we use pre-NIO2 way to copy files, as for some reason it works. Beat me why.
+ * Moves the source file to the target path without ever truncating the target in place: the content visible at
+ * the target path is either the old file or the complete new file, never a partially written one.
+ * <p>
+ * Historically, on Windows the final move was implemented by opening the target path with a truncating stream
+ * and copying the source into it ({@link #classicCopy(Path, Path)}). That left a window during which a
+ * concurrent reader (for example a forked JVM resolving the same artifact) or an untimely process kill would
+ * observe, or durably leave behind, a truncated file at the final path - after checksum validation has already
+ * happened, so the torn content would from then on be trusted as verified. Instead, this method attempts the
+ * rename a bounded number of times ({@link #WINDOWS_MOVE_ATTEMPTS}, file locking by virus scanners, indexers or
+ * concurrent readers is transient on Windows), and if the file system refuses an atomic move (for example when
+ * source and target are on different stores), the source is first staged into a collocated temporary file next
+ * to the target and then renamed into place - the target path itself is never opened for writing.
+ */
+ protected void retryingMove(Path source, Path target, StandardCopyOption[] copyOptions) throws IOException {
+ FileSystemException lastException = null;
+ for (int attempt = 0; attempt < WINDOWS_MOVE_ATTEMPTS; attempt++) {
+ try {
+ fileSystemMove(source, target, copyOptions);
+ return;
+ } catch (AtomicMoveNotSupportedException e) {
+ lastException = e;
+ break; // retrying will not help; stage a collocated copy instead, see below
+ } catch (FileSystemException e) {
+ // covers AccessDeniedException and sharing violations: transient on Windows
+ lastException = e;
+ try {
+ Thread.sleep(WINDOWS_MOVE_RETRY_DELAY);
+ } catch (InterruptedException ie) {
+ Thread.currentThread().interrupt();
+ InterruptedIOException interrupted =
+ new InterruptedIOException("Interrupted while moving " + source + " to " + target);
+ interrupted.initCause(ie);
+ interrupted.addSuppressed(e);
+ throw interrupted;
+ }
+ }
+ }
+ if (lastException instanceof AtomicMoveNotSupportedException) {
+ Path staged = target.resolveSibling(target.getFileName() + "."
+ + Long.toUnsignedString(ThreadLocalRandom.current().nextLong()) + ".tmp");
+ try {
+ classicCopy(source, staged);
+ fileSystemMove(staged, target, copyOptions);
+ return;
+ } finally {
+ Files.deleteIfExists(staged);
+ }
+ }
+ throw lastException;
+ }
+
+ /**
+ * Testable seam for {@link Files#move(Path, Path, java.nio.file.CopyOption...)}.
+ */
+ protected void fileSystemMove(Path source, Path target, StandardCopyOption... copyOptions) throws IOException {
+ Files.move(source, target, copyOptions);
+ }
+
+ /**
+ * Pre-NIO2 way to copy files. Important: this method must never be pointed at a "final" (published) path, as it
+ * opens the target with truncation; callers stage into a temporary file and rename into place instead.
*/
protected void classicCopy(Path source, Path target) throws IOException {
ByteBuffer buffer = ByteBuffer.allocate(1024 * 32);
diff --git a/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java b/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java
new file mode 100644
index 000000000..ef845a1d5
--- /dev/null
+++ b/maven-resolver-spi/src/test/java/org/eclipse/aether/spi/io/PathProcessorSupportTest.java
@@ -0,0 +1,170 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.eclipse.aether.spi.io;
+
+import java.io.IOException;
+import java.nio.charset.StandardCharsets;
+import java.nio.file.AccessDeniedException;
+import java.nio.file.AtomicMoveNotSupportedException;
+import java.nio.file.Files;
+import java.nio.file.Path;
+import java.nio.file.StandardCopyOption;
+import java.util.concurrent.atomic.AtomicInteger;
+import java.util.stream.Stream;
+
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+/**
+ * UT for {@link PathProcessorSupport}, in particular the never-truncate-the-target contract of
+ * {@link PathProcessorSupport#retryingMove(Path, Path, StandardCopyOption[])} used for the final move on Windows.
+ */
+class PathProcessorSupportTest {
+ private static final StandardCopyOption[] ATOMIC_OPTIONS = {
+ StandardCopyOption.ATOMIC_MOVE, StandardCopyOption.REPLACE_EXISTING
+ };
+
+ private Path write(Path dir, String name, String content) throws IOException {
+ Path file = dir.resolve(name);
+ Files.write(file, content.getBytes(StandardCharsets.UTF_8));
+ return file;
+ }
+
+ private String read(Path file) throws IOException {
+ return new String(Files.readAllBytes(file), StandardCharsets.UTF_8);
+ }
+
+ @Test
+ void retryingMoveMovesFile(@TempDir Path tempDir) throws IOException {
+ PathProcessorSupport subject = new PathProcessorSupport();
+ Path source = write(tempDir, "source.txt", "content");
+ Path target = tempDir.resolve("target.txt");
+
+ subject.retryingMove(source, target, ATOMIC_OPTIONS);
+
+ assertEquals("content", read(target));
+ assertFalse(Files.exists(source));
+ }
+
+ @Test
+ void retryingMoveReplacesExistingTarget(@TempDir Path tempDir) throws IOException {
+ PathProcessorSupport subject = new PathProcessorSupport();
+ Path source = write(tempDir, "source.txt", "new content");
+ Path target = write(tempDir, "target.txt", "old content");
+
+ subject.retryingMove(source, target, ATOMIC_OPTIONS);
+
+ assertEquals("new content", read(target));
+ assertFalse(Files.exists(source));
+ }
+
+ @Test
+ void retryingMoveRetriesOnTransientLock(@TempDir Path tempDir) throws IOException {
+ AtomicInteger attempts = new AtomicInteger();
+ PathProcessorSupport subject = new PathProcessorSupport() {
+ @Override
+ protected void fileSystemMove(Path source, Path target, StandardCopyOption... copyOptions)
+ throws IOException {
+ if (attempts.incrementAndGet() < 3) {
+ // simulate a virus scanner / concurrent reader transiently holding the target
+ throw new AccessDeniedException(target.toString());
+ }
+ super.fileSystemMove(source, target, copyOptions);
+ }
+ };
+ Path source = write(tempDir, "source.txt", "new content");
+ Path target = write(tempDir, "target.txt", "old content");
+
+ subject.retryingMove(source, target, ATOMIC_OPTIONS);
+
+ assertEquals(3, attempts.get());
+ assertEquals("new content", read(target));
+ assertFalse(Files.exists(source));
+ }
+
+ @Test
+ void retryingMoveNeverTruncatesTargetOnFailure(@TempDir Path tempDir) throws IOException {
+ AtomicInteger attempts = new AtomicInteger();
+ PathProcessorSupport subject = new PathProcessorSupport() {
+ @Override
+ protected void fileSystemMove(Path source, Path target, StandardCopyOption... copyOptions)
+ throws IOException {
+ attempts.incrementAndGet();
+ throw new AccessDeniedException(target.toString());
+ }
+ };
+ Path source = write(tempDir, "source.txt", "new content");
+ Path target = write(tempDir, "target.txt", "old content");
+
+ assertThrows(AccessDeniedException.class, () -> subject.retryingMove(source, target, ATOMIC_OPTIONS));
+
+ assertEquals(PathProcessorSupport.WINDOWS_MOVE_ATTEMPTS, attempts.get());
+ // the crucial guarantee: a failed move leaves the previously published content fully intact,
+ // it never opens the final path with truncation
+ assertEquals("old content", read(target));
+ assertEquals("new content", read(source));
+ }
+
+ @Test
+ void retryingMoveStagesCollocatedCopyWhenAtomicMoveNotSupported(@TempDir Path tempDir) throws IOException {
+ PathProcessorSupport subject = new PathProcessorSupport() {
+ @Override
+ protected void fileSystemMove(Path source, Path target, StandardCopyOption... copyOptions)
+ throws IOException {
+ if (source.getFileName().toString().equals("source.txt")) {
+ // simulate source and target residing on different stores
+ throw new AtomicMoveNotSupportedException(source.toString(), target.toString(), "test");
+ }
+ super.fileSystemMove(source, target, copyOptions);
+ }
+ };
+ Path source = write(tempDir, "source.txt", "new content");
+ Path target = write(tempDir, "target.txt", "old content");
+
+ subject.retryingMove(source, target, ATOMIC_OPTIONS);
+
+ assertEquals("new content", read(target));
+ // no staging leftovers next to the target
+ try (Stream<Path> files = Files.list(tempDir)) {
+ assertEquals(
+ 0,
+ files.filter(p -> p.getFileName().toString().endsWith(".tmp"))
+ .count());
+ }
+ }
+
+ @Test
+ void writeFilePublishesCompleteContent(@TempDir Path tempDir) throws IOException {
+ PathProcessorSupport subject = new PathProcessorSupport();
+ Path target = write(tempDir, "target.txt", "old content");
+
+ subject.write(target, "new content");
+
+ assertEquals("new content", read(target));
+ // the collocated temp file used for staging is cleaned up
+ try (Stream<Path> files = Files.list(tempDir)) {
+ assertTrue(files.allMatch(p -> p.getFileName().toString().equals("target.txt")));
+ }
+ }
+}
diff --git a/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java b/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java
index a5db75623..1dbc4fd56 100644
--- a/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java
+++ b/maven-resolver-transport-file/src/main/java/org/eclipse/aether/transport/file/FileTransporter.java
@@ -163,10 +163,22 @@ final class FileTransporter extends AbstractTransporter {
private Path getPath(TransportTask task, boolean required) throws Exception {
String path = task.getLocation().getPath();
- if (path.contains("../")) {
+ if (path == null) {
+ throw new IllegalArgumentException("illegal resource path: null");
+ }
+ Path relative = fileSystem.getPath(path);
+ if (relative.isAbsolute()) {
+ throw new IllegalArgumentException("illegal resource path: " + path);
+ }
+ for (Path segment : relative) {
+ if ("..".equals(segment.toString())) {
+ throw new IllegalArgumentException("illegal resource path: " + path);
+ }
+ }
+ Path file = basePath.resolve(relative).normalize();
+ if (!file.startsWith(basePath.normalize())) {
throw new IllegalArgumentException("illegal resource path: " + path);
}
- Path file = basePath.resolve(path);
if (required && !Files.isRegularFile(file)) {
throw new ResourceNotFoundException("Could not locate " + file);
}
diff --git a/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java b/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java
index 0a24bce75..e52ee8eb3 100644
--- a/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java
+++ b/maven-resolver-transport-file/src/test/java/org/eclipse/aether/transport/file/FileTransporterTest.java
@@ -106,6 +106,20 @@ public class FileTransporterTest {
transporter = factory.newInstance(session, newRepo(url));
}
+ /**
+ * Builds a task location URI carrying the absolute path of the given resource within the repository base
+ * directory, in a platform and file system neutral way (e.g. {@code /C:/repo/file.txt} on Windows).
+ */
+ private URI absoluteLocation(String resource) throws Exception {
+ Path absolute =
+ ((FileTransporter) transporter).getBasePath().resolve(resource).toAbsolutePath();
+ String rawPath = absolute.toString().replace('\\', '/');
+ if (!rawPath.startsWith("/")) {
+ rawPath = "/" + rawPath;
+ }
+ return new URI(null, null, rawPath, null);
+ }
+
void setUp(FS fs) {
try {
fileSystem = fs.name().startsWith("JIMFS") ? Jimfs.newFileSystem() : null;
@@ -300,6 +314,46 @@ public class FileTransporterTest {
}
}
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testGet_AbsoluteResourcePathRejected(FS fs) throws Exception {
+ setUp(fs);
+ GetTask task = new GetTask(absoluteLocation("file.txt"));
+ assertThrows(IllegalArgumentException.class, () -> transporter.get(task));
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testGet_ParentDirTraversalRejected(FS fs) {
+ setUp(fs);
+ assertThrows(
+ IllegalArgumentException.class, () -> transporter.get(new GetTask(URI.create("../file.txt"))));
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testGet_BareParentDirRejected(FS fs) {
+ setUp(fs);
+ assertThrows(IllegalArgumentException.class, () -> transporter.get(new GetTask(URI.create(".."))));
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testGet_NestedParentDirEscapeRejected(FS fs) {
+ setUp(fs);
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> transporter.get(new GetTask(URI.create("a/../../file.txt"))));
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testPeek_ParentDirTraversalRejected(FS fs) {
+ setUp(fs);
+ assertThrows(
+ IllegalArgumentException.class, () -> transporter.peek(new PeekTask(URI.create("../file.txt"))));
+ }
+
@ParameterizedTest
@EnumSource(FS.class)
void testGet_Closed(FS fs) throws Exception {
@@ -449,6 +503,38 @@ public class FileTransporterTest {
}
}
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testPut_AbsoluteResourcePathRejected(FS fs) throws Exception {
+ assumeTrue(fs.writable);
+ setUp(fs);
+ PutTask task = new PutTask(absoluteLocation("absolute-upload.txt")).setDataString("upload");
+ assertThrows(IllegalArgumentException.class, () -> transporter.put(task));
+ assertFalse(Files.exists(repoDir.resolve("absolute-upload.txt")));
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testPut_ParentDirTraversalRejected(FS fs) throws Exception {
+ assumeTrue(fs.writable);
+ setUp(fs);
+ PutTask task = new PutTask(URI.create("../escaped.txt")).setDataString("upload");
+ assertThrows(IllegalArgumentException.class, () -> transporter.put(task));
+ Path base = ((FileTransporter) transporter).getBasePath();
+ assertFalse(Files.exists(base.getParent().resolve("escaped.txt")));
+ }
+
+ @ParameterizedTest
+ @EnumSource(FS.class)
+ void testPut_NestedParentDirEscapeRejected(FS fs) throws Exception {
+ assumeTrue(fs.writable);
+ setUp(fs);
+ PutTask task = new PutTask(URI.create("a/../../escaped.txt")).setDataString("upload");
+ assertThrows(IllegalArgumentException.class, () -> transporter.put(task));
+ Path base = ((FileTransporter) transporter).getBasePath();
+ assertFalse(Files.exists(base.getParent().resolve("escaped.txt")));
+ }
+
@ParameterizedTest
@EnumSource(FS.class)
void testPut_Closed(FS fs) throws Exception {
diff --git a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterConfigurationKeys.java b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterConfigurationKeys.java
index cb7ef34ac..793a34487 100644
--- a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterConfigurationKeys.java
+++ b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterConfigurationKeys.java
@@ -56,4 +56,21 @@ public final class MinioTransporterConfigurationKeys {
public static final String CONFIG_PROP_FIXED_BUCKET_NAME = CONFIG_PROPS_PREFIX + "fixedBucketName";
public static final String DEFAULT_FIXED_BUCKET_NAME = "maven";
+
+ /**
+ * Whether plaintext HTTP object storage endpoints (repository URLs in {@code minio+http} or {@code s3+http}
+ * form) are allowed. Plaintext endpoints offer no transport integrity (artifacts and their checksum objects
+ * travel over the same tamperable channel) and, because the repository protocol string is not {@code http},
+ * such URLs are not matched by {@code external:http:*} mirror blocking. They are therefore refused unless this
+ * property is explicitly set to {@code true}.
+ *
+ * @since 2.0.22
+ * @configurationSource {@link RepositorySystemSession#getConfigProperties()}
+ * @configurationType {@link Boolean}
+ * @configurationDefaultValue {@link #DEFAULT_ALLOW_INSECURE_PROTOCOL}
+ * @configurationRepoIdSuffix Yes
+ */
+ public static final String CONFIG_PROP_ALLOW_INSECURE_PROTOCOL = CONFIG_PROPS_PREFIX + "allowInsecureProtocol";
+
+ public static final boolean DEFAULT_ALLOW_INSECURE_PROTOCOL = false;
}
diff --git a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterFactory.java b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterFactory.java
index 1f1851772..ec348e8e0 100644
--- a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterFactory.java
+++ b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/MinioTransporterFactory.java
@@ -76,21 +76,46 @@ public final class MinioTransporterFactory implements TransporterFactory {
// this check is here only to support "minio+http" and "s3+http" protocols by default. But also when
// raised priorities by user, allow to "overtake" plain HTTP repositories, if needed.
RemoteRepository adjusted = repository;
+ boolean prefixStripped = false;
if ("minio+http".equalsIgnoreCase(repository.getProtocol())
|| "minio+https".equalsIgnoreCase(repository.getProtocol())) {
adjusted = new RemoteRepository.Builder(repository)
.setUrl(repository.getUrl().substring("minio+".length()))
.build();
+ prefixStripped = true;
} else if ("s3+http".equalsIgnoreCase(repository.getProtocol())
|| "s3+https".equalsIgnoreCase(repository.getProtocol())) {
adjusted = new RemoteRepository.Builder(repository)
.setUrl(repository.getUrl().substring("s3+".length()))
.build();
+ prefixStripped = true;
} else if (priority == DEFAULT_PRIORITY) {
throw new NoTransporterException(
repository,
"To use Minio transport with plain HTTP/HTTPS repositories, increase the Minio transport priority");
}
+ // Plaintext endpoints from "minio+http"/"s3+http" URLs are not matched by "external:http:*" mirror
+ // blocking (the repository protocol string is not "http") and offer no transport integrity: artifacts
+ // and their checksum objects travel over the same tamperable channel. Refuse them unless the operator
+ // explicitly opted in.
+ if (prefixStripped && "http".equalsIgnoreCase(adjusted.getProtocol())) {
+ boolean allowInsecureProtocol = ConfigUtils.getBoolean(
+ session,
+ MinioTransporterConfigurationKeys.DEFAULT_ALLOW_INSECURE_PROTOCOL,
+ MinioTransporterConfigurationKeys.CONFIG_PROP_ALLOW_INSECURE_PROTOCOL + "."
+ + repository.getId(),
+ MinioTransporterConfigurationKeys.CONFIG_PROP_ALLOW_INSECURE_PROTOCOL);
+ if (!allowInsecureProtocol) {
+ throw new NoTransporterException(
+ repository,
+ "Plaintext HTTP object storage endpoints are insecure (no transport integrity; not subject"
+ + " to 'external:http:*' mirror blocking) and are refused by default: use the"
+ + " 'minio+https'/'s3+https' URL form, or explicitly opt in by setting the"
+ + " configuration property '"
+ + MinioTransporterConfigurationKeys.CONFIG_PROP_ALLOW_INSECURE_PROTOCOL
+ + "' (optionally suffixed with '." + repository.getId() + "') to 'true'");
+ }
+ }
String objectNameMapperConf = ConfigUtils.getString(
session,
MinioTransporterConfigurationKeys.DEFAULT_OBJECT_NAME_MAPPER,
diff --git a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactory.java b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactory.java
index 01de8dc35..1fdd69542 100644
--- a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactory.java
+++ b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactory.java
@@ -47,11 +47,13 @@ public class FixedBucketObjectNameMapperFactory implements ObjectNameMapperFacto
RemoteRepository repository,
MinioClient unused,
Map<String, String> headers) {
+ // ConfigUtils.getString(session, defaultValue, keys...): the default value ("maven") comes first,
+ // then the repository-suffixed key, then the global key.
String bucket = ConfigUtils.getString(
session,
- MinioTransporterConfigurationKeys.CONFIG_PROP_FIXED_BUCKET_NAME,
+ MinioTransporterConfigurationKeys.DEFAULT_FIXED_BUCKET_NAME,
MinioTransporterConfigurationKeys.CONFIG_PROP_FIXED_BUCKET_NAME + "." + repository.getId(),
- MinioTransporterConfigurationKeys.DEFAULT_FIXED_BUCKET_NAME);
+ MinioTransporterConfigurationKeys.CONFIG_PROP_FIXED_BUCKET_NAME);
return path -> new ObjectName(bucket, repository.getId() + "/" + ObjectName.normalize(path));
}
}
diff --git a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/package-info.java b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/package-info.java
index 9d6fad1d6..e32f44c27 100644
--- a/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/package-info.java
+++ b/maven-resolver-transport-minio/src/main/java/org/eclipse/aether/transport/minio/package-info.java
@@ -20,9 +20,11 @@
/**
* Support for downloads/uploads via the S3 protocol. The implementation is backed by
* <a href="https://github.com/minio/minio-java">MinIO Java</a>.
- * The repository URL should be defined with protocol {@code minio+http} or {@code s3+http}. Note: use "https" if
- * you are going for HTTPS remote, factory will merely strip "minio+" or "s3+" prefix assuming resulting URL will
- * point to expected S3 endpoint.
+ * The repository URL should be defined with protocol {@code minio+https} or {@code s3+https}; the factory will
+ * merely strip the "minio+" or "s3+" prefix assuming the resulting URL will point to the expected S3 endpoint.
+ * The plaintext {@code minio+http}/{@code s3+http} forms are refused by default: they offer no transport integrity
+ * and are not matched by {@code external:http:*} mirror blocking. To use a plaintext endpoint anyway, explicitly
+ * set the configuration property {@code aether.transport.minio.allowInsecureProtocol} to {@code true}.
*
* @since 2.0.2
*/
diff --git a/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/MinioTransporterFactoryTest.java b/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/MinioTransporterFactoryTest.java
index b2c3c83d0..7a839148b 100644
--- a/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/MinioTransporterFactoryTest.java
+++ b/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/MinioTransporterFactoryTest.java
@@ -58,7 +58,7 @@ class MinioTransporterFactoryTest {
factory = new MinioTransporterFactory(factories, new PathProcessorSupport());
session = new DefaultRepositorySystemSession(h -> true);
- repository = new RemoteRepository.Builder("repo", "default", "minio+http://localhost")
+ repository = new RemoteRepository.Builder("repo", "default", "minio+https://localhost")
.setAuthentication(new AuthenticationBuilder()
.addUsername("username")
.addPassword("password")
@@ -88,6 +88,35 @@ class MinioTransporterFactoryTest {
assertThrows(NoTransporterException.class, () -> factory.newInstance(session, remoteRepository.get()));
}
+ @Test
+ void plaintextHttpRefusedByDefault() {
+ for (String url : new String[] {"minio+http://localhost", "s3+http://localhost"}) {
+ RemoteRepository plaintext = new RemoteRepository.Builder("repo", "default", url)
+ .setAuthentication(new AuthenticationBuilder()
+ .addUsername("username")
+ .addPassword("password")
+ .build())
+ .build();
+ assertThrows(NoTransporterException.class, () -> factory.newInstance(session, plaintext));
+ }
+ }
+
+ @Test
+ void plaintextHttpAllowedWithExplicitOptIn() throws Exception {
+ DefaultRepositorySystemSession insecureSession = new DefaultRepositorySystemSession(h -> true);
+ insecureSession.setConfigProperty(
+ MinioTransporterConfigurationKeys.CONFIG_PROP_ALLOW_INSECURE_PROTOCOL, Boolean.TRUE);
+ RemoteRepository plaintext = new RemoteRepository.Builder("repo", "default", "minio+http://localhost")
+ .setAuthentication(new AuthenticationBuilder()
+ .addUsername("username")
+ .addPassword("password")
+ .build())
+ .build();
+ try (Transporter transporter = factory.newInstance(insecureSession, plaintext)) {
+ // nope
+ }
+ }
+
@Test
void httpOvertake() throws Exception {
factory.setPriority(1000);
diff --git a/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactoryTest.java b/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactoryTest.java
new file mode 100644
index 000000000..0c9b495ef
--- /dev/null
+++ b/maven-resolver-transport-minio/src/test/java/org/eclipse/aether/transport/minio/internal/FixedBucketObjectNameMapperFactoryTest.java
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.eclipse.aether.transport.minio.internal;
+
+import java.util.Collections;
+
+import org.eclipse.aether.DefaultRepositorySystemSession;
+import org.eclipse.aether.repository.RemoteRepository;
+import org.eclipse.aether.transport.minio.MinioTransporterConfigurationKeys;
+import org.eclipse.aether.transport.minio.ObjectNameMapper;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+
+/**
+ * UT asserting the effective bucket is exactly the one the operator configured, with the documented default:
+ * a swapped {@code ConfigUtils.getString} argument order previously made the effective bucket the literal
+ * configuration key string and ignored the documented global key.
+ */
+class FixedBucketObjectNameMapperFactoryTest {
+ private final FixedBucketObjectNameMapperFactory factory = new FixedBucketObjectNameMapperFactory();
+
+ private final RemoteRepository repository =
+ new RemoteRepository.Builder("repo", "default", "minio+https://localhost").build();
+
+ private String effectiveBucket(DefaultRepositorySystemSession session) {
+ ObjectNameMapper mapper = factory.create(session, repository, null, Collections.emptyMap());
+ return mapper.name("g/a/v/a.jar").getBucket();
+ }
+
+ @Test
+ void documentedDefaultBucketNameIsUsed() {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> true);
+ assertEquals(MinioTransporterConfigurationKeys.DEFAULT_FIXED_BUCKET_NAME, effectiveBucket(session));
+ }
+
+ @Test
+ void globalConfigurationKeyIsHonored() {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> true);
+ session.setConfigProperty(MinioTransporterConfigurationKeys.CONFIG_PROP_FIXED_BUCKET_NAME, "custom-bucket");
+ assertEquals("custom-bucket", effectiveBucket(session));
+ }
+
+ @Test
+ void repositoryIdSuffixedKeyTakesPrecedence() {
+ DefaultRepositorySystemSession session = new DefaultRepositorySystemSession(h -> true);
+ session.setConfigProperty(MinioTransporterConfigurationKeys.CONFIG_PROP_FIXED_BUCKET_NAME, "custom-bucket");
+ session.setConfigProperty(
+ MinioTransporterConfigurationKeys.CONFIG_PROP_FIXED_BUCKET_NAME + "." + repository.getId(),
+ "repo-bucket");
+ assertEquals("repo-bucket", effectiveBucket(session));
+ }
+}
diff --git a/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java b/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java
index b853ae111..fb656fb5e 100644
--- a/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java
+++ b/maven-resolver-util/src/main/java/org/eclipse/aether/util/PathUtils.java
@@ -73,7 +73,17 @@ public final class PathUtils {
pos = result.indexOf(illegal);
}
}
- return result.toString();
+ // Strings consisting solely of dots contain no illegal character, yet "." and ".." carry path meaning:
+ // used as a path segment, ".." escapes the intended base directory (for example a repository id of ".."
+ // redirects origin-aware trusted-checksums lookups and split local repository prefixes outside their
+ // basedir). Map them to explicit tokens, mirroring the character replacements above.
+ String segment = result.toString();
+ if (segment.equals(".")) {
+ return "-DOT-";
+ } else if (segment.equals("..")) {
+ return "-DOTDOT-";
+ }
+ return segment;
}
/**
@@ -87,9 +97,33 @@ public final class PathUtils {
if (value != null && !value.isEmpty()) {
// Important: "equals .." and not "contains ..", as if escape attempted, it will contain path separators
// OTOH: version "1.." is valid version string!
- if (value.equals("..") || value.contains("/") || value.contains("\\")) {
+ // Colon is never valid in a coordinate component: it enables drive-absolute paths on Windows and
+ // scheme/authority confusion when the component ends up in a URI.
+ if (value.equals("..") || value.contains("/") || value.contains("\\") || value.contains(":")) {
throw new IllegalArgumentException(
- "Invalid " + label + ": must not contain '..', '/' or '\\': " + value);
+ "Invalid " + label + ": must not contain '..', '/', '\\' or ':': " + value);
+ }
+ }
+ }
+
+ /**
+ * Validates a coordinate component that is expanded into multiple path segments by replacing each dot with a
+ * path separator, like the group ID is. Beside the checks done by
+ * {@link #validatePathComponent(String, String)}, it rejects values containing empty dot-separated segments
+ * (leading, trailing or consecutive dots): after dot-to-slash expansion those would produce absolute paths
+ * (".home.ci" becomes "/home/ci", escaping the local repository), protocol-relative authority URIs
+ * ("..evilhost" becomes "//evilhost", redirecting remote fetches to another host) or empty path segments.
+ *
+ * @since 2.0.22
+ */
+ public static void validateDotSeparatedPathComponent(String value, String label) {
+ validatePathComponent(value, label);
+ if (value != null && !value.isEmpty()) {
+ for (String segment : value.split("\\.", -1)) {
+ if (segment.isEmpty()) {
+ throw new IllegalArgumentException("Invalid " + label
+ + ": must not contain empty segments (leading, trailing or consecutive dots): " + value);
+ }
}
}
}
@@ -98,10 +132,11 @@ public final class PathUtils {
* Validates all coordinate components of an {@link Artifact}.
*
* @see #validatePathComponent(String, String)
+ * @see #validateDotSeparatedPathComponent(String, String)
* @since 2.0.21
*/
public static void validateArtifactComponents(Artifact artifact) {
- validatePathComponent(artifact.getGroupId(), "groupId");
+ validateDotSeparatedPathComponent(artifact.getGroupId(), "groupId");
validatePathComponent(artifact.getArtifactId(), "artifactId");
validatePathComponent(artifact.getVersion(), "version");
validatePathComponent(artifact.getBaseVersion(), "baseVersion");
@@ -113,10 +148,11 @@ public final class PathUtils {
* Validates all coordinate components of a {@link Metadata}.
*
* @see #validatePathComponent(String, String)
+ * @see #validateDotSeparatedPathComponent(String, String)
* @since 2.0.21
*/
public static void validateMetadataComponents(Metadata metadata) {
- validatePathComponent(metadata.getGroupId(), "groupId");
+ validateDotSeparatedPathComponent(metadata.getGroupId(), "groupId");
validatePathComponent(metadata.getArtifactId(), "artifactId");
validatePathComponent(metadata.getVersion(), "version");
// note: type may contain string like ".meta/prefixes.txt"!
diff --git a/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java b/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java
index 97c1b4563..9ec4d5a93 100644
--- a/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java
+++ b/maven-resolver-util/src/test/java/org/eclipse/aether/util/PathUtilsTest.java
@@ -22,8 +22,10 @@ import java.util.function.UnaryOperator;
import org.junit.jupiter.api.Test;
+import static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
public class PathUtilsTest {
@Test
@@ -40,6 +42,20 @@ public class PathUtilsTest {
assertEquals("bad-COLON-id", badFixedId);
}
+ @Test
+ void stringToPathSegment_dotSegments() {
+ // "." and ".." contain no illegal character, but carry path meaning: a repository id of ".." used as a
+ // path segment escapes the intended base directory of every sink that splices it into a path
+ // (origin-aware trusted checksums, split local repository prefixes)
+ assertEquals("-DOTDOT-", PathUtils.stringToPathSegment(".."));
+ assertEquals("-DOT-", PathUtils.stringToPathSegment("."));
+ // dotted names and longer dot runs are inert as single path segments and stay untouched
+ assertEquals("...", PathUtils.stringToPathSegment("..."));
+ assertEquals("my.repo", PathUtils.stringToPathSegment("my.repo"));
+ assertEquals("repo..id", PathUtils.stringToPathSegment("repo..id"));
+ assertEquals("..id", PathUtils.stringToPathSegment("..id"));
+ }
+
@Test
void stringToPathSegment_allCharsBad() {
String veryBad = "\\/:\"<>|?*";
@@ -47,4 +63,58 @@ public class PathUtilsTest {
assertNotEquals(veryBad, badFixedId);
assertEquals("-BACKSLASH--SLASH--COLON--QUOTE--LT--GT--PIPE--QMARK--ASTERISK-", badFixedId);
}
+
+ @Test
+ void validatePathComponent_rejectsTraversalSeparatorsAndColon() {
+ assertThrows(IllegalArgumentException.class, () -> PathUtils.validatePathComponent("..", "version"));
+ assertThrows(IllegalArgumentException.class, () -> PathUtils.validatePathComponent("1.0/../etc", "version"));
+ assertThrows(
+ IllegalArgumentException.class, () -> PathUtils.validatePathComponent("1.0\\..\\etc", "version"));
+ assertThrows(IllegalArgumentException.class, () -> PathUtils.validatePathComponent("C:", "groupId"));
+ assertThrows(IllegalArgumentException.class, () -> PathUtils.validatePathComponent("1.0:evil", "version"));
+ }
+
+ @Test
+ void validatePathComponent_acceptsNormalValues() {
+ assertDoesNotThrow(() -> PathUtils.validatePathComponent(null, "version"));
+ assertDoesNotThrow(() -> PathUtils.validatePathComponent("", "version"));
+ assertDoesNotThrow(() -> PathUtils.validatePathComponent("commons-io", "artifactId"));
+ assertDoesNotThrow(() -> PathUtils.validatePathComponent("1.0-SNAPSHOT", "version"));
+ // version "1.." is a valid version string, and is not dot-expanded into path segments
+ assertDoesNotThrow(() -> PathUtils.validatePathComponent("1..", "version"));
+ }
+
+ @Test
+ void validateDotSeparatedPathComponent_rejectsEmptyDotSegments() {
+ // leading dot: ".home.ci" would expand to absolute path "/home/ci"
+ assertThrows(
+ IllegalArgumentException.class, () -> PathUtils.validateDotSeparatedPathComponent(".home.ci", "groupId"));
+ // leading double dot: "..evilhost" would expand to authority URI "//evilhost"
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> PathUtils.validateDotSeparatedPathComponent("..evilhost", "groupId"));
+ // consecutive dots
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> PathUtils.validateDotSeparatedPathComponent("com..example", "groupId"));
+ // trailing dot
+ assertThrows(
+ IllegalArgumentException.class,
+ () -> PathUtils.validateDotSeparatedPathComponent("com.example.", "groupId"));
+ // dot(s) only
+ assertThrows(IllegalArgumentException.class, () -> PathUtils.validateDotSeparatedPathComponent(".", "groupId"));
+ // checks of validatePathComponent still apply
+ assertThrows(
+ IllegalArgumentException.class, () -> PathUtils.validateDotSeparatedPathComponent("a:b", "groupId"));
+ assertThrows(
+ IllegalArgumentException.class, () -> PathUtils.validateDotSeparatedPathComponent("a/b", "groupId"));
+ }
+
+ @Test
+ void validateDotSeparatedPathComponent_acceptsNormalValues() {
+ assertDoesNotThrow(() -> PathUtils.validateDotSeparatedPathComponent(null, "groupId"));
+ assertDoesNotThrow(() -> PathUtils.validateDotSeparatedPathComponent("", "groupId"));
+ assertDoesNotThrow(() -> PathUtils.validateDotSeparatedPathComponent("org.apache.maven", "groupId"));
+ assertDoesNotThrow(() -> PathUtils.validateDotSeparatedPathComponent("commons-io", "groupId"));
+ }
}