From a7c749b253faf27e65b46340961a1bccbe4f3ed5 Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Wed, 8 Jul 2026 22:53:43 +0530 Subject: [PATCH 1/8] fix: propagate session permissions correctly Sub-agents now inherit the exact Allow, Ask, and Deny rules from the parent session instead of bypassing them. This resolves a known prompt-injection loophole for background agents. Signed-off-by: piyush0049 --- pkg/runtime/agent_delegation.go | 7 +++---- pkg/runtime/agent_delegation_test.go | 25 +++++++++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index bcb1cab425..1c87d2af0f 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -184,6 +184,9 @@ func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *ag if cfg.PinAgent { opts = append(opts, session.WithAgentName(cfg.AgentName)) } + if parent.Permissions != nil { + opts = append(opts, session.WithPermissions(parent.Permissions)) + } // Merge parent's excluded tools with config's excluded tools so that // nested sub-sessions (e.g. skill → transfer_task → child) inherit // exclusions from all ancestors and don't re-introduce filtered tools. @@ -482,10 +485,6 @@ func (r *LocalRuntime) CurrentAgentSubAgentNames() []string { // authorises all tool calls made by the sub-agent when they approve // run_background_agent. Callers should be aware that prompt injection in // the sub-agent's context could exploit this gate-bypass. -// -// TODO: propagate the parent session's per-tool permission rules once the -// runtime supports per-session permission scoping rather than a single -// shared ToolsApproved flag. func (r *LocalRuntime) RunAgent(ctx context.Context, params agenttool.RunParams) *agenttool.RunResult { return r.runCollecting(ctx, params.ParentSession, SubSessionConfig{ Task: params.Task, diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go index 6c1df4affe..15e38b5be3 100644 --- a/pkg/runtime/agent_delegation_test.go +++ b/pkg/runtime/agent_delegation_test.go @@ -266,3 +266,28 @@ func TestSubSessionWithoutAttachedFilesOmitsBlock(t *testing.T) { assert.NotContains(t, m.Content, "") } } + +func TestSubSessionInheritsPermissions(t *testing.T) { + t.Parallel() + + perms := &session.PermissionsConfig{ + Allow: []string{"read_*"}, + Deny: []string{"write_*"}, + Ask: []string{"edit_*"}, + } + parent := session.New(session.WithPermissions(perms)) + + childAgent := agent.New("worker", "") + cfg := SubSessionConfig{ + Task: "refactor", + AgentName: "worker", + Title: "Refactor", + } + + s := newSubSession(parent, cfg, childAgent) + + require.NotNil(t, s.Permissions) + assert.Equal(t, perms.Allow, s.Permissions.Allow) + assert.Equal(t, perms.Deny, s.Permissions.Deny) + assert.Equal(t, perms.Ask, s.Permissions.Ask) +} From f90d2283ff042ad3cd7c5e4dc0149171de23cb7e Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Thu, 9 Jul 2026 15:32:15 +0530 Subject: [PATCH 2/8] fix: address review feedback on permission isolation and dispatch - Deep clone parent permissions into the child session to prevent aliasing - Re-order yolo check in Decide to ensure inherited Deny/ForceAsk overrides ToolsApproved: true - Enhance TestSubSessionInheritsPermissions to assert actual dispatch behavior Signed-off-by: piyush0049 --- pkg/runtime/agent_delegation.go | 2 +- pkg/runtime/agent_delegation_test.go | 19 +++++++++++++++++++ pkg/runtime/toolexec/permissions.go | 8 ++++---- pkg/runtime/toolexec/permissions_test.go | 4 ++-- pkg/session/branch.go | 16 ++-------------- pkg/session/session.go | 12 ++++++++++++ pkg/session/store.go | 4 ++-- 7 files changed, 42 insertions(+), 23 deletions(-) diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index 1c87d2af0f..185d4276f8 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -185,7 +185,7 @@ func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *ag opts = append(opts, session.WithAgentName(cfg.AgentName)) } if parent.Permissions != nil { - opts = append(opts, session.WithPermissions(parent.Permissions)) + opts = append(opts, session.WithPermissions(parent.Permissions.Clone())) } // Merge parent's excluded tools with config's excluded tools so that // nested sub-sessions (e.g. skill → transfer_task → child) inherit diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go index 15e38b5be3..d5a5e45a40 100644 --- a/pkg/runtime/agent_delegation_test.go +++ b/pkg/runtime/agent_delegation_test.go @@ -8,6 +8,9 @@ import ( "github.com/stretchr/testify/require" "github.com/docker/docker-agent/pkg/agent" + "github.com/docker/docker-agent/pkg/config/latest" + "github.com/docker/docker-agent/pkg/permissions" + "github.com/docker/docker-agent/pkg/runtime/toolexec" "github.com/docker/docker-agent/pkg/session" ) @@ -290,4 +293,20 @@ func TestSubSessionInheritsPermissions(t *testing.T) { assert.Equal(t, perms.Allow, s.Permissions.Allow) assert.Equal(t, perms.Deny, s.Permissions.Deny) assert.Equal(t, perms.Ask, s.Permissions.Ask) + + // Verify the gap flagged in PR review: even if ToolsApproved is true (yolo flag), + // the inherited Deny should correctly override the yolo flag during dispatch. + s.ToolsApproved = true + + checker := permissions.NewChecker(&latest.PermissionsConfig{ + Allow: s.Permissions.Allow, + Ask: s.Permissions.Ask, + Deny: s.Permissions.Deny, + }) + namedCheckers := []toolexec.NamedChecker{ + {Checker: checker, Source: "session permissions"}, + } + + decision := toolexec.Decide(s.ToolsApproved, namedCheckers, "write_file", map[string]any{"path": "foo"}, false) + assert.Equal(t, toolexec.OutcomeDeny, decision.Outcome, "Inherited Deny should override ToolsApproved: true (yolo)") } diff --git a/pkg/runtime/toolexec/permissions.go b/pkg/runtime/toolexec/permissions.go index e872bfd41a..cd2c0020c9 100644 --- a/pkg/runtime/toolexec/permissions.go +++ b/pkg/runtime/toolexec/permissions.go @@ -75,10 +75,6 @@ func Decide( toolArgs map[string]any, readOnlyHint bool, ) PermissionDecision { - if yoloApproved { - return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo} - } - for _, pc := range checkers { switch pc.Checker.CheckWithArgs(toolName, toolArgs) { case permissions.Deny: @@ -92,6 +88,10 @@ func Decide( } } + if yoloApproved { + return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo} + } + if readOnlyHint { return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonReadOnlyHint} } diff --git a/pkg/runtime/toolexec/permissions_test.go b/pkg/runtime/toolexec/permissions_test.go index 16bf6c7690..4ad1c521e6 100644 --- a/pkg/runtime/toolexec/permissions_test.go +++ b/pkg/runtime/toolexec/permissions_test.go @@ -18,13 +18,13 @@ func newChecker(t *testing.T, allow, ask, deny []string) *permissions.Checker { }) } -func TestDecide_YoloShortCircuits(t *testing.T) { +func TestDecide_DenyOverridesYolo(t *testing.T) { t.Parallel() d := Decide(true, []NamedChecker{ {Checker: newChecker(t, nil, nil, []string{"shell"}), Source: "team"}, }, "shell", nil, false) - assert.Equal(t, PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo}, d) + assert.Equal(t, PermissionDecision{Outcome: OutcomeDeny, Reason: ReasonChecker, Source: "team"}, d) } func TestDecide_DenyFromCheckerWins(t *testing.T) { diff --git a/pkg/session/branch.go b/pkg/session/branch.go index 0113d9e2cb..775ea8090c 100644 --- a/pkg/session/branch.go +++ b/pkg/session/branch.go @@ -91,7 +91,7 @@ func (s *Session) Clone() *Session { InputTokens: s.InputTokens, OutputTokens: s.OutputTokens, Cost: s.Cost, - Permissions: clonePermissionsConfig(s.Permissions), + Permissions: s.Permissions.Clone(), AgentModelOverrides: cloneStringMap(s.AgentModelOverrides), CustomModelsUsed: cloneStringSlice(s.CustomModelsUsed), AttachedFiles: cloneStringSlice(s.AttachedFiles), @@ -190,7 +190,7 @@ func copySessionMetadata(dst, src *Session, title string) { dst.MaxConsecutiveToolCalls = src.MaxConsecutiveToolCalls dst.MaxOldToolCallTokens = src.MaxOldToolCallTokens dst.Starred = src.Starred - dst.Permissions = clonePermissionsConfig(src.Permissions) + dst.Permissions = src.Permissions.Clone() dst.AgentModelOverrides = cloneStringMap(src.AgentModelOverrides) dst.CustomModelsUsed = cloneStringSlice(src.CustomModelsUsed) dst.AttachedFiles = src.AttachedFilesSnapshot() @@ -313,23 +313,11 @@ func cloneEvalResultChecks(src EvalResultChecks) EvalResultChecks { } if src.Relevance != nil { relevance := *src.Relevance - relevance.Results = slices.Clone(src.Relevance.Results) cp.Relevance = &relevance } return cp } -func clonePermissionsConfig(src *PermissionsConfig) *PermissionsConfig { - if src == nil { - return nil - } - return &PermissionsConfig{ - Allow: cloneStringSlice(src.Allow), - Ask: cloneStringSlice(src.Ask), - Deny: cloneStringSlice(src.Deny), - } -} - func cloneStringMap(src map[string]string) map[string]string { if len(src) == 0 { return nil diff --git a/pkg/session/session.go b/pkg/session/session.go index 150fab456d..a52a322680 100644 --- a/pkg/session/session.go +++ b/pkg/session/session.go @@ -270,6 +270,18 @@ type PermissionsConfig struct { Deny []string `json:"deny,omitempty"` } +// Clone returns a deep copy of the permissions configuration. +func (c *PermissionsConfig) Clone() *PermissionsConfig { + if c == nil { + return nil + } + return &PermissionsConfig{ + Allow: slices.Clone(c.Allow), + Ask: slices.Clone(c.Ask), + Deny: slices.Clone(c.Deny), + } +} + // Message is a message from an agent type Message struct { // ID is the database ID of the message (used for persistence tracking) diff --git a/pkg/session/store.go b/pkg/session/store.go index 8b9d70f0e0..8cb7a6963d 100644 --- a/pkg/session/store.go +++ b/pkg/session/store.go @@ -232,7 +232,7 @@ func (s *InMemorySessionStore) UpdateSession(_ context.Context, session *Session InputTokens: session.InputTokens, OutputTokens: session.OutputTokens, Cost: session.Cost, - Permissions: clonePermissionsConfig(session.Permissions), + Permissions: session.Permissions.Clone(), AgentModelOverrides: cloneStringMap(session.AgentModelOverrides), CustomModelsUsed: cloneStringSlice(session.CustomModelsUsed), AttachedFiles: slices.Clone(session.AttachedFiles), @@ -954,7 +954,7 @@ func (s *SQLiteSessionStore) UpdateSession(ctx context.Context, session *Session InputTokens: session.InputTokens, OutputTokens: session.OutputTokens, Cost: session.Cost, - Permissions: clonePermissionsConfig(session.Permissions), + Permissions: session.Permissions.Clone(), AgentModelOverrides: cloneStringMap(session.AgentModelOverrides), CustomModelsUsed: cloneStringSlice(session.CustomModelsUsed), ParentID: session.ParentID, From 425b99a86adde7e82a58fd486fd3aff5b2cba10f Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Thu, 9 Jul 2026 22:07:26 +0530 Subject: [PATCH 3/8] test: update stale yolo tests and restore branch cloning Signed-off-by: piyush0049 --- pkg/runtime/runtime_test.go | 21 ++++++++++++--------- pkg/session/branch.go | 1 + 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 858b556b33..07c565d457 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -2517,7 +2517,8 @@ func TestTransferTaskPersistsSubSessionOnError(t *testing.T) { "SubSessionCompletedEvent must fire on the error path so observers persist the sub-session") } -func TestYoloMode_OverridesPermissionsDeny(t *testing.T) { +// TestDenyOverridesYoloMode verifies that Deny permissions take precedence over the yolo flag. +func TestDenyOverridesYoloMode(t *testing.T) { t.Parallel() // Test that --yolo flag takes precedence over deny permissions @@ -2561,11 +2562,12 @@ func TestYoloMode_OverridesPermissionsDeny(t *testing.T) { rt.processToolCalls(t.Context(), sess, calls, agentTools, NewChannelSink(events)) close(events) - // With --yolo, the tool should execute despite deny permission - require.True(t, executed, "expected tool to be executed in --yolo mode despite deny permission") + // With --yolo and Deny/ForceAsk precedence, the tool should NOT execute. + require.False(t, executed, "expected tool to NOT be executed in --yolo mode because Deny wins") } -func TestYoloMode_OverridesForceAsk(t *testing.T) { +// TestForceAskOverridesYoloMode verifies that ForceAsk permissions take precedence over the yolo flag. +func TestForceAskOverridesYoloMode(t *testing.T) { t.Parallel() // Test that --yolo flag takes precedence over ForceAsk permissions @@ -2609,11 +2611,12 @@ func TestYoloMode_OverridesForceAsk(t *testing.T) { rt.processToolCalls(t.Context(), sess, calls, agentTools, NewChannelSink(events)) close(events) - // With --yolo, the tool should execute without asking - require.True(t, executed, "expected tool to be executed in --yolo mode despite ForceAsk permission") + // With --yolo and Deny/ForceAsk precedence, the tool should NOT execute. + require.False(t, executed, "expected tool to NOT be executed in --yolo mode because ForceAsk wins") } -func TestYoloMode_OverridesSessionDeny(t *testing.T) { +// TestSessionDenyOverridesYoloMode verifies that session-level Deny permissions take precedence over the yolo flag. +func TestSessionDenyOverridesYoloMode(t *testing.T) { t.Parallel() // Test that --yolo flag takes precedence over session-level deny @@ -2656,8 +2659,8 @@ func TestYoloMode_OverridesSessionDeny(t *testing.T) { rt.processToolCalls(t.Context(), sess, calls, agentTools, NewChannelSink(events)) close(events) - // With --yolo, the tool should execute despite session deny - require.True(t, executed, "expected tool to be executed in --yolo mode despite session deny permission") + // With --yolo and Deny/ForceAsk precedence, the tool should NOT execute. + require.False(t, executed, "expected tool to NOT be executed in --yolo mode because session Deny wins") } func TestStripImageContent(t *testing.T) { diff --git a/pkg/session/branch.go b/pkg/session/branch.go index 775ea8090c..bd1d13a234 100644 --- a/pkg/session/branch.go +++ b/pkg/session/branch.go @@ -313,6 +313,7 @@ func cloneEvalResultChecks(src EvalResultChecks) EvalResultChecks { } if src.Relevance != nil { relevance := *src.Relevance + relevance.Results = slices.Clone(src.Relevance.Results) cp.Relevance = &relevance } return cp From 96d089ebb9a3210dfcdf135662f5e3b6fec7af01 Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Fri, 10 Jul 2026 17:36:19 +0530 Subject: [PATCH 4/8] chore: fix formatting for gofumpt and gci Signed-off-by: piyush0049 --- pkg/runtime/agent_delegation.go | 6 ++++-- pkg/runtime/agent_delegation_test.go | 14 ++++++++------ 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index 4de78d9e06..8763906a30 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -535,7 +535,8 @@ func (r *LocalRuntime) handleTaskTransfer(ctx context.Context, sess *session.Ses delegationAttrs = append(delegationAttrs, attribute.Int("cagent.delegation.task_length", len(params.Task))) } if genai.EmitLegacyAttributes() { - delegationAttrs = append(delegationAttrs, + delegationAttrs = append( + delegationAttrs, attribute.String("from.agent", a.Name()), attribute.String("to.agent", params.Agent), attribute.String("session.id", sess.ID), @@ -633,5 +634,6 @@ func (r *LocalRuntime) applyForceHandoff(ctx context.Context, sess *session.Sess "off to agents that you see in the conversation history from previous agents, as those were " + "available to different agents with different capabilities. Look at the conversation history " + "for context, continue the work from where the previous agent stopped, and complete your " + - "part of the task.")) + "part of the task.", + )) } diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go index 954a33f093..a22960378e 100644 --- a/pkg/runtime/agent_delegation_test.go +++ b/pkg/runtime/agent_delegation_test.go @@ -85,7 +85,8 @@ func TestNewSubSession(t *testing.T) { t.Parallel() parent := session.New(session.WithUserMessage("hello")) - childAgent := agent.New("worker", "a worker agent", + childAgent := agent.New( + "worker", "a worker agent", agent.WithMaxIterations(10), ) @@ -190,7 +191,8 @@ func TestSubSessionConfig_InheritsAgentLimits(t *testing.T) { parent := session.New(session.WithUserMessage("hello")) t.Run("with custom limits", func(t *testing.T) { - childAgent := agent.New("worker", "", + childAgent := agent.New( + "worker", "", agent.WithMaxIterations(42), agent.WithMaxConsecutiveToolCalls(7), ) @@ -273,7 +275,6 @@ func TestSubSessionWithoutAttachedFilesOmitsBlock(t *testing.T) { } } - func TestSubSessionInheritsPermissions(t *testing.T) { t.Parallel() @@ -410,7 +411,8 @@ func TestRunAgent_InheritsParentPermissions(t *testing.T) { agent.WithSubAgents(worker)(root) tm := team.New(team.WithAgents(root, worker)) - rt, err := NewLocalRuntime(t.Context(), tm, + rt, err := NewLocalRuntime( + t.Context(), tm, WithSessionCompaction(false), WithModelStore(mockModelStore{}), ) @@ -466,7 +468,8 @@ func TestTransferTask_PropagatesPermissions(t *testing.T) { agent.WithSubAgents(librarian)(root) tm := team.New(team.WithAgents(root, librarian)) - rt, err := NewLocalRuntime(t.Context(), tm, + rt, err := NewLocalRuntime( + t.Context(), tm, WithSessionCompaction(false), WithModelStore(mockModelStore{}), ) @@ -517,5 +520,4 @@ func TestTransferTask_PropagatesPermissions(t *testing.T) { parentClone := sess.ClonePermissions() assert.Equal(t, []string{"safe_tool"}, parentClone.Allow, "parent permissions must remain isolated from child mutations after transfer_task") - } From 1be0cd37812581364bc8dd4228c821da8bca365e Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Fri, 10 Jul 2026 19:22:45 +0530 Subject: [PATCH 5/8] test(runtime): fix TestForceAskOverridesYoloMode hanging on CI Signed-off-by: piyush0049 --- pkg/runtime/runtime_test.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 07c565d457..f7f1e0e91b 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -2599,6 +2599,7 @@ func TestForceAskOverridesYoloMode(t *testing.T) { require.NoError(t, err) sess := session.New(session.WithUserMessage("Test"), session.WithToolsApproved(true)) + sess.NonInteractive = true require.True(t, sess.ToolsApproved) calls := []tools.ToolCall{{ @@ -2611,7 +2612,8 @@ func TestForceAskOverridesYoloMode(t *testing.T) { rt.processToolCalls(t.Context(), sess, calls, agentTools, NewChannelSink(events)) close(events) - // With --yolo and Deny/ForceAsk precedence, the tool should NOT execute. + // ForceAsk overrides --yolo: the checker's ForceAsk verdict routes to + // askUser, which denies in non-interactive mode instead of blocking. require.False(t, executed, "expected tool to NOT be executed in --yolo mode because ForceAsk wins") } From 16665863dcbcb3ecdf411ebdbf5a9e87c30cbb24 Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Fri, 10 Jul 2026 22:29:01 +0530 Subject: [PATCH 6/8] fix: address maintainer feedback on permission overrides Restores YOLO overriding ForceAsk while keeping Deny overriding YOLO. Reverts unrelated formatting noise in agent_delegation.go and tests. Adds end-to-end TestRunAgent_EndToEndPermissions to verify dispatch path inheritance. Signed-off-by: piyush0049 --- pkg/runtime/agent_delegation.go | 6 +-- pkg/runtime/agent_delegation_test.go | 61 ++++++++++++++++++++++++++-- pkg/runtime/runtime_test.go | 10 ++--- pkg/runtime/toolexec/permissions.go | 11 +++-- 4 files changed, 71 insertions(+), 17 deletions(-) diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index 8763906a30..4de78d9e06 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -535,8 +535,7 @@ func (r *LocalRuntime) handleTaskTransfer(ctx context.Context, sess *session.Ses delegationAttrs = append(delegationAttrs, attribute.Int("cagent.delegation.task_length", len(params.Task))) } if genai.EmitLegacyAttributes() { - delegationAttrs = append( - delegationAttrs, + delegationAttrs = append(delegationAttrs, attribute.String("from.agent", a.Name()), attribute.String("to.agent", params.Agent), attribute.String("session.id", sess.ID), @@ -634,6 +633,5 @@ func (r *LocalRuntime) applyForceHandoff(ctx context.Context, sess *session.Sess "off to agents that you see in the conversation history from previous agents, as those were " + "available to different agents with different capabilities. Look at the conversation history " + "for context, continue the work from where the previous agent stopped, and complete your " + - "part of the task.", - )) + "part of the task.")) } diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go index a22960378e..25b7d522d2 100644 --- a/pkg/runtime/agent_delegation_test.go +++ b/pkg/runtime/agent_delegation_test.go @@ -1,6 +1,7 @@ package runtime import ( + "context" "strings" "testing" @@ -85,8 +86,7 @@ func TestNewSubSession(t *testing.T) { t.Parallel() parent := session.New(session.WithUserMessage("hello")) - childAgent := agent.New( - "worker", "a worker agent", + childAgent := agent.New("worker", "a worker agent", agent.WithMaxIterations(10), ) @@ -191,8 +191,7 @@ func TestSubSessionConfig_InheritsAgentLimits(t *testing.T) { parent := session.New(session.WithUserMessage("hello")) t.Run("with custom limits", func(t *testing.T) { - childAgent := agent.New( - "worker", "", + childAgent := agent.New("worker", "", agent.WithMaxIterations(42), agent.WithMaxConsecutiveToolCalls(7), ) @@ -457,6 +456,60 @@ func TestRunAgent_InheritsParentPermissions(t *testing.T) { "parent permissions must be isolated from child mutations") } +func TestRunAgent_EndToEndPermissions(t *testing.T) { + t.Parallel() + + var executed bool + agentTools := []tools.Tool{{ + Name: "dangerous_tool", + Parameters: map[string]any{}, + Handler: func(_ context.Context, _ tools.ToolCall, _ tools.Runtime) (*tools.ToolCallResult, error) { + executed = true + return tools.ResultSuccess("executed"), nil + }, + }} + + workerStream := newStreamBuilder(). + AddToolCallName("call_1", "dangerous_tool"). + AddToolCallArguments("call_1", "{}"). + Build() + parentProv := &mockProvider{id: "test/mock-model", stream: &mockStream{}} + workerProv := &mockProvider{id: "test/mock-model", stream: workerStream} + + worker := agent.New("worker", "Worker agent", + agent.WithModel(workerProv), + agent.WithToolSets(newStubToolSet(nil, agentTools, nil)), + ) + root := agent.New("root", "Root agent", agent.WithModel(parentProv)) + agent.WithSubAgents(worker)(root) + + tm := team.New(team.WithAgents(root, worker)) + rt, err := NewLocalRuntime( + t.Context(), tm, + WithSessionCompaction(false), + WithModelStore(mockModelStore{}), + ) + require.NoError(t, err) + + parentPerms := &session.PermissionsConfig{ + Allow: []string{"safe_tool"}, + Deny: []string{"dangerous_tool"}, + } + parentSession := session.New( + session.WithUserMessage("Test"), + session.WithToolsApproved(true), + session.WithPermissions(parentPerms), + ) + + rt.RunAgent(t.Context(), agenttool.RunParams{ + AgentName: "worker", + Task: "do something", + ParentSession: parentSession, + }) + + require.False(t, executed, "expected dangerous_tool to NOT be executed because it is denied by inherited permissions") +} + func TestTransferTask_PropagatesPermissions(t *testing.T) { t.Parallel() diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index f7f1e0e91b..130c3ed416 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -2566,8 +2566,8 @@ func TestDenyOverridesYoloMode(t *testing.T) { require.False(t, executed, "expected tool to NOT be executed in --yolo mode because Deny wins") } -// TestForceAskOverridesYoloMode verifies that ForceAsk permissions take precedence over the yolo flag. -func TestForceAskOverridesYoloMode(t *testing.T) { +// TestYoloMode_OverridesForceAsk verifies that the yolo flag takes precedence over ForceAsk permissions. +func TestYoloMode_OverridesForceAsk(t *testing.T) { t.Parallel() // Test that --yolo flag takes precedence over ForceAsk permissions @@ -2612,9 +2612,9 @@ func TestForceAskOverridesYoloMode(t *testing.T) { rt.processToolCalls(t.Context(), sess, calls, agentTools, NewChannelSink(events)) close(events) - // ForceAsk overrides --yolo: the checker's ForceAsk verdict routes to - // askUser, which denies in non-interactive mode instead of blocking. - require.False(t, executed, "expected tool to NOT be executed in --yolo mode because ForceAsk wins") + // YOLO overrides ForceAsk: the checker's ForceAsk verdict is bypassed + // and the tool executes automatically. + require.True(t, executed, "expected tool to be executed in --yolo mode because YOLO wins over ForceAsk") } // TestSessionDenyOverridesYoloMode verifies that session-level Deny permissions take precedence over the yolo flag. diff --git a/pkg/runtime/toolexec/permissions.go b/pkg/runtime/toolexec/permissions.go index cd2c0020c9..dbf8fdda4d 100644 --- a/pkg/runtime/toolexec/permissions.go +++ b/pkg/runtime/toolexec/permissions.go @@ -58,11 +58,11 @@ type PermissionDecision struct { // Decide resolves the final permission outcome for a tool call by walking // the configured pipeline in priority order: // -// 1. yoloApproved (--yolo) — auto-allow everything. -// 2. checkers (in order; typically session-level first, then team-level) +// 1. checkers (in order; typically session-level first, then team-level) // — the first checker that returns Allow / Deny / ForceAsk wins. -// ForceAsk produces [OutcomeAsk]: an explicit ask pattern always -// overrides the read-only fast path below. +// However, if the outcome is ForceAsk and yoloApproved is true, +// YOLO overrides ForceAsk and auto-allows the call. +// 2. yoloApproved (--yolo) — auto-allow everything else. // 3. readOnlyHint — auto-allow. // 4. default — Ask. // @@ -82,6 +82,9 @@ func Decide( case permissions.Allow: return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonChecker, Source: pc.Source} case permissions.ForceAsk: + if yoloApproved { + return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo} + } return PermissionDecision{Outcome: OutcomeAsk, Reason: ReasonChecker, Source: pc.Source} case permissions.Ask: // No explicit match at this level; fall through to next checker. From f6014829b880aaf3f5089bca4dd1d911007a8ef1 Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Mon, 13 Jul 2026 22:50:57 +0530 Subject: [PATCH 7/8] fix: address PR feedback on permission scoping and docs Signed-off-by: piyush0049 --- docs/configuration/hooks/index.md | 4 ++-- docs/features/cli/index.md | 4 ++-- pkg/runtime/agent_delegation.go | 6 ------ pkg/runtime/agent_delegation_test.go | 21 ++++++++++++++------- pkg/runtime/runtime_test.go | 6 +++--- pkg/runtime/toolexec/permissions_test.go | 14 ++++++++++++++ 6 files changed, 35 insertions(+), 20 deletions(-) diff --git a/docs/configuration/hooks/index.md b/docs/configuration/hooks/index.md index 4ec0c8c6ff..05454ec943 100644 --- a/docs/configuration/hooks/index.md +++ b/docs/configuration/hooks/index.md @@ -57,7 +57,7 @@ docker-agent dispatches the following hook events: | `on_max_iterations` | When the runtime reaches its configured `max_iterations` limit | No | | `on_agent_switch` | When the runtime moves the active agent (transfer_task, handoff, return) | No | | `on_session_resume` | When the user explicitly approves continuation past `max_iterations` | No | -| `on_tool_approval_decision` | After the runtime's approval chain (yolo / permissions / readonly / ask) resolves | No | +| `on_tool_approval_decision` | After the runtime's approval chain (permissions / yolo / readonly / ask) resolves | No | | `worktree_create` | After `docker agent run --worktree` creates a git worktree, before the session | Yes | > [!NOTE] @@ -713,7 +713,7 @@ At every transfer the runtime ships a snapshot of the previous agent's model end ### Tool-Approval-Decision: who-approved-what audit trail -`on_tool_approval_decision` fires after the runtime's tool-approval chain (yolo / permissions / readonly / pre_tool_use hooks / interactive prompt) has resolved a verdict for a tool call. `approval_decision` is `allow`, `deny`, or `canceled`; `approval_source` is a stable classifier of which step produced the verdict. Observational only — it gives audit pipelines a single, structured "who approved what" record without re-implementing the chain. +`on_tool_approval_decision` fires after the runtime's tool-approval chain (permissions / yolo / readonly / pre_tool_use hooks / interactive prompt) has resolved a verdict for a tool call. `approval_decision` is `allow`, `deny`, or `canceled`; `approval_source` is a stable classifier of which step produced the verdict. Observational only — it gives audit pipelines a single, structured "who approved what" record without re-implementing the chain. ### Worktree-Create: prepare an isolated checkout diff --git a/docs/features/cli/index.md b/docs/features/cli/index.md index e3052e0476..307231d84c 100644 --- a/docs/features/cli/index.md +++ b/docs/features/cli/index.md @@ -28,7 +28,7 @@ $ docker agent run [config] [message...] [flags] | Flag | Description | | --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- | | `-a, --agent ` | Run a specific agent from the config | -| `--yolo` | Auto-approve all tool calls | +| `--yolo` | Auto-approve tool calls (unless explicitly denied) | | `--model ` | Override model(s). Use `provider/model` for all agents, or `agent=provider/model` for specific agents. Comma-separate multiple overrides. | | `--session ` | Resume a previous session. Supports relative refs (`-1` = last, `-2` = second to last). An explicit ID that does not exist yet is created with that ID, so a supervisor can own the session ID upfront and reuse it across runs. | | `-s, --session-db ` | Path to the SQLite session database (default: `/session.db`, so `~/.cagent/session.db` unless `--data-dir` is set) | @@ -479,7 +479,7 @@ $ docker agent run yolo-coder **Alias Options:** Aliases can include runtime options that apply automatically when used: -- `--yolo` — Auto-approve all tool calls when running the alias +- `--yolo` — Auto-approve tool calls (unless explicitly denied) when running the alias - `--model ` — Override the model for the alias - `--hide-tool-results` — Hide tool call results in the TUI when running the alias - `--sandbox` — Always run the alias inside a [Docker sandbox](../../configuration/sandbox/index.md) diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index 4de78d9e06..1ce322a94e 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -322,12 +322,6 @@ func (r *LocalRuntime) runForwarding(ctx context.Context, parent *session.Sessio return nil, subSessionErr } - // Only propagate ToolsApproved and Permissions on success. A failed sub-session - // must not silently escalate the parent's tool-approval gate: the user approved - // tools within a sub-session scope that ended in error, and that approval - // should not carry over to the parent's remaining turns. - parent.SetToolsApproved(s.IsToolsApproved()) - parent.SetPermissions(s.ClonePermissions()) span.SetStatus(codes.Ok, "sub-session completed") return tools.ResultSuccess(s.GetLastAssistantMessageContent()), nil } diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go index 25b7d522d2..00c197b3f2 100644 --- a/pkg/runtime/agent_delegation_test.go +++ b/pkg/runtime/agent_delegation_test.go @@ -299,8 +299,7 @@ func TestSubSessionInheritsPermissions(t *testing.T) { assert.Equal(t, perms.Deny, s.Permissions.Deny) assert.Equal(t, perms.Ask, s.Permissions.Ask) - // Verify the gap flagged in PR review: even if ToolsApproved is true (yolo flag), - // the inherited Deny should correctly override the yolo flag during dispatch. + // Even with ToolsApproved set (yolo), an inherited Deny must win during dispatch. s.ToolsApproved = true checker := permissions.NewChecker(&latest.PermissionsConfig{ @@ -410,8 +409,7 @@ func TestRunAgent_InheritsParentPermissions(t *testing.T) { agent.WithSubAgents(worker)(root) tm := team.New(team.WithAgents(root, worker)) - rt, err := NewLocalRuntime( - t.Context(), tm, + rt, err := NewLocalRuntime(t.Context(), tm, WithSessionCompaction(false), WithModelStore(mockModelStore{}), ) @@ -501,11 +499,21 @@ func TestRunAgent_EndToEndPermissions(t *testing.T) { session.WithPermissions(parentPerms), ) - rt.RunAgent(t.Context(), agenttool.RunParams{ + result := rt.RunAgent(t.Context(), agenttool.RunParams{ AgentName: "worker", Task: "do something", ParentSession: parentSession, }) + require.Empty(t, result.ErrMsg, "RunAgent should succeed") + + var childSession *session.Session + for _, item := range parentSession.Messages { + if item.SubSession != nil { + childSession = item.SubSession + break + } + } + require.NotNil(t, childSession, "parent must have a sub-session") require.False(t, executed, "expected dangerous_tool to NOT be executed because it is denied by inherited permissions") } @@ -521,8 +529,7 @@ func TestTransferTask_PropagatesPermissions(t *testing.T) { agent.WithSubAgents(librarian)(root) tm := team.New(team.WithAgents(root, librarian)) - rt, err := NewLocalRuntime( - t.Context(), tm, + rt, err := NewLocalRuntime(t.Context(), tm, WithSessionCompaction(false), WithModelStore(mockModelStore{}), ) diff --git a/pkg/runtime/runtime_test.go b/pkg/runtime/runtime_test.go index 130c3ed416..bd9cfa39b8 100644 --- a/pkg/runtime/runtime_test.go +++ b/pkg/runtime/runtime_test.go @@ -2521,7 +2521,7 @@ func TestTransferTaskPersistsSubSessionOnError(t *testing.T) { func TestDenyOverridesYoloMode(t *testing.T) { t.Parallel() - // Test that --yolo flag takes precedence over deny permissions + // Test that deny permissions take precedence over the --yolo flag permChecker := permissions.NewChecker(&latest.PermissionsConfig{ Deny: []string{"dangerous_tool"}, }) @@ -2599,7 +2599,7 @@ func TestYoloMode_OverridesForceAsk(t *testing.T) { require.NoError(t, err) sess := session.New(session.WithUserMessage("Test"), session.WithToolsApproved(true)) - sess.NonInteractive = true + sess.NonInteractive = true // fail fast instead of hanging on askUser if this regresses require.True(t, sess.ToolsApproved) calls := []tools.ToolCall{{ @@ -2621,7 +2621,7 @@ func TestYoloMode_OverridesForceAsk(t *testing.T) { func TestSessionDenyOverridesYoloMode(t *testing.T) { t.Parallel() - // Test that --yolo flag takes precedence over session-level deny + // Test that session-level deny takes precedence over the --yolo flag var executed bool agentTools := []tools.Tool{{ Name: "blocked_tool", diff --git a/pkg/runtime/toolexec/permissions_test.go b/pkg/runtime/toolexec/permissions_test.go index 4ad1c521e6..1803f95f0c 100644 --- a/pkg/runtime/toolexec/permissions_test.go +++ b/pkg/runtime/toolexec/permissions_test.go @@ -27,6 +27,20 @@ func TestDecide_DenyOverridesYolo(t *testing.T) { assert.Equal(t, PermissionDecision{Outcome: OutcomeDeny, Reason: ReasonChecker, Source: "team"}, d) } +func TestDecide_YoloAllowsWhenNoCheckerMatches(t *testing.T) { + t.Parallel() + d := Decide(true, nil, "shell", nil, false) + assert.Equal(t, PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo}, d) +} + +func TestDecide_YoloOverridesForceAsk(t *testing.T) { + t.Parallel() + d := Decide(true, []NamedChecker{ + {Checker: newChecker(t, nil, []string{"shell"}, nil), Source: "team"}, + }, "shell", nil, false) + assert.Equal(t, PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo}, d) +} + func TestDecide_DenyFromCheckerWins(t *testing.T) { t.Parallel() d := Decide(false, []NamedChecker{ From 5faaafc3e087f5ce609ca6d25eab7ab6d54164ef Mon Sep 17 00:00:00 2001 From: piyush0049 Date: Wed, 15 Jul 2026 15:04:13 +0530 Subject: [PATCH 8/8] fix: resolve runForwarding invariant and test feedback Signed-off-by: piyush0049 --- pkg/runtime/agent_delegation.go | 6 +-- pkg/runtime/agent_delegation_test.go | 57 +++++++++++++++++++++++++++- 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/pkg/runtime/agent_delegation.go b/pkg/runtime/agent_delegation.go index 1ce322a94e..9f8d4fe27d 100644 --- a/pkg/runtime/agent_delegation.go +++ b/pkg/runtime/agent_delegation.go @@ -247,9 +247,9 @@ func (r *LocalRuntime) swapCurrentAgent(ctx context.Context, sessionID string, f } } -// runForwarding runs a child session synchronously, forwarding all of its -// events to evts and propagating tool-approval state back to the parent -// on completion. This is the "interactive" path used by transfer_task and +// runForwarding manages the lifecycle of a blocking sub-session, forwarding +// events to evts. The child's approval state stays scoped to the sub-session +// and never flows back to the parent. This is the "interactive" path used by transfer_task and // run_skill: the parent loop is blocked while the child executes, and // the user sees the child's events live. // diff --git a/pkg/runtime/agent_delegation_test.go b/pkg/runtime/agent_delegation_test.go index 00c197b3f2..e187c1a624 100644 --- a/pkg/runtime/agent_delegation_test.go +++ b/pkg/runtime/agent_delegation_test.go @@ -454,6 +454,59 @@ func TestRunAgent_InheritsParentPermissions(t *testing.T) { "parent permissions must be isolated from child mutations") } +// TestRunForwarding_DoesNotBackPropagateApprovals locks the "permissions only +// flow downwards" invariant: approvals granted within a sub-session scope must +// not escalate the parent's ToolsApproved gate or permission rules. +func TestRunForwarding_DoesNotBackPropagateApprovals(t *testing.T) { + t.Parallel() + + childStream := newStreamBuilder().AddContent("done").AddStopWithUsage(10, 5).Build() + prov := &mockProvider{id: "test/mock-model", stream: childStream} + + librarian := agent.New("librarian", "Library agent", agent.WithModel(prov)) + root := agent.New("root", "Root agent", agent.WithModel(prov)) + agent.WithSubAgents(librarian)(root) + + tm := team.New(team.WithAgents(root, librarian)) + rt, err := NewLocalRuntime(t.Context(), tm, + WithSessionCompaction(false), + WithModelStore(mockModelStore{}), + ) + require.NoError(t, err) + + parent := session.New( + session.WithUserMessage("Test"), + session.WithPermissions(&session.PermissionsConfig{Deny: []string{"dangerous_tool"}}), + ) + require.False(t, parent.IsToolsApproved()) + + evts := make(chan Event, 128) + // Child scope broader than the parent's, as if the user had clicked + // "approve all" / "always allow" inside the sub-session. + _, err = rt.runForwarding(t.Context(), parent, NewChannelSink(evts), delegationRequest{ + SubSessionConfig: SubSessionConfig{ + Task: "find a book", + AgentName: "librarian", + Title: "Transferred task", + ToolsApproved: true, + Permissions: &session.PermissionsConfig{ + Allow: []string{"exploit_tool"}, + Deny: []string{"dangerous_tool"}, + }, + }, + SwitchCurrentAgent: true, + }) + require.NoError(t, err) + + assert.False(t, parent.IsToolsApproved(), + "a sub-session must not escalate the parent's ToolsApproved gate") + parentPerms := parent.ClonePermissions() + require.NotNil(t, parentPerms) + assert.Empty(t, parentPerms.Allow, + "child-scope approvals must not leak into the parent's Allow list") + assert.Equal(t, []string{"dangerous_tool"}, parentPerms.Deny) +} + func TestRunAgent_EndToEndPermissions(t *testing.T) { t.Parallel() @@ -514,7 +567,9 @@ func TestRunAgent_EndToEndPermissions(t *testing.T) { } } require.NotNil(t, childSession, "parent must have a sub-session") - + require.NotNil(t, childSession.Permissions) + assert.Equal(t, []string{"dangerous_tool"}, childSession.Permissions.Deny, + "child must inherit the parent's Deny rules") require.False(t, executed, "expected dangerous_tool to NOT be executed because it is denied by inherited permissions") }