diff --git a/danger/specs/Dangerfile b/danger/specs/Dangerfile index 35476ae645d..df4e324a4c8 100644 --- a/danger/specs/Dangerfile +++ b/danger/specs/Dangerfile @@ -32,11 +32,22 @@ request specs (and/or feature specs). Please add request specs under See https://gitlab.com/groups/gitlab-org/-/epics/5076 for information. MSG -has_app_changes = helper.all_changed_files.grep(%r{\A(app|lib|db/(geo/)?(post_)?migrate)/}).any? -has_ee_app_changes = helper.all_changed_files.grep(%r{\Aee/(app|lib|db/(geo/)?(post_)?migrate)/}).any? -spec_changes = helper.all_changed_files.grep(%r{\Aspec/}) +MATCH_WITH_ARRAY_REGEX = /(?match|eq)(?[( ]?\[)/.freeze + +SUGGEST_MR_COMMENT = <<~SUGGEST_COMMENT +```suggestion +%s + +If order of the result is not important, please consider using `match_array` to avoid flakiness. +``` +SUGGEST_COMMENT + +all_changed_files = helper.all_changed_files +has_app_changes = all_changed_files.grep(%r{\A(app|lib|db/(geo/)?(post_)?migrate)/}).any? +has_ee_app_changes = all_changed_files.grep(%r{\Aee/(app|lib|db/(geo/)?(post_)?migrate)/}).any? +spec_changes = all_changed_files.grep(%r{\Aspec/}) has_spec_changes = spec_changes.any? -has_ee_spec_changes = helper.all_changed_files.grep(%r{\Aee/spec/}).any? +has_ee_spec_changes = all_changed_files.grep(%r{\Aee/spec/}).any? new_specs_needed = (gitlab.mr_labels & NO_SPECS_LABELS).empty? if (has_app_changes || has_ee_app_changes) && !(has_spec_changes || has_ee_spec_changes) && new_specs_needed @@ -49,6 +60,22 @@ if has_ee_app_changes && has_spec_changes && !(has_app_changes || has_ee_spec_ch end # Forbidding a new file addition under `/spec/controllers` or `/ee/spec/controllers` -if git.added_files.grep(%r{^(ee/)?spec/controllers/}).any? +if project_helper.changes.added.files.grep(%r{^(ee/)?spec/controllers/}).any? warn CONTROLLER_SPEC_DEPRECATION_MESSAGE end + +def check_for_match_with_array! + (project_helper.changes.added.files + project_helper.changes.modified.files + project_helper.changes.renamed_after.files).grep(%r{\A(ee/)?spec/}).each do |filename| + added_lines = helper.changed_lines(filename).grep(/\A\+ /) + next unless added_lines.any? { |line| line =~ MATCH_WITH_ARRAY_REGEX } + + spec_file_lines = File.read(filename).lines(chomp: true) + + added_lines.each do |added_line| + mr_line = spec_file_lines.find_index(added_line.delete_prefix('+')) + markdown(format(SUGGEST_MR_COMMENT, suggested_line: spec_file_lines[mr_line].gsub(MATCH_WITH_ARRAY_REGEX, 'match_array\k')), file: filename, line: mr_line.succ) + end + end +end + +check_for_match_with_array! diff --git a/doc/api/vulnerability_findings.md b/doc/api/vulnerability_findings.md index dfc6074a1aa..36604ebf87d 100644 --- a/doc/api/vulnerability_findings.md +++ b/doc/api/vulnerability_findings.md @@ -74,77 +74,66 @@ Example response: [ { "id": null, - "report_type": "dependency_scanning", - "name": "Authentication bypass via incorrect DOM traversal and canonicalization in saml2-js", - "severity": "unknown", - "confidence": "undefined", + "report_type": "sast", + "name": "Possible command injection", + "severity": "high", + "confidence": "high", "scanner": { - "external_id": "gemnasium", - "name": "Gemnasium" + "external_id": "brakeman", + "name": "Brakeman", + "vendor": "GitLab" }, "identifiers": [ { - "external_type": "gemnasium", - "external_id": "9952e574-7b5b-46fa-a270-aeb694198a98", - "name": "Gemnasium-9952e574-7b5b-46fa-a270-aeb694198a98", - "url": "https://deps.sec.gitlab.com/packages/npm/saml2-js/versions/1.5.0/advisories" - }, - { - "external_type": "cve", - "external_id": "CVE-2017-11429", - "name": "CVE-2017-11429", - "url": "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2017-11429" + "external_type": "brakeman_warning_code", + "external_id": "14", + "name": "Brakeman Warning Code 14", + "url": "https://brakemanscanner.org/docs/warning_types/command_injection/" } ], - "project_fingerprint": "fa6f5b6c5d240b834ac5e901dc69f9484cef89ec", - "uuid": "31f483bc-bfc0-586d-9b92-f1015c4535b8", - "create_vulnerability_feedback_issue_path": "/tests/yarn-remediation-test/vulnerability_feedback", - "create_vulnerability_feedback_merge_request_path": "/tests/yarn-remediation-test/vulnerability_feedback", - "create_vulnerability_feedback_dismissal_path": "/tests/yarn-remediation-test/vulnerability_feedback", + "project_fingerprint": "ac218b1770af030cfeef967752ab803c55afb36d", + "uuid": "ad5e3be3-a193-55f5-a200-bc12865fb09c", + "create_jira_issue_url": null, + "false_positive": true, + "create_vulnerability_feedback_issue_path": "/root/test-false-positive/-/vulnerability_feedback", + "create_vulnerability_feedback_merge_request_path": "/root/test-false-positive/-/vulnerability_feedback", + "create_vulnerability_feedback_dismissal_path": "/root/test-false-positive/-/vulnerability_feedback", "project": { - "id": 31, - "name": "yarn-remediation-test", - "full_path": "/tests/yarn-remediation-test", - "full_name": "tests / yarn-remediation-test" + "id": 2, + "name": "Test False Positive", + "full_path": "/root/test-false-positive", + "full_name": "Administrator / Test False Positive" }, "dismissal_feedback": null, "issue_feedback": null, "merge_request_feedback": null, - "description": "Some XML DOM traversal and canonicalization APIs may be inconsistent in handling of comments within XML nodes. Incorrect use of these APIs by some SAML libraries results in incorrect parsing of the inner text of XML nodes such that any inner text after the comment is lost prior to cryptographically signing the SAML message. Text after the comment therefore has no impact on the signature on the SAML message.\r\n\r\nA remote attacker can modify SAML content for a SAML service provider without invalidating the cryptographic signature, which may allow attackers to bypass primary authentication for the affected SAML service provider.", - "links": [ - { - "url": "https://github.com/Clever/saml2/commit/3546cb61fd541f219abda364c5b919633609ef3d#diff-af730f9f738de1c9ad87596df3f6de84R279" - }, - { - "url": "https://www.kb.cert.org/vuls/id/475445" - }, - { - "url": "https://github.com/Clever/saml2/issues/127" - } - ], + "description": null, + "links": [], "location": { - "file": "yarn.lock", - "dependency": { - "package": { - "name": "saml2-js" - }, - "version": "1.5.0" - } + "file": "app/controllers/users_controller.rb", + "start_line": 42, + "class": "UsersController", + "method": "list_users" }, - "details": { - "custom_field": { - "name": "URLs", - "type": "list", - "items": [ - { - "type": "url", - "href": "http://site.com/page/1" - } - ] - } + "remediations": [ + null + ], + "solution": null, + "evidence": null, + "request": null, + "response": null, + "evidence_source": null, + "supporting_messages": [], + "assets": [], + "details": {}, + "state": "detected", + "scan": { + "type": "sast", + "status": "success", + "start_time": "2021-09-02T20:55:48", + "end_time": "2021-09-02T20:55:48" }, - "solution": "Upgrade to fixed version.\r\n", - "blob_path": "/tests/yarn-remediation-test/blob/cc6c4a0778460455ae5d16ca7025ca9ca1ca75ac/yarn.lock" + "blob_path": "/root/test-false-positive/-/blob/dfd75607752a839bbc9c7362d111effaa470fecd/app/controllers/users_controller.rb#L42" } ] ``` diff --git a/doc/user/compliance/license_compliance/img/policies_maintainer_add_v13_2.png b/doc/user/compliance/license_compliance/img/policies_maintainer_add_v13_2.png deleted file mode 100644 index 2d5946503cf..00000000000 Binary files a/doc/user/compliance/license_compliance/img/policies_maintainer_add_v13_2.png and /dev/null differ diff --git a/doc/user/compliance/license_compliance/img/policies_maintainer_add_v14_3.png b/doc/user/compliance/license_compliance/img/policies_maintainer_add_v14_3.png new file mode 100644 index 00000000000..7a27899f8c9 Binary files /dev/null and b/doc/user/compliance/license_compliance/img/policies_maintainer_add_v14_3.png differ diff --git a/doc/user/compliance/license_compliance/img/policies_maintainer_edit_v14_2.png b/doc/user/compliance/license_compliance/img/policies_maintainer_edit_v14_2.png deleted file mode 100644 index 2ad08919f86..00000000000 Binary files a/doc/user/compliance/license_compliance/img/policies_maintainer_edit_v14_2.png and /dev/null differ diff --git a/doc/user/compliance/license_compliance/img/policies_maintainer_edit_v14_3.png b/doc/user/compliance/license_compliance/img/policies_maintainer_edit_v14_3.png new file mode 100644 index 00000000000..85b2a52e04b Binary files /dev/null and b/doc/user/compliance/license_compliance/img/policies_maintainer_edit_v14_3.png differ diff --git a/doc/user/compliance/license_compliance/index.md b/doc/user/compliance/license_compliance/index.md index 165150a58a1..e8e2452f919 100644 --- a/doc/user/compliance/license_compliance/index.md +++ b/doc/user/compliance/license_compliance/index.md @@ -47,7 +47,7 @@ When GitLab detects a **Denied** license, you can view it in the [license list]( ![License List](img/license_list_v13_0.png) You can view and modify existing policies from the [policies](#policies) tab. -![Edit Policy](img/policies_maintainer_edit_v14_2.png) +![Edit Policy](img/policies_maintainer_edit_v14_3.png) ## License expressions @@ -742,8 +742,9 @@ which enables a designated approver that can approve and then merge a merge requ The **Policies** tab in the project's license compliance section displays your project's license policies. Project maintainers can specify policies in this section. -![Edit Policy](img/policies_maintainer_edit_v14_2.png) -![Add Policy](img/policies_maintainer_add_v13_2.png) +![Edit Policy](img/policies_maintainer_edit_v14_3.png) + +![Add Policy](img/policies_maintainer_add_v14_3.png) Developers of the project can view the policies configured in a project. diff --git a/qa/qa/resource/project.rb b/qa/qa/resource/project.rb index 5ad55090f8c..01cb68d64bf 100644 --- a/qa/qa/resource/project.rb +++ b/qa/qa/resource/project.rb @@ -12,13 +12,13 @@ module QA :auto_devops_enabled, :github_personal_access_token, :github_repository_path, - :gitlab_repository_path + :gitlab_repository_path, + :personal_namespace attributes :id, :name, :add_name_uuid, :description, - :personal_namespace, :runners_token, :visibility, :template_name, @@ -31,13 +31,15 @@ module QA end attribute :path_with_namespace do - "#{group.full_path}/#{name}" + "#{personal_namespace || group.full_path}/#{name}" end alias_method :full_path, :path_with_namespace def sandbox_path - group.respond_to?('sandbox') ? "#{group.sandbox.path}/" : '' + return '' if personal_namespace || !group.respond_to?('sandbox') + + "#{group.sandbox.path}/" end attribute :repository_ssh_location do @@ -50,12 +52,12 @@ module QA def initialize @add_name_uuid = true - @personal_namespace = false @description = 'My awesome project' @initialize_with_readme = false @auto_devops_enabled = false @visibility = :public @template_name = nil + @personal_namespace = nil @import = false self.name = "the_awesome_project" @@ -68,7 +70,7 @@ module QA def fabricate! return if @import - if @personal_namespace + if personal_namespace Page::Dashboard::Projects.perform(&:click_new_project_button) else group.visit! diff --git a/qa/qa/resource/project_imported_from_github.rb b/qa/qa/resource/project_imported_from_github.rb index cffeed7a64b..28a0f12b3e3 100644 --- a/qa/qa/resource/project_imported_from_github.rb +++ b/qa/qa/resource/project_imported_from_github.rb @@ -34,7 +34,7 @@ module QA def fabricate_via_api! super rescue ResourceURLMissingError - "#{Runtime::Scenario.gitlab_address}/#{group.full_path}/#{name}" + "#{Runtime::Scenario.gitlab_address}/#{full_path}" end def api_post_path @@ -49,7 +49,7 @@ module QA { repo_id: github_repo_id, new_name: name, - target_namespace: group.full_path, + target_namespace: @personal_namespace || group.full_path, personal_access_token: github_personal_access_token, ci_cd_only: false } diff --git a/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb b/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb index b51a79f239c..3a2f960d812 100644 --- a/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb +++ b/qa/qa/specs/features/api/1_manage/import_large_github_repo_spec.rb @@ -9,11 +9,6 @@ module QA let(:differ) { RSpec::Support::Differ.new(color: true) } let(:api_client) { Runtime::API::Client.as_admin } - let(:group) do - Resource::Group.fabricate_via_api! do |resource| - resource.api_client = api_client - end - end let(:user) do Resource::User.fabricate_via_api! do |resource| @@ -86,19 +81,15 @@ module QA Resource::ProjectImportedFromGithub.fabricate_via_api! do |project| project.add_name_uuid = false project.name = 'imported-project' - project.group = group project.github_personal_access_token = Runtime::Env.github_access_token project.github_repository_path = github_repo + project.personal_namespace = user.username project.api_client = api_client end end - before do - group.add_member(user, Resource::Members::AccessLevel::MAINTAINER) - end - after do |example| - user.remove_via_api! + user.remove_via_api! unless example.exception next unless defined?(@import_time) # save data for comparison after run finished @@ -128,7 +119,10 @@ module QA ) end - it 'imports large Github repo via api', testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1880' do + it( + 'imports large Github repo via api', + testcase: 'https://gitlab.com/gitlab-org/quality/testcases/-/quality/test_cases/1880' + ) do start = Time.now Runtime::Logger.info("Importing project '#{imported_project.full_path}'") # import the project and log path diff --git a/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb b/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb index caaa615149d..db75d4639ff 100644 --- a/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb +++ b/qa/qa/specs/features/api/3_create/repository/project_archive_compare_spec.rb @@ -46,10 +46,9 @@ module QA def create_project(user, api_client, project_name) project = Resource::Project.fabricate_via_api! do |project| - project.personal_namespace = true + project.personal_namespace = user.username project.add_name_uuid = false project.name = project_name - project.path_with_namespace = "#{user.username}/#{project_name}" project.api_client = api_client end diff --git a/qa/qa/specs/features/browser_ui/1_manage/project/create_project_spec.rb b/qa/qa/specs/features/browser_ui/1_manage/project/create_project_spec.rb index 974d9b02f4d..e41d87612de 100644 --- a/qa/qa/specs/features/browser_ui/1_manage/project/create_project_spec.rb +++ b/qa/qa/specs/features/browser_ui/1_manage/project/create_project_spec.rb @@ -39,7 +39,7 @@ module QA Resource::Project.fabricate_via_browser_ui! do |project| project.name = project_name project.description = 'create awesome project test' - project.personal_namespace = true + project.personal_namespace = Runtime::User.username end end diff --git a/qa/qa/support/api.rb b/qa/qa/support/api.rb index 205ddf7ad3a..302527f4029 100644 --- a/qa/qa/support/api.rb +++ b/qa/qa/support/api.rb @@ -8,63 +8,93 @@ module QA HTTP_STATUS_NO_CONTENT = 204 HTTP_STATUS_ACCEPTED = 202 HTTP_STATUS_NOT_FOUND = 404 + HTTP_STATUS_TOO_MANY_REQUESTS = 429 HTTP_STATUS_SERVER_ERROR = 500 def post(url, payload, args = {}) - default_args = { - method: :post, - url: url, - payload: payload, - verify_ssl: false - } + with_retry_on_too_many_requests do + default_args = { + method: :post, + url: url, + payload: payload, + verify_ssl: false + } - RestClient::Request.execute( - default_args.merge(args) - ) - rescue RestClient::ExceptionWithResponse => e - return_response_or_raise(e) + RestClient::Request.execute( + default_args.merge(args) + ) + rescue RestClient::ExceptionWithResponse => e + return_response_or_raise(e) + end end def get(url, args = {}) - default_args = { - method: :get, - url: url, - verify_ssl: false - } + with_retry_on_too_many_requests do + default_args = { + method: :get, + url: url, + verify_ssl: false + } - RestClient::Request.execute( - default_args.merge(args) - ) - rescue RestClient::ExceptionWithResponse => e - return_response_or_raise(e) + RestClient::Request.execute( + default_args.merge(args) + ) + rescue RestClient::ExceptionWithResponse => e + return_response_or_raise(e) + end end def put(url, payload = nil) - RestClient::Request.execute( - method: :put, - url: url, - payload: payload, - verify_ssl: false) - rescue RestClient::ExceptionWithResponse => e - return_response_or_raise(e) + with_retry_on_too_many_requests do + RestClient::Request.execute( + method: :put, + url: url, + payload: payload, + verify_ssl: false) + rescue RestClient::ExceptionWithResponse => e + return_response_or_raise(e) + end end def delete(url) - RestClient::Request.execute( - method: :delete, - url: url, - verify_ssl: false) - rescue RestClient::ExceptionWithResponse => e - return_response_or_raise(e) + with_retry_on_too_many_requests do + RestClient::Request.execute( + method: :delete, + url: url, + verify_ssl: false) + rescue RestClient::ExceptionWithResponse => e + return_response_or_raise(e) + end end def head(url) - RestClient::Request.execute( - method: :head, - url: url, - verify_ssl: false) - rescue RestClient::ExceptionWithResponse => e - return_response_or_raise(e) + with_retry_on_too_many_requests do + RestClient::Request.execute( + method: :head, + url: url, + verify_ssl: false) + rescue RestClient::ExceptionWithResponse => e + return_response_or_raise(e) + end + end + + def with_retry_on_too_many_requests + response = nil + + Support::Retrier.retry_until do + response = yield + + if response.code == HTTP_STATUS_TOO_MANY_REQUESTS + wait_seconds = response.headers[:retry_after].to_i + QA::Runtime::Logger.debug("Received 429 - Too many requests. Waiting for #{wait_seconds} seconds.") + + sleep wait_seconds + end + + response.code != HTTP_STATUS_TOO_MANY_REQUESTS + end + + response end def parse_body(response)