From 9caf824027c53f45c9a183a7aebb8fe5c66db60d Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Thu, 20 Aug 2026 07:59:30 +0000 Subject: [PATCH] chore: adopt the standards baseline docs and config MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit STANDARD.md sections 1.9, 4.11, 5.2, 5.4 and 5.7. Low risk: no CI change, no ruleset work, no build behaviour change. global.json (1.9) previously pinned only the Microsoft.Testing.Platform runner and left the SDK floating, so a contributor on an older SDK gets different analyzer results from CI — and with TreatWarningsAsErrors that is a build which fails for them and passes for everyone else. Now 10.0.100 with rollForward: latestFeature. The canonical .editorconfig (5.2) carries one substantive fix over this repo's copy: the private-field naming rule matched constants, because a const IS a field under applicable_kinds = field, so it demanded _nonceSize for `private const int NonceSize`. An empty required_modifiers scopes it to instance fields. EnforceCodeStyleInBuild is off (1.2.1 is blocked), so this changes no build outcome today and is correct for when it does. The canonical .gitignore is this repo's own, upstreamed — the only delta is dropping a duplicate trailing `artifacts/` that the canonical copy already covers twice. Verified artifacts/ is still ignored. SECURITY.md (4.11) is not the template verbatim. Reporting and Supported versions are; Scope is rewritten, because the template's is about credential storage and would have described types this package does not contain. The replacement states what is actually claimed: SHA-256 gives integrity, not authenticity — the expected hash ships from the same release as the asset and there is no signature checking; AllowInsecureManifestSource and UseDefaultSha256Verifier=false defeat verification by design and are documented as such; traversal defence is ZipFile/TarFile's, not ours; GhCliReleaseSource trusts whatever `gh` is on PATH. CLAUDE.md (5.7) points at the standards repo for the baseline rather than restating clauses locally, and spends its repo-specific section on things an agent would otherwise get wrong here. Verified: Release build 0 warnings, 392 tests passing across net8.0 and net10.0 on the pinned SDK. Co-Authored-By: Claude Opus 5 (1M context) --- .editorconfig | 6 ++ .github/PULL_REQUEST_TEMPLATE.md | 21 +++++++ .gitignore | 3 - CHANGELOG.md | 4 ++ CLAUDE.md | 101 +++++++++++++++++++++++++++++++ CONTRIBUTING.md | 32 ++++++++++ SECURITY.md | 54 +++++++++++++++++ global.json | 4 ++ 8 files changed, 222 insertions(+), 3 deletions(-) create mode 100644 .github/PULL_REQUEST_TEMPLATE.md create mode 100644 CLAUDE.md create mode 100644 CONTRIBUTING.md create mode 100644 SECURITY.md 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" }