-
Notifications
You must be signed in to change notification settings - Fork 410
fix(runtime): propagate session permissions to background agents and skills #3490
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
8f21080
fd89580
08fe4e6
3d6fbcb
a9d44c4
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -93,6 +93,8 @@ type SubSessionConfig struct { | |
| Title string | ||
| // ToolsApproved overrides whether tools are pre-approved in the child session. | ||
| ToolsApproved bool | ||
| // Permissions defines session-level tool permission overrides. | ||
| Permissions *session.PermissionsConfig | ||
| // NonInteractive marks the child session as running without a user present | ||
| // (e.g. MCP server, A2A adapter, background agent). This causes the runtime | ||
| // to auto-stop on max iterations instead of blocking for user input. | ||
|
|
@@ -184,6 +186,7 @@ func newSubSession(parent *session.Session, cfg SubSessionConfig, childAgent *ag | |
| if cfg.PinAgent { | ||
| opts = append(opts, session.WithAgentName(cfg.AgentName)) | ||
| } | ||
| opts = append(opts, session.WithPermissions(cfg.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. | ||
|
|
@@ -319,11 +322,12 @@ func (r *LocalRuntime) runForwarding(ctx context.Context, parent *session.Sessio | |
| return nil, subSessionErr | ||
| } | ||
|
|
||
| // Only propagate ToolsApproved on success. A failed sub-session must not | ||
| // silently escalate the parent's tool-approval gate: the user approved | ||
| // 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.ToolsApproved = s.ToolsApproved | ||
| parent.SetToolsApproved(s.IsToolsApproved()) | ||
| parent.SetPermissions(s.ClonePermissions()) | ||
| span.SetStatus(codes.Ok, "sub-session completed") | ||
| return tools.ResultSuccess(s.GetLastAssistantMessageContent()), nil | ||
| } | ||
|
|
@@ -476,23 +480,18 @@ func (r *LocalRuntime) CurrentAgentSubAgentNames() []string { | |
| // RunAgent implements agenttool.Runner. It starts a sub-agent synchronously | ||
| // and blocks until completion or cancellation. | ||
| // | ||
| // Background tasks run with tools pre-approved because there is no user | ||
| // present to respond to interactive approval prompts during async | ||
| // execution. This is a deliberate design trade-off: the user implicitly | ||
| // 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. | ||
| // Background tasks inherit the parent session's permissions because there is no user | ||
| // present to respond to interactive approval prompts during async execution. | ||
| // Tool calls that result in an "Ask" outcome will be auto-denied by the dispatcher | ||
| // due to the non-interactive context. | ||
| func (r *LocalRuntime) RunAgent(ctx context.Context, params agenttool.RunParams) *agenttool.RunResult { | ||
| return r.runCollecting(ctx, params.ParentSession, SubSessionConfig{ | ||
| Task: params.Task, | ||
| ExpectedOutput: params.ExpectedOutput, | ||
| AgentName: params.AgentName, | ||
| Title: "Background agent task", | ||
| ToolsApproved: true, | ||
| ToolsApproved: params.ParentSession.IsToolsApproved(), | ||
| Permissions: params.ParentSession.ClonePermissions(), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Minor: clone style is inconsistent across call sites. Here
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've updated both |
||
| NonInteractive: true, | ||
| PinAgent: true, | ||
| }, params.OnContent) | ||
|
|
@@ -551,7 +550,8 @@ func (r *LocalRuntime) handleTaskTransfer(ctx context.Context, sess *session.Ses | |
| ExpectedOutput: params.ExpectedOutput, | ||
| AgentName: params.Agent, | ||
| Title: "Transferred task", | ||
| ToolsApproved: sess.ToolsApproved, | ||
| ToolsApproved: sess.IsToolsApproved(), | ||
| Permissions: sess.ClonePermissions(), | ||
| NonInteractive: sess.NonInteractive, | ||
| }, | ||
| SwitchCurrentAgent: true, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Needs a decision: this flips
run_background_agentfrom blanket pre-approval to "inherit parent permissions, auto-deny anything that would prompt". In a default interactive session that makes background agents effectively read-only unless YOLO mode or explicit allow rules are set. The wording reads well; just confirming this is the intended default posture and not only an internal-safety tweak, since it can break setups that relied on background agents performing mutating actions.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, confirming this is the intended default safety posture.
The previous blanket pre-approval allowed background agents to bypass user authorization gates entirely. Restricting them to inherit the parent session's permissions (and auto-denying any prompt-requiring tools due to their non-interactive environment) ensures they default to a safe, read-only-equivalent mode unless explicitly authorized by the user (e.g., via YOLO mode or explicit allow rules).