Repository navigation
Conversation
…ched for Introduces a small, frozen model that records whether the serving edge authenticated the caller of an invocation, and who it authenticated them as. This exists so that trust decisions downstream (notably the human-in-the-loop tool confirmation gate) can key off an authentication fact rather than off a transport marker. A transport is a proxy for identity, and a proxy for identity fails in both directions: it refuses legitimate authenticated peers and it admits anyone who can reach an ungated path. No behavior change on its own. Wiring and the gate follow in later commits. For google#6461.
Adds an optional CallerPrincipal to the invocation context so downstream code can ask an authentication question instead of a transport question. None keeps the existing meaning for every in-process caller: no remote trust boundary was crossed, so there is nothing to vouch for. Nothing reads the field yet. For google#6461.
Runner.run_async takes an optional caller_principal and passes it through both invocation setup paths into _new_invocation_context. Keyword-only with a default, so every existing caller is unaffected and the synchronous Runner.run wrapper stays as it is: a direct in-process call has no serving layer to vouch for it. For google#6461.
build_caller_principal reads the same call_context._get_user_id already reads, and keeps the part _get_user_id throws away: whether an authenticator produced the name, or whether it was synthesized from the caller-supplied context id. _get_user_id keeps its return contract. The principal rides on AgentRunRequest, which both executors splat into Runner.run_async. For google#6461.
A2aAgentExecutorConfig.request_converter is replaceable, so a custom converter could otherwise drop the principal and silently reopen the hole. Re-deriving it in the executor from the RequestContext makes it structural rather than convention-dependent, in both directions: a custom converter can neither remove the principal nor assert one the server never established. For google#6461.
Same backstop as the legacy executor. Both implementations splat the AgentRunRequest into Runner.run_async, so stamping here is what actually reaches the invocation. For google#6461.
The human-in-the-loop gate now asks whether the serving layer authenticated the caller, instead of asking nothing at all. No principal allows: no remote trust boundary was crossed. Authenticated allows. Present-but-unauthenticated is the only refusal, and it is explicit -- the confirmation is rewritten to confirmed=False so the pending tool call resolves as rejected, rather than dropped, which is what made the earlier guard stall every HITL tool and get reverted. Warn by default, strict behind ADK_STRICT_CALLER_PRINCIPAL, so upgrading cannot break a deployment that runs A2A without an authenticator. For google#6461.
… path Six tests: absent principal, authenticated, unauthenticated under the default warn behavior, unauthenticated under strict, strict against an in-process caller, and the multi-turn re-entry case that the previous guard broke. The strict test asserts the refusal is explicit -- execution is reached with confirmed=False -- rather than asserting no events, which is what a stall would also produce. For google#6461.
The first cut keyed off is_authenticated alone and passed user_name straight through. Two problems: user_name is typed Optional[str] on CallerPrincipal, so any non-str value raises at construction time, and the principal could disagree with the user id derived from the same request. Now a caller is authenticated exactly when _get_user_id takes its call-context branch -- a real, non-empty user name -- with one extra check that an a2a User reporting is_authenticated False is not vouched for even if it carries a name. Stricter by one condition, never looser, and the two functions cannot drift apart. For google#6461.
…ser_id Seven cases, including a parametrized one asserting the principal is authenticated exactly when _get_user_id takes its call-context branch. That is the invariant that keeps the two from drifting apart again. For google#6461.
… policy New files under src/google/adk/ are private by default (check-new-py-prefix); public symbols are exported through the package __init__. Renames caller_principal.py to _caller_principal.py. The public export and the import-site updates follow in the next commits. Also trims one 83-column docstring line to fit the 80-column limit. For google#6461.
The module is private by policy; this is the public surface for it, next to InvocationContext and RunConfig, which are the two things it is used with. For google#6461.
Follows the rename. Import order matches isort: an underscore-prefixed module sorts before base_agent. For google#6461.
Follows the rename. For google#6461.
Follows the rename. For google#6461.
Puts the strict switch where ADK already keeps staged behavior changes. Operators opt in with ADK_ENABLE_STRICT_CALLER_PRINCIPAL=1 or override_feature_enabled(FeatureName.STRICT_CALLER_PRINCIPAL, True); the next-major flip is a one-line default_on change here. For google#6461.
Replaces the bespoke ADK_STRICT_CALLER_PRINCIPAL env var with is_feature_enabled(FeatureName.STRICT_CALLER_PRINCIPAL), which gives operators the env var (ADK_ENABLE_STRICT_CALLER_PRINCIPAL), the programmatic override, and the registry default through one mechanism the codebase already uses for staged behavior changes. For google#6461.
Strict mode is now enabled the way every other ADK feature is, through ADK_ENABLE_STRICT_CALLER_PRINCIPAL. Also flattens the event-building helper so no line exceeds 80 columns. For google#6461.
Required for a new source unit by check-new-py-prefix. Follows the adk-unit-guide template: get started, how it works, the three principal states, strict mode, configuration options, limitations. For google#6461.
Exactly what the pre-commit run reported.
Exactly what the pre-commit run reported.
main now runs a root LlmAgent through the node runtime, which builds its own invocation context. Thread caller_principal through _run_node_async into _node_runner_utils.run_node_async and set it on the context after construction, the way abort_signal is, so a principal set by a serving layer reaches the confirmation gate on that path too. Without this the gate saw None (in-process caller) for every A2A-served root agent.
…de path Fails without the previous commit: the principal arrived as None on the node runtime path, which is where every root LlmAgent runs now.
Review found a second consumer of human-in-the-loop answers, the workflow tool node, that the gate in the agent confirmation flow did not cover. The decision moves next to CallerPrincipal so both consumers refuse the same callers.
No behaviour change on this path; the gate keeps rewriting refused confirmations to confirmed=False and now shares the decision with the tool node.
…pal (3/5) _ToolNode._apply_confirmation_resume (ce132b9) stored ctx.resume_inputs on ctx.tool_confirmation with no principal check, and apply_confirmation_gate is a no-op for tools whose check_require_confirmation is False yet read tool_confirmation themselves, ExecuteBashTool included. With strict mode on, an unauthenticated caller could resume a paused tool node with confirmed=True and the tool ran. The answer is now rewritten to confirmed=False before it is stored, so every reader sees the refusal.
…callers (4/5) Covers the require_confirmation path and the request_confirmation path on the tool node in strict mode, the default-mode warning, and an authenticated caller as control. All six fail on the tree without the tool node change.
The guide said only the tool confirmation flow reads the principal; it now lists the LlmAgent confirmation flow and the workflow tool node resume, and notes that tools reading tool_confirmation directly see the refusal.
…ncipal gate Moves the principal decision made in _ToolNode._apply_confirmation_resume into _workflow_hitl_utils.resumed_tool_confirmation, next to confirmation_interrupt_id and parse_tool_confirmation from main, so every workflow consumer of a resumed confirmation answer makes the same decision. FunctionNode require_confirmation (988fd1a on main) is the second consumer and is switched over in the next commit.
…ncipal @node(require_confirmation=True) (988fd1a, after this PR's last push) reads the resumed answer from ctx.resume_inputs the same way the tool node does, and had no principal check: in strict mode an approval arriving on an invocation with an unauthenticated principal ran the function. It now goes through resumed_tool_confirmation, so the answer is stored as a rejection for a caller the serving layer could not authenticate. Tests in the next commit fail on main without this.
…cated caller in strict mode Mirrors the tool node tests: a Workflow with one @node(require_confirmation=True) node is paused, then resumed with {confirmed: True} on an invocation carrying an unauthenticated principal. In strict mode the function must not run and the downstream node sees the rejection output; an authenticated principal is honored. The first test fails on main without the previous commit (the function ran).
…tring Docstring only.
…nodes too Guide text only.
Strict mode stores an approval from an unauthenticated principal as confirmed=False and keeps one from an authenticated principal; the existing test pins the in-process case (no principal).
The API server does not authenticate callers. A deployment that does can set request.state.caller_principal from middleware; without one, /run and /run_sse now run the invocation as CallerPrincipal(authenticated=False, source="api_server"), so a tool confirmation carried by the request is warned about (default) or refused (ADK_ENABLE_STRICT_CALLER_PRINCIPAL=1), the same decision made for an unauthenticated A2A caller. The body user_id is a claim, not a verified identity, and is not consulted. The kwarg is only passed when runner.run_async accepts it. Stacked on google#7134 (caller-principal-gate). Relates to google#7569.
Unit tests: the routes pass CallerPrincipal(authenticated=False, source="api_server") by default, use a principal set on request.state by middleware, and omit the kwarg for a runner whose run_async does not accept it. End to end over /run: strict mode refuses a confirmation from an unauthenticated caller, honors one from an authenticated operator set by middleware, and default mode honors it with a warning.
Documents the unauthenticated default on /run and /run_sse, the request.state.caller_principal convention for deployments that authenticate in middleware, and the strict-mode note for the adk web dev UI. Rewrites the limitations bullet: /run_live sessions still carry no principal.
Contributor
|
Warning You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again! |
Contributor
Author
|
@xuanyang15 ready for review |
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
2 participants
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.
Stacked on #7134. The first 41 commits are #7134 unchanged; this PR is the last three commits (9631358, d4ff60a, 9699e88): cybrdude:adk-python:caller-principal-gate...cybrdude:adk-python:api-server-caller-principal. Once #7134 merges I will rebase and the diff collapses to those three files.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
Problem:
#7134 gates human-in-the-loop tool confirmations on
InvocationContext.caller_principal, which the A2A executor sets from the server call context. The ADK API server never sets one, so on/runand/run_ssethe gate seesNoneandcaller_may_confirm(None)returnsTrue. Any client that can reach the API server can answeradk_request_confirmationwith{"confirmed": true}and the gated tool runs, in strict mode too.user_idin the request body is a claim the caller made, not something the server checked, so it cannot stand in for a principal.Solution:
/runand/run_ssenow pass aCallerPrincipaltoRunner.run_async:request.state.caller_principalto aCallerPrincipal, that value is used as is.CallerPrincipal(authenticated=False, source="api_server"), so an approval carried by the request is warned about (default) or refused (ADK_ENABLE_STRICT_CALLER_PRINCIPAL=1), the same decision feat: gate human-in-the-loop tool confirmation on the caller principal #7134 already makes for an unauthenticated A2A caller.The kwarg is only passed when
runner.run_asyncaccepts it (sameinspect.signaturecheck asabort_signal, factored into_run_async_accepts), so custom runners without the parameter keep working.run_agent_ssegainsrequest: Request = None; FastAPI injects it over HTTP, and tests that call the handler directly are unaffected (Optional[Request]is rejected by FastAPI, hence the default)./run_liveis out of scope and is documented as not carrying a principal.Docs: a new "ADK API server" section in
docs/guides/agents/caller_principal/index.mdwith the middleware convention and a note that theadk webdev UI has no authentication of its own, so strict mode needs middleware that recognizes its operator; the Limitations bullet is rewritten accordingly.Testing Plan
Unit Tests:
New in
tests/unittests/cli/test_fast_api.py:test_agent_run_records_unauthenticated_caller_by_default[/run, /run_sse]test_agent_run_uses_caller_principal_set_by_middleware[/run, /run_sse]test_agent_run_omits_caller_principal_for_runner_without_the_parametertest_agent_run_strict_mode_refuses_confirmation_without_authentication(realRunner, confirmation-gatedFunctionTool, forged approval over/run; tool does not run, result is{"error": "This tool call is rejected."})test_agent_run_strict_mode_honors_confirmation_from_authenticated_operatortest_agent_run_default_mode_honors_confirmation_with_a_warningNegative control: with
api_server.pyfrom the base branch and the new tests, the three gating tests fail (assert None == CallerPrincipal(...),assert ['rm_all'] == [], no warning logged). The rest oftest_fast_api.pyis unchanged: the failures I see there are pre-existing in my environment (missing optional eval/vertex extras) and identical before and after this change.Manual End-to-End (E2E) Tests:
Agent dir with one
LlmAgentwhose first turn callsrm_all(aFunctionTool(require_confirmation=True)that writes a marker file), mock model, served withget_fast_api_app(agents_dir=..., web=False)under uvicorn. Driver: create a session,POST /runwith a text message, read theadk_request_confirmationcall id from the events,POST /runagain withfunctionResponse {"id": <id>, "name": "adk_request_confirmation", "response": {"confirmed": true}}from the same unauthenticated HTTP client.Checklist
Additional context
The principal records the outcome of authentication; it does not perform it. Deployments that already front the API server with their own auth get the strict-mode behavior by setting one attribute in middleware (example in the guide). Deployments with nothing in front keep today's behavior plus a warning per approval, and get a refusal only when they opt into strict mode.