Preserve dollar signs in command substitution output in secrets files - #1911
Preserve dollar signs in command substitution output in secrets files#1911dptsec wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR fixes a subtle but high-impact secrets parsing bug where $... sequences produced by $(...) command substitution in .kamal/secrets* were being treated as variable references and silently stripped by Dotenv’s subsequent variable-substitution pass. The change reorders Dotenv substitutions so variable substitution runs before command substitution, ensuring command output is inserted literally (matching shell expectations) while still allowing variables inside the command text to be expanded.
Changes:
- Reorders Dotenv substitution passes to run
Variablebefore (inline)Commandsubstitution, preventing$corruption in command output. - Adds regression tests covering: literal
$preservation, variable substitution inside command text, and the documentedfetch/extractinline pattern.
Tip
If you aren't ready for review, convert to a draft PR.
Click "Convert to draft" or run gh pr ready --undo.
Click "Ready for review" or run gh pr ready to reengage.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
lib/kamal/secrets/dotenv/inline_command_substitution.rb |
Reorders Dotenv substitutions so command output isn’t re-processed by variable expansion. |
test/secrets_test.rb |
Adds regression tests ensuring $ is preserved and inline kamal secrets extract continues to work. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Problem
Secret values produced by
$(...)command substitution in.kamal/secrets*files are corrupted when they contain$. Dotenv runs its substitutions in the order[Command, Variable], so after a command substitution executes, its output receives a variable-substitution pass: any$wordsequence in the output is treated as a reference to a (usually undefined) variable and silently replaced with an empty string.This affects the officially documented secrets patterns, including adapter fetch/extract:
If the stored password is
pa$1wor$D, the deployed value ispa— the deploy succeeds and the corruption only surfaces later as downstream authentication failures, which makes it very hard to trace back. Passwords and tokens containing$are common.Minimal reproduction (no vault needed):
kamal secrets print→SECRET=paFix
Run variable substitution before command substitution, so command output is inserted literally. This matches POSIX shell semantics, where the result of command substitution is not subject to further expansion.
Variable references inside the command text itself (e.g.
${SECRETS}in the extract pattern above) are unaffected:InlineCommandSubstitution#callalready substitutes variables in the command string before executing it, and the parser-level variable pass now simply runs before commands execute instead of after.The behavior change: command output containing
$FOOis no longer expanded against other secrets/env. Relying on that was indistinguishable from the corruption above, so this should be strictly less surprising.Tests
$is preservedkamal secrets extract ${SECRETS}pattern preserves$in extracted valuesFull suite passes (the two
linux/amd64/linux/arm64builder-test failures on an arm64 host are pre-existing and unrelated).