fix(security): align audit symlink_escape boundary with skill loader

carnie[bot] · carnie[bot] · commit bf84e3a065d3 · 2026-04-03T13:17:02.000-07:00
The skills.workspace.symlink_escape audit probe checked whether skill file realpaths escaped the workspace root (~/.openclaw/workspace/), but the skill loader (resolveContainedSkillPath) checks against the skills directory root (~/.openclaw/workspace/skills/). This mismatch meant symlinks like: workspace/skills/my-skill -> workspace/other-dir/skills/my-skill ...would resolve inside the workspace root (audit says OK) but outside the skills directory (loader silently rejects). The skill would fail to load with zero feedback from `openclaw security audit`. Fix: check against the skills directory realpath in the audit probe, matching the loader's boundary. Add test for the in-workspace but outside-skills-dir case. Relates to #49408
diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml
diff --git a/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts
@@ -881,7 +881,8 @@ export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
 
   for (const workspaceDir of workspaceDirs) {
     const workspacePath = path.resolve(workspaceDir);
-    const workspaceRealPath = await fs.realpath(workspacePath).catch(() => workspacePath);
+    const skillsDirPath = path.join(workspacePath, "skills");
+    const skillsDirRealPath = await fs.realpath(skillsDirPath).catch(() => skillsDirPath);
     const skillFilePaths = await listWorkspaceSkillMarkdownFiles(workspacePath);
 
     for (const skillFilePath of skillFilePaths) {
@@ -895,7 +896,7 @@ export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
       if (!skillRealPath) {
         continue;
       }
-      if (isPathInside(workspaceRealPath, skillRealPath)) {
+      if (isPathInside(skillsDirRealPath, skillRealPath)) {
         continue;
       }
       escapedSkillFiles.push({
@@ -913,9 +914,9 @@ export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
   findings.push({
     checkId: "skills.workspace.symlink_escape",
     severity: "warn",
-    title: "Workspace skill files resolve outside the workspace root",
+    title: "Workspace skill files resolve outside the skills directory",
     detail:
-      "Detected workspace `skills/**/SKILL.md` paths whose realpath escapes their workspace root:\n" +
+      "Detected workspace `skills/**/SKILL.md` paths whose realpath escapes their skills directory:\n" +
       escapedSkillFiles
         .slice(0, MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS)
         .map(
diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts
@@ -1391,7 +1391,35 @@ describe("security audit", () => {
         },
       },
       {
-        name: "does not warn for workspace skills that stay inside workspace root",
+        name: "warns when workspace skill resolves inside workspace but outside skills dir",
+        supported: !isWindows,
+        setup: async () => {
+          const tmp = await makeTmpDir("workspace-skill-in-root-outside-skills");
+          const stateDir = path.join(tmp, "state");
+          const workspaceDir = path.join(tmp, "workspace");
+          const otherDir = path.join(workspaceDir, "other-dir");
+          await fs.mkdir(stateDir, { recursive: true, mode: 0o700 });
+          await fs.mkdir(path.join(workspaceDir, "skills", "leak"), { recursive: true });
+          await fs.mkdir(otherDir, { recursive: true });
+
+          const otherSkillPath = path.join(otherDir, "SKILL.md");
+          await fs.writeFile(otherSkillPath, "# inside workspace, outside skills dir\n", "utf-8");
+          await fs.symlink(otherSkillPath, path.join(workspaceDir, "skills", "leak", "SKILL.md"));
+
+          return { stateDir, workspaceDir, otherSkillPath };
+        },
+        assert: (
+          res: SecurityAuditReport,
+          fixture: { stateDir: string; workspaceDir: string; otherSkillPath?: string },
+        ) => {
+          const finding = res.findings.find((f) => f.checkId === "skills.workspace.symlink_escape");
+          expect(finding?.severity).toBe("warn");
+          expect(fixture.otherSkillPath).toBeTruthy();
+          expect(finding?.detail).toContain(fixture.otherSkillPath ?? "");
+        },
+      },
+      {
+        name: "does not warn for workspace skills that stay inside skills dir",
         supported: true,
         setup: async () => {
           const tmp = await makeTmpDir("workspace-skill-in-root");
