Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Incorrectly parsed arguments from signWithParams #45

Open
ShGKme opened this issue Nov 19, 2024 · 1 comment
Open

Incorrectly parsed arguments from signWithParams #45

ShGKme opened this issue Nov 19, 2024 · 1 comment

Comments

@ShGKme
Copy link

ShGKme commented Nov 19, 2024

Problem

When additional arguments are passed via signWithParams, they are parsed into arguments using RegExp.

// Split up at spaces and doublequotes
extraArgs.push(...options.signWithParams.match(/(?:[^\s"]+|"[^"]*")+/g) as Array<string>);
}

For example, signWithParams: '/n "My Awesome Company"' is parsed into ['/n', '"My Awesome Company"']. Double quotes are kept in place as the arg value. Then it's passed into signtool via Node.js fork, it's passed as an argument.

As a result, signtool receives value with double quotes "My Awesome Company" instead of the actual value My Awesome Company.

Same problem with device tokens and /csp, /kc params.

Proposals

1. Don't parse signWithParams

Currently params from the result of parsing are never used individually.

extraArgs.push(options.signWithParams)

2. Parse params with values

extraArgs.push(...[...options.signWithParams.matchAll(/(?:([^\s"]+)|"([^"]*)")+/g)].map((matched) => matched[1] || matched[2]));

Then it results into ['/n', 'My Awesome Company'] instead of ['/n', '"My Awesome Company"']

3. use windowsVerbatimArguments: true and quote other params instead

Solve the problem the other way around — quote args provided from @electron/windows-sign.


I'd prefer option 2, as it allows fixing another issue.

I'm ready to prove a PR, if that's ok.

@nikwen
Copy link
Contributor

nikwen commented Nov 29, 2024

I saw this, too. I think a PR would make sense.

@erickzhao erickzhao added bug Something isn't working and removed bug Something isn't working labels Feb 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants