diff --git a/application/single_app/config.py b/application/single_app/config.py index f2d9bfa54..0c548824e 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.017" +VERSION = "0.260.018" IS_DEVELOPMENT = is_development_env_enabled() SESSION_COOKIE_SAMESITE = os.getenv('SESSION_COOKIE_SAMESITE', 'Lax') diff --git a/application/single_app/functions_azure_endpoint_validation.py b/application/single_app/functions_azure_endpoint_validation.py new file mode 100644 index 000000000..1fbf7cd2c --- /dev/null +++ b/application/single_app/functions_azure_endpoint_validation.py @@ -0,0 +1,250 @@ +# functions_azure_endpoint_validation.py +"""Shared Azure endpoint origin validation for actions that use the application identity. + +Actions can be configured with a caller-supplied endpoint while authenticating with the +application's own workload identity. Without an origin check, a caller who can save an +action is able to direct an application-identity token to a destination they control. +Each validator here rejects endpoints that are not canonical Azure service hostnames for +a supported Azure cloud and returns a rebuilt origin, so only the validated destination +ever reaches an SDK client. + +This module is intentionally dependency-light so manifest validation, plugin construction, +and File Sync can all import it without creating import cycles. +""" + +import ipaddress +import re +from typing import Any, Iterable, Tuple +from urllib.parse import ParseResult, urlparse + +# Azure Storage service suffixes for the public, US Government, China, and Germany clouds. +AZURE_STORAGE_ENDPOINT_SUFFIXES = ( + "core.windows.net", + "core.usgovcloudapi.net", + "core.chinacloudapi.cn", + "core.cloudapi.de", +) +AZURE_COSMOS_ENDPOINT_SUFFIXES = ( + "documents.azure.com", + "documents.azure.us", + "documents.azure.cn", + "documents.microsoftazure.de", +) +AZURE_DATABRICKS_ENDPOINT_SUFFIXES = ( + "azuredatabricks.net", + "databricks.azure.us", + "databricks.azure.cn", +) +AZURE_MONITOR_QUERY_HOSTS = ( + "api.loganalytics.io", + "api.loganalytics.us", + "api.loganalytics.azure.cn", +) +# Mirrors azure.identity.AzureAuthorityHosts so a caller cannot select a token authority. +AZURE_ENTRA_AUTHORITY_HOSTS = ( + "login.microsoftonline.com", + "login.microsoftonline.us", + "login.chinacloudapi.cn", + "login.microsoftonline.de", +) + +AZURE_BLOB_SERVICE_LABEL = "blob" +AZURE_QUEUE_SERVICE_LABEL = "queue" +AZURE_FILE_SERVICE_LABEL = "file" + +MAX_ENDPOINT_LENGTH = 2048 +STORAGE_ACCOUNT_LABEL_PATTERN = re.compile(r"^[a-z0-9]{3,24}$") +DNS_LABEL_PATTERN = re.compile(r"^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?$") +DNS_NAME_PATTERN = re.compile( + r"^[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?(\.[a-z0-9]([a-z0-9-]{0,61}[a-z0-9])?)*$" +) + +AZURE_BLOB_ENDPOINT_ERROR = ( + "Blob Storage actions require an HTTPS Azure Blob service endpoint such as " + "https://account.blob.core.windows.net" +) +AZURE_QUEUE_ENDPOINT_ERROR = ( + "Queue Storage actions require an HTTPS Azure Queue service endpoint such as " + "https://account.queue.core.windows.net" +) +AZURE_COSMOS_ENDPOINT_ERROR = ( + "Cosmos actions require an HTTPS Azure Cosmos DB endpoint such as " + "https://account.documents.azure.com" +) +AZURE_DATABRICKS_ENDPOINT_ERROR = ( + "Databricks actions require an HTTPS Azure Databricks workspace endpoint such as " + "https://adb-0000000000000000.0.azuredatabricks.net" +) +AZURE_MONITOR_QUERY_ENDPOINT_ERROR = ( + "Log Analytics actions require a supported Azure Monitor query endpoint such as " + "https://api.loganalytics.io" +) +AZURE_AUTHORITY_HOST_ERROR = ( + "Log Analytics actions require a supported Microsoft Entra authority host such as " + "login.microsoftonline.com" +) + + +def _normalize_endpoint_text(value: Any) -> str: + return str(value or "").strip()[:MAX_ENDPOINT_LENGTH] + + +def _reject_unsafe_host(hostname: str, error_message: str) -> None: + """Reject local hostnames and IP literals, which are never Azure service endpoints.""" + normalized_host = hostname.strip("[]") + if ( + normalized_host in {"localhost", "localhost.localdomain"} + or normalized_host.endswith(".localhost") + ): + raise ValueError(error_message) + + try: + ipaddress.ip_address(normalized_host) + except ValueError: + return + raise ValueError(error_message) + + +def parse_azure_https_endpoint( + value: Any, + error_message: str, + allow_default_port: bool = False, +) -> Tuple[ParseResult, str]: + """Structurally validate an HTTPS endpoint and return its parsed form and hostname. + + Rejects non-HTTPS schemes, embedded credentials, explicit ports, query strings, + parameters, fragments, local hostnames, and IP literals. A bare hostname is accepted + and treated as HTTPS so stored values without a scheme still validate. + """ + raw_url = _normalize_endpoint_text(value) + if not raw_url: + raise ValueError(error_message) + if "://" not in raw_url: + raw_url = f"https://{raw_url}" + + parsed_url = urlparse(raw_url) + try: + parsed_port = parsed_url.port + except ValueError as error: + raise ValueError(error_message) from error + + if parsed_port is not None and not (allow_default_port and parsed_port == 443): + raise ValueError(error_message) + + if ( + parsed_url.scheme != "https" + or not parsed_url.hostname + or parsed_url.username is not None + or parsed_url.password is not None + or parsed_url.query + or parsed_url.fragment + or parsed_url.params + ): + raise ValueError(error_message) + + hostname = parsed_url.hostname.lower() + _reject_unsafe_host(hostname, error_message) + return parsed_url, hostname + + +def _match_endpoint_suffix( + hostname: str, + service_label: str, + endpoint_suffixes: Iterable[str], + error_message: str, +) -> Tuple[str, str]: + """Return the resource label and matched Azure cloud suffix for a hostname.""" + for endpoint_suffix in endpoint_suffixes: + host_suffix = f".{service_label}.{endpoint_suffix}" if service_label else f".{endpoint_suffix}" + if not hostname.endswith(host_suffix): + continue + resource_label = hostname[: -len(host_suffix)] + if resource_label: + return resource_label, endpoint_suffix + raise ValueError(error_message) + + +def azure_storage_endpoint_suffix_for_hostname( + hostname: Any, + service_label: str = AZURE_BLOB_SERVICE_LABEL, + error_message: str = AZURE_BLOB_ENDPOINT_ERROR, +) -> Tuple[str, str]: + """Return the storage account name and Azure cloud suffix for a service hostname.""" + normalized_hostname = str(hostname or "").strip().lower() + account_name, endpoint_suffix = _match_endpoint_suffix( + normalized_hostname, + service_label, + AZURE_STORAGE_ENDPOINT_SUFFIXES, + error_message, + ) + if not STORAGE_ACCOUNT_LABEL_PATTERN.match(account_name): + raise ValueError(error_message) + return account_name, endpoint_suffix + + +def _validate_storage_endpoint(value: Any, service_label: str, error_message: str) -> str: + _, hostname = parse_azure_https_endpoint(value, error_message) + account_name, endpoint_suffix = azure_storage_endpoint_suffix_for_hostname( + hostname, + service_label, + error_message, + ) + return f"https://{account_name}.{service_label}.{endpoint_suffix}" + + +def validate_azure_blob_endpoint(value: Any) -> str: + """Return a canonical Azure Blob service origin, or raise ValueError.""" + return _validate_storage_endpoint(value, AZURE_BLOB_SERVICE_LABEL, AZURE_BLOB_ENDPOINT_ERROR) + + +def validate_azure_queue_endpoint(value: Any) -> str: + """Return a canonical Azure Queue service origin, or raise ValueError.""" + return _validate_storage_endpoint(value, AZURE_QUEUE_SERVICE_LABEL, AZURE_QUEUE_ENDPOINT_ERROR) + + +def validate_azure_cosmos_endpoint(value: Any) -> str: + """Return a canonical Azure Cosmos DB origin, or raise ValueError.""" + _, hostname = parse_azure_https_endpoint( + value, + AZURE_COSMOS_ENDPOINT_ERROR, + allow_default_port=True, + ) + account_name, endpoint_suffix = _match_endpoint_suffix( + hostname, + "", + AZURE_COSMOS_ENDPOINT_SUFFIXES, + AZURE_COSMOS_ENDPOINT_ERROR, + ) + if not DNS_LABEL_PATTERN.match(account_name): + raise ValueError(AZURE_COSMOS_ENDPOINT_ERROR) + return f"https://{account_name}.{endpoint_suffix}" + + +def validate_azure_databricks_endpoint(value: Any) -> str: + """Return a canonical Azure Databricks workspace origin, or raise ValueError.""" + _, hostname = parse_azure_https_endpoint(value, AZURE_DATABRICKS_ENDPOINT_ERROR) + workspace_label, endpoint_suffix = _match_endpoint_suffix( + hostname, + "", + AZURE_DATABRICKS_ENDPOINT_SUFFIXES, + AZURE_DATABRICKS_ENDPOINT_ERROR, + ) + if not DNS_NAME_PATTERN.match(workspace_label): + raise ValueError(AZURE_DATABRICKS_ENDPOINT_ERROR) + return f"https://{workspace_label}.{endpoint_suffix}" + + +def validate_azure_monitor_query_endpoint(value: Any) -> str: + """Return a supported Azure Monitor query endpoint origin, or raise ValueError.""" + _, hostname = parse_azure_https_endpoint(value, AZURE_MONITOR_QUERY_ENDPOINT_ERROR) + if hostname not in AZURE_MONITOR_QUERY_HOSTS: + raise ValueError(AZURE_MONITOR_QUERY_ENDPOINT_ERROR) + return f"https://{hostname}" + + +def validate_azure_entra_authority_host(value: Any) -> str: + """Return a supported Microsoft Entra authority hostname, or raise ValueError.""" + _, hostname = parse_azure_https_endpoint(value, AZURE_AUTHORITY_HOST_ERROR) + if hostname not in AZURE_ENTRA_AUTHORITY_HOSTS: + raise ValueError(AZURE_AUTHORITY_HOST_ERROR) + return hostname diff --git a/application/single_app/functions_file_sync.py b/application/single_app/functions_file_sync.py index ceaf14d14..11809a96d 100644 --- a/application/single_app/functions_file_sync.py +++ b/application/single_app/functions_file_sync.py @@ -36,6 +36,10 @@ ) from functions_appinsights import log_event from functions_authentication import get_graph_authority, get_graph_base_url, get_graph_endpoint +from functions_azure_endpoint_validation import ( + AZURE_STORAGE_ENDPOINT_SUFFIXES, + azure_storage_endpoint_suffix_for_hostname, +) from functions_debug import debug_print from functions_documents import ( allowed_file, @@ -110,12 +114,6 @@ FILE_SYNC_SOURCE_TYPE_SHAREPOINT_ON_PREM: "On-prem SharePoint", FILE_SYNC_SOURCE_TYPE_GOOGLE_WORKSPACE: "Google Workspace", } -AZURE_STORAGE_ENDPOINT_SUFFIXES = ( - "core.windows.net", - "core.usgovcloudapi.net", - "core.chinacloudapi.cn", - "core.cloudapi.de", -) AZURE_BLOB_SAS_PERMISSION_LABELS = { "r": "Read", "l": "List", @@ -768,15 +766,10 @@ def _normalize_azure_files_connection( def _azure_blob_endpoint_suffix_for_hostname(hostname: Any) -> Tuple[str, str]: - normalized_hostname = str(hostname or "").strip().lower() - for endpoint_suffix in AZURE_STORAGE_ENDPOINT_SUFFIXES: - blob_hostname_suffix = f".blob.{endpoint_suffix}" - if not normalized_hostname.endswith(blob_hostname_suffix): - continue - account_name = normalized_hostname[:-len(blob_hostname_suffix)] - if 3 <= len(account_name) <= 24 and account_name.isascii() and account_name.isalnum(): - return account_name, endpoint_suffix - raise ValueError("Azure Blob Storage endpoint must use a supported Azure Blob service hostname") + return azure_storage_endpoint_suffix_for_hostname( + hostname, + error_message="Azure Blob Storage endpoint must use a supported Azure Blob service hostname", + ) def _normalize_azure_blob_url(value: Any) -> Tuple[str, List[str]]: diff --git a/application/single_app/functions_generated_file_exports.py b/application/single_app/functions_generated_file_exports.py index 72a38c29b..d4f444fe5 100644 --- a/application/single_app/functions_generated_file_exports.py +++ b/application/single_app/functions_generated_file_exports.py @@ -8,7 +8,7 @@ import re import tempfile from datetime import datetime -from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple +from typing import Any, Callable, Dict, Iterable, List, Optional, Sequence, Tuple from xml.etree import ElementTree from defusedxml import ElementTree as DefusedElementTree @@ -101,6 +101,25 @@ 'output', 'payload', ) +FUNCTION_RESULT_ROW_COUNT_KEYS = ( + 'row_count', + 'returned_rows', + 'total_rows', + 'total_count', + 'record_count', +) +# Below this a stored action reads as a lookup, not the dataset a request is asking for. +PRIOR_TURN_SUBSTANTIVE_ROW_COUNT = 10 +# Actions advertise partial results in different ways, so accept the common spellings. +FUNCTION_RESULT_TRUNCATION_KEYS = { + 'truncated', + 'istruncated', + 'wastruncated', + 'resultstruncated', + 'rowstruncated', +} +FUNCTION_RESULT_TRUNCATION_MAX_DEPTH = 3 +FUNCTION_RESULT_TRUNCATION_MAX_ITEMS = 20 FUNCTION_RESULT_CONTROL_KEYS = { 'count', 'detail', @@ -195,6 +214,36 @@ re.compile(r'\b(?:csv|json|xml|docx|word|pdf|spreadsheet)\b[\w\s,.:;\-/]{0,100}\b(?:export|download|file|format|copy)\b'), ) +# The one schema clarification build_csv_output_clarification_guidance() asks the model to send. +CSV_CLARIFICATION_REPLY_PATTERNS = ( + re.compile(r'\b(?:should|shall|does|do)\s+(?:each|every|the)\s+(?:csv\s+)?(?:row|record|line|entry)\b'), + re.compile(r'\b(?:which|what)\s+(?:columns?|fields?)\b'), + re.compile(r'\b(?:columns?|fields?)\s+(?:should|would|do)\s+(?:it|they|the\s+\w+)\s+include\b'), +) +CSV_CLARIFICATION_REPLY_MAX_CHARS = 500 +# A retrieval action that dwarfs the turn's other calls is the payload; the rest are lookups. +DOMINANT_FUNCTION_RESULT_MIN_ROWS = 10 +DOMINANT_FUNCTION_RESULT_RATIO = 5 +ROW_SOURCE_ASSISTANT = 'assistant response' +ROW_SOURCE_CURRENT_ACTION = 'structured function result' +ROW_SOURCE_EARLIER_ACTION = 'earlier action result' +FUNCTION_RESULT_ROW_SOURCES = frozenset({ROW_SOURCE_CURRENT_ACTION, ROW_SOURCE_EARLIER_ACTION}) +CSV_PUBLICATION_GUIDANCE = ( + 'The server serializes the authorized action results into the requested CSV artifact and ' + 'attaches the file after generation. Do not claim that you cannot create or attach files, ' + 'do not tell the user to copy or save rows manually, and do not paste a sample of the rows ' + 'in place of the artifact.' +) +# Agents commonly re-request an action from the same start time, which re-reads rows already held. +GENERATED_FILE_PAGING_GUIDANCE = ( + 'When an action reports that its results were truncated, request the remaining data with a ' + 'window that starts after the last row you already have instead of repeating the original ' + 'range, and say plainly that the data is partial if you cannot retrieve the rest.' +) +GENERATED_FILE_TRUNCATED_ROWS_NOTE = ( + 'The source action reported truncated results, so this file covers only the rows it returned.' +) + def _normalize_question_for_artifact_detection(user_question: str) -> str: return re.sub(r'\s+', ' ', str(user_question or '').strip().casefold()) @@ -397,6 +446,7 @@ def evaluate_generated_file_passthrough_eligibility( user_question: str, rows: Optional[Sequence[Dict[str, Any]]] = None, public_output_schema: Optional[Sequence[str]] = None, + carried_output_format: Optional[str] = None, ) -> Dict[str, Any]: """Return whether raw rows can satisfy a requested generated file contract.""" normalized_question = _normalize_question_for_artifact_detection(user_question) @@ -422,6 +472,9 @@ def evaluate_generated_file_passthrough_eligibility( return {'allowed': False, 'reason_code': 'derived_output_requires_transform'} if _question_requests_serialization(normalized_question): return {'allowed': True, 'reason_code': 'explicit_format_conversion'} + if carried_output_format: + # The format contract came from the earlier request this reply answers. + return {'allowed': True, 'reason_code': 'pending_format_clarification'} return {'allowed': False, 'reason_code': 'no_explicit_passthrough_contract'} @@ -436,7 +489,16 @@ def build_generated_file_output_guidance( or get_requested_structured_artifact_format(user_question) ) if output_format == GENERATED_FILE_FORMAT_CSV: - return build_csv_output_clarification_guidance(user_question) + clarification_guidance = build_csv_output_clarification_guidance(user_question) + return ' '.join( + guidance_part + for guidance_part in ( + clarification_guidance, + CSV_PUBLICATION_GUIDANCE, + GENERATED_FILE_PAGING_GUIDANCE, + ) + if guidance_part + ) if output_format == 'json': return ( 'The user requested a downloadable JSON artifact. The server will validate and attach the file after ' @@ -453,10 +515,12 @@ def build_generated_file_output_guidance( ) if output_format in {GENERATED_FILE_FORMAT_DOCX, GENERATED_FILE_FORMAT_PDF}: return ( - f'The user requested a downloadable {output_format.upper()} artifact. Provide a clear final ' - 'response grounded in the available evidence. Structured function results from this turn may ' - 'be included as labeled tables in the generated file; do not invent rows or claim an attachment ' - 'exists before the file-output finalizer publishes it.' + f'The user requested a downloadable {output_format.upper()} artifact. The server renders and ' + 'attaches the file after generation. Provide a clear final response grounded in the available ' + 'evidence. Structured function results from this turn may be included as labeled tables in the ' + 'generated file. Do not claim that you cannot create or attach files, do not tell the user to ' + 'copy or save the content manually, do not invent rows, and do not claim an attachment exists ' + f'before the file-output finalizer publishes it. {GENERATED_FILE_PAGING_GUIDANCE}' ) return '' @@ -479,32 +543,56 @@ def build_generated_file_export( user_question: str, assistant_content: str, function_results: Optional[List[Dict[str, Any]]] = None, + prior_function_results_loader: Optional[Callable[[], Optional[List[Dict[str, Any]]]]] = None, + pending_output_format: Optional[str] = None, ) -> Optional[Dict[str, Any]]: """Build a generated file payload from final assistant content and function-result evidence.""" - output_format = get_requested_generated_file_format(user_question) + output_format = get_requested_generated_file_format(user_question) or _normalize_pending_output_format( + pending_output_format, + ) if output_format not in GENERATED_FILE_FORMATS: return None + carried_output_format = ( + output_format if not get_requested_generated_file_format(user_question) else None + ) assistant_text = str(assistant_content or '').strip() assistant_rows = extract_assistant_table_entries(assistant_text) + if not assistant_rows and _assistant_reply_requests_clarification(assistant_text): + return None function_rows = extract_authorized_function_result_rows(function_results) + row_provenance = ROW_SOURCE_CURRENT_ACTION + rows_truncated = function_results_report_truncated_rows(function_results) if function_rows else False + if not assistant_rows and not function_rows and prior_function_results_loader is not None: + # This turn answered from evidence gathered earlier, so reuse those rows instead of none. + prior_function_results = prior_function_results_loader() + function_rows = extract_authorized_function_result_rows(prior_function_results) + if function_rows: + row_provenance = ROW_SOURCE_EARLIER_ACTION + rows_truncated = function_results_report_truncated_rows(prior_function_results) function_passthrough = evaluate_generated_file_passthrough_eligibility( user_question, rows=function_rows, + carried_output_format=carried_output_format, ) if function_rows else {'allowed': False, 'reason_code': 'source_result_incomplete'} if output_format == GENERATED_FILE_FORMAT_CSV: - rows = assistant_rows or (function_rows if function_passthrough.get('allowed') else []) + use_function_rows = bool(function_rows) and bool(function_passthrough.get('allowed')) and ( + not assistant_rows + or _assistant_rows_sample_function_rows(assistant_rows, function_rows) + ) + rows = function_rows if use_function_rows else assistant_rows if not rows: return None - row_source = 'assistant response' if assistant_rows else 'structured function result' + row_source = row_provenance if use_function_rows else ROW_SOURCE_ASSISTANT return _build_generated_file_payload( output_format=output_format, file_content=build_assistant_table_csv(rows), rows=rows, row_source=row_source, assistant_content=assistant_text, - passthrough_reason_code=(None if assistant_rows else function_passthrough.get('reason_code')), + passthrough_reason_code=(function_passthrough.get('reason_code') if use_function_rows else None), + rows_truncated=rows_truncated if use_function_rows else False, ) if not assistant_text and not assistant_rows and not function_rows: @@ -512,7 +600,7 @@ def build_generated_file_export( rows = function_rows if function_rows and function_passthrough.get('allowed') else assistant_rows if not assistant_text and not rows: return None - row_source = 'structured function result' if rows and rows is function_rows else 'assistant response' + row_source = row_provenance if rows and rows is function_rows else ROW_SOURCE_ASSISTANT title = _build_generated_file_title(output_format) if output_format == GENERATED_FILE_FORMAT_DOCX: file_content = _render_docx_file_export(title, assistant_text, rows, row_source) @@ -525,31 +613,66 @@ def build_generated_file_export( row_source=row_source, assistant_content=assistant_text, title=title, - passthrough_reason_code=(function_passthrough.get('reason_code') if row_source == 'structured function result' else None), + passthrough_reason_code=( + function_passthrough.get('reason_code') if row_source in FUNCTION_RESULT_ROW_SOURCES else None + ), + rows_truncated=rows_truncated if row_source in FUNCTION_RESULT_ROW_SOURCES else False, ) -def extract_authorized_function_result_rows(function_results: Optional[List[Dict[str, Any]]]) -> List[Dict[str, Any]]: - """Return structured rows from successful current-turn non-tabular function results.""" - function_row_groups: List[Tuple[str, List[Dict[str, Any]]]] = [] - for function_result in function_results or []: - if not isinstance(function_result, dict): - continue - if function_result.get('success') is False or _is_tabular_function_result(function_result): - continue +def build_structured_artifact_rows_payload( + user_question: str, + output_format: str, + function_results: Optional[List[Dict[str, Any]]] = None, + prior_function_results_loader: Optional[Callable[[], Optional[List[Dict[str, Any]]]]] = None, +) -> Optional[Dict[str, Any]]: + """Serialize authorized action rows when a JSON or XML reply did not carry the payload.""" + normalized_output_format = str(output_format or '').strip().lower() + if normalized_output_format not in {'json', 'xml'}: + return None + + function_rows = extract_authorized_function_result_rows(function_results) + row_provenance = ROW_SOURCE_CURRENT_ACTION + rows_truncated = function_results_report_truncated_rows(function_results) if function_rows else False + if not function_rows and prior_function_results_loader is not None: + prior_function_results = prior_function_results_loader() + function_rows = extract_authorized_function_result_rows(prior_function_results) + if function_rows: + row_provenance = ROW_SOURCE_EARLIER_ACTION + rows_truncated = function_results_report_truncated_rows(prior_function_results) + if not function_rows: + return None + + function_passthrough = evaluate_generated_file_passthrough_eligibility( + user_question, + rows=function_rows, + ) + if not function_passthrough.get('allowed'): + return None + + if normalized_output_format == 'json': + file_content = serialize_generated_json(function_rows) + else: + file_content = serialize_generated_xml(function_rows, root_name='GeneratedRows') + return { + 'file_content': file_content, + 'rows': function_rows, + 'row_count': len(function_rows), + 'row_source': row_provenance, + 'passthrough_reason_code': function_passthrough.get('reason_code'), + 'rows_truncated': rows_truncated, + } - structured_rows = _extract_function_result_rows( - _parse_function_result_payload(function_result.get('function_result')), - ) - if not structured_rows: - continue - function_row_groups.append(( - _get_function_result_label(function_result), - structured_rows, - )) +def extract_authorized_function_result_rows(function_results: Optional[List[Dict[str, Any]]]) -> List[Dict[str, Any]]: + """Return structured rows from successful non-tabular action results.""" + function_row_groups = _collect_authorized_function_row_groups(function_results) if not function_row_groups: return [] + + dominant_rows = _select_dominant_function_row_group(function_row_groups) + if dominant_rows is not None: + return dominant_rows if len(function_row_groups) == 1: return function_row_groups[0][1] @@ -563,6 +686,286 @@ def extract_authorized_function_result_rows(function_results: Optional[List[Dict return combined_rows +def _collect_authorized_function_row_groups( + function_results: Optional[List[Dict[str, Any]]], +) -> List[Tuple[str, List[Dict[str, Any]]]]: + """Group authorized rows by action, so paged calls to one action stay one dataset.""" + grouped_rows: Dict[str, List[Dict[str, Any]]] = {} + # Agents often re-page an action from the same start, so pages overlap rather than extend. + seen_group_rows: Dict[str, set] = {} + for function_result in function_results or []: + if not isinstance(function_result, dict): + continue + if function_result.get('success') is False or _is_tabular_function_result(function_result): + continue + + structured_rows = _extract_function_result_rows( + _parse_function_result_payload(function_result.get('function_result')), + ) + if not structured_rows: + continue + function_label = _get_function_result_label(function_result) + group_rows = grouped_rows.setdefault(function_label, []) + seen_rows = seen_group_rows.setdefault(function_label, set()) + page_signatures = set() + for structured_row in structured_rows: + row_signature = _build_function_result_row_signature(structured_row) + # Repeats inside one response are records the action counted, so only drop rows an + # earlier page of the same action already contributed. + if row_signature is not None: + if row_signature in seen_rows: + continue + page_signatures.add(row_signature) + group_rows.append(structured_row) + seen_rows.update(page_signatures) + return list(grouped_rows.items()) + + +def _build_function_result_row_signature(row: Any) -> Optional[Tuple[Tuple[str, str], ...]]: + if not isinstance(row, dict): + return None + try: + return tuple(sorted( + (_normalize_function_result_key(key), _serialize_row_signature_value(value)) + for key, value in row.items() + )) + except TypeError: + return None + + +def _serialize_row_signature_value(value: Any) -> str: + if isinstance(value, (dict, list, tuple, set)): + return json.dumps(value, default=str, sort_keys=True) + return '' if value is None else str(value) + + +def function_results_report_truncated_rows( + function_results: Optional[List[Dict[str, Any]]], +) -> bool: + """Report whether a row-contributing action said it returned only part of the matching data.""" + for function_result in function_results or []: + if not isinstance(function_result, dict): + continue + if function_result.get('success') is False or _is_tabular_function_result(function_result): + continue + payload = _parse_function_result_payload(function_result.get('function_result')) + if not _extract_function_result_rows(payload): + continue + if _payload_reports_truncation(payload): + return True + return False + + +def _payload_reports_truncation(payload: Any, depth: int = 0) -> bool: + if depth > FUNCTION_RESULT_TRUNCATION_MAX_DEPTH: + return False + if isinstance(payload, dict): + for key, value in payload.items(): + if _normalize_function_result_key(key) not in FUNCTION_RESULT_TRUNCATION_KEYS: + continue + if value is True or str(value).strip().casefold() == 'true': + return True + return any( + _payload_reports_truncation(value, depth + 1) + for value in payload.values() + if isinstance(value, (dict, list)) + ) + if isinstance(payload, list): + return any( + _payload_reports_truncation(item, depth + 1) + for item in payload[:FUNCTION_RESULT_TRUNCATION_MAX_ITEMS] + if isinstance(item, (dict, list)) + ) + return False + + +def _select_dominant_function_row_group( + function_row_groups: Sequence[Tuple[str, List[Dict[str, Any]]]], +) -> Optional[List[Dict[str, Any]]]: + """Return the retrieval result set when the turn's other actions are only lookups.""" + if len(function_row_groups) < 2: + return None + + dominant_label, dominant_rows = max(function_row_groups, key=lambda group: len(group[1])) + if len(dominant_rows) < DOMINANT_FUNCTION_RESULT_MIN_ROWS: + return None + + supporting_row_count = sum( + len(rows) for label, rows in function_row_groups if label != dominant_label + ) + if len(dominant_rows) < supporting_row_count * DOMINANT_FUNCTION_RESULT_RATIO: + return None + return dominant_rows + + +def select_prior_turn_action_citations( + assistant_messages: Optional[Sequence[Dict[str, Any]]], + substantive_row_count: int = PRIOR_TURN_SUBSTANTIVE_ROW_COUNT, +) -> List[Dict[str, Any]]: + """Pick the newest stored action group holding a dataset, reading compact citations only. + + Callers pass assistant messages newest first and hydrate only the returned citations. + """ + fallback_citations: List[Dict[str, Any]] = [] + for assistant_message in assistant_messages or []: + if not isinstance(assistant_message, dict): + continue + compact_citations = assistant_message.get('agent_citations') + if not isinstance(compact_citations, list): + continue + + grouped_citations: Dict[str, Dict[str, Any]] = {} + for citation in compact_citations: + if not isinstance(citation, dict): + continue + estimated_rows = estimate_function_result_row_count(citation) + if not estimated_rows: + continue + action_label = str( + citation.get('function_name') or citation.get('plugin_name') or '', + ).strip() + action_group = grouped_citations.setdefault( + action_label, + {'citations': [], 'estimated_rows': 0}, + ) + action_group['citations'].append(citation) + action_group['estimated_rows'] += estimated_rows + + if not grouped_citations: + continue + best_group = max(grouped_citations.values(), key=lambda group: group['estimated_rows']) + if best_group['estimated_rows'] >= substantive_row_count: + return list(best_group['citations']) + if not fallback_citations: + fallback_citations = list(best_group['citations']) + return fallback_citations + + +def estimate_function_result_row_count(function_result: Optional[Dict[str, Any]]) -> int: + """Estimate an action's stored rows from its compacted citation, reading no payload.""" + if not isinstance(function_result, dict): + return 0 + if function_result.get('success') is False or _is_tabular_function_result(function_result): + return 0 + return _estimate_payload_row_count( + _parse_function_result_payload(function_result.get('function_result')), + ) + + +def _estimate_payload_row_count(payload: Any, depth: int = 0) -> int: + if depth > 4: + return 0 + if isinstance(payload, list): + estimated_rows = 0 + for item in payload: + # Compaction replaces the dropped tail of a list with this marker. + if isinstance(item, dict) and set(item) == {'remaining_items'}: + estimated_rows += _coerce_row_count(item.get('remaining_items')) + else: + estimated_rows += 1 + return estimated_rows + if not isinstance(payload, dict): + return 0 + + normalized_keys = {_normalize_function_result_key(key): key for key in payload} + for count_key in FUNCTION_RESULT_ROW_COUNT_KEYS: + matching_key = normalized_keys.get(_normalize_function_result_key(count_key)) + declared_row_count = _coerce_row_count(payload.get(matching_key)) if matching_key else 0 + if declared_row_count: + return declared_row_count + + for row_key in FUNCTION_RESULT_ROW_KEYS: + matching_key = normalized_keys.get(_normalize_function_result_key(row_key)) + if matching_key is None: + continue + estimated_rows = _estimate_payload_row_count(payload.get(matching_key), depth + 1) + if estimated_rows: + return estimated_rows + return 1 if _normalize_function_result_row(payload, is_data_row=False) else 0 + + +def _coerce_row_count(value: Any) -> int: + try: + row_count = int(value) + except (TypeError, ValueError): + return 0 + return row_count if row_count > 0 else 0 + + +def _normalize_pending_output_format(pending_output_format: Optional[str]) -> Optional[str]: + normalized_format = str(pending_output_format or '').strip().lower() + return normalized_format if normalized_format in GENERATED_FILE_FORMATS else None + + +def resolve_pending_generated_file_format( + user_question: str, + previous_assistant_content: str, +) -> Optional[str]: + """Carry one unanswered CSV schema clarification forward to the reply that answers it.""" + if not str(user_question or '').strip(): + return None + if get_requested_artifact_formats(user_question): + return None + if not _assistant_reply_requests_clarification(previous_assistant_content): + return None + return GENERATED_FILE_FORMAT_CSV + + +def _assistant_reply_requests_clarification(assistant_content: str) -> bool: + """Return whether the reply is the schema clarification rather than the answer.""" + normalized_content = _normalize_question_for_artifact_detection(assistant_content) + if '?' not in normalized_content or len(normalized_content) > CSV_CLARIFICATION_REPLY_MAX_CHARS: + return False + return any(pattern.search(normalized_content) for pattern in CSV_CLARIFICATION_REPLY_PATTERNS) + + +def _normalized_row_text_map(row: Dict[str, Any]) -> Dict[str, str]: + return { + _normalize_function_result_key(column_name): ( + '' if value is None else str(value).strip() + ) + for column_name, value in row.items() + } + + +def _assistant_rows_sample_function_rows( + assistant_rows: Sequence[Dict[str, Any]], + function_rows: Sequence[Dict[str, Any]], +) -> bool: + """Return whether the assistant table is an excerpt of the same authorized rows.""" + if not assistant_rows or len(assistant_rows) >= len(function_rows): + return False + + assistant_row_maps = [] + assistant_columns = [] + seen_columns = set() + for assistant_row in assistant_rows: + if not isinstance(assistant_row, dict): + return False + row_map = _normalized_row_text_map(assistant_row) + assistant_row_maps.append(row_map) + for column_name in row_map: + if column_name and column_name not in seen_columns: + seen_columns.add(column_name) + assistant_columns.append(column_name) + if not assistant_columns: + return False + + function_projections = set() + for function_row in function_rows: + if not isinstance(function_row, dict): + return False + row_map = _normalized_row_text_map(function_row) + if not seen_columns.issubset(row_map): + return False + function_projections.add(tuple(row_map[column_name] for column_name in assistant_columns)) + + return all( + tuple(row_map.get(column_name, '') for column_name in assistant_columns) in function_projections + for row_map in assistant_row_maps + ) + + def has_generated_file_output(existing_outputs: Optional[List[Dict[str, Any]]], output_format: str) -> bool: """Return whether an existing generated artifact already covers an output format.""" normalized_output_format = str(output_format or '').strip().lower() @@ -616,6 +1019,8 @@ 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 + if export_payload.get('rows_truncated'): + artifact_metadata['rows_truncated'] = True # 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. @@ -638,6 +1043,7 @@ def _build_generated_file_payload( assistant_content: str, title: str = '', passthrough_reason_code: Optional[str] = None, + rows_truncated: bool = False, ) -> Dict[str, Any]: normalized_output_format = str(output_format or '').strip().lower() row_count = len(rows or []) @@ -651,12 +1057,14 @@ def _build_generated_file_payload( 'preview_rows': list(rows or [])[:GENERATED_FILE_PREVIEW_ROWS], 'preview_lines': _build_preview_lines(assistant_content), 'row_source': row_source, + 'rows_truncated': bool(rows_truncated), '_structured_rows': list(rows or []), 'summary': _build_generated_file_summary( normalized_output_format, row_count, row_source, normalized_title, + bool(rows_truncated), ), } if passthrough_reason_code: @@ -678,9 +1086,14 @@ def _build_generated_file_summary( row_count: int, row_source: str, title: str, + rows_truncated: bool = False, ) -> str: row_detail = f' with {row_count} structured row(s)' if row_count else '' - return f'Prepared {title}{row_detail} from the {row_source}.' + summary = f'Prepared {title}{row_detail} from the {row_source}.' + if rows_truncated: + # The action capped its own response, so the file is a partial view of the matching data. + summary = f'{summary} {GENERATED_FILE_TRUNCATED_ROWS_NOTE}' + return summary def _build_preview_lines(assistant_content: str) -> List[str]: @@ -950,7 +1363,7 @@ def _format_structured_cell(value: Any) -> str: def _build_structured_rows_heading(row_source: str) -> str: - if row_source == 'structured function result': + if row_source in FUNCTION_RESULT_ROW_SOURCES: return 'Structured function results' return 'Structured response rows' diff --git a/application/single_app/json_schema_validation.py b/application/single_app/json_schema_validation.py index f59c7c229..9bc76e24c 100644 --- a/application/single_app/json_schema_validation.py +++ b/application/single_app/json_schema_validation.py @@ -2,6 +2,7 @@ # Utility for loading and validating JSON schemas for agents and plugins import os import json +import re from functools import lru_cache from jsonschema import validate, ValidationError, Draft7Validator, Draft6Validator, RefResolver @@ -61,6 +62,72 @@ def validate_agent(agent): return None +def normalize_plugin_definition_type(plugin_type): + """Return the filesystem-safe definition name for a plugin type.""" + return re.sub(r'[^a-zA-Z0-9_]', '_', str(plugin_type or '')).lower() + + +@lru_cache(maxsize=64) +def get_allowed_auth_types_for_plugin_type(plugin_type): + """Return the auth types a plugin type supports. + + The per-type ``.definition.json`` file is authoritative when present. Otherwise the + shared ``AuthType`` enum in ``plugin.schema.json`` is used, so unknown types keep working. + """ + normalized_type = normalize_plugin_definition_type(plugin_type) + if normalized_type: + definition_path = os.path.join(SCHEMA_DIR, f'{normalized_type}.definition.json') + if os.path.exists(definition_path): + try: + with open(definition_path, encoding='utf-8') as definition_file: + definition = json.load(definition_file) + allowed_auth_types = definition.get('allowedAuthTypes') + if isinstance(allowed_auth_types, list) and allowed_auth_types: + return frozenset(str(auth_type) for auth_type in allowed_auth_types) + except (OSError, ValueError): + pass + + schema = load_schema('plugin.schema.json') + enum_values = schema.get('definitions', {}).get('AuthType', {}).get('enum', []) + return frozenset(str(auth_type) for auth_type in enum_values) + + +def validate_plugin_auth_type_allowed(plugin): + """Return an error message when a manifest declares an auth type its plugin type disallows. + + Without this check a caller can declare any auth type on any action type, including + ``identity``, which makes the application authenticate with its own workload identity to a + caller-supplied destination. + + Identity-bound manifests are exempt because their auth type is resolved server-side from a + stored workspace identity rather than declared by the caller, and that hydration legitimately + rewrites ``auth.type`` into values a definition file does not list. + """ + if not isinstance(plugin, dict): + return None + + additional_fields = plugin.get('additionalFields') if isinstance(plugin.get('additionalFields'), dict) else {} + if str(plugin.get('identity_id') or '').strip() or str(additional_fields.get('identity_id') or '').strip(): + return None + + auth = plugin.get('auth') if isinstance(plugin.get('auth'), dict) else {} + declared_auth_type = str(auth.get('type') or '').strip() + if not declared_auth_type: + return None + + plugin_type = str(plugin.get('type') or '').strip().lower() + if plugin_type == DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE: + plugin_type = DATABRICKS_PLUGIN_TYPE + + allowed_auth_types = get_allowed_auth_types_for_plugin_type(plugin_type) + if not allowed_auth_types or declared_auth_type in allowed_auth_types: + return None + + return ( + f"Authentication type '{declared_auth_type}' is not supported by action type '{plugin_type}'." + ) + + def apply_plugin_validation_defaults(plugin): plugin_copy = plugin.copy() if isinstance(plugin, dict) else {} plugin_type = str(plugin_copy.get('type', '') or '').strip().lower() diff --git a/application/single_app/route_backend_chats.py b/application/single_app/route_backend_chats.py index 1587cd4ad..13411322e 100644 --- a/application/single_app/route_backend_chats.py +++ b/application/single_app/route_backend_chats.py @@ -150,7 +150,9 @@ build_generated_file_artifact_metadata, build_generated_file_export, build_generated_file_output_guidance, + build_structured_artifact_rows_payload, evaluate_generated_file_passthrough_eligibility, + estimate_function_result_row_count, get_generated_file_export_content, get_requested_generated_file_format, get_requested_structured_artifact_format, @@ -158,6 +160,8 @@ normalize_json_artifact_payload, normalize_generated_output_format, normalize_xml_artifact_payload, + resolve_pending_generated_file_format, + select_prior_turn_action_citations, serialize_generated_json, serialize_generated_xml, ) @@ -2257,6 +2261,127 @@ def _has_generated_tabular_csv_output(generated_outputs): return has_generated_tabular_csv_output(generated_outputs) +PRIOR_TURN_ACTION_RESULT_MAX_MESSAGES = 40 +PRIOR_TURN_ACTION_RESULT_ARTIFACT_LIMIT = 25 + + +def _resolve_prior_turn_history_window(settings=None): + """Reach back as far as the history the model itself can still see.""" + resolved_settings = settings if isinstance(settings, dict) else get_settings() + return _bounded_int( + resolved_settings.get('conversation_history_limit', 10), + default=10, + minimum=1, + maximum=PRIOR_TURN_ACTION_RESULT_MAX_MESSAGES, + ) + + +def _read_recent_assistant_messages(conversation_id, message_limit): + query = ( + f'SELECT TOP {int(message_limit)} c.id, c.content, c.agent_citations FROM c ' + 'WHERE c.conversation_id = @conversation_id AND c.role = @role ' + 'ORDER BY c.timestamp DESC' + ) + return list(cosmos_messages_container.query_items( + query=query, + parameters=[ + {'name': '@conversation_id', 'value': conversation_id}, + {'name': '@role', 'value': 'assistant'}, + ], + partition_key=conversation_id, + )) + + +def _read_agent_citation_artifact_payloads(conversation_id, artifact_ids): + if not artifact_ids: + return {} + artifact_documents = list(cosmos_messages_container.query_items( + query=( + 'SELECT * FROM c WHERE c.conversation_id = @conversation_id ' + 'AND (ARRAY_CONTAINS(@artifact_ids, c.id) ' + 'OR ARRAY_CONTAINS(@artifact_ids, c.parent_message_id))' + ), + parameters=[ + {'name': '@conversation_id', 'value': conversation_id}, + {'name': '@artifact_ids', 'value': artifact_ids}, + ], + partition_key=conversation_id, + )) + return build_message_artifact_payload_map(artifact_documents) + + +def _load_prior_turn_function_results(user_id, conversation_id, settings=None): + """Return the full action results an earlier turn already gathered in this conversation.""" + normalized_user_id = str(user_id or '').strip() + normalized_conversation_id = str(conversation_id or '').strip() + if not normalized_user_id or not normalized_conversation_id: + return [] + + try: + _authorize_personal_conversation_access(normalized_user_id, normalized_conversation_id) + assistant_messages = _read_recent_assistant_messages( + normalized_conversation_id, + _resolve_prior_turn_history_window(settings), + ) + selected_citations = select_prior_turn_action_citations( + assistant_messages, + )[:PRIOR_TURN_ACTION_RESULT_ARTIFACT_LIMIT] + if not selected_citations: + return [] + + artifact_payload_map = _read_agent_citation_artifact_payloads( + normalized_conversation_id, + [ + str(citation.get('artifact_id') or '').strip() + for citation in selected_citations + if str(citation.get('artifact_id') or '').strip() + ], + ) + + resolved_citations = [] + for citation in selected_citations: + artifact_payload = artifact_payload_map.get(str(citation.get('artifact_id') or '')) + stored_citation = artifact_payload.get('citation') if isinstance(artifact_payload, dict) else None + resolved_citations.append(stored_citation if isinstance(stored_citation, dict) else citation) + return resolved_citations + except Exception as exc: + log_event( + '[GENERATED_FILE_EXPORT] Could not reuse earlier action results', + { + 'conversation_id': normalized_conversation_id, + 'error': str(exc), + }, + debug_only=True, + ) + return [] + + +def _resolve_pending_generated_file_format(user_question, conversation_id, user_id): + """Return the artifact format still owed from an unanswered schema clarification.""" + try: + _authorize_personal_conversation_access(str(user_id or '').strip(), str(conversation_id or '').strip()) + recent_assistant_messages = _read_recent_assistant_messages(str(conversation_id or '').strip(), 1) + except Exception: + return None + if not recent_assistant_messages: + return None + return resolve_pending_generated_file_format( + user_question, + recent_assistant_messages[0].get('content') or '', + ) + + +def _resolve_generated_file_guidance_format(user_question, conversation_id, user_id): + """Return the artifact format this reply owes, including one carried from a clarification.""" + requested_format = str(get_tabular_generated_output_format(user_question) or '').strip().lower() + if requested_format or not conversation_id: + return requested_format + # A reply that only answers "which rows and columns?" still owes the originally requested file, + # so the publication contract has to reach the model on this turn too. + pending_format = _resolve_pending_generated_file_format(user_question, conversation_id, user_id) + return str(pending_format or '').strip().lower() + + def maybe_create_generated_file_output( user_question, assistant_content, @@ -2273,6 +2398,12 @@ def maybe_create_generated_file_output( request_correlation_id=request_correlation_id, ) output_format = get_requested_generated_file_format(user_question) + if not output_format: + output_format = _resolve_pending_generated_file_format( + user_question, + conversation_id, + get_current_user_id(), + ) if not output_format: return None if output_format == 'csv' and _has_generated_tabular_csv_output(existing_outputs): @@ -2284,6 +2415,12 @@ def maybe_create_generated_file_output( user_question, assistant_content, function_results=function_results, + prior_function_results_loader=lambda: _load_prior_turn_function_results( + get_current_user_id(), + conversation_id, + settings=get_settings(), + ), + pending_output_format=output_format, ) if not export_payload: return None @@ -2508,11 +2645,26 @@ def _build_streaming_assistant_file_status(output_format): return f'Generating the {normalized_output_format} file. It will appear here when ready.' +def _build_structured_artifact_rows_payload(user_question, output_format, conversation_id, function_results): + """Fall back to authorized action rows when a JSON/XML reply carried no payload.""" + return build_structured_artifact_rows_payload( + user_question, + output_format, + function_results=function_results, + prior_function_results_loader=lambda: _load_prior_turn_function_results( + get_current_user_id(), + conversation_id, + settings=get_settings(), + ), + ) + + def maybe_create_assistant_file_generated_output( user_question, assistant_content, conversation_id, existing_outputs=None, + function_results=None, ): """Save assistant-generated JSON/XML content as a downloadable chat artifact.""" output_format = get_tabular_generated_output_format(user_question) @@ -2525,20 +2677,40 @@ def maybe_create_assistant_file_generated_output( preview_items = [] preview_lines = [] + row_payload = None if output_format == 'json': json_payload = normalize_json_artifact_payload(assistant_content) if json_payload is None: - return None - file_content = serialize_generated_json(json_payload) - if isinstance(json_payload, list): - preview_items = json_payload[:3] - elif isinstance(json_payload, dict): - preview_items = [json_payload] + row_payload = _build_structured_artifact_rows_payload( + user_question, + output_format, + conversation_id, + function_results, + ) + if row_payload is None: + return None + file_content = row_payload['file_content'] + preview_items = row_payload['rows'][:3] + else: + file_content = serialize_generated_json(json_payload) + if isinstance(json_payload, list): + preview_items = json_payload[:3] + elif isinstance(json_payload, dict): + preview_items = [json_payload] else: xml_payload = normalize_xml_artifact_payload(assistant_content) if not xml_payload: - return None - file_content = xml_payload + row_payload = _build_structured_artifact_rows_payload( + user_question, + output_format, + conversation_id, + function_results, + ) + if row_payload is None: + return None + file_content = row_payload['file_content'] + else: + file_content = xml_payload preview_lines = _build_assistant_file_preview_lines(file_content) generated_file_name = _build_assistant_file_export_name(output_format) @@ -15404,6 +15576,7 @@ def execute_document_action_chat_request( assistant_content=document_action_reply_content, conversation_id=conversation_id, existing_outputs=document_generated_analysis_artifacts + document_generated_tabular_outputs, + function_results=execution_result.get('agent_citations') or [], ) if assistant_file_generated_output: document_generated_analysis_artifacts.append(assistant_file_generated_output) @@ -16116,7 +16289,11 @@ def result_requires_message_reload(result: Any) -> bool: system_messages_for_augmentation = [] # Collect system messages from search generated_file_output_guidance = build_generated_file_output_guidance( user_message, - requested_format=get_tabular_generated_output_format(user_message), + requested_format=_resolve_generated_file_guidance_format( + user_message, + conversation_id, + user_id, + ), ) if generated_file_output_guidance: system_messages_for_augmentation.append({ @@ -19844,6 +20021,7 @@ def gpt_error(e): assistant_content=ai_message, conversation_id=conversation_id, existing_outputs=generated_analysis_artifacts_list + generated_tabular_outputs_list, + function_results=agent_citations_list, ) if assistant_file_generated_output: generated_analysis_artifacts_list.append(assistant_file_generated_output) @@ -20468,9 +20646,11 @@ def stream_cancel_requested(): generated_tabular_outputs_list = [] generated_analysis_artifacts_list = [] system_messages_for_augmentation = [] - requested_streamed_file_format = str( - get_tabular_generated_output_format(user_message) or '' - ).strip().lower() + requested_streamed_file_format = _resolve_generated_file_guidance_format( + user_message, + conversation_id, + user_id, + ) suppress_streamed_file_payload = requested_streamed_file_format in {'json', 'xml'} streamed_file_status_content = _build_streaming_assistant_file_status( requested_streamed_file_format @@ -23792,6 +23972,7 @@ def finalize_cancelled_agent_stream_response(): assistant_content=accumulated_content, conversation_id=conversation_id, existing_outputs=generated_analysis_artifacts_list + generated_tabular_outputs_list, + function_results=agent_citations_list, ) if assistant_file_generated_output: generated_analysis_artifacts_list.append(assistant_file_generated_output) diff --git a/application/single_app/route_backend_plugins.py b/application/single_app/route_backend_plugins.py index 309a80753..305b6ea30 100644 --- a/application/single_app/route_backend_plugins.py +++ b/application/single_app/route_backend_plugins.py @@ -58,7 +58,14 @@ #from functions_personal_actions import delete_personal_action from functions_debug import debug_print -from json_schema_validation import PLUGIN_STORAGE_MANAGED_FIELDS, apply_plugin_validation_defaults, validate_plugin +from functions_azure_endpoint_validation import validate_azure_cosmos_endpoint +from json_schema_validation import ( + PLUGIN_STORAGE_MANAGED_FIELDS, + apply_plugin_validation_defaults, + get_allowed_auth_types_for_plugin_type, + normalize_plugin_definition_type, + validate_plugin, +) from functions_activity_logging import ( log_action_creation, log_action_update, @@ -1868,41 +1875,14 @@ def get_plugin_auth_types(plugin_type): otherwise falls back to AuthType enum in plugin.schema.json. """ schema_dir = os.path.join(current_app.root_path, 'static', 'json', 'schemas') - safe_type = re.sub(r'[^a-zA-Z0-9_]', '_', plugin_type).lower() + safe_type = normalize_plugin_definition_type(plugin_type) definition_path = os.path.join(schema_dir, f'{safe_type}.definition.json') - schema_path = os.path.join(schema_dir, 'plugin.schema.json') - - allowed_auth_types = [] - source = "schema" - - try: - with open(schema_path, 'r', encoding='utf-8') as schema_file: - schema = json.load(schema_file) - allowed_auth_types = ( - schema - .get('definitions', {}) - .get('AuthType', {}) - .get('enum', []) - ) - except Exception as exc: - debug_print(f"Failed to read plugin.schema.json: {exc}") - allowed_auth_types = [] - if os.path.exists(definition_path): - try: - with open(definition_path, 'r', encoding='utf-8') as definition_file: - definition = json.load(definition_file) - allowed_from_definition = definition.get('allowedAuthTypes') - if isinstance(allowed_from_definition, list) and allowed_from_definition: - allowed_auth_types = allowed_from_definition - source = "definition" - except Exception as exc: - debug_print(f"Failed to read {definition_path}: {exc}") - - if not allowed_auth_types: - allowed_auth_types = [] - source = "schema" + # Resolved through the same helper the save paths enforce, so the modal cannot offer an + # auth type the backend will later reject. + allowed_auth_types = sorted(get_allowed_auth_types_for_plugin_type(plugin_type)) + source = "definition" if os.path.exists(definition_path) else "schema" return jsonify({ "allowedAuthTypes": allowed_auth_types, @@ -2477,6 +2457,12 @@ def test_cosmos_connection(): if not endpoint: return jsonify({'success': False, 'error': 'Cosmos DB account endpoint is required.'}), 400 + try: + # This route builds a Cosmos client directly, so the endpoint is validated here as well + # as in the action manifest and plugin paths. + endpoint = validate_azure_cosmos_endpoint(endpoint) + except ValueError as exc: + return jsonify({'success': False, 'error': str(exc)}), 400 if not database_name: return jsonify({'success': False, 'error': 'Database name is required.'}), 400 if not container_name: diff --git a/application/single_app/semantic_kernel_plugins/blob_storage_plugin.py b/application/single_app/semantic_kernel_plugins/blob_storage_plugin.py index 91b4fd31a..b5bc8fa51 100644 --- a/application/single_app/semantic_kernel_plugins/blob_storage_plugin.py +++ b/application/single_app/semantic_kernel_plugins/blob_storage_plugin.py @@ -11,6 +11,7 @@ from functions_appinsights import log_event from functions_debug import debug_print +from functions_azure_endpoint_validation import validate_azure_blob_endpoint from functions_blob_storage_operations import ( BLOB_STORAGE_CAPABILITY_DEFINITIONS, BLOB_STORAGE_PLUGIN_TYPE, @@ -201,10 +202,19 @@ def _build_service_client(self) -> BlobServiceClient: f"container={self.container_name} auth_type={self.auth_type} prefix={self.blob_prefix or ''}" ) if self.auth_type == "connection_string": + # Revalidate the derived endpoint so a stored connection string cannot point the + # SDK at a non-Storage origin. + validate_azure_blob_endpoint( + derive_blob_endpoint_from_connection_string(self.connection_string) or self.endpoint + ) return BlobServiceClient.from_connection_string(self.connection_string) + + # Revalidated immediately before client construction so actions stored before this + # check, or mutated afterwards, cannot receive application credentials. + validated_endpoint = validate_azure_blob_endpoint(self.endpoint) if self.auth_type == "identity": - return BlobServiceClient(account_url=self.endpoint, credential=DefaultAzureCredential()) - return BlobServiceClient(account_url=self.endpoint, credential=self.auth_key) + return BlobServiceClient(account_url=validated_endpoint, credential=DefaultAzureCredential()) + return BlobServiceClient(account_url=validated_endpoint, credential=self.auth_key) def _resolve_effective_prefix(self, prefix: str = "") -> str: default_prefix = self.blob_prefix diff --git a/application/single_app/semantic_kernel_plugins/cosmos_query_plugin.py b/application/single_app/semantic_kernel_plugins/cosmos_query_plugin.py index 714cf7c6f..99a779f3c 100644 --- a/application/single_app/semantic_kernel_plugins/cosmos_query_plugin.py +++ b/application/single_app/semantic_kernel_plugins/cosmos_query_plugin.py @@ -15,6 +15,7 @@ from semantic_kernel.functions import kernel_function from functions_appinsights import log_event +from functions_azure_endpoint_validation import validate_azure_cosmos_endpoint from semantic_kernel_plugins.base_plugin import BasePlugin from semantic_kernel_plugins.plugin_invocation_logger import plugin_function_logger @@ -310,8 +311,10 @@ def _get_container_client(self): client_cache_key = self._get_client_cache_key() client = self._client_cache.get(client_cache_key) if client is None: + # Revalidated immediately before client construction so a stored endpoint + # cannot direct application credentials to a non-Cosmos origin. client = azure_cosmos.CosmosClient( - self.endpoint, + validate_azure_cosmos_endpoint(self.endpoint), credential=self._get_client_credential(), timeout=self.timeout, connection_timeout=self.timeout, @@ -326,6 +329,8 @@ def _validate_configuration(self) -> None: missing_fields = [] if not self.endpoint: missing_fields.append("endpoint") + else: + validate_azure_cosmos_endpoint(self.endpoint) if not self.database_name: missing_fields.append("database_name") if not self.container_name: diff --git a/application/single_app/semantic_kernel_plugins/databricks_plugin.py b/application/single_app/semantic_kernel_plugins/databricks_plugin.py index f95d1ef4e..ec54ce08d 100644 --- a/application/single_app/semantic_kernel_plugins/databricks_plugin.py +++ b/application/single_app/semantic_kernel_plugins/databricks_plugin.py @@ -15,6 +15,7 @@ from functions_appinsights import log_event from functions_debug import debug_print +from functions_azure_endpoint_validation import validate_azure_databricks_endpoint from functions_databricks_operations import ( DATABRICKS_ALLOWED_READ_STATEMENTS, DATABRICKS_AZURE_COMMERCIAL_TOKEN_SCOPE, @@ -154,6 +155,8 @@ def _validate_configuration(self) -> None: parsed_endpoint = urlparse(self.endpoint) if parsed_endpoint.scheme != "https" or not parsed_endpoint.netloc: raise ValueError("Databricks action requires an HTTPS workspace URL endpoint.") + # Revalidated before any managed-identity token is minted for this workspace URL. + validate_azure_databricks_endpoint(self.endpoint) if not self.warehouse_id: raise ValueError("Databricks action requires additionalFields.warehouse_id.") if self.auth_type not in {"key", "identity", "servicePrincipal"}: @@ -178,6 +181,8 @@ def _get_access_token(self) -> str: client_secret=str(self._auth.get("key") or ""), ) return credential.get_token(DATABRICKS_AZURE_COMMERCIAL_TOKEN_SCOPE).token + # Revalidate before minting an application-identity token for this workspace URL. + validate_azure_databricks_endpoint(self.endpoint) credential = DefaultAzureCredential() return credential.get_token(DATABRICKS_AZURE_COMMERCIAL_TOKEN_SCOPE).token diff --git a/application/single_app/semantic_kernel_plugins/log_analytics_plugin.py b/application/single_app/semantic_kernel_plugins/log_analytics_plugin.py index 7c5a69326..d6d24fd55 100644 --- a/application/single_app/semantic_kernel_plugins/log_analytics_plugin.py +++ b/application/single_app/semantic_kernel_plugins/log_analytics_plugin.py @@ -11,6 +11,10 @@ from semantic_kernel_plugins.base_plugin import BasePlugin from semantic_kernel.functions import kernel_function from semantic_kernel_plugins.plugin_invocation_logger import plugin_function_logger +from functions_azure_endpoint_validation import ( + validate_azure_entra_authority_host, + validate_azure_monitor_query_endpoint, +) from config import * import json import logging @@ -53,7 +57,9 @@ def __init__(self, manifest: Optional[Dict[str, Any]] = None): def _init_client(self): # Determine authority host for the selected cloud if self.cloud == "custom": - authority_host = self.authority_host + # A custom cloud lets the manifest choose the token authority, so it is revalidated + # before any credential is built for it. + authority_host = validate_azure_entra_authority_host(self.authority_host) elif self.cloud == "usgovernment": authority_host = AzureAuthorityHosts.AZURE_GOVERNMENT else: @@ -107,7 +113,13 @@ def get_token(self, *args, **kwargs): return AccessToken(token, expires_on) if self.cloud == "custom": - scope = f"{self.endpoint_override}/.default" if self.endpoint_override else "https://api.loganalytics.io/.default" + # The override becomes the delegated-token OAuth scope, so it must be a + # documented Azure Monitor query resource rather than a caller-chosen one. + scope = ( + f"{validate_azure_monitor_query_endpoint(self.endpoint_override)}/.default" + if self.endpoint_override + else "https://api.loganalytics.io/.default" + ) elif self.cloud == "usgovernment": scope = "https://api.loganalytics.us/.default" else: @@ -121,7 +133,11 @@ def get_token(self, *args, **kwargs): # Endpoint selection: prefer additionalFields.cloud, then manifest["endpoint"], then fallback endpoint = None if self.cloud == "custom": - endpoint = self.endpoint_override or self.manifest.get("endpoint") + endpoint = ( + validate_azure_monitor_query_endpoint(self.endpoint_override) + if self.endpoint_override + else self.manifest.get("endpoint") + ) elif self.cloud == "usgovernment": endpoint = "https://api.loganalytics.us" elif self.cloud == "public": diff --git a/application/single_app/semantic_kernel_plugins/plugin_health_checker.py b/application/single_app/semantic_kernel_plugins/plugin_health_checker.py index 358f13d43..4d7088676 100644 --- a/application/single_app/semantic_kernel_plugins/plugin_health_checker.py +++ b/application/single_app/semantic_kernel_plugins/plugin_health_checker.py @@ -10,8 +10,19 @@ from urllib.parse import urlparse from semantic_kernel_plugins.base_plugin import BasePlugin from functions_appinsights import log_event +from functions_azure_endpoint_validation import ( + validate_azure_blob_endpoint, + validate_azure_cosmos_endpoint, + validate_azure_databricks_endpoint, + validate_azure_entra_authority_host, + validate_azure_monitor_query_endpoint, + validate_azure_queue_endpoint, +) from functions_azure_maps import AZURE_MAPS_DEFAULT_ENDPOINT, AZURE_MAPS_PLUGIN_TYPE -from functions_blob_storage_operations import BLOB_STORAGE_PLUGIN_TYPE +from functions_blob_storage_operations import ( + BLOB_STORAGE_PLUGIN_TYPE, + derive_blob_endpoint_from_connection_string, +) from functions_databricks_operations import ( DATABRICKS_CLOUD_AZURE_COMMERCIAL, DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE, @@ -73,6 +84,7 @@ validate_mcp_endpoint_for_transport, ) from functions_simplechat_operations import SIMPLECHAT_DEFAULT_ENDPOINT +from json_schema_validation import validate_plugin_auth_type_allowed from semantic_kernel_plugins.rocksdb_plugin import ( AUTH_SCHEME_NONE, MAX_RESULTS_CEILING, @@ -88,7 +100,23 @@ class PluginHealthChecker: """Utility class for checking plugin health and validity.""" - + + @staticmethod + def _endpoint_origin_errors(endpoint, validator): + """Return validation errors for an endpoint that receives application credentials. + + Actions that authenticate with the application identity must only reach canonical Azure + service origins. Without this check a caller-supplied endpoint would receive a token + minted for the application's own workload identity. + """ + if not str(endpoint or '').strip(): + return [] + try: + validator(endpoint) + except ValueError as exc: + return [str(exc)] + return [] + @staticmethod def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tuple[bool, List[str]]: """ @@ -113,13 +141,24 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl for field in required_fields: if field not in manifest: errors.append(f"Missing required field: {field}") - + + auth_type_error = validate_plugin_auth_type_allowed(manifest) + if auth_type_error: + errors.append(auth_type_error) + # Validate specific plugin types if plugin_type in ['azure_function', 'queue_storage']: if 'endpoint' not in manifest: errors.append(f"Plugin type '{plugin_type}' requires 'endpoint' field") if 'auth' not in manifest: errors.append(f"Plugin type '{plugin_type}' requires 'auth' field") + if plugin_type == 'queue_storage': + errors.extend( + PluginHealthChecker._endpoint_origin_errors( + manifest.get('endpoint'), + validate_azure_queue_endpoint, + ) + ) elif plugin_type == BLOB_STORAGE_PLUGIN_TYPE: additional_fields = manifest.get('additionalFields', {}) @@ -139,8 +178,17 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl errors.append("Blob storage plugin requires 'container_name' in additionalFields") if auth_type not in {'connection_string', 'identity', 'key'}: errors.append("Blob storage plugin requires auth.type values 'connection_string', 'identity', or 'key'") - if auth_type == 'connection_string' and not auth.get('key'): - errors.append("Blob storage plugin requires auth.key when auth.type='connection_string'") + if auth_type == 'connection_string': + if not auth.get('key'): + errors.append("Blob storage plugin requires auth.key when auth.type='connection_string'") + else: + derived_endpoint = derive_blob_endpoint_from_connection_string(auth.get('key') or '') + errors.extend( + PluginHealthChecker._endpoint_origin_errors( + derived_endpoint or endpoint, + validate_azure_blob_endpoint, + ) + ) if auth_type == 'key': if not endpoint: errors.append("Blob storage plugin requires an 'endpoint' field when auth.type='key'") @@ -148,6 +196,10 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl errors.append("Blob storage plugin requires auth.key when auth.type='key'") if auth_type == 'identity' and not endpoint: errors.append("Blob storage plugin requires an 'endpoint' field when auth.type='identity'") + if auth_type in {'identity', 'key'} and endpoint: + errors.extend( + PluginHealthChecker._endpoint_origin_errors(endpoint, validate_azure_blob_endpoint) + ) elif plugin_type in {DATABRICKS_PLUGIN_TYPE, DATABRICKS_LEGACY_TABLE_PLUGIN_TYPE}: auth = manifest.get('auth', {}) if isinstance(manifest.get('auth'), dict) else {} @@ -164,6 +216,10 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl errors.append("Databricks plugin currently supports only additionalFields.cloud='azure_commercial'") if parsed_endpoint.scheme != 'https' or not parsed_endpoint.netloc: errors.append("Databricks plugin requires an HTTPS workspace endpoint") + else: + errors.extend( + PluginHealthChecker._endpoint_origin_errors(endpoint, validate_azure_databricks_endpoint) + ) if not additional_fields.get('warehouse_id'): errors.append("Databricks plugin requires additionalFields.warehouse_id") if auth_type not in {'key', 'identity', 'servicePrincipal'}: @@ -347,6 +403,10 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl if not endpoint: errors.append("Cosmos plugin requires an 'endpoint' field") + else: + errors.extend( + PluginHealthChecker._endpoint_origin_errors(endpoint, validate_azure_cosmos_endpoint) + ) if not database_name: errors.append("Cosmos plugin requires 'database_name' in additionalFields") if not container_name: @@ -363,9 +423,28 @@ def validate_plugin_manifest(manifest: Dict[str, Any], plugin_type: str) -> Tupl elif plugin_type == 'log_analytics': additional_fields = manifest.get('additionalFields', {}) + if not isinstance(additional_fields, dict): + additional_fields = {} if 'workspaceId' not in additional_fields: errors.append("Log Analytics plugin requires 'workspaceId' in additionalFields") + # A custom cloud lets the manifest choose the token authority and the query endpoint. + # For delegated auth the endpoint override also becomes the OAuth scope, so both values + # must be constrained to documented Azure hosts. + if str(additional_fields.get('cloud') or '').strip().lower() == 'custom': + errors.extend( + PluginHealthChecker._endpoint_origin_errors( + additional_fields.get('authorityHost'), + validate_azure_entra_authority_host, + ) + ) + errors.extend( + PluginHealthChecker._endpoint_origin_errors( + additional_fields.get('endpointOverride'), + validate_azure_monitor_query_endpoint, + ) + ) + elif plugin_type == 'simplechat': endpoint = manifest.get('endpoint') auth = manifest.get('auth', {}) if isinstance(manifest.get('auth'), dict) else {} diff --git a/application/single_app/semantic_kernel_plugins/queue_storage_plugin.py b/application/single_app/semantic_kernel_plugins/queue_storage_plugin.py index 58e918bcd..04eeaf041 100644 --- a/application/single_app/semantic_kernel_plugins/queue_storage_plugin.py +++ b/application/single_app/semantic_kernel_plugins/queue_storage_plugin.py @@ -4,6 +4,7 @@ from semantic_kernel.functions import kernel_function from semantic_kernel_plugins.plugin_invocation_logger import plugin_function_logger from azure.identity import DefaultAzureCredential +from functions_azure_endpoint_validation import validate_azure_queue_endpoint class QueueStoragePlugin(BasePlugin): def __init__(self, manifest: Dict[str, Any]): @@ -16,12 +17,15 @@ def __init__(self, manifest: Dict[str, Any]): self._metadata = manifest.get('metadata', {}) if not self.endpoint or not self.auth_type or not self.queue_name: raise ValueError("QueueStoragePlugin requires 'endpoint', 'queue_name', and 'auth.type' in the manifest.") + # Revalidated immediately before client construction so a stored endpoint cannot direct + # application credentials to a non-Storage origin. + validated_endpoint = validate_azure_queue_endpoint(self.endpoint) if self.auth_type == 'identity': - self.queue_client = QueueClient(account_url=self.endpoint, queue_name=self.queue_name, credential=DefaultAzureCredential()) + self.queue_client = QueueClient(account_url=validated_endpoint, queue_name=self.queue_name, credential=DefaultAzureCredential()) elif self.auth_type == 'key': if not self.key: raise ValueError("QueueStoragePlugin requires 'auth.key' when using key authentication.") - self.queue_client = QueueClient(account_url=self.endpoint, queue_name=self.queue_name, credential=self.key) + self.queue_client = QueueClient(account_url=validated_endpoint, queue_name=self.queue_name, credential=self.key) else: raise ValueError(f"Unsupported auth.type: {self.auth_type}") diff --git a/application/single_app/static/js/admin/admin_settings.js b/application/single_app/static/js/admin/admin_settings.js index d30fabf78..722a3d49f 100644 --- a/application/single_app/static/js/admin/admin_settings.js +++ b/application/single_app/static/js/admin/admin_settings.js @@ -1508,7 +1508,12 @@ function validateCosmosThroughputSettings(options = {}) { setCosmosThroughputValidationMessage(errors); if (options.report && invalidFields.length > 0) { - document.getElementById('scale-tab')?.click(); + // Navigate to wherever the invalid field actually lives rather than to + // a named tab, so this keeps working when settings are regrouped. + const owningCard = invalidFields[0].closest('.card[id]'); + if (owningCard && typeof window.openAdminCard === 'function') { + window.openAdminCard(owningCard.id); + } invalidFields[0].focus({ preventScroll: false }); invalidFields[0].reportValidity(); } @@ -10048,111 +10053,55 @@ function findPreviousApplicableStep(currentStep) { } /** - * Navigate to the appropriate tab based on the walkthrough step + * Navigate to the setting a walkthrough step is about. + * + * Steps used to name a tab id directly, which meant the same knowledge was + * recorded twice and went stale the moment a setting moved to another tab. + * Steps now name the card they are about, and openAdminCard finds the owning + * tab from the page itself, so this cannot drift again. + * * @param {number} stepNumber - The current step number */ function handleTabNavigation(stepNumber) { - // Map steps to tabs that need to be activated - const stepToTab = { - 1: 'general-tab', // App title and logo (General tab) - 2: 'ai-models-tab', // GPT settings (now in AI Models tab) - 3: 'ai-models-tab', // GPT model selection (now in AI Models tab) - 4: 'workspaces-tab', // Workspace and groups settings - 5: 'ai-models-tab', // Embedding settings (now in AI Models tab) - 6: 'search-extract-tab', // AI Search settings - 7: 'search-extract-tab', // Document Intelligence settings - 8: 'search-extract-tab', // Video support - 9: 'search-extract-tab', // Audio support - 10: 'safety-tab', // Content safety - 11: 'safety-tab', // User feedback and archiving (changed from system-tab) - 12: 'citation-tab' // Enhanced Citations and Image Generation + // card: the setting this step is about. + // focus: an optional finer scroll target inside that card. + const stepToCard = { + 1: { card: 'branding-section' }, + 2: { card: 'multi-endpoint-configuration' }, + 3: { card: 'multi-endpoint-configuration' }, + 4: { card: 'personal-workspaces-section' }, + 5: { card: 'embeddings-configuration' }, + 6: { card: 'azure-ai-search-section' }, + 7: { card: 'document-intelligence-section' }, + 8: { card: 'video-intelligence-section', focus: 'enable_video_file_support' }, + 9: { card: 'ai-voice-chat-section', focus: 'enable_audio_file_support' }, + 10: { card: 'content-safety-section' }, + 11: { card: 'user-feedback-section' }, + 12: { card: 'enhanced-citations-section' }, }; - - // Activate the appropriate tab - const tabId = stepToTab[stepNumber]; - if (tabId) { - // Check if we're using sidebar navigation or tab navigation - const sidebarToggle = document.getElementById('admin-settings-toggle'); - - if (sidebarToggle) { - // Using sidebar navigation - call showAdminTab function - const tabName = tabId.replace('-tab', ''); // Remove '-tab' suffix - if (typeof showAdminTab === 'function') { - showAdminTab(tabName); - } else if (typeof window.showAdminTab === 'function') { - window.showAdminTab(tabName); - } - } else { - // Using Bootstrap tabs - const tab = document.getElementById(tabId); - if (tab) { - // Use bootstrap Tab to show the tab - const bootstrapTab = new bootstrap.Tab(tab); - bootstrapTab.show(); - } - } - - // Scroll to the relevant section after a small delay to allow tab to switch - setTimeout(() => { - scrollToRelevantSection(stepNumber, tabId); - }, 300); + + const target = stepToCard[stepNumber]; + if (!target) { + return; } -} -/** - * Scroll to relevant section within a tab based on the step - * @param {number} stepNumber - The current step number - * @param {string} tabId - The ID of the tab that was activated - */ -function scrollToRelevantSection(stepNumber, tabId) { - // Define which sections to scroll to for each step - let targetElement = null; - - switch (stepNumber) { - case 1: // App title and logo - targetElement = document.getElementById('branding-section'); - break; - case 2: // GPT settings - targetElement = document.getElementById('gpt-configuration'); - break; - case 3: // GPT model selection - targetElement = document.getElementById('gpt_models_list')?.closest('.mb-3'); - break; - case 4: // Workspaces toggle section - targetElement = document.getElementById('personal-workspaces-section'); - break; - case 5: // Embedding settings - targetElement = document.getElementById('embeddings-configuration'); - break; - case 6: // AI Search settings - targetElement = document.getElementById('azure-ai-search-section'); - break; - case 7: // Document Intelligence settings - targetElement = document.getElementById('document-intelligence-section'); - break; - case 8: // Video file support - targetElement = document.getElementById('enable_video_file_support')?.closest('.form-group'); - break; - case 9: // Audio file support - targetElement = document.getElementById('enable_audio_file_support')?.closest('.form-group'); - break; - case 10: // Content safety - targetElement = document.getElementById('content-safety-section'); - break; - case 11: // User feedback and archiving - targetElement = document.getElementById('user-feedback-section'); - break; - case 12: // Enhanced citations and image generation - targetElement = document.getElementById('enhanced-citations-section'); - break; - default: - // For other steps, no specific scrolling - break; + const opened = typeof window.openAdminCard === 'function' + ? window.openAdminCard(target.card) + : false; + + if (!opened) { + console.warn(`handleTabNavigation: could not open card "${target.card}" for step ${stepNumber}`); + return; } - - // If we found a target element, scroll to it - if (targetElement) { - targetElement.scrollIntoView({ behavior: 'smooth', block: 'start' }); + + if (target.focus) { + // openAdminCard scrolls to the card first, so refine afterwards. + window.setTimeout(() => { + const element = document.getElementById(target.focus); + if (element) { + element.scrollIntoView({ behavior: 'smooth', block: 'center' }); + } + }, 400); } } diff --git a/application/single_app/static/js/chat/chat-messages.js b/application/single_app/static/js/chat/chat-messages.js index 5df9d3986..7faec1a20 100644 --- a/application/single_app/static/js/chat/chat-messages.js +++ b/application/single_app/static/js/chat/chat-messages.js @@ -5347,6 +5347,15 @@ function renderReplyQuoteHtml(fullMessageObject = null) { backgroundDetailElements.push(rowCountDetail); } + if (outputMetadata?.rows_truncated) { + // The completed-artifact layout hides the summary, so partial coverage needs its own badge. + const truncatedBadge = document.createElement('span'); + truncatedBadge.className = 'badge text-bg-warning'; + truncatedBadge.textContent = 'Partial'; + truncatedBadge.title = 'The source action reported truncated results, so this file covers only the rows it returned.'; + header.appendChild(truncatedBadge); + } + card.appendChild(header); if (!isCompletedTabularArtifact) { diff --git a/application/single_app/static/js/plugin_modal_stepper.js b/application/single_app/static/js/plugin_modal_stepper.js index 66b213ff5..ccacd6e9f 100644 --- a/application/single_app/static/js/plugin_modal_stepper.js +++ b/application/single_app/static/js/plugin_modal_stepper.js @@ -15,6 +15,12 @@ const TABLEAU_ACTION_IDENTITY_AUTH_TYPES = ['api_key', 'username_password']; const YAMCS_ACTION_IDENTITY_AUTH_TYPES = ['api_key', 'bearer_token', 'username_password']; const LOG_ANALYTICS_ACTION_IDENTITY_AUTH_TYPES = ['client_secret', 'managed_identity']; const BLOB_STORAGE_PLUGIN_TYPE = 'blob_storage'; +const AZURE_STORAGE_ENDPOINT_SUFFIXES = [ + 'core.windows.net', + 'core.usgovcloudapi.net', + 'core.chinacloudapi.cn', + 'core.cloudapi.de' +]; const DATABRICKS_PLUGIN_TYPE = 'databricks'; const DATABRICKS_DEFAULT_CLOUD = 'azure_commercial'; const SNOWFLAKE_PLUGIN_TYPE = 'snowflake'; @@ -770,6 +776,11 @@ export class PluginModalStepper { document.getElementById('sql-identity-select').addEventListener('change', () => this.handleActionIdentityChange('sql')); document.getElementById('cosmos-auth-type').addEventListener('change', () => this.handleCosmosAuthTypeChange()); + const blobStorageAuthTypeSelect = document.getElementById('blob-storage-auth-type'); + if (blobStorageAuthTypeSelect) { + blobStorageAuthTypeSelect.addEventListener('change', () => this.handleBlobStorageAuthTypeChange()); + } + const rocksDbAuthSchemeSelect = document.getElementById('rocksdb-auth-scheme'); if (rocksDbAuthSchemeSelect) { rocksDbAuthSchemeSelect.addEventListener('change', () => this.handleRocksDbAuthSchemeChange()); @@ -2053,6 +2064,35 @@ export class PluginModalStepper { return String(prefix || '').trim().replace(/^\/+|\/+$/g, ''); } + isAzureBlobEndpoint(endpoint = '') { + // Mirrors the server-side allowlist so the modal fails fast with a clear message. + const normalizedEndpoint = String(endpoint || '').trim(); + if (!normalizedEndpoint) { + return false; + } + + let parsedUrl; + try { + parsedUrl = new URL(normalizedEndpoint.includes('://') ? normalizedEndpoint : `https://${normalizedEndpoint}`); + } catch (err) { + return false; + } + + if (parsedUrl.protocol !== 'https:' || parsedUrl.port || parsedUrl.username || parsedUrl.password + || parsedUrl.search || parsedUrl.hash) { + return false; + } + + return AZURE_STORAGE_ENDPOINT_SUFFIXES.some(suffix => { + const hostSuffix = `.blob.${suffix}`; + if (!parsedUrl.hostname.toLowerCase().endsWith(hostSuffix)) { + return false; + } + const accountName = parsedUrl.hostname.toLowerCase().slice(0, -hostSuffix.length); + return /^[a-z0-9]{3,24}$/.test(accountName); + }); + } + deriveBlobStorageEndpointFromConnectionString(connectionString = '') { const normalizedConnectionString = String(connectionString || '').trim(); if (!normalizedConnectionString || normalizedConnectionString === 'Stored_In_KeyVault') { @@ -3994,16 +4034,31 @@ export class PluginModalStepper { return false; } } else if (isBlobStorageVisible) { + const blobAuthType = this.getBlobStorageAuthType(); const connectionString = document.getElementById('blob-storage-connection-string').value.trim(); + const blobEndpoint = document.getElementById('blob-storage-endpoint')?.value.trim() || ''; + const blobAccountKey = document.getElementById('blob-storage-account-key')?.value.trim() || ''; const containerName = document.getElementById('blob-storage-container-name').value.trim(); const capabilityValues = Object.values(this.getSelectedBlobStorageCapabilities()); const readTypeValues = Object.values(this.getSelectedBlobStorageReadFileTypes()); const uploadTypeValues = Object.values(this.getSelectedBlobStorageUploadFileTypes()); - if (!connectionString) { + if (blobAuthType === 'connection_string' && !connectionString) { this.showError('Blob storage connection string is required.'); return false; } + if (blobAuthType !== 'connection_string' && !blobEndpoint) { + this.showError('Blob service endpoint is required.'); + return false; + } + if (blobAuthType !== 'connection_string' && !this.isAzureBlobEndpoint(blobEndpoint)) { + this.showError('Blob service endpoint must be an Azure Blob service URL, for example https://account.blob.core.windows.net.'); + return false; + } + if (blobAuthType === 'key' && !blobAccountKey) { + this.showError('Blob storage account key is required.'); + return false; + } if (!containerName) { this.showError('Blob storage container name is required.'); return false; @@ -5235,8 +5290,26 @@ export class PluginModalStepper { }; } + getBlobStorageAuthType() { + return document.getElementById('blob-storage-auth-type')?.value || 'connection_string'; + } + + handleBlobStorageAuthTypeChange() { + const authType = this.getBlobStorageAuthType(); + const connectionStringGroup = document.getElementById('blob-storage-connection-string-group'); + const endpointGroup = document.getElementById('blob-storage-endpoint-group'); + const accountKeyGroup = document.getElementById('blob-storage-account-key-group'); + + connectionStringGroup?.classList.toggle('d-none', authType !== 'connection_string'); + endpointGroup?.classList.toggle('d-none', authType === 'connection_string'); + accountKeyGroup?.classList.toggle('d-none', authType !== 'key'); + } + getBlobStorageConfiguration() { + const authType = this.getBlobStorageAuthType(); const connectionString = document.getElementById('blob-storage-connection-string')?.value.trim() || ''; + const endpoint = document.getElementById('blob-storage-endpoint')?.value.trim() || ''; + const accountKey = document.getElementById('blob-storage-account-key')?.value.trim() || ''; const containerName = document.getElementById('blob-storage-container-name')?.value.trim() || ''; const blobPrefix = this.normalizeBlobStoragePrefix(document.getElementById('blob-storage-blob-prefix')?.value || ''); @@ -5250,6 +5323,24 @@ export class PluginModalStepper { additionalFields.blob_prefix = blobPrefix; } + if (authType === 'identity') { + return { + endpoint, + auth: { type: 'identity', identity: 'managed_identity' }, + additionalFields, + identityId: '' + }; + } + + if (authType === 'key') { + return { + endpoint, + auth: { type: 'key', key: accountKey }, + additionalFields, + identityId: '' + }; + } + return { endpoint: this.deriveBlobStorageEndpointFromConnectionString(connectionString) || this.originalPlugin?.endpoint || '', auth: { type: 'connection_string', key: connectionString }, @@ -5535,9 +5626,15 @@ export class PluginModalStepper { if (!blobStorageConfig.additionalFields.container_name) { throw new Error('Enter the container name before testing the connection.'); } - if (!blobStorageConfig.auth.key && !this.isEditMode) { + if (blobStorageConfig.auth.type === 'connection_string' && !blobStorageConfig.auth.key && !this.isEditMode) { throw new Error('Enter the storage connection string before testing the connection.'); } + if (blobStorageConfig.auth.type !== 'connection_string' && !blobStorageConfig.endpoint) { + throw new Error('Enter the blob service endpoint before testing the connection.'); + } + if (blobStorageConfig.auth.type === 'key' && !blobStorageConfig.auth.key && !this.isEditMode) { + throw new Error('Enter the storage account key before testing the connection.'); + } return blobStorageConfig; } @@ -6249,10 +6346,24 @@ export class PluginModalStepper { } else if (this.isBlobStorageType(plugin.type)) { const additionalFields = plugin.additionalFields || plugin.additional_fields || {}; const auth = plugin.auth || {}; + const blobAuthType = ['identity', 'key'].includes(auth.type) ? auth.type : 'connection_string'; - document.getElementById('blob-storage-connection-string').value = auth.key || ''; + const blobAuthTypeField = document.getElementById('blob-storage-auth-type'); + if (blobAuthTypeField) { + blobAuthTypeField.value = blobAuthType; + } + document.getElementById('blob-storage-connection-string').value = blobAuthType === 'connection_string' ? (auth.key || '') : ''; + const blobEndpointField = document.getElementById('blob-storage-endpoint'); + if (blobEndpointField) { + blobEndpointField.value = blobAuthType === 'connection_string' ? '' : (plugin.endpoint || ''); + } + const blobAccountKeyField = document.getElementById('blob-storage-account-key'); + if (blobAccountKeyField) { + blobAccountKeyField.value = blobAuthType === 'key' ? (auth.key || '') : ''; + } document.getElementById('blob-storage-container-name').value = additionalFields.container_name || ''; document.getElementById('blob-storage-blob-prefix').value = additionalFields.blob_prefix || ''; + this.handleBlobStorageAuthTypeChange(); this.setBlobStorageConfiguration({ blob_storage_capabilities: additionalFields.blob_storage_capabilities || plugin.blob_storage_capabilities || null, blob_storage_read_file_types: additionalFields.blob_storage_read_file_types || plugin.blob_storage_read_file_types || null, @@ -6862,6 +6973,9 @@ export class PluginModalStepper { } else if (isDocumentSearchType) { return INTERNAL_DOCUMENT_SEARCH_ENDPOINT; } else if (isBlobStorageType) { + if (this.getBlobStorageAuthType() !== 'connection_string') { + return document.getElementById('blob-storage-endpoint')?.value.trim() || ''; + } const connectionString = document.getElementById('blob-storage-connection-string').value.trim(); return this.deriveBlobStorageEndpointFromConnectionString(connectionString) || this.originalPlugin?.endpoint || ''; } else if (isDatabricksType) { diff --git a/application/single_app/static/json/schemas/blob_storage.definition.json b/application/single_app/static/json/schemas/blob_storage.definition.json index d38a41a88..e0f7daa6b 100644 --- a/application/single_app/static/json/schemas/blob_storage.definition.json +++ b/application/single_app/static/json/schemas/blob_storage.definition.json @@ -1,6 +1,8 @@ { "$schema": "./plugin.definition.schema.json", "allowedAuthTypes": [ - "connection_string" + "connection_string", + "identity", + "key" ] } diff --git a/application/single_app/templates/_plugin_modal.html b/application/single_app/templates/_plugin_modal.html index 8cbe78851..177797fda 100644 --- a/application/single_app/templates/_plugin_modal.html +++ b/application/single_app/templates/_plugin_modal.html @@ -1110,10 +1110,29 @@
API Information
Blob Storage Connection
+ + +
Managed identity avoids stored secrets. The endpoint must be an Azure Blob service hostname.
+
+
Use an Azure Storage account connection string with access to the target container.
+
+ + +
Must be an Azure Blob service endpoint, for example https://account.blob.core.windows.net.
+
+
+ + +
If this action already uses a Key Vault-backed key, leave the stored value unchanged to preserve the existing secret.
+
diff --git a/docs/explanation/features/ADMIN_SETTINGS_IA_REWORK_STATUS.md b/docs/explanation/features/ADMIN_SETTINGS_IA_REWORK_STATUS.md index 8b6054ec4..20ec4bf7a 100644 --- a/docs/explanation/features/ADMIN_SETTINGS_IA_REWORK_STATUS.md +++ b/docs/explanation/features/ADMIN_SETTINGS_IA_REWORK_STATUS.md @@ -165,8 +165,28 @@ proxy convention, so the setting is still posted exactly once. Sync is two-way: the mirror drives the canonical input and dispatches a `change` event, and the canonical input updates the mirror when changed on its own tab. -### A structural guard worth keeping +### Stage F: the last hardcoded tab ids + +Two places still navigated by naming a tab, and both were broken by the rework: + +- **The setup walkthrough** mapped each of twelve steps to a tab id. Eleven + named tabs that no longer existed, so those steps would have moved nowhere. + Each step already knew which card it was about, so the tab id was a duplicate + copy of that knowledge. Steps now name the card and `openAdminCard()` finds + the tab. `scrollToRelevantSection()` became redundant and was deleted. +- **Cosmos throughput validation** clicked `scale-tab` to reveal an invalid + field. It now walks up from the invalid field to its own card, so it goes to + wherever that field actually lives. +`test_admin_settings_walkthrough_targets.py` asserts every step points at an +element that exists, that no tab id is hardcoded, and that step numbering has no +gaps. A scan confirms **no stale `-tab` literal remains in executable admin +JavaScript**. + +The general rule this rework converged on: **never name a tab; name the setting +and resolve the tab from the page.** + +### A structural guard worth keeping `test_every_pane_partial_is_balanced` checks `
` and `
` balance in every pane. An unbalanced pane does not fail to render — it silently nests the panes that follow it, so the failure surfaces somewhere unrelated and confusing. diff --git a/docs/explanation/fixes/ACTION_APP_IDENTITY_ENDPOINT_HARDENING_FIX.md b/docs/explanation/fixes/ACTION_APP_IDENTITY_ENDPOINT_HARDENING_FIX.md new file mode 100644 index 000000000..9ffb39b3b --- /dev/null +++ b/docs/explanation/fixes/ACTION_APP_IDENTITY_ENDPOINT_HARDENING_FIX.md @@ -0,0 +1,147 @@ +# Action App-Identity Endpoint Hardening Fix + +Fixed in version: **0.260.006** + +Reported through MSRC as the application-side boundary in a report about cross-resource token +selection in the `azure-storage-blob` bearer-challenge policy. No public issue was filed for this +work. + +## Issue Description + +Actions could be configured with a caller-supplied endpoint while authenticating with the +application's own workload identity. When the optional Semantic Kernel, per-user Semantic Kernel, +personal agents, and personal plugins features were enabled, a caller holding only the normal +`User` role could save a personal `blob_storage` action with an arbitrary endpoint and +`auth.type=identity`, then bind it to a personal agent. At runtime the application constructed a +Blob client for that endpoint using `DefaultAzureCredential()`, so a token minted for the +application's own managed identity was sent to a destination the caller chose. + +The reporter's broader finding is a defect in the Azure SDK for Python, where +`StorageBearerTokenCredentialPolicy.on_challenge` trusts the `resource_id` returned in a +`WWW-Authenticate` challenge without confirming it is an Azure Storage resource. That allows a +hostile endpoint to escalate a Storage token into a token for an unrelated resource such as Azure +Resource Manager. That defect belongs to `Azure/azure-sdk-for-python` and is not addressed here. + +This fix removes the application-side precondition: the ability for a lower-privileged caller to +direct application credentials at an endpoint of their choosing. + +## Root Cause Analysis + +Three independent gaps combined: + +1. **`allowedAuthTypes` was advisory only.** Each `static/json/schemas/.definition.json` + declares the auth types an action type supports, but the value was read only by the action modal + and by `GET /api/plugins//auth-types`. No save path enforced it, so a manifest could + declare any auth type for any action type. + +2. **No origin validation on endpoints that receive application credentials.** The blob storage, + queue storage, Cosmos query, and Databricks plugins each built a client or minted a token + against a fully caller-supplied endpoint. The `/api/plugins/test-cosmos-connection` route did the + same directly, bypassing both manifest validation and the plugin class. + +3. **Log Analytics custom clouds were unconstrained.** With `additionalFields.cloud='custom'`, + `authorityHost` flowed straight into `DefaultAzureCredential(authority=...)` and + `endpointOverride` became the query endpoint. For `auth.type='user'` the override also became the + OAuth scope, letting the manifest choose the resource for a delegated user token. + +Requiring an administrator-created identity reference would not have been sufficient on its own. +Any caller with the `User` role can create a personal workspace identity with +`auth_type='managed_identity'`, and `_apply_generic_action_identity_auth` maps that back to +`auth.type='identity'` and the same application credential. Endpoint origin validation is the +effective control; auth-type enforcement is the supporting one. + +## Technical Details + +### Files Modified + +- `application/single_app/functions_azure_endpoint_validation.py` (new) +- `application/single_app/json_schema_validation.py` +- `application/single_app/semantic_kernel_plugins/plugin_health_checker.py` +- `application/single_app/semantic_kernel_plugins/blob_storage_plugin.py` +- `application/single_app/semantic_kernel_plugins/queue_storage_plugin.py` +- `application/single_app/semantic_kernel_plugins/cosmos_query_plugin.py` +- `application/single_app/semantic_kernel_plugins/databricks_plugin.py` +- `application/single_app/semantic_kernel_plugins/log_analytics_plugin.py` +- `application/single_app/route_backend_plugins.py` +- `application/single_app/functions_file_sync.py` +- `application/single_app/static/json/schemas/blob_storage.definition.json` +- `application/single_app/static/js/plugin_modal_stepper.js` +- `application/single_app/templates/_plugin_modal.html` +- `application/single_app/config.py` +- `functional_tests/test_action_app_identity_endpoint_hardening.py` (new) +- `functional_tests/test_file_sync_azure_blob_storage.py` + +### Code Changes + +- Added a shared, dependency-light endpoint validation module with hardcoded allowlists for Azure + Storage blob and queue hostnames, Cosmos DB accounts, Azure Databricks workspaces, Azure Monitor + query endpoints, and Microsoft Entra authority hosts, covering the public, US Government, China, + and Germany clouds. Each validator rejects non-HTTPS schemes, embedded credentials, explicit + ports, query strings, parameters, fragments, local hostnames, and IP literals, then returns a + canonical rebuilt origin so only the validated destination reaches an SDK client. +- Enforced each action type's `allowedAuthTypes` inside + `PluginHealthChecker.validate_plugin_manifest`, the single choke point shared by personal saves, + group create and patch, admin add and edit, every `test-*-connection` route, and MCP discovery. + Identity-bound manifests are exempt because their auth type is resolved server-side from a stored + workspace identity, and that hydration legitimately rewrites `auth.type`. +- Repointed `GET /api/plugins//auth-types` at the same helper so the modal cannot offer an + auth type the backend will reject. +- Added endpoint origin validation to the blob storage, queue storage, Cosmos query, Databricks, and + Log Analytics branches of manifest validation, including the endpoint derived from a blob storage + connection string. +- Revalidated endpoints immediately before client construction or token acquisition in each plugin, + so actions stored before this change stop working rather than continuing to leak credentials. +- Constrained Log Analytics custom clouds so `authorityHost` must be a known Entra authority and + `endpointOverride` must be a known Azure Monitor query endpoint, at save time and at runtime. +- Validated the endpoint in `/api/plugins/test-cosmos-connection`, which builds its own client. +- Widened `blob_storage.definition.json` to `connection_string`, `identity`, and `key`, and extended + the blob storage modal with an authentication selector plus endpoint and account-key fields, + including a client-side mirror of the Azure Blob hostname allowlist. +- Refactored `functions_file_sync.py` to source the Azure Storage suffix allowlist and blob hostname + check from the shared module, removing the duplicated definition introduced in 0.250.068. + +## Testing Approach + +`functional_tests/test_action_app_identity_endpoint_hardening.py` covers accepted canonical Azure +endpoints for each supported cloud, rejection of hostile, internal, metadata-service, lookalike, +credential-bearing, port-bearing, query-bearing, and fragment-bearing endpoints, cross-service +endpoint mismatches, `allowedAuthTypes` enforcement including the identity-bound exemption and the +legacy Databricks type, Log Analytics custom-cloud constraints, runtime rejection of a stored +hostile manifest, modal wiring, and the File Sync refactor. + +`functional_tests/test_file_sync_azure_blob_storage.py` continues to pass, confirming the shared +allowlist preserves the existing File Sync SSRF protections. + +## Validation + +- Before: a `User`-role caller could save an action with an arbitrary endpoint and application + managed-identity authentication, and the application would send a token minted for its own + workload identity to that endpoint. +- After: only canonical Azure service origins reach a credentialed client. Save-time validation + rejects the configuration, and runtime revalidation blocks actions stored before this change. +- Two pre-existing unrelated test failures were confirmed to exist on the unmodified branch and were + not introduced here: `test_file_sync_routes_do_not_disclose_exception_details`, and + `test_blob_storage_action_capabilities.py`, which requires live Azure credentials to import. + +## Impact Analysis + +Deployments using standard Azure hostnames are unaffected. Azure Private Endpoint configurations +continue to work through the standard account hostname with private DNS resolution. Custom domains, +Azurite and development storage, Azure Stack endpoints, and direct private-link hostnames are +intentionally rejected, matching the precedent set by the Azure Blob File Sync fix in 0.250.068. + +## Known Limitations and Follow-Ups + +- A non-administrator can still create a personal workspace identity with + `auth_type='managed_identity'` and bind it to an action. After this change that no longer permits + arbitrary-origin credential delivery, but it still allows a non-administrator to use the + application's workload identity against legitimate Azure resources the application can reach. + Restricting `managed_identity` workspace identities to administrator or global scope is worth a + separate decision. +- The bundled Bicep grants the web application built-in ARM `Contributor` on the Cosmos DB account + (`deployers/bicep/modules/setPermissions.bicep`), which widens the blast radius of any leaked + token. A least-privilege review of that assignment is a separate change; no files under + `deployers/` were modified here, so `deployers/version.txt` was not incremented. +- The underlying `resource_id` validation defect still requires an upstream fix in + `Azure/azure-sdk-for-python` across the Blob, Queue, File Share, and Data Lake challenge policies. + Nothing in this change substitutes for that. diff --git a/docs/explanation/fixes/GENERATED_ARTIFACT_PAGING_AND_GUIDANCE_FIX.md b/docs/explanation/fixes/GENERATED_ARTIFACT_PAGING_AND_GUIDANCE_FIX.md new file mode 100644 index 000000000..a0d94206d --- /dev/null +++ b/docs/explanation/fixes/GENERATED_ARTIFACT_PAGING_AND_GUIDANCE_FIX.md @@ -0,0 +1,167 @@ +# Generated Artifact Paging, Truncation, and Guidance Carry-Forward Fix + +**Fixed in version: 0.260.011** + +## Issue Description + +A deployment test of the generated-file work from `0.260.004` through `0.260.010` surfaced three +defects in agent conversations that produced CSV artifacts from a Yamcs telemetry action. + +1. **The model denied it could create a file, then the file appeared anyway.** After answering a + schema clarification with "yes and all columns", the assistant replied *"I cannot create or + attach a CSV file in this interface"* and listed the columns as prose. The server published the + CSV regardless, so the user saw a contradiction between the reply and the artifact card. +2. **A 1,000-row file was produced for a window holding roughly 500 distinct samples.** The agent + called `list_parameter_history` twice, and the server concatenated both responses. +3. **A partial file was published with no indication it was partial.** The source action returned + 500 rows and reported `truncated=True`, and the assistant said so in prose, but the artifact card + presented a plain 500-row file. + +## Root Cause Analysis + +### 1. Publication contract resolved, guidance not + +Version `0.260.008` added `resolve_pending_generated_file_format` so a reply that only answers a +clarification still *publishes* the originally requested artifact. Guidance injection was never +updated to match. Both chat paths resolved the guidance format from the current user message alone: + +```python +requested_format=get_tabular_generated_output_format(user_message) +``` + +"yes and all columns" contains no format keyword, so `build_generated_file_output_guidance` returned +an empty string and no system message was added. The model was never told that the server attaches +the file, so it fell back to its default belief that it cannot produce attachments — while the +finalizer, which *did* carry the format forward, published the CSV. + +### 2. Overlapping pages concatenated + +Version `0.260.008` grouped rows by action so paged calls to one action stayed one dataset, using +`.extend()`. The deployment log shows the agent did not page forward; it re-requested the same +window with an earlier stop time: + +``` +list_parameter_history start=20:40:57.445 stop=20:55:57.445 row_count=500 truncated=True +list_parameter_history start=20:40:57.445 stop=20:49:17.445 row_count=500 truncated=True +``` + +Both calls share a start time, and both were capped at the action's own `max_rows` limit, so the +second response returned the same first 500 samples. Concatenation produced 1,000 rows for roughly +500 distinct samples. The same pattern explains the earlier "901 acquired samples" claim +(500 truncated + 401 overlapping). + +### 3. Truncation signal discarded + +Action payloads carried `truncated: true`, but nothing read it. `_assistant_content_disclaims_complete_file` +existed for JSON and XML only and matched assistant prose rather than the action's own report, so no +format surfaced source truncation on the artifact. + +The 500-row cap itself is the action's configured `max_rows`, not a SimpleChat limit. The fix makes +the cap visible and teaches the model to page past it rather than silently changing plugin behavior. + +## Technical Details + +### Files Modified + +| File | Change | +|------|--------| +| `application/single_app/functions_generated_file_exports.py` | Cross-page row de-duplication, truncation detection and propagation, paging guidance | +| `application/single_app/route_backend_chats.py` | `_resolve_generated_file_guidance_format` helper wired into both chat paths | +| `application/single_app/static/js/chat/chat-messages.js` | `Partial` badge on the generated artifact card | +| `application/single_app/config.py` | Version bump to `0.260.011` | +| `functional_tests/test_generated_csv_uses_authorized_action_rows.py` | Telemetry fixture now uses distinct sample times | + +### Code Changes + +**Guidance carry-forward.** A new route helper resolves the format the reply owes, falling back to the +pending clarification when the message itself names no format: + +```python +def _resolve_generated_file_guidance_format(user_question, conversation_id, user_id): + requested_format = str(get_tabular_generated_output_format(user_question) or '').strip().lower() + if requested_format or not conversation_id: + return requested_format + pending_format = _resolve_pending_generated_file_format(user_question, conversation_id, user_id) + return str(pending_format or '').strip().lower() +``` + +Both the streaming and non-streaming chat paths now call it. In the streaming path it also drives the +payload-suppression decision and the "generating file" status banner, so the answer turn behaves like +the turn that made the original request. + +**Cross-page de-duplication.** `_collect_authorized_function_row_groups` now tracks a row signature per +action label. Repeats *inside* one response are kept, because the action counted them as distinct +records; rows an *earlier page of the same action* already contributed are dropped: + +```python +page_signatures = set() +for structured_row in structured_rows: + row_signature = _build_function_result_row_signature(structured_row) + if row_signature is not None: + if row_signature in seen_rows: + continue + page_signatures.add(row_signature) + group_rows.append(structured_row) +seen_rows.update(page_signatures) +``` + +**Truncation disclosure.** `function_results_report_truncated_rows` scans row-contributing action +payloads for `truncated`, `is_truncated`, `was_truncated`, `results_truncated`, or `rows_truncated`. +The flag flows into the export payload, the summary text, and the artifact metadata, and is captured +separately for reach-back rows so a file built from an earlier turn keeps that turn's signal. + +**Paging guidance.** CSV, DOCX, and PDF guidance now includes: + +> When an action reports that its results were truncated, request the remaining data with a window +> that starts after the last row you already have instead of repeating the original range, and say +> plainly that the data is partial if you cannot retrieve the rest. + +JSON and XML are excluded because their guidance requires a payload-only reply. + +### Fixture Correction + +`TELEMETRY_ROWS` in `test_generated_csv_uses_authorized_action_rows.py` previously derived timestamps +from `index % 60`, so its 900 "samples" were only 60 distinct rows repeated fifteen times. That made +genuine continuation pages indistinguishable from re-reads. Timestamps now roll over into minutes, +which matches the 15-minute high-granularity window the fixture's question asks for. + +## Validation + +### Test Results + +| Suite | Result | +|---|---| +| `test_generated_artifact_paging_and_guidance.py` | 11/11 | +| `test_generated_csv_uses_authorized_action_rows.py` | 15/15 | +| `test_generated_structured_artifact_parity.py` | 8/8 | +| `test_assistant_table_csv_artifact.py` | 36/36 | +| `test_generated_json_xml_exports.py` | 7/7 | +| `test_tabular_passthrough_heterogeneous_rows.py` | 4/4 | +| `test_tabular_background_generated_exports.py` | 8/8 | +| `test_tabular_row_orchestration_scale.py` | 15/15 | +| `route_tests/test_route_blueprint_policy_inventory.py` | 6/6 | +| `route_tests/test_route_unauthenticated_policy_contract.py` | 4/4 | + +### Before and After + +| Scenario | Before | After | +|---|---|---| +| Answering a schema clarification | No file guidance injected; model denies it can attach files, server publishes anyway | Publication contract injected; reply and artifact agree | +| Two calls re-reading one window | 1,000 rows, roughly half duplicated | Distinct rows only | +| Two calls paging forward | Both pages kept | Both pages kept (unchanged) | +| Action reports `truncated=True` | Silent, file looks complete | `Partial` badge, summary note, `rows_truncated` on the artifact | +| Repeated identical rows in one response | All kept | All kept (unchanged) | + +## Known Limitations + +- De-duplication compares full row values. A continuation page whose rows differ only in a field the + action omits from its response would still be treated as a repeat. +- The paging instruction is model guidance, not enforcement. An agent that ignores it still produces + a truncated dataset, but the file is now labeled partial. +- The per-call row cap belongs to the action's own configuration. Raising it is a plugin + configuration change, not an application change. + +## Related Documentation + +- `docs/explanation/fixes/GENERATED_JSON_XML_EXPORTS_FIX.md` +- `docs/explanation/fixes/GENERATED_STRUCTURED_ARTIFACT_INTENT_FIX.md` diff --git a/docs/explanation/fixes/index.md b/docs/explanation/fixes/index.md index d81b36e2a..201728c95 100644 --- a/docs/explanation/fixes/index.md +++ b/docs/explanation/fixes/index.md @@ -35,3 +35,4 @@ category: Version History - [Chat Citation Whitespace Collapse Fix](CHAT_CITATION_WHITESPACE_COLLAPSE_FIX.md) - [New Chat Conversation Documents Drawer Reset Fix](NEW_CHAT_CONVERSATION_DOCUMENTS_DRAWER_RESET_FIX.md) - [Collaboration Mention Tab Autocomplete Fix](COLLABORATION_MENTION_TAB_AUTOCOMPLETE_FIX.md) +- [Generated Artifact Paging, Truncation, and Guidance Carry-Forward Fix](GENERATED_ARTIFACT_PAGING_AND_GUIDANCE_FIX.md) diff --git a/docs/explanation/release_notes.md b/docs/explanation/release_notes.md index dcb472088..ca1ac6fe9 100644 --- a/docs/explanation/release_notes.md +++ b/docs/explanation/release_notes.md @@ -2,6 +2,26 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Version](/explanation/features/) and [Fixes by Version](/explanation/fixes/). +### **(v0.260.018)** + +#### Bug Fixes + +* **Setup Walkthrough Lands On The Right Settings Again** + * The guided setup walkthrough sent each step to a named tab. After the Admin Settings restructure, eleven of its twelve steps named tabs that no longer existed, so those steps would have moved nowhere and left the admin looking at whatever was already on screen. + * Each step now names the setting it is about and the tab is worked out from the page, so the walkthrough follows settings wherever they live. + * (Ref: setup walkthrough, `admin_settings.js`, `admin_card_links.js`) + +* **Cosmos Throughput Validation Reveals The Invalid Field** + * When Cosmos throughput values failed validation, the page tried to switch to a tab that no longer exists, so the field needing attention could be left on a hidden tab with no indication of where to look. + * Validation now jumps to wherever the invalid field actually is. + * (Ref: Cosmos throughput validation, `admin_settings.js`) + +#### User Interface Enhancements + +* **Admin Settings Restructure Merged With Current Development** + * Version bump covering the merge of the Admin Settings information architecture work with the generated file output fixes developed in parallel. Both reached v0.260.011 independently, so their release notes are combined under that version. + * (Ref: Admin Settings navigation, generated file exports) + ### **(v0.260.017)** #### New Features @@ -169,6 +189,22 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver * It now sits outside the tabs and is visible wherever you are in Governance. * (Ref: `governance-status`, `admin_governance.js`) +#### Bug Fixes + +* **Reliable File Generation From Agent Action Results** + * Asking an agent for a downloadable file built from action results now produces the complete dataset in the requested format. Previously these requests could fail outright, publish a three-row sample of a large result, overwrite the assistant's written answer, or return nothing at all. Delivered across v0.260.004 through v0.260.011. + * **Files no longer fail to generate.** A CSV built from several actions in one turn could stop with `Generated output schema mismatch at row 2`, because each action returned a different set of columns. The export now pins a union of every column before the run starts and pads the missing cells, so mixed-shape results serialize instead of failing. + * **The written answer is no longer replaced by the file card.** CSV replies were suppressed alongside JSON and XML, but only JSON and XML withhold their payload from the response. CSV, DOCX, and PDF now keep the assistant's answer and append the file card beneath it. + * **Files contain the retrieved data, not a sample of it.** When the assistant pasted a few example rows above its answer, that excerpt outranked the real result set, producing a 3-row file from a 900-row query. Pasted rows are now used only when they are not an excerpt of the data actually retrieved. + * **Discovery calls no longer dilute the dataset.** A turn that lists instances, lists parameters, then retrieves history used to blend all three into one file. Rows are grouped by the action that produced them, and the action holding the substantive dataset wins. + * **Follow-up requests reuse data already gathered.** Asking "now make that a CSV" after the data was retrieved in an earlier turn no longer returns an empty result. The export reaches back through stored conversation citations, bounded by the **conversation history limit** in Admin Settings, and reuses the rows already collected instead of re-querying the source. + * **Answering a clarifying question now delivers the file.** When the assistant asks which rows and columns to include, replying "yes, all columns" now publishes the file that was originally requested. The clarification turn itself no longer publishes a placeholder file built from the question text. + * **The assistant no longer claims it cannot create files.** Every format now states the publication contract to the model, including on the turn that only answers a clarification, so replies stop saying "I cannot create or attach a file in this interface" and then producing one anyway. + * **Overlapping result pages no longer double the row count.** Agents frequently re-request a range from the same start time rather than paging forward, which produced a 1,000-row file for a window holding roughly 500 distinct records. Rows an earlier page of the same action already returned are dropped, while genuinely repeated records inside a single response are preserved. + * **Partial data is now labeled.** When an action reports that it truncated its own results, the file carries a **Partial** badge and a note explaining that it covers only the rows the action returned. Agents are also instructed to request the remainder starting after the last row they already hold, rather than repeating the original range. + * **CSV, DOCX, PDF, JSON, and XML now behave identically.** All five formats resolve rows the same way, reach back to earlier turns, decline to publish on a clarification turn, and report truncation. + * (Ref: `functions_generated_file_exports.py`, `functions_tabular_generated_exports.py`, `route_backend_chats.py`, `chat-messages.js`, [Generated Artifact Paging, Truncation, and Guidance Carry-Forward Fix](fixes/GENERATED_ARTIFACT_PAGING_AND_GUIDANCE_FIX.md), Refs #1071) + ### **(v0.260.010)** #### New Features @@ -288,6 +324,27 @@ For feature-focused and fix-focused drill-downs by version, see [Features by Ver #### Bug Fixes +* **Actions Using the Application Identity Are Now Restricted to Azure Endpoints** + * Actions that authenticate with the application's own managed identity can no longer be pointed at an arbitrary endpoint. Blob Storage, Queue Storage, Cosmos, Databricks, and Log Analytics actions now accept only canonical Azure service hostnames for the public, US Government, China, and Germany clouds. + * Previously a caller holding only the normal **User** role could save a personal action with an attacker-controlled endpoint and application managed-identity authentication, causing the application to send a token minted for its own workload identity to that destination. + * Endpoints are validated when the action is saved and again immediately before the client is built, so actions stored before this release stop working rather than continuing to send credentials. + * Log Analytics custom clouds can no longer choose the Microsoft Entra token authority or the OAuth resource used for delegated tokens. + * Existing actions using standard Azure hostnames are unaffected. Custom domains, development storage, Azure Stack, and direct private-link hostnames are intentionally rejected, matching the Azure Blob File Sync hardening in v0.250.068. + * (Ref: `functions_azure_endpoint_validation.py`, `plugin_health_checker.py`, `blob_storage_plugin.py`, `queue_storage_plugin.py`, `cosmos_query_plugin.py`, `databricks_plugin.py`, `log_analytics_plugin.py`, [Action App-Identity Endpoint Hardening Fix](fixes/ACTION_APP_IDENTITY_ENDPOINT_HARDENING_FIX.md)) + +* **Action Authentication Types Are Now Enforced on the Server** + * Each action type's supported authentication methods, declared in its schema definition file, are now enforced when an action is saved or tested. Previously the list was only used to populate the action modal and was never checked by the backend. + * This prevents an action type from being configured with an authentication method it was never designed to support, such as requesting application-identity authentication for an OpenAPI or Microsoft Graph action. + * The auth-types API now resolves through the same helper the save paths use, so the modal and the backend cannot drift apart. + * (Ref: `json_schema_validation.py`, `get_allowed_auth_types_for_plugin_type`, `validate_plugin_auth_type_allowed`, `route_backend_plugins.py`) + +#### User Interface Enhancements + +* **Blob Storage Actions Can Now Use Managed Identity or an Account Key** + * The Blob Storage action modal gained an authentication selector offering **Connection String**, **Managed Identity**, and **Account Key**, along with blob service endpoint and account key fields. + * Previously the modal only collected a connection string even though the backend accepted other methods, so managed identity was not reachable through the UI. + * The endpoint field is validated against the Azure Blob hostname allowlist before the action is saved. + * (Ref: `_plugin_modal.html`, `plugin_modal_stepper.js`, `blob_storage.definition.json`) * **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. diff --git a/functional_tests/test_action_app_identity_endpoint_hardening.py b/functional_tests/test_action_app_identity_endpoint_hardening.py new file mode 100644 index 000000000..00336390d --- /dev/null +++ b/functional_tests/test_action_app_identity_endpoint_hardening.py @@ -0,0 +1,400 @@ +#!/usr/bin/env python3 +# test_action_app_identity_endpoint_hardening.py +""" +Functional test for action app-identity endpoint hardening. +Version: 0.260.006 +Implemented in: 0.260.006 + +Actions can be configured with a caller-supplied endpoint while authenticating with the +application's own workload identity. This test ensures such endpoints are constrained to +canonical Azure service origins at save time and again at plugin construction, that the +per-type allowedAuthTypes contract is enforced server-side, and that Log Analytics custom +clouds cannot select an arbitrary token authority or OAuth resource. +""" + +import ast +import sys +import types +from pathlib import Path +from unittest.mock import MagicMock + +REPO_ROOT = Path(__file__).resolve().parents[1] +APP_ROOT = REPO_ROOT / "application" / "single_app" +sys.path.insert(0, str(APP_ROOT)) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + + +def _install_config_stub(): + """Stub config.py, which builds live Azure clients at import time. + + Names are harvested from the real module so `from config import *` consumers still + resolve every symbol they expect. + """ + if isinstance(sys.modules.get("config"), types.ModuleType) and hasattr(sys.modules["config"], "_is_test_stub"): + return + + config_stub = types.ModuleType("config") + config_stub._is_test_stub = True + config_tree = ast.parse((APP_ROOT / "config.py").read_text(encoding="utf-8")) + for node in ast.walk(config_tree): + names = [] + if isinstance(node, ast.Assign): + names = [target.id for target in node.targets if isinstance(target, ast.Name)] + elif isinstance(node, ast.AnnAssign) and isinstance(node.target, ast.Name): + names = [node.target.id] + elif isinstance(node, (ast.Import, ast.ImportFrom)): + names = [(alias.asname or alias.name).split(".")[0] for alias in node.names if alias.name != "*"] + elif isinstance(node, (ast.FunctionDef, ast.ClassDef)): + names = [node.name] + for name in names: + if hasattr(config_stub, name): + continue + try: + setattr(config_stub, name, __import__(name)) + except Exception: + setattr(config_stub, name, MagicMock(name=f"config.{name}")) + sys.modules["config"] = config_stub + + +def read_text(relative_path): + """Read a repository file as UTF-8 text.""" + return (REPO_ROOT / relative_path).read_text(encoding="utf-8") + + +# Endpoints that must never receive credentials minted from the application identity. +HOSTILE_ENDPOINTS = ( + "https://attacker.invalid", + "https://evil.example.com", + "https://169.254.169.254", + "https://127.0.0.1", + "https://localhost", + "http://account.blob.core.windows.net", + "https://user:pass@account.blob.core.windows.net", + "https://account.blob.core.windows.net:8443", + "https://account.blob.core.windows.net.evil.com", + "https://account.blob.core.windows.net?x=1", + "https://account.blob.core.windows.net#frag", +) + + +def test_version_and_definition_contract(): + """Validate the version bump and the blob storage auth type declaration.""" + import json + + assert_app_version_at_least("0.260.006") + + definition = json.loads(read_text("application/single_app/static/json/schemas/blob_storage.definition.json")) + allowed = definition["allowedAuthTypes"] + assert set(allowed) == {"connection_string", "identity", "key"}, allowed + + +def test_blob_and_queue_endpoint_allowlist(): + """Blob and queue endpoints must be canonical Azure Storage origins.""" + from functions_azure_endpoint_validation import ( + validate_azure_blob_endpoint, + validate_azure_queue_endpoint, + ) + + assert validate_azure_blob_endpoint("https://acct.blob.core.windows.net") == "https://acct.blob.core.windows.net" + assert validate_azure_blob_endpoint("https://acct.blob.core.usgovcloudapi.net/") == "https://acct.blob.core.usgovcloudapi.net" + assert validate_azure_blob_endpoint("https://acct.blob.core.chinacloudapi.cn") == "https://acct.blob.core.chinacloudapi.cn" + assert validate_azure_queue_endpoint("https://acct.queue.core.windows.net") == "https://acct.queue.core.windows.net" + + for hostile_endpoint in HOSTILE_ENDPOINTS: + try: + validate_azure_blob_endpoint(hostile_endpoint) + except ValueError: + continue + raise AssertionError(f"Blob endpoint should have been rejected: {hostile_endpoint}") + + # A queue action must not be able to point at the blob service and vice versa. + for mismatched_endpoint, validator in ( + ("https://acct.queue.core.windows.net", validate_azure_blob_endpoint), + ("https://acct.blob.core.windows.net", validate_azure_queue_endpoint), + ): + try: + validator(mismatched_endpoint) + except ValueError: + continue + raise AssertionError(f"Cross-service endpoint should have been rejected: {mismatched_endpoint}") + + +def test_cosmos_databricks_and_monitor_endpoint_allowlist(): + """Cosmos, Databricks, Azure Monitor, and Entra authority values must be Azure origins.""" + from functions_azure_endpoint_validation import ( + validate_azure_cosmos_endpoint, + validate_azure_databricks_endpoint, + validate_azure_entra_authority_host, + validate_azure_monitor_query_endpoint, + ) + + assert validate_azure_cosmos_endpoint("https://acct.documents.azure.com") == "https://acct.documents.azure.com" + # Cosmos endpoints are commonly pasted with the default HTTPS port. + assert validate_azure_cosmos_endpoint("https://acct.documents.azure.com:443/") == "https://acct.documents.azure.com" + assert validate_azure_databricks_endpoint( + "https://adb-1234567890123456.7.azuredatabricks.net" + ) == "https://adb-1234567890123456.7.azuredatabricks.net" + assert validate_azure_monitor_query_endpoint("https://api.loganalytics.io") == "https://api.loganalytics.io" + assert validate_azure_entra_authority_host("https://login.microsoftonline.us") == "login.microsoftonline.us" + + rejections = ( + (validate_azure_cosmos_endpoint, "https://evil.example.com"), + (validate_azure_cosmos_endpoint, "https://acct.documents.azure.com.evil.com"), + (validate_azure_databricks_endpoint, "https://evil.example.com"), + (validate_azure_databricks_endpoint, "https://adb-1.azuredatabricks.net.evil.com"), + # Azure Resource Manager is the pivot target described in the report. + (validate_azure_monitor_query_endpoint, "https://management.azure.com"), + (validate_azure_monitor_query_endpoint, "https://evil.example.com"), + (validate_azure_entra_authority_host, "https://evil.example.com"), + (validate_azure_entra_authority_host, "https://login.microsoftonline.com.evil.com"), + ) + for validator, hostile_value in rejections: + try: + validator(hostile_value) + except ValueError: + continue + raise AssertionError(f"{validator.__name__} should have rejected {hostile_value}") + + +def test_allowed_auth_types_are_enforced_server_side(): + """A caller must not be able to declare an auth type its action type does not support.""" + from json_schema_validation import ( + get_allowed_auth_types_for_plugin_type, + validate_plugin_auth_type_allowed, + ) + + assert get_allowed_auth_types_for_plugin_type("blob_storage") == {"connection_string", "identity", "key"} + assert get_allowed_auth_types_for_plugin_type("openapi") == {"key"} + # Unknown types fall back to the shared enum so they keep working. + assert "identity" in get_allowed_auth_types_for_plugin_type("some_unknown_action_type") + + # The reported application boundary: an app-identity auth type on a type that never declared it. + assert validate_plugin_auth_type_allowed({"type": "openapi", "auth": {"type": "identity"}}) + assert validate_plugin_auth_type_allowed({"type": "msgraph", "auth": {"type": "identity"}}) + + # Legitimate configurations must keep working. + for allowed_manifest in ( + {"type": "blob_storage", "auth": {"type": "connection_string", "key": "x"}}, + {"type": "blob_storage", "auth": {"type": "identity"}}, + {"type": "openapi", "auth": {"type": "key", "key": "x"}}, + {"type": "sql_query", "auth": {"type": "user"}}, + {"type": "sql_query", "auth": {"type": "servicePrincipal"}}, + {"type": "cosmos_query", "auth": {"type": "identity"}}, + # Legacy Databricks type resolves to the current type's contract. + {"type": "databricks_table", "auth": {"type": "identity"}}, + # Identity-bound actions resolve auth server-side, so hydration output is exempt. + {"type": "openapi", "auth": {"type": "username_password"}, "identity_id": "abc"}, + ): + error = validate_plugin_auth_type_allowed(allowed_manifest) + assert error is None, f"{allowed_manifest} unexpectedly rejected: {error}" + + +def test_manifest_validation_rejects_hostile_endpoints(): + """Save-time manifest validation must reject non-Azure endpoints for app-identity actions.""" + _install_config_stub() + from semantic_kernel_plugins.plugin_health_checker import PluginHealthChecker + + def is_valid(manifest, plugin_type): + valid, _ = PluginHealthChecker.validate_plugin_manifest(manifest, plugin_type) + return valid + + blob_manifest = {"name": "b", "type": "blob_storage", "additionalFields": {"container_name": "c"}} + assert not is_valid({**blob_manifest, "endpoint": "https://attacker.invalid", "auth": {"type": "identity"}}, "blob_storage") + assert is_valid({**blob_manifest, "endpoint": "https://acct.blob.core.windows.net", "auth": {"type": "identity"}}, "blob_storage") + + # An endpoint derived from a stored connection string is validated too. + hostile_connection_string = ( + "DefaultEndpointsProtocol=https;AccountName=a;AccountKey=k;BlobEndpoint=https://attacker.invalid" + ) + assert not is_valid( + {**blob_manifest, "auth": {"type": "connection_string", "key": hostile_connection_string}}, + "blob_storage", + ) + + queue_manifest = {"name": "q", "type": "queue_storage", "auth": {"type": "identity"}} + assert not is_valid({**queue_manifest, "endpoint": "https://attacker.invalid"}, "queue_storage") + assert is_valid({**queue_manifest, "endpoint": "https://acct.queue.core.windows.net"}, "queue_storage") + + cosmos_manifest = { + "name": "c", + "type": "cosmos_query", + "auth": {"type": "identity"}, + "additionalFields": {"database_name": "d", "container_name": "c", "partition_key_path": "/id"}, + } + assert not is_valid({**cosmos_manifest, "endpoint": "https://attacker.invalid"}, "cosmos_query") + assert is_valid({**cosmos_manifest, "endpoint": "https://acct.documents.azure.com"}, "cosmos_query") + + databricks_manifest = { + "name": "d", + "type": "databricks", + "auth": {"type": "identity", "identity": "managed_identity"}, + "additionalFields": {"cloud": "azure_commercial", "warehouse_id": "w"}, + } + assert not is_valid({**databricks_manifest, "endpoint": "https://attacker.invalid"}, "databricks") + assert is_valid( + {**databricks_manifest, "endpoint": "https://adb-1234567890123456.7.azuredatabricks.net"}, + "databricks", + ) + + +def test_log_analytics_custom_cloud_is_constrained(): + """A custom Log Analytics cloud must not select the token authority or OAuth resource.""" + _install_config_stub() + from semantic_kernel_plugins.plugin_health_checker import PluginHealthChecker + + def is_valid(additional_fields): + valid, _ = PluginHealthChecker.validate_plugin_manifest( + {"name": "l", "type": "log_analytics", "additionalFields": additional_fields}, + "log_analytics", + ) + return valid + + assert not is_valid({ + "workspaceId": "w", + "cloud": "custom", + "authorityHost": "https://evil.example.com", + "endpointOverride": "https://api.loganalytics.io", + }) + # endpointOverride becomes the delegated-token scope, so ARM must be rejected. + assert not is_valid({ + "workspaceId": "w", + "cloud": "custom", + "authorityHost": "https://login.microsoftonline.com", + "endpointOverride": "https://management.azure.com", + }) + assert is_valid({ + "workspaceId": "w", + "cloud": "custom", + "authorityHost": "https://login.microsoftonline.us", + "endpointOverride": "https://api.loganalytics.us", + }) + assert is_valid({"workspaceId": "w", "cloud": "public"}) + + +def test_stored_hostile_actions_are_rejected_at_runtime(): + """Actions stored before this hardening must fail at plugin construction.""" + _install_config_stub() + from semantic_kernel_plugins.blob_storage_plugin import BlobStoragePlugin + from semantic_kernel_plugins.queue_storage_plugin import QueueStoragePlugin + + stored_hostile_blob = { + "name": "stored", + "type": "blob_storage", + "endpoint": "https://attacker.invalid", + "auth": {"type": "identity"}, + "additionalFields": {"container_name": "proof"}, + } + try: + BlobStoragePlugin(stored_hostile_blob) + except ValueError: + pass + else: + raise AssertionError("A stored hostile blob endpoint must not reach the Blob SDK client.") + + # The equivalent valid action must still construct. + BlobStoragePlugin({**stored_hostile_blob, "endpoint": "https://acct.blob.core.windows.net"}) + + stored_hostile_queue = { + "name": "stored", + "type": "queue_storage", + "endpoint": "https://attacker.invalid", + "auth": {"type": "identity"}, + "additional_settings": {"queue_name": "q1"}, + } + try: + QueueStoragePlugin(stored_hostile_queue) + except ValueError: + pass + else: + raise AssertionError("A stored hostile queue endpoint must not reach the Queue SDK client.") + + QueueStoragePlugin({**stored_hostile_queue, "endpoint": "https://acct.queue.core.windows.net"}) + + +def test_runtime_validation_is_wired_into_credentialed_clients(): + """Each app-identity client construction site must revalidate its endpoint.""" + blob_source = read_text("application/single_app/semantic_kernel_plugins/blob_storage_plugin.py") + queue_source = read_text("application/single_app/semantic_kernel_plugins/queue_storage_plugin.py") + cosmos_source = read_text("application/single_app/semantic_kernel_plugins/cosmos_query_plugin.py") + databricks_source = read_text("application/single_app/semantic_kernel_plugins/databricks_plugin.py") + log_analytics_source = read_text("application/single_app/semantic_kernel_plugins/log_analytics_plugin.py") + plugin_routes_source = read_text("application/single_app/route_backend_plugins.py") + + assert "validate_azure_blob_endpoint" in blob_source + assert "account_url=self.endpoint" not in blob_source + assert "validate_azure_queue_endpoint" in queue_source + assert "account_url=self.endpoint" not in queue_source + assert "validate_azure_cosmos_endpoint" in cosmos_source + assert "validate_azure_databricks_endpoint" in databricks_source + assert "validate_azure_entra_authority_host" in log_analytics_source + assert "validate_azure_monitor_query_endpoint" in log_analytics_source + # The raw caller-supplied authority must no longer flow straight into the credential. + assert "authority_host = self.authority_host" not in log_analytics_source + # The Cosmos test-connection route builds its own client and must validate independently. + assert "validate_azure_cosmos_endpoint" in plugin_routes_source + + +def test_blob_storage_modal_exposes_identity_and_key(): + """The blob modal must offer the auth types the backend now accepts.""" + modal_source = read_text("application/single_app/templates/_plugin_modal.html") + stepper_source = read_text("application/single_app/static/js/plugin_modal_stepper.js") + + for element_id in ( + "blob-storage-auth-type", + "blob-storage-endpoint", + "blob-storage-account-key", + "blob-storage-connection-string-group", + "blob-storage-endpoint-group", + "blob-storage-account-key-group", + ): + assert element_id in modal_source, element_id + + assert "handleBlobStorageAuthTypeChange" in stepper_source + assert "isAzureBlobEndpoint" in stepper_source + assert "AZURE_STORAGE_ENDPOINT_SUFFIXES" in stepper_source + # Visibility must use Bootstrap classes rather than inline display styles. + toggle_body = stepper_source.split("handleBlobStorageAuthTypeChange() {", 1)[1][:600] + assert "classList.toggle('d-none'" in toggle_body + assert "style.display" not in toggle_body + + +def test_file_sync_reuses_the_shared_allowlist(): + """File Sync must source its Azure Storage allowlist from the shared module.""" + file_sync_source = read_text("application/single_app/functions_file_sync.py") + + assert "from functions_azure_endpoint_validation import" in file_sync_source + assert "azure_storage_endpoint_suffix_for_hostname" in file_sync_source + # The duplicated literal suffix tuple must be gone. + assert 'AZURE_STORAGE_ENDPOINT_SUFFIXES = (' not in file_sync_source + + +if __name__ == "__main__": + tests = [ + test_version_and_definition_contract, + test_blob_and_queue_endpoint_allowlist, + test_cosmos_databricks_and_monitor_endpoint_allowlist, + test_allowed_auth_types_are_enforced_server_side, + test_manifest_validation_rejects_hostile_endpoints, + test_log_analytics_custom_cloud_is_constrained, + test_stored_hostile_actions_are_rejected_at_runtime, + test_runtime_validation_is_wired_into_credentialed_clients, + test_blob_storage_modal_exposes_identity_and_key, + test_file_sync_reuses_the_shared_allowlist, + ] + + results = [] + for test in tests: + try: + test() + print(f"PASS {test.__name__}") + results.append(True) + except Exception as error: + print(f"FAIL {test.__name__}: {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_admin_settings_walkthrough_targets.py b/functional_tests/test_admin_settings_walkthrough_targets.py new file mode 100644 index 000000000..8d0a4015c --- /dev/null +++ b/functional_tests/test_admin_settings_walkthrough_targets.py @@ -0,0 +1,121 @@ +#!/usr/bin/env python3 +""" +Functional test for the Admin Settings setup walkthrough targets. +Version: 0.260.018 +Implemented in: 0.260.018 + +The setup walkthrough used to name a tab id for each step. That recorded the +same knowledge twice: the step already knew which setting it was about, and the +tab id was a second copy that went stale the moment a setting moved. After the +information architecture rework, eleven of its twelve steps pointed at tabs that +no longer existed, and the walkthrough would have silently gone nowhere. + +Steps now name the card they are about and the owning tab is resolved from the +page. This test ensures every step still points at a card that exists. +""" + +import os +import re +import sys + +sys.path.append(os.path.dirname(os.path.abspath(__file__))) + +from test_support.templates import read_admin_settings_template # noqa: E402 + +APP_ROOT = os.path.join( + os.path.dirname(os.path.dirname(os.path.abspath(__file__))), + "application", + "single_app", +) +ADMIN_JS = os.path.join(APP_ROOT, "static", "js", "admin", "admin_settings.js") + +CARD_PATTERN = re.compile(r"^\s*(\d+):\s*\{\s*card:\s*'([^']+)'(?:,\s*focus:\s*'([^']+)')?", re.M) + + +def _read(path): + with open(path, "r", encoding="utf-8") as handle: + return handle.read() + + +def _walkthrough_targets(): + source = _read(ADMIN_JS) + start = source.index("const stepToCard = {") + end = source.index("};", start) + return CARD_PATTERN.findall(source[start:end]) + + +def test_walkthrough_targets_exist(): + """A step pointing at a missing card sends the walkthrough nowhere.""" + print("Testing setup walkthrough targets exist...") + + markup = read_admin_settings_template() + targets = _walkthrough_targets() + assert targets, "Expected the walkthrough step map to be readable" + + missing = [] + for step, card, focus in targets: + if f'id="{card}"' not in markup: + missing.append(f"step {step}: card '{card}'") + if focus and f'id="{focus}"' not in markup: + missing.append(f"step {step}: focus '{focus}'") + + assert not missing, "Walkthrough steps point at elements that do not exist:\n " + "\n ".join(missing) + + print(f"All {len(targets)} walkthrough steps point at elements that exist.") + return True + + +def test_walkthrough_does_not_hardcode_tab_ids(): + """Naming a tab id duplicates knowledge that then goes stale.""" + print("Testing the walkthrough resolves tabs from the page...") + + source = _read(ADMIN_JS) + assert "const stepToTab" not in source, ( + "The walkthrough should not map steps to tab ids. Name the card the " + "step is about and let openAdminCard find the owning tab, so the map " + "cannot go stale when a setting moves between tabs." + ) + assert "window.openAdminCard" in source, ( + "The walkthrough should navigate through openAdminCard, which resolves " + "the owning tab from the page" + ) + + print("The walkthrough resolves tabs from the page.") + return True + + +def test_every_walkthrough_step_is_mapped(): + """A gap in the map silently skips navigation for that step.""" + print("Testing walkthrough step numbering is contiguous...") + + steps = sorted(int(step) for step, _, _ in _walkthrough_targets()) + assert steps, "Expected walkthrough steps" + expected = list(range(1, len(steps) + 1)) + assert steps == expected, ( + f"Walkthrough steps should be numbered 1..{len(steps)} without gaps, got {steps}" + ) + + print(f"Steps 1..{len(steps)} are all mapped.") + return True + + +if __name__ == "__main__": + tests = [ + test_walkthrough_targets_exist, + test_walkthrough_does_not_hardcode_tab_ids, + test_every_walkthrough_step_is_mapped, + ] + results = [] + for test in tests: + print(f"\nRunning {test.__name__}...") + try: + results.append(bool(test())) + except Exception as error: # noqa: BLE001 - report and continue + print(f"Test 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_assistant_table_csv_artifact.py b/functional_tests/test_assistant_table_csv_artifact.py index 256305ccf..295758068 100644 --- a/functional_tests/test_assistant_table_csv_artifact.py +++ b/functional_tests/test_assistant_table_csv_artifact.py @@ -1154,12 +1154,13 @@ def test_chat_route_wires_assistant_table_artifacts(): 'Expected workflow model and agent execution to apply the same file-output guidance.', ) assert_true( - chat_route_content.count('function_results=agent_citations_list') == 2, - 'Expected normal and streaming Chat to pass current-turn action results to generated-file exports.', + chat_route_content.count('function_results=agent_citations_list') == 4, + 'Expected normal and streaming Chat to pass current-turn action results to both the ' + 'generated-file export and the assistant JSON/XML export.', ) assert_true( - 'function_results=execution_result.get(\'agent_citations\') or []' in chat_route_content, - 'Expected document actions to pass current-turn action results to generated-file exports.', + chat_route_content.count('function_results=execution_result.get(\'agent_citations\') or []') == 2, + 'Expected document actions to pass current-turn action results to both export paths.', ) assert_true( 'function_results=raw_agent_citations' in workflow_runner_content, diff --git a/functional_tests/test_file_sync_azure_blob_storage.py b/functional_tests/test_file_sync_azure_blob_storage.py index 67ffd15be..0f7607513 100644 --- a/functional_tests/test_file_sync_azure_blob_storage.py +++ b/functional_tests/test_file_sync_azure_blob_storage.py @@ -31,6 +31,11 @@ REPO_ROOT = Path(__file__).resolve().parents[1] +sys.path.insert(0, str(REPO_ROOT / "application" / "single_app")) + +from functions_azure_endpoint_validation import ( # noqa: E402 Imported after sys.path setup. + azure_storage_endpoint_suffix_for_hostname, +) def read_text(relative_path): @@ -80,6 +85,7 @@ def load_functions(relative_path, names, additional_globals=None): "timezone": timezone, "unquote": unquote, "urlparse": urlparse, + "azure_storage_endpoint_suffix_for_hostname": azure_storage_endpoint_suffix_for_hostname, } namespace.update(additional_globals or {}) exec(compile(module, relative_path, "exec"), namespace) diff --git a/functional_tests/test_generated_artifact_paging_and_guidance.py b/functional_tests/test_generated_artifact_paging_and_guidance.py new file mode 100644 index 000000000..69ac4e288 --- /dev/null +++ b/functional_tests/test_generated_artifact_paging_and_guidance.py @@ -0,0 +1,355 @@ +# test_generated_artifact_paging_and_guidance.py +#!/usr/bin/env python3 +""" +Functional test for paged action rows, truncation disclosure, and carried guidance. +Version: 0.260.011 +Implemented in: 0.260.011 + +A deployment test produced three defects that the log confirmed: + + 1. An agent answered "yes and all columns" to a schema clarification, replied + "I cannot create or attach a CSV file in this interface", and the server + published the CSV anyway. The publication contract was resolved from the + current user message only, so the answer turn injected no guidance at all + even though 0.260.008 already carried the format forward for publishing. + 2. Two `list_parameter_history` calls in one turn both started at the same + timestamp, so the second page re-read rows the first page already held. + The action grouping added in 0.260.008 concatenated them, producing a + 1,000-row file for a window that held roughly 500 distinct samples. + 3. Those same calls reported `truncated=True`, yet the published file gave no + indication that it covered only part of the matching data. + +This test ensures overlapping pages collapse to distinct rows, truncated source +results are disclosed on the artifact, and the guidance format falls back to the +clarification the reply is answering. +""" + +import sys +import traceback +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +APP_DIR = ROOT / 'application' / 'single_app' +if str(APP_DIR) not in sys.path: + sys.path.insert(0, str(APP_DIR)) +if str(Path(__file__).resolve().parent) not in sys.path: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +from functions_generated_file_exports import ( # noqa: E402 + GENERATED_FILE_PAGING_GUIDANCE, + GENERATED_FILE_TRUNCATED_ROWS_NOTE, + build_generated_file_artifact_metadata, + build_generated_file_export, + build_generated_file_output_guidance, + build_structured_artifact_rows_payload, + extract_authorized_function_result_rows, + function_results_report_truncated_rows, +) + + +def _telemetry_rows(start_index, count): + return [ + { + 'generation_time': f'2026-08-19T20:40:{index % 60:02d}.000Z', + 'eng_value': round(11.0 + (index * 0.001), 4), + 'raw_value': index, + 'monitoring_result': 'IN_LIMITS', + } + for index in range(start_index, start_index + count) + ] + + +def _history_call(rows, truncated): + return { + 'success': True, + 'plugin_name': 'yamcs_plugin', + 'function_name': 'list_parameter_history', + 'function_result': { + 'instance': 'simulator', + 'row_count': len(rows), + 'truncated': truncated, + 'rows': rows, + }, + } + + +def test_overlapping_pages_collapse_to_distinct_rows(): + """Re-reading an action from the same start must not double the dataset.""" + print("Testing overlapping paged action results...") + try: + full_page = _telemetry_rows(0, 500) + # The agent narrowed only the stop time, so this page repeats the first 300 rows. + overlapping_page = _telemetry_rows(0, 300) + rows = extract_authorized_function_result_rows([ + _history_call(full_page, True), + _history_call(overlapping_page, False), + ]) + + assert len(rows) == 500, f'expected 500 distinct rows, got {len(rows)}' + signatures = {tuple(sorted(row.items())) for row in rows} + assert len(signatures) == 500, 'distinct row signatures were not preserved' + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_non_overlapping_pages_still_combine(): + """Genuine continuation pages must still extend the dataset.""" + print("Testing non-overlapping paged action results...") + try: + rows = extract_authorized_function_result_rows([ + _history_call(_telemetry_rows(0, 500), True), + _history_call(_telemetry_rows(500, 401), False), + ]) + + assert len(rows) == 901, f'expected 901 combined rows, got {len(rows)}' + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_truncated_source_is_reported(): + """A capped action result must be recognizable as partial.""" + print("Testing truncation detection...") + try: + assert function_results_report_truncated_rows([ + _history_call(_telemetry_rows(0, 500), True), + ]) is True, 'truncated result was not detected' + assert function_results_report_truncated_rows([ + _history_call(_telemetry_rows(0, 401), False), + ]) is False, 'complete result was reported as truncated' + assert function_results_report_truncated_rows([]) is False, 'empty results reported truncation' + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_csv_export_discloses_truncated_rows(): + """The CSV artifact must say the source action returned only part of the data.""" + print("Testing CSV truncation disclosure...") + try: + export_payload = build_generated_file_export( + 'convert the parameter history to csv without changes', + 'Here is the telemetry export.', + function_results=[_history_call(_telemetry_rows(0, 500), True)], + ) + + assert export_payload is not None, 'no CSV export was produced' + assert export_payload['row_count'] == 500, f"unexpected row count {export_payload['row_count']}" + assert export_payload['rows_truncated'] is True, 'truncation flag missing from payload' + assert GENERATED_FILE_TRUNCATED_ROWS_NOTE in export_payload['summary'], ( + 'summary did not disclose the truncation' + ) + + artifact_metadata = build_generated_file_artifact_metadata( + export_payload, + {'message': {'id': 'artifact-1', 'file_name': export_payload['file_name']}}, + 'conversation-1', + ) + assert artifact_metadata is not None, 'no artifact metadata was produced' + assert artifact_metadata.get('rows_truncated') is True, 'truncation flag missing from artifact' + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_complete_export_is_not_marked_partial(): + """A complete action result must not be labeled partial.""" + print("Testing complete export labeling...") + try: + export_payload = build_generated_file_export( + 'convert the parameter history to csv without changes', + 'Here is the telemetry export.', + function_results=[_history_call(_telemetry_rows(0, 401), False)], + ) + + assert export_payload is not None, 'no CSV export was produced' + assert export_payload['rows_truncated'] is False, 'complete export was flagged as truncated' + assert GENERATED_FILE_TRUNCATED_ROWS_NOTE not in export_payload['summary'], ( + 'complete export summary claimed truncation' + ) + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_structured_payload_reports_truncation(): + """JSON and XML artifacts must carry the same truncation signal as CSV.""" + print("Testing structured artifact truncation signal...") + try: + for output_format in ('json', 'xml'): + payload = build_structured_artifact_rows_payload( + f'convert the parameter history to {output_format} without changes', + output_format, + function_results=[_history_call(_telemetry_rows(0, 500), True)], + ) + assert payload is not None, f'no {output_format} payload was produced' + assert payload['rows_truncated'] is True, f'{output_format} payload lost the truncation flag' + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_reachback_rows_carry_prior_truncation(): + """Rows recovered from an earlier turn must keep that turn's truncation signal.""" + print("Testing reach-back truncation signal...") + try: + prior_results = [_history_call(_telemetry_rows(0, 500), True)] + export_payload = build_generated_file_export( + 'convert those results to csv without changes', + 'Using the telemetry already retrieved.', + function_results=[], + prior_function_results_loader=lambda: prior_results, + ) + + assert export_payload is not None, 'no CSV export was produced from prior rows' + assert export_payload['row_source'] == 'earlier action result', ( + f"unexpected row source {export_payload['row_source']}" + ) + assert export_payload['rows_truncated'] is True, 'reach-back lost the truncation flag' + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_guidance_carries_pending_format(): + """A clarification answer must still receive the publication contract.""" + print("Testing carried guidance format...") + try: + answer_guidance = build_generated_file_output_guidance( + 'yes and all columns', + requested_format='csv', + ) + assert 'cannot create or attach files' in answer_guidance, ( + 'carried guidance omitted the publication contract' + ) + + # Without a carried format the answer turn has no artifact contract to state. + assert build_generated_file_output_guidance('yes and all columns') == '', ( + 'guidance was emitted without a requested or pending format' + ) + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_paging_guidance_reaches_prose_formats(): + """Formats that can explain themselves must be told how to page past truncation.""" + print("Testing paging guidance coverage...") + try: + for output_format in ('csv', 'docx', 'pdf'): + guidance = build_generated_file_output_guidance( + f'create a {output_format} of the parameter history', + ) + assert GENERATED_FILE_PAGING_GUIDANCE in guidance, ( + f'{output_format} guidance omitted the paging instruction' + ) + + # JSON and XML must return only the payload, so prose paging advice would conflict. + for output_format in ('json', 'xml'): + guidance = build_generated_file_output_guidance( + f'create a {output_format} of the parameter history', + ) + assert GENERATED_FILE_PAGING_GUIDANCE not in guidance, ( + f'{output_format} guidance added prose that conflicts with payload-only output' + ) + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_route_resolves_guidance_format_from_clarification(): + """The chat routes must resolve guidance from the pending format, not the raw message.""" + print("Testing route guidance format resolution...") + try: + route_source = (APP_DIR / 'route_backend_chats.py').read_text(encoding='utf-8') + + assert route_source.count('_resolve_generated_file_guidance_format(') == 3, ( + 'expected one helper definition and both chat paths to use it' + ) + assert 'requested_format=get_tabular_generated_output_format(user_message)' not in route_source, ( + 'a guidance call still resolves the format from the current message only' + ) + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +def test_artifact_card_renders_the_partial_badge(): + """The completed-artifact card hides the summary, so truncation needs a visible badge.""" + print("Testing artifact card truncation badge...") + try: + card_source = ( + APP_DIR / 'static' / 'js' / 'chat' / 'chat-messages.js' + ).read_text(encoding='utf-8') + + assert "outputMetadata?.rows_truncated" in card_source, ( + 'the artifact card never reads the truncation flag' + ) + assert "truncatedBadge.textContent = 'Partial'" in card_source, ( + 'the artifact card does not render a partial badge' + ) + print("Test passed!") + return True + except Exception as e: + print(f"Test failed: {e}") + traceback.print_exc() + return False + + +if __name__ == "__main__": + assert_app_version_at_least("0.260.011") + + tests = [ + test_overlapping_pages_collapse_to_distinct_rows, + test_non_overlapping_pages_still_combine, + test_truncated_source_is_reported, + test_csv_export_discloses_truncated_rows, + test_complete_export_is_not_marked_partial, + test_structured_payload_reports_truncation, + test_reachback_rows_carry_prior_truncation, + test_guidance_carries_pending_format, + test_paging_guidance_reaches_prose_formats, + test_route_resolves_guidance_format_from_clarification, + test_artifact_card_renders_the_partial_badge, + ] + + results = [] + for test in tests: + print(f"\nRunning {test.__name__}...") + results.append(test()) + + print(f"\nResults: {sum(results)}/{len(results)} tests passed") + sys.exit(0 if all(results) else 1) diff --git a/functional_tests/test_generated_csv_uses_authorized_action_rows.py b/functional_tests/test_generated_csv_uses_authorized_action_rows.py new file mode 100644 index 000000000..698327642 --- /dev/null +++ b/functional_tests/test_generated_csv_uses_authorized_action_rows.py @@ -0,0 +1,602 @@ +# test_generated_csv_uses_authorized_action_rows.py +#!/usr/bin/env python3 +""" +Functional test for CSV artifacts built from authorized action rows. +Version: 0.260.009 +Implemented in: 0.260.007; earlier-turn reach-back and clarification carry-forward in 0.260.008; history-limit window and compact-citation scan in 0.260.009 + +Two customer conversations asked a telemetry agent to retrieve BatteryVoltage1 +and create a CSV. The agent retrieved 900 authorized samples, but the published +artifact held 3 rows in one conversation and 2 rows of server discovery metadata +in the other. + +Three defects combined: + 1. build_generated_file_export() let any assistant-rendered table outrank the + authorized action rows, so an illustrative excerpt replaced the dataset. + 2. CSV never received the publication contract JSON/XML already had, so the + model said it could not attach files and pasted a sample instead. + 3. A turn whose reply was the schema clarification still published a CSV, + built from whatever incidental discovery rows the turn happened to produce. + 4. Discovery and lookup calls were merged into the retrieved dataset, adding + junk rows and a Source action column to the artifact. + 5. Nothing reached back to data an earlier turn already gathered, so a + follow-up "create a csv" and an answered clarification both had no rows. + +This test ensures the authorized rows win over an excerpt, a deliberate +assistant table still wins, a clarification turn publishes nothing, and an +answered clarification resumes the original request using the earlier rows. +""" + +import json +import sys +import traceback +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +APP_DIR = ROOT / 'application' / 'single_app' +if str(APP_DIR) not in sys.path: + sys.path.insert(0, str(APP_DIR)) +if str(Path(__file__).resolve().parent) not in sys.path: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +from functions_generated_file_exports import ( # noqa: E402 + build_generated_file_export, + build_generated_file_output_guidance, + estimate_function_result_row_count, + extract_authorized_function_result_rows, + resolve_pending_generated_file_format, + select_prior_turn_action_citations, +) + +IMPLEMENTED_VERSION = '0.260.007' + +TELEMETRY_COLUMNS = ( + 'generation_time_utc', + 'reception_time_utc', + 'battery_voltage_1_v', + 'raw_value', + 'monitoring_result', + 'validity_status', +) +TELEMETRY_ROWS = [ + { + # High-granularity telemetry carries a distinct acquisition time per sample, which is what + # lets 0.260.011 tell a genuine continuation page apart from a re-read of the same window. + 'generation_time_utc': f'2026-08-19T15:{37 + index // 60:02d}:{index % 60:02d}.371000+00:00', + 'reception_time_utc': f'2026-08-19T15:{37 + index // 60:02d}:{index % 60:02d}.372000+00:00', + 'battery_voltage_1_v': 27 + (index % 5), + 'raw_value': 27 + (index % 5), + 'monitoring_result': 'CRITICAL', + 'validity_status': 'ACQUIRED', + } + for index in range(900) +] +TELEMETRY_FUNCTION_RESULTS = [{ + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'row_count': 900, 'rows': TELEMETRY_ROWS}, +}] +# The real turn also ran two discovery calls before paging the history. +FULL_TURN_FUNCTION_RESULTS = [ + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_instances', + 'success': True, + 'function_result': {'rows': [{'name': 'simulator', 'state': 'RUNNING'}]}, + }, + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameters', + 'success': True, + 'function_result': {'rows': [{'qualified_name': '/YSS/SIMULATOR/BatteryVoltage1', 'units': 'V'}]}, + }, + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'rows': TELEMETRY_ROWS[:450]}, + }, + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'rows': TELEMETRY_ROWS[450:]}, + }, +] +TELEMETRY_QUESTION = ( + 'grab BatteryVoltage1 over the last 15 minutes, provide high granularity and create a csv' +) + +DISCOVERY_FUNCTION_RESULTS = [ + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_instances', + 'success': True, + 'function_result': {'rows': [{'name': 'simulator', 'state': 'RUNNING'}]}, + }, + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameters', + 'success': True, + 'function_result': {'rows': [{'qualified_name': '/YSS/SIMULATOR/BatteryVoltage1', 'units': 'V'}]}, + }, +] +CLARIFICATION_REPLY = ( + 'Paul, should each CSV row represent an extracted BatteryVoltage1 telemetry sample, ' + 'and which columns should it include, for example: generation time, voltage (V), ' + 'monitoring result, and validity status?' +) + + +def assert_true(condition, message): + if not condition: + raise AssertionError(message) + + +def build_sample_excerpt_reply(excerpt_row_count=3): + """Rebuild the assistant reply that pasted real rows as an illustrative sample.""" + sample_lines = [','.join(TELEMETRY_COLUMNS)] + for row in TELEMETRY_ROWS[:excerpt_row_count]: + sample_lines.append(','.join(str(row[column]) for column in TELEMETRY_COLUMNS)) + rendered_sample = '\n'.join(sample_lines) + return ( + 'Paul, I retrieved **BatteryVoltage1** at 1 Hz. Samples: 900. Observed range 27 to 31 V.\n\n' + 'I do not have a file-creation or attachment capability in this session, so I cannot ' + 'deliver a downloadable CSV artifact. The CSV schema is:\n\n' + f'```\n{rendered_sample}\n```\n\n' + 'The archive response establishes the full 900-row dataset.\n' + ) + + +def test_pasted_sample_does_not_replace_authorized_rows(): + """An excerpt of the authorized rows must not shrink the published artifact.""" + print('Testing pasted-sample precedence...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + TELEMETRY_QUESTION, + build_sample_excerpt_reply(), + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected the telemetry request to publish a CSV artifact.') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + f"Expected all {len(TELEMETRY_ROWS)} authorized rows; got {export_payload['row_count']}.", + ) + assert_true( + export_payload['row_source'] == 'structured function result', + 'Expected the authorized action rows to supply the artifact.', + ) + assert_true( + list(export_payload['preview_rows'][0]) == list(TELEMETRY_COLUMNS), + 'Expected the artifact to keep the telemetry column schema.', + ) + + +def test_deliberate_assistant_table_still_wins(): + """A table the model composed itself is not an excerpt and must stay authoritative.""" + print('Testing deliberate assistant table precedence...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + directory_results = [ + { + 'plugin_name': 'DirectoryPlugin', + 'function_name': 'list_people', + 'success': True, + 'function_result': {'value': [{'Name': 'Ada', 'Department': 'Engineering'}]}, + }, + { + 'plugin_name': 'DirectoryPlugin', + 'function_name': 'list_contractors', + 'success': True, + 'function_result': {'items': [{'Name': 'Grace', 'Department': 'Operations'}]}, + }, + ] + export_payload = build_generated_file_export( + 'create a combined CSV', + '| Name | Department |\n| --- | --- |\n| Assistant-selected | Finance |\n', + function_results=directory_results, + ) + + assert_true(export_payload is not None, 'Expected the combined CSV request to publish an artifact.') + assert_true( + export_payload['row_source'] == 'assistant response', + 'Expected a model-composed table to remain authoritative over action rows.', + ) + assert_true(export_payload['row_count'] == 1, 'Expected the model-composed row to be preserved.') + + +def test_partial_excerpt_of_larger_result_is_still_replaced(): + """A longer excerpt is still an excerpt, not a derived answer.""" + print('Testing multi-row excerpt precedence...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + TELEMETRY_QUESTION, + build_sample_excerpt_reply(excerpt_row_count=25), + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected the telemetry request to publish a CSV artifact.') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected a 25-row excerpt to be replaced by the full authorized result set.', + ) + + +def test_clarification_turn_publishes_no_artifact(): + """The turn that asks the schema clarification must not publish a CSV.""" + print('Testing clarification-turn suppression...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + 'create a csv', + CLARIFICATION_REPLY, + function_results=DISCOVERY_FUNCTION_RESULTS, + ) + + assert_true( + export_payload is None, + 'Expected a clarifying question to publish no artifact instead of discovery metadata.', + ) + + +def test_answered_turn_still_publishes_action_rows(): + """Suppression must be limited to clarification replies, not ordinary summaries.""" + print('Testing normal reply still publishes action rows...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + 'create a csv', + 'Retrieved the BatteryVoltage1 archive window and prepared the export.', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected an ordinary reply to still publish the artifact.') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected the full authorized result set for an ordinary reply.', + ) + + +def test_discovery_calls_do_not_dilute_the_retrieved_dataset(): + """Lookup and discovery calls in the same turn must not become artifact rows.""" + print('Testing dominant action-result selection...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + rows = extract_authorized_function_result_rows(FULL_TURN_FUNCTION_RESULTS) + + assert_true( + len(rows) == len(TELEMETRY_ROWS), + f'Expected only the {len(TELEMETRY_ROWS)} retrieved samples; got {len(rows)}.', + ) + assert_true( + list(rows[0]) == list(TELEMETRY_COLUMNS), + 'Expected the telemetry schema without a Source action column from discovery calls.', + ) + + +def test_paged_calls_to_one_action_stay_one_dataset(): + """Two pages of the same action are one dataset, not two labeled groups.""" + print('Testing paged action-result grouping...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + paged_results = [ + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'rows': TELEMETRY_ROWS[:450]}, + }, + { + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'rows': TELEMETRY_ROWS[450:]}, + }, + ] + rows = extract_authorized_function_result_rows(paged_results) + + assert_true(len(rows) == len(TELEMETRY_ROWS), 'Expected both pages to be kept.') + assert_true( + 'Source action' not in rows[0], + 'Expected one action to stay one dataset without a Source action column.', + ) + + +def test_followup_request_reuses_earlier_turn_rows(): + """'create a csv' must reach back to data an earlier turn already gathered.""" + print('Testing earlier-turn reach-back...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + 'create a csv', + 'Here is the CSV of the BatteryVoltage1 samples I retrieved.', + function_results=[], + prior_function_results_loader=lambda: FULL_TURN_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected the follow-up request to publish an artifact.') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected the earlier turn\'s full result set to be reused.', + ) + assert_true( + export_payload['row_source'] == 'earlier action result', + 'Expected reused rows to declare their earlier-turn provenance.', + ) + + +def test_current_turn_rows_are_never_double_counted(): + """The reach-back must not run when the turn gathered its own rows.""" + print('Testing reach-back suppression when the turn has data...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + 'create a csv', + 'Retrieved the archive window and prepared the export.', + function_results=FULL_TURN_FUNCTION_RESULTS, + prior_function_results_loader=lambda: FULL_TURN_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected the request to publish an artifact.') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected current-turn rows only, with no earlier-turn rows appended.', + ) + assert_true( + export_payload['row_source'] == 'structured function result', + 'Expected current-turn provenance when the turn gathered its own rows.', + ) + + +def test_answering_the_clarification_publishes_the_csv(): + """Answering the schema clarification must resume the original CSV request.""" + print('Testing clarification answer carry-forward...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + pending_format = resolve_pending_generated_file_format( + 'yes, one row per sample', + CLARIFICATION_REPLY, + ) + assert_true(pending_format == 'csv', 'Expected the answered clarification to resume a CSV request.') + + export_payload = build_generated_file_export( + 'yes, one row per sample', + 'Understood, one row per sample.', + function_results=[], + prior_function_results_loader=lambda: FULL_TURN_FUNCTION_RESULTS, + pending_output_format=pending_format, + ) + + assert_true(export_payload is not None, 'Expected the answered clarification to publish the CSV.') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected the answered clarification to export the already-gathered samples.', + ) + + +def test_pending_clarification_does_not_override_a_new_request(): + """A reply that asks for another format or follows no clarification carries nothing.""" + print('Testing pending clarification boundaries...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + assert_true( + resolve_pending_generated_file_format('actually make it a pdf', CLARIFICATION_REPLY) is None, + 'Expected a different requested format to cancel the pending CSV request.', + ) + assert_true( + resolve_pending_generated_file_format('yes, one row per sample', 'Here are the results.') is None, + 'Expected no pending request when the previous turn asked nothing.', + ) + assert_true( + resolve_pending_generated_file_format('', CLARIFICATION_REPLY) is None, + 'Expected an empty reply to carry no pending request.', + ) + + +def _build_compact_citation(artifact_id, plugin_name, function_name, function_result): + """Compact a citation exactly the way the chat pipeline stores it on a message.""" + import types + + if 'functions_azure_maps' not in sys.modules: + # functions_message_artifacts only needs this for map payloads, and importing the real + # module pulls in config.py and a live Cosmos client. + azure_maps_stub = types.ModuleType('functions_azure_maps') + azure_maps_stub.refresh_azure_maps_citation_payload = lambda payload: payload + sys.modules['functions_azure_maps'] = azure_maps_stub + + from functions_message_artifacts import build_compact_agent_citation + + return build_compact_agent_citation( + { + 'plugin_name': plugin_name, + 'function_name': function_name, + 'success': True, + 'function_result': function_result, + }, + artifact_id=artifact_id, + ) + + +def test_row_count_estimate_survives_citation_compaction(): + """The cheap scan depends on compaction keeping a usable row signal on the message.""" + print('Testing compacted row-count estimate...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + compact_citation = _build_compact_citation( + 'm1_artifact_2', + 'YamcsPlugin', + 'list_parameter_history', + {'instance': 'simulator', 'row_count': 900, 'rows': TELEMETRY_ROWS}, + ) + + assert_true( + len(json.dumps(compact_citation)) < 4000, + 'Expected the stored citation to stay compact.', + ) + assert_true( + estimate_function_result_row_count(compact_citation) == len(TELEMETRY_ROWS), + 'Expected the compacted citation to still report its full row count.', + ) + + unlabeled_citation = _build_compact_citation( + 'm1_artifact_3', + 'YamcsPlugin', + 'list_parameter_history', + {'rows': TELEMETRY_ROWS}, + ) + assert_true( + estimate_function_result_row_count(unlabeled_citation) == len(TELEMETRY_ROWS), + 'Expected the truncated row list marker to restore the full count.', + ) + + +def test_reach_back_prefers_the_dataset_over_newer_lookups(): + """A newer memory or discovery call must not outrank the dataset a request means.""" + print('Testing prior-turn action selection...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + assistant_messages = [ + { + 'id': 'm3', + 'agent_citations': [_build_compact_citation( + 'm3_artifact_1', 'FactMemoryPlugin', 'get_facts', + {'facts': [{'key': 'name', 'value': 'Paul'}]}, + )], + }, + { + 'id': 'm2', + 'agent_citations': [_build_compact_citation( + 'm2_artifact_1', 'YamcsPlugin', 'list_instances', + {'rows': [{'name': 'simulator', 'state': 'RUNNING'}]}, + )], + }, + { + 'id': 'm1', + 'agent_citations': [ + _build_compact_citation( + 'm1_artifact_1', 'YamcsPlugin', 'list_instances', + {'rows': [{'name': 'simulator', 'state': 'RUNNING'}]}, + ), + _build_compact_citation( + 'm1_artifact_2', 'YamcsPlugin', 'list_parameter_history', + {'row_count': 450, 'rows': TELEMETRY_ROWS[:450]}, + ), + _build_compact_citation( + 'm1_artifact_3', 'YamcsPlugin', 'list_parameter_history', + {'row_count': 450, 'rows': TELEMETRY_ROWS[450:]}, + ), + ], + }, + ] + + selected_citations = select_prior_turn_action_citations(assistant_messages) + + assert_true(len(selected_citations) == 2, 'Expected both pages of the dataset action.') + assert_true( + [citation['artifact_id'] for citation in selected_citations] == ['m1_artifact_2', 'm1_artifact_3'], + 'Expected only the dataset artifacts to be selected for hydration.', + ) + assert_true( + all(citation['function_name'] == 'list_parameter_history' for citation in selected_citations), + 'Expected discovery and memory lookups to be skipped.', + ) + + +def test_reach_back_falls_back_to_the_only_rows_available(): + """When no turn holds a dataset, the most recent rows are still better than nothing.""" + print('Testing prior-turn fallback selection...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + assistant_messages = [ + {'id': 'm2', 'agent_citations': [_build_compact_citation( + 'm2_artifact_1', 'DirectoryPlugin', 'list_people', + {'rows': [{'Name': 'Ada'}, {'Name': 'Grace'}]}, + )]}, + {'id': 'm1', 'agent_citations': []}, + ] + + selected_citations = select_prior_turn_action_citations(assistant_messages) + + assert_true(len(selected_citations) == 1, 'Expected the only available action to be selected.') + assert_true( + selected_citations[0]['artifact_id'] == 'm2_artifact_1', + 'Expected the most recent available rows to be used.', + ) + assert_true( + select_prior_turn_action_citations([]) == [], + 'Expected an empty history to select nothing.', + ) + + +def test_csv_guidance_states_the_publication_contract(): + """CSV must receive the same publication contract JSON and XML already state.""" + print('Testing CSV publication guidance parity...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + csv_guidance = build_generated_file_output_guidance('create a csv') + assert_true( + 'attaches the file after generation' in csv_guidance, + 'Expected CSV guidance to state that the server attaches the artifact.', + ) + assert_true( + 'cannot create or attach files' in csv_guidance, + 'Expected CSV guidance to forbid claiming files cannot be attached.', + ) + assert_true( + 'do not paste a sample of the rows' in csv_guidance, + 'Expected CSV guidance to forbid pasting a row sample instead of the artifact.', + ) + assert_true( + 'ask exactly one concise clarification' in csv_guidance, + 'Expected the existing one-clarification rule to survive.', + ) + assert_true( + build_generated_file_output_guidance('summarize the selected sources') == '', + 'Expected non-artifact requests to receive no generated-file guidance.', + ) + + +def run_tests() -> bool: + tests = [ + test_pasted_sample_does_not_replace_authorized_rows, + test_deliberate_assistant_table_still_wins, + test_partial_excerpt_of_larger_result_is_still_replaced, + test_clarification_turn_publishes_no_artifact, + test_answered_turn_still_publishes_action_rows, + test_discovery_calls_do_not_dilute_the_retrieved_dataset, + test_paged_calls_to_one_action_stay_one_dataset, + test_followup_request_reuses_earlier_turn_rows, + test_current_turn_rows_are_never_double_counted, + test_answering_the_clarification_publishes_the_csv, + test_pending_clarification_does_not_override_a_new_request, + test_row_count_estimate_survives_citation_compaction, + test_reach_back_prefers_the_dataset_over_newer_lookups, + test_reach_back_falls_back_to_the_only_rows_available, + test_csv_guidance_states_the_publication_contract, + ] + + results = [] + for test in tests: + print(f'\nRunning {test.__name__}...') + try: + test() + print(f'{test.__name__} passed') + results.append(True) + except Exception as exc: + print(f'{test.__name__} failed: {exc}') + traceback.print_exc() + results.append(False) + + passed = sum(1 for result in results if result) + print(f'\nResults: {passed}/{len(tests)} tests passed') + return all(results) + + +if __name__ == '__main__': + sys.exit(0 if run_tests() else 1) diff --git a/functional_tests/test_generated_structured_artifact_parity.py b/functional_tests/test_generated_structured_artifact_parity.py new file mode 100644 index 000000000..5e24d2e82 --- /dev/null +++ b/functional_tests/test_generated_structured_artifact_parity.py @@ -0,0 +1,261 @@ +# test_generated_structured_artifact_parity.py +#!/usr/bin/env python3 +""" +Functional test for generated artifact parity across CSV, DOCX, PDF, JSON, and XML. +Version: 0.260.010 +Implemented in: 0.260.010 + +The CSV work in 0.260.007 through 0.260.009 left three gaps in the sibling +formats and one loose end: + + 1. JSON and XML were built only by parsing the payload out of the assistant + reply, so they could never use authorized action rows and never reached + back to rows an earlier turn already gathered. + 2. The schema clarification gate lived inside the CSV branch, so a turn that + asked a clarifying question could still publish a DOCX or PDF. + 3. DOCX and PDF guidance never forbade claiming that files cannot be created, + which is the exact sentence that produced the truncated CSV. + 4. The DOCX and PDF passthrough reason code compared one row-source literal, + so it was dropped when rows came from an earlier turn. + +This test ensures every generated file format resolves rows the same way and +refuses to publish on a clarification turn. +""" + +import sys +import traceback +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +APP_DIR = ROOT / 'application' / 'single_app' +if str(APP_DIR) not in sys.path: + sys.path.insert(0, str(APP_DIR)) +if str(Path(__file__).resolve().parent) not in sys.path: + sys.path.insert(0, str(Path(__file__).resolve().parent)) + +from test_support.versioning import assert_app_version_at_least # noqa: E402 + +from functions_generated_file_exports import ( # noqa: E402 + build_generated_file_export, + build_generated_file_output_guidance, + build_structured_artifact_rows_payload, +) + +IMPLEMENTED_VERSION = '0.260.010' + +TELEMETRY_ROWS = [ + {'id': f'R-{index}', 'voltage': 27 + index % 5, 'state': 'CRITICAL'} + for index in range(30) +] +TELEMETRY_FUNCTION_RESULTS = [{ + 'plugin_name': 'YamcsPlugin', + 'function_name': 'list_parameter_history', + 'success': True, + 'function_result': {'row_count': len(TELEMETRY_ROWS), 'rows': TELEMETRY_ROWS}, +}] +CLARIFICATION_REPLY = ( + 'Should each row represent one telemetry sample, and which columns should it include?' +) + + +def assert_true(condition, message): + if not condition: + raise AssertionError(message) + + +def test_json_and_xml_use_authorized_action_rows(): + """A JSON or XML request must serialize action rows when the reply carries no payload.""" + print('Testing JSON/XML row fallback...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + json_payload = build_structured_artifact_rows_payload( + 'create a json of that data', + 'json', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + xml_payload = build_structured_artifact_rows_payload( + 'create an xml of that data', + 'xml', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(json_payload is not None, 'Expected a JSON payload from authorized action rows.') + assert_true( + json_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected every authorized row in the JSON artifact.', + ) + assert_true(json_payload['file_content'].lstrip().startswith('['), 'Expected a JSON array payload.') + assert_true(xml_payload is not None, 'Expected an XML payload from authorized action rows.') + assert_true( + '' in xml_payload['file_content'], + 'Expected the shared generated-rows XML root.', + ) + + +def test_json_and_xml_reach_back_to_earlier_turns(): + """JSON and XML must reuse rows an earlier turn gathered, the same way CSV does.""" + print('Testing JSON/XML earlier-turn reach-back...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + json_payload = build_structured_artifact_rows_payload( + 'create a json', + 'json', + function_results=[], + prior_function_results_loader=lambda: TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(json_payload is not None, 'Expected the follow-up JSON request to reuse earlier rows.') + assert_true( + json_payload['row_source'] == 'earlier action result', + 'Expected reused rows to declare their earlier-turn provenance.', + ) + assert_true( + json_payload['row_count'] == len(TELEMETRY_ROWS), + 'Expected the full earlier result set in the JSON artifact.', + ) + + +def test_row_fallback_is_limited_to_structured_formats(): + """CSV, DOCX, and PDF keep their own renderers; the fallback must not claim them.""" + print('Testing structured row fallback scope...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for output_format in ('csv', 'docx', 'pdf', '', None): + assert_true( + build_structured_artifact_rows_payload( + 'create a file', + output_format, + function_results=TELEMETRY_FUNCTION_RESULTS, + ) is None, + f'Expected {output_format!r} to be handled by its own renderer.', + ) + + +def test_row_fallback_requires_a_passthrough_contract(): + """Rows must still satisfy the passthrough contract before they become an artifact.""" + print('Testing structured row fallback eligibility...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + assert_true( + build_structured_artifact_rows_payload( + 'analyze the telemetry and summarize each sample', + 'json', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) is None, + 'Expected a derived-output request to refuse raw row passthrough.', + ) + assert_true( + build_structured_artifact_rows_payload('create a json', 'json', function_results=[]) is None, + 'Expected no artifact when no rows are available.', + ) + + +def test_clarification_turn_publishes_no_artifact_in_any_format(): + """A clarifying question must not publish a CSV, DOCX, or PDF.""" + print('Testing clarification gate across formats...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for question in ('create a csv', 'create a word document', 'create a pdf'): + assert_true( + build_generated_file_export( + question, + CLARIFICATION_REPLY, + function_results=TELEMETRY_FUNCTION_RESULTS, + ) is None, + f'Expected no artifact for a clarification turn on: {question}', + ) + + +def test_docx_and_pdf_still_publish_a_normal_answer(): + """The clarification gate must not suppress ordinary document requests.""" + print('Testing DOCX/PDF normal publication...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for question in ('create a word document', 'create a pdf'): + export_payload = build_generated_file_export( + question, + 'Here is the summary of the retrieved samples.', + function_results=TELEMETRY_FUNCTION_RESULTS, + ) + assert_true(export_payload is not None, f'Expected an artifact for: {question}') + assert_true( + export_payload['row_count'] == len(TELEMETRY_ROWS), + f'Expected the authorized rows to be embedded for: {question}', + ) + + +def test_docx_and_pdf_keep_the_reason_code_for_earlier_rows(): + """Provenance must not drop the passthrough reason code when rows come from earlier.""" + print('Testing DOCX/PDF passthrough reason code...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + export_payload = build_generated_file_export( + 'create a word document', + '', + function_results=[], + prior_function_results_loader=lambda: TELEMETRY_FUNCTION_RESULTS, + ) + + assert_true(export_payload is not None, 'Expected a DOCX artifact from earlier rows.') + assert_true( + export_payload['row_source'] == 'earlier action result', + 'Expected earlier-turn provenance on the DOCX artifact.', + ) + assert_true( + export_payload.get('passthrough_reason_code') == 'explicit_format_conversion', + 'Expected the passthrough reason code to survive earlier-turn provenance.', + ) + + +def test_docx_and_pdf_guidance_states_the_publication_contract(): + """DOCX and PDF must not be left able to claim they cannot create files.""" + print('Testing DOCX/PDF guidance parity...') + assert_app_version_at_least(IMPLEMENTED_VERSION) + + for question in ('create a word document', 'create a pdf'): + guidance = build_generated_file_output_guidance(question) + assert_true( + 'attaches the file after generation' in guidance, + f'Expected the publication contract in guidance for: {question}', + ) + assert_true( + 'cannot create or attach files' in guidance, + f'Expected the no-refusal rule in guidance for: {question}', + ) + assert_true( + 'do not invent rows' in guidance, + f'Expected the existing no-invented-rows rule to survive for: {question}', + ) + + +def run_tests() -> bool: + tests = [ + test_json_and_xml_use_authorized_action_rows, + test_json_and_xml_reach_back_to_earlier_turns, + test_row_fallback_is_limited_to_structured_formats, + test_row_fallback_requires_a_passthrough_contract, + test_clarification_turn_publishes_no_artifact_in_any_format, + test_docx_and_pdf_still_publish_a_normal_answer, + test_docx_and_pdf_keep_the_reason_code_for_earlier_rows, + test_docx_and_pdf_guidance_states_the_publication_contract, + ] + + results = [] + for test in tests: + print(f'\nRunning {test.__name__}...') + try: + test() + print(f'{test.__name__} passed') + results.append(True) + except Exception as exc: + print(f'{test.__name__} failed: {exc}') + traceback.print_exc() + results.append(False) + + passed = sum(1 for result in results if result) + print(f'\nResults: {passed}/{len(tests)} tests passed') + return all(results) + + +if __name__ == '__main__': + sys.exit(0 if run_tests() else 1)