Rewrite windowsupdate package to fix COM memory leaks

Makefile

@@ -233,7 +233,7 @@ test: generate
 
 # -run=^$ will never match any of our regular non-benchmark tests, ensuring those don't run during benchmarking
 test-bench-tables: generate
-	go test ./ee/tables/... ./pkg/windows/windowsupdate/... ./pkg/osquery/table/... -bench=. -count=20 -run=^$ -benchmem
+	go test ./ee/tables/... ./ee/windowsupdate/... ./pkg/osquery/table/... -bench=. -count=20 -run=^$ -benchmem
 
 ##
 ## Lint

cmd/launcher/query_windowsupdates_windows.go

@@ -11,8 +11,8 @@ import (
 
 	comshim "github.com/NozomiNetworks/go-comshim"
 	"github.com/kolide/launcher/v2/ee/tables/windowsupdatetable"
+	"github.com/kolide/launcher/v2/ee/windowsupdate"
 	"github.com/kolide/launcher/v2/pkg/log/multislogger"
-	"github.com/kolide/launcher/v2/pkg/windows/windowsupdate"
 	"github.com/peterbourgon/ff/v3"
 )
 

ee/tables/windowsupdatetable/windowsupdate.go

@@ -18,7 +18,7 @@ import (
 	"github.com/kolide/launcher/v2/ee/tables/dataflattentable"
 	"github.com/kolide/launcher/v2/ee/tables/tablehelpers"
 	"github.com/kolide/launcher/v2/ee/tables/tablewrapper"
-	"github.com/kolide/launcher/v2/pkg/windows/windowsupdate"
+	"github.com/kolide/launcher/v2/ee/windowsupdate"
 	"github.com/osquery/osquery-go/plugin/table"
 )
 

ee/windowsupdate/COM_LEAK_ANALYSIS-2026-04.md

@@ -0,0 +1,105 @@
+# COM Memory Leak Analysis: windowsupdate package
+
+## Background
+
+The `pkg/windows/windowsupdate` package provides a Go binding for the Windows
+Update Agent API using go-ole. It derives from
+https://github.com/ceshihao/windowsupdate (which has the same bugs).
+
+A memory leak was observed when calling `IUpdateSearcher.Search` in-process.
+The leak was severe enough that a workaround was added: run the query in a
+subprocess (`launcher query-windowsupdates`) so the leaked memory is reclaimed
+on process exit. See `ee/tables/windowsupdatetable/windowsupdate.go` and
+`cmd/launcher/query_windowsupdates_windows.go`.
+
+## Root Cause
+
+The leak is caused by **missing COM reference cleanup** throughout the package.
+There are two categories:
+
+### 1. VARIANTs never cleared
+
+Every call to `oleutil.GetProperty` or `oleutil.CallMethod` returns a
+`*ole.VARIANT`. The VARIANT owns a reference to the underlying COM data (a
+BSTR for strings, an IDispatch for objects, etc.). The caller must call
+`result.Clear()` when done, which calls the Win32 `VariantClear` function to
+free that reference.
+
+The package used an `oleconv` helper layer (`pkg/windows/oleconv`) that
+consumed the VARIANT, extracted the Go value, and returned -- but **never
+called `Clear()`**. Since every property access on every COM object went through
+oleconv, this leaked on every single `GetProperty` call. For a search returning
+200 updates with ~40 properties each, that's ~8000+ leaked VARIANTs per query.
+
+### 2. IDispatch pointers never Released
+
+Every struct (`IUpdateSession`, `IUpdateSearcher`, `ISearchResult`, `IUpdate`,
+`ICategory`, `IUpdateHistoryEntry`, etc.) stored a `disp *ole.IDispatch` field
+but never called `Release()` on it. Additionally, intermediate IDispatch
+pointers from collection iteration (e.g. the IUpdateCollection dispatch from
+`GetProperty(searchResult, "Updates")`) were used and then abandoned.
+
+### 3. IUnknown from CreateObject never Released
+
+`oleutil.CreateObject` returns an `*ole.IUnknown` with refcount 1.
+`QueryInterface(IID_IDispatch)` adds another reference. The original IUnknown
+was never Released, leaking one reference per session creation. (This was fixed
+in the first pass of this investigation.)
+
+## What about STA vs MTA?
+
+The go-comshim library initializes COM in MTA (Multi-Threaded Apartment) mode.
+A note from a Windows developer suggested the leak might be related to
+STA/MTA threading and IUnknown reference count bugs (off-by-one, "0 vs 1").
+
+Investigation found that `Microsoft.Update.Session` (CLSID
+`{4CB43D7F-7EEE-4906-8698-60DA1C38F2FE}`) is registered with
+`ThreadingModel = "Both"`, meaning it works in either STA or MTA. So apartment
+model mismatch is **not** the root cause for this specific COM class.
+
+However, go-ole does have documented apartment-sensitive leak behavior
+(https://github.com/go-ole/go-ole/issues/135), so apartment model can be a
+contributing factor in general.
+
+## The oleconv problem
+
+`oleconv` (`pkg/windows/oleconv/oleconv.go`) was a thin wrapper that took a
+`(*ole.VARIANT, error)` tuple from oleutil and returned a typed Go value. It
+existed only to make property access a one-liner:
+
+    oleconv.ToStringErr(oleutil.GetProperty(disp, "Title"))
+
+The problem: it consumed the VARIANT and discarded it, making cleanup
+impossible at the call site. The VARIANT couldn't be `Clear()`'d because it
+was already lost. oleconv had no other consumers outside the windowsupdate
+package.
+
+## The fix: drop oleconv, follow wmi.go pattern
+
+The `ee/wmi/wmi.go` package in this same codebase does COM correctly:
+- `defer unknown.Release()` after CreateObject
+- `defer disp.Release()` after QueryInterface
+- `defer raw.Clear()` after GetProperty/CallMethod
+- Does NOT call Release on an IDispatch obtained via ToIDispatch(), because
+  ToIDispatch() is just a cast to the same memory; Clear() handles it
+
+The fix rewrites the windowsupdate package to follow this same pattern:
+- Work with raw `*ole.VARIANT` directly
+- `defer variant.Clear()` immediately after receiving it
+- Extract the Go value from the variant after deferring the clear
+- `defer disp.Release()` in each `toI*` function for the dispatch being consumed
+- Remove the `disp` field from structs (it's never used after construction
+  in the query path)
+- Delete oleconv entirely
+
+## Key references
+
+- COM refcount rules: https://learn.microsoft.com/en-us/windows/win32/com/rules-for-managing-reference-counts
+- IUnknown: https://learn.microsoft.com/en-us/windows/win32/com/using-and-implementing-iunknown
+- CoInitializeEx (STA/MTA): https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializeex
+- go-ole issue #135 (apartment-sensitive leak): https://github.com/go-ole/go-ole/issues/135
+- go-ole IUnknown: https://github.com/go-ole/go-ole/blob/master/iunknown.go
+- Upstream library (same bugs): https://github.com/ceshihao/windowsupdate
+- wmi.go arm64 panic note: ToIDispatch() is a cast, not a new reference;
+  calling Release() on the IDispatch AND Clear() on the VARIANT double-frees
+  and panics on arm64. Clear() the VARIANT only.

ee/windowsupdate/docs.go

@@ -0,0 +1,28 @@
+// Package windowsupdate provides a go-ole interface to the windows
+// update agent.
+//
+// This code derives from https://github.com/ceshihao/windowsupdate
+//
+// # COM lifecycle management
+//
+// This package follows the same COM cleanup pattern as ee/wmi/wmi.go:
+//
+//   - oleutil.GetProperty / CallMethod return a *ole.VARIANT. We call
+//     defer v.Clear() immediately to free the underlying COM reference
+//     (BSTR, IDispatch, etc.) via Win32 VariantClear.
+//   - For IDispatch results, we do NOT clear the VARIANT because
+//     ToIDispatch() is a pointer cast into the same memory. Instead,
+//     the caller calls Release() on the IDispatch when done. See the
+//     arm64 panic note in ee/wmi/wmi.go for why this matters.
+//   - CreateObject returns an IUnknown with refcount 1. QueryInterface
+//     adds another ref. We defer unknown.Release() immediately.
+//   - Each toI* function receives an IDispatch and (for types that
+//     are fully consumed during construction) calls defer disp.Release().
+//
+// The helper functions in olehelpers.go centralize the VARIANT
+// extraction + Clear() pattern so individual files don't need to
+// repeat it.
+//
+// For background on the memory leak this addresses, see
+// COM_LEAK_ANALYSIS.md in this directory.
+package windowsupdate

ee/windowsupdate/icategory.go

@@ -0,0 +1,97 @@
+package windowsupdate
+
+import (
+	"fmt"
+
+	"github.com/go-ole/go-ole"
+)
+
+// ICategory represents the category to which an update belongs.
+// https://docs.microsoft.com/en-us/windows/win32/api/wuapi/nn-wuapi-icategory
+type ICategory struct {
+	CategoryID  string
+	Children    []*ICategory
+	Description string
+	Image       *IImageInformation
+	Name        string
+	Order       int32
+	Parent      *ICategory
+	Type        string
+	Updates     []*IUpdate
+}
+
+func toICategories(categoriesDisp *ole.IDispatch) ([]*ICategory, error) {
+	count, err := getPropertyInt32(categoriesDisp, "Count")
+	if err != nil {
+		return nil, fmt.Errorf("Count: %w", err)
+	}
+
+	categories := make([]*ICategory, 0, count)
+	for i := 0; i < int(count); i++ {
+		categoryDisp, err := getPropertyDispatch(categoriesDisp, "Item", i)
+		if err != nil {
+			return nil, fmt.Errorf("Item[%d/%d]: %w", i, count, err)
+		}
+
+		category, err := toICategory(categoryDisp)
+		if err != nil {
+			return nil, fmt.Errorf("converting Item[%d/%d]: %w", i, count, err)
+		}
+
+		categories = append(categories, category)
+	}
+	return categories, nil
+}
+
+func toICategory(categoryDisp *ole.IDispatch) (*ICategory, error) {
+	defer categoryDisp.Release()
+
+	var err error
+	iCategory := &ICategory{}
+
+	if iCategory.CategoryID, err = getPropertyString(categoryDisp, "CategoryID"); err != nil {
+		return nil, fmt.Errorf("CategoryID: %w", err)
+	}
+
+	if childrenDisp, err := getPropertyDispatch(categoryDisp, "Children"); err != nil {
+		return nil, fmt.Errorf("Children: %w", err)
+	} else if childrenDisp != nil {
+		defer childrenDisp.Release()
+		if iCategory.Children, err = toICategories(childrenDisp); err != nil {
+			return nil, fmt.Errorf("converting Children: %w", err)
+		}
+	}
+
+	if iCategory.Description, err = getPropertyString(categoryDisp, "Description"); err != nil {
+		return nil, fmt.Errorf("Description: %w", err)
+	}
+
+	if imageDisp, err := getPropertyDispatch(categoryDisp, "Image"); err != nil {
+		return nil, fmt.Errorf("Image: %w", err)
+	} else if imageDisp != nil {
+		defer imageDisp.Release()
+		if iCategory.Image, err = toIImageInformation(imageDisp); err != nil {
+			return nil, fmt.Errorf("converting Image: %w", err)
+		}
+	}
+
+	if iCategory.Name, err = getPropertyString(categoryDisp, "Name"); err != nil {
+		return nil, fmt.Errorf("Name: %w", err)
+	}
+
+	if iCategory.Order, err = getPropertyInt32(categoryDisp, "Order"); err != nil {
+		return nil, fmt.Errorf("Order: %w", err)
+	}
+
+	// Parent is commented out to avoid infinite recursion (Parent -> Category -> Parent ...)
+	// See original code.
+
+	if iCategory.Type, err = getPropertyString(categoryDisp, "Type"); err != nil {
+		return nil, fmt.Errorf("Type: %w", err)
+	}
+
+	// Updates is commented out to avoid pulling the full update tree per category.
+	// See original code.
+
+	return iCategory, nil
+}

pkg/windows/windowsupdate/iimageinformation.go → ee/windowsupdate/iimageinformation.go

@@ -7,14 +7,13 @@ import (
 // IImageInformation contains information about a localized image that is associated with an update or a category.
 // https://docs.microsoft.com/en-us/windows/win32/api/wuapi/nn-wuapi-iimageinformation
 type IImageInformation struct {
-	disp    *ole.IDispatch //nolint:unused
 	AltText string
 	Height  int64
 	Source  string
 	Width   int64
 }
 
 func toIImageInformation(imageInformationDisp *ole.IDispatch) (*IImageInformation, error) {
-	// TODO
+	// TODO: implement property extraction
 	return nil, nil
 }

pkg/windows/windowsupdate/iinstallationbehavior.go → ee/windowsupdate/iinstallationbehavior.go

@@ -7,14 +7,13 @@ import (
 // IInstallationBehavior represents the installation and uninstallation options of an update.
 // https://docs.microsoft.com/en-us/windows/win32/api/wuapi/nn-wuapi-iinstallationbehavior
 type IInstallationBehavior struct {
-	disp                        *ole.IDispatch //nolint:unused
 	CanRequestUserInput         bool
 	Impact                      int32 // enum https://docs.microsoft.com/en-us/windows/win32/api/wuapi/ne-wuapi-installationimpact
 	RebootBehavior              int32 // enum https://docs.microsoft.com/en-us/windows/win32/api/wuapi/ne-wuapi-installationrebootbehavior
 	RequiresNetworkConnectivity bool
 }
 
 func toIInstallationBehavior(installationBehaviorDisp *ole.IDispatch) (*IInstallationBehavior, error) {
-	// TODO
+	// TODO: implement property extraction
 	return nil, nil
 }

ee/windowsupdate/isearchresult.go

@@ -0,0 +1,62 @@
+package windowsupdate
+
+import (
+	"fmt"
+
+	"github.com/go-ole/go-ole"
+)
+
+// ISearchResult represents the result of a search.
+// https://docs.microsoft.com/en-us/windows/win32/api/wuapi/nn-wuapi-isearchresult
+type ISearchResult struct {
+	ResultCode     int32 // enum https://docs.microsoft.com/en-us/windows/win32/api/wuapi/ne-wuapi-operationresultcode
+	RootCategories []*ICategory
+	Updates        []*IUpdate
+	Warnings       []*IUpdateException
+}
+
+func toISearchResult(searchResultDisp *ole.IDispatch) (*ISearchResult, error) {
+	defer searchResultDisp.Release()
+
+	var err error
+	iSearchResult := &ISearchResult{}
+
+	if iSearchResult.ResultCode, err = getPropertyInt32(searchResultDisp, "ResultCode"); err != nil {
+		return nil, fmt.Errorf("ResultCode: %w", err)
+	}
+
+	rootCategoriesDisp, err := getPropertyDispatch(searchResultDisp, "RootCategories")
+	if err != nil {
+		return nil, fmt.Errorf("RootCategories: %w", err)
+	}
+	if rootCategoriesDisp != nil {
+		defer rootCategoriesDisp.Release()
+		if iSearchResult.RootCategories, err = toICategories(rootCategoriesDisp); err != nil {
+			return nil, fmt.Errorf("converting RootCategories: %w", err)
+		}
+	}
+
+	updatesDisp, err := getPropertyDispatch(searchResultDisp, "Updates")
+	if err != nil {
+		return nil, fmt.Errorf("Updates: %w", err)
+	}
+	if updatesDisp != nil {
+		defer updatesDisp.Release()
+		if iSearchResult.Updates, err = toIUpdates(updatesDisp); err != nil {
+			return nil, fmt.Errorf("converting Updates: %w", err)
+		}
+	}
+
+	warningsDisp, err := getPropertyDispatch(searchResultDisp, "Warnings")
+	if err != nil {
+		return nil, fmt.Errorf("Warnings: %w", err)
+	}
+	if warningsDisp != nil {
+		defer warningsDisp.Release()
+		if iSearchResult.Warnings, err = toIUpdateExceptions(warningsDisp); err != nil {
+			return nil, fmt.Errorf("converting Warnings: %w", err)
+		}
+	}
+
+	return iSearchResult, nil
+}

ee/windowsupdate/istringcollection.go

@@ -0,0 +1,33 @@
+package windowsupdate
+
+import (
+	"fmt"
+
+	"github.com/go-ole/go-ole"
+)
+
+// iStringCollectionToStringArray takes an IDispatch to a string collection
+// and returns the array of strings.
+// https://docs.microsoft.com/en-us/windows/win32/api/wuapi/nn-wuapi-istringcollection
+func iStringCollectionToStringArray(disp *ole.IDispatch) ([]string, error) {
+	if disp == nil {
+		return nil, nil
+	}
+
+	count, err := getPropertyInt32(disp, "Count")
+	if err != nil {
+		return nil, fmt.Errorf("Count: %w", err)
+	}
+
+	stringCollection := make([]string, count)
+
+	for i := 0; i < int(count); i++ {
+		str, err := getPropertyString(disp, "Item", i)
+		if err != nil {
+			return nil, fmt.Errorf("Item[%d/%d]: %w", i, count, err)
+		}
+
+		stringCollection[i] = str
+	}
+	return stringCollection, nil
+}