Skip to content

Commit f03fc24

Browse files
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.
1 parent 756c736 commit f03fc24

23 files changed

Lines changed: 841 additions & 142 deletions

File tree

‎2‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
Show error on AV removal on desktop $264, $304
2+
3+
This solves $264 where users do not get error messages when running
4+
script file fails due to antivirus intervention (it being blocking the
5+
script file as soon as privacy.sexy generates it to run it). Now if the
6+
desktop app users tries to save or run a script file and it afils due to
7+
antivirus removal, they'll get a special error message with guiding next
8+
steps.
9+
10+
- Add additional check to able to fail if the file writing fails. This
11+
includes trying to reading the written file back as suggested in $304.
12+
This successfully detects antivirus (Defender) intervation as read
13+
file operation triggers the antivirus scan that deletes the file.
14+
- Show directory and file path in error messages as suggested in $304.
15+
- Show an error message with more detailed information if an antivirus
16+
is detected.
17+
18+
# Please enter the commit message for your changes. Lines starting
19+
# with '#' will be ignored, and an empty message aborts the commit.
20+
#
21+
# Date: Tue Jan 16 16:23:08 2024 +0100
22+
#
23+
# On branch master
24+
# Your branch is ahead of 'origin/master' by 1 commit.
25+
# (use "git push" to publish your local commits)
26+
#
27+
# Changes to be committed:
28+
# modified: ../../application/CodeRunner/CodeRunner.ts
29+
# new file: NodeReliableFileWriter.ts
30+
# new file: ReliableFileWriter.ts
31+
#

‎docs/tests.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,9 @@ There are different types of tests executed:
2929

3030
- Evaluate individual components in isolation.
3131
- Located in [`./tests/unit`](./../tests/unit).
32-
- Achieve isolation using [stubs](./../tests/unit/shared/Stubs).
32+
- Achieve isolation using stubs where you place:
33+
- Common stubs in [`./shared/Stubs`](./../tests/unit/shared/Stubs),
34+
- Component-specific stubs in same folder as test file.
3335
- Include Vue component tests, enabled by `@vue/test-utils`.
3436

3537
#### Unit tests naming

‎src/application/CodeRunner/CodeRunner.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,16 +5,17 @@ export interface CodeRunner {
55
): Promise<CodeRunOutcome>;
66
}
77

8+
export type CodeRunOutcome = SuccessfulCodeRun | FailedCodeRun;
9+
810
export type CodeRunErrorType =
911
| 'FileWriteError'
12+
| 'FileReadbackVerificationError'
1013
| 'FilePathGenerationError'
1114
| 'UnsupportedOperatingSystem'
1215
| 'FileExecutionError'
1316
| 'DirectoryCreationError'
1417
| 'UnexpectedError';
1518

16-
export type CodeRunOutcome = SuccessfulCodeRun | FailedCodeRun;
17-
1819
interface CodeRunStatus {
1920
readonly success: boolean;
2021
readonly error?: CodeRunError;

‎src/infrastructure/CodeRunner/Creation/ScriptFileCreationOrchestrator.ts‎

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger';
22
import { Logger } from '@/application/Common/Log/Logger';
33
import { CodeRunError, CodeRunErrorType } from '@/application/CodeRunner/CodeRunner';
4+
import { FileReadbackVerificationErrors, ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter';
5+
import { NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter';
46
import { SystemOperations } from '../System/SystemOperations';
57
import { NodeElectronSystemOperations } from '../System/NodeElectronSystemOperations';
68
import { FilenameGenerator } from './Filename/FilenameGenerator';
@@ -14,6 +16,7 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator {
1416
private readonly system: SystemOperations = new NodeElectronSystemOperations(),
1517
private readonly filenameGenerator: FilenameGenerator = new TimestampedFilenameGenerator(),
1618
private readonly directoryProvider: ScriptDirectoryProvider = new PersistentDirectoryProvider(),
19+
private readonly fileWriter: ReadbackFileWriter = new NodeReadbackFileWriter(),
1720
private readonly logger: Logger = ElectronLogger,
1821
) { }
1922

@@ -65,17 +68,19 @@ export class ScriptFileCreationOrchestrator implements ScriptFileCreator {
6568
filePath: string,
6669
contents: string,
6770
): Promise<FileWriteOutcome> {
68-
try {
69-
this.logger.info(`Creating file at ${filePath}, size: ${contents.length} characters`);
70-
await this.system.fileSystem.writeToFile(filePath, contents);
71-
this.logger.info(`File created successfully at ${filePath}`);
71+
const {
72+
success, error,
73+
} = await this.fileWriter.writeAndVerifyFile(filePath, contents);
74+
if (success) {
7275
return { success: true };
73-
} catch (error) {
74-
return {
75-
success: false,
76-
error: this.handleException(error, 'FileWriteError'),
77-
};
7876
}
77+
return {
78+
success: false,
79+
error: {
80+
message: error.message,
81+
type: FileReadbackVerificationErrors.find((e) => e === error.type) ? 'FileReadbackVerificationError' : 'FileWriteError',
82+
},
83+
};
7984
}
8085

8186
private handleException(

‎src/infrastructure/CodeRunner/System/NodeElectronSystemOperations.ts‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { join } from 'node:path';
2-
import { chmod, mkdir, writeFile } from 'node:fs/promises';
2+
import { chmod, mkdir } from 'node:fs/promises';
33
import { exec } from 'node:child_process';
44
import { app } from 'electron/main';
55
import {
@@ -46,10 +46,6 @@ export class NodeElectronSystemOperations implements SystemOperations {
4646
// when `recursive` is true, or empty return value.
4747
// See https://github.com/nodejs/node/pull/31530
4848
},
49-
writeToFile: (
50-
filePath: string,
51-
data: string,
52-
) => writeFile(filePath, data),
5349
};
5450

5551
public readonly command: CommandOps = {

‎src/infrastructure/CodeRunner/System/SystemOperations.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,5 +20,4 @@ export interface CommandOps {
2020
export interface FileSystemOps {
2121
setFilePermissions(filePath: string, mode: string | number): Promise<void>;
2222
createDirectory(directoryPath: string, isRecursive?: boolean): Promise<void>;
23-
writeToFile(filePath: string, data: string): Promise<void>;
2423
}

‎src/infrastructure/Dialog/Electron/NodeElectronSaveFileDialog.ts‎

Lines changed: 16 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
import { join } from 'node:path';
2-
import { writeFile } from 'node:fs/promises';
32
import { app, dialog } from 'electron/main';
43
import { Logger } from '@/application/Common/Log/Logger';
54
import { ElectronLogger } from '@/infrastructure/Log/ElectronLogger';
65
import {
76
FileType, SaveFileError, SaveFileErrorType, SaveFileOutcome,
87
} from '@/presentation/common/Dialog';
8+
import { FileReadbackVerificationErrors, ReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/ReadbackFileWriter';
9+
import { NodeReadbackFileWriter } from '@/infrastructure/ReadbackFileWriter/NodeReadbackFileWriter';
910
import { ElectronSaveFileDialog } from './ElectronSaveFileDialog';
1011

1112
export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog {
@@ -15,10 +16,8 @@ export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog {
1516
getUserDownloadsPath: () => app.getPath('downloads'),
1617
showSaveDialog: dialog.showSaveDialog.bind(dialog),
1718
},
18-
private readonly node: NodeFileOperations = {
19-
join,
20-
writeFile,
21-
},
19+
private readonly node: NodeFileOperations = { join },
20+
private readonly fileWriter: ReadbackFileWriter = new NodeReadbackFileWriter(),
2221
) { }
2322

2423
public async saveFile(
@@ -55,19 +54,19 @@ export class NodeElectronSaveFileDialog implements ElectronSaveFileDialog {
5554
filePath: string,
5655
fileContents: string,
5756
): Promise<SaveFileOutcome> {
58-
try {
59-
this.logger.info(`Saving file: ${filePath}`);
60-
await this.node.writeFile(filePath, fileContents);
61-
this.logger.info(`File saved: ${filePath}`);
62-
return {
63-
success: true,
64-
};
65-
} catch (error) {
66-
return {
67-
success: false,
68-
error: this.handleException(error, 'FileCreationError'),
69-
};
57+
const {
58+
success, error,
59+
} = await this.fileWriter.writeAndVerifyFile(filePath, fileContents);
60+
if (success) {
61+
return { success: true };
7062
}
63+
return {
64+
success: false,
65+
error: {
66+
message: error.message,
67+
type: FileReadbackVerificationErrors.find((e) => e === error.type) ? 'FileReadbackVerificationError' : 'FileCreationError',
68+
},
69+
};
7170
}
7271

7372
private async showSaveFileDialog(
@@ -139,7 +138,6 @@ export interface ElectronFileDialogOperations {
139138

140139
export interface NodeFileOperations {
141140
readonly join: typeof join;
142-
writeFile(file: string, data: string): Promise<void>;
143141
}
144142

145143
function getDialogFileFilters(fileType: FileType): Electron.FileFilter[] {
Lines changed: 120 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
1+
import { writeFile, access, readFile } from 'node:fs/promises';
2+
import { constants } from 'node:fs';
3+
import { Logger } from '@/application/Common/Log/Logger';
4+
import { ElectronLogger } from '../Log/ElectronLogger';
5+
import {
6+
FailedFileWrite, ReadbackFileWriter, FileWriteErrorType,
7+
FileWriteOutcome, SuccessfulFileWrite,
8+
} from './ReadbackFileWriter';
9+
10+
const FILE_ENCODING: NodeJS.BufferEncoding = 'utf-8';
11+
12+
export class NodeReadbackFileWriter implements ReadbackFileWriter {
13+
constructor(
14+
private readonly logger: Logger = ElectronLogger,
15+
private readonly fileSystem: FileReadWriteOperations = {
16+
writeFile,
17+
readFile: (path, encoding) => readFile(path, encoding),
18+
access,
19+
},
20+
) { }
21+
22+
public async writeAndVerifyFile(
23+
filePath: string,
24+
fileContents: string,
25+
): Promise<FileWriteOutcome> {
26+
this.logger.info(`Starting file write and verification process for: ${filePath}`);
27+
const fileWritePipelineActions: ReadonlyArray<() => Promise<FileWriteOutcome>> = [
28+
() => this.createOrOverwriteFile(filePath, fileContents),
29+
() => this.verifyFileExistsWithoutReading(filePath),
30+
/*
31+
Reading the file contents back, we can detect if the file has been altered or
32+
removed post-creation. Removal of scripts when reading back is seen by some antivirus
33+
software when it falsely identifies a script as harmful.
34+
*/
35+
() => this.verifyFileContentsByReading(filePath, fileContents),
36+
];
37+
for (const action of fileWritePipelineActions) {
38+
const actionOutcome = await action(); // eslint-disable-line no-await-in-loop
39+
if (!actionOutcome.success) {
40+
return actionOutcome;
41+
}
42+
}
43+
return this.reportSuccess(`File successfully written and verified: ${filePath}`);
44+
}
45+
46+
private async createOrOverwriteFile(
47+
filePath: string,
48+
fileContents: string,
49+
): Promise<FileWriteOutcome> {
50+
try {
51+
this.logger.info(`Creating file at ${filePath}, size: ${fileContents.length} characters`);
52+
await this.fileSystem.writeFile(filePath, fileContents, FILE_ENCODING);
53+
return this.reportSuccess('Created file.');
54+
} catch (error) {
55+
return this.reportFailure('WriteOperationFailed', error);
56+
}
57+
}
58+
59+
private async verifyFileExistsWithoutReading(
60+
filePath: string,
61+
): Promise<FileWriteOutcome> {
62+
try {
63+
await this.fileSystem.access(filePath, constants.F_OK);
64+
return this.reportSuccess('Verified file existence without reading.');
65+
} catch (error) {
66+
return this.reportFailure('FileExistenceVerificationFailed', error);
67+
}
68+
}
69+
70+
private async verifyFileContentsByReading(
71+
filePath: string,
72+
expectedFileContents: string,
73+
): Promise<FileWriteOutcome> {
74+
try {
75+
const actualFileContents = await this.fileSystem.readFile(filePath, FILE_ENCODING);
76+
if (actualFileContents !== expectedFileContents) {
77+
return this.reportFailure(
78+
'ContentVerificationFailed',
79+
[
80+
'The contents of the written file do not match the expected contents.',
81+
'Written file contents do not match the expected file contents',
82+
`File path: ${filePath}`,
83+
`Expected total characters: ${actualFileContents.length}`,
84+
`Actual total characters: ${expectedFileContents.length}`,
85+
].join('\n'),
86+
);
87+
}
88+
return this.reportSuccess('Verified file content by reading.');
89+
} catch (error) {
90+
return this.reportFailure('ReadVerificationFailed', error);
91+
}
92+
}
93+
94+
private reportFailure(
95+
errorType: FileWriteErrorType,
96+
error: Error | string,
97+
): FailedFileWrite {
98+
this.logger.error('Error saving file', errorType, error);
99+
return {
100+
success: false,
101+
error: {
102+
type: errorType,
103+
message: typeof error === 'string' ? error : error.message,
104+
},
105+
};
106+
}
107+
108+
private reportSuccess(successAction: string): SuccessfulFileWrite {
109+
this.logger.info(`Successful file save: ${successAction}`);
110+
return {
111+
success: true,
112+
};
113+
}
114+
}
115+
116+
export interface FileReadWriteOperations {
117+
readonly writeFile: typeof writeFile;
118+
readonly access: typeof access;
119+
readFile: (filePath: string, encoding: NodeJS.BufferEncoding) => Promise<string>;
120+
}
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
/**
2+
* It defines the contract for file writing operations with an added layer of
3+
* verification. This approach is useful in environments where file write operations
4+
* might be silently intercepted or manipulated by external factors, such as antivirus software.
5+
*
6+
* This additional verification provides a more reliable and transparent file writing
7+
* process, enhancing the application's resilience against external disruptions and
8+
* improving the overall user experience. It enables the application to notify users
9+
* of potential issues, such as antivirus interventions, and offer guidance on how to
10+
* resolve them.
11+
*/
12+
export interface ReadbackFileWriter {
13+
writeAndVerifyFile(filePath: string, fileContents: string): Promise<FileWriteOutcome>;
14+
}
15+
16+
export type FileWriteOutcome = SuccessfulFileWrite | FailedFileWrite;
17+
18+
export type FileWriteErrorType =
19+
| UnionOfConstArray<typeof FileWriteOperationErrors>
20+
| UnionOfConstArray<typeof FileReadbackVerificationErrors>;
21+
22+
export const FileWriteOperationErrors = [
23+
'WriteOperationFailed',
24+
] as const;
25+
26+
export const FileReadbackVerificationErrors = [
27+
'FileExistenceVerificationFailed',
28+
'ContentVerificationFailed',
29+
30+
/*
31+
This error indicates a failure in verifying the contents of a written file.
32+
This error often occurs when antivirus software falsely identifies a script as harmful and
33+
either alters or removes it during the readback process. This verification step is crucial
34+
for detecting and handling such antivirus interventions.
35+
*/
36+
'ReadVerificationFailed',
37+
] as const;
38+
39+
interface FileWriteStatus {
40+
readonly success: boolean;
41+
readonly error?: FileWriteError;
42+
}
43+
44+
export interface SuccessfulFileWrite extends FileWriteStatus {
45+
readonly success: true;
46+
readonly error?: undefined;
47+
}
48+
49+
export interface FailedFileWrite extends FileWriteStatus {
50+
readonly success: false;
51+
readonly error: FileWriteError;
52+
}
53+
54+
export interface FileWriteError {
55+
readonly type: FileWriteErrorType;
56+
readonly message: string;
57+
}
58+
59+
type UnionOfConstArray<T extends ReadonlyArray<unknown>> = T[number];

‎src/presentation/common/Dialog.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,4 +32,5 @@ export interface SaveFileError {
3232

3333
export type SaveFileErrorType =
3434
| 'FileCreationError'
35+
| 'FileReadbackVerificationError'
3536
| 'DialogDisplayError';

0 commit comments

Comments
 (0)