Harden Windows signing verification in CI

.buildkite/commands/setup_windows_code_signing.ps1

@@ -7,6 +7,68 @@
 # Windows AMI as of a8c-ci-toolkit 6.0.0, so there is no separate host-preparation step.
 
 $ErrorActionPreference = "Stop"
+Set-StrictMode -Version Latest
 
 & "setup_azure_trusted_signing.ps1"
 if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
+
+function Assert-SigningToolIntegrity {
+    param (
+        [Parameter(Mandatory = $true)]
+        [string]$Path,
+        [Parameter(Mandatory = $true)]
+        [string]$Description,
+        [Parameter(Mandatory = $true)]
+        [string]$ExpectedSha256EnvVar
+    )
+
+    if ([string]::IsNullOrWhiteSpace($Path)) {
+        Write-Host "[!] $Description path is not set."
+        exit 1
+    }
+
+    if (-not (Test-Path -LiteralPath $Path)) {
+        Write-Host "[!] $Description was not found at $Path."
+        exit 1
+    }
+
+    $expectedSha256 = [Environment]::GetEnvironmentVariable($ExpectedSha256EnvVar)
+    if (-not [string]::IsNullOrWhiteSpace($expectedSha256)) {
+        $actualSha256 = (Get-FileHash -LiteralPath $Path -Algorithm SHA256).Hash
+        if ($actualSha256 -ne $expectedSha256.Trim().ToUpperInvariant()) {
+            Write-Host "[!] $Description SHA256 mismatch."
+            Write-Host "Expected: $($expectedSha256.Trim().ToUpperInvariant())"
+            Write-Host "Actual:   $actualSha256"
+            exit 1
+        }
+
+        Write-Host "$Description SHA256 matched $ExpectedSha256EnvVar."
+        return
+    }
+
+    $signature = Get-AuthenticodeSignature -FilePath $Path
+    if ($signature.Status -ne "Valid" -or $null -eq $signature.SignerCertificate) {
+        Write-Host "[!] $Description does not have a valid Authenticode signature."
+        Write-Host "Status: $($signature.Status)"
+        exit 1
+    }
+
+    $subject = $signature.SignerCertificate.Subject
+    if ($subject.IndexOf("Microsoft", [System.StringComparison]::OrdinalIgnoreCase) -lt 0) {
+        Write-Host "[!] $Description is not signed by an expected Microsoft certificate."
+        Write-Host "Actual signer subject: $subject"
+        exit 1
+    }
+
+    Write-Host "$Description Authenticode signature is valid: $subject"
+}
+
+Assert-SigningToolIntegrity `
+    -Path $env:SIGNTOOL_PATH `
+    -Description "signtool.exe" `
+    -ExpectedSha256EnvVar "WINDOWS_SIGNTOOL_SHA256"
+
+Assert-SigningToolIntegrity `
+    -Path $env:AZURE_CODE_SIGNING_DLIB `
+    -Description "Azure Trusted Signing DLib" `
+    -ExpectedSha256EnvVar "AZURE_CODE_SIGNING_DLIB_SHA256"

.buildkite/commands/verify_windows_signature.ps1

@@ -0,0 +1,51 @@
+$ErrorActionPreference = "Stop"
+Set-StrictMode -Version Latest
+
+$artifacts = @(Get-ChildItem -Path "dist\*.exe" -File -ErrorAction SilentlyContinue)
+if ($artifacts.Count -eq 0) {
+    Write-Host "[!] No Windows .exe artifacts found in dist."
+    exit 1
+}
+
+if ([string]::IsNullOrWhiteSpace($env:SIGNTOOL_PATH)) {
+    Write-Host "[!] SIGNTOOL_PATH is not set."
+    exit 1
+}
+
+if ([string]::IsNullOrWhiteSpace($env:WINDOWS_EXPECTED_SIGNER_SUBJECT)) {
+    Write-Host "[!] WINDOWS_EXPECTED_SIGNER_SUBJECT must be set to verify the artifact signer identity."
+    exit 1
+}
+
+foreach ($artifact in $artifacts) {
+    Write-Host "Verifying Authenticode signature for $($artifact.FullName)"
+    & $env:SIGNTOOL_PATH verify /pa /v $artifact.FullName
+    if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
+
+    $signature = Get-AuthenticodeSignature -FilePath $artifact.FullName
+    if ($signature.Status -ne "Valid" -or $null -eq $signature.SignerCertificate) {
+        Write-Host "[!] $($artifact.Name) does not have a valid Authenticode signature."
+        Write-Host "Status: $($signature.Status)"
+        exit 1
+    }
+
+    $actualSubject = $signature.SignerCertificate.Subject
+    if ($actualSubject.IndexOf($env:WINDOWS_EXPECTED_SIGNER_SUBJECT.Trim(), [System.StringComparison]::OrdinalIgnoreCase) -lt 0) {
+        Write-Host "[!] $($artifact.Name) was not signed by the expected subject."
+        Write-Host "Expected subject to contain: $($env:WINDOWS_EXPECTED_SIGNER_SUBJECT.Trim())"
+        Write-Host "Actual signer subject:       $actualSubject"
+        exit 1
+    }
+
+    if (-not [string]::IsNullOrWhiteSpace($env:WINDOWS_EXPECTED_SIGNER_ISSUER)) {
+        $actualIssuer = $signature.SignerCertificate.Issuer
+        if ($actualIssuer.IndexOf($env:WINDOWS_EXPECTED_SIGNER_ISSUER.Trim(), [System.StringComparison]::OrdinalIgnoreCase) -lt 0) {
+            Write-Host "[!] $($artifact.Name) was not signed by the expected issuer."
+            Write-Host "Expected issuer to contain: $($env:WINDOWS_EXPECTED_SIGNER_ISSUER.Trim())"
+            Write-Host "Actual signer issuer:       $actualIssuer"
+            exit 1
+        }
+    }
+
+    Write-Host "$($artifact.Name) signer identity verified."
+}

.buildkite/pipeline.yml

@@ -9,28 +9,53 @@ steps:
     agents: { queue: windows }
     plugins: [$CI_TOOLKIT, $NVM_PLUGIN]
     command: |
+      $ErrorActionPreference = "Stop"
+
+      function Invoke-NativeCommand {
+        param (
+          [Parameter(Mandatory = $true)]
+          [string]$FilePath,
+          [Parameter(ValueFromRemainingArguments = $true)]
+          [string[]]$ArgumentList
+        )
+
+        & $FilePath @ArgumentList
+        if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
+      }
+
       echo "~~~ Setup code signing"
       .buildkite/commands/setup_windows_code_signing.ps1
 
       echo "~~~ Install dependencies"
-      bash .buildkite/commands/install_node_dependencies.sh
+      Invoke-NativeCommand bash .buildkite/commands/install_node_dependencies.sh
 
       echo "~~~ Build renderer"
-      npm run build:once
+      Invoke-NativeCommand npm run build:once
 
       echo "~~~ Build Windows artifact"
-      npm run dist:win
+      Invoke-NativeCommand npm run dist:win
 
       echo "~~~ Verify Azure Trusted Signing signature"
-      Get-ChildItem dist\*.exe | ForEach-Object {
-        & $env:SIGNTOOL_PATH verify /pa /v $_.FullName
-        if ($LASTEXITCODE -ne 0) { exit $LASTEXITCODE }
-      }
+      .buildkite/commands/verify_windows_signature.ps1
     artifact_paths:
       - dist\*.exe
     notify:
       - github_commit_status: { context: "Windows Build" }
 
+  ########################################################
+  # Tests
+  ########################################################
+  - label: ":node: Tests"
+    plugins: [$CI_TOOLKIT, $NVM_PLUGIN]
+    command: |
+      echo "~~~ Install dependencies"
+      .buildkite/commands/install_node_dependencies.sh
+
+      echo "~~~ Run tests"
+      npm test
+    notify:
+      - github_commit_status: { context: "Tests" }
+
   ########################################################
   # macOS build
   ########################################################

scripts/azure-sign.cjs

@@ -11,6 +11,8 @@
 const { spawnSync } = require('node:child_process');
 
 const REQUIRED_ENV_VARS = ['SIGNTOOL_PATH', 'AZURE_CODE_SIGNING_DLIB', 'AZURE_METADATA_JSON'];
+const DEFAULT_SIGNTOOL_TIMEOUT_MS = 10 * 60 * 1000;
+const MAX_SIGNTOOL_TIMEOUT_MS = 60 * 60 * 1000;
 
 function nonBlank(value) {
   return typeof value === 'string' && value.trim() !== '' ? value.trim() : undefined;
@@ -20,24 +22,42 @@ function shouldSign(env) {
   return REQUIRED_ENV_VARS.every((name) => nonBlank(env[name]) !== undefined);
 }
 
+function signtoolTimeoutMs(env) {
+  const rawTimeout = nonBlank(env.SIGNTOOL_TIMEOUT);
+  if (rawTimeout === undefined || !/^\d+$/.test(rawTimeout)) {
+    return DEFAULT_SIGNTOOL_TIMEOUT_MS;
+  }
+
+  const timeout = Number.parseInt(rawTimeout, 10);
+  if (!Number.isSafeInteger(timeout) || timeout <= 0 || timeout > MAX_SIGNTOOL_TIMEOUT_MS) {
+    return DEFAULT_SIGNTOOL_TIMEOUT_MS;
+  }
+
+  return timeout;
+}
+
+function debugEnabled(env) {
+  return ['1', 'true', 'yes'].includes((nonBlank(env.AZURE_SIGN_DEBUG) || '').toLowerCase());
+}
+
 function buildSigntoolArgs(file, env) {
   const fileDigest = nonBlank(env.AZURE_FILE_DIGEST) || 'SHA256';
   const timestampServer = nonBlank(env.AZURE_TIMESTAMP_SERVER) || 'http://timestamp.acs.microsoft.com';
   const timestampDigest = nonBlank(env.AZURE_TIMESTAMP_DIGEST) || 'SHA256';
 
-  // `/debug` surfaces Azure auth/quota/network diagnostics on failure instead of a generic
-  // SignTool error.
-  return [
+  const args = [
     'sign',
     '/v',
-    '/debug',
+    ...(debugEnabled(env) ? ['/debug'] : []),
     '/fd', fileDigest,
     '/tr', timestampServer,
     '/td', timestampDigest,
     '/dlib', nonBlank(env.AZURE_CODE_SIGNING_DLIB),
     '/dmdf', nonBlank(env.AZURE_METADATA_JSON),
     file,
   ];
+
+  return args;
 }
 
 module.exports = async function sign(configuration) {
@@ -52,9 +72,16 @@ module.exports = async function sign(configuration) {
   }
 
   console.log(`[azure-sign] Signing ${file} with Azure Trusted Signing`);
-  const result = spawnSync(nonBlank(env.SIGNTOOL_PATH), buildSigntoolArgs(file, env), { stdio: 'inherit' });
+  const timeout = signtoolTimeoutMs(env);
+  const result = spawnSync(nonBlank(env.SIGNTOOL_PATH), buildSigntoolArgs(file, env), {
+    stdio: 'inherit',
+    timeout,
+  });
 
   if (result.error) {
+    if (result.error.code === 'ETIMEDOUT') {
+      throw new Error(`[azure-sign] signtool timed out after ${timeout}ms signing ${file}`);
+    }
     throw result.error;
   }
   if (result.signal) {
@@ -67,3 +94,4 @@ module.exports = async function sign(configuration) {
 
 module.exports.shouldSign = shouldSign;
 module.exports.buildSigntoolArgs = buildSigntoolArgs;
+module.exports.signtoolTimeoutMs = signtoolTimeoutMs;

test/azure-sign.test.cjs

@@ -1,7 +1,7 @@
 const test = require('node:test');
 const assert = require('node:assert/strict');
 
-const { shouldSign, buildSigntoolArgs } = require('../scripts/azure-sign.cjs');
+const { shouldSign, buildSigntoolArgs, signtoolTimeoutMs } = require('../scripts/azure-sign.cjs');
 
 const FULL_ENV = {
   SIGNTOOL_PATH: 'C:/sdk/x64/signtool.exe',
@@ -41,6 +41,14 @@ test('buildSigntoolArgs signs SHA256-only with dlib, metadata, timestamp, and ta
   assert.equal(valueAfter(args, '/dmdf'), FULL_ENV.AZURE_METADATA_JSON);
   assert.equal(args.at(-1), 'app.exe');
   assert.ok(!args.includes('SHA1'));
+  assert.ok(!args.includes('/debug'));
+});
+
+test('buildSigntoolArgs includes debug diagnostics only when enabled', () => {
+  const args = buildSigntoolArgs('app.exe', { ...FULL_ENV, AZURE_SIGN_DEBUG: 'true' });
+
+  assert.ok(args.includes('/debug'));
+  assert.deepEqual(args.slice(0, 6), ['sign', '/v', '/debug', '/fd', 'SHA256', '/tr']);
 });
 
 test('buildSigntoolArgs trims surrounding whitespace from dlib and metadata paths', () => {
@@ -79,3 +87,22 @@ test('buildSigntoolArgs honors the toolkit-exported digest/timestamp overrides',
   assert.equal(valueAfter(args, '/tr'), 'http://ts.example/test');
   assert.equal(valueAfter(args, '/td'), 'SHA512');
 });
+
+test('signtoolTimeoutMs defaults to ten minutes', () => {
+  assert.equal(signtoolTimeoutMs({}), 10 * 60 * 1000);
+});
+
+test('signtoolTimeoutMs honors positive SIGNTOOL_TIMEOUT values', () => {
+  assert.equal(signtoolTimeoutMs({ SIGNTOOL_TIMEOUT: '30000' }), 30000);
+});
+
+test('signtoolTimeoutMs ignores blank, invalid, and zero SIGNTOOL_TIMEOUT values', () => {
+  assert.equal(signtoolTimeoutMs({ SIGNTOOL_TIMEOUT: '   ' }), 10 * 60 * 1000);
+  assert.equal(signtoolTimeoutMs({ SIGNTOOL_TIMEOUT: 'soon' }), 10 * 60 * 1000);
+  assert.equal(signtoolTimeoutMs({ SIGNTOOL_TIMEOUT: '0' }), 10 * 60 * 1000);
+});
+
+test('signtoolTimeoutMs ignores unsafe and over-limit SIGNTOOL_TIMEOUT values', () => {
+  assert.equal(signtoolTimeoutMs({ SIGNTOOL_TIMEOUT: '3600001' }), 10 * 60 * 1000);
+  assert.equal(signtoolTimeoutMs({ SIGNTOOL_TIMEOUT: '999999999999999999999999999999999999999' }), 10 * 60 * 1000);
+});