From 49cc838d0ab3f36dafa8a126f6463ec625af4746 Mon Sep 17 00:00:00 2001 From: Sigurd Spieckermann Date: Wed, 16 Jul 2025 20:23:06 +0200 Subject: [PATCH 1/2] Fix missing JSON serialization in custom webhook template field interpolation --- app/services/web_hook_service.rb | 6 ++- spec/services/web_hook_service_spec.rb | 72 ++++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 91babfdfebc605..6f97e36cf8bca9 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -311,7 +311,11 @@ def request_payload strong_memoize_attr :request_payload def render_custom_template(template, params) - template.gsub(CUSTOM_TEMPLATE_INTERPOLATION_REGEX) { params.dig(*Regexp.last_match(1).split('.')) } + template.gsub(CUSTOM_TEMPLATE_INTERPOLATION_REGEX) do + value = params.dig(*Regexp.last_match(1).split('.')) + value_json = value.to_json + value.is_a?(String) ? value_json[1..-2] : value_json + end end def raise_custom_webhook_template_error!(message) diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 2b561ba201895b..12ecdd44f7b4f1 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -425,6 +425,78 @@ stub_full_request(project_hook.url, method: :post) end + context 'when description contains a backslash' do + let(:data) do + { changes: { description: "\\This has a backslash" } } + end + + before do + project_hook.custom_webhook_template = '{"description":"{{changes.description}}"}' + end + + it 'escapes the backslash properly and delivers the webhook' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(body: '{"description":"\\\\This has a backslash"}') + .once + end + end + + context 'when description contains a double quote' do + let(:data) do + { changes: { description: '"This has a double quote' } } + end + + before do + project_hook.custom_webhook_template = '{"description":"{{changes.description}}"}' + end + + it 'escapes the double quote properly and delivers the webhook' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(body: '{"description":"\"This has a double quote"}') + .once + end + end + + context 'when template renders a non-primitive value (object)' do + let(:data) do + { complex: { string: 'value', boolean: true, number: 1, nil: nil, object: {}, array: [] } } + end + + before do + project_hook.custom_webhook_template = '{"complex":{{complex}}}' + end + + it 'serializes the object and delivers the webhook' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(body: '{"complex":{"string":"value","boolean":true,"number":1,"nil":null,"object":{},"array":[]}}') + .once + end + end + + context 'when template renders a non-primitive value (array)' do + let(:data) do + { complex: ['value', true, 1, nil, {}, []] } + end + + before do + project_hook.custom_webhook_template = '{"complex":{{complex}}}' + end + + it 'serializes the array and delivers the webhook' do + service_instance.execute + + expect(WebMock).to have_requested(:post, stubbed_hostname(project_hook.url)) + .with(body: '{"complex":["value",true,1,null,{},[]]}') + .once + end + end + context 'when template is valid' do before do project_hook.custom_webhook_template = '{"before":"{{before}}"}' -- GitLab From 54a091c6111bbced915bb8287caa6cb8c521e715 Mon Sep 17 00:00:00 2001 From: Sigurd Spieckermann Date: Tue, 12 Aug 2025 10:52:35 +0200 Subject: [PATCH 2/2] Add `custom_webhook_template_serialization` feature flag --- app/services/web_hook_service.rb | 12 ++++++--- .../custom_webhook_template_serialization.yml | 10 +++++++ doc/user/project/integrations/webhooks.md | 1 + spec/services/web_hook_service_spec.rb | 26 +++++++++++++++++++ 4 files changed, 45 insertions(+), 4 deletions(-) create mode 100644 config/feature_flags/beta/custom_webhook_template_serialization.yml diff --git a/app/services/web_hook_service.rb b/app/services/web_hook_service.rb index 6f97e36cf8bca9..e7a35a8ded489b 100644 --- a/app/services/web_hook_service.rb +++ b/app/services/web_hook_service.rb @@ -311,10 +311,14 @@ def request_payload strong_memoize_attr :request_payload def render_custom_template(template, params) - template.gsub(CUSTOM_TEMPLATE_INTERPOLATION_REGEX) do - value = params.dig(*Regexp.last_match(1).split('.')) - value_json = value.to_json - value.is_a?(String) ? value_json[1..-2] : value_json + if Feature.enabled?(:custom_webhook_template_serialization, hook.parent, type: :beta) + template.gsub(CUSTOM_TEMPLATE_INTERPOLATION_REGEX) do + value = params.dig(*Regexp.last_match(1).split('.')) + value_json = value.to_json + value.is_a?(String) ? value_json[1..-2] : value_json + end + else + template.gsub(CUSTOM_TEMPLATE_INTERPOLATION_REGEX) { params.dig(*Regexp.last_match(1).split('.')) } end end diff --git a/config/feature_flags/beta/custom_webhook_template_serialization.yml b/config/feature_flags/beta/custom_webhook_template_serialization.yml new file mode 100644 index 00000000000000..9424921935dbc7 --- /dev/null +++ b/config/feature_flags/beta/custom_webhook_template_serialization.yml @@ -0,0 +1,10 @@ +--- +name: custom_webhook_template_serialization +description: Enable JSON serialization in custom webhook template field interpolation +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/511476 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197992 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/560952 +milestone: '18.4' +group: group::import +type: beta +default_enabled: false diff --git a/doc/user/project/integrations/webhooks.md b/doc/user/project/integrations/webhooks.md index 5d1fff053b4022..f5499e883fde2e 100644 --- a/doc/user/project/integrations/webhooks.md +++ b/doc/user/project/integrations/webhooks.md @@ -164,6 +164,7 @@ Custom headers show in [**Recent events**](#view-webhook-request-history) with m - [Introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/142738) in GitLab 16.10 [with a flag](../../../administration/feature_flags/_index.md) named `custom_webhook_template`. Enabled by default. - [Generally available](https://gitlab.com/gitlab-org/gitlab/-/issues/439610) in GitLab 17.0. Feature flag `custom_webhook_template` removed. +- JSON serialization of interpolated field values [introduced](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/197992) in GitLab 18.4 [with a flag](../../../administration/feature_flags/_index.md) named `custom_webhook_template_serialization`. Disabled by default. {{< /history >}} diff --git a/spec/services/web_hook_service_spec.rb b/spec/services/web_hook_service_spec.rb index 12ecdd44f7b4f1..7f58a5398604b0 100644 --- a/spec/services/web_hook_service_spec.rb +++ b/spec/services/web_hook_service_spec.rb @@ -497,6 +497,32 @@ end end + context 'when serialization feature flag is disabled' do + before do + stub_feature_flags(custom_webhook_template_serialization: false) + end + + context 'when description contains a backslash' do + let(:data) do + { changes: { description: "\\This has a backslash" } } + end + + before do + project_hook.custom_webhook_template = '{"description":"{{changes.description}}"}' + end + + it 'handles the error', :aggregate_failures do + expect(service_instance.execute).to have_attributes( + status: :error, + message: 'Error while parsing rendered custom webhook template: invalid escaped character ' \ + '(after description) at line 1, column 17 [parse.c:315] in ' \ + '\'{"description":"\\This has a backslash"}' + ) + expect { service_instance.execute }.not_to raise_error + end + end + end + context 'when template is valid' do before do project_hook.custom_webhook_template = '{"before":"{{before}}"}' -- GitLab