feat(developer-settings): add webhook_headers backend support#116901
Closed
sentry-junior[bot] wants to merge 28 commits into
Closed
feat(developer-settings): add webhook_headers backend support#116901sentry-junior[bot] wants to merge 28 commits into
sentry-junior[bot] wants to merge 28 commits into
Conversation
Contributor
Backend Test FailuresFailures on
|
| schema=data["schema"], | ||
| overview=data["overview"], | ||
| allowed_origins=data["allowedOrigins"], | ||
| webhook_headers=data["webhookHeaders"], |
Contributor
There was a problem hiding this comment.
Custom webhook headers exposed unmasked in GET responses for published Sentry Apps
SentryAppSerializer returns webhookHeaders verbatim in the GET response for any Sentry App, and SentryAppPermission.has_object_permission lets any authenticated user GET a published app's details. Since webhook_headers is designed to carry per-app credentials (e.g. Authorization: Bearer ...), any Sentry user can read another org's webhook secrets via GET /api/0/sentry-apps/{slug}/. Mask the values like clientSecret does, or restrict exposure to org members with org:write.
Evidence
src/sentry/sentry_apps/api/serializers/sentry_app.py:150setswebhookHeaders=obj.webhook_headersunconditionally inSentryAppSerializerResponse, with noMASKED_VALUEbranch (contrast withclientSecretlater in the sameserialize).src/sentry/sentry_apps/api/bases/sentryapps.py:272returnsTruefromSentryAppPermission.has_object_permissionwheneversentry_app.is_published and request.method == 'GET', so any authenticated user passes the object permission check on the details endpoint.SentryAppDetailsEndpoint(endpoints/sentry_app_details.py:57-65) usesSentryAppDetailsEndpointPermission(a subclass ofSentryAppAndStaffPermission→SentryAppPermission) and callsserialize(sentry_app, ..., ResponseSentryAppSerializer())for GET, so the unmasked headers reach the response body.- Parser docs/tests in this PR describe values like
Authorization: Bearer ..., confirming the field is intended to carry secret credentials and is not just metadata.
Also found at 6 additional locations
src/sentry/sentry_apps/api/serializers/sentry_app.py:48src/sentry/sentry_apps/api/serializers/sentry_app.py:150src/sentry/sentry_apps/logic.py:117src/sentry/sentry_apps/logic.py:352src/sentry/sentry_apps/services/app/model.py:88tests/sentry/sentry_apps/api/endpoints/test_sentry_apps.py:762-766
Identified by Warden security-review · XPX-475
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Backend implementation of custom webhook headers for Sentry App integrations. This is the follow-up to the frontend-only PR #116732.
What changed
Model (
SentryApp): addswebhook_headers = ArrayField(TextField, default=list)— stores header lines as a plaintext[]column, same shape asevents.Migration (
1109_sentryapp_webhook_headers):AddFieldonsentry_sentryapp.RPC layer (
RpcSentryApp,serialize_sentry_app): exposeswebhook_headersacross the hybrid-cloud boundary.Creator / Updater (
SentryAppCreator,SentryAppUpdater): accept and persistwebhook_headers(Updater method:_update_webhook_headers).Parser (
SentryAppParser): newwebhookHeadersfield (list of strings). Validated: each item must beName: valueformat; reserved Sentry prefixes (sentry-hook-*,content-type,request-id) are rejected.Response serializer (
SentryAppSerializer): includeswebhookHeadersin all GET / POST / PUT responses.Endpoints:
SentryAppsEndpoint(POST) andSentryAppDetailsEndpoint(PUT) wire the new field through.Webhook delivery (
AppPlatformEvent.headers): custom headers are merged into the outgoing request dict before Sentry's own headers, so Sentry system headers always take precedence and cannot be spoofed.Notes
sentry-hook-,content-type, orrequest-idto prevent HMAC/resource spoofing.Action taken on behalf of Billy.
View Session in Sentry