summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2022-04-29 08:18:14 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2022-04-29 08:18:14 +0000
commitdeb2f3a60831afda2ad7ec144eb58aaf269abe58 (patch)
tree66c001da2aeba9b3e0204af1407c91994057f403
parent88da5554d9626377fe7868e956a47a0498e04eb5 (diff)
downloadgitlab-ce-deb2f3a60831afda2ad7ec144eb58aaf269abe58.tar.gz
Add latest changes from gitlab-org/security/gitlab@14-10-stable-ee
-rw-r--r--app/controllers/projects/application_controller.rb15
-rw-r--r--app/controllers/projects/artifacts_controller.rb7
-rw-r--r--app/controllers/projects/jobs_controller.rb12
-rw-r--r--app/models/commit_status.rb8
-rw-r--r--app/models/integrations/asana.rb4
-rw-r--r--app/models/integrations/assembla.rb15
-rw-r--r--app/models/integrations/bamboo.rb6
-rw-r--r--app/models/integrations/base_slash_commands.rb8
-rw-r--r--app/models/integrations/buildkite.rb4
-rw-r--r--app/models/integrations/campfire.rb6
-rw-r--r--app/models/integrations/drone_ci.rb17
-rw-r--r--app/models/integrations/flowdock.rb10
-rw-r--r--app/models/integrations/harbor.rb7
-rw-r--r--app/models/integrations/packagist.rb4
-rw-r--r--app/models/integrations/pivotaltracker.rb4
-rw-r--r--app/models/integrations/pushover.rb8
-rw-r--r--doc/ci/jobs/index.md2
-rw-r--r--locale/gitlab.pot22
-rw-r--r--spec/controllers/projects/artifacts_controller_spec.rb38
-rw-r--r--spec/models/commit_status_spec.rb1
-rw-r--r--spec/models/integrations/every_integration_spec.rb36
21 files changed, 200 insertions, 34 deletions
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 &#39;CI_DEBUG_TRACE&#39; variable to &#39;false&#39; 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