fix: don't duplicate /tr, /td, /fd when supplied via signWithParams

src/sign-with-signtool.ts

@@ -66,6 +66,25 @@ export function getSigntoolArgs(options: InternalSignToolOptions) {
   const { certificateFile, certificatePassword, hash, timestampServer } = options;
   const args = ['sign'];
 
+  // Parse signWithParams up front so we can avoid appending defaults for any
+  // flag the user already supplied. Passing e.g. /tr or /fd twice makes
+  // signtool.exe error on the duplicate. See issue #46.
+  let extraArgs: Array<string> = [];
+  if (options.signWithParams) {
+    if (Array.isArray(options.signWithParams)) {
+      // The array form is a verbatim passthrough - each entry is already a
+      // discrete argument, so we must not re-parse or strip quotes from it.
+      extraArgs = [...options.signWithParams];
+    } else {
+      // Split up at spaces, keeping double-quoted spans together and stripping
+      // the surrounding quotes (see parseSignWithParams).
+      extraArgs = parseSignWithParams(options.signWithParams);
+    }
+    log('Parsed signWithParams as:', extraArgs);
+  }
+
+  const hasUserFlag = (flag: string) => extraArgs.includes(flag);
+
   // Automatically select cert
   if (options.automaticallySelectCertificate) {
     args.push('/a');
@@ -78,8 +97,12 @@ export function getSigntoolArgs(options: InternalSignToolOptions) {
 
   // Timestamp
   if (hash === HASHES.sha256) {
-    args.push('/tr', timestampServer);
-    args.push('/td', hash);
+    if (!hasUserFlag('/tr')) {
+      args.push('/tr', timestampServer);
+    }
+    if (!hasUserFlag('/td')) {
+      args.push('/td', hash);
+    }
   } else {
     args.push('/t', timestampServer);
   }
@@ -95,7 +118,9 @@ export function getSigntoolArgs(options: InternalSignToolOptions) {
   }
 
   // Hash
-  args.push('/fd', hash);
+  if (!hasUserFlag('/fd')) {
+    args.push('/fd', hash);
+  }
 
   // Description
   if (options.description) {
@@ -112,21 +137,7 @@ export function getSigntoolArgs(options: InternalSignToolOptions) {
     args.push('/debug');
   }
 
-  if (options.signWithParams) {
-    const extraArgs: Array<string> = [];
-
-    if (Array.isArray(options.signWithParams)) {
-      // The array form is a verbatim passthrough - each entry is already a
-      // discrete argument, so we must not re-parse or strip quotes from it.
-      extraArgs.push(...options.signWithParams);
-    } else {
-      // Split up at spaces, keeping double-quoted spans together and stripping
-      // the surrounding quotes (see parseSignWithParams).
-      extraArgs.push(...parseSignWithParams(options.signWithParams));
-    }
-
-    log('Parsed signWithParams as:', extraArgs);
-
+  if (extraArgs.length > 0) {
     args.push(...extraArgs);
   }
 

test/sign-with-signtool.spec.ts

@@ -109,6 +109,22 @@ void describe('getSigntoolArgs signWithParams handling', async () => {
     assert.ok(!args.includes('"DigiCert Signing Manager KSP"'));
   });
 
+  // Regression test for https://github.com/electron/windows-sign/issues/46:
+  // when the user supplies /tr, /td, or /fd via signWithParams, getSigntoolArgs
+  // must not also append its own defaults, or signtool.exe errors on the
+  // duplicate flags. The user-supplied value wins.
+  void it('does not duplicate /tr, /td, or /fd when supplied via signWithParams (issue #46)', () => {
+    const args = getSigntoolArgs({
+      ...baseOptions,
+      signWithParams:
+        '/tr http://my.timestamp.example /td sha384 /fd sha384 /csp "My Provider" /kc alias',
+    });
+
+    assert.strictEqual(args.filter((arg) => arg === '/tr').length, 1);
+    assert.strictEqual(args.filter((arg) => arg === '/td').length, 1);
+    assert.strictEqual(args.filter((arg) => arg === '/fd').length, 1);
+  });
+
   void it('passes the array form through verbatim without re-parsing or stripping quotes', () => {
     // An array entry that itself contains quotes/spaces must stay intact.
     const verbatim = ['/csp', 'DigiCert Signing Manager KSP', '/literal', '"keep these quotes"'];