Probe all config-derived secrets for set/unset in the support bundle#739
Open
ak684 wants to merge 2 commits into
Open
Probe all config-derived secrets for set/unset in the support bundle#739ak684 wants to merge 2 commits into
ak684 wants to merge 2 commits into
Conversation
Replace the 5 hand-written secret collectors with a loop over every secret the chart creates, emitting one includeValue:false probe per key (77 total). Each key renders only when its KOTS value is set, so keyExists reports set/unset without capturing the value -- so a bundle now shows which credentials (e.g. the BBDC bot-token) are configured. Bump chart to 0.7.66.
Contributor
|
🔍 Review in progress… We are performing the review through OpenHands Cloud Automation. You can log in and view the conversation here. |
Drop the always-generated internal secrets (jwt, admin/keycloak passwords, automation-*, plugin-directory, etc.) that always read "set" and add no signal, plus UI_PASSWORD/LITELLM_SALT_KEY which now default. Keeps integration/auth/LLM provider/storage/BYO-TLS creds and the baseline infra probes: 60 probes, still all includeValue:false. Complements #727's env snapshot but works via the k8s API even when the app pod is down.
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.
What
Make the support bundle report which configured secrets are set — without ever capturing a value.
Before, the bundle had 5 secret collectors (
postgres-password,redis,sandbox-api-key,github-app,keycloak-realm), two of which only checked the secret existed — not which keys inside it were populated. So for a customer using an integration, we couldn't tell from the bundle whether a given credential was actually set.This adds per-key
secret+includeValue: falseprobes (via a Helmdict/rangeloop) for the admin-configured / optional secrets — 60 probes total.Why it's a yes/no without leaking anything
Every config-derived secret key is rendered only when its KOTS value is set (
{{- if .Values.config.X }}incharts/openhands-secrets/templates/*). So withincludeValue: false, the collector recordssecretExists/keyExists(booleans) and never the value —keyExists == true⇔ that credential was configured.A future bundle will contain e.g.
secrets/openhands/bitbucket-data-center-app/bot-token.json→{"secretExists":true,"keyExists":false,...}— the exact signal we were missing while debugging a recent BBDC upgrade (we couldn't tell whether the bot token had been set).Scope
Covers secrets whose set/unset reflects an admin choice, so the yes/no is meaningful:
bot-token), jira-dc, azure-devops, gitlab, slack, github, keycloak-realm SMTPlitellm-env-secrets(shows which provider is configured) +openhands-env-secretsopenhands-tls)Intentionally omits the always-generated internal secrets (jwt, admin/keycloak-admin passwords, automation-*, plugin-directory, default-api-key, lite-llm-api-key, and the now-defaulted
UI_PASSWORD/LITELLM_SALT_KEY) — they'd readsetunconditionally and add no signal.Relationship to #727
Complements (doesn't duplicate) the
openhands-config-snapshotenv collector from #727. That oneexecs into the runningopenhandspod and reports set/unset for a curatedSECRET_PRESENCEenv list. This PR instead queries the k8s Secret objects directly, which:execcan't run then);Validation
helm template(valid YAML, deterministic alphabetical order).includeValue: false, zeroincludeValue: true, zerovalue:fields — no secret material can land in a bundle.charts/openhands0.7.64 → 0.7.66(0.7.65 is already a release tag) for the chart-template-version-bump check. NoChart.lock/replicated/pin churn.