Skip to content

feat(ACI): Make ProjectRuleDetailsEndpoint GET method backwards compatible #109387

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

Merged
ceorourke merged 7 commits into from
Mar 2, 2026
Merged

feat(ACI): Make ProjectRuleDetailsEndpoint GET method backwards compatible #109387

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
d25557d
feat(ACI): Use WorkflowEngineRuleSerializer for ProjectRuleDetailsEnd…
ceorourke Feb 24, 2026
1b8e68f
Attempt to make it work for single written workflows
ceorourke Feb 25, 2026
75b7dce
update test, use fake id, move workflow lookup logic
ceorourke Feb 25, 2026
c13dc8b
new convert_args
ceorourke Feb 25, 2026
b057ce8
oops that wasn't supposed to change
ceorourke Feb 25, 2026
1657dca
filter to active workflow, catch arw & wf not existing
ceorourke Feb 25, 2026
7b50880
prevent from delete method for now too
ceorourke Mar 2, 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
44 changes: 44 additions & 0 deletions src/sentry/api/bases/rule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,15 @@

from rest_framework.request import Request

from sentry import features
from sentry.api.api_owners import ApiOwner
from sentry.api.bases import ProjectAlertRulePermission, ProjectEndpoint
from sentry.api.exceptions import ResourceDoesNotExist
from sentry.constants import ObjectStatus
from sentry.incidents.endpoints.serializers.utils import get_object_id_from_fake_id
from sentry.models.rule import Rule
from sentry.workflow_engine.models.alertrule_workflow import AlertRuleWorkflow
from sentry.workflow_engine.models.workflow import Workflow
from sentry.workflow_engine.utils.legacy_metric_tracking import report_used_legacy_models


Expand Down Expand Up @@ -34,3 +39,42 @@
raise ResourceDoesNotExist

return args, kwargs


class WorkflowEngineRuleEndpoint(RuleEndpoint):
Copy link
Copy Markdown
Member Author

@ceorourke ceorourke Feb 25, 2026

Choose a reason for hiding this comment

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

I had to make a new class here because RuleEndpoint encompasses ProjectRuleGroupHistoryIndexEndpoint and ProjectRuleStatsIndexEndpoint as well which we don't want to affect.

def convert_args(
self, request: Request, rule_id: str, *args: Any, **kwargs: Any
) -> tuple[Any, Any]:
args, kwargs = super(RuleEndpoint, self).convert_args(request, *args, **kwargs)
project = kwargs["project"]

if not rule_id.isdigit():
raise ResourceDoesNotExist

if features.has("organizations:workflow-engine-rule-serializers", project.organization):
try:
arw = AlertRuleWorkflow.objects.get(
rule_id=rule_id, workflow__organization=project.organization
)
kwargs["rule"] = arw.workflow

Check warning on line 59 in src/sentry/api/bases/rule.py

View check run for this annotation

@sentry/warden / warden: sentry-security

Cross-project IDOR: Workflow lookup scoped by organization instead of project 

The new `WorkflowEngineRuleEndpoint.convert_args` method only scopes the AlertRuleWorkflow/Workflow lookup by organization (`workflow__organization=project.organization`), while the original `RuleEndpoint` properly scopes Rule lookups by project (`project=project`). This allows a user with access to project A to access workflows from project B by using project A's URL with a `rule_id` that belongs to project B. The `get()` method then serializes and returns the workflow's sensitive configuration including conditions, filters, actions, and associated project information.
Comment thread
sentry-warden[bot] marked this conversation as resolved.
except AlertRuleWorkflow.DoesNotExist:
# XXX: this means the workflow was single written and has no ARW or related Rule object
try:
workflow_id = get_object_id_from_fake_id(int(rule_id))
Copy link
Copy Markdown
Member Author

@ceorourke ceorourke Feb 25, 2026

Choose a reason for hiding this comment

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

We assume we are being passed a "fake id" here because for singly written rules we will not have a rule id, so the rule index endpoint will generate a fake id for a workflow so it can be looked up here.

kwargs["rule"] = Workflow.objects.get(
id=workflow_id,
organization=project.organization,
status=ObjectStatus.ACTIVE,
)
except (AlertRuleWorkflow.DoesNotExist, Workflow.DoesNotExist):
Comment thread
ceorourke marked this conversation as resolved.
raise ResourceDoesNotExist
Comment thread
cursor[bot] marked this conversation as resolved.

return args, kwargs
Copy link
Copy Markdown
Contributor

@cursor cursor Bot Feb 25, 2026

Choose a reason for hiding this comment

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

Legacy rules return 404 when feature flag enabled

High Severity

When organizations:workflow-engine-rule-serializers is enabled, WorkflowEngineRuleEndpoint.convert_args returns 404 for legacy rules. If AlertRuleWorkflow lookup fails, it only tries Workflow via get_object_id_from_fake_id. When that also fails (e.g. for a legacy Rule id), it raises ResourceDoesNotExist instead of falling back to the legacy Rule lookup, breaking backwards compatibility.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Member Author

@ceorourke ceorourke Feb 25, 2026

Choose a reason for hiding this comment

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

That's intentional

Comment thread
ceorourke marked this conversation as resolved.

report_used_legacy_models()
try:
kwargs["rule"] = Rule.objects.get(project=project, id=rule_id)
except Rule.DoesNotExist:
raise ResourceDoesNotExist

return args, kwargs
60 changes: 46 additions & 14 deletions src/sentry/api/endpoints/project_rule_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@
from rest_framework.request import Request
from rest_framework.response import Response

from sentry import analytics, audit_log
from sentry import analytics, audit_log, features
from sentry.analytics.events.rule_disable_opt_out import (
RuleDisableOptOutEdit,
RuleDisableOptOutExplicit,
)
from sentry.analytics.events.rule_reenable import RuleReenableEdit
from sentry.api.api_publish_status import ApiPublishStatus
from sentry.api.base import region_silo_endpoint
from sentry.api.bases.rule import RuleEndpoint
from sentry.api.bases.rule import WorkflowEngineRuleEndpoint
from sentry.api.endpoints.project_rules import find_duplicate_rule
from sentry.api.fields.actor import OwnerActorField
from sentry.api.serializers import serialize
from sentry.api.serializers.models.rule import RuleSerializer
from sentry.api.serializers.models.rule import RuleSerializer, WorkflowEngineRuleSerializer
from sentry.api.serializers.rest_framework.rule import RuleNodeField
from sentry.api.serializers.rest_framework.rule import RuleSerializer as DrfRuleSerializer
from sentry.apidocs.constants import (
Expand All @@ -38,12 +38,14 @@
from sentry.integrations.jira_server.actions.create_ticket import JiraServerCreateTicketAction
from sentry.integrations.slack.tasks.find_channel_id_for_rule import find_channel_id_for_rule
from sentry.integrations.slack.utils.rule_status import RedisRuleStatus
from sentry.models.project import Project
from sentry.models.rule import NeglectedRule, Rule, RuleActivity, RuleActivityType
from sentry.projects.project_rules.updater import ProjectRuleUpdater
from sentry.rules.actions import trigger_sentry_app_action_creators_for_issues
from sentry.sentry_apps.utils.errors import SentryAppBaseError
from sentry.signals import alert_rule_edited
from sentry.types.actor import Actor
from sentry.workflow_engine.models.workflow import Workflow
from sentry.workflow_engine.utils.legacy_metric_tracking import (
report_used_legacy_models,
track_alert_endpoint_execution,
Expand Down Expand Up @@ -99,7 +101,7 @@ class ProjectRuleDetailsPutSerializer(serializers.Serializer):

@extend_schema(tags=["Alerts"])
@region_silo_endpoint
class ProjectRuleDetailsEndpoint(RuleEndpoint):
class ProjectRuleDetailsEndpoint(WorkflowEngineRuleEndpoint):
publish_status = {
"DELETE": ApiPublishStatus.PUBLIC,
"GET": ApiPublishStatus.PUBLIC,
Expand All @@ -122,7 +124,7 @@ class ProjectRuleDetailsEndpoint(RuleEndpoint):
examples=IssueAlertExamples.GET_PROJECT_RULE,
)
@track_alert_endpoint_execution("GET", "sentry-api-0-project-rule-details")
def get(self, request: Request, project, rule) -> Response:
def get(self, request: Request, project: Project, rule: Rule | Workflow) -> Response:
"""
## Deprecated
🚧 Use [Fetch an Alert](/api/monitors/fetch-an-alert) instead.
Expand All @@ -135,13 +137,23 @@ def get(self, request: Request, project, rule) -> Response:
- Filters - help control noise by triggering an alert only if the issue matches the specified criteria.
- Actions - specify what should happen when the trigger conditions are met and the filters match.
"""
# Serialize Rule object
rule_serializer = RuleSerializer(
expand=request.GET.getlist("expand", []),
prepare_component_fields=True,
project_slug=project.slug,
)
serialized_rule = serialize(rule, request.user, rule_serializer)
if features.has("organizations:workflow-engine-rule-serializers", project.organization):
workflow_engine_rule_serializer = WorkflowEngineRuleSerializer(
expand=request.GET.getlist("expand", []),
prepare_component_fields=True,
project_slug=project.slug,
)
# XXX: Note 'rule' here is actually a workflow object
serialized_rule = serialize(rule, request.user, workflow_engine_rule_serializer)
else:
# Serialize Rule object
rule_serializer = RuleSerializer(
expand=request.GET.getlist("expand", []),
prepare_component_fields=True,
project_slug=project.slug,
)
serialized_rule = serialize(rule, request.user, rule_serializer)
Copy link
Copy Markdown
Contributor

@cursor cursor Bot Feb 25, 2026

Choose a reason for hiding this comment

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

GET uses wrong serializer when legacy Rule returned

High Severity

The GET handler selects the serializer using only the feature flag. If convert_args is fixed to fall back to legacy Rule when both AlertRuleWorkflow and Workflow lookups fail, the handler would pass a Rule to WorkflowEngineRuleSerializer, which expects Workflow objects. That would cause get_attrs and serialize to fail (e.g. KeyError on attrs["rule_id"]). Serializer selection should branch on isinstance(rule, Workflow) so legacy rules use RuleSerializer.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Member Author

@ceorourke ceorourke Feb 25, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

If convert_args is fixed to fall back to legacy Rule when both AlertRuleWorkflow and Workflow lookups fail

idk why you'd think that would happen


Comment thread
cursor[bot] marked this conversation as resolved.
# Prepare Rule Actions that are SentryApp components using the meta fields
for action in serialized_rule.get("actions", []):
# TODO(nisanthan): This is a temporary fix. We need to save both the label and value of
Expand Down Expand Up @@ -174,7 +186,7 @@ def get(self, request: Request, project, rule) -> Response:
examples=IssueAlertExamples.UPDATE_PROJECT_RULE,
)
@track_alert_endpoint_execution("PUT", "sentry-api-0-project-rule-details")
def put(self, request: Request, project, rule) -> Response:
def put(self, request: Request, project: Project, rule: Rule | Workflow) -> Response:
"""
## Deprecated
🚧 Use [Update an Alert by ID](/api/monitors/update-an-alert-by-id) instead.
Expand All @@ -188,6 +200,16 @@ def put(self, request: Request, project, rule) -> Response:
- Filters - help control noise by triggering an alert only if the issue matches the specified criteria.
- Actions - specify what should happen when the trigger conditions are met and the filters match.
"""
if isinstance(rule, Workflow):
return Response(
{
"rule": [
"Passing a workflow through this endpoint is not yet supported",
]
},
status=status.HTTP_400_BAD_REQUEST,
)

rule_data_before = dict(rule.data)
if rule.environment_id:
rule_data_before["environment_id"] = rule.environment_id
Expand Down Expand Up @@ -383,7 +405,7 @@ def put(self, request: Request, project, rule) -> Response:
},
)
@track_alert_endpoint_execution("DELETE", "sentry-api-0-project-rule-details")
def delete(self, request: Request, project, rule) -> Response:
def delete(self, request: Request, project: Project, rule: Rule | Workflow) -> Response:
"""
## Deprecated
🚧 Use [Delete an Alert](/api/monitors/delete-an-alert) instead.
Expand All @@ -396,6 +418,16 @@ def delete(self, request: Request, project, rule) -> Response:
- Filters: help control noise by triggering an alert only if the issue matches the specified criteria.
- Actions: specify what should happen when the trigger conditions are met and the filters match.
"""
if isinstance(rule, Workflow):
return Response(
{
"rule": [
"Passing a workflow through this endpoint is not yet supported",
]
},
status=status.HTTP_400_BAD_REQUEST,
)

report_used_legacy_models()
with transaction.atomic(router.db_for_write(Rule)):
rule.update(status=ObjectStatus.PENDING_DELETION)
Expand Down
6 changes: 4 additions & 2 deletions src/sentry/api/serializers/models/rule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,7 @@ def _fetch_workflow_last_triggered(self, item_list: Sequence[Workflow]) -> dict[
return {wf_id: date for wf_id, date in results.items() if date is not None}

def get_attrs(self, item_list: Sequence[Workflow], user, **kwargs):
from sentry.incidents.endpoints.serializers.utils import get_fake_id_from_object_id
from sentry.notifications.notification_action.registry import issue_alert_handler_registry

# Bulk fetch users that created workflows
Expand Down Expand Up @@ -477,7 +478,9 @@ def get_attrs(self, item_list: Sequence[Workflow], user, **kwargs):

result[workflow]["environment"] = workflow.environment
result[workflow]["projects"] = list(workflow_to_projects[workflow])
result[workflow]["rule_id"] = workflow_rule_ids[workflow.id]
result[workflow]["rule_id"] = workflow_rule_ids.get(
workflow.id, get_fake_id_from_object_id(workflow.id)
)
Copy link
Copy Markdown
Contributor

@cursor cursor Bot Feb 25, 2026

Choose a reason for hiding this comment

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

IndexError when workflow has no WorkflowDataConditionGroup

Medium Severity

WorkflowEngineRuleSerializer.get_attrs assumes every workflow has at least one WorkflowDataConditionGroup and does workflow.prefetched_wdcgs[0] without checking. A workflow with no WorkflowDataConditionGroup raises IndexError during serialization.

Fix in Cursor Fix in Web

Copy link
Copy Markdown
Member Author

@ceorourke ceorourke Feb 25, 2026

Choose a reason for hiding this comment

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

I think it'll always have one


result[workflow]["action_match"] = (
workflow.when_condition_group.logic_type if workflow.when_condition_group else None
Copy link
Copy Markdown
Contributor

@sentry sentry Bot Mar 2, 2026

Choose a reason for hiding this comment

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

Bug: The code accesses prefetched_wdcgs[0] without checking if the list is empty, which can cause an IndexError when serializing a workflow with no action filters.
Severity: HIGH

Suggested Fix

Add a conditional check to ensure workflow.prefetched_wdcgs is not empty before accessing its first element, for example: if workflow.prefetched_wdcgs:. Alternatively, consider making action_filters a required field in the validator to prevent this data state from occurring.

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/api/serializers/models/rule.py#L486

Potential issue: In `WorkflowEngineRuleSerializer.get_attrs()`, the code directly
accesses the first element of `workflow.prefetched_wdcgs`. However, the
`WorkflowValidator` allows for the creation of workflows without `action_filters`, which
results in `prefetched_wdcgs` being an empty list. When the serializer attempts to
process such a workflow, accessing `workflow.prefetched_wdcgs[0]` will raise an
`IndexError`, causing the API endpoint for retrieving the rule to return a 500 error.

Expand Down Expand Up @@ -521,7 +524,6 @@ def get_attrs(self, item_list: Sequence[Workflow], user, **kwargs):
conditions, filters = self._generate_rule_conditions_filters(
workflow, result[workflow]["projects"][0], workflow_dcg
)

result[workflow]["conditions"] = conditions
result[workflow]["filters"] = filters

Expand Down
67 changes: 64 additions & 3 deletions tests/sentry/api/endpoints/test_project_rule_details.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from sentry.analytics.events.rule_reenable import RuleReenableEdit
from sentry.constants import ObjectStatus
from sentry.deletions.tasks.scheduled import run_scheduled_deletions
from sentry.incidents.endpoints.serializers.utils import get_fake_id_from_object_id
from sentry.integrations.slack.utils.channel import strip_channel_name
from sentry.models.environment import Environment
from sentry.models.rule import NeglectedRule, Rule, RuleActivity, RuleActivityType
Expand All @@ -24,16 +25,17 @@
from sentry.sentry_apps.utils.errors import SentryAppErrorType
from sentry.silo.base import SiloMode
from sentry.testutils.cases import APITestCase
from sentry.testutils.helpers import install_slack
from sentry.testutils.helpers import install_slack, with_feature
from sentry.testutils.helpers.analytics import assert_any_analytics_event
from sentry.testutils.helpers.datetime import freeze_time
from sentry.testutils.silo import assume_test_silo_mode
from sentry.types.actor import Actor
from sentry.workflow_engine.models import AlertRuleWorkflow
from sentry.workflow_engine.models.data_condition import DataCondition
from sentry.workflow_engine.models.data_condition import Condition, DataCondition
from sentry.workflow_engine.models.data_condition_group import DataConditionGroup
from sentry.workflow_engine.models.workflow import Workflow
from sentry.workflow_engine.models.workflow_data_condition_group import WorkflowDataConditionGroup
from tests.sentry.workflow_engine.test_base import BaseWorkflowTest


def assert_rule_from_payload(rule: Rule, payload: Mapping[str, Any]) -> None:
Expand Down Expand Up @@ -92,7 +94,7 @@ def assert_rule_from_payload(rule: Rule, payload: Mapping[str, Any]) -> None:
assert RuleActivity.objects.filter(rule=rule, type=RuleActivityType.UPDATED.value).exists()


class ProjectRuleDetailsBaseTestCase(APITestCase):
class ProjectRuleDetailsBaseTestCase(APITestCase, BaseWorkflowTest):
endpoint = "sentry-api-0-project-rule-details"

def setUp(self) -> None:
Expand Down Expand Up @@ -134,6 +136,34 @@ def setUp(self) -> None:
"id": "sentry.rules.conditions.first_seen_event.FirstSeenEventCondition",
}
]
# create single written workflow
self.detector = self.create_detector()
self.workflow_triggers = self.create_data_condition_group()
self.workflow = self.create_workflow(
when_condition_group=self.workflow_triggers,
organization=self.detector.project.organization,
)
self.detector_workflow = self.create_detector_workflow(
detector=self.detector, workflow=self.workflow
)
self.create_data_condition( # trigger condition
condition_group=self.workflow_triggers,
type=Condition.EVENT_FREQUENCY_COUNT,
comparison={"interval": "1d", "value": 100},
condition_result=True,
)
self.workflow_filters = self.create_data_condition_group()
self.workflow_dcg = self.create_workflow_data_condition_group(
workflow=self.workflow, condition_group=self.workflow_filters
)
self.create_data_condition( # filter condition
condition_group=self.workflow_filters,
type=Condition.EVENT_ATTRIBUTE,
comparison={"attribute": "platform", "match": "eq", "value": "python"},
condition_result=True,
)
self.action_group, self.action = self.create_workflow_action(self.workflow)
self.fake_workflow_id = get_fake_id_from_object_id(self.workflow.id)


class ProjectRuleDetailsTest(ProjectRuleDetailsBaseTestCase):
Expand All @@ -145,6 +175,25 @@ def test_simple(self) -> None:
assert response.data["environment"] is None
assert response.data["conditions"][0]["name"]

@with_feature("organizations:workflow-engine-rule-serializers")
def test_workflow_engine_serializer_dual_written_rule(self) -> None:
response = self.get_success_response(
self.organization.slug, self.project.slug, self.rule.id, status_code=200
)
assert response.data["id"] == str(self.rule.id)
assert response.data["environment"] is None
assert response.data["conditions"][0]["name"]

@with_feature("organizations:workflow-engine-rule-serializers")
def test_workflow_engine_serializer_single_written_rule(self) -> None:
response = self.get_success_response(
self.organization.slug, self.project.slug, self.fake_workflow_id, status_code=200
)
assert response.data["id"] == str(self.fake_workflow_id)
assert response.data["environment"] is None
assert response.data["conditions"][0]["name"]
assert response.data["filters"][0]["name"]

def test_non_existing_rule(self) -> None:
self.get_error_response(self.organization.slug, self.project.slug, 12345, status_code=404)

Expand Down Expand Up @@ -612,6 +661,12 @@ def test_simple(self, send_robust: MagicMock) -> None:
assert_rule_from_payload(self.rule, payload)
assert send_robust.called

@with_feature("organizations:workflow-engine-rule-serializers")
def test_workflow_passed(self) -> None:
self.get_error_response(
self.organization.slug, self.project.slug, self.fake_workflow_id, status_code=400
)

def test_no_owner(self) -> None:
conditions = [
{
Expand Down Expand Up @@ -1585,6 +1640,12 @@ def test_simple(self) -> None:
id=self.rule.id, project=self.project, status=ObjectStatus.PENDING_DELETION
).exists()

@with_feature("organizations:workflow-engine-rule-serializers")
def test_workflow_passed(self) -> None:
self.get_error_response(
self.organization.slug, self.project.slug, self.fake_workflow_id, status_code=400
)

def test_dual_delete_workflow_engine(self) -> None:
rule = self.create_project_rule(
self.project,
Expand Down
Loading