Skip to content

Make sure that the signing of .js and .vbs files with the AzureSignToolSigner uses STA context. #881

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Make sure that the signing of .js and .vbs files with the AzureSignToolSigner uses STA context. #881

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6e81ca
Make sure that the signing of .js and .vbs files with the AzureSignTo…
dlemstra May 23, 2025
4504884
Added .js as a supported extensions.
dlemstra May 24, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 41 additions & 12 deletions src/Sign.Core/DataFormatSigners/AzureSignToolSigner.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ namespace Sign.Core
{
internal sealed class AzureSignToolSigner : IAzureSignToolDataFormatSigner
{
private static readonly string[] StaThreadExtensions = [".js", ".vbs"];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparisons should be case-insensitive.

Suggested change
private static readonly string[] StaThreadExtensions = [".js", ".vbs"];
private static readonly HashSet<string> StaThreadExtensions = new(StringComparer.OrdinalIgnoreCase)
{
".js",
".vbs"
};


private readonly ICertificateProvider _certificateProvider;
private readonly ISignatureAlgorithmProvider _signatureAlgorithmProvider;
private readonly ILogger<IDataFormatSigner> _logger;
Expand Down Expand Up @@ -51,6 +53,7 @@ public AzureSignToolSigner(
".emsix",
".emsixbundle",
".exe",
".js",
".msi",
".msix",
".msixbundle",
Expand Down Expand Up @@ -113,25 +116,23 @@ public async Task SignAsync(IEnumerable<FileInfo> files, SignOptions options)
options.FileHashAlgorithm,
timestampConfiguration))
{
FileInfo[] staThreadFiles = [.. files.Where(file => StaThreadExtensions.Contains(file.Extension))];
foreach (FileInfo file in staThreadFiles)
{
await RunOnStaThread(() => SignAsync(signer, file, options));
Copy link
Collaborator

@dtivel dtivel Jul 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works because the actual signing operation happens to execute synchronously before any await calls. SignAsync(...) does contain an await Thread.Delay(...), but it only executes on retry. With the current implementation, only the first try has any chance of success while subsequent retries are guaranteed to fail because execution will resume on the thread pool (MTA).

The proper fix involves setting a synchronization context on the STA thread so that all continuations resume on the STA thread.

https://devblogs.microsoft.com/dotnet/await-synchronizationcontext-and-console-apps/
https://devblogs.microsoft.com/dotnet/await-synchronizationcontext-and-console-apps-part-2/

}

// loop through all of the files here, looking for appx/eappx
// mark each as being signed and strip appx
await Parallel.ForEachAsync(files, async (file, state) =>
Copy link
Collaborator

@dtivel dtivel Jul 27, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested the fix by signing a single .js file. The STA signing above succeeded, but this failed because it tries to sign all files on an MTA thread, which was the original problem.

This statement should sign all files that don't have a file extension in StaThreadExtensions.

We should have test coverage.

{
if (!await SignAsync(signer, file, options))
{
string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, file.FullName);

throw new SigningException(message);
}
await SignAsync(signer, file, options);
});
}
}

// Inspired from https://github.com/squaredup/bettersigntool/blob/master/bettersigntool/bettersigntool/SignCommand.cs
private async Task<bool> SignAsync(
AuthenticodeKeyVaultSigner signer,
FileInfo file,
SignOptions options)
private async Task SignAsync(AuthenticodeKeyVaultSigner signer, FileInfo file, SignOptions options)
{
TimeSpan retry = TimeSpan.FromSeconds(5);
const int maxAttempts = 3;
Expand All @@ -148,7 +149,7 @@ private async Task<bool> SignAsync(

if (RunSignTool(signer, file, options))
{
return true;
return;
}

++attempt;
Expand All @@ -157,7 +158,9 @@ private async Task<bool> SignAsync(

_logger.LogError(Resources.SigningFailedAfterAllAttempts);

return false;
string message = string.Format(CultureInfo.CurrentCulture, Resources.SigningFailed, file.FullName);

throw new SigningException(message);
}

private bool RunSignTool(AuthenticodeKeyVaultSigner signer, FileInfo file, SignOptions options)
Expand Down Expand Up @@ -198,5 +201,31 @@ private bool RunSignTool(AuthenticodeKeyVaultSigner signer, FileInfo file, SignO

return false;
}

private static Task<bool> RunOnStaThread(Func<Task> func)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
private static Task<bool> RunOnStaThread(Func<Task> func)
private static Task<bool> RunOnStaThreadAsync(Func<Task> func)

{
if (!OperatingSystem.IsWindows())
{
throw new NotSupportedException();
}

TaskCompletionSource<bool> taskCompletionSource = new();
var thread = new Thread(async () =>
{
try
{
await func();
taskCompletionSource.SetResult(true);
}
catch (Exception exception)
{
taskCompletionSource.SetException(exception);
}
});

thread.SetApartmentState(ApartmentState.STA);
thread.Start();
return taskCompletionSource.Task;
}
}
}