Fix invisible script execution on Windows #264

This commit addresses an issue in the privacy.sexy desktop application
where scripts executed as administrator on Windows were running in the
background. This was observed in environments like Windows Pro VMs on
Azure, where operations typically run with administrative privileges.

Previously, the application used the `"$path"` shell command to execute
scripts. This mechanism failed to activate the logic for requesting
admin privileges if the app itself was running as an administrator.
To resolve this, the script execution process has been modified to
explicitly ask for administrator privileges using the `VerbAs` method.
This ensures that the script always runs in a new `cmd.exe` window,
enhancing visibility and user interaction.

Other supporting changes:

- Rename the generated script file from `run-{timestamp}-{extension}` er
  to `{timestamp}-privacy-script-{extension}` for clearer identification
  and better file sorting.
- Refactor `ScriptFileCreator` to parameterize file extension and
  script name.
- Rename `OsTimestampedFilenameGenerator` to
  `TimestampedFilenameGenerator` to better reflect its new and more
  scoped functionality after refactoring mentioned abvoe.
- Remove `setAppName()` due to ineffective behavior in Windows.
- Update `SECURITY.md` to highlight that the app doesn't require admin
  rights for standard operations.
- Add `.editorconfig` settings for PowerShell scripts.
- Add a integration test for script execution logic. Improve environment
  detection for more reliable test execution.
- Disable application logging during unit/integration tests to keep test
  outputs clean and focused.
This commit is contained in:
undergroundwires
2024-01-09 20:44:06 +01:00
parent 728584240c
commit b404a91ada
32 changed files with 716 additions and 290 deletions

View File

@@ -1,3 +1,5 @@
import { ScriptFileNameParts } from '../ScriptFileCreator';
export interface FilenameGenerator {
generateFilename(): string;
generateFilename(scriptFileNameParts: ScriptFileNameParts): string;
}

View File

@@ -1,47 +0,0 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory';
import { FilenameGenerator } from './FilenameGenerator';
/**
* Generates a timestamped filename specific to the given operating system.
*
* The filename includes:
* - A timestamp for uniqueness and easier auditability.
* - File extension based on the operating system.
*/
export class OsTimestampedFilenameGenerator implements FilenameGenerator {
private readonly currentOs?: OperatingSystem;
constructor(
environment: RuntimeEnvironment = CurrentEnvironment,
) {
this.currentOs = environment.os;
}
public generateFilename(
date = new Date(),
): string {
const baseFileName = `run-${createTimeStampForFile(date)}`;
const extension = this.currentOs === undefined ? undefined : FileExtensions[this.currentOs];
return extension ? `${baseFileName}.${extension}` : baseFileName;
}
}
const FileExtensions: Partial<Record<OperatingSystem, string>> = {
// '.bat' for Windows batch files; required for executability.
[OperatingSystem.Windows]: 'bat',
// '.sh' for Unix-like systems; enhances recognition as a shell script
[OperatingSystem.macOS]: 'sh',
[OperatingSystem.Linux]: 'sh',
};
/** Generates a timestamp for the filename in 'YYYY-MM-DD_HH-MM-SS' format. */
function createTimeStampForFile(date: Date): string {
return date
.toISOString()
.replace(/T/, '_')
.replace(/:/g, '-')
.replace(/\..+/, '');
}

View File

@@ -0,0 +1,31 @@
import { ScriptFileNameParts } from '../ScriptFileCreator';
import { FilenameGenerator } from './FilenameGenerator';
export class TimestampedFilenameGenerator implements FilenameGenerator {
public generateFilename(
scriptFileNameParts: ScriptFileNameParts,
date = new Date(),
): string {
validateScriptFileNameParts(scriptFileNameParts);
const baseFileName = `${createTimeStampForFile(date)}-${scriptFileNameParts.scriptName}`;
return scriptFileNameParts.scriptFileExtension ? `${baseFileName}.${scriptFileNameParts.scriptFileExtension}` : baseFileName;
}
}
/** Generates a timestamp for the filename in 'YYYY-MM-DD_HH-MM-SS' format. */
function createTimeStampForFile(date: Date): string {
return date
.toISOString()
.replace(/T/, '_')
.replace(/:/g, '-')
.replace(/\..+/, '');
}
function validateScriptFileNameParts(scriptFileNameParts: ScriptFileNameParts) {
if (!scriptFileNameParts.scriptName) {
throw new Error('Script name is required but not provided.');
}
if (scriptFileNameParts.scriptFileExtension?.startsWith('.')) {
throw new Error('File extension should not start with a dot.');
}
}

View File

@@ -3,27 +3,30 @@ import { Logger } from '@/application/Common/Log/Logger';
import { SystemOperations } from '../System/SystemOperations';
import { NodeElectronSystemOperations } from '../System/NodeElectronSystemOperations';
import { FilenameGenerator } from './Filename/FilenameGenerator';
import { ScriptFileCreator } from './ScriptFileCreator';
import { OsTimestampedFilenameGenerator } from './Filename/OsTimestampedFilenameGenerator';
import { ScriptFileNameParts, ScriptFileCreator } from './ScriptFileCreator';
import { TimestampedFilenameGenerator } from './Filename/TimestampedFilenameGenerator';
import { ScriptDirectoryProvider } from './Directory/ScriptDirectoryProvider';
import { PersistentDirectoryProvider } from './Directory/PersistentDirectoryProvider';
export class ScriptFileCreationOrchestrator implements ScriptFileCreator {
constructor(
private readonly system: SystemOperations = new NodeElectronSystemOperations(),
private readonly filenameGenerator: FilenameGenerator = new OsTimestampedFilenameGenerator(),
private readonly filenameGenerator: FilenameGenerator = new TimestampedFilenameGenerator(),
private readonly directoryProvider: ScriptDirectoryProvider = new PersistentDirectoryProvider(),
private readonly logger: Logger = ElectronLogger,
) { }
public async createScriptFile(contents: string): Promise<string> {
const filePath = await this.provideFilePath();
public async createScriptFile(
contents: string,
scriptFileNameParts: ScriptFileNameParts,
): Promise<string> {
const filePath = await this.provideFilePath(scriptFileNameParts);
await this.createFile(filePath, contents);
return filePath;
}
private async provideFilePath(): Promise<string> {
const filename = this.filenameGenerator.generateFilename();
private async provideFilePath(scriptFileNameParts: ScriptFileNameParts): Promise<string> {
const filename = this.filenameGenerator.generateFilename(scriptFileNameParts);
const directoryPath = await this.directoryProvider.provideScriptDirectory();
const filePath = this.system.location.combinePaths(directoryPath, filename);
return filePath;

View File

@@ -1,3 +1,11 @@
export interface ScriptFileCreator {
createScriptFile(contents: string): Promise<string>;
createScriptFile(
contents: string,
scriptFileNameParts: ScriptFileNameParts,
): Promise<string>;
}
export interface ScriptFileNameParts {
readonly scriptName: string;
readonly scriptFileExtension: string | undefined;
}

View File

@@ -24,6 +24,10 @@ export class VisibleTerminalScriptExecutor implements ScriptFileExecutor {
}
private async setFileExecutablePermissions(filePath: string): Promise<void> {
/*
This is required on macOS and Linux otherwise the terminal emulators will refuse to
execute the script. It's not needed on Windows.
*/
this.logger.info(`Setting execution permissions for file at ${filePath}`);
await this.system.fileSystem.setFilePermissions(filePath, '755');
this.logger.info(`Execution permissions set successfully for ${filePath}`);
@@ -53,43 +57,93 @@ interface TerminalExecutionContext {
type TerminalRunner = (context: TerminalExecutionContext) => Promise<void>;
export const LinuxTerminalEmulator = 'x-terminal-emulator';
const TerminalRunners: Partial<Record<OperatingSystem, TerminalRunner>> = {
[OperatingSystem.Windows]: async (context) => {
const command = [
'PowerShell',
'Start-Process',
'-Verb RunAs', // Run as administrator with GUI sudo prompt
`-FilePath ${cmdShellPathArgumentEscape(context.scriptFilePath)}`,
].join(' ');
/*
Options:
"path":
✅ Launches the script within `cmd.exe`.
✅ Uses user-friendly GUI sudo prompt.
📝 Options:
`child_process.execFile()`
"path", `cmd.exe /c "path"`
❌ Script execution in the background without a visible terminal.
This occurs only when the user runs the application as administrator, as seen
in Windows Pro VMs on Azure.
`PowerShell Start -Verb RunAs "path"`
✅ Visible terminal window
✅ GUI sudo prompt (through `RunAs` option)
`PowerShell Start "path"`
`explorer.exe "path"`
`electron.shell.openPath`
`start cmd.exe /c "$path"`
✅ Visible terminal window
✅ GUI sudo prompt (through `RunAs` option)
👍 Among all options `start` command is the most explicit one, being the most resilient
against the potential changes in Windows or Electron framework (e.g. https://github.com/electron/electron/issues/36765).
`%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
*/
const command = cmdShellPathArgumentEscape(context.scriptFilePath);
await runCommand(command, context);
},
[OperatingSystem.Linux]: async (context) => {
const command = `x-terminal-emulator -e ${posixShellPathArgumentEscape(context.scriptFilePath)}`;
const command = `${LinuxTerminalEmulator} -e ${posixShellPathArgumentEscape(context.scriptFilePath)}`;
/*
Options:
`x-terminal-emulator -e`:
✅ Launches the script within the default terminal emulator.
❌ Requires terminal-based (not GUI) sudo prompt, which may not be very user friendly.
🤔 Potential improvements:
Use user-friendly GUI sudo prompt (not terminal-based).
If `pkexec` exists, we could do `x-terminal-emulator -e pkexec 'path'`, which always
prompts with user-friendly GUI sudo prompt.
📝 Options:
`x-terminal-emulator -e 'path'`:
✅ Visible terminal window
❌ Terminal-based (not GUI) sudo prompt.
`x-terminal-emulator -e pkexec 'path'
✅ Visible terminal window
✅ Always prompts with user-friendly GUI sudo prompt.
🤔 Not using `pkexec` as it is not in all Linux distributions. It should have smarter
logic to handle if it does not exist.
`electron.shell.openPath`:
❌ Opens the script in the default text editor, verified on
Debian/Ubuntu-based distributions.
`child_process.execFile()`:
❌ Script execution in the background without a visible terminal.
*/
await runCommand(command, context);
},
[OperatingSystem.macOS]: async (context) => {
const command = `open -a Terminal.app ${posixShellPathArgumentEscape(context.scriptFilePath)}`;
// -a Specifies the application to use for opening the file
/* eslint-disable vue/max-len */
/*
Options:
`open -a Terminal.app`:
✅ Launches the script within Terminal app, that exists natively in all modern macOS
versions.
❌ Requires terminal-based (not GUI) sudo prompt, which may not be very user friendly.
❌ Terminal app requires many privileges to execute the script, this would prompt user
to grant privileges to the Terminal app.
`osascript -e "do shell script \\"${scriptPath}\\" with administrator privileges"`:
Uses user-friendly GUI sudo prompt.
❌ Executes the script in the background, which does not provide the user with immediate
visual feedback or allow interaction with the script as it runs.
🤔 Potential improvements:
Use user-friendly GUI sudo prompt for running the script.
📝 Options:
`open -a Terminal.app 'path'`
✅ Visible terminal window
Terminal-based (not GUI) sudo prompt.
Terminal app requires many privileges to execute the script, this prompts user
to grant privileges to the Terminal app.
`osascript -e 'do shell script "'/tmp/test.sh'" with administrator privileges'`
Script as root
✅ GUI sudo prompt.
❌ Script execution in the background without a visible terminal.
`osascript -e 'do shell script "open -a 'Terminal.app' '/tmp/test.sh'" with administrator privileges'`
❌ Script as user, not root
✅ GUI sudo prompt.
✅ Visible terminal window
`osascript -e 'do shell script "/System/Applications/Utilities/Terminal.app/Contents/MacOS/Terminal '/tmp/test.sh'" with administrator privileges'`
✅ Script as root
✅ GUI sudo prompt.
✅ Visible terminal window
Useful resources about `do shell script .. with administrator privileges`:
- Change "osascript wants to make changes" prompt: https://web.archive.org/web/20240109191128/https://apple.stackexchange.com/questions/283353/how-to-rename-osascript-in-the-administrator-privileges-dialog
- More about `do shell script`: https://web.archive.org/web/20100906222226/http://developer.apple.com/mac/library/technotes/tn2002/tn2065.html
*/
/* eslint-enable vue/max-len */
await runCommand(command, context);
},
} as const;
@@ -101,11 +155,13 @@ async function runCommand(command: string, context: TerminalExecutionContext): P
}
function posixShellPathArgumentEscape(pathArgument: string): string {
// - Wraps the path in single quotes, which is a standard practice in POSIX shells
// (like bash and zsh) found on macOS/Linux to ensure that characters like spaces, '*', and
// '?' are treated as literals, not as special characters.
// - Escapes any single quotes within the path itself. This allows paths containing single
// quotes to be correctly interpreted in POSIX-compliant systems, such as Linux and macOS.
/*
- Wraps the path in single quotes, which is a standard practice in POSIX shells
(like bash and zsh) found on macOS/Linux to ensure that characters like spaces, '*', and
'?' are treated as literals, not as special characters.
- Escapes any single quotes within the path itself. This allows paths containing single
quotes to be correctly interpreted in POSIX-compliant systems, such as Linux and macOS.
*/
return `'${pathArgument.replaceAll('\'', '\'\\\'\'')}'`;
}

View File

@@ -1,24 +1,30 @@
import { CodeRunner } from '@/application/CodeRunner';
import { Logger } from '@/application/Common/Log/Logger';
import { ScriptFileName } from '@/application/CodeRunner/ScriptFileName';
import { CodeRunner } from '@/application/CodeRunner/CodeRunner';
import { ElectronLogger } from '../Log/ElectronLogger';
import { ScriptFileExecutor } from './Execution/ScriptFileExecutor';
import { VisibleTerminalScriptExecutor } from './Execution/VisibleTerminalScriptFileExecutor';
import { ScriptFileCreator } from './Creation/ScriptFileCreator';
import { ScriptFileCreationOrchestrator } from './Creation/ScriptFileCreationOrchestrator';
import { VisibleTerminalScriptExecutor } from './Execution/VisibleTerminalScriptFileExecutor';
export class ScriptFileCodeRunner implements CodeRunner {
constructor(
private readonly scriptFileExecutor: ScriptFileExecutor = new VisibleTerminalScriptExecutor(),
private readonly scriptFileExecutor
: ScriptFileExecutor = new VisibleTerminalScriptExecutor(),
private readonly scriptFileCreator: ScriptFileCreator = new ScriptFileCreationOrchestrator(),
private readonly logger: Logger = ElectronLogger,
) { }
public async runCode(
code: string,
fileExtension: string,
): Promise<void> {
this.logger.info('Initiating script running process.');
try {
const scriptFilePath = await this.scriptFileCreator.createScriptFile(code);
const scriptFilePath = await this.scriptFileCreator.createScriptFile(code, {
scriptName: ScriptFileName,
scriptFileExtension: fileExtension,
});
await this.scriptFileExecutor.executeScriptFile(scriptFilePath);
this.logger.info(`Successfully ran script at ${scriptFilePath}`);
} catch (error) {

View File

@@ -2,26 +2,33 @@ import { BrowserRuntimeEnvironment } from './Browser/BrowserRuntimeEnvironment';
import { NodeRuntimeEnvironment } from './Node/NodeRuntimeEnvironment';
import { RuntimeEnvironment } from './RuntimeEnvironment';
export const CurrentEnvironment = determineAndCreateRuntimeEnvironment(
() => globalThis.window,
);
export const CurrentEnvironment = determineAndCreateRuntimeEnvironment({
getGlobalWindow: () => globalThis.window,
getGlobalProcess: () => globalThis.process,
});
export function determineAndCreateRuntimeEnvironment(
windowAccessor: WindowAccessor,
globalAccessor: GlobalAccessor,
browserEnvironmentFactory: BrowserRuntimeEnvironmentFactory = (
window,
) => new BrowserRuntimeEnvironment(window),
nodeEnvironmentFactory: NodeRuntimeEnvironmentFactory = () => new NodeRuntimeEnvironment(),
): RuntimeEnvironment {
const window = windowAccessor();
if (globalAccessor.getGlobalProcess()) {
return nodeEnvironmentFactory();
}
const window = globalAccessor.getGlobalWindow();
if (window) {
return browserEnvironmentFactory(window);
}
return nodeEnvironmentFactory();
throw new Error('Unsupported runtime environment: The current context is neither a recognized browser nor a Node.js environment.');
}
export type BrowserRuntimeEnvironmentFactory = (window: Window) => RuntimeEnvironment;
export type NodeRuntimeEnvironmentFactory = () => NodeRuntimeEnvironment;
export type WindowAccessor = () => Window | undefined;
export interface GlobalAccessor {
getGlobalWindow(): Window | undefined;
getGlobalProcess(): NodeJS.Process | undefined;
}

View File

@@ -1,6 +1,6 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { Logger } from '@/application/Common/Log/Logger';
import { CodeRunner } from '@/application/CodeRunner';
import { CodeRunner } from '@/application/CodeRunner/CodeRunner';
/* Primary entry point for platform-specific injections */
export interface WindowVariables {