mirror of
https://github.com/microsoft/agent-framework.git
synced 2026-06-16 21:04:09 +08:00
Python: follow up FIDES security flow (#5330)
* Python: follow up FIDES security flow Refine the secure approval path, mark the security classes with the FIDES experimental feature label, and clean up the related docs/tests. Also fix workspace-level validation regressions uncovered while running the full Python check suite. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Python: remove FIDES GitHub MCP sample Drop the GitHub MCP security sample from the FIDES follow-up branch while keeping the remaining security docs and samples intact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
committed by
eavanvalkenburg
Unverified
parent
8a08776a32
commit
912961b10c
@@ -100,24 +100,15 @@ from ._middleware import (
|
||||
chat_middleware,
|
||||
function_middleware,
|
||||
)
|
||||
from ._sessions import (
|
||||
AgentSession,
|
||||
ContextProvider,
|
||||
FileHistoryProvider,
|
||||
HistoryProvider,
|
||||
InMemoryHistoryProvider,
|
||||
SessionContext,
|
||||
register_state_type,
|
||||
)
|
||||
from ._security import (
|
||||
ContentLabel,
|
||||
IntegrityLabel,
|
||||
SECURITY_TOOL_INSTRUCTIONS,
|
||||
ConfidentialityLabel,
|
||||
ContentLabel,
|
||||
ContentVariableStore,
|
||||
IntegrityLabel,
|
||||
LabeledMessage,
|
||||
LabelTrackingFunctionMiddleware,
|
||||
PolicyEnforcementFunctionMiddleware,
|
||||
SECURITY_TOOL_INSTRUCTIONS,
|
||||
SecureAgentConfig,
|
||||
VariableReferenceContent,
|
||||
check_confidentiality_allowed,
|
||||
@@ -128,6 +119,15 @@ from ._security import (
|
||||
set_quarantine_client,
|
||||
store_untrusted_content,
|
||||
)
|
||||
from ._sessions import (
|
||||
AgentSession,
|
||||
ContextProvider,
|
||||
FileHistoryProvider,
|
||||
HistoryProvider,
|
||||
InMemoryHistoryProvider,
|
||||
SessionContext,
|
||||
register_state_type,
|
||||
)
|
||||
from ._settings import SecretString, load_settings
|
||||
from ._skills import (
|
||||
Skill,
|
||||
@@ -289,6 +289,7 @@ __all__ = [
|
||||
"GROUP_INDEX_KEY",
|
||||
"GROUP_KIND_KEY",
|
||||
"GROUP_TOKEN_COUNT_KEY",
|
||||
"SECURITY_TOOL_INSTRUCTIONS",
|
||||
"SKIP_PARSING",
|
||||
"SUMMARIZED_BY_SUMMARY_ID_KEY",
|
||||
"SUMMARY_OF_GROUP_IDS_KEY",
|
||||
@@ -384,11 +385,11 @@ __all__ = [
|
||||
"Message",
|
||||
"MiddlewareException",
|
||||
"MiddlewareTermination",
|
||||
"PolicyEnforcementFunctionMiddleware",
|
||||
"MiddlewareType",
|
||||
"MiddlewareTypes",
|
||||
"OuterFinalT",
|
||||
"OuterUpdateT",
|
||||
"PolicyEnforcementFunctionMiddleware",
|
||||
"RawAgent",
|
||||
"ReleaseCandidateFeature",
|
||||
"ResponseStream",
|
||||
@@ -397,9 +398,8 @@ __all__ = [
|
||||
"RunContext",
|
||||
"Runner",
|
||||
"RunnerContext",
|
||||
"SECURITY_TOOL_INSTRUCTIONS",
|
||||
"SecureAgentConfig",
|
||||
"SecretString",
|
||||
"SecureAgentConfig",
|
||||
"SelectiveToolCallCompactionStrategy",
|
||||
"SessionContext",
|
||||
"SingleEdgeGroup",
|
||||
@@ -458,8 +458,8 @@ __all__ = [
|
||||
"WorkflowViz",
|
||||
"__version__",
|
||||
"add_usage_details",
|
||||
"ai_function",
|
||||
"agent_middleware",
|
||||
"ai_function",
|
||||
"annotate_message_groups",
|
||||
"apply_compaction",
|
||||
"chat_middleware",
|
||||
|
||||
@@ -48,6 +48,7 @@ class ExperimentalFeature(str, Enum):
|
||||
|
||||
EVALS = "EVALS"
|
||||
FILE_HISTORY = "FILE_HISTORY"
|
||||
FIDES = "FIDES"
|
||||
FUNCTIONAL_WORKFLOWS = "FUNCTIONAL_WORKFLOWS"
|
||||
SKILLS = "SKILLS"
|
||||
TOOLBOXES = "TOOLBOXES"
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1448,9 +1448,8 @@ async def _auto_invoke_function(
|
||||
# non-declaration-only functions.
|
||||
|
||||
tool: FunctionTool | None = None
|
||||
# Track if this is a re-invocation after policy violation approval
|
||||
policy_approval_granted = False
|
||||
|
||||
approval_response: Content | None = None
|
||||
|
||||
if function_call_content.type == "function_call":
|
||||
tool = tool_map.get(function_call_content.name) # type: ignore[arg-type]
|
||||
# Tool should exist because _try_execute_function_calls validates this
|
||||
@@ -1465,21 +1464,20 @@ async def _auto_invoke_function(
|
||||
else:
|
||||
# Note: Unapproved tools (approved=False) are handled in _replace_approval_contents_with_results
|
||||
# and never reach this function, so we only handle approved=True cases here.
|
||||
inner_call = function_call_content.function_call # type: ignore[attr-defined]
|
||||
if inner_call.type != "function_call": # type: ignore[union-attr]
|
||||
approved_function_call = function_call_content.function_call # type: ignore[attr-defined]
|
||||
if (
|
||||
approved_function_call is None
|
||||
or approved_function_call.type != "function_call"
|
||||
or approved_function_call.name is None
|
||||
):
|
||||
return function_call_content
|
||||
tool = tool_map.get(inner_call.name) # type: ignore[attr-defined, union-attr, arg-type]
|
||||
tool = tool_map.get(approved_function_call.name)
|
||||
if tool is None:
|
||||
# we assume it is a hosted tool
|
||||
return function_call_content
|
||||
|
||||
# Check if this is an approval for a policy violation
|
||||
# The additional_properties may contain {"policy_violation": True, ...} or just truthy value
|
||||
approval_props = getattr(function_call_content, "additional_properties", None) or {}
|
||||
if approval_props.get("policy_violation"):
|
||||
policy_approval_granted = True
|
||||
|
||||
function_call_content = function_call_content.function_call
|
||||
|
||||
approval_response = function_call_content
|
||||
function_call_content = approved_function_call
|
||||
|
||||
parsed_args: dict[str, Any] = dict(function_call_content.parse_arguments() or {})
|
||||
|
||||
@@ -1555,19 +1553,24 @@ async def _auto_invoke_function(
|
||||
session=invocation_session,
|
||||
kwargs=runtime_kwargs.copy(),
|
||||
)
|
||||
|
||||
|
||||
call_id = function_call_content.call_id
|
||||
if call_id is None:
|
||||
raise KeyError(f'Function "{function_call_content.name}" is missing call_id.')
|
||||
|
||||
# Always pass call_id to middleware for policy violation approval flow
|
||||
middleware_context.metadata["call_id"] = function_call_content.call_id
|
||||
|
||||
# Pass policy approval flag to middleware via metadata (for re-invocation after approval)
|
||||
if policy_approval_granted:
|
||||
middleware_context.metadata["policy_approval_granted"] = True
|
||||
middleware_context.metadata["call_id"] = call_id
|
||||
|
||||
# Pass through the original approval response so middleware can decide whether
|
||||
# this replay corresponds to a middleware-specific approval flow.
|
||||
if approval_response is not None:
|
||||
middleware_context.metadata["approval_response"] = approval_response
|
||||
|
||||
async def final_function_handler(context_obj: Any) -> Any:
|
||||
return await tool.invoke(
|
||||
arguments=context_obj.arguments,
|
||||
context=context_obj,
|
||||
tool_call_id=function_call_content.call_id,
|
||||
tool_call_id=call_id,
|
||||
)
|
||||
|
||||
from ._middleware import MiddlewareTermination
|
||||
@@ -1578,23 +1581,18 @@ async def _auto_invoke_function(
|
||||
context=middleware_context,
|
||||
final_handler=final_function_handler,
|
||||
)
|
||||
|
||||
|
||||
# Pass through function_approval_request directly (e.g., from security middleware)
|
||||
if isinstance(function_result, Content) and function_result.type == "function_approval_request":
|
||||
return function_result
|
||||
|
||||
result_content = Content.from_function_result(
|
||||
call_id=function_call_content.call_id,
|
||||
result=function_result,
|
||||
)
|
||||
|
||||
return result_content
|
||||
|
||||
return Content.from_function_result(call_id=call_id, result=function_result)
|
||||
except MiddlewareTermination as term_exc:
|
||||
# Re-raise to signal loop termination, but first capture any result set by middleware
|
||||
if middleware_context.result is not None:
|
||||
# Store result in exception for caller to extract
|
||||
term_exc.result = Content.from_function_result(
|
||||
call_id=function_call_content.call_id, # type: ignore[arg-type]
|
||||
call_id=call_id,
|
||||
result=middleware_context.result,
|
||||
additional_properties=function_call_content.additional_properties,
|
||||
)
|
||||
@@ -1904,7 +1902,7 @@ def _replace_approval_contents_with_results(
|
||||
approved_function_results: list[Content],
|
||||
) -> None:
|
||||
"""Replace approval request/response contents with function call/result contents in-place.
|
||||
|
||||
|
||||
Also replaces placeholder tool results (marked with [APPROVAL_PENDING]) with actual results.
|
||||
"""
|
||||
from ._types import (
|
||||
@@ -1912,16 +1910,16 @@ def _replace_approval_contents_with_results(
|
||||
)
|
||||
|
||||
# Build a map of call_id -> actual result for replacing placeholders
|
||||
result_by_call_id: dict[str, Contents] = {}
|
||||
result_by_call_id: dict[str, Content] = {}
|
||||
for resp in fcc_todo.values():
|
||||
if resp.approved:
|
||||
if resp.approved and resp.function_call is not None and resp.function_call.call_id is not None:
|
||||
# Map the call_id from the function_call to be replaced
|
||||
call_id = resp.function_call.call_id
|
||||
if call_id not in result_by_call_id and approved_function_results:
|
||||
idx = len(result_by_call_id)
|
||||
if idx < len(approved_function_results):
|
||||
result_by_call_id[call_id] = approved_function_results[idx]
|
||||
|
||||
|
||||
# Track which call_ids had their placeholders replaced
|
||||
placeholders_replaced: set[str] = set()
|
||||
|
||||
@@ -1943,16 +1941,18 @@ def _replace_approval_contents_with_results(
|
||||
if _is_hosted_tool_approval(content):
|
||||
continue
|
||||
# Don't add the function call if it already exists (would create duplicate)
|
||||
if content.function_call.call_id in existing_call_ids: # type: ignore[attr-defined, union-attr, operator]
|
||||
if content.function_call is not None and content.function_call.call_id in existing_call_ids:
|
||||
# Just mark for removal - the function call already exists
|
||||
contents_to_remove.append(content_idx)
|
||||
else:
|
||||
elif content.function_call is not None:
|
||||
# Put back the function call content only if it doesn't exist
|
||||
msg.contents[content_idx] = content.function_call
|
||||
elif content.type == "function_approval_response":
|
||||
# Skip hosted tool approvals — they must pass through to the API unchanged
|
||||
if _is_hosted_tool_approval(content):
|
||||
continue
|
||||
if content.function_call is None or content.function_call.call_id is None:
|
||||
continue
|
||||
call_id = content.function_call.call_id
|
||||
if content.approved and content.id in fcc_todo:
|
||||
# Check if we already replaced a placeholder for this call_id
|
||||
@@ -1977,7 +1977,7 @@ def _replace_approval_contents_with_results(
|
||||
elif content.type == "function_result":
|
||||
# Check if this is a placeholder result that should be replaced
|
||||
if (
|
||||
hasattr(content, "result")
|
||||
hasattr(content, "result")
|
||||
and isinstance(content.result, str)
|
||||
and "[APPROVAL_PENDING]" in content.result
|
||||
and content.call_id in result_by_call_id
|
||||
@@ -1989,10 +1989,10 @@ def _replace_approval_contents_with_results(
|
||||
# Remove contents marked for removal (in reverse order to preserve indices)
|
||||
for idx in reversed(contents_to_remove):
|
||||
msg.contents.pop(idx)
|
||||
|
||||
|
||||
# Second pass: Remove messages that are now empty after content removal
|
||||
# We need to iterate in reverse to safely remove by index
|
||||
messages_to_remove = []
|
||||
messages_to_remove: list[int] = []
|
||||
for msg_idx, msg in enumerate(messages):
|
||||
if not msg.contents:
|
||||
messages_to_remove.append(msg_idx)
|
||||
|
||||
@@ -2121,7 +2121,7 @@ def _get_response_attributes(
|
||||
finish_reason = (
|
||||
getattr(response.raw_representation, "finish_reason", None) if response.raw_representation else None
|
||||
)
|
||||
if finish_reason:
|
||||
if isinstance(finish_reason, str) and finish_reason:
|
||||
attributes[OtelAttr.FINISH_REASONS] = json.dumps([finish_reason])
|
||||
if model := getattr(response, "model", None):
|
||||
attributes[OtelAttr.RESPONSE_MODEL] = model
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -746,9 +746,12 @@ class AgentFrameworkExecutor:
|
||||
|
||||
# Extract policy_violation info if present (from security middleware)
|
||||
policy_violation_data = content_dict.get("policy_violation")
|
||||
additional_props: dict[str, Any] | None = None
|
||||
approval_additional_props: dict[str, Any] | None = None
|
||||
if isinstance(policy_violation_data, dict):
|
||||
additional_props = {"policy_violation": True, **policy_violation_data}
|
||||
approval_additional_props = {
|
||||
"policy_violation": True,
|
||||
**policy_violation_data,
|
||||
}
|
||||
|
||||
# Reconstruct function_call from server-stored data
|
||||
function_call = Content.from_function_call(
|
||||
@@ -762,7 +765,7 @@ class AgentFrameworkExecutor:
|
||||
approved,
|
||||
id=request_id,
|
||||
function_call=function_call,
|
||||
additional_properties=additional_props,
|
||||
additional_properties=approval_additional_props,
|
||||
)
|
||||
contents.append(approval_response)
|
||||
logger.info(
|
||||
@@ -771,7 +774,7 @@ class AgentFrameworkExecutor:
|
||||
request_id,
|
||||
approved,
|
||||
stored_fc["name"],
|
||||
additional_props is not None,
|
||||
approval_additional_props is not None,
|
||||
)
|
||||
except ImportError:
|
||||
logger.warning(
|
||||
|
||||
@@ -1756,17 +1756,16 @@ class MessageMapper:
|
||||
"output_index": context["output_index"],
|
||||
"sequence_number": self._next_sequence(context),
|
||||
}
|
||||
|
||||
|
||||
# Include policy violation details if present (from security middleware)
|
||||
additional_props = getattr(content, "additional_properties", None)
|
||||
if additional_props and isinstance(additional_props, dict):
|
||||
if additional_props.get("policy_violation"):
|
||||
result["policy_violation"] = {
|
||||
"reason": additional_props.get("reason", "Policy violation detected"),
|
||||
"violation_type": additional_props.get("violation_type"),
|
||||
"context_label": additional_props.get("context_label"),
|
||||
}
|
||||
|
||||
additional_props = cast(dict[str, Any] | None, getattr(content, "additional_properties", None))
|
||||
if additional_props and isinstance(additional_props, dict) and additional_props.get("policy_violation"):
|
||||
result["policy_violation"] = {
|
||||
"reason": additional_props.get("reason", "Policy violation detected"),
|
||||
"violation_type": additional_props.get("violation_type"),
|
||||
"context_label": additional_props.get("context_label"),
|
||||
}
|
||||
|
||||
return result
|
||||
|
||||
async def _map_approval_response_content(self, content: Any, context: dict[str, Any]) -> dict[str, Any]:
|
||||
|
||||
Reference in New Issue
Block a user