feat: GCP Secret Manager integration for external secrets management#16663
feat: GCP Secret Manager integration for external secrets management#16663amor71 wants to merge 18 commits into
Conversation
Add a secrets resolution layer that intercepts ${gcp:SECRET_NAME} references
in openclaw.json config values and resolves them from Google Cloud Secret Manager.
Features:
- New ${gcp:secret-name} syntax for referencing secrets in config values
- Lazy loading: secrets are fetched only when referenced
- In-memory caching with configurable TTL (default: 5 minutes)
- Parallel resolution of multiple secret references
- Dynamic import of @google-cloud/secret-manager (optional dependency)
- Clear error messages for missing providers or failed resolution
Config syntax:
{
"secrets": {
"providers": {
"gcp": { "project": "my-project" }
}
},
"channels": {
"slack": {
"botToken": "${gcp:slack-bot-token}"
}
}
}
Closes openclaw#13610 (GCP portion)
🤖 AI-assisted (Claude) — fully tested
|
GCP Secret Manager integration for config secrets is a solid choice. We use secret references in config files ( |
Red-phase tests covering: - Secret reference parsing (valid, invalid, escaped, mixed, env var distinction) - Config tree walking (nested objects, arrays, primitives, fast path) - Cache behavior (TTL, expiry, stale-while-revalidate, clear) - GCP provider (mock client: fetch, not found, permission denied, timeout, version pin, dynamic import) - Error handling (unknown provider, unconfigured provider, path context, partial failure) - CLI commands (secrets test/list/setup/migrate/set)
…t suite - Add secret-resolution.ts: core resolution module with GcpSecretProvider - Add secrets.ts: CLI commands (setup, migrate, test, list, set) - 96 tests passing (74 unit + 22 CLI) - reference syntax, in-memory cache with TTL - Stale-while-revalidate, parallel resolution, clean error handling - Dynamic import of @google-cloud/secret-manager (optional dep) - Remove old secret-substitution files, update config.ts exports Refs: openclaw#13610 AI-assisted: Yes (Claude Opus via OpenClaw)
- scripts/secrets/bootstrap-gcp.sh — enable APIs, create SAs, generate keys - scripts/secrets/migrate-secrets.sh — scan, store, verify, purge plaintext - scripts/secrets/set-iam-bindings.sh — per-secret IAM for agent isolation - docs/concepts/secrets-gcp-console-guide.md — step-by-step Console walkthrough
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a62240644b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| resolveConfigSecrets, | ||
| configNeedsSecretResolution, |
There was a problem hiding this comment.
Invoke secret resolution in the config load path
This change only re-exports resolveConfigSecrets but does not hook it into any runtime config-loading path; in this commit, non-test code still reads config synchronously and never calls the resolver, so ${gcp:...} values remain unresolved during normal execution. That makes the new secrets feature effectively inert unless every caller manually adds an extra async resolution step.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
This is intentional per the design doc (Option A): loadConfig() stays synchronous and returns config with unresolved ${gcp:...} refs. A separate async resolveConfigSecrets(config) is called after load. The gateway startup code would await secret resolution before accepting connections.
Wiring into io.ts or the gateway startup is left for the maintainers to decide where it best fits in the existing startup flow — this PR provides the building blocks without touching the critical sync config path.
| constructor(config: { project: string; cacheTtlSeconds?: number; credentialsFile?: string }) { | ||
| this.project = config.project; | ||
| this.cacheTtlMs = (config.cacheTtlSeconds ?? 300) * 1000; | ||
| } |
There was a problem hiding this comment.
Honor credentialsFile when creating GCP clients
Although credentialsFile is accepted in the GCP provider config type, the constructor drops it and getClient() always instantiates SecretManagerServiceClient with no options. In multi-agent deployments this forces ADC/default credentials for all secret reads, so configured per-agent keyfiles are ignored and IAM isolation-by-keyfile cannot work as intended.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
You're right — credentialsFile was accepted but never passed to the client constructor. Fixed: getClient() now passes { keyFilename: this.credentialsFile } when configured. This is essential for per-agent isolation where each agent authenticates with its own service account key.
|
@jheeanny Great questions! Secret rotation: Secrets are cached in memory with a configurable TTL (default 5 minutes). When you update a secret in GCP Secret Manager, agents pick up the new value after the cache expires — no restart needed. You can also force a cache clear via Access error fallback: We use stale-while-revalidate — if the provider is unreachable and there's an expired cache entry, we serve the stale value and log a warning. For hard failures (secret not found, permission denied), we throw a Network timeouts get one automatic retry before falling back to cache or throwing. |
…etClient() - Added credentialsFile to Zod schema and TypeScript types - GcpSecretProvider now passes keyFilename to SecretManagerServiceClient - Essential for per-agent isolation (each agent uses its own SA key)
|
Nice work on this! We have a similar PR open (#11539) that takes a multi-provider approach (GCP, env, OS keyring, + stubs for AWS/Vault/1Password/etc), but honestly your GCP implementation is more polished — the CLI tooling, per-agent isolation via IAM bindings, migration scripts, and stale-while-revalidate caching are all things we didn't get to. We'd love to build the multi-provider abstraction on top of your implementation rather than compete with it. If this lands, we could contribute a follow-up PR that extracts a Would you be open to that kind of collaboration? |
|
@akoscz Really appreciate this — and yeah, collaboration makes way more sense than competing PRs. Looking at your PR (#11539), the Merge strategy:
What we'll contribute next:
We'll follow the same patterns as our GCP implementation (CLI tooling, caching with TTL, stale-while-revalidate, per-agent isolation where the platform supports it, comprehensive tests). Division of labor proposal:
Happy to coordinate on the interface contract before we both start building. Want to open a tracking issue or discussion thread for this? |
|
@jheeanny Following up on your rotation question — we've added a full manual key rotation reminder system to our requirements. Many secrets (Alpaca, Anthropic, OpenAI, etc.) can't be rotated programmatically, so we designed a system to handle those:
For keys that can be auto-rotated, we're also adding provider-native rotation support (AWS Lambda, Azure Event Grid, GCP Pub/Sub, Vault dynamic secrets/leases) — see our updated design docs. Full requirements: https://github.com/amor71/openclaw-secrets/blob/main/REQUIREMENTS-gcp-secrets.md (§7) |
Update: Security PR StatusThis PR (GCP Secret Manager + AWS + Azure + Vault integration) is still open and ready for review. 130+ tests, auto-rotation, NIST-based intervals. Related WorkThe Agents Plane project (issue #17299) now uses Secret Manager extensively for agent API key management:
This validates the Secret Manager integration in a real multi-tenant deployment. Happy to address any review comments. Fork: https://github.com/amor71/openclaw (branch: |
|
It'll take me a while to look into this - right now focus is mostly on fixes and security. |
| secrets: z | ||
| .object({ | ||
| providers: z | ||
| .object({ | ||
| gcp: z | ||
| .object({ | ||
| project: z.string(), | ||
| cacheTtlSeconds: z.number().int().nonnegative().optional(), | ||
| credentialsFile: z.string().optional(), | ||
| }) | ||
| .strict() | ||
| .optional(), | ||
| }) | ||
| .strict() | ||
| .optional(), | ||
| }) | ||
| .strict() | ||
| .optional(), |
There was a problem hiding this comment.
Schema only allows gcp provider but buildSecretProviders() in secret-resolution.ts:287-320 registers both gcp and aws providers. AWS config won't validate.
| secrets: z | |
| .object({ | |
| providers: z | |
| .object({ | |
| gcp: z | |
| .object({ | |
| project: z.string(), | |
| cacheTtlSeconds: z.number().int().nonnegative().optional(), | |
| credentialsFile: z.string().optional(), | |
| }) | |
| .strict() | |
| .optional(), | |
| }) | |
| .strict() | |
| .optional(), | |
| }) | |
| .strict() | |
| .optional(), | |
| secrets: z | |
| .object({ | |
| providers: z | |
| .object({ | |
| gcp: z | |
| .object({ | |
| project: z.string(), | |
| cacheTtlSeconds: z.number().int().nonnegative().optional(), | |
| credentialsFile: z.string().optional(), | |
| }) | |
| .strict() | |
| .optional(), | |
| aws: z | |
| .object({ | |
| region: z.string(), | |
| cacheTtlSeconds: z.number().int().nonnegative().optional(), | |
| profile: z.string().optional(), | |
| credentialsFile: z.string().optional(), | |
| roleArn: z.string().optional(), | |
| externalId: z.string().optional(), | |
| }) | |
| .strict() | |
| .optional(), | |
| }) | |
| .strict() | |
| .optional(), | |
| }) | |
| .strict() | |
| .optional(), |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/config/zod-schema.ts
Line: 634:651
Comment:
Schema only allows `gcp` provider but `buildSecretProviders()` in `secret-resolution.ts:287-320` registers both `gcp` and `aws` providers. AWS config won't validate.
```suggestion
secrets: z
.object({
providers: z
.object({
gcp: z
.object({
project: z.string(),
cacheTtlSeconds: z.number().int().nonnegative().optional(),
credentialsFile: z.string().optional(),
})
.strict()
.optional(),
aws: z
.object({
region: z.string(),
cacheTtlSeconds: z.number().int().nonnegative().optional(),
profile: z.string().optional(),
credentialsFile: z.string().optional(),
roleArn: z.string().optional(),
externalId: z.string().optional(),
})
.strict()
.optional(),
})
.strict()
.optional(),
})
.strict()
.optional(),
```
How can I resolve this? If you propose a fix, please make it concise.| // CreateSecret already stores the value, but if we want to be explicit: | ||
| await client.send( | ||
| new sdk.PutSecretValueCommand({ | ||
| SecretId: secretName, | ||
| SecretString: value, | ||
| }), | ||
| ); |
There was a problem hiding this comment.
Calling PutSecretValueCommand after CreateSecretCommand is redundant since CreateSecretCommand already stores the initial value.
| // CreateSecret already stores the value, but if we want to be explicit: | |
| await client.send( | |
| new sdk.PutSecretValueCommand({ | |
| SecretId: secretName, | |
| SecretString: value, | |
| }), | |
| ); | |
| await client.send( | |
| new sdk.CreateSecretCommand({ | |
| Name: secretName, | |
| SecretString: value, | |
| }), | |
| ); |
Prompt To Fix With AI
This is a comment left during a code review.
Path: src/config/aws-secret-provider.ts
Line: 194:200
Comment:
Calling `PutSecretValueCommand` after `CreateSecretCommand` is redundant since `CreateSecretCommand` already stores the initial value.
```suggestion
await client.send(
new sdk.CreateSecretCommand({
Name: secretName,
SecretString: value,
}),
);
```
How can I resolve this? If you propose a fix, please make it concise.|
|
||
| | Provider | Status | | ||
| | ------------------- | ------------ | | ||
| | GCP Secret Manager | ✅ Supported | | ||
| | AWS Secrets Manager | Planned | | ||
| | HashiCorp Vault | Planned | | ||
| | Azure Key Vault | Planned | | ||
|
|
There was a problem hiding this comment.
Documentation says AWS/Vault/Azure are "Planned" but they're fully implemented in src/config/aws-secret-provider.ts, src/config/azure-secret-provider.ts, src/config/vault-secret-provider.ts. Update status to "✅ Supported".
Prompt To Fix With AI
This is a comment left during a code review.
Path: docs/concepts/secrets.md
Line: 24:31
Comment:
Documentation says AWS/Vault/Azure are "Planned" but they're fully implemented in `src/config/aws-secret-provider.ts`, `src/config/azure-secret-provider.ts`, `src/config/vault-secret-provider.ts`. Update status to "✅ Supported".
How can I resolve this? If you propose a fix, please make it concise.Add SecretAgentProvider that implements the SecretProvider interface by shelling out to the secret-agent CLI. Secrets never enter Node.js process memory beyond the returned value. Features: - getSecret via `secret-agent exec --env` (avoids exposing values in ps) - setSecret via `secret-agent import --replace` - Bucket support for environment separation (prod/dev/staging) - Custom binary path configuration - Provider-level caching with configurable TTL - Full test suite with mocked child_process Resolves part of openclaw#14411, builds on openclaw#16663. Co-Authored-By: Claude Opus 4.6 <[email protected]>
|
This pull request has been automatically marked as stale due to inactivity. |
|
This pull request has been automatically marked as stale due to inactivity. |
|
Closing due to inactivity. |
|
Hi @amor71 — wanted to close the loop on this thread and your sibling PRs (#43640, #43641, #43643, #43644, #43645, #43646). The parallel The alternative architecture is now in #72548 — a manifest-first plugin SDK seam that hooks into the existing Your resolver bodies port over with mostly mechanical changes. The implementations you wrote (GCP at #16663, AWS/Azure/Vault/1Password across the sibling PRs) are still useful — the SDK calls, payload decoding, error mapping, and stale-while-revalidate logic all carry. What changes is the wrapping shell: drop the If you'd like to revisit any of the vendor work as plugins on the new seam, happy to review. The branch ( Thanks again for the original work; it directly informed the shape of the seam. |
Closes #13610
Labels:
size: XSenhancementsecuritySummary
Adds native GCP Secret Manager integration so credentials can be stored in an encrypted, access-controlled secrets store instead of plaintext files on disk.
What
${gcp:secret-name}in config files, resolved at runtime@google-cloud/secret-manager(optional, dynamically imported)openclaw secrets setup|test|list|set|migratesecretssection inopenclaw.json(Zod-validated)resolveConfigSecrets()runs afterloadConfig(); existing sync call sites unchangedFiles
src/config/secret-resolution.tssrc/config/secret-resolution.test.tssrc/commands/secrets.tssrc/commands/secrets.test.tsdocs/concepts/secrets.mddocs/concepts/secrets-gcp-console-guide.mdscripts/secrets/bootstrap-gcp.shscripts/secrets/migrate-secrets.shscripts/secrets/set-iam-bindings.shsrc/config/config.tssrc/config/types.openclaw.tssrc/config/zod-schema.tstest/setup.tspackage.json@google-cloud/secret-manageras devDependencyDesign decisions
loadConfig()stays synchronous;resolveConfigSecrets()is a separate async step. Least invasive.@google-cloud/secret-manageris dynamically imported. Clear error if not installed.secretsconfig = no behavior change. Entirely opt-in.Testing
Requirements & Design docs
Greptile Summary
This PR adds comprehensive secrets management support for GCP, AWS, Azure, and HashiCorp Vault, along with rotation reminders and auto-rotation for gateway tokens. The implementation includes secret reference syntax (
${provider:secret-name}), in-memory caching with TTL, lazy-loaded optional dependencies, CLI commands, migration scripts, and extensive test coverage (96 tests).Key changes:
secretsconfig section)Issues found:
zod-schema.ts:634-651only allowsgcpprovider but code registers bothgcpandawsinbuildSecretProviders(), causing AWS configs to fail validationdocs/concepts/secrets.mdlists AWS/Azure/Vault as "Planned" but they're fully implemented with testssetSecret()callsPutSecretValueCommandafterCreateSecretCommandwhen the create already stores the valueConfidence Score: 3/5
src/config/zod-schema.tsneeds AWS/Azure/Vault provider schemas addedLast reviewed commit: 6ec4efc