Conversation
EscapeForBash was called on signArguments inside the batching loop, so each
iteration escaped an already-escaped string. The first batch was fine and
every subsequent one was progressively mangled.
This is not an edge case: a {{file}} template forces parallelism to 1, giving
each file its own batch, so any template containing \ $ ` " or ' fails on
the second file onwards with output like
cp: cannot create regular file '"/tmp/out_\$V.bin"': No such file or directory
A {{file...}} template hits it once the file count exceeds --signParallel.
Escaping is now done once, before the loop. Adds three regression tests, each
of which fails without the fix, and corrects the log message on the
{{file...}} branch that described itself as a single file template.
This branch has not been deployed
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.
--signTemplatecorrupts its own command on Linux/macOS for every file after the first.EscapeForBash(signArguments)is called inside the batching loop, so each iteration escapes an already-escaped string:Batch 1 is correct; batch 2 has been escaped twice, batch 3 three times.
Why this isn't an edge case
A
{{file}}template pinsparallelismto 1, so every file is its own batch. Any template containing\,$,`,"or'— which is most real signing commands, since credentials and URLs get quoted — fails on the second file onwards.A
{{file...}}template hits the same thing once the file count exceeds--signParallel(default 10).Reproduction
Two files,
parallelism: 1, templatecp {{file}} "/tmp/out_$V.bin", driven throughCodeSign.Signon Ubuntu:The destination has picked up literal
"and\$. The first file had already been copied correctly.The fix
Escape once, before the loop. The
filesToSignStrbranch collapses to a ternary since only the quoting differs per platform now.Tests
Three regression tests added to
CodeSignTests, covering the{{file}}multi-batch path, the{{file...}}path with more files than--signParallel, and an end-to-end copy to a quoted destination across two batches.All three fail on
developand pass with the fix:totalfailedsucceededskippedFull
Velopack.Packaging.Testsrun: 86 total, 61 passed, 24 skipped, 1 failed —ResourceEditTests.CommitResourcesInCorrectOrder, which fails identically on an unmodified checkout ofdevelopand is unrelated to this change.Also corrected one log message: the
{{file...}}branch announced itself as "a single file signing template".Found while checking whether Velopack could sign Windows packages from Linux — it can, and does, which is how the multi-batch path got exercised. Disclosure: the signing tool I was driving it with is my own, but nothing here references or depends on it.