fix: anchor .sisyphus path check to ctx.directory to prevent false positives
- Uses path.join(ctx.directory, '.sisyphus') + sep as prefix instead of loose .includes() - Prevents false positive when .sisyphus exists in parent directories outside project root - Adds test for the false positive case (cubic review feedback)
This commit is contained in:
@@ -267,6 +267,25 @@ describe("createWriteExistingFileGuardHook", () => {
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
})
|
||||
|
||||
test("blocks write when .sisyphus is in parent path but not under ctx.directory", async () => {
|
||||
//#given
|
||||
const fakeSisyphusParent = path.join(os.tmpdir(), ".sisyphus", "evil-project")
|
||||
fs.mkdirSync(fakeSisyphusParent, { recursive: true })
|
||||
const evilFile = path.join(fakeSisyphusParent, "plan.md")
|
||||
fs.writeFileSync(evilFile, "# Evil Plan")
|
||||
const input = { tool: "Write", sessionID: "ses_1", callID: "call_1" }
|
||||
const output = { args: { filePath: evilFile, content: "# Hacked" } }
|
||||
|
||||
//#when
|
||||
const result = hook["tool.execute.before"]?.(input as any, output as any)
|
||||
|
||||
//#then
|
||||
await expect(result).rejects.toThrow("File already exists. Use edit tool instead.")
|
||||
|
||||
// cleanup
|
||||
fs.rmSync(path.join(os.tmpdir(), ".sisyphus"), { recursive: true, force: true })
|
||||
})
|
||||
|
||||
test("blocks write to existing regular file (not in .sisyphus)", async () => {
|
||||
//#given
|
||||
const regularFile = path.join(tempDir, "regular.md")
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import type { Hooks, PluginInput } from "@opencode-ai/plugin"
|
||||
import { existsSync } from "fs"
|
||||
import { resolve, isAbsolute } from "path"
|
||||
import { resolve, isAbsolute, join, sep } from "path"
|
||||
import { log } from "../../shared"
|
||||
|
||||
export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks {
|
||||
@@ -20,7 +20,8 @@ export function createWriteExistingFileGuardHook(ctx: PluginInput): Hooks {
|
||||
const resolvedPath = isAbsolute(filePath) ? filePath : resolve(ctx.directory, filePath)
|
||||
|
||||
if (existsSync(resolvedPath)) {
|
||||
const isSisyphusMarkdown = resolvedPath.replace(/\\/g, "/").includes("/.sisyphus/") && filePath.endsWith(".md")
|
||||
const sisyphusRoot = join(ctx.directory, ".sisyphus") + sep
|
||||
const isSisyphusMarkdown = resolvedPath.startsWith(sisyphusRoot) && resolvedPath.endsWith(".md")
|
||||
if (isSisyphusMarkdown) {
|
||||
log("[write-existing-file-guard] Allowing .sisyphus/*.md overwrite", {
|
||||
sessionID: input.sessionID,
|
||||
|
||||
Reference in New Issue
Block a user