You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Adds external secret providers to the Credentials plugin so a credential's secret fields can be resolved at poll time from a third‑party secret store, while the existing local encrypted database remains the default. Supports any number of providers, including multiple instances of the same type, chosen per credential.
Ships five drivers:
Provider
Auth
HashiCorp Vault
KV v1/v2; Token or AppRole
AWS Secrets Manager
IAM keys or EC2/ECS instance role (SigV4)
Azure Key Vault
Entra ID service principal or Managed Identity
Google Cloud Secret Manager
Service account key (signed JWT) or instance SA
CyberArk Conjur
Host/user identity + API key
How it works
Driver abstraction — a CredentialProvider interface + registry (includes/providers.php, includes/providers/). Drivers are stateless; a provider instance (a credential_providers row) supplies per‑connection config. Adding a new store is just a new class registered in credentials_provider_registry() — no storage/UI changes needed.
Storage — new provider_id column on credentials (0 = local) and a new credential_providers table. Provider connection secrets (tokens, keys, client secrets) are encrypted with the Cacti core secret functions. Both schema steps are idempotent and run on install and upgrade.
Resolution — credentials_get() resolves provider‑backed secret references through the driver, cached per request and failing closed (logged, no secret value leaked). A reference supports an optional #key selector and dotted JSON paths (e.g. network/snmp#community).
UI — a new Secret Providers page (list/add/edit/enable/disable/delete) with a per‑provider Test Connection self‑check; a Secret Provider selector on the credential form (secret fields become reference inputs); a provider column on the list. Both pages are gated on the core Users/Groups realm.
Backward compatibility
provider_id defaults to 0, so every existing credential keeps using the local encrypted store with no migration. The local code path is unchanged.
Testing
Run against a Cacti 1.2.x checkout with Pest (as CI does):
✅ pest — 80 passed (720 assertions), including new tests for the provider registry, reference/JSON selection, the AWS SigV4 implementation (published AWS test vectors) and the GCP RS256 JWT (generated keypair verified end‑to‑end).
Note: the five drivers' live network calls (resolve()/test()) were validated structurally and by unit‑testing their pure crypto/parsing helpers; they should be exercised against live stores before release.
Notes
Version bumped to 1.1; CHANGELOG, README.md and the gettext template updated.
Extend the credential store so a credential's secret fields can be resolved
at poll time from an external secret store instead of the local encrypted
database, while keeping the local store as the default.
Architecture
- Add a pluggable CredentialProvider driver interface and a registry
(includes/providers.php, includes/providers/). Drivers are stateless
strategies; a provider instance supplies per-connection configuration, so
any number of providers - including several instances of the same type -
can run side by side and be chosen per credential.
- Ship five drivers: HashiCorp Vault (KV v1/v2, Token/AppRole), AWS Secrets
Manager (SigV4, static keys or instance role), Azure Key Vault (service
principal or Managed Identity), Google Cloud Secret Manager (service
account JWT or instance SA) and CyberArk Conjur.
Storage and resolution
- Add a provider_id column to the credentials table and a new
credential_providers table (per-connection secrets encrypted with the Cacti
core secret functions). Both schema steps are idempotent and run on
install and upgrade.
- credentials_get() resolves provider-backed secret references through the
driver, caching per request and failing closed (logged, no secret leaked).
A reference supports an optional '#key' selector and dotted JSON paths.
User interface
- Add a Secret Providers management page (list/add/edit/enable/disable/delete)
with a per-provider Test Connection self-check, gated on the Users/Groups
realm, with its own menu pick and breadcrumbs.
- Add a Secret Provider selector to the credential edit form; provider-backed
secret fields become reference inputs, and the list gains a provider column.
Tests and docs
- Add unit tests for the registry, reference/JSON selection, the AWS SigV4
implementation (published AWS vectors) and the GCP RS256 JWT.
- Update the lifecycle test for the two-table install/upgrade, refresh the
manifest, gettext template, README, CHANGELOG and bump the version to 1.1.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…coverage
Code review follow-ups on PR #4:
Security / correctness
- credential_providers.php: require POST before a save so the handler cannot
be driven by a crafted GET (CSRF), matching the bulk-action/test handlers.
- credentials.php: preserve a credential's previous properties only when both
the type AND the backing provider are unchanged, so switching an external
credential to the local store no longer keeps a reference as if it were
ciphertext and bypasses required-secret validation.
- setup.php: add an idempotent provider_id column/index migration
(api_plugin_db_add_column + guarded index) so upgrades from 1.0 on the Cacti
1.2.29 floor gain the column instead of failing the new JOIN and saves.
- credential_providers.php: import the page-scope $actions map into
provider_list() so the bulk-action dropdown renders.
Drivers
- AbstractProvider::flag(): treat an explicit empty checkbox value as false so
a disabled "Verify TLS Certificate" is honoured (previously fell back to the
true default, keeping verification on).
- AwsSecretsManagerProvider: read the ECS/EKS container credentials endpoint
(AWS_CONTAINER_CREDENTIALS_RELATIVE_URI / FULL_URI, with token and endpoint
validation) before falling back to EC2 IMDSv2, so Fargate/ECS task roles work.
- VaultProvider::resolve(): JSON-encode a structured single field instead of
casting it to 'Array', and route key selection through selectValue() so the
documented dotted-path syntax works for nested KV values.
Tests / coverage
- Measure includes/providers.php in phpunit.xml and allowlist the web entry
point and the live-network drivers in patch-coverage.php.
- Add offline tests (in-memory driver) for provider config decryption, option
building, resolution success/missing/disabled/failure, per-request caching,
unchanged non-secret fields, and credentials_get() local vs provider paths;
add a VaultProvider::resolve() regression test for dotted/structured values.
- Assert the provider_id column migration in the install lifecycle test.
Docs
- README: add a worked programmatic-use example showing how a plugin requests
a credential with credentials_get() and reads a published type's fields,
including provider fail-closed handling.
- Regenerate locales/po/cacti.pot.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Follow-ups from the re-review of PR #4:
- credentials.php: require POST before a save so the credential save path
(provider/reference changes) cannot be driven by a crafted GET, matching
credential_providers.php.
- AwsSecretsManagerProvider::allowedContainerUri(): only accept a 127.0.0.0/8
host when it is a real IPv4 literal (rejecting DNS names like
127.attacker.example), and strip the brackets parse_url keeps around an IPv6
literal so [::1] and [fd00:ec2::23] match the loopback/link-local allowlist.
- VaultProvider::resolve(): percent-encode the mount and secret path
segment-by-segment (preserving slashes) so spaces and reserved characters
produce a valid, unambiguous URL.
Tests:
- AwsContainerCredentialsTest: HTTPS/loopback/link-local accepted (incl. IPv6
bracket forms); public hosts, 127-prefixed DNS names and non-http schemes
rejected.
- VaultResolveTest: assert the requested Vault URL percent-encodes a path
containing a space.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Optional secrets cannot be cleared from provider configuration
credential_providers.php:164
An empty secret input always preserves its stored value, so the form cannot remove an optional secret. For example, replacing AWS STS credentials with permanent IAM keys leaves the old session_token stored; callApi() sends it with the new keys and authentication fails. Add an explicit clear control for optional provider secrets and handle it before the blank-means-keep branch, while preserving that behavior for ordinary edits.
Connection tests incorrectly require secret list permissions
The connection checks require list permissions that secret resolution does not need. AWS GetSecretValue permission does not grant ListSecrets; Azure secret Get does not grant List; and GCP's roles/secretmanager.secretAccessor does not grant secretmanager.secrets.list. Consequently valid read-only configurations resolve secrets but fail Test Connection. This also affects AzureKeyVaultProvider::test() and GcpSecretManagerProvider::test(). Test a supplied reference with the read operation, or distinguish unavailable list permission from authentication/connectivity failure without treating every authorization error as success.
AWS China regions use the wrong Secrets Manager hostname
AWS China regions (cn-north-1 and cn-northwest-1) use the amazonaws.com.cn suffix. The generated hostname therefore cannot reach Secrets Manager in those regions unless an endpoint override is supplied. Select the China DNS suffix for cn- regions while retaining explicit endpoint overrides.
The AWS_CONTAINER_CREDENTIALS_RELATIVE_URI branch assembled the URL without
validation, so a value like '@attacker.example/credentials' turned the fixed
169.254.170.2 link-local address into userinfo and sent the container
authorization token to an attacker-controlled host. Validate the assembled URL
through allowedContainerUri() (host must remain the link-local address), and add
regression cases for the userinfo trick.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
A role with secretsmanager:GetSecretValue on the configured secrets can resolve credentials successfully but still fails Test Connection because this call additionally requires secretsmanager:ListSecrets. Probe a configured test reference with GetSecretValue, or distinguish successful authentication from missing list permission, rather than requiring broader IAM access for the self-check.
A valid Key Vault access policy granting get but not list can resolve every configured reference, yet this listing request returns 403 and makes Test Connection report failure. Support probing a configured secret with the same get operation used by resolve(), or report list authorization separately, so administrators need not grant unrelated list access.
Secret Manager self-check incorrectly requires list permission
The standard roles/secretmanager.secretAccessor role permits reading secret versions but does not grant secretmanager.secrets.list. Consequently a correctly configured read-only service account fails this self-check even though resolve() works. Probe a configured test secret/version instead, or distinguish missing list permission from an authentication/connectivity failure.
- Provider deletion now runs under a transaction that locks the provider row
(SELECT ... FOR UPDATE) before counting references, and the credential save
locks the same row before persisting a provider-backed credential. The shared
row lock serialises the two operations, so a provider can no longer be deleted
between a save's validation and its commit and leave an orphaned credential.
- The resolution cache is now keyed per process with an explicit
credentials_provider_flush_cache(), which long-lived consumers (poller /
script-server) call once per polling cycle so rotated secrets, recovered
providers and newly disabled providers are re-evaluated instead of returning a
value cached for the life of the process.
- Add a flush regression test (re-resolution after a cycle boundary).
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The reason will be displayed to describe this comment to others. Learn more.
🔵 Needs a closer look
Security-sensitive provider authentication and schema upgrades need human review and integration validation, alongside correction of the retained defects.
An IAM identity granted only secretsmanager:GetSecretValue for its configured secrets can resolve credentials but fails Test Connection because secretsmanager:ListSecrets requires separate permission. Probe a known secret reference, or report listing authorization separately from connectivity/authentication, so a working least-privilege provider is not reported as broken. Do not treat every access-denied response as a successful connection test.
Test Connection incorrectly requires Key Vault list permission
A Key Vault access policy granting secret get without list supports resolve() but fails this Test Connection request with HTTP 403. Probe a known secret reference, or report listing authorization separately from connectivity/authentication, rather than requiring broader permissions for a working provider. Do not treat every HTTP 403 as success.
Test Connection incorrectly requires Secret Manager list permission
The standard roles/secretmanager.secretAccessor role permits reading secret versions but does not grant secretmanager.secrets.list. A correctly configured accessor-only provider can therefore resolve secrets while failing Test Connection here with HTTP 403. Probe a known reference, or report listing authorization separately from connectivity/authentication, without requiring an additional role or treating every permission error as success.
cn-north-1 and cn-northwest-1 resolve to secretsmanager.<region>.amazonaws.com.cn
rather than amazonaws.com, so resolution and Test Connection reach the correct
host without an endpoint override. Add a host() regression test covering the
standard, China and endpoint-override cases.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Provider secret confirmation mismatches are not rejected
credential_providers.php:160
Provider secrets use textbox_password, but this handler ignores their submitted confirmation fields. A mistyped token or client secret is saved even when its confirmation differs, potentially breaking every credential using that provider. Reject mismatches before encryption, as credentials.php:210–220 already does.
Stale AWS session tokens cannot be cleared
credential_providers.php:164
Once an AWS provider has a Session Token stored, entering permanent IAM keys and leaving the token blank preserves the old token. The driver then sends that stale X-Amz-Security-Token with the new keys, breaking authentication. Add an explicit clear control for optional secrets, separate from blank-to-keep, so administrators can remove the session token without recreating the provider or resetting its configuration.
Test Connection wrongly requires secret enumeration permissions
Test Connection requires enumeration permissions that secret resolution does not need. AWS identities granted only GetSecretValue, Azure access policies granting Get without List, and GCP identities using roles/secretmanager.secretAccessor can resolve their secrets but fail these listing probes. The same issue appears in AzureKeyVaultProvider::test() and GcpSecretManagerProvider::test(). Test a supplied secret reference with the read operation, or distinguish an unavailable enumeration check from failed authentication/connectivity. Do not require broader listing permissions just to pass the self-check.
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
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.
Summary
Adds external secret providers to the Credentials plugin so a credential's secret fields can be resolved at poll time from a third‑party secret store, while the existing local encrypted database remains the default. Supports any number of providers, including multiple instances of the same type, chosen per credential.
Ships five drivers:
How it works
CredentialProviderinterface + registry (includes/providers.php,includes/providers/). Drivers are stateless; a provider instance (acredential_providersrow) supplies per‑connection config. Adding a new store is just a new class registered incredentials_provider_registry()— no storage/UI changes needed.provider_idcolumn oncredentials(0= local) and a newcredential_providerstable. Provider connection secrets (tokens, keys, client secrets) are encrypted with the Cacti core secret functions. Both schema steps are idempotent and run on install and upgrade.credentials_get()resolves provider‑backed secret references through the driver, cached per request and failing closed (logged, no secret value leaked). A reference supports an optional#keyselector and dotted JSON paths (e.g.network/snmp#community).Backward compatibility
provider_iddefaults to0, so every existing credential keeps using the local encrypted store with no migration. The local code path is unchanged.Testing
Run against a Cacti 1.2.x checkout with Pest (as CI does):
pest— 80 passed (720 assertions), including new tests for the provider registry, reference/JSON selection, the AWS SigV4 implementation (published AWS test vectors) and the GCP RS256 JWT (generated keypair verified end‑to‑end).php -lon every plugin PHP file.validate-manifest.php;locales/po/cacti.potregenerated.Notes
CHANGELOG,README.mdand the gettext template updated.