Conversation
Detailed single-file reference covering the public API, internal generation flow, confirmed bugs (error-string returns, off-by-one character selection, modulo bias, non-uniform shuffle, static RNG), build/test results, packaging hygiene, feature gaps, and a prioritised v3 plan. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
The default MSBuild step on the old image used MSBuild 14, which cannot parse SDK-style csproj files (MSB4041). Switch to dotnet restore/build/ test/pack on the VS2019 image and publish the nupkg as a build artifact. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Add 'deploy: off' so no NuGet (or other) deployment runs, regardless of any deploy provider configured in the AppVeyor project UI. CI only builds, tests, and packs the nupkg as a downloadable artifact. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Re-checks every original-review bug (§5) and feature gap (§8) against the current master source, marking each Confirmed / Already Fixed / Partially Fixed with code references. Corrects the addendum's premise that the Guid shuffle was removed (it still exists at Password.cs:247-249 on both master and dev/v2) and confirms master is ahead of the older dev/v2 line. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
New docs/ index plus paired current-state/ (as-is architecture, generation flow, API surface) and v3-target/ (target architecture, generation flow, API surface, config & DI, before/after, roadmap) sets. Diagrams show what the package does today, where v3 is headed, and why each change is better, tied back to verified issues. All 33 mermaid diagrams validated with the mermaid parser. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Phase 0 (toolchain: install .NET SDK via bash + green baseline) through Phase 6 (docs/migration). Each phase has objective, tasks, files, and exit criteria, plus an issue-to-phase traceability table and a per-session checklist. Linked from the docs index. All 34 mermaid diagrams validated. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
Add a "Definition of done" loop (implement -> test -> green -> commit/push) that applies to every phase, strengthen the working principles, and append an explicit tests-green gate plus a per-phase commit step to each phase's exit criteria and the per-session checklist. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd
…5dOd Add v3 package documentation and code review
* Phase 1: correctness & security core Replace biased/static randomness and error-string returns with a fail-loud, unbiased, instance-scoped design: - Add IRandomSource + CryptoRandomSource (rejection sampling, no modulo bias, no off-by-one; instance-based and disposable) (§5.2, §5.3, §5.6) - Delete the Guid shuffle and dead GetRngCryptoSeed; use crypto Fisher-Yates via IRandomSource (§5.5, §5.7) - Guarantee included classes by construction (seed one per class, fill, shuffle) and drop the validate-and-retry loop / "Try again" string - Next() throws ArgumentException on invalid config; add TryNext(out string); validate empty custom special set up front (§5.1, §5.8) - Expose IPasswordSettings.CharacterGroups; make Password IDisposable - Update tests to expect exceptions; add Phase1CorrectnessTests All 32 tests pass (run on net8 since netcoreapp2.2 is EOL); library and netcoreapp2.2 test build both succeed. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd * Phase 2: targets & test modernisation - Multi-target netstandard2.0;net8.0 with nullable enabled; net8 uses RandomNumberGenerator.GetInt32, netstandard2.0 keeps rejection sampling - Migrate tests to net8.0 + NUnit 4 (constraint-model asserts), dropping the EOL netcoreapp2.2 target and its vulnerable Microsoft.NETCore.App 2.2.0 - Add edge-case/property tests (uniqueness, custom-pool exclusivity, length boundaries, no-classes, null random source) - Add a BenchmarkDotNet project (single vs batch, sizes 1/100/1000/10000, MemoryDiagnoser) and wire it into the solution - Bump AppVeyor image to Visual Studio 2022 for the net8 SDK - Tighten .gitignore for bin/obj/artifacts and BenchmarkDotNet output 39/39 tests pass on net8; both library TFMs and the full solution build with no nullable warnings. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd * Phase 3: async API, DI registration, remove v2 wrappers - Add IPasswordGenerator (Next/TryNext/NextAsync/Generate/GenerateAsync); Password implements it alongside IPassword. Async honours CancellationToken - Keep sync methods fully supported (no [Obsolete]): generation is CPU-bound, so obsoleting sync in favour of async would be an anti-pattern - Add opt-in DI: AddPasswordGenerator(Action<PasswordOptions>) and AddPasswordGenerator(IConfiguration) in the core package, taking Microsoft.Extensions.DependencyInjection.Abstractions and Configuration.Binder dependencies; new vs DI behave identically - Add PasswordOptions for code/appSettings configuration - Remove the [Obsolete] PasswordGenerator/PasswordGeneratorSettings wrappers and their tests, clearing all 5 CS0108 warnings - Enable nullable in the test project; add Phase3Tests (async, cancellation, batch, DI binding + equivalence) Library builds with 0 warnings/0 errors; 39/39 tests pass on net8; pack declares the new dependencies per target framework. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd * Phase 4: presets, custom pools, exclude-ambiguous, min-counts, entropy - Custom pools: WithCharacters(string) and WithAllAscii() alongside Include* - ExcludeAmbiguous() strips look-alike characters (CharacterFilter) from the pool and per-class groups - RequireAtLeast(CharacterClass, count): generalises the one-per-class guarantee; auto-enables the class; validated against length - IEntropyEstimator/PoolEntropyEstimator and Password.EstimateEntropyBits() (length * log2(effective pool size)) - Presets ForOwasp/ForNist/ForOtp/ForApiKey/ForEnvironmentName (static factories over the fluent builder) and ForPassphrase backed by a small built-in word list (PassphraseGenerator) - Batch: parameterless Generate()/GenerateAsync() driven by a configurable DefaultBatchCount; kept Generate(count) (no .Count(n) chaining) - appSettings precedence via AddPasswordGenerator(IConfiguration, Action): code-configure > appSettings > default; PasswordOptions gains ExcludeAmbiguous and DefaultBatchCount Library builds with 0 warnings/0 errors; 55/55 tests pass on net8 (16 new Phase 4 tests). https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd * build: clean packaging, SourceLink, snupkg, bump to 3.0.0 - Delete stale PasswordGenerator.nuspec (2.0.5); csproj is now the single source of version truth, bumped to 3.0.0 - Replace PackageIconUrl with PackageIcon (clears NU5048) and add PackageReadmeFile (root Readme.md packed as README.md) - Add SourceLink (Microsoft.SourceLink.GitHub), deterministic build, CI build flag, symbol package (.snupkg), embedded untracked sources - Refresh Description/ReleaseNotes/Tags/Copyright for v3 - appveyor: bump build version label to 3.0, capture .snupkg artifact, keep deploy: off dotnet pack -c Release produces PasswordGenerator.3.0.0.nupkg + .snupkg with no NU5048 / no missing-readme warnings; 55/55 tests still pass. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd * docs: v2->v3 migration guide, standards mapping, readme refresh - Add docs/v3-target/migration-v2-to-v3.md: error-string->exception/TryNext breaking change, opt-in DI/async/batch, OWASP/NIST preset mapping, and the broader use cases (OTP/API key/identifier/passphrase) - Add root CHANGELOG.md with the 3.0.0 entry - Refresh root Readme.md: fix the stale 8-128 length claim (actual 4-256), replace ```javascript fences with ```csharp, document presets, quality controls, error handling, async/batch and DI; link docs + changelog - Mark current-state/ docs as historical (issues resolved in v3) and link the migration guide from docs/README.md - Back Readme/migration snippets with DocumentationSnippetTests 59/59 tests pass. https://claude.ai/code/session_01NNRvLbK1UWy49i4Yg43QEd --------- Co-authored-by: Claude <[email protected]>
* docs: reconcile v3-target design docs with shipped v3 API The v3-target docs were written as a pre-implementation proposal and described API that diverged from what shipped (IPasswordBuilder/Build(), Generate().Count(n), [Obsolete] sync methods, IRandomSource.Fill, net10.0, and a "Special" appSettings key). Update them to match the released v3.0.0 surface: IPassword as the fluent builder, static Password presets, parameterless Generate(), sync kept and not obsoleted, and the correct "SpecialCharacters" option key. Also drop the "not yet implemented" framing now that v3 has shipped. https://claude.ai/code/session_01FshNNNTHNHupkfMJVLmotu * ci: migrate from AppVeyor to GitHub Actions Add a combined CI workflow (build + test + pack on push to master and on every pull request, uploading test results and the .nupkg/.snupkg) and a Release workflow that publishes to NuGet.org when a GitHub Release is published. The release tag drives the published package version, and the .snupkg symbol package is pushed alongside the main package. Remove the AppVeyor configuration it replaces. Requires a NUGET_API_KEY repository secret for the publish step. https://claude.ai/code/session_01FshNNNTHNHupkfMJVLmotu --------- Co-authored-by: Claude <[email protected]>
* Add v3.0.0 local NuGet package test report Documents an end-to-end consumer test of the PasswordGenerator 3.0.0 package built from dev/v3: pack, local feed registration, install into a fresh console app, and 12 usage scenarios with the code added, the actual output, and whether each result was expected. https://claude.ai/code/session_01Hk3he4x9TKXfuopS3caYKE * Ignore PasswordGenerator/bin build output Broaden the bin ignore from bin/Debug to the whole bin folder so Release pack artifacts are not tracked. https://claude.ai/code/session_01Hk3he4x9TKXfuopS3caYKE --------- Co-authored-by: Claude <[email protected]>
* Add BenchmarkDotNet pipeline with GitHub Actions summary Expand the benchmark project into structured scenarios (single, batch, async, presets, DI-vs-direct, and a v3 batch-vs-loop comparison), all with memory diagnostics. Multi-target net8.0/net10.0 and emit GitHub markdown reports via the default exporter. Add a Benchmarks workflow that runs on push/dispatch, publishes the report tables to the job summary, and uploads the raw results. The planned v2-vs-v3 comparison via a second PasswordGenerator package reference is omitted: v2 and v3 both produce PasswordGenerator.dll and collide in one bin folder (extern alias fails with CS0430), so the comparison instead measures v3 usage patterns. https://claude.ai/code/session_01NZJPrT8QFtXsMG6JJ5AJfv * Target net10.0 and benchmark across both runtimes Add net10.0 to the package's target frameworks (the existing NET8_0_OR_GREATER conditionals cover it) and to the CI/release SDK setup so the new target builds and packs. Upgrade BenchmarkDotNet to 0.15.8 (0.14.0 has no .NET 10 moniker) and add .NET 8 and .NET 10 runtime jobs so every benchmark runs on both, with a Runtime column comparing them side by side in one report. https://claude.ai/code/session_01NZJPrT8QFtXsMG6JJ5AJfv * Add v3.0.0 benchmark results for .NET 8 and .NET 10 Record a sample BenchmarkDotNet run (both runtimes) under benchmarks/ as a readable historical reference. Reduced-sampling numbers; the workflow publishes full-precision results to the run summary. https://claude.ai/code/session_01NZJPrT8QFtXsMG6JJ5AJfv --------- Co-authored-by: Claude <[email protected]>
* Update non-shipped test/benchmark packages and SourceLink to latest Bump test tooling (NUnit, NUnit3TestAdapter, Test.Sdk) and the Microsoft.Extensions.* packages used only by the test and benchmark projects to their latest stable versions, plus SourceLink in the library (build-only, PrivateAssets=All). The shipped library's Microsoft.Extensions.* dependencies stay pinned at 8.0.0 on purpose: for a multi-target library those versions are a minimum floor imposed on consumers, so keeping the LTS floor maximizes compatibility across netstandard2.0/net8.0/net10.0. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL * Correct SourceLink.GitHub to latest stable 10.0.300 11.0.100 is preview-only and does not exist as a stable release, so restore failed. Pin to the actual latest stable, 10.0.300. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL * Drop netstandard2.0 from v3; make v3 docs the current docs Target net8.0;net10.0 only and remove the netstandard2.0 fallback in CryptoRandomSource (the #if rejection-sampling path, the RNG field, and IDisposable), which are no longer needed now that GetInt32 is available on every target. Restructure docs so v3 is the current state: flatten the former v3-target/* into docs/ (present tense, no "target/future" framing) and move the v2.1.0 snapshot, review/verification docs, and v3 planning docs into docs/archive/ with historical banners noting netstandard2.0 was dropped. Update Readme, CHANGELOG, migration guide, and cross-links. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL * Run tests against both net8.0 and net10.0 Multi-target the test project so the suite exercises the library on both shipped target frameworks rather than net8.0 only. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL --------- Co-authored-by: Claude <[email protected]>
* Ship XML documentation file in NuGet package Enable GenerateDocumentationFile so the heavily-documented public API provides IntelliSense to consumers of the package. https://claude.ai/code/session_01AikHWpKd7zQCfPDJcE2ZkX * Use ValueTask and cancelled-task semantics for async APIs Switch NextAsync/GenerateAsync to ValueTask<T> so the synchronous completion path allocates no Task, and surface cancellation as a cancelled task (ValueTask.FromCanceled) instead of throwing synchronously, so the result composes correctly when not awaited immediately. Argument validation still throws synchronously, matching BCL conventions. https://claude.ai/code/session_01AikHWpKd7zQCfPDJcE2ZkX * Fix benchmark build against ValueTask async APIs and silence CS1591 The async APIs now return ValueTask, so the AsyncBenchmarks methods must return ValueTask too (the Task return types no longer compile on either target framework). Add CS1591 to NoWarn so enabling the XML doc file does not warn on intentionally-undocumented public members. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL * Document the public API instead of suppressing CS1591 With the XML doc file enabled, add XML comments to every public type and member so the shipped documentation is complete and CS1591 no longer warns. Interface implementations use <inheritdoc/>; the NoWarn CS1591 suppression added earlier is removed. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL * Bump GitHub Actions to Node 24 runtimes The v4 majors of checkout, setup-dotnet, and upload-artifact run on the deprecated Node.js 20 runner. Bump to checkout@v6, setup-dotnet@v5, and upload-artifact@v7, which run on Node.js 24. Inputs used here are unchanged across these majors. https://claude.ai/code/session_01WVsnjDXjACmcLWJLGNMKVL --------- Co-authored-by: Claude <[email protected]>
* Use EFF Large Wordlist for passphrases Replace the 390-word built-in list with the EFF Large Wordlist (7,776 words, ~12.9 bits/word), raising a 6-word passphrase to ~77 bits. Add CC BY 3.0 attribution (WordList.cs header, THIRD-PARTY-NOTICES.md, Readme, CHANGELOG), an InternalsVisibleTo for the test project, and word-list integrity tests. Switch the passphrase smoke test to a '.' separator since a few EFF words are hyphenated. https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB * Add entropy-targeted passphrases and an entropy floor Add ForPassphraseWithEntropy(targetBits) which derives the word count needed to reach a target and enforces it, plus an optional minimumEntropyBits floor on ForPassphrase that rejects weak configs. Expose PassphraseGenerator.WordCountForEntropy and surface EstimateEntropyBits on the IPasswordGenerator interface. https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB * Add optional symbol injection for composition rules Add includeSymbol to the passphrase generator and factories: a random symbol is attached to one randomly chosen word so phrases satisfy "needs a number and a symbol" validators while staying memorable. Entropy estimation now accounts for the symbol's value and position. Document that some EFF words are hyphenated, so callers tokenizing the output should use a separator that does not appear in any word. https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB * Wire passphrases through DI and add ForMemorable preset Add PassphraseOptions and a PasswordOptions.Passphrase property so the DI registration can resolve a passphrase IPasswordGenerator, in code or bound from a "Passphrase" configuration section. Add a ForMemorable() preset (capitalized words sized to at least 80 bits). https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB * Add deterministic passphrase tests and update design docs Add seeded-RNG tests covering word selection by index, capitalization, symbol placement, plus a broad-sampling distribution check. Update api-surface.md and architecture.md to reflect the EFF Large Wordlist, entropy targeting/floor, symbol injection, EstimateEntropyBits on the interface, and passphrase configuration through DI. https://claude.ai/code/session_01Y5PCRCxt2rJFBLnbaQdQRB --------- Co-authored-by: Claude <[email protected]>
* Fix NuGet README rendering and harden package metadata - Use raw.githubusercontent.com URL for the logo so it resolves on NuGet.org (the /blob/ URL served HTML, not the image) and point it at master instead of the stale dev/v2 branch - Make the migration-guide link absolute so it works on the package page - Enable package validation and TreatWarningsAsErrors; add Company and tidy the Copyright string * Allow passphrases with no separator (char? separator) Make the passphrase separator a nullable char where null means no separator, so words are concatenated directly (e.g. mapleriverquartzbloom42). - PassphraseGenerator/PassphraseOptions/presets take char? separator - Generation appends the separator only when present (between words and before the trailing number) - DI: an explicitly empty Passphrase:Separator binds to null, since the config binder otherwise skips empty values and keeps the default - Entropy is unaffected (the separator never contributed to it) - Tests for no-separator output, capitalize, entropy parity and config binding; Readme and CHANGELOG updated * Tidy docs, XML exception tags, and rename Readme.md to README.md - README: fix duplicate variable in the presets example, absolutize all repo-relative links so they resolve on NuGet.org, give the async snippet an async context, and document CharacterClass values, DefaultBatchCount and an appSettings.json example - docs: note the optional/no separator and WordCountForEntropy in api-surface; add a passphrase appSettings example in configuration-and-di - Add <exception> tags to public throwing members that lacked them - Rename root Readme.md to README.md to match PackageReadmeFile and the package path (was only working via case-insensitive Windows)
* Fix NuGet README rendering and harden package metadata - Use raw.githubusercontent.com URL for the logo so it resolves on NuGet.org (the /blob/ URL served HTML, not the image) and point it at master instead of the stale dev/v2 branch - Make the migration-guide link absolute so it works on the package page - Enable package validation and TreatWarningsAsErrors; add Company and tidy the Copyright string * Allow passphrases with no separator (char? separator) Make the passphrase separator a nullable char where null means no separator, so words are concatenated directly (e.g. mapleriverquartzbloom42). - PassphraseGenerator/PassphraseOptions/presets take char? separator - Generation appends the separator only when present (between words and before the trailing number) - DI: an explicitly empty Passphrase:Separator binds to null, since the config binder otherwise skips empty values and keeps the default - Entropy is unaffected (the separator never contributed to it) - Tests for no-separator output, capitalize, entropy parity and config binding; Readme and CHANGELOG updated * Tidy docs, XML exception tags, and rename Readme.md to README.md - README: fix duplicate variable in the presets example, absolutize all repo-relative links so they resolve on NuGet.org, give the async snippet an async context, and document CharacterClass values, DefaultBatchCount and an appSettings.json example - docs: note the optional/no separator and WordCountForEntropy in api-surface; add a passphrase appSettings example in configuration-and-di - Add <exception> tags to public throwing members that lacked them - Rename root Readme.md to README.md to match PackageReadmeFile and the package path (was only working via case-insensitive Windows) * Refresh v3 docs, add doc navigation, and target benchmarks at .NET 10 - Remove archived planning/review docs and their references - Phrase the docs in present tense with v3 as the current release - Add prev/next/index navigation footers linking the docs in reading order - Drop .NET 8 from the benchmark project, job config, and CI workflow - Re-run the full benchmark suite on .NET 10 and refresh benchmarks/v3.0.0.md * Bump NuGet packages to latest versions - Microsoft.Extensions.DependencyInjection.Abstractions 8.0.0 → 10.0.8 - Microsoft.Extensions.Configuration.Binder 8.0.0 → 10.0.8 - Microsoft.NET.Test.Sdk 18.5.1 → 18.6.0 - Update v3-local-nuget-test.md consumer example versions to match No vulnerabilities found. All 91 tests pass on net8.0 and net10.0.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Release v3.0.0 - cryptographic rewrite with async, DI, presets and passphrases
BREAKING CHANGES
returning an error string. Use TryNext(out var password) for a
non-throwing alternative.
is dropped. Consumers on .NET Framework or older runtimes should stay
on the 2.x line.
SECURITY / CORRECTNESS
RandomNumberGenerator.GetInt32 — unbiased sampling, no modulo bias.
weakening output.
ADDED
configuration and appsettings.json binding via IConfiguration.
ForPassphrase, ForPassphraseWithEntropy, ForMemorable.
word); a 6-word phrase is ~77 bits. Optional symbol injection for
policies that require a special character. Optional null separator to
concatenate words without a delimiter.
PasswordOptions.DefaultBatchCount.
PACKAGING
.snupkg symbol package.
and Microsoft.Extensions.Configuration.Binder (both 10.0.8).
COMPATIBILITY
The v2 public surface (Next, NextGroup, constructors, IncludeX,
LengthRequired) is unchanged and continues to work, except for the
error-handling breaking change above. See docs/migration-v2-to-v3.md.