Repository navigation
Refuse an unregistered Credential subclass in every store's Save, not only AddOrReplace [patch] - #207
Merged
matt-edmondson merged 1 commit intoNov 10, 2026
Conversation
… only AddOrReplace [patch] #194 added ThrowIfUnknownType, but only CredentialCache.AddOrReplace called it. A store used directly, through CredentialStoreFactory or CredentialCache.Instance.Store, skipped the check: each platform store's Save let System.Text.Json's raw NotSupportedException escape from Serialize, while InMemoryCredentialStore.Save accepted the credential. Tests against the in-memory store passed and production threw an exception type nothing around a store call expects. CredentialSerialization.Serialize and SerializeToString now call ThrowIfUnknownType first, so the Windows, macOS and Linux stores and any custom store built on those helpers refuse an unregistered subclass with the same CredentialStoreException and message as AddOrReplace. InMemoryCredentialStore.Save calls it too, and keeps nothing. Fixes #206 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW46SewAZ45qMGEBCjL
|
matt-edmondson
deleted the
fix/206-reject-unregistered-type-in-every-store
branch
November 10, 2026 23:52
matt-edmondson
pushed a commit
that referenced
this pull request
Nov 11, 2026
Resolve the conflict with #207 in Serialize and SerializeToString by keeping both checks: the unknown-type check runs first, then a JsonException from a null non-nullable property is reported as an ArgumentException. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017vfJW3kaZKRNvfNtvM4CX6
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
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.
Fixes #206
What was wrong
#194 added
CredentialSerialization.ThrowIfUnknownType, but onlyCredentialCache.AddOrReplacecalled it. A store used directly, throughCredentialStoreFactory.CreateDefault(...)orCredentialCache.Instance.Store, skipped the check:Savelet System.Text.Json's rawNotSupportedExceptionescape fromSerialize.InMemoryCredentialStore.Saveaccepted the credential.As a result, tests against the in-memory store passed while production threw an exception type that nothing around a store call expects.
What changed
CredentialSerialization.SerializeandSerializeToStringnow callThrowIfUnknownTypefirst. The Windows, macOS and Linux stores, and any custom store built on these helpers, now refuse an unregistered subclass with the sameCredentialStoreExceptionand message asAddOrReplace.InMemoryCredentialStore.Savecalls it too, and keeps nothing.AddOrReplaceparagraph.Tests
EveryStoreRefusesAnUnregisteredSubclassWithTheSameException:AddOrReplace,InMemoryCredentialStore.Saveand the default platform store'sSaveall throwCredentialStoreExceptionwith the same message, and the in-memory store keeps nothing. The test compares messages, which separates the type refusal from a keyring that isn't available.TheSerializationHelpersRefuseAnUnregisteredSubclass:SerializeandSerializeToStringthrowCredentialStoreExceptionnaming the type.Both tests fail on
main: the in-memorySavethrows nothing, andSerializethrowsNotSupportedException. Both pass with the fix. Full suite on Linux: 82 passed, plus 5 inconclusive that need libsecret or Windows, the same as onmain.PR #203 also edits
Serialize(relaxed escaping), so whichever of the two lands second may need a trivial rebase.Generated by Claude Code