feat(malware): allowlist false positives so trusted files stop being flagged#193
Conversation
After a clean, "x threats quarantined" was plain text and the quarantine list could not show why a file was flagged (the manifest only stored the original path and timestamp). - Dashboard result links straight to the quarantine list and opens the Quarantine tab directly via router state. - Detection metadata (name, severity, source, details) is now threaded through quarantine and persisted in the manifest; the list shows a severity badge and detection name, with details on hover. Backward compatible: meta is optional, so the CLI/cloud-agent callers and pre-existing quarantined entries continue to work (older entries simply omit the reason). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Detection enums are read back from a JSON manifest on disk and arrive over IPC, so an unexpected value (corrupt/edited manifest) could be passed to the renderer and index severityConfig to undefined, crashing the quarantine view. - Normalize severity/source to known values at the IPC boundary, both when writing the manifest and when listing quarantined files. - Defensively guard the renderer so an unknown severity omits the badge instead of throwing. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
…flagged Adds a "Not a virus / ignore" action on scan-result detections and a "Restore & always ignore" action on quarantined items, plus an Allowlist tab to view and undo those decisions. Detections are matched against the allowlist by file content SHA-256, so a known-good file (e.g. a game .exe) stays trusted even if moved, while a different binary at the same path is still scanned. Suppression happens at the single point where all detection sources are aggregated, so it covers every engine (signature, heuristic, scripts, persistence, native AV). Only real files can be allowlisted; registry/hosts/ADS detections are excluded. The allowlist persists in settings via lock-safe add/remove mutators mirroring updateRegistryIgnoredTweaks (de-dupe by hash, capped at 500).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6e439a1933
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const prev = writeLock | ||
| let unlock: () => void | ||
| writeLock = new Promise<void>((r) => { unlock = r }) | ||
| prev.then(() => { |
There was a problem hiding this comment.
When a user marks a detection safe (or removes an allowlist entry), this queues the settings write in prev.then(...) but returns before the write has run. The IPC handlers therefore resolve successfully while getMalwareAllowlist() may still read the old config; an immediate rescan after “Not a virus” can flag the same file again, and the Remove button can reload the stale entry right after reporting success. Make these helpers return the queued promise and await it from the IPC handlers before returning.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a946219be
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (allowed.has(await hashFileSha256(threat.path))) { | ||
| suppressed++ | ||
| continue |
There was a problem hiding this comment.
On Windows, ADS detections are created with path: ads.path for the base file (Heuristic.HiddenADS), so stat(threat.path).isFile() is true and this hashes only the unnamed file stream. If a user has allowlisted that base file (or clicks “Not a virus” on an ADS row), future hidden alternate streams on the same file are suppressed even though the stream contents were never hashed or trusted. Exclude ADS detections from hash suppression or include the stream identity/content in the allowlist key.
Useful? React with 👍 / 👎.
# Conflicts: # src/main/ipc/malware-scanner.ipc.ts # src/renderer/src/pages/MalwareScannerPage.tsx # src/shared/types.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 88443a97f3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (!ok) { failed.push(item.originalFileName); continue } | ||
| restored++ | ||
| // Allowlist the restored file by content hash so it isn't re-flagged. | ||
| await window.kudu.malwareIgnore(item.originalPath, { |
There was a problem hiding this comment.
In the restore-and-ignore flow, malwareIgnore can return null when the restored target is not allowlistable (for example ADS metadata, a path outside the current scan dirs, or a stat/hash failure), but this return value is ignored and the code still increments restored and shows “restored and added to the allowlist.” In those cases the file is restored but not trusted, so the next scan can flag it again while the UI told the user it was permanently ignored; treat a null result as a partial failure or at least avoid the allowlist success message for that item.
Useful? React with 👍 / 👎.
1 participant
What & why
Users had no way to tell Kudu that a malware detection is a false positive. The scanner would re-flag a known-good file (the reporting user's Guild Wars 1
.exe) on every scan, with no override. This adds an allowlist so a trusted file can be marked safe and stops being reported.How it works
threatsarray; the filter runs there, so it covers every source. Only flagged files are hashed (not every scanned file). The scan summary reports how many detections were suppressed — no silent drops.updateRegistryIgnoredTweakslock-safe pattern: de-dupe by hash, capped at 500 entries.UI
Tests
Verify manually