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" }