Add AD detection on desktop app #264, #304

This commit addresses issues #264 and #304, where users were not
receiving error messages when script execution failed due to
antivirus intervention, particularly with Microsoft Defender.
Now, desktop app users will see a detailed error message with
guidance on next steps if script saving or execution fails due
to antivirus removal.

Key changes:

- Implement a check to detect failure in file writing,
  including reading the written file back. This method effectively
  detects antivirus interventions, as the read operation triggers
  an antivirus scan, leading to file deletion by the antivirus.
- Introduce a specific error message for scenarios where an
  antivirus intervention is detected.
This commit is contained in:
undergroundwires
2024-01-16 22:26:28 +01:00
parent 756c736e21
commit f03fc24098
23 changed files with 841 additions and 142 deletions

View File

@@ -1,6 +1,8 @@
import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger';
import { Logger } from '@/application/Common/Log/Logger';
import { CodeRunError, CodeRunErrorType } from '@/application/CodeRunner/CodeRunner';
import { FileReadbackVerificationErrors, ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter';
import { NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter';
import { SystemOperations } from '../System/SystemOperations';
import { NodeElectronSystemOperations } from '../System/NodeElectronSystemOperations';
import { FilenameGenerator } from './Filename/FilenameGenerator';
@@ -14,6 +16,7 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator {
private readonly system: SystemOperations = new NodeElectronSystemOperations(),
private readonly filenameGenerator: FilenameGenerator = new TimestampedFilenameGenerator(),
private readonly directoryProvider: ScriptDirectoryProvider = new PersistentDirectoryProvider(),
private readonly fileWriter: ReadbackFileWriter = new NodeReadbackFileWriter(),
private readonly logger: Logger = ElectronLogger,
) { }
@@ -65,17 +68,19 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator {
filePath: string,
contents: string,
): Promise<FileWriteOutcome> {
try {
this.logger.info(`Creating file at ${filePath}, size: ${contents.length} characters`);
await this.system.fileSystem.writeToFile(filePath, contents);
this.logger.info(`File created successfully at ${filePath}`);
const {
success, error,
} = await this.fileWriter.writeAndVerifyFile(filePath, contents);
if (success) {
return { success: true };
} catch (error) {
return {
success: false,
error: this.handleException(error, 'FileWriteError'),
};
}
return {
success: false,
error: {
message: error.message,
type: FileReadbackVerificationErrors.find((e) => e === error.type) ? 'FileReadbackVerificationError' : 'FileWriteError',
},
};
}
private handleException(

View File

@@ -1,5 +1,5 @@
import { join } from 'node:path';
import { chmod, mkdir, writeFile } from 'node:fs/promises';
import { chmod, mkdir } from 'node:fs/promises';
import { exec } from 'node:child_process';
import { app } from 'electron/main';
import {
@@ -46,10 +46,6 @@ export class NodeElectronSystemOperations implements SystemOperations {
// when `recursive` is true, or empty return value.
// See https://github.com/nodejs/node/pull/31530
},
writeToFile: (
filePath: string,
data: string,
) => writeFile(filePath, data),
};
public readonly command: CommandOps = {

View File

@@ -20,5 +20,4 @@ export interface CommandOps {
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,11 +1,12 @@
import { join } from 'node:path';
import { writeFile } from 'node:fs/promises';
import { app, dialog } from 'electron/main';
import { Logger } from '@/application/Common/Log/Logger';
import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger';
import {
FileType, SaveFileError, SaveFileErrorType, SaveFileOutcome,
} from '@/presentation/common/Dialog';
import { FileReadbackVerificationErrors, ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter';
import { NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter';
import { ElectronSaveFileDialog } from './ElectronSaveFileDialog';
export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog {
@@ -15,10 +16,8 @@ export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog {
getUserDownloadsPath: () => app.getPath('downloads'),
showSaveDialog: dialog.showSaveDialog.bind(dialog),
},
private readonly node: NodeFileOperations = {
join,
writeFile,
},
private readonly node: NodeFileOperations = { join },
private readonly fileWriter: ReadbackFileWriter = new NodeReadbackFileWriter(),
) { }
public async saveFile(
@@ -55,19 +54,19 @@ export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog {
filePath: string,
fileContents: string,
): Promise<SaveFileOutcome> {
try {
this.logger.info(`Saving file: ${filePath}`);
await this.node.writeFile(filePath, fileContents);
this.logger.info(`File saved: ${filePath}`);
return {
success: true,
};
} catch (error) {
return {
success: false,
error: this.handleException(error, 'FileCreationError'),
};
const {
success, error,
} = await this.fileWriter.writeAndVerifyFile(filePath, fileContents);
if (success) {
return { success: true };
}
return {
success: false,
error: {
message: error.message,
type: FileReadbackVerificationErrors.find((e) => e === error.type) ? 'FileReadbackVerificationError' : 'FileCreationError',
},
};
}
private async showSaveFileDialog(
@@ -139,7 +138,6 @@ export interface ElectronFileDialogOperations {
export interface NodeFileOperations {
readonly join: typeof join;
writeFile(file: string, data: string): Promise<void>;
}
function getDialogFileFilters(fileType: FileType): Electron.FileFilter[] {

View File

@@ -0,0 +1,120 @@
import { writeFile, access, readFile } from 'node:fs/promises';
import { constants } from 'node:fs';
import { Logger } from '@/application/Common/Log/Logger';
import { ElectronLogger } from '../Log/ElectronLogger';
import {
FailedFileWrite, ReadbackFileWriter, FileWriteErrorType,
FileWriteOutcome, SuccessfulFileWrite,
} from './ReadbackFileWriter';
const FILE_ENCODING: NodeJS.BufferEncoding = 'utf-8';
export class NodeReadbackFileWriter implements ReadbackFileWriter {
constructor(
private readonly logger: Logger = ElectronLogger,
private readonly fileSystem: FileReadWriteOperations = {
writeFile,
readFile: (path, encoding) => readFile(path, encoding),
access,
},
) { }
public async writeAndVerifyFile(
filePath: string,
fileContents: string,
): Promise<FileWriteOutcome> {
this.logger.info(`Starting file write and verification process for: ${filePath}`);
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) {
const actionOutcome = await action(); // eslint-disable-line no-await-in-loop
if (!actionOutcome.success) {
return actionOutcome;
}
}
return this.reportSuccess(`File successfully written and verified: ${filePath}`);
}
private async createOrOverwriteFile(
filePath: string,
fileContents: string,
): Promise<FileWriteOutcome> {
try {
this.logger.info(`Creating file at ${filePath}, size: ${fileContents.length} characters`);
await this.fileSystem.writeFile(filePath, fileContents, FILE_ENCODING);
return this.reportSuccess('Created file.');
} catch (error) {
return this.reportFailure('WriteOperationFailed', error);
}
}
private async verifyFileExistsWithoutReading(
filePath: string,
): Promise<FileWriteOutcome> {
try {
await this.fileSystem.access(filePath, constants.F_OK);
return this.reportSuccess('Verified file existence without reading.');
} catch (error) {
return this.reportFailure('FileExistenceVerificationFailed', error);
}
}
private async verifyFileContentsByReading(
filePath: string,
expectedFileContents: string,
): Promise<FileWriteOutcome> {
try {
const actualFileContents = await this.fileSystem.readFile(filePath, FILE_ENCODING);
if (actualFileContents !== expectedFileContents) {
return this.reportFailure(
'ContentVerificationFailed',
[
'The contents of the written file do not match the expected contents.',
'Written file contents do not match the expected file contents',
`File path: ${filePath}`,
`Expected total characters: ${actualFileContents.length}`,
`Actual total characters: ${expectedFileContents.length}`,
].join('\n'),
);
}
return this.reportSuccess('Verified file content by reading.');
} catch (error) {
return this.reportFailure('ReadVerificationFailed', error);
}
}
private reportFailure(
errorType: FileWriteErrorType,
error: Error | string,
): FailedFileWrite {
this.logger.error('Error saving file', errorType, error);
return {
success: false,
error: {
type: errorType,
message: typeof error === 'string' ? error : error.message,
},
};
}
private reportSuccess(successAction: string): SuccessfulFileWrite {
this.logger.info(`Successful file save: ${successAction}`);
return {
success: true,
};
}
}
export interface FileReadWriteOperations {
readonly writeFile: typeof writeFile;
readonly access: typeof access;
readFile: (filePath: string, encoding: NodeJS.BufferEncoding) => Promise<string>;
}

View File

@@ -0,0 +1,59 @@
/**
* It defines the contract for file writing operations with an added layer of
* verification. This approach is useful in environments where file write operations
* might be silently intercepted or manipulated by external factors, such as antivirus software.
*
* This additional verification provides a more reliable and transparent file writing
* process, enhancing the application's resilience against external disruptions and
* improving the overall user experience. It enables the application to notify users
* of potential issues, such as antivirus interventions, and offer guidance on how to
* resolve them.
*/
export interface ReadbackFileWriter {
writeAndVerifyFile(filePath: string, fileContents: string): Promise<FileWriteOutcome>;
}
export type FileWriteOutcome = SuccessfulFileWrite | FailedFileWrite;
export type FileWriteErrorType =
| UnionOfConstArray<typeof FileWriteOperationErrors>
| UnionOfConstArray<typeof FileReadbackVerificationErrors>;
export const FileWriteOperationErrors = [
'WriteOperationFailed',
] as const;
export const FileReadbackVerificationErrors = [
'FileExistenceVerificationFailed',
'ContentVerificationFailed',
/*
This error indicates a failure in verifying the contents of a written file.
This error often occurs when antivirus software falsely identifies a script as harmful and
either alters or removes it during the readback process. This verification step is crucial
for detecting and handling such antivirus interventions.
*/
'ReadVerificationFailed',
] as const;
interface FileWriteStatus {
readonly success: boolean;
readonly error?: FileWriteError;
}
export interface SuccessfulFileWrite extends FileWriteStatus {
readonly success: true;
readonly error?: undefined;
}
export interface FailedFileWrite extends FileWriteStatus {
readonly success: false;
readonly error: FileWriteError;
}
export interface FileWriteError {
readonly type: FileWriteErrorType;
readonly message: string;
}
type UnionOfConstArray<T extends ReadonlyArray<unknown>> = T[number];