diff --git a/docs/gateway/sandboxing.md b/docs/gateway/sandboxing.md index 80860a851556..8bf468d30728 100644 --- a/docs/gateway/sandboxing.md +++ b/docs/gateway/sandboxing.md @@ -318,7 +318,7 @@ With the OpenShell backend: Inbound media is copied into the active sandbox workspace (`media/inbound/*`). -**Skills note:** the `read` tool is sandbox-rooted. With `workspaceAccess: "none"`, OpenClaw mirrors eligible skills into the sandbox workspace (`.../skills`) so they can be read. With `"rw"`, workspace skills are readable from `/workspace/skills`. +**Skills note:** the `read` tool is sandbox-rooted. With `workspaceAccess: "none"`, OpenClaw mirrors eligible skills into the sandbox workspace (`.../skills`) so they can be read. 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`. ## Custom bind mounts diff --git a/extensions/openshell/src/backend.ts b/extensions/openshell/src/backend.ts index 5e36f00c2f24..8faedb061380 100644 --- a/extensions/openshell/src/backend.ts +++ b/extensions/openshell/src/backend.ts @@ -31,6 +31,7 @@ import { resolveOpenShellPluginConfig, type ResolvedOpenShellPluginConfig } from import { createOpenShellFsBridge } from "./fs-bridge.js"; import { DEFAULT_OPEN_SHELL_MIRROR_EXCLUDE_DIRS, + movePathWithCopyFallback, replaceDirectoryContents, stageDirectoryContents, } from "./mirror.js"; @@ -43,6 +44,48 @@ type PendingExec = { sshSession: SshSandboxSession; }; +const MATERIALIZED_SKILLS_REMOTE_PARTS = [".openclaw", "sandbox-skills"] as const; +const ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT = [ + "set -e", + 'target="$1"', + 'root="${2:-$1}"', + 'case "$target" in /*) ;; *) echo "remote directory must be absolute: $target" >&2; exit 1 ;; esac', + 'case "$root" in /*) ;; *) echo "remote root must be absolute: $root" >&2; exit 1 ;; esac', + 'target="${target%/}"', + 'root="${root%/}"', + '[ -n "$target" ] || target="/"', + '[ -n "$root" ] || root="/"', + 'case "$target/" in "$root"/*|"$root/") ;; *) echo "remote directory must stay under root: $target" >&2; exit 1 ;; esac', + 'old_ifs="$IFS"', + 'IFS="/"', + "set -- ${target#/} ${root#/}", + 'IFS="$old_ifs"', + "for part do", + ' [ -n "$part" ] || continue', + ' case "$part" in "."|"..") echo "unsafe remote directory component: $part" >&2; exit 1 ;; esac', + "done", + 'if [ -L "$root" ]; then echo "unsafe remote root symlink: $root" >&2; exit 1; fi', + 'mkdir -p -- "$root"', + 'canonical_root="$(cd "$root" && pwd -P)"', + 'relative="${target#"$root"}"', + 'relative="${relative#/}"', + 'current="$canonical_root"', + 'IFS="/"', + "set -- $relative", + 'IFS="$old_ifs"', + "for part do", + ' [ -n "$part" ] || continue', + ' if [ "$current" = "/" ]; then next="/$part"; else next="$current/$part"; fi', + ' if [ -L "$next" ]; then echo "unsafe remote directory symlink: $next" >&2; exit 1; fi', + ' if [ -e "$next" ]; then', + ' if [ ! -d "$next" ]; then echo "unsafe remote directory component: $next" >&2; exit 1; fi', + " else", + ' mkdir -- "$next"', + " fi", + ' current="$next"', + "done", +].join("\n"); + export function buildOpenShellSshExecEnv(): NodeJS.ProcessEnv { return sanitizeEnvVars(process.env).allowed; } @@ -221,7 +264,10 @@ class OpenShellSandboxBackendImpl { if (this.params.execContext.config.mode === "mirror") { await this.syncWorkspaceToRemote(); } else { - await this.maybeSeedRemoteWorkspace(); + const seeded = await this.maybeSeedRemoteWorkspace(); + if (!seeded) { + await this.syncSkillsWorkspaceToRemote(); + } } const sshSession = await createOpenShellSshSession({ context: this.params.execContext, @@ -257,7 +303,10 @@ class OpenShellSandboxBackendImpl { params: SandboxBackendCommandParams, ): Promise { await this.ensureSandboxExists(); - await this.maybeSeedRemoteWorkspace(); + const seeded = await this.maybeSeedRemoteWorkspace(); + if (!seeded) { + await this.syncSkillsWorkspaceToRemote(); + } return await this.runRemoteShellScriptInternal(params); } @@ -410,6 +459,31 @@ class OpenShellSandboxBackendImpl { this.params.remoteAgentWorkspaceDir, ); } + await this.syncSkillsWorkspaceToRemote(); + } + + private async syncSkillsWorkspaceToRemote(): Promise { + if ( + this.params.createParams.cfg.workspaceAccess !== "rw" || + !this.params.createParams.skillsWorkspaceDir + ) { + return; + } + const remoteSkillsWorkspaceDir = resolveRemoteMaterializedSkillsWorkspaceDir( + this.params.remoteWorkspaceDir, + ); + await this.runRemoteShellScriptInternal({ + script: `${ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT}\nfind "$1" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +`, + args: [remoteSkillsWorkspaceDir, this.params.remoteWorkspaceDir], + }); + const stats = await fs.lstat(this.params.createParams.skillsWorkspaceDir).catch(() => null); + if (!stats?.isDirectory() || stats.isSymbolicLink()) { + return; + } + await this.uploadPathToRemote( + this.params.createParams.skillsWorkspaceDir, + remoteSkillsWorkspaceDir, + ); } private async syncWorkspaceFromRemote(): Promise { @@ -430,13 +504,25 @@ class OpenShellSandboxBackendImpl { if (result.code !== 0) { throw new Error(result.stderr.trim() || "openshell sandbox download failed"); } - await replaceDirectoryContents({ - sourceDir: tmpDir, - targetDir: this.params.createParams.workspaceDir, - // Never sync trusted host hook directories or repository metadata from - // the remote sandbox. - excludeDirs: DEFAULT_OPEN_SHELL_MIRROR_EXCLUDE_DIRS, + await removeMaterializedSkillsFromDownloadedWorkspace(tmpDir); + const preservedSandboxSkills = await moveMaterializedSkillsShadowAside({ + workspaceDir: this.params.createParams.workspaceDir, + tmpDir, }); + try { + await replaceDirectoryContents({ + sourceDir: tmpDir, + targetDir: this.params.createParams.workspaceDir, + // Never sync trusted host hook directories or repository metadata from + // the remote sandbox. + excludeDirs: DEFAULT_OPEN_SHELL_MIRROR_EXCLUDE_DIRS, + }); + } finally { + await restoreMaterializedSkillsShadow({ + workspaceDir: this.params.createParams.workspaceDir, + preserved: preservedSandboxSkills, + }); + } }, ); } @@ -470,13 +556,14 @@ class OpenShellSandboxBackendImpl { ); } - private async maybeSeedRemoteWorkspace(): Promise { + private async maybeSeedRemoteWorkspace(): Promise { if (!this.remoteSeedPending) { - return; + return false; } this.remoteSeedPending = false; try { await this.syncWorkspaceToRemote(); + return true; } catch (error) { this.remoteSeedPending = true; throw error; @@ -508,6 +595,84 @@ export function buildOpenShellSandboxName(scopeKey: string): string { return `openclaw-${safe || "session"}-${hash.toString(16).slice(0, 8)}`; } +function resolveRemoteMaterializedSkillsWorkspaceDir(remoteWorkspaceDir: string): string { + const root = remoteWorkspaceDir.replace(/\\/g, "/").replace(/\/+$/, "") || "/"; + return path.posix.join(root, ...MATERIALIZED_SKILLS_REMOTE_PARTS); +} + +async function removeMaterializedSkillsFromDownloadedWorkspace(tmpDir: string): Promise { + let cursor = tmpDir; + for (const [index, part] of MATERIALIZED_SKILLS_REMOTE_PARTS.entries()) { + const next = path.join(cursor, part); + const stats = await fs.lstat(next).catch(() => null); + if (!stats) { + return; + } + if (index === MATERIALIZED_SKILLS_REMOTE_PARTS.length - 1) { + await fs.rm(next, { recursive: true, force: true }); + return; + } + if (stats.isSymbolicLink() || !stats.isDirectory()) { + await fs.rm(next, { recursive: true, force: true }); + return; + } + cursor = next; + } +} + +async function moveMaterializedSkillsShadowAside(params: { + workspaceDir: string; + tmpDir: string; +}): Promise<{ preservedPath: string; preserveRoot: string } | undefined> { + const shadowPath = path.join(params.workspaceDir, ...MATERIALIZED_SKILLS_REMOTE_PARTS); + const parentStats = await fs.lstat(path.dirname(shadowPath)).catch(() => null); + if (!parentStats?.isDirectory() || parentStats.isSymbolicLink()) { + return undefined; + } + const shadowStats = await fs.lstat(shadowPath).catch(() => null); + if (!shadowStats || shadowStats.isSymbolicLink()) { + return undefined; + } + const preserveRoot = await fs.mkdtemp( + path.join(path.dirname(params.tmpDir), "openclaw-openshell-preserve-"), + ); + const preservedPath = path.join(preserveRoot, "sandbox-skills"); + await movePathWithCopyFallback({ from: shadowPath, to: preservedPath }); + return { preservedPath, preserveRoot }; +} + +async function restoreMaterializedSkillsShadow(params: { + workspaceDir: string; + preserved?: { preservedPath: string; preserveRoot: string }; +}): Promise { + if (!params.preserved) { + return; + } + let restored = false; + try { + const shadowPath = path.join(params.workspaceDir, ...MATERIALIZED_SKILLS_REMOTE_PARTS); + const parentPath = path.dirname(shadowPath); + const parentStats = await fs.lstat(parentPath).catch(() => null); + if (parentStats?.isSymbolicLink()) { + throw new Error(`Refusing to restore sandbox skills through symlink parent: ${parentPath}`); + } + if (parentStats && !parentStats.isDirectory()) { + await fs.rm(parentPath, { recursive: true, force: true }); + } + await fs.mkdir(parentPath, { recursive: true }); + await fs.rm(shadowPath, { recursive: true, force: true }); + await movePathWithCopyFallback({ + from: params.preserved.preservedPath, + to: shadowPath, + }); + restored = true; + } finally { + if (restored) { + await fs.rm(params.preserved.preserveRoot, { recursive: true, force: true }); + } + } +} + function resolveOpenShellTmpRoot(): string { return path.resolve(resolvePreferredOpenClawTmpDir()); } diff --git a/extensions/openshell/src/fs-bridge.ts b/extensions/openshell/src/fs-bridge.ts index 74b763543196..0854e2b918d1 100644 --- a/extensions/openshell/src/fs-bridge.ts +++ b/extensions/openshell/src/fs-bridge.ts @@ -15,9 +15,11 @@ import { movePathWithCopyFallback } from "./mirror.js"; type ResolvedMountPath = SandboxResolvedPath & { mountHostRoot: string; writable: boolean; - source: "workspace" | "agent"; + source: "workspace" | "agent" | "protectedSkill"; }; +const MATERIALIZED_SKILLS_CONTAINER_PARTS = [".openclaw", "sandbox-skills", "skills"] as const; + export function createOpenShellFsBridge(params: { sandbox: OpenShellFsBridgeContext; backend: OpenShellSandboxBackend; @@ -230,8 +232,30 @@ class OpenShellFsBridge implements SandboxFsBridge { "/", ); const workspaceContainerRoot = this.sandbox.containerWorkdir.replace(/\\/g, "/"); + const skillsRoot = this.sandbox.skillsWorkspaceDir + ? path.resolve(this.sandbox.skillsWorkspaceDir, "skills") + : undefined; + const skillsContainerRoot = path.posix.join( + workspaceContainerRoot, + ...MATERIALIZED_SKILLS_CONTAINER_PARTS, + ); + const workspaceSkillsShadowRoot = path.resolve( + workspaceRoot, + ...MATERIALIZED_SKILLS_CONTAINER_PARTS, + ); const input = params.filePath.trim(); + if (skillsRoot && this.sandbox.workspaceAccess === "rw") { + const protectedSkillTarget = resolveProtectedSkillTarget({ + input, + skillsRoot, + skillsContainerRoot, + }); + if (protectedSkillTarget) { + return protectedSkillTarget; + } + } + if (input.startsWith(`${workspaceContainerRoot}/`) || input === workspaceContainerRoot) { const relative = path.posix.relative(workspaceContainerRoot, input) || ""; const hostPath = relative @@ -270,6 +294,18 @@ class OpenShellFsBridge implements SandboxFsBridge { const cwd = params.cwd ? path.resolve(params.cwd) : workspaceRoot; const hostPath = path.isAbsolute(input) ? path.resolve(input) : path.resolve(cwd, input); + if (skillsRoot && this.sandbox.workspaceAccess === "rw") { + const protectedSkillShadowTarget = resolveProtectedSkillShadowTarget({ + hostPath, + workspaceSkillsShadowRoot, + skillsRoot, + skillsContainerRoot, + }); + if (protectedSkillShadowTarget) { + return protectedSkillShadowTarget; + } + } + if (isPathInside(workspaceRoot, hostPath)) { const relative = path.relative(workspaceRoot, hostPath).split(path.sep).join(path.posix.sep); return { @@ -284,6 +320,22 @@ class OpenShellFsBridge implements SandboxFsBridge { }; } + if (skillsRoot && this.sandbox.workspaceAccess === "rw" && isPathInside(skillsRoot, hostPath)) { + const relative = path.relative(skillsRoot, hostPath).split(path.sep).join(path.posix.sep); + return { + hostPath, + relativePath: relative + ? path.posix.join(...MATERIALIZED_SKILLS_CONTAINER_PARTS, relative) + : path.posix.join(...MATERIALIZED_SKILLS_CONTAINER_PARTS), + containerPath: relative + ? path.posix.join(skillsContainerRoot, relative) + : skillsContainerRoot, + mountHostRoot: skillsRoot, + writable: false, + source: "protectedSkill", + }; + } + if (hasAgentMount && isPathInside(agentRoot, hostPath)) { const relative = path.relative(agentRoot, hostPath).split(path.sep).join(path.posix.sep); return { @@ -302,6 +354,72 @@ class OpenShellFsBridge implements SandboxFsBridge { } } +function resolveProtectedSkillTarget(params: { + input: string; + skillsRoot: string; + skillsContainerRoot: string; +}): ResolvedMountPath | null { + const relativeRoot = path.posix.join(...MATERIALIZED_SKILLS_CONTAINER_PARTS); + const normalizedInput = path.posix.normalize(params.input.replace(/\\/g, "/")); + const isAbsoluteContainer = + normalizedInput === params.skillsContainerRoot || + normalizedInput.startsWith(`${params.skillsContainerRoot}/`); + const isRelativeContainer = + normalizedInput === relativeRoot || normalizedInput.startsWith(`${relativeRoot}/`); + if (!isAbsoluteContainer && !isRelativeContainer) { + return null; + } + + const relative = isAbsoluteContainer + ? path.posix.relative(params.skillsContainerRoot, normalizedInput) + : path.posix.relative(relativeRoot, normalizedInput); + const safeRelative = relative === "." ? "" : relative; + const hostPath = safeRelative + ? path.resolve(params.skillsRoot, ...safeRelative.split("/")) + : params.skillsRoot; + return { + hostPath, + relativePath: safeRelative ? path.posix.join(relativeRoot, safeRelative) : relativeRoot, + containerPath: safeRelative + ? path.posix.join(params.skillsContainerRoot, safeRelative) + : params.skillsContainerRoot, + mountHostRoot: params.skillsRoot, + writable: false, + source: "protectedSkill", + }; +} + +function resolveProtectedSkillShadowTarget(params: { + hostPath: string; + workspaceSkillsShadowRoot: string; + skillsRoot: string; + skillsContainerRoot: string; +}): ResolvedMountPath | null { + if (!isPathInside(params.workspaceSkillsShadowRoot, params.hostPath)) { + return null; + } + + const relative = path + .relative(params.workspaceSkillsShadowRoot, params.hostPath) + .split(path.sep) + .join(path.posix.sep); + const safeRelative = relative === "." ? "" : relative; + const hostPath = safeRelative + ? path.resolve(params.skillsRoot, ...safeRelative.split("/")) + : params.skillsRoot; + const relativeRoot = path.posix.join(...MATERIALIZED_SKILLS_CONTAINER_PARTS); + return { + hostPath, + relativePath: safeRelative ? path.posix.join(relativeRoot, safeRelative) : relativeRoot, + containerPath: safeRelative + ? path.posix.join(params.skillsContainerRoot, safeRelative) + : params.skillsContainerRoot, + mountHostRoot: params.skillsRoot, + writable: false, + source: "protectedSkill", + }; +} + async function assertLocalPathSafety(params: { target: ResolvedMountPath; root: string; diff --git a/extensions/openshell/src/openshell-core.test.ts b/extensions/openshell/src/openshell-core.test.ts index 332e5dce3dd0..b40bccd13895 100644 --- a/extensions/openshell/src/openshell-core.test.ts +++ b/extensions/openshell/src/openshell-core.test.ts @@ -304,6 +304,155 @@ describe("openshell backend manager", () => { ).rejects.toThrow(/unresolved placeholder token /); expect(cliMocks.runOpenShellCli).not.toHaveBeenCalled(); }); + + it("preserves a local sandbox skills shadow when mirror sync crosses filesystems", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-workspace-"); + const shadowFile = path.join(workspaceDir, ".openclaw", "sandbox-skills", "user-note.txt"); + await fs.mkdir(path.dirname(shadowFile), { recursive: true }); + await fs.writeFile(shadowFile, "local shadow", "utf8"); + + const originalRename = fs.rename.bind(fs); + const renameSpy = vi.spyOn(fs, "rename").mockImplementation(async (from, to) => { + const source = String(from); + const target = String(to); + const shadowDir = path.dirname(shadowFile); + const isFallbackStagedMove = path.basename(source).startsWith(".fs-safe-move-"); + if (source === shadowDir || (target === shadowDir && !isFallbackStagedMove)) { + throw Object.assign(new Error("cross-device link not permitted"), { code: "EXDEV" }); + } + return await originalRename(from, to); + }); + cliMocks.runOpenShellCli.mockImplementation(async ({ args }: { args: string[] }) => { + if (args[0] === "sandbox" && args[1] === "download") { + const tmpDir = args[4]; + await fs.writeFile(path.join(tmpDir, "from-remote.txt"), "remote", "utf8"); + await fs.mkdir(path.join(tmpDir, ".openclaw", "sandbox-skills", "skills"), { + recursive: true, + }); + await fs.writeFile( + path.join(tmpDir, ".openclaw", "sandbox-skills", "skills", "generated.txt"), + "generated", + "utf8", + ); + } + return { code: 0, stdout: "", stderr: "" }; + }); + + const factory = createOpenShellSandboxBackendFactory({ + pluginConfig: resolveOpenShellPluginConfig({ + command: "openshell", + mode: "mirror", + }), + }); + const backend = await factory({ + sessionKey: "agent:main:turn", + scopeKey: "agent:main", + workspaceDir, + agentWorkspaceDir: workspaceDir, + cfg: createOpenShellBackendSandboxConfig(), + }); + + try { + await backend.finalizeExec?.({ + status: "completed", + exitCode: 0, + timedOut: false, + token: undefined, + }); + + expect(renameSpy).toHaveBeenCalled(); + await expect(fs.readFile(shadowFile, "utf8")).resolves.toBe("local shadow"); + await expect(fs.readFile(path.join(workspaceDir, "from-remote.txt"), "utf8")).resolves.toBe( + "remote", + ); + await expectPathMissing( + path.join(workspaceDir, ".openclaw", "sandbox-skills", "skills", "generated.txt"), + ); + } finally { + renameSpy.mockRestore(); + } + }); + + it("drops non-directory materialized sandbox skills from mirror downloads", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-workspace-"); + cliMocks.runOpenShellCli.mockImplementation(async ({ args }: { args: string[] }) => { + if (args[0] === "sandbox" && args[1] === "download") { + const tmpDir = args[4]; + await fs.writeFile(path.join(tmpDir, "from-remote.txt"), "remote", "utf8"); + await fs.mkdir(path.join(tmpDir, ".openclaw"), { recursive: true }); + await fs.writeFile(path.join(tmpDir, ".openclaw", "sandbox-skills"), "poison", "utf8"); + } + return { code: 0, stdout: "", stderr: "" }; + }); + + const factory = createOpenShellSandboxBackendFactory({ + pluginConfig: resolveOpenShellPluginConfig({ + command: "openshell", + mode: "mirror", + }), + }); + const backend = await factory({ + sessionKey: "agent:main:turn", + scopeKey: "agent:main", + workspaceDir, + agentWorkspaceDir: workspaceDir, + cfg: createOpenShellBackendSandboxConfig(), + }); + + await backend.finalizeExec?.({ + status: "completed", + exitCode: 0, + timedOut: false, + token: undefined, + }); + + await expect(fs.readFile(path.join(workspaceDir, "from-remote.txt"), "utf8")).resolves.toBe( + "remote", + ); + await expectPathMissing(path.join(workspaceDir, ".openclaw", "sandbox-skills")); + }); + + it("restores a local sandbox skills shadow when mirror download has a file parent", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-workspace-"); + const shadowFile = path.join(workspaceDir, ".openclaw", "sandbox-skills", "user-note.txt"); + await fs.mkdir(path.dirname(shadowFile), { recursive: true }); + await fs.writeFile(shadowFile, "local shadow", "utf8"); + cliMocks.runOpenShellCli.mockImplementation(async ({ args }: { args: string[] }) => { + if (args[0] === "sandbox" && args[1] === "download") { + const tmpDir = args[4]; + await fs.writeFile(path.join(tmpDir, "from-remote.txt"), "remote", "utf8"); + await fs.writeFile(path.join(tmpDir, ".openclaw"), "poison", "utf8"); + } + return { code: 0, stdout: "", stderr: "" }; + }); + + const factory = createOpenShellSandboxBackendFactory({ + pluginConfig: resolveOpenShellPluginConfig({ + command: "openshell", + mode: "mirror", + }), + }); + const backend = await factory({ + sessionKey: "agent:main:turn", + scopeKey: "agent:main", + workspaceDir, + agentWorkspaceDir: workspaceDir, + cfg: createOpenShellBackendSandboxConfig(), + }); + + await backend.finalizeExec?.({ + status: "completed", + exitCode: 0, + timedOut: false, + token: undefined, + }); + + await expect(fs.readFile(path.join(workspaceDir, "from-remote.txt"), "utf8")).resolves.toBe( + "remote", + ); + await expect(fs.readFile(shadowFile, "utf8")).resolves.toBe("local shadow"); + expect((await fs.stat(path.join(workspaceDir, ".openclaw"))).isDirectory()).toBe(true); + }); }); const tempDirs: string[] = []; @@ -517,6 +666,64 @@ describe("openshell fs bridges", () => { ); }); + it("reads materialized sandbox skills from the protected skills workspace", async () => { + const workspaceDir = await makeTempDir("openclaw-openshell-fs-"); + const skillsWorkspaceDir = await makeTempDir("openclaw-openshell-skills-"); + const skillFile = path.join(skillsWorkspaceDir, "skills", "demo", "SKILL.md"); + const shadowFile = path.join( + workspaceDir, + ".openclaw", + "sandbox-skills", + "skills", + "demo", + "SKILL.md", + ); + await fs.mkdir(path.dirname(skillFile), { recursive: true }); + await fs.mkdir(path.dirname(shadowFile), { recursive: true }); + await fs.writeFile(skillFile, "# Demo\nmaterialized\n", "utf8"); + await fs.writeFile(shadowFile, "# Demo\nworkspace shadow\n", "utf8"); + + const backend = createMirrorBackendMock(); + const sandbox = createSandboxTestContext({ + overrides: { + backendId: "openshell", + workspaceDir, + agentWorkspaceDir: workspaceDir, + skillsWorkspaceDir, + workspaceAccess: "rw", + containerWorkdir: "/sandbox", + }, + }); + + const { createOpenShellFsBridge } = await import("./fs-bridge.js"); + const bridge = createOpenShellFsBridge({ sandbox, backend }); + + await expect( + bridge.readFile({ + filePath: "/sandbox/.openclaw/sandbox-skills/skills/demo/SKILL.md", + }), + ).resolves.toEqual(Buffer.from("# Demo\nmaterialized\n")); + await expect( + bridge.readFile({ + filePath: ".openclaw/sandbox-skills/skills/demo/SKILL.md", + }), + ).resolves.toEqual(Buffer.from("# Demo\nmaterialized\n")); + await expect( + bridge.writeFile({ + filePath: ".openclaw/sandbox-skills/skills/demo/SKILL.md", + data: "owned", + }), + ).rejects.toThrow(/read-only/); + await expect( + bridge.writeFile({ + filePath: shadowFile, + data: "owned", + }), + ).rejects.toThrow(/read-only/); + expect(await fs.readFile(shadowFile, "utf8")).toContain("workspace shadow"); + expect(backend["syncLocalPathToRemote"]).not.toHaveBeenCalled(); + }); + it("rejects reads of a symlinked leaf", async () => { const workspaceDir = await makeTempDir("openclaw-openshell-fs-"); const outsideDir = await makeTempDir("openclaw-openshell-outside-"); diff --git a/src/agents/agent-tools.read.ts b/src/agents/agent-tools.read.ts index 682427133096..a16d10c13dfc 100644 --- a/src/agents/agent-tools.read.ts +++ b/src/agents/agent-tools.read.ts @@ -36,7 +36,6 @@ import type { AgentToolResult } from "./runtime/index.js"; import { assertSandboxPath } from "./sandbox-paths.js"; import type { SandboxFsBridge } from "./sandbox/fs-bridge.js"; import { createEditTool, createReadTool, createWriteTool } from "./sessions/index.js"; -import { resolveReadPath } from "./sessions/tools/path-utils.js"; import { sanitizeToolResultImages } from "./tool-images.js"; export { @@ -515,7 +514,7 @@ function mapContainerPathToRoot(params: { return { filePath: params.filePath, matched: false }; } - const normalizedCandidate = candidate.replace(/\\/g, "/"); + const normalizedCandidate = path.posix.normalize(candidate.replace(/\\/g, "/")); if (normalizedCandidate === normalizedRoot) { return { filePath: path.resolve(params.root), matched: true }; } @@ -779,28 +778,34 @@ export function wrapToolWorkspaceRootGuardWithOptions( normalizedRecord[key] = filePath; } let guardedRoot = root; - const workspaceMapping = mapContainerPathToRoot({ - filePath, - root, - containerRoot: options?.containerWorkdir, - }); - let sandboxPath = workspaceMapping.filePath; - if (!workspaceMapping.matched) { - for (const mount of options?.additionalContainerMounts ?? []) { - const mountMapping = mapContainerPathToRoot({ - filePath, - root: mount.hostRoot, - containerRoot: mount.containerRoot, - }); - if (mountMapping.matched) { - guardedRoot = path.resolve(mount.hostRoot); - sandboxPath = mountMapping.filePath; - break; - } + let workspaceMapping: ReturnType | undefined; + let sandboxPath = filePath; + for (const mount of [...(options?.additionalContainerMounts ?? [])].toSorted( + (a, b) => b.containerRoot.length - a.containerRoot.length, + )) { + const mountMapping = mapContainerPathToRoot({ + filePath, + root: mount.hostRoot, + containerRoot: mount.containerRoot, + }); + if (mountMapping.matched) { + guardedRoot = path.resolve(mount.hostRoot); + sandboxPath = mountMapping.filePath; + break; } } + if (guardedRoot === root) { + workspaceMapping = mapContainerPathToRoot({ + filePath, + root, + containerRoot: options?.containerWorkdir, + }); + sandboxPath = workspaceMapping.filePath; + } const additionalRoots = - guardedRoot === root && !workspaceMapping.matched ? (options?.additionalRoots ?? []) : []; + guardedRoot === root && !workspaceMapping?.matched + ? (options?.additionalRoots ?? []) + : []; let sandboxResult: Awaited>; try { sandboxResult = await assertSandboxPathWithinAnyRoot({ @@ -905,12 +910,12 @@ export function createOpenClawReadTool( function createSandboxReadOperations(params: SandboxToolParams) { return { - resolvePath: (filePath: string, cwd: string) => { + resolvePath: (filePath: string) => { const normalizedMediaSource = normalizeMediaReferenceSource(filePath); - const resolvedPath = classifyMediaReferenceSource(normalizedMediaSource).isMediaStoreUrl - ? resolveMediaReferenceSandboxPath(normalizedMediaSource, "media/inbound").resolved - : filePath; - return resolveReadPath(resolvedPath, cwd); + if (classifyMediaReferenceSource(normalizedMediaSource).isMediaStoreUrl) { + return resolveMediaReferenceSandboxPath(normalizedMediaSource, "media/inbound").resolved; + } + return resolveContainerPathCandidate(filePath) ?? filePath; }, readFile: (absolutePath: string) => params.bridge.readFile({ filePath: absolutePath, cwd: params.root }), diff --git a/src/agents/agent-tools.read.workspace-root-guard.test.ts b/src/agents/agent-tools.read.workspace-root-guard.test.ts index f192b02be3c3..4a4569567ffe 100644 --- a/src/agents/agent-tools.read.workspace-root-guard.test.ts +++ b/src/agents/agent-tools.read.workspace-root-guard.test.ts @@ -40,7 +40,7 @@ async function loadModule() { let wrapToolWorkspaceRootGuardWithOptions: typeof import("./agent-tools.read.js").wrapToolWorkspaceRootGuardWithOptions; describe("wrapToolWorkspaceRootGuardWithOptions", () => { - const root = "/tmp/root"; + const root = path.resolve("/tmp/root"); const assertSandboxPathImpl: AssertSandboxPath = async ({ filePath }) => ({ resolved: filePath.startsWith("file://") || path.isAbsolute(filePath) @@ -216,7 +216,7 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => { it("maps additional container mounts to their own guarded host roots", async () => { const { tool } = createToolHarness(); - const agentRoot = "/tmp/agent-root"; + const agentRoot = path.resolve("/tmp/agent-root"); const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { additionalContainerMounts: [{ containerRoot: "/agent", hostRoot: agentRoot }], containerWorkdir: "/workspace", @@ -231,9 +231,26 @@ describe("wrapToolWorkspaceRootGuardWithOptions", () => { }); }); + it("normalizes container paths before matching additional mounts", async () => { + const { tool } = createToolHarness(); + const skillRoot = path.resolve("/tmp/skill-root"); + const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { + additionalContainerMounts: [{ containerRoot: "/workspace/skills", hostRoot: skillRoot }], + containerWorkdir: "/workspace", + }); + + await wrapped.execute("tc-skill-traverse", { path: "/workspace/skills/../README.md" }); + + expect(mocks.assertSandboxPath).toHaveBeenCalledWith({ + filePath: path.resolve(root, "README.md"), + cwd: root, + root, + }); + }); + it("maps file URLs under additional container mounts", async () => { const { tool } = createToolHarness(); - const agentRoot = "/tmp/agent-root"; + const agentRoot = path.resolve("/tmp/agent-root"); const wrapped = wrapToolWorkspaceRootGuardWithOptions(tool, root, { additionalContainerMounts: [{ containerRoot: "/agent", hostRoot: agentRoot }], containerWorkdir: "/workspace", diff --git a/src/agents/agent-tools.sandbox-mounted-paths.workspace-only.test.ts b/src/agents/agent-tools.sandbox-mounted-paths.workspace-only.test.ts index 010d1371a09d..9bfb8f802277 100644 --- a/src/agents/agent-tools.sandbox-mounted-paths.workspace-only.test.ts +++ b/src/agents/agent-tools.sandbox-mounted-paths.workspace-only.test.ts @@ -15,6 +15,7 @@ import { } from "./agent-tools.read.js"; import { createApplyPatchTool } from "./apply-patch.js"; import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./sandbox/constants.js"; +import { resolveReadOnlyWorkspaceSkillMounts } from "./sandbox/workspace-mounts.js"; import { expectReadWriteEditTools, expectReadWriteTools, @@ -75,12 +76,26 @@ function createSandboxFsTools(params: { sandbox: UnsafeMountedSandbox; workspace return tools.map((tool) => wrapToolWorkspaceRootGuardWithOptions(tool, params.sandbox.workspaceDir, { additionalContainerMounts: - tool.name === "read" && params.sandbox.workspaceAccess === "ro" + tool.name === "read" ? [ - { - containerRoot: SANDBOX_AGENT_WORKSPACE_MOUNT, - hostRoot: params.sandbox.agentWorkspaceDir, - }, + ...(params.sandbox.workspaceAccess === "ro" + ? [ + { + containerRoot: SANDBOX_AGENT_WORKSPACE_MOUNT, + hostRoot: params.sandbox.agentWorkspaceDir, + }, + ] + : []), + ...resolveReadOnlyWorkspaceSkillMounts({ + workspaceDir: params.sandbox.workspaceDir, + agentWorkspaceDir: params.sandbox.agentWorkspaceDir, + skillsWorkspaceDir: params.sandbox.skillsWorkspaceDir, + workdir: params.sandbox.containerWorkdir, + workspaceAccess: params.sandbox.workspaceAccess, + }).map((mount) => ({ + containerRoot: mount.containerPath, + hostRoot: mount.hostPath, + })), ] : undefined, containerWorkdir: params.sandbox.containerWorkdir, @@ -158,6 +173,55 @@ describe("tools.fs.workspaceOnly", () => { ); }); + it("allows read-only materialized sandbox skills for sandbox reads only", async () => { + await withUnsafeMountedSandboxHarness( + async ({ sandbox, skillsWorkspaceDir }) => { + expect(skillsWorkspaceDir).toBeTruthy(); + const skillDir = path.join(skillsWorkspaceDir!, "skills", "demo"); + const userOwnedShadowDir = path.join( + sandbox.workspaceDir, + ".openclaw", + "sandbox-skills", + "skills", + "demo", + ); + await fs.mkdir(skillDir, { recursive: true }); + await fs.mkdir(userOwnedShadowDir, { recursive: true }); + await fs.writeFile(path.join(skillDir, "SKILL.md"), "# Demo\nmaterialized\n", "utf8"); + await fs.writeFile( + path.join(userOwnedShadowDir, "SKILL.md"), + "# Demo\nuser-owned shadow\n", + "utf8", + ); + + const tools = createSandboxFsTools({ sandbox, workspaceOnly: true }); + const { readTool } = expectReadWriteEditTools(tools); + const containerSkillPath = "/workspace/.openclaw/sandbox-skills/skills/demo/SKILL.md"; + + const readResult = await readTool?.execute("t1", { path: containerSkillPath }); + expect(getTextContent(readResult)).toContain("materialized"); + expect(getTextContent(readResult)).not.toContain("user-owned shadow"); + const relativeReadResult = await readTool?.execute("t2", { + path: ".openclaw/sandbox-skills/skills/demo/SKILL.md", + }); + expect(getTextContent(relativeReadResult)).toContain("materialized"); + expect(getTextContent(relativeReadResult)).not.toContain("user-owned shadow"); + const fileUrlReadResult = await readTool?.execute("t3", { + path: "file:///workspace/.openclaw/sandbox-skills/skills/demo/SKILL.md", + }); + expect(getTextContent(fileUrlReadResult)).toContain("materialized"); + expect(getTextContent(fileUrlReadResult)).not.toContain("user-owned shadow"); + expect(await fs.readFile(path.join(skillDir, "SKILL.md"), "utf8")).toContain( + "materialized", + ); + expect(await fs.readFile(path.join(userOwnedShadowDir, "SKILL.md"), "utf8")).toContain( + "user-owned shadow", + ); + }, + { includeSkillsWorkspace: true, workspaceAccess: "rw" }, + ); + }); + it("enforces apply_patch workspace-only in sandbox mounts by default", async () => { await withUnsafeMountedSandboxHarness(async ({ agentRoot, sandbox }) => { const applyPatchTool = resolveApplyPatchTool({ diff --git a/src/agents/agent-tools.ts b/src/agents/agent-tools.ts index 95a11246786c..4b1492d4bdcc 100644 --- a/src/agents/agent-tools.ts +++ b/src/agents/agent-tools.ts @@ -77,6 +77,7 @@ import { resolveOpenClawPluginToolsForOptions } from "./openclaw-plugin-tools.js import { createOpenClawTools } from "./openclaw-tools.js"; import type { SandboxContext } from "./sandbox.js"; import { SANDBOX_AGENT_WORKSPACE_MOUNT } from "./sandbox/constants.js"; +import { resolveReadOnlyWorkspaceSkillMounts } from "./sandbox/workspace-mounts.js"; import { resolveSenderToolPolicy } from "./sender-tool-policy.js"; import { createCodingTools, createReadTool } from "./sessions/index.js"; import { @@ -122,22 +123,34 @@ type GuardContainerMount = { hostRoot: string; }; -function readOnlyAgentWorkspaceMount( +function readOnlySandboxReadMounts( sandbox: SandboxContext | null | undefined, ): GuardContainerMount[] | undefined { - if ( - !sandbox || - sandbox.workspaceAccess !== "ro" || - sandbox.agentWorkspaceDir === sandbox.workspaceDir - ) { + if (!sandbox) { return undefined; } - return [ - { + const mounts: GuardContainerMount[] = []; + if (sandbox.workspaceAccess === "ro" && sandbox.agentWorkspaceDir !== sandbox.workspaceDir) { + mounts.push({ containerRoot: SANDBOX_AGENT_WORKSPACE_MOUNT, hostRoot: sandbox.agentWorkspaceDir, - }, - ]; + }); + } + if (sandbox.workspaceAccess === "rw") { + mounts.push( + ...resolveReadOnlyWorkspaceSkillMounts({ + workspaceDir: sandbox.workspaceDir, + agentWorkspaceDir: sandbox.agentWorkspaceDir, + skillsWorkspaceDir: sandbox.skillsWorkspaceDir, + workdir: sandbox.containerWorkdir, + workspaceAccess: sandbox.workspaceAccess, + }).map((mount) => ({ + containerRoot: mount.containerPath, + hostRoot: mount.hostPath, + })), + ); + } + return mounts.length > 0 ? mounts : undefined; } function resolveSkillReadRoots(skillsSnapshot?: SkillSnapshot): string[] | undefined { @@ -743,7 +756,7 @@ export function createOpenClawCodingTools(options?: { base.push( workspaceOnly ? wrapToolWorkspaceRootGuardWithOptions(sandboxed, sandboxRoot, { - additionalContainerMounts: readOnlyAgentWorkspaceMount(sandbox), + additionalContainerMounts: readOnlySandboxReadMounts(sandbox), containerWorkdir: sandbox.containerWorkdir, }) : sandboxed, diff --git a/src/agents/embedded-agent-runner/compact.ts b/src/agents/embedded-agent-runner/compact.ts index 261801f97330..c8a27be38253 100644 --- a/src/agents/embedded-agent-runner/compact.ts +++ b/src/agents/embedded-agent-runner/compact.ts @@ -49,6 +49,10 @@ import { } from "../agent-hooks/compaction-safeguard-runtime.js"; import { createPreparedEmbeddedAgentSettingsManager } from "../agent-project-settings.js"; import { isDefaultAgentRuntimeId } from "../agent-runtime-id.js"; +import { + mapSandboxSkillEntriesForPrompt, + resolveSandboxSkillRuntimeInputs, +} from "./sandbox-skills.js"; import { resolveAgentDir, resolveRunModelFallbacksOverride, @@ -685,13 +689,24 @@ async function compactEmbeddedAgentSessionDirectOnce( let checkpointSnapshot: CapturedCompactionCheckpointSnapshot | null = null; let checkpointSnapshotRetained = false; try { - const skillsSnapshotForRun = - sandbox?.enabled && sandbox.workspaceAccess !== "rw" ? undefined : params.skillsSnapshot; + const { + skillsEligibility, + skillsPromptWorkspaceDir: effectiveSkillsPromptWorkspace, + skillsSnapshot: skillsSnapshotForRun, + skillsWorkspaceDir: effectiveSkillsWorkspace, + workspaceOnly: loadSkillsWorkspaceOnly, + } = resolveSandboxSkillRuntimeInputs({ + sandbox, + effectiveWorkspace, + skillsSnapshot: params.skillsSnapshot, + }); const { shouldLoadSkillEntries, skillEntries } = resolveEmbeddedRunSkillEntries({ - workspaceDir: effectiveWorkspace, + workspaceDir: effectiveSkillsWorkspace, config: params.config, agentId: effectiveSkillAgentId, + eligibility: skillsEligibility, skillsSnapshot: skillsSnapshotForRun, + workspaceOnly: loadSkillsWorkspaceOnly, }); restoreSkillEnv = skillsSnapshotForRun ? applySkillEnvOverridesFromSnapshot({ @@ -702,12 +717,18 @@ async function compactEmbeddedAgentSessionDirectOnce( skills: skillEntries ?? [], config: params.config, }); + const promptSkillEntries = mapSandboxSkillEntriesForPrompt({ + entries: shouldLoadSkillEntries ? skillEntries : undefined, + skillsWorkspaceDir: effectiveSkillsWorkspace, + skillsPromptWorkspaceDir: effectiveSkillsPromptWorkspace, + }); const skillsPrompt = resolveSkillsPromptForRun({ skillsSnapshot: skillsSnapshotForRun, - entries: shouldLoadSkillEntries ? skillEntries : undefined, + entries: promptSkillEntries, config: params.config, - workspaceDir: effectiveWorkspace, + workspaceDir: effectiveSkillsPromptWorkspace, agentId: effectiveSkillAgentId, + eligibility: skillsEligibility, }); const sessionLabel = params.sessionKey ?? params.sessionId; diff --git a/src/agents/embedded-agent-runner/run/attempt.ts b/src/agents/embedded-agent-runner/run/attempt.ts index 012b5d9d6253..757448b6ae81 100644 --- a/src/agents/embedded-agent-runner/run/attempt.ts +++ b/src/agents/embedded-agent-runner/run/attempt.ts @@ -176,6 +176,10 @@ import { import type { AgentMessage } from "../../runtime/index.js"; import { resolveSandboxContext } from "../../sandbox.js"; import { resolveSandboxRuntimeStatus } from "../../sandbox/runtime-status.js"; +import { + mapSandboxSkillEntriesForPrompt, + resolveSandboxSkillRuntimeInputs, +} from "../sandbox-skills.js"; import { repairSessionFileIfNeeded } from "../../session-file-repair.js"; import { guardSessionManager } from "../../session-tool-result-guard-wrapper.js"; import { sanitizeToolUseResultPairing } from "../../session-transcript-repair.js"; @@ -1034,13 +1038,24 @@ export async function runEmbeddedAttempt( } }; try { - const skillsSnapshotForRun = - sandbox?.enabled && sandbox.workspaceAccess !== "rw" ? undefined : params.skillsSnapshot; + const { + skillsEligibility, + skillsPromptWorkspaceDir: effectiveSkillsPromptWorkspace, + skillsSnapshot: skillsSnapshotForRun, + skillsWorkspaceDir: effectiveSkillsWorkspace, + workspaceOnly: loadSkillsWorkspaceOnly, + } = resolveSandboxSkillRuntimeInputs({ + sandbox, + effectiveWorkspace, + skillsSnapshot: params.skillsSnapshot, + }); const { shouldLoadSkillEntries, skillEntries } = resolveEmbeddedRunSkillEntries({ - workspaceDir: effectiveWorkspace, + workspaceDir: effectiveSkillsWorkspace, config: params.config, agentId: sessionAgentId, + eligibility: skillsEligibility, skillsSnapshot: skillsSnapshotForRun, + workspaceOnly: loadSkillsWorkspaceOnly, }); restoreSkillEnv = skillsSnapshotForRun ? applySkillEnvOverridesFromSnapshot({ @@ -1048,16 +1063,22 @@ export async function runEmbeddedAttempt( config: params.config, }) : applySkillEnvOverrides({ - skills: skillEntries ?? [], - config: params.config, - }); + skills: skillEntries ?? [], + config: params.config, + }); + const promptSkillEntries = mapSandboxSkillEntriesForPrompt({ + entries: shouldLoadSkillEntries ? skillEntries : undefined, + skillsWorkspaceDir: effectiveSkillsWorkspace, + skillsPromptWorkspaceDir: effectiveSkillsPromptWorkspace, + }); const skillsPrompt = resolveSkillsPromptForRun({ skillsSnapshot: skillsSnapshotForRun, - entries: shouldLoadSkillEntries ? skillEntries : undefined, + entries: promptSkillEntries, config: params.config, - workspaceDir: effectiveWorkspace, + workspaceDir: effectiveSkillsPromptWorkspace, agentId: sessionAgentId, + eligibility: skillsEligibility, }); prepStages.mark("skills"); diff --git a/src/agents/embedded-agent-runner/sandbox-skills.test.ts b/src/agents/embedded-agent-runner/sandbox-skills.test.ts new file mode 100644 index 000000000000..a169a80e433c --- /dev/null +++ b/src/agents/embedded-agent-runner/sandbox-skills.test.ts @@ -0,0 +1,216 @@ +// Sandbox skill input tests cover snapshot suppression and synced skill workspace selection. +import fs from "node:fs/promises"; +import os from "node:os"; +import path from "node:path"; +import { describe, expect, it } from "vitest"; +import { createSyntheticSourceInfo } from "../../skills/loading/skill-contract.js"; +import { resolveSkillsPromptForRun } from "../../skills/loading/workspace.js"; +import { resolveEmbeddedRunSkillEntries } from "../../skills/runtime/embedded-run-entries.js"; +import type { SkillSnapshot } from "../../skills/types.js"; +import { + mapSandboxSkillEntriesForPrompt, + resolveSandboxSkillRuntimeInputs, +} from "./sandbox-skills.js"; + +const hostSkillPath = "/usr/lib/node_modules/openclaw/skills/demo/SKILL.md"; +const hostSkillBaseDir = "/usr/lib/node_modules/openclaw/skills/demo"; +const snapshot: SkillSnapshot = { + prompt: + "/usr/lib/node_modules/openclaw/skills/demo/SKILL.md", + skills: [{ name: "demo" }], + resolvedSkills: [ + { + name: "demo", + description: "Demo skill", + filePath: hostSkillPath, + baseDir: hostSkillBaseDir, + source: "openclaw-bundled", + sourceInfo: createSyntheticSourceInfo(hostSkillPath, { + source: "openclaw-bundled", + baseDir: hostSkillBaseDir, + }), + disableModelInvocation: false, + }, + ], +}; + +describe("resolveSandboxSkillRuntimeInputs", () => { + it("keeps snapshots for non-sandboxed runs", () => { + expect( + resolveSandboxSkillRuntimeInputs({ + effectiveWorkspace: "/workspace", + skillsSnapshot: snapshot, + }), + ).toEqual({ + skillsSnapshot: snapshot, + skillsPromptWorkspaceDir: "/workspace", + skillsWorkspaceDir: "/workspace", + workspaceOnly: false, + }); + }); + + it("uses the materialized skills workspace and drops host-path snapshots for sandboxes", () => { + const skillsEligibility = { + remote: { + platforms: ["linux"], + hasBin: () => true, + hasAnyBin: () => true, + note: "sandbox", + }, + }; + + expect( + resolveSandboxSkillRuntimeInputs({ + sandbox: { + enabled: true, + containerWorkdir: "/workspace", + skillsEligibility, + skillsWorkspaceDir: "/state/sandbox-skills", + workspaceAccess: "rw", + }, + effectiveWorkspace: "/workspace", + skillsSnapshot: snapshot, + }), + ).toEqual({ + skillsEligibility, + skillsSnapshot: undefined, + skillsPromptWorkspaceDir: "/workspace/.openclaw/sandbox-skills", + skillsWorkspaceDir: "/state/sandbox-skills", + workspaceOnly: true, + }); + }); + + it("falls back to the effective workspace for older sandbox contexts", () => { + expect( + resolveSandboxSkillRuntimeInputs({ + sandbox: { enabled: true }, + effectiveWorkspace: "/workspace", + skillsSnapshot: snapshot, + }), + ).toEqual({ + skillsSnapshot: undefined, + skillsPromptWorkspaceDir: "/workspace", + skillsWorkspaceDir: "/workspace", + workspaceOnly: true, + }); + }); + + it("rebuilds sandbox prompts from materialized skill paths", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sandbox-skills-")); + try { + const effectiveWorkspace = path.join(root, "workspace"); + const materializedWorkspace = path.join(root, "state", "sandbox-skills"); + const skillDir = path.join(materializedWorkspace, "skills", "demo"); + await fs.mkdir(skillDir, { recursive: true }); + await fs.writeFile( + path.join(skillDir, "SKILL.md"), + [ + "---", + "name: demo", + "description: Demo skill", + 'openclaw: {"requires":{"anyBins":["sandboxbin"]}}', + "---", + "# Demo", + "", + ].join("\n"), + "utf8", + ); + const skillsEligibility = { + remote: { + platforms: ["linux"], + hasBin: () => false, + hasAnyBin: (bins: string[]) => bins.includes("sandboxbin"), + note: "sandbox", + }, + }; + + const { + skillsEligibility: skillsEligibilityForRun, + skillsPromptWorkspaceDir, + skillsSnapshot: skillsSnapshotForRun, + skillsWorkspaceDir, + workspaceOnly, + } = resolveSandboxSkillRuntimeInputs({ + sandbox: { + enabled: true, + containerWorkdir: "/workspace", + skillsEligibility, + skillsWorkspaceDir: materializedWorkspace, + workspaceAccess: "rw", + }, + effectiveWorkspace, + skillsSnapshot: snapshot, + }); + const { shouldLoadSkillEntries, skillEntries } = resolveEmbeddedRunSkillEntries({ + workspaceDir: skillsWorkspaceDir, + eligibility: skillsEligibilityForRun, + skillsSnapshot: skillsSnapshotForRun, + workspaceOnly, + }); + const promptSkillEntries = mapSandboxSkillEntriesForPrompt({ + entries: shouldLoadSkillEntries ? skillEntries : undefined, + skillsWorkspaceDir, + skillsPromptWorkspaceDir, + }); + const prompt = resolveSkillsPromptForRun({ + skillsSnapshot: skillsSnapshotForRun, + entries: promptSkillEntries, + workspaceDir: skillsPromptWorkspaceDir, + eligibility: skillsEligibilityForRun, + }); + + expect(prompt).toContain("/workspace/.openclaw/sandbox-skills/skills/demo/SKILL.md"); + expect(prompt.replaceAll("\\", "/")).not.toContain(materializedWorkspace.replaceAll("\\", "/")); + expect(prompt).not.toContain(hostSkillPath); + expect(prompt).not.toContain("plugin-skills"); + expect(prompt.replaceAll("\\", "/")).not.toContain("/skills/canvas/SKILL.md"); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); + + it("preserves remote eligibility when rebuilding sandbox prompts", async () => { + const root = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sandbox-skills-")); + try { + const skillDir = path.join(root, "skills", "macskill"); + await fs.mkdir(skillDir, { recursive: true }); + await fs.writeFile( + path.join(skillDir, "SKILL.md"), + [ + "---", + "name: macskill", + "description: Mac-only remote skill", + 'openclaw: {"os":["darwin"]}', + "---", + "# Mac Skill", + "", + ].join("\n"), + "utf8", + ); + const skillsEligibility = { + remote: { + platforms: ["darwin"], + hasBin: () => false, + hasAnyBin: () => false, + note: "remote mac available", + }, + }; + + const { shouldLoadSkillEntries, skillEntries } = resolveEmbeddedRunSkillEntries({ + workspaceDir: root, + eligibility: skillsEligibility, + workspaceOnly: true, + }); + const prompt = resolveSkillsPromptForRun({ + entries: shouldLoadSkillEntries ? skillEntries : undefined, + workspaceDir: root, + eligibility: skillsEligibility, + }); + + expect(prompt).toContain("remote mac available"); + expect(prompt).toContain("macskill"); + } finally { + await fs.rm(root, { recursive: true, force: true }); + } + }); +}); diff --git a/src/agents/embedded-agent-runner/sandbox-skills.ts b/src/agents/embedded-agent-runner/sandbox-skills.ts new file mode 100644 index 000000000000..77d89080148c --- /dev/null +++ b/src/agents/embedded-agent-runner/sandbox-skills.ts @@ -0,0 +1,148 @@ +/** + * Sandbox skill runtime input selection. + * + * Sandboxed runs must build prompt-facing skill entries from readable in-sandbox + * copies instead of reusing host-path snapshots. + */ +import path from "node:path"; +import type { SkillEligibilityContext, SkillSnapshot } from "../../skills/types.js"; +import type { SkillEntry } from "../../skills/types.js"; +import type { SandboxContext } from "../sandbox/types.js"; + +const MATERIALIZED_SKILLS_WORKSPACE_CONTAINER_PARTS = [".openclaw", "sandbox-skills"] as const; +type SandboxSkillRuntimeContext = Pick & + Partial< + Pick< + SandboxContext, + "skillsEligibility" | "skillsWorkspaceDir" | "containerWorkdir" | "workspaceAccess" + > + >; + +function containerJoin(root: string, ...parts: string[]): string { + const normalizedRoot = root.replace(/\\/g, "/").replace(/\/+$/, "") || "/"; + const suffix = parts + .map((part) => part.replace(/^\/+|\/+$/g, "")) + .filter(Boolean) + .join("/"); + return suffix ? `${normalizedRoot}/${suffix}` : normalizedRoot; +} + +function pathEscapesRoot(relativePath: string): boolean { + return ( + relativePath === ".." || + relativePath.startsWith(`..${path.sep}`) || + path.isAbsolute(relativePath) + ); +} + +function mapPathFromWorkspaceToContainer(params: { + filePath: string | undefined; + sourceWorkspaceDir: string; + targetWorkspaceDir: string; +}): string | undefined { + if (!params.filePath || !path.isAbsolute(params.filePath)) { + return params.filePath; + } + const relativePath = path.relative( + path.resolve(params.sourceWorkspaceDir), + path.resolve(params.filePath), + ); + if (pathEscapesRoot(relativePath)) { + return params.filePath; + } + if (!relativePath) { + return params.targetWorkspaceDir.replace(/\\/g, "/"); + } + return containerJoin( + params.targetWorkspaceDir, + ...relativePath.split(path.sep).filter(Boolean), + ); +} + +export function mapSandboxSkillEntriesForPrompt(params: { + entries?: SkillEntry[]; + skillsWorkspaceDir: string; + skillsPromptWorkspaceDir: string; +}): SkillEntry[] | undefined { + if (!params.entries || params.skillsWorkspaceDir === params.skillsPromptWorkspaceDir) { + return params.entries; + } + return params.entries.map((entry) => { + const filePath = + mapPathFromWorkspaceToContainer({ + filePath: entry.skill.filePath, + sourceWorkspaceDir: params.skillsWorkspaceDir, + targetWorkspaceDir: params.skillsPromptWorkspaceDir, + }) ?? entry.skill.filePath; + const baseDir = + mapPathFromWorkspaceToContainer({ + filePath: entry.skill.baseDir, + sourceWorkspaceDir: params.skillsWorkspaceDir, + targetWorkspaceDir: params.skillsPromptWorkspaceDir, + }) ?? entry.skill.baseDir; + const sourceInfoPath = + mapPathFromWorkspaceToContainer({ + filePath: entry.skill.sourceInfo.path, + sourceWorkspaceDir: params.skillsWorkspaceDir, + targetWorkspaceDir: params.skillsPromptWorkspaceDir, + }) ?? entry.skill.sourceInfo.path; + const sourceInfoBaseDir = mapPathFromWorkspaceToContainer({ + filePath: entry.skill.sourceInfo.baseDir, + sourceWorkspaceDir: params.skillsWorkspaceDir, + targetWorkspaceDir: params.skillsPromptWorkspaceDir, + }); + return { + ...entry, + skill: { + ...entry.skill, + filePath, + baseDir, + sourceInfo: { + ...entry.skill.sourceInfo, + path: sourceInfoPath, + ...(sourceInfoBaseDir === undefined ? {} : { baseDir: sourceInfoBaseDir }), + }, + }, + }; + }); +} + +export function resolveSandboxSkillRuntimeInputs(params: { + sandbox?: SandboxSkillRuntimeContext | null; + effectiveWorkspace: string; + skillsSnapshot?: SkillSnapshot; +}): { + skillsEligibility?: SkillEligibilityContext; + skillsPromptWorkspaceDir: string; + skillsSnapshot?: SkillSnapshot; + skillsWorkspaceDir: string; + workspaceOnly: boolean; +} { + if (params.sandbox?.enabled === true) { + const skillsWorkspaceDir = params.sandbox.skillsWorkspaceDir ?? params.effectiveWorkspace; + const skillsPromptWorkspaceDir = + params.sandbox.workspaceAccess === "rw" && + params.sandbox.skillsWorkspaceDir && + params.sandbox.containerWorkdir + ? containerJoin( + params.sandbox.containerWorkdir, + ...MATERIALIZED_SKILLS_WORKSPACE_CONTAINER_PARTS, + ) + : (params.sandbox.containerWorkdir ?? skillsWorkspaceDir); + return { + ...(params.sandbox.skillsEligibility + ? { skillsEligibility: params.sandbox.skillsEligibility } + : {}), + skillsPromptWorkspaceDir, + skillsSnapshot: undefined, + skillsWorkspaceDir, + workspaceOnly: true, + }; + } + return { + skillsPromptWorkspaceDir: params.effectiveWorkspace, + skillsSnapshot: params.skillsSnapshot, + skillsWorkspaceDir: params.effectiveWorkspace, + workspaceOnly: false, + }; +} diff --git a/src/agents/sandbox.resolveSandboxContext.test.ts b/src/agents/sandbox.resolveSandboxContext.test.ts index 8f54f9884d55..b065a3fd056e 100644 --- a/src/agents/sandbox.resolveSandboxContext.test.ts +++ b/src/agents/sandbox.resolveSandboxContext.test.ts @@ -347,4 +347,127 @@ describe("resolveSandboxContext", () => { expect(syncOptions?.agentId).toBe("main"); expect(syncOptions?.eligibility).toEqual({ remote: { note: "test-remote" } }); }, 15_000); + + it("materializes skills into a hidden read-only workspace for writable sandboxes", async () => { + syncSkillsToWorkspaceMock.mockClear(); + const workspaceDir = await createSandboxFixtureDir("workspace"); + const userOwnedSandboxSkillsDir = path.join( + workspaceDir, + ".openclaw", + "sandbox-skills", + "skills", + "user-owned", + ); + await fs.mkdir(userOwnedSandboxSkillsDir, { recursive: true }); + await fs.writeFile(path.join(userOwnedSandboxSkillsDir, "SKILL.md"), "# User owned\n"); + + const cfg: OpenClawConfig = { + agents: { + defaults: { + sandbox: { + mode: "all", + scope: "session", + workspaceAccess: "rw", + workspaceRoot: path.join(workspaceDir, ".openclaw", "sandboxes"), + }, + }, + }, + }; + + const result = await ensureSandboxWorkspaceForSession({ + config: cfg, + sessionKey: "agent:main:main", + workspaceDir, + }); + + expect(result?.workspaceDir).toBe(workspaceDir); + const syncCalls = syncSkillsToWorkspaceMock.mock.calls as unknown as Array< + [ + { + sourceWorkspaceDir?: string; + targetWorkspaceDir?: string; + config?: OpenClawConfig; + agentId?: string; + eligibility?: unknown; + }, + ] + >; + const [syncOptions] = syncCalls[0] ?? []; + expect(syncOptions?.sourceWorkspaceDir).toBe(workspaceDir); + expect(syncOptions?.targetWorkspaceDir).toContain( + path.join(".openclaw", "sandbox", "skills-workspaces"), + ); + expect(syncOptions?.targetWorkspaceDir).toMatch( + /[\\/]agent-main-main-[a-f0-9]{8}[\\/]\.openclaw[\\/]sandbox-skills$/, + ); + expect(syncOptions?.targetWorkspaceDir).not.toBe( + path.join(workspaceDir, ".openclaw", "sandbox-skills"), + ); + expect(syncOptions?.targetWorkspaceDir?.startsWith(path.join(workspaceDir, ".openclaw"))).toBe( + false, + ); + expect(syncOptions?.config).toBe(cfg); + expect(syncOptions?.agentId).toBe("main"); + expect(syncOptions?.eligibility).toEqual({ remote: { note: "test-remote" } }); + await expect( + fs.readFile(path.join(userOwnedSandboxSkillsDir, "SKILL.md"), "utf8"), + ).resolves.toBe("# User owned\n"); + }, 15_000); + + it("materializes skills for shared writable sandboxes even when roots match", async () => { + syncSkillsToWorkspaceMock.mockClear(); + const workspaceDir = await createSandboxFixtureDir("shared-workspace"); + const userOwnedSandboxSkillsDir = path.join( + workspaceDir, + ".openclaw", + "sandbox-skills", + "skills", + "user-owned", + ); + await fs.mkdir(userOwnedSandboxSkillsDir, { recursive: true }); + await fs.writeFile(path.join(userOwnedSandboxSkillsDir, "SKILL.md"), "# User owned\n"); + + const cfg: OpenClawConfig = { + agents: { + defaults: { + sandbox: { + mode: "all", + scope: "shared", + workspaceAccess: "rw", + workspaceRoot: workspaceDir, + }, + }, + }, + }; + + const result = await ensureSandboxWorkspaceForSession({ + config: cfg, + sessionKey: "agent:main:main", + workspaceDir, + }); + + expect(result?.workspaceDir).toBe(workspaceDir); + const syncCalls = syncSkillsToWorkspaceMock.mock.calls as unknown as Array< + [ + { + sourceWorkspaceDir?: string; + targetWorkspaceDir?: string; + }, + ] + >; + const [syncOptions] = syncCalls[0] ?? []; + expect(syncOptions?.sourceWorkspaceDir).toBe(workspaceDir); + expect(syncOptions?.targetWorkspaceDir).toContain( + path.join(".openclaw", "sandbox", "skills-workspaces"), + ); + expect(syncOptions?.targetWorkspaceDir).toMatch( + /[\\/]shared-[a-f0-9]{8}[\\/]\.openclaw[\\/]sandbox-skills$/, + ); + expect(syncOptions?.targetWorkspaceDir).not.toBe( + path.join(workspaceDir, ".openclaw", "sandbox-skills"), + ); + await expect( + fs.readFile(path.join(userOwnedSandboxSkillsDir, "SKILL.md"), "utf8"), + ).resolves.toBe("# User owned\n"); + }, 15_000); }); diff --git a/src/agents/sandbox/backend-handle.types.ts b/src/agents/sandbox/backend-handle.types.ts index f56761bc91f9..c59600fb40f5 100644 --- a/src/agents/sandbox/backend-handle.types.ts +++ b/src/agents/sandbox/backend-handle.types.ts @@ -38,6 +38,7 @@ export type SandboxBackendCommandResult = { export type SandboxFsBridgeContext = { workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; workspaceAccess: "none" | "ro" | "rw"; containerName: string; containerWorkdir: string; diff --git a/src/agents/sandbox/backend.types.ts b/src/agents/sandbox/backend.types.ts index 50665c22329f..7f4e57df5cce 100644 --- a/src/agents/sandbox/backend.types.ts +++ b/src/agents/sandbox/backend.types.ts @@ -35,6 +35,7 @@ export type CreateSandboxBackendParams = { scopeKey: string; workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; cfg: SandboxConfig; }; diff --git a/src/agents/sandbox/browser.ts b/src/agents/sandbox/browser.ts index ad45b208d29e..7a682e37ffef 100644 --- a/src/agents/sandbox/browser.ts +++ b/src/agents/sandbox/browser.ts @@ -218,6 +218,7 @@ export async function ensureSandboxBrowser(params: { scopeKey: string; workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; cfg: SandboxConfig; evaluateEnabled?: boolean; bridgeAuth?: { token?: string; password?: string }; @@ -243,6 +244,7 @@ export async function ensureSandboxBrowser(params: { const readOnlyWorkspaceSkillMounts = resolveReadOnlyWorkspaceSkillMounts({ workspaceDir: params.workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, workdir: params.cfg.docker.workdir, workspaceAccess: params.cfg.workspaceAccess, }); @@ -354,6 +356,7 @@ export async function ensureSandboxBrowser(params: { args, workspaceDir: params.workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, workdir: params.cfg.docker.workdir, workspaceAccess: params.cfg.workspaceAccess, readOnlyWorkspaceSkillMounts, diff --git a/src/agents/sandbox/context.ts b/src/agents/sandbox/context.ts index 6fc23e7834d6..678ab2e68f4d 100644 --- a/src/agents/sandbox/context.ts +++ b/src/agents/sandbox/context.ts @@ -4,6 +4,7 @@ * Prepares workspace layout, backend handle, filesystem bridge, browser bridge, and registry state for one run. */ import fs from "node:fs/promises"; +import path from "node:path"; import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { ensureBrowserControlAuth, @@ -14,18 +15,59 @@ import { resolveBrowserConfig, } from "../../plugin-sdk/browser-profiles.js"; import { defaultRuntime } from "../../runtime.js"; +import type { SkillEligibilityContext } from "../../skills/types.js"; import { resolveUserPath } from "../../utils.js"; import { DEFAULT_AGENT_WORKSPACE_DIR } from "../workspace.js"; import { requireSandboxBackendFactory } from "./backend.js"; import { ensureSandboxBrowser } from "./browser.js"; import { resolveSandboxConfigForAgent } from "./config.js"; +import { SANDBOX_STATE_DIR } from "./constants.js"; import { createSandboxFsBridge } from "./fs-bridge.js"; import { updateRegistry } from "./registry.js"; import { resolveSandboxRuntimeStatus } from "./runtime-status.js"; import { resolveSandboxScopeKey, resolveSandboxWorkspaceDir } from "./shared.js"; import type { SandboxContext, SandboxDockerConfig, SandboxWorkspaceInfo } from "./types.js"; +import { resolveMaterializedSandboxSkillsWorkspaceDir } from "./workspace-mounts.js"; import { ensureSandboxWorkspace } from "./workspace.js"; +async function syncSandboxSkillsToWorkspace(params: { + sourceWorkspaceDir: string; + targetWorkspaceDir: string; + config?: OpenClawConfig; + agentId: string; + rawSessionKey: string; +}): Promise { + try { + const [{ syncSkillsToWorkspace }, { getRemoteSkillEligibility }, { canExecRequestNode }] = + await Promise.all([ + import("../../skills/loading/workspace.js"), + import("../../skills/runtime/remote.js"), + import("../exec-defaults.js"), + ]); + const eligibility: SkillEligibilityContext = { + remote: getRemoteSkillEligibility({ + advertiseExecNode: canExecRequestNode({ + cfg: params.config, + sessionKey: params.rawSessionKey, + agentId: params.agentId, + }), + }), + }; + await syncSkillsToWorkspace({ + sourceWorkspaceDir: params.sourceWorkspaceDir, + targetWorkspaceDir: params.targetWorkspaceDir, + config: params.config, + agentId: params.agentId, + eligibility, + }); + return eligibility; + } catch (error) { + const message = error instanceof Error ? error.message : JSON.stringify(error); + defaultRuntime.error?.(`Sandbox skill sync failed: ${message}`); + return undefined; + } +} + async function ensureSandboxWorkspaceLayout(params: { cfg: ReturnType; agentId: string; @@ -36,6 +78,8 @@ async function ensureSandboxWorkspaceLayout(params: { agentWorkspaceDir: string; scopeKey: string; sandboxWorkspaceDir: string; + skillsWorkspaceDir: string; + skillsEligibility?: SkillEligibilityContext; workspaceDir: string; }> { const { cfg, rawSessionKey } = params; @@ -48,47 +92,49 @@ async function ensureSandboxWorkspaceLayout(params: { const sandboxWorkspaceDir = cfg.scope === "shared" ? workspaceRoot : resolveSandboxWorkspaceDir(workspaceRoot, scopeKey); const workspaceDir = cfg.workspaceAccess === "rw" ? agentWorkspaceDir : sandboxWorkspaceDir; + const materializedSkillsRoot = resolveSandboxWorkspaceDir( + path.join(SANDBOX_STATE_DIR, "skills-workspaces"), + scopeKey, + ); + const skillsWorkspaceDir = + cfg.workspaceAccess === "rw" + ? resolveMaterializedSandboxSkillsWorkspaceDir(materializedSkillsRoot) + : sandboxWorkspaceDir; - if (workspaceDir === sandboxWorkspaceDir) { + let skillsEligibility: SkillEligibilityContext | undefined; + if (cfg.workspaceAccess !== "rw") { await ensureSandboxWorkspace( sandboxWorkspaceDir, agentWorkspaceDir, params.config?.agents?.defaults?.skipBootstrap, params.config?.agents?.defaults?.skipOptionalBootstrapFiles, ); - if (cfg.workspaceAccess !== "rw") { - try { - const [{ syncSkillsToWorkspace }, { getRemoteSkillEligibility }, { canExecRequestNode }] = - await Promise.all([ - import("../../skills/loading/workspace.js"), - import("../../skills/runtime/remote.js"), - import("../exec-defaults.js"), - ]); - await syncSkillsToWorkspace({ - sourceWorkspaceDir: agentWorkspaceDir, - targetWorkspaceDir: sandboxWorkspaceDir, - config: params.config, - agentId: params.agentId, - eligibility: { - remote: getRemoteSkillEligibility({ - advertiseExecNode: canExecRequestNode({ - cfg: params.config, - sessionKey: rawSessionKey, - agentId: params.agentId, - }), - }), - }, - }); - } catch (error) { - const message = error instanceof Error ? error.message : JSON.stringify(error); - defaultRuntime.error?.(`Sandbox skill sync failed: ${message}`); - } - } + skillsEligibility = await syncSandboxSkillsToWorkspace({ + sourceWorkspaceDir: agentWorkspaceDir, + targetWorkspaceDir: sandboxWorkspaceDir, + config: params.config, + agentId: params.agentId, + rawSessionKey, + }); } else { await fs.mkdir(workspaceDir, { recursive: true }); + skillsEligibility = await syncSandboxSkillsToWorkspace({ + sourceWorkspaceDir: agentWorkspaceDir, + targetWorkspaceDir: skillsWorkspaceDir, + config: params.config, + agentId: params.agentId, + rawSessionKey, + }); } - return { agentWorkspaceDir, scopeKey, sandboxWorkspaceDir, workspaceDir }; + return { + agentWorkspaceDir, + scopeKey, + sandboxWorkspaceDir, + skillsWorkspaceDir, + ...(skillsEligibility ? { skillsEligibility } : {}), + workspaceDir, + }; } export async function resolveSandboxDockerUser(params: { @@ -147,13 +193,14 @@ export async function resolveSandboxContext(params: { await (await import("./prune.js")).maybePruneSandboxes(cfg); } - const { agentWorkspaceDir, scopeKey, workspaceDir } = await ensureSandboxWorkspaceLayout({ - cfg, - agentId: runtime.agentId, - rawSessionKey, - config: params.config, - workspaceDir: params.workspaceDir, - }); + const { agentWorkspaceDir, scopeKey, skillsEligibility, skillsWorkspaceDir, workspaceDir } = + await ensureSandboxWorkspaceLayout({ + cfg, + agentId: runtime.agentId, + rawSessionKey, + config: params.config, + workspaceDir: params.workspaceDir, + }); const docker = await resolveSandboxDockerUser({ docker: cfg.docker, @@ -167,6 +214,7 @@ export async function resolveSandboxContext(params: { scopeKey, workspaceDir, agentWorkspaceDir, + skillsWorkspaceDir, cfg: resolvedCfg, }); await updateRegistry({ @@ -214,6 +262,7 @@ export async function resolveSandboxContext(params: { scopeKey, workspaceDir, agentWorkspaceDir, + skillsWorkspaceDir, cfg: resolvedCfg, evaluateEnabled, bridgeAuth, @@ -227,6 +276,8 @@ export async function resolveSandboxContext(params: { sessionKey: rawSessionKey, workspaceDir, agentWorkspaceDir, + skillsWorkspaceDir, + ...(skillsEligibility ? { skillsEligibility } : {}), workspaceAccess: resolvedCfg.workspaceAccess, runtimeId: backend.runtimeId, runtimeLabel: backend.runtimeLabel, diff --git a/src/agents/sandbox/docker-backend.ts b/src/agents/sandbox/docker-backend.ts index d15c82550835..4aa4518de1a8 100644 --- a/src/agents/sandbox/docker-backend.ts +++ b/src/agents/sandbox/docker-backend.ts @@ -39,6 +39,7 @@ export async function createDockerSandboxBackend( sessionKey: params.sessionKey, workspaceDir: params.workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, cfg: params.cfg, }); return createDockerSandboxBackendHandle({ diff --git a/src/agents/sandbox/docker.ts b/src/agents/sandbox/docker.ts index 06d15822745e..0038b8b8a415 100644 --- a/src/agents/sandbox/docker.ts +++ b/src/agents/sandbox/docker.ts @@ -564,6 +564,7 @@ async function createSandboxContainer(params: { workspaceDir: string; workspaceAccess: SandboxWorkspaceAccess; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; scopeKey: string; configHash?: string; readOnlyWorkspaceSkillMounts: readonly ReadOnlyWorkspaceSkillMount[]; @@ -584,6 +585,7 @@ async function createSandboxContainer(params: { args, workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, workdir: cfg.workdir, workspaceAccess: params.workspaceAccess, readOnlyWorkspaceSkillMounts: params.readOnlyWorkspaceSkillMounts, @@ -623,6 +625,7 @@ export async function ensureSandboxContainer(params: { sessionKey: string; workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; cfg: SandboxConfig; }) { const scopeKey = resolveSandboxScopeKey(params.cfg.scope, params.sessionKey); @@ -632,6 +635,7 @@ export async function ensureSandboxContainer(params: { const readOnlyWorkspaceSkillMounts = resolveReadOnlyWorkspaceSkillMounts({ workspaceDir: params.workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, workdir: params.cfg.docker.workdir, workspaceAccess: params.cfg.workspaceAccess, }); @@ -689,6 +693,7 @@ export async function ensureSandboxContainer(params: { workspaceDir: params.workspaceDir, workspaceAccess: params.cfg.workspaceAccess, agentWorkspaceDir: params.agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, scopeKey, configHash: expectedHash, readOnlyWorkspaceSkillMounts, diff --git a/src/agents/sandbox/fs-paths.ts b/src/agents/sandbox/fs-paths.ts index 5e92d8f19d2b..a4bafd91ffea 100644 --- a/src/agents/sandbox/fs-paths.ts +++ b/src/agents/sandbox/fs-paths.ts @@ -91,6 +91,7 @@ export function buildSandboxFsMounts(sandbox: SandboxFsBridgeContext): SandboxFs for (const mount of resolveReadOnlyWorkspaceSkillMounts({ workspaceDir: sandbox.workspaceDir, agentWorkspaceDir: sandbox.agentWorkspaceDir, + skillsWorkspaceDir: sandbox.skillsWorkspaceDir, workdir: sandbox.containerWorkdir, workspaceAccess: sandbox.workspaceAccess, })) { @@ -183,23 +184,31 @@ export function resolveSandboxFsPathWithMounts(params: { if (path.posix.isAbsolute(inputPosix)) { const containerMount = findMountByContainerPath(mountsByContainer, inputPosix); if (containerMount) { - const rel = path.posix.relative(containerMount.containerRoot, inputPosix); - const hostPath = rel - ? path.resolve(containerMount.hostRoot, ...toHostSegments(rel)) - : containerMount.hostRoot; - return { - hostPath, - containerPath: rel - ? path.posix.join(containerMount.containerRoot, rel) - : containerMount.containerRoot, - relativePath: toDisplayRelative({ - containerPath: rel - ? path.posix.join(containerMount.containerRoot, rel) - : containerMount.containerRoot, - defaultContainerRoot: params.defaultContainerRoot, - }), - writable: containerMount.writable, - }; + return resolveMountedContainerPath({ + mount: containerMount, + containerPath: inputPosix, + defaultContainerRoot: params.defaultContainerRoot, + }); + } + } + + if (!path.posix.isAbsolute(inputPosix)) { + const protectedContainerCandidate = resolveRelativeContainerCandidate({ + inputPosix, + cwd: params.cwd, + defaultContainerRoot: params.defaultContainerRoot, + mountsByHost, + }); + const protectedContainerMount = findMountByContainerPath( + mountsByContainer, + protectedContainerCandidate, + ); + if (protectedContainerMount?.source === "protectedSkill") { + return resolveMountedContainerPath({ + mount: protectedContainerMount, + containerPath: protectedContainerCandidate, + defaultContainerRoot: params.defaultContainerRoot, + }); } } @@ -239,6 +248,51 @@ export function resolveSandboxFsPathWithMounts(params: { throw new Error(escapeMessage); } +function resolveMountedContainerPath(params: { + mount: SandboxFsMount; + containerPath: string; + defaultContainerRoot: string; +}): SandboxResolvedFsPath { + const rel = path.posix.relative(params.mount.containerRoot, params.containerPath); + const hostPath = rel + ? path.resolve(params.mount.hostRoot, ...toHostSegments(rel)) + : params.mount.hostRoot; + const containerPath = rel + ? path.posix.join(params.mount.containerRoot, rel) + : params.mount.containerRoot; + return { + hostPath, + containerPath, + relativePath: toDisplayRelative({ + containerPath, + defaultContainerRoot: params.defaultContainerRoot, + }), + writable: params.mount.writable, + }; +} + +function resolveRelativeContainerCandidate(params: { + inputPosix: string; + cwd: string; + defaultContainerRoot: string; + mountsByHost: SandboxFsMount[]; +}): string { + const cwdMount = findMountByHostPath(params.mountsByHost, path.resolve(params.cwd)); + if (cwdMount) { + const relHost = path.relative(cwdMount.hostRoot, path.resolve(params.cwd)); + const relPosix = relHost ? relHost.split(path.sep).join(path.posix.sep) : ""; + const containerCwd = relPosix + ? path.posix.join(cwdMount.containerRoot, relPosix) + : cwdMount.containerRoot; + return normalizeContainerPath(path.posix.resolve(containerCwd, params.inputPosix)); + } + const cwdPosix = normalizePosixInput(params.cwd); + if (path.posix.isAbsolute(cwdPosix)) { + return normalizeContainerPath(path.posix.resolve(cwdPosix, params.inputPosix)); + } + return normalizeContainerPath(path.posix.resolve(params.defaultContainerRoot, params.inputPosix)); +} + function formatSandboxRootEscapeMessage(params: { input: string; defaultWorkspaceRoot: string; diff --git a/src/agents/sandbox/remote-fs-bridge.ts b/src/agents/sandbox/remote-fs-bridge.ts index b06d53c73b31..7edc2c8d526d 100644 --- a/src/agents/sandbox/remote-fs-bridge.ts +++ b/src/agents/sandbox/remote-fs-bridge.ts @@ -20,7 +20,10 @@ import { normalizeContainerPath as normalizeSandboxContainerPath, relativePathEscapesContainerRoot, } from "./path-utils.js"; -import { isExistingWorkspaceSkillMountSource } from "./workspace-mounts.js"; +import { + isExistingWorkspaceSkillMountSource, + resolveMaterializedSandboxSkillsWorkspaceDir, +} from "./workspace-mounts.js"; type RemoteMountSource = "workspace" | "agent" | "protectedSkill"; @@ -295,6 +298,7 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge { mounts.push( ...buildRemoteProtectedSkillMounts({ localRoot: agentRoot, + skillsWorkspaceDir: this.sandbox.skillsWorkspaceDir, workspaceContainerRoot, agentContainerRoot, includeAgentMount: @@ -454,11 +458,13 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge { const roots = [ path.posix.join(workspaceContainerRoot, "skills"), path.posix.join(workspaceContainerRoot, ".agents", "skills"), + path.posix.join(workspaceContainerRoot, ".openclaw", "sandbox-skills", "skills"), ]; if (path.resolve(this.sandbox.agentWorkspaceDir) !== path.resolve(this.sandbox.workspaceDir)) { roots.push( path.posix.join(agentContainerRoot, "skills"), path.posix.join(agentContainerRoot, ".agents", "skills"), + path.posix.join(agentContainerRoot, ".openclaw", "sandbox-skills", "skills"), ); } return roots; @@ -629,22 +635,40 @@ class RemoteShellSandboxFsBridge implements SandboxFsBridge { function buildRemoteProtectedSkillMounts(params: { localRoot: string; + skillsWorkspaceDir?: string; workspaceContainerRoot: string; agentContainerRoot: string; includeAgentMount: boolean; }): MountInfo[] { - const mounts: MountInfo[] = [ + const materializedSkillsWorkspaceDir = path.resolve( + params.skillsWorkspaceDir ?? resolveMaterializedSandboxSkillsWorkspaceDir(params.localRoot), + ); + const mounts: Array = [ { localRoot: path.join(params.localRoot, "skills"), containerRoot: path.posix.join(params.workspaceContainerRoot, "skills"), writable: false, source: "protectedSkill", + allowedRoot: params.localRoot, }, { localRoot: path.join(params.localRoot, ".agents", "skills"), containerRoot: path.posix.join(params.workspaceContainerRoot, ".agents", "skills"), writable: false, source: "protectedSkill", + allowedRoot: params.localRoot, + }, + { + localRoot: path.join(materializedSkillsWorkspaceDir, "skills"), + containerRoot: path.posix.join( + params.workspaceContainerRoot, + ".openclaw", + "sandbox-skills", + "skills", + ), + writable: false, + source: "protectedSkill", + allowedRoot: materializedSkillsWorkspaceDir, }, ]; if (params.includeAgentMount) { @@ -654,21 +678,37 @@ function buildRemoteProtectedSkillMounts(params: { containerRoot: path.posix.join(params.agentContainerRoot, "skills"), writable: false, source: "protectedSkill", + allowedRoot: params.localRoot, }, { localRoot: path.join(params.localRoot, ".agents", "skills"), containerRoot: path.posix.join(params.agentContainerRoot, ".agents", "skills"), writable: false, source: "protectedSkill", + allowedRoot: params.localRoot, + }, + { + localRoot: path.join(materializedSkillsWorkspaceDir, "skills"), + containerRoot: path.posix.join( + params.agentContainerRoot, + ".openclaw", + "sandbox-skills", + "skills", + ), + writable: false, + source: "protectedSkill", + allowedRoot: materializedSkillsWorkspaceDir, }, ); } - return mounts.filter((mount) => - isExistingWorkspaceSkillMountSource({ - agentWorkspaceDir: params.localRoot, - hostPath: mount.localRoot, - }), - ); + return mounts + .filter((mount) => + isExistingWorkspaceSkillMountSource({ + rootDir: mount.allowedRoot, + hostPath: mount.localRoot, + }), + ) + .map(({ allowedRoot: _allowedRoot, ...mount }) => mount); } function compareRemoteMountsByContainerPath(a: MountInfo, b: MountInfo): number { diff --git a/src/agents/sandbox/ssh-backend.test.ts b/src/agents/sandbox/ssh-backend.test.ts index 6b821f80d383..c6d71ffec5ce 100644 --- a/src/agents/sandbox/ssh-backend.test.ts +++ b/src/agents/sandbox/ssh-backend.test.ts @@ -1,5 +1,6 @@ // SSH sandbox backend tests cover runtime description/removal, remote seeding, // command execution, bind validation, and backend config plumbing. +import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; import { @@ -33,6 +34,13 @@ vi.mock("./ssh.js", async () => { }); const { createSshSandboxBackend, sshSandboxBackendManager } = await import("./ssh-backend.js"); +const tempDirs: string[] = []; + +async function createTempDir(prefix: string): Promise { + const dir = await fs.mkdtemp(path.join(os.tmpdir(), prefix)); + tempDirs.push(dir); + return dir; +} function createConfig(): OpenClawConfig { return { @@ -165,8 +173,11 @@ describe("ssh sandbox backend", () => { ]); }); - afterEach(() => { + afterEach(async () => { envSnapshot.restore(); + for (const dir of tempDirs.splice(0)) { + await fs.rm(dir, { recursive: true, force: true }); + } vi.restoreAllMocks(); }); @@ -238,12 +249,15 @@ describe("ssh sandbox backend", () => { stderr: Buffer.alloc(0), code: 0, }); + const skillsWorkspaceDir = await createTempDir("openclaw-ssh-skills-"); + await fs.mkdir(path.join(skillsWorkspaceDir, "skills"), { recursive: true }); const backend = await createSshSandboxBackend({ sessionKey: "agent:worker:task", scopeKey: "agent:worker", workspaceDir: "/tmp/workspace", agentWorkspaceDir: "/tmp/agent", + skillsWorkspaceDir, cfg: { mode: "all", backend: "ssh", @@ -300,7 +314,7 @@ describe("ssh sandbox backend", () => { createSession().host, ]); expect(execSpec.argv.at(-1)).toContain("/remote/openclaw/openclaw-ssh-agent-worker"); - expect(sshMocks.uploadDirectoryToSshTarget).toHaveBeenCalledTimes(2); + expect(sshMocks.uploadDirectoryToSshTarget).toHaveBeenCalledTimes(3); const workspaceUploadParams = requireSshUploadParams(0, "workspace upload params"); expect(workspaceUploadParams.localDir).toBe("/tmp/workspace"); expect(workspaceUploadParams.remoteDir).toContain("/workspace"); @@ -310,6 +324,12 @@ describe("ssh sandbox backend", () => { ); expect(agentUploadParams.localDir).toBe("/tmp/agent"); expect(agentUploadParams.remoteDir).toContain("/agent"); + const skillsUploadParams = requireRecord( + sshMocks.uploadDirectoryToSshTarget.mock.calls.at(2)?.[0], + "skills upload params", + ); + expect(skillsUploadParams.localDir).toBe(skillsWorkspaceDir); + expect(skillsUploadParams.remoteDir).toContain("/workspace/.openclaw/sandbox-skills"); await backend.finalizeExec?.({ status: "completed", @@ -321,6 +341,111 @@ describe("ssh sandbox backend", () => { expect(sshMocks.disposeSshSandboxSession).toHaveBeenCalledTimes(2); }); + it("refreshes materialized skills before each exec and remote fs command", async () => { + const skillsWorkspaceDir = await createTempDir("openclaw-ssh-skills-"); + await fs.mkdir(path.join(skillsWorkspaceDir, "skills"), { recursive: true }); + const backend = await createSshSandboxBackend({ + sessionKey: "agent:worker:task", + scopeKey: "agent:worker", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + skillsWorkspaceDir, + cfg: createBackendSandboxConfig({ + target: "peter@example.com:2222", + }), + }); + + const firstExec = await backend.buildExecSpec({ + command: "pwd", + env: {}, + usePty: false, + }); + const secondExec = await backend.buildExecSpec({ + command: "pwd", + env: {}, + usePty: false, + }); + await backend.runShellCommand({ + script: "printf ok", + }); + + expect(sshMocks.uploadDirectoryToSshTarget).toHaveBeenCalledTimes(3); + const skillsUploadParams = requireSshUploadParams(0, "skills upload params"); + expect(skillsUploadParams.localDir).toBe(skillsWorkspaceDir); + expect(skillsUploadParams.remoteDir).toContain("/workspace/.openclaw/sandbox-skills"); + await backend.finalizeExec?.({ + status: "completed", + exitCode: 0, + timedOut: false, + token: firstExec.finalizeToken, + }); + await backend.finalizeExec?.({ + status: "completed", + exitCode: 0, + timedOut: false, + token: secondExec.finalizeToken, + }); + }); + + it("clears stale remote materialized skills when the local copy is missing", async () => { + const tmpDir = await createTempDir("openclaw-ssh-skills-"); + const skillsWorkspaceDir = path.join(tmpDir, "missing"); + const backend = await createSshSandboxBackend({ + sessionKey: "agent:worker:task", + scopeKey: "agent:worker", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + skillsWorkspaceDir, + cfg: createBackendSandboxConfig({ + target: "peter@example.com:2222", + }), + }); + + const execSpec = await backend.buildExecSpec({ + command: "pwd", + env: {}, + usePty: false, + }); + + expect(sshMocks.uploadDirectoryToSshTarget).not.toHaveBeenCalled(); + const commandParams = requireSshRunCommandParams(1); + expect(commandParams.remoteCommand).toContain("openclaw-sandbox-clear"); + expect(commandParams.remoteCommand).toContain("/workspace/.openclaw/sandbox-skills"); + await backend.finalizeExec?.({ + status: "completed", + exitCode: 0, + timedOut: false, + token: execSpec.finalizeToken, + }); + }); + + it("disposes the exec ssh session when materialized skills refresh fails", async () => { + const skillsWorkspaceDir = await createTempDir("openclaw-ssh-skills-"); + await fs.mkdir(path.join(skillsWorkspaceDir, "skills"), { recursive: true }); + const backend = await createSshSandboxBackend({ + sessionKey: "agent:worker:task", + scopeKey: "agent:worker", + workspaceDir: "/tmp/workspace", + agentWorkspaceDir: "/tmp/workspace", + skillsWorkspaceDir, + cfg: createBackendSandboxConfig({ + target: "peter@example.com:2222", + }), + }); + sshMocks.uploadDirectoryToSshTarget.mockRejectedValueOnce(new Error("upload failed")); + + await expect( + backend.buildExecSpec({ + command: "pwd", + env: {}, + usePty: false, + }), + ).rejects.toThrow("upload failed"); + + expect(sshMocks.uploadDirectoryToSshTarget).toHaveBeenCalledTimes(1); + expect(sshMocks.disposeSshSandboxSession).toHaveBeenCalledTimes(2); + }); + it("filters blocked secrets from exec subprocess env", async () => { process.env.OPENAI_API_KEY = "sk-test-secret"; process.env.LANG = "en_US.UTF-8"; diff --git a/src/agents/sandbox/ssh-backend.ts b/src/agents/sandbox/ssh-backend.ts index 441fdccd2d19..ba8a4fa58fd4 100644 --- a/src/agents/sandbox/ssh-backend.ts +++ b/src/agents/sandbox/ssh-backend.ts @@ -3,6 +3,7 @@ * * Creates remote workspace copies, builds remote exec specs, and exposes a backend-neutral filesystem bridge. */ +import fs from "node:fs/promises"; import path from "node:path"; import { normalizeLowercaseStringOrEmpty } from "@openclaw/normalization-core/string-coerce"; import type { @@ -26,6 +27,7 @@ import { buildValidatedExecRemoteCommand, createSshSandboxSessionFromSettings, disposeSshSandboxSession, + ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT, runSshSandboxCommand, uploadDirectoryToSshTarget, type SshSandboxSession, @@ -40,6 +42,7 @@ type ResolvedSshRuntimePaths = { runtimeRootDir: string; remoteWorkspaceDir: string; remoteAgentWorkspaceDir: string; + remoteSkillsWorkspaceDir: string; }; /** SSH backend lifecycle hooks for probing and removing remote sandbox copies. */ @@ -157,16 +160,22 @@ class SshSandboxBackendImpl { }); await this.ensureRuntime(); const sshSession = await this.createSession(); - return { - argv: buildSshSandboxArgv({ - session: sshSession, - remoteCommand, - tty: usePty, - }), - env: sanitizeEnvVars(process.env).allowed, - stdinMode: "pipe-open", - finalizeToken: { sshSession } satisfies PendingExec, - }; + try { + await this.refreshRemoteSkillsWorkspace(sshSession); + return { + argv: buildSshSandboxArgv({ + session: sshSession, + remoteCommand, + tty: usePty, + }), + env: sanitizeEnvVars(process.env).allowed, + stdinMode: "pipe-open", + finalizeToken: { sshSession } satisfies PendingExec, + }; + } catch (error) { + await disposeSshSandboxSession(sshSession); + throw error; + } }, finalizeExec: async ({ token }) => { const sshSession = (token as PendingExec | undefined)?.sshSession; @@ -243,25 +252,50 @@ class SshSandboxBackendImpl { } } - private async replaceRemoteDirectoryFromLocal( - session: SshSandboxSession, - localDir: string, - remoteDir: string, - ): Promise { + private async refreshRemoteSkillsWorkspace(session: SshSandboxSession): Promise { + if ( + this.params.createParams.cfg.workspaceAccess !== "rw" || + !this.params.createParams.skillsWorkspaceDir + ) { + return; + } + await this.clearRemoteDirectory(session, this.params.runtimePaths.remoteSkillsWorkspaceDir); + if (!(await isExistingDirectory(this.params.createParams.skillsWorkspaceDir))) { + return; + } + await uploadDirectoryToSshTarget({ + session, + localDir: this.params.createParams.skillsWorkspaceDir, + remoteDir: this.params.runtimePaths.remoteSkillsWorkspaceDir, + remoteRootDir: this.params.runtimePaths.runtimeRootDir, + }); + } + + private async clearRemoteDirectory(session: SshSandboxSession, remoteDir: string): Promise { await runSshSandboxCommand({ session, remoteCommand: buildRemoteCommand([ "/bin/sh", "-c", - 'mkdir -p -- "$1" && find "$1" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +', + `${ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT}\nfind "$1" -mindepth 1 -maxdepth 1 -exec rm -rf -- {} +`, "openclaw-sandbox-clear", remoteDir, + this.params.runtimePaths.runtimeRootDir, ]), }); + } + + private async replaceRemoteDirectoryFromLocal( + session: SshSandboxSession, + localDir: string, + remoteDir: string, + ): Promise { + await this.clearRemoteDirectory(session, remoteDir); await uploadDirectoryToSshTarget({ session, localDir, remoteDir, + remoteRootDir: this.params.runtimePaths.runtimeRootDir, }); } @@ -271,6 +305,7 @@ class SshSandboxBackendImpl { await this.ensureRuntime(); const session = await this.createSession(); try { + await this.refreshRemoteSkillsWorkspace(session); return await runSshSandboxCommand({ session, remoteCommand: buildRemoteCommand([ @@ -290,6 +325,14 @@ class SshSandboxBackendImpl { } } +async function isExistingDirectory(dir: string): Promise { + try { + return (await fs.stat(dir)).isDirectory(); + } catch { + return false; + } +} + function resolveSshRuntimePaths(workspaceRoot: string, scopeKey: string): ResolvedSshRuntimePaths { const runtimeId = buildSshSandboxRuntimeId(scopeKey); const runtimeRootDir = path.posix.join(workspaceRoot, runtimeId); @@ -298,6 +341,12 @@ function resolveSshRuntimePaths(workspaceRoot: string, scopeKey: string): Resolv runtimeRootDir, remoteWorkspaceDir: path.posix.join(runtimeRootDir, "workspace"), remoteAgentWorkspaceDir: path.posix.join(runtimeRootDir, "agent"), + remoteSkillsWorkspaceDir: path.posix.join( + runtimeRootDir, + "workspace", + ".openclaw", + "sandbox-skills", + ), }; } diff --git a/src/agents/sandbox/ssh.test.ts b/src/agents/sandbox/ssh.test.ts index 6911fb3b658f..1ceb9c26a074 100644 --- a/src/agents/sandbox/ssh.test.ts +++ b/src/agents/sandbox/ssh.test.ts @@ -1,20 +1,24 @@ // SSH sandbox helper tests cover temp auth materialization, remote command // validation, shell quoting, and upload symlink safety. +import { execFile } from "node:child_process"; import fs from "node:fs/promises"; import os from "node:os"; import path from "node:path"; +import { promisify } from "node:util"; import { afterEach, describe, expect, it } from "vitest"; import { buildExecRemoteCommand, buildValidatedExecRemoteCommand, createSshSandboxSessionFromSettings, disposeSshSandboxSession, + ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT, type SshSandboxSession, uploadDirectoryToSshTarget, } from "./ssh.js"; const sessions: SshSandboxSession[] = []; const tempDirs: string[] = []; +const execFileAsync = promisify(execFile); afterEach(async () => { await Promise.all( @@ -176,6 +180,59 @@ describe("sandbox ssh helpers", () => { ).not.toThrow(); }); + it.runIf(process.platform !== "win32")( + "fails closed when remote upload directory validation fails", + () => { + expect(ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT.split("\n")[0]).toBe("set -e"); + }, + ); + + it.runIf(process.platform !== "win32")( + "allows symlinked ancestors before the trusted remote root", + async () => { + const realParent = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-ssh-real-")); + tempDirs.push(realParent); + const linkParent = `${realParent}-link`; + tempDirs.push(linkParent); + await fs.symlink(realParent, linkParent); + + const root = path.join(linkParent, "runtime"); + const target = path.join(root, "workspace", ".openclaw", "sandbox-skills"); + await execFileAsync("/bin/sh", [ + "-c", + ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT, + "openclaw-remote-dir", + target, + root, + ]); + + await expect( + fs.stat(path.join(realParent, "runtime", "workspace", ".openclaw", "sandbox-skills")), + ).resolves.toMatchObject({ dev: expect.any(Number) }); + }, + ); + + it.runIf(process.platform !== "win32")( + "rejects symlinked directories inside the trusted remote root", + async () => { + const realParent = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-ssh-real-")); + tempDirs.push(realParent); + const root = path.join(realParent, "runtime"); + await fs.mkdir(path.join(root, "workspace"), { recursive: true }); + await fs.symlink(realParent, path.join(root, "workspace", ".openclaw")); + + await expect( + execFileAsync("/bin/sh", [ + "-c", + ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT, + "openclaw-remote-dir", + path.join(root, "workspace", ".openclaw", "sandbox-skills"), + root, + ]), + ).rejects.toThrow(/unsafe remote directory symlink/); + }, + ); + it.runIf(process.platform !== "win32")( "rejects upload trees with symlinks that escape the local workspace", async () => { diff --git a/src/agents/sandbox/ssh.ts b/src/agents/sandbox/ssh.ts index e38f25f7c159..643adae511ea 100644 --- a/src/agents/sandbox/ssh.ts +++ b/src/agents/sandbox/ssh.ts @@ -655,20 +655,63 @@ export async function runSshSandboxCommand( }); } +export const ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT = [ + "set -e", + 'target="$1"', + 'root="${2:-$1}"', + 'case "$target" in /*) ;; *) echo "remote directory must be absolute: $target" >&2; exit 1 ;; esac', + 'case "$root" in /*) ;; *) echo "remote root must be absolute: $root" >&2; exit 1 ;; esac', + 'target="${target%/}"', + 'root="${root%/}"', + '[ -n "$target" ] || target="/"', + '[ -n "$root" ] || root="/"', + 'case "$target/" in "$root"/*|"$root/") ;; *) echo "remote directory must stay under root: $target" >&2; exit 1 ;; esac', + 'old_ifs="$IFS"', + 'IFS="/"', + "set -- ${target#/} ${root#/}", + 'IFS="$old_ifs"', + "for part do", + ' [ -n "$part" ] || continue', + ' case "$part" in "."|"..") echo "unsafe remote directory component: $part" >&2; exit 1 ;; esac', + "done", + 'if [ -L "$root" ]; then echo "unsafe remote root symlink: $root" >&2; exit 1; fi', + 'mkdir -p -- "$root"', + 'canonical_root="$(cd "$root" && pwd -P)"', + 'relative="${target#"$root"}"', + 'relative="${relative#/}"', + 'current="$canonical_root"', + 'IFS="/"', + "set -- $relative", + 'IFS="$old_ifs"', + "for part do", + ' [ -n "$part" ] || continue', + ' if [ "$current" = "/" ]; then next="/$part"; else next="$current/$part"; fi', + ' if [ -L "$next" ]; then echo "unsafe remote directory symlink: $next" >&2; exit 1; fi', + ' if [ -e "$next" ]; then', + ' if [ ! -d "$next" ]; then echo "unsafe remote directory component: $next" >&2; exit 1; fi', + " else", + ' mkdir -- "$next"', + " fi", + ' current="$next"', + "done", +].join("\n"); + /** Stream a local directory to the remote sandbox with tar over ssh. */ export async function uploadDirectoryToSshTarget(params: { session: SshSandboxSession; localDir: string; remoteDir: string; + remoteRootDir?: string; signal?: AbortSignal; }): Promise { await assertSafeUploadSymlinks(params.localDir); const remoteCommand = buildRemoteCommand([ "/bin/sh", "-c", - 'mkdir -p -- "$1" && tar -xf - -C "$1"', + `${ENSURE_REMOTE_REAL_DIRECTORY_SCRIPT}\ntar -xf - -C "$1"`, "openclaw-sandbox-upload", params.remoteDir, + params.remoteRootDir ?? params.remoteDir, ]); const sshArgv = buildSshSandboxArgv({ session: params.session, diff --git a/src/agents/sandbox/types.ts b/src/agents/sandbox/types.ts index 987729704821..ee6e9202a192 100644 --- a/src/agents/sandbox/types.ts +++ b/src/agents/sandbox/types.ts @@ -6,6 +6,7 @@ import type { SandboxBackendHandle, SandboxBackendId } from "./backend-handle.types.js"; import type { SandboxFsBridge } from "./fs-bridge.types.js"; import type { SandboxDockerConfig } from "./types.docker.js"; +import type { SkillEligibilityContext } from "../../skills/types.js"; export type { SandboxDockerConfig } from "./types.docker.js"; @@ -97,6 +98,8 @@ export type SandboxContext = { sessionKey: string; workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; + skillsEligibility?: SkillEligibilityContext; workspaceAccess: SandboxWorkspaceAccess; runtimeId: string; runtimeLabel: string; diff --git a/src/agents/sandbox/workspace-mounts.test.ts b/src/agents/sandbox/workspace-mounts.test.ts index 91d6ef88eb36..0eae79315191 100644 --- a/src/agents/sandbox/workspace-mounts.test.ts +++ b/src/agents/sandbox/workspace-mounts.test.ts @@ -173,6 +173,32 @@ describe("appendWorkspaceMountArgs", () => { ]); }); + it("overlays materialized sandbox skills read-only when workspaceAccess is rw", () => { + const agentWorkspaceDir = makeTempWorkspace(); + const skillsWorkspaceDir = makeTempWorkspace(); + const materializedSkillsDir = path.join(skillsWorkspaceDir, "skills"); + fs.mkdirSync(path.join(materializedSkillsDir, "demo"), { recursive: true }); + fs.writeFileSync(path.join(materializedSkillsDir, "demo", "SKILL.md"), "# Demo\n"); + + const args: string[] = []; + appendWorkspaceMountArgs({ + args, + workspaceDir: agentWorkspaceDir, + agentWorkspaceDir, + skillsWorkspaceDir, + workdir: "/workspace", + workspaceAccess: "rw", + }); + + const mounts = args.filter( + (arg) => arg.startsWith(agentWorkspaceDir) || arg.startsWith(skillsWorkspaceDir), + ); + expect(mounts).toEqual([ + `${agentWorkspaceDir}:/workspace:z`, + `${materializedSkillsDir}:/workspace/.openclaw/sandbox-skills/skills:ro,z`, + ]); + }); + it("does not add a separate synced skill overlay when workspaceAccess is ro", () => { const agentWorkspaceDir = makeTempWorkspace(); const sandboxWorkspaceDir = makeTempWorkspace(); diff --git a/src/agents/sandbox/workspace-mounts.ts b/src/agents/sandbox/workspace-mounts.ts index 7daaa76740d1..579845ffb42c 100644 --- a/src/agents/sandbox/workspace-mounts.ts +++ b/src/agents/sandbox/workspace-mounts.ts @@ -11,6 +11,7 @@ import { resolveSandboxHostPathViaExistingAncestor } from "./host-paths.js"; import type { SandboxWorkspaceAccess } from "./types.js"; export const SANDBOX_MOUNT_FORMAT_VERSION = 3; +const MATERIALIZED_SANDBOX_SKILLS_WORKSPACE_PARTS = [".openclaw", "sandbox-skills"] as const; /** Read-only skill directory mounted from the agent workspace into the sandbox workspace. */ export type ReadOnlyWorkspaceSkillMount = { @@ -35,9 +36,18 @@ function containerJoin(root: string, ...parts: string[]): string { return suffix ? `${normalizedRoot}/${suffix}` : normalizedRoot; } -/** Returns true when a skill mount source exists inside the canonical agent workspace. */ +/** Hidden workspace used to materialize non-workspace skills for rw sandboxes. */ +export function resolveMaterializedSandboxSkillsWorkspaceDir(rootDir: string): string { + return path.join(rootDir, ...MATERIALIZED_SANDBOX_SKILLS_WORKSPACE_PARTS); +} + +export function resolveMaterializedSandboxSkillsRoot(rootDir: string): string { + return path.join(resolveMaterializedSandboxSkillsWorkspaceDir(rootDir), "skills"); +} + +/** Returns true when a skill mount source exists inside the canonical mount root. */ export function isExistingWorkspaceSkillMountSource(params: { - agentWorkspaceDir: string; + rootDir: string; hostPath: string; }): boolean { try { @@ -48,9 +58,7 @@ export function isExistingWorkspaceSkillMountSource(params: { return false; } - const agentRoot = resolveSandboxHostPathViaExistingAncestor( - path.resolve(params.agentWorkspaceDir), - ); + const agentRoot = resolveSandboxHostPathViaExistingAncestor(path.resolve(params.rootDir)); const canonicalSource = resolveSandboxHostPathViaExistingAncestor(path.resolve(params.hostPath)); return isPathInside(agentRoot, canonicalSource); } @@ -59,6 +67,7 @@ export function isExistingWorkspaceSkillMountSource(params: { export function resolveReadOnlyWorkspaceSkillMounts(params: { workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; workdir: string; workspaceAccess: SandboxWorkspaceAccess; }): ReadOnlyWorkspaceSkillMount[] { @@ -68,23 +77,38 @@ export function resolveReadOnlyWorkspaceSkillMounts(params: { // RW workspaces mount the project as writable, but skill sources remain read-only so agent // instructions are visible without letting sandbox commands mutate them. + const materializedSkillsWorkspaceDir = + params.skillsWorkspaceDir ?? resolveMaterializedSandboxSkillsWorkspaceDir(params.agentWorkspaceDir); const mounts = [ { hostPath: path.join(params.agentWorkspaceDir, "skills"), containerPath: containerJoin(params.workdir, "skills"), + rootDir: params.agentWorkspaceDir, }, { hostPath: path.join(params.agentWorkspaceDir, ".agents", "skills"), containerPath: containerJoin(params.workdir, ".agents", "skills"), + rootDir: params.agentWorkspaceDir, + }, + { + hostPath: path.join(materializedSkillsWorkspaceDir, "skills"), + containerPath: containerJoin( + params.workdir, + ...MATERIALIZED_SANDBOX_SKILLS_WORKSPACE_PARTS, + "skills", + ), + rootDir: materializedSkillsWorkspaceDir, }, ]; - return mounts.filter((mount) => - isExistingWorkspaceSkillMountSource({ - agentWorkspaceDir: params.agentWorkspaceDir, - hostPath: mount.hostPath, - }), - ); + return mounts + .filter((mount) => + isExistingWorkspaceSkillMountSource({ + rootDir: mount.rootDir, + hostPath: mount.hostPath, + }), + ) + .map(({ hostPath, containerPath }) => ({ hostPath, containerPath })); } /** Returns stable mount state for sandbox config hashes. */ @@ -116,6 +140,7 @@ export function appendWorkspaceMountArgs(params: { args: string[]; workspaceDir: string; agentWorkspaceDir: string; + skillsWorkspaceDir?: string; workdir: string; workspaceAccess: SandboxWorkspaceAccess; readOnlyWorkspaceSkillMounts?: readonly ReadOnlyWorkspaceSkillMount[]; @@ -151,6 +176,7 @@ export function appendWorkspaceMountArgs(params: { resolveReadOnlyWorkspaceSkillMounts({ workspaceDir, agentWorkspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, workdir, workspaceAccess, }), diff --git a/src/agents/sandbox/workspace-skills-bridge-readonly.test.ts b/src/agents/sandbox/workspace-skills-bridge-readonly.test.ts index af965b6f990f..c7d91d024d70 100644 --- a/src/agents/sandbox/workspace-skills-bridge-readonly.test.ts +++ b/src/agents/sandbox/workspace-skills-bridge-readonly.test.ts @@ -45,10 +45,16 @@ describe("workspace skills bridge mount policy", () => { it("resolves workspace skill roots as read-only", async () => { await withTempDir("openclaw-skills-bridge-", async (stateDir) => { const workspaceDir = path.join(stateDir, "workspace"); + const skillsWorkspaceDir = path.join(stateDir, "sandbox-state"); await fs.mkdir(path.join(workspaceDir, "skills", "demo"), { recursive: true }); await fs.mkdir(path.join(workspaceDir, ".agents", "skills", "demo"), { recursive: true }); + await fs.mkdir(path.join(skillsWorkspaceDir, "skills", "demo"), { recursive: true }); - const sandbox = createSandbox({ workspaceDir, agentWorkspaceDir: workspaceDir }); + const sandbox = createSandbox({ + workspaceDir, + agentWorkspaceDir: workspaceDir, + skillsWorkspaceDir, + }); const mounts = buildSandboxFsMounts(sandbox); const resolve = (filePath: string) => resolveSandboxFsPathWithMounts({ @@ -62,7 +68,14 @@ describe("workspace skills bridge mount policy", () => { expect(resolve("normal.txt").writable).toBe(true); expect(resolve("skills/demo/SKILL.md").writable).toBe(false); expect(resolve(".agents/skills/demo/SKILL.md").writable).toBe(false); + expect(resolve(".openclaw/sandbox-skills/skills/demo/SKILL.md").writable).toBe(false); + expect(resolve(".openclaw/sandbox-skills/skills/demo/SKILL.md").hostPath).toBe( + path.join(skillsWorkspaceDir, "skills", "demo", "SKILL.md"), + ); expect(resolve("/workspace/skills/demo/SKILL.md").writable).toBe(false); + expect(resolve("/workspace/.openclaw/sandbox-skills/skills/demo/SKILL.md").writable).toBe( + false, + ); }); }); @@ -99,16 +112,20 @@ describe("workspace skills bridge mount policy", () => { async () => { await withTempDir("openclaw-skills-remote-only-", async (stateDir) => { const workspaceDir = path.join(stateDir, "workspace"); + const skillsWorkspaceDir = path.join(stateDir, "sandbox-state"); const remoteWorkspaceDir = path.join(stateDir, "remote-workspace"); await fs.mkdir(workspaceDir, { recursive: true }); await fs.mkdir(path.join(remoteWorkspaceDir, "skills", "demo"), { recursive: true }); + await fs.mkdir(path.join(skillsWorkspaceDir, "skills", "demo"), { recursive: true }); const canonicalWorkspaceDir = await fs.realpath(workspaceDir); + const canonicalSkillsWorkspaceDir = await fs.realpath(skillsWorkspaceDir); const canonicalRemoteWorkspaceDir = await fs.realpath(remoteWorkspaceDir); const bridge = createRemoteShellSandboxFsBridge({ sandbox: createSandbox({ workspaceDir: canonicalWorkspaceDir, agentWorkspaceDir: canonicalWorkspaceDir, + skillsWorkspaceDir: canonicalSkillsWorkspaceDir, }), runtime: { remoteWorkspaceDir: canonicalRemoteWorkspaceDir, @@ -127,6 +144,26 @@ describe("workspace skills bridge mount policy", () => { await expect( fs.stat(path.join(canonicalRemoteWorkspaceDir, "skills", "demo", "SKILL.md")), ).rejects.toMatchObject({ code: "ENOENT" }); + + await expect( + bridge.writeFile({ + filePath: ".openclaw/sandbox-skills/skills/demo/SKILL.md", + cwd: canonicalRemoteWorkspaceDir, + data: "# Demo\n", + }), + ).rejects.toThrow(/read-only/); + await expect( + fs.stat( + path.join( + canonicalRemoteWorkspaceDir, + ".openclaw", + "sandbox-skills", + "skills", + "demo", + "SKILL.md", + ), + ), + ).rejects.toMatchObject({ code: "ENOENT" }); }); }, ); diff --git a/src/agents/test-helpers/agent-tools-sandbox-context.ts b/src/agents/test-helpers/agent-tools-sandbox-context.ts index 6759da729ffd..1ca3e8165212 100644 --- a/src/agents/test-helpers/agent-tools-sandbox-context.ts +++ b/src/agents/test-helpers/agent-tools-sandbox-context.ts @@ -10,6 +10,7 @@ type AgentToolsSandboxContextParams = { workspaceDir: string; agentWorkspaceDir?: string; workspaceAccess?: SandboxWorkspaceAccess; + skillsWorkspaceDir?: string; fsBridge?: SandboxFsBridge; tools?: SandboxToolPolicy; browserAllowHostControl?: boolean; @@ -30,6 +31,7 @@ export function createAgentToolsSandboxContext( sessionKey: params.sessionKey ?? "sandbox:test", workspaceDir, agentWorkspaceDir: params.agentWorkspaceDir ?? workspaceDir, + skillsWorkspaceDir: params.skillsWorkspaceDir, workspaceAccess: params.workspaceAccess ?? "rw", runtimeId: params.containerName ?? "openclaw-sbx-test", runtimeLabel: params.containerName ?? "openclaw-sbx-test", diff --git a/src/agents/test-helpers/unsafe-mounted-sandbox.ts b/src/agents/test-helpers/unsafe-mounted-sandbox.ts index 92b0d9337d4e..74b466ab2a95 100644 --- a/src/agents/test-helpers/unsafe-mounted-sandbox.ts +++ b/src/agents/test-helpers/unsafe-mounted-sandbox.ts @@ -14,13 +14,29 @@ import { createSandboxFsBridgeFromResolver } from "./host-sandbox-fs-bridge.js"; function createUnsafeMountedBridge(params: { root: string; agentHostRoot: string; + skillsHostRoot?: string; workspaceContainerRoot?: string; }): SandboxFsBridge { const root = path.resolve(params.root); const agentHostRoot = path.resolve(params.agentHostRoot); + const skillsHostRoot = params.skillsHostRoot ? path.resolve(params.skillsHostRoot) : undefined; const workspaceContainerRoot = params.workspaceContainerRoot ?? "/workspace"; + const skillsContainerRoot = path.posix.join( + workspaceContainerRoot, + ".openclaw", + "sandbox-skills", + "skills", + ); + const skillsRelativeRoot = ".openclaw/sandbox-skills/skills"; const resolvePath = (filePath: string, cwd?: string): SandboxResolvedPath => { + const normalizedRelativePath = path.posix.normalize(filePath.replace(/\\/g, "/")); + const skillsRelativePath = + normalizedRelativePath === skillsRelativeRoot + ? "" + : normalizedRelativePath.startsWith(`${skillsRelativeRoot}/`) + ? normalizedRelativePath.slice(skillsRelativeRoot.length + 1) + : undefined; // Intentionally unsafe: simulate a sandbox FS bridge that maps /agent/* into a host path // outside the workspace root (e.g. an operator-configured bind mount). const hostPath = @@ -29,9 +45,14 @@ function createUnsafeMountedBridge(params: { agentHostRoot, filePath === "/agent" || filePath === "/agent/" ? "" : filePath.slice("/agent/".length), ) - : path.isAbsolute(filePath) - ? filePath - : path.resolve(cwd ?? root, filePath); + : skillsHostRoot && + (filePath === skillsContainerRoot || filePath.startsWith(`${skillsContainerRoot}/`)) + ? path.join(skillsHostRoot, filePath.slice(skillsContainerRoot.length + 1)) + : skillsHostRoot && skillsRelativePath !== undefined + ? path.join(skillsHostRoot, skillsRelativePath) + : path.isAbsolute(filePath) + ? filePath + : path.resolve(cwd ?? root, filePath); const relFromRoot = path.relative(root, hostPath); const relativePath = @@ -54,17 +75,22 @@ function createUnsafeMountedBridge(params: { export function createUnsafeMountedSandbox(params: { sandboxRoot: string; agentRoot: string; + skillsWorkspaceDir?: string; workspaceAccess?: "none" | "ro" | "rw"; workspaceContainerRoot?: string; }): SandboxContext { const bridge = createUnsafeMountedBridge({ root: params.sandboxRoot, agentHostRoot: params.agentRoot, + skillsHostRoot: params.skillsWorkspaceDir + ? path.join(params.skillsWorkspaceDir, "skills") + : undefined, workspaceContainerRoot: params.workspaceContainerRoot, }); return createAgentToolsSandboxContext({ workspaceDir: params.sandboxRoot, agentWorkspaceDir: params.agentRoot, + skillsWorkspaceDir: params.skillsWorkspaceDir, workspaceAccess: params.workspaceAccess ?? "rw", fsBridge: bridge, tools: { allow: [], deny: [] }, @@ -72,21 +98,34 @@ export function createUnsafeMountedSandbox(params: { } export async function withUnsafeMountedSandboxHarness( - run: (ctx: { sandboxRoot: string; agentRoot: string; sandbox: SandboxContext }) => Promise, - options?: { workspaceAccess?: "none" | "ro" | "rw" }, + run: (ctx: { + sandboxRoot: string; + agentRoot: string; + skillsWorkspaceDir?: string; + sandbox: SandboxContext; + }) => Promise, + options?: { + includeSkillsWorkspace?: boolean; + skillsWorkspaceDir?: string; + workspaceAccess?: "none" | "ro" | "rw"; + }, ) { const stateDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sbx-mounts-")); const sandboxRoot = path.join(stateDir, "sandbox"); const agentRoot = path.join(stateDir, "agent"); + const skillsWorkspaceDir = + options?.skillsWorkspaceDir ?? + (options?.includeSkillsWorkspace ? path.join(stateDir, "skills-state") : undefined); await fs.mkdir(sandboxRoot, { recursive: true }); await fs.mkdir(agentRoot, { recursive: true }); const sandbox = createUnsafeMountedSandbox({ sandboxRoot, agentRoot, + skillsWorkspaceDir, workspaceAccess: options?.workspaceAccess, }); try { - await run({ sandboxRoot, agentRoot, sandbox }); + await run({ sandboxRoot, agentRoot, skillsWorkspaceDir, sandbox }); } finally { await fs.rm(stateDir, { recursive: true, force: true }); } diff --git a/src/skills/loading/workspace-load.test.ts b/src/skills/loading/workspace-load.test.ts index dd7cd9b9bb94..dc35a2878a76 100644 --- a/src/skills/loading/workspace-load.test.ts +++ b/src/skills/loading/workspace-load.test.ts @@ -461,6 +461,34 @@ describe("loadWorkspaceSkillEntries", () => { expect(entries.map((entry) => entry.skill.name)).toEqual(["remote-only"]); }); + it("filters remote-ineligible skills when no agent skill filter is active", async () => { + const workspaceDir = await createTempWorkspaceDir(); + await writeSkill({ + dir: path.join(workspaceDir, "skills", "local-only"), + name: "local-only", + description: "Always available", + }); + await writeSkill({ + dir: path.join(workspaceDir, "skills", "remote-only"), + name: "remote-only", + description: "Needs a remote bin", + metadata: '{"openclaw":{"requires":{"anyBins":["missingbin","sandboxbin"]}}}', + }); + + const entries = loadTestWorkspaceSkillEntries(workspaceDir, { + eligibility: { + remote: { + platforms: ["linux"], + hasBin: () => false, + hasAnyBin: () => false, + note: "sandbox", + }, + }, + }); + + expect(entries.map((entry) => entry.skill.name)).toEqual(["local-only"]); + }); + it.runIf(process.platform !== "win32")( "skips workspace skill paths that resolve outside the workspace root", async () => { diff --git a/src/skills/loading/workspace-sync.test.ts b/src/skills/loading/workspace-sync.test.ts index f7ec04105438..99943140ece6 100644 --- a/src/skills/loading/workspace-sync.test.ts +++ b/src/skills/loading/workspace-sync.test.ts @@ -145,6 +145,25 @@ describe("buildWorkspaceSkillsPrompt", () => { ).toBe(false); }); + it.runIf(process.platform !== "win32")( + "preserves the target skills directory while refreshing children", + 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 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); + }, + ); + it("syncs the explicit agent skill subset instead of inherited defaults", async () => { const sourceWorkspace = await createCaseDir("source"); const targetWorkspace = await createCaseDir("target"); diff --git a/src/skills/loading/workspace.ts b/src/skills/loading/workspace.ts index a2ce97b9964a..1148f640b26e 100644 --- a/src/skills/loading/workspace.ts +++ b/src/skills/loading/workspace.ts @@ -853,6 +853,7 @@ function loadSkillEntries( managedSkillsDir?: string; bundledSkillsDir?: string; pluginSkillsDir?: string; + workspaceOnly?: boolean; }, ): SkillEntry[] { const limits = resolveSkillsLimits(opts?.config, opts?.agentId); @@ -1112,17 +1113,22 @@ function loadSkillEntries( return loadedSkills; }; + const workspaceOnly = opts?.workspaceOnly === true; const managedSkillsDir = opts?.managedSkillsDir ?? path.join(CONFIG_DIR, "skills"); const workspaceSkillsDir = path.resolve(workspaceDir, "skills"); - const bundledSkillsDir = opts?.bundledSkillsDir ?? resolveBundledSkillsDir(); + const bundledSkillsDir = workspaceOnly + ? undefined + : (opts?.bundledSkillsDir ?? resolveBundledSkillsDir()); const pluginSkillsDir = opts?.pluginSkillsDir ?? path.join(CONFIG_DIR, "plugin-skills"); - const extraDirsRaw = opts?.config?.skills?.load?.extraDirs ?? []; + const extraDirsRaw = workspaceOnly ? [] : (opts?.config?.skills?.load?.extraDirs ?? []); const extraDirs = normalizeTrimmedStringList(extraDirsRaw); - const pluginSkillDirs = resolvePluginSkillDirs({ - workspaceDir, - config: opts?.config, - pluginSkillsDir, - }); + const pluginSkillDirs = workspaceOnly + ? [] + : resolvePluginSkillDirs({ + workspaceDir, + config: opts?.config, + pluginSkillsDir, + }); const mergedExtraDirs = [...extraDirs, ...pluginSkillDirs]; const bundledSkills = bundledSkillsDir @@ -1146,23 +1152,29 @@ function loadSkillEntries( limits, }), ]; - const managedSkills = loadSkills({ - dir: managedSkillsDir, - source: "openclaw-managed", - }); + const managedSkills = workspaceOnly + ? [] + : loadSkills({ + dir: managedSkillsDir, + source: "openclaw-managed", + }); const osHomeDir = resolveUserHomeDir(); const personalAgentsSkillsDir = osHomeDir ? path.resolve(osHomeDir, ".agents", "skills") : path.resolve(".agents", "skills"); - const personalAgentsSkills = loadSkills({ - dir: personalAgentsSkillsDir, - source: "agents-skills-personal", - }); + const personalAgentsSkills = workspaceOnly + ? [] + : loadSkills({ + dir: personalAgentsSkillsDir, + source: "agents-skills-personal", + }); const projectAgentsSkillsDir = path.resolve(workspaceDir, ".agents", "skills"); - const projectAgentsSkills = loadSkills({ - dir: projectAgentsSkillsDir, - source: "agents-skills-project", - }); + const projectAgentsSkills = workspaceOnly + ? [] + : loadSkills({ + dir: projectAgentsSkillsDir, + source: "agents-skills-project", + }); const workspaceSkills = loadSkills({ dir: workspaceSkillsDir, source: "openclaw-workspace", @@ -1425,6 +1437,7 @@ export function resolveSkillsPromptForRun(params: { config?: OpenClawConfig; workspaceDir: string; agentId?: string; + eligibility?: SkillEligibilityContext; }): string { const snapshotPrompt = params.skillsSnapshot?.prompt?.trim(); if (snapshotPrompt) { @@ -1435,6 +1448,7 @@ export function resolveSkillsPromptForRun(params: { entries: params.entries, config: params.config, agentId: params.agentId, + eligibility: params.eligibility, }); return prompt.trim() ? prompt : ""; } @@ -1451,11 +1465,12 @@ export function loadWorkspaceSkillEntries( skillFilter?: string[]; agentId?: string; eligibility?: SkillEligibilityContext; + workspaceOnly?: boolean; }, ): SkillEntry[] { const entries = loadSkillEntries(workspaceDir, opts); const effectiveSkillFilter = resolveEffectiveWorkspaceSkillFilter(opts); - if (effectiveSkillFilter === undefined) { + if (effectiveSkillFilter === undefined && opts?.eligibility === undefined) { return entries; } return filterSkillEntries(entries, opts?.config, effectiveSkillFilter, opts?.eligibility); @@ -1518,6 +1533,29 @@ function resolveSyncedSkillDestinationPath(params: { }).resolved; } +async function prepareSyncedSkillsDirectory(targetSkillsDir: string): Promise { + let stats: fs.Stats; + try { + stats = await fsp.lstat(targetSkillsDir); + } 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 }); + return; + } + + for (const entry of await fsp.readdir(targetSkillsDir)) { + await fsp.rm(path.join(targetSkillsDir, entry), { recursive: true, force: true }); + } +} + export async function syncSkillsToWorkspace(params: { sourceWorkspaceDir: string; targetWorkspaceDir: string; @@ -1548,8 +1586,7 @@ export async function syncSkillsToWorkspace(params: { pluginSkillsDir: params.pluginSkillsDir, }); - await fsp.rm(targetSkillsDir, { recursive: true, force: true }); - await fsp.mkdir(targetSkillsDir, { recursive: true }); + await prepareSyncedSkillsDirectory(targetSkillsDir); const usedDirNames = new Set(); for (const entry of entries) { diff --git a/src/skills/runtime/embedded-run-entries.test.ts b/src/skills/runtime/embedded-run-entries.test.ts index 4c478b8e7b0a..cb862bc51cd0 100644 --- a/src/skills/runtime/embedded-run-entries.test.ts +++ b/src/skills/runtime/embedded-run-entries.test.ts @@ -58,6 +58,37 @@ describe("resolveEmbeddedRunSkillEntries", () => { }); }); + it("can constrain live loading to materialized workspace skills", () => { + const eligibility = { + remote: { + platforms: ["linux"], + hasBin: () => false, + hasAnyBin: () => true, + note: "sandbox", + }, + }; + + resolveEmbeddedRunSkillEntries({ + workspaceDir: "/tmp/workspace/.openclaw/sandbox-skills", + config: {}, + eligibility, + skillsSnapshot: { + prompt: "skills prompt", + skills: [], + }, + workspaceOnly: true, + }); + + expect(loadWorkspaceSkillEntriesSpy).toHaveBeenCalledWith( + "/tmp/workspace/.openclaw/sandbox-skills", + { + config: {}, + eligibility, + workspaceOnly: true, + }, + ); + }); + it("prefers the active runtime snapshot when caller config still contains SecretRefs", () => { const sourceConfig: OpenClawConfig = { skills: { diff --git a/src/skills/runtime/embedded-run-entries.ts b/src/skills/runtime/embedded-run-entries.ts index dca3f40b27e1..e7f3eaf40824 100644 --- a/src/skills/runtime/embedded-run-entries.ts +++ b/src/skills/runtime/embedded-run-entries.ts @@ -2,14 +2,16 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js"; import { resolveSkillRuntimeConfig } from "../loading/runtime-config.js"; import { loadWorkspaceSkillEntries } from "../loading/workspace.js"; -import type { SkillEntry, SkillSnapshot } from "../types.js"; +import type { SkillEligibilityContext, SkillEntry, SkillSnapshot } from "../types.js"; /** Resolves skill entries embedded into a run payload into runtime-visible entries. */ export function resolveEmbeddedRunSkillEntries(params: { workspaceDir: string; config?: OpenClawConfig; agentId?: string; + eligibility?: SkillEligibilityContext; skillsSnapshot?: SkillSnapshot; + workspaceOnly?: boolean; }): { shouldLoadSkillEntries: boolean; skillEntries: SkillEntry[]; @@ -19,7 +21,12 @@ export function resolveEmbeddedRunSkillEntries(params: { return { shouldLoadSkillEntries, skillEntries: shouldLoadSkillEntries - ? loadWorkspaceSkillEntries(params.workspaceDir, { config, agentId: params.agentId }) + ? loadWorkspaceSkillEntries(params.workspaceDir, { + config, + agentId: params.agentId, + ...(params.eligibility ? { eligibility: params.eligibility } : {}), + ...(params.workspaceOnly === true ? { workspaceOnly: true } : {}), + }) : [], }; }