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:
- remove the backwards compatibility patch (introduced in https://gitlab.com/gitlab-org/security/gitlab/-/merge_requests/2912) in the upcoming release %16.0
- add a migration to delete not redacted
X-Gitlab-Token
logs - add a removal entry for 16.0 about deleting not redacted
X-Gitlab-Token
logs
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 🤖