Fix handling special chars in script paths
This commit improves the handling of paths with spaces or special characters during script execution in the desktop application. Key improvements: - Paths are now quoted for macOS/Linux, addressing issues with whitespace or single quotes. - Windows paths are enclosed in double quotes to handle special characters. Other supporting changes: - Add more documentation for terminal execution commands. - Refactor terminal script file execution into a dedicated file for improved separation of concerns. - Refactor naming of `RuntimeEnvironment` to align with naming conventions (no interface with I prefix) and for clarity. - Refactor `TemporaryFileCodeRunner` to simplify it by removing the `os` parameter and handling OS-specific logic within the filename generator instead. - Refactor `fileName` to `filename` for consistency.
This commit is contained in:
@@ -0,0 +1,3 @@
|
||||
export interface ScriptFileExecutor {
|
||||
executeScriptFile(filePath: string): Promise<void>;
|
||||
}
|
||||
@@ -0,0 +1,117 @@
|
||||
import { OperatingSystem } from '@/domain/OperatingSystem';
|
||||
import { CommandOps, SystemOperations } from '@/infrastructure/CodeRunner/SystemOperations/SystemOperations';
|
||||
import { Logger } from '@/application/Common/Log/Logger';
|
||||
import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger';
|
||||
import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
|
||||
import { HostRuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/HostRuntimeEnvironment';
|
||||
import { createNodeSystemOperations } from '@/infrastructure/CodeRunner/SystemOperations/NodeSystemOperations';
|
||||
import { ScriptFileExecutor } from './ScriptFileExecutor';
|
||||
|
||||
export class VisibleTerminalScriptExecutor implements ScriptFileExecutor {
|
||||
constructor(
|
||||
private readonly system: SystemOperations = createNodeSystemOperations(),
|
||||
private readonly logger: Logger = ElectronLogger,
|
||||
private readonly environment: RuntimeEnvironment = HostRuntimeEnvironment.CurrentEnvironment,
|
||||
) { }
|
||||
|
||||
public async executeScriptFile(filePath: string): Promise<void> {
|
||||
const { os } = this.environment;
|
||||
if (os === undefined) {
|
||||
throw new Error('Unknown operating system');
|
||||
}
|
||||
await this.setFileExecutablePermissions(filePath);
|
||||
await this.runFileWithRunner(filePath, os);
|
||||
}
|
||||
|
||||
private async setFileExecutablePermissions(filePath: string): Promise<void> {
|
||||
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}`);
|
||||
}
|
||||
|
||||
private async runFileWithRunner(filePath: string, os: OperatingSystem): Promise<void> {
|
||||
this.logger.info(`Executing script file: ${filePath} on ${OperatingSystem[os]}.`);
|
||||
const runner = TerminalRunners[os];
|
||||
if (!runner) {
|
||||
throw new Error(`Unsupported operating system: ${OperatingSystem[os]}`);
|
||||
}
|
||||
const context: TerminalExecutionContext = {
|
||||
scriptFilePath: filePath,
|
||||
commandOps: this.system.command,
|
||||
logger: this.logger,
|
||||
};
|
||||
await runner(context);
|
||||
this.logger.info('Command script file successfully.');
|
||||
}
|
||||
}
|
||||
|
||||
interface TerminalExecutionContext {
|
||||
readonly scriptFilePath: string;
|
||||
readonly commandOps: CommandOps;
|
||||
readonly logger: Logger;
|
||||
}
|
||||
|
||||
type TerminalRunner = (context: TerminalExecutionContext) => Promise<void>;
|
||||
|
||||
const TerminalRunners: Partial<Record<OperatingSystem, TerminalRunner>> = {
|
||||
[OperatingSystem.Windows]: async (context) => {
|
||||
/*
|
||||
Options:
|
||||
"path":
|
||||
✅ Launches the script within `cmd.exe`.
|
||||
✅ Uses user-friendly GUI sudo prompt.
|
||||
*/
|
||||
const command = cmdShellPathArgumentEscape(context.scriptFilePath);
|
||||
await runCommand(command, context);
|
||||
},
|
||||
[OperatingSystem.Linux]: async (context) => {
|
||||
const command = `x-terminal-emulator -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.
|
||||
*/
|
||||
await runCommand(command, context);
|
||||
},
|
||||
[OperatingSystem.macOS]: async (context) => {
|
||||
const command = `open -a Terminal.app ${posixShellPathArgumentEscape(context.scriptFilePath)}`;
|
||||
/*
|
||||
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.
|
||||
*/
|
||||
await runCommand(command, context);
|
||||
},
|
||||
} as const;
|
||||
|
||||
async function runCommand(command: string, context: TerminalExecutionContext): Promise<void> {
|
||||
context.logger.info(`Executing command:\n${command}`);
|
||||
await context.commandOps.exec(command);
|
||||
context.logger.info('Executed command successfully.');
|
||||
}
|
||||
|
||||
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.
|
||||
return `'${pathArgument.replaceAll('\'', '\'\\\'\'')}'`;
|
||||
}
|
||||
|
||||
function cmdShellPathArgumentEscape(pathArgument: string): string {
|
||||
// - Encloses the path in double quotes, which is necessary for Windows command line (cmd.exe)
|
||||
// to correctly handle paths containing spaces.
|
||||
// - Paths in Windows cannot include double quotes `"` themselves, so these are not escaped.
|
||||
return `"${pathArgument}"`;
|
||||
}
|
||||
@@ -0,0 +1,3 @@
|
||||
export interface FilenameGenerator {
|
||||
generateFilename(): string;
|
||||
}
|
||||
@@ -1,4 +1,7 @@
|
||||
import { OperatingSystem } from '@/domain/OperatingSystem';
|
||||
import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
|
||||
import { HostRuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/HostRuntimeEnvironment';
|
||||
import { FilenameGenerator } from './FilenameGenerator';
|
||||
|
||||
/**
|
||||
* Generates a timestamped filename specific to the given operating system.
|
||||
@@ -7,13 +10,22 @@ import { OperatingSystem } from '@/domain/OperatingSystem';
|
||||
* - A timestamp for uniqueness and easier auditability.
|
||||
* - File extension based on the operating system.
|
||||
*/
|
||||
export function generateOsTimestampedFileName(
|
||||
currentOs: OperatingSystem,
|
||||
date = new Date(),
|
||||
): string {
|
||||
const baseFileName = `run-${createTimeStampForFile(date)}`;
|
||||
const extension = FileExtensions[currentOs];
|
||||
return extension ? `${baseFileName}.${extension}` : baseFileName;
|
||||
export class OsTimestampedFilenameGenerator implements FilenameGenerator {
|
||||
private readonly currentOs?: OperatingSystem;
|
||||
|
||||
constructor(
|
||||
environment: RuntimeEnvironment = HostRuntimeEnvironment.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>> = {
|
||||
@@ -33,7 +33,7 @@ export function createNodeSystemOperations(): SystemOperations {
|
||||
) => writeFile(filePath, data),
|
||||
},
|
||||
command: {
|
||||
execute: (command) => new Promise((resolve, reject) => {
|
||||
exec: (command) => new Promise((resolve, reject) => {
|
||||
exec(command, (error) => {
|
||||
if (error) {
|
||||
reject(error);
|
||||
|
||||
@@ -14,7 +14,7 @@ export interface LocationOps {
|
||||
}
|
||||
|
||||
export interface CommandOps {
|
||||
execute(command: string): Promise<void>;
|
||||
exec(command: string): Promise<void>;
|
||||
}
|
||||
|
||||
export interface FileSystemOps {
|
||||
|
||||
@@ -1,30 +1,30 @@
|
||||
import { OperatingSystem } from '@/domain/OperatingSystem';
|
||||
import { CodeRunner } from '@/application/CodeRunner';
|
||||
import { Logger } from '@/application/Common/Log/Logger';
|
||||
import { ElectronLogger } from '../Log/ElectronLogger';
|
||||
import { SystemOperations } from './SystemOperations/SystemOperations';
|
||||
import { createNodeSystemOperations } from './SystemOperations/NodeSystemOperations';
|
||||
import { generateOsTimestampedFileName } from './FileNameGenerator';
|
||||
|
||||
export type FileNameGenerator = (os: OperatingSystem) => string;
|
||||
import { FilenameGenerator } from './Filename/FilenameGenerator';
|
||||
import { ScriptFileExecutor } from './Execution/ScriptFileExecutor';
|
||||
import { VisibleTerminalScriptExecutor } from './Execution/VisibleTerminalScriptFileExecutor';
|
||||
import { OsTimestampedFilenameGenerator } from './Filename/OsTimestampedFilenameGenerator';
|
||||
|
||||
export class TemporaryFileCodeRunner implements CodeRunner {
|
||||
constructor(
|
||||
private readonly system: SystemOperations = createNodeSystemOperations(),
|
||||
private readonly fileNameGenerator: FileNameGenerator = generateOsTimestampedFileName,
|
||||
private readonly filenameGenerator: FilenameGenerator = new OsTimestampedFilenameGenerator(),
|
||||
private readonly logger: Logger = ElectronLogger,
|
||||
private readonly scriptFileExecutor: ScriptFileExecutor = new VisibleTerminalScriptExecutor(),
|
||||
) { }
|
||||
|
||||
public async runCode(
|
||||
code: string,
|
||||
tempScriptFolderName: string,
|
||||
os: OperatingSystem,
|
||||
): Promise<void> {
|
||||
this.logger.info(`Starting running code for OS: ${OperatingSystem[os]}`);
|
||||
this.logger.info('Starting running code.');
|
||||
try {
|
||||
const fileName = this.fileNameGenerator(os);
|
||||
const filePath = await this.createTemporaryFile(fileName, tempScriptFolderName, code);
|
||||
await this.executeFile(filePath, os);
|
||||
const filename = this.filenameGenerator.generateFilename();
|
||||
const filePath = await this.createTemporaryFile(filename, tempScriptFolderName, code);
|
||||
await this.scriptFileExecutor.executeScriptFile(filePath);
|
||||
this.logger.info(`Successfully executed script at ${filePath}`);
|
||||
} catch (error) {
|
||||
this.logger.error(`Error executing script: ${error.message}`, error);
|
||||
@@ -33,7 +33,7 @@ export class TemporaryFileCodeRunner implements CodeRunner {
|
||||
}
|
||||
|
||||
private async createTemporaryFile(
|
||||
fileName: string,
|
||||
filename: string,
|
||||
tempScriptFolderName: string,
|
||||
contents: string,
|
||||
): Promise<string> {
|
||||
@@ -42,7 +42,7 @@ export class TemporaryFileCodeRunner implements CodeRunner {
|
||||
tempScriptFolderName,
|
||||
);
|
||||
await this.createDirectoryIfNotExists(directoryPath);
|
||||
const filePath = this.system.location.combinePaths(directoryPath, fileName);
|
||||
const filePath = this.system.location.combinePaths(directoryPath, filename);
|
||||
await this.createFile(filePath, contents);
|
||||
return filePath;
|
||||
}
|
||||
@@ -58,41 +58,4 @@ export class TemporaryFileCodeRunner implements CodeRunner {
|
||||
await this.system.fileSystem.createDirectory(directoryPath, true);
|
||||
this.logger.info(`Directory confirmed at: ${directoryPath}`);
|
||||
}
|
||||
|
||||
private async executeFile(filePath: string, os: OperatingSystem): Promise<void> {
|
||||
await this.setFileExecutablePermissions(filePath);
|
||||
const command = getExecuteCommand(filePath, os);
|
||||
await this.executeCommand(command);
|
||||
}
|
||||
|
||||
private async setFileExecutablePermissions(filePath: string): Promise<void> {
|
||||
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}`);
|
||||
}
|
||||
|
||||
private async executeCommand(command: string): Promise<void> {
|
||||
this.logger.info(`Executing command: ${command}`);
|
||||
await this.system.command.execute(command);
|
||||
this.logger.info('Command executed successfully.');
|
||||
}
|
||||
}
|
||||
|
||||
function getExecuteCommand(
|
||||
scriptPath: string,
|
||||
os: OperatingSystem,
|
||||
): string {
|
||||
switch (os) {
|
||||
case OperatingSystem.Linux:
|
||||
return `x-terminal-emulator -e '${scriptPath}'`;
|
||||
case OperatingSystem.macOS:
|
||||
return `open -a Terminal.app ${scriptPath}`;
|
||||
// Another option with graphical sudo would be
|
||||
// `osascript -e "do shell script \\"${scriptPath}\\" with administrator privileges"`
|
||||
// However it runs in background
|
||||
case OperatingSystem.Windows:
|
||||
return scriptPath;
|
||||
default:
|
||||
throw Error(`unsupported os: ${OperatingSystem[os]}`);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user