From fe429eb59596b910bbd5af6ec36d51ab11324b2b Mon Sep 17 00:00:00 2001 From: zhengchuyi Date: Tue, 15 Sep 2026 11:17:43 +0800 Subject: [PATCH 1/2] fix: isolate private skill visibility --- frontend/server/skills/repository.py | 212 ++++++++++++++---- frontend/server/skills/service.py | 6 +- tests/frontend/server/skills/test_versions.py | 98 ++++++++ veadk/cli/cli_frontend.py | 81 +++++-- 4 files changed, 337 insertions(+), 60 deletions(-) diff --git a/frontend/server/skills/repository.py b/frontend/server/skills/repository.py index c87e04c2d..cd17d6ab2 100644 --- a/frontend/server/skills/repository.py +++ b/frontend/server/skills/repository.py @@ -46,6 +46,74 @@ class SkillSpaceListResult: degraded: bool = False +def _tags(value: Any) -> dict[str, str]: + return { + str(getattr(tag, "key", "") or ""): str(getattr(tag, "value", "") or "") + for tag in (getattr(value, "tags", None) or []) + } + + +def skill_space_visible_to_author( + space: Any, *, author: str, is_admin: bool = False +) -> bool: + from .system_spaces import is_shared_space + + if is_admin: + return True + return is_shared_space(space) or _tags(space).get("author") == author + + +def _skill_author( + client: Any, + skills_types: Any, + *, + skill_id: str, + fallback: str = "", +) -> str: + if not skill_id: + return fallback + try: + skill = client.get_skill(skills_types.GetSkillRequest(Id=skill_id)) + except Exception: + if fallback: + return fallback + raise + return _tags(skill).get("author") or fallback + + +def require_skill_read( + client: Any, + skills_types: Any, + *, + space_id: str, + skill_id: str, + author: str, + is_admin: bool = False, +) -> None: + from .system_spaces import is_review_space, is_shared_space, require_review_read + + require_review_read(client, space_id, skill_id=skill_id, is_admin=is_admin) + if is_admin: + return + space = client.get_skill_space(skills_types.GetSkillSpaceRequest(Id=space_id)) + if is_shared_space(space): + return + if is_review_space(space): + raise SkillRepositoryError( + "SKILL_REVIEW_FORBIDDEN", "仅管理员可以查看审核申请", status_code=403 + ) + owner = _skill_author( + client, + skills_types, + skill_id=skill_id, + fallback=_tags(space).get("author", ""), + ) + if owner != author: + raise SkillRepositoryError( + "SKILL_READ_FORBIDDEN", "只能查看自己创建的 Skill", status_code=403 + ) + + def _is_missing_skill_relation(error: BaseException) -> bool: expected = "ResourceNotFound.skill" for name in ("code", "error_code", "Code"): @@ -62,21 +130,41 @@ def list_skill_space_items( page: int = 1, page_size: int = 100, include_display_metadata: bool = False, + visible_author: str | None = None, ) -> SkillSpaceListResult: """List authoritative relations, recovering readable names only on one 404.""" - try: - response = client.list_skills_by_skill_space( + def load_space() -> Any: + return client.get_skill_space(skills_types.GetSkillSpaceRequest(Id=space_id)) + + def visible(item: Any, space: Any) -> bool: + from .system_spaces import is_shared_space + + if not visible_author or is_shared_space(space): + return True + owner = _skill_author( + client, + skills_types, + skill_id=str(getattr(item, "skill_id", "") or ""), + fallback=_tags(space).get("author", ""), + ) + return owner == visible_author + + def list_relations(request_page: int, request_page_size: int) -> Any: + return client.list_skills_by_skill_space( skills_types.ListSkillsBySkillSpaceRequest( SkillSpaceId=space_id, - PageNumber=page, - PageSize=page_size, + PageNumber=request_page, + PageSize=request_page_size, ) ) + + try: + response = list_relations(page, page_size) except Exception as relation_error: if not _is_missing_skill_relation(relation_error): raise - space = client.get_skill_space(skills_types.GetSkillSpaceRequest(Id=space_id)) + space = load_space() space_name = str(getattr(space, "name", "") or "").strip() if not space_name: raise relation_error @@ -87,37 +175,40 @@ def list_skill_space_items( ) ) recovered: list[dict[str, object]] = [] - for basic in list(getattr(fallback, "items", None) or []): - name = str(getattr(basic, "name", "") or "").strip() - if not name: - continue - try: - info = client.get_skill_info( - skills_types.GetSkillInfoRequest( - SkillName=name, - SkillSpaceName=space_name, - SkillSpaceId=space_id, + if skill_space_visible_to_author( + space, author=visible_author or "", is_admin=not visible_author + ): + for basic in list(getattr(fallback, "items", None) or []): + name = str(getattr(basic, "name", "") or "").strip() + if not name: + continue + try: + info = client.get_skill_info( + skills_types.GetSkillInfoRequest( + SkillName=name, + SkillSpaceName=space_name, + SkillSpaceId=space_id, + ) ) + except Exception as info_error: + if _is_missing_skill_relation(info_error): + continue + raise + recovered.append( + { + "skillId": "", + "skillName": str(getattr(info, "skill_name", "") or name), + "skillDescription": str( + getattr(info, "description", "") + or getattr(basic, "description", "") + or "" + ), + "version": "", + "skillStatus": "", + "lookupByName": True, + "degraded": True, + } ) - except Exception as info_error: - if _is_missing_skill_relation(info_error): - continue - raise - recovered.append( - { - "skillId": "", - "skillName": str(getattr(info, "skill_name", "") or name), - "skillDescription": str( - getattr(info, "description", "") - or getattr(basic, "description", "") - or "" - ), - "version": "", - "skillStatus": "", - "lookupByName": True, - "degraded": True, - } - ) start = (page - 1) * page_size return SkillSpaceListResult( items=tuple(recovered[start : start + page_size]), @@ -126,19 +217,38 @@ def list_skill_space_items( ) raw_items = list(getattr(response, "items", None) or []) + filtered_total_count: int | None = None + space = None + if visible_author: + space = load_space() + if not skill_space_visible_to_author(space, author=visible_author): + return SkillSpaceListResult(items=(), total_count=0) + filtered: list[Any] = [] + scanned = 0 + request_page = 1 + while True: + page_response = list_relations(request_page, 100) + page_items = list(getattr(page_response, "items", None) or []) + filtered.extend(item for item in page_items if visible(item, space)) + scanned += len(page_items) + total = getattr(page_response, "total_count", None) + if len(page_items) < 100 or (total is not None and scanned >= total): + break + request_page += 1 + start = (page - 1) * page_size + raw_items = filtered[start : start + page_size] + filtered_total_count = len(filtered) metadata: dict[str, dict[str, str]] = {} if include_display_metadata and raw_items: from .system_spaces import is_shared_space - space = client.get_skill_space(skills_types.GetSkillSpaceRequest(Id=space_id)) + space = space or load_space() if is_shared_space(space): for item in raw_items: skill_id = str(getattr(item, "skill_id", "") or "") if skill_id and skill_id not in metadata: skill = client.get_skill(skills_types.GetSkillRequest(Id=skill_id)) - tags = { - tag.key: tag.value for tag in getattr(skill, "tags", None) or [] - } + tags = _tags(skill) metadata[skill_id] = { "author": tags.get("author", ""), "sourceVersion": tags.get(SHARED_SOURCE_VERSION_TAG, ""), @@ -155,7 +265,9 @@ def list_skill_space_items( } for item in raw_items ), - total_count=( + total_count=filtered_total_count + if filtered_total_count is not None + else ( int(response.total_count) if getattr(response, "total_count", None) is not None else len(raw_items) @@ -285,6 +397,26 @@ def require_review_read( self._client_factory(region), space_id, skill_id=skill_id, is_admin=is_admin ) + def require_skill_read( + self, + *, + region: str, + space_id: str, + skill_id: str, + author: str, + is_admin: bool, + ) -> None: + from agentkit.sdk.skills import types as skills_types + + require_skill_read( + self._client_factory(region), + skills_types, + space_id=space_id, + skill_id=skill_id, + author=author, + is_admin=is_admin, + ) + def ensure_shared_space(self, *, region: str) -> dict[str, object]: from .consts import SHARE_SPACE @@ -798,4 +930,6 @@ def _space_item(value: Any, region: str) -> dict[str, object]: "SkillRepositoryError", "SkillSpaceListResult", "list_skill_space_items", + "require_skill_read", + "skill_space_visible_to_author", ] diff --git a/frontend/server/skills/service.py b/frontend/server/skills/service.py index f29a7bf14..aa193aa10 100644 --- a/frontend/server/skills/service.py +++ b/frontend/server/skills/service.py @@ -168,10 +168,11 @@ def skill_files( skill_space_name: str | None = None, skill_name: str | None = None, ) -> dict[str, object]: - self._repository.require_review_read( + self._repository.require_skill_read( region=region, space_id=space_id, skill_id=skill_id, + author=identity.author, is_admin=identity.is_admin, ) return self._repository.skill_files( @@ -194,10 +195,11 @@ def skill_archive( skill_space_name: str | None = None, skill_name: str | None = None, ) -> tuple[bytes, str]: - self._repository.require_review_read( + self._repository.require_skill_read( region=region, space_id=space_id, skill_id=skill_id, + author=identity.author, is_admin=identity.is_admin, ) return self._repository.skill_archive( diff --git a/tests/frontend/server/skills/test_versions.py b/tests/frontend/server/skills/test_versions.py index 8066e1f4e..99d66eee9 100644 --- a/tests/frontend/server/skills/test_versions.py +++ b/tests/frontend/server/skills/test_versions.py @@ -186,6 +186,104 @@ def test_shared_history_only_exposes_shared_versions_without_update(setup): assert not cloud.updated +def test_skill_space_listing_filters_personal_skills_by_author(): + from agentkit.sdk.skills import types as sdk + from frontend.server.skills.repository import list_skill_space_items + + class CatalogClient: + def __init__(self) -> None: + self.space = SimpleNamespace( + name="personal", + description="", + tags=[SimpleNamespace(key="author", value="alice")], + ) + self.skills = { + "alice-skill": SimpleNamespace( + id="alice-skill", + tags=[SimpleNamespace(key="author", value="alice")], + ), + "bob-skill": SimpleNamespace( + id="bob-skill", + tags=[SimpleNamespace(key="author", value="bob")], + ), + } + self.relations = [ + SimpleNamespace( + skill_id="bob-skill", + skill_name="bob-only", + skill_description="Hidden", + version="v1", + skill_status="running", + ), + SimpleNamespace( + skill_id="alice-skill", + skill_name="alice-only", + skill_description="Visible", + version="v2", + skill_status="running", + ), + ] + + def get_skill_space(self, _: Any) -> Any: + return self.space + + def get_skill(self, request: Any) -> Any: + return self.skills[request.id] + + def list_skills_by_skill_space(self, request: Any) -> Any: + start = (request.page_number - 1) * request.page_size + return SimpleNamespace( + items=self.relations[start : start + request.page_size], + total_count=len(self.relations), + ) + + catalog = list_skill_space_items( + CatalogClient(), + sdk, + space_id="personal", + page=1, + page_size=1, + visible_author="alice", + ) + + assert catalog.total_count == 1 + assert [item["skillName"] for item in catalog.items] == ["alice-only"] + + +def test_personal_skill_files_reject_other_users(setup): + cloud, repository, _ = setup + + with pytest.raises(SkillRepositoryError) as raised: + repository.require_skill_read( + region="cn-beijing", + space_id="space", + skill_id="skill", + author="bob", + is_admin=False, + ) + + assert raised.value.status_code == 403 + repository.require_skill_read( + region="cn-beijing", + space_id="space", + skill_id="skill", + author="alice", + is_admin=False, + ) + cloud.space = SimpleNamespace( + name=SHARE_SPACE.name, + description=SHARE_SPACE.managed_description, + tags=[], + ) + repository.require_skill_read( + region="cn-beijing", + space_id="space", + skill_id="skill", + author="bob", + is_admin=False, + ) + + def test_review_version_history_is_admin_only_and_never_updatable(setup): cloud, _, versions = setup cloud.space = SimpleNamespace( diff --git a/veadk/cli/cli_frontend.py b/veadk/cli/cli_frontend.py index c4643539d..01ab40696 100644 --- a/veadk/cli/cli_frontend.py +++ b/veadk/cli/cli_frontend.py @@ -14102,7 +14102,9 @@ def _skills_client(region: str): AgentKitSkillRepository, DEGRADED_SKILLSPACE_WARNING, list_skill_space_items, + require_skill_read, resolve_skill_response, + skill_space_visible_to_author, ) from frontend.server.skills.routes import _convert_error, mount_skill_routes from frontend.server.skills.auto_scoring import SkillAutoScoring @@ -14134,6 +14136,7 @@ def _skills_client(region: str): @app.get("/web/skill-spaces") async def _web_list_skill_spaces( + request: Request, region: str = "", page: int = Query(default=1, ge=1), page_size: int = Query(default=50, ge=1, le=100), @@ -14156,21 +14159,60 @@ async def _web_list_skill_spaces( all_items = [] total_count = 0 project_name = (project or "").strip() or None + identity = _skill_identity(request) for reg in regions: try: client = _skills_client(reg) - request_page = 1 if aggregate_regions else page - request_page_size = 50 if aggregate_regions else page_size - resp = await asyncio.to_thread( - client.list_skill_spaces, - ListSkillSpacesRequest( - PageNumber=request_page, - PageSize=request_page_size, - ProjectName=project_name, - ), + visible_spaces = [] + scan_page = ( + 1 if not identity.is_admin else (1 if aggregate_regions else page) ) - for s in resp.items or []: + scan_page_size = ( + 100 + if not identity.is_admin + else (50 if aggregate_regions else page_size) + ) + scanned = 0 + while True: + resp = await asyncio.to_thread( + client.list_skill_spaces, + ListSkillSpacesRequest( + PageNumber=scan_page, + PageSize=scan_page_size, + ProjectName=project_name, + ), + ) + spaces = list(resp.items or []) + for s in spaces: + if is_review_space(s): + continue + if skill_space_visible_to_author( + s, author=identity.author, is_admin=identity.is_admin + ): + visible_spaces.append(s) + scanned += len(spaces) + native_total = resp.total_count + if ( + identity.is_admin + or len(spaces) < scan_page_size + or (native_total is not None and scanned >= native_total) + ): + break + scan_page += 1 + if not identity.is_admin and not aggregate_regions: + start = (page - 1) * page_size + page_spaces = visible_spaces[start : start + page_size] + total_count = len(visible_spaces) + else: + page_spaces = visible_spaces + if not aggregate_regions: + total_count = ( + resp.total_count + if identity.is_admin and resp.total_count is not None + else len(visible_spaces) + ) + for s in page_spaces: if is_review_space(s): continue all_items.append( @@ -14186,12 +14228,6 @@ async def _web_list_skill_spaces( "skillCount": len(s.relations or []), } ) - if not aggregate_regions: - total_count = ( - resp.total_count - if resp.total_count is not None - else len(all_items) - ) except HTTPException: raise except Exception as e: @@ -14222,11 +14258,12 @@ async def _web_list_skills_in_space( region = _coerce_studio_resource_region(region) try: client = _skills_client(region) + identity = _skill_identity(request) await asyncio.to_thread( require_review_read, client, space_id, - is_admin=_skill_identity(request).is_admin, + is_admin=identity.is_admin, ) result = await asyncio.to_thread( list_skill_space_items, @@ -14236,6 +14273,7 @@ async def _web_list_skills_in_space( page=page, page_size=page_size, include_display_metadata=True, + visible_author=None if identity.is_admin else identity.author, ) except HTTPException: raise @@ -14270,15 +14308,20 @@ async def _web_get_skill_detail( skill_name: str | None = None, ): """Fetch a specific skill version's SKILL.md content plus package files.""" + from agentkit.sdk.skills import types as skills_types + region = _coerce_studio_resource_region(region) try: client = _skills_client(region) + identity = _skill_identity(request) await asyncio.to_thread( - require_review_read, + require_skill_read, client, + skills_types, space_id, skill_id=skill_id, - is_admin=_skill_identity(request).is_admin, + author=identity.author, + is_admin=identity.is_admin, ) resp = await asyncio.to_thread( resolve_skill_response, From 62c5cac52b81738ea2dfa85f87f7ee987b4a47a8 Mon Sep 17 00:00:00 2001 From: zhengchuyi Date: Tue, 15 Sep 2026 14:54:25 +0800 Subject: [PATCH 2/2] fix: visible skill space --- frontend/server/skills/consts.py | 2 + frontend/server/skills/repository.py | 218 ++++++++---------- frontend/server/skills/reviews.py | 8 + frontend/server/skills/service.py | 6 +- frontend/server/skills/versions.py | 10 +- tests/frontend/server/skills/test_reviews.py | 16 ++ .../server/skills/test_shared_space.py | 77 +++++++ tests/frontend/server/skills/test_versions.py | 49 ++-- veadk/cli/cli_frontend.py | 15 +- 9 files changed, 234 insertions(+), 167 deletions(-) diff --git a/frontend/server/skills/consts.py b/frontend/server/skills/consts.py index 6f3cde9be..d6bd3eef8 100644 --- a/frontend/server/skills/consts.py +++ b/frontend/server/skills/consts.py @@ -42,6 +42,8 @@ def managed_description(self) -> str: SYSTEM_SKILL_SPACES = (SHARE_SPACE, REVIEW_SPACE) RESERVED_SKILL_SPACE_NAMES = frozenset(space.name for space in SYSTEM_SKILL_SPACES) SKILL_SPACE_DISPLAY_NAME_TAG = "display_name" +SKILL_VISIBILITY_TAG = "veadk:visibility" +SKILL_VISIBILITY_SHARED = "shared" REVIEW_SOURCE_SPACE_TAG = "studio:review-source-space" REVIEW_SOURCE_SKILL_TAG = "studio:review-source-skill" REVIEW_SOURCE_VERSION_TAG = "studio:review-source-version" diff --git a/frontend/server/skills/repository.py b/frontend/server/skills/repository.py index cd17d6ab2..5ceee39f0 100644 --- a/frontend/server/skills/repository.py +++ b/frontend/server/skills/repository.py @@ -30,7 +30,12 @@ from uuid import uuid4 from .archive import SkillArchive -from .consts import SHARED_SOURCE_VERSION_TAG, SKILL_SPACE_DISPLAY_NAME_TAG +from .consts import ( + SHARED_SOURCE_VERSION_TAG, + SKILL_SPACE_DISPLAY_NAME_TAG, + SKILL_VISIBILITY_SHARED, + SKILL_VISIBILITY_TAG, +) from .space_names import skill_space_display_name if TYPE_CHECKING: @@ -63,36 +68,16 @@ def skill_space_visible_to_author( return is_shared_space(space) or _tags(space).get("author") == author -def _skill_author( - client: Any, - skills_types: Any, - *, - skill_id: str, - fallback: str = "", -) -> str: - if not skill_id: - return fallback - try: - skill = client.get_skill(skills_types.GetSkillRequest(Id=skill_id)) - except Exception: - if fallback: - return fallback - raise - return _tags(skill).get("author") or fallback - - -def require_skill_read( +def require_space_read( client: Any, skills_types: Any, *, space_id: str, - skill_id: str, author: str, is_admin: bool = False, ) -> None: - from .system_spaces import is_review_space, is_shared_space, require_review_read + from .system_spaces import is_review_space, is_shared_space - require_review_read(client, space_id, skill_id=skill_id, is_admin=is_admin) if is_admin: return space = client.get_skill_space(skills_types.GetSkillSpaceRequest(Id=space_id)) @@ -102,15 +87,11 @@ def require_skill_read( raise SkillRepositoryError( "SKILL_REVIEW_FORBIDDEN", "仅管理员可以查看审核申请", status_code=403 ) - owner = _skill_author( - client, - skills_types, - skill_id=skill_id, - fallback=_tags(space).get("author", ""), - ) - if owner != author: + if _tags(space).get("author") != author: raise SkillRepositoryError( - "SKILL_READ_FORBIDDEN", "只能查看自己创建的 Skill", status_code=403 + "SKILL_SPACE_READ_FORBIDDEN", + "只能查看自己创建的 Skill 空间", + status_code=403, ) @@ -130,26 +111,12 @@ def list_skill_space_items( page: int = 1, page_size: int = 100, include_display_metadata: bool = False, - visible_author: str | None = None, ) -> SkillSpaceListResult: """List authoritative relations, recovering readable names only on one 404.""" def load_space() -> Any: return client.get_skill_space(skills_types.GetSkillSpaceRequest(Id=space_id)) - def visible(item: Any, space: Any) -> bool: - from .system_spaces import is_shared_space - - if not visible_author or is_shared_space(space): - return True - owner = _skill_author( - client, - skills_types, - skill_id=str(getattr(item, "skill_id", "") or ""), - fallback=_tags(space).get("author", ""), - ) - return owner == visible_author - def list_relations(request_page: int, request_page_size: int) -> Any: return client.list_skills_by_skill_space( skills_types.ListSkillsBySkillSpaceRequest( @@ -175,40 +142,37 @@ def list_relations(request_page: int, request_page_size: int) -> Any: ) ) recovered: list[dict[str, object]] = [] - if skill_space_visible_to_author( - space, author=visible_author or "", is_admin=not visible_author - ): - for basic in list(getattr(fallback, "items", None) or []): - name = str(getattr(basic, "name", "") or "").strip() - if not name: - continue - try: - info = client.get_skill_info( - skills_types.GetSkillInfoRequest( - SkillName=name, - SkillSpaceName=space_name, - SkillSpaceId=space_id, - ) + for basic in list(getattr(fallback, "items", None) or []): + name = str(getattr(basic, "name", "") or "").strip() + if not name: + continue + try: + info = client.get_skill_info( + skills_types.GetSkillInfoRequest( + SkillName=name, + SkillSpaceName=space_name, + SkillSpaceId=space_id, ) - except Exception as info_error: - if _is_missing_skill_relation(info_error): - continue - raise - recovered.append( - { - "skillId": "", - "skillName": str(getattr(info, "skill_name", "") or name), - "skillDescription": str( - getattr(info, "description", "") - or getattr(basic, "description", "") - or "" - ), - "version": "", - "skillStatus": "", - "lookupByName": True, - "degraded": True, - } ) + except Exception as info_error: + if _is_missing_skill_relation(info_error): + continue + raise + recovered.append( + { + "skillId": "", + "skillName": str(getattr(info, "skill_name", "") or name), + "skillDescription": str( + getattr(info, "description", "") + or getattr(basic, "description", "") + or "" + ), + "version": "", + "skillStatus": "", + "lookupByName": True, + "degraded": True, + } + ) start = (page - 1) * page_size return SkillSpaceListResult( items=tuple(recovered[start : start + page_size]), @@ -217,27 +181,7 @@ def list_relations(request_page: int, request_page_size: int) -> Any: ) raw_items = list(getattr(response, "items", None) or []) - filtered_total_count: int | None = None space = None - if visible_author: - space = load_space() - if not skill_space_visible_to_author(space, author=visible_author): - return SkillSpaceListResult(items=(), total_count=0) - filtered: list[Any] = [] - scanned = 0 - request_page = 1 - while True: - page_response = list_relations(request_page, 100) - page_items = list(getattr(page_response, "items", None) or []) - filtered.extend(item for item in page_items if visible(item, space)) - scanned += len(page_items) - total = getattr(page_response, "total_count", None) - if len(page_items) < 100 or (total is not None and scanned >= total): - break - request_page += 1 - start = (page - 1) * page_size - raw_items = filtered[start : start + page_size] - filtered_total_count = len(filtered) metadata: dict[str, dict[str, str]] = {} if include_display_metadata and raw_items: from .system_spaces import is_shared_space @@ -265,13 +209,9 @@ def list_relations(request_page: int, request_page_size: int) -> Any: } for item in raw_items ), - total_count=filtered_total_count - if filtered_total_count is not None - else ( - int(response.total_count) - if getattr(response, "total_count", None) is not None - else len(raw_items) - ), + total_count=int(response.total_count) + if getattr(response, "total_count", None) is not None + else len(raw_items), ) @@ -397,22 +337,20 @@ def require_review_read( self._client_factory(region), space_id, skill_id=skill_id, is_admin=is_admin ) - def require_skill_read( + def require_space_read( self, *, region: str, space_id: str, - skill_id: str, author: str, is_admin: bool, ) -> None: from agentkit.sdk.skills import types as skills_types - require_skill_read( + require_space_read( self._client_factory(region), skills_types, space_id=space_id, - skill_id=skill_id, author=author, is_admin=is_admin, ) @@ -449,27 +387,61 @@ def list_spaces( ) -> dict[str, object]: from agentkit.sdk.skills import types as skills_types + from .system_spaces import is_review_space, is_shared_space + tag_filters = None if author: tag_filters = [ skills_types.TagFilterForSkill(Key="author", Values=[author]) ] - response = self._client_factory(region).list_skill_spaces( - skills_types.ListSkillSpacesRequest( - PageNumber=page, - PageSize=page_size, - ProjectName=project_name, - TagFilters=tag_filters, + + def list_page(request_page: int, request_page_size: int) -> Any: + return self._client_factory(region).list_skill_spaces( + skills_types.ListSkillSpacesRequest( + PageNumber=request_page, + PageSize=request_page_size, + ProjectName=project_name, + TagFilters=tag_filters, + ) ) - ) - from .system_spaces import is_review_space + + def visible(item: Any) -> bool: + if is_review_space(item) or is_shared_space(item): + return False + return author is None or _tags(item).get("author") == author + + if author: + visible_items: list[Any] = [] + scanned = 0 + request_page = 1 + request_page_size = 100 + while True: + response = list_page(request_page, request_page_size) + items = list(response.items or []) + scanned += len(items) + visible_items.extend(item for item in items if visible(item)) + total = response.total_count + if len(items) < request_page_size or ( + total is not None and scanned >= total + ): + break + request_page += 1 + start = (page - 1) * page_size + page_items = visible_items[start : start + page_size] + return { + "items": [self._space_item(item, region) for item in page_items], + "scannedCount": len(page_items), + "totalCount": len(visible_items), + "page": page, + "pageSize": page_size, + } + + response = list_page(page, page_size) items = list(response.items or []) return { "items": [ - self._space_item(item, region) - for item in items - if not is_review_space(item) + self._space_item(item, region) for item in items if visible(item) ], "scannedCount": len(items), "totalCount": response.total_count @@ -826,7 +798,11 @@ def publish_archive( ProjectName=project_name, Tags=[skills_types.TagForSkill(Key="author", Value=author)] + ( - [skills_types.TagForSkill(Key="veadk:visibility", Value="shared")] + [ + skills_types.TagForSkill( + Key=SKILL_VISIBILITY_TAG, Value=SKILL_VISIBILITY_SHARED + ) + ] if shared else [] ), @@ -930,6 +906,6 @@ def _space_item(value: Any, region: str) -> dict[str, object]: "SkillRepositoryError", "SkillSpaceListResult", "list_skill_space_items", - "require_skill_read", + "require_space_read", "skill_space_visible_to_author", ] diff --git a/frontend/server/skills/reviews.py b/frontend/server/skills/reviews.py index 2de4f05f2..67f73f5fe 100644 --- a/frontend/server/skills/reviews.py +++ b/frontend/server/skills/reviews.py @@ -45,6 +45,8 @@ REVIEWER_OWNER_TAG, SHARED_REVIEW_TAG, SHARED_SOURCE_VERSION_TAG, + SKILL_VISIBILITY_SHARED, + SKILL_VISIBILITY_TAG, SCORE_STATUS_TAG, SCORE_TOTAL_TAG, SCORE_TIME_TAG, @@ -371,6 +373,7 @@ def decide( "author": application["author"], SHARED_REVIEW_TAG: application_id, SHARED_SOURCE_VERSION_TAG: application["version"], + SKILL_VISIBILITY_TAG: SKILL_VISIBILITY_SHARED, } shared_id, shared_version = self._copy_archive( client, @@ -417,6 +420,11 @@ def decide( "共享副本信息不完整，请重试", status_code=502, ) + update_skill_tags( + client, + shared_id, + {SKILL_VISIBILITY_TAG: SKILL_VISIBILITY_SHARED}, + ) update_skill_tags( client, application_id, diff --git a/frontend/server/skills/service.py b/frontend/server/skills/service.py index aa193aa10..cf11fcfba 100644 --- a/frontend/server/skills/service.py +++ b/frontend/server/skills/service.py @@ -168,10 +168,9 @@ def skill_files( skill_space_name: str | None = None, skill_name: str | None = None, ) -> dict[str, object]: - self._repository.require_skill_read( + self._repository.require_space_read( region=region, space_id=space_id, - skill_id=skill_id, author=identity.author, is_admin=identity.is_admin, ) @@ -195,10 +194,9 @@ def skill_archive( skill_space_name: str | None = None, skill_name: str | None = None, ) -> tuple[bytes, str]: - self._repository.require_skill_read( + self._repository.require_space_read( region=region, space_id=space_id, - skill_id=skill_id, author=identity.author, is_admin=identity.is_admin, ) diff --git a/frontend/server/skills/versions.py b/frontend/server/skills/versions.py index 6e3288964..308e6ff4d 100644 --- a/frontend/server/skills/versions.py +++ b/frontend/server/skills/versions.py @@ -101,12 +101,14 @@ def _source( status_code=404, ) personal = not is_shared_space(space) and not is_review_space(space) - author = _tags(skill).get("author") or _tags(space).get("author") - if personal and author and author != identity.author and not identity.is_admin: + space_owner = _tags(space).get("author", "") + if personal and space_owner != identity.author and not identity.is_admin: raise SkillRepositoryError( - "SKILL_VERSION_FORBIDDEN", "只能查看自己创建的技能版本", status_code=403 + "SKILL_VERSION_FORBIDDEN", + "只能查看自己创建的技能空间中的版本", + status_code=403, ) - can_update = personal and (identity.is_admin or author == identity.author) + can_update = personal and (identity.is_admin or space_owner == identity.author) return space, skill, relations, can_update def list( diff --git a/tests/frontend/server/skills/test_reviews.py b/tests/frontend/server/skills/test_reviews.py index 0b6734e53..bba52b2c7 100644 --- a/tests/frontend/server/skills/test_reviews.py +++ b/tests/frontend/server/skills/test_reviews.py @@ -29,6 +29,8 @@ REVIEW_STATUS_TAG, REVIEW_SOURCE_SKILL_TAG, SHARED_SOURCE_VERSION_TAG, + SKILL_VISIBILITY_SHARED, + SKILL_VISIBILITY_TAG, ) from frontend.server.skills.models import SkillIdentity from frontend.server.skills.repository import ( @@ -349,6 +351,7 @@ def test_approval_publishes_exact_snapshot_and_retains_audit_on_retry(setup): } assert shared_tags["author"] == "alice" assert shared_tags[SHARED_SOURCE_VERSION_TAG] == "v3" + assert shared_tags[SKILL_VISIBILITY_TAG] == SKILL_VISIBILITY_SHARED assert REVIEW_SOURCE_SKILL_TAG not in shared_tags require_review_read(cloud, "shared", skill_id=approved["sharedSkillId"]) assert decide(repository, pending["id"], actor="another-admin") == approved @@ -397,10 +400,23 @@ def test_failed_final_tag_write_resumes_without_duplicate_public_copy(setup): decide(repository, pending["id"], "returned", "cannot return while publishing") assert error.value.code == "SKILL_REVIEW_PUBLISHING" cloud.fail_tag_call = 0 + shared_id = cloud.relations["shared"][0].skill_id + cloud.skills[shared_id].tags = [ + tag for tag in cloud.skills[shared_id].tags if tag.key != SKILL_VISIBILITY_TAG + ] + cloud.tag_calls.clear() approved = decide(repository, pending["id"], actor="second-admin") assert approved["reviewedBy"] == "admin" assert approved["status"] == "approved" assert len(cloud.relations["shared"]) == 1 + shared_tags = {tag.key: tag.value for tag in cloud.skills[shared_id].tags} + assert shared_tags[SKILL_VISIBILITY_TAG] == SKILL_VISIBILITY_SHARED + assert any( + call[1]["ResourceIds"] == [shared_id] + and call[1]["Tags"] + == [{"Key": SKILL_VISIBILITY_TAG, "Value": SKILL_VISIBILITY_SHARED}] + for call in cloud.tag_calls + ) def test_failed_initial_tags_never_publish_and_failed_publication_can_retry(setup): diff --git a/tests/frontend/server/skills/test_shared_space.py b/tests/frontend/server/skills/test_shared_space.py index 33f585f70..c51093924 100644 --- a/tests/frontend/server/skills/test_shared_space.py +++ b/tests/frontend/server/skills/test_shared_space.py @@ -38,6 +38,7 @@ class Client: def __init__(self): self.spaces = [] self.created = [] + self.list_requests = [] self.fail_list = False self.race = False self.omit_tags = False @@ -46,6 +47,7 @@ def __init__(self): def list_skill_spaces(self, request): if self.fail_list: raise RuntimeError("list unavailable") + self.list_requests.append(request) items = [ space for space in self.spaces @@ -89,6 +91,28 @@ def get_skill_space(self, request): return next(space for space in self.spaces if space.id == request.id) +def personal_space( + space_id: str, + display_name: str, + author: str | None, + *, + project: str = "default", +): + tags = [SimpleNamespace(key="display_name", value=display_name)] + if author is not None: + tags.append(SimpleNamespace(key="author", value=author)) + return SimpleNamespace( + id=space_id, + name=f"studio_space_{space_id}", + description="", + project_name=project, + status="Running", + tags=tags, + relations=[], + update_time_stamp="", + ) + + @pytest.mark.parametrize("region", ["cn-beijing", "cn-shanghai", "ap-southeast-1"]) def test_default_space_is_created_once_and_reused_across_users(region, monkeypatch): monkeypatch.setenv("VEADK_STUDIO_PROJECT", "test-project") @@ -248,6 +272,59 @@ def test_shared_endpoint_uses_trusted_identity_and_preserves_personal_filter(): ) +def test_personal_space_list_locally_filters_when_cloud_ignores_tag_filters(): + client = Client() + client.ignore_tag_filters = True + repository = AgentKitSkillRepository(lambda _: client) + repository.ensure_shared_space(region="cn-beijing") + repository.ensure_review_space(region="cn-beijing") + client.spaces.extend( + [ + personal_space("own", "Mine", "member"), + personal_space("other", "Other", "someone-else"), + personal_space("legacy", "Legacy", None), + ] + ) + + result = repository.list_spaces( + region="cn-beijing", + page=1, + page_size=20, + project_name=None, + author="member", + ) + + assert [item["displayName"] for item in result["items"]] == ["Mine"] + assert result["totalCount"] == 1 + assert result["scannedCount"] == 1 + request = client.list_requests[-1] + assert request.tag_filters[0].key == "author" + assert request.tag_filters[0].values == ["member"] + + +def test_personal_space_list_scans_past_unowned_native_pages(): + client = Client() + client.ignore_tag_filters = True + client.spaces = [ + personal_space(f"other-{index}", f"Other {index}", "someone-else") + for index in range(100) + ] + [personal_space("own", "Mine", "member")] + repository = AgentKitSkillRepository(lambda _: client) + + result = repository.list_spaces( + region="cn-beijing", + page=1, + page_size=20, + project_name=None, + author="member", + ) + + assert [item["displayName"] for item in result["items"]] == ["Mine"] + assert result["totalCount"] == 1 + assert result["scannedCount"] == 1 + assert [request.page_number for request in client.list_requests] == [1, 2] + + def test_shared_skill_cannot_be_deleted_through_another_space_id(): client = SimpleNamespace() deleted = [] diff --git a/tests/frontend/server/skills/test_versions.py b/tests/frontend/server/skills/test_versions.py index 99d66eee9..cd21ef7cf 100644 --- a/tests/frontend/server/skills/test_versions.py +++ b/tests/frontend/server/skills/test_versions.py @@ -62,7 +62,11 @@ def version(name: str, status: str = "running") -> Any: class VersionClient: def __init__(self) -> None: - self.space = SimpleNamespace(name="personal", description="", tags=[]) + self.space = SimpleNamespace( + name="personal", + description="", + tags=[SimpleNamespace(key="author", value="alice")], + ) self.skill = SimpleNamespace( name="daily-summary", tags=[SimpleNamespace(key="author", value="alice")] ) @@ -186,7 +190,7 @@ def test_shared_history_only_exposes_shared_versions_without_update(setup): assert not cloud.updated -def test_skill_space_listing_filters_personal_skills_by_author(): +def test_skill_space_listing_returns_all_skills_in_readable_personal_space(): from agentkit.sdk.skills import types as sdk from frontend.server.skills.repository import list_skill_space_items @@ -197,16 +201,6 @@ def __init__(self) -> None: description="", tags=[SimpleNamespace(key="author", value="alice")], ) - self.skills = { - "alice-skill": SimpleNamespace( - id="alice-skill", - tags=[SimpleNamespace(key="author", value="alice")], - ), - "bob-skill": SimpleNamespace( - id="bob-skill", - tags=[SimpleNamespace(key="author", value="bob")], - ), - } self.relations = [ SimpleNamespace( skill_id="bob-skill", @@ -227,9 +221,6 @@ def __init__(self) -> None: def get_skill_space(self, _: Any) -> Any: return self.space - def get_skill(self, request: Any) -> Any: - return self.skills[request.id] - def list_skills_by_skill_space(self, request: Any) -> Any: start = (request.page_number - 1) * request.page_size return SimpleNamespace( @@ -243,30 +234,28 @@ def list_skills_by_skill_space(self, request: Any) -> Any: space_id="personal", page=1, page_size=1, - visible_author="alice", ) - assert catalog.total_count == 1 - assert [item["skillName"] for item in catalog.items] == ["alice-only"] + assert catalog.total_count == 2 + assert [item["skillName"] for item in catalog.items] == ["bob-only"] -def test_personal_skill_files_reject_other_users(setup): +def test_personal_space_read_rejects_other_users(setup): cloud, repository, _ = setup with pytest.raises(SkillRepositoryError) as raised: - repository.require_skill_read( + repository.require_space_read( region="cn-beijing", space_id="space", - skill_id="skill", author="bob", is_admin=False, ) assert raised.value.status_code == 403 - repository.require_skill_read( + assert raised.value.code == "SKILL_SPACE_READ_FORBIDDEN" + repository.require_space_read( region="cn-beijing", space_id="space", - skill_id="skill", author="alice", is_admin=False, ) @@ -275,10 +264,9 @@ def test_personal_skill_files_reject_other_users(setup): description=SHARE_SPACE.managed_description, tags=[], ) - repository.require_skill_read( + repository.require_space_read( region="cn-beijing", space_id="space", - skill_id="skill", author="bob", is_admin=False, ) @@ -295,12 +283,13 @@ def test_review_version_history_is_admin_only_and_never_updatable(setup): assert listed(versions, "admin", True)["canUpdate"] is False -def test_legacy_missing_author_history_readable_but_not_claimed_for_update(setup): +def test_personal_version_history_requires_space_owner_tag(setup): cloud, _, versions = setup - cloud.skill.tags = [] - assert listed(versions)["canUpdate"] is False - cloud.space.tags = [SimpleNamespace(key="author", value="alice")] - assert listed(versions)["canUpdate"] is True + cloud.skill.tags = [SimpleNamespace(key="author", value="alice")] + cloud.space.tags = [] + with pytest.raises(SkillRepositoryError) as error: + listed(versions) + assert error.value.status_code == 403 def test_upload_rejects_renamed_archive_before_cloud_mutation(setup): diff --git a/veadk/cli/cli_frontend.py b/veadk/cli/cli_frontend.py index 01ab40696..9b514c3dd 100644 --- a/veadk/cli/cli_frontend.py +++ b/veadk/cli/cli_frontend.py @@ -14102,7 +14102,7 @@ def _skills_client(region: str): AgentKitSkillRepository, DEGRADED_SKILLSPACE_WARNING, list_skill_space_items, - require_skill_read, + require_space_read, resolve_skill_response, skill_space_visible_to_author, ) @@ -14113,7 +14113,6 @@ def _skills_client(region: str): from frontend.server.skills.space_names import skill_space_display_name from frontend.server.skills.system_spaces import ( is_review_space, - require_review_read, ) identity_management = getattr(app.state, "studio_user_management", None) @@ -14260,9 +14259,11 @@ async def _web_list_skills_in_space( client = _skills_client(region) identity = _skill_identity(request) await asyncio.to_thread( - require_review_read, + require_space_read, client, - space_id, + skills_types, + space_id=space_id, + author=identity.author, is_admin=identity.is_admin, ) result = await asyncio.to_thread( @@ -14273,7 +14274,6 @@ async def _web_list_skills_in_space( page=page, page_size=page_size, include_display_metadata=True, - visible_author=None if identity.is_admin else identity.author, ) except HTTPException: raise @@ -14315,11 +14315,10 @@ async def _web_get_skill_detail( client = _skills_client(region) identity = _skill_identity(request) await asyncio.to_thread( - require_skill_read, + require_space_read, client, skills_types, - space_id, - skill_id=skill_id, + space_id=space_id, author=identity.author, is_admin=identity.is_admin, )