From 90f15e050d48dfada581e7c24ff99b2b2b4bc806 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Fri, 29 Apr 2022 08:23:02 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@14-9-stable-ee --- app/models/packages/package_file.rb | 1 + lib/api/pypi_packages.rb | 2 +- lib/gitlab/regex.rb | 4 +++ spec/lib/gitlab/regex_spec.rb | 15 ++++++++++ spec/models/packages/package_file_spec.rb | 35 ++++++++++++++++++++++ spec/requests/api/pypi_packages_spec.rb | 15 +++++++++- .../packages/pypi/create_package_service_spec.rb | 19 +++++++----- 7 files changed, 81 insertions(+), 10 deletions(-) diff --git a/app/models/packages/package_file.rb b/app/models/packages/package_file.rb index ad8140ac684..7be18c0e55d 100644 --- a/app/models/packages/package_file.rb +++ b/app/models/packages/package_file.rb @@ -35,6 +35,7 @@ class Packages::PackageFile < ApplicationRecord validates :file_name, presence: true validates :file_name, uniqueness: { scope: :package }, if: -> { package&.pypi? } + validates :file_sha256, format: { with: Gitlab::Regex.sha256_regex }, if: -> { package&.pypi? }, allow_nil: true scope :recent, -> { order(id: :desc) } scope :limit_recent, ->(limit) { recent.limit(limit) } diff --git a/lib/api/pypi_packages.rb b/lib/api/pypi_packages.rb index 86f36d4fc00..d4f51beb2e5 100644 --- a/lib/api/pypi_packages.rb +++ b/lib/api/pypi_packages.rb @@ -174,7 +174,7 @@ module API requires :version, type: String optional :requires_python, type: String optional :md5_digest, type: String - optional :sha256_digest, type: String + optional :sha256_digest, type: String, regexp: Gitlab::Regex.sha256_regex end route_setting :authentication, deploy_token_allowed: true, basic_auth_personal_access_token: true, job_token_allowed: :basic_auth diff --git a/lib/gitlab/regex.rb b/lib/gitlab/regex.rb index c9202c6c54c..205106afddb 100644 --- a/lib/gitlab/regex.rb +++ b/lib/gitlab/regex.rb @@ -237,6 +237,10 @@ module Gitlab generic_package_name_regex end + def sha256_regex + @sha256_regex ||= /\A[0-9a-f]{64}\z/i.freeze + end + private def conan_name_regex diff --git a/spec/lib/gitlab/regex_spec.rb b/spec/lib/gitlab/regex_spec.rb index f3e8c440fba..b4c1f3b689b 100644 --- a/spec/lib/gitlab/regex_spec.rb +++ b/spec/lib/gitlab/regex_spec.rb @@ -1005,4 +1005,19 @@ RSpec.describe Gitlab::Regex do it { is_expected.not_to match('.xt.est_') } it { is_expected.not_to match('0test1') } end + + describe '.sha256_regex' do + subject { described_class.sha256_regex } + + it { is_expected.to match('a' * 64) } + it { is_expected.to match('abcdefABCDEF1234567890abcdefABCDEF1234567890abcdefABCDEF12345678') } + it { is_expected.not_to match('a' * 63) } + it { is_expected.not_to match('a' * 65) } + it { is_expected.not_to match('a' * 63 + 'g') } + it { is_expected.not_to match('a' * 63 + '{') } + it { is_expected.not_to match('a' * 63 + '%') } + it { is_expected.not_to match('a' * 63 + '*') } + it { is_expected.not_to match('a' * 63 + '#') } + it { is_expected.not_to match('') } + end end diff --git a/spec/models/packages/package_file_spec.rb b/spec/models/packages/package_file_spec.rb index fd453d8e5a9..0fe5c772261 100644 --- a/spec/models/packages/package_file_spec.rb +++ b/spec/models/packages/package_file_spec.rb @@ -23,6 +23,41 @@ RSpec.describe Packages::PackageFile, type: :model do describe 'validations' do it { is_expected.to validate_presence_of(:package) } + + context 'with pypi package' do + let_it_be(:package) { create(:pypi_package) } + + let(:package_file) { package.package_files.first } + let(:status) { :default } + let(:file_name) { 'foo' } + let(:file) { fixture_file_upload('spec/fixtures/dk.png') } + let(:params) { { file: file, file_name: file_name, status: status } } + + subject { package.package_files.create!(params) } + + context 'file_sha256' do + where(:sha256_value, :expected_success) do + 'a' * 64 | true + nil | true + 'a' * 63 | false + 'a' * 65 | false + 'a' * 63 + '%' | false + '' | false + end + + with_them do + let(:params) { super().merge({ file_sha256: sha256_value }) } + + it 'does not allow invalid sha256 characters' do + if expected_success + expect { subject }.not_to raise_error + else + expect { subject }.to raise_error(ActiveRecord::RecordInvalid, "Validation failed: File sha256 is invalid") + end + end + end + end + end end context 'with package filenames' do diff --git a/spec/requests/api/pypi_packages_spec.rb b/spec/requests/api/pypi_packages_spec.rb index 078db4f1509..8fa5f409298 100644 --- a/spec/requests/api/pypi_packages_spec.rb +++ b/spec/requests/api/pypi_packages_spec.rb @@ -136,7 +136,7 @@ RSpec.describe API::PypiPackages do let(:url) { "/projects/#{project.id}/packages/pypi" } let(:headers) { {} } let(:requires_python) { '>=3.7' } - let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '123' } } + let(:base_params) { { requires_python: requires_python, version: '1.0.0', name: 'sample-project', sha256_digest: '1' * 64 } } let(:params) { base_params.merge(content: temp_file(file_name)) } let(:send_rewritten_field) { true } let(:snowplow_gitlab_standard_context) { { project: project, namespace: project.namespace, user: user } } @@ -221,6 +221,19 @@ RSpec.describe API::PypiPackages do it_behaves_like 'returning response status', :bad_request end + context 'with an invalid sha256' do + let(:token) { personal_access_token.token } + let(:user_headers) { basic_auth_header(user.username, token) } + let(:headers) { user_headers.merge(workhorse_headers) } + + before do + params[:sha256_digest] = 'a' * 63 + '%' + project.add_developer(user) + end + + it_behaves_like 'returning response status', :bad_request + end + it_behaves_like 'deploy token for package uploads' it_behaves_like 'job token for package uploads' diff --git a/spec/services/packages/pypi/create_package_service_spec.rb b/spec/services/packages/pypi/create_package_service_spec.rb index f84a77f80f7..354ac92b99a 100644 --- a/spec/services/packages/pypi/create_package_service_spec.rb +++ b/spec/services/packages/pypi/create_package_service_spec.rb @@ -7,6 +7,9 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do let_it_be(:project) { create(:project) } let_it_be(:user) { create(:user) } + let(:sha256) { '1' * 64 } + let(:md5) { '567' } + let(:requires_python) { '>=2.7' } let(:params) do { @@ -14,8 +17,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do version: '1.0', content: temp_file('foo.tgz'), requires_python: requires_python, - sha256_digest: '123', - md5_digest: '567' + sha256_digest: sha256, + md5_digest: md5 } end @@ -34,8 +37,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' expect(created_package.package_files.size).to eq 1 expect(created_package.package_files.first.file_name).to eq 'foo.tgz' - expect(created_package.package_files.first.file_sha256).to eq '123' - expect(created_package.package_files.first.file_md5).to eq '567' + expect(created_package.package_files.first.file_sha256).to eq sha256 + expect(created_package.package_files.first.file_md5).to eq md5 end end @@ -74,8 +77,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do context 'with an existing file' do before do params[:content] = temp_file('foo.tgz') - params[:sha256_digest] = 'abc' - params[:md5_digest] = 'def' + params[:sha256_digest] = sha256 + params[:md5_digest] = md5 end it 'throws an error' do @@ -101,8 +104,8 @@ RSpec.describe Packages::Pypi::CreatePackageService, :aggregate_failures do expect(created_package.pypi_metadatum.required_python).to eq '>=2.7' expect(created_package.package_files.size).to eq 1 expect(created_package.package_files.first.file_name).to eq 'foo.tgz' - expect(created_package.package_files.first.file_sha256).to eq 'abc' - expect(created_package.package_files.first.file_md5).to eq 'def' + expect(created_package.package_files.first.file_sha256).to eq sha256 + expect(created_package.package_files.first.file_md5).to eq md5 end end end -- cgit v1.2.1