fix(auth): stop OAuth2 client_secret and tokens from leaking over /run, /run_sse, /run_live

[prasanna8585]

Summary

When a tool requires OAuth2 authentication, ADK attaches the credential to an adk_request_credential function call so the client can complete the interactive auth flow. That credential — including client_secret, access_token, refresh_token, id_token, auth_code, and code_verifier — was serialized in full and sent to whatever client is connected to /run, /run_sse, or /run_live.

These fields are already marked Field(repr=False) on AuthCredential, but repr=False only affects repr()/str() output (logs, error strings) — it has no effect on model_dump()/model_dump_json(), which is what actually leaves the process in these three responses. A client_secret is meant to stay server-side per the OAuth2 spec; sending it to any client capable of connecting to these endpoints lets that client impersonate the application itself to the identity provider.

Why the fix lives where it does

The redaction can't happen by excluding the fields on the model, or by redacting the event before it's returned from the agent run:

• FunctionCall.args is an opaque dict[str, Any], not a nested pydantic model, so exclude= can't reach a secret embedded inside it by field path.
• Later turns reconstruct the original request's credential by re-parsing the persisted adk_request_credential call's args — so stripping the secret before SessionService.append_event would make it unrecoverable for that mechanism.
  Instead, this adds CREDENTIAL_SECRET_KEYS (the by-alias counterpart of every field already marked repr=False) and a small recursive redaction step applied only to the outbound wire representation in /run, /run_sse, and /run_live — after the event has already been produced and persisted.

Testing

• Added a consistency test asserting CREDENTIAL_SECRET_KEYS can't silently drift from the set of repr=False fields.
• Added an end-to-end /run_sse test confirming a credential's secret fields are absent from the streamed response, while the fields a client legitimately needs (client_id, the authorization URL, the credential key) are preserved.
• Verified the new /run_sse test fails against the pre-fix code and passes against the fix.
• tests/unittests/auth/: 241 passed.
• tests/unittests/cli/test_fast_api.py: all /run, /run_sse, /run_live, session, and auth/credential tests pass.

[tonydzi]

disclosure: i am an AI agent (Claude) running on Anton Dzyatkovsky's machine (github user tonydzi). autonomous run, nobody read this before it posted, so re-run the numbers rather than taking them. no stake in this repo beyond wanting the fix to hold.

read the three touched source files whole, not just the diff. the diagnosis is right and the placement argument in the description is right: repr=False does nothing for model_dump(), FunctionCall.args is an opaque dict so exclude= cannot reach into it, and the secret has to survive in the persisted event for the merge-backfill to work. so redacting only the outbound representation is the correct shape.

three things, in order of how much they matter.

1. five of the eight redaction sites are not covered by any test, including /run and /run_live

i neutralised each call site in turn (_redact_credential_secrets(x) -> identity) and ran tests/unittests/cli/test_fast_api.py + tests/unittests/auth/:

mutant	verdict	killed by
/run	SURVIVED	nothing
/run_sse	killed	test_agent_run_sse_redacts_oauth2_client_secret
/run_live (websocket)	SURVIVED	nothing
_redacted_session_response (get/create/update session)	killed	test_get_session_redacts_oauth2_client_secret
_redacted_sessions_response (list_sessions)	SURVIVED	nothing
dev get_eval_result_legacy	SURVIVED	nothing
dev get_eval	killed	test_get_eval_redacts_oauth2_client_secret
dev get_eval_result	SURVIVED	nothing

the title of the PR names three endpoints and only one of them is guarded. /run is a separate code path from /run_sse (it builds its own JSONResponse from a list, the SSE path builds one dict per event through json.dumps), and /run_live is a third. reverting the /run line alone leaves the whole suite green.

a twin of the existing SSE test, driven through /run. same fixtures, same auth_config_dict, same monkeypatched Runner.run_async, only the request and the assertions on the response change:

  payload = {
      "app_name": info["app_name"],
      "user_id": info["user_id"],
      "session_id": info["session_id"],
      "new_message": {"role": "user", "parts": [{"text": "Hello agent"}]},
      "streaming": False,
  }

  response = test_app.post("/run", json=payload)
  assert response.status_code == 200
  assert "should-never-reach-the-client" not in response.text
  assert "pkce-verifier-should-not-leak-either" not in response.text

  events = response.json()
  assert len(events) == 1
  args = events[0]["content"]["parts"][0]["functionCall"]["args"]
  raw_oauth2 = args["authConfig"]["rawAuthCredential"]["oauth2"]
  exchanged_oauth2 = args["authConfig"]["exchangedAuthCredential"]["oauth2"]
  assert "clientSecret" not in raw_oauth2
  assert "clientSecret" not in exchanged_oauth2
  assert "codeVerifier" not in exchanged_oauth2
  assert raw_oauth2["clientId"] == "public-client-id"
  assert exchanged_oauth2["authUri"].startswith("https://idp.example.com/oauth2/auth")
  assert args["authConfig"]["credentialKey"] == "my_tool:oauth2:abcd1234"

measured: green on the PR as it stands, and with only the /run redaction neutralised it goes red, while the other three redaction tests stay green, so it is red for the right reason and it is the only thing that catches that mutant.

2. the filter deletes any key with one of those names, anywhere in the payload, and deletes rather than marks

_redact_credential_secrets walks the whole dumped event or session unconditionally, and thirteen of the names are generic: token, password, apiKey, accessToken, refreshToken, idToken, additionalHeaders. tool results and session state are dict[str, Any] the app controls.

probe through /run, tool returning a page cursor and a couple of documented field names:

tool returned : {"results": ["a","b"], "token": "next-page-cursor-abc",
                 "password": "...", "apiKey": "...", "nested": {"accessToken": "..."}}
client sees   : {"results": ["a","b"], "nested": {}}
stateDelta {"token": 42, "user:password_hint": "x"} -> {"user:password_hint": "x"}

and through the session endpoints, an app posting its own state:

POST /apps/{a}/users/{u}/sessions  state={"token": "csrf-abc", "apiKey": "...", "page": 3}
  create_session -> {"page": 3}
  get_session    -> {"page": 3}

the server stored all three; the client can never read two of them back. the model saw the tool's token, the transcript the client renders does not, and nothing marks the difference: an omitted key is indistinguishable from a key the tool never returned.

two cheap ways to soften it, either is fine:

• replace instead of delete. auth_credential.py already defines _REDACTED = "<redacted>" and uses it in __repr_args__; reusing it here keeps the payload shape and makes the removal legible.
• or scope the walk: only descend into subtrees that are actually credential-shaped (a dict carrying authType, or the values under authConfig / rawAuthCredential / exchangedAuthCredential / requestedAuthConfigs) instead of every dict in the response.

worth saying plainly: this is not a security hole, it is a behaviour change to non-credential data on production endpoints, and it is silent. i did not find a caller in src/ that is harmed. ADK's own SessionStateCredentialService stores under auth_config.credential_key, which does not collide.

3. the drift guard cannot see a credential model that is added later

test_credential_secret_keys_covers_every_repr_hidden_field iterates a hardcoded tuple of five classes. ServiceAccount is already outside it (no repr=False fields today, so no gap yet). i added a hypothetical MtlsCredential(BaseModelWithConfig) with one repr=False field to the same module:

test result       : 1 passed
dumped by_alias   : {"certChain": "x", "clientCertificateKey": "LEAKED-PRIVATE-KEY"}
after redaction   : {"certChain": "x", "clientCertificateKey": "LEAKED-PRIVATE-KEY"}

green suite, secret on the wire. discovering the models by reflection instead closes it: inspect.getmembers(module, inspect.isclass) filtered to issubclass(obj, BaseModelWithConfig). run that way against the PR as it stands: six models found, zero missing and zero stale, so the set is exactly right today; with the probe class present it reports clientCertificateKey missing. also note the assertion is one-directional (<=), so a key that stops existing stays in the set forever; an equality assert would catch that too.

things i checked that came out clean, and scope

• i suspected the camelCase key set would miss a credential nested in an Any-typed field, since by_alias has nothing to bite on there. it does not: pydantic applies the alias generator to a BaseModel sitting inside state / state_delta too, so a credential parked in session state by SessionStateCredentialService dumps as clientSecret and is stripped on every session endpoint and in actions.stateDelta on /run and /run_sse. hypothesis dropped rather than published.
• tests/unittests/auth/: 241 passed. combined test_fast_api.py + tests/unittests/auth/: 360 passed, 5 failed, 5 collection errors, and the same five failures by name on main (test_list_metrics_info, four test_finalize_agent_identity_credentials_*), all missing optional deps on this box, so nothing here is the PR's.
• i did not exercise the dev UI, a real OAuth provider, or the /dev/apps/{app}/debug/trace/... endpoints. the trace ones return raw span attributes and are gated behind should_add_content_to_legacy_spans; i did not measure whether an adk_request_credential call reaches them, so treat that as an open question, not a finding.

finding 1 is the one worth acting on.

[prasanna8585]

Thanks for this - genuinely one of the most useful reviews I've gotten on this PR. All three findings confirmed and fixed, verified the same way you found them (neutering each call site and checking the right test catches it):

Added tests for all 5 previously-uncovered redaction sites. Also traced why list_sessions "survived" - the existing check couldn't have failed either way, since InMemorySessionService.list_sessions() strips events before redaction is even in play. Fixed by exercising a response that actually includes events.
Rescoped redaction to only strip within dicts that are actual AuthCredential dumps (detected by the authType field) instead of matching secret-like names anywhere. Confirmed your probe case now passes through untouched, and confirmed the session-state case you checked is still fully covered.
Replaced the hardcoded class list in the drift guard with reflection over every credential class in the module, plus tightened the assertion to exact equality. Re-ran your MtlsCredential probe against it - now caught correctly.

Appreciate the rigor - this is a meaningfully better PR because of it.