From e3065a178555f28f8149affa26946d26aaf5fe0b Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 16:24:41 +0000 Subject: [PATCH 1/2] Throw CredentialStoreException, not TypeInitializationException, when libsecret is missing [patch] The Linux store built its schema handle in a static initializer, so a missing libsecret-1.so.0 reached every call as a TypeInitializationException that callers following the documented "detect and fall back" advice don't catch. Build the schema lazily under a lock and translate DllNotFoundException and EntryPointNotFoundException into CredentialStoreException, and name that type in the class remarks. Fixes ktsu-dev/CredentialCache#168 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W83XxSnDX5Fu4sVPDKjXem --- CredentialCache.Test/LibsecretMissingTests.cs | 105 ++++++++++++++++++ .../LinuxSecretServiceCredentialStore.cs | 60 ++++++++-- 2 files changed, 153 insertions(+), 12 deletions(-) create mode 100644 CredentialCache.Test/LibsecretMissingTests.cs diff --git a/CredentialCache.Test/LibsecretMissingTests.cs b/CredentialCache.Test/LibsecretMissingTests.cs new file mode 100644 index 0000000..91a6cef --- /dev/null +++ b/CredentialCache.Test/LibsecretMissingTests.cs @@ -0,0 +1,105 @@ +// Copyright (c) 2023-2026 ktsu-dev contributors + +namespace ktsu.CredentialCache.Test; + +using System.Runtime.InteropServices; +using System.Runtime.Versioning; +using ktsu.CredentialCache.Storage; + +/// +/// Covers the Linux store on a host without libsecret (a headless server, SSH session or +/// container). Callers are told to catch and fall back, +/// so that is what every operation must throw, never a . +/// +[TestClass] +public class LibsecretMissingTests +{ + [TestMethod] + public void TranslateMissingLibraryWrapsDllNotFoundException() + { + if (!OperatingSystem.IsLinux()) + { + Assert.Inconclusive("The libsecret store is only built on Linux."); + return; + } + + AssertTranslated(new DllNotFoundException("Unable to load shared library 'libsecret-1.so.0'")); + } + + [TestMethod] + public void TranslateMissingLibraryWrapsEntryPointNotFoundException() + { + if (!OperatingSystem.IsLinux()) + { + Assert.Inconclusive("The libsecret store is only built on Linux."); + return; + } + + AssertTranslated(new EntryPointNotFoundException("secret_schema_new")); + } + + [TestMethod] + public void TranslateMissingLibraryLeavesOtherFailuresAlone() + { + if (!OperatingSystem.IsLinux()) + { + Assert.Inconclusive("The libsecret store is only built on Linux."); + return; + } + + AssertNotTranslated(); + } + + [TestMethod] + public void EveryOperationThrowsCredentialStoreExceptionWhenLibsecretIsMissing() + { + if (!OperatingSystem.IsLinux()) + { + Assert.Inconclusive("The libsecret store is only built on Linux."); + return; + } + + if (NativeLibrary.TryLoad("libsecret-1.so.0", out IntPtr handle)) + { + NativeLibrary.Free(handle); + Assert.Inconclusive("libsecret is installed here, so the missing-library path cannot be reached."); + return; + } + + AssertEveryOperationThrows(); + } + + [SupportedOSPlatform("linux")] + private static void AssertTranslated(Exception nativeFailure) + { + CredentialStoreException exception = Assert.ThrowsExactly( + () => LinuxSecretServiceCredentialStore.TranslateMissingLibrary(() => throw nativeFailure)); + + Assert.AreSame(nativeFailure, exception.InnerException); + StringAssert.Contains(exception.Message, "libsecret"); + } + + [SupportedOSPlatform("linux")] + private static void AssertNotTranslated() + { + Assert.AreEqual(42, LinuxSecretServiceCredentialStore.TranslateMissingLibrary(() => 42)); + Assert.ThrowsExactly( + () => LinuxSecretServiceCredentialStore.TranslateMissingLibrary(() => throw new InvalidOperationException())); + } + + [SupportedOSPlatform("linux")] + private static void AssertEveryOperationThrows() + { + LinuxSecretServiceCredentialStore store = new($"ktsu.CredentialCache.MissingLibsecretTest.{Guid.NewGuid():N}"); + PersonaGUID persona = CredentialCache.CreatePersonaGUID(); + + // Twice each: a type initializer would fail once and then rethrow its cached + // TypeInitializationException on every later call. + for (int attempt = 0; attempt < 2; attempt++) + { + Assert.ThrowsExactly(() => store.TryLoad(persona, out _)); + Assert.ThrowsExactly(() => store.Remove(persona)); + Assert.ThrowsExactly(() => store.Save(persona, new CredentialWithNothing())); + } + } +} diff --git a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs index e4e7079..71da1d5 100644 --- a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs +++ b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs @@ -15,8 +15,10 @@ namespace ktsu.CredentialCache.Storage; /// /// Requires libsecret to be installed on the host. On headless systems without /// a running secret-service implementation (e.g. minimal containers, build agents) -/// this provider will fail at the first operation; consumers should detect this -/// and fall back to if appropriate. +/// this provider throws from every operation; +/// consumers should catch it and fall back to if +/// appropriate. A missing libsecret-1.so.0 surfaces the same way, with the +/// as its inner exception. /// /// /// Plaintext credential bytes are scrubbed on the same terms as the Windows and macOS @@ -50,7 +52,7 @@ public bool TryLoad(PersonaGUID persona, out Credential? credential) IntPtr error = IntPtr.Zero; IntPtr passwordPtr = NativeMethods.secret_password_lookup_sync( - Schema.Handle, + GetSchemaHandle(), IntPtr.Zero, ref error, "service", _serviceName, @@ -94,7 +96,7 @@ public void Save(PersonaGUID persona, Credential credential) IntPtr error = IntPtr.Zero; bool stored = NativeMethods.secret_password_store_sync( - Schema.Handle, + GetSchemaHandle(), IntPtr.Zero, label, value.Pointer, @@ -119,7 +121,7 @@ public bool Remove(PersonaGUID persona) IntPtr error = IntPtr.Zero; bool removed = NativeMethods.secret_password_clear_sync( - Schema.Handle, + GetSchemaHandle(), IntPtr.Zero, ref error, "service", _serviceName, @@ -153,14 +155,48 @@ internal static void ThrowIfError(IntPtr error, string operation) throw new CredentialStoreException($"{operation} failed: {message ?? ""}"); } - private static class Schema + private static readonly Lock SchemaLock = new(); + private static IntPtr schemaHandle; + + /// + /// Builds the schema on first use rather than in a type initializer: a load failure + /// there would reach callers as a on every + /// call, which nobody would think to catch. + /// + private static IntPtr GetSchemaHandle() { - internal static readonly IntPtr Handle = NativeMethods.secret_schema_new( - "dev.ktsu.CredentialCache", - flags: 0, - "service", 0, - "account", 0, - IntPtr.Zero); + lock (SchemaLock) + { + if (schemaHandle == IntPtr.Zero) + { + schemaHandle = TranslateMissingLibrary(() => NativeMethods.secret_schema_new( + "dev.ktsu.CredentialCache", + flags: 0, + "service", 0, + "account", 0, + IntPtr.Zero)); + } + + return schemaHandle; + } + } + + /// + /// Runs a native call, turning a missing or incompatible libsecret into the store's + /// documented . + /// + internal static T TranslateMissingLibrary(Func nativeCall) + { + try + { + return nativeCall(); + } + catch (Exception ex) when (ex is DllNotFoundException or EntryPointNotFoundException) + { + throw new CredentialStoreException( + $"libsecret-1.so.0 could not be loaded. Install libsecret-1-0 and a Secret Service implementation, or use {nameof(InMemoryCredentialStore)}.", + ex); + } } private static class NativeMethods From 72b2574af4932ce84ff9d37445cbdf8c5a07fb4e Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 6 Oct 2026 16:33:17 +0000 Subject: [PATCH 2/2] Move schema caching into a testable NativeHandleCache The cache-hit path only ran on hosts with libsecret, which the coverage host lacks. A small cache type lets a test cover both the cached handle and the retry after a failed creation without loading libsecret. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01W83XxSnDX5Fu4sVPDKjXem --- CredentialCache.Test/LibsecretMissingTests.cs | 41 ++++++++++++++++++ .../LinuxSecretServiceCredentialStore.cs | 43 +++++++++++++------ 2 files changed, 71 insertions(+), 13 deletions(-) diff --git a/CredentialCache.Test/LibsecretMissingTests.cs b/CredentialCache.Test/LibsecretMissingTests.cs index 91a6cef..550d8d4 100644 --- a/CredentialCache.Test/LibsecretMissingTests.cs +++ b/CredentialCache.Test/LibsecretMissingTests.cs @@ -50,6 +50,18 @@ public void TranslateMissingLibraryLeavesOtherFailuresAlone() AssertNotTranslated(); } + [TestMethod] + public void SchemaCacheCreatesTheHandleOnceAndRetriesAfterAFailure() + { + if (!OperatingSystem.IsLinux()) + { + Assert.Inconclusive("The libsecret store is only built on Linux."); + return; + } + + AssertSchemaCacheBehaviour(); + } + [TestMethod] public void EveryOperationThrowsCredentialStoreExceptionWhenLibsecretIsMissing() { @@ -87,6 +99,35 @@ private static void AssertNotTranslated() () => LinuxSecretServiceCredentialStore.TranslateMissingLibrary(() => throw new InvalidOperationException())); } + [SupportedOSPlatform("linux")] + private static void AssertSchemaCacheBehaviour() + { + LinuxSecretServiceCredentialStore.NativeHandleCache cache = new(); + int calls = 0; + + // A failure is not cached: the next call tries again rather than rethrowing. + Assert.ThrowsExactly(() => cache.GetOrCreate(() => + { + calls++; + throw new CredentialStoreException("libsecret-1.so.0 could not be loaded."); + })); + + IntPtr created = cache.GetOrCreate(() => + { + calls++; + return new IntPtr(42); + }); + IntPtr cached = cache.GetOrCreate(() => + { + calls++; + return new IntPtr(7); + }); + + Assert.AreEqual(new IntPtr(42), created); + Assert.AreEqual(created, cached); + Assert.AreEqual(2, calls); + } + [SupportedOSPlatform("linux")] private static void AssertEveryOperationThrows() { diff --git a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs index 71da1d5..df8b557 100644 --- a/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs +++ b/CredentialCache/Storage/LinuxSecretServiceCredentialStore.cs @@ -155,29 +155,46 @@ internal static void ThrowIfError(IntPtr error, string operation) throw new CredentialStoreException($"{operation} failed: {message ?? ""}"); } - private static readonly Lock SchemaLock = new(); - private static IntPtr schemaHandle; + private static readonly NativeHandleCache Schema = new(); /// /// Builds the schema on first use rather than in a type initializer: a load failure /// there would reach callers as a on every /// call, which nobody would think to catch. /// - private static IntPtr GetSchemaHandle() + private static IntPtr GetSchemaHandle() => + Schema.GetOrCreate(() => TranslateMissingLibrary(() => NativeMethods.secret_schema_new( + "dev.ktsu.CredentialCache", + flags: 0, + "service", 0, + "account", 0, + IntPtr.Zero))); + + /// + /// Holds a native handle created on first use. A failed creation is not cached, so every + /// later call fails the same way as the first instead of rethrowing a stale exception. + /// + internal sealed class NativeHandleCache { - lock (SchemaLock) + private readonly Lock _lock = new(); + private IntPtr _handle; + + /// + /// Returns the cached handle, creating it with if there is none. + /// + /// Creates the handle. + /// The handle. + internal IntPtr GetOrCreate(Func create) { - if (schemaHandle == IntPtr.Zero) + lock (_lock) { - schemaHandle = TranslateMissingLibrary(() => NativeMethods.secret_schema_new( - "dev.ktsu.CredentialCache", - flags: 0, - "service", 0, - "account", 0, - IntPtr.Zero)); - } + if (_handle == IntPtr.Zero) + { + _handle = create(); + } - return schemaHandle; + return _handle; + } } }