Work with variable references as long as possible (commands promises) - #6345
Open
nickanderson wants to merge 3 commits into
Open
Work with variable references as long as possible (commands promises)#6345nickanderson wants to merge 3 commits into
nickanderson wants to merge 3 commits into
Conversation
Variable expansion has to decide what to do about a secret-tagged variable before the plaintext exists in a buffer, which EvalContextVariableGet() with get_secret=false cannot do: it produces a value either way. This asks the question on its own, straight through VariableResolve(). Ticket: CFE-3298 Changelog: None
ExpandScalar() substitutes the value of every variable it meets, so by the time a promise is evaluated the plaintext of a secret is already in the promiser and in the constraints, and every log line, lock name and report downstream can leak it. ExpandScalarKeepSecrets() leaves "$(name)" in place instead, taking the same path an unresolvable variable takes, and ExpandScalarSecretsOnly() substitutes those references and nothing else. Between the two, the value only ever exists in the string the point of use builds for itself. ExpandScalar() is unchanged, so nothing defers until a caller asks for it. Nested references are left alone when resolving, because the outer name is not known until the inner one is substituted; a secret used as part of a variable name therefore stays unresolved rather than being expanded early. Ticket: CFE-3298 Changelog: None
…ommand The promiser of a commands promise is now expanded with the references to secret-tagged variables left in place, and RepairExec() resolves them only in the string it hands to cf_popen(). The lock name, $(this.promiser), the promise banner, "Executing ...", "Would execute script ..." and the module output all keep the reference, so a secret used in a command no longer reaches the log at inform level, nor the lock database, nor a --dry-run report. The executable check needs the value, since "$(x)" is never a path, so it resolves the promiser too and frees it immediately; its own diagnostics still print the reference. Ordinary variables are unaffected, and a commands promise with no references left in its promiser skips the extra pass entirely. 'args' and 'arglist' are ordinary constraints and still expand eagerly. So does the vars promise's own debug logging, which prints the value at -d; that is a separate call site, covered by CFE-3294. Ticket: CFE-3298 Changelog: Title
nickanderson
force-pushed
the
CFE-3298/master
branch
from
September 3, 2026 16:45
8293e7c to
ed85744
Compare
|
Thank you for submitting a pull request! Maybe @larsewi can review this? |
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.
A commands promise now works with the reference to a secret-tagged variable
instead of its value: the promiser is expanded with
ExpandScalarKeepSecrets(),and
RepairExec()resolves the references only in the string it hands tocf_popen(). The lock name,$(this.promiser), the promise banner,Executing ...,Would execute script ...and the module output all keep$(password), so the value no longer reaches the log at inform level, nor thelock database, nor a
--dry-runreport.Ordinary variables are untouched —
ExpandScalar()behaves exactly as before,and a promiser with no references left skips the extra pass.
argsandarglistare ordinary constraints and still expand eagerly.31_tickets/CFE-3298covers all three shell types and pairs each absent valuewith the reference that replaced it, so a build that expanded nothing fails it
too; disabling
PromiseTypeResolvesSecretsAtUse()turns it red naming bothleaks. No enterprise or nova changes needed.