fix(tool-guard): trust shared skill root outside the workspace boundary

This commit is contained in:
matevip 2026-05-29 09:59:19 +08:00
parent f3e6e1152a
commit 237f649e7e
6 changed files with 136 additions and 4 deletions

View File

@ -87,12 +87,40 @@ public final class RuntimeContextInjector {
sb.append("\n[system-context] Working directory: ").append(workspaceBasePath);
sb.append("\nYou can only read/write files and execute commands within this directory and its subdirectories.");
}
appendSkillRootHintIfPresent(sb, workspaceBasePath, i18n);
}
appendSenderBlockIfPresent(sb, origin);
return sb.toString();
}
/**
* Tell the model that the shared skill repository is reachable in addition
* to the workspace. Without this, a model that strictly honors the
* "working directory only" hint refuses to read or run skill files that
* live outside the workspace even though the path sandbox now allows
* them. Skipped when the skill root is unknown or already sits inside the
* workspace (no separate boundary to explain).
*/
private static void appendSkillRootHintIfPresent(StringBuilder sb, String workspaceBasePath,
vip.mate.i18n.I18nService i18n) {
java.nio.file.Path skillRoot = vip.mate.tool.guard.WorkspacePathGuard.getSkillRoot();
if (skillRoot == null) {
return;
}
java.nio.file.Path wsRoot = java.nio.file.Paths.get(workspaceBasePath).toAbsolutePath().normalize();
if (skillRoot.startsWith(wsRoot)) {
return;
}
String skillRootStr = skillRoot.toString();
if (i18n != null) {
sb.append("\n").append(i18n.msg("context.skill_dir_hint", skillRootStr));
} else {
sb.append("\nShared skills live under ").append(skillRootStr)
.append("; you may also read and run files there, even though it is outside the working directory.");
}
}
/**
* Append a sender / channel / chat block when the origin carries
* meaningful IM context. Format is intentionally one line per

View File

@ -3,6 +3,7 @@ package vip.mate.skill.workspace;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.context.annotation.Configuration;
import vip.mate.skill.installer.SkillHubProperties;
import vip.mate.tool.guard.WorkspacePathGuard;
/**
* Skill 工作区与安装器自动配置
@ -12,4 +13,15 @@ import vip.mate.skill.installer.SkillHubProperties;
@Configuration
@EnableConfigurationProperties({SkillWorkspaceProperties.class, SkillHubProperties.class})
public class SkillWorkspaceAutoConfiguration {
/**
* Register the shared skill repository root with the workspace path sandbox.
* Skills are shared across all workspaces and live outside any single
* workspace directory, so the sandbox must trust their root in addition to
* the active workspace otherwise reading or running a skill's files from a
* workspace configured elsewhere is rejected as a boundary violation.
*/
public SkillWorkspaceAutoConfiguration(SkillWorkspaceProperties skillWorkspaceProperties) {
WorkspacePathGuard.setSkillRoot(skillWorkspaceProperties.getRoot());
}
}

View File

@ -28,6 +28,58 @@ public final class WorkspacePathGuard {
private WorkspacePathGuard() {}
/**
* Shared skill repository root, trusted in addition to the per-conversation
* workspace boundary. System-level skills live under this root (one
* subdirectory per skill) and are shared across every workspace, so the
* agent must be able to read and run their files even when the active
* workspace points elsewhere. Registered once at startup from the
* {@code mateclaw.skill.workspace.root} setting. {@code null} until set
* (then no extra root is trusted pure workspace-only behaviour).
*/
private static volatile Path skillRoot;
/**
* Register the shared skill repository root. A {@code null} or blank path
* clears it, restoring workspace-only enforcement.
*/
public static void setSkillRoot(@Nullable String path) {
skillRoot = (path == null || path.isBlank())
? null
: Paths.get(path).toAbsolutePath().normalize();
log.info("[WorkspacePathGuard] Trusted skill root: {}", skillRoot);
}
/** The registered shared skill repository root, or {@code null} if none is set. */
@Nullable
public static Path getSkillRoot() {
return skillRoot;
}
/** True when {@code normalized} lives under the shared skill root (if one is set). */
private static boolean isUnderSkillRoot(Path normalized) {
Path sr = skillRoot;
return sr != null && normalized.startsWith(sr);
}
/**
* Symlink-resolved variant of {@link #isUnderSkillRoot}. Resolves the skill
* root's real path so a path whose real location lands inside the skill
* repository is accepted even when reached through a symlink.
*/
private static boolean isUnderSkillRootReal(Path realPath) {
Path sr = skillRoot;
if (sr == null) {
return false;
}
try {
Path realSkillRoot = sr.toFile().exists() ? sr.toRealPath() : sr;
return realPath.startsWith(realSkillRoot);
} catch (IOException e) {
return realPath.startsWith(sr);
}
}
/**
* 校验文件路径是否在当前工作区活动目录范围内
* <p>
@ -59,7 +111,7 @@ public final class WorkspacePathGuard {
Path root = Paths.get(basePath).toAbsolutePath().normalize();
// 先用 normalize 检查再尝试 toRealPath 防符号链接逃逸
if (!normalized.startsWith(root)) {
if (!normalized.startsWith(root) && !isUnderSkillRoot(normalized)) {
throw new IllegalArgumentException(
"Path is outside workspace boundary: " + normalized + ", allowed root: " + root);
}
@ -69,7 +121,7 @@ public final class WorkspacePathGuard {
if (normalized.toFile().exists()) {
Path realPath = normalized.toRealPath();
Path realRoot = root.toFile().exists() ? root.toRealPath() : root;
if (!realPath.startsWith(realRoot)) {
if (!realPath.startsWith(realRoot) && !isUnderSkillRootReal(realPath)) {
throw new IllegalArgumentException(
"Path escapes workspace via symlink: " + realPath + ", allowed root: " + realRoot);
}
@ -192,7 +244,7 @@ public final class WorkspacePathGuard {
// idioms (`2>/dev/null`, `cmd <(cat file)`) keep working.
continue;
}
if (!normalized.startsWith(root)) {
if (!normalized.startsWith(root) && !isUnderSkillRoot(normalized)) {
throw new IllegalArgumentException(
"Shell command references path outside workspace boundary: "
+ normalized + ", allowed root: " + root);
@ -213,7 +265,7 @@ public final class WorkspacePathGuard {
continue;
}
if (isAllowedDeviceNode(resolved)) continue;
if (!resolved.startsWith(root)) {
if (!resolved.startsWith(root) && !isUnderSkillRoot(resolved)) {
throw new IllegalArgumentException(
"Shell command uses parent-directory traversal that escapes the workspace: '"
+ candidate + "' would resolve to " + resolved

View File

@ -293,6 +293,7 @@ guard.path.symlink_escape=\u8def\u5f84\u901a\u8fc7\u7b26\u53f7\u94fe\u63a5\u9003
context.current_time=[system-context] \u5f53\u524d\u65f6\u95f4: {0} {1} (Asia/Shanghai)
context.working_dir=[system-context] \u5de5\u4f5c\u76ee\u5f55: {0}
context.working_dir_hint=\u4f60\u53ea\u80fd\u5728\u6b64\u76ee\u5f55\u53ca\u5176\u5b50\u76ee\u5f55\u5185\u8bfb\u5199\u6587\u4ef6\u548c\u6267\u884c\u547d\u4ee4\u3002
context.skill_dir_hint=\u5171\u4eab\u6280\u80fd\u4f4d\u4e8e {0}\uff0c\u4f60\u4e5f\u53ef\u4ee5\u8bfb\u53d6\u548c\u8fd0\u884c\u5176\u4e2d\u7684\u6587\u4ef6\uff08\u5373\u4f7f\u5728\u5de5\u4f5c\u76ee\u5f55\u4e4b\u5916\uff09\u3002
# --- Wiki Research Fallback (RFC: prompt-cleanup) ---
research.fallback.no_plan=\u65e0\u6cd5\u4e3a\u8be5\u4e3b\u9898\u751f\u6210\u7814\u7a76\u8ba1\u5212\u3002

View File

@ -300,6 +300,7 @@ err.approval.not_found=Approval record not found or expired
context.current_time=[system-context] Current time: {0} {1} (Asia/Shanghai)
context.working_dir=[system-context] Working directory: {0}
context.working_dir_hint=You can only read/write files and execute commands within this directory and its subdirectories.
context.skill_dir_hint=Shared skills live under {0}; you may also read and run files there, even though it is outside the working directory.
# --- Wiki Research Fallback (RFC: prompt-cleanup) ---
research.fallback.no_plan=Unable to generate a research plan for this topic.

View File

@ -24,6 +24,7 @@ import static org.junit.jupiter.api.Assertions.assertThrows;
class WorkspacePathGuardShellTest {
private static final String WORKSPACE = "/tmp/ws-guard-shell-test";
private static final String SKILL_ROOT = "/tmp/ws-guard-skill-root";
@BeforeEach
void setup() {
@ -33,6 +34,7 @@ class WorkspacePathGuardShellTest {
@AfterEach
void teardown() {
ToolExecutionContext.clear();
WorkspacePathGuard.setSkillRoot(null);
}
// ==================== No-op when sandbox absent ====================
@ -262,6 +264,42 @@ class WorkspacePathGuardShellTest {
WorkspacePathGuard.validateShellCommand("echo version=1.2..3"));
}
// ==================== Shared skill root allowance ====================
@Test
@DisplayName("Skill root is trusted in addition to the workspace")
void skillRoot_pass() {
WorkspacePathGuard.setSkillRoot(SKILL_ROOT);
// Reading and running a shared skill's files from a workspace elsewhere.
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand(
"cat " + SKILL_ROOT + "/zclt-toolkit/SKILL.md"));
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand(
"bash " + SKILL_ROOT + "/zclt-toolkit/scripts/run.sh"));
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand(
"cd " + SKILL_ROOT + "/zclt-toolkit && ls"));
// The workspace itself still passes.
assertDoesNotThrow(() -> WorkspacePathGuard.validateShellCommand(
"cat " + WORKSPACE + "/foo.txt"));
}
@Test
@DisplayName("Without a skill root, the same skill path is still blocked")
void skillRoot_unset_blocked() {
// No skill root registered skill path is just another outside path.
assertThrows(IllegalArgumentException.class, () ->
WorkspacePathGuard.validateShellCommand("cat " + SKILL_ROOT + "/zclt-toolkit/SKILL.md"));
}
@Test
@DisplayName("A skill root does not widen the boundary to unrelated outside paths")
void skillRoot_doesNotWidenOtherPaths() {
WorkspacePathGuard.setSkillRoot(SKILL_ROOT);
assertThrows(IllegalArgumentException.class, () ->
WorkspacePathGuard.validateShellCommand("cat /etc/passwd"));
assertThrows(IllegalArgumentException.class, () ->
WorkspacePathGuard.validateShellCommand("echo evil > /tmp/leak.txt"));
}
// ==================== Device-node negative cases ====================
@Test