Prevent blacklisted files being pushed into repository

CHANGELOG

@@ -7,6 +7,7 @@ v 8.12.0 (unreleased)
   - Add ability to fork to a specific namespace using API. (ritave)
   - Cleanup misalignments in Issue list view !6206
   - Prune events older than 12 months. (ritave)
+  - Prevent secrets to be pushed to the repository
   - Prepend blank line to `Closes` message on merge request linked to issue (lukehowell)
   - Fix issues/merge-request templates dropdown for forked projects
   - Filter tags by name !6121

app/controllers/projects/push_rules_controller.rb

@@ -28,6 +28,6 @@ class Projects::PushRulesController < Projects::ApplicationController
   # Only allow a trusted parameter "white list" through.
   def push_rule_params
     params.require(:push_rule).permit(:deny_delete_tag, :delete_branch_regex,
-      :commit_message_regex, :force_push_regex, :author_email_regex, :member_check, :file_name_regex, :max_file_size)
+      :commit_message_regex, :force_push_regex, :author_email_regex, :member_check, :file_name_regex, :max_file_size, :prevent_secrets)
   end
 end

app/models/push_rule.rb

@@ -4,25 +4,36 @@ class PushRule < ActiveRecord::Base
   validates :project, presence: true, unless: "is_sample?"
   validates :max_file_size, numericality: { greater_than_or_equal_to: 0, only_integer: true }
 
+  FILES_BLACKLIST = YAML.load_file(Rails.root.join('lib/gitlab/checks/files_blacklist.yml'))
+
   def commit_validation?
     commit_message_regex.present? ||
       author_email_regex.present? ||
       member_check ||
       file_name_regex.present? ||
-      max_file_size > 0
+      max_file_size > 0 ||
+      prevent_secrets
   end
 
   def commit_message_allowed?(message)
-    data_valid?(message, commit_message_regex)
+    data_match?(message, commit_message_regex)
   end
 
   def author_email_allowed?(email)
-    data_valid?(email, author_email_regex)
+    data_match?(email, author_email_regex)
+  end
+
+  def filename_blacklisted?(file_path)
+    regex_list = []
+    regex_list.concat(FILES_BLACKLIST) if prevent_secrets
+    regex_list << file_name_regex if file_name_regex
+
+    regex_list.find { |regex| data_match?(file_path, regex) }
   end
 
   private
 
-  def data_valid?(data, regex)
+  def data_match?(data, regex)
     if regex.present?
       !!(data =~ Regexp.new(regex))
     else

app/views/projects/push_rules/index.html.haml

@@ -15,5 +15,4 @@
     = form_for [@project.namespace.becomes(Namespace), @project, @push_rule] do |f|
       = form_errors(@push_rule)
 
-      = render "shared/push_rules_form", f: f,
-        prevent_committing_secrets_check: true
+      = render "shared/push_rules_form", f: f

app/views/shared/_push_rules_form.html.haml

@@ -14,16 +14,16 @@
     %p.light.append-bottom-0
       Restrict commits by author (email) to existing GitLab users
 
-- if local_assigns.fetch(:prevent_committing_secrets_check, false)
-  .form-group
-    = f.check_box :member_check, class: "pull-left"
-    .prepend-left-20
-      = f.label :member_check, "Prevent committing secrets to git", class: "label-light append-bottom-0"
-      %p.light.append-bottom-0
-        We'll reject anything that is likely to contain secrets from your commits.
-        The list of things we reject is available in
-        %a{ href: '/' }
-          the documentation.
+
+.form-group
+  = f.check_box :prevent_secrets, class: "pull-left"
+  .prepend-left-20
+    = f.label :prevent_secrets, "Prevent committing secrets to Git", class: "label-light append-bottom-0"
+    %p.light.append-bottom-0
+      GitLab will reject any files that are likely to contain secrets.
+      The list of file names we reject is available in the
+      = link_to "documentation", help_page_path('push_rules/push_rules')
+      \.
 
 .form-group
   = f.label :commit_message_regex, "Commit message", class: 'label-light'

db/migrate/20160915201649_add_prevent_secrets_to_push_rules.rb

+class AddPreventSecretsToPushRules < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column_with_default(:push_rules, :prevent_secrets, :boolean, default: false)
+  end
+
+  def down
+    remove_column(:push_rules, :prevent_secrets)
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20160913212128) do
+ActiveRecord::Schema.define(version: 20160915201649) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1017,6 +1017,7 @@ ActiveRecord::Schema.define(version: 20160913212128) do
     t.string   "file_name_regex"
     t.boolean  "is_sample",            default: false
     t.integer  "max_file_size",        default: 0,     null: false
+    t.boolean  "prevent_secrets",      default: false, null: false
   end
 
   add_index "push_rules", ["project_id"], name: "index_push_rules_on_project_id", using: :btree

doc/push_rules/push_rules.md

@@ -28,3 +28,70 @@ See the screenshot below:
 
 Now when a user tries to push a commit like `Bugfix` - their push will be declined.
 And pushing commit with message like `Bugfix according to JIRA-123` will be accepted.
+
+
+## Prevent pushing secrets to the repository
+
+You can turn on a predefined blacklist of files which won't be allowed to be pushed to a repository.
+
+By selecting the checkbox *Prevent committing secrets to Git*, GitLab prevents pushes to the repository when a file matches a regular expression as read from `lib/gitlab/checks/files_blacklist.yml` (make sure you are at the right branch as your GitLab version when viewing this file).
+
+Below is the list of what will be rejected by these regular expressions :
+
+```shell
+#####################
+# AWS CLI credential blobs
+#####################
+.aws/credentials
+aws/credentials
+homefolder/aws/credentials
+
+#####################
+# Private RSA SSH keys
+#####################
+/ssh/id_rsa
+/.ssh/personal_rsa
+/config/server_rsa
+id_rsa
+.id_rsa
+
+#####################
+# Private DSA SSH keys
+#####################
+/ssh/id_dsa
+/.ssh/personal_dsa
+/config/server_dsa
+id_dsa
+.id_dsa
+
+#####################
+# Private ed25519 SSH keys
+#####################
+/ssh/id_ed25519
+/.ssh/personal_ed25519
+/config/server_ed25519
+id_ed25519
+.id_ed25519
+
+#####################
+# Private ECDSA SSH keys
+#####################
+/ssh/id_ecdsa
+/.ssh/personal_ecdsa
+/config/server_ecdsa
+id_ecdsa
+.id_ecdsa
+
+#####################
+# Any file with .pem or .key extensions
+#####################
+secret.pem
+private.key
+
+#####################
+# Any file ending with _history or .history extension
+#####################
+pry.history
+bash_history
+
+```

lib/gitlab/checks/change_access.rb

@@ -163,9 +163,7 @@ module Gitlab
 
         return validations unless push_rule
 
-        if push_rule.file_name_regex.present?
-          validations << file_name_validation(push_rule.file_name_regex)
-        end
+        validations << file_name_validation(push_rule)
 
         if push_rule.max_file_size > 0
           validations << file_size_validation(commit, push_rule.max_file_size)
@@ -196,12 +194,12 @@ module Gitlab
         end
       end
 
-      def file_name_validation(file_name_regex)
-        regexp = Regexp.new(file_name_regex)
-
+      def file_name_validation(push_rule)
         lambda do |diff|
-          if (diff.renamed_file || diff.new_file) && diff.new_path =~ regexp
-            return "File name #{diff.new_path.inspect} is prohibited by the pattern '#{file_name_regex}'"
+          if (diff.renamed_file || diff.new_file) && blacklisted_regex = push_rule.filename_blacklisted?(diff.new_path)
+            return nil unless blacklisted_regex.present?
+
+            "File name #{diff.new_path} was blacklisted by the pattern #{blacklisted_regex}."
           end
         end
       end

lib/gitlab/checks/files_blacklist.yml

+# List of regular expressions to prevent secrets
+# being pushed to repository.
+# This list is checked only if project.push_rule.prevent_secrets is true
+# Any changes to this file should be documented in: doc/push_rules/push_rules.md
+
+# AWS CLI credential blobs
+
+- aws\/credentials$
+
+# RSA DSA ECSDA and ED25519 SSH keys
+
+- (ssh|config)\/(personal|server)_(rsa|dsa|ed\d+|ecdsa)
+- id_rsa$
+- id_dsa$
+- id_ed25519$
+- id_ecdsa$
+
+# privatekey.pem and secret.key
+
+- \.(pem|key)$
+
+# files ending in .history or _history
+- "[._]history$"
+

spec/lib/gitlab/checks/change_access_spec.rb

@@ -171,11 +171,66 @@ describe Gitlab::Checks::ChangeAccess, lib: true do
 
       context 'file name rules' do
         # Notice that the commit used creates a file named 'README'
-        let(:push_rule) { create(:push_rule, file_name_regex: 'READ*') }
+        context 'file name regex check' do
+          let(:push_rule) { create(:push_rule, file_name_regex: 'READ*') }
 
-        it "returns an error if a new or renamed filed doesn't match the file name regex" do
-          expect(subject.status).to be(false)
-          expect(subject.message).to eq("File name \"README\" is prohibited by the pattern 'READ*'")
+          it "returns an error if a new or renamed filed doesn't match the file name regex" do
+            expect(subject.status).to be(false)
+            expect(subject.message).to eq("File name README was blacklisted by the pattern READ*.")
+          end
+        end
+
+        context 'blacklisted files check' do
+          let(:push_rule) { create(:push_rule, prevent_secrets: true) }
+          let(:checker) { described_class.new(changes, project: project, user_access: user_access) }
+
+          it "returns status true if there is no blacklisted files" do
+            new_rev = nil
+
+            white_listed =
+              [
+                'readme.txt', 'any/ida_rsa.pub', 'any/id_dsa.pub', 'any_2/id_ed25519.pub',
+                'random_file.pdf', 'folder/id_ecdsa.pub', 'docs/aws/credentials.md', 'ending_withhistory'
+              ]
+
+            white_listed.each do |file_path|
+              old_rev = 'be93687618e4b132087f430a4d8fc3a609c9b77c'
+              old_rev = new_rev if new_rev
+              new_rev = project.repository.commit_file(user, file_path, "commit #{file_path}", "commit #{file_path}", "master", false)
+
+              allow(project.repository).to receive(:new_commits).and_return(
+                project.repository.commits_between(old_rev, new_rev)
+              )
+
+              expect(checker.exec.status).to be(true)
+            end
+          end
+
+          it "returns an error if a new or renamed filed doesn't match the file name regex" do
+            new_rev = nil
+
+            black_listed =
+              [
+                'aws/credentials', '.ssh/personal_rsa', 'config/server_rsa', '.ssh/id_rsa', '.ssh/id_dsa',
+                '.ssh/personal_dsa', 'config/server_ed25519', 'any/id_ed25519', '.ssh/personal_ecdsa', 'config/server_ecdsa',
+                'any_place/id_ecdsa', 'some_pLace/file.key', 'other_PlAcE/other_file.pem', 'bye_bug.history, pg_sql_history'
+              ]
+
+            black_listed.each do |file_path|
+              old_rev = 'be93687618e4b132087f430a4d8fc3a609c9b77c'
+              old_rev = new_rev if new_rev
+              new_rev = project.repository.commit_file(user, file_path, "commit #{file_path}", "commit #{file_path}", "master", false)
+
+              allow(project.repository).to receive(:new_commits).and_return(
+                project.repository.commits_between(old_rev, new_rev)
+              )
+
+              result = checker.exec
+
+              expect(result.status).to be(false)
+              expect(result.message).to include("File name #{file_path} was blacklisted by the pattern")
+            end
+          end
         end
       end