FIX: Handle .discourse-compatibility syntax errors (#10891)

Previously, any errors in those files would e.g. blow up the update process in docker_manager.
Now it prints out an error and proceeds as if there was no compatibility file.

Includes:

* DEV: Extract setup_git_repo
* DEV: Use `Dir.mktmpdir`
* DEV: Default to `main` branch (The latest versions of git already do this, so to avoid problems do this by default)

lib/version.rb

@@ -16,6 +16,8 @@ module Discourse
     end
   end
 
+  class InvalidVersionListError < StandardError; end
+
   def self.has_needed_version?(current, needed)
     Gem::Version.new(current) >= Gem::Version.new(needed)
   end
@@ -29,10 +31,16 @@ module Discourse
   #  2.4.4.beta6: some-other-branch-ref
   #  2.4.2.beta1: v1-tag
   def self.find_compatible_resource(version_list, version = ::Discourse::VERSION::STRING)
-
     return unless version_list
 
-    version_list = YAML.load(version_list).sort_by { |v, pin| Gem::Version.new(v) }.reverse
+    begin
+      version_list = YAML.safe_load(version_list)
+    rescue Psych::SyntaxError, Psych::DisallowedClass => e
+    end
+
+    raise InvalidVersionListError unless version_list.is_a?(Hash)
+
+    version_list = version_list.sort_by { |v, pin| Gem::Version.new(v) }.reverse
 
     # If plugin compat version is listed as less than current Discourse version, take the version/hash listed before.
     checkout_version = nil
@@ -54,5 +62,7 @@ module Discourse
     return unless File.directory?("#{path}/.git")
     compat_resource, std_error, s = Open3.capture3("git -C '#{path}' show HEAD@{upstream}:#{Discourse::VERSION_COMPATIBILITY_FILENAME}")
     Discourse.find_compatible_resource(compat_resource) if s.success?
+  rescue InvalidVersionListError => e
+    $stderr.puts "Invalid version list in #{path}"
   end
 end

spec/components/version_spec.rb

@@ -4,9 +4,7 @@ require 'rails_helper'
 require 'version'
 
 describe Discourse::VERSION do
-
   context "has_needed_version?" do
-
     it "works for major comparisons" do
       expect(Discourse.has_needed_version?('1.0.0', '1.0.0')).to eq(true)
       expect(Discourse.has_needed_version?('2.0.0', '1.0.0')).to eq(true)
@@ -44,10 +42,9 @@ describe Discourse::VERSION do
       expect(Discourse.has_needed_version?('1.3.0.beta4', '1.3.0.beta3')).to eq(true)
       expect(Discourse.has_needed_version?('1.3.0', '1.3.0.beta3')).to eq(true)
     end
-
   end
 
-  context "compatible_resource" do
+  context "find_compatible_resource" do
     shared_examples "test compatible resource" do
       it "returns nil when the current version is above all pinned versions" do
         expect(Discourse.find_compatible_resource(version_list, "2.6.0")).to be_nil
@@ -70,6 +67,10 @@ describe Discourse::VERSION do
       expect(Discourse.find_compatible_resource(nil)).to be_nil
     end
 
+    it "raises an error on invalid input" do
+      expect { Discourse.find_compatible_resource("1.0.0.beta1 12f82d5") }.to raise_error(Discourse::InvalidVersionListError)
+    end
+
     context "with a regular compatible list" do
       let(:version_list) { <<~VERSION_LIST
         2.5.0.beta6: twofivebetasix
@@ -94,4 +95,33 @@ describe Discourse::VERSION do
       include_examples "test compatible resource"
     end
   end
+
+  context "find_compatible_git_resource" do
+    let!(:git_directory) do
+      path = nil
+
+      capture_stdout do
+        # Note the lack of colon between version and hash
+        path = setup_git_repo(".discourse-compatibility" => "1.0.0.beta1 12f82d5")
+
+        # Simulate a remote upstream
+        `cd #{path} && git remote add origin #{path}/.git && git fetch -q`
+        `cd #{path} && git branch -u origin/main`
+      end
+
+      path
+    end
+
+    after do
+      FileUtils.remove_entry(git_directory)
+    end
+
+    it "gracefully handles invalid input" do
+      output = capture_stderr do
+        expect(Discourse.find_compatible_git_resource(git_directory)).to be_nil
+      end
+
+      expect(output).to include("Invalid version list")
+    end
+  end
 end

spec/models/remote_theme_spec.rb

@@ -4,23 +4,6 @@ require 'rails_helper'
 
 describe RemoteTheme do
   context '#import_remote' do
-    def setup_git_repo(files)
-      dir = Dir.tmpdir
-      repo_dir = "#{dir}/#{SecureRandom.hex}"
-      `mkdir #{repo_dir}`
-      `cd #{repo_dir} && git init . `
-      `cd #{repo_dir} && git config user.email 'someone@cool.com'`
-      `cd #{repo_dir} && git config user.name 'The Cool One'`
-      `cd #{repo_dir} && git config commit.gpgsign 'false'`
-      files.each do |name, data|
-        FileUtils.mkdir_p(Pathname.new("#{repo_dir}/#{name}").dirname)
-        File.write("#{repo_dir}/#{name}", data)
-        `cd #{repo_dir} && git add #{name}`
-      end
-      `cd #{repo_dir} && git commit -am 'first commit'`
-      repo_dir
-    end
-
     def about_json(love_color: "FAFAFA", tertiary_low_color: "FFFFFF", color_scheme_name: "Amazing", about_url: "https://www.site.com/about")
       <<~JSON
         {

spec/services/themes_spec.rb

@@ -10,23 +10,6 @@ describe ThemesInstallTask do
   end
 
   describe '.new' do
-    def setup_git_repo(files)
-      dir = Dir.tmpdir
-      repo_dir = "#{dir}/#{SecureRandom.hex}"
-      `mkdir #{repo_dir}`
-      `cd #{repo_dir} && git init . `
-      `cd #{repo_dir} && git config user.email 'someone@cool.com'`
-      `cd #{repo_dir} && git config user.name 'The Cool One'`
-      `cd #{repo_dir} && git config commit.gpgsign 'false'`
-      files.each do |name, data|
-        FileUtils.mkdir_p(Pathname.new("#{repo_dir}/#{name}").dirname)
-        File.write("#{repo_dir}/#{name}", data)
-        `cd #{repo_dir} && git add #{name}`
-      end
-      `cd #{repo_dir} && git commit -am 'first commit'`
-      repo_dir
-    end
-
     THEME_NAME = "awesome theme"
 
     def about_json(love_color: "FAFAFA", tertiary_low_color: "FFFFFF", color_scheme_name: "Amazing", about_url: "https://www.site.com/about", component: false)

spec/support/helpers.rb

@@ -128,18 +128,29 @@ module Helpers
     expect(sorted_tag_names(a)).to eq(sorted_tag_names(b))
   end
 
-  def capture_stdout
-    old_stdout = $stdout
+  def capture_output(output_name)
     if ENV['RAILS_ENABLE_TEST_STDOUT']
       yield
       return
     end
+
+    previous_output = output_name == :stdout ? $stdout : $stderr
+
     io = StringIO.new
-    $stdout = io
+    output_name == :stdout ? $stdout = io : $stderr = io
+
     yield
     io.string
   ensure
-    $stdout = old_stdout
+    output_name == :stdout ? $stdout = previous_output : $stderr = previous_output
+  end
+
+  def capture_stdout(&block)
+    capture_output(:stdout, &block)
+  end
+
+  def capture_stderr(&block)
+    capture_output(:stderr, &block)
   end
 
   def set_subfolder(f)
@@ -152,6 +163,21 @@ module Helpers
     end
   end
 
+  def setup_git_repo(files)
+    repo_dir = Dir.mktmpdir
+    `cd #{repo_dir} && git init . --initial-branch=main`
+    `cd #{repo_dir} && git config user.email 'someone@cool.com'`
+    `cd #{repo_dir} && git config user.name 'The Cool One'`
+    `cd #{repo_dir} && git config commit.gpgsign 'false'`
+    files.each do |name, data|
+      FileUtils.mkdir_p(Pathname.new("#{repo_dir}/#{name}").dirname)
+      File.write("#{repo_dir}/#{name}", data)
+      `cd #{repo_dir} && git add #{name}`
+    end
+    `cd #{repo_dir} && git commit -am 'first commit'`
+    repo_dir
+  end
+
   class StubbedJob
     def initialize; end
     def perform(args); end