Harden production reliability, security, and CI#133
Conversation
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
There was a problem hiding this comment.
Pull request overview
This PR performs a broad hardening pass across WireSockUI: tightening trust boundaries (startup identity, config validation, filesystem operations), bounding/race-proofing tunnel lifecycle behaviors, improving UI responsiveness for large data, and strengthening CI/reproducibility via pinned toolchains and locked restores.
Changes:
- Add security validations around startup identity, profile parsing/allowlisting, and safer shortcut + secured-folder handling.
- Make tunnel lifecycle behaviors more bounded/race-safe (virtual-adapter rename queuing/timeout handling, recovery flows) and reduce expensive UI rendering/retained data.
- Pin .NET SDK + dependency lockfiles and expand CI/build matrix to x86/x64 with locked restores and safer workflow defaults.
Reviewed changes
Copilot reviewed 41 out of 41 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| WireSockUI/WireSockUI.csproj | Adds RIDs and enables NuGet lockfiles/locked-mode in CI. |
| WireSockUI/WireSockManager.cs | Introduces bounded, cancellable virtual-adapter rename queue + WMI renamer abstraction. |
| WireSockUI/Program.cs | Adds interactive-user vs process-user validation and special-folder root validation at startup. |
| WireSockUI/packages.lock.json | New lockfile for deterministic restores. |
| WireSockUI/Notifications/Notifications.cs | Ensures notification shortcut on demand and improves activation visibility. |
| WireSockUI/Native/WireguardConfigParser.cs | Escapes diagnostic tokens to prevent unsafe/unreadable error output. |
| WireSockUI/Native/WindowsApplicationContext.cs | Hardens notification shortcut creation against TOCTOU and untrusted existing objects. |
| WireSockUI/Native/ShellLink.cs | Hardens PROPVARIANT interop and string extraction. |
| WireSockUI/Native/SecureFileSystem.cs | Adds stable directory-chain open mode and validates newly created regular files + handle-based deletes. |
| WireSockUI/Native/ProcessSnapshotCache.cs | Adds serialized, cached process enumeration for UI use. |
| WireSockUI/Native/ProcessList.cs | Adds cancellation support and switches process “user” identity to raw SID for performance. |
| WireSockUI/Native/ProcessEntry.cs | Updates docs to reflect SID-based ownership storage. |
| WireSockUI/Native/CommonControlExtensions.cs | Adds SuspendRedraw() scope helper to bound WinForms redraw costs. |
| WireSockUI/Global.cs | Validates special-folder roots and tightens ACL mutation trust checks. |
| WireSockUI/Forms/TunnelMonitor.cs | Defers connecting monitor start until native connect completion signal. |
| WireSockUI/Forms/TunnelLifecycleController.cs | Improves shutdown diagnostics and preserves network-lock release diagnostics. |
| WireSockUI/Forms/TaskManager.cs | Uses cached process snapshots and avoids per-process account-name lookups; simplifies icon handling. |
| WireSockUI/Forms/ProfileDisplayFormatter.cs | Adds bounded formatting for large comma-delimited profile values. |
| WireSockUI/Forms/NativeOperationRecoveryPolicy.cs | Adds helper to await/normalize timed-out native-operation completions. |
| WireSockUI/Forms/MainWindowStatePresentation.cs | Centralizes UI state/presentation decisions for main window. |
| WireSockUI/Forms/frmSettings.cs | Hardens autorun inspection/migration flows and serializes autorun operations. |
| WireSockUI/Forms/frmMain.cs | Improves recovery handling, virtualized log rendering, and taskbar visibility behavior. |
| WireSockUI/Forms/frmEdit.cs | Bounds syntax highlighting work and uses redraw suspension for responsiveness. |
| WireSockUI/Forms/BoundedRingBuffer.cs | Adds fixed-capacity ring buffer used for virtual log list. |
| WireSockUI/Config/SettingsUpdateCoordinator.cs | Adds optional “commit” step to compensating transaction for deferred autorun migration cleanup. |
| WireSockUI/Config/ProfileFileTransaction.cs | Cleans managed transaction orphans before enforcing entry limits. |
| WireSockUI/Config/ProfileConfigurationEditor.cs | Adds section-aware, safe insertion/append logic for peer application rules. |
| WireSockUI/Config/Profile.cs | Enforces allowlisted sections/keys and improves diagnostics for invalid config values. |
| WireSockUI/Config/LegacyProfileMigrationService.cs | Cleans managed staging temporaries before catalog limit enforcement. |
| WireSockUI.Tests/WireSockUI.Tests.csproj | Enables lockfiles/locked-mode and adds x86 test platform coverage. |
| WireSockUI.Tests/SecurityConfigRegressionTests.cs | Adds regression tests for hardened filesystem + shortcut behaviors and interop safety. |
| WireSockUI.Tests/Program.UiReliabilityTests.cs | Adds regression tests for editor insertion, autorun classification, and process snapshot caching. |
| WireSockUI.Tests/packages.lock.json | New lockfile for deterministic test restores. |
| WireSockUI.sln | Adds x86 solution configurations/mappings. |
| README.md | Documents new security/CI expectations and trust boundaries. |
| global.json | Pins .NET SDK used by CI and builds. |
| .github/workflows/sdk-integration.yml | Prevents PR code from running on privileged hosts; uses global.json + locked restore; removes persisted creds. |
| .github/workflows/sdk-contract-drift.yml | Disables persisted credentials for checkout. |
| .github/workflows/main.yml | Pins SDK via global.json, locked restore, x86/x64 test matrix, and tightens signing/SBOM steps. |
| .github/workflows/ci.yml | Adds least-privilege permissions, locked restores, x86/x64 tests, and prevents PR execution on elevated runners. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 41 out of 41 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
WireSockUI/Forms/frmSettings.cs:874
IsTaskDefinitionOwnedByExecutablevalidatessettings.PriorityagainstProcessPriorityClass.BelowNormal, butTaskSettings.Priorityis a numeric Task Scheduler priority (0–10). This check will incorrectly fail even for tasks created by WireSock UI, causing canonical/ownership detection to break.
settings.Hidden ||
!settings.AllowDemandStart ||
settings.DeleteExpiredTaskAfter != TimeSpan.Zero ||
settings.Priority != ProcessPriorityClass.BelowNormal ||
settings.Volatile ||
settings.DisallowStartOnRemoteAppSession ||
Summary
Validation
Known SDK-level limitation
The shared fixed Global\WiresockClientService event can still be pre-created to cause an availability-only startup denial. WireSock UI validates ownership and ACLs and fails closed, so the object is never trusted and cannot grant driver control. Eliminating name squatting requires a coordinated WireSock SDK/service bootstrap or protected-private-namespace change and is documented in the README.