fix: correct Decide precedence and remove sub-session scope escalation

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
 

docs/features/cli/index.md

@@ -28,7 +28,7 @@ $ docker agent run [config] [message...] [flags]
 | Flag                                    | Description                                                                                                                               |
 | --------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------------- |
 | `-a, --agent <name>`                    | Run a specific agent from the config                                                                                                      |
-| `--yolo`                                | Auto-approve all tool calls                                                                                                               |
+| `--yolo`                                | Auto-approve tool calls (unless explicitly denied)                                                                                        |
 | `--model <ref>`                         | Override model(s). Use `provider/model` for all agents, or `agent=provider/model` for specific agents. Comma-separate multiple overrides. |
 | `--session <id>`                        | 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>`               | Path to the SQLite session database (default: `<data-dir>/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 <ref>` — 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)

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.
 //
@@ -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
 }

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,47 @@ 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)
+
+	// Even with ToolsApproved set (yolo), an inherited Deny must win 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()
 
@@ -409,6 +454,125 @@ 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()
+
+	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),
+	)
+
+	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.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")
+}
+
 func TestTransferTask_PropagatesPermissions(t *testing.T) {
 	t.Parallel()
 

pkg/runtime/runtime_test.go

@@ -2517,10 +2517,11 @@ 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
+	// Test that deny permissions take precedence over the --yolo flag
 	permChecker := permissions.NewChecker(&latest.PermissionsConfig{
 		Deny: []string{"dangerous_tool"},
 	})
@@ -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 // fail fast instead of hanging on askUser if this regresses
 	require.True(t, sess.ToolsApproved)
 
 	calls := []tools.ToolCall{{
@@ -2609,14 +2612,16 @@ 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
+	// Test that session-level deny takes precedence over the --yolo flag
 	var executed bool
 	agentTools := []tools.Tool{{
 		Name:       "blocked_tool",
@@ -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,12 +18,26 @@ 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: 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)
 }