fix(runtime): discover contained symlinked skill directories

.github/workflows/windows-recovery.yml

@@ -158,8 +158,8 @@ jobs:
           $exitCode = $LASTEXITCODE
           if ($exitCode -ne 0) { exit $exitCode }
           $output = Get-Content "$env:RUNNER_TEMP/skill-catalog.tap"
-          if ($output -notcontains '# tests 90' -or $output -notcontains '# pass 90' -or $output -notcontains '# skipped 0') {
-            Write-Error 'Skill catalog gate did not run exactly 90 passing Windows tests'
+          if ($output -notcontains '# tests 91' -or $output -notcontains '# pass 91' -or $output -notcontains '# skipped 0') {
+            Write-Error 'Skill catalog gate did not run exactly 91 passing Windows tests'
             exit 1
           }
 

packages/runtime-host/src/__tests__/skill-catalog-repository.test.ts

@@ -21,7 +21,7 @@ import { RuntimeHostProtocolError } from '../protocol/errors.js';
 import assert from 'node:assert/strict';
 import { execFile } from 'node:child_process';
 import { createHash } from 'node:crypto';
-import { chmod, mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises';
+import { chmod, mkdir, mkdtemp, readFile, rm, symlink, writeFile } from 'node:fs/promises';
 import { tmpdir } from 'node:os';
 import { join } from 'node:path';
 import { afterEach, test } from 'node:test';
@@ -903,6 +903,78 @@ test('non-force managed update preserves a local edit made after its snapshot',
   assert.equal(await readFile(installedPath, 'utf8'), localEdit);
 });
 
+test('managed update blocks a symlink redirected outside its discovery root after scanning', async () => {
+  const fixture = await createFixture();
+  const sourceId = 'managed-symlink-race';
+  const installedContent = skillBody('Managed Symlink Race', 'installed');
+  const updateContent = skillBody('Managed Symlink Race', 'source update');
+  const outsideContent = skillBody('Managed Symlink Race', 'outside replacement');
+  await createSkill(fixture.sources, sourceId, updateContent);
+
+  const containedSkill = await createSkill(
+    join(fixture.root, 'linked-skill-sources'),
+    sourceId,
+    installedContent,
+  );
+  await mkdir(join(containedSkill, '.maka', 'baseline'), { recursive: true });
+  await writeFile(
+    join(containedSkill, 'skill.lock.json'),
+    `${JSON.stringify(
+      createManagedSkillLock(sourceId, sha256(installedContent), sha256(installedContent)),
+      null,
+      2,
+    )}\n`,
+  );
+  await writeFile(join(containedSkill, '.maka', 'baseline', 'SKILL.md'), installedContent);
+
+  const skillsDirectory = join(fixture.root, 'skills');
+  const linkedSkill = join(skillsDirectory, sourceId);
+  await mkdir(skillsDirectory, { recursive: true });
+  await symlink(containedSkill, linkedSkill, 'dir');
+
+  const outsideSkill = await createSkill(
+    await tempDirectory('maka-skill-symlink-race-outside-'),
+    sourceId,
+    outsideContent,
+  );
+  await mkdir(join(outsideSkill, '.maka', 'baseline'), { recursive: true });
+  await writeFile(
+    join(outsideSkill, 'skill.lock.json'),
+    `${JSON.stringify(
+      createManagedSkillLock(sourceId, sha256(outsideContent), sha256(outsideContent)),
+      null,
+      2,
+    )}\n`,
+  );
+  await writeFile(join(outsideSkill, '.maka', 'baseline', 'SKILL.md'), outsideContent);
+
+  let redirectAfterScan = false;
+  const repository = fixture.repository(undefined, {
+    beforeManagedInstalledArtifactsRead: async () => {
+      if (!redirectAfterScan) return;
+      redirectAfterScan = false;
+      await rm(linkedSkill);
+      await symlink(outsideSkill, linkedSkill, 'dir');
+    },
+  });
+  const snapshot = await start(repository, fixture.project, 'governance');
+
+  redirectAfterScan = true;
+  const result = await repository.mutate({
+    expectedRevision: snapshot.revision,
+    mutation: {
+      kind: 'update_managed',
+      ref: `workspace:legacy:${sourceId}`,
+      force: false,
+      expectedCurrentSha256: null,
+      expectedSourceSha256: null,
+    },
+  });
+
+  assert.deepEqual(result, { kind: 'rejected', reason: 'metadata_error' });
+  assert.equal(await readFile(join(outsideSkill, 'SKILL.md'), 'utf8'), outsideContent);
+});
+
 test('revision covers exact managed lock and baseline bytes and rejects post-snapshot edits', async () => {
   const fixture = await createFixture();
   const sourceId = 'managed-artifact-race';

packages/runtime-host/src/server/skill-catalog-repository.ts

@@ -403,7 +403,10 @@ export class SkillCatalogRepository {
     if (!validateSkillMetadata(source.content).valid) {
       return { kind: 'rejected', reason: 'source_invalid' };
     }
-    const current = await readContainedArtifact(fact.skill.path, join(fact.skill.path, SKILL_FILE));
+    const current = await readContainedArtifact(
+      fact.skill.discoveryRoot,
+      join(fact.skill.path, SKILL_FILE),
+    );
     if (current.status !== 'available') {
       return { kind: 'rejected', reason: 'metadata_error' };
     }
@@ -1012,7 +1015,10 @@ async function governanceForSkill(
     return { governance: missingSkillLockStatus(), baselineAvailable: false };
   }
   const lockPath = join(skill.path, LOCK_FILE);
-  const baselineRead = await readContainedArtifact(skill.path, join(skill.path, BASELINE_FILE));
+  const baselineRead = await readContainedArtifact(
+    skill.discoveryRoot,
+    join(skill.path, BASELINE_FILE),
+  );
   const baselineAvailable = baselineRead.status === 'available';
   const baselineSha256 = baselineRead.status === 'unavailable' ? undefined : baselineRead.sha256;
   const lockStat = await lstat(lockPath).catch((error: NodeJS.ErrnoException) =>
@@ -1039,7 +1045,7 @@ async function governanceForSkill(
       ...(baselineSha256 === undefined ? {} : { baselineSha256 }),
     };
   }
-  const lockRead = await readContainedArtifact(skill.path, lockPath);
+  const lockRead = await readContainedArtifact(skill.discoveryRoot, lockPath);
   if (lockRead.status === 'unavailable') {
     return {
       governance: invalidSkillLockStatus('invalid_json', 'Skill lock could not be read safely.'),
@@ -1681,9 +1687,9 @@ async function readManagedArtifacts(skill: ScannedSkill): Promise<{
   baselineSha256: string;
 } | null> {
   const [skillFile, lock, baseline] = await Promise.all([
-    readContainedArtifact(skill.path, join(skill.path, SKILL_FILE)),
-    readContainedArtifact(skill.path, join(skill.path, LOCK_FILE)),
-    readContainedArtifact(skill.path, join(skill.path, BASELINE_FILE)),
+    readContainedArtifact(skill.discoveryRoot, join(skill.path, SKILL_FILE)),
+    readContainedArtifact(skill.discoveryRoot, join(skill.path, LOCK_FILE)),
+    readContainedArtifact(skill.discoveryRoot, join(skill.path, BASELINE_FILE)),
   ]);
   if (
     skillFile.status !== 'available' ||

packages/runtime/src/__tests__/skills.test.ts

@@ -520,6 +520,106 @@ Open local targets carefully.`,
     });
   });
 
+  it('discovers contained symlinked skill directories using the link identity', async () => {
+    await withWorkspace(async (workspaceRoot) => {
+      const skillsDir = join(workspaceRoot, 'skills');
+      const sourceDir = join(workspaceRoot, 'linked-skill-sources');
+      const linkedSkill = join(skillsDir, 'linked-alias');
+      await mkdir(skillsDir, { recursive: true });
+      await writeSkillInDirectory(
+        sourceDir,
+        'source-name',
+        'Linked Skill',
+        'A contained symlinked skill.',
+      );
+      await symlink(join(sourceDir, 'source-name'), linkedSkill, 'dir');
+
+      const scan = await scanSkillsWithDiagnostics(workspaceRoot);
+
+      assert.deepEqual(
+        scan.skills.map((skill) => ({ id: skill.id, ref: skill.ref, path: skill.path })),
+        [
+          {
+            id: 'linked-alias',
+            ref: 'workspace:legacy:linked-alias',
+            path: linkedSkill,
+          },
+        ],
+      );
+      assert.deepEqual(scan.discoveryDiagnostics, []);
+    });
+  });
+
+  it('diagnoses unusable symlinked skill directories without hiding valid skills', async () => {
+    await withWorkspace(async (workspaceRoot) => {
+      const outside = await mkdtemp(join(tmpdir(), 'maka-skill-entry-outside-'));
+      const skillsDir = join(workspaceRoot, 'skills');
+      const containedFile = join(workspaceRoot, 'contained-file');
+      const containedFileLink = join(skillsDir, 'contained-file-link');
+      const cyclicSkill = join(skillsDir, 'cyclic-link');
+      const danglingSkill = join(skillsDir, 'dangling-link');
+      const notDirectorySkill = join(skillsDir, 'not-directory-link');
+      const escapingSkill = join(skillsDir, 'escaping-link');
+      try {
+        await writeSkill(
+          workspaceRoot,
+          'valid',
+          `---
+name: Valid
+description: A valid local skill.
+---
+# Valid`,
+        );
+        await writeSkillInDirectory(outside, 'source-name', 'Outside', 'An escaping skill.');
+        await writeFile(containedFile, 'not a directory', 'utf8');
+        await symlink(containedFile, containedFileLink, 'file');
+        await symlink(cyclicSkill, cyclicSkill, 'dir');
+        await symlink(join(workspaceRoot, 'missing-target'), danglingSkill, 'dir');
+        await symlink(join(containedFile, 'child'), notDirectorySkill, 'dir');
+        await symlink(join(outside, 'source-name'), escapingSkill, 'dir');
+
+        const scan = await scanSkillsWithDiagnostics(workspaceRoot);
+
+        assert.deepEqual(
+          scan.skills.map((skill) => skill.id),
+          ['valid'],
+        );
+        assert.deepEqual(scan.discoveryDiagnostics, [
+          {
+            path: cyclicSkill,
+            scope: 'workspace',
+            source: 'legacy',
+            precedence: 0,
+            reason: 'read_failed',
+          },
+          {
+            path: danglingSkill,
+            scope: 'workspace',
+            source: 'legacy',
+            precedence: 0,
+            reason: 'blocked_path',
+          },
+          {
+            path: escapingSkill,
+            scope: 'workspace',
+            source: 'legacy',
+            precedence: 0,
+            reason: 'blocked_path',
+          },
+          {
+            path: notDirectorySkill,
+            scope: 'workspace',
+            source: 'legacy',
+            precedence: 0,
+            reason: 'blocked_path',
+          },
+        ]);
+      } finally {
+        await rm(outside, { recursive: true, force: true });
+      }
+    });
+  });
+
   it('readSkillRuntimeState does not read through a symlinked state file', async () => {
     await withWorkspace(async (workspaceRoot) => {
       const outside = await mkdtemp(join(tmpdir(), 'maka-skill-state-file-outside-'));

packages/runtime/src/skills-discovery.ts

@@ -19,7 +19,7 @@
 
 import { createHash } from 'node:crypto';
 import { homedir } from 'node:os';
-import { lstat, readdir, realpath } from 'node:fs/promises';
+import { lstat, readdir, realpath, stat } from 'node:fs/promises';
 import { join } from 'node:path';
 import { isPathInside, readContainedRegularFile } from './path-containment.js';
 import { validateSkillMetadata } from './skills-metadata.js';
@@ -109,9 +109,10 @@ export interface ScannedSkill extends RuntimeSkillDefinition {
   contentSha256: string;
   /**
    * The containment root this skill was discovered under (e.g. workspace root,
-   * home dir). Used to compute `relativePath` in `loadSkillInstructions` so
-   * legacy callers see `skills/<id>/SKILL.md` while multi-path callers see
-   * the actual subpath.
+   * home dir). Remains the authority for later contained reads even when
+   * `path` is a mutable symlink, and is used to compute `relativePath` in
+   * `loadSkillInstructions` so legacy callers see `skills/<id>/SKILL.md`
+   * while multi-path callers see the actual subpath.
    */
   discoveryRoot: string;
 }
@@ -361,10 +362,11 @@ function normalizeSkillSource(source: SkillSource): {
  * `SKILL.md` is parsed. Metadata validation errors exclude only the malformed
  * skill and are returned as structured diagnostics.
  *
- * The directory itself must be a real directory (not a symlink) and its
- * realpath must be contained within the realpath of its parent directory.
- * This prevents ancestor-level symlinks (e.g. `repo/.agents -> /outside`)
- * from escaping the expected boundary.
+ * The discovery directory itself must be a real directory (not a symlink) and
+ * its realpath must be contained within its configured containment root. This
+ * prevents ancestor-level symlinks (e.g. `repo/.agents -> /outside`) from
+ * escaping the expected boundary. Immediate symlinked skill entries are
+ * followed only when their targets remain inside that same containment root.
  */
 async function scanSkillDir(
   discovery: SkillDiscoveryEntry,
@@ -382,45 +384,72 @@ async function scanSkillDir(
     discoveryDiagnostics,
     runtimeState,
   });
-  const sourceDiagnostic = (
+  const discoveryDiagnostic = (
+    path: string,
     reason: SkillDiscoveryDiagnostic['reason'],
   ): SkillDiscoveryDiagnostic => ({
-    path: dir,
+    path,
     scope,
     source,
     precedence,
     reason,
   });
   let entries: import('node:fs').Dirent[];
+  let rootReal: string;
   try {
     const dirStat = await lstat(dir);
     if (!dirStat.isDirectory() || dirStat.isSymbolicLink()) {
-      return empty([sourceDiagnostic('blocked_path')]);
+      return empty([discoveryDiagnostic(dir, 'blocked_path')]);
     }
     // Verify the resolved directory has not escaped its containment root via
     // an ancestor symlink (e.g. `repo/.agents -> /outside`).
-    const [rootReal, dirReal] = await Promise.all([realpath(containmentRoot), realpath(dir)]);
+    const [resolvedRoot, dirReal] = await Promise.all([realpath(containmentRoot), realpath(dir)]);
+    rootReal = resolvedRoot;
     if (!isPathInside(rootReal, dirReal)) {
-      return empty([sourceDiagnostic('blocked_path')]);
+      return empty([discoveryDiagnostic(dir, 'blocked_path')]);
     }
     entries = await readdir(dir, { withFileTypes: true });
     entries.sort((a, b) => a.name.localeCompare(b.name));
   } catch (error) {
     // An absent optional discovery root is normal. A configured path that
     // exists but cannot be inspected is not: expose it to governance clients.
     if ((error as NodeJS.ErrnoException).code === 'ENOENT') return empty();
-    return empty([sourceDiagnostic('read_failed')]);
+    return empty([discoveryDiagnostic(dir, 'read_failed')]);
   }
 
   const out: ScannedSkill[] = [];
   const rejected: RejectedSkillDefinition[] = [];
   const diagnostics: SkillScanDiagnostic[] = [];
+  const discoveryDiagnostics: SkillDiscoveryDiagnostic[] = [];
   for (const dirEntry of entries) {
-    if (!dirEntry.isDirectory()) continue;
     const skillPath = join(dir, dirEntry.name);
-    const skillFile = join(skillPath, 'SKILL.md');
+    let skillReadRoot = skillPath;
+    let skillDirectory = skillPath;
+    if (!dirEntry.isDirectory()) {
+      if (!dirEntry.isSymbolicLink()) continue;
+      try {
+        const skillReal = await realpath(skillPath);
+        if (!isPathInside(rootReal, skillReal)) {
+          discoveryDiagnostics.push(discoveryDiagnostic(skillPath, 'blocked_path'));
+          continue;
+        }
+        if (!(await stat(skillReal)).isDirectory()) continue;
+        skillReadRoot = rootReal;
+        skillDirectory = skillReal;
+      } catch (error) {
+        const code = (error as NodeJS.ErrnoException).code;
+        discoveryDiagnostics.push(
+          discoveryDiagnostic(
+            skillPath,
+            code === 'ENOENT' || code === 'ENOTDIR' ? 'blocked_path' : 'read_failed',
+          ),
+        );
+        continue;
+      }
+    }
+    const skillFile = join(skillDirectory, 'SKILL.md');
     try {
-      const read = await readContainedRegularFile(skillPath, skillFile);
+      const read = await readContainedRegularFile(skillReadRoot, skillFile);
       if (!read.ok) continue;
       const bytes = read.bytes;
       const text = bytes.toString('utf8');
@@ -485,7 +514,7 @@ async function scanSkillDir(
     inventory: out,
     rejected,
     diagnostics,
-    discoveryDiagnostics: [],
+    discoveryDiagnostics,
     runtimeState,
   };
 }

scripts/ci-test-plan.test.mjs

@@ -630,8 +630,8 @@ test('Windows recovery executes the complete Skill catalog suite', () => {
   assert.match(recovery, /skill-catalog-repository\.test\.js/u);
   assert.match(recovery, /skill-catalog-transaction\.test\.js/u);
   assert.match(recovery, /skill-catalog-two-client-uds\.test\.js/u);
-  assert.match(recovery, /# tests 90/u);
-  assert.match(recovery, /# pass 90/u);
+  assert.match(recovery, /# tests 91/u);
+  assert.match(recovery, /# pass 91/u);
   assert.match(recovery, /# skipped 0/u);
 });