Skip to content

Commit 287b10a

Browse files
abnershangshakkernerd
abnershang
authored and
shakkernerd
committed
feat(skills): allow trusted workshop symlink targets
1 parent ea813a2 commit 287b10a

11 files changed

Lines changed: 233 additions & 45 deletions

File tree

‎docs/tools/skills-config.md‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -364,6 +364,8 @@ To allow an intentional symlink layout, declare the trusted target:
364364
With this config, `<workspace>/skills/manager -> ~/Projects/manager/skills` is
365365
accepted after realpath resolution. `extraDirs` scans the sibling repo directly;
366366
`allowSymlinkTargets` preserves the symlinked path for existing layouts.
367+
Skill Workshop apply uses the same trust list before publishing proposals
368+
through symlinked workspace skill paths.
367369
368370
Managed `~/.openclaw/skills` and personal `~/.agents/skills` directories
369371
already accept skill-directory symlinks (per-skill `SKILL.md` containment still

‎src/agents/tools/skill-workshop-tool.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,7 @@ export function createSkillWorkshopTool(options: SkillWorkshopToolOptions): AnyA
170170
if (action === "apply") {
171171
const applied = await applySkillProposal({
172172
workspaceDir: options.workspaceDir,
173+
config: options.config,
173174
proposalId: readLifecycleProposalIdParam(params),
174175
reason: readStringParam(params, "reason"),
175176
});

‎src/cli/skills-cli.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -734,8 +734,8 @@ export function registerSkillsCli(program: Command) {
734734
.action(
735735
async (proposalId: string, opts: { json?: boolean; agent?: string }, command: Command) => {
736736
try {
737-
const { workspaceDir } = resolveSkillsWorkspaceForCommand(command.parent, opts);
738-
const applied = await applySkillProposal({ workspaceDir, proposalId });
737+
const { config, workspaceDir } = resolveSkillsWorkspaceForCommand(command.parent, opts);
738+
const applied = await applySkillProposal({ workspaceDir, config, proposalId });
739739
if (opts.json) {
740740
defaultRuntime.writeJson(applied);
741741
return;

‎src/gateway/server-methods/skills.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -483,6 +483,7 @@ export const skillsHandlers: GatewayRequestHandlers = {
483483
run: (parsedParams, resolved) =>
484484
applySkillProposal({
485485
workspaceDir: resolved.workspaceDir,
486+
config: resolved.cfg,
486487
proposalId: parsedParams.proposalId,
487488
reason: parsedParams.reason,
488489
}),

‎src/skills/loading/symlink-targets.ts‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Shared helpers for config-trusted skill symlink targets.
2+
import fs from "node:fs";
3+
import path from "node:path";
4+
import { normalizeOptionalString } from "@openclaw/normalization-core/string-coerce";
5+
import { uniqueStrings } from "@openclaw/normalization-core/string-normalization";
6+
import type { OpenClawConfig } from "../../config/types.openclaw.js";
7+
import { isPathInside } from "../../infra/path-guards.js";
8+
import { resolveUserPath } from "../../utils.js";
9+
10+
export function resolveAllowedSkillSymlinkTargetRealPaths(config?: OpenClawConfig): string[] {
11+
const rawTargets = config?.skills?.load?.allowSymlinkTargets ?? [];
12+
const targetPaths = rawTargets
13+
.map((dir) => normalizeOptionalString(dir) ?? "")
14+
.filter(Boolean)
15+
.map((dir) => tryRealpath(resolveUserPath(dir)))
16+
.filter((dir): dir is string => Boolean(dir));
17+
return uniqueStrings(targetPaths);
18+
}
19+
20+
export function findContainingAllowedSkillSymlinkTarget(
21+
rootRealPaths: readonly string[],
22+
candidateRealPath: string,
23+
): string | null {
24+
const resolvedCandidate = path.resolve(candidateRealPath);
25+
for (const rootRealPath of rootRealPaths) {
26+
const resolvedRoot = path.resolve(rootRealPath);
27+
if (isPathInside(resolvedRoot, resolvedCandidate)) {
28+
return resolvedRoot;
29+
}
30+
}
31+
return null;
32+
}
33+
34+
export function isPathInsideAnyAllowedSkillSymlinkTarget(
35+
rootRealPaths: readonly string[],
36+
candidateRealPath: string,
37+
): boolean {
38+
return findContainingAllowedSkillSymlinkTarget(rootRealPaths, candidateRealPath) !== null;
39+
}
40+
41+
export function tryRealpath(filePath: string): string | null {
42+
try {
43+
return fs.realpathSync(filePath);
44+
} catch {
45+
return null;
46+
}
47+
}

‎src/skills/loading/workspace.ts‎

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ import { loadSkillsFromDirSafe, readSkillFrontmatterSafe } from "./local-loader.
3434
import { resolvePluginSkillDirs } from "./plugin-skills.js";
3535
import { serializeByKey } from "./serialize.js";
3636
import { formatSkillsForPrompt, type Skill } from "./skill-contract.js";
37+
import { resolveAllowedSkillSymlinkTargetRealPaths, tryRealpath } from "./symlink-targets.js";
3738

3839
const fsp = fs.promises;
3940
const skillsLogger = createSubsystemLogger("skills");
@@ -375,14 +376,6 @@ function hasLoadableSkillFrontmatter(
375376
return Boolean(name) && Boolean(frontmatter?.description?.trim());
376377
}
377378

378-
function tryRealpath(filePath: string): string | null {
379-
try {
380-
return fs.realpathSync(filePath);
381-
} catch {
382-
return null;
383-
}
384-
}
385-
386379
function isSymlinkPath(filePath: string): boolean {
387380
try {
388381
return fs.lstatSync(filePath).isSymbolicLink();
@@ -735,16 +728,6 @@ function resolvePluginSkillRootRealPaths(pluginSkillDirs: readonly string[]): st
735728
);
736729
}
737730

738-
function resolveAllowedSymlinkTargetRealPaths(config?: OpenClawConfig): string[] {
739-
const rawTargets = config?.skills?.load?.allowSymlinkTargets ?? [];
740-
const targetPaths = rawTargets
741-
.map((dir) => normalizeOptionalString(dir) ?? "")
742-
.filter(Boolean)
743-
.map((dir) => tryRealpath(resolveUserPath(dir)))
744-
.filter((dir): dir is string => Boolean(dir));
745-
return uniqueStrings(targetPaths);
746-
}
747-
748731
function loadGeneratedPluginSkillRecords(params: {
749732
pluginSkillsDir: string;
750733
pluginSkillDirs: readonly string[];
@@ -857,7 +840,7 @@ function loadSkillEntries(
857840
},
858841
): SkillEntry[] {
859842
const limits = resolveSkillsLimits(opts?.config, opts?.agentId);
860-
const allowedSymlinkTargetRealPaths = resolveAllowedSymlinkTargetRealPaths(opts?.config);
843+
const allowedSymlinkTargetRealPaths = resolveAllowedSkillSymlinkTargetRealPaths(opts?.config);
861844

862845
const loadSkills = (params: { dir: string; source: string }): LoadedSkillRecord[] => {
863846
const rootDir = path.resolve(params.dir);

‎src/skills/runtime/refresh.ts‎

Lines changed: 5 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@ import type { OpenClawConfig } from "../../config/types.openclaw.js";
88
import { createSubsystemLogger } from "../../logging/subsystem.js";
99
import { CONFIG_DIR, resolveUserPath } from "../../utils.js";
1010
import { resolvePluginSkillDirs } from "../loading/plugin-skills.js";
11+
import {
12+
resolveAllowedSkillSymlinkTargetRealPaths,
13+
tryRealpath,
14+
} from "../loading/symlink-targets.js";
1115
import {
1216
bumpSkillsSnapshotVersion,
1317
clearSkillsSnapshotVersionForWorkspace,
@@ -111,7 +115,7 @@ function resolveWatchTargets(workspaceDir: string, config?: OpenClawConfig): Wat
111115
.filter(Boolean)
112116
.map((dir) => resolveUserPath(dir));
113117
const pluginSkillDirs = resolvePluginSkillDirs({ workspaceDir, config });
114-
const allowedSymlinkTargetRealPaths = resolveAllowedSymlinkTargetRealPaths(config);
118+
const allowedSymlinkTargetRealPaths = resolveAllowedSkillSymlinkTargetRealPaths(config);
115119
const signature = JSON.stringify({
116120
basePaths: baseRoots.map((root) => toWatchRoot(root.path)),
117121
extraDirs: extraDirs.map(toWatchRoot),
@@ -365,23 +369,6 @@ function watchDepthForPath(raw: string, depth: number): number {
365369
return depth + missingSegments;
366370
}
367371

368-
function resolveAllowedSymlinkTargetRealPaths(config?: OpenClawConfig): string[] {
369-
const rawTargets = config?.skills?.load?.allowSymlinkTargets ?? [];
370-
return rawTargets
371-
.map((dir) => normalizeOptionalString(dir) ?? "")
372-
.filter(Boolean)
373-
.map((dir) => tryRealpath(resolveUserPath(dir)))
374-
.filter((dir): dir is string => Boolean(dir));
375-
}
376-
377-
function tryRealpath(filePath: string): string | null {
378-
try {
379-
return fs.realpathSync(filePath);
380-
} catch {
381-
return null;
382-
}
383-
}
384-
385372
function isPathInside(parent: string, child: string): boolean {
386373
const relative = path.relative(parent, child);
387374
return (

‎src/skills/workshop/service.test.ts‎

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,75 @@ describe("skill workshop proposals", () => {
138138
expect((await inspectSkillProposal(proposal.record.id))?.record.status).toBe("applied");
139139
});
140140

141+
it.runIf(process.platform !== "win32")(
142+
"applies updates through trusted workspace skills symlink targets",
143+
async () => {
144+
const workspaceDir = await makeWorkspace();
145+
const targetSkillsDir = await tempDirs.make("openclaw-skill-workshop-target-skills-");
146+
await fs.symlink(targetSkillsDir, path.join(workspaceDir, "skills"), "dir");
147+
const skillDir = path.join(targetSkillsDir, "shared-skill");
148+
await writeSkill({
149+
dir: skillDir,
150+
name: "shared-skill",
151+
description: "Shared skill target",
152+
body: "# Shared Skill\n\nOld body.\n",
153+
});
154+
const config = { skills: { load: { allowSymlinkTargets: [targetSkillsDir] } } };
155+
const proposal = await proposeUpdateSkill({
156+
workspaceDir,
157+
config,
158+
skillName: "shared-skill",
159+
content: "# Shared Skill\n\nNew body.\n",
160+
});
161+
162+
const applied = await applySkillProposal({
163+
workspaceDir,
164+
config,
165+
proposalId: proposal.record.id,
166+
});
167+
168+
expect(applied.targetSkillFile).toBe(
169+
path.join(workspaceDir, "skills", "shared-skill", "SKILL.md"),
170+
);
171+
await expect(fs.readFile(path.join(skillDir, "SKILL.md"), "utf8")).resolves.toContain(
172+
"New body.",
173+
);
174+
},
175+
);
176+
177+
it.runIf(process.platform !== "win32")(
178+
"blocks untrusted workspace skills symlink targets before support files are written",
179+
async () => {
180+
const workspaceDir = await makeWorkspace();
181+
const targetSkillsDir = await tempDirs.make("openclaw-skill-workshop-untrusted-skills-");
182+
await fs.symlink(targetSkillsDir, path.join(workspaceDir, "skills"), "dir");
183+
const proposal = await proposeCreateSkill({
184+
workspaceDir,
185+
name: "Untrusted Symlink Skill",
186+
description: "Must not write through an untrusted symlink",
187+
content: "# Untrusted\n\nDo not write.\n",
188+
supportFiles: [
189+
{
190+
path: "references/details.md",
191+
content: "This support file must not be written.\n",
192+
},
193+
],
194+
});
195+
196+
await expect(
197+
applySkillProposal({ workspaceDir, proposalId: proposal.record.id }),
198+
).rejects.toThrow("untrusted symlink target");
199+
await expect(
200+
fs.access(path.join(targetSkillsDir, "untrusted-symlink-skill", "SKILL.md")),
201+
).rejects.toThrow();
202+
await expect(
203+
fs.access(
204+
path.join(targetSkillsDir, "untrusted-symlink-skill", "references", "details.md"),
205+
),
206+
).rejects.toThrow();
207+
},
208+
);
209+
141210
it("preserves non-proposal frontmatter when proposals become active skills", async () => {
142211
const workspaceDir = await makeWorkspace();
143212
const created = await proposeCreateSkill({

‎src/skills/workshop/service.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import {
1010
resolveSkillStatusEntry,
1111
type SkillStatusEntry,
1212
} from "../discovery/status.js";
13+
import { resolveAllowedSkillSymlinkTargetRealPaths } from "../loading/symlink-targets.js";
1314
import { bumpSkillsSnapshotVersion } from "../runtime/refresh-state.js";
1415
import { scanSkillContent, scanSource } from "../security/scanner.js";
1516
import { resolveSkillWorkshopConfig, type SkillWorkshopConfig } from "./config.js";
@@ -20,6 +21,7 @@ import {
2021
} from "./frontmatter.js";
2122
import {
2223
assertInsideWorkspace,
24+
assertWorkspaceSkillWriteTarget,
2325
createSkillProposalId,
2426
createSkillProposalRollback,
2527
hashSkillProposalContent,
@@ -530,6 +532,12 @@ export async function applySkillProposal(
530532

531533
assertInsideWorkspace(input.workspaceDir, record.target.skillFile, "skill file");
532534
assertInsideWorkspace(input.workspaceDir, record.target.skillDir, "skill directory");
535+
const allowedSymlinkTargetRealPaths = resolveAllowedSkillSymlinkTargetRealPaths(input.config);
536+
await assertWorkspaceSkillWriteTarget({
537+
workspaceDir: input.workspaceDir,
538+
filePath: record.target.skillFile,
539+
allowedSymlinkTargetRealPaths,
540+
});
533541
const targetState = await readApplyTargetState(record, supportFiles);
534542
const rollback = createSkillProposalRollback({
535543
proposalId: record.id,
@@ -554,6 +562,7 @@ export async function applySkillProposal(
554562
skillContent,
555563
supportFiles,
556564
previousSupportFiles: targetState.previousSupportFiles,
565+
allowedSymlinkTargetRealPaths,
557566
});
558567
bumpSkillsSnapshotVersion({
559568
workspaceDir: input.workspaceDir,
@@ -646,6 +655,7 @@ async function publishProposalTarget(params: {
646655
skillContent: string;
647656
supportFiles: readonly PreparedSkillProposalSupportFile[];
648657
previousSupportFiles: NonNullable<SkillProposalRollback["supportFiles"]>;
658+
allowedSymlinkTargetRealPaths: readonly string[];
649659
}): Promise<void> {
650660
const writtenSupportPaths: string[] = [];
651661
try {
@@ -663,6 +673,7 @@ async function publishProposalTarget(params: {
663673
filePath: params.record.target.skillFile,
664674
content: params.skillContent,
665675
overwrite: params.record.kind === "update",
676+
allowedSymlinkTargetRealPaths: params.allowedSymlinkTargetRealPaths,
666677
});
667678
} catch (error) {
668679
if (params.record.kind === "create") {

0 commit comments

Comments
 (0)