fix(windows): silent default dump on double-click#591
Conversation
Detect Explorer-launched binaries via mousetrap and hide the auto-created console window so the .exe runs the default dump silently. Command-line invocations are unaffected. Closes #344.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #591 +/- ##
=======================================
Coverage 73.60% 73.60%
=======================================
Files 61 61
Lines 2815 2815
=======================================
Hits 2072 2072
Misses 553 553
Partials 190 190
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements a Windows-only “double-click mode” so Explorer-launched executions don’t show Cobra’s mousetrap warning and the console window is hidden, while keeping normal CLI behavior unchanged.
Changes:
- Promote
github.com/inconshreveable/mousetrapto a direct dependency for Explorer-launch detection. - Add
configureDoubleClickMode()with Windows and non-Windows implementations. - Invoke
configureDoubleClickMode()at the start ofmain().
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| go.mod | Adds mousetrap as a direct dependency to support Windows double-click detection. |
| cmd/hack-browser-data/main.go | Calls configureDoubleClickMode() before executing the Cobra root command. |
| cmd/hack-browser-data/main_windows.go | Windows implementation: detect Explorer launch, disable Cobra mousetrap help text, and hide the console window via Win32 APIs. |
| cmd/hack-browser-data/main_others.go | Non-Windows stub implementation (no-op). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var ( | ||
| kernel32 = windows.NewLazySystemDLL("kernel32.dll") | ||
| user32 = windows.NewLazySystemDLL("user32.dll") | ||
| procGetConsoleWindow = kernel32.NewProc("GetConsoleWindow") | ||
| procShowWindow = user32.NewProc("ShowWindow") | ||
| ) |
There was a problem hiding this comment.
This reintroduces per-file NewLazySystemDLL/NewProc boilerplate and bypasses the existing utils/winapi convention that centralizes Win32 DLL/proc handles + error-handling (see utils/winapi/winapi_windows.go:3-45). Consider moving GetConsoleWindow/ShowWindow proc definitions (and a User32 handle) into utils/winapi and calling a typed wrapper from here, so future Win32 calls follow one pattern and can share consistent error handling.
Move ShowWindow / GetConsoleWindow proc declarations into utils/winapi to follow the existing centralization convention (see review feedback). Adds a User32 LazyDLL handle and a typed HideConsoleWindow() wrapper. main_windows.go is now a thin caller that uses the wrapper.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| configureDoubleClickMode() | ||
| if err := rootCmd().Execute(); err != nil { | ||
| os.Exit(1) | ||
| } |
There was a problem hiding this comment.
When StartedByExplorer() is true, the console is hidden and cobra’s mousetrap help is suppressed, but errors from rootCmd().Execute() still only surface via stderr and exit code. In double-click mode this can lead to “nothing happened” with no user-visible diagnostics if extraction fails (e.g., permission issues writing results). Consider special-casing Explorer launches to persist errors to a log file under the output directory and/or showing a Windows dialog for fatal errors, while keeping CLI behavior unchanged for terminal launches.
| func configureDoubleClickMode() { | ||
| if !mousetrap.StartedByExplorer() { | ||
| return | ||
| } | ||
|
|
||
| cobra.MousetrapHelpText = "" | ||
| winapi.HideConsoleWindow() |
There was a problem hiding this comment.
In Explorer/double-click mode the default output dir is still the relative "results" (see dump.go), so where data is written depends on the current working directory. If a user launches via a shortcut or ShellExecute with a different working dir, results may not end up next to the .exe as described in the PR. Consider, when StartedByExplorer() is true, setting the process working directory to the executable’s directory (os.Executable + filepath.Dir + os.Chdir) or otherwise resolving the output path relative to the .exe to make the behavior deterministic.
Capture ShowWindow's previous-visibility return as the function's boolean result instead of dropping all three returns with a blank assignment. Cleaner than blank-identifier juggling and gives the caller actionable info if needed.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
mousetrapdependency and hide the auto-created console windowdump -b all -c allsilently, writingresults/next to the .exeImplementation
cmd/hack-browser-data/main_windows.go— Windows-only; callsmousetrap.StartedByExplorer()anduser32!ShowWindowto hide consolecmd/hack-browser-data/main_others.go— No-op stub for non-Windows platformscmd/hack-browser-data/main.go— CallsconfigureDoubleClickMode()as the first line ofmain()mousetrapupgraded from indirect → direct dependencyTest plan
hbd.exe -c passwordshows full logs, exports 6 passwords with ABE retrieval intact (parent=ssh,mousetrap.StartedByExplorer()→ false,configureDoubleClickModeno-op)Shell.Application.ShellExecute(parent=explorer.exe) — manually verified by maintainer: no cmd window, results written silently next to the .exeCloses #344.