From 498afcaa624b46e114c712d43a8d726841fcef55 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 4 Jul 2026 10:21:40 -0400 Subject: [PATCH 1/3] fix(specialist): cap max specialist nesting depth A Task specialist can recursively spawn further specialists via --depth. Without a ceiling, a runaway or malicious chain of Task calls can recurse indefinitely, exhausting process and memory resources. Reject any run whose depth exceeds a hard cap of 8. Related to #470, not closing it: today's normal specialist/swarm path cannot reach this recursively, since child sessions are launched with --tag specialist and shouldRegisterExecSpecialistTools suppresses the Task tool for that tag (see PR description for details). This is a defense-in-depth guard, not a fix for an active exploit. Split out of #468 at reviewer request. Co-Authored-By: Claude Sonnet 5 --- internal/specialist/exec.go | 4 ++++ internal/specialist/exec_test.go | 10 ++++++++++ 2 files changed, 14 insertions(+) diff --git a/internal/specialist/exec.go b/internal/specialist/exec.go index 5a6f183e..66a65717 100644 --- a/internal/specialist/exec.go +++ b/internal/specialist/exec.go @@ -25,6 +25,7 @@ import ( const ( sessionTagSpecialist = "specialist" promptFileThresholdBytes = 4 * 1024 + maxSpecialistDepth = 8 // hard cap to prevent infinite recursion/resource exhaustion via Task ) const SessionTagSpecialist = sessionTagSpecialist @@ -218,6 +219,9 @@ func (executor Executor) Run(ctx context.Context, params TaskParameters, options if options.CurrentDepth < 0 { return ExecResult{}, fmt.Errorf("current depth cannot be negative") } + if options.CurrentDepth > maxSpecialistDepth { + return ExecResult{}, fmt.Errorf("specialist nesting depth %d exceeds maximum %d", options.CurrentDepth, maxSpecialistDepth) + } if strings.TrimSpace(params.Prompt) == "" { return ExecResult{}, fmt.Errorf("specialist prompt is required") } diff --git a/internal/specialist/exec_test.go b/internal/specialist/exec_test.go index e25ed7c4..8aa2f2a5 100644 --- a/internal/specialist/exec_test.go +++ b/internal/specialist/exec_test.go @@ -1,6 +1,7 @@ package specialist import ( + "context" "os" "path/filepath" "reflect" @@ -336,6 +337,15 @@ func TestBuildArgsRejectsInvalidInputs(t *testing.T) { } } +func TestRunRejectsDepthExceedingMax(t *testing.T) { + _, err := (Executor{}).Run(context.Background(), TaskParameters{ + Prompt: "hi", + }, TaskRunOptions{CurrentDepth: maxSpecialistDepth + 1}) + if err == nil || !strings.Contains(err.Error(), "depth") { + t.Fatalf("Run error = %v, want depth error", err) + } +} + func TestBuildArgsRejectsInvalidSessionIDs(t *testing.T) { _, err := (Executor{NewSessionID: func() (string, error) { return "../escape", nil }}).BuildArgs(BuildArgsInput{ Prompt: "hi", From 004406c343e5e1fdfc2779dbbf7062bef35375fe Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 4 Jul 2026 17:32:51 -0400 Subject: [PATCH 2/3] fix(specialist): reject depth cap at the boundary, not past it Addresses jatmn's review on PR #491: - Executor.Run checked `CurrentDepth > maxSpecialistDepth`, but this call is what launches the child at CurrentDepth+1. A parent already AT the cap passed the check and started a child one level past it, only getting rejected on that child's own next call. Change to `>=` so the guard fires before the over-cap child is launched. - Add TestRunRejectsDepthAtMax to cover the boundary explicitly. --- internal/specialist/exec.go | 8 ++++++-- internal/specialist/exec_test.go | 13 +++++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/internal/specialist/exec.go b/internal/specialist/exec.go index 66a65717..6dac1d25 100644 --- a/internal/specialist/exec.go +++ b/internal/specialist/exec.go @@ -219,8 +219,12 @@ func (executor Executor) Run(ctx context.Context, params TaskParameters, options if options.CurrentDepth < 0 { return ExecResult{}, fmt.Errorf("current depth cannot be negative") } - if options.CurrentDepth > maxSpecialistDepth { - return ExecResult{}, fmt.Errorf("specialist nesting depth %d exceeds maximum %d", options.CurrentDepth, maxSpecialistDepth) + // >= (not >): this Run call spawns a CHILD at options.CurrentDepth+1, so a + // parent already AT the cap must still be rejected here rather than being + // allowed to launch one more level before the child's own next call trips + // the guard. + if options.CurrentDepth >= maxSpecialistDepth { + return ExecResult{}, fmt.Errorf("spawning a specialist at depth %d would exceed maximum nesting depth %d", options.CurrentDepth+1, maxSpecialistDepth) } if strings.TrimSpace(params.Prompt) == "" { return ExecResult{}, fmt.Errorf("specialist prompt is required") diff --git a/internal/specialist/exec_test.go b/internal/specialist/exec_test.go index 8aa2f2a5..626627cb 100644 --- a/internal/specialist/exec_test.go +++ b/internal/specialist/exec_test.go @@ -346,6 +346,19 @@ func TestRunRejectsDepthExceedingMax(t *testing.T) { } } +// TestRunRejectsDepthAtMax covers the boundary: a parent already AT the cap +// must be rejected too, since this Run call would launch a child one level +// past it (--depth CurrentDepth+1). Only checking ">" here would let that +// child start before the guard ever caught it. +func TestRunRejectsDepthAtMax(t *testing.T) { + _, err := (Executor{}).Run(context.Background(), TaskParameters{ + Prompt: "hi", + }, TaskRunOptions{CurrentDepth: maxSpecialistDepth}) + if err == nil || !strings.Contains(err.Error(), "depth") { + t.Fatalf("Run error = %v, want depth error", err) + } +} + func TestBuildArgsRejectsInvalidSessionIDs(t *testing.T) { _, err := (Executor{NewSessionID: func() (string, error) { return "../escape", nil }}).BuildArgs(BuildArgsInput{ Prompt: "hi", From c89032ff555af489bb1443077610dcb8b04c1bf5 Mon Sep 17 00:00:00 2001 From: euxaristia <25621994+euxaristia@users.noreply.github.com> Date: Sat, 4 Jul 2026 20:21:53 -0400 Subject: [PATCH 3/3] test(specialist): cover resume boundary in depth-cap guard The pre-branch depth guard in Run sits before the fresh/resume split, but the boundary test only exercised the fresh path, leaving a future refactor free to move the guard without any test catching a resume regression. --- internal/specialist/exec_test.go | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/internal/specialist/exec_test.go b/internal/specialist/exec_test.go index 626627cb..b0de1b6a 100644 --- a/internal/specialist/exec_test.go +++ b/internal/specialist/exec_test.go @@ -349,13 +349,24 @@ func TestRunRejectsDepthExceedingMax(t *testing.T) { // TestRunRejectsDepthAtMax covers the boundary: a parent already AT the cap // must be rejected too, since this Run call would launch a child one level // past it (--depth CurrentDepth+1). Only checking ">" here would let that -// child start before the guard ever caught it. +// child start before the guard ever caught it. The guard sits before the +// fresh/resume branch in Run, so both call shapes must be proven to reject +// here rather than falling through to runFresh/runResume. func TestRunRejectsDepthAtMax(t *testing.T) { - _, err := (Executor{}).Run(context.Background(), TaskParameters{ - Prompt: "hi", - }, TaskRunOptions{CurrentDepth: maxSpecialistDepth}) - if err == nil || !strings.Contains(err.Error(), "depth") { - t.Fatalf("Run error = %v, want depth error", err) + tests := []struct { + name string + params TaskParameters + }{ + {name: "fresh", params: TaskParameters{Prompt: "hi"}}, + {name: "resume", params: TaskParameters{Prompt: "hi", Resume: "child_session"}}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + _, err := (Executor{}).Run(context.Background(), tc.params, TaskRunOptions{CurrentDepth: maxSpecialistDepth}) + if err == nil || !strings.Contains(err.Error(), "depth") { + t.Fatalf("Run error = %v, want depth error", err) + } + }) } }