Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions src/sentry/apidocs/examples/sentry_app_examples.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ class SentryAppExamples:
"uuid": "77cebea3-019e-484d-8673-6c3969698827",
"verifyInstall": True,
"webhookUrl": "https://example.com/webhook",
"webhookHeaders": ["X-Custom-Header: value"],
"clientId": "ed06141686bb60102d878c607eff449fa9907fa7a8cb70f0d337a8fb0b6566c3",
"clientSecret": "**********",
"owner": {"id": 42, "slug": "acme-corp"},
Expand Down Expand Up @@ -73,6 +74,7 @@ class SentryAppExamples:
"uuid": "77cebea3-019e-484d-8673-6c3969698827",
"verifyInstall": True,
"webhookUrl": "https://example.com/webhook",
"webhookHeaders": ["X-Custom-Header: value"],
"clientId": "ed06141686bb60102d878c607eff449fa9907fa7a8cb70f0d337a8fb0b6566c3",
"clientSecret": "**********",
"owner": {"id": 42, "slug": "acme-corp"},
Expand Down Expand Up @@ -131,6 +133,7 @@ class SentryAppExamples:
"uuid": "77cebea3-019e-484d-8673-6c3969698827",
"verifyInstall": True,
"webhookUrl": "https://example.com/webhook",
"webhookHeaders": ["X-Custom-Header: value"],
"clientId": "ed06141686bb60102d878c607eff449fa9907fa7a8cb70f0d337a8fb0b6566c3",
"clientSecret": "**********",
"owner": {"id": 42, "slug": "acme-corp"},
Expand All @@ -155,6 +158,7 @@ class SentryAppExamples:
"uuid": "77cebea3-019e-484d-8673-123124234",
"verifyInstall": True,
"webhookUrl": "https://example.com/webhook",
"webhookHeaders": ["X-Custom-Header: value"],
"clientId": "2730a92919437a7b052e6827cd2c9f119be37101asdasdad123131231231231",
"clientSecret": "**********",
"owner": {"id": 42, "slug": "acme-corp"},
Expand Down
1 change: 1 addition & 0 deletions src/sentry/sentry_apps/api/endpoints/sentry_app_details.py
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,7 @@ def put(
schema=result.get("schema"),
overview=result.get("overview"),
allowed_origins=result.get("allowedOrigins"),
webhook_headers=result.get("webhookHeaders"),
popularity=result.get("popularity"),
is_disabled=result.get("isDisabled"),
).run(user=request.user)
Expand Down
4 changes: 4 additions & 0 deletions src/sentry/sentry_apps/api/endpoints/sentry_apps.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@ def post(self, request: Request, organization) -> Response:
"schema": request.data.get("schema", {}),
"overview": request.data.get("overview"),
"allowedOrigins": request.data.get("allowedOrigins", []),
"webhookHeaders": request.data.get("webhookHeaders", []),
"popularity": (
request.data.get("popularity") if is_active_superuser(request) else None
),
Expand All @@ -111,6 +112,8 @@ def post(self, request: Request, organization) -> Response:
serializer = SentryAppParser(data=data, access=request.access, context={"request": request})

if serializer.is_valid():
validated_data = serializer.validated_data

if data.get("isInternal"):
data["verifyInstall"] = False
data["author"] = data["author"] or organization.name
Expand All @@ -133,6 +136,7 @@ def post(self, request: Request, organization) -> Response:
schema=data["schema"],
overview=data["overview"],
allowed_origins=data["allowedOrigins"],
webhook_headers=validated_data.get("webhookHeaders", []),
popularity=data["popularity"],
).run(user=request.user, request=request, skip_default_auth_token=True)
# We want to stop creating the default auth token for new apps and installations through the API
Expand Down
69 changes: 69 additions & 0 deletions src/sentry/sentry_apps/api/parsers/sentry_app.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import logging
import re

from drf_spectacular.types import OpenApiTypes
from drf_spectacular.utils import extend_schema_field, extend_schema_serializer
Expand All @@ -15,6 +16,27 @@
from sentry.sentry_apps.utils.webhooks import VALID_EVENT_RESOURCES
from sentry.utils.display_name_filter import is_spam_display_name

# Custom webhook headers are intentionally limited to Authorization and X-*
# custom headers. Names are compared case-insensitively.
ALLOWED_WEBHOOK_HEADERS = frozenset({"authorization"})

# RFC 7230 §3.2.6 — header field names are "tokens": letters, digits, and
# the limited punctuation set below. Excludes separators and control chars.
_HTTP_TOKEN_RE = re.compile(r"^[!#$%&'*+\-.^_`|~A-Za-z0-9]+$")

# X-* headers Sentry owns, or transport/proxy identity headers that must not be
# user-controlled. These are the exceptions carved out of the X-* allowance
# below — every other non-allowed header is already rejected by the allow list,
# so only X-* names need to be reserved here.
RESERVED_WEBHOOK_HEADERS = frozenset(
{
"x-forwarded",
"x-real-ip",
"x-sentry",
}
)
RESERVED_WEBHOOK_HEADER_PREFIXES = ("x-forwarded-", "x-sentry-")


@extend_schema_field(build_typed_list(OpenApiTypes.STR))
class ApiScopesField(serializers.Field):
Expand Down Expand Up @@ -144,6 +166,14 @@ class SentryAppParser(Serializer):
required=False,
help_text="The list of allowed origins for CORS.",
)
webhookHeaders = serializers.ListField(
Comment thread
sentry-warden[bot] marked this conversation as resolved.
child=serializers.CharField(max_length=1024),
required=False,
help_text=(
"Custom headers sent with every webhook request. Each entry is a single "
"'Header-Name: value' pair."
),
)
# Bounds chosen to match PositiveSmallIntegerField (https://docs.djangoproject.com/en/3.2/ref/models/fields/#positivesmallintegerfield)
popularity = serializers.IntegerField(
min_value=0,
Expand Down Expand Up @@ -199,6 +229,45 @@ def validate_allowedOrigins(self, value):
raise ValidationError("'*' not allowed in origin")
return value

def validate_webhookHeaders(self, value):
if len(value) > 20:
raise ValidationError("Cannot configure more than 20 custom webhook headers.")
seen_names = set()
for header in value:
# Reject CR/LF to prevent header injection / request splitting.
if "\n" in header or "\r" in header:
raise ValidationError("Webhook headers cannot contain newlines.")
name, separator, _header_value = header.partition(":")
name = name.strip()
if not separator or not name:
raise ValidationError(
f"Invalid webhook header '{header}'. Use the format 'Header-Name: value'."
)
if not _HTTP_TOKEN_RE.match(name):
raise ValidationError(
f"'{name}' contains invalid characters. Header names must only use "
"letters, digits, and the punctuation characters !#$%&'*+-.^_`|~"
)
normalized = name.lower()
if normalized in RESERVED_WEBHOOK_HEADERS or normalized.startswith(
RESERVED_WEBHOOK_HEADER_PREFIXES
):
raise ValidationError(f"'{name}' is a reserved header and cannot be overridden.")
if normalized not in ALLOWED_WEBHOOK_HEADERS and not normalized.startswith("x-"):
raise ValidationError(
f"'{name}' is not an allowed webhook header. Use Authorization "
"or X-* custom headers."
)
# Reject duplicate names (case-insensitive). This keeps the masked-value
# round-trip unambiguous: the updater re-pairs masked entries to stored
# values by header name, which only works if names are unique.
if normalized in seen_names:
raise ValidationError(
f"Duplicate webhook header '{name}'. Each header may only be set once."
)
seen_names.add(normalized)
return value

def validate_scopes(self, value):
if not value:
return value
Expand Down
27 changes: 24 additions & 3 deletions src/sentry/sentry_apps/api/serializers/sentry_app.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,20 @@
from sentry.users.services.user.service import user_service


def mask_webhook_header_values(headers: Sequence[str]) -> list[str]:
"""Replace each header's value with MASKED_VALUE, preserving the header name.

Webhook header values may carry secrets (e.g. bearer tokens), so they are masked
for viewers without elevated access. The name is kept visible so the form can
still render which headers are set.
"""
masked = []
for header in headers:
name, separator, _value = header.partition(":")
masked.append(f"{name}: {MASKED_VALUE}" if separator else MASKED_VALUE)
return masked


class OwnerResponseField(TypedDict):
id: int
slug: str
Expand All @@ -45,6 +59,8 @@ class SentryAppSerializerResponse(TypedDict):
status: str
uuid: str
verifyInstall: bool
# Header values are masked unless the viewer is allowed to see secrets.
webhookHeaders: list[str]

# Optional fields
isDisabled: NotRequired[bool]
Expand Down Expand Up @@ -145,6 +161,9 @@ def serialize(
uuid=obj.uuid,
verifyInstall=obj.verify_install,
webhookUrl=obj.webhook_url,
# Header values are write-only after save; masked values can still be
# resubmitted unchanged because the updater preserves stored values.
webhookHeaders=mask_webhook_header_values(obj.webhook_headers),
Comment thread
sentry-warden[bot] marked this conversation as resolved.
)

if obj.status != SentryAppStatus.INTERNAL:
Expand All @@ -167,13 +186,15 @@ def serialize(

assert application, "Sentry App must have an associated ApiApplication"

client_secret = MASKED_VALUE
if elevated_user or (
can_view_secrets = elevated_user or (
owner_context
and owner_context.member
and "org:write" in owner_context.member.scopes
and obj.show_auth_info(owner_context.member)
):
)

client_secret = MASKED_VALUE
if can_view_secrets:
client_secret = application.client_secret

data.update(
Expand Down
39 changes: 39 additions & 0 deletions src/sentry/sentry_apps/logic.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@
SentryAppInteractionType,
)
from sentry.sentry_apps.models.sentry_app import (
MASKED_VALUE,
REQUIRED_EVENT_PERMISSIONS,
UUID_CHARS_IN_SLUG,
SentryApp,
Expand Down Expand Up @@ -114,6 +115,7 @@ class SentryAppUpdater:
schema: Schema | None = None
overview: str | None = None
allowed_origins: list[str] | None = None
webhook_headers: list[str] | None = None
popularity: int | None = None
features: list[int] | None = None
is_disabled: bool | None = None
Expand All @@ -137,6 +139,7 @@ def run(self, user: User | RpcUser) -> SentryApp:
self._update_verify_install()
self._update_overview()
self._update_allowed_origins()
self._update_webhook_headers()
new_schema_elements = self._update_schema()
self._update_popularity(user=user)
self.sentry_app.save()
Expand Down Expand Up @@ -286,6 +289,40 @@ def _update_allowed_origins(self) -> None:
self.sentry_app.application.allowed_origins = "\n".join(self.allowed_origins)
self.sentry_app.application.save()

def _update_webhook_headers(self) -> None:
# None means "not provided" (leave unchanged); an empty list clears all headers.
if self.webhook_headers is None:
return

# The serializer masks header values on read, so an unchanged entry comes back
# as "Header-Name: <MASKED_VALUE>". Substitute the stored value for any masked
# entry (matched by name) so a prefill+resave doesn't overwrite real secrets.
# Drop masked entries with no stored match.
#
# Names are unique (the parser rejects duplicates), so this re-pairing is
# unambiguous. Known limitation: renaming a header while leaving its value
# masked can't be matched by the new name and will drop the entry — only
# reachable by an editor who sees masks (org:write without scope coverage);
# they should re-enter the value when renaming.
existing_by_name = {}
for header in self.sentry_app.webhook_headers:
name, separator, _value = header.partition(":")
if separator:
existing_by_name[name.strip().lower()] = header
Comment on lines +307 to +311

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

create a map of existing header names to the header so that we can check if the user has updated an existing header's value


resolved: list[str] = []
for header in self.webhook_headers:
name, separator, value = header.partition(":")
if separator and value.strip() == MASKED_VALUE:
stored = existing_by_name.get(name.strip().lower())
if stored is not None:
resolved.append(stored)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since the read api only sends masked values, the user will be sending us a mix of masked and unmasked. assume that they are not modifying a header if its value is the masked value, also SOL if you want to change to the masked value.

else:
resolved.append(header)

self.sentry_app.webhook_headers = resolved
# Persisted by the sentry_app.save() call at the end of run().

def _update_popularity(self, user: User | RpcUser) -> None:
if self.popularity is not None:
if _is_elevated_user(user):
Expand Down Expand Up @@ -343,6 +380,7 @@ class SentryAppCreator:
schema: Schema = dataclasses.field(default_factory=dict)
overview: str | None = None
allowed_origins: list[str] = dataclasses.field(default_factory=list)
webhook_headers: list[str] = dataclasses.field(default_factory=list)
popularity: int | None = None
metadata: dict | None = field(default_factory=dict)

Expand Down Expand Up @@ -426,6 +464,7 @@ def _create_sentry_app(
"events": expand_events(self.events),
"schema": self.schema or {},
"webhook_url": self.webhook_url,
"webhook_headers": self.webhook_headers,
"redirect_url": self.redirect_url,
"is_alertable": self.is_alertable,
"verify_install": self.verify_install,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ def test_gets_all_apps_in_own_org(self) -> None:
"clientSecret": self.unpublished_app.application.client_secret,
"overview": self.unpublished_app.overview,
"allowedOrigins": [],
"webhookHeaders": [],
"schema": {},
"owner": {"id": self.org.id, "slug": self.org.slug},
"featureData": [
Expand Down
Loading
Loading