refactor(windows): split Windows code into winapi + winutil + payload by moonD4rk · Pull Request #575 · moonD4rk/HackBrowserData

moonD4rk · 2026-04-19T10:04:49Z

Summary

Post-ABE cleanup addressing four organizational pain points surfaced after #573 shipped.

Add utils/winapi/ — low-level Win32 API layer. Centralizes Kernel32/Ntdll/Crypt32 LazyDLL handles previously declared 3× across utils/injector, filemanager, and crypto_windows.go (the latter re-created them per DecryptDPAPI call).
Add utils/winutil/ (replaces utils/browserutil/) — high-level Windows browser utilities. Unifies browser metadata (ExeName, InstallFallbacks, ABEKind enum) into a single Table. Adding a Chromium fork now touches 2 files instead of 3.
Add crypto/windows/payload/ — move ABE payload embed/stub + .bin artifact out of crypto/ root, so crypto/ root only holds pure-Go primitives.
Shrink crypto/crypto_windows.go from 78 LOC to a thin shim delegating to utils/winapi.
Add running-process probe as 3rd tier in ExecutablePath (HKLM → HKCU → running process → install fallbacks) — picks up portable / non-standard installs not registered in App Paths.
Fix pre-existing bug: Table.InstallFallbacks used Windows-style %VAR% paths but went through os.ExpandEnv which only recognizes Unix $VAR. Swap to winapi.ExpandEnvString (kernel32!ExpandEnvironmentStringsW). Verified on Windows 10 19044 + Go 1.20.14.
Add PE parsing unit tests + cross-reference test ensuring winutil.Table keys stay aligned with platformBrowsers() Storage keys.
Delete utils/injector/strategy.go (YAGNI — single impl, zero interface-typed callers).

Net: +876 / −456 across 21 files. No change to the RFC-010 injection flow itself.

Test plan

go build ./... on linux/darwin/windows + -tags abe_embed
go test ./... -count=1 on host (darwin) and Windows 10 sandbox (all _windows_test.go files executed on the real target OS)
go vet ./... on 3 GOOS
golangci-lint run × 3 GOOS: 0 issues
typos: clean
make payload-clean && make build-windows produces 10 MB exe; payload-verify confirms Bootstrap export
make gen-layout-verify: no drift

Post-ABE cleanup addressing four organizational pain points surfaced after the Chrome App-Bound Encryption feature shipped (#573). Layout changes: * utils/winapi/ (NEW) — low-level Windows API layer. Centralizes Kernel32/Ntdll/Crypt32 LazyDLL handles previously declared 3 times across utils/injector, filemanager, and crypto_windows.go (the latter re-created the handles on every DecryptDPAPI call). Typed wrappers for VirtualAllocEx / CreateRemoteThread / QuerySystemHandles / MapFile / ExpandEnvString / DecryptDPAPI; CallBoolErr handles Win32 errno-0 ambiguity. * utils/winutil/ (NEW, replaces utils/browserutil/) — high-level Windows browser utilities. Unifies browser metadata (ExeName, InstallFallbacks, ABEKind enum) in a single Table; adding a Chromium fork now touches two files (winutil.Table + com_iid.c) instead of three. utils/browserutil/ was misleadingly named (Windows-only in practice). * crypto/windows/payload/ (NEW) — ABE payload embed/stub + .bin artifact moved out of crypto/ root, which now contains only pure-Go primitives. * crypto/crypto_windows.go — shrunk from 78 LOC to a thin shim delegating to utils/winapi. Functional changes: * ExecutablePath: add running-process probe as 3rd resolution tier (registry HKLM -> registry HKCU -> running process -> install fallbacks). Picks up portable / non-standard installs not registered in App Paths. * Fix latent bug in InstallFallbacks expansion: os.ExpandEnv recognizes only Unix-style \$VAR / \${VAR} and leaves Windows-style %VAR% untouched, so fallback paths silently never resolved. Switched to winapi.ExpandEnvString (kernel32!ExpandEnvironmentStringsW). Verified on Windows 10 19044 + Go 1.20.14. * Delete utils/injector/strategy.go (YAGNI): 6-line interface, single implementation, zero interface-typed callers. Tests: * utils/injector/pe_windows_test.go — PE parsing unit tests using C:\\Windows\\System32\\kernel32.dll as fixture (DetectPEArch + FindExportFileOffset, including missing / garbage / truncated inputs). * browser/browser_windows_test.go — TestWinUtilTableCoversABEBrowsers cross-checks winutil.Table against platformBrowsers() Storage keys to catch drift when a fork is added to one but not the other. Validation: * go build ./... on linux/darwin/windows + -tags abe_embed * go test ./... passes on host and on Windows 10 sandbox (all _windows_test.go files executed against real Win32 APIs) * go vet + golangci-lint clean for GOOS=linux/darwin/windows * make payload-clean && make build-windows produces 10 MB exe; payload-verify confirms Bootstrap export * make gen-layout-verify: no drift

codecov-commenter · 2026-04-19T10:05:34Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.25%. Comparing base (76e2615) to head (21548d2).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #575   +/-   ##
=======================================
  Coverage   72.25%   72.25%           
=======================================
  Files          55       55           
  Lines        2278     2278           
=======================================
  Hits         1646     1646           
  Misses        478      478           
  Partials      154      154

Flag	Coverage Δ
unittests	`72.25% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copilot

Pull request overview

This PR reorganizes Windows-specific implementation details by introducing a dedicated low-level Win32 wrapper package (utils/winapi), a higher-level Windows browser utility package (utils/winutil), and relocating the ABE payload embed/stub into crypto/windows/payload, while also improving Windows browser executable discovery and fixing %VAR% environment expansion on Windows.

Changes:

Add utils/winapi (shared LazyDLL handles + typed wrappers) and refactor injector/filemanager/crypto Windows syscall usage to use it.
Replace utils/browserutil with utils/winutil, centralizing Windows browser metadata and executable resolution (HKLM → HKCU → running process → fallbacks) and switching fallback expansion to ExpandEnvironmentStringsW.
Move ABE payload embed/stub to crypto/windows/payload, update build artifacts/output paths, and add Windows unit tests (PE parsing + winutil table cross-check).

Reviewed changes

Copilot reviewed 21 out of 21 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
utils/winutil/browser_path_windows.go	New Windows executable resolver with registry + running-process probe + fallbacks using Win32 env expansion.
utils/winutil/browser_meta_windows.go	New centralized Windows browser metadata table (ExeName/fallbacks/ABE kind).
utils/winapi/winapi_windows.go	New shared LazyDLL handles + `CallBoolErr` helper for consistent Win32 call error reporting.
utils/winapi/sysinfo_windows.go	New wrappers for handle enumeration + file path/type/size + mapping utilities (used by locked-file copy).
utils/winapi/process_windows.go	New wrappers for process enumeration, remote thread primitives, and image path lookup.
utils/winapi/dpapi_windows.go	New centralized DPAPI decrypt wrapper (used by `crypto.DecryptDPAPI`).
utils/injector/winapi_windows.go	Deleted in favor of `utils/winapi`.
utils/injector/strategy.go	Deleted unused interface.
utils/injector/reflective_windows.go	Updated injector implementation to use `utils/winapi` wrappers and raw proc addresses.
utils/injector/pe_windows_test.go	Added PE parsing/export-walking tests using kernel32 as a real-world fixture.
utils/browserutil/path_windows.go	Removed legacy executable path resolver.
rfcs/010-chrome-abe-integration.md	Updated RFC references to new winutil/payload locations.
filemanager/copy_windows.go	Refactored locked-file copy to use centralized `utils/winapi` helpers.
crypto/windows/payload/stub_windows.go	New payload stub (non-`abe_embed`) moved under `crypto/windows/payload`.
crypto/windows/payload/embed_windows.go	New embedded payload (with `abe_embed`) moved under `crypto/windows/payload`.
crypto/windows/abe_native/Makefile.frag	Updated payload output directory and clean targets to match new payload location.
crypto/keyretriever/abe_windows.go	Updated ABE retriever to use `payload.Get` and `winutil.ExecutablePath`.
crypto/crypto_windows.go	Simplified DPAPI decrypt shim to delegate to `utils/winapi`.
crypto/abe_stub_windows.go	Removed old payload stub from `crypto` root.
crypto/abe_embed_windows.go	Removed old payload embed from `crypto` root.
browser/browser_windows_test.go	Added test ensuring ABE-capable `winutil.Table` entries align with `platformBrowsers()` Storage keys.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-19T10:09:37Z

@@ -258,12 +120,13 @@ func readFileContent(handle windows.Handle) ([]byte, error) {
 	size := int(fileSize)

 	// Try FileMapping first — reads from kernel file cache, includes WAL data
-	if data, err := readViaFileMapping(handle, size); err == nil {
+	if data, err := winapi.MapFile(handle, size); err == nil {


readFileContent converts the int64 file size to int without a bounds check. On very large files (or 32-bit builds), this can overflow and lead to negative/incorrect lengths and panics in downstream allocations/mapping. Consider validating the size fits in an int (and is <= max practical mapping size) before casting.

Copilot · 2026-04-19T10:09:37Z

+	d := make([]byte, b.cbData)
+	copy(d, (*[1 << 30]byte)(unsafe.Pointer(b.pbData))[:])


dataBlob.bytes copies from an unsafe (*[1<<30]byte) slice without bounding it to cbData, which bakes in a 1GB limit and relies on copy() to not read past the valid allocation. Consider using unsafe.Slice(b.pbData, cbData) (with appropriate int conversion/bounds checks) to make the length explicit and avoid potential panics for unusually large DPAPI blobs.

Suggested change

d := make([]byte, b.cbData)

copy(d, (*[1 << 30]byte)(unsafe.Pointer(b.pbData))[:])

if b.cbData == 0 {

return nil

}

n := int(b.cbData)

if uint32(n) != b.cbData {

panic("dpapi blob too large")

}

d := make([]byte, n)

copy(d, unsafe.Slice(b.pbData, n))

Copilot · 2026-04-19T10:09:38Z

+// GetFinalPathName returns the normalized file path for h, with the
+// \\?\ prefix stripped.
+func GetFinalPathName(h windows.Handle) (string, error) {
+	size := 512
+	for {
+		buf := make([]uint16, size)
+		n, _, err := procGetFinalPathNameByHandleW.Call(
+			uintptr(h),
+			uintptr(unsafe.Pointer(&buf[0])),
+			uintptr(len(buf)),
+			0, // FILE_NAME_NORMALIZED
+		)
+		if n == 0 {
+			return "", fmt.Errorf("GetFinalPathNameByHandle: %w", err)
+		}
+		if int(n) > len(buf) {
+			size = int(n)
+			continue
+		}
+		path := windows.UTF16ToString(buf[:n])
+		return strings.TrimPrefix(path, `\\?\`), nil
+	}


GetFinalPathName strips the \\?\ prefix via TrimPrefix, but this produces an invalid path for UNC handles (e.g. \\?\UNC\server\share\... becomes UNC\server\share\...). Consider handling the UNC form explicitly (mapping \\?\UNC\ back to \\) or otherwise returning a correctly normalized path for both local and UNC targets.

Copilot · 2026-04-19T10:09:38Z

+	buf := make([]uint16, windows.MAX_PATH)
+	size := uint32(len(buf))
+	r, _, err := procQueryFullProcessImageName.Call(
+		uintptr(h),
+		0,
+		uintptr(unsafe.Pointer(&buf[0])),
+		uintptr(unsafe.Pointer(&size)),
+	)
+	if r == 0 {
+		return "", fmt.Errorf("QueryFullProcessImageNameW: %w", err)
+	}
+	return windows.UTF16ToString(buf[:size]), nil


QueryFullProcessImageName uses a fixed MAX_PATH-sized buffer and returns an error when the path is longer (e.g. ERROR_INSUFFICIENT_BUFFER). Since this backs the new running-process probe, consider retrying with a dynamically grown buffer (similar to GetFinalPathNameByHandle) so long install paths don’t cause false negatives.

Suggested change

buf := make([]uint16, windows.MAX_PATH)

size := uint32(len(buf))

r, _, err := procQueryFullProcessImageName.Call(

uintptr(h),

0,

uintptr(unsafe.Pointer(&buf[0])),

uintptr(unsafe.Pointer(&size)),

)

if r == 0 {

return "", fmt.Errorf("QueryFullProcessImageNameW: %w", err)

}

return windows.UTF16ToString(buf[:size]), nil

size := uint32(windows.MAX_PATH)

for {

buf := make([]uint16, size)

n := uint32(len(buf))

r, _, err := procQueryFullProcessImageName.Call(

uintptr(h),

0,

uintptr(unsafe.Pointer(&buf[0])),

uintptr(unsafe.Pointer(&n)),

)

if r != 0 {

return windows.UTF16ToString(buf[:n]), nil

}

if err != windows.ERROR_INSUFFICIENT_BUFFER {

return "", fmt.Errorf("QueryFullProcessImageNameW: %w", err)

}

size *= 2

if size > 32768 {

return "", fmt.Errorf("QueryFullProcessImageNameW: image path exceeded 32768 UTF-16 code units")

}

}

Copilot · 2026-04-19T10:09:38Z

+// MapFile creates a read-only file mapping over h, copies the first
+// size bytes into a Go-owned slice, and releases the mapping. Reads go
+// through the OS kernel's file cache, which includes SQLite WAL data
+// that has not yet been checkpointed into the main file.
+func MapFile(h windows.Handle, size int) ([]byte, error) {
+	mapping, _, err := procCreateFileMappingW.Call(
+		uintptr(h),
+		0, pageReadonly,
+		0, 0, 0,
+	)
+	if mapping == 0 {
+		return nil, fmt.Errorf("CreateFileMapping: %w", err)
+	}
+	defer windows.CloseHandle(windows.Handle(mapping))
+
+	viewPtr, _, err := procMapViewOfFile.Call(
+		mapping, fileMapRead,
+		0, 0, 0,
+	)
+	if viewPtr == 0 {
+		return nil, fmt.Errorf("MapViewOfFile: %w", err)
+	}
+	defer procUnmapViewOfFile.Call(viewPtr)
+
+	// viewPtr is a valid pointer from MapViewOfFile syscall.
+	// go vet flags this as "possible misuse of unsafe.Pointer" but it's
+	// correct usage for Windows memory-mapped I/O.
+	data := make([]byte, size)
+	copy(data, (*[1 << 30]byte)(unsafe.Pointer(viewPtr))[:size]) //nolint:govet
+	return data, nil


MapFile builds a slice from (*[1<<30]byte)(unsafe.Pointer(viewPtr))[:size], which will panic if size exceeds 1<<30 (and also bakes in an arbitrary 1GB limit). Consider using unsafe.Slice to create a byte slice of length size (after validating size >= 0) to avoid the hard limit and potential panics on large files.

Copilot AI review requested due to automatic review settings April 19, 2026 10:04

Copilot started reviewing on behalf of moonD4rk April 19, 2026 10:05 View session

Copilot AI reviewed Apr 19, 2026

View reviewed changes

moonD4rk merged commit ae1ec66 into main Apr 19, 2026
13 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor(windows): split Windows code into winapi + winutil + payload#575

refactor(windows): split Windows code into winapi + winutil + payload#575
moonD4rk merged 1 commit intomainfrom
refactor/windows-code-layout

moonD4rk commented Apr 19, 2026

Uh oh!

codecov-commenter commented Apr 19, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Copilot AI Apr 19, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

		d := make([]byte, b.cbData)
		copy(d, (*[1 << 30]byte)(unsafe.Pointer(b.pbData))[:])

-	d := make([]byte, b.cbData)
-	copy(d, (*[1 << 30]byte)(unsafe.Pointer(b.pbData))[:])
+	if b.cbData == 0 {
+		return nil
+	}
+	n := int(b.cbData)
+	if uint32(n) != b.cbData {
+		panic("dpapi blob too large")
+	}
+	d := make([]byte, n)
+	copy(d, unsafe.Slice(b.pbData, n))

Conversation

moonD4rk commented Apr 19, 2026

Summary

Test plan

Uh oh!

codecov-commenter commented Apr 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 19, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov-commenter commented Apr 19, 2026 •

edited

Loading