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.

[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.

Re-ran all three findings on a17b896c the same way they were found. All three close. Below that, four things the re-run turned up: one new leak path, one call site that still has no test, one test that only works because of a monkeypatch, and a note on what the reflection guard does and does not reach.

Baseline, so the counts are comparable. tests/unittests/auth + tests/unittests/cli/test_fast_api.py on a17b896c: 376 passed, 4 failed. Same command on main: 366 passed, 4 failed, the same four by name (test_finalize_agent_identity_credentials_*, optional deps that will not install under py3.12 here), so none of them is from the PR. Environment: python 3.12.13, pydantic 2.13.5.

---

1. Call-site mutation, now one site at a time: 8 of 12 killed

Last round I mutated _redacted_session_response as a single site. That was too coarse: it hides the fact that six separate endpoints route through it. This time each individual call was neutered on its own (identity pass-through, everything else untouched) and the full suite re-run:

call site	mutant
get_session	killed by test_get_session_redacts_oauth2_client_secret
list_sessions	killed by test_list_sessions_redacts_oauth2_client_secret
/run	killed by test_agent_run_redacts_oauth2_client_secret
/run_sse	killed by test_agent_run_sse_redacts_oauth2_client_secret
/run_live (ws)	killed by test_run_live_websocket_redacts_oauth2_client_secret
dev get_eval	killed by test_get_eval_redacts_oauth2_client_secret
dev get_eval_result	killed by test_get_eval_result_redacts_oauth2_client_secret
dev get_eval_result_legacy	killed by test_get_eval_result_legacy_redacts_oauth2_client_secret
update_session	survived
create_session (no body)	survived
create_session (with body)	survived
create_session_with_id	survived

The five you set out to cover are all genuinely covered. The four survivors are the session-write endpoints, which the coarse round-1 mutant lumped in with get_session.

2. Of those four, update_session is the one that matters

PATCH .../sessions/{id} does not return the state delta the caller sent, it returns the whole Session. So an adk_request_credential call persisted by an earlier turn rides out on it exactly as it does on GET, and right now nothing proves the redaction is wired there.

The other three are create_session, and I could not build a real leak through them: _validate_session_initialization_events rejects client-supplied events whose function names are ADK-reserved, so a freshly created session cannot carry an ADK-generated credential. Redaction there is defence in depth against state the caller itself just supplied. I am not proposing tests for those three.

For update_session, this one is red exactly when that call site is neutered, green otherwise, and it does not fire on any of the other eleven mutants:

@pytest.mark.asyncio
async def test_update_session_redacts_oauth2_client_secret(
    test_app, create_test_session, mock_session_service
):
  """PATCH .../sessions/{id} returns the whole session, history included."""
  info = create_test_session

  auth_config_dict = {
      "authScheme": {"type": "oauth2", "flows": {}},
      "rawAuthCredential": {
          "authType": "oauth2",
          "oauth2": {
              "clientId": "public-client-id",
              "clientSecret": "should-never-reach-the-client-via-patch",
          },
      },
      "credentialKey": "my_tool:oauth2:abcd1234",
  }

  session = await mock_session_service.get_session(
      app_name=info["app_name"],
      user_id=info["user_id"],
      session_id=info["session_id"],
  )
  await mock_session_service.append_event(
      session=session,
      event=Event(
          author="agent",
          invocation_id="invocation_id",
          content=types.Content(
              role="user",
              parts=[
                  types.Part(
                      function_call=types.FunctionCall(
                          name="adk_request_credential",
                          id="adk-req-cred-id",
                          args={
                              "functionCallId": "adk-original-fc-id",
                              "authConfig": auth_config_dict,
                          },
                      )
                  )
              ],
          ),
      ),
  )

  url = (
      f"/apps/{info['app_name']}/users/{info['user_id']}"
      f"/sessions/{info['session_id']}"
  )
  response = test_app.patch(url, json={"state_delta": {"counter": 1}})

  assert response.status_code == 200
  assert "should-never-reach-the-client-via-patch" not in response.text
  event = response.json()["events"][0]
  raw_oauth2 = event["content"]["parts"][0]["functionCall"]["args"][
      "authConfig"
  ]["rawAuthCredential"]["oauth2"]
  assert "clientSecret" not in raw_oauth2
  assert raw_oauth2["clientId"] == "public-client-id"

3. test_list_sessions_redacts_oauth2_client_secret is non-vacuous only because of the monkeypatch, and it does not need to be

Your docstring is right that a secret-in-events check against the real backend passes vacuously. I measured how far that goes: neither bundled backend returns events from list_sessions.

InMemorySessionService             list_sessions -> events per session: [0]
DatabaseSessionService (sqlite)    list_sessions -> events per session: [0]

So the monkeypatch does not just make the assertion sharper, it constructs a response shape no shipped SessionService produces. The test is still worth keeping as a wiring check, but on its own it does not show the endpoint guards anything real.

It does guard something real, through state rather than events. SessionStateCredentialService.save_credential parks a live AuthCredential in session state under auth_config.credential_key, and list_sessions returns state while dropping events. Measured on both backends, dumped the way the endpoint dumps:

InMemorySessionService             events=[0] secret_in_raw_list=True  secret_after_redaction=False
DatabaseSessionService (sqlite)    events=[0] secret_in_raw_list=True  secret_after_redaction=False

That is a real leak on main and a real fix on the PR, and it needs no monkeypatch:

@pytest.mark.asyncio
async def test_list_sessions_redacts_credential_parked_in_state(
    test_app, mock_session_service
):
  """list_sessions strips events but returns state, and a credential lives there."""
  from google.adk.auth.auth_credential import AuthCredential
  from google.adk.auth.auth_credential import AuthCredentialTypes
  from google.adk.auth.auth_credential import OAuth2Auth

  credential = AuthCredential(
      auth_type=AuthCredentialTypes.OAUTH2,
      oauth2=OAuth2Auth(
          client_id="public-client-id",
          client_secret="should-never-reach-the-client-via-state",
      ),
  )
  await mock_session_service.create_session(
      app_name="test_app_name",
      user_id="test_user",
      state={"adk_oauth2_scheme_oauth2_cred": credential},
  )

  response = test_app.get("/apps/test_app_name/users/test_user/sessions")

  assert response.status_code == 200
  assert "should-never-reach-the-client-via-state" not in response.text
  parked = response.json()[0]["state"]["adk_oauth2_scheme_oauth2_cred"]
  assert "clientSecret" not in parked["oauth2"]
  assert parked["oauth2"]["clientId"] == "public-client-id"

Both added tests pass on the PR (376 -> 378, same 4 pre-existing failures), and the state one kills the list_sessions mutant on its own.

4. New: the dev-UI debug-trace endpoints still hand out the same secret, and this walker cannot clean them

You extended redaction to the dev eval endpoints "for consistency and defense in depth". Two dev endpoints in the same server carry the same credential and are not covered:

• GET /dev/apps/{app_name}/debug/trace/{event_id} returns trace_dict[event_id] verbatim
• GET /dev/apps/{app_name}/debug/trace/session/{session_id} returns dict(span.attributes) per span

trace_dict is filled by ApiServerSpanExporter, which stores dict(span.attributes) for every call_llm span. trace_call_llm writes gcp.vertex.agent.llm_request from _build_llm_request_for_trace(llm_request), and that includes contents in full. The conversation contains the adk_request_credential function call, so the clientSecret is in there. ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS defaults on.

Probe run against production code, nothing mocked, building the function call through AuthToolArguments(...).model_dump(mode="json", exclude_none=True, by_alias=True) exactly as functions.py does, then a real span through a real ApiServerSpanExporter:

1) secret is inside the function-call args the flow builds: True
2) span landed in trace_dict under the event id: True
3) secret present in what GET /dev/.../debug/trace/<event_id> returns: True | code_verifier: True
   attribute carrying it: ['gcp.vertex.agent.llm_request']
   that attribute's python type: str
4) secret STILL present after _redact_credential_secrets(attrs): True

Line 4 is the part I would flag hardest. Wiring _redact_credential_secrets into these two endpoints would not help: the walker descends dicts and lists, and here the credential is inside a JSON string that safe_json_serialize already produced. The fix has to happen upstream, where the attribute is built.

There is already precedent for that in the same function. _build_llm_request_for_trace excludes config.http_options with the comment that headers "commonly holds an Authorization bearer token" and "None of it may reach an exported span attribute". contents is the sibling case and is not filtered.

Severity: dev-server only (adk web), same tier as the eval endpoints you just covered, not the /run family. Pre-existing on main, not caused by this PR. I am raising it here because this PR is what establishes the invariant, and an obvious next reader will assume the invariant now holds server-wide.

5. The other two findings, re-measured

Finding 2 (over-broad name matching), closed, measured over HTTP rather than through the helper. A tool response with the five coincidental names now arrives intact:

{"apiKey": "user-supplied-key", "nested": {"accessToken": "unrelated-nested-value"},
 "password": "scraped-page-password", "results": ["a", "b"], "token": "next-page-cursor-abc"}

and POST /sessions with state={"token": "csrf-abc", "apiKey": "k", "page": 3} reads back as all three keys, where before it was {"page": 3}.

I also tried to break the new authType shape check and could not. A credential parked in state survives a DatabaseSessionService (sqlite) round trip still carrying authType (['apiKey', 'authType', 'http', 'oauth2', 'resourceRef', 'serviceAccount'], exclude_none is lost in the round trip but the alias casing is not), so _is_credential_shaped still finds it and the secret is still stripped. That was my main worry about shape-based detection and it did not hold up.

Finding 3 (drift guard), closed. Re-ran last round's probe: added an MtlsCredential(BaseModelWithConfig) with one repr=False field. Previously green with clientCertificateKey: LEAKED-PRIVATE-KEY on the wire, now:

E  Missing from CREDENTIAL_SECRET_KEYS: {'clientCertificateKey'}
E  No longer used by any repr=False field: frozenset()

The exact-equality assert also fixes the one-directional problem, so a stale key cannot sit in the set forever. One boundary worth writing down somewhere: inspect.getmembers(auth_credential, ...) reaches classes in that module, so a credential model added in a different module would not be discovered. Fine today, and the docstring says "in this module", just easy to forget.

What I did not measure

The live dev UI, a real OAuth provider, and the a2a surface. The trace probe drives trace_call_llm with a hand-built InvocationContext rather than a full agent run, so what it proves is that the attribute is built and stored with the secret in it, not that a particular real agent run reaches that line.

[prasanna8585]

Thanks — this was another genuinely useful pass, all four addressed:

• update_session and list_sessions: added your tests, both mutation-verified (isolated the actual return-statement mutation for update_session since it shares a helper with get_session; confirmed the state-based list_sessions test kills the mutant on its own, no monkeypatch needed).

• Dev-trace leak: fixed upstream in _build_llm_request_for_trace, redacting contents before serialization rather than trying to reach into the already-stringified span attribute afterward — confirmed with a real trace_call_llm call built the same way build_auth_request_event constructs the credential call. Had to relocate redact_credential_secrets into auth_credential.py to avoid a circular import with telemetry/tracing.py; api_server.py and dev_server.py now both import it from there.

• One thing I did not fully verify the way you did the others: llm_response's parallel span attribute. I reasoned through why it's architecturally different (model output vs. ADK's own injected conversation history) but didn't build a dynamic PoC for it — flagging that rather than claiming it's covered.

All green: 245 auth tests, the fast_api suite, and test_spans.py (134 + 1 new), same known-unrelated failures as before.

[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.

All three verified, your unverified one answered, and one thing the fix does not reach yet.

1. The trace fix is right, and it is the right shape

Checked against the failure mode I was worried about (redacting by killing the attribute, which would blank the dev UI). Built the credential call the way build_auth_request_event does, then called _build_llm_request_for_trace directly:

	contents present	function call survives	clientSecret	accessToken	refreshToken
this PR	yes	yes	absent	absent	absent
merge-base (control)	yes	yes	present	present	present

Same probe, same process, only PYTHONPATH swapped between two worktrees, so the instrument is proven red before I trusted it green.

Your new test is load-bearing, not decorative. Mutant: drop the redact_credential_secrets(...) wrapper and change nothing else, and test_trace_call_llm_redacts_oauth2_client_secret_from_contents dies on its own line, assert 'should-never-reach-the-trace' not in serialized. Suite went 11 failed / 125 passed to 12 failed / 124 passed, so exactly one test noticed.

Your "same known-unrelated failures" claim also holds exactly: merge-base is 11 failed / 124 passed, this PR is 11 failed / 125 passed. The delta is your new test and nothing else.

2. Your open question on llm_response: your reasoning is right, by enumeration

You flagged that you reasoned it through instead of building a PoC. The reasoning survives a check. Every LlmResponse construction in the tree is in a model adapter, a model connection, streaming_utils, or _reflect_retry_model_plugin (and that one only builds its own retry/error text). trace_call_llm has exactly two call sites, both in base_llm_flow at the model boundary. ADK never injects its own conversation history into an LlmResponse, so the adk_request_credential args cannot arrive there the way they arrive in contents.

One residual worth stating out loud, since it is not covered by anything here: redaction is trace-only, the real credential is still what actually goes to the model in the request. A model that echoes a token back in its output would land it in gcp.vertex.agent.llm_response, which is dumped with no redaction at all. That is model behaviour, not an ADK path, and I would not hold the PR for it.

3. What the fix does not reach: trace_merged_tool_calls

Its own docstring says it exists only to serve the dev UI:

Calling this function is not needed for telemetry purposes. This is provided for preventing /debug/trace requests (typically sent by web UI).

That is the same surface this PR is closing, and it still dumps the whole event unredacted. actions.state_delta is where SessionStateCredentialService.save_credential parks the exchanged AuthCredential under an app-chosen key, which is the exact vector your _is_credential_shaped docstring already describes.

Measured end to end, not by calling the traced function directly. Two tools in one model turn, one of them completing OAuth the documented way, real runner:

spans: generate_content mock, execute_tool connect_calendar,
       execute_tool list_events_tool, execute_tool (merged), call_llm, ...

execute_tool (merged) -> gcp.vertex.agent.tool_response  (862 bytes)
  clientSecret  present
  accessToken   present
  refreshToken  present

Then the same real spans through the real exporters that back both endpoints:

• ApiServerSpanExporter (GET /debug/trace/{event_id}): 5 keys served, 1 leaks. The merged span is keyed because it sets gcp.vertex.agent.event_id itself.
• InMemoryExporter (GET /debug/trace/session/{session_id}, whose body is "attributes": dict(s.attributes)): 9 spans in the body, 1 leaks.
• Individual execute_tool <name> spans do not leak, because those set tool_response from function_response.response only. So this is exactly one span, and it is the one nobody touched.

Two more things I checked before calling it real: it pre-exists on the merge-base identically, so it is a gap and not a regression from this PR; and should_add_content_to_legacy_spans falls back to ADK_CAPTURE_MESSAGE_CONTENT_IN_SPANS, which defaults on, so it is the default path and not an opt-in one.

Suggested fix, same idea as yours, redact the dict before it becomes a string:

       function_response_event_json = json.dumps(
           redact_credential_secrets(
               function_response_event.model_dump(exclude_none=True, mode="json")
           ),
           ensure_ascii=False,
           separators=(",", ":"),
       )

I checked the thing that actually matters for the UI: with no credential in the event, the attribute is byte-identical to what model_dump_json(exclude_none=True) produced, including unicode, floats, -0.0 and 1e300. With a credential, the diff over the attribute's full key-path set is exactly three paths removed, oauth2.clientSecret, oauth2.accessToken, oauth2.refreshToken, and nothing renamed or dropped. A non-credential key literally named token is preserved, so your scoping rule is respected.

Regression test in your style, shown red first: red on this PR's head, green with the change, and the rest at merge-base parity (with fix 2 failed / 575 passed, without 3 failed / 574 passed, merge-base 2 failed / 572 passed, same two pre-existing failures each time).

@pytest.mark.asyncio
async def test_trace_merged_tool_calls_redacts_credential_in_state_delta():
  span = mock.MagicMock()
  span.is_recording.return_value = True
  credential = AuthCredential(
      auth_type='oauth2',
      oauth2=OAuth2Auth(client_id='public-client-id',
                        client_secret='should-never-reach-the-trace'))
  merged_event = Event(
      invocation_id='inv-1', author='root_agent', id='merged-event-id',
      content=types.Content(role='user', parts=[types.Part(
          function_response=types.FunctionResponse(
              id='fc-1', name='connect_calendar',
              response={'status': 'connected'}))]),
      actions=EventActions(state_delta={
          'my_tool:oauth2:abcd1234': credential.model_dump(
              by_alias=True, exclude_none=True, mode='json')}))
  with mock.patch('opentelemetry.trace.get_current_span', return_value=span):
    trace_merged_tool_calls(response_event_id=merged_event.id,
                            function_response_event=merged_event)
  calls = [c for c in span.set_attribute.call_args_list
           if c.args[0] == 'gcp.vertex.agent.tool_response']
  assert len(calls) == 1
  serialized = calls[0].args[1]
  assert 'should-never-reach-the-trace' not in serialized
  # must not "redact" by blanking the attribute the UI renders
  assert 'public-client-id' in serialized
  assert 'connect_calendar' in serialized
  assert 'my_tool:oauth2:abcd1234' in serialized

4. Two traps I walked into, since they cost me more than the fix did

The obvious fix is wrong on whitespace. My first version used safe_json_serialize(...), which is json.dumps with default separators. It broke the existing test_trace_merged_tool_calls_sets_correct_attributes, purely on {"content": { versus {"content":{. Hence the explicit separators=(",", ":") above. Worth knowing if anyone else touches this attribute.

redact_credential_secrets is coupled to who serialized upstream, and this is not a defect in your PR. My first regression test put the raw AuthCredential object into state_delta. In that form both model_dump_json() and model_dump(mode="json") emit field names, so it is auth_type and client_secret, _is_credential_shaped looks for authType, returns False, and the redactor silently no-ops and returns the secret intact.

I chased that before reporting it, because it would have been a much bigger claim than it deserves. On the live path the value arrives as a by-alias dict: verified against a real runner pass that the stored value is byte-equal to model_dump(by_alias=True, exclude_none=True, mode='json'), carrying authType and clientSecret. So the shipped SessionStateCredentialService is fine and your redactor does fire. The only thing I would suggest is a sentence in the redact_credential_secrets docstring saying it keys on the by-alias spelling, since BaseCredentialService.save_credential is public and an implementation that stores the model object rather than its by-alias dump would get silent no-op redaction rather than a failure.

Scope of what I ran: tests/unittests/telemetry/ and tests/unittests/auth/ on py3.12.13, base-parity as above. I did not run tests/unittests/cli/test_fast_api.py locally, its optional google.cloud deps would not resolve in my sandbox, and my change does not touch that module. CI covers it.

[prasanna8585]

Thanks — implemented exactly as suggested, including both traps you flagged (verified separators=(",", ":") produces byte-identical output to the old model_dump_json() call for non-credential events, confirmed dynamically; added the by-alias docstring note). New regression test mutation-verified — fails on exactly one assertion when reverted, with the leaked secret visible in the diff. Full suite green at merge-base parity.