From 3d8d266e3b9d38f4650e19da369acd856d6ef3ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=80=AA=E7=A8=8B=E4=BC=9F?= Date: Sun, 7 Jun 2026 17:30:41 +0800 Subject: [PATCH] fix(skill): enforce agent skill bindings in skill meta-tools at runtime (#265) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The skill meta-tools (listAvailableSkills, readSkillFile, listSkillFiles, load_skill, runSkillScript) queried the full skill catalog without checking the calling agent's bindings, so an agent scoped to a subset of skills could still read, load, or execute any skill via direct tool calls. Resolve the agent's bound skill ids from the tool context and filter/deny accordingly: a null binding set means no restriction (backward compatible), a non-null set (including empty) restricts access to that set — matching the system-prompt catalog filtering. Closes #264 --- .../vip/mate/tool/builtin/SkillFileTool.java | 47 ++++++++++++++++++- .../vip/mate/tool/builtin/SkillLoadTool.java | 18 +++++++ .../mate/tool/builtin/SkillScriptTool.java | 22 ++++++++- 3 files changed, 84 insertions(+), 3 deletions(-) diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java index b4b73e8b..8b4e2a47 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillFileTool.java @@ -6,10 +6,13 @@ import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; import org.springframework.ai.chat.model.ToolContext; import org.springframework.ai.tool.annotation.Tool; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; import vip.mate.agent.context.ChatOrigin; import vip.mate.agent.context.TokenEstimator; +import vip.mate.llm.routing.AgentBindingResolver; import vip.mate.skill.runtime.SkillCatalogSort; import vip.mate.skill.runtime.SkillCatalogSorter; import vip.mate.skill.runtime.SkillFileAccessPolicy; @@ -21,6 +24,7 @@ import java.nio.file.Files; import java.nio.file.Path; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.stream.Collectors; /** @@ -39,6 +43,10 @@ public class SkillFileTool { private final SkillFileAccessPolicy accessPolicy; private final SkillUsageService usageService; + @Lazy + @Autowired + private AgentBindingResolver agentBindingResolver; + @Tool(description = """ Read a file from a skill's directory (SKILL.md, references/, scripts/, or templates/). Use this when you need to access skill documentation or reference files. @@ -80,6 +88,9 @@ public class SkillFileTool { if (skill == null) { return "Error: Skill '" + skillName + "' not found or not enabled"; } + if (!isSkillAllowedForAgent(skill, ctx)) { + return "Error: Skill '" + skillName + "' is not available for this agent."; + } // 特殊处理：读取 SKILL.md if ("SKILL.md".equals(filePath)) { @@ -228,7 +239,9 @@ public class SkillFileTool { public String listSkillFiles( @JsonProperty(required = true) @JsonPropertyDescription("Skill name") - String skillName + String skillName, + + @Nullable ToolContext ctx ) { log.info("Listing skill files: skill={}", skillName); @@ -236,6 +249,9 @@ public class SkillFileTool { if (skill == null) { return "Error: Skill '" + skillName + "' not found or not enabled"; } + if (!isSkillAllowedForAgent(skill, ctx)) { + return "Error: Skill '" + skillName + "' is not available for this agent."; + } StringBuilder sb = new StringBuilder(); sb.append("Skill: ").append(skillName).append("\n\n"); @@ -305,10 +321,13 @@ public class SkillFileTool { @JsonProperty(required = false) @JsonPropertyDescription("Maximum number of skills to return, default 20, max 50") - Integer limit + Integer limit, + + @Nullable ToolContext ctx ) { log.info("Listing available skills"); + Set boundSkillIds = boundSkillIdsFromCtx(ctx); int safeLimit = limit == null || limit <= 0 ? 20 : Math.min(limit, 50); String kw = keyword == null ? "" : keyword.trim().toLowerCase(); // Push freshly installed skills to the top of the truncated page so @@ -325,6 +344,8 @@ public class SkillFileTool { runtimeService.getActiveSkills().stream() .filter(s -> SkillCatalogSorter.sourceMatches(s, source)) .filter(s -> SkillCatalogSorter.runtimeMatches(s, status)) + .filter(s -> boundSkillIds == null + || (s.getId() != null && boundSkillIds.contains(s.getId()))) .filter(s -> kw.isEmpty() || containsIgnoreCase(s.getName(), kw) || containsIgnoreCase(s.getDescription(), kw)) @@ -381,6 +402,28 @@ public class SkillFileTool { return value != null && value.toLowerCase().contains(lowerCaseNeedle); } + /** + * Returns the agent's bound skill IDs from the tool context, or {@code null} + * when no binding restriction applies (agentId missing or agent has no explicit bindings). + */ + @Nullable + private Set boundSkillIdsFromCtx(@Nullable ToolContext ctx) { + Long agentId = ChatOrigin.from(ctx).agentId(); + if (agentId == null) return null; + return agentBindingResolver.getBoundSkillIds(agentId); + } + + /** + * Returns {@code true} when the agent (identified via {@code ctx}) is allowed + * to access {@code skill}: either no explicit binding restriction, or the skill + * id is in the agent's bound set. + */ + private boolean isSkillAllowedForAgent(ResolvedSkill skill, @Nullable ToolContext ctx) { + Set boundSkillIds = boundSkillIdsFromCtx(ctx); + if (boundSkillIds == null) return true; + return skill.getId() != null && boundSkillIds.contains(skill.getId()); + } + private static String statusToken(ResolvedSkill skill) { if (skill.isSecurityBlocked()) return "blocked"; if (!skill.isEnabled()) return "disabled"; diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillLoadTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillLoadTool.java index 7574903e..5409977c 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillLoadTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillLoadTool.java @@ -5,11 +5,17 @@ import lombok.extern.slf4j.Slf4j; import org.springframework.ai.chat.model.ToolContext; import org.springframework.ai.tool.annotation.Tool; import org.springframework.ai.tool.annotation.ToolParam; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import vip.mate.agent.context.ChatOrigin; +import vip.mate.llm.routing.AgentBindingResolver; import vip.mate.skill.runtime.SkillRuntimeService; import vip.mate.skill.runtime.model.ResolvedSkill; +import java.util.Set; + /** * Explicit skill-load entry point. *

@@ -32,6 +38,10 @@ public class SkillLoadTool { private final SkillRuntimeService runtimeService; private final SkillFileTool skillFileTool; + @Lazy + @Autowired + private AgentBindingResolver agentBindingResolver; + @Tool(name = "load_skill", description = """ Load a skill package's SKILL.md into the conversation. Call this when a skill in the catalog matches the task. @@ -65,6 +75,14 @@ public class SkillLoadTool { return "Error: Skill '" + skillName + "' not found or not enabled. " + "Call listAvailableSkills(keyword=\"" + skillName + "\") to find the correct name."; } + Long agentId = ChatOrigin.from(ctx).agentId(); + if (agentId != null) { + Set boundSkillIds = agentBindingResolver.getBoundSkillIds(agentId); + if (boundSkillIds != null && (skill.getId() == null || !boundSkillIds.contains(skill.getId()))) { + log.info("load_skill: agent {} is not allowed to load skill '{}'", agentId, skillName); + return "Error: Skill '" + skillName + "' is not available for this agent."; + } + } String path = (filePath == null || filePath.isBlank()) ? "SKILL.md" : filePath; log.info("load_skill: loading skill='{}', path='{}'", skillName, path); // Delegate to the shared reader: it resolves the skill, paginates large diff --git a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillScriptTool.java b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillScriptTool.java index 1f9bb38b..ae3a5a49 100644 --- a/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillScriptTool.java +++ b/mateclaw-server/src/main/java/vip/mate/tool/builtin/SkillScriptTool.java @@ -7,8 +7,14 @@ import com.fasterxml.jackson.databind.JsonNode; import com.fasterxml.jackson.databind.ObjectMapper; import lombok.RequiredArgsConstructor; import lombok.extern.slf4j.Slf4j; +import org.springframework.ai.chat.model.ToolContext; import org.springframework.ai.tool.annotation.Tool; +import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.context.annotation.Lazy; +import org.springframework.lang.Nullable; import org.springframework.stereotype.Component; +import vip.mate.agent.context.ChatOrigin; +import vip.mate.llm.routing.AgentBindingResolver; import vip.mate.skill.runtime.SkillFileAccessPolicy; import vip.mate.skill.runtime.SkillRuntimeService; import vip.mate.skill.runtime.SkillScriptExecutionService; @@ -20,6 +26,7 @@ import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Map; +import java.util.Set; /** * 技能脚本执行工具 @@ -36,6 +43,10 @@ public class SkillScriptTool { private final SkillSecretService skillSecretService; private final ObjectMapper objectMapper; + @Lazy + @Autowired + private AgentBindingResolver agentBindingResolver; + @vip.mate.tool.ConcurrencyUnsafe("script execution can have arbitrary side effects on the host process and filesystem") @Tool(description = """ Execute a script from a skill's scripts/ directory. @@ -67,7 +78,9 @@ public class SkillScriptTool { @JsonProperty(required = false) @JsonPropertyDescription("Optional script arguments as ONE JSON-encoded string: a JSON array for multiple positional args, a JSON object for a single JSON payload, or plain text for one literal argument.") - String args + String args, + + @Nullable ToolContext ctx ) { log.info("Executing skill script: skill={}, script={}, args={}", skillName, scriptPath, args); @@ -76,6 +89,13 @@ public class SkillScriptTool { if (skill == null) { return formatError("Skill '" + skillName + "' not found or not enabled"); } + Long agentId = ChatOrigin.from(ctx).agentId(); + if (agentId != null) { + Set boundSkillIds = agentBindingResolver.getBoundSkillIds(agentId); + if (boundSkillIds != null && (skill.getId() == null || !boundSkillIds.contains(skill.getId()))) { + return formatError("Skill '" + skillName + "' is not available for this agent."); + } + } // Must be a directory-backed skill. if (skill.getSkillDir() == null) {