fix(ACI): Parse and persist owner field#110807
Conversation
github-actions
bot
added
the
Scope: Backend
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![sentry-warden[bot]](https://avatars.githubusercontent.com/in/2755838?s=60&v=4){kind=small}
Backend Test FailuresFailures on
|
ceorourke
force-pushed
the
ceorourke/ISWF-2237
branch
from
fd70cc7 to
178731a
Compare
src/sentry/workflow_engine/endpoints/validators/base/workflow.py
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
| if actor is not None: | ||
| workflow_payload["owner"] = actor.identifier | ||
| elif owner is None: # user has expliticly passed None so as to clear the owner field | ||
| workflow_payload["owner"] = None |
There was a problem hiding this comment.
Redundant owner validation causes duplicate DB queries
Low Severity
The parse_and_validate_actor call in format_request_data validates the owner (resolving the actor and checking org membership via DB queries), then stores actor.identifier (e.g., "user:123"). The WorkflowValidator's OwnerActorField then calls parse_and_validate_actor again on the same value, repeating the same DB queries. The OwnerActorField can handle all raw owner formats (int, string, None) directly, so the pre-processing in format_request_data is unnecessary. Additionally, a validation error raised here produces a flat error response, while one from the WorkflowValidator would be keyed under the owner field — an inconsistency.
Co-authored-by: Josh Callender <[email protected]>
| owner = data.get("owner", "") | ||
| if owner: | ||
| actor = parse_and_validate_actor(str(owner), project.organization_id) | ||
| if actor is not None: |
There was a problem hiding this comment.
Bug: The WorkflowValidator is missing the current_owner context, causing incorrect validation failures when a user reassigns rule ownership from their own team to another team.
Severity: MEDIUM
Suggested Fix
When instantiating the WorkflowValidator in the project_rule_details.py PUT endpoint, add the current_owner to the context dictionary, similar to how it is provided to the DrfRuleSerializer in the non-feature-flagged code path.
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/endpoints/project_rules.py#L759-L762
Potential issue: When updating a rule via the PUT endpoint with the
`organizations:workflow-engine-rule-serializers` feature flag enabled, the
`WorkflowValidator` is instantiated without the `current_owner` in its context. This
prevents the `OwnerActorField` from correctly validating team reassignments.
Specifically, it blocks a user from reassigning ownership from a team they are a member
of to a team they are not, as the logic to check the `current_owner` is skipped. This
results in an incorrect validation failure for a legitimate user action.
Follow up to #109926 (comment) to increase test coverage for the issue alert rule POST endpoint backwards compatibility. Adding this test coverage uncovered a few inconsistencies that are fixed in this PR as well. Note that I will need to rebase off of #110807 once it is merged.
2 participants
Follow up to #110785 and #110381 to write and update the owner on a workflow if that data is passed to the legacy endpoint and we have the feature flag flipped so that we single write.