Skip to content

Harden runtime integrity and modernize CI tooling#131

Merged
wiresock merged 2 commits into
mainfrom
codex/runtime-integrity-review
Jul 18, 2026
Merged

Harden runtime integrity and modernize CI tooling#131
wiresock merged 2 commits into
mainfrom
codex/runtime-integrity-review

Conversation

@wiresock

Copy link
Copy Markdown
Owner

Summary

This PR follows a full production-readiness review of startup/shutdown, direct wgbooster.dll integration, 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

  • Medium - X25519 scalar decoding: public-key and shared-secret helpers assumed callers had already clamped private scalars. They now clone and clamp every scalar according to RFC 7748 without mutating caller buffers.
  • Medium - X25519 peer decoding: the top bit of peer u-coordinates was not masked. Decoding now follows RFC 7748 and has a high-bit regression vector.
  • Medium - signing-key arithmetic: the significant-byte scan started out of bounds and searched for zero rather than non-zero bytes, making the signing-key path throw. The scan is now bounded and correct.
  • Low - key generation contract: KeyGenInline documented nullable signing output but rejected null. The implementation now matches its contract.
  • Low - profile deletion race: UI deletion validated a pathname and later deleted by pathname. It now validates, bounds, rejects hard links, and deletes the same open file handle.
  • Low - GDI resource leaks: dynamically generated menu and refresh bitmaps are now explicitly owned, detached, and disposed by their forms.
  • Low - localization: the 24-48 hour age label now uses the existing localized resource rather than hardcoded English.
  • CI/release maintenance: all official actions are pinned to current immutable releases: checkout v7, setup-dotnet v6, upload-artifact v7, download-artifact v8, and actions/attest v4. The self-hosted runner requirement is documented as 2.329.0 or newer.

Compatibility review

  • Direct wgbooster.dll loading and the elevated minimal SDK installation model are unchanged.
  • The managed ABI still matches all 24 pinned native exports.
  • Current upstream SDK main is ahead of the pinned snapshot, but wgbooster.h and wgbooster.def have no contract drift; no snapshot change is required.
  • Existing Amnezia 2.0 support and validation for H1-H4, S1-S4, Id, Ip, and Ib remain covered.

Verification

  • Release x64 solution build with warnings as errors
  • Release UWP x64 solution build with warnings as errors
  • Full Release and Release UWP test harnesses
  • RFC 7748 public-key/shared-secret, non-mutation, high-bit, and signing regressions
  • Pinned SDK contract verification (24 exports)
  • Six-entry publish matrix: Release/Release UWP x AnyCPU/ARM64/x64
  • dotnet format verification
  • Workflow YAML parsing and staged diff checks
  • NuGet direct/transitive vulnerability audit: no known vulnerable packages

One 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-sdk runners and repository variables documented in the README.

Copilot AI review requested due to automatic review settings July 18, 2026 08:25

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread WireSockUI/Config/Curve25519.cs

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@wiresock
wiresock merged commit e1ef30f into main Jul 18, 2026
10 checks passed
@wiresock
wiresock deleted the codex/runtime-integrity-review branch July 18, 2026 14:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants