summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
-rw-r--r--app/services/terraform/remote_state_handler.rb2
-rw-r--r--changelogs/unreleased/security-fix-terraform-state-exposed-object-storage-url.yml5
-rw-r--r--lib/api/terraform/state.rb9
-rw-r--r--spec/requests/api/terraform/state_spec.rb7
-rw-r--r--spec/services/terraform/remote_state_handler_spec.rb18
5 files changed, 29 insertions, 12 deletions
diff --git a/app/services/terraform/remote_state_handler.rb b/app/services/terraform/remote_state_handler.rb
index d2c44d4a265..7e79cb9e007 100644
--- a/app/services/terraform/remote_state_handler.rb
+++ b/app/services/terraform/remote_state_handler.rb
@@ -23,6 +23,8 @@ module Terraform
state.save! unless state.destroyed?
end
+
+ nil
end
def lock!
diff --git a/changelogs/unreleased/security-fix-terraform-state-exposed-object-storage-url.yml b/changelogs/unreleased/security-fix-terraform-state-exposed-object-storage-url.yml
new file mode 100644
index 00000000000..1e37aed6ca0
--- /dev/null
+++ b/changelogs/unreleased/security-fix-terraform-state-exposed-object-storage-url.yml
@@ -0,0 +1,5 @@
+---
+title: Do not expose Terraform state record in API
+merge_request:
+author:
+type: security
diff --git a/lib/api/terraform/state.rb b/lib/api/terraform/state.rb
index 4168cce21ef..3dbde4639ca 100644
--- a/lib/api/terraform/state.rb
+++ b/lib/api/terraform/state.rb
@@ -39,7 +39,6 @@ module API
env['api.format'] = :binary # this bypasses json serialization
body state.latest_file.read
- status :ok
end
end
@@ -53,8 +52,10 @@ module API
remote_state_handler.handle_with_lock do |state|
state.update_file!(CarrierWaveStringFile.new(data), version: params[:serial])
- status :ok
end
+
+ body false
+ status :ok
end
desc 'Delete a terraform state of a certain name'
@@ -64,8 +65,10 @@ module API
remote_state_handler.handle_with_lock do |state|
state.destroy!
- status :ok
end
+
+ body false
+ status :ok
end
desc 'Lock a terraform state of a certain name'
diff --git a/spec/requests/api/terraform/state_spec.rb b/spec/requests/api/terraform/state_spec.rb
index aff41ff5974..b91f6e1aa88 100644
--- a/spec/requests/api/terraform/state_spec.rb
+++ b/spec/requests/api/terraform/state_spec.rb
@@ -125,6 +125,7 @@ RSpec.describe API::Terraform::State do
expect { request }.to change { Terraform::State.count }.by(0)
expect(response).to have_gitlab_http_status(:ok)
+ expect(Gitlab::Json.parse(response.body)).to be_empty
end
context 'on Unicorn', :unicorn do
@@ -132,6 +133,7 @@ RSpec.describe API::Terraform::State do
expect { request }.to change { Terraform::State.count }.by(0)
expect(response).to have_gitlab_http_status(:ok)
+ expect(Gitlab::Json.parse(response.body)).to be_empty
end
end
end
@@ -167,6 +169,7 @@ RSpec.describe API::Terraform::State do
expect { request }.to change { Terraform::State.count }.by(1)
expect(response).to have_gitlab_http_status(:ok)
+ expect(Gitlab::Json.parse(response.body)).to be_empty
end
context 'on Unicorn', :unicorn do
@@ -174,6 +177,7 @@ RSpec.describe API::Terraform::State do
expect { request }.to change { Terraform::State.count }.by(1)
expect(response).to have_gitlab_http_status(:ok)
+ expect(Gitlab::Json.parse(response.body)).to be_empty
end
end
end
@@ -206,10 +210,11 @@ RSpec.describe API::Terraform::State do
context 'with maintainer permissions' do
let(:current_user) { maintainer }
- it 'deletes the state' do
+ it 'deletes the state and returns empty body' do
expect { request }.to change { Terraform::State.count }.by(-1)
expect(response).to have_gitlab_http_status(:ok)
+ expect(Gitlab::Json.parse(response.body)).to be_empty
end
end
diff --git a/spec/services/terraform/remote_state_handler_spec.rb b/spec/services/terraform/remote_state_handler_spec.rb
index c47367feb14..ca392849d49 100644
--- a/spec/services/terraform/remote_state_handler_spec.rb
+++ b/spec/services/terraform/remote_state_handler_spec.rb
@@ -42,17 +42,17 @@ RSpec.describe Terraform::RemoteStateHandler do
describe '#handle_with_lock' do
it 'allows to modify a state using database locking' do
- state = subject.handle_with_lock do |state|
+ record = nil
+ subject.handle_with_lock do |state|
+ record = state
state.name = 'updated-name'
end
- expect(state.name).to eq 'updated-name'
+ expect(record.reload.name).to eq 'updated-name'
end
- it 'returns the state object itself' do
- state = subject.handle_with_lock
-
- expect(state.name).to eq 'my-state'
+ it 'returns nil' do
+ expect(subject.handle_with_lock).to be_nil
end
end
@@ -70,11 +70,13 @@ RSpec.describe Terraform::RemoteStateHandler do
it 'handles a locked state using exclusive read lock' do
handler.lock!
- state = handler.handle_with_lock do |state|
+ record = nil
+ handler.handle_with_lock do |state|
+ record = state
state.name = 'new-name'
end
- expect(state.name).to eq 'new-name'
+ expect(record.reload.name).to eq 'new-name'
end
it 'raises exception if lock has not been acquired before' do