fix(sync): harden skill scanning against bad paths, reject filesystem-root plugin sources

src/cli/commands/plugin.ts

@@ -224,6 +224,12 @@ async function runUserSyncAndPrint(): Promise<{ ok: boolean; syncData: ReturnTyp
       }
     }
 
+    if (result.warnings && result.warnings.length > 0) {
+      console.log('\nWarnings:');
+      for (const warning of result.warnings) {
+        console.log(`  ⚠ ${warning}`);
+      }
+    }
   }
 
   return { ok: result.success && result.totalFailed === 0, syncData };

src/core/skills.ts

@@ -1,4 +1,4 @@
-import { existsSync } from 'node:fs';
+import { existsSync, lstatSync, type Dirent } from 'node:fs';
 import { readFile, readdir } from 'node:fs/promises';
 import { basename, join, relative, resolve } from 'node:path';
 import { load } from 'js-yaml';
@@ -101,28 +101,50 @@ async function resolvePluginPath(
 
 export async function discoverNestedSkillEntries(
   scanRoot: string,
+  warnings: string[] = [],
 ): Promise<DiscoveredSkillEntry[]> {
-  return walkForSkillMd(scanRoot, scanRoot);
+  return walkForSkillMd(scanRoot, scanRoot, warnings);
 }
 
 async function walkForSkillMd(
   scanRoot: string,
   currentDir: string,
+  warnings: string[],
 ): Promise<DiscoveredSkillEntry[]> {
-  const entries = await readdir(currentDir, { withFileTypes: true });
+  let entries: Dirent[];
+  try {
+    entries = await readdir(currentDir, { withFileTypes: true });
+  } catch (err) {
+    const code =
+      err && typeof err === 'object' && 'code' in err
+        ? String((err as NodeJS.ErrnoException).code)
+        : undefined;
+    warnings.push(
+      `Could not read '${currentDir}'${code ? ` (${code})` : ''} while scanning for skills. If this path looks unexpected (e.g. rooted near a drive root), check ~/.allagents/workspace.yaml for a stale plugin path, or rename ~/.allagents to ~/.allagents.bak and reinstall your plugins to reset it.`,
+    );
+    return [];
+  }
   const discovered: DiscoveredSkillEntry[] = [];
 
   for (const entry of entries) {
     if (!entry.isDirectory()) continue;
 
     const skillPath = join(currentDir, entry.name);
+
+    // Skip symlinks/junctions to avoid unbounded recursion through loops.
+    try {
+      if (lstatSync(skillPath).isSymbolicLink()) continue;
+    } catch {
+      continue;
+    }
+
     if (existsSync(join(skillPath, 'SKILL.md'))) {
       const subpath = relative(scanRoot, skillPath).split(/[\\/]/).join('/');
       discovered.push({ name: entry.name, subpath, skillPath });
       continue;
     }
 
-    discovered.push(...(await walkForSkillMd(scanRoot, skillPath)));
+    discovered.push(...(await walkForSkillMd(scanRoot, skillPath, warnings)));
   }
 
   return discovered;

src/core/sync.ts

@@ -1098,10 +1098,19 @@ export function computeDeletedArtifacts(
  */
 async function collectAvailableSkillNames(
   validPlugins: ValidatedPlugin[],
+  warnings?: string[],
 ): Promise<Set<string>> {
   const names = new Set<string>();
   for (const plugin of validPlugins) {
-    const skills = await collectPluginSkills(plugin.resolved, plugin.plugin);
+    const skills = await collectPluginSkills(
+      plugin.resolved,
+      plugin.plugin,
+      undefined,
+      undefined,
+      undefined,
+      undefined,
+      warnings,
+    );
     for (const skill of skills) {
       names.add(skill.folderName);
     }
@@ -1458,6 +1467,7 @@ async function collectAllSkills(
   validatedPlugins: ValidatedPlugin[],
   disabledSkills?: Set<string>,
   enabledSkills?: Set<string>,
+  warnings?: string[],
 ): Promise<CollectedSkillEntry[]> {
   const allSkills: CollectedSkillEntry[] = [];
 
@@ -1470,6 +1480,7 @@ async function collectAllSkills(
       pluginName,
       enabledSkills,
       plugin.pluginSkillsConfig,
+      warnings,
     );
 
     for (const skill of skills) {
@@ -2154,7 +2165,7 @@ export async function syncWorkspace(
       ? new Set(config.enabledSkills)
       : undefined;
   const allSkills = await sw.measure('skill-collection', () =>
-    collectAllSkills(validPlugins, disabledSkillsSet, enabledSkillsSet),
+    collectAllSkills(validPlugins, disabledSkillsSet, enabledSkillsSet, warnings),
   );
 
   // Build per-plugin skill name maps (handles conflicts automatically)
@@ -2373,7 +2384,10 @@ export async function syncWorkspace(
   // Compute deleted artifacts: compare previous state vs what was just synced
   // Collect all skill names from installed plugins (including disabled) so that
   // skills that are still available but just not synced are not reported as deleted.
-  const availableSkillNames = await collectAvailableSkillNames(validPlugins);
+  const availableSkillNames = await collectAvailableSkillNames(
+    validPlugins,
+    warnings,
+  );
   const allCopyResultsForState = [
     ...pluginResults.flatMap((r) => r.copyResults),
     ...workspaceFileResults,
@@ -2428,6 +2442,7 @@ export async function syncWorkspace(
     );
   }
 
+  const uniqueWarnings = [...new Set(warnings)];
   return {
     success: !hasFailures,
     pluginResults,
@@ -2437,7 +2452,7 @@ export async function syncWorkspace(
     totalGenerated,
     purgedPaths,
     ...(deletedArtifacts.length > 0 && { deletedArtifacts }),
-    ...(warnings.length > 0 && { warnings }),
+    ...(uniqueWarnings.length > 0 && { warnings: uniqueWarnings }),
     ...(messages.length > 0 && { messages }),
     ...(Object.keys(mcpResults).length > 0 && { mcpResults }),
     ...(nativeResult && { nativeResult }),
@@ -2577,7 +2592,7 @@ export async function syncUserWorkspace(
       ? new Set(config.enabledSkills)
       : undefined;
   const allSkills = await sw.measure('skill-collection', () =>
-    collectAllSkills(validPlugins, disabledSkillsSet, enabledSkillsSet),
+    collectAllSkills(validPlugins, disabledSkillsSet, enabledSkillsSet, warnings),
   );
   const pluginSkillMaps = buildPluginSkillNameMaps(allSkills);
 
@@ -2751,8 +2766,10 @@ export async function syncUserWorkspace(
   );
 
   // Compute deleted artifacts: compare previous state vs what was just synced
-  const availableUserSkillNames =
-    await collectAvailableSkillNames(validPlugins);
+  const availableUserSkillNames = await collectAvailableSkillNames(
+    validPlugins,
+    warnings,
+  );
   const allCopyResultsForState = pluginResults.flatMap((r) => r.copyResults);
   const resolvedUserMappings = resolveClientMappings(
     syncClients,
@@ -2799,6 +2816,7 @@ export async function syncUserWorkspace(
     );
   }
 
+  const uniqueWarnings = [...new Set(warnings)];
   return {
     success: totalFailed === 0,
     pluginResults,
@@ -2807,7 +2825,7 @@ export async function syncUserWorkspace(
     totalSkipped,
     totalGenerated,
     ...(deletedArtifacts.length > 0 && { deletedArtifacts }),
-    ...(warnings.length > 0 && { warnings }),
+    ...(uniqueWarnings.length > 0 && { warnings: uniqueWarnings }),
     ...(messages.length > 0 && { messages }),
     ...(Object.keys(mcpResults).length > 0 && { mcpResults }),
     ...(nativeResult && { nativeResult }),

src/core/transform.ts

@@ -485,8 +485,10 @@ export async function collectPluginSkills(
   pluginName?: string,
   enabledSkills?: Set<string>,
   pluginSkillsConfig?: PluginSkillsConfig,
+  warnings?: string[],
 ): Promise<CollectedSkill[]> {
   const skillsDir = join(pluginPath, 'skills');
+  const skillWalkWarnings: string[] = [];
 
   // v1 fallback: only apply enabledSkills to plugins that actually have entries in the set
   const hasEnabledEntries =
@@ -498,20 +500,23 @@ export async function collectPluginSkills(
   let candidateDirs: { name: string; subpath: string; path: string }[];
 
   if (existsSync(skillsDir)) {
-    const entries = await discoverNestedSkillEntries(skillsDir);
+    const entries = await discoverNestedSkillEntries(
+      skillsDir,
+      skillWalkWarnings,
+    );
     candidateDirs = entries.map((entry) => ({
       name: entry.name,
       subpath: entry.subpath,
       path: entry.skillPath,
     }));
   } else {
-    const nestedDirs = (await discoverNestedSkillEntries(pluginPath)).map(
-      (entry) => ({
-        name: entry.name,
-        subpath: entry.subpath,
-        path: entry.skillPath,
-      }),
-    );
+    const nestedDirs = (
+      await discoverNestedSkillEntries(pluginPath, skillWalkWarnings)
+    ).map((entry) => ({
+      name: entry.name,
+      subpath: entry.subpath,
+      path: entry.skillPath,
+    }));
 
     if (nestedDirs.length > 0) {
       candidateDirs = nestedDirs;
@@ -571,6 +576,12 @@ export async function collectPluginSkills(
     filteredDirs = candidateDirs;
   }
 
+  if (warnings && skillWalkWarnings.length > 0) {
+    warnings.push(
+      ...skillWalkWarnings.map((w) => `Plugin '${pluginSource}': ${w}`),
+    );
+  }
+
   return filteredDirs.map((entry) => ({
     folderName: entry.name,
     skillPath: entry.path,

src/core/user-workspace.ts

@@ -10,6 +10,7 @@ import type {
 } from '../models/workspace-config.js';
 import { getPluginSource } from '../models/workspace-config.js';
 import {
+  isFilesystemRoot,
   isGitHubUrl,
   parseGitHubUrl,
   validatePluginSource,
@@ -160,6 +161,12 @@ export async function addUserPlugin(
         error: `Plugin not found at ${plugin}`,
       };
     }
+    if (isFilesystemRoot(plugin)) {
+      return {
+        success: false,
+        error: `Plugin source cannot be a filesystem root directory: ${plugin}`,
+      };
+    }
   }
 
   return addPluginToUserConfig(plugin, configPath, undefined, force);

src/core/workspace-modify.ts

@@ -12,6 +12,7 @@ import type {
 import { getPluginSource } from '../models/workspace-config.js';
 import { parseMarketplaceManifest } from '../utils/marketplace-manifest-parser.js';
 import {
+  isFilesystemRoot,
   isGitHubUrl,
   parseGitHubUrl,
   validatePluginSource,
@@ -163,6 +164,12 @@ export async function addPlugin(
         error: `Plugin not found at ${plugin}`,
       };
     }
+    if (isFilesystemRoot(fullPath) || isFilesystemRoot(plugin)) {
+      return {
+        success: false,
+        error: `Plugin source cannot be a filesystem root directory: ${plugin}`,
+      };
+    }
   }
 
   return await addPluginToConfig(plugin, configPath, undefined, force);

src/utils/plugin-path.ts

@@ -1,5 +1,5 @@
 import { existsSync } from 'node:fs';
-import { isAbsolute, join, resolve } from 'node:path';
+import { isAbsolute, join, parse, resolve } from 'node:path';
 import { getHomeDir } from '../constants.js';
 import {
   GitCloneError,
@@ -405,6 +405,17 @@ export function validatePluginSource(source: string): {
   return { valid: true };
 }
 
+/**
+ * Check whether a path resolves to the root of a filesystem (e.g. `C:\` on
+ * Windows or `/` on POSIX). A local-path plugin source can never legitimately
+ * be a filesystem root — every real plugin is a specific, self-contained
+ * directory.
+ */
+export function isFilesystemRoot(candidatePath: string): boolean {
+  const resolved = resolve(candidatePath);
+  return resolved === parse(resolved).root;
+}
+
 /**
  * Parsed file source information
  */

tests/unit/core/skills.test.ts

@@ -1,9 +1,13 @@
-import { describe, it, expect, beforeEach, afterEach } from 'bun:test';
-import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises';
+import { describe, it, expect, beforeEach, afterEach, test } from 'bun:test';
+import { mkdtemp, rm, mkdir, writeFile, symlink } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { dump } from 'js-yaml';
-import { getAllSkillsFromPlugins, type SkillInfo } from '../../../src/core/skills.js';
+import {
+  getAllSkillsFromPlugins,
+  discoverNestedSkillEntries,
+  type SkillInfo,
+} from '../../../src/core/skills.js';
 import { getPluginCachePath } from '../../../src/utils/plugin-path.js';
 
 describe('getAllSkillsFromPlugins', () => {
@@ -240,3 +244,65 @@ describe('getAllSkillsFromPlugins', () => {
     }
   });
 });
+
+describe('discoverNestedSkillEntries error handling', () => {
+  let tmpDir: string;
+
+  beforeEach(async () => {
+    tmpDir = await mkdtemp(join(tmpdir(), 'allagents-skills-walk-test-'));
+  });
+
+  afterEach(async () => {
+    await rm(tmpDir, { recursive: true, force: true });
+  });
+
+  it('records a warning instead of throwing when the scan root cannot be read', async () => {
+    // A file (not a directory) makes readdir() throw ENOTDIR, exercising the
+    // same catch path a real EPERM/EACCES on a restricted directory would.
+    const notADir = join(tmpDir, 'not-a-directory');
+    await writeFile(notADir, 'not a directory');
+
+    const warnings: string[] = [];
+    const entries = await discoverNestedSkillEntries(notADir, warnings);
+
+    expect(entries).toEqual([]);
+    expect(warnings).toHaveLength(1);
+    expect(warnings[0]).toContain(notADir);
+    expect(warnings[0]).toContain('workspace.yaml');
+  });
+
+  it('still discovers sibling skills when one plugin path cannot be read', async () => {
+    const goodPlugin = join(tmpDir, 'good-plugin');
+    await mkdir(join(goodPlugin, 'skills', 'good-skill'), { recursive: true });
+    await writeFile(join(goodPlugin, 'skills', 'good-skill', 'SKILL.md'), '# good');
+
+    const badPlugin = join(tmpDir, 'bad-plugin-is-a-file');
+    await writeFile(badPlugin, 'not a directory');
+
+    const warnings: string[] = [];
+    const goodEntries = await discoverNestedSkillEntries(
+      join(goodPlugin, 'skills'),
+      warnings,
+    );
+    const badEntries = await discoverNestedSkillEntries(badPlugin, warnings);
+
+    expect(goodEntries.map((e) => e.name)).toEqual(['good-skill']);
+    expect(badEntries).toEqual([]);
+    expect(warnings).toHaveLength(1);
+  });
+
+  test.skipIf(process.platform === 'win32')(
+    'skips symlinked directories instead of following loops',
+    async () => {
+      const pluginDir = join(tmpDir, 'symlink-plugin');
+      await mkdir(join(pluginDir, 'real-skill'), { recursive: true });
+      await writeFile(join(pluginDir, 'real-skill', 'SKILL.md'), '# real');
+
+      // A symlink back to the plugin root would recurse forever if followed.
+      await symlink(pluginDir, join(pluginDir, 'loop'), 'dir');
+
+      const entries = await discoverNestedSkillEntries(pluginDir);
+      expect(entries.map((e) => e.name)).toEqual(['real-skill']);
+    },
+  );
+});