mirror of
https://github.com/Significant-Gravitas/AutoGPT.git
synced 2026-04-08 03:00:28 -04:00
fix(backend): allow admins to download submitted agents pending review (#12535)
## Why
Admins cannot download submitted-but-not-yet-approved agents from
`/admin/marketplace`. Clicking "Download" fails silently with a Server
Components render error. This blocks admins from reviewing agents that
companies have submitted.
## What
Remove the redundant ownership/marketplace check from
`get_graph_as_admin()` that was silently tightened in PR #11323 (Nov
2025). Add regression tests for both the admin download path and the
non-admin marketplace access control.
## How
**Root cause:** In PR #11323, Reinier refactored an inline
`StoreListingVersion` query (which had no status filter) into a call to
`is_graph_published_in_marketplace()` (which requires `submissionStatus:
APPROVED`). This was collateral cleanup — his PR focused on sub-agent
execution permissions — but it broke admin download of pending agents.
**Fix:** Remove the ownership/marketplace check from
`get_graph_as_admin()`, keeping only the null guard. This is safe
because `get_graph_as_admin` is only callable through admin-protected
routes (`requires_admin_user` at router level).
**Tests added:**
- `test_admin_can_access_pending_agent_not_owned` — admin can access a
graph they don't own that isn't APPROVED
- `test_admin_download_pending_agent_with_subagents` — admin export
includes sub-graphs
- `test_get_graph_non_owner_approved_marketplace_agent` — protects PR
#11323: non-owners CAN access APPROVED agents
- `test_get_graph_non_owner_pending_marketplace_agent_denied` — protects
PR #11323: non-owners CANNOT access PENDING agents
### Checklist
- [x] I have clearly listed my changes in the PR description
- [x] I have made a test plan
- [x] I have tested my changes according to the test plan:
- [x] 4 regression tests pass locally
- [x] Admin can download pending agents (verified via unit test)
- [x] Non-admin marketplace access control preserved
## Test plan
- [ ] Verify admin can download a submitted-but-not-approved agent from
`/admin/marketplace`
- [ ] Verify non-admin users still cannot access admin endpoints
- [ ] Verify the download succeeds without console errors
🤖 Generated with [Claude Code](https://claude.com/claude-code)
<!-- CURSOR_SUMMARY -->
---
> [!NOTE]
> **Medium Risk**
> Changes access-control behavior for admin graph retrieval; risk is
mitigated by route-level admin auth but misuse of `get_graph_as_admin()`
outside admin-protected routes would expose non-approved graphs.
>
> **Overview**
> Admins can now download/review **submitted-but-not-approved**
marketplace agents: `get_graph_as_admin()` no longer enforces ownership
or *marketplace APPROVED* checks, only returning `None` when the graph
doesn’t exist.
>
> Adds regression tests covering the admin download/export path
(including sub-graphs) and confirming non-admin behavior is unchanged:
non-owners can fetch **APPROVED** marketplace graphs but cannot access
**pending** ones.
>
> <sup>Written by [Cursor
Bugbot](https://cursor.com/dashboard?tab=bugbot) for commit
a6d2d69ae4. This will update automatically
on new commits. Configure
[here](https://cursor.com/dashboard?tab=bugbot).</sup>
<!-- /CURSOR_SUMMARY -->
---------
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,93 @@
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.data.graph import get_graph_as_admin
|
||||
|
||||
# Shared constants
|
||||
ADMIN_USER_ID = "admin-user-id"
|
||||
CREATOR_USER_ID = "other-creator-id"
|
||||
GRAPH_ID = "test-graph-id"
|
||||
GRAPH_VERSION = 3
|
||||
|
||||
|
||||
def _make_mock_graph(user_id: str = CREATOR_USER_ID) -> MagicMock:
|
||||
graph = MagicMock()
|
||||
graph.userId = user_id
|
||||
graph.id = GRAPH_ID
|
||||
graph.version = GRAPH_VERSION
|
||||
graph.Nodes = []
|
||||
return graph
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_admin_can_access_pending_agent_not_owned() -> None:
|
||||
"""Admin must be able to access a graph they don't own even if it's not
|
||||
APPROVED in the marketplace. This is the core use case: reviewing a
|
||||
submitted-but-pending agent from the admin dashboard."""
|
||||
mock_graph = _make_mock_graph()
|
||||
mock_graph_model = MagicMock(name="GraphModel")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"backend.data.graph.AgentGraph.prisma",
|
||||
) as mock_prisma,
|
||||
patch(
|
||||
"backend.data.graph.GraphModel.from_db",
|
||||
return_value=mock_graph_model,
|
||||
),
|
||||
):
|
||||
mock_prisma.return_value.find_first = AsyncMock(return_value=mock_graph)
|
||||
|
||||
result = await get_graph_as_admin(
|
||||
graph_id=GRAPH_ID,
|
||||
version=GRAPH_VERSION,
|
||||
user_id=ADMIN_USER_ID,
|
||||
for_export=False,
|
||||
)
|
||||
|
||||
assert (
|
||||
result is not None
|
||||
), "Admin should be able to access a pending agent they don't own"
|
||||
assert result is mock_graph_model
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_admin_download_pending_agent_with_subagents() -> None:
|
||||
"""Admin export (for_export=True) of a pending agent must include
|
||||
sub-graphs. This exercises the full export code path that the Download
|
||||
button uses."""
|
||||
mock_graph = _make_mock_graph()
|
||||
mock_sub_graph = MagicMock(name="SubGraph")
|
||||
mock_graph_model = MagicMock(name="GraphModel")
|
||||
|
||||
with (
|
||||
patch(
|
||||
"backend.data.graph.AgentGraph.prisma",
|
||||
) as mock_prisma,
|
||||
patch(
|
||||
"backend.data.graph.get_sub_graphs",
|
||||
new_callable=AsyncMock,
|
||||
return_value=[mock_sub_graph],
|
||||
) as mock_get_sub,
|
||||
patch(
|
||||
"backend.data.graph.GraphModel.from_db",
|
||||
return_value=mock_graph_model,
|
||||
) as mock_from_db,
|
||||
):
|
||||
mock_prisma.return_value.find_first = AsyncMock(return_value=mock_graph)
|
||||
|
||||
result = await get_graph_as_admin(
|
||||
graph_id=GRAPH_ID,
|
||||
version=GRAPH_VERSION,
|
||||
user_id=ADMIN_USER_ID,
|
||||
for_export=True,
|
||||
)
|
||||
|
||||
assert result is not None, "Admin export of pending agent must succeed"
|
||||
mock_get_sub.assert_awaited_once_with(mock_graph)
|
||||
mock_from_db.assert_called_once_with(
|
||||
graph=mock_graph,
|
||||
sub_graphs=[mock_sub_graph],
|
||||
for_export=True,
|
||||
)
|
||||
@@ -1207,13 +1207,9 @@ async def get_graph_as_admin(
|
||||
order={"version": "desc"},
|
||||
)
|
||||
|
||||
# For access, the graph must be owned by the user or listed in the store
|
||||
if graph is None or (
|
||||
graph.userId != user_id
|
||||
and not await is_graph_published_in_marketplace(
|
||||
graph_id, version or graph.version
|
||||
)
|
||||
):
|
||||
# Admin access bypasses ownership and marketplace checks — route-level
|
||||
# auth already ensures only admins can call this function.
|
||||
if graph is None:
|
||||
return None
|
||||
|
||||
if for_export:
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import json
|
||||
from typing import Any
|
||||
from unittest.mock import AsyncMock, patch
|
||||
from unittest.mock import AsyncMock, MagicMock, patch
|
||||
from uuid import UUID
|
||||
|
||||
import fastapi.exceptions
|
||||
@@ -13,7 +13,7 @@ from backend.api.model import CreateGraph
|
||||
from backend.blocks._base import BlockSchema, BlockSchemaInput
|
||||
from backend.blocks.basic import StoreValueBlock
|
||||
from backend.blocks.io import AgentInputBlock, AgentOutputBlock
|
||||
from backend.data.graph import Graph, Link, Node
|
||||
from backend.data.graph import Graph, Link, Node, get_graph
|
||||
from backend.data.model import SchemaField
|
||||
from backend.data.user import DEFAULT_USER_ID
|
||||
from backend.usecases.sample import create_test_user
|
||||
@@ -595,3 +595,82 @@ def test_mcp_credential_combine_no_discriminator_values():
|
||||
f"Expected 1 credential entry for MCP blocks without discriminator_values, "
|
||||
f"got {len(combined)}: {list(combined.keys())}"
|
||||
)
|
||||
|
||||
|
||||
# --------------- get_graph access-control regression tests --------------- #
|
||||
# These protect the behavior introduced in PR #11323 (Reinier, 2025-11-05):
|
||||
# non-owners can access APPROVED marketplace agents but NOT pending ones.
|
||||
|
||||
|
||||
def _make_mock_db_graph(user_id: str = "owner-user-id") -> MagicMock:
|
||||
graph = MagicMock()
|
||||
graph.userId = user_id
|
||||
graph.id = "graph-id"
|
||||
graph.version = 1
|
||||
graph.Nodes = []
|
||||
return graph
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_graph_non_owner_approved_marketplace_agent() -> None:
|
||||
"""A non-owner should be able to access a graph that has an APPROVED
|
||||
marketplace listing. This is the normal marketplace download flow."""
|
||||
owner_id = "owner-user-id"
|
||||
requester_id = "different-user-id"
|
||||
graph_id = "graph-id"
|
||||
mock_graph = _make_mock_db_graph(owner_id)
|
||||
mock_graph_model = MagicMock(name="GraphModel")
|
||||
|
||||
mock_listing = MagicMock()
|
||||
mock_listing.AgentGraph = mock_graph
|
||||
|
||||
with (
|
||||
patch("backend.data.graph.AgentGraph.prisma") as mock_ag_prisma,
|
||||
patch(
|
||||
"backend.data.graph.StoreListingVersion.prisma",
|
||||
) as mock_slv_prisma,
|
||||
patch(
|
||||
"backend.data.graph.GraphModel.from_db",
|
||||
return_value=mock_graph_model,
|
||||
),
|
||||
):
|
||||
# First lookup (owned graph) returns None — requester != owner
|
||||
mock_ag_prisma.return_value.find_first = AsyncMock(return_value=None)
|
||||
# Marketplace fallback finds an APPROVED listing
|
||||
mock_slv_prisma.return_value.find_first = AsyncMock(return_value=mock_listing)
|
||||
|
||||
result = await get_graph(
|
||||
graph_id=graph_id,
|
||||
version=1,
|
||||
user_id=requester_id,
|
||||
)
|
||||
|
||||
assert result is not None, "Non-owner should access APPROVED marketplace agent"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_get_graph_non_owner_pending_marketplace_agent_denied() -> None:
|
||||
"""A non-owner must NOT be able to access a graph that only has a PENDING
|
||||
(not APPROVED) marketplace listing. The marketplace fallback filters on
|
||||
submissionStatus=APPROVED, so pending agents should be invisible."""
|
||||
requester_id = "different-user-id"
|
||||
graph_id = "graph-id"
|
||||
|
||||
with (
|
||||
patch("backend.data.graph.AgentGraph.prisma") as mock_ag_prisma,
|
||||
patch(
|
||||
"backend.data.graph.StoreListingVersion.prisma",
|
||||
) as mock_slv_prisma,
|
||||
):
|
||||
# First lookup (owned graph) returns None
|
||||
mock_ag_prisma.return_value.find_first = AsyncMock(return_value=None)
|
||||
# Marketplace fallback finds nothing (not APPROVED)
|
||||
mock_slv_prisma.return_value.find_first = AsyncMock(return_value=None)
|
||||
|
||||
result = await get_graph(
|
||||
graph_id=graph_id,
|
||||
version=1,
|
||||
user_id=requester_id,
|
||||
)
|
||||
|
||||
assert result is None, "Non-owner must not access a pending marketplace agent"
|
||||
|
||||
Reference in New Issue
Block a user