Skip to content

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

Merged
GabeVillalobos merged 16 commits intomasterfrom
ceorourke/ISWF-2400
Apr 13, 2026
Merged

fix(integrations): Fix security vulnerabilities in Jira#112409
GabeVillalobos merged 16 commits intomasterfrom
ceorourke/ISWF-2400

Conversation

@ceorourke
Copy link
Copy Markdown
Member

@ceorourke ceorourke commented Apr 7, 2026

Require a kid to be passed to the Jira installed endpoint to prevent attacks. 👶

@linear-code
Copy link
Copy Markdown

linear-code bot commented Apr 7, 2026

@github-actions github-actions bot added the Scope: Backend Automatically applied to PRs that change backend components label Apr 7, 2026
Comment thread src/sentry/integrations/bitbucket/installed.py Outdated
Comment thread src/sentry/integrations/jira/webhooks/installed.py Outdated
Comment thread src/sentry/integrations/bitbucket/installed.py Outdated
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 7, 2026

Backend Test Failures

Failures on 6b4e119 in this run:

tests/sentry/integrations/jira/test_installed.py::JiraInstalledTest::test_with_shared_secretlog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/jira/test_installed.py:116: in test_with_shared_secret
    self.get_success_response(
src/sentry/testutils/cases.py:641: in get_success_response
    assert_status_code(response, 200, 300)
src/sentry/testutils/asserts.py:46: in assert_status_code
    assert minimum <= response.status_code < maximum, response
E   AssertionError: <Response status_code=400, "application/json">
E   assert 400 < 300
E    +  where 400 = <Response status_code=400, "application/json">.status_code

ceorourke and others added 2 commits April 7, 2026 14:58
Co-authored-by: sentry-warden[bot] <258096371+sentry-warden[bot]@users.noreply.github.com>
Co-authored-by: sentry-warden[bot] <258096371+sentry-warden[bot]@users.noreply.github.com>
Comment thread src/sentry/integrations/jira/webhooks/installed.py Outdated
Comment thread src/sentry/integrations/bitbucket/installed.py Outdated
@ceorourke ceorourke changed the title fix(integrations): Fix security vulnerabilities in Jira and Bitbucket fix(integrations): Fix security vulnerabilities in Jira Apr 7, 2026
Comment thread src/sentry/integrations/jira/webhooks/installed.py Outdated
Comment thread src/sentry/integrations/jira/webhooks/installed.py
@getsentry getsentry deleted a comment from github-actions bot Apr 7, 2026
@ceorourke ceorourke marked this pull request as ready for review April 7, 2026 23:51
@ceorourke ceorourke requested review from a team as code owners April 7, 2026 23:51
Comment thread src/sentry/integrations/jira/webhooks/installed.py
Comment thread src/sentry/integrations/jira/webhooks/installed.py
Comment thread src/sentry/integrations/jira/webhooks/installed.py
Comment thread tests/sentry/integrations/jira/test_installed.py
Comment thread src/sentry/integrations/jira/webhooks/installed.py
@github-actions
Copy link
Copy Markdown
Contributor

github-actions bot commented Apr 8, 2026

Backend Test Failures

Failures on 57b416b in this run:

tests/sentry/integrations/jira/test_installed.py::JiraInstalledTest::test_no_claimslog
[gw1] linux -- Python 3.13.1 /home/runner/work/sentry/sentry/.venv/bin/python3
tests/sentry/integrations/jira/test_installed.py:104: in test_no_claims
    self.get_error_response(
src/sentry/testutils/cases.py:663: in get_error_response
    assert_status_code(response, status_code)
src/sentry/testutils/asserts.py:46: in assert_status_code
    assert minimum <= response.status_code < maximum, response
E   AssertionError: <Response status_code=400, "application/json">
E   assert 409 <= 400
E    +  where 400 = <Response status_code=400, "application/json">.status_code

Comment thread src/sentry/integrations/jira/webhooks/installed.py
return self.respond(
{"detail": "Invalid or expired signature"}, status=status.HTTP_400_BAD_REQUEST
)
except DecodeError:
Copy link
Copy Markdown
Member

@GabeVillalobos GabeVillalobos Apr 8, 2026

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

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

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.

Comment thread src/sentry/integrations/jira/webhooks/installed.py Outdated
Comment thread src/sentry/integrations/jira/webhooks/installed.py Outdated
Copy link
Copy Markdown
Contributor

@cursor cursor bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b0b411c. Configure here.

Comment thread src/sentry/integrations/jira/webhooks/installed.py Outdated
Copy link
Copy Markdown
Member

@GabeVillalobos GabeVillalobos left a comment

Choose a reason for hiding this comment

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

Looks good to me, pending security signoff. We should address the warden comment and wrap the peek_header call with a try:catch

Copy link
Copy Markdown
Contributor

@geoffg-sentry geoffg-sentry left a comment

Choose a reason for hiding this comment

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

I noticed there's no specific tests for a fully forged JWT. We mock authenticate_asymmetric_jwt and test for error handling, but there's no test here for for sending a JWT without a kid which kinda

Comment thread src/sentry/integrations/jira/webhooks/installed.py
@geoffg-sentry geoffg-sentry dismissed their stale review April 13, 2026 17:47

missed the tests

@GabeVillalobos GabeVillalobos merged commit f355d89 into master Apr 13, 2026
56 checks passed
@GabeVillalobos GabeVillalobos deleted the ceorourke/ISWF-2400 branch April 13, 2026 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Scope: Backend Automatically applied to PRs that change backend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants