fix(skill): enforce agent skill bindings in skill meta-tools at runtime (#265)

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
This commit is contained in:
倪程伟 2026-06-07 17:30:41 +08:00 committed by GitHub
parent 6c8283e77a
commit 3d8d266e3b
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 84 additions and 3 deletions

View File

@ -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<Long> 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<Long> 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<Long> 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";

View File

@ -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.
* <p>
@ -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<Long> 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

View File

@ -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<Long> 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) {