Skip to content

Commit

Permalink
Fix win execution with whitespace in username #351
Browse files Browse the repository at this point in the history
This commit addresses the issue where scripts fail to execute on Windows
environments with usernames containing spaces. The problem stemmed from
PowerShell and cmd shell's handling of spaces in quoted arguments.

The solution involves encoding PowerShell commands before execution,
which mitigates the quoting issues previously causing script failures.
This approach is now integrated into the execution flow, ensuring that
commands are correctly handled irrespective of user names or other
variables that may include spaces.

Changes:

- Implement encoding for PowerShell commands to handle spaces in usernames
  and other similar scenarios.
- Update script documentation URLs to reflect changes in directory
  structure.

Fixes #351
  • Loading branch information
undergroundwires committed May 7, 2024
1 parent 1d7cafc commit a334320
Show file tree
Hide file tree
Showing 13 changed files with 241 additions and 50 deletions.
16 changes: 8 additions & 8 deletions src/application/collections/linux.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ actions:
> - Logs are valuable for diagnosing issues and understanding past actions [1].
> - Script files can help review changes made to the system and aid in reverting those changes if needed.
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
children:
-
Expand All @@ -202,7 +202,7 @@ actions:
> - This action is irreversible. Deleted script files cannot be retrieved.
> - These files might be necessary for troubleshooting if you experience issues after using privacy.sexy scripts.
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
call:
function: ClearDirectoryContents
Expand All @@ -223,7 +223,7 @@ actions:
> - Removing logs will prevent you from reviewing the application's activities, which could be helpful in diagnosing issues.
> - Logs can contain valuable information for technical support should you need assistance.
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
call:
function: ClearDirectoryContents
Expand Down Expand Up @@ -2761,7 +2761,7 @@ actions:
docs: |-
Firefox provides an option for Enhanced Tracking Protection [1], which blocks trackers that
gather information about your browsing behavior without disrupting site functionality [1].
This feature also includes protections against harmful scripts such as malware that drains
This feature also includes protections against harmful scripts such as malware that drain
your battery [1].
This script enables the `privacy.resistFingerprinting` preference,
Expand Down Expand Up @@ -2791,7 +2791,7 @@ actions:
This script enables the `privacy.resistFingerprinting` preference, activating
anti-fingerprinting [1][2].
As an experimental feature, it might cause some website breakage [2], such as impacting web
As an experimental feature, it might cause some website breakages [2], such as impacting web
speech functionality [3] and favicons [4].
[1]: https://web.archive.org/web/20221025201025/https://support.mozilla.org/en-US/kb/firefox-protection-against-fingerprinting "Firefox's protection against fingerprinting | Firefox Help | support.mozilla.org"
Expand Down Expand Up @@ -2876,7 +2876,7 @@ actions:
It's configured to be enabled in nightly, aurora, beta, or default (developer) builds.
In release builds, however, it's set to false [1]. This setting is hard-coded into the C++
code to prevent easy disabling [2]. Developers have been approached about this issue but
code to prevent easy disabling [2]. Developers have been approached about this issue, but
have rejected proposals to unlock it [3].
Mozilla's plan is to deprecate this setting eventually, followed by removal [1].
Expand Down Expand Up @@ -3012,7 +3012,7 @@ actions:
recommend: standard
docs: |-
This script sets `toolkit.telemetry.server` to be empty.
This preference defines the server to which Telemetry pings are sent [1].
This preference defines the server to which telemetry pings are sent [1].
[1]: https://web.archive.org/web/20221015102124/https://firefox-source-docs.mozilla.org/toolkit/components/telemetry/internals/preferences.html "Preferences and Defines — Firefox Source Docs documentation | firefox-source-docs.mozilla.org"
call:
Expand Down Expand Up @@ -3133,7 +3133,7 @@ actions:
name: Disable Firefox Pioneer study monitoring
recommend: standard
docs: |-
This script configures `toolkit.telemetry.pioneer-new-studies-available` to be disabled to opt out.
This script configures `toolkit.telemetry.pioneer-new-studies-available` to be disabled to opt out
Firefox Pioneer program.
This setting disables availability check for Firefox Pioneer studies [1].
Expand Down
6 changes: 3 additions & 3 deletions src/application/collections/macos.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -300,7 +300,7 @@ actions:
> - Logs are valuable for diagnosing issues and understanding past actions [1].
> - Script files can help review changes made to the system and aid in reverting those changes if needed.
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
children:
-
Expand All @@ -318,7 +318,7 @@ actions:
> - This action is irreversible. Deleted script files cannot be retrieved.
> - These files might be necessary for troubleshooting if you experience issues after using privacy.sexy scripts.
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
call:
function: ClearDirectoryContents
Expand All @@ -339,7 +339,7 @@ actions:
> - Removing logs will prevent you from reviewing the application's activities, which could be helpful in diagnosing issues.
> - Logs can contain valuable information for technical support should you need assistance.
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
call:
function: ClearDirectoryContents
Expand Down
6 changes: 3 additions & 3 deletions src/application/collections/windows.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ actions:
> - Logs are valuable for diagnosing issues and understanding past actions [1].
> - Script files can help review changes made to the system and aid in reverting those changes if needed.

[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
children:
-
Expand All @@ -230,7 +230,7 @@ actions:
> - This action is irreversible. Deleted script files cannot be retrieved.
> - These files might be necessary for troubleshooting if you experience issues after using privacy.sexy scripts.

[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
call:
function: ClearDirectoryContents
Expand All @@ -251,7 +251,7 @@ actions:
> - Removing logs will prevent you from reviewing the application's activities, which could be helpful in diagnosing issues.
> - Logs can contain valuable information for technical support should you need assistance.

[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[1]: https://github.com/undergroundwires/privacy.sexy/blob/master/docs/desktop/desktop-vs-web-features.md "Desktop vs. Web Features | privacy.sexy | github.com"
[2]: https://github.com/undergroundwires/privacy.sexy/blob/master/SECURITY.md "SECURITY.md | privacy.sexy | github.com"
call:
function: ClearDirectoryContents
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
import type { PowerShellInvokeShellCommandCreator } from './PowerShellInvokeShellCommandCreator';

/**
Encoding PowerShell commands resolve issues with quote handling.
There are known problems with PowerShell's handling of double quotes in command line arguments:
- Quote stripping in PowerShell command line arguments: https://web.archive.org/web/20240507102706/https://stackoverflow.com/questions/6714165/powershell-stripping-double-quotes-from-command-line-arguments
- privacy.sexy double quotes issue when calling PowerShell from command line: https://web.archive.org/web/20240507102841/https://github.com/undergroundwires/privacy.sexy/issues/351
- Challenges with single quotes in PowerShell command line: https://web.archive.org/web/20240507102047/https://stackoverflow.com/questions/20958388/command-line-escaping-single-quote-for-powershell
Using the `EncodedCommand` parameter is recommended by Microsoft for handling
complex quoting scenarios. This approach helps avoid issues by encoding the entire
command as a Base64 string:
- Microsoft's documentation on using the `EncodedCommand` parameter: https://web.archive.org/web/20240507102733/https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_powershell_exe?view=powershell-5.1#-encodedcommand-base64encodedcommand
*/
export class EncodedPowerShellInvokeCmdCommandCreator
implements PowerShellInvokeShellCommandCreator {
public createCommandToInvokePowerShell(powerShellScript: string): string {
return generateEncodedPowershellCommand(powerShellScript);
}
}

function generateEncodedPowershellCommand(powerShellScript: string): string {
const encodedCommand = encodeForPowershellExecution(powerShellScript);
return `PowerShell -EncodedCommand ${encodedCommand}`;
}

function encodeForPowershellExecution(script: string): string {
// The string must be formatted using UTF-16LE character encoding, see: https://web.archive.org/web/20240507102733/https://learn.microsoft.com/en-us/powershell/module/microsoft.powershell.core/about/about_powershell_exe?view=powershell-5.1#-encodedcommand-base64encodedcommand
const buffer = Buffer.from(script, 'utf16le');
return buffer.toString('base64');
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
export interface PowerShellInvokeShellCommandCreator {
createCommandToInvokePowerShell(powerShellCommand: string): string;
}

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
import type { ShellArgumentEscaper } from './ShellArgumentEscaper';

export class PowerShellArgumentEscaper implements ShellArgumentEscaper {
public escapePathArgument(pathArgument: string): string {
return powerShellPathArgumentEscape(pathArgument);
}
}

function powerShellPathArgumentEscape(pathArgument: string): string {
// - Encloses the path in single quotes to handle spaces and most special characters.
// - Single quotes are used in PowerShell to ensure the string is treated as a literal string.
// - Paths in Windows can include single quotes ('), so any internal single quotes are escaped
// using double quotes.
return `'${pathArgument.replace(/'/g, "''")}'`;
}
Original file line number Diff line number Diff line change
@@ -1,22 +1,29 @@
import { CmdShellArgumentEscaper } from './ShellArgument/CmdShellArgumentEscaper';
import type { Logger } from '@/application/Common/Log/Logger';
import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger';
import { PowerShellArgumentEscaper } from './ShellArgument/PowerShellArgumentEscaper';
import { EncodedPowerShellInvokeCmdCommandCreator } from './PowerShellInvoke/EncodedPowerShellInvokeCmdCommandCreator';
import type { ShellArgumentEscaper } from './ShellArgument/ShellArgumentEscaper';
import type { CommandDefinition } from '../CommandDefinition';
import type { PowerShellInvokeShellCommandCreator } from './PowerShellInvoke/PowerShellInvokeShellCommandCreator';

export class WindowsVisibleTerminalCommand implements CommandDefinition {
constructor(
private readonly escaper: ShellArgumentEscaper = new CmdShellArgumentEscaper(),
private readonly escaper: ShellArgumentEscaper = new PowerShellArgumentEscaper(),
private readonly powershellCommandCreator: PowerShellInvokeShellCommandCreator
= new EncodedPowerShellInvokeCmdCommandCreator(),
private readonly logger: Logger = ElectronLogger,
) { }

public buildShellCommand(filePath: string): string {
const command = [
'PowerShell',
const powershellCommand = [
'Start-Process',
'-Verb RunAs', // Run as administrator with GUI sudo prompt
`-FilePath ${this.escaper.escapePathArgument(filePath)}`,
].join(' ');
return command;
/*
📝 Options:
Running PowerShell command is preferred due to its flexibility and the way it provides
GUI sudo prompt through `RunAs` argument.
Other options considered:
`child_process.execFile()`
"path", `cmd.exe /c "path"`
❌ Script execution in the background without a visible terminal.
Expand All @@ -36,6 +43,8 @@ export class WindowsVisibleTerminalCommand implements CommandDefinition {
`%COMSPEC%` environment variable should be checked before defaulting to `cmd.exe.
Related docs: https://web.archive.org/web/20240106002357/https://nodejs.org/api/child_process.html#spawning-bat-and-cmd-files-on-windows
*/
this.logger.info(`Building command for PowerShell execution:\n\tCommand: ${powershellCommand}`);
return this.powershellCommandCreator.createCommandToInvokePowerShell(powershellCommand);
}

public isExecutionTerminatedExternally(): boolean {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { describe, it, expect } from 'vitest';
import { EncodedPowerShellInvokeCmdCommandCreator } from '@/infrastructure/CodeRunner/Execution/CommandDefinition/Commands/PowerShellInvoke/EncodedPowerShellInvokeCmdCommandCreator';

describe('EncodedPowerShellInvokeCmdCommandCreator', () => {
describe('createCommandToInvokePowerShell', () => {
it('starts with PowerShell base command', () => {
// arrange
const sut = new EncodedPowerShellInvokeCmdCommandCreator();
// act
const command = sut.createCommandToInvokePowerShell('non-important-command');
// assert
expect(command.startsWith('PowerShell ')).to.equal(true);
});
it('includes encoded command as parameter', () => {
// arrange
const expectedParameterName = '-EncodedCommand';
const sut = new EncodedPowerShellInvokeCmdCommandCreator();
// act
const command = sut.createCommandToInvokePowerShell('non-important-command');
// assert
const args = parsePowerShellArgs(command);
const parameterNames = [...args.keys()];
expect(parameterNames).to.include(expectedParameterName);
});
it('correctly encode the command as utf16le base64', () => {
// arrange
const givenCode = 'Write-Output "Today is $(Get-Date -Format \'dddd, MMMM dd\')."';
const expectedEncodedCommand = 'VwByAGkAdABlAC0ATwB1AHQAcAB1AHQAIAAiAFQAbwBkAGEAeQAgAGkAcwAgACQAKABHAGUAdAAtAEQAYQB0AGUAIAAtAEYAbwByAG0AYQB0ACAAJwBkAGQAZABkACwAIABNAE0ATQBNACAAZABkACcAKQAuACIA';
const sut = new EncodedPowerShellInvokeCmdCommandCreator();
// act
const command = sut.createCommandToInvokePowerShell(givenCode);
// assert
const args = parsePowerShellArgs(command);
const actualEncodedCommand = args.get('-EncodedCommand');
expect(actualEncodedCommand).to.equal(expectedEncodedCommand);
});
});
});

function parsePowerShellArgs(command: string): Map<string, string | undefined> {
const argsMap = new Map<string, string | undefined>();
const argRegex = /(-\w+)(\s+([^ ]+))?/g;
let match = argRegex.exec(command);
while (match !== null) {
const arg = match[1];
const value = match[3];
argsMap.set(arg, value);
match = argRegex.exec(command);
}
return argsMap;
}

This file was deleted.

Loading

0 comments on commit a334320

Please sign in to comment.