From 57fa740b3285d40d7363764222025da761d7b81e Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 18:21:53 +0200 Subject: [PATCH 1/7] Fail closed on metadata checksum mismatches under the fail policy DefaultRepositoryMetadataManager.resolve() caught ChecksumFailedException through the generic TransferFailedException handler, so a checksum mismatch under checksumPolicy=fail only logged a warning and kept the previously cached metadata, and the finally block touched the update-check file regardless, deferring the next check for a full update interval. Catch ChecksumFailedException ahead of the generic handler and fail resolution instead, and only touch the update-check file on success, not-found, or a generic transfer failure, so a checksum failure is retried on the next build rather than cached. --- .../DefaultRepositoryMetadataManager.java | 14 +++- .../DefaultRepositoryMetadataManagerTest.java | 81 +++++++++++++++++++ 2 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java diff --git a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java index a7d052455ad7..dce85691c196 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java @@ -41,6 +41,7 @@ import org.apache.maven.artifact.repository.RepositoryRequest; import org.apache.maven.metadata.v4.MetadataStaxReader; import org.apache.maven.metadata.v4.MetadataStaxWriter; +import org.apache.maven.repository.legacy.ChecksumFailedException; import org.apache.maven.repository.legacy.UpdateCheckManager; import org.apache.maven.repository.legacy.WagonManager; import org.apache.maven.wagon.ResourceDoesNotExistException; @@ -118,6 +119,17 @@ public void resolve(RepositoryMetadata metadata, RepositoryRequest request) getLogger().info(metadata.getKey() + ": checking for updates from " + repository.getId()); try { wagonManager.getArtifactMetadata(metadata, repository, file, policy.getChecksumPolicy()); + updateCheckManager.touch(metadata, repository, file); + } catch (ChecksumFailedException e) { + // ChecksumFailedException is only thrown by the wagon manager under + // CHECKSUM_POLICY_FAIL: honor the strict policy by failing metadata resolution + // instead of downgrading the integrity failure to a warning. The update + // tracking file is deliberately not touched, so the next build retries + // immediately instead of trusting stale metadata for a full update interval. + throw new RepositoryMetadataResolutionException( + metadata + " failed checksum verification against repository: " + repository.getId() + + " due to an error: " + e.getMessage(), + e); } catch (ResourceDoesNotExistException e) { getLogger().debug(metadata + " could not be found on repository: " + repository.getId()); @@ -133,12 +145,12 @@ public void resolve(RepositoryMetadata metadata, RepositoryRequest request) file.delete(); // if this fails, forget about it } } + updateCheckManager.touch(metadata, repository, file); } catch (TransferFailedException e) { getLogger() .warn(metadata + " could not be retrieved from repository: " + repository.getId() + " due to an error: " + e.getMessage()); getLogger().debug("Exception", e); - } finally { updateCheckManager.touch(metadata, repository, file); } } diff --git a/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java new file mode 100644 index 000000000000..1dcae79b6330 --- /dev/null +++ b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerTest.java @@ -0,0 +1,81 @@ +/* + * 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.apache.maven.artifact.repository.metadata; + +import javax.inject.Inject; +import javax.inject.Named; + +import java.io.File; +import java.util.Collections; + +import org.apache.maven.artifact.AbstractArtifactComponentTestCase; +import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy; +import org.apache.maven.artifact.repository.layout.ArtifactRepositoryLayout; +import org.codehaus.plexus.util.FileUtils; +import org.junit.jupiter.api.Test; + +import static org.codehaus.plexus.testing.PlexusExtension.getBasedir; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Tests {@link DefaultRepositoryMetadataManager}. + */ +@Deprecated +class DefaultRepositoryMetadataManagerTest extends AbstractArtifactComponentTestCase { + + @Inject + private RepositoryMetadataManager repositoryMetadataManager; + + @Inject + @Named("default") + private ArtifactRepositoryLayout layout; + + @Override + protected String component() { + return "repositoryMetadataManager"; + } + + @Test + void testResolveHonorsConfiguredFailChecksumPolicy() throws Exception { + RepositoryMetadata metadata = new GroupRepositoryMetadata("checksum-policy-test-group"); + + ArtifactRepositoryPolicy failPolicy = new ArtifactRepositoryPolicy( + true, ArtifactRepositoryPolicy.UPDATE_POLICY_ALWAYS, ArtifactRepositoryPolicy.CHECKSUM_POLICY_FAIL); + + File remoteBase = new File(getBasedir(), "target/test-repositories/" + component() + "/remote-repository"); + FileUtils.deleteDirectory(remoteBase); + + ArtifactRepository remoteRepo = artifactRepositoryFactory.createArtifactRepository( + "test", "file://" + remoteBase.getPath(), layout, failPolicy, failPolicy); + + String remotePath = remoteRepo.pathOfRemoteRepositoryMetadata(metadata); + File remoteFile = new File(remoteBase, remotePath); + remoteFile.getParentFile().mkdirs(); + FileUtils.fileWrite(remoteFile.getAbsolutePath(), ""); + FileUtils.fileWrite(remoteFile.getAbsolutePath() + ".sha1", "0000000000000000000000000000000000000000"); + + ArtifactRepository localRepo = localRepository(); + FileUtils.deleteDirectory(new File(localRepo.getBasedir())); + + assertThrows( + RepositoryMetadataResolutionException.class, + () -> repositoryMetadataManager.resolve(metadata, Collections.singletonList(remoteRepo), localRepo)); + } +} From 9326805219e44615499953c5734f3f4236b35317 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 19:18:32 +0200 Subject: [PATCH 2/7] Thread the configured checksum policy through two legacy metadata fetches DefaultRepositoryMetadataManager.getArtifactMetadataFromDeploymentRepository and LegacyRepositorySystem.retrieve hardcoded checksumPolicy=warn for every transfer, so a repository configured with checksumPolicy=fail (or the global -C/--strict-checksums flag) was not honored on the deploy-metadata fetch or on generic retrieve() calls. Resolve the effective policy from the repository instead: the metadata fetch uses the same per-metadata policy resolution the download path already uses, and retrieve() takes the stricter of the repository's release and snapshot policies since a generic path cannot be classified as either. --- .../DefaultRepositoryMetadataManager.java | 18 +++++++++- .../legacy/LegacyRepositorySystem.java | 35 ++++++++++++++++++- .../legacy/LegacyRepositorySystemTest.java | 25 +++++++++++++ 3 files changed, 76 insertions(+), 2 deletions(-) diff --git a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java index dce85691c196..477bbe47c67f 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java @@ -367,7 +367,7 @@ private File getArtifactMetadataFromDeploymentRepository( try { wagonManager.getArtifactMetadataFromDeploymentRepository( - metadata, remoteRepository, file, ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN); + metadata, remoteRepository, file, getChecksumPolicy(metadata, remoteRepository)); } catch (ResourceDoesNotExistException e) { getLogger() .info(metadata + " could not be found on repository: " + remoteRepository.getId() @@ -393,6 +393,22 @@ private File getArtifactMetadataFromDeploymentRepository( return file; } + /** + * Determines the effective checksum policy for a transfer from the given repository. The + * operator-configured policy (e.g. {@code fail} via {@code -C}/{@code --strict-checksums} or a + * per-repository {@code checksumPolicy}) must govern every remote transfer, so it must not be + * hardcoded at the call sites. + */ + private String getChecksumPolicy(ArtifactMetadata metadata, ArtifactRepository repository) { + if (metadata instanceof RepositoryMetadata repositoryMetadata) { + ArtifactRepositoryPolicy policy = repositoryMetadata.getPolicy(repository); + if (policy != null && policy.getChecksumPolicy() != null) { + return policy.getChecksumPolicy(); + } + } + return ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN; + } + @Override public void deploy( ArtifactMetadata metadata, ArtifactRepository localRepository, ArtifactRepository deploymentRepository) diff --git a/compat/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java b/compat/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java index 281f4069e678..e703e23da943 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/repository/legacy/LegacyRepositorySystem.java @@ -660,7 +660,7 @@ public void retrieve( destination, remotePath, TransferListenerAdapter.newAdapter(transferListener), - ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN, + getChecksumPolicy(repository), true); } catch (org.apache.maven.wagon.TransferFailedException e) { throw new ArtifactTransferFailedException(getMessage(e, "Error transferring artifact."), e); @@ -669,6 +669,39 @@ public void retrieve( } } + /** + * Determines the effective checksum policy for a generic retrieval from the given repository. + * The operator-configured policy (e.g. {@code fail} via {@code -C}/{@code --strict-checksums}) + * must govern every remote transfer instead of a hardcoded lenient default. A generic remote + * path cannot be classified as release or snapshot, so the stricter of the two configured + * policies applies. + */ + private static String getChecksumPolicy(ArtifactRepository repository) { + String releases = + (repository.getReleases() != null) ? repository.getReleases().getChecksumPolicy() : null; + String snapshots = + (repository.getSnapshots() != null) ? repository.getSnapshots().getChecksumPolicy() : null; + String policy; + if (releases == null) { + policy = snapshots; + } else if (snapshots == null || checksumRank(releases) >= checksumRank(snapshots)) { + policy = releases; + } else { + policy = snapshots; + } + return (policy != null) ? policy : ArtifactRepositoryPolicy.CHECKSUM_POLICY_WARN; + } + + private static int checksumRank(String policy) { + if (ArtifactRepositoryPolicy.CHECKSUM_POLICY_FAIL.equals(policy)) { + return 2; + } else if (ArtifactRepositoryPolicy.CHECKSUM_POLICY_IGNORE.equals(policy)) { + return 0; + } else { + return 1; + } + } + @Override public void publish( ArtifactRepository repository, File source, String remotePath, ArtifactTransferListener transferListener) diff --git a/compat/maven-compat/src/test/java/org/apache/maven/repository/legacy/LegacyRepositorySystemTest.java b/compat/maven-compat/src/test/java/org/apache/maven/repository/legacy/LegacyRepositorySystemTest.java index 82554ea83ac7..879722f82847 100644 --- a/compat/maven-compat/src/test/java/org/apache/maven/repository/legacy/LegacyRepositorySystemTest.java +++ b/compat/maven-compat/src/test/java/org/apache/maven/repository/legacy/LegacyRepositorySystemTest.java @@ -21,16 +21,21 @@ import javax.inject.Inject; import java.io.File; +import java.nio.file.Files; import java.util.Arrays; import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.repository.ArtifactRepositoryPolicy; import org.apache.maven.artifact.repository.Authentication; +import org.apache.maven.repository.ArtifactTransferFailedException; import org.apache.maven.settings.Server; import org.codehaus.plexus.testing.PlexusTest; 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.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; /** * Tests {@link LegacyRepositorySystem}. @@ -64,4 +69,24 @@ void testAuthenticationHandling() { assertEquals("jason", authentication.getUsername()); assertEquals("abc123", authentication.getPassword()); } + + @Test + void testRetrieveHonorsConfiguredFailChecksumPolicy(@TempDir File tempDir) throws Exception { + File remoteBase = new File(tempDir, "remote"); + remoteBase.mkdirs(); + File remoteFile = new File(remoteBase, "sample.txt"); + Files.writeString(remoteFile.toPath(), "content"); + Files.writeString(new File(remoteBase, "sample.txt.sha1").toPath(), "0000000000000000000000000000000000000000"); + + ArtifactRepositoryPolicy failPolicy = new ArtifactRepositoryPolicy( + true, ArtifactRepositoryPolicy.UPDATE_POLICY_ALWAYS, ArtifactRepositoryPolicy.CHECKSUM_POLICY_FAIL); + ArtifactRepository repository = repositorySystem.createArtifactRepository( + "test", "file://" + remoteBase.getAbsolutePath(), null, failPolicy, failPolicy); + + File destination = new File(tempDir, "sample.txt"); + + assertThrows( + ArtifactTransferFailedException.class, + () -> repositorySystem.retrieve(repository, destination, "sample.txt", null)); + } } From 1a20766709815af1fd4d431143289bd0b363a7f9 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 19:30:22 +0200 Subject: [PATCH 3/7] Validate repository key and metadata version tokens on the legacy path AbstractRepositoryMetadata.getLocalFilename rejects a repository key that is the ".." token, contains '/', '\', ':', or an ISO control character, before using it in a local file name. DefaultRepositoryMetadataManager.readMetadata applies the same check to every version token carried by parsed repository metadata (latest, release, versions, snapshot versions, snapshot timestamp) before the metadata is merged and used to resolve a version. Matches the check already used for relocation and version-range coordinates elsewhere in the resolver. --- .../metadata/AbstractRepositoryMetadata.java | 28 +++++++- .../DefaultRepositoryMetadataManager.java | 51 +++++++++++++- .../AbstractRepositoryMetadataTest.java | 70 +++++++++++++++++++ ...positoryMetadataManagerValidationTest.java | 63 +++++++++++++++++ .../maven-metadata.xml | 31 ++++++++ .../metadata-invalid-token/maven-metadata.xml | 28 ++++++++ 6 files changed, 269 insertions(+), 2 deletions(-) create mode 100644 compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadataTest.java create mode 100644 compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java create mode 100644 compat/maven-compat/src/test/resources/metadata-invalid-timestamp/maven-metadata.xml create mode 100644 compat/maven-compat/src/test/resources/metadata-invalid-token/maven-metadata.xml diff --git a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadata.java b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadata.java index c9c7d83c563e..307051b4d141 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadata.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadata.java @@ -54,7 +54,33 @@ public String getRemoteFilename() { @Override public String getLocalFilename(ArtifactRepository repository) { - return "maven-metadata-" + repository.getKey() + ".xml"; + return "maven-metadata-" + validateRepositoryKey(repository.getKey()) + ".xml"; + } + + /** + * The repository key (its id) is used verbatim as part of a local file name, so it must lie within + * the usual coordinate character set. + */ + private static String validateRepositoryKey(String key) { + if (key == null || key.isEmpty()) { + return key; + } + if (isInvalidPathToken(key)) { + throw new IllegalArgumentException("Invalid repository key '" + key + "'"); + } + return key; + } + + private static boolean isInvalidPathToken(String value) { + if ("..".equals(value) || value.indexOf('/') >= 0 || value.indexOf('\\') >= 0 || value.indexOf(':') >= 0) { + return true; + } + for (int i = 0; i < value.length(); i++) { + if (Character.isISOControl(value.charAt(i))) { + return true; + } + } + return false; } @Override diff --git a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java index 477bbe47c67f..c45134ec0d67 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManager.java @@ -285,7 +285,11 @@ private boolean loadMetadata( protected Metadata readMetadata(File mappingFile) throws RepositoryMetadataReadException { try (InputStream in = Files.newInputStream(mappingFile.toPath())) { - return new Metadata(new MetadataStaxReader().read(in, false)); + Metadata result = new Metadata(new MetadataStaxReader().read(in, false)); + + validateVersioning(result); + + return result; } catch (FileNotFoundException e) { throw new RepositoryMetadataReadException("Cannot read metadata from '" + mappingFile + "'", e); } catch (IOException | XMLStreamException e) { @@ -294,6 +298,51 @@ protected Metadata readMetadata(File mappingFile) throws RepositoryMetadataReadE } } + /** + * Version tokens adopted from repository metadata must be valid coordinate components; metadata carrying + * anything else is treated as invalid. + */ + private static void validateVersioning(Metadata metadata) throws RepositoryMetadataReadException { + if (metadata == null) { + return; + } + Versioning versioning = metadata.getVersioning(); + if (versioning == null) { + return; + } + validateVersionToken(versioning.getLatest()); + validateVersionToken(versioning.getRelease()); + for (String version : versioning.getVersions()) { + validateVersionToken(version); + } + for (SnapshotVersion snapshotVersion : versioning.getSnapshotVersions()) { + validateVersionToken(snapshotVersion.getVersion()); + } + Snapshot snapshot = versioning.getSnapshot(); + if (snapshot != null) { + validateVersionToken(snapshot.getTimestamp()); + } + } + + private static void validateVersionToken(String value) throws RepositoryMetadataReadException { + if (value == null || value.isEmpty()) { + return; + } + boolean valid = !"..".equals(value); + if (valid) { + for (int i = 0; i < value.length(); i++) { + char c = value.charAt(i); + if (c == '/' || c == '\\' || c == ':' || Character.isISOControl(c)) { + valid = false; + break; + } + } + } + if (!valid) { + throw new RepositoryMetadataReadException("Metadata contains an invalid version token: '" + value + "'"); + } + } + /** * Ensures the last updated timestamp of the specified metadata does not refer to the future and fixes the local * metadata if necessary to allow proper merging/updating of metadata during deployment. diff --git a/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadataTest.java b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadataTest.java new file mode 100644 index 000000000000..c24186976349 --- /dev/null +++ b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/AbstractRepositoryMetadataTest.java @@ -0,0 +1,70 @@ +/* + * 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.apache.maven.artifact.repository.metadata; + +import org.apache.maven.artifact.repository.ArtifactRepository; +import org.apache.maven.artifact.repository.DefaultArtifactRepository; +import org.apache.maven.artifact.repository.layout.DefaultRepositoryLayout; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Ensures that a repository key which may originate from a downloaded POM's {@code } section + * cannot select a local metadata file name outside the intended one. + */ +class AbstractRepositoryMetadataTest { + + private static ArtifactRepository repository(String id) { + return new DefaultArtifactRepository(id, "http://repo.example/r", new DefaultRepositoryLayout()); + } + + @Test + void repositoryKeyWithColonIsRejected() { + RepositoryMetadata metadata = new GroupRepositoryMetadata("org.test"); + ArtifactRepository repo = repository("central:1.0"); + + assertThrows(IllegalArgumentException.class, () -> metadata.getLocalFilename(repo)); + } + + @Test + void repositoryKeyWithDotDotSegmentIsRejected() { + RepositoryMetadata metadata = new GroupRepositoryMetadata("org.test"); + ArtifactRepository repo = repository("x/../../../../../../settings"); + + assertThrows(IllegalArgumentException.class, () -> metadata.getLocalFilename(repo)); + } + + @Test + void repositoryKeyWithBackslashIsRejected() { + RepositoryMetadata metadata = new GroupRepositoryMetadata("org.test"); + ArtifactRepository repo = repository("x\\..\\..\\settings"); + + assertThrows(IllegalArgumentException.class, () -> metadata.getLocalFilename(repo)); + } + + @Test + void wellFormedRepositoryKeyProducesExpectedFilename() { + RepositoryMetadata metadata = new GroupRepositoryMetadata("org.test"); + ArtifactRepository repo = repository("central"); + + assertEquals("maven-metadata-central.xml", metadata.getLocalFilename(repo)); + } +} diff --git a/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.java new file mode 100644 index 000000000000..2167606b810b --- /dev/null +++ b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/metadata/DefaultRepositoryMetadataManagerValidationTest.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.apache.maven.artifact.repository.metadata; + +import java.io.File; +import java.net.URL; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; + +/** + * Tests that {@link DefaultRepositoryMetadataManager} rejects repository metadata carrying version tokens that + * are not valid coordinate components, on the legacy read path used when metadata is loaded for merging. + */ +class DefaultRepositoryMetadataManagerValidationTest { + + private final DefaultRepositoryMetadataManager manager = new DefaultRepositoryMetadataManager(); + + @Test + void testMetadataWithInvalidVersionTokenIsRejected() { + File metadataFile = testFile("metadata-invalid-token/maven-metadata.xml"); + + RepositoryMetadataReadException exception = + assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile)); + + assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage()); + } + + @Test + void testMetadataWithInvalidSnapshotTimestampIsRejected() { + File metadataFile = testFile("metadata-invalid-timestamp/maven-metadata.xml"); + + RepositoryMetadataReadException exception = + assertThrows(RepositoryMetadataReadException.class, () -> manager.readMetadata(metadataFile)); + + assertTrue(exception.getMessage().contains("invalid version token"), exception.getMessage()); + } + + private static File testFile(String resource) { + URL url = Thread.currentThread().getContextClassLoader().getResource(resource); + assertNotNull(url, "test resource not found: " + resource); + return new File(url.getFile()); + } +} diff --git a/compat/maven-compat/src/test/resources/metadata-invalid-timestamp/maven-metadata.xml b/compat/maven-compat/src/test/resources/metadata-invalid-timestamp/maven-metadata.xml new file mode 100644 index 000000000000..68cf46a41f3d --- /dev/null +++ b/compat/maven-compat/src/test/resources/metadata-invalid-timestamp/maven-metadata.xml @@ -0,0 +1,31 @@ + + + + org.apache.maven.its + dep-invalid-timestamp + 1.0-SNAPSHOT + + + 20120809.112920:1 + 1 + + 20120809112920 + + diff --git a/compat/maven-compat/src/test/resources/metadata-invalid-token/maven-metadata.xml b/compat/maven-compat/src/test/resources/metadata-invalid-token/maven-metadata.xml new file mode 100644 index 000000000000..8f4eb7200c7b --- /dev/null +++ b/compat/maven-compat/src/test/resources/metadata-invalid-token/maven-metadata.xml @@ -0,0 +1,28 @@ + + + + org.apache.maven.its + dep-invalid-token + 1.0-SNAPSHOT + + 1.0:2.0 + 20120809112920 + + From ba38de88126aa9ba8ff28db82f125439f7e9342f Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 17:50:02 +0200 Subject: [PATCH 4/7] Clone proxy before decrypting its password The proxy loop in DefaultSettingsDecrypter.decrypt mutated the caller's live Proxy objects directly, unlike the server loop just above it which already clones before mutating. Since compat model objects propagate setter changes up into their parent Settings delegate, this meant a decrypted proxy password could end up written back into the session-wide Settings object. Clone the proxy first, mirroring the server handling, so decryption only ever touches the copies returned in the result. --- .../crypto/DefaultSettingsDecrypter.java | 5 ++ .../crypto/DefaultSettingsDecrypterTest.java | 62 +++++++++++++++++++ 2 files changed, 67 insertions(+) create mode 100644 compat/maven-settings-builder/src/test/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypterTest.java diff --git a/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypter.java b/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypter.java index 26bd1c552453..7fc8a2f12afd 100644 --- a/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypter.java +++ b/compat/maven-settings-builder/src/main/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypter.java @@ -114,6 +114,11 @@ public SettingsDecryptionResult decrypt(SettingsDecryptionRequest request) { List proxies = new ArrayList<>(); for (Proxy proxy : request.getProxies()) { + // Clone the proxy (mirroring the server handling above) so that decrypted + // plaintext only ever lands in the SettingsDecryptionResult copies and never + // mutates the caller's live Settings object. + proxy = proxy.clone(); + String password = proxy.getPassword(); if (securityDispatcher.isAnyEncryptedString(password)) { try { diff --git a/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypterTest.java b/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypterTest.java new file mode 100644 index 000000000000..db26a01f7d0b --- /dev/null +++ b/compat/maven-settings-builder/src/test/java/org/apache/maven/settings/crypto/DefaultSettingsDecrypterTest.java @@ -0,0 +1,62 @@ +/* + * 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.apache.maven.settings.crypto; + +import java.util.ArrayList; +import java.util.List; +import java.util.Map; + +import org.apache.maven.settings.Proxy; +import org.apache.maven.settings.Server; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotSame; + +@Deprecated +class DefaultSettingsDecrypterTest { + + @Test + void testDecryptionDoesNotMutateCallerObjects() { + String ciphertext = "{COQLCE6DU6GtcS5P=}"; + + Server server = new Server(); + server.setId("test-server"); + server.setPassword(ciphertext); + + Proxy proxy = new Proxy(); + proxy.setId("test-proxy"); + proxy.setPassword(ciphertext); + + DefaultSettingsDecryptionRequest request = new DefaultSettingsDecryptionRequest(); + request.setServers(new ArrayList<>(List.of(server))); + request.setProxies(new ArrayList<>(List.of(proxy))); + + DefaultSettingsDecrypter decrypter = new DefaultSettingsDecrypter(new MavenSecDispatcher(Map.of())); + SettingsDecryptionResult result = decrypter.decrypt(request); + + // decryption state (including plaintext on success) must only ever land in the + // result copies, never in the caller's live settings objects + assertNotSame(server, result.getServers().get(0)); + assertNotSame(proxy, result.getProxies().get(0)); + assertEquals(ciphertext, server.getPassword()); + assertEquals(ciphertext, proxy.getPassword()); + assertEquals("test-proxy", result.getProxies().get(0).getId()); + } +} From 9d03e0ffd67372a8bba51bed6fe80bb91bb9d865 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 22:28:49 +0200 Subject: [PATCH 5/7] Validate the repository key used in legacy metadata file names --- .../LegacyLocalRepositoryManager.java | 28 ++++++++- .../LegacyLocalRepositoryManagerTest.java | 58 +++++++++++++++++++ 2 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManagerTest.java diff --git a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManager.java b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManager.java index 0d36d42f1f8c..65d64cf279dd 100644 --- a/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManager.java +++ b/compat/maven-compat/src/main/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManager.java @@ -228,7 +228,7 @@ public String getRemoteFilename() { @Override public String getLocalFilename(ArtifactRepository repository) { - return insertRepositoryKey(getRemoteFilename(), repository.getKey()); + return insertRepositoryKey(getRemoteFilename(), validateRepositoryKey(repository.getKey())); } private String insertRepositoryKey(String filename, String repositoryKey) { @@ -242,6 +242,32 @@ private String insertRepositoryKey(String filename, String repositoryKey) { return result; } + /** + * The repository key (its id) is used verbatim as part of a local file name, so it must lie within + * the usual coordinate character set. + */ + private static String validateRepositoryKey(String key) { + if (key == null || key.isEmpty()) { + return key; + } + if (isInvalidPathToken(key)) { + throw new IllegalArgumentException("Invalid repository key '" + key + "'"); + } + return key; + } + + private static boolean isInvalidPathToken(String value) { + if ("..".equals(value) || value.indexOf('/') >= 0 || value.indexOf('\\') >= 0 || value.indexOf(':') >= 0) { + return true; + } + for (int i = 0; i < value.length(); i++) { + if (Character.isISOControl(value.charAt(i))) { + return true; + } + } + return false; + } + @Override public void merge(org.apache.maven.repository.legacy.metadata.ArtifactMetadata metadata) { // not used diff --git a/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManagerTest.java b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManagerTest.java new file mode 100644 index 000000000000..2c2a8fabea06 --- /dev/null +++ b/compat/maven-compat/src/test/java/org/apache/maven/artifact/repository/LegacyLocalRepositoryManagerTest.java @@ -0,0 +1,58 @@ +/* + * 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.apache.maven.artifact.repository; + +import org.apache.maven.artifact.repository.layout.DefaultRepositoryLayout; +import org.eclipse.aether.metadata.DefaultMetadata; +import org.eclipse.aether.metadata.Metadata; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; + +class LegacyLocalRepositoryManagerTest { + + private static LegacyLocalRepositoryManager.ArtifactMetadataAdapter newAdapter() { + Metadata metadata = + new DefaultMetadata("g", "a", "1.0", "maven-metadata.xml", Metadata.Nature.RELEASE_OR_SNAPSHOT); + return new LegacyLocalRepositoryManager.ArtifactMetadataAdapter(metadata); + } + + private static ArtifactRepository repositoryWithId(String id) { + return new DefaultArtifactRepository(id, "http://example.invalid/repo", new DefaultRepositoryLayout()); + } + + @Test + void getLocalFilenameKeepsWellFormedRepositoryKeyUnchanged() { + String filename = newAdapter().getLocalFilename(repositoryWithId("central")); + + assertEquals("maven-metadata-central.xml", filename); + } + + @Test + void getLocalFilenameRejectsRepositoryKeyContainingPathSeparator() { + assertThrows( + IllegalArgumentException.class, () -> newAdapter().getLocalFilename(repositoryWithId("repo/evil"))); + } + + @Test + void getLocalFilenameRejectsRepositoryKeyThatIsAParentDirectoryReference() { + assertThrows(IllegalArgumentException.class, () -> newAdapter().getLocalFilename(repositoryWithId(".."))); + } +} From e1d9a852b9f7a5f81c6ea97ab344df81e0e62cbe Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 1 Sep 2026 11:12:59 +0200 Subject: [PATCH 6/7] Validate metadata inputs and relocation coordinates in the new API resolvers Add MetadataInputValidator shared utility to consolidate validation logic for the impl/maven-impl resolver layer. Wire validation into DefaultVersionResolver, DefaultVersionRangeResolver, and DistributionManagementArtifactRelocationSource to reject path-traversal sequences (..), separators (/, \), drive-letter delimiters (:), and ISO control characters in metadata version tokens and relocation coordinates. Co-Authored-By: Claude Opus 4.6 --- .../resolver/DefaultVersionRangeResolver.java | 6 +- .../impl/resolver/DefaultVersionResolver.java | 6 +- .../impl/resolver/MetadataInputValidator.java | 103 ++++++++++++++++ ...ionManagementArtifactRelocationSource.java | 21 +++- .../resolver/MetadataInputValidatorTest.java | Bin 0 -> 5876 bytes ...anagementArtifactRelocationSourceTest.java | 115 ++++++++++++++++++ 6 files changed, 243 insertions(+), 8 deletions(-) create mode 100644 impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/MetadataInputValidatorTest.java create mode 100644 impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSourceTest.java diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionRangeResolver.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionRangeResolver.java index b16caa3b72e7..97a8853cd7b8 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionRangeResolver.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionRangeResolver.java @@ -231,8 +231,12 @@ private Versioning readVersions( if (metadata.getPath() != null && Files.exists(metadata.getPath())) { try (InputStream in = Files.newInputStream(metadata.getPath())) { - versioning = + Versioning parsed = new MetadataStaxReader().read(in, false).getVersioning(); + + MetadataInputValidator.validateVersioning(parsed); + + versioning = parsed; } } } diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionResolver.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionResolver.java index 05a756cc2016..24f6b3488bc1 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionResolver.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/DefaultVersionResolver.java @@ -243,9 +243,13 @@ private Versioning readVersions( if (metadata.getPath() != null && Files.exists(metadata.getPath())) { try (InputStream in = Files.newInputStream(metadata.getPath())) { - versioning = + Versioning parsed = new MetadataStaxReader().read(in, false).getVersioning(); + MetadataInputValidator.validateVersioning(parsed); + + versioning = parsed; + /* NOTE: Users occasionally misuse the id "local" for remote repos which screws up the metadata of the local repository. This is especially troublesome during snapshot resolution so we try diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java new file mode 100644 index 000000000000..0f6d431d2aa6 --- /dev/null +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java @@ -0,0 +1,103 @@ +/* + * 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.apache.maven.impl.resolver; + +import java.io.IOException; + +import org.apache.maven.api.metadata.Snapshot; +import org.apache.maven.api.metadata.SnapshotVersion; +import org.apache.maven.api.metadata.Versioning; + +/** + * Validates metadata content parsed from remote {@code maven-metadata.xml} files before the values + * are used to compose filesystem paths or artifact coordinates. + *

+ * Repository metadata is not covered by the model validator (it is not a POM), so version tokens, + * snapshot timestamps, and relocation coordinates must be checked at the point of use. Values that + * would map onto filesystem path-traversal segments ({@code ..}), separators ({@code /}, {@code \}), + * drive-letter delimiters ({@code :}), or ISO control characters are rejected. + * + * @since 4.1.0 + */ +public final class MetadataInputValidator { + + private MetadataInputValidator() {} + + /** + * Returns {@code true} if the value is unsafe for use as a coordinate component in a filesystem + * path: it is {@code ".."}, contains a separator ({@code /}, {@code \}, {@code :}), or contains + * an ISO control character. + */ + public static boolean isInvalidCoordinateComponent(String value) { + if (value == null || value.isEmpty()) { + return false; + } + if ("..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":")) { + return true; + } + for (int i = 0; i < value.length(); i++) { + if (Character.isISOControl(value.charAt(i))) { + return true; + } + } + return false; + } + + /** + * Validates all version-related tokens inside a parsed {@link Versioning} element. + * + * @throws IOException if any token is invalid + */ + public static void validateVersioning(Versioning versioning) throws IOException { + if (versioning == null) { + return; + } + validateVersionToken(versioning.getLatest(), "latest version"); + validateVersionToken(versioning.getRelease(), "release version"); + for (String version : versioning.getVersions()) { + validateVersionToken(version, "version"); + } + for (SnapshotVersion snapshotVersion : versioning.getSnapshotVersions()) { + validateVersionToken(snapshotVersion.getVersion(), "snapshot version"); + } + Snapshot snapshot = versioning.getSnapshot(); + if (snapshot != null) { + validateVersionToken(snapshot.getTimestamp(), "snapshot timestamp"); + } + } + + /** + * Validates a single version token from repository metadata. + * + * @throws IOException if the token contains path-traversal sequences, separators, or control characters + */ + public static void validateVersionToken(String value, String description) throws IOException { + if (value == null || value.isEmpty()) { + return; + } + boolean invalid = "..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":"); + for (int i = 0; i < value.length() && !invalid; i++) { + invalid = Character.isISOControl(value.charAt(i)); + } + if (invalid) { + throw new IOException("Rejecting metadata with invalid " + description + " '" + value + + "': must not contain '..', '/', '\\', ':' or control characters"); + } + } +} diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSource.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSource.java index 2549b4f653eb..7326e25dc7ef 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSource.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSource.java @@ -25,6 +25,7 @@ import org.apache.maven.api.model.Model; import org.apache.maven.api.model.Relocation; import org.apache.maven.impl.resolver.MavenArtifactRelocationSource; +import org.apache.maven.impl.resolver.MetadataInputValidator; import org.apache.maven.impl.resolver.RelocatedArtifact; import org.eclipse.aether.RepositorySystemSession; import org.eclipse.aether.artifact.Artifact; @@ -54,22 +55,30 @@ public Artifact relocatedTarget( if (distMgmt != null) { Relocation relocation = distMgmt.getRelocation(); if (relocation != null) { + Artifact original = artifactDescriptorResult.getRequest().getArtifact(); + validateRelocationCoordinate(relocation.getGroupId(), "groupId", original); + validateRelocationCoordinate(relocation.getArtifactId(), "artifactId", original); + validateRelocationCoordinate(relocation.getVersion(), "version", original); + Artifact result = new RelocatedArtifact( - artifactDescriptorResult.getRequest().getArtifact(), + original, relocation.getGroupId(), relocation.getArtifactId(), null, null, relocation.getVersion(), relocation.getMessage()); - LOGGER.debug( - "The artifact {} has been relocated to {}: {}", - artifactDescriptorResult.getRequest().getArtifact(), - result, - relocation.getMessage()); + LOGGER.debug("The artifact {} has been relocated to {}: {}", original, result, relocation.getMessage()); return result; } } return null; } + + private static void validateRelocationCoordinate(String value, String component, Artifact artifact) { + if (value != null && !value.isEmpty() && MetadataInputValidator.isInvalidCoordinateComponent(value)) { + throw new IllegalArgumentException("Invalid relocation " + component + " '" + value + "' for " + artifact + + ": not a valid artifact coordinate component"); + } + } } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/MetadataInputValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/MetadataInputValidatorTest.java new file mode 100644 index 0000000000000000000000000000000000000000..198af1c0f097cc5fbf0e080ac33a65f5e1b06b73 GIT binary patch literal 5876 zcmd5=+iu%95bd+SV&Esai0LHV?n8qv>gIwKnspYj+oC{TXo;^^ueP<}j zc48&X#o28TB$hOnGiNRwx%aXKFX2=sLRpc5)8J+T`UOw$-%wB7f*S!Rx>PB5QY(o1 z!;=o+r7)nCAZ=|R*C>@}<%}Gc4sT?YMc`8-L@t!G3}7gPH$ONV#fQf*k(t*|rLFW3 zz(Tqi%18?fZ7yMgrg)l4a?LZ43UA~d2st)l%1tWO6kQirMowoA^g;<^XR<(lBMN1B zQiHLT;o2?6dZo({%39Qcj2-xdhA8BHwhIvokjr*>+U^{9?VMkM(hf>Xp&@Lt4&qxP z3WsnJSYBk3tHf)E=tNC8V*G*-$cP?0j`G}x0(#=R`*Kn#3c07>JD)Z6_b#KShc?+@eQ4t$PBf1Z6D!RP+@d4Dj9kB4w} z4u@xhqj(gb4e;s&`hzc|`5_)0?SQ~?z(Nz>3PV94Qb~M@bgi0d3{)#2OSTY+oX7;j zQ`3@91x)o^7)7O02$M@oiM0qa#p1}N^Ce};??!A4@Ag_q^(EOsvTNRRJ{O7+9){p_ zh7Jx|Exgvo!B;-#OllUN9s629cdLgRMtC7vE*!@m<7}vSVQ1PM)VA&?9;)@8T0t5K zxu)>7RMN4pWg(p~7^q|;VG&6U#S&AAXJJ#cZ>=!Ysg`m7@kk3h&~7v{da-o#FcT*{ zv*MAu-miVEvMJGTtM}6P9kF2H`@2l4OZY2fDpp15KJiRq+O+vm6-ts}?6-)J75Lt2 zVH-oQ2ia^_OdM)Wql`Ooh$90ht8yez3BMyvuQt~*h1(Qt)IsxPoqnnucmktl z$m}lfQ5$Dhj>+s&6TND8ItSY`y~lRh?siSy?2_$TXET|mLT%S5XKcK*_@gn^s?4&! zN{{oxUGZ*Ja-j%LqH~aRtlRPn=t9&e`+$=Dk%aSKasc zSop=o%3yPCBs$aTd7?Rd^AjfhuBWrZ88Sgi~VX$%k6);Z=+ z`W<*gPH|4%8>1gt4}rKmq^UsYXIq$ijM1-fNw2_KU!?aBVr^PHB$VwS#}0aN2h`^7 zr#FQ=*RK!<%2DHs>#e_f2=l6K$==)gjS?@I5{vhx%TINpWjKedi`w`aM|;wy zfDxJCR@C#MK_WP|k$)8_oy^!NzGyQ&sggZ&2eF@cLVcD_f?8jLwwaZzs^&3#ED zVntyCztw8{BrNNDPcsTxE#{{_6#-7ed{_a!SFV?-rDcWP!t!>e%|j>;!Jb!TwY_MU zbxMQ;vi{tahNL(c>(G-87)ecW`qlDqEDhA&*g^QJu6d8ff~HHQATX01sa)yrB# z@zyd>2Z^UpeeXI8%at!`uSV0L%*S{_?KK3yV)Gj7x1DjU1`yRB8f^X-g#Kv#3o(#~ AVgLXD literal 0 HcmV?d00001 diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSourceTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSourceTest.java new file mode 100644 index 000000000000..fc463a1b6bf6 --- /dev/null +++ b/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/relocation/DistributionManagementArtifactRelocationSourceTest.java @@ -0,0 +1,115 @@ +/* + * 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.apache.maven.impl.resolver.relocation; + +import org.apache.maven.api.model.DistributionManagement; +import org.apache.maven.api.model.Model; +import org.apache.maven.api.model.Relocation; +import org.eclipse.aether.artifact.Artifact; +import org.eclipse.aether.artifact.DefaultArtifact; +import org.eclipse.aether.resolution.ArtifactDescriptorRequest; +import org.eclipse.aether.resolution.ArtifactDescriptorResult; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; +import static org.junit.jupiter.api.Assertions.assertThrows; + +/** + * Tests {@link DistributionManagementArtifactRelocationSource}. + */ +class DistributionManagementArtifactRelocationSourceTest { + + private final DistributionManagementArtifactRelocationSource source = + new DistributionManagementArtifactRelocationSource(); + + @Test + void validRelocationReturnsRelocatedArtifact() { + Model model = Model.newBuilder() + .distributionManagement(DistributionManagement.newBuilder() + .relocation(Relocation.newBuilder() + .groupId("org.apache.new") + .artifactId("new-artifact") + .version("2.0.0") + .build()) + .build()) + .build(); + + Artifact result = source.relocatedTarget(null, descriptorResult(), model); + assertNotNull(result); + } + + @Test + void noRelocationReturnsNull() { + Model model = Model.newBuilder().build(); + Artifact result = source.relocatedTarget(null, descriptorResult(), model); + assertNull(result); + } + + @Test + void pathTraversalGroupIdThrows() { + Model model = modelWithRelocation("..", "new-artifact", "1.0"); + assertThrows(IllegalArgumentException.class, () -> source.relocatedTarget(null, descriptorResult(), model)); + } + + @Test + void pathTraversalArtifactIdThrows() { + Model model = modelWithRelocation("org.apache", "../../../etc/passwd", "1.0"); + assertThrows(IllegalArgumentException.class, () -> source.relocatedTarget(null, descriptorResult(), model)); + } + + @Test + void pathTraversalVersionThrows() { + Model model = modelWithRelocation("org.apache", "artifact", "../../evil"); + assertThrows(IllegalArgumentException.class, () -> source.relocatedTarget(null, descriptorResult(), model)); + } + + @Test + void colonInGroupIdThrows() { + Model model = modelWithRelocation("C:", "artifact", "1.0"); + assertThrows(IllegalArgumentException.class, () -> source.relocatedTarget(null, descriptorResult(), model)); + } + + @Test + void emptyRelocationFieldsAreAccepted() { + // Empty fields mean "keep the original coordinate" + Model model = modelWithRelocation("", "", ""); + Artifact result = source.relocatedTarget(null, descriptorResult(), model); + assertNotNull(result); + } + + private static Model modelWithRelocation(String groupId, String artifactId, String version) { + return Model.newBuilder() + .distributionManagement(DistributionManagement.newBuilder() + .relocation(Relocation.newBuilder() + .groupId(groupId) + .artifactId(artifactId) + .version(version) + .build()) + .build()) + .build(); + } + + private static ArtifactDescriptorResult descriptorResult() { + Artifact artifact = new DefaultArtifact("org.example", "old-artifact", "jar", "1.0"); + ArtifactDescriptorRequest request = new ArtifactDescriptorRequest(); + request.setArtifact(artifact); + return new ArtifactDescriptorResult(request); + } +} From b2433840f09a87c991644bfe9b6b5ca06eff6ff8 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 1 Sep 2026 15:29:09 +0200 Subject: [PATCH 7/7] Address review: fix @since tag, simplify validateVersionToken, replace NUL bytes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Fix @since 4.1.0 → 4.0.0-rc-7 (correct for the maven-4.0.x branch) - validateVersionToken now delegates to isInvalidCoordinateComponent instead of reimplementing the same logic - Replace raw NUL bytes in MetadataInputValidatorTest with \0 escape sequences so git treats the file as text and diffs are visible Co-Authored-By: Claude Opus 4.6 --- .../impl/resolver/MetadataInputValidator.java | 11 ++--------- .../resolver/MetadataInputValidatorTest.java | Bin 5876 -> 5878 bytes 2 files changed, 2 insertions(+), 9 deletions(-) diff --git a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java index 0f6d431d2aa6..25fc2facd652 100644 --- a/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java +++ b/impl/maven-impl/src/main/java/org/apache/maven/impl/resolver/MetadataInputValidator.java @@ -33,7 +33,7 @@ * would map onto filesystem path-traversal segments ({@code ..}), separators ({@code /}, {@code \}), * drive-letter delimiters ({@code :}), or ISO control characters are rejected. * - * @since 4.1.0 + * @since 4.0.0 */ public final class MetadataInputValidator { @@ -88,14 +88,7 @@ public static void validateVersioning(Versioning versioning) throws IOException * @throws IOException if the token contains path-traversal sequences, separators, or control characters */ public static void validateVersionToken(String value, String description) throws IOException { - if (value == null || value.isEmpty()) { - return; - } - boolean invalid = "..".equals(value) || value.contains("/") || value.contains("\\") || value.contains(":"); - for (int i = 0; i < value.length() && !invalid; i++) { - invalid = Character.isISOControl(value.charAt(i)); - } - if (invalid) { + if (isInvalidCoordinateComponent(value)) { throw new IOException("Rejecting metadata with invalid " + description + " '" + value + "': must not contain '..', '/', '\\', ':' or control characters"); } diff --git a/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/MetadataInputValidatorTest.java b/impl/maven-impl/src/test/java/org/apache/maven/impl/resolver/MetadataInputValidatorTest.java index 198af1c0f097cc5fbf0e080ac33a65f5e1b06b73..0cf168684921f489b6faa10d871e387144c2cd31 100644 GIT binary patch delta 42 ycmeyO`%QPlLN3-AgOt>?$s4(aC-ZY!ZT`gdg_$YFV6qLL^kf!pvCR_vch~_}rVdB| delta 40 wcmeyS`$c!dLM|4Dl+?7ziaf%T`8lmNf9Cqa%*Zg=mQQ*zE4SEYDgHa`03g2%qyPW_