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-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/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..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,6 +13,8 @@ import java.io.FileNotFoundException; import java.io.IOException; +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 +48,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 getTrustedOrigins() { + Set trustedOrigins = new HashSet<>(super.getTrustedOrigins()); + originOfUrl(BitbucketApiClient.BITBUCKET_API_SERVER).ifPresent(trustedOrigins::add); + return trustedOrigins; + } + @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 8eb74db1a50..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 @@ -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/ @@ -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; @@ -22,6 +24,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 +102,34 @@ public void shouldFetchContent() throws Exception { // then 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.*") + 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-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 00967226c60..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 @@ -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/ @@ -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; @@ -134,22 +135,85 @@ public void shouldThrowDevfileException() throws Exception { } @Test - public void shouldNotAskGitHubAPIForDifferentDomain() throws Exception { - String raw_url = "https://ghserver.com/foo/bar/branch-name/devfile.yaml"; + public void shouldNotSendTokenToForeignHost() throws Exception { + String foreignUrl = "https://attacker.example/collect"; - URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); 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); - var personalAccessToken = new PersonalAccessToken(raw_url, "provider", "che", "my-token"); - when(personalAccessTokenManager.getAndStore(anyString())).thenReturn(personalAccessToken); - fileContentProvider.fetchContent(raw_url); + fileContentProvider.fetchContent(foreignUrl); - verify(urlFetcher).fetch(eq(raw_url), eq("token my-token")); + 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 + 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.*") + public void shouldRejectFileSchemeUrl() throws Exception { + URLFetcher urlFetcher = Mockito.mock(URLFetcher.class); + GithubUrl githubUrl = + new GithubUrl("github") + .withUsername("eclipse") + .withRepository("che") + .withServerUrl("https://github.com"); + FileContentProvider fileContentProvider = + new GithubAuthorizingFileContentProvider(githubUrl, urlFetcher, personalAccessTokenManager); + + fileContentProvider.fetchContent("file:///etc/passwd"); } } 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/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-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/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..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 @@ -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,10 +166,93 @@ 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. 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 trustedOrigins = getTrustedOrigins(); + Optional origin = originOfUrl(requestURL); + if (origin.isPresent() && trustedOrigins.contains(origin.get())) { + return true; + } + LOG.warn( + "Fetching a file from '{}' without credentials: it is not one of the {} provider" + + " origins {}.", + origin.orElse(""), + remoteFactoryUrl.getProviderName(), + trustedOrigins); + return false; + } + + /** + * 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 getTrustedOrigins() { + Set trustedOrigins = new HashSet<>(); + originOfUrlOrHostName(remoteFactoryUrl.getProviderUrl()).ifPresent(trustedOrigins::add); + originOfUrlOrHostName(remoteFactoryUrl.getHostName()).ifPresent(trustedOrigins::add); + try { + originOfUrl(remoteFactoryUrl.rawFileLocation(RAW_CONTENT_PROBE_FILE)) + .ifPresent(trustedOrigins::add); + } catch (RuntimeException e) { + LOG.debug( + "Unable to resolve the raw content origin of {}", remoteFactoryUrl.getProviderUrl(), e); + } + return trustedOrigins; + } + + /** 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 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 originOfUrlOrHostName(String urlOrHostName) { + if (isNullOrEmpty(urlOrHostName)) { + return Optional.empty(); + } + return parseOrigin(urlOrHostName.contains("://") ? urlOrHostName : "https://" + urlOrHostName); + } + + private static Optional parseOrigin(String url) { + try { + 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(); + } + } + 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..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 @@ -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; @@ -51,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 @@ -78,4 +95,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..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 @@ -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/ @@ -23,8 +23,11 @@ import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; +import java.net.HttpURLConnection; import java.net.URL; import java.net.URLConnection; +import java.util.Locale; +import java.util.Optional; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -32,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; @@ -49,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$"); @@ -128,13 +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"); - URLConnection connection = new URL(sanitized(url)).openConnection(); - connection.setConnectTimeout(timeout); - connection.setReadTimeout(timeout); - if (!isNullOrEmpty(authorization)) { - connection.setRequestProperty(HttpHeaders.AUTHORIZATION, authorization); + // 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; } - return fetch(connection); + } + + /** + * 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 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(); } /** 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..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 @@ -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,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; @@ -46,6 +50,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) { @@ -58,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( @@ -75,6 +85,55 @@ 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 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 URI requestURI; + try { + requestURI = new URI(requestURL); + } catch (URISyntaxException e) { + return false; + } + if (isSameOrigin(requestURI, devfileLocation)) { + return true; + } + LOG.warn( + "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 7f4c7896ea5..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 @@ -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/ @@ -18,16 +18,22 @@ 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; -import org.testng.Assert; import org.testng.annotations.Listeners; import org.testng.annotations.Test; @@ -50,13 +56,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 +80,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"); + } + + /** 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"); + } - // test to download this class object - URL urlJson = getClass().getClassLoader().getResource("devfile/url_fetcher_test_resource.json"); - Assert.assertNotNull(urlJson); + /** 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); - // add extra path to make url not found - String content = urlFetcher.fetch(urlJson.toString() + "-invalid"); - assertNull(content); + 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 */ @@ -174,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 { @@ -202,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 f067c3ed43c..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 @@ -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,102 @@ 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 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 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"; + 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";