Skip to content

Commit 7346200

Browse files
saneGuyclaude
andcommitted
fix: address review — run_in_executor, bounded audit, consistent style
- Wrap verify() in loop.run_in_executor() to avoid blocking (matches AgentGovernancePlugin pattern) - Remove unused tool_args from _log_denial/_log_allow - Use collections.deque(maxlen=) for bounded audit log (default 1000) - Move import json to top of file - Use Optional[] consistently instead of mixing | None - Fix docstring: plugin enforces permission checks at execution time, delegation narrowing is enforced at credential issuance Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 0629527 commit 7346200

1 file changed

Lines changed: 46 additions & 41 deletions

File tree

src/google/adk_community/plugins/delegation_auth_plugin.py

Lines changed: 46 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -12,14 +12,13 @@
1212
# See the License for the specific language governing permissions and
1313
# limitations under the License.
1414

15-
"""ADK plugin for delegation-scoped agent authorization.
15+
"""ADK plugin for permission-scoped agent authorization.
1616
1717
Verifies that an agent holds a valid credential with sufficient permissions
18-
before executing a tool. Credentials are scoped: a delegated agent can only
19-
use a subset of its parent's permissions, never more.
18+
before executing a tool. Uses a pluggable verifier interface so any identity
19+
system (DID, JWT, ZKP, API keys, etc.) can be wired in.
2020
21-
The plugin uses a pluggable verifier interface. A structural verifier is
22-
included for development; plug in a real verifier (DID, JWT, ZKP, etc.)
21+
A structural verifier is included for development; plug in a real verifier
2322
for production.
2423
2524
Example::
@@ -34,6 +33,10 @@
3433

3534
from __future__ import annotations
3635

36+
import asyncio
37+
import collections
38+
import functools
39+
import json
3740
import logging
3841
import time
3942
from abc import ABC, abstractmethod
@@ -46,6 +49,8 @@
4649

4750
logger = logging.getLogger(__name__)
4851

52+
_MAX_AUDIT_ENTRIES_DEFAULT = 1000
53+
4954

5055
# ---------------------------------------------------------------------------
5156
# Pluggable verifier interface
@@ -68,6 +73,10 @@ class CredentialVerifier(ABC):
6873
6974
Implement this to plug in any identity system: DID verification,
7075
JWT validation, ZKP proof checking, API key lookup, etc.
76+
77+
Note: ``verify()`` is called in a thread executor to avoid blocking
78+
the event loop. Implementations may perform synchronous I/O (network
79+
calls, file reads) safely.
7180
"""
7281

7382
@abstractmethod
@@ -87,12 +96,10 @@ class StructuralVerifier(CredentialVerifier):
8796
"""Development verifier that checks credential structure only.
8897
8998
Accepts any JSON-parseable credential with the required fields.
90-
NOT for production use a real verifier (DID, JWT, ZKP, etc.).
99+
NOT for production -- use a real verifier (DID, JWT, etc.).
91100
"""
92101

93102
def verify(self, credential: str) -> VerificationResult:
94-
import json
95-
96103
try:
97104
data = json.loads(credential)
98105
except (json.JSONDecodeError, TypeError):
@@ -125,32 +132,23 @@ def verify(self, credential: str) -> VerificationResult:
125132
)
126133

127134

128-
# ---------------------------------------------------------------------------
129-
# Per-tool permission mapping
130-
# ---------------------------------------------------------------------------
131-
132-
# Default: every tool requires these permissions. Override with
133-
# tool_permissions to set per-tool requirements.
134-
_DEFAULT_PERMISSIONS: set[str] = {"read_data"}
135-
136-
137135
# ---------------------------------------------------------------------------
138136
# Plugin
139137
# ---------------------------------------------------------------------------
140138

141139
class DelegationAuthPlugin(BasePlugin):
142-
"""ADK plugin that enforces delegation-scoped authorization.
140+
"""ADK plugin that enforces permission-scoped authorization.
143141
144142
Before each tool call, the plugin:
145143
1. Reads the agent's credential from session state
146-
2. Verifies it using the configured verifier
144+
2. Verifies it using the configured verifier (in a thread executor)
147145
3. Checks that the credential's permissions cover the tool's requirements
148146
4. Blocks execution if any check fails
149147
150-
Delegation scoping: credentials can only narrow permissions, never expand.
151-
If agent A delegates to agent B, B's permission set is always a subset
152-
of A's. The verifier enforces this at credential issuance; this plugin
153-
enforces it at execution time.
148+
Permission scoping: this plugin enforces permission checks at execution
149+
time. Delegation scope narrowing (ensuring child agents can only hold a
150+
subset of parent permissions) is enforced at credential issuance by the
151+
identity system, not by this plugin.
154152
155153
Args:
156154
required_permissions: Default permissions required for any tool call.
@@ -163,6 +161,8 @@ class DelegationAuthPlugin(BasePlugin):
163161
is stored. Defaults to ``"agent_credential"``.
164162
fail_open: If True, allow tool calls when no credential is present.
165163
Defaults to False.
164+
max_audit_entries: Maximum audit log entries before FIFO eviction.
165+
Defaults to 1000.
166166
167167
Example::
168168
@@ -177,19 +177,22 @@ class DelegationAuthPlugin(BasePlugin):
177177

178178
def __init__(
179179
self,
180-
required_permissions: set[str] | None = None,
181-
tool_permissions: dict[str, set[str]] | None = None,
182-
verifier: CredentialVerifier | None = None,
180+
required_permissions: Optional[set[str]] = None,
181+
tool_permissions: Optional[dict[str, set[str]]] = None,
182+
verifier: Optional[CredentialVerifier] = None,
183183
credential_key: str = "agent_credential",
184184
fail_open: bool = False,
185+
max_audit_entries: int = _MAX_AUDIT_ENTRIES_DEFAULT,
185186
) -> None:
186187
super().__init__(name="delegation_auth")
187-
self._required = required_permissions or _DEFAULT_PERMISSIONS
188+
self._required = required_permissions or {"read_data"}
188189
self._tool_permissions = tool_permissions or {}
189190
self._verifier = verifier or StructuralVerifier()
190191
self._credential_key = credential_key
191192
self._fail_open = fail_open
192-
self._audit_log: list[dict[str, Any]] = []
193+
self._audit_log: collections.deque[dict[str, Any]] = (
194+
collections.deque(maxlen=max_audit_entries)
195+
)
193196

194197
async def before_tool_callback(
195198
self,
@@ -215,7 +218,7 @@ async def before_tool_callback(
215218
tool.name,
216219
)
217220
return None
218-
self._log_denial(tool.name, "no_credential", tool_args)
221+
self._log_denial(tool.name, "no_credential")
219222
return {
220223
"error": "authorization_required",
221224
"message": (
@@ -224,20 +227,26 @@ async def before_tool_callback(
224227
),
225228
}
226229

227-
# Verify the credential
230+
# Verify the credential in a thread executor to avoid blocking
231+
loop = asyncio.get_running_loop()
228232
try:
229-
result = self._verifier.verify(credential)
233+
result = await loop.run_in_executor(
234+
None,
235+
functools.partial(self._verifier.verify, credential),
236+
)
230237
except Exception as exc:
231238
logger.error("Verifier raised for tool %s: %s", tool.name, exc)
232-
self._log_denial(tool.name, "verifier_error", tool_args)
239+
self._log_denial(tool.name, "verifier_error")
233240
return {
234241
"error": "verification_failed",
235242
"message": f"Credential verification error: {exc}",
236243
}
237244

238245
if not result.valid:
239246
self._log_denial(
240-
tool.name, result.reason or "invalid_credential", tool_args
247+
tool.name,
248+
result.reason or "invalid_credential",
249+
agent_id=result.agent_id,
241250
)
242251
return {
243252
"error": "authorization_denied",
@@ -254,7 +263,6 @@ async def before_tool_callback(
254263
self._log_denial(
255264
tool.name,
256265
f"missing_permissions: {sorted(missing)}",
257-
tool_args,
258266
agent_id=result.agent_id,
259267
)
260268
return {
@@ -265,19 +273,17 @@ async def before_tool_callback(
265273
),
266274
}
267275

268-
# Authorized — log and proceed
276+
# Authorized
269277
logger.info(
270278
"Agent %s authorized for %s (permissions: %s)",
271279
result.agent_id,
272280
tool.name,
273281
sorted(result.permissions),
274282
)
275-
self._log_allow(tool.name, result.agent_id, tool_args)
283+
self._log_allow(tool.name, result.agent_id)
276284
return None
277285

278-
def _log_allow(
279-
self, tool: str, agent_id: str, tool_args: dict[str, Any]
280-
) -> None:
286+
def _log_allow(self, tool: str, agent_id: str) -> None:
281287
self._audit_log.append({
282288
"action": "allow",
283289
"tool": tool,
@@ -289,7 +295,6 @@ def _log_denial(
289295
self,
290296
tool: str,
291297
reason: str,
292-
tool_args: dict[str, Any],
293298
agent_id: str = "",
294299
) -> None:
295300
self._audit_log.append({
@@ -303,5 +308,5 @@ def _log_denial(
303308

304309
@property
305310
def audit_log(self) -> list[dict[str, Any]]:
306-
"""Read-only access to the audit trail."""
311+
"""Read-only copy of the audit trail."""
307312
return list(self._audit_log)

0 commit comments

Comments
 (0)