Skip to content

fix(integrations): Fix security vulnerabilities in Jira #112409

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ceorourke wants to merge 16 commits into
base: master
Choose a base branch
from
+141 −22
Open

fix(integrations): Fix security vulnerabilities in Jira #112409

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
16 commits
Select commit Hold shift + click to select a range
7eed62c
require kid
ceorourke Apr 7, 2026
7fc4602
validate jwt in bitbucket
ceorourke Apr 7, 2026
0deeb99
Update src/sentry/integrations/jira/webhooks/installed.py
ceorourke Apr 7, 2026
80d877e
Update src/sentry/integrations/bitbucket/installed.py
ceorourke Apr 7, 2026
4e0e5a3
rm bitbucket changes, handle symmetric and assymetric paths
ceorourke Apr 7, 2026
af7091b
require kid
ceorourke Apr 7, 2026
b20f977
get rid of that
ceorourke Apr 7, 2026
30a7fcd
Add slo stuff to test
ceorourke Apr 7, 2026
3d2c180
verify claims in try, update test
ceorourke Apr 8, 2026
02c42be
handle other errors
ceorourke Apr 8, 2026
16c17c1
generic error handling
ceorourke Apr 8, 2026
3e7afa4
unused code
ceorourke Apr 8, 2026
0d2da12
specific
ceorourke Apr 8, 2026
b0b411c
record fail for expired signature, add test cases
ceorourke Apr 8, 2026
f0be9b7
dont broadly catch exceptions
ceorourke Apr 8, 2026
a48ed95
wrap peek_header in try/catch
ceorourke Apr 9, 2026
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
45 changes: 35 additions & 10 deletions src/sentry/integrations/jira/webhooks/installed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import sentry_sdk
from django.db import router, transaction
from jwt import DecodeError, ExpiredSignatureError, InvalidKeyError, InvalidSignatureError
from rest_framework import status
from rest_framework.request import Request
from rest_framework.response import Response
Expand All @@ -20,9 +21,6 @@
)
from sentry.utils import jwt

# Atlassian sends scanner bots to "test" Atlassian apps and they often hit this endpoint with a bad kid causing errors
INVALID_KEY_IDS = ["fake-kid"]


@control_silo_endpoint
class JiraSentryInstalledWebhook(JiraWebhookBase):
Expand All @@ -46,7 +44,14 @@
lifecycle.record_failure(ProjectManagementFailuresReason.INSTALLATION_STATE_MISSING)
return self.respond(status=status.HTTP_400_BAD_REQUEST)

key_id = jwt.peek_header(token).get("kid")
try:
key_id = jwt.peek_header(token).get("kid")
except DecodeError:
lifecycle.record_halt(halt_reason="Failed to fetch key id")
return self.respond(
{"detail": "Failed to fetch key id"}, status=status.HTTP_400_BAD_REQUEST
)

lifecycle.add_extras(
{
"key_id": key_id,
Expand All @@ -56,14 +61,34 @@
}
)

if key_id:
if key_id in INVALID_KEY_IDS:
lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)")
return self.respond(
{"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST
)
if not key_id:
lifecycle.record_halt(halt_reason="Missing key_id (kid)")
return self.respond(
{"detail": "Missing key id"}, status=status.HTTP_400_BAD_REQUEST
)
try:
decoded_claims = authenticate_asymmetric_jwt(token, key_id)
verify_claims(decoded_claims, request.path, request.GET, method="POST")
except InvalidKeyError:
lifecycle.record_halt(halt_reason="JWT contained invalid key_id (kid)")
return self.respond(
{"detail": "Invalid key id"}, status=status.HTTP_400_BAD_REQUEST
)
except ExpiredSignatureError as e:
lifecycle.record_failure(e)
return self.respond(
{"detail": "Expired signature"}, status=status.HTTP_400_BAD_REQUEST
)
except InvalidSignatureError:
lifecycle.record_halt(halt_reason="JWT contained invalid signature")
return self.respond(
{"detail": "Invalid signature"}, status=status.HTTP_400_BAD_REQUEST
)
except DecodeError:
Copy link
Copy Markdown
Member

@GabeVillalobos GabeVillalobos Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Should we broaden this to cover all exceptions? It'd be bad if some other exception case could fall through to the next parts of the installation pipeline. A 400 exception when decoding the jwt fails would probably be a better response than a 500.

Copy link
Copy Markdown
Member

@GabeVillalobos GabeVillalobos Apr 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

On a similar note, we should add some tests for these cases to ensure error handling doesn't allow the pipeline to continue, and responds with the correct status code.

Copy link
Copy Markdown
Member Author

@ceorourke ceorourke Apr 8, 2026

Choose a reason for hiding this comment

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

I added tests for the specific errors we're catching including a DecodeError. The bots are yelling about my broader exception catching though which might be valid. I do worry I might be missing some other jwt exceptions but I'm not sure how to find them all.

lifecycle.record_halt(halt_reason="Could not decode JWT token")
return self.respond(
{"detail": "Could not decode JWT token"}, status=status.HTTP_400_BAD_REQUEST
)

Check warning on line 91 in src/sentry/integrations/jira/webhooks/installed.py

View check run for this annotation

@sentry/warden / warden: sentry-backend-bugs

Unhandled KeyError when JWT claims lack 'qsh' field 

The `verify_claims` function accesses `claims["qsh"]` directly without using `.get()`. If a malformed JWT is submitted that passes signature verification but lacks the `qsh` claim, a `KeyError` will be raised. This exception is not caught by the try/except block around `authenticate_asymmetric_jwt` and `verify_claims` (lines 69-91), which only catches `InvalidKeyError`, `ExpiredSignatureError`, `InvalidSignatureError`, and `DecodeError`. The unhandled `KeyError` would propagate up to the generic exception handler and return a 500 error instead of a proper 400 response.

data = JiraIntegrationProvider().build_integration(state)
integration = ensure_integration(self.provider, data)
Expand Down
118 changes: 106 additions & 12 deletions tests/sentry/integrations/jira/test_installed.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

import jwt
import responses
from jwt import DecodeError, ExpiredSignatureError, InvalidSignatureError
from rest_framework import status

from sentry.constants import ObjectStatus
Expand All @@ -16,7 +17,11 @@
AtlassianConnectValidationError,
get_query_hash,
)
from sentry.testutils.asserts import assert_count_of_metric, assert_halt_metric
from sentry.testutils.asserts import (
assert_count_of_metric,
assert_failure_metric,
assert_halt_metric,
)
from sentry.testutils.cases import APITestCase
from sentry.testutils.silo import control_silo_test
from sentry.utils.http import absolute_uri
Expand Down Expand Up @@ -49,9 +54,6 @@ def _jwt_token(
headers={**(headers or {}), "alg": jira_signing_algorithm},
)

def jwt_token_secret(self):
return self._jwt_token("HS256", self.shared_secret)

def jwt_token_cdn(self):
return self._jwt_token("RS256", RS256_KEY, headers={"kid": self.kid})

Expand Down Expand Up @@ -110,18 +112,83 @@ def test_no_claims(self, mock_authenticate_asymmetric_jwt: MagicMock) -> None:
status_code=status.HTTP_409_CONFLICT,
)

@patch(
"sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt",
side_effect=ExpiredSignatureError(),
)
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@patch("sentry_sdk.set_tag")
def test_with_shared_secret(self, mock_set_tag: MagicMock, mock_record_event) -> None:
self.get_success_response(
@responses.activate
def test_expired_signature(
self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock
) -> None:
self.add_response()

self.get_error_response(
**self.body(),
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_secret()),
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()),
status_code=status.HTTP_400_BAD_REQUEST,
)
# SLO metric asserts
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (failure) -> GET_CONTROL_RESPONSE (success)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.FAILURE, 1)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
assert_failure_metric(
mock_record_event,
ExpiredSignatureError(),
)
integration = Integration.objects.get(provider="jira", external_id=self.external_id)

mock_set_tag.assert_any_call("integration_id", integration.id)
assert integration.status == ObjectStatus.ACTIVE
mock_record_event.assert_called_with(EventLifecycleOutcome.SUCCESS, None, False, None)
@patch(
"sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt",
side_effect=InvalidSignatureError(),
)
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@responses.activate
def test_invalid_signature(
self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock
) -> None:
self.add_response()

self.get_error_response(
**self.body(),
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()),
status_code=status.HTTP_400_BAD_REQUEST,
)
# SLO metric asserts
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
assert_halt_metric(
mock_record_event,
"JWT contained invalid signature",
)

@patch(
"sentry.integrations.jira.webhooks.installed.authenticate_asymmetric_jwt",
side_effect=DecodeError(),
)
@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@responses.activate
def test_decode_error(
self, mock_record_event: MagicMock, mock_authenticate_asymmetric_jwt: MagicMock
) -> None:
self.add_response()

self.get_error_response(
**self.body(),
extra_headers=dict(HTTP_AUTHORIZATION="JWT " + self.jwt_token_cdn()),
status_code=status.HTTP_400_BAD_REQUEST,
)
# SLO metric asserts
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
assert_halt_metric(
mock_record_event,
"Could not decode JWT token",
)

@patch("sentry_sdk.set_tag")
@responses.activate
Expand All @@ -138,7 +205,34 @@ def test_with_key_id(self, mock_set_tag: MagicMock) -> None:
assert integration.status == ObjectStatus.ACTIVE

@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
def test_without_key_id(self, mock_record_event: MagicMock) -> None:
self.get_error_response(
**self.body(),
extra_headers=dict(
HTTP_AUTHORIZATION="JWT " + self._jwt_token("RS256", RS256_KEY, headers={})
),
status_code=status.HTTP_400_BAD_REQUEST,
)
# SLO metric asserts
# ENSURE_CONTROL_SILO (success) -> VERIFY_INSTALLATION (halt) -> GET_CONTROL_RESPONSE (success)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.STARTED, 3)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.HALTED, 1)
assert_count_of_metric(mock_record_event, EventLifecycleOutcome.SUCCESS, 2)
assert_halt_metric(
mock_record_event,
"Missing key_id (kid)",
)

@patch("sentry.integrations.utils.metrics.EventLifecycle.record_event")
@responses.activate
def test_with_invalid_key_id(self, mock_record_event: MagicMock) -> None:
responses.add(
responses.GET,
"https://connect-install-keys.atlassian.com/fake-kid",
body=b"Not Found",
status=404,
)

self.get_error_response(
**self.body(),
extra_headers=dict(
Expand Down
Loading