mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
fix(backend): add user-scoped validation to cancel_pending_reviews_for_execution
- Add user_id parameter to validate ownership before cancelling reviews - Update call site in executor/utils.py to pass user_id - Update all test assertions to expect user_id parameter - Prevents cross-tenant cancellation if graph_exec_id is misused
This commit is contained in:
@@ -393,7 +393,9 @@ async def update_review_processed_status(node_exec_id: str, processed: bool) ->
|
||||
)
|
||||
|
||||
|
||||
async def cancel_pending_reviews_for_execution(graph_exec_id: str) -> int:
|
||||
async def cancel_pending_reviews_for_execution(
|
||||
graph_exec_id: str, user_id: str
|
||||
) -> int:
|
||||
"""
|
||||
Cancel all pending reviews for a graph execution (e.g., when execution is stopped).
|
||||
|
||||
@@ -401,13 +403,27 @@ async def cancel_pending_reviews_for_execution(graph_exec_id: str) -> int:
|
||||
|
||||
Args:
|
||||
graph_exec_id: The graph execution ID
|
||||
user_id: User ID who owns the execution (for security validation)
|
||||
|
||||
Returns:
|
||||
Number of reviews cancelled
|
||||
|
||||
Raises:
|
||||
ValueError: If the graph execution doesn't belong to the user
|
||||
"""
|
||||
# Validate user ownership before cancelling reviews
|
||||
graph_exec = await get_graph_execution_meta(
|
||||
user_id=user_id, execution_id=graph_exec_id
|
||||
)
|
||||
if not graph_exec:
|
||||
raise ValueError(
|
||||
f"Graph execution {graph_exec_id} not found or doesn't belong to user {user_id}"
|
||||
)
|
||||
|
||||
result = await PendingHumanReview.prisma().update_many(
|
||||
where={
|
||||
"graphExecId": graph_exec_id,
|
||||
"userId": user_id,
|
||||
"status": ReviewStatus.WAITING,
|
||||
},
|
||||
data={
|
||||
|
||||
@@ -765,7 +765,7 @@ async def stop_graph_execution(
|
||||
)
|
||||
# Mark all pending reviews as rejected/cancelled
|
||||
cancelled_count = await review_db.cancel_pending_reviews_for_execution(
|
||||
graph_exec_id
|
||||
graph_exec_id, user_id
|
||||
)
|
||||
logger.info(
|
||||
f"Cancelled {cancelled_count} pending review(s) for stopped execution {graph_exec_id}"
|
||||
|
||||
@@ -729,7 +729,7 @@ async def test_stop_graph_execution_in_review_status_cancels_pending_reviews(
|
||||
|
||||
# Verify pending reviews were cancelled
|
||||
mock_human_review_db.cancel_pending_reviews_for_execution.assert_called_once_with(
|
||||
graph_exec_id
|
||||
graph_exec_id, user_id
|
||||
)
|
||||
|
||||
# Verify execution status was updated to TERMINATED
|
||||
@@ -800,7 +800,7 @@ async def test_stop_graph_execution_with_database_manager_when_prisma_disconnect
|
||||
|
||||
# Verify database manager was used for cancel_pending_reviews
|
||||
mock_db_manager.cancel_pending_reviews_for_execution.assert_called_once_with(
|
||||
graph_exec_id
|
||||
graph_exec_id, user_id
|
||||
)
|
||||
|
||||
# Verify execution status was updated via database manager
|
||||
@@ -894,7 +894,7 @@ async def test_stop_graph_execution_cascades_to_child_with_reviews(
|
||||
|
||||
# Verify child reviews were cancelled
|
||||
mock_human_review_db.cancel_pending_reviews_for_execution.assert_called_once_with(
|
||||
child_exec_id
|
||||
child_exec_id, user_id
|
||||
)
|
||||
|
||||
# Verify both parent and child status updates
|
||||
|
||||
Reference in New Issue
Block a user