fix: propagate session permissions correctly to sub-sessions

pkg/runtime/agent_delegation_test.go

@@ -1,13 +1,17 @@
 package runtime
 
 import (
+	"context"
 	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
 	"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"
 	"github.com/docker/docker-agent/pkg/team"
 	"github.com/docker/docker-agent/pkg/tools"
@@ -270,6 +274,48 @@ func TestSubSessionWithoutAttachedFilesOmitsBlock(t *testing.T) {
 	}
 }
 
+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",
+		Permissions: parent.ClonePermissions(),
+	}
+
+	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)
+
+	// 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)")
+}
+
 func TestNewSubSession_PermissionsIsolation(t *testing.T) {
 	t.Parallel()
 
@@ -364,7 +410,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{}),
 	)
@@ -409,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()
 
@@ -420,7 +521,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{}),
 	)

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,10 +2562,11 @@ 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")
 }
 
+// TestYoloMode_OverridesForceAsk verifies that the yolo flag takes precedence over ForceAsk permissions.
 func TestYoloMode_OverridesForceAsk(t *testing.T) {
 	t.Parallel()
 
@@ -2597,6 +2599,7 @@ func TestYoloMode_OverridesForceAsk(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{{
@@ -2609,11 +2612,13 @@ 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")
+	// 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")
 }
 
-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 +2661,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) {

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.
 //
@@ -75,23 +75,26 @@ 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:
 			return PermissionDecision{Outcome: OutcomeDeny, Reason: ReasonChecker, Source: pc.Source}
 		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.
 		}
 	}
 
+	if yoloApproved {
+		return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonYolo}
+	}
+
 	if readOnlyHint {
 		return PermissionDecision{Outcome: OutcomeAllow, Reason: ReasonReadOnlyHint}
 	}

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) {

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()
@@ -319,17 +319,6 @@ func cloneEvalResultChecks(src EvalResultChecks) EvalResultChecks {
 	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

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)
@@ -888,7 +900,7 @@ func WithSendUserMessage(sendUserMessage bool) Opt {
 
 func WithPermissions(perms *PermissionsConfig) Opt {
 	return func(s *Session) {
-		s.Permissions = clonePermissionsConfig(perms)
+		s.Permissions = perms.Clone()
 	}
 }
 
@@ -1046,7 +1058,7 @@ func (s *Session) IsToolsApproved() bool {
 func (s *Session) ClonePermissions() *PermissionsConfig {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
-	return clonePermissionsConfig(s.Permissions)
+	return s.Permissions.Clone()
 }
 
 // SetPermissions safely updates the session's PermissionsConfig.

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,