fix(adk): normalize ca_cert_path in GDCH SA JSON before token exchange#1901
Merged
EItanya merged 2 commits intoMay 22, 2026
Merged
Conversation
jjamroga
requested review from
EItanya,
iplay88keys,
jmhbh,
peterj,
supreme-gg-gg and
yuval-k
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR addresses GDCH service-account JSONs that embed a machine-local ca_cert_path, which google.oauth2.gdch_credentials can honor during STS token exchange and inadvertently override the agent’s intended TLS configuration inside the pod. The fix normalizes the in-memory SA JSON at refresh time so kagent’s ModelConfig TLS settings control the STS call behavior.
Changes:
- Load GDCH credentials from an in-memory, normalized SA JSON dict (rewrite/remove
ca_cert_path) before token exchange. - Ensure kagent TLS config takes precedence for STS verification behavior, including
tls_disable_verify. - Add unit tests covering normalization branches, on-disk JSON immutability, expiry handling, and token caching.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
python/packages/kagent-adk/src/kagent/adk/models/_token_source.py |
Normalizes ca_cert_path in GDCH SA JSON prior to STS refresh and switches to from_service_account_info for credential construction. |
python/packages/kagent-adk/tests/unittests/models/test_token_source.py |
Adds unit tests validating CA path normalization behavior, immutability of the SA file, expiry usage, and caching. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from google.auth.transport import requests as google_requests | ||
| from google.oauth2 import gdch_credentials | ||
|
|
||
| with open(self._sa_path) as f: |
Comment on lines
70
to
75
| session = requests.Session() | ||
| if self._tls_disable_verify: | ||
| session.verify = False | ||
| elif self._ca_cert_path: | ||
| # Replaces the REQUESTS_CA_BUNDLE environment variable | ||
| session.verify = self._ca_cert_path | ||
| creds.refresh(google_requests.Request(session=session)) |
The ca_cert_path field embedded in GDCH service-account JSON by `gdcloud iam service-accounts keys create` references a path on the machine where the key was generated, not a path that exists inside the agent pod. google.oauth2.gdch_credentials reads that field and uses it for TLS verification of the STS call, overriding the requests.Session verify setting — so token exchange fails before the request leaves the process. Make the kagent ModelConfig tls.* fields authoritative for the STS call's TLS configuration: - When tls.caCertSecretRef is set, rewrite the JSON's ca_cert_path to the mounted CA path (/etc/ssl/certs/custom/<key>). - When tls.disableVerify is set, strip ca_cert_path and disable verify. - Otherwise, strip ca_cert_path and fall back to system CAs. Load credentials from the mutated dict via gdch_credentials.ServiceAccountCredentials.from_service_account_info instead of google.auth.load_credentials_from_file. The on-disk SA JSON is not modified. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
jjamroga
force-pushed
the
fix/gdch-ca-cert-path-normalization
branch
from
3ad6979 to
e63a6be
Compare
- Pass `"r", encoding="utf-8"` to open() for the SA JSON, matching the rest of the package (adk/cli.py, adk/_token.py). - Use requests.Session as a context manager so adapters/connections are closed after each STS refresh. Test mock for the disable_verify case configures __enter__ to return the same mock instance so verify assignments inside the `with` block remain observable. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> Signed-off-by: Jonathan Jamroga <jjamroga@gmail.com>
EItanya
approved these changes
May 22, 2026
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
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.
Summary
gdcloud iam service-accounts keys createincludes aca_cert_pathpointing at the machine where the key was created — invalid inside the agent pod.google.oauth2.gdch_credentialshonors this field and overridesrequests.Session.verify, breaking token exchange.tls.*fields authoritative for the STS call: rewrite the JSON'sca_cert_pathto the kagent-mounted CA path when one is configured, strip it otherwise. The on-disk Secret is not modified.gdch_credentials.ServiceAccountCredentials.from_service_account_inforather thanload_credentials_from_file.Test plan
tests/unittests/models/test_token_source.py, 6 cases).uv run ruff checkclean on changed files.uv run pytest tests/unittests/models/— no regressions beyond pre-existingtest_tls_e2e.pyfixture failures unrelated to this change.tls.caCertSecretRefand a system-trusted STS CA, (b)tls.caCertSecretRefset to a private CA secret, (c)tls.disableVerify=trueagainst a self-signed STS.🤖 Generated with Claude Code