Skip to content
Merged
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
36 changes: 31 additions & 5 deletions src/sentry/sentry_apps/external_requests/utils.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import logging
import re
from collections.abc import Mapping
from typing import Any
from urllib.parse import urlparse

Expand All @@ -9,7 +10,9 @@
from requests.models import Response
from rest_framework import serializers

from sentry import features
from sentry.http import safe_urlopen
from sentry.organizations.services.organization.service import organization_service
from sentry.sentry_apps.event_types import SentryAppEventType
from sentry.sentry_apps.metrics import (
SentryAppExternalRequestFailureReason,
Expand All @@ -19,6 +22,7 @@
from sentry.sentry_apps.models.sentry_app import SentryApp, track_response_code
from sentry.sentry_apps.services.app.model import RpcSentryApp
from sentry.sentry_apps.utils.errors import SentryAppIntegratorError
from sentry.sentry_apps.utils.headers import mask_header_values, parse_custom_headers
from sentry.utils.sentry_apps import SentryAppWebhookRequestsBuffer
from sentry.utils.sentry_apps.webhooks import TIMEOUT_STATUS_CODE

Expand Down Expand Up @@ -102,16 +106,33 @@ def validate(instance, schema_type):
return True


def _custom_request_headers(sentry_app: SentryApp | RpcSentryApp) -> dict[str, str]:

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

this is a relatively inelegant way to check that the feature flag is enabled. we can remove it once we remove the flag

if not sentry_app.webhook_headers:
return {}
owner_context = organization_service.get_organization_by_id(
id=sentry_app.owner_id,
include_projects=False,
include_teams=False,
)
if owner_context is None or not features.has(
"organizations:sentry-apps-custom-webhook-headers", owner_context.organization
):
return {}
return parse_custom_headers(sentry_app.webhook_headers)


def send_and_save_sentry_app_request(
url: str,
sentry_app: SentryApp | RpcSentryApp,
org_id: int,
event: str,
headers: Mapping[str, str],
**kwargs: Any,
) -> Response:
"""
Send a webhook request, and save the request into the Redis buffer for the
app dashboard request log. Returns the response of the request.
Send a request to a Sentry App's endpoint, attaching the app's custom
headers, and save the request into the Redis buffer for the app dashboard
request log. Returns the response of the request.

kwargs ends up being the arguments passed into safe_urlopen
"""
Expand All @@ -123,8 +144,13 @@ def send_and_save_sentry_app_request(
buffer = SentryAppWebhookRequestsBuffer(sentry_app)
slug = sentry_app.slug_for_metrics

custom_headers = _custom_request_headers(sentry_app)
send_headers = {**custom_headers, **headers}
# Since some headers may carry secrets, we mask them to avoid logging them
loggable_headers = {**mask_header_values(custom_headers), **headers}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

note that we mask all custom headers in the log, rather than just potentially sensitive fields. i figure this is a little easier + more secure if we treat all the custom headers as sensitive.


try:
resp = safe_urlopen(url=url, **kwargs)
resp = safe_urlopen(url=url, headers=send_headers, **kwargs)
except (Timeout, ConnectionError) as e:
error_type = e.__class__.__name__.lower()
lifecycle.add_extras(
Expand All @@ -142,7 +168,7 @@ def send_and_save_sentry_app_request(
org_id=org_id,
event=event,
url=url,
headers=kwargs.get("headers"),
headers=loggable_headers,
)
lifecycle.record_halt(e)
# Re-raise the exception because some of these tasks might retry on the exception
Expand All @@ -157,7 +183,7 @@ def send_and_save_sentry_app_request(
error_id=resp.headers.get("Sentry-Hook-Error"),
project_id=resp.headers.get("Sentry-Hook-Project"),
response=resp,
headers=kwargs.get("headers"),
headers=loggable_headers,
)
try:
resp.raise_for_status()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,27 @@ def setUp(self) -> None:
self.error_message = "Channel not found!"
self.success_message = "Created alert!"

@responses.activate
def test_sends_custom_headers(self) -> None:
self.install.sentry_app.update(webhook_headers=["Authorization: Bearer secret-token"])

responses.add(
method=responses.POST,
url="https://example.com/sentry/alert-rule",
status=200,
)

with self.feature("organizations:sentry-apps-custom-webhook-headers"):
result = SentryAppAlertRuleActionRequester(
install=self.install,
uri="/sentry/alert-rule",
fields=self.fields,
).run()

assert result["success"]
request = responses.calls[0].request
assert request.headers["Authorization"] == "Bearer secret-token"

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_makes_successful_request(self, mock_record: MagicMock) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,37 @@ def setUp(self) -> None:
self.rpc_user = serialize_rpc_user(self.user)
self.install = app_service.get_many(filter=dict(installation_ids=[self.orm_install.id]))[0]

@responses.activate
def test_sends_custom_headers(self) -> None:
with assume_test_silo_mode_of(SentryApp):
self.sentry_app.update(webhook_headers=["Authorization: Bearer secret-token"])
self.install = app_service.get_many(filter=dict(installation_ids=[self.orm_install.id]))[0]

responses.add(
method=responses.POST,
url="https://example.com/link-issue",
json={
"project": "ProjectName",
"webUrl": "https://example.com/project/issue-id",
"identifier": "issue-1",
},
status=200,
content_type="application/json",
)

with self.feature("organizations:sentry-apps-custom-webhook-headers"):
IssueLinkRequester(
install=self.install,
group=self.group,
uri="/link-issue",
fields={},
user=self.rpc_user,
action=IssueRequestActionType("create"),
).run()

request = responses.calls[0].request
assert request.headers["Authorization"] == "Bearer secret-token"

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_makes_request(self, mock_record: MagicMock) -> None:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
SentryAppExternalRequestFailureReason,
SentryAppExternalRequestHaltReason,
)
from sentry.sentry_apps.models.sentry_app import SentryApp
from sentry.sentry_apps.models.sentry_app import MASKED_VALUE, SentryApp
from sentry.sentry_apps.services.app import app_service
from sentry.sentry_apps.utils.errors import SentryAppIntegratorError, SentryAppSentryError
from sentry.testutils.asserts import (
Expand Down Expand Up @@ -90,6 +90,64 @@ def test_makes_request(self, mock_record: MagicMock) -> None:
mock_record=mock_record, outcome=EventLifecycleOutcome.SUCCESS, outcome_count=2
)

@responses.activate
def test_sends_custom_headers_and_masks_them_in_buffer(self) -> None:
with assume_test_silo_mode_of(SentryApp):
self.sentry_app.update(
webhook_headers=[
"Authorization: Bearer secret-token",
"Content-Type: text/plain",
]
)
self.install = app_service.get_many(filter=dict(installation_ids=[self.orm_install.id]))[0]

responses.add(
method=responses.GET,
url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}",
body="Something failed",
status=500,
)

with self.feature("organizations:sentry-apps-custom-webhook-headers"):
with pytest.raises(SentryAppIntegratorError):
SelectRequester(
install=self.install, project_slug=self.project.slug, uri="/get-issues"
).run()

request = responses.calls[0].request
assert request.headers["Authorization"] == "Bearer secret-token"
# Sentry's own headers win when a custom header collides.
assert request.headers["Content-Type"] == "application/json"

requests = SentryAppWebhookRequestsBuffer(self.sentry_app).get_requests()
assert len(requests) == 1
logged_headers = requests[0]["request_headers"]
assert logged_headers is not None
assert logged_headers["Authorization"] == MASKED_VALUE
assert logged_headers["Content-Type"] == "application/json"
assert logged_headers["Sentry-App-Signature"] == self.sentry_app.build_signature("")
assert "secret-token" not in logged_headers.values()

@responses.activate
def test_no_custom_headers_without_feature(self) -> None:
with assume_test_silo_mode_of(SentryApp):
self.sentry_app.update(webhook_headers=["Authorization: Bearer secret-token"])
self.install = app_service.get_many(filter=dict(installation_ids=[self.orm_install.id]))[0]

responses.add(
method=responses.GET,
url=f"https://example.com/get-issues?installationId={self.install.uuid}&projectSlug={self.project.slug}",
json=[{"label": "An Issue", "value": "123"}],
status=200,
content_type="application/json",
)

SelectRequester(
install=self.install, project_slug=self.project.slug, uri="/get-issues"
).run()

assert "Authorization" not in responses.calls[0].request.headers

@responses.activate
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_invalid_response_missing_label(self, mock_record: MagicMock) -> None:
Expand Down
Loading