fix(core): sandbox path resolution in Plan Mode to prevent hallucinations and traversal

This commit is contained in:
A.K.M. Adib
2026-03-16 16:56:09 -04:00
parent c81c64cbb0
commit 92cd5fe28f
19 changed files with 81 additions and 103 deletions
+3 -13
View File
@@ -35,19 +35,13 @@ describe('planUtils', () => {
const fullPath = path.join(tempRootDir, planPath);
fs.writeFileSync(fullPath, '# My Plan');
const result = await validatePlanPath(planPath, plansDir, tempRootDir);
const result = await validatePlanPath(planPath, plansDir);
expect(result).toBeNull();
});
it('should return error for path traversal', async () => {
const planPath = path.join('..', 'secret.txt');
const result = await validatePlanPath(planPath, plansDir, tempRootDir);
expect(result).toContain('Access denied');
});
it('should return error for non-existent file', async () => {
const planPath = path.join('plans', 'ghost.md');
const result = await validatePlanPath(planPath, plansDir, tempRootDir);
const result = await validatePlanPath(planPath, plansDir);
expect(result).toContain('Plan file does not exist');
});
@@ -60,11 +54,7 @@ describe('planUtils', () => {
// Create a symbolic link pointing outside the plans directory
fs.symlinkSync(outsideFile, fullMaliciousPath);
const result = await validatePlanPath(
maliciousPath,
plansDir,
tempRootDir,
);
const result = await validatePlanPath(maliciousPath, plansDir);
expect(result).toContain('Access denied');
});
});
+5 -5
View File
@@ -13,8 +13,8 @@ import { isSubpath, resolveToRealPath } from './paths.js';
* Shared between backend tools and CLI UI for consistency.
*/
export const PlanErrorMessages = {
PATH_ACCESS_DENIED:
'Access denied: plan path must be within the designated plans directory.',
PATH_ACCESS_DENIED: (planPath: string, plansDir: string) =>
`Access denied: plan path (${planPath}) must be within the designated plans directory (${plansDir}).`,
FILE_NOT_FOUND: (path: string) =>
`Plan file does not exist: ${path}. You must create the plan file before requesting approval.`,
FILE_EMPTY:
@@ -32,14 +32,14 @@ export const PlanErrorMessages = {
export async function validatePlanPath(
planPath: string,
plansDir: string,
targetDir: string,
): Promise<string | null> {
const resolvedPath = path.resolve(targetDir, planPath);
const safeFilename = path.basename(planPath);
const resolvedPath = path.resolve(plansDir, safeFilename);
const realPath = resolveToRealPath(resolvedPath);
const realPlansDir = resolveToRealPath(plansDir);
if (!isSubpath(realPlansDir, realPath)) {
return PlanErrorMessages.PATH_ACCESS_DENIED;
return PlanErrorMessages.PATH_ACCESS_DENIED(planPath, realPlansDir);
}
if (!(await fileExists(resolvedPath))) {