Skip to content

Commit bfcfdcc

Browse files
billyvgclaude
andcommitted
test(integrations): Cover webhook header masking edge cases and secret scrubbing
Lock in the security and correctness guarantees of the webhook header masking logic that were previously only described in comments: - CR/LF rejection guards against header injection / request splitting - reserved-header check is case-insensitive (relies on .lower()) - a masked entry with no stored match is dropped, never persisting the literal mask placeholder as a real header value - the documented rename-while-masked drop behavior is pinned - relocation export scrubs webhook_headers so secrets never leave Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent d8b1a0e commit bfcfdcc

2 files changed

Lines changed: 91 additions & 0 deletions

File tree

tests/sentry/sentry_apps/api/endpoints/test_sentry_app_details.py

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -820,6 +820,72 @@ def test_clear_webhook_headers(self) -> None:
820820
self.published_app.refresh_from_db()
821821
assert self.published_app.webhook_headers == []
822822

823+
@override_options({"staff.ga-rollout": True})
824+
def test_webhook_headers_reject_newline_injection(self) -> None:
825+
# CR/LF in a header would let a caller smuggle extra headers / split the
826+
# request when these are written to the outgoing webhook. The validator must
827+
# reject them regardless of which line terminator is used.
828+
for malicious in [
829+
"X-Evil: value\r\nInjected: gotcha",
830+
"X-Evil: value\nInjected: gotcha",
831+
"X-Evil: value\rInjected: gotcha",
832+
]:
833+
response = self.get_error_response(
834+
self.published_app.slug,
835+
webhookHeaders=[malicious],
836+
status_code=400,
837+
)
838+
assert "webhookHeaders" in response.data
839+
self.published_app.refresh_from_db()
840+
assert self.published_app.webhook_headers == []
841+
842+
@override_options({"staff.ga-rollout": True})
843+
def test_webhook_headers_reserved_check_is_case_insensitive(self) -> None:
844+
# The reserved-name guard normalizes with .lower(), so non-canonical casing
845+
# must not slip a reserved header through.
846+
for reserved in ["content-TYPE: text/plain", "HOST: evil.test", "SENTRY-HOOK-foo: x"]:
847+
response = self.get_error_response(
848+
self.published_app.slug,
849+
webhookHeaders=[reserved],
850+
status_code=400,
851+
)
852+
assert "webhookHeaders" in response.data
853+
854+
@override_options({"staff.ga-rollout": True})
855+
def test_webhook_headers_unmatched_masked_entry_dropped(self) -> None:
856+
# A masked entry whose name matches no stored header can't be re-paired to a
857+
# real value. It must be dropped rather than persisting the literal mask as a
858+
# header value (which would then be sent on every outgoing webhook).
859+
self.published_app.webhook_headers = ["Authorization: Bearer secret-token"]
860+
self.published_app.save()
861+
862+
self.get_success_response(
863+
self.published_app.slug,
864+
webhookHeaders=[
865+
f"Authorization: {MASKED_VALUE}",
866+
f"X-Ghost: {MASKED_VALUE}",
867+
],
868+
status_code=200,
869+
)
870+
self.published_app.refresh_from_db()
871+
assert self.published_app.webhook_headers == ["Authorization: Bearer secret-token"]
872+
873+
@override_options({"staff.ga-rollout": True})
874+
def test_webhook_headers_rename_while_masked_drops_entry(self) -> None:
875+
# Documented limitation: renaming a header while leaving its value masked
876+
# can't be matched by the new name, so the entry is dropped. This pins that
877+
# behavior so any future change to it is a deliberate decision.
878+
self.published_app.webhook_headers = ["Authorization: Bearer secret-token"]
879+
self.published_app.save()
880+
881+
self.get_success_response(
882+
self.published_app.slug,
883+
webhookHeaders=[f"X-Renamed: {MASKED_VALUE}"],
884+
status_code=200,
885+
)
886+
self.published_app.refresh_from_db()
887+
assert self.published_app.webhook_headers == []
888+
823889
@override_options({"staff.ga-rollout": True})
824890
def test_members_cant_update(self) -> None:
825891
with assume_test_silo_mode(SiloMode.CELL):

tests/sentry/sentry_apps/models/test_sentryapp.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,8 @@
1+
import orjson
2+
from django.core.serializers import serialize as django_serialize
3+
4+
from sentry.backup.helpers import DatetimeSafeDjangoJSONEncoder
5+
from sentry.backup.sanitize import sanitize
16
from sentry.constants import SentryAppStatus
27
from sentry.hybridcloud.models.outbox import ControlOutbox
38
from sentry.hybridcloud.outbox.category import OutboxCategory
@@ -105,3 +110,23 @@ def test_cells_with_installations(self) -> None:
105110
organization=self.eu_org, slug=self.sentry_app.slug, prevent_token_exchange=True
106111
)
107112
assert self.sentry_app.cells_with_installations() == {"us", "eu"}
113+
114+
def test_sanitize_relocation_json_scrubs_webhook_headers(self) -> None:
115+
# webhook_headers can hold auth tokens, so the relocation export must never
116+
# include them. Exercise the real export path: serialize -> sanitize.
117+
self.sentry_app.webhook_headers = ["Authorization: Bearer super-secret-token"]
118+
self.sentry_app.save()
119+
120+
json_data = orjson.loads(
121+
django_serialize(
122+
"json",
123+
[self.sentry_app],
124+
use_natural_foreign_keys=False,
125+
cls=DatetimeSafeDjangoJSONEncoder,
126+
)
127+
)
128+
sanitized = sanitize(json_data)
129+
130+
fields = sanitized[0]["fields"]
131+
assert fields["webhook_headers"] == "[]"
132+
assert "super-secret-token" not in orjson.dumps(sanitized).decode()

0 commit comments

Comments
 (0)