From 6a9f712bbe19eb4adc7f11e4add6bb91ceb9c5df Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Fri, 21 Aug 2026 19:56:36 -0700 Subject: [PATCH 1/2] fix(pi): Confine file tools to checkout Prevent Pi read and search tools from traversing outside the active checkout. Mistaken root searches can otherwise turn into very long Warden runs. Preserve local behavior by using the Git root when available and the invocation directory otherwise. Co-Authored-By: GPT-5.6 Sol --- packages/warden/src/cli/context.test.ts | 35 +++++ packages/warden/src/cli/context.ts | 22 +++- packages/warden/src/cli/files.ts | 13 +- .../src/sdk/runtimes/pi-file-tools.test.ts | 65 ++++++++++ .../warden/src/sdk/runtimes/pi-file-tools.ts | 121 ++++++++++++++++++ packages/warden/src/sdk/runtimes/pi.test.ts | 12 +- packages/warden/src/sdk/runtimes/pi.ts | 12 +- 7 files changed, 272 insertions(+), 8 deletions(-) create mode 100644 packages/warden/src/cli/context.test.ts create mode 100644 packages/warden/src/sdk/runtimes/pi-file-tools.test.ts create mode 100644 packages/warden/src/sdk/runtimes/pi-file-tools.ts diff --git a/packages/warden/src/cli/context.test.ts b/packages/warden/src/cli/context.test.ts new file mode 100644 index 000000000..e9dc2e8ca --- /dev/null +++ b/packages/warden/src/cli/context.test.ts @@ -0,0 +1,35 @@ +import { execFileSync } from 'node:child_process'; +import { mkdirSync, mkdtempSync, realpathSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import { buildFileEventContext } from './context.js'; + +describe('buildFileEventContext', () => { + let repoPath: string; + + beforeEach(() => { + repoPath = mkdtempSync(join(tmpdir(), 'warden-file-context-')); + }); + + afterEach(() => { + rmSync(repoPath, { recursive: true, force: true }); + }); + + it('uses the checkout root when file analysis starts from a subdirectory', async () => { + const sourcePath = join(repoPath, 'packages', 'widget'); + mkdirSync(sourcePath, { recursive: true }); + writeFileSync(join(sourcePath, 'index.ts'), 'export const widget = true;\n'); + execFileSync('git', ['init'], { cwd: repoPath, stdio: 'ignore' }); + + const context = await buildFileEventContext({ + patterns: ['index.ts'], + cwd: sourcePath, + }); + + expect(context.repoPath).toBe(realpathSync(repoPath)); + expect(context.pullRequest?.files.map((file) => file.filename)).toEqual([ + 'packages/widget/index.ts', + ]); + }); +}); diff --git a/packages/warden/src/cli/context.ts b/packages/warden/src/cli/context.ts index fc0af12a1..f740360f8 100644 --- a/packages/warden/src/cli/context.ts +++ b/packages/warden/src/cli/context.ts @@ -1,4 +1,5 @@ -import { basename } from 'node:path'; +import { realpathSync } from 'node:fs'; +import { basename, resolve } from 'node:path'; import type { EventContext, FileChange } from '../types/index.js'; import type { IgnoreConfig, ScanConfig } from '../config/schema.js'; import { pluralize } from './output/index.js'; @@ -121,12 +122,25 @@ export interface FileContextOptions { * This allows analysis without requiring git or a warden.toml config. */ export async function buildFileEventContext(options: FileContextOptions): Promise { - const cwd = options.cwd ?? process.cwd(); - const dirName = basename(cwd); + const requestedCwd = resolve(options.cwd ?? process.cwd()); + let cwd = requestedCwd; + try { + cwd = realpathSync(requestedCwd); + } catch { + // File expansion reports missing or inaccessible invocation directories. + } + let repoPath = cwd; + try { + repoPath = getRepoRoot(cwd); + } catch { + // Explicit file analysis also works outside a Git checkout. + } + const dirName = basename(repoPath); const files = await expandAndCreateFileChanges(options.patterns, cwd, { ignore: options.ignore, scan: options.scan, + basePath: repoPath, }); return { @@ -151,6 +165,6 @@ export async function buildFileEventContext(options: FileContextOptions): Promis }, diffContextSource: { type: 'working-tree' }, explicitFileTargets: true, - repoPath: cwd, + repoPath, }; } diff --git a/packages/warden/src/cli/files.ts b/packages/warden/src/cli/files.ts index 9fc395584..6c53527c4 100644 --- a/packages/warden/src/cli/files.ts +++ b/packages/warden/src/cli/files.ts @@ -21,6 +21,11 @@ export interface SyntheticFileChangeOptions { scan?: ScanConfig; } +export interface ExpandAndCreateFileChangesOptions extends SyntheticFileChangeOptions { + /** Base path used for FileChange filenames and scan policy (default: cwd). */ + basePath?: string; +} + function hasGlobCharacters(pattern: string): boolean { return pattern.includes('*') || pattern.includes('?'); } @@ -295,9 +300,13 @@ export function createSyntheticFileChanges( export async function expandAndCreateFileChanges( patterns: string[], cwd: string = process.cwd(), - options: SyntheticFileChangeOptions = {} + options: ExpandAndCreateFileChangesOptions = {} ): Promise { const resolvedCwd = resolve(cwd); const files = await expandFileGlobs(patterns, resolvedCwd); - return createSyntheticFileChanges(files, resolvedCwd, options); + const basePath = resolve(options.basePath ?? resolvedCwd); + return createSyntheticFileChanges(files, basePath, { + ignore: options.ignore, + scan: options.scan, + }); } diff --git a/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts b/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts new file mode 100644 index 000000000..5e62c914f --- /dev/null +++ b/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts @@ -0,0 +1,65 @@ +import { mkdtemp, mkdir, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'vitest'; +import type { ToolDefinition } from '@earendil-works/pi-coding-agent'; +import { createCheckoutFileTools } from './pi-file-tools.js'; + +describe('createCheckoutFileTools', () => { + let testRoot: string; + let checkoutPath: string; + + beforeEach(async () => { + testRoot = await mkdtemp(join(tmpdir(), 'warden-pi-file-tools-')); + checkoutPath = join(testRoot, 'checkout'); + await mkdir(join(checkoutPath, 'src'), { recursive: true }); + await writeFile(join(checkoutPath, 'src', 'index.ts'), 'export const answer = 42;\n'); + }); + + afterEach(async () => { + await rm(testRoot, { recursive: true, force: true }); + }); + + function getTool(name: string): ToolDefinition { + const tool = createCheckoutFileTools(checkoutPath, ['read', 'grep', 'find', 'ls']) + .find((candidate) => candidate.name === name); + if (!tool) { + throw new Error(`Missing ${name} tool`); + } + return tool; + } + + function executeTool(name: string, params: Record) { + return getTool(name).execute('tool-1', params, undefined, undefined, undefined as never); + } + + it('allows repository-relative file access', async () => { + const read = getTool('read'); + const result = await executeTool('read', { path: 'src/index.ts' }); + + expect(result.content).toEqual([ + expect.objectContaining({ type: 'text', text: expect.stringContaining('export const answer = 42;') }), + ]); + expect(read.promptGuidelines).toContain( + 'Stay inside the current checkout. Use repository-relative paths.', + ); + }); + + it('rejects searches from the filesystem root with checkout guidance', async () => { + await expect(executeTool('grep', { + pattern: 'BillingService', + path: '/', + })).rejects.toThrow( + `Path "/" is outside the checkout at "${checkoutPath}". Stay inside the current checkout. Use repository-relative paths.`, + ); + }); + + it('rejects symlinks that resolve outside the checkout', async () => { + const outsidePath = join(testRoot, 'outside.ts'); + await writeFile(outsidePath, 'export const secret = true;\n'); + await symlink(outsidePath, join(checkoutPath, 'linked.ts')); + + await expect(executeTool('read', { path: 'linked.ts' })) + .rejects.toThrow('is outside the checkout'); + }); +}); diff --git a/packages/warden/src/sdk/runtimes/pi-file-tools.ts b/packages/warden/src/sdk/runtimes/pi-file-tools.ts new file mode 100644 index 000000000..3702ff953 --- /dev/null +++ b/packages/warden/src/sdk/runtimes/pi-file-tools.ts @@ -0,0 +1,121 @@ +import { realpath } from 'node:fs/promises'; +import { basename, dirname, isAbsolute, relative, resolve, sep } from 'node:path'; +import { + createFindToolDefinition, + createGrepToolDefinition, + createLsToolDefinition, + createReadToolDefinition, + defineTool, + type ToolDefinition, +} from '@earendil-works/pi-coding-agent'; +import type { TSchema } from '@earendil-works/pi-ai'; + +const CHECKOUT_GUIDANCE = 'Stay inside the current checkout. Use repository-relative paths.'; + +interface FileToolInput { + path?: unknown; +} + +function isMissingPathError(error: unknown): boolean { + return error instanceof Error + && 'code' in error + && (error.code === 'ENOENT' || error.code === 'ENOTDIR'); +} + +function isWithinPath(root: string, target: string): boolean { + const relativePath = relative(root, target); + return relativePath === '' + || (relativePath !== '..' + && !relativePath.startsWith(`..${sep}`) + && !isAbsolute(relativePath)); +} + +async function resolveThroughExistingAncestor(target: string): Promise { + let candidate = target; + const missingSegments: string[] = []; + + while (true) { + try { + const existingPath = await realpath(candidate); + return resolve(existingPath, ...missingSegments.reverse()); + } catch (error) { + if (!isMissingPathError(error)) { + throw error; + } + + const parent = dirname(candidate); + if (parent === candidate) { + throw error; + } + missingSegments.push(basename(candidate)); + candidate = parent; + } + } +} + +function checkoutPathError(requestedPath: string, checkoutPath: string): Error { + return new Error( + `Path "${requestedPath}" is outside the checkout at "${checkoutPath}". ${CHECKOUT_GUIDANCE}`, + ); +} + +async function assertPathWithinCheckout(checkoutPath: string, requestedPath: string): Promise { + const checkout = resolve(checkoutPath); + const target = resolve(checkout, requestedPath); + + if (!isWithinPath(checkout, target)) { + throw checkoutPathError(requestedPath, checkout); + } + + const [canonicalCheckout, canonicalTarget] = await Promise.all([ + resolveThroughExistingAncestor(checkout), + resolveThroughExistingAncestor(target), + ]); + if (!isWithinPath(canonicalCheckout, canonicalTarget)) { + throw checkoutPathError(requestedPath, checkout); + } +} + +function confineTool( + tool: ToolDefinition, + checkoutPath: string, +): ToolDefinition { + return defineTool({ + ...tool, + description: `${tool.description} Paths must stay within the current checkout.`, + promptGuidelines: [ + ...(tool.promptGuidelines ?? []), + CHECKOUT_GUIDANCE, + ], + async execute(toolCallId, params, signal, onUpdate, context) { + const input = params as FileToolInput; + const requestedPath = typeof input.path === 'string' ? input.path : '.'; + await assertPathWithinCheckout(checkoutPath, requestedPath); + return tool.execute(toolCallId, params, signal, onUpdate, context); + }, + }); +} + +/** Create Pi file tools that reject reads and searches outside the current checkout. */ +export function createCheckoutFileTools( + checkoutPath: string, + toolNames: readonly string[], +): ToolDefinition[] { + const requestedTools = new Set(toolNames); + const tools: ToolDefinition[] = []; + + if (requestedTools.has('read')) { + tools.push(confineTool(createReadToolDefinition(checkoutPath), checkoutPath)); + } + if (requestedTools.has('grep')) { + tools.push(confineTool(createGrepToolDefinition(checkoutPath), checkoutPath)); + } + if (requestedTools.has('find')) { + tools.push(confineTool(createFindToolDefinition(checkoutPath), checkoutPath)); + } + if (requestedTools.has('ls')) { + tools.push(confineTool(createLsToolDefinition(checkoutPath), checkoutPath)); + } + + return tools; +} diff --git a/packages/warden/src/sdk/runtimes/pi.test.ts b/packages/warden/src/sdk/runtimes/pi.test.ts index d34e77290..f2079483d 100644 --- a/packages/warden/src/sdk/runtimes/pi.test.ts +++ b/packages/warden/src/sdk/runtimes/pi.test.ts @@ -73,6 +73,11 @@ vi.mock('@earendil-works/pi-ai', () => ({ }, })); +vi.mock('./pi-file-tools.js', () => ({ + createCheckoutFileTools: vi.fn((_cwd: string, toolNames: readonly string[]) => + toolNames.map((name) => ({ name }))), +})); + vi.mock('@earendil-works/pi-coding-agent', () => ({ DefaultResourceLoader: vi.fn(function (options: unknown) { piMocks.resourceLoaderOptions.push(options); @@ -313,7 +318,12 @@ describe('piRuntime.runSkill', () => { modelRuntime: piMocks.modelRuntime, model: piMocks.model, tools: ['read', 'grep', 'find', 'ls'], - customTools: undefined, + customTools: [ + expect.objectContaining({ name: 'read' }), + expect.objectContaining({ name: 'grep' }), + expect.objectContaining({ name: 'find' }), + expect.objectContaining({ name: 'ls' }), + ], resourceLoader: piMocks.resourceLoader, sessionManager: piMocks.sessionManager, settingsManager: piMocks.settingsManager, diff --git a/packages/warden/src/sdk/runtimes/pi.ts b/packages/warden/src/sdk/runtimes/pi.ts index fea027f20..dcb3dfa51 100644 --- a/packages/warden/src/sdk/runtimes/pi.ts +++ b/packages/warden/src/sdk/runtimes/pi.ts @@ -52,6 +52,7 @@ import { import { aggregateUsage, emptyUsage } from '../usage.js'; import { InvalidPiModelSelectorError, isPiModelSelector } from './model-selectors.js'; import { isWardenOffline } from '../offline.js'; +import { createCheckoutFileTools } from './pi-file-tools.js'; import type { AuxiliaryRunRequest, AuxiliaryRunResult, @@ -594,6 +595,15 @@ async function runPiPrompt(options: PiPromptOptions): Promise { const startedAt = Date.now(); const activeToolSpans = new Map(); const conversationMessages: GenAiMessage[] = [{ role: 'user', content: options.userPrompt }]; + // Pi uses cwd for path resolution but does not treat it as a filesystem boundary. + // Same-name custom tools override its built-ins, so file access stays in the checkout. + const customTools = options.customTools ?? []; + const customToolNames = new Set(customTools.map((tool) => tool.name)); + const checkoutFileTools = createCheckoutFileTools( + options.cwd, + options.toolNames.filter((toolName) => !customToolNames.has(toolName)), + ); + const sessionCustomTools = [...customTools, ...checkoutFileTools]; const buildToolAttributes = (args: { toolName: string; @@ -725,7 +735,7 @@ async function runPiPrompt(options: PiPromptOptions): Promise { thinkingLevel: options.effort, tools: options.toolNames, noTools: options.toolNames.length === 0 ? 'all' : undefined, - customTools: options.customTools, + customTools: sessionCustomTools.length > 0 ? sessionCustomTools : undefined, resourceLoader, sessionManager: SessionManager.inMemory(options.cwd), settingsManager, From 841a38d386fffdaa8096cdf36ed0b7b4e5b50190 Mon Sep 17 00:00:00 2001 From: Greg Pstrucha <875316+gricha@users.noreply.github.com> Date: Mon, 24 Aug 2026 10:21:38 -0700 Subject: [PATCH 2/2] fix(pi): Use validated file tool paths Execute Pi file tools with the canonical path that passed checkout confinement. This prevents Pi from reinterpreting alternate path syntax after validation and escaping the checkout. Co-Authored-By: GPT-5.6 Sol --- .../warden/src/sdk/runtimes/pi-file-tools.test.ts | 8 ++++++++ packages/warden/src/sdk/runtimes/pi-file-tools.ts | 13 ++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts b/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts index 5e62c914f..3d33aaf4a 100644 --- a/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts +++ b/packages/warden/src/sdk/runtimes/pi-file-tools.test.ts @@ -54,6 +54,14 @@ describe('createCheckoutFileTools', () => { ); }); + it.each(['@/', '~/', 'file:///'])( + 'prevents Pi from reinterpreting %s outside the checkout', + async (path) => { + await expect(executeTool('ls', { path })) + .rejects.toThrow(checkoutPath); + }, + ); + it('rejects symlinks that resolve outside the checkout', async () => { const outsidePath = join(testRoot, 'outside.ts'); await writeFile(outsidePath, 'export const secret = true;\n'); diff --git a/packages/warden/src/sdk/runtimes/pi-file-tools.ts b/packages/warden/src/sdk/runtimes/pi-file-tools.ts index 3702ff953..77fa10aca 100644 --- a/packages/warden/src/sdk/runtimes/pi-file-tools.ts +++ b/packages/warden/src/sdk/runtimes/pi-file-tools.ts @@ -1,5 +1,6 @@ import { realpath } from 'node:fs/promises'; import { basename, dirname, isAbsolute, relative, resolve, sep } from 'node:path'; +import { pathToFileURL } from 'node:url'; import { createFindToolDefinition, createGrepToolDefinition, @@ -59,7 +60,7 @@ function checkoutPathError(requestedPath: string, checkoutPath: string): Error { ); } -async function assertPathWithinCheckout(checkoutPath: string, requestedPath: string): Promise { +async function resolvePathWithinCheckout(checkoutPath: string, requestedPath: string): Promise { const checkout = resolve(checkoutPath); const target = resolve(checkout, requestedPath); @@ -74,6 +75,8 @@ async function assertPathWithinCheckout(checkoutPath: string, requestedPath: str if (!isWithinPath(canonicalCheckout, canonicalTarget)) { throw checkoutPathError(requestedPath, checkout); } + + return canonicalTarget; } function confineTool( @@ -90,8 +93,12 @@ function confineTool( async execute(toolCallId, params, signal, onUpdate, context) { const input = params as FileToolInput; const requestedPath = typeof input.path === 'string' ? input.path : '.'; - await assertPathWithinCheckout(checkoutPath, requestedPath); - return tool.execute(toolCallId, params, signal, onUpdate, context); + const confinedPath = await resolvePathWithinCheckout(checkoutPath, requestedPath); + const confinedParams = { + ...params, + path: pathToFileURL(confinedPath).href, + } as typeof params; + return tool.execute(toolCallId, confinedParams, signal, onUpdate, context); }, }); }