diff --git a/doc/user/project/repository/push_rules.md b/doc/user/project/repository/push_rules.md index 244cb1245f0318c5e564140018396bf606bab8a9..9e4fa7abdea06b38049a0abfd27113986ed8e198 100644 --- a/doc/user/project/repository/push_rules.md +++ b/doc/user/project/repository/push_rules.md @@ -78,6 +78,15 @@ Use these rules for your commit messages. the expression. To allow any commit message, leave empty. Uses multiline mode, which can be disabled by using `(?-m)`. +## Reject commits that aren't DCO certified + +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/98810) in GitLab 15.5. + +Commits signed with the [Developer Certificate of Origin](https://developercertificate.org/) (DCO) +certify the contributor wrote, or has the right to submit, the code contributed in that commit. +You can require all commits to your project to comply with the DCO. This push rule requires a +`Signed-off-by:` trailer in every commit message, and rejects any commits that lack it. + ## Validate branch names To validate your branch names, enter a regular expression for **Branch name**. diff --git a/ee/app/controllers/admin/push_rules_controller.rb b/ee/app/controllers/admin/push_rules_controller.rb index e2efd0042fc70eb99d9d3c041899fe7865e78948..03c8e3084f6b72ec5dc38c223afa6ffd8c0689ec 100644 --- a/ee/app/controllers/admin/push_rules_controller.rb +++ b/ee/app/controllers/admin/push_rules_controller.rb @@ -42,6 +42,10 @@ def push_rule_params allowed_fields << :commit_committer_check end + if @push_rule.available?(:reject_non_dco_commits) + allowed_fields << :reject_non_dco_commits + end + params.require(:push_rule).permit(allowed_fields) end diff --git a/ee/app/controllers/groups/push_rules_controller.rb b/ee/app/controllers/groups/push_rules_controller.rb index ce005af7f9108a71d4040627b936ca63802ac5a1..98f222c2cca955afe7462ce32522cd522c4f96be 100644 --- a/ee/app/controllers/groups/push_rules_controller.rb +++ b/ee/app/controllers/groups/push_rules_controller.rb @@ -42,6 +42,10 @@ def push_rule_params allowed_fields << :commit_committer_check end + if can?(current_user, :change_reject_non_dco_commits, group) + allowed_fields << :reject_non_dco_commits + end + params.require(:push_rule).permit(allowed_fields) end diff --git a/ee/app/controllers/projects/push_rules_controller.rb b/ee/app/controllers/projects/push_rules_controller.rb index 900dab706a0776195baa8fe3b5fbda0ba3f93a3a..22768716c240a39ff4c734cea3dba98768bf7ca7 100644 --- a/ee/app/controllers/projects/push_rules_controller.rb +++ b/ee/app/controllers/projects/push_rules_controller.rb @@ -45,6 +45,10 @@ def push_rule_params allowed_fields << :commit_committer_check end + if can?(current_user, :change_reject_non_dco_commits, project) + allowed_fields << :reject_non_dco_commits + end + params.require(:push_rule).permit(allowed_fields) end end diff --git a/ee/app/helpers/push_rules_helper.rb b/ee/app/helpers/push_rules_helper.rb index b645d93e2007799cf1f6beeebbeaad8d38326582..4c3723ed8a1ddbc9788b596f2fd5393f6176548f 100644 --- a/ee/app/helpers/push_rules_helper.rb +++ b/ee/app/helpers/push_rules_helper.rb @@ -11,6 +11,12 @@ def reject_unsigned_commits_description(push_rule) push_rule_update_description(message, push_rule, :reject_unsigned_commits) end + def reject_non_dco_commits_description(push_rule) + message = format(s_("ProjectSettings|Only commits that include a %{code_block_start}Signed-off-by:%{code_block_end} element can be pushed to this repository.").html_safe, code_block_start: ''.html_safe, code_block_end: ''.html_safe) + + push_rule_update_description(message, push_rule, :reject_non_dco_commits) + end + def commit_committer_check_description(push_rule) message = s_("ProjectSettings|Users can only push commits to this repository "\ "if the committer email is one of their own verified emails.") @@ -39,6 +45,6 @@ def push_rule_update_description(message, push_rule, rule) end end - messages.join(' ') + messages.join(' ').html_safe end end diff --git a/ee/app/models/gitlab_subscriptions/features.rb b/ee/app/models/gitlab_subscriptions/features.rb index 5fb6f3c15a6ad9bf0f45b26995fadc56d2ee85f1..9cb4c90e21d11cde079a8f8b28a0eada411d7ff9 100644 --- a/ee/app/models/gitlab_subscriptions/features.rb +++ b/ee/app/models/gitlab_subscriptions/features.rb @@ -143,6 +143,7 @@ class Features productivity_analytics project_aliases protected_environments + reject_non_dco_commits reject_unsigned_commits saml_group_sync scoped_labels diff --git a/ee/app/models/push_rule.rb b/ee/app/models/push_rule.rb index ff3b8758738fc8bfc5f30deb790350e4aceaf88a..718cb5749c9379923e91cac6401b1bdf27efe187 100644 --- a/ee/app/models/push_rule.rb +++ b/ee/app/models/push_rule.rb @@ -22,6 +22,7 @@ class PushRule < ApplicationRecord AUDIT_LOG_ALLOWLIST = { commit_committer_check: 'reject unverified users', reject_unsigned_commits: 'reject unsigned commits', + reject_non_dco_commits: 'reject non-dco commits', deny_delete_tag: 'do not allow users to remove Git tags with git push', member_check: 'check whether the commit author is a GitLab user', prevent_secrets: 'prevent pushing secret files', @@ -45,8 +46,11 @@ class PushRule < ApplicationRecord SETTINGS_WITH_GLOBAL_DEFAULT = %i[ reject_unsigned_commits commit_committer_check + reject_non_dco_commits ].freeze + DCO_COMMIT_REGEX = 'Signed-off-by:.+<.+@.+>' + def self.global find_by(is_sample: true) end @@ -57,6 +61,7 @@ def commit_validation? branch_name_regex.present? || author_email_regex.present? || reject_unsigned_commits || + reject_non_dco_commits || commit_committer_check || member_check || file_name_regex.present? || @@ -77,6 +82,13 @@ def committer_allowed?(committer_email, current_user) current_user.verified_email?(committer_email) end + def non_dco_commit_allowed?(message) + return true unless available?(:reject_non_dco_commits) + return true unless reject_non_dco_commits + + data_match?(message, DCO_COMMIT_REGEX) + end + def commit_message_allowed?(message) data_match?(message, commit_message_regex, multiline: true) end @@ -132,6 +144,15 @@ def commit_committer_check=(value) write_setting_with_global_default(:commit_committer_check, value) end + def reject_non_dco_commits + read_setting_with_global_default(:reject_non_dco_commits) + end + alias_method :reject_non_dco_commits?, :reject_non_dco_commits + + def reject_non_dco_commits=(value) + write_setting_with_global_default(:reject_non_dco_commits, value) + end + private def data_match?(data, regex, multiline: false) diff --git a/ee/app/policies/ee/group_policy.rb b/ee/app/policies/ee/group_policy.rb index 99cd934d4d62bc9af639f64d9dcacabca04b4e62..38bae7d9c3535ff0713a0a70ec69a1190f3e7b4e 100644 --- a/ee/app/policies/ee/group_policy.rb +++ b/ee/app/policies/ee/group_policy.rb @@ -132,6 +132,10 @@ module GroupPolicy @subject.feature_available?(:reject_unsigned_commits) end + condition(:reject_non_dco_commits_available, scope: :subject) do + @subject.feature_available?(:reject_non_dco_commits) + end + condition(:push_rules_available, scope: :subject) do @subject.feature_available?(:push_rules) end @@ -448,6 +452,12 @@ module GroupPolicy rule { ~reject_unsigned_commits_available }.prevent :change_reject_unsigned_commits + rule { admin | maintainer }.enable :change_reject_non_dco_commits + + rule { reject_non_dco_commits_available }.enable :read_reject_non_dco_commits + + rule { ~reject_non_dco_commits_available }.prevent :change_reject_non_dco_commits + rule { can?(:maintainer_access) & push_rules_available }.enable :change_push_rules rule { admin & is_gitlab_com }.enable :update_subscription_limit diff --git a/ee/app/policies/ee/project_policy.rb b/ee/app/policies/ee/project_policy.rb index 098a1a523f56f7a266068333bac39437c135c308..5607224348290b39a40946f4e1f7190dc66d488e 100644 --- a/ee/app/policies/ee/project_policy.rb +++ b/ee/app/policies/ee/project_policy.rb @@ -77,6 +77,11 @@ module ProjectPolicy @subject.feature_available?(:reject_unsigned_commits) end + with_scope :subject + condition(:reject_non_dco_commits_available) do + @subject.feature_available?(:reject_non_dco_commits) + end + with_scope :subject condition(:security_orchestration_policies_enabled) do @subject.feature_available?(:security_orchestration_policies) @@ -370,6 +375,12 @@ module ProjectPolicy rule { ~commit_committer_check_available }.prevent :change_commit_committer_check + rule { admin | maintainer }.enable :change_reject_non_dco_commits + + rule { reject_non_dco_commits_available }.enable :read_reject_non_dco_commits + + rule { ~reject_non_dco_commits_available }.prevent :change_reject_non_dco_commits + rule { owner | reporter | internal_access | public_project }.enable :build_read_project rule { ~admin & owner & owner_cannot_destroy_project }.prevent :remove_project diff --git a/ee/app/services/ee/merge_requests/merge_base_service.rb b/ee/app/services/ee/merge_requests/merge_base_service.rb index 96921c5aeffaf032999504050cb811bc6db843fc..1675d5988f65802f71c4c9167e1019c868848d88 100644 --- a/ee/app/services/ee/merge_requests/merge_base_service.rb +++ b/ee/app/services/ee/merge_requests/merge_base_service.rb @@ -48,6 +48,8 @@ def hooks_validation_error(merge_request, validate_squash_message: false) "Author's commit email '#{current_user.commit_email_or_default}' does not follow the pattern '#{push_rule.author_email_regex}'" elsif validate_squash_message squash_message_validation_error + elsif !push_rule.non_dco_commit_allowed?(params[:commit_message]) + "Commit message must contain a DCO signoff" end end diff --git a/ee/app/views/shared/push_rules/_form.html.haml b/ee/app/views/shared/push_rules/_form.html.haml index b5cdebdf3a3a84247d758494b393b05b902dc74b..fc61833a2b73c46b15354226e93416bf075ad709 100644 --- a/ee/app/views/shared/push_rules/_form.html.haml +++ b/ee/app/views/shared/push_rules/_form.html.haml @@ -1,5 +1,6 @@ = render 'shared/push_rules/commit_committer_check_setting', form: f, push_rule: f.object, context: context = render 'shared/push_rules/reject_unsigned_commits_setting', form: f, push_rule: f.object, context: context += render 'shared/push_rules/reject_commits_not_dco_signed_setting', form: f, push_rule: f.object, context: context - wiki_syntax_link_url = 'https://github.com/google/re2/wiki/Syntax' - wiki_syntax_link_start = ''.html_safe % { url: wiki_syntax_link_url } diff --git a/ee/app/views/shared/push_rules/_reject_commits_not_dco_signed_setting.haml b/ee/app/views/shared/push_rules/_reject_commits_not_dco_signed_setting.haml new file mode 100644 index 0000000000000000000000000000000000000000..cb7595dd74727cc653820842a0a3590ed9a04e3f --- /dev/null +++ b/ee/app/views/shared/push_rules/_reject_commits_not_dco_signed_setting.haml @@ -0,0 +1,11 @@ +- context = local_assigns.fetch(:context) + +- return unless push_rule.available?(:reject_non_dco_commits, object: context) + +- form = local_assigns.fetch(:form) +- push_rule = local_assigns.fetch(:push_rule) + += form.gitlab_ui_checkbox_component :reject_non_dco_commits, + s_("PushRules|Reject commits that aren't DCO certified"), + checkbox_options: { data: { qa_selector: 'reject_non_dco_commits_checkbox' }, disabled: !can_change_push_rule?(form.object, :reject_non_dco_commits, context) }, + help_text: reject_non_dco_commits_description(push_rule) diff --git a/ee/lib/ee/gitlab/checks/push_rules/commit_check.rb b/ee/lib/ee/gitlab/checks/push_rules/commit_check.rb index ccfad10166274104695d6d67acb443dddc41c919..5edc6afdad1f9a929683724777e8bfa3096eb6a3 100644 --- a/ee/lib/ee/gitlab/checks/push_rules/commit_check.rb +++ b/ee/lib/ee/gitlab/checks/push_rules/commit_check.rb @@ -60,6 +60,10 @@ def check_commit(commit) committer_error_message = committer_check(commit) return committer_error_message if committer_error_message + unless push_rule.non_dco_commit_allowed?(commit.safe_message) + return "Commit message must contain a DCO signoff" + end + if !updated_from_web? && !push_rule.commit_signature_allowed?(commit) return "Commit must be signed with a GPG key" end diff --git a/ee/spec/controllers/admin/push_rules_controller_spec.rb b/ee/spec/controllers/admin/push_rules_controller_spec.rb index 72155f5910da93a1bdf6c4d076fb38c2b1d8e96b..6ba6c3e28e53dcde2d4222e1cd3a9f41bd52455a 100644 --- a/ee/spec/controllers/admin/push_rules_controller_spec.rb +++ b/ee/spec/controllers/admin/push_rules_controller_spec.rb @@ -16,12 +16,14 @@ { deny_delete_tag: "true", delete_branch_regex: "any", commit_message_regex: "any", branch_name_regex: "any", force_push_regex: "any", author_email_regex: "any", member_check: "true", file_name_regex: "any", - max_file_size: "0", prevent_secrets: "true" + max_file_size: "0", prevent_secrets: "true", commit_committer_check: "true", reject_unsigned_commits: "true", + reject_non_dco_commits: "true" } end before do stub_env('IN_MEMORY_APPLICATION_SETTINGS', 'false') + stub_licensed_features(commit_committer_check: true, reject_unsigned_commits: true, reject_non_dco_commits: true) end it 'updates sample push rule' do diff --git a/ee/spec/helpers/push_rules_helper_spec.rb b/ee/spec/helpers/push_rules_helper_spec.rb index 5991a531e992be9fefd7554fc17f6a1d6bd3c4e8..bfb604454006997a46fca43d26adb1aadb251acf 100644 --- a/ee/spec/helpers/push_rules_helper_spec.rb +++ b/ee/spec/helpers/push_rules_helper_spec.rb @@ -11,6 +11,7 @@ { commit_committer_check_base_help: /Users can only push commits to this repository if the committer email is one of their own verified emails/, reject_unsigned_commits_base_help: /Only signed commits can be pushed to this repository/, + reject_non_dco_commits_base_help: %r{Only commits that include a Signed-off-by: element can be pushed to this repository}, default_admin_help: /This setting will be applied to all projects unless overridden by an admin/, setting_can_be_overridden: /This setting is applied on the server level and can be overridden by an admin/, setting_has_been_overridden: /This setting is applied on the server level but has been overridden for this project/, diff --git a/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb b/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb index 564c9e6f931a3ca767dea1f01d7b9098c2337b52..16428e8407802ad9ba0fb288ef4234f43f559a8d 100644 --- a/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb +++ b/ee/spec/lib/ee/gitlab/checks/push_rules/commit_check_spec.rb @@ -38,6 +38,32 @@ end end + context 'DCO check rules' do + let(:push_rule) { create(:push_rule, reject_non_dco_commits: true) } + + before do + stub_licensed_features(reject_non_dco_commits: true) + end + + it_behaves_like 'check ignored when push rule unlicensed' + + context 'when enabled in Project and commit is not DCO signed' do + it 'returns an error' do + expect { subject.validate! }.to raise_error(Gitlab::GitAccess::ForbiddenError, "Commit message must contain a DCO signoff") + end + end + + context 'when enabled in Project and the commit is DCO signed' do + it 'does not return an error' do + commit_message = "DCO Signed Commit\n\nSigned-off-by: Test user " + + allow_any_instance_of(Commit).to receive(:safe_message).and_return(commit_message) + + expect { subject.validate! }.not_to raise_error + end + end + end + context 'author email rules' do let!(:push_rule) { create(:push_rule, author_email_regex: '.*@valid.com') } diff --git a/ee/spec/support/shared_examples/services/merge_merge_requests_shared_examples.rb b/ee/spec/support/shared_examples/services/merge_merge_requests_shared_examples.rb index 8fd331ec83f760410e0263da25159f987c21ee1f..c3638402f92933c2cca17973c16a8c09b3b38e41 100644 --- a/ee/spec/support/shared_examples/services/merge_merge_requests_shared_examples.rb +++ b/ee/spec/support/shared_examples/services/merge_merge_requests_shared_examples.rb @@ -87,6 +87,38 @@ def hooks_pass?(squashing: false) end end + context 'DCO signoff validation' do + before do + stub_licensed_features(reject_non_dco_commits: true) + allow(project).to receive(:push_rule) { build(:push_rule, reject_non_dco_commits: true) } + end + + it_behaves_like 'hook validations are skipped when push rules unlicensed' + + context 'when a non DCO commit message is used' do + it 'returns false and saves error when invalid' do + expect(hooks_pass?).to be(false) + expect(hooks_error).not_to be_empty + + if args[:persisted] + expect(merge_request.merge_error).to eq(hooks_error) + else + expect(merge_request.merge_error).to be_nil + end + end + end + + context 'when a DCO compliant commit message is used' do + let(:dco_commit_message) { 'DCO Signed Commit\n\nSigned-off-by: Test user ' } + let(:params) { super().merge(commit_message: dco_commit_message) } + + it 'accepts the commit message' do + expect(hooks_pass?).to be(true) + expect(hooks_error).to be_nil + end + end + end + context 'fast forward merge request' do it 'returns true when fast forward is enabled' do allow(project).to receive(:merge_requests_ff_only_enabled) { true } diff --git a/locale/gitlab.pot b/locale/gitlab.pot index d912e4536b5b92bdff87be39d1ee03b01cb13c89..e2b940939646ed02475581c6e74395858f4f5e23 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -31677,6 +31677,9 @@ msgstr "" msgid "ProjectSettings|Note: The container registry is always visible when a project is public and the container registry is set to '%{access_level_description}'" msgstr "" +msgid "ProjectSettings|Only commits that include a %{code_block_start}Signed-off-by:%{code_block_end} element can be pushed to this repository." +msgstr "" + msgid "ProjectSettings|Only signed commits can be pushed to this repository." msgstr "" @@ -32808,6 +32811,9 @@ msgstr "" msgid "PushRules|Reject any files likely to contain secrets. %{secret_files_link_start}What secret files are rejected?%{secret_files_link_end}" msgstr "" +msgid "PushRules|Reject commits that aren't DCO certified" +msgstr "" + msgid "PushRules|Reject expression in commit messages" msgstr ""