Repository navigation
Python: FIDES keys five security decisions on a tool-name string instead of the identity marker it already defines #9492
Description
All line references are against 44fe223da7ea6853eefb7053e25c2c213a2e5c15, file
python/packages/core/agent_framework/security.py unless stated otherwise. The clone has no
release tags, so paths are commit-pinned rather than version-pinned. security.py, _tools.py
and _agents.py in the published agent-framework-core 1.22.0 wheel are byte-identical to the
files at that commit (verified with diff), so the observations below apply to the released
package as well.
What the code does
security.py:99 defines a sentinel for exactly this purpose:
_INTERNAL_SECURITY_TOOL = "_agent_framework_internal_security_tool"
_INTERNAL_SECURITY_TOOL_MARKER = object()It is stamped onto the two framework security tools via additional_properties — quarantined_llm
at :3832 and inspect_variable at :4122 — and the principal-propagation branch tests it by
object identity, which is unforgeable from a tool definition:
# :3298
if function_props.get(_INTERNAL_SECURITY_TOOL) is _INTERNAL_SECURITY_TOOL_MARKER:
destination_principals = source_principalsFive sibling decisions about the same two tools are instead keyed on the tool's name:
| Site | Decision | Keyed on |
|---|---|---|
:3105 |
whether a tool may run in an UNTRUSTED context (accepts_untrusted) |
function_name in self.allow_untrusted_tools |
:1878 |
whether an UNTRUSTED result is auto-hidden | function_name != "inspect_variable" |
:105 |
whether var_xxx arguments are expanded before execution |
_VARIABLE_ID_CONSUMERS = frozenset({"inspect_variable", "quarantined_llm"}) |
:1538 |
whether stored content is treated as quarantined-LLM output | metadata.get("function_name") == "quarantined_llm" |
:2297 |
whether an inspect_variable error marker is honoured |
function_name == "inspect_variable" |
The set :3105 consults is populated at :3454, in SecureAgentConfig.__init__:
self._allow_untrusted_tools = {"quarantined_llm", "inspect_variable"}Why it matters
The module's own documented property is that externally supplied metadata must not be able to
relax local policy. :4422:
Server annotations cannot relax local policy. Use
annotation_overridesfor explicit local
per-tool static policy.
A name string is externally supplied in the MCP case — a connected server chooses its own tool
names — whereas the marker at :99 is an in-process object identity that nothing outside the
module can produce. Having one decision use the unforgeable form and five neighbours use the
forgeable one is a consistency gap in a module whose entire job is deciding what untrusted
content may do.
What this is not
This is not exploitable in any documented configuration, and I am not reporting it as a
vulnerability. I tested the obvious attack — a hostile MCP server registering a tool named
inspect_variable — and the product fails closed, because the two halves of that attack are
mutually exclusive:
allow_untrusted_toolsis non-empty only when the enforcer comes fromSecureAgentConfig;
a directly constructedPolicyEnforcementFunctionMiddlewaregetsallow_untrusted_tools or set()
(:2718), i.e. empty.- That same
SecureAgentConfigregisters tools literally namedinspect_variableand
quarantined_llmthroughget_tools()(:3535), so_append_unique_tools
(_agents.py:1655,:1661→_tools.py:1450-1478) rejects a colliding remote tool at agent
tool-list construction.
Run against the published 1.22.0 wheel in a clean venv (Python 3.10.1):
SecureAgentConfig.get_tools() names: ['quarantined_llm', 'inspect_variable']
allow_untrusted_tools on its enforcer: ['inspect_variable', 'quarantined_llm']
allow_untrusted_tools on a BARE enforcer: []
documented order: [hostile] then security tools: REJECTED -> ValueError: Duplicate tool name 'inspect_variable'. Tool names must be unique.
reverse order: security tools then [hostile]: REJECTED -> ValueError: Duplicate tool name 'inspect_variable'. Tool names must be unique.
The only configuration in which the name-keyed exemption is reachable with a colliding tool is
one where an application builds SecureAgentConfig, installs get_middleware(), and discards
get_tools(). No sample or document in the repository shows that wiring, and it is not something
a caller can cause from outside the application.
So: a defense-in-depth and consistency issue, not a security boundary failure. Severity is
whatever you judge a latent-correctness issue in a security module to be worth; I would not
assign it one.
Two things I did not check: whether any other name-keyed decision exists outside security.py,
and whether the duplicate-name rejection holds for tool sources other than MCPTool and plain
function tools.
Suggested fix
Make the five sites agree with :3298 and test the marker by identity, keeping the name check only
as a fallback for callers who pass their own allow_untrusted_tools. For :3105:
function_props = _get_additional_properties(context.function)
+ is_internal_security_tool = (
+ function_props.get(_INTERNAL_SECURITY_TOOL) is _INTERNAL_SECURITY_TOOL_MARKER
+ )
accepts_untrusted = (
- function_name in self.allow_untrusted_tools or function_props.get("accepts_untrusted") is True
+ is_internal_security_tool
+ or function_name in self.allow_untrusted_tools
+ or function_props.get("accepts_untrusted") is True
)and drop "quarantined_llm" / "inspect_variable" from the seed set at :3454, so the names are
no longer load-bearing. :1878, :105, :1538 and :2297 take the same treatment: derive the
predicate from the marker on the function's additional_properties rather than from the string.
Alternatively, if the duplicate-name rejection is considered the intended guarantee, state that in
the docstring at :4422 — right now the module reads as though the name checks are themselves
meant to be trustworthy.
Activity
[-]FIDES keys five security decisions on a tool-name string instead of the identity marker it already defines[/-][+]Python: FIDES keys five security decisions on a tool-name string instead of the identity marker it already defines[/+]SiyuanChenToDo commentedon Oct 19, 2026
I'd like to take this on.
I confirmed the five name-string checks are still present on current
main:_VARIABLE_ID_CONSUMERSatsecurity.py:105, used at:1670function_name == "quarantined_llm"at:1538_should_hide'sfunction_name != "inspect_variable"at:1878inspect_errorat:2297allow_untrusted_toolsat:3105, populated at:3454Proposed approach:
_is_internal_security_tool(function: Any) -> boolthat checks the existing_INTERNAL_SECURITY_TOOL_MARKERthrough_get_additional_properties.context.function(_expand_variable_references_in_contextand the policy-enforcement path), use the helper directly instead of the tool name.function_name(_should_hide,_extract_content_label,_hide_item), compute the boolean once at the invocation boundary and pass it down. Persist it in the hidden-variable metadata so_lookup_variablecan use it; keep a name fallback only for metadata written by older versions.inspect_variableorquarantined_llmdoes not receive internal-tool privileges, while the real tools still bypass variable expansion, auto-hide, and principal checks as today.Planned validation:
python/packages/core/tests/test_security.py, plus targeted tests for a colliding remote tool,_should_hide, variable expansion, and hidden-variable metadata.uv run pytest python/packages/core/tests/test_security.pyruff/pyrightper the repository tooling.AI assistance: Claude Code was used to trace the call sites and draft this proposal; I reviewed the analysis and can implement and defend the change.
Would a maintainer be willing to add
ready-for-implementationor confirm the approach? I'll wait for that before opening a PR.eavanvalkenburg commentedon Oct 26, 2026
Thank you for the detailed analysis and the implementation offer. We will consider this as core FIDES defense-in-depth hardening, rather than a sample or external package. This is not a confirmed vulnerability in the documented configuration, which already rejects duplicate tool names. The policy-path and persisted-metadata implications need design before we agree to an implementation, so we will request planning first. This does not promise implementation or timing; please continue to wait for agreement on the resulting approach.
eavanvalkenburg commentedon Oct 26, 2026
/plan Investigate bounded core FIDES identity-marker hardening while preserving documented policy behavior. Existing supported tool assembly rejects duplicate names; do not frame this as a demonstrated exploit. Trace the name-based special cases and the existing local identity marker across variable expansion, untrusted execution, hiding/quarantined output, inspector error handling and principal propagation. Distinguish inspect_variable from quarantined_llm where their roles differ; a shared internal-tool boolean must not grant both tools the same special cases. Preserve explicit caller allow_untrusted_tools and local annotation policies. Establish trusted provenance and compatibility for persisted hidden-variable metadata: do not trust caller/server-provided booleans or blindly use legacy name fallback to grant internal privileges. Compare a minimal identity-based change with documenting the existing invariant, and recommend the smallest maintainable design. Cover real internal tools, unmarked same-name functions/MCP tools, duplicate-name rejection, aliases/wrappers, direct middleware construction, genuine caller overrides and old metadata. No implementation or assignment until maintainers agree on the plan.