From 2db1b9d9b4226c30da4b2437db421feb9235dbac Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 11 Nov 2026 22:24:37 +0000 Subject: [PATCH] Refuse a null non-nullable property in the in-memory store too [patch] InMemoryCredentialStore.Save and CredentialCache.AddOrReplace only checked for an unregistered type, so a credential such as `Token = null!` saved fine in memory and TryGet handed it back, while every platform store throws ArgumentException for it. Both now run the same serialization the platform stores do (zeroing the bytes) before storing anything. Fixes #208 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW4nQyg9qMFhK6fYVqQ --- CredentialCache.Test/NullPropertyTests.cs | 35 ++++++++++++++++--- CredentialCache/CredentialCache.cs | 12 ++++--- .../Storage/CredentialSerialization.cs | 15 ++++++++ .../Storage/InMemoryCredentialStore.cs | 2 +- 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/CredentialCache.Test/NullPropertyTests.cs b/CredentialCache.Test/NullPropertyTests.cs index ffbb2f7..7575449 100644 --- a/CredentialCache.Test/NullPropertyTests.cs +++ b/CredentialCache.Test/NullPropertyTests.cs @@ -65,10 +65,13 @@ public void SerializeRejectsANullNonNullableProperty() } [TestMethod] - public void AddOrReplaceRejectsANullNonNullablePropertyAndLeavesTheStoreAndCacheUnchanged() + [DataRow(false)] + [DataRow(true)] + public void AddOrReplaceRejectsANullNonNullablePropertyAndLeavesTheStoreAndCacheUnchanged(bool inMemory) { - RawBlobCredentialStore store = new(); - using CredentialCache cache = new(store); + RawBlobCredentialStore rawStore = new(); + InMemoryCredentialStore memoryStore = new(); + using CredentialCache cache = inMemory ? new(memoryStore) : new(rawStore); PersonaGUID persona = CredentialCache.CreatePersonaGUID(); CredentialWithUsernamePassword credential = new() { @@ -78,7 +81,31 @@ public void AddOrReplaceRejectsANullNonNullablePropertyAndLeavesTheStoreAndCache Assert.ThrowsExactly(() => cache.AddOrReplace(persona, credential)); - Assert.IsFalse(store.Blobs.ContainsKey(persona)); + Assert.IsFalse(rawStore.Blobs.ContainsKey(persona)); + Assert.IsFalse(memoryStore.TryLoad(persona, out _)); Assert.IsFalse(cache.TryGet(persona, out _)); } + + [TestMethod] + public void InMemoryStoreSaveRejectsANullNonNullablePropertyAsAPlatformStoreWould() + { + InMemoryCredentialStore store = new(); + PersonaGUID persona = CredentialCache.CreatePersonaGUID(); + + Assert.ThrowsExactly(() => store.Save(persona, new CredentialWithToken { Token = null! })); + + Assert.IsFalse(store.TryLoad(persona, out _)); + } + + [TestMethod] + public void InMemoryStoreStillSavesACompleteCredential() + { + using CredentialCache cache = new(new InMemoryCredentialStore()); + PersonaGUID persona = CredentialCache.CreatePersonaGUID(); + + cache.AddOrReplace(persona, new CredentialWithToken { Token = SemanticString.Create("abc") }); + + Assert.IsTrue(cache.TryGet(persona, out Credential? credential)); + Assert.AreEqual("abc", ((CredentialWithToken)credential!).Token.ToString()); + } } diff --git a/CredentialCache/CredentialCache.cs b/CredentialCache/CredentialCache.cs index 0d3a179..891d982 100644 --- a/CredentialCache/CredentialCache.cs +++ b/CredentialCache/CredentialCache.cs @@ -212,8 +212,9 @@ public bool TryGet(PersonaGUID persona, out Credential? credential) /// cache and the store are left unchanged. /// /// - /// The store serializes credentials and has a non-nullable - /// property set to null. The cache and the store are left unchanged. + /// has a non-nullable property set to null, so no + /// serializing store could read it back. The cache and the store are left unchanged, whichever + /// store is configured. /// public void AddOrReplace(PersonaGUID persona, Credential credential) { @@ -221,9 +222,10 @@ public void AddOrReplace(PersonaGUID persona, Credential credential) ArgumentNullException.ThrowIfNull(credential); ThrowIfDisposed(); - // Checked here rather than left to the store, so every store refuses the same types. The - // in-memory store never serializes, and it accepted types every platform store rejects. - CredentialSerialization.ThrowIfUnknownType(credential); + // Checked here rather than left to the store, so every store refuses the same credentials. + // The in-memory store never serializes, and it accepted unregistered types and null + // non-nullable properties that every platform store rejects. + CredentialSerialization.ThrowIfNotSerializable(credential); lock (LockFor(persona)) { diff --git a/CredentialCache/Storage/CredentialSerialization.cs b/CredentialCache/Storage/CredentialSerialization.cs index a72b3bc..12a1280 100644 --- a/CredentialCache/Storage/CredentialSerialization.cs +++ b/CredentialCache/Storage/CredentialSerialization.cs @@ -166,6 +166,21 @@ internal static void ThrowIfUnknownType(Credential credential) } } + /// + /// Throws what would throw for , + /// without keeping its serialized form, so a store that never serializes refuses the same + /// credentials as one that does. + /// + /// + /// The credential's type is neither one of the library's own nor registered with + /// . + /// + /// + /// has a non-nullable property set to null, so it could + /// not be read back. + /// + internal static void ThrowIfNotSerializable(Credential credential) => Zero(Serialize(credential)); + /// /// Serializes the credential to a UTF-8 JSON byte array. /// diff --git a/CredentialCache/Storage/InMemoryCredentialStore.cs b/CredentialCache/Storage/InMemoryCredentialStore.cs index 9d60696..6e36f2b 100644 --- a/CredentialCache/Storage/InMemoryCredentialStore.cs +++ b/CredentialCache/Storage/InMemoryCredentialStore.cs @@ -30,7 +30,7 @@ public void Save(PersonaGUID persona, Credential credential) // Refuse what a platform store could not serialize, so code tested against this store // sees the same failure it would in production. - CredentialSerialization.ThrowIfUnknownType(credential); + CredentialSerialization.ThrowIfNotSerializable(credential); _items[persona] = credential; }