From fd27ab81b7ae1842243a614f2d74b804f70f6ab2 Mon Sep 17 00:00:00 2001 From: Michael Ramos Date: Mon, 20 Apr 2026 07:11:00 -0700 Subject: [PATCH] fix(pi): loosen plan-mode write gate to directory scope The write/edit gate required an exact path match against planFilePath, so `/plannotator plans/` (or any directory-style path) blocked every write the agent attempted, including files inside that same directory. Agents then fell back to bash/python heredocs to bypass the gate. Now directory-scoped: writes inside the plan's directory are allowed when planFilePath ends with a separator, has no file extension, or points to a subdirectory. Default `PLAN.md` behavior is unchanged. Extracted the gate into a pure function in tool-scope.ts with test coverage for the main cases (default file, trailing-slash dir, bare dir name, subdir sibling, traversal rejection, absolute paths). For provenance purposes, this commit was AI assisted. --- apps/pi-extension/index.ts | 22 +++++-------------- apps/pi-extension/tool-scope.test.ts | 33 ++++++++++++++++++++++++++++ apps/pi-extension/tool-scope.ts | 31 ++++++++++++++++++++++++++ 3 files changed, 70 insertions(+), 16 deletions(-) diff --git a/apps/pi-extension/index.ts b/apps/pi-extension/index.ts index 1cba685f..2b3aa021 100644 --- a/apps/pi-extension/index.ts +++ b/apps/pi-extension/index.ts @@ -53,6 +53,7 @@ import { } from "./plannotator-events.js"; import { getToolsForPhase, + isPlanWritePathAllowed, PLAN_SUBMIT_TOOL, type Phase, stripPlanningOnlyTools, @@ -703,24 +704,13 @@ export default function plannotator(pi: ExtensionAPI): void { pi.on("tool_call", async (event, ctx) => { if (phase !== "planning") return; - if (event.toolName === "write") { - const targetPath = resolve(ctx.cwd, event.input.path as string); - const allowedPath = resolvePlanPath(ctx.cwd); - if (targetPath !== allowedPath) { + if (event.toolName === "write" || event.toolName === "edit") { + const inputPath = event.input.path as string; + if (!isPlanWritePathAllowed(planFilePath, inputPath, ctx.cwd)) { + const verb = event.toolName === "write" ? "writes" : "edits"; return { block: true, - reason: `Plannotator: writes are restricted to ${planFilePath} during planning. Blocked: ${event.input.path}`, - }; - } - } - - if (event.toolName === "edit") { - const targetPath = resolve(ctx.cwd, event.input.path as string); - const allowedPath = resolvePlanPath(ctx.cwd); - if (targetPath !== allowedPath) { - return { - block: true, - reason: `Plannotator: edits are restricted to ${planFilePath} during planning. Blocked: ${event.input.path}`, + reason: `Plannotator: ${verb} are restricted to ${planFilePath} during planning. Blocked: ${inputPath}`, }; } } diff --git a/apps/pi-extension/tool-scope.test.ts b/apps/pi-extension/tool-scope.test.ts index a8434ad8..bbb89349 100644 --- a/apps/pi-extension/tool-scope.test.ts +++ b/apps/pi-extension/tool-scope.test.ts @@ -1,6 +1,7 @@ import { describe, expect, test } from "bun:test"; import { getToolsForPhase, + isPlanWritePathAllowed, PLAN_SUBMIT_TOOL, stripPlanningOnlyTools, } from "./tool-scope"; @@ -44,3 +45,35 @@ describe("pi plan tool scoping", () => { ]); }); }); + +describe("plan write path gate", () => { + const cwd = "/r"; + + test("default PLAN.md allows exact file and blocks everything else", () => { + expect(isPlanWritePathAllowed("PLAN.md", "PLAN.md", cwd)).toBe(true); + expect(isPlanWritePathAllowed("PLAN.md", "src/app.ts", cwd)).toBe(false); + }); + + test("trailing-slash directory scopes to files inside", () => { + expect(isPlanWritePathAllowed("plans/", "plans/foo.md", cwd)).toBe(true); + expect(isPlanWritePathAllowed("plans/", "src/app.ts", cwd)).toBe(false); + }); + + test("bare directory name (no slash, no extension) scopes to files inside", () => { + expect(isPlanWritePathAllowed("plans", "plans/foo.md", cwd)).toBe(true); + expect(isPlanWritePathAllowed("plans", "src/app.ts", cwd)).toBe(false); + }); + + test("file inside a subdir allows siblings and blocks outside", () => { + expect(isPlanWritePathAllowed("plans/foo.md", "plans/bar.md", cwd)).toBe(true); + expect(isPlanWritePathAllowed("plans/foo.md", "src/app.ts", cwd)).toBe(false); + }); + + test("path traversal is rejected", () => { + expect(isPlanWritePathAllowed("plans/", "../../etc/passwd", cwd)).toBe(false); + }); + + test("absolute input paths resolve the same as relative", () => { + expect(isPlanWritePathAllowed("plans/", "/r/plans/foo.md", cwd)).toBe(true); + }); +}); diff --git a/apps/pi-extension/tool-scope.ts b/apps/pi-extension/tool-scope.ts index c229fed0..6fc729d7 100644 --- a/apps/pi-extension/tool-scope.ts +++ b/apps/pi-extension/tool-scope.ts @@ -1,3 +1,5 @@ +import { basename, dirname, isAbsolute, relative, resolve, sep } from "node:path"; + export type Phase = "idle" | "planning" | "executing"; export const PLAN_SUBMIT_TOOL = "plannotator_submit_plan"; @@ -22,3 +24,32 @@ export function getToolsForPhase( ...new Set([...tools, ...PLANNING_DISCOVERY_TOOLS, PLAN_SUBMIT_TOOL]), ]; } + +// Treat planFilePath as directory-scoped when it ends with a separator or its +// basename has no extension (e.g. "plans/", "plans"). Otherwise scope is the +// single file. +function isPlanPathDirectoryScoped(planFilePath: string): boolean { + if (planFilePath.endsWith("/") || planFilePath.endsWith(sep)) return true; + const base = basename(planFilePath); + return base.length > 0 && !base.includes("."); +} + +export function isPlanWritePathAllowed( + planFilePath: string, + inputPath: string, + cwd: string, +): boolean { + const targetAbs = resolve(cwd, inputPath); + const allowedAbs = resolve(cwd, planFilePath); + if (targetAbs === allowedAbs) return true; + + const dirScoped = isPlanPathDirectoryScoped(planFilePath); + const scopeDir = dirScoped ? allowedAbs : dirname(allowedAbs); + + // Never scope to cwd root — a default like "PLAN.md" would otherwise unlock + // every file in the project. + if (resolve(scopeDir) === resolve(cwd)) return false; + + const rel = relative(scopeDir, targetAbs); + return rel !== "" && !rel.startsWith("..") && !isAbsolute(rel); +}