fix: complete local-first improvement plan and restore app startup / tool IPC - #98
fix: complete local-first improvement plan and restore app startup / tool IPC#98schvarts1 wants to merge 3 commits into
Conversation
…tool IPC Split main.js into lifecycle.js and windowManager.js; harden toast BrowserWindow with contextIsolation/sandbox Add validateArgs IPC validation, structured error classes, and ALLOWED_INVOKE/ALLOWED_ON preload allowlists Wire IPC validation across 18+ handlers in scan, firewall, network, quarantine, process, system Register missing tools:list and tools:run handlers so tool actions work again Add auditLog module and wire into FirewallManager, EmergencyLockdown, QuarantineManager, ProcessInspector, maintenanceScheduler Extend database schema with user_blocklist, user_domain_blocklist, audit_log, scanned_files Add multi-folder scans, incremental scan support, and empty-catch cleanup Extract toast and scan-report templates; replace empty catch blocks with logging Add cross-platform ClamAV engine stubs and platform-aware serviceRegistry Preserve 282-test green baseline; verify with npm test
|
ⓘ Qodo reviews are paused because your trial has ended. Ask your workspace admin to add credits to resume reviews. Manage billing |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesThe change centralizes Electron lifecycle and window management, adds typed IPC validation and errors, introduces audit and scan-cache persistence, refactors ClamAV by platform, strengthens quarantine encryption, adds network scripts and blocklists, improves diagnostics, and validates native-host installation. Application foundation
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant Electron
participant lifecycle
participant serviceRegistry
participant windowManager
participant IPC
Electron->>lifecycle: start application
lifecycle->>serviceRegistry: construct services
lifecycle->>windowManager: initialize windows and splash state
lifecycle->>IPC: register validated handlers
IPC->>serviceRegistry: execute service operation
lifecycle->>windowManager: forward progress and notifications
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Note
Due to the large number of review comments, Critical severity comments were prioritized as inline comments.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/security/NetworkMonitor.js (1)
36-65: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winDo not discard the interface statistics when the PowerShell script fails.
si.networkStats()runs first and succeeds on every platform.runPs1('network-stats.ps1')then throws on non-Windows hosts, on a missing script, and on a script timeout. The singlecatchreturnsinterfaces: [], so the working interface data is lost together with the connection counts.Wrap only the script call in its own
try/catchand keepinterfaceStats.🐛 Proposed fix
- const stdout = await runPs1('network-stats.ps1'); - const data = JSON.parse(stdout || '{}'); - const conn = data.connections || {}; + let conn = {}; + try { + const stdout = await runPs1('network-stats.ps1'); + conn = JSON.parse(stdout || '{}').connections || {}; + } catch (e) { + logger.error('Failed to get connection counts', { error: e.message || String(e) }); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/NetworkMonitor.js` around lines 36 - 65, Update getStats so the runPs1('network-stats.ps1') call and its JSON parsing have a dedicated try/catch, preserving the previously computed interfaceStats when the script fails. Return zeroed connection counts for script errors while retaining the existing outer handling for failures from si.networkStats().
🟠 Major comments (23)
src/security/ClamAVEngine.linux.js-9-18 (1)
9-18: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winThe system ClamAV fallback never applies in either platform adapter.
ClamAVEngineBasefilters its candidate list with.filter(Boolean)and then calls_clamscanPath(dir)with a non-empty directory. Thebundled || systemPathidiom therefore always returns the bundled path, and both adapters resolve to a missing file inassets/clamavwhen no binary is bundled.init()then setsisReady = false, and an installed system ClamAV is never used.
src/security/ClamAVEngine.linux.js#L9-L18: test the bundled path withfs.existsSyncbefore you return it, and fall back to/usr/bin/clamscanand/usr/bin/freshclam.src/security/ClamAVEngine.macos.js#L9-L18: apply the same existence check, and fall back to/usr/local/binand/opt/homebrew/binfor Apple Silicon hosts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/ClamAVEngine.linux.js` around lines 9 - 18, The ClamAV path helpers always return non-existent bundled paths instead of using installed system binaries. In src/security/ClamAVEngine.linux.js lines 9-18, update _clamscanPath and _freshclamPath to return the bundled path only when fs.existsSync confirms it exists, otherwise fall back to /usr/bin/clamscan and /usr/bin/freshclam; apply the same existence checks in src/security/ClamAVEngine.macos.js lines 9-18, falling back to the macOS Intel and Apple Silicon paths under /usr/local/bin and /opt/homebrew/bin.src/security/ClamAVEngineBase.js-229-253 (1)
229-253: 🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy liftBound the scan output buffers.
outputkeeps every byte thatclamscanwrites.lineskeeps every non-empty line. A full-disk scan produces one line per file, so both grow without a limit. In addition,lines = lines.concat(...)allocates a new array on every chunk, which makes the accumulation quadratic.Push into
linesinstead of reallocating. Keep running counters forfileLines,foundLines,accessDeniedLines, andrealErrorLinesinstead of retaining all lines. Capoutputto a trailing window, because it is only used for error text.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/ClamAVEngineBase.js` around lines 229 - 253, Update the scan output handling around finish, handleOutput, and the stdout/stderr listeners to avoid unbounded memory growth: append non-empty lines with push rather than concat, replace retained line data with running fileLines, foundLines, accessDeniedLines, and realErrorLines counters, and keep output limited to a trailing error-text window. Preserve progress reporting and ensure the counters provide the same classifications currently derived from the complete lines collection.src/security/ClamAVEngineBase.js-90-164 (1)
90-164: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winAdd a timeout to the freshclam process.
updateDefinitionsreturns a promise that settles only oncloseorerror. Iffreshclamhangs on a slow mirror, the promise never settles.init()awaits this call, so service initialization stalls. TheConnectTimeoutandReceiveTimeoutvalues in the config file do not cover a wedged child process.Add a watchdog timer that kills the process and resolves with a failure result.
🛡️ Proposed fix
const finish = (result) => { + clearTimeout(watchdog); if (this.activeUpdateProcess === freshclam) this.activeUpdateProcess = null; resolve(result); }; + + const watchdog = setTimeout(() => { + try { freshclam.kill(); } catch (e) { + logger.debug('freshclam kill failed', { error: e?.message || String(e) }); + } + finish({ success: false, error: 'Definition update timed out', output }); + }, UPDATE_TIMEOUT_MS);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/ClamAVEngineBase.js` around lines 90 - 164, Update updateDefinitions to start a watchdog timer after spawning freshclam, killing the child process and resolving with a failure result when the timeout expires. Clear the timer whenever the process settles through close, error, or spawn failure, and ensure the timeout path cannot resolve more than once while preserving existing cancellation and output handling.src/security/NetworkMonitor.js-12-21 (1)
12-21: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winSet
maxBufferand guard the PowerShell call by platform.Two points:
execFileuses a defaultmaxBufferof 1 MB.network-connections.ps1returns one JSON object per connection. On a busy host the output can exceed 1 MB, and the call then fails withERR_CHILD_PROCESS_STDIO_MAXBUFFER.getConnectionsreturns an empty list, so the user sees no connections at all. RaisemaxBuffer.powershell.exedoes not exist on Linux and macOS. This PR adds cross-platform ClamAV adapters, soNetworkMonitoris now reachable on those platforms. Add an explicitprocess.platform !== 'win32'check and return a documented unsupported result instead of relying on a spawn failure.🛡️ Proposed fix
async function runPs1(scriptName) { + if (process.platform !== 'win32') { + throw new NotFoundError('PowerShell scripts are only supported on Windows.'); + } const scriptPath = path.join(PS_SCRIPTS_DIR, scriptName); if (!fs.existsSync(scriptPath)) { throw new NotFoundError(`PowerShell script not found: ${scriptPath}`); } const { stdout } = await execFilePromise('powershell.exe', [ '-NoProfile', '-NonInteractive', '-ExecutionPolicy', 'Bypass', '-File', scriptPath - ], { timeout: 15000, windowsHide: true }); + ], { timeout: 15000, windowsHide: true, maxBuffer: 32 * 1024 * 1024 }); return stdout; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/NetworkMonitor.js` around lines 12 - 21, Update runPs1 to return the documented unsupported result before invoking PowerShell when process.platform is not win32, and preserve the existing script-not-found validation on Windows. Increase the execFilePromise options’ maxBuffer above the default 1 MB while retaining the current timeout and Windows-specific settings, so large network-connections.ps1 output is handled successfully.src/security/FirewallManager.js-401-403 (1)
401-403: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftRestore the original rule when overwrite recreation fails.
importRulesdeletes the existing rule before it creates the replacement. This branch reports the failure but leaves the prior rule deleted. Preserve the exported rule configuration and restore it on replacement failure, or use a replacement flow that does not delete the old rule until the new rule is ready.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/FirewallManager.js` around lines 401 - 403, Update the importRules overwrite flow so a failed replacement in the rule recreation branch restores the original exported rule configuration before raising InvalidInputError. Preserve the existing error reporting, and ensure the prior rule is not left deleted when recreation fails.src/security/QuarantineManager.js-122-127 (1)
122-127: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winAudit failed integrity checks before returning.
This return bypasses the outer
catch, so a tampered or corrupt quarantine payload produces noACTIONS.QUARANTINE_RESTOREaudit entry. Log the failed restore with an integrity-specific result before returning.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/QuarantineManager.js` around lines 122 - 127, Update the _decrypt failure handling in the quarantine restore flow to record an ACTIONS.QUARANTINE_RESTORE audit entry with an integrity-specific failure result before returning the existing error response. Ensure corrupt or tampered payloads are audited without changing the user-facing failure message.src/security/QuarantineManager.js-27-33 (1)
27-33: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftUse a protected random key instead of observable machine metadata.
os.hostname(),os.userInfo().username, andPBKDF2_SALTare not secrets. An attacker who can read a quarantined file can derive the same AES key. Generate a random key and store it with an OS-protected credential mechanism.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/QuarantineManager.js` around lines 27 - 33, Replace the machine-metadata-derived key setup in the QuarantineManager constructor with a cryptographically random key, and persist or retrieve it through the platform’s OS-protected credential mechanism. Stop deriving _key from os.hostname(), os.userInfo().username, and PBKDF2_SALT; ensure existing quarantined data remains readable or is explicitly handled during key migration.src/security/EmergencyLockdown.js-9-9 (1)
9-9: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAllow valid interface names that contain spaces.
SAFE_INTERFACE_NAMErejects whitespace. Therefore,disableInterface('Ethernet 2')andenableInterface('Ethernet 2')always reject a valid interface name.execFileSyncalready passes the name as one argument. Normalize a string withtrim()and reject only an empty or non-string value.Also applies to: 117-121, 132-136
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/EmergencyLockdown.js` at line 9, Update SAFE_INTERFACE_NAME and the validation in disableInterface and enableInterface to allow internal spaces while still rejecting unsafe characters. Normalize string inputs with trim(), and reject non-string or empty normalized values before calling execFileSync.src/security/QuarantineManager.js-36-53 (1)
36-53: 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy liftKeep pre-upgrade quarantine records restorable.
Existing records contain XOR payloads.
_decryptnow interprets every payload as AES-GCM, so each old record fails authentication and cannot be restored. Add a format version or magic header, then retain legacy decryption or migrate existing records before enabling AES-GCM.Also applies to: 72-74, 122-127
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/QuarantineManager.js` around lines 36 - 53, Update _encrypt and _decrypt to include and recognize a format version or magic header, while preserving the existing XOR decryption path for records without the new marker. Ensure legacy quarantine records remain restorable and AES-GCM is used only for newly formatted records; apply the same format handling to the related record processing at the referenced call sites.browser-extension/package.json-9-10 (1)
9-10: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winGenerate the host manifest before automatic validation.
browser-extension/package.json#L10runsnode ../tools/validate-native-host.jsduring installation, butbrowser-extension/native-host-manifest.json#L7still containschrome-extension://__EXTENSION_ID_PLACEHOLDER__/. Run the extension-ID substitution before the installer package runs itspostinstall, or move validation to a release/CI step.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@browser-extension/package.json` around lines 9 - 10, Ensure the native-host manifest’s extension-ID substitution runs before the browser-extension package’s automatic postinstall validation. Update browser-extension/package.json around install:host and postinstall, and browser-extension/native-host-manifest.json at lines 6-7 as needed; alternatively remove automatic validation and invoke validate-native-host.js only from the release/CI flow, while preserving manifest validation after substitution.tools/validate-native-host.js-12-27 (1)
12-27: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject invalid entries inside
allowed_origins.
[""],[null], and[{}]pass the current checks and can causeOKeven though Chrome requires each entry to be a complete extension origin string. Validate every entry as a non-emptychrome-extension://.../extension origin and reject wildcards before printingOK.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/validate-native-host.js` around lines 12 - 27, Update the allowed_origins validation in the manifest validation flow to inspect every entry, rejecting empty, null, non-string, malformed, wildcard, or otherwise incomplete values unless they are complete chrome-extension://<extension-id>/ origins. Preserve the existing placeholder rejection and empty-list handling, and ensure invalid entries exit before the script prints OK.src/main/ipc/scan.js-7-10 (1)
7-10: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winMigrate persisted
customPathsettings.Existing custom schedules store
customPath.loadScheduleConfig()now suppliescustomPaths: [], so an enabled existing custom schedule stops without scanning.Map a legacy
customPathvalue to a one-itemcustomPathsarray when loading settings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/scan.js` around lines 7 - 10, Update loadScheduleConfig() to migrate persisted customPath values into customPaths as a single-item array when loading existing settings. Preserve the default empty customPaths value when no legacy customPath is present, and ensure enabled custom schedules continue scanning their configured path.src/main/ipc/scan.js-114-121 (1)
114-121: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winValidate schedule fields from
config.Passing
[config]makesvalidateArgsuse positional lookup. Only the firstconfigrule reads the payload. Theconfig.enabled,config.scanType,config.intervalHours, andconfig.customPathsrules read missing array indexes.First validate
configas an object. Then validate direct field names againstconfig. Also requirecustomPathselements to be non-empty strings.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/scan.js` around lines 114 - 121, Update the schedule validation before saveScheduleConfig so validateArgs first validates config as an object, then validates enabled, scanType, intervalHours, and customPaths using direct field names against config rather than positional lookup. Extend customPaths validation to require every element to be a non-empty string, while preserving the existing allowed values and numeric bounds.src/main/ipc/network.js-201-208 (1)
201-208: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass property schemas an object, not a positional array.
validateArgstreats arrays as positional arguments. Each handler therefore validates the entire payload object as its first property and rejects valid requests.
src/main/ipc/network.js#L201-L208: validatespecas an object, then validatelocalAddress,localPort,remoteAddress, andremotePortagainstspec.src/main/ipc/network.js#L215-L220: validateentryas an object, then validateipandreasonagainstentry.src/main/ipc/network.js#L239-L244: validateentryas an object, then validatedomainandreasonagainstentry.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/network.js` around lines 201 - 208, Update the three network IPC handlers in src/main/ipc/network.js at lines 201-208, 215-220, and 239-244 to pass property schemas as an object to validateArgs, not a positional array. Validate spec as an object before validating localAddress, localPort, remoteAddress, and remotePort against it; likewise validate entry as an object before validating ip/reason and domain/reason against it, preserving the existing measureConnectionBandwidth and handler flows.src/main/ipc/system.js-386-389 (1)
386-389: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winNormalize rejected external requests to
AppError.
requestTextrejects on request errors, timeouts, and oversized responses. Neither handler catches those rejections. Generic errors then bypass the structured IPC error contract.
src/main/ipc/system.js#L386-L389: catch a rejected HIBP request and throwAppErrorwith the original error ascause.src/main/ipc/system.js#L399-L402: catch a rejected XposedOrNot request and throwAppErrorwith the original error ascause.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/system.js` around lines 386 - 389, Wrap the rejected requestText call in the HIBP flow around the existing status check at src/main/ipc/system.js:386-389, converting request failures into AppError while preserving the original error as cause. Apply the same handling to the XposedOrNot request at src/main/ipc/system.js:399-402 so both external request paths satisfy the structured IPC error contract.src/main/ipc/system.js-553-555 (1)
553-555: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winReject malformed allowlist entries before apply time.
lockdown:setAllowlistonly validates that the payload is an array, whileEmergencyLockdown.setAllowlist()stores the top-level sections directly. Malformed entries stored through this path, including from persistedlockdown_allowlist, can bypass the single-entry validation used bylockdown:addToAllowlist. Reject entries that are not strings or that contain command/control characters before saving them.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/system.js` around lines 553 - 555, Update the lockdown:setAllowlist validation around validateArgs so every allowlist entry is validated as a safe string before EmergencyLockdown.setAllowlist stores it. Reject non-string entries and strings containing command/control characters, including entries loaded from persisted lockdown_allowlist, while preserving the existing array size constraints and addToAllowlist validation behavior.src/main/ipc/system.js-143-156 (1)
143-156: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winPass object payloads directly to
validateArgs.Both handlers wrap an object in a one-element array.
validateArgsthen treats the object as the first positional value.audit:logalways rejectsentryas a non-stringaction.alerts:listalways rejectsoptionsas a non-numberlimit.
src/main/ipc/system.js#L143-L156: passentrydirectly tovalidateArgs.src/main/ipc/system.js#L158-L164: passoptionsdirectly tovalidateArgs.Proposed fix
- ], [entry]); + ], entry); @@ - ], [options]); + ], options);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/system.js` around lines 143 - 156, Update the validateArgs calls in the audit:log handler at src/main/ipc/system.js lines 143-156 and the alerts:list handler at lines 158-164 to pass entry and options directly, respectively, rather than wrapping them in one-element arrays. Preserve the existing validation schemas and handler behavior.src/main/lifecycle.js-485-494 (1)
485-494: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winRemove one of the two prune timers.
This interval prunes
network_statsandmaintenance_runsevery hour.src/main/main.jslines 140-149 create a second interval with the same period and the same two calls. Both run for the lifetime of the process, so pruning happens twice per hour.Keep the timer in one module only.
main.jsalso assignsservices._pruneTimer, which overwrites the handle stored here, so the timer created in this file can never be cleared.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/lifecycle.js` around lines 485 - 494, Remove the duplicate prune interval created in the lifecycle initialization around pruneTimer, including its unref and services._pruneTimer assignment, and retain the equivalent timer managed by main.js. Ensure only one hourly timer invokes db.pruneNetworkStats and db.pruneMaintenanceRuns.src/main/lifecycle.js-412-448 (1)
412-448: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy liftValidate
startup:toggleinput before touching the registry or the filesystem.The handler trusts the renderer-supplied
itemobject:
- Line 418 writes an arbitrary value name and arbitrary command data into the
HKLMorHKCURunkey.item.commandis never checked, so any string becomes a persistence entry.- Line 439 builds
path.join(disabledDir, item.name). A..sequence initem.nameescapesdisabledDir.- Line 440 renames
item.path, which is an unconstrained absolute path. The renderer can move any file the process can access.- Line 432 renames a backup onto
item.pathwith the same problem.
execFileSyncwith an argument array prevents shell injection, but it does not prevent unauthorized registry writes. Validateitem.nameagainst the entries returned by the startup-items enumeration, and confirm that bothitem.pathand the destination resolve inside the expected startup directories.🛡️ Proposed guards
ipcMain.handle('startup:toggle', async (_event, item, enable) => { try { + if (!item || typeof item !== 'object') return { ok: false, error: 'Invalid item' }; + if (typeof item.name !== 'string' || !item.name || item.name !== path.basename(item.name)) { + return { ok: false, error: 'Invalid item name' }; + } if (item.source === 'registry') {} else if (item.source === 'startup-folder') { const appData = process.env.APPDATA || ''; const programData = process.env.ProgramData || ''; const userStartup = path.join(appData, 'Microsoft', 'Windows', 'Start Menu', 'Programs', 'Startup'); const allStartup = path.join(programData, 'Microsoft', 'Windows', 'Start Menu', 'Programs', 'Startup'); const startupDir = item.scope === 'user' ? userStartup : allStartup; + const target = path.resolve(String(item.path || '')); + if (path.dirname(target) !== path.resolve(startupDir)) { + return { ok: false, error: 'Item is outside the startup folder' }; + }Replace the later uses of
item.pathwithtarget.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/lifecycle.js` around lines 412 - 448, Validate the renderer-supplied item in the startup:toggle handler before any registry or filesystem operation: require item.name to match an entry from the startup-items enumeration, validate item.command against that trusted entry, and resolve both item.path and the computed destination/backup paths so they remain within the expected user or all-users startup directory without traversal. Apply these checks in both registry and startup-folder branches, and replace subsequent item.path uses with the validated target path.Source: Linters/SAST tools
src/main/lifecycle.js-310-337 (1)
310-337: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
services.splashWindowis never assigned, so every splash progress update is discarded.
serviceRegistry.createdoes not returnsplashWindow, andstartnever sets it.windowManager.sendSplashProgressreceivesundefinedat lines 310, 319, 323, and 337, and returns immediately. Thesplash:progressIPC handler at lines 464-468 has the same problem.main.jsline 128 confirms the value is undefined.The splash window is created in
main.jsthroughwindowManager.createSplashWindow. Pass that reference intostart, or read it fromwindowManager.🐛 Proposed fix: accept the splash window through options
async function start(db, eventBus, options = {}) { - const { userDataPath, notify } = options; + const { userDataPath, notify, splashWindow } = options; const locale = getLocale(db, options.startupLocale || 'en');const services = wireServices(db, eventBus, { userDataPath, locale, notify: (title, body, level) => notify(title, body, level), }); services.notify = notify; + services.splashWindow = splashWindow || null;Then in
src/main/main.js:const services = await lifecycle.start(db, eventBus, { userDataPath: app.getPath('userData'), startupLocale, + splashWindow: windowManager.splashWindow ?? undefined, notify: (title, body, level) => windowManager.showNotification(lifecycle.t(title), lifecycle.t(body), level), });The
main.jsside depends onwindowManagerexposingsplashWindow; see the consolidated comment about window-manager module state.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/lifecycle.js` around lines 310 - 337, Update the lifecycle start flow and its caller so the splash window created by windowManager.createSplashWindow is passed into start and assigned to services.splashWindow before any sendSplashProgress calls. Ensure the existing splash:progress IPC handler also uses this initialized reference, preserving all current progress updates.src/main/ipc/validate.js-58-73 (1)
58-73: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
min/maxare not enforced for string arguments.The docstring (Line 19) lists
minandmaxas generic constraints, but the implementation only appliesrule.min/rule.maxinside thenumberbranch (Lines 80-89). Thestringbranch here (Lines 58-73) checks onlypatternandallowed, neverrule.min/rule.max.The referenced
src/main/ipc/firewall.jssnippet uses{ name: 'name', type: 'string', required: true, max: 256 }forfirewall:deleteRule, expecting a 256-character cap on the rule name. That cap is silently never enforced, so an arbitrarily long string passes validation.🛠️ Proposed fix
if (type === 'string') { const str = String(value); if (str.length === 0) { throw new InvalidInputError(`Argument "${name}" must be a non-empty string.`); } + if (rule.min != null && str.length < rule.min) { + throw new InvalidInputError(`Argument "${name}" must be at least ${rule.min} character(s).`); + } + if (rule.max != null && str.length > rule.max) { + throw new InvalidInputError(`Argument "${name}" must be at most ${rule.max} character(s).`); + } if (rule.pattern && !rule.pattern.test(str)) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/validate.js` around lines 58 - 73, Update the string-validation branch in the argument validator to enforce rule.min and rule.max against the string’s length, alongside the existing empty-string, pattern, and allowed-value checks. Preserve the current InvalidInputError behavior and messages for other string constraints, and ensure configurations such as firewall:deleteRule’s max: 256 reject longer names.src/core/database.js-517-528 (1)
517-528: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winKeep
detail/resultformatting consistent inaddAuditEntry.
auditLog.log()stores JSON strings, butaudit:logstores the raw IPC strings. Move the JSON-encoding fallback intoaddAuditEntryor handle it on every caller so existing JSON-encoded audits are preserved and future callers use the same format.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/database.js` around lines 517 - 528, Update addAuditEntry to normalize detail and result before inserting them into audit_log: preserve values that are already JSON-encoded while JSON-encoding non-encoded values, matching auditLog.log() formatting. Ensure every caller, including audit:log, stores the same consistent representation.src/main/ipc/validate.js-24-49 (1)
24-49: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winFix the required-array validation modes to avoid false failures.
validateArgscurrently wraps non-array input in a one-element object and then treats every one-element array as a valid positional args list. This rejects named-field schemas that need a required optional argument unless callers pass an explicit[], and it still allows multi-rule positional schemas with fewer runtime arguments than rules. Use separate supported call patterns explicitly, such as single-field positional values ([{ name: 'id', ... }], id) and named fields ([{ ... }], { id }), and avoid implicit unwrapping.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/validate.js` around lines 24 - 49, Update validateArgs to distinguish supported argument modes explicitly: treat an array as positional arguments, an object as named arguments, and allow a scalar only for a single-rule positional schema. Do not spread or implicitly unwrap non-array inputs; resolve the single positional value directly, while preserving required checks so positional arrays with fewer values than schema rules fail appropriately.
🟡 Minor comments (12)
tools/validate-native-host.js-29-33 (1)
29-33: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winRequire
manifest.pathto reference a regular file.
fs.existsSync(hostScript)only checks existence. A directory namednative-host.batwould pass validation, but Chrome’s native messagingpathmust resolve to an executable file. Validatemanifest.pathas a string, then callfs.statSync(hostScript).isFile()before loggingOK.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tools/validate-native-host.js` around lines 29 - 33, Update the validation around manifest.path and hostScript to first ensure manifest.path is a string, then use fs.statSync(hostScript).isFile() rather than fs.existsSync alone; reject missing paths, directories, and other non-file targets before reporting validation success.src/main/ipc/firewall.js-43-46 (1)
43-46: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winEnforce the rule-name length limit.
validateArgsonly appliesmaxtonumbervalues. Themax: 256rule does not limitname.Add string-length support to
validateArgs, then use it for this handler.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/firewall.js` around lines 43 - 46, The validateArgs implementation must enforce max for string values by validating their length, not only numeric magnitude. Update validateArgs accordingly, then retain the max: 256 constraint in the firewall:deleteRule handler’s name argument so rule names are limited to 256 characters.src/main/ipc/scan.js-61-63 (1)
61-63: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate every custom scan path.
The schema validates only the array size. Arrays containing empty strings or non-string values reach
scanEngine.runCustomScan().Reject invalid path elements before starting the scan.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/scan.js` around lines 61 - 63, Update the validation around validateArgs in the custom scan flow to validate each targetPaths element as a non-empty string, not just the array length. Reject arrays containing empty strings or non-string values before invoking scanEngine.runCustomScan().src/main/ipc/firewall.js-105-105 (1)
105-105: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winUse a correct IP-address parser.
isValidIp()accepts invalid IPv4 values such as999.999.999.999and malformed IPv6 strings such as:::. Use a parser that checks IPv4 octet ranges and IPv6 structure before adding trusted addresses or sending WHOIS requests.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/firewall.js` at line 105, Replace the isValidIp checks at src/main/ipc/firewall.js lines 105 and 120 with a correct IP parser that enforces IPv4 octet ranges and valid IPv6 structure before trusted-address insertion or WHOIS requests; preserve the existing InvalidInputError behavior for rejected addresses.src/core/workerManager.js-34-40 (1)
34-40: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winHandle rejected
Worker.terminate()promises.
Worker.terminate()returns a Promise, andtry/catchdoes not catch its rejections. Add.catch(...)to each termination call so errors such asERR_WORKER_NOT_RUNNINGare logged and do not become unhandled:
src/core/workerManager.js#L34-L40: timeout and abort paths.src/core/workerManager.js#L83-L83: explicit cancel path.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/workerManager.js` around lines 34 - 40, The worker termination calls in the timeout and abort paths of src/core/workerManager.js lines 34-40, and the explicit cancel path at src/core/workerManager.js line 83, must handle rejected Worker.terminate() promises. Add rejection handling to each call so termination errors are logged through the existing logger.debug pattern and never become unhandled; retain the existing synchronous try/catch handling.src/preload/preload.js-7-100 (1)
7-100: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winKeep renderer IPC callers aligned with the preload allowlists.
src/preload/preload.jsnow allows only specific channels, but renderer code still calls unallowed IPC channels. Add the missingwindow.api.invokechannels (firewall:trustConnection,firewall:untrustConnection,folderwatch:toggle,network:whois,rtp:status,rtp:toggle,scan:custom,splash:progress) andwindow.api.onchannel (tray:summary) to the allowlists, or remove the corresponding UI paths before release.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/preload/preload.js` around lines 7 - 100, Update the ALLOWED_INVOKE and ALLOWED_ON sets in preload.js to include the renderer-used channels: firewall:trustConnection, firewall:untrustConnection, folderwatch:toggle, network:whois, rtp:status, rtp:toggle, and scan:custom in ALLOWED_INVOKE, plus splash:progress and tray:summary in the appropriate allowlist based on their window.api usage. Keep the existing allowlist structure intact.src/main/ipc/system.js-598-616 (1)
598-616: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick winEnforce argument validation before delegating to plugins.
tools:runforwards renderer input totoolRegistry.run, but the registry only checks for registered and implemented tools before callingtool.run(args || {}, ctx || {}). Add entry-point validation fromsrc/main/ipc/validate.jsor require each registered tool to define/apply its input schema before filesystem, command, or DB use.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/system.js` around lines 598 - 616, Update the tools:run IPC handler to validate the renderer-provided toolId and args using the existing validation utilities from validate.js before calling toolRegistry.run. Ensure invalid input returns the established failure response and never reaches plugin execution, while preserving the existing registry delegation for validated requests.src/main/maintenanceScheduler.js-162-162 (1)
162-162: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winScheduled maintenance runs are recorded as user-initiated.
The fifth argument of
logisuserInitiated. It is hardcoded totrue.runIfDuecallsrunNow({ dryRunCleanup: true })withoutmanual, so timer-driven runs also produceuserInitiated: 1. The audit log then cannot separate operator actions from scheduled ones.Line 201 already uses
options.manualfor the same distinction. Reuse it.🐛 Proposed fix
- log(this.db, ACTIONS.MAINTENANCE_RUN, { scriptIds: config.scriptIds, dryRunCleanup }, { startedAt }, true); + log(this.db, ACTIONS.MAINTENANCE_RUN, { scriptIds: config.scriptIds, dryRunCleanup }, { startedAt }, !!options.manual);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/maintenanceScheduler.js` at line 162, Update the maintenance audit call in runNow to pass options.manual as the log function’s userInitiated argument instead of hardcoding true, matching the existing behavior at line 201 and preserving the distinction between scheduled and manually triggered runs.src/main/lifecycle.js-316-317 (1)
316-317: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
buildAppMenuruns before the main window exists.Line 316 passes
services.mainWindow, which is still undefined at that point.createWindow()runs on line 317.buildAppMenuuses the argument as the parent of the About dialog, so the dialog is not window-modal.Move the call after
createWindow().🐛 Proposed fix
- windowManager.buildAppMenu(services.mainWindow); const { mainWindow, splashTimeoutId } = windowManager.createWindow(); services.mainWindow = mainWindow; + windowManager.buildAppMenu(mainWindow); windowManager.sendSplashProgress(services.splashWindow, 9, t('splash.buildingInterface'));🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/lifecycle.js` around lines 316 - 317, Move the windowManager.buildAppMenu call to after windowManager.createWindow() in the lifecycle initialization flow, passing the newly created mainWindow so the About dialog uses the valid parent window.src/main/ipc/_shared.js-16-27 (1)
16-27: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winStop appending chunks once the request is destroyed.
req.destroy()does not guarantee the response stream stops emittingdataevents right away. Until the socket fully tears down,body += chunkkeeps running, sobodycan grow pastMAX_API_BODY_BYTESbefore the connection actually closes. Add a guard so the handler stops concatenating oncereq.destroyedis true.🛠️ Proposed fix
res.on('data', chunk => { + if (req.destroyed) return; body += chunk; if (Buffer.byteLength(body) > MAX_API_BODY_BYTES) { req.destroy(new Error('Response body exceeds size limit')); reject(new Error('Response too large')); } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/ipc/_shared.js` around lines 16 - 27, Update the response `data` handler to check `req.destroyed` before appending each chunk, returning immediately when the request has been destroyed. Preserve the existing size-limit rejection and `req.destroy()` behavior for the first oversized chunk.src/core/database.js-550-556 (1)
550-556: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winHandle duplicate-domain inserts gracefully.
user_domain_blocklist.domainhas aUNIQUEconstraint (Line 181).addUserDomainBlocklistEntryruns a plainINSERTwith no conflict handling, unlikesetBlocklistCacheandsetGeoCachein this same file, which useON CONFLICT ... DO UPDATE. Adding a domain that is already blocklisted throws an unhandledSQLITE_CONSTRAINTerror up to the caller instead of a clear outcome.🛠️ Proposed fix
addUserDomainBlocklistEntry(entry) { const stmt = this.db.prepare(` - INSERT INTO user_domain_blocklist (domain, reason) VALUES (`@domain`, `@reason`) + INSERT INTO user_domain_blocklist (domain, reason) VALUES (`@domain`, `@reason`) + ON CONFLICT(domain) DO UPDATE SET reason = excluded.reason `); return stmt.run({ domain: entry.domain, reason: entry.reason || null }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/database.js` around lines 550 - 556, Update addUserDomainBlocklistEntry to handle the unique domain conflict instead of allowing SQLITE_CONSTRAINT to escape. Follow the existing conflict-handling pattern used by setBlocklistCache and setGeoCache, preserving the entry’s domain and updating its reason when the domain already exists, while returning the statement result.src/core/database.js-164-185 (1)
164-185: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winPrevent duplicate IPs from adding to
user_blocklist.
user_blocklist.iphas noUNIQUEconstraint, whileuser_domain_blocklist.domaindoes.db.addUserBlocklistEntry()and the only tracked caller onnetwork:userBlocklist:addboth insert without checking for an existingip. UseINSERT OR IGNORE/UPDATE, or enforce uniqueness at the database level.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/database.js` around lines 164 - 185, The user_blocklist schema allows duplicate IP entries, unlike user_domain_blocklist. Update the user_blocklist definition in the database initialization to enforce uniqueness for ip, and adjust db.addUserBlocklistEntry or its network:userBlocklist:add caller as needed to preserve the intended insert-or-ignore/update behavior for existing IPs.
🧹 Nitpick comments (6)
src/security/ClamAVEngine.linux.js (1)
4-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the pass-through constructor.
The constructor only calls
super(options). Class fields and defaults are unchanged. Delete it and let the base constructor apply. The same applies toClamAVEngine.macos.jsandClamAVEngine.js.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/ClamAVEngine.linux.js` around lines 4 - 7, Remove the pass-through constructor from ClamAVEngineLinux and the corresponding classes in ClamAVEngine.macos.js and ClamAVEngine.js, allowing each class to inherit its base constructor while preserving existing class fields and defaults.src/security/BlocklistService.js (1)
178-196: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winTrack the byte count incrementally and stop accumulating after the limit.
Two points in the size guard:
Buffer.byteLength(data)rescans the whole accumulated string on every chunk. For a body near 10 MB this is quadratic work.- After
req.destroy(...)andreject(...), the handler does not return. Furtherdataevents keep appending todataand callrejectagain.Track a running byte counter and return after the limit is reached. Also set the response encoding so that multi-byte characters are not split across chunk boundaries.
♻️ Proposed refactor
return new Promise((resolve, reject) => { const req = https.get(source.url, { headers: { 'User-Agent': 'Soterios' } }, (res) => { let data = ''; + let bytes = 0; + let aborted = false; + res.setEncoding('utf8'); res.on('data', chunk => { + if (aborted) return; + bytes += Buffer.byteLength(chunk, 'utf8'); + if (bytes > MAX_BLOCKLIST_BODY_BYTES) { + aborted = true; + req.destroy(new Error('Blocklist response exceeds size limit')); + reject(new Error('Blocklist response too large')); + return; + } data += chunk; - if (Buffer.byteLength(data) > MAX_BLOCKLIST_BODY_BYTES) { - req.destroy(new Error('Blocklist response exceeds size limit')); - reject(new Error('Blocklist response too large')); - } });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/BlocklistService.js` around lines 178 - 196, Update the response handling in the blocklist fetch method to track accumulated bytes incrementally instead of recalculating Buffer.byteLength(data) for every chunk. Set the response encoding to preserve multi-byte characters across chunks, and when the running total exceeds MAX_BLOCKLIST_BODY_BYTES, destroy the request, reject once, and immediately return from the data handler so no further chunks are appended or rejected.src/security/GeoLocationService.js (1)
1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winMove
requestTextout ofsrc/main/ipc/_shared.
requestTexthas the expected{ statusCode, body }shape, 15s timeout, and 1MB body limit. Keep that, but avoid tyingGeoLocationServiceto the Electron main IPC module path; put the shared HTTP helper insrc/utilsinstead.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/security/GeoLocationService.js` at line 1, Move the shared `requestText` helper out of `src/main/ipc/_shared` into an appropriate module under `src/utils`, preserving its `{ statusCode, body }` result shape, 15-second timeout, and 1MB response-body limit. Update `GeoLocationService` and any other consumers to import the helper from its new utility location, removing the dependency on the Electron main IPC module.src/utils/templates.js (1)
3-16: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winDocument that callers must escape values before rendering.
renderTemplateperforms plain string substitution with no HTML escaping. Current callers (toastHtmlandrenderScanReportHtml, per the referenced snippets) escape values before calling this function. Add this contract to the JSDoc so future callers do not accidentally pass unescaped, untrusted content into a rendered template.♻️ Proposed fix
/** * Render a template file by replacing {{KEY}} placeholders with values. * + * This function does not escape `data` values. Callers must HTML-escape + * any value that originates from untrusted or user-controlled input + * before passing it here. + * * `@param` {string} filePath - Absolute path to the template file.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/utils/templates.js` around lines 3 - 16, Update the JSDoc for renderTemplate to state that it performs plain substitution without HTML escaping and that callers must escape values before passing untrusted content. Keep the implementation and existing parameter documentation unchanged.src/core/auditLog.js (1)
17-29: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winLog audit-write failures instead of swallowing them silently.
The
catchblock discards the error with no diagnostic output. Ifdb.addAuditEntryfails repeatedly (full disk, locked database), the audit trail silently stops growing and nobody notices. Keep the "never break the primary action" behavior, but log the failure.♻️ Proposed fix
} catch (_) { - // Audit logging must never break the primary action. + // Audit logging must never break the primary action, but a silent + // failure would hide gaps in the audit trail, so log for diagnosis. + console.error('[auditLog] Failed to write audit entry:', _ && _.message); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/auditLog.js` around lines 17 - 29, Update the catch block in log to record the db.addAuditEntry failure through the project’s established logging mechanism, including the caught error for diagnostics, while still suppressing propagation so audit logging never breaks the primary action.src/core/database.js (1)
187-197: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winAdd an index on
audit_log.timestamp.
getAuditLog(Line 527) always orders bytimestamp DESC. The table has no index ontimestamp, unlikenetwork_stats, which hasidx_network_stats_recorded_at. Add a matching index so audit-log queries stay fast as the table grows.♻️ Proposed fix
this.db.exec(` CREATE TABLE IF NOT EXISTS audit_log ( id INTEGER PRIMARY KEY AUTOINCREMENT, timestamp DATETIME DEFAULT CURRENT_TIMESTAMP, action TEXT NOT NULL, detail TEXT, result TEXT, user_initiated INTEGER DEFAULT 0 ) `); + this.db.exec(` + CREATE INDEX IF NOT EXISTS idx_audit_log_timestamp ON audit_log(timestamp) + `);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/database.js` around lines 187 - 197, Update the audit-log schema setup near the audit_log table creation to create an index on audit_log.timestamp, matching the existing index pattern used for network_stats.recorded_at. Ensure getAuditLog’s timestamp-descending ordering can use this index without changing the table schema or query behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0d05f249-498e-4ed0-b407-7757b47b7f7c
📒 Files selected for processing (57)
browser-extension/native-host-manifest.jsonbrowser-extension/package.jsonpackage.jsonsrc/core/auditLog.jssrc/core/database.jssrc/core/eventBus.jssrc/core/workerManager.jssrc/main/healthSummary.jssrc/main/ipc/_shared.jssrc/main/ipc/firewall.jssrc/main/ipc/network.jssrc/main/ipc/process.jssrc/main/ipc/quarantine.jssrc/main/ipc/scan.jssrc/main/ipc/system.jssrc/main/ipc/validate.jssrc/main/ipcHandlers.jssrc/main/lifecycle.jssrc/main/main.jssrc/main/maintenanceScheduler.jssrc/main/serviceRegistry.jssrc/main/trayDashboard.jssrc/main/updater.jssrc/main/windowManager.jssrc/preload/preload.jssrc/preload/toastPreload.jssrc/scripts/childRunner.jssrc/scripts/safeScripts/browserCacheReport.jssrc/scripts/safeScripts/largeFilesReport.jssrc/scripts/safeScripts/listStartupItems.jssrc/scripts/safeScripts/uninstallLaunchUtils.jssrc/security/BlocklistService.jssrc/security/ClamAVEngine.jssrc/security/ClamAVEngine.linux.jssrc/security/ClamAVEngine.macos.jssrc/security/ClamAVEngineBase.jssrc/security/EmergencyLockdown.jssrc/security/FirewallManager.jssrc/security/FolderWatcher.jssrc/security/GeoLocationService.jssrc/security/NetworkMonitor.jssrc/security/ProcessInspector.jssrc/security/QuarantineManager.jssrc/security/ScanEngine.jssrc/security/reportExport.jssrc/security/scripts/network-connections.ps1src/security/scripts/network-stats.ps1src/ui/js/api.jssrc/ui/js/pages/network.jssrc/ui/js/pages/settings.jssrc/ui/js/router.jssrc/ui/templates/scan-report.htmlsrc/ui/templates/toast.htmlsrc/utils/errors.jssrc/utils/templates.jstests/baseline-coverage.txttools/validate-native-host.js
| // --- Incremental Scan Cache --- | ||
| recordScannedFile({ path, size, modifiedAt }) { | ||
| const stmt = this.db.prepare(` | ||
| INSERT INTO scanned_files (path, size, modified_at) | ||
| VALUES (@path, @size, @modifiedAt) | ||
| ON CONFLICT(path) DO UPDATE SET | ||
| size = excluded.size, | ||
| modified_at = excluded.modified_at, | ||
| last_scanned_at = CURRENT_TIMESTAMP | ||
| `); | ||
| return stmt.run({ path, size: size || null, modifiedAt: modifiedAt || null }); | ||
| } | ||
|
|
||
| getFilesToSkip(paths) { | ||
| if (!paths || !paths.length) return new Set(); | ||
| const placeholders = paths.map(() => '?').join(','); | ||
| const rows = this.db.prepare(` | ||
| SELECT path, modified_at FROM scanned_files WHERE path IN (${placeholders}) | ||
| `).all(...paths); | ||
| const skip = new Set(); | ||
| for (const row of rows) { | ||
| skip.add(row.path); | ||
| } | ||
| return skip; | ||
| } | ||
|
|
||
| pruneScannedFiles(olderThanDays = 30) { | ||
| const cutoff = new Date(Date.now() - Number(olderThanDays) * 86400 * 1000).toISOString(); | ||
| return this.db.prepare('DELETE FROM scanned_files WHERE last_scanned_at < ?').run(cutoff); | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Find the caller(s) of getFilesToSkip to confirm the current
# call shape and plan the coordinated fix.
rg -n -C8 'getFilesToSkip' --type=jsRepository: chrisriv10/Soterios
Length of output: 157
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Tracked files matching database/scan names:"
git ls-files | grep -Ei '(^|/)(database|ScanEngine|scanner|scan).*\.js$|src/core/database\.js' || true
echo
echo "Search getFilesToSkip and scanned_files occurrences:"
rg -n -C4 'getFilesToSkip|scanned_files|recordScannedFile|pruneScannedFiles' . || true
echo
echo "Locate src/core/database.js:"
fd -a 'database\.js$' . || trueRepository: chrisriv10/Soterios
Length of output: 5090
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "src/security/ScanEngine.js relevant sections:"
sed -n '1,260p' src/security/ScanEngine.js | cat -n
echo
echo "src/core/database.js schema and methods relevant sections:"
sed -n '180,215p;580,620p' src/core/database.js | cat -nRepository: chrisriv10/Soterios
Length of output: 14867
Compare cached metadata before skipping incremental paths.
getFilesToSkip only checks whether paths exist in scanned_files; it selects modified_at but never compares it against the current file state. ScanEngine.runScan passes bare paths to this check, while recordScannedFile stores size and modifiedAt after scanning. To avoid skipping changed or injected files, update the API/callers to pass per-file size and modifiedAt, then skip only rows where both stale values still match.
🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 604-606: Avoid SQL injections
Context: SELECT path, modified_at FROM scanned_files WHERE path IN (${placeholders})
Note: [CWE-89] Improper Neutralization of Special Elements used in an SQL Command ('SQL Injection'). Security best practice.
(variable-sql-statement-injection)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/core/database.js` around lines 589 - 618, Update getFilesToSkip and its
ScanEngine.runScan callers to accept per-file metadata objects containing path,
size, and modifiedAt instead of bare paths. Query and compare each cached row’s
size and modified_at, and add a path to the skip set only when both current
values match the cached values; preserve empty-input behavior and ensure
recordScannedFile’s stored metadata is used for comparison.
| const services = lifecycle.start(db, eventBus, { | ||
| userDataPath: app.getPath('userData'), | ||
| startupLocale, | ||
| notify: (title, body, level) => windowManager.showNotification(lifecycle.t(title), lifecycle.t(body), level), | ||
| }); | ||
|
|
||
| // loadPlugins() is a synchronous filesystem scan, not a network call, so | ||
| // it's cheap enough to keep here rather than deferring it. | ||
| loadPlugins(); | ||
| sendSplashProgress(6, t('splash.loadingEngines')); | ||
|
|
||
| // Show the window as soon as possible instead of waiting on ClamAV/RTP | ||
| // initialization below -- those can take a while (definitions download, | ||
| // spawning PowerShell) and previously blocked the window from appearing | ||
| // at all until they finished. | ||
| buildAppMenu(); | ||
| createWindow(); | ||
| sendSplashProgress(9, t('splash.buildingInterface')); | ||
|
|
||
| // Register IPC handlers only once mainWindow actually exists. Previously | ||
| // this ran before createWindow(), so the mainWindow parameter passed in | ||
| // was always undefined (a plain variable copied by value at call time) -- | ||
| // handlers like dialog:pickFolder/pickFiles silently fell back to | ||
| // BrowserWindow.getFocusedWindow() instead of targeting the real window. | ||
| registerIpcHandlers(mainWindow, services); | ||
| sendSplashProgress(12, t('splash.registeringServices')); | ||
|
|
||
| try { | ||
| lifecycleRefs.trayController = initTrayDashboard({ | ||
| app, | ||
| mainWindow, | ||
| getSummary: () => getTrayHealthSummary(db, toolRegistry) | ||
| }); | ||
| services.trayController = lifecycleRefs.trayController; | ||
|
|
||
| mainWindow.on('close', (event) => { | ||
| if (!isQuitting && lifecycleRefs.trayController?.tray) { | ||
| event.preventDefault(); | ||
| mainWindow.hide(); | ||
| } | ||
| }); | ||
| } catch (err) { | ||
| logLine('warn', 'Tray dashboard unavailable', { error: err.message }); | ||
| } | ||
|
|
||
| setTimeout(() => { | ||
| if (featureFlags.getFlag(db, 'autoUpdates', true)) { | ||
| updater.checkForUpdates().catch(() => {}); | ||
| } | ||
| }, 30_000); | ||
|
|
||
| // Module-level tracking for announced progress milestones to prevent duplicate notifications | ||
| let announcedProgress = new Set(); | ||
| let progressListenersRegistered = false; | ||
|
|
||
| function registerScanProgressListeners() { | ||
| if (progressListenersRegistered) return; | ||
| progressListenersRegistered = true; | ||
|
|
||
| const resolveScanType = (data) => data?.scanType || data?.report?.scanType || null; | ||
| const isBackgroundScan = (scanType) => scanType === 'folderwatch'; | ||
|
|
||
| eventBus.on('scan:progress', (data) => { | ||
| const scanType = resolveScanType(data); | ||
| // Don't forward folder watch progress to UI to prevent interference | ||
| if (!isBackgroundScan(scanType) && mainWindow && !mainWindow.isDestroyed()) { | ||
| mainWindow.webContents.send('scan:progress', data); | ||
| } | ||
| if (!data || typeof data.pct !== 'number') return; | ||
| if (dbRef && !featureFlags.getFlag(dbRef, 'scanNotifications', true)) return; | ||
| // Explicitly filter out folder watch, definitions, and custom scans from notifications | ||
| if (scanType === 'definitions' || isBackgroundScan(scanType) || scanType === 'custom') return; | ||
| const milestone = [0, 25, 50, 75].find((value) => data.pct >= value && !announcedProgress.has(value)); | ||
| if (milestone !== undefined) { | ||
| announcedProgress.add(milestone); | ||
| const files = data.filesScanned || 0; | ||
| showNotification(t('toast.scanProgressTitle'), t('scan.progress', { files, pct: data.pct }), 'info'); | ||
| } | ||
| }); | ||
|
|
||
| // Forward scan complete events to renderer | ||
| eventBus.on('scan:complete', (data) => { | ||
| const scanType = resolveScanType(data); | ||
| // Clear announced progress milestones when scan completes | ||
| announcedProgress.clear(); | ||
| if (!isBackgroundScan(scanType) && mainWindow && !mainWindow.isDestroyed()) { | ||
| mainWindow.webContents.send('scan:complete', data); | ||
| } | ||
| if (isBackgroundScan(scanType) || scanType === 'custom') return; | ||
|
|
||
| let label; | ||
| let body; | ||
| let level; | ||
| if (data.scanType === 'definitions') { | ||
| if (data.status === 'completed') { | ||
| label = t('toast.signaturesUpdated'); | ||
| body = t('toast.definitionsUpdatedDetail'); | ||
| level = 'success'; | ||
| } else if (data.status === 'canceled') { | ||
| label = t('toast.definitionsUpdateCanceled'); | ||
| body = t('toast.definitionsUpdateCanceledDetail'); | ||
| level = 'warn'; | ||
| } else { | ||
| label = t('toast.definitionsUpdateFailed'); | ||
| body = data.error || t('toast.definitionsUpdateFailedDetail'); | ||
| level = 'danger'; | ||
| } | ||
| } else { | ||
| // Only show notification if not canceled | ||
| if (data.status === 'canceled') { | ||
| label = t('toast.scanCanceled'); | ||
| body = t('toast.scanCanceledDetail', { count: data.filesScanned || 0 }); | ||
| level = 'warn'; | ||
| } else { | ||
| label = data.status === 'completed' ? t('toast.scanCompleted') : t('toast.scanFinishedWithIssues'); | ||
| body = t('toast.scanSummary', { files: data.filesScanned || 0, threats: data.threatsFound || 0 }); | ||
| level = data.status !== 'completed' ? 'warn' : (data.threatsFound ? 'danger' : 'success'); | ||
| } | ||
| } | ||
| const iconOverride = (data.threatsFound && data.threatsFound > 0) ? TOAST_ICONS.threat : null; | ||
| showNotification(label, body, level, iconOverride); | ||
| // Auto-generate a scan report | ||
| (async () => { | ||
| try { | ||
| if (!featureFlags.getFlag(db, 'autoReports', true)) return; | ||
| const isCanceled = data.status === 'canceled' || data.report?.status === 'canceled'; | ||
| if (isCanceled || (scanType !== 'quick' && scanType !== 'full')) return; | ||
| logLine('info', 'Generating scan report...'); | ||
|
|
||
| const result = await toolRegistry.run('generate-security-report', { version: app.getVersion() }, { toolRegistry, db, log: logLine }); | ||
| logLine('info', 'Scan report ' + (result.ok ? 'generated' : 'failed: ' + (result.error || 'unknown'))); | ||
| } catch (err) { | ||
| logLine('error', 'Auto-report generation threw: ' + (err.message || err)); | ||
| } | ||
| })(); | ||
| }); | ||
| } | ||
|
|
||
| // Register the scan progress listeners | ||
| registerScanProgressListeners(); | ||
|
|
||
| // 4. Expose legacy utilities | ||
| // Expose legacy utility running mechanism | ||
| ipcMain.handle('tools:list', () => toolRegistry.list()); | ||
| ipcMain.handle('tools:run', async (event, toolId, args) => { | ||
| // Note: appStore is removed, so we mock it for utilities if needed | ||
| // or just let them use basic features. | ||
| return toolRegistry.run(toolId, args, { | ||
| toolRegistry, | ||
| db, | ||
| log: logLine, | ||
| sendProgress: (payload) => { | ||
| event.sender.send(`tools:progress:${toolId}`, payload); | ||
| } | ||
| }); | ||
| }); | ||
| // Keep a reference for IPC handlers that still reach into main.js state. | ||
| windowManager.mainWindow = services.mainWindow; | ||
| // splashWindow was created earlier via windowManager.createSplashWindow(); | ||
| // do NOT overwrite it with services.splashWindow (which is undefined). | ||
| windowManager.splashTimeoutId = services.splashTimeoutId; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
lifecycle.start is async, but the call does not await it.
start is declared async in src/main/lifecycle.js line 240, so line 119 assigns a Promise to services. Three consequences follow:
- Line 126 sets
windowManager.mainWindowtoundefined. - Line 129 sets
windowManager.splashTimeoutIdtoundefined, soapp:readycannot clear the 8-second splash timeout. - Line 149 attaches
_pruneTimerto the Promise object, not to the service map.
Add await. The app.whenReady() callback must be async.
🐛 Proposed fix
- const services = lifecycle.start(db, eventBus, {
+ const services = await lifecycle.start(db, eventBus, {
userDataPath: app.getPath('userData'),
startupLocale,
notify: (title, body, level) => windowManager.showNotification(lifecycle.t(title), lifecycle.t(body), level),
});Confirm the enclosing app.whenReady().then(...) callback is declared async. If it is not, add the keyword.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/main.js` around lines 119 - 129, Await the async lifecycle.start
call before assigning its result to services, and ensure the enclosing
app.whenReady().then callback is async so await is valid. Preserve the existing
windowManager.mainWindow, windowManager.splashTimeoutId, and _pruneTimer
assignments against the resolved service map.
| module.exports = { | ||
| init, | ||
| createSplashWindow, | ||
| sendSplashProgress, | ||
| dismissSplash, | ||
| createWindow, | ||
| buildAppMenu, | ||
| showNotification, | ||
| repositionToasts, | ||
| toastHtml, | ||
| escToastHtml, | ||
| getToastMarkDataUri, | ||
| getToastWordmarkDataUri, | ||
| readPngAsDataUri, | ||
| scheduleScreenshotCapture, | ||
| failScreenshotCapture, | ||
| getScreenshotConfig, | ||
| isScreenshotCaptureMode, | ||
| createIcon, | ||
| activeToasts, | ||
| TOAST_WIDTH, | ||
| TOAST_HEIGHT, | ||
| TOAST_MARGIN, | ||
| TOAST_GAP, | ||
| TOAST_LIFETIME_MS, | ||
| TOAST_ICONS, | ||
| }; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | 🏗️ Heavy lift
Mutable window-manager state is not reachable through the module exports.
src/main/windowManager.js holds dbRef, startupLocale, mainWindow, splashWindow, and splashTimeoutId in module-level let bindings (lines 13-22) and exports only functions and toast constants. In CommonJS, an assignment such as windowManager.mainWindow = win adds a property to the exports object; it does not update the internal binding. A read of windowManager.dbRef therefore returns undefined. windowManager.lifecycleRefs is never defined in any file.
Expose the state through accessors, or move the shared references into an explicit state object that both modules import.
src/main/windowManager.js#L375-L401: add getters fordbRef,startupLocale,mainWindow,splashWindow, andsplashTimeoutId, plus setters for the values thatmain.jsassigns. Add alifecycleRefsaccessor, or remove that concept and letlifecycle.startreturn the handles that shutdown needs.src/main/main.js#L56-L60: the second-instance handler readswindowManager.mainWindow. It resolves toundefined, so an existing instance is never restored or focused, and asoterios://URL is never forwarded. Read the window through the new accessor.src/main/main.js#L160-L177:windowManager.lifecycleRefsis alwaysundefined, sobefore-quitnever stopsmaintenanceScheduler, never disposes the tray, and never clears the interval timers.windowManager.dbRef?.dbis alsoundefined, so the SQLite handle is never closed. Inwindow-all-closed, the tray guard never matches, so the app quits on window close even when the tray is active. Populate these references from the value thatlifecycle.startreturns.src/main/lifecycle.js#L71-L73:t()passeswindowManager.dbRefandwindowManager.startupLocaletogetLocale. Both areundefined, sogetLocalereturnsundefinedand every main-process translation falls back to the default locale. The user language selection is ignored for notifications and splash labels. Read both values through the new accessors.
♻️ Proposed accessor pattern for src/main/windowManager.js
+let lifecycleRefs = null;
+
+function setLifecycleRefs(refs) {
+ lifecycleRefs = refs;
+}
+
module.exports = {
init,
+ setLifecycleRefs,
createSplashWindow, TOAST_ICONS,
};
+
+Object.defineProperties(module.exports, {
+ dbRef: { get: () => dbRef, enumerable: true },
+ startupLocale: { get: () => startupLocale, enumerable: true },
+ currentUiTheme: { get: () => currentUiTheme, enumerable: true },
+ lifecycleRefs: { get: () => lifecycleRefs, enumerable: true },
+ mainWindow: {
+ get: () => mainWindow,
+ set: (win) => { mainWindow = win; },
+ enumerable: true,
+ },
+ splashWindow: {
+ get: () => splashWindow,
+ set: (win) => { splashWindow = win; },
+ enumerable: true,
+ },
+ splashTimeoutId: {
+ get: () => splashTimeoutId,
+ set: (id) => { splashTimeoutId = id; },
+ enumerable: true,
+ },
+});src/main/main.js must then populate the shutdown handles after lifecycle.start resolves:
windowManager.mainWindow = services.mainWindow;
+ windowManager.setLifecycleRefs({
+ maintenanceScheduler: services.maintenanceScheduler,
+ trayController: services.trayController,
+ networkStatsTimer: services._networkStatsTimer,
+ pruneTimer: services._pruneTimer,
+ });networkStatsTimer and pruneTimer are created after start returns in some paths. Read them from services inside the before-quit handler instead of copying them once, or keep a reference to services itself.
📍 Affects 3 files
src/main/windowManager.js#L375-L401(this comment)src/main/main.js#L56-L60src/main/main.js#L160-L177src/main/lifecycle.js#L71-L73
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/main/windowManager.js` around lines 375 - 401, Expose module-level state
in src/main/windowManager.js:375-401 through getters for dbRef, startupLocale,
mainWindow, splashWindow, and splashTimeoutId, setters for values assigned by
main.js, and a lifecycleRefs accessor or equivalent lifecycle handle state. In
src/main/main.js:56-60, use the mainWindow accessor for second-instance
restoration and URL forwarding. In src/main/main.js:160-177, populate lifecycle
references from lifecycle.start’s returned handles, retain access to services
for timers created later, and use the accessors for shutdown, database cleanup,
and tray checks. In src/main/lifecycle.js:71-73, read dbRef and startupLocale
through the new window-manager accessors when calling getLocale.
|
|
||
| // Incremental scan: skip paths that haven't changed since last scan. | ||
| if (skipPaths.has(targetPath)) { | ||
| emitProgress(basePct, 'Skipping unchanged: ' + targetPath + '...', { filesScanned: cumulativeFiles }); | ||
| continue; | ||
| } | ||
|
|
||
| const basePct = Math.round((i / paths.length) * 80 + 10); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate ScanEngine.js =="
fd -a 'ScanEngine\.js$' . || true
echo "== relevant lines =="
if [ -f src/security/ScanEngine.js ]; then
cat -n src/security/ScanEngine.js | sed -n '150,185p'
fi
echo "== JS TDZ behavior probe =="
node - <<'JS'
function loop(paths) {
let cumulativeFiles = 0;
const emitProgress = () => { ++cumulativeFiles; };
const skipPaths = new Set(['skip-me']);
for (let i = 0; i < paths.length; i++) {
const targetPath = paths[i];
// Mirrors the snippet’s ordering.
if (skipPaths.has(targetPath)) {
emitProgress(basePct, 'Skipping unchanged...', { filesScanned: cumulativeFiles });
continue;
}
const basePct = Math.round((i / paths.length) * 80 + 10);
}
}
try {
loop(['keep-me', 'skip-me']);
console.log('RESULT: no error');
} catch (e) {
console.log('RESULT: ' + e.name + ': ' + e.message);
}
JSRepository: chrisriv10/Soterios
Length of output: 1822
Initialize basePct before the incremental-skip branch.
Line 172 reads basePct while it is still in the Temporal Dead Zone. If skipPaths.has(targetPath) is true, the full scan throws a ReferenceError. Move the declaration before the skip check.
Proposed fix
const targetPath = paths[i];
+ const basePct = Math.round((i / paths.length) * 80 + 10);
// Incremental scan: skip paths that haven't changed since last scan.
if (skipPaths.has(targetPath)) {
emitProgress(basePct, 'Skipping unchanged: ' + targetPath + '...', { filesScanned: cumulativeFiles });
continue;
}
-
- const basePct = Math.round((i / paths.length) * 80 + 10);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/security/ScanEngine.js` around lines 169 - 176, Move the basePct
declaration in the scan loop before the skipPaths.has(targetPath) branch so
emitProgress can safely use it for unchanged paths. Preserve the existing
percentage calculation and skip behavior for all paths.
|
@schvarts1
Minor: ClamAV system-path fallback never triggers, duplicate prune timer in CodeRabbit's inline comments have suggested diffs for most of these. Ping me once they're in and I'll take another look. |
|
mb i was perplex on what could make the app not work properly but no logs in console and not much errors in the app itself. |
|
Ill fix it |
2 participants
Split main.js into lifecycle.js and windowManager.js; harden toast BrowserWindow with contextIsolation/sandbox
Add validateArgs IPC validation, structured error classes, and ALLOWED_INVOKE/ALLOWED_ON preload allowlists
Wire IPC validation across 18+ handlers in scan, firewall, network, quarantine, process, system
Register missing tools:list and tools:run handlers so tool actions work again
Add auditLog module and wire into FirewallManager, EmergencyLockdown, QuarantineManager, ProcessInspector, maintenanceScheduler
Extend database schema with user_blocklist, user_domain_blocklist, audit_log, scanned_files
Add multi-folder scans, incremental scan support, and empty-catch cleanup
Extract toast and scan-report templates; replace empty catch blocks with logging
Add cross-platform ClamAV engine stubs and platform-aware serviceRegistry
Preserve 282-test green baseline; verify with npm test
Summary by CodeRabbit