Skip to content

Do not sign .js files by default #15760

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

Merged
merged 3 commits into from
Apr 22, 2025
Merged

Do not sign .js files by default #15760

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9b59032
Do not sign .js files by default
ellahathaway Apr 18, 2025
2da35cb
Link tracking issue to remove .js signing warning
ellahathaway Apr 21, 2025
2d379d7
Update expected test results
ellahathaway Apr 21, 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
6 changes: 5 additions & 1 deletion src/Microsoft.DotNet.Arcade.Sdk/tools/Sign.props
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@
-->
<FileExtensionSignInfo Include=".deb" CertificateName="LinuxSign" />
<FileExtensionSignInfo Include=".jar" CertificateName="MicrosoftJARSHA2" />
<FileExtensionSignInfo Include=".js;.ps1;.psd1;.psm1;.psc1;.py" CertificateName="Microsoft400" />
<FileExtensionSignInfo Include=".ps1;.psd1;.psm1;.psc1;.py" CertificateName="Microsoft400" />
<FileExtensionSignInfo Include=".dll;.exe;.mibc;.msi" CertificateName="Microsoft400" />
<FileExtensionSignInfo Include=".nupkg" CertificateName="NuGet" />
<FileExtensionSignInfo Include=".vsix" CertificateName="VsixSHA2" />
Expand All @@ -95,6 +95,10 @@
<!-- Runtime hardening is required for notarization. -->
<FileExtensionSignInfo Include=".dylib" CertificateName="MacDeveloperHarden" />

<!-- Removing the default .js signing -->
<FileExtensionSignInfo Include=".js" CertificateName="BreakingSignatureChange" Condition="'$(NoSignJS)' != 'true'"/>
<FileExtensionSignInfo Include=".js" CertificateName="None" Condition="'$(NoSignJS)' == 'true'"/>

<!-- Explicitly use the azure linux cert for azl binaries -->
<AzureLinuxRPM Include="$(ArtifactsPackagesDir)**/*-azl-*.rpm" />
<AzureLinuxRPM Include="$(ArtifactsPackagesDir)**/*-azl.*-*.rpm" />
Expand Down
39 changes: 12 additions & 27 deletions src/Microsoft.DotNet.SignTool.Tests/SignToolTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ public class SignToolTests : IDisposable
// Default extension based signing information
private static readonly Dictionary<string, List<SignInfo>> s_fileExtensionSignInfo = new Dictionary<string, List<SignInfo>>()
{
{".js", new List<SignInfo>{ new SignInfo("JSCertificate") } },
{".jar", new List<SignInfo>{ new SignInfo("JARCertificate") } },
{".ps1", new List<SignInfo>{ new SignInfo("PSCertificate") } },
{".psd1", new List<SignInfo>{ new SignInfo("PSDCertificate") } },
Expand All @@ -53,7 +52,6 @@ public class SignToolTests : IDisposable
private static readonly Dictionary<string, List<SignInfo>> s_fileExtensionSignInfoWithCollisionId =
new Dictionary<string, List<SignInfo>>()
{
{".js", new List<SignInfo>{ new SignInfo("JSCertificate", collisionPriorityId: "123") } },
{".jar", new List<SignInfo>{ new SignInfo("JARCertificate", collisionPriorityId: "123") } },
{ ".ps1", new List<SignInfo>{ new SignInfo("PSCertificate", collisionPriorityId: "123") } },
{ ".psd1", new List<SignInfo>{ new SignInfo("PSDCertificate", collisionPriorityId: "123") } },
Expand Down Expand Up @@ -81,10 +79,6 @@ public class SignToolTests : IDisposable
// Default extension based signing information post build
private static readonly ITaskItem[] s_fileExtensionSignInfoPostBuild = new ITaskItem[]
{
new TaskItem(".js", new Dictionary<string, string> {
{ "CertificateName", "JSCertificate" },
{ SignToolConstants.CollisionPriorityId, "123" }
}),
new TaskItem(".jar", new Dictionary<string, string> {
{ "CertificateName", "JARCertificate" },
{ SignToolConstants.CollisionPriorityId, "123" }
Expand Down Expand Up @@ -145,10 +139,6 @@ public class SignToolTests : IDisposable
{ "CertificateName", "VsixSHA2" },
{ SignToolConstants.CollisionPriorityId, "123" }
}),
new TaskItem(".js", new Dictionary<string, string> {
{ "CertificateName", "JSCertificate" },
{ SignToolConstants.CollisionPriorityId, "234" }
}),
new TaskItem(".jar", new Dictionary<string, string> {
{ "CertificateName", "JARCertificate" },
{ SignToolConstants.CollisionPriorityId, "234" }
Expand Down Expand Up @@ -2381,7 +2371,6 @@ public void CheckFileExtensionSignInfo()
var itemsToSign = new List<ItemToSign>()
{
new ItemToSign(CreateTestResource("dynalib.dylib"), "123"),
new ItemToSign(CreateTestResource("javascript.js"), "123"),
new ItemToSign(CreateTestResource("javatest.jar"), "123"),
new ItemToSign(CreateTestResource("power.ps1"), "123"),
new ItemToSign(CreateTestResource("powerc.psc1"), "123"),
Expand All @@ -2398,7 +2387,6 @@ public void CheckFileExtensionSignInfo()
ValidateFileSignInfos(itemsToSign, strongNameSignInfo, fileSignInfo, s_fileExtensionSignInfoWithCollisionId, new[]
{
"File 'dynalib.dylib' Certificate='DylibCertificate'",
"File 'javascript.js' Certificate='JSCertificate'",
"File 'javatest.jar' Certificate='JARCertificate'",
"File 'power.ps1' Certificate='PSCertificate'",
"File 'powerc.psc1' Certificate='PSCCertificate'",
Expand All @@ -2413,12 +2401,12 @@ public void ValidateParseFileExtensionEntriesForSameCollisionPriorityIdFails()
var fileExtensionSignInfo = new List<ITaskItem>();

// Validate that multiple entries will collide and fail
fileExtensionSignInfo.Add(new TaskItem(".js", new Dictionary<string, string>
fileExtensionSignInfo.Add(new TaskItem(".ps1", new Dictionary<string, string>
{
{ "CertificateName", "JSCertificate" },
{ "CertificateName", "PS1Certificate" },
{ "CollisionPriorityId", "123" }
}));
fileExtensionSignInfo.Add(new TaskItem(".js", new Dictionary<string, string>{
fileExtensionSignInfo.Add(new TaskItem(".ps1", new Dictionary<string, string>{
{ "CertificateName", "None" },
{ "CollisionPriorityId", "123" }
}));
Expand All @@ -2432,17 +2420,17 @@ public void ValidateParseFileExtensionEntriesForDifferentCollisionPriorityIdSucc
var fileExtensionSignInfo = new List<ITaskItem>();

// Validate that multiple entries will collide and fail
fileExtensionSignInfo.Add(new TaskItem(".js", new Dictionary<string, string>
fileExtensionSignInfo.Add(new TaskItem(".ps1", new Dictionary<string, string>
{
{ "CertificateName", "JSCertificate" },
{ "CertificateName", "PS1Certificate" },
{ "CollisionPriorityId", "123" }
}));
fileExtensionSignInfo.Add(new TaskItem(".js", new Dictionary<string, string>{
fileExtensionSignInfo.Add(new TaskItem(".ps1", new Dictionary<string, string>{
{ "CertificateName", "None" }
}));
fileExtensionSignInfo.Add(new TaskItem(".js", new Dictionary<string, string>
fileExtensionSignInfo.Add(new TaskItem(".ps1", new Dictionary<string, string>
{
{ "CertificateName", "JSCertificate" },
{ "CertificateName", "PS1Certificate" },
{ "CollisionPriorityId", "456" }
}));

Expand Down Expand Up @@ -2747,7 +2735,6 @@ public void SpecificFileSignInfos()
// List of files to be considered for signing
var itemsToSign = new List<ItemToSign>()
{
new ItemToSign(CreateTestResource("test.js"), "123"),
new ItemToSign(CreateTestResource("test.jar"), "123"),
new ItemToSign(CreateTestResource("test.ps1"), "123"),
new ItemToSign(CreateTestResource("test.psd1"), "123"),
Expand Down Expand Up @@ -2776,7 +2763,6 @@ public void SpecificFileSignInfos()
// Overriding information
var fileSignInfo = new Dictionary<ExplicitCertificateKey, string>()
{
{ new ExplicitCertificateKey("test.js", collisionPriorityId: "123"), "JSCertificate" },
{ new ExplicitCertificateKey("test.jar", collisionPriorityId: "123"), "JARCertificate" },
{ new ExplicitCertificateKey("test.ps1", collisionPriorityId: "123"), "PS1Certificate" },
{ new ExplicitCertificateKey("test.psd1", collisionPriorityId: "123"), "PSD1Certificate" },
Expand Down Expand Up @@ -2809,7 +2795,6 @@ public void SpecificFileSignInfos()

ValidateFileSignInfos(itemsToSign, strongNameSignInfo, fileSignInfo, s_fileExtensionSignInfo, new[]
{
"File 'test.js' Certificate='JSCertificate'",
"File 'test.jar' Certificate='JARCertificate'",
"File 'test.ps1' Certificate='PS1Certificate'",
"File 'test.psd1' Certificate='PSD1Certificate'",
Expand All @@ -2832,10 +2817,10 @@ public void SpecificFileSignInfos()
expectedWarnings: new[]
{
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "EmptyPKT.dll")}' with Microsoft certificate 'DLLCertificate'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "9", "lib/net461/ProjectOne.dll")}' with Microsoft certificate 'DLLCertificate3'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "10", "lib/netstandard2.0/ProjectOne.dll")}' with Microsoft certificate 'DLLCertificate4'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "16", "Contents/Common7/IDE/PrivateAssemblies/ProjectOne.dll")}' with Microsoft certificate 'DLLCertificate5'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "23", "Simple.dll")}' with Microsoft certificate 'DLLCertificate2'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "8", "lib/net461/ProjectOne.dll")}' with Microsoft certificate 'DLLCertificate3'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "9", "lib/netstandard2.0/ProjectOne.dll")}' with Microsoft certificate 'DLLCertificate4'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "15", "Contents/Common7/IDE/PrivateAssemblies/ProjectOne.dll")}' with Microsoft certificate 'DLLCertificate5'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "ContainerSigning", "22", "Simple.dll")}' with Microsoft certificate 'DLLCertificate2'. The library is considered 3rd party library due to its copyright: ''.",
$@"SIGN004: Signing 3rd party library '{Path.Combine(_tmpDir, "Simple.exe")}' with Microsoft certificate 'MacDeveloperHarden'. The library is considered 3rd party library due to its copyright: ''."
});
}
Expand Down
9 changes: 9 additions & 0 deletions src/Microsoft.DotNet.SignTool/src/Configuration.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -481,6 +481,15 @@ private FileSignInfo ExtractSignInfo(
}
}

// Tracking issue to remove this warning - https://github.com/dotnet/arcade/issues/15761
if (signInfo.Certificate != null && signInfo.Certificate.Equals("BreakingSignatureChange", StringComparison.OrdinalIgnoreCase))
{
_log.LogWarning($"Skipping file '{file.FullPath}' because .js files are no longer signed by default. " +
"To disable this warning, please explicitly define the FileExtensionSignInfo for the .js extension " +
"or set the MSBuild property 'NoSignJS' to 'true'.");
Copy link
Member

Choose a reason for hiding this comment

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

Can you open an issue to remove this breaking change and make NoSignJS the default, and link it in a comment here?

Ideally we won't leave this option around long.

Copy link
Member Author

@ellahathaway ellahathaway Apr 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Yes - linked the issue in 2da35cb

return new FileSignInfo(file, SignInfo.Ignore, wixContentFilePath: wixContentFilePath);
}

// If the file is already signed and we are not allowed to dual sign, and we are not doing a mac notarization operation,
// then we should not sign the file.
if (isAlreadyAuthenticodeSigned && !dualCertsAllowed && string.IsNullOrEmpty(macNotarizationAppName))
Expand Down
Loading