fix(security): block SSRF via Git SSH connection — JGit code path bypasses WebClientUtils SSRF filter #41684
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| import net.minidev.json.JSONObject; | ||
| import org.eclipse.jgit.util.StringUtils; | ||
| import reactor.core.publisher.Mono; | ||
| import reactor.core.scheduler.Schedulers; | ||
| import reactor.netty.http.client.HttpClientRequest; | ||
|
|
||
| import java.io.IOException; | ||
|
|
@@ -42,6 +43,86 @@ public class GitUtils { | |
| public static final Pattern URL_PATTERN_WITHOUT_SCHEME = | ||
| Pattern.compile("^[a-z_][\\w-]+@(?<host>[\\w-.]+):/*(?<path>.+?)(\\.git)?$"); | ||
|
|
||
| /** | ||
| * Extracts the hostname from a Git SSH URL. | ||
| * Supports both scheme-based (ssh://git@host/path) and SCP-style (git@host:path) formats. | ||
| * | ||
| * @param url the Git remote URL | ||
| * @return the extracted hostname, or null if the URL doesn't match any known SSH format | ||
| */ | ||
| public static String extractHostFromGitUrl(String url) { | ||
| if (StringUtils.isEmptyOrNull(url)) { | ||
| return null; | ||
| } | ||
|
|
||
| Matcher match = URL_PATTERN_WITH_SCHEME.matcher(url); | ||
| if (match.matches()) { | ||
| return match.group("host"); | ||
| } | ||
|
|
||
| match = URL_PATTERN_WITHOUT_SCHEME.matcher(url); | ||
| if (match.matches()) { | ||
| return match.group("host"); | ||
| } | ||
|
|
||
| return null; | ||
| } | ||
|
|
||
| /** | ||
| * Validates that the host in a Git SSH URL is not a blocked internal/reserved address. | ||
| * Uses {@link WebClientUtils#resolveIfAllowed(String)} to resolve the hostname and check | ||
| * against loopback, link-local, multicast, cloud metadata, and ULA addresses. | ||
| * RFC 1918 private ranges (10/8, 172.16/12, 192.168/16) are intentionally allowed | ||
| * to support self-hosted Git servers on private networks. | ||
| * | ||
| * @param remoteUrl the Git SSH remote URL to validate | ||
| * @return a Mono that completes empty if allowed, or errors with AppsmithException if blocked | ||
| */ | ||
| public static Mono<Void> validateGitSshUrl(String remoteUrl) { | ||
| if (StringUtils.isEmptyOrNull(remoteUrl)) { | ||
| return Mono.error(new AppsmithException(AppsmithError.INVALID_PARAMETER, "remote url")); | ||
| } | ||
|
|
||
| String host = extractHostFromGitUrl(remoteUrl); | ||
| if (host == null) { | ||
| return Mono.error(new AppsmithException( | ||
| AppsmithError.INVALID_GIT_CONFIGURATION, | ||
| "Remote URL is not a valid SSH URL. Example: git@example.com:username/reponame.git")); | ||
| } | ||
|
|
||
| return Mono.fromCallable(() -> WebClientUtils.resolveIfAllowed(host)) | ||
| .subscribeOn(Schedulers.boundedElastic()) | ||
| .flatMap(resolved -> resolved.isPresent() | ||
| ? Mono.<Void>empty() | ||
| : Mono.error(new AppsmithException(AppsmithError.GIT_REMOTE_URL_HOST_BLOCKED, host))); | ||
|
Comment on lines
+93
to
+97
Collaborator
Author
There was a problem hiding this comment. @coderabbitai I think this is impractical because JGit’s SSH session factory resolves hostnames internally - there’s no API to pass a pre-resolved IP for SSH connections. The attack window is negligible (validation and connect happen in the same reactive chain), and SSH host key verification provides additional protection against rebinding. WDYT?
Comment on lines
+93
to
+97
Contributor
There was a problem hiding this comment. Separate DNS misses from blocked hosts.
Also applies to: 119-123 🤖 Prompt for AI Agents |
||
| } | ||
|
|
||
| /** | ||
| * Defense-in-depth validation for persisted remote URLs before outbound SSH operations. | ||
| * Unlike {@link #validateGitSshUrl(String)}, this method gracefully skips null/empty URLs | ||
| * and non-SSH URLs (e.g. HTTPS), since those don't trigger the SSH SSRF vector. | ||
| * | ||
| * @param remoteUrl the persisted Git remote URL to validate | ||
| * @return a Mono that completes empty if allowed or not an SSH URL, or errors if blocked | ||
| */ | ||
| public static Mono<Void> validatePersistedGitSshUrl(String remoteUrl) { | ||
| if (StringUtils.isEmptyOrNull(remoteUrl)) { | ||
| return Mono.empty(); | ||
| } | ||
|
|
||
| String host = extractHostFromGitUrl(remoteUrl); | ||
| if (host == null) { | ||
| // Not an SSH URL (e.g. HTTPS) — no SSH SSRF risk, skip validation | ||
| return Mono.empty(); | ||
| } | ||
|
|
||
| return Mono.fromCallable(() -> WebClientUtils.resolveIfAllowed(host)) | ||
| .subscribeOn(Schedulers.boundedElastic()) | ||
| .flatMap(resolved -> resolved.isPresent() | ||
| ? Mono.<Void>empty() | ||
| : Mono.error(new AppsmithException(AppsmithError.GIT_REMOTE_URL_HOST_BLOCKED, host))); | ||
| } | ||
|
|
||
| /** | ||
| * Sample repo urls : | ||
| * git@example.com:user/repoName.git | ||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Preserve
GIT_REMOTE_URL_HOST_BLOCKEDincreateReference().fetchRemoteMonocan now fail withAppsmithException, but the downstreamonErrorResumeat Line 767 rewrites every failure asGIT_ACTION_FAILED. The branch creation flow still aborts, but clients lose the specific 400 and message introduced by this PR.🤖 Prompt for AI Agents