Visitar URL original
Fix --signTemplate double-escaping every batch after the first on Linux/macOS by virzak · Pull Request #1018 · velopack/velopack · GitHub
Skip to content

Fix --signTemplate double-escaping every batch after the first on Linux/macOS - #1018

Merged
caesay merged 6 commits into
velopack:developfrom
virzak:fix/signtemplate-double-escape
Oct 10, 2026
Merged

caesay merged 6 commits into
velopack:developfrom
virzak:fix/signtemplate-double-escape

Conversation

@virzak

@virzak virzak commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

--signTemplate corrupts 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:

do {
    ...
    } else {
        filesToSignStr = QuoteFileArgsBash(filesToSign);
        signArguments = EscapeForBash(signArguments);   // <-- re-escapes each batch
    }

Batch 1 is correct; batch 2 has been escaped twice, batch 3 three times.

Why this isn't an edge case

A {{file}} template pins parallelism to 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, template cp {{file}} "/tmp/out_$V.bin", driven through CodeSign.Sign on Ubuntu:

Sign() THREW: UserInfoException: Signing command failed.
Output was:
cp: cannot create regular file '"/tmp/vrepro-912b4c40/out_\$V.bin"': No such file or directory

The destination has picked up literal " and \$. The first file had already been copied correctly.

The fix

Escape once, before the loop. The filesToSignStr branch 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 develop and pass with the fix:

total failed succeeded skipped
without fix 34 3 25 6
with fix 34 0 28 6

Full Velopack.Packaging.Tests run: 86 total, 61 passed, 24 skipped, 1 failed — ResourceEditTests.CommitResourcesInCorrectOrder, which fails identically on an unmodified checkout of develop and 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.

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.
@caesay

caesay commented Oct 10, 2026 •

Copy link
Copy Markdown
Member

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, $VAR couldn't be used, paths containing ' or " broke, "{{file}}" ended up with literal quotes in it), so I pushed some commits on top of yours and merged develop in, since the old base no longer restores.

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 {{file}} works inside quotes or $(...). I also fixed a crash when there are no files left to sign. Nothing changes on Windows.

This is a behaviour change on Linux/macOS: if a template has a literal $ in it (e.g. a password), it now needs single quotes or should come from an env var.

Tested on Windows and Linux, including real signing through vpk pack (signtool on Windows, osslsigncode on Linux).

@codecov

codecov Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 0% with 73 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.01%. Comparing base (f31022e) to head (793ca25).

Files with missing lines Patch % Lines
src/vpk/Velopack.Packaging.Windows/CodeSign.cs 0.00% 73 Missing ⚠️
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              
Flag Coverage Δ
nodejs 1.43% <ø> (ø)
rust 54.80% <ø> (+<0.01%) ⬆️
vpk 33.41% <0.00%> (-0.11%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@caesay
caesay merged commit a7935e6 into velopack:develop Oct 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants