diff --git a/docs/adr/0018-agent-execution-surface-bounded-by-workspace.md b/docs/adr/0018-agent-execution-surface-bounded-by-workspace.md index 9fc181c..0c315f5 100644 --- a/docs/adr/0018-agent-execution-surface-bounded-by-workspace.md +++ b/docs/adr/0018-agent-execution-surface-bounded-by-workspace.md @@ -123,15 +123,23 @@ The boundary is enforced by the Claude Code SDK's built-in sandbox workspace or service-user config from widening tools, hooks, MCP servers, or sandbox paths. - Agent skills are Organization-scoped runtime configuration, not Hub release - assets. A controlled host-console installer imports each version into a - content-addressed persistent store and records its digest in PostgreSQL. A - role selects enabled Organization skills alongside its model, system prompt - and tool allowlist. Each run copies only those selected immutable versions - into a run-scoped plugin outside the project workspace; the sandbox exposes - that snapshot read-only and deletes it after the run. SDK-bundled skills and - filesystem setting sources remain disabled, so project `.claude` content - cannot register skills or widen tools. Requested skill versions are recorded - on `run.created`; SDK initialization remains authoritative loading evidence. + assets. Skill content is imported into a content-addressed persistent store + through a shared ingestion pipeline (`importSkillDirectory` for host-console + CLI, `importSkillFromFiles` for org-admin web surface) that enforces the same + safety checks: `SKILL.md` manifest required, 512-file / 16-byte limits, + symlink rejection, SHA-256 content addressing. The web surface + (`OrganizationAgentConfiguration.installSkillFromFiles`) writes to the same + store as the host-console CLI (`installSkill`); both flow through + `commitSkillContent` for deduplication and atomic rename into + `versions//`. Org-admin authentication gates the web surface; the + content-addressed store remains platform-controlled. A role selects enabled + Organization skills alongside its model, system prompt and tool allowlist. + Each run copies only those selected immutable versions into a run-scoped + plugin outside the project workspace; the sandbox exposes that snapshot + read-only and deletes it after the run. SDK-bundled skills and filesystem + setting sources remain disabled, so project `.claude` content cannot register + skills or widen tools. Requested skill versions are recorded on + `run.created`; SDK initialization remains authoritative loading evidence. - Network: open (see Open Questions). `bypassPermissions` is kept (headless server — no interactive prompts); the diff --git a/hub/admin-web/src/lib/api.ts b/hub/admin-web/src/lib/api.ts index cc1614a..a037efb 100644 --- a/hub/admin-web/src/lib/api.ts +++ b/hub/admin-web/src/lib/api.ts @@ -253,6 +253,17 @@ export interface AgentSkillRow { boundRoleIds: readonly string[]; } +export interface SkillFileEntry { + path: string; + content: string; +} + +export interface InstalledSkillResult { + id: string; + name: string; + contentDigest: string; +} + export interface AgentModelRow { id: string; label: string; @@ -392,5 +403,11 @@ export const api = { skillNames: string[]; }>, agentSkills: (slug: string) => get(`${orgBase(slug)}/agent-skills`) as Promise<{ skills: AgentSkillRow[] }>, + agentSkillFiles: (slug: string, name: string) => + get(`${orgBase(slug)}/agent-skills/${encodeURIComponent(name)}/files`) as Promise<{ files: SkillFileEntry[] }>, + installAgentSkill: (slug: string, name: string, body: { version: string; files: readonly SkillFileEntry[] }) => + put(`${orgBase(slug)}/agent-skills/${encodeURIComponent(name)}`, body) as Promise, + patchAgentSkill: (slug: string, name: string, body: { description?: string; disabled?: boolean }) => + patch(`${orgBase(slug)}/agent-skills/${encodeURIComponent(name)}`, body) as Promise<{ disabled?: boolean; updated?: boolean }>, agentModels: (slug: string) => get(`${orgBase(slug)}/agent-models`) as Promise<{ models: AgentModelRow[] }>, }; diff --git a/hub/admin-web/src/lib/components/SkillEditor.svelte b/hub/admin-web/src/lib/components/SkillEditor.svelte new file mode 100644 index 0000000..3ff6a06 --- /dev/null +++ b/hub/admin-web/src/lib/components/SkillEditor.svelte @@ -0,0 +1,305 @@ + + +
+
+ {skill.name} + v{skill.version} + {#if skill.disabledAt} + 已禁用 + {/if} +
+ +
+
+ + +
+
+ +
+ +
+
+
+ +
+ digest: {skill.contentDigest.slice(0, 12)} + · + 更新于 {fmtDate(skill.updatedAt)} + {#if skill.boundRoleIds.length > 0} + · + 绑定角色: {skill.boundRoleIds.join(', ')} + {/if} + {#if dirty} + · + 未保存 + {/if} +
+ + {#if loading} +
加载文件中…
+ {:else} +
+
+
+ 文件 + +
+ {#if showNewFile} +
+ { + if (e.key === 'Enter') addFile(); + }} + /> +
+ {/if} +
+ {#each sortedFiles as f (f.path)} + + {/each} + {#if files.length === 0} +
暂无文件
+ {/if} +
+
+ +
+ {#if selectedFile} +
+ {selectedFile.path} +
+ + {:else} +
+ 选择一个文件或新建文件 +
+ {/if} +
+
+ +
+ + {#if !skill.disabledAt} + + {/if} +
+ {/if} +
diff --git a/hub/admin-web/src/routes/+layout.svelte b/hub/admin-web/src/routes/+layout.svelte index 1413de6..252a38e 100644 --- a/hub/admin-web/src/routes/+layout.svelte +++ b/hub/admin-web/src/routes/+layout.svelte @@ -25,6 +25,7 @@ { key: 'projects', label: '项目', icon: 'projects' as const }, { key: 'capacity', label: '容量', icon: 'overview' as const }, { key: 'provider', label: '供应方', icon: 'provider' as const }, + { key: 'skills', label: '技能', icon: 'roles' as const }, { key: 'roles', label: '角色', icon: 'roles' as const }, { key: 'feishu', label: '飞书', icon: 'feishu' as const }, ]; diff --git a/hub/admin-web/src/routes/admin/org/[slug]/skills/+page.svelte b/hub/admin-web/src/routes/admin/org/[slug]/skills/+page.svelte new file mode 100644 index 0000000..2d06f91 --- /dev/null +++ b/hub/admin-web/src/routes/admin/org/[slug]/skills/+page.svelte @@ -0,0 +1,141 @@ + + + + +{#if loading} + +{:else if error} + +{:else} +
+
+

新建技能

+ +
+ {#if showNewSkill} +
+ + + + +
+

+ 技能名称仅允许小写字母、数字和连字符，且以字母或数字开头。创建后会生成 SKILL.md 模板。 +

+ {/if} +
+ + {#if skills.length === 0} +
+ +
+ {:else} +
+ {#each skills as skill (skill.id)} + + {/each} +
+ {/if} +{/if} diff --git a/hub/src/admin/routes/agentConfigRoutes.ts b/hub/src/admin/routes/agentConfigRoutes.ts index 7b56269..4488124 100644 --- a/hub/src/admin/routes/agentConfigRoutes.ts +++ b/hub/src/admin/routes/agentConfigRoutes.ts @@ -1,13 +1,18 @@ /** * Org-admin Agent configuration routes (ADR-0017 / ADR-0018). * - * Surface for browsing and editing Organization-scoped Agent roles and the - * skills bound to them. Roles are org-owned data; the default-model picker is - * constrained to the env-default model registry (ADR-0017: there is no - * org-scoped model list — `OrganizationAgentRole.defaultModel` selects from the - * platform-enabled set). Skill *installation* is out of band (CLI / seed) per - * spec `Skill 管理是 org-scoped; 存储机制 OPEN`; this route only lists installed - * skills and binds them to roles. + * Surface for browsing and editing Organization-scoped Agent roles and skills. + * Roles are org-owned data; the default-model picker is constrained to the + * env-default model registry (ADR-0017: there is no org-scoped model list — + * `OrganizationAgentRole.defaultModel` selects from the platform-enabled set). + * + * Skill management (create, read, edit, disable) is performed in-band through + * the deep module `OrganizationAgentConfiguration`, which writes to the same + * content-addressed persistent store used by the host-console CLI. All skill + * content flows through `importSkillFromFiles` → `inspectSkillDirectory` → + * `commitSkillContent`, so the web path and CLI path share one ingestion + * pipeline and one set of safety checks (SKILL.md manifest required, 512-file + * / 16-byte limits, symlink rejection). */ import type { PrismaClient } from "@prisma/client"; import type { FastifyInstance } from "fastify"; @@ -19,6 +24,7 @@ import { handleRouteError } from "../errors.js"; export interface AgentConfigRouteConfig { readonly prisma: PrismaClient; readonly sessionSecret: string; + readonly skillStoreRoot: string; } export async function registerAgentConfigRoutes( @@ -26,8 +32,7 @@ export async function registerAgentConfigRoutes( config: AgentConfigRouteConfig, ): Promise { const guardDeps: GuardDeps = { prisma: config.prisma, sessionSecret: config.sessionSecret }; - // skillStoreRoot is null: this surface does not install skills (see header). - const agentConfig = new OrganizationAgentConfiguration(config.prisma, null); + const agentConfig = new OrganizationAgentConfiguration(config.prisma, config.skillStoreRoot); app.get("/api/org/:orgSlug/agent-roles", async (request, reply) => { try { @@ -115,6 +120,107 @@ export async function registerAgentConfigRoutes( } }); + app.get("/api/org/:orgSlug/agent-skills/:name/files", async (request, reply) => { + try { + const { orgSlug, name } = request.params as { orgSlug: string; name: string }; + const auth = await requireOrgRole(request, reply, guardDeps, { orgSlug }); + if (auth === null) return; + const skills = await agentConfig.listSkills({ organizationId: auth.organization.id }); + const skill = skills.find((s) => s.name === name && s.disabledAt === null); + if (skill === undefined) { + return reply.status(404).send({ + error: { code: "not_found", message: `skill not found: ${name}` }, + }); + } + const files = await agentConfig.readSkillFiles({ + organizationId: auth.organization.id, + contentDigest: skill.contentDigest, + }); + return { files }; + } catch (err) { + return handleRouteError(reply, err); + } + }); + + app.put("/api/org/:orgSlug/agent-skills/:name", async (request, reply) => { + try { + const { orgSlug, name } = request.params as { orgSlug: string; name: string }; + const auth = await requireOrgRole(request, reply, guardDeps, { orgSlug }); + if (auth === null) return; + const body = request.body as { version?: unknown; files?: unknown }; + if (typeof body.version !== "string" || body.version.trim() === "") { + return reply.status(400).send({ + error: { code: "bad_request", message: "version is required" }, + }); + } + if (!Array.isArray(body.files) || body.files.length === 0) { + return reply.status(400).send({ + error: { code: "bad_request", message: "files must be a non-empty array" }, + }); + } + const skillFiles: Array<{ readonly path: string; readonly bytes: Buffer }> = []; + for (const entry of body.files) { + if (typeof entry !== "object" || entry === null || Array.isArray(entry)) { + return reply.status(400).send({ + error: { code: "bad_request", message: "each file must be an object" }, + }); + } + const e = entry as { path?: unknown; content?: unknown }; + if (typeof e.path !== "string" || typeof e.content !== "string") { + return reply.status(400).send({ + error: { code: "bad_request", message: "each file needs string path and content" }, + }); + } + skillFiles.push({ path: e.path, bytes: Buffer.from(e.content, "utf8") }); + } + const result = await agentConfig.installSkillFromFiles({ + organizationId: auth.organization.id, + files: skillFiles, + version: body.version, + }); + // The manifest name is authoritative — reject if it doesn't match the + // URL param, so clients can't silently rename a skill under a different + // route key. + if (result.name !== name) { + return reply.status(400).send({ + error: { + code: "bad_request", + message: `skill manifest name "${result.name}" does not match route "${name}"`, + }, + }); + } + return { id: result.id, name: result.name, contentDigest: result.contentDigest }; + } catch (err) { + return handleRouteError(reply, err); + } + }); + + app.patch("/api/org/:orgSlug/agent-skills/:name", async (request, reply) => { + try { + const { orgSlug, name } = request.params as { orgSlug: string; name: string }; + const auth = await requireOrgRole(request, reply, guardDeps, { orgSlug }); + if (auth === null) return; + const body = request.body as { description?: unknown; disabled?: unknown }; + if (body.disabled === true) { + await agentConfig.disableSkill({ organizationId: auth.organization.id, name }); + return { disabled: true }; + } + if (typeof body.description === "string") { + await agentConfig.updateSkillDescription({ + organizationId: auth.organization.id, + name, + description: body.description, + }); + return { updated: true }; + } + return reply.status(400).send({ + error: { code: "bad_request", message: "provide description or disabled: true" }, + }); + } catch (err) { + return handleRouteError(reply, err); + } + }); + app.get("/api/org/:orgSlug/agent-models", async (request, reply) => { try { const { orgSlug } = request.params as { orgSlug: string }; diff --git a/hub/src/admin/routes/orgRoutes.ts b/hub/src/admin/routes/orgRoutes.ts index 5c7ad14..4e154c7 100644 --- a/hub/src/admin/routes/orgRoutes.ts +++ b/hub/src/admin/routes/orgRoutes.ts @@ -16,6 +16,7 @@ import { registerSessionsAndUsageRoutes } from "./sessionsRoutes.js"; import { registerTeamsRoutes } from "./teamsRoutes.js"; import { registerProviderConnectionRoutes } from "./providerConnectionRoutes.js"; import { registerCapacityRoutes } from "./capacityRoutes.js"; +import { readSkillStoreRoot } from "../../agent/skillStore.js"; import { registerAgentConfigRoutes } from "./agentConfigRoutes.js"; import type { LocalSecretEnvelope } from "../../security/secretEnvelope.js"; import type { ProviderReadinessProbe } from "../../connections/providerReadiness.js"; @@ -114,6 +115,7 @@ export async function registerOrgRoutes(app: FastifyInstance, config: OrgRouteCo await registerAgentConfigRoutes(app, { prisma: config.prisma, sessionSecret: config.sessionSecret, + skillStoreRoot: readSkillStoreRoot(), }); await registerSessionsAndUsageRoutes(app, { prisma: config.prisma, diff --git a/hub/src/agent/configuration.ts b/hub/src/agent/configuration.ts index c7b544c..1dd7e7b 100644 --- a/hub/src/agent/configuration.ts +++ b/hub/src/agent/configuration.ts @@ -1,7 +1,7 @@ import type { PrismaClient } from "@prisma/client"; import { Prisma } from "@prisma/client"; import { assertSupportedRoleTools } from "./roleTools.js"; -import { importSkillDirectory } from "./skillStore.js"; +import { importSkillDirectory, importSkillFromFiles, readSkillFiles, type SkillFileInput, type SkillFileOutput } from "./skillStore.js"; const ROLE_ID_PATTERN = /^[a-z0-9][a-z0-9_-]{0,63}$/; @@ -88,38 +88,179 @@ export class OrganizationAgentConfiguration { readonly sourceDir: string; readonly version: string; }): Promise<{ readonly id: string; readonly name: string; readonly contentDigest: string }> { - if (this.skillStoreRoot === null) { - throw new Error("skill store root is not configured for this OrganizationAgentConfiguration"); - } + const storeRoot = this.requireSkillStoreRoot(); await this.requireActiveOrganization(input.organizationId); const version = nonEmpty(input.version, "skill version"); const imported = await importSkillDirectory({ sourceDir: input.sourceDir, - storeRoot: this.skillStoreRoot, + storeRoot, }); + return this.commitInstalledSkill({ + organizationId: input.organizationId, + imported, + version, + action: "agent_skill.installed", + }); + } + + /** + * Install or replace a skill from an in-memory file list (web upload path). + * Same content-addressed storage, same session archival semantics as + * `installSkill`; only the ingestion source differs. + */ + async installSkillFromFiles(input: { + readonly organizationId: string; + readonly files: readonly SkillFileInput[]; + readonly version: string; + }): Promise<{ readonly id: string; readonly name: string; readonly contentDigest: string }> { + const storeRoot = this.requireSkillStoreRoot(); + await this.requireActiveOrganization(input.organizationId); + const version = nonEmpty(input.version, "skill version"); + const imported = await importSkillFromFiles({ + files: input.files, + storeRoot, + }); + return this.commitInstalledSkill({ + organizationId: input.organizationId, + imported, + version, + action: "agent_skill.installed", + }); + } + + /** + * Read all files from the stored skill version identified by content digest. + * Returns UTF-8 content for each file; the web editor uses this to populate + * its file tree. + */ + async readSkillFiles(input: { + readonly organizationId: string; + readonly contentDigest: string; + }): Promise { + const storeRoot = this.requireSkillStoreRoot(); + await this.requireActiveOrganization(input.organizationId); + const skill = await this.prisma.organizationAgentSkill.findFirst({ + where: { + organizationId: input.organizationId, + contentDigest: input.contentDigest, + }, + select: { id: true, disabledAt: true }, + }); + if (skill === null) { + throw new Error(`skill not found in organization for digest: ${input.contentDigest}`); + } + return readSkillFiles({ + storeRoot, + contentDigest: input.contentDigest, + }); + } + + /** + * Disable a skill (soft-delete). Sets `disabledAt` and archives active + * sessions of every role bound to this skill, since the execution surface + * changes when a bound skill disappears. + */ + async disableSkill(input: { readonly organizationId: string; readonly name: string }): Promise { + await this.requireActiveOrganization(input.organizationId); + await this.prisma.$transaction(async (tx) => { + const skill = await tx.organizationAgentSkill.findUnique({ + where: { organizationId_name: { organizationId: input.organizationId, name: input.name } }, + select: { id: true, disabledAt: true, roleBindings: { select: { role: { select: { roleId: true } } } } }, + }); + if (skill === null) throw new Error(`active skill not found in organization: ${input.name}`); + if (skill.disabledAt !== null) return; + await tx.organizationAgentSkill.update({ + where: { id: skill.id }, + data: { disabledAt: new Date() }, + }); + await archiveRoleSessions( + tx, + input.organizationId, + skill.roleBindings.map((binding) => binding.role.roleId), + ); + await tx.auditEntry.create({ + data: { + organizationId: input.organizationId, + action: "agent_skill.disabled", + metadata: { name: input.name }, + }, + }); + }); + } + + /** + * Update only the `description` label of a skill. This is a label change, + * not an execution-surface change — no session archival (ADR-0017). + */ + async updateSkillDescription(input: { + readonly organizationId: string; + readonly name: string; + readonly description: string; + }): Promise { + await this.requireActiveOrganization(input.organizationId); + const description = input.description.trim(); + await this.prisma.$transaction(async (tx) => { + const skill = await tx.organizationAgentSkill.findUnique({ + where: { organizationId_name: { organizationId: input.organizationId, name: input.name } }, + select: { id: true, disabledAt: true }, + }); + if (skill === null || skill.disabledAt !== null) { + throw new Error(`active skill not found in organization: ${input.name}`); + } + await tx.organizationAgentSkill.update({ + where: { id: skill.id }, + data: { description: description === "" ? null : description }, + }); + await tx.auditEntry.create({ + data: { + organizationId: input.organizationId, + action: "agent_skill.description_updated", + metadata: { name: input.name, description: description === "" ? null : description }, + }, + }); + }); + } + + private requireSkillStoreRoot(): string { + if (this.skillStoreRoot === null) { + throw new Error("skill store root is not configured for this OrganizationAgentConfiguration"); + } + return this.skillStoreRoot; + } + + /** + * Shared DB commit for an imported skill: upsert the skill record, archive + * affected sessions if the content digest changed, and write an audit entry. + */ + private async commitInstalledSkill(input: { + readonly organizationId: string; + readonly imported: { readonly name: string; readonly description: string | undefined; readonly contentDigest: string }; + readonly version: string; + readonly action: string; + }): Promise<{ readonly id: string; readonly name: string; readonly contentDigest: string }> { return this.prisma.$transaction(async (tx) => { const previous = await tx.organizationAgentSkill.findUnique({ - where: { organizationId_name: { organizationId: input.organizationId, name: imported.name } }, + where: { organizationId_name: { organizationId: input.organizationId, name: input.imported.name } }, select: { contentDigest: true }, }); const skill = await tx.organizationAgentSkill.upsert({ where: { organizationId_name: { organizationId: input.organizationId, - name: imported.name, + name: input.imported.name, }, }, create: { organizationId: input.organizationId, - name: imported.name, - version, - description: imported.description ?? null, - contentDigest: imported.contentDigest, + name: input.imported.name, + version: input.version, + description: input.imported.description ?? null, + contentDigest: input.imported.contentDigest, }, update: { - version, - description: imported.description ?? null, - contentDigest: imported.contentDigest, + version: input.version, + description: input.imported.description ?? null, + contentDigest: input.imported.contentDigest, disabledAt: null, }, select: { @@ -139,10 +280,10 @@ export class OrganizationAgentConfiguration { await tx.auditEntry.create({ data: { organizationId: input.organizationId, - action: "agent_skill.installed", + action: input.action, metadata: { name: skill.name, - version, + version: input.version, contentDigest: skill.contentDigest, }, }, diff --git a/hub/src/agent/skillStore.ts b/hub/src/agent/skillStore.ts index e2422f0..1016e73 100644 --- a/hub/src/agent/skillStore.ts +++ b/hub/src/agent/skillStore.ts @@ -34,40 +34,134 @@ export async function importSkillDirectory(input: { readonly sourceDir: string; readonly storeRoot: string; }): Promise { - const source = await inspectSkillDirectory(input.sourceDir); + return commitSkillContent({ + storeRoot: input.storeRoot, + prepare: async (dir) => { + await cp(input.sourceDir, dir, { recursive: true, force: false, errorOnExist: true }); + return inspectSkillDirectory(dir); + }, + }); +} + +/** + * Import a skill from an in-memory file list (web upload path). Each file + * path is relative to the skill root and must be a simple relative path + * (no absolute, no `..`). A `SKILL.md` manifest is required. + */ +export async function importSkillFromFiles(input: { + readonly files: readonly SkillFileInput[]; + readonly storeRoot: string; +}): Promise { + const seen = new Set(); + for (const file of input.files) { + validateSkillFilePath(file.path); + if (seen.has(file.path)) throw new Error(`duplicate skill file path: ${file.path}`); + seen.add(file.path); + } + return commitSkillContent({ + storeRoot: input.storeRoot, + prepare: async (dir) => { + for (const file of input.files) { + const dest = join(dir, file.path); + const parent = join(dest, ".."); + await mkdir(parent, { recursive: true, mode: 0o750 }); + await writeFile(dest, file.bytes, { mode: 0o640 }); + } + return inspectSkillDirectory(dir); + }, + }); +} + +/** + * Read all files from a stored skill version (by content digest). Returns + * relative paths and UTF-8 decoded content for the web editor. + */ +export async function readSkillFiles(input: { + readonly storeRoot: string; + readonly contentDigest: string; +}): Promise { + if (!DIGEST_PATTERN.test(input.contentDigest)) { + throw new Error(`invalid content digest: ${input.contentDigest}`); + } + const dir = join(input.storeRoot, "versions", input.contentDigest); + const files: Array<{ readonly path: string; readonly bytes: Buffer }> = []; + await walk(resolve(dir), resolve(dir), files); + return files.sort((a, b) => a.path.localeCompare(b.path)).map((f) => ({ + path: f.path, + content: f.bytes.toString("utf8"), + })); +} + +export interface SkillFileInput { + readonly path: string; + readonly bytes: Buffer; +} + +export interface SkillFileOutput { + readonly path: string; + readonly content: string; +} + +/** + * Shared commit logic: populate a temp dir, inspect it, deduplicate against + * an existing `versions//` destination, and atomically rename. + * + * `prepare(dir)` writes the skill content into `dir` and returns the inspected + * result (name, description, contentDigest). The caller owns the write; + * commitSkillContent owns the move. + */ +async function commitSkillContent(input: { + readonly storeRoot: string; + readonly prepare: (dir: string) => Promise; +}): Promise { const versionsRoot = join(input.storeRoot, "versions"); await mkdir(versionsRoot, { recursive: true, mode: 0o750 }); + const temporary = join(versionsRoot, `.tmp-${randomUUID()}`); + + let source: ImportedSkillContent; + try { + source = await input.prepare(temporary); + } catch (error) { + await rm(temporary, { recursive: true, force: true }); + throw error; + } + const destination = join(versionsRoot, source.contentDigest); + // If the destination already exists with identical content, reuse it. try { const existing = await inspectSkillDirectory(destination); if (existing.contentDigest !== source.contentDigest || existing.name !== source.name) { throw new Error(`stored skill content digest mismatch: ${source.name}`); } + await rm(temporary, { recursive: true, force: true }); return source; } catch (error) { if (!isMissingPath(error)) throw error; } - const temporary = join(versionsRoot, `.tmp-${randomUUID()}`); try { - await cp(input.sourceDir, temporary, { recursive: true, force: false, errorOnExist: true }); - const copied = await inspectSkillDirectory(temporary); - if (copied.contentDigest !== source.contentDigest || copied.name !== source.name) { - throw new Error(`skill changed while importing: ${source.name}`); - } await rename(temporary, destination); } catch (error) { await rm(temporary, { recursive: true, force: true }); if (isDestinationExists(error)) { const existing = await inspectSkillDirectory(destination); - if (existing.contentDigest === source.contentDigest && existing.name === source.name) return source; + if (existing.contentDigest === source.contentDigest && existing.name === source.name) { + return source; + } } throw error; } return source; } +function validateSkillFilePath(path: string): void { + if (path === "") throw new Error("skill file path is empty"); + if (path.startsWith("/")) throw new Error(`skill file path must be relative: ${path}`); + if (path.includes("..")) throw new Error(`skill file path must not escape: ${path}`); + if (path.startsWith("\\")) throw new Error(`skill file path must be relative: ${path}`); +} + export async function prepareRunSkillPlugin(input: { readonly storeRoot: string; readonly runId: string; diff --git a/hub/test/unit/skill-store.test.ts b/hub/test/unit/skill-store.test.ts index 2276b9d..c74e5fc 100644 --- a/hub/test/unit/skill-store.test.ts +++ b/hub/test/unit/skill-store.test.ts @@ -2,7 +2,12 @@ import { mkdir, mkdtemp, readFile, rm, symlink, writeFile } from "node:fs/promis import { tmpdir } from "node:os"; import { join } from "node:path"; import { afterEach, describe, expect, it } from "vitest"; -import { importSkillDirectory, prepareRunSkillPlugin } from "../../src/agent/skillStore.js"; +import { + importSkillDirectory, + importSkillFromFiles, + prepareRunSkillPlugin, + readSkillFiles, +} from "../../src/agent/skillStore.js"; describe("content-addressed Agent skill store", () => { const roots: string[] = []; @@ -55,6 +60,47 @@ describe("content-addressed Agent skill store", () => { })).rejects.toThrow(/content digest mismatch/); }); + it("imports a skill from an in-memory file list and reads it back", async () => { + const root = await makeRoot(); + const storeRoot = join(root, "store"); + const manifest = `---\nname: web-skill\ndescription: Web uploaded\n---\n# web-skill\n`; + const files = [ + { path: "SKILL.md", bytes: Buffer.from(manifest, "utf8") }, + { path: "reference.md", bytes: Buffer.from("ref\n", "utf8") }, + ]; + const installed = await importSkillFromFiles({ files, storeRoot }); + expect(installed).toMatchObject({ name: "web-skill", description: "Web uploaded" }); + expect(installed.contentDigest).toMatch(/^[a-f0-9]{64}$/); + + const readBack = await readSkillFiles({ storeRoot, contentDigest: installed.contentDigest }); + expect(readBack).toHaveLength(2); + const byPath = Object.fromEntries(readBack.map((f) => [f.path, f.content])); + expect(byPath["SKILL.md"]).toContain("name: web-skill"); + expect(byPath["reference.md"]).toBe("ref\n"); + }); + + it("rejects file paths that escape the skill root", async () => { + const root = await makeRoot(); + const storeRoot = join(root, "store"); + const manifest = `---\nname: escape\ndescription: test\n---\n# escape\n`; + const files = [ + { path: "SKILL.md", bytes: Buffer.from(manifest, "utf8") }, + { path: "../escape.md", bytes: Buffer.from("x", "utf8") }, + ]; + await expect(importSkillFromFiles({ files, storeRoot })).rejects.toThrow(/must not escape/); + }); + + it("rejects duplicate file paths in importSkillFromFiles", async () => { + const root = await makeRoot(); + const storeRoot = join(root, "store"); + const manifest = `---\nname: dup\ndescription: test\n---\n# dup\n`; + const files = [ + { path: "SKILL.md", bytes: Buffer.from(manifest, "utf8") }, + { path: "SKILL.md", bytes: Buffer.from(manifest, "utf8") }, + ]; + await expect(importSkillFromFiles({ files, storeRoot })).rejects.toThrow(/duplicate skill file path/); + }); + async function makeRoot(): Promise { const root = await mkdtemp(join(tmpdir(), "cph-skill-store-")); roots.push(root);