From ab85ad461b2ee8c9f4cec046ef1af271062d9bf0 Mon Sep 17 00:00:00 2001 From: Ihor Vinokur Date: Tue, 1 Sep 2026 14:45:25 +0300 Subject: [PATCH 1/4] fix(security): prevent SSRF by validating URL schemes in file content providers Restrict URL fetching to http/https schemes only, rejecting file://, ftp://, jar:, and other schemes that could be exploited to read local files or access cloud metadata endpoints. Also fix incorrect null-check in ScmService that validated `repository` twice instead of `filePath`. Co-Authored-By: Claude Opus 4.6 --- ...verAuthorizingFileContentProviderTest.java | 16 ++- ...ketAuthorizingFileContentProviderTest.java | 16 ++- ...hubAuthorizingFileContentProviderTest.java | 16 ++- ...labAuthorizingFileContentProviderTest.java | 13 ++- .../che/api/factory/server/ScmService.java | 4 +- .../scm/AuthorizingFileContentProvider.java | 9 +- ...thorizingFactoryParameterResolverTest.java | 42 +++++++- .../workspace/server/devfile/URLFetcher.java | 10 +- .../devfile/URLFileContentProvider.java | 7 +- .../server/devfile/URLFetcherTest.java | 101 ++++++++++-------- .../devfile/URLFileContentProviderTest.java | 44 +++++++- 11 files changed, 215 insertions(+), 63 deletions(-) diff --git a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFileContentProviderTest.java index 439569ed44e..6f3b84f7910 100644 --- a/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-bitbucket-server/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketServerAuthorizingFileContentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2024 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -20,6 +20,7 @@ import org.eclipse.che.api.factory.server.scm.PersonalAccessToken; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.DataProvider; @@ -123,4 +124,17 @@ public static Object[][] relativePathsProvider() { } }; } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeUrl() throws Exception { + BitbucketServerUrl url = + new BitbucketServerUrl().withHostName(TEST_HOSTNAME).withScheme(TEST_SCHEME); + BitbucketServerAuthorizingFileContentProvider fileContentProvider = + new BitbucketServerAuthorizingFileContentProvider( + url, urlFetcher, personalAccessTokenManager); + + fileContentProvider.fetchContent("file:///var/run/secrets/kubernetes.io/serviceaccount/token"); + } } diff --git a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java index 8eb74db1a50..344c4245b3c 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2024 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -22,6 +22,7 @@ import org.eclipse.che.api.factory.server.scm.exception.UnknownScmProviderException; import org.eclipse.che.api.workspace.server.devfile.FileContentProvider; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.mockito.Mock; import org.mockito.Mockito; import org.mockito.testng.MockitoTestNGListener; @@ -99,4 +100,17 @@ public void shouldFetchContent() throws Exception { // then assertEquals(content, "content"); } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeUrl() throws Exception { + URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); + BitbucketUrl bitbucketUrl = new BitbucketUrl().withWorkspaceId("eclipse").withRepository("che"); + FileContentProvider fileContentProvider = + new BitbucketAuthorizingFileContentProvider( + bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient); + + fileContentProvider.fetchContent("file:///etc/passwd"); + } } diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java index 00967226c60..46617371972 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2024 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -133,10 +133,10 @@ public void shouldThrowDevfileException() throws Exception { fileContentProvider.fetchContent(url); } - @Test - public void shouldNotAskGitHubAPIForDifferentDomain() throws Exception { - String raw_url = "https://ghserver.com/foo/bar/branch-name/devfile.yaml"; - + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeUrl() throws Exception { URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); GithubUrl githubUrl = new GithubUrl("github") @@ -145,11 +145,7 @@ public void shouldNotAskGitHubAPIForDifferentDomain() throws Exception { .withServerUrl("https://github.com"); FileContentProvider fileContentProvider = new GithubAuthorizingFileContentProvider(githubUrl, urlFetcher, personalAccessTokenManager); - var personalAccessToken = new PersonalAccessToken(raw_url, "provider", "che", "my-token"); - when(personalAccessTokenManager.getAndStore(anyString())).thenReturn(personalAccessToken); - fileContentProvider.fetchContent(raw_url); - - verify(urlFetcher).fetch(eq(raw_url), eq("token my-token")); + fileContentProvider.fetchContent("file:///etc/passwd"); } } diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java index 8749e03251d..f6e686dfd0e 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabAuthorizingFileContentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2024 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -143,4 +143,15 @@ public void shouldThrowDevfileException() throws Exception { // when fileContentProvider.fetchContent("devfile.yaml"); } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeUrl() throws Exception { + GitlabUrl gitlabUrl = new GitlabUrl().withHostName("gitlab.net").withSubGroups("eclipse/che"); + FileContentProvider fileContentProvider = + new GitlabAuthorizingFileContentProvider(gitlabUrl, urlFetcher, personalAccessTokenManager); + + fileContentProvider.fetchContent("file:///etc/passwd"); + } } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/ScmService.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/ScmService.java index 0bf49a1da6d..ba85e894fe0 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/ScmService.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/ScmService.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -53,7 +53,7 @@ public Response resolveFile( @Parameter(description = "File name or path") @QueryParam("file") String filePath) throws ApiException { requireNonNull(repository, "Repository"); - requireNonNull(repository, "File"); + requireNonNull(filePath, "File"); String content = getScmFileResolver(repository).fileContent(repository, filePath); return Response.ok().entity(content).build(); } diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java index d3ce5ef6e5a..873612d87f1 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java @@ -163,7 +163,14 @@ protected boolean isPublicRepository(T remoteFactoryUrl) { protected String formatUrl(String fileURL) throws DevfileException { String requestURL; try { - if (new URI(fileURL).isAbsolute()) { + URI fileURI = new URI(fileURL); + if (fileURI.isAbsolute()) { + String scheme = fileURI.getScheme(); + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + throw new DevfileException( + String.format( + "URL '%s' is not allowed: only http and https schemes are permitted", fileURL)); + } requestURL = fileURL; } else { // since files retrieved via REST, we cannot use path like '.' or one that starts with './' diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java index 4986a640ddc..7ca3a356b7b 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2023 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -20,6 +20,7 @@ import org.eclipse.che.api.factory.server.urlfactory.RemoteFactoryUrl; import org.eclipse.che.api.workspace.server.devfile.URLFetcher; +import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.mockito.Mock; import org.mockito.testng.MockitoTestNGListener; import org.testng.annotations.BeforeMethod; @@ -78,4 +79,43 @@ public void shouldOmitDotInTheResourceName() throws Exception { public void shouldKeepResourceNameUnchanged() throws Exception { assertEquals(provider.formatUrl(".gitconfig"), ".gitconfig"); } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeUrl() throws Exception { + provider.formatUrl("file:///etc/passwd"); + } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFtpSchemeUrl() throws Exception { + provider.formatUrl("ftp://evil.com/secret"); + } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectJarSchemeUrl() throws Exception { + provider.formatUrl("jar:file:///tmp/evil.jar!/payload"); + } + + @Test + public void shouldStillResolveRelativePaths() throws Exception { + when(remoteFactoryUrl.rawFileLocation("devfile.yaml")).thenReturn("resolved-url"); + + String result = provider.formatUrl("devfile.yaml"); + + assertEquals(result, "resolved-url"); + } + + @Test + public void shouldStillResolveRelativePathsWithDotSlash() throws Exception { + when(remoteFactoryUrl.rawFileLocation("subdir/file.yaml")).thenReturn("resolved-subdir-url"); + + String result = provider.formatUrl("./subdir/file.yaml"); + + assertEquals(result, "resolved-subdir-url"); + } } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java index 982f7cf5452..4cc00f9d5a6 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2025 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -128,7 +128,13 @@ String fetch(@NotNull final String url, int timeout) throws IOException { String fetch(@NotNull final String url, int timeout, @Nullable String authorization) throws IOException { requireNonNull(url, "url parameter can't be null"); - URLConnection connection = new URL(sanitized(url)).openConnection(); + URL parsedUrl = new URL(sanitized(url)); + String scheme = parsedUrl.getProtocol(); + if (!"http".equals(scheme) && !"https".equals(scheme)) { + throw new IOException( + "Only http and https URLs are allowed, got: " + scheme + " in URL " + url); + } + URLConnection connection = parsedUrl.openConnection(); connection.setConnectTimeout(timeout); connection.setReadTimeout(timeout); if (!isNullOrEmpty(authorization)) { diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java index 0dadfe54c9e..83e2f86c985 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2023 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -46,6 +46,11 @@ private String fetchContentInternal(String fileURL, @Nullable String credentials } if (fileURI.isAbsolute()) { + String scheme = fileURI.getScheme(); + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + throw new DevfileException( + format("URL '%s' is not allowed: only http and https schemes are permitted", fileURL)); + } requestURL = fileURL; } else { if (devfileLocation == null) { diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java index 7f4c7896ea5..cf2e1c517b9 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2021 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -27,7 +27,6 @@ import java.util.function.Consumer; import org.mockito.Mockito; import org.mockito.testng.MockitoTestNGListener; -import org.testng.Assert; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -50,13 +49,11 @@ public void checkNullURL() { /** Check that when url exists the content is retrieved */ @Test - public void checkGetContent() { - - // test to download this class object - URL urlJson = getClass().getClassLoader().getResource("devfile/url_fetcher_test_resource.json"); - Assert.assertNotNull(urlJson); - - String content = urlFetcher.fetchSafely(urlJson.toString()); + public void checkGetContent() throws IOException { + URLConnection urlConnection = Mockito.mock(URLConnection.class); + when(urlConnection.getInputStream()) + .thenReturn(new ByteArrayInputStream("Hello".getBytes(UTF_8))); + String content = urlFetcher.fetch(urlConnection); assertEquals(content, "Hello"); } @@ -76,53 +73,73 @@ public void checkUnsafeGetUrlFileIsInvalid() throws Exception { assertNull(result); } - /** Check Sanitizing of Git URL works */ - @Test - public void checkDotGitRemovedFromURL() { - String result = urlFetcher.sanitized("https://github.com/acme/demo.git"); - assertEquals("https://github.com/acme/demo", result); - - result = urlFetcher.sanitized("http://github.com/acme/demo.git"); - assertEquals("http://github.com/acme/demo", result); + /** Check that non-http schemes are rejected */ + @Test( + expectedExceptions = IOException.class, + expectedExceptionsMessageRegExp = "Only http and https URLs are allowed.*") + public void checkFileSchemeIsRejected() throws Exception { + urlFetcher.fetch("file:///etc/passwd"); } - /** Check that when url doesn't exist */ + /** Check that non-http schemes are rejected via fetchSafely */ @Test - public void checkMissingContent() { - - // test to download this class object - URL urlJson = getClass().getClassLoader().getResource("devfile/url_fetcher_test_resource.json"); - Assert.assertNotNull(urlJson); + public void checkFileSchemeIsRejectedSafely() { + String result = urlFetcher.fetchSafely("file:///etc/passwd"); + assertNull(result); + } - // add extra path to make url not found - String content = urlFetcher.fetchSafely(urlJson.toString() + "-invalid"); - assertNull(content); + /** Check that non-http schemes are rejected */ + @Test( + expectedExceptions = IOException.class, + expectedExceptionsMessageRegExp = "Only http and https URLs are allowed.*") + public void checkFtpSchemeIsRejected() throws Exception { + urlFetcher.fetch("ftp://evil.com/file"); } - /** Check that when url doesn't exist */ + /** Check that non-http schemes are rejected */ @Test( expectedExceptions = IOException.class, - expectedExceptionsMessageRegExp = - ".*url_fetcher_test_resource.json-invalid \\(No such file or directory\\)") - public void checkMissingContentUnsafeGet() throws Exception { + expectedExceptionsMessageRegExp = "Only http and https URLs are allowed.*") + public void checkJarSchemeIsRejected() throws Exception { + urlFetcher.fetch("jar:file:///tmp/evil.jar!/payload"); + } + + /** Check that http scheme is allowed */ + @Test + public void checkHttpSchemeIsAllowed() throws IOException { + URLFetcher fetcher = + new TimeoutCheckURLFetcher( + timeout -> assertEquals(timeout.intValue(), CONNECTION_READ_TIMEOUT)); + fetcher.fetch("http://example.com/devfile.yaml"); + } - // test to download this class object - URL urlJson = getClass().getClassLoader().getResource("devfile/url_fetcher_test_resource.json"); - Assert.assertNotNull(urlJson); + /** Check that https scheme is allowed */ + @Test + public void checkHttpsSchemeIsAllowed() throws IOException { + URLFetcher fetcher = + new TimeoutCheckURLFetcher( + timeout -> assertEquals(timeout.intValue(), CONNECTION_READ_TIMEOUT)); + fetcher.fetch("https://example.com/devfile.yaml"); + } - // add extra path to make url not found - String content = urlFetcher.fetch(urlJson.toString() + "-invalid"); - assertNull(content); + /** Check Sanitizing of Git URL works */ + @Test + public void checkDotGitRemovedFromURL() { + String result = urlFetcher.sanitized("https://github.com/acme/demo.git"); + assertEquals("https://github.com/acme/demo", result); + + result = urlFetcher.sanitized("http://github.com/acme/demo.git"); + assertEquals("http://github.com/acme/demo", result); } /** Check when we reach custom limit */ @Test - public void checkPartialContent() { - URL urlJson = getClass().getClassLoader().getResource("devfile/url_fetcher_test_resource.json"); - Assert.assertNotNull(urlJson); - - String content = new OneByteURLFetcher(1).fetchSafely(urlJson.toString()); - assertEquals(content, "Hello".substring(0, 1)); + public void checkPartialContent() throws IOException { + URLConnection urlConnection = Mockito.mock(URLConnection.class); + when(urlConnection.getInputStream()) + .thenReturn(new ByteArrayInputStream("Hello".getBytes(UTF_8))); + String content = new OneByteURLFetcher(1).fetch(urlConnection); + assertEquals(content, "H"); } /** Check when we reach custom limit */ diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java index f067c3ed43c..4620a277a12 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2023 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -47,6 +47,48 @@ public void shouldFetchByAbsoluteURL() throws Exception { assertEquals(captor.getValue(), url); } + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeURL() throws Exception { + URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); + provider.fetchContent("file:///etc/passwd"); + } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFtpSchemeURL() throws Exception { + URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); + provider.fetchContent("ftp://evil.com/file"); + } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectJarSchemeURL() throws Exception { + URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); + provider.fetchContent("jar:file:///tmp/evil.jar!/payload"); + } + + @Test( + expectedExceptions = DevfileException.class, + expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") + public void shouldRejectFileSchemeViaFetchWithoutAuthentication() throws Exception { + URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); + provider.fetchContentWithoutAuthentication("file:///etc/passwd"); + } + + @Test + public void shouldAllowHttpsAbsoluteURL() throws Exception { + String url = "https://secure.example.com/devfile.yaml"; + URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); + ArgumentCaptor captor = ArgumentCaptor.forClass(String.class); + provider.fetchContent(url); + verify(urlFetcher).fetch(captor.capture(), eq(null)); + assertEquals(captor.getValue(), url); + } + @Test public void shouldMergeDevfileLocationAndRelativeURL() throws Exception { String devfileUrl = "http://myhost.com/relative/devile.yaml"; From c3a741e4cdbe8e738aa140d6857906ce017a3790 Mon Sep 17 00:00:00 2001 From: Ihor Vinokur Date: Tue, 8 Sep 2026 15:23:42 +0300 Subject: [PATCH 2/4] Potential fix for pull request finding 'CodeQL / Server-side request forgery' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../workspace/server/devfile/URLFetcher.java | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java index 4cc00f9d5a6..5d2a22ad568 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java @@ -23,8 +23,12 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; import java.net.URL; import java.net.URLConnection; +import java.net.UnknownHostException; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -134,6 +138,7 @@ String fetch(@NotNull final String url, int timeout, @Nullable String authorizat throw new IOException( "Only http and https URLs are allowed, got: " + scheme + " in URL " + url); } + validateUrlTarget(parsedUrl, url); URLConnection connection = parsedUrl.openConnection(); connection.setConnectTimeout(timeout); connection.setReadTimeout(timeout); @@ -177,6 +182,34 @@ protected long getLimit() { return maximumReadBytes; } + private void validateUrlTarget(URL parsedUrl, String originalUrl) throws IOException { + final String host; + try { + host = new URI(parsedUrl.toString()).getHost(); + } catch (URISyntaxException e) { + throw new IOException("Invalid URL " + originalUrl, e); + } + + if (isNullOrEmpty(host)) { + throw new IOException("URL host is missing in " + originalUrl); + } + + final InetAddress address; + try { + address = InetAddress.getByName(host); + } catch (UnknownHostException e) { + throw new IOException("Unable to resolve URL host " + host + " in " + originalUrl, e); + } + + if (address.isAnyLocalAddress() + || address.isLoopbackAddress() + || address.isLinkLocalAddress() + || address.isSiteLocalAddress() + || address.isMulticastAddress()) { + throw new IOException("URL host is not allowed: " + host); + } + } + /** * Simple method to sanitize the Git urls like "https://github.com/demo.git" or * "http://myowngit.example.com/demo.git" From 08d0971c3899a5a253765dc529291b7d8a4f6c6b Mon Sep 17 00:00:00 2001 From: Ihor Vinokur Date: Wed, 9 Sep 2026 12:48:44 +0300 Subject: [PATCH 3/4] fix(security): send SCM credentials only to the provider's own hosts A devfile can reference an absolute URL on an arbitrary host. Both file content providers attached the caller's credentials to such requests regardless of the destination, disclosing the user's personal access token, or the Basic credentials taken from the devfile URL, to any host a devfile chose to reference. Credentials are now only sent to hosts belonging to the SCM provider they were issued for. The allowlist is derived from the provider URL, the host name and the raw file location, as the latter is not necessarily the same host (github.com vs raw.githubusercontent.com, or the raw. subdomain of a GitHub Enterprise server). Requests to any other host fall back to an anonymous fetch, so devfiles referencing public content elsewhere keep working. Bitbucket Cloud additionally trusts api.bitbucket.org, which its API client legitimately authenticates against, and applies the same check to the fetchContent override that passes the token to that client. Co-Authored-By: Claude Opus 5 --- ...tbucketAuthorizingFileContentProvider.java | 14 +++ ...ketAuthorizingFileContentProviderTest.java | 19 ++++ ...hubAuthorizingFileContentProviderTest.java | 46 ++++++++ .../scm/AuthorizingFileContentProvider.java | 106 +++++++++++++++--- ...thorizingFactoryParameterResolverTest.java | 18 ++- .../devfile/URLFileContentProvider.java | 33 +++++- .../devfile/URLFileContentProviderTest.java | 43 +++++++ 7 files changed, 261 insertions(+), 18 deletions(-) diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java index bdee138fcc9..942c24105ab 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java @@ -13,6 +13,9 @@ import java.io.FileNotFoundException; import java.io.IOException; +import java.net.URI; +import java.util.HashSet; +import java.util.Set; import org.eclipse.che.api.factory.server.scm.AuthorizingFileContentProvider; import org.eclipse.che.api.factory.server.scm.PersonalAccessToken; import org.eclipse.che.api.factory.server.scm.PersonalAccessTokenManager; @@ -46,9 +49,20 @@ protected String formatAuthorization(String token, boolean isPAT) { return "Bearer " + token; } + /** Along with the Bitbucket host itself, the token is also valid for the Bitbucket API host. */ + @Override + protected Set getTrustedHosts() { + Set trustedHosts = new HashSet<>(super.getTrustedHosts()); + trustedHosts.add(URI.create(BitbucketApiClient.BITBUCKET_API_SERVER).getHost()); + return trustedHosts; + } + @Override public String fetchContent(String fileURL) throws IOException, DevfileException { final String requestURL = formatUrl(fileURL); + if (!canSendCredentialsTo(requestURL)) { + return fetchContentWithoutToken(requestURL); + } try { // try to authenticate for the given URL PersonalAccessToken token = diff --git a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java index 344c4245b3c..2c3b8d91498 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/test/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProviderTest.java @@ -12,7 +12,9 @@ package org.eclipse.che.api.factory.server.bitbucket; import static org.mockito.ArgumentMatchers.*; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.verifyNoInteractions; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; @@ -101,6 +103,23 @@ public void shouldFetchContent() throws Exception { assertEquals(content, "content"); } + @Test + public void shouldNotSendTokenToForeignHost() throws Exception { + URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); + String foreignUrl = "https://attacker.example/collect"; + BitbucketUrl bitbucketUrl = + new BitbucketUrl().withUsername("eclipse").withWorkspaceId("eclipse").withRepository("che"); + FileContentProvider fileContentProvider = + new BitbucketAuthorizingFileContentProvider( + bitbucketUrl, urlFetcher, personalAccessTokenManager, bitbucketApiClient); + + fileContentProvider.fetchContent(foreignUrl); + + verify(urlFetcher).fetch(eq(foreignUrl)); + verifyNoInteractions(bitbucketApiClient); + verify(personalAccessTokenManager, never()).getAndStore(anyString()); + } + @Test( expectedExceptions = DevfileException.class, expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java index 46617371972..9c9d7613260 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java @@ -13,6 +13,7 @@ import static org.mockito.ArgumentMatchers.*; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -133,6 +134,51 @@ public void shouldThrowDevfileException() throws Exception { fileContentProvider.fetchContent(url); } + @Test + public void shouldNotSendTokenToForeignHost() throws Exception { + String foreignUrl = "https://attacker.example/collect"; + + GithubUrl githubUrl = + new GithubUrl("github") + .withUsername("eclipse") + .withRepository("che") + .withBranch("main") + .withServerUrl("https://github.com"); + + URLFetcher urlFetcher = mock(URLFetcher.class); + FileContentProvider fileContentProvider = + new GithubAuthorizingFileContentProvider(githubUrl, urlFetcher, personalAccessTokenManager); + + fileContentProvider.fetchContent(foreignUrl); + + verify(urlFetcher).fetch(eq(foreignUrl)); + verify(urlFetcher, never()).fetch(anyString(), anyString()); + verify(personalAccessTokenManager, never()).getAndStore(anyString()); + } + + @Test + public void shouldSendTokenToRawContentHostOfGithubEnterprise() throws Exception { + String rawUrl = "https://raw.ghe.example.com/eclipse/che/main/devfile.yaml"; + + GithubUrl githubUrl = + new GithubUrl("github") + .withUsername("eclipse") + .withRepository("che") + .withBranch("main") + .withServerUrl("https://ghe.example.com"); + + URLFetcher urlFetcher = mock(URLFetcher.class); + FileContentProvider fileContentProvider = + new GithubAuthorizingFileContentProvider(githubUrl, urlFetcher, personalAccessTokenManager); + + when(personalAccessTokenManager.getAndStore(anyString())) + .thenReturn(new PersonalAccessToken(rawUrl, "provider", "che", "my-token")); + + fileContentProvider.fetchContent(rawUrl); + + verify(urlFetcher).fetch(eq(rawUrl), eq("token my-token")); + } + @Test( expectedExceptions = DevfileException.class, expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java index 873612d87f1..b0f25b252bc 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java @@ -20,6 +20,10 @@ import java.net.URI; import java.net.URISyntaxException; import java.util.Base64; +import java.util.HashSet; +import java.util.Locale; +import java.util.Optional; +import java.util.Set; import javax.net.ssl.SSLException; import org.eclipse.che.api.factory.server.scm.exception.ScmCommunicationException; import org.eclipse.che.api.factory.server.scm.exception.ScmConfigurationPersistenceException; @@ -43,6 +47,9 @@ public class AuthorizingFileContentProvider private static final Logger LOG = LoggerFactory.getLogger(AuthorizingFileContentProvider.class); + /** Placeholder file name used to derive the host that serves the raw repository content. */ + private static final String RAW_CONTENT_PROBE_FILE = "devfile.yaml"; + protected final T remoteFactoryUrl; protected final PersonalAccessTokenManager personalAccessTokenManager; protected final URLFetcher urlFetcher; @@ -77,25 +84,24 @@ private String fetchContent( String fileURL, boolean skipAuthentication, @Nullable String credentials) throws IOException, DevfileException { final String requestURL = formatUrl(fileURL); + if (skipAuthentication || !canSendCredentialsTo(requestURL)) { + return urlFetcher.fetch(requestURL); + } try { - if (skipAuthentication) { - return urlFetcher.fetch(requestURL); + // try to authenticate for the given URL + String authorization; + if (isNullOrEmpty(credentials)) { + PersonalAccessToken token = + personalAccessTokenManager.getAndStore(remoteFactoryUrl.getProviderUrl()); + authorization = + formatAuthorization( + token.getToken(), + token.getScmTokenName() == null + || !token.getScmTokenName().startsWith(OAUTH_2_PREFIX)); } else { - // try to authenticate for the given URL - String authorization; - if (isNullOrEmpty(credentials)) { - PersonalAccessToken token = - personalAccessTokenManager.getAndStore(remoteFactoryUrl.getProviderUrl()); - authorization = - formatAuthorization( - token.getToken(), - token.getScmTokenName() == null - || !token.getScmTokenName().startsWith(OAUTH_2_PREFIX)); - } else { - authorization = getCredentialsAuthorization(credentials); - } - return urlFetcher.fetch(requestURL, authorization); + authorization = getCredentialsAuthorization(credentials); } + return urlFetcher.fetch(requestURL, authorization); } catch (UnknownScmProviderException | ScmConfigurationPersistenceException | UnsatisfiedScmPreconditionException e) { @@ -160,6 +166,74 @@ protected boolean isPublicRepository(T remoteFactoryUrl) { return false; } + /** + * Tells whether the user's credentials may be sent to the given URL. A devfile can reference an + * absolute URL on an arbitrary host, and attaching the personal access token to such a request + * would disclose it to that host, so credentials are only sent to the SCM provider they were + * issued for. + * + * @param requestURL the URL about to be fetched + * @return true if the URL belongs to this provider, false if it must be fetched anonymously + */ + protected boolean canSendCredentialsTo(String requestURL) { + Set trustedHosts = getTrustedHosts(); + Optional host = hostOfUrl(requestURL); + if (host.isPresent() && trustedHosts.contains(host.get())) { + return true; + } + LOG.warn( + "Fetching a file from host '{}' without credentials: it is not one of the {} provider" + + " hosts {}.", + host.orElse(""), + remoteFactoryUrl.getProviderName(), + trustedHosts); + return false; + } + + /** + * Returns the hosts allowed to receive the user's credentials. Besides the SCM provider host + * itself, it holds the host serving the raw repository content, as the two are not necessarily + * the same (e.g. {@code github.com} and {@code raw.githubusercontent.com}). + */ + protected Set getTrustedHosts() { + Set trustedHosts = new HashSet<>(); + hostOfUrlOrHostName(remoteFactoryUrl.getProviderUrl()).ifPresent(trustedHosts::add); + hostOfUrlOrHostName(remoteFactoryUrl.getHostName()).ifPresent(trustedHosts::add); + try { + hostOfUrl(remoteFactoryUrl.rawFileLocation(RAW_CONTENT_PROBE_FILE)) + .ifPresent(trustedHosts::add); + } catch (RuntimeException e) { + LOG.debug( + "Unable to resolve the raw content host of {}", remoteFactoryUrl.getProviderUrl(), e); + } + return trustedHosts; + } + + /** Extracts the host of an absolute URL, empty if it is not one. */ + private static Optional hostOfUrl(String url) { + return isNullOrEmpty(url) || !url.contains("://") ? Optional.empty() : parseHost(url); + } + + /** + * Extracts the host of a value that {@link RemoteFactoryUrl} implementations return either as a + * bare host name or as a full URL. + */ + private static Optional hostOfUrlOrHostName(String urlOrHostName) { + if (isNullOrEmpty(urlOrHostName)) { + return Optional.empty(); + } + return parseHost(urlOrHostName.contains("://") ? urlOrHostName : "https://" + urlOrHostName); + } + + private static Optional parseHost(String url) { + try { + String host = new URI(url).getHost(); + return isNullOrEmpty(host) ? Optional.empty() : Optional.of(host.toLowerCase(Locale.ROOT)); + } catch (URISyntaxException e) { + return Optional.empty(); + } + } + protected String formatUrl(String fileURL) throws DevfileException { String requestURL; try { diff --git a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java index 7ca3a356b7b..01e2a62019f 100644 --- a/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java +++ b/wsmaster/che-core-api-factory/src/test/java/org/eclipse/che/api/factory/server/scm/AuthorizingFactoryParameterResolverTest.java @@ -52,12 +52,28 @@ public void shouldFetchContentWithAuthentication() throws Exception { when(personalAccessTokenManager.getAndStore(anyString())).thenReturn(personalAccessToken); // when - provider.fetchContent("url"); + provider.fetchContent("https://provider.url/devfile.yaml"); // then verify(personalAccessTokenManager).getAndStore(anyString()); } + @Test + public void shouldNotSendCredentialsToAForeignHost() throws Exception { + // given + String foreignUrl = "https://attacker.example/collect"; + when(remoteFactoryUrl.getProviderUrl()).thenReturn("https://provider.url"); + when(urlFetcher.fetch(anyString())).thenReturn("content"); + + // when + provider.fetchContent(foreignUrl); + + // then + verify(personalAccessTokenManager, never()).getAndStore(anyString()); + verify(urlFetcher).fetch(foreignUrl); + verify(urlFetcher, never()).fetch(anyString(), anyString()); + } + @Test public void shouldFetchContentWithoutAuthentication() throws Exception { // given diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java index 83e2f86c985..e4eb9aca234 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java @@ -20,6 +20,8 @@ import java.util.Base64; import org.eclipse.che.api.workspace.server.devfile.exception.DevfileException; import org.eclipse.che.commons.annotation.Nullable; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * A simple implementation of the FileContentProvider that merely uses the function resolve relative @@ -27,6 +29,8 @@ */ public class URLFileContentProvider implements FileContentProvider { + private static final Logger LOG = LoggerFactory.getLogger(URLFileContentProvider.class); + private final URI devfileLocation; private final URLFetcher urlFetcher; @@ -63,8 +67,9 @@ private String fetchContentInternal(String fileURL, @Nullable String credentials requestURL = devfileLocation.resolve(fileURI).toString(); } try { + boolean authorize = !isNullOrEmpty(credentials) && canSendCredentialsTo(requestURL); return urlFetcher.fetch( - requestURL, isNullOrEmpty(credentials) ? null : getCredentialsAuthorization(credentials)); + requestURL, authorize ? getCredentialsAuthorization(credentials) : null); } catch (IOException e) { throw new IOException( format( @@ -80,6 +85,32 @@ private String fetchContentInternal(String fileURL, @Nullable String credentials } } + /** + * Tells whether the credentials taken from the devfile URL may be sent to the given URL. A + * devfile can reference an absolute URL on an arbitrary host, and attaching the credentials to + * such a request would disclose them to that host, so they are only sent back to the host the + * devfile itself was loaded from. + */ + private boolean canSendCredentialsTo(String requestURL) { + if (devfileLocation == null) { + return false; + } + final String host; + try { + host = new URI(requestURL).getHost(); + } catch (URISyntaxException e) { + return false; + } + if (host != null && host.equalsIgnoreCase(devfileLocation.getHost())) { + return true; + } + LOG.warn( + "Fetching a file from host '{}' without credentials: the devfile was loaded from '{}'.", + host, + devfileLocation.getHost()); + return false; + } + private String getCredentialsAuthorization(String credentials) { return "Basic " + new String(Base64.getEncoder().encode(credentials.getBytes())); } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java index 4620a277a12..774eebda897 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java @@ -89,6 +89,49 @@ public void shouldAllowHttpsAbsoluteURL() throws Exception { assertEquals(captor.getValue(), url); } + @Test + public void shouldSendCredentialsToTheDevfileHost() throws Exception { + String devfileUrl = "https://myhost.com/relative/devfile.yaml"; + String url = "https://myhost.com/relative/dependent.yaml"; + URLFileContentProvider provider = new URLFileContentProvider(new URI(devfileUrl), urlFetcher); + + provider.fetchContent(url, "user:pass"); + + verify(urlFetcher).fetch(eq(url), eq("Basic dXNlcjpwYXNz")); + } + + @Test + public void shouldSendCredentialsForRelativeURL() throws Exception { + String devfileUrl = "https://myhost.com/relative/devfile.yaml"; + URLFileContentProvider provider = new URLFileContentProvider(new URI(devfileUrl), urlFetcher); + + provider.fetchContent("dependent.yaml", "user:pass"); + + verify(urlFetcher) + .fetch(eq("https://myhost.com/relative/dependent.yaml"), eq("Basic dXNlcjpwYXNz")); + } + + @Test + public void shouldNotSendCredentialsToAForeignHost() throws Exception { + String devfileUrl = "https://myhost.com/relative/devfile.yaml"; + String foreignUrl = "https://attacker.example/collect"; + URLFileContentProvider provider = new URLFileContentProvider(new URI(devfileUrl), urlFetcher); + + provider.fetchContent(foreignUrl, "user:pass"); + + verify(urlFetcher).fetch(eq(foreignUrl), eq(null)); + } + + @Test + public void shouldNotSendCredentialsWhenTheDevfileLocationIsUnknown() throws Exception { + String url = "https://myhost.com/relative/devfile.yaml"; + URLFileContentProvider provider = new URLFileContentProvider(null, urlFetcher); + + provider.fetchContent(url, "user:pass"); + + verify(urlFetcher).fetch(eq(url), eq(null)); + } + @Test public void shouldMergeDevfileLocationAndRelativeURL() throws Exception { String devfileUrl = "http://myhost.com/relative/devile.yaml"; From 06c9aaca2d209216bbf85b0b549a3a62ccdb2ca3 Mon Sep 17 00:00:00 2001 From: Ihor Vinokur Date: Wed, 9 Sep 2026 13:42:18 +0300 Subject: [PATCH 4/4] fix(security): validate every SSRF-reachable request target (CWE-918) The URL a caller hands to POST /factory/resolver ends up being requested by the server. The existing check ran once, on the URL as given, and HttpURLConnection then followed redirects on its own - so a 302 to 169.254.169.254 or to a neighbouring pod was never checked. The check also missed several address ranges that are not the public internet. Add UrlTargetValidator to che-core-commons-lang as the one place that decides whether a target may be requested: http/https only, host required, and every address the host resolves to must be publicly routable. On top of the ranges the previous check covered, this rejects fc00::/7 (which InetAddress#isSiteLocalAddress does not cover), 100.64.0.0/10, 192.0.0.0/24, 198.18.0.0/15 and 240.0.0.0/4. URLFetcher no longer lets HttpURLConnection follow redirects. It follows them itself, up to five hops, validating each one, refusing an https to http downgrade, and dropping the Authorization header on a cross-origin hop. The GitHub and GitLab URL parsers probe an unknown host to find out whether it is an SCM server, which reports the outcome back through the factory endpoint. Only publicly routable hosts are probed now. A private-network SCM server is still reached through the configured provider endpoints or a personal access token, both of which are checked before the probe. Credential allowlisting moves from bare host to origin, so a devfile can no longer make the server send a token or basic credentials over plaintext http to a host it normally reaches over https. On-prem providers configured with http keep working, since their configured origin carries that scheme. Co-Authored-By: Claude Opus 5 --- .../che/commons/lang/UrlTargetValidator.java | 137 +++++++++++++++ .../commons/lang/UrlTargetValidatorTest.java | 77 +++++++++ ...tbucketAuthorizingFileContentProvider.java | 11 +- .../github/AbstractGithubURLParser.java | 19 +++ ...hubAuthorizingFileContentProviderTest.java | 22 +++ .../server/github/GithubURLParserTest.java | 28 ++++ .../gitlab/AbstractGitlabUrlParser.java | 26 ++- .../gitlab/GitlabCustomPortUrlParserTest.java | 27 ++- .../server/gitlab/GitlabUrlParserTest.java | 23 +++ .../scm/AuthorizingFileContentProvider.java | 66 ++++---- .../workspace/server/devfile/URLFetcher.java | 146 +++++++++++----- .../devfile/URLFileContentProvider.java | 39 ++++- .../server/devfile/URLFetcherTest.java | 156 ++++++++++++++++++ .../devfile/URLFileContentProviderTest.java | 11 ++ 14 files changed, 699 insertions(+), 89 deletions(-) create mode 100644 core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/UrlTargetValidator.java create mode 100644 core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/UrlTargetValidatorTest.java diff --git a/core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/UrlTargetValidator.java b/core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/UrlTargetValidator.java new file mode 100644 index 00000000000..94ab4b85173 --- /dev/null +++ b/core/commons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/UrlTargetValidator.java @@ -0,0 +1,137 @@ +/* + * Copyright (c) 2012-2026 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.commons.lang; + +import static com.google.common.base.Strings.isNullOrEmpty; + +import java.io.IOException; +import java.net.InetAddress; +import java.net.URI; +import java.net.URISyntaxException; +import java.net.UnknownHostException; + +/** + * Checks that a URL derived from user input points at a target the server is allowed to reach. + * + *

Several endpoints take a URL from the user and make the server fetch it. Without a check on + * the destination, such a URL can be aimed at a service that is only reachable from inside the + * cluster - the cloud metadata endpoint, the Kubernetes API, a neighbouring pod - turning the + * server into a proxy for the caller (SSRF). + */ +public final class UrlTargetValidator { + + private UrlTargetValidator() {} + + /** + * Throws if the given URL may not be requested by the server, either because of its scheme or + * because its host resolves to an address that is not publicly routable. + * + * @param url the URL about to be requested + * @throws IOException if the URL is malformed or its target is not allowed + */ + public static void validate(String url) throws IOException { + // the scheme is read off the raw string: an opaque URL such as jar:file:/x!/y is rejected here + // rather than reported as a parsing failure + int schemeEnd = url == null ? -1 : url.indexOf(':'); + String scheme = schemeEnd > 0 ? url.substring(0, schemeEnd) : null; + if (!"http".equalsIgnoreCase(scheme) && !"https".equalsIgnoreCase(scheme)) { + throw new IOException( + "Only http and https URLs are allowed, got: " + scheme + " in URL " + url); + } + + final URI uri; + try { + uri = new URI(url); + } catch (URISyntaxException e) { + throw new IOException("Invalid URL " + url, e); + } + + String host = uri.getHost(); + if (isNullOrEmpty(host)) { + throw new IOException("URL host is missing in " + url); + } + + final InetAddress[] addresses; + try { + // all records are checked, so that a host publishing both a public and an internal address + // cannot pass the check and then be connected to on the internal one + addresses = InetAddress.getAllByName(host); + } catch (UnknownHostException e) { + throw new IOException("Unable to resolve URL host " + host + " in " + url, e); + } + + for (InetAddress address : addresses) { + if (!isPubliclyRoutable(address)) { + throw new IOException("URL host is not allowed: " + host); + } + } + } + + /** + * Same check as {@link #validate(String)}, in a form usable where a URL is probed on a best + * effort basis and a disallowed target simply means "not a match". + * + * @param url the URL about to be requested + * @return true if the server may request the URL + */ + public static boolean isAllowed(String url) { + try { + validate(url); + return true; + } catch (IOException e) { + return false; + } + } + + /** + * Tells whether an address belongs to the public internet, as opposed to the ranges reserved for + * private networks, the host itself, or protocol machinery. + */ + private static boolean isPubliclyRoutable(InetAddress address) { + if (address.isAnyLocalAddress() + || address.isLoopbackAddress() + || address.isLinkLocalAddress() + // IPv4 private ranges and the deprecated IPv6 site-local fec0::/10 + || address.isSiteLocalAddress() + || address.isMulticastAddress()) { + return false; + } + + byte[] bytes = address.getAddress(); + if (bytes.length == 4) { + int first = bytes[0] & 0xFF; + int second = bytes[1] & 0xFF; + // 100.64.0.0/10 shared address space (RFC 6598), used by several CNI plugins + if (first == 100 && second >= 64 && second <= 127) { + return false; + } + // 192.0.0.0/24 IETF protocol assignments (RFC 6890) + if (first == 192 && second == 0 && (bytes[2] & 0xFF) == 0) { + return false; + } + // 198.18.0.0/15 benchmarking (RFC 2544) + if (first == 198 && (second == 18 || second == 19)) { + return false; + } + // 240.0.0.0/4 reserved, including the 255.255.255.255 broadcast address + if (first >= 240) { + return false; + } + } else if (bytes.length == 16) { + // fc00::/7 unique local addresses (RFC 4193), which isSiteLocalAddress does not cover + if ((bytes[0] & 0xFE) == 0xFC) { + return false; + } + } + return true; + } +} diff --git a/core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/UrlTargetValidatorTest.java b/core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/UrlTargetValidatorTest.java new file mode 100644 index 00000000000..bcd4fe114fd --- /dev/null +++ b/core/commons/che-core-commons-lang/src/test/java/org/eclipse/che/commons/lang/UrlTargetValidatorTest.java @@ -0,0 +1,77 @@ +/* + * Copyright (c) 2012-2026 Red Hat, Inc. + * This program and the accompanying materials are made + * available under the terms of the Eclipse Public License 2.0 + * which is available at https://www.eclipse.org/legal/epl-2.0/ + * + * SPDX-License-Identifier: EPL-2.0 + * + * Contributors: + * Red Hat, Inc. - initial API and implementation + */ +package org.eclipse.che.commons.lang; + +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; + +import org.testng.annotations.DataProvider; +import org.testng.annotations.Test; + +/** Tests of {@link UrlTargetValidator}. */ +public class UrlTargetValidatorTest { + + @DataProvider + public Object[][] disallowedUrls() { + return new Object[][] { + // schemes that are not a web request at all + {"file:///etc/passwd"}, + {"ftp://example.com/file"}, + {"jar:file:///tmp/evil.jar!/payload"}, + {"gopher://example.com/"}, + {"no-scheme-at-all"}, + // the host itself + {"http://127.0.0.1/"}, + {"http://[::1]/"}, + {"http://0/"}, + {"http://[::ffff:127.0.0.1]/"}, + // cloud metadata, reachable on the link-local range + {"http://169.254.169.254/latest/meta-data/"}, + {"http://[fe80::1]/"}, + {"http://[0:0:0:0:0:ffff:a9fe:a9fe]/"}, + // IPv4 private ranges + {"http://10.0.0.1/"}, + {"http://172.16.0.1/"}, + {"http://192.168.1.1/"}, + // IPv6 unique local addresses, which InetAddress#isSiteLocalAddress does not cover + {"http://[fc00::1]/"}, + {"http://[fd12:3456:789a::1]/"}, + // ranges that are not the public internet either + {"http://100.64.1.1/"}, + {"http://192.0.0.1/"}, + {"http://198.18.0.1/"}, + {"http://240.0.0.1/"}, + {"http://255.255.255.255/"}, + // no host to check + {"http:///path"}, + }; + } + + @Test(dataProvider = "disallowedUrls") + public void shouldRejectUrl(String url) { + assertFalse(UrlTargetValidator.isAllowed(url), url + " should not be reachable"); + } + + @DataProvider + public Object[][] allowedUrls() { + return new Object[][] { + {"https://93.184.216.34/devfile.yaml"}, + {"http://8.8.8.8/"}, + {"https://[2001:4860:4860::8888]/"}, + }; + } + + @Test(dataProvider = "allowedUrls") + public void shouldAllowUrl(String url) { + assertTrue(UrlTargetValidator.isAllowed(url), url + " should be reachable"); + } +} diff --git a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java index 942c24105ab..85a2378abd3 100644 --- a/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory-bitbucket/src/main/java/org/eclipse/che/api/factory/server/bitbucket/BitbucketAuthorizingFileContentProvider.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2023 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -13,7 +13,6 @@ import java.io.FileNotFoundException; import java.io.IOException; -import java.net.URI; import java.util.HashSet; import java.util.Set; import org.eclipse.che.api.factory.server.scm.AuthorizingFileContentProvider; @@ -51,10 +50,10 @@ protected String formatAuthorization(String token, boolean isPAT) { /** Along with the Bitbucket host itself, the token is also valid for the Bitbucket API host. */ @Override - protected Set getTrustedHosts() { - Set trustedHosts = new HashSet<>(super.getTrustedHosts()); - trustedHosts.add(URI.create(BitbucketApiClient.BITBUCKET_API_SERVER).getHost()); - return trustedHosts; + protected Set getTrustedOrigins() { + Set trustedOrigins = new HashSet<>(super.getTrustedOrigins()); + originOfUrl(BitbucketApiClient.BITBUCKET_API_SERVER).ifPresent(trustedOrigins::add); + return trustedOrigins; } @Override diff --git a/wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java b/wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java index 703c05fcc8b..54ec9102804 100644 --- a/wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java +++ b/wsmaster/che-core-api-factory-github-common/src/main/java/org/eclipse/che/api/factory/server/github/AbstractGithubURLParser.java @@ -19,6 +19,7 @@ import static org.eclipse.che.api.factory.server.github.GithubApiClient.GITHUB_SAAS_ENDPOINT; import static org.eclipse.che.commons.lang.StringUtils.trimEnd; +import com.google.common.annotations.VisibleForTesting; import jakarta.validation.constraints.NotNull; import java.net.URI; import java.net.URISyntaxException; @@ -36,6 +37,7 @@ import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.lang.UrlTargetValidator; import org.eclipse.che.commons.subject.Subject; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -133,11 +135,28 @@ private boolean isUserTokenPresent(String repositoryUrl) { return false; } + /** + * Tells whether a URL that is not known to belong to any configured provider may nonetheless be + * probed. Such a URL comes straight from the caller, so probing it unconditionally would let + * anyone use the server to reach services only it can see and read the outcome off the answer the + * factory endpoint returns, which is why only publicly routable hosts are probed. An SCM server + * on a private network is reached through the configured provider endpoints or a personal access + * token, both of which are checked before it comes to this. + */ + @VisibleForTesting + boolean canProbe(String serverUrl) { + return UrlTargetValidator.isAllowed(serverUrl); + } + // Try to call an API request to see if the given url matches self-hosted GitHub Enterprise. private boolean isApiRequestRelevant(String repositoryUrl) { Optional serverUrlOptional = getServerUrl(repositoryUrl); if (serverUrlOptional.isPresent()) { String serverUrl = serverUrlOptional.get(); + if (!canProbe(serverUrl)) { + LOG.warn("Not probing {}: it does not point to a publicly routable host.", serverUrl); + return false; + } GithubApiClient githubApiClient = new GithubApiClient(serverUrl); try { // If the user request catches the unauthorised error, it means that the provided url diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java index 9c9d7613260..a2c689ffad4 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubAuthorizingFileContentProviderTest.java @@ -179,6 +179,28 @@ public void shouldSendTokenToRawContentHostOfGithubEnterprise() throws Exception verify(urlFetcher).fetch(eq(rawUrl), eq("token my-token")); } + @Test + public void shouldNotSendTokenOverPlainHttpToATrustedHost() throws Exception { + String plaintextUrl = "http://raw.githubusercontent.com/eclipse/che/main/devfile.yaml"; + + GithubUrl githubUrl = + new GithubUrl("github") + .withUsername("eclipse") + .withRepository("che") + .withBranch("main") + .withServerUrl("https://github.com"); + + URLFetcher urlFetcher = mock(URLFetcher.class); + FileContentProvider fileContentProvider = + new GithubAuthorizingFileContentProvider(githubUrl, urlFetcher, personalAccessTokenManager); + + fileContentProvider.fetchContent(plaintextUrl); + + verify(urlFetcher).fetch(eq(plaintextUrl)); + verify(urlFetcher, never()).fetch(anyString(), anyString()); + verify(personalAccessTokenManager, never()).getAndStore(anyString()); + } + @Test( expectedExceptions = DevfileException.class, expectedExceptionsMessageRegExp = ".*only http and https schemes are permitted.*") diff --git a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java index d6cea0951b8..e6a510e8fca 100644 --- a/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java +++ b/wsmaster/che-core-api-factory-github/src/test/java/org/eclipse/che/api/factory/server/github/GithubURLParserTest.java @@ -27,6 +27,7 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; import static org.testng.Assert.assertTrue; import com.github.tomakehurst.wiremock.WireMockServer; @@ -549,6 +550,7 @@ public void shouldParseServerUrWithPullRequestId() throws Exception { @Test public void shouldValidateOldVersionGitHubServerUrl() throws Exception { // given + githubUrlParser = probingLoopbackParser(); Field endpoint = AbstractGithubURLParser.class.getDeclaredField("endpoint"); endpoint.setAccessible(true); endpoint.set(githubUrlParser, wireMockServer.baseUrl()); @@ -570,6 +572,7 @@ public void shouldValidateOldVersionGitHubServerUrl() throws Exception { @Test public void shouldValidateGitHubServerUrl() throws Exception { // given + githubUrlParser = probingLoopbackParser(); Field endpoint = AbstractGithubURLParser.class.getDeclaredField("endpoint"); endpoint.setAccessible(true); endpoint.set(githubUrlParser, wireMockServer.baseUrl()); @@ -596,4 +599,29 @@ public void shouldNotRequestGitHubSAASUrl() throws Exception { // then verify(githubApiClient, never()).getUser(anyString()); } + + /** + * An unconfigured URL is probed to find out whether it is a GitHub server, which must not become + * a way of having the server reach whatever the caller names. + */ + @Test + public void shouldNotProbeAPrivateAddress() throws Exception { + // when + boolean valid = githubUrlParser.isValid("http://10.0.0.1/user/repo"); + + // then + assertFalse(valid); + verify(githubApiClient, never()).getUser(anyString()); + } + + /** The wiremock server stands in for a GitHub server, but is only reachable over loopback. */ + private GithubURLParser probingLoopbackParser() { + return new GithubURLParser( + personalAccessTokenManager, devfileFilenamesProvider, githubApiClient, null, false) { + @Override + boolean canProbe(String serverUrl) { + return true; + } + }; + } } diff --git a/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/AbstractGitlabUrlParser.java b/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/AbstractGitlabUrlParser.java index 74ad4b18e2a..85f1aa8a722 100644 --- a/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/AbstractGitlabUrlParser.java +++ b/wsmaster/che-core-api-factory-gitlab-common/src/main/java/org/eclipse/che/api/factory/server/gitlab/AbstractGitlabUrlParser.java @@ -16,6 +16,7 @@ import static java.util.regex.Pattern.compile; import static org.eclipse.che.commons.lang.StringUtils.trimEnd; +import com.google.common.annotations.VisibleForTesting; import com.google.gson.JsonObject; import com.google.gson.JsonParser; import jakarta.validation.constraints.NotNull; @@ -35,6 +36,9 @@ import org.eclipse.che.api.factory.server.urlfactory.DevfileFilenamesProvider; import org.eclipse.che.commons.annotation.Nullable; import org.eclipse.che.commons.env.EnvironmentContext; +import org.eclipse.che.commons.lang.UrlTargetValidator; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * Parser of String Gitlab URLs and provide {@link GitlabUrl} objects. @@ -43,6 +47,8 @@ */ public class AbstractGitlabUrlParser { + private static final Logger LOG = LoggerFactory.getLogger(AbstractGitlabUrlParser.class); + private final DevfileFilenamesProvider devfileFilenamesProvider; private final PersonalAccessTokenManager personalAccessTokenManager; private final String providerName; @@ -155,10 +161,28 @@ public boolean isValid(@NotNull String url) { || isApiRequestRelevant(url); } + /** + * Tells whether a URL that is not known to belong to any configured provider may nonetheless be + * probed. Such a URL comes straight from the caller, so probing it unconditionally would let + * anyone use the server to reach services only it can see and read the outcome off the answer the + * factory endpoint returns, which is why only publicly routable hosts are probed. An SCM server + * on a private network is reached through the configured provider endpoints or a personal access + * token, both of which are checked before it comes to this. + */ + @VisibleForTesting + boolean canProbe(String serverUrl) { + return UrlTargetValidator.isAllowed(serverUrl); + } + private boolean isApiRequestRelevant(String repositoryUrl) { Optional serverUrlOptional = getServerUrl(repositoryUrl); if (serverUrlOptional.isPresent()) { - GitlabApiClient gitlabApiClient = new GitlabApiClient(serverUrlOptional.get()); + String serverUrl = serverUrlOptional.get(); + if (!canProbe(serverUrl)) { + LOG.warn("Not probing {}: it does not point to a publicly routable host.", serverUrl); + return false; + } + GitlabApiClient gitlabApiClient = new GitlabApiClient(serverUrl); try { // If the token request catches the unauthorised error, it means that the provided url // belongs to Gitlab. diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabCustomPortUrlParserTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabCustomPortUrlParserTest.java index cd43d712a6e..a94b0867c67 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabCustomPortUrlParserTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabCustomPortUrlParserTest.java @@ -1,5 +1,5 @@ /* - * Copyright (c) 2012-2025 Red Hat, Inc. + * Copyright (c) 2012-2026 Red Hat, Inc. * This program and the accompanying materials are made * available under the terms of the Eclipse Public License 2.0 * which is available at https://www.eclipse.org/legal/epl-2.0/ @@ -98,6 +98,7 @@ public void shouldParseWithoutPredefinedEndpoint( @Test public void shouldValidateUrlByApiRequest() { // given + gitlabUrlParser = probingLoopbackParser(); String url = wireMockServer.url("/user/repo"); stubFor( get(urlEqualTo("/oauth/token/info")) @@ -117,6 +118,7 @@ public void shouldValidateUrlByApiRequest() { @Test public void shouldNotValidateUrlByApiRequestWithPlainStringResponse() { // given + gitlabUrlParser = probingLoopbackParser(); String url = wireMockServer.url("/user/repo"); stubFor( get(urlEqualTo("/oauth/token/info")) @@ -132,6 +134,7 @@ public void shouldNotValidateUrlByApiRequestWithPlainStringResponse() { @Test public void shouldNotValidateUrlByApiRequest() { // given + gitlabUrlParser = probingLoopbackParser(); String url = wireMockServer.url("/user/repo"); stubFor(get(urlEqualTo("/oauth/token/info")).willReturn(aResponse().withStatus(500))); @@ -142,6 +145,28 @@ public void shouldNotValidateUrlByApiRequest() { assertFalse(result); } + @Test + public void shouldNotProbeAPrivateAddress() { + // when + boolean result = gitlabUrlParser.isValid("http://10.0.0.1/user/repo"); + + // then + assertFalse(result); + } + + /** The wiremock server stands in for a GitLab server, but is only reachable over loopback. */ + private GitlabUrlParser probingLoopbackParser() { + return new GitlabUrlParser( + "https://gitlab.custom.com:31280", + devfileFilenamesProvider, + mock(PersonalAccessTokenManager.class)) { + @Override + boolean canProbe(String serverUrl) { + return true; + } + }; + } + @DataProvider(name = "UrlsProvider") public Object[][] urls() { return new Object[][] { diff --git a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParserTest.java b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParserTest.java index 0b9215212a0..e7a13917bd6 100644 --- a/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParserTest.java +++ b/wsmaster/che-core-api-factory-gitlab/src/test/java/org/eclipse/che/api/factory/server/gitlab/GitlabUrlParserTest.java @@ -339,6 +339,7 @@ public void shouldParseWithoutPredefinedEndpoint( @Test public void shouldValidateUrlByApiRequest() { // given + gitlabUrlParser = probingLoopbackParser(); String url = wireMockServer.url("/user/repo"); stubFor( get(urlEqualTo("/oauth/token/info")) @@ -358,6 +359,7 @@ public void shouldValidateUrlByApiRequest() { @Test public void shouldNotValidateUrlByApiRequestWithPlainStringResponse() { // given + gitlabUrlParser = probingLoopbackParser(); String url = wireMockServer.url("/user/repo"); stubFor( get(urlEqualTo("/oauth/token/info")) @@ -373,6 +375,7 @@ public void shouldNotValidateUrlByApiRequestWithPlainStringResponse() { @Test public void shouldNotValidateUrlByApiRequest() { // given + gitlabUrlParser = probingLoopbackParser(); String url = wireMockServer.url("/user/repo"); stubFor(get(urlEqualTo("/oauth/token/info")).willReturn(aResponse().withStatus(500))); @@ -383,6 +386,26 @@ public void shouldNotValidateUrlByApiRequest() { assertFalse(result); } + @Test + public void shouldNotProbeAPrivateAddress() { + // when + boolean result = gitlabUrlParser.isValid("http://10.0.0.1/user/repo"); + + // then + assertFalse(result); + } + + /** The wiremock server stands in for a GitLab server, but is only reachable over loopback. */ + private GitlabUrlParser probingLoopbackParser() { + return new GitlabUrlParser( + "https://gitlab1.com", devfileFilenamesProvider, mock(PersonalAccessTokenManager.class)) { + @Override + boolean canProbe(String serverUrl) { + return true; + } + }; + } + @DataProvider(name = "UrlsProvider") public Object[][] urls() { return new Object[][] { diff --git a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java index b0f25b252bc..2e7041607f3 100644 --- a/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java +++ b/wsmaster/che-core-api-factory/src/main/java/org/eclipse/che/api/factory/server/scm/AuthorizingFileContentProvider.java @@ -170,65 +170,73 @@ protected boolean isPublicRepository(T remoteFactoryUrl) { * Tells whether the user's credentials may be sent to the given URL. A devfile can reference an * absolute URL on an arbitrary host, and attaching the personal access token to such a request * would disclose it to that host, so credentials are only sent to the SCM provider they were - * issued for. + * issued for. The comparison is on the whole origin rather than the host alone, so that a devfile + * cannot downgrade the request to plain http and put the token on the wire in the clear. * * @param requestURL the URL about to be fetched * @return true if the URL belongs to this provider, false if it must be fetched anonymously */ protected boolean canSendCredentialsTo(String requestURL) { - Set trustedHosts = getTrustedHosts(); - Optional host = hostOfUrl(requestURL); - if (host.isPresent() && trustedHosts.contains(host.get())) { + Set trustedOrigins = getTrustedOrigins(); + Optional origin = originOfUrl(requestURL); + if (origin.isPresent() && trustedOrigins.contains(origin.get())) { return true; } LOG.warn( - "Fetching a file from host '{}' without credentials: it is not one of the {} provider" - + " hosts {}.", - host.orElse(""), + "Fetching a file from '{}' without credentials: it is not one of the {} provider" + + " origins {}.", + origin.orElse(""), remoteFactoryUrl.getProviderName(), - trustedHosts); + trustedOrigins); return false; } /** - * Returns the hosts allowed to receive the user's credentials. Besides the SCM provider host - * itself, it holds the host serving the raw repository content, as the two are not necessarily - * the same (e.g. {@code github.com} and {@code raw.githubusercontent.com}). + * Returns the origins ({@code scheme://host[:port]}) allowed to receive the user's credentials. + * Besides the SCM provider itself, it holds the origin serving the raw repository content, as the + * two are not necessarily the same (e.g. {@code github.com} and {@code raw.githubusercontent.com} + * ). */ - protected Set getTrustedHosts() { - Set trustedHosts = new HashSet<>(); - hostOfUrlOrHostName(remoteFactoryUrl.getProviderUrl()).ifPresent(trustedHosts::add); - hostOfUrlOrHostName(remoteFactoryUrl.getHostName()).ifPresent(trustedHosts::add); + protected Set getTrustedOrigins() { + Set trustedOrigins = new HashSet<>(); + originOfUrlOrHostName(remoteFactoryUrl.getProviderUrl()).ifPresent(trustedOrigins::add); + originOfUrlOrHostName(remoteFactoryUrl.getHostName()).ifPresent(trustedOrigins::add); try { - hostOfUrl(remoteFactoryUrl.rawFileLocation(RAW_CONTENT_PROBE_FILE)) - .ifPresent(trustedHosts::add); + originOfUrl(remoteFactoryUrl.rawFileLocation(RAW_CONTENT_PROBE_FILE)) + .ifPresent(trustedOrigins::add); } catch (RuntimeException e) { LOG.debug( - "Unable to resolve the raw content host of {}", remoteFactoryUrl.getProviderUrl(), e); + "Unable to resolve the raw content origin of {}", remoteFactoryUrl.getProviderUrl(), e); } - return trustedHosts; + return trustedOrigins; } - /** Extracts the host of an absolute URL, empty if it is not one. */ - private static Optional hostOfUrl(String url) { - return isNullOrEmpty(url) || !url.contains("://") ? Optional.empty() : parseHost(url); + /** Extracts the origin of an absolute URL, empty if it is not one. */ + protected static Optional originOfUrl(String url) { + return isNullOrEmpty(url) || !url.contains("://") ? Optional.empty() : parseOrigin(url); } /** - * Extracts the host of a value that {@link RemoteFactoryUrl} implementations return either as a - * bare host name or as a full URL. + * Extracts the origin of a value that {@link RemoteFactoryUrl} implementations return either as a + * bare host name or as a full URL. A bare host name is assumed to be served over https. */ - private static Optional hostOfUrlOrHostName(String urlOrHostName) { + private static Optional originOfUrlOrHostName(String urlOrHostName) { if (isNullOrEmpty(urlOrHostName)) { return Optional.empty(); } - return parseHost(urlOrHostName.contains("://") ? urlOrHostName : "https://" + urlOrHostName); + return parseOrigin(urlOrHostName.contains("://") ? urlOrHostName : "https://" + urlOrHostName); } - private static Optional parseHost(String url) { + private static Optional parseOrigin(String url) { try { - String host = new URI(url).getHost(); - return isNullOrEmpty(host) ? Optional.empty() : Optional.of(host.toLowerCase(Locale.ROOT)); + URI uri = new URI(url); + String scheme = uri.getScheme(); + String host = uri.getHost(); + if (isNullOrEmpty(scheme) || isNullOrEmpty(host)) { + return Optional.empty(); + } + String origin = scheme.toLowerCase(Locale.ROOT) + "://" + host.toLowerCase(Locale.ROOT); + return Optional.of(uri.getPort() == -1 ? origin : origin + ":" + uri.getPort()); } catch (URISyntaxException e) { return Optional.empty(); } diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java index 5d2a22ad568..f4c20204b1f 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java @@ -23,12 +23,11 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; -import java.net.InetAddress; -import java.net.URI; -import java.net.URISyntaxException; +import java.net.HttpURLConnection; import java.net.URL; import java.net.URLConnection; -import java.net.UnknownHostException; +import java.util.Locale; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -36,6 +35,7 @@ import javax.inject.Named; import javax.inject.Singleton; import org.eclipse.che.commons.annotation.Nullable; +import org.eclipse.che.commons.lang.UrlTargetValidator; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -53,6 +53,9 @@ public class URLFetcher { /** timeout when reading */ @VisibleForTesting static final int CONNECTION_READ_TIMEOUT = 10 * 1000; // 10s + /** How many redirects are followed before a request is given up on. */ + @VisibleForTesting static final int MAX_REDIRECTS = 5; + /** The Compiled REGEX PATTERN that can be used for http|https git urls */ final Pattern GIT_HTTP_URL_PATTERN = Pattern.compile("(?^http[s]?://.*)\\.git$"); @@ -132,20 +135,103 @@ String fetch(@NotNull final String url, int timeout) throws IOException { String fetch(@NotNull final String url, int timeout, @Nullable String authorization) throws IOException { requireNonNull(url, "url parameter can't be null"); - URL parsedUrl = new URL(sanitized(url)); - String scheme = parsedUrl.getProtocol(); - if (!"http".equals(scheme) && !"https".equals(scheme)) { - throw new IOException( - "Only http and https URLs are allowed, got: " + scheme + " in URL " + url); + // new URL() first, so that a malformed URL keeps reporting the parsing error it always did + URL currentUrl = new URL(sanitized(url)); + String currentAuthorization = authorization; + + for (int hop = 0; ; hop++) { + // every hop is validated: a redirect is as much under the control of whoever supplied the + // URL as the URL itself, so validating only the first one would leave the check bypassable + validateTarget(currentUrl.toString()); + + URLConnection connection = currentUrl.openConnection(); + connection.setConnectTimeout(timeout); + connection.setReadTimeout(timeout); + if (!isNullOrEmpty(currentAuthorization)) { + connection.setRequestProperty(HttpHeaders.AUTHORIZATION, currentAuthorization); + } + if (!(connection instanceof HttpURLConnection)) { + return fetch(connection); + } + + HttpURLConnection httpConnection = (HttpURLConnection) connection; + httpConnection.setInstanceFollowRedirects(false); + Optional location = redirectLocation(httpConnection); + if (location.isEmpty()) { + return fetch(httpConnection); + } + httpConnection.disconnect(); + + if (hop == MAX_REDIRECTS) { + throw new IOException( + "Too many redirects (more than " + MAX_REDIRECTS + ") while fetching " + url); + } + URL nextUrl = new URL(currentUrl, location.get()); + if (isSchemeDowngrade(currentUrl, nextUrl)) { + throw new IOException( + "Refusing to follow the redirect from " + currentUrl + " to " + nextUrl + " over http"); + } + if (!isSameOrigin(currentUrl, nextUrl)) { + // do not hand the caller's credentials to whoever the redirect points at + currentAuthorization = null; + } + currentUrl = nextUrl; } - validateUrlTarget(parsedUrl, url); - URLConnection connection = parsedUrl.openConnection(); - connection.setConnectTimeout(timeout); - connection.setReadTimeout(timeout); - if (!isNullOrEmpty(authorization)) { - connection.setRequestProperty(HttpHeaders.AUTHORIZATION, authorization); + } + + /** + * Checks that the server is allowed to request the given URL. Called once per hop of a redirect + * chain. + * + * @param url the URL about to be requested + * @throws IOException if the URL may not be requested + */ + @VisibleForTesting + void validateTarget(String url) throws IOException { + UrlTargetValidator.validate(url); + } + + /** + * Issues the request held by the given connection and returns where it redirects to, or an empty + * optional if the response is not a redirect. + * + * @param connection the connection to send the request on + * @return the value of the {@code Location} header of a redirect response + * @throws IOException if the request fails, or if a redirect carries no location + */ + @VisibleForTesting + Optional redirectLocation(HttpURLConnection connection) throws IOException { + int status = connection.getResponseCode(); + if (status != HttpURLConnection.HTTP_MOVED_PERM + && status != HttpURLConnection.HTTP_MOVED_TEMP + && status != HttpURLConnection.HTTP_SEE_OTHER + && status != 307 + && status != 308) { + return Optional.empty(); + } + String location = connection.getHeaderField("Location"); + if (isNullOrEmpty(location)) { + throw new IOException( + "Got a redirect response " + status + " without a location from " + connection.getURL()); } - return fetch(connection); + return Optional.of(location); + } + + private static boolean isSchemeDowngrade(URL from, URL to) { + return "https".equalsIgnoreCase(from.getProtocol()) + && !"https".equalsIgnoreCase(to.getProtocol()); + } + + private static boolean isSameOrigin(URL first, URL second) { + return first.getProtocol().equalsIgnoreCase(second.getProtocol()) + && String.valueOf(first.getHost()) + .toLowerCase(Locale.ROOT) + .equals(String.valueOf(second.getHost()).toLowerCase(Locale.ROOT)) + && effectivePort(first) == effectivePort(second); + } + + private static int effectivePort(URL url) { + return url.getPort() == -1 ? url.getDefaultPort() : url.getPort(); } /** @@ -182,34 +268,6 @@ protected long getLimit() { return maximumReadBytes; } - private void validateUrlTarget(URL parsedUrl, String originalUrl) throws IOException { - final String host; - try { - host = new URI(parsedUrl.toString()).getHost(); - } catch (URISyntaxException e) { - throw new IOException("Invalid URL " + originalUrl, e); - } - - if (isNullOrEmpty(host)) { - throw new IOException("URL host is missing in " + originalUrl); - } - - final InetAddress address; - try { - address = InetAddress.getByName(host); - } catch (UnknownHostException e) { - throw new IOException("Unable to resolve URL host " + host + " in " + originalUrl, e); - } - - if (address.isAnyLocalAddress() - || address.isLoopbackAddress() - || address.isLinkLocalAddress() - || address.isSiteLocalAddress() - || address.isMulticastAddress()) { - throw new IOException("URL host is not allowed: " + host); - } - } - /** * Simple method to sanitize the Git urls like "https://github.com/demo.git" or * "http://myowngit.example.com/demo.git" diff --git a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java index e4eb9aca234..9d8a4f062bd 100644 --- a/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java +++ b/wsmaster/che-core-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProvider.java @@ -88,29 +88,52 @@ private String fetchContentInternal(String fileURL, @Nullable String credentials /** * Tells whether the credentials taken from the devfile URL may be sent to the given URL. A * devfile can reference an absolute URL on an arbitrary host, and attaching the credentials to - * such a request would disclose them to that host, so they are only sent back to the host the - * devfile itself was loaded from. + * such a request would disclose them to that host, so they are only sent back to the origin the + * devfile itself was loaded from. The scheme is part of that comparison, so that a devfile cannot + * downgrade the request to plain http and put the credentials on the wire in the clear. */ private boolean canSendCredentialsTo(String requestURL) { if (devfileLocation == null) { return false; } - final String host; + final URI requestURI; try { - host = new URI(requestURL).getHost(); + requestURI = new URI(requestURL); } catch (URISyntaxException e) { return false; } - if (host != null && host.equalsIgnoreCase(devfileLocation.getHost())) { + if (isSameOrigin(requestURI, devfileLocation)) { return true; } LOG.warn( - "Fetching a file from host '{}' without credentials: the devfile was loaded from '{}'.", - host, - devfileLocation.getHost()); + "Fetching a file from '{}' without credentials: the devfile was loaded from '{}'.", + originOf(requestURI), + originOf(devfileLocation)); return false; } + private static boolean isSameOrigin(URI first, URI second) { + return first.getScheme() != null + && first.getScheme().equalsIgnoreCase(second.getScheme()) + && first.getHost() != null + && first.getHost().equalsIgnoreCase(second.getHost()) + && effectivePort(first) == effectivePort(second); + } + + private static int effectivePort(URI uri) { + if (uri.getPort() != -1) { + return uri.getPort(); + } + return "https".equalsIgnoreCase(uri.getScheme()) ? 443 : 80; + } + + private static String originOf(URI uri) { + return uri.getScheme() + + "://" + + uri.getHost() + + (uri.getPort() == -1 ? "" : ":" + uri.getPort()); + } + private String getCredentialsAuthorization(String credentials) { return "Basic " + new String(Base64.getEncoder().encode(credentials.getBytes())); } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java index cf2e1c517b9..e78cb069646 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFetcherTest.java @@ -18,12 +18,19 @@ import static org.testng.Assert.assertNull; import com.google.common.base.Strings; +import com.sun.net.httpserver.HttpExchange; +import com.sun.net.httpserver.HttpServer; import java.io.ByteArrayInputStream; import java.io.IOException; import java.io.InputStream; +import java.net.HttpURLConnection; +import java.net.InetSocketAddress; import java.net.SocketTimeoutException; import java.net.URL; import java.net.URLConnection; +import java.util.ArrayList; +import java.util.List; +import java.util.Optional; import java.util.function.Consumer; import org.mockito.Mockito; import org.mockito.testng.MockitoTestNGListener; @@ -191,6 +198,149 @@ public InputStream getInputStream() throws IOException { fetcher.fetch(connection); } + /** + * A redirect is as much under the control of whoever supplied the URL as the URL itself, so the + * target check has to be re-run on every hop rather than on the first one only. + */ + @Test + public void checkEveryRedirectHopIsValidated() throws Exception { + try (LocalServers servers = new LocalServers()) { + String target = servers.serveContent("target", "content"); + String entry = servers.redirectTo("entry", target); + + LoopbackURLFetcher fetcher = new LoopbackURLFetcher(); + assertEquals(fetcher.fetch(entry), "content"); + assertEquals(fetcher.validated, List.of(entry, target)); + } + } + + /** The credentials of the caller must not be handed to whoever a redirect points at. */ + @Test + public void checkAuthorizationIsDroppedOnCrossOriginRedirect() throws Exception { + try (LocalServers servers = new LocalServers()) { + String target = servers.echoAuthorization("target"); + String entry = servers.redirectTo("entry", target); + + assertEquals(new LoopbackURLFetcher().fetch(entry, "Bearer secret"), "authorization=null"); + } + } + + /** Within a single origin there is nobody new to disclose the credentials to. */ + @Test + public void checkAuthorizationIsKeptOnSameOriginRedirect() throws Exception { + try (LocalServers servers = new LocalServers()) { + HttpServer server = servers.newServer(); + servers.echoAuthorization(server, "target"); + String entry = servers.redirectTo(server, "entry", "/target"); + + assertEquals( + new LoopbackURLFetcher().fetch(entry, "Bearer secret"), "authorization=Bearer secret"); + } + } + + @Test( + expectedExceptions = IOException.class, + expectedExceptionsMessageRegExp = "Too many redirects.*") + public void checkRedirectLoopIsGivenUpOn() throws Exception { + try (LocalServers servers = new LocalServers()) { + HttpServer server = servers.newServer(); + String entry = servers.redirectTo(server, "entry", "/entry"); + new LoopbackURLFetcher().fetch(entry); + } + } + + @Test( + expectedExceptions = IOException.class, + expectedExceptionsMessageRegExp = "Only http and https URLs are allowed.*") + public void checkRedirectToNonHttpSchemeIsRejected() throws Exception { + try (LocalServers servers = new LocalServers()) { + String entry = servers.redirectTo("entry", "file:///etc/passwd"); + new LoopbackURLFetcher().fetch(entry); + } + } + + /** A fetcher that accepts loopback targets, so that local servers can stand in for real hosts. */ + private static class LoopbackURLFetcher extends URLFetcher { + private final List validated = new ArrayList<>(); + + LoopbackURLFetcher() { + super(1024); + } + + @Override + void validateTarget(String url) throws IOException { + validated.add(url); + if (!url.startsWith("http://") && !url.startsWith("https://")) { + throw new IOException("Only http and https URLs are allowed, got: " + url); + } + } + } + + /** A handful of throwaway HTTP servers bound to the loopback interface. */ + private static class LocalServers implements AutoCloseable { + private final List servers = new ArrayList<>(); + + HttpServer newServer() throws IOException { + HttpServer server = HttpServer.create(new InetSocketAddress("127.0.0.1", 0), 0); + server.start(); + servers.add(server); + return server; + } + + private String url(HttpServer server, String path) { + return "http://127.0.0.1:" + server.getAddress().getPort() + "/" + path; + } + + String serveContent(String path, String content) throws IOException { + HttpServer server = newServer(); + server.createContext("/" + path, exchange -> respond(exchange, 200, content)); + return url(server, path); + } + + String echoAuthorization(String path) throws IOException { + HttpServer server = newServer(); + echoAuthorization(server, path); + return url(server, path); + } + + void echoAuthorization(HttpServer server, String path) { + server.createContext( + "/" + path, + exchange -> + respond( + exchange, + 200, + "authorization=" + exchange.getRequestHeaders().getFirst("Authorization"))); + } + + String redirectTo(String path, String location) throws IOException { + return redirectTo(newServer(), path, location); + } + + String redirectTo(HttpServer server, String path, String location) { + server.createContext( + "/" + path, + exchange -> { + exchange.getResponseHeaders().add("Location", location); + exchange.sendResponseHeaders(302, -1); + exchange.close(); + }); + return url(server, path); + } + + private static void respond(HttpExchange exchange, int code, String body) throws IOException { + byte[] bytes = body.getBytes(UTF_8); + exchange.sendResponseHeaders(code, bytes.length); + exchange.getResponseBody().write(bytes); + exchange.close(); + } + + @Override + public void close() { + servers.forEach(server -> server.stop(0)); + } + } + /** Limit to only one Byte. */ static class OneByteURLFetcher extends URLFetcher { @@ -219,5 +369,11 @@ String fetch(URLConnection urlConnection) { assertion.accept(urlConnection.getConnectTimeout()); return "NOOP"; } + + /** Answers "not a redirect" without issuing the request. */ + @Override + Optional redirectLocation(HttpURLConnection connection) { + return Optional.empty(); + } } } diff --git a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java index 774eebda897..f7bd25c8383 100644 --- a/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java +++ b/wsmaster/che-core-api-workspace/src/test/java/org/eclipse/che/api/workspace/server/devfile/URLFileContentProviderTest.java @@ -122,6 +122,17 @@ public void shouldNotSendCredentialsToAForeignHost() throws Exception { verify(urlFetcher).fetch(eq(foreignUrl), eq(null)); } + @Test + public void shouldNotSendCredentialsOverPlainHttpToTheDevfileHost() throws Exception { + String devfileUrl = "https://myhost.com/relative/devfile.yaml"; + String plaintextUrl = "http://myhost.com/relative/dependent.yaml"; + URLFileContentProvider provider = new URLFileContentProvider(new URI(devfileUrl), urlFetcher); + + provider.fetchContent(plaintextUrl, "user:pass"); + + verify(urlFetcher).fetch(eq(plaintextUrl), eq(null)); + } + @Test public void shouldNotSendCredentialsWhenTheDevfileLocationIsUnknown() throws Exception { String url = "https://myhost.com/relative/devfile.yaml";