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 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")); }