From d5b17ecaefd4380945a7bfe560df7a6b987b5d0a Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 6 Nov 2026 05:26:22 +0000 Subject: [PATCH] Refuse a null non-nullable credential property when reading or saving CredentialSerialization's options didn't respect nullable annotations, so a stored "Token":null or "Password":null deserialized into a found credential whose property was null, and TryGet handed it to a caller whose first read threw NullReferenceException. Save also persisted a credential built with null!, so the bad entry round-tripped. Set RespectNullableAnnotations, so such an entry reads back as not found through the existing JsonException path, and have Serialize and SerializeToString report a credential that can't be read back as an ArgumentException. Fixes ktsu-dev/CredentialCache#180 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_017vfJW3kaZKRNvfNtvM4CX6 --- CredentialCache.Test/NullPropertyTests.cs | 84 +++++++++++++++++++ CredentialCache/CredentialCache.cs | 4 + .../Storage/CredentialSerialization.cs | 45 +++++++++- 3 files changed, 129 insertions(+), 4 deletions(-) create mode 100644 CredentialCache.Test/NullPropertyTests.cs diff --git a/CredentialCache.Test/NullPropertyTests.cs b/CredentialCache.Test/NullPropertyTests.cs new file mode 100644 index 0000000..ffbb2f7 --- /dev/null +++ b/CredentialCache.Test/NullPropertyTests.cs @@ -0,0 +1,84 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.CredentialCache.Test; + +using System.Text; +using ktsu.CredentialCache.Storage; +using ktsu.Semantics.Strings; + +/// +/// Tests that a credential whose non-nullable property is null is neither read back as found +/// nor written to a store, so no caller is handed a credential that throws on first use. +/// +[TestClass] +public class NullPropertyTests +{ + private const string NullToken = "{\"$type\":\"CredentialWithToken\",\"Token\":null}"; + private const string NullUsername = "{\"$type\":\"CredentialWithUsernamePassword\",\"Username\":null,\"Password\":\"hunter2\"}"; + private const string NullPassword = "{\"$type\":\"CredentialWithUsernamePassword\",\"Username\":\"alice\",\"Password\":null}"; + + [TestMethod] + [DataRow(NullToken)] + [DataRow(NullUsername)] + [DataRow(NullPassword)] + public void DeserializeReturnsNullForANullNonNullableProperty(string json) => + Assert.IsNull(CredentialSerialization.Deserialize(Encoding.UTF8.GetBytes(json))); + + [TestMethod] + [DataRow(NullToken)] + [DataRow(NullUsername)] + [DataRow(NullPassword)] + public void DeserializeFromStringReturnsNullForANullNonNullableProperty(string json) => + Assert.IsNull(CredentialSerialization.DeserializeFromString(json)); + + [TestMethod] + [DataRow("{\"$type\":\"CredentialWithToken\",\"Token\":\"abc\"}")] + [DataRow("{\"$type\":\"CredentialWithUsernamePassword\",\"Username\":\"alice\",\"Password\":\"hunter2\"}")] + [DataRow("{\"$type\":\"CredentialWithNothing\"}")] + public void DeserializeStillReadsCompleteCredentials(string json) => + Assert.IsNotNull(CredentialSerialization.Deserialize(Encoding.UTF8.GetBytes(json))); + + [TestMethod] + [DataRow(NullToken)] + [DataRow(NullUsername)] + [DataRow(NullPassword)] + public void TryGetReturnsFalseForAStoredEntryWithANullNonNullableProperty(string json) + { + RawBlobCredentialStore store = new(); + using CredentialCache cache = new(store); + PersonaGUID persona = CredentialCache.CreatePersonaGUID(); + store.Blobs[persona] = Encoding.UTF8.GetBytes(json); + + bool found = cache.TryGet(persona, out Credential? credential); + + Assert.IsFalse(found); + Assert.IsNull(credential); + } + + [TestMethod] + public void SerializeRejectsANullNonNullableProperty() + { + CredentialWithToken credential = new() { Token = null! }; + + Assert.ThrowsExactly(() => CredentialSerialization.Serialize(credential)); + Assert.ThrowsExactly(() => CredentialSerialization.SerializeToString(credential)); + } + + [TestMethod] + public void AddOrReplaceRejectsANullNonNullablePropertyAndLeavesTheStoreAndCacheUnchanged() + { + RawBlobCredentialStore store = new(); + using CredentialCache cache = new(store); + PersonaGUID persona = CredentialCache.CreatePersonaGUID(); + CredentialWithUsernamePassword credential = new() + { + Username = SemanticString.Create("alice"), + Password = null!, + }; + + Assert.ThrowsExactly(() => cache.AddOrReplace(persona, credential)); + + Assert.IsFalse(store.Blobs.ContainsKey(persona)); + Assert.IsFalse(cache.TryGet(persona, out _)); + } +} diff --git a/CredentialCache/CredentialCache.cs b/CredentialCache/CredentialCache.cs index b5ffca9..854c7d4 100644 --- a/CredentialCache/CredentialCache.cs +++ b/CredentialCache/CredentialCache.cs @@ -197,6 +197,10 @@ public bool TryGet(PersonaGUID persona, out Credential? credential) /// registered with . The /// 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. + /// public void AddOrReplace(PersonaGUID persona, Credential credential) { ArgumentNullException.ThrowIfNull(persona); diff --git a/CredentialCache/Storage/CredentialSerialization.cs b/CredentialCache/Storage/CredentialSerialization.cs index 0faf895..8ae38ec 100644 --- a/CredentialCache/Storage/CredentialSerialization.cs +++ b/CredentialCache/Storage/CredentialSerialization.cs @@ -66,6 +66,10 @@ private static JsonSerializerOptions BuildOptions(Dictionary deriv // Persuade System.Text.Json to use the SemanticString factory methods (Create / FromString) // rather than treating SemanticString as an IEnumerable collection. built.Converters.Add(new RoundTripStringJsonConverterFactory()); + // Refuse a JSON null for a non-nullable property, in both directions. Otherwise a stored + // "Token":null reads back as a found credential whose Token throws on first use, and a + // credential with a null! property is persisted and round-trips. + built.RespectNullableAnnotations = true; return built; } @@ -159,14 +163,47 @@ internal static void ThrowIfUnknownType(Credential credential) /// /// Serializes the credential to a UTF-8 JSON byte array. /// - public static byte[] Serialize(Credential credential) => - JsonSerializer.SerializeToUtf8Bytes(credential, options); + /// + /// has a non-nullable property set to null, so it could + /// not be read back. + /// + public static byte[] Serialize(Credential credential) + { + ArgumentNullException.ThrowIfNull(credential); + + try + { + return JsonSerializer.SerializeToUtf8Bytes(credential, options); + } + catch (JsonException exception) + { + throw NotSerializable(credential, exception); + } + } /// /// Serializes the credential to a UTF-8 JSON string. /// - public static string SerializeToString(Credential credential) => - JsonSerializer.Serialize(credential, options); + /// + /// has a non-nullable property set to null, so it could + /// not be read back. + /// + public static string SerializeToString(Credential credential) + { + ArgumentNullException.ThrowIfNull(credential); + + try + { + return JsonSerializer.Serialize(credential, options); + } + catch (JsonException exception) + { + throw NotSerializable(credential, exception); + } + } + + private static ArgumentException NotSerializable(Credential credential, JsonException exception) => + new($"{credential.GetType()} cannot be stored: {exception.Message}", nameof(credential), exception); /// /// Deserializes a credential from a UTF-8 JSON byte array. Returns null if the bytes