feat(gcp): enumerate external product grants by probing as the credential - #5577
Open
blarghmatey wants to merge 9 commits into
Open
feat(gcp): enumerate external product grants by probing as the credential#5577blarghmatey wants to merge 9 commits into
blarghmatey wants to merge 9 commits into
Conversation
…tial The migration is gated on knowing what has been granted TO each GCP credential outside GCP, because service accounts, API keys and reCAPTCHA keys cannot be moved between projects. Consolidating one means creating a replacement with a new identity, and every grant naming the old one has to be re-issued by hand. That work was scoped as a manual walk of each product's admin UI. Half of that premise is wrong. No GCP API finds these grants because they do not live in GCP -- they live in Drive, Sheets, Analytics, BigQuery and YouTube, and each of those answers "what can I see?" when asked AS the credential. A service-account key mints a token for those scopes, which turns "audit every product" into "mint one token per credential and ask five questions". bin/gcp-external-grants does that with five read-only probes. The one that earns it is bigquery.projects.list: it returns projects the caller can reach, so a third party's grant into their own project -- the edx.org datasets, which we cannot re-issue on our own timeline -- shows up without asking them. Two classification rules are load-bearing and both default to "unknown" rather than guessing, because a missed third-party grant is the expensive error: - Service-account addresses are classified by the project id embedded in them, checked against `gcloud projects list`, not by domain. Every project's SAs live under *.iam.gserviceaccount.com, a third party's included, so a domain check would silently mark theirs as ours. - The owned-project set is derived at runtime, never hardcoded. A stale name list mislabels OL projects as third-party and invents negotiations that do not need to happen -- it flagged ol-engineers and both ovs-* projects that way before this was fixed. Credential loading handles the three secret shapes this estate actually uses, across both KV versions: a JSON string in one field (xPro), a nested object (OCW Studio), and a whole secret body whose fields are the key's fields (Dagster), the last of which stores its PEM with literal backslash-n. Effective access is read from each item's capabilities rather than permissions.list, which returns 403 to a reader -- precisely where the answer matters. Verified end-to-end against two live gcloud identities; bigquery.projects.list returned 13 correctly-classified projects. The service-account path is tested but NOT yet exercised against a real key, and the register says so rather than implying coverage it does not have. The register also records what no tooling will reach: generic OAuth clients (no list API), Ads links, YouTube channel permissions, group-mediated grants, and GCS buckets. And it keeps granted YouTube quota out, which attaches to the project and follows no consumer anywhere. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Pull request overview
Adds tooling and documentation to inventory external Google-product grants before migrating GCP credentials.
Changes:
- Adds read-only probes for Drive, Analytics, BigQuery, and YouTube.
- Supports service-account credentials from files, Vault KV v1/v2, and gcloud.
- Documents the audit workflow and adds credential/classification tests.
Reviewed changes
Copilot reviewed 1 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
bin/gcp-external-grants |
Implements credential loading, API probes, classification, and reporting. |
docs/plans/gcp-external-grant-register.md |
Documents the audit process, credential sources, and known limitations. |
tests/bin/test_gcp_external_grants.py |
Tests secret extraction, PEM normalization, and ownership classification. |
tests/bin/__init__.py |
Initializes the test package. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
The register listed six commands to run by hand, which is six chances to skip one and no record of which were actually run. `probe-all` carries the estate as a manifest and needs no arguments. It reports progress on stderr and findings on stdout, so `--markdown` piped straight into the register's Findings table stays clean. `--json` keeps the raw output for the record. A credential that fails to load is never skipped silently: it gets a NOT ENUMERATED row carrying the reason. A row that is simply absent reads downstream as "asked, found nothing", which is the conflation this whole exercise exists to avoid -- and the one an operator running six commands by hand is most likely to introduce. Also adds a --heroku APP:VAR source, because ol-eng-library-platform@ is not in Vault at all and is otherwise unreachable without staging its key in a temp file first. Two things probe-all deliberately does not hide: - ocw-studio-production@ and ocw-studio-rc@ share one Vault path, so which one answers depends entirely on VAULT_ADDR. The run header prints the cluster used, results are labelled by the client_email that actually answered rather than by the name the manifest guessed, and the register now says to run it once per cluster. - When `gcloud projects list` returns nothing, third-party classification degrades to "unknown" and the run says so up front, rather than letting every row quietly read as OL-owned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
Ardiea
approved these changes
Aug 24, 2026
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| def _print_report(results: list[dict[str, Any]]) -> None: | ||
| for result in results: | ||
| print(f"\n=== {result['credential']} (project {result['project'] or '?'})") |
…he active one The first production run reported mitx-pipeline-main-dc29 as a third-party BigQuery grant. OL owns it; this project has a completed access task for it. Cause: ownership came from `gcloud projects list` for whichever account happened to be active. OL's estate spans two identities, and as tmacey@mit.edu that call returns only mitol-engineering and mitol01 -- so all 20 legacy projects, the entire population this migration is about, classified as external. That is the error runtime derivation was introduced to prevent, reintroduced in a worse form: a hardcoded list is at least stable, while this changed answer with whatever `gcloud config set account` was last run, silently, on an input unrelated to the credential being probed. Now unioned across every account gcloud holds credentials for, via --account so the active account is never mutated. 22 projects across two identities locally, which reclassifies mitx-pipeline-main-dc29 correctly and leaves mitx-residential-pipeline-main and mitir-mitx-surveys external -- neither is visible to any OL identity. Since this input is the one most likely to be quietly wrong, every run now prints what the classification rests on: the accounts consulted and the project count. --owned-project pins anything gcloud cannot see at all. Also adds a duplicate-identity warning to probe-all, prompted by the same run: secret-data/pipelines/edx/org/gcp-oauth-client and secret-data/pipelines/google-service-account both returned ol-data-platform-production@. So ol-data-platform-qa@ was never probed, the edx.org grants are still unenumerated, and its rows appeared twice in a way that reads as corroboration. Two sources resolving to one identity always means some credential went unprobed, so it is now called out on stderr. The register records the run: the Shared Drive collision is settled ( 0AErNBMZMmOz3Uk9PVA is "OL Engineering (ARCHIVED)", and both consumer-map labels for it were wrong), xPro's enrollments folder is owned by a personal Gmail account, and what the run did not cover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
| result["source_label"] = label | ||
| results.append(result) | ||
| print( | ||
| f" {result['credential']}: {len(result['grants'])} grant(s)", |
Reverses the previous commit's classification change, which was wrong in the direction that costs the most. That commit reclassified mitx-pipeline-main-dc29 from third-party to OL-owned by unioning `gcloud projects list` across every credentialed account. But that call returns every project an identity can SEE, which includes third-party projects OL has been granted into -- the exact population this tool exists to find. Keying ownership on visibility launders those grants into "internal" and drops them from the migration plan. The grant shows up as proof of ownership. mitx-pipeline-main-dc29 is the case in point: it sits in folder/249626760288, which mitx.devops@gmail.com cannot even describe. It appears in `projects list` solely because of the viewer grant recorded in tk-grant-mitx-devops-gmail-com-viewer-access-to-mit-0caa08. The original third-party flag was right. Ownership is now the project's parent, which has no such failure mode: ocw-studio-production no parent OL's (every legacy project) mitol01 folder/551004127831 OL's (migration target) mitx-pipeline-main-dc29 folder/249626760288 third party mitir-mitx-surveys not describable third party The owned-parent set is seeded from the credentials being probed and unioned across the run, so the verdict does not move with `gcloud config set account`, and a legacy-estate credential does not judge mitol01 -- OL's own destination -- as external. --owned-parent and --owned-project pin the rest. Also stops reporting Shared-Drive-resident files as "unknown owner". Those carry no `owners` field because the drive owns them, which rendered a dozen enrollment sheets as unknown and buried the genuinely unknown rows. The probe now resolves driveId against the drives the credential belongs to. The register records both cluster runs, including three misdirected shares from personal Gmail accounts onto the xPro testing SA, and the still-unresolved absence of any edx.org-owned dataset in either ol-data-platform credential's BigQuery grants. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
| if as_json: | ||
| print(json.dumps(results, indent=2, sort_keys=True)) | ||
| elif markdown: | ||
| print(_markdown_rows(results)) |
…ts" story The register's highest-value open item was "enumerate the edx.org-owned BigQuery datasets". Two things were wrong with the question. FIRST, the consumer map's attribution was internally inconsistent. It credited ol-data-platform-qa@'s BigQuery traffic to the Dagster edxorg/legacy_openedx code locations "via GCSConnection". A GCS connection makes no BigQuery calls, and searching ol-data-platform/dg_projects finds no BigQuery client at all -- the only Google client there is storage_client.list_blobs. The BigQuery consumer is Airbyte (confirmed by the owner). So this credential has two consumers and only one was recorded, which matters because replacing it breaks both. SECOND, the bucket is not edx.org's. edxorg_archive.py:74 names it simeon-mitx-pipeline-main -- the MITx "simeon" pipeline, matching BigQuery project mitx-pipeline-main-dc29. The DATA originates at edx.org (the asset description says so) but the grant counterparty is whoever runs that pipeline. The register had been conflating the data's origin with the grant's counterparty, which would have sent us to negotiate with the wrong organisation. Traffic re-verified live over 30 days, which also corrects the map's figure: 50,631 TableDataService.List rather than 48,258. Adds dataset-level enumeration, because project-level cannot answer what was actually granted: a credential appears against a project whether it holds one dataset or all of them, and for a third party that difference is the entire negotiation. Failures degrade to an empty list rather than dropping the project -- one we can list but not enumerate is still a real grant. Adds secret-operations/institutional-research-bigquery-service-account to the manifest. It appears in no inventory pass and no consumer-map row; it was found in dagster_server_policy.hcl:125, and its name matches the mitir-mitx-surveys project. If MIT Institutional Research issued it from their own project then OL cannot re-create it, which consolidation has no answer for. Its relationship to the Airbyte traffic is not yet established and the register says so. Also caches gcloud account discovery, which was re-shelling `gcloud auth list` once per project while resolving parentage and dominated a run's wall clock. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
Open Learning owns that service account. The previous commit speculated it might have been issued by MIT Institutional Research from their own project, which would have made it un-re-creatable and given consolidation no answer for it. That was wrong, and the correction goes in the register rather than being left to propagate -- an overstated blocker distorts sequencing as much as a missed one. What actually remains is a lead-time dependency: a replacement SA in mitol01 gets a new identity, so any access it needs to IR-held data must be granted to that new identity before the old credential is retired. OL can ask for that grant. Sequence it like the YouTube quota increases -- ask early, cut over after. The open question here is now the consumer, not the owner: the Vault policy granting this secret is Dagster's while the BigQuery traffic is attributed to Airbyte, so either Dagster holds a credential it does not use or there is a second BigQuery consumer nobody has mapped. Scope note kept deliberately narrow: the confirmation covers the institutional-research SA. Ownership of mitx-pipeline-main-dc29 (the MITx "simeon" pipeline) and mitx-residential-pipeline-main is still unknown and is not assumed from it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
… inventoried IRx owns all three BigQuery projects (mitx-pipeline-main-dc29, mitx-residential-pipeline-main, mitir-mitx-surveys), per the project owner. That collapses what read as three unknown counterparties into one internal MIT department. The tool still marks them third party, correctly -- they sit outside OL's project hierarchy, so a replacement SA needs its grants re-issued -- but the register now says to read that as "another MIT department we can ask". Corroborated in-repo rather than taken on trust: lakehouse/definitions.py:243-244 declares Airbyte asset groups irx_bigquery__s3_data_lake and irx_bigquery_email_opt_in__s3_data_lake on a 24h production interval, which is exactly the traffic measured. Chasing that corroboration turned up the larger finding. The same file lists emeritus_bigquery__s3_data_lake, and no Emeritus credential exists in Vault -- because src/bridge/secrets/airbyte/data.production.yaml holds THREE Google service accounts in SOPS: google_service_account_json, emeritus_google_service_account_json and global_alumni_google_service_account_json. None appears in the consumer map, the credential inventory, or any earlier triage. Every prior pass looked in Vault. Two of them are the case the earlier IRx alarm was reaching for and missing. Emeritus and Global Alumni are external commercial partners, not MIT departments, so if those SAs were issued from the partners' own projects they genuinely cannot be re-created by OL. Their traffic is also invisible to gcp-credential-usage, which reads per-project monitoring for projects we own. Probing them settles it, since client_email names the owning project. Adds --sops FILE:FIELD to reach them and puts all three in probe-all's manifest. The manifest test now checks source kinds generically and asserts every kind the manifest uses is one probe-all can actually dispatch -- it caught this change knowing only about vault and heroku. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
Review of PR #5577 found seven paths where gcp-external-grants raises instead of reporting. They share one root cause: error handling was built around ProbeError, but several call sites raise types no caller catches. This matters more here than in most CLIs. probe-all's whole contract is that no credential is skipped silently, and a traceback breaks that harder than a missing row does -- every credential after the crash is lost, and the partial output looks exactly like a complete one. Fixed at the root rather than by sprinkling try/except: - All subprocess calls now go through _run(), which converts a missing binary into a ProbeError naming what is absent. check=True is gone: a non-zero exit is data the caller interprets, a missing binary is not. - _read_vault_secret's v1 fallback raised hvac's own VaultError, which appears in no caller's except clause. The v2 attempt failing is routine (it is how the KV version is probed); the v1 attempt failing is the real error and now surfaces as ProbeError with the path. - _get no longer assumes an error body parses as JSON -- a proxy returning HTML on a 502 turned a reportable HTTP error into a ValueError. Same for success bodies and for the token endpoint's response. - --key-file is loaded per-file with failures collected, like every other credential source, instead of in a comprehension outside the handler. - _gcloud_token failure is reported into result["errors"] rather than raised. Two correctness fixes alongside: - _warn_on_duplicate_identities required grants-or-no-errors, so two sources resolving to one credential whose probes then all failed produced no warning AND no rows -- invisible twice over. It now keys on whether an identity was resolved at all, excluding load failures, which never reached an identity. - _probe_analytics and _probe_youtube ignored nextPageToken. Silent truncation is indistinguishable from a complete answer, which is the one thing a register meant to be exhaustive cannot afford. Adds tests/bin/test_robustness.py: eleven tests, one per crash path. The copilot thread about writing a service-account key to a predictable /tmp path is already addressed -- that recipe was replaced by the --heroku source two commits ago, so nothing stages the key on disk any more. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
Review caught the ownership model laundering third-party grants a second time, by a different route than the first. OL's legacy gmail-estate projects are all standalone -- no org, no folder -- so "" was being added to owned_parents and then used as a membership test. Every standalone project on earth matched. Sentry rated it LOW on the grounds that a third-party standalone project is an uncommon case; it is not uncommon here. The manifest gained Emeritus and Global Alumni credentials two commits ago, and a small commercial partner running a project with no org above it is exactly the shape this mislabels. Marking a partner's project internal reads as "no re-issue needed" -- the same failure as keying on `gcloud projects list`, wearing a different hat. Parentage genuinely cannot separate our legacy estate from a partner's standalone project, so the fix is not a cleverer parent rule. A probed credential's own project is now recorded by id: holding its key in our own secret store is the strongest evidence of ownership available, and it is precise where "has no parent" is not. Any other standalone project reports unknown, and --owned-project pins the ones a human can vouch for. This gives the tool a useful property: the more of the estate one run covers, the better it classifies, because each credential vouches for its own project. The register now says to prefer probe-all over one-off probe calls for that reason. Also loosens the "nothing to judge against" guard to consider home projects, not just owned parents. Without that, a run made up only of legacy credentials had an empty parent set and reported unknown for genuinely foreign projects -- suppressing the mitx-pipeline-main-dc29 verdict the previous fix established. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs
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.
What are the relevant tickets?
Advances the p0 gating task on the GCP migration: enumerate external Google-product grants per credential.
Description (What does it do?)
No credential can be re-homed into
mitol01until its external grants are known. Service accounts, API keys and reCAPTCHA keys cannot be moved between GCP projects, so consolidating one means creating a replacement with a new identity, and every grant naming the old one stops working at that moment.That work was scoped as a manual walk of each product's admin UI. Half of that premise is wrong. No GCP API finds these grants because they do not live in GCP -- they live in Drive, Sheets, Analytics, BigQuery and YouTube, and each of those has an API that answers "what can I see?" when asked as the credential itself. A service-account key mints a token for those scopes, which turns "audit every product" into "mint one token per credential and ask five questions".
Adds:
bin/gcp-external-grants-- five read-only probes per credential (Shared Drives, direct file shares, GA4 properties, BigQuery projects, YouTube channels). The probe that earns the tool isbigquery.projects.list: it returns projects the caller can reach, so a third party's grant into their own project -- the edx.org datasets, which we cannot re-issue on our own timeline -- shows up without having to ask them.docs/plans/gcp-external-grant-register.md-- the method, the per-credential run list with literal Vault paths, a findings table to populate, and an explicit list of what no tooling will reach.tests/bin/test_gcp_external_grants.py-- 11 tests over credential loading and grant classification.Two classification rules are load-bearing, and both report unknown rather than guessing, because a missed third-party grant is the expensive direction of the error:
gcloud projects list-- not by domain. Every project's service accounts live under*.iam.gserviceaccount.com, a third party's included, so a domain check would silently mark theirs as ours.ol-engineersand bothovs-*projects that way before this was fixed.Effective access is read from each item's
capabilitiesrather thanpermissions.list: it comes back inline with the listing (no extra call per item) and reports access inherited from a parent folder or arriving via a group, which a permission entry naming the credential would not show.Credential loading handles the three secret shapes this estate actually uses, across both KV versions: a JSON string in one field (xPro,
secret-xprokv-v1,service_account_creds), a nested object (OCW Studio,secret-ocw-studiokv-v2,google->drive_service_json), and a whole secret body whose fields are the key's fields (Dagster,secret-datakv-v1) -- the last of which stores its PEM with literal backslash-n, matching the.replace("\\n", "\n")its own consumer does atdg_projects/canvas/canvas/lib/canvas.py:32.How can this be tested?
--gcloudprobes the active gcloud identity and needs no secret.Verification performed. The probe path (token acquisition -> HTTP -> classification -> report) was exercised end-to-end against two live gcloud identities.
bigquery.projects.listreturned 13 projects formitx.devops@gmail.com, all correctly classified as OL-owned. The Drive/Analytics/YouTube probes returned403 insufficient authentication scopes, which is the correct result for a gcloud-minted token -- gcloud issuescloud-platformscope only.Not verified: the service-account path against a real key. The register says so rather than implying coverage it does not have. Running those probes is the next step, and the highest-value one (
ol-data-platform-qa@, the edx.org BigQuery grants) is listed first.Additional Context
Three findings that came out of the work and are worth a reviewer's attention:
ol-eng-library-platform@is not in Vault. A repo-wide search finds it in documentation only -- noOLVaultK8SSecret, no vault-agent template, no policy grant, no SOPS entry. It reaches its three Heroku apps as aGOOGLE_APPLICATION_JSONconfig var. The credential the consumer map credits with the estate's highest Drive traffic (592kFiles.Listcalls/30d) is stored outside every secret-management path this team operates.ocw-studio-production@andocw-studio-rc@share one Vault path.k8s_secrets.py:241sets"path": "collected"with no environment prefix, and mount and field are identical too; the two SAs are distinguished only by which Vault cluster answers. Running the same command twice against the same cluster probes the same credential twice and looks like agreement.0AErNBMZMmOz3Uk9PVAis credited to bothol-eng-library-platform@(as "ol-eng-library") andocw-studio-rc@(as the QA drive) --gcp-service-account-consumer-map.md:64and:68. At most one label is right;shared_drivesreports the drive's real name.Two caveats are enforced in the tool's output, not just the prose. Absence is not proof -- this estate has already produced two triage passes that read a zero as a death certificate and were wrong both times, so every "no grants found" line says so. And granted quota is not a grant -- the 21x YouTube increase on
ocw-studio-qaattaches to the project, follows no consumer, and will never appear in probe output.🤖 Generated with Claude Code
https://claude.ai/code/session_01LPdAC3MbeGhHdNTsXUXTZs