Skip to content

v0.1.1: security & correctness fixes from external review#1

Merged
StuartMeeks merged 2 commits intomainfrom
security-fixes-from-review
May 3, 2026
Merged

v0.1.1: security & correctness fixes from external review#1
StuartMeeks merged 2 commits intomainfrom
security-fixes-from-review

Conversation

@StuartMeeks
Copy link
Copy Markdown
Owner

Summary

Six findings from an OpenAI Code review of v0.1.0, all addressed:

# Severity Fix
1 Critical Lock acquired before any staging mutation. Otherwise a second installer could wipe a first installer's in-flight staging dir on its way to losing the lock race.
2 Critical Swap rolls back from .old/ on copy failure. New RestoreFromOld helper, tested directly + via deterministic Phase 2 failure scenario.
3 High New ValidateAssetName rejects path separators, parent refs, rooted paths, and any name whose Path.GetFileName doesn't round-trip — closes a path-traversal vector.
4 High HttpManifestSource rejects non-HTTPS manifest URLs and non-HTTPS asset URLs by default (plain HTTP defeats SHA-256 verification). Opt in via new SelfUpdaterOptions.AllowInsecureManifestSource.
5 Medium TOCTOU-safe install path: ISelfUpdater.GetLatestReleaseAsync() + InstallAsync(RemoteRelease, …) overload. UpdateCommand fetches the release once and installs that exact instance. Parameterless InstallAsync retained as a documented convenience.
6 Medium csproj realigned with the Auth repo: GeneratePackageOnBuild is Release-only; PackageOutputPath moved from C:\nuget-local\ to $(MSBuildThisFileDirectory)..\..\artifacts\packages (platform-neutral, repo-local, gitignored). CI no longer needs -p:PackageOutputPath= override.

Version bumped to 0.1.1.

Test plan

  • dotnet build --configuration Release clean (TreatWarningsAsErrors)
  • dotnet test --configuration Release150 tests, 0 failures (was 125)
  • dotnet build --configuration Debug no longer auto-packs (verified locally; no .nupkg produced)
  • dotnet build --configuration Release packs to artifacts/packages/ (no stray C:/ dir)
  • CI green on Linux / macOS / Windows
  • CHANGELOG entry reflects the changes

🤖 Generated with Claude Code

StuartMeeks and others added 2 commits May 3, 2026 04:36
- lock before staging mutation (prevents concurrent installer wiping
  another installer's in-flight staging dir on its way to losing the
  lock race)
- rollback swap on copy failure (restore from .old/ instead of
  leaving a half-populated install dir)
- validate asset name (reject path separators, parent refs, rooted
  paths — closes a path-traversal vector for malicious sources)
- enforce HTTPS in HttpManifestSource by default (plain HTTP defeats
  SHA-256 verification because the SHA itself is MITM-able); opt in
  via SelfUpdaterOptions.AllowInsecureManifestSource for tests / dev
- TOCTOU-safe install path: UpdateCommand fetches the release once
  and installs that exact instance. Adds ISelfUpdater.GetLatestRelease
  Async + InstallAsync(RemoteRelease, ...) overload; keeps
  parameterless InstallAsync as a documented convenience
- realign csproj with sibling repos: GeneratePackageOnBuild only on
  Release; PackageOutputPath moved from C:\\nuget-local to
  artifacts/packages (platform-neutral, repo-local, gitignored)

150 tests, all passing (was 125).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Matches the CHANGELOG entry for the security & correctness fixes
in this branch.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@StuartMeeks StuartMeeks merged commit 021128b into main May 3, 2026
5 checks passed
@StuartMeeks StuartMeeks deleted the security-fixes-from-review branch May 3, 2026 11:14
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.

1 participant