From b4112ce4639d7eed1e3b2e0792eb7533f7cb125f Mon Sep 17 00:00:00 2001 From: James Roberts Date: Mon, 31 Oct 2022 00:31:53 -0400 Subject: [PATCH] Update version comments for SHA-pinned GitHub Actions (#5951) GitHub encourages pinning third-party GitHub Actions to a full length commit SHA. It's common for actions pinned by commit SHA to include a comment specifying the version associated with the commit. For example: - uses: actions/checkout@01aecc # v2.1.0 This change updates the GitHub Actions manager to bump versions in comments that follow SHA-pinned actions, so the comment stays up-to-date with the SHA being updated. The file_updater now searches the comment string for all references to the previous version and replaces them with the new version. To avoid changing unrelated comments, the comment updater only updates dependencies that pin SHA refs. If the version in a comment is followed by additional text, it could be documentation explaining some behavior, rather than a tag corresponding to the SHA in the action version. To be safe, don't update the version unless it's the end of the comment. --- common/lib/dependabot/git_commit_checker.rb | 4 + .../dependabot/github_actions/file_updater.rb | 32 +++++- .../github_actions/file_updater_spec.rb | 101 ++++++++++++++++++ .../pinned_sources_version_comments.yml | 30 ++++++ 4 files changed, 163 insertions(+), 4 deletions(-) create mode 100644 github_actions/spec/fixtures/workflow_files/pinned_sources_version_comments.yml diff --git a/common/lib/dependabot/git_commit_checker.rb b/common/lib/dependabot/git_commit_checker.rb index d91db6c1d3..804a952882 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 65372f677b..d6aa9a47ec 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 9262046b52..a679a3b1c5 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 0000000000..3128412273 --- /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