diff --git a/.editorconfig b/.editorconfig index 41f2e91..bd0de72 100644 --- a/.editorconfig +++ b/.editorconfig @@ -59,6 +59,12 @@ dotnet_naming_rule.private_fields_should_be_camel_case.style = camel_case_with_u dotnet_naming_symbols.private_fields.applicable_kinds = field dotnet_naming_symbols.private_fields.applicable_accessibilities = private +# A const IS a field, so without this the rule demands `_nonceSize` for +# `private const int NonceSize` — PascalCase constants are correct .NET style and +# the codebase uses them throughout. Restricting the rule to instance fields keeps +# it aimed at what it was written for. Found when EnforceCodeStyleInBuild surfaced +# 76 IDE1006 violations, every one of them a constant. +dotnet_naming_symbols.private_fields.required_modifiers = dotnet_naming_style.camel_case_with_underscore.capitalization = camel_case dotnet_naming_style.camel_case_with_underscore.required_prefix = _ diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md new file mode 100644 index 0000000..923388a --- /dev/null +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -0,0 +1,21 @@ +## What changed + + + +## Why + + + +## Checklist + +- [ ] Build is clean — no new warnings (`TreatWarningsAsErrors` is on) +- [ ] Tests pass on **every** shipped target framework +- [ ] Public API changes carry XML docs +- [ ] `CHANGELOG.md` updated under `[Unreleased]` +- [ ] Dependency floors unchanged, or the consumer impact is described below + +## Consumer impact + + diff --git a/.gitignore b/.gitignore index da2d068..8fdfdb9 100644 --- a/.gitignore +++ b/.gitignore @@ -436,6 +436,3 @@ FodyWeavers.xsd *.msix *.msm *.msp - -# Local NuGet pack output (csproj writes to artifacts/packages on Release builds). -artifacts/ diff --git a/CHANGELOG.md b/CHANGELOG.md index 7799271..137a0e4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,9 +15,13 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **CI now has a single aggregating gate job, `ci`, and it is the only required status check.** `build` and `test` were required directly before, which couples the branch ruleset to the matrix: `test`'s check names carry the matrix values, so adding or dropping a platform broke protection. The gate declares `needs: [build, test]` with `if: always()` and fails on any upstream result that is not success — including `skipped`, which branch protection would otherwise read as satisfied. - **Every workflow declares `concurrency`, explicit `permissions`, and per-job `timeout-minutes`.** Superseded pushes cancel instead of stacking up, except on tags — a half-cancelled release can leave an incomplete package set on nuget.org. NuGet restore is cached on `~/.nuget/packages`. - **`.github/dependabot.yml` rewritten.** Minor and patch bumps are grouped into one PR per ecosystem; majors are deliberately left ungrouped so each arrives separately and stays open for review. The two runtime-aligned packages carrying per-TFM floors (`Microsoft.Extensions.DependencyInjection.Abstractions`, `Microsoft.Extensions.Http`) are now under `ignore` for major updates, because an 8.x → 10.x bump on the net8 floor is never mergeable and was weekly noise. +- **`global.json` now pins the SDK**, not just the Microsoft.Testing.Platform runner: `10.0.100` with `rollForward: latestFeature`. An unpinned SDK means a contributor on an older one gets different analyzer results from CI, and with `TreatWarningsAsErrors` that is a build which fails for them and passes for everyone else. +- **`.gitignore` and `.editorconfig` replaced with the canonical copies.** The `.editorconfig` change is one line that matters: the private-field naming rule had `applicable_kinds = field`, and a `const` *is* a field, so the rule demanded `_nonceSize` for `private const int NonceSize`. An empty `required_modifiers` scopes it to instance fields. Nothing enforces these rules at build time yet (`EnforceCodeStyleInBuild` is off), so this is a no-op for the build today and correct for when it is not. ### Added +- **`SECURITY.md`**, with a scope section specific to this library: SHA-256 verification establishes integrity but not authenticity (the expected hash ships from the same release as the asset, and there is no signature checking); `AllowInsecureManifestSource` and `UseDefaultSha256Verifier = false` are documented opt-outs that defeat verification by design; archive path-traversal defence is the framework's `ZipFile`/`TarFile` guard rather than this library's; and `GhCliReleaseSource` trusts whatever `gh` is on `PATH`. Stating the boundary is the point — a report that only restates a documented limitation is not a vulnerability. +- **`CONTRIBUTING.md`, `.github/PULL_REQUEST_TEMPLATE.md` and `CLAUDE.md`.** `CLAUDE.md` records the constraints an agent would otherwise violate here — why the three-platform matrix is load-bearing, why one install-lock test returns early on Windows by design, and that `PackageValidationBaselineVersion` tracks the last shipped release. - **CodeQL code scanning** (`codeql.yml`), weekly plus on every push and PR, with the `security-and-quality` query pack. The build is explicit rather than `autobuild`, which has been observed to pick a single TFM and silently analyse half a multi-targeted codebase. - **Dependabot auto-merge for minor and patch bumps** (`dependabot-auto-merge.yml`), queued behind the `ci` gate. Majors are never auto-merged. Approval uses an `AUTO_MERGE_PAT` Dependabot secret owned by a code owner — an Actions secret of the same name resolves to an empty string in a Dependabot-triggered workflow, and a `GITHUB_TOKEN` approval cannot satisfy a code-owner review. diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..becb0f2 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,101 @@ +# CLAUDE.md — NextIteration.SpectreConsole.SelfUpdate + +## This package + +Self-update for Spectre.Console CLIs. A consumer registers an update source — +GitHub Releases over HTTP, GitHub Releases via the `gh` CLI for private repos, a +generic HTTPS manifest, or a custom `IUpdateSource` — and gets a drop-in `update` +command plus an `update check` variant wired into its existing `CommandApp`. The +pipeline resolves the right asset for the running OS/architecture, downloads it, +verifies a SHA-256, expands the archive, and swaps the installed files atomically +with rollback on failure. + +Consumed by CLI tools that ship as self-contained directories and replace +themselves in place. The install path is the part that matters: a failed update +must leave a working application behind, not a half-written one. + +## Things that are easy to get wrong here + +- **The three-platform test matrix is load-bearing, not ceremony.** The library + resolves an OS/arch RID token, picks a per-OS cache directory (`AppData`, + `~/Library/Caches`, `XDG_CACHE_HOME`), replaces a *running* executable, and + takes an install lock whose `FileShare.None` + `FileOptions.DeleteOnClose` + semantics differ between POSIX and Windows. Dropping a leg stops testing a + shipped code path. `InstallLockTests.Acquire_when_directory_not_writable_throws_not_writable` + returns early on Windows *by design* — read its comment before "fixing" it; a + platform-guarded test passes vacuously off its platform. +- **SHA-256 verification proves integrity, not authenticity.** The expected hash + comes from the same release as the asset. Do not describe it, in docs or in + XML comments, as protecting against a malicious publisher — see `SECURITY.md` + for the boundary that is actually claimed. +- **`AllowInsecureManifestSource` and `UseDefaultSha256Verifier=false` are + deliberate opt-outs**, documented as tests/trusted-network only. Do not + "harden" them away; do not widen where they apply. +- **Per-TFM dependency floors are deliberate.** `Microsoft.Extensions.Http` and + `Microsoft.Extensions.DependencyInjection.Abstractions` are floored at 8.0.x + for `net8.0` and 10.0.x for `net10.0`. Raising the net8 floor to a 10.x version + drags every net8 LTS consumer off its own servicing line. Dependabot is + configured never to propose it; do not do it by hand either. +- **`PackageValidationBaselineVersion` is set to the last shipped release.** An + accidental public-API break fails the build rather than shipping. When the + version is bumped for a release, the baseline moves with it — not before. +- **Cleanup must stay silent when there is nothing to clean.** `UpdateCleanup` + shows a status message only when `HasPendingCleanup` is true, because it runs at + the very start of every `Main` and the no-leftovers case is the common one. +- **The demo project is not shipped** (`IsPackable=false`, `net10.0` only) but it + is in the solution and it builds in CI, so it must compile warning-free like + everything else. + +## Repository baseline + +This repo conforms to +[NextIteration.Standards](https://github.com/StuartMeeks/NextIteration.Standards). +Build properties, test stack, CI shape, and branch protection are defined there, not +here. Before changing any of those, read `STANDARD.md`; if this repo needs to deviate, +that is an `EXCEPTIONS.md` entry in the standards repo, not a local difference. + +## Non-negotiables + +- **The build must be clean.** `TreatWarningsAsErrors` is on and analyzers run at + `latest`. A warning is a build failure. +- **Tests must pass on every shipped target framework** (`net8.0` and `net10.0`). A change + that only passes on one is not finished. Shipping a target you do not test is a defect, + not a scoping decision. +- **Dependency floors are deliberate and per-TFM.** A `PackageReference` version in a + library is a *minimum* NuGet forces on every consumer, so raising a floor is a + consumer-visible change even when nothing in the code needs it. Never raise one to + silence a warning. +- **Public API changes need XML docs.** `GenerateDocumentationFile` is on and the public + surface is fully documented. +- **Update `CHANGELOG.md`** under `[Unreleased]`, saying what changed and why. + +## Dependabot + +Minor and patch updates auto-merge behind CI. Major updates stay open for a human — that +is deliberate, not a backlog to clear. Packages with per-TFM floors have major updates +suppressed entirely via `ignore`; bump those by hand when a new .NET major lands. + +## After opening a pull request + +Watch CI to completion, report the real check results, then **offer to merge** in the same +message. Do not stop silently and wait to be asked. + +- If branch protection blocks the merge, say so and offer `gh pr merge --admin`. These + repos require a code-owner review only the maintainer can give, which is why `--admin` is + the tool — but that mechanic is not the reason the offer is wanted. The reason is simply + that the maintainer has grown comfortable delegating this to an agent, so treat the + latest instruction as authoritative over this file. +- **Merge only on an explicit yes.** The offer is pre-approved; the action is not. +- Never offer while checks are failing or still running. Report that state instead. +- Report the checks that actually ran. A skipped check is not a passing check, and branch + protection treats them differently from how they read in a summary. + +## CI + +The single required status check is `ci` — an aggregating gate over `build` and `test`. +Renaming those jobs is safe; the ruleset never names them. Do not make them required +checks directly. + +Publishing lives in `ci.yml`, not a separate `release.yml` — and the nuget.org Trusted +Publishing policy is bound to that *filename*. Renaming the workflow file requires +updating the policy in the same change, or the next publish fails to authenticate. diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md new file mode 100644 index 0000000..ebf67cd --- /dev/null +++ b/CONTRIBUTING.md @@ -0,0 +1,32 @@ +# Contributing + +Issues and pull requests are welcome. + +## Before you open a PR + +- **The build must be clean.** `TreatWarningsAsErrors` is on and analyzers run at + `latest`. A warning is a build failure, not a suggestion. +- **Tests run on every target framework** the project ships. `dotnet test` covers + `net8.0` and `net10.0`; a change that only passes on one is not finished. +- **Public API changes need XML docs.** `GenerateDocumentationFile` is on and the + public surface is fully documented — keep it that way. +- **Update `CHANGELOG.md`.** Keep a Changelog format, under `[Unreleased]`. Say what + changed and why; "bump dependency" without a reason is not useful six months later. + +## Dependency changes + +Dependency floors are deliberate and per target framework. A `PackageReference` +version in a library is a *minimum* NuGet forces on every consumer, so raising a +floor is a consumer-visible change even when nothing in the code needs it. Read +`STANDARD.md` sections 1.4 and 1.5 in `NextIteration.Standards` before changing one. + +Minor and patch bumps arrive automatically via Dependabot and merge behind CI. +Major bumps stay open for a human — that is deliberate, not a backlog. + +## Repository conventions + +These repositories share a baseline defined in +[NextIteration.Standards](https://github.com/StuartMeeks/NextIteration.Standards): +build properties, test stack, CI shape, and branch protection. If a change would +deviate from it, raise that there first — a per-repo exception is a documented +entry, not a quiet difference. diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 0000000..34b1843 --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,54 @@ +# Security policy + +## Reporting a vulnerability + +Report privately through GitHub's **Report a vulnerability** button under this +repository's Security tab, which opens a private advisory visible only to the +maintainers. Please do not open a public issue for a suspected vulnerability. + +Include the affected package and version, what an attacker can achieve, and a +reproduction if you have one. + +You can expect an acknowledgement within 7 days, an assessment within 14, and +credit in the advisory and changelog unless you ask otherwise. + +## Supported versions + +Only the latest released minor of each package receives security fixes. These are +pre-1.0 libraries and there are no long-term support branches. + +## Scope + +This library downloads a release archive, checks it against a published SHA-256 +hash, expands it, and replaces the running application's files. Four things are +explicitly **not** claimed: + +- **SHA-256 verification establishes integrity, not authenticity.** The expected + hash comes from the same place as the asset — either the asset's own `sha256` + metadata, or a `SHA256SUMS.txt` sibling on the *same* release. Whoever can + replace the asset can replace the hash alongside it. `Sha256ChecksumVerifier` + detects corruption and tampering in transit; it does not prove who built the + release. There is no signature checking. Supply your own `IPackageVerifier` if + you need provenance rather than integrity. +- **HTTPS is the authenticity boundary, and it is defeatable by configuration.** + `HttpManifestSource` refuses non-`https` manifest and asset URLs unless + `SelfUpdaterOptions.AllowInsecureManifestSource` is set, and setting it defeats + the verifier outright — a hash served over plain HTTP is as MITM-able as the + bytes it describes. Likewise `UseDefaultSha256Verifier = false` removes hash + checking entirely. Both are documented opt-outs for tests and trusted networks; + a report that either is "insecure when enabled" restates the documentation. +- **Archive path-traversal defence is the framework's, not this library's.** + Extraction goes through `ZipFile.ExtractToDirectory` and + `TarFile.ExtractToDirectoryAsync`, which reject entries resolving outside the + destination directory. A traversal escape is a .NET issue, and should be + reported upstream — though tell us too, so this library can guard explicitly. +- **The `gh` CLI source trusts the local `gh`.** `GhCliReleaseSource` starts the + `gh` executable found on `PATH` and inherits whatever credentials it holds. A + shadowed or compromised `gh` on `PATH` is outside the boundary, as is anything + reachable by an attacker who can already write to the install directory — the + installer runs as the invoking user and replaces files that user could replace + anyway. + +Reports demonstrating a break *within* those stated boundaries are in scope and +welcome. Reports that only restate a documented limitation are not +vulnerabilities. diff --git a/global.json b/global.json index 3140116..1d364c6 100644 --- a/global.json +++ b/global.json @@ -1,4 +1,8 @@ { + "sdk": { + "version": "10.0.100", + "rollForward": "latestFeature" + }, "test": { "runner": "Microsoft.Testing.Platform" }