fix(security): prevent SSRF and credential leaks in URL fetching by vinokurig · Pull Request #1061 · eclipse-che/che-server

vinokurig · 2026-09-10T07:01:50Z

What does this PR do?

Several endpoints take a URL from the caller and make the server fetch it: the factory devfile resolvers, the SCM file endpoint and the devfile file://-style references resolved by URLFileContentProvider. Neither the destination nor the scheme of those URLs was checked, and the user's personal access token was attached to the request regardless of which host it ended up being sent to.

CWE-918 (SSRF)
Add UrlTargetValidator in che-core-commons-lang, which rejects anything that is not http/https and resolves the host, refusing addresses that are not publicly routable: loopback, link-local (169.254.169.254 and the cloud metadata endpoints behind it), private and site-local ranges, multicast, 100.64.0.0/10 shared space used by CNI plugins, 192.0.0.0/24, 198.18.0.0/15, 240.0.0.0/4 and IPv6 fc00::/7 unique local addresses. All records returned for a host are checked, so a name publishing both a public and an internal address cannot pass the check and then be connected to on the internal one.

URLFetcher applies the check on every hop instead of letting the JDK follow redirects: redirects are as much under the control of whoever supplied the URL as the URL itself, so validating only the first hop left the check bypassable. Redirects are now followed manually, capped at MAX_REDIRECTS.

The GitHub and GitLab URL parsers probe unknown hosts with an API call to decide whether a URL belongs to a self-hosted instance. That probe is now limited to publicly routable hosts, otherwise the factory endpoint's answer turns into an internal port scanner. Private SCM servers are still reached through the configured provider endpoints or a personal access token, both of which are matched before it comes to probing.

CWE-73 (external control of file name or path)
AuthorizingFileContentProvider.formatUrl and URLFileContentProvider reject absolute devfile references whose scheme is not http/https, so a devfile can no longer make the server read file:///etc/... or a jar:/ftp: target and return it in the response. UrlTargetValidator reads the scheme off the raw string so that opaque URLs such as jar:file:/x!/y are rejected as schemes rather than reported as parse failures.

CWE-522 (insufficiently protected credentials)
Credentials are now bound to the origin they belong to. AuthorizingFileContentProvider sends the personal access token only to the provider's own origins - provider URL, host name and the raw content host, plus the Bitbucket API host for the Bitbucket provider - and fetches anything else anonymously. URLFileContentProvider sends the credentials taken from the devfile URL only back to the origin the devfile was loaded from. Both compare the full origin, not just the host, so a devfile cannot downgrade the request to plain http and put the token on the wire in the clear. URLFetcher drops the Authorization header when a redirect crosses origins, and refuses https -> http redirects outright.

Also fix a copy-paste bug in ScmService#getFileContent, where the file parameter was never null-checked because repository was checked twice.

Screenshot/screencast of this PR

What issues does this PR fix or reference?

https://redhat.atlassian.net/browse/CRW-11956

How to test this PR?

PR Checklist

As the author of this Pull Request I made sure that:

Release Notes

Reviewers

Reviewers, please comment how you tested the PR when approving it.

…-11956) Several endpoints take a URL from the caller and make the server fetch it: the factory devfile resolvers, the SCM file endpoint and the devfile `file://`-style references resolved by URLFileContentProvider. Neither the destination nor the scheme of those URLs was checked, and the user's personal access token was attached to the request regardless of which host it ended up being sent to. CWE-918 (SSRF) Add UrlTargetValidator in che-core-commons-lang, which rejects anything that is not http/https and resolves the host, refusing addresses that are not publicly routable: loopback, link-local (169.254.169.254 and the cloud metadata endpoints behind it), private and site-local ranges, multicast, 100.64.0.0/10 shared space used by CNI plugins, 192.0.0.0/24, 198.18.0.0/15, 240.0.0.0/4 and IPv6 fc00::/7 unique local addresses. All records returned for a host are checked, so a name publishing both a public and an internal address cannot pass the check and then be connected to on the internal one. URLFetcher applies the check on every hop instead of letting the JDK follow redirects: redirects are as much under the control of whoever supplied the URL as the URL itself, so validating only the first hop left the check bypassable. Redirects are now followed manually, capped at MAX_REDIRECTS. The GitHub and GitLab URL parsers probe unknown hosts with an API call to decide whether a URL belongs to a self-hosted instance. That probe is now limited to publicly routable hosts, otherwise the factory endpoint's answer turns into an internal port scanner. Private SCM servers are still reached through the configured provider endpoints or a personal access token, both of which are matched before it comes to probing. CWE-73 (external control of file name or path) AuthorizingFileContentProvider.formatUrl and URLFileContentProvider reject absolute devfile references whose scheme is not http/https, so a devfile can no longer make the server read `file:///etc/...` or a `jar:`/`ftp:` target and return it in the response. UrlTargetValidator reads the scheme off the raw string so that opaque URLs such as `jar:file:/x!/y` are rejected as schemes rather than reported as parse failures. CWE-522 (insufficiently protected credentials) Credentials are now bound to the origin they belong to. AuthorizingFileContentProvider sends the personal access token only to the provider's own origins - provider URL, host name and the raw content host, plus the Bitbucket API host for the Bitbucket provider - and fetches anything else anonymously. URLFileContentProvider sends the credentials taken from the devfile URL only back to the origin the devfile was loaded from. Both compare the full origin, not just the host, so a devfile cannot downgrade the request to plain http and put the token on the wire in the clear. URLFetcher drops the Authorization header when a redirect crosses origins, and refuses https -> http redirects outright. Also fix a copy-paste bug in ScmService#getFileContent, where the `file` parameter was never null-checked because `repository` was checked twice. Signed-off-by: Ihor Vinokur <ivinokur@redhat.com> Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

openshift-ci · 2026-09-10T07:02:11Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vinokurig

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

github-actions · 2026-09-10T07:16:23Z

Docker image build succeeded: quay.io/eclipse/che-server:pr-1061

kubectl patch command

kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1061", name: che}]}}]"

…input (CRW-11956) Two families of outbound request still chose their destination from something the caller controls, so either could be used to have the server reach a service only it can see and report back on the outcome (CWE-918). Bitbucket Server URL parser: a repository URL that matches no configured provider was probed to find out whether it is a Bitbucket Server. The URL comes straight off the factory endpoint, so the probe is now limited to publicly routable hosts, matching the guard the GitHub and GitLab parsers already have. A Bitbucket Server on a private network is still reached through che.integration.bitbucket.server_endpoints or a personal access token, both of which are matched first. Token and user-data fetchers: seven fetchers built an API client from the provider URL of a personal access token. That URL comes from a secret in the user's namespace, so anyone holding a namespace could name any host. Each now refuses a URL that is neither the configured provider endpoint nor publicly routable. Guarding the fetchers also closes the paths that reach a host by way of a stored token: KubernetesPersonalAccessTokenManager only returns a token whose provider URL some fetcher accepted, so a secret naming a private address no longer makes AbstractGithubURLParser.isUserTokenPresent accept the host, and no longer reaches getPullRequest, getLatestCommit or BitbucketServerUserDataFetcher. Note for private deployments: UrlTargetValidator has no allowlist, so an install whose SCM server is on a private network cannot be reached this way at all. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

tolusha

Great work on the defense-in-depth design - the four independent SSRF layers (UrlTargetValidator → URLFetcher → AuthorizingFileContentProvider → URLFileContentProvider) are exactly the right approach for this class of vulnerability. The per-hop redirect validation, origin-scoped credential binding, and scheme enforcement are all architecturally sound. The PR description's CWE-by-CWE breakdown makes the security intent unusually clear.

Requesting changes on one critical gap and a few items worth addressing before merge.

🔴 Critical: BitbucketServerURLParser.isApiRequestRelevant() is not guarded

BitbucketServerURLParser.isApiRequestRelevant() (line 160 in BitbucketServerURLParser.java) probes unknown hosts by instantiating HttpBitbucketServerApiClient from the user-supplied URL and calling getUser() — the exact same SSRF-vulnerable pattern this PR fixes in AbstractGithubURLParser and AbstractGitlabUrlParser. The Bitbucket Server parser was not given a canProbe() guard.

Adding the same guard as the GitHub/GitLab parsers would fix it:

if (!canProbe(serverUrl)) {
    LOG.warn("Not probing {}: it does not point to a publicly routable host.", serverUrl);
    return false;
}

⚠️ All 12 CI integration tests failed on commit a55daf3

The failure list includes every provider (GitHub, GitLab, Bitbucket, Azure, Gitea, and the smoke test). This may indicate the SSRF validator is rejecting legitimate in-cluster traffic - e.g., if test SCM servers run on private addresses. Please investigate before merging. If this is expected (e.g., flaky infra), a retest result confirming that would help reviewers.

⚠️ DNS rebinding (TOCTOU): UrlTargetValidator resolves the host at check time, URLConnection re-resolves at connect time

Between InetAddress.getAllByName(host) in UrlTargetValidator.validate() and the subsequent currentUrl.openConnection() in URLFetcher, a malicious DNS server can switch from a public IP (passes the check) to a private IP (actually connected to). This is a known TOCTOU gap in SSRF defenses. Mitigating it would require pinning the resolved address for the connection (e.g., by resolving the host once, connecting by IP with the Host header set manually). This can be a follow-up if addressing it now is out of scope.

⚠️ Behavioral change for private-network SCM: no migration path documented

After this PR, GitHub Enterprise and GitLab instances on private IP ranges (10.x.x.x, 172.16.x.x, etc.) will no longer be auto-discovered via the isApiRequestRelevant probe. Operators who relied on this must configure the endpoint explicitly (e.g., che.integration.github.server_endpoints). The "How to test this PR" section is empty and no release note documents this. Operators upgrading will hit silent failures with no obvious cause. Could you add a note in the release notes / upgrade guide?

Suggestion: canProbe() is not enforced by the type system

The standard review found that BitbucketServerURLParser was missed. A future SCM provider that probes unknown hosts could silently omit the guard. Consider adding canProbe(String serverUrl) to an abstract URL parser base class or interface so new parser implementations are forced to decide. Even a default-false no-op with a doc comment explaining the security implication would help.

vinokurig · 2026-09-10T09:19:41Z

@tolusha

🔴 Critical: BitbucketServerURLParser.isApiRequestRelevant() is not guarded

BitbucketServerURLParser.isApiRequestRelevant() (line 160 in BitbucketServerURLParser.java) probes unknown hosts by instantiating HttpBitbucketServerApiClient from the user-supplied URL and calling getUser() — the exact same SSRF-vulnerable pattern this PR fixes in AbstractGithubURLParser and AbstractGitlabUrlParser. The Bitbucket Server parser was not given a canProbe() guard.

Adding the same guard as the GitHub/GitLab parsers would fix it:

if (!canProbe(serverUrl)) {
LOG.warn("Not probing {}: it does not point to a publicly routable host.", serverUrl);
return false;
}

This is not relevant to the latest commit, the check does exist in the BitbucketServerURLParser.isApiRequestRelevant() function.

InetAddress.isAnyLocalAddress matches 0.0.0.0 alone, not the whole 0.0.0.0/8 "this network" block of RFC 791, so an address such as 0.1.2.3 passed the destination check. On Linux the kernel treats 0.0.0.0/8 as the local host, which makes those addresses another spelling of a loopback target. The documentation ranges of RFC 5737 - 192.0.2.0/24, 198.51.100.0/24 and 203.0.113.0/24 - are not routable on the public internet either, and can be configured on an internal interface. They are now refused as well. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

…forgery' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

…W-11956

…(CRW-11956) Suppress the CodeQL SSRF alert on URLFetcher with an explanation: the target host is user supplied by design, since devfiles are fetched from arbitrary SCM hosts including self hosted ones, so no fixed allowlist applies. validateTarget enforces http/https and a publicly routable destination on every redirect hop. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

github-actions · 2026-09-14T10:05:11Z

Docker image build succeeded: quay.io/eclipse/che-server:pr-1061

kubectl patch command

kubectl patch -n eclipse-che "checluster/eclipse-che" --type=json -p="[{"op": "replace", "path": "/spec/components/cheServer/deployment", "value": {containers: [{image: "quay.io/eclipse/che-server:pr-1061", name: che}]}}]"

vinokurig · 2026-09-14T12:21:32Z

/retest

openshift-ci · 2026-09-14T14:06:01Z

@vinokurig: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
ci/prow/v19-azure-no-pat-oauth-flow-ssh-url	`a2c0f3e`	link	true	`/test v19-azure-no-pat-oauth-flow-ssh-url`
ci/prow/v19-bitbucket-no-pat-oauth-flow	`a2c0f3e`	link	true	`/test v19-bitbucket-no-pat-oauth-flow`
ci/prow/v19-gitlab-no-pat-oauth-flow-raw-devfile-url	`a2c0f3e`	link	true	`/test v19-gitlab-no-pat-oauth-flow-raw-devfile-url`
ci/prow/v19-github-with-pat-setup-flow	`a2c0f3e`	link	true	`/test v19-github-with-pat-setup-flow`
ci/prow/v19-azure-no-pat-oauth-flow-raw-devfile-url	`a2c0f3e`	link	true	`/test v19-azure-no-pat-oauth-flow-raw-devfile-url`
ci/prow/v19-github-no-pat-oauth-flow	`a2c0f3e`	link	true	`/test v19-github-no-pat-oauth-flow`
ci/prow/v19-github-no-pat-oauth-flow-ssh-url	`a2c0f3e`	link	true	`/test v19-github-no-pat-oauth-flow-ssh-url`
ci/prow/v19-bitbucket-no-pat-oauth-flow-ssh-url	`a2c0f3e`	link	true	`/test v19-bitbucket-no-pat-oauth-flow-ssh-url`
ci/prow/v19-gitlab-with-oauth-setup-flow	`a2c0f3e`	link	true	`/test v19-gitlab-with-oauth-setup-flow`

Full PR test history. Your PR dashboard.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

tolusha · 2026-09-14T14:07:07Z

+   * checked before it comes to this.
+   */
+  @VisibleForTesting
+  boolean canProbe(String serverUrl) {


just to mention. It is named canContact in other files

tolusha · 2026-09-14T14:08:49Z

+   */
+  @VisibleForTesting
+  boolean canContact(String scmServerUrl) {
+    return serverUrl.equals(scmServerUrl) || UrlTargetValidator.isAllowed(scmServerUrl);


I didn't see serverUrl.equals(scmServerUrl) for other provides. Is it only for gitlab?

vinokurig requested review from SDawley, ibuziuk and tolusha as code owners September 10, 2026 07:01

github-advanced-security Bot found potential problems Sep 10, 2026

View reviewed changes

Comment thread ...ore-api-workspace/src/main/java/org/eclipse/che/api/workspace/server/devfile/URLFetcher.java Dismissed

tolusha requested changes Sep 10, 2026

View reviewed changes

openshift-ci Bot assigned tolusha Sep 10, 2026

tolusha reviewed Sep 10, 2026

View reviewed changes

Comment thread ...ons/che-core-commons-lang/src/main/java/org/eclipse/che/commons/lang/UrlTargetValidator.java

eclipse-che deleted a comment from openshift-ci Bot Sep 10, 2026

eclipse-che deleted a comment from openshift-ci Bot Sep 11, 2026

vinokurig and others added 4 commits September 14, 2026 10:48

Merge branch 'main' of github.com:eclipse-che/che-server into CRW-11956

d60c367

Potential fix for pull request finding 'CodeQL / Server-side request …

49bf508

…forgery' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>

Merge branch 'CRW-11956' of github.com:eclipse-che/che-server into CR…

868b143

…W-11956

Navigation Menu

Uh oh!

fix(security): prevent SSRF and credential leaks in URL fetching - #1061

fix(security): prevent SSRF and credential leaks in URL fetching#1061
vinokurig wants to merge 7 commits into
mainfrom
CRW-11956

vinokurig commented Sep 10, 2026

Uh oh!

openshift-ci Bot commented Sep 10, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

tolusha left a comment

Uh oh!

Uh oh!

vinokurig commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 14, 2026

Uh oh!

vinokurig commented Sep 14, 2026

Uh oh!

openshift-ci Bot commented Sep 14, 2026

Uh oh!

tolusha Sep 14, 2026

Uh oh!

tolusha Sep 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

vinokurig commented Sep 10, 2026

What does this PR do?

Screenshot/screencast of this PR

What issues does this PR fix or reference?

How to test this PR?

PR Checklist

Release Notes

Reviewers

Uh oh!

openshift-ci Bot commented Sep 10, 2026

Uh oh!

Uh oh!

github-actions Bot commented Sep 10, 2026

Uh oh!

tolusha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vinokurig commented Sep 10, 2026

Uh oh!

github-actions Bot commented Sep 14, 2026

Uh oh!

vinokurig commented Sep 14, 2026

Uh oh!

openshift-ci Bot commented Sep 14, 2026

Uh oh!

tolusha Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

tolusha Sep 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants