[go: up one dir, main page]

Skip to content

Remove the backwards compatibility patch from web hook logs

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

This issue is a follow-up on #381895 (closed). Based on the discussions 1(https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2912#note_1185071546) and 2(#384379 (comment 1194352120)), we decided to:

Proposal

Remove the following code from:

  • app/models/hooks/web_hook_log.rb
def request_headers
  super unless web_hook.token?
  super if self[:request_headers]['X-Gitlab-Token'] == _('[REDACTED]')

  self[:request_headers].merge('X-Gitlab-Token' => _('[REDACTED]'))
end
  • spec/models/hooks/web_hook_log_spec.rb
describe '#request_headers' do
  let(:hook) { build(:project_hook, :token) }
  let(:web_hook_log) { build(:web_hook_log, request_headers: request_headers) }
  let(:expected_headers) { { 'X-Gitlab-Token' => _('[REDACTED]') } }

  context 'with redacted headers token' do
    let(:request_headers) { { 'X-Gitlab-Token' => _('[REDACTED]') } }

    it { expect(web_hook_log.request_headers).to eq(expected_headers) }
  end

  context 'with exposed headers token' do
    let(:request_headers) { { 'X-Gitlab-Token' => hook.token } }

    it { expect(web_hook_log.request_headers).to eq(expected_headers) }
  end
end

Create a post deployment migration:

WebHookLog.joins(:web_hook).where.not(web_hook: { encrypted_token: [nil, ''] }).delete_all
Edited by 🤖 GitLab Bot 🤖