From 4d4de1e7774dbe1b722ee5d256f38b503125ec05 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Wed, 15 Oct 2025 19:43:40 +0800 Subject: [PATCH 1/2] Upgrade Rack to 2.2.20 Upgrade the Rack gem to the latest patch release --- Gemfile.checksum | 2 +- Gemfile.lock | 2 +- Gemfile.next.checksum | 2 +- Gemfile.next.lock | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.checksum b/Gemfile.checksum index 7c2db7f440ae0e..f7f4c136575b65 100644 --- a/Gemfile.checksum +++ b/Gemfile.checksum @@ -564,7 +564,7 @@ {"name":"raabro","version":"1.4.0","platform":"ruby","checksum":"d4fa9ff5172391edb92b242eed8be802d1934b1464061ae5e70d80962c5da882"}, {"name":"racc","version":"1.8.1","platform":"java","checksum":"54f2e6d1e1b91c154013277d986f52a90e5ececbe91465d29172e49342732b98"}, {"name":"racc","version":"1.8.1","platform":"ruby","checksum":"4a7f6929691dbec8b5209a0b373bc2614882b55fc5d2e447a21aaa691303d62f"}, -{"name":"rack","version":"2.2.18","platform":"ruby","checksum":"ca4feea715d3576a7d613412ce409d04ceac1133ab109fcd490ec1d835107f4f"}, +{"name":"rack","version":"2.2.20","platform":"ruby","checksum":"85da3447a4845230a00803d47413ed4d1dd9823f948a8d93c0b2d1e149c07170"}, {"name":"rack-accept","version":"0.4.5","platform":"ruby","checksum":"66247b5449db64ebb93ae2ec4af4764b87d1ae8a7463c7c68893ac13fa8d4da2"}, {"name":"rack-attack","version":"6.7.0","platform":"ruby","checksum":"3ca47e8f66cd33b2c96af53ea4754525cd928ed3fa8da10ee6dad0277791d77c"}, {"name":"rack-cors","version":"2.0.2","platform":"ruby","checksum":"415d4e1599891760c5dc9ef0349c7fecdf94f7c6a03e75b2e7c2b54b82adda1b"}, diff --git a/Gemfile.lock b/Gemfile.lock index c5a8006895f9e2..2f58a5961bddab 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1549,7 +1549,7 @@ GEM pyu-ruby-sasl (0.0.3.3) raabro (1.4.0) racc (1.8.1) - rack (2.2.18) + rack (2.2.20) rack-accept (0.4.5) rack (>= 0.4) rack-attack (6.7.0) diff --git a/Gemfile.next.checksum b/Gemfile.next.checksum index 5ba54d624ca1a3..63e0816e3b22e0 100644 --- a/Gemfile.next.checksum +++ b/Gemfile.next.checksum @@ -564,7 +564,7 @@ {"name":"raabro","version":"1.4.0","platform":"ruby","checksum":"d4fa9ff5172391edb92b242eed8be802d1934b1464061ae5e70d80962c5da882"}, {"name":"racc","version":"1.8.1","platform":"java","checksum":"54f2e6d1e1b91c154013277d986f52a90e5ececbe91465d29172e49342732b98"}, {"name":"racc","version":"1.8.1","platform":"ruby","checksum":"4a7f6929691dbec8b5209a0b373bc2614882b55fc5d2e447a21aaa691303d62f"}, -{"name":"rack","version":"2.2.18","platform":"ruby","checksum":"ca4feea715d3576a7d613412ce409d04ceac1133ab109fcd490ec1d835107f4f"}, +{"name":"rack","version":"2.2.20","platform":"ruby","checksum":"85da3447a4845230a00803d47413ed4d1dd9823f948a8d93c0b2d1e149c07170"}, {"name":"rack-accept","version":"0.4.5","platform":"ruby","checksum":"66247b5449db64ebb93ae2ec4af4764b87d1ae8a7463c7c68893ac13fa8d4da2"}, {"name":"rack-attack","version":"6.7.0","platform":"ruby","checksum":"3ca47e8f66cd33b2c96af53ea4754525cd928ed3fa8da10ee6dad0277791d77c"}, {"name":"rack-cors","version":"2.0.2","platform":"ruby","checksum":"415d4e1599891760c5dc9ef0349c7fecdf94f7c6a03e75b2e7c2b54b82adda1b"}, diff --git a/Gemfile.next.lock b/Gemfile.next.lock index 9b64c52fae8ee9..60fec70d80fc24 100644 --- a/Gemfile.next.lock +++ b/Gemfile.next.lock @@ -1543,7 +1543,7 @@ GEM pyu-ruby-sasl (0.0.3.3) raabro (1.4.0) racc (1.8.1) - rack (2.2.18) + rack (2.2.20) rack-accept (0.4.5) rack (>= 0.4) rack-attack (6.7.0) -- GitLab From 99ae69224bd9ed5ff7f36c186cbf09581a405c67 Mon Sep 17 00:00:00 2001 From: Heinrich Lee Yu Date: Thu, 16 Oct 2025 19:09:37 +0800 Subject: [PATCH 2/2] Enable sendfile in Rails Previously, this was enabled when Workhorse sends the X-Sendfile-Type header. But with the Rack upgrade, the header is no longer parsed for security reasons. See https://github.com/rack/rack/commit/fba2c8bc63eb787ff4b19bc612d315fda6126d85 --- config/application.rb | 3 +++ ee/spec/requests/api/ci/jobs_spec.rb | 6 ++---- ee/spec/requests/api/dependency_list_exports_spec.rb | 4 +++- .../vulnerability_scanning/sbom_scans_spec.rb | 2 +- .../api/vulnerability_archive_exports_spec.rb | 2 +- ee/spec/requests/api/vulnerability_exports_spec.rb | 2 +- lib/api/internal/lfs.rb | 2 +- .../artifacts/user_downloads_artifacts_spec.rb | 2 +- spec/features/projects/jobs/permissions_spec.rb | 2 +- spec/features/projects/jobs_spec.rb | 6 +++--- spec/requests/api/ci/job_artifacts_spec.rb | 4 ++-- spec/requests/api/helm_packages_spec.rb | 2 +- spec/requests/api/internal/lfs_spec.rb | 11 ----------- .../api/ml/mlflow_artifacts/artifacts_spec.rb | 7 ++----- spec/requests/api/npm_project_packages_spec.rb | 3 +-- .../terraform/modules/v1/namespace_packages_spec.rb | 3 +-- .../requests/projects/attestations_controller_spec.rb | 2 +- .../requests/api/debian_packages_shared_examples.rb | 6 +++++- .../requests/api/nuget_packages_shared_examples.rb | 2 +- .../requests/api/pypi_packages_shared_examples.rb | 2 +- .../terraform/modules/v1/packages_shared_examples.rb | 2 +- .../requests/lfs_http_shared_examples.rb | 3 +-- 22 files changed, 34 insertions(+), 44 deletions(-) diff --git a/config/application.rb b/config/application.rb index cc962e72650d5e..1e839952e1dbec 100644 --- a/config/application.rb +++ b/config/application.rb @@ -266,6 +266,9 @@ class Application < Rails::Application # Disable adding field_with_errors wrapper to form elements config.action_view.field_error_proc = proc { |html_tag, instance| html_tag } + # Enable Sendfile middleware to offload serving of files to Workhorse + config.action_dispatch.x_sendfile_header = 'X-Sendfile' + # Support legacy unicode file named img emojis, `1F939.png` config.assets.paths << TanukiEmoji.images_path config.assets.paths << "#{config.root}/vendor/assets/fonts" diff --git a/ee/spec/requests/api/ci/jobs_spec.rb b/ee/spec/requests/api/ci/jobs_spec.rb index ce8745875aea76..7f605745e9605c 100644 --- a/ee/spec/requests/api/ci/jobs_spec.rb +++ b/ee/spec/requests/api/ci/jobs_spec.rb @@ -15,8 +15,8 @@ let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => - %(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}) } + 'Content-Disposition' => %(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename}), + 'X-Sendfile' => job.artifacts_file.file.path } end before do @@ -40,7 +40,6 @@ expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h).to include(download_headers) - expect(response.body).to match_file(job.artifacts_file.file.file) end end @@ -121,7 +120,6 @@ expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h).to include(download_headers) - expect(response.body).to match_file(job.artifacts_file.file.file) end end diff --git a/ee/spec/requests/api/dependency_list_exports_spec.rb b/ee/spec/requests/api/dependency_list_exports_spec.rb index 9777240ab460aa..dd8e0641b3304c 100644 --- a/ee/spec/requests/api/dependency_list_exports_spec.rb +++ b/ee/spec/requests/api/dependency_list_exports_spec.rb @@ -466,7 +466,9 @@ download_dependency_list_export expect(response).to have_gitlab_http_status(:ok) - expect(json_response).to eq(Gitlab::Json.parse(dependency_list_export.file.read)) + + json = Gitlab::Json.parse(File.read(response.headers['X-Sendfile'])) + expect(json).to eq(Gitlab::Json.parse(dependency_list_export.file.read)) end context 'with dependency list export not finished' do diff --git a/ee/spec/requests/api/security/vulnerability_scanning/sbom_scans_spec.rb b/ee/spec/requests/api/security/vulnerability_scanning/sbom_scans_spec.rb index fe2c7e62da8399..7130fc9e737250 100644 --- a/ee/spec/requests/api/security/vulnerability_scanning/sbom_scans_spec.rb +++ b/ee/spec/requests/api/security/vulnerability_scanning/sbom_scans_spec.rb @@ -493,7 +493,7 @@ def request_with_second_scope download_results expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to eq(sbom_scan.result_file.read) + expect(response.headers['X-Sendfile']).to eq(sbom_scan.result_file.path) expect(response.content_type).to eq('application/json') end end diff --git a/ee/spec/requests/api/vulnerability_archive_exports_spec.rb b/ee/spec/requests/api/vulnerability_archive_exports_spec.rb index ba4bdf7c617c2d..d46944d0d0faa8 100644 --- a/ee/spec/requests/api/vulnerability_archive_exports_spec.rb +++ b/ee/spec/requests/api/vulnerability_archive_exports_spec.rb @@ -198,7 +198,7 @@ download_export expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to include 'Scanner Type,Scanner Name' + expect(File.read(response.headers['X-Sendfile'])).to include 'Scanner Type,Scanner Name' end end end diff --git a/ee/spec/requests/api/vulnerability_exports_spec.rb b/ee/spec/requests/api/vulnerability_exports_spec.rb index c5cac586d6468f..bd7084d49990cf 100644 --- a/ee/spec/requests/api/vulnerability_exports_spec.rb +++ b/ee/spec/requests/api/vulnerability_exports_spec.rb @@ -277,7 +277,7 @@ download_vulnerability_export expect(response).to have_gitlab_http_status(:ok) - expect(response.body).to include 'Scanner Type,Scanner Name,Status,Vulnerability,Details,Additional Info,Severity,CVE' + expect(File.read(response.headers['X-Sendfile'])).to include 'Scanner Type,Scanner Name,Status,Vulnerability,Details,Additional Info,Severity,CVE' expect(response.headers['Poll-Interval']).to be_blank end end diff --git a/lib/api/internal/lfs.rb b/lib/api/internal/lfs.rb index e94da8d34e0ab9..e68f715b9aea9b 100644 --- a/lib/api/internal/lfs.rb +++ b/lib/api/internal/lfs.rb @@ -3,7 +3,7 @@ module API module Internal class Lfs < ::API::Base - use Rack::Sendfile + use Rack::Sendfile, 'X-Sendfile' before { authenticate_by_gitlab_shell_token! } diff --git a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb index e7d5890424f00d..c0ebb78cf23765 100644 --- a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb +++ b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb @@ -12,7 +12,7 @@ expect(page.response_headers['Content-Disposition']).to eq(%(attachment; filename="#{job.artifacts_file.filename}"; filename*=UTF-8''#{job.artifacts_file.filename})) expect(page.response_headers['Content-Transfer-Encoding']).to eq("binary") expect(page.response_headers['Content-Type']).to eq("application/zip") - expect(page.source.b).to eq(job.artifacts_file.file.read.b) + expect(page.response_headers['X-Sendfile']).to eq(job.artifacts_file.file.path) end end diff --git a/spec/features/projects/jobs/permissions_spec.rb b/spec/features/projects/jobs/permissions_spec.rb index 873a0079508d9f..7c0cb93600b6f0 100644 --- a/spec/features/projects/jobs/permissions_spec.rb +++ b/spec/features/projects/jobs/permissions_spec.rb @@ -189,7 +189,7 @@ visit raw_project_job_path(project, job) expect(status_code).to eq(expected_status_code) - expect(page).to have_content(expected_msg) + expect(page).to have_content(expected_msg) if expected_status_code != 200 # rubocop:disable RSpec/AvoidConditionalStatements -- have_content does not work with empty non-HTML response body end end end diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb index c082fb06b3ef24..e962704c963df4 100644 --- a/spec/features/projects/jobs_spec.rb +++ b/spec/features/projects/jobs_spec.rb @@ -1074,7 +1074,7 @@ end it 'sends the right headers' do - requests = inspect_requests(inject_headers: { 'X-Sendfile-Type' => 'X-Sendfile' }) do + requests = inspect_requests do visit raw_project_job_path(project, job) end @@ -1088,7 +1088,7 @@ let(:job) { create(:ci_build, :success, :trace_artifact, pipeline: pipeline) } it 'sends the right headers' do - requests = inspect_requests(inject_headers: { 'X-Sendfile-Type' => 'X-Sendfile' }) do + requests = inspect_requests do visit raw_project_job_path(project, job) end @@ -1105,7 +1105,7 @@ end it 'sends the right headers' do - requests = inspect_requests(inject_headers: { 'X-Sendfile-Type' => 'X-Sendfile' }) do + requests = inspect_requests do visit raw_project_job_path(project, job2) end expect(requests.first.status_code).to eq(404) diff --git a/spec/requests/api/ci/job_artifacts_spec.rb b/spec/requests/api/ci/job_artifacts_spec.rb index 40553e16ad6107..cf82a054ba8402 100644 --- a/spec/requests/api/ci/job_artifacts_spec.rb +++ b/spec/requests/api/ci/job_artifacts_spec.rb @@ -289,7 +289,8 @@ def get_artifact_file(artifact_path) shared_examples 'downloads artifact' do let(:download_headers) do { 'Content-Transfer-Encoding' => 'binary', - 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip) } + 'Content-Disposition' => %q(attachment; filename="ci_build_artifacts.zip"; filename*=UTF-8''ci_build_artifacts.zip), + 'X-Sendfile' => job.artifacts_file.file.path } end let(:expected_params) { { artifact_size: job.artifacts_file.size } } @@ -300,7 +301,6 @@ def get_artifact_file(artifact_path) expect(response).to have_gitlab_http_status(:ok) expect(response.headers.to_h).to include(download_headers) - expect(response.body).to match_file(job.artifacts_file.file.file) end it_behaves_like 'storing arguments in the application context' diff --git a/spec/requests/api/helm_packages_spec.rb b/spec/requests/api/helm_packages_spec.rb index bc023a34185986..3a8a05125f7ace 100644 --- a/spec/requests/api/helm_packages_spec.rb +++ b/spec/requests/api/helm_packages_spec.rb @@ -104,7 +104,7 @@ def snowplow_context(user_role: :developer) api_request - expect(response.body).to eq(metadata_cache.file.read) + expect(response.headers['X-Sendfile']).to eq(metadata_cache.file.path) end it 'updates last_downloaded_at' do diff --git a/spec/requests/api/internal/lfs_spec.rb b/spec/requests/api/internal/lfs_spec.rb index 1021c03f736f67..617fade90e22af 100644 --- a/spec/requests/api/internal/lfs_spec.rb +++ b/spec/requests/api/internal/lfs_spec.rb @@ -27,20 +27,9 @@ context 'with valid auth' do context 'LFS in local storage' do - it 'sends the file' do - get api("/internal/lfs"), params: valid_params, headers: gitlab_shell_internal_api_request_header - - expect(response).to have_gitlab_http_status(:ok) - expect(response.headers['Content-Type']).to eq('application/octet-stream') - expect(response.headers['Content-Length'].to_i).to eq(File.stat(filename).size) - expect(response.body).to eq(File.open(filename, 'rb', &:read)) - end - - # https://www.rubydoc.info/github/rack/rack/master/Rack/Sendfile it 'delegates sending to Web server' do get api("/internal/lfs"), params: valid_params, - env: { 'HTTP_X_SENDFILE_TYPE' => 'X-Sendfile' }, headers: gitlab_shell_internal_api_request_header expect(response).to have_gitlab_http_status(:ok) diff --git a/spec/requests/api/ml/mlflow_artifacts/artifacts_spec.rb b/spec/requests/api/ml/mlflow_artifacts/artifacts_spec.rb index 8ac8a3414c510a..d220d4f7154b4e 100644 --- a/spec/requests/api/ml/mlflow_artifacts/artifacts_spec.rb +++ b/spec/requests/api/ml/mlflow_artifacts/artifacts_spec.rb @@ -118,9 +118,7 @@ it 'returns the artifact file', :aggregate_failures do is_expected.to have_gitlab_http_status(:ok) expect(response.headers['Content-Disposition']).to match("attachment; filename=\"#{package_file.file_name}\"") - expect(response.body) - .to eq(package_file.file.read) - expect(response.headers['Content-Length']).to eq(package_file.size.to_s) + expect(response.headers['X-Sendfile']).to eq(package_file.file.path) end end @@ -135,8 +133,7 @@ expect(response.headers['Content-Disposition']).to match( "attachment; filename=\"#{candidate_package_file.file_name}\"" ) - expect(response.body).to eq(candidate_package_file.file.read) - expect(response.headers['Content-Length']).to eq(candidate_package_file.size.to_s) + expect(response.headers['X-Sendfile']).to eq(candidate_package_file.file.path) end end diff --git a/spec/requests/api/npm_project_packages_spec.rb b/spec/requests/api/npm_project_packages_spec.rb index 81ed265675a049..eaa10db2f7b98f 100644 --- a/spec/requests/api/npm_project_packages_spec.rb +++ b/spec/requests/api/npm_project_packages_spec.rb @@ -29,7 +29,6 @@ context 'when metadata cache exists', :aggregate_failures do let!(:npm_metadata_cache) { create(:npm_metadata_cache, package_name: package.name, project_id: project.id) } - let(:metadata) { Gitlab::Json.parse(npm_metadata_cache.file.read.gsub('dist_tags', 'dist-tags')) } subject { get(url) } @@ -44,7 +43,7 @@ subject - expect(json_response).to eq(metadata) + expect(response.headers['X-Sendfile']).to eq(npm_metadata_cache.file.path) end it 'bumps last_downloaded_at of metadata cache' do diff --git a/spec/requests/api/terraform/modules/v1/namespace_packages_spec.rb b/spec/requests/api/terraform/modules/v1/namespace_packages_spec.rb index 3fc5c7dad60e66..1cc40fed0d8476 100644 --- a/spec/requests/api/terraform/modules/v1/namespace_packages_spec.rb +++ b/spec/requests/api/terraform/modules/v1/namespace_packages_spec.rb @@ -508,8 +508,7 @@ get_file expect(response).to have_gitlab_http_status(:ok) - expect(response.body).not_to eq(package_file_pending_destruction.file.file.read) - expect(response.body).to eq(package_file.file.file.read) + expect(File.open(response.headers['X-Sendfile'], 'rb').read).to eq(package_file.file.file.read) end end diff --git a/spec/requests/projects/attestations_controller_spec.rb b/spec/requests/projects/attestations_controller_spec.rb index 29393282bc6141..696ee38437422b 100644 --- a/spec/requests/projects/attestations_controller_spec.rb +++ b/spec/requests/projects/attestations_controller_spec.rb @@ -28,7 +28,7 @@ def download_attestation expect(response).to have_gitlab_http_status(:ok) expect(response.headers['Content-Disposition']) .to eq(%(attachment; filename="#{filename}"; filename*=UTF-8''#{filename})) - expect(response.body).to eq(fixture_file('supply_chain/attestation.json')) + expect(response.headers['X-Sendfile']).to eq(attestation.file.path) end end diff --git a/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb index 9950caca4107db..1142d6fdca5dc3 100644 --- a/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/debian_packages_shared_examples.rb @@ -8,7 +8,11 @@ expect(response).to have_gitlab_http_status(status) - unless body.nil? + next if body.nil? + + if response.headers['X-Sendfile'].present? + expect(File.open(response.headers['X-Sendfile'], 'rb').read).to match(body) + else expect(response.body).to match(body) end end diff --git a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb index f50f2616b59030..75c14849a1de92 100644 --- a/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/nuget_packages_shared_examples.rb @@ -1008,7 +1008,7 @@ expect(response).to have_gitlab_http_status(:ok) expect(response.media_type).to eq('application/octet-stream') - expect(response.body).to eq(symbol.file.read) + expect(response.headers['X-Sendfile']).to eq(symbol.file.path) end end diff --git a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb index 21a55a3ddae8f0..3e3a747b9be979 100644 --- a/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/pypi_packages_shared_examples.rb @@ -146,7 +146,7 @@ it 'returns the package listing' do subject - expect(response.body).to eq(File.open(package.package_files.first.file.path, "rb").read) + expect(response.headers['X-Sendfile']).to eq(package.package_files.first.file.path) end it_behaves_like 'returning response status', status diff --git a/spec/support/shared_examples/requests/api/terraform/modules/v1/packages_shared_examples.rb b/spec/support/shared_examples/requests/api/terraform/modules/v1/packages_shared_examples.rb index 19eb5592ec9769..5ffc17fd898db7 100644 --- a/spec/support/shared_examples/requests/api/terraform/modules/v1/packages_shared_examples.rb +++ b/spec/support/shared_examples/requests/api/terraform/modules/v1/packages_shared_examples.rb @@ -157,7 +157,7 @@ expect(response).to have_gitlab_http_status(status) expect(response.media_type).to eq('application/octet-stream') - expect(response.body).to eq(package.package_files.last.file.read) + expect(response.headers['X-Sendfile']).to eq(package.package_files.last.file.path) end end end diff --git a/spec/support/shared_examples/requests/lfs_http_shared_examples.rb b/spec/support/shared_examples/requests/lfs_http_shared_examples.rb index 640a367d768824..26bb6eec417ce9 100644 --- a/spec/support/shared_examples/requests/lfs_http_shared_examples.rb +++ b/spec/support/shared_examples/requests/lfs_http_shared_examples.rb @@ -82,8 +82,7 @@ let(:authorization) { authorize_user } let(:headers) do { - 'Authorization' => authorization, - 'X-Sendfile-Type' => 'X-Sendfile' + 'Authorization' => authorization } end -- GitLab