fix(skills): sandboxed turns fail with EACCES when refreshing read-only skill copies (#162295)

* fix(skills): refresh read-only sandbox skill copies

Repair owner access through confined directory descriptors before removing stale materialized skill trees. Preserve read-only sandbox mounts and source permissions. Cover prior read-only copies, nested cleanup, symlink confinement, and mount identity.

* fix(skills): unlink stale top-level skill symlinks

docs/gateway/sandboxing/workspace-access.md

@@ -94,6 +94,8 @@ Inbound media is copied into the active sandbox workspace (`media/inbound/*`).
 **Skills**: the `read` tool is sandbox-rooted. With `workspaceAccess: "none"`, OpenClaw mirrors eligible skills into the sandbox workspace (`.../skills`) as read-only instruction roots; other private workspace files remain writable. With `"rw"`, workspace skills are readable from `/workspace/skills`, and eligible managed, bundled, or plugin skills are materialized into the generated read-only path `/workspace/.openclaw/sandbox-skills/skills`.
 
 Local container mounts and sandbox file tools enforce these read-only roots.
+The Gateway refreshes its own mirrored copies even when an earlier copy inherited
+read-only directory permissions; no manual permission repair is needed.
 SSH and OpenShell shell execution relies on the remote host or OpenShell policy
 for filesystem restrictions; `workspaceAccess` alone does not make remote shell
 paths read-only.

src/skills/loading/workspace-skill-sync.runtime.test.ts

@@ -591,21 +591,50 @@ describe("syncWorkspaceSkills", () => {
   });
 
   it.runIf(process.platform !== "win32")(
-    "preserves the target skills directory while refreshing children",
+    "refreshes prior read-only copies without changing linked targets or the skills mount",
     async () => {
       const sourceWorkspace = await cloneSourceTemplate();
       const targetWorkspace = await createCaseDir("target");
       const targetSkillsDir = path.join(targetWorkspace, "skills");
-      await fs.mkdir(path.join(targetSkillsDir, "stale"), { recursive: true });
-      await fs.writeFile(path.join(targetSkillsDir, "stale", "SKILL.md"), "# Stale\n", "utf8");
+      const staleDir = path.join(targetSkillsDir, "demo-skill");
+      const nestedDir = path.join(staleDir, "references");
+      const outsideDir = await createCaseDir("outside");
+      const outsideFile = path.join(outsideDir, "SKILL.md");
+      await fs.mkdir(nestedDir, { recursive: true });
+      await fs.writeFile(path.join(staleDir, "SKILL.md"), "# Stale\n", { mode: 0o444 });
+      await fs.writeFile(path.join(nestedDir, "old.txt"), "old", { mode: 0o444 });
+      await fs.writeFile(outsideFile, "outside", { mode: 0o444 });
+      await fs.symlink(outsideDir, path.join(staleDir, "linked-directory"));
+      await fs.symlink(outsideFile, path.join(staleDir, "linked-file"));
+      await fs.symlink(outsideDir, path.join(targetSkillsDir, "linked-directory"));
+      await fs.symlink(outsideFile, path.join(targetSkillsDir, "linked-file"));
+      await fs.chmod(outsideDir, 0o555);
+      await fs.chmod(nestedDir, 0o555);
+      await fs.chmod(staleDir, 0o555);
       const before = await fs.stat(targetSkillsDir);
 
-      await syncSourceSkillsToTarget(sourceWorkspace, targetWorkspace);
-
-      const after = await fs.stat(targetSkillsDir);
-      expect(after.ino).toBe(before.ino);
-      expect(await pathExists(path.join(targetSkillsDir, "stale", "SKILL.md"))).toBe(false);
-      expect(await pathExists(path.join(targetSkillsDir, "demo-skill", "SKILL.md"))).toBe(true);
+      try {
+        await syncSourceSkillsToTarget(sourceWorkspace, targetWorkspace);
+        expect((await fs.stat(targetSkillsDir)).ino).toBe(before.ino);
+        expect(await fs.readFile(path.join(staleDir, "SKILL.md"), "utf8")).toContain(
+          "Workspace version",
+        );
+        expect(await pathExists(nestedDir)).toBe(false);
+        expect(await pathExists(path.join(targetSkillsDir, "linked-directory"))).toBe(false);
+        expect(await pathExists(path.join(targetSkillsDir, "linked-file"))).toBe(false);
+        expect(await fs.readFile(outsideFile, "utf8")).toBe("outside");
+        expect((await fs.stat(outsideDir)).mode & 0o777).toBe(0o555);
+        expect((await fs.stat(outsideFile)).mode & 0o777).toBe(0o444);
+        expect((await fs.stat(staleDir)).mode & 0o022).toBe(0);
+      } finally {
+        await fs.chmod(outsideDir, 0o755);
+        if (await pathExists(staleDir)) {
+          await fs.chmod(staleDir, 0o755);
+        }
+        if (await pathExists(nestedDir)) {
+          await fs.chmod(nestedDir, 0o755);
+        }
+      }
     },
   );
 

src/skills/loading/workspace-skill-sync.runtime.ts

@@ -6,7 +6,7 @@ import { resolveSandboxPath } from "../../agents/sandbox-paths.js";
 import { canonicalizePath } from "../../agents/utils/paths.js";
 import type { OpenClawConfig } from "../../config/types.openclaw.js";
 import { sha256Hex } from "../../infra/crypto-digest.js";
-import { hasErrnoCode } from "../../infra/errno.js";
+import { removePathWithinRoot } from "../../infra/fs-safe-remove.js";
 import { tryReadJson, writeJson } from "../../infra/json-files.js";
 import { pruneMapToMaxSize } from "../../infra/map-size.js";
 import { createSubsystemLogger } from "../../logging/subsystem.js";
@@ -32,15 +32,6 @@ const fsp = fs.promises;
 const skillsLogger = createSubsystemLogger("skills");
 const skillsSyncQueue = new KeyedAsyncQueue();
 
-function resolveUniqueSyncedSkillDirName(base: string, used: Set<string>): string {
-  let candidate = base;
-  for (let index = 2; used.has(candidate); index += 1) {
-    candidate = `${base}-${index}`;
-  }
-  used.add(candidate);
-  return candidate;
-}
-
 const SYNCED_SKILLS_MANIFEST_NAME = ".openclaw-sync.json";
 
 type SyncedSkillsManifest = {
@@ -89,41 +80,18 @@ function resolveSyncedSkillsManifestKey(manifest: SyncedSkillsManifest): string
   ]);
 }
 
-function resolveSyncedSkillDestinationPath(params: {
-  targetSkillsDir: string;
-  entry: SkillEntry;
-  usedDirNames: Set<string>;
-}): string | null {
-  const sourceDirName = (
-    params.entry.syncDirName ?? 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;
-}
-
 async function ensureSyncedSkillsDirectory(targetSkillsDir: string): Promise<void> {
-  let stats: fs.Stats;
   try {
-    stats = await fsp.lstat(targetSkillsDir);
+    if ((await fsp.lstat(targetSkillsDir)).isDirectory()) {
+      return;
+    }
+    await fsp.rm(targetSkillsDir, { recursive: true, force: true });
   } catch (error) {
     if ((error as NodeJS.ErrnoException).code !== "ENOENT") {
       throw error;
     }
-    await fsp.mkdir(targetSkillsDir, { recursive: true });
-    return;
-  }
-
-  if (!stats.isDirectory() || stats.isSymbolicLink()) {
-    await fsp.rm(targetSkillsDir, { recursive: true, force: true });
-    await fsp.mkdir(targetSkillsDir, { recursive: true });
   }
+  await fsp.mkdir(targetSkillsDir, { recursive: true });
 }
 
 export async function syncWorkspaceSkills(params: {
@@ -240,24 +208,27 @@ export async function syncWorkspaceSkills(params: {
         plans.push({ entry, identity });
         continue;
       }
-      let destinationPath: string | null;
+      let destinationPath: string;
       try {
-        destinationPath = resolveSyncedSkillDestinationPath({
-          targetSkillsDir,
-          entry,
-          usedDirNames,
-        });
+        const base = (entry.syncDirName ?? path.basename(entry.skill.baseDir)).trim();
+        if (!base || base === "." || base === "..") {
+          throw new Error("invalid source directory name");
+        }
+        let name = base;
+        for (let index = 2; usedDirNames.has(name); index += 1) {
+          name = `${base}-${index}`;
+        }
+        usedDirNames.add(name);
+        destinationPath = resolveSandboxPath({
+          filePath: name,
+          cwd: targetSkillsDir,
+          root: targetSkillsDir,
+        }).resolved;
       } catch (error) {
         const message = error instanceof Error ? error.message : JSON.stringify(error);
         skillsLogger.warn(`Failed to resolve safe destination for ${entry.skill.name}: ${message}`);
         continue;
       }
-      if (!destinationPath) {
-        skillsLogger.warn(
-          `Failed to resolve safe destination for ${entry.skill.name}: invalid source directory name`,
-        );
-        continue;
-      }
       plans.push({ destinationPath, entry, identity });
     }
 
@@ -281,17 +252,15 @@ export async function syncWorkspaceSkills(params: {
     );
     for (const child of await fsp.readdir(targetSkillsDir)) {
       if (!preservedDestinations.has(child)) {
-        const childPath = path.join(targetSkillsDir, child);
-        try {
-          await fsp.rm(childPath, { recursive: true, force: true });
-        } catch (error) {
-          const permissionDenied = hasErrnoCode(error, "EACCES") || hasErrnoCode(error, "EPERM");
-          if (process.platform === "win32" || !permissionDenied) {
-            throw error;
-          }
+        if ((await fsp.lstat(path.join(targetSkillsDir, child))).isDirectory()) {
           await ensureWritableSkillDirectories(targetSkillsDir, child);
-          await fsp.rm(childPath, { recursive: true, force: true });
         }
+        await removePathWithinRoot({
+          rootDir: targetDir,
+          relativePath: path.join("skills", child),
+          recursive: true,
+          symlinks: "unlink",
+        });
       }
     }