From deb2f3a60831afda2ad7ec144eb58aaf269abe58 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Apr 2022 08:18:14 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee --- app/controllers/projects/application_controller.rb | 15 +++++++++ app/controllers/projects/artifacts_controller.rb | 7 ++++ app/controllers/projects/jobs_controller.rb | 12 +------ app/models/commit_status.rb | 8 ++++- app/models/integrations/asana.rb | 4 ++- app/models/integrations/assembla.rb | 15 +++++++-- app/models/integrations/bamboo.rb | 6 ++-- app/models/integrations/base_slash_commands.rb | 8 ++++- app/models/integrations/buildkite.rb | 4 ++- app/models/integrations/campfire.rb | 6 ++-- app/models/integrations/drone_ci.rb | 17 ++++++++-- app/models/integrations/flowdock.rb | 10 +++++- app/models/integrations/harbor.rb | 7 ++-- app/models/integrations/packagist.rb | 4 ++- app/models/integrations/pivotaltracker.rb | 4 ++- app/models/integrations/pushover.rb | 8 +++-- doc/ci/jobs/index.md | 2 +- locale/gitlab.pot | 22 +++++++++++-- .../projects/artifacts_controller_spec.rb | 38 ++++++++++++++++++++++ spec/models/commit_status_spec.rb | 1 + spec/models/integrations/every_integration_spec.rb | 36 ++++++++++++++++++++ 21 files changed, 200 insertions(+), 34 deletions(-) create mode 100644 spec/models/integrations/every_integration_spec.rb diff --git a/app/controllers/projects/application_controller.rb b/app/controllers/projects/application_controller.rb index 62233c8c3c9..028b7af02c9 100644 --- a/app/controllers/projects/application_controller.rb +++ b/app/controllers/projects/application_controller.rb @@ -32,6 +32,21 @@ class Projects::ApplicationController < ApplicationController ->(project) { !project.pending_delete? } end + def authorize_read_build_trace! + return if can?(current_user, :read_build_trace, build) + + if build.debug_mode? + access_denied!( + _('You must have developer or higher permissions in the associated project to view job logs when debug trace ' \ + "is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline " \ + 'configuration or CI/CD settings. If you need to view this job log, a project maintainer must add you to ' \ + 'the project with developer permissions or higher.') + ) + else + access_denied!(_('The current user is not authorized to access the job log.')) + end + end + def build_canonical_path(project) params[:namespace_id] = project.namespace.to_param params[:project_id] = project.to_param diff --git a/app/controllers/projects/artifacts_controller.rb b/app/controllers/projects/artifacts_controller.rb index feed94708f6..997d321ac24 100644 --- a/app/controllers/projects/artifacts_controller.rb +++ b/app/controllers/projects/artifacts_controller.rb @@ -9,6 +9,7 @@ class Projects::ArtifactsController < Projects::ApplicationController layout 'project' before_action :authorize_read_build! + before_action :authorize_read_build_trace!, only: [:download] before_action :authorize_update_build!, only: [:keep] before_action :authorize_destroy_artifacts!, only: [:destroy] before_action :extract_ref_name_and_path @@ -164,4 +165,10 @@ class Projects::ArtifactsController < Projects::ApplicationController render_404 unless @entry.exists? end + + def authorize_read_build_trace! + return unless params[:file_type] == 'trace' + + super + end end diff --git a/app/controllers/projects/jobs_controller.rb b/app/controllers/projects/jobs_controller.rb index 4189419c3ba..0f6cf97d69d 100644 --- a/app/controllers/projects/jobs_controller.rb +++ b/app/controllers/projects/jobs_controller.rb @@ -177,17 +177,7 @@ class Projects::JobsController < Projects::ApplicationController private - def authorize_read_build_trace! - return if can?(current_user, :read_build_trace, @build) - - msg = _( - "You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. " \ - "If you need to view this job log, a project maintainer must add you to the project with developer permissions or higher." - ) - return access_denied!(msg) if @build.debug_mode? - - access_denied!(_('The current user is not authorized to access the job log.')) - end + attr_reader :build def authorize_update_build! return access_denied! unless can?(current_user, :update_build, @build) diff --git a/app/models/commit_status.rb b/app/models/commit_status.rb index 08fed353755..ac9d8c39bd2 100644 --- a/app/models/commit_status.rb +++ b/app/models/commit_status.rb @@ -229,7 +229,13 @@ class CommitStatus < Ci::ApplicationRecord end def group_name - name.to_s.sub(%r{([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+)))+\s*\z}, '').strip + # [\b\s:] -> whitespace or column + # (\[.*\])|(\d+[\s:\/\\]+\d+) -> variables/matrix or parallel-jobs numbers + # {1,3} -> number of times that matches the variables/matrix or parallel-jobs numbers + # we limit this to 3 because of possible abuse + regex = %r{([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+))){1,3}\s*\z} + + name.to_s.sub(regex, '').strip end def failed_but_allowed? diff --git a/app/models/integrations/asana.rb b/app/models/integrations/asana.rb index e7634f51ec4..d25bf8b1b1e 100644 --- a/app/models/integrations/asana.rb +++ b/app/models/integrations/asana.rb @@ -27,10 +27,12 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'api_key', title: 'API key', help: s_('AsanaService|User Personal Access Token. User must have access to the task. All comments are attributed to this user.'), + non_empty_password_title: s_('ProjectService|Enter new API key'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current API key.'), # Example Personal Access Token from Asana docs placeholder: '0/68a9e79b868c6789e79a124c30b0', required: true diff --git a/app/models/integrations/assembla.rb b/app/models/integrations/assembla.rb index 6a36045330a..ccd24c1fb2c 100644 --- a/app/models/integrations/assembla.rb +++ b/app/models/integrations/assembla.rb @@ -19,8 +19,19 @@ module Integrations def fields [ - { type: 'text', name: 'token', placeholder: '', required: true }, - { type: 'text', name: 'subdomain', placeholder: '' } + { + type: 'password', + name: 'token', + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: '', + required: true + }, + { + type: 'text', + name: 'subdomain', + placeholder: '' + } ] end diff --git a/app/models/integrations/bamboo.rb b/app/models/integrations/bamboo.rb index c614a9415ab..b384a94d713 100644 --- a/app/models/integrations/bamboo.rb +++ b/app/models/integrations/bamboo.rb @@ -54,10 +54,12 @@ module Integrations required: true }, { - type: 'text', + type: 'password', name: 'build_key', - placeholder: s_('KEY'), help: s_('BambooService|Bamboo build plan key.'), + non_empty_password_title: s_('BambooService|Enter new build key'), + non_empty_password_help: s_('BambooService|Leave blank to use your current build key.'), + placeholder: s_('KEY'), required: true }, { diff --git a/app/models/integrations/base_slash_commands.rb b/app/models/integrations/base_slash_commands.rb index 1d271e75a91..a0ac5474893 100644 --- a/app/models/integrations/base_slash_commands.rb +++ b/app/models/integrations/base_slash_commands.rb @@ -26,7 +26,13 @@ module Integrations def fields [ - { type: 'text', name: 'token', placeholder: 'XXxxXXxxXXxxXXxxXXxxXXxx' } + { + type: 'password', + name: 'token', + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: 'XXxxXXxxXXxxXXxxXXxxXXxx' + } ] end diff --git a/app/models/integrations/buildkite.rb b/app/models/integrations/buildkite.rb index b816f90ef52..3b802271a36 100644 --- a/app/models/integrations/buildkite.rb +++ b/app/models/integrations/buildkite.rb @@ -76,10 +76,12 @@ module Integrations def fields [ - { type: 'text', + { type: 'password', name: 'token', title: _('Token'), help: s_('ProjectService|The token you get after you create a Buildkite pipeline with a GitLab repository.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), required: true }, { type: 'text', diff --git a/app/models/integrations/campfire.rb b/app/models/integrations/campfire.rb index 81e6c2411b8..7889cd8f9a9 100644 --- a/app/models/integrations/campfire.rb +++ b/app/models/integrations/campfire.rb @@ -25,11 +25,13 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'token', title: _('Campfire token'), - placeholder: '', help: s_('CampfireService|API authentication token from Campfire.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: '', required: true }, { diff --git a/app/models/integrations/drone_ci.rb b/app/models/integrations/drone_ci.rb index 3c18e5d8732..73f78bec381 100644 --- a/app/models/integrations/drone_ci.rb +++ b/app/models/integrations/drone_ci.rb @@ -96,8 +96,21 @@ module Integrations def fields [ - { type: 'text', name: 'token', help: s_('ProjectService|Token for the Drone project.'), required: true }, - { type: 'text', name: 'drone_url', title: s_('ProjectService|Drone server URL'), placeholder: 'http://drone.example.com', required: true } + { + type: 'password', + name: 'token', + help: s_('ProjectService|Token for the Drone project.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + required: true + }, + { + type: 'text', + name: 'drone_url', + title: s_('ProjectService|Drone server URL'), + placeholder: 'http://drone.example.com', + required: true + } ] end diff --git a/app/models/integrations/flowdock.rb b/app/models/integrations/flowdock.rb index 476cdc35585..703d8013bab 100644 --- a/app/models/integrations/flowdock.rb +++ b/app/models/integrations/flowdock.rb @@ -24,7 +24,15 @@ module Integrations def fields [ - { type: 'text', name: 'token', placeholder: s_('FlowdockService|1b609b52537...'), required: true, help: 'Enter your Flowdock token.' } + { + type: 'password', + name: 'token', + help: s_('FlowdockService|Enter your Flowdock token.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), + placeholder: '1b609b52537...', + required: true + } ] end diff --git a/app/models/integrations/harbor.rb b/app/models/integrations/harbor.rb index 4c76e418886..6b561575f30 100644 --- a/app/models/integrations/harbor.rb +++ b/app/models/integrations/harbor.rb @@ -64,11 +64,12 @@ module Integrations required: true }, { - type: 'text', + type: 'password', name: 'password', title: s_('HarborIntegration|Harbor password'), - non_empty_password_title: s_('HarborIntegration|Enter Harbor password'), - non_empty_password_help: s_('HarborIntegration|Password for your Harbor username.'), + help: s_('HarborIntegration|Password for your Harbor username.'), + non_empty_password_title: s_('HarborIntegration|Enter new Harbor password'), + non_empty_password_help: s_('HarborIntegration|Leave blank to use your current password.'), required: true } ] diff --git a/app/models/integrations/packagist.rb b/app/models/integrations/packagist.rb index f616bc5faf2..738319ce835 100644 --- a/app/models/integrations/packagist.rb +++ b/app/models/integrations/packagist.rb @@ -36,10 +36,12 @@ module Integrations required: true }, { - type: 'text', + type: 'password', name: 'token', title: _('Token'), help: s_('Enter your Packagist token.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), placeholder: '', required: true }, diff --git a/app/models/integrations/pivotaltracker.rb b/app/models/integrations/pivotaltracker.rb index 5b9ac023b7e..931ccf46655 100644 --- a/app/models/integrations/pivotaltracker.rb +++ b/app/models/integrations/pivotaltracker.rb @@ -27,9 +27,11 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'token', help: s_('PivotalTrackerService|Pivotal Tracker API token. User must have access to the story. All comments are attributed to this user.'), + non_empty_password_title: s_('ProjectService|Enter new token'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current token.'), required: true }, { diff --git a/app/models/integrations/pushover.rb b/app/models/integrations/pushover.rb index db39a4c68bd..7fd5efa8765 100644 --- a/app/models/integrations/pushover.rb +++ b/app/models/integrations/pushover.rb @@ -22,18 +22,22 @@ module Integrations def fields [ { - type: 'text', + type: 'password', name: 'api_key', title: _('API key'), help: s_('PushoverService|Enter your application key.'), + non_empty_password_title: s_('ProjectService|Enter new API key'), + non_empty_password_help: s_('ProjectService|Leave blank to use your current API key.'), placeholder: '', required: true }, { - type: 'text', + type: 'password', name: 'user_key', title: _('User key'), help: s_('PushoverService|Enter your user key.'), + non_empty_password_title: s_('PushoverService|Enter new user key'), + non_empty_password_help: s_('PushoverService|Leave blank to use your current user key.'), placeholder: '', required: true }, diff --git a/doc/ci/jobs/index.md b/doc/ci/jobs/index.md index b8129e1cf18..e589fd8b045 100644 --- a/doc/ci/jobs/index.md +++ b/doc/ci/jobs/index.md @@ -167,7 +167,7 @@ The jobs are ordered by comparing the numbers from left to right. You usually want the first number to be the index and the second number to be the total. [This regular expression](https://gitlab.com/gitlab-org/gitlab/-/blob/2f3dc314f42dbd79813e6251792853bc231e69dd/app/models/commit_status.rb#L99) -evaluates the job names: `([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+)))+\s*\z`. +evaluates the job names: `([\b\s:]+((\[.*\])|(\d+[\s:\/\\]+\d+))){1,3}\s*\z`. One or more `: [...]`, `X Y`, `X/Y`, or `X\Y` sequences are removed from the **end** of job names only. Matching substrings found at the beginning or in the middle of job names are not removed. diff --git a/locale/gitlab.pot b/locale/gitlab.pot index b7c3378db63..fa3357c723f 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -5614,6 +5614,12 @@ msgstr "" msgid "BambooService|Bamboo service root URL." msgstr "" +msgid "BambooService|Enter new build key" +msgstr "" + +msgid "BambooService|Leave blank to use your current build key." +msgstr "" + msgid "BambooService|Run CI/CD pipelines with Atlassian Bamboo." msgstr "" @@ -16101,7 +16107,7 @@ msgstr "" msgid "FloC|Federated Learning of Cohorts" msgstr "" -msgid "FlowdockService|1b609b52537..." +msgid "FlowdockService|Enter your Flowdock token." msgstr "" msgid "FlowdockService|Send event notifications from GitLab to Flowdock flows." @@ -18357,7 +18363,7 @@ msgstr "" msgid "HarborIntegration|Base URL of the Harbor instance." msgstr "" -msgid "HarborIntegration|Enter Harbor password" +msgid "HarborIntegration|Enter new Harbor password" msgstr "" msgid "HarborIntegration|Harbor URL" @@ -18372,6 +18378,9 @@ msgstr "" msgid "HarborIntegration|Harbor username" msgstr "" +msgid "HarborIntegration|Leave blank to use your current password." +msgstr "" + msgid "HarborIntegration|Password for your Harbor username." msgstr "" @@ -29398,6 +29407,9 @@ msgstr "" msgid "ProjectService|Leave blank to use your current API key" msgstr "" +msgid "ProjectService|Leave blank to use your current API key." +msgstr "" + msgid "ProjectService|Leave blank to use your current password" msgstr "" @@ -30751,6 +30763,9 @@ msgstr "" msgid "PushoverService|%{user_name} pushed new branch \"%{ref}\"." msgstr "" +msgid "PushoverService|Enter new user key" +msgstr "" + msgid "PushoverService|Enter your application key." msgstr "" @@ -30766,6 +30781,9 @@ msgstr "" msgid "PushoverService|Leave blank for all active devices." msgstr "" +msgid "PushoverService|Leave blank to use your current user key." +msgstr "" + msgid "PushoverService|Low priority" msgstr "" diff --git a/spec/controllers/projects/artifacts_controller_spec.rb b/spec/controllers/projects/artifacts_controller_spec.rb index d51880b282d..958fcd4360c 100644 --- a/spec/controllers/projects/artifacts_controller_spec.rb +++ b/spec/controllers/projects/artifacts_controller_spec.rb @@ -204,6 +204,44 @@ RSpec.describe Projects::ArtifactsController do end end end + + context 'when downloading a debug trace' do + let(:file_type) { 'trace' } + let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } + + before do + create(:ci_job_variable, key: 'CI_DEBUG_TRACE', value: 'true', job: job) + end + + context 'when the user does not have update_build permissions' do + let(:user) { create(:user) } + + before do + project.add_guest(user) + end + + render_views + + it 'denies the user access' do + download_artifact(file_type: file_type) + + expect(response).to have_gitlab_http_status(:forbidden) + expect(response.body).to include( + 'You must have developer or higher permissions in the associated project to view job logs when debug trace is enabled. ' \ + 'To disable debug trace, set the 'CI_DEBUG_TRACE' variable to 'false' in your pipeline configuration or CI/CD settings. ' \ + 'If you need to view this job log, a project maintainer must add you to the project with developer permissions or higher.' + ) + end + end + + context 'when the user has update_build permissions' do + it 'sends the trace' do + download_artifact(file_type: file_type) + + expect(response).to have_gitlab_http_status(:ok) + end + end + end end describe 'GET browse' do diff --git a/spec/models/commit_status_spec.rb b/spec/models/commit_status_spec.rb index 155e0fbb0e9..d158a99ef9f 100644 --- a/spec/models/commit_status_spec.rb +++ b/spec/models/commit_status_spec.rb @@ -618,6 +618,7 @@ RSpec.describe CommitStatus do 'rspec:windows 10000 20000' | 'rspec:windows' 'rspec:windows 0 : / 1' | 'rspec:windows' 'rspec:windows 0 : / 1 name' | 'rspec:windows 0 : / 1 name' + 'rspec [inception: [something, other thing], value]' | 'rspec' '0 1 name ruby' | '0 1 name ruby' '0 :/ 1 name ruby' | '0 :/ 1 name ruby' 'rspec: [aws]' | 'rspec' diff --git a/spec/models/integrations/every_integration_spec.rb b/spec/models/integrations/every_integration_spec.rb new file mode 100644 index 00000000000..33e89b3dabc --- /dev/null +++ b/spec/models/integrations/every_integration_spec.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe 'Every integration' do + all_integration_names = Integration.available_integration_names + + all_integration_names.each do |integration_name| + describe integration_name do + let(:integration_class) { Integration.integration_name_to_model(integration_name) } + let(:integration) { integration_class.new } + + context 'secret fields', :aggregate_failures do + it "uses type: 'password' for all secret fields" do + integration.fields.each do |field| + next unless Integrations::Field::SECRET_NAME.match?(field[:name]) + + expect(field[:type]).to eq('password'), + "Field '#{field[:name]}' should use type 'password'" + end + end + + it 'defines non-empty titles and help texts for all secret fields' do + integration.fields.each do |field| + next unless field[:type] == 'password' + + expect(field[:non_empty_password_title]).to be_present, + "Field '#{field[:name]}' should define :non_empty_password_title" + expect(field[:non_empty_password_help]).to be_present, + "Field '#{field[:name]}' should define :non_empty_password_help" + end + end + end + end + end +end -- cgit v1.2.3