fix: confine sandbox skill sync destinations

CHANGELOG.md

@@ -14,6 +14,7 @@ Docs: https://docs.openclaw.ai
 
 - Security: fix unauthenticated Nostr profile API remote config tampering. (#13719) Thanks @coygeek.
 - Security: remove bundled soul-evil hook. (#14757) Thanks @Imccccc.
+- Security/Sandbox: confine mirrored skill sync destinations to the sandbox `skills/` root and stop using frontmatter-controlled skill names as filesystem destination paths. Thanks @1seal.
 - Security/Web tools: treat browser/web content as untrusted by default (wrapped outputs for browser snapshot/tabs/console and structured external-content metadata for web tools), and strip `toolResult.details` from model-facing transcript/compaction inputs to reduce prompt-injection replay risk.
 - Security/Hooks: harden webhook and device token verification with shared constant-time secret comparison, and add per-client auth-failure throttling for hook endpoints (`429` + `Retry-After`). Thanks @akhmittra.
 - Gateway: raise WS payload/buffer limits so 5,000,000-byte image attachments work reliably. (#14486) Thanks @0xRaini.

src/agents/skills.build-workspace-skills-prompt.syncs-merged-skills-into-target-workspace.test.ts

@@ -26,6 +26,15 @@ ${body ?? `# ${name}\n`}
   );
 }
 
+async function pathExists(filePath: string): Promise<boolean> {
+  try {
+    await fs.access(filePath);
+    return true;
+  } catch {
+    return false;
+  }
+}
+
 describe("buildWorkspaceSkillsPrompt", () => {
   it("syncs merged skills into a target workspace", async () => {
     const sourceWorkspace = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-"));
@@ -74,6 +83,63 @@ describe("buildWorkspaceSkillsPrompt", () => {
     expect(prompt).not.toContain("Extra version");
     expect(prompt).toContain(path.join(targetWorkspace, "skills", "demo-skill", "SKILL.md"));
   });
+  it("keeps synced skills confined under target workspace when frontmatter name uses traversal", async () => {
+    const sourceWorkspace = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-"));
+    const targetWorkspace = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-"));
+    const escapeId = `${Date.now()}-${process.pid}-${Math.random().toString(16).slice(2)}`;
+    const traversalName = `../../../skill-sync-escape-${escapeId}`;
+    const escapedDest = path.resolve(targetWorkspace, "skills", traversalName);
+
+    await writeSkill({
+      dir: path.join(sourceWorkspace, "skills", "safe-traversal-skill"),
+      name: traversalName,
+      description: "Traversal skill",
+    });
+
+    expect(path.relative(path.join(targetWorkspace, "skills"), escapedDest).startsWith("..")).toBe(
+      true,
+    );
+    expect(await pathExists(escapedDest)).toBe(false);
+
+    await syncSkillsToWorkspace({
+      sourceWorkspaceDir: sourceWorkspace,
+      targetWorkspaceDir: targetWorkspace,
+      bundledSkillsDir: path.join(sourceWorkspace, ".bundled"),
+      managedSkillsDir: path.join(sourceWorkspace, ".managed"),
+    });
+
+    expect(
+      await pathExists(path.join(targetWorkspace, "skills", "safe-traversal-skill", "SKILL.md")),
+    ).toBe(true);
+    expect(await pathExists(escapedDest)).toBe(false);
+  });
+  it("keeps synced skills confined under target workspace when frontmatter name is absolute", async () => {
+    const sourceWorkspace = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-"));
+    const targetWorkspace = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-"));
+    const escapeId = `${Date.now()}-${process.pid}-${Math.random().toString(16).slice(2)}`;
+    const absoluteDest = path.join(os.tmpdir(), `skill-sync-abs-escape-${escapeId}`);
+
+    await fs.rm(absoluteDest, { recursive: true, force: true });
+    await writeSkill({
+      dir: path.join(sourceWorkspace, "skills", "safe-absolute-skill"),
+      name: absoluteDest,
+      description: "Absolute skill",
+    });
+
+    expect(await pathExists(absoluteDest)).toBe(false);
+
+    await syncSkillsToWorkspace({
+      sourceWorkspaceDir: sourceWorkspace,
+      targetWorkspaceDir: targetWorkspace,
+      bundledSkillsDir: path.join(sourceWorkspace, ".bundled"),
+      managedSkillsDir: path.join(sourceWorkspace, ".managed"),
+    });
+
+    expect(
+      await pathExists(path.join(targetWorkspace, "skills", "safe-absolute-skill", "SKILL.md")),
+    ).toBe(true);
+    expect(await pathExists(absoluteDest)).toBe(false);
+  });
   it("filters skills based on env/config gates", async () => {
     const workspaceDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-"));
     const skillDir = path.join(workspaceDir, "skills", "nano-banana-pro");

src/agents/skills/workspace.ts

@@ -16,6 +16,7 @@ import type {
 } from "./types.js";
 import { createSubsystemLogger } from "../../logging/subsystem.js";
 import { CONFIG_DIR, resolveUserPath } from "../../utils.js";
+import { resolveSandboxPath } from "../sandbox-paths.js";
 import { resolveBundledSkillsDir } from "./bundled-dir.js";
 import { shouldIncludeSkill } from "./config.js";
 import {
@@ -301,6 +302,45 @@ export function loadWorkspaceSkillEntries(
   return loadSkillEntries(workspaceDir, opts);
 }
 
+function resolveUniqueSyncedSkillDirName(base: string, used: Set<string>): string {
+  if (!used.has(base)) {
+    used.add(base);
+    return base;
+  }
+  for (let index = 2; index < 10_000; index += 1) {
+    const candidate = `${base}-${index}`;
+    if (!used.has(candidate)) {
+      used.add(candidate);
+      return candidate;
+    }
+  }
+  let fallbackIndex = 10_000;
+  let fallback = `${base}-${fallbackIndex}`;
+  while (used.has(fallback)) {
+    fallbackIndex += 1;
+    fallback = `${base}-${fallbackIndex}`;
+  }
+  used.add(fallback);
+  return fallback;
+}
+
+function resolveSyncedSkillDestinationPath(params: {
+  targetSkillsDir: string;
+  entry: SkillEntry;
+  usedDirNames: Set<string>;
+}): string | null {
+  const sourceDirName = path.basename(params.entry.skill.baseDir).trim();
+  if (!sourceDirName || sourceDirName === "." || sourceDirName === "..") {
+    return null;
+  }
+  const uniqueDirName = resolveUniqueSyncedSkillDirName(sourceDirName, params.usedDirNames);
+  return resolveSandboxPath({
+    filePath: uniqueDirName,
+    cwd: params.targetSkillsDir,
+    root: params.targetSkillsDir,
+  }).resolved;
+}
+
 export async function syncSkillsToWorkspace(params: {
   sourceWorkspaceDir: string;
   targetWorkspaceDir: string;
@@ -326,8 +366,28 @@ export async function syncSkillsToWorkspace(params: {
     await fsp.rm(targetSkillsDir, { recursive: true, force: true });
     await fsp.mkdir(targetSkillsDir, { recursive: true });
 
+    const usedDirNames = new Set<string>();
     for (const entry of entries) {
-      const dest = path.join(targetSkillsDir, entry.skill.name);
+      let dest: string | null = null;
+      try {
+        dest = resolveSyncedSkillDestinationPath({
+          targetSkillsDir,
+          entry,
+          usedDirNames,
+        });
+      } catch (error) {
+        const message = error instanceof Error ? error.message : JSON.stringify(error);
+        console.warn(
+          `[skills] Failed to resolve safe destination for ${entry.skill.name}: ${message}`,
+        );
+        continue;
+      }
+      if (!dest) {
+        console.warn(
+          `[skills] Failed to resolve safe destination for ${entry.skill.name}: invalid source directory name`,
+        );
+        continue;
+      }
       try {
         await fsp.cp(entry.skill.baseDir, dest, {
           recursive: true,