From 8795b1f4fe4435dd02129f5abc3cd64629bb4cb6 Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 27 Jul 2026 05:12:52 +0200 Subject: [PATCH 1/2] fix(registry): stop deleting whole registry branches on a failed key parse MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit reg.exe echoes long-form hive names (HKEY_LOCAL_MACHINE\...), but nine scans matched key headers against short forms (HKLM\...), so those matches never succeeded. The App Paths scan fell back to its container key when the match failed, so a single stale entry produced a delete of HKLM\...\CurrentVersion\App Paths itself — removing every App Paths registration on the machine rather than the one dead entry. The other eight scans guarded on the match and were silently dead, so they have never run in production. - normalize hive names in execReg output so key headers parse - drop the container-key fallbacks in the App Paths and shellex scans - refuse delete-key on hive roots and scan container keys at execution time - ship the eight revived scans deselected pending validation Refs #244 --- src/main/ipc/registry-cleaner.ipc.ts | 147 +++++++++++-- src/main/ipc/registry-cleaner.scan.test.ts | 235 +++++++++++++++++++++ 2 files changed, 366 insertions(+), 16 deletions(-) create mode 100644 src/main/ipc/registry-cleaner.scan.test.ts diff --git a/src/main/ipc/registry-cleaner.ipc.ts b/src/main/ipc/registry-cleaner.ipc.ts index b67fd0a3..688dcc13 100644 --- a/src/main/ipc/registry-cleaner.ipc.ts +++ b/src/main/ipc/registry-cleaner.ipc.ts @@ -16,9 +16,42 @@ import { execNativeUtf8, execTracked, psUtf8 } from '../services/exec-utf8' const execFileAsync = promisify(execFile) +/** Long-form hive names as printed by reg.exe, mapped to the short form used throughout this file. */ +const HIVE_ALIASES: Record = { + HKEY_LOCAL_MACHINE: 'HKLM', + HKEY_CURRENT_USER: 'HKCU', + HKEY_CLASSES_ROOT: 'HKCR', + HKEY_USERS: 'HKU', + HKEY_CURRENT_CONFIG: 'HKCC' +} + +/** + * Rewrite the long-form hive names in reg.exe's key headers to their short form. + * + * `reg query HKLM\...` echoes its results as `HKEY_LOCAL_MACHINE\...`, but every + * scan below matches key headers against — and every fix below deletes — short + * forms like `HKLM\...`. Without this the header patterns never match, which + * silently disables most scans and, worse, makes the App Paths scan fall back to + * its container key (see the `keyPath` guard there). + * + * Only line-leading hive names are rewritten: value lines are indented by four + * spaces, so registry *data* containing a hive name is left untouched. + */ +export function normalizeHiveNames(stdout: string): string { + return stdout.replace(/^(HKEY_[A-Z_]+)/gm, (match) => HIVE_ALIASES[match] ?? match) +} + +/** Convert a single key path's hive to short form (`HKEY_LOCAL_MACHINE\Foo` → `HKLM\Foo`). */ +function normalizeKeyPath(key: string): string { + const idx = key.indexOf('\\') + if (idx < 0) return HIVE_ALIASES[key] ?? key + return (HIVE_ALIASES[key.substring(0, idx)] ?? key.substring(0, idx)) + key.substring(idx) +} + /** Run reg.exe with UTF-8 code page so accented characters decode correctly */ async function execReg(args: string[], opts?: { timeout?: number; signal?: AbortSignal }): Promise<{ stdout: string; stderr: string }> { - return execNativeUtf8('reg', args, opts) + const result = await execNativeUtf8('reg', args, opts) + return { ...result, stdout: normalizeHiveNames(result.stdout) } } // ── Active AbortControllers for cancellable operations ── @@ -85,6 +118,19 @@ function splitTaskPath(fullPath: string): { path: string; name: string } | null // Session-scoped scan results keyed by scan ID to prevent race conditions const scanSessions = new Map>() +/** + * Pre-selection state for scans that were unreachable until hive-name + * normalization landed. + * + * Their key-header patterns only ever matched short-form hives (`HKCR\…`), + * which reg.exe never prints, so `keyMatch` was always null and every one of + * them was silently dead. Now that they parse, they are reported but left + * unticked: none has ever run against a real machine, and several delete whole + * COM registration keys. The user opts in per finding rather than having an + * unvalidated heuristic pre-selected for deletion. + */ +const UNVALIDATED_SCAN_SELECTED = false + /** Expand common Windows environment variables in a registry path. */ function expandEnvVars(path: string): string { return path @@ -245,17 +291,21 @@ export async function scanRegistry(signal?: AbortSignal): Promise k.toLowerCase())) + +/** + * True when `key` is a hive root or one of the container keys above, in either + * long or short hive form. Exported for tests. + */ +export function isProtectedDeleteKey(key: string): boolean { + const normalized = normalizeKeyPath(key.trim()).replace(/\\+$/, '') + if (!normalized) return true + // A hive with no subkey ("HKLM", "HKEY_LOCAL_MACHINE") is always the whole hive. + if (!normalized.includes('\\')) return true + return PROTECTED_DELETE_KEYS.has(normalized.toLowerCase()) +} + export async function fixRegistryEntries( entries: RegistryEntry[], onProgress?: (current: number, total: number, label: string) => void, @@ -1884,6 +1996,9 @@ export async function fixRegistryEntries( break case 'delete-key': + if (isProtectedDeleteKey(key)) { + throw new Error(`Refusing to delete protected registry key: ${key}`) + } await execReg(['delete', key, '/f'], { timeout: 10000, signal }) break diff --git a/src/main/ipc/registry-cleaner.scan.test.ts b/src/main/ipc/registry-cleaner.scan.test.ts new file mode 100644 index 00000000..332124ff --- /dev/null +++ b/src/main/ipc/registry-cleaner.scan.test.ts @@ -0,0 +1,235 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest' +import { promisify } from 'util' + +// ── Mocks ─────────────────────────────────────────────────────────── +// These tests drive the real scan/fix code against realistic reg.exe +// output. The pure-helper tests in registry-cleaner.ipc.test.ts use +// replicas, which cannot catch a mismatch between a scan's regex and +// what reg.exe actually prints — the defect this file guards against. + +const mockExecNative = vi.fn() + +function createExecFileMock() { + const fn = (...args: unknown[]) => (args[args.length - 1] as any)?.(null, '', '') + ;(fn as any)[promisify.custom] = () => Promise.resolve({ stdout: '', stderr: '' }) + return fn +} + +vi.mock('child_process', () => ({ execFile: createExecFileMock() })) + +vi.mock('electron', () => ({ ipcMain: { handle: vi.fn() } })) + +vi.mock('../services/exec-utf8', () => ({ + execNativeUtf8: (tool: string, args: string[], opts?: any) => mockExecNative(tool, args, opts), + execTracked: vi.fn(async () => ({ stdout: '', stderr: '' })), + psUtf8: (cmd: string) => cmd, +})) + +const mockExistsSync = vi.fn((_p: string): boolean => true) + +vi.mock('fs', () => ({ + existsSync: (p: string) => mockExistsSync(p), + statSync: () => ({ isFile: () => true }), + readdirSync: () => [], + unlinkSync: vi.fn(), + mkdirSync: vi.fn(), + mkdtempSync: () => 'C:\\temp\\kudu-test', + readFileSync: () => '', + writeFileSync: vi.fn(), + rmSync: vi.fn(), +})) + +vi.mock('../services/backup-dir', () => ({ getBackupDir: () => 'C:\\temp\\backups' })) + +vi.mock('../services/settings-store', () => ({ + getSettings: () => ({}), + updateRegistryIgnoredTweaks: vi.fn(), +})) + +vi.mock('../services/ipc-validation', () => ({ validateStringArray: (a: string[]) => a })) + +import { + scanRegistry, + fixRegistryEntries, + normalizeHiveNames, + isProtectedDeleteKey, +} from './registry-cleaner.ipc' + +const APP_PATHS_ROOT = 'HKLM\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\App Paths' + +/** Verbatim `reg query "HKLM\...\App Paths" /s` output — note the long hive names. */ +const APP_PATHS_OUTPUT = [ + 'HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\App Paths\\chrome.exe', + ' (Default) REG_SZ C:\\Program Files\\Google\\Chrome\\Application\\chrome.exe', + ' Path REG_SZ C:\\Program Files\\Google\\Chrome\\Application', + '', + 'HKEY_LOCAL_MACHINE\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\App Paths\\ghost.exe', + ' (Default) REG_SZ C:\\Program Files\\Uninstalled\\ghost.exe', + '', +].join('\r\n') + +beforeEach(() => { + vi.clearAllMocks() + mockExistsSync.mockImplementation(() => true) + mockExecNative.mockImplementation(async () => ({ stdout: '', stderr: '' })) +}) + +describe('normalizeHiveNames', () => { + it('rewrites long-form hive names in key headers to short form', () => { + expect(normalizeHiveNames('HKEY_LOCAL_MACHINE\\SOFTWARE\\Foo')).toBe('HKLM\\SOFTWARE\\Foo') + expect(normalizeHiveNames('HKEY_CURRENT_USER\\SOFTWARE\\Foo')).toBe('HKCU\\SOFTWARE\\Foo') + expect(normalizeHiveNames('HKEY_CLASSES_ROOT\\CLSID\\{abc}')).toBe('HKCR\\CLSID\\{abc}') + }) + + it('leaves indented value lines untouched', () => { + const input = 'HKEY_LOCAL_MACHINE\\SOFTWARE\\Foo\r\n Data REG_SZ HKEY_LOCAL_MACHINE\\Bar\r\n' + expect(normalizeHiveNames(input)).toBe('HKLM\\SOFTWARE\\Foo\r\n Data REG_SZ HKEY_LOCAL_MACHINE\\Bar\r\n') + }) + + it('leaves already-short and unknown hives alone', () => { + expect(normalizeHiveNames('HKLM\\SOFTWARE\\Foo')).toBe('HKLM\\SOFTWARE\\Foo') + expect(normalizeHiveNames('HKEY_MADE_UP\\Foo')).toBe('HKEY_MADE_UP\\Foo') + }) +}) + +describe('scanRegistry — App Paths', () => { + function stubAppPaths(): void { + mockExecNative.mockImplementation(async (tool: string, args: string[]) => { + if (tool === 'reg' && args[0] === 'query' && args[1] === APP_PATHS_ROOT) { + return { stdout: APP_PATHS_OUTPUT, stderr: '' } + } + if (tool === 'schtasks') throw new Error('no tasks') + return { stdout: '', stderr: '' } + }) + mockExistsSync.mockImplementation((p: string) => !String(p).includes('Uninstalled')) + } + + it('targets the stale subkey, never the App Paths container', async () => { + stubAppPaths() + const entries = await scanRegistry() + const appPathFindings = entries.filter(e => e.issue.startsWith('App path points to missing file')) + + expect(appPathFindings).toHaveLength(1) + expect(appPathFindings[0].keyPath).toBe(`${APP_PATHS_ROOT}\\ghost.exe`) + // The regression: a failed header parse used to fall back to the container, + // so applying the fix deleted every App Paths registration on the machine. + expect(appPathFindings[0].keyPath).not.toBe(APP_PATHS_ROOT) + }) + + it('does not flag app paths whose executable still exists', async () => { + stubAppPaths() + const entries = await scanRegistry() + expect(entries.some(e => e.issue.includes('chrome.exe'))).toBe(false) + }) + + it('emits nothing for App Paths when the key header cannot be parsed', async () => { + mockExecNative.mockImplementation(async (tool: string, args: string[]) => { + if (tool === 'reg' && args[0] === 'query' && args[1] === APP_PATHS_ROOT) { + // Header line missing entirely — the scan must not guess a key. + return { stdout: ' (Default) REG_SZ C:\\Gone\\gone.exe\r\n', stderr: '' } + } + if (tool === 'schtasks') throw new Error('no tasks') + return { stdout: '', stderr: '' } + }) + mockExistsSync.mockImplementation(() => false) + + const entries = await scanRegistry() + expect(entries.some(e => e.issue.startsWith('App path points to missing file'))).toBe(false) + }) +}) + +describe('previously-dead scans ship unticked', () => { + const TYPELIB_OUTPUT = [ + 'HKEY_CLASSES_ROOT\\TypeLib\\{11111111-2222-3333-4444-555555555555}\\1.0\\0\\win32', + ' (Default) REG_SZ C:\\Program Files\\Gone\\gone.tlb', + '', + ].join('\r\n') + + it('reports a revived finding but leaves it deselected', async () => { + mockExecNative.mockImplementation(async (tool: string, args: string[]) => { + if (tool === 'reg' && args[0] === 'query' && args[1] === 'HKCR\\TypeLib') { + return { stdout: TYPELIB_OUTPUT, stderr: '' } + } + if (tool === 'schtasks') throw new Error('no tasks') + return { stdout: '', stderr: '' } + }) + mockExistsSync.mockImplementation((p: string) => !String(p).includes('Gone')) + + const entries = await scanRegistry() + const tlb = entries.filter(e => e.issue.startsWith('Type library file missing')) + + expect(tlb).toHaveLength(1) + // These scans never executed before hive normalization landed, so a finding + // must not arrive pre-ticked for deletion. + expect(tlb[0].selected).toBe(false) + expect(tlb[0].keyPath).toContain('{11111111-2222-3333-4444-555555555555}') + }) +}) + +describe('isProtectedDeleteKey', () => { + it('blocks bare hive roots in both forms', () => { + for (const k of ['HKLM', 'HKCU', 'HKCR', 'HKEY_LOCAL_MACHINE', 'HKEY_CLASSES_ROOT']) { + expect(isProtectedDeleteKey(k)).toBe(true) + } + }) + + it('blocks the container keys the scans enumerate', () => { + for (const k of [ + APP_PATHS_ROOT, + 'HKLM\\SYSTEM\\CurrentControlSet\\Services', + 'HKCR\\CLSID', + 'HKCR\\Interface', + 'HKCR\\TypeLib', + 'HKLM\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Uninstall', + 'HKCR\\*\\shellex\\ContextMenuHandlers', + ]) { + expect(isProtectedDeleteKey(k)).toBe(true) + } + }) + + it('matches regardless of hive form, casing, or trailing slash', () => { + expect(isProtectedDeleteKey('HKEY_CLASSES_ROOT\\CLSID')).toBe(true) + expect(isProtectedDeleteKey('hkcr\\clsid')).toBe(true) + expect(isProtectedDeleteKey('HKCR\\CLSID\\')).toBe(true) + }) + + it('allows genuine leaf keys', () => { + expect(isProtectedDeleteKey(`${APP_PATHS_ROOT}\\ghost.exe`)).toBe(false) + expect(isProtectedDeleteKey('HKCR\\CLSID\\{abc-def}')).toBe(false) + expect(isProtectedDeleteKey('HKLM\\SYSTEM\\CurrentControlSet\\Services\\SomeApp')).toBe(false) + }) +}) + +describe('fixRegistryEntries — protected key guard', () => { + const entry = (keyPath: string) => ({ + id: 'e1', + type: 'invalid' as const, + keyPath, + valueName: '(Default)', + issue: 'test', + risk: 'low' as const, + selected: true, + fix: { op: 'delete-key' as const }, + }) + + it('refuses to delete a container key and reports it as a failure', async () => { + const result = await fixRegistryEntries([entry(APP_PATHS_ROOT)] as any) + + expect(result.fixed).toBe(0) + expect(result.failed).toBe(1) + expect(result.failures[0].reason).toContain('protected registry key') + + const deletes = mockExecNative.mock.calls.filter(c => c[0] === 'reg' && c[1][0] === 'delete') + expect(deletes).toHaveLength(0) + }) + + it('still deletes a genuine leaf key', async () => { + const target = `${APP_PATHS_ROOT}\\ghost.exe` + const result = await fixRegistryEntries([entry(target)] as any) + + expect(result.fixed).toBe(1) + const deletes = mockExecNative.mock.calls.filter(c => c[0] === 'reg' && c[1][0] === 'delete') + expect(deletes).toHaveLength(1) + expect(deletes[0][1]).toEqual(['delete', target, '/f']) + }) +}) From 746af3b15b5cd5624317a1f3d55d614f15d7b4eb Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 27 Jul 2026 05:30:30 +0200 Subject: [PATCH 2/2] fix(registry): target the stale leaf in the TypeLib and OpenWithList scans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defects in scans this branch revives, both found in review. TypeLib offered a fix whose key was the whole HKCR\TypeLib\{GUID} parent. A GUID node holds every registered version and architecture — {00000300-...} carries both 2.8 and 6.0 on a stock install — so one missing win32 file would have deleted the surviving registrations. It now deletes the matched platform key alone, and ignores a GUID node with no version segment below it. OpenWithList stores the app in the value data under an ordinal value name (`a REG_SZ firefox.exe`), but the scan matched REG_SZ alone and stored the data as valueName, producing `reg delete /v firefox.exe` — a delete that can only fail. It now parses name and data separately, checks every value in the block rather than the first, and skips the MRUList ordering index. --- src/main/ipc/registry-cleaner.ipc.ts | 51 ++++++++---- src/main/ipc/registry-cleaner.scan.test.ts | 97 ++++++++++++++++++++++ 2 files changed, 130 insertions(+), 18 deletions(-) diff --git a/src/main/ipc/registry-cleaner.ipc.ts b/src/main/ipc/registry-cleaner.ipc.ts index 688dcc13..12a7f87c 100644 --- a/src/main/ipc/registry-cleaner.ipc.ts +++ b/src/main/ipc/registry-cleaner.ipc.ts @@ -402,27 +402,37 @@ export async function scanRegistry(signal?: AbortSignal): Promise= 30) break - const keyMatch = block.match(/^(HKCR\\TypeLib\\(\{[^}]+\})[^\r\n]*)/m) + // Group 3 is everything below the GUID, e.g. `\2.0\0\win32`. A GUID node + // holds every registered version and architecture of the type library + // ({00000300-…} carries both 2.8 and 6.0 on a stock machine), so the + // stale platform key is deleted on its own — deleting the GUID parent + // because one win32 file is missing would take the surviving versions + // with it. + const keyMatch = block.match(/^(HKCR\\TypeLib\\(\{[^}]+\})(\\[^\r\n]+))/m) const valMatch = block.match(/\(Default\)\s+REG_SZ\s+(.+)/i) if (keyMatch && valMatch) { const tlbPath = valMatch[1].trim().replace(/"/g, '') if (tlbPath && tlbPath.includes('\\') && !tlbPath.startsWith('%') && !existsSync(tlbPath)) { - const parentTypeLibKey = `HKCR\\TypeLib\\${keyMatch[2]}` entries.push({ id: randomUUID(), type: 'orphaned', @@ -679,7 +694,7 @@ export async function scanRegistry(signal?: AbortSignal): Promise { }) }) +describe('scanRegistry — TypeLib', () => { + // {00000300-…} carries both 2.8 and 6.0 on a stock Windows install. + const MULTI_VERSION_OUTPUT = [ + 'HKEY_CLASSES_ROOT\\TypeLib\\{00000300-0000-0010-8000-00AA006D2EA4}\\2.8\\0\\win32', + ' (Default) REG_SZ C:\\Program Files\\Gone\\old.tlb', + '', + 'HKEY_CLASSES_ROOT\\TypeLib\\{00000300-0000-0010-8000-00AA006D2EA4}\\6.0\\0\\win32', + ' (Default) REG_SZ C:\\Windows\\System32\\current.tlb', + '', + ].join('\r\n') + + it('deletes only the stale version key, never the GUID parent', async () => { + mockExecNative.mockImplementation(async (tool: string, args: string[]) => { + if (tool === 'reg' && args[0] === 'query' && args[1] === 'HKCR\\TypeLib') { + return { stdout: MULTI_VERSION_OUTPUT, stderr: '' } + } + if (tool === 'schtasks') throw new Error('no tasks') + return { stdout: '', stderr: '' } + }) + mockExistsSync.mockImplementation((p: string) => !String(p).includes('Gone')) + + const entries = await scanRegistry() + const tlb = entries.filter(e => e.issue.startsWith('Type library file missing')) + + expect(tlb).toHaveLength(1) + const target = tlb[0].fix?.key ?? tlb[0].keyPath + // Deleting the GUID parent would take the healthy 6.0 registration with it. + expect(target).toBe('HKCR\\TypeLib\\{00000300-0000-0010-8000-00AA006D2EA4}\\2.8\\0\\win32') + expect(target).not.toBe('HKCR\\TypeLib\\{00000300-0000-0010-8000-00AA006D2EA4}') + }) + + it('ignores a GUID node with no version segment below it', async () => { + mockExecNative.mockImplementation(async (tool: string, args: string[]) => { + if (tool === 'reg' && args[0] === 'query' && args[1] === 'HKCR\\TypeLib') { + return { + stdout: 'HKEY_CLASSES_ROOT\\TypeLib\\{00000300-0000-0010-8000-00AA006D2EA4}\r\n (Default) REG_SZ C:\\Gone\\x.tlb\r\n', + stderr: '', + } + } + if (tool === 'schtasks') throw new Error('no tasks') + return { stdout: '', stderr: '' } + }) + mockExistsSync.mockImplementation(() => false) + + const entries = await scanRegistry() + expect(entries.some(e => e.issue.startsWith('Type library file missing'))).toBe(false) + }) +}) + +describe('scanRegistry — OpenWithList', () => { + // Verbatim layout: the app is the value *data*, under an ordinal value *name*. + const OPEN_WITH_OUTPUT = [ + 'HKEY_CURRENT_USER\\SOFTWARE\\Microsoft\\Windows\\CurrentVersion\\Explorer\\FileExts\\.txt\\OpenWithList', + ' a REG_SZ notepad.exe', + ' b REG_SZ ghost.exe', + ' MRUList REG_SZ ab', + '', + ].join('\r\n') + + function stubOpenWith(): void { + mockExecNative.mockImplementation(async (tool: string, args: string[]) => { + if (tool === 'reg' && args[0] === 'query' && String(args[1]).endsWith('FileExts')) { + return { stdout: OPEN_WITH_OUTPUT, stderr: '' } + } + // notepad.exe is a registered App Path; ghost.exe is not. + if (tool === 'reg' && args[0] === 'query' && String(args[1]).includes('App Paths\\notepad.exe')) { + return { stdout: 'ok', stderr: '' } + } + if (tool === 'reg' && args[0] === 'query' && String(args[1]).includes('App Paths\\')) { + throw new Error('not found') + } + if (tool === 'schtasks') throw new Error('no tasks') + return { stdout: '', stderr: '' } + }) + } + + it('reports the ordinal value name, not the executable data', async () => { + stubOpenWith() + const entries = await scanRegistry() + const found = entries.filter(e => e.issue.startsWith('File association references unregistered app')) + + expect(found).toHaveLength(1) + // `reg delete /v ghost.exe` would always fail — the value is named "b". + expect(found[0].valueName).toBe('b') + expect(found[0].issue).toContain('ghost.exe') + }) + + it('leaves registered apps and the MRUList index alone', async () => { + stubOpenWith() + const entries = await scanRegistry() + const found = entries.filter(e => e.issue.startsWith('File association references unregistered app')) + + expect(found.some(e => e.valueName === 'a')).toBe(false) + expect(found.some(e => e.valueName.toLowerCase() === 'mrulist')).toBe(false) + }) +}) + describe('isProtectedDeleteKey', () => { it('blocks bare hive roots in both forms', () => { for (const k of ['HKLM', 'HKCU', 'HKCR', 'HKEY_LOCAL_MACHINE', 'HKEY_CLASSES_ROOT']) {