diff --git a/app/api/routers/breeze_buddy/chat/__init__.py b/app/api/routers/breeze_buddy/chat/__init__.py index da5867238..494158ebe 100644 --- a/app/api/routers/breeze_buddy/chat/__init__.py +++ b/app/api/routers/breeze_buddy/chat/__init__.py @@ -38,6 +38,7 @@ from app.schemas import UserInfo from app.schemas.breeze_buddy.chat import ( ApproveToolRequest, + ChatSession, ChatSessionStatus, ChatTranscriptResponse, CreateChatSessionRequest, @@ -47,6 +48,10 @@ ListChatSessionsResponse, SendChatMessageRequest, ) +from app.services.breeze_buddy.copilot.scope import ( + CopilotScopeError, + validate_persisted_copilot_scope_access, +) from .demo import router as demo_router from .handlers import ( @@ -66,6 +71,31 @@ router = APIRouter(prefix="/chat", tags=["chat"]) +def _hidden_scope_error(error: CopilotScopeError) -> HTTPException: + if error.status_code == status.HTTP_404_NOT_FOUND: + return HTTPException( + status_code=status.HTTP_404_NOT_FOUND, + detail="Chat session not found", + ) + return HTTPException( + status_code=error.status_code, + detail={"code": error.code, "message": error.message}, + ) + + +async def _validate_chat_and_copilot_session_access( + current_user: UserInfo, + session: ChatSession, + *, + operation: str, +) -> None: + validate_chat_session_access(current_user, session, operation=operation) + try: + await validate_persisted_copilot_scope_access(session.metadata, current_user) + except CopilotScopeError as error: + raise _hidden_scope_error(error) from error + + @router.post( "/session", status_code=status.HTTP_201_CREATED, @@ -184,7 +214,11 @@ async def get_session( (returns 404 to avoid leaking existence) """ session = await load_chat_session_or_404(session_id) - validate_chat_session_access(current_user, session, operation="get_session") + await _validate_chat_and_copilot_session_access( + current_user, + session, + operation="get_session", + ) return await get_chat_session_handler(session) @@ -216,8 +250,12 @@ async def send_message( # (the demo router passes ``access_check=None`` โ€” the demo token is # already bound to a specific session_id and there's nothing further # to authorise). - def _check(session) -> None: - validate_chat_session_access(current_user, session, operation="send_message") + async def _check(session) -> None: + await _validate_chat_and_copilot_session_access( + current_user, + session, + operation="send_message", + ) return await send_chat_message_handler(session_id, req, access_check=_check) @@ -247,8 +285,12 @@ async def approve_tool( ``lock_contended``) on conflicts. """ - def _check(session) -> None: - validate_chat_session_access(current_user, session, operation="approve_tool") + async def _check(session) -> None: + await _validate_chat_and_copilot_session_access( + current_user, + session, + operation="approve_tool", + ) return await approve_chat_tool_handler(session_id, req, access_check=_check) @@ -281,7 +323,11 @@ async def cancel_turn( we're cancelling. """ session = await load_chat_session_or_404(session_id) - validate_chat_session_access(current_user, session, operation="cancel_turn") + await _validate_chat_and_copilot_session_access( + current_user, + session, + operation="cancel_turn", + ) await cancel_chat_turn_handler(session_id) return Response(status_code=status.HTTP_202_ACCEPTED) @@ -302,7 +348,11 @@ async def end_session( - Reseller / Merchant: Must own the session (404 otherwise) """ session = await load_chat_session_or_404(session_id) - validate_chat_session_access(current_user, session, operation="end_session") + await _validate_chat_and_copilot_session_access( + current_user, + session, + operation="end_session", + ) return await end_chat_session_handler(session_id, session) @@ -322,7 +372,11 @@ async def get_transcript( - Reseller / Merchant: Must own the session (404 otherwise) """ session = await load_chat_session_or_404(session_id) - validate_chat_session_access(current_user, session, operation="get_transcript") + await _validate_chat_and_copilot_session_access( + current_user, + session, + operation="get_transcript", + ) return await get_chat_transcript_handler(session) diff --git a/app/api/routers/breeze_buddy/chat/handlers.py b/app/api/routers/breeze_buddy/chat/handlers.py index b79d0deb2..7bf15cc9b 100644 --- a/app/api/routers/breeze_buddy/chat/handlers.py +++ b/app/api/routers/breeze_buddy/chat/handlers.py @@ -9,7 +9,8 @@ import asyncio import time from datetime import datetime -from typing import Any, AsyncIterator, Callable, Dict, List, Optional +from inspect import isawaitable +from typing import Any, AsyncIterator, Awaitable, Callable, Dict, List, Optional from fastapi import HTTPException, status from fastapi.responses import StreamingResponse @@ -86,6 +87,11 @@ ToolApprovalStatus, ) from app.schemas.breeze_buddy.conversation_analysis import ConversationChannel +from app.schemas.breeze_buddy.copilot import COPILOT_SCOPE_METADATA_KEY +from app.services.breeze_buddy.copilot.scope import ( + CopilotScopeError, + resolve_copilot_scope, +) from app.services.redis.locks import LockAcquireError, RedisLock from . import cancel_bus @@ -94,6 +100,8 @@ # is well under this โ€” if it isn't, the upstream LLM is hung and we # want the lock to expire so retries can recover. No mid-turn renewal. _SESSION_LOCK_TTL_SECONDS = 180 +_RESERVED_METADATA_KEYS = frozenset({"template_vars", COPILOT_SCOPE_METADATA_KEY}) +AccessCheck = Callable[[ChatSession], Awaitable[None] | None] def _lock_key(session_id: str) -> str: @@ -101,6 +109,50 @@ def _lock_key(session_id: str) -> str: return f"chat:session:{session_id}:lock" +def _scope_http_error(error: CopilotScopeError) -> HTTPException: + return HTTPException( + status_code=error.status_code, + detail={"code": error.code, "message": error.message}, + ) + + +async def _run_access_check( + access_check: Optional[AccessCheck], + session: ChatSession, +) -> None: + if access_check is None: + return + result = access_check(session) + if isawaitable(result): + await result + + +def _validate_client_metadata(metadata: Optional[Dict[str, Any]]) -> Dict[str, Any]: + client_metadata = dict(metadata or {}) + reserved_keys = _RESERVED_METADATA_KEYS.intersection(client_metadata) + if reserved_keys: + key = sorted(reserved_keys)[0] + raise HTTPException( + status_code=status.HTTP_422_UNPROCESSABLE_ENTITY, + detail=f"metadata.{key} is server-owned", + ) + return client_metadata + + +def _build_session_metadata( + *, + client_metadata: Dict[str, Any], + template_vars: Dict[str, Any], + server_metadata: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """Merge session metadata while protecting server-owned namespaces.""" + return { + **client_metadata, + **(server_metadata or {}), + "template_vars": template_vars, + } + + async def _persist_turn_metrics(metrics: TurnMetrics) -> None: """Best-effort write of one turn's metrics to chat_turn_metrics. @@ -330,13 +382,20 @@ async def create_chat_session_handler( transformed_template_vars = _apply_payload_transformations( req.template_vars, template.expected_payload_schema ) - # Server-owned `template_vars` must win over any client-supplied - # metadata: a crafted `metadata={"template_vars": ...}` would otherwise - # corrupt prompt rendering for every subsequent turn on this session. - persisted_metadata = { - **(req.metadata or {}), - "template_vars": transformed_template_vars, - } + client_metadata = _validate_client_metadata(req.metadata) + resolved_server_metadata: Dict[str, Any] = {} + if req.copilot_scope is not None: + try: + scope = await resolve_copilot_scope(req.copilot_scope, current_user) + except CopilotScopeError as error: + raise _scope_http_error(error) from error + resolved_server_metadata.update(scope.session_metadata()) + + persisted_metadata = _build_session_metadata( + client_metadata=client_metadata, + template_vars=transformed_template_vars, + server_metadata=resolved_server_metadata, + ) db_session = await create_chat_session( template_id=req.template_id, reseller_id=template.reseller_id, @@ -445,7 +504,7 @@ async def send_chat_message_handler( session_id: str, req: SendChatMessageRequest, *, - access_check: Optional[Callable[[ChatSession], None]] = None, + access_check: Optional[AccessCheck] = None, internal: bool = False, ) -> StreamingResponse: """Drive one turn; stream SSE events until ``turn_end``. @@ -504,8 +563,7 @@ async def send_chat_message_handler( status_code=status.HTTP_404_NOT_FOUND, detail=f"Chat session '{session_id}' not found", ) - if access_check is not None: - access_check(fresh) + await _run_access_check(access_check, fresh) if fresh.status == ChatSessionStatus.ENDED: raise HTTPException( status_code=status.HTTP_410_GONE, @@ -671,7 +729,7 @@ async def send_chat_intent_handler( parsed: ParsedIntent, *, context: Any = None, - access_check: Optional[Callable[[ChatSession], None]] = None, + access_check: Optional[AccessCheck] = None, ) -> StreamingResponse: """Drive one DIRECT-routed UI intent (RFC-001 ยง3.3); stream SSE until ``turn_end``. The no-LLM sibling of ``send_chat_message_handler`` โ€” @@ -705,8 +763,7 @@ async def send_chat_intent_handler( status_code=status.HTTP_404_NOT_FOUND, detail=f"Chat session '{session_id}' not found", ) - if access_check is not None: - access_check(fresh) + await _run_access_check(access_check, fresh) if fresh.status == ChatSessionStatus.ENDED: raise HTTPException( status_code=status.HTTP_410_GONE, @@ -894,7 +951,7 @@ async def approve_chat_tool_handler( session_id: str, req: ApproveToolRequest, *, - access_check: Optional[Callable[[ChatSession], None]] = None, + access_check: Optional[AccessCheck] = None, ) -> StreamingResponse: """Apply a HITL decision to a pending tool approval and stream the resumed turn (same SSE shape as ``/message``). @@ -932,8 +989,7 @@ async def approve_chat_tool_handler( status_code=status.HTTP_404_NOT_FOUND, detail=f"Chat session '{session_id}' not found", ) - if access_check is not None: - access_check(fresh) + await _run_access_check(access_check, fresh) if fresh.status == ChatSessionStatus.ENDED: raise HTTPException( status_code=status.HTTP_410_GONE, diff --git a/app/schemas/breeze_buddy/chat.py b/app/schemas/breeze_buddy/chat.py index dddb683f1..289ae1d57 100644 --- a/app/schemas/breeze_buddy/chat.py +++ b/app/schemas/breeze_buddy/chat.py @@ -12,6 +12,7 @@ from pydantic import BaseModel, Field, model_validator from app.ai.voice.agents.breeze_buddy.template.ui_catalog import ActionUnion, Icon +from app.schemas.breeze_buddy.copilot import CopilotScopeRequest class ChatSessionStatus(str, Enum): @@ -229,6 +230,14 @@ class CreateChatSessionRequest(BaseModel): default_factory=dict, description="Opaque caller-provided context, persisted on chat_session.metadata.", ) + copilot_scope: Optional[CopilotScopeRequest] = Field( + default=None, + description=( + "Optional dashboard data scope for Buddy Copilot-style Assist " + "templates. The server validates and persists the resolved scope " + "under metadata.copilot." + ), + ) class GreetingMessage(BaseModel): diff --git a/app/schemas/breeze_buddy/copilot.py b/app/schemas/breeze_buddy/copilot.py index 41c0fd5f4..95836a6c2 100644 --- a/app/schemas/breeze_buddy/copilot.py +++ b/app/schemas/breeze_buddy/copilot.py @@ -31,6 +31,8 @@ class CopilotDateRangeSource(str, Enum): class CopilotRequestedDateRange(BaseModel): """Optional dashboard-provided date range for Copilot data reads.""" + model_config = ConfigDict(extra="forbid") + date_from: date date_to: date @@ -50,6 +52,8 @@ class CopilotScopeRequest(BaseModel): remains authoritative and returns the immutable CopilotScope. """ + model_config = ConfigDict(extra="forbid") + data_merchant_id: Optional[str] = Field( default=None, description="Selected merchant whose analytics/conversations are queried.", @@ -141,8 +145,10 @@ class CopilotScope(BaseModel): capabilities: tuple[CopilotCapability, ...] def session_metadata(self) -> Dict[str, Dict[str, object]]: - """Return semantic scope metadata for the normal Assist chat session.""" - return {COPILOT_SCOPE_METADATA_KEY: self.model_dump(mode="json")} + """Return redacted scope metadata for the normal Assist chat session.""" + scope_payload = self.model_dump(mode="json", exclude={"actor"}) + scope_payload["actor"] = {"user_id": self.actor.user_id} + return {COPILOT_SCOPE_METADATA_KEY: scope_payload} class CopilotResolvedScopeResponse(BaseModel): diff --git a/app/services/breeze_buddy/copilot/__init__.py b/app/services/breeze_buddy/copilot/__init__.py index 7b3af4e9d..22af9c351 100644 --- a/app/services/breeze_buddy/copilot/__init__.py +++ b/app/services/breeze_buddy/copilot/__init__.py @@ -3,9 +3,11 @@ from app.services.breeze_buddy.copilot.scope import ( CopilotScopeError, resolve_copilot_scope, + validate_persisted_copilot_scope_access, ) __all__ = [ "CopilotScopeError", "resolve_copilot_scope", + "validate_persisted_copilot_scope_access", ] diff --git a/app/services/breeze_buddy/copilot/scope.py b/app/services/breeze_buddy/copilot/scope.py index 6595e1388..cf49dc5e9 100644 --- a/app/services/breeze_buddy/copilot/scope.py +++ b/app/services/breeze_buddy/copilot/scope.py @@ -4,13 +4,14 @@ from datetime import datetime, timedelta from inspect import isawaitable -from typing import Awaitable, Callable, List, Optional +from typing import Any, Awaitable, Callable, List, Optional from zoneinfo import ZoneInfo, ZoneInfoNotFoundError from app.core.security.scope import resolve_merchant_ids from app.database.accessor.breeze_buddy.template import get_template_merchant_id from app.schemas import UserInfo from app.schemas.breeze_buddy.copilot import ( + COPILOT_SCOPE_METADATA_KEY, DEFAULT_COPILOT_TIMEZONE, CopilotActorScope, CopilotCapability, @@ -149,6 +150,8 @@ async def _load_template_merchant_id( async def _validate_data_template( data: CopilotDataScope, template_merchant_loader: TemplateMerchantLoader, + *, + status_code: int = 403, ) -> None: if not data.data_template_id: return @@ -162,7 +165,7 @@ async def _validate_data_template( "unauthorized_template", "Selected Copilot data template was not found or does not belong " "to the selected merchant.", - status_code=403, + status_code=status_code, ) @@ -199,3 +202,99 @@ async def resolve_copilot_scope( date_window=date_window, capabilities=_PHASE_ONE_CAPABILITIES, ) + + +def _load_persisted_data_scope( + metadata: Optional[dict[str, Any]], +) -> Optional[dict[str, Any]]: + if not metadata: + return None + + copilot_metadata = metadata.get(COPILOT_SCOPE_METADATA_KEY) + if copilot_metadata is None: + return None + + if not isinstance(copilot_metadata, dict): + raise CopilotScopeError( + "invalid_persisted_scope", + "Stored Copilot scope is invalid.", + status_code=404, + ) + + data = copilot_metadata.get("data") + if not isinstance(data, dict): + raise CopilotScopeError( + "invalid_persisted_scope", + "Stored Copilot data scope is invalid.", + status_code=404, + ) + + return data + + +async def validate_persisted_copilot_scope_access( + metadata: Optional[dict[str, Any]], + current_user: UserInfo, + *, + template_merchant_loader: Optional[TemplateMerchantLoader] = None, + merchant_scope_resolver: Optional[MerchantScopeResolver] = None, +) -> None: + """Revalidate access to a stored Copilot data scope. + + Normal Assist chat-session RBAC controls the runtime session. When a + session carries ``metadata.copilot``, future Copilot tools will also trust + that server-owned data scope, so every dashboard resume/message path must + confirm the current actor still has access to the persisted data merchant + and optional data template. + """ + data_payload = _load_persisted_data_scope(metadata) + if data_payload is None: + return + + data_merchant_id = data_payload.get("data_merchant_id") + if not isinstance(data_merchant_id, str) or not data_merchant_id.strip(): + raise CopilotScopeError( + "invalid_persisted_scope", + "Stored Copilot data merchant is invalid.", + status_code=404, + ) + data_merchant_id = data_merchant_id.strip() + + data_template_id = data_payload.get("data_template_id") + if data_template_id is not None and not isinstance(data_template_id, str): + raise CopilotScopeError( + "invalid_persisted_scope", + "Stored Copilot data template is invalid.", + status_code=404, + ) + if isinstance(data_template_id, str): + data_template_id = data_template_id.strip() + if not data_template_id: + raise CopilotScopeError( + "invalid_persisted_scope", + "Stored Copilot data template is invalid.", + status_code=404, + ) + + allowed_merchant_ids = await _resolve_allowed_merchant_ids( + current_user, + merchant_scope_resolver or resolve_merchant_ids, + ) + if ( + allowed_merchant_ids is not None + and data_merchant_id not in allowed_merchant_ids + ): + raise CopilotScopeError( + "unauthorized_merchant", + "Access denied to the stored Copilot data merchant.", + status_code=404, + ) + + await _validate_data_template( + CopilotDataScope( + data_merchant_id=data_merchant_id, + data_template_id=data_template_id, + ), + template_merchant_loader or get_template_merchant_id, + status_code=404, + ) diff --git a/docs/breeze_buddy/copilot/dashboard_session_creation.md b/docs/breeze_buddy/copilot/dashboard_session_creation.md new file mode 100644 index 000000000..7a36a4b98 --- /dev/null +++ b/docs/breeze_buddy/copilot/dashboard_session_creation.md @@ -0,0 +1,268 @@ +# Buddy Copilot Dashboard Session Creation + +## Purpose + +This slice wires Buddy Copilot into the existing Breeze Assist chat widget and +chat session behavior. Copilot is not a separate runtime and does not get a +separate session endpoint. It is a normal dashboard Assist chat bot whose +session can carry an optional, server-validated Copilot data scope. + +The main result is the handoff between the BZN-40679 scope foundation and the +future BZN-40681 tool boundary: + +```text +Dashboard-hosted Breeze Assist chat widget + -> existing Assist chat session creation behavior + -> existing POST /api/breeze_buddy/chat/session + -> selected Assist template controls chat runtime + -> optional copilot_scope requests dashboard data scope + -> server resolves and persists metadata.copilot + -> ChatAgent continues through normal Breeze Assist runtime + -> resume/message paths revalidate metadata.copilot for the current user + -> future Copilot tools load metadata.copilot before data reads +``` + +## Architecture Fit + +Buddy Copilot has two identities that must stay separate: + +- Assist runtime identity: the selected chat template and its merchant. +- Copilot data scope: the merchant and optional template whose dashboard data + tools may read. + +`CreateChatSessionRequest.template_id` remains the Assist template id. The +persisted `chat_session.template_id` and `chat_session.merchant_id` still come +from that Assist template. They control the normal ChatAgent behavior, prompt, +and session access rules. + +`CreateChatSessionRequest.copilot_scope`, when present, is only a request for +dashboard data scope. The server resolves it through the Copilot scope +foundation and persists the resolved result under `metadata.copilot`. + +That means downstream Copilot tools must read data scope from +`metadata.copilot.data`, not from `chat_session.merchant_id`. + +## Request Shape + +The dashboard should use the normal Breeze Assist chat widget/session creation +path. At the API layer, that remains the existing chat session endpoint: + +```http +POST /api/breeze_buddy/chat/session +``` + +Normal Assist requests continue to work without any Copilot fields: + +```json +{ + "template_id": "dashboard-assist-template-id", + "template_vars": {}, + "metadata": { + "source": "dashboard" + } +} +``` + +A Copilot-enabled dashboard session adds `copilot_scope`: + +```json +{ + "template_id": "dashboard-assist-template-id", + "template_vars": {}, + "metadata": { + "source": "dashboard" + }, + "copilot_scope": { + "data_merchant_id": "merchant-1", + "data_template_id": "11111111-1111-4111-8111-111111111111", + "timezone": "Asia/Kolkata", + "date_range": { + "date_from": "2026-07-01", + "date_to": "2026-07-31" + } + } +} +``` + +`data_template_id` is optional. When it is omitted, later Copilot reads should +treat the scope as all agents under `data_merchant_id`. + +## Persisted Metadata + +Session creation now builds metadata from three sources: + +- caller metadata that is safe for clients to provide +- transformed `template_vars` +- server-owned metadata, currently `metadata.copilot` + +An abridged persisted shape is: + +```json +{ + "source": "dashboard", + "template_vars": {}, + "copilot": { + "actor": { + "user_id": "user-1" + }, + "data": { + "data_merchant_id": "merchant-1", + "data_template_id": "11111111-1111-4111-8111-111111111111" + }, + "date_window": { + "timezone": "Asia/Kolkata", + "date_from": "2026-07-01", + "date_to": "2026-07-31", + "source": "request", + "label": "2026-07-01 to 2026-07-31" + }, + "capabilities": [ + "get_analytics_summary", + "query_conversations", + "get_conversation_detail" + ] + } +} +``` + +`metadata.copilot` intentionally does not store runtime template or runtime +merchant fields. The runtime values are already represented by the +`chat_session` row. Duplicating them inside the Copilot scope would make it +easier for later tools to accidentally read the wrong boundary. + +The persisted `metadata.copilot.actor` is intentionally minimal too. It stores +only `user_id` for audit correlation and does not persist the creator's +username, role, permissions, reseller scope, or merchant scope. Session metadata +is returned by session-resume APIs to any user who can access the chat session, +so durable Copilot metadata must not expose the original actor's permission +snapshot. + +## Server-Owned Namespaces + +Clients cannot provide these metadata keys: + +- `metadata.copilot` +- `metadata.template_vars` + +`metadata.copilot` is server-owned because it is the authority later tools will +trust. `metadata.template_vars` is server-owned because session creation stores +the transformed template variables there after applying the template payload +schema. + +If a client tries to set either key directly, session creation rejects the +request with HTTP 422 before any chat session row is persisted. + +## Failure Behavior + +When `copilot_scope` is absent, session creation follows the ordinary Buddy +Assist path. + +When `copilot_scope` is present, the handler resolves it before persisting the +session. Scope errors are returned as structured HTTP errors: + +```json +{ + "code": "unauthorized_merchant", + "message": "..." +} +``` + +The same pattern is used for ambiguous merchant selection, unauthorized +template ownership, invalid timezone, and other scope-resolution failures. + +For existing sessions, normal chat-session RBAC is still checked first. If the +session contains `metadata.copilot`, dashboard resume/message/approval/cancel/ +end/transcript paths also revalidate the persisted Copilot data merchant and +optional data template against the current user before continuing. A user who +can still access the runtime Assist session but no longer has access to the +stored Copilot data scope receives the same hidden 404 shape used by chat +session RBAC. + +## Why There Is No Copilot Session Endpoint + +Copilot is a full Breeze Assist chat bot. Adding a separate Copilot session +endpoint would duplicate the chat widget/session lifecycle and make Copilot +drift away from the Assist architecture. + +This slice keeps the existing lifecycle intact: + +- the dashboard hosts the normal Assist chat widget behavior +- the selected Assist template controls runtime behavior +- `POST /chat/session` creates the session +- `POST /chat/message` runs the normal ChatAgent +- tool availability and behavior remain template/runtime concerns +- Copilot data safety is carried as `metadata.copilot` + +Routing Copilot-specific tools should happen through the agent template and +the future tool provider/guard, not through a special session API. + +## Connection To BZN-40681 + +BZN-40681 can treat `metadata.copilot` as the durable server-owned data-scope +contract for tool execution. + +This slice already revalidates that stored scope on normal dashboard session +access. BZN-40681 should still revalidate and inject the scope at tool +execution time, because the tool boundary is the last server-owned boundary +before analytics or conversation data is read. + +The expected tool flow is: + +```text +ChatAgent + -> asks CopilotToolProvider for read-only schemas + -> model emits tool call without data_merchant_id/data_template_id + -> provider/guard loads chat_session.metadata.copilot + -> guard injects scope.data into the executor + -> executor queries only the scoped merchant/template data + -> tool result returns scoped structured data with provenance + -> normal Assist ChatAgent renders prose or existing AI UI +``` + +The LLM must never be allowed to provide or override +`data_merchant_id` or `data_template_id` in tool arguments. Those values are +already persisted by this session creation slice and must be injected by the +tool boundary. + +Copilot should not introduce a Loom-specific rendering contract. If the Copilot +Assist template enables the right `ui_catalog` groups and gives the bot clear +UI instructions, the existing Assist chat UI path can render smart UI through +normal `ui_op` events and persisted `ui_blocks`. Without those template +instructions, the same tool result may simply become a prose answer. + +## Invariants + +- Copilot uses the existing Buddy Assist chat session endpoint. +- `template_id` is always the Assist runtime template id. +- `chat_session.merchant_id` is runtime identity, not Copilot data scope. +- `copilot_scope` is optional and only affects server-owned metadata. +- `metadata.copilot.data.data_merchant_id` is the data merchant for tools. +- `metadata.copilot.data.data_template_id` is the optional selected agent. +- `metadata.copilot.actor` stores only `user_id`; permission snapshots stay out + of durable session metadata. +- Clients cannot write or override `metadata.copilot`. +- Invalid Copilot scope requests fail before the session row is persisted. +- Existing Copilot sessions revalidate `metadata.copilot.data` for the current + user before resume/message/approval/cancel/end/transcript operations. +- Normal chat session creation and RBAC remain unchanged when Copilot is not + requested. + +## Files In This PR + +```text +app/api/routers/breeze_buddy/chat/handlers.py +app/api/routers/breeze_buddy/chat/__init__.py +app/schemas/breeze_buddy/chat.py +app/schemas/breeze_buddy/copilot.py +app/services/breeze_buddy/copilot/scope.py +tests/test_copilot_scope.py +tests/test_copilot_session.py +``` + +## Out Of Scope + +This slice does not register Copilot tools, execute analytics or conversation +queries, add AI UI instructions, or add Loom-specific UI behavior. + +Those later pieces should consume `metadata.copilot` and preserve the same +runtime-vs-data boundary documented here and in `scope_foundation.md`. diff --git a/docs/breeze_buddy/copilot/scope_foundation.md b/docs/breeze_buddy/copilot/scope_foundation.md index 4077a0dc8..867dee768 100644 --- a/docs/breeze_buddy/copilot/scope_foundation.md +++ b/docs/breeze_buddy/copilot/scope_foundation.md @@ -23,7 +23,8 @@ Loom dashboard -> normal Buddy Assist chat session -> metadata.copilot -> guarded read-only Copilot tools - -> typed results/events for Loom rendering + -> scoped data returned to the normal Assist chat runtime + -> existing Assist AI UI / widget rendering path when the bot emits UI ``` Loom owns the visible user context: the dashboard login, selected merchant, and @@ -38,6 +39,11 @@ Future Copilot tools will load the resolved scope from `metadata.copilot` and use only `scope.data.data_merchant_id` and optional `scope.data.data_template_id` for analytics and conversation reads. +Any path that lets a dashboard user resume or operate on a Copilot session must +also revalidate that persisted data scope for the current user. The runtime +session RBAC and the Copilot data-scope RBAC are related, but they are not the +same boundary. + ## What This PR Adds ### Scope Request @@ -70,12 +76,20 @@ The resolver returns an immutable `CopilotScope` with: - `date_window`: normalized date range and timezone. - `capabilities`: read-only Phase 1 capability names. +The resolved in-memory scope contains the full authenticated actor snapshot, but +durable session metadata stores only the minimum actor field needed for audit +correlation. Session metadata is visible through chat-session resume APIs, so +the persisted Copilot actor must not include username, role, permissions, +reseller scope, or merchant scope. + The resolved scope can be injected into the normal chat session as: ```json { "copilot": { - "actor": {}, + "actor": { + "user_id": "user-1" + }, "data": { "data_merchant_id": "merchant-1", "data_template_id": "11111111-1111-4111-8111-111111111111" @@ -122,6 +136,11 @@ The ownership lookup uses a lightweight template accessor that selects only `merchant_id`. It deliberately avoids loading the full template flow, configuration, or secrets for an authorization check. +The same merchant/template ownership checks are reused when an existing chat +session already has `metadata.copilot`. If a user's merchant access changes +after creation, the session fails closed before the normal Assist runtime can +resume or message with a stale Copilot data scope. + ### Date Window Normalization The resolver accepts an explicit date range or falls back to the previous seven @@ -151,6 +170,10 @@ Clairvoyance still validates the data boundary after authentication: - The dashboard Assist template merchant must not be used as Copilot data scope. - Runtime template/merchant identity is not stored in `metadata.copilot`. +- Only `actor.user_id` is stored in durable `metadata.copilot`; actor + permission and scope snapshots are not persisted there. +- Existing sessions carrying `metadata.copilot` must revalidate the stored data + merchant/template for the current user before dashboard session operations. - Missing `data_template_id` means all agents under `data_merchant_id`. - A provided `data_template_id` must be owned by `data_merchant_id`. @@ -167,8 +190,10 @@ tests/test_copilot_scope.py ## Out Of Scope This PR does not create Copilot chat sessions, provision the dashboard Assist -template, register tools, execute analytics queries, stream typed events, or add -Loom UI. Those later pieces should consume the scope contract defined here. +template, register tools, execute analytics queries, or add UI behavior. Later +UI should use the existing Assist chat AI UI path, not a Loom-specific Copilot +rendering contract. Those later pieces should consume the scope contract +defined here. The next backend slices can rely on `metadata.copilot` as the stable handoff between session creation and tool execution. diff --git a/tests/test_copilot_scope.py b/tests/test_copilot_scope.py index 853774623..f051b9181 100644 --- a/tests/test_copilot_scope.py +++ b/tests/test_copilot_scope.py @@ -18,6 +18,7 @@ from app.services.breeze_buddy.copilot.scope import ( CopilotScopeError, resolve_copilot_scope, + validate_persisted_copilot_scope_access, ) DATA_TEMPLATE_ID = "11111111-1111-4111-8111-111111111111" @@ -101,6 +102,7 @@ def test_scope_metadata_does_not_include_runtime_identity(): assert "runtime_merchant_id" not in metadata assert "runtime_template_id" not in metadata + assert metadata["actor"] == {"user_id": "user-1"} assert isinstance(data_metadata, dict) assert "merchant_id" not in data_metadata @@ -340,3 +342,91 @@ async def run_query(query: str, values: list[object]): assert "SELECT merchant_id" in str(captured["query"]) assert "flow" not in str(captured["query"]) assert "secrets" not in str(captured["query"]) + + +def test_persisted_scope_access_allows_current_data_merchant(): + scope = asyncio.run( + resolve_copilot_scope( + CopilotScopeRequest( + data_merchant_id="merchant-1", + data_template_id=DATA_TEMPLATE_ID, + ), + _user(), + template_merchant_loader=_template_merchant_loader(_templates()), + merchant_scope_resolver=_merchant_scope(["merchant-1"]), + ) + ) + + asyncio.run( + validate_persisted_copilot_scope_access( + scope.session_metadata(), + _user(merchant_ids=["merchant-1"]), + template_merchant_loader=_template_merchant_loader(_templates()), + merchant_scope_resolver=_merchant_scope(["merchant-1"]), + ) + ) + + +def test_persisted_scope_access_noops_for_ordinary_chat_metadata(): + asyncio.run( + validate_persisted_copilot_scope_access( + {"template_vars": {}}, + _user(merchant_ids=[]), + template_merchant_loader=_template_merchant_loader({}), + merchant_scope_resolver=_merchant_scope([]), + ) + ) + + +def test_persisted_scope_access_rejects_unauthorized_data_merchant(): + with pytest.raises(CopilotScopeError) as exc: + asyncio.run( + validate_persisted_copilot_scope_access( + {"copilot": {"data": {"data_merchant_id": "merchant-2"}}}, + _user(merchant_ids=["merchant-1"]), + template_merchant_loader=_template_merchant_loader({}), + merchant_scope_resolver=_merchant_scope(["merchant-1"]), + ) + ) + + assert exc.value.code == "unauthorized_merchant" + assert exc.value.status_code == 404 + + +def test_persisted_scope_access_rejects_stale_data_template_mapping(): + with pytest.raises(CopilotScopeError) as exc: + asyncio.run( + validate_persisted_copilot_scope_access( + { + "copilot": { + "data": { + "data_merchant_id": "merchant-1", + "data_template_id": DATA_TEMPLATE_ID, + } + } + }, + _user(merchant_ids=["merchant-1"]), + template_merchant_loader=_template_merchant_loader( + _templates(data_merchant="merchant-2") + ), + merchant_scope_resolver=_merchant_scope(["merchant-1"]), + ) + ) + + assert exc.value.code == "unauthorized_template" + assert exc.value.status_code == 404 + + +def test_persisted_scope_access_rejects_malformed_copilot_metadata(): + with pytest.raises(CopilotScopeError) as exc: + asyncio.run( + validate_persisted_copilot_scope_access( + {"copilot": {"data": {"data_template_id": DATA_TEMPLATE_ID}}}, + _user(merchant_ids=["merchant-1"]), + template_merchant_loader=_template_merchant_loader({}), + merchant_scope_resolver=_merchant_scope(["merchant-1"]), + ) + ) + + assert exc.value.code == "invalid_persisted_scope" + assert exc.value.status_code == 404 diff --git a/tests/test_copilot_session.py b/tests/test_copilot_session.py new file mode 100644 index 000000000..572914ef6 --- /dev/null +++ b/tests/test_copilot_session.py @@ -0,0 +1,394 @@ +"""Tests for Buddy Copilot scope injection into normal chat sessions.""" + +from __future__ import annotations + +import asyncio +from typing import Any, cast + +import pytest +from fastapi import HTTPException +from pydantic import ValidationError + +import app.api.routers.breeze_buddy.chat as chat_router +from app.ai.voice.agents.breeze_buddy.template.types import TemplateModel +from app.api.routers.breeze_buddy.chat import handlers as chat_handlers +from app.api.routers.breeze_buddy.chat.rbac import validate_chat_session_access +from app.schemas import UserInfo, UserRole +from app.schemas.breeze_buddy.chat import ( + ChatSession, + ChatSessionStatus, + CreateChatSessionRequest, + SendChatMessageRequest, +) +from app.schemas.breeze_buddy.copilot import CopilotScopeRequest +from app.services.breeze_buddy.copilot import scope as scope_service +from app.services.breeze_buddy.copilot.scope import CopilotScopeError + +DATA_TEMPLATE_ID = "11111111-1111-4111-8111-111111111111" + + +def _user( + *, + user_id: str = "user-1", + role: UserRole = UserRole.MERCHANT, + reseller_ids: list[str] | None = None, + merchant_ids: list[str] | None = None, + permissions: list[str] | None = None, +) -> UserInfo: + return UserInfo( + id=user_id, + username=user_id, + role=role, + reseller_ids=reseller_ids or ["dashboard-reseller"], + merchant_ids=merchant_ids or ["data-merchant"], + permissions=permissions or ["analytics:own"], + ) + + +def _dashboard_template() -> TemplateModel: + return TemplateModel( + id="dashboard-template", + reseller_id="dashboard-reseller", + merchant_id="dashboard-runtime", + name="Dashboard Assist", + flow={"nodes": []}, + supported_channels=["chat"], + ) + + +def _capture_session_persist(monkeypatch) -> dict[str, Any]: + captured: dict[str, Any] = {} + + async def persist_session(*, metadata, **kwargs): + captured["metadata"] = metadata + captured["persist_kwargs"] = kwargs + return ChatSession( + id="session-1", + template_id=kwargs["template_id"], + reseller_id=kwargs["reseller_id"], + merchant_id=kwargs["merchant_id"], + metadata=metadata, + ) + + async def render_vars(_template, _persisted): + return {} + + monkeypatch.setattr(chat_handlers, "create_chat_session", persist_session) + monkeypatch.setattr(chat_handlers, "build_render_template_vars", render_vars) + return captured + + +def test_normal_chat_session_creation_is_unchanged(monkeypatch): + template = _dashboard_template() + captured = _capture_session_persist(monkeypatch) + + response = asyncio.run( + chat_handlers.create_chat_session_handler( + CreateChatSessionRequest( + template_id=template.id, + template_vars={"customer_name": "Asha"}, + metadata={"safe": True}, + ), + template, + _user(), + ) + ) + + assert response.session_id == "session-1" + assert response.status == ChatSessionStatus.ACTIVE + assert response.greeting is None + assert captured["metadata"] == { + "safe": True, + "template_vars": {"customer_name": "Asha"}, + } + assert captured["persist_kwargs"] == { + "template_id": "dashboard-template", + "reseller_id": "dashboard-reseller", + "merchant_id": "dashboard-runtime", + } + + +def test_copilot_scope_is_resolved_and_persisted_on_normal_chat_session( + monkeypatch, +): + template = _dashboard_template() + captured = _capture_session_persist(monkeypatch) + + response = asyncio.run( + chat_handlers.create_chat_session_handler( + CreateChatSessionRequest( + template_id=template.id, + metadata={"source": "dashboard"}, + copilot_scope=CopilotScopeRequest( + data_merchant_id="data-merchant", + ), + ), + template, + _user(), + ) + ) + + assert response.session_id == "session-1" + metadata = cast(dict[str, Any], captured["metadata"]) + copilot_metadata = cast(dict[str, Any], metadata["copilot"]) + copilot_data = cast(dict[str, Any], copilot_metadata["data"]) + copilot_actor = cast(dict[str, Any], copilot_metadata["actor"]) + persist_kwargs = cast(dict[str, Any], captured["persist_kwargs"]) + + assert metadata["source"] == "dashboard" + assert metadata["template_vars"] == {} + assert copilot_data == { + "data_merchant_id": "data-merchant", + "data_template_id": None, + } + assert copilot_actor == {"user_id": "user-1"} + assert "runtime_merchant_id" not in copilot_metadata + assert "runtime_template_id" not in copilot_metadata + assert persist_kwargs["merchant_id"] == "dashboard-runtime" + + +def test_copilot_scope_does_not_require_analytics_permission(monkeypatch): + template = _dashboard_template() + captured = _capture_session_persist(monkeypatch) + + asyncio.run( + chat_handlers.create_chat_session_handler( + CreateChatSessionRequest( + template_id=template.id, + copilot_scope=CopilotScopeRequest( + data_merchant_id="data-merchant", + ), + ), + template, + _user(permissions=["read:own_data"]), + ) + ) + + metadata = cast(dict[str, Any], captured["metadata"]) + copilot_metadata = cast(dict[str, Any], metadata["copilot"]) + copilot_data = cast(dict[str, Any], copilot_metadata["data"]) + assert copilot_data["data_merchant_id"] == "data-merchant" + + +@pytest.mark.parametrize( + ("metadata_key", "expected_detail"), + [ + ("copilot", "metadata.copilot is server-owned"), + ("template_vars", "metadata.template_vars is server-owned"), + ], +) +def test_client_cannot_set_server_owned_metadata( + monkeypatch, + metadata_key, + expected_detail, +): + template = _dashboard_template() + captured = _capture_session_persist(monkeypatch) + + with pytest.raises(HTTPException) as exc: + asyncio.run( + chat_handlers.create_chat_session_handler( + CreateChatSessionRequest( + template_id=template.id, + metadata={metadata_key: {"forged": True}}, + copilot_scope=CopilotScopeRequest( + data_merchant_id="data-merchant", + ), + ), + template, + _user(), + ) + ) + + assert exc.value.status_code == 422 + assert exc.value.detail == expected_detail + assert "metadata" not in captured + + +@pytest.mark.parametrize( + ("scope_request", "user", "expected_status", "expected_code"), + [ + ( + CopilotScopeRequest(data_merchant_id="other-merchant"), + _user(), + 403, + "unauthorized_merchant", + ), + ( + CopilotScopeRequest(), + _user(role=UserRole.ADMIN, merchant_ids=["*"]), + 400, + "ambiguous_merchant", + ), + ], +) +def test_rejects_invalid_copilot_scope( + monkeypatch, + scope_request, + user, + expected_status, + expected_code, +): + template = _dashboard_template() + _capture_session_persist(monkeypatch) + + with pytest.raises(HTTPException) as exc: + asyncio.run( + chat_handlers.create_chat_session_handler( + CreateChatSessionRequest( + template_id=template.id, + copilot_scope=scope_request, + ), + template, + user, + ) + ) + + assert exc.value.status_code == expected_status + detail = exc.value.detail + assert isinstance(detail, dict) + assert detail["code"] == expected_code + + +def test_rejects_data_template_from_another_merchant(monkeypatch): + template = _dashboard_template() + _capture_session_persist(monkeypatch) + + async def load_template_merchant(_template_id: str): + return "other-merchant" + + monkeypatch.setattr( + scope_service, + "get_template_merchant_id", + load_template_merchant, + ) + + with pytest.raises(HTTPException) as exc: + asyncio.run( + chat_handlers.create_chat_session_handler( + CreateChatSessionRequest( + template_id=template.id, + copilot_scope=CopilotScopeRequest( + data_merchant_id="data-merchant", + data_template_id=DATA_TEMPLATE_ID, + ), + ), + template, + _user(), + ) + ) + + assert exc.value.status_code == 403 + detail = exc.value.detail + assert isinstance(detail, dict) + assert detail["code"] == "unauthorized_template" + + +def test_copilot_scope_request_cannot_override_assist_template(): + with pytest.raises(ValidationError): + CopilotScopeRequest.model_validate( + { + "data_merchant_id": "data-merchant", + "template_id": "attacker-selected-template", + } + ) + + +def test_ordinary_chat_session_rbac_is_unchanged(): + session = ChatSession( + id="session-1", + template_id="merchant-template", + reseller_id="dashboard-reseller", + merchant_id="dashboard-runtime", + ) + + validate_chat_session_access( + _user( + user_id="another-user", + reseller_ids=["dashboard-reseller"], + merchant_ids=["dashboard-runtime"], + ), + session, + operation="send_message", + ) + + +def test_route_access_check_hides_copilot_scope_revalidation_failures(monkeypatch): + session = ChatSession( + id="session-1", + template_id="dashboard-template", + reseller_id="dashboard-reseller", + merchant_id="dashboard-runtime", + metadata={"copilot": {"data": {"data_merchant_id": "data-merchant"}}}, + ) + + async def deny_copilot_scope(_metadata, _current_user): + raise CopilotScopeError( + "unauthorized_merchant", + "Access denied to the stored Copilot data merchant.", + status_code=404, + ) + + monkeypatch.setattr( + chat_router, + "validate_persisted_copilot_scope_access", + deny_copilot_scope, + ) + + with pytest.raises(HTTPException) as exc: + asyncio.run( + chat_router._validate_chat_and_copilot_session_access( + _user( + reseller_ids=["dashboard-reseller"], + merchant_ids=["dashboard-runtime"], + ), + session, + operation="get_session", + ) + ) + + assert exc.value.status_code == 404 + assert exc.value.detail == "Chat session not found" + + +def test_send_message_handler_awaits_async_access_check(monkeypatch): + class FakeLock: + def __init__(self, *_args, **_kwargs): + self.released = False + + async def acquire(self): + return None + + async def release(self): + self.released = True + + session = ChatSession( + id="session-1", + template_id="dashboard-template", + reseller_id="dashboard-reseller", + merchant_id="dashboard-runtime", + ) + called: dict[str, bool] = {} + + async def load_session(_session_id: str): + return session + + async def deny(_session: ChatSession): + called["awaited"] = True + raise HTTPException(status_code=404, detail="Chat session not found") + + monkeypatch.setattr(chat_handlers, "RedisLock", FakeLock) + monkeypatch.setattr(chat_handlers, "get_chat_session_by_id", load_session) + + with pytest.raises(HTTPException) as exc: + asyncio.run( + chat_handlers.send_chat_message_handler( + "session-1", + SendChatMessageRequest(content="hello"), + access_check=deny, + ) + ) + + assert called["awaited"] is True + assert exc.value.status_code == 404 + assert exc.value.detail == "Chat session not found"