From 171477dc50711cde67941f46c14bde5db932f5b6 Mon Sep 17 00:00:00 2001 From: Josh Hagins Date: Sun, 5 Apr 2015 19:51:51 -0400 Subject: [PATCH 1/6] Rename pushed_commits to pushed_refs pushed_refs is more accurate, since each push can contain multiple commits. --- lib/overcommit/hook/pre_push/base.rb | 2 +- lib/overcommit/hook_context/pre_push.rb | 6 +++--- spec/overcommit/hook_context/pre_push_spec.rb | 16 ++++++++-------- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/overcommit/hook/pre_push/base.rb b/lib/overcommit/hook/pre_push/base.rb index 934ce58d..7f07a9ac 100644 --- a/lib/overcommit/hook/pre_push/base.rb +++ b/lib/overcommit/hook/pre_push/base.rb @@ -5,6 +5,6 @@ module Overcommit::Hook::PrePush class Base < Overcommit::Hook::Base extend Forwardable - def_delegators :@context, :remote_name, :remote_url, :pushed_commits + def_delegators :@context, :remote_name, :remote_url, :pushed_refs end end diff --git a/lib/overcommit/hook_context/pre_push.rb b/lib/overcommit/hook_context/pre_push.rb index b0ac8c17..93c31bfe 100644 --- a/lib/overcommit/hook_context/pre_push.rb +++ b/lib/overcommit/hook_context/pre_push.rb @@ -11,13 +11,13 @@ def remote_url @args[1] end - def pushed_commits + def pushed_refs input_lines.map do |line| - PushedCommit.new(*line.split(' ')) + PushedRef.new(*line.split(' ')) end end - PushedCommit = Struct.new(:local_ref, :local_sha1, :remote_ref, :remote_sha1) do + PushedRef = Struct.new(:local_ref, :local_sha1, :remote_ref, :remote_sha1) do def to_s "#{local_ref} #{local_sha1} #{remote_ref} #{remote_sha1}" end diff --git a/spec/overcommit/hook_context/pre_push_spec.rb b/spec/overcommit/hook_context/pre_push_spec.rb index 2c3b43b7..da0f7831 100644 --- a/spec/overcommit/hook_context/pre_push_spec.rb +++ b/spec/overcommit/hook_context/pre_push_spec.rb @@ -21,8 +21,8 @@ it { should == remote_url } end - describe '#pushed_commits' do - subject(:pushed_commits) { context.pushed_commits } + describe '#pushed_refs' do + subject(:pushed_refs) { context.pushed_refs } let(:local_ref) { 'refs/heads/master' } let(:local_sha1) { random_hash } @@ -34,12 +34,12 @@ end it 'should parse commit info from the input' do - pushed_commits.length.should == 1 - pushed_commits.each do |pushed_commit| - pushed_commit.local_ref.should == local_ref - pushed_commit.local_sha1.should == local_sha1 - pushed_commit.remote_ref.should == remote_ref - pushed_commit.remote_sha1.should == remote_sha1 + pushed_refs.length.should == 1 + pushed_refs.each do |pushed_ref| + pushed_ref.local_ref.should == local_ref + pushed_ref.local_sha1.should == local_sha1 + pushed_ref.remote_ref.should == remote_ref + pushed_ref.remote_sha1.should == remote_sha1 end end end From 106b68afeacb3e21e9c1407931e2a790930815c3 Mon Sep 17 00:00:00 2001 From: Josh Hagins Date: Sun, 5 Apr 2015 19:52:34 -0400 Subject: [PATCH 2/6] Add helpers to PushedRef Add helpers which indicate whether the ref has been created, deleted, or force-pushed. --- lib/overcommit/hook_context/pre_push.rb | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/lib/overcommit/hook_context/pre_push.rb b/lib/overcommit/hook_context/pre_push.rb index 93c31bfe..6c255a48 100644 --- a/lib/overcommit/hook_context/pre_push.rb +++ b/lib/overcommit/hook_context/pre_push.rb @@ -18,6 +18,18 @@ def pushed_refs end PushedRef = Struct.new(:local_ref, :local_sha1, :remote_ref, :remote_sha1) do + def forced? + `git rev-list #{remote_sha1} ^#{local_sha1}`.chomp.any? + end + + def created? + remote_sha1 == '0' * 40 + end + + def deleted? + local_sha1 == '0' * 40 + end + def to_s "#{local_ref} #{local_sha1} #{remote_ref} #{remote_sha1}" end From 004866cf48a3caa59169e1f326df517ab9bd898b Mon Sep 17 00:00:00 2001 From: Josh Hagins Date: Sun, 5 Apr 2015 19:54:27 -0400 Subject: [PATCH 3/6] Add ProtectedBranches pre-push hook This hook guards against destructive updates to specified branches. --- .../hook/pre_push/protected_branches.rb | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 lib/overcommit/hook/pre_push/protected_branches.rb diff --git a/lib/overcommit/hook/pre_push/protected_branches.rb b/lib/overcommit/hook/pre_push/protected_branches.rb new file mode 100644 index 00000000..f9c5bc9f --- /dev/null +++ b/lib/overcommit/hook/pre_push/protected_branches.rb @@ -0,0 +1,27 @@ +module Overcommit::Hook::PrePush + # Prevents destructive updates to specified branches. + class ProtectedBranches < Base + def run + return :pass unless illegal_pushes.any? + + messages = illegal_pushes.map do |pushed_ref| + "Deleting or force-pushing to #{pushed_ref.remote_ref} is not allowed." + end + + [:fail, messages.join("\n")] + end + + private + + def branches + @branches ||= config['branches'] + end + + def illegal_pushes + @illegal_pushes ||= pushed_refs.select do |pushed_ref| + (pushed_ref.deleted? || pushed_ref.forced?) && + branches.any? { |branch| pushed_ref.remote_ref =~ /#{branch}/ } + end + end + end +end From 0c964c88dc86cbbb89948a379e00cdd4ffef50ea Mon Sep 17 00:00:00 2001 From: Josh Hagins Date: Sun, 5 Apr 2015 19:56:20 -0400 Subject: [PATCH 4/6] Add config for ProtectedBranches pre-push hook --- config/default.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/config/default.yml b/config/default.yml index 8cc0f505..b3ba0aea 100644 --- a/config/default.yml +++ b/config/default.yml @@ -474,6 +474,11 @@ PrePush: required: false quiet: false + ProtectedBranches: + enabled: false + description: 'Checking for illegal pushes to protected branches' + branches: ['master'] + Rspec: enabled: false description: 'Running rspec test suite' From f05782dc9fd6d4f13b9733639477111542db71fc Mon Sep 17 00:00:00 2001 From: Josh Hagins Date: Sun, 5 Apr 2015 19:56:33 -0400 Subject: [PATCH 5/6] Add spec for ProtectedBranches pre-push hook --- .../hook/pre_push/protected_branches_spec.rb | 76 +++++++++++++++++++ 1 file changed, 76 insertions(+) create mode 100644 spec/overcommit/hook/pre_push/protected_branches_spec.rb diff --git a/spec/overcommit/hook/pre_push/protected_branches_spec.rb b/spec/overcommit/hook/pre_push/protected_branches_spec.rb new file mode 100644 index 00000000..43df022c --- /dev/null +++ b/spec/overcommit/hook/pre_push/protected_branches_spec.rb @@ -0,0 +1,76 @@ +require 'spec_helper' + +describe Overcommit::Hook::PrePush::ProtectedBranches do + let(:config) { Overcommit::ConfigurationLoader.default_configuration } + let(:context) { double('context') } + subject { described_class.new(config, context) } + + let(:protected_branch) { 'master' } + let(:unprotected_branch) { 'other' } + let(:pushed_ref) { double('pushed_ref') } + + before do + subject.stub(:branches).and_return([protected_branch]) + subject.stub(:pushed_refs).and_return([pushed_ref]) + end + + context 'when pushing to unprotected branch' do + before do + pushed_ref.stub(:remote_ref).and_return("refs/heads/#{unprotected_branch}") + end + + context 'when ref is not deleted or force-pushed' do + before do + pushed_ref.stub(deleted?: false, forced?: false) + end + + it { should pass } + end + + context 'when ref is deleted' do + before do + pushed_ref.stub(deleted?: true) + end + + it { should pass } + end + + context 'when ref is force-pushed' do + before do + pushed_ref.stub(deleted?: false, forced?: true) + end + + it { should pass } + end + end + + context 'when pushing to protected branch' do + before do + pushed_ref.stub(:remote_ref).and_return("refs/heads/#{protected_branch}") + end + + context 'when ref is not deleted or force-pushed' do + before do + pushed_ref.stub(deleted?: false, forced?: false) + end + + it { should pass } + end + + context 'when ref is deleted' do + before do + pushed_ref.stub(deleted?: true) + end + + it { should fail_hook } + end + + context 'when ref is force-pushed' do + before do + pushed_ref.stub(deleted?: false, forced?: true) + end + + it { should fail_hook } + end + end +end From c0e0db7db47a4b837bac7b97ad032f05dfe7da04 Mon Sep 17 00:00:00 2001 From: Josh Hagins Date: Sun, 5 Apr 2015 21:13:58 -0400 Subject: [PATCH 6/6] Check for exact match in remote ref name --- lib/overcommit/hook/pre_push/protected_branches.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/overcommit/hook/pre_push/protected_branches.rb b/lib/overcommit/hook/pre_push/protected_branches.rb index f9c5bc9f..93f77064 100644 --- a/lib/overcommit/hook/pre_push/protected_branches.rb +++ b/lib/overcommit/hook/pre_push/protected_branches.rb @@ -20,7 +20,7 @@ def branches def illegal_pushes @illegal_pushes ||= pushed_refs.select do |pushed_ref| (pushed_ref.deleted? || pushed_ref.forced?) && - branches.any? { |branch| pushed_ref.remote_ref =~ /#{branch}/ } + branches.any? { |branch| pushed_ref.remote_ref == "refs/heads/#{branch}" } end end end