mirror of
https://github.com/pbakaus/impeccable.git
synced 2026-09-15 23:56:29 +03:00
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.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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 });
|
||||
|
||||
Reference in New Issue
Block a user