From 0d84eb4e70431b6808556af0717808f5cb6bcbb0 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 11 Jul 2026 17:54:14 -0700 Subject: [PATCH] fix(skills): read node-hosted skills from their publishing node (#104832) * fix(skills): make node skill locators readable * test(skills): track node skill temp directory * chore: defer node skill changelog to release process --- docs/nodes/index.md | 9 ++- .../agent-tools.before-tool-call.e2e.test.ts | 35 ++++++++++++ src/agents/agent-tools.before-tool-call.ts | 11 +++- ...aliases-schemas-without-dropping-g.test.ts | 28 +++++++++- ...tools.create-openclaw-coding-tools.test.ts | 34 ++++++++++++ src/agents/agent-tools.read.ts | 55 +++++++++++++++++++ src/agents/agent-tools.ts | 31 +++++++---- src/node-host/invoke.test.ts | 42 +++++++++++++- src/node-host/invoke.ts | 21 ++++++- src/node-host/skills.test.ts | 27 ++++++++- src/node-host/skills.ts | 26 +++++++++ src/skills/loading/skill-contract.ts | 2 + src/skills/runtime/remote-skills.test.ts | 12 +++- src/skills/runtime/remote-skills.ts | 4 +- 14 files changed, 314 insertions(+), 23 deletions(-) diff --git a/docs/nodes/index.md b/docs/nodes/index.md index 68a36cb17f96..a0fc14982553 100644 --- a/docs/nodes/index.md +++ b/docs/nodes/index.md @@ -203,8 +203,13 @@ node skill files; the node host does not watch the skills directory. Node-hosted skill entries identify their node and carry their execution location. Skill files, referenced relative paths, and binaries remain on that -node. Load instructions and run commands with -`exec host=node node=` so relative paths resolve on the node rather +node. The agent reads the advertised `node://.../SKILL.md` location with the +normal `read` tool. `file_fetch` accepts operator-approved absolute node paths, +not node skill locators; runtimes without the normal read tool can instead run +`cat SKILL.md` through `exec host=node node=` with the advertised +`node://.../skills/` directory as `workdir`. Referenced files and binaries +use the same exec target and workdir. The node host resolves that locator against +its active OpenClaw state directory, so relative paths resolve on the node rather than the Gateway machine. The publishing node must have approved `system.run`, and the agent's exec policy must allow `host=node`; otherwise the skill stays out of that agent's snapshot. diff --git a/src/agents/agent-tools.before-tool-call.e2e.test.ts b/src/agents/agent-tools.before-tool-call.e2e.test.ts index 6640f70ec7f9..6321e59f69cb 100644 --- a/src/agents/agent-tools.before-tool-call.e2e.test.ts +++ b/src/agents/agent-tools.before-tool-call.e2e.test.ts @@ -977,6 +977,41 @@ describe("before_tool_call loop detection behavior", () => { }); }); + it("emits skill usage diagnostics for node skill locators", async () => { + const locator = "node://node-1/skills/remote-skill/SKILL.md"; + const execute = vi.fn().mockResolvedValue({ content: [{ type: "text", text: "skill" }] }); + const tool = wrapToolWithBeforeToolCallHook({ name: "read", execute } as any, { + agentId: "main", + sessionKey: "session-key", + skillsSnapshot: { + prompt: "", + skills: [{ name: "remote-skill" }], + resolvedSkills: [ + createCanonicalFixtureSkill({ + name: "remote-skill", + description: "Remote skill", + filePath: locator, + baseDir: "node://node-1/skills/remote-skill", + source: "openclaw-node", + }), + ], + }, + loopDetection: { enabled: false }, + }); + + await withSkillUsageDiagnosticEvents(async (emitted, _privateData, flush) => { + await tool.execute("tool-call-node-skill", { path: locator }, undefined, undefined); + await flush(); + + expectEventFields(emitted[1], { + type: "skill.used", + skillName: "remote-skill", + activation: "read", + toolName: "read", + }); + }); + }); + it("accounts sandbox skill reads against the original canonical file", async () => { const workspaceDir = "/workspace"; const readPath = "/workspace/.openclaw/sandbox-skills/skills/demo/SKILL.md"; diff --git a/src/agents/agent-tools.before-tool-call.ts b/src/agents/agent-tools.before-tool-call.ts index 3173662a8669..fbf19e3d67cd 100644 --- a/src/agents/agent-tools.before-tool-call.ts +++ b/src/agents/agent-tools.before-tool-call.ts @@ -638,6 +638,9 @@ function resolveRelativeToolPath(candidate: string, ctx?: HookContext): string | if (!trimmed) { return undefined; } + if (trimmed.startsWith("node://")) { + return trimmed; + } if (trimmed === "~") { return os.homedir(); } @@ -670,8 +673,12 @@ function skillInstructionPaths(snapshot: SkillSnapshot | undefined): Map { + it("reads exact node skill locators without sending them to the filesystem backend", async () => { + const locator = "node://node-1/skills/pond/SKILL.md"; + const execute = vi.fn(async () => { + throw new Error("filesystem backend should not run"); + }); + const tool = wrapReadToolWithSkillContent( + { + name: "read", + label: "read", + description: "read a file", + parameters: {}, + execute, + } as never, + [{ filePath: locator, readContent: "# Pond\nremote-marker" }], + ); + + const result = await tool.execute("node-skill-read", { path: locator }); + + expect(extractToolText(result)).toContain("remote-marker"); + expect(execute).not.toHaveBeenCalled(); + }); + it("uses host decoding only for host-backed sandbox paths", async () => { const tmpDir = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-sbx-encoding-")); await fs.writeFile(path.join(tmpDir, "notes.txt"), "hello", "utf8"); diff --git a/src/agents/agent-tools.create-openclaw-coding-tools.test.ts b/src/agents/agent-tools.create-openclaw-coding-tools.test.ts index 3524aa7cbc36..f827a4af5e87 100644 --- a/src/agents/agent-tools.create-openclaw-coding-tools.test.ts +++ b/src/agents/agent-tools.create-openclaw-coding-tools.test.ts @@ -158,6 +158,40 @@ function cronCreatorToolNames( } describe("createOpenClawCodingTools", () => { + it("reads node-hosted skill content through the assembled workspace-only read tool", async () => { + const locator = "node://node-1/skills/pond/SKILL.md"; + const tools = createOpenClawCodingTools({ + config: { tools: { fs: { workspaceOnly: true } } }, + skillsSnapshot: { + prompt: "", + skills: [{ name: "pond" }], + resolvedSkills: [ + { + name: "pond", + description: "Pond skill", + filePath: locator, + baseDir: "node://node-1/skills/pond", + readContent: "# Pond\nassembled-marker", + source: "openclaw-node", + sourceInfo: { + source: "openclaw-node", + path: locator, + scope: "temporary", + origin: "top-level", + }, + disableModelInvocation: false, + }, + ], + }, + }); + + const result = await requireTool(tools, "read").execute("node-skill-read", { + path: locator, + }); + + expect(JSON.stringify(result)).toContain("assembled-marker"); + }); + const testConfig: OpenClawConfig = {}; afterEach(() => { diff --git a/src/agents/agent-tools.read.ts b/src/agents/agent-tools.read.ts index b3bf74259ca6..857e2daabc56 100644 --- a/src/agents/agent-tools.read.ts +++ b/src/agents/agent-tools.read.ts @@ -65,6 +65,11 @@ type OpenClawReadToolOptions = { imageSanitization?: ImageSanitizationLimits; }; +type SkillReadContent = { + filePath: string; + readContent?: string; +}; + type ReadTruncationDetails = { truncated: boolean; outputLines: number; @@ -906,6 +911,56 @@ export function createOpenClawReadTool( }; } +/** Serve exact non-filesystem skill locators before workspace path guards run. */ +export function wrapReadToolWithSkillContent( + tool: AnyAgentTool, + skills: readonly SkillReadContent[] | undefined, + options?: OpenClawReadToolOptions, +): AnyAgentTool { + const contentByPath = new Map( + (skills ?? []).flatMap((skill) => + skill.filePath.startsWith("node://") && typeof skill.readContent === "string" + ? [[skill.filePath, skill.readContent] as const] + : [], + ), + ); + if (contentByPath.size === 0) { + return tool; + } + const readContent = (filePath: string): string => { + const content = contentByPath.get(filePath); + if (content === undefined) { + throw Object.assign(new Error(`Virtual skill file not found: ${filePath}`), { + code: "ENOENT", + }); + } + return content; + }; + const virtualBase = createReadTool("/", { + operations: { + resolvePath: (filePath) => filePath, + access: async (filePath) => void readContent(filePath), + readFile: async (filePath) => Buffer.from(readContent(filePath), "utf8"), + }, + }) as unknown as AnyAgentTool; + const virtualRead = createOpenClawReadTool(virtualBase, options); + return { + ...tool, + execute: async (toolCallId, args, signal, onUpdate) => { + const record = getToolParamsRecord(args); + const rawPath = record?.path; + const normalizedPath = + typeof rawPath === "string" ? normalizeFileToolPathParam(rawPath) : undefined; + if (normalizedPath && contentByPath.has(normalizedPath)) { + const virtualArgs = + normalizedPath === rawPath || !record ? args : { ...record, path: normalizedPath }; + return virtualRead.execute(toolCallId, virtualArgs, signal, onUpdate); + } + return tool.execute(toolCallId, args, signal, onUpdate); + }, + }; +} + function createSandboxReadOperations(params: SandboxToolParams) { return { resolvePath: (filePath: string) => { diff --git a/src/agents/agent-tools.ts b/src/agents/agent-tools.ts index 3fda3d3db745..5d8e883786c1 100644 --- a/src/agents/agent-tools.ts +++ b/src/agents/agent-tools.ts @@ -44,6 +44,7 @@ import { createSandboxedEditTool, createSandboxedReadTool, createSandboxedWriteTool, + wrapReadToolWithSkillContent, getToolParamsRecord, wrapToolMemoryFlushAppendOnlyWrite, wrapToolWorkspaceRootGuard, @@ -702,13 +703,17 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions) modelContextWindowTokens: options?.modelContextWindowTokens, imageSanitization, }); + const guarded = workspaceOnly + ? wrapToolWorkspaceRootGuardWithOptions(sandboxed, sandboxRoot, { + additionalContainerMounts: readOnlySandboxReadMounts(sandbox), + containerWorkdir: sandbox.containerWorkdir, + }) + : sandboxed; base.push( - workspaceOnly - ? wrapToolWorkspaceRootGuardWithOptions(sandboxed, sandboxRoot, { - additionalContainerMounts: readOnlySandboxReadMounts(sandbox), - containerWorkdir: sandbox.containerWorkdir, - }) - : sandboxed, + wrapReadToolWithSkillContent(guarded, options?.skillsSnapshot?.resolvedSkills, { + modelContextWindowTokens: options?.modelContextWindowTokens, + imageSanitization, + }), ); continue; } @@ -717,12 +722,16 @@ function createOpenClawCodingToolsInternal(options?: OpenClawCodingToolsOptions) modelContextWindowTokens: options?.modelContextWindowTokens, imageSanitization, }); + const guarded = workspaceOnly + ? wrapToolWorkspaceRootGuardWithOptions(wrapped, codingRoot, { + additionalRoots: skillReadRoots, + }) + : wrapped; base.push( - workspaceOnly - ? wrapToolWorkspaceRootGuardWithOptions(wrapped, codingRoot, { - additionalRoots: skillReadRoots, - }) - : wrapped, + wrapReadToolWithSkillContent(guarded, options?.skillsSnapshot?.resolvedSkills, { + modelContextWindowTokens: options?.modelContextWindowTokens, + imageSanitization, + }), ); continue; } diff --git a/src/node-host/invoke.test.ts b/src/node-host/invoke.test.ts index f5bebe7b9d7f..5e55c6e9adb0 100644 --- a/src/node-host/invoke.test.ts +++ b/src/node-host/invoke.test.ts @@ -2,13 +2,16 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; -import { beforeEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { useAutoCleanupTempDirTracker } from "../../test/helpers/temp-dir.js"; import type { GatewayClient } from "../gateway/client.js"; import { saveExecApprovals, type ExecApprovalsSnapshot } from "../infra/exec-approvals.js"; import { withEnvAsync } from "../test-utils/env.js"; import type { SkillBinsProvider } from "./invoke-types.js"; import { handleInvoke } from "./invoke.js"; +const tempDirs = useAutoCleanupTempDirTracker(afterEach); + const approvalResolutionFailure = vi.hoisted(() => ({ error: null as Error | null })); type ExecApprovalsUpdate = Parameters< typeof import("../infra/exec-approvals.js").updateExecApprovals @@ -400,6 +403,43 @@ describe("node host invoke", () => { }, ); + it.runIf(process.platform !== "win32")( + "resolves node skill cwd locators before preparing system.run", + async () => { + const stateDir = fs.realpathSync(tempDirs.make("openclaw-node-skill-cwd-")); + const skillDir = path.join(stateDir, "skills", "cwd-skill"); + fs.mkdirSync(skillDir, { recursive: true }); + fs.writeFileSync( + path.join(skillDir, "SKILL.md"), + "---\nname: cwd-skill\ndescription: Cwd skill\n---\n", + ); + + await withEnvAsync({ OPENCLAW_STATE_DIR: stateDir }, async () => { + const request = vi.fn().mockResolvedValue(null); + const skillBins: SkillBinsProvider = { current: async () => [] }; + await handleInvoke( + { + id: "invoke-skill-cwd", + nodeId: "node-1", + command: "system.run.prepare", + paramsJSON: JSON.stringify({ + command: ["/bin/pwd"], + cwd: "node://node-1/skills/cwd-skill", + }), + }, + { request } as unknown as GatewayClient, + skillBins, + ); + + const result = request.mock.calls[0]?.[1] as { payloadJSON?: string } | undefined; + const payload = JSON.parse(result?.payloadJSON ?? "{}") as { + plan?: { cwd?: string }; + }; + expect(payload.plan?.cwd).toBe(fs.realpathSync(skillDir)); + }); + }, + ); + it.runIf(process.platform !== "win32")( "keeps prepared allow-always coverage incomplete when any planned command is prompt-only", async () => { diff --git a/src/node-host/invoke.ts b/src/node-host/invoke.ts index c0aaa860e149..15140f6a0ce0 100644 --- a/src/node-host/invoke.ts +++ b/src/node-host/invoke.ts @@ -58,6 +58,7 @@ import type { } from "./invoke-types.js"; import { NodeHostMcpError, type NodeHostMcpManager } from "./mcp.js"; import { invokeRegisteredNodeHostCommand } from "./plugin-node-host.js"; +import { resolveNodeHostedSkillDirectory } from "./skills.js"; const OUTPUT_CAP = 200_000; const MCP_TEXT_CONTENT_MAX_BYTES = 1024 * 1024; @@ -110,6 +111,16 @@ type SystemRunPrepareEnv = message: string; }; +function resolveNodeSkillCwdParam(params: T, nodeId: string): T { + if (typeof params.cwd !== "string") { + return params; + } + // Resolve before approval planning so the plan, policy, and spawn all bind + // the same canonical node-local directory instead of trusting a URI at exec time. + const resolved = resolveNodeHostedSkillDirectory(params.cwd, nodeId); + return resolved ? { ...params, cwd: resolved } : params; +} + function buildEnvOverrideRejectionMessage(params: { rejectedOverrideBlockedKeys: string[]; rejectedOverrideInvalidKeys: string[]; @@ -770,7 +781,10 @@ async function dispatchInvoke( if (command === "system.run.prepare") { try { - const params = decodeParams(frame.paramsJSON); + const params = resolveNodeSkillCwdParam( + decodeParams(frame.paramsJSON), + frame.nodeId, + ); const prepared = buildSystemRunApprovalPlan(params); if (!prepared.ok) { await sendErrorResult(client, frame, "INVALID_REQUEST", prepared.message); @@ -826,7 +840,10 @@ async function dispatchInvoke( let params: SystemRunParams; try { - params = decodeParams(frame.paramsJSON); + params = resolveNodeSkillCwdParam( + decodeParams(frame.paramsJSON), + frame.nodeId, + ); } catch (err) { await sendInvalidRequestResult(client, frame, err); return; diff --git a/src/node-host/skills.test.ts b/src/node-host/skills.test.ts index 4b5a90174b36..1d49817c041e 100644 --- a/src/node-host/skills.test.ts +++ b/src/node-host/skills.test.ts @@ -2,7 +2,7 @@ import fs from "node:fs"; import os from "node:os"; import path from "node:path"; import { afterEach, describe, expect, it, vi } from "vitest"; -import { scanNodeHostedSkills } from "./skills.js"; +import { resolveNodeHostedSkillDirectory, scanNodeHostedSkills } from "./skills.js"; const roots: string[] = []; @@ -33,6 +33,31 @@ afterEach(() => { }); describe("scanNodeHostedSkills", () => { + it("resolves a matching node skill locator against a custom state directory", () => { + const stateDir = createRoot(); + const skillDir = path.join(stateDir, "skills", "profile-skill"); + writeSkill(path.join(stateDir, "skills"), "profile-skill", "Profile skill"); + vi.stubEnv("OPENCLAW_STATE_DIR", stateDir); + + expect(resolveNodeHostedSkillDirectory("node://node-1/skills/profile-skill", "node-1")).toBe( + fs.realpathSync(skillDir), + ); + expect(() => + resolveNodeHostedSkillDirectory("node://node-2/skills/profile-skill", "node-1"), + ).toThrow("invalid for this node"); + expect(() => + resolveNodeHostedSkillDirectory("node://node-1/skills/../profile-skill", "node-1"), + ).toThrow("invalid for this node"); + if (process.platform !== "win32") { + const outsideDir = path.join(stateDir, "outside"); + writeSkill(stateDir, "outside", "Outside"); + fs.symlinkSync(outsideDir, path.join(stateDir, "skills", "escape")); + expect(() => + resolveNodeHostedSkillDirectory("node://node-1/skills/escape", "node-1"), + ).toThrow("unavailable"); + } + }); + it("uses the active OpenClaw profile skills directory by default", () => { const stateDir = createRoot(); const content = writeSkill(path.join(stateDir, "skills"), "profile-skill", "Profile skill"); diff --git a/src/node-host/skills.ts b/src/node-host/skills.ts index 78061533913d..235f188cc568 100644 --- a/src/node-host/skills.ts +++ b/src/node-host/skills.ts @@ -1,6 +1,7 @@ import fs from "node:fs"; import path from "node:path"; import type { NodeSkillDescriptor } from "../../packages/gateway-protocol/src/schema/nodes.js"; +import { isPathInside } from "../infra/path-guards.js"; import { NODE_SKILL_MAX_CONTENT_BYTES, NODE_SKILL_MAX_COUNT, @@ -16,6 +17,31 @@ type ScanNodeHostedSkillsOptions = { warn?: (message: string) => void; }; +/** Resolve an advertised node skill directory locator to this node's canonical path. */ +export function resolveNodeHostedSkillDirectory(locator: string, nodeId: string): string | null { + if (!locator.startsWith("node://")) { + return null; + } + const prefix = `node://${encodeURIComponent(nodeId)}/skills/`; + const name = locator.startsWith(prefix) ? locator.slice(prefix.length) : ""; + if (!NODE_SKILL_NAME_RE.test(name)) { + throw new Error("INVALID_REQUEST: node skill cwd locator is invalid for this node"); + } + try { + const skillsDir = fs.realpathSync(path.join(resolveConfigDir(), "skills")); + const skillDir = fs.realpathSync(path.join(skillsDir, name)); + if ( + !isPathInside(skillsDir, skillDir) || + !fs.statSync(path.join(skillDir, "SKILL.md")).isFile() + ) { + throw new Error("missing SKILL.md"); + } + return skillDir; + } catch { + throw new Error("INVALID_REQUEST: node skill cwd locator is unavailable"); + } +} + function listCandidateSkillFiles(skillsDir: string, warn: (message: string) => void): string[] { let entries: fs.Dirent[]; try { diff --git a/src/skills/loading/skill-contract.ts b/src/skills/loading/skill-contract.ts index a785e2aefe41..5fcd3aceba09 100644 --- a/src/skills/loading/skill-contract.ts +++ b/src/skills/loading/skill-contract.ts @@ -6,6 +6,8 @@ export interface Skill { description: string; /** Additional loading guidance rendered with the location in full and compact catalogs. */ locationNote?: string; + /** Runtime-only content for non-filesystem skill locators such as node://. */ + readContent?: string; filePath: string; baseDir: string; /** Deterministic marker for the SKILL.md content rendered as . */ diff --git a/src/skills/runtime/remote-skills.test.ts b/src/skills/runtime/remote-skills.test.ts index 5db71c89101a..9fc95b1cdaa4 100644 --- a/src/skills/runtime/remote-skills.test.ts +++ b/src/skills/runtime/remote-skills.test.ts @@ -65,10 +65,18 @@ describe("node-hosted skill snapshots", () => { const snapshot = buildWorkspaceSkillSnapshot("/workspace", { entries }); expect(snapshot.skills.map((skill) => skill.name)).toEqual(["release-helper"]); expect(snapshot.prompt).toContain("Build Mac (node-1)"); + expect(snapshot.prompt).toContain( + "Read this SKILL.md with the normal read tool at its exact node:// location", + ); + expect(snapshot.prompt).toContain("do not use file_fetch"); + expect(snapshot.prompt).toContain("to run cat SKILL.md"); expect(snapshot.prompt).toContain("exec host=node node=node-1"); - expect(snapshot.prompt).toContain("skills/release-helper/ in its OpenClaw state dir"); - expect(snapshot.prompt).toContain("relative paths resolve on the node"); + expect(snapshot.prompt).toContain("workdir=node://node-1/skills/release-helper"); + expect(snapshot.prompt).toContain("node host resolves that locator"); expect(snapshot.prompt).toContain("node://node-1/skills/release-helper/SKILL.md"); + expect(snapshot.resolvedSkills?.[0]?.readContent).toBe( + content("release-helper", "Prepare a release"), + ); expect(getSkillsSnapshotVersion()).toBeGreaterThan(before); const connectedVersion = getSkillsSnapshotVersion(); diff --git a/src/skills/runtime/remote-skills.ts b/src/skills/runtime/remote-skills.ts index 350ab8494b71..372ba41c2c77 100644 --- a/src/skills/runtime/remote-skills.ts +++ b/src/skills/runtime/remote-skills.ts @@ -158,7 +158,8 @@ function remoteSkillLocation(nodeId: string, name: string): string { function locatorNote(node: RemoteSkillNode, skillName: string): string { const label = node.displayName?.trim() || node.nodeId; - return `Node-hosted on ${label} (${node.nodeId}): files and bins live on that node under skills/${skillName}/ in its OpenClaw state dir. Run every command via exec host=node node=${node.nodeId}; relative paths resolve on the node.`; + const cwd = remoteSkillLocation(node.nodeId, skillName).slice(0, -"/SKILL.md".length); + return `Node-hosted on ${label} (${node.nodeId}). Read this SKILL.md with the normal read tool at its exact node:// location; do not use file_fetch, which only accepts approved absolute node paths. If read is unavailable, use exec host=node node=${node.nodeId} with workdir=${cwd} to run cat SKILL.md. Run referenced files and bins with the same exec target and workdir; the node host resolves that locator to the node-local skill directory.`; } export function mergeRemoteNodeSkillEntries( @@ -215,6 +216,7 @@ export function mergeRemoteNodeSkillEntries( name: exposedName, description: skill.description, locationNote: locatorNote(node, skill.name), + readContent: skill.content, filePath, baseDir: filePath.slice(0, -"/SKILL.md".length), promptVersion: computeSkillPromptVersion(skill.content),