From 97349b374f08316f17b08fdcca8e9fdf0bf65ae7 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 17 Jun 2022 00:09:23 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .gitlab/ci/review-apps/qa.gitlab-ci.yml | 24 +++++++++++++ .gitlab/ci/rules.gitlab-ci.yml | 4 +++ app/models/merge_request_diff_file.rb | 6 ++-- .../application_settings/_sentry.html.haml | 2 +- app/views/snippets/notes/_actions.html.haml | 5 ++- .../remove_mergeable_state_check.yml | 8 ----- doc/development/contributing/verify/index.md | 20 +++++++++++ .../application_security/dast/checks/601.1.md | 34 +++++++++++++++++++ .../application_security/dast/checks/index.md | 1 + doc/user/group/index.md | 4 ++- doc/user/tasks.md | 25 ++++++++++++-- .../features/sanity/feature_flags_spec.rb | 4 +-- qa/qa/tools/delete_subgroups.rb | 2 +- spec/models/merge_request_diff_file_spec.rb | 8 +++++ 14 files changed, 126 insertions(+), 21 deletions(-) delete mode 100644 config/feature_flags/development/remove_mergeable_state_check.yml create mode 100644 doc/user/application_security/dast/checks/601.1.md diff --git a/.gitlab/ci/review-apps/qa.gitlab-ci.yml b/.gitlab/ci/review-apps/qa.gitlab-ci.yml index bc75bf51e03..07ad5a31135 100644 --- a/.gitlab/ci/review-apps/qa.gitlab-ci.yml +++ b/.gitlab/ci/review-apps/qa.gitlab-ci.yml @@ -52,6 +52,7 @@ include: --tag ~skip_signup_disabled \ --tag ~requires_git_protocol_v2 \ --tag ~requires_praefect \ + --tag ~sanity_feature_flags \ --force-color \ --order random \ --format documentation \ @@ -99,6 +100,29 @@ download-knapsack-report: - qa/knapsack/review-qa-*.json expire_in: 1 day +review-qa-sanity: + extends: + - .review-qa-base + - .review:rules:review-qa-sanity + retry: 1 + variables: + QA_RUN_TYPE: review-qa-sanity + script: + - qa_run_status=0 + - | + bundle exec rake "knapsack:rspec[\ + --tag sanity_feature_flags \ + --force-color \ + --order random \ + --format documentation \ + --format RspecJunitFormatter --out tmp/rspec.xml \ + ]" || qa_run_status=$? + - if [ ${qa_run_status} -ne 0 ]; then + release_sha=$(echo "${CI_MERGE_REQUEST_SOURCE_BRANCH_SHA:-${CI_COMMIT_SHA}}" | cut -c1-11); + echo "Errors can be found at https://sentry.gitlab.net/gitlab/gitlab-review-apps/releases/${release_sha}/all-events/."; + fi + - exit ${qa_run_status} + review-qa-smoke: extends: - .review-qa-base diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index ba64a1d91b1..ccdc2c1b90a 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -1637,6 +1637,10 @@ rules: - when: on_success +.review:rules:review-qa-sanity: + rules: + - when: on_success + .review:rules:review-qa-smoke: rules: - when: on_success diff --git a/app/models/merge_request_diff_file.rb b/app/models/merge_request_diff_file.rb index f3f64971426..f7648937c1d 100644 --- a/app/models/merge_request_diff_file.rb +++ b/app/models/merge_request_diff_file.rb @@ -15,9 +15,11 @@ class MergeRequestDiffFile < ApplicationRecord end def utf8_diff - return '' if diff.blank? + fetched_diff = diff - encode_utf8(diff) if diff.respond_to?(:encoding) + return '' if fetched_diff.blank? + + encode_utf8(fetched_diff) if fetched_diff.respond_to?(:encoding) end def diff diff --git a/app/views/admin/application_settings/_sentry.html.haml b/app/views/admin/application_settings/_sentry.html.haml index cfd34f6ca15..ece8f50151a 100644 --- a/app/views/admin/application_settings/_sentry.html.haml +++ b/app/views/admin/application_settings/_sentry.html.haml @@ -1,5 +1,5 @@ = gitlab_ui_form_for @application_setting, url: metrics_and_profiling_admin_application_settings_path(anchor: 'js-sentry-settings'), html: { class: 'fieldset-form', id: 'sentry-settings' } do |f| - = form_errors(@application_setting) + = form_errors(@application_setting, pajamas_alert: true) %span.text-muted = _('Changing any setting here requires an application restart') diff --git a/app/views/snippets/notes/_actions.html.haml b/app/views/snippets/notes/_actions.html.haml index 80d680d8b5a..e3a14b0454e 100644 --- a/app/views/snippets/notes/_actions.html.haml +++ b/app/views/snippets/notes/_actions.html.haml @@ -1,14 +1,13 @@ - if current_user - if note.emoji_awardable? .note-actions-item - = link_to '#', title: _('Add reaction'), class: "note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip btn gl-button btn-icon btn-default-tertiary", data: { position: 'right' } do + = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { title: _('Add reaction'), class: 'btn-icon note-action-button note-emoji-button js-add-award js-note-emoji has-tooltip' }) do = sprite_icon('slight-smile', css_class: 'award-control-icon-neutral gl-button-icon gl-icon') = sprite_icon('smiley', css_class: 'award-control-icon-positive gl-button-icon gl-icon gl-left-3!') = sprite_icon('smile', css_class: 'award-control-icon-super-positive gl-button-icon gl-icon gl-left-3! ') - if note_editable .note-actions-item.gl-ml-0 - = button_tag title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip gl-button btn btn-default-tertiary btn-icon', data: { container: 'body', qa_selector: 'edit_comment_button' } do - = sprite_icon('pencil') + = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'pencil', button_options: { title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', qa_selector: 'edit_comment_button' } }) = render 'projects/notes/more_actions_dropdown', note: note, note_editable: note_editable diff --git a/config/feature_flags/development/remove_mergeable_state_check.yml b/config/feature_flags/development/remove_mergeable_state_check.yml deleted file mode 100644 index 536b82c2b5d..00000000000 --- a/config/feature_flags/development/remove_mergeable_state_check.yml +++ /dev/null @@ -1,8 +0,0 @@ ---- -name: remove_mergeable_state_check -introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/86612 -rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/362555 -milestone: '15.1' -type: development -group: group::code review -default_enabled: false diff --git a/doc/development/contributing/verify/index.md b/doc/development/contributing/verify/index.md index 203d64d13cf..09b206d59aa 100644 --- a/doc/development/contributing/verify/index.md +++ b/doc/development/contributing/verify/index.md @@ -253,3 +253,23 @@ There are much more likely scenarios that can have disastrous consequences. GitLab CI/CD is being used by companies building medical, aviation, and automotive software. Continuous Integration is a mission critical part of software engineering. + +### Definition of Done + +In Verify, we follow our Development team's [Definition of Done](../merge_request_workflow.md#definition-of-done). +We also want to keep things efficient and [DRY](https://en.wikipedia.org/wiki/Don%27t_repeat_yourself) when we answer questions +and solve problems for our users. + +For any issue that is resolved because the solution is supported with existing `.gitlab-ci.yml` syntax, +create a project in the [`ci-sample-projects`](https://gitlab.com/gitlab-org/ci-sample-projects) group +that demonstrates the solution. + +The project must have: + +- A simple title. +- A clear description. +- A `README.md` with: + - A link to the resolved issue. You should also direct users to collaborate in the + resolved issue if any questions arise. + - A link to any relevant documentation. + - A detailed explanation of what the example is doing. diff --git a/doc/user/application_security/dast/checks/601.1.md b/doc/user/application_security/dast/checks/601.1.md new file mode 100644 index 00000000000..26ccd877104 --- /dev/null +++ b/doc/user/application_security/dast/checks/601.1.md @@ -0,0 +1,34 @@ +--- +stage: Secure +group: Dynamic Analysis +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/engineering/ux/technical-writing/#assignments +--- + +# URL redirection to untrusted site ('open redirect') + +## Description + +This site was found to allow open redirects from user supplied input. Open redirects are commonly +abused in phishing attacks where the original domain or URL looks like a legitimate link, but then +redirects a user to a malicious site. An example would be +`https://example.com/redirect?url=https://%62%61%64%2e%63%6f%6d%2f%66%61%6b%65%6c%6f%67%69%6e` which, +when decoded turns into `bad.com/fakelogin`. + +## Remediation + +Never redirect a client based on user input found in a `GET` request. It is recommended that the list +of target links to redirect a user to are contained server side, and retrieved using a numerical value +as an index to return the link to be redirected to. For example, `/redirect?id=1` would cause the +application to look up the `1` index and return a URL such as `https://example.com`. This URL would +then be used to redirect the user, using the 301 response code and `Location` header. + +## Details + +| ID | Aggregated | CWE | Type | Risk | +|:---|:--------|:--------|:--------|:--------| +| 601.1 | true | 601 | Passive | Low | + +## Links + +- [OWASP](https://owasp.org/www-project-cheat-sheets/cheatsheets/Unvalidated_Redirects_and_Forwards_Cheat_Sheet.html) +- [CWE](https://cwe.mitre.org/data/definitions/601.html) diff --git a/doc/user/application_security/dast/checks/index.md b/doc/user/application_security/dast/checks/index.md index 98994019e46..e2947d5b120 100644 --- a/doc/user/application_security/dast/checks/index.md +++ b/doc/user/application_security/dast/checks/index.md @@ -28,6 +28,7 @@ The [DAST browser-based crawler](../browser_based.md) provides a number of vulne | [598.1](598.1.md) | Use of GET request method with sensitive query strings (session ID) | Medium | Passive | | [598.2](598.2.md) | Use of GET request method with sensitive query strings (password) | Medium | Passive | | [598.3](598.3.md) | Use of GET request method with sensitive query strings (Authorization header details) | Medium | Passive | +| [601.1](601.1.md) | URL redirection to untrusted site ('open redirect') | Low | Passive | | [614.1](614.1.md) | Sensitive cookie without Secure attribute | Low | Passive | | [693.1](693.1.md) | Missing X-Content-Type-Options: nosniff | Low | Passive | | [829.1](829.1.md) | Inclusion of Functionality from Untrusted Control Sphere | Low | Passive | diff --git a/doc/user/group/index.md b/doc/user/group/index.md index 8c116c2de28..c0ae721e3b4 100644 --- a/doc/user/group/index.md +++ b/doc/user/group/index.md @@ -759,7 +759,9 @@ To enable delayed deletion of projects in a group: 1. Scroll to: - (GitLab 15.1 and later) **Deletion protection** and select **Keep deleted projects**. - (GitLab 15.0 and earlier) **Enable delayed project deletion** and tick the checkbox. -1. Optional. To prevent subgroups from changing this setting, select **Enforce for all subgroups**. Renamed to **Enforce deletion protection for all subgroups** in GitLab 15.1. +1. Optional. To prevent subgroups from changing this setting, select: + - (GitLab 15.1 and later), **Enforce deletion protection for all subgroups** + - (GitLab 15.0 and earlier), **Enforce for all subgroups**. 1. Select **Save changes**. NOTE: diff --git a/doc/user/tasks.md b/doc/user/tasks.md index 8be7fbca213..ea56ae70c61 100644 --- a/doc/user/tasks.md +++ b/doc/user/tasks.md @@ -31,7 +31,26 @@ to work items and adding custom work item types, visit [epic 6033](https://gitlab.com/groups/gitlab-org/-/epics/6033) or [Plan direction page](https://about.gitlab.com/direction/plan/). -## View a task +## Create a task -The only way to view a task is to open it with a deep link, -for example: `///-/work_item/1`. +To create a task: + +1. In an issue description, create a [task list](markdown.md#task-lists). +1. Hover over a task item and select **Convert to work item** (**{doc-new}**). +1. Confirm or edit the title, and select **Create work item**. + +## Edit a task + +To edit a task: + +1. In the issue description, view the task links. +1. Select a link. The task is displayed. + - To edit the description, select **Edit**, then select **Save**. + - To edit the title or state, make your changes, then click outside the field. The changes are saved automatically. + +## Delete a task + +To delete a task: + +1. In the issue description, select the task. +1. From the options menu (**{ellipsis_v}**), select **Delete work item**. diff --git a/qa/qa/specs/features/sanity/feature_flags_spec.rb b/qa/qa/specs/features/sanity/feature_flags_spec.rb index 83c0b045730..7e68c70ee09 100644 --- a/qa/qa/specs/features/sanity/feature_flags_spec.rb +++ b/qa/qa/specs/features/sanity/feature_flags_spec.rb @@ -49,7 +49,7 @@ module QA it 'reads as enabled after the flag is enabled' do QA::Runtime::Feature.enable(flag) - expect(QA::Runtime::Feature.enabled?(flag)).to be true + expect { QA::Runtime::Feature.enabled?(flag) }.to eventually_be_truthy end end @@ -63,7 +63,7 @@ module QA it 'reads as disabled after the flag is disabled' do QA::Runtime::Feature.disable(flag) - expect(QA::Runtime::Feature.enabled?(flag)).to be false + expect { QA::Runtime::Feature.enabled?(flag) }.to eventually_be_falsey end end end diff --git a/qa/qa/tools/delete_subgroups.rb b/qa/qa/tools/delete_subgroups.rb index bcbd308ae3f..355bd6bf10d 100644 --- a/qa/qa/tools/delete_subgroups.rb +++ b/qa/qa/tools/delete_subgroups.rb @@ -37,7 +37,7 @@ module QA $stdout.puts "\nDeleting subgroup #{path}..." delete_response = delete(request_url) - dot_or_f = delete_response.code == 404 ? "\e[32m.\e[0m" : "\e[31mF - #{delete_response}\e[0m" + dot_or_f = delete_response.code == 202 ? "\e[32m.\e[0m" : "\e[31mF - #{delete_response}\e[0m" print dot_or_f end end diff --git a/spec/models/merge_request_diff_file_spec.rb b/spec/models/merge_request_diff_file_spec.rb index 5a48438adab..c9bcb900eca 100644 --- a/spec/models/merge_request_diff_file_spec.rb +++ b/spec/models/merge_request_diff_file_spec.rb @@ -85,5 +85,13 @@ RSpec.describe MergeRequestDiffFile do expect { subject.utf8_diff }.not_to raise_error end + + it 'calls #diff once' do + allow(subject).to receive(:diff).and_return('test') + + expect(subject).to receive(:diff).once + + subject.utf8_diff + end end end