Skip to content

fix(workflow-engine): Prevent ValueError for missing ActionTarget in serializer #119119

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

Open
sentry wants to merge 3 commits into
base: master
Choose a base branch
from
+32 −2
Open

fix(workflow-engine): Prevent ValueError for missing ActionTarget in serializer #119119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8b67358
fix(serializers): Handle None for ActionTarget in workflow engine ser…
sentry[bot] Jul 7, 2026
f719f7c
fix(workflow-engine): Handle missing ActionTarget in action serializer
sentry[bot] Jul 7, 2026
f3b7c1c
fix(workflow-engine): Prevent ValueError for missing ActionTarget in …
sentry[bot] Jul 7, 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
15 changes: 13 additions & 2 deletions src/sentry/incidents/endpoints/serializers/workflow_engine_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import logging
from collections.abc import Mapping
from typing import Any, Sequence

Expand All @@ -16,8 +17,10 @@
)
from sentry.users.models.user import User
from sentry.users.services.user.model import RpcUser
from sentry.workflow_engine.models import Action, ActionAlertRuleTriggerAction

Check warning on line 20 in src/sentry/incidents/endpoints/serializers/workflow_engine_action.py

View check run for this annotation

@sentry/warden / warden: sentry-backend-bugs

TypeError when `target_identifier` is None for Sentry App action 

The PR guards against `target_type` being `None` but the same `obj.config.get(...)` pattern is used unguarded on the next line: `sentry_app_id = int(obj.config.get("target_identifier"))` will raise `TypeError: int() argument must be a string…not 'NoneType'` if `target_identifier` is absent from config for a Sentry App action. Add a `None` guard identical to the one added for `target_type`.

logger = logging.getLogger(__name__)


class WorkflowEngineActionSerializer(Serializer[dict[str, Any]]):
def get_attrs(
Expand Down Expand Up @@ -57,10 +60,16 @@
type_value: int | None = ActionService.get_value(obj.type)
target = attrs.get("target")

target_type: int = obj.config.get("target_type")
target_type: int | None = obj.config.get("target_type")
target_identifier: str | None = obj.config.get("target_identifier")
target_display: str | None = obj.config.get("target_display")

if target_type is None:
logger.warning(
"workflow_engine_action.missing_target_type",
extra={"action_id": obj.id, "action_type": obj.type},
)

sentry_app_id = None
sentry_app_config = None
if obj.type == Action.Type.SENTRY_APP.value:
Expand All @@ -75,7 +84,9 @@
),
"alertRuleTriggerId": str(alert_rule_trigger_id),
"type": obj.type,
Comment on lines 84 to 86

@sentry sentry Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bug: The serializer can crash with a ValueError when a PagerDuty or Sentry App action is missing a target_identifier, as it attempts to run int("None").
Severity: HIGH

Suggested Fix

Before calling get_identifier_from_action, check if target_identifier is None. If it is, handle it gracefully, perhaps by returning None or a default value, to prevent the str() and subsequent int() conversion from crashing when the action type is PagerDuty or Sentry App.

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/incidents/endpoints/serializers/workflow_engine_action.py#L84-L86

Potential issue: When serializing a PagerDuty or Sentry App action that is missing a
`target_identifier` in its configuration, the value defaults to `None`. This `None`
value is then cast to the string "None" and passed to the `get_identifier_from_action`
function. For PagerDuty or Sentry App action types, this function attempts to cast the
string "None" to an integer, which raises a `ValueError`. This scenario is likely for
older actions created before the field was required, and the incomplete fix in the pull
request fails to guard against it.

"targetType": ACTION_TARGET_TYPE_TO_STRING[ActionTarget(target_type)],
"targetType": ACTION_TARGET_TYPE_TO_STRING[ActionTarget(target_type)]
if target_type is not None
else None,
"targetIdentifier": get_identifier_from_action(
type_value, str(target_identifier), target_display
),
Comment on lines +88 to 92

@sentry sentry Bot Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Bug: If target_identifier is None in the config, casting it to an int will cause a TypeError or ValueError, crashing the serializer.
Severity: HIGH

Suggested Fix

Add a check to ensure target_identifier is not None before attempting to cast it to an integer. Provide a default value or handle the None case gracefully. For example, retrieve the value with target_identifier = obj.config.get("target_identifier") and then add a conditional check if target_identifier is not None: before calling int().

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/incidents/endpoints/serializers/workflow_engine_action.py#L79-L83

Potential issue: If an action's configuration in the database is missing the
`target_identifier` field, the serializer will fail. For actions of type `SENTRY_APP`,
an attempt to call `int(obj.config.get("target_identifier"))` will receive `None`,
raising a `TypeError`. For `PAGERDUTY` or `SENTRY_APP` actions, the code calls
`get_identifier_from_action` with `str(target_identifier)`. If `target_identifier` is
`None`, this becomes `int("None")`, which raises a `ValueError`. This scenario is
plausible in production, as evidenced by the similar bug for `target_type` that this
pull request aims to fix.

Did we get this right? 👍 / 👎 to inform future reviews.

Expand Down
19 changes: 19 additions & 0 deletions tests/sentry/incidents/serializers/test_workflow_engine_action.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,25 @@ def test_sentry_app_action(self) -> None:
sentry_app_expected["desc"] = f"Send a notification via {sentry_app.name}"
assert serialized_action == sentry_app_expected

def test_missing_target_type_does_not_raise(self) -> None:
"""
Regression test for: ValueError: None is not a valid ActionTarget.
Actions whose config dict lacks a 'target_type' key (e.g. actions created
before the field was required) should serialize without raising.
"""
# Mutate the migrated action's config to remove target_type, simulating
# the DB state that triggers the bug in production.
self.critical_action.config.pop("target_type", None)
self.critical_action.save()

serialized_action = serialize(
self.critical_action,
self.user,
WorkflowEngineActionSerializer(),
alert_rule_trigger_id=self.critical_trigger.id,
)
assert serialized_action["targetType"] is None

def test_slack_action(self) -> None:
self.integration = self.create_slack_integration(
self.organization,
Expand Down
Loading