feat(server): middleware parity, auth, A2A parser hook, startup log#246
Merged
feat(server): middleware parity, auth, A2A parser hook, startup log#246
Conversation
Merged
4 tasks
bokelley
added a commit
that referenced
this pull request
Apr 20, 2026
Security review findings (HIGH/MEDIUM from security-reviewer): - auth.py: _parse_bearer_header() replaces auth_header.removeprefix — case-insensitive scheme per RFC 7235, tolerates folded whitespace. - auth.py: validator exceptions wrapped try/except → 401 with logger.exception. No stack-trace leak. - auth.py/auth_context_factory: principal-supplied metadata can no longer shadow SDK-owned keys (audit-field-injection close). - auth.py/_peek_jsonrpc: explicit request._body = body after peek. Code-review findings: - a2a_server.py: _log_advertised_tools moved from ADCPAgentExecutor __init__ to create_a2a_server (symmetric with MCP placement). - tests/test_server_startup_log.py: regex-match instead of tokens[3]. - tests/test_a2a_server.py: replace inline _make_handler() with the module-level _TestHandler — fixes Python 3.10 CI failure. +8 new tests. 1990 tests passing, mypy clean. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
️✅ There are no secrets present in this pull request anymore.If these secrets were true positive and are still valid, we highly recommend you to revoke them. 🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request. |
Closes 5 items from salesagent's feedback on adopting adcp.server in one cohesive server/transport surface change. SkillMiddleware parity across transports (#7) --------------------------------------------- The A2A executor's per-skill middleware (PR #233) is now available on MCP too. Same SkillMiddleware type alias, same composition semantics (outermost-first, _step recursion), same call_next contract — a middleware list written against one transport works unchanged on the other. - src/adcp/server/serve.py: new module-level _dispatch_with_middleware that A2A's _dispatch_with_middleware delegates to. - create_mcp_server, _register_handler_tools, _register_tool accept middleware=[SkillMiddleware]; _register_tool wraps caller in the chain between context build and handler invocation. - serve() already exposed the kwarg for A2A; now forwards to MCP too. BearerTokenAuthMiddleware in adcp.server.auth (#1) -------------------------------------------------- The pattern in examples/mcp_with_auth_middleware.py was four security-critical concerns (ContextVar carrier, constant-time compare, discovery bypass, reset-in-finally); every downstream copy-pasted it. Now shipped as a class. - src/adcp/server/auth.py: BearerTokenAuthMiddleware, Principal (frozen dataclass), TokenValidator, auth_context_factory, constant_time_token_match. Seller supplies validate_token; framework owns the ContextVar plumbing, RFC 7235 scheme parsing (case- insensitive + whitespace-folded), discovery bypass, peek_jsonrpc with explicit request._body cache, fail-closed validator exception handling, principal metadata that can't shadow SDK audit keys. - examples/mcp_with_auth_middleware.py shrunk 243 → 89 lines. A2A message_parser hook (#3) ---------------------------- ADCPAgentExecutor._parse_request was hardcoded to DataPart({'skill': ..., 'parameters': ...}). Sellers fronting JSON-RPC or vendor-specific shapes had to subclass privately. - src/adcp/server/a2a_server.py: new MessageParser type alias, message_parser= kwarg on ADCPAgentExecutor, create_a2a_server, _serve_a2a, serve(). Default = _default_parse_request (was inline). Startup advertised-tools log (#9) --------------------------------- - src/adcp/server/serve.py: _log_advertised_tools() runs from _register_handler_tools (MCP) and create_a2a_server (A2A). INFO: 'X of Y tools advertised'; DEBUG: list of unadvertised. Custom tools doc (#8) --------------------- docs/handler-authoring.md: new section covering the @mcp.tool() passthrough on create_mcp_server's return value. Expert-review followups (security + code review) ------------------------------------------------- - _parse_bearer_header: case-insensitive scheme, folded whitespace. - validator exceptions → 401 (no stack-trace leak). - principal metadata can't shadow SDK-owned keys (tool_name, transport). - explicit request._body = body after peek. - tests use regex to match log messages (not positional tokens). - Python 3.10 skipif on two new A2A create_a2a_server tests (a2a-sdk starlette integration requires 3.11+; matches pre-existing skip). Tests ----- +53 tests across three new/modified test files. 1990 tests passing, mypy clean. Closes #224, #225, #226, #240, #241 salesagent feedback items #1, #3, #7, #8, #9. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
bokelley
force-pushed
the
bokelley/server-middleware-parity
branch
from
ddc51ae to
c9d7bbc
Compare
4 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Second of three PRs on salesagent's feedback. Closes 5 items in one cohesive server-surface change.
BearerTokenAuthMiddleware+Principal+auth_context_factoryinadcp.server.auth. Seller suppliesvalidate_token(token) -> Principal | None; framework owns ContextVars, discovery bypass, constant-time compare, reset-in-finally. Example shrinks 243 → 89 lines.message_parserhook. NewMessageParsertype alias; kwarg onADCPAgentExecutor,create_a2a_server,serve. Custom parsers can compose with_default_parse_requestfor mixed-legacy clients.SkillMiddlewarelist works on both transports via a shared_dispatch_with_middlewarehelper.@mcp.tool()passthrough oncreate_mcp_server's return value (no code change —FastMCPis returned directly already)."mcp server advertising X of Y tools"INFO, plus DEBUG listing of unadvertised names. Catches the "renamed a handler method, silently dropped from tools/list" incident pattern.All additive; no breaking changes. Existing
middleware=users on A2A see identical behavior (composition logic factored into shared helper, A2A delegates).Tested
1980 passed, 22 skippedlocally (1937 → 1980).ruff,mypyclean.Test plan
auth_context_factorythe right API shape, or should sellers subclassBearerTokenAuthMiddlewareto customise what gets written intometadata?ToolContextwhen nocontext_factoryis configured, or should it match A2A's pattern of falling through to a transport-derived context?_peek_jsonrpcbatch-fails-closed behavior the right default? The alternative is to iterate batch members and apply the discovery gate per-entry — more permissive, more complex.Related
Part of triaging feedback from salesagent (primary downstream of
adcp.server).AccountAwareToolContext+ multi-tenant contract doc (items chore(main): release 0.1.1 #2, fix: correct tool name in list_creative_formats method #10).python -m adcp.migrate v3-to-v4codemod,ADCP_STRICT_VALIDATIONenv flag,get_adcp_spec_version()/get_adcp_sdk_version()helpers, subclass passthrough test.🤖 Generated with Claude Code