diff --git a/docs/manage-sandboxes/workspace-files.mdx b/docs/manage-sandboxes/workspace-files.mdx index e913106556b..0d63c0b3f4c 100644 --- a/docs/manage-sandboxes/workspace-files.mdx +++ b/docs/manage-sandboxes/workspace-files.mdx @@ -170,8 +170,8 @@ Deep Agents Code creates them when you start the first `dcode` session. | `/sandbox/.deepagents/.state/` | Deep Agents Code runtime state, including persisted session and MCP-related state. | | `/sandbox/.deepagents/agent/AGENTS.md` | Global memory file for the default Deep Agents Code agent, loaded at session start. | | `/sandbox/.deepagents/agent/memories/` | Topic-specific markdown memories that Deep Agents Code can read and update across sessions. | -| `/sandbox/.deepagents/skills/` | User-level skills available to Deep Agents Code in the sandbox. | -| `/sandbox/.deepagents/agent/skills/` | Default-agent skills created by the built-in skill creator and preserved by NemoClaw snapshots. | +| `/sandbox/.deepagents/skills/` | Legacy NemoClaw skill-upload state. Deep Agents Code does not load skills from this path, and `$$nemoclaw skill install` leaves it untouched. | +| `/sandbox/.deepagents/agent/skills/` | Skills that Deep Agents Code loads at session start, including skills created by the built-in skill creator and fresh-name skills installed directly by `skill install`. Preserved by NemoClaw snapshots. | | `/sandbox/.deepagents/.nemoclaw-mcp.json` | NemoClaw-generated managed MCP projection with OpenShell credential placeholders. NemoClaw reconstructs it from host-side registry state. | | `/sandbox/.deepagents/.state/auth.json` | Upstream auth state. The managed launchers refuse to start when this file contains credentials. | | `/sandbox/.deepagents/.state/chatgpt-auth.json` | Upstream ChatGPT auth state. The managed launchers refuse to start when this file exists. | diff --git a/docs/reference/commands.mdx b/docs/reference/commands.mdx index 28d9c8f78fa..e1d14995ae2 100644 --- a/docs/reference/commands.mdx +++ b/docs/reference/commands.mdx @@ -2338,6 +2338,7 @@ To install an OpenClaw plugin, refer to [Install OpenClaw Plugins](../manage-san For OpenClaw, the command uploads the skill to the OpenClaw state directory and mirrors it into `$HOME/.openclaw/skills/` when the agent home directory differs from the state directory. That mirror makes skills listed by `openclaw skills list` available at session startup. If mirror creation fails, NemoClaw prints a warning so you can reinstall or inspect the home directory permissions. +OpenClaw caches skill content per session, so the command also refreshes the OpenClaw session index after every install and update to avoid stale `SKILL.md` data. @@ -2348,8 +2349,17 @@ Hermes plugins are different from NemoClaw skills. -For Deep Agents, the command uploads skills under `/sandbox/.deepagents/skills/` and mirrors agent-facing skills under `/sandbox/.deepagents/agent/skills/` when the manifest requires that view. -The managed `dcode` launchers discover those skills on the next session without accepting executable hook configuration. +For Deep Agents, the command installs a fresh skill directly into `/sandbox/.deepagents/agent/skills/`, the directory Deep Agents Code loads at session start. +The command creates an archive from the selected regular files and records their paths and hashes. +It rejects symlinks and special files. +Inside the sandbox, it stages the archive and verifies the same path-and-byte manifest. +It then moves the staged directory into place only if the destination is still absent. +Because Deep Agents and its built-in skill creator also own this directory, the command refuses any name whose file, directory, or symlink already exists. +Updates are not automatic. +Inspect the existing skill and use the agent's native or manual lifecycle after confirming ownership. +The legacy `/sandbox/.deepagents/skills/` path is not written or treated as ownership proof. +The managed `dcode` launchers discover newly installed skills on the next session without accepting executable hook configuration. +Installation does not enable project hooks or unmanaged MCP files. @@ -2358,18 +2368,20 @@ If you pass a plugin-shaped directory to `skill install`, the CLI prints a plugi Files with names starting with `.` (dotfiles) are skipped and listed in the output. Files with unsafe path characters are rejected to prevent shell injection. +Symlinks and other non-regular paths are rejected rather than followed or copied. -If the skill already exists on the sandbox, the command updates it in place and preserves chat history. -For new installs, the agent session index is refreshed so the agent discovers the skill on the next session. +For OpenClaw and Hermes, an existing sandbox skill is updated in place and chat history is preserved. +Deep Agents supports only fresh-name installs because its active skill directory is shared with agent-authored content. +Follow the agent-specific activation guidance above after installation. ### `$$nemoclaw skill remove ` Remove an installed skill from a running sandbox by skill name. -The command validates the skill name, removes the sandbox upload directory, and refreshes the agent session index so the remaining skills are rediscovered on the next session. +The command validates the skill name before it applies the agent-specific removal behavior below. -For OpenClaw, the command also removes the OpenClaw home-directory mirror when present. +For OpenClaw, the command also removes the OpenClaw home-directory mirror when present and refreshes the OpenClaw session index. @@ -2379,8 +2391,9 @@ Run `$$nemoclaw gateway restart` if prompted so the removal takes effect. -For Deep Agents, the command removes the uploaded skill from the managed `/sandbox/.deepagents` skill paths and refreshes the session index for future `dcode` sessions. -It does not enable project hooks or unmanaged MCP files. +For Deep Agents, automatic removal is refused before any sandbox files change. +The active `/sandbox/.deepagents/agent/skills/` directory is shared with agent-authored content, so its presence alone cannot prove NemoClaw owns it. +Inspect the skill and use the agent's native or manual lifecycle after confirming ownership. diff --git a/src/lib/actions/sandbox/skill-install.test.ts b/src/lib/actions/sandbox/skill-install.test.ts index 44356b8bfd6..ec8d15cc846 100644 --- a/src/lib/actions/sandbox/skill-install.test.ts +++ b/src/lib/actions/sandbox/skill-install.test.ts @@ -18,6 +18,7 @@ const skillInstall = vi.hoisted(() => ({ parseFrontmatter: vi.fn(), collectFiles: vi.fn(), uploadDirectory: vi.fn(), + installFreshSharedSkill: vi.fn(), postInstall: vi.fn(), verifyInstall: vi.fn(), })); @@ -39,13 +40,27 @@ vi.mock("./gateway-state", () => ({ import { installSandboxSkill, removeSandboxSkill } from "./skill-install"; const paths = { + stateDir: "/sandbox/.openclaw", uploadDir: "/sandbox/.openclaw/skills/demo-skill", mirrorDir: "$HOME/.openclaw/skills/demo-skill", + uploadDirSharedWithAgent: false, sessionFile: "/sandbox/.openclaw/agents/main/sessions/sessions.json", isOpenClaw: true, }; const agent = { name: "openclaw", configPaths: { dir: "/sandbox/.openclaw" } }; +const deepAgent = { + name: "langchain-deepagents-code", + configPaths: { dir: "/sandbox/.deepagents" }, +}; +const sharedPaths = { + stateDir: "/sandbox/.deepagents", + uploadDir: "/sandbox/.deepagents/agent/skills/demo-skill", + mirrorDir: null, + uploadDirSharedWithAgent: true, + sessionFile: null, + isOpenClaw: false, +}; function makeSkillDir(): string { const dir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-action-skill-")); @@ -92,6 +107,7 @@ describe("sandbox skill action orchestration", () => { files: ["SKILL.md"], skippedDotfiles: [], unsafePaths: [], + unsupportedPaths: [], }); skillInstall.uploadDirectory.mockReturnValue({ uploaded: 1, @@ -99,6 +115,11 @@ describe("sandbox skill action orchestration", () => { skippedDotfiles: [], unsafePaths: [], }); + skillInstall.installFreshSharedSkill.mockReturnValue({ + success: true, + uploaded: 1, + contentDigest: "a".repeat(64), + }); skillInstall.postInstall.mockReturnValue({ success: true, messages: [] }); skillInstall.verifyInstall.mockReturnValue(true); }); @@ -193,6 +214,22 @@ describe("sandbox skill action orchestration", () => { expect(process.exitCode).toBeUndefined(); }); + it("refuses Deep Agents removal before obtaining SSH configuration", async () => { + getSessionAgent.mockReturnValue(deepAgent); + skillInstall.resolveSkillPaths.mockReturnValue(sharedPaths); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + + await removeSandboxSkill("alpha", { name: "demo-skill" }); + + expect(process.exitCode).toBe(1); + expect(error).toHaveBeenCalledWith( + expect.stringContaining("Automatic removal is unavailable for Deep Agents skills"), + ); + expect(captureSandboxSshConfig).not.toHaveBeenCalled(); + expect(skillInstall.checkExisting).not.toHaveBeenCalled(); + expect(skillInstall.removeSkill).not.toHaveBeenCalled(); + }); + it("stops skill installation at the shared gateway liveness guard (#2276)", async () => { const skillDir = makeSkillDir(); ensureLiveSandboxOrExit.mockRejectedValueOnce(new Error("wrong gateway active")); @@ -211,6 +248,63 @@ describe("sandbox skill action orchestration", () => { expect(skillInstall.uploadDirectory).not.toHaveBeenCalled(); }); + it("refuses a SKILL.md symlink before parsing or contacting the sandbox", async () => { + const skillDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-action-skill-link-")); + const target = path.join(skillDir, "target.md"); + fs.writeFileSync(target, "---\nname: demo-skill\n---\n# Demo\n"); + fs.symlinkSync(target, path.join(skillDir, "SKILL.md")); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const exit = vi.spyOn(process, "exit").mockImplementation(((code?: string | number | null) => { + throw new Error(`process.exit ${code}`); + }) as typeof process.exit); + + try { + await expect( + installSandboxSkill("alpha", { command: "install", path: skillDir }), + ).rejects.toThrow("process.exit 1"); + } finally { + fs.rmSync(skillDir, { recursive: true, force: true }); + } + + expect(error).toHaveBeenCalledWith(expect.stringContaining("must be a regular file")); + expect(skillInstall.parseFrontmatter).not.toHaveBeenCalled(); + expect(captureSandboxSshConfig).not.toHaveBeenCalled(); + expect(exit).toHaveBeenCalledWith(1); + }); + + it("fails closed when SKILL.md is replaced between path validation and descriptor open", async () => { + const skillDir = makeSkillDir(); + const skillMdPath = path.join(skillDir, "SKILL.md"); + const replacement = path.join(skillDir, "replacement.md"); + fs.writeFileSync(replacement, "---\nname: attacker\n---\n# Replacement\n"); + let openedFlags = 0; + vi.spyOn(fs, "openSync").mockImplementationOnce((_candidatePath, flags) => { + openedFlags = flags as number; + fs.rmSync(skillMdPath); + fs.symlinkSync(replacement, skillMdPath); + throw Object.assign(new Error("symbolic link refused"), { code: "ELOOP" }); + }); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + const exit = vi.spyOn(process, "exit").mockImplementation(((code?: string | number | null) => { + throw new Error(`process.exit ${code}`); + }) as typeof process.exit); + + try { + await expect( + installSandboxSkill("alpha", { command: "install", path: skillDir }), + ).rejects.toThrow("process.exit 1"); + } finally { + fs.rmSync(skillDir, { recursive: true, force: true }); + } + + expect(openedFlags & fs.constants.O_NOFOLLOW).toBe(fs.constants.O_NOFOLLOW); + expect(openedFlags & fs.constants.O_NONBLOCK).toBe(fs.constants.O_NONBLOCK); + expect(error).toHaveBeenCalledWith(expect.stringContaining("must be a regular file")); + expect(skillInstall.parseFrontmatter).not.toHaveBeenCalled(); + expect(captureSandboxSshConfig).not.toHaveBeenCalled(); + expect(exit).toHaveBeenCalledWith(1); + }); + it("continues skill install when the existence probe is unknown because upload plus verify are authoritative", async () => { const skillDir = makeSkillDir(); let tempConfig = ""; @@ -247,6 +341,93 @@ describe("sandbox skill action orchestration", () => { expect(process.exitCode).toBeUndefined(); }); + it("fresh-installs and verifies Deep Agents content without generic upload mutation", async () => { + const skillDir = makeSkillDir(); + getSessionAgent.mockReturnValue(deepAgent); + skillInstall.resolveSkillPaths.mockReturnValue(sharedPaths); + let tempConfig = ""; + skillInstall.installFreshSharedSkill.mockImplementation((ctx) => { + tempConfig = ctx.configFile; + return { success: true, uploaded: 1, contentDigest: "a".repeat(64) }; + }); + const log = vi.spyOn(console, "log").mockImplementation(() => undefined); + + try { + await installSandboxSkill("alpha", { command: "install", path: skillDir }); + } finally { + fs.rmSync(skillDir, { recursive: true, force: true }); + } + + expect(skillInstall.installFreshSharedSkill).toHaveBeenCalledWith( + expect.objectContaining({ configFile: tempConfig, sandboxName: "alpha" }), + skillDir, + sharedPaths, + ); + expect(skillInstall.checkExisting).not.toHaveBeenCalled(); + expect(skillInstall.uploadDirectory).not.toHaveBeenCalled(); + expect(skillInstall.postInstall).not.toHaveBeenCalled(); + expect(log).toHaveBeenCalledWith(expect.stringContaining("Skill 'demo-skill' installed")); + expect(log).toHaveBeenCalledWith( + expect.stringContaining("Start a new Deep Agents session to load the skill."), + ); + expectTempSshConfigCleanedUp(tempConfig); + expect(process.exitCode).toBeUndefined(); + }); + + it("refuses a colliding Deep Agents destination without generic mutation", async () => { + const skillDir = makeSkillDir(); + getSessionAgent.mockReturnValue(deepAgent); + skillInstall.resolveSkillPaths.mockReturnValue(sharedPaths); + skillInstall.installFreshSharedSkill.mockReturnValue({ + success: false, + uploaded: 0, + reason: "destination_exists", + }); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + + try { + await installSandboxSkill("alpha", { command: "install", path: skillDir }); + } finally { + fs.rmSync(skillDir, { recursive: true, force: true }); + } + + expect(process.exitCode).toBe(1); + expect(error).toHaveBeenCalledWith( + expect.stringContaining("the Deep Agents skill destination already exists"), + ); + expect(skillInstall.checkExisting).not.toHaveBeenCalled(); + expect(skillInstall.uploadDirectory).not.toHaveBeenCalled(); + expect(skillInstall.postInstall).not.toHaveBeenCalled(); + }); + + it("reports unknown Deep Agents commit state with inspect-before-retry guidance", async () => { + const skillDir = makeSkillDir(); + getSessionAgent.mockReturnValue(deepAgent); + skillInstall.resolveSkillPaths.mockReturnValue(sharedPaths); + skillInstall.installFreshSharedSkill.mockReturnValue({ + success: false, + uploaded: 0, + reason: "remote_state_unknown", + }); + const error = vi.spyOn(console, "error").mockImplementation(() => undefined); + + try { + await installSandboxSkill("alpha", { command: "install", path: skillDir }); + } finally { + fs.rmSync(skillDir, { recursive: true, force: true }); + } + + const output = error.mock.calls.map((args) => args.join(" ")).join("\n"); + expect(process.exitCode).toBe(1); + expect(output).toContain("did not confirm whether the Deep Agents skill was committed"); + expect(output).toContain( + "Inspect /sandbox/.deepagents/agent/skills/demo-skill before retrying", + ); + expect(skillInstall.checkExisting).not.toHaveBeenCalled(); + expect(skillInstall.uploadDirectory).not.toHaveBeenCalled(); + expect(skillInstall.postInstall).not.toHaveBeenCalled(); + }); + it("adds shields recovery guidance when skill upload fails (#6859)", async () => { const skillDir = makeSkillDir(); skillInstall.uploadDirectory.mockReturnValue({ diff --git a/src/lib/actions/sandbox/skill-install.ts b/src/lib/actions/sandbox/skill-install.ts index c92509e0196..9968254d43c 100644 --- a/src/lib/actions/sandbox/skill-install.ts +++ b/src/lib/actions/sandbox/skill-install.ts @@ -72,6 +72,43 @@ export type SkillRemoveRequest = { extraArgs?: string[]; }; +function lstatOrNull(candidatePath: string): fs.Stats | null { + try { + return fs.lstatSync(candidatePath); + } catch { + return null; + } +} + +type RegularFileRead = + | { content: string; success: true } + | { reason: "invalid" | "missing"; success: false }; + +function readRegularFileNoFollow(candidatePath: string): RegularFileRead { + const noFollow = fs.constants.O_NOFOLLOW; + const nonblock = fs.constants.O_NONBLOCK; + if (typeof noFollow !== "number" || typeof nonblock !== "number") { + return { reason: "invalid", success: false }; + } + + let descriptor: number | undefined; + try { + descriptor = fs.openSync(candidatePath, fs.constants.O_RDONLY | noFollow | nonblock); + if (!fs.fstatSync(descriptor).isFile()) return { reason: "invalid", success: false }; + return { content: fs.readFileSync(descriptor, "utf8"), success: true }; + } catch (error) { + return { + reason: + error instanceof Error && "code" in error && error.code === "ENOENT" + ? "missing" + : "invalid", + success: false, + }; + } finally { + if (descriptor !== undefined) fs.closeSync(descriptor); + } +} + export function printPluginInstallHint(): void { console.error(" This looks like an OpenClaw plugin, not a SKILL.md agent skill."); console.error(" `skill install` only accepts skill directories or direct SKILL.md paths."); @@ -122,6 +159,16 @@ export async function removeSandboxSkill( const agent = agentRuntime.getSessionAgent(sandboxName); const paths = skillInstall.resolveSkillPaths(agent, skillName); + if (paths.uploadDirSharedWithAgent) { + console.error( + " Automatic removal is unavailable for Deep Agents skills because the destination is shared with agent-authored content.", + ); + console.error( + " Inspect and remove the skill with the agent's native or manual workflow after confirming ownership.", + ); + process.exitCode = 1; + return; + } const sshConfigResult = captureSandboxSshConfig(sandboxName, { ignoreError: true, @@ -221,14 +268,19 @@ export async function installSandboxSkill( } const resolvedPath = path.resolve(skillPath); + const resolvedStat = lstatOrNull(resolvedPath); + if (resolvedStat?.isSymbolicLink()) { + console.error(` Skill path '${resolvedPath}' must not be a symbolic link.`); + process.exit(1); + } // Accept a directory containing SKILL.md, or a direct path to SKILL.md. let skillDir: string; let skillMdPath: string; - if (fs.existsSync(resolvedPath) && fs.statSync(resolvedPath).isDirectory()) { + if (resolvedStat?.isDirectory()) { skillDir = resolvedPath; skillMdPath = path.join(resolvedPath, "SKILL.md"); - } else if (fs.existsSync(resolvedPath) && resolvedPath.endsWith("SKILL.md")) { + } else if (resolvedStat?.isFile() && resolvedPath.endsWith("SKILL.md")) { skillDir = path.dirname(resolvedPath); skillMdPath = resolvedPath; } else { @@ -240,7 +292,8 @@ export async function installSandboxSkill( process.exit(1); } - if (!fs.existsSync(skillMdPath)) { + const skillMdRead = readRegularFileNoFollow(skillMdPath); + if (!skillMdRead.success && skillMdRead.reason === "missing") { console.error(` No SKILL.md found in '${skillDir}'.`); console.error(" The skill directory must contain a SKILL.md file."); if (looksLikeOpenClawPlugin(skillDir)) { @@ -248,12 +301,15 @@ export async function installSandboxSkill( } process.exit(1); } + if (!skillMdRead.success) { + console.error(` SKILL.md at '${skillMdPath}' must be a regular file, not a symbolic link.`); + process.exit(1); + } // 1. Validate frontmatter let frontmatter; try { - const content = fs.readFileSync(skillMdPath, "utf-8"); - frontmatter = skillInstall.parseFrontmatter(content); + frontmatter = skillInstall.parseFrontmatter(skillMdRead.content); } catch (err) { const errorMessage = err instanceof Error ? err.message : String(err); console.error(` ${errorMessage}`); @@ -267,6 +323,12 @@ export async function installSandboxSkill( console.error(" File names must match [A-Za-z0-9._-/]. Rename or remove them."); process.exit(1); } + if (collected.unsupportedPaths.length > 0) { + console.error(" Skill directory contains unsupported non-regular paths:"); + for (const p of collected.unsupportedPaths) console.error(` ${p}`); + console.error(" Skills may contain only regular files and directories."); + process.exit(1); + } if (collected.skippedDotfiles.length > 0) { console.log( ` ${D}Skipping ${collected.skippedDotfiles.length} hidden path(s): ${collected.skippedDotfiles.join(", ")}${R}`, @@ -297,6 +359,35 @@ export async function installSandboxSkill( try { const ctx = { configFile: tmpSshConfig.file, sandboxName }; + if (paths.uploadDirSharedWithAgent) { + const fresh = skillInstall.installFreshSharedSkill(ctx, skillDir, paths); + if (!fresh.success || !fresh.contentDigest) { + if (fresh.reason === "destination_exists") { + console.error( + ` Refusing to replace '${frontmatter.name}': the Deep Agents skill destination already exists.`, + ); + console.error( + " Deep Agents skill install supports fresh names only because that directory also contains agent-authored skills.", + ); + } else if (fresh.reason === "snapshot_failed") { + console.error(" Failed to create an exact regular-file snapshot of the local skill."); + } else { + console.error( + " The remote install did not confirm whether the Deep Agents skill was committed.", + ); + console.error( + ` Inspect ${paths.uploadDir} before retrying; NemoClaw will not replace or delete shared agent content.`, + ); + } + process.exitCode = 1; + return; + } + console.log(` ${G}✓${R} Installed ${fresh.uploaded} file(s) into the agent skill directory`); + console.log(` ${G}✓${R} Skill '${frontmatter.name}' installed`); + console.log(` ${D}Start a new Deep Agents session to load the skill.${R}`); + return; + } + // 5. Check if skill already exists (update vs fresh install). This probe is // advisory for install only: stale SSH config files and transient remote // shell startup failures can make the stat probe inconclusive even when a @@ -342,7 +433,7 @@ export async function installSandboxSkill( } else { console.error( ` Skill uploaded but verification failed: SKILL.md missing at ${paths.uploadDir}` + - (paths.isOpenClaw && paths.mirrorDir ? ` or its agent mirror ${paths.mirrorDir}` : ""), + (paths.mirrorDir ? ` or its agent mirror ${paths.mirrorDir}` : ""), ); process.exit(1); } diff --git a/src/lib/skill-install-shared.test.ts b/src/lib/skill-install-shared.test.ts new file mode 100644 index 00000000000..57a383789b3 --- /dev/null +++ b/src/lib/skill-install-shared.test.ts @@ -0,0 +1,325 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +import { spawnSync } from "node:child_process"; +import { + chmodSync, + existsSync, + lstatSync, + mkdirSync, + mkdtempSync, + readdirSync, + readFileSync, + rmSync, + symlinkSync, + writeFileSync, +} from "node:fs"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { describe, expect, it } from "vitest"; + +import { + computeSkillContentDigest, + installFreshSharedSkill, + resolveSkillPaths, +} from "./skill-install"; +import type { SshResult } from "./skill-remote"; + +const CTX = { configFile: "/tmp/ssh-config", sandboxName: "alpha" }; +const AGENT_NAME = "langchain-deepagents-code"; + +function makeSkill(): string { + const dir = mkdtempSync(join(tmpdir(), "nemoclaw-shared-skill-")); + writeFileSync(join(dir, "SKILL.md"), "---\nname: note-summarizer\n---\n# Notes\n"); + mkdirSync(join(dir, "scripts")); + writeFileSync(join(dir, "scripts", "summarize.js"), "export default 'exact';\n"); + chmodSync(join(dir, "scripts", "summarize.js"), 0o755); + return dir; +} + +function pathsFor(stateDir: string) { + return resolveSkillPaths({ name: AGENT_NAME, configPaths: { dir: stateDir } }, "note-summarizer"); +} + +const COLLISION_CASES = [ + { + kind: "file", + prepare: (destination: string, _outside: string) => writeFileSync(destination, "agent file\n"), + assertUnchanged: (destination: string) => + expect(readFileSync(destination, "utf8")).toBe("agent file\n"), + }, + { + kind: "directory", + prepare: (destination: string, _outside: string) => { + mkdirSync(destination); + writeFileSync(join(destination, "agent.txt"), "agent directory\n"); + }, + assertUnchanged: (destination: string) => + expect(readFileSync(join(destination, "agent.txt"), "utf8")).toBe("agent directory\n"), + }, + { + kind: "symlink", + prepare: (destination: string, outside: string) => symlinkSync(outside, destination), + assertUnchanged: (destination: string) => + expect(readFileSync(destination, "utf8")).toBe("outside\n"), + }, +] as const; + +function executeShell( + command: string, + input: string | Buffer | undefined, + env: NodeJS.ProcessEnv = process.env, +): SshResult { + const run = spawnSync("/bin/sh", ["-c", command], { + encoding: "utf8", + env, + input, + }); + return { + status: run.status ?? 1, + stdout: (run.stdout || "").trim(), + stderr: (run.stderr || "").trim(), + }; +} + +describe("fresh shared-agent skill install", () => { + it("streams one host-attested archive to an atomic no-clobber activation", () => { + const skillDir = makeSkill(); + const paths = pathsFor("/sandbox/.deepagents"); + const expected = computeSkillContentDigest(skillDir); + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: (_ctx, _command, opts) => { + expect(Buffer.isBuffer(opts?.input)).toBe(true); + return { status: 0, stdout: `INSTALLED ${expected}`, stderr: "" }; + }, + }); + + expect(result).toEqual({ + success: true, + uploaded: 2, + contentDigest: expected, + }); + expect(paths.uploadDir).toBe("/sandbox/.deepagents/agent/skills/note-summarizer"); + expect(paths.mirrorDir).toBeNull(); + expect(paths.uploadDirSharedWithAgent).toBe(true); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + } + }); + + it("includes normalized executable modes in the immutable content digest", () => { + const skillDir = makeSkill(); + try { + const executableDigest = computeSkillContentDigest(skillDir); + chmodSync(join(skillDir, "scripts", "summarize.js"), 0o644); + + expect(computeSkillContentDigest(skillDir)).not.toBe(executableDigest); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + } + }); + + it("rejects a snapshot whose SKILL.md name does not match the resolved destination", () => { + const skillDir = makeSkill(); + const paths = pathsFor("/sandbox/.deepagents"); + writeFileSync(join(skillDir, "SKILL.md"), "---\nname: different-skill\n---\n# Notes\n"); + let called = false; + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: () => { + called = true; + return { status: 0, stdout: "", stderr: "" }; + }, + }); + + expect(result).toEqual({ + success: false, + uploaded: 0, + reason: "snapshot_failed", + }); + expect(called).toBe(false); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + } + }); + + it("rejects a snapshot that does not contain a regular SKILL.md", () => { + const skillDir = makeSkill(); + const paths = pathsFor("/sandbox/.deepagents"); + rmSync(join(skillDir, "SKILL.md")); + let called = false; + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: () => { + called = true; + return { status: 0, stdout: "", stderr: "" }; + }, + }); + + expect(result).toEqual({ + success: false, + uploaded: 0, + reason: "snapshot_failed", + }); + expect(called).toBe(false); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + } + }); + + it.runIf(process.platform === "linux")( + "installs exact bytes directly and leaves the legacy upload path untouched", + () => { + const skillDir = makeSkill(); + const stateDir = mkdtempSync(join(tmpdir(), "nemoclaw-shared-state-")); + const paths = pathsFor(stateDir); + const legacy = join(stateDir, "skills", "note-summarizer"); + mkdirSync(legacy, { recursive: true }); + writeFileSync(join(legacy, "legacy.txt"), "preserve me\n"); + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: (_ctx, command, opts) => executeShell(command, opts?.input), + }); + + expect(result.success).toBe(true); + expect(readFileSync(join(paths.uploadDir, "SKILL.md"), "utf8")).toContain("# Notes"); + expect(readFileSync(join(paths.uploadDir, "scripts", "summarize.js"), "utf8")).toBe( + "export default 'exact';\n", + ); + expect(lstatSync(join(paths.uploadDir, "SKILL.md")).mode & 0o777).toBe(0o644); + expect(lstatSync(join(paths.uploadDir, "scripts", "summarize.js")).mode & 0o777).toBe( + 0o755, + ); + expect(readFileSync(join(legacy, "legacy.txt"), "utf8")).toBe("preserve me\n"); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + rmSync(stateDir, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform === "linux").each(COLLISION_CASES)( + "refuses an existing $kind without changing it (#7634)", + ({ kind, prepare, assertUnchanged }) => { + const skillDir = makeSkill(); + const stateDir = mkdtempSync(join(tmpdir(), `nemoclaw-shared-${kind}-`)); + const paths = pathsFor(stateDir); + mkdirSync(join(stateDir, "agent", "skills"), { recursive: true }); + const outside = join(stateDir, "outside"); + writeFileSync(outside, "outside\n"); + prepare(paths.uploadDir, outside); + const before = lstatSync(paths.uploadDir); + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: (_ctx, command, opts) => executeShell(command, opts?.input), + }); + + expect(result).toEqual({ + success: false, + uploaded: 0, + reason: "destination_exists", + }); + expect(lstatSync(paths.uploadDir).isSymbolicLink()).toBe(before.isSymbolicLink()); + assertUnchanged(paths.uploadDir); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + rmSync(stateDir, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform === "linux")( + "detects a no-clobber race when mv reports success but leaves staging", + () => { + const skillDir = makeSkill(); + const stateDir = mkdtempSync(join(tmpdir(), "nemoclaw-shared-race-")); + const fakeBin = mkdtempSync(join(tmpdir(), "nemoclaw-fake-mv-")); + const paths = pathsFor(stateDir); + const fakeMv = join(fakeBin, "mv"); + writeFileSync(fakeMv, '#!/bin/sh\nmkdir -- "$RACE_DEST"\nexec /usr/bin/mv "$@"\n'); + chmodSync(fakeMv, 0o755); + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: (_ctx, command, opts) => + executeShell(command, opts?.input, { + ...process.env, + PATH: `${fakeBin}:${process.env.PATH}`, + RACE_DEST: "note-summarizer", + }), + }); + + expect(result.reason).toBe("destination_exists"); + expect(existsSync(paths.uploadDir)).toBe(true); + expect(readdirSync(paths.uploadDir)).toEqual([]); + expect( + readdirSync(join(stateDir, "agent", "skills")).some((name) => + name.startsWith(".nemoclaw-skill."), + ), + ).toBe(false); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + rmSync(stateDir, { recursive: true, force: true }); + rmSync(fakeBin, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform === "linux")( + "fails closed on a corrupt archive and leaves no active destination", + () => { + const skillDir = makeSkill(); + const stateDir = mkdtempSync(join(tmpdir(), "nemoclaw-shared-corrupt-")); + const paths = pathsFor(stateDir); + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: (_ctx, command) => executeShell(command, Buffer.from("not a tar archive")), + }); + + expect(result.reason).toBe("remote_state_unknown"); + expect(existsSync(paths.uploadDir)).toBe(false); + expect( + readdirSync(join(stateDir, "agent", "skills")).some((name) => + name.startsWith(".nemoclaw-skill."), + ), + ).toBe(false); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + rmSync(stateDir, { recursive: true, force: true }); + } + }, + ); + + it.runIf(process.platform === "linux")( + "classifies a non-collision move failure as unknown remote state", + () => { + const skillDir = makeSkill(); + const stateDir = mkdtempSync(join(tmpdir(), "nemoclaw-shared-move-failure-")); + const fakeBin = mkdtempSync(join(tmpdir(), "nemoclaw-failing-mv-")); + const paths = pathsFor(stateDir); + const fakeMv = join(fakeBin, "mv"); + writeFileSync(fakeMv, "#!/bin/sh\nexit 1\n"); + chmodSync(fakeMv, 0o755); + try { + const result = installFreshSharedSkill(CTX, skillDir, paths, { + sshExecImpl: (_ctx, command, opts) => + executeShell(command, opts?.input, { + ...process.env, + PATH: `${fakeBin}:${process.env.PATH}`, + }), + }); + + expect(result).toEqual({ + success: false, + uploaded: 0, + reason: "remote_state_unknown", + }); + expect(existsSync(paths.uploadDir)).toBe(false); + } finally { + rmSync(skillDir, { recursive: true, force: true }); + rmSync(stateDir, { recursive: true, force: true }); + rmSync(fakeBin, { recursive: true, force: true }); + } + }, + ); +}); diff --git a/src/lib/skill-install.test.ts b/src/lib/skill-install.test.ts index d3d1438327b..3a6a1585359 100644 --- a/src/lib/skill-install.test.ts +++ b/src/lib/skill-install.test.ts @@ -1,7 +1,7 @@ // SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. // SPDX-License-Identifier: Apache-2.0 -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { describe, expect, it } from "vitest"; @@ -196,13 +196,27 @@ describe("collectFiles", () => { cleanup(); } }); + + it("rejects visible symlinks instead of following their targets", () => { + setup({ "SKILL.md": "---\nname: linked\n---\n" }); + try { + symlinkSync("SKILL.md", join(tmpDir, "alias.md")); + const { files, unsupportedPaths } = collectFiles(tmpDir); + expect(files).toEqual(["SKILL.md"]); + expect(unsupportedPaths).toEqual(["alias.md"]); + } finally { + cleanup(); + } + }); }); describe("resolveSkillPaths", () => { it("returns OpenClaw defaults when agent is null", () => { const paths = resolveSkillPaths(null, "weather"); + expect(paths.stateDir).toBe("/sandbox/.openclaw"); expect(paths.uploadDir).toBe("/sandbox/.openclaw/skills/weather"); expect(paths.mirrorDir).toBe("$HOME/.openclaw/skills/weather"); + expect(paths.uploadDirSharedWithAgent).toBe(false); expect(paths.sessionFile).toBe("/sandbox/.openclaw/agents/main/sessions/sessions.json"); expect(paths.isOpenClaw).toBe(true); }); @@ -215,8 +229,10 @@ describe("resolveSkillPaths", () => { }, }; const paths = resolveSkillPaths(agent, "my-skill"); + expect(paths.stateDir).toBe("/sandbox/.openclaw"); expect(paths.uploadDir).toBe("/sandbox/.openclaw/skills/my-skill"); expect(paths.mirrorDir).toBe("$HOME/.openclaw/skills/my-skill"); + expect(paths.uploadDirSharedWithAgent).toBe(false); expect(paths.sessionFile).toBe("/sandbox/.openclaw/agents/main/sessions/sessions.json"); expect(paths.isOpenClaw).toBe(true); }); @@ -229,8 +245,29 @@ describe("resolveSkillPaths", () => { }, }; const paths = resolveSkillPaths(agent, "demo-skill"); + expect(paths.stateDir).toBe("/sandbox/.hermes"); expect(paths.uploadDir).toBe("/sandbox/.hermes/skills/demo-skill"); expect(paths.mirrorDir).toBeNull(); + expect(paths.uploadDirSharedWithAgent).toBe(false); + expect(paths.sessionFile).toBeNull(); + expect(paths.isOpenClaw).toBe(false); + }); + + it("installs Deep Agents skills directly into the agent skills dir dcode loads (#7634)", () => { + // dcode's user skill dir is ~/.deepagents/{agent}/skills (HOME=/sandbox, + // DEFAULT_AGENT_NAME="agent"); it never scans ~/.deepagents/skills, so the + // upload dir alone leaves the skill installed but unloadable. + const agent = { + name: "langchain-deepagents-code", + configPaths: { + dir: "/sandbox/.deepagents", + }, + }; + const paths = resolveSkillPaths(agent, "note-summarizer"); + expect(paths.stateDir).toBe("/sandbox/.deepagents"); + expect(paths.uploadDir).toBe("/sandbox/.deepagents/agent/skills/note-summarizer"); + expect(paths.mirrorDir).toBeNull(); + expect(paths.uploadDirSharedWithAgent).toBe(true); expect(paths.sessionFile).toBeNull(); expect(paths.isOpenClaw).toBe(false); }); @@ -243,8 +280,10 @@ describe("resolveSkillPaths", () => { }, }; const paths = resolveSkillPaths(agent, "test-skill"); + expect(paths.stateDir).toBe("/sandbox/.future"); expect(paths.uploadDir).toBe("/sandbox/.future/skills/test-skill"); expect(paths.mirrorDir).toBeNull(); + expect(paths.uploadDirSharedWithAgent).toBe(false); expect(paths.sessionFile).toBeNull(); expect(paths.isOpenClaw).toBe(false); }); diff --git a/src/lib/skill-install.ts b/src/lib/skill-install.ts index ffdcaf06e70..a7128783429 100644 --- a/src/lib/skill-install.ts +++ b/src/lib/skill-install.ts @@ -8,7 +8,10 @@ // OpenClaw). Non-OpenClaw agents get a "restart gateway" hint until a // generic refresh contract is defined in the manifest schema. +import { spawnSync } from "node:child_process"; +import { createHash } from "node:crypto"; import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; // yaml is a production dependency (used by policies.ts, onboard.ts) @@ -94,16 +97,50 @@ export function parseFrontmatter(content: string): SkillFrontmatter { // ── Path resolution ────────────────────────────────────────────── export interface SkillPaths { + /** Agent state root that contains the resolved skill paths. */ + stateDir: string; /** Upload target directory for the skill */ uploadDir: string; - /** OpenClaw-only mirror directory under the remote home dir, or null */ + /** Directory the agent actually loads skills from, or null when it equals uploadDir */ mirrorDir: string | null; + /** + * Whether the agent's own tooling also writes into uploadDir. Shared + * destinations support only atomic fresh installs because their existing + * content is not proof that NemoClaw owns it. + */ + uploadDirSharedWithAgent: boolean; /** OpenClaw-only: session index to clear, or null */ sessionFile: string | null; /** Whether the agent is OpenClaw (drives refresh behavior) */ isOpenClaw: boolean; } +/** + * Agents whose loader reads skills from somewhere other than `uploadDir`. + * + * NemoClaw uploads to `uploadDir`, which the agent manifest declares as durable + * `state_dirs`, but the agent scans its own directory at session start. Where + * those diverge, an upload without a mirror leaves the skill on disk and + * unregistered — absent from the agent's skill list (#4819 for OpenClaw, #7634 + * for Deep Agents Code, whose user skill dir is `~/.deepagents/{agent}/skills`). + * + * Remove an entry when its agent starts loading skills from `uploadDir`. + */ +const AGENT_SKILL_MIRRORS: Record string> = { + openclaw: (_dir, skillName) => `$HOME/.openclaw/skills/${skillName}`, +}; + +/** + * Agent-owned skill directories that are also the loader's canonical source. + * + * Deep Agents Code's built-in skill creator writes directly to + * `agent/skills` (#5753). NemoClaw therefore installs there only when the + * destination is absent and never treats an existing directory as managed. + */ +const AGENT_SHARED_SKILL_DIRS: Record string> = { + "langchain-deepagents-code": (dir, skillName) => `${dir}/agent/skills/${skillName}`, +}; + /** * Resolve skill install paths from the agent definition. * Uses a single directory for skill uploads (no immutable/writable split). @@ -117,12 +154,15 @@ export function resolveSkillPaths( const isOpenClaw = !agent || agent.name === "openclaw"; const dir = agent ? agent.configPaths.dir : "/sandbox/.openclaw"; - - const uploadDir = `${dir}/skills/${skillName}`; + const agentName = agent ? agent.name : "openclaw"; + const sharedDir = AGENT_SHARED_SKILL_DIRS[agentName]; + const mirror = AGENT_SKILL_MIRRORS[agentName]; return { - uploadDir, - mirrorDir: isOpenClaw ? `$HOME/.openclaw/skills/${skillName}` : null, + stateDir: dir, + uploadDir: sharedDir ? sharedDir(dir, skillName) : `${dir}/skills/${skillName}`, + mirrorDir: mirror ? mirror(dir, skillName) : null, + uploadDirSharedWithAgent: Boolean(sharedDir), sessionFile: isOpenClaw ? `${dir}/agents/main/sessions/sessions.json` : null, isOpenClaw, }; @@ -165,6 +205,7 @@ export interface CollectedFiles { files: string[]; skippedDotfiles: string[]; unsafePaths: string[]; + unsupportedPaths: string[]; } /** @@ -177,6 +218,7 @@ export function collectFiles(dir: string): CollectedFiles { const files: string[] = []; const skippedDotfiles: string[] = []; const unsafePaths: string[] = []; + const unsupportedPaths: string[] = []; function walk(current: string, prefix: string) { for (const entry of fs.readdirSync(current, { withFileTypes: true })) { @@ -193,11 +235,19 @@ export function collectFiles(dir: string): CollectedFiles { } else { files.push(rel); } + } else { + // Never follow symlinks or copy sockets, FIFOs, or device nodes across + // the host-to-sandbox trust boundary. + unsupportedPaths.push(rel); } } } walk(dir, ""); - return { files, skippedDotfiles, unsafePaths }; + files.sort(); + skippedDotfiles.sort(); + unsafePaths.sort(); + unsupportedPaths.sort(); + return { files, skippedDotfiles, unsafePaths, unsupportedPaths }; } /** @@ -209,9 +259,10 @@ export function uploadDirectory( localDir: string, remoteDir: string, ): { uploaded: number; failed: string[]; skippedDotfiles: string[]; unsafePaths: string[] } { - const { files, skippedDotfiles, unsafePaths } = collectFiles(localDir); - if (unsafePaths.length > 0) { - return { uploaded: 0, failed: unsafePaths, skippedDotfiles, unsafePaths }; + const { files, skippedDotfiles, unsafePaths, unsupportedPaths } = collectFiles(localDir); + const rejected = [...unsafePaths, ...unsupportedPaths]; + if (rejected.length > 0) { + return { uploaded: 0, failed: rejected, skippedDotfiles, unsafePaths }; } const failed: string[] = []; for (const rel of files) { @@ -225,9 +276,218 @@ export function uploadDirectory( return { uploaded: files.length - failed.length, failed, skippedDotfiles, unsafePaths }; } +const SHA256_RE = /^[a-f0-9]{64}$/; +const SKILL_SNAPSHOT_TIMEOUT_MS = 30_000; + +function fileSha256(filePath: string): string { + return createHash("sha256").update(fs.readFileSync(filePath)).digest("hex"); +} + +function normalizedSkillFileMode(filePath: string): "644" | "755" { + return (fs.lstatSync(filePath).mode & 0o111) === 0 ? "644" : "755"; +} + +/** Hash the sorted regular-file path, normalized mode, and byte set used by a skill archive. */ +export function computeSkillContentDigest(localDir: string, files?: string[]): string { + const selected = files ?? collectFiles(localDir).files; + const manifest = selected + .slice() + .sort() + .map((rel) => { + const filePath = path.join(localDir, rel); + return `${normalizedSkillFileMode(filePath)} ${fileSha256(filePath)} ${rel}\n`; + }) + .join(""); + return createHash("sha256").update(manifest).digest("hex"); +} + +interface SkillArchiveSnapshot { + archive: Buffer; + contentDigest: string; + files: string[]; + skillName: string; +} + /** - * Run post-install steps: session refresh for OpenClaw, or - * non-OpenClaw restart hint. + * Create one immutable archive, then derive the expected manifest from that + * archive rather than re-reading the mutable source tree. + */ +function createSkillArchiveSnapshot( + localDir: string, + files: string[], +): SkillArchiveSnapshot | null { + const archiveResult = spawnSync("tar", ["-cf", "-", "-C", localDir, "--", ...files], { + encoding: null, + env: { ...process.env, COPYFILE_DISABLE: "1" }, + maxBuffer: 256 * 1024 * 1024, + timeout: SKILL_SNAPSHOT_TIMEOUT_MS, + }); + if (archiveResult.status !== 0 || !Buffer.isBuffer(archiveResult.stdout)) return null; + + const snapshotDir = fs.mkdtempSync(path.join(os.tmpdir(), "nemoclaw-skill-snapshot-")); + try { + const extractResult = spawnSync("tar", ["-xf", "-", "-C", snapshotDir], { + encoding: null, + input: archiveResult.stdout, + maxBuffer: 16 * 1024 * 1024, + timeout: SKILL_SNAPSHOT_TIMEOUT_MS, + }); + if (extractResult.status !== 0) return null; + const snapshot = collectFiles(snapshotDir); + if ( + snapshot.unsafePaths.length > 0 || + snapshot.unsupportedPaths.length > 0 || + !snapshot.files.includes("SKILL.md") || + snapshot.files.join("\n") !== files.slice().sort().join("\n") + ) { + return null; + } + let skillName: string; + try { + skillName = parseFrontmatter( + fs.readFileSync(path.join(snapshotDir, "SKILL.md"), "utf8"), + ).name; + } catch { + return null; + } + return { + archive: archiveResult.stdout, + contentDigest: computeSkillContentDigest(snapshotDir, snapshot.files), + files: snapshot.files, + skillName, + }; + } finally { + fs.rmSync(snapshotDir, { recursive: true, force: true }); + } +} + +function buildFreshSharedInstallScript(paths: SkillPaths, expectedDigest: string): string { + const relativeUpload = path.posix.relative(paths.stateDir, paths.uploadDir); + const parentParts = path.posix.dirname(relativeUpload).split("/"); + const leaf = path.posix.basename(relativeUpload); + if ( + !validateRelativePath(relativeUpload) || + parentParts.some((part) => !validateRelativePath(part)) || + !validateRelativePath(leaf) + ) { + throw new Error("Shared agent skill path is not a safe relative destination"); + } + + const lines = [ + "set -eu", + `root=${shellQuote(paths.stateDir)}`, + `leaf=${shellQuote(leaf)}`, + `expected=${shellQuote(expectedDigest)}`, + 'exists() { [ -e "$1" ] || [ -L "$1" ]; }', + 'safe_rel() { case "$1" in ""|/*|*//*|*[!A-Za-z0-9._/-]*) return 1 ;; esac; case "/$1/" in *"/./"*|*"/../"*) return 1 ;; esac; }', + '[ -d "$root" ] && [ ! -L "$root" ] && [ "$(realpath -e -- "$root")" = "$root" ]', + 'cd -P -- "$root"', + '[ "$(pwd -P)" = "$root" ]', + ]; + + let expectedParent = paths.stateDir; + for (const part of parentParts) { + expectedParent = `${expectedParent}/${part}`; + lines.push( + `part=${shellQuote(part)}`, + '[ ! -L "$part" ]', + 'if [ ! -e "$part" ]; then mkdir -- "$part"; fi', + '[ -d "$part" ] && [ ! -L "$part" ]', + 'cd -P -- "$part"', + `[ "$(pwd -P)" = ${shellQuote(expectedParent)} ]`, + ); + } + + lines.push( + 'if exists "$leaf"; then echo EXISTS; exit 2; fi', + 'workspace="$(mktemp -d .nemoclaw-skill.XXXXXX)"', + 'chmod 700 "$workspace"', + 'payload="$workspace/payload"', + 'cleanup() { if exists "$workspace"; then rm -rf -- "$workspace"; fi; }', + "trap cleanup EXIT HUP INT TERM", + 'mkdir -- "$payload"', + 'tar --no-same-owner -xf - -C "$payload"', + '[ -z "$(find "$payload" -mindepth 1 ! -type d ! -type f -print -quit)" ]', + 'find "$payload" -type d -exec chmod 755 {} +', + 'find "$payload" -type f -perm /111 -exec chmod 755 {} +', + 'find "$payload" -type f ! -perm /111 -exec chmod 644 {} +', + 'find "$payload" -type f -printf "%P\\n" | LC_ALL=C sort > "$workspace/files"', + ': > "$workspace/manifest"', + 'while IFS= read -r rel; do safe_rel "$rel"; mode="$(stat -c "%a" "$payload/$rel")"; hash="$(sha256sum "$payload/$rel" | cut -d " " -f 1)"; printf "%s %s %s\\n" "$mode" "$hash" "$rel" >> "$workspace/manifest"; done < "$workspace/files"', + 'staged="$(sha256sum "$workspace/manifest" | cut -d " " -f 1)"', + '[ "$staged" = "$expected" ]', + 'if exists "$leaf"; then echo EXISTS; exit 2; fi', + 'if ! mv -nT -- "$payload" "$leaf"; then if exists "$leaf"; then echo EXISTS; exit 2; fi; echo MOVE_FAILED; exit 3; fi', + 'if exists "$payload"; then echo EXISTS; exit 2; fi', + 'printf "INSTALLED %s\\n" "$expected"', + ); + return lines.join("; "); +} + +export interface FreshSharedSkillInstallResult { + success: boolean; + uploaded: number; + contentDigest?: string; + reason?: "destination_exists" | "snapshot_failed" | "remote_state_unknown"; +} + +/** + * Install into an agent-owned loader directory only when the destination is + * absent. Existing content is never renamed, deleted, or replaced. + */ +export function installFreshSharedSkill( + ctx: SshContext, + localDir: string, + paths: SkillPaths, + opts: { sshExecImpl?: typeof sshExec } = {}, +): FreshSharedSkillInstallResult { + if (!paths.uploadDirSharedWithAgent || paths.mirrorDir) { + return { success: false, uploaded: 0, reason: "remote_state_unknown" }; + } + const collected = collectFiles(localDir); + if ( + collected.files.length === 0 || + collected.unsafePaths.length > 0 || + collected.unsupportedPaths.length > 0 + ) { + return { success: false, uploaded: 0, reason: "snapshot_failed" }; + } + const snapshot = createSkillArchiveSnapshot(localDir, collected.files); + if ( + !snapshot || + !SHA256_RE.test(snapshot.contentDigest) || + snapshot.skillName !== path.posix.basename(paths.uploadDir) + ) { + return { success: false, uploaded: 0, reason: "snapshot_failed" }; + } + const runSsh = opts.sshExecImpl ?? sshExec; + const result = runSsh(ctx, buildFreshSharedInstallScript(paths, snapshot.contentDigest), { + input: snapshot.archive, + }); + if ( + result !== null && + result.status === 0 && + result.stdout === `INSTALLED ${snapshot.contentDigest}` + ) { + return { + success: true, + uploaded: snapshot.files.length, + contentDigest: snapshot.contentDigest, + }; + } + return { + success: false, + uploaded: 0, + reason: + result?.status === 2 && result.stdout === "EXISTS" + ? "destination_exists" + : "remote_state_unknown", + }; +} + +/** + * Run post-install steps: skill-load mirror for every agent that needs one, + * session refresh for OpenClaw, and a restart hint when neither applies. */ export function postInstall( ctx: SshContext, @@ -241,44 +501,40 @@ export function postInstall( const messages: string[] = []; const runSsh = opts.sshExecImpl ?? sshExec; - if (paths.isOpenClaw) { - // Mirror the uploaded skill into the agent's home dir - // ($HOME/.openclaw/skills/). The skill is uploaded to the OpenClaw - // state dir (uploadDir), which `openclaw skills list` reads, but the agent - // loads skills from $HOME/.openclaw/skills at session start. On sandboxes - // where the agent's $HOME differs from the state dir these paths diverge, - // so without this mirror the skill is listed but never invoked (#4819). - // `skill remove` already deletes this mirror, so install must create it to - // stay symmetric. The copy is skipped when both paths resolve to the same - // directory (the common case where $HOME is the state dir's parent), so it - // is a safe no-op there. - if (paths.mirrorDir) { - const src = shellQuote(paths.uploadDir); - // mirrorDir contains $HOME, which must expand on the remote shell, so we - // use double quotes (not shellQuote). Safe because skill names are - // restricted to [A-Za-z0-9._-] by parseFrontmatter / the name regex. - const dst = `"${paths.mirrorDir}"`; - const mirrorParent = `"${paths.mirrorDir.slice(0, paths.mirrorDir.lastIndexOf("/"))}"`; - const mirrorResult = runSsh( - ctx, - `[ ${src} -ef ${dst} ] || { mkdir -p ${mirrorParent} && rm -rf ${dst} && cp -a ${src} ${dst}; }`, + // Copy the skill into the directory the agent's loader actually reads; see + // AGENT_SKILL_MIRRORS for which agents need this and why. Without it the + // upload never registers as a skill. `skill remove` deletes the mirror, so + // install must create it to stay symmetric. The copy is skipped when both + // paths resolve to the same directory, so it is a no-op for agents whose + // loader reads uploadDir directly. + if (paths.mirrorDir) { + const src = shellQuote(paths.uploadDir); + // mirrorDir may contain $HOME, which must expand on the remote shell, so we + // use double quotes (not shellQuote). Safe because skill names are + // restricted to [A-Za-z0-9._-] by parseFrontmatter / the name regex. + const dst = `"${paths.mirrorDir}"`; + const mirrorParent = `"${paths.mirrorDir.slice(0, paths.mirrorDir.lastIndexOf("/"))}"`; + const mirrorResult = runSsh( + ctx, + `[ ${src} -ef ${dst} ] || { mkdir -p ${mirrorParent} && rm -rf ${dst} && cp -a ${src} ${dst}; }`, + ); + if (!mirrorResult || mirrorResult.status !== 0) { + messages.push( + `Warning: failed to mirror skill into ${paths.mirrorDir} (agent may not load it)`, ); - if (!mirrorResult || mirrorResult.status !== 0) { - messages.push( - `Warning: failed to mirror skill into ${paths.mirrorDir} (agent may not load it)`, - ); - } } + } - // Clear sessions.json so OpenClaw re-discovers skills on the next - // session even after an in-place skill update. - if (paths.sessionFile && !opts.skipRefresh) { - const refreshResult = runSsh(ctx, `printf '{}' > ${shellQuote(paths.sessionFile)}`); - if (!refreshResult || refreshResult.status !== 0) { - messages.push("Warning: failed to clear sessions (agent may need manual restart)"); - } + // Clear sessions.json so OpenClaw re-discovers skills on the next + // session even after an in-place skill update. + if (paths.sessionFile && !opts.skipRefresh) { + const refreshResult = runSsh(ctx, `printf '{}' > ${shellQuote(paths.sessionFile)}`); + if (!refreshResult || refreshResult.status !== 0) { + messages.push("Warning: failed to clear sessions (agent may need manual restart)"); } - } else { + } + + if (!paths.mirrorDir && !paths.sessionFile) { messages.push("Restart the agent gateway to pick up the new skill."); } @@ -288,11 +544,12 @@ export function postInstall( /** * Verify the SKILL.md file exists on the sandbox. * - * For OpenClaw the home mirror ($HOME/.openclaw/skills/) must also exist: - * that is the path the agent loads skills from at session start (#4819), so a - * successful upload whose mirror copy failed must NOT verify as installed — - * otherwise the CLI reports success while the skill stays invisible to the - * agent. This mirrors verifyRemove(), which already checks both paths. + * When the agent has a mirror directory it must also exist: that is the path + * the agent loads skills from at session start (#4819 for OpenClaw, #7634 for + * Deep Agents Code), so a successful upload whose mirror copy failed must NOT + * verify as installed — otherwise the CLI reports success while the skill stays + * invisible to the agent. This mirrors verifyRemove(), which already checks + * both paths. */ export function verifyInstall( ctx: SshContext, @@ -300,8 +557,8 @@ export function verifyInstall( opts: { sshExecImpl?: typeof sshExec } = {}, ): boolean { const checks = [`test -f ${shellQuote(`${paths.uploadDir}/SKILL.md`)}`]; - if (paths.isOpenClaw && paths.mirrorDir) { - // mirrorDir contains $HOME, which must expand on the remote shell, so we + if (paths.mirrorDir) { + // mirrorDir may contain $HOME, which must expand on the remote shell, so we // use double quotes (not shellQuote) — safe because skill names are // restricted to [A-Za-z0-9._-]. checks.push(`test -f "${paths.mirrorDir}/SKILL.md"`); diff --git a/src/lib/skill-remote.test.ts b/src/lib/skill-remote.test.ts index 729514f657d..fd6d12b6ec3 100644 --- a/src/lib/skill-remote.test.ts +++ b/src/lib/skill-remote.test.ts @@ -84,6 +84,50 @@ describe("removeSkill (unit — no SSH)", () => { "printf '{}' > '/sandbox/.openclaw/agents/main/sessions/sessions.json'", ]); }); + + it("probes the canonical Deep Agents directory for diagnostics (#7634)", () => { + const ctx = { configFile: "/tmp/ssh.conf", sandboxName: "test-sandbox" }; + const paths = resolveSkillPaths( + { name: "langchain-deepagents-code", configPaths: { dir: "/sandbox/.deepagents" } }, + "user-authored", + ); + const commands: string[] = []; + checkExisting(ctx, paths, { + sshExecImpl: (_ctx, command) => { + commands.push(command); + return { status: 0, stdout: "ABSENT", stderr: "" }; + }, + }); + + expect(paths.uploadDirSharedWithAgent).toBe(true); + expect(commands).toEqual([ + "{ test -e '/sandbox/.deepagents/agent/skills/user-authored'; } && echo EXISTS || echo ABSENT", + ]); + }); + + it("refuses to remove from the agent-owned Deep Agents directory (#7634)", () => { + const ctx = { configFile: "/tmp/ssh.conf", sandboxName: "test-sandbox" }; + const paths = resolveSkillPaths( + { name: "langchain-deepagents-code", configPaths: { dir: "/sandbox/.deepagents" } }, + "test-skill", + ); + const commands: string[] = []; + const result = removeSkill(ctx, paths, { + sshExecImpl: (_ctx, command) => { + commands.push(command); + return { status: 0, stdout: "", stderr: "" }; + }, + }); + + expect(result.success).toBe(false); + expect(result.removedUploadDir).toBe(false); + expect(result.removedMirrorDir).toBe(false); + expect(result.clearedSessions).toBe(false); + expect(result.messages).toEqual([ + "Error: automatic removal is unavailable for the agent-owned skill directory /sandbox/.deepagents/agent/skills/test-skill.", + ]); + expect(commands).toEqual([]); + }); }); describe("verifyRemove (unit — no SSH)", () => { @@ -102,6 +146,23 @@ describe("verifyRemove (unit — no SSH)", () => { expect(verifyRemove(ctx, paths)).toBe(false); }); + it("refuses shared Deep Agents verification without an SSH call", () => { + const paths = resolveSkillPaths( + { name: "langchain-deepagents-code", configPaths: { dir: "/sandbox/.deepagents" } }, + "test-skill", + ); + const commands: string[] = []; + const gone = verifyRemove({ configFile: "/tmp/ssh.conf", sandboxName: "test-sandbox" }, paths, { + sshExecImpl: (_ctx, command) => { + commands.push(command); + return { status: 0, stdout: "GONE", stderr: "" }; + }, + }); + + expect(gone).toBe(false); + expect(commands).toEqual([]); + }); + it("verifies both OpenClaw skill directories are gone", () => { const paths = resolveSkillPaths(null, "test-skill"); const ctx = { configFile: "/tmp/ssh.conf", sandboxName: "test-sandbox" }; diff --git a/src/lib/skill-remote.ts b/src/lib/skill-remote.ts index 8b19d1780a7..1285ede89d1 100644 --- a/src/lib/skill-remote.ts +++ b/src/lib/skill-remote.ts @@ -78,8 +78,11 @@ export function checkExisting( paths: SkillPaths, opts: { sshExecImpl?: typeof sshExec } = {}, ): boolean | null { + // Existence gate for `skill remove`. Shared agent destinations are probed + // only for user-facing diagnostics; removeSkill() still refuses to mutate + // them because their presence is not proof of NemoClaw ownership (#5753). const checks = [`test -e ${shellQuote(paths.uploadDir)}`]; - if (paths.isOpenClaw && paths.mirrorDir) { + if (paths.mirrorDir) { checks.push(`test -e "${paths.mirrorDir}"`); } const runSsh = opts.sshExecImpl ?? sshExec; @@ -102,9 +105,9 @@ export interface RemoveResult { /** * Remove a skill from the sandbox by name. - * Deletes the immutable upload directory, the OpenClaw mirror directory - * (if applicable), and clears sessions.json so the agent re-discovers - * the remaining skills on the next session. + * Deletes the immutable upload directory, the agent's skill-load mirror + * directory (if applicable), and clears sessions.json so the agent + * re-discovers the remaining skills on the next session. * * Only the named skill directory is deleted — other skills are untouched. */ @@ -116,6 +119,18 @@ export function removeSkill( const messages: string[] = []; const runSsh = opts.sshExecImpl ?? sshExec; + if (paths.uploadDirSharedWithAgent) { + return { + success: false, + removedUploadDir: false, + removedMirrorDir: false, + clearedSessions: false, + messages: [ + `Error: automatic removal is unavailable for the agent-owned skill directory ${paths.uploadDir}.`, + ], + }; + } + // 1. Remove the immutable upload directory (/sandbox/.openclaw/skills//) const uploadDir = shellQuote(paths.uploadDir); const removeUpload = runSsh(ctx, `rm -rf ${uploadDir}`); @@ -124,13 +139,14 @@ export function removeSkill( messages.push(`Warning: failed to remove upload directory ${paths.uploadDir}`); } - // 2. Remove the OpenClaw mirror ($HOME/.openclaw/skills//) - // mirrorDir contains $HOME which must expand on the remote shell, so we + // 2. Remove the agent's skill-load mirror (see AGENT_SKILL_MIRRORS); leaving + // it behind keeps the removed skill loadable even though uploadDir is gone. + // mirrorDir may contain $HOME which must expand on the remote shell, so we // use double quotes (not shellQuote). This is safe because skill names // are restricted to [A-Za-z0-9._-] by parseFrontmatter / the name // validation regex, so $HOME expansion is the only variable substitution. let removedMirrorDir = false; - if (paths.isOpenClaw && paths.mirrorDir) { + if (paths.mirrorDir) { const removeMirror = runSsh(ctx, `rm -rf "${paths.mirrorDir}"`); removedMirrorDir = removeMirror !== null && removeMirror.status === 0; if (!removedMirrorDir) { @@ -140,18 +156,20 @@ export function removeSkill( // 3. Clear sessions.json so the agent re-discovers the remaining skills. let clearedSessions = false; - if (paths.isOpenClaw && paths.sessionFile) { + if (paths.sessionFile) { const clearResult = runSsh(ctx, `printf '{}' > ${shellQuote(paths.sessionFile)}`); clearedSessions = clearResult !== null && clearResult.status === 0; if (!clearedSessions) { messages.push("Warning: failed to clear sessions (agent may need manual restart)"); } - } else if (!paths.isOpenClaw) { + } + + if (!paths.mirrorDir && !paths.sessionFile) { messages.push("Restart the agent gateway for the removal to take effect."); } return { - success: removedUploadDir && (!paths.isOpenClaw || removedMirrorDir), + success: removedUploadDir && (!paths.mirrorDir || removedMirrorDir), removedUploadDir, removedMirrorDir, clearedSessions, @@ -161,15 +179,17 @@ export function removeSkill( /** * Verify the skill directory no longer exists on the sandbox. - * For OpenClaw sandboxes, both the upload dir and the mirror dir must be gone. + * For agents with a skill-load mirror, both the upload dir and the mirror dir + * must be gone. */ export function verifyRemove( ctx: SshContext, paths: SkillPaths, opts: { sshExecImpl?: typeof sshExec } = {}, ): boolean { + if (paths.uploadDirSharedWithAgent) return false; const checks = [`test ! -e ${shellQuote(paths.uploadDir)}`]; - if (paths.isOpenClaw && paths.mirrorDir) { + if (paths.mirrorDir) { checks.push(`test ! -e "${paths.mirrorDir}"`); } const runSsh = opts.sshExecImpl ?? sshExec;