mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-30 03:00:41 -04:00
fix: add user ownership validation to create_auto_approval_record
- Add defense-in-depth check that graph_exec_id belongs to user_id - Validates ownership before creating auto-approval records - Prevents potential misuse if function called from other contexts - Addresses CodeRabbit security concern (comment 2718990979)
This commit is contained in:
@@ -25,7 +25,7 @@ logger = logging.getLogger(__name__)
|
||||
# Context variable to track errors logged in the current task/operation
|
||||
# This prevents spamming the same error multiple times when processing batches
|
||||
_logged_errors: contextvars.ContextVar[set[str]] = contextvars.ContextVar(
|
||||
"_logged_errors", default=set()
|
||||
"_logged_errors"
|
||||
)
|
||||
|
||||
# OpenAI embedding model configuration
|
||||
@@ -56,7 +56,12 @@ def log_once_per_task(error_key: str, log_fn, message: str, **kwargs) -> bool:
|
||||
Example:
|
||||
log_once_per_task("missing_api_key", logger.error, "API key not set")
|
||||
"""
|
||||
logged = _logged_errors.get()
|
||||
# Get current logged errors, or create a new set if this is the first call in this context
|
||||
logged = _logged_errors.get(None)
|
||||
if logged is None:
|
||||
logged = set()
|
||||
_logged_errors.set(logged)
|
||||
|
||||
if error_key in logged:
|
||||
return False
|
||||
|
||||
@@ -65,7 +70,6 @@ def log_once_per_task(error_key: str, log_fn, message: str, **kwargs) -> bool:
|
||||
|
||||
# Mark as logged
|
||||
logged.add(error_key)
|
||||
_logged_errors.set(logged)
|
||||
return True
|
||||
|
||||
|
||||
|
||||
@@ -17,6 +17,7 @@ from backend.api.features.executions.review.model import (
|
||||
PendingHumanReviewModel,
|
||||
SafeJsonData,
|
||||
)
|
||||
from backend.data.execution import get_graph_execution_meta
|
||||
from backend.util.json import SafeJson
|
||||
|
||||
logger = logging.getLogger(__name__)
|
||||
@@ -107,7 +108,19 @@ async def create_auto_approval_record(
|
||||
|
||||
This is stored as a PendingHumanReview with a special nodeExecId pattern
|
||||
and status=APPROVED, so future executions of the same node can skip review.
|
||||
|
||||
Raises:
|
||||
ValueError: If the graph execution doesn't belong to the user
|
||||
"""
|
||||
# Validate that the graph execution belongs to this user (defense in depth)
|
||||
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}"
|
||||
)
|
||||
|
||||
auto_approve_key = get_auto_approve_key(graph_exec_id, node_id)
|
||||
|
||||
await PendingHumanReview.prisma().upsert(
|
||||
|
||||
Reference in New Issue
Block a user