mirror of
https://github.com/openclaw/openclaw.git
synced 2026-05-18 17:14:45 +00:00
fix(skills): trust managed skill symlink roots
This commit is contained in:
committed by
Peter Steinberger
parent
f8ae0fb1c4
commit
cca4f3c63c
@@ -135,6 +135,7 @@ Docs: https://docs.openclaw.ai
|
||||
- Control UI: keep shared form, config, and usage text-entry controls at 16px on touch-primary devices while preserving chat composer input sizing, so iOS Safari no longer auto-zooms focused fields. Fixes #64651; carries forward #64673. Thanks @NianJiuZst and @BunsDev.
|
||||
- Codex harness: classify native app-server token-refresh logout and relogin failures as authentication refresh errors, so users get re-authentication guidance instead of a raw runtime failure.
|
||||
- Agents/trajectory: make the trajectory flush cleanup timeout configurable with `OPENCLAW_TRAJECTORY_FLUSH_TIMEOUT_MS`, preserving the 10s default while slower stores drain. Refs #75839. Thanks @BunsDev.
|
||||
- Skills: load ClawHub and local-manager skill-directory symlinks from managed `~/.openclaw/skills` and personal `~/.agents/skills` roots while keeping workspace, extra, bundled, and per-skill `SKILL.md` containment fail-closed. Fixes #44051. Refs #59219. Thanks @Devattom, @ArthurNie, and @luoxiao6645.
|
||||
- Codex startup: treat selectable configured OpenAI agent models as Codex runtime requirements during plugin auto-enable, startup planning, and doctor install repair, so Anthropic-primary configs can still switch to OpenAI/Codex cleanly.
|
||||
- Agents: preserve source-reply delivery metadata when merging tool-returned media into the final reply, keeping message-tool-only replies deliverable and mirrored. Thanks @pashpashpash and @vincentkoc.
|
||||
- Replies: treat rich presentation, interactive controls, and channel-native payload data as outbound content across follow-up, heartbeat, cron, ACP, and block-streaming delivery paths, preventing card/button-only replies from being dropped as empty.
|
||||
|
||||
@@ -90,9 +90,13 @@ Rules:
|
||||
bundled skills in the list are eligible (managed, agent, and workspace skills unaffected).
|
||||
- `load.extraDirs`: additional skill directories to scan (lowest precedence).
|
||||
- `load.allowSymlinkTargets`: trusted real target directories that symlinked
|
||||
skill folders may resolve into even when the symlink lives outside that
|
||||
target root. Use this for intentional sibling-repo layouts such as
|
||||
`~/.agents/skills/manager -> ~/Projects/manager/skills`.
|
||||
workspace, project-agent, or extra-dir skill folders may resolve into even
|
||||
when the symlink lives outside that target root. Use this for intentional
|
||||
sibling-repo layouts such as
|
||||
`<workspace>/skills/manager -> ~/Projects/manager/skills`. Managed
|
||||
`~/.openclaw/skills` and personal `~/.agents/skills` roots may follow
|
||||
skill-directory symlinks from local skill managers by default, but every
|
||||
`SKILL.md` still has to resolve inside its own skill directory.
|
||||
- `load.watch`: watch skill folders and refresh the skills snapshot (default: true).
|
||||
- `load.watchDebounceMs`: debounce for skill watcher events in milliseconds (default: 250).
|
||||
- `install.preferBrew`: prefer brew installers when available (default: true).
|
||||
@@ -114,10 +118,10 @@ Rules:
|
||||
|
||||
## Symlinked sibling repos
|
||||
|
||||
By default, each skill root is a containment boundary. If a skill folder under
|
||||
`~/.agents/skills` is a symlink that resolves outside `~/.agents/skills`,
|
||||
OpenClaw skips it and logs `Skipping escaped skill path outside its configured
|
||||
root`.
|
||||
By default, workspace, project-agent, extra-dir, and bundled skill roots are
|
||||
containment boundaries. If a skill folder under `<workspace>/skills` is a
|
||||
symlink that resolves outside `<workspace>/skills`, OpenClaw skips it and logs
|
||||
`Skipping escaped skill path outside its configured root`.
|
||||
|
||||
Keep the symlink layout and allow only the trusted target root:
|
||||
|
||||
@@ -133,10 +137,13 @@ Keep the symlink layout and allow only the trusted target root:
|
||||
```
|
||||
|
||||
With this config, a symlink such as
|
||||
`~/.agents/skills/manager -> ~/Projects/manager/skills` is accepted after
|
||||
`<workspace>/skills/manager -> ~/Projects/manager/skills` is accepted after
|
||||
realpath resolution. `extraDirs` also scans the sibling repo directly, while
|
||||
`allowSymlinkTargets` preserves the symlinked path for existing agent-skill
|
||||
layouts. Keep target entries narrow; do not point at broad roots such as `~` or
|
||||
`allowSymlinkTargets` preserves the symlinked path for existing workspace-skill
|
||||
layouts. Managed `~/.openclaw/skills` and personal `~/.agents/skills`
|
||||
directories already accept skill-directory symlinks because those roots are
|
||||
user-owned local skill-manager surfaces; per-skill `SKILL.md` containment still
|
||||
applies. Keep target entries narrow; do not point at broad roots such as `~` or
|
||||
`~/Projects` unless every skill tree under that root is trusted.
|
||||
|
||||
Per-skill fields:
|
||||
|
||||
@@ -170,7 +170,7 @@ Prefer sandboxed runs for untrusted inputs and risky tools. See
|
||||
[Sandboxing](/gateway/sandboxing) for the agent-side controls.
|
||||
</Warning>
|
||||
|
||||
- Workspace and extra-dir skill discovery only accepts skill roots and `SKILL.md` files whose resolved realpath stays inside the configured root.
|
||||
- Workspace, project-agent, and extra-dir skill discovery only accepts skill roots whose resolved realpath stays inside the configured root unless `skills.load.allowSymlinkTargets` explicitly trusts a target root. Bundled skills always stay contained. Managed `~/.openclaw/skills` and personal `~/.agents/skills` roots may contain symlinked skill folders installed by ClawHub or another local skill manager, but every `SKILL.md` realpath must still stay inside its resolved skill directory.
|
||||
- Gateway private archive installs are off by default. When explicitly enabled,
|
||||
they require a committed zip upload containing `SKILL.md` and reuse the same
|
||||
archive extraction, path traversal, symlink, force, and rollback protections as
|
||||
@@ -448,10 +448,12 @@ when `SKILL.md` files change. Configure under `skills.load`:
|
||||
}
|
||||
```
|
||||
|
||||
Use `allowSymlinkTargets` for intentional sibling-repo layouts where a built-in
|
||||
skill root contains a symlink, for example
|
||||
`~/.agents/skills/manager -> ~/Projects/manager/skills`. The target list is
|
||||
matched after realpath resolution and should stay narrow.
|
||||
Use `allowSymlinkTargets` for intentional workspace, project-agent, or extra-dir
|
||||
layouts where a skill root contains a symlink, for example
|
||||
`<workspace>/skills/manager -> ~/Projects/manager/skills`. Managed
|
||||
`~/.openclaw/skills` and personal `~/.agents/skills` can follow skill-directory
|
||||
symlinks from local skill managers by default, but the target list is still
|
||||
matched after realpath resolution and should stay narrow when configured.
|
||||
|
||||
### Remote macOS nodes (Linux gateway)
|
||||
|
||||
|
||||
@@ -395,9 +395,9 @@ describe("loadWorkspaceSkillEntries", () => {
|
||||
name: skillName,
|
||||
description: "Manager skill",
|
||||
});
|
||||
const personalSkillsDir = path.join(fakeHome, ".agents", "skills");
|
||||
await fs.mkdir(personalSkillsDir, { recursive: true });
|
||||
const symlinkPath = path.join(personalSkillsDir, skillName);
|
||||
const workspaceSkillsDir = path.join(workspaceDir, "skills");
|
||||
await fs.mkdir(workspaceSkillsDir, { recursive: true });
|
||||
const symlinkPath = path.join(workspaceSkillsDir, skillName);
|
||||
await fs.symlink(targetSkillDir, symlinkPath, "dir");
|
||||
const warn = captureWarningLogger();
|
||||
|
||||
@@ -420,6 +420,73 @@ describe("loadWorkspaceSkillEntries", () => {
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"loads managed skill directory symlinks outside the managed root",
|
||||
async () => {
|
||||
const workspaceDir = await createTempWorkspaceDir();
|
||||
const managedDir = path.join(workspaceDir, ".managed");
|
||||
const skillName = `managed-${++workspaceCaseIndex}`;
|
||||
const targetSkillDir = path.join(tempRoot, `${skillName}-target`, skillName);
|
||||
await writeSkill({
|
||||
dir: targetSkillDir,
|
||||
name: skillName,
|
||||
description: "Managed symlink target",
|
||||
});
|
||||
await fs.mkdir(managedDir, { recursive: true });
|
||||
const symlinkPath = path.join(managedDir, skillName);
|
||||
await fs.symlink(targetSkillDir, symlinkPath, "dir");
|
||||
const warn = captureWarningLogger();
|
||||
|
||||
try {
|
||||
const entries = loadTestWorkspaceSkillEntries(workspaceDir, {
|
||||
managedSkillsDir: managedDir,
|
||||
});
|
||||
|
||||
expect(entries.map((entry) => entry.skill.name)).toContain(skillName);
|
||||
expect(warn).not.toHaveBeenCalled();
|
||||
} finally {
|
||||
await fs.unlink(symlinkPath).catch(() => undefined);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"keeps SKILL.md containment for managed symlinked skill directories",
|
||||
async () => {
|
||||
const workspaceDir = await createTempWorkspaceDir();
|
||||
const managedDir = path.join(workspaceDir, ".managed");
|
||||
const skillName = `managed-file-link-${++workspaceCaseIndex}`;
|
||||
const targetSkillDir = path.join(tempRoot, `${skillName}-target`, skillName);
|
||||
const outsideDir = path.join(tempRoot, `${skillName}-outside`);
|
||||
await fs.mkdir(targetSkillDir, { recursive: true });
|
||||
await fs.mkdir(outsideDir, { recursive: true });
|
||||
await writeSkill({
|
||||
dir: outsideDir,
|
||||
name: skillName,
|
||||
description: "Escaped metadata",
|
||||
});
|
||||
await fs.symlink(path.join(outsideDir, "SKILL.md"), path.join(targetSkillDir, "SKILL.md"));
|
||||
await fs.mkdir(managedDir, { recursive: true });
|
||||
const symlinkPath = path.join(managedDir, skillName);
|
||||
await fs.symlink(targetSkillDir, symlinkPath, "dir");
|
||||
const warn = captureWarningLogger();
|
||||
|
||||
try {
|
||||
const entries = loadTestWorkspaceSkillEntries(workspaceDir, {
|
||||
managedSkillsDir: managedDir,
|
||||
});
|
||||
|
||||
expect(entries.map((entry) => entry.skill.name)).not.toContain(skillName);
|
||||
const warningLine = firstWarningLine(warn);
|
||||
expect(warningLine).toContain("Skipping escaped skill path outside its configured root:");
|
||||
expect(warningLine).toContain("source=openclaw-managed");
|
||||
expect(warningLine).toContain("reason=symlink-escape");
|
||||
} finally {
|
||||
await fs.unlink(symlinkPath).catch(() => undefined);
|
||||
}
|
||||
},
|
||||
);
|
||||
|
||||
it.runIf(process.platform !== "win32")(
|
||||
"calls out bundled symlink escapes with compact home-relative paths",
|
||||
async () => {
|
||||
|
||||
@@ -404,6 +404,53 @@ function isPathInsideAnyRoot(rootRealPaths: readonly string[], candidateRealPath
|
||||
return rootRealPaths.some((rootRealPath) => isPathInside(rootRealPath, candidateRealPath));
|
||||
}
|
||||
|
||||
function shouldEnforceConfiguredSkillRootContainment(source: string): boolean {
|
||||
return source !== "openclaw-managed" && source !== "agents-skills-personal";
|
||||
}
|
||||
|
||||
function shouldUseConfiguredSymlinkTargets(source: string): boolean {
|
||||
return (
|
||||
source === "openclaw-workspace" ||
|
||||
source === "openclaw-extra" ||
|
||||
source === "agents-skills-project"
|
||||
);
|
||||
}
|
||||
|
||||
function resolveSkillRootCandidatePath(params: {
|
||||
source: string;
|
||||
rootDir: string;
|
||||
rootRealPath: string;
|
||||
candidatePath: string;
|
||||
allowedSymlinkTargetRealPaths: readonly string[];
|
||||
}): string | null {
|
||||
if (!shouldEnforceConfiguredSkillRootContainment(params.source)) {
|
||||
return tryRealpath(params.candidatePath);
|
||||
}
|
||||
return resolveContainedSkillPath({
|
||||
source: params.source,
|
||||
rootDir: params.rootDir,
|
||||
rootRealPath: params.rootRealPath,
|
||||
candidatePath: params.candidatePath,
|
||||
allowedSymlinkTargetRealPaths: shouldUseConfiguredSymlinkTargets(params.source)
|
||||
? params.allowedSymlinkTargetRealPaths
|
||||
: [],
|
||||
});
|
||||
}
|
||||
|
||||
function resolveSkillFilePath(params: {
|
||||
source: string;
|
||||
skillDir: string;
|
||||
skillDirRealPath: string;
|
||||
candidatePath: string;
|
||||
}): string | null {
|
||||
return resolveContainedSkillPath({
|
||||
source: params.source,
|
||||
rootDir: params.skillDir,
|
||||
rootRealPath: params.skillDirRealPath,
|
||||
candidatePath: params.candidatePath,
|
||||
});
|
||||
}
|
||||
|
||||
function resolvePluginSkillRootRealPaths(pluginSkillDirs: readonly string[]): string[] {
|
||||
return pluginSkillDirs
|
||||
.map((dir) => tryRealpath(dir))
|
||||
@@ -535,7 +582,7 @@ function loadSkillEntries(
|
||||
maxEntriesToScan: limits.maxCandidatesPerRoot,
|
||||
});
|
||||
const baseDir = resolved.baseDir;
|
||||
const baseDirRealPath = resolveContainedSkillPath({
|
||||
const baseDirRealPath = resolveSkillRootCandidatePath({
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath,
|
||||
@@ -549,12 +596,11 @@ function loadSkillEntries(
|
||||
// If the root itself is a skill directory, just load it directly (but enforce size cap).
|
||||
const rootSkillMd = path.join(baseDir, "SKILL.md");
|
||||
if (fs.existsSync(rootSkillMd)) {
|
||||
const rootSkillRealPath = resolveContainedSkillPath({
|
||||
const rootSkillRealPath = resolveSkillFilePath({
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
skillDir: baseDir,
|
||||
skillDirRealPath: baseDirRealPath,
|
||||
candidatePath: rootSkillMd,
|
||||
allowedSymlinkTargetRealPaths,
|
||||
});
|
||||
if (!rootSkillRealPath) {
|
||||
return [];
|
||||
@@ -642,7 +688,7 @@ function loadSkillEntries(
|
||||
// skill directories (e.g. ~/.openclaw/skills/coze/koze-retrieval/SKILL.md).
|
||||
for (const name of limitedChildren) {
|
||||
const skillDir = path.join(baseDir, name);
|
||||
const skillDirRealPath = resolveContainedSkillPath({
|
||||
const skillDirRealPath = resolveSkillRootCandidatePath({
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
@@ -654,12 +700,11 @@ function loadSkillEntries(
|
||||
}
|
||||
const skillMd = path.join(skillDir, "SKILL.md");
|
||||
if (fs.existsSync(skillMd)) {
|
||||
const skillMdRealPath = resolveContainedSkillPath({
|
||||
const skillMdRealPath = resolveSkillFilePath({
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
skillDir,
|
||||
skillDirRealPath,
|
||||
candidatePath: skillMd,
|
||||
allowedSymlinkTargetRealPaths,
|
||||
});
|
||||
if (skillMdRealPath) {
|
||||
loadCandidateSkill({ skillDir, name, skillMdRealPath });
|
||||
@@ -701,20 +746,21 @@ function loadSkillEntries(
|
||||
const nestedDir = path.join(skillDir, nestedName);
|
||||
const nestedSkillMd = path.join(nestedDir, "SKILL.md");
|
||||
if (fs.existsSync(nestedSkillMd)) {
|
||||
const nestedDirRealPath = resolveContainedSkillPath({
|
||||
const nestedDirRealPath = resolveSkillRootCandidatePath({
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
candidatePath: nestedDir,
|
||||
allowedSymlinkTargetRealPaths,
|
||||
});
|
||||
const nestedSkillMdRealPath = resolveContainedSkillPath({
|
||||
source: params.source,
|
||||
rootDir,
|
||||
rootRealPath: baseDirRealPath,
|
||||
candidatePath: nestedSkillMd,
|
||||
allowedSymlinkTargetRealPaths,
|
||||
});
|
||||
const nestedSkillMdRealPath = nestedDirRealPath
|
||||
? resolveSkillFilePath({
|
||||
source: params.source,
|
||||
skillDir: nestedDir,
|
||||
skillDirRealPath: nestedDirRealPath,
|
||||
candidatePath: nestedSkillMd,
|
||||
})
|
||||
: null;
|
||||
if (nestedDirRealPath && nestedSkillMdRealPath) {
|
||||
loadCandidateSkill({
|
||||
skillDir: nestedDir,
|
||||
|
||||
Reference in New Issue
Block a user