From 0be855d78c97e4e07ad15929260408ad2ab50ce1 Mon Sep 17 00:00:00 2001 From: Dave Date: Mon, 27 Jul 2026 05:26:30 +0200 Subject: [PATCH 1/2] fix(driver-manager): stop flagging installed drivers as stale duplicates The stale-driver scan grouped driver packages by provider + device class and declared every package in a group except the highest-versioned one stale, unless it was bound to a device that was present at scan time. Both halves of that are wrong, and the scan pre-selects its results, so a Scan then Clean pass ran `pnputil /delete-driver` over them. Provider + class is not a driver identity. Vendors ship several unrelated drivers in one class (Intel's Ethernet and Wi-Fi packages are both Net), so unrelated packages were treated as one another's versions. The originalName fallback chain ended at the provider name, which collapsed the identity further when pnputil reported no original INF name. "Not bound to a present device" is not evidence of staleness either. Virtual network adapters only materialise a device while they are up, so a mesh/VPN tunnel driver looks unbound whenever the client is idle; removable hardware looks unbound whenever it is unplugged. pnputil only refuses to delete a package whose device is present, so nothing stopped the removal. Group by the original INF name instead, keying packages that report none on themselves so they can never look like a duplicate, and only call a package superseded when the same INF has a strictly newer package that is actually bound to hardware. A driver nothing is bound to is now left alone, and a failed bound-driver query yields an empty stale list rather than marking everything stale. cleanDrivers re-checks bindings before removing, since the list it acts on can be minutes old. Fixes #242 --- src/main/ipc/driver-manager.ipc.test.ts | 355 ++++++++++++++++++++++++ src/main/ipc/driver-manager.ipc.ts | 197 +++++++++---- 2 files changed, 491 insertions(+), 61 deletions(-) create mode 100644 src/main/ipc/driver-manager.ipc.test.ts diff --git a/src/main/ipc/driver-manager.ipc.test.ts b/src/main/ipc/driver-manager.ipc.test.ts new file mode 100644 index 00000000..942b4107 --- /dev/null +++ b/src/main/ipc/driver-manager.ipc.test.ts @@ -0,0 +1,355 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest' + +// ── Mocks ── + +const { mockExecFile, mockExecNative } = vi.hoisted(() => ({ + mockExecFile: vi.fn(), + mockExecNative: vi.fn(), +})) + +vi.mock('electron', () => ({ + ipcMain: { handle: vi.fn() }, +})) + +vi.mock('child_process', () => { + const execFile = (...args: unknown[]): unknown => mockExecFile(...args) + // promisify(execFile) must yield { stdout, stderr } like the real one does. + ;(execFile as unknown as Record)[ + Symbol.for('nodejs.util.promisify.custom') + ] = (...args: unknown[]): unknown => mockExecFile(...args) + return { execFile } +}) + +vi.mock('../services/exec-utf8', () => ({ + psUtf8: (cmd: string) => cmd, + execNativeUtf8: (...args: unknown[]) => mockExecNative(...args), +})) + +import { + parseEnumDrivers, + driverIdentityKey, + findSupersededDrivers, + scanDrivers, + cleanDrivers, +} from './driver-manager.ipc' +import type { RawDriver } from './driver-manager.ipc' + +const originalPlatform = process.platform + +function setPlatform(p: string): void { + Object.defineProperty(process, 'platform', { value: p, configurable: true }) +} + +// ── Fixtures ── + +function driver(over: Partial & { publishedName: string }): RawDriver { + return { + originalName: over.publishedName, + provider: 'Unknown', + className: 'Unknown', + version: '1.0.0.0', + date: '01/01/2024', + signer: '', + ...over, + } +} + +/** Tailscale's tunnel adapter: installed and working, but no device is bound while the tunnel is down. */ +const WINTUN = driver({ + publishedName: 'oem12.inf', + originalName: 'wintun.inf', + provider: 'Tailscale Inc.', + className: 'Network adapters', + version: '0.14.1', +}) + +/** A second, unrelated driver from the same vendor in the same class. */ +const TAILSCALE_TAP = driver({ + publishedName: 'oem13.inf', + originalName: 'tailscale-tap.inf', + provider: 'Tailscale Inc.', + className: 'Network adapters', + version: '1.2.0', +}) + +/** Renders a driver as a pnputil `-e` block. */ +function enumBlock(d: RawDriver): string { + return [ + `Published Name: ${d.publishedName}`, + `Original Name: ${d.originalName}`, + `Provider Name: ${d.provider}`, + `Class Name: ${d.className}`, + `Driver Date: ${d.date}`, + `Driver Version: ${d.version}`, + `Signer Name: ${d.signer}`, + ].join('\n') +} + +function enumOutput(drivers: RawDriver[]): string { + return `Microsoft PnP Utility\n\n${drivers.map(enumBlock).join('\n\n')}\n` +} + +/** + * Route the two PowerShell queries scanDrivers makes: the DriverDatabase + * registry read (OEM → FileRepository folders) and the bound-driver query. + */ +function stubPowerShell(activeInfNames: string[]): void { + mockExecFile.mockImplementation(async (_file: string, args: string[]) => { + const script = args.join(' ') + if (script.includes('Win32_PnPSignedDriver')) { + return { stdout: activeInfNames.join('\n'), stderr: '' } + } + // DriverInfFiles registry read — no folder mapping needed for these tests. + return { stdout: '', stderr: '' } + }) +} + +beforeEach(() => { + mockExecFile.mockReset() + mockExecNative.mockReset() + setPlatform('win32') +}) + +afterEach(() => { + setPlatform(originalPlatform) +}) + +// ── parseEnumDrivers ── + +describe('parseEnumDrivers', () => { + it('extracts the original INF name', () => { + const [d] = parseEnumDrivers(enumOutput([WINTUN])) + expect(d.publishedName).toBe('oem12.inf') + expect(d.originalName).toBe('wintun.inf') + expect(d.provider).toBe('Tailscale Inc.') + expect(d.version).toBe('0.14.1') + }) + + it('falls back to the published name — never the provider — when Original Name is absent', () => { + const stdout = [ + 'Microsoft PnP Utility', + '', + 'Published Name: oem7.inf', + 'Driver Package Provider: Contoso Corp', + 'Class Name: Printers', + 'Driver Version: 3.0.0', + '', + ].join('\n') + const [d] = parseEnumDrivers(stdout) + // Grouping keys off originalName; a provider shared by every package a + // vendor ships would collapse unrelated drivers into one group. + expect(d.originalName).toBe('oem7.inf') + expect(d.provider).toBe('Contoso Corp') + }) + + it('splits the combined "Driver date and version" field used on Windows 11 24H2+', () => { + const stdout = [ + 'Published Name: oem3.inf', + 'Original Name: nvlddmkm.inf', + 'Class Name: Display adapters', + 'Driver date and version: 07/18/2024 32.0.15.6094', + '', + ].join('\n') + const [d] = parseEnumDrivers(stdout) + expect(d.date).toBe('07/18/2024') + expect(d.version).toBe('32.0.15.6094') + }) + + it('ignores non-OEM packages', () => { + const stdout = 'Published Name: usbstor.inf\nClass Name: USB\n' + expect(parseEnumDrivers(stdout)).toEqual([]) + }) +}) + +// ── driverIdentityKey ── + +describe('driverIdentityKey', () => { + it('keys on the original INF name', () => { + expect(driverIdentityKey(WINTUN)).toBe('inf:wintun.inf') + }) + + it('gives distinct keys to different INFs from the same vendor and class', () => { + expect(driverIdentityKey(WINTUN)).not.toBe(driverIdentityKey(TAILSCALE_TAP)) + }) + + it('keys a package on itself when no original INF name is reported', () => { + const d = driver({ publishedName: 'oem7.inf', provider: 'Contoso Corp' }) + expect(driverIdentityKey(d)).toBe('pkg:oem7.inf') + }) +}) + +// ── findSupersededDrivers ── + +describe('findSupersededDrivers', () => { + it('removes an older copy that a newer, bound copy of the same INF replaced', () => { + const old = driver({ publishedName: 'oem20.inf', originalName: 'nvlddmkm.inf', version: '31.0.15.3623' }) + const current = driver({ publishedName: 'oem21.inf', originalName: 'nvlddmkm.inf', version: '32.0.15.6094' }) + const result = findSupersededDrivers([old, current], new Set(['oem21.inf'])) + expect([...result]).toEqual(['oem20.inf']) + }) + + it('never marks the bound copy itself as superseded', () => { + const old = driver({ publishedName: 'oem20.inf', originalName: 'nvlddmkm.inf', version: '31.0' }) + const current = driver({ publishedName: 'oem21.inf', originalName: 'nvlddmkm.inf', version: '32.0' }) + expect(findSupersededDrivers([old, current], new Set(['oem21.inf'])).has('oem21.inf')).toBe(false) + }) + + // ── Regression: issue #242 ── + + it('keeps an unbound virtual network adapter driver (#242)', () => { + // Tailscale's Wintun adapter only exists while the tunnel is up, so its + // driver is unbound at scan time. It is still the only copy installed. + expect(findSupersededDrivers([WINTUN], new Set()).size).toBe(0) + }) + + it('does not treat two different INFs from one vendor and class as versions of each other (#242)', () => { + // The old grouping key was provider + class, which made the lower-versioned + // package look like a stale duplicate of the higher-versioned one. + const result = findSupersededDrivers([WINTUN, TAILSCALE_TAP], new Set(['oem13.inf'])) + expect(result.size).toBe(0) + }) + + it('keeps unrelated Intel Net drivers grouped only by vendor and class', () => { + const ethernet = driver({ + publishedName: 'oem30.inf', originalName: 'e1d68x64.inf', + provider: 'Intel', className: 'Net', version: '12.19.2.45', + }) + const wifi = driver({ + publishedName: 'oem31.inf', originalName: 'netwtw10.inf', + provider: 'Intel', className: 'Net', version: '23.40.0.7', + }) + expect(findSupersededDrivers([ethernet, wifi], new Set(['oem31.inf'])).size).toBe(0) + }) + + it('keeps both copies when no copy of the driver is bound to hardware', () => { + // An unplugged printer or dock: two versions installed, neither bound. + // Nothing proves either was superseded, so neither is removable. + const old = driver({ publishedName: 'oem40.inf', originalName: 'prn.inf', version: '1.0' }) + const newer = driver({ publishedName: 'oem41.inf', originalName: 'prn.inf', version: '2.0' }) + expect(findSupersededDrivers([old, newer], new Set()).size).toBe(0) + }) + + it('keeps everything when the bound-driver query returns nothing', () => { + // getActiveDriverNames() yields an empty set when both WMI and pnputil + // fail. Failing to an empty stale list is the safe direction. + const old = driver({ publishedName: 'oem40.inf', originalName: 'prn.inf', version: '1.0' }) + const newer = driver({ publishedName: 'oem41.inf', originalName: 'prn.inf', version: '2.0' }) + expect(findSupersededDrivers([old, newer], new Set()).size).toBe(0) + }) + + it('keeps copies with equal versions', () => { + const a = driver({ publishedName: 'oem50.inf', originalName: 'foo.inf', version: '1.0.0' }) + const b = driver({ publishedName: 'oem51.inf', originalName: 'foo.inf', version: '1.0.0' }) + expect(findSupersededDrivers([a, b], new Set(['oem51.inf'])).size).toBe(0) + }) + + it('keeps copies whose versions cannot be parsed', () => { + const a = driver({ publishedName: 'oem50.inf', originalName: 'foo.inf', version: '' }) + const b = driver({ publishedName: 'oem51.inf', originalName: 'foo.inf', version: '' }) + expect(findSupersededDrivers([a, b], new Set(['oem51.inf'])).size).toBe(0) + }) + + it('does not remove a copy that is newer than the bound one', () => { + const staged = driver({ publishedName: 'oem60.inf', originalName: 'foo.inf', version: '3.0' }) + const bound = driver({ publishedName: 'oem61.inf', originalName: 'foo.inf', version: '2.0' }) + expect(findSupersededDrivers([staged, bound], new Set(['oem61.inf'])).size).toBe(0) + }) + + it('anchors on the newest bound copy when several are bound', () => { + const oldest = driver({ publishedName: 'oem70.inf', originalName: 'foo.inf', version: '1.0' }) + const middle = driver({ publishedName: 'oem71.inf', originalName: 'foo.inf', version: '2.0' }) + const newest = driver({ publishedName: 'oem72.inf', originalName: 'foo.inf', version: '3.0' }) + const result = findSupersededDrivers([oldest, middle, newest], new Set(['oem71.inf', 'oem72.inf'])) + expect([...result]).toEqual(['oem70.inf']) + }) + + it('never groups packages that report no original INF name', () => { + const a = driver({ publishedName: 'oem80.inf', provider: 'Contoso', className: 'Net', version: '1.0' }) + const b = driver({ publishedName: 'oem81.inf', provider: 'Contoso', className: 'Net', version: '2.0' }) + expect(findSupersededDrivers([a, b], new Set(['oem81.inf'])).size).toBe(0) + }) +}) + +// ── scanDrivers ── + +describe('scanDrivers', () => { + it('returns an empty result off Windows', async () => { + setPlatform('linux') + const result = await scanDrivers() + expect(result).toEqual({ packages: [], totalStaleSize: 0, totalStaleCount: 0, totalCurrentCount: 0 }) + expect(mockExecNative).not.toHaveBeenCalled() + }) + + it('reports no stale packages for an idle Tailscale install (#242)', async () => { + mockExecNative.mockResolvedValue({ stdout: enumOutput([WINTUN, TAILSCALE_TAP]), stderr: '' }) + stubPowerShell([]) // tunnel is down — neither adapter is present + + const result = await scanDrivers() + + expect(result.totalStaleCount).toBe(0) + expect(result.totalCurrentCount).toBe(2) + expect(result.packages.every((p) => p.isCurrent && !p.selected)).toBe(true) + }) + + it('pre-selects only genuinely superseded packages', async () => { + const old = driver({ publishedName: 'oem20.inf', originalName: 'nvlddmkm.inf', provider: 'NVIDIA', className: 'Display', version: '31.0.15.3623' }) + const current = driver({ publishedName: 'oem21.inf', originalName: 'nvlddmkm.inf', provider: 'NVIDIA', className: 'Display', version: '32.0.15.6094' }) + mockExecNative.mockResolvedValue({ stdout: enumOutput([old, current, WINTUN]), stderr: '' }) + stubPowerShell(['oem21.inf']) + + const result = await scanDrivers() + + expect(result.totalStaleCount).toBe(1) + const selected = result.packages.filter((p) => p.selected) + expect(selected.map((p) => p.publishedName)).toEqual(['oem20.inf']) + }) + + it('reports every package as current when driver enumeration yields nothing', async () => { + mockExecNative.mockResolvedValue({ stdout: '', stderr: '' }) + stubPowerShell([]) + const result = await scanDrivers() + expect(result.packages).toEqual([]) + }) +}) + +// ── cleanDrivers ── + +describe('cleanDrivers', () => { + it('refuses a driver that is bound to a device', async () => { + stubPowerShell(['oem12.inf']) + + const result = await cleanDrivers(['oem12.inf']) + + expect(result.removed).toBe(0) + expect(result.failed).toBe(1) + expect(result.errors[0].reason).toMatch(/in use/i) + expect(mockExecNative).not.toHaveBeenCalledWith('pnputil', ['/delete-driver', 'oem12.inf'], expect.anything()) + }) + + it('rejects names that are not oem*.inf packages', async () => { + stubPowerShell([]) + + const result = await cleanDrivers(['..\\..\\windows\\system32\\drivers\\tcpip.sys']) + + expect(result.failed).toBe(1) + expect(result.errors[0].reason).toBe('Invalid driver package name') + expect(mockExecNative).not.toHaveBeenCalled() + }) + + it('removes an unbound package', async () => { + stubPowerShell(['oem21.inf']) + mockExecNative.mockResolvedValue({ stdout: '', stderr: '' }) + + const result = await cleanDrivers(['oem20.inf']) + + expect(result.removed).toBe(1) + expect(result.failed).toBe(0) + expect(mockExecNative).toHaveBeenCalledWith('pnputil', ['/delete-driver', 'oem20.inf'], expect.anything()) + }) + + it('returns an empty result off Windows', async () => { + setPlatform('linux') + const result = await cleanDrivers(['oem20.inf']) + expect(result).toEqual({ removed: 0, failed: 0, spaceRecovered: 0, errors: [] }) + }) +}) diff --git a/src/main/ipc/driver-manager.ipc.ts b/src/main/ipc/driver-manager.ipc.ts index e8931490..05f14cd2 100644 --- a/src/main/ipc/driver-manager.ipc.ts +++ b/src/main/ipc/driver-manager.ipc.ts @@ -75,7 +75,7 @@ function compareVersions(a: string, b: string): number { return 0 } -interface RawDriver { +export interface RawDriver { publishedName: string originalName: string provider: string @@ -90,7 +90,7 @@ interface RawDriver { * Handles both modern and legacy field names, including the combined * "Driver date and version" field found on Windows 11 24H2+. */ -function parseEnumDrivers(stdout: string): RawDriver[] { +export function parseEnumDrivers(stdout: string): RawDriver[] { const drivers: RawDriver[] = [] // Split into blocks separated by blank lines const blocks = stdout.split(/\n\s*\n/) @@ -128,10 +128,12 @@ function parseEnumDrivers(stdout: string): RawDriver[] { drivers.push({ publishedName, + // Never fall back to the provider here: originalName is the package's + // identity for duplicate detection, and a provider name shared by every + // driver a vendor ships is the opposite of an identity. originalName: fields['original name'] || fields['original inf'] || - fields['driver package provider'] || publishedName, provider: fields['driver package provider'] || @@ -155,6 +157,86 @@ function parseEnumDrivers(stdout: string): RawDriver[] { return drivers } +/** + * Identity of a driver package for duplicate detection: the original INF name + * (e.g. "wintun.inf"). Two packages are versions of *the same driver* only when + * they came from the same INF. + * + * Provider + device class is not an identity. Vendors routinely ship several + * unrelated drivers in one class — Intel's Ethernet and Wi-Fi packages are both + * class "Net" — and treating them as one another's versions marks working + * drivers as stale. + * + * When pnputil reports no original name the package is keyed on itself, so it + * can never be considered a superseded copy of anything. + */ +export function driverIdentityKey(d: RawDriver): string { + const published = d.publishedName.trim().toLowerCase() + const original = d.originalName.trim().toLowerCase() + if (original.endsWith('.inf') && original !== published) return `inf:${original}` + return `pkg:${published}` +} + +/** + * Identify driver packages that a newer copy of the same driver has replaced. + * Returns the set of published names (lowercased) that are safe to remove. + * + * A package is only superseded when the same INF has another package that is + * both strictly newer AND actually bound to hardware. Everything else is left + * alone — in particular, a package that is not bound to any device is NOT + * evidence of staleness: + * + * - Virtual network adapters (Tailscale/Wintun, WireGuard, VPN tunnels) only + * materialise a device while the tunnel is up, so their driver looks + * unbound whenever the app is idle. Deleting it leaves the client unable to + * create its adapter — the failure reported in #242. + * - Removable hardware — docks, printers, phones, dongles — is unbound + * whenever it is unplugged. + * + * If the active-driver query fails entirely the anchor set is empty, nothing is + * superseded, and the scan reports no stale packages. That is the safe + * direction to fail in. + */ +export function findSupersededDrivers( + rawDrivers: RawDriver[], + activeNames: Set +): Set { + const groups = new Map() + for (const d of rawDrivers) { + const key = driverIdentityKey(d) + const group = groups.get(key) + if (group) group.push(d) + else groups.set(key, [d]) + } + + const superseded = new Set() + + for (const group of groups.values()) { + if (group.length < 2) continue + + // Anchor: the highest-versioned package of this driver that Windows has + // actually bound to a device. Without one there is no proof any copy was + // replaced, so the whole group stays. + let anchor: RawDriver | null = null + for (const d of group) { + if (!activeNames.has(d.publishedName.toLowerCase())) continue + if (!anchor || compareVersions(d.version, anchor.version) > 0) anchor = d + } + if (!anchor) continue + + for (const d of group) { + if (activeNames.has(d.publishedName.toLowerCase())) continue + // Strictly older than the bound copy. Equal, missing or unparseable + // versions compare as 0 and are left alone rather than guessed at. + if (compareVersions(anchor.version, d.version) > 0) { + superseded.add(d.publishedName.toLowerCase()) + } + } + } + + return superseded +} + /** * Build a mapping from OEM published name (e.g. "oem7.inf") to FileRepository * folder names by reading the DriverDatabase registry. Each oem*.inf maps to @@ -297,69 +379,50 @@ export async function scanDrivers( getOemFolderMap() ]) - // Step 3: Group by provider + class to find duplicates (since legacy pnputil - // doesn't expose the original inf name, we use provider+class as the grouping key) - const groups = new Map() - for (const d of rawDrivers) { - const key = `${d.provider.toLowerCase()}::${d.className.toLowerCase()}` - const group = groups.get(key) || [] - group.push(d) - groups.set(key, group) - } + // Step 3: Identify packages that a newer, actively-bound copy of the same + // driver has replaced. Everything else counts as current and is never + // offered for removal. + const superseded = findSupersededDrivers(rawDrivers, activeNames) - // Within each group, mark the newest as current; the rest are stale - // Also mark any driver actively bound to hardware as current const packages: DriverPackage[] = [] let idx = 0 - for (const [, group] of groups) { - // Sort by version descending using numeric comparison - group.sort((a, b) => compareVersions(b.version, a.version)) - - for (let i = 0; i < group.length; i++) { - const d = group[i] - const isActive = activeNames.has(d.publishedName.toLowerCase()) - const isNewest = i === 0 - - onProgress?.({ - phase: 'measuring', - current: ++idx, - total: rawDrivers.length, - currentDriver: `${d.provider} - ${d.className} (${d.version})` - }) - - // Find folder in FileRepository using registry-based OEM→folder mapping - let folderPath = '' - let size = 0 - try { - const folders = oemFolderMap.get(d.publishedName.toLowerCase()) || [] - if (folders.length > 0) { - // Use the first (and usually only) matching folder - folderPath = join(DRIVER_STORE, folders[0]) - size = dirSize(folderPath) - } - } catch { /* skip */ } - - packages.push({ - id: makeId(d.publishedName, d.version), - publishedName: d.publishedName, - originalName: d.originalName, - provider: d.provider, - className: d.className, - version: d.version, - date: d.date, - signer: d.signer, - folderPath, - size, - isCurrent: isActive || isNewest, - selected: false - }) - } - } + for (const d of rawDrivers) { + onProgress?.({ + phase: 'measuring', + current: ++idx, + total: rawDrivers.length, + currentDriver: `${d.provider} - ${d.className} (${d.version})` + }) - // Pre-select stale (non-current) drivers - for (const pkg of packages) { - if (!pkg.isCurrent) pkg.selected = true + // Find folder in FileRepository using registry-based OEM→folder mapping + let folderPath = '' + let size = 0 + try { + const folders = oemFolderMap.get(d.publishedName.toLowerCase()) || [] + if (folders.length > 0) { + // Use the first (and usually only) matching folder + folderPath = join(DRIVER_STORE, folders[0]) + size = dirSize(folderPath) + } + } catch { /* skip */ } + + const isStale = superseded.has(d.publishedName.toLowerCase()) + + packages.push({ + id: makeId(d.publishedName, d.version), + publishedName: d.publishedName, + originalName: d.originalName, + provider: d.provider, + className: d.className, + version: d.version, + date: d.date, + signer: d.signer, + folderPath, + size, + isCurrent: !isStale, + selected: isStale + }) } const stale = packages.filter((p) => !p.isCurrent) @@ -384,6 +447,12 @@ export async function cleanDrivers(publishedNames: string[]): Promise Date: Mon, 27 Jul 2026 05:38:51 +0200 Subject: [PATCH 2/2] fix(driver-manager): key driver identity on publisher and reject unknown versions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two gaps in the supersession rule, both from review. The original INF name is a filename, not a globally unique id: generic names such as driver.inf ship from more than one vendor, so keying on it alone could still group two unrelated packages and mark the unbound one stale. Include the publisher in the key, and fall back to keying a package on itself when either half is missing. compareVersions() reads an absent or non-numeric version as zero, so a bound package at "2.0" ordered above a package whose version pnputil did not report and marked it superseded — the opposite of the documented fail-safe. Only order two packages when both versions are readable dotted numbers, on both the anchor and the candidate. --- src/main/ipc/driver-manager.ipc.test.ts | 56 +++++++++++++++++++++++-- src/main/ipc/driver-manager.ipc.ts | 38 +++++++++++++---- 2 files changed, 82 insertions(+), 12 deletions(-) diff --git a/src/main/ipc/driver-manager.ipc.test.ts b/src/main/ipc/driver-manager.ipc.test.ts index 942b4107..e3c72569 100644 --- a/src/main/ipc/driver-manager.ipc.test.ts +++ b/src/main/ipc/driver-manager.ipc.test.ts @@ -45,7 +45,7 @@ function setPlatform(p: string): void { function driver(over: Partial & { publishedName: string }): RawDriver { return { originalName: over.publishedName, - provider: 'Unknown', + provider: 'Contoso', className: 'Unknown', version: '1.0.0.0', date: '01/01/2024', @@ -164,18 +164,30 @@ describe('parseEnumDrivers', () => { // ── driverIdentityKey ── describe('driverIdentityKey', () => { - it('keys on the original INF name', () => { - expect(driverIdentityKey(WINTUN)).toBe('inf:wintun.inf') + it('keys on the original INF name and its publisher', () => { + expect(driverIdentityKey(WINTUN)).toBe('inf:wintun.inf|tailscale inc.') }) it('gives distinct keys to different INFs from the same vendor and class', () => { expect(driverIdentityKey(WINTUN)).not.toBe(driverIdentityKey(TAILSCALE_TAP)) }) + it('gives distinct keys to the same INF filename from different vendors', () => { + // "driver.inf" and friends are filenames, not globally unique ids. + const a = driver({ publishedName: 'oem90.inf', originalName: 'driver.inf', provider: 'Contoso' }) + const b = driver({ publishedName: 'oem91.inf', originalName: 'driver.inf', provider: 'Fabrikam' }) + expect(driverIdentityKey(a)).not.toBe(driverIdentityKey(b)) + }) + it('keys a package on itself when no original INF name is reported', () => { const d = driver({ publishedName: 'oem7.inf', provider: 'Contoso Corp' }) expect(driverIdentityKey(d)).toBe('pkg:oem7.inf') }) + + it('keys a package on itself when the publisher is unknown', () => { + const d = driver({ publishedName: 'oem8.inf', originalName: 'foo.inf', provider: 'Unknown' }) + expect(driverIdentityKey(d)).toBe('pkg:oem8.inf') + }) }) // ── findSupersededDrivers ── @@ -249,6 +261,26 @@ describe('findSupersededDrivers', () => { expect(findSupersededDrivers([a, b], new Set(['oem51.inf'])).size).toBe(0) }) + it('keeps a copy with no version against a bound copy that has one', () => { + // compareVersions() reads a missing version as 0, so without an explicit + // guard "2.0" would order above "" and delete an unverified package. + const unknown = driver({ publishedName: 'oem52.inf', originalName: 'foo.inf', version: '' }) + const bound = driver({ publishedName: 'oem53.inf', originalName: 'foo.inf', version: '2.0' }) + expect(findSupersededDrivers([unknown, bound], new Set(['oem53.inf'])).size).toBe(0) + }) + + it('keeps a copy with a non-numeric version against a bound copy', () => { + const unknown = driver({ publishedName: 'oem54.inf', originalName: 'foo.inf', version: 'n/a' }) + const bound = driver({ publishedName: 'oem55.inf', originalName: 'foo.inf', version: '2.0' }) + expect(findSupersededDrivers([unknown, bound], new Set(['oem55.inf'])).size).toBe(0) + }) + + it('does not anchor on a bound copy whose version cannot be read', () => { + const old = driver({ publishedName: 'oem56.inf', originalName: 'foo.inf', version: '1.0' }) + const bound = driver({ publishedName: 'oem57.inf', originalName: 'foo.inf', version: '' }) + expect(findSupersededDrivers([old, bound], new Set(['oem57.inf'])).size).toBe(0) + }) + it('does not remove a copy that is newer than the bound one', () => { const staged = driver({ publishedName: 'oem60.inf', originalName: 'foo.inf', version: '3.0' }) const bound = driver({ publishedName: 'oem61.inf', originalName: 'foo.inf', version: '2.0' }) @@ -268,6 +300,24 @@ describe('findSupersededDrivers', () => { const b = driver({ publishedName: 'oem81.inf', provider: 'Contoso', className: 'Net', version: '2.0' }) expect(findSupersededDrivers([a, b], new Set(['oem81.inf'])).size).toBe(0) }) + + it('does not treat one vendor\'s INF as a version of another vendor\'s same-named INF', () => { + const contoso = driver({ + publishedName: 'oem90.inf', originalName: 'driver.inf', + provider: 'Contoso', className: 'Net', version: '1.0', + }) + const fabrikam = driver({ + publishedName: 'oem91.inf', originalName: 'driver.inf', + provider: 'Fabrikam', className: 'Net', version: '2.0', + }) + expect(findSupersededDrivers([contoso, fabrikam], new Set(['oem91.inf'])).size).toBe(0) + }) + + it('never groups packages whose publisher is unknown', () => { + const a = driver({ publishedName: 'oem92.inf', originalName: 'foo.inf', provider: 'Unknown', version: '1.0' }) + const b = driver({ publishedName: 'oem93.inf', originalName: 'foo.inf', provider: 'Unknown', version: '2.0' }) + expect(findSupersededDrivers([a, b], new Set(['oem93.inf'])).size).toBe(0) + }) }) // ── scanDrivers ── diff --git a/src/main/ipc/driver-manager.ipc.ts b/src/main/ipc/driver-manager.ipc.ts index 05f14cd2..02107d31 100644 --- a/src/main/ipc/driver-manager.ipc.ts +++ b/src/main/ipc/driver-manager.ipc.ts @@ -157,23 +157,35 @@ export function parseEnumDrivers(stdout: string): RawDriver[] { return drivers } +/** A dotted numeric version we can order against another one. */ +function isComparableVersion(version: string): boolean { + return /^\d+(\.\d+)*$/.test(version.trim()) +} + /** * Identity of a driver package for duplicate detection: the original INF name - * (e.g. "wintun.inf"). Two packages are versions of *the same driver* only when - * they came from the same INF. + * plus its publisher (e.g. "wintun.inf" from "Tailscale Inc."). Two packages + * are versions of *the same driver* only when both halves match. * - * Provider + device class is not an identity. Vendors routinely ship several + * The INF name alone is not enough — it is a filename, not a globally unique + * id, and generic names like "driver.inf" ship from more than one vendor. + * + * Provider + device class is not enough either. Vendors routinely ship several * unrelated drivers in one class — Intel's Ethernet and Wi-Fi packages are both * class "Net" — and treating them as one another's versions marks working * drivers as stale. * - * When pnputil reports no original name the package is keyed on itself, so it - * can never be considered a superseded copy of anything. + * When either half is missing there is no identity to establish, so the package + * is keyed on itself and can never be considered a superseded copy of anything. */ export function driverIdentityKey(d: RawDriver): string { const published = d.publishedName.trim().toLowerCase() const original = d.originalName.trim().toLowerCase() - if (original.endsWith('.inf') && original !== published) return `inf:${original}` + // 'Unknown' is what parseEnumDrivers substitutes for an absent provider. + const provider = d.provider.trim().toLowerCase() + const hasOriginal = original.endsWith('.inf') && original !== published + const hasProvider = provider !== '' && provider !== 'unknown' + if (hasOriginal && hasProvider) return `inf:${original}|${provider}` return `pkg:${published}` } @@ -193,6 +205,10 @@ export function driverIdentityKey(d: RawDriver): string { * - Removable hardware — docks, printers, phones, dongles — is unbound * whenever it is unplugged. * + * Ordering only happens between two packages whose versions are both readable + * dotted numbers. compareVersions() reads an absent or non-numeric version as + * zero, which would make every known version look newer than it. + * * If the active-driver query fails entirely the anchor set is empty, nothing is * superseded, and the scan reports no stale packages. That is the safe * direction to fail in. @@ -216,18 +232,22 @@ export function findSupersededDrivers( // Anchor: the highest-versioned package of this driver that Windows has // actually bound to a device. Without one there is no proof any copy was - // replaced, so the whole group stays. + // replaced, so the whole group stays. A bound package whose version we + // cannot read can't prove anything is older than it, so it is not an + // anchor either. let anchor: RawDriver | null = null for (const d of group) { if (!activeNames.has(d.publishedName.toLowerCase())) continue + if (!isComparableVersion(d.version)) continue if (!anchor || compareVersions(d.version, anchor.version) > 0) anchor = d } if (!anchor) continue for (const d of group) { if (activeNames.has(d.publishedName.toLowerCase())) continue - // Strictly older than the bound copy. Equal, missing or unparseable - // versions compare as 0 and are left alone rather than guessed at. + // Unknown version — leave it alone rather than guess at its age. + if (!isComparableVersion(d.version)) continue + // Strictly older than the bound copy. Equal versions are left alone. if (compareVersions(anchor.version, d.version) > 0) { superseded.add(d.publishedName.toLowerCase()) }