Conversation
Microsoft's vulnerable-driver blocklist now includes WinRing0, the ring-0 driver that upstream LibreHardwareMonitorLib uses to read CPU MSRs and motherboard SuperIO chips. Defender and a number of EDR products flag WinRing0 on sight, so the previous build produced an "unwanted driver" warning every time WinState was launched on a fresh machine. Switch to namazso's PawnIO fork of LibreHardwareMonitor (signed driver, sandboxed Pawn bytecode) without losing any sensor coverage. The LHM API surface is unchanged - SystemInfoService compiles against the fork without a single edit. Library swap: - Vendor/LibreHardwareMonitor-PawnIO added as a git submodule pinned to the pawnio-squashed branch (1d58096). - WinState.csproj drops the LibreHardwareMonitorLib NuGet package in favour of a ProjectReference to the submodule's csproj. - DefaultItemExcludes adds Vendor/** so the WPF SDK's default Compile glob does not try to compile the fork's sibling projects (Aga.Controls, the WinForms UI) inside WinState. - System.Management bumped to 9.0.9 to match the fork's transitive minimum. - LibreHardwareMonitorLib added to WinState.sln so Visual Studio's IDE build resolves the ProjectReference dependency (CLI MSBuild was fine, but the IDE only builds projects loaded in the solution). Driver presence UI: - New Services/PawnIODriverService.cs probes the SCM for the "PawnIO" service via ServiceController, returning NotInstalled / Stopped / Running / Unknown. Also exposes helpers to kick off `winget install -e --id namazso.PawnIO` or open pawnio.eu. - SettingsViewModel surfaces a status string and three commands; initialisation refreshes the state. - SettingsPage.xaml adds a "hardware driver" card between the General and tray-icon sections, with Install via WinGet / pawnio.eu / refresh buttons. CI: - actions/checkout gets `submodules: recursive` so the runner has the fork available before dotnet restore. README: - Both language sections explain the PawnIO driver requirement on first launch, point at pawnio.eu and the WinGet command, and switch the build instructions to `git clone --recursive` (with the submodule update fallback).
Opening the SENSORS popup the first time after launch suffered the same layout-shift bug the CPU section did before PrepopulateUiCollections: the ItemsControl bound to DetailedSensors was empty at first measure because UpdateDetailedSensors only runs while a UI surface is visible, so the popup measured itself short, the positioning anchored against that short height, and the first data tick then filled the list with ~50 SensorItems and grew the window below the working area. Call UpdateDetailedSensors once at the end of SystemInfoService's constructor. InitializeHardwareAndSensors already runs hardware.Update() on every IHardware so sensor.Value is populated; the seed call binds those values into the pooled SensorItems and the public list. Wrapped in try/catch so an unexpected failure here can never block startup.
- English download step 4 referenced the settings tab by its Chinese name (`設定頁的「硬體驅動程式」`); rename to `Settings → Hardware driver` so English readers can find it. - The settings table missed two real settings groups: General (launch at logon, backed by the Scheduled Task that StartupManager creates) and Hardware driver (the PawnIO install / status card added in e814db9). List them in both language versions.
A bespoke WinUI 3 unpackaged setup wizard wrapping the existing self-contained WinState exe. Replaces the "download the exe and figure it out yourself" flow with an actual Windows installer (Mica background, Fluent controls, UAC up front, registers under Apps & features, supports `--uninstall`). Wizard flow - Welcome / Options / Progress / Finished, plus a UninstallConfirm page when relaunched with --uninstall. - Options page exposes install location + three checkboxes: install PawnIO driver, launch at logon, add Start Menu shortcut. - Progress page streams a step log from the same DispatcherQueue. - Finished page optionally launches the installed exe on close. Install logic (Services/InstallerLogic.cs) - Copies payload/WinState.exe (and the installer itself, so Apps & features keeps working even if the user threw the download away) into %ProgramFiles%\WinState\. - Common Start Menu shortcut via WScript.Shell COM (no extra deps). - Registers the same logon Scheduled Task that StartupManager creates, but pointing at the installed path — both sides write the same task name so flipping the toggle inside WinState stays consistent. - Writes HKLM Uninstall\WinState (DisplayName / Publisher / Display Version / InstallLocation / DisplayIcon / UninstallString / NoModify / NoRepair / EstimatedSize) so the entry shows up under Apps & features and routes uninstall back into the same exe. - PawnIO is delegated to `winget install -e --id namazso.PawnIO` (silent, accepts agreements); exit code 3010 is treated as success with a reboot hint. Uninstall mode - Triggered by --uninstall on the command line. Removes the Scheduled Task, Start Menu shortcut, registry key, and the install directory contents. Leaves PawnIO alone because other apps (FanControl etc.) may also rely on it. - Settings under %AppData%\WinState are kept; documented on the confirm page. Payload + CI - Build flow: scripts/build-installer.ps1 publishes WinState, copies the single-file exe into WinState.Installer/payload/, then publishes the installer. CI runs the same steps inline and uploads the installer-publish folder as a per-RID artifact. - WinState.Installer.csproj includes payload/WinState.exe as Content with CopyToPublishDirectory guarded by Exists(), so the installer still builds cleanly on the first pass. - WinState.csproj's DefaultItemExcludes now also excludes WinState.Installer\** so the WPF SDK's XAML/code globs don't drag WinUI 3 files into the main app's compilation. - payload/ and artifacts/ added to .gitignore.
The Next button stayed enabled during the install task, so a user clicking it skipped straight to the Finished screen before file copy / registry / scheduled task had actually completed. Disable Next while the Progress page is shown and re-enable it through a dedicated GoNextProgrammatic path that ProgressPage uses to auto-advance once the install task returns.
WinUI 3 unpackaged apps publish as a folder of ~350 files (WindowsApp
Runtime, XAML controls, native DLLs, payload). Shipping that as a
.zip and asking the user to unzip and double-click the right exe is
ugly, so wrap the whole folder in a tiny self-extractor.
WinState.Bootstrapper
- Tiny .NET 8 WinExe that embeds payload.zip as an EmbeddedResource.
- Manifest requireAdministrator, same as the inner installer, so the
user only sees one UAC prompt at launch and elevation flows down
the chain via UseShellExecute=false.
- On launch: extracts payload.zip into %TEMP%\WinState-Setup-<guid>,
Process.Starts WinState.Installer.exe, waits for exit, deletes the
temp folder. Errors surface through User32 MessageBox so the EXE
needs no WinForms / WPF dependency.
Build pipeline
- scripts/build-installer.ps1 now runs in three stages:
1) publish WinState (self-contained single-file exe)
2) stage that exe into WinState.Installer/payload/ and publish the
installer
3) zip the installer-publish folder into
WinState.Bootstrapper/payload.zip and publish the bootstrapper
as a single self-contained exe
End result: one `WinState-Setup.exe` (~200 MB).
- build.yml mirrors the same stages and uploads
WinState-Setup-<rid>.exe as the only user-facing artifact (the
installer-publish folder upload still ships for power users / CI
forensics).
- WinState.csproj's DefaultItemExcludes adds WinState.Bootstrapper\**
so the WPF SDK's globs don't pick up the bootstrapper's Program.cs.
- .gitignore excludes the generated payload.zip.
…trapper from CI GitHub Actions wraps every upload-artifact output in a .zip on download — even a single file we built — so the 200 MB self-extracting bootstrapper turned into a 200 MB .zip containing one .exe at no benefit to the user. Drop the bootstrapper publish + upload steps from the workflow and let GitHub auto-zip the installer-publish folder instead. The user downloads one WinState-Setup-<rid>.zip, unzips once, and double-clicks WinState.Installer.exe inside. The WinState.Bootstrapper project stays in the tree for two reasons: - `scripts/build-installer.ps1` still produces a single WinState-Setup.exe locally for distribution outside of GitHub (sideload, mirrors, future Releases). - A future Releases-on-tag workflow can serve that exe raw (Release assets are not wrapped in zip the way Actions artifacts are). README's Download section gets a small refresh to point at the new artifact names and to mention the wizard's PawnIO toggle instead of the original "run WinState.exe and the app handles it" wording.
The previous attempt uploaded the installer-publish folder so GitHub Actions auto-zipped it; the resulting WinState-Setup-<rid>.zip expanded into 350+ files (WindowsAppRuntime DLLs, locale folders, XAML controls), which looks broken to anyone who expected an installer. Switch back to publishing the bootstrapper and uploading only the single self-extracting WinState-Setup-<rid>.exe. The artifact still arrives as a .zip (GitHub wraps everything), but inside is exactly one file — double-click and the bootstrapper extracts the wizard to %TEMP% and launches it. Trade-off: the artifact is now ~200 MB instead of ~30 MB because the bootstrapper bundles WinUI 3 and the inner exe. Accepted on the strength of the unzip UX. Folder-style "WinState-Setup-<rid>" can be revived later as a separate "power-user" artifact if anyone asks.
Two installer UX fixes: - Lock the wizard window. WinUI 3 windows default to resizable; a setup wizard shouldn't be. ConfigureWindow() sets IsResizable / IsMaximizable / IsMinimizable to false and widens the default from 720x520 to 860x560 so the option rows and log don't wrap. - Replace the Browse folder picker. The WinRT Windows.Storage.Pickers.FolderPicker silently no-ops in an unpackaged, self-contained WinUI 3 app (no package identity for the activation factory), so clicking Browse did nothing. Swap in a Win32 IFileOpenDialog wrapper (NativeFolderPicker, FOS_PICKFOLDERS) — the same dialog Explorer uses, which works in any Win32 process. It seeds the dialog at the first existing ancestor of the current install path and avoids appending a second \WinState segment when the user picks the existing folder.
Three installer flow/UX changes:
- Add a Summary ("Ready to install") page between Options and Progress.
Nothing touches the disk until the user reviews their choices and
clicks Install. The Next button relabels to Install (or Uninstall in
uninstall mode) on the step immediately before Progress.
- Size the window in DIPs, not raw pixels. AppWindow.Resize takes
physical pixels, so the old 860x560 rendered half-size on 150/200%
displays. Scale 900x600 DIPs by GetDpiForWindow so the wizard is a
consistent physical size regardless of display scaling.
- Stop auto-advancing off the Progress page. The install task used to
jump straight to Finished on completion; now Next simply re-enables
(via OnProgressFinished) so the user can read the step log and
proceed themselves. On failure Next stays disabled — there is no
successful state to move to.
The build was green but threw 22 annotations. Clear them: - Bump actions/checkout, actions/setup-dotnet, actions/upload-artifact from v4 to v5 — v4 runs on Node.js 20, which GitHub deprecates on 2026-06-16 and removes 2026-09-16. v5 runs on Node 24. - Silence the vendored LibreHardwareMonitor PawnIO fork's own build noise without touching the pinned submodule. Pass NoWarn (CA1416 platform, CS0067/CS0168/CS0219 unused, CS0618 obsolete, NU1701) plus SuppressTfmSupportBuildWarnings=true and GeneratePackageOnBuild=false as AdditionalProperties on the ProjectReference, so they apply only to the fork's compilation. NU1701 also propagates into WinState's own restore graph, so it's additionally listed in WinState's NoWarn. Result: clean 0-warning build for both WinState and the installer. The remaining "windows-latest redirect" / "Node 24 default" lines are GitHub-side informational notices, not actionable in the workflow.
Main app — settings: - Add UpdateSourceTrigger=PropertyChanged to every process-count and refresh-interval NumberBox so the WPF-UI control commits to the bound property immediately instead of only on lost focus (the cause of edits appearing not to save). - Show a green "✓ 已儲存" indicator next to a field for ~1.6s after it persists. SettingsViewModel.FlashSaved sets RecentlySavedField and a DispatcherTimer clears it; a SavedFieldToVisibilityConverter lights the matching row. The popup already re-reads the singleton settings cache each tick, so process-list counts update live. Installer — uninstall: - UninstallConfirm page gains two checkboxes: remove saved settings (%AppData%\WinState, default on) and remove the PawnIO driver (default off, with a caution that other apps may share it). - UninstallAsync honours them: deletes the settings folder and runs `winget uninstall namazso.PawnIO` when chosen. Program files are removed last; since the uninstaller runs from inside that folder, a detached cmd retries the locked leftover after the process exits. - Progress heading and window title/title-bar now read Uninstall vs Install based on mode (previously always "Installing"/"Installer"). - Add launchSettings.json with an "Uninstall" profile so the wizard's uninstall path can be launched from Visual Studio.
Add a repo-root Directory.Build.props as the single source of truth for the version (VersionPrefix 0.1.0, VersionSuffix pre). WinState, the installer and the bootstrapper all inherit it; the vendored LibreHardwareMonitor submodule keeps its own version via its own Directory.Build.props. The About page, the installer's Apps & features DisplayVersion and the release tag now all derive from one place instead of drifting (assembly was defaulting to 1.0.0.0 while the git tag said 0.0.1.0).
Move WinState, the WinUI 3 installer and the bootstrapper from net8 to net10 (LTS, supported through Nov 2028 vs net8's Nov 2026, plus runtime perf gains). All three build and the full single-file setup pipeline runs clean on net10: - WinState.csproj: net8.0-windows -> net10.0-windows. WPF-UI 4.0 and all packages resolve unchanged. - WinState.Installer: net8.0-windows10.0.19041.0 -> net10. WindowsApp SDK 1.6 builds and self-contained-publishes on net10 without a bump. - WinState.Bootstrapper: net8.0-windows -> net10.0-windows. - The vendored LibreHardwareMonitor fork is untouched: it multi-targets through net8.0 and a net10 app references the net8.0 target fine. - CI: setup-dotnet pulls 10.0.x. - README: badge + tech-stack + requirements bumped to .NET 10.
Add live, restart-free English / Traditional Chinese localization for
the main app and convert the settings page.
Infrastructure:
- LocalizationService: ResourceManager-backed singleton with a string
indexer; raising PropertyChanged("Item[]") on a culture change
refreshes every bound string instantly (no restart).
- {helpers:Loc Key=...} markup extension binds XAML strings to it.
- Resources/Strings.resx (English, neutral) + Strings.zh-Hant.resx
satellite, auto-embedded by the SDK.
Language setting:
- UserSettingsService persists a Language code ("Auto" / "en" /
"zh-Hant"); Auto follows the system UI culture (any zh* -> zh-Hant,
else English). Applied at startup before the first window shows.
- Settings → Appearance gains a Language dropdown. Switching applies
the culture live and saves it.
Settings page strings (~30) moved from hardcoded zh-TW to {Loc}.
Convert the popup flyout's ~45 static labels (section headers, RAM
breakdown rows, network/disk detail labels, GPU detail rows) to {Loc}
with English + Traditional Chinese entries, and localize the settings
window title. Acronyms and universal technical terms (CPU/GPU/RAM,
PCIe Rx/Tx, R:/W:, the D/M/N icon-placeholder letters) are left as-is.
The popup re-pulls its strings live when the language changes, same as
the settings page, via the LocalizationService indexer.
Installer (WinUI 3): add a static L string table that follows the
system UI language (any zh* -> Traditional Chinese, else English) and
wire every wizard string to it — XAML via {x:Bind loc:L.Xxx} (OneTime,
which suits a once-run installer with no live switch) and code-behind
for the dynamic headings / buttons. Covers Welcome, Options, Summary,
Progress, Finished and UninstallConfirm pages plus the window title and
Back / Next / Cancel / Install / Uninstall / Close buttons.
Main app: the About section showed the leftover WPF-UI placeholder
icon (wpfui-icon-256.png). Register Assets/WinState-icon-512.png as a
Resource and point the About image at it so it shows the real WinState
logo.
程序 leans Simplified-Chinese / means "procedure" in Taiwan usage. Use 行程, the standard Taiwanese term for an OS process, in the process-list settings and the popup process header.
The CPU info rows (Clock / Temp / Volt / Uptime / Processes / Threads / Handles) baked their English label into the binding's StringFormat, so they stayed English under Traditional Chinese. Rewrite each as a MultiBinding that joins a localized label (from LocalizationService) with the formatted value; the label re-pulls on a language switch like the rest of the popup.
Move the uptime row to the bottom-left cell and place processes directly under threads. New fill order: Clock/Temp, Volt/Threads, Handles/Processes, Uptime.
Convert the installer's L string table from a static class to an
INotifyPropertyChanged singleton (L.Instance) and switch every binding
to {x:Bind L.Instance.X, Mode=OneWay}. The Welcome page gains an
English / 繁體中文 dropdown; selecting one flips L.Instance.IsChinese,
which raises PropertyChanged and refreshes every bound string live.
The window title and Back/Next/Cancel/Install buttons are set in code,
so MainWindow subscribes to L.Instance.PropertyChanged and re-runs
RefreshChrome on a language change. Defaults to the system UI language;
the picker just overrides it.
- Move the English / 繁體中文 picker from the Welcome page to the bottom-left of the persistent button bar, so it's reachable from every wizard step. SelectionChanged + initial sync now live in MainWindow; the Welcome page is back to title-only. - Page titles were a mix of FontSize 24 (Options/Progress/Summary) and 28 (Welcome/Finished/Uninstall). Standardize all to 28 so the heading doesn't jump size between steps.
The Progress page step log (Copying…, Creating Start Menu shortcut, Registering Scheduled Task, Installing PawnIO, Removing…, etc.) was hardcoded English even under Traditional Chinese. Route the common step messages through L.Instance so they follow the selected language; rare error-path lines (winget unavailable, non-zero exit codes, exceptions) stay English.
The Progress page heading stayed "Installing" / "安裝中" even after the install completed. Switch it to "Installed" / "已安裝" (or "Uninstalled" / "已解除安裝") on success so the title reflects the finished state.
Switching language on the Progress page reverted the heading to "Installing/安裝中" because the x:Bind OneWay re-applied its bound value over the code-set text. Manage the heading + status entirely in code via a phase state (Running / Complete / Failed / Cancelled) and a RefreshTexts() that the page calls on each phase change and on L.Instance.PropertyChanged. The heading now re-derives correctly (Installing↔Installed, Uninstalling↔Uninstalled) in the active language; the live per-step log line is left untouched mid-run.
Switching language on the Progress page re-ran UpdateButtons, which unconditionally disabled Next on that page — so the Next that OnProgressFinished had enabled after a completed install/uninstall went dead. Track a _progressFinished flag (set on completion, cleared on navigation) and gate Next on it instead of a bare false, so a language switch no longer re-disables an already-available Next.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
More reviews will be available in 27 minutes and 4 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (14)
📝 WalkthroughWalkthroughThis PR adds comprehensive localization infrastructure supporting English and Traditional Chinese, integrates PawnIO hardware driver management into application settings, and introduces a complete single-file installer pipeline combining a bootstrapper wrapper, WinUI multi-page wizard, and installation logic, alongside a .NET 10 upgrade and vendored dependency integration. ChangesLocalization, PawnIO Driver Integration, and Installer Implementation
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request upgrades the application to .NET 10, integrates namazso's PawnIO driver fork of LibreHardwareMonitor to replace the vulnerable WinRing0 driver, and implements live, restart-free localization for English and Traditional Chinese. It also introduces a new WinUI 3 installer, a lightweight bootstrapper, and a PowerShell build script to package the application into a single setup executable. The code review feedback highlights several critical improvement opportunities, including preventing potential deadlocks when executing winget with unread redirected streams, resolving a potential COM object leak in the folder picker, handling enumeration exceptions during uninstallation cleanup, preventing the installer window from being closed mid-progress, and ensuring the main window title dynamically updates upon language changes.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
Comment on lines
+164
to
+172
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "winget", | ||
| Arguments = "uninstall -e --id namazso.PawnIO --silent", | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| }; |
There was a problem hiding this comment.
Redirecting standard output and error streams without reading them can lead to a deadlock if the child process (winget) writes more data than the internal buffer limit. Since the output is not being used, it is safer to disable redirection entirely.
var psi = new ProcessStartInfo
{
FileName = "winget",
Arguments = "uninstall -e --id namazso.PawnIO --silent",
UseShellExecute = false,
CreateNoWindow = true,
};
Comment on lines
+401
to
+409
| var psi = new ProcessStartInfo | ||
| { | ||
| FileName = "winget", | ||
| Arguments = "install -e --id namazso.PawnIO --silent --accept-package-agreements --accept-source-agreements", | ||
| UseShellExecute = false, | ||
| CreateNoWindow = true, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| }; |
There was a problem hiding this comment.
Similarly, redirecting standard output and error streams here without reading them can cause a deadlock if winget produces significant output. Disabling redirection avoids this risk.
var psi = new ProcessStartInfo
{
FileName = "winget",
Arguments = "install -e --id namazso.PawnIO --silent --accept-package-agreements --accept-source-agreements",
UseShellExecute = false,
CreateNoWindow = true,
};
Comment on lines
+388
to
+392
| foreach (var f in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) | ||
| { | ||
| try { File.SetAttributes(f, FileAttributes.Normal); File.Delete(f); } | ||
| catch (Exception ex) { _log($" (skipping {f}: {ex.Message})"); } | ||
| } |
There was a problem hiding this comment.
If Directory.EnumerateFiles throws an exception (e.g., due to unauthorized access or a missing directory), the exception will bubble up and bypass the final Directory.Delete(path, recursive: true) call. Wrapping the file enumeration in a try-catch block ensures that the uninstaller still attempts the final directory deletion.
try
{
foreach (var f in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories))
{
try { File.SetAttributes(f, FileAttributes.Normal); File.Delete(f); }
catch (Exception ex) { _log($" (skipping {f}: {ex.Message})"); }
}
}
catch (Exception ex)
{
_log($" (error enumerating files: {ex.Message})");
}
Comment on lines
+101
to
+108
| if (AppWindow?.Presenter is OverlappedPresenter presenter) | ||
| { | ||
| presenter.IsResizable = false; | ||
| presenter.IsMaximizable = false; | ||
| // Minimizing a modal installer is harmless, but the maximize button next to a | ||
| // non-resizable window looks broken, so drop it too. | ||
| presenter.IsMinimizable = false; | ||
| } |
There was a problem hiding this comment.
To prevent the user from accidentally closing the installer window while an installation or uninstallation is actively running (which could leave the system in a corrupted state), consider subscribing to the AppWindow.Closing event and cancelling it during the progress phase.
if (AppWindow?.Presenter is OverlappedPresenter presenter)
{
presenter.IsResizable = false;
presenter.IsMaximizable = false;
// Minimizing a modal installer is harmless, but the maximize button next to a
// non-resizable window looks broken, so drop it too.
presenter.IsMinimizable = false;
}
if (AppWindow != null)
{
AppWindow.Closing += (sender, args) =>
{
if (PageOrder[_currentIndex] == typeof(ProgressPage) && !_progressFinished)
{
args.Cancel = true;
}
};
}
Comment on lines
+35
to
+45
| dialog.GetResult(out IShellItem item); | ||
| item.GetDisplayName(SIGDN_FILESYSPATH, out IntPtr pszPath); | ||
| try | ||
| { | ||
| return Marshal.PtrToStringUni(pszPath); | ||
| } | ||
| finally | ||
| { | ||
| Marshal.FreeCoTaskMem(pszPath); | ||
| Marshal.ReleaseComObject(item); | ||
| } |
There was a problem hiding this comment.
If item.GetDisplayName throws an exception, the execution will jump to the outer catch block, bypassing the finally block where Marshal.ReleaseComObject(item) is called. This will leak the item COM object. Wrapping the retrieval and usage of item in a nested try-finally block ensures it is always released.
dialog.GetResult(out IShellItem item);
try
{
item.GetDisplayName(SIGDN_FILESYSPATH, out IntPtr pszPath);
try
{
return Marshal.PtrToStringUni(pszPath);
}
finally
{
Marshal.FreeCoTaskMem(pszPath);
}
}
finally
{
Marshal.ReleaseComObject(item);
}| { | ||
| [ObservableProperty] | ||
| private string _applicationTitle = "WinState 設定"; | ||
| private string _applicationTitle = WinState.Services.LocalizationService.Instance.Get("Tray_SettingsTitle"); |
There was a problem hiding this comment.
The _applicationTitle field is initialized once during construction. If the user changes the language live in the settings, the window title will not update. To support live, restart-free localization for the window title, subscribe to LocalizationService.Instance.PropertyChanged and update ApplicationTitle when the language changes.
- Add a "Bilingual (English / 繁體中文, switchable live)" note to both language intros and the Appearance settings row. - Sync the Traditional Chinese Download section to the guided-installer flow (was still describing the old bare-exe path). - Use 行程 for "process" throughout the Chinese copy to match the app.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 13
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ViewModels/Pages/SettingsViewModel.cs (1)
215-225:⚠️ Potential issue | 🟠 Major | ⚡ Quick winClamp refresh intervals before persisting settings.
SaveRefreshSettings()writes raw values. If any refresh field is temporarily invalid, a valid edit in another field can still persist that bad value. Clamp all fields at save time (same pattern already used inSaveProcessListSettings).Suggested patch
private void SaveRefreshSettings() { if (!_isInitialized) return; _userSettingsService.SaveRefreshSettings(new RefreshSettings { - Cpu = CpuRefreshMs, - Gpu = GpuRefreshMs, - Memory = MemoryRefreshMs, - Disk = DiskRefreshMs, - Network = NetworkRefreshMs + Cpu = Math.Clamp(CpuRefreshMs, RefreshSettings.Min, RefreshSettings.Max), + Gpu = Math.Clamp(GpuRefreshMs, RefreshSettings.Min, RefreshSettings.Max), + Memory = Math.Clamp(MemoryRefreshMs, RefreshSettings.Min, RefreshSettings.Max), + Disk = Math.Clamp(DiskRefreshMs, RefreshSettings.Min, RefreshSettings.Max), + Network = Math.Clamp(NetworkRefreshMs, RefreshSettings.Min, RefreshSettings.Max) }); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ViewModels/Pages/SettingsViewModel.cs` around lines 215 - 225, SaveRefreshSettings persists raw refresh values and can save invalid intervals; modify SaveRefreshSettings so it clamps each refresh field before calling _userSettingsService.SaveRefreshSettings. Follow the same clamping pattern used in SaveProcessListSettings: compute clamped values for CpuRefreshMs, GpuRefreshMs, MemoryRefreshMs, DiskRefreshMs and NetworkRefreshMs (or call the existing clamp helper if present), populate the new RefreshSettings with those clamped values (Cpu, Gpu, Memory, Disk, Network) and then call _userSettingsService.SaveRefreshSettings with the sanitized object.
🧹 Nitpick comments (4)
.gitignore (1)
380-380: ⚡ Quick winConsider consolidating duplicate
artifacts/pattern.The
artifacts/pattern is already present at line 64 (from the standard .NET Core gitignore section). Line 380 duplicates it with installer-specific documentation.While functionally harmless, duplicate gitignore entries can cause maintenance confusion. Consider either:
- Removing line 380 and updating line 64's comment to note it covers both .NET Core and installer artifacts, or
- Keeping line 380 and adding a note that it intentionally duplicates line 64 for documentation clarity.
♻️ Proposed consolidation approach
Option 1: Update line 64 comment and remove line 380
# .NET Core project.lock.json project.fragment.lock.json -artifacts/ +artifacts/ # Covers both .NET build artifacts and local installer artifacts from scripts/build-installer.ps1Then remove line 380.
Option 2: Document intentional duplication at line 380
-# Local install artifacts produced by scripts/build-installer.ps1. +# Local install artifacts produced by scripts/build-installer.ps1. Note: artifacts/ +# is also present at line 64 from the standard .NET template; this is intentional +# for documentation clarity. artifacts/🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.gitignore at line 380, Remove the duplicate "artifacts/" entry at the later occurrence and update the existing "artifacts/" entry in the .gitignore (the one in the .NET Core section) to include a brief comment noting it also covers installer-generated artifacts, so a single entry documents both uses and eliminates redundancy.WinState.Installer/Services/InstallerLogic.cs (2)
384-395: ⚖️ Poor tradeoffConsider symbolic link protection in install directory deletion.
DeleteInstallDirectoryrecursively enumerates and deletes all files without checking for symbolic links or junctions. If a malicious user creates a symbolic link inside the install directory pointing to a system folder, uninstall could delete system files (running as admin).This is a low-probability attack vector since the user would need to create the symlink after installation but before uninstall, and the installer already runs as admin. However, validating that enumerated paths remain within the install directory boundary adds defense in depth.
🛡️ Optional hardening
Check that each file path being deleted is actually under the install directory:
private void DeleteInstallDirectory(string path) { + var installDirFullPath = Path.GetFullPath(path); // Hand-roll instead of Directory.Delete(recursive) so we can swallow ACL hiccups on // individual files without aborting the whole uninstall. foreach (var f in Directory.EnumerateFiles(path, "*", SearchOption.AllDirectories)) { - try { File.SetAttributes(f, FileAttributes.Normal); File.Delete(f); } + try + { + // Ensure file is actually under install dir (protect against symlink attacks) + if (!Path.GetFullPath(f).StartsWith(installDirFullPath, StringComparison.OrdinalIgnoreCase)) + { + _log($" (skipping {f}: outside install directory)"); + continue; + } + File.SetAttributes(f, FileAttributes.Normal); + File.Delete(f); + } catch (Exception ex) { _log($" (skipping {f}: {ex.Message})"); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@WinState.Installer/Services/InstallerLogic.cs` around lines 384 - 395, DeleteInstallDirectory currently enumerates and deletes every file under the given path without guarding against symbolic links/junctions; update the deletion loop in DeleteInstallDirectory to (1) resolve each file's absolute path (Path.GetFullPath) and verify it starts with the intended install directory path to ensure it stays inside the boundary, (2) skip entries whose attributes include FileAttributes.ReparsePoint (symlinks/junctions) to avoid following links, and (3) continue to set normal attributes and delete only when the path check passes; keep the existing try/catch logging behavior when skipping or failing to delete.
267-284: 💤 Low valueProcess kill on timeout may leave scheduled task in inconsistent state.
If
schtasks.exedoes not complete within 10 seconds,RunSchTaskskills the entire process tree. This could leave the scheduled task store in an inconsistent or corrupted state, especially during task creation or deletion operations. Windows Task Scheduler operations are typically fast, but network delays or high system load could cause legitimate timeouts.Consider increasing the timeout or removing the kill logic in favor of letting the process complete, since scheduled task corruption is more severe than a slightly longer installer run.
♻️ Possible improvements
Option 1: Increase timeout to 30 seconds:
- if (!p.WaitForExit(10_000)) + if (!p.WaitForExit(30_000))Option 2: Remove the kill entirely and wait indefinitely (schtasks operations should complete quickly in practice):
- if (!p.WaitForExit(10_000)) - { - try { p.Kill(entireProcessTree: true); } catch { } - return -1; - } + p.WaitForExit();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@WinState.Installer/Services/InstallerLogic.cs` around lines 267 - 284, RunSchTasks currently kills the schtasks process tree after a hard 10_000ms timeout which risks corrupting the Task Scheduler; change the timeout to 30_000ms by updating the p.WaitForExit call to use 30_000 and remove the p.Kill(entireProcessTree: true) try/catch block (and its associated early return) so the function waits longer for schtasks to finish rather than forcibly terminating it.WinState.Installer/Pages/FinishedPage.xaml.cs (1)
15-26: 💤 Low valueUninstall-mode text won't update on language switch.
In uninstall mode, Lines 23-24 directly assign localized strings to
HeadingText.TextandBodyText.Textrather than using reactive bindings. If the user switches language while on the FinishedPage during uninstall, the heading and body text will remain in the previous language (though other UI elements bound viax:Bindwill update).Given that FinishedPage is the final step where Cancel is hidden, this edge case is unlikely to impact users in practice.
♻️ Optional fix to maintain consistency
Subscribe to language changes in the page and refresh the text:
protected override void OnNavigatedTo(NavigationEventArgs e) { base.OnNavigatedTo(e); // In uninstall mode the "Launch WinState now" choice doesn't apply. if ((App.Current as App)?.IsUninstallMode == true) { LaunchNowCheckbox.IsChecked = false; LaunchNowCheckbox.Visibility = Microsoft.UI.Xaml.Visibility.Collapsed; - HeadingText.Text = L.Instance.FinishedUninstalledTitle; - BodyText.Text = L.Instance.FinishedUninstalledBody; + RefreshUninstallText(); + L.Instance.PropertyChanged += (_, _) => RefreshUninstallText(); } } + +private void RefreshUninstallText() +{ + HeadingText.Text = L.Instance.FinishedUninstalledTitle; + BodyText.Text = L.Instance.FinishedUninstalledBody; +}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@WinState.Installer/Pages/FinishedPage.xaml.cs` around lines 15 - 26, The direct assignments to HeadingText.Text and BodyText.Text in FinishedPage.OnNavigatedTo (when (App.Current as App)?.IsUninstallMode == true) cause the heading/body to not update on runtime language changes; instead either convert these text properties to use the same x:Bind/multi-bind pattern as other UI elements or subscribe to the app language-changed event (e.g., App.LanguageChanged or similar) in FinishedPage, and in the handler set HeadingText.Text = L.Instance.FinishedUninstalledTitle and BodyText.Text = L.Instance.FinishedUninstalledBody (and unsubscribe on page unload). Ensure this change is applied only in the uninstall branch of OnNavigatedTo and that you unsubscribe to avoid leaks.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/build.yml:
- Line 28: The workflow currently pins third-party steps by tag (e.g.,
actions/checkout@v5, actions/setup-dotnet@v5, actions/upload-artifact@v5);
replace each tag reference with the corresponding immutable commit SHA (you can
keep the `@v5` as a trailing comment for readability) so the `uses:` entries point
to a full commit SHA instead of a movable tag; update every occurrence of these
symbols (actions/checkout, actions/setup-dotnet, actions/upload-artifact) in the
workflow to their verified SHAs and commit the changes.
In `@README.md`:
- Line 207: In the README.md Traditional Chinese section there is a blank line
inside a blockquote causing markdownlint errors; fix it the same way as the
English section example by either merging the blockquote lines (remove the blank
line) or converting the blank line into a continued blockquote line by adding a
leading ">" so the entire quote remains a single blockquote; update the
Traditional Chinese blockquote accordingly to eliminate the empty line inside
the quote.
- Line 109: There is a blank line inside a blockquote that breaks markdownlint;
edit the blockquote so it either removes that blank line to merge the two
paragraphs or add a leading ">" on the blank line to continue the blockquote;
ensure the change updates the blockquote markers (">") accordingly so the
paragraphs remain inside the same blockquote and markdownlint no longer reports
the warning.
In `@ViewModels/Pages/SettingsViewModel.cs`:
- Around line 108-114: Replace the hardcoded English status messages in the
PawnIODriverStatusText assignment so they come from localized resources; instead
of returning string literals in the switch on PawnIODriverService.GetState(),
map each PawnIODriverState (Running, Stopped, NotInstalled, default) to a
resource lookup (e.g., Resources.PawnIODriver_Running,
IStringLocalizer["PawnIODriver_Stopped"], etc.) and set PawnIODriverStatusText
to the localized value; ensure you add the corresponding keys to your .resx or
localization source and use the same resource identifiers across cultures so the
UI language switches correctly.
In `@ViewModels/Windows/MainWindowViewModel.cs`:
- Line 113: The ApplicationTitle is initialized once into the private field
_applicationTitle and never updated; subscribe to
LocalizationService.Instance.PropertyChanged (or its Item[]/Culture change
event) inside the MainWindowViewModel constructor, and when the localization
change fires, re-read the localized string (use
WinState.Services.LocalizationService.Instance.Get("Tray_SettingsTitle")),
assign it to _applicationTitle (or set the ApplicationTitle property) and raise
the property change notification (e.g.,
OnPropertyChanged(nameof(ApplicationTitle))); also unsubscribe from the
localization event when the view model is disposed/closed to avoid leaks.
In `@Views/Pages/SettingsPage.xaml`:
- Around line 54-62: Replace the hardcoded Content strings so the UI uses the
localization resources instead of English literals: update the RadioButton
controls (the ones with GroupName="themeSelect", IsChecked bound to
ViewModel.CurrentTheme and Command bound to ViewModel.ChangeThemeCommand) to set
Content from your resource/localization system (e.g., a StaticResource or
resource binding like {x:Static ...} or a localized string via x:Uid) rather
than Content="Light"/"Dark", and likewise replace the literal "PawnIO driver" at
the referenced control (around the other control at the same file) with the
corresponding localized resource key; ensure keys match your resource file and
keep existing bindings/CommandParameter values unchanged.
In `@WinState.Bootstrapper/Program.cs`:
- Around line 70-72: Process.Start(psi) is being null-forgiven and then used
directly; replace the null-forgiving usage with an explicit null check: call
Process.Start(psi), assign to variable p, if p is null throw or return a clear
error (e.g., throw new InvalidOperationException or log and return a specific
exit code) before calling p.WaitForExit(), then return p.ExitCode; update the
block around Process.Start, p.WaitForExit and return p.ExitCode to handle the
null case explicitly.
In `@WinState.Installer/Pages/OptionsPage.xaml.cs`:
- Around line 20-24: Remove the empty/dead if block that checks "if (App.Current
is App && Microsoft.UI.Xaml.Window.Current is null)" along with its comment;
this code does nothing and should be deleted from OptionsPage.xaml.cs so the
method/class no longer contains an empty conditional—simply remove the entire if
(...) { /* comment */ } block to improve clarity.
In `@WinState.Installer/Pages/ProgressPage.xaml.cs`:
- Line 17: The CancellationTokenSource field _cts is never disposed; update the
page to call _cts.Dispose() when the page is unloaded (hook the Unloaded event
handler) and/or dispose it when the long-running operations complete (e.g., in
the finally blocks that follow the operations) so timer resources are released;
ensure you reference the _cts instance and avoid using it after disposal, and if
you add an Unloaded handler implement a null-check or guard to prevent
double-dispose.
In `@WinState.Installer/Pages/SummaryPage.xaml.cs`:
- Around line 20-22: Replace the hardcoded English strings in the SummaryPage
code by pulling localized strings from the resource accessor L: change the
expressions that set PawnIOText.Text, LaunchText.Text and ShortcutText.Text to
use L.SummaryInstallPawnIO / L.SummarySkipPawnIO, L.SummaryLaunchAtLogon /
L.SummaryDontLaunch, and L.SummaryAddStartMenuShortcut /
L.SummaryNoStartMenuShortcut (or similarly named keys) depending on the boolean
flags (o.InstallPawnIO, o.LaunchAtLogon, o.CreateStartMenuShortcut); then add
those new keys and their Traditional Chinese and English values to the
localization resource files so the UI shows the correct language.
In `@WinState.Installer/Services/InstallerLogic.cs`:
- Around line 30-43: InstallAsync currently creates and writes to
options.InstallPath without validation; before calling Directory.CreateDirectory
or copying the payload, normalize options.InstallPath (e.g., Path.GetFullPath)
and validate it does not point to or reside under protected system locations
such as Environment.SystemDirectory,
Environment.GetFolderPath(SpecialFolder.Windows) (and their subpaths) or the
root of the system drive; if validation fails, throw a descriptive exception and
stop the install. Apply this check in InstallAsync immediately after
ResolvePayloadPath succeeds and before Directory.CreateDirectory/ File.Copy
(affecting InstallAsync, InstallOptions.InstallPath, ResolvePayloadPath,
Directory.CreateDirectory, File.Copy, _log) so malicious or mistaken paths like
C:\Windows\System32 are rejected.
- Around line 207-223: In CreateStartMenuShortcut, the dynamic result of
shell.CreateShortcut(path) isn't validated and may be null; after calling
dynamic shortcut = shell.CreateShortcut(path) add a null check (e.g., if
(shortcut is null) return; or log/throw) before accessing shortcut.TargetPath,
WorkingDirectory, IconLocation, Description, or calling shortcut.Save(), so you
avoid a RuntimeBinderException when shell.CreateShortcut returns null.
In `@WinState.Installer/Services/NativeFolderPicker.cs`:
- Around line 35-45: The current code calls dialog.GetResult and then
item.GetDisplayName but only releases the COM item in a finally that runs after
GetDisplayName, so if GetDisplayName throws the IShellItem referenced by item is
leaked; to fix, expand the try/finally that calls Marshal.FreeCoTaskMem and
Marshal.ReleaseComObject to wrap both dialog.GetResult(out IShellItem item) and
item.GetDisplayName(SIGDN_FILESYSPATH, out IntPtr pszPath) (or move the
declaration of item outside and release it in an outer finally), ensuring
Marshal.FreeCoTaskMem(pszPath) is only called if pszPath was set and always
calling Marshal.ReleaseComObject(item) to avoid COM leaks.
---
Outside diff comments:
In `@ViewModels/Pages/SettingsViewModel.cs`:
- Around line 215-225: SaveRefreshSettings persists raw refresh values and can
save invalid intervals; modify SaveRefreshSettings so it clamps each refresh
field before calling _userSettingsService.SaveRefreshSettings. Follow the same
clamping pattern used in SaveProcessListSettings: compute clamped values for
CpuRefreshMs, GpuRefreshMs, MemoryRefreshMs, DiskRefreshMs and NetworkRefreshMs
(or call the existing clamp helper if present), populate the new RefreshSettings
with those clamped values (Cpu, Gpu, Memory, Disk, Network) and then call
_userSettingsService.SaveRefreshSettings with the sanitized object.
---
Nitpick comments:
In @.gitignore:
- Line 380: Remove the duplicate "artifacts/" entry at the later occurrence and
update the existing "artifacts/" entry in the .gitignore (the one in the .NET
Core section) to include a brief comment noting it also covers
installer-generated artifacts, so a single entry documents both uses and
eliminates redundancy.
In `@WinState.Installer/Pages/FinishedPage.xaml.cs`:
- Around line 15-26: The direct assignments to HeadingText.Text and
BodyText.Text in FinishedPage.OnNavigatedTo (when (App.Current as
App)?.IsUninstallMode == true) cause the heading/body to not update on runtime
language changes; instead either convert these text properties to use the same
x:Bind/multi-bind pattern as other UI elements or subscribe to the app
language-changed event (e.g., App.LanguageChanged or similar) in FinishedPage,
and in the handler set HeadingText.Text = L.Instance.FinishedUninstalledTitle
and BodyText.Text = L.Instance.FinishedUninstalledBody (and unsubscribe on page
unload). Ensure this change is applied only in the uninstall branch of
OnNavigatedTo and that you unsubscribe to avoid leaks.
In `@WinState.Installer/Services/InstallerLogic.cs`:
- Around line 384-395: DeleteInstallDirectory currently enumerates and deletes
every file under the given path without guarding against symbolic
links/junctions; update the deletion loop in DeleteInstallDirectory to (1)
resolve each file's absolute path (Path.GetFullPath) and verify it starts with
the intended install directory path to ensure it stays inside the boundary, (2)
skip entries whose attributes include FileAttributes.ReparsePoint
(symlinks/junctions) to avoid following links, and (3) continue to set normal
attributes and delete only when the path check passes; keep the existing
try/catch logging behavior when skipping or failing to delete.
- Around line 267-284: RunSchTasks currently kills the schtasks process tree
after a hard 10_000ms timeout which risks corrupting the Task Scheduler; change
the timeout to 30_000ms by updating the p.WaitForExit call to use 30_000 and
remove the p.Kill(entireProcessTree: true) try/catch block (and its associated
early return) so the function waits longer for schtasks to finish rather than
forcibly terminating it.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e0599342-651d-453a-96b3-58d397a833f3
📒 Files selected for processing (47)
.github/workflows/build.yml.gitignore.gitmodulesApp.xaml.csDirectory.Build.propsHelpers/LocExtension.csHelpers/SavedFieldToVisibilityConverter.csREADME.mdResources/Strings.resxResources/Strings.zh-Hant.resxServices/LocalizationService.csServices/PawnIODriverService.csServices/SystemInfoService.csServices/UserSettingsService.csVendor/LibreHardwareMonitor-PawnIOViewModels/Pages/SettingsViewModel.csViewModels/Windows/MainWindowViewModel.csViewModels/Windows/PopupControl.xamlViews/Pages/SettingsPage.xamlWinState.Bootstrapper/Program.csWinState.Bootstrapper/WinState.Bootstrapper.csprojWinState.Bootstrapper/app.manifestWinState.Installer/App.xamlWinState.Installer/App.xaml.csWinState.Installer/L.csWinState.Installer/MainWindow.xamlWinState.Installer/MainWindow.xaml.csWinState.Installer/Pages/FinishedPage.xamlWinState.Installer/Pages/FinishedPage.xaml.csWinState.Installer/Pages/OptionsPage.xamlWinState.Installer/Pages/OptionsPage.xaml.csWinState.Installer/Pages/ProgressPage.xamlWinState.Installer/Pages/ProgressPage.xaml.csWinState.Installer/Pages/SummaryPage.xamlWinState.Installer/Pages/SummaryPage.xaml.csWinState.Installer/Pages/UninstallConfirmPage.xamlWinState.Installer/Pages/UninstallConfirmPage.xaml.csWinState.Installer/Pages/WelcomePage.xamlWinState.Installer/Pages/WelcomePage.xaml.csWinState.Installer/Properties/launchSettings.jsonWinState.Installer/Services/InstallerLogic.csWinState.Installer/Services/NativeFolderPicker.csWinState.Installer/WinState.Installer.csprojWinState.Installer/app.manifestWinState.csprojWinState.slnscripts/build-installer.ps1
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| - uses: actions/checkout@v5 |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify all workflow 'uses:' references are SHA-pinned
rg -nP '^\s*uses:\s*[^@\s]+@([0-9a-fA-F]{40})\s*$' .github/workflows || true
echo "---- Non-SHA pinned uses ----"
rg -nP '^\s*uses:\s*[^@\s]+@([^ \t#]+)\s*$' .github/workflows | rg -v '[0-9a-fA-F]{40}' || trueRepository: KoukeNeko/WinState
Length of output: 306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- build.yml uses lines (with line numbers) ----"
nl -ba .github/workflows/build.yml | sed -n '1,160p' | rg 'uses:'
echo "---- Raw lines 20-120 for context ----"
nl -ba .github/workflows/build.yml | sed -n '20,120p'
echo "---- Non-SHA pinned uses (as detected by previous regex) ----"
rg -nP '^\s*uses:\s*[^@\s]+@([^ \t#]+)\s*$' .github/workflows | rg -v '[0-9a-fA-F]{40}' || trueRepository: KoukeNeko/WinState
Length of output: 172
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "---- build.yml uses lines (with line numbers) ----"
awk '{printf "%5d:%s\n", NR, $0}' .github/workflows/build.yml | rg 'uses:'
echo "---- build.yml lines 15-130 for context ----"
awk '{printf "%5d:%s\n", NR, $0}' .github/workflows/build.yml | sed -n '15,130p'
echo "---- Non-SHA pinned uses (as detected by previous regex) ----"
rg -nP '^\s*uses:\s*[^@\s]+@([^ \t#]+)\s*$' .github/workflows | rg -v '[0-9a-fA-F]{40}' || true
echo "---- SHA-pinned uses (should match 40-hex SHAs only) ----"
rg -nP '^\s*uses:\s*[^@\s]+@([0-9a-fA-F]{40})\s*$' .github/workflows || trueRepository: KoukeNeko/WinState
Length of output: 4818
Pin third-party GitHub Actions to immutable commit SHAs.
actions/checkout@v5, actions/setup-dotnet@v5, and actions/upload-artifact@v5 are tag-pinned but not SHA-pinned, leaving CI exposed to tag retargeting/supply-chain drift; pin each uses: to the full commit SHA (optionally keep @v5 as a readability comment).
Also applies to: 39-39, 61-61, 108-108.
🧰 Tools
🪛 zizmor (1.25.2)
[error] 28-28: unpinned action reference (unpinned-uses): action is not pinned to a hash (required by blanket policy)
(unpinned-uses)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/build.yml at line 28, The workflow currently pins
third-party steps by tag (e.g., actions/checkout@v5, actions/setup-dotnet@v5,
actions/upload-artifact@v5); replace each tag reference with the corresponding
immutable commit SHA (you can keep the `@v5` as a trailing comment for
readability) so the `uses:` entries point to a full commit SHA instead of a
movable tag; update every occurrence of these symbols (actions/checkout,
actions/setup-dotnet, actions/upload-artifact) in the workflow to their verified
SHAs and commit the changes.
Source: Linters/SAST tools
- InstallerLogic: drop the unread stdout/stderr redirects on both winget calls (install + uninstall) — a chatty winget could fill the pipe and deadlock WaitForExit. Localize the now-reachable "PawnIO removed" line. - InstallerLogic.DeleteInstallDirectory: wrap the EnumerateFiles walk in try/catch so an access error mid-walk doesn't skip the final Directory.Delete. - NativeFolderPicker: nest the item usage in try/finally so a throwing GetDisplayName doesn't leak the COM IShellItem. - MainWindow: cancel AppWindow.Closing while the Progress page is mid install/uninstall so the window can't be torn down into a half- installed state. - MainWindowViewModel: subscribe to LocalizationService.PropertyChanged and refresh ApplicationTitle so the settings window title follows a live language switch (was set once at construction).
- Localize installer summary lines and Settings driver-status text - Validate install path, rejecting system/root dirs - Null-check Start Menu shortcut and bootstrapper child process - Dispose CTS on ProgressPage unload; drop dead OptionsPage branch - Merge adjacent README blockquotes to fix MD028
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Brings
masterup to the latestdev. Headline changes since the last merge:Bilingual UI (English + 繁體中文)
LocalizationServiceindexer +{Loc}markup extension. Settings → Appearance gains a language dropdown (Auto / English / 繁體中文); Auto follows the system language.Strings.resx(English neutral) +Strings.zh-Hant.resxsatellite..NET 10
Installer polish
Settings + versioning
Directory.Build.props(0.1.2-pre); CI bumped to .NET 10 SDK and actions v5.Test plan
Summary by CodeRabbit
Release Notes
New Features
Chores