fix(envvar): expand only whole-value placeholders in secret config fields#367
Draft
NikolayS wants to merge 2 commits into
Draft
fix(envvar): expand only whole-value placeholders in secret config fields#367NikolayS wants to merge 2 commits into
NikolayS wants to merge 2 commits into
Conversation
Encode whole-value-only expansion semantics: values that merely contain a
$ (passwords, regex backreferences, unmatched braces) must be preserved
verbatim, and only a value that is exactly ${VAR} or $VAR is expanded.
These tests fail against the current os.Expand-based implementation, which
corrupts embedded references and silently truncates malformed placeholders.
Resolve env references only when a field's value is exactly ${VAR} or
$VAR. A value that merely contains a $ (a password, a regex
backreference, an unmatched brace) is now returned verbatim instead of
being passed through os.Expand.
This fixes two issues in secret fields (verificationToken, accessToken,
webhook secret, source token, CLI token):
- literal $ in a secret was reinterpreted as a variable reference,
silently corrupting the secret when the embedded name matched a set
env var, or failing startup when it did not.
- malformed placeholders (${UNCLOSED, ${}) were silently truncated to a
partial string or an empty value instead of being preserved.
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
2 participants
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
envvar.ExpandStrictexpand environment placeholders only when a config field's value is exactly a single${VAR}or$VAR. Any value that merely contains a$(a password, a regex backreference, an unmatched brace) is now returned verbatim.Why
Two confirmed bugs in the env-token placeholder feature (commit 362c7b0), both affecting secret fields (
verificationToken,accessToken, webhooksecret, sourcetoken, CLI token):$in a secret was reinterpreted byos.Expandas a variable reference: silently corrupted when the embedded name matched a set env var, or a hard startup failure otherwise. No escape existed.${UNCLOSED,${}) were silently truncated to a partial string or empty value, contradicting the function's "fail loudly" contract.How
ExpandStrictnow matches the value against^\$\{NAME\}$ | ^\$NAME$. On a match it resolves the variable (unset → loud error, preserving documented startup-safety); otherwise it returns the input unchanged. All intended whole-value placeholders in the example configs and existing tests are unaffected.Tests (red/green TDD)
***$1,pa$$w0rd,tok$en(with a colliding env var set),${UNCLOSED,${},$, and embedded$DBLAB_TOKEN-suffix— these fail against the oldos.Expandimplementation.go vetandgofmtclean.Canonical review happens on the GitLab MR (postgres-ai/database-lab!1158), which closes the two filed work items. This mirror PR exists to run CI and collect review feedback.
Generated by Claude Code