Skip to content

Enlighten SignFile for multithreaded mode #13621

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
jankratochvilcz wants to merge 2 commits into
base: main
Choose a base branch
from
+13 −8
Open

Enlighten SignFile for multithreaded mode #13621

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
2ddc5bc
Enlighten SignFile for multithreaded mode
Apr 27, 2026
e78f448
Address SignFile review feedback: preserve original path in error mes…
Apr 27, 2026
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
21 changes: 13 additions & 8 deletions src/Tasks/SignFile.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,11 @@ namespace Microsoft.Build.Tasks
/// It can sign ClickOnce manifests as well as exe's.
/// </summary>
[SupportedOSPlatform("windows")]
public sealed class SignFile : Task
[MSBuildMultiThreadableTask]
public sealed class SignFile : Task, IMultiThreadableTask
{
public TaskEnvironment TaskEnvironment { get; set; } = TaskEnvironment.Fallback;

public SignFile()
: base(AssemblyResources.PrimaryResources, "MSBuild.")
{
Expand All @@ -49,12 +52,14 @@ public override bool Execute()
Log.LogErrorWithCodeFromResources("General.TaskRequiresWindows", nameof(SignFile));
return false;
}
AbsolutePath signingTargetPath = TaskEnvironment.GetAbsolutePath(SigningTarget.ItemSpec);
string SanitizeMessage(string msg) => msg?.Replace((string)signingTargetPath, signingTargetPath.OriginalValue) ?? msg;
try
{
SecurityUtilities.SignFile(
Copy link
Copy Markdown
Member

@JanProvaznik JanProvaznik Apr 28, 2026

Choose a reason for hiding this comment

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

the insides of SecurityUtilities.SignFile depend on process global state

CertificateThumbprint,
TimestampUrl == null ? null : new Uri(TimestampUrl),
SigningTarget.ItemSpec,
signingTargetPath,
TargetFrameworkVersion,
TargetFrameworkIdentifier,
DisallowMansignTimestampFallback);
Comment on lines 59 to 65
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

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

Passing an absolute path into SecurityUtilities.SignFile changes the contents of exceptions thrown by SecurityUtilities (e.g., signtool failure/warning messages include the {path} argument). The task logs ex.Message.Trim() verbatim for MSB3482/MSB3483, so this will now surface the absolute path (and may violate the "no absolute path in user-visible output" requirement). Consider rewriting/normalizing the exception message before logging (e.g., replace the absolutized path with the original input) or emitting a task-local error message that uses the original path string.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

@jankratochvilcz jankratochvilcz Apr 27, 2026

Choose a reason for hiding this comment

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

Good catch — also Sin 2 territory. Addressed in the latest commit by sanitizing ex.Message before logging: any embedded absolutized path is replaced with OriginalValue. The replacement is a no-op when the input was already absolute, so no behavior change for existing callers.

Expand All @@ -65,29 +70,29 @@ public override bool Execute()
Log.LogErrorWithCodeFromResources("SignFile.CertNotInStore");
return false;
}
catch (FileNotFoundException ex)
catch (FileNotFoundException)
{
Log.LogErrorWithCodeFromResources("SignFile.TargetFileNotFound", ex.FileName);
Log.LogErrorWithCodeFromResources("SignFile.TargetFileNotFound", signingTargetPath.OriginalValue);
return false;
}
catch (ApplicationException ex)
{
Log.LogErrorWithCodeFromResources("SignFile.SignToolError", ex.Message.Trim());
Log.LogErrorWithCodeFromResources("SignFile.SignToolError", SanitizeMessage(ex.Message).Trim());
return false;
}
catch (WarningException ex)
{
Log.LogWarningWithCodeFromResources("SignFile.SignToolWarning", ex.Message.Trim());
Log.LogWarningWithCodeFromResources("SignFile.SignToolWarning", SanitizeMessage(ex.Message).Trim());
return true;
}
catch (CryptographicException ex)
{
Log.LogErrorWithCodeFromResources("SignFile.SignToolError", ex.Message.Trim());
Log.LogErrorWithCodeFromResources("SignFile.SignToolError", SanitizeMessage(ex.Message).Trim());
return false;
}
catch (Win32Exception ex)
{
Log.LogErrorWithCodeFromResources("SignFile.SignToolError", ex.Message.Trim());
Log.LogErrorWithCodeFromResources("SignFile.SignToolError", SanitizeMessage(ex.Message).Trim());
return false;
}
catch (UriFormatException ex)
Expand Down
Loading