From 88702805104263d405fba4374930346197e907fe Mon Sep 17 00:00:00 2001 From: NeuroKoder3 Date: Sat, 1 Aug 2026 19:32:13 -0500 Subject: [PATCH 1/4] fix(release): make signing fail closed and verify the artifact, not the filename Four defects meant a release could be believed signed when it was not. Both signing hooks warned and returned whenever a credential was missing. That is correct for a developer build and dangerous for a release: the build went green, the warning scrolled past in electron-builder's output, and the first contradiction arrived on a customer's machine. When a build is designated for distribution -- TRANSTRACK_RELEASE_CHANNEL=public, which release.yml now sets on both jobs, or the explicit REQUIRE_ flags -- a missing credential is now a build failure that names the variable. The release gate accepted any file matching the installer's expected name as proof of signing. verify-artifact-signature.mjs reads the artifact instead: the OS verdict on Windows, the PE Attribute Certificate Table elsewhere, with the weaker check labelled as reduced assurance rather than overstated. A catalog-only signature is rejected -- Windows calls it valid, but it lives outside the file and cannot travel with a download. Neither CI signing mode could have worked. pfx treated CSC_LINK strictly as a path, but a certificate in a CI secret is base64 bytes; it is now written to a temporary file with owner-only permissions and removed in a finally block. ssl_esigner had no ESIGNER_TOOL_PATH and no CodeSignTool on the runner. The existing signWin suite never awaited its async cases, so every assert.rejects counted as a pass without running -- the error paths had never executed. The harness is now async-aware, and two suites join it: notarize, and artifactSignature, which parses synthetic PE images on every platform and on Windows checks a genuinely signed binary and a catalog-signed one. Co-authored-by: Cursor --- .github/workflows/release.yml | 63 ++++++ scripts/notarize.cjs | 87 ++++++-- scripts/release-readiness-check.mjs | 16 +- scripts/run-test-suites.cjs | 4 + scripts/sign-win.cjs | 197 +++++++++++++++--- scripts/verify-artifact-signature.mjs | 199 ++++++++++++++++++ tests/artifactSignature.test.mjs | 198 ++++++++++++++++++ tests/notarize.test.cjs | 172 +++++++++++++++ tests/signWin.test.cjs | 287 +++++++++++++++++++++----- 9 files changed, 1124 insertions(+), 99 deletions(-) create mode 100644 scripts/verify-artifact-signature.mjs create mode 100644 tests/artifactSignature.test.mjs create mode 100644 tests/notarize.test.cjs diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 5e4bc62..b013885 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -123,11 +123,18 @@ jobs: needs: preflight runs-on: windows-latest env: + # Marks this as a release build. The signer and the notarization hook both + # read it and refuse to produce an unsigned artifact, rather than warning + # and carrying on as they do for developer builds. + TRANSTRACK_RELEASE_CHANNEL: public TRANSTRACK_SIGN_MODE: ${{ needs.preflight.outputs.windows_mode }} ESIGNER_USERNAME: ${{ secrets.ESIGNER_USERNAME }} ESIGNER_PASSWORD: ${{ secrets.ESIGNER_PASSWORD }} ESIGNER_CREDENTIAL_ID: ${{ secrets.ESIGNER_CREDENTIAL_ID }} ESIGNER_TOTP_SECRET: ${{ secrets.ESIGNER_TOTP_SECRET }} + # CodeSignTool is not present on the runner image; the step below installs + # it here. Overridable for self-hosted runners that pre-provision it. + ESIGNER_TOOL_PATH: ${{ vars.ESIGNER_TOOL_PATH || 'D:\CodeSignTool\CodeSignTool.bat' }} CSC_LINK: ${{ secrets.CSC_LINK }} CSC_KEY_PASSWORD: ${{ secrets.CSC_KEY_PASSWORD }} steps: @@ -138,6 +145,35 @@ jobs: node-version: '22' cache: 'npm' + # Without this the eSigner path cannot work: the signer resolves + # ESIGNER_TOOL_PATH and the runner image has no CodeSignTool on it, so the + # build failed part-way through with "not found: undefined". + - name: Install SSL.com CodeSignTool + if: needs.preflight.outputs.windows_mode == 'ssl_esigner' + shell: pwsh + run: | + $dest = Split-Path -Parent $env:ESIGNER_TOOL_PATH + if (Test-Path $env:ESIGNER_TOOL_PATH) { + Write-Host "CodeSignTool already present at $env:ESIGNER_TOOL_PATH" + exit 0 + } + if (-not $env:ESIGNER_TOOL_URL) { + Write-Host "::error::ssl_esigner mode selected but CodeSignTool is not installed and no ESIGNER_TOOL_URL repository variable is set." + Write-Host "::error::Set the ESIGNER_TOOL_URL variable to the CodeSignTool zip from your SSL.com dashboard, or set ESIGNER_TOOL_PATH on a self-hosted runner that already has it." + exit 1 + } + New-Item -ItemType Directory -Force -Path $dest | Out-Null + $zip = Join-Path $env:RUNNER_TEMP 'codesigntool.zip' + Invoke-WebRequest -Uri $env:ESIGNER_TOOL_URL -OutFile $zip + Expand-Archive -Path $zip -DestinationPath $dest -Force + if (-not (Test-Path $env:ESIGNER_TOOL_PATH)) { + Write-Host "::error::CodeSignTool was extracted to $dest but $env:ESIGNER_TOOL_PATH does not exist. Check the archive layout and adjust the ESIGNER_TOOL_PATH variable." + Get-ChildItem -Recurse $dest | Select-Object -First 40 | ForEach-Object { Write-Host $_.FullName } + exit 1 + } + env: + ESIGNER_TOOL_URL: ${{ vars.ESIGNER_TOOL_URL }} + - name: Install npm dependencies run: npm ci @@ -147,6 +183,16 @@ jobs: - name: Build & sign Windows installer (electron-builder) run: npm run dist:win:enterprise + # electron-builder reports success whether or not the hook signed + # anything, so the artifact is inspected rather than assumed. This reads + # the PE certificate table and asks Windows for the trust verdict. + - name: Verify the installer is actually signed + shell: pwsh + run: | + $exe = Get-ChildItem release/enterprise/*.exe | Select-Object -First 1 + if (-not $exe) { Write-Host "::error::No installer produced"; exit 1 } + node scripts/verify-artifact-signature.mjs $exe.FullName + - name: Upload installer uses: actions/upload-artifact@v7 with: @@ -159,9 +205,14 @@ jobs: needs: preflight runs-on: macos-latest env: + # See build-windows: makes an un-notarized artifact a build failure rather + # than a warning that scrolls past. + TRANSTRACK_RELEASE_CHANNEL: public APPLE_ID: ${{ secrets.APPLE_ID }} APPLE_APP_PASSWORD: ${{ secrets.APPLE_APP_PASSWORD }} APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + # electron-builder accepts either a path or base64 content here, and a + # secret can only hold the latter. CSC_LINK: ${{ secrets.APPLE_CERT_BASE64 }} CSC_KEY_PASSWORD: ${{ secrets.APPLE_CERT_PASSWORD }} steps: @@ -181,6 +232,18 @@ jobs: - name: Build, sign & notarize macOS DMG run: npm run dist:mac:enterprise + # Gatekeeper's own verdict, rather than trusting that the afterSign hook + # ran. `source=Notarized Developer ID` is the string that matters. + - name: Verify the app is signed and notarized + run: | + app=$(find release/enterprise -maxdepth 3 -name '*.app' | head -n 1) + if [ -z "$app" ]; then echo "::error::No .app produced"; exit 1; fi + codesign -dv --verbose=4 "$app" 2>&1 | sed 's/^/ /' + if ! spctl -a -vv "$app" 2>&1 | tee /dev/stderr | grep -q 'source=Notarized Developer ID'; then + echo "::error::$app is not notarized (spctl did not report a Notarized Developer ID source)" + exit 1 + fi + - name: Upload DMG uses: actions/upload-artifact@v7 with: diff --git a/scripts/notarize.cjs b/scripts/notarize.cjs index 7f04f56..b41d2ac 100644 --- a/scripts/notarize.cjs +++ b/scripts/notarize.cjs @@ -1,34 +1,92 @@ /** - * macOS Notarization Script for electron-builder afterSign hook. + * macOS notarization — electron-builder `afterSign` hook. * * Required environment variables: * APPLE_ID – Apple Developer account email - * APPLE_APP_PASSWORD – App-specific password (not account password) + * APPLE_APP_PASSWORD – app-specific password (NOT the account password) * APPLE_TEAM_ID – 10-character Team ID * - * Skipped automatically on non-macOS platforms and when env vars are absent. + * Always skipped on non-macOS platforms. + * + * On a release build every other skip is an error. This hook used to warn and + * return whenever a variable was missing or `@electron/notarize` was absent, + * which meant the most likely way to ship an un-notarized DMG was to believe + * you had notarized it: the build succeeded, the log line scrolled past, and + * Gatekeeper rejected the download on the customer's machine. A missing + * credential is now fatal when the build is a release, and only a developer + * convenience otherwise. + * + * The variable name has caught people out too. `docs/DEPLOYMENT_PRODUCTION.md` + * previously said APPLE_APP_SPECIFIC_PASSWORD, which is what Apple calls the + * thing but not what this reads. If that spelling is present and the expected + * one is not, the mistake is named rather than silently treated as absent. */ 'use strict'; +const REQUIRED = ['APPLE_ID', 'APPLE_APP_PASSWORD', 'APPLE_TEAM_ID']; + +/** Mistaken spellings that are worth naming instead of reporting as "not set". */ +const ALIASES = Object.freeze({ + APPLE_APP_PASSWORD: ['APPLE_APP_SPECIFIC_PASSWORD', 'APPLE_ID_PASSWORD'], + APPLE_TEAM_ID: ['APPLE_TEAMID'], +}); + +function notarizationRequired(env = process.env) { + if (env.TRANSTRACK_RELEASE_CHANNEL === 'public') return true; + const explicit = String(env.TRANSTRACK_REQUIRE_NOTARIZATION || '').toLowerCase(); + return explicit === '1' || explicit === 'true'; +} + +/** + * @returns {{ missing: string[], hints: string[] }} + */ +function inspectCredentials(env = process.env) { + const missing = REQUIRED.filter((v) => !env[v]); + const hints = []; + for (const name of missing) { + for (const alias of ALIASES[name] || []) { + if (env[alias]) { + hints.push(`${alias} is set but this hook reads ${name} — rename it.`); + } + } + } + return { missing, hints }; +} + exports.default = async function notarizing(context) { const { electronPlatformName, appOutDir } = context; if (electronPlatformName !== 'darwin') return; + const required = notarizationRequired(); + let notarize; try { notarize = require('@electron/notarize').notarize; } catch { - console.warn('Skipping notarization: @electron/notarize not installed'); + const msg = '@electron/notarize is not installed'; + if (required) { + throw new Error( + `Cannot notarize: ${msg}. This build is a release, so an un-notarized ` + + `artifact is not acceptable. Run: npm install --save-dev @electron/notarize`, + ); + } + console.warn(`Skipping notarization: ${msg}`); return; } - const appleId = process.env.APPLE_ID; - const appleIdPassword = process.env.APPLE_APP_PASSWORD; - const teamId = process.env.APPLE_TEAM_ID; - - if (!appleId || !appleIdPassword || !teamId) { - console.warn('Skipping notarization: APPLE_ID, APPLE_APP_PASSWORD, or APPLE_TEAM_ID not set'); + const { missing, hints } = inspectCredentials(); + if (missing.length > 0) { + const detail = [`${missing.join(', ')} not set`, ...hints].join('. '); + if (required) { + throw new Error( + `Cannot notarize: ${detail}. This build is a release, so it must not ` + + `produce an un-notarized artifact. See docs/CODE_SIGNING.md. To build ` + + `without notarization deliberately, unset TRANSTRACK_RELEASE_CHANNEL ` + + `and TRANSTRACK_REQUIRE_NOTARIZATION.`, + ); + } + console.warn(`Skipping notarization: ${detail}`); return; } @@ -39,10 +97,13 @@ exports.default = async function notarizing(context) { await notarize({ appBundleId: context.packager.config.appId, appPath: `${appOutDir}/${appName}.app`, - appleId, - appleIdPassword, - teamId, + appleId: process.env.APPLE_ID, + appleIdPassword: process.env.APPLE_APP_PASSWORD, + teamId: process.env.APPLE_TEAM_ID, }); console.log('Notarization complete.'); }; + +// Exported for unit tests; not part of the electron-builder contract. +exports.__testing__ = { notarizationRequired, inspectCredentials, REQUIRED, ALIASES }; diff --git a/scripts/release-readiness-check.mjs b/scripts/release-readiness-check.mjs index 6f64e3b..a1ea5bf 100644 --- a/scripts/release-readiness-check.mjs +++ b/scripts/release-readiness-check.mjs @@ -19,6 +19,7 @@ import { spawnSync } from 'node:child_process'; import { existsSync, readFileSync, statSync, readdirSync } from 'node:fs'; import { resolve, dirname } from 'node:path'; import { fileURLToPath } from 'node:url'; +import { inspectWindowsArtifact } from './verify-artifact-signature.mjs'; const __dirname = dirname(fileURLToPath(import.meta.url)); const repoRoot = resolve(__dirname, '..'); @@ -296,7 +297,20 @@ await runStep('Code-signed Windows installer present (release/enterprise)', sign 'rebuild before release so the signed artifact matches the tested source', ); } - return `${newest.f} (${(statSync(resolve(dir, newest.f)).size / 1024 / 1024).toFixed(1)} MB)`; + + // Look inside the file. This step is named for code signing, but until now it + // only matched a filename — an entirely unsigned installer called the right + // thing passed it. On Windows the signature is checked for validity; on Linux + // (where the release gate job runs) the PE certificate table is parsed + // directly, which proves a signature is embedded without needing any tooling. + const artifact = resolve(dir, newest.f); + const sig = inspectWindowsArtifact(artifact); + if (!sig.signed) { + throw new Error(`${newest.f} is not signed: ${sig.detail}`); + } + + const sizeMb = (statSync(artifact).size / 1024 / 1024).toFixed(1); + return `${newest.f} (${sizeMb} MB) — ${sig.detail}`; }); await runStep('Windows code-signing configured (any supported mode)', signingSeverity, () => { diff --git a/scripts/run-test-suites.cjs b/scripts/run-test-suites.cjs index 84e5dea..a40ba9e 100644 --- a/scripts/run-test-suites.cjs +++ b/scripts/run-test-suites.cjs @@ -100,6 +100,10 @@ const FUNCTIONAL_SUITES = [ // The validation package is a deliverable; its cross-references are checked // on the same cadence as the code they describe. 'complianceDocs.test.mjs', + // Release signing: the evidence that a shipped artifact is actually signed, + // and that a release build refuses to produce one that is not. + 'artifactSignature.test.mjs', + 'notarize.test.cjs', ]; const GROUPS = { diff --git a/scripts/sign-win.cjs b/scripts/sign-win.cjs index 5d694a6..84bb416 100644 --- a/scripts/sign-win.cjs +++ b/scripts/sign-win.cjs @@ -16,20 +16,22 @@ * ESIGNER_TOOL_PATH - absolute path to CodeSignTool.bat (or .sh on linux/mac) * * MODE 2 TRANSTRACK_SIGN_MODE=pfx - * Local .pfx file (works for OV certificates that ship as a file - * and for EV certs exported into a software-protected PFX). - * Required env vars: - * CSC_LINK - absolute path to the .pfx file + * A PKCS#12 certificate held as a file. Required env vars: + * CSC_LINK - path to the .pfx, OR its base64 contents + * (CI secrets carry the bytes, not a path) * CSC_KEY_PASSWORD - PFX export password * * MODE 3 TRANSTRACK_SIGN_MODE=skip - * No-op. Used for unsigned local development builds. The artifact - * will still be produced but Windows SmartScreen will block it on - * any machine other than the build machine. Never use for release. + * No-op. Used for unsigned local development builds. The artifact is + * still produced but arrives unverifiable on any other machine, so + * Windows warns the user before it will run. Never use for release — + * and on a release build (see _signingRequired) this mode is refused + * rather than warned about. * * Auto-detect: when TRANSTRACK_SIGN_MODE is unset, the script picks the * first mode for which all required env vars are present, in the order - * ssl_esigner -> pfx -> skip. + * ssl_esigner -> pfx -> skip. A mode named *explicitly* whose variables are + * incomplete is an error, not a reason to fall through to skip. * * The script accepts the file-to-sign path as the first argv after node / * the script itself, OR as `process.env.SIGNTOOL_PATH` (electron-builder @@ -45,6 +47,7 @@ const child_process = require('child_process'); const fs = require('fs'); +const os = require('os'); const path = require('path'); const MODE = (process.env.TRANSTRACK_SIGN_MODE || _autoDetectMode()).toLowerCase(); @@ -68,6 +71,104 @@ function _autoDetectMode() { function _log(msg) { process.stdout.write(`[sign-win] ${msg}\n`); } function _warn(msg) { process.stderr.write(`[sign-win] WARN ${msg}\n`); } +/** + * Is an unsigned artifact an error rather than a warning? + * + * The dangerous failure here is the quiet one. A developer build that comes out + * unsigned is fine and expected; a *release* build that comes out unsigned while + * the pipeline reports success is how an unsigned installer reaches a hospital. + * Warning-and-continuing is the right behaviour for the first case and + * indefensible for the second, so the two cases are distinguished explicitly + * rather than by hoping whoever ran the build read stderr. + * + * `TRANSTRACK_RELEASE_CHANNEL=public` is the same signal the release gate uses + * to promote signing checks to mandatory, so the build and the gate cannot + * disagree about whether a given run is a release. + */ +function _signingRequired() { + if (process.env.TRANSTRACK_RELEASE_CHANNEL === 'public') return true; + const explicit = String(process.env.TRANSTRACK_REQUIRE_SIGNING || '').toLowerCase(); + return explicit === '1' || explicit === 'true'; +} + +const MODE_REQUIREMENTS = Object.freeze({ + ssl_esigner: [ + 'ESIGNER_USERNAME', + 'ESIGNER_PASSWORD', + 'ESIGNER_CREDENTIAL_ID', + 'ESIGNER_TOTP_SECRET', + 'ESIGNER_TOOL_PATH', + ], + pfx: ['CSC_LINK', 'CSC_KEY_PASSWORD'], + skip: [], +}); + +/** + * Fail before doing any work if the selected mode is missing a variable. + * + * Without this, `ESIGNER_TOOL_PATH` being unset surfaces as + * "ESIGNER_TOOL_PATH not found: undefined" from an existsSync deep in the + * signing call, part-way through a long build. Naming the missing variable up + * front turns a confusing late failure into an obvious early one. + */ +function _assertModeConfigured(mode) { + const required = MODE_REQUIREMENTS[mode]; + if (!required) throw new Error(`Unknown TRANSTRACK_SIGN_MODE: ${mode}`); + const missing = required.filter((v) => !process.env[v]); + if (missing.length > 0) { + throw new Error( + `TRANSTRACK_SIGN_MODE=${mode} but ${missing.join(', ')} ` + + `${missing.length === 1 ? 'is' : 'are'} not set. ` + + `See docs/CODE_SIGNING.md for the full variable set for this mode.`, + ); + } +} + +/** + * Resolve CSC_LINK to a certificate file on disk. + * + * CSC_LINK is conventionally either a path or the base64 content of the .p12 / + * .pfx, and in CI it can only ever be the latter: a secret store holds bytes, + * not files, so no path a secret could contain would exist on the runner. The + * previous implementation accepted only a path, which made pfx mode + * unreachable from CI and produced the memorable error + * "CSC_LINK not found: MIIKfAIBAzCCCjIGCSqGSIb3..." for anyone who tried. + * + * Returns a cleanup function; the caller must invoke it. The temporary copy is + * a private key, so it is written under a 0600 file in a 0700 directory and + * removed in a finally block. + */ +function _materializeCertificate(cscLink) { + if (fs.existsSync(cscLink)) { + return { file: cscLink, cleanup: () => {} }; + } + + const der = Buffer.from(cscLink, 'base64'); + // A PKCS#12 file is DER: it begins with a SEQUENCE tag. Checking this + // distinguishes "base64 of a real certificate" from "a path that is simply + // wrong", so a typo'd path does not get reported as a corrupt certificate. + if (der.length < 64 || der[0] !== 0x30) { + throw new Error( + `CSC_LINK is neither an existing file nor base64-encoded PKCS#12 content. ` + + `In CI, set it to the base64 of your .pfx/.p12 ` + + `(PowerShell: [Convert]::ToBase64String([IO.File]::ReadAllBytes('cert.pfx'))).`, + ); + } + + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'tt-csc-')); + const file = path.join(dir, 'certificate.pfx'); + fs.writeFileSync(file, der, { mode: 0o600 }); + _log('Materialised certificate from base64 CSC_LINK into a temporary file'); + + return { + file, + cleanup: () => { + try { fs.rmSync(dir, { recursive: true, force: true }); } + catch (e) { _warn(`could not remove temporary certificate directory: ${e.message}`); } + }, + }; +} + function _resolveFilePath(input) { // electron-builder@26 may pass a string OR a {path} object. if (!input) return null; @@ -107,33 +208,37 @@ function _runSslEsigner(filePath) { } function _runPfxSign(filePath) { - const pfx = process.env.CSC_LINK; + const cert = _materializeCertificate(process.env.CSC_LINK); const pfxPwd = process.env.CSC_KEY_PASSWORD; - if (!fs.existsSync(pfx)) { - throw new Error(`CSC_LINK not found: ${pfx}`); - } - // Use the Windows SDK's signtool from PATH. CI runners (GitHub Actions - // windows-latest) ship with it; locally, install via the Windows 10/11 SDK. - const args = [ - 'sign', - '/fd', 'sha256', - '/td', 'sha256', - '/tr', process.env.SIGN_TIMESTAMP_URL || 'http://timestamp.sectigo.com', - '/f', pfx, - '/p', pfxPwd, - filePath, - ]; - _log(`Signing via signtool/PFX: ${path.basename(filePath)}`); - const result = child_process.spawnSync('signtool', args, { - stdio: ['ignore', 'pipe', 'pipe'], - shell: true, - }); - if (result.status !== 0) { - process.stderr.write(result.stderr?.toString() || ''); - throw new Error(`signtool failed (exit ${result.status})`); + try { + // Use the Windows SDK's signtool from PATH. CI runners (GitHub Actions + // windows-latest) ship with it; locally, install via the Windows 10/11 SDK. + const args = [ + 'sign', + '/fd', 'sha256', + '/td', 'sha256', + '/tr', process.env.SIGN_TIMESTAMP_URL || 'http://timestamp.sectigo.com', + '/f', cert.file, + '/p', pfxPwd, + filePath, + ]; + _log(`Signing via signtool/PFX: ${path.basename(filePath)}`); + const result = child_process.spawnSync('signtool', args, { + stdio: ['ignore', 'pipe', 'pipe'], + shell: true, + }); + if (result.status !== 0) { + // signtool echoes the /p value in some diagnostics; the password is the + // one thing that must not reach a build log. + const stderr = (result.stderr?.toString() || '').split(pfxPwd).join('***'); + process.stderr.write(stderr); + throw new Error(`signtool failed (exit ${result.status})`); + } + process.stdout.write(result.stdout?.toString() || ''); + _log(`Signed (PFX): ${path.basename(filePath)}`); + } finally { + cert.cleanup(); } - process.stdout.write(result.stdout?.toString() || ''); - _log(`Signed (PFX): ${path.basename(filePath)}`); } function _generateTotp(base32Secret) { @@ -174,10 +279,29 @@ function _base32Decode(input) { async function sign(configuration) { const filePath = _resolveFilePath(configuration); if (!filePath) { + // A missing path with signing required means electron-builder called the + // hook in a shape we do not understand — silently producing an unsigned + // release artifact is not an acceptable response to that. + if (_signingRequired()) { + throw new Error( + 'No file path provided to the signer, and signing is required for this build. ' + + 'The electron-builder hook contract may have changed.', + ); + } _warn('No file path provided to signer; skipping'); return; } + if (MODE === 'skip') { + if (_signingRequired()) { + throw new Error( + `Signing is required for this build but no credentials are configured, so ` + + `"${path.basename(filePath)}" would be UNSIGNED. Set the variables for one of ` + + `the supported modes (${Object.keys(MODE_REQUIREMENTS).filter((m) => m !== 'skip').join(', ')}) ` + + `— see docs/CODE_SIGNING.md. To build unsigned deliberately, unset ` + + `TRANSTRACK_RELEASE_CHANNEL and TRANSTRACK_REQUIRE_SIGNING.`, + ); + } _warn( `TRANSTRACK_SIGN_MODE=skip (auto-detected: no signing credentials in environment). ` + `Artifact "${path.basename(filePath)}" will be UNSIGNED. ` + @@ -185,6 +309,9 @@ async function sign(configuration) { ); return; } + + _assertModeConfigured(MODE); + if (MODE === 'ssl_esigner') return _runSslEsigner(filePath); if (MODE === 'pfx') return _runPfxSign(filePath); throw new Error(`Unknown TRANSTRACK_SIGN_MODE: ${MODE}`); @@ -199,4 +326,8 @@ module.exports.__testing__ = { _generateTotp, _base32Decode, _resolveFilePath, + _signingRequired, + _assertModeConfigured, + _materializeCertificate, + MODE_REQUIREMENTS, }; diff --git a/scripts/verify-artifact-signature.mjs b/scripts/verify-artifact-signature.mjs new file mode 100644 index 0000000..6fb11ad --- /dev/null +++ b/scripts/verify-artifact-signature.mjs @@ -0,0 +1,199 @@ +#!/usr/bin/env node +/** + * TransTrack — prove that a shipped Windows artifact is actually signed. + * + * The release gate previously checked that an installer existed with the right + * filename and version, under a step named "Code-signed Windows installer + * present". It never looked at the file. An entirely unsigned installer named + * correctly passed, which is the wrong way round: the filename is the part an + * attacker or an accident controls most easily, and the signature is the part + * that matters. + * + * Two levels of evidence, because they prove different things and are available + * in different places: + * + * 1. `readEmbeddedSignature()` parses the PE Certificate Table directly. This + * proves a signature is *embedded*, runs anywhere, and needs no tooling — + * which matters because the release gate job runs on Linux, where + * Get-AuthenticodeSignature does not exist. It cannot tell you the + * signature is valid or trusted. + * 2. `verifyAuthenticode()` shells out to Get-AuthenticodeSignature on + * Windows, which does establish validity, trust chain, and signer name. + * + * `inspectWindowsArtifact()` combines them and reports which level of assurance + * it actually achieved, rather than implying the stronger one everywhere. + * + * Run standalone: node scripts/verify-artifact-signature.mjs + */ + +import { readFileSync, existsSync } from 'node:fs'; +import { spawnSync } from 'node:child_process'; +import { pathToFileURL } from 'node:url'; + +/** Data directory index of the Attribute Certificate Table in a PE image. */ +const CERTIFICATE_TABLE_INDEX = 4; +const PE32_MAGIC = 0x10b; +const PE32PLUS_MAGIC = 0x20b; + +/** + * Locate the Attribute Certificate Table of a PE file. + * + * @param {string} filePath + * @returns {{ present: boolean, size: number, offset: number }} + * @throws if the file is not a PE image at all + */ +export function readEmbeddedSignature(filePath) { + const buf = readFileSync(filePath); + + if (buf.length < 0x40 || buf.readUInt16LE(0) !== 0x5a4d /* 'MZ' */) { + throw new Error('not a PE image (missing MZ header)'); + } + + const peOffset = buf.readUInt32LE(0x3c); + if (peOffset + 24 > buf.length || buf.readUInt32LE(peOffset) !== 0x00004550 /* 'PE\0\0' */) { + throw new Error('not a PE image (missing PE signature)'); + } + + const optionalHeaderOffset = peOffset + 24; + const magic = buf.readUInt16LE(optionalHeaderOffset); + + // The data directories sit after the optional header's fixed part, whose + // length differs between PE32 and PE32+ (the latter widens several fields to + // 64 bits and drops BaseOfData). + let dataDirectoryOffset; + if (magic === PE32_MAGIC) dataDirectoryOffset = optionalHeaderOffset + 96; + else if (magic === PE32PLUS_MAGIC) dataDirectoryOffset = optionalHeaderOffset + 112; + else throw new Error(`unrecognised PE optional header magic 0x${magic.toString(16)}`); + + const entryOffset = dataDirectoryOffset + CERTIFICATE_TABLE_INDEX * 8; + if (entryOffset + 8 > buf.length) { + return { present: false, size: 0, offset: 0 }; + } + + // Unlike every other data directory entry, this one holds a file offset + // rather than a relative virtual address. + const offset = buf.readUInt32LE(entryOffset); + const size = buf.readUInt32LE(entryOffset + 4); + + return { present: size > 0 && offset > 0, size, offset }; +} + +/** + * Ask Windows whether the signature is valid and who signed it. + * + * @returns {{ available: false } | { available: true, status: string, subject: string|null, valid: boolean }} + */ +export function verifyAuthenticode(filePath) { + if (process.platform !== 'win32') return { available: false }; + + const ps = spawnSync( + 'powershell', + [ + '-NoProfile', + '-NonInteractive', + '-Command', + `$s = Get-AuthenticodeSignature -LiteralPath '${filePath.replace(/'/g, "''")}'; ` + + `Write-Output ("STATUS=" + $s.Status); ` + + `Write-Output ("KIND=" + $s.SignatureType); ` + + `Write-Output ("SUBJECT=" + $s.SignerCertificate.Subject)`, + ], + { encoding: 'utf8' }, + ); + + if (ps.error || typeof ps.stdout !== 'string') return { available: false }; + + const status = /STATUS=(.*)/.exec(ps.stdout)?.[1]?.trim() || 'Unknown'; + const kind = /KIND=(.*)/.exec(ps.stdout)?.[1]?.trim() || 'Unknown'; + const subjectRaw = /SUBJECT=(.*)/.exec(ps.stdout)?.[1]?.trim() || ''; + + return { + available: true, + status, + kind, + subject: subjectRaw === '' ? null : subjectRaw, + valid: status === 'Valid', + }; +} + +/** + * Full assessment of a Windows artifact. + * + * @returns {{ signed: boolean, assurance: 'valid'|'embedded'|'none', detail: string }} + */ +export function inspectWindowsArtifact(filePath) { + if (!existsSync(filePath)) throw new Error(`artifact not found: ${filePath}`); + + const embedded = readEmbeddedSignature(filePath); + const authenticode = verifyAuthenticode(filePath); + + if (authenticode.available) { + // Windows is authoritative on validity, so ask it first rather than + // inferring from the file layout. + if (!authenticode.valid) { + return { + signed: false, + assurance: 'none', + detail: `Authenticode status is ${authenticode.status}, not Valid`, + }; + } + + // Valid, but is the signature actually part of the file? Windows reports + // catalog-signed system binaries as Valid even though nothing is embedded — + // notepad.exe is the canonical example. A catalog lives on the machine that + // installed it, so it cannot travel with a download: an installer we hand a + // hospital must carry its signature inside the file. + if (!embedded.present) { + return { + signed: false, + assurance: 'none', + detail: + `signature is ${authenticode.kind}-based, not embedded in the file. ` + + `A distributed installer must carry an embedded Authenticode signature, ` + + `because a catalog signature does not travel with the download`, + }; + } + + const who = authenticode.subject + ? authenticode.subject.split(',')[0].replace(/^CN=/, '').trim() + : 'unknown signer'; + return { signed: true, assurance: 'valid', detail: `Valid — signed by ${who}` }; + } + + // Not on Windows: the PE certificate table is the only evidence available. + if (!embedded.present) { + return { + signed: false, + assurance: 'none', + detail: 'no Authenticode signature is embedded in the executable', + }; + } + + return { + signed: true, + assurance: 'embedded', + detail: + `signature present (${embedded.size} bytes); validity not checked ` + + `because Get-AuthenticodeSignature is unavailable on ${process.platform}`, + }; +} + +// CLI. Compared as a URL rather than by filename so that importing this module +// from the release gate never triggers the command-line path. +const invokedDirectly = + process.argv[1] && pathToFileURL(process.argv[1]).href === import.meta.url; + +if (invokedDirectly) { + const target = process.argv[2]; + if (!target) { + console.error('usage: node scripts/verify-artifact-signature.mjs '); + process.exit(2); + } + try { + const result = inspectWindowsArtifact(target); + console.log(`${result.signed ? 'SIGNED' : 'UNSIGNED'} [${result.assurance}] ${result.detail}`); + process.exit(result.signed ? 0 : 1); + } catch (e) { + console.error(`ERROR ${e.message}`); + process.exit(2); + } +} diff --git a/tests/artifactSignature.test.mjs b/tests/artifactSignature.test.mjs new file mode 100644 index 0000000..bd815d7 --- /dev/null +++ b/tests/artifactSignature.test.mjs @@ -0,0 +1,198 @@ +/** + * TransTrack — Windows artifact signature verification. + * + * The release gate used to accept any file named like an installer as proof of + * a signed release. This suite covers the check that replaced that: does the + * artifact actually carry an Authenticode signature, and on Windows, does the + * OS consider it valid. + * + * PE fixtures are synthesised rather than committed, so the parser is exercised + * identically on every platform (the release gate job runs on Linux, where no + * signed .exe is available). On Windows the suite additionally checks two real + * binaries, because a hand-built fixture only proves the parser agrees with the + * author's reading of the PE specification. + * + * Run standalone: node tests/artifactSignature.test.mjs + */ + +import assert from 'node:assert'; +import { mkdtempSync, writeFileSync, rmSync, existsSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +import { + readEmbeddedSignature, + inspectWindowsArtifact, +} from '../scripts/verify-artifact-signature.mjs'; + +let PASS = 0, FAIL = 0; +const failures = []; +function test(name, fn) { + try { fn(); PASS++; console.log(` ok ${name}`); } + catch (e) { FAIL++; failures.push({ name, error: e }); console.log(` FAIL ${name}: ${e.message}`); } +} + +const SANDBOX = mkdtempSync(join(tmpdir(), 'tt-pe-')); + +const PE_OFFSET = 0x80; +const OPTIONAL_HEADER = PE_OFFSET + 24; + +/** + * Build a minimal but structurally valid PE image. + * + * @param {{ plus?: boolean, certOffset?: number, certSize?: number }} opts + */ +function makePe({ plus = true, certOffset = 0, certSize = 0 } = {}) { + const buf = Buffer.alloc(0x400); + buf.writeUInt16LE(0x5a4d, 0); // 'MZ' + buf.writeUInt32LE(PE_OFFSET, 0x3c); // e_lfanew + buf.writeUInt32LE(0x00004550, PE_OFFSET); // 'PE\0\0' + buf.writeUInt16LE(plus ? 0x20b : 0x10b, OPTIONAL_HEADER); + + // Data directories follow the fixed part of the optional header, whose size + // differs between the two formats. + const dataDirs = OPTIONAL_HEADER + (plus ? 112 : 96); + const certEntry = dataDirs + 4 * 8; // index 4 = Attribute Certificate Table + buf.writeUInt32LE(certOffset, certEntry); + buf.writeUInt32LE(certSize, certEntry + 4); + return buf; +} + +function fixture(name, buf) { + const p = join(SANDBOX, name); + writeFileSync(p, buf); + return p; +} + +console.log('\nPE certificate table parsing'); + +test('a PE32+ image with a certificate table reports a signature', () => { + const p = fixture('signed64.exe', makePe({ plus: true, certOffset: 0x300, certSize: 0x40 })); + const sig = readEmbeddedSignature(p); + assert.strictEqual(sig.present, true); + assert.strictEqual(sig.size, 0x40); + assert.strictEqual(sig.offset, 0x300); +}); + +test('a PE32 image with a certificate table reports a signature', () => { + // The 32-bit optional header is 16 bytes shorter; reading the directories at + // the 64-bit offset would silently look at the wrong entry. + const p = fixture('signed32.exe', makePe({ plus: false, certOffset: 0x200, certSize: 0x18 })); + const sig = readEmbeddedSignature(p); + assert.strictEqual(sig.present, true); + assert.strictEqual(sig.size, 0x18); +}); + +test('an image with an empty certificate table reports no signature', () => { + const p = fixture('unsigned.exe', makePe({ certOffset: 0, certSize: 0 })); + assert.strictEqual(readEmbeddedSignature(p).present, false); +}); + +test('a zero-size entry with a non-zero offset still counts as unsigned', () => { + const p = fixture('empty-cert.exe', makePe({ certOffset: 0x300, certSize: 0 })); + assert.strictEqual(readEmbeddedSignature(p).present, false); +}); + +console.log('\nMalformed input'); + +test('a non-PE file is rejected rather than read as unsigned', () => { + // Reporting "unsigned" for a file that is not an executable at all would let + // a truncated or wrong-format artifact fail for a misleading reason. + const p = fixture('notpe.txt', Buffer.from('this is not an executable')); + assert.throws(() => readEmbeddedSignature(p), /not a PE image \(missing MZ header\)/); +}); + +test('an MZ file with no PE header is rejected', () => { + const buf = Buffer.alloc(0x200); + buf.writeUInt16LE(0x5a4d, 0); + buf.writeUInt32LE(PE_OFFSET, 0x3c); + assert.throws(() => readEmbeddedSignature(fixture('nope.exe', buf)), /missing PE signature/); +}); + +test('an unrecognised optional header magic is rejected', () => { + const buf = makePe(); + buf.writeUInt16LE(0xdead, OPTIONAL_HEADER); + assert.throws( + () => readEmbeddedSignature(fixture('badmagic.exe', buf)), + /unrecognised PE optional header magic/, + ); +}); + +test('a missing artifact is reported as missing', () => { + assert.throws( + () => inspectWindowsArtifact(join(SANDBOX, 'absent.exe')), + /artifact not found/, + ); +}); + +console.log('\nOverall verdict'); + +test('an unsigned artifact is not accepted', () => { + const p = fixture('verdict-unsigned.exe', makePe()); + const r = inspectWindowsArtifact(p); + assert.strictEqual(r.signed, false); + assert.strictEqual(r.assurance, 'none'); +}); + +test('off Windows, an embedded signature is accepted with reduced assurance', () => { + if (process.platform === 'win32') { + console.log(' (skipped on Windows — the OS verdict is authoritative there)'); + return; + } + const p = fixture('verdict-signed.exe', makePe({ certOffset: 0x300, certSize: 0x40 })); + const r = inspectWindowsArtifact(p); + assert.strictEqual(r.signed, true); + assert.strictEqual(r.assurance, 'embedded', 'must not claim validity it did not check'); + assert.match(r.detail, /validity not checked/); +}); + +test('on Windows, a fixture with a fake certificate table is rejected as invalid', () => { + if (process.platform !== 'win32') { + console.log(' (skipped off Windows)'); + return; + } + // The table points at filler, not a PKCS#7 blob, so Windows must not call it + // Valid. This is the case the PE-only check cannot catch. + const p = fixture('verdict-fake.exe', makePe({ certOffset: 0x300, certSize: 0x40 })); + const r = inspectWindowsArtifact(p); + assert.strictEqual(r.signed, false, 'a forged certificate table must not pass on Windows'); +}); + +console.log('\nReal binaries (Windows only)'); + +test('a genuinely signed executable is accepted with full assurance', () => { + if (process.platform !== 'win32') { + console.log(' (skipped off Windows)'); + return; + } + // node.exe carries an embedded Authenticode signature. + const r = inspectWindowsArtifact(process.execPath); + assert.strictEqual(r.signed, true, `expected ${process.execPath} to be signed: ${r.detail}`); + assert.strictEqual(r.assurance, 'valid'); +}); + +test('a catalog-signed system binary is rejected for distribution', () => { + if (process.platform !== 'win32') { + console.log(' (skipped off Windows)'); + return; + } + const notepad = 'C:\\Windows\\System32\\notepad.exe'; + if (!existsSync(notepad)) { + console.log(' (skipped — notepad.exe not present)'); + return; + } + // Windows reports this Valid, but the signature lives in a system catalog + // rather than in the file. A catalog cannot travel with a download, so an + // installer signed only this way would arrive at a customer unverifiable. + const r = inspectWindowsArtifact(notepad); + assert.strictEqual(r.signed, false, 'catalog-only signing must not satisfy the release gate'); + assert.match(r.detail, /not embedded/); +}); + +rmSync(SANDBOX, { recursive: true, force: true }); + +console.log(`\n${PASS} passed, ${FAIL} failed\n`); +if (FAIL > 0) { + for (const f of failures) console.error(`${f.name}\n${f.error.stack || f.error.message}\n`); + process.exit(1); +} diff --git a/tests/notarize.test.cjs b/tests/notarize.test.cjs new file mode 100644 index 0000000..6566dff --- /dev/null +++ b/tests/notarize.test.cjs @@ -0,0 +1,172 @@ +/** + * TransTrack — macOS notarization hook. + * + * The hook cannot be exercised end to end without an Apple Developer account, + * so what is tested here is the decision it makes before calling Apple: whether + * a missing credential is a warning or a failure. + * + * That distinction is the whole point. The hook used to warn and return in every + * failure case, which meant the most likely way to ship an un-notarized DMG was + * to believe you had notarized it — the build went green, the warning scrolled + * past in a few thousand lines of electron-builder output, and Gatekeeper + * rejected the download on the customer's machine. + * + * Run standalone: node tests/notarize.test.cjs + */ + +'use strict'; + +const assert = require('assert'); + +const { __testing__: t } = require('../scripts/notarize.cjs'); + +let PASS = 0, FAIL = 0; +const failures = []; +function test(name, fn) { + try { fn(); PASS++; console.log(` ok ${name}`); } + catch (e) { FAIL++; failures.push({ name, error: e }); console.log(` FAIL ${name}: ${e.message}`); } +} + +const FULL = Object.freeze({ + APPLE_ID: 'dev@example.org', + APPLE_APP_PASSWORD: 'abcd-efgh-ijkl-mnop', + APPLE_TEAM_ID: 'ABCDE12345', +}); + +console.log('\nWhen is notarization mandatory'); + +test('a public release requires notarization', () => { + assert.strictEqual(t.notarizationRequired({ TRANSTRACK_RELEASE_CHANNEL: 'public' }), true); +}); + +test('the explicit flag requires notarization on its own', () => { + assert.strictEqual(t.notarizationRequired({ TRANSTRACK_REQUIRE_NOTARIZATION: '1' }), true); + assert.strictEqual(t.notarizationRequired({ TRANSTRACK_REQUIRE_NOTARIZATION: 'true' }), true); +}); + +test('a developer build does not', () => { + assert.strictEqual(t.notarizationRequired({}), false); + assert.strictEqual(t.notarizationRequired({ TRANSTRACK_RELEASE_CHANNEL: 'internal' }), false); + assert.strictEqual(t.notarizationRequired({ TRANSTRACK_REQUIRE_NOTARIZATION: '0' }), false); +}); + +console.log('\nCredential inspection'); + +test('a complete credential set reports nothing missing', () => { + assert.deepStrictEqual(t.inspectCredentials(FULL), { missing: [], hints: [] }); +}); + +test('each required variable is reported when absent', () => { + for (const key of t.REQUIRED) { + const env = { ...FULL }; + delete env[key]; + const { missing } = t.inspectCredentials(env); + assert.deepStrictEqual(missing, [key], `expected ${key} to be reported missing`); + } +}); + +test("Apple's own name for the password is diagnosed, not reported as absent", () => { + // Apple calls it an "app-specific password" and docs/DEPLOYMENT_PRODUCTION.md + // said APPLE_APP_SPECIFIC_PASSWORD, while the hook reads APPLE_APP_PASSWORD. + // Anyone who followed that doc got a silent skip; naming the mistake turns a + // twenty-minute puzzle into a one-line fix. + const env = { ...FULL }; + delete env.APPLE_APP_PASSWORD; + env.APPLE_APP_SPECIFIC_PASSWORD = 'abcd-efgh-ijkl-mnop'; + + const { missing, hints } = t.inspectCredentials(env); + assert.deepStrictEqual(missing, ['APPLE_APP_PASSWORD']); + assert.strictEqual(hints.length, 1); + assert.match(hints[0], /APPLE_APP_SPECIFIC_PASSWORD is set/); + assert.match(hints[0], /rename it/); +}); + +test('a misspelled team id is diagnosed too', () => { + const env = { ...FULL }; + delete env.APPLE_TEAM_ID; + env.APPLE_TEAMID = 'ABCDE12345'; + + const { hints } = t.inspectCredentials(env); + assert.strictEqual(hints.length, 1); + assert.match(hints[0], /APPLE_TEAMID is set/); +}); + +test('no hint is offered when nothing resembling the variable is present', () => { + const env = { ...FULL }; + delete env.APPLE_APP_PASSWORD; + const { missing, hints } = t.inspectCredentials(env); + assert.deepStrictEqual(missing, ['APPLE_APP_PASSWORD']); + assert.deepStrictEqual(hints, []); +}); + +console.log('\nHook behaviour'); + +/** Minimal electron-builder afterSign context. */ +function context(platform) { + return { + electronPlatformName: platform, + appOutDir: '/tmp/out', + packager: { appInfo: { productFilename: 'TransTrack Enterprise' }, config: { appId: 'com.x' } }, + }; +} + +const hook = require('../scripts/notarize.cjs').default; + +async function withEnv(env, body) { + const original = { ...process.env }; + for (const k of Object.keys(process.env)) { + if (k.startsWith('APPLE_') || k.startsWith('TRANSTRACK_')) delete process.env[k]; + } + Object.assign(process.env, env); + try { return await body(); } + finally { process.env = original; } +} + +const queue = []; +function asyncTest(name, fn) { + queue.push(async () => { + try { await fn(); PASS++; console.log(` ok ${name}`); } + catch (e) { FAIL++; failures.push({ name, error: e }); console.log(` FAIL ${name}: ${e.message}`); } + }); +} + +asyncTest('a non-macOS build returns without doing anything', async () => { + await withEnv({ TRANSTRACK_RELEASE_CHANNEL: 'public' }, async () => { + // Even on a release, there is nothing to notarize when the platform is not + // darwin, so this must not throw. + await hook(context('win32')); + }); +}); + +asyncTest('a release build with no credentials fails rather than skipping', async () => { + await withEnv({ TRANSTRACK_RELEASE_CHANNEL: 'public' }, async () => { + await assert.rejects(() => hook(context('darwin')), /Cannot notarize/); + }); +}); + +asyncTest('the failure names the missing variables', async () => { + await withEnv({ TRANSTRACK_RELEASE_CHANNEL: 'public' }, async () => { + await assert.rejects(() => hook(context('darwin')), (e) => { + assert.match(e.message, /APPLE_ID/); + assert.match(e.message, /APPLE_APP_PASSWORD/); + assert.match(e.message, /APPLE_TEAM_ID/); + return true; + }); + }); +}); + +asyncTest('a developer build with no credentials still skips quietly', async () => { + await withEnv({}, async () => { + await hook(context('darwin')); + }); +}); + +(async () => { + for (const run of queue) await run(); + + console.log(`\n${PASS} passed, ${FAIL} failed\n`); + if (FAIL > 0) { + for (const f of failures) console.error(`${f.name}\n${f.error.stack || f.error.message}\n`); + process.exit(1); + } +})(); diff --git a/tests/signWin.test.cjs b/tests/signWin.test.cjs index 8a6a8e3..08a8a9d 100644 --- a/tests/signWin.test.cjs +++ b/tests/signWin.test.cjs @@ -1,87 +1,266 @@ /** * TransTrack — sign-win.cjs unit tests. * - * Validates the parts that DON'T need a real Authenticode certificate: - * - Auto-detect mode based on env vars - * - Base32 decoder + TOTP RFC 6238 vector - * - Skip-mode is a no-op (no exception) - * - Path resolver handles both string and {path} shapes + * Validates the parts that don't need a real Authenticode certificate: mode + * auto-detection, the fail-closed behaviour on release builds, base32/TOTP, + * certificate materialisation from base64, and the path resolver. + * + * The harness awaits its tests. It previously did not: `test()` called an + * `async` function and incremented the pass counter on the next line, so every + * asynchronous test was recorded as passing before its assertions had run, and + * a rejection surfaced later as an unhandled rejection rather than a failure. + * The `assert.rejects` cases below — the signer's entire error surface — were + * therefore never actually checked. + * + * Run standalone: node tests/signWin.test.cjs */ 'use strict'; const assert = require('assert'); +const fs = require('fs'); +const os = require('os'); const path = require('path'); let PASS = 0, FAIL = 0; const failures = []; +const queue = []; + function test(name, fn) { - try { fn(); PASS++; console.log(` PASS ${name}`); } - catch (e) { - FAIL++; failures.push({ name, error: e }); - console.log(` FAIL ${name}\n ${e.message}`); - } + queue.push(async () => { + try { await fn(); PASS++; console.log(` PASS ${name}`); } + catch (e) { + FAIL++; failures.push({ name, error: e }); + console.log(` FAIL ${name}\n ${e.message}`); + } + }); } -// We re-require the module fresh between tests because module-load reads env. -function freshSigner(env) { +const SIGNING_ENV_KEYS = (k) => + k.startsWith('ESIGNER_') || + k.startsWith('CSC_') || + k === 'TRANSTRACK_SIGN_MODE' || + k === 'TRANSTRACK_RELEASE_CHANNEL' || + k === 'TRANSTRACK_REQUIRE_SIGNING' || + k === 'SIGN_TIMESTAMP_URL'; + +/** + * Load the signer with a controlled environment. + * + * The module reads TRANSTRACK_SIGN_MODE at load time but evaluates the + * "is signing required" question at call time, so the environment has to stay + * in place for the duration of the call rather than only for the require. + */ +async function withSigner(env, body) { const original = { ...process.env }; - // Clear all signing-related env vars for (const k of Object.keys(process.env)) { - if ( - k.startsWith('ESIGNER_') || - k.startsWith('CSC_') || - k === 'TRANSTRACK_SIGN_MODE' || - k === 'SIGN_TIMESTAMP_URL' - ) { - delete process.env[k]; - } + if (SIGNING_ENV_KEYS(k)) delete process.env[k]; } Object.assign(process.env, env || {}); delete require.cache[require.resolve('../scripts/sign-win.cjs')]; const mod = require('../scripts/sign-win.cjs'); - process.env = original; - return mod; + try { + return await body(mod); + } finally { + process.env = original; + } } console.log('\n=== sign-win.cjs ==='); -test('skip mode is a no-op (does not throw)', async () => { - const sign = freshSigner({ TRANSTRACK_SIGN_MODE: 'skip' }); - await sign('C:/tmp/some/file.exe'); - await sign({ path: 'C:/tmp/some/file.exe' }); +test('skip mode is a no-op on a developer build', async () => { + await withSigner({ TRANSTRACK_SIGN_MODE: 'skip' }, async (sign) => { + await sign('C:/tmp/some/file.exe'); + await sign({ path: 'C:/tmp/some/file.exe' }); + }); }); -test('auto-detect: no env vars → skip', async () => { - const sign = freshSigner({}); - await sign('C:/tmp/file.exe'); // should not throw +test('auto-detect with no credentials yields skip', async () => { + await withSigner({}, async (sign) => { + await sign('C:/tmp/file.exe'); + }); }); test('unknown mode throws', async () => { - const sign = freshSigner({ TRANSTRACK_SIGN_MODE: 'magic_unicorn' }); - await assert.rejects(() => sign('C:/tmp/file.exe'), - /Unknown TRANSTRACK_SIGN_MODE/); + await withSigner({ TRANSTRACK_SIGN_MODE: 'magic_unicorn' }, async (sign) => { + await assert.rejects(() => sign('C:/tmp/file.exe'), /Unknown TRANSTRACK_SIGN_MODE/); + }); }); -test('null/undefined input is tolerated (warn + return)', async () => { - const sign = freshSigner({ TRANSTRACK_SIGN_MODE: 'skip' }); - await sign(null); - await sign(undefined); - await sign({}); +test('missing file path is tolerated on a developer build', async () => { + await withSigner({ TRANSTRACK_SIGN_MODE: 'skip' }, async (sign) => { + await sign(null); + await sign(undefined); + await sign({}); + }); }); -test('exports both default and named function (electron-builder shapes)', () => { - const mod = freshSigner({ TRANSTRACK_SIGN_MODE: 'skip' }); - assert.strictEqual(typeof mod, 'function'); - assert.strictEqual(typeof mod.default, 'function'); - assert.strictEqual(mod, mod.default); +test('exports both default and named function (electron-builder shapes)', async () => { + await withSigner({ TRANSTRACK_SIGN_MODE: 'skip' }, async (mod) => { + assert.strictEqual(typeof mod, 'function'); + assert.strictEqual(typeof mod.default, 'function'); + assert.strictEqual(mod, mod.default); + }); }); -console.log('\n=== TOTP RFC 6238 vectors (via base32 decoder) ==='); +console.log('\n=== fail-closed on release builds ==='); + +test('a release build refuses to produce an unsigned artifact', async () => { + await withSigner( + { TRANSTRACK_SIGN_MODE: 'skip', TRANSTRACK_RELEASE_CHANNEL: 'public' }, + async (sign) => { + await assert.rejects( + () => sign('C:/tmp/TransTrack-Enterprise-1.2.1-x64.exe'), + /Signing is required for this build/, + ); + }, + ); +}); + +test('TRANSTRACK_REQUIRE_SIGNING alone is enough to fail closed', async () => { + await withSigner( + { TRANSTRACK_SIGN_MODE: 'skip', TRANSTRACK_REQUIRE_SIGNING: '1' }, + async (sign) => { + await assert.rejects(() => sign('C:/tmp/file.exe'), /Signing is required/); + }, + ); +}); + +test('a release build refuses a missing file path rather than skipping', async () => { + await withSigner( + { TRANSTRACK_SIGN_MODE: 'skip', TRANSTRACK_RELEASE_CHANNEL: 'public' }, + async (sign) => { + await assert.rejects(() => sign(null), /No file path provided/); + }, + ); +}); + +test('a non-release build with no credentials still just warns', async () => { + await withSigner( + { TRANSTRACK_SIGN_MODE: 'skip', TRANSTRACK_RELEASE_CHANNEL: 'internal' }, + async (sign) => { + await sign('C:/tmp/file.exe'); + }, + ); +}); + +test('esigner mode names the missing variable instead of failing deep in the call', async () => { + await withSigner( + { + TRANSTRACK_SIGN_MODE: 'ssl_esigner', + ESIGNER_USERNAME: 'u', + ESIGNER_PASSWORD: 'p', + ESIGNER_CREDENTIAL_ID: 'c', + ESIGNER_TOTP_SECRET: 'JBSWY3DPEHPK3PXP', + // ESIGNER_TOOL_PATH deliberately absent — the exact gap that broke the + // release workflow. + }, + async (sign) => { + await assert.rejects( + () => sign('C:/tmp/file.exe'), + (e) => { + assert.match(e.message, /ESIGNER_TOOL_PATH/); + assert.match(e.message, /is not set/); + assert.ok( + !/not found: undefined/.test(e.message), + 'must not report the old confusing "not found: undefined" message', + ); + return true; + }, + ); + }, + ); +}); + +test('pfx mode names both missing variables at once', async () => { + await withSigner({ TRANSTRACK_SIGN_MODE: 'pfx' }, async (sign) => { + await assert.rejects(() => sign('C:/tmp/file.exe'), (e) => { + assert.match(e.message, /CSC_LINK/); + assert.match(e.message, /CSC_KEY_PASSWORD/); + return true; + }); + }); +}); + +console.log('\n=== signing-required detection ==='); const exposed = require('../scripts/sign-win.cjs').__testing__; -test('base32 decode of known vector: "JBSWY3DPEHPK3PXP"', () => { +function requiredWith(env) { + const original = { ...process.env }; + for (const k of Object.keys(process.env)) { + if (SIGNING_ENV_KEYS(k)) delete process.env[k]; + } + Object.assign(process.env, env); + try { return exposed._signingRequired(); } + finally { process.env = original; } +} + +test('release channel and explicit flag both require signing', async () => { + assert.strictEqual(requiredWith({ TRANSTRACK_RELEASE_CHANNEL: 'public' }), true); + assert.strictEqual(requiredWith({ TRANSTRACK_REQUIRE_SIGNING: '1' }), true); + assert.strictEqual(requiredWith({ TRANSTRACK_REQUIRE_SIGNING: 'true' }), true); + assert.strictEqual(requiredWith({ TRANSTRACK_REQUIRE_SIGNING: 'TRUE' }), true); +}); + +test('an ordinary developer build does not require signing', async () => { + assert.strictEqual(requiredWith({}), false); + assert.strictEqual(requiredWith({ TRANSTRACK_RELEASE_CHANNEL: 'internal' }), false); + assert.strictEqual(requiredWith({ TRANSTRACK_REQUIRE_SIGNING: '0' }), false); + assert.strictEqual(requiredWith({ TRANSTRACK_REQUIRE_SIGNING: 'no' }), false); +}); + +console.log('\n=== certificate materialisation ==='); + +// Minimal DER SEQUENCE header followed by filler; enough to satisfy the shape +// check without shipping a real key. +const FAKE_PKCS12 = Buffer.concat([ + Buffer.from([0x30, 0x82, 0x01, 0x00]), + Buffer.alloc(200, 0x41), +]); + +test('an existing file path is used as-is and never deleted', async () => { + const dir = fs.mkdtempSync(path.join(os.tmpdir(), 'tt-cert-')); + const file = path.join(dir, 'real.pfx'); + fs.writeFileSync(file, FAKE_PKCS12); + + const cert = exposed._materializeCertificate(file); + assert.strictEqual(cert.file, file); + cert.cleanup(); + assert.ok(fs.existsSync(file), 'a caller-supplied certificate must survive cleanup'); + + fs.rmSync(dir, { recursive: true, force: true }); +}); + +test('base64 content is written to a temporary file and removed on cleanup', async () => { + const cert = exposed._materializeCertificate(FAKE_PKCS12.toString('base64')); + assert.ok(fs.existsSync(cert.file), 'certificate should have been materialised'); + assert.deepStrictEqual(fs.readFileSync(cert.file), FAKE_PKCS12, 'content must round-trip'); + + if (process.platform !== 'win32') { + const mode = fs.statSync(cert.file).mode & 0o777; + assert.strictEqual(mode, 0o600, 'a private key must not be group- or world-readable'); + } + + cert.cleanup(); + assert.ok(!fs.existsSync(cert.file), 'the temporary private key must not be left behind'); +}); + +test('a wrong path is reported as a path problem, not a corrupt certificate', async () => { + assert.throws( + () => exposed._materializeCertificate('C:/no/such/cert.pfx'), + /neither an existing file nor base64-encoded PKCS#12/, + ); +}); + +test('base64 of something that is not a PKCS#12 is rejected', async () => { + const notACert = Buffer.alloc(200, 0x41).toString('base64'); + assert.throws(() => exposed._materializeCertificate(notACert), /PKCS#12/); +}); + +console.log('\n=== TOTP RFC 6238 vectors (via base32 decoder) ==='); + +test('base32 decode of known vector: "JBSWY3DPEHPK3PXP"', async () => { const buf = exposed._base32Decode('JBSWY3DPEHPK3PXP'); // "Hello!" then DE AD BE EF assert.deepStrictEqual( @@ -90,20 +269,24 @@ test('base32 decode of known vector: "JBSWY3DPEHPK3PXP"', () => { ); }); -test('TOTP digits are 6, all numeric', () => { +test('TOTP digits are 6, all numeric', async () => { const code = exposed._generateTotp('GEZDGNBVGY3TQOJQGEZDGNBVGY3TQOJQ'); assert.match(code, /^\d{6}$/); }); -test('_resolveFilePath: handles string and {path} shapes', () => { +test('_resolveFilePath: handles string and {path} shapes', async () => { assert.strictEqual(exposed._resolveFilePath('C:/x/y.exe'), 'C:/x/y.exe'); assert.strictEqual(exposed._resolveFilePath({ path: 'C:/x/y.exe' }), 'C:/x/y.exe'); assert.strictEqual(exposed._resolveFilePath(null), null); assert.strictEqual(exposed._resolveFilePath({}), null); }); -console.log(`\nResults: ${PASS} passed, ${FAIL} failed.`); -if (FAIL > 0) { - for (const f of failures) console.error(`\n${f.name}:\n${f.error.stack || f.error.message}`); - process.exit(1); -} +(async () => { + for (const run of queue) await run(); + + console.log(`\nResults: ${PASS} passed, ${FAIL} failed.`); + if (FAIL > 0) { + for (const f of failures) console.error(`\n${f.name}:\n${f.error.stack || f.error.message}`); + process.exit(1); + } +})(); From 73348a1dc190b1cf2a11f3f63c5e06368b7ad6d8 Mon Sep 17 00:00:00 2001 From: NeuroKoder3 Date: Sat, 1 Aug 2026 19:32:37 -0500 Subject: [PATCH 2/4] docs(signing): correct the EV recommendation and the Apple password variable The guidance to buy an EV certificate rested entirely on EV granting immediate SmartScreen reputation. Microsoft removed that behaviour; OV and EV now produce the same first-download experience, and reputation accrues per file hash either way. Following the old advice costs several hundred dollars a year for nothing except a procurement checkbox, which is a real but different reason to buy it. Two constraints that change what is even purchasable are now stated: since June 2023 every code signing key, OV included, must live in hardware, so a copyable .pfx is no longer issuable and pfx mode is for certificates already held; and since February 2026 certificates are capped at 460 days. DEPLOYMENT_PRODUCTION.md told readers to set APPLE_APP_SPECIFIC_PASSWORD, which the hook does not read. Anyone who followed it got a silent skip. The doc is corrected and the hook now names the mistake when it sees it. Validation package: TT-R146 and TT-R147 for release authenticity, SDS section 17, risk R-028, and OQ-146/147 -- the latter having the receiving site verify the installer's signature themselves before installing, which is the only check that does not depend on trusting the vendor's own build log. Co-authored-by: Cursor --- CHANGELOG.md | 50 ++++ docs/CODE_SIGNING.md | 220 ++++++++++++++---- docs/DEPLOYMENT_PRODUCTION.md | 18 +- docs/compliance/RISK_REGISTER.md | 3 +- .../SOFTWARE_DESIGN_SPECIFICATION.md | 37 +++ .../SYSTEM_REQUIREMENTS_SPECIFICATION.md | 2 + docs/compliance/TRACEABILITY_MATRIX.md | 3 + .../templates/OQ_PROTOCOL_TEMPLATE.md | 2 + docs/legal/COMMERCIALIZATION_CHECKLIST.md | 46 ++-- 9 files changed, 315 insertions(+), 66 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f0dabe2..8fc0c3c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,53 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). ## [Unreleased] +### Fixed — release signing + +- **A release build can no longer emit an unsigned artifact.** Both + `sign-win.cjs` and `notarize.cjs` warned and returned whenever a credential + was missing. That is right for a developer build and wrong for a release: the + build went green, the warning scrolled past in electron-builder's output, and + nobody found out until a customer's machine refused the download. When + `TRANSTRACK_RELEASE_CHANNEL=public` (set by `release.yml` on both build jobs), + or `TRANSTRACK_REQUIRE_SIGNING` / `TRANSTRACK_REQUIRE_NOTARIZATION` is set, + the build now fails and names the missing variable. +- **The release gate now inspects the installer instead of its filename.** + "Code-signed Windows installer present" was satisfied by any file matching the + expected name. `scripts/verify-artifact-signature.mjs` reads the artifact: the + OS verdict via `Get-AuthenticodeSignature` on Windows, the PE Attribute + Certificate Table elsewhere, with the weaker assurance labelled as such rather + than overstated. A catalog-only signature is rejected — Windows calls it + valid, but it lives outside the file and so cannot reach the receiving site. +- **`pfx` mode works in CI.** `CSC_LINK` was treated strictly as a filesystem + path, but a certificate in a CI secret is base64 bytes. Base64 content is now + written to a temporary file with owner-only permissions and removed in a + `finally` block. +- **`ssl_esigner` mode works in CI.** The workflow never set + `ESIGNER_TOOL_PATH` and never installed CodeSignTool, so the mode could not + have signed anything. Both are now handled, and a missing `ESIGNER_TOOL_URL` + fails the job rather than yielding an unsigned build. +- **Notarization diagnoses the likely credential mistake.** Apple's own term is + "app-specific password", and `docs/DEPLOYMENT_PRODUCTION.md` said + `APPLE_APP_SPECIFIC_PASSWORD`, while the hook reads `APPLE_APP_PASSWORD`. The + doc is corrected, and the hook now says so by name when it finds the longer + spelling set. +- **`tests/signWin.test.cjs` never awaited its async cases**, so every + `assert.rejects` counted as a pass without running. The harness is now + async-aware, and the suite covers mode selection, fail-closed behaviour, and + certificate materialisation. New: `tests/notarize.test.cjs` and + `tests/artifactSignature.test.mjs`, the latter parsing synthetic PE images on + every platform and, on Windows, checking a genuinely signed binary and a + catalog-signed one. + +### Changed — code signing guidance + +- **EV is no longer recommended by default.** The guidance to buy EV rested on + it granting immediate SmartScreen reputation; Microsoft removed that + behaviour, and OV now gives the same first-download experience. The docs now + recommend Azure Artifact Signing (~$10/month) or an OV certificate with cloud + HSM signing, and note that since June 2023 all code signing keys — OV + included — must live in hardware, so a copyable `.pfx` is no longer issuable. + ### Fixed - **Support bundle log tail no longer loses a race with log rotation.** @@ -34,6 +81,9 @@ This project uses [Semantic Versioning](https://semver.org/spec/v2.0.0.html). pipeline, chart filing, migration safety, and support bundles — including the adversarial one that matters: plant a patient name in free text, export a default bundle, and search the file for it. +- **Release authenticity added to the validation package** as TT-R146 and + TT-R147, SDS §17, R-028, and OQ-146/147 — the last of which has the receiving + site verify the installer's signature themselves before installing it. - **Risk register extended** with R-020 to R-027 (missed notification deadline, duplicate or misfiled chart document, bundle PHI leakage, notice altered after filing, template missing a statutory element, stale diff --git a/docs/CODE_SIGNING.md b/docs/CODE_SIGNING.md index 3e44912..98c95c2 100644 --- a/docs/CODE_SIGNING.md +++ b/docs/CODE_SIGNING.md @@ -28,6 +28,70 @@ release path**. --- +## Signing fails closed on a release + +A developer build with no certificate produces an unsigned artifact and a +warning, which is what you want day to day. A **release** build that came out +unsigned used to do exactly the same thing, which is the dangerous case: the +build went green, the warning scrolled past in electron-builder's output, and +nothing downstream looked at the file. + +Both the Windows signer and the macOS notarization hook now treat a missing +credential as a build failure when either of these is set: + +```text +TRANSTRACK_RELEASE_CHANNEL=public # also what promotes the release gate to mandatory +TRANSTRACK_REQUIRE_SIGNING=1 # Windows only +TRANSTRACK_REQUIRE_NOTARIZATION=1 # macOS only +``` + +`.github/workflows/release.yml` sets `TRANSTRACK_RELEASE_CHANNEL=public` on both +build jobs, so a tagged release cannot silently produce an unsigned binary. + +Separately, the release gate now **inspects the artifact** rather than trusting +that the hook ran. See "Verifying a signed artifact" below. + +--- + +## Choosing a Windows certificate + +Read this before buying anything — the guidance that circulated for years is out +of date. + +**EV no longer buys you SmartScreen trust.** Historically an Extended Validation +certificate granted immediate SmartScreen reputation, and that was the reason to +pay the premium. Microsoft removed that behaviour; their current documentation +states that EV certificates no longer bypass SmartScreen and that "paying a +premium for EV solely to avoid SmartScreen warnings is no longer justified." An +OV certificate and an EV certificate now produce the same first-download +experience: a warning that fades as download volume accumulates reputation, per +file hash. + +EV may still be worth it for one non-technical reason: **enterprise procurement**. +Hospital security reviews sometimes name EV explicitly. That is a sales question, +not an engineering one. + +| Option | Indicative cost | Notes | +|---|---|---| +| **Azure Artifact Signing** (formerly Trusted Signing) | ~$10/month | Microsoft's recommended route for non-Store distribution. No hardware token, CI-native. Organisations in US/Canada/EU/UK; individuals US/Canada only. **Not yet implemented in `sign-win.cjs`** — needs a new mode. | +| **OV certificate** (Sectigo, DigiCert, Certum, SSL.com) | ~$150–300/yr | Same SmartScreen behaviour as EV. Works with `pfx` mode, or with a cloud HSM via `ssl_esigner`. | +| **EV certificate** | ~$400–700/yr | Choose only if a customer's procurement process demands it. | +| Apple Developer Program (Organization) | $99/yr | Required for notarization; no alternative. | +| D-U-N-S registration | Free | Needed for Apple organisation enrolment and for EV vetting. | + +Two constraints worth knowing before you commit: + +* Since **June 2023** the CA/Browser Forum requires the private key for *any* + code signing certificate — OV as well as EV — to live in a FIPS-compliant + hardware module. That means either a shipped USB token or a cloud HSM + (SSL.com eSigner, DigiCert KeyLocker, Certum SimplySign). A plain `.pfx` you + can copy around is no longer issuable, so `pfx` mode is for certificates you + already hold, internal builds, and test signing. +* Since **February 2026** certificates are capped at 460 days. A multi-year + purchase from a traditional CA now means a new hardware device each year. + +--- + ## Windows Authenticode ### Modes supported @@ -39,22 +103,26 @@ Windows artifact. It supports three modes selected by the | Mode | Use case | Required env vars | |----------------|------------------------------------------------------------------|-------------------| | `ssl_esigner` | Recommended for CI/CD. SSL.com eSigner cloud HSM (no USB token). | `ESIGNER_USERNAME`, `ESIGNER_PASSWORD`, `ESIGNER_CREDENTIAL_ID`, `ESIGNER_TOTP_SECRET`, `ESIGNER_TOOL_PATH` | -| `pfx` | Local builds with a software-protected `.pfx` file. | `CSC_LINK` (path to .pfx), `CSC_KEY_PASSWORD` | +| `pfx` | Local builds with a software-protected `.pfx` file. | `CSC_LINK` (path **or** base64 content), `CSC_KEY_PASSWORD` | | `skip` | Unsigned development builds. Never use for release. | (none) | If `TRANSTRACK_SIGN_MODE` is **unset**, the script auto-detects in the -order `ssl_esigner` → `pfx` → `skip`. +order `ssl_esigner` → `pfx` → `skip`. When a mode is named explicitly but its +variables are incomplete, the signer fails immediately and names the missing +variable rather than falling through to `skip`. -### Recommended: SSL.com eSigner Cloud HSM +### Cloud HSM via SSL.com eSigner -eSigner is preferable to a physical USB token because it works in -unattended CI without anyone physically present to insert the token. +A cloud HSM is preferable to a physical USB token because it works in +unattended CI without anyone present to insert the token. Since the 2023 +hardware-key requirement this is effectively the only workable CI option for a +traditional CA certificate. Procurement steps: -1. Purchase **SSL.com EV Code Signing Certificate** with **eSigner - Cloud Signing** (or DigiCert KeyLocker / Certum SimplySign — same - shape). +1. Purchase an **SSL.com Code Signing Certificate** — OV unless a customer + requires EV — with **eSigner Cloud Signing** (or DigiCert KeyLocker / + Certum SimplySign — same shape). 2. Complete the SSL.com vetting process (D-U-N-S number required for EV). 3. Download **CodeSignTool** from the SSL.com dashboard. The tool ships as a `.bat` (Windows) or `.sh` (Linux/macOS) wrapper around a Java jar. @@ -72,37 +140,68 @@ ESIGNER_USERNAME= ESIGNER_PASSWORD= ESIGNER_CREDENTIAL_ID= ESIGNER_TOTP_SECRET= -ESIGNER_TOOL_PATH=C:\\CodeSignTool\\CodeSignTool.bat +ESIGNER_TOOL_PATH=C:\CodeSignTool\CodeSignTool.bat +ESIGNER_TOOL_URL= ``` +`ESIGNER_TOOL_URL` is used by the release workflow to install CodeSignTool on +the runner before the build; SSL.com does not publish a stable URL, so take the +current one from your dashboard and store it as a repository secret. If the +mode is active and the URL is missing, the workflow fails rather than building +an unsigned installer. + The signer derives a one-time TOTP code at sign time using the seed (RFC 6238, SHA1, 30-second step, 6 digits). -### Alternate: PFX file (local-only) +### Alternate: PFX file -For OV certificates or for one-off local release builds: +For a certificate you already hold, or for internal and test builds: ```text TRANSTRACK_SIGN_MODE=pfx -CSC_LINK=C:\\path\\to\\TransTrack-codesign.pfx +CSC_LINK=C:\path\to\TransTrack-codesign.pfx CSC_KEY_PASSWORD= SIGN_TIMESTAMP_URL=http://timestamp.sectigo.com (optional override) ``` +`CSC_LINK` accepts either a filesystem path or the base64-encoded contents of +the `.p12`/`.pfx` itself — the latter is how a certificate is normally carried +in a CI secret. Base64 content is written to a temporary file with owner-only +permissions and deleted in a `finally` block whether signing succeeds or not. + The Windows SDK's `signtool.exe` must be on `PATH`. On GitHub Actions the `windows-latest` runner ships with it; locally, install it via the Windows 10/11 SDK. ### Verifying a signed artifact -On Windows: +The release gate does this for you — `scripts/release-readiness-check.mjs` +inspects the installer and fails if it is not really signed. To run the check by +hand: + +```bash +node scripts/verify-artifact-signature.mjs "release/enterprise/TransTrack-Enterprise-1.2.1-x64.exe" +``` + +On Windows it asks the OS (`Get-AuthenticodeSignature`) and requires a `Valid` +verdict. Elsewhere — the release gate runs on Linux — it parses the PE +Attribute Certificate Table directly, which proves a signature is embedded but +not that it chains to a trusted root; the output labels that reduced assurance +rather than claiming more than it checked. + +A **catalog-only** signature is rejected even though Windows reports it `Valid`. +Catalog signatures live in a system-wide `.cat` file, not in the executable, so +they do not survive a download to a customer's machine. + +To inspect the signer identity: ```powershell -Get-AuthenticodeSignature .\release\enterprise\TransTrack-Enterprise-1.2.0-x64.exe +Get-AuthenticodeSignature .\release\enterprise\TransTrack-Enterprise-1.2.1-x64.exe | + Format-List Status, SignerCertificate ``` -`Status` should be `Valid`, `SignerCertificate.Subject` should match -your organisation's name as registered with the CA. +`Status` should be `Valid` and the certificate subject should match your +organisation's name as registered with the CA. --- @@ -123,6 +222,11 @@ APPLE_TEAM_ID=<10-character Team ID, visible in App Store Connect> Generate the app-specific password at → **Sign-In and Security** → **App-Specific Passwords**. +Apple's own term is "app-specific password", so `APPLE_APP_SPECIFIC_PASSWORD` +is a natural guess and an easy mistake — the hook reads `APPLE_APP_PASSWORD`. If +it finds the longer name set instead, it says so by name rather than reporting +the variable as simply absent. + The Developer ID Application certificate must be installed in the build machine's Keychain, with private key marked as exportable. On GitHub Actions, install via `import-codesign-certs` action (from a @@ -155,50 +259,70 @@ codesign -dv --verbose=4 "TransTrack Enterprise.app" --- -## Verifying the local installation of the signer +## Testing the signing path without a certificate ```powershell -node tests/signWin.test.cjs +node tests/signWin.test.cjs # mode selection, fail-closed, TOTP, cert materialisation +node tests/notarize.test.cjs # notarization credential handling +node tests/artifactSignature.test.mjs # PE parsing and the release-gate verdict ``` -This validates the auto-detect logic, base32 / TOTP, and the -input-shape resolver without needing a real certificate. +All three run in `npm test` as part of the functional suite. None needs a real +certificate: the signer tests drive the decision logic with a synthetic +environment, and the signature tests synthesise PE images. On Windows the +signature suite additionally checks `node.exe` (genuinely signed, must be +accepted) and `notepad.exe` (catalog-signed, must be rejected). --- -## CI matrix (GitHub Actions example) +## CI + +`.github/workflows/release.yml` builds and signs on tag push. The relevant +environment for the Windows job: ```yaml -- name: Build signed installers - shell: pwsh - env: - TRANSTRACK_SIGN_MODE: ssl_esigner - ESIGNER_USERNAME: ${{ secrets.ESIGNER_USERNAME }} - ESIGNER_PASSWORD: ${{ secrets.ESIGNER_PASSWORD }} - ESIGNER_CREDENTIAL_ID: ${{ secrets.ESIGNER_CREDENTIAL_ID }} - ESIGNER_TOTP_SECRET: ${{ secrets.ESIGNER_TOTP_SECRET }} - ESIGNER_TOOL_PATH: C:\CodeSignTool\CodeSignTool.bat - APPLE_ID: ${{ secrets.APPLE_ID }} - APPLE_APP_PASSWORD: ${{ secrets.APPLE_APP_PASSWORD }} - APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} - CSC_LINK: ${{ secrets.MAC_DEVELOPER_ID_P12_BASE64 }} - CSC_KEY_PASSWORD: ${{ secrets.MAC_DEVELOPER_ID_P12_PASSWORD }} - run: | - npm ci - npm run build:all +env: + TRANSTRACK_RELEASE_CHANNEL: public # makes signing mandatory + TRANSTRACK_SIGN_MODE: ${{ vars.TRANSTRACK_SIGN_MODE || 'ssl_esigner' }} + ESIGNER_USERNAME: ${{ secrets.ESIGNER_USERNAME }} + ESIGNER_PASSWORD: ${{ secrets.ESIGNER_PASSWORD }} + ESIGNER_CREDENTIAL_ID: ${{ secrets.ESIGNER_CREDENTIAL_ID }} + ESIGNER_TOTP_SECRET: ${{ secrets.ESIGNER_TOTP_SECRET }} + ESIGNER_TOOL_URL: ${{ secrets.ESIGNER_TOOL_URL }} + ESIGNER_TOOL_PATH: ${{ vars.ESIGNER_TOOL_PATH || 'C:\CodeSignTool\CodeSignTool.bat' }} ``` +and for macOS: + +```yaml +env: + TRANSTRACK_RELEASE_CHANNEL: public # makes notarization mandatory + APPLE_ID: ${{ secrets.APPLE_ID }} + APPLE_APP_PASSWORD: ${{ secrets.APPLE_APP_PASSWORD }} + APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }} + CSC_LINK: ${{ secrets.MAC_DEVELOPER_ID_P12_BASE64 }} + CSC_KEY_PASSWORD: ${{ secrets.MAC_DEVELOPER_ID_P12_PASSWORD }} +``` + +Each job verifies its own output before uploading: the Windows job runs +`verify-artifact-signature.mjs` against the installer, and the macOS job runs +`spctl`/`codesign` against the app bundle. A build that produced an unsigned or +un-notarized artifact fails there even if the hooks somehow did not. + --- -## Cost reference +## What to do first + +If you have not bought anything yet, the shortest path to a signed Windows +installer is **Azure Artifact Signing** at roughly $10/month, with no hardware +token and no annual re-issue. It needs a new mode in `sign-win.cjs` (the Azure +signing tool has a different invocation than CodeSignTool) — that is a small, +self-contained piece of work, not a blocker. -| Item | Indicative cost (USD/year) | -|--------------------------------------------------|----------------------------| -| SSL.com EV Code Signing + eSigner Tier 1 (1 yr) | ~$330 (year 1 promo) → $499 | -| Certum EV Code Signing (1 yr) | ~$200 | -| DigiCert EV Code Signing + KeyLocker (1 yr) | ~$700 | -| Apple Developer Program (Organization) | $99 | -| D-U-N-S registration | Free | +If you want to ship with what is already implemented, buy an **OV certificate +with cloud HSM signing** (SSL.com eSigner is what `ssl_esigner` mode targets) +and skip EV unless a customer asks for it in writing. -These are reference numbers as of writing; reconfirm with the CAs at -purchase time. +macOS has no equivalent decision: Apple Developer Program Organization +enrolment at $99/year, and the D-U-N-S number takes about two weeks, so start +that first if a macOS build matters to you. diff --git a/docs/DEPLOYMENT_PRODUCTION.md b/docs/DEPLOYMENT_PRODUCTION.md index bce6e7d..4d5c36a 100644 --- a/docs/DEPLOYMENT_PRODUCTION.md +++ b/docs/DEPLOYMENT_PRODUCTION.md @@ -86,20 +86,32 @@ npm audit --production --audit-level=high ### 2.2 Configure Code Signing -Set environment variables for code signing: +Set environment variables for code signing. See `docs/CODE_SIGNING.md` for the +full reference, including cloud-HSM signing and how to choose a certificate. ```bash # Windows -set CSC_LINK=path/to/certificate.pfx +set TRANSTRACK_SIGN_MODE=pfx +set CSC_LINK=path/to/certificate.pfx # path, or base64 of the .pfx set CSC_KEY_PASSWORD=your-certificate-password # macOS export CSC_LINK=path/to/certificate.p12 export CSC_KEY_PASSWORD=your-certificate-password export APPLE_ID=your-apple-id -export APPLE_APP_SPECIFIC_PASSWORD=your-app-password +export APPLE_APP_PASSWORD=your-app-specific-password # not APPLE_APP_SPECIFIC_PASSWORD +export APPLE_TEAM_ID=your-10-char-team-id ``` +For a build you intend to distribute, also set: + +```bash +export TRANSTRACK_RELEASE_CHANNEL=public +``` + +This makes signing and notarization mandatory: if a credential is missing, the +build fails instead of quietly emitting an unsigned artifact. + ### 2.3 Build ```bash diff --git a/docs/compliance/RISK_REGISTER.md b/docs/compliance/RISK_REGISTER.md index fbd3b9c..63a0b9d 100644 --- a/docs/compliance/RISK_REGISTER.md +++ b/docs/compliance/RISK_REGISTER.md @@ -12,7 +12,7 @@ | Ver | Change | Rationale | |---|---|---| | 1.0 | Baseline. | Initial issue. | -| 1.1 | Added R-020 to R-027. Revised the mitigation for R-013. | New hazards arising from the IOTA notification pipeline, chart filing, and diagnostics export added in software version 1.2.1. R-013 was revised because its original mitigation — transactional rollback — does not cover a multi-migration sequence that fails partway, which is now addressed by a verified pre-migration copy. | +| 1.1 | Added R-020 to R-028. Revised the mitigation for R-013. | New hazards arising from the IOTA notification pipeline, chart filing, and diagnostics export added in software version 1.2.1. R-013 was revised because its original mitigation — transactional rollback — does not cover a multi-migration sequence that fails partway, which is now addressed by a verified pre-migration copy. | ## Severity scale @@ -76,3 +76,4 @@ Mitigations move risk to **Acceptable** when residual risk is **Low** or | R-025 | Hospital-authored notice template omits a content element required by § 512.442(d) | 2 | B | Templates are validated at configuration time against all five required elements and rejected if any is missing or an unrecognised placeholder is used. The organ-offer-eligibility statement is system-supplied and not editable through template configuration. | D | Low | Engineering | | R-026 | A documented dependency-vulnerability exception becomes a permanent, unreviewed suppression | 2 | C | Exceptions carry a `reviewBy` date and the release gate fails once it passes; the gate also fails on an undocumented finding, on a severity increase beyond what the exception assessed, and on an exception that no longer matches any real finding. | D | Med (Acceptable) | Engineering | | R-027 | A feature works in development but is unwired in the packaged build, failing first in front of a clinician | 3 | B | Every `api..()` call in the renderer is checked against the real preload surface by automated test; the source entry point is guarded against being overwritten by a build artifact; the release gate verifies the installer version matches the source version. | D | Low | Engineering | +| R-028 | An unsigned or tampered installer is distributed and accepted at a site as authentic vendor software | 2 | C | A build designated for distribution fails rather than emitting an unsigned artifact, and names the missing credential; the release gate inspects the produced installer for an embedded Authenticode signature instead of trusting the build configuration; catalog-only signatures are rejected because they cannot travel with a download; each CI build job verifies its own artifact before upload. | E | Low | Engineering | diff --git a/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md b/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md index dac7be7..85eada9 100644 --- a/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md +++ b/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md @@ -352,3 +352,40 @@ point: a stub would drift and re-open the gap it exists to close. `tests/buildEntryIntegrity.test.mjs` asserts that the source `index.html` still loads `/src/main.jsx` and carries no hashed build-output references, guarding against a build artifact overwriting the source entry point. + +## 17. Release artifact authenticity + +A receiving site's only means of confirming that an installer came from the +vendor and arrived unmodified is its code signature. Two controls protect that +property. + +**Fail closed on a designated release.** `scripts/sign-win.cjs` and +`scripts/notarize.cjs` both distinguish a developer build, where a missing +certificate is a warning, from a distribution build, where it is a build +failure. The distinction is drawn from `TRANSTRACK_RELEASE_CHANNEL=public` or +the explicit `TRANSTRACK_REQUIRE_SIGNING` / `TRANSTRACK_REQUIRE_NOTARIZATION` +flags; `.github/workflows/release.yml` sets the former on both build jobs. A +mode named explicitly whose credentials are incomplete is also an error rather +than a fall-through to `skip`, and the error names the missing variable. + +The previous fail-open behaviour was the more dangerous configuration precisely +because it was quiet: the build went green, and the operator's belief that the +artifact was signed was never contradicted until a customer's machine refused +it. + +**Verify the artifact, not the intent.** `scripts/verify-artifact-signature.mjs` +inspects the produced installer. On Windows it takes the operating system's +verdict via `Get-AuthenticodeSignature` and requires `Valid`. On other platforms +— the release gate runs on Linux — it parses the PE Attribute Certificate Table +directly, which establishes that a signature is embedded but not that it chains +to a trusted root; the result records that reduced assurance explicitly rather +than overstating what was checked. + +A catalog-only signature is rejected even when Windows reports it valid. Catalog +signatures reside in a system-wide store rather than in the file, so they do not +travel with a downloaded installer and cannot serve as evidence of authenticity +at the receiving site. + +`scripts/release-readiness-check.mjs` calls the verifier, so the gate's +"code-signed installer present" item now reflects the artifact's actual contents +rather than its filename. diff --git a/docs/compliance/SYSTEM_REQUIREMENTS_SPECIFICATION.md b/docs/compliance/SYSTEM_REQUIREMENTS_SPECIFICATION.md index 72d319c..1e26001 100644 --- a/docs/compliance/SYSTEM_REQUIREMENTS_SPECIFICATION.md +++ b/docs/compliance/SYSTEM_REQUIREMENTS_SPECIFICATION.md @@ -148,3 +148,5 @@ D=Demonstration). All `M` requirements must trace to at least one OQ test case. | TT-R143 | M | The system shall provide an "About" dialog stating the regulatory design alignment (not certification). | I | | TT-R144 | M | The release gate shall fail on any dependency vulnerability that is not covered by a documented exception, whose severity exceeds what its exception assessed, or whose exception has passed its review date. An exception that no longer matches a real finding shall also fail the gate, so that the exception set cannot silently diverge from the dependency tree. | T | | TT-R145 | M | Every renderer call to an inter-process API shall be verified against the actual bridge surface exposed by the main process, and the release gate shall fail if the packaged application's version does not match the source version. | T | +| TT-R146 | M | A build designated for distribution shall fail if the produced artifact cannot be code signed, rather than emitting an unsigned artifact with a warning. The same shall apply to macOS notarization. | T | +| TT-R147 | M | The release gate shall determine whether a distributed installer is signed by inspecting the artifact itself, not by inspecting its filename or the build configuration. A signature that is not embedded in the artifact shall not satisfy this requirement, because it cannot accompany the artifact to the receiving site. | T | diff --git a/docs/compliance/TRACEABILITY_MATRIX.md b/docs/compliance/TRACEABILITY_MATRIX.md index 6f6dc57..acf692e 100644 --- a/docs/compliance/TRACEABILITY_MATRIX.md +++ b/docs/compliance/TRACEABILITY_MATRIX.md @@ -93,6 +93,8 @@ every Mandatory requirement, and resolvable SDS, OQ and risk references. | TT-R143 | §2 | `electron/main.cjs` About menu | OQ-143 | | TT-R144 | §15 | `scripts/audit-with-exceptions.mjs`, `security/vulnerability-exceptions.json` | `tests/auditExceptions.test.mjs`; OQ-144 | | TT-R145 | §16 | `tests/rendererBridgeCoverage.test.mjs`, `tests/buildEntryIntegrity.test.mjs`, `scripts/release-readiness-check.mjs` (installer version check) | `tests/rendererBridgeCoverage.test.mjs`, `tests/buildEntryIntegrity.test.mjs`; OQ-145 | +| TT-R146 | §17 | `scripts/sign-win.cjs`, `scripts/notarize.cjs`, `.github/workflows/release.yml` | `tests/signWin.test.cjs`, `tests/notarize.test.cjs`; OQ-146 | +| TT-R147 | §17 | `scripts/verify-artifact-signature.mjs`, `scripts/release-readiness-check.mjs` | `tests/artifactSignature.test.mjs`; OQ-147 | ## Risk linkage @@ -110,3 +112,4 @@ Requirement groups added in software version 1.2.1: | R-025 Template omits a statutory element | TT-R077, TT-R078, TT-R131 | | R-026 Vulnerability exception becomes permanent | TT-R144 | | R-027 Feature unwired in packaged build | TT-R145 | +| R-028 Unsigned build distributed as authentic | TT-R146, TT-R147 | diff --git a/docs/compliance/templates/OQ_PROTOCOL_TEMPLATE.md b/docs/compliance/templates/OQ_PROTOCOL_TEMPLATE.md index ede80a0..669d75a 100644 --- a/docs/compliance/templates/OQ_PROTOCOL_TEMPLATE.md +++ b/docs/compliance/templates/OQ_PROTOCOL_TEMPLATE.md @@ -140,6 +140,8 @@ Executed on a copy of a populated non-PHI test database. | OQ-143 | Open About dialog. | Design alignment statement present (not "certified"). | | | | OQ-144 | Review the release evidence for the build under test: the dependency audit output and the exception file. | Every finding is either resolved or covered by an unexpired documented exception. Confirm by back-dating one exception's review date in a scratch copy that the gate then fails. | | | | OQ-145 | Confirm the installed application's version matches the version in the release record; exercise each administrative screen including Disaster Recovery and System Health. | Versions match; every control performs its action rather than failing at the bridge. | | | +| OQ-146 | Review the release build log for the artifact under test. | The signing and notarization steps report success. Confirm the control is real by inspecting the vendor's evidence that a build with a deliberately removed signing credential failed rather than producing an unsigned artifact. | | | +| OQ-147 | On the installation host, run `Get-AuthenticodeSignature` against the received installer before installing it. | `Status` is `Valid`, `SignatureType` is `Authenticode` (not `Catalog`), and the signer certificate subject matches the vendor named in the purchase agreement. | | | ## Acceptance diff --git a/docs/legal/COMMERCIALIZATION_CHECKLIST.md b/docs/legal/COMMERCIALIZATION_CHECKLIST.md index 023ef21..5baa164 100644 --- a/docs/legal/COMMERCIALIZATION_CHECKLIST.md +++ b/docs/legal/COMMERCIALIZATION_CHECKLIST.md @@ -84,15 +84,28 @@ the four required GitHub Actions secrets. ### Vendor list -| Cert | Vendor | Cost | Mode | -| ----------------------------------------- | ------------------------------------------ | --------------- | ---------------------- | -| Windows EV Code Signing (Authenticode) | SSL.com eSigner (cloud HSM) | ~$300/yr | `TRANSTRACK_SIGN_MODE=ssl_esigner` | -| Windows EV Code Signing (USB token) | DigiCert / Sectigo / SSL.com (hardware token) | ~$300–$700/yr | `TRANSTRACK_SIGN_MODE=pfx` | -| Apple Developer Program | Apple | $99/yr | `APPLE_*` secrets | - -**Recommendation:** SSL.com eSigner. It's cloud-HSM-backed, eliminates -the lost-USB-token nightmare, and works out of the box with the existing -CI workflow. +| Cert | Vendor | Cost | Mode | +| -------------------------------------- | --------------------------------------------- | ------------- | ---------------------------------- | +| Windows — Azure Artifact Signing | Microsoft | ~$10/mo | *not yet implemented in the signer* | +| Windows OV Code Signing (cloud HSM) | SSL.com eSigner / DigiCert KeyLocker | ~$150–300/yr | `TRANSTRACK_SIGN_MODE=ssl_esigner` | +| Windows OV/EV Code Signing (USB token) | DigiCert / Sectigo / SSL.com (hardware token) | ~$300–$700/yr | `TRANSTRACK_SIGN_MODE=pfx` | +| Apple Developer Program (Organization) | Apple | $99/yr | `APPLE_*` secrets | + +**On EV:** older guidance said EV was required to avoid SmartScreen warnings. +Microsoft has since removed that behaviour — EV and OV now give the same +first-download experience, and reputation accrues per file hash either way. Buy +EV only if a customer's procurement process names it. + +**Recommendation:** Azure Artifact Signing is the cheapest and least +operationally painful route, but needs a new mode in `scripts/sign-win.cjs` +(small, self-contained). To ship with what exists today, buy an OV certificate +with cloud-HSM signing — `ssl_esigner` mode works out of the box with the +existing CI workflow and avoids the lost-USB-token failure mode. + +Note that since June 2023 the CA/Browser Forum requires *all* code signing +private keys, OV included, to live in hardware. A copyable `.pfx` is no longer +issuable, so `pfx` mode is for certificates you already hold and for test +signing. ### GitHub Actions secrets to set (settings → secrets and variables → actions) @@ -101,9 +114,11 @@ ESIGNER_USERNAME ESIGNER_PASSWORD ESIGNER_CREDENTIAL_ID ESIGNER_TOTP_SECRET +ESIGNER_TOOL_URL (download URL for CodeSignTool, from the SSL.com dashboard) APPLE_ID -APPLE_APP_PASSWORD (app-specific password from appleid.apple.com) +APPLE_APP_PASSWORD (app-specific password from appleid.apple.com — + note the name: not APPLE_APP_SPECIFIC_PASSWORD) APPLE_TEAM_ID APPLE_CERT_BASE64 (base64 of your Developer ID Application .p12) APPLE_CERT_PASSWORD @@ -116,9 +131,12 @@ git tag v1.3.0-rc1 git push origin v1.3.0-rc1 ``` -If credentials are missing, the `preflight` job will fail with a clear -error message. If credentials are present, you'll get signed installers -in the GitHub Releases artifact set within ~25 minutes. +Release builds set `TRANSTRACK_RELEASE_CHANNEL=public`, which makes signing and +notarization mandatory. A missing credential now fails the build and names the +variable, and each job independently verifies its own artifact before upload — +so a green release means a genuinely signed installer, not just a hook that +chose to skip. With credentials present you'll get signed installers in the +GitHub Releases artifact set within ~25 minutes. --- @@ -334,7 +352,7 @@ mark every line: - [ ] **C-2-d** Vendor domain owned (e.g., transtrack.health) - [ ] **C-2-e** Workspace email live for sales@, support@, security@ - [ ] **C-2-f** Privacy Policy + ToS published at the vendor domain -- [ ] **C-3-a** EV Code Signing certificate purchased and provisioned +- [ ] **C-3-a** Windows code signing certificate purchased and provisioned (OV is sufficient — see "On EV" above) - [ ] **C-3-b** Apple Developer Program enrolled, notarization creds in env - [ ] **C-3-c** GitHub Actions secrets set for both platforms - [ ] **C-3-d** Test release tag (`v1.3.0-rc1`) successfully signed in CI From f22ff42b27bff29ebe4d0f8a622729d553a63ac1 Mon Sep 17 00:00:00 2001 From: NeuroKoder3 Date: Sat, 1 Aug 2026 19:43:14 -0500 Subject: [PATCH 3/4] fix(release): do not call an artifact unsigned when the machine cannot tell The Windows CI job caught this on the suite's first run there: node.exe, which is genuinely signed, was reported UNSIGNED. Two causes, both of which would have failed a legitimate release. verifyAuthenticode defaulted the status to the literal string 'Unknown' when the regex found no STATUS line, so a PowerShell that produced no output at all became a verdict of "not Valid". It now returns available:false with the reason -- powershell failed to start, or exited without a verdict -- and the caller falls back to the PE certificate table, the same evidence a Linux host uses. Windows also has a real UnknownError status, meaning it could not complete trust evaluation, most often an offline revocation check. That is an absence of a conclusion, not a negative one, and is now a downgrade to 'embedded' assurance rather than a rejection. The genuinely negative statuses are unaffected: a forged certificate table answers NotSigned, which is a conclusion, and is still rejected -- verified against a fixture. Reordered the embedded-signature check ahead of the OS verdict, since whether a signature is inside the file is read from the file and does not depend on the OS being able to evaluate it. A catalog-signed binary is therefore still rejected on a machine that cannot validate chains. The two real-binary tests now adapt: they require the signature to be found either way, and assert full assurance only where trust evaluation actually works. Co-authored-by: Cursor --- scripts/verify-artifact-signature.mjs | 117 +++++++++++++++----------- tests/artifactSignature.test.mjs | 51 +++++++++-- 2 files changed, 115 insertions(+), 53 deletions(-) diff --git a/scripts/verify-artifact-signature.mjs b/scripts/verify-artifact-signature.mjs index 6fb11ad..875eb3d 100644 --- a/scripts/verify-artifact-signature.mjs +++ b/scripts/verify-artifact-signature.mjs @@ -81,10 +81,19 @@ export function readEmbeddedSignature(filePath) { /** * Ask Windows whether the signature is valid and who signed it. * - * @returns {{ available: false } | { available: true, status: string, subject: string|null, valid: boolean }} + * Returns `available: false` — with a reason — whenever no verdict was + * obtained, rather than inventing a placeholder status. The distinction is not + * academic: a missing verdict means "this machine could not tell us", and + * reporting that as "not valid" would condemn a correctly signed artifact + * because of something about the build machine. + * + * @returns {{ available: false, reason: string } + * | { available: true, status: string, kind: string, subject: string|null, valid: boolean }} */ export function verifyAuthenticode(filePath) { - if (process.platform !== 'win32') return { available: false }; + if (process.platform !== 'win32') { + return { available: false, reason: `Get-AuthenticodeSignature does not exist on ${process.platform}` }; + } const ps = spawnSync( 'powershell', @@ -100,11 +109,19 @@ export function verifyAuthenticode(filePath) { { encoding: 'utf8' }, ); - if (ps.error || typeof ps.stdout !== 'string') return { available: false }; + if (ps.error) { + return { available: false, reason: `could not run powershell: ${ps.error.message}` }; + } + + const stdout = typeof ps.stdout === 'string' ? ps.stdout : ''; + const status = /STATUS=(.+)/.exec(stdout)?.[1]?.trim(); + if (!status) { + const why = (ps.stderr || '').trim().split(/\r?\n/)[0] || `powershell exited ${ps.status}`; + return { available: false, reason: `Get-AuthenticodeSignature returned no verdict (${why})` }; + } - const status = /STATUS=(.*)/.exec(ps.stdout)?.[1]?.trim() || 'Unknown'; - const kind = /KIND=(.*)/.exec(ps.stdout)?.[1]?.trim() || 'Unknown'; - const subjectRaw = /SUBJECT=(.*)/.exec(ps.stdout)?.[1]?.trim() || ''; + const kind = /KIND=(.+)/.exec(stdout)?.[1]?.trim() || 'Unknown'; + const subjectRaw = /SUBJECT=(.+)/.exec(stdout)?.[1]?.trim() || ''; return { available: true, @@ -124,56 +141,62 @@ export function inspectWindowsArtifact(filePath) { if (!existsSync(filePath)) throw new Error(`artifact not found: ${filePath}`); const embedded = readEmbeddedSignature(filePath); - const authenticode = verifyAuthenticode(filePath); - - if (authenticode.available) { - // Windows is authoritative on validity, so ask it first rather than - // inferring from the file layout. - if (!authenticode.valid) { - return { - signed: false, - assurance: 'none', - detail: `Authenticode status is ${authenticode.status}, not Valid`, - }; - } - - // Valid, but is the signature actually part of the file? Windows reports - // catalog-signed system binaries as Valid even though nothing is embedded — - // notepad.exe is the canonical example. A catalog lives on the machine that - // installed it, so it cannot travel with a download: an installer we hand a - // hospital must carry its signature inside the file. - if (!embedded.present) { - return { - signed: false, - assurance: 'none', - detail: - `signature is ${authenticode.kind}-based, not embedded in the file. ` + - `A distributed installer must carry an embedded Authenticode signature, ` + - `because a catalog signature does not travel with the download`, - }; - } - - const who = authenticode.subject - ? authenticode.subject.split(',')[0].replace(/^CN=/, '').trim() - : 'unknown signer'; - return { signed: true, assurance: 'valid', detail: `Valid — signed by ${who}` }; - } + const os = verifyAuthenticode(filePath); - // Not on Windows: the PE certificate table is the only evidence available. + // An installer must carry its signature inside the file. Windows reports + // catalog-signed binaries as Valid — notepad.exe is the canonical example — + // but a catalog lives on the machine that installed it and cannot travel with + // a download, so it is worthless as evidence at the receiving site. if (!embedded.present) { + const catalog = os.available && os.valid; return { signed: false, assurance: 'none', - detail: 'no Authenticode signature is embedded in the executable', + detail: catalog + ? `signature is ${os.kind}-based, not embedded in the file. A distributed ` + + `installer must carry an embedded Authenticode signature, because a ` + + `catalog signature does not travel with the download` + : 'no Authenticode signature is embedded in the executable', + }; + } + + if (!os.available) { + return { + signed: true, + assurance: 'embedded', + detail: `signature present (${embedded.size} bytes); validity not checked — ${os.reason}`, + }; + } + + if (os.valid) { + const who = os.subject + ? os.subject.split(',')[0].replace(/^CN=/, '').trim() + : 'unknown signer'; + return { signed: true, assurance: 'valid', detail: `Valid — signed by ${who}` }; + } + + // UnknownError is Windows saying it could not reach a conclusion, not that + // the signature is bad — most often a revocation check that needs network the + // build machine does not have. Every other non-Valid status is a conclusion: + // NotSigned when the certificate table holds no usable PKCS#7, HashMismatch + // when the file changed after signing, NotTrusted when the chain was built + // and rejected. Those are rejections; this one is a downgrade to the same + // assurance a non-Windows host gives. + if (os.status === 'UnknownError') { + return { + signed: true, + assurance: 'embedded', + detail: + `signature present (${embedded.size} bytes) but Windows could not complete ` + + `trust evaluation (UnknownError) — commonly an offline revocation check. ` + + `Confirm on a networked host before distributing`, }; } return { - signed: true, - assurance: 'embedded', - detail: - `signature present (${embedded.size} bytes); validity not checked ` + - `because Get-AuthenticodeSignature is unavailable on ${process.platform}`, + signed: false, + assurance: 'none', + detail: `Authenticode status is ${os.status}, not Valid`, }; } diff --git a/tests/artifactSignature.test.mjs b/tests/artifactSignature.test.mjs index bd815d7..72248af 100644 --- a/tests/artifactSignature.test.mjs +++ b/tests/artifactSignature.test.mjs @@ -22,6 +22,7 @@ import { join } from 'node:path'; import { readEmbeddedSignature, + verifyAuthenticode, inspectWindowsArtifact, } from '../scripts/verify-artifact-signature.mjs'; @@ -151,16 +152,42 @@ test('on Windows, a fixture with a fake certificate table is rejected as invalid console.log(' (skipped off Windows)'); return; } - // The table points at filler, not a PKCS#7 blob, so Windows must not call it - // Valid. This is the case the PE-only check cannot catch. + // The table points at filler, not a PKCS#7 blob. Windows answers NotSigned — + // a conclusion, not an inability to reach one — so this must be rejected even + // though a certificate table is present. It is the case the PE-only check + // cannot catch. const p = fixture('verdict-fake.exe', makePe({ certOffset: 0x300, certSize: 0x40 })); const r = inspectWindowsArtifact(p); assert.strictEqual(r.signed, false, 'a forged certificate table must not pass on Windows'); }); +console.log('\nWhen the OS cannot reach a verdict'); + +test('an absent Windows verdict is reported as unavailable, not as invalid', () => { + // A build machine that cannot complete a revocation check, or a PowerShell + // that fails to start, says nothing about the artifact. Treating silence as + // "not valid" would fail a correctly signed release for a reason that has + // nothing to do with the file — which is exactly what happened on a hosted + // runner the first time this suite ran there. + const v = verifyAuthenticode(join(SANDBOX, 'absent.exe')); + if (v.available) { + // The file does not exist, so a verdict here can only be a negative one. + assert.notStrictEqual(v.status, undefined); + assert.strictEqual(v.valid, false); + } else { + assert.ok(v.reason, 'unavailability must carry a reason the operator can act on'); + } +}); + console.log('\nReal binaries (Windows only)'); -test('a genuinely signed executable is accepted with full assurance', () => { +/** True when this machine can actually evaluate a trust chain. */ +function osVerdictWorks() { + const v = verifyAuthenticode(process.execPath); + return v.available && v.valid; +} + +test('a genuinely signed executable is accepted', () => { if (process.platform !== 'win32') { console.log(' (skipped off Windows)'); return; @@ -168,7 +195,17 @@ test('a genuinely signed executable is accepted with full assurance', () => { // node.exe carries an embedded Authenticode signature. const r = inspectWindowsArtifact(process.execPath); assert.strictEqual(r.signed, true, `expected ${process.execPath} to be signed: ${r.detail}`); - assert.strictEqual(r.assurance, 'valid'); + + if (osVerdictWorks()) { + assert.strictEqual(r.assurance, 'valid'); + } else { + // Some build environments cannot validate a chain at all. The verifier must + // still find the signature and must say plainly that it did not confirm + // trust, rather than claiming either more or less than it knows. + assert.strictEqual(r.assurance, 'embedded'); + assert.match(r.detail, /not checked|could not complete/); + console.log(' (trust evaluation unavailable here — checked the degraded path instead)'); + } }); test('a catalog-signed system binary is rejected for distribution', () => { @@ -176,7 +213,7 @@ test('a catalog-signed system binary is rejected for distribution', () => { console.log(' (skipped off Windows)'); return; } - const notepad = 'C:\\Windows\\System32\\notepad.exe'; + const notepad = join(process.env.SystemRoot || 'C:\\Windows', 'System32', 'notepad.exe'); if (!existsSync(notepad)) { console.log(' (skipped — notepad.exe not present)'); return; @@ -184,9 +221,11 @@ test('a catalog-signed system binary is rejected for distribution', () => { // Windows reports this Valid, but the signature lives in a system catalog // rather than in the file. A catalog cannot travel with a download, so an // installer signed only this way would arrive at a customer unverifiable. + // This holds whether or not the OS verdict is available, because the + // deciding fact — nothing embedded in the file — is read from the file. const r = inspectWindowsArtifact(notepad); assert.strictEqual(r.signed, false, 'catalog-only signing must not satisfy the release gate'); - assert.match(r.detail, /not embedded/); + assert.match(r.detail, /not embedded|no Authenticode signature is embedded/); }); rmSync(SANDBOX, { recursive: true, force: true }); From faa4b42c4d119687738de2665221af5d52a68c04 Mon Sep 17 00:00:00 2001 From: NeuroKoder3 Date: Sat, 1 Aug 2026 19:52:22 -0500 Subject: [PATCH 4/4] test(release): gate the forged-signature assertion on the OS being able to look The remaining Windows CI failure was the test's premise, not the verifier. It asserted that a fixture with a fake certificate table is rejected, which is only true where Get-AuthenticodeSignature answers -- and on that runner it answers nothing at all, even for node.exe. Without an OS verdict a forged table is indistinguishable from a real signature by file layout alone. So the assertion now splits along what the environment can actually establish: reject where the OS can look, and where it cannot, require that the verifier does not claim validity it did not check. The log records why no verdict was available, so the next new runner is diagnosable without a reproduction. Documented the consequence in SDS section 17 and CODE_SIGNING.md: the strongest evidence obtainable depends on where the check runs, which is why OQ-147 has the receiving site verify the installer on its own hardware -- the one execution of this check that does not depend on the vendor's build environment. Co-authored-by: Cursor --- docs/CODE_SIGNING.md | 6 +++ .../SOFTWARE_DESIGN_SPECIFICATION.md | 18 ++++++- tests/artifactSignature.test.mjs | 51 ++++++++++++++----- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/docs/CODE_SIGNING.md b/docs/CODE_SIGNING.md index 98c95c2..1eb58f2 100644 --- a/docs/CODE_SIGNING.md +++ b/docs/CODE_SIGNING.md @@ -193,6 +193,12 @@ A **catalog-only** signature is rejected even though Windows reports it `Valid`. Catalog signatures live in a system-wide `.cat` file, not in the executable, so they do not survive a download to a customer's machine. +If the machine cannot evaluate a trust chain at all — no network for the +revocation check, or a `Get-AuthenticodeSignature` that returns nothing, which +is what some hosted runners do — the check reports that a signature is embedded +and says plainly that validity was not established, rather than reporting the +artifact as unsigned. Always confirm on a networked host before distributing. + To inspect the signer identity: ```powershell diff --git a/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md b/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md index 85eada9..a49d825 100644 --- a/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md +++ b/docs/compliance/SOFTWARE_DESIGN_SPECIFICATION.md @@ -384,7 +384,23 @@ than overstating what was checked. A catalog-only signature is rejected even when Windows reports it valid. Catalog signatures reside in a system-wide store rather than in the file, so they do not travel with a downloaded installer and cannot serve as evidence of authenticity -at the receiving site. +at the receiving site. Because the deciding fact is read from the file, this +rejection holds even where the operating system cannot be consulted. + +The absence of an operating system verdict is distinguished from a negative one. +Not every host can evaluate a trust chain — a build machine without network +cannot complete a revocation check, and one hosted runner returns no verdict at +all — and treating that silence as "not signed" would reject a correctly signed +artifact for a reason having nothing to do with the artifact. Where no verdict +is obtainable, the result is the same reduced assurance a non-Windows host +reports, with the cause stated. Windows' own `UnknownError` is treated the same +way, since it means the same thing. Statuses that are conclusions — `NotSigned`, +`HashMismatch`, `NotTrusted` — remain rejections. + +This means the strongest available evidence depends on where the check runs, so +OQ-147 has the receiving site verify the installer on its own hardware. That is +the only execution of this check that does not depend on the vendor's build +environment being able to answer. `scripts/release-readiness-check.mjs` calls the verifier, so the gate's "code-signed installer present" item now reflects the artifact's actual contents diff --git a/tests/artifactSignature.test.mjs b/tests/artifactSignature.test.mjs index 72248af..34db42e 100644 --- a/tests/artifactSignature.test.mjs +++ b/tests/artifactSignature.test.mjs @@ -147,18 +147,46 @@ test('off Windows, an embedded signature is accepted with reduced assurance', () assert.match(r.detail, /validity not checked/); }); -test('on Windows, a fixture with a fake certificate table is rejected as invalid', () => { +/** + * Whether this machine can actually evaluate a trust chain. + * + * Not every Windows host can. A hosted CI runner returned no verdict at all for + * node.exe, which is genuinely signed. Where that is the case the verifier + * degrades to the same evidence a Linux host has — a signature is embedded, its + * trust unestablished — and the assertions below have to degrade with it rather + * than assert a capability the environment does not have. + */ +let osProbe = null; +function osVerdict() { + if (osProbe === null) osProbe = verifyAuthenticode(process.execPath); + return osProbe; +} +function osVerdictWorks() { + const v = osVerdict(); + return v.available && v.valid; +} + +test('a fixture with a fake certificate table does not pass as validly signed', () => { if (process.platform !== 'win32') { console.log(' (skipped off Windows)'); return; } - // The table points at filler, not a PKCS#7 blob. Windows answers NotSigned — - // a conclusion, not an inability to reach one — so this must be rejected even - // though a certificate table is present. It is the case the PE-only check - // cannot catch. + // The table points at filler, not a PKCS#7 blob. This is the case PE parsing + // alone cannot catch, so what can be asserted depends on whether the OS is + // able to look. const p = fixture('verdict-fake.exe', makePe({ certOffset: 0x300, certSize: 0x40 })); const r = inspectWindowsArtifact(p); - assert.strictEqual(r.signed, false, 'a forged certificate table must not pass on Windows'); + + if (osVerdictWorks()) { + // Windows answers NotSigned — a conclusion, not an inability to reach one. + assert.strictEqual(r.signed, false, 'a forged certificate table must be rejected'); + } else { + // Without an OS verdict the forgery is indistinguishable from a real + // signature by file layout alone. The verifier must not claim otherwise. + assert.notStrictEqual(r.assurance, 'valid', 'must not claim validity it could not check'); + assert.match(r.detail, /not checked|could not complete/); + console.log(' (no OS verdict here — checked that validity is not claimed)'); + } }); console.log('\nWhen the OS cannot reach a verdict'); @@ -181,12 +209,6 @@ test('an absent Windows verdict is reported as unavailable, not as invalid', () console.log('\nReal binaries (Windows only)'); -/** True when this machine can actually evaluate a trust chain. */ -function osVerdictWorks() { - const v = verifyAuthenticode(process.execPath); - return v.available && v.valid; -} - test('a genuinely signed executable is accepted', () => { if (process.platform !== 'win32') { console.log(' (skipped off Windows)'); @@ -204,7 +226,10 @@ test('a genuinely signed executable is accepted', () => { // trust, rather than claiming either more or less than it knows. assert.strictEqual(r.assurance, 'embedded'); assert.match(r.detail, /not checked|could not complete/); - console.log(' (trust evaluation unavailable here — checked the degraded path instead)'); + // Print why, so a future failure on a new runner is diagnosable from the + // log rather than needing a reproduction. + const v = osVerdict(); + console.log(` (no trust evaluation here: ${v.available ? `status ${v.status}` : v.reason})`); } });