Skip to content
Open
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
2 changes: 1 addition & 1 deletion src/sentry/conf/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -837,7 +837,7 @@ def SOCIAL_AUTH_DEFAULT_USERNAME() -> str:

# Settings for encrypted database fields.
DATABASE_ENCRYPTION_SETTINGS: EncryptedFieldSettings = {
"method": "plaintext",
"method": "fernet",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

"fernet_primary_key_id": os.getenv("DATABASE_ENCRYPTION_FERNET_PRIMARY_KEY_ID"),
"fernet_keys_location": os.getenv("DATABASE_ENCRYPTION_FERNET_KEYS_LOCATION"),
}
Expand Down
6 changes: 3 additions & 3 deletions src/sentry/options/defaults.py
Original file line number Diff line number Diff line change
Expand Up @@ -3497,13 +3497,13 @@

# Database field encryption method
# Supported values:
# - 'plaintext': No encryption (default)
# - 'fernet': Fernet symmetric encryption
# - 'plaintext': No encryption
# - 'fernet': Fernet symmetric encryption (default)
# - 'keysets': (Future) Google Tink keysets for key rotation
register(
"database.encryption.method",
type=String,
default="plaintext",
default="fernet",
flags=FLAG_PRIORITIZE_DISK | FLAG_AUTOMATOR_MODIFIABLE,
)

Expand Down
26 changes: 25 additions & 1 deletion src/sentry_plugins/jira/plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
import re
from urllib.parse import parse_qs, quote_plus, unquote_plus, urlencode, urlsplit, urlunsplit

from django.conf import settings
from django.urls import re_path
from rest_framework.request import Request
from rest_framework.response import Response

from sentry.db.models.fields.encryption import EncryptedTextField
from sentry.exceptions import PluginError
from sentry.integrations.base import FeatureDescription, IntegrationFeatures
from sentry.models.groupmeta import GroupMeta
Expand Down Expand Up @@ -44,6 +44,30 @@ class JiraPlugin(CorePluginMixin, IssuePlugin2):
IntegrationFeatures.ISSUE_BASIC,
)
]
_password_field = EncryptedTextField()

def set_option(self, key, value, project=None, user=None) -> None:
if key == "password" and isinstance(value, str) and value:
# Avoid re-encrypting already-encrypted values.
if not value.startswith("enc:"):
value = self._password_field.get_prep_value(value)
super().set_option(key, value, project=project, user=user)

def get_option(self, key, project=None, user=None):
value = super().get_option(key, project=project, user=user)
if key != "password" or not isinstance(value, str) or not value:
return value

try:
decrypted = self._password_field.to_python(value)
if isinstance(decrypted, bytes):
return decrypted.decode("utf-8")
return decrypted
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a65e6ef. Configure here.

except Exception:
logger.warning(
"jira.password.decrypt.failed", extra={"project_id": getattr(project, "id", None)}
)
return None
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit a65e6ef. Configure here.


def get_group_urls(self):
_patterns = super().get_group_urls()
Expand Down
8 changes: 8 additions & 0 deletions tests/sentry/integrations/models/test_integration_security.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
from sentry.db.models.fields.encryption import EncryptedJSONField
from sentry.integrations.models.integration import Integration
from sentry.testutils.cases import TestCase


class IntegrationSecurityTest(TestCase):
def test_metadata_field_is_encrypted_json(self) -> None:
assert isinstance(Integration._meta.get_field("metadata"), EncryptedJSONField)
5 changes: 5 additions & 0 deletions tests/sentry/users/models/test_identity.py
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
from sentry.identity import register
from sentry.identity.providers.dummy import DummyProvider
from sentry.db.models.fields.encryption import EncryptedJSONField
from sentry.users.models.identity import Identity
from sentry.testutils.cases import TestCase
from sentry.testutils.silo import control_silo_test


@control_silo_test
class IdentityTestCase(TestCase):
def test_data_field_is_encrypted_json(self) -> None:
assert isinstance(Identity._meta.get_field("data"), EncryptedJSONField)

def test_get_provider(self) -> None:
integration = self.create_integration(
organization=self.organization, provider="dummy", external_id="tester_id"
Expand Down
15 changes: 15 additions & 0 deletions tests/sentry_plugins/jira/test_plugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from django.test import RequestFactory
from django.urls import reverse

from sentry.models.options.project_option import ProjectOption
from sentry.testutils.cases import TestCase
from sentry.testutils.requests import drf_request_from_request
from sentry_plugins.jira.plugin import JiraPlugin
Expand Down Expand Up @@ -302,6 +303,20 @@ def test_no_secrets(self) -> None:
assert password_config.get("hasSavedValue") is True
assert password_config.get("prefix") == ""

def test_password_is_encrypted_in_project_option_storage(self) -> None:
self.plugin.set_option("password", "abcdef", self.project)

stored = ProjectOption.objects.get(project=self.project, key="jira:password").value
assert isinstance(stored, str)
assert stored.startswith("enc:")
assert stored != "abcdef"
assert self.plugin.get_option("password", self.project) == "abcdef"

def test_plaintext_password_is_still_readable(self) -> None:
ProjectOption.objects.set_value(self.project, "jira:password", "legacy-plaintext")

assert self.plugin.get_option("password", self.project) == "legacy-plaintext"

def test_get_formatted_user(self) -> None:
assert self.plugin._get_formatted_user(
{"displayName": "Foo Bar", "emailAddress": "foo@sentry.io", "name": "foobar"}
Expand Down
Loading