diff --git a/common/lib/dependabot/git_commit_checker.rb b/common/lib/dependabot/git_commit_checker.rb index d91db6c1d35..804a952882b 100644 --- a/common/lib/dependabot/git_commit_checker.rb +++ b/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? diff --git a/github_actions/lib/dependabot/github_actions/file_updater.rb b/github_actions/lib/dependabot/github_actions/file_updater.rb index 65372f677bc..d6aa9a47ec8 100644 --- a/github_actions/lib/dependabot/github_actions/file_updater.rb +++ b/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)}(?\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 diff --git a/github_actions/spec/dependabot/github_actions/file_updater_spec.rb b/github_actions/spec/dependabot/github_actions/file_updater_spec.rb index 9262046b52a..a679a3b1c59 100644 --- a/github_actions/spec/dependabot/github_actions/file_updater_spec.rb +++ b/github_actions/spec/dependabot/github_actions/file_updater_spec.rb @@ -293,6 +293,107 @@ expect(subject.content).not_to include "actions/cache@v2.1.2\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/checkout@v2.1.0" } + }], + 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/checkout@v2.2.0" } + }] + ) + 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 diff --git a/github_actions/spec/fixtures/workflow_files/pinned_sources_version_comments.yml b/github_actions/spec/fixtures/workflow_files/pinned_sources_version_comments.yml new file mode 100644 index 00000000000..3128412273b --- /dev/null +++ b/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/checkout@v2.1.0 # 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