From 96ba0a04e8d3ebc6df1ef316952d575045f03200 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 12:50:49 -0400 Subject: [PATCH 1/5] Fix shared conversation AI access and add file approvals A collaborative conversation is backed by a hidden source conversation whose user_id is always the shared conversation creator. The collaboration stream bridge proxies into chat_stream_api using that source conversation while keeping the requesting participant's session, so every owner-equality check failed for participants. The visible symptom was "Stream interrupted: Forbidden" with no content whenever an invited user invoked the AI. Four gates were involved: - chat_stream_api returned 403 Forbidden, blocking all AI invocation by participants. This only surfaced on explicit AI requests because shared conversations default to ai_invocation_mode 'explicit_only'. - The generated artifact write raised PermissionError, which maybe_create_generated_file_output swallowed, so the file silently vanished. - Group workspace document saves required Owner/Admin/DocumentManager. - Artifact downloads were owner-only, so participants could not retrieve files even once they existed. Authorization is now resolved through a single collaboration-aware helper, build_conversation_participation_context, mirroring the pattern already used by chat file uploads in _resolve_chat_upload_context. Personal conversations with no collaboration link remain strictly owner-only. On top of that, downloadable files generated by a non-owner participant are now staged for approval instead of failing. The artifact is written immediately in a pending_approval state and released by an approver, so the model never re-runs. The conversation owner approves in personal shared conversations; any group Owner, Admin, or DocumentManager approves in group ones. A requester is never their own approver. Denial deletes the stored blob and records the decision, and anything left unapproved auto-denies after three days on the existing approval expiry sweep. Only downloadable deliverables are gated; generated images and charts stay ungated. Behavior is controlled by a new admin setting that defaults to enabled. Also fixed while in this path, both of which affect the reported case because 900 rows exceeds the 500-row inline export threshold: - assert_generated_chat_artifact_is_published_for_user read the export run using the caller's id as the partition key, so an approved large CSV queued by a participant would have been unreadable by the owner. The run owner is now recorded on the artifact. - commit_generated_chat_artifact_publication_for_user and delete_generated_chat_artifact_for_user carried the same owner-only check and would have broken publication and rollback for participant-queued exports. Group workspace writes are deliberately not staged. They feed the group search index, so withholding them would mean withholding indexing. That path now returns an actionable message naming the roles that can complete the request. Version bumped to 0.260.004. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/background_tasks.py | 13 + .../single_app/collaboration_models.py | 1 + application/single_app/config.py | 2 +- .../single_app/functions_collaboration.py | 90 ++- .../functions_generated_file_approvals.py | 334 +++++++++ .../functions_generated_file_exports.py | 11 + .../single_app/functions_notifications.py | 12 + application/single_app/functions_settings.py | 2 + .../functions_simplechat_operations.py | 524 ++++++++++++- application/single_app/route_backend_chats.py | 20 +- .../single_app/route_backend_collaboration.py | 76 ++ .../single_app/route_enhanced_citations.py | 24 +- .../route_frontend_admin_settings.py | 1 + .../static/js/chat/chat-file-approvals.js | 184 +++++ .../static/js/chat/chat-messages.js | 25 + .../single_app/templates/admin_settings.html | 21 + .../SHARED_CONVERSATION_FILE_APPROVALS.md | 168 +++++ docs/explanation/features/index.md | 4 + ...VERSATION_FILE_GENERATION_FORBIDDEN_FIX.md | 158 ++++ docs/explanation/fixes/index.md | 1 + docs/explanation/release_notes.md | 30 + ...erated_artifact_lifecycle_authorization.py | 21 +- ...t_shared_conversation_file_approval_fix.py | 688 ++++++++++++++++++ .../test_tabular_row_orchestration_scale.py | 17 + 24 files changed, 2399 insertions(+), 28 deletions(-) create mode 100644 application/single_app/functions_generated_file_approvals.py create mode 100644 application/single_app/static/js/chat/chat-file-approvals.js create mode 100644 docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md create mode 100644 docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md create mode 100644 functional_tests/test_shared_conversation_file_approval_fix.py diff --git a/application/single_app/background_tasks.py b/application/single_app/background_tasks.py index 4d72fde48..b6cc370b1 100644 --- a/application/single_app/background_tasks.py +++ b/application/single_app/background_tasks.py @@ -245,6 +245,7 @@ def check_logging_timers_once(): def check_expired_approvals_once(): """Auto-deny expired approval requests and return the affected count.""" from functions_approvals import auto_deny_expired_approvals + from functions_simplechat_operations import auto_deny_expired_generated_file_approvals lock_document = acquire_distributed_task_lock('approval_expiry', lease_seconds=1800) if not lock_document: @@ -255,6 +256,18 @@ def check_expired_approvals_once(): denied_count = auto_deny_expired_approvals() if denied_count > 0: print(f"Auto-denied {denied_count} expired approval request(s).") + + try: + expired_file_count = auto_deny_expired_generated_file_approvals() + if expired_file_count > 0: + print(f"Auto-denied {expired_file_count} expired generated file approval(s).") + except Exception as exc: + # Staged file expiry must never take down the Control Center approval sweep. + print(f"Error expiring staged generated file approvals: {exc}") + log_event( + f"Error expiring staged generated file approvals: {exc}", + level=logging.ERROR, + ) finally: release_distributed_task_lock(lock_document) diff --git a/application/single_app/collaboration_models.py b/application/single_app/collaboration_models.py index e2f322ea2..60dbd875a 100644 --- a/application/single_app/collaboration_models.py +++ b/application/single_app/collaboration_models.py @@ -7,6 +7,7 @@ COLLABORATION_KIND = 'collaborative' +COLLABORATION_SOURCE_KIND = 'collaboration_source' PERSONAL_MULTI_USER_CHAT_TYPE = 'personal_multi_user' GROUP_MULTI_USER_CHAT_TYPE = 'group_multi_user' diff --git a/application/single_app/config.py b/application/single_app/config.py index e3f9a3646..7a4b93be9 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -96,7 +96,7 @@ EXECUTOR_TYPE = 'thread' EXECUTOR_MAX_WORKERS = 30 SESSION_TYPE = 'filesystem' -VERSION = "0.260.003" +VERSION = "0.260.004" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_collaboration.py b/application/single_app/functions_collaboration.py index 75e9e78c0..f0ec2325b 100644 --- a/application/single_app/functions_collaboration.py +++ b/application/single_app/functions_collaboration.py @@ -8,6 +8,7 @@ from config import * from collaboration_models import ( COLLABORATION_KIND, + COLLABORATION_SOURCE_KIND, GROUP_MULTI_USER_CHAT_TYPE, MEMBERSHIP_ROLE_ADMIN, MEMBERSHIP_ROLE_MEMBER, @@ -1568,6 +1569,91 @@ def assert_user_can_participate_in_collaboration_conversation(user_id, conversat return access_context +def is_collaboration_source_conversation(conversation_item): + """Return True when a personal conversation is the hidden backing store of a shared one.""" + normalized_item = conversation_item or {} + return bool( + str(normalized_item.get('conversation_kind') or '').strip() == COLLABORATION_SOURCE_KIND + or str(normalized_item.get('collaboration_conversation_id') or '').strip() + ) + + +def get_collaboration_conversation_for_source(conversation_item): + """Load the shared conversation that owns a collaboration source conversation.""" + collaboration_conversation_id = str( + (conversation_item or {}).get('collaboration_conversation_id') or '' + ).strip() + if not collaboration_conversation_id: + return None + + try: + return get_collaboration_conversation(collaboration_conversation_id) + except CosmosResourceNotFoundError: + return None + + +def build_conversation_participation_context(user_id, conversation_item): + """Authorize one caller against a personal conversation or its linked shared conversation. + + Ordinary personal conversations stay owner-only. Collaboration source conversations are + always owned by the shared conversation creator, so every other participant fails a plain + ownership comparison even though they are legitimate members. Those callers are authorized + against the linked collaboration conversation instead, mirroring the chat upload path in + ``route_frontend_chats._resolve_chat_upload_context``. + + Returns a context describing how access was granted so callers can distinguish an owner + acting on their own conversation from a participant acting inside a shared one. + """ + normalized_user_id = str(user_id or '').strip() + normalized_item = conversation_item or {} + owner_user_id = str(normalized_item.get('user_id') or '').strip() + conversation_id = str(normalized_item.get('id') or '').strip() + + if normalized_user_id and owner_user_id == normalized_user_id: + return { + 'user_id': normalized_user_id, + 'conversation_id': conversation_id, + 'owner_user_id': owner_user_id, + 'is_owner': True, + 'is_collaboration_source': is_collaboration_source_conversation(normalized_item), + 'collaboration_conversation': None, + 'collaboration_conversation_id': str( + normalized_item.get('collaboration_conversation_id') or '' + ).strip(), + 'collaboration_access': None, + 'group_id': '', + 'group_role': '', + } + + collaboration_conversation = get_collaboration_conversation_for_source(normalized_item) + if not collaboration_conversation: + raise PermissionError('You can only access your own conversations') + + collaboration_access = assert_user_can_participate_in_collaboration_conversation( + normalized_user_id, + collaboration_conversation, + ) + + group_id = '' + if is_group_collaboration_conversation(collaboration_conversation): + group_id = str( + (collaboration_conversation.get('scope') or {}).get('group_id') or '' + ).strip() + + return { + 'user_id': normalized_user_id, + 'conversation_id': conversation_id, + 'owner_user_id': owner_user_id, + 'is_owner': False, + 'is_collaboration_source': True, + 'collaboration_conversation': collaboration_conversation, + 'collaboration_conversation_id': str(collaboration_conversation.get('id') or '').strip(), + 'collaboration_access': collaboration_access, + 'group_id': group_id, + 'group_role': str((collaboration_access or {}).get('group_role') or '').strip(), + } + + def record_personal_invite_response(conversation_id, user_id, action): conversation_doc = get_collaboration_conversation(conversation_id) if not is_explicit_membership_collaboration(conversation_doc): @@ -1930,7 +2016,7 @@ def ensure_collaboration_source_conversation(conversation_doc, current_user): 'locked_contexts': list((conversation_doc or {}).get('locked_contexts', []) or []), 'classification': list((conversation_doc or {}).get('classification', []) or []), 'summary': (conversation_doc or {}).get('summary'), - 'conversation_kind': 'collaboration_source', + 'conversation_kind': COLLABORATION_SOURCE_KIND, 'collaboration_conversation_id': (conversation_doc or {}).get('id'), 'is_hidden': True, } @@ -1950,7 +2036,7 @@ def ensure_collaboration_source_conversation(conversation_doc, current_user): 'locked_contexts': list((conversation_doc or {}).get('locked_contexts', []) or source_conversation_doc.get('locked_contexts', []) or []), 'classification': list((conversation_doc or {}).get('classification', []) or source_conversation_doc.get('classification', []) or []), 'summary': (conversation_doc or {}).get('summary', source_conversation_doc.get('summary')), - 'conversation_kind': 'collaboration_source', + 'conversation_kind': COLLABORATION_SOURCE_KIND, 'collaboration_conversation_id': (conversation_doc or {}).get('id'), 'is_hidden': True, } diff --git a/application/single_app/functions_generated_file_approvals.py b/application/single_app/functions_generated_file_approvals.py new file mode 100644 index 000000000..f84fd7673 --- /dev/null +++ b/application/single_app/functions_generated_file_approvals.py @@ -0,0 +1,334 @@ +# functions_generated_file_approvals.py + +"""Owner approval gate for files generated by participants in shared conversations. + +Shared conversations are backed by a hidden source conversation owned by the shared +conversation creator, so any generated file a participant asks for is written into the +owner's storage scope. Rather than failing those requests outright, the artifact is written +immediately in a ``pending_approval`` state and only becomes readable once an authorized +approver releases it. Approval is therefore a cheap state flip and the model never re-runs. + +Approval state lives on the artifact message metadata so it travels with the artifact and +cannot be bypassed by reaching the download route directly. Approver discovery is handled by +notifications, which avoids a second index that could drift out of sync with the artifact. +""" + +import logging +import os +from datetime import datetime, timedelta, timezone +from typing import Any, Dict, List, Optional + +from config import cosmos_messages_container +from functions_appinsights import log_event +from functions_group import find_group_by_id, get_user_role_in_group +from functions_settings import get_settings + + +APPROVAL_STATE_PENDING = "pending_approval" +APPROVAL_STATE_APPROVED = "approved" +APPROVAL_STATE_DENIED = "denied" +APPROVAL_STATE_AUTO_DENIED = "auto_denied" + +APPROVAL_TERMINAL_STATES = frozenset({ + APPROVAL_STATE_APPROVED, + APPROVAL_STATE_DENIED, + APPROVAL_STATE_AUTO_DENIED, +}) + +APPROVAL_SCOPE_PERSONAL = "personal" +APPROVAL_SCOPE_GROUP = "group" + +# Mirrors functions_approvals.TTL_AUTO_DENY_DAYS so both approval surfaces expire alike. +APPROVAL_TTL_DAYS = 3 +APPROVAL_TTL_SECONDS = APPROVAL_TTL_DAYS * 24 * 60 * 60 + +APPROVAL_SETTING_KEY = "require_shared_conversation_file_approval" + +# Downloadable deliverables only. Generated images and charts are inline conversation +# rendering, and gating those would make ordinary chat look broken. +APPROVAL_GATED_FILE_EXTENSIONS = frozenset({ + "csv", + "xlsx", + "xls", + "xlsm", + "docx", + "pdf", + "json", + "xml", +}) + +GROUP_DOCUMENT_APPROVER_ROLES = ("Owner", "Admin", "DocumentManager") + +GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE = "generated_file_approval_pending" + + +def _utc_now() -> datetime: + return datetime.now(timezone.utc) + + +def _utc_now_iso() -> str: + return _utc_now().isoformat() + + +def _clean(value: Any) -> str: + return str(value or "").strip() + + +def _metadata_of(message_item: Optional[Dict[str, Any]]) -> Dict[str, Any]: + metadata = (message_item or {}).get("metadata") + return metadata if isinstance(metadata, dict) else {} + + +def is_generated_file_approval_enabled(settings: Optional[Dict[str, Any]] = None) -> bool: + """Return whether the approval gate is switched on for this tenant.""" + resolved_settings = settings if isinstance(settings, dict) else (get_settings() or {}) + return bool(resolved_settings.get(APPROVAL_SETTING_KEY, True)) + + +def normalize_approval_file_extension(file_name: str = "", output_format: str = "") -> str: + """Return the lowercase extension used to decide whether a write is gated.""" + normalized_format = _clean(output_format).lower().lstrip(".") + if normalized_format: + return normalized_format + return os.path.splitext(_clean(file_name))[1].lower().lstrip(".") + + +def is_approval_gated_file(file_name: str = "", output_format: str = "") -> bool: + """Return whether this artifact format is a downloadable deliverable needing approval.""" + return normalize_approval_file_extension(file_name, output_format) in APPROVAL_GATED_FILE_EXTENSIONS + + +def requires_generated_file_approval( + access_context: Optional[Dict[str, Any]], + file_name: str = "", + output_format: str = "", + settings: Optional[Dict[str, Any]] = None, +) -> bool: + """Return whether one artifact write must be staged for approval. + + Owners writing into their own conversation are never gated. Only a non-owner participant + of a shared conversation producing a downloadable deliverable triggers the gate. + """ + context = access_context if isinstance(access_context, dict) else {} + if not context or context.get("is_owner", True): + return False + if not context.get("collaboration_conversation_id"): + return False + if not is_approval_gated_file(file_name, output_format): + return False + return is_generated_file_approval_enabled(settings) + + +def build_generated_file_approval_metadata( + access_context: Dict[str, Any], + requester: Optional[Dict[str, Any]] = None, + requested_at: str = "", +) -> Dict[str, Any]: + """Build the pending-approval metadata fragment stored on the artifact message.""" + context = access_context if isinstance(access_context, dict) else {} + requester_info = requester if isinstance(requester, dict) else {} + normalized_requested_at = _clean(requested_at) or _utc_now_iso() + expires_at = ( + _utc_now() + timedelta(days=APPROVAL_TTL_DAYS) + ).isoformat() + + group_id = _clean(context.get("group_id")) + approval_scope = APPROVAL_SCOPE_GROUP if group_id else APPROVAL_SCOPE_PERSONAL + + return { + "generated_artifact_approval_required": True, + "generated_artifact_approval_state": APPROVAL_STATE_PENDING, + "generated_artifact_approval_scope": approval_scope, + "generated_artifact_approval_group_id": group_id, + "generated_artifact_approval_owner_user_id": _clean(context.get("owner_user_id")), + "generated_artifact_approval_collaboration_conversation_id": _clean( + context.get("collaboration_conversation_id") + ), + "generated_artifact_approval_requested_by_id": _clean( + requester_info.get("user_id") or requester_info.get("userId") or context.get("user_id") + ), + "generated_artifact_approval_requested_by_name": _clean( + requester_info.get("display_name") or requester_info.get("displayName") + ), + "generated_artifact_approval_requested_by_email": _clean(requester_info.get("email")), + "generated_artifact_approval_requested_at": normalized_requested_at, + "generated_artifact_approval_expires_at": expires_at, + "generated_artifact_approval_resolved_by_id": None, + "generated_artifact_approval_resolved_by_name": None, + "generated_artifact_approval_resolved_at": None, + } + + +def get_generated_file_approval_state(message_item: Optional[Dict[str, Any]]) -> str: + """Return the approval state recorded on an artifact message, or an empty string.""" + metadata = _metadata_of(message_item) + if not metadata.get("generated_artifact_approval_required"): + return "" + return _clean(metadata.get("generated_artifact_approval_state")).lower() + + +def generated_file_approval_is_pending(message_item: Optional[Dict[str, Any]]) -> bool: + """Return whether an artifact is still waiting on an approval decision.""" + return get_generated_file_approval_state(message_item) == APPROVAL_STATE_PENDING + + +def assert_generated_file_approval_allows_download( + user_id: str, + message_item: Optional[Dict[str, Any]], +) -> None: + """Block artifact content access until an approver has released the file. + + Enforced independently of the tabular export manifest checks so a staged artifact cannot + be reached by any caller, including the participant who requested it. + """ + approval_state = get_generated_file_approval_state(message_item) + if not approval_state or approval_state == APPROVAL_STATE_APPROVED: + return + + if approval_state == APPROVAL_STATE_PENDING: + raise PermissionError("This file is waiting for owner approval") + raise PermissionError("This file was not approved") + + +def resolve_generated_file_approver_role( + user_id: str, + message_item: Optional[Dict[str, Any]], +) -> str: + """Return how a user qualifies to approve an artifact, or an empty string if they cannot.""" + normalized_user_id = _clean(user_id) + if not normalized_user_id: + return "" + + metadata = _metadata_of(message_item) + if not metadata.get("generated_artifact_approval_required"): + return "" + + # A requester is never their own approver, whatever else they hold. Without this a group + # Admin or DocumentManager who is only a participant could stage a file and release it + # themselves, which would make the gate meaningless for them. + if _clean(metadata.get("generated_artifact_approval_requested_by_id")) == normalized_user_id: + return "" + + approval_scope = _clean(metadata.get("generated_artifact_approval_scope")).lower() + if approval_scope == APPROVAL_SCOPE_GROUP: + group_id = _clean(metadata.get("generated_artifact_approval_group_id")) + if not group_id: + return "" + group_doc = find_group_by_id(group_id) + if not group_doc: + return "" + group_role = _clean(get_user_role_in_group(group_doc, normalized_user_id)) + allowed_roles = {role.lower() for role in GROUP_DOCUMENT_APPROVER_ROLES} + if group_role.lower() in allowed_roles: + return group_role + return "" + + if _clean(metadata.get("generated_artifact_approval_owner_user_id")) == normalized_user_id: + return "ConversationOwner" + return "" + + +def user_can_approve_generated_file(user_id: str, message_item: Optional[Dict[str, Any]]) -> bool: + """Return whether a user may approve or deny one staged artifact.""" + return bool(resolve_generated_file_approver_role(user_id, message_item)) + + +def apply_generated_file_approval_decision( + message_item: Dict[str, Any], + decision: str, + resolver: Optional[Dict[str, Any]] = None, +) -> Dict[str, Any]: + """Record an approval decision on the artifact message and return the updated document. + + Approving also releases the underlying artifact lifecycle so the existing publication + checks treat the file as readable. + """ + normalized_decision = _clean(decision).lower() + if normalized_decision not in APPROVAL_TERMINAL_STATES: + raise ValueError(f"Unsupported generated file approval decision: {decision}") + + resolver_info = resolver if isinstance(resolver, dict) else {} + metadata = _metadata_of(message_item) + if not metadata.get("generated_artifact_approval_required"): + raise ValueError("Artifact is not gated by an approval") + + current_state = _clean(metadata.get("generated_artifact_approval_state")).lower() + if current_state in APPROVAL_TERMINAL_STATES: + raise ValueError("This file approval was already resolved") + + metadata.update({ + "generated_artifact_approval_state": normalized_decision, + "generated_artifact_approval_resolved_by_id": _clean( + resolver_info.get("user_id") or resolver_info.get("userId") + ) or None, + "generated_artifact_approval_resolved_by_name": _clean( + resolver_info.get("display_name") or resolver_info.get("displayName") + ) or None, + "generated_artifact_approval_resolved_at": _utc_now_iso(), + }) + message_item["metadata"] = metadata + return message_item + + +def build_generated_file_approval_client_payload( + message_item: Optional[Dict[str, Any]], + viewer_user_id: str = "", +) -> Dict[str, Any]: + """Build the approval descriptor the chat UI renders on a gated artifact.""" + approval_state = get_generated_file_approval_state(message_item) + if not approval_state: + return {} + + metadata = _metadata_of(message_item) + normalized_viewer_id = _clean(viewer_user_id) + requested_by_id = _clean(metadata.get("generated_artifact_approval_requested_by_id")) + + return { + "state": approval_state, + "is_pending": approval_state == APPROVAL_STATE_PENDING, + "scope": _clean(metadata.get("generated_artifact_approval_scope")), + "requested_by_id": requested_by_id, + "requested_by_name": _clean(metadata.get("generated_artifact_approval_requested_by_name")), + "requested_at": _clean(metadata.get("generated_artifact_approval_requested_at")), + "expires_at": _clean(metadata.get("generated_artifact_approval_expires_at")), + "resolved_by_name": _clean(metadata.get("generated_artifact_approval_resolved_by_name")), + "resolved_at": _clean(metadata.get("generated_artifact_approval_resolved_at")), + "viewer_is_requester": bool( + normalized_viewer_id and normalized_viewer_id == requested_by_id + ), + "viewer_can_approve": bool( + approval_state == APPROVAL_STATE_PENDING + and user_can_approve_generated_file(normalized_viewer_id, message_item) + ), + } + + +def list_expired_pending_generated_file_artifacts(limit: int = 200) -> List[Dict[str, Any]]: + """Return staged artifacts whose approval window has elapsed.""" + normalized_limit = max(1, min(int(limit or 200), 1000)) + query = ( + "SELECT TOP @limit * FROM c " + "WHERE c.role = 'file' " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_state) " + "AND c.metadata.generated_artifact_approval_state = @pending_state " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_expires_at) " + "AND c.metadata.generated_artifact_approval_expires_at < @now" + ) + try: + return list(cosmos_messages_container.query_items( + query=query, + parameters=[ + {"name": "@limit", "value": normalized_limit}, + {"name": "@pending_state", "value": APPROVAL_STATE_PENDING}, + {"name": "@now", "value": _utc_now_iso()}, + ], + enable_cross_partition_query=True, + )) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to query expired staged artifacts", + {"error": str(exc)}, + level=logging.ERROR, + exceptionTraceback=True, + ) + return [] diff --git a/application/single_app/functions_generated_file_exports.py b/application/single_app/functions_generated_file_exports.py index edd46aa24..c36df4ef5 100644 --- a/application/single_app/functions_generated_file_exports.py +++ b/application/single_app/functions_generated_file_exports.py @@ -613,6 +613,17 @@ def build_generated_file_artifact_metadata( row_source = str(export_payload.get('row_source') or '').strip() if row_source: artifact_metadata['row_source'] = row_source + + # Surfaced immediately so the participant who asked for the file sees the pending state in + # the same response instead of a download button that would be refused. + approval_state = str(uploaded_message.get('approval_state') or '').strip() + if approval_state: + artifact_metadata['approval'] = { + 'state': approval_state, + 'is_pending': approval_state == 'pending_approval', + 'viewer_is_requester': True, + 'viewer_can_approve': False, + } return artifact_metadata diff --git a/application/single_app/functions_notifications.py b/application/single_app/functions_notifications.py index d8865fd43..d0f608d3a 100644 --- a/application/single_app/functions_notifications.py +++ b/application/single_app/functions_notifications.py @@ -174,6 +174,18 @@ 'icon': 'bi-trash', 'color': 'secondary' }, + 'generated_file_approval_pending': { + 'icon': 'bi-file-earmark-lock', + 'color': 'warning' + }, + 'generated_file_approval_approved': { + 'icon': 'bi-file-earmark-check', + 'color': 'success' + }, + 'generated_file_approval_denied': { + 'icon': 'bi-file-earmark-x', + 'color': 'danger' + }, WORKFLOW_ALERT_NOTIFICATION_TYPE: { 'icon': 'bi-bell', 'color': 'secondary' diff --git a/application/single_app/functions_settings.py b/application/single_app/functions_settings.py index 0f542118e..73ad6c8e5 100644 --- a/application/single_app/functions_settings.py +++ b/application/single_app/functions_settings.py @@ -1694,6 +1694,8 @@ def get_settings(use_cosmos=False, include_source=False): # Collaborative Conversations 'enable_collaborative_conversations': True, + # Stage files generated by non-owner participants until an approver releases them. + 'require_shared_conversation_file_approval': True, # Search and Extract 'azure_ai_search_endpoint': '', diff --git a/application/single_app/functions_simplechat_operations.py b/application/single_app/functions_simplechat_operations.py index 2aff415c3..567877176 100644 --- a/application/single_app/functions_simplechat_operations.py +++ b/application/single_app/functions_simplechat_operations.py @@ -43,6 +43,7 @@ ) from functions_collaboration import ( assert_user_can_participate_in_collaboration_conversation, + build_conversation_participation_context, create_collaboration_message_notifications, create_group_collaboration_conversation_record, create_personal_collaboration_conversation_record, @@ -52,16 +53,30 @@ persist_collaboration_message, ) from functions_documents import allowed_file, create_document, process_document_upload_background, update_document +from functions_generated_file_approvals import ( + APPROVAL_STATE_APPROVED, + APPROVAL_STATE_AUTO_DENIED, + APPROVAL_STATE_DENIED, + APPROVAL_STATE_PENDING, + GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE, + apply_generated_file_approval_decision, + build_generated_file_approval_client_payload, + build_generated_file_approval_metadata, + list_expired_pending_generated_file_artifacts, + requires_generated_file_approval, + user_can_approve_generated_file, +) from functions_chat_bootstrap_cache import bump_chat_bootstrap_global_cache_version from functions_group import ( assert_group_role, check_group_status_allows_operation, create_group, find_group_by_id, + get_user_groups, get_user_role_in_group, require_active_group, ) -from functions_notifications import create_notification +from functions_notifications import create_notification, delete_notifications_by_metadata from functions_personal_workflows import save_personal_workflow from functions_public_workspaces import ( check_public_workspace_status_allows_operation, @@ -1329,8 +1344,9 @@ def delete_generated_chat_artifact_for_user( except CosmosResourceNotFoundError: return False - if str(conversation_item.get("user_id") or "").strip() != current_user_id: - raise PermissionError("Forbidden") + # Shared conversations are owned by their creator, so a participant rolling back their own + # generated artifact is authorized through the linked shared conversation instead. + build_conversation_participation_context(current_user_id, conversation_item) message_metadata = message_item.get("metadata") if isinstance(message_item.get("metadata"), dict) else {} if ( str(message_item.get("conversation_id") or "").strip() != normalized_conversation_id @@ -2553,6 +2569,433 @@ def _upload_generated_document_for_current_user( raise +def _get_current_user_summary_or_none(fallback_user_id: str = "") -> Dict[str, str]: + """Return a best-effort identity summary for the acting user. + + Background workers run outside a request context, so the caller-supplied id is used as the + fallback rather than failing the artifact write. + """ + normalized_fallback_id = str(fallback_user_id or "").strip() + try: + current_user = get_current_user_info() or {} + except Exception: + current_user = {} + + resolved_user_id = str(current_user.get("userId") or "").strip() or normalized_fallback_id + return { + "user_id": resolved_user_id, + "display_name": str(current_user.get("displayName") or "").strip(), + "email": str(current_user.get("email") or "").strip(), + } + + +def _resolve_generated_file_approver_ids(access_context: Dict[str, Any]) -> List[str]: + """Return the users allowed to release one staged artifact.""" + context = access_context if isinstance(access_context, dict) else {} + group_id = str(context.get("group_id") or "").strip() + + if not group_id: + owner_user_id = str(context.get("owner_user_id") or "").strip() + return [owner_user_id] if owner_user_id else [] + + group_doc = find_group_by_id(group_id) + if not group_doc: + return [] + + approver_ids = [] + owner_user_id = str((group_doc.get("owner") or {}).get("id") or "").strip() + if owner_user_id: + approver_ids.append(owner_user_id) + for candidate_id in list(group_doc.get("admins", []) or []) + list(group_doc.get("documentManagers", []) or []): + normalized_candidate_id = str(candidate_id or "").strip() + if normalized_candidate_id and normalized_candidate_id not in approver_ids: + approver_ids.append(normalized_candidate_id) + return approver_ids + + +def _notify_generated_file_approval_requested( + message_doc: Dict[str, Any], + access_context: Dict[str, Any], +) -> None: + """Notify every eligible approver that a participant staged a file for release.""" + metadata = message_doc.get("metadata") if isinstance(message_doc.get("metadata"), dict) else {} + collaboration_conversation_id = str( + metadata.get("generated_artifact_approval_collaboration_conversation_id") or "" + ).strip() + requester_name = str( + metadata.get("generated_artifact_approval_requested_by_name") or "" + ).strip() or "A participant" + file_name = str(message_doc.get("filename") or "").strip() or "a generated file" + + link_url = ( + f"/chats?conversationId={collaboration_conversation_id}" + if collaboration_conversation_id + else "" + ) + notification_metadata = { + "generated_artifact_message_id": str(message_doc.get("id") or "").strip(), + "source_conversation_id": str(message_doc.get("conversation_id") or "").strip(), + "collaboration_conversation_id": collaboration_conversation_id, + "approval_scope": str(metadata.get("generated_artifact_approval_scope") or "").strip(), + "group_id": str(metadata.get("generated_artifact_approval_group_id") or "").strip(), + } + + for approver_user_id in _resolve_generated_file_approver_ids(access_context): + _create_personal_notification( + user_id=approver_user_id, + notification_type=GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE, + title="File approval requested", + message=f"{requester_name} generated {file_name} in a shared conversation and needs your approval.", + link_url=link_url, + link_context={ + "conversation_id": collaboration_conversation_id, + "conversation_kind": "collaborative", + }, + metadata=notification_metadata, + ) + + +def list_pending_generated_file_approvals_for_user(user_id: str, limit: int = 50) -> List[Dict[str, Any]]: + """Return staged artifacts the supplied user is allowed to release. + + Candidates are narrowed in the query by the approval scopes this user could possibly + approve, so the row cap never truncates another user's items ahead of the caller's. + """ + normalized_user_id = str(user_id or "").strip() + if not normalized_user_id: + return [] + + normalized_limit = max(1, min(int(limit or 50), 200)) + approver_group_ids = [] + try: + for group_doc in get_user_groups(normalized_user_id) or []: + group_id = str((group_doc or {}).get("id") or "").strip() + if not group_id or group_id in approver_group_ids: + continue + if str(get_user_role_in_group(group_doc, normalized_user_id) or "").strip() in ( + "Owner", + "Admin", + "DocumentManager", + ): + approver_group_ids.append(group_id) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to resolve approver group scope", + {"error": str(exc)}, + level=logging.WARNING, + exceptionTraceback=True, + ) + + scope_clauses = ["c.metadata.generated_artifact_approval_owner_user_id = @user_id"] + parameters = [ + {"name": "@limit", "value": normalized_limit}, + {"name": "@pending_state", "value": APPROVAL_STATE_PENDING}, + {"name": "@user_id", "value": normalized_user_id}, + ] + if approver_group_ids: + group_placeholders = [] + for index, group_id in enumerate(approver_group_ids): + placeholder = f"@group_id_{index}" + group_placeholders.append(placeholder) + parameters.append({"name": placeholder, "value": group_id}) + scope_clauses.append( + f"c.metadata.generated_artifact_approval_group_id IN ({', '.join(group_placeholders)})" + ) + + query = ( + "SELECT TOP @limit * FROM c " + "WHERE c.role = 'file' " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_state) " + "AND c.metadata.generated_artifact_approval_state = @pending_state " + "AND c.metadata.generated_artifact_approval_requested_by_id != @user_id " + f"AND ({' OR '.join(scope_clauses)})" + ) + try: + candidates = list(cosmos_messages_container.query_items( + query=query, + parameters=parameters, + enable_cross_partition_query=True, + )) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to list pending approvals", + {"error": str(exc)}, + level=logging.ERROR, + exceptionTraceback=True, + ) + return [] + + pending_approvals = [] + for candidate in candidates: + # The query narrows candidates; authorization is still decided by the shared predicate. + if not user_can_approve_generated_file(normalized_user_id, candidate): + continue + candidate_metadata = candidate.get("metadata") or {} + pending_approvals.append({ + "artifact_message_id": str(candidate.get("id") or "").strip(), + "source_conversation_id": str(candidate.get("conversation_id") or "").strip(), + "collaboration_conversation_id": str( + candidate_metadata.get("generated_artifact_approval_collaboration_conversation_id") or "" + ).strip(), + "file_name": str(candidate.get("filename") or "").strip(), + "output_format": str(candidate_metadata.get("generated_artifact_output_format") or "").strip(), + "approval": build_generated_file_approval_client_payload(candidate, normalized_user_id), + }) + return pending_approvals + + +def resolve_generated_file_approval_for_user( + user_id: str, + source_conversation_id: str, + artifact_message_id: str, + decision: str, +) -> Dict[str, Any]: + """Approve or deny one staged artifact after re-authorizing the acting user. + + Denial keeps the artifact message so the conversation still shows who declined it, but the + stored blob is removed immediately so unapproved content does not linger in storage. + """ + normalized_user_id = str(user_id or "").strip() + normalized_conversation_id = str(source_conversation_id or "").strip() + normalized_message_id = str(artifact_message_id or "").strip() + normalized_decision = str(decision or "").strip().lower() + + if not normalized_user_id or not normalized_conversation_id or not normalized_message_id: + raise ValueError("Approval target is incomplete") + if normalized_decision not in {APPROVAL_STATE_APPROVED, APPROVAL_STATE_DENIED}: + raise ValueError("Approval decision must be approved or denied") + + try: + message_item = cosmos_messages_container.read_item( + item=normalized_message_id, + partition_key=normalized_conversation_id, + ) + except CosmosResourceNotFoundError as exc: + raise LookupError("Generated file approval not found") from exc + + metadata = message_item.get("metadata") if isinstance(message_item.get("metadata"), dict) else {} + if ( + str(message_item.get("conversation_id") or "").strip() != normalized_conversation_id + or message_item.get("role") != "file" + or not metadata.get("is_generated_chat_artifact") + or not metadata.get("generated_artifact_approval_required") + ): + raise LookupError("Generated file approval not found") + + # Re-authorize against the stored approval scope on every call so a client can never + # nominate itself as the approver. + if not user_can_approve_generated_file(normalized_user_id, message_item): + raise PermissionError("You are not allowed to approve this file") + + resolver_summary = _get_current_user_summary_or_none(normalized_user_id) + updated_message = apply_generated_file_approval_decision( + message_item, + normalized_decision, + resolver=resolver_summary, + ) + + if normalized_decision == APPROVAL_STATE_DENIED: + delete_blob_backed_chat_message_files([updated_message]) + + cosmos_messages_container.upsert_item(updated_message) + _clear_generated_file_approval_notifications(normalized_message_id) + _notify_generated_file_approval_resolved(updated_message, normalized_decision, resolver_summary) + + log_event( + "[GENERATED_FILE_APPROVALS] Resolved staged artifact", + { + "conversation_id": normalized_conversation_id, + "message_id": normalized_message_id, + "decision": normalized_decision, + }, + debug_only=True, + ) + return updated_message + + +def _clear_generated_file_approval_notifications(artifact_message_id: str) -> None: + """Remove approver notifications once a staged artifact has been resolved.""" + normalized_message_id = str(artifact_message_id or "").strip() + if not normalized_message_id: + return + + try: + delete_notifications_by_metadata( + metadata_filters={"generated_artifact_message_id": normalized_message_id}, + notification_types=[GENERATED_FILE_APPROVAL_NOTIFICATION_TYPE], + ) + except Exception as exc: + log_event( + f"[GENERATED_FILE_APPROVALS] Failed to clear approval notifications: {exc}", + level=logging.WARNING, + exceptionTraceback=True, + ) + + +def _notify_generated_file_approval_resolved( + message_item: Dict[str, Any], + decision: str, + resolver: Optional[Dict[str, Any]] = None, +) -> None: + """Tell the requester what happened to the file they generated.""" + metadata = message_item.get("metadata") if isinstance(message_item.get("metadata"), dict) else {} + requester_user_id = str(metadata.get("generated_artifact_approval_requested_by_id") or "").strip() + if not requester_user_id: + return + + collaboration_conversation_id = str( + metadata.get("generated_artifact_approval_collaboration_conversation_id") or "" + ).strip() + file_name = str(message_item.get("filename") or "").strip() or "your generated file" + resolver_name = str((resolver or {}).get("display_name") or "").strip() or "An approver" + approved = decision == APPROVAL_STATE_APPROVED + + _create_personal_notification( + user_id=requester_user_id, + notification_type=( + "generated_file_approval_approved" if approved else "generated_file_approval_denied" + ), + title="File approved" if approved else "File not approved", + message=( + f"{resolver_name} approved {file_name}. It is now available to download." + if approved + else f"{resolver_name} declined {file_name}." + ), + link_url=( + f"/chats?conversationId={collaboration_conversation_id}" + if collaboration_conversation_id + else "" + ), + link_context={ + "conversation_id": collaboration_conversation_id, + "conversation_kind": "collaborative", + }, + metadata={ + "generated_artifact_message_id": str(message_item.get("id") or "").strip(), + "source_conversation_id": str(message_item.get("conversation_id") or "").strip(), + "collaboration_conversation_id": collaboration_conversation_id, + }, + ) + + +def _collect_generated_artifact_entries(message: Dict[str, Any]) -> List[Dict[str, Any]]: + """Return the generated-artifact descriptors embedded in one serialized message.""" + metadata = message.get("metadata") if isinstance(message.get("metadata"), dict) else {} + entries = [] + for collection_key in ("generated_analysis_artifacts", "generated_tabular_outputs"): + for entry in metadata.get(collection_key) or []: + if isinstance(entry, dict) and str(entry.get("artifact_message_id") or "").strip(): + entries.append(entry) + return entries + + +def attach_generated_file_approval_state( + messages: Optional[List[Dict[str, Any]]], + viewer_user_id: str, +) -> Optional[List[Dict[str, Any]]]: + """Attach live approval state to generated-file artifacts in serialized messages. + + Approval state is read fresh rather than trusted from the stored assistant metadata so an + approver sees actionable controls and a requester sees the current decision after a reload. + """ + normalized_viewer_id = str(viewer_user_id or "").strip() + if not messages: + return messages + + entries_by_conversation: Dict[str, List[Dict[str, Any]]] = {} + for message in messages: + if not isinstance(message, dict): + continue + for entry in _collect_generated_artifact_entries(message): + entry_conversation_id = str(entry.get("conversation_id") or "").strip() + if entry_conversation_id: + entries_by_conversation.setdefault(entry_conversation_id, []).append(entry) + + if not entries_by_conversation: + return messages + + query = ( + "SELECT c.id, c.conversation_id, c.filename, c.role, c.metadata FROM c " + "WHERE c.role = 'file' " + "AND IS_DEFINED(c.metadata.generated_artifact_approval_state)" + ) + for entry_conversation_id, entries in entries_by_conversation.items(): + try: + artifact_docs = list(cosmos_messages_container.query_items( + query=query, + partition_key=entry_conversation_id, + )) + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to hydrate approval state for a conversation", + {"conversation_id": entry_conversation_id, "error": str(exc)}, + debug_only=True, + ) + continue + + approval_by_message_id = { + str(artifact_doc.get("id") or "").strip(): build_generated_file_approval_client_payload( + artifact_doc, + normalized_viewer_id, + ) + for artifact_doc in artifact_docs + } + for entry in entries: + approval_payload = approval_by_message_id.get( + str(entry.get("artifact_message_id") or "").strip() + ) + if approval_payload: + entry["approval"] = approval_payload + + return messages + + +def auto_deny_expired_generated_file_approvals() -> int: + """Auto-deny staged artifacts whose approval window elapsed and drop their blobs.""" + expired_artifacts = list_expired_pending_generated_file_artifacts() + denied_count = 0 + + for message_item in expired_artifacts: + message_id = str(message_item.get("id") or "").strip() + conversation_id = str(message_item.get("conversation_id") or "").strip() + if not message_id or not conversation_id: + continue + + try: + updated_message = apply_generated_file_approval_decision( + message_item, + APPROVAL_STATE_AUTO_DENIED, + resolver={"display_name": "Automatic expiry"}, + ) + delete_blob_backed_chat_message_files([updated_message]) + cosmos_messages_container.upsert_item(updated_message) + _clear_generated_file_approval_notifications(message_id) + _notify_generated_file_approval_resolved( + updated_message, + APPROVAL_STATE_AUTO_DENIED, + resolver={"display_name": "Automatic expiry"}, + ) + denied_count += 1 + except Exception as exc: + log_event( + "[GENERATED_FILE_APPROVALS] Failed to auto-deny an expired staged artifact", + { + "conversation_id": conversation_id, + "message_id": message_id, + "error": str(exc), + }, + level=logging.WARNING, + exceptionTraceback=True, + ) + + if denied_count: + log_event( + "[GENERATED_FILE_APPROVALS] Auto-denied expired staged artifacts", + {"artifact_count": denied_count}, + ) + return denied_count + + def _upload_generated_chat_artifact_for_current_user( current_user_id: str, conversation_id: str, @@ -2569,8 +3012,23 @@ def _upload_generated_chat_artifact_for_current_user( except CosmosResourceNotFoundError as exc: raise LookupError(f"Conversation {conversation_id} not found") from exc - if str(conversation_item.get("user_id") or "").strip() != current_user_id: - raise PermissionError("Forbidden") + # Shared conversations are backed by a source conversation owned by their creator, so a + # participant can never satisfy a plain ownership comparison even though they are a + # legitimate member. Authorize against the linked shared conversation instead, then decide + # whether this deliverable has to be staged for approval. + access_context = build_conversation_participation_context(current_user_id, conversation_item) + + artifact_metadata = artifact_metadata if isinstance(artifact_metadata, dict) else {} + approval_metadata = {} + if requires_generated_file_approval( + access_context, + file_name=normalized_file_name, + output_format=str(artifact_metadata.get("output_format") or ""), + ): + approval_metadata = build_generated_file_approval_metadata( + access_context, + requester=_get_current_user_summary_or_none(current_user_id), + ) blob_service_client = CLIENTS.get("storage_account_office_docs_client") if not blob_service_client: @@ -2639,7 +3097,10 @@ def _upload_generated_chat_artifact_for_current_user( artifact_capability = str(artifact_metadata.get("capability") or "analysis").strip().lower() or "analysis" artifact_output_format = str(artifact_metadata.get("output_format") or file_extension).strip().lower() or file_extension artifact_summary = str(artifact_metadata.get("summary") or "").strip() - lifecycle_metadata = _build_generated_chat_artifact_lifecycle_metadata(artifact_metadata) + lifecycle_metadata = _build_generated_chat_artifact_lifecycle_metadata( + artifact_metadata, + artifact_run_user_id=current_user_id, + ) message_doc = { "id": artifact_message_id, @@ -2660,6 +3121,7 @@ def _upload_generated_chat_artifact_for_current_user( "generated_artifact_summary": artifact_summary, "generated_artifact_idempotency_key": normalized_idempotency_key or None, **lifecycle_metadata, + **approval_metadata, "thread_info": { "thread_id": current_thread_id, "previous_thread_id": previous_thread_id, @@ -2670,6 +3132,9 @@ def _upload_generated_chat_artifact_for_current_user( } cosmos_messages_container.upsert_item(message_doc) + if approval_metadata: + _notify_generated_file_approval_requested(message_doc, access_context) + log_event( "[SIMPLE_CHAT] Generated chat artifact saved", { @@ -2680,6 +3145,7 @@ def _upload_generated_chat_artifact_for_current_user( "storage_scope": "chat", "capability": artifact_capability, "output_format": artifact_output_format, + "approval_state": approval_metadata.get("generated_artifact_approval_state") or "", }, debug_only=True, ) @@ -2692,6 +3158,8 @@ def _upload_generated_chat_artifact_for_current_user( "blob_path": blob_path, "capability": artifact_capability, "output_format": artifact_output_format, + "approval_state": approval_metadata.get("generated_artifact_approval_state") or "", + "approval_required": bool(approval_metadata), **_build_generated_chat_artifact_lifecycle_response(message_doc.get("metadata")), }, "conversation_id": conversation_id, @@ -2705,7 +3173,10 @@ def _safe_positive_int(value: Any) -> int: return 0 -def _build_generated_chat_artifact_lifecycle_metadata(artifact_metadata: Dict[str, Any]) -> Dict[str, Any]: +def _build_generated_chat_artifact_lifecycle_metadata( + artifact_metadata: Dict[str, Any], + artifact_run_user_id: str = "", +) -> Dict[str, Any]: metadata = artifact_metadata if isinstance(artifact_metadata, dict) else {} run_id = str(metadata.get("artifact_run_id") or metadata.get("run_id") or "").strip() set_id = str(metadata.get("artifact_set_id") or "").strip() @@ -2727,6 +3198,12 @@ def _build_generated_chat_artifact_lifecycle_metadata(artifact_metadata: Dict[st "generated_artifact_run_id": run_id, "generated_artifact_set_id": set_id, "generated_artifact_member_id": member_id, + # Export runs are partitioned by the user who queued them. In a shared conversation the + # participant queues the run while the owner may be the one downloading, so the run + # owner is recorded here instead of being inferred from the caller. + "generated_artifact_run_user_id": str( + metadata.get("artifact_run_user_id") or artifact_run_user_id or "" + ).strip(), "generated_artifact_lifecycle_state": lifecycle_state, "generated_artifact_validation_state": validation_state, "generated_artifact_publication_generation": _safe_positive_int( @@ -2782,13 +3259,17 @@ def assert_generated_chat_artifact_is_published_for_user(current_user_id: str, m set_id = str(metadata.get("generated_artifact_set_id") or "").strip() member_id = str(metadata.get("generated_artifact_member_id") or "").strip() conversation_id = str(message_item.get("conversation_id") or "").strip() + # Older artifacts predate the recorded run owner, so fall back to the caller for them. + run_user_id = str( + metadata.get("generated_artifact_run_user_id") or current_user_id or "" + ).strip() if not run_id or not set_id or not member_id or not conversation_id: raise PermissionError("Artifact publication metadata is incomplete") try: run = cosmos_tabular_export_runs_container.read_item( item=run_id, - partition_key=str(current_user_id or "").strip(), + partition_key=run_user_id, ) except CosmosResourceNotFoundError as exc: raise PermissionError("Artifact publication run is unavailable") from exc @@ -2796,7 +3277,7 @@ def assert_generated_chat_artifact_is_published_for_user(current_user_id: str, m manifest = run.get("artifact_set_manifest") if isinstance(run.get("artifact_set_manifest"), dict) else {} if ( str(run.get("conversation_id") or "").strip() != conversation_id - or str(run.get("user_id") or "").strip() != str(current_user_id or "").strip() + or str(run.get("user_id") or "").strip() != run_user_id or str(manifest.get("set_id") or "").strip() != set_id or str(manifest.get("lifecycle_state") or "").strip().lower() != "completed" or str(manifest.get("validation_state") or "").strip().lower() != GENERATED_CHAT_ARTIFACT_VALIDATION_VALIDATED @@ -2838,8 +3319,9 @@ def commit_generated_chat_artifact_publication_for_user( item=normalized_conversation_id, partition_key=normalized_conversation_id, ) - if str(conversation_item.get("user_id") or "").strip() != current_user_id: - raise PermissionError("Forbidden") + # Shared conversations are owned by their creator, so the participant whose export run + # produced this artifact must be authorized through the linked shared conversation. + build_conversation_participation_context(current_user_id, conversation_item) message_item = cosmos_messages_container.read_item( item=normalized_message_id, partition_key=normalized_conversation_id, @@ -2883,11 +3365,21 @@ def _resolve_group_upload_target_for_current_user( if not allowed: raise PermissionError(reason) - assert_group_role( - current_user_id, - normalized_group_id, - allowed_roles=("Owner", "Admin", "DocumentManager"), - ) + try: + assert_group_role( + current_user_id, + normalized_group_id, + allowed_roles=("Owner", "Admin", "DocumentManager"), + ) + except PermissionError as exc: + # Workspace writes feed the group search index, so they cannot be staged the way chat + # deliverables are. Tell the requester who can complete it instead of failing blankly. + group_name = str(group_doc.get("name") or "this group").strip() or "this group" + raise PermissionError( + f"Saving documents to the {group_name} workspace requires the Owner, Admin, or " + "Document Manager role. Ask a document manager to add this file, or request it as " + "a downloadable file in the conversation instead." + ) from exc return normalized_group_id diff --git a/application/single_app/route_backend_chats.py b/application/single_app/route_backend_chats.py index b4224c1a0..1587cd4ad 100644 --- a/application/single_app/route_backend_chats.py +++ b/application/single_app/route_backend_chats.py @@ -186,6 +186,7 @@ merge_cited_documents_into_conversation, resolve_citation_location, ) +from functions_collaboration import build_conversation_participation_context from functions_conversation_metadata import collect_conversation_metadata, update_conversation_with_metadata from functions_conversation_unread import mark_conversation_unread from functions_image_messages import build_image_message_documents, decode_image_content @@ -3465,8 +3466,15 @@ def _create_personal_conversation(user_id, conversation_id=None): return conversation_item -def _authorize_personal_conversation_access(user_id, conversation_id): - """Load a personal conversation and ensure the caller owns it.""" +def _resolve_authorized_conversation_context(user_id, conversation_id): + """Load a conversation plus the access context that authorized the caller. + + Shared conversations are backed by a hidden source conversation owned by the shared + conversation creator, so participants can never satisfy a plain ownership comparison even + though they are legitimate members. The returned context distinguishes an owner acting in + their own conversation from a participant acting inside a shared one, which downstream + artifact writes use to decide whether an approval is required. + """ try: conversation_item = cosmos_conversations_container.read_item( item=conversation_id, @@ -3475,9 +3483,13 @@ def _authorize_personal_conversation_access(user_id, conversation_id): except CosmosResourceNotFoundError as exc: raise LookupError(f"Conversation {conversation_id} not found") from exc - if conversation_item.get('user_id') != user_id: - raise PermissionError('You can only access your own conversations') + access_context = build_conversation_participation_context(user_id, conversation_item) + return conversation_item, access_context + +def _authorize_personal_conversation_access(user_id, conversation_id): + """Load a personal conversation and ensure the caller may act in it.""" + conversation_item, _ = _resolve_authorized_conversation_context(user_id, conversation_id) return conversation_item diff --git a/application/single_app/route_backend_collaboration.py b/application/single_app/route_backend_collaboration.py index 3c706ff45..c08925dbb 100644 --- a/application/single_app/route_backend_collaboration.py +++ b/application/single_app/route_backend_collaboration.py @@ -59,6 +59,11 @@ ) from functions_notifications import mark_collaboration_message_notifications_read_for_conversation from functions_message_artifacts import make_json_serializable +from functions_simplechat_operations import ( + attach_generated_file_approval_state, + list_pending_generated_file_approvals_for_user, + resolve_generated_file_approval_for_user, +) from functions_settings import get_settings from swagger_wrapper import swagger_route, get_auth_security @@ -413,6 +418,76 @@ def _sync_collaboration_mask_metadata_to_source(message_doc): def register_route_backend_collaboration(bp): + @bp.route('/api/collaboration/file-approvals', methods=['GET']) + @swagger_route(security=get_auth_security()) + @login_required + @user_required + def list_generated_file_approvals_api(): + """List generated files staged in shared conversations that this user may release.""" + try: + _require_collaboration_feature_enabled() + current_user = _get_current_collaboration_user() + if not current_user: + return jsonify({'error': 'User not authenticated'}), 401 + + approvals = list_pending_generated_file_approvals_for_user(current_user['user_id']) + return jsonify({'approvals': approvals}) + except PermissionError as exc: + return jsonify({'error': str(exc)}), 403 + except Exception as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Failed to list pending file approvals: {exc}', + level=logging.ERROR, + exceptionTraceback=True, + ) + return jsonify({'error': 'Failed to load pending file approvals'}), 500 + + @bp.route( + '/api/collaboration/file-approvals///', + methods=['POST'], + ) + @swagger_route(security=get_auth_security()) + @login_required + @user_required + def resolve_generated_file_approval_api(source_conversation_id, artifact_message_id, decision): + """Approve or deny one generated file staged in a shared conversation.""" + try: + _require_collaboration_feature_enabled() + current_user = _get_current_collaboration_user() + if not current_user: + return jsonify({'error': 'User not authenticated'}), 401 + + normalized_decision = str(decision or '').strip().lower() + if normalized_decision not in ('approve', 'deny'): + return jsonify({'error': 'Decision must be approve or deny'}), 400 + + updated_message = resolve_generated_file_approval_for_user( + user_id=current_user['user_id'], + source_conversation_id=source_conversation_id, + artifact_message_id=artifact_message_id, + decision='approved' if normalized_decision == 'approve' else 'denied', + ) + message_metadata = updated_message.get('metadata') or {} + return jsonify({ + 'artifact_message_id': updated_message.get('id'), + 'approval_state': message_metadata.get('generated_artifact_approval_state'), + 'resolved_by_name': message_metadata.get('generated_artifact_approval_resolved_by_name'), + 'resolved_at': message_metadata.get('generated_artifact_approval_resolved_at'), + }) + except LookupError: + return jsonify({'error': 'Generated file approval not found'}), 404 + except PermissionError as exc: + return jsonify({'error': str(exc)}), 403 + except ValueError as exc: + return jsonify({'error': str(exc)}), 400 + except Exception as exc: + log_event( + f'[GENERATED_FILE_APPROVALS] Failed to resolve file approval: {exc}', + level=logging.ERROR, + exceptionTraceback=True, + ) + return jsonify({'error': 'Failed to resolve the file approval'}), 500 + @bp.route('/api/collaboration/conversations', methods=['GET']) @swagger_route(security=get_auth_security()) @login_required @@ -1178,6 +1253,7 @@ def get_collaboration_messages_api(conversation_id): allow_pending=True, ) messages = [serialize_collaboration_message(doc) for doc in list_collaboration_messages(conversation_id)] + attach_generated_file_approval_state(messages, current_user['user_id']) return jsonify({'messages': messages}), 200 except CosmosResourceNotFoundError: return jsonify({'error': 'Collaborative conversation not found'}), 404 diff --git a/application/single_app/route_enhanced_citations.py b/application/single_app/route_enhanced_citations.py index e4e380814..36831008e 100644 --- a/application/single_app/route_enhanced_citations.py +++ b/application/single_app/route_enhanced_citations.py @@ -24,6 +24,8 @@ from functions_group import check_group_status_allows_operation, find_group_by_id, get_user_groups, require_active_group from functions_notifications import create_group_notification, create_notification, create_public_workspace_notification from functions_public_workspaces import check_public_workspace_status_allows_operation, get_user_visible_public_workspace_ids_from_settings, require_active_public_workspace +from functions_collaboration import build_conversation_participation_context +from functions_generated_file_approvals import assert_generated_file_approval_allows_download from functions_simplechat_operations import ( assert_generated_chat_artifact_is_published_for_user, download_blob_content, @@ -48,8 +50,9 @@ def _get_authorized_chat_artifact_message(user_id, conversation_id, message_id): except CosmosResourceNotFoundError as exc: raise LookupError('Conversation not found') from exc - if str(conversation_item.get('user_id') or '').strip() != str(user_id or '').strip(): - raise PermissionError('Forbidden') + # Shared conversations are owned by their creator, so participants fail a plain ownership + # comparison. Authorize them against the linked shared conversation instead. + build_conversation_participation_context(user_id, conversation_item) try: message_item = cosmos_messages_container.read_item( @@ -69,6 +72,9 @@ def _get_authorized_chat_artifact_message(user_id, conversation_id, message_id): if not str(message_item.get('blob_container') or '').strip() or not str(message_item.get('blob_path') or '').strip(): raise LookupError('Chat artifact content is unavailable') + # Enforced independently of the export manifest checks below so a staged artifact stays + # unreachable for every caller, including the participant who requested it. + assert_generated_file_approval_allows_download(user_id, message_item) assert_generated_chat_artifact_is_published_for_user(user_id, message_item) return message_item @@ -482,6 +488,12 @@ def get_enhanced_citation_tabular(): file_msg = items[0] file_content_source = file_msg.get('file_content_source', '') + # Generated artifacts can be served here too, so the approval gate must be enforced + # on this reader as well. The source conversation owner is not necessarily an + # approver: a plain group User can create a group shared conversation while the + # approvers are that group's Owner, Admin, and Document Manager roles. + assert_generated_file_approval_allows_download(user_id, file_msg) + if file_content_source != 'blob': return jsonify({"error": "File is not stored in blob storage"}), 400 @@ -518,6 +530,8 @@ def get_enhanced_citation_tabular(): } ) + except PermissionError as exc: + return jsonify({"error": str(exc) or "Forbidden"}), 403 except Exception as e: debug_print(f"Error serving tabular citation: {e}") return jsonify({"error": str(e)}), 500 @@ -608,8 +622,10 @@ def download_chat_artifact(): }, force_download=True, ) - except PermissionError: - return jsonify({"error": "Forbidden"}), 403 + except PermissionError as exc: + # Surface the specific reason (for example a file awaiting approval) instead of a + # bare Forbidden, which is what made the original failure so hard to act on. + return jsonify({"error": str(exc) or "Forbidden"}), 403 except LookupError as exc: return jsonify({"error": str(exc)}), 404 except ValueError as exc: diff --git a/application/single_app/route_frontend_admin_settings.py b/application/single_app/route_frontend_admin_settings.py index 610cea6d0..c81e9f65d 100644 --- a/application/single_app/route_frontend_admin_settings.py +++ b/application/single_app/route_frontend_admin_settings.py @@ -2681,6 +2681,7 @@ def is_valid_url(url): 'enable_desktop_notifications': form_data.get('enable_desktop_notifications') == 'on', 'enable_conversation_archiving': form_data.get('enable_conversation_archiving') == 'on', 'enable_thoughts': form_data.get('enable_thoughts') == 'on', + 'require_shared_conversation_file_approval': form_data.get('require_shared_conversation_file_approval') == 'on', # Search (Web Search via Azure AI Foundry agent) 'enable_web_search': enable_web_search, diff --git a/application/single_app/static/js/chat/chat-file-approvals.js b/application/single_app/static/js/chat/chat-file-approvals.js new file mode 100644 index 000000000..f335a650f --- /dev/null +++ b/application/single_app/static/js/chat/chat-file-approvals.js @@ -0,0 +1,184 @@ +// chat-file-approvals.js + +import { showToast } from "./chat-toast.js"; + +const APPROVAL_STATE_PENDING = 'pending_approval'; +const APPROVAL_STATE_APPROVED = 'approved'; +const APPROVAL_STATE_DENIED = 'denied'; +const APPROVAL_STATE_AUTO_DENIED = 'auto_denied'; + +/** + * Read the approval descriptor attached to a generated artifact, when one exists. + * Artifacts produced by a conversation owner carry no descriptor and stay ungated. + */ +export function getGeneratedFileApproval(outputMetadata) { + const approval = outputMetadata && typeof outputMetadata.approval === 'object' + ? outputMetadata.approval + : null; + if (!approval) { + return null; + } + + const state = String(approval.state || '').trim().toLowerCase(); + if (!state) { + return null; + } + + return { + state, + isPending: state === APPROVAL_STATE_PENDING, + isApproved: state === APPROVAL_STATE_APPROVED, + isDenied: state === APPROVAL_STATE_DENIED || state === APPROVAL_STATE_AUTO_DENIED, + isAutoDenied: state === APPROVAL_STATE_AUTO_DENIED, + viewerCanApprove: approval.viewer_can_approve === true, + viewerIsRequester: approval.viewer_is_requester === true, + requestedByName: String(approval.requested_by_name || '').trim(), + resolvedByName: String(approval.resolved_by_name || '').trim(), + }; +} + +/** + * Return whether the artifact content is currently unavailable to everyone. + * A staged file is withheld from the requester too, so the download control is suppressed. + */ +export function generatedFileApprovalBlocksDownload(outputMetadata) { + const approval = getGeneratedFileApproval(outputMetadata); + if (!approval) { + return false; + } + return approval.isPending || approval.isDenied; +} + +function buildApprovalMessage(approval) { + if (approval.isApproved) { + return approval.resolvedByName + ? `Approved by ${approval.resolvedByName}.` + : 'Approved.'; + } + + if (approval.isAutoDenied) { + return 'This file expired before it was approved and is no longer available.'; + } + + if (approval.isDenied) { + return approval.resolvedByName + ? `${approval.resolvedByName} declined this file, so it is not available.` + : 'This file was declined and is not available.'; + } + + if (approval.viewerCanApprove) { + return approval.requestedByName + ? `${approval.requestedByName} generated this file in a shared conversation. Approve it to make it available.` + : 'A participant generated this file in a shared conversation. Approve it to make it available.'; + } + + if (approval.viewerIsRequester) { + return 'This file is waiting for the conversation owner to approve it before it can be downloaded.'; + } + + return 'This file is waiting for approval before it can be downloaded.'; +} + +async function submitApprovalDecision(outputMetadata, decision) { + const sourceConversationId = String(outputMetadata?.conversation_id || '').trim(); + const artifactMessageId = String(outputMetadata?.artifact_message_id || '').trim(); + if (!sourceConversationId || !artifactMessageId) { + throw new Error('This file approval is missing its conversation reference.'); + } + + const response = await fetch( + `/api/collaboration/file-approvals/${encodeURIComponent(sourceConversationId)}` + + `/${encodeURIComponent(artifactMessageId)}/${encodeURIComponent(decision)}`, + { + method: 'POST', + headers: { 'Content-Type': 'application/json' }, + credentials: 'same-origin', + }, + ); + + const payload = await response.json().catch(() => ({})); + if (!response.ok) { + throw new Error(payload.error || `Failed to ${decision} this file.`); + } + return payload; +} + +/** + * Build the approval banner rendered on a gated artifact card. + * Returns null when the artifact is approved or was never gated, so ordinary + * downloads render unchanged. + */ +export function buildGeneratedFileApprovalBlock(outputMetadata, onResolved = null) { + const approval = getGeneratedFileApproval(outputMetadata); + if (!approval || approval.isApproved) { + return null; + } + + const wrapper = document.createElement('div'); + wrapper.className = approval.isDenied + ? 'alert alert-secondary d-flex flex-column gap-2 mt-3 mb-0' + : 'alert alert-warning d-flex flex-column gap-2 mt-3 mb-0'; + wrapper.setAttribute('role', 'status'); + + const headline = document.createElement('div'); + headline.className = 'd-flex align-items-start gap-2'; + + const icon = document.createElement('i'); + icon.className = approval.isDenied ? 'bi bi-file-earmark-x' : 'bi bi-file-earmark-lock'; + icon.setAttribute('aria-hidden', 'true'); + headline.appendChild(icon); + + const messageText = document.createElement('span'); + messageText.className = 'small'; + messageText.textContent = buildApprovalMessage(approval); + headline.appendChild(messageText); + wrapper.appendChild(headline); + + if (!approval.isPending || !approval.viewerCanApprove) { + return wrapper; + } + + const actions = document.createElement('div'); + actions.className = 'd-flex flex-wrap gap-2'; + + const approveButton = document.createElement('button'); + approveButton.type = 'button'; + approveButton.className = 'btn btn-sm btn-success generated-file-approve-btn'; + approveButton.textContent = 'Approve'; + + const denyButton = document.createElement('button'); + denyButton.type = 'button'; + denyButton.className = 'btn btn-sm btn-outline-danger generated-file-deny-btn'; + denyButton.textContent = 'Deny'; + + const setBusy = isBusy => { + approveButton.disabled = isBusy; + denyButton.disabled = isBusy; + }; + + const handleDecision = async decision => { + setBusy(true); + try { + const payload = await submitApprovalDecision(outputMetadata, decision); + showToast( + decision === 'approve' ? 'File approved.' : 'File declined.', + decision === 'approve' ? 'success' : 'secondary', + ); + if (typeof onResolved === 'function') { + onResolved(decision, payload); + } + } catch (error) { + showToast(error.message || 'Failed to update this file approval.', 'danger'); + setBusy(false); + } + }; + + approveButton.addEventListener('click', () => handleDecision('approve')); + denyButton.addEventListener('click', () => handleDecision('deny')); + + actions.appendChild(approveButton); + actions.appendChild(denyButton); + wrapper.appendChild(actions); + + return wrapper; +} diff --git a/application/single_app/static/js/chat/chat-messages.js b/application/single_app/static/js/chat/chat-messages.js index 68061045d..5df9d3986 100644 --- a/application/single_app/static/js/chat/chat-messages.js +++ b/application/single_app/static/js/chat/chat-messages.js @@ -17,6 +17,10 @@ import { updateSidebarConversationTitle } from "./chat-sidebar-conversations.js" import { getActiveConversationContext, getActiveConversationScope } from "./chat-conversation-scope.js"; import { escapeHtml, isColorLight, addTargetBlankToExternalLinks, sanitizeHttpUrl } from "./chat-utils.js"; import { showToast } from "./chat-toast.js"; +import { + buildGeneratedFileApprovalBlock, + generatedFileApprovalBlocksDownload, +} from "./chat-file-approvals.js"; import { autoplayTTSIfEnabled, isTTSAutoplayEnabled, playTTS } from "./chat-tts.js"; import { saveUserSetting } from "./chat-layout.js"; import { sendMessageWithStreaming } from "./chat-streaming.js"; @@ -5429,6 +5433,27 @@ function renderReplyQuoteHtml(fullMessageObject = null) { const actions = document.createElement('div'); actions.className = 'd-flex flex-wrap gap-2 mt-3'; + // A staged file is withheld from everyone until an approver releases it, so the download + // and preview controls are replaced by the approval banner rather than left to fail. + const approvalBlock = buildGeneratedFileApprovalBlock(outputMetadata, (decision, payload) => { + if (outputMetadata && typeof outputMetadata.approval === 'object') { + outputMetadata.approval.state = payload?.approval_state + || (decision === 'approve' ? 'approved' : 'denied'); + outputMetadata.approval.viewer_can_approve = false; + outputMetadata.approval.resolved_by_name = payload?.resolved_by_name || ''; + } + const refreshedCard = createGeneratedAnalysisArtifactCard(outputMetadata); + if (refreshedCard && card.parentNode) { + card.parentNode.replaceChild(refreshedCard, card); + } + }); + if (approvalBlock) { + card.appendChild(approvalBlock); + } + if (generatedFileApprovalBlocksDownload(outputMetadata)) { + return card; + } + if (outputMetadata?.background_export) { const backgroundRunId = String(outputMetadata?.export_run_id || outputMetadata?.run_id || '').trim(); if (!backgroundRunId) { diff --git a/application/single_app/templates/admin_settings.html b/application/single_app/templates/admin_settings.html index 7bcd8410f..947f7f1ba 100644 --- a/application/single_app/templates/admin_settings.html +++ b/application/single_app/templates/admin_settings.html @@ -5016,6 +5016,27 @@
+ +
+
+ Shared Conversation File Approvals +
+

Files generated by participants in a shared conversation are saved into the conversation owner's storage. When enabled, those files are held until an approver releases them.

+
+ + + +
+
+
diff --git a/docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md b/docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md new file mode 100644 index 000000000..4d7f4f73c --- /dev/null +++ b/docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md @@ -0,0 +1,168 @@ +# Shared Conversation File Approvals + +## Overview + +Files that a participant asks the assistant to generate inside a shared (collaborative) +conversation are saved into the conversation owner's storage scope. This feature lets those +requests succeed by creating the file immediately and holding it in a `pending_approval` state +until an authorized approver releases it, instead of refusing the request outright. + +- **Implemented in version:** **0.260.004** +- **Depends on:** Collaborative conversations (`enable_collaborative_conversations`), generated + chat artifacts, notifications +- **Related fix:** `docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md` + +## Technical Specifications + +### Architecture + +A collaborative conversation is backed by a hidden **source conversation** +(`conversation_kind: 'collaboration_source'`) whose `user_id` is always the shared conversation +creator. Every participant streams through that source conversation, so any artifact they cause +to be written lands under the owner's conversation. + +```mermaid +flowchart TD + A[Participant asks for a CSV] --> B[Collaboration stream bridge] + B --> C[chat_stream_api on the source conversation] + C --> D{Is the caller the conversation owner?} + D -- Yes --> E[Artifact written and downloadable] + D -- No --> F{Downloadable format and approval enabled?} + F -- No --> E + F -- Yes --> G[Artifact written as pending_approval] + G --> H[Approvers notified] + H --> I{Decision} + I -- Approve --> J[Artifact released for download] + I -- Deny --> K[Blob deleted, decision recorded] + I -- No action for 3 days --> L[Auto-denied, blob deleted] +``` + +### Approval scope + +Only downloadable deliverables are gated. Generated images and charts are inline conversation +rendering and are never gated. + +| Gated | Not gated | +|-------|-----------| +| `csv`, `xlsx`, `xls`, `xlsm`, `docx`, `pdf`, `json`, `xml` | images, charts, plain assistant text | + +### Approvers + +| Conversation type | Who can approve | +|-------------------|-----------------| +| Personal shared conversation | The conversation owner | +| Group shared conversation | Any group `Owner`, `Admin`, or `DocumentManager` | + +The requester can never approve their own file — the requester check is applied before the +scope branch, so a group `Admin` or `DocumentManager` who is only a participant still needs a +different approver. A staged file is not downloadable by anyone, including the requester, until +it is released. + +Every route that streams a stored artifact blob enforces the gate, not just the generated +artifact download: `/api/chat_artifacts/download`, `/api/chat_artifacts/promote`, and +`/api/enhanced_citations/tabular` all call +`assert_generated_file_approval_allows_download` before reading blob content. This matters +because the source conversation owner is not necessarily an approver — a plain group `User` can +create a group shared conversation while approval belongs to that group's document roles. + +### Approval states + +`pending_approval` -> `approved` | `denied` | `auto_denied` + +State is stored on the artifact message `metadata` under +`generated_artifact_approval_*` fields, so it travels with the artifact and cannot be bypassed +by calling the download route directly. A decision is single-use; re-applying one raises a +`ValueError`. + +### Configuration + +| Setting | Default | Description | +|---------|---------|-------------| +| `require_shared_conversation_file_approval` | `True` | When disabled, participant-generated files are saved and downloadable immediately. | + +The toggle is exposed in **Admin Settings -> Shared Conversation File Approvals**. It contains +no sensitive terms, so it passes through `sanitize_settings_for_user()` and is readable by the +chat UI. + +### API endpoints + +| Method | Route | Purpose | +|--------|-------|---------| +| `GET` | `/api/collaboration/file-approvals` | List staged files the caller may release | +| `POST` | `/api/collaboration/file-approvals///approve` | Approve one staged file | +| `POST` | `/api/collaboration/file-approvals///deny` | Deny one staged file | + +Every route carries `@swagger_route(security=get_auth_security())`, `@login_required`, and +`@user_required`. The approver is re-authorized from the stored approval scope on every call, +so a client can never nominate itself as the approver. The listing endpoint narrows candidates +to the caller's own approval scopes inside the query, so the row cap cannot truncate another +user's items ahead of the caller's. + +### File structure + +| File | Purpose | +|------|---------| +| `functions_generated_file_approvals.py` | Approval states, gating decision, approver resolution, client payloads, expiry query | +| `functions_simplechat_operations.py` | Staging on artifact write, approval resolution, notifications, auto-deny sweep | +| `functions_collaboration.py` | `build_conversation_participation_context` shared authorization helper | +| `route_backend_collaboration.py` | Approval list and decision endpoints | +| `route_enhanced_citations.py` | Download-time approval enforcement | +| `static/js/chat/chat-file-approvals.js` | Inline approve/deny card and pending state | +| `background_tasks.py` | 3-day auto-deny sweep | + +### Expiry + +Staged files auto-deny after **3 days**, matching `functions_approvals.TTL_AUTO_DENY_DAYS`. The +sweep runs on the existing approval expiration loop and deletes the stored blob so unapproved +content does not linger in storage. + +## Usage Instructions + +### Enabling + +The feature is on by default. To disable it, clear **Require approval for participant-generated +files** in Admin Settings. + +### Participant workflow + +1. A participant asks the assistant for a file in a shared conversation. +2. The assistant answers normally and the file is created. +3. The artifact card shows *"This file is waiting for the conversation owner to approve it."* + No download button is offered. +4. When approved, the participant receives a notification and the file becomes downloadable. + +### Approver workflow + +1. A notification appears in the bell: *"File approval requested."* +2. Opening the conversation shows an inline card with **Approve** and **Deny** buttons on the + pending artifact. +3. Approving releases the file for everyone in the conversation. Denying deletes the stored file + and records who declined it. + +### Group workspace documents + +Saving a generated **document into a group workspace** is a different operation: it feeds the +group search index and still requires the `Owner`, `Admin`, or `DocumentManager` role. Users +without that role now receive an actionable message naming who can complete the request rather +than a bare permission error. Requesting the same content as a downloadable file in the +conversation goes through the approval flow instead. + +## Testing and Validation + +- `functional_tests/test_shared_conversation_file_approval_fix.py` (16 checks) covers format + scoping, owner bypass, the admin toggle, staged download refusal for all callers, personal and + group approver resolution, the requester self-approval guard across every group role, approval + enforcement on every artifact blob reader, scoped approval listing, single-use decisions, and + the wiring of each authorization gate. +- `functional_tests/route_tests/` confirms the new routes carry the required security + decorators and unauthenticated-access policy. + +### Known limitations + +- Workspace document writes are not staged, only chat deliverables. Holding a workspace document + back would require withholding search indexing, which is tracked as follow-up work. +- Participants can already upload files into a shared conversation without approval, so the + policy is intentionally asymmetric between uploading and generating. +- While a background export is still running, the live status poll may briefly offer a download + control. The download itself is refused with *"This file is waiting for owner approval"* and + the card corrects itself on reload. diff --git a/docs/explanation/features/index.md b/docs/explanation/features/index.md index 93ae1e6d8..03353656b 100644 --- a/docs/explanation/features/index.md +++ b/docs/explanation/features/index.md @@ -41,6 +41,10 @@ category: Version History - [Outlook MSG File Ingestion](v0.242.063/MSG_FILE_INGESTION.md) - [Chat Upload Personal Workspace Handoff](CHAT_UPLOAD_PERSONAL_WORKSPACE_HANDOFF.md) +## Collaborative Conversation Features + +- [Shared Conversation File Approvals](SHARED_CONVERSATION_FILE_APPROVALS.md) + ## Workspace Branding Features - [Group And Public Workspace Custom Hero Colors](GROUP_PUBLIC_WORKSPACE_CUSTOM_HERO_COLORS.md) diff --git a/docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md b/docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md new file mode 100644 index 000000000..afab69475 --- /dev/null +++ b/docs/explanation/fixes/SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md @@ -0,0 +1,158 @@ +# Shared Conversation File Generation "Forbidden" Fix + +## Issue + +In a shared (collaborative) conversation, a user who was invited into the conversation received: + +``` +Stream interrupted before any content was received. +Stream interrupted: Forbidden +``` + +The reported case was a participant asking `@Telemetry generate a csv of the 900 samples` in a +conversation shared from a personal chat. No content was returned at all. + +- **Fixed in version:** **0.260.004** +- **Related feature:** `docs/explanation/features/SHARED_CONVERSATION_FILE_APPROVALS.md` + +## Root Cause Analysis + +A collaborative conversation is backed by a hidden **source conversation** +(`conversation_kind: 'collaboration_source'`, created in +`functions_collaboration.ensure_collaboration_source_conversation`) whose `user_id` is always the +shared conversation creator. + +`/api/collaboration/conversations//stream` bridges into the internal `chat_stream_api` view +using that source conversation id, but keeps the **requesting participant's** session. Every +downstream owner-equality comparison therefore failed for participants. + +Four separate gates were involved: + +| # | Location | Check | Effect | +|---|----------|-------|--------| +| 1 | `route_backend_chats._authorize_personal_conversation_access`, called by `chat_stream_api` | `conversation_item.get('user_id') != user_id` | Returned `{'error': 'Forbidden'}, 403`. The collaboration bridge converts any `>= 400` response into a stream error, producing the reported banner. This blocked **all** AI invocation by participants, not just file generation. | +| 2 | `functions_simplechat_operations._upload_generated_chat_artifact_for_current_user` | `conversation.user_id != current_user_id` | Raised `PermissionError("Forbidden")` when persisting the artifact. `maybe_create_generated_file_output` swallowed it, so the file silently disappeared. | +| 3 | `functions_simplechat_operations._resolve_group_upload_target_for_current_user` | `assert_group_role(Owner/Admin/DocumentManager)` | Plain group `User` members could not save generated documents into a group workspace. | +| 4 | `route_enhanced_citations._get_authorized_chat_artifact_message` | `conversation.user_id != user_id` | Participants could not **download** a generated artifact even once it existed. | + +Gate 1 explains why the failure looked file-specific: collaborative conversations default to +`ai_invocation_mode: 'explicit_only'`, so ordinary participant messages never reach the stream +bridge. The first explicit AI invocation was the first time the gate was hit. + +Two secondary defects were found while fixing this: + +- `assert_generated_chat_artifact_is_published_for_user` read the export run using the + **caller's** id as the partition key. Background exports are queued by the participant while + the owner may be the one downloading, so an approved large CSV would have been unreadable. +- `commit_generated_chat_artifact_publication_for_user` and + `delete_generated_chat_artifact_for_user` carried the same owner-only comparison, which would + have broken publication and rollback for participant-queued background exports. 900 rows + exceeds the 500-row inline threshold, so the reported case used exactly this path. + +Three further defects were caught in review of the approval gate itself and fixed before +merge: + +- **Requester self-approval.** `resolve_generated_file_approver_role` resolved group-scope + approvers purely by group role, so a group `Admin` or `DocumentManager` who was only a + participant could stage a file and immediately approve it themselves. The requester check now + runs before the scope branch. +- **Bypass via `/api/enhanced_citations/tabular`.** That route streams any blob-backed file + message after a single conversation-ownership check and never consulted the approval gate. A + plain group `User` who created a group shared conversation — explicitly not an approver — could + fetch a staged CSV or XLSX directly. The gate now runs there before the blob is read, and the + route returns `403` rather than `500` for a withheld file. +- **Truncation before authorization.** `list_pending_generated_file_approvals_for_user` applied + `TOP @limit` across the whole messages container and filtered by approver afterwards in + Python, so a tenant with more than 50 pending files could return an empty list to an approver + who genuinely had items waiting. Candidates are now narrowed to the caller's own approval + scopes inside the query. + +## Technical Details + +### Files modified + +| File | Change | +|------|--------| +| `collaboration_models.py` | Added `COLLABORATION_SOURCE_KIND` and replaced the string literals | +| `functions_collaboration.py` | Added `is_collaboration_source_conversation`, `get_collaboration_conversation_for_source`, and `build_conversation_participation_context` | +| `route_backend_chats.py` | `_authorize_personal_conversation_access` now delegates to the collaboration-aware `_resolve_authorized_conversation_context` | +| `functions_simplechat_operations.py` | Artifact upload, publication commit, and rollback authorize through the participation context; artifact writes by participants are staged; run owner recorded for publication checks | +| `route_enhanced_citations.py` | Download authorizes participants and enforces the approval gate before the export manifest check | +| `functions_generated_file_approvals.py` | New module holding approval state and gating logic | +| `functions_notifications.py` | Added the three generated-file approval notification types | +| `functions_settings.py` | Added `require_shared_conversation_file_approval`, default `True` | +| `background_tasks.py` | Auto-deny sweep for expired staged files | +| `static/js/chat/chat-file-approvals.js` | New inline approval card | +| `static/js/chat/chat-messages.js` | Renders the approval card and suppresses downloads while staged | + +### Key change + +All five call sites of `_authorize_personal_conversation_access` now resolve access through a +single shared helper, which mirrors the pattern already used by chat file uploads in +`route_frontend_chats._resolve_chat_upload_context`: + +```python +def build_conversation_participation_context(user_id, conversation_item): + # Owners keep their existing behavior. + if normalized_user_id and owner_user_id == normalized_user_id: + return {..., 'is_owner': True} + + # Participants are authorized against the linked shared conversation instead. + collaboration_conversation = get_collaboration_conversation_for_source(normalized_item) + if not collaboration_conversation: + raise PermissionError('You can only access your own conversations') + + collaboration_access = assert_user_can_participate_in_collaboration_conversation( + normalized_user_id, + collaboration_conversation, + ) + return {..., 'is_owner': False} +``` + +Ordinary personal conversations with no collaboration link remain strictly owner-only. + +### Testing approach + +`functional_tests/test_shared_conversation_file_approval_fix.py` loads the approval module +against dependency stubs, because `config.py` connects to Cosmos DB at import time. The +authorization wiring for each gate is asserted against the real source using AST extraction, the +same technique used by `test_broken_access_control_findings_fix.py`. + +Two existing tests that extract these functions via AST were updated to supply the new +dependency: `test_generated_artifact_lifecycle_authorization.py` and +`test_tabular_row_orchestration_scale.py`. + +### Impact analysis + +- Participants can now invoke the AI in shared conversations at all — the primary regression. +- Owners see no behavior change: `is_owner` short-circuits before any approval logic. +- Non-shared personal conversations are unchanged and remain owner-only. +- Downloads gain one extra check that runs before the existing publication assertion, so a + staged file can never be retrieved. + +## Validation + +| Test | Result | +|------|--------| +| `functional_tests/test_shared_conversation_file_approval_fix.py` | 16/16 passed | +| `functional_tests/test_generated_artifact_lifecycle_authorization.py` | 6/6 passed | +| `functional_tests/test_tabular_row_orchestration_scale.py` | passed | +| `functional_tests/test_assistant_table_csv_artifact.py` | 35/35 passed | +| `functional_tests/test_generated_json_xml_exports.py` | 7/7 passed | +| `functional_tests/route_tests/` | 12/12 passed across three suites | + +Pre-existing failures unrelated to this change and confirmed against a clean baseline: +`test_mixed_source_hardening.py` (one assertion), `test_tabular_generated_output_exports.py` +(stale exact-version assertions pinned to `0.241.144`), and any test requiring live Azure +credentials. + +### Before and after + +| Scenario | Before | After | +|----------|--------|-------| +| Participant asks the AI anything in a shared conversation | `Stream interrupted: Forbidden`, no content | Normal response | +| Participant asks for a CSV | Forbidden, or artifact silently dropped | File created, held for approval, approver notified | +| Owner approves | Not possible | File becomes downloadable for the conversation | +| Owner denies | Not possible | Stored file deleted, decision recorded in the card | +| Nobody responds | Not possible | Auto-denied after 3 days, blob deleted | +| Owner generates their own file | Worked | Unchanged | diff --git a/docs/explanation/fixes/index.md b/docs/explanation/fixes/index.md index e80b770e4..4f288ef52 100644 --- a/docs/explanation/fixes/index.md +++ b/docs/explanation/fixes/index.md @@ -14,6 +14,7 @@ category: Version History - [CosmosClient Import Binding CodeQL Fix](COSMOSCLIENT_IMPORT_BINDING_CODEQL_FIX.md) - [Log Credential Key Redaction Fix](LOG_CREDENTIAL_KEY_REDACTION_FIX.md) - [Conversation Cache Invalidation Authorization Fix](CONVERSATION_CACHE_INVALIDATION_AUTHORIZATION_FIX.md) +- [Shared Conversation File Generation Forbidden Fix](SHARED_CONVERSATION_FILE_GENERATION_FORBIDDEN_FIX.md) - [Chat Completion Background Unread Guard Fix](CHAT_COMPLETION_BACKGROUND_UNREAD_GUARD_FIX.md) - [Settings Container RU Write Suppression Fix](SETTINGS_CONTAINER_RU_WRITE_SUPPRESSION_FIX.md) - [Tabular SK Python 3.13 Kernel Parameter Fix](v0.242.068/TABULAR_SK_PY313_KERNEL_PARAMETER_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index 2c9f83afd..06a5491a9 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,36 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.260.004)** + +#### New Features + +* **Shared Conversation File Approvals** + * Files generated by a participant in a shared conversation are now created immediately and held in a **pending approval** state instead of being refused, because they are saved into the conversation owner's storage. + * The conversation owner approves personal shared conversations; any group **Owner**, **Admin**, or **Document Manager** approves group shared conversations. Requesters can never approve their own file. + * Approvers get an inline **Approve / Deny** card on the pending file plus a notification. Approving releases the file, denying deletes the stored file and records who declined it. + * A staged file is not downloadable by anyone, including the requester, until it is released. + * Only downloadable deliverables are gated (CSV, XLSX, DOCX, PDF, JSON, XML). Generated images and charts are never gated. + * Unapproved files are automatically declined and deleted after 3 days, matching the existing Control Center approval window. + * New Admin Settings toggle **Require approval for participant-generated files**, enabled by default. + * (Ref: `functions_generated_file_approvals.py`, `chat-file-approvals.js`, `require_shared_conversation_file_approval`, `/api/collaboration/file-approvals`) + +#### Bug Fixes + +* **Shared Conversations No Longer Fail With "Stream interrupted: Forbidden"** + * Fixed invited participants being unable to invoke the AI at all in a shared conversation. Any explicit AI request returned `Forbidden` with no content. + * Root cause was the hidden source conversation behind every shared conversation being owned by its creator, so participants failed a plain ownership comparison in the chat streaming route even though they are legitimate members. + * Because shared conversations only call the AI on an explicit mention, this surfaced the first time a participant asked the assistant for something, which made it look file-specific. + * Participants can also now download generated files from a shared conversation, which was blocked by the same comparison. + * Also fixed background CSV exports queued by a participant becoming unreadable for the owner, because publication checks looked up the export run under the wrong user partition. + * (Ref: `build_conversation_participation_context`, `route_backend_chats.py`, `route_enhanced_citations.py`, `functions_simplechat_operations.py`) + +#### User Interface Enhancements + +* **Clearer Group Workspace Save Errors** + * Attempting to save a generated document into a group workspace without document rights now names the roles that can complete it and suggests requesting the content as a downloadable file instead of failing with a bare permission error. + * (Ref: `_resolve_group_upload_target_for_current_user`) + ### **(v0.260.003)** #### Bug Fixes diff --git a/functional_tests/test_generated_artifact_lifecycle_authorization.py b/functional_tests/test_generated_artifact_lifecycle_authorization.py index feda366c3..5d1f9680b 100644 --- a/functional_tests/test_generated_artifact_lifecycle_authorization.py +++ b/functional_tests/test_generated_artifact_lifecycle_authorization.py @@ -47,6 +47,20 @@ def upsert_item(self, body): return body +def _fake_participation_context(user_id, conversation_item): + """Stand in for the collaboration-aware authorization used by artifact helpers.""" + owner_user_id = str((conversation_item or {}).get("user_id") or "").strip() + if owner_user_id and owner_user_id != str(user_id or "").strip(): + raise PermissionError("You can only access your own conversations") + return { + "user_id": user_id, + "owner_user_id": owner_user_id, + "is_owner": True, + "collaboration_conversation_id": "", + "group_id": "", + } + + def load_operation_helpers(conversation_item, message_item, run_item=None): source = OPERATIONS_FILE.read_text(encoding="utf-8") tree = ast.parse(source, filename=str(OPERATIONS_FILE)) @@ -73,6 +87,9 @@ def load_operation_helpers(conversation_item, message_item, run_item=None): "datetime": datetime, "timezone": timezone, "CosmosResourceNotFoundError": FakeNotFound, + # Shared conversations authorize through the participation context, which owner-only + # fixtures satisfy without any collaboration linkage. + "build_conversation_participation_context": _fake_participation_context, "cosmos_conversations_container": FakeContainer({"conversation-1": conversation_item}), "cosmos_messages_container": FakeContainer({"message-1": message_item}), "cosmos_tabular_export_runs_container": FakeContainer({"run-1": run_item} if run_item else {}), @@ -83,7 +100,7 @@ def load_operation_helpers(conversation_item, message_item, run_item=None): return namespace -def load_route_helper(message_item, publication_assertion): +def load_route_helper(message_item, publication_assertion, approval_assertion=None): source = ROUTE_FILE.read_text(encoding="utf-8") tree = ast.parse(source, filename=str(ROUTE_FILE)) helper = next( @@ -94,6 +111,8 @@ def load_route_helper(message_item, publication_assertion): "CosmosResourceNotFoundError": FakeNotFound, "cosmos_conversations_container": FakeContainer({"conversation-1": {"user_id": "user-1"}}), "cosmos_messages_container": FakeContainer({"message-1": message_item}), + "build_conversation_participation_context": _fake_participation_context, + "assert_generated_file_approval_allows_download": approval_assertion or (lambda user_id, message_item: None), "assert_generated_chat_artifact_is_published_for_user": publication_assertion, } module = ast.Module(body=[helper], type_ignores=[]) diff --git a/functional_tests/test_shared_conversation_file_approval_fix.py b/functional_tests/test_shared_conversation_file_approval_fix.py new file mode 100644 index 000000000..30eb3a74c --- /dev/null +++ b/functional_tests/test_shared_conversation_file_approval_fix.py @@ -0,0 +1,688 @@ +#!/usr/bin/env python3 +# test_shared_conversation_file_approval_fix.py +""" +Functional test for shared conversation file generation and owner approvals. +Version: 0.260.004 +Implemented in: 0.260.004 + +This test ensures that a non-owner participant of a shared (collaborative) conversation can +invoke the AI without a "Forbidden" stream interruption, that downloadable files they generate +are staged for approval instead of failing, and that a staged file stays unreachable until an +authorized approver releases it. + +The application package connects to Cosmos DB at import time, so the approval module is loaded +against lightweight dependency stubs. Wiring into the routes that own each authorization gate +is verified against the real source. +""" + +import ast +import importlib.util +import os +import sys +import types + +REPO_ROOT = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) +APP_ROOT = os.path.join(REPO_ROOT, 'application', 'single_app') +TESTS_ROOT = os.path.dirname(os.path.abspath(__file__)) +if TESTS_ROOT not in sys.path: + sys.path.insert(0, TESTS_ROOT) + +from test_support.versioning import assert_app_version_at_least + + +OWNER_USER_ID = 'owner-user-001' +PARTICIPANT_USER_ID = 'participant-user-002' +OUTSIDER_USER_ID = 'outsider-user-003' +GROUP_ADMIN_USER_ID = 'group-admin-004' +GROUP_DOC_MANAGER_USER_ID = 'group-docmgr-005' +COLLABORATION_CONVERSATION_ID = 'collaboration-conversation-001' +GROUP_ID = 'group-001' + +GROUP_DOC = { + 'id': GROUP_ID, + 'owner': {'id': OWNER_USER_ID}, + 'admins': [GROUP_ADMIN_USER_ID], + 'documentManagers': [GROUP_DOC_MANAGER_USER_ID], + 'users': [ + {'userId': PARTICIPANT_USER_ID}, + {'userId': GROUP_ADMIN_USER_ID}, + {'userId': GROUP_DOC_MANAGER_USER_ID}, + {'userId': OWNER_USER_ID}, + ], +} + + +def read_app_source(file_name): + with open(os.path.join(APP_ROOT, file_name), 'r', encoding='utf-8') as source_file: + return source_file.read() + + +def extract_function_source(source_text, function_name): + parsed = ast.parse(source_text) + for node in ast.walk(parsed): + if isinstance(node, ast.FunctionDef) and node.name == function_name: + return ast.get_source_segment(source_text, node) + raise AssertionError(f'Function {function_name} not found') + + +def _stub_get_user_role_in_group(group_doc, user_id): + """Mirror functions_group.get_user_role_in_group without importing the Azure-backed module.""" + if not group_doc: + return None + if group_doc.get('owner', {}).get('id') == user_id: + return 'Owner' + if user_id in group_doc.get('admins', []): + return 'Admin' + if user_id in group_doc.get('documentManagers', []): + return 'DocumentManager' + for member in group_doc.get('users', []): + if member.get('userId') == user_id: + return 'User' + return None + + +def load_approvals_module(settings=None): + """Load functions_generated_file_approvals against dependency stubs.""" + resolved_settings = settings or {'require_shared_conversation_file_approval': True} + + config_stub = types.ModuleType('config') + config_stub.cosmos_messages_container = None + sys.modules['config'] = config_stub + + appinsights_stub = types.ModuleType('functions_appinsights') + appinsights_stub.log_event = lambda *args, **kwargs: None + sys.modules['functions_appinsights'] = appinsights_stub + + group_stub = types.ModuleType('functions_group') + group_stub.find_group_by_id = lambda group_id: GROUP_DOC if group_id == GROUP_ID else None + group_stub.get_user_role_in_group = _stub_get_user_role_in_group + sys.modules['functions_group'] = group_stub + + settings_stub = types.ModuleType('functions_settings') + settings_stub.get_settings = lambda: dict(resolved_settings) + sys.modules['functions_settings'] = settings_stub + + module_path = os.path.join(APP_ROOT, 'functions_generated_file_approvals.py') + spec = importlib.util.spec_from_file_location('functions_generated_file_approvals', module_path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +def build_participant_context(group_id=''): + return { + 'is_owner': False, + 'user_id': PARTICIPANT_USER_ID, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': group_id, + } + + +def test_participation_context_allows_shared_conversation_participants(): + """Gate 1: participants resolve access through the linked shared conversation.""" + print('Testing shared conversation participation authorization...') + source = read_app_source('functions_collaboration.py') + function_source = extract_function_source(source, 'build_conversation_participation_context') + + assert 'assert_user_can_participate_in_collaboration_conversation' in function_source, ( + 'Participants must be authorized against the linked collaboration conversation' + ) + assert 'get_collaboration_conversation_for_source' in function_source, ( + 'The helper must resolve the shared conversation from the source conversation' + ) + assert "raise PermissionError('You can only access your own conversations')" in function_source, ( + 'Unlinked personal conversations must stay owner-only' + ) + assert "'is_owner': True" in function_source and "'is_owner': False" in function_source, ( + 'The context must distinguish owners from participants for staging decisions' + ) + + print('Shared conversation participation authorization verified.') + return True + + +def test_stream_authorization_no_longer_rejects_participants(): + """Gate 1 regression: chat_stream_api must not hard-compare conversation ownership.""" + print('Testing chat stream authorization source...') + source = read_app_source('route_backend_chats.py') + + authorize_source = extract_function_source(source, '_authorize_personal_conversation_access') + assert "conversation_item.get('user_id') != user_id" not in authorize_source, ( + 'The owner-only equality check must no longer gate conversation access' + ) + assert '_resolve_authorized_conversation_context' in authorize_source, ( + 'Authorization must flow through the collaboration-aware resolver' + ) + + resolver_source = extract_function_source(source, '_resolve_authorized_conversation_context') + assert 'build_conversation_participation_context' in resolver_source, ( + 'The resolver must use the shared participation context' + ) + assert 'from functions_collaboration import build_conversation_participation_context' in source, ( + 'route_backend_chats must import the shared participation context helper' + ) + + print('Chat stream authorization verified.') + return True + + +def test_only_gated_formats_require_approval(): + """Only downloadable deliverables are gated; images and charts stay ungated.""" + print('Testing approval format scope...') + approvals = load_approvals_module() + + for gated_format in ('csv', 'xlsx', 'xls', 'xlsm', 'docx', 'pdf', 'json', 'xml'): + assert approvals.is_approval_gated_file(f'report.{gated_format}'), ( + f'{gated_format} should require approval' + ) + + for ungated_format in ('png', 'jpg', 'gif', 'svg', 'webp', 'txt', 'md'): + assert not approvals.is_approval_gated_file(f'chart.{ungated_format}'), ( + f'{ungated_format} should not require approval' + ) + + assert approvals.is_approval_gated_file('', output_format='csv'), ( + 'An explicit output format should be honored when no file name is present' + ) + + print('Approval format scope verified.') + return True + + +def test_owner_writes_are_never_gated(): + """Owners bypass the gate, participants are staged, and the admin setting disables it.""" + print('Testing owner bypass and setting toggle...') + approvals = load_approvals_module() + + owner_context = { + 'is_owner': True, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': '', + } + enabled_settings = {'require_shared_conversation_file_approval': True} + disabled_settings = {'require_shared_conversation_file_approval': False} + + assert not approvals.requires_generated_file_approval( + owner_context, file_name='report.csv', settings=enabled_settings, + ), 'Owners must never be gated' + + assert approvals.requires_generated_file_approval( + build_participant_context(), file_name='report.csv', settings=enabled_settings, + ), 'Participants generating downloadable files must be gated' + + assert not approvals.requires_generated_file_approval( + build_participant_context(), file_name='report.csv', settings=disabled_settings, + ), 'Disabling the admin setting must restore direct behavior' + + assert not approvals.requires_generated_file_approval( + build_participant_context(), file_name='diagram.png', settings=enabled_settings, + ), 'Inline image artifacts must stay ungated' + + solo_context = { + 'is_owner': False, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': '', + 'group_id': '', + } + assert not approvals.requires_generated_file_approval( + solo_context, file_name='report.csv', settings=enabled_settings, + ), 'Non-shared conversations must not create approvals' + + print('Owner bypass and setting toggle verified.') + return True + + +def test_staged_artifact_is_not_downloadable_until_approved(): + """A staged artifact must be unreachable for every caller, including the requester.""" + print('Testing staged artifact download enforcement...') + approvals = load_approvals_module() + + approval_metadata = approvals.build_generated_file_approval_metadata( + build_participant_context(), + requester={'user_id': PARTICIPANT_USER_ID, 'display_name': 'Participant'}, + ) + assert approval_metadata['generated_artifact_approval_state'] == approvals.APPROVAL_STATE_PENDING + assert approval_metadata['generated_artifact_approval_expires_at'], 'Staged files must expire' + assert approval_metadata['generated_artifact_approval_scope'] == approvals.APPROVAL_SCOPE_PERSONAL + + staged_message = {'id': 'artifact-1', 'metadata': dict(approval_metadata)} + for caller_id in (PARTICIPANT_USER_ID, OWNER_USER_ID, OUTSIDER_USER_ID): + try: + approvals.assert_generated_file_approval_allows_download(caller_id, staged_message) + raise AssertionError('Pending artifacts must never be downloadable') + except PermissionError: + pass + + denied_message = {'id': 'artifact-1', 'metadata': dict(approval_metadata)} + denied_message['metadata']['generated_artifact_approval_state'] = approvals.APPROVAL_STATE_DENIED + try: + approvals.assert_generated_file_approval_allows_download(OWNER_USER_ID, denied_message) + raise AssertionError('Denied artifacts must not be downloadable') + except PermissionError: + pass + + approved_message = {'id': 'artifact-1', 'metadata': dict(approval_metadata)} + approved_message['metadata']['generated_artifact_approval_state'] = approvals.APPROVAL_STATE_APPROVED + approvals.assert_generated_file_approval_allows_download(OWNER_USER_ID, approved_message) + approvals.assert_generated_file_approval_allows_download(PARTICIPANT_USER_ID, approved_message) + + # Artifacts written by an owner carry no approval contract and stay downloadable. + approvals.assert_generated_file_approval_allows_download(OWNER_USER_ID, {'id': 'a2', 'metadata': {}}) + + print('Staged artifact download enforcement verified.') + return True + + +def test_personal_and_group_approver_resolution(): + """Personal approvals route to the owner; group approvals route to document managers.""" + print('Testing approver resolution...') + approvals = load_approvals_module() + + personal_message = { + 'id': 'artifact-1', + 'metadata': approvals.build_generated_file_approval_metadata( + build_participant_context(), + requester={'user_id': PARTICIPANT_USER_ID}, + ), + } + assert approvals.user_can_approve_generated_file(OWNER_USER_ID, personal_message) + assert not approvals.user_can_approve_generated_file(PARTICIPANT_USER_ID, personal_message), ( + 'A requester must not approve their own file' + ) + assert not approvals.user_can_approve_generated_file(OUTSIDER_USER_ID, personal_message) + + group_metadata = approvals.build_generated_file_approval_metadata( + build_participant_context(group_id=GROUP_ID), + requester={'user_id': PARTICIPANT_USER_ID}, + ) + assert group_metadata['generated_artifact_approval_scope'] == approvals.APPROVAL_SCOPE_GROUP + group_message = {'id': 'artifact-2', 'metadata': group_metadata} + + assert approvals.user_can_approve_generated_file(OWNER_USER_ID, group_message), 'Group owner approves' + assert approvals.user_can_approve_generated_file(GROUP_ADMIN_USER_ID, group_message), 'Group admin approves' + assert approvals.user_can_approve_generated_file(GROUP_DOC_MANAGER_USER_ID, group_message), ( + 'Group document manager approves' + ) + assert not approvals.user_can_approve_generated_file(PARTICIPANT_USER_ID, group_message), ( + 'A plain group User must not approve their own file' + ) + assert not approvals.user_can_approve_generated_file(OUTSIDER_USER_ID, group_message), ( + 'Non-members must not approve' + ) + + print('Approver resolution verified.') + return True + + +def test_requester_can_never_approve_their_own_file(): + """A requester is never their own approver, whatever group role they hold.""" + print('Testing requester self-approval guard...') + approvals = load_approvals_module() + + # A group Admin or DocumentManager who is only a participant still gets staged, so the + # requester check must win over their group role. + for privileged_requester in (GROUP_ADMIN_USER_ID, GROUP_DOC_MANAGER_USER_ID, OWNER_USER_ID): + metadata = approvals.build_generated_file_approval_metadata( + { + 'is_owner': False, + 'user_id': privileged_requester, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': GROUP_ID, + }, + requester={'user_id': privileged_requester}, + ) + message_item = {'id': 'artifact-self', 'metadata': metadata} + assert not approvals.user_can_approve_generated_file(privileged_requester, message_item), ( + f'{privileged_requester} must not approve a file they requested' + ) + payload = approvals.build_generated_file_approval_client_payload( + message_item, privileged_requester, + ) + assert payload['viewer_can_approve'] is False, ( + 'The requester must not be offered approve controls' + ) + assert payload['viewer_is_requester'] is True + + # Another eligible approver must still be able to release it. + assert approvals.user_can_approve_generated_file( + GROUP_ADMIN_USER_ID if privileged_requester != GROUP_ADMIN_USER_ID else GROUP_DOC_MANAGER_USER_ID, + message_item, + ), 'A different eligible approver must still be able to release the file' + + # The same guard applies in a personal shared conversation. + personal_metadata = approvals.build_generated_file_approval_metadata( + { + 'is_owner': False, + 'user_id': OWNER_USER_ID, + 'owner_user_id': OWNER_USER_ID, + 'collaboration_conversation_id': COLLABORATION_CONVERSATION_ID, + 'group_id': '', + }, + requester={'user_id': OWNER_USER_ID}, + ) + assert not approvals.user_can_approve_generated_file( + OWNER_USER_ID, {'id': 'artifact-self-personal', 'metadata': personal_metadata}, + ), 'An owner-requester must not self-approve' + + print('Requester self-approval guard verified.') + return True + + +def test_all_artifact_blob_readers_enforce_the_approval_gate(): + """Every route that streams a stored artifact blob must consult the approval gate.""" + print('Testing artifact blob reader coverage...') + source = read_app_source('route_enhanced_citations.py') + + # The tabular citation route serves arbitrary blob-backed file messages from a conversation, + # and the source conversation owner is not necessarily an approver in a group shared + # conversation, so it must enforce the gate too. + enforcement_count = source.count('assert_generated_file_approval_allows_download(user_id, ') + assert enforcement_count >= 2, ( + 'Both the generated-artifact reader and the tabular citation reader must enforce ' + f'the approval gate (found {enforcement_count})' + ) + + tabular_index = source.index('def get_enhanced_citation_tabular') + tabular_source = source[tabular_index:source.index('@bp.route', tabular_index + 1)] + assert 'assert_generated_file_approval_allows_download' in tabular_source, ( + 'The tabular citation route must enforce the approval gate before streaming a blob' + ) + assert 'except PermissionError' in tabular_source, ( + 'The tabular citation route must return 403 rather than 500 for a withheld file' + ) + gate_index = tabular_source.index('assert_generated_file_approval_allows_download') + download_index = tabular_source.index('blob_client.download_blob()') + assert gate_index < download_index, 'The gate must run before the blob is read' + + print('Artifact blob reader coverage verified.') + return True + + +def test_pending_approval_listing_is_scoped_before_truncation(): + """The pending list must narrow to the caller's scopes before applying the row cap.""" + print('Testing pending approval listing scope...') + source = read_app_source('functions_simplechat_operations.py') + function_source = extract_function_source(source, 'list_pending_generated_file_approvals_for_user') + + assert 'generated_artifact_approval_owner_user_id = @user_id' in function_source, ( + 'Personal-scope candidates must be filtered in the query' + ) + assert 'generated_artifact_approval_group_id IN (' in function_source, ( + 'Group-scope candidates must be filtered in the query' + ) + assert 'generated_artifact_approval_requested_by_id != @user_id' in function_source, ( + 'A requester must never appear in their own approval queue' + ) + assert 'get_user_groups' in function_source, ( + 'Group scope must come from the caller group membership' + ) + + scope_index = function_source.index('scope_clauses') + query_index = function_source.index('"SELECT TOP @limit * FROM c "') + assert scope_index < query_index, 'Scope filtering must be built before the capped query' + + # Authorization is still decided by the shared predicate, not by the query alone. + assert 'user_can_approve_generated_file' in function_source, ( + 'The shared authorization predicate must still gate every returned row' + ) + + print('Pending approval listing scope verified.') + return True + + +def test_approval_decisions_are_recorded_and_single_use(): + """A decision must be recorded once and never re-applied.""" + print('Testing approval decision transitions...') + approvals = load_approvals_module() + + message_item = { + 'id': 'artifact-1', + 'metadata': approvals.build_generated_file_approval_metadata( + build_participant_context(), + requester={'user_id': PARTICIPANT_USER_ID, 'display_name': 'Participant'}, + ), + } + + pending_payload = approvals.build_generated_file_approval_client_payload(message_item, OWNER_USER_ID) + assert pending_payload['is_pending'] is True + assert pending_payload['viewer_can_approve'] is True, 'Owner should see approve controls' + + requester_payload = approvals.build_generated_file_approval_client_payload( + message_item, PARTICIPANT_USER_ID, + ) + assert requester_payload['viewer_can_approve'] is False + assert requester_payload['viewer_is_requester'] is True + + updated = approvals.apply_generated_file_approval_decision( + message_item, + approvals.APPROVAL_STATE_APPROVED, + resolver={'user_id': OWNER_USER_ID, 'display_name': 'Owner User'}, + ) + assert updated['metadata']['generated_artifact_approval_state'] == approvals.APPROVAL_STATE_APPROVED + assert updated['metadata']['generated_artifact_approval_resolved_by_name'] == 'Owner User' + assert updated['metadata']['generated_artifact_approval_resolved_at'] + + try: + approvals.apply_generated_file_approval_decision( + updated, + approvals.APPROVAL_STATE_APPROVED, + resolver={'user_id': OWNER_USER_ID}, + ) + raise AssertionError('A resolved approval must not be re-applied') + except ValueError: + pass + + try: + approvals.apply_generated_file_approval_decision( + {'id': 'ungated', 'metadata': {}}, + approvals.APPROVAL_STATE_APPROVED, + ) + raise AssertionError('Ungated artifacts must not accept approval decisions') + except ValueError: + pass + + print('Approval decision transitions verified.') + return True + + +def test_download_route_enforces_approval_before_publication(): + """Gate 4: the download route checks approval independently of the export manifest.""" + print('Testing download route enforcement...') + source = read_app_source('route_enhanced_citations.py') + function_source = extract_function_source(source, '_get_authorized_chat_artifact_message') + + assert 'build_conversation_participation_context' in function_source, ( + 'The download route must authorize shared conversation participants' + ) + assert "raise PermissionError('Forbidden')" not in function_source, ( + 'The owner-only equality check must no longer gate artifact downloads' + ) + + approval_index = function_source.index('assert_generated_file_approval_allows_download') + publication_index = function_source.index('assert_generated_chat_artifact_is_published_for_user') + assert approval_index < publication_index, ( + 'Approval must be enforced before the export manifest short-circuit' + ) + + # The original bug was an unactionable bare "Forbidden"; the download route must surface + # the specific reason so a pending file explains itself. + assert 'return jsonify({"error": str(exc) or "Forbidden"}), 403' in source, ( + 'The artifact download route must surface the specific permission message' + ) + + print('Download route enforcement verified.') + return True + + +def test_artifact_upload_stages_participant_files(): + """Gate 2: participant artifact writes are staged rather than refused.""" + print('Testing artifact staging wiring...') + source = read_app_source('functions_simplechat_operations.py') + function_source = extract_function_source(source, '_upload_generated_chat_artifact_for_current_user') + + assert 'build_conversation_participation_context' in function_source, ( + 'Artifact uploads must authorize through the shared participation context' + ) + assert 'raise PermissionError("Forbidden")' not in function_source, ( + 'Participant artifact writes must no longer fail with a bare Forbidden' + ) + assert 'requires_generated_file_approval' in function_source, ( + 'Artifact uploads must consult the approval decision' + ) + assert '**approval_metadata' in function_source, ( + 'Approval state must be persisted on the artifact message metadata' + ) + assert '_notify_generated_file_approval_requested' in function_source, ( + 'Approvers must be notified when a file is staged' + ) + + resolve_source = extract_function_source(source, 'resolve_generated_file_approval_for_user') + assert 'user_can_approve_generated_file' in resolve_source, ( + 'Approval resolution must re-authorize the acting user on every call' + ) + assert 'delete_blob_backed_chat_message_files' in resolve_source, ( + 'Denied files must have their stored blob removed' + ) + + print('Artifact staging wiring verified.') + return True + + +def test_expired_staged_files_are_auto_denied(): + """Staged files that nobody approves expire and release their storage.""" + print('Testing auto-deny sweep wiring...') + approvals_source = read_app_source('functions_generated_file_approvals.py') + assert 'APPROVAL_TTL_DAYS = 3' in approvals_source, ( + 'Staged files should expire on the same 3 day window as Control Center approvals' + ) + + operations_source = read_app_source('functions_simplechat_operations.py') + sweep_source = extract_function_source( + operations_source, 'auto_deny_expired_generated_file_approvals', + ) + assert 'list_expired_pending_generated_file_artifacts' in sweep_source + assert 'APPROVAL_STATE_AUTO_DENIED' in sweep_source + assert 'delete_blob_backed_chat_message_files' in sweep_source, ( + 'Expired staged files must not leak blob storage' + ) + + background_source = read_app_source('background_tasks.py') + assert 'auto_deny_expired_generated_file_approvals' in background_source, ( + 'The expiry sweep must be scheduled' + ) + + print('Auto-deny sweep wiring verified.') + return True + + +def test_admin_setting_is_exposed_and_safe_to_share(): + """The admin toggle exists, defaults on, and survives settings sanitization.""" + print('Testing admin setting...') + settings_source = read_app_source('functions_settings.py') + assert "'require_shared_conversation_file_approval': True," in settings_source, ( + 'The approval requirement must default to enabled' + ) + + admin_route_source = read_app_source('route_frontend_admin_settings.py') + assert "'require_shared_conversation_file_approval': form_data.get(" in admin_route_source, ( + 'The admin form must persist the approval toggle' + ) + + template_path = os.path.join(APP_ROOT, 'templates', 'admin_settings.html') + with open(template_path, 'r', encoding='utf-8') as template_file: + template_source = template_file.read() + assert 'id="require_shared_conversation_file_approval"' in template_source, ( + 'The admin settings page must expose the toggle' + ) + + # sanitize_settings_for_user drops keys containing these terms; the toggle must survive. + sensitive_terms = ('key', 'secret', 'password', 'connection', 'base64', 'storage_account_url') + setting_key = 'require_shared_conversation_file_approval' + assert not any(term in setting_key for term in sensitive_terms), ( + 'The toggle name must not collide with sanitization filters' + ) + + print('Admin setting verified.') + return True + + +def test_frontend_approval_module_is_local_and_safe(): + """The approval UI is a local asset that renders untrusted values safely.""" + print('Testing frontend approval module...') + module_path = os.path.join(APP_ROOT, 'static', 'js', 'chat', 'chat-file-approvals.js') + assert os.path.exists(module_path), 'The approval UI module must be a local static asset' + + with open(module_path, 'r', encoding='utf-8') as module_file: + module_source = module_file.read() + + assert module_source.startswith('// chat-file-approvals.js'), 'Missing filename comment' + assert 'innerHTML' not in module_source, 'Approval UI must not use innerHTML' + assert 'display:none' not in module_source and 'display: none' not in module_source, ( + 'Use Bootstrap d-none instead of inline display styles' + ) + assert 'alert(' not in module_source, 'Use Bootstrap alerts and toasts, not alert()' + assert '//cdn' not in module_source and 'https://' not in module_source, ( + 'Browser assets must not reference remote sources' + ) + + messages_path = os.path.join(APP_ROOT, 'static', 'js', 'chat', 'chat-messages.js') + with open(messages_path, 'r', encoding='utf-8') as messages_file: + messages_source = messages_file.read() + assert 'buildGeneratedFileApprovalBlock' in messages_source, ( + 'The artifact card must render the approval block' + ) + assert 'generatedFileApprovalBlocksDownload' in messages_source, ( + 'The artifact card must suppress downloads while a file is staged' + ) + + print('Frontend approval module verified.') + return True + + +def test_version_supports_shared_conversation_file_approvals(): + """The fix must be present in at least its implementation version.""" + print('Testing application version...') + assert_app_version_at_least( + '0.260.004', + reason='Shared conversation file approvals were implemented in 0.260.004', + ) + print('Application version verified.') + return True + + +if __name__ == '__main__': + tests = [ + test_participation_context_allows_shared_conversation_participants, + test_stream_authorization_no_longer_rejects_participants, + test_only_gated_formats_require_approval, + test_owner_writes_are_never_gated, + test_staged_artifact_is_not_downloadable_until_approved, + test_personal_and_group_approver_resolution, + test_requester_can_never_approve_their_own_file, + test_approval_decisions_are_recorded_and_single_use, + test_download_route_enforces_approval_before_publication, + test_all_artifact_blob_readers_enforce_the_approval_gate, + test_pending_approval_listing_is_scoped_before_truncation, + test_artifact_upload_stages_participant_files, + test_expired_staged_files_are_auto_denied, + test_admin_setting_is_exposed_and_safe_to_share, + test_frontend_approval_module_is_local_and_safe, + test_version_supports_shared_conversation_file_approvals, + ] + + results = [] + for test in tests: + print(f'\nRunning {test.__name__}...') + try: + results.append(bool(test())) + except Exception as error: + print(f'FAILED: {error}') + import traceback + traceback.print_exc() + results.append(False) + + print(f'\nResults: {sum(results)}/{len(results)} tests passed') + sys.exit(0 if all(results) else 1) diff --git a/functional_tests/test_tabular_row_orchestration_scale.py b/functional_tests/test_tabular_row_orchestration_scale.py index 504a3daaa..a8757caf7 100644 --- a/functional_tests/test_tabular_row_orchestration_scale.py +++ b/functional_tests/test_tabular_row_orchestration_scale.py @@ -1422,6 +1422,23 @@ def get_blob_client(self, container, blob): 'uuid': uuid, 'storage_account_personal_chat_container_name': 'personal-chat', '_get_latest_personal_thread_id': lambda conversation_id: None, + # Shared conversations authorize through the participation context. This fixture is a + # single-owner conversation, so no approval staging applies. + 'build_conversation_participation_context': lambda user_id, conversation_item: { + 'user_id': user_id, + 'owner_user_id': (conversation_item or {}).get('user_id', ''), + 'is_owner': True, + 'collaboration_conversation_id': '', + 'group_id': '', + }, + 'requires_generated_file_approval': lambda *args, **kwargs: False, + 'build_generated_file_approval_metadata': lambda *args, **kwargs: {}, + '_get_current_user_summary_or_none': lambda fallback_user_id='': { + 'user_id': fallback_user_id, + 'display_name': '', + 'email': '', + }, + '_notify_generated_file_approval_requested': lambda *args, **kwargs: None, 'log_event': lambda *args, **kwargs: None, } extracted_module = ast.Module(body=selected_nodes, type_ignores=[]) From 3f7a47cc6f6efe5ac2be14f184d13fd6a3261fba Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 13:08:41 -0400 Subject: [PATCH 2/5] Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- application/single_app/route_backend_collaboration.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/application/single_app/route_backend_collaboration.py b/application/single_app/route_backend_collaboration.py index c08925dbb..accd892a7 100644 --- a/application/single_app/route_backend_collaboration.py +++ b/application/single_app/route_backend_collaboration.py @@ -433,7 +433,12 @@ def list_generated_file_approvals_api(): approvals = list_pending_generated_file_approvals_for_user(current_user['user_id']) return jsonify({'approvals': approvals}) except PermissionError as exc: - return jsonify({'error': str(exc)}), 403 + log_event( + f'[GENERATED_FILE_APPROVALS] Permission denied while listing pending file approvals: {exc}', + level=logging.WARNING, + exceptionTraceback=True, + ) + return jsonify({'error': 'Permission denied'}), 403 except Exception as exc: log_event( f'[GENERATED_FILE_APPROVALS] Failed to list pending file approvals: {exc}', From b5a1c64acbb781d57294aac59762f03fb5dbce3c Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 13:08:55 -0400 Subject: [PATCH 3/5] Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- .../single_app/route_backend_collaboration.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/application/single_app/route_backend_collaboration.py b/application/single_app/route_backend_collaboration.py index accd892a7..a3246d71a 100644 --- a/application/single_app/route_backend_collaboration.py +++ b/application/single_app/route_backend_collaboration.py @@ -482,9 +482,19 @@ def resolve_generated_file_approval_api(source_conversation_id, artifact_message except LookupError: return jsonify({'error': 'Generated file approval not found'}), 404 except PermissionError as exc: - return jsonify({'error': str(exc)}), 403 + log_event( + f'[GENERATED_FILE_APPROVALS] Permission denied while resolving file approval: {exc}', + level=logging.WARNING, + exceptionTraceback=True, + ) + return jsonify({'error': 'Permission denied'}), 403 except ValueError as exc: - return jsonify({'error': str(exc)}), 400 + log_event( + f'[GENERATED_FILE_APPROVALS] Invalid request while resolving file approval: {exc}', + level=logging.WARNING, + exceptionTraceback=True, + ) + return jsonify({'error': 'Invalid request'}), 400 except Exception as exc: log_event( f'[GENERATED_FILE_APPROVALS] Failed to resolve file approval: {exc}', From e4a6b693b3e23045ea7ec3487a829850b5da86ce Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 13:09:13 -0400 Subject: [PATCH 4/5] Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- application/single_app/route_enhanced_citations.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/application/single_app/route_enhanced_citations.py b/application/single_app/route_enhanced_citations.py index 36831008e..5cdd71400 100644 --- a/application/single_app/route_enhanced_citations.py +++ b/application/single_app/route_enhanced_citations.py @@ -531,7 +531,8 @@ def get_enhanced_citation_tabular(): ) except PermissionError as exc: - return jsonify({"error": str(exc) or "Forbidden"}), 403 + debug_print(f"Forbidden serving tabular citation: {exc}") + return jsonify({"error": "Forbidden"}), 403 except Exception as e: debug_print(f"Error serving tabular citation: {e}") return jsonify({"error": str(e)}), 500 From a683cf3a0a466d3257fcaf4d5bb2d8b218b51090 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 19 Aug 2026 13:09:22 -0400 Subject: [PATCH 5/5] Potential fix for pull request finding 'CodeQL / Information exposure through an exception' Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- application/single_app/route_enhanced_citations.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/application/single_app/route_enhanced_citations.py b/application/single_app/route_enhanced_citations.py index 5cdd71400..a6057cee9 100644 --- a/application/single_app/route_enhanced_citations.py +++ b/application/single_app/route_enhanced_citations.py @@ -624,16 +624,15 @@ def download_chat_artifact(): force_download=True, ) except PermissionError as exc: - # Surface the specific reason (for example a file awaiting approval) instead of a - # bare Forbidden, which is what made the original failure so hard to act on. - return jsonify({"error": str(exc) or "Forbidden"}), 403 + debug_print(f"Forbidden chat artifact download attempt: {exc}") + return jsonify({"error": "Forbidden"}), 403 except LookupError as exc: return jsonify({"error": str(exc)}), 404 except ValueError as exc: return jsonify({"error": str(exc)}), 400 except Exception as e: debug_print(f"Error serving chat artifact download: {e}") - return jsonify({"error": str(e)}), 500 + return jsonify({"error": "An internal error has occurred"}), 500 @bp.route("/api/chat_artifacts/promote", methods=["POST"]) @swagger_route(security=get_auth_security())