fix: harden workspace skill path containment

CHANGELOG.md

@@ -109,6 +109,7 @@ Docs: https://docs.openclaw.ai
 - Agents/Nodes media outputs: add dedicated `photos_latest` action handling, block media-returning `nodes invoke` commands, keep metadata-only `camera.list` invoke allowed, and normalize empty `photos_latest` results to a consistent response shape to prevent base64 context bloat. (#34332) Thanks @obviyus.
 - TUI/session-key canonicalization: normalize `openclaw tui --session` values to lowercase so uppercase session names no longer drop real-time streaming updates due to gateway/TUI key mismatches. (#33866, #34013) thanks @lynnzc.
 - iMessage/echo loop hardening: strip leaked assistant-internal scaffolding from outbound iMessage replies, drop reflected assistant-content messages before they re-enter inbound processing, extend echo-cache text retention for delayed reflections, and suppress repeated loop traffic before it amplifies into queue overflow. (#33295) Thanks @joelnishanth.
+- Skills/workspace boundary hardening: reject workspace and extra-dir skill roots or `SKILL.md` files whose realpath escapes the configured source root, and skip syncing those escaped skills into sandbox workspaces.
 - Outbound/send config threading: pass resolved SecretRef config through outbound adapters and helper send paths so send flows do not reload unresolved runtime config. (#33987) Thanks @joshavant.
 - Secrets/models.json persistence hardening: keep SecretRef-managed api keys + headers from persisting in generated models.json, expand audit/apply coverage, and harden marker handling/serialization. (#38955) Thanks @joshavant.
 - Sessions/subagent attachments: remove `attachments[].content.maxLength` from `sessions_spawn` schema to avoid llama.cpp GBNF repetition overflow, and preflight UTF-8 byte size before buffer allocation while keeping runtime file-size enforcement unchanged. (#33648) Thanks @anisoptera.

docs/tools/skills.md

@@ -70,6 +70,7 @@ that up as `<workspace>/skills` on the next session.
 
 - Treat third-party skills as **untrusted code**. Read them before enabling.
 - Prefer sandboxed runs for untrusted inputs and risky tools. See [Sandboxing](/gateway/sandboxing).
+- Workspace and extra-dir skill discovery only accepts skill roots and `SKILL.md` files whose resolved realpath stays inside the configured root.
 - `skills.entries.*.env` and `skills.entries.*.apiKey` inject secrets into the **host** process
   for that agent turn (not the sandbox). Keep secrets out of prompts and logs.
 - For a broader threat model and checklists, see [Security](/gateway/security).

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

@@ -95,6 +95,46 @@ describe("buildWorkspaceSkillsPrompt", () => {
     expect(prompt).not.toContain("Extra version");
     expect(prompt.replaceAll("\\", "/")).toContain("demo-skill/SKILL.md");
   });
+  it.runIf(process.platform !== "win32")(
+    "does not sync workspace skills that resolve outside the source workspace root",
+    async () => {
+      const sourceWorkspace = await createCaseDir("source");
+      const targetWorkspace = await createCaseDir("target");
+      const outsideRoot = await createCaseDir("outside");
+      const outsideSkillDir = path.join(outsideRoot, "escaped-skill");
+
+      await writeSkill({
+        dir: outsideSkillDir,
+        name: "escaped-skill",
+        description: "Outside source workspace",
+      });
+      await fs.mkdir(path.join(sourceWorkspace, "skills"), { recursive: true });
+      await fs.symlink(
+        outsideSkillDir,
+        path.join(sourceWorkspace, "skills", "escaped-skill"),
+        "dir",
+      );
+
+      await withEnv({ HOME: sourceWorkspace, PATH: "" }, () =>
+        syncSkillsToWorkspace({
+          sourceWorkspaceDir: sourceWorkspace,
+          targetWorkspaceDir: targetWorkspace,
+          bundledSkillsDir: path.join(sourceWorkspace, ".bundled"),
+          managedSkillsDir: path.join(sourceWorkspace, ".managed"),
+        }),
+      );
+
+      const prompt = buildPrompt(targetWorkspace, {
+        bundledSkillsDir: path.join(targetWorkspace, ".bundled"),
+        managedSkillsDir: path.join(targetWorkspace, ".managed"),
+      });
+
+      expect(prompt).not.toContain("escaped-skill");
+      expect(
+        await pathExists(path.join(targetWorkspace, "skills", "escaped-skill", "SKILL.md")),
+      ).toBe(false);
+    },
+  );
   it("keeps synced skills confined under target workspace when frontmatter name uses traversal", async () => {
     const sourceWorkspace = await createCaseDir("source");
     const targetWorkspace = await createCaseDir("target");

src/agents/skills.loadworkspaceskillentries.test.ts

@@ -2,6 +2,7 @@ import fs from "node:fs/promises";
 import os from "node:os";
 import path from "node:path";
 import { afterEach, describe, expect, it } from "vitest";
+import { writeSkill } from "./skills.e2e-test-helpers.js";
 import { loadWorkspaceSkillEntries } from "./skills.js";
 import { writePluginWithSkill } from "./test-helpers/skill-plugin-fixtures.js";
 
@@ -128,4 +129,50 @@ describe("loadWorkspaceSkillEntries", () => {
 
     expect(entries.map((entry) => entry.skill.name)).not.toContain("diffs");
   });
+
+  it.runIf(process.platform !== "win32")(
+    "skips workspace skill directories that resolve outside the workspace root",
+    async () => {
+      const workspaceDir = await createTempWorkspaceDir();
+      const outsideDir = await createTempWorkspaceDir();
+      const escapedSkillDir = path.join(outsideDir, "outside-skill");
+      await writeSkill({
+        dir: escapedSkillDir,
+        name: "outside-skill",
+        description: "Outside",
+      });
+      await fs.mkdir(path.join(workspaceDir, "skills"), { recursive: true });
+      await fs.symlink(escapedSkillDir, path.join(workspaceDir, "skills", "escaped-skill"), "dir");
+
+      const entries = loadWorkspaceSkillEntries(workspaceDir, {
+        managedSkillsDir: path.join(workspaceDir, ".managed"),
+        bundledSkillsDir: path.join(workspaceDir, ".bundled"),
+      });
+
+      expect(entries.map((entry) => entry.skill.name)).not.toContain("outside-skill");
+    },
+  );
+
+  it.runIf(process.platform !== "win32")(
+    "skips workspace skill files that resolve outside the workspace root",
+    async () => {
+      const workspaceDir = await createTempWorkspaceDir();
+      const outsideDir = await createTempWorkspaceDir();
+      await writeSkill({
+        dir: outsideDir,
+        name: "outside-file-skill",
+        description: "Outside file",
+      });
+      const skillDir = path.join(workspaceDir, "skills", "escaped-file");
+      await fs.mkdir(skillDir, { recursive: true });
+      await fs.symlink(path.join(outsideDir, "SKILL.md"), path.join(skillDir, "SKILL.md"));
+
+      const entries = loadWorkspaceSkillEntries(workspaceDir, {
+        managedSkillsDir: path.join(workspaceDir, ".managed"),
+        bundledSkillsDir: path.join(workspaceDir, ".bundled"),
+      });
+
+      expect(entries.map((entry) => entry.skill.name)).not.toContain("outside-file-skill");
+    },
+  );
 });

src/agents/skills/workspace.ts

@@ -7,6 +7,7 @@ import {
   type Skill,
 } from "@mariozechner/pi-coding-agent";
 import type { OpenClawConfig } from "../../config/config.js";
+import { isPathInside } from "../../infra/path-guards.js";
 import { createSubsystemLogger } from "../../logging/subsystem.js";
 import { CONFIG_DIR, resolveUserPath } from "../../utils.js";
 import { resolveSandboxPath } from "../sandbox-paths.js";
@@ -175,6 +176,76 @@ function listChildDirectories(dir: string): string[] {
   }
 }
 
+function tryRealpath(filePath: string): string | null {
+  try {
+    return fs.realpathSync(filePath);
+  } catch {
+    return null;
+  }
+}
+
+function warnEscapedSkillPath(params: {
+  source: string;
+  rootDir: string;
+  candidatePath: string;
+  candidateRealPath: string;
+}) {
+  skillsLogger.warn("Skipping skill path that resolves outside its configured root.", {
+    source: params.source,
+    rootDir: params.rootDir,
+    path: params.candidatePath,
+    realPath: params.candidateRealPath,
+  });
+}
+
+function resolveContainedSkillPath(params: {
+  source: string;
+  rootDir: string;
+  rootRealPath: string;
+  candidatePath: string;
+}): string | null {
+  const candidateRealPath = tryRealpath(params.candidatePath);
+  if (!candidateRealPath) {
+    return null;
+  }
+  if (isPathInside(params.rootRealPath, candidateRealPath)) {
+    return candidateRealPath;
+  }
+  warnEscapedSkillPath({
+    source: params.source,
+    rootDir: params.rootDir,
+    candidatePath: path.resolve(params.candidatePath),
+    candidateRealPath,
+  });
+  return null;
+}
+
+function filterLoadedSkillsInsideRoot(params: {
+  skills: Skill[];
+  source: string;
+  rootDir: string;
+  rootRealPath: string;
+}): Skill[] {
+  return params.skills.filter((skill) => {
+    const baseDirRealPath = resolveContainedSkillPath({
+      source: params.source,
+      rootDir: params.rootDir,
+      rootRealPath: params.rootRealPath,
+      candidatePath: skill.baseDir,
+    });
+    if (!baseDirRealPath) {
+      return false;
+    }
+    const skillFileRealPath = resolveContainedSkillPath({
+      source: params.source,
+      rootDir: params.rootDir,
+      rootRealPath: params.rootRealPath,
+      candidatePath: skill.filePath,
+    });
+    return Boolean(skillFileRealPath);
+  });
+}
+
 function resolveNestedSkillsRoot(
   dir: string,
   opts?: {
@@ -229,16 +300,36 @@ function loadSkillEntries(
   const limits = resolveSkillsLimits(opts?.config);
 
   const loadSkills = (params: { dir: string; source: string }): Skill[] => {
+    const rootDir = path.resolve(params.dir);
+    const rootRealPath = tryRealpath(rootDir) ?? rootDir;
     const resolved = resolveNestedSkillsRoot(params.dir, {
       maxEntriesToScan: limits.maxCandidatesPerRoot,
     });
     const baseDir = resolved.baseDir;
+    const baseDirRealPath = resolveContainedSkillPath({
+      source: params.source,
+      rootDir,
+      rootRealPath,
+      candidatePath: baseDir,
+    });
+    if (!baseDirRealPath) {
+      return [];
+    }
 
     // If the root itself is a skill directory, just load it directly (but enforce size cap).
     const rootSkillMd = path.join(baseDir, "SKILL.md");
     if (fs.existsSync(rootSkillMd)) {
+      const rootSkillRealPath = resolveContainedSkillPath({
+        source: params.source,
+        rootDir,
+        rootRealPath: baseDirRealPath,
+        candidatePath: rootSkillMd,
+      });
+      if (!rootSkillRealPath) {
+        return [];
+      }
       try {
-        const size = fs.statSync(rootSkillMd).size;
+        const size = fs.statSync(rootSkillRealPath).size;
         if (size > limits.maxSkillFileBytes) {
           skillsLogger.warn("Skipping skills root due to oversized SKILL.md.", {
             dir: baseDir,
@@ -253,7 +344,12 @@ function loadSkillEntries(
       }
 
       const loaded = loadSkillsFromDir({ dir: baseDir, source: params.source });
-      return unwrapLoadedSkills(loaded);
+      return filterLoadedSkillsInsideRoot({
+        skills: unwrapLoadedSkills(loaded),
+        source: params.source,
+        rootDir,
+        rootRealPath: baseDirRealPath,
+      });
     }
 
     const childDirs = listChildDirectories(baseDir);
@@ -284,12 +380,30 @@ function loadSkillEntries(
     // Only consider immediate subfolders that look like skills (have SKILL.md) and are under size cap.
     for (const name of limitedChildren) {
       const skillDir = path.join(baseDir, name);
+      const skillDirRealPath = resolveContainedSkillPath({
+        source: params.source,
+        rootDir,
+        rootRealPath: baseDirRealPath,
+        candidatePath: skillDir,
+      });
+      if (!skillDirRealPath) {
+        continue;
+      }
       const skillMd = path.join(skillDir, "SKILL.md");
       if (!fs.existsSync(skillMd)) {
         continue;
       }
+      const skillMdRealPath = resolveContainedSkillPath({
+        source: params.source,
+        rootDir,
+        rootRealPath: baseDirRealPath,
+        candidatePath: skillMd,
+      });
+      if (!skillMdRealPath) {
+        continue;
+      }
       try {
-        const size = fs.statSync(skillMd).size;
+        const size = fs.statSync(skillMdRealPath).size;
         if (size > limits.maxSkillFileBytes) {
           skillsLogger.warn("Skipping skill due to oversized SKILL.md.", {
             skill: name,
@@ -304,7 +418,14 @@ function loadSkillEntries(
       }
 
       const loaded = loadSkillsFromDir({ dir: skillDir, source: params.source });
-      loadedSkills.push(...unwrapLoadedSkills(loaded));
+      loadedSkills.push(
+        ...filterLoadedSkillsInsideRoot({
+          skills: unwrapLoadedSkills(loaded),
+          source: params.source,
+          rootDir,
+          rootRealPath: baseDirRealPath,
+        }),
+      );
 
       if (loadedSkills.length >= limits.maxSkillsLoadedPerSource) {
         break;