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.
This commit is contained in:
parent
1398bd5f1f
commit
291bbc4fb9
|
@ -117,12 +117,19 @@ 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
|
||||
|
||||
uris.each do |uri|
|
||||
@uri = uri
|
||||
@url = @uri.to_s
|
||||
|
||||
unless ["http", "https"].include?(@uri.scheme)
|
||||
|
@ -131,10 +138,7 @@ class ThemeStore::GitImporter
|
|||
|
||||
addresses = FinalDestination::SSRFDetector.lookup_and_filter_ips(@uri.host)
|
||||
|
||||
if addresses.empty?
|
||||
raise_import_error!
|
||||
end
|
||||
|
||||
unless addresses.empty?
|
||||
env = { "GIT_TERMINAL_PROMPT" => "0" }
|
||||
|
||||
args = clone_args(
|
||||
|
@ -144,10 +148,14 @@ class ThemeStore::GitImporter
|
|||
|
||||
begin
|
||||
Discourse::Utils.execute_command(env, *args, timeout: COMMAND_TIMEOUT_SECONDS)
|
||||
return
|
||||
rescue RuntimeError
|
||||
raise_import_error!
|
||||
end
|
||||
end
|
||||
end
|
||||
|
||||
raise_import_error!
|
||||
end
|
||||
|
||||
def clone_ssh!
|
||||
unless @private_key.present?
|
||||
|
|
Loading…
Reference in New Issue