Python: scope under-specified approve-for-session permission decisions

python/packages/github_copilot/README.md

@@ -50,6 +50,34 @@ agent = GitHubCopilotAgent(
 > Note: with the default (deny-all) permission handler, an `always_require` tool is denied
 > unless you wire an approving `on_permission_request`.
 
+### Approving for the rest of the session
+
+`PermissionDecisionApproveForSession` scopes its approval with either an `approval` (tool
+prompts) or a `domain` (URL prompts). Both are optional, so a bare
+`PermissionDecisionApproveForSession()` carries no scope at all and the Copilot CLI cannot
+interpret it.
+
+`GitHubCopilotAgent` therefore scopes such a decision automatically, using the request that
+triggered it — a shell prompt becomes an approval for that prompt's command identifiers, an
+MCP prompt an approval for that server and tool, a URL prompt an approval for that URL's
+domain, and so on:
+
+```python
+from copilot.generated.rpc import PermissionDecisionApproveForSession
+
+
+def on_permission_request(request, invocation):
+    # Scoped to `request` automatically; approves that kind of call for the whole session.
+    return PermissionDecisionApproveForSession()
+```
+
+The decision is only ever narrowed, never widened. When the prompt reports that it cannot
+offer session-scoped approval (`can_offer_session_approval=False`), or the request kind has
+no session-scoped approval at all (such as a `hook` prompt), the decision is downgraded to a
+single-use approval and a warning is logged. Pass an explicit `approval=` or `domain=` when
+you want to approve something other than the request being handled — decisions that already
+specify a scope are forwarded unchanged.
+
 ### Deprecated: `on_function_approval`
 
 The `on_function_approval` callback is **deprecated**. It still works (and is still enforced

python/packages/github_copilot/agent_framework_github_copilot/_agent.py

@@ -10,6 +10,7 @@
 import warnings
 from collections.abc import AsyncIterable, Awaitable, Callable, Mapping, MutableMapping, Sequence
 from typing import Any, ClassVar, Generic, Literal, TypedDict, cast, overload
+from urllib.parse import urlparse
 
 from agent_framework import (
     AgentMiddlewareLayer,
@@ -52,7 +53,20 @@
 
 try:
     from copilot import CopilotClient, CopilotSession, RuntimeConnection
-    from copilot.generated.rpc import PermissionDecisionUserNotAvailable
+    from copilot.generated.rpc import (
+        PermissionDecisionApproveForSession,
+        PermissionDecisionApproveForSessionApproval,
+        PermissionDecisionApproveForSessionApprovalCommands,
+        PermissionDecisionApproveForSessionApprovalCustomTool,
+        PermissionDecisionApproveForSessionApprovalExtensionManagement,
+        PermissionDecisionApproveForSessionApprovalExtensionPermissionAccess,
+        PermissionDecisionApproveForSessionApprovalMCP,
+        PermissionDecisionApproveForSessionApprovalMemory,
+        PermissionDecisionApproveForSessionApprovalRead,
+        PermissionDecisionApproveForSessionApprovalWrite,
+        PermissionDecisionApproveOnce,
+        PermissionDecisionUserNotAvailable,
+    )
     from copilot.session import (
         Attachment,
         BlobAttachment,
@@ -64,7 +78,21 @@
         SessionHooks,
         SystemMessageConfig,
     )
-    from copilot.session_events import AssistantUsageData, PermissionRequest, SessionEvent, SessionEventType
+    from copilot.session_events import (
+        AssistantUsageData,
+        PermissionRequest,
+        PermissionRequestCustomTool,
+        PermissionRequestExtensionManagement,
+        PermissionRequestExtensionPermissionAccess,
+        PermissionRequestMcp,
+        PermissionRequestMemory,
+        PermissionRequestRead,
+        PermissionRequestShell,
+        PermissionRequestUrl,
+        PermissionRequestWrite,
+        SessionEvent,
+        SessionEventType,
+    )
     from copilot.tools import Tool as CopilotTool
     from copilot.tools import ToolInvocation, ToolResult
 except ImportError as _copilot_import_error:
@@ -81,6 +109,9 @@
 ]
 """Type for permission request handlers. Supports both sync and async callbacks."""
 
+AsyncPermissionHandlerType = Callable[[PermissionRequest, dict[str, str]], "Awaitable[PermissionRequestResult]"]
+"""Type for permission request handlers that are always asynchronous."""
+
 
 FunctionApprovalCallback = Callable[[Content], "bool | Awaitable[bool]"]
 """Deprecated approval callback for ``FunctionTool`` instances declared with
@@ -140,6 +171,138 @@ def _deny_all_permissions(
     return PermissionDecisionUserNotAvailable()
 
 
+def _derive_session_approval(request: PermissionRequest) -> PermissionDecisionApproveForSessionApproval | None:
+    """Build the session-scoped approval implied by ``request``.
+
+    ``PermissionDecisionApproveForSession.approval`` describes *what* is being approved for
+    the remainder of the session. Its shape is dictated by the prompt that triggered it, so
+    it can be reconstructed from the request itself.
+
+    Args:
+        request: The permission request the decision is responding to.
+
+    Returns:
+        The approval covering ``request``, or ``None`` for request kinds that have no
+        session-scoped approval representation (such as ``hook`` prompts).
+    """
+    if isinstance(request, PermissionRequestShell):
+        return PermissionDecisionApproveForSessionApprovalCommands(
+            command_identifiers=[command.identifier for command in request.commands]
+        )
+    if isinstance(request, PermissionRequestRead):
+        return PermissionDecisionApproveForSessionApprovalRead()
+    if isinstance(request, PermissionRequestWrite):
+        return PermissionDecisionApproveForSessionApprovalWrite()
+    if isinstance(request, PermissionRequestMcp):
+        return PermissionDecisionApproveForSessionApprovalMCP(
+            server_name=request.server_name, tool_name=request.tool_name
+        )
+    if isinstance(request, PermissionRequestCustomTool):
+        return PermissionDecisionApproveForSessionApprovalCustomTool(tool_name=request.tool_name)
+    if isinstance(request, PermissionRequestMemory):
+        return PermissionDecisionApproveForSessionApprovalMemory()
+    if isinstance(request, PermissionRequestExtensionManagement):
+        return PermissionDecisionApproveForSessionApprovalExtensionManagement(operation=request.operation)
+    if isinstance(request, PermissionRequestExtensionPermissionAccess):
+        return PermissionDecisionApproveForSessionApprovalExtensionPermissionAccess(
+            extension_name=request.extension_name
+        )
+    return None
+
+
+def _normalize_permission_decision(
+    decision: PermissionRequestResult,
+    request: PermissionRequest,
+) -> PermissionRequestResult:
+    """Fill in the missing scope of an under-specified ``approve-for-session`` decision.
+
+    ``PermissionDecisionApproveForSession`` carries an optional ``approval`` (tool prompts)
+    and an optional ``domain`` (URL prompts), so ``PermissionDecisionApproveForSession()``
+    is constructible with neither. That serializes to ``{"kind": "approve-for-session"}``,
+    which the Copilot CLI cannot interpret -- it crashes with ``Cannot read properties of
+    undefined (reading 'commandIdentifiers')``, taking the whole run down with it. This
+    reconstructs the intended scope from ``request``.
+
+    The decision is only ever narrowed, never widened: when the prompt does not offer
+    session-scoped approval, or the request kind has no session approval representation,
+    the decision is downgraded to a single-use approval.
+
+    Args:
+        decision: The decision returned by the caller's permission handler.
+        request: The permission request the decision is responding to.
+
+    Returns:
+        ``decision`` unchanged unless it is an ``approve-for-session`` decision missing both
+        ``approval`` and ``domain``, in which case an equivalent fully-scoped decision (or a
+        narrower single-use approval) is returned. The input is never mutated.
+    """
+    if not isinstance(decision, PermissionDecisionApproveForSession):
+        return decision
+    if decision.approval is not None or decision.domain is not None:
+        return decision
+
+    try:
+        if isinstance(request, PermissionRequestUrl):
+            domain = urlparse(request.url).hostname
+            if domain:
+                return PermissionDecisionApproveForSession(domain=domain)
+            logger.warning(
+                "Permission handler returned an unscoped 'approve-for-session' decision for a URL prompt, "
+                "but no domain could be derived from '%s'. Approving this request only. Return "
+                "PermissionDecisionApproveForSession(domain=...) to approve a domain for the session.",
+                request.url,
+            )
+            return PermissionDecisionApproveOnce()
+
+        # Only shell and write prompts advertise this; other kinds always allow session approval.
+        if not getattr(request, "can_offer_session_approval", True):
+            logger.warning(
+                "Permission handler returned an 'approve-for-session' decision for a '%s' prompt that does not "
+                "offer session-scoped approval. Approving this request only.",
+                request.kind,
+            )
+            return PermissionDecisionApproveOnce()
+
+        approval = _derive_session_approval(request)
+    except Exception:
+        logger.exception(
+            "Failed to derive the session approval for a '%s' permission prompt. Approving this request only.",
+            getattr(request, "kind", "unknown"),
+        )
+        return PermissionDecisionApproveOnce()
+
+    if approval is None:
+        logger.warning(
+            "Permission handler returned an unscoped 'approve-for-session' decision for a '%s' prompt, which has "
+            "no session-scoped approval. Approving this request only.",
+            request.kind,
+        )
+        return PermissionDecisionApproveOnce()
+    return PermissionDecisionApproveForSession(approval=approval)
+
+
+def _with_normalized_permission_decisions(handler: PermissionHandlerType) -> AsyncPermissionHandlerType:
+    """Wrap a permission handler so its decisions are normalized before reaching the SDK.
+
+    Exceptions raised by ``handler`` deliberately propagate: the SDK already catches them
+    and denies the request, and preserving that keeps the secure-by-default behavior.
+
+    Args:
+        handler: The caller-supplied permission handler. May be sync or async.
+
+    Returns:
+        An async handler delegating to ``handler`` and normalizing its result.
+    """
+
+    async def normalized_handler(request: PermissionRequest, invocation: dict[str, str]) -> PermissionRequestResult:
+        result = handler(request, invocation)
+        if inspect.isawaitable(result):
+            result = await result
+        return _normalize_permission_decision(result, request)
+
+    return normalized_handler
+
+
 class GitHubCopilotSettings(TypedDict, total=False):
     """GitHub Copilot model settings.
 
@@ -1201,9 +1364,10 @@ def _build_session_kwargs(
         the Copilot SDK, so any ``create_session`` parameter is supported without a
         dedicated mapping here (an unknown name surfaces as a ``TypeError`` from the
         SDK). A few keys are handled specially because they need a secure default
-        (``on_permission_request`` defaults to denying all requests) or transforming:
-        ``tools`` are merged with the agent's tools and converted to SDK tools, and
-        approval callbacks are turned into ``hooks``.
+        (``on_permission_request`` defaults to denying all requests, and is wrapped so
+        under-specified ``approve-for-session`` decisions are scoped to the request that
+        triggered them) or transforming: ``tools`` are merged with the agent's tools and
+        converted to SDK tools, and approval callbacks are turned into ``hooks``.
 
         Args:
             streaming: Whether to enable streaming for the session.
@@ -1227,7 +1391,7 @@ def _build_session_kwargs(
         # back to the resolved setting (which carries the default_options / env model).
         if not kwargs.get("model"):
             kwargs["model"] = self._settings.get("model") or None
-        kwargs["on_permission_request"] = (
+        kwargs["on_permission_request"] = _with_normalized_permission_decisions(
             opts.get("on_permission_request") or self._permission_handler or _deny_all_permissions
         )
         kwargs["hooks"] = self._build_session_hooks(all_tools, kwargs)