From 4fd021ac638fbe8d67478a8840fe4651c665ac93 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Thu, 20 Aug 2026 11:48:42 +0000 Subject: [PATCH] chore: adopt the standard docs, editor config and SDK pin PR B of the NextIteration.Standards adoption sequence. Closes 1.9, 4.11, 5.2, 5.4 and 5.7 for this repo. Touches no CI and no ruleset. SECURITY.md and CLAUDE.md are written for this repo rather than copied. The template's SECURITY.md scope section describes Auth's credential encryption, which is not what this library does: it writes plain-text JSON, so the useful thing to state is that settings are not secrets and that atomic writes are a crash-consistency guarantee rather than a concurrency one. CLAUDE.md's "easy to get wrong here" list names the four traps this codebase actually has -- the inert-until-Bind rule that stops load from triggering a write, the debounced fire-and-forget path that no test may sleep against, AtomicFile's deliberately per-platform replace primitive, and tolerant deserialisation being the on-disk contract rather than a convenience. global.json gains the SDK pin. It previously set only the test runner, which left the SDK floating, so a contributor on an older SDK gets different analyzer results from CI and TreatWarningsAsErrors turns that into a build that fails for them and passes for everyone else. The canonical .editorconfig scopes the private-field naming rule to instance fields; a const is a field, so the old rule wanted _nonceSize for `private const int NonceSize`. EnforceCodeStyleInBuild is NOT enabled -- 1.2.1 is blocked. No tracked file becomes ignored by the canonical .gitignore. Build clean at zero warnings and 64/64 tests pass on net8.0 and net10.0 with the pinned SDK (10.0.111, via latestFeature from the 10.0.100 band). Co-Authored-By: Claude Opus 5 (1M context) --- .editorconfig | 6 +++ .github/PULL_REQUEST_TEMPLATE.md | 21 ++++++++ .gitignore | 13 +++-- CHANGELOG.md | 11 ++++ CLAUDE.md | 93 ++++++++++++++++++++++++++++++++ CONTRIBUTING.md | 32 +++++++++++ SECURITY.md | 46 ++++++++++++++++ global.json | 4 ++ 8 files changed, 223 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 7ec041a..8fdfdb9 100644 --- a/.gitignore +++ b/.gitignore @@ -225,6 +225,16 @@ PublishScripts/ *.nupkg # NuGet Symbol Packages *.snupkg +# Stray "C:/" / "c:/" directory created on non-Windows when the csproj's +# Windows-style PackageOutputPath is interpreted as a relative path. +**/[Cc]:/ + +# Per-user Claude Code state — never commit +.claude/settings.local.json +.claude/projects/ + +# Pack output (matches CI's --output flag) +artifacts/ # The packages folder can be ignored because of Package Restore **/[Pp]ackages/* # except build/, which is used as an MSBuild target. @@ -426,6 +436,3 @@ FodyWeavers.xsd *.msix *.msm *.msp - -# Claude Code local (personal) settings -.claude/settings.local.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 81bf2bc..fca0c89 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,10 +69,21 @@ floor. referenced but never invoked. Workflows gained `concurrency`, `timeout-minutes`, a least-privilege `permissions` block and a NuGet cache. +- `global.json` now pins the SDK (`10.0.100`, `rollForward: latestFeature`) as well + as the test runner. Without a pin a contributor on an older SDK gets different + analyzer results from CI, and `TreatWarningsAsErrors` turns that into a build that + fails for them and passes for everyone else. +- Adopted the canonical `.gitignore` and `.editorconfig`. The `.editorconfig` change + scopes the private-field naming rule to instance fields — a `const` is a field, so + the rule previously demanded `_nonceSize` for `private const int NonceSize`. + ### Added - CodeQL code scanning (`security-and-quality` query pack), weekly plus on every push and pull request. +- `SECURITY.md`, `CONTRIBUTING.md`, a pull request template, and a root `CLAUDE.md`. + `SECURITY.md` states the scope this library does and does not claim — settings are + stored as plain-text JSON and are explicitly not a place for secrets. - Dependabot for NuGet and GitHub Actions, with minor and patch updates grouped and auto-merged behind CI, and majors left open for review. Major updates to `Microsoft.Extensions.DependencyInjection.Abstractions` are suppressed, because diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..4658352 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,93 @@ +# CLAUDE.md — NextIteration.SpectreConsole.Settings + +## This package + +Strongly-typed, JSON-persisted settings for CLI tools built on Spectre.Console. A +consumer derives a class from `SettingsBase`, registers it with `AddSettings` (giving +an explicit `SettingsDirectory` — there is no default), and injects it into their +commands. Each settings class gets its own `{ClassName}.json` file in that directory. +Property changes persist automatically on a debounced background write, or only on +`Save()` in `Explicit` mode. `AddSettingsCommands()` wires a ready-made `settings +list` / `settings reset` branch into an existing `CommandApp`. Nothing here consumes +another package in the estate, and nothing in the estate consumes it. + +## Things that are easy to get wrong here + +- **A `SettingsBase` instance is inert until the framework `Bind`s it.** That is what + stops the JSON deserializer's own property assignments from scheduling a write during + load — the deserializer runs first, `Bind` runs after. Moving `Bind` earlier turns + every load into a write. +- **Automatic persistence is debounced *and* fire-and-forget.** Nothing signals that a + write completed, so a test cannot sleep a fixed interval and assert; it must poll for + the observable effect. That is what `Infrastructure/Wait.cs` is for. A negative + assertion ("no write happened") is the one case that keeps an explicit quiet window, + because there is nothing to wait for and too short a wait can only mask a bug. +- **`AtomicFile` deliberately uses a different replace primitive per platform.** + `File.Move(overwrite: true)` is `rename(2)` on POSIX, which replaces a destination + another handle holds open and serialises concurrent renames; on Windows it is + `MoveFileEx`, which throws in both cases. Windows must go through `File.Replace`. Do + not "simplify" the two branches back into one — that is a bug that shipped for three + releases because the test matrix was Linux-only. +- **Tolerant deserialisation is the on-disk contract, not a convenience.** Unknown JSON + properties are ignored and missing ones fall back to constructed defaults, which is + what lets a consumer add or remove a setting without a migration. Tightening the + serializer options — `UnmappedMemberHandling`, a strict naming policy, dropping + `JsonStringEnumConverter` — breaks every settings file already on disk. +- **`SettingsDirectory` has no smart default and registration throws without one.** That + is deliberate: guessing at `~/.config/{something}` on a consumer's behalf picks a name + the consumer has to live with forever. + +## 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. Here that is + `Microsoft.Extensions.DependencyInjection.Abstractions`: 8.0.x for `net8.0`, 10.0.x for + `net10.0`. +- **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. + +`ci.yml` also carries a `release` job beyond the four `STANDARD.md` 3.1 names. It is +tag-gated and downstream of `publish`, and cuts the GitHub release from `CHANGELOG.md`. 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..44f4cba --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,46 @@ +# 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 writes application settings to **plain-text JSON** on the local +filesystem, at a directory the consuming application chooses. Three things are +explicitly **not** claimed: + +- **Settings are not secrets.** Nothing is encrypted, obfuscated, or held in + protected memory, and the file is created with whatever permissions the calling + process's umask and the parent directory give it. Do not store API keys, tokens + or passwords in a `SettingsBase` class. Use + [NextIteration.SpectreConsole.Auth](https://github.com/StuartMeeks/NextIteration.SpectreConsole.Auth) + for credentials — that is what it is for. +- **The consumer chooses the directory, and owns it.** `SettingsDirectory` is + required and unvalidated beyond being a path; pointing it at a world-writable + location, or at a path assembled from untrusted input, is the caller's decision + and the caller's exposure. +- **Atomic writes are a crash-consistency guarantee, not a concurrency one.** + `AtomicFile` guarantees a reader sees either the whole old file or the whole new + file. It does not serialise writers: two processes writing concurrently observe + last-write-wins, and one process's changes can be lost. + +In scope and welcome: anything that breaks *within* those boundaries — a write that +leaves a partial or corrupt file readable, a path in the library itself that escapes +`SettingsDirectory`, deserialisation of a settings file causing something worse than +a thrown exception, or a settings value reaching disk somewhere other than the file +it was registered for. Reports that only restate a documented limitation above 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" }