From edca350ed539bef142ced8ab211965b535448257 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 19:41:06 +0200 Subject: [PATCH 1/6] 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 a7d052455ad7..8bc6ad71fc04 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 @@ -273,7 +273,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) { @@ -282,6 +286,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 9b3b7b646a1658cd97d1ac57fce199ce2afa9b26 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 17:52:44 +0200 Subject: [PATCH 2/6] 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 8ac81cd7e6c2607a574645e9d65a521cbd9cdc31 Mon Sep 17 00:00:00 2001 From: Sylwester Lachiewicz Date: Sun, 30 Aug 2026 22:26:37 +0200 Subject: [PATCH 3/6] 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 6ff4b8afa4dcc6d30412378a6e6a15f646d76413 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Mon, 31 Aug 2026 21:50:26 +0200 Subject: [PATCH 4/6] Honour configured checksum policy on the legacy compat paths The legacy metadata manager and repository system hardcoded CHECKSUM_POLICY_WARN, silently ignoring the operator's --strict-checksums / -C flag and per-repository checksumPolicy settings. This brings the master branch in line with the maven-4.0.x fix (PR #12945): - Catch ChecksumFailedException in resolve() and propagate it as a RepositoryMetadataResolutionException under CHECKSUM_POLICY_FAIL - Move updateCheckManager.touch() out of the finally block so that failed transfers do not suppress retries for a full update interval - Use the repository's configured checksum policy for deployment metadata retrieval instead of hardcoded WARN - Pick the stricter of release/snapshot policies in LegacyRepositorySystem.retrieve() instead of hardcoded WARN Co-Authored-By: Claude Opus 4.6 --- .../DefaultRepositoryMetadataManager.java | 32 +++++++- .../legacy/LegacyRepositorySystem.java | 35 +++++++- .../DefaultRepositoryMetadataManagerTest.java | 81 +++++++++++++++++++ .../legacy/LegacyRepositorySystemTest.java | 25 ++++++ 4 files changed, 170 insertions(+), 3 deletions(-) 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 8bc6ad71fc04..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 @@ -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); } } @@ -404,7 +416,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() @@ -430,6 +442,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/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)); + } +} 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 5e1e17ee609c5d253af86615d2ffcb1965aee980 Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 1 Sep 2026 07:40:31 +0200 Subject: [PATCH 5/6] Validate metadata inputs and relocation coordinates in the new API resolvers Add MetadataInputValidator, a shared utility in impl/maven-impl that rejects version tokens, snapshot timestamps, and relocation coordinates containing path-traversal sequences (..), separators (/, \, :), or ISO control characters. Wire the validator into the new-API implementations: - DefaultVersionResolver.readVersions() validates parsed Versioning - DefaultVersionRangeResolver.readVersions() validates parsed Versioning - DistributionManagementArtifactRelocationSource validates relocation groupId, artifactId, and version before applying This mirrors the compat-layer guards already present in this PR and eliminates the gap where maven-metadata.xml content bypasses the model validator entirely. 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 6d62d5b8880855ac9ebffd8f04e69841194f20df Mon Sep 17 00:00:00 2001 From: Guillaume Nodet Date: Tue, 1 Sep 2026 15:29:06 +0200 Subject: [PATCH 6/6] Address review: simplify validateVersionToken, replace NUL bytes in test - 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 | 9 +-------- .../resolver/MetadataInputValidatorTest.java | Bin 5876 -> 5878 bytes 2 files changed, 1 insertion(+), 8 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..ae0b1c4ea066 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 @@ -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_