fix(rules): Sanitize legacy Rule dynamic_form_fields choice labels

migrations_lockfile.txt

@@ -31,7 +31,7 @@ replays: 0007_organizationmember_replay_access
 
 seer: 0017_drop_old_fk_columns
 
-sentry: 1100_add_relocation_file_bucket_path
+sentry: 1101_sanitize_rule_dynamic_form_field_choices
 
 social_auth: 0003_social_auth_json_field
 

src/sentry/migrations/1101_sanitize_rule_dynamic_form_field_choices.py

@@ -0,0 +1,156 @@
+import logging
+
+from django.db import migrations
+from django.db.backends.base.schema import BaseDatabaseSchemaEditor
+from django.db.migrations.state import StateApps
+
+from sentry.new_migrations.migrations import CheckedMigration
+from sentry.utils.query import RangeQuerySetWrapper
+
+logger = logging.getLogger(__name__)
+
+# The legacy issue-alert ticket-create actions whose `data.dynamic_form_fields`
+# can hold corrupted choice labels left behind by the old AbstractExternalIssueForm.
+# Mirrors the action types covered by workflow_engine migration 0114.
+TICKET_ACTION_IDS = frozenset(
+    [
+        "sentry.integrations.github.notify_action.GitHubCreateTicketAction",
+        "sentry.integrations.github_enterprise.notify_action.GitHubEnterpriseCreateTicketAction",
+        "sentry.integrations.jira.notify_action.JiraCreateTicketAction",
+        "sentry.integrations.jira_server.notify_action.JiraServerCreateTicketAction",
+        "sentry.integrations.vsts.notify_action.AzureDevopsCreateTicketAction",
+    ]
+)
+
+
+def _extract_choice_label(value: object, label: object) -> tuple[object, bool]:
+    """
+    The legacy frontend persisted React elements as choice labels. JSON
+    serialization stripped $$typeof (Symbol) and type (function), leaving
+    plain {key, ref, props} shells which the new form components reject.
+
+    Recover the readable text from props.children when possible; otherwise
+    fall back to the choice value so the row at least renders.
+
+    All serialized React elements look like:
+
+    ```
+    {
+        "key": None,
+        "ref": None,
+        "props": {
+            "children": [
+                {...},
+                " ",
+                "This is the actual label text",
+            ]
+        }
+    }
+    ```
+    """
+    if isinstance(label, dict) and "props" in label:
+        props = label.get("props")
+        children = props.get("children") if isinstance(props, dict) else None
+        if isinstance(children, list):
+            text = "".join(c for c in children if isinstance(c, str)).strip()
+            if text:
+                return text, True
+
+        return value, True
+
+    return label, False
+
+
+def sanitize_rule_dynamic_form_field_choices(
+    apps: StateApps, schema_editor: BaseDatabaseSchemaEditor
+) -> None:
+    Rule = apps.get_model("sentry", "Rule")
+
+    count = 0
+    updated = 0
+
+    for rule in RangeQuerySetWrapper(Rule.objects.all()):
+        count += 1
+
+        actions = (rule.data or {}).get("actions")
+        if not isinstance(actions, list):
+            continue
+
+        row_changed = False
+
+        for action in actions:
+            if not isinstance(action, dict):
+                continue
+            if action.get("id") not in TICKET_ACTION_IDS:
+                continue
+
+            dynamic_form_fields = action.get("dynamic_form_fields")
+            if not isinstance(dynamic_form_fields, list):
+                continue
+
+            for field_def in dynamic_form_fields:
+                if not isinstance(field_def, dict):
+                    continue
+
+                choices = field_def.get("choices")
+                if not isinstance(choices, list):
+                    continue
+
+                for choice in choices:
+                    if not isinstance(choice, list) or len(choice) < 2:
+                        continue
+
+                    new_label, was_changed = _extract_choice_label(choice[0], choice[1])
+                    if was_changed:
+                        choice[1] = new_label
+                        row_changed = True
+
+        if row_changed:
+            rule.save(update_fields=["data"])
+            updated += 1
+
+        if count % 1000 == 0:
+            logger.info(
+                "sanitize_rule_dynamic_form_field_choices.progress",
+                extra={
+                    "count": count,
+                    "updated": updated,
+                    "current_rule_id": rule.id,
+                },
+            )
+
+    logger.info(
+        "sanitize_rule_dynamic_form_field_choices.complete",
+        extra={
+            "count": count,
+            "updated": updated,
+        },
+    )
+
+
+class Migration(CheckedMigration):
+    # This flag is used to mark that a migration shouldn't be automatically run in production.
+    # This should only be used for operations where it's safe to run the migration after your
+    # code has deployed. So this should not be used for most operations that alter the schema
+    # of a table.
+    # Here are some things that make sense to mark as post deployment:
+    # - Large data migrations. Typically we want these to be run manually so that they can be
+    #   monitored and not block the deploy for a long period of time while they run.
+    # - Adding indexes to large tables. Since this can take a long time, we'd generally prefer to
+    #   run this outside deployments so that we don't block them. Note that while adding an index
+    #   is a schema change, it's completely safe to run the operation after the code has deployed.
+    # Once deployed, run these manually via: https://develop.sentry.dev/database-migrations/#migration-deployment
+
+    is_post_deployment = True
+
+    dependencies = [
+        ("sentry", "1100_add_relocation_file_bucket_path"),
+    ]
+
+    operations = [
+        migrations.RunPython(
+            code=sanitize_rule_dynamic_form_field_choices,
+            reverse_code=migrations.RunPython.noop,
+            hints={"tables": ["sentry_rule"]},
+        ),
+    ]

tests/sentry/migrations/test_1101_sanitize_rule_dynamic_form_field_choices.py

@@ -0,0 +1,160 @@
+import pytest
+
+from sentry.models.rule import Rule
+from sentry.testutils.cases import TestMigrations
+
+JIRA_ACTION_ID = "sentry.integrations.jira.notify_action.JiraCreateTicketAction"
+EMAIL_ACTION_ID = "sentry.mail.actions.NotifyEmailAction"
+
+
+@pytest.mark.skip
+class TestSanitizeRuleDynamicFormFieldChoices(TestMigrations):
+    migrate_from = "1100_add_relocation_file_bucket_path"
+    migrate_to = "1101_sanitize_rule_dynamic_form_field_choices"
+    app = "sentry"
+
+    def setup_initial_state(self) -> None:
+        project = self.create_project(organization=self.create_organization())
+
+        # Recoverable: the carcass has string children we can extract ("Jane Doe").
+        # The second choice is a clean tuple — proves we don't touch healthy rows.
+        self.corrupted_rule = Rule.objects.create(
+            project_id=project.id,
+            label="corrupted-jira-rule",
+            data={
+                "actions": [
+                    {
+                        "id": JIRA_ACTION_ID,
+                        "dynamic_form_fields": [
+                            {
+                                "name": "reporter",
+                                "choices": [
+                                    [
+                                        "user-123",
+                                        {
+                                            "key": None,
+                                            "ref": None,
+                                            "props": {
+                                                "children": [
+                                                    {
+                                                        "key": None,
+                                                        "ref": None,
+                                                        "props": {"size": "xs", "title": "tooltip"},
+                                                    },
+                                                    " ",
+                                                    "Jane Doe",
+                                                ],
+                                            },
+                                        },
+                                    ],
+                                    ["user-456", "Already Clean"],
+                                ],
+                            }
+                        ],
+                    }
+                ],
+            },
+        )
+
+        # Carcass with no string children — recovery fails, helper falls back to value.
+        self.unrecoverable_rule = Rule.objects.create(
+            project_id=project.id,
+            label="unrecoverable-jira-rule",
+            data={
+                "actions": [
+                    {
+                        "id": JIRA_ACTION_ID,
+                        "dynamic_form_fields": [
+                            {
+                                "name": "reporter",
+                                "choices": [
+                                    [
+                                        "user-789",
+                                        {"key": None, "ref": None, "props": {"children": []}},
+                                    ],
+                                ],
+                            }
+                        ],
+                    }
+                ],
+            },
+        )
+
+        # Already-clean ticket action — must be untouched.
+        self.clean_rule = Rule.objects.create(
+            project_id=project.id,
+            label="clean-jira-rule",
+            data={
+                "actions": [
+                    {
+                        "id": JIRA_ACTION_ID,
+                        "dynamic_form_fields": [
+                            {
+                                "name": "assignee",
+                                "choices": [["user-1", "Alice"], ["user-2", "Bob"]],
+                            }
+                        ],
+                    }
+                ],
+            },
+        )
+
+        # Non-ticket action whose label coincidentally looks like a carcass.
+        # Proves the action-id scoping prevents collateral damage.
+        self.non_ticket_rule = Rule.objects.create(
+            project_id=project.id,
+            label="email-rule-with-suspicious-shape",
+            data={
+                "actions": [
+                    {
+                        "id": EMAIL_ACTION_ID,
+                        "dynamic_form_fields": [
+                            {
+                                "name": "whatever",
+                                "choices": [
+                                    [
+                                        "x",
+                                        {
+                                            "key": None,
+                                            "ref": None,
+                                            "props": {"children": ["leave me alone"]},
+                                        },
+                                    ],
+                                ],
+                            }
+                        ],
+                    }
+                ],
+            },
+        )
+
+        # Rule whose data has no actions key at all — migration must not crash.
+        self.no_actions_rule = Rule.objects.create(
+            project_id=project.id,
+            label="rule-with-no-actions",
+            data={"conditions": []},
+        )
+
+    def _get_choices(self, rule_id: int) -> list:
+        rule = Rule.objects.get(id=rule_id)
+        return rule.data["actions"][0]["dynamic_form_fields"][0]["choices"]
+
+    def test_recovers_text_from_react_element_carcass(self) -> None:
+        assert self._get_choices(self.corrupted_rule.id)[0] == ["user-123", "Jane Doe"]
+
+    def test_preserves_clean_tuple_within_corrupted_rule(self) -> None:
+        assert self._get_choices(self.corrupted_rule.id)[1] == ["user-456", "Already Clean"]
+
+    def test_falls_back_to_value_when_unrecoverable(self) -> None:
+        assert self._get_choices(self.unrecoverable_rule.id)[0] == ["user-789", "user-789"]
+
+    def test_does_not_touch_clean_rule(self) -> None:
+        assert self._get_choices(self.clean_rule.id) == [["user-1", "Alice"], ["user-2", "Bob"]]
+
+    def test_does_not_touch_non_ticket_actions(self) -> None:
+        # The label is still the original dict — action-id filter held.
+        assert isinstance(self._get_choices(self.non_ticket_rule.id)[0][1], dict)
+
+    def test_skips_rules_without_actions(self) -> None:
+        rule = Rule.objects.get(id=self.no_actions_rule.id)
+        assert "actions" not in rule.data