Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions common/lib/dependabot/git_commit_checker.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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?
Expand Down
26 changes: 22 additions & 4 deletions github_actions/lib/dependabot/github_actions/file_updater.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,17 +65,35 @@ 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, attempt to update
# any version comments associated with SHA source refs.
updated_content =
updated_content.
gsub(
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you could use updated_content.gsub! instead of updated_content = updated_content.gsub. The ! mutates the original updated_content

/(?<=\W|"|')#{Regexp.escape(old_declaration)}(?=\s|"|'|$)/,
new_declaration
)
/(?<=\W|"|')#{Regexp.escape(old_declaration)}(?<comment>\s+#.*)?(?=\s|"|'|$)/
) do |match|
comment = Regexp.last_match(:comment)
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Something like this might also work

s.gsub!(old_declaration, new_declaration)
if (comment = Regexp.last_match(:comment))
  if (updated_comment = updated_version_comment(comment, new_req))
    s.gsub!(comment, updated_comment)
  end
end

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I got this suggestion working and I like it much better. Thanks!

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
return unless dependency.previous_version && dependency.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
Expand Down
95 changes: 95 additions & 0 deletions github_actions/spec/dependabot/github_actions/file_updater_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -293,6 +293,101 @@
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}/)

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
end
end
end
end
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
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
- 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.