fix(shield): warn when Sysdig credentials are supplied inline (helm release metadata exposure)#2627
Open
EdwardArchive wants to merge 1 commit into
Conversation
…labs#2622) When access_key or secure_api_token is passed through values, the chart base64-encodes them into a Secret manifest that Helm then persists in its release storage (sh.helm.release.v1.<release>.v<rev> Secret). The credentials are also retrievable via `helm get values`, `helm get manifest`, and any GitOps tool that snapshots rendered manifests — broadening their blast radius beyond the application Secret itself. The chart already supports `*_existing_secret` references, but the trade-off was not documented and users hit the issue silently. This patch keeps the rendering behavior unchanged and adds: - A NOTES.txt block emitted only when inline credentials are detected, listing the affected keys and pointing at the existing-secret alternative. - Stronger inline comments in values.yaml flagging the trade-off. - Updated README rows reflecting the new guidance. - Five helm-unittest cases covering the warning being shown for each inline path and suppressed when existing-secret references are used. Refs: sysdiglabs#2622
Contributor
|
Hi @EdwardArchive. Thanks for your PR. After inspecting your changes someone with write access to this repo needs |
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.
Fixes #2622
Summary
When
sysdig_endpoint.access_keyorsysdig_endpoint.secure_api_tokenis passed through values, the chart base64-encodes them into a Secret manifest. The rendered manifest — including the secret values — is then persisted by Helm in its release storage:…and is retrievable via
helm get values --all,helm get manifest, or any GitOps tool that snapshots rendered manifests. This broadens the blast radius of the credential beyond the application Secret itself, but the trade-off was not documented and users hit it silently.The chart already supports
access_key_existing_secret/secure_api_token_existing_secretreferences — users pre-create a Secret out-of-band (Vault, External Secrets, SOPS, sealed-secrets) and the chart just references it.What this PR does NOT change
The Secret rendering itself. Inline values continue to work for users who prefer them — this is purely a guidance / observability change.
Changes
templates/NOTES.txt— new conditional block that fires only when at least one credential was supplied inline. Lists the affected key(s), explains the Helm release-metadata exposure, shows the recommended pattern, and links back to [Security] credentials-secret.yaml stores access_key / secure_api_token in Helm release metadata as plaintext (base64); prefer *_existing_secret pattern #2622 for context.values.yaml— stronger comments onaccess_key,access_key_existing_secret,secure_api_token,secure_api_token_existing_secretflagging the trade-off and marking the existing-secret variants as the recommended path for production.README.md— corresponding row updates.tests/common/notes_credentials_warning_test.yaml— 5 helm-unittest cases:Chart.yaml— bump to 1.37.2.Test plan
helm unittest --strict -f tests/common/notes_credentials_warning_test.yaml charts/shield→ 5/5 passedtests/common/credentials-secret_test.yaml→ 4/4 still pass (no rendering change)helm install --dry-runagainsttests/values/base.yaml(inline access_key) → notice rendered with correct bullethelm install --dry-runagainst existing-secret values → notice suppressedSample rendered notice:
Checklist
fix(shield):)tests/with_testsuffix🤖 Generated with Claude Code