diff --git a/CHANGELOG.md b/CHANGELOG.md index 9f2a3ba4..d60354e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,9 @@ ## [Unreleased] +### 新增 +- 专家:发布专家模板时新增「允许查看技能详情」开关;关闭后,安装者只能使用技能,无法查看或修改技能内容(发布者与管理员除外) + ### 修复 - 插件工具使用中文等非 ASCII 名称时 LLM 调用失败:主流 API 要求工具名匹配 `^[a-zA-Z0-9_-]{1,64}$`,现自动将非法名称转写为合法拼音名(`pypinyin` 缺失时退回下划线替换),冲突追加 `_2`/`_3` 后缀,并在工具描述前缀 `[原名: …]` 保留原名映射;`config_json.plugins` 配置键与插件内部仍使用原始名称,路由不受影响 diff --git a/dashboard/src/api/modules/publishedExperts.test.ts b/dashboard/src/api/modules/publishedExperts.test.ts index ecad37c6..d6526153 100644 --- a/dashboard/src/api/modules/publishedExperts.test.ts +++ b/dashboard/src/api/modules/publishedExperts.test.ts @@ -43,6 +43,11 @@ describe("publishedExpertsApi", () => { "/experts/published/expert%2F1/refresh", { method: "POST", + body: JSON.stringify({ + name: "Updated", + description: "New description", + welcome_message: { zh: "欢迎", en: "Welcome" }, + }), }, ); expect(request).toHaveBeenNthCalledWith( diff --git a/dashboard/src/api/modules/publishedExperts.ts b/dashboard/src/api/modules/publishedExperts.ts index eb61f1ce..cb5268da 100644 --- a/dashboard/src/api/modules/publishedExperts.ts +++ b/dashboard/src/api/modules/publishedExperts.ts @@ -12,6 +12,9 @@ export interface PublishedExpert { color: string | null; created_at: string; updated_at: string; + /** When false, installers can use the expert's skills but not view or + * edit their contents. */ + allow_skill_details?: boolean; } export interface PublishExpertBody { @@ -19,6 +22,7 @@ export interface PublishExpertBody { description?: string; slug?: string; welcome_message?: { zh?: string; en?: string }; + allow_skill_details?: boolean; } export type RefreshPublishedExpertBody = PublishExpertBody; diff --git a/dashboard/src/locales/en.json b/dashboard/src/locales/en.json index 81940d9b..6b808e7f 100644 --- a/dashboard/src/locales/en.json +++ b/dashboard/src/locales/en.json @@ -114,6 +114,7 @@ "TLS_DOMAIN_MISMATCH": "The domain does not match the existing certificate.", "WORKSPACE_OP_UNSUPPORTED": "This workspace backend does not support that file operation.", "SKILL_IMPORT_UNSUPPORTED_URL": "This skill URL source is not supported.", + "SKILL_DETAILS_PROTECTED": "The publisher of this expert has disabled viewing or editing its skill contents.", "SKILL_IMPORT_FAILED": "Skill import failed: {{reason}}", "SKILL_ALREADY_EXISTS": "Skill {{name}} already exists. Enable overwrite to replace it.", "SKILL_PACKAGE_NOT_FOUND": "Skill package not found.", @@ -594,7 +595,10 @@ "fieldDescription": "Description", "fieldWelcomeZh": "Welcome message (Chinese)", "fieldWelcomeEn": "Welcome message (English)", - "fieldWelcomePlaceholder": "One-line greeting shown on the chat welcome screen" + "fieldWelcomePlaceholder": "One-line greeting shown on the chat welcome screen", + "fieldAllowSkillDetails": "Allow installers to view skill details", + "allowSkillDetailsHint": "When off, users who install this template can use its skills but cannot view or edit skill contents.", + "skillsProtected": "Skill details locked after install" }, "noExperts": "No experts available", "selectExpertHint": "Pick an expert on the left to view details", @@ -1073,6 +1077,8 @@ "dockFileListCount": "{{count}} files", "dockFileListHint": "This list only shows files produced during execution. They are not guaranteed to be kept, and the model may delete them after processing finishes.", "dockFileMaybeDeleted": "This file may have been a temporary artifact during processing and has already been deleted.", + "dockFileDelete": "Delete file", + "dockFileDeleteConfirm": "Delete this file? This cannot be undone.", "remoteBrowserTitle": "Remote Browser", "dockTerminalTitle": "Terminal", "loadEarlierMessages": "Load earlier messages" @@ -1322,6 +1328,8 @@ "importFailed": "Failed to import skill", "loadFailed": "Failed to load skills", "loadDetailFailed": "Failed to load skill details", + "protectedTag": "Protected", + "protectedHint": "This skill is protected by the expert publisher. It can be used but its contents cannot be viewed or edited.", "enabledSuccess": "Skill enabled", "disabledSuccess": "Skill disabled", "operationFailed": "Operation failed", diff --git a/dashboard/src/locales/zh.json b/dashboard/src/locales/zh.json index 11af5bdd..8400ca39 100644 --- a/dashboard/src/locales/zh.json +++ b/dashboard/src/locales/zh.json @@ -114,6 +114,7 @@ "TLS_DOMAIN_MISMATCH": "域名与现有证书不一致。", "WORKSPACE_OP_UNSUPPORTED": "当前工作区后端不支持该文件操作。", "SKILL_IMPORT_UNSUPPORTED_URL": "不支持该技能 URL 来源。", + "SKILL_DETAILS_PROTECTED": "该专家的发布者未开放技能内容的查看与修改权限。", "SKILL_IMPORT_FAILED": "技能导入失败:{{reason}}", "SKILL_ALREADY_EXISTS": "技能 {{name}} 已存在,如需覆盖请启用 overwrite。", "SKILL_PACKAGE_NOT_FOUND": "未找到技能包。", @@ -594,7 +595,10 @@ "fieldDescription": "模板描述", "fieldWelcomeZh": "中文引导语", "fieldWelcomeEn": "英文引导语", - "fieldWelcomePlaceholder": "聊天欢迎页展示的一行引导语" + "fieldWelcomePlaceholder": "聊天欢迎页展示的一行引导语", + "fieldAllowSkillDetails": "允许安装者查看技能详情", + "allowSkillDetailsHint": "关闭后,安装该模板的用户只能使用其中的技能,无法查看或修改技能内容。", + "skillsProtected": "安装后技能详情不可查看" }, "noExperts": "暂无可用专家", "selectExpertHint": "请从左侧选择一个专家以查看详情", @@ -1073,6 +1077,8 @@ "dockFileListCount": "{{count}} 个文件", "dockFileListHint": "当前仅列出执行过程中生成的文件,不代表最终一定存储,可能在处理结束后被大模型删除。", "dockFileMaybeDeleted": "该文件可能为处理过程中的临时文件,当前已经被删除。", + "dockFileDelete": "删除文件", + "dockFileDeleteConfirm": "确定删除该文件吗?删除后不可恢复。", "remoteBrowserTitle": "远程浏览器", "dockTerminalTitle": "终端", "loadEarlierMessages": "加载更早的消息" @@ -1321,6 +1327,8 @@ "importFailed": "技能导入失败", "loadFailed": "加载技能失败", "loadDetailFailed": "加载技能详情失败", + "protectedTag": "已保护", + "protectedHint": "该技能由专家模板发布者保护,仅可使用,无法查看或修改内容。", "enabledSuccess": "技能已启用", "disabledSuccess": "技能已禁用", "operationFailed": "操作失败", diff --git a/dashboard/src/pages/Agent/Skills/components/InstalledSkillsTab.tsx b/dashboard/src/pages/Agent/Skills/components/InstalledSkillsTab.tsx index 5085ad99..da08406d 100644 --- a/dashboard/src/pages/Agent/Skills/components/InstalledSkillsTab.tsx +++ b/dashboard/src/pages/Agent/Skills/components/InstalledSkillsTab.tsx @@ -1,5 +1,6 @@ import { useMemo, useState, useCallback } from "react"; import { Form, Segmented, Tooltip } from "antd"; +import { message } from "@/utils/antdMessage"; import { Download, LayoutGrid, List, Plus, RefreshCw } from "lucide-react"; import { useTranslation } from "react-i18next"; import { useAgent } from "../../../../context/AgentContext"; @@ -117,6 +118,10 @@ export default function InstalledSkillsTab({ }; const handleEdit = async (skill: SkillSpec) => { + if (skill.protected) { + message.warning(t("skills.protectedHint")); + return; + } const detail = await getDetail(skill.slug); if (detail) { setEditingSkill(detail); @@ -180,7 +185,9 @@ export default function InstalledSkillsTab({ onMouseLeave={() => setHoverKey(null)} onToggleEnabled={(e) => void handleToggleEnabled(skill, e)} onDelete={ - kind === "custom" ? (e) => void handleDelete(skill, e) : undefined + kind === "custom" && !skill.protected + ? (e) => void handleDelete(skill, e) + : undefined } /> ))} @@ -192,7 +199,12 @@ export default function InstalledSkillsTab({ onView={(skill) => void handleEdit(skill)} onToggleEnabled={(skill) => void handleToggleEnabled(skill)} onDelete={ - kind === "custom" ? (skill) => void handleDelete(skill) : undefined + kind === "custom" + ? (skill) => { + if (skill.protected) return; + void handleDelete(skill); + } + : undefined } /> ); diff --git a/dashboard/src/pages/Agent/Skills/components/SkillCard.tsx b/dashboard/src/pages/Agent/Skills/components/SkillCard.tsx index cb34f0ec..e5cfb7b6 100644 --- a/dashboard/src/pages/Agent/Skills/components/SkillCard.tsx +++ b/dashboard/src/pages/Agent/Skills/components/SkillCard.tsx @@ -5,6 +5,7 @@ import { FileText, Image, Info, + Lock, Presentation, Sparkles, Trash2, @@ -251,6 +252,12 @@ export function SkillCard({
{displayName}
+ {skill.protected && ( + + {" "} + {t("skills.protectedTag")} + + )} {isBuiltin && ( {t("skills.kindBuiltin")} diff --git a/dashboard/src/pages/Agent/Skills/components/SkillDrawer.tsx b/dashboard/src/pages/Agent/Skills/components/SkillDrawer.tsx index 219e9c2b..53901e15 100644 --- a/dashboard/src/pages/Agent/Skills/components/SkillDrawer.tsx +++ b/dashboard/src/pages/Agent/Skills/components/SkillDrawer.tsx @@ -814,7 +814,9 @@ export function SkillDrawer({ ) : ( <> - {editingSkill?.kind === "workspace" && viewingSkillMd ? ( + {editingSkill?.kind === "workspace" && + viewingSkillMd && + !editingSkill?.protected ? ( - {kind === "custom" && onDelete ? ( + {kind === "custom" && onDelete && !row.protected ? ( ({ : "workspace", emoji: row.emoji, iconUrl: row.icon_url, + protected: row.protected, }); const toDetail = (row: ServerDetail): SkillDetail => ({ diff --git a/dashboard/src/pages/Experts/components/PublishExpertDrawer.tsx b/dashboard/src/pages/Experts/components/PublishExpertDrawer.tsx index 90ae4506..974f4261 100644 --- a/dashboard/src/pages/Experts/components/PublishExpertDrawer.tsx +++ b/dashboard/src/pages/Experts/components/PublishExpertDrawer.tsx @@ -1,6 +1,6 @@ import { useEffect, useState } from "react"; import { useTranslation } from "react-i18next"; -import { Alert, Button, Drawer, Form, Input, Spin } from "antd"; +import { Alert, Button, Drawer, Form, Input, Spin, Switch } from "antd"; import { message } from "@/utils/antdMessage"; import { agentChatApi } from "../../../api/modules/agentChat"; @@ -28,6 +28,7 @@ interface PublishFormValues { description?: string; welcome_zh?: string; welcome_en?: string; + allow_skill_details?: boolean; } export default function PublishExpertDrawer({ @@ -64,6 +65,10 @@ export default function PublishExpertDrawer({ description: baseDescription, welcome_zh: pickLocale(welcome.welcome_message, "zh"), welcome_en: pickLocale(welcome.welcome_message, "en"), + allow_skill_details: + mode === "refresh" && published + ? published.allow_skill_details !== false + : true, }); } catch { if (cancelled) return; @@ -72,6 +77,10 @@ export default function PublishExpertDrawer({ description: baseDescription, welcome_zh: "", welcome_en: "", + allow_skill_details: + mode === "refresh" && published + ? published.allow_skill_details !== false + : true, }); } finally { if (!cancelled) setPrefillLoading(false); @@ -91,6 +100,7 @@ export default function PublishExpertDrawer({ zh: values.welcome_zh?.trim() || "", en: values.welcome_en?.trim() || "", }, + allow_skill_details: values.allow_skill_details !== false, }); const handleSubmit = async () => { @@ -221,6 +231,14 @@ export default function PublishExpertDrawer({ placeholder={t("experts.published.fieldWelcomePlaceholder")} /> + + +
diff --git a/dashboard/src/pages/Experts/components/PublishedExpertCard.tsx b/dashboard/src/pages/Experts/components/PublishedExpertCard.tsx index c5f300b4..af0d13c6 100644 --- a/dashboard/src/pages/Experts/components/PublishedExpertCard.tsx +++ b/dashboard/src/pages/Experts/components/PublishedExpertCard.tsx @@ -104,7 +104,9 @@ export const PublishedExpertCard = memo(function PublishedExpertCard({
- {t("experts.published.install")} + {expert.allow_skill_details === false + ? t("experts.published.skillsProtected") + : t("experts.published.install")}
{expert.creator_username && (
diff --git a/src/octop/api/routers/experts.py b/src/octop/api/routers/experts.py index 989390da..574a1959 100644 --- a/src/octop/api/routers/experts.py +++ b/src/octop/api/routers/experts.py @@ -89,6 +89,7 @@ class PublishExpertBody(BaseModel): slug: str | None = None welcome_message: LocalizedTextResponse | None = None quick_prompts: list[QuickPromptResponse] | None = None + allow_skill_details: bool = True class RefreshPublishedExpertBody(BaseModel): @@ -96,6 +97,7 @@ class RefreshPublishedExpertBody(BaseModel): description: str | None = None welcome_message: LocalizedTextResponse | None = None quick_prompts: list[QuickPromptResponse] | None = None + allow_skill_details: bool | None = None class InstallPublishedExpertBody(AgentRuntimeFields): @@ -279,6 +281,7 @@ def _published_summary_dict(row: Any, server: Any) -> dict[str, Any]: "color": row.color or None, "created_at": row.created_at, "updated_at": row.updated_at, + "allow_skill_details": bool(getattr(row, "allow_skill_details", True)), } @@ -302,7 +305,7 @@ async def list_published_experts( @router.get("/experts/published/{expert_id}", summary="Get published expert template detail") async def get_published_expert( expert_id: str, - _: Any = Depends(current_user), + user: Any = Depends(current_user), server: Any = Depends(get_server), ) -> dict[str, Any]: """Return published-expert metadata and a previewable snapshot file inventory.""" @@ -312,6 +315,12 @@ async def get_published_expert( files = await asyncio.to_thread(discover_seed_paths, snapshot_dir) if (snapshot_dir / MANIFEST_FILENAME).is_file(): files.insert(0, MANIFEST_FILENAME) + hide_skill_details = not bool( + getattr(row, "allow_skill_details", True) + ) and not _user_can_view_published_skill_details(row, user) + if hide_skill_details: + preview_paths = [p for p in preview_paths if not p.startswith("skills/")] + files = [p for p in files if not p.startswith("skills/")] return { **_published_summary_dict(row, server), "files": files, @@ -323,6 +332,11 @@ async def get_published_expert( } +def _user_can_view_published_skill_details(row: Any, user: Any) -> bool: + """Publishers and admins may always preview a snapshot's skill files.""" + return bool(user.is_admin) or str(user.id) == row.created_by + + @router.post( "/agents/{agent_id}/publish-expert", status_code=201, @@ -356,6 +370,7 @@ async def publish_agent_expert( welcome_message_zh=welcome_zh, welcome_message_en=welcome_en, quick_prompts=tuple(_quick_prompt_body_dict(p) for p in (body.quick_prompts or [])), + allow_skill_details=body.allow_skill_details, ) return _published_summary_dict(row, server) @@ -400,6 +415,7 @@ async def refresh_published_expert( welcome_message_zh=welcome_zh, welcome_message_en=welcome_en, quick_prompts=quick_prompts, + allow_skill_details=(body.allow_skill_details if body is not None else None), ) return _published_summary_dict(updated, server) diff --git a/src/octop/api/routers/skills.py b/src/octop/api/routers/skills.py index 370860af..3cb0935e 100644 --- a/src/octop/api/routers/skills.py +++ b/src/octop/api/routers/skills.py @@ -46,6 +46,10 @@ from octop.api.common.agent import require_agent_owner_row from octop.api.deps import current_user, get_server +from octop.infra.agents.experts.skill_protection import ( + assert_skill_details_visible, + skill_details_protected, +) from octop.infra.agents.manager import ( skill_package_ids_list, ) @@ -480,12 +484,21 @@ async def list_skills( user: Any = Depends(current_user), server: Any = Depends(get_server), ) -> list[dict[str, Any]]: - await _ctx(agent_id, user=user, as_user=as_user, server=server) + ctx = await _ctx(agent_id, user=user, as_user=as_user, server=server) assert server.app_runtime is not None - return cast( + summaries = cast( list[dict[str, Any]], await server.app_runtime.agent_registry.list_skill_summaries(agent_id), ) + # Flag skills installed from a restricted published expert so the UI can + # hide detail/edit affordances; enforcement itself lives in get/update/delete. + for summary in summaries: + slug = str(summary.get("slug") or summary.get("name") or "") + if slug: + summary["protected"] = skill_details_protected( + ctx.config, user=user, services=server.services, slug=slug + ) + return summaries class SkillPackageMountBody(BaseModel): @@ -553,6 +566,7 @@ async def get_skill( server: Any = Depends(get_server), ) -> dict[str, Any]: ctx = await _ctx(agent_id, user=user, as_user=as_user, server=server) + assert_skill_details_visible(ctx.config, user=user, services=server.services, slug=name) resolved = await _resolve_skill(ctx.workspace, name) if resolved is None: raise OctopError(ErrorCode.NOT_FOUND, f"skill {name!r} not found") @@ -653,6 +667,7 @@ async def create_skill( raise OctopError(ErrorCode.SLASH_BAD_ARGS, str(exc)) from exc except SkillPackageError: raise OctopError(ErrorCode.NOT_FOUND, "invalid skill name") from None + assert_skill_details_visible(ctx.config, user=user, services=server.services, slug=name) await _guard_package_only_skill_write(ctx.workspace, ctx.config, server, name) # Conflict check must use SKILL.md — ZIP payloads often list siblings first, # and soft-delete only marks the manifest (leaving sibling files behind). @@ -701,6 +716,7 @@ async def update_skill( except SkillPackageError: raise OctopError(ErrorCode.NOT_FOUND, "invalid skill name") from None + assert_skill_details_visible(ctx.config, user=user, services=server.services, slug=slug) await _guard_package_only_skill_write(ctx.workspace, ctx.config, server, slug) existing = await _aread_text(ctx.workspace, f"skills/{slug}/SKILL.md") if existing is None: @@ -799,6 +815,9 @@ async def import_skill_from_url( bundle_url=bundle_url, version=body.version, ) + assert_skill_details_visible( + ctx.config, user=user, services=server.services, slug=package.slug + ) await commit_skill_install( target, package, @@ -863,6 +882,7 @@ async def delete_skill( slug = validate_skill_slug(name) except SkillPackageError: raise OctopError(ErrorCode.NOT_FOUND, "invalid skill name") from None + assert_skill_details_visible(ctx.config, user=user, services=server.services, slug=slug) await _guard_package_only_skill_write(ctx.workspace, ctx.config, server, slug) resolved = await _resolve_skill(ctx.workspace, slug) if resolved is None: @@ -1224,6 +1244,7 @@ async def hub_install_skill( raise HTTPException(status_code=400, detail="icon_url must be an HTTP(S) URL") ctx = await _ctx(agent_id, user=user, as_user=as_user, server=server) + assert_skill_details_visible(ctx.config, user=user, services=server.services, slug=skill_name) target = _AgentWorkspaceInstallTarget( workspace=ctx.workspace, config=ctx.config, diff --git a/src/octop/api/routers/workspace.py b/src/octop/api/routers/workspace.py index 5761669d..d6ecab1c 100644 --- a/src/octop/api/routers/workspace.py +++ b/src/octop/api/routers/workspace.py @@ -4,6 +4,7 @@ import logging import re +from pathlib import Path from typing import Any, Literal from fastapi import APIRouter, Depends, File, Query, UploadFile @@ -21,6 +22,10 @@ workspace_api_path, ) from octop.api.deps import current_user, get_server +from octop.infra.agents.experts.skill_protection import ( + RESTRICTED_CONFIG_KEY, + assert_workspace_path_allowed, +) from octop.infra.backup.workspace_archive import export_workspace_zip, import_workspace_zip from octop.infra.errors import ErrorCode, OctopError from octop.infra.gateway.media.backend_files import ( @@ -46,6 +51,46 @@ def _assert_workspace_mutable(path: str) -> str: return rel +def _guard_rel_path(io_path: str, ws: Any) -> str | None: + """Normalise *io_path* to a workspace-relative path when it lives in the workspace.""" + if is_host_absolute_path(io_path): + ws_dir = str(getattr(ws, "workspace_dir", "") or "") + if not ws_dir: + return None + try: + rel = Path(io_path).resolve().relative_to(Path(ws_dir).resolve()) + except ValueError: + return None + return rel.as_posix() + return io_path.replace("\\", "/").lstrip("/") + + +def _assert_no_protected_skill_files( + server: Any, + agent_id: str, + user: Any, + ws: Any, + *io_paths: str, +) -> None: + """Reject raw workspace IO that would read or rewrite protected skill files. + + Applies to agents installed from a published expert whose publisher + disabled skill-detail viewing; publishers and admins stay exempt. + """ + try: + config = server.app_runtime.agent_registry.get_config(agent_id) + except Exception: + return + if not isinstance(config, dict) or not config.get(RESTRICTED_CONFIG_KEY): + return + for io_path in io_paths: + if not io_path: + continue + rel = _guard_rel_path(io_path, ws) + if rel: + assert_workspace_path_allowed(config, user=user, services=server.services, rel_path=rel) + + def _map_workspace_fs_error(exc: Exception, *, operation: str, path: str) -> OctopError: if isinstance(exc, BackendOperationNotSupportedError): return OctopError(ErrorCode.WORKSPACE_OP_UNSUPPORTED, str(exc)) @@ -156,7 +201,9 @@ async def read_file( ) -> dict[str, Any]: """Read a UTF-8 text file.""" ws = await require_running_workspace(agent_id, user=user, as_user=as_user, server=server) - content = await ws.aread_text(_workspace_io_path(path, from_workspace=from_workspace)) + io_path = _workspace_io_path(path, from_workspace=from_workspace) + _assert_no_protected_skill_files(server, agent_id, user, ws, io_path) + content = await ws.aread_text(io_path) if content is None: raise OctopError(ErrorCode.NOT_FOUND, f"cannot read {path!r}") return {"path": path, "content": coerce_read_content(content)} @@ -176,6 +223,8 @@ async def write_file( ws = await require_running_workspace( agent_id, user=user, as_user=as_user, server=server, owner_only=True ) + io_path = _workspace_io_path(path, from_workspace=from_workspace) + _assert_no_protected_skill_files(server, agent_id, user, ws, io_path) converter = get_doc_converter(path) if converter is not None: # Editable-document paths are always stored as the binary document @@ -192,7 +241,7 @@ async def write_file( else: data = body.content.encode("utf-8") try: - await ws.aupload_bytes(_workspace_io_path(path, from_workspace=from_workspace), data) + await ws.aupload_bytes(io_path, data) except Exception as exc: raise OctopError(ErrorCode.NOT_FOUND, f"cannot write {path!r}: {exc}") from exc return {"path": path, "size": len(data)} @@ -255,6 +304,7 @@ async def delete_workspace_file( ws = await require_running_workspace( agent_id, user=user, as_user=as_user, server=server, owner_only=True ) + _assert_no_protected_skill_files(server, agent_id, user, ws, rel) try: await ws.adelete(rel) except Exception as exc: @@ -285,6 +335,7 @@ async def move_workspace_file( ws = await require_running_workspace( agent_id, user=user, as_user=as_user, server=server, owner_only=True ) + _assert_no_protected_skill_files(server, agent_id, user, ws, src, dest) try: await ws.amove(src, dest) except Exception as exc: @@ -309,11 +360,10 @@ async def upload_file( ) target = path or f"/{file.filename or 'upload.bin'}" data = await file.read() + io_path = _workspace_io_path(target, from_workspace=from_workspace) + _assert_no_protected_skill_files(server, agent_id, user, ws, io_path) try: - await ws.aupload_bytes( - _workspace_io_path(target, from_workspace=from_workspace), - data, - ) + await ws.aupload_bytes(io_path, data) except Exception as exc: raise OctopError(ErrorCode.NOT_FOUND, f"cannot upload to {target!r}: {exc}") from exc return {"path": target, "size": len(data)} @@ -337,6 +387,7 @@ async def download_file( """ ws = await require_running_workspace(agent_id, user=user, as_user=as_user, server=server) io_path = _workspace_io_path(path, from_workspace=from_workspace) + _assert_no_protected_skill_files(server, agent_id, user, ws, io_path) if is_host_absolute_path(io_path) and not is_allowed_host_download_abs_path( io_path, workspace=ws.workspace_dir, @@ -371,6 +422,7 @@ async def read_doc( converter = _ensure_editable_doc(path) ws = await require_running_workspace(agent_id, user=user, as_user=as_user, server=server) io_path = _workspace_io_path(path, from_workspace=from_workspace) + _assert_no_protected_skill_files(server, agent_id, user, ws, io_path) try: blob = await ws.adownload_bytes(io_path) except PermissionError as exc: @@ -402,6 +454,7 @@ async def write_doc( rel = _assert_workspace_mutable(path) converter = _ensure_editable_doc(path) ws = await require_running_workspace(agent_id, user=user, as_user=as_user, server=server) + _assert_no_protected_skill_files(server, agent_id, user, ws, rel) try: data = converter.from_markdown(body.content) except Exception as exc: @@ -463,6 +516,7 @@ async def glob_files( agent_id, user=user, as_user=as_user, server=server, owner_only=True ) root = _workspace_io_path(path, from_workspace=from_workspace) + _assert_no_protected_skill_files(server, agent_id, user, ws, root) if pattern in ("**/*.md", "*.md") and root == ".": ls_result = await ws.als(".") if ls_result is None: @@ -522,6 +576,8 @@ async def export_workspace_archive( ws = await require_running_workspace( agent_id, user=user, as_user=as_user, server=server, owner_only=True ) + # A full-workspace zip would leak protected skill contents. + _assert_no_protected_skill_files(server, agent_id, user, ws, "skills") data = await export_workspace_zip(ws) filename = f"workspace-{agent_id}.zip" return StreamingResponse( @@ -553,6 +609,8 @@ async def import_workspace_archive( ws = await require_running_workspace( agent_id, user=user, as_user=as_user, server=server, owner_only=True ) + # An imported archive could overwrite protected skill files wholesale. + _assert_no_protected_skill_files(server, agent_id, user, ws, "skills") local_ws = resolve_agent_workspace_dir(server, agent_id) result = await import_workspace_zip( ws, diff --git a/src/octop/i18n/en.json b/src/octop/i18n/en.json index 55c73726..16ce4ef2 100644 --- a/src/octop/i18n/en.json +++ b/src/octop/i18n/en.json @@ -329,6 +329,7 @@ "TLS_DOMAIN_MISMATCH": "The domain does not match the existing certificate.", "WORKSPACE_OP_UNSUPPORTED": "This workspace backend does not support that file operation.", "SKILL_IMPORT_UNSUPPORTED_URL": "This skill URL source is not supported.", + "SKILL_DETAILS_PROTECTED": "The publisher of this expert has disabled viewing or editing its skill contents.", "SKILL_IMPORT_FAILED": "Skill import failed: {reason}", "SKILL_ALREADY_EXISTS": "Skill {name} already exists. Enable overwrite to replace it.", "SKILL_PACKAGE_NOT_FOUND": "Skill package not found.", diff --git a/src/octop/i18n/zh.json b/src/octop/i18n/zh.json index 135df9d4..1a5e7565 100644 --- a/src/octop/i18n/zh.json +++ b/src/octop/i18n/zh.json @@ -329,6 +329,7 @@ "TLS_DOMAIN_MISMATCH": "域名与现有证书不一致。", "WORKSPACE_OP_UNSUPPORTED": "当前工作区后端不支持该文件操作。", "SKILL_IMPORT_UNSUPPORTED_URL": "不支持该技能 URL 来源。", + "SKILL_DETAILS_PROTECTED": "该专家的发布者未开放技能内容的查看与修改权限。", "SKILL_IMPORT_FAILED": "技能导入失败:{reason}", "SKILL_ALREADY_EXISTS": "技能 {name} 已存在,如需覆盖请启用 overwrite。", "SKILL_PACKAGE_NOT_FOUND": "未找到技能包。", diff --git a/src/octop/infra/agents/experts/publish.py b/src/octop/infra/agents/experts/publish.py index 20227590..67f32620 100644 --- a/src/octop/infra/agents/experts/publish.py +++ b/src/octop/infra/agents/experts/publish.py @@ -62,6 +62,7 @@ class PublishedExpertSnapshotMeta: welcome_message_zh: str welcome_message_en: str quick_prompts: tuple[dict[str, Any], ...] = () + allow_skill_details: bool = True def assert_can_mutate_published(row: PublishedExpertRow, user: User) -> None: @@ -207,6 +208,7 @@ def _manifest_from_metadata( data["color"] = metadata.color if metadata.quick_prompts: data["quick_prompts"] = list(metadata.quick_prompts) + data["allow_skill_details"] = bool(metadata.allow_skill_details) return json.dumps(data, ensure_ascii=False).encode("utf-8") diff --git a/src/octop/infra/agents/experts/published_creation.py b/src/octop/infra/agents/experts/published_creation.py index 36d07de9..1e9fae8a 100644 --- a/src/octop/infra/agents/experts/published_creation.py +++ b/src/octop/infra/agents/experts/published_creation.py @@ -19,6 +19,11 @@ export_agent_workspace_to_dir, resolve_published_expert_slug, ) +from octop.infra.agents.experts.skill_protection import ( + PROTECTED_SKILLS_KEY, + RESTRICTED_CONFIG_KEY, + snapshot_protected_skill_slugs, +) from octop.infra.agents.manager import AgentCreateSpec from octop.infra.db.repos.published_experts import PublishedExpertRow from octop.infra.errors import ErrorCode, OctopError @@ -92,6 +97,7 @@ def _snapshot_meta( welcome_message_zh: str = "", welcome_message_en: str = "", quick_prompts: tuple[dict[str, Any], ...] = (), + allow_skill_details: bool = True, ) -> PublishedExpertSnapshotMeta: return PublishedExpertSnapshotMeta( name=name, @@ -103,6 +109,7 @@ def _snapshot_meta( welcome_message_zh=welcome_message_zh, welcome_message_en=welcome_message_en, quick_prompts=quick_prompts, + allow_skill_details=allow_skill_details, ) @@ -126,6 +133,7 @@ async def publish_agent_expert( welcome_message_zh: str = "", welcome_message_en: str = "", quick_prompts: tuple[dict[str, Any], ...] = (), + allow_skill_details: bool = True, ) -> PublishedExpertRow: """Snapshot an owned agent workspace into a globally installable expert template.""" repo = services.published_expert_repo @@ -155,6 +163,7 @@ async def publish_agent_expert( welcome_message_zh=welcome_message_zh, welcome_message_en=welcome_message_en, quick_prompts=quick_prompts, + allow_skill_details=allow_skill_details, ), manifest_id=resolved_slug, ) @@ -169,6 +178,7 @@ async def publish_agent_expert( source_agent_id=source.agent_id, icon_name=icon_name, color=color, + allow_skill_details=allow_skill_details, ), ) except (SqliteIntegrityError, PsycopgIntegrityError) as exc: @@ -196,6 +206,7 @@ async def refresh_published_expert( welcome_message_zh: str | None = None, welcome_message_en: str | None = None, quick_prompts: tuple[dict[str, Any], ...] | None = None, + allow_skill_details: bool | None = None, ) -> PublishedExpertRow: """Replace a published snapshot using its still-owned source agent workspace.""" row = require_published_expert(services, expert_id) @@ -218,6 +229,9 @@ async def refresh_published_expert( welcome_message_en if welcome_message_en is not None else existing_welcome_en ) resolved_quick_prompts = quick_prompts if quick_prompts is not None else existing_quick_prompts + resolved_allow_skill_details = ( + allow_skill_details if allow_skill_details is not None else row.allow_skill_details + ) await export_agent_workspace_to_dir( workspace=workspace, dest=snapshot_dir, @@ -229,6 +243,7 @@ async def refresh_published_expert( welcome_message_zh=resolved_welcome_zh, welcome_message_en=resolved_welcome_en, quick_prompts=resolved_quick_prompts, + allow_skill_details=resolved_allow_skill_details, ), manifest_id=row.slug, ) @@ -240,6 +255,7 @@ async def refresh_published_expert( color=color, name=resolved_name, description=resolved_description, + allow_skill_details=resolved_allow_skill_details, ), ) @@ -275,6 +291,13 @@ async def install_published_expert( registry.assert_backend_supports_skill_packages(options.backend) config_extra: dict[str, Any] = {"published_expert_id": row.id} + if not row.allow_skill_details: + # Stamp the restriction into the fork's config so skill detail APIs can + # enforce it even after the expert is later refreshed or unpublished. + config_extra[RESTRICTED_CONFIG_KEY] = True + config_extra[PROTECTED_SKILLS_KEY] = await asyncio.to_thread( + snapshot_protected_skill_slugs, snapshot_dir + ) if options.providers: config_extra["providers"] = list(options.providers) if options.backend: diff --git a/src/octop/infra/agents/experts/skill_protection.py b/src/octop/infra/agents/experts/skill_protection.py new file mode 100644 index 00000000..cb0d82bf --- /dev/null +++ b/src/octop/infra/agents/experts/skill_protection.py @@ -0,0 +1,124 @@ +"""Guards for skills shipped by experts published with hidden skill details. + +When an expert is published with ``allow_skill_details = False``, users who +install it may *use* its skills but must not view or modify their content. +The restriction is stamped into the installed agent's config at install time +(``skill_details_restricted`` + ``protected_skills``) so enforcement survives +the expert later being refreshed or unpublished. + +The publishing user (and admins) stay exempt: they can always inspect skills +of experts they published. +""" + +from __future__ import annotations + +from pathlib import Path, PurePosixPath +from typing import Any + +from octop.infra.db.repos.published_experts import PublishedExpertRow +from octop.infra.errors import ErrorCode, OctopError +from octop.infra.users.identity import User + +RESTRICTED_CONFIG_KEY = "skill_details_restricted" +PROTECTED_SKILLS_KEY = "protected_skills" +PUBLISHED_EXPERT_ID_KEY = "published_expert_id" + + +def protected_slugs_from_config(config: dict[str, Any] | None) -> frozenset[str]: + """Return the skill slugs an installed agent must keep opaque.""" + if not isinstance(config, dict) or not config.get(RESTRICTED_CONFIG_KEY): + return frozenset() + raw = config.get(PROTECTED_SKILLS_KEY) + if not isinstance(raw, list): + return frozenset() + return frozenset(str(slug) for slug in raw if slug) + + +def snapshot_protected_skill_slugs(snapshot_dir: Path) -> list[str]: + """List ``skills//`` directories present in a published snapshot.""" + skills_root = snapshot_dir / "skills" + if not skills_root.is_dir(): + return [] + return sorted( + entry.name + for entry in skills_root.iterdir() + if entry.is_dir() and (entry / "SKILL.md").is_file() + ) + + +def _is_exempt(config: dict[str, Any], user: User, services: Any) -> bool: + """The expert's publisher and admins may always view skill details.""" + if user.is_admin: + return True + expert_id = config.get(PUBLISHED_EXPERT_ID_KEY) + if not expert_id or services is None: + return False + row: PublishedExpertRow | None = services.published_expert_repo.get(str(expert_id)) + return row is not None and str(user.id) == row.created_by + + +def _restricted(config: dict[str, Any] | None) -> bool: + return isinstance(config, dict) and bool(config.get(RESTRICTED_CONFIG_KEY)) + + +def skill_details_protected( + config: dict[str, Any] | None, + *, + user: User, + services: Any, + slug: str, +) -> bool: + """Return True when *slug* must stay opaque for this user.""" + if not _restricted(config): + return False + if slug not in protected_slugs_from_config(config): + return False + return not _is_exempt(config if isinstance(config, dict) else {}, user, services) + + +def assert_skill_details_visible( + config: dict[str, Any] | None, + *, + user: User, + services: Any, + slug: str, +) -> None: + """Raise ``SKILL_DETAILS_PROTECTED`` when the user may not view/modify *slug*.""" + if skill_details_protected(config, user=user, services=services, slug=slug): + raise OctopError( + ErrorCode.SKILL_DETAILS_PROTECTED, + f"skill {slug!r} details are protected by the expert publisher", + details={"slug": slug}, + ) + + +def assert_workspace_path_allowed( + config: dict[str, Any] | None, + *, + user: User, + services: Any, + rel_path: str, +) -> None: + """Guard raw workspace file access (read/write/move/delete/download). + + *rel_path* is workspace-relative with forward slashes (``skills/a/b.md``). + Blocking the whole ``skills`` root as well prevents deleting / moving the + tree out from under the protected skills. + """ + if not _restricted(config): + return + parts = PurePosixPath(rel_path.strip().replace("\\", "/").lstrip("/")).parts + if not parts or parts[0] != "skills": + return + protected = protected_slugs_from_config(config) + if not protected: + return + cfg = config if isinstance(config, dict) else {} + if (len(parts) == 1 or (len(parts) > 1 and parts[1] in protected)) and not _is_exempt( + cfg, user, services + ): + raise OctopError( + ErrorCode.SKILL_DETAILS_PROTECTED, + "workspace skill files are protected by the expert publisher", + details={"path": rel_path}, + ) diff --git a/src/octop/infra/db/migrate.py b/src/octop/infra/db/migrate.py index 719b1bbf..acd83387 100644 --- a/src/octop/infra/db/migrate.py +++ b/src/octop/infra/db/migrate.py @@ -356,6 +356,7 @@ def _repair_legacy_schema(db: DatabasePool) -> None: _ensure_skill_packages_name_unique_index(db) _ensure_published_experts_table(db) _ensure_published_experts_indexes(db) + _ensure_column(db, "published_experts", "allow_skill_details", "INTEGER NOT NULL DEFAULT 1") # Cover pre-squash develop DBs that already recorded version ≥5 but only # applied a subset of the former 005–009 files (or the old thin 005). # Require ``users`` first — SSO rebuild and knowledge FKs need it, and a @@ -416,6 +417,8 @@ def _apply_sqlite_migration(db: DatabasePool, version: int, path: Path) -> None: bases idempotently after legacy schema repair. Version 6 adds ``users.permissions`` idempotently (also covered by ``_repair_legacy_schema`` for DBs whose version was clamped past 006). + Version 7 adds ``published_experts.allow_skill_details`` idempotently + (``_repair_legacy_schema`` may have added it before 007 runs). """ if version == 2: if _table_exists(db, "cron_jobs"): @@ -470,6 +473,17 @@ def _apply_sqlite_migration(db: DatabasePool, version: int, path: Path) -> None: with db.connect() as conn: conn.execute("UPDATE _schema_version SET version = ?", (version,)) return + if version == 7: + if _table_exists(db, "published_experts"): + _ensure_column( + db, + "published_experts", + "allow_skill_details", + "INTEGER NOT NULL DEFAULT 1", + ) + with db.connect() as conn: + conn.execute("UPDATE _schema_version SET version = ?", (version,)) + return sql = path.read_text(encoding="utf-8") with db.connect() as conn: conn.executescript(sql) diff --git a/src/octop/infra/db/migrations/007_published_expert_skill_details.pg.sql b/src/octop/infra/db/migrations/007_published_expert_skill_details.pg.sql new file mode 100644 index 00000000..550e33e0 --- /dev/null +++ b/src/octop/infra/db/migrations/007_published_expert_skill_details.pg.sql @@ -0,0 +1,3 @@ +ALTER TABLE published_experts ADD COLUMN allow_skill_details BOOLEAN NOT NULL DEFAULT TRUE; + +UPDATE _schema_version SET version = 7; diff --git a/src/octop/infra/db/migrations/007_published_expert_skill_details.sql b/src/octop/infra/db/migrations/007_published_expert_skill_details.sql new file mode 100644 index 00000000..59eed62c --- /dev/null +++ b/src/octop/infra/db/migrations/007_published_expert_skill_details.sql @@ -0,0 +1,4 @@ +-- Schema v7: per-expert control over whether installers may view skill details. +ALTER TABLE published_experts ADD COLUMN allow_skill_details INTEGER NOT NULL DEFAULT 1; + +UPDATE _schema_version SET version = 7; diff --git a/src/octop/infra/db/repos/published_experts.py b/src/octop/infra/db/repos/published_experts.py index c8c95a82..a87d2e01 100644 --- a/src/octop/infra/db/repos/published_experts.py +++ b/src/octop/infra/db/repos/published_experts.py @@ -20,9 +20,13 @@ class PublishedExpertRow: color: str created_at: int updated_at: int + allow_skill_details: bool = True @classmethod def from_row(cls, r: DbRow) -> PublishedExpertRow: + # sqlite3.Row's ``in`` checks values, not column names - use ``.keys()``. + keys = frozenset(r.keys()) if hasattr(r, "keys") else frozenset() + allow = r["allow_skill_details"] if "allow_skill_details" in keys else 1 return cls( id=r["id"], slug=r["slug"], @@ -34,6 +38,7 @@ def from_row(cls, r: DbRow) -> PublishedExpertRow: color=r["color"], created_at=int(r["created_at"]), updated_at=int(r["updated_at"]), + allow_skill_details=bool(allow), ) @@ -81,14 +86,15 @@ def create( source_agent_id: str | None = None, icon_name: str = "", color: str = "", + allow_skill_details: bool = True, ) -> PublishedExpertRow: ts = now_ts() with self._db.transaction() as conn: conn.execute( "INSERT INTO published_experts(" "id, slug, name, description, created_by, source_agent_id, " - "icon_name, color, created_at, updated_at" - ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", + "icon_name, color, allow_skill_details, created_at, updated_at" + ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?)", ( id, slug, @@ -98,6 +104,7 @@ def create( source_agent_id, icon_name, color, + 1 if allow_skill_details else 0, ts, ts, ), @@ -115,6 +122,7 @@ def update_snapshot_meta( color: str | None = None, description: str | None = None, name: str | None = None, + allow_skill_details: bool | None = None, ) -> PublishedExpertRow: """Refresh listing metadata and bump ``updated_at`` after a snapshot rewrite.""" fields: list[str] = ["updated_at = ?"] @@ -131,6 +139,9 @@ def update_snapshot_meta( if name is not None: fields.append("name = ?") values.append(name) + if allow_skill_details is not None: + fields.append("allow_skill_details = ?") + values.append(1 if allow_skill_details else 0) values.append(expert_id) with self._db.transaction() as conn: conn.execute( diff --git a/src/octop/infra/errors.py b/src/octop/infra/errors.py index dea8315d..a7610768 100644 --- a/src/octop/infra/errors.py +++ b/src/octop/infra/errors.py @@ -66,6 +66,7 @@ class ErrorCode(StrEnum): SKILL_PACKAGE_NOT_FOUND = "SKILL_PACKAGE_NOT_FOUND" SKILL_PACKAGE_NAME_TAKEN = "SKILL_PACKAGE_NAME_TAKEN" SKILL_PACKAGE_BACKEND_UNSUPPORTED = "SKILL_PACKAGE_BACKEND_UNSUPPORTED" + SKILL_DETAILS_PROTECTED = "SKILL_DETAILS_PROTECTED" PUBLISHED_EXPERT_SLUG_TAKEN = "PUBLISHED_EXPERT_SLUG_TAKEN" PUBLISHED_EXPERT_ALREADY_EXISTS = "PUBLISHED_EXPERT_ALREADY_EXISTS" OIDC_BAD_REQUEST = "OIDC_BAD_REQUEST" @@ -143,6 +144,7 @@ class ErrorCode(StrEnum): ErrorCode.SKILL_PACKAGE_NOT_FOUND: 404, ErrorCode.SKILL_PACKAGE_NAME_TAKEN: 409, ErrorCode.SKILL_PACKAGE_BACKEND_UNSUPPORTED: 400, + ErrorCode.SKILL_DETAILS_PROTECTED: 403, ErrorCode.PUBLISHED_EXPERT_SLUG_TAKEN: 409, ErrorCode.PUBLISHED_EXPERT_ALREADY_EXISTS: 409, ErrorCode.OIDC_BAD_REQUEST: 400, diff --git a/tests/integration/test_published_experts.py b/tests/integration/test_published_experts.py index e3f0b4b1..093c28d7 100644 --- a/tests/integration/test_published_experts.py +++ b/tests/integration/test_published_experts.py @@ -4,7 +4,7 @@ from typing import Any -from tests.support.auth import create_user +from tests.support.auth import create_agent, create_user async def _owner_and_peer( @@ -162,3 +162,113 @@ async def test_admin_cannot_publish_another_users_agent( json={"name": "Admin must not publish"}, ) assert published.status_code == 403, published.text + + +async def test_publish_without_skill_details_blocks_peer_view_and_edit( + env_with_provider: tuple[Any, Any, dict[str, str]], +) -> None: + client, server, admin_auth = env_with_provider + owner_auth = await create_user(client, admin_auth, username="guarded_owner") + peer_auth = await create_user(client, admin_auth, username="guarded_peer") + source_agent_id = await create_agent(client, owner_auth) + + # Seed a workspace skill directly via the workspace backend so the test + # does not depend on the source agent being started. + workspace = server.app_runtime.agent_registry.workspace_for_agent(source_agent_id) + assert workspace is not None + await workspace.aupload_many( + [ + ( + "skills/secret-flow/SKILL.md", + ( + b"---\nname: secret-flow\ndescription: Secret workflow\n---\n" + b"# Secret steps\nDo the thing.\n" + ), + ) + ] + ) + + published = await client.post( + f"/api/agents/{source_agent_id}/publish-expert", + headers=owner_auth, + json={"name": "Guarded expert", "allow_skill_details": False}, + ) + assert published.status_code == 201, published.text + assert published.json()["allow_skill_details"] is False + expert_id = published.json()["id"] + + # Snapshot preview hides skill files from peers but not from the publisher. + peer_detail = await client.get(f"/api/experts/published/{expert_id}", headers=peer_auth) + assert peer_detail.status_code == 200, peer_detail.text + assert not any(p.startswith("skills/") for p in peer_detail.json()["files"]) + owner_detail = await client.get(f"/api/experts/published/{expert_id}", headers=owner_auth) + assert any(p.startswith("skills/") for p in owner_detail.json()["files"]) + + installed = await client.post( + f"/api/experts/published/{expert_id}/install", + headers=peer_auth, + json={"name": "Peer guarded install"}, + ) + assert installed.status_code == 201, installed.text + agent_id = installed.json()["agent_id"] + + listed = await client.get(f"/api/agents/{agent_id}/skills", headers=peer_auth) + assert listed.status_code == 200, listed.text + summary = next(s for s in listed.json() if s.get("slug") == "secret-flow") + assert summary["protected"] is True + + detail = await client.get(f"/api/agents/{agent_id}/skills/secret-flow", headers=peer_auth) + assert detail.status_code == 403, detail.text + assert detail.json()["error"]["code"] == "SKILL_DETAILS_PROTECTED" + + updated = await client.put( + f"/api/agents/{agent_id}/skills/secret-flow", + headers=peer_auth, + json={"content": "---\nname: secret-flow\ndescription: hacked\n---\n"}, + ) + assert updated.status_code == 403, updated.text + deleted = await client.delete(f"/api/agents/{agent_id}/skills/secret-flow", headers=peer_auth) + assert deleted.status_code == 403, deleted.text + + # Raw workspace file access must not bypass the guard. + raw_read = await client.get( + f"/api/agents/{agent_id}/workspace/file", + headers=peer_auth, + params={"path": "skills/secret-flow/SKILL.md", "from_workspace": True}, + ) + assert raw_read.status_code == 403, raw_read.text + + # The publisher installing their own expert keeps full access. + own_install = await client.post( + f"/api/experts/published/{expert_id}/install", + headers=owner_auth, + json={"name": "Owner guarded install"}, + ) + assert own_install.status_code == 201, own_install.text + own_agent = own_install.json()["agent_id"] + own_detail = await client.get(f"/api/agents/{own_agent}/skills/secret-flow", headers=owner_auth) + assert own_detail.status_code == 200, own_detail.text + own_list = await client.get(f"/api/agents/{own_agent}/skills", headers=owner_auth) + own_summary = next(s for s in own_list.json() if s.get("slug") == "secret-flow") + assert own_summary["protected"] is False + + +async def test_refresh_can_toggle_allow_skill_details( + env: tuple[Any, Any, dict[str, str]], +) -> None: + client, _server, owner_auth, _peer_auth, source_agent_id = await _owner_and_peer(env) + published = await client.post( + f"/api/agents/{source_agent_id}/publish-expert", + headers=owner_auth, + json={"name": "Toggleable expert", "allow_skill_details": False}, + ) + assert published.status_code == 201, published.text + assert published.json()["allow_skill_details"] is False + + refreshed = await client.post( + f"/api/experts/published/{published.json()['id']}/refresh", + headers=owner_auth, + json={"allow_skill_details": True}, + ) + assert refreshed.status_code == 200, refreshed.text + assert refreshed.json()["allow_skill_details"] is True diff --git a/tests/unit/agents/test_expert_publish_snapshot.py b/tests/unit/agents/test_expert_publish_snapshot.py index ee49355d..6d217ab2 100644 --- a/tests/unit/agents/test_expert_publish_snapshot.py +++ b/tests/unit/agents/test_expert_publish_snapshot.py @@ -387,4 +387,5 @@ async def test_export_snapshot_builds_manifest_from_publish_metadata(tmp_path: P "icon_name": "search", "color": "#123456", "prompt_files": ["SOUL.md"], + "allow_skill_details": True, } diff --git a/tests/unit/agents/test_skill_protection.py b/tests/unit/agents/test_skill_protection.py new file mode 100644 index 00000000..293e927f --- /dev/null +++ b/tests/unit/agents/test_skill_protection.py @@ -0,0 +1,137 @@ +"""Unit tests for published-expert skill-detail protection guards.""" + +from __future__ import annotations + +from pathlib import Path +from types import SimpleNamespace +from typing import Any + +import pytest + +from octop.infra.agents.experts.skill_protection import ( + assert_skill_details_visible, + assert_workspace_path_allowed, + protected_slugs_from_config, + skill_details_protected, + snapshot_protected_skill_slugs, +) +from octop.infra.errors import ErrorCode, OctopError +from octop.infra.users.identity import Role, User + + +def _user(uid: int, *, admin: bool = False) -> User: + return User( + id=uid, + username=f"user{uid}", + role=Role.ADMIN if admin else Role.USER, + display_name=None, + ) + + +class _Repo: + def __init__(self, rows: dict[str, Any]) -> None: + self._rows = rows + + def get(self, expert_id: str) -> Any: + return self._rows.get(expert_id) + + +def _services(created_by: str | None = "7") -> Any: + row = None if created_by is None else SimpleNamespace(id="e1", created_by=created_by) + return SimpleNamespace(published_expert_repo=_Repo({"e1": row})) + + +RESTRICTED: dict[str, Any] = { + "skill_details_restricted": True, + "protected_skills": ["secret-flow"], + "published_expert_id": "e1", +} + + +def test_unrestricted_config_has_no_protected_slugs() -> None: + assert protected_slugs_from_config({}) == frozenset() + assert protected_slugs_from_config({"protected_skills": ["a"]}) == frozenset() + + +def test_skill_details_protected_respects_membership_and_exempt_users() -> None: + peer = _user(9) + publisher = _user(7) + admin = _user(8, admin=True) + + assert skill_details_protected(RESTRICTED, user=peer, services=_services(), slug="secret-flow") + assert not skill_details_protected( + RESTRICTED, user=publisher, services=_services(), slug="secret-flow" + ) + assert not skill_details_protected( + RESTRICTED, user=admin, services=_services(), slug="secret-flow" + ) + # Non-protected skills stay fully usable / editable. + assert not skill_details_protected( + RESTRICTED, user=peer, services=_services(), slug="my-own-skill" + ) + # Once the expert is unpublished the stamped restriction persists for + # everyone - the config is the source of truth, not the live row. + assert skill_details_protected( + RESTRICTED, user=publisher, services=_services(created_by=None), slug="secret-flow" + ) + + +def test_assert_skill_details_visible_raises_for_protected_slug() -> None: + with pytest.raises(OctopError) as exc: + assert_skill_details_visible( + RESTRICTED, user=_user(9), services=_services(), slug="secret-flow" + ) + assert exc.value.code is ErrorCode.SKILL_DETAILS_PROTECTED + assert exc.value.status == 403 + + # No-op for unrestricted agents / non-protected slugs. + assert_skill_details_visible({}, user=_user(9), services=_services(), slug="x") + assert_skill_details_visible(RESTRICTED, user=_user(9), services=_services(), slug="other") + + +@pytest.mark.parametrize( + ("rel_path", "blocked"), + [ + ("skills/secret-flow/SKILL.md", True), + ("skills/secret-flow", True), + ("skills", True), # whole-tree mutation (delete / move / archive) + ("skills/my-own-skill/SKILL.md", False), + ("SOUL.md", False), + ("daily/2026-01-01.md", False), + ("agents/reviewer.md", False), + ], +) +def test_workspace_path_guard(rel_path: str, blocked: bool) -> None: + peer = _user(9) + if blocked: + with pytest.raises(OctopError) as exc: + assert_workspace_path_allowed( + RESTRICTED, user=peer, services=_services(), rel_path=rel_path + ) + assert exc.value.code is ErrorCode.SKILL_DETAILS_PROTECTED + else: + assert_workspace_path_allowed( + RESTRICTED, user=peer, services=_services(), rel_path=rel_path + ) + + +def test_workspace_path_guard_allows_publisher_and_ignores_unrestricted() -> None: + assert_workspace_path_allowed( + RESTRICTED, + user=_user(7), + services=_services(), + rel_path="skills/secret-flow/SKILL.md", + ) + assert_workspace_path_allowed( + {}, user=_user(9), services=_services(), rel_path="skills/secret-flow/SKILL.md" + ) + + +def test_snapshot_protected_skill_slugs_lists_only_skill_dirs(tmp_path: Path) -> None: + (tmp_path / "skills" / "alpha").mkdir(parents=True) + (tmp_path / "skills" / "alpha" / "SKILL.md").write_text("x", encoding="utf-8") + (tmp_path / "skills" / "beta").mkdir(parents=True) # no SKILL.md + (tmp_path / "skills" / "loose.md").write_text("x", encoding="utf-8") + + assert snapshot_protected_skill_slugs(tmp_path) == ["alpha"] + assert snapshot_protected_skill_slugs(tmp_path / "missing") == [] diff --git a/tests/unit/db/test_clip_thread_title.py b/tests/unit/db/test_clip_thread_title.py index 657bb1a7..8c4d0775 100644 --- a/tests/unit/db/test_clip_thread_title.py +++ b/tests/unit/db/test_clip_thread_title.py @@ -82,7 +82,7 @@ def test_migration_003_repairs_stored_hard_cuts(tmp_path: Path) -> None: with pool.connect() as conn: v = conn.execute("SELECT version FROM _schema_version").fetchone()[0] title = conn.execute("SELECT title FROM threads WHERE thread_id = ?", ("t1",)).fetchone()[0] - assert v == 6 + assert v == 7 assert title == "x" * 39 + "…" # Idempotent repair assert repair_all_legacy_thread_titles(pool) == 0 diff --git a/tests/unit/db/test_db_pool.py b/tests/unit/db/test_db_pool.py index 76353bce..d2d43ad9 100644 --- a/tests/unit/db/test_db_pool.py +++ b/tests/unit/db/test_db_pool.py @@ -76,7 +76,7 @@ def test_run_migrations_idempotent(db: SqlitePool): cols = {r["name"] for r in conn.execute("PRAGMA table_info(users)").fetchall()} cron_cols = {r["name"] for r in conn.execute("PRAGMA table_info(cron_jobs)").fetchall()} thread_cols = {r["name"] for r in conn.execute("PRAGMA table_info(threads)").fetchall()} - assert v == 6 + assert v == 7 assert "login_failed_count" in cols assert "login_locked_until" in cols assert "preferences_json" in cols @@ -126,7 +126,7 @@ def test_migration_002_idempotent_when_column_already_present(tmp_path: Path) -> with pool.connect() as conn: v = conn.execute("SELECT version FROM _schema_version").fetchone()[0] cron_cols = {r["name"] for r in conn.execute("PRAGMA table_info(cron_jobs)").fetchall()} - assert v == 6 + assert v == 7 assert "mcp_servers" in cron_cols assert "skill_packages" in { r["name"] @@ -263,7 +263,7 @@ def test_stuck_version_6_without_permissions_column_is_repaired(tmp_path: Path) with pool.connect() as conn: cols = {r["name"] for r in conn.execute("PRAGMA table_info(users)").fetchall()} version = conn.execute("SELECT version FROM _schema_version").fetchone()[0] - assert version == 6 + assert version == 7 assert "permissions" in cols @@ -300,8 +300,12 @@ def test_pre_squash_schema_version_clamped_and_knowledge_tables_filled( for row in conn.execute("SELECT name FROM sqlite_master WHERE type='table'").fetchall() } user_cols = {r["name"] for r in conn.execute("PRAGMA table_info(users)").fetchall()} - assert version == 6 + expert_cols = { + r["name"] for r in conn.execute("PRAGMA table_info(published_experts)").fetchall() + } + assert version == 7 assert "permissions" in user_cols + assert "allow_skill_details" in expert_cols assert { "published_experts", "sso_providers", diff --git a/tests/unit/db/test_published_experts_repo.py b/tests/unit/db/test_published_experts_repo.py index fc5a6927..8b8fa325 100644 --- a/tests/unit/db/test_published_experts_repo.py +++ b/tests/unit/db/test_published_experts_repo.py @@ -26,7 +26,7 @@ def test_published_experts_table_exists(db: SqlitePool) -> None: } v = conn.execute("SELECT version FROM _schema_version").fetchone()[0] assert "published_experts" in names - assert v == 6 + assert v == 7 def test_published_expert_repo_create_get_list_delete(db: SqlitePool) -> None: @@ -105,3 +105,30 @@ def test_published_expert_repo_get_by_source_and_update_meta(db: SqlitePool) -> assert updated.color == "#abcdef" assert updated.updated_at >= row.updated_at assert updated.updated_at >= row.created_at + + +def test_published_expert_repo_allow_skill_details(db: SqlitePool) -> None: + repo = PublishedExpertRepo(db) + row = repo.create( + id="01TESTEXPERT000000000004", + slug="guarded", + name="Guarded", + created_by="user-1", + allow_skill_details=False, + ) + assert row.allow_skill_details is False + assert repo.get(row.id).allow_skill_details is False + + # Default remains permissive for legacy rows / omitted flag. + permissive = repo.create( + id="01TESTEXPERT000000000005", + slug="open", + name="Open", + created_by="user-1", + ) + assert permissive.allow_skill_details is True + + toggled = repo.update_snapshot_meta(row.id, allow_skill_details=True) + assert toggled.allow_skill_details is True + untouched = repo.update_snapshot_meta(permissive.id, icon_name="zap") + assert untouched.allow_skill_details is True diff --git a/tests/unit/db/test_repo_knowledge.py b/tests/unit/db/test_repo_knowledge.py index 4b34a14b..cd22f107 100644 --- a/tests/unit/db/test_repo_knowledge.py +++ b/tests/unit/db/test_repo_knowledge.py @@ -51,7 +51,7 @@ def test_knowledge_tables_migrated(db: SqlitePool) -> None: "knowledge_base_members", "knowledge_documents", }.issubset(names) - assert v == 6 + assert v == 7 def test_path_layout_knowledge_dir(tmp_path: Path) -> None: diff --git a/tests/unit/db/test_skill_package_icons.py b/tests/unit/db/test_skill_package_icons.py index e08a3b85..aabdd30f 100644 --- a/tests/unit/db/test_skill_package_icons.py +++ b/tests/unit/db/test_skill_package_icons.py @@ -91,7 +91,7 @@ def test_migration_002_idempotent_when_icon_columns_already_present(tmp_path: Pa "SELECT name FROM sqlite_master WHERE type='index' AND tbl_name='skill_packages'" ) } - assert v == 6 + assert v == 7 assert "icon_name" in cols assert "icon_url" in cols assert "idx_skill_packages_name" in indexes @@ -109,6 +109,6 @@ def test_repair_legacy_schema_adds_icon_columns_at_version_2(tmp_path: Path) -> with pool.connect() as conn: v = conn.execute("SELECT version FROM _schema_version").fetchone()[0] cols = {r["name"] for r in conn.execute("PRAGMA table_info(skill_packages)").fetchall()} - assert v == 6 + assert v == 7 assert "icon_name" in cols assert "icon_url" in cols diff --git a/tests/unit/db/test_skill_packages_repo.py b/tests/unit/db/test_skill_packages_repo.py index e20e6139..cd47fcde 100644 --- a/tests/unit/db/test_skill_packages_repo.py +++ b/tests/unit/db/test_skill_packages_repo.py @@ -25,7 +25,7 @@ def test_skill_packages_table_exists(db: SqlitePool) -> None: } v = conn.execute("SELECT version FROM _schema_version").fetchone()[0] assert "skill_packages" in names - assert v == 6 + assert v == 7 def test_skill_package_repo_create_get(db: SqlitePool) -> None: