Harden runtime integrity and modernize CI tooling - #131
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces several robustness and security improvements across the codebase. Key changes include fixing potential memory leaks by properly disposing of GDI+ images in forms, localizing time-lapse strings, updating the Curve25519 implementation to clamp private keys safely within try-finally blocks (clearing sensitive data afterwards), and using a secure file system wrapper for profile deletion. Additionally, a bug in GetNumSize was resolved, and unit tests were expanded. The reviewer feedback correctly identifies a security vulnerability in GetSharedSecret where a null peerPublicKey is not validated, potentially leading to incorrect cryptographic operations, and suggests adding a null check.
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.
There was a problem hiding this comment.
Pull request overview
This PR hardens WireSockUI runtime integrity around cryptographic scalar handling, profile deletion, and UI resource ownership, while updating CI/release workflows to pinned, current GitHub Action releases.
Changes:
- Hardened Curve25519/X25519 helpers to clamp private scalars internally, mask peer u-coordinate high bits, and fix signing-key arithmetic.
- Eliminated path-based profile deletion race by deleting via a validated open handle, and fixed GDI image ownership/disposal in WinForms menus/buttons.
- Modernized CI/release workflows by pinning official actions to updated immutable releases and documenting runner requirements.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| WireSockUI/Forms/TaskManager.cs | Owns/disposes the refresh button bitmap to prevent GDI leaks. |
| WireSockUI/Forms/frmMain.cs | Tracks/disposes menu images and deletes profiles via a validated handle. |
| WireSockUI/Forms/frmEdit.cs | Owns/disposes editor menu images to prevent GDI leaks. |
| WireSockUI/Extensions/TimeExtensions.cs | Uses localized “day”/“yesterday” resource text. |
| WireSockUI/Config/Curve25519.cs | Clamps private scalars internally, masks peer high bit, and fixes signing-size scan. |
| WireSockUI.Tests/Program.cs | Adds regressions for localization, non-mutation, high-bit masking, and optional signing keys. |
| README.md | Documents updated self-hosted runner minimum version requirement. |
| .github/workflows/sdk-integration.yml | Updates pinned checkout/setup-dotnet action SHAs. |
| .github/workflows/sdk-contract-drift.yml | Updates pinned checkout action SHAs for drift workflow. |
| .github/workflows/main.yml | Updates pinned checkout/setup-dotnet/artifact/attestation actions for releases. |
| .github/workflows/ci.yml | Updates pinned checkout/setup-dotnet action SHAs for CI. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
This PR follows a full production-readiness review of startup/shutdown, direct
wgbooster.dllintegration, profile/configuration handling, persistence, native lifecycle recovery, security boundaries, UI resource ownership, tests, and CI/release workflows.No Critical or High defects were found. It fixes all confirmed Medium and Low findings from the review while preserving the direct SDK/minimal-install architecture.
Findings fixed
KeyGenInlinedocumented nullable signing output but rejected null. The implementation now matches its contract.Compatibility review
wgbooster.dllloading and the elevated minimal SDK installation model are unchanged.wgbooster.handwgbooster.defhave no contract drift; no snapshot change is required.Verification
dotnet formatverificationOne symbolic-link test remains skipped on this host because file symlink creation is unavailable; its handle-based cleanup companion test passes. Elevated real-SDK lifecycle smoke cannot be reproduced locally and remains dependent on the protected x64/ARM64
wiresock-sdkrunners and repository variables documented in the README.