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

Minh Nguyen · Minh Nguyen · commit 0daf50308af1 · 2026-04-03T01:18:24.000-07:00
The audit probe checked against the workspace root, but the skill loader
(resolveContainedSkillPath) checks against the skills directory root.
Symlinks resolving inside the workspace but outside workspace/skills/
passed audit but were silently rejected at load time.

Align the audit boundary with the loader so these cases are flagged.
diff --git a/src/security/audit-extra.async.ts b/src/security/audit-extra.async.ts
@@ -847,7 +847,9 @@ export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
 
   for (const workspaceDir of workspaceDirs) {
     const workspacePath = path.resolve(workspaceDir);
-    const workspaceRealPath = await fs.realpath(workspacePath).catch(() => workspacePath);
+    // Match the skill loader boundary (resolveContainedSkillPath): skills dir root, not workspace root.
+    const skillsDir = path.join(workspacePath, "skills");
+    const skillsDirRealPath = await fs.realpath(skillsDir).catch(() => skillsDir);
     const skillFilePaths = await listWorkspaceSkillMarkdownFiles(workspacePath);
 
     for (const skillFilePath of skillFilePaths) {
@@ -861,7 +863,7 @@ export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
       if (!skillRealPath) {
         continue;
       }
-      if (isPathInside(workspaceRealPath, skillRealPath)) {
+      if (isPathInside(skillsDirRealPath, skillRealPath)) {
         continue;
       }
       escapedSkillFiles.push({
@@ -879,9 +881,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 the skills directory root:\n" +
       escapedSkillFiles
         .slice(0, MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS)
         .map(
@@ -895,7 +897,7 @@ export async function collectWorkspaceSkillSymlinkEscapeFindings(params: {
         ? `\n- +${escapedSkillFiles.length - MAX_WORKSPACE_SKILL_ESCAPE_DETAIL_ROWS} more`
         : ""),
     remediation:
-      "Keep workspace skills inside the workspace root (replace symlinked escapes with real in-workspace files), or move trusted shared skills to managed/bundled skill locations.",
+      "Keep workspace skills inside the skills directory (replace symlinked escapes with real in-workspace files), or move trusted shared skills to managed/bundled skill locations or `skills.load.extraDirs`.",
   });
 
   return findings;
diff --git a/src/security/audit.test.ts b/src/security/audit.test.ts
@@ -1359,7 +1359,7 @@ description: test skill
         },
       },
       {
-        name: "does not warn for workspace skills that stay inside workspace root",
+        name: "does not warn for workspace skills that stay inside skills directory",
         supported: true,
         setup: async () => {
           const tmp = await makeTmpDir("workspace-skill-in-root");
@@ -1378,6 +1378,35 @@ description: test skill
           expectNoFinding(res, "skills.workspace.symlink_escape");
         },
       },
+      {
+        name: "warns when skill symlink resolves inside workspace but outside skills directory",
+        supported: !isWindows,
+        setup: async () => {
+          const tmp = await makeTmpDir("workspace-skill-inside-ws-outside-skills");
+          const stateDir = path.join(tmp, "state");
+          const workspaceDir = path.join(tmp, "workspace");
+          const sharedSkillDir = path.join(workspaceDir, "shared", "skills", "my-skill");
+          await fs.mkdir(stateDir, { recursive: true, mode: 0o700 });
+          await fs.mkdir(path.join(workspaceDir, "skills", "linked-skill"), { recursive: true });
+          await fs.mkdir(sharedSkillDir, { recursive: true });
+          await fs.writeFile(
+            path.join(sharedSkillDir, "SKILL.md"),
+            "# inside workspace, outside skills dir\n",
+            "utf-8",
+          );
+          // Symlink: skills/linked-skill/SKILL.md -> ../../shared/skills/my-skill/SKILL.md
+          await fs.symlink(
+            path.join(sharedSkillDir, "SKILL.md"),
+            path.join(workspaceDir, "skills", "linked-skill", "SKILL.md"),
+          );
+          return { stateDir, workspaceDir };
+        },
+        assert: (res: SecurityAuditReport) => {
+          const finding = res.findings.find((f) => f.checkId === "skills.workspace.symlink_escape");
+          expect(finding).toBeDefined();
+          expect(finding!.severity).toBe("warn");
+        },
+      },
     ] as const;
 
     await runAuditCases(
