Repository navigation
Fix --signTemplate double-escaping every batch after the first on Linux/macOS - #1018
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.
The template used to be escaped character by character and then pushed through
.NET's Windows-style argument parser into `bash -c "..."`. That made single
quotes in a template literal, made $VAR and backticks impossible to use, broke
on file paths containing ' or ", and wrapped "{{file}}" in literal quotes.
The command is now handed to bash verbatim via ProcessStartInfo.ArgumentList,
so a template behaves exactly like the same command typed into bash (matching
how cmd.exe already interprets it on Windows). File paths are single-quoted
with ' written as '\'', and if {{file}} sits inside a "..." or '...' string,
that string is closed around the paths so each path stays one word. Output is
captured with `exec >> log 2>&1`, so every command in a chained template is
logged, not only the last one.
Sign() also returns early when every file was skipped, instead of throwing
from Queue.Dequeue(). The Windows command line is unchanged.
…{file}} for bash
A placeholder inside "$(dirname {{file}})" is unquoted at its own nesting
level, so closing the outer double quote around it handed the command a path
with literal quotes. Also report full progress when every file is skipped.
An apostrophe in a '# comment' line sent the quote tracker into a single-quoted state, leaving later paths unquoted. A CRLF template glued '\r' onto the last path.
|
Thanks for this, good catch. While reviewing it I found the bash escaping had a few other problems (single quotes in a template were passed through literally, The template is now passed to bash as-is, so it behaves the same as typing the command into a terminal, just like cmd.exe on Windows. File paths are quoted safely, and This is a behaviour change on Linux/macOS: if a template has a literal Tested on Windows and Linux, including real signing through |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1018 +/- ##
===========================================
- Coverage 52.13% 52.01% -0.12%
===========================================
Files 257 257
Lines 19645 19691 +46
Branches 1455 1479 +24
===========================================
+ Hits 10242 10243 +1
- Misses 8956 9001 +45
Partials 447 447
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
--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.