Repository navigation
Security: resolve_code_reference() allows importing arbitrary Python modules via YAML agent config #5822
Description
Bug Description
The resolve_code_reference(), resolve_fully_qualified_name(), and _resolve_tools() functions in config_agent_utils.py call importlib.import_module() with no restriction on which modules can be imported. While the CVE-2026-4810 fix blocks the args key in YAML configurations, it does not prevent referencing dangerous standard library callables (e.g., os.system, subprocess.call) in callback, tool, schema, or model code-reference fields.
Steps to Reproduce
-
Create a YAML agent config with a blocked module reference:
name: test_agent model: gemini-2.0-flash instruction: harmless before_agent_callbacks: - name: os.system -
Load via from_config() with _ENFORCE_DENYLIST=True - the os module is imported without restriction.
Expected Behavior
YAML agent configs should reject references to dangerous standard library modules when the denylist is enforced.
Actual Behavior
Any Python module can be imported via code reference fields, including os, subprocess, builtins, pickle, etc.
Proposed Fix
Add a _BLOCKED_MODULES set and _validate_module_reference() function to check module references before calling importlib.import_module(). See PR #5821.
Activity
surajksharma07 commentedon May 25, 2026
@Ashutosh0x confirmed the issue on HEAD — resolve_code_reference('os.system') goes right through with no block even when _ENFORCE_DENYLIST=True, so the gap you've identified is real and the reproduction steps check out.
The _BLOCKED_MODULES approach in #5821 looks like a solid fix.
One thing worth raising with reviewers: a strict allowlist (only permitting google.adk.* imports) might be more future-proof than a denylist, since new dangerous stdlib modules could always be added — just worth putting on the table for the design discussion.
Your test coverage looks thorough and the CI results are clean.
Could you do a quick sanity check to make sure the _resolve_tools() path in AgentConfigMapper is also fully gated in your PR?
Once you've confirmed that and done a final end-to-end test pass, we can push this toward a maintainer review.
Thanks for the detailed write-up!
Ashutosh0x commentedon May 25, 2026
Thanks for the thorough review @surajksharma07!
_resolve_tools()path -- confirmed gatedThe
_resolve_tools()method inAgentConfigMapper(line 161) calls_validate_module_reference(tool_config.name)beforeimportlib.import_module()for any user-defined tool (i.e., when'.' in tool_config.name). ADK built-in tools (no dot in name) go through the safegoogle.adk.toolsimport path, so they are fine.Here is the full audit of every
importlib.import_module()call site in the file:_validate_module_reference()?resolve_code_reference()resolve_fully_qualified_name()_resolve_tools()(user-defined)_resolve_tools()(ADK built-in)google.adk.toolsresolve_callbacks()resolve_code_reference()_resolve_agent_code_reference()One gap found:
_resolve_agent_code_reference()Good catch on asking me to audit all paths. I found that
_resolve_agent_code_reference()(line 630) callsimportlib.import_module(module_path)without validation. This is reachable viaresolve_agent_reference()whenref_config.codeis set.The risk is lower here because the function immediately checks
isinstance(obj, BaseAgent)and rejects non-agent objects, but the import itself still executes, which could trigger module-level side effects.I will add
_validate_module_reference(code)at the top of_resolve_agent_code_reference()and push an update with a test for this path.On allowlist vs denylist
Agree that an allowlist (
google.adk.*only) would be more future-proof. The denylist approach was chosen as a less disruptive first step -- it blocks the known-dangerous modules without breaking any existing user-defined tools that import from third-party packages. An allowlist could be a follow-up hardening if the team prefers stricter controls.Happy to adjust the approach based on reviewer preference. Will push the
_resolve_agent_code_reference()fix shortly.arian-gogani commentedon May 26, 2026
arbitrary code import through resolve_code_reference is a supply-chain risk. the complementary control: a tamper-evident receipt for every code reference resolution.
when ADK resolves a code reference at runtime, the resolution decision (which module, which version, which hash) should be captured in a signed receipt. if a malicious module gets imported, the receipt chain shows exactly when it happened and what was resolved.
this pattern applies to any agent framework that dynamically loads tools or plugins. the receipt proves what was loaded, not just what was configured.
we built this receipt layer: Ed25519 signed, JCS canonical (RFC 8785), hash-linked chains. open source.
github.com/arian-gogani/nobulex
surajksharma07 commentedon Jun 4, 2026
@Ashutosh0x Good catch on _resolve_agent_code_reference() — and the right instinct to gate it before the import, not after.
The callable(obj) and isinstance(obj, BaseAgent) checks at lines 167/170 both run post-import, meaning importlib.import_module(module_path) at line 164 has already executed all module-level code by the time those guards fire.
A malicious module with os.system(...) or a reverse-shell setup at global scope would complete that execution before isinstance(obj, BaseAgent) even gets a chance to reject it.
The check prevents the resolved object from being used as an agent, but not the import side effects from running.
Blocking at the module name level, before import_module() is called, is the correct control point.
On the approach shift in the latest rebase: if the PR has moved to an allowlist of permitted module prefixes (rather than the original 28-module denylist), that's the right architectural direction.
A denylist is inherently incomplete — modules like ftplib, smtplib, xmlrpc.client, or asyncore have network capabilities but weren't in the original blocklist.
A "deny-by-default, explicitly-allow" posture (permitting google.adk.* and any namespaces the operator configures) is more future-proof and matches what we'd want for a YAML agent config surface running in the adk web server context.
Worth calling out explicitly in the PR description so reviewers understand the model has changed.
On @arian-gogani's comment: the signed receipt / hash-linked chain approach they describe is a valid post-resolution audit layer — it gives you forensic evidence of what was loaded and when.
This PR is a pre-resolution blocking layer — it prevents dangerous modules from loading at all.
Both controls sit at different points in the chain and are complementary, but the immediate fix for this attack surface is what's in PR #5821.
The PR looks in good shape at this point — all 3 import points gated, CI green, rebased on main, and adapted to the new AgentConfig/BaseAgentConfig architecture.
Good to push for a maintainer review!
security: add module blocklist for YAML agent config code references
Ashutosh0x commentedon Jun 5, 2026
All import sites are now gated (including _resolve_tools() in llm_agent.py), _ENFORCE_DENYLIST defaults to True, and the blocklist has been expanded to 36 modules covering network-capable stdlib ( tplib, smtplib, xmlrpc, etc.). All 52 tests pass — CI is green.
Ready for maintainer merge.
@surajksharma07 @arian-gogani
security: add module blocklist for YAML agent config code references
adk-bot commentedon Jun 5, 2026
🚨 Automated Spam Detection Alert 🚨
@maintainers, a suspected spam comment was detected in this thread.
Reason:
surajksharma07 commentedon Jun 16, 2026
@Ashutosh0x Pulled both branches and diffed against main to check the latest state. #5821 (df75a3b) looks complete — 36-module blocklist, _ENFORCE_DENYLIST=True by default, and llm_agent.py:_resolve_tools() gated at line 1048 before the import, exactly as discussed.
#6117 is a problem though: it only touches config_agent_utils.py (the 3 functions) and never touches llm_agent.py.
I checked the _resolve_tools() user-defined tool branch on the pr-6117 branch directly — importlib.import_module(module_path) at line 1048 still has zero validation in front of it.
That's the exact gap we spent the last two weeks closing on #5821, reintroduced because #6117 was built from an older base instead of rebasing #5821.
Its blocklist is also thinner (25 entries vs. 36) — missing threading, urllib, and the whole network batch (ftplib, smtplib, poplib, imaplib, nntplib, telnetlib, xmlrpc, asyncio) that got added to #5821 specifically because reviewers flagged them.
Merging #6117 as-is would ship a regression relative to #5821, not a fix.
On @arian-gogani's comment — agreed with adk-bot, that's unrelated promotional content for a third-party repo and doesn't need a response here.
fix: add module blocklist for YAML agent config code references
fix: add module blocklist for YAML agent config code references