ci(check-file-contents): exclude OAuth scope URLs from endpoint scan#5931
Closed
caohy1988 wants to merge 1 commit into
Closed
ci(check-file-contents): exclude OAuth scope URLs from endpoint scan#5931caohy1988 wants to merge 1 commit into
caohy1988 wants to merge 1 commit into
Conversation
) The "Check for hardcoded googleapis.com endpoints" step in .github/workflows/check-file-contents.yml uses grep -lE 'https?://[a-zA-Z0-9.-]+\.googleapis\.com' to find files that should also declare an `.mtls.googleapis.com` counterpart for dynamic endpoint selection. The regex matches any googleapis.com URL — including OAuth 2.0 scope URLs like https://www.googleapis.com/auth/cloud-platform and .../auth/bigquery — which are identity strings, not API endpoints. They don't have mTLS counterparts and never will. Any file that legitimately declares an OAuth scope (very common for ADK plugins integrating Google APIs) trips the gate even when no real endpoint is hardcoded. Fix: add a second pass that filters the candidate set down to files that have at least one googleapis.com URL OUTSIDE the OAuth scope namespace (i.e. not matching `googleapis.com/auth/`). The mTLS check runs only against that filtered set. Verified against four synthesized cases: only_oauth.py (only OAuth scopes) → ignored ✓ real_endpoint.py (endpoint, no mTLS) → flagged ✓ real_endpoint_with_mtls (endpoint + mTLS) → passes ✓ mixed.py (OAuth + endpoint, no mTLS)→ flagged ✓ No effect on the surrounding `logger`, `from __future__`, or `cli` import checks. CI policy intent unchanged: real hardcoded googleapis.com endpoints still must declare their `.mtls` counterpart. Refs: - #2 (the BQAA Storage Write regional routing fix that surfaced this false positive) - GoogleCloudPlatform/BigQuery-Agent-Analytics-SDK#262
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
2 participants
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.
The "Check for hardcoded googleapis.com endpoints" step in
.github/workflows/check-file-contents.ymlusesgrep -lE 'https?://[a-zA-Z0-9.-]+\.googleapis\.com'to find files that should also declare an
.mtls.googleapis.comcounterpart for dynamic endpoint selection. The regex matches anygoogleapis.comURL — including OAuth 2.0 scope URLs likehttps://www.googleapis.com/auth/cloud-platformand.../auth/bigquery— which are identity strings, not API endpoints. They don't have mTLS counterparts and never will. Any file that legitimately declares an OAuth scope (very common for ADK plugins integrating Google APIs) trips the gate even when no real endpoint is hardcoded.Surfaces today on any PR touching
src/google/adk/plugins/bigquery_agent_analytics_plugin.py(the file already declareshttps://www.googleapis.com/auth/bigqueryat L2145 for the BigQuery API scope). The same false positive affects any plugin that needs to reference a standard Google API scope.Fix
Add a second pass that filters the candidate set down to files that have at least one
googleapis.comURL outside the OAuth scope namespace (i.e., not matchinggoogleapis.com/auth/). The mTLS check runs only against that filtered set.Truth table verified by running the patched logic against four synthesized test files locally.
Scope
Workflow-only. No source-code changes. No effect on the sibling steps in the same workflow (
loggerpattern,from __future__ import annotations,cliimports) — they live in independent regex blocks.The check's intent — real hardcoded
googleapis.comendpoints must declare their.mtlscounterpart — is preserved.Background
Originated as caohy1988/adk-python#4 where it was reviewed and merged on the fork. Surfaced while preparing a separate Storage Write API regional-routing fix for the BQAA plugin (queued behind this one upstream so the policy gate gives an honest signal on real changes).
Test plan
check-file-contentsis filtered topaths: '**.py'and this PR only touches YAML, so the step is skipped on its own diff. Validation runs on follow-up PRs touching.pyfiles (specifically the queued BQAA Storage Write fix).