From 291bbc4fb966165c9f7bbc7af6bea705b8c09a7d Mon Sep 17 00:00:00 2001 From: Daniel Waterworth Date: Thu, 3 Nov 2022 15:19:08 -0500 Subject: [PATCH] FIX: When cloning themes via HTTP, try the original URI too (#18870) This should fix fetching from gitlab. In order to get SSRF protection, we had to prevent redirects when cloning via git, but some repos are behind redirects and we want to support those too. We use `FinalDestination` before cloning to try to simulate git with redirects, but this isn't quite how git works, so there's some discrepancies between our SSRF protected cloning behavior and normal git behavior that I'm trying to work around. This is temporary fix. It would be better to use `FinalDestination` to simulate the first request that git makes. I aim to make it work like that in the not too distant future, but this is better for now. --- lib/theme_store/git_importer.rb | 54 +++++++++++++++++++-------------- 1 file changed, 31 insertions(+), 23 deletions(-) diff --git a/lib/theme_store/git_importer.rb b/lib/theme_store/git_importer.rb index c386fea5ee7..93878b4e21d 100644 --- a/lib/theme_store/git_importer.rb +++ b/lib/theme_store/git_importer.rb @@ -117,36 +117,44 @@ class ThemeStore::GitImporter end def clone_http! + uris = [@uri] + begin - @uri = FinalDestination.resolve(@uri.to_s) + resolved_uri = FinalDestination.resolve(@uri.to_s) + if resolved_uri && resolved_uri != @uri + uris.unshift(resolved_uri) + end rescue - raise_import_error! + # If this fails, we can stil attempt to clone using the original URI end - @url = @uri.to_s + uris.each do |uri| + @uri = uri + @url = @uri.to_s - unless ["http", "https"].include?(@uri.scheme) - raise_import_error! + unless ["http", "https"].include?(@uri.scheme) + raise_import_error! + end + + addresses = FinalDestination::SSRFDetector.lookup_and_filter_ips(@uri.host) + + unless addresses.empty? + env = { "GIT_TERMINAL_PROMPT" => "0" } + + args = clone_args( + "http.followRedirects" => "false", + "http.curloptResolve" => "#{@uri.host}:#{@uri.port}:#{addresses.join(',')}", + ) + + begin + Discourse::Utils.execute_command(env, *args, timeout: COMMAND_TIMEOUT_SECONDS) + return + rescue RuntimeError + end + end end - addresses = FinalDestination::SSRFDetector.lookup_and_filter_ips(@uri.host) - - if addresses.empty? - raise_import_error! - end - - env = { "GIT_TERMINAL_PROMPT" => "0" } - - args = clone_args( - "http.followRedirects" => "false", - "http.curloptResolve" => "#{@uri.host}:#{@uri.port}:#{addresses.join(',')}", - ) - - begin - Discourse::Utils.execute_command(env, *args, timeout: COMMAND_TIMEOUT_SECONDS) - rescue RuntimeError - raise_import_error! - end + raise_import_error! end def clone_ssh!