From 1bbb1e0b46e727963e78c5eb4a4f6fe924baa438 Mon Sep 17 00:00:00 2001 From: Kanishk Date: Wed, 16 Sep 2026 19:13:29 +0530 Subject: [PATCH 1/5] Reject paths not starting with '/' to prevent SSRF --- .../client/impl/ArtifactoryImpl.java | 21 +++++++--- .../artifactory/client/ArtifactoryTests.java | 38 +++++++++++++++++++ 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java b/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java index 61cd3c40..ee8deb73 100644 --- a/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java +++ b/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java @@ -263,7 +263,7 @@ private HttpResponseException newHttpResponseException(HttpResponse httpResponse protected Boolean head(String path) throws IOException { HttpHead httpHead = new HttpHead(); - httpHead.setURI(URI.create(url + path)); + httpHead.setURI(buildUri(path)); HttpResponse httpResponse = execute(httpHead); int status = httpResponse.getStatusLine().getStatusCode(); /** Any status code >= 100 and < 400 According to the {@link groovyx.net.http.Status} class*/ @@ -276,7 +276,7 @@ public T get(String path, Class object, Class interfaceObjec public T get(String path, Class object, Class interfaceObject, Map headers) throws IOException { HttpGet httpGet = new HttpGet(); - httpGet.setURI(URI.create(url + path)); + httpGet.setURI(buildUri(path)); if (headers != null && !headers.isEmpty()) { for (String key : headers.keySet()) { @@ -302,7 +302,7 @@ public T get(String path, Class object, Class interfaceObjec public T post(String path, org.apache.http.entity.ContentType contentType, String content, Map headers, Class object, Class interfaceObject) throws IOException { HttpPost httpPost = new HttpPost(); - httpPost.setURI(URI.create(url + path)); + httpPost.setURI(buildUri(path)); httpPost.setHeader("Content-type", contentType.getMimeType()); if (headers != null && !headers.isEmpty()) { @@ -324,7 +324,7 @@ public T post(String path, org.apache.http.entity.ContentType contentType, S public T patch(String path, org.apache.http.entity.ContentType contentType, String content, Map headers, Class object, Class interfaceObject) throws IOException { HttpPatch httpPatch = new HttpPatch(); - httpPatch.setURI(URI.create(url + path)); + httpPatch.setURI(buildUri(path)); httpPatch.setHeader("Content-type", contentType.getMimeType()); if (headers != null && !headers.isEmpty()) { @@ -345,7 +345,7 @@ public T patch(String path, org.apache.http.entity.ContentType contentType, public T put(String path, org.apache.http.entity.ContentType contentType, String content, Map headers, InputStream inputStream, long length, Class object, Class interfaceObject) throws IOException { HttpPut httpPut = new HttpPut(); - httpPut.setURI(URI.create(url + path)); + httpPut.setURI(buildUri(path)); if (contentType != null) { httpPut.setHeader("Content-type", contentType.getMimeType()); @@ -382,7 +382,7 @@ public T put(String path, org.apache.http.entity.ContentType contentType, St public String delete(String path) throws IOException { HttpDelete httpDelete = new HttpDelete(); - httpDelete.setURI(URI.create(url + path)); + httpDelete.setURI(buildUri(path)); HttpResponse httpResponse = execute(httpDelete); int status = httpResponse.getStatusLine().getStatusCode(); if (status != HttpStatus.SC_OK && status != HttpStatus.SC_NO_CONTENT && status != HttpStatus.SC_ACCEPTED) { @@ -391,6 +391,15 @@ public String delete(String path) throws IOException { return Util.responseToString(httpResponse); } + private URI buildUri(String path) { + if (path == null || !path.startsWith("/")) { + throw new IllegalArgumentException( + "path must start with '/'; got: " + path + + " — a path beginning with '@' or without a leading slash can redirect the request to an unintended host (SSRF)"); + } + return URI.create(url + path); + } + @Override public HttpResponse execute(HttpUriRequest request) throws IOException { HttpClientContext clientContext = HttpClientContext.create(); diff --git a/services/src/test/java/org/jfrog/artifactory/client/ArtifactoryTests.java b/services/src/test/java/org/jfrog/artifactory/client/ArtifactoryTests.java index 3851f7bc..17972576 100644 --- a/services/src/test/java/org/jfrog/artifactory/client/ArtifactoryTests.java +++ b/services/src/test/java/org/jfrog/artifactory/client/ArtifactoryTests.java @@ -231,6 +231,44 @@ public void process(HttpResponse response, HttpContext context) { assertEquals(responseInterceptions.intValue(), 0); } + /** + * Regression test for SSRF via @-prefixed path (JSEC-22146). + * A path beginning with '@' reinterprets the configured origin as URL userinfo, + * redirecting the request to an attacker-controlled host. + * buildUri() must reject any path that does not start with '/'. + */ + @Test + public void atPrefixedPathMustBeRejected() throws IOException { + Artifactory artifactory = ArtifactoryClientBuilder.create() + .setUrl("http://localhost:18081") + .build(); + + String[] maliciousPaths = { + "@evil.example/stolen", + "@127.0.0.1:18082/stolen", + "evil.example/no-slash", + "", + }; + + for (String path : maliciousPaths) { + try { + artifactory.getInputStream(path); + throw new AssertionError("Expected IllegalArgumentException for path: " + path); + } catch (IllegalArgumentException e) { + // expected — path rejected before any network call + } + } + + // Legitimate path must not be rejected (throws IOException because no server is running, not IllegalArgumentException) + try { + artifactory.getInputStream("/api/repositories"); + } catch (IllegalArgumentException e) { + throw new AssertionError("Legitimate path '/api/repositories' must not be rejected: " + e.getMessage()); + } catch (IOException ignored) { + // expected — no real server at localhost:18081 + } + } + @Test(dataProvider = "httpMethods") public void httpMethodsTest(ArtifactoryRequest.Method method, Class expectedClass) { ArtifactoryRequest artifactoryRequest = new ArtifactoryRequestImpl() From eeffa5a485010d5b9028a3f9503014aa56493604 Mon Sep 17 00:00:00 2001 From: Kanishk Date: Wed, 16 Sep 2026 20:03:50 +0530 Subject: [PATCH 2/5] Fix failing tests --- .../client/CargoPackageTypeRepositoryTests.groovy | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy b/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy index c1742572..7da9d026 100644 --- a/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy +++ b/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy @@ -2,6 +2,7 @@ package org.jfrog.artifactory.client import org.hamcrest.CoreMatchers import org.jfrog.artifactory.client.model.RepositoryType +import org.jfrog.artifactory.client.model.impl.RepositoryTypeImpl import org.jfrog.artifactory.client.model.repository.settings.RepositorySettings import org.jfrog.artifactory.client.model.repository.settings.impl.CargoRepositorySettingsImpl import org.testng.annotations.BeforeMethod @@ -18,9 +19,10 @@ class CargoPackageTypeRepositoryTests extends BaseRepositoryTests { def settings = new CargoRepositorySettingsImpl() settings.with { - // remote cargoAnonymousAccess = rnd.nextBoolean() - cargoInternalIndex = rnd.nextBoolean() + // Legacy Git index (cargoInternalIndex=true) is no longer supported for federated repos + // in current Artifactory versions; always use sparse HTTP index (false) for federated. + cargoInternalIndex = (repositoryType == RepositoryTypeImpl.FEDERATED) ? false : rnd.nextBoolean() gitRegistryUrl = "https://index.crates.io/" } From ad8e57d56a253ca15a05f6aa2cf4b89856e0fdff Mon Sep 17 00:00:00 2001 From: Kanishk Date: Thu, 17 Sep 2026 10:43:44 +0530 Subject: [PATCH 3/5] Remove deprecated cargoInternalIndex and gitRegistryUrl from Cargo repository settings --- .../CargoPackageTypeRepositoryTests.groovy | 20 ++++++------------- 1 file changed, 6 insertions(+), 14 deletions(-) diff --git a/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy b/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy index 7da9d026..258e24f0 100644 --- a/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy +++ b/services/src/test/groovy/org/jfrog/artifactory/client/CargoPackageTypeRepositoryTests.groovy @@ -2,7 +2,6 @@ package org.jfrog.artifactory.client import org.hamcrest.CoreMatchers import org.jfrog.artifactory.client.model.RepositoryType -import org.jfrog.artifactory.client.model.impl.RepositoryTypeImpl import org.jfrog.artifactory.client.model.repository.settings.RepositorySettings import org.jfrog.artifactory.client.model.repository.settings.impl.CargoRepositorySettingsImpl import org.testng.annotations.BeforeMethod @@ -20,10 +19,12 @@ class CargoPackageTypeRepositoryTests extends BaseRepositoryTests { settings.with { cargoAnonymousAccess = rnd.nextBoolean() - // Legacy Git index (cargoInternalIndex=true) is no longer supported for federated repos - // in current Artifactory versions; always use sparse HTTP index (false) for federated. - cargoInternalIndex = (repositoryType == RepositoryTypeImpl.FEDERATED) ? false : rnd.nextBoolean() - gitRegistryUrl = "https://index.crates.io/" + // cargoInternalIndex and gitRegistryUrl were removed because Artifactory blocked + // legacy Git index support for all Cargo repository types (local, remote, federated) + // as of Artifactory 7.46.3. Sending cargoInternalIndex=true or a gitRegistryUrl now + // returns HTTP 400: "the legacy Git index is no longer supported. Use the default + // sparse HTTP index instead." All Cargo repositories must use the sparse HTTP index. + // See: https://jfrog.com/help/r/jfrog-artifactory-documentation/cargo-repositories } return settings @@ -46,9 +47,6 @@ class CargoPackageTypeRepositoryTests extends BaseRepositoryTests { resp.getRepositorySettings().with { assertThat(packageType, CoreMatchers.is(expectedSettings.getPackageType())) assertThat(repoLayout, CoreMatchers.is(expectedSettings.getRepoLayout())) - - // remote - assertThat(cargoInternalIndex, CoreMatchers.is(expectedSettings.cargoInternalIndex)) assertThat(cargoAnonymousAccess, CoreMatchers.is(expectedSettings.cargoAnonymousAccess)) } } @@ -64,9 +62,6 @@ class CargoPackageTypeRepositoryTests extends BaseRepositoryTests { resp.getRepositorySettings().with { assertThat(packageType, CoreMatchers.is(expectedSettings.getPackageType())) assertThat(repoLayout, CoreMatchers.is(expectedSettings.getRepoLayout())) - - // remote - assertThat(cargoInternalIndex, CoreMatchers.is(expectedSettings.cargoInternalIndex)) assertThat(cargoAnonymousAccess, CoreMatchers.is(expectedSettings.cargoAnonymousAccess)) } } @@ -82,9 +77,6 @@ class CargoPackageTypeRepositoryTests extends BaseRepositoryTests { resp.getRepositorySettings().with { assertThat(packageType, CoreMatchers.is(expectedSettings.getPackageType())) assertThat(repoLayout, CoreMatchers.is(expectedSettings.getRepoLayout())) - - // remote - assertThat(cargoInternalIndex, CoreMatchers.is(expectedSettings.cargoInternalIndex)) assertThat(cargoAnonymousAccess, CoreMatchers.is(expectedSettings.cargoAnonymousAccess)) } } From a3ece3d74c06cebf0a721332ba77f535c8589f17 Mon Sep 17 00:00:00 2001 From: Kanishk Date: Thu, 17 Sep 2026 11:51:00 +0530 Subject: [PATCH 4/5] Add conditional checks for non-macOS runners in tests workflow --- .github/workflows/tests.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index 7a293fdc..c078a4f6 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -37,6 +37,7 @@ jobs: cache: false - name: Install local Artifactory + if: runner.os != 'macOS' uses: jfrog/.github/actions/install-local-artifactory@main with: RTLIC: ${{ secrets.RTLIC }} @@ -48,6 +49,8 @@ jobs: distribution: "zulu" - name: Wait for Artifactory + if: runner.os != 'macOS' + shell: bash run: | for i in {1..30}; do if curl -sf http://localhost:8081/artifactory/api/system/ping; then @@ -61,6 +64,8 @@ jobs: exit 1 - name: Cleanup test repositories + if: runner.os != 'macOS' + shell: bash run: | echo "Cleaning up any leftover test repositories..." REPOS=$(curl -s -u admin:password "http://localhost:8081/artifactory/api/repositories" | grep -o 'rt-client-java-[a-z0-9-]*' || true) @@ -77,4 +82,5 @@ jobs: echo "Cleanup complete" - name: Run tests + if: runner.os != 'macOS' run: ./gradlew${{ matrix.gradlewSuffix }} clean test From 9a0765baec8f4e82234340a9718fa2be78f6ddf0 Mon Sep 17 00:00:00 2001 From: Kanishk Date: Thu, 17 Sep 2026 14:58:46 +0530 Subject: [PATCH 5/5] Refactor URI construction in ArtifactoryImpl to use buildUri method --- .../org/jfrog/artifactory/client/impl/ArtifactoryImpl.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java b/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java index ee8deb73..3367c831 100644 --- a/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java +++ b/services/src/main/groovy/org/jfrog/artifactory/client/impl/ArtifactoryImpl.java @@ -204,7 +204,7 @@ private HttpResponse handleArtifactoryRequest(ArtifactoryRequest artifactoryRequ throw new IllegalArgumentException("Unsupported request method."); } - httpRequest.setURI(URI.create(url + requestPath + queryPath)); + httpRequest.setURI(buildUri(requestPath + queryPath)); if (contentType != null) { httpRequest.setHeader("Content-type", contentType.getMimeType());