Skip to content

Commit 0bfcbb1

Browse files
authored
Merge pull request #417 from alvinunreal/fix/apply-patch-external-directory-pass-through
Let native apply_patch handle external paths
2 parents 0dd5541 + 3ca60f5 commit 0bfcbb1

2 files changed

Lines changed: 91 additions & 8 deletions

File tree

src/hooks/apply-patch/hook.test.ts

Lines changed: 72 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,74 @@ PATCH`,
115115
});
116116
});
117117

118+
test('passes through an absolute target outside root/worktree before native execution', async () => {
119+
const root = await createTempDir('apply-patch-hook-');
120+
const outsideDir = await createTempDir('apply-patch-hook-outside-');
121+
const outsidePath = path.join(outsideDir, 'outside.txt');
122+
await writeFile(outsidePath, 'outside\n', 'utf-8');
123+
const hook = createApplyPatchHook({
124+
client: {} as never,
125+
directory: root,
126+
worktree: root,
127+
} as never);
128+
const patchText = `*** Begin Patch
129+
*** Update File: ${outsidePath}
130+
@@
131+
-outside
132+
+changed
133+
*** End Patch`;
134+
const output = { args: { patchText } };
135+
136+
await expect(
137+
hook['tool.execute.before'](
138+
{ tool: 'apply_patch', directory: root },
139+
output,
140+
),
141+
).resolves.toBeUndefined();
142+
143+
expect(output.args.patchText).toBe(patchText);
144+
expect(await readFile(outsidePath, 'utf-8')).toBe('outside\n');
145+
});
146+
147+
test('passes through mixed stale inside and absolute outside patch without partial rewrite', async () => {
148+
const root = await createTempDir('apply-patch-hook-');
149+
const outsideDir = await createTempDir('apply-patch-hook-outside-');
150+
const outsidePath = path.join(outsideDir, 'outside.txt');
151+
await writeFixture(root, 'sample.txt', 'prefix\nstale-value\nsuffix\n');
152+
await writeFile(outsidePath, 'outside\n', 'utf-8');
153+
const hook = createApplyPatchHook({
154+
client: {} as never,
155+
directory: root,
156+
worktree: root,
157+
} as never);
158+
const patchText = `*** Begin Patch
159+
*** Update File: sample.txt
160+
@@
161+
prefix
162+
-old-value
163+
+new-value
164+
suffix
165+
*** Update File: ${outsidePath}
166+
@@
167+
-outside
168+
+changed
169+
*** End Patch`;
170+
const output = { args: { patchText } };
171+
172+
await expect(
173+
hook['tool.execute.before'](
174+
{ tool: 'apply_patch', directory: root },
175+
output,
176+
),
177+
).resolves.toBeUndefined();
178+
179+
expect(output.args.patchText).toBe(patchText);
180+
expect(await readFile(path.join(root, 'sample.txt'), 'utf-8')).toBe(
181+
'prefix\nstale-value\nsuffix\n',
182+
);
183+
expect(await readFile(outsidePath, 'utf-8')).toBe('outside\n');
184+
});
185+
118186
test('rewrites a stale prefix patch and remains applicable', async () => {
119187
const root = await createTempDir('apply-patch-hook-');
120188
await writeFixture(
@@ -555,7 +623,7 @@ garbage
555623
);
556624
});
557625

558-
test('aborts early when the patch only targets outside root/worktree', async () => {
626+
test('passes through sibling-directory targets outside root/worktree before native execution', async () => {
559627
const root = await createTempDir('apply-patch-hook-');
560628
const outside = path.join(path.dirname(root), 'outside.txt');
561629
await writeFile(outside, 'outside\n', 'utf-8');
@@ -573,9 +641,7 @@ garbage
573641
{ tool: 'apply_patch', directory: root },
574642
output,
575643
),
576-
).rejects.toThrow(
577-
`apply_patch blocked: patch contains path outside workspace root: ${outside}`,
578-
);
644+
).resolves.toBeUndefined();
579645

580646
expect(output.args.patchText).toBe(patchText);
581647
expect(await readFile(outside, 'utf-8')).toBe('outside\n');
@@ -611,7 +677,7 @@ garbage
611677
});
612678
});
613679

614-
test('aborts early and applies nothing when a mixed patch has outside paths', async () => {
680+
test('passes through mixed patches with outside paths without partial rewrite', async () => {
615681
const root = await createTempDir('apply-patch-hook-');
616682
const outsideDir = await createTempDir('apply-patch-hook-outside-');
617683
await writeFixture(root, 'sample.txt', 'prefix\nstale-value\nsuffix\n');
@@ -636,9 +702,7 @@ garbage
636702
{ tool: 'apply_patch', directory: root },
637703
output,
638704
),
639-
).rejects.toThrow(
640-
`apply_patch blocked: patch contains path outside workspace root: ${outsideAdded}`,
641-
);
705+
).resolves.toBeUndefined();
642706

643707
expect(output.args.patchText).toBe(patchText);
644708
expect(await readFile(path.join(root, 'sample.txt'), 'utf-8')).toBe(

src/hooks/apply-patch/index.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export function createApplyPatchHook(ctx: PluginInput) {
3232
state:
3333
| 'rewrite'
3434
| 'unchanged'
35+
| 'skipped'
3536
| 'blocked'
3637
| 'validation'
3738
| 'verification'
@@ -83,6 +84,24 @@ export function createApplyPatchHook(ctx: PluginInput) {
8384
);
8485
const details = getApplyPatchErrorDetails(normalizedError);
8586

87+
if (
88+
normalizedError.kind === 'blocked' &&
89+
// Only the plugin-side outside-workspace preflight should fail open.
90+
// Keep the code check explicit so any future blocked error remains
91+
// fail-closed by default.
92+
details?.code === 'outside_workspace'
93+
) {
94+
logHookStatus('skipped', {
95+
kind: details.kind,
96+
code: details.code,
97+
reason: normalizedError.message,
98+
failOpen: true,
99+
rescueOptions: APPLY_PATCH_RESCUE_OPTIONS,
100+
rewriteStage: 'before-native',
101+
});
102+
return;
103+
}
104+
86105
logHookStatus(
87106
isApplyPatchVerificationError(normalizedError)
88107
? 'verification'

0 commit comments

Comments
 (0)