From 88a167097ea2c454cb019d496e6f232e84a91304 Mon Sep 17 00:00:00 2001 From: Paul Lizer Date: Wed, 11 Nov 2026 10:02:34 -0500 Subject: [PATCH] Harden shared action and agent credential responses Separate shared group/global discovery payloads from credential-bearing configuration. Reuse native secret-preserving writes and conditional revisions for classic group editors, including explicit clears and safe masked-credential handling. Add offline API, SDK lifecycle, and browser regressions, update usage guidance, and bump the application patch version. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- application/single_app/config.py | 2 +- .../functions_workspace_authoring.py | 88 ++++- .../single_app/json_schema_validation.py | 12 +- .../single_app/route_backend_agents.py | 173 ++++----- .../single_app/route_backend_plugins.py | 215 ++++------- .../static/js/agent_modal_stepper.js | 4 + .../static/js/plugin_modal_stepper.js | 11 +- .../static/js/shared-credential-editor.js | 127 +++++++ .../static/js/workspace/group_agents.js | 30 +- .../static/js/workspace/group_plugins.js | 23 +- docs/guides/manage-group-workspaces.md | 9 + docs/guides/workspace-agents-and-actions.md | 6 +- .../cosmos_response_boundaries.json | 215 ++++++----- .../test_shared_action_secret_responses.py | 214 +++++++++++ .../test_shared_agent_secret_responses.py | 250 +++++++++++++ .../test_support/shared_secret_responses.py | 219 +++++++++++ .../test_workspace_authoring_backend.py | 15 +- ...test_workspace_authoring_sdk_boundaries.py | 204 +++++++++++ .../test_shared_workspace_secret_editing.py | 339 ++++++++++++++++++ 19 files changed, 1793 insertions(+), 363 deletions(-) create mode 100644 application/single_app/static/js/shared-credential-editor.js create mode 100644 functional_tests/test_shared_action_secret_responses.py create mode 100644 functional_tests/test_shared_agent_secret_responses.py create mode 100644 functional_tests/test_support/shared_secret_responses.py create mode 100644 functional_tests/test_workspace_authoring_sdk_boundaries.py create mode 100644 ui_tests/test_shared_workspace_secret_editing.py diff --git a/application/single_app/config.py b/application/single_app/config.py index fb149585b..eac683437 100644 --- a/application/single_app/config.py +++ b/application/single_app/config.py @@ -103,7 +103,7 @@ EXECUTOR_MAX_WORKERS = 30 CHAT_STREAM_EXECUTOR_MAX_WORKERS = max(8, int(os.getenv('SIMPLECHAT_CHAT_STREAM_WORKERS', '32') or '32')) SESSION_TYPE = 'filesystem' -VERSION = "0.261.427" +VERSION = "0.261.428" IS_DEVELOPMENT = is_development_env_enabled() # Opt-out for deployments where App Service Easy Auth is active but the platform diff --git a/application/single_app/functions_workspace_authoring.py b/application/single_app/functions_workspace_authoring.py index e3e0f9769..096d6f5fe 100644 --- a/application/single_app/functions_workspace_authoring.py +++ b/application/single_app/functions_workspace_authoring.py @@ -214,7 +214,10 @@ def editor_secret_paths(record, kind): redacted = import_module("functions_keyvault").redact_plugin_secret_values(record) paths.update(path for path, value in _walk(redacted) if value == EDITOR_SECRET_MASK) else: - paths.update({("azure_openai_gpt_key",), ("azure_agent_apim_gpt_subscription_key",)}) + paths.update({ + ("azure_openai_gpt_key",), ("azure_agent_apim_gpt_subscription_key",), + ("azure_apim_gpt_subscription_key",), + }) for path, value in _walk(record): if not path: continue @@ -269,6 +272,78 @@ def editor_resource(record, kind, *, global_scope=False, group_scope=False, read } +def project_shared_record(record, kind, *, can_manage=False, global_scope=False, group_scope=False): + """Keep legacy envelopes while separating capability metadata from editor configuration.""" + if kind not in {"actions", "agents"}: + raise WorkspaceAuthoringValidation("Invalid workspace resource.") + resource = None + if can_manage and not global_scope: + resource = editor_resource(record, kind, group_scope=group_scope, read_only=False) + result = resource["record"] + result.update({key: resource[key] for key in ("revision", "secret_paths", "read_only")}) + else: + fields = ( + "id", "name", "displayName", "display_name", "description", "type", + "agent_type", "enabled", "is_enabled", "scope", "scope_id", "group_id", + "group_name", "catalog_key", "execution_status", "is_legacy", + ) + result = { + key: value for key in fields + if isinstance((value := record.get(key)), (str, bool)) + } + if kind == "agents": + tags = record.get("tags") + result["tags"] = [tag for tag in tags if isinstance(tag, str)] if isinstance(tags, list) else [] + icon = record.get("icon") + if isinstance(icon, dict): + result["icon"] = { + key: value for key in ("kind", "type", "value", "name", "color") + if isinstance((value := icon.get(key)), str) + } + result.update(is_global=global_scope, is_group=group_scope, read_only=True) + if kind == "agents": + required = record.get("credential_reentry_required") + # Cleaned list rows have no bindings; only stored records can recompute readiness. + if record.get("_etag"): + if resource is None: + resource = editor_resource(record, kind, global_scope=global_scope, group_scope=group_scope, read_only=True) + required = resource.get("credential_reentry_required", required) + result.pop("credential_reentry_required", None) + if isinstance(required, list): + result["credential_reentry_required"] = [ + {"path": field["path"], "label": field["label"]} + for field in required if isinstance(field, dict) + and isinstance(field.get("path"), str) and isinstance(field.get("label"), str) + ] + return result + + +def legacy_editor_write_body(payload, existing, kind): + """Adapt classic manifests to the canonical merge/clear/CAS contract.""" + if not isinstance(payload, dict): + raise WorkspaceAuthoringValidation("Updates must be an object.") + presentation = {"revision", "secret_paths", "read_only", "credential_reentry_required"} + intent = {"expected_revision", "clear_secret_paths", "removed_paths"} + updates = { + key: deepcopy(value) for key, value in payload.items() + if key not in presentation | intent + and not key.startswith("_") + and (key not in _MANAGED_FIELDS or (key == "id" and not existing and kind == "agents")) + } + body = { + "updates": updates, + "clear_secret_paths": payload.get("clear_secret_paths", []), + "removed_paths": payload.get("removed_paths", []), + } + if existing: + # Older partial-update clients omit revisions; their authoritative snapshot + # still goes through CAS. Updated editors supply their actual read revision. + body["expected_revision"] = payload.get( + "expected_revision", payload.get("revision", existing.get("_etag")), + ) + return body + + def clear_editor_test_secrets(record, clear_secret_paths): """Remove explicit clears from authorized, transient test/discovery fallback state.""" if not isinstance(record, dict): @@ -432,6 +507,7 @@ def merge_editor_write(existing, payload, kind, *, creating=False, resolve_store raise WorkspaceAuthoringValidation("A field cannot be both removed and cleared.") existing = deepcopy(existing) existing_secrets = editor_secret_paths(existing, kind) + submitted_secrets = editor_secret_paths(updates, kind) if resolve_stored_placeholder: for path in existing_secrets - paths["clear_secret_paths"]: submitted = _get(updates, path) @@ -464,6 +540,10 @@ def merge(before, changes, path=(), *, replace_objects=False): if path not in existing_secrets or before is _MISSING or _is_placeholder(before): raise WorkspaceAuthoringValidation("A stored secret is unavailable. Re-enter its value.") return deepcopy(before) + if path in existing_secrets | submitted_secrets and isinstance(changes, str) and any( + marker in changes for marker in EDITOR_SECRET_PLACEHOLDERS + ): + raise WorkspaceAuthoringValidation("Re-enter the complete credential rather than combining it with a stored-value marker.") if _is_reference(changes): if path not in existing_secrets or before != changes: raise WorkspaceAuthoringValidation("Secret references cannot be supplied by an editor.") @@ -479,7 +559,11 @@ def merge(before, changes, path=(), *, replace_objects=False): continue parent = _get(result, path[:-1]) if isinstance(parent, dict): - parent.pop(path[-1]) + if kind == "actions" and path == ("auth", "key") and parent.get("type") == "key": + # The manifest schema requires this leaf even for unauthenticated OpenAPI. + parent["key"] = "" + else: + parent.pop(path[-1]) else: _set(result, path, "") for path in paths["removed_paths"]: diff --git a/application/single_app/json_schema_validation.py b/application/single_app/json_schema_validation.py index 9ddf6003f..d951e0694 100644 --- a/application/single_app/json_schema_validation.py +++ b/application/single_app/json_schema_validation.py @@ -189,13 +189,21 @@ def load_schema(schema_name): schema = json.load(f) return schema -def validate_agent(agent): +def validate_agent(agent, *, existing_id=None): + """Keep creation strict while accepting an unchanged, authorized legacy ID.""" schema = load_schema('agent.schema.json') if schema.get("$ref") and schema.get("definitions"): validator = Draft7Validator(schema, resolver=RefResolver.from_schema(schema)) else: validator = Draft7Validator(schema) - errors = sorted(validator.iter_errors(agent), key=lambda e: e.path) + errors = sorted(( + error for error in validator.iter_errors(agent) + if not ( + isinstance(existing_id, str) and existing_id + and agent.get("id") == existing_id + and tuple(error.path) == ("id",) and error.validator == "pattern" + ) + ), key=lambda error: error.path) if errors: return '; '.join([e.message for e in errors]) return None diff --git a/application/single_app/route_backend_agents.py b/application/single_app/route_backend_agents.py index d907199f4..51b2dc3d7 100644 --- a/application/single_app/route_backend_agents.py +++ b/application/single_app/route_backend_agents.py @@ -17,7 +17,7 @@ from functions_ai_connections import filter_model_endpoints_by_capability, supports_model_capability from functions_global_agents import get_global_agents, save_global_agent, delete_global_agent, update_global_agent_enabled from functions_personal_agents import get_personal_agents, ensure_migration_complete, save_personal_agent, delete_personal_agent -from functions_group import require_active_group, assert_group_role +from functions_group import require_active_group, assert_group_role, find_group_by_id from functions_agent_payload import ( AgentPayloadError, can_agent_use_default_multi_endpoint_model, @@ -68,12 +68,16 @@ validate_agent_delegation_bindings, ) from functions_workspace_authoring import ( + apply_group_agent_write, build_agent_editor_options, editor_error_response, ensure_editor_options_access, + legacy_editor_write_body, personal_editor_response, + project_shared_record, + read_group_editor_record, ) -from functions_group_agent_policy import group_agent_write_roles, group_agents_available +from functions_group_agent_policy import group_agent_management_operations, group_agent_write_roles, group_agents_available from functions_settings import is_agent_assistant_enabled from functions_editor_assist import EditorAssistError from functions_editor_assist_runtime import ( @@ -1139,6 +1143,10 @@ def get_user_agents(): agent.setdefault('agent_type', 'local') global_agents = [agent for agent in global_agents if _is_agent_allowed_for_user_selection(user_id, agent)] + global_agents = [ + project_shared_record(agent, 'agents', global_scope=True) + for agent in global_agents + ] # Merge agents using ID as key to avoid name conflicts # This allows both personal and global agents with same name to coexist @@ -1318,7 +1326,7 @@ def _prepare_group_agent_payload(user_id, group_id, agent, settings, existing): except AssignedKnowledgeError as exc: return None, (jsonify({'error': str(exc)}), 400) - validation_error = validate_agent(cleaned_agent) + validation_error = validate_agent(cleaned_agent, existing_id=(existing or {}).get('id')) if validation_error: return None, (jsonify({'error': f'Agent validation failed: {validation_error}'}), 400) @@ -1650,7 +1658,7 @@ def get_group_agents_route(): user_id = get_current_user_id() try: active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) - assert_group_role( + role = assert_group_role( user_id, active_group, allowed_roles=("Owner", "Admin", "DocumentManager", "User"), @@ -1669,7 +1677,16 @@ def get_group_agents_route(): agent['is_group'] = True agent['group_id'] = active_group agents = [agent for agent in agents if isinstance(agent, dict) and _is_agent_allowed_for_user_selection(user_id, agent)] - return jsonify({'agents': agents}), 200 + settings = get_settings() + can_manage = bool(group_agent_management_operations( + user_id, find_group_by_id(active_group), role, settings, + )) + response = jsonify({'agents': [ + project_shared_record(agent, 'agents', can_manage=can_manage, group_scope=True) + for agent in agents + ]}) + response.headers['Cache-Control'] = 'no-store' + return response, 200 @bpa.route('/api/group/agents/', methods=['GET']) @@ -1682,7 +1699,7 @@ def get_group_agent_route(agent_id): user_id = get_current_user_id() try: active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) - assert_group_role( + role = assert_group_role( user_id, active_group, allowed_roles=("Owner", "Admin", "DocumentManager", "User"), @@ -1694,10 +1711,17 @@ def get_group_agent_route(agent_id): except PermissionError as exc: return jsonify({'error': str(exc)}), 403 - agent = get_group_agent(active_group, agent_id) - if not agent: - return jsonify({'error': 'Agent not found'}), 404 - return jsonify(agent), 200 + settings = get_settings() + can_manage = bool(group_agent_management_operations( + user_id, find_group_by_id(active_group), role, settings, + )) + try: + agent = read_group_editor_record('agents', user_id, active_group, agent_id, settings) + response = jsonify(project_shared_record(agent, 'agents', can_manage=can_manage, group_scope=True)) + except Exception as exc: + return editor_error_response(exc) + response.headers['Cache-Control'] = 'no-store' + return response, 200 @bpa.route('/api/group/agents', methods=['POST']) @@ -1709,10 +1733,13 @@ def get_group_agent_route(agent_id): def create_group_agent_route(): user_id = get_current_user_id() try: - active_group = require_active_group(user_id) + active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) app_settings = get_settings() allowed_roles = ("Owner",) if app_settings.get('require_owner_for_group_agent_management') else ("Owner", "Admin") - assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + role = assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + group = find_group_by_id(active_group) + if not group or 'create' not in group_agent_management_operations(user_id, group, role, app_settings): + raise PermissionError('Group agent management is unavailable.') except ValueError as exc: return jsonify({'error': str(exc)}), 400 except LookupError as exc: @@ -1720,43 +1747,20 @@ def create_group_agent_route(): except PermissionError as exc: return jsonify({'error': str(exc)}), 403 - payload = request.get_json(silent=True) or {} - try: - validate_group_agent_payload(payload, partial=False) - cleaned_payload = sanitize_agent_payload(payload) - except (ValueError, AgentPayloadError) as exc: - return jsonify({'error': str(exc)}), 400 - - settings = get_settings() - if not settings.get('allow_group_custom_endpoints', False): - _strip_disallowed_local_custom_connection_fields(cleaned_payload) - - for key in ('group_id', 'last_updated', 'is_global', 'is_group'): - cleaned_payload.pop(key, None) - try: - cleaned_payload = apply_assigned_knowledge_to_agent_payload( - cleaned_payload, - user_id=user_id, - agent_scope='group', - group_id=active_group, - is_admin=False, + payload = request.get_json(silent=True) + body = legacy_editor_write_body(payload, None, 'agents') + body['updates'].setdefault('id', str(uuid.uuid4())) + saved = apply_group_agent_write( + user_id, active_group, None, body, _prepare_group_agent_payload, app_settings, ) - except AssignedKnowledgeError as exc: - return jsonify({'error': str(exc)}), 400 - - try: - saved = save_group_agent(active_group, cleaned_payload, user_id=user_id) - except (ValueError, LookupError): - return jsonify({'error': 'Invalid or unavailable Call agent action selection.'}), 400 - except PermissionError as exc: - return jsonify({'error': str(exc)}), 403 except Exception as exc: - debug_print('Failed to save group agent: %s', exc) - return jsonify({'error': 'Unable to save agent'}), 500 + return editor_error_response(exc) log_agent_creation(user_id=user_id, agent_id=saved.get('id', ''), agent_name=saved.get('name', ''), agent_display_name=saved.get('display_name', ''), scope='group', group_id=active_group) - return jsonify(saved), 201 + response = jsonify(project_shared_record(saved, 'agents', can_manage=True, group_scope=True)) + response.headers['Cache-Control'] = 'no-store' + return response, 201 @bpa.route('/api/group/agents/', methods=['PATCH']) @@ -1768,10 +1772,13 @@ def create_group_agent_route(): def update_group_agent_route(agent_id): user_id = get_current_user_id() try: - active_group = require_active_group(user_id) + active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) app_settings = get_settings() allowed_roles = ("Owner",) if app_settings.get('require_owner_for_group_agent_management') else ("Owner", "Admin") - assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + role = assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + group = find_group_by_id(active_group) + if not group or 'edit' not in group_agent_management_operations(user_id, group, role, app_settings): + raise PermissionError('Group agent management is unavailable.') except ValueError as exc: return jsonify({'error': str(exc)}), 400 except LookupError as exc: @@ -1779,60 +1786,20 @@ def update_group_agent_route(agent_id): except PermissionError as exc: return jsonify({'error': str(exc)}), 403 - existing = get_group_agent(active_group, agent_id) - if not existing: - return jsonify({'error': 'Agent not found'}), 404 - - updates = request.get_json(silent=True) or {} - for key in ('id', 'group_id', 'last_updated', 'is_global', 'is_group'): - updates.pop(key, None) - - try: - validate_group_agent_payload(updates, partial=True) - except ValueError as exc: - return jsonify({'error': str(exc)}), 400 - - merged = dict(existing) - merged.update(updates) - merged['id'] = agent_id - - try: - validate_group_agent_payload(merged, partial=False) - except ValueError as exc: - return jsonify({'error': str(exc)}), 400 - - try: - cleaned_payload = sanitize_agent_payload(merged) - except AgentPayloadError as exc: - return jsonify({'error': str(exc)}), 400 - - settings = get_settings() - if not settings.get('allow_group_custom_endpoints', False): - _strip_disallowed_local_custom_connection_fields(cleaned_payload) - try: - cleaned_payload = apply_assigned_knowledge_to_agent_payload( - cleaned_payload, - user_id=user_id, - agent_scope='group', - group_id=active_group, - is_admin=False, + existing = read_group_editor_record('agents', user_id, active_group, agent_id, app_settings) + saved = apply_group_agent_write( + user_id, active_group, existing, + legacy_editor_write_body(request.get_json(silent=True), existing, 'agents'), + _prepare_group_agent_payload, app_settings, ) - except AssignedKnowledgeError as exc: - return jsonify({'error': str(exc)}), 400 - - try: - saved = save_group_agent(active_group, cleaned_payload, user_id=user_id) - except (ValueError, LookupError): - return jsonify({'error': 'Invalid or unavailable Call agent action selection.'}), 400 - except PermissionError as exc: - return jsonify({'error': str(exc)}), 403 except Exception as exc: - debug_print('Failed to update group agent %s: %s', agent_id, exc) - return jsonify({'error': 'Unable to update agent'}), 500 + return editor_error_response(exc) log_agent_update(user_id=user_id, agent_id=agent_id, agent_name=saved.get('name', ''), agent_display_name=saved.get('display_name', ''), scope='group', group_id=active_group) - return jsonify(saved), 200 + response = jsonify(project_shared_record(saved, 'agents', can_manage=True, group_scope=True)) + response.headers['Cache-Control'] = 'no-store' + return response, 200 @bpa.route('/api/group/agents/', methods=['DELETE']) @@ -2692,6 +2659,16 @@ def build_combined_model_endpoints(settings, user_id=None, group_id=None): def get_global_agent_settings(include_admin_extras=False, user_id=None, group_id=None): settings = get_settings() agents = get_global_agents() + selected_agent = settings.get("global_selected_agent", {}) + if not include_admin_extras: + agents = [ + project_shared_record(agent, 'agents', global_scope=True) + for agent in agents if _is_agent_allowed_for_user_selection(user_id, agent) + ] + selected_agent = ( + project_shared_record(selected_agent, 'agents', global_scope=True) + if isinstance(selected_agent, dict) and selected_agent else {} + ) combined_endpoints = [] base_endpoints = settings.get("model_endpoints", []) or [] multi_flag = settings.get("enable_multi_model_endpoints", False) @@ -2704,7 +2681,7 @@ def get_global_agent_settings(include_admin_extras=False, user_id=None, group_id effective_multi_flag = bool(multi_flag or any(endpoint.get("models") for endpoint in combined_endpoints)) # Return selected_agent and any other relevant settings for admin UI - return jsonify({ + response = jsonify({ "semantic_kernel_agents": agents, "orchestration_type": settings.get("orchestration_type", "default_agent"), "enable_multi_agent_orchestration": settings.get("enable_multi_agent_orchestration", False), @@ -2717,7 +2694,7 @@ def get_global_agent_settings(include_admin_extras=False, user_id=None, group_id "enable_http_plugin": settings.get("enable_http_plugin", False), "enable_wait_plugin": settings.get("enable_wait_plugin", False), "enable_default_embedding_model_plugin": settings.get("enable_default_embedding_model_plugin", False), - "global_selected_agent": settings.get("global_selected_agent", {}), + "global_selected_agent": selected_agent, "merge_global_semantic_kernel_with_workspace": settings.get("merge_global_semantic_kernel_with_workspace", False), "enable_gpt_apim": settings.get("enable_gpt_apim", False), "azure_apim_gpt_deployment": settings.get("azure_apim_gpt_deployment", ""), @@ -2737,3 +2714,5 @@ def get_global_agent_settings(include_admin_extras=False, user_id=None, group_id "enable_multi_model_endpoints": effective_multi_flag, "model_endpoints": combined_endpoints, }) + response.headers['Cache-Control'] = 'no-store' + return response diff --git a/application/single_app/route_backend_plugins.py b/application/single_app/route_backend_plugins.py index df7277202..0309e6346 100644 --- a/application/single_app/route_backend_plugins.py +++ b/application/single_app/route_backend_plugins.py @@ -220,11 +220,15 @@ ) from functions_workspace_authoring import ( WorkspaceAuthoringValidation, + apply_group_action_write, build_action_editor_types, clear_editor_test_secrets, editor_error_response, ensure_editor_access, + legacy_editor_write_body, personal_editor_response, + project_shared_record, + read_group_editor_record, validate_editor_action_manifest, ) @@ -1408,8 +1412,10 @@ def get_user_plugins(): filtered_global_plugins = filter_governed_global_actions_for_user(user_id, get_global_actions()) # Mark global plugins - for plugin in filtered_global_plugins: - plugin['is_global'] = True + filtered_global_plugins = [ + project_shared_record(plugin, 'actions', global_scope=True) + for plugin in filtered_global_plugins + ] # Merge plugins using ID as key to avoid name conflicts # This allows both personal and global plugins with same name to coexist @@ -2052,7 +2058,7 @@ def get_group_actions_route(): user_id = get_current_user_id() try: active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) - assert_group_role( + role = assert_group_role( user_id, active_group, allowed_roles=("Owner", "Admin", "DocumentManager", "User"), @@ -2079,7 +2085,19 @@ def get_group_actions_route(): merged_actions = [_normalize_group_action(action) for action in actions] merged_actions.sort(key=lambda item: (item.get('displayName') or item.get('display_name') or item.get('name') or '').lower()) - return jsonify({'actions': merged_actions}), 200 + can_manage = bool(group_action_management_operations( + user_id, find_group_by_id(active_group), role, settings, + )) + response = jsonify({'actions': [ + project_shared_record( + action, 'actions', can_manage=can_manage, + global_scope=bool(action.get('is_global')), + group_scope=not bool(action.get('is_global')), + ) + for action in merged_actions + ]}) + response.headers['Cache-Control'] = 'no-store' + return response, 200 @bpap.route('/api/group/plugins/', methods=['GET']) @@ -2091,7 +2109,7 @@ def get_group_action_route(action_id): user_id = get_current_user_id() try: active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) - assert_group_role( + role = assert_group_role( user_id, active_group, allowed_roles=("Owner", "Admin", "DocumentManager", "User"), @@ -2103,15 +2121,26 @@ def get_group_action_route(action_id): except PermissionError: return jsonify({'error': 'You are not authorized to view this group action.'}), 403 - action = get_group_action(active_group, action_id, return_type=SecretReturnType.TRIGGER) + action = get_group_action(active_group, action_id, return_type=SecretReturnType.NAME) if not action: return jsonify({'error': 'Action not found'}), 404 try: if not is_retired_mcp_stdio(action): ensure_action_type_access('governance_group_actions', user_id, resolve_action_type(action), 'group') - except PermissionError as exc: - return jsonify({'error': str(exc)}), 403 - return jsonify(action), 200 + except PermissionError: + return jsonify({'error': 'You are not authorized to view this group action.'}), 403 + settings = get_settings() + can_manage = bool(group_action_management_operations( + user_id, find_group_by_id(active_group), role, settings, + )) + try: + if can_manage: + action = read_group_editor_record('actions', user_id, active_group, action['id'], settings) + response = jsonify(project_shared_record(action, 'actions', can_manage=can_manage, group_scope=True)) + except Exception as exc: + return editor_error_response(exc) + response.headers['Cache-Control'] = 'no-store' + return response, 200 @bpap.route('/api/group/plugins', methods=['POST']) @@ -2125,7 +2154,10 @@ def create_group_action_route(): active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) app_settings = get_settings() allowed_roles = ("Owner",) if app_settings.get('require_owner_for_group_agent_management') else ("Owner", "Admin") - assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + role = assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + group = find_group_by_id(active_group) + if not group or 'create' not in group_action_management_operations(user_id, group, role, app_settings): + raise PermissionError('Group action management is unavailable.') except ValueError: return jsonify({'error': 'Group action scope is invalid.'}), 400 except LookupError: @@ -2133,70 +2165,21 @@ def create_group_action_route(): except PermissionError: return jsonify({'error': 'You are not authorized to create this group action.'}), 403 - payload = request.get_json(silent=True) or {} - _apply_plugin_runtime_defaults(payload) - if is_legacy_msgraph_type(payload.get('type')): - return jsonify({'error': LEGACY_ACTION_CREATION_MESSAGE}), 400 - try: - validate_group_action_payload(payload, partial=False) - except ValueError as exc: - return jsonify({'error': str(exc)}), 400 - - if payload.get('is_global'): - return jsonify({'error': 'Global actions are managed centrally and cannot be created within a group.'}), 400 - - for key in ('group_id', 'last_updated', 'user_id', 'is_global', 'is_group', 'scope'): - payload.pop(key, None) - - # Merge with schema to ensure all required fields are present (same as global actions) - schema_dir = os.path.join(current_app.root_path, 'static', 'json', 'schemas') - merged = get_merged_plugin_settings(payload.get('type'), payload, schema_dir) - payload['metadata'] = merged.get('metadata', payload.get('metadata', {})) - payload['additionalFields'] = merged.get('additionalFields', payload.get('additionalFields', {})) - - try: - _validate_action_identity_for_scope( - payload, - WORKSPACE_IDENTITY_SCOPE_GROUP, - active_group, - ) - except (ValueError, LookupError, PermissionError): - return jsonify({'error': 'Group action identity configuration is invalid.'}), 400 - - is_valid, validation_errors = PluginHealthChecker.validate_plugin_manifest(payload, payload.get('type')) - if not is_valid: - return jsonify({'error': f'Plugin validation failed: {"; ".join(validation_errors)}'}), 400 - try: - _enforce_mcp_destination_policy( - payload, - WORKSPACE_IDENTITY_SCOPE_GROUP, - active_group, - operation='group_action_create', - user_id=user_id, + payload = request.get_json(silent=True) + if isinstance(payload, dict) and payload.get('is_global'): + raise WorkspaceAuthoringValidation('Global actions are managed centrally.') + saved = apply_group_action_write( + user_id, active_group, None, legacy_editor_write_body(payload, None, 'actions'), + _prepare_group_action_payload, app_settings, ) - except McpDestinationPolicyError: - return jsonify({'error': 'MCP destination is not allowed by governance policy.'}), 403 - except ValueError: - return jsonify({'error': 'MCP destination configuration is invalid.'}), 400 - - try: - saved = save_group_action(active_group, payload, user_id=user_id) - except (ValueError, LookupError) as exc: - debug_print('Validation error saving group action: %s', exc) - return jsonify({'error': ACTION_VALIDATION_ERROR_MESSAGE}), 400 - except PermissionError as exc: - debug_print('Permission denied saving group action: %s', exc) - return jsonify({'error': ACTION_PERMISSION_ERROR_MESSAGE}), 403 - except RuntimeError as exc: - debug_print('Key Vault error saving group action: %s', exc) - return jsonify({'error': ACTION_KEY_VAULT_ERROR_MESSAGE}), 500 except Exception as exc: - debug_print('Failed to save group action: %s', exc) - return jsonify({'error': 'Unable to save action'}), 500 + return editor_error_response(exc) log_action_creation(user_id=user_id, action_id=saved.get('id', ''), action_name=saved.get('name', ''), action_type=saved.get('type', ''), scope='group', group_id=active_group) - return jsonify(saved), 201 + response = jsonify(project_shared_record(saved, 'actions', can_manage=True, group_scope=True)) + response.headers['Cache-Control'] = 'no-store' + return response, 201 @bpap.route('/api/group/plugins/', methods=['PATCH']) @@ -2210,89 +2193,37 @@ def update_group_action_route(action_id): active_group = resolve_delegation_group_scope(user_id, request.args.get('group_id')) app_settings = get_settings() allowed_roles = ("Owner",) if app_settings.get('require_owner_for_group_agent_management') else ("Owner", "Admin") - assert_group_role(user_id, active_group, allowed_roles=allowed_roles) - except ValueError as exc: - return jsonify({'error': str(exc)}), 400 - except LookupError as exc: - return jsonify({'error': str(exc)}), 404 - except PermissionError as exc: - return jsonify({'error': str(exc)}), 403 + role = assert_group_role(user_id, active_group, allowed_roles=allowed_roles) + group = find_group_by_id(active_group) + if not group or 'edit' not in group_action_management_operations(user_id, group, role, app_settings): + raise PermissionError('Group action management is unavailable.') + except ValueError: + return jsonify({'error': 'Group action scope is invalid.'}), 400 + except LookupError: + return jsonify({'error': 'The selected group context was not found.'}), 404 + except PermissionError: + return jsonify({'error': 'You are not authorized to update this group action.'}), 403 existing = get_group_action(active_group, action_id, return_type=SecretReturnType.NAME) if not existing: return jsonify({'error': 'Action not found'}), 404 - updates = request.get_json(silent=True) or {} - if not isinstance(updates, dict): - raise McpConfigurationError("Action configuration must be an object.") - if updates.get('is_global'): - return jsonify({'error': 'Global actions cannot be modified within a group.'}), 400 - - for key in ('id', 'group_id', 'last_updated', 'user_id', 'is_global', 'is_group', 'scope'): - updates.pop(key, None) - - merged = dict(existing) - merged.update(updates) - merged['is_global'] = False - merged['is_group'] = True - merged['id'] = existing.get('id', action_id) - - _apply_plugin_runtime_defaults(merged) - try: - validate_group_action_payload(merged, partial=False) - except ValueError as exc: - return jsonify({'error': str(exc)}), 400 - - # Merge with schema to ensure all required fields are present (same as global actions) - schema_dir = os.path.join(current_app.root_path, 'static', 'json', 'schemas') - schema_merged = get_merged_plugin_settings(merged.get('type'), merged, schema_dir) - merged['metadata'] = schema_merged.get('metadata', merged.get('metadata', {})) - merged['additionalFields'] = schema_merged.get('additionalFields', merged.get('additionalFields', {})) - - try: - _validate_action_identity_for_scope( - merged, - WORKSPACE_IDENTITY_SCOPE_GROUP, - active_group, - ) - except (ValueError, LookupError, PermissionError): - return jsonify({'error': 'Group action identity configuration is invalid.'}), 400 - - is_valid, validation_errors = PluginHealthChecker.validate_plugin_manifest(merged, merged.get('type')) - if not is_valid: - return jsonify({'error': f'Plugin validation failed: {"; ".join(validation_errors)}'}), 400 - - try: - _enforce_mcp_destination_policy( - merged, - WORKSPACE_IDENTITY_SCOPE_GROUP, - active_group, - operation='group_action_update', - user_id=user_id, + existing = read_group_editor_record('actions', user_id, active_group, existing['id'], app_settings) + updates = request.get_json(silent=True) + if isinstance(updates, dict) and updates.get('is_global'): + raise WorkspaceAuthoringValidation('Global actions are managed centrally.') + saved = apply_group_action_write( + user_id, active_group, existing, legacy_editor_write_body(updates, existing, 'actions'), + _prepare_group_action_payload, app_settings, ) - except McpDestinationPolicyError: - return jsonify({'error': 'MCP destination is not allowed by governance policy.'}), 403 - except ValueError: - return jsonify({'error': 'MCP destination configuration is invalid.'}), 400 - - try: - saved = save_group_action(active_group, merged, user_id=user_id) - except (ValueError, LookupError) as exc: - debug_print('Validation error updating group action %s: %s', action_id, exc) - return jsonify({'error': ACTION_VALIDATION_ERROR_MESSAGE}), 400 - except PermissionError as exc: - debug_print('Permission denied updating group action %s: %s', action_id, exc) - return jsonify({'error': ACTION_PERMISSION_ERROR_MESSAGE}), 403 - except RuntimeError as exc: - debug_print('Key Vault error updating group action %s: %s', action_id, exc) - return jsonify({'error': ACTION_KEY_VAULT_ERROR_MESSAGE}), 500 except Exception as exc: - debug_print('Failed to update group action %s: %s', action_id, exc) - return jsonify({'error': 'Unable to update action'}), 500 + return editor_error_response(exc) log_action_update(user_id=user_id, action_id=action_id, action_name=saved.get('name', ''), action_type=saved.get('type', ''), scope='group', group_id=active_group) - return jsonify(saved), 200 + response = jsonify(project_shared_record(saved, 'actions', can_manage=True, group_scope=True)) + response.headers['Cache-Control'] = 'no-store' + return response, 200 @bpap.route('/api/group/plugins/', methods=['DELETE']) diff --git a/application/single_app/static/js/agent_modal_stepper.js b/application/single_app/static/js/agent_modal_stepper.js index 48be6a782..1c4ba95c6 100644 --- a/application/single_app/static/js/agent_modal_stepper.js +++ b/application/single_app/static/js/agent_modal_stepper.js @@ -4,6 +4,7 @@ import { showToast } from "./chat/chat-toast.js"; import * as agentsCommon from "./agents_common.js"; import { getModelSupportedLevels } from "./chat/chat-reasoning.js"; import { AgentInstructionMentions, buildActionToken, buildKnowledgeToken } from "./agent_instruction_mentions.js"; +import { SharedCredentialEditor } from "./shared-credential-editor.js"; const ACTION_CAPABILITIES_KEY = 'action_capabilities'; const M365_ACTION_TYPES = ['m365_calendar', 'm365_email', 'm365_onedrive', 'm365_sharepoint']; @@ -2130,6 +2131,7 @@ export class AgentModalStepper { // Store original state for change detection this.originalAgent = agent ? JSON.parse(JSON.stringify(agent)) : null; + this.sharedCredentials = new SharedCredentialEditor(agent); // Reset modal state this.currentStep = 1; @@ -2170,6 +2172,7 @@ export class AgentModalStepper { this.syncAgentTypeSelector(); this.applyAgentTypeVisibility(); this.updateAgentTypeLock(); + this.sharedCredentials.render(document.querySelector('#agentModal .modal-body')); // Load models for the modal this.loadModelsForModal(); @@ -4708,6 +4711,7 @@ export class AgentModalStepper { delete agentData[field]; } }); + this.sharedCredentials?.apply(agentData); // Use appropriate endpoint and save method based on context let saveBtn = document.getElementById('agent-modal-save-btn'); diff --git a/application/single_app/static/js/plugin_modal_stepper.js b/application/single_app/static/js/plugin_modal_stepper.js index f2744567a..fe2f3d2f9 100644 --- a/application/single_app/static/js/plugin_modal_stepper.js +++ b/application/single_app/static/js/plugin_modal_stepper.js @@ -2,6 +2,7 @@ // Multi-step modal functionality for action/plugin creation import { showToast } from "./chat/chat-toast.js"; import { getTypeIcon, getMcpRetirementStatus, isMcpActionType } from "./workspace/view-utils.js"; +import { SharedCredentialEditor, isStoredCredentialValue } from "./shared-credential-editor.js"; // Action types hidden from the creation UI (backend plugins remain intact). // control_center is created in the V2 admin action editor, the one surface that configures it. @@ -1020,6 +1021,7 @@ export class PluginModalStepper { // Store original plugin state for change tracking this.originalPlugin = plugin ? JSON.parse(JSON.stringify(plugin)) : null; + this.sharedCredentials = new SharedCredentialEditor(plugin); const retirementStatus = getMcpRetirementStatus(plugin); const retirementNotice = document.getElementById('mcp-retirement-notice'); if (retirementNotice) { @@ -1064,6 +1066,8 @@ export class PluginModalStepper { this.populateActionTypeCards(); } + this.sharedCredentials.render(document.querySelector('#plugin-modal .modal-body')); + // Show the modal const modal = new bootstrap.Modal(document.getElementById('plugin-modal')); modal.show(); @@ -5871,7 +5875,7 @@ export class PluginModalStepper { auth.type = 'key'; const username = document.getElementById('plugin-auth-basic-username')?.value.trim() || ''; const password = document.getElementById('plugin-auth-basic-password')?.value.trim() || ''; - auth.key = `${username}:${password}`; + auth.key = this.sharedCredentials?.basicAuth(username, password) ?? `${username}:${password}`; additionalFields.auth_method = 'basic'; } else if (authType === 'oauth2') { auth.type = 'key'; @@ -6834,7 +6838,8 @@ export class PluginModalStepper { } else if (authMethod === 'basic') { authType = 'basic'; // Split the combined username:password format - const credentials = auth.key ? auth.key.split(':') : ['', '']; + const credentials = this.sharedCredentials?.has('/auth/key') && isStoredCredentialValue(auth.key) + ? [auth.key, auth.key] : (auth.key ? auth.key.split(':') : ['', '']); document.getElementById('plugin-auth-basic-username').value = credentials[0] || ''; document.getElementById('plugin-auth-basic-password').value = credentials[1] || ''; } else if (authMethod === 'oauth2') { @@ -7410,7 +7415,7 @@ export class PluginModalStepper { formData.id = this.originalPlugin.id; } - return formData; + return this.sharedCredentials?.apply(formData) || formData; } generateActionName(displayName) { diff --git a/application/single_app/static/js/shared-credential-editor.js b/application/single_app/static/js/shared-credential-editor.js new file mode 100644 index 000000000..11f5f6f84 --- /dev/null +++ b/application/single_app/static/js/shared-credential-editor.js @@ -0,0 +1,127 @@ +// shared-credential-editor.js + +const storedSecretMarkers = new Set([ + '***REDACTED***', 'Stored_In_KeyVault', '[REDACTED]', '********', '**********' +]); + +export function isStoredCredentialValue(value) { + return typeof value === 'string' && storedSecretMarkers.has(value); +} + +function pointerParts(pointer) { + if (typeof pointer !== 'string' || !pointer.startsWith('/') || /~(?![01])/.test(pointer)) { + throw new Error('Stored credential fields are invalid. Reload this editor.'); + } + const parts = pointer.slice(1).split('/').map(part => part.replace(/~1/g, '/').replace(/~0/g, '~')); + if (parts.some(part => ['__proto__', 'prototype', 'constructor'].includes(part))) { + throw new Error('Stored credential fields are invalid. Reload this editor.'); + } + return parts; +} + +function readPath(record, parts) { + let value = record; + for (const part of parts) { + if (!value || typeof value !== 'object' || !Object.hasOwn(value, part)) return undefined; + value = value[part]; + } + return value; +} + +function writePath(record, parts, value, remove = false) { + let parent = record; + for (const part of parts.slice(0, -1)) { + if (!parent[part] || typeof parent[part] !== 'object') { + if (remove) return; + parent[part] = {}; + } + parent = parent[part]; + } + const leaf = parts[parts.length - 1]; + if (remove) delete parent[leaf]; + else parent[leaf] = value; +} + +export class SharedCredentialEditor { + constructor(record) { + this.record = record; + this.active = Boolean(record?.is_group && record.read_only === false && Array.isArray(record.secret_paths)); + this.paths = this.active ? record.secret_paths.map(pointer => ({ pointer, parts: pointerParts(pointer) })) : []; + this.clears = new Set(); + const required = Array.isArray(record?.credential_reentry_required) ? record.credential_reentry_required : []; + this.reentry = new Set(required.filter(field => typeof field?.path === 'string').map(field => field.path)); + } + + has(pointer) { + return this.paths.some(field => field.pointer === pointer); + } + + render(container) { + container?.querySelector('[data-shared-credentials]')?.remove(); + if (!container || !this.active || !this.paths.length) return; + const notice = document.createElement('div'); + notice.dataset.sharedCredentials = 'true'; + notice.className = 'alert alert-secondary mt-3'; + const title = document.createElement('strong'); + title.textContent = 'Stored credentials are write-only'; + const guidance = document.createElement('p'); + guidance.className = 'mb-2'; + guidance.textContent = 'Keep configured values unchanged, or enter replacements. Select a field below to clear it explicitly; change the authentication method if that field is required.'; + notice.append(title, guidance); + for (const [index, field] of this.paths.entries()) { + const row = document.createElement('div'); + row.className = 'form-check text-break'; + const checkbox = document.createElement('input'); + checkbox.type = 'checkbox'; + checkbox.className = 'form-check-input'; + checkbox.id = `${container.closest('.modal')?.id || 'shared'}-clear-credential-${index}`; + checkbox.addEventListener('change', () => { + if (checkbox.checked) this.clears.add(field.pointer); + else this.clears.delete(field.pointer); + }); + const label = document.createElement('label'); + label.className = 'form-check-label'; + label.htmlFor = checkbox.id; + label.textContent = `Clear ${field.parts.join(' / ')}`; + row.append(checkbox, label); + notice.appendChild(row); + } + container.prepend(notice); + } + + basicAuth(username, password) { + if (this.has('/auth/key')) { + if (isStoredCredentialValue(username) && isStoredCredentialValue(password)) { + return this.record.auth.key; + } + if (isStoredCredentialValue(username) || isStoredCredentialValue(password)) { + throw new Error('Re-enter both the username and password to replace stored Basic authentication.'); + } + } + return `${username}:${password}`; + } + + apply(payload) { + if (!this.active) return payload; + if (typeof this.record.revision !== 'string' || !this.record.revision) { + throw new Error('Reload this resource before saving; its revision is unavailable.'); + } + for (const field of this.paths) { + const value = readPath(payload, field.parts); + if (this.clears.has(field.pointer)) { + const requiredKey = field.pointer === '/auth/key' && payload.auth?.type === 'key'; + writePath(payload, field.parts, requiredKey ? '' : undefined, !requiredKey); + } else if (this.reentry.has(field.pointer)) { + if (isStoredCredentialValue(value)) writePath(payload, field.parts, undefined, true); + } else if (value === '' || value === null) { + writePath(payload, field.parts, readPath(this.record, field.parts)); + } else if (typeof value === 'string' && !isStoredCredentialValue(value) + && [...storedSecretMarkers].some(marker => value.includes(marker))) { + throw new Error('Re-enter the complete credential rather than combining it with a stored-value marker.'); + } + } + payload.expected_revision = this.record.revision; + payload.clear_secret_paths = [...this.clears]; + return payload; + } +} diff --git a/application/single_app/static/js/workspace/group_agents.js b/application/single_app/static/js/workspace/group_agents.js index b7e1c884d..6e6393fb5 100644 --- a/application/single_app/static/js/workspace/group_agents.js +++ b/application/single_app/static/js/workspace/group_agents.js @@ -127,7 +127,7 @@ function renderAgentsTable(list) { `; - if (canManage) { + if (canManage && agent.read_only !== true) { actionsHtml += ` `; - if (canManage && !isGlobal) { + if (canManage && !isGlobal && plugin.read_only !== true) { actionsHtml += `