Activity

1. adk-bot

   added

   services[Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc[Component] This issue is related to runtime services, e.g. sessions, memory, artifacts, etc

   on Apr 2, 2026
2. adk-bot

   added the

   Feature

   issue type on Apr 2, 2026
3. caohy1988 commented on Apr 2, 2026

   

   caohy1988

   on Apr 2, 2026

   More actions

   Validation against latest main (9fec5030)

   Validated the claims in this issue against the current codebase. The core security concern is confirmed, with some additional findings and one design correction.

   ---

   1. Confirmed: sensitive credentials are persisted in session state

   • _google_credentials.py:207 writes creds.to_json() (containing token, refresh_token, client_id, client_secret) into tool_context.state["bigquery_token_cache"] — an unprefixed key.
   • SessionStateCredentialService (session_state_credential_service.py:81) writes full AuthCredential objects (with OAuth2Auth fields: access_token, refresh_token, client_secret, id_token) under auth_config.credential_key — also unprefixed.
   • extract_state_delta() (_session_util.py:48) routes all unprefixed keys into the persisted "session" bucket.

   This means all tokens and secrets are stored in plaintext in whatever session backend is configured (Vertex AI sessions, Spanner, SQLite, etc.). The issue author's curl output is an accurate reproduction.

   2. Confirmed: temp: only protects intermediate auth state

   • auth_handler.py:58 correctly uses "temp:" + credential_key during the OAuth exchange flow.
   • But SessionStateCredentialService re-stores the final exchanged credentials without the temp: prefix (session_state_credential_service.py:81), so they are persisted.

   3. Confirmed: BQAA redaction is incomplete

   The BQAA plugin redacts known sensitive dict keys (client_secret, access_token, refresh_token, id_token, api_key, password) via _recursive_smart_truncate() at bigquery_agent_analytics_plugin.py:269. However:

   • JSON blob gap: bigquery_token_cache is stored as a JSON-encoded string, not a nested dict. The redaction logic walks dict keys but does not parse opaque JSON strings, so secrets inside the blob pass through unredacted.
   • Arbitrary key gap: external_access_token_key (_google_credentials.py:78) is user-defined. If the token is stored under a custom key that doesn't match _SENSITIVE_KEYS, BQAA will log it in plaintext.

   4. Design correction: secret: scope is more than a prefix change

   The proposed secret: scope is directionally right but architecturally more complex than the temp: pattern it mirrors:

   • temp: works because it only needs to survive within a single invocation — it's applied to the in-memory session.state object (base_session_service.py:118) and trimmed before persistence (base_session_service.py:131). The same session object is reused across agents within one Runner.run_async() call.
   • A secret: scope that survives across turns but is never persisted would require a non-persistent, cross-turn cache in every session service implementation. Current get_session() paths rebuild state entirely from storage (database_session_service.py:537, vertex_ai_session_service.py:127), so secret-scoped values would be lost between turns unless an additional per-process cache (keyed by session identity) is introduced.
   • This is not a blocking concern — it's feasible — but the design RFC should explicitly call out that DB/Vertex-backed session services need a real cache layer, not just a filtering rule.

   Summary

   Finding	Severity	Status
   Credentials persisted in plaintext via session state	High	Confirmed
   temp: only covers intermediate auth, not final credentials	High	Confirmed
   BQAA redaction misses JSON blobs and arbitrary keys	High	Confirmed
   secret: scope needs cross-turn cache design for non-in-memory backends	Medium	Design nuance — should be addressed in RFC

   The issue is valid and well-motivated. For the implementation design, I'd suggest the RFC explicitly separates two concerns:

   1. Short-term mitigation: ensure BQAA redaction covers JSON-encoded blobs and arbitrary token keys (can be done independently).
   2. secret: scope: design the cross-turn, non-persistent cache model for DB/Vertex-backed session services before implementation, since it's not a straightforward extension of the temp: pattern.
4. caohy1988 commented on Apr 2, 2026

   

   caohy1988

   on Apr 2, 2026

   Last edited by caohy1988

   More actions

   RFC: Non-Persistent secret: Session State Scope

   Summary

   This RFC proposes adding a new secret: session state scope to ADK that is never persisted to storage and never logged by built-in analytics integrations (such as BQAA), but survives across turns within the same process. This addresses a confirmed security gap where OAuth credentials (tokens, client secrets, refresh tokens) are stored in plaintext in session backends and can leak into BigQuery Agent Analytics logs.

   Motivation

   Today, ADK stores OAuth credentials in session state under unprefixed keys (e.g., bigquery_token_cache). Because unprefixed keys are routed to the persisted "session" bucket by extract_state_delta(), these secrets end up in plaintext in whatever session backend is configured — Vertex AI sessions, Spanner, SQLite, or in-memory storage.

   This creates two concrete problems:

   • Persistence exposure: Full credential JSON (containing access_token, refresh_token, client_id, client_secret) is stored in session backends. Any reader of the session store can extract valid tokens. See #5112 for a reproduction showing credentials in Vertex AI session state and BigQuery analytics.
   • Analytics leakage: The BQAA plugin logs session state when log_session_metadata=True (the default). Its _SENSITIVE_KEYS redaction catches nested dict keys like client_secret, but misses:
     • Opaque JSON blobs — bigquery_token_cache is a JSON-encoded string, not a nested dict, so secrets inside it pass through unredacted.
     • Arbitrary user-defined keys — external_access_token_key is user-defined and won't match _SENSITIVE_KEYS unless the key name happens to collide.

   The existing temp: scope avoids persistence but only survives within a single invocation — it is trimmed from event.actions.state_delta before storage and lost when the session is re-fetched. This makes it unsuitable for credentials that need to survive across multiple conversational turns.

   Evidence (current main)

   Path	What happens
   tools/_google_credentials.py:207	creds.to_json() written to state["bigquery_token_cache"]
   tools/bigquery/bigquery_credentials.py:21	Key hardcoded as "bigquery_token_cache" (no prefix)
   auth/credential_service/session_state_credential_service.py:81	Full AuthCredential written to state[credential_key] (no prefix)
   sessions/_session_util.py:48	Unprefixed keys routed to persisted "session" bucket
   auth/auth_handler.py:58	Intermediate auth uses "temp:" prefix (correct)
   auth/credential_service/session_state_credential_service.py:81	Final save drops the "temp:" prefix (credentials persisted)
   plugins/bigquery_agent_analytics_plugin.py:269	Redacts dict keys in _SENSITIVE_KEYS, but not JSON string blobs

   Proposal

   Add a new secret: state scope with these semantics:

   Property	app:	user:	(session)	temp:	secret:
   Prefix	app:	user:	(none)	temp:	secret:
   Persisted	Yes	Yes	Yes	No	No
   Logged (BQAA)	Yes	Yes	Yes	Redacted	Redacted
   Lifetime	Permanent	Permanent	Permanent	Single invocation	Process-lifetime
   Survives across turns	Yes	Yes	Yes	No	Yes (same process)
   Survives process restart	Yes	Yes	Yes	No	No

   Key Design Decision: Process-Lifetime Cache

   Unlike temp:, which piggybacks on the in-memory session object within a single Runner.run_async() call, secret: state must survive across turns. This means it cannot simply be trimmed from events and forgotten — it needs a non-persistent, process-scoped cache in each session service.

   Current get_session() implementations rebuild state entirely from storage:

   • database_session_service.py:537 — queries DB
   • vertex_ai_session_service.py:127 — calls Vertex AI API

   After a round-trip through storage, secret:-scoped values would be lost unless they are held in a separate in-process cache.

   Cache Design

   The cache is a process-local dict keyed by (app_name, user_id, session_id) → dict[str, Any] for secret-scoped state.

   Initialization constraint: BaseSessionService does not define __init__(), and concrete session services (InMemorySessionService, DatabaseSessionService, VertexAiSessionService) define their own constructors without calling super().__init__(). To avoid requiring all subclasses (including third-party implementations) to change their constructors, the cache should be exposed as a lazy property on BaseSessionService:

   class BaseSessionService(abc.ABC):
   
       @property
       def _secret_state_cache(self) -> dict[tuple[str, str, str], dict[str, Any]]:
           """Process-local cache for secret-scoped state.
   
           Keyed by (app_name, user_id, session_id).
           Lazily initialized to avoid requiring subclasses to call
           super().__init__().
           """
           try:
               return self.__secret_state_cache
           except AttributeError:
               self.__secret_state_cache: dict[
                   tuple[str, str, str], dict[str, Any]
               ] = {}
               return self.__secret_state_cache

   This is fully backward-compatible — no existing constructor needs to change.

   The cache must be touched on all session lifecycle paths:

   • append_event: secret-scoped keys are applied to the cache (like _apply_temp_state) and trimmed from the event delta (like _trim_temp_delta_state).
   • create_session: if initial state contains secret:-prefixed keys, they must be seeded into the cache and preserved in the returned in-memory Session.state, but excluded from the persisted state sent to storage. Without this, create_session(state={"secret:token": "..."}) would silently drop the secret keys once extract_state_delta() filters them.
   • get_session: after loading persisted state, merge in any cached secret state for that session identity. This is the only read path that restores secret: state — all other read paths (list_sessions) do not.
   • list_sessions: secret state is not merged into listed sessions. list_sessions is a read-only summary operation; injecting process-local secret state would produce inconsistent results across processes and add overhead for a bulk operation that rarely needs credential access. Callers that need secret state should use get_session on individual sessions.
   • delete_session: evict the cache entry. This must be implemented in every built-in session service: InMemorySessionService, DatabaseSessionService, VertexAiSessionService, and SqliteSessionService.
   • On process restart: cache is empty — credentials must be re-acquired. This is an intentional tradeoff.

   Changes

   Phase 1: Core scope + BQAA hardening (short-term)

   Phase 1 establishes the new secret: scope in the framework and hardens analytics redaction. It does not by itself fix the credential persistence bug — existing credential writers still use unprefixed keys until Phase 2 migrates them.

   1a. Add secret: prefix to State

   File: sessions/state.py
   

   class State:
       APP_PREFIX = "app:"
       USER_PREFIX = "user:"
       TEMP_PREFIX = "temp:"
       SECRET_PREFIX = "secret:"  # new

   1b. Filter secret: keys from persistence

   File: sessions/_session_util.py
   

   Update extract_state_delta() to exclude secret: keys alongside temp::

   elif not key.startswith(State.TEMP_PREFIX) and not key.startswith(State.SECRET_PREFIX):
       deltas["session"][key] = state[key]

   1c. Add secret-state cache and lifecycle methods in BaseSessionService

   File: sessions/base_session_service.py
   

   Add the lazy _secret_state_cache property (see Cache Design above), then add lifecycle methods:

   def _apply_secret_state(self, session: Session, event: Event) -> None:
       """Applies secret-scoped state to in-memory session and process cache."""
       if not event.actions or not event.actions.state_delta:
           return
       cache_key = (session.app_name, session.user_id, session.id)
       for key, value in event.actions.state_delta.items():
           if key.startswith(State.SECRET_PREFIX):
               session.state[key] = value
               self._secret_state_cache.setdefault(cache_key, {})[key] = value
   
   def _trim_secret_delta_state(self, event: Event) -> Event:
       """Removes secret-scoped keys from event delta before persistence."""
       if not event.actions or not event.actions.state_delta:
           return event
       event.actions.state_delta = {
           key: value
           for key, value in event.actions.state_delta.items()
           if not key.startswith(State.SECRET_PREFIX)
       }
       return event
   
   def _seed_secret_state_on_create(
       self,
       *,
       app_name: str,
       user_id: str,
       session_id: str,
       state: dict[str, Any] | None,
   ) -> dict[str, Any] | None:
       """Extracts secret-scoped keys from initial state into the cache.
   
       Returns the state dict with secret keys removed (for persistence)
       but seeds them in the process-local cache so get_session() can
       restore them.
       """
       if not state:
           return state
       secret_keys = {
           k: v for k, v in state.items()
           if k.startswith(State.SECRET_PREFIX)
       }
       if secret_keys:
           cache_key = (app_name, user_id, session_id)
           self._secret_state_cache.setdefault(cache_key, {}).update(
               secret_keys
           )
       # Return state without secret keys for persistence
       return {
           k: v for k, v in state.items()
           if not k.startswith(State.SECRET_PREFIX)
       }
   
   def _restore_secret_state(self, session: Session) -> None:
       """Merges cached secret state into an in-memory session."""
       cache_key = (session.app_name, session.user_id, session.id)
       secret_state = self._secret_state_cache.get(cache_key, {})
       for key, value in secret_state.items():
           session.state[key] = value
   
   def _evict_secret_state(
       self, app_name: str, user_id: str, session_id: str
   ) -> None:
       """Removes cached secret state for a deleted session."""
       cache_key = (app_name, user_id, session_id)
       self._secret_state_cache.pop(cache_key, None)

   Update append_event() to call _apply_secret_state and _trim_secret_delta_state before persistence.

   1d. Update session service implementations

   Each concrete session service must:

   • Call _seed_secret_state_on_create() in create_session() before persisting state, and include the secret keys in the returned Session.state.
   • Call _restore_secret_state() in get_session() after loading from storage.
   • Call _evict_secret_state() in delete_session().
   • Not restore secret state in list_sessions() (see rationale above).

   Affected services:

   • InMemorySessionService
   • DatabaseSessionService
   • VertexAiSessionService
   • SqliteSessionService

   1e. Add secret: to validation

   File: utils/instructions_utils.py
   

   Add State.SECRET_PREFIX to the valid prefixes list.

   1f. BQAA: redact secret: keys and harden JSON blob handling

   File: plugins/bigquery_agent_analytics_plugin.py
   

   • Add key.startswith("secret:") to the redaction check alongside "temp:".
   • Add JSON-string detection: if a value is a string that parses as JSON containing any _SENSITIVE_KEYS, redact the entire value.

   Phase 2: Migrate credential storage (follow-up)

   Once the secret: scope is available, migrate existing credential paths to use it. This is where the credential persistence bug is actually eliminated.

   2a. BigQuery token cache

   File: tools/bigquery/bigquery_credentials.py
   

   Change:

   BIGQUERY_TOKEN_CACHE_KEY = "bigquery_token_cache"

   To:

   BIGQUERY_TOKEN_CACHE_KEY = "secret:bigquery_token_cache"

   2b. SessionStateCredentialService

   File: auth/credential_service/session_state_credential_service.py
   

   Both the save and load paths must be migrated. The load path (load_credential) currently reads from state[credential_key] — after migration it must read from state[State.SECRET_PREFIX + credential_key]. If only the save path is updated, the service will write to secret:credential_key but look up the old unprefixed key, breaking credential retrieval silently.

   Save:

   callback_context.state[State.SECRET_PREFIX + auth_config.credential_key] = (
       auth_config.exchanged_auth_credential
   )

   Load:

   return callback_context.state.get(State.SECRET_PREFIX + auth_config.credential_key)

   Backward Compatibility

   • Default behavior unchanged. Existing unprefixed keys continue to work as session-scoped, persisted state.
   • Phase 2 is a breaking change for credential storage. After migrating bigquery_token_cache to secret:bigquery_token_cache, existing sessions with cached credentials under the old key will require re-authentication. This should be called out in release notes.
   • No API changes. Developers use the new scope by prefixing keys with secret: — same pattern as app:, user:, temp:.
   • No constructor changes required for existing subclasses. The lazy property pattern means third-party BaseSessionService subclasses continue to work without modification.

   Side Effects and Tradeoffs

   • Process restart clears secret state. In stateless/distributed deployments (e.g., Cloud Run with scale-to-zero), secret:-scoped credentials are lost on restart, requiring re-authentication. This is the intentional security tradeoff. Sticky sessions can mitigate this. A pluggable secret storage backend is a potential future extension (see Future Direction).
   • Per-process memory cost. The secret state cache grows with the number of active sessions. For most deployments this is negligible (one small dict entry per session), but should be documented.
   • No cross-process sharing. Two processes serving the same session will have independent secret caches. This is acceptable — credentials can be re-acquired — but should be documented.
   • list_sessions() does not include secret state. This is intentional — secret state is process-local and would produce inconsistent results across processes. Callers needing secret state should use get_session().

   Alternatives Considered

   • Encrypt credentials before persisting. Would address the storage exposure but adds key management complexity and doesn't solve the analytics leakage. Also, encrypted blobs in session state are opaque to developers debugging state issues.
   • Extend temp: to survive across turns. Would change existing temp: semantics and break code that depends on invocation-scoped cleanup.
   • BQAA-only fix (redaction hardening). Would address analytics leakage but not the storage exposure. Treats the symptom, not the cause.
   • Per-key sensitivity annotation. More flexible but requires API changes (e.g., state.set("key", value, sensitive=True)) rather than the simpler prefix convention already established by ADK.
   • Constructor-based cache initialization. Would require all BaseSessionService subclasses (including third-party) to call super().__init__(). Rejected in favor of a lazy property that is backward-compatible.

   Future Direction: Pluggable Secret Storage

   This RFC deliberately scopes secret: state as non-persistent and process-local. For stateless/distributed deployments where process restart is frequent (Cloud Run scale-to-zero, multi-replica Kubernetes), this means re-authentication on every cold start.

   A natural follow-up — likely as a separate RFC — would introduce an optional SecretStateService interface allowing customers to bring their own secret manager (e.g., Google Cloud Secret Manager, AWS Secrets Manager, HashiCorp Vault). That design has several open questions that are out of scope here:

   • Sync/async boundary. Phase 1's _restore_secret_state() and _apply_secret_state() are synchronous helpers. An external backend requires async I/O, so the helper signatures or call sites would need to change.
   • Consistency. Session state and secret state would live in separate stores with no cross-store transaction guarantee. Failures during append_event() or delete_session() could orphan or diverge secret state.
   • Existing client gaps. The repo's SecretManagerClient (integrations/secret_manager/secret_client.py) currently exposes only get_secret() — a full backend would need write and delete operations, not just a thin wrapper.

   These are tractable problems, but they deserve their own design discussion rather than being bolted onto this security-focused RFC.

   Timeline

   • Phase 1: Core secret: scope + BQAA hardening + session service support — can ship as one coordinated change. Establishes the framework but does not yet fix credential persistence.
   • Phase 2: Migrate credential storage to secret: prefix — follow-up PR that eliminates the persistence bug. Requires release note for re-auth impact.
   • Phase 3: Documentation, developer guide for using secret: scope with custom sensitive data.

   Outcome

   • OAuth credentials and tokens are no longer persisted in session backends (after Phase 2).
   • BQAA analytics logs no longer leak secrets via JSON blobs or arbitrary keys (after Phase 1). Note: this covers the built-in BQAA plugin specifically; third-party or future plugins must independently respect the secret: prefix contract.
   • Developers get a general-purpose secret: scope for any sensitive data (credit cards, API keys, PII) that needs to survive across turns without persistence.
   • Existing code continues to work unchanged until explicitly migrated.

   👍React with 👍2Reacted by guillaume blaquiere and Gwendal Le Cren
5. caohy1988

   added 4 commits that reference this issue on Apr 4, 2026

   feat(sessions): add `secret:` session state scope (Phase 1)

   ...

   This issue will close once commit ce8d2a3 is merged into the 'main' branch.

   ce8d2a3

   fix(auth): migrate credential storage to secret: scope (Phase 2)

   ...

   This issue will close once commit 2b46204 is merged into the 'main' branch.

   2b46204

   fix(auth): migrate credential storage to secret: scope (Phase 2)

   ...

   This issue will close once commit 0f7856d is merged into the 'main' branch.

   0f7856d

   fix(auth): migrate credential storage to secret: scope (Phase 2)

   ...

   This issue will close once commit 0d01bf7 is merged into the 'main' branch.

   0d01bf7
6. timof1308 commented on Apr 6, 2026

   

   timof1308

   on Apr 6, 2026

   More actions

   hey, want to flag #3675 as it adds request_state which is related to this
   It's a per-request ephemeral overlay on session.state that never gets persisted and doesn't survive across turns

   Maybe this is complementary to secret: rather than overlapping: request_state is for short-lived tokens the client sends with each request (JWTs), while secret is for credentials that stick around across turns (OAuth refresh tokens). Both avoid persistence, just with different lifetimes.
7. adk-bot

   added

   auth[Component] This issue is related to authorization[Component] This issue is related to authorization

   on Apr 10, 2026
8. adk-bot

   assigned

   klateefa

   on Apr 10, 2026
9. haiyuan-eng-google commented on Apr 21, 2026

   

   haiyuan-eng-google

   on Apr 21, 2026

   Collaborator

   More actions

   for

   Lifetime	Permanent	Permanent	Permanent	Single invocation	Process-lifetime

   comment is: each turn can be sent to a different server, how do you achieve this without persisting?
10. caohy1988 commented on Apr 21, 2026

    

    caohy1988

    on Apr 21, 2026

    More actions

    Good question — this is the fundamental tradeoff the secret: scope makes explicit.

    Short answer

    You can't have all three of (non-persistent) + (survives across turns) + (works across arbitrary servers) simultaneously. The secret: scope chooses the first two and acknowledges the third as a deployment constraint.

    How it works in practice

    Deployment model	Behavior	Mitigation
    Single process (dev, adk web)	Works — secret state lives in process memory across turns	None needed
    Sticky sessions (Cloud Run session affinity, k8s sessionAffinity: ClientIP)	Works — same pod handles all turns for a session	Configure session affinity at the load balancer
    Stateless / scale-to-zero (Cloud Run default, multi-replica without affinity)	Secret state lost on cold start or routing to a new pod	Re-authentication triggered; user re-enters OAuth flow

    The re-authentication fallback is intentional — it's what happens today with temp:-scoped auth state anyway. The difference is secret: survives across turns within the same process, so for sticky deployments or warm instances, users don't re-auth every single turn.

    Why not encrypt-and-persist?

    It's a valid alternative, but trades one set of concerns for another:

    • Key management complexity (who provisions/rotates encryption keys?)
    • Encrypted blobs in session state are opaque for debugging
    • Doesn't solve analytics leakage unless combined with redaction anyway
    • Broader blast radius if the key is compromised (all sessions, all time) vs. process-local (one pod, only while running)

    Future Direction: Pluggable SecretStateService

    The RFC's "Future Direction" section outlines the proper solution for stateless distributed deployments: an optional SecretStateService interface where customers bring their own secure backend (Secret Manager, Vault, encrypted Redis). That design has open questions (async I/O in the restore path, cross-store consistency, lifecycle management) that warrant their own RFC rather than being bolted onto the core secret: scope.

    The shipped Phase 1 + Phase 2 gives a working, secure-by-default solution for the majority of ADK deployments today (single process + sticky sessions). The pluggable backend extends it to fully stateless architectures as a follow-up.
11. DeanChensj commented on Oct 7, 2026

    

    DeanChensj

    on Oct 7, 2026

    Collaborator

    More actions

    Hi @guillaumeblaquiere, thank you so much for raising this detailed security report and for your thoughtful proposal!

    We have addressed the underlying credential persistence and observability exposure issues across the codebase through three complementary mechanisms (which also avoid the sticky-session limitation in distributed deployments):

    1. Cloud KMS Envelope Encryption at Rest (KmsEncryptedCredentials): BaseGoogleCredentialsConfig (inherited by BigQueryCredentialsConfig, SpannerCredentialsConfig, GcsCredentialsConfig, etc.) now supports a kms_key_name parameter (or the GOOGLE_CREDENTIAL_KMS_KEY environment variable) via Encrypt sensitive credential in session state with KMS #6614. When configured, OAuth credentials cached in session.state (such as bigquery_token_cache) are automatically envelope-encrypted using Cloud KMS and Fernet DEKs before being written to session state.
    2. Automatic Secret Redaction in BigQueryAgentAnalyticsPlugin (BQAA): The BQAA plugin (fix(plugins): close BQAA fail-open privacy, GCS concurrency, and startup-loss gaps (#6356) #6360) now automatically sanitizes and redacts sensitive keys (client_secret, access_token, refresh_token, id_token, token, secret, api_key, password, private_key, authorization, as well as any temp:* keys) to [REDACTED] across both structured mappings and serialized JSON payloads before logging to BigQuery.
    3. Auth Payload Redaction in Telemetry & API Server: _redact_credential_secrets in google.adk.auth.auth_credential automatically strips credential secrets from adk_request_credential events, requested_auth_configs, OpenTelemetry spans, and API server responses.

    I am closing this issue as resolved. If you run into any edge cases or have additional requirements around credential protection, please feel free to open a new issue or PR!
12. DeanChensj

    closed this as completedon Oct 7, 2026
13. guillaumeblaquiere commented on Oct 7, 2026

    

    guillaumeblaquiere

    on Oct 7, 2026

    ContributorAuthor

    More actions

    @DeanChensj Thank you for the follow up and the different step to improve security and trust in agent (and having accepted my PR).