diff --git a/.changeset/fix-checkpoint-cleanup-parent-access.md b/.changeset/fix-checkpoint-cleanup-parent-access.md new file mode 100644 index 00000000000..22774c4b223 --- /dev/null +++ b/.changeset/fix-checkpoint-cleanup-parent-access.md @@ -0,0 +1,6 @@ +--- +"@kilocode/cli": patch +"kilo-code": patch +--- + +Avoid checkpoint cleanup errors when deleting worktrees under protected parent folders. diff --git a/packages/opencode/src/kilocode/snapshot/cleanup.ts b/packages/opencode/src/kilocode/snapshot/cleanup.ts index 762352610ac..f6645600145 100644 --- a/packages/opencode/src/kilocode/snapshot/cleanup.ts +++ b/packages/opencode/src/kilocode/snapshot/cleanup.ts @@ -31,56 +31,41 @@ export namespace KiloSnapshotCleanup { const alias = (value: string, canonical: string) => process.platform === "darwin" && - ((value === "/var" && canonical === "/private/var") || (value === "/tmp" && canonical === "/private/tmp")) + canonical === `/private${value}` && + ["/var", "/tmp"].some((root) => value === root || value.startsWith(`${root}/`)) const inspect = Effect.fnUntraced(function* (fs: FSUtil.Interface, target: string) { - const root = path.parse(target).root - const parts = path.relative(root, target).split(path.sep).filter(Boolean) - let current = root - let canonical = yield* fs - .realPath(root) - .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(root))) - - for (const [index, name] of parts.entries()) { - const info = yield* fs - .stat(current) - .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))) - if (!info) { - return { - canonical: path.join(canonical, ...parts.slice(index)), - exists: false, - } satisfies Checked - } - if (info.type !== "Directory") return yield* Effect.fail(new Error("trusted path parent is not a directory")) + const missing: string[] = [] + let current = target - const entries = yield* fs.readDirectoryEntries(current) - const entry = entries.find((item) => item.name === name) - if (!entry) { - canonical = yield* fs.realPath(current) + while (true) { + const canonical = yield* fs + .realPath(current) + .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))) + if (canonical !== undefined) { + if (normalized(canonical) !== normalized(current) && !alias(current, canonical)) + return yield* Effect.fail(new Error("trusted path contains an unexpected symlink")) + const info = yield* fs.stat(current) + if (missing.length > 0 && info.type !== "Directory") + return yield* Effect.fail(new Error("trusted path parent is not a directory")) + if (normalized(yield* fs.realPath(current)) !== normalized(canonical)) + return yield* Effect.fail(new Error("trusted path changed during inspection")) return { - canonical: path.join(canonical, ...parts.slice(index)), - exists: false, + canonical: path.join(canonical, ...missing), + exists: missing.length === 0, + type: info.type === "Directory" ? "directory" : "other", } satisfies Checked } - const next = path.join(current, name) - const real = yield* fs.realPath(next) - const again = (yield* fs.readDirectoryEntries(current)).find((item) => item.name === name) - if (!again || (again.type === "symlink" && !alias(next, real)) || again.type !== entry.type) - return yield* Effect.fail(new Error("trusted path contains an unexpected symlink")) - - current = next - canonical = real - if (index === parts.length - 1) { - return { - canonical, - exists: true, - type: again.type === "symlink" && alias(next, real) ? "directory" : again.type, - } satisfies Checked - } + const link = yield* fs + .readLink(current) + .pipe(Effect.catchReason("PlatformError", "NotFound", () => Effect.succeed(undefined))) + if (link !== undefined) return yield* Effect.fail(new Error("trusted path contains an unexpected symlink")) + const parent = path.dirname(current) + if (parent === current) return yield* Effect.fail(new Error("trusted path root is missing")) + missing.unshift(path.basename(current)) + current = parent } - - return { canonical, exists: true, type: "directory" } satisfies Checked }) const dir = (value: Checked, name: string) => { diff --git a/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts b/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts index 03b28457331..e5566433a3f 100644 --- a/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts +++ b/packages/opencode/test/kilocode/snapshot-repository-cleanup.test.ts @@ -297,6 +297,36 @@ it.live("removes an absent snapshot repository idempotently", () => }), ) +for (const stored of [false, true]) { + it.live(`cleans ${stored ? "existing" : "absent"} checkpoints without accessing workspace ancestors`, () => + Effect.gen(function* () { + const base = yield* tmpdirScoped() + const input = item(path.join(base, "allowed")) + const current = stored ? yield* repo(input) : undefined + const fs = yield* FSUtil.Service + const flock = yield* EffectFlock.Service + yield* fs.ensureDir(path.dirname(input.worktree)).pipe(Effect.orDie) + yield* drop(input.worktree) + const denied = Effect.die(new Error("Workspace ancestor access denied")) + + expect( + yield* KiloSnapshotCleanup.remove({ + ...input, + flock, + fs: { + ...fs, + realPath: (target) => (target === base ? denied : fs.realPath(target)), + stat: (target) => (target === base ? denied : fs.stat(target)), + readDirectoryEntries: (target) => (target === base ? denied : fs.readDirectoryEntries(target)), + }, + }), + ).toBe(true) + if (current) expect(yield* exist(current.dir)).toBe(false) + expect(yield* exist(input.directory)).toBe(true) + }), + ) +} + it.live("removes safely when the snapshot root, project, or repository is missing", () => Effect.gen(function* () { const base = yield* tmpdirScoped() @@ -384,6 +414,23 @@ it.live("rejects a symlinked snapshot root", () => }), ) +it.live("rejects symlinks in ancestors of the workspace and snapshot root", () => + Effect.gen(function* () { + const base = yield* tmpdirScoped() + const ancestor = path.join(base, "ancestor") + const input = item(ancestor) + const current = yield* repo(input) + yield* drop(input.worktree) + const fs = yield* FSUtil.Service + const outside = path.join(base, "outside") + yield* fs.rename(ancestor, outside) + yield* link(outside, ancestor) + + expect(Exit.isFailure(yield* remove(input).pipe(Effect.exit))).toBe(true) + expect(yield* exist(current.dir)).toBe(true) + }), +) + it.live("rejects a symlinked snapshot project", () => Effect.gen(function* () { const base = yield* tmpdirScoped()