Fix the eSigner signing path and remove Azure Artifact Signing - #228
Merged
Conversation
Azure Artifact Signing requires an organisation verifiable for three years or more, which TransTrack Medical Software does not meet, so the mode could not have been used. Leaving it in would also have left the validation package describing a signing path that cannot be demonstrated on request. That makes SSL.com eSigner with an OV certificate the production route, so it was audited against SSL.com's CodeSignTool command guide. It would have failed on the first real attempt for three independent reasons: - -totp_secret takes the secret from the dashboard, not a code. CodeSignTool derives the six-digit code itself. The signer derived one and passed that, which would have surfaced as "invalid otp" and looked like broken 2FA. - -overwrite is not a CodeSignTool option, so every invocation would have been rejected. Signing now writes to a temporary directory and moves the result over the original, which also avoids the interactive overwrite confirmation that would hang CI outright. - Arguments were passed as "-name value"; SSL.com documents "-name=value". Two further problems in the same path: a password containing shell metacharacters was silently corrupted, because CodeSignTool is a batch file and its arguments are re-parsed by cmd.exe (SSL.com's own example password is P!@^^ssword12). Values are now quoted, and the two characters that cannot survive quoting are refused with an explanation rather than sent wrong. And a zero exit status was treated as proof of signing, which CodeSignTool does not guarantee; every mode now re-reads the artifact and fails unless it carries an embedded signature. The release gate now also rejects an unrecognised TRANSTRACK_SIGN_MODE instead of silently falling through to auto-detect, so a stale azure setting fails loudly. Co-authored-by: Cursor <cursoragent@cursor.com>
|
Strix is installed on this repository, but we couldn't run this PR security review because this workspace's trial has ended. Add a card to resume code reviews here. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Removes Azure Artifact Signing and makes SSL.com eSigner with an OV certificate the production Windows signing route — then fixes that route, which had never been run against a real certificate and would not have worked.
Azure Artifact Signing requires an organisation verifiable for three years or more. TransTrack Medical Software does not meet that bar, so the mode could not have been used, and keeping it would have left the validation package describing a signing path that cannot be demonstrated on request.
Defects found in
ssl_esignerAudited against SSL.com's CodeSignTool command guide. Three would each have been fatal on the first attempt:
-totp_secretwas given a generated 6-digit codeError: invalid otp, which reads like a broken 2FA enrolment.-overwriteis not a CodeSignTool option-name value-name=valuethroughout.Two more in the same path:
.bat, so its arguments are re-parsed bycmd.exe— SSL.com's own example password isP!@^^ssword12. Values are now quoted, and the two characters that cannot survive quoting ("and%) are refused up front with an explanation rather than sent wrong and surfacing as an authentication failure. The PFX path no longer uses a shell at all, so Node quotes for it.The now-unused RFC 6238 TOTP helpers were removed rather than left as an attractive nuisance.
Also
The release gate rejected nothing when handed
TRANSTRACK_SIGN_MODE=azure— it silently fell through to auto-detect and reported a pass for a mode nobody asked for. An unrecognised mode, andskip, now fail explicitly.Test plan
signWin.test.cjs— 26 pass, including the CodeSignTool argument contract, the password-quoting rules, and the post-sign check (verified against a genuinely signednode.exeon Windows)coresuite — 50/50 passnpm run lint,npm run typecheck— cleancomplianceDocs.test.mjs— traceability intact after the SDS editssl_esigner(pass),azure(fails as unsupported),skip(fails as unsigned)release.ymlparses; jobs unchangedNot verifiable here: an end-to-end signature against a real certificate. That needs the purchased OV certificate. The argument contract is now pinned to SSL.com's published spec by tests, so the remaining risk is account setup rather than code.