From 4baf6ad5cc73c90d099c3940814a62cba2309483 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Wed, 26 Aug 2026 12:15:47 +0800 Subject: [PATCH 1/2] fix: resolve the three open CodeQL alerts (#207, #208, #209) All three are in code this cycle added, not pre-existing. #208 cs/local-not-disposed (warning) -- AtomicFile.WriteTempAsync assigned the FileStream to a local and then opened `await using (stream...)`. It is disposed in practice, but the analyser cannot prove it for the window between construction and the using taking effect. Rewritten as an `await using` declaration, which is both provably correct and shorter. #207 cs/linq/missed-select (note) -- ResolveStoredProviderName ran a filter-then-project foreach. Expressed as Select + FirstOrDefault, which says what it does. #209 cs/inefficient-containskey (note) -- a test called ContainsKey and then the indexer, two lookups where TryGetValue is one, and the out value makes the assertion read better besides. No behaviour change; 416 tests unchanged. Co-Authored-By: Claude Opus 5 --- .../Persistence/AtomicFile.cs | 7 ++----- .../Libsecret/LibsecretCredentialManager.cs | 15 +++++---------- .../Persistence/FileCredentialManagerTests.cs | 4 ++-- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs index 5143015..375bf0c 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/AtomicFile.cs @@ -68,11 +68,8 @@ private static async Task WriteTempAsync(string tempPath, byte[] bytes, UnixFile options.UnixCreateMode = unixMode.Value; } - var stream = new FileStream(tempPath, options); - await using (stream.ConfigureAwait(false)) - { - await stream.WriteAsync(bytes, cancellationToken).ConfigureAwait(false); - } + await using var stream = new FileStream(tempPath, options); + await stream.WriteAsync(bytes, cancellationToken).ConfigureAwait(false); } /// diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs index 3f5944f..cd7d009 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs @@ -511,17 +511,12 @@ private string ResolveStoredProviderName(string providerName, CancellationToken }, loadSecrets: false, cancellationToken); - foreach (var item in items) - { - var stored = item.Attributes.GetValueOrDefault(AttrProvider); - if (!string.IsNullOrEmpty(stored) && + return items + .Select(i => i.Attributes.GetValueOrDefault(AttrProvider)) + .FirstOrDefault(stored => + !string.IsNullOrEmpty(stored) && stored.Equals(providerName, StringComparison.OrdinalIgnoreCase)) - { - return stored; - } - } - - return providerName; + ?? providerName; } /// diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs index 74730bd..847afb5 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs @@ -314,8 +314,8 @@ public async Task DeleteCredentialAsync_ClearsSelection_IfItWasSelected() Assert.False(selections.ContainsKey("Adobe")); // And the delete must not disturb an unrelated provider's selection. - Assert.True(selections.ContainsKey("Airtable")); - Assert.Equal(unrelated, selections["Airtable"]); + Assert.True(selections.TryGetValue("Airtable", out var untouched)); + Assert.Equal(unrelated, untouched); Assert.Null(await manager.GetSelectedCredentialAsync("Adobe")); } From 1c2deb3a18843a59f0f9bd3308d035903c385235 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Wed, 26 Aug 2026 12:18:17 +0800 Subject: [PATCH 2/2] docs: add the CHANGELOG entry for the CodeQL alert fixes The entry was missing from the previous commit: the scripted edit asserted on a unique '### Fixed' heading, which is no longer unique now that [Unreleased] has both Breaking and Fixed sections. The assertion fired, the CHANGELOG was left untouched, and the commit went ahead with only the source changes. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5fea08..b82616e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Three CodeQL alerts introduced by this cycle's changes**, all in code added since 1.1.0. + `AtomicFile.WriteTempAsync` held its `FileStream` in a local before an `await using` block, + which the analyser could not prove disposed on every path (`cs/local-not-disposed`, the only + warning of the three) — it is now an `await using` declaration. + `LibsecretCredentialManager.ResolveStoredProviderName` used a filter-then-project loop where + LINQ says it plainly (`cs/linq/missed-select`), and a test paired `ContainsKey` with the + indexer instead of `TryGetValue` (`cs/inefficient-containskey`). + - **The Keychain backend could see another CLI's credentials** (#55). App scoping was a dot-prefix match on the service string, and since the service is `{app}.{provider}` and provider names may contain dots, app `com.acme.cli` could not distinguish its own