#59 Implement retrieval of SMART data for Windows - Privilege escalation version - #194
Conversation
Implemented Windows-specific SMART data retrieval using smartctl.exe and PowerShell partition mapping.
- Merge origin/dev into smart-win (resolve conflicts in BenchmarkRunner, MainFrame, Smart, Gui) - Bundle smartctl 7.5 (Windows x86_64) from jdm-deps into MSI via jdm-core/target/smartctl/ - Fix resolveSmartctlPath() on Windows: use jar code-source URL and java.home (jpackage does not set APPDIR on Windows) - getSmart() Windows: merge stderr into stdout, try /dev/pdN then bare pdN, log non-JSON responses - Gui.runSmart(): admin check guard on Windows with actionable error messages in SmartPanel status bar - SmartPanel: add setStatus(String) for external status updates - windows-msi.yml: add Fetch bundled smartctl step (mirrors linux-deb.yml) - jdm-msi/pom.xml: stage-smartctl-windows antrun step auto-downloads smartctl.exe from jdm-deps
- MainFrame: add macOS to SMART tab condition (was Linux+Windows only, but macOS SMART is fully implemented in Gui.runSmart() and Smart.java) - Smart.getSmart() Windows: replace unbounded p.waitFor() with 15-second timeout; forcibly destroy hung smartctl process and continue to next device arg - Smart.getSmart() Windows: split IOException|InterruptedException multi-catch; restore thread interrupt flag on InterruptedException - BenchmarkRunner: guard Windows SMART fetch with App.isAdmin check, consistent with Gui.runSmart() — avoids noisy log failures when not elevated - windows-msi.yml: correct Fetch bundled smartctl comment and $dest path from old app-content location to jdm-core/target/smartctl/ (actual --input path) - jdm-msi/pom.xml: add Ant fail task after smartctl staging check so MSI build fails fast instead of silently producing a bundle without the expected binary
…s admin)
Previously, SMART data on Windows required launching the entire application as
Administrator. This commit introduces on-demand UAC escalation so the app can
run as a normal user and still read SMART data.
How it works:
- When the user clicks Run SMART (and is not already admin), SmartEscalation
writes a PowerShell helper script to %LOCALAPPDATA%\JDiskMark\smart-helper.ps1
- The script is launched elevated via Start-Process -Verb RunAs -Wait, which
triggers a standard Windows UAC prompt (shows "Windows PowerShell")
- The elevated helper runs smartctl and writes the JSON result to
%LOCALAPPDATA%\JDiskMark\smart-ipc-<device>.json
- Both processes share %LOCALAPPDATA% because they run as the same Windows user
(just different privilege tokens), so the file is readable by the non-elevated app
- The main process reads, parses and displays the result; UAC cancellation is
handled gracefully with a clear status message
New file:
SmartEscalation.java: IPC helper (write script, launch elevated, read result)
Modified files:
Smart.java: getSmart() splits into fast path (already admin -> getSmartDirect)
and escalation path (not admin -> SmartEscalation.runElevated)
Gui.java: remove early-return admin guard; show UAC hint in status bar;
update failure message to mention UAC cancellation
BenchmarkRunner.java: remove App.isAdmin guard (escalation is now transparent)
Future improvement: a native jdm-smart-helper.exe with requireAdministrator
manifest would show "JDiskMark" in the UAC dialog instead of "Windows PowerShell"
Two bugs fixed:
1. Script file path with spaces (e.g. 'Ian Reyes' in %LOCALAPPDATA%) broke the
-File argument when passed through Start-Process -ArgumentList, causing the
elevated PowerShell to silently not run the script. Fixed by switching to
-EncodedCommand (UTF-16LE base64) — no script file path needed at all.
2. .NET's [System.Text.Encoding]::UTF8 writes a UTF-8 BOM (EF BB BF) by
default, causing the JSON check startsWith('{') to fail (the string actually
started with U+FEFF). Fixed by using New-Object System.Text.UTF8Encoding(\False)
in the PowerShell script, and adding a defensive BOM-strip in Java before
parsing.
There was a problem hiding this comment.
Pull request overview
Adds Windows SMART data collection support by bundling smartctl.exe into the Windows MSI and introducing a Windows-specific elevation flow (UAC) so SMART can be queried without requiring the whole app to run as Administrator.
Changes:
- Bundle
smartctl.exeinto the Windows MSI build inputs (Maven + CI workflow staging). - Add Windows device resolution (drive letter → physical disk number) and Windows SMART retrieval paths (direct when elevated, otherwise UAC-elevated helper).
- Expose SMART UI/benchmark integration on Windows and store SMART data on the
Benchmarkobject (non-persisted).
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
jdm-dist/jdm-msi/pom.xml |
Stages a bundled smartctl.exe into jdm-core/target so jpackage --input includes it in the MSI image. |
jdm-core/src/main/java/jdiskmark/UtilOs.java |
Adds Windows drive-letter → physical disk number resolution via PowerShell. |
jdm-core/src/main/java/jdiskmark/SmartPanel.java |
Adds a thread-safe status setter for SMART UI messaging. |
jdm-core/src/main/java/jdiskmark/SmartEscalation.java |
New Windows UAC escalation helper that runs smartctl --json elevated and returns JSON via an IPC file. |
jdm-core/src/main/java/jdiskmark/Smart.java |
Adds Windows smartctl path resolution and Windows SMART execution (direct/elevated). Disables privileged shell/heartbeat on Windows. |
jdm-core/src/main/java/jdiskmark/MainFrame.java |
Enables the SMART tab on Windows. |
jdm-core/src/main/java/jdiskmark/Gui.java |
Allows SMART to run on Windows; resolves Windows device name as pdN; adds UAC prompt status messaging and failure hints. |
jdm-core/src/main/java/jdiskmark/BenchmarkRunner.java |
Fetches SMART for benchmarks on Windows as well, and triggers SMART UI updates in GUI mode. |
jdm-core/src/main/java/jdiskmark/Benchmark.java |
Adds a non-persisted (@Transient) Smart field to attach SMART data to a benchmark run. |
.github/workflows/windows-msi.yml |
CI step to download/unzip smartctl into jdm-core/target/smartctl prior to MSI build. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- SmartEscalation: drain process input stream asynchronously to prevent deadlock during timeouts - SmartEscalation: validate device name format to prevent injection/path traversal - SmartEscalation: generalize BOM stripping rationale comment - BenchmarkRunner: populate SMART UI using already-fetched data instead of triggering duplicate SMART query/UAC prompt - jdm-msi/pom.xml: verify SHA-256 checksum of downloaded smartctl.zip in AntRun step - windows-msi.yml: verify SHA-256 checksum of downloaded smartctl.zip in CI workflow
How to Test Windows UAC Privilege Escalation for SMART DataThis branch eliminates the requirement to launch the entire JDiskMark application as Administrator. Instead, the application runs with standard user privileges, and only triggers a scoped UAC prompt when SMART data is requested. Prerequisites
Test Flow 1: Manual SMART Retrieval ("Run SMART" Button)
Test Flow 2: Benchmark Execution SMART Integration
Test Flow 3: Cancelling UAC Prompt (Negative Test)
Under the Hood / Troubleshooting VerificationIf you'd like to inspect the inter-process communication (IPC) artifacts:
|
|
@IanReyes44 when i do a sequential test it will do two prompts but the purpose of the heartbeat is to keep the original process active, alive and not timed out so we don't have to prompt the user unnecessarily. Can we see if it's possible to reuse the last admin process if it is still available? If not and we have to prompt the user every time perhaps for now we don't need the heartbeat as it's not keeping an admin session alive as we would like. |
|
regardless it's looking really good, let me try to get jasmine's branch merged into dev as i think it fixes the extra space at the end of the smart panel. |
jamesmarkchan
left a comment
There was a problem hiding this comment.
hi @IanReyes44 , can you pull updates from dev? there are a bunch of items that came from the smart branch and generally it looks safe to take the version on dev. this is from sonnet:
After that let's add a TODO here:
public static void startHeartbeat() {
if (App.isWindows()) {
// TODO: implement persistent process to avoid repeated UAC auth prompt
return; // ← exits immediately
}
and then merge we can re-branch for the single UAC auth prompt later but a merge sounds good to lock in what you have.
|
Hi @jamesmarkchan, All the suggestions have been addressed and merged:
Ready for review / merge! |
|
good to merge! :) |
Windows packaging for smartctl with privilege escalation (no more running as admin on Windows)