Skip to content

Commit

Permalink
Improve desktop security by isolating Electron
Browse files Browse the repository at this point in the history
Enable `contextIsolation` in Electron to securely expose a limited set
of Node.js APIs to the renderer process. It:

1. Isolates renderer and main process contexts. It ensures that the
   powerful main process functions aren't directly accessible from
   renderer process(es), adding a security boundary.
2. Mitigates remote exploitation risks. By isolating contexts, potential
   malicious code injections in the renderer can't directly reach and
   compromise the main process.
3. Reduces attack surface.
4. Protect against prototype pollution: It prevents tampering of
   JavaScript object prototypes in one context from affecting another
   context, improving app reliability and security.

Supporting changes include:

- Extract environment and system operations classes to the infrastructure
  layer. This removes node dependencies from core domain and application
  code.
- Introduce `ISystemOperations` to encapsulate OS interactions. Use it
  from `CodeRunner` to isolate node API usage.
- Add a preloader script to inject validated environment variables into
  renderer context. This keeps Electron integration details
  encapsulated.
- Add new sanity check to fail fast on issues with preloader injected
  variables.
- Improve test coverage of runtime sanity checks and environment
  components. Move validation logic into separate classes for Single
  Responsibility.
- Improve absent value test case generation.
  • Loading branch information
undergroundwires committed Aug 25, 2023
1 parent 62f8bfa commit e9e0001
Show file tree
Hide file tree
Showing 83 changed files with 1,843 additions and 766 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/checks.desktop-runtime-errors.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ jobs:
-
name: Test
shell: bash
run: node scripts/check-desktop-runtime-errors --screenshot
run: node ./scripts/check-desktop-runtime-errors --screenshot
-
name: Upload screenshot
if: always() # Run even if previous step fails
Expand Down
20 changes: 16 additions & 4 deletions docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,23 @@ Application is

Application uses highly decoupled models & services in different DDD layers:

- presentation layer (see [presentation.md](./presentation.md)),
- application layer (see [application.md](./application.md)),
- and domain layer.
**Application layer** (see [application.md](./application.md)):

Application layer depends on and consumes domain layer. [Presentation layer](./presentation.md) consumes and depends on application layer along with domain layer. Application and presentation layers can communicate through domain model.
- Coordinates application activities and consumes the domain layer.

**Presentation layer** (see [presentation.md](./presentation.md)):

- Handles UI/UX, consumes both the application and domain layers.
- May communicate directly with the infrastructure layer for technical needs, but avoids domain logic.

**Domain layer**:

- Serves as the system's core and central truth.
- Facilitates communication between the application and presentation layers through the domain model.

**Infrastructure layer**:

- Manages technical implementations without dependencies on other layers or domain knowledge.

![DDD + vue.js](./../img/architecture/app-ddd.png)

Expand Down
1 change: 0 additions & 1 deletion electron.vite.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export default defineConfig({
input: {
index: WEB_INDEX_HTML_PATH,
},
external: ['os', 'child_process', 'fs', 'path'],
},
},
},
Expand Down
4 changes: 4 additions & 0 deletions src/TypeHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,3 +9,7 @@ export type PropertyKeys<T> = {

export type ConstructorArguments<T> =
T extends new (...args: infer U) => unknown ? U : never;

export type FunctionKeys<T> = {
[K in keyof T]: T[K] extends (...args: unknown[]) => unknown ? K : never;
}[keyof T];
4 changes: 2 additions & 2 deletions src/application/Context/ApplicationContextFactory.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
import { IApplicationContext } from '@/application/Context/IApplicationContext';
import { OperatingSystem } from '@/domain/OperatingSystem';
import { IApplication } from '@/domain/IApplication';
import { Environment } from '../Environment/Environment';
import { IEnvironment } from '../Environment/IEnvironment';
import { Environment } from '@/infrastructure/Environment/Environment';
import { IEnvironment } from '@/infrastructure/Environment/IEnvironment';
import { IApplicationFactory } from '../IApplicationFactory';
import { ApplicationFactory } from '../ApplicationFactory';
import { ApplicationContext } from './ApplicationContext';
Expand Down
89 changes: 0 additions & 89 deletions src/application/Environment/Environment.ts

This file was deleted.

6 changes: 0 additions & 6 deletions src/application/Environment/IEnvironment.ts

This file was deleted.

2 changes: 1 addition & 1 deletion src/application/Parser/ApplicationParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { parseCategoryCollection } from './CategoryCollectionParser';
export function parseApplication(
categoryParser = parseCategoryCollection,
informationParser = parseProjectInformation,
metadata: IAppMetadata = AppMetadataFactory.Current,
metadata: IAppMetadata = AppMetadataFactory.Current.instance,
collectionsData = PreParsedCollections,
): IApplication {
validateCollectionsData(collectionsData);
Expand Down
2 changes: 1 addition & 1 deletion src/application/Parser/ProjectInformationParser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { ConstructorArguments } from '@/TypeHelpers';

export function
parseProjectInformation(
metadata: IAppMetadata = AppMetadataFactory.Current,
metadata: IAppMetadata = AppMetadataFactory.Current.instance,
createProjectInformation: ProjectInformationFactory = (
...args
) => new ProjectInformation(...args),
Expand Down
66 changes: 14 additions & 52 deletions src/infrastructure/CodeRunner.ts
Original file line number Diff line number Diff line change
@@ -1,25 +1,27 @@
import os from 'os';
import path from 'path';
import fs from 'fs';
import child_process from 'child_process';
import { Environment } from '@/application/Environment/Environment';
import { Environment } from '@/infrastructure/Environment/Environment';
import { OperatingSystem } from '@/domain/OperatingSystem';

export class CodeRunner {
constructor(
private readonly node = getNodeJs(),
private readonly environment = Environment.CurrentEnvironment,
) {
if (!environment.system) {
throw new Error('missing system operations');
}
}

public async runCode(code: string, folderName: string, fileExtension: string): Promise<void> {
const dir = this.node.path.join(this.node.os.tmpdir(), folderName);
await this.node.fs.promises.mkdir(dir, { recursive: true });
const filePath = this.node.path.join(dir, `run.${fileExtension}`);
await this.node.fs.promises.writeFile(filePath, code);
await this.node.fs.promises.chmod(filePath, '755');
const { system } = this.environment;
const dir = system.location.combinePaths(
system.operatingSystem.getTempDirectory(),
folderName,
);
await system.fileSystem.createDirectory(dir, true);
const filePath = system.location.combinePaths(dir, `run.${fileExtension}`);
await system.fileSystem.writeToFile(filePath, code);
await system.fileSystem.setFilePermissions(filePath, '755');
const command = getExecuteCommand(filePath, this.environment);
this.node.child_process.exec(command);
system.command.execute(command);
}
}

Expand All @@ -38,43 +40,3 @@ function getExecuteCommand(scriptPath: string, environment: Environment): string
throw Error(`unsupported os: ${OperatingSystem[environment.os]}`);
}
}

function getNodeJs(): INodeJs {
return {
os, path, fs, child_process,
};
}

export interface INodeJs {
os: INodeOs;
path: INodePath;
fs: INodeFs;
// eslint-disable-next-line camelcase
child_process: INodeChildProcess;
}

export interface INodeOs {
tmpdir(): string;
}

export interface INodePath {
join(...paths: string[]): string;
}

export interface INodeChildProcess {
exec(command: string): void;
}

export interface INodeFs {
readonly promises: INodeFsPromises;
}

interface INodeFsPromisesMakeDirectoryOptions {
recursive?: boolean;
}

interface INodeFsPromises { // https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v13/fs.d.ts
chmod(path: string, mode: string | number): Promise<void>;
mkdir(path: string, options: INodeFsPromisesMakeDirectoryOptions): Promise<string>;
writeFile(path: string, data: string): Promise<void>;
}
49 changes: 49 additions & 0 deletions src/infrastructure/Environment/Environment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { ISystemOperations } from '@/infrastructure/Environment/SystemOperations/ISystemOperations';
import { BrowserOsDetector } from './BrowserOs/BrowserOsDetector';
import { IBrowserOsDetector } from './BrowserOs/IBrowserOsDetector';
import { IEnvironment } from './IEnvironment';
import { WindowVariables } from './WindowVariables';
import { validateWindowVariables } from './WindowVariablesValidator';

export class Environment implements IEnvironment {
public static readonly CurrentEnvironment: IEnvironment = new Environment(window);

public readonly isDesktop: boolean;

public readonly os: OperatingSystem | undefined;

public readonly system: ISystemOperations | undefined;

protected constructor(
window: Partial<Window>,
browserOsDetector: IBrowserOsDetector = new BrowserOsDetector(),
windowValidator: WindowValidator = validateWindowVariables,
) {
if (!window) {
throw new Error('missing window');
}
windowValidator(window);
this.isDesktop = isDesktop(window);
if (this.isDesktop) {
this.os = window?.os;
} else {
this.os = undefined;
const userAgent = getUserAgent(window);
if (userAgent) {
this.os = browserOsDetector.detect(userAgent);
}
}
this.system = window?.system;
}
}

function getUserAgent(window: Partial<Window>): string {
return window?.navigator?.userAgent;
}

function isDesktop(window: Partial<WindowVariables>): boolean {
return window?.isDesktop === true;
}

export type WindowValidator = typeof validateWindowVariables;
8 changes: 8 additions & 0 deletions src/infrastructure/Environment/IEnvironment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { ISystemOperations } from '@/infrastructure/Environment/SystemOperations/ISystemOperations';

export interface IEnvironment {
readonly isDesktop: boolean;
readonly os: OperatingSystem | undefined;
readonly system: ISystemOperations | undefined;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
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<string>;
writeToFile(filePath: string, data: string): Promise<void>;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import { tmpdir } from 'os';
import { join } from 'path';
import { chmod, mkdir, writeFile } from 'fs/promises';
import { exec } from 'child_process';
import { ISystemOperations } from './ISystemOperations';

export function createNodeSystemOperations(): ISystemOperations {
return {
operatingSystem: {
getTempDirectory: () => tmpdir(),
},
location: {
combinePaths: (...pathSegments) => join(...pathSegments),
},
fileSystem: {
setFilePermissions: (
filePath: string,
mode: string | number,
) => chmod(filePath, mode),
createDirectory: (
directoryPath: string,
isRecursive?: boolean,
) => mkdir(directoryPath, { recursive: isRecursive }),
writeToFile: (
filePath: string,
data: string,
) => writeFile(filePath, data),
},
command: {
execute: (command) => exec(command),
},
};
}
13 changes: 13 additions & 0 deletions src/infrastructure/Environment/WindowVariables.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { OperatingSystem } from '@/domain/OperatingSystem';
import { ISystemOperations } from './SystemOperations/ISystemOperations';

export type WindowVariables = {
system: ISystemOperations;
isDesktop: boolean;
os: OperatingSystem;
};

declare global {
// eslint-disable-next-line @typescript-eslint/no-empty-interface
interface Window extends WindowVariables { }
}
Loading

0 comments on commit e9e0001

Please sign in to comment.