Skip to content

feat: support GCP broker metadata auth#1017

Open
dani29 wants to merge 4 commits into
openclaw:mainfrom
dani29:agent/gcp-workload-identity
Open

feat: support GCP broker metadata auth#1017
dani29 wants to merge 4 commits into
openclaw:mainfrom
dani29:agent/gcp-workload-identity

Conversation

@dani29

@dani29 dani29 commented Jul 8, 2026

Copy link
Copy Markdown

Summary

  • Adds explicit CRABBOX_GCP_CREDENTIAL_SOURCE=metadata support for brokered GCP coordinators running with an attached Google Cloud service identity.
  • Preserves service-account-key auth as the default and fails closed for partial key pairs, invalid source names, or keyless config without explicit metadata opt-in.
  • Retries documented transient metadata-server 429 and 503 responses with bounded exponential backoff.
  • Preserves the HTTP status for non-JSON metadata errors instead of replacing the provider diagnostic with a JSON parse exception.
  • Updates readiness, environment typing, provider docs, and maintainer-side changelog credit.

Refs #1016.

Config and secret implications

Google-hosted coordinators can set CRABBOX_GCP_CREDENTIAL_SOURCE=metadata plus GCP_PROJECT_ID or CRABBOX_GCP_PROJECT. Portable coordinators continue to use both GCP_CLIENT_EMAIL and GCP_PRIVATE_KEY. Metadata mode sends no Crabbox credential to the metadata endpoint; it makes the documented fixed-host request with Metadata-Flavor: Google and uses the returned short-lived access token only for Google Compute API calls.

Verification

  • npm test --prefix worker — 916 passed, 3 skipped
  • npm run format:check --prefix worker
  • npm run lint --prefix worker
  • npm run check --prefix worker
  • npm run check:node --prefix worker
  • HOME=/tmp/crabbox-home npm run build --prefix worker
  • npm run build:node --prefix worker
  • ./scripts/check-docs.sh
  • .agents/skills/autoreview/scripts/autoreview --mode branch --base origin/main --parallel-tests "npm test --prefix worker -- --run test/gcp.test.ts test/fleet.test.ts test/gcp-release.live.test.ts" --stream-engine-output — clean, no accepted or actionable findings
  • Endpoint/header/retry contract checked against current official Compute Engine, Cloud Run, and GKE Workload Identity documentation

Live proof

Exact candidate e87367e9 passed on a real GCP e2-standard-2 lease, cbx_ed35f8631aef. The branch's bundled GCPClient ran with only CRABBOX_GCP_CREDENTIAL_SOURCE=metadata and the metadata-derived project ID, acquired the attached service-account token, and completed the real aggregated Compute instance-list request:

metadata_token_acquisition=ok compute_api_request=ok crabbox_machine_count=0

The lease was deleted after the command, and go run ./cmd/crabbox list --provider gcp returned an empty inventory. No static GCP key was copied into the lease.

@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

Codex review: needs changes before merge. Reviewed July 9, 2026, 4:19 AM ET / 08:19 UTC.

Summary
Adds explicit CRABBOX_GCP_CREDENTIAL_SOURCE=metadata support for brokered GCP coordinators, including readiness/docs/types updates plus mocked and live-test coverage while keeping service-account-key auth as the default.

Reproducibility: not applicable. as a bug reproduction; this is a feature PR for a new brokered GCP credential source. Source inspection confirms current main and v0.36.0 still require static service-account key credentials.

Review metrics: 2 noteworthy metrics.

  • Changed files: 8 files, +330/-20. The diff spans provider runtime code, readiness, Env typing, docs, and tests, so maintainer review needs both auth and upgrade-boundary context.
  • Brokered GCP credential modes: 1 existing mode preserved, 1 explicit mode added. Service-account-key remains the default while metadata auth becomes a new opt-in credential path.

Root-cause cluster
Relationship: fixed_by_candidate
Canonical: #1016
Summary: This PR is the implementation candidate for the open brokered GCP metadata-server auth issue; the issue should remain open until this PR merges or is otherwise resolved.

Members:

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🐚 platinum hermit
Proof: 🦞 diamond lobster
Patch quality: 🐚 platinum hermit
Result: ready for maintainer review.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

  • Remove the CHANGELOG.md entry from the feature branch and keep release-note context in the PR body or commit message.
  • [P2] Get maintainer acceptance for the new brokered GCP metadata credential boundary before merge.

Risk before merge

  • [P1] Merging this PR accepts a new explicit brokered GCP runtime-identity credential path, so maintainers should own the supported runtime and metadata-token boundary before release.
  • [P1] The branch still edits release-owned CHANGELOG.md; that should be cleaned up before merge while preserving release-note context in the PR body or commit message.

Maintainer options:

  1. Accept after release-note cleanup (recommended)
    Maintainers can approve the metadata credential boundary after the branch removes its CHANGELOG.md entry.
  2. Narrow the metadata-auth contract
    Require additional runtime constraints, documentation, or proof before supporting metadata tokens for brokered GCP.
  3. Pause this credential source
    If metadata auth is not an accepted brokered GCP direction, pause this PR and keep the linked issue as the design venue.

Next step before merge

  • [P2] A narrow automated repair can remove the release-owned CHANGELOG.md entry, but merge still needs maintainer acceptance of the provider auth/security boundary.

Maintainer decision needed

  • Question: Should Crabbox support CRABBOX_GCP_CREDENTIAL_SOURCE=metadata as an explicit brokered GCP credential path for Google-hosted coordinators once the release-owned changelog edit is removed?
  • Rationale: The implementation and live proof are credible, but accepting metadata-server tokens changes coordinator credential acquisition and the supported provider-auth boundary.
  • Likely owner: steipete — This account has the clearest current GCP provider history and authored the hardening/proof commits on the PR branch.
  • Options:
    • Accept after changelog cleanup (recommended): Approve the explicit metadata credential source and require removing the feature-branch CHANGELOG.md entry before merge.
    • Require narrower runtime constraints: Ask for stricter docs, runtime limits, or additional proof if the Google-hosted coordinator boundary is still too broad.
    • Pause provider-auth expansion: Keep the linked issue open as the design venue if maintainers do not want metadata credentials supported yet.

Security
Cleared: No concrete secret persistence, token exposure, or broadened credential leakage was found; metadata mode is explicit and uses short-lived tokens for GCP API calls.

Review findings

  • [P3] Remove release notes from the feature branch — CHANGELOG.md:7
Review details

Best possible solution:

Land the provider-local metadata auth path after maintainer security-boundary acceptance, with the release-owned changelog entry removed from the feature branch.

Do we have a high-confidence way to reproduce the issue?

Not applicable as a bug reproduction; this is a feature PR for a new brokered GCP credential source. Source inspection confirms current main and v0.36.0 still require static service-account key credentials.

Is this the best way to solve the issue?

Mostly yes: the provider-local implementation preserves the default key path, fails partial config closed, and includes focused tests plus live proof. The remaining issue is release-note ownership and maintainer approval of the auth boundary.

Full review comments:

  • [P3] Remove release notes from the feature branch — CHANGELOG.md:7
    CHANGELOG.md is release-owned by repository policy, so this feature branch should leave that entry for maintainer release handling. This prior P3 finding is still present at the current head; keep the release-note context in the PR body or commit message instead.
    Confidence: 0.88

Overall correctness: patch is correct
Overall confidence: 0.86

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 7239e753df64.

Label changes

Label justifications:

  • P2: This is a normal-priority provider auth improvement with bounded blast radius and no evidence of a current outage.
  • merge-risk: 🚨 auth-provider: The PR changes how brokered GCP obtains and caches credentials for Compute API calls.
  • merge-risk: 🚨 security-boundary: The PR adds an explicit metadata-server token boundary whose supported runtime and token handling need maintainer acceptance before release.
  • rating: 🐚 platinum hermit: Overall readiness is 🐚 platinum hermit; proof is 🦞 diamond lobster and patch quality is 🐚 platinum hermit.
  • status: 👀 ready for maintainer look: ClawSweeper has no concrete contributor-facing blocker left for this PR. Sufficient (live_output): Sufficient: the PR body includes copied live output for exact candidate e87367e showing metadata-token acquisition, a successful Compute API request, and cleanup on a real GCP lease.
  • proof: sufficient: Contributor real behavior proof is sufficient. Sufficient: the PR body includes copied live output for exact candidate e87367e showing metadata-token acquisition, a successful Compute API request, and cleanup on a real GCP lease.
Evidence reviewed

Acceptance criteria:

  • [P1] git diff --check.
  • [P1] ./scripts/check-docs.sh.

What I checked:

  • Repository policy read: AGENTS.md was read fully; its provider-boundary guidance applies to the GCP auth change, and its release-process expectations inform the CHANGELOG.md finding. (AGENTS.md:1, 7239e753df64)
  • Current main lacks metadata broker auth: Current main's GCP client constructor still requires both static service-account key variables, so the requested metadata credential path is not already implemented on main. (worker/src/gcp.ts:115, 7239e753df64)
  • Current main readiness is static-key based: Current main's GCP readiness still derives missing values from providerRequiredSecrets, which includes the static key pair. (worker/src/fleet.ts:12788, 7239e753df64)
  • Latest release lacks the requested auth path: The v0.36.0 release requires GCP_CLIENT_EMAIL and GCP_PRIVATE_KEY in code and docs, so this PR is not obsolete because of a shipped release. (docs/providers/gcp.md:176, 4ea85a4f1b7e)
  • PR adds explicit metadata credential path: The PR head adds credential-source parsing, partial-key fail-closed checks, metadata token acquisition with the Metadata-Flavor header, bounded 429/503 retries, and token caching. (worker/src/gcp.ts:688, e87367e906eb)
  • PR adds focused coverage: The PR head covers metadata token use, transient retry behavior, non-JSON metadata errors, partial key rejection, invalid credential sources, and readiness permutations. (worker/test/gcp.test.ts:43, e87367e906eb)

Likely related people:

  • steipete: git blame ties the current static-key GCP constructor, readiness logic, and provider secret registry to the v0.36.0 release commit, and recent GCP cleanup history touches the same provider files. (role: introduced behavior and recent GCP area contributor; confidence: high; commits: 4ea85a4f1b7e, bbcfe346fdde; files: worker/src/gcp.ts, worker/src/fleet.ts, worker/src/types.ts)
  • vincentkoc: Recent provider/coordinator work touched shared Env and fleet surfaces adjacent to this auth-readiness change, though not the central GCP auth path. (role: adjacent provider registry contributor; confidence: low; commits: caa79b1eb5c8; files: worker/src/types.ts, worker/src/fleet.ts, worker/test/fleet.test.ts)
What the crustacean ranks mean
  • 🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
  • 🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
  • 🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
  • 🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
  • 🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
  • 🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
  • 🌊 off-meta tidepool: rating does not apply to this item.

Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

How this review workflow works
  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.
Review history (7 earlier review cycles)
  • reviewed 2026-07-08T15:23:00.459Z sha 4d17f40 :: needs real behavior proof before merge. :: [P1] Keep GCP readiness fail-closed until metadata auth is proven
  • reviewed 2026-07-08T15:36:25.337Z sha 4d17f40 :: needs real behavior proof before merge. :: [P1] Keep GCP readiness fail-closed until metadata auth is proven
  • reviewed 2026-07-08T17:06:54.641Z sha 199ca80 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-08T17:12:29.558Z sha 199ca80 :: needs real behavior proof before merge. :: none
  • reviewed 2026-07-09T05:06:25.105Z sha 272e957 :: needs real behavior proof before merge. :: [P3] Remove release notes from the feature branch
  • reviewed 2026-07-09T05:12:02.464Z sha 272e957 :: needs real behavior proof before merge. :: [P3] Remove release notes from the feature branch
  • reviewed 2026-07-09T08:13:21.505Z sha e87367e :: found issues before merge. :: [P3] Remove release notes from the feature branch

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels Jul 8, 2026
@dani29 dani29 marked this pull request as ready for review July 8, 2026 15:31
@dani29 dani29 force-pushed the agent/gcp-workload-identity branch from 4d17f40 to 199ca80 Compare July 8, 2026 17:00
@dani29

dani29 commented Jul 8, 2026

Copy link
Copy Markdown
Author

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@clawsweeper clawsweeper Bot removed the merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. label Jul 8, 2026
@steipete steipete force-pushed the agent/gcp-workload-identity branch from 199ca80 to 272e957 Compare July 9, 2026 05:01
@steipete

steipete commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

@clawsweeper re-review

@clawsweeper

clawsweeper Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor

🦞🧹
ClawSweeper re-review requested.

I asked ClawSweeper to review this item again.
Action: item re-review queued (workflow sweep.yml, event repository_dispatch).
Result: the existing ClawSweeper review comment will be edited in place when the review finishes.

@dani29

dani29 commented Jul 9, 2026

Copy link
Copy Markdown
Author

Thanks @steipete & his claws army! :)

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. labels Jul 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🐚 platinum hermit Good normal PR readiness with ordinary maintainer review expected. status: 👀 ready for maintainer look ClawSweeper has no concrete contributor-facing blocker left for this PR.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants