fix(task-tool): add task ID validation and improve lock acquisition safety
- Add task ID pattern validation (T-[A-Za-z0-9-]+) to prevent path traversal - Refactor lock mechanism to use UUID-based IDs for reliable ownership tracking - Implement atomic lock creation with stale lock detection and cleanup - Add lock acquisition checks in create/update/delete handlers - Expand task-reminder hook to track split tool names and clean up on session deletion - Add comprehensive test coverage for validation and lock handling
This commit is contained in:
@@ -351,6 +351,22 @@ describe("task_tool", () => {
|
||||
expect(result.task).toBeNull()
|
||||
})
|
||||
|
||||
test("rejects invalid task id", async () => {
|
||||
//#given
|
||||
const args = {
|
||||
action: "get" as const,
|
||||
id: "../package",
|
||||
}
|
||||
|
||||
//#when
|
||||
const resultStr = await taskTool.execute(args, TEST_CONTEXT)
|
||||
const result = JSON.parse(resultStr)
|
||||
|
||||
//#then
|
||||
expect(result).toHaveProperty("error")
|
||||
expect(result.error).toBe("invalid_task_id")
|
||||
})
|
||||
|
||||
test("returns result as JSON string with task property", async () => {
|
||||
//#given
|
||||
const testId = await createTestTask("Test task")
|
||||
@@ -480,6 +496,41 @@ describe("task_tool", () => {
|
||||
expect(result.error).toBe("task_not_found")
|
||||
})
|
||||
|
||||
test("rejects invalid task id", async () => {
|
||||
//#given
|
||||
const args = {
|
||||
action: "update" as const,
|
||||
id: "../package",
|
||||
title: "New title",
|
||||
}
|
||||
|
||||
//#when
|
||||
const resultStr = await taskTool.execute(args, TEST_CONTEXT)
|
||||
const result = JSON.parse(resultStr)
|
||||
|
||||
//#then
|
||||
expect(result).toHaveProperty("error")
|
||||
expect(result.error).toBe("invalid_task_id")
|
||||
})
|
||||
|
||||
test("returns lock unavailable when lock is held", async () => {
|
||||
//#given
|
||||
writeFileSync(join(TEST_DIR, ".lock"), JSON.stringify({ id: "test", timestamp: Date.now() }))
|
||||
const args = {
|
||||
action: "update" as const,
|
||||
id: "T-nonexistent",
|
||||
title: "New title",
|
||||
}
|
||||
|
||||
//#when
|
||||
const resultStr = await taskTool.execute(args, TEST_CONTEXT)
|
||||
const result = JSON.parse(resultStr)
|
||||
|
||||
//#then
|
||||
expect(result).toHaveProperty("error")
|
||||
expect(result.error).toBe("task_lock_unavailable")
|
||||
})
|
||||
|
||||
test("returns result as JSON string with task property", async () => {
|
||||
//#given
|
||||
const testId = await createTestTask("Test task")
|
||||
@@ -574,6 +625,22 @@ describe("task_tool", () => {
|
||||
expect(result.error).toBe("task_not_found")
|
||||
})
|
||||
|
||||
test("rejects invalid task id", async () => {
|
||||
//#given
|
||||
const args = {
|
||||
action: "delete" as const,
|
||||
id: "../package",
|
||||
}
|
||||
|
||||
//#when
|
||||
const resultStr = await taskTool.execute(args, TEST_CONTEXT)
|
||||
const result = JSON.parse(resultStr)
|
||||
|
||||
//#then
|
||||
expect(result).toHaveProperty("error")
|
||||
expect(result.error).toBe("invalid_task_id")
|
||||
})
|
||||
|
||||
test("returns result as JSON string", async () => {
|
||||
//#given
|
||||
const testId = await createTestTask("Test task")
|
||||
|
||||
+34
-3
@@ -27,6 +27,13 @@ import {
|
||||
listTaskFiles,
|
||||
} from "../../features/claude-tasks/storage"
|
||||
|
||||
const TASK_ID_PATTERN = /^T-[A-Za-z0-9-]+$/
|
||||
|
||||
function parseTaskId(id: string): string | null {
|
||||
if (!TASK_ID_PATTERN.test(id)) return null
|
||||
return id
|
||||
}
|
||||
|
||||
export function createTask(config: Partial<OhMyOpenCodeConfig>): ToolDefinition {
|
||||
return tool({
|
||||
description: `Unified task management tool with create, list, get, update, delete actions.
|
||||
@@ -88,6 +95,10 @@ async function handleCreate(
|
||||
const taskDir = getTaskDir(config)
|
||||
const lock = acquireLock(taskDir)
|
||||
|
||||
if (!lock.acquired) {
|
||||
return JSON.stringify({ error: "task_lock_unavailable" })
|
||||
}
|
||||
|
||||
try {
|
||||
const taskId = generateTaskId()
|
||||
const task: TaskObject = {
|
||||
@@ -176,8 +187,12 @@ async function handleGet(
|
||||
config: Partial<OhMyOpenCodeConfig>
|
||||
): Promise<string> {
|
||||
const validatedArgs = TaskGetInputSchema.parse(args)
|
||||
const taskId = parseTaskId(validatedArgs.id)
|
||||
if (!taskId) {
|
||||
return JSON.stringify({ error: "invalid_task_id" })
|
||||
}
|
||||
const taskDir = getTaskDir(config)
|
||||
const taskPath = join(taskDir, `${validatedArgs.id}.json`)
|
||||
const taskPath = join(taskDir, `${taskId}.json`)
|
||||
|
||||
const task = readJsonSafe(taskPath, TaskObjectSchema)
|
||||
|
||||
@@ -189,11 +204,19 @@ async function handleUpdate(
|
||||
config: Partial<OhMyOpenCodeConfig>
|
||||
): Promise<string> {
|
||||
const validatedArgs = TaskUpdateInputSchema.parse(args)
|
||||
const taskId = parseTaskId(validatedArgs.id)
|
||||
if (!taskId) {
|
||||
return JSON.stringify({ error: "invalid_task_id" })
|
||||
}
|
||||
const taskDir = getTaskDir(config)
|
||||
const lock = acquireLock(taskDir)
|
||||
|
||||
if (!lock.acquired) {
|
||||
return JSON.stringify({ error: "task_lock_unavailable" })
|
||||
}
|
||||
|
||||
try {
|
||||
const taskPath = join(taskDir, `${validatedArgs.id}.json`)
|
||||
const taskPath = join(taskDir, `${taskId}.json`)
|
||||
const task = readJsonSafe(taskPath, TaskObjectSchema)
|
||||
|
||||
if (!task) {
|
||||
@@ -234,11 +257,19 @@ async function handleDelete(
|
||||
config: Partial<OhMyOpenCodeConfig>
|
||||
): Promise<string> {
|
||||
const validatedArgs = TaskDeleteInputSchema.parse(args)
|
||||
const taskId = parseTaskId(validatedArgs.id)
|
||||
if (!taskId) {
|
||||
return JSON.stringify({ error: "invalid_task_id" })
|
||||
}
|
||||
const taskDir = getTaskDir(config)
|
||||
const lock = acquireLock(taskDir)
|
||||
|
||||
if (!lock.acquired) {
|
||||
return JSON.stringify({ error: "task_lock_unavailable" })
|
||||
}
|
||||
|
||||
try {
|
||||
const taskPath = join(taskDir, `${validatedArgs.id}.json`)
|
||||
const taskPath = join(taskDir, `${taskId}.json`)
|
||||
|
||||
if (!existsSync(taskPath)) {
|
||||
return JSON.stringify({ error: "task_not_found" })
|
||||
|
||||
Reference in New Issue
Block a user