diff options
author | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-11-02 12:18:00 +0000 |
---|---|---|
committer | GitLab Release Tools Bot <delivery-team+release-tools@gitlab.com> | 2022-11-02 12:18:00 +0000 |
commit | 0428f8df7e3456df4953760b94b8e782da2a9935 (patch) | |
tree | df4cb86f8177febe037ef692f50b93ba3d4bb72b | |
parent | a3cc240658e56b307e60b7e4eb8ee2dae8d90d06 (diff) | |
parent | 2165c18956e16945f93018023cd0e45df692c5c9 (diff) | |
download | gitlab-ce-0428f8df7e3456df4953760b94b8e782da2a9935.tar.gz |
Merge remote-tracking branch 'dev/15-5-stable' into 15-5-stable
41 files changed, 525 insertions, 196 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c357c90a384..a497f58b902 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,22 @@ documentation](doc/development/changelog.md) for instructions on adding your own entry. +## 15.5.2 (2022-11-02) + +### Security (11 changes) + +- [Redact confidential references in Jira issue descriptions](gitlab-org/security/gitlab@b6df9d1e4e0c996655a41831fbfae8f457fe1e6b) ([merge request](gitlab-org/security/gitlab!2870)) +- [Forbid reading emojis on internal notes](gitlab-org/security/gitlab@0015523a32c38c184ffef9067d9952d0ef54e3f2) ([merge request](gitlab-org/security/gitlab!2854)) +- [Same-site redirect vulnerability](gitlab-org/security/gitlab@7fd87a5f0b8317d45171fb565c198cda4e65fa34) ([merge request](gitlab-org/security/gitlab!2878)) +- [BYPASS: Stored-XSS with CSP-bypass via scoped labels' color](gitlab-org/security/gitlab@2f1777b305d632b3256076967a798dab65fe6bf4) ([merge request](gitlab-org/security/gitlab!2860)) +- [Fix Running Upstream Pipelines Jobs Without Permission](gitlab-org/security/gitlab@9b3f469da7c0295eb12120027a45ac04f76cdad5) ([merge request](gitlab-org/security/gitlab!2881)) +- [Add length limit to addressable URLs](gitlab-org/security/gitlab@82ffc5825c9a7761d787c66b8c4a1593b3330c50) ([merge request](gitlab-org/security/gitlab!2856)) +- [Add a redirect wall before artifact redirect to pages](gitlab-org/security/gitlab@41a4480b3302ba8a67e94de5420d41298d258585) ([merge request](gitlab-org/security/gitlab!2875)) +- [Sandbox swagger-ui to prevent injection attacks](gitlab-org/security/gitlab@432913f802a093b67f2e5d46cc51b5f13bb16590) ([merge request](gitlab-org/security/gitlab!2857)) +- [Fix external project permission when using CI prefill variables](gitlab-org/security/gitlab@ec872da0ab949f447aec35d64d1db45b5d25b7fd) ([merge request](gitlab-org/security/gitlab!2853)) +- [Resolve users can view audit events from other members](gitlab-org/security/gitlab@34ffe2e88fa462b055f22d6af84fdb93a62fa575) ([merge request](gitlab-org/security/gitlab!2855)) +- [Path traversal fix for Secure Files](gitlab-org/security/gitlab@568c36b34a884cc877b6292b340de9da66537bc8) ([merge request](gitlab-org/security/gitlab!2858)) + ## 15.5.1 (2022-10-24) ### Fixed (2 changes) diff --git a/GITALY_SERVER_VERSION b/GITALY_SERVER_VERSION index ec8a15b48bf..5816f489cf5 100644 --- a/GITALY_SERVER_VERSION +++ b/GITALY_SERVER_VERSION @@ -1 +1 @@ -15.5.1
\ No newline at end of file +15.5.2
\ No newline at end of file @@ -1 +1 @@ -15.5.1
\ No newline at end of file +15.5.2
\ No newline at end of file diff --git a/app/assets/javascripts/blob/openapi/index.js b/app/assets/javascripts/blob/openapi/index.js index 44b75cc3e68..943001b7ec4 100644 --- a/app/assets/javascripts/blob/openapi/index.js +++ b/app/assets/javascripts/blob/openapi/index.js @@ -1,23 +1,29 @@ -import { SwaggerUIBundle } from 'swagger-ui-dist'; -import { createAlert } from '~/flash'; -import { __ } from '~/locale'; +import { setAttributes } from '~/lib/utils/dom_utils'; +import axios from '~/lib/utils/axios_utils'; -export default () => { - const el = document.getElementById('js-openapi-viewer'); +const createSandbox = () => { + const iframeEl = document.createElement('iframe'); + setAttributes(iframeEl, { + src: '/-/sandbox/swagger', + sandbox: 'allow-scripts', + frameBorder: 0, + width: '100%', + // The height will be adjusted dynamically. + // Follow-up issue: https://gitlab.com/gitlab-org/gitlab/-/issues/377969 + height: '1000', + }); + return iframeEl; +}; + +export default async () => { + const wrapperEl = document.getElementById('js-openapi-viewer'); + const sandboxEl = createSandbox(); + + const { data } = await axios.get(wrapperEl.dataset.endpoint); + + wrapperEl.appendChild(sandboxEl); - Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) - .then(() => { - SwaggerUIBundle({ - url: el.dataset.endpoint, - dom_id: '#js-openapi-viewer', - deepLinking: true, - displayOperationId: true, - }); - }) - .catch((error) => { - createAlert({ - message: __('Something went wrong while initializing the OpenAPI viewer'), - }); - throw error; - }); + sandboxEl.addEventListener('load', () => { + sandboxEl.contentWindow.postMessage(data, '*'); + }); }; diff --git a/app/assets/javascripts/lib/swagger.js b/app/assets/javascripts/lib/swagger.js new file mode 100644 index 00000000000..ed646176604 --- /dev/null +++ b/app/assets/javascripts/lib/swagger.js @@ -0,0 +1,43 @@ +import { SwaggerUIBundle } from 'swagger-ui-dist'; +import { safeLoad } from 'js-yaml'; +import { isObject } from '~/lib/utils/type_utility'; + +const renderSwaggerUI = (value) => { + /* SwaggerUIBundle accepts openapi definition + * in only JSON format, so we convert the YAML + * config to JSON if it's not JSON value + */ + let spec = value; + if (!isObject(spec)) { + spec = safeLoad(spec, { json: true }); + } + + Promise.all([import(/* webpackChunkName: 'openapi' */ 'swagger-ui-dist/swagger-ui.css')]) + .then(() => { + SwaggerUIBundle({ + spec, + dom_id: '#swagger-ui', + deepLinking: true, + displayOperationId: true, + }); + }) + .catch((error) => { + throw error; + }); +}; + +const addInitHook = () => { + window.addEventListener( + 'message', + (event) => { + if (event.origin !== window.location.origin) { + return; + } + renderSwaggerUI(event.data); + }, + false, + ); +}; + +addInitHook(); +export default {}; diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index 997d321ac24..40e89a06b46 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -14,7 +14,7 @@ class Projects::ArtifactsController < Projects::ApplicationController before_action :authorize_destroy_artifacts!, only: [:destroy] before_action :extract_ref_name_and_path before_action :validate_artifacts!, except: [:index, :download, :raw, :destroy] - before_action :entry, only: [:file] + before_action :entry, only: [:external_file, :file] MAX_PER_PAGE = 20 @@ -58,12 +58,19 @@ class Projects::ArtifactsController < Projects::ApplicationController render_404 unless @entry.exists? end + # External files are redirected to Gitlab Pages and might have unsecure content + # To warn the user about the possible unsecure content, we show a warning page + # before redirecting the user. + def external_file + @blob = @entry.blob + end + def file blob = @entry.blob conditionally_expand_blob(blob) if blob.external_link?(build) - redirect_to blob.external_url(@project, build) + redirect_to external_file_project_job_artifacts_path(@project, @build, path: params[:path]) else respond_to do |format| format.html do diff --git a/app/controllers/projects/pipelines_controller.rb b/app/controllers/projects/pipelines_controller.rb index 2a8f7171f9c..01f7bb9e2cf 100644 --- a/app/controllers/projects/pipelines_controller.rb +++ b/app/controllers/projects/pipelines_controller.rb @@ -239,8 +239,7 @@ class Projects::PipelinesController < Projects::ApplicationController def config_variables respond_to do |format| format.json do - project = @project.uses_external_project_ci_config? ? @project.ci_config_external_project : @project - result = Ci::ListConfigVariablesService.new(project, current_user).execute(params[:sha]) + result = Ci::ListConfigVariablesService.new(@project, current_user).execute(params[:sha]) result.nil? ? head(:no_content) : render(json: result) end diff --git a/app/controllers/sandbox_controller.rb b/app/controllers/sandbox_controller.rb index a48b2b8a314..dffe6797831 100644 --- a/app/controllers/sandbox_controller.rb +++ b/app/controllers/sandbox_controller.rb @@ -8,4 +8,8 @@ class SandboxController < ApplicationController # rubocop:disable Gitlab/Namespa def mermaid render layout: false end + + def swagger + render layout: false + end end diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 950e0a583bc..cc5ba41191b 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -1338,7 +1338,9 @@ module Ci end def reset_source_bridge!(current_user) + # break recursion when no source_pipeline bridge (first upstream pipeline) return unless bridge_waiting? + return unless current_user.can?(:update_pipeline, source_bridge.pipeline) source_bridge.pending! Ci::AfterRequeueJobService.new(project, current_user).execute(source_bridge) # rubocop:disable CodeReuse/ServiceClass diff --git a/app/services/ci/list_config_variables_service.rb b/app/services/ci/list_config_variables_service.rb index c791a89b804..3890882b3d4 100644 --- a/app/services/ci/list_config_variables_service.rb +++ b/app/services/ci/list_config_variables_service.rb @@ -22,12 +22,13 @@ module Ci end def calculate_reactive_cache(sha) - config = project.ci_config_for(sha) - return {} unless config + config = ::Gitlab::Ci::ProjectConfig.new(project: project, sha: sha) - result = Gitlab::Ci::YamlProcessor.new(config, project: project, - user: current_user, - sha: sha).execute + return {} unless config.exists? + + result = Gitlab::Ci::YamlProcessor.new(config.content, project: project, + user: current_user, + sha: sha).execute result.valid? ? result.variables_with_data : {} end diff --git a/app/views/projects/artifacts/_file_navigation.html.haml b/app/views/projects/artifacts/_file_navigation.html.haml new file mode 100644 index 00000000000..e9109451a69 --- /dev/null +++ b/app/views/projects/artifacts/_file_navigation.html.haml @@ -0,0 +1,12 @@ +.nav-block + %ul.breadcrumb.repo-breadcrumb + %li.breadcrumb-item + = link_to _('Artifacts'), browse_project_job_artifacts_path(project, build) + - path_breadcrumbs do |title, breadcrumb| + - title = truncate(title, length: 40) + %li.breadcrumb-item + - if path == breadcrumb + = link_to file_project_job_artifacts_path(project, build, breadcrumb) do + %strong= title + - else + = link_to title, browse_project_job_artifacts_path(project, build, breadcrumb) diff --git a/app/views/projects/artifacts/_tree_file.html.haml b/app/views/projects/artifacts/_tree_file.html.haml index 03d35c1c989..e120975a8f9 100644 --- a/app/views/projects/artifacts/_tree_file.html.haml +++ b/app/views/projects/artifacts/_tree_file.html.haml @@ -1,13 +1,15 @@ - blob = file.blob -- path_to_file = file_project_job_artifacts_path(@project, @build, path: file.path) - external_link = blob.external_link?(@build) +- if external_link + - path_to_file = external_file_project_job_artifacts_path(@project, @build, path: file.path) +- else + - path_to_file = file_project_job_artifacts_path(@project, @build, path: file.path) %tr.tree-item.js-artifact-tree-row{ data: { link: path_to_file, external_link: "#{external_link}" } } %td.tree-item-file-name = tree_icon('file', blob.mode, blob.name) - if external_link - = link_to path_to_file, class: 'tree-item-file-external-link js-artifact-tree-tooltip str-truncated', - target: '_blank', rel: 'noopener noreferrer', title: _('Opens in a new window') do + = link_to path_to_file, class: 'tree-item-file-external-link js-artifact-tree-tooltip str-truncated' do %span>= blob.name = sprite_icon('external-link', css_class: 'js-artifact-tree-external-icon') - else diff --git a/app/views/projects/artifacts/external_file.html.haml b/app/views/projects/artifacts/external_file.html.haml new file mode 100644 index 00000000000..a014d134e31 --- /dev/null +++ b/app/views/projects/artifacts/external_file.html.haml @@ -0,0 +1,15 @@ +- page_title @path, _('Artifacts'), "#{@build.name} (##{@build.id})", _('Jobs') + += render "projects/jobs/header" + +.tree-holder + = render 'projects/artifacts/file_navigation', project: @project, build: @build, path: @path + + %h2= _("You are being redirected away from GitLab") + %p= _("This page is hosted on GitLab pages but contains user-generated content and may contain malicious code. Do not accept unless you trust the author and source.") + + = link_to @blob.external_url(@project, @build), + @blob.external_url(@project, @build), + target: '_blank', + title: _('Opens in a new window'), + rel: 'noopener noreferrer' diff --git a/app/views/projects/artifacts/file.html.haml b/app/views/projects/artifacts/file.html.haml index e16e3ef266d..5b9e5ad584f 100644 --- a/app/views/projects/artifacts/file.html.haml +++ b/app/views/projects/artifacts/file.html.haml @@ -4,19 +4,7 @@ = render "projects/jobs/header" .tree-holder - .nav-block - %ul.breadcrumb.repo-breadcrumb - %li.breadcrumb-item - = link_to 'Artifacts', browse_project_job_artifacts_path(@project, @build) - - path_breadcrumbs do |title, path| - - title = truncate(title, length: 40) - %li.breadcrumb-item - - if path == @path - = link_to file_project_job_artifacts_path(@project, @build, path) do - %strong= title - - else - = link_to title, browse_project_job_artifacts_path(@project, @build, path) - + = render 'projects/artifacts/file_navigation', project: @project, build: @build, path: @path %article.file-holder - blob = @entry.blob diff --git a/app/views/sandbox/swagger.html.erb b/app/views/sandbox/swagger.html.erb new file mode 100644 index 00000000000..ab3c36e5f4a --- /dev/null +++ b/app/views/sandbox/swagger.html.erb @@ -0,0 +1,9 @@ +<!DOCTYPE html> +<html> + <head> + <%= webpack_bundle_tag("sandboxed_swagger") %> + </head> + <body> + <div id="swagger-ui"></div> + </body> +</html> diff --git a/config/initializers/uri.rb b/config/initializers/uri.rb new file mode 100644 index 00000000000..624f7c4d031 --- /dev/null +++ b/config/initializers/uri.rb @@ -0,0 +1,3 @@ +# frozen_string_literal: true + +URI.singleton_class.prepend(Gitlab::Patch::Uri::ClassMethods) diff --git a/config/routes.rb b/config/routes.rb index 28c08e9bbe7..6cfbb969acb 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -119,6 +119,7 @@ InitializerConnections.with_disabled_database_connections do # sandbox get '/sandbox/mermaid' => 'sandbox#mermaid' + get '/sandbox/swagger' => 'sandbox#swagger' get '/whats_new' => 'whats_new#index' diff --git a/config/routes/project.rb b/config/routes/project.rb index cd9315ba2aa..0b0d370223c 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -86,6 +86,7 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do get :download get :browse, path: 'browse(/*path)', format: false get :file, path: 'file/*path', format: false + get :external_file, path: 'external_file/*path', format: false get :raw, path: 'raw/*path', format: false post :keep end diff --git a/config/routes/repository_deprecated.rb b/config/routes/repository_deprecated.rb index e611b4f665b..32682000941 100644 --- a/config/routes/repository_deprecated.rb +++ b/config/routes/repository_deprecated.rb @@ -18,8 +18,12 @@ scope format: false do constraints: { id: Gitlab::PathRegex.git_reference_regex } get '/refs/:id/logs_tree/*path', - to: redirect('%{namespace_id}/%{project_id}/-/refs/%{id}/logs_tree/%{path}'), - constraints: { id: /.*/, path: /[^\0]*/ } + constraints: { id: /.*/, path: /[^\0]*/ }, + to: redirect { |params, _request| + path = params[:path] + path.gsub!('@', '-/') + Addressable::URI.escape("#{params[:namespace_id]}/#{params[:project_id]}/-/refs/#{params[:id]}/logs_tree/#{path}") + } scope constraints: { id: /[^\0]+/ } do # Deprecated. Keep for compatibility. diff --git a/config/webpack.config.js b/config/webpack.config.js index 05523952769..d79e6e12b39 100644 --- a/config/webpack.config.js +++ b/config/webpack.config.js @@ -165,6 +165,7 @@ function generateEntries() { jira_connect_app: './jira_connect/subscriptions/index.js', sandboxed_mermaid: './lib/mermaid.js', redirect_listbox: './entrypoints/behaviors/redirect_listbox.js', + sandboxed_swagger: './lib/swagger.js', }; return Object.assign(manualEntries, incrementalCompiler.filterEntryPoints(autoEntries)); diff --git a/lib/api/award_emoji.rb b/lib/api/award_emoji.rb index fd36b364d56..e419a025508 100644 --- a/lib/api/award_emoji.rb +++ b/lib/api/award_emoji.rb @@ -100,7 +100,7 @@ module API def read_ability(awardable) case awardable when Note - read_ability(awardable.noteable) + awardable.issuable_ability_name when Snippet, ProjectSnippet :read_snippet else diff --git a/lib/api/ci/secure_files.rb b/lib/api/ci/secure_files.rb index 68431df203b..511b6e06cd3 100644 --- a/lib/api/ci/secure_files.rb +++ b/lib/api/ci/secure_files.rb @@ -66,7 +66,7 @@ module API route_setting :authentication, basic_auth_personal_access_token: true, job_token_allowed: true post ':id/secure_files' do secure_file = user_project.secure_files.new( - name: params[:name] + name: Gitlab::Utils.check_path_traversal!(params[:name]) ) secure_file.file = params[:file] diff --git a/lib/gitlab/content_security_policy/config_loader.rb b/lib/gitlab/content_security_policy/config_loader.rb index 8648ffe5f49..f1faade250e 100644 --- a/lib/gitlab/content_security_policy/config_loader.rb +++ b/lib/gitlab/content_security_policy/config_loader.rb @@ -154,7 +154,7 @@ module Gitlab # Using 'self' in the CSP introduces several CSP bypass opportunities # for this reason we list the URLs where GitLab frames itself instead def self.allow_framed_gitlab_paths(directives) - ['/admin/', '/assets/', '/-/speedscope/index.html', '/-/sandbox/mermaid'].map do |path| + ['/admin/', '/assets/', '/-/speedscope/index.html', '/-/sandbox/'].map do |path| append_to_directive(directives, 'frame_src', Gitlab::Utils.append_path(Gitlab.config.gitlab.url, path)) end end diff --git a/lib/gitlab/patch/uri.rb b/lib/gitlab/patch/uri.rb new file mode 100644 index 00000000000..b3a34b5a68e --- /dev/null +++ b/lib/gitlab/patch/uri.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +module Gitlab + module Patch + module Uri + module ClassMethods + def parse(uri) + raise URI::InvalidURIError, "URI is too long" if uri && uri.to_s.length > 15_000 + + super + end + end + end + end +end diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 545a2ca4855..8160d48fd95 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -38137,9 +38137,6 @@ msgstr "" msgid "Something went wrong while fetching the packages list." msgstr "" -msgid "Something went wrong while initializing the OpenAPI viewer" -msgstr "" - msgid "Something went wrong while obtaining the Let's Encrypt certificate." msgstr "" @@ -41564,6 +41561,9 @@ msgstr "" msgid "This only applies to repository indexing operations." msgstr "" +msgid "This page is hosted on GitLab pages but contains user-generated content and may contain malicious code. Do not accept unless you trust the author and source." +msgstr "" + msgid "This page is unavailable because you are not allowed to read information across multiple projects." msgstr "" @@ -46074,6 +46074,9 @@ msgstr "" msgid "You are attempting to update a file that has changed since you started editing it." msgstr "" +msgid "You are being redirected away from GitLab" +msgstr "" + msgid "You are billed if you exceed this number. %{qsrOverageLinkStart}How does billing work?%{qsrOverageLinkEnd}" msgstr "" diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index 808e67eff3d..f79a2c6a6d0 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -262,6 +262,31 @@ RSpec.describe Projects::ArtifactsController do end end + describe 'GET external_file' do + before do + allow(Gitlab.config.pages).to receive(:enabled).and_return(true) + allow(Gitlab.config.pages).to receive(:artifacts_server).and_return(true) + end + + context 'when the file exists' do + it 'renders the file view' do + path = 'ci_artifacts.txt' + + get :external_file, params: { namespace_id: project.namespace, project_id: project, job_id: job, path: path } + + expect(response).to have_gitlab_http_status(:ok) + end + end + + context 'when the file does not exist' do + it 'responds Not Found' do + get :external_file, params: { namespace_id: project.namespace, project_id: project, job_id: job, path: 'unknown' } + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + describe 'GET file' do before do allow(Gitlab.config.pages).to receive(:enabled).and_return(true) @@ -274,17 +299,11 @@ RSpec.describe Projects::ArtifactsController do context 'when the file exists' do it 'renders the file view' do - get :file, params: { namespace_id: project.namespace, project_id: project, job_id: job, path: 'ci_artifacts.txt' } + path = 'ci_artifacts.txt' - expect(response).to have_gitlab_http_status(:found) - end - end - - context 'when the file does not exist' do - it 'responds Not Found' do - get :file, params: { namespace_id: project.namespace, project_id: project, job_id: job, path: 'unknown' } + get :file, params: { namespace_id: project.namespace, project_id: project, job_id: job, path: path } - expect(response).to be_not_found + expect(response).to redirect_to(external_file_project_job_artifacts_path(project, job, path: path)) end end end diff --git a/spec/controllers/projects/pipelines_controller_spec.rb b/spec/controllers/projects/pipelines_controller_spec.rb index 6e2de0c4d57..b132c0b5a69 100644 --- a/spec/controllers/projects/pipelines_controller_spec.rb +++ b/spec/controllers/projects/pipelines_controller_spec.rb @@ -1360,11 +1360,12 @@ RSpec.describe Projects::PipelinesController do describe 'GET config_variables.json', :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers - let(:result) { YAML.dump(ci_config) } - let(:service) { Ci::ListConfigVariablesService.new(project, user) } + let(:ci_config) { '' } + let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config) } } + let(:project) { create(:project, :auto_devops_disabled, :custom_repo, files: files) } + let(:service) { Ci::ListConfigVariablesService.new(project, user) } before do - stub_gitlab_ci_yml_for_sha(sha, result) allow(Ci::ListConfigVariablesService) .to receive(:new) .and_return(service) @@ -1398,7 +1399,6 @@ RSpec.describe Projects::PipelinesController do context 'when sending an invalid sha' do let(:sha) { 'invalid-sha' } - let(:ci_config) { nil } before do synchronous_reactive_cache(service) @@ -1460,11 +1460,11 @@ RSpec.describe Projects::PipelinesController do end context 'when project uses external project ci config' do - let(:other_project) { create(:project) } + let(:other_project) { create(:project, :custom_repo, files: other_project_files) } + let(:other_project_files) { { '.gitlab-ci.yml' => YAML.dump(other_project_ci_config) } } let(:sha) { 'master' } - let(:service) { ::Ci::ListConfigVariablesService.new(other_project, user) } - let(:ci_config) do + let(:other_project_ci_config) do { variables: { KEY1: { value: 'val 1', description: 'description 1' } @@ -1477,13 +1477,12 @@ RSpec.describe Projects::PipelinesController do end before do - project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}") + other_project.add_developer(user) + project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}:master") synchronous_reactive_cache(service) end it 'returns other project config variables' do - expect(::Ci::ListConfigVariablesService).to receive(:new).with(other_project, anything).and_return(service) - get_config_variables expect(response).to have_gitlab_http_status(:ok) @@ -1493,13 +1492,6 @@ RSpec.describe Projects::PipelinesController do private - def stub_gitlab_ci_yml_for_sha(sha, result) - allow_any_instance_of(Repository) - .to receive(:gitlab_ci_yml_for) - .with(sha, '.gitlab-ci.yml') - .and_return(result) - end - def get_config_variables get :config_variables, params: { namespace_id: project.namespace, project_id: project, diff --git a/spec/controllers/search_controller_spec.rb b/spec/controllers/search_controller_spec.rb index 7ab66b04a6e..392dc2229aa 100644 --- a/spec/controllers/search_controller_spec.rb +++ b/spec/controllers/search_controller_spec.rb @@ -395,7 +395,7 @@ RSpec.describe SearchController do it_behaves_like 'support for active record query timeouts', :autocomplete, { term: 'hello' }, :project, :json it 'returns an empty array when given abusive search term' do - get :autocomplete, params: { term: ('hal' * 9000), scope: 'projects' } + get :autocomplete, params: { term: ('hal' * 4000), scope: 'projects' } expect(response).to have_gitlab_http_status(:ok) expect(json_response).to match_array([]) end diff --git a/spec/features/projects/artifacts/user_browses_artifacts_spec.rb b/spec/features/projects/artifacts/user_browses_artifacts_spec.rb index 2d09f5a4263..c0d710fe186 100644 --- a/spec/features/projects/artifacts/user_browses_artifacts_spec.rb +++ b/spec/features/projects/artifacts/user_browses_artifacts_spec.rb @@ -81,12 +81,11 @@ RSpec.describe "User browses artifacts" do end it "shows correct content" do - link = first(".tree-item-file-external-link") - - expect(link[:target]).to eq("_blank") - expect(link[:rel]).to include("noopener").and include("noreferrer") - expect(page).to have_link("doc_sample.txt", href: file_project_job_artifacts_path(project, job, path: txt_entry.blob.path)) - .and have_selector(".js-artifact-tree-external-icon") + expect(page) + .to have_link( + "doc_sample.txt", + href: external_file_project_job_artifacts_path(project, job, path: txt_entry.blob.path) + ).and have_selector(".js-artifact-tree-external-icon") page.within(".tree-table") do expect(page).to have_content("..").and have_content("another-subdirectory") diff --git a/spec/features/projects/blobs/blob_show_spec.rb b/spec/features/projects/blobs/blob_show_spec.rb index 93e5be18229..d679d1eeeb9 100644 --- a/spec/features/projects/blobs/blob_show_spec.rb +++ b/spec/features/projects/blobs/blob_show_spec.rb @@ -1001,11 +1001,9 @@ RSpec.describe 'File blob', :js do wait_for_requests end - it 'removes `style`, `class`, and `data-*`` attributes from HTML' do - expect(page).to have_css('h1', text: 'Swagger API documentation') - expect(page).not_to have_css('.foo-bar') - expect(page).not_to have_css('[style="background-color: red;"]') - expect(page).not_to have_css('[data-foo-bar="baz"]') + it 'renders sandboxed iframe' do + expected = %(<iframe src="/-/sandbox/swagger" sandbox="allow-scripts" frameborder="0" width="100%" height="1000">) + expect(page.html).to include(expected) end end end diff --git a/spec/frontend/blob/openapi/index_spec.js b/spec/frontend/blob/openapi/index_spec.js index 53220809f80..5884b27d951 100644 --- a/spec/frontend/blob/openapi/index_spec.js +++ b/spec/frontend/blob/openapi/index_spec.js @@ -1,28 +1,27 @@ -import { SwaggerUIBundle } from 'swagger-ui-dist'; +import axios from 'axios'; +import MockAdapter from 'axios-mock-adapter'; import { setHTMLFixture, resetHTMLFixture } from 'helpers/fixtures'; import renderOpenApi from '~/blob/openapi'; -jest.mock('swagger-ui-dist'); - describe('OpenAPI blob viewer', () => { const id = 'js-openapi-viewer'; const mockEndpoint = 'some/endpoint'; + let mock; - beforeEach(() => { + beforeEach(async () => { setHTMLFixture(`<div id="${id}" data-endpoint="${mockEndpoint}"></div>`); - renderOpenApi(); + mock = new MockAdapter(axios).onGet().reply(200); + await renderOpenApi(); }); afterEach(() => { resetHTMLFixture(); + mock.restore(); }); it('initializes SwaggerUI with the correct configuration', () => { - expect(SwaggerUIBundle).toHaveBeenCalledWith({ - url: mockEndpoint, - dom_id: `#${id}`, - deepLinking: true, - displayOperationId: true, - }); + expect(document.body.innerHTML).toContain( + '<iframe src="/-/sandbox/swagger" sandbox="allow-scripts" frameborder="0" width="100%" height="1000"></iframe>', + ); }); }); diff --git a/spec/helpers/labels_helper_spec.rb b/spec/helpers/labels_helper_spec.rb index 90366d7772c..85420d4afda 100644 --- a/spec/helpers/labels_helper_spec.rb +++ b/spec/helpers/labels_helper_spec.rb @@ -310,4 +310,15 @@ RSpec.describe LabelsHelper do end end end + + describe '#wrap_label_html' do + let(:project) { build_stubbed(:project) } + let(:xss_label) do + build_stubbed(:label, name: 'xsslabel', project: project, color: '"><img src=x onerror=prompt(1)>') + end + + it 'does not include the color' do + expect(wrap_label_html('xss', label: xss_label, small: false)).not_to include('color:') + end + end end diff --git a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb index 616fe15c1a6..6b1d8d8d1af 100644 --- a/spec/lib/gitlab/content_security_policy/config_loader_spec.rb +++ b/spec/lib/gitlab/content_security_policy/config_loader_spec.rb @@ -85,7 +85,7 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do expect(directives['style_src']).to eq("'self' 'unsafe-inline' https://cdn.example.com") expect(directives['font_src']).to eq("'self' https://cdn.example.com") expect(directives['worker_src']).to eq('http://localhost/assets/ blob: data: https://cdn.example.com') - expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " https://cdn.example.com http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html http://localhost/-/sandbox/mermaid") + expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " https://cdn.example.com http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html http://localhost/-/sandbox/") end end @@ -108,7 +108,7 @@ RSpec.describe Gitlab::ContentSecurityPolicy::ConfigLoader do end it 'adds CUSTOMER_PORTAL_URL to CSP' do - expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html http://localhost/-/sandbox/mermaid #{customer_portal_url}") + expect(directives['frame_src']).to eq(::Gitlab::ContentSecurityPolicy::Directives.frame_src + " http://localhost/admin/ http://localhost/assets/ http://localhost/-/speedscope/index.html http://localhost/-/sandbox/ #{customer_portal_url}") end end diff --git a/spec/lib/gitlab/patch/uri_spec.rb b/spec/lib/gitlab/patch/uri_spec.rb new file mode 100644 index 00000000000..8232bb71fa4 --- /dev/null +++ b/spec/lib/gitlab/patch/uri_spec.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::Patch::Uri do + describe '#parse' do + it 'raises an error if the URI is too long' do + expect { URI.parse("https://example.com/#{'a' * 25_000}") }.to raise_error(URI::InvalidURIError) + end + + it 'does not raise an error if the URI is not too long' do + expect { URI.parse("https://example.com/#{'a' * 14_000}") }.not_to raise_error + end + end +end diff --git a/spec/models/ci/pipeline_spec.rb b/spec/models/ci/pipeline_spec.rb index b2316949497..42b5210a080 100644 --- a/spec/models/ci/pipeline_spec.rb +++ b/spec/models/ci/pipeline_spec.rb @@ -5158,74 +5158,147 @@ RSpec.describe Ci::Pipeline, :mailer, factory_default: :keep do end describe '#reset_source_bridge!' do - let(:pipeline) { create(:ci_pipeline, :created, project: project) } + subject(:reset_bridge) { pipeline.reset_source_bridge!(current_user) } - subject(:reset_bridge) { pipeline.reset_source_bridge!(project.first_owner) } + context 'with downstream pipeline' do + let_it_be(:owner) { project.first_owner } - context 'when the pipeline is a child pipeline and the bridge is depended' do - let!(:parent_pipeline) { create(:ci_pipeline) } - let!(:bridge) { create_bridge(parent_pipeline, pipeline, true) } + let!(:first_upstream_pipeline) { create(:ci_pipeline, user: owner) } + let_it_be_with_reload(:pipeline) { create(:ci_pipeline, :created, project: project, user: owner) } - it 'marks source bridge as pending' do - reset_bridge - - expect(bridge.reload).to be_pending + let!(:bridge) do + create_bridge( + upstream: first_upstream_pipeline, + downstream: pipeline, + depends: true + ) end - context 'when the parent pipeline has subsequent jobs after the bridge' do - let!(:after_bridge_job) { create(:ci_build, :skipped, pipeline: parent_pipeline, stage_idx: bridge.stage_idx + 1) } + context 'when the user has permissions for the processable' do + let(:current_user) { owner } - it 'marks subsequent jobs of the bridge as processable' do - reset_bridge + context 'when the downstream has strategy: depend' do + it 'marks source bridge as pending' do + expect { reset_bridge } + .to change { bridge.reload.status } + .to('pending') + end - expect(after_bridge_job.reload).to be_created - end - end + context 'with subsequent jobs' do + let!(:after_bridge_job) { add_bridge_dependant_job } + let!(:bridge_dependant_dag_job) { add_bridge_dependant_dag_job } - context 'when the parent pipeline has a dependent upstream pipeline' do - let!(:upstream_bridge) do - create_bridge(create(:ci_pipeline, project: create(:project)), parent_pipeline, true) - end + it 'changes subsequent job statuses to created' do + expect { reset_bridge } + .to change { after_bridge_job.reload.status } + .from('skipped').to('created') + .and change { bridge_dependant_dag_job.reload.status } + .from('skipped').to('created') + end - it 'marks all source bridges as pending' do - reset_bridge + context 'when the user is not the build user' do + let(:current_user) { create(:user) } - expect(bridge.reload).to be_pending - expect(upstream_bridge.reload).to be_pending - end - end - end + before do + project.add_maintainer(current_user) + end - context 'when the pipeline is a child pipeline and the bridge is not depended' do - let!(:parent_pipeline) { create(:ci_pipeline) } - let!(:bridge) { create_bridge(parent_pipeline, pipeline, false) } + it 'changes subsequent jobs user' do + expect { reset_bridge } + .to change { after_bridge_job.reload.user } + .from(owner).to(current_user) + .and change { bridge_dependant_dag_job.reload.user } + .from(owner).to(current_user) + end + end + end - it 'does not touch source bridge' do - reset_bridge + context 'when the upstream pipeline pipeline has a dependent upstream pipeline' do + let(:upstream_of_upstream) { create(:ci_pipeline, project: create(:project)) } + let!(:upstream_bridge) do + create_bridge( + upstream: upstream_of_upstream, + downstream: first_upstream_pipeline, + depends: true + ) + end + + it 'marks all source bridges as pending' do + expect { reset_bridge } + .to change { bridge.reload.status } + .from('skipped').to('pending') + .and change { upstream_bridge.reload.status } + .from('skipped').to('pending') + end + end - expect(bridge.reload).to be_success + context 'without strategy: depend' do + let!(:upstream_pipeline) { create(:ci_pipeline) } + let!(:bridge) do + create_bridge( + upstream: first_upstream_pipeline, + downstream: pipeline, + depends: false + ) + end + + it 'does not touch source bridge' do + expect { reset_bridge }.to not_change { bridge.status } + end + + context 'when the upstream pipeline has a dependent upstream pipeline' do + let!(:upstream_bridge) do + create_bridge( + upstream: create(:ci_pipeline, project: create(:project)), + downstream: first_upstream_pipeline, + depends: true + ) + end + + it 'does not touch any source bridge' do + expect { reset_bridge }.to not_change { bridge.status } + .and not_change { upstream_bridge.status } + end + end + end + end end - context 'when the parent pipeline has a dependent upstream pipeline' do - let!(:upstream_bridge) do - create_bridge(create(:ci_pipeline, project: create(:project)), parent_pipeline, true) + context 'when the user does not have permissions for the processable' do + let(:current_user) { create(:user) } + + it 'does not change bridge status' do + expect { reset_bridge }.to not_change { bridge.status } end - it 'does not touch any source bridge' do - reset_bridge + context 'with subsequent jobs' do + let!(:after_bridge_job) { add_bridge_dependant_job } + let!(:bridge_dependant_dag_job) { add_bridge_dependant_dag_job } - expect(bridge.reload).to be_success - expect(upstream_bridge.reload).to be_success + it 'does not change job statuses' do + expect { reset_bridge }.to not_change { after_bridge_job.reload.status } + .and not_change { bridge_dependant_dag_job.reload.status } + end end end end private - def create_bridge(upstream, downstream, depend = false) - options = depend ? { trigger: { strategy: 'depend' } } : {} + def add_bridge_dependant_job + create(:ci_build, :skipped, pipeline: first_upstream_pipeline, stage_idx: bridge.stage_idx + 1, user: owner) + end + + def add_bridge_dependant_dag_job + create(:ci_build, :skipped, name: 'dependant-build-1', pipeline: first_upstream_pipeline, user: owner).tap do |build| + create(:ci_build_need, build: build, name: bridge.name) + end + end + + def create_bridge(upstream:, downstream:, depends: false) + options = depends ? { trigger: { strategy: 'depend' } } : {} - bridge = create(:ci_bridge, pipeline: upstream, status: 'success', options: options) + bridge = create(:ci_bridge, pipeline: upstream, status: 'skipped', options: options, user: owner) create(:ci_sources_pipeline, pipeline: downstream, source_job: bridge) bridge diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb index 670a6237788..1b44da75c40 100644 --- a/spec/models/note_spec.rb +++ b/spec/models/note_spec.rb @@ -1888,4 +1888,20 @@ RSpec.describe Note do end end end + + describe '#issuable_ability_name' do + subject { note.issuable_ability_name } + + context 'when not confidential note' do + let(:note) { build(:note) } + + it { is_expected.to eq :read_note } + end + + context 'when confidential note' do + let(:note) { build(:note, :confidential) } + + it { is_expected.to eq :read_internal_note } + end + end end diff --git a/spec/requests/api/award_emoji_spec.rb b/spec/requests/api/award_emoji_spec.rb index 67ddaf2fda5..bb563f93bfe 100644 --- a/spec/requests/api/award_emoji_spec.rb +++ b/spec/requests/api/award_emoji_spec.rb @@ -191,6 +191,36 @@ RSpec.describe API::AwardEmoji do expect(json_response['name']).to eq(rocket.name) end + context 'when a confidential note' do + subject(:perform_request) { get api(request_path, current_user) } + + let_it_be(:group) { create(:group) } + let_it_be(:project) { create(:project, :public, namespace: group) } + let_it_be(:issue) { create(:issue, project: project) } + let_it_be(:note) { create(:note, :confidential, project: project, noteable: issue, author: user) } + + context 'with sufficient persmissions' do + let(:current_user) { user } + + it 'returns an award emoji' do + perform_request + + expect(response).to have_gitlab_http_status(:ok) + expect(json_response['name']).to eq(rocket.name) + end + end + + context 'with insufficient permissions' do + let(:current_user) { nil } + + it 'returns 404' do + perform_request + + expect(response).to have_gitlab_http_status(:not_found) + end + end + end + it_behaves_like 'unauthenticated request to public awardable' it_behaves_like 'request with insufficient permissions', :get end diff --git a/spec/requests/api/ci/secure_files_spec.rb b/spec/requests/api/ci/secure_files_spec.rb index f1f22dfadc2..0b8116d5e20 100644 --- a/spec/requests/api/ci/secure_files_spec.rb +++ b/spec/requests/api/ci/secure_files_spec.rb @@ -341,6 +341,15 @@ RSpec.describe API::Ci::SecureFiles do expect(response).to have_gitlab_http_status(:payload_too_large) end + + it 'returns an error when and invalid file name is supplied' do + params = file_params.merge(name: '../../upload-keystore.jks') + expect do + post api("/projects/#{project.id}/secure_files", maintainer), params: params + end.not_to change { project.secure_files.count } + + expect(response).to have_gitlab_http_status(:internal_server_error) + end end context 'authenticated user with read permissions' do diff --git a/spec/routing/project_routing_spec.rb b/spec/routing/project_routing_spec.rb index 9317a661188..875a54de3d1 100644 --- a/spec/routing/project_routing_spec.rb +++ b/spec/routing/project_routing_spec.rb @@ -189,6 +189,7 @@ RSpec.describe 'project routing' do end it 'to #logs_tree' do + expect(get('/gitlab/gitlabhq/-/refs/stable/logs_tree/..%2F..%2F..%2F..%2F..%2F@example.com/tree/a')).to route_to('projects/refs#logs_tree', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'stable', path: '../../../../../@example.com/tree/a') expect(get('/gitlab/gitlabhq/-/refs/stable/logs_tree')).to route_to('projects/refs#logs_tree', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'stable') expect(get('/gitlab/gitlabhq/-/refs/feature%2345/logs_tree')).to route_to('projects/refs#logs_tree', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature#45') expect(get('/gitlab/gitlabhq/-/refs/feature%2B45/logs_tree')).to route_to('projects/refs#logs_tree', namespace_id: 'gitlab', project_id: 'gitlabhq', id: 'feature+45') @@ -214,6 +215,10 @@ RSpec.describe 'project routing' do it_behaves_like 'redirecting a legacy path', '/gitlab/gitlabhq/refs/stable/logs_tree/new%0A%0Aline.txt', '/gitlab/gitlabhq/-/refs/stable/logs_tree/new%0A%0Aline.txt' + + it_behaves_like 'redirecting a legacy path', + '/gitlab/gitlabhq/refs/feature%2345/logs_tree/../../../../../@example.com/tree/a', + '/gitlab/gitlabhq/-/refs/feature#45/logs_tree/../../../../../-/example.com/tree/a' end describe Projects::MergeRequestsController, 'routing' do diff --git a/spec/services/ci/list_config_variables_service_spec.rb b/spec/services/ci/list_config_variables_service_spec.rb index 4953b18bfcc..5b865914d1b 100644 --- a/spec/services/ci/list_config_variables_service_spec.rb +++ b/spec/services/ci/list_config_variables_service_spec.rb @@ -5,19 +5,16 @@ require 'spec_helper' RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_caching do include ReactiveCachingHelpers - let(:project) { create(:project, :repository) } + let(:ci_config) { {} } + let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config) } } + let(:project) { create(:project, :custom_repo, :auto_devops_disabled, files: files) } let(:user) { project.creator } + let(:sha) { project.default_branch } let(:service) { described_class.new(project, user) } - let(:result) { YAML.dump(ci_config) } - subject { service.execute(sha) } - - before do - stub_gitlab_ci_yml_for_sha(sha, result) - end + subject(:result) { service.execute(sha) } context 'when sending a valid sha' do - let(:sha) { 'master' } let(:ci_config) do { variables: { @@ -38,15 +35,14 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac end it 'returns variable list' do - expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) - expect(subject['KEY2']).to eq({ value: 'val 2', description: '' }) - expect(subject['KEY3']).to eq({ value: 'val 3' }) - expect(subject['KEY4']).to eq({ value: 'val 4' }) + expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) + expect(result['KEY2']).to eq({ value: 'val 2', description: '' }) + expect(result['KEY3']).to eq({ value: 'val 3' }) + expect(result['KEY4']).to eq({ value: 'val 4' }) end end context 'when config has includes' do - let(:sha) { 'master' } let(:ci_config) do { include: [{ local: 'other_file.yml' }], @@ -60,24 +56,56 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac } end - before do - allow_next_instance_of(Repository) do |repository| - allow(repository).to receive(:blob_data_at).with(sha, 'other_file.yml') do - <<~HEREDOC - variables: - KEY2: - value: 'val 2' - description: 'description 2' - HEREDOC - end - end + let(:other_file) do + { + variables: { + KEY2: { value: 'val 2', description: 'description 2' } + } + } + end + + let(:files) { { '.gitlab-ci.yml' => YAML.dump(ci_config), 'other_file.yml' => YAML.dump(other_file) } } + before do synchronous_reactive_cache(service) end it 'returns variable list' do - expect(subject['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) - expect(subject['KEY2']).to eq({ value: 'val 2', description: 'description 2' }) + expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) + expect(result['KEY2']).to eq({ value: 'val 2', description: 'description 2' }) + end + end + + context 'when project CI config is external' do + let(:other_project_ci_config) do + { + variables: { KEY1: { value: 'val 1', description: 'description 1' } }, + test: { script: 'echo' } + } + end + + let(:other_project_files) { { '.gitlab-ci.yml' => YAML.dump(other_project_ci_config) } } + let(:other_project) { create(:project, :custom_repo, files: other_project_files) } + + before do + project.update!(ci_config_path: ".gitlab-ci.yml@#{other_project.full_path}:master") + synchronous_reactive_cache(service) + end + + context 'when the user has access to the external project' do + before do + other_project.add_developer(user) + end + + it 'returns variable list' do + expect(result['KEY1']).to eq({ value: 'val 1', description: 'description 1' }) + end + end + + context 'when the user has no access to the external project' do + it 'returns empty json' do + expect(result).to eq({}) + end end end @@ -90,12 +118,11 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac end it 'returns empty json' do - expect(subject).to eq({}) + expect(result).to eq({}) end end context 'when sending an invalid config' do - let(:sha) { 'master' } let(:ci_config) do { variables: { @@ -113,13 +140,11 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac end it 'returns empty result' do - expect(subject).to eq({}) + expect(result).to eq({}) end end context 'when reading from cache' do - let(:sha) { 'master' } - let(:ci_config) { {} } let(:reactive_cache_params) { [sha] } let(:return_value) { { 'KEY1' => { value: 'val 1', description: 'description 1' } } } @@ -128,13 +153,11 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac end it 'returns variable list' do - expect(subject).to eq(return_value) + expect(result).to eq(return_value) end end context 'when the cache is empty' do - let(:sha) { 'master' } - let(:ci_config) { {} } let(:reactive_cache_params) { [sha] } it 'returns nil and enquques the worker to fill cache' do @@ -142,16 +165,7 @@ RSpec.describe Ci::ListConfigVariablesService, :use_clean_rails_memory_store_cac .to receive(:perform_async) .with(service.class, service.id, *reactive_cache_params) - expect(subject).to be_nil + expect(result).to be_nil end end - - private - - def stub_gitlab_ci_yml_for_sha(sha, result) - allow_any_instance_of(Repository) - .to receive(:gitlab_ci_yml_for) - .with(sha, '.gitlab-ci.yml') - .and_return(result) - end end diff --git a/spec/services/ci/retry_pipeline_service_spec.rb b/spec/services/ci/retry_pipeline_service_spec.rb index 0a1e767539d..96437290ae3 100644 --- a/spec/services/ci/retry_pipeline_service_spec.rb +++ b/spec/services/ci/retry_pipeline_service_spec.rb @@ -313,10 +313,27 @@ RSpec.describe Ci::RetryPipelineService, '#execute' do create(:ci_sources_pipeline, pipeline: pipeline, source_job: bridge) end - it 'marks source bridge as pending' do - service.execute(pipeline) + context 'without permission' do + it 'does nothing to the bridge' do + expect { service.execute(pipeline) }.to not_change { bridge.reload.status } + .and not_change { bridge.reload.user } + end + end + + context 'with permission' do + let!(:bridge_pipeline) { create(:ci_pipeline, project: create(:project)) } + let!(:bridge) do + create(:ci_bridge, :strategy_depend, status: 'success', pipeline: bridge_pipeline) + end - expect(bridge.reload).to be_pending + before do + bridge_pipeline.project.add_maintainer(user) + end + + it 'marks source bridge as pending' do + expect { service.execute(pipeline) }.to change { bridge.reload.status }.to('pending') + .and not_change { bridge.reload.user } + end end end |