Improve security by isolating code execution more

This commit enhances application security against potential attacks by
isolating dependencies that access the host system (like file
operations) from the renderer process. It narrows the exposed
functionality to script execution only, adding an extra security layer.

The changes allow secure and scalable API exposure, preparing for future
functionalities such as desktop notifications for script errors (#264),
improved script execution handling (#296), and creating restore points
(#50) in a secure and repeatable way.

Changes include:

- Inject `CodeRunner` into Vue components via dependency injection.
- Move `CodeRunner` to the application layer as an abstraction for
  better domain-driven design alignment.
- Refactor `SystemOperations` and related interfaces, removing the `I`
  prefix.
- Update architecture documentation for clarity.
- Update return types in `NodeSystemOperations` to match the Node APIs.
- Improve `WindowVariablesProvider` integration tests for better error
  context.
- Centralize type checks with common functions like `isArray` and
  `isNumber`.
- Change `CodeRunner` to use `os` parameter, ensuring correct window
  variable injection.
- Streamline API exposure to the renderer process:
  - Automatically bind function contexts to prevent loss of original
    context.
  - Implement a way to create facades (wrapper/proxy objects) for
    increased security.
This commit is contained in:
undergroundwires
2023-12-18 17:30:56 +01:00
parent 940febc3e8
commit efa05f42bc
60 changed files with 939 additions and 423 deletions

View File

@@ -1,10 +1,10 @@
import { tmpdir } from 'os';
import { join } from 'path';
import { chmod, mkdir, writeFile } from 'fs/promises';
import { exec } from 'child_process';
import { ISystemOperations } from './ISystemOperations';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { chmod, mkdir, writeFile } from 'node:fs/promises';
import { exec } from 'node:child_process';
import { SystemOperations } from './SystemOperations';
export function createNodeSystemOperations(): ISystemOperations {
export function createNodeSystemOperations(): SystemOperations {
return {
operatingSystem: {
getTempDirectory: () => tmpdir(),
@@ -33,7 +33,14 @@ export function createNodeSystemOperations(): ISystemOperations {
) => writeFile(filePath, data),
},
command: {
execute: (command) => exec(command),
execute: (command) => new Promise((resolve, reject) => {
exec(command, (error) => {
if (error) {
reject(error);
}
resolve();
});
}),
},
};
}

View File

@@ -0,0 +1,24 @@
export interface SystemOperations {
readonly operatingSystem: OperatingSystemOps;
readonly location: LocationOps;
readonly fileSystem: FileSystemOps;
readonly command: CommandOps;
}
export interface OperatingSystemOps {
getTempDirectory(): string;
}
export interface LocationOps {
combinePaths(...pathSegments: string[]): string;
}
export interface CommandOps {
execute(command: string): Promise<void>;
}
export interface FileSystemOps {
setFilePermissions(filePath: string, mode: string | number): Promise<void>;
createDirectory(directoryPath: string, isRecursive?: boolean): Promise<void>;
writeToFile(filePath: string, data: string): Promise<void>;
}

View File

@@ -1,18 +1,19 @@
import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
import { OperatingSystem } from '@/domain/OperatingSystem';
import { getWindowInjectedSystemOperations } from './SystemOperations/WindowInjectedSystemOperations';
import { CodeRunner } from '@/application/CodeRunner';
import { SystemOperations } from './SystemOperations/SystemOperations';
import { createNodeSystemOperations } from './SystemOperations/NodeSystemOperations';
export class CodeRunner {
export class TemporaryFileCodeRunner implements CodeRunner {
constructor(
private readonly system = getWindowInjectedSystemOperations(),
private readonly environment = RuntimeEnvironment.CurrentEnvironment,
private readonly system: SystemOperations = createNodeSystemOperations(),
) { }
public async runCode(code: string, folderName: string, fileExtension: string): Promise<void> {
const { os } = this.environment;
if (os === undefined) {
throw new Error('Unidentified operating system');
}
public async runCode(
code: string,
folderName: string,
fileExtension: string,
os: OperatingSystem,
): Promise<void> {
const dir = this.system.location.combinePaths(
this.system.operatingSystem.getTempDirectory(),
folderName,
@@ -22,7 +23,7 @@ export class CodeRunner {
await this.system.fileSystem.writeToFile(filePath, code);
await this.system.fileSystem.setFilePermissions(filePath, '755');
const command = getExecuteCommand(filePath, os);
this.system.command.execute(command);
await this.system.command.execute(command);
}
}

View File

@@ -1,8 +1,9 @@
import { isNumber } from '@/TypeHelpers';
import { IEntity } from './IEntity';
export abstract class BaseEntity<TId> implements IEntity<TId> {
protected constructor(public id: TId) {
if (typeof id !== 'number' && !id) {
if (!isNumber(id) && !id) {
throw new Error('Id cannot be null or empty');
}
}

View File

@@ -1,3 +1,4 @@
import { isBoolean, isFunction } from '@/TypeHelpers';
import { IEnvironmentVariables } from './IEnvironmentVariables';
/* Validation is externalized to keep the environment objects simple */
@@ -15,7 +16,7 @@ export function validateEnvironmentVariables(environment: IEnvironmentVariables)
function getKeysMissingValues(keyValuePairs: Record<string, unknown>): string[] {
return Object.entries(keyValuePairs)
.reduce((acc, [key, value]) => {
if (!value && typeof value !== 'boolean') {
if (!value && !isBoolean(value)) {
acc.push(key);
}
return acc;
@@ -38,7 +39,7 @@ function capturePropertyValues(instance: object): Record<string, unknown> {
// Capture getter properties from the instance's prototype
for (const [key, descriptor] of Object.entries(descriptors)) {
if (typeof descriptor.get === 'function') {
if (isFunction(descriptor.get)) {
obj[key] = descriptor.get.call(instance);
}
}

View File

@@ -2,14 +2,8 @@ import log from 'electron-log/main';
import { Logger } from '@/application/Common/Log/Logger';
import type { LogFunctions } from 'electron-log';
// Using plain-function rather than class so it can be used in Electron's context-bridging.
export function createElectronLogger(logger: LogFunctions = log): Logger {
return {
info: (...params) => logger.info(...params),
debug: (...params) => logger.debug(...params),
warn: (...params) => logger.warn(...params),
error: (...params) => logger.error(...params),
};
return logger;
}
export const ElectronLogger = createElectronLogger();

View File

@@ -1,24 +0,0 @@
export interface ISystemOperations {
readonly operatingSystem: IOperatingSystemOps;
readonly location: ILocationOps;
readonly fileSystem: IFileSystemOps;
readonly command: ICommandOps;
}
export interface IOperatingSystemOps {
getTempDirectory(): string;
}
export interface ILocationOps {
combinePaths(...pathSegments: string[]): string;
}
export interface ICommandOps {
execute(command: string): void;
}
export interface IFileSystemOps {
setFilePermissions(filePath: string, mode: string | number): Promise<void>;
createDirectory(directoryPath: string, isRecursive?: boolean): Promise<void>;
writeToFile(filePath: string, data: string): Promise<void>;
}

View File

@@ -1,14 +0,0 @@
import { WindowVariables } from '../WindowVariables/WindowVariables';
import { ISystemOperations } from './ISystemOperations';
export function getWindowInjectedSystemOperations(
windowVariables: Partial<WindowVariables> = window,
): ISystemOperations {
if (!windowVariables) {
throw new Error('missing window');
}
if (!windowVariables.system) {
throw new Error('missing system');
}
return windowVariables.system;
}

View File

@@ -1,11 +1,11 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { ISystemOperations } from '@/infrastructure/SystemOperations/ISystemOperations';
import { Logger } from '@/application/Common/Log/Logger';
import { CodeRunner } from '@/application/CodeRunner';
/* Primary entry point for platform-specific injections */
export interface WindowVariables {
readonly isDesktop: boolean;
readonly system?: ISystemOperations;
readonly codeRunner?: CodeRunner;
readonly os?: OperatingSystem;
readonly log: Logger;
}

View File

@@ -1,12 +1,14 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { PropertyKeys } from '@/TypeHelpers';
import {
PropertyKeys, isBoolean, isFunction, isNumber, isPlainObject,
} from '@/TypeHelpers';
import { WindowVariables } from './WindowVariables';
/**
* Checks for consistency in runtime environment properties injected by Electron preloader.
*/
export function validateWindowVariables(variables: Partial<WindowVariables>) {
if (!isObject(variables)) {
if (!isPlainObject(variables)) {
throw new Error('window is not an object');
}
const errors = [...testEveryProperty(variables)];
@@ -21,7 +23,7 @@ function* testEveryProperty(variables: Partial<WindowVariables>): Iterable<strin
} = {
os: testOperatingSystem(variables.os),
isDesktop: testIsDesktop(variables.isDesktop),
system: testSystem(variables),
codeRunner: testCodeRunner(variables),
log: testLogger(variables),
};
@@ -49,14 +51,15 @@ function testLogger(variables: Partial<WindowVariables>): boolean {
if (!variables.isDesktop) {
return true;
}
return isObject(variables.log);
return isPlainObject(variables.log);
}
function testSystem(variables: Partial<WindowVariables>): boolean {
function testCodeRunner(variables: Partial<WindowVariables>): boolean {
if (!variables.isDesktop) {
return true;
}
return isObject(variables.system);
return isPlainObject(variables.codeRunner)
&& isFunction(variables.codeRunner.runCode);
}
function testIsDesktop(isDesktop: unknown): boolean {
@@ -65,17 +68,3 @@ function testIsDesktop(isDesktop: unknown): boolean {
}
return isBoolean(isDesktop);
}
function isNumber(variable: unknown): variable is number {
return typeof variable === 'number';
}
function isBoolean(variable: unknown): variable is boolean {
return typeof variable === 'boolean';
}
function isObject(variable: unknown): variable is object {
return Boolean(variable) // the data type of null is an object
&& typeof variable === 'object'
&& !Array.isArray(variable);
}