feat(agent): add provider-agnostic secret reference scheme#442
feat(agent): add provider-agnostic secret reference scheme#442leoparente wants to merge 1 commit into
Conversation
|
Go test coverage
Total coverage: 74.4% |
Vulnerability Scan: PassedImage: No vulnerabilities found. Commit: 4fcb8d5 |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3fddf99539
ℹ️ 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".
There was a problem hiding this comment.
Pull request overview
This PR introduces a provider-agnostic secret placeholder scheme (${secret://…}) that resolves through whichever secrets manager is active, while preserving backward compatibility with provider-specific schemes (e.g., ${vault://…}, ${fleet://…}), and adds docs/tests to cover the new behavior.
Changes:
- Extend shared placeholder matching to accept
${secret://…}as an alternative to the active provider scheme. - Make the dummy (no-manager) secrets manager fail fast on
${secret://…}references instead of passing them through. - Add documentation and tests for the generic scheme across polling providers and the Fleet secrets manager.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| README.md | Links and briefly introduces the provider-agnostic ${secret://…} reference scheme. |
| docs/secretsmgr/vault.md | Documents that ${secret://…} can be used in place of ${vault://…}. |
| docs/secretsmgr/doppler.md | Documents that ${secret://…} can be used in place of ${doppler://…}. |
| docs/secretsmgr/delinea.md | Documents that ${secret://…} can be used in place of ${delinea://…}. |
| docs/secretsmgr/cyberark.md | Documents that ${secret://…} can be used in place of ${cyberark://…}. |
| docs/secretsmgr/secret_references.md | Adds a dedicated doc describing generic references, provider body grammar, and no-manager behavior. |
| agent/secretsmgr/resolver.go | Updates the shared placeholder regex to match either the provider scheme or secret. |
| agent/secretsmgr/polling_test.go | Adds tests ensuring polling-base resolution supports ${secret://…} for both policy and config paths. |
| agent/secretsmgr/manager.go | Makes the dummy manager reject ${secret://…} references during policy/config secret solving. |
| agent/secretsmgr/manager_test.go | Adds tests for dummy-manager rejection/passthrough behaviors. |
| agent/secretsmgr/fleet.go | Extends Fleet secret reference matching to also accept ${secret://…}. |
| agent/secretsmgr/fleet_test.go | Adds a test case validating ${secret://…} resolution via the Fleet secrets manager. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
3fddf99 to
f0d725b
Compare
90b965c to
862cd86
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
bdf5570 to
75ec647
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Keep it up! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
75ec647 to
40efbb7
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40efbb7b3b
ℹ️ 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".
Policies and config can now reference managed secrets as ${secret://<body>}
instead of naming a specific provider. The reference is resolved by whichever
secrets manager is active (vault, delinea, doppler, cyberark, or fleet); the
body is still interpreted using that provider's own grammar unchanged. All
provider-specific schemes (${vault://…} etc.) keep working exactly as before.
When no secrets manager is configured, the dummy manager now rejects any
${secret://…} reference with a clear error instead of forwarding it to the
backend as a literal string; provider-specific placeholders continue to pass
through untouched in that case, as before.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
40efbb7 to
fc5de31
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Already looking forward to the next diff. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
|
Please don't merge this. While talking to Dom it came out that we are going to need to support multiple secrets providers per client - something that this proposal seems to implicitly assume won't be true. |
What
Adds a provider-agnostic secret reference scheme:
${secret://<body>}is resolved by whichever secrets manager is active on the agent (vault, delinea, doppler, cyberark, or fleet). A policy or config can now reference a managed secret without naming the provider, so the same reference works on any agent regardless of which secrets manager it is configured with, and upstream layers can submit references without knowing the agent's provider.Fully backward compatible: the provider-specific schemes (
${vault://…},${fleet://…}, …) keep working unchanged.secretis a reserved scheme (no provider uses that name). The reference body stays in the active provider's own grammar —secret://removes the provider name from the reference, not the provider's path format.How
agent/secretsmgr/resolver.go: the shared placeholder matcher now accepts the generic scheme as an alternative to the provider's own (${(?:<scheme>|secret)://…}, non-capturing so match indices are unchanged). All four polling providers pick this up with zero per-provider changes, for both policy and config resolution; first-match/whole-string semantics are untouched.agent/secretsmgr/fleet.go: same alternation in the fleet reference regex, so the generic scheme also works with the fleet secrets manager (policy references).${fleet://…}config references. The previously crypticMQTT publisher not bounderror now explains this case. Policy references are unaffected.${secret://…}reference now fails fast witha managed secret reference (${secret://…}) was found but no secrets manager is configured— the generic scheme explicitly requests the active manager, so a clear error beats the literal placeholder reaching the backend. Provider-specific schemes keep their pre-existing passthrough behavior.Docs
New
docs/secretsmgr/secret_references.mddocumenting the generic scheme, the per-provider body grammar, the no-manager fail-fast error, and the fleet config-level limitation; linked from the README and from each provider page.Testing
go build ./...,go test -count=1 -race ./agent/secretsmgr/...(including the Docker-backed Vault tests),golangci-lint --config .github/golangci.yaml= 0 issues, gofumpt clean.🤖 Generated with Claude Code