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

src/google/adk/auth/auth_credential.py

@@ -30,6 +30,133 @@
 
 _REDACTED = "<redacted>"
 
+# By-alias (camelCase) names of every field on a credential model that is
+# marked `repr=False` above. `repr=False` only redacts these from Python's
+# `repr()`/`str()` (logs, error strings); it has no effect on
+# `model_dump()`/`model_dump_json()`, which is what actually goes out over
+# the network (e.g. FastAPI's /run, /run_sse, /run_live responses). Anything
+# serializing an `AuthCredential`-derived object for an external, untrusted
+# client -- as opposed to internal persistence via SessionService -- must
+# additionally strip these keys. Kept as one set here, next to the field
+# declarations, so the two lists can't silently drift apart.
+CREDENTIAL_SECRET_KEYS = frozenset({
+    "password",
+    "token",
+    "additionalHeaders",
+    "clientSecret",
+    "authResponseUri",
+    "authCode",
+    "accessToken",
+    "refreshToken",
+    "idToken",
+    "codeVerifier",
+    "privateKeyId",
+    "privateKey",
+    "apiKey",
+})
+
+
+def _is_credential_shaped(value: Any) -> bool:
+  """Reports whether `value` is a dict that is itself an `AuthCredential`.
+
+  `authType` is `AuthCredential.auth_type`'s by-alias serialized name, and
+  every `AuthCredential` dump carries it -- including one parked directly
+  in session state by `SessionStateCredentialService` under an
+  app-or-tool-chosen key, which is why this checks shape rather than a
+  fixed set of container key names like `authConfig`: a credential can
+  legitimately appear anywhere a `dict[str, Any]` the app controls can
+  hold one, not only nested under the names ADK's own code happens to use.
+  """
+  return isinstance(value, dict) and "authType" in value
+
+
+def _strip_secret_keys(value: Any) -> Any:
+  """Unconditionally strips `CREDENTIAL_SECRET_KEYS` from every dict in
+  `value`, at any depth.
+
+  Only safe to call once a `_is_credential_shaped` dict has already been
+  found: everything below that point genuinely belongs to a credential
+  object, so a key named e.g. `token` here is actually a secret, not
+  coincidentally-named application data.
+  """
+  if isinstance(value, dict):
+    return {
+        key: _strip_secret_keys(val)
+        for key, val in value.items()
+        if key not in CREDENTIAL_SECRET_KEYS
+    }
+  if isinstance(value, list):
+    return [_strip_secret_keys(item) for item in value]
+  return value
+
+
+def redact_credential_secrets(value: Any) -> Any:
+  """Recursively strips credential-secret keys from a JSON-like value.
+
+  An event's `function_call.args` (e.g. the `adk_request_credential` call
+  ADK issues when a tool needs OAuth) is an opaque `dict[str, Any]`, not a
+  nested pydantic model -- so `Event.model_dump(exclude=...)` cannot reach
+  a secret embedded inside it by field path. This walks the already-dumped
+  event unconditionally, so a credential's `client_secret`, tokens, etc.
+  never reach an external client over /run, /run_sse, /run_live, any
+  endpoint that returns session history or eval data, or a trace/span
+  attribute. This must only be applied to the outbound wire representation
+  -- never to what SessionService persists -- since later turns
+  reconstruct the original request's credential by re-parsing the
+  persisted `adk_request_credential` call's args.
+
+  Stripping is scoped to dicts that are actually `AuthCredential` dumps
+  (identified by carrying `authType`, see `_is_credential_shaped`), not to
+  every `CREDENTIAL_SECRET_KEYS` name anywhere in the structure. Several of
+  those names -- `token`, `password`, `apiKey`, `accessToken` among them --
+  are ordinary words a tool's own return value or an app's own session
+  state can legitimately use for something that isn't a credential at all
+  (a pagination cursor named `token`, a scraped page's own `password`
+  field). Deleting those unconditionally would silently drop data the
+  caller never asked to have redacted, indistinguishable from a key a tool
+  simply never returned. Scoping to credential-shaped subtrees avoids that
+  while still catching every real credential location, including one
+  stored in session state under an arbitrary key -- that dump still
+  carries `authType`, so shape-based detection finds it without needing a
+  fixed list of container key names.
+
+  This function is exported publicly (not module-private) because it is
+  shared across api_server.py, dev_server.py, and telemetry/tracing.py --
+  every place a credential-bearing structure reaches an external client or
+  an exported trace attribute needs the same redaction, and a fixed list
+  of container key names would need updating at every one of those call
+  sites for a new credential-bearing structure; shape-based detection does
+  not.
+
+  Note one thing this does *not* reach: a value that is already a JSON
+  *string* by the time this runs (e.g. an OpenTelemetry span attribute
+  built by json-serializing a dict first) is opaque to this walker, which
+  only descends real dicts and lists. Call this before serializing to a
+  string, not after.
+
+  `_is_credential_shaped` keys on the by-alias field spelling
+  (`authType`, not `auth_type`), since that's what every dump this
+  function is actually called on carries in practice -- an
+  outbound-facing dict is by-alias by construction (matching the wire
+  format), and a dict parked in session state by
+  `SessionStateCredentialService.save_credential` is too, verified
+  against a real end-to-end run to be byte-equal to
+  `AuthCredential.model_dump(by_alias=True, exclude_none=True,
+  mode="json")`. `BaseCredentialService.save_credential` is a public
+  extension point, though, and a different implementation that stores
+  the model object itself, or dumps it without `by_alias=True`, would
+  get a silent no-op here rather than an error: `_is_credential_shaped`
+  would find no `authType` key, conclude the dict isn't a credential,
+  and leave it untouched.
+  """
+  if _is_credential_shaped(value):
+    return _strip_secret_keys(value)
+  if isinstance(value, dict):
+    return {key: redact_credential_secrets(val) for key, val in value.items()}
+  if isinstance(value, list):
+    return [redact_credential_secrets(item) for item in value]
+  return value
+
 
 # Pydantic echoes the rejected value into ValidationError messages
 # ("input_value=..."), which would put a malformed secret straight into logs and

src/google/adk/cli/api_server.py

@@ -46,6 +46,7 @@
 from fastapi import Request
 from fastapi import Response
 from fastapi.middleware.cors import CORSMiddleware
+from fastapi.responses import JSONResponse
 from fastapi.responses import RedirectResponse
 from fastapi.responses import StreamingResponse
 from fastapi.staticfiles import StaticFiles
@@ -77,6 +78,8 @@
 from ..errors.already_exists_error import AlreadyExistsError
 from ..errors.input_validation_error import InputValidationError
 from ..errors.session_not_found_error import SessionNotFoundError
+from ..auth.auth_credential import CREDENTIAL_SECRET_KEYS
+from ..auth.auth_credential import redact_credential_secrets as _redact_credential_secrets
 from ..events.event import Event
 from ..events.event_actions import EventActions
 from ..flows.llm_flows.functions import REQUEST_CONFIRMATION_FUNCTION_CALL_NAME
@@ -574,6 +577,35 @@ def _invalid_event_error(event_index: int, disallowed: str) -> HTTPException:
   )
 
 
+def _redacted_session_response(session: Session) -> JSONResponse:
+  """Returns a `Session` as a `JSONResponse` with credential secrets stripped.
+
+  A session's persisted events may include an `adk_request_credential`
+  function call carrying a tool's full `AuthCredential` -- SessionService
+  intentionally retains the secret there so a later turn can recover it via
+  the credential merge-backfill mechanism, which means every endpoint that
+  returns a `Session` (not just /run, /run_sse, /run_live) must redact it
+  before it reaches an external client.
+  """
+  return JSONResponse(
+      content=_redact_credential_secrets(
+          session.model_dump(exclude_none=True, by_alias=True, mode="json")
+      )
+  )
+
+
+def _redacted_sessions_response(sessions: list[Session]) -> JSONResponse:
+  """Same as `_redacted_session_response`, for a list of sessions."""
+  return JSONResponse(
+      content=[
+          _redact_credential_secrets(
+              session.model_dump(exclude_none=True, by_alias=True, mode="json")
+          )
+          for session in sessions
+      ]
+  )
+
+
 def _validate_session_initialization_events(events: list[Event]) -> None:
   """Rejects client-supplied events that claim to be ADK-generated.
 
@@ -1452,36 +1484,39 @@ async def get_adk_app_info(app_name: str) -> AppInfo:
 
     @app.get(
         "/apps/{app_name}/users/{user_id}/sessions/{session_id}",
+        response_model=Session,
         response_model_exclude_none=True,
     )
     async def get_session(
         app_name: str, user_id: str, session_id: str
-    ) -> Session:
+    ) -> Response:
       session = await self.session_service.get_session(
           app_name=app_name, user_id=user_id, session_id=session_id
       )
       if not session:
         raise HTTPException(status_code=404, detail="Session not found")
       self.current_app_name_ref.value = app_name
-      return session
+      return _redacted_session_response(session)
 
     @app.get(
         "/apps/{app_name}/users/{user_id}/sessions",
+        response_model=list[Session],
         response_model_exclude_none=True,
     )
-    async def list_sessions(app_name: str, user_id: str) -> list[Session]:
+    async def list_sessions(app_name: str, user_id: str) -> Response:
       list_sessions_response = await self.session_service.list_sessions(
           app_name=app_name, user_id=user_id
       )
-      return [
+      return _redacted_sessions_response([
           session
           for session in list_sessions_response.sessions
           # Remove sessions that were generated as a part of Eval.
           if not session.id.startswith(EVAL_SESSION_ID_PREFIX)
-      ]
+      ])
 
     @app.post(
         "/apps/{app_name}/users/{user_id}/sessions/{session_id}",
+        response_model=Session,
         response_model_exclude_none=True,
     )
     @deprecated(
@@ -1493,25 +1528,28 @@ async def create_session_with_id(
         user_id: str,
         session_id: str,
         state: Optional[dict[str, Any]] = None,
-    ) -> Session:
-      return await self._create_session(
+    ) -> Response:
+      session = await self._create_session(
           app_name=app_name,
           user_id=user_id,
           state=state,
           session_id=session_id,
       )
+      return _redacted_session_response(session)
 
     @app.post(
         "/apps/{app_name}/users/{user_id}/sessions",
+        response_model=Session,
         response_model_exclude_none=True,
     )
     async def create_session(
         app_name: str,
         user_id: str,
         req: Optional[CreateSessionRequest] = None,
-    ) -> Session:
+    ) -> Response:
       if not req:
-        return await self._create_session(app_name=app_name, user_id=user_id)
+        session = await self._create_session(app_name=app_name, user_id=user_id)
+        return _redacted_session_response(session)
 
       if req.events:
         _validate_session_initialization_events(req.events)
@@ -1527,7 +1565,7 @@ async def create_session(
         for event in req.events:
           await self.session_service.append_event(session=session, event=event)
 
-      return session
+      return _redacted_session_response(session)
 
     @app.delete("/apps/{app_name}/users/{user_id}/sessions/{session_id}")
     async def delete_session(
@@ -1539,14 +1577,15 @@ async def delete_session(
 
     @app.patch(
         "/apps/{app_name}/users/{user_id}/sessions/{session_id}",
+        response_model=Session,
         response_model_exclude_none=True,
     )
     async def update_session(
         app_name: str,
         user_id: str,
         session_id: str,
         req: UpdateSessionRequest,
-    ) -> Session:
+    ) -> Response:
       """Updates session state without running the agent.
 
       Args:
@@ -1585,7 +1624,7 @@ async def update_session(
           session=session, event=state_update_event
       )
 
-      return session
+      return _redacted_session_response(session)
 
     @app.get(
         "/apps/{app_name}/users/{user_id}/sessions/{session_id}/artifacts/{artifact_name:path}/versions/{version_id}/metadata",
@@ -1825,8 +1864,10 @@ def _set_telemetry_context_if_needed(runner: Runner):
       else:
         _is_visual_builder.set(False)
 
-    @app.post("/run", response_model_exclude_none=True)
-    async def run_agent(req: RunAgentRequest, request: Request) -> list[Event]:
+    @app.post(
+        "/run", response_model=list[Event], response_model_exclude_none=True
+    )
+    async def run_agent(req: RunAgentRequest, request: Request) -> Response:
       app_name = req.app_name or self.default_app_name
       if not app_name:
         raise HTTPException(
@@ -1883,7 +1924,14 @@ async def monitor():
         events = await worker_task
         logger.info("Generated %s events in agent run", len(events))
         logger.debug("Events generated: %s", events)
-        return events
+        return JSONResponse(
+            content=[
+                _redact_credential_secrets(
+                    event.model_dump(exclude_none=True, by_alias=True, mode="json")
+                )
+                for event in events
+            ]
+        )
       except asyncio.CancelledError:
         if await request.is_disconnected():
           return Response(status_code=499)
@@ -1961,9 +2009,14 @@ async def event_generator():
                   events_to_stream = [content_event, artifact_event]
 
                 for event_to_stream in events_to_stream:
-                  sse_event = event_to_stream.model_dump_json(
-                      exclude_none=True,
-                      by_alias=True,
+                  sse_event = json.dumps(
+                      _redact_credential_secrets(
+                          event_to_stream.model_dump(
+                              exclude_none=True,
+                              by_alias=True,
+                              mode="json",
+                          )
+                      )
                   )
                   logger.debug(
                       "Generated event in agent run streaming: %s", sse_event
@@ -2101,7 +2154,13 @@ async def forward_events():
         ) as agen:
           async for event in agen:
             await websocket.send_text(
-                event.model_dump_json(exclude_none=True, by_alias=True)
+                json.dumps(
+                    _redact_credential_secrets(
+                        event.model_dump(
+                            exclude_none=True, by_alias=True, mode="json"
+                        )
+                    )
+                )
             )
 
       async def process_messages():