Skip to content

Commit

Permalink
Improve script error dialogs #304
Browse files Browse the repository at this point in the history
- Include the script's directory path #304.
- Exclude Windows-specific instructions on non-Windows OS.
- Standardize language across dialogs for consistency.

Other supporting changes:

- Add script diagnostics data collection from main process.
- Document script file storage and execution tamper protection in
  SECURITY.md.
- Remove redundant comment in `NodeReadbackFileWriter`.
- Centralize error display for uniformity and simplicity.
- Simpify `WindowVariablesValidator` to omit checks when not on the
  renderer process.
- Improve and centralize Electron environment detection.
- Use more emphatic language (don't worry) in error messages.
  • Loading branch information
undergroundwires committed Jan 17, 2024
1 parent f03fc24 commit 6ada8d4
Show file tree
Hide file tree
Showing 34 changed files with 1,167 additions and 435 deletions.
4 changes: 4 additions & 0 deletions SECURITY.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ privacy.sexy adopts a defense in depth strategy to protect users on multiple lay
The desktop application operates without persistent administrative or `sudo` privileges, reinforcing its security posture. It requests
elevation of privileges for system modifications with explicit user consent and logs every action taken with high privileges. This
approach actively minimizes potential security risks by limiting privileged operations and aligning with the principle of least privilege.
- **Secure Script Execution/Storage:**
Before executing any script, the desktop application stores a copy to allow antivirus software to perform scans. This safeguards against
any unwanted modifications. Furthermore, the application incorporates integrity checks for tamper protection. If the script file differs from
the user's selected script, the application will not execute or save the script, ensuring the processing of authentic scripts.

### Update Security and Integrity

Expand Down
21 changes: 20 additions & 1 deletion docs/desktop-vs-web-features.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@ This table highlights differences between the desktop and web versions of `priva
| [Logging](#logging) | 🟢 Available | 🔴 Not available |
| [Script execution](#script-execution) | 🟢 Available | 🔴 Not available |
| [Error handling](#error-handling) | 🟢 Advanced | 🟡 Limited |
| [Native dialogs](#error-handling) | 🟢 Available | 🔴 Not available |
| [Native dialogs](#native-dialogs) | 🟢 Available | 🔴 Not available |
| [Secure script execution/storage](#secure-script-executionstorage) | 🟢 Available | 🔴 Not available |

## Feature descriptions

Expand Down Expand Up @@ -74,3 +75,21 @@ In contrast, the web version has more basic error handling due to browser limita

The desktop version uses native dialogs, offering more features and reliability compared to the browser's file system dialogs.
These native dialogs provide a more integrated and user-friendly experience, aligning with the operating system's standard interface and functionalities.

### Secure script execution/storage

**Integrity checks:**

The desktop version of privacy.sexy implements robust integrity checks for both script execution and storage.
Featuring tamper protection, the application actively verifies the integrity of script files before executing or saving them.
If the actual contents of a script file do not align with the expected contents, the application refuses to execute or save the script.
This proactive approach ensures only unaltered and verified scripts undergo processing, thereby enhancing both security and reliability.
Due to browser constraints, this feature is absent in the web version.

**Error handling:**

In scenarios where script execution or storage encounters failure, the desktop application initiates automated troubleshooting and self-healing processes.
It also guides users through potential issues with filesystem or third-party software, such as antivirus interventions.
Specifically, the application is capable of identifying when antivirus software blocks or removes a script, providing users with tailored error messages
and detailed resolution steps. This level of proactive error handling and user guidance enhances the application's security and reliability,
offering a feature not achievable in the web version due to browser limitations.
10 changes: 10 additions & 0 deletions src/application/ScriptDiagnostics/ScriptDiagnosticsCollector.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { OperatingSystem } from '@/domain/OperatingSystem';

export interface ScriptDiagnosticsCollector {
collectDiagnosticInformation(): Promise<ScriptDiagnosticData>;
}

export interface ScriptDiagnosticData {
readonly scriptsDirectoryAbsolutePath?: string;
readonly currentOperatingSystem?: OperatingSystem;
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,6 @@ export class NodeReadbackFileWriter implements ReadbackFileWriter {
const fileWritePipelineActions: ReadonlyArray<() => Promise<FileWriteOutcome>> = [
() => this.createOrOverwriteFile(filePath, fileContents),
() => this.verifyFileExistsWithoutReading(filePath),
/*
Reading the file contents back, we can detect if the file has been altered or
removed post-creation. Removal of scripts when reading back is seen by some antivirus
software when it falsely identifies a script as harmful.
*/
() => this.verifyFileContentsByReading(filePath, fileContents),
];
for (const action of fileWritePipelineActions) {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { ElectronEnvironmentDetector, ElectronProcessType } from './ElectronEnvironmentDetector';

export class ContextIsolatedElectronDetector implements ElectronEnvironmentDetector {
constructor(
private readonly nodeProcessAccessor: NodeProcessAccessor = () => globalThis?.process,
private readonly userAgentAccessor: UserAgentAccessor = () => globalThis?.navigator?.userAgent,
) { }

public isRunningInsideElectron(): boolean {
return isNodeProcessElectronBased(this.nodeProcessAccessor)
|| isUserAgentElectronBased(this.userAgentAccessor);
}

public determineElectronProcessType(): ElectronProcessType {
const isNodeAccessible = isNodeProcessElectronBased(this.nodeProcessAccessor);
const isBrowserAccessible = isUserAgentElectronBased(this.userAgentAccessor);
if (!isNodeAccessible && !isBrowserAccessible) {
throw new Error('Unable to determine the Electron process type. Neither Node.js nor browser-based Electron contexts were detected.');
}
if (isNodeAccessible && isBrowserAccessible) {
return 'preloader'; // Only preloader can access both Node.js and browser contexts in Electron with context isolation.
}
if (isNodeAccessible) {
return 'main';
}
return 'renderer';
}
}

export type NodeProcessAccessor = () => NodeJS.Process | undefined;

function isNodeProcessElectronBased(nodeProcessAccessor: NodeProcessAccessor): boolean {
const nodeProcess = nodeProcessAccessor();
if (!nodeProcess) {
return false;
}
if (nodeProcess.versions.electron) {
// Electron populates `nodeProcess.versions.electron` with its version, see https://web.archive.org/web/20240113162837/https://www.electronjs.org/docs/latest/api/process#processversionselectron-readonly.
return true;
}
return false;
}

export type UserAgentAccessor = () => string | undefined;

function isUserAgentElectronBased(
userAgentAccessor: UserAgentAccessor,
): boolean {
const userAgent = userAgentAccessor();
if (userAgent?.includes('Electron')) {
return true;
}
return false;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
export interface ElectronEnvironmentDetector {
isRunningInsideElectron(): boolean;
determineElectronProcessType(): ElectronProcessType;
}

export type ElectronProcessType = 'main' | 'preloader' | 'renderer';
45 changes: 14 additions & 31 deletions src/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory.ts
Original file line number Diff line number Diff line change
@@ -1,49 +1,32 @@
import { ElectronEnvironmentDetector } from './Electron/ElectronEnvironmentDetector';
import { BrowserRuntimeEnvironment } from './Browser/BrowserRuntimeEnvironment';
import { NodeRuntimeEnvironment } from './Node/NodeRuntimeEnvironment';
import { RuntimeEnvironment } from './RuntimeEnvironment';
import { ContextIsolatedElectronDetector } from './Electron/ContextIsolatedElectronDetector';

export const CurrentEnvironment = determineAndCreateRuntimeEnvironment({
window: globalThis.window,
process: globalThis.process,
});
export const CurrentEnvironment = determineAndCreateRuntimeEnvironment(globalThis.window);

export function determineAndCreateRuntimeEnvironment(
globalAccessor: GlobalPropertiesAccessor,
globalWindow: Window | undefined | null = globalThis.window,
electronDetector: ElectronEnvironmentDetector = new ContextIsolatedElectronDetector(),
browserEnvironmentFactory: BrowserRuntimeEnvironmentFactory = (
window,
) => new BrowserRuntimeEnvironment(window),
nodeEnvironmentFactory: NodeRuntimeEnvironmentFactory = (
process: NodeJS.Process,
) => new NodeRuntimeEnvironment(process),
nodeEnvironmentFactory: NodeRuntimeEnvironmentFactory = () => new NodeRuntimeEnvironment(),
): RuntimeEnvironment {
if (isElectronMainProcess(globalAccessor.process)) {
return nodeEnvironmentFactory(globalAccessor.process);
if (
electronDetector.isRunningInsideElectron()
&& electronDetector.determineElectronProcessType() === 'main') {
return nodeEnvironmentFactory();
}
const { window } = globalAccessor;
if (!window) {
if (!globalWindow) {
throw new Error('Unsupported runtime environment: The current context is neither a recognized browser nor a desktop environment.');
}
return browserEnvironmentFactory(window);
}

function isElectronMainProcess(
nodeProcess: NodeJS.Process | undefined,
): nodeProcess is NodeJS.Process {
// Electron populates `nodeProcess.versions.electron` with its version, see https://web.archive.org/web/20240113162837/https://www.electronjs.org/docs/latest/api/process#processversionselectron-readonly.
if (!nodeProcess) {
return false;
}
if (nodeProcess.versions.electron) {
return true;
}
return false;
return browserEnvironmentFactory(globalWindow);
}

export type BrowserRuntimeEnvironmentFactory = (window: Window) => RuntimeEnvironment;

export type NodeRuntimeEnvironmentFactory = (process: NodeJS.Process) => NodeRuntimeEnvironment;
export type NodeRuntimeEnvironmentFactory = () => NodeRuntimeEnvironment;

export interface GlobalPropertiesAccessor {
readonly window: Window | undefined;
readonly process: NodeJS.Process | undefined;
}
export type GlobalWindowAccessor = Window | undefined;
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import { ScriptDiagnosticData, ScriptDiagnosticsCollector } from '@/application/ScriptDiagnostics/ScriptDiagnosticsCollector';
import { RuntimeEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironment';
import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory';
import { PersistentDirectoryProvider } from '@/infrastructure/CodeRunner/Creation/Directory/PersistentDirectoryProvider';
import { ScriptDirectoryProvider } from '../CodeRunner/Creation/Directory/ScriptDirectoryProvider';

export class ScriptEnvironmentDiagnosticsCollector implements ScriptDiagnosticsCollector {
constructor(
private readonly directoryProvider: ScriptDirectoryProvider = new PersistentDirectoryProvider(),
private readonly environment: RuntimeEnvironment = CurrentEnvironment,
) { }

public async collectDiagnosticInformation(): Promise<ScriptDiagnosticData> {
const { directoryAbsolutePath } = await this.directoryProvider.provideScriptDirectory();
return {
scriptsDirectoryAbsolutePath: directoryAbsolutePath,
currentOperatingSystem: this.environment.os,
};
}
}
2 changes: 2 additions & 0 deletions src/infrastructure/WindowVariables/WindowVariables.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { OperatingSystem } from '@/domain/OperatingSystem';
import { Logger } from '@/application/Common/Log/Logger';
import { CodeRunner } from '@/application/CodeRunner/CodeRunner';
import { Dialog } from '@/presentation/common/Dialog';
import { ScriptDiagnosticsCollector } from '@/application/ScriptDiagnostics/ScriptDiagnosticsCollector';

/* Primary entry point for platform-specific injections */
export interface WindowVariables {
Expand All @@ -10,4 +11,5 @@ export interface WindowVariables {
readonly os?: OperatingSystem;
readonly log?: Logger;
readonly dialog?: Dialog;
readonly scriptDiagnosticsCollector?: ScriptDiagnosticsCollector;
}
48 changes: 28 additions & 20 deletions src/infrastructure/WindowVariables/WindowVariablesValidator.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { ContextIsolatedElectronDetector } from '@/infrastructure/RuntimeEnvironment/Electron/ContextIsolatedElectronDetector';
import { ElectronEnvironmentDetector } from '@/infrastructure/RuntimeEnvironment/Electron/ElectronEnvironmentDetector';
import { OperatingSystem } from '@/domain/OperatingSystem';
import {
PropertyKeys, isBoolean, isFunction, isNumber, isPlainObject,
Expand All @@ -7,7 +9,14 @@ import { WindowVariables } from './WindowVariables';
/**
* Checks for consistency in runtime environment properties injected by Electron preloader.
*/
export function validateWindowVariables(variables: Partial<WindowVariables>) {
export function validateWindowVariables(
variables: Partial<WindowVariables>,
electronDetector: ElectronEnvironmentDetector = new ContextIsolatedElectronDetector(),
) {
if (!electronDetector.isRunningInsideElectron()
|| electronDetector.determineElectronProcessType() !== 'renderer') {
return;
}
if (!isPlainObject(variables)) {
throw new Error('window is not an object');
}
Expand All @@ -20,12 +29,11 @@ export function validateWindowVariables(variables: Partial<WindowVariables>) {
function* testEveryProperty(variables: Partial<WindowVariables>): Iterable<string> {
const tests: Record<PropertyKeys<Required<WindowVariables>>, boolean> = {
os: testOperatingSystem(variables.os),
isRunningAsDesktopApplication: testIsRunningAsDesktopApplication(
variables.isRunningAsDesktopApplication,
),
isRunningAsDesktopApplication: testIsRunningAsDesktopApplication(variables),
codeRunner: testCodeRunner(variables),
log: testLogger(variables),
dialog: testDialog(variables),
scriptDiagnosticsCollector: testScriptDiagnosticsCollector(variables),
};

for (const [propertyName, testResult] of Object.entries(tests)) {
Expand All @@ -49,30 +57,30 @@ function testOperatingSystem(os: unknown): boolean {
}

function testLogger(variables: Partial<WindowVariables>): boolean {
if (!variables.isRunningAsDesktopApplication) {
return true;
}
return isPlainObject(variables.log);
return isPlainObject(variables.log)
&& isFunction(variables.log.debug)
&& isFunction(variables.log.info)
&& isFunction(variables.log.error)
&& isFunction(variables.log.warn);
}

function testCodeRunner(variables: Partial<WindowVariables>): boolean {
if (!variables.isRunningAsDesktopApplication) {
return true;
}
return isPlainObject(variables.codeRunner)
&& isFunction(variables.codeRunner.runCode);
}

function testIsRunningAsDesktopApplication(isRunningAsDesktopApplication: unknown): boolean {
if (isRunningAsDesktopApplication === undefined) {
return true;
}
return isBoolean(isRunningAsDesktopApplication);
function testIsRunningAsDesktopApplication(variables: Partial<WindowVariables>): boolean {
return isBoolean(variables.isRunningAsDesktopApplication)
&& variables.isRunningAsDesktopApplication === true;
}

function testDialog(variables: Partial<WindowVariables>): boolean {
if (!variables.isRunningAsDesktopApplication) {
return true;
}
return isPlainObject(variables.dialog);
return isPlainObject(variables.dialog)
&& isFunction(variables.dialog.saveFile)
&& isFunction(variables.dialog.showError);
}

function testScriptDiagnosticsCollector(variables: Partial<WindowVariables>): boolean {
return isPlainObject(variables.scriptDiagnosticsCollector)
&& isFunction(variables.scriptDiagnosticsCollector.collectDiagnosticInformation);
}
5 changes: 5 additions & 0 deletions src/presentation/bootstrapping/DependencyProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import { useLogger } from '@/presentation/components/Shared/Hooks/Log/UseLogger'
import { useCodeRunner } from '@/presentation/components/Shared/Hooks/UseCodeRunner';
import { CurrentEnvironment } from '@/infrastructure/RuntimeEnvironment/RuntimeEnvironmentFactory';
import { useDialog } from '@/presentation/components/Shared/Hooks/Dialog/UseDialog';
import { useScriptDiagnosticsCollector } from '@/presentation/components/Shared/Hooks/UseScriptDiagnosticsCollector';

export function provideDependencies(
context: IApplicationContext,
Expand Down Expand Up @@ -72,6 +73,10 @@ export function provideDependencies(
InjectionKeys.useDialog,
useDialog,
),
useScriptDiagnosticsCollector: (di) => di.provide(
InjectionKeys.useScriptDiagnosticsCollector,
useScriptDiagnosticsCollector,
),
};
registerAll(Object.values(resolvers), api);
}
Expand Down
Loading

0 comments on commit 6ada8d4

Please sign in to comment.