Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
141 changes: 104 additions & 37 deletions backend/app/services/plugin_marketplace_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,15 @@ class PublishedRelease:
created: bool


@dataclass(frozen=True)
class _UserPluginAccessContext:
"""Preloaded namespace membership used by marketplace list access checks."""

namespace_ids: set[str]
namespace_names: list[str]
namespace_names_by_id: dict[str, str]


class PluginMarketplaceService:
"""Own the cloud marketplace while leaving runtime installation to Codex."""

Expand Down Expand Up @@ -304,8 +313,12 @@ def list_plugins(
if owner_ids
else {}
)
grants_by_plugin_id = self._load_grants_by_plugin_ids(
db, [plugin.id for plugin in rows]
plugin_ids = [plugin.id for plugin in rows]
grants_by_plugin_id = self._load_grants_by_plugin_ids(db, plugin_ids)
access_context = self._load_user_plugin_access_context(
db,
user_id=user_id,
grants_by_plugin_id=grants_by_plugin_id,
)
installed_kind_ids = [row.id for row in installed_by_plugin_id.values()]
device_rows_by_kind_id: dict[int, PluginDeviceInstallation] = {}
Expand All @@ -322,7 +335,13 @@ def list_plugins(

items: list[PluginMarketplaceItem] = []
for plugin in rows:
if not self._can_access_plugin(db, plugin=plugin, user_id=user_id):
if not self._can_access_plugin(
db,
plugin=plugin,
user_id=user_id,
grants=grants_by_plugin_id.get(plugin.id, []),
access_context=access_context,
):
continue
if listing_type and plugin.listing_type != listing_type:
continue
Expand Down Expand Up @@ -1880,14 +1899,22 @@ def _published_plugin(self, db, plugin_id, *, user_id: int | None = None):
return plugin

def _can_access_plugin(
self, db: Session, *, plugin: Plugin, user_id: int | None
self,
db: Session,
*,
plugin: Plugin,
user_id: int | None,
grants: list[ResourceMember] | None = None,
access_context: _UserPluginAccessContext | None = None,
) -> bool:
"""Apply optional direct-user or department grants to workspace plugins.

Args:
db: Database session
plugin: Plugin to check access for
user_id: User ID, or None for unauthenticated access
grants: Optional preloaded approved plugin grants
access_context: Optional preloaded user/namespace membership

Returns:
True if user can access plugin, False otherwise
Expand All @@ -1908,17 +1935,19 @@ def _can_access_plugin(
# Owner can always access their own plugins
if plugin.owner_user_id == user_id:
return True
plugin_type_values = (ResourceType.PLUGIN.value, ResourceType.PLUGIN.name)
approved_values = (MemberStatus.APPROVED.value, MemberStatus.APPROVED.name)
grants = (
db.query(ResourceMember)
.filter(
ResourceMember.resource_type.in_(plugin_type_values),
ResourceMember.resource_id == plugin.id,
ResourceMember.status.in_(approved_values),

if grants is None:
plugin_type_values = (ResourceType.PLUGIN.value, ResourceType.PLUGIN.name)
approved_values = (MemberStatus.APPROVED.value, MemberStatus.APPROVED.name)
grants = (
db.query(ResourceMember)
.filter(
ResourceMember.resource_type.in_(plugin_type_values),
ResourceMember.resource_id == plugin.id,
ResourceMember.status.in_(approved_values),
)
.all()
)
.all()
)
if not grants:
return plugin.visibility == "workspace"
if any(
Expand All @@ -1931,6 +1960,42 @@ def _can_access_plugin(
}
if not granted_namespace_ids:
return False

if access_context is None:
access_context = self._load_user_plugin_access_context(
db,
user_id=user_id,
grants_by_plugin_id={plugin.id: grants},
)
if access_context is None:
return False
if access_context.namespace_ids & granted_namespace_ids:
return True
if not access_context.namespace_names:
return False
granted_names = [
access_context.namespace_names_by_id[namespace_id]
for namespace_id in granted_namespace_ids
if namespace_id in access_context.namespace_names_by_id
]
return any(
member_name == granted_name or member_name.startswith(f"{granted_name}/")
for member_name in access_context.namespace_names
for granted_name in granted_names
)

def _load_user_plugin_access_context(
self,
db: Session,
*,
user_id: int | None,
grants_by_plugin_id: dict[int, list[ResourceMember]],
) -> _UserPluginAccessContext | None:
"""Load user namespace membership once for a marketplace list/access pass."""
if user_id is None:
return None

approved_values = (MemberStatus.APPROVED.value, MemberStatus.APPROVED.name)
user_namespaces = (
db.query(Namespace.id, Namespace.name)
.join(
Expand All @@ -1948,29 +2013,31 @@ def _can_access_plugin(
)
.all()
)
direct_ids = {str(row.id) for row in user_namespaces}
if direct_ids & granted_namespace_ids:
return True
user_namespace_names = [row.name for row in user_namespaces]
if not user_namespace_names:
return False
granted_names = [
row.name
for row in db.query(Namespace.name)
.filter(
Namespace.id.in_(
int(namespace_id)
for namespace_id in granted_namespace_ids
if namespace_id.isdigit()
),
Namespace.is_active.is_(True),
)
.all()
]
return any(
member_name == granted_name or member_name.startswith(f"{granted_name}/")
for member_name in user_namespace_names
for granted_name in granted_names
granted_namespace_ids = {
grant.entity_id
for grants in grants_by_plugin_id.values()
for grant in grants
if grant.entity_type == "namespace"
}
namespace_names_by_id: dict[str, str] = {}
if granted_namespace_ids:
namespace_names_by_id = {
str(row.id): row.name
for row in db.query(Namespace.id, Namespace.name)
.filter(
Namespace.id.in_(
int(namespace_id)
for namespace_id in granted_namespace_ids
if namespace_id.isdigit()
),
Namespace.is_active.is_(True),
)
.all()
}
return _UserPluginAccessContext(
namespace_ids={str(row.id) for row in user_namespaces},
namespace_names=[row.name for row in user_namespaces],
namespace_names_by_id=namespace_names_by_id,
)

def get_plugin_access(
Expand Down
79 changes: 79 additions & 0 deletions backend/tests/services/test_plugin_marketplace_v2.py
Original file line number Diff line number Diff line change
Expand Up @@ -931,6 +931,85 @@ def test_plugin_visibility_requires_an_approved_grant(test_db, test_user):
] == [plugin.id]


def test_list_plugins_batches_grant_lookups_instead_of_per_plugin_queries(
test_db, test_user
):
owner = User(
user_name="other-plugin-owner",
password_hash=test_user.password_hash,
email="other-plugin-owner@example.com",
is_active=True,
git_info=None,
)
test_db.add(owner)
test_db.flush()
for index in range(12):
plugin = Plugin(
slug=f"personal-{index}",
name=f"personal-{index}",
display_name=f"Personal {index}",
keywords_json=[],
interface_json={},
visibility="personal",
status="published",
owner_user_id=owner.id,
)
test_db.add(plugin)
test_db.flush()
release = PluginRelease(
plugin_id=plugin.id,
version="1.0.0",
manifest_json={"name": plugin.name, "version": "1.0.0"},
interface_json={},
storage_key=f"plugins/{plugin.slug}.zip",
sha256=f"{index:064d}",
size_bytes=10,
status="ready",
scan_status="passed",
scan_report_json={},
)
test_db.add(release)
test_db.flush()
plugin.latest_release_id = release.id
test_db.add(
ResourceMember.create(
resource_type="Plugin",
resource_id=plugin.id,
entity_type="user",
entity_id=str(owner.id),
status=MemberStatus.APPROVED.value,
)
)
test_db.commit()

statements: list[str] = []

def before_cursor_execute(
_conn, _cursor, statement, _parameters, _context, _executemany
):
statements.append(str(statement))
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

bind = test_db.get_bind()
from sqlalchemy import event

event.listen(bind, "before_cursor_execute", before_cursor_execute)
try:
service = PluginMarketplaceService()
items = service.list_plugins(test_db, user_id=test_user.id).items
finally:
event.remove(bind, "before_cursor_execute", before_cursor_execute)

assert items == []
plugin_grant_lookups = [
statement
for statement in statements
if "resource_members" in statement.lower()
and "resource_id" in statement.lower()
]
# One batched grants query, plus at most one user-namespace membership query.
assert len(plugin_grant_lookups) <= 2


def test_restricted_submission_is_owner_only_until_access_is_granted(
test_db, test_user, monkeypatch
):
Expand Down
1 change: 1 addition & 0 deletions wework/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4916,6 +4916,7 @@ pub fn run() {
local_executor::local_executor_migrate_native_codex_home,
local_executor::local_executor_package_plugin,
local_executor::local_executor_read_plugin_cloud_links,
local_executor::local_executor_list_personal_marketplace_plugins,
local_executor::local_executor_read_plugin_manifest,
local_executor::local_executor_read_codex_local_config,
local_executor::local_executor_read_log,
Expand Down
Loading
Loading