fix(council-finalize): patch path traversal vulnerabilities in name, task_ids, and prompt_file

2026-03-01 15:51:15 +01:00
parent a33a3d1095
commit a13cc7b877
2 changed files with 235 additions and 8 deletions
@@ -389,4 +389,177 @@ describe("createCouncilFinalize", () => {
      expect(result).toContain("NOT_A_REAL_INTENT")
    })
  })
+
+  describe("#given path traversal attempts", () => {
+    it("#then rejects task IDs with path traversal characters", async () => {
+      await writeFile(
+        join(tmpDir, ".sisyphus", "task-outputs", "bg_valid.md"),
+        mockTaskOutput("Agent", "Valid response"),
+        "utf-8",
+      )
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["../../etc/passwd", "bg_valid", "foo/bar"], name: "traversal" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.members).toHaveLength(3)
+      expect(result.members[0].has_response).toBe(false)
+      expect(result.members[0].error).toContain("Invalid task ID")
+      expect(result.members[1].has_response).toBe(true)
+      expect(result.members[2].has_response).toBe(false)
+      expect(result.members[2].error).toContain("Invalid task ID")
+    })
+
+    it("#then sanitizes name with path traversal characters", async () => {
+      await writeFile(
+        join(tmpDir, ".sisyphus", "task-outputs", "bg_safe.md"),
+        mockTaskOutput("Agent", "Response"),
+        "utf-8",
+      )
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_safe"], name: "../../etc" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.archive_dir).toMatch(/^\.sisyphus\/athena\/council-/)
+      expect(result.archive_dir).not.toContain("..")
+    })
+
+    it("#then sanitizes name with absolute path", async () => {
+      await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "bg_abs.md"), mockTaskOutput("Agent", "Response"), "utf-8")
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_abs"], name: "/absolute/path" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.archive_dir).toMatch(/^\.sisyphus\/athena\/council-/)
+      expect(result.archive_dir).not.toContain("/absolute/path")
+    })
+
+    it("#then sanitizes name with dot-dot-slash in the middle", async () => {
+      await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "bg_mid.md"), mockTaskOutput("Agent", "Response"), "utf-8")
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_mid"], name: "foo/../bar" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.archive_dir).toMatch(/^\.sisyphus\/athena\/council-/)
+      expect(result.archive_dir).not.toContain("..")
+    })
+
+    it("#then accepts valid task IDs", async () => {
+      await writeFile(join(tmpDir, ".sisyphus", "task-outputs", "valid-id_123.md"), mockTaskOutput("Agent", "Response"), "utf-8")
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["valid-id_123"], name: "valid" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.members).toHaveLength(1)
+      expect(result.members[0].has_response).toBe(true)
+      expect(result.members[0].error).toBeUndefined()
+    })
+
+    it("#then rejects prompt_file with absolute path outside workspace", async () => {
+      await writeFile(
+        join(tmpDir, ".sisyphus", "task-outputs", "bg_prompt.md"),
+        mockTaskOutput("Agent", "Response"),
+        "utf-8",
+      )
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_prompt"], name: "prompt-test", prompt_file: "/etc/passwd" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.archive_dir).toBeDefined()
+      const metaContent = await readFile(join(tmpDir, result.meta_file), "utf-8")
+      expect(metaContent).not.toContain("prompt_file:")
+    })
+
+    it("#then rejects prompt_file with relative traversal", async () => {
+      await writeFile(
+        join(tmpDir, ".sisyphus", "task-outputs", "bg_rel.md"),
+        mockTaskOutput("Agent", "Response"),
+        "utf-8",
+      )
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_rel"], name: "rel-test", prompt_file: "../../etc/passwd" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.archive_dir).toBeDefined()
+      const metaContent = await readFile(join(tmpDir, result.meta_file), "utf-8")
+      expect(metaContent).not.toContain("prompt_file:")
+    })
+
+    it("#then accepts valid prompt_file under .sisyphus/tmp/", async () => {
+      await mkdir(join(tmpDir, ".sisyphus", "tmp"), { recursive: true })
+      await writeFile(join(tmpDir, ".sisyphus", "tmp", "athena-council-test.md"), "Test prompt", "utf-8")
+      await writeFile(
+        join(tmpDir, ".sisyphus", "task-outputs", "bg_ok.md"),
+        mockTaskOutput("Agent", "Response"),
+        "utf-8",
+      )
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_ok"], name: "valid-prompt", prompt_file: ".sisyphus/tmp/athena-council-test.md" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      const metaContent = await readFile(join(tmpDir, result.meta_file), "utf-8")
+      expect(metaContent).toContain("prompt_file:")
+    })
+
+    it("#then rejects task IDs with backslash characters", async () => {
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg\\evil"], name: "backslash" },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.members).toHaveLength(1)
+      expect(result.members[0].has_response).toBe(false)
+      expect(result.members[0].error).toContain("Invalid task ID")
+    })
+
+    it("#then handles name that slugifies to empty string", async () => {
+      await writeFile(
+        join(tmpDir, ".sisyphus", "task-outputs", "bg_empty_name.md"),
+        mockTaskOutput("Agent", "Response"),
+        "utf-8",
+      )
+
+      const toolDef = createCouncilFinalize(tmpDir)
+      const resultStr = await toolDef.execute(
+        { task_ids: ["bg_empty_name"], name: "///..." },
+        mockCtx,
+      )
+      const result: CouncilFinalizeResult = JSON.parse(resultStr)
+
+      expect(result.archive_dir).toMatch(/^\.sisyphus\/athena\/council-unnamed-[a-f0-9]{4}$/)
+    })
+  })
 })
@@ -1,6 +1,6 @@
 import { tool, type ToolDefinition } from "@opencode-ai/plugin"
 import { readFile, writeFile, mkdir, rename } from "node:fs/promises"
-import { join, isAbsolute, resolve } from "node:path"
+import { join, isAbsolute, resolve, relative } from "node:path"
 import { randomBytes } from "node:crypto"
 import { extractCouncilResponse } from "./council-response-extractor"
 import {
@@ -28,6 +28,7 @@ type CouncilFinalizeToolContext = {
  sessionID?: string
 }

+const TASK_ID_PATTERN = /^[a-zA-Z0-9_-]+$/
 function slugify(text: string): string {
  return text
    .toLowerCase()
@@ -120,21 +121,68 @@ export function createCouncilFinalize(

      const base = basePath ?? process.cwd()
      const hexId = randomBytes(2).toString("hex")
-      const archiveName = `council-${args.name}-${hexId}`
+      const safeName = slugify(args.name) || "unnamed"
+      const archiveName = `council-${safeName}-${hexId}`
      const relArchiveDir = join(".sisyphus", "athena", archiveName)
      const relArchiveDirForOutput = toPosixPath(relArchiveDir)
      const absArchiveDir = join(base, relArchiveDir)

+      const expectedArchiveRoot = join(base, ".sisyphus", "athena")
+      const relFromArchiveRoot = relative(expectedArchiveRoot, absArchiveDir)
+      if (relFromArchiveRoot.startsWith("..") || isAbsolute(relFromArchiveRoot)) {
+        return `Security error: archive directory would escape .sisyphus/athena/`
+      }
+
      await mkdir(absArchiveDir, { recursive: true })

      const members: CouncilMemberResult[] = []
      const metaMembers: MetaMember[] = []

      for (const taskId of args.task_ids) {
+        if (!TASK_ID_PATTERN.test(taskId)) {
+          members.push({
+            task_id: taskId,
+            member: "unknown",
+            has_response: false,
+            error: "Invalid task ID: contains unsafe characters",
+          })
+          metaMembers.push({
+            task_id: taskId,
+            member: "unknown",
+            member_slug: "unknown",
+            task_output_path: "",
+            archive_file: "",
+            has_response: false,
+            response_complete: false,
+          })
+          continue
+        }
+
        const relTaskOutput = join(".sisyphus", "task-outputs", `${taskId}.md`)
        const relTaskOutputForOutput = toPosixPath(relTaskOutput)
        const absTaskOutput = join(base, relTaskOutput)

+        const expectedTaskOutputRoot = join(base, ".sisyphus", "task-outputs")
+        const relFromTaskOutputRoot = relative(expectedTaskOutputRoot, absTaskOutput)
+        if (relFromTaskOutputRoot.startsWith("..") || isAbsolute(relFromTaskOutputRoot)) {
+          members.push({
+            task_id: taskId,
+            member: "unknown",
+            has_response: false,
+            error: "Invalid task ID: resolved path escapes task-outputs directory",
+          })
+          metaMembers.push({
+            task_id: taskId,
+            member: "unknown",
+            member_slug: "unknown",
+            task_output_path: "",
+            archive_file: "",
+            has_response: false,
+            response_complete: false,
+          })
+          continue
+        }
+
        let fileContent: string
        try {
          fileContent = await readFile(absTaskOutput, "utf-8")
@@ -198,12 +246,18 @@ export function createCouncilFinalize(
        try {
          const promptFilename = "council-prompt.md"
          const absPromptSrc = isAbsolute(args.prompt_file) ? args.prompt_file : resolve(base, args.prompt_file)
-          const absPromptDest = join(absArchiveDir, promptFilename)
-          await rename(absPromptSrc, absPromptDest).catch(async () => {
-            const content = await readFile(absPromptSrc, "utf-8")
-            await writeFile(absPromptDest, content, "utf-8")
-          })
-          relPromptFile = toPosixPath(join(relArchiveDir, promptFilename))
+          const expectedPromptRoot = join(base, ".sisyphus", "tmp")
+          const relFromPromptRoot = relative(expectedPromptRoot, absPromptSrc)
+          if (relFromPromptRoot.startsWith("..") || isAbsolute(relFromPromptRoot)) {
+            log("[council-finalize] Rejected prompt_file outside .sisyphus/tmp/", { promptFile: args.prompt_file })
+          } else {
+            const absPromptDest = join(absArchiveDir, promptFilename)
+            await rename(absPromptSrc, absPromptDest).catch(async () => {
+              const content = await readFile(absPromptSrc, "utf-8")
+              await writeFile(absPromptDest, content, "utf-8")
+            })
+            relPromptFile = toPosixPath(join(relArchiveDir, promptFilename))
+          }
        } catch (err) {
          log("[council-finalize] Failed to move prompt file", { promptFile: args.prompt_file, error: String(err) })
        }