Power Event Callback Utils

[VishalDalwadi]

Describe your changes

Provide Issue ticket number if applicable/not in title

Provide link to Netmaker PR if required

Provide testing steps

Checklist before requesting a review

• My changes affect only 10 files or less.
• I have performed a self-review of my code and tested it.
• If it is a new feature, I have added thorough tests, my code is <= 1450 lines.
• If it is a bugfix, my code is <= 200 lines.
• My functions are <= 80 lines.
• I have had my code reviewed by a peer.
• My unit tests pass locally.
• Netclient & Netmaker are awesome.

[tenki-reviewer]

Review complete. 1 potential issue to review.

Files Reviewed: 3
Findings: 1

By Severity:

• 🟠 High: 1

---

Data race on unsynchronized callback globals in Windows power event handling — Register/Unregister write to handlers from Go goroutines while the callback reads them from a system thread with no synchronization, risking nil-panic and dropped events.

Files Reviewed (3 files)

ncutils/power_darwin.go
ncutils/power_linux.go
ncutils/power_windows.go

[tenki-reviewer]

Review complete. 1 potential issue to review.

---

Summary

This PR adds cross-platform power suspend/resume notification stubs (ncutils/power_*.go) for the netclient WireGuard mesh VPN agent. Linux and Darwin are no-op stubs; Windows delivers events via RegisterSuspendResumeNotification.

Key Finding

finding-001 — High Severity: Data Race on Unsynchronized Callback Globals

File: ncutils/power_windows.go lines 35-96

Package-level globals onResumeAutomatic, onResumeSuspend (function pointers), and suspendResumeNotifyHandle (uintptr) are accessed concurrently without synchronization:

• Writers (Go goroutines): RegisterPowerEventHandlers and UnregisterPowerEventHandlers mutate these variables.
• Reader (Windows system thread): powerEventCallback, invoked via syscall.NewCallback, reads and calls them.

This creates a TOCTOU (time-of-check-time-of-use) vulnerability: if UnregisterPowerEventHandlers nulls a function pointer between the nil check and the call, a nil pointer dereference panic occurs on the system thread. Additionally, freshly registered handlers may be invisible to an already-dispatched callback due to lack of memory barriers.

Fix: Protect all accesses with a sync.RWMutex. The callback should acquire a read lock, snapshot the function pointers, release the lock, then invoke the snapshots.

Rejected Finding

A medium-severity edge case (finding-002, confidence 75) regarding misleading error messages when RegisterSuspendResumeNotification returns a NULL handle with GetLastError() == 0 was noted but fell below confidence threshold.

[tenki-reviewer]

🟠 Data race on unsynchronized callback globals (onResumeAutomatic, onResumeSuspend, suspendResumeNotifyHandle) (bug)

The package-level variables onResumeAutomatic, onResumeSuspend (function pointers declared at lines 38-39), and suspendResumeNotifyHandle (uintptr at line 35) are accessed concurrently with no synchronization.

Writers (Go goroutines):

• RegisterPowerEventHandlers writes onResumeAutomatic and onResumeSuspend at lines 50-51, and suspendResumeNotifyHandle at line 68.
• UnregisterPowerEventHandlers writes suspendResumeNotifyHandle at line 76-77, and onResumeAutomatic/onResumeSuspend (setting them to nil) at lines 79-80.

Reader (Windows system thread):

• powerEventCallback (line 85) is invoked by Windows on a system thread via syscall.NewCallback (line 54). It reads onResumeAutomatic at line 89 and calls it at line 90; reads onResumeSuspend at line 94 and calls it at line 95.

This is a data race under the Go memory model (Go 1.25 race detector would flag it). The critical danger is the TOCTOU (time-of-check-time-of-use) pattern: if onResumeAutomatic != nil { onResumeAutomatic() } — if UnregisterPowerEventHandlers nulls the function pointer between the nil check and the call, a nil function invocation (panic: nil pointer dereference) occurs. Conversely, a freshly registered handler may not be visible to an already-dispatched callback, causing events to be silently dropped.

💡 Suggestion: Protect all accesses to onResumeAutomatic, onResumeSuspend, and suspendResumeNotifyHandle with a sync.RWMutex. The callback (reader) acquires a read lock, snapshots the two function pointers into locals, releases the lock, then calls the non-nil snapshots. Register/Unregister (writers) acquire the write lock.

📋 Prompt for AI Agents

In ncutils/power_windows.go: (1) add var cbMu sync.RWMutex to the var block on line 30. (2) In RegisterPowerEventHandlers (line 47), hold cbMu.Lock() before the UnregisterPowerEventHandlers() call at line 48, and release it after suspendResumeNotifyHandle = handle at line 68. Also reorder: if the syscall fails at line 65-66, restore the handlers to nil before returning so the registration is atomic. (3) In UnregisterPowerEventHandlers (line 74), hold cbMu.Lock() around lines 75-80. (4) In powerEventCallback (line 85), acquire cbMu.RLock(), snapshot onResumeAutomatic and onResumeSuspend into local variables, release cbMu.RUnlock(), then check and call the local snapshots — this avoids holding the lock during user code execution on the system thread.