fix(agent-teams): harden task operations against traversal
This commit is contained in:
parent
0ec6afcd9e
commit
dbcad8fd97
44
src/tools/agent-teams/team-task-store.test.ts
Normal file
44
src/tools/agent-teams/team-task-store.test.ts
Normal file
@ -0,0 +1,44 @@
|
|||||||
|
/// <reference types="bun-types" />
|
||||||
|
import { afterEach, beforeEach, describe, expect, test } from "bun:test"
|
||||||
|
import { mkdtempSync, rmSync } from "node:fs"
|
||||||
|
import { tmpdir } from "node:os"
|
||||||
|
import { join } from "node:path"
|
||||||
|
import { createTeamConfig, deleteTeamData } from "./team-config-store"
|
||||||
|
import { createTeamTask, deleteTeamTaskFile, readTeamTask } from "./team-task-store"
|
||||||
|
|
||||||
|
describe("agent-teams task store", () => {
|
||||||
|
let originalCwd: string
|
||||||
|
let tempProjectDir: string
|
||||||
|
|
||||||
|
beforeEach(() => {
|
||||||
|
originalCwd = process.cwd()
|
||||||
|
tempProjectDir = mkdtempSync(join(tmpdir(), "agent-teams-task-store-"))
|
||||||
|
process.chdir(tempProjectDir)
|
||||||
|
createTeamConfig("core", "Core team", "ses-main", tempProjectDir, "sisyphus")
|
||||||
|
})
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
deleteTeamData("core")
|
||||||
|
process.chdir(originalCwd)
|
||||||
|
rmSync(tempProjectDir, { recursive: true, force: true })
|
||||||
|
})
|
||||||
|
|
||||||
|
test("creates and reads a task", () => {
|
||||||
|
//#given
|
||||||
|
const created = createTeamTask("core", "Subject", "Description")
|
||||||
|
|
||||||
|
//#when
|
||||||
|
const loaded = readTeamTask("core", created.id)
|
||||||
|
|
||||||
|
//#then
|
||||||
|
expect(loaded?.id).toBe(created.id)
|
||||||
|
expect(loaded?.subject).toBe("Subject")
|
||||||
|
})
|
||||||
|
|
||||||
|
test("rejects invalid team name and task id", () => {
|
||||||
|
//#then
|
||||||
|
expect(() => readTeamTask("../../etc", "T-1")).toThrow("team_name_invalid")
|
||||||
|
expect(() => readTeamTask("core", "../../passwd")).toThrow("task_id_invalid")
|
||||||
|
expect(() => deleteTeamTaskFile("core", "../../passwd")).toThrow("task_id_invalid")
|
||||||
|
})
|
||||||
|
})
|
||||||
@ -9,8 +9,24 @@ import {
|
|||||||
} from "../../features/claude-tasks/storage"
|
} from "../../features/claude-tasks/storage"
|
||||||
import { getTeamTaskDir, getTeamTaskPath } from "./paths"
|
import { getTeamTaskDir, getTeamTaskPath } from "./paths"
|
||||||
import { TeamTask, TeamTaskSchema } from "./types"
|
import { TeamTask, TeamTaskSchema } from "./types"
|
||||||
|
import { validateTaskId, validateTeamName } from "./name-validation"
|
||||||
|
|
||||||
|
function assertValidTeamName(teamName: string): void {
|
||||||
|
const validationError = validateTeamName(teamName)
|
||||||
|
if (validationError) {
|
||||||
|
throw new Error(validationError)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function assertValidTaskId(taskId: string): void {
|
||||||
|
const validationError = validateTaskId(taskId)
|
||||||
|
if (validationError) {
|
||||||
|
throw new Error(validationError)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function withTaskLock<T>(teamName: string, operation: () => T): T {
|
function withTaskLock<T>(teamName: string, operation: () => T): T {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
const taskDir = getTeamTaskDir(teamName)
|
const taskDir = getTeamTaskDir(teamName)
|
||||||
ensureDir(taskDir)
|
ensureDir(taskDir)
|
||||||
const lock = acquireLock(taskDir)
|
const lock = acquireLock(taskDir)
|
||||||
@ -26,6 +42,8 @@ function withTaskLock<T>(teamName: string, operation: () => T): T {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function readTeamTask(teamName: string, taskId: string): TeamTask | null {
|
export function readTeamTask(teamName: string, taskId: string): TeamTask | null {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
|
assertValidTaskId(taskId)
|
||||||
return readJsonSafe(getTeamTaskPath(teamName, taskId), TeamTaskSchema)
|
return readJsonSafe(getTeamTaskPath(teamName, taskId), TeamTaskSchema)
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -38,6 +56,7 @@ export function readTeamTaskOrThrow(teamName: string, taskId: string): TeamTask
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function listTeamTasks(teamName: string): TeamTask[] {
|
export function listTeamTasks(teamName: string): TeamTask[] {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
const taskDir = getTeamTaskDir(teamName)
|
const taskDir = getTeamTaskDir(teamName)
|
||||||
if (!existsSync(taskDir)) {
|
if (!existsSync(taskDir)) {
|
||||||
return []
|
return []
|
||||||
@ -50,6 +69,9 @@ export function listTeamTasks(teamName: string): TeamTask[] {
|
|||||||
const tasks: TeamTask[] = []
|
const tasks: TeamTask[] = []
|
||||||
for (const file of files) {
|
for (const file of files) {
|
||||||
const taskId = file.replace(/\.json$/, "")
|
const taskId = file.replace(/\.json$/, "")
|
||||||
|
if (validateTaskId(taskId)) {
|
||||||
|
continue
|
||||||
|
}
|
||||||
const task = readTeamTask(teamName, taskId)
|
const task = readTeamTask(teamName, taskId)
|
||||||
if (task) {
|
if (task) {
|
||||||
tasks.push(task)
|
tasks.push(task)
|
||||||
@ -66,6 +88,7 @@ export function createTeamTask(
|
|||||||
activeForm?: string,
|
activeForm?: string,
|
||||||
metadata?: Record<string, unknown>,
|
metadata?: Record<string, unknown>,
|
||||||
): TeamTask {
|
): TeamTask {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
if (!subject.trim()) {
|
if (!subject.trim()) {
|
||||||
throw new Error("team_task_subject_required")
|
throw new Error("team_task_subject_required")
|
||||||
}
|
}
|
||||||
@ -89,12 +112,16 @@ export function createTeamTask(
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function writeTeamTask(teamName: string, task: TeamTask): TeamTask {
|
export function writeTeamTask(teamName: string, task: TeamTask): TeamTask {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
|
assertValidTaskId(task.id)
|
||||||
const validated = TeamTaskSchema.parse(task)
|
const validated = TeamTaskSchema.parse(task)
|
||||||
writeJsonAtomic(getTeamTaskPath(teamName, validated.id), validated)
|
writeJsonAtomic(getTeamTaskPath(teamName, validated.id), validated)
|
||||||
return validated
|
return validated
|
||||||
}
|
}
|
||||||
|
|
||||||
export function deleteTeamTaskFile(teamName: string, taskId: string): void {
|
export function deleteTeamTaskFile(teamName: string, taskId: string): void {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
|
assertValidTaskId(taskId)
|
||||||
const taskPath = getTeamTaskPath(teamName, taskId)
|
const taskPath = getTeamTaskPath(teamName, taskId)
|
||||||
if (existsSync(taskPath)) {
|
if (existsSync(taskPath)) {
|
||||||
unlinkSync(taskPath)
|
unlinkSync(taskPath)
|
||||||
@ -102,10 +129,12 @@ export function deleteTeamTaskFile(teamName: string, taskId: string): void {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function readTaskFromDirectory(taskDir: string, taskId: string): TeamTask | null {
|
export function readTaskFromDirectory(taskDir: string, taskId: string): TeamTask | null {
|
||||||
|
assertValidTaskId(taskId)
|
||||||
return readJsonSafe(join(taskDir, `${taskId}.json`), TeamTaskSchema)
|
return readJsonSafe(join(taskDir, `${taskId}.json`), TeamTaskSchema)
|
||||||
}
|
}
|
||||||
|
|
||||||
export function resetOwnerTasks(teamName: string, ownerName: string): void {
|
export function resetOwnerTasks(teamName: string, ownerName: string): void {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
withTaskLock(teamName, () => {
|
withTaskLock(teamName, () => {
|
||||||
const tasks = listTeamTasks(teamName)
|
const tasks = listTeamTasks(teamName)
|
||||||
for (const task of tasks) {
|
for (const task of tasks) {
|
||||||
@ -123,5 +152,6 @@ export function resetOwnerTasks(teamName: string, ownerName: string): void {
|
|||||||
}
|
}
|
||||||
|
|
||||||
export function withTeamTaskLock<T>(teamName: string, operation: () => T): T {
|
export function withTeamTaskLock<T>(teamName: string, operation: () => T): T {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
return withTaskLock(teamName, operation)
|
return withTaskLock(teamName, operation)
|
||||||
}
|
}
|
||||||
|
|||||||
@ -1,6 +1,7 @@
|
|||||||
import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
|
import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
|
||||||
import { sendStructuredInboxMessage } from "./inbox-store"
|
import { sendStructuredInboxMessage } from "./inbox-store"
|
||||||
import { readTeamConfigOrThrow } from "./team-config-store"
|
import { readTeamConfigOrThrow } from "./team-config-store"
|
||||||
|
import { validateAgentName, validateTaskId, validateTeamName } from "./name-validation"
|
||||||
import {
|
import {
|
||||||
TeamTaskCreateInputSchema,
|
TeamTaskCreateInputSchema,
|
||||||
TeamTaskGetInputSchema,
|
TeamTaskGetInputSchema,
|
||||||
@ -32,6 +33,10 @@ export function createTeamTaskCreateTool(): ToolDefinition {
|
|||||||
execute: async (args: Record<string, unknown>): Promise<string> => {
|
execute: async (args: Record<string, unknown>): Promise<string> => {
|
||||||
try {
|
try {
|
||||||
const input = TeamTaskCreateInputSchema.parse(args)
|
const input = TeamTaskCreateInputSchema.parse(args)
|
||||||
|
const teamError = validateTeamName(input.team_name)
|
||||||
|
if (teamError) {
|
||||||
|
return JSON.stringify({ error: teamError })
|
||||||
|
}
|
||||||
readTeamConfigOrThrow(input.team_name)
|
readTeamConfigOrThrow(input.team_name)
|
||||||
|
|
||||||
const task = createTeamTask(
|
const task = createTeamTask(
|
||||||
@ -59,6 +64,10 @@ export function createTeamTaskListTool(): ToolDefinition {
|
|||||||
execute: async (args: Record<string, unknown>): Promise<string> => {
|
execute: async (args: Record<string, unknown>): Promise<string> => {
|
||||||
try {
|
try {
|
||||||
const input = TeamTaskListInputSchema.parse(args)
|
const input = TeamTaskListInputSchema.parse(args)
|
||||||
|
const teamError = validateTeamName(input.team_name)
|
||||||
|
if (teamError) {
|
||||||
|
return JSON.stringify({ error: teamError })
|
||||||
|
}
|
||||||
readTeamConfigOrThrow(input.team_name)
|
readTeamConfigOrThrow(input.team_name)
|
||||||
return JSON.stringify(listTeamTasks(input.team_name))
|
return JSON.stringify(listTeamTasks(input.team_name))
|
||||||
} catch (error) {
|
} catch (error) {
|
||||||
@ -78,6 +87,14 @@ export function createTeamTaskGetTool(): ToolDefinition {
|
|||||||
execute: async (args: Record<string, unknown>): Promise<string> => {
|
execute: async (args: Record<string, unknown>): Promise<string> => {
|
||||||
try {
|
try {
|
||||||
const input = TeamTaskGetInputSchema.parse(args)
|
const input = TeamTaskGetInputSchema.parse(args)
|
||||||
|
const teamError = validateTeamName(input.team_name)
|
||||||
|
if (teamError) {
|
||||||
|
return JSON.stringify({ error: teamError })
|
||||||
|
}
|
||||||
|
const taskIdError = validateTaskId(input.task_id)
|
||||||
|
if (taskIdError) {
|
||||||
|
return JSON.stringify({ error: taskIdError })
|
||||||
|
}
|
||||||
readTeamConfigOrThrow(input.team_name)
|
readTeamConfigOrThrow(input.team_name)
|
||||||
const task = readTeamTask(input.team_name, input.task_id)
|
const task = readTeamTask(input.team_name, input.task_id)
|
||||||
if (!task) {
|
if (!task) {
|
||||||
@ -96,6 +113,14 @@ export function notifyOwnerAssignment(teamName: string, task: TeamTask): void {
|
|||||||
return
|
return
|
||||||
}
|
}
|
||||||
|
|
||||||
|
if (validateTeamName(teamName)) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
|
if (validateAgentName(task.owner)) {
|
||||||
|
return
|
||||||
|
}
|
||||||
|
|
||||||
sendStructuredInboxMessage(
|
sendStructuredInboxMessage(
|
||||||
teamName,
|
teamName,
|
||||||
"team-lead",
|
"team-lead",
|
||||||
|
|||||||
@ -1,5 +1,6 @@
|
|||||||
import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
|
import { tool, type ToolDefinition } from "@opencode-ai/plugin/tool"
|
||||||
import { readTeamConfigOrThrow } from "./team-config-store"
|
import { readTeamConfigOrThrow } from "./team-config-store"
|
||||||
|
import { validateAgentName, validateTaskId, validateTeamName } from "./name-validation"
|
||||||
import { TeamTaskUpdateInputSchema } from "./types"
|
import { TeamTaskUpdateInputSchema } from "./types"
|
||||||
import { updateTeamTask } from "./team-task-update"
|
import { updateTeamTask } from "./team-task-update"
|
||||||
import { notifyOwnerAssignment } from "./team-task-tools"
|
import { notifyOwnerAssignment } from "./team-task-tools"
|
||||||
@ -22,6 +23,36 @@ export function createTeamTaskUpdateTool(): ToolDefinition {
|
|||||||
execute: async (args: Record<string, unknown>): Promise<string> => {
|
execute: async (args: Record<string, unknown>): Promise<string> => {
|
||||||
try {
|
try {
|
||||||
const input = TeamTaskUpdateInputSchema.parse(args)
|
const input = TeamTaskUpdateInputSchema.parse(args)
|
||||||
|
const teamError = validateTeamName(input.team_name)
|
||||||
|
if (teamError) {
|
||||||
|
return JSON.stringify({ error: teamError })
|
||||||
|
}
|
||||||
|
const taskIdError = validateTaskId(input.task_id)
|
||||||
|
if (taskIdError) {
|
||||||
|
return JSON.stringify({ error: taskIdError })
|
||||||
|
}
|
||||||
|
if (input.owner !== undefined) {
|
||||||
|
const ownerError = validateAgentName(input.owner)
|
||||||
|
if (ownerError) {
|
||||||
|
return JSON.stringify({ error: ownerError })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (input.add_blocks) {
|
||||||
|
for (const blockerId of input.add_blocks) {
|
||||||
|
const blockerError = validateTaskId(blockerId)
|
||||||
|
if (blockerError) {
|
||||||
|
return JSON.stringify({ error: blockerError })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if (input.add_blocked_by) {
|
||||||
|
for (const dependencyId of input.add_blocked_by) {
|
||||||
|
const dependencyError = validateTaskId(dependencyId)
|
||||||
|
if (dependencyError) {
|
||||||
|
return JSON.stringify({ error: dependencyError })
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
readTeamConfigOrThrow(input.team_name)
|
readTeamConfigOrThrow(input.team_name)
|
||||||
|
|
||||||
const task = updateTeamTask(input.team_name, input.task_id, {
|
const task = updateTeamTask(input.team_name, input.task_id, {
|
||||||
|
|||||||
@ -1,6 +1,7 @@
|
|||||||
import { existsSync, readdirSync, unlinkSync } from "node:fs"
|
import { existsSync, readdirSync, unlinkSync } from "node:fs"
|
||||||
import { join } from "node:path"
|
import { join } from "node:path"
|
||||||
import { readJsonSafe, writeJsonAtomic } from "../../features/claude-tasks/storage"
|
import { readJsonSafe, writeJsonAtomic } from "../../features/claude-tasks/storage"
|
||||||
|
import { validateTaskId, validateTeamName } from "./name-validation"
|
||||||
import { getTeamTaskDir, getTeamTaskPath } from "./paths"
|
import { getTeamTaskDir, getTeamTaskPath } from "./paths"
|
||||||
import {
|
import {
|
||||||
addPendingEdge,
|
addPendingEdge,
|
||||||
@ -23,11 +24,40 @@ export interface TeamTaskUpdatePatch {
|
|||||||
metadata?: Record<string, unknown>
|
metadata?: Record<string, unknown>
|
||||||
}
|
}
|
||||||
|
|
||||||
|
function assertValidTeamName(teamName: string): void {
|
||||||
|
const validationError = validateTeamName(teamName)
|
||||||
|
if (validationError) {
|
||||||
|
throw new Error(validationError)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function assertValidTaskId(taskId: string): void {
|
||||||
|
const validationError = validateTaskId(taskId)
|
||||||
|
if (validationError) {
|
||||||
|
throw new Error(validationError)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
function writeTaskToPath(path: string, task: TeamTask): void {
|
function writeTaskToPath(path: string, task: TeamTask): void {
|
||||||
writeJsonAtomic(path, TeamTaskSchema.parse(task))
|
writeJsonAtomic(path, TeamTaskSchema.parse(task))
|
||||||
}
|
}
|
||||||
|
|
||||||
export function updateTeamTask(teamName: string, taskId: string, patch: TeamTaskUpdatePatch): TeamTask {
|
export function updateTeamTask(teamName: string, taskId: string, patch: TeamTaskUpdatePatch): TeamTask {
|
||||||
|
assertValidTeamName(teamName)
|
||||||
|
assertValidTaskId(taskId)
|
||||||
|
|
||||||
|
if (patch.addBlocks) {
|
||||||
|
for (const blockedTaskId of patch.addBlocks) {
|
||||||
|
assertValidTaskId(blockedTaskId)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (patch.addBlockedBy) {
|
||||||
|
for (const blockerId of patch.addBlockedBy) {
|
||||||
|
assertValidTaskId(blockerId)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
return withTeamTaskLock(teamName, () => {
|
return withTeamTaskLock(teamName, () => {
|
||||||
const taskDir = getTeamTaskDir(teamName)
|
const taskDir = getTeamTaskDir(teamName)
|
||||||
const taskPath = getTeamTaskPath(teamName, taskId)
|
const taskPath = getTeamTaskPath(teamName, taskId)
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user