fix(core): authorize patch move destinations (#38133)

This commit is contained in:
Aiden Cline
2026-07-21 12:18:23 -05:00
committed by GitHub
parent 99b2e78d75
commit dbfbb13ccc
4 changed files with 58 additions and 4 deletions
+1
View File
@@ -189,6 +189,7 @@ export const Plugin = {
catch: (error) => new ToolFailure({ message: `patch verification failed: ${String(error)}` }),
})
const moveTarget = hunk.movePath ? resolveTarget(location, hunk.movePath) : undefined
if (moveTarget) targets.push(moveTarget)
if (moveTarget?.externalDirectory) {
yield* permission.assert({
action: "external_directory",
+31
View File
@@ -40,6 +40,18 @@ describe("Patch", () => {
expect(() => parse("*** Begin Patch\n*** Add File: add.txt\n+added")).toThrow(
"The last line of the patch must be '*** End Patch'",
)
expect(() => parse("extra\n*** Begin Patch\n*** End Patch")).toThrow(
"The first line of the patch must be '*** Begin Patch'",
)
expect(() => parse("*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch\nextra")).toThrow(
"The last line of the patch must be '*** End Patch'",
)
})
test("allows whitespace after the end marker", () => {
expect(parse("*** Begin Patch\n*** Add File: add.txt\n+added\n*** End Patch\n \t\n")).toEqual([
{ type: "add", path: "add.txt", contents: "added" },
])
})
test("strips a heredoc wrapper", () => {
@@ -169,6 +181,25 @@ describe("Patch", () => {
).toBe('He said "hi"\n')
})
test("matches Unicode minus signs and spaces", () => {
expect(
Patch.derive("minus.txt", [{ oldLines: ["value - 1"], newLines: ["value - 2"] }], "value 1\n")
.content,
).toBe("value - 2\n")
const spaces = ["\u00A0", "\u2002", "\u2003", "\u2004", "\u2005", "\u2006", "\u2007", "\u2008", "\u2009", "\u200A", "\u202F", "\u205F", "\u3000"]
spaces.forEach(
(space) => {
expect(
Patch.derive(
"spaces.txt",
[{ oldLines: ["hello world"], newLines: ["hello there"] }],
`hello${space}world\n`,
).content,
).toBe("hello there\n")
},
)
})
test("matches EOF-anchored chunks from the end", () => {
expect(
Patch.derive(
+22
View File
@@ -351,6 +351,28 @@ describe("PatchTool", () => {
),
)
it.live("includes the move destination in edit permission resources", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
const source = path.join(directory, "old", "name.txt")
yield* Effect.promise(() => fs.mkdir(path.dirname(source), { recursive: true }))
yield* Effect.promise(() => fs.writeFile(source, "old content\n"))
yield* executeTool(
registry,
call(
"*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch",
),
)
expect(assertions).toMatchObject([
{
action: "edit",
resources: ["old/name.txt", "renamed/dir/name.txt"],
},
])
}),
),
)
it.live("inserts lines with an insert-only hunk", () =>
withTempTool((directory, registry) =>
Effect.gen(function* () {
+4 -4
View File
@@ -47,8 +47,8 @@ export function parse(patchText: string): Result.Result<ReadonlyArray<Hunk>, Par
const lines = stripHeredoc(patchText.trim())
.split("\n")
.map((line) => (line.endsWith("\r") ? line.slice(0, -1) : line))
const begin = lines.findIndex((line) => line.trim() === "*** Begin Patch")
const end = lines.findIndex((line) => line.trim() === "*** End Patch")
const begin = lines[0]?.trim() === "*** Begin Patch" ? 0 : -1
const end = lines.at(-1)?.trim() === "*** End Patch" ? lines.length - 1 : -1
if (begin === -1) return Result.fail(new BoundaryError({ boundary: "first" }))
if (end === -1 || begin >= end) return Result.fail(new BoundaryError({ boundary: "last" }))
@@ -227,9 +227,9 @@ const normalize = (value: string) =>
value
.replace(/[‘’‚‛]/g, "'")
.replace(/[“”„‟]/g, '"')
.replace(/[‐‑‒–—―]/g, "-")
.replace(/[‐‑‒–—―]/g, "-")
.replace(/…/g, "...")
.replace(/ /g, " ")
.replace(/[\u00A0\u2002-\u200A\u202F\u205F\u3000]/g, " ")
const splitBom = (text: string) =>
text.startsWith("\uFEFF") ? { bom: true, text: text.slice(1) } : { bom: false, text }
const stripHeredoc = (input: string) =>