fix(skill): enforce agent restriction in createSkillTool (#1018)
* fix(skill): enforce agent restriction in createSkillTool Ultraworked with [Sisyphus](https://github.com/code-yeongyu/oh-my-opencode) Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai> * fix(skill): block restricted skills when agent context missing Addresses cubic review feedback: previously agent-restricted skills could be invoked when ctx or ctx.agent was undefined because the guard only ran when ctx?.agent was truthy. Changed condition from: skill.definition.agent && ctx?.agent && skill.definition.agent !== ctx.agent To: skill.definition.agent && (!ctx?.agent || skill.definition.agent !== ctx.agent) This ensures restricted skills are blocked unless the exact matching agent is present in the context. --------- Co-authored-by: justsisyphus <justsisyphus@users.noreply.github.com> Co-authored-by: Sisyphus <clio-agent@sisyphuslabs.ai>
This commit is contained in:
@@ -20,6 +20,21 @@ Test skill body content`
|
||||
},
|
||||
}))
|
||||
|
||||
function createMockSkill(name: string, options: { agent?: string } = {}): LoadedSkill {
|
||||
return {
|
||||
name,
|
||||
path: `/test/skills/${name}/SKILL.md`,
|
||||
resolvedPath: `/test/skills/${name}`,
|
||||
definition: {
|
||||
name,
|
||||
description: `Test skill ${name}`,
|
||||
template: "Test template",
|
||||
agent: options.agent,
|
||||
},
|
||||
scope: "opencode-project",
|
||||
}
|
||||
}
|
||||
|
||||
function createMockSkillWithMcp(name: string, mcpServers: Record<string, unknown>): LoadedSkill {
|
||||
return {
|
||||
name,
|
||||
@@ -42,6 +57,59 @@ const mockContext = {
|
||||
abort: new AbortController().signal,
|
||||
}
|
||||
|
||||
describe("skill tool - agent restriction", () => {
|
||||
it("allows skill without agent restriction to any agent", async () => {
|
||||
// #given
|
||||
const loadedSkills = [createMockSkill("public-skill")]
|
||||
const tool = createSkillTool({ skills: loadedSkills })
|
||||
const context = { ...mockContext, agent: "any-agent" }
|
||||
|
||||
// #when
|
||||
const result = await tool.execute({ name: "public-skill" }, context)
|
||||
|
||||
// #then
|
||||
expect(result).toContain("public-skill")
|
||||
})
|
||||
|
||||
it("allows skill when agent matches restriction", async () => {
|
||||
// #given
|
||||
const loadedSkills = [createMockSkill("restricted-skill", { agent: "sisyphus" })]
|
||||
const tool = createSkillTool({ skills: loadedSkills })
|
||||
const context = { ...mockContext, agent: "sisyphus" }
|
||||
|
||||
// #when
|
||||
const result = await tool.execute({ name: "restricted-skill" }, context)
|
||||
|
||||
// #then
|
||||
expect(result).toContain("restricted-skill")
|
||||
})
|
||||
|
||||
it("throws error when agent does not match restriction", async () => {
|
||||
// #given
|
||||
const loadedSkills = [createMockSkill("sisyphus-only-skill", { agent: "sisyphus" })]
|
||||
const tool = createSkillTool({ skills: loadedSkills })
|
||||
const context = { ...mockContext, agent: "oracle" }
|
||||
|
||||
// #when / #then
|
||||
await expect(tool.execute({ name: "sisyphus-only-skill" }, context)).rejects.toThrow(
|
||||
'Skill "sisyphus-only-skill" is restricted to agent "sisyphus"'
|
||||
)
|
||||
})
|
||||
|
||||
it("throws error when context agent is undefined for restricted skill", async () => {
|
||||
// #given
|
||||
const loadedSkills = [createMockSkill("sisyphus-only-skill", { agent: "sisyphus" })]
|
||||
const tool = createSkillTool({ skills: loadedSkills })
|
||||
const contextWithoutAgent = { ...mockContext, agent: undefined as unknown as string }
|
||||
|
||||
// #when / #then
|
||||
await expect(tool.execute({ name: "sisyphus-only-skill" }, contextWithoutAgent)).rejects.toThrow(
|
||||
'Skill "sisyphus-only-skill" is restricted to agent "sisyphus"'
|
||||
)
|
||||
})
|
||||
|
||||
})
|
||||
|
||||
describe("skill tool - MCP schema display", () => {
|
||||
let manager: SkillMcpManager
|
||||
let loadedSkills: LoadedSkill[]
|
||||
|
||||
@@ -156,7 +156,7 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
||||
args: {
|
||||
name: tool.schema.string().describe("The skill identifier from available_skills (e.g., 'code-review')"),
|
||||
},
|
||||
async execute(args: SkillArgs) {
|
||||
async execute(args: SkillArgs, ctx?: { agent?: string }) {
|
||||
const skills = await getSkills()
|
||||
const skill = skills.find(s => s.name === args.name)
|
||||
|
||||
@@ -165,6 +165,10 @@ export function createSkillTool(options: SkillLoadOptions = {}): ToolDefinition
|
||||
throw new Error(`Skill "${args.name}" not found. Available skills: ${available || "none"}`)
|
||||
}
|
||||
|
||||
if (skill.definition.agent && (!ctx?.agent || skill.definition.agent !== ctx.agent)) {
|
||||
throw new Error(`Skill "${args.name}" is restricted to agent "${skill.definition.agent}"`)
|
||||
}
|
||||
|
||||
let body = await extractSkillBody(skill)
|
||||
|
||||
if (args.name === "git-master") {
|
||||
|
||||
Reference in New Issue
Block a user