when a shell tool call invokes apply_patch, resolve relative paths against workdir, if specified

This commit is contained in:
Michael Bolin
2025-04-22 12:56:07 -07:00
parent f99c9080fd
commit 012ad7e69e
3 changed files with 40 additions and 8 deletions

View File

@@ -71,13 +71,14 @@ export type ApprovalPolicy =
*/
export function canAutoApprove(
command: ReadonlyArray<string>,
workdir: string | undefined,
policy: ApprovalPolicy,
writableRoots: ReadonlyArray<string>,
env: NodeJS.ProcessEnv = process.env,
): SafetyAssessment {
if (command[0] === "apply_patch") {
return command.length === 2 && typeof command[1] === "string"
? canAutoApproveApplyPatch(command[1], writableRoots, policy)
? canAutoApproveApplyPatch(command[1], workdir, writableRoots, policy)
: {
type: "reject",
reason: "Invalid apply_patch command",
@@ -103,7 +104,12 @@ export function canAutoApprove(
) {
const applyPatchArg = tryParseApplyPatch(command[2]);
if (applyPatchArg != null) {
return canAutoApproveApplyPatch(applyPatchArg, writableRoots, policy);
return canAutoApproveApplyPatch(
applyPatchArg,
workdir,
writableRoots,
policy,
);
}
let bashCmd;
@@ -162,6 +168,7 @@ export function canAutoApprove(
function canAutoApproveApplyPatch(
applyPatchArg: string,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
policy: ApprovalPolicy,
): SafetyAssessment {
@@ -179,7 +186,13 @@ function canAutoApproveApplyPatch(
break;
}
if (isWritePatchConstrainedToWritablePaths(applyPatchArg, writableRoots)) {
if (
isWritePatchConstrainedToWritablePaths(
applyPatchArg,
workdir,
writableRoots,
)
) {
return {
type: "auto-approve",
reason: "apply_patch command is constrained to writable paths",
@@ -208,6 +221,7 @@ function canAutoApproveApplyPatch(
*/
function isWritePatchConstrainedToWritablePaths(
applyPatchArg: string,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
): boolean {
// `identify_files_needed()` returns a list of files that will be modified or
@@ -222,10 +236,12 @@ function isWritePatchConstrainedToWritablePaths(
return (
allPathsConstrainedTowritablePaths(
identify_files_needed(applyPatchArg),
workdir,
writableRoots,
) &&
allPathsConstrainedTowritablePaths(
identify_files_added(applyPatchArg),
workdir,
writableRoots,
)
);
@@ -233,19 +249,29 @@ function isWritePatchConstrainedToWritablePaths(
function allPathsConstrainedTowritablePaths(
candidatePaths: ReadonlyArray<string>,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
): boolean {
return candidatePaths.every((candidatePath) =>
isPathConstrainedTowritablePaths(candidatePath, writableRoots),
isPathConstrainedTowritablePaths(candidatePath, workdir, writableRoots),
);
}
/** If candidatePath is relative, it will be resolved against cwd. */
function isPathConstrainedTowritablePaths(
candidatePath: string,
workdir: string | undefined,
writableRoots: ReadonlyArray<string>,
): boolean {
const candidateAbsolutePath = path.resolve(candidatePath);
let candidateAbsolutePath: string;
if (path.isAbsolute(candidatePath)) {
candidateAbsolutePath = candidatePath;
} else if (workdir != null) {
candidateAbsolutePath = path.resolve(workdir, candidatePath);
} else {
candidateAbsolutePath = path.resolve(candidatePath);
}
return writableRoots.some((writablePath) =>
pathContains(writablePath, candidateAbsolutePath),
);

View File

@@ -81,7 +81,7 @@ export async function handleExecCommand(
) => Promise<CommandConfirmation>,
abortSignal?: AbortSignal,
): Promise<HandleExecCommandResult> {
const { cmd: command } = args;
const { cmd: command, workdir } = args;
const key = deriveCommandKey(command);
@@ -103,7 +103,7 @@ export async function handleExecCommand(
// working directory so that edits are constrained to the project root. If
// the caller wishes to broaden or restrict the set it can be made
// configurable in the future.
const safety = canAutoApprove(command, policy, [process.cwd()]);
const safety = canAutoApprove(command, workdir, policy, [process.cwd()]);
let runInSandbox: boolean;
switch (safety.type) {

View File

@@ -11,7 +11,13 @@ describe("canAutoApprove()", () => {
const writeablePaths: Array<string> = [];
const check = (command: ReadonlyArray<string>): SafetyAssessment =>
canAutoApprove(command, "suggest", writeablePaths, env);
canAutoApprove(
command,
/* workdir */ undefined,
"suggest",
writeablePaths,
env,
);
test("simple safe commands", () => {
expect(check(["ls"])).toEqual({