From ecf9023f26c671cf322d2bc4d47f63bdf5206328 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 14:25:19 +0000 Subject: [PATCH 1/2] test: build request-outcome owners through their provider, against an in-memory store GitHubRequestOutcomeTests built owners with `new Owner { ... }`, so BuildProvider was null. Since tokens moved into the secret store (#285), reading or writing an owner's token derives its persona from BuildProvider.Name, so the four workflow action tests threw NullReferenceException on every platform. Build owners with provider.CreateOwner(...), and inject an in-memory credential store for every test so an authorization failure clearing the provider token never reaches the developer's real OS secret store. Fixes #300 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01T3Dc4pH8DW9RqEzV5znBXQ --- .../GitHubRequestOutcomeTests.cs | 45 +++++++++++++++---- 1 file changed, 37 insertions(+), 8 deletions(-) diff --git a/BuildMonitor.Test/GitHubRequestOutcomeTests.cs b/BuildMonitor.Test/GitHubRequestOutcomeTests.cs index 005944c..178ca2a 100644 --- a/BuildMonitor.Test/GitHubRequestOutcomeTests.cs +++ b/BuildMonitor.Test/GitHubRequestOutcomeTests.cs @@ -8,10 +8,14 @@ namespace ktsu.BuildMonitor.Test; using System.Net.Http; using System.Threading.Tasks; +using ktsu.CredentialCache.Storage; + using Microsoft.VisualStudio.TestTools.UnitTesting; using Octokit; +using CredentialCache = ktsu.CredentialCache.CredentialCache; + /// /// Tests whether one GitHub API request reports having succeeded. /// @@ -32,6 +36,26 @@ public sealed class GitHubRequestOutcomeTests { private const string RequestName = "test/request"; + private CredentialCache Cache { get; set; } = null!; + + /// + /// Tokens live in the OS secret store, and an authorization failure clears the provider's token, + /// so every test runs against an in-memory store rather than the developer's real credentials. + /// + [TestInitialize] + public void SetUp() + { + Cache = new CredentialCache(new InMemoryCredentialStore()); + TokenStorage.UseCache(Cache); + } + + [TestCleanup] + public void TearDown() + { + TokenStorage.UseCache(null); + Cache.Dispose(); + } + /// /// A response built to order. Octokit's own Response is internal, and the provider reads /// only the status code and the headers off it. @@ -127,11 +151,16 @@ public async Task AConnectionErrorReportsFailure() Assert.AreEqual(ProviderStatus.Error, provider.Status); } - private static Owner OwnerWithToken() => new() + /// + /// Builds an owner carrying its own token. The owner is made by the provider, because its token + /// is stored under a persona derived from the provider's name. + /// + private static Owner OwnerWithToken(GitHub provider) { - Name = OwnerName.Create("alpha"), - Token = BuildProviderToken.Create("alpha-pat"), - }; + Owner owner = provider.CreateOwner(OwnerName.Create("alpha")); + owner.Token = BuildProviderToken.Create("alpha-pat"); + return owner; + } /// /// The workflow actions themselves: a refused request must come back as a failure, not as the @@ -151,7 +180,7 @@ public async Task AWorkflowActionRefusedByTheApiReportsFailure() new Dictionary { ["X-RateLimit-Remaining"] = "0" }); bool succeeded = await provider.RunWorkflowActionAsync( - OwnerWithToken(), RequestName, () => Task.FromException(rateLimited)).ConfigureAwait(false); + OwnerWithToken(provider), RequestName, () => Task.FromException(rateLimited)).ConfigureAwait(false); Assert.IsFalse(succeeded, "A cancel or re-run the API refused must not be reported as having worked."); Assert.AreEqual(ProviderStatus.RateLimited, provider.Status); @@ -163,7 +192,7 @@ public async Task AWorkflowActionThatSucceedsReportsSuccess() GitHub provider = new(); bool succeeded = await provider.RunWorkflowActionAsync( - OwnerWithToken(), RequestName, () => Task.CompletedTask).ConfigureAwait(false); + OwnerWithToken(provider), RequestName, () => Task.CompletedTask).ConfigureAwait(false); Assert.IsTrue(succeeded); } @@ -178,7 +207,7 @@ public async Task AWorkflowActionOnAMissingRunReportsFailure() NotFoundException missing = new(new FakeResponse(HttpStatusCode.NotFound, new Dictionary())); bool succeeded = await provider.RunWorkflowActionAsync( - OwnerWithToken(), RequestName, () => Task.FromException(missing)).ConfigureAwait(false); + OwnerWithToken(provider), RequestName, () => Task.FromException(missing)).ConfigureAwait(false); Assert.IsFalse(succeeded); } @@ -190,7 +219,7 @@ public async Task AWorkflowActionWithoutCredentialsReportsFailureWithoutCalling( bool called = false; bool succeeded = await provider.RunWorkflowActionAsync( - new Owner { Name = OwnerName.Create("beta") }, + provider.CreateOwner(OwnerName.Create("beta")), RequestName, () => { From cf0ede3430f081e4241fbf7a5d607ea8f7063534 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 26 Sep 2026 14:27:14 +0000 Subject: [PATCH 2/2] fix: read tokens as empty when the secret store's type initializer failed [patch] On Linux the credential store first reaches libsecret from a static constructor, so a missing libsecret-1.so.0 arrives as a TypeInitializationException wrapping the DllNotFoundException. IsUnavailable only matched the bare exception types, so the wrapped form escaped every TokenStorage read and write, and the runtime rethrows it on each later access. Unwrap TypeInitializationException in IsUnavailable, and cover the wrapped form with a test double. Fixes #296 Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01T3Dc4pH8DW9RqEzV5znBXQ (cherry picked from commit 21376587f2ed773bb8dc0edd09eb8ed0423a2c1e) --- BuildMonitor.Test/TokenStorageTests.cs | 37 ++++++++++++++++++++++++++ BuildMonitor/TokenStorage.cs | 9 ++++++- 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/BuildMonitor.Test/TokenStorageTests.cs b/BuildMonitor.Test/TokenStorageTests.cs index 1bf342a..9398d69 100644 --- a/BuildMonitor.Test/TokenStorageTests.cs +++ b/BuildMonitor.Test/TokenStorageTests.cs @@ -78,6 +78,25 @@ public void Save(PersonaGUID persona, Credential credential) => public bool Remove(PersonaGUID persona) => throw new DllNotFoundException("libsecret-1.so.0"); } + /// + /// The same missing library as , but in the form it + /// actually takes on Linux: the store first reaches libsecret from a static constructor, so the + /// failure arrives wrapped in a . + /// + private sealed class TypeInitializerFailureCredentialStore : ICredentialStore + { + public string Name => "TypeInitializerFailure"; + + public bool TryLoad(PersonaGUID persona, out Credential? credential) => throw Failure(); + + public void Save(PersonaGUID persona, Credential credential) => throw Failure(); + + public bool Remove(PersonaGUID persona) => throw Failure(); + + private static TypeInitializationException Failure() => + new("ktsu.CredentialCache.Storage.LinuxSecretServiceCredentialStore+Schema", new DllNotFoundException("libsecret-1.so.0")); + } + /// /// Mirrors how writes the app data file: references preserved, /// and semantic strings round-tripped as plain strings rather than as char arrays. @@ -339,6 +358,24 @@ public void ReadingWithoutASecretStoreIsEmptyRatherThanFatal() Assert.IsFalse(TokenStorage.Write(provider.TokenPersona, "ghp_x".As())); } + /// + /// The Linux form of a missing secret library, wrapped in a type initializer failure, reads as + /// "no token" too, rather than escaping every token read. + /// + [TestMethod] + public void ReadingWhenTheSecretStoreTypeFailedToInitializeIsEmptyRatherThanFatal() + { + using CredentialCache unavailable = new(new TypeInitializerFailureCredentialStore()); + TokenStorage.UseCache(unavailable); + + TestProvider provider = NewProvider(); + Owner owner = AddOwner(provider, "ktsu-dev"); + + Assert.IsTrue(provider.ReadToken().IsEmpty()); + Assert.IsFalse(owner.HasToken); + Assert.IsFalse(TokenStorage.Write(provider.TokenPersona, "ghp_x".As())); + } + /// /// Two owners under one provider do not share a token. /// diff --git a/BuildMonitor/TokenStorage.cs b/BuildMonitor/TokenStorage.cs index f38363b..2b2ca9a 100644 --- a/BuildMonitor/TokenStorage.cs +++ b/BuildMonitor/TokenStorage.cs @@ -192,11 +192,18 @@ private static PersonaGUID DerivePersona(string seed) /// Recognises a machine with no usable secret store: the factory refusing the platform, the /// native library failing to resolve, or the store itself reporting a failure. /// + /// + /// A native library first reached from a static constructor surfaces as a + /// wrapping the real failure, and the runtime rethrows + /// that same exception on every later access. The Linux store reaches libsecret that way, so a + /// host without it would otherwise throw out of every token read instead of reading as empty. + /// private static bool IsUnavailable(Exception exception) => exception is PlatformNotSupportedException or DllNotFoundException or EntryPointNotFoundException - or CredentialStoreException; + or CredentialStoreException + || (exception is TypeInitializationException { InnerException: { } inner } && IsUnavailable(inner)); /// /// Logs the unavailable store once per process. Tokens are read on request paths that run every