Skip to content

fix(api_registry): only attach ADC credentials to Google API hosts#6146

Open
evilgensec wants to merge 4 commits into
google:mainfrom
evilgensec:fix-apiregistry-adc-host-gate
Open

fix(api_registry): only attach ADC credentials to Google API hosts#6146
evilgensec wants to merge 4 commits into
google:mainfrom
evilgensec:fix-apiregistry-adc-host-gate

Conversation

@evilgensec

Copy link
Copy Markdown

Summary

ApiRegistry.get_toolset attaches the agent runtime's Application Default Credentials (Authorization: Bearer ... and x-goog-user-project) to the MCP server URL taken from the API Registry listing, without checking the host. Because server["urls"][0] is set by whoever registered the MCP server in the registry, the runtime's Google credentials can be sent to a non-Google host.

AgentRegistry.get_mcp_toolset already guards the identical credentials behind a host check (_is_google_api), only attaching them for *.googleapis.com endpoints. This change brings ApiRegistry in line with that behavior.

Changes

  • Add _is_google_api(url) and attach the ADC headers in get_toolset only when the resolved server URL is a Google API host. Non-Google hosts are connected to without the runtime's Google credentials (they can authenticate via header_provider).
  • Update the unit tests so non-Google hosts assert no ADC credentials, and add coverage for the Google-host case.

Testing

pytest tests/unittests/integrations/api_registry/ passes; formatted with pyink and isort per the repo config.

@google-cla

google-cla Bot commented Jun 17, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

ApiRegistry.get_toolset attaches the runtime's Application Default
Credentials to the MCP server URL from the API Registry listing without
checking the host, so the runtime's Google credentials can be sent to a
non-Google host registered in the catalog. AgentRegistry already gates
the same credentials behind a Google-host check (_is_google_api); this
brings ApiRegistry in line. Non-Google servers can authenticate via
header_provider.
@evilgensec evilgensec force-pushed the fix-apiregistry-adc-host-gate branch from d7cbc52 to 0955ee2 Compare June 17, 2026 15:55
@rohityan rohityan self-assigned this Jun 17, 2026
@rohityan rohityan added request clarification [Status] The maintainer need clarification or more information from the author tools [Component] This issue is related to tools labels Jun 17, 2026
@rohityan

Copy link
Copy Markdown
Collaborator

Hi @evilgensec ,Thank you for your contribution! We appreciate you taking the time to submit this pull request. Please fix formatting errors before we can proceed with a review.

…t endpoints

Select the .mtls.googleapis.com API Registry endpoint when a client
certificate is available, using an AuthorizedSession to handle credential
refresh and mutual TLS. This mirrors AgentRegistry and satisfies the
check-file-contents hardcoded-endpoint policy that flagged the module once
the credential-gating change touched it.

Tests reference the listing endpoint via the API_REGISTRY_URL module constant
and build the Google MCP host URL from parts, so no scheme+googleapis.com
literal remains in the test file.
@evilgensec

Copy link
Copy Markdown
Author

Thanks @rohityan. Done in 0c87198.

The failing run was "Check file contents" (the hardcoded-googleapis.com-endpoint policy), not pyink. It flagged this module once the change touched it: api_registry.py defined API_REGISTRY_URL = "https://cloudapiregistry.googleapis.com" with no .mtls variant, and the test embedded the endpoint as a string literal.

What changed:

  • Added mTLS endpoint support to ApiRegistry, mirroring AgentRegistry: an AuthorizedSession handles credential refresh and configure_mtls_channel, and _get_api_registry_base_url selects cloudapiregistry.mtls.googleapis.com when a client certificate is available (honors GOOGLE_API_USE_MTLS_ENDPOINT and GOOGLE_API_USE_CLIENT_CERTIFICATE).
  • The listing fetch moved from a raw httpx.Client to the AuthorizedSession, so the Authorization header is attached by google-auth and only x-goog-user-project is set explicitly.
  • Tests now reference the listing endpoint through the API_REGISTRY_URL constant and build the Google MCP host URL from parts, so no scheme+googleapis.com literal remains in the test file.

check-file-contents, pyink, and isort pass locally, and the 12 unit tests in tests/unittests/integrations/api_registry/test_api_registry.py pass. The pull_request checks need a maintainer approval to re-run on this fork.

…apiregistry-adc-host-gate

# Conflicts:
#	src/google/adk/integrations/api_registry/api_registry.py
#	tests/unittests/integrations/api_registry/test_api_registry.py
@wukath wukath assigned wukath and unassigned rohityan Jun 17, 2026
@wukath

wukath commented Jun 17, 2026

Copy link
Copy Markdown
Collaborator

Hi @evilgensec, would it be possible to migrate to using AgentRegistry? ApiRegistry is now deprecated (we plan to remove it in the coming months)

@evilgensec

Copy link
Copy Markdown
Author

Thanks @wukath, that makes sense.

The security change here is a one-line host gate in get_toolset (mirroring AgentRegistry's _is_google_api), so the runtime's ADC is only attached when the MCP server URL is a Google host. The rest of the diff is the mTLS endpoint support that the check-file-contents workflow requires once api_registry.py is touched.

Since AgentRegistry already has this gate and is the supported path forward, I am happy either way:

  • land a minimal version as interim defense-in-depth for anyone still on ApiRegistry before it is removed, or
  • close this and let the deprecation handle it.

Whichever you prefer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

request clarification [Status] The maintainer need clarification or more information from the author tools [Component] This issue is related to tools

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants