feat(malware): allowlist false positives so trusted files stop being flagged

src/main/ipc/malware-scanner.ipc.ts

@@ -2,22 +2,25 @@ import { ipcMain, app, BrowserWindow } from 'electron'
 import { execFile } from 'child_process'
 import { promisify } from 'util'
 import { readFile, readdir, writeFile, stat, mkdir, rename, rm } from 'fs/promises'
+import { createReadStream } from 'fs'
 import { join, basename, extname, resolve, normalize } from 'path'
-import { randomUUID } from 'crypto'
+import { randomUUID, createHash } from 'crypto'
 import { IPC } from '../../shared/channels'
 import type {
   MalwareThreat,
   MalwareScanResult,
   MalwareScanProgress,
   MalwareActionResult,
   QuarantinedItem,
+  QuarantineMeta,
+  MalwareAllowlistEntry,
   YaraRulesInfo
 } from '../../shared/types'
 import type { WindowGetter } from './index'
 import { getPlatform } from '../platform'
 import { validateStringArray } from '../services/ipc-validation'
 import { psUtf8, execNativeUtf8 } from '../services/exec-utf8'
-import { getSettings } from '../services/settings-store'
+import { getSettings, getMalwareAllowlist, addMalwareAllowlistEntry, removeMalwareAllowlistEntry } from '../services/settings-store'
 import { isExcluded } from '../services/file-utils'
 import { createYaraEngine, yaraMatchToThreatFields, type YaraEngine } from '../services/yara-engine'
 import { getAllRulePaths, getCachedRulePaths, getRulesMetadata, startPeriodicRuleChecks, fetchAndCacheRules, RULES_ENDPOINT } from '../services/yara-rules-store'
@@ -38,9 +41,23 @@ function getQuarantinePath(): string {
 interface ManifestEntry {
   originalPath: string
   quarantinedAt: number
+  detectionName?: string
+  severity?: 'critical' | 'high' | 'medium' | 'low'
+  source?: 'defender' | 'heuristic' | 'signature'
+  details?: string
 }
 type Manifest = Record<string, ManifestEntry>
 
+// Detection metadata is read back from a JSON manifest on disk (and arrives
+// over IPC), so normalize the enums to known values — an unexpected string
+// would otherwise be passed to the renderer and break severity-keyed lookups.
+const VALID_SEVERITIES = new Set<QuarantinedItem['severity']>(['critical', 'high', 'medium', 'low'])
+const VALID_SOURCES = new Set<QuarantinedItem['source']>(['defender', 'heuristic', 'signature'])
+const asSeverity = (v: unknown): QuarantinedItem['severity'] | undefined =>
+  typeof v === 'string' && VALID_SEVERITIES.has(v as QuarantinedItem['severity']) ? (v as QuarantinedItem['severity']) : undefined
+const asSource = (v: unknown): QuarantinedItem['source'] | undefined =>
+  typeof v === 'string' && VALID_SOURCES.has(v as QuarantinedItem['source']) ? (v as QuarantinedItem['source']) : undefined
+
 function getManifestPath(): string {
   return join(getQuarantinePath(), 'quarantine-manifest.json')
 }
@@ -1011,6 +1028,51 @@ function isPathInAllowedDirs(filePath: string, allowedDirs: string[]): boolean {
   })
 }
 
+/** Stream a file's full SHA-256, used to match the false-positive allowlist. */
+function hashFileSha256(filePath: string): Promise<string> {
+  return new Promise((resolve, reject) => {
+    const hash = createHash('sha256')
+    const stream = createReadStream(filePath, { highWaterMark: 65536 })
+    stream.on('data', (chunk) => hash.update(chunk))
+    stream.on('end', () => resolve(hash.digest('hex')))
+    stream.on('error', reject)
+  })
+}
+
+/**
+ * Drop any detection whose file content hash is on the user's false-positive
+ * allowlist. Only real files are hashed (non-file detections — registry, hosts,
+ * ADS — can't be allowlisted and are left untouched). Hashing failures are
+ * swallowed so an unreadable path never aborts a scan. Returns the surviving
+ * threats plus the count suppressed, so the caller can report it (no silent caps).
+ */
+async function filterAllowlistedThreats(threats: MalwareThreat[]): Promise<{ kept: MalwareThreat[]; suppressed: number }> {
+  const allowed = new Set(getMalwareAllowlist().map((e) => e.sha256))
+  if (allowed.size === 0) return { kept: threats, suppressed: 0 }
+  const kept: MalwareThreat[] = []
+  let suppressed = 0
+  for (const threat of threats) {
+    let isFile = false
+    try {
+      isFile = (await stat(threat.path)).isFile()
+    } catch {
+      isFile = false
+    }
+    if (isFile) {
+      try {
+        if (allowed.has(await hashFileSha256(threat.path))) {
+          suppressed++
+          continue
+        }
+      } catch {
+        // Unreadable file — keep the detection rather than silently dropping it.
+      }
+    }
+    kept.push(threat)
+  }
+  return { kept, suppressed }
+}
+
 // Only scan files with these extensions for performance
 function getScannableExtensions(): Set<string> {
   const platformExts = getPlatform().malware.scannableExtensions()
@@ -1660,7 +1722,14 @@ export async function scanMalware(
       updateCategory('defender', { status: 'skipped' })
     }
 
-    completedSteps.push(`Scan complete — ${threats.length} total threat${threats.length !== 1 ? 's' : ''} detected`)
+    // Suppress detections the user has marked as false positives (matched by
+    // file content hash) before reporting results.
+    const { kept: visibleThreats, suppressed } = await filterAllowlistedThreats(threats)
+    if (suppressed > 0) {
+      completedSteps.push(`Ignored ${suppressed} detection${suppressed !== 1 ? 's' : ''} you previously marked safe`)
+    }
+
+    completedSteps.push(`Scan complete — ${visibleThreats.length} total threat${visibleThreats.length !== 1 ? 's' : ''} detected`)
     sendProgress({
       step: 'complete',
       stepLabel: 'Scan complete',
@@ -1671,19 +1740,29 @@ export async function scanMalware(
     })
 
     return {
-      threats,
+      threats: visibleThreats,
       filesScanned,
       duration: Date.now() - startTime,
       engines
     }
 }
 
-export async function quarantineMalware(paths: string[]): Promise<MalwareActionResult> {
+export async function quarantineMalware(paths: string[], meta?: QuarantineMeta[]): Promise<MalwareActionResult> {
     const allowedDirs = getScanPaths()
     const validPaths = Array.isArray(paths)
       ? paths.filter((v): v is string => typeof v === 'string' && isPathInAllowedDirs(v, allowedDirs))
       : []
 
+    // Index detection metadata by path so each quarantined file records why it was flagged.
+    const metaByPath = new Map<string, QuarantineMeta>()
+    if (Array.isArray(meta)) {
+      for (const m of meta) {
+        if (m && typeof m.path === 'string') metaByPath.set(m.path, m)
+      }
+    }
+    const clip = (s: unknown): string | undefined =>
+      typeof s === 'string' && s.length > 0 ? s.slice(0, 500) : undefined
+
     const quarantineDir = getQuarantinePath()
     await mkdir(quarantineDir, { recursive: true })
 
@@ -1699,7 +1778,15 @@ export async function quarantineMalware(paths: string[]): Promise<MalwareActionR
         const quarantinedName = `${now}_${randomUUID()}_${fileName}.quarantined`
         const destPath = join(quarantineDir, quarantinedName)
         await rename(filePath, destPath)
-        manifest[quarantinedName] = { originalPath: filePath, quarantinedAt: now }
+        const m = metaByPath.get(filePath)
+        manifest[quarantinedName] = {
+          originalPath: filePath,
+          quarantinedAt: now,
+          detectionName: clip(m?.detectionName),
+          severity: asSeverity(m?.severity),
+          source: asSource(m?.source),
+          details: clip(m?.details)
+        }
         succeeded++
       } catch (err: any) {
         failed++
@@ -1779,7 +1866,11 @@ export async function listQuarantinedFiles(): Promise<QuarantinedItem[]> {
         originalPath: manifestEntry?.originalPath ?? '',
         originalFileName,
         quarantinedAt: manifestEntry?.quarantinedAt ?? (parsedTimestamp || 0),
-        size
+        size,
+        detectionName: manifestEntry?.detectionName,
+        severity: asSeverity(manifestEntry?.severity),
+        source: asSource(manifestEntry?.source),
+        details: manifestEntry?.details
       })
     }
 
@@ -1804,16 +1895,47 @@ export async function restoreMalware(quarantinedPath: string, originalPath: stri
     }
 }
 
+/**
+ * Mark a file as a false positive: hash its current contents and add it to the
+ * allowlist so future scans suppress it. Returns the created entry, or null if
+ * the path is outside the allowed scan dirs, isn't a readable file, or hashing
+ * fails. `meta.path` is ignored — the explicit `path` argument is authoritative.
+ */
+export async function ignoreMalware(path: string, meta?: QuarantineMeta): Promise<MalwareAllowlistEntry | null> {
+    if (typeof path !== 'string' || !isPathInAllowedDirs(path, getScanPaths())) return null
+    try {
+      if (!(await stat(path)).isFile()) return null
+      const sha256 = await hashFileSha256(path)
+      const detectionName = typeof meta?.detectionName === 'string' && meta.detectionName.length > 0
+        ? meta.detectionName.slice(0, 500)
+        : undefined
+      const entry: MalwareAllowlistEntry = {
+        sha256,
+        path,
+        fileName: basename(path),
+        detectionName,
+        addedAt: Date.now()
+      }
+      await addMalwareAllowlistEntry(entry)
+      return entry
+    } catch {
+      return null
+    }
+}
+
 export function registerMalwareScannerIpc(getWindow: WindowGetter): void {
   ipcMain.handle(IPC.MALWARE_SCAN, () => scanMalware((data) => {
     const win = getWindow()
     if (win && !win.isDestroyed()) win.webContents.send(IPC.MALWARE_PROGRESS, data)
   }))
 
-  ipcMain.handle(IPC.MALWARE_QUARANTINE, async (_event, paths: string[]) => {
+  ipcMain.handle(IPC.MALWARE_QUARANTINE, async (_event, paths: string[], meta?: QuarantineMeta[]) => {
     const valid = validateStringArray(paths, 5_000)
     if (!valid) return { succeeded: 0, failed: 0, errors: [] }
-    return quarantineMalware(valid)
+    const validMeta = Array.isArray(meta)
+      ? meta.filter((m): m is QuarantineMeta => !!m && typeof m.path === 'string').slice(0, 5_000)
+      : undefined
+    return quarantineMalware(valid, validMeta)
   })
 
   ipcMain.handle(IPC.MALWARE_DELETE, async (_event, paths: string[]) => {
@@ -1828,6 +1950,19 @@ export function registerMalwareScannerIpc(getWindow: WindowGetter): void {
 
   ipcMain.handle(IPC.MALWARE_QUARANTINE_LIST, () => listQuarantinedFiles())
 
+  ipcMain.handle(IPC.MALWARE_IGNORE, async (_event, path: string, meta?: QuarantineMeta) => {
+    const validMeta = meta && typeof meta === 'object' && typeof meta.path === 'string' ? meta : undefined
+    return ignoreMalware(path, validMeta)
+  })
+
+  ipcMain.handle(IPC.MALWARE_ALLOWLIST_LIST, () => getMalwareAllowlist())
+
+  ipcMain.handle(IPC.MALWARE_ALLOWLIST_REMOVE, async (_event, sha256: string) => {
+    if (typeof sha256 !== 'string' || !sha256) return false
+    await removeMalwareAllowlistEntry(sha256)
+    return true
+  })
+
   ipcMain.handle(IPC.MALWARE_YARA_INFO, () => getYaraRulesInfo())
 
   ipcMain.handle(IPC.MALWARE_YARA_UPDATE, async () => {

src/main/ipc/malware-scanner.test.ts

@@ -317,3 +317,67 @@ describe('PE heuristic scoring', () => {
     })).toBe(false)
   })
 })
+
+// ─── Allowlist suppression decision (replica) ────────────────────
+// Mirrors filterAllowlistedThreats: a detection is suppressed only when it is
+// a real file whose content hash is on the allowlist. Non-files and unreadable
+// files are always kept (never silently dropped).
+
+function isSuppressed(opts: { isFile: boolean; hash: string | null; allowed: Set<string> }): boolean {
+  if (!opts.isFile) return false       // registry/hosts/ADS targets can't be allowlisted
+  if (opts.hash === null) return false // unreadable file — keep the detection
+  return opts.allowed.has(opts.hash)
+}
+
+describe('allowlist suppression decision', () => {
+  const allowed = new Set(['abc123'])
+
+  it('suppresses a file whose hash is allowlisted', () => {
+    expect(isSuppressed({ isFile: true, hash: 'abc123', allowed })).toBe(true)
+  })
+
+  it('keeps a file whose hash is not allowlisted', () => {
+    expect(isSuppressed({ isFile: true, hash: 'def456', allowed })).toBe(false)
+  })
+
+  it('keeps a non-file detection even if its path string somehow matches', () => {
+    expect(isSuppressed({ isFile: false, hash: 'abc123', allowed })).toBe(false)
+  })
+
+  it('keeps an unreadable file rather than dropping it', () => {
+    expect(isSuppressed({ isFile: true, hash: null, allowed })).toBe(false)
+  })
+
+  it('suppresses nothing when the allowlist is empty', () => {
+    expect(isSuppressed({ isFile: true, hash: 'abc123', allowed: new Set() })).toBe(false)
+  })
+})
+
+// ─── isAllowlistablePath (replica of MalwareScannerPage) ─────────
+// The "Not a virus" action only appears for real filesystem paths.
+
+function isAllowlistablePath(path: string): boolean {
+  return /^([a-zA-Z]:[\\/]|\\\\|\/)/.test(path)
+}
+
+describe('isAllowlistablePath', () => {
+  it('accepts a Windows drive path', () => {
+    expect(isAllowlistablePath('C:\\Games\\Guild Wars\\Gw.exe')).toBe(true)
+  })
+
+  it('accepts a UNC path', () => {
+    expect(isAllowlistablePath('\\\\server\\share\\app.exe')).toBe(true)
+  })
+
+  it('accepts a POSIX path', () => {
+    expect(isAllowlistablePath('/usr/local/bin/app')).toBe(true)
+  })
+
+  it('rejects a registry key target', () => {
+    expect(isAllowlistablePath('HKLM\\Software\\Microsoft\\Windows\\CurrentVersion\\Run')).toBe(false)
+  })
+
+  it('rejects a scheduled-task path (single leading backslash, not UNC)', () => {
+    expect(isAllowlistablePath('\\Microsoft\\Windows\\SuspiciousTask')).toBe(false)
+  })
+})

src/main/services/settings-store.persistence.test.ts

@@ -18,7 +18,8 @@ vi.mock('electron', () => ({
   },
 }))
 
-import { setSettings, getSettings, flushSettings, updateRegistryIgnoredTweaks } from './settings-store'
+import { setSettings, getSettings, flushSettings, updateRegistryIgnoredTweaks, getMalwareAllowlist, addMalwareAllowlistEntry, removeMalwareAllowlistEntry } from './settings-store'
+import type { MalwareAllowlistEntry } from '../../shared/types'
 
 describe('settings persistence — game mode toggle round-trip (issue #172)', () => {
   afterAll(() => {
@@ -91,3 +92,57 @@ describe('settings persistence — game mode toggle round-trip (issue #172)', ()
     expect(getSettings().registryIgnoredTweaks).toEqual([b])
   })
 })
+
+describe('malware allowlist (false positives)', () => {
+  afterAll(() => {
+    if (existsSync(TEST_DIR)) rmSync(TEST_DIR, { recursive: true, force: true })
+  })
+
+  const entry = (sha256: string, over: Partial<MalwareAllowlistEntry> = {}): MalwareAllowlistEntry => ({
+    sha256,
+    path: `C:/Games/${sha256}.exe`,
+    fileName: `${sha256}.exe`,
+    detectionName: 'Heuristic.Suspicious.PE',
+    addedAt: 1,
+    ...over,
+  })
+
+  it('defaults the allowlist to an empty array', () => {
+    expect(getMalwareAllowlist()).toEqual([])
+  })
+
+  it('adds an entry that survives a simulated restart', async () => {
+    addMalwareAllowlistEntry(entry('aaa'))
+    await flushSettings()
+    const list = getMalwareAllowlist()
+    expect(list).toHaveLength(1)
+    expect(list[0].sha256).toBe('aaa')
+  })
+
+  it('de-dupes by content hash, refreshing the existing entry', async () => {
+    addMalwareAllowlistEntry(entry('aaa', { path: 'C:/Moved/Gw.exe', fileName: 'Gw.exe' }))
+    await flushSettings()
+    const list = getMalwareAllowlist()
+    expect(list.filter((e) => e.sha256 === 'aaa')).toHaveLength(1)
+    expect(list.find((e) => e.sha256 === 'aaa')?.fileName).toBe('Gw.exe')
+  })
+
+  it('removes an entry by hash', async () => {
+    addMalwareAllowlistEntry(entry('bbb'))
+    await flushSettings()
+    expect(getMalwareAllowlist().some((e) => e.sha256 === 'bbb')).toBe(true)
+
+    removeMalwareAllowlistEntry('bbb')
+    await flushSettings()
+    expect(getMalwareAllowlist().some((e) => e.sha256 === 'bbb')).toBe(false)
+  })
+
+  it('merges concurrent adds without dropping earlier entries', async () => {
+    addMalwareAllowlistEntry(entry('c1'))
+    addMalwareAllowlistEntry(entry('c2'))
+    await flushSettings()
+    const hashes = getMalwareAllowlist().map((e) => e.sha256)
+    expect(hashes).toContain('c1')
+    expect(hashes).toContain('c2')
+  })
+})