From 6523fc56ec236fa38a4f792cad0b26a1cc4f3f73 Mon Sep 17 00:00:00 2001
From: Paul Bakaus <paul.bakaus@gmail.com>
Date: Mon, 7 Sep 2026 19:08:29 -0700
Subject: [PATCH] Keep fixture tool paths contained through symlinks

Validate canonical existing ancestors for new targets and preserve staged-skill write protection through aliases. Regression reproduced the external read escape before the fix; all 20 harness tests and the full non-billed suite pass.

AI assistance: Codex, under maintainer direction.
---
 tests/skill-behavior-harness.test.mjs | 24 ++++++++++++++++++++++++
 tests/skill-behavior/harness.mjs      | 22 ++++++++++++++++++++--
 2 files changed, 44 insertions(+), 2 deletions(-)

diff --git a/tests/skill-behavior-harness.test.mjs b/tests/skill-behavior-harness.test.mjs
index e3133d1e6..8192655ab 100644
--- a/tests/skill-behavior-harness.test.mjs
+++ b/tests/skill-behavior-harness.test.mjs
@@ -379,6 +379,30 @@ it('successful-loader controls accept a workspace-relative target', { skip: !pro
   }
 });
 
+it('workspace tools reject symlink escapes and preserve staged-skill write protection', async () => {
+  const workspace = prepareWorkspace({ files: { 'local/value.txt': 'local' } });
+  const outside = prepareWorkspace({ files: { 'value.txt': 'outside' } });
+  try {
+    fs.symlinkSync(outside, path.join(workspace, 'escape'), 'junction');
+    fs.symlinkSync(path.join(workspace, 'local'), path.join(workspace, 'alias'), 'junction');
+    fs.symlinkSync(path.join(workspace, '.claude'), path.join(workspace, 'skill-alias'), 'junction');
+    const { tools } = makeTools(workspace, {}, {}, { contextOnlyBash: true });
+    assert.match(await tools.read.execute({ path: 'escape/value.txt' }), /^Error:/);
+    assert.match(await tools.list.execute({ path: 'escape' }), /^Error:/);
+    assert.match(await tools.write.execute({ path: 'escape/new/file.txt', contents: 'bad' }), /^Error:/);
+    assert.match(await tools.bash.execute({ command: '.claude/skills/impeccable/scripts/impeccable context --target escape/value.txt' }), /^Error:/);
+    assert.match(await tools.write.execute({ path: 'skill-alias/skills/impeccable/reference/routing.md', contents: 'bad' }), /^Error:/);
+    assert.equal(await tools.read.execute({ path: 'alias/value.txt' }), 'local');
+    await tools.write.execute({ path: 'alias/nested/new.txt', contents: 'allowed' });
+    assert.equal(fs.readFileSync(path.join(workspace, 'local/nested/new.txt'), 'utf8'), 'allowed');
+    assert.equal(fs.existsSync(path.join(outside, 'new')), false);
+    assert.equal(fs.readFileSync(path.join(outside, 'value.txt'), 'utf8'), 'outside');
+  } finally {
+    cleanupWorkspace(workspace);
+    cleanupWorkspace(outside);
+  }
+});
+
 it('context-only routing tools keep project writes observable but protect the staged skill', async () => {
   const workspace = prepareWorkspace({ files: { 'index.html': 'before' } });
   try {
diff --git a/tests/skill-behavior/harness.mjs b/tests/skill-behavior/harness.mjs
index 2c3393527..84f8d86d1 100644
--- a/tests/skill-behavior/harness.mjs
+++ b/tests/skill-behavior/harness.mjs
@@ -150,7 +150,25 @@ function safeResolve(root, userPath) {
   if (rel.startsWith('..') || rel.split(path.sep).includes('..')) {
     return { error: 'path escapes the workspace' };
   }
-  return resolved;
+  try {
+    // New write targets need not exist; validate their nearest existing
+    // ancestor, including dangling links, before appending the missing suffix.
+    let ancestor = resolved;
+    while (!fs.existsSync(ancestor)) {
+      if (fs.lstatSync(ancestor, { throwIfNoEntry: false })?.isSymbolicLink()) {
+        return { error: 'path follows a dangling symlink' };
+      }
+      ancestor = path.dirname(ancestor);
+    }
+    const canonical = path.resolve(fs.realpathSync(ancestor), path.relative(ancestor, resolved));
+    const realRel = path.relative(fs.realpathSync(root), canonical);
+    if (realRel === '..' || realRel.startsWith(`..${path.sep}`) || path.isAbsolute(realRel)) {
+      return { error: 'path escapes the workspace through a symlink' };
+    }
+    return canonical;
+  } catch {
+    return { error: 'path cannot be resolved safely' };
+  }
 }
 
 function isContextOnlyCommand(workspace, command) {
@@ -322,7 +340,7 @@ export function makeTools(workspace, extraEnv = {}, simulatedUser = {}, { contex
         const call = record('write', { path: p, contents });
         const resolved = safeResolve(workspace, p);
         if (typeof resolved !== 'string') return `Error: ${resolved.error}`;
-        if (path.relative(workspace, resolved).split(path.sep)[0] === '.claude') {
+        if (path.relative(fs.realpathSync(workspace), resolved).split(path.sep)[0] === '.claude') {
           return 'Error: the staged skill is read-only; edits must target project files.';
         }
         fs.mkdirSync(path.dirname(resolved), { recursive: true });