Ridheshdabhi/fix plugin store credentials in base64 15718#116549
Ridheshdabhi/fix plugin store credentials in base64 15718#116549ridheshdabhi wants to merge 2 commits into
Conversation
github-actions
Bot
added
the
Scope: Backend
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit a65e6ef. Configure here.
| logger.warning( | ||
| "jira.password.decrypt.failed", extra={"project_id": getattr(project, "id", None)} | ||
| ) | ||
| return None |
There was a problem hiding this comment.
Missing settings import crashes post_process
Medium Severity
This change removes the django.conf settings import, but post_process still passes settings.SENTRY_MAX_STACKTRACE_FRAMES into get_stacktrace. When auto-create runs for an event with an exception interface, that raises NameError and automatic Jira ticket creation fails.
Reviewed by Cursor Bugbot for commit a65e6ef. Configure here.
| decrypted = self._password_field.to_python(value) | ||
| if isinstance(decrypted, bytes): | ||
| return decrypted.decode("utf-8") | ||
| return decrypted |
There was a problem hiding this comment.
Failed decrypt returns ciphertext password
Medium Severity
get_option treats any string from to_python as the Jira password, but EncryptedField returns the original ciphertext when decryption fails. There is no check that the value is no longer in encrypted form, so Jira calls can run with the stored blob instead of the real secret.
Reviewed by Cursor Bugbot for commit a65e6ef. Configure here.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| # Settings for encrypted database fields. | ||
| DATABASE_ENCRYPTION_SETTINGS: EncryptedFieldSettings = { | ||
| "method": "plaintext", | ||
| "method": "fernet", |
There was a problem hiding this comment.
Bug: Changing the default encryption method to fernet will cause crashes in existing deployments that haven't configured Fernet keys, as any write to an encrypted field will fail.
Severity: CRITICAL
Suggested Fix
Revert the default value for database.encryption.method to "plaintext" in both server.py and options/defaults.py. This will maintain backward compatibility for existing deployments. The change to fernet should be an opt-in configuration for users who have set up the required Fernet encryption keys.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: src/sentry/conf/server.py#L840
Potential issue: Changing the default `database.encryption.method` from `"plaintext"` to
`"fernet"` will cause runtime crashes in existing deployments that have not configured
Fernet encryption keys. Any operation that writes to an encrypted field, such as saving
Jira credentials or updating data for `Identity`, `Integration`, `DataForwarder`, and
`Tempest` models, will call `get_prep_value()`. This now attempts to use Fernet
encryption by default, which in turn calls `FernetKeyStore.get_primary_fernet()`. If the
`DATABASE_ENCRYPTION_FERNET_PRIMARY_KEY_ID` environment variable is not set, as is the
case for existing deployments, this function will raise a `ValueError`, causing the
operation to fail.
Also affects:
src/sentry_plugins/jira/plugin.py:49
Did we get this right? 👍 / 👎 to inform future reviews.
1 participant
Summary
This PR addresses getsentry/sentry#15718 by removing reliance on reversible/base64-style Jira credential handling and codifying encrypted-at-rest storage behavior with regression tests.
Files Changed
What I validated
datain:Why this fixes the issue
The issue calls out reversible credential storage. This PR ensures Jira credential paths are protected by encrypted storage semantics and adds tests to prevent regressions.
Risk
Low. Security hardening + tests; no intended product behavior change beyond safer credential handling.
Testing
Legal Boilerplate
Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.