feat: implement pluggable taxonomy and policy security plugin#151
feat: implement pluggable taxonomy and policy security plugin#151ViktorVeselov wants to merge 11 commits into
Conversation
|
@gemini-cli /review |
|
🤖 Hi @DeanChensj, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
There was a problem hiding this comment.
Thanks for the detailed implementation of the Taxonomy & Policy Security Engine! The architecture looks solid and well-integrated into the ADK plugin lifecycle.
I found a few issues that should be addressed:
- Missing Imports:
policy.pyis missingAnyandOptionalimports fromtyping, which will cause issues when the code is executed or type-checked. - Robustness in Skill Reconstruction: In
_filter_list_skills, manually recreatingSkillandFrontmattermight drop fields. Usingmodel_copyis a safer approach. - Variable Interpolation: Consider adding some logging or keeping placeholders when interpolation fails to aid debugging.
- Path Validation: The current path validation is a bit light for a security-focused plugin.
Overall, a great addition to the community plugins!
| skill_name, tool_context.invocation_id | ||
| ) | ||
| if skill and not self.policy.is_skill_allowed( | ||
| skill, tool_context, active_taxonomies |
There was a problem hiding this comment.
The file_path validation here is quite basic. While it blocks some traversal attempts, it might not catch absolute paths on all operating systems (e.g., C:\path on Windows).
Since this plugin aims for resilience, consider using a more robust path validation or pathlib.Path.is_absolute() if possible, although keeping it dependency-light is also understood.
There was a problem hiding this comment.
That's a great catch, men.
| """ | ||
| return skills | ||
|
|
||
|
|
There was a problem hiding this comment.
If a variable is not found in the registry, it is replaced with an empty string. This might make debugging difficult if there's a typo in the template (e.g., {taxonomy:typo}).
Consider logging a warning when a variable is not found, or keeping the placeholder text to make it obvious that interpolation failed.
| from __future__ import annotations | ||
|
|
||
| from abc import ABC | ||
| from abc import abstractmethod |
There was a problem hiding this comment.
Missing imports for Any and Optional. Please add from typing import Any, Optional to ensure the type hints resolve correctly.
| ) | ||
| return {"result": prompt.format_skills_as_xml(shaped_skills)} | ||
|
|
||
|
|
There was a problem hiding this comment.
Manually recreating the Skill and Frontmatter objects here is risky as it might drop other fields defined in the models (e.g., version, author, or other metadata) that aren't captured in model_extra.
Consider using skill.frontmatter.model_copy(update={"description": shaped_desc}) instead to preserve all existing fields.
There was a problem hiding this comment.
Let me think about it because one of the architectural goals is to use it and change prompts dynamically and enable devs. to apply their custom business roles, features, flags, entitlements, etc., without having too much of a headache. I will have to think about it.
Have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
Link to an existing issue / related PR:
adk-python-community: feature: pluggable policy & taxonomy for more accurate and more deterministic skill/tool execution #152adk-python): feat: add pluggable policy & taxonomy adk-python#5898adk-python: feat: add pluggable policy & taxonomy adk-python#5898 (Closed on core to release as a pluggable community package) and marked withcommunity-repotag.adk-python: [RFC] Taxonomy-Driven Skill Routing & Dynamic Context Mutation adk-python#5891adk-pythonfeature: Modular Domain Taxonomy adk-python#5895 that was transferred toadk-python-communityas per the request of Rahityan's suggestion/recommendation: feat: add pluggable policy & taxonomy adk-python#5898 (comment)Problem:
Developers and system administrators need a robust, standardized way to enforce security policies and access controls on skills based on organizational hierarchies and taxonomies (e.g., flat department structures or rich SKOS JSON-LD classifications).
Originally, we designed this to be embedded directly into the core
adk-pythonSDK. However, in line with keeping the core framework minimalist, light, and secure, the core reviewers pointed me here to implement this as an official community extension plugin (referencing the closed PR adk-python/pull/5898).Solution:
Implemented the Taxonomy & Policy Security Engine as a pluggable, opt-in plugin (
TaxonomyPlugin) inadk-python-community. This gives developers enterprise-grade taxonomy controls and skill-access guardrails without cluttering the core SDK. Additionally, it can be used as a tool for usage reinforcers and for dynamic prompting that helps to provide an agent with well-polished context, skills/tools executions tailored toward determined cases.Key design points include:
_get_taxonomy_binds()pattern to readtaxonomy-bindskeys fromFrontmatter.model_extra(natively populated because the core SDK'sFrontmattermodel hasextra="allow"). This ensures perfect runtime compatibility with the unmodified coreadk-pythonlibrary.before_tool_callbackinstead of importing private core helper functions, making the plugin entirely resilient against future internal changes in core SDK versions.TaxonomyRegistryparses both standard Flat Key-Value JSON (id/parentId/name/definition) and rich JSON-LD with SKOS (Concept,prefLabel,altLabel,definition,broader) standards to resolve organizational hierarchies.TaxonomyPipelineandTaxonomyResolverinterfaces to dynamically chain multiple contextual policy resolvers (e.g., checking active user directories, roles, and entitlements) during runtime execution.BasePluginlifecycle callbacks to intercept tool boundaries (list_skills,load_skill, etc.), returning filtered lists in the expected dictionary-wrapped XML response structure.shape_system_instruction(), the plugin dynamically tailors the global agent instructions at runtime based on active user context and classifications (e.g., automatically injecting compliance instructions).shape_description(), policies can actively rewrite tool description properties to guide the LLM's action selection dynamically (e.g., adding warning prefixes).prioritize_skills(), the engine reorders available tools inside the formatted prompt XML, ensuring context-relevant or preferred skills are bubbled to the top of the LLM's consideration set.Testing Plan
1. Unit & Integration Tests (Command & Logs)
Added a complete integration test suite in
tests/plugins/test_taxonomy_plugin.pyto match the community plugin testing conventions (e.g., alongside the existingtest_agent_governance_plugin.py).To run the tests, set the
PYTHONPATHto include the community source files and your standard core SDK source directory:Verifiable Test Execution Log Output:
Note
Environmental Package Mock Details:
Due to potential environment variations where the
google-genailibrary is either outdated (lacking the expectedAvatarConfigattribute) or completely absent, we pre-emptively mock/patchgoogle.genai.types.AvatarConfigdirectly insidetests/conftest.py. This ensures that the test runner bypasses environment-level import conflicts and allows the unit test suite to compile cleanly in any CI/CD or local environment.2. Manual Verification E2E
To manually verify the taxonomy filter pipeline:
DefaultSkillPolicy:taxonomy-bindsto your skill's frontmatter configuration:list_skillswith a dynamic context (e.g. passing a user identity or role insidetool_context) and observe that skills bound to unauthorized taxonomies are securely filtered out of the response.Checklist
autoformat.sh/ standard formatting).Additional context
dictexpected by__build_response_event.