diff options
77 files changed, 1339 insertions, 365 deletions
diff --git a/.gitlab/ci/database.gitlab-ci.yml b/.gitlab/ci/database.gitlab-ci.yml index b21df6e50c6..32de6c68926 100644 --- a/.gitlab/ci/database.gitlab-ci.yml +++ b/.gitlab/ci/database.gitlab-ci.yml @@ -51,7 +51,7 @@ db:check-migrations-single-db: extends: - db:check-migrations - .single-db - - .rails:rules:single-db + - .rails:rules:db:check-migrations-single-db db:post_deployment_migrations_validator: extends: @@ -66,7 +66,7 @@ db:post_deployment_migrations_validator-single-db: extends: - db:post_deployment_migrations_validator - .single-db - - .rails:rules:single-db + - .rails:rules:db:check-migrations-single-db db:migrate-non-superuser: extends: diff --git a/.gitlab/ci/rails/shared.gitlab-ci.yml b/.gitlab/ci/rails/shared.gitlab-ci.yml index b95a4c45f1e..3f9a43c1ced 100644 --- a/.gitlab/ci/rails/shared.gitlab-ci.yml +++ b/.gitlab/ci/rails/shared.gitlab-ci.yml @@ -107,7 +107,12 @@ include: - .rspec-base - .as-if-foss - .use-pg12 - needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets as-if-foss", "detect-tests"] + needs: + - job: "setup-test-env" + - job: "retrieve-tests-metadata" + - job: "compile-test-assets as-if-foss" + - job: "detect-tests" + optional: true .rspec-base-pg13: extends: diff --git a/.gitlab/ci/rules.gitlab-ci.yml b/.gitlab/ci/rules.gitlab-ci.yml index d16dd28f597..975554f59e3 100644 --- a/.gitlab/ci/rules.gitlab-ci.yml +++ b/.gitlab/ci/rules.gitlab-ci.yml @@ -1205,6 +1205,12 @@ changes: *db-patterns - <<: *if-default-branch-schedule-nightly +.rails:rules:db:check-migrations-single-db: + rules: + - <<: *if-merge-request-labels-run-single-db + - <<: *if-merge-request + changes: *db-patterns + .rails:rules:db-backup: rules: - <<: *if-merge-request-labels-run-all-rspec @@ -2191,7 +2197,7 @@ - <<: *if-not-ee when: never - <<: *if-dot-com-ee-schedule-default-branch-maintenance - - <<: *if-default-refs + - <<: *if-default-branch-refs changes: - ".gitlab/ci/setup.gitlab-ci.yml" - ".gitlab/ci/test-metadata.gitlab-ci.yml" @@ -2213,7 +2219,7 @@ - <<: *if-not-ee when: never - <<: *if-dot-com-ee-schedule-default-branch-maintenance - - <<: *if-default-refs + - <<: *if-default-branch-refs changes: - ".gitlab/ci/test-metadata.gitlab-ci.yml" - "scripts/rspec_helpers.sh" diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f06c5517d9..d8e13f337e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,16 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.8.1 (2023-01-30) + +### Security (5 changes) + +- [Remove parameter validation for registry notification request [15.8]](gitlab-org/security/gitlab@bf5a28cc21ffa3e7b63eeca02f220c1312314f75) ([merge request](gitlab-org/security/gitlab!3028)) +- [Add size validation for Chart.yaml during file extraction](gitlab-org/security/gitlab@f4afa319cffded561731c117c808969b5261ca52) ([merge request](gitlab-org/security/gitlab!3018)) +- [Prevent default branches from storing paths](gitlab-org/security/gitlab@a906e14f6891e84cfe854be960266adc7f0f6092) ([merge request](gitlab-org/security/gitlab!3011)) +- [Validate Issuable description max length on update](gitlab-org/security/gitlab@312fbac888d0452d9beb9d6545b22972b7e1f09d) ([merge request](gitlab-org/security/gitlab!3004)) +- [Security fix dynamic child pipeline zip extraction](gitlab-org/security/gitlab@ea09503c67eb1eb1f17ea49b7748543d2676e393) ([merge request](gitlab-org/security/gitlab!3007)) + ## 15.8.0 (2023-01-20) ### Added (120 changes) @@ -470,6 +480,16 @@ entry. - [Do not use _test when not necessary](gitlab-org/gitlab@1bde73aba2bd1d7f9e833c7325cffa0c90d1c106) ([merge request](gitlab-org/gitlab!107373)) - [Add config/redis.yml unified config file](gitlab-org/gitlab@ace8301236eecc07a511975b57f80e21ec7be3c2) ([merge request](gitlab-org/gitlab!106854)) +## 15.7.6 (2023-01-30) + +### Security (5 changes) + +- [Remove parameter validation for registry notification request [15.7]](gitlab-org/security/gitlab@ca091312787e3a01f39551357095409fcf6fc840) ([merge request](gitlab-org/security/gitlab!3029)) +- [Add size validation for Chart.yaml during file extraction](gitlab-org/security/gitlab@d43bf6a5b147350668e538bd981af5f9400e6fca) ([merge request](gitlab-org/security/gitlab!3019)) +- [Prevent default branches from storing paths](gitlab-org/security/gitlab@9f18aa40711e334b172d208255a4b396b89c50a9) ([merge request](gitlab-org/security/gitlab!3012)) +- [Validate Issuable description max length on update](gitlab-org/security/gitlab@9c2799bef06ada7d48b682fa4fe403ec00e74c4a) ([merge request](gitlab-org/security/gitlab!3003)) +- [Security fix dynamic child pipeline zip extraction](gitlab-org/security/gitlab@cada7e3290da854f437538cdc1680c3f5284278e) ([merge request](gitlab-org/security/gitlab!2980)) + ## 15.7.5 (2023-01-12) No changes. @@ -1320,6 +1340,21 @@ No changes. - [Propagate RemoteIP to Gitaly via Workhorse](gitlab-org/gitlab@71da945c85931bac0263c193902dc1b54e2e62da) ([merge request](gitlab-org/gitlab!103635)) - [Documentation to reflect 100MB upload limit](gitlab-org/gitlab@33063bb26ab7699802ecb2b325cc8619d6fe7b86) ([merge request](gitlab-org/gitlab!103978)) +## 15.6.7 (2023-01-30) + +### Fixed (2 changes) + +- [Clear DuplicateJobs cookies from post-deployment migration](gitlab-org/security/gitlab@9071bc623c81f4ecbccb63bcfc78d6d503421e2b) +- [Geo: Container Repository push events don't work](gitlab-org/security/gitlab@00ca7dd923444da0b19afa7d72d5e3b505889290) + +### Security (5 changes) + +- [Quarantine features/users/login_spec line 292 [15.6]](gitlab-org/security/gitlab@d202f35e1cac8df0bcbb5d40d42cea2312c09762) ([merge request](gitlab-org/security/gitlab!3025)) +- [Add size validation for Chart.yaml during file extraction](gitlab-org/security/gitlab@59df02bf2658468f9f254c34ed009a6414d6c6b3) ([merge request](gitlab-org/security/gitlab!3020)) +- [Prevent default branches from storing paths](gitlab-org/security/gitlab@b7b402a0a37bb839b601569a035a62fe79febe72) ([merge request](gitlab-org/security/gitlab!3013)) +- [Validate Issuable description max length on update](gitlab-org/security/gitlab@fa68365e853a5701b217ccafea9885705d4a4133) ([merge request](gitlab-org/security/gitlab!3002)) +- [Security fix dynamic child pipeline zip extraction](gitlab-org/security/gitlab@2285d716f10f33d8dbea5112de95d9d7e5cd8b00) ([merge request](gitlab-org/security/gitlab!2981)) + ## 15.6.6 (2023-01-12) No changes. diff --git a/app/assets/javascripts/analytics/shared/constants.js b/app/assets/javascripts/analytics/shared/constants.js index c62736d55a8..a82633035b5 100644 --- a/app/assets/javascripts/analytics/shared/constants.js +++ b/app/assets/javascripts/analytics/shared/constants.js @@ -1,5 +1,6 @@ import { masks } from '~/lib/dateformat'; import { s__ } from '~/locale'; +import { helpPagePath } from '~/helpers/help_page_helper'; export const DATE_RANGE_LIMIT = 180; export const PROJECTS_PER_PAGE = 50; @@ -12,8 +13,101 @@ export const dateFormats = { month: 'mmmm', }; -// Some content is duplicated due to backward compatibility. -// It will be removed with https://gitlab.com/gitlab-org/gitlab/-/issues/350614 in 14.9 +export const KEY_METRICS = { + LEAD_TIME: 'lead_time', + CYCLE_TIME: 'cycle_time', + ISSUES: 'issues', + COMMITS: 'commits', + DEPLOYS: 'deploys', +}; + +export const DORA_METRICS = { + DEPLOYMENT_FREQUENCY: 'deployment_frequency', + LEAD_TIME_FOR_CHANGES: 'lead_time_for_changes', + TIME_TO_RESTORE_SERVICE: 'time_to_restore_service', + CHANGE_FAILURE_RATE: 'change_failure_rate', +}; + +export const VSA_METRICS_GROUPS = [ + { + key: 'key_metrics', + title: s__('ValueStreamAnalytics|Key metrics'), + keys: Object.values(KEY_METRICS), + }, + { + key: 'dora_metrics', + title: s__('ValueStreamAnalytics|DORA metrics'), + keys: Object.values(DORA_METRICS), + }, +]; + +export const METRIC_TOOLTIPS = { + [DORA_METRICS.DEPLOYMENT_FREQUENCY]: { + description: s__( + 'ValueStreamAnalytics|Average number of deployments to production per day. This metric measures how often value is delivered to end users.', + ), + groupLink: '-/analytics/ci_cd?tab=deployment-frequency', + projectLink: '-/pipelines/charts?chart=deployment-frequency', + docsLink: helpPagePath('user/analytics/dora_metrics', { anchor: 'deployment-frequency' }), + }, + [DORA_METRICS.LEAD_TIME_FOR_CHANGES]: { + description: s__( + 'ValueStreamAnalytics|The time to successfully deliver a commit into production. This metric reflects the efficiency of CI/CD pipelines.', + ), + groupLink: '-/analytics/ci_cd?tab=lead-time', + projectLink: '-/pipelines/charts?chart=lead-time', + docsLink: helpPagePath('user/analytics/dora_metrics', { anchor: 'lead-time-for-changes' }), + }, + [DORA_METRICS.TIME_TO_RESTORE_SERVICE]: { + description: s__( + 'ValueStreamAnalytics|The time it takes an organization to recover from a failure in production.', + ), + groupLink: '-/analytics/ci_cd?tab=time-to-restore-service', + projectLink: '-/pipelines/charts?chart=time-to-restore-service', + docsLink: helpPagePath('user/analytics/dora_metrics', { anchor: 'time-to-restore-service' }), + }, + [DORA_METRICS.CHANGE_FAILURE_RATE]: { + description: s__( + 'ValueStreamAnalytics|Percentage of deployments that cause an incident in production.', + ), + groupLink: '-/analytics/ci_cd?tab=change-failure-rate', + projectLink: '-/pipelines/charts?chart=change-failure-rate', + docsLink: helpPagePath('user/analytics/dora_metrics', { anchor: 'change-failure-rate' }), + }, + [KEY_METRICS.LEAD_TIME]: { + description: s__('ValueStreamAnalytics|Median time from issue created to issue closed.'), + groupLink: '-/analytics/value_stream_analytics', + projectLink: '-/value_stream_analytics', + docsLink: helpPagePath('user/analytics/value_stream_analytics', { + anchor: 'view-the-lead-time-and-cycle-time-for-issues', + }), + }, + [KEY_METRICS.CYCLE_TIME]: { + description: s__( + "ValueStreamAnalytics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed.", + ), + groupLink: '-/analytics/value_stream_analytics', + projectLink: '-/value_stream_analytics', + docsLink: helpPagePath('user/analytics/value_stream_analytics', { + anchor: 'view-the-lead-time-and-cycle-time-for-issues', + }), + }, + [KEY_METRICS.ISSUES]: { + description: s__('ValueStreamAnalytics|Number of new issues created.'), + groupLink: '-/issues_analytics', + projectLink: '-/analytics/issues_analytics', + docsLink: helpPagePath('user/analytics/issue_analytics'), + }, + [KEY_METRICS.DEPLOYS]: { + description: s__('ValueStreamAnalytics|Total number of deploys to production.'), + groupLink: '-/analytics/productivity_analytics', + projectLink: '-/analytics/merge_request_analytics', + docsLink: helpPagePath('user/analytics/merge_request_analytics'), + }, +}; + +// TODO: Remove this once the migration to METRIC_TOOLTIPS is complete +// https://gitlab.com/gitlab-org/gitlab/-/issues/388067 export const METRICS_POPOVER_CONTENT = { lead_time: { description: s__('ValueStreamAnalytics|Median time from issue created to issue closed.'), @@ -47,19 +141,3 @@ export const METRICS_POPOVER_CONTENT = { ), }, }; - -const KEY_METRICS_TITLE = s__('ValueStreamAnalytics|Key metrics'); -const KEY_METRICS_KEYS = ['lead_time', 'cycle_time', 'issues', 'commits', 'deploys']; - -const DORA_METRICS_TITLE = s__('ValueStreamAnalytics|DORA metrics'); -const DORA_METRICS_KEYS = [ - 'deployment_frequency', - 'lead_time_for_changes', - 'time_to_restore_service', - 'change_failure_rate', -]; - -export const VSA_METRICS_GROUPS = [ - { key: 'key_metrics', title: KEY_METRICS_TITLE, keys: KEY_METRICS_KEYS }, - { key: 'dora_metrics', title: DORA_METRICS_TITLE, keys: DORA_METRICS_KEYS }, -]; diff --git a/app/assets/javascripts/behaviors/shortcuts.js b/app/assets/javascripts/behaviors/shortcuts.js index 7352be0dbd5..12fdb2e2981 100644 --- a/app/assets/javascripts/behaviors/shortcuts.js +++ b/app/assets/javascripts/behaviors/shortcuts.js @@ -28,7 +28,10 @@ export default function initPageShortcuts() { // TODO: replace this whitelist with something more automated/maintainable if (page && !pagesWithCustomShortcuts.includes(page)) { import(/* webpackChunkName: 'shortcutsBundle' */ './shortcuts/shortcuts') - .then(({ default: Shortcuts }) => new Shortcuts()) + .then(({ default: Shortcuts }) => { + const shortcuts = new Shortcuts(); + window.toggleShortcutsHelp = shortcuts.onToggleHelp; + }) .catch(() => {}); } return false; diff --git a/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue b/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue index b8719d0cd4d..430498e7194 100644 --- a/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue +++ b/app/assets/javascripts/frequent_items/components/frequent_items_list_item.vue @@ -1,6 +1,5 @@ <script> import { GlButton, GlTooltipDirective, GlIcon } from '@gitlab/ui'; -import { snakeCase } from 'lodash'; import SafeHtml from '~/vue_shared/directives/safe_html'; import highlight from '~/lib/utils/highlight'; import { truncateNamespace } from '~/lib/utils/text_utility'; @@ -61,7 +60,7 @@ export default { return highlight(this.itemName, this.matcher); }, itemTrackingLabel() { - return `${this.dropdownType}_dropdown_frequent_items_list_item_${snakeCase(this.itemName)}`; + return `${this.dropdownType}_dropdown_frequent_items_list_item`; }, }, methods: { diff --git a/app/assets/javascripts/issuable/issuable_bulk_update_sidebar.js b/app/assets/javascripts/issuable/issuable_bulk_update_sidebar.js index 095da60a583..9c891bcfc9e 100644 --- a/app/assets/javascripts/issuable/issuable_bulk_update_sidebar.js +++ b/app/assets/javascripts/issuable/issuable_bulk_update_sidebar.js @@ -1,9 +1,9 @@ /* eslint-disable class-methods-use-this, no-new */ - import $ from 'jquery'; import issuableEventHub from '~/issues/list/eventhub'; import LabelsSelect from '~/labels/labels_select'; import { + mountAssigneesDropdown, mountMilestoneDropdown, mountMoveIssuesButton, mountStatusDropdown, @@ -64,6 +64,7 @@ export default class IssuableBulkUpdateSidebar { mountMoveIssuesButton(); mountStatusDropdown(); mountSubscriptionsDropdown(); + mountAssigneesDropdown(); // Checking IS_EE and using ee_else_ce is odd, but we do it here to satisfy // the import/no-unresolved lint rule when FOSS_ONLY=1, even though at diff --git a/app/assets/javascripts/issues/list/components/issues_list_app.vue b/app/assets/javascripts/issues/list/components/issues_list_app.vue index 22e9dd6a5c5..28908478e02 100644 --- a/app/assets/javascripts/issues/list/components/issues_list_app.vue +++ b/app/assets/javascripts/issues/list/components/issues_list_app.vue @@ -592,9 +592,6 @@ export default { const bulkUpdateSidebar = await import('~/issuable'); bulkUpdateSidebar.initBulkUpdateSidebar('issuable_'); - const UsersSelect = (await import('~/users_select')).default; - new UsersSelect(); // eslint-disable-line no-new - this.hasInitBulkEdit = true; } diff --git a/app/assets/javascripts/layout_nav.js b/app/assets/javascripts/layout_nav.js index 90c1b31286a..b8138f34d45 100644 --- a/app/assets/javascripts/layout_nav.js +++ b/app/assets/javascripts/layout_nav.js @@ -56,7 +56,11 @@ function initDeferred() { if (!appEl) return; setNotification(appEl); - document.querySelector('.js-whats-new-trigger').addEventListener('click', () => { + + const triggerEl = document.querySelector('.js-whats-new-trigger'); + if (!triggerEl) return; + + triggerEl.addEventListener('click', () => { import(/* webpackChunkName: 'whatsNewApp' */ '~/whats_new') .then(({ default: initWhatsNew }) => { initWhatsNew(appEl); diff --git a/app/assets/javascripts/sidebar/mount_sidebar.js b/app/assets/javascripts/sidebar/mount_sidebar.js index facc07e8ce5..bef4c6af6ab 100644 --- a/app/assets/javascripts/sidebar/mount_sidebar.js +++ b/app/assets/javascripts/sidebar/mount_sidebar.js @@ -1,21 +1,22 @@ import Vue from 'vue'; import VueApollo from 'vue-apollo'; import { TYPE_ISSUE, TYPE_MERGE_REQUEST } from '~/graphql_shared/constants'; -import { convertToGraphQLId } from '~/graphql_shared/utils'; +import { convertToGraphQLId, getIdFromGraphQLId } from '~/graphql_shared/utils'; import initInviteMembersModal from '~/invite_members/init_invite_members_modal'; import initInviteMembersTrigger from '~/invite_members/init_invite_members_trigger'; import { IssuableType } from '~/issues/constants'; import { gqlClient } from '~/issues/list/graphql'; import { - isInIssuePage, isInDesignPage, isInIncidentPage, + isInIssuePage, isInMRPage, parseBoolean, } from '~/lib/utils/common_utils'; import { __ } from '~/locale'; import { apolloProvider } from '~/graphql_shared/issuable_client'; import Translate from '~/vue_shared/translate'; +import UserSelect from '~/vue_shared/components/user_select/user_select.vue'; import CollapsedAssigneeList from './components/assignees/collapsed_assignee_list.vue'; import SidebarAssignees from './components/assignees/sidebar_assignees.vue'; import SidebarAssigneesWidget from './components/assignees/sidebar_assignees_widget.vue'; @@ -725,6 +726,70 @@ export function mountMoveIssueButton() { }); } +export function mountAssigneesDropdown() { + const el = document.querySelector('.js-assignee-dropdown'); + const assigneeIdsInput = document.querySelector('.js-assignee-ids-input'); + + if (!el || !assigneeIdsInput) { + return null; + } + + const { fullPath } = el.dataset; + const currentUser = { + id: gon?.current_user_id, + username: gon?.current_username, + name: gon?.current_user_fullname, + avatarUrl: gon?.current_user_avatar_url, + }; + + return new Vue({ + el, + apolloProvider, + data() { + return { + selectedUserName: '', + value: [], + }; + }, + methods: { + onSelectedUnassigned() { + assigneeIdsInput.value = 0; + this.value = []; + this.selectedUserName = __('Unassigned'); + }, + onSelected(selected) { + assigneeIdsInput.value = selected.map((user) => getIdFromGraphQLId(user.id)); + this.value = selected; + this.selectedUserName = selected.map((user) => user.name).join(', '); + }, + }, + render(h) { + const component = this; + + return h(UserSelect, { + props: { + text: component.selectedUserName || __('Select assignee'), + headerText: __('Assign to'), + fullPath, + currentUser, + value: component.value, + }, + on: { + input(selected) { + if (!selected.length) { + component.onSelectedUnassigned(); + return; + } + + component.onSelected(selected); + }, + }, + class: 'gl-w-full', + }); + }, + }); +} + const isAssigneesWidgetShown = (isInIssuePage() || isInDesignPage() || isInMRPage()) && gon.features.issueAssigneesWidget; diff --git a/app/assets/javascripts/super_sidebar/components/bottom_bar.vue b/app/assets/javascripts/super_sidebar/components/bottom_bar.vue deleted file mode 100644 index fea29458f45..00000000000 --- a/app/assets/javascripts/super_sidebar/components/bottom_bar.vue +++ /dev/null @@ -1,24 +0,0 @@ -<script> -import { GlIcon } from '@gitlab/ui'; -import { __ } from '~/locale'; - -export default { - components: { - GlIcon, - }, - i18n: { - help: __('Help'), - new: __('New'), - }, -}; -</script> - -<template> - <div class="bottom-links gl-p-3"> - <a href="#" class="gl-text-black-normal" - ><gl-icon name="question-o" class="gl-mr-3 gl-text-gray-300 gl-text-black-normal!" />{{ - $options.i18n.help - }}</a - > - </div> -</template> diff --git a/app/assets/javascripts/super_sidebar/components/help_center.vue b/app/assets/javascripts/super_sidebar/components/help_center.vue new file mode 100644 index 00000000000..6c82a4bbf86 --- /dev/null +++ b/app/assets/javascripts/super_sidebar/components/help_center.vue @@ -0,0 +1,88 @@ +<script> +import { GlDisclosureDropdown } from '@gitlab/ui'; +import { helpPagePath } from '~/helpers/help_page_helper'; +import { PROMO_URL } from 'jh_else_ce/lib/utils/url_utility'; +import { __ } from '~/locale'; + +export default { + components: { + GlDisclosureDropdown, + }, + i18n: { + help: __('Help'), + support: __('Support'), + docs: __('GitLab documentation'), + plans: __('Compare GitLab plans'), + forum: __('Community forum'), + contribute: __('Contribute to GitLab'), + feedback: __('Provide feedback'), + shortcuts: __('Keyboard shortcuts'), + whatsnew: __("What's new"), + }, + props: { + sidebarData: { + type: Object, + required: true, + }, + }, + data() { + return { + items: [ + { + items: [ + { text: this.$options.i18n.help, href: helpPagePath() }, + { text: this.$options.i18n.support, href: this.sidebarData.support_path }, + { text: this.$options.i18n.docs, href: 'https://docs.gitlab.com' }, + { text: this.$options.i18n.plans, href: `${PROMO_URL}/pricing` }, + { text: this.$options.i18n.forum, href: 'https://forum.gitlab.com/' }, + { + text: this.$options.i18n.contribute, + href: helpPagePath('', { anchor: 'contributing-to-gitlab' }), + }, + { text: this.$options.i18n.feedback, href: 'https://about.gitlab.com/submit-feedback' }, + ], + }, + { + items: [ + { text: this.$options.i18n.shortcuts, action: this.showKeyboardShortcuts }, + this.sidebarData.display_whats_new && { + text: this.$options.i18n.whatsnew, + action: this.showWhatsNew, + }, + ].filter(Boolean), + }, + ], + }; + }, + methods: { + showKeyboardShortcuts() { + this.$refs.dropdown.close(); + window?.toggleShortcutsHelp(); + }, + async showWhatsNew() { + this.$refs.dropdown.close(); + if (!this.toggleWhatsNewDrawer) { + const appEl = document.getElementById('whats-new-app'); + const { default: toggleWhatsNewDrawer } = await import( + /* webpackChunkName: 'whatsNewApp' */ '~/whats_new' + ); + this.toggleWhatsNewDrawer = toggleWhatsNewDrawer; + this.toggleWhatsNewDrawer(appEl); + } else { + this.toggleWhatsNewDrawer(); + } + }, + }, +}; +</script> + +<template> + <gl-disclosure-dropdown + ref="dropdown" + icon="question-o" + :items="items" + :toggle-text="$options.i18n.help" + category="tertiary" + no-caret + /> +</template> diff --git a/app/assets/javascripts/super_sidebar/components/super_sidebar.vue b/app/assets/javascripts/super_sidebar/components/super_sidebar.vue index 78d761e9a59..c4b769dcf24 100644 --- a/app/assets/javascripts/super_sidebar/components/super_sidebar.vue +++ b/app/assets/javascripts/super_sidebar/components/super_sidebar.vue @@ -4,7 +4,7 @@ import { context } from '../mock_data'; import UserBar from './user_bar.vue'; import ContextSwitcherToggle from './context_switcher_toggle.vue'; import ContextSwitcher from './context_switcher.vue'; -import BottomBar from './bottom_bar.vue'; +import HelpCenter from './help_center.vue'; export default { context, @@ -13,7 +13,7 @@ export default { UserBar, ContextSwitcherToggle, ContextSwitcher, - BottomBar, + HelpCenter, }, props: { sidebarData: { @@ -32,7 +32,7 @@ export default { <template> <aside id="super-sidebar" - class="super-sidebar gl-fixed gl-bottom-0 gl-left-0 gl-display-flex gl-flex-direction-column gl-bg-gray-10 gl-border-r gl-border-gray-a-08 gl-z-index-9999" + class="super-sidebar gl-fixed gl-bottom-0 gl-left-0 gl-display-flex gl-flex-direction-column gl-bg-gray-10 gl-border-r gl-border-gray-a-08" data-testid="super-sidebar" > <user-bar :sidebar-data="sidebarData" /> @@ -43,8 +43,8 @@ export default { <context-switcher /> </gl-collapse> </div> - <div class="gl-px-3"> - <bottom-bar /> + <div class="gl-p-3"> + <help-center :sidebar-data="sidebarData" /> </div> </div> </aside> diff --git a/app/assets/javascripts/vue_shared/components/user_select/user_select.vue b/app/assets/javascripts/vue_shared/components/user_select/user_select.vue index 86a99b8f0ed..0c8cd7fd95c 100644 --- a/app/assets/javascripts/vue_shared/components/user_select/user_select.vue +++ b/app/assets/javascripts/vue_shared/components/user_select/user_select.vue @@ -2,11 +2,11 @@ import { debounce } from 'lodash'; import { GlDropdown, - GlDropdownForm, GlDropdownDivider, + GlDropdownForm, GlDropdownItem, - GlSearchBoxByType, GlLoadingIcon, + GlSearchBoxByType, GlTooltipDirective, } from '@gitlab/ui'; import { __ } from '~/locale'; @@ -47,7 +47,8 @@ export default { }, iid: { type: String, - required: true, + required: false, + default: null, }, value: { type: Array, @@ -167,13 +168,10 @@ export default { return this.$apollo.queries.searchUsers.loading || this.$apollo.queries.participants.loading; }, users() { - if (!this.participants) { - return []; - } - - const filteredParticipants = this.participants.filter( - (user) => user.name.includes(this.search) || user.username.includes(this.search), - ); + const filteredParticipants = + this.participants?.filter( + (user) => user.name.includes(this.search) || user.username.includes(this.search), + ) || []; // TODO this de-duplication is temporary (BE fix required) // https://gitlab.com/gitlab-org/gitlab/-/issues/327822 @@ -254,6 +252,10 @@ export default { this.$emit('input', selected); } }, + unassign() { + this.$emit('input', []); + this.$refs.dropdown.hide(); + }, unselect(name) { const selected = this.value.filter((user) => user.username !== name); this.$emit('input', selected); @@ -323,7 +325,7 @@ export default { :is-checked="selectedIsEmpty" is-check-centered data-testid="unassign" - @click.native.capture.stop="$emit('input', [])" + @click.native.capture.stop="unassign" > <span :class="selectedIsEmpty ? 'gl-pl-0' : 'gl-pl-6'" class="gl-font-weight-bold">{{ $options.i18n.unassigned diff --git a/app/assets/javascripts/whats_new/utils/notification.js b/app/assets/javascripts/whats_new/utils/notification.js index 41aff202f48..f9b725ed429 100644 --- a/app/assets/javascripts/whats_new/utils/notification.js +++ b/app/assets/javascripts/whats_new/utils/notification.js @@ -5,6 +5,8 @@ export const getVersionDigest = (appEl) => appEl.dataset.versionDigest; export const setNotification = (appEl) => { const versionDigest = getVersionDigest(appEl); const notificationEl = document.querySelector('.header-help'); + if (!notificationEl) return; + let notificationCountEl = notificationEl.querySelector('.js-whats-new-notification-count'); if (localStorage.getItem(STORAGE_KEY) === versionDigest) { diff --git a/app/assets/stylesheets/framework/super_sidebar.scss b/app/assets/stylesheets/framework/super_sidebar.scss index 59a9df9ede0..575fbc03f46 100644 --- a/app/assets/stylesheets/framework/super_sidebar.scss +++ b/app/assets/stylesheets/framework/super_sidebar.scss @@ -1,6 +1,7 @@ .super-sidebar { top: 0; width: $contextual-sidebar-width; + z-index: 600; .user-bar { background-color: $t-gray-a-04; diff --git a/app/helpers/sidebars_helper.rb b/app/helpers/sidebars_helper.rb index 5a19ce6149a..2ca6a46906c 100644 --- a/app/helpers/sidebars_helper.rb +++ b/app/helpers/sidebars_helper.rb @@ -42,7 +42,9 @@ module SidebarsHelper assigned_open_merge_requests_count: user.assigned_open_merge_requests_count, todos_pending_count: user.todos_pending_count, issues_dashboard_path: issues_dashboard_path(assignee_username: user.username), - create_new_menu_groups: create_new_menu_groups(group: group, project: project) + create_new_menu_groups: create_new_menu_groups(group: group, project: project), + support_path: support_url, + display_whats_new: display_whats_new? } end diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 9f0cd96a8f8..50696c7b5e1 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -92,10 +92,9 @@ module Issuable validates :author, presence: true validates :title, presence: true, length: { maximum: TITLE_LENGTH_MAX } - # we validate the description against DESCRIPTION_LENGTH_MAX only for Issuables being created - # to avoid breaking the existing Issuables which may have their descriptions longer - validates :description, length: { maximum: DESCRIPTION_LENGTH_MAX }, allow_blank: true, on: :create - validate :description_max_length_for_new_records_is_valid, on: :update + # we validate the description against DESCRIPTION_LENGTH_MAX only for Issuables being created and on updates if + # the description changes to avoid breaking the existing Issuables which may have their descriptions longer + validates :description, bytesize: { maximum: -> { DESCRIPTION_LENGTH_MAX } }, if: :validate_description_length? validate :validate_assignee_size_length, unless: :importing? before_validation :truncate_description_on_import! @@ -229,10 +228,14 @@ module Issuable private - def description_max_length_for_new_records_is_valid - if new_record? && description.length > Issuable::DESCRIPTION_LENGTH_MAX - errors.add(:description, :too_long, count: Issuable::DESCRIPTION_LENGTH_MAX) - end + def validate_description_length? + return false unless description_changed? + + previous_description = changes['description'].first + # previous_description will be nil for new records + return true if previous_description.blank? + + previous_description.bytesize <= DESCRIPTION_LENGTH_MAX end def truncate_description_on_import! diff --git a/app/models/concerns/sanitizable.rb b/app/models/concerns/sanitizable.rb index 05756beb404..653d7a4875d 100644 --- a/app/models/concerns/sanitizable.rb +++ b/app/models/concerns/sanitizable.rb @@ -45,6 +45,15 @@ module Sanitizable unless input.to_s == CGI.unescapeHTML(input.to_s) record.errors.add(attr, 'cannot contain escaped HTML entities') end + + # This method raises an exception on failure so perform this + # last if multiple errors should be returned. + Gitlab::Utils.check_path_traversal!(input.to_s) + + rescue Gitlab::Utils::DoubleEncodingError + record.errors.add(attr, 'cannot contain escaped components') + rescue Gitlab::Utils::PathTraversalAttackError + record.errors.add(attr, "cannot contain a path traversal component") end end end diff --git a/app/models/namespace_setting.rb b/app/models/namespace_setting.rb index 7f65fb3a378..aeb4d7a5694 100644 --- a/app/models/namespace_setting.rb +++ b/app/models/namespace_setting.rb @@ -14,10 +14,11 @@ class NamespaceSetting < ApplicationRecord validates :enabled_git_access_protocol, inclusion: { in: enabled_git_access_protocols.keys } - validate :default_branch_name_content validate :allow_mfa_for_group validate :allow_resource_access_token_creation_for_group + sanitizes! :default_branch_name + before_validation :normalize_default_branch_name chronic_duration_attr :runner_token_expiration_interval_human_readable, :runner_token_expiration_interval @@ -45,8 +46,6 @@ class NamespaceSetting < ApplicationRecord NAMESPACE_SETTINGS_PARAMS end - sanitizes! :default_branch_name - def prevent_sharing_groups_outside_hierarchy return super if namespace.root? @@ -85,14 +84,6 @@ class NamespaceSetting < ApplicationRecord self.default_branch_name = default_branch_name.presence end - def default_branch_name_content - return if default_branch_name.nil? - - if default_branch_name.blank? - errors.add(:default_branch_name, "can not be an empty string") - end - end - def allow_mfa_for_group if namespace&.subgroup? && allow_mfa_for_subgroups == false errors.add(:allow_mfa_for_subgroups, _('is not allowed since the group is not top-level group.')) diff --git a/app/services/packages/helm/extract_file_metadata_service.rb b/app/services/packages/helm/extract_file_metadata_service.rb index e7373d8ea8f..77efa65f1d1 100644 --- a/app/services/packages/helm/extract_file_metadata_service.rb +++ b/app/services/packages/helm/extract_file_metadata_service.rb @@ -7,6 +7,10 @@ module Packages class ExtractFileMetadataService ExtractionError = Class.new(StandardError) + # Charts must be smaller than 1M because of the storage limitations of Kubernetes objects. + # based on https://helm.sh/docs/chart_template_guide/accessing_files/ + MAX_FILE_SIZE = 1.megabytes.freeze + def initialize(package_file) @package_file = package_file end @@ -42,6 +46,7 @@ module Packages end raise ExtractionError, 'Chart.yaml not found within a directory' unless chart_yaml + raise ExtractionError, 'Chart.yaml too big' if chart_yaml.size > MAX_FILE_SIZE chart_yaml.read ensure diff --git a/app/views/admin/application_settings/appearances/_form.html.haml b/app/views/admin/application_settings/appearances/_form.html.haml index 5f51e91436c..c20a86b9f9c 100644 --- a/app/views/admin/application_settings/appearances/_form.html.haml +++ b/app/views/admin/application_settings/appearances/_form.html.haml @@ -72,7 +72,7 @@ = f.hidden_field :logo_cache = f.file_field :logo, class: "", accept: 'image/*' .form-text.text-muted - = _('Maximum file size is 1MB. Pages are optimized for a 640x360 px logo.') + = _('Maximum file size is 1 MB. Pages are optimized for a 128x128 px logo.') %hr .row diff --git a/app/views/layouts/_page.html.haml b/app/views/layouts/_page.html.haml index 8e39ef7301d..d2ed70d6b48 100644 --- a/app/views/layouts/_page.html.haml +++ b/app/views/layouts/_page.html.haml @@ -4,6 +4,10 @@ - if show_super_sidebar? - sidebar_data = super_sidebar_context(current_user, group: @group, project: @project).to_json %aside.js-super-sidebar.nav-sidebar{ data: { root_path: root_path, sidebar: sidebar_data, toggle_new_nav_endpoint: profile_preferences_url } } + + - if display_whats_new? + #whats-new-app{ data: { version_digest: whats_new_version_digest } } + - elsif defined?(nav) && nav = render "layouts/nav/sidebar/#{nav}" .content-wrapper.content-wrapper-margin{ class: "#{@content_wrapper_class}" } diff --git a/app/views/shared/issuable/_bulk_update_sidebar.html.haml b/app/views/shared/issuable/_bulk_update_sidebar.html.haml index da8477f4b2e..b125fe34464 100644 --- a/app/views/shared/issuable/_bulk_update_sidebar.html.haml +++ b/app/views/shared/issuable/_bulk_update_sidebar.html.haml @@ -20,9 +20,8 @@ .title = _('Assignee') .filter-item - - field_name = "update[assignee_ids][]" - = dropdown_tag(_("Select assignee"), options: { toggle_class: "js-user-search js-update-assignee js-filter-submit js-filter-bulk-update", title: _("Assign to"), filter: true, dropdown_class: "dropdown-menu-user dropdown-menu-selectable", - placeholder: _("Search authors"), data: { first_user: (current_user.username if current_user), null_user: true, current_user: true, project_id: @project.id, field_name: field_name } }) + %input.js-assignee-ids-input{ type: "hidden", name: "update[assignee_ids][]" } + .js-assignee-dropdown{ data: { full_path: @project.full_path } } - if is_issue = render_if_exists 'shared/issuable/epic_dropdown', parent: @project.group .block diff --git a/db/post_migrate/20230127123947_add_fk_index_to_ci_sources_pipelines_on_source_partition_id_and_source_job_id.rb b/db/post_migrate/20230127123947_add_fk_index_to_ci_sources_pipelines_on_source_partition_id_and_source_job_id.rb new file mode 100644 index 00000000000..f2a9d362cc4 --- /dev/null +++ b/db/post_migrate/20230127123947_add_fk_index_to_ci_sources_pipelines_on_source_partition_id_and_source_job_id.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class AddFkIndexToCiSourcesPipelinesOnSourcePartitionIdAndSourceJobId < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + INDEX_NAME = :index_ci_sources_pipelines_on_source_partition_id_source_job_id + TABLE_NAME = :ci_sources_pipelines + COLUMNS = [:source_partition_id, :source_job_id] + + def up + add_concurrent_index(TABLE_NAME, COLUMNS, name: INDEX_NAME) + end + + def down + remove_concurrent_index_by_name(TABLE_NAME, INDEX_NAME) + end +end diff --git a/db/post_migrate/20230127123948_add_fk_to_ci_sources_pipelines_on_source_partition_id_and_source_job_id.rb b/db/post_migrate/20230127123948_add_fk_to_ci_sources_pipelines_on_source_partition_id_and_source_job_id.rb new file mode 100644 index 00000000000..7adf97268ff --- /dev/null +++ b/db/post_migrate/20230127123948_add_fk_to_ci_sources_pipelines_on_source_partition_id_and_source_job_id.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +class AddFkToCiSourcesPipelinesOnSourcePartitionIdAndSourceJobId < Gitlab::Database::Migration[2.1] + disable_ddl_transaction! + + SOURCE_TABLE_NAME = :ci_sources_pipelines + TARGET_TABLE_NAME = :ci_builds + COLUMN = :source_job_id + TARGET_COLUMN = :id + FK_NAME = :fk_be5624bf37_p + PARTITION_COLUMN = :source_partition_id + + def up + add_concurrent_foreign_key( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + column: [PARTITION_COLUMN, COLUMN], + target_column: [:partition_id, TARGET_COLUMN], + validate: false, + reverse_lock_order: true, + on_update: :cascade, + on_delete: :cascade, + name: FK_NAME + ) + end + + def down + with_lock_retries do + remove_foreign_key_if_exists( + SOURCE_TABLE_NAME, + TARGET_TABLE_NAME, + name: FK_NAME, + reverse_lock_order: true + ) + end + end +end diff --git a/db/schema_migrations/20230127123947 b/db/schema_migrations/20230127123947 new file mode 100644 index 00000000000..cc3981f1302 --- /dev/null +++ b/db/schema_migrations/20230127123947 @@ -0,0 +1 @@ +dd57ab40a4263df49d8f52f8f737c5cc62101f932752cbb984cb6072d766d6f0
\ No newline at end of file diff --git a/db/schema_migrations/20230127123948 b/db/schema_migrations/20230127123948 new file mode 100644 index 00000000000..0fa5c113d57 --- /dev/null +++ b/db/schema_migrations/20230127123948 @@ -0,0 +1 @@ +aec7695c7e1cd2eb61625c1c08f7d8ee955bd729a8d70ea2753afcb7b545bfe6
\ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index 6475c5e7b9b..3aba342d1db 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -29283,6 +29283,8 @@ CREATE INDEX index_ci_sources_pipelines_on_project_id ON ci_sources_pipelines US CREATE INDEX index_ci_sources_pipelines_on_source_job_id ON ci_sources_pipelines USING btree (source_job_id); +CREATE INDEX index_ci_sources_pipelines_on_source_partition_id_source_job_id ON ci_sources_pipelines USING btree (source_partition_id, source_job_id); + CREATE INDEX index_ci_sources_pipelines_on_source_pipeline_id ON ci_sources_pipelines USING btree (source_pipeline_id); CREATE INDEX index_ci_sources_pipelines_on_source_project_id ON ci_sources_pipelines USING btree (source_project_id); @@ -34153,6 +34155,9 @@ ALTER TABLE ONLY snippets ALTER TABLE ONLY ci_sources_pipelines ADD CONSTRAINT fk_be5624bf37 FOREIGN KEY (source_job_id) REFERENCES ci_builds(id) ON DELETE CASCADE; +ALTER TABLE ONLY ci_sources_pipelines + ADD CONSTRAINT fk_be5624bf37_p FOREIGN KEY (source_partition_id, source_job_id) REFERENCES ci_builds(partition_id, id) ON UPDATE CASCADE ON DELETE CASCADE NOT VALID; + ALTER TABLE ONLY packages_maven_metadata ADD CONSTRAINT fk_be88aed360 FOREIGN KEY (package_id) REFERENCES packages_packages(id) ON DELETE CASCADE; diff --git a/doc/api/groups.md b/doc/api/groups.md index dd3f8baf338..c39b8584e93 100644 --- a/doc/api/groups.md +++ b/doc/api/groups.md @@ -1201,7 +1201,8 @@ GET /groups?search=foobar > Introduced in GitLab 14.8. -Get a list of users provisioned by a given group. Does not include users provisioned by subgroups. +Get a list of users provisioned by a given group. Does not include subgroups. +Users in this list are considered [enterprise users](../user/enterprise_user/index.md). Requires at least the Maintainer role on the group. diff --git a/doc/api/members.md b/doc/api/members.md index 2da84866b92..4032ab1d651 100644 --- a/doc/api/members.md +++ b/doc/api/members.md @@ -29,8 +29,7 @@ In GitLab 14.8 and earlier, projects in personal namespaces have an `access_leve The `group_saml_identity` attribute is only visible to a group owner for [SSO enabled groups](../user/group/saml_sso/index.md). -The `email` attribute is only visible to group owners when the user was provisioned by the group. -Users are provisioned by the group when the account was created via [SCIM](../user/group/saml_sso/scim_setup.md) or by first sign-in with [SAML SSO for GitLab.com groups](../user/group/saml_sso/index.md). +The `email` attribute is only visible to group Owners for any [enterprise user](../user/enterprise_user/index.md). ## List all members of a group or project diff --git a/doc/architecture/blueprints/runner_tokens/index.md b/doc/architecture/blueprints/runner_tokens/index.md index 7d21dd594ad..84933bc132b 100644 --- a/doc/architecture/blueprints/runner_tokens/index.md +++ b/doc/architecture/blueprints/runner_tokens/index.md @@ -82,17 +82,18 @@ This approach reduces disruption to users responsible for deploying runners. ### Reusing the runner authentication token across many machines -In the existing model, a new runner is created whenever a new worker is required. This -has led to many situations where runners are left behind and become stale. +In the existing autoscaling model, a new runner is created whenever a new job needs to be executed. +This has led to many situations where runners are left behind and become stale. In the proposed model, a `ci_runners` table entry describes a configuration that the user can reuse -across multiple machines. +across multiple machines, and runner state from each machine (for example, IP address, platform, +or architecture) is moved to a separate table (`ci_runner_machines`). A unique system identifier is [generated automatically](#generating-a-system_id-value) whenever the runner application starts up or the configuration is saved. -This allows differentiating the context in which the runner is being used. +This allows differentiating the machine in which the runner is being used. -The `system_id` value complements the short runner token that is currently used to identify a -runner in command line output, CI job logs, and GitLab UI. +The `system_id` value complements the short runner token that is used to identify a runner in +command line output, CI job logs, and GitLab UI. Given that the creation of runners involves user interaction, it should be possible to eventually lower the per-plan limit of CI runners that can be registered per scope. @@ -295,8 +296,11 @@ enum column created in the `ci_runners` table. ### Runner creation through API -Automated runner creation may be allowed, although always through authenticated API calls - -using PAT tokens for example - such that every runner is associated with an owner. +Automated runner creation is possible through a new GraphQL mutation and the existing +[`POST /runners` REST API endpoint](../../../api/runners.md#register-a-new-runner). +The difference in the REST API endpoint is that it is modified to accept a request from an +authorized user with a scope (instance, a group, or a project) instead of the registration token. +These endpoints are only available to users that are allowed to create runners at the specified scope. ## Implementation plan @@ -315,7 +319,7 @@ using PAT tokens for example - such that every runner is associated with an owne | Component | Milestone | Changes | |------------------|----------:|---------| | GitLab Runner | `15.7` | Ensure a sidecar TOML file exists with a `system_id` value.<br/>Log new system ID values with `INFO` level as they get assigned. | -| GitLab Runner | `15.7` | Log unique system ID in the build logs. | +| GitLab Runner | `15.9` | Log unique system ID in the build logs. | | GitLab Runner | `15.9` | Label Prometheus metrics with unique system ID. | | GitLab Runner | `15.8` | Prepare `register` command to fail if runner server-side configuration options are passed together with a new `glrt-` token. | @@ -327,21 +331,23 @@ using PAT tokens for example - such that every runner is associated with an owne | GitLab Rails app | `%15.8` | Create database migration to add `ci_runner_machines` table. | | GitLab Rails app | `%15.9` | Create database migration to add `ci_runner_machines.id` foreign key to `ci_builds_metadata` table. | | GitLab Rails app | `%15.8` | Create database migrations to add `allow_runner_registration_token` setting to `application_settings` and `namespace_settings` tables (default: `true`). | -| GitLab Rails app | `%15.8` | Create database migration to add config column to `ci_runner_machines` table. | -| GitLab Runner | | Start sending `system_id` value in `POST /jobs/request` request and other follow-up requests that require identifying the unique system. | -| GitLab Rails app | | Create service similar to `StaleGroupRunnersPruneCronWorker` service to clean up `ci_runner_machines` records instead of `ci_runners` records.<br/>Existing service continues to exist but focuses only on legacy runners. | -| GitLab Rails app | | Create `ci_runner_machines` record in `POST /runners/verify` request if the runner token is prefixed with `glrt-`. | -| GitLab Rails app | | Use runner token + `system_id` JSON parameters in `POST /jobs/request` request in the [heartbeat request](https://gitlab.com/gitlab-org/gitlab/blob/c73c96a8ffd515295842d72a3635a8ae873d688c/lib/api/ci/helpers/runner.rb#L14-20) to update the `ci_runner_machines` cache/table. | +| GitLab Rails app | `%15.8` | Create database migration to add `config` column to `ci_runner_machines` table. | +| GitLab Runner | `%15.9` | Start sending `system_id` value in `POST /jobs/request` request and other follow-up requests that require identifying the unique system. | +| GitLab Rails app | `%15.9` | Create service similar to `StaleGroupRunnersPruneCronWorker` service to clean up `ci_runner_machines` records instead of `ci_runners` records.<br/>Existing service continues to exist but focuses only on legacy runners. | +| GitLab Rails app | `%15.9` | Create `ci_runner_machines` record in `POST /runners/verify` request if the runner token is prefixed with `glrt-`. | +| GitLab Rails app | `%15.9` | Use runner token + `system_id` JSON parameters in `POST /jobs/request` request in the [heartbeat request](https://gitlab.com/gitlab-org/gitlab/blob/c73c96a8ffd515295842d72a3635a8ae873d688c/lib/api/ci/helpers/runner.rb#L14-20) to update the `ci_runner_machines` cache/table. | +| GitLab Rails app | `%15.9` | [Feature flag] Enable runner creation workflow (`create_runner_workflow`). | ### Stage 4 - New UI | Component | Milestone | Changes | |------------------|----------:|---------| -| GitLab Rails app | `%15.10` | Implement new GraphQL user-authenticated API to create a new runner. | -| GitLab Rails app | `%15.10` | [Add prefix to newly generated runner authentication tokens](https://gitlab.com/gitlab-org/gitlab/-/issues/383198). | +| GitLab Rails app | `%15.9` | Implement new GraphQL user-authenticated API to create a new runner. | +| GitLab Rails app | `%15.9` | [Add prefix to newly generated runner authentication tokens](https://gitlab.com/gitlab-org/gitlab/-/issues/383198). | | GitLab Rails app | `%15.10` | Implement UI to create new runner. | | GitLab Rails app | `%15.10` | GraphQL changes to `CiRunner` type. | | GitLab Rails app | `%15.10` | UI changes to runner details view (listing of platform, architecture, IP address, etc.) (?) | +| GitLab Rails app | `%15.11` | Adapt `POST /api/v4/runners` REST endpoint to accept a request from an authorized user with a scope instead of a registration token. | ### Stage 5 - Optional disabling of registration token diff --git a/doc/ci/yaml/index.md b/doc/ci/yaml/index.md index a51fbd6188d..663900006dd 100644 --- a/doc/ci/yaml/index.md +++ b/doc/ci/yaml/index.md @@ -1112,6 +1112,12 @@ Caches are: - By default, not shared between [protected](../../user/project/protected_branches.md) and unprotected branches. - Restored before [artifacts](#artifacts). +You can [disable caching for specific jobs](../caching/index.md#disable-cache-for-specific-jobs), +for example to override: + +- A default cache defined with [`default`](#default). +- The configuration for a job added with [`include`](#include). + Learn more about caches in [Caching in GitLab CI/CD](../caching/index.md). #### `cache:paths` diff --git a/doc/development/database/avoiding_downtime_in_migrations.md b/doc/development/database/avoiding_downtime_in_migrations.md index 833f9dbebf6..a18e998f1d2 100644 --- a/doc/development/database/avoiding_downtime_in_migrations.md +++ b/doc/development/database/avoiding_downtime_in_migrations.md @@ -450,11 +450,10 @@ To start the process, add a regular migration to create the new `bigint` columns to keep in sync both columns for any new records ([see an example](https://gitlab.com/gitlab-org/gitlab/-/blob/41fbe34a4725a4e357a83fda66afb382828767b2/db/migrate/20210608072312_initialize_conversion_of_ci_stages_to_bigint.rb)): ```ruby -class InitializeConversionOfCiStagesToBigint < ActiveRecord::Migration[6.1] - include Gitlab::Database::MigrationHelpers +class InitializeConversionOfCiStagesToBigint < Gitlab::Database::Migration[2.1] TABLE = :ci_stages - COLUMNS = %i(id) + COLUMNS = %i[id] def up initialize_conversion_of_integer_to_bigint(TABLE, COLUMNS) @@ -484,11 +483,10 @@ and collecting metrics. To start the process, use the provided `backfill_convers helper ([example](https://gitlab.com/gitlab-org/gitlab/-/blob/41fbe34a4725a4e357a83fda66afb382828767b2/db/migrate/20210608072346_backfill_ci_stages_for_bigint_conversion.rb)): ```ruby -class BackfillCiStagesForBigintConversion < ActiveRecord::Migration[6.1] - include Gitlab::Database::MigrationHelpers +class BackfillCiStagesForBigintConversion < Gitlab::Database::Migration[2.1] TABLE = :ci_stages - COLUMNS = %i(id) + COLUMNS = %i[id] def up backfill_conversion_of_integer_to_bigint(TABLE, COLUMNS) diff --git a/doc/user/admin_area/appearance.md b/doc/user/admin_area/appearance.md index d9bdf964aed..a1fae7e8712 100644 --- a/doc/user/admin_area/appearance.md +++ b/doc/user/admin_area/appearance.md @@ -60,7 +60,7 @@ to activate it in the GitLab instance. You can replace the default message on the sign in / sign up page with your own message and logo. You can make full use of [Markdown](../markdown.md) in the description. -The optimal size for the logo is 640 x 360 pixels, but any image can be used (below 1 MB) +The optimal size for the logo is 128 x 128 pixels, but any image can be used (below 1 MB) and it is resized automatically. The logo image appears between the title and the description, on the left of the sign-up page. diff --git a/doc/user/enterprise_user/index.md b/doc/user/enterprise_user/index.md new file mode 100644 index 00000000000..3daee460956 --- /dev/null +++ b/doc/user/enterprise_user/index.md @@ -0,0 +1,71 @@ +--- +stage: Manage +group: Authentication and Authorization +info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments +type: reference +--- + +# Enterprise users **(PREMIUM SAAS)** + +Enterprise users have user accounts that are administered by an organization that +has purchased a [GitLab subscription](../../subscriptions/index.md). + +Enterprise users are identified by the [**Enterprise** badge](../project/badges.md) +next to their names on the [Members list](../group/manage.md#filter-and-sort-members-in-a-group). + +## Provision an enterprise user + +A user account is considered an enterprise account when: + +- A user without an existing GitLab user account uses the group's + [SAML SSO](../group/saml_sso/index.md) to sign in for the first time. +- [SCIM](../group/saml_sso/scim_setup.md) creates the user account on behalf of + the group. + +A user can also [manually connect an identity provider (IdP) to a GitLab account whose email address matches the subscribing organization's domain](../group/saml_sso/index.md#linking-saml-to-your-existing-gitlabcom-account). +By selecting **Authorize** when connecting these two accounts, the user account +with the matching email address is classified as an enterprise user. However, this +user account does not have an **Enterprise** badge in GitLab. + +Although a user can be a member of more than one group, each user account can be +provisioned by only one group. As a result, a user is considered an enterprise +user under one top-level group only. + +## Manage enterprise users in a namespace + +A top-level Owner of a namespace on a paid plan can retrieve information about and +manage enterprise user accounts in that namespace. + +These enterprise user-specific actions are in addition to the standard +[group member permissions](../permissions.md#group-members-permissions). + +### Disable two-factor authentication + +> [Introduced](https://gitlab.com/groups/gitlab-org/-/epics/9484) in GitLab 15.8. + +Top-level group Owners can disable two-factor authentication (2FA) for enterprise users. + +To disable 2FA: + +1. On the top bar, select **Main menu > Groups** and find your group. +1. On the left sidebar, select **Group information > Members**. +1. Find a user with the **Enterprise** and **2FA** badges. +1. Select **More actions** (**{ellipsis_v}**) and select **Disable two-factor authentication**. + +### Prevent users from creating groups and projects outside the corporate group + +A SAML identity administrator can configure the SAML response to set: + +- Whether users can create groups. +- The maximum number of personal projects users can create. + +For more information, see the [supported user attributes for SAML responses](../group/saml_sso/index.md#supported-user-attributes). + +### Bypass email confirmation for provisioned users + +A top-level group Owner can [set up verified domains to bypass confirmation emails](../group/saml_sso/index.md#bypass-user-email-confirmation-with-verified-domains). + +### Get users' email addresses through the API + +A top-level group Owner can use the [group and project members API](../../api/members.md) +to access users' information, including email addresses. diff --git a/doc/user/group/manage.md b/doc/user/group/manage.md index b524a17eae3..026e5acd6cb 100644 --- a/doc/user/group/manage.md +++ b/doc/user/group/manage.md @@ -131,7 +131,7 @@ Filter a group to find members. By default, all members in the group and subgrou In lists of group members, entries can display the following badges: - **SAML**, to indicate the member has a [SAML account](saml_sso/index.md) connected to them. -- **Enterprise**, to indicate that [SCIM created the account](saml_sso/scim_setup.md). +- **Enterprise**, to indicate that the member is an [enterprise user](../enterprise_user/index.md). 1. On the top bar, select **Main menu > Groups** and find your group. 1. Above the list of members, in the **Filter members** box, enter filter criteria. diff --git a/doc/user/group/saml_sso/index.md b/doc/user/group/saml_sso/index.md index 89536269305..2f12ff68723 100644 --- a/doc/user/group/saml_sso/index.md +++ b/doc/user/group/saml_sso/index.md @@ -347,7 +347,7 @@ To migrate users to a new email domain, users must: ## User access and management > - SAML user provisioning [introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/268142) in GitLab 13.7. -> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325712) in GitLab 14.0, GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning are displayed with an **Enterprise** badge in the **Members** view. +> - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325712) in GitLab 14.0, GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning are displayed with an ][**Enterprise**](../../enterprise_user/index.md) badge in the **Members** view. After group SSO is configured and enabled, users can access the GitLab.com group through the identity provider's dashboard. If [SCIM](scim_setup.md) is configured, see [user access](scim_setup.md#user-access) on the SCIM page. @@ -445,8 +445,9 @@ convert the information to XML. An example SAML response is shown here. By default, users provisioned with SAML or SCIM are sent a verification email to verify their identity. Instead, you can [configure GitLab with a custom domain](../../project/pages/custom_domains_ssl_tls_certification/index.md) and GitLab -automatically confirms user accounts. Users still receive an enterprise user welcome email. Confirmation is bypassed for -users: +automatically confirms user accounts. Users still receive an +[enterprise user](../../enterprise_user/index.md) welcome email. Confirmation is +bypassed for users: - That are provisioned with SAML or SCIM. - That have an email address that belongs to the verified domain. diff --git a/doc/user/group/saml_sso/scim_setup.md b/doc/user/group/saml_sso/scim_setup.md index 8c30c246566..79fc1ab310a 100644 --- a/doc/user/group/saml_sso/scim_setup.md +++ b/doc/user/group/saml_sso/scim_setup.md @@ -170,7 +170,7 @@ encounter issues. ## User access -> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325712) in GitLab 14.0, GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning are displayed with an **Enterprise** badge in the **Members** view. +> [Introduced](https://gitlab.com/gitlab-org/gitlab/-/issues/325712) in GitLab 14.0, GitLab users created by [SAML SSO](index.md#user-access-and-management) or SCIM provisioning are displayed with an [**Enterprise**](../../enterprise_user/index.md) badge in the **Members** view. During the synchronization process, all new users: diff --git a/doc/user/packages/container_registry/build_and_push_images.md b/doc/user/packages/container_registry/build_and_push_images.md index bbb82300488..aa86b87660b 100644 --- a/doc/user/packages/container_registry/build_and_push_images.md +++ b/doc/user/packages/container_registry/build_and_push_images.md @@ -144,7 +144,7 @@ In this example, `$CI_REGISTRY_IMAGE` resolves to the address of the registry ti to this project. `$CI_COMMIT_REF_NAME` resolves to the branch or tag name, which can contain forward slashes. Image tags can't contain forward slashes. Use `$CI_COMMIT_REF_SLUG` as the image tag. You can declare the variable, `$IMAGE_TAG`, -combining `$CI_REGISTRY_IMAGE` and `$CI_REGISTRY_IMAGE` to save some typing in the +combining `$CI_REGISTRY_IMAGE` and `$CI_COMMIT_REF_NAME` to save some typing in the `script` section. This example splits the tasks into 4 pipeline stages, including two tests that run in parallel. The `build` is stored in the container diff --git a/doc/user/packages/npm_registry/index.md b/doc/user/packages/npm_registry/index.md index ba5fddc9faf..11e3d0e5131 100644 --- a/doc/user/packages/npm_registry/index.md +++ b/doc/user/packages/npm_registry/index.md @@ -264,6 +264,22 @@ The GitLab npm repository supports the following commands for the npm CLI (`npm` ## Troubleshooting +### `404 Not Found` errors are happening on `npm install` or `yarn` + +Using `CI_JOB_TOKEN` to install npm packages with dependencies in another project gives you 404 Not Found errors. A fix for this problem is proposed in [issue 352962](https://gitlab.com/gitlab-org/gitlab/-/issues/352962). + +As a workaround, you can: + +1. Create a [personal access token](../../profile/personal_access_tokens.md). +1. Authenticate at both the instance level and project level for each package: + + ```ini + @foo:registry=https://gitlab.example.com/api/v4/packages/npm/ + //gitlab.example.com/api/v4/packages/npm/:_authToken=${MY_TOKEN} + //gitlab.example.com/api/v4/projects/<your_project_id_a>/packages/npm/:_authToken=${MY_TOKEN} + //gitlab.example.com/api/v4/projects/<your_project_id_b>/packages/npm/:_authToken=${MY_TOKEN} + ``` + ### `npm publish` targets default npm registry (`registry.npmjs.org`) Ensure that your package scope is set consistently in your `package.json` and `.npmrc` files. diff --git a/doc/user/project/members/share_project_with_groups.md b/doc/user/project/members/share_project_with_groups.md index d1ee42f723d..4dda68a6d08 100644 --- a/doc/user/project/members/share_project_with_groups.md +++ b/doc/user/project/members/share_project_with_groups.md @@ -4,60 +4,41 @@ group: Organization info: To determine the technical writer assigned to the Stage/Group associated with this page, see https://about.gitlab.com/handbook/product/ux/technical-writing/#assignments --- -# Share projects with other groups **(FREE)** +# Share a project with a group **(FREE)** -You can share projects with other [groups](../../group/index.md). This makes it -possible to add a group of users to a project with a single action. +When you want a group to have access to your project, +you can invite [a group](../../group/index.md) to the project. +The group's members get access to the project, which becomes a *shared project*. -For example, if `Project A` belongs to `Group 1`, the members of `Group 1` have access to the project. -If `Project A` already belongs to another `Group 2`, the owner of `Group 2` can share `Project A` -with `Group 1`, so that both members of `Group 1` and `Group 2` have access to the project. +## Example -When a project is shared with a group: +For a project that was created by `Group 1`: -- All group members, including members of subgroups or projects that belong to the group, - are assigned the same role in the project. - Each member's role is displayed in **Project information > Members**, in the **Max role** column. - When sharing a project with a group, a user's assigned **Max role** is the lowest - of either: +- The members of `Group 1` have access to the project. +- The owner of `Group 1` can invite `Group 2` to the project. +This way, members of both `Group 1` and `Group 2` have access to the shared project. - - The role assigned in the group membership. - - The maximum role selected when sharing the project with the group. +## Prerequisites - Assigning a higher maximum role to the group doesn't give group users higher roles than - the roles already assigned to them in the group. -- The group is listed in the **Groups** tab. -- The project is listed on the group dashboard. +To invite a group to a project, you must be at least one of the following: -Be aware of the restrictions that apply when you share projects with: +- Explicitly defined as a [member](index.md) of the project. +- Explicitly defined as a member of a group or subgroup that has access to the project. +- An administrator. -- [Groups with a more restrictive visibility level](#share-projects-with-groups-with-a-more-restrictive-visibility-level). -- [Restricted sharing](#prevent-project-sharing). +In addition: -## Share projects with groups with a more restrictive visibility level +- The group you're inviting must have a more restrictive + [visibility level](../../public_access.md#project-and-group-visibility) + than the project. For example, you can invite: + - A private group to a public project. + - An internal group to a public project. + - A private group to an internal project. -You can share projects only down the group's organization structure. -This means you can share a project with a group that has a more restrictive -[visibility level](../../public_access.md#project-and-group-visibility) than the project, -but not with a group that has a less restrictive visibility level. - -For example, you can share: - -- A public project with a private group. -- A public project with an internal group. -- An internal project with a private group. - -This restriction applies to subgroups as well. For example, `group/subgroup01/project`: - -- Can't be shared with `group`. -- Can be shared with `group/subgroup02` or `group/subgroup01/subgroup03`. - -When you share a project with a group that has a more restrictive visibility level than the project: - -- The group name is visible to all users that can view the project members page. -- Owners of the project have access to members of the group when they mention them in issues or merge requests. -- Project members who are direct or indirect members of the group can see -group members listed in addition to members of the project. +- The group or subgroup must be in the project's [namespace](../../namespace/index.md). + For example, a project in the namespace `group/subgroup01/project`: + - Can be shared with `group/subgroup02` or `group/subgroup01/subgroup03`. + - Cannot be shared with `group`. ## Share a project with a group @@ -68,22 +49,57 @@ group members listed in addition to members of the project. > - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/352526) in GitLab 14.9. [Feature flag `invite_members_group_modal`](https://gitlab.com/gitlab-org/gitlab/-/issues/352526) removed. -You can share a project only with groups: +You can share a project with a group by inviting that group to the project. -- Where you have an explicitly defined [membership](index.md). -- That contain a nested subgroup or project you have an explicitly defined role for. -- You are an administrator of. - -To share a project with a group: +To invite a group to a project: 1. On the top bar, select **Main menu > Projects** and find your project. -1. In the left navigation menu, select **Project information > Members**. +1. On the left sidebar, select **Project information > Members**. 1. Select **Invite a group**. 1. **Select a group** you want to add to the project. 1. **Select a role** you want to assign to the group. 1. Optional. Select an **Access expiration date**. 1. Select **Invite**. -## Prevent project sharing +All group members, members of subgroups, and members of other projects the group has access to +are given access to the project. In addition: + +- On the group's page, the project is listed on the **Shared projects** tab. +- On the project's **Members** page, the group is listed on the **Groups** tab. +- Each user is assigned a maximum role. + +## Maximum role + +When multiple groups contain the same members, and the groups +have access to the same project, the group members are +given the most restrictive role for the project. + +This most restrictive role is called the *maximum role*, or **Max role**. + +The member's **Max role** is the more restrictive of: + +- The role the user is assigned for the group. +- The role you chose when you invited the group to the project. + +### View the member's Max role + +To view the maximum role assigned to a member: + +1. On the top bar, select **Main menu > Projects** and find your project. +1. On the left sidebar, select **Project information > Members**. +1. In the **Max role** column, view the user's maximum assigned role. + +## View a group's shared projects + +In a group, a shared project is a project to which the group members gained access through the [**Invite group**](#share-a-project-with-a-group) action. + +To view a group's shared projects: + +1. On the top bar, select **Main menu > Group** and find your group. +1. On the group page, select the **Shared projects** tab. + +A list of shared projects is displayed. + +## Related topics -For more information, see [Prevent a project from being shared with groups](../../group/access_and_permissions.md#prevent-a-project-from-being-shared-with-groups). +- [Prevent a project from being shared with groups](../../group/access_and_permissions.md#prevent-a-project-from-being-shared-with-groups). diff --git a/lib/api/api.rb b/lib/api/api.rb index 81b96207863..4b7fe6bdc7a 100644 --- a/lib/api/api.rb +++ b/lib/api/api.rb @@ -209,6 +209,7 @@ module API mount ::API::DeployKeys mount ::API::DeployTokens mount ::API::Deployments + mount ::API::DraftNotes mount ::API::Environments mount ::API::ErrorTracking::ClientKeys mount ::API::ErrorTracking::ProjectSettings diff --git a/lib/api/draft_notes.rb b/lib/api/draft_notes.rb new file mode 100644 index 00000000000..ece4e1a6584 --- /dev/null +++ b/lib/api/draft_notes.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +module API + class DraftNotes < ::API::Base + before { authenticate! } + + urgency :low + + helpers do + def merge_request(params:) + strong_memoize(:merge_request) do + find_project_merge_request(params[:merge_request_iid]) + end + end + + def load_draft_notes(params:) + merge_request(params: params).draft_notes.authored_by(current_user) + end + end + + resource :projects, requirements: API::NAMESPACE_OR_PROJECT_REQUIREMENTS do + desc "Get a list of merge request draft notes" do + success Entities::DraftNote + is_array true + failure [ + { code: 401, message: 'Unauthorized' }, + { code: 404, message: 'Not found' } + ] + end + params do + requires :id, type: String, desc: "The ID of a project" + requires :merge_request_iid, type: Integer, desc: "The ID of a merge request" + end + get ":id/merge_requests/:merge_request_iid/draft_notes", feature_category: :code_review_workflow do + present load_draft_notes(params: params), with: Entities::DraftNote + end + end + end +end diff --git a/lib/api/entities/draft_note.rb b/lib/api/entities/draft_note.rb new file mode 100644 index 00000000000..70b32bac502 --- /dev/null +++ b/lib/api/entities/draft_note.rb @@ -0,0 +1,45 @@ +# frozen_string_literal: true + +module API + module Entities + class DraftNote < Grape::Entity + expose :id, documentation: { type: 'integer', example: 2 } + expose :author_id, documentation: { type: 'integer', example: 4 } + expose :merge_request_id, documentation: { type: 'integer', example: 52 } + expose :resolve_discussion, documentation: { type: 'boolean', example: true } + expose :discussion_id, documentation: { type: 'integer', example: 613 } + expose :note, documentation: { type: 'string', example: 'This is a note' } + expose :commit_id, documentation: { type: 'integer', example: 4 } + expose :line_code, documentation: { type: 'string', example: '1c497fbb3a46b78edf0_2_4' } + expose :position, documentation: { + type: 'Hash', + example: { + base_sha: "aa149113", + start_sha: "b3a0a8c4", + head_sha: "be3020c7", + old_path: "example.md", + new_path: "example.md", + position_type: "text", + old_line: 2, + new_line: 4, + line_range: { + start: { + line_code: "1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4", + type: nil, + old_line: 2, + new_line: 4 + }, + end: { + line_code: "1c497fbb3a46b78edf04cc2a2fa33f67e3ffbe2a_2_4", + type: nil, + old_line: 2, + new_line: 4 + } + } + } + } do |note| + note.position.to_h + end + end + end +end diff --git a/lib/gitlab/ci/artifact_file_reader.rb b/lib/gitlab/ci/artifact_file_reader.rb index b0fad026ec5..2eb8df01d58 100644 --- a/lib/gitlab/ci/artifact_file_reader.rb +++ b/lib/gitlab/ci/artifact_file_reader.rb @@ -9,6 +9,7 @@ module Gitlab Error = Class.new(StandardError) MAX_ARCHIVE_SIZE = 5.megabytes + TMP_ARTIFACT_EXTRACTION_DIR = "extracted_artifacts_job_%{id}" def initialize(job) @job = job @@ -45,20 +46,20 @@ module Gitlab end def read_zip_file!(file_path) - job.artifacts_file.use_open_file do |file| - zip_file = Zip::File.new(file, false, true) - entry = zip_file.find_entry(file_path) + dir_name = format(TMP_ARTIFACT_EXTRACTION_DIR, id: job.id.to_i) - unless entry - raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" + job.artifacts_file.use_open_file(unlink_early: false) do |tmp_open_file| + Dir.mktmpdir(dir_name) do |tmp_dir| + SafeZip::Extract.new(tmp_open_file.file_path).extract(files: [file_path], to: tmp_dir) + File.read(File.join(tmp_dir, file_path)) end - - if entry.name_is_directory? - raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" - end - - zip_file.read(entry) end + rescue SafeZip::Extract::NoMatchingError + raise Error, "Path `#{file_path}` does not exist inside the `#{job.name}` artifacts archive!" + rescue SafeZip::Extract::EntrySizeError + raise Error, "Path `#{file_path}` has invalid size in the zip!" + rescue Errno::EISDIR + raise Error, "Path `#{file_path}` was expected to be a file but it was a directory!" end def max_archive_size_in_mb diff --git a/lib/gitlab/utils.rb b/lib/gitlab/utils.rb index 65572c237ec..b92e7dbb725 100644 --- a/lib/gitlab/utils.rb +++ b/lib/gitlab/utils.rb @@ -4,6 +4,7 @@ module Gitlab module Utils extend self PathTraversalAttackError ||= Class.new(StandardError) + DoubleEncodingError ||= Class.new(StandardError) private_class_method def logger @logger ||= Gitlab::AppLogger @@ -55,7 +56,7 @@ module Gitlab def decode_path(encoded_path) decoded = CGI.unescape(encoded_path) if decoded != CGI.unescape(decoded) - raise StandardError, "path #{encoded_path} is not allowed" + raise DoubleEncodingError, "path #{encoded_path} is not allowed" end decoded diff --git a/lib/safe_zip/entry.rb b/lib/safe_zip/entry.rb index 52d70e83154..88647b9b1eb 100644 --- a/lib/safe_zip/entry.rb +++ b/lib/safe_zip/entry.rb @@ -25,8 +25,8 @@ module SafeZip end def extract - # do not extract if file is not part of target directory - return false unless matching_target_directory + # do not extract if file is not part of target directory or target file + return false unless matching_target_directory || matching_target_file # do not overwrite existing file raise SafeZip::Extract::AlreadyExistsError, "File already exists #{zip_entry.name}" if exist? @@ -44,6 +44,8 @@ module SafeZip end rescue SafeZip::Extract::Error raise + rescue Zip::EntrySizeError => e + raise SafeZip::Extract::EntrySizeError, e.message rescue StandardError => e raise SafeZip::Extract::ExtractError, e.message end @@ -84,6 +86,10 @@ module SafeZip params.matching_target_directory(path) end + def matching_target_file + params.matching_target_file(path) + end + def read_symlink zip_archive.read(zip_entry) end diff --git a/lib/safe_zip/extract.rb b/lib/safe_zip/extract.rb index 74df7895afe..b86941e6bea 100644 --- a/lib/safe_zip/extract.rb +++ b/lib/safe_zip/extract.rb @@ -6,6 +6,7 @@ module SafeZip PermissionDeniedError = Class.new(Error) SymlinkSourceDoesNotExistError = Class.new(Error) UnsupportedEntryError = Class.new(Error) + EntrySizeError = Class.new(Error) AlreadyExistsError = Class.new(Error) NoMatchingError = Class.new(Error) ExtractError = Class.new(Error) diff --git a/lib/safe_zip/extract_params.rb b/lib/safe_zip/extract_params.rb index bd3b788bac9..96881ad1abc 100644 --- a/lib/safe_zip/extract_params.rb +++ b/lib/safe_zip/extract_params.rb @@ -4,11 +4,13 @@ module SafeZip class ExtractParams include Gitlab::Utils::StrongMemoize - attr_reader :directories, :extract_path + attr_reader :directories, :files, :extract_path - def initialize(directories:, to:) + def initialize(to:, directories: [], files: []) @directories = directories + @files = files @extract_path = ::File.realpath(to) + validate! end def matching_target_directory(path) @@ -32,5 +34,23 @@ module SafeZip end end end + + def matching_target_file(path) + target_files.include?(path) + end + + private + + def target_files + strong_memoize(:target_files) do + files.map do |file| + ::File.join(extract_path, file) + end + end + end + + def validate! + raise ArgumentError, 'Either directories or files are required' if directories.empty? && files.empty? + end end end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 1eafc5c0049..590bebb49bd 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -3649,15 +3649,6 @@ msgstr "" msgid "After a successful password update, you will be redirected to the login page where you can log in with your new password." msgstr "" -msgid "After it expires, you can't use merge approvals, code quality, or many other features." -msgstr "" - -msgid "After it expires, you can't use merge approvals, epics, or many other features." -msgstr "" - -msgid "After it expires, you can't use merge approvals, epics, or many security features." -msgstr "" - msgid "After the Apple App Store Connect integration is activated, the following protected variables will be created for CI/CD use." msgstr "" @@ -10187,6 +10178,9 @@ msgstr "" msgid "Compare GitLab editions" msgstr "" +msgid "Compare GitLab plans" +msgstr "" + msgid "Compare Revisions" msgstr "" @@ -12373,9 +12367,6 @@ msgstr "" msgid "DORA4Metrics|Average (last %{days}d)" msgstr "" -msgid "DORA4Metrics|Average number of deployments to production per day. This metric measures how often value is delivered to end users." -msgstr "" - msgid "DORA4Metrics|Change Failure Rate" msgstr "" @@ -12433,12 +12424,6 @@ msgstr "" msgid "DORA4Metrics|Median time an incident was open in a production environment over the given time period." msgstr "" -msgid "DORA4Metrics|Median time from issue created to issue closed." -msgstr "" - -msgid "DORA4Metrics|Median time from the earliest commit of a linked issue's merge request to when that issue is closed." -msgstr "" - msgid "DORA4Metrics|Metrics comparison for %{name} group" msgstr "" @@ -12463,15 +12448,9 @@ msgstr "" msgid "DORA4Metrics|Number of incidents divided by the number of deployments to a production environment in the given time period." msgstr "" -msgid "DORA4Metrics|Number of new issues created." -msgstr "" - msgid "DORA4Metrics|Past 6 Months" msgstr "" -msgid "DORA4Metrics|Percentage of deployments that cause an incident in production." -msgstr "" - msgid "DORA4Metrics|Percentage of failed deployments" msgstr "" @@ -12493,12 +12472,6 @@ msgstr "" msgid "DORA4Metrics|The chart displays the median time between a merge request being merged and deployed to production environment(s) that are based on the %{linkStart}deployment_tier%{linkEnd} value." msgstr "" -msgid "DORA4Metrics|The time it takes an organization to recover from a failure in production." -msgstr "" - -msgid "DORA4Metrics|The time to successfully deliver a commit into production. This metric reflects the efficiency of CI/CD pipelines." -msgstr "" - msgid "DORA4Metrics|Time to Restore Service" msgstr "" @@ -12508,9 +12481,6 @@ msgstr "" msgid "DORA4Metrics|Time to restore service (median days)" msgstr "" -msgid "DORA4Metrics|Total number of deploys to production." -msgstr "" - msgid "DSN" msgstr "" @@ -18742,6 +18712,9 @@ msgstr "" msgid "GitLab commit" msgstr "" +msgid "GitLab documentation" +msgstr "" + msgid "GitLab events trigger webhooks. Use the request details of a webhook to help troubleshoot problems. %{link_start}How do I troubleshoot?%{link_end}" msgstr "" @@ -21101,6 +21074,12 @@ msgstr "" msgid "If you did not recently try to sign in, you should immediately change your password: %{password_link}." msgstr "" +msgid "If you do not renew by %{strong}%{downgrades_on}%{strong_close}, you can't use merge approvals, %{end_message}" +msgstr "" + +msgid "If you don't renew by %{strong}%{downgrades_on}%{strong_close} your instance will become read-only, and you won't be able to create issues or merge requests. You will also lose access to your paid features and support entitlement. %{learn_more_link}" +msgstr "" + msgid "If you get a lot of false alarms from repository checks, you can clear all repository check information from the database." msgstr "" @@ -25816,10 +25795,10 @@ msgstr "" msgid "Maximum file size is 1 MB. Image size must be 32 x 32 pixels. Allowed image formats are %{favicon_extension_allowlist}." msgstr "" -msgid "Maximum file size is 1MB. Pages are optimized for a 24px tall header logo" +msgid "Maximum file size is 1 MB. Pages are optimized for a 128x128 px logo." msgstr "" -msgid "Maximum file size is 1MB. Pages are optimized for a 640x360 px logo." +msgid "Maximum file size is 1MB. Pages are optimized for a 24px tall header logo" msgstr "" msgid "Maximum files in a diff" @@ -34430,6 +34409,9 @@ msgstr "" msgid "Provide a number our sales team can use to call you." msgstr "" +msgid "Provide feedback" +msgstr "" + msgid "Provider" msgstr "" @@ -46265,6 +46247,9 @@ msgstr "" msgid "ValueStreamAnalytics|Average number of deployments to production per day." msgstr "" +msgid "ValueStreamAnalytics|Average number of deployments to production per day. This metric measures how often value is delivered to end users." +msgstr "" + msgid "ValueStreamAnalytics|DORA metrics" msgstr "" @@ -46319,6 +46304,12 @@ msgstr "" msgid "ValueStreamAnalytics|Tasks by type" msgstr "" +msgid "ValueStreamAnalytics|The time it takes an organization to recover from a failure in production." +msgstr "" + +msgid "ValueStreamAnalytics|The time to successfully deliver a commit into production. This metric reflects the efficiency of CI/CD pipelines." +msgstr "" + msgid "ValueStreamAnalytics|There was an error while fetching value stream analytics %{requestTypeName} data." msgstr "" @@ -48851,13 +48842,13 @@ msgstr "" msgid "Your %{group} membership will now expire in %{days}." msgstr "" -msgid "Your %{spammable_entity_type} has been recognized as spam and has been discarded." +msgid "Your %{plan_name} subscription will expire on %{expires_on}" msgstr "" -msgid "Your %{spammable_entity_type} has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed." +msgid "Your %{spammable_entity_type} has been recognized as spam and has been discarded." msgstr "" -msgid "Your %{strong}%{plan_name}%{strong_close} subscription expires on %{strong}%{expires_on}%{strong_close}. If you do not renew, you will lose access to your paid features on %{strong}%{downgrades_on}%{strong_close}. After that date, you can't create issues or merge requests, or use many other features." +msgid "Your %{spammable_entity_type} has been recognized as spam. Please, change the content or solve the reCAPTCHA to proceed." msgstr "" msgid "Your %{strong}%{plan_name}%{strong_close} subscription for %{strong}%{namespace_name}%{strong_close} will expire on %{strong}%{expires_on}%{strong_close}." @@ -49161,9 +49152,6 @@ msgid_plural "Your subscription has %{remaining_seat_count} out of %{total_seat_ msgstr[0] "" msgstr[1] "" -msgid "Your subscription will expire on %{expires_on}" -msgstr "" - msgid "Your top-level group %{namespace_name} has reached the %{free_limit} user limit" msgstr "" diff --git a/scripts/trigger-build.rb b/scripts/trigger-build.rb index 033c2e55329..69eea7488fb 100755 --- a/scripts/trigger-build.rb +++ b/scripts/trigger-build.rb @@ -9,10 +9,6 @@ module Trigger %w[gitlab gitlab-ee].include?(ENV['CI_PROJECT_NAME']) end - def self.security? - %r{\Agitlab-org/security(\z|/)}.match?(ENV['CI_PROJECT_NAMESPACE']) - end - def self.non_empty_variable_value(variable) variable_value = ENV[variable] @@ -30,10 +26,10 @@ module Trigger class Base # Can be overridden def self.access_token - ENV['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] + ENV['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] end - def invoke!(downstream_job_name: nil) + def invoke! pipeline_variables = variables puts "Triggering downstream pipeline on #{downstream_project_path}" @@ -48,18 +44,7 @@ module Trigger puts "Triggered downstream pipeline: #{pipeline.web_url}\n" puts "Waiting for downstream pipeline status" - downstream_job = - if downstream_job_name - downstream_client.pipeline_jobs(downstream_project_path, pipeline.id).auto_paginate.find do |potential_job| - potential_job.name == downstream_job_name - end - end - - if downstream_job - Trigger::Job.new(downstream_project_path, downstream_job.id, downstream_client) - else - Trigger::Pipeline.new(downstream_project_path, pipeline.id, downstream_client) - end + Trigger::Pipeline.new(downstream_project_path, pipeline.id, downstream_client) end def variables @@ -221,6 +206,11 @@ module Trigger end end + # This is used in: + # - https://gitlab.com/gitlab-org/gitlab-runner/-/blob/ddaf90761c917a42ed4aab60541b6bc33871fe68/.gitlab/ci/docs.gitlab-ci.yml#L1-47 + # - https://gitlab.com/gitlab-org/charts/gitlab/-/blob/fa348e709e901196803051669b4874b657b4ea91/.gitlab-ci.yml#L497-543 + # - https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/b44483f05c5e22628ba3b49ec4c7f8761c688af0/gitlab-ci-config/gitlab-com.yml#L199-224 + # - https://gitlab.com/gitlab-org/omnibus-gitlab/-/blob/b44483f05c5e22628ba3b49ec4c7f8761c688af0/gitlab-ci-config/gitlab-com.yml#L356-380 class Docs < Base def self.access_token # Default to "DOCS_PROJECT_API_TOKEN" at https://gitlab.com/gitlab-org/gitlab-docs/-/settings/access_tokens @@ -321,7 +311,7 @@ module Trigger class DatabaseTesting < Base IDENTIFIABLE_NOTE_TAG = 'gitlab-org/database-team/gitlab-com-database-testing:identifiable-note' - def invoke!(downstream_job_name: nil) + def invoke! pipeline = super project_path = variables['TOP_UPSTREAM_SOURCE_PROJECT'] merge_request_id = variables['TOP_UPSTREAM_MERGE_REQUEST_IID'] @@ -438,14 +428,10 @@ module Trigger attr_reader :project, :gitlab_client, :start_time end - - Job = Class.new(Pipeline) end if $PROGRAM_NAME == __FILE__ case ARGV[0] - when 'cng' - Trigger::CNG.new.invoke!.wait! when 'gitlab-com-database-testing' Trigger::DatabaseTesting.new.invoke! when 'docs' @@ -463,7 +449,6 @@ if $PROGRAM_NAME == __FILE__ else puts "Please provide a valid option: omnibus - Triggers a pipeline that builds the omnibus-gitlab package - cng - Triggers a pipeline that builds images used by the GitLab helm chart gitlab-com-database-testing - Triggers a pipeline that tests database changes on GitLab.com data" end end diff --git a/spec/features/issues/user_bulk_edits_issues_spec.rb b/spec/features/issues/user_bulk_edits_issues_spec.rb index fc48bc4baf9..5696bde4069 100644 --- a/spec/features/issues/user_bulk_edits_issues_spec.rb +++ b/spec/features/issues/user_bulk_edits_issues_spec.rb @@ -46,7 +46,7 @@ RSpec.describe 'Multiple issue updating from issues#index', :js, feature_categor click_button 'Edit issues' check 'Select all' click_update_assignee_button - click_link user.username + click_button user.username click_update_issues_button @@ -64,7 +64,7 @@ RSpec.describe 'Multiple issue updating from issues#index', :js, feature_categor click_button 'Edit issues' check 'Select all' click_update_assignee_button - click_link 'Unassigned' + click_button 'Unassigned' click_update_issues_button expect(find('.issue:first-of-type')).not_to have_link "Assigned to #{user.name}" diff --git a/spec/features/merge_requests/user_mass_updates_spec.rb b/spec/features/merge_requests/user_mass_updates_spec.rb index 5a9054ece48..b0be76d386a 100644 --- a/spec/features/merge_requests/user_mass_updates_spec.rb +++ b/spec/features/merge_requests/user_mass_updates_spec.rb @@ -121,7 +121,7 @@ RSpec.describe 'Merge requests > User mass updates', :js, feature_category: :cod within 'aside[aria-label="Bulk update"]' do click_button 'Select assignee' wait_for_requests - click_link text + click_button text end click_update_merge_requests_button end diff --git a/spec/fixtures/packages/helm/corrupted_chart.tgz b/spec/fixtures/packages/helm/corrupted_chart.tgz Binary files differnew file mode 100644 index 00000000000..b2ac93b271e --- /dev/null +++ b/spec/fixtures/packages/helm/corrupted_chart.tgz diff --git a/spec/fixtures/safe_zip/invalid-unexpected-large.zip b/spec/fixtures/safe_zip/invalid-unexpected-large.zip Binary files differnew file mode 100644 index 00000000000..3005da8c779 --- /dev/null +++ b/spec/fixtures/safe_zip/invalid-unexpected-large.zip diff --git a/spec/fixtures/safe_zip/valid-symlinks-first.zip b/spec/fixtures/safe_zip/valid-symlinks-first.zip Binary files differindex f5952ef71c9..1d7ecfd7bed 100644 --- a/spec/fixtures/safe_zip/valid-symlinks-first.zip +++ b/spec/fixtures/safe_zip/valid-symlinks-first.zip diff --git a/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js b/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js index 4f2badf869d..bbc27a621ea 100644 --- a/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js +++ b/spec/frontend/frequent_items/components/frequent_items_list_item_spec.js @@ -154,7 +154,7 @@ describe('FrequentItemsListItemComponent', () => { link.vm.$emit('click'); expect(trackingSpy).toHaveBeenCalledWith(undefined, 'click_link', { - label: 'projects_dropdown_frequent_items_list_item_git_lab_community_edition', + label: 'projects_dropdown_frequent_items_list_item', }); }); }); diff --git a/spec/frontend/super_sidebar/components/help_center_spec.js b/spec/frontend/super_sidebar/components/help_center_spec.js new file mode 100644 index 00000000000..f1db755a711 --- /dev/null +++ b/spec/frontend/super_sidebar/components/help_center_spec.js @@ -0,0 +1,87 @@ +import { GlDisclosureDropdown } from '@gitlab/ui'; +import { within } from '@testing-library/dom'; +import toggleWhatsNewDrawer from '~/whats_new'; +import { mountExtended } from 'helpers/vue_test_utils_helper'; +import HelpCenter from '~/super_sidebar/components/help_center.vue'; +import { helpPagePath } from '~/helpers/help_page_helper'; +import { PROMO_URL } from 'jh_else_ce/lib/utils/url_utility'; +import { sidebarData } from '../mock_data'; + +jest.mock('~/whats_new'); + +describe('HelpCenter component', () => { + let wrapper; + + const findDropdown = () => wrapper.findComponent(GlDisclosureDropdown); + const withinComponent = () => within(wrapper.element); + const findButton = (name) => withinComponent().getByRole('button', { name }); + + const createWrapper = () => { + wrapper = mountExtended(HelpCenter, { + propsData: { sidebarData }, + }); + }; + + describe('default', () => { + beforeEach(() => { + createWrapper(); + }); + + it('renders menu items', () => { + expect(findDropdown().props('items')[0].items).toEqual([ + { text: HelpCenter.i18n.help, href: helpPagePath() }, + { text: HelpCenter.i18n.support, href: sidebarData.support_path }, + { text: HelpCenter.i18n.docs, href: 'https://docs.gitlab.com' }, + { text: HelpCenter.i18n.plans, href: `${PROMO_URL}/pricing` }, + { text: HelpCenter.i18n.forum, href: 'https://forum.gitlab.com/' }, + { + text: HelpCenter.i18n.contribute, + href: helpPagePath('', { anchor: 'contributing-to-gitlab' }), + }, + { text: HelpCenter.i18n.feedback, href: 'https://about.gitlab.com/submit-feedback' }, + ]); + + expect(findDropdown().props('items')[1].items).toEqual([ + expect.objectContaining({ text: HelpCenter.i18n.shortcuts }), + expect.objectContaining({ text: HelpCenter.i18n.whatsnew }), + ]); + }); + + describe('showKeyboardShortcuts', () => { + beforeEach(() => { + jest.spyOn(wrapper.vm.$refs.dropdown, 'close'); + window.toggleShortcutsHelp = jest.fn(); + findButton('Keyboard shortcuts').click(); + }); + + it('closes the dropdown', () => { + expect(wrapper.vm.$refs.dropdown.close).toHaveBeenCalled(); + }); + + it('shows the keyboard shortcuts modal', () => { + expect(window.toggleShortcutsHelp).toHaveBeenCalled(); + }); + }); + + describe('showWhatsNew', () => { + beforeEach(() => { + jest.spyOn(wrapper.vm.$refs.dropdown, 'close'); + findButton("What's new").click(); + }); + + it('closes the dropdown', () => { + expect(wrapper.vm.$refs.dropdown.close).toHaveBeenCalled(); + }); + + it('shows the "What\'s new" slideout', () => { + expect(toggleWhatsNewDrawer).toHaveBeenCalledWith(expect.any(Object)); + }); + + it('shows the existing "What\'s new" slideout instance on subsequent clicks', () => { + findButton("What's new").click(); + expect(toggleWhatsNewDrawer).toHaveBeenCalledTimes(2); + expect(toggleWhatsNewDrawer).toHaveBeenLastCalledWith(); + }); + }); + }); +}); diff --git a/spec/frontend/super_sidebar/components/super_sidebar_spec.js b/spec/frontend/super_sidebar/components/super_sidebar_spec.js index 86ba1c1ea45..45fc30c08f0 100644 --- a/spec/frontend/super_sidebar/components/super_sidebar_spec.js +++ b/spec/frontend/super_sidebar/components/super_sidebar_spec.js @@ -1,5 +1,6 @@ import { shallowMountExtended } from 'helpers/vue_test_utils_helper'; import SuperSidebar from '~/super_sidebar/components/super_sidebar.vue'; +import HelpCenter from '~/super_sidebar/components/help_center.vue'; import UserBar from '~/super_sidebar/components/user_bar.vue'; import { sidebarData } from '../mock_data'; @@ -7,6 +8,7 @@ describe('SuperSidebar component', () => { let wrapper; const findUserBar = () => wrapper.findComponent(UserBar); + const findHelpCenter = () => wrapper.findComponent(HelpCenter); const createWrapper = (props = {}) => { wrapper = shallowMountExtended(SuperSidebar, { @@ -25,5 +27,9 @@ describe('SuperSidebar component', () => { it('renders UserBar with sidebarData', () => { expect(findUserBar().props('sidebarData')).toBe(sidebarData); }); + + it('renders HelpCenter with sidebarData', () => { + expect(findHelpCenter().props('sidebarData')).toBe(sidebarData); + }); }); }); diff --git a/spec/frontend/super_sidebar/mock_data.js b/spec/frontend/super_sidebar/mock_data.js index bdbc25e49f0..379e4c2bffb 100644 --- a/spec/frontend/super_sidebar/mock_data.js +++ b/spec/frontend/super_sidebar/mock_data.js @@ -48,4 +48,6 @@ export const sidebarData = { todos_pending_count: 3, issues_dashboard_path: 'path/to/issues', create_new_menu_groups: createNewMenuGroups, + support_path: '/support', + display_whats_new: true, }; diff --git a/spec/frontend/vue_shared/components/user_select_spec.js b/spec/frontend/vue_shared/components/user_select_spec.js index 874796f653a..b0e9584a15b 100644 --- a/spec/frontend/vue_shared/components/user_select_spec.js +++ b/spec/frontend/vue_shared/components/user_select_spec.js @@ -285,6 +285,20 @@ describe('User select dropdown', () => { expect(wrapper.emitted('input')).toEqual([[[]]]); }); + it('hides the dropdown after clicking on `Unassigned`', async () => { + createComponent({ + props: { + value: [assignee], + }, + }); + wrapper.vm.$refs.dropdown.hide = jest.fn(); + await waitForPromises(); + + findUnassignLink().trigger('click'); + + expect(wrapper.vm.$refs.dropdown.hide).toHaveBeenCalledTimes(1); + }); + it('emits an empty array after unselecting the only selected assignee', async () => { createComponent({ props: { diff --git a/spec/helpers/sidebars_helper_spec.rb b/spec/helpers/sidebars_helper_spec.rb index fcaff83ada4..ecbc1597bdf 100644 --- a/spec/helpers/sidebars_helper_spec.rb +++ b/spec/helpers/sidebars_helper_spec.rb @@ -66,7 +66,9 @@ RSpec.describe SidebarsHelper do assigned_open_issues_count: 1, assigned_open_merge_requests_count: 2, todos_pending_count: 3, - issues_dashboard_path: issues_dashboard_path(assignee_username: user.username) + issues_dashboard_path: issues_dashboard_path(assignee_username: user.username), + support_path: helper.support_url, + display_whats_new: helper.display_whats_new? }) end diff --git a/spec/lib/api/entities/draft_note_spec.rb b/spec/lib/api/entities/draft_note_spec.rb new file mode 100644 index 00000000000..59555319bb1 --- /dev/null +++ b/spec/lib/api/entities/draft_note_spec.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::Entities::DraftNote, feature_category: :code_review_workflow do + let_it_be(:entity) { create(:draft_note, :on_discussion) } + let_it_be(:json) { entity.as_json } + + it 'exposes correct attributes' do + expect(json["id"]).to eq entity.id + expect(json["author_id"]).to eq entity.author_id + expect(json["merge_request_id"]).to eq entity.merge_request_id + expect(json["resolve_discussion"]).to eq entity.resolve_discussion + expect(json["discussion_id"]).to eq entity.discussion_id + expect(json["note"]).to eq entity.note + expect(json["position"].transform_keys(&:to_sym)).to eq entity.position.to_h + end +end diff --git a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb index e982f0eb015..813dc15e79f 100644 --- a/spec/lib/gitlab/ci/artifact_file_reader_spec.rb +++ b/spec/lib/gitlab/ci/artifact_file_reader_spec.rb @@ -10,71 +10,117 @@ RSpec.describe Gitlab::Ci::ArtifactFileReader do subject { described_class.new(job).read(path) } context 'when job has artifacts and metadata' do - let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } - let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) } + shared_examples 'extracting job artifact archive' do + it 'returns the content at the path' do + is_expected.to be_present + expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') + end - it 'returns the content at the path' do - is_expected.to be_present - expect(YAML.safe_load(subject).keys).to contain_exactly('rspec', 'time', 'custom') - end + context 'when path does not exist' do + let(:path) { 'file/does/not/exist.txt' } + let(:expected_error) do + "Path `#{path}` does not exist inside the `#{job.name}` artifacts archive!" + end - context 'when path does not exist' do - let(:path) { 'file/does/not/exist.txt' } - let(:expected_error) do - "Path `#{path}` does not exist inside the `#{job.name}` artifacts archive!" + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end - it 'raises an error' do - expect { subject }.to raise_error(described_class::Error, expected_error) + context 'when path points to a directory' do + let(:path) { 'other_artifacts_0.1.2' } + let(:expected_error) do + "Path `#{path}` was expected to be a file but it was a directory!" + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end - end - context 'when path points to a directory' do - let(:path) { 'other_artifacts_0.1.2' } - let(:expected_error) do - "Path `#{path}` was expected to be a file but it was a directory!" + context 'when path is nested' do + # path exists in ci_build_artifacts.zip + let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' } + + it 'returns the content at the nested path' do + is_expected.to be_present + end end - it 'raises an error' do - expect { subject }.to raise_error(described_class::Error, expected_error) + context 'when artifact archive size is greater than the limit' do + let(:expected_error) do + "Artifacts archive for job `#{job.name}` is too large: max 1 KB" + end + + before do + stub_const("#{described_class}::MAX_ARCHIVE_SIZE", 1.kilobyte) + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end - end - context 'when path is nested' do - # path exists in ci_build_artifacts.zip - let(:path) { 'other_artifacts_0.1.2/doc_sample.txt' } + context 'when metadata entry shows size greater than the limit' do + let(:expected_error) do + "Artifacts archive for job `#{job.name}` is too large: max 5 MB" + end - it 'returns the content at the nested path' do - is_expected.to be_present + before do + expect_next_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) do |entry| + expect(entry).to receive(:total_size).and_return(10.megabytes) + end + end + + it 'raises an error' do + expect { subject }.to raise_error(described_class::Error, expected_error) + end end end - context 'when artifact archive size is greater than the limit' do - let(:expected_error) do - "Artifacts archive for job `#{job.name}` is too large: max 1 KB" - end + context 'when job artifact is on local storage' do + let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } + let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) } + + it_behaves_like 'extracting job artifact archive' + end + context 'when job artifact is on remote storage' do before do - stub_const("#{described_class}::MAX_ARCHIVE_SIZE", 1.kilobyte) + stub_artifacts_object_storage + stub_request(:get, %r{https://artifacts.+ci_build_artifacts\.zip}) + .to_return( + status: 200, + body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts.zip')), + headers: {} + ) + stub_request(:get, %r{https://artifacts.+ci_build_artifacts_metadata}) + .to_return( + status: 200, + body: File.open(Rails.root.join('spec/fixtures/ci_build_artifacts_metadata.gz')), + headers: {} + ) end - it 'raises an error' do - expect { subject }.to raise_error(described_class::Error, expected_error) - end + let!(:artifacts) { create(:ci_job_artifact, :archive, :remote_store, job: job) } + let!(:metadata) { create(:ci_job_artifact, :metadata, :remote_store, job: job) } + + it_behaves_like 'extracting job artifact archive' end - context 'when metadata entry shows size greater than the limit' do - let(:expected_error) do - "Artifacts archive for job `#{job.name}` is too large: max 5 MB" - end + context 'when extracting job artifact raises entry size error' do + let!(:artifacts) { create(:ci_job_artifact, :archive, job: job) } + let!(:metadata) { create(:ci_job_artifact, :metadata, job: job) } before do - expect_next_instance_of(Gitlab::Ci::Build::Artifacts::Metadata::Entry) do |entry| - expect(entry).to receive(:total_size).and_return(10.megabytes) + allow_next_instance_of(SafeZip::Extract, anything) do |extractor| + allow(extractor).to receive(:extract).and_raise(SafeZip::Extract::EntrySizeError) end end it 'raises an error' do + expected_error = "Path `#{path}` has invalid size in the zip!" + expect { subject }.to raise_error(described_class::Error, expected_error) end end diff --git a/spec/lib/safe_zip/entry_spec.rb b/spec/lib/safe_zip/entry_spec.rb index 9929b8073a0..8d49e2bcece 100644 --- a/spec/lib/safe_zip/entry_spec.rb +++ b/spec/lib/safe_zip/entry_spec.rb @@ -5,12 +5,13 @@ require 'spec_helper' RSpec.describe SafeZip::Entry do let(:target_path) { Dir.mktmpdir('safe-zip') } let(:directories) { %w(public folder/with/subfolder) } - let(:params) { SafeZip::ExtractParams.new(directories: directories, to: target_path) } + let(:files) { %w(public/index.html public/assets/image.png) } + let(:params) { SafeZip::ExtractParams.new(directories: directories, files: files, to: target_path) } let(:entry) { described_class.new(zip_archive, zip_entry, params) } let(:entry_name) { 'public/folder/index.html' } let(:entry_path_dir) { File.join(target_path, File.dirname(entry_name)) } - let(:entry_path) { File.join(target_path, entry_name) } + let(:entry_path) { File.join(File.realpath(target_path), entry_name) } let(:zip_archive) { double } let(:zip_entry) do @@ -28,7 +29,7 @@ RSpec.describe SafeZip::Entry do describe '#path_dir' do subject { entry.path_dir } - it { is_expected.to eq(target_path + '/public/folder') } + it { is_expected.to eq(File.realpath(target_path) + '/public/folder') } end describe '#exist?' do @@ -51,6 +52,9 @@ RSpec.describe SafeZip::Entry do subject { entry.extract } context 'when entry does not match the filtered directories' do + let(:directories) { %w(public folder/with/subfolder) } + let(:files) { [] } + using RSpec::Parameterized::TableSyntax where(:entry_name) do @@ -70,7 +74,30 @@ RSpec.describe SafeZip::Entry do end end - context 'when entry does exist' do + context 'when entry does not match the filtered files' do + let(:directories) { [] } + let(:files) { %w(public/index.html public/assets/image.png) } + + using RSpec::Parameterized::TableSyntax + + where(:entry_name) do + [ + 'assets/folder/index.html', + 'public/../folder/index.html', + 'public/../../../../../index.html', + '../../../../../public/index.html', + '/etc/passwd' + ] + end + + with_them do + it 'does not extract file' do + is_expected.to be_falsey + end + end + end + + context 'when there is an existing extracted entry' do before do create_entry end diff --git a/spec/lib/safe_zip/extract_params_spec.rb b/spec/lib/safe_zip/extract_params_spec.rb index 880c4358663..0ebfb7430c5 100644 --- a/spec/lib/safe_zip/extract_params_spec.rb +++ b/spec/lib/safe_zip/extract_params_spec.rb @@ -4,8 +4,10 @@ require 'spec_helper' RSpec.describe SafeZip::ExtractParams do let(:target_path) { Dir.mktmpdir("safe-zip") } - let(:params) { described_class.new(directories: directories, to: target_path) } + let(:real_target_path) { File.realpath(target_path) } + let(:params) { described_class.new(directories: directories, files: files, to: target_path) } let(:directories) { %w(public folder/with/subfolder) } + let(:files) { %w(public/index.html public/assets/image.png) } after do FileUtils.remove_entry_secure(target_path) @@ -14,13 +16,13 @@ RSpec.describe SafeZip::ExtractParams do describe '#extract_path' do subject { params.extract_path } - it { is_expected.to eq(target_path) } + it { is_expected.to eq(real_target_path) } end describe '#matching_target_directory' do using RSpec::Parameterized::TableSyntax - subject { params.matching_target_directory(target_path + path) } + subject { params.matching_target_directory(real_target_path + path) } where(:path, :result) do '/public/index.html' | '/public/' @@ -30,7 +32,7 @@ RSpec.describe SafeZip::ExtractParams do end with_them do - it { is_expected.to eq(result ? target_path + result : nil) } + it { is_expected.to eq(result ? real_target_path + result : nil) } end end @@ -38,7 +40,7 @@ RSpec.describe SafeZip::ExtractParams do subject { params.target_directories } it 'starts with target_path' do - is_expected.to all(start_with(target_path + '/')) + is_expected.to all(start_with(real_target_path + '/')) end it 'ends with / for all paths' do @@ -53,4 +55,27 @@ RSpec.describe SafeZip::ExtractParams do is_expected.to all(end_with('/*')) end end + + describe '#matching_target_file' do + using RSpec::Parameterized::TableSyntax + + subject { params.matching_target_file(real_target_path + path) } + + where(:path, :result) do + '/public/index.html' | true + '/non/existing/path' | false + '/public/' | false + '/folder/with/index.html' | false + end + + with_them do + it { is_expected.to eq(result) } + end + end + + context 'when directories and files are empty' do + it 'is invalid' do + expect { described_class.new(to: target_path) }.to raise_error(ArgumentError, /directories or files are required/) + end + end end diff --git a/spec/lib/safe_zip/extract_spec.rb b/spec/lib/safe_zip/extract_spec.rb index 443430b267d..c727475e271 100644 --- a/spec/lib/safe_zip/extract_spec.rb +++ b/spec/lib/safe_zip/extract_spec.rb @@ -5,6 +5,7 @@ require 'spec_helper' RSpec.describe SafeZip::Extract do let(:target_path) { Dir.mktmpdir('safe-zip') } let(:directories) { %w(public) } + let(:files) { %w(public/index.html) } let(:object) { described_class.new(archive) } let(:archive) { Rails.root.join('spec', 'fixtures', 'safe_zip', archive_name) } @@ -13,20 +14,36 @@ RSpec.describe SafeZip::Extract do end describe '#extract' do - subject { object.extract(directories: directories, to: target_path) } + subject { object.extract(directories: directories, files: files, to: target_path) } shared_examples 'extracts archive' do - it 'does extract archive' do - subject + context 'when specifying directories' do + subject { object.extract(directories: directories, to: target_path) } - expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) - expect(File.exist?(File.join(target_path, 'source'))).to eq(false) + it 'does extract archive' do + subject + + expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) + expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(true) + expect(File.exist?(File.join(target_path, 'source'))).to eq(false) + end + end + + context 'when specifying files' do + subject { object.extract(files: files, to: target_path) } + + it 'does extract archive' do + subject + + expect(File.exist?(File.join(target_path, 'public', 'index.html'))).to eq(true) + expect(File.exist?(File.join(target_path, 'public', 'assets', 'image.png'))).to eq(false) + end end end shared_examples 'fails to extract archive' do it 'does not extract archive' do - expect { subject }.to raise_error(SafeZip::Extract::Error) + expect { subject }.to raise_error(SafeZip::Extract::Error, including(error_message)) end end @@ -38,9 +55,18 @@ RSpec.describe SafeZip::Extract do end end - %w(invalid-symlink-does-not-exist.zip invalid-symlinks-outside.zip).each do |name| - context "when using #{name} archive" do + context 'when zip files are invalid' do + using RSpec::Parameterized::TableSyntax + + where(:name, :message) do + 'invalid-symlink-does-not-exist.zip' | 'does not exist' + 'invalid-symlinks-outside.zip' | 'Symlink cannot be created' + 'invalid-unexpected-large.zip' | 'larger when inflated' + end + + with_them do let(:archive_name) { name } + let(:error_message) { message } it_behaves_like 'fails to extract archive' end @@ -49,6 +75,19 @@ RSpec.describe SafeZip::Extract do context 'when no matching directories are found' do let(:archive_name) { 'valid-simple.zip' } let(:directories) { %w(non/existing) } + let(:error_message) { 'No entries extracted' } + + subject { object.extract(directories: directories, to: target_path) } + + it_behaves_like 'fails to extract archive' + end + + context 'when no matching files are found' do + let(:archive_name) { 'valid-simple.zip' } + let(:files) { %w(non/existing) } + let(:error_message) { 'No entries extracted' } + + subject { object.extract(files: files, to: target_path) } it_behaves_like 'fails to extract archive' end diff --git a/spec/models/concerns/issuable_spec.rb b/spec/models/concerns/issuable_spec.rb index e553e34ab51..206b3ae61cf 100644 --- a/spec/models/concerns/issuable_spec.rb +++ b/spec/models/concerns/issuable_spec.rb @@ -65,7 +65,6 @@ RSpec.describe Issuable do it { is_expected.to validate_presence_of(:author) } it { is_expected.to validate_presence_of(:title) } it { is_expected.to validate_length_of(:title).is_at_most(described_class::TITLE_LENGTH_MAX) } - it { is_expected.to validate_length_of(:description).is_at_most(described_class::DESCRIPTION_LENGTH_MAX).on(:create) } it_behaves_like 'validates description length with custom validation' do before do diff --git a/spec/models/concerns/sanitizable_spec.rb b/spec/models/concerns/sanitizable_spec.rb index 4a1d463d666..be7169f8dca 100644 --- a/spec/models/concerns/sanitizable_spec.rb +++ b/spec/models/concerns/sanitizable_spec.rb @@ -75,7 +75,58 @@ RSpec.describe Sanitizable do it 'is not valid', :aggregate_failures do expect(record).not_to be_valid - expect(record.errors.full_messages).to include('Name cannot contain escaped HTML entities') + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain escaped HTML entities', + 'Description cannot contain escaped HTML entities' + ) + end + end + + context 'when input contains double-escaped data' do + let_it_be(:input) do + '%2526lt%253Bscript%2526gt%253Balert%25281%2529%2526lt%253B%252Fscript%2526gt%253B' + end + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain escaped components', + 'Description cannot contain escaped components' + ) + end + end + + context 'when input contains a path traversal attempt' do + let_it_be(:input) { 'main../../../../../../api/v4/projects/1/import_project_members/2' } + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain a path traversal component', + 'Description cannot contain a path traversal component' + ) + end + end + + context 'when input contains both path traversal attempt and pre-escaped entities' do + let_it_be(:input) do + 'main../../../../../../api/v4/projects/1/import_project_members/2<script>alert(1)</script>' + end + + it_behaves_like 'noop' + + it 'is not valid', :aggregate_failures do + expect(record).not_to be_valid + expect(record.errors.full_messages).to contain_exactly( + 'Name cannot contain a path traversal component', + 'Name cannot contain escaped HTML entities', + 'Description cannot contain a path traversal component', + 'Description cannot contain escaped HTML entities' + ) end end end diff --git a/spec/models/namespace_setting_spec.rb b/spec/models/namespace_setting_spec.rb index 0bf6fdf4fa0..15b80749aa2 100644 --- a/spec/models/namespace_setting_spec.rb +++ b/spec/models/namespace_setting_spec.rb @@ -18,7 +18,7 @@ RSpec.describe NamespaceSetting, feature_category: :subgroups, type: :model do describe "#default_branch_name_content" do let_it_be(:group) { create(:group) } - let(:namespace_settings) { group.namespace_settings } + subject(:namespace_settings) { group.namespace_settings } shared_examples "doesn't return an error" do it "doesn't return an error" do @@ -28,6 +28,10 @@ RSpec.describe NamespaceSetting, feature_category: :subgroups, type: :model do end context "when not set" do + before do + namespace_settings.default_branch_name = nil + end + it_behaves_like "doesn't return an error" end diff --git a/spec/requests/api/draft_notes_spec.rb b/spec/requests/api/draft_notes_spec.rb new file mode 100644 index 00000000000..fad93fff839 --- /dev/null +++ b/spec/requests/api/draft_notes_spec.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe API::DraftNotes, feature_category: :code_review_workflow do + let_it_be(:user) { create(:user) } + let_it_be(:project) { create(:project, :public) } + let_it_be(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + + let_it_be(:merge_request_note) { create(:note, noteable: merge_request, project: project, author: user) } + let_it_be(:draft_note_by_current_user) { create(:draft_note, merge_request: merge_request, author: user) } + let_it_be(:draft_note_by_random_user) { create(:draft_note, merge_request: merge_request) } + + before do + project.add_developer(user) + end + + describe "Get a list of merge request draft notes" do + it "returns 200 OK status" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes", user) + + expect(response).to have_gitlab_http_status(:ok) + end + + it "returns only draft notes authored by the current user" do + get api("/projects/#{project.id}/merge_requests/#{merge_request.iid}/draft_notes", user) + + draft_note_ids = json_response.pluck("id") + + expect(draft_note_ids).to include(draft_note_by_current_user.id) + expect(draft_note_ids).not_to include(draft_note_by_random_user.id) + expect(draft_note_ids).not_to include(merge_request_note.id) + end + end +end diff --git a/spec/scripts/trigger-build_spec.rb b/spec/scripts/trigger-build_spec.rb index 760b9bda541..78cc57b6c91 100644 --- a/spec/scripts/trigger-build_spec.rb +++ b/spec/scripts/trigger-build_spec.rb @@ -16,7 +16,7 @@ RSpec.describe Trigger, feature_category: :tooling do 'CI_COMMIT_SHA' => 'ci_commit_sha', 'CI_MERGE_REQUEST_PROJECT_ID' => 'ci_merge_request_project_id', 'CI_MERGE_REQUEST_IID' => 'ci_merge_request_iid', - 'GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN' => 'bot-token', + 'PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE' => 'bot-token', 'CI_JOB_TOKEN' => 'job-token', 'GITLAB_USER_NAME' => 'gitlab_user_name', 'GITLAB_USER_LOGIN' => 'gitlab_user_login', @@ -26,7 +26,7 @@ RSpec.describe Trigger, feature_category: :tooling do end let(:com_api_endpoint) { 'https://gitlab.com/api/v4' } - let(:com_api_token) { env['GITLAB_BOT_MULTI_PROJECT_PIPELINE_POLLING_TOKEN'] } + let(:com_api_token) { env['PROJECT_TOKEN_FOR_CI_SCRIPTS_API_USAGE'] } let(:com_gitlab_client) { double('com_gitlab_client') } let(:downstream_gitlab_client_endpoint) { com_api_endpoint } @@ -114,25 +114,6 @@ RSpec.describe Trigger, feature_category: :tooling do subject.invoke! end - - context 'with downstream_job_name: "foo"' do - let(:downstream_job) { Struct.new(:id, :name).new(42, 'foo') } - let(:paginated_resources) { Struct.new(:auto_paginate).new([downstream_job]) } - - before do - stub_env('CI_COMMIT_REF_NAME', "#{ref}-ee") - end - - it 'fetches the downstream job' do - expect_run_trigger_with_params - expect(downstream_gitlab_client).to receive(:pipeline_jobs) - .with(downstream_project_path, stubbed_pipeline.id).and_return(paginated_resources) - expect(Trigger::Job).to receive(:new) - .with(downstream_project_path, downstream_job.id, downstream_gitlab_client) - - subject.invoke!(downstream_job_name: 'foo') - end - end end end diff --git a/spec/services/packages/helm/extract_file_metadata_service_spec.rb b/spec/services/packages/helm/extract_file_metadata_service_spec.rb index 273f679b736..f4c61c12344 100644 --- a/spec/services/packages/helm/extract_file_metadata_service_spec.rb +++ b/spec/services/packages/helm/extract_file_metadata_service_spec.rb @@ -54,4 +54,17 @@ RSpec.describe Packages::Helm::ExtractFileMetadataService do it { expect { subject }.to raise_error(described_class::ExtractionError, 'Error while parsing Chart.yaml: (<unknown>): did not find expected node content while parsing a flow node at line 2 column 1') } end + + context 'with a corrupted Chart.yaml of incorrect size' do + let(:helm_fixture_path) { expand_fixture_path('packages/helm/corrupted_chart.tgz') } + let(:expected_error_message) { 'Chart.yaml too big' } + + before do + allow(Zlib::GzipReader).to receive(:new).and_return(Zlib::GzipReader.new(File.open(helm_fixture_path))) + end + + it 'raises an error with the expected message' do + expect { subject }.to raise_error(::Packages::Helm::ExtractFileMetadataService::ExtractionError, expected_error_message) + end + end end diff --git a/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb b/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb index 3a407088997..f49ec906382 100644 --- a/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/issuable_shared_examples.rb @@ -10,40 +10,111 @@ RSpec.shared_examples 'matches_cross_reference_regex? fails fast' do end RSpec.shared_examples 'validates description length with custom validation' do - let(:issuable) { build(:issue, description: 'x' * (::Issuable::DESCRIPTION_LENGTH_MAX + 1)) } - let(:context) { :update } + let(:invalid_description) { 'x' * (::Issuable::DESCRIPTION_LENGTH_MAX + 1) } + let(:valid_description) { 'short description' } + let(:issuable) { build(:issue, description: description) } + + let(:error_message) do + format( + _('is too long (%{size}). The maximum size is %{max_size}.'), + size: ActiveSupport::NumberHelper.number_to_human_size(invalid_description.bytesize), + max_size: ActiveSupport::NumberHelper.number_to_human_size(::Issuable::DESCRIPTION_LENGTH_MAX) + ) + end - subject { issuable.validate(context) } + subject(:validate) { issuable.validate(context) } context 'when Issuable is a new record' do - it 'validates the maximum description length' do - subject - expect(issuable.errors[:description]).to eq(["is too long (maximum is #{::Issuable::DESCRIPTION_LENGTH_MAX} characters)"]) - end + let(:context) { :create } + + context 'when description exceeds the maximum size' do + let(:description) { invalid_description } - context 'on create' do - let(:context) { :create } + it 'adds a description too long error' do + validate - it 'does not validate the maximum description length' do - allow(issuable).to receive(:description_max_length_for_new_records_is_valid).and_call_original + expect(issuable.errors[:description]).to contain_exactly(error_message) + end + end - subject + context 'when description is within the allowed limits' do + let(:description) { valid_description } - expect(issuable).not_to have_received(:description_max_length_for_new_records_is_valid) + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) end end end context 'when Issuable is an existing record' do + let(:context) { :update } + before do allow(issuable).to receive(:expire_etag_cache) # to skip the expire_etag_cache callback + issuable.description = existing_description issuable.save!(validate: false) + issuable.description = description + end + + context 'when record already had a valid description' do + let(:existing_description) { 'small difference so it triggers description_changed?' } + + context 'when new description exceeds the maximum size' do + let(:description) { invalid_description } + + it 'adds a description too long error' do + validate + + expect(issuable.errors[:description]).to contain_exactly(error_message) + end + end + + context 'when new description is within the allowed limits' do + let(:description) { valid_description } + + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end end - it 'does not validate the maximum description length' do - subject - expect(issuable.errors).not_to have_key(:description) + context 'when record existed with an invalid description' do + let(:existing_description) { "#{invalid_description} small difference so it triggers description_changed?" } + + context 'when description is not changed' do + let(:description) { existing_description } + + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end + + context 'when new description exceeds the maximum size' do + let(:description) { invalid_description } + + it 'allows updating descriptions that already existed above the limit' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end + + context 'when new description is within the allowed limits' do + let(:description) { valid_description } + + it 'does not add a validation error' do + validate + + expect(issuable.errors).not_to have_key(:description) + end + end end end end diff --git a/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb b/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb index aedbfe4deb3..9bfa4ace05c 100644 --- a/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb +++ b/spec/support/shared_examples/models/concerns/sanitizable_shared_examples.rb @@ -32,8 +32,25 @@ RSpec.shared_examples 'sanitizable' do |factory, fields| subject { build(factory, attributes) } it 'is not valid', :aggregate_failures do + error = 'cannot contain escaped HTML entities' + + expect(subject).not_to be_valid + expect(subject.errors.details[field].flat_map(&:values)).to contain_exactly(error) + end + end + + context 'when it contains a path component' do + let_it_be(:input) do + 'main../../../../../../api/v4/projects/1/import_project_members/2' + end + + subject { build(factory, attributes) } + + it 'is not valid', :aggregate_failures do + error = 'cannot contain a path traversal component' + expect(subject).not_to be_valid - expect(subject.errors.details[field].flat_map(&:values)).to include('cannot contain escaped HTML entities') + expect(subject.errors.details[field].flat_map(&:values)).to contain_exactly(error) end end end |