Update version comments for SHA-pinned GitHub Actions

common/lib/dependabot/git_commit_checker.rb

@@ -67,6 +67,10 @@ def pinned_ref_looks_like_version?
 
     def pinned_ref_looks_like_commit_sha?
       ref = dependency_source_details.fetch(:ref)
+      ref_looks_like_commit_sha?(ref)
+    end
+
+    def ref_looks_like_commit_sha?(ref)
       return false unless ref&.match?(/^[0-9a-f]{6,40}$/)
 
       return false unless pinned?

github_actions/lib/dependabot/github_actions/file_updater.rb

@@ -65,17 +65,41 @@ def updated_workflow_file_content(file)
             gsub(/@.*+/, "@#{new_req.fetch(:source).fetch(:ref)}")
 
           # Replace the old declaration that's preceded by a non-word character
-          # and followed by a whitespace character (comments) or EOL
+          # and followed by a whitespace character (comments) or EOL.
+          # If the declaration is followed by a comment that lists the version associated
+          # with the SHA source ref, then update the comment to the human-readable new version.
+          # However, if the comment includes additional text beyond the version, for safety
+          # we skip updating the comment in case it's a custom note, todo, warning etc of some kind.
+          # See the related unit tests for examples.
           updated_content =
             updated_content.
             gsub(
-              /(?<=\W|"|')#{Regexp.escape(old_declaration)}(?=\s|"|'|$)/,
-              new_declaration
-            )
+              /(?<=\W|"|')#{Regexp.escape(old_declaration)}(?<comment>\s+#.*)?(?=\s|"|'|$)/
+            ) do |match|
+              comment = Regexp.last_match(:comment)
+              match.gsub!(old_declaration, new_declaration)
+              if comment && (updated_comment = updated_version_comment(comment, new_req))
+                match.gsub!(comment, updated_comment)
+              end
+              match
+            end
         end
 
         updated_content
       end
+
+      def updated_version_comment(comment, new_req)
+        raise "No comment!" unless comment
+
+        comment = comment.rstrip
+        return unless dependency.previous_version && dependency.version
+        return unless comment.end_with? dependency.previous_version
+
+        git_checker = Dependabot::GitCommitChecker.new(dependency: dependency, credentials: credentials)
+        return unless git_checker.ref_looks_like_commit_sha?(new_req.fetch(:source).fetch(:ref))
+
+        comment.gsub(dependency.previous_version, dependency.version)
+      end
     end
   end
 end

github_actions/spec/dependabot/github_actions/file_updater_spec.rb

@@ -293,6 +293,107 @@
           expect(subject.content).not_to include "actions/[email protected]\n"
         end
       end
+
+      context "with pinned SHA hash and version in comment" do
+        let(:service_pack_url) do
+          "https://github.com/actions/checkout.git/info/refs" \
+            "?service=git-upload-pack"
+        end
+        before do
+          stub_request(:get, service_pack_url).
+            to_return(
+              status: 200,
+              body: fixture("git", "upload_packs", "checkout"),
+              headers: {
+                "content-type" => "application/x-git-upload-pack-advertisement"
+              }
+            )
+        end
+
+        let(:workflow_file_body) do
+          fixture("workflow_files", "pinned_sources_version_comments.yml")
+        end
+        let(:dependency) do
+          Dependabot::Dependency.new(
+            name: "actions/checkout",
+            version: "2.2.0",
+            package_manager: "github_actions",
+            previous_version: "2.1.0",
+            previous_requirements: [{
+              requirement: nil,
+              groups: [],
+              file: ".github/workflows/workflow.yml",
+              source: {
+                type: "git",
+                url: "https://github.com/actions/checkout",
+                ref: "01aecccf739ca6ff86c0539fbc67a7a5007bbc81",
+                branch: nil
+              },
+              metadata: { declaration_string: "actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81" }
+            }, {
+              requirement: nil,
+              groups: [],
+              file: ".github/workflows/workflow.yml",
+              source: {
+                type: "git",
+                url: "https://github.com/actions/checkout",
+                ref: "v2.1.0",
+                branch: nil
+              },
+              metadata: { declaration_string: "actions/[email protected]" }
+            }],
+            requirements: [{
+              requirement: nil,
+              groups: [],
+              file: ".github/workflows/workflow.yml",
+              source: {
+                type: "git",
+                url: "https://github.com/actions/checkout",
+                ref: "aabbfeb2ce60b5bd82389903509092c4648a9713",
+                branch: nil
+              },
+              metadata: { declaration_string: "actions/checkout@aabbfeb2ce60b5bd82389903509092c4648a9713" }
+            }, {
+              requirement: nil,
+              groups: [],
+              file: ".github/workflows/workflow.yml",
+              source: {
+                type: "git",
+                url: "https://github.com/actions/checkout",
+                ref: "v2.2.0",
+                branch: nil
+              },
+              metadata: { declaration_string: "actions/[email protected]" }
+            }]
+          )
+        end
+
+        it "updates SHA version" do
+          old_sha = dependency.previous_requirements.first.dig(:source, :ref)
+          expect(subject.content).to include "#{dependency.name}@#{dependency.requirements.first.dig(:source, :ref)}"
+          expect(subject.content).not_to match(/#{old_sha}\s+#.*#{dependency.previous_version}/)
+        end
+        it "updates version comment" do
+          new_sha = dependency.requirements.first.dig(:source, :ref)
+          expect(subject.content).not_to match(/@#{new_sha}\s+#.*#{dependency.previous_version}\s*$/)
+
+          expect(subject.content).to include "# v#{dependency.version}"
+          expect(subject.content).to include "# #{dependency.version}"
+          expect(subject.content).to include "# @v#{dependency.version}"
+          expect(subject.content).to include "# pin @v#{dependency.version}"
+          expect(subject.content).to include "# tag=v#{dependency.version}"
+        end
+        it "doesn't update version comments when @ref is not a SHA" do
+          old_version = dependency.previous_requirements[1].dig(:source, :ref)
+          expect(subject.content).not_to match(/@#{old_version}\s+#.*#{dependency.version}/)
+        end
+        it "doesn't update version comments in the middle of sentences" do
+          # rubocop:disable Layout/LineLength
+          expect(subject.content).to include "Versions older than v#{dependency.previous_version} have a security vulnerability"
+          expect(subject.content).not_to include "Versions older than v#{dependency.version} have a security vulnerability"
+          # rubocop:enable Layout/LineLength
+        end
+      end
     end
   end
 end

github_actions/spec/fixtures/workflow_files/pinned_sources_version_comments.yml

@@ -0,0 +1,30 @@
+on: [push]
+
+name: Integration
+jobs:
+  chore:
+    steps:
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # 2.1.0
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # @v2.1.0
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # pin @v2.1.0
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # tag=v2.1.0
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81  #   v2.1.0
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0
+    # The comment on the next line has a trailing tab. The version should still be updated.
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 #v2.1.0 
+    - uses: actions/checkout@01aecc # v2.1.0
+  integration:
+    - uses: actions/[email protected] # comments that include the version (v2.1.0) shouldn't be updated for non-SHA refs
+    - uses: actions/checkout@01aecc#v2.1.0 # this shouldn't be updated, because the version is part of the ref, not a comment.
+
+    # The version in the comment for the next action shouldn't be updated
+    # because it refers to past behavior.
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # Versions older than v2.1.0 have a security vulnerability
+
+    # The versions in the comment for the next action won't be updated.
+    # The first version could be updated, but it's difficult to create
+    # a heuristic that recognizes the first version as a version alias
+    # for the SHA commit, and the second version as a concrete version
+    # that shouldn't change. For simplicity, we don't update either.
+    - uses: actions/checkout@01aecccf739ca6ff86c0539fbc67a7a5007bbc81 # v2.1.0 - Versions older than v2.1.0 have a security vulnerability