Merge pull request #6052 from dependabot/deivid-rodriguez/fix-github-…

…actions-crash

Fix GitHub actions updater crashing and cloning the wrong repository

common/lib/dependabot/git_commit_checker.rb

@@ -70,6 +70,11 @@ def pinned_ref_looks_like_commit_sha?
       ref_looks_like_commit_sha?(ref)
     end
 
+    def head_commit_for_pinned_ref
+      ref = dependency_source_details.fetch(:ref)
+      local_repo_git_metadata_fetcher.head_commit_for_ref_sha(ref)
+    end
+
     def ref_looks_like_commit_sha?(ref)
       return false unless ref&.match?(/^[0-9a-f]{6,40}$/)
 
@@ -85,13 +90,8 @@ def branch_or_ref_in_release?(version)
     def head_commit_for_current_branch
       ref = ref_or_branch || "HEAD"
 
-      if pinned?
-        return dependency.version ||
-               local_repo_git_metadata_fetcher.head_commit_for_ref(ref)
-      end
-
-      sha = local_repo_git_metadata_fetcher.head_commit_for_ref(ref)
-      return sha if sha
+      sha = head_commit_for_local_branch(ref)
+      return sha if pinned? || sha
 
       raise Dependabot::GitDependencyReferenceNotFound, dependency.name
     end

common/lib/dependabot/git_metadata_fetcher.rb

@@ -44,6 +44,12 @@ def head_commit_for_ref(ref)
         commit_sha
     end
 
+    def head_commit_for_ref_sha(ref)
+      refs_for_upload_pack.
+        find { |r| r.ref_sha == ref }&.
+        commit_sha
+    end
+
     private
 
     attr_reader :url, :credentials

common/spec/dependabot/git_commit_checker_spec.rb

@@ -510,31 +510,32 @@
           ref: "v1.0.0"
         }
       end
-      it { is_expected.to eq(dependency.version) }
 
-      context "without a version" do
-        let(:version) { nil }
+      let(:git_header) do
+        { "content-type" => "application/x-git-upload-pack-advertisement" }
+      end
+      let(:auth_header) { "Basic eC1hY2Nlc3MtdG9rZW46dG9rZW4=" }
 
-        let(:git_header) do
-          { "content-type" => "application/x-git-upload-pack-advertisement" }
-        end
-        let(:auth_header) { "Basic eC1hY2Nlc3MtdG9rZW46dG9rZW4=" }
+      let(:git_url) do
+        "https://github.com/gocardless/business.git" \
+          "/info/refs?service=git-upload-pack"
+      end
 
-        let(:git_url) do
-          "https://github.com/gocardless/business.git" \
-            "/info/refs?service=git-upload-pack"
+      context "that can be reached just fine" do
+        before do
+          stub_request(:get, git_url).
+            with(headers: { "Authorization" => auth_header }).
+            to_return(
+              status: 200,
+              body: fixture("git", "upload_packs", "business"),
+              headers: git_header
+            )
         end
 
-        context "that can be reached just fine" do
-          before do
-            stub_request(:get, git_url).
-              with(headers: { "Authorization" => auth_header }).
-              to_return(
-                status: 200,
-                body: fixture("git", "upload_packs", "business"),
-                headers: git_header
-              )
-          end
+        it { is_expected.to eq(dependency.version) }
+
+        context "without a version" do
+          let(:version) { nil }
 
           it { is_expected.to eq("df9f605d7111b6814fe493cf8f41de3f9f0978b2") }
 

github_actions/lib/dependabot/github_actions.rb

@@ -22,6 +22,3 @@
 require "dependabot/dependency"
 Dependabot::Dependency.
   register_production_check("github_actions", ->(_) { true })
-
-require "dependabot/utils"
-Dependabot::Utils.register_always_clone("github_actions")

github_actions/lib/dependabot/github_actions/update_checker.rb

@@ -83,12 +83,27 @@ def fetch_latest_version_for_git_dependency
       end
 
       def latest_commit_for_pinned_ref
-        @latest_commit_for_pinned_ref ||=
-          SharedHelpers.in_a_temporary_repo_directory("/", repo_contents_path) do
-            ref_branch = find_container_branch(current_commit)
+        @latest_commit_for_pinned_ref ||= begin
+          head_commit_for_ref_sha = git_commit_checker.head_commit_for_pinned_ref
+          if head_commit_for_ref_sha
+            head_commit_for_ref_sha
+          else
+            url = dependency_source_details[:url]
+            source = Source.from_url(url)
 
-            git_commit_checker.head_commit_for_local_branch(ref_branch)
+            SharedHelpers.in_a_temporary_directory(File.dirname(source.repo)) do |temp_dir|
+              repo_contents_path = File.join(temp_dir, File.basename(source.repo))
+
+              SharedHelpers.run_shell_command("git clone --no-recurse-submodules #{url} #{repo_contents_path}")
+
+              Dir.chdir(repo_contents_path) do
+                ref_branch = find_container_branch(dependency_source_details[:ref])
+
+                git_commit_checker.head_commit_for_local_branch(ref_branch)
+              end
+            end
           end
+        end
       end
 
       def latest_version_tag
@@ -184,17 +199,17 @@ def shortened_semver_eq?(base, other)
       end
 
       def find_container_branch(sha)
-        SharedHelpers.run_shell_command("git fetch #{current_commit}")
-
-        branches_including_ref = SharedHelpers.run_shell_command("git branch --contains #{sha}").split("\n")
+        branches_including_ref = SharedHelpers.run_shell_command(
+          "git branch --remotes --contains #{sha}"
+        ).split("\n").map { |branch| branch.strip.gsub("origin/", "") }
 
-        current_branch = branches_including_ref.find { |line| line.start_with?("* ") }
+        current_branch = branches_including_ref.find { |branch| branch.start_with?("HEAD -> ") }
 
         if current_branch
-          current_branch.delete_prefix("* ")
+          current_branch.delete_prefix("HEAD -> ")
         elsif branches_including_ref.size > 1
           # If there are multiple non default branches including the pinned SHA, then it's unclear how we should proceed
-          raise "Multiple ambiguous branches (#{branches_including_ref.join(', ')}) include #{current_commit}!"
+          raise "Multiple ambiguous branches (#{branches_including_ref.join(', ')}) include #{sha}!"
         else
           branches_including_ref.first
         end

github_actions/spec/dependabot/github_actions/update_checker_spec.rb

@@ -392,6 +392,67 @@
       end
     end
 
+    context "given a realworld repository", :vcr do
+      let(:dependency) do
+        Dependabot::Dependency.new(
+          name: dependency_name,
+          version: dependency_version,
+          requirements: [{
+            requirement: nil,
+            groups: [],
+            file: ".github/workflows/main.yml",
+            source: dependency_source
+          }],
+          package_manager: "github_actions"
+        )
+      end
+      let(:dependency_name) { "dependabot-fixtures/github-action-push-to-another-repository" }
+      let(:dependency_version) { nil }
+      let(:dependency_source) do
+        {
+          type: "git",
+          url: "https://github.com/dependabot-fixtures/github-action-push-to-another-repository",
+          ref: reference,
+          branch: nil
+        }
+      end
+
+      let(:latest_commit_in_main) { "9e487f29582587eeb4837c0552c886bb0644b6b9" }
+      let(:latest_commit_in_devel) { "c7563454dd4fbe0ea69095188860a62a19658a04" }
+
+      context "when pinned to an up to date commit in the default branch" do
+        let(:reference) { latest_commit_in_main }
+
+        it "returns the expected value" do
+          expect(subject).to eq(latest_commit_in_main)
+        end
+      end
+
+      context "when pinned to an out of date commit in the default branch" do
+        let(:reference) { "f4b9c90516ad3bdcfdc6f4fcf8ba937d0bd40465" }
+
+        it "returns the expected value" do
+          expect(subject).to eq(latest_commit_in_main)
+        end
+      end
+
+      context "when pinned to an up to date commit in a non default branch" do
+        let(:reference) { latest_commit_in_devel }
+
+        it "returns the expected value" do
+          expect(subject).to eq(latest_commit_in_devel)
+        end
+      end
+
+      context "when pinned to an out of date commit in a non default branch" do
+        let(:reference) { "96e7dec17bbeed08477b9edab6c3a573614b829d" }
+
+        it "returns the expected value" do
+          expect(subject).to eq(latest_commit_in_devel)
+        end
+      end
+    end
+
     context "that is a git commit SHA not pointing to the tip of a branch" do
       let(:reference) { "1c24df3" }
       let(:exit_status) { double(success?: true) }
@@ -401,14 +462,18 @@
         allow(git_commit_checker).to receive(:branch_or_ref_in_release?).and_return(false)
         allow(git_commit_checker).to receive(:head_commit_for_current_branch).and_return(reference)
 
-        allow(Open3).to receive(:capture2e).with(anything, "git fetch #{reference}").and_return(["", exit_status])
+        allow(Dir).to receive(:chdir).and_yield
+
+        allow(Open3).to receive(:capture2e).
+          with(anything, %r{git clone --no-recurse-submodules https://github\.com/actions/setup-node}).
+          and_return(["", exit_status])
       end
 
       context "and it's in the current (default) branch" do
         before do
           allow(Open3).to receive(:capture2e).
-            with(anything, "git branch --contains #{reference}").
-            and_return(["* master\n", exit_status])
+            with(anything, "git branch --remotes --contains #{reference}").
+            and_return(["  origin/HEAD -> origin/master\n  origin/master", exit_status])
         end
 
         it "can update to the latest version" do
@@ -421,8 +486,8 @@
 
         before do
           allow(Open3).to receive(:capture2e).
-            with(anything, "git branch --contains #{reference}").
-            and_return(["releases/v1\n", exit_status])
+            with(anything, "git branch --remotes --contains #{reference}").
+            and_return(["  origin/releases/v1\n", exit_status])
         end
 
         it "can update to the latest version" do
@@ -433,8 +498,8 @@
       context "and multiple branches include it, the current (default) branch among them" do
         before do
           allow(Open3).to receive(:capture2e).
-            with(anything, "git branch --contains #{reference}").
-            and_return(["* master\nv1.1\n", exit_status])
+            with(anything, "git branch --remotes --contains #{reference}").
+            and_return(["  origin/HEAD -> origin/master\n  origin/master\n  origin/v1.1\n", exit_status])
         end
 
         it "can update to the latest version" do
@@ -445,8 +510,8 @@
       context "and multiple branches include it, the current (default) branch NOT among them" do
         before do
           allow(Open3).to receive(:capture2e).
-            with(anything, "git branch --contains #{reference}").
-            and_return(["3.3-stable\nproduction\n", exit_status])
+            with(anything, "git branch --remotes --contains #{reference}").
+            and_return(["  origin/3.3-stable\n  origin/production\n", exit_status])
         end
 
         it "raises an error" do