chore: resolve open CodeQL code-quality alerts

CHANGELOG.md

@@ -38,6 +38,23 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ### Changed
 
+- **Cleared the open CodeQL code-quality alerts.** All genuine fixes, no suppression
+  hacks:
+  - Repo-wide `Path.Combine` → `Path.Join` in `src` and `tests` (resolves #24). `Path.Join`
+    never treats a later segment as rooted, so it can't silently drop earlier arguments —
+    defence-in-depth for the credential/keystore path construction, which is already
+    input-validated.
+  - Idiomatic LINQ in place of filter-style loops: `providerName.Any(...)` for the
+    `ValidateProviderName` checks, `Distinct(OrdinalIgnoreCase)` for the `accounts list`
+    column-key union, and `.Where(...)` / `.FirstOrDefault(predicate)` in the Keychain
+    backend.
+  - Narrowed the exception handling in `DpapiCredentialEncryption` and
+    `LocalFileCredentialEncryption` from `catch (Exception)` to the specific types actually
+    expected (`CryptographicException`, `FormatException`, `IOException`,
+    `UnauthorizedAccessException`); a truly unexpected exception now propagates instead of
+    being masked as a generic encrypt/decrypt failure. The deliberately-broad boundary
+    catches (CLI command handlers, best-effort cleanup) are unchanged by design.
+
 - **`TODO.md` retired; its backlog moved to GitHub issues.** Completed items shipped in
   this cycle (the flaky secret-store delete test, keystore format versioning, and
   zero-on-dispose — see above). Remaining items that need an environment or a

src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs

@@ -80,19 +80,13 @@ private async Task DisplayCredentialsForProvider(string provider)
 
             // Union of display-field keys across this provider's credentials,
             // preserving first-seen order so columns render in the order the
-            // summary provider intended.
-            var displayFieldKeys = new List<string>();
-            var seenKeys = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
-            foreach (var credential in credentials)
-            {
-                foreach (var kvp in credential.DisplayFields)
-                {
-                    if (seenKeys.Add(kvp.Key))
-                    {
-                        displayFieldKeys.Add(kvp.Key);
-                    }
-                }
-            }
+            // summary provider intended. Distinct() yields first occurrences in
+            // source order, which is exactly the ordered de-duplication wanted.
+            var displayFieldKeys = credentials
+                .SelectMany(c => c.DisplayFields)
+                .Select(kvp => kvp.Key)
+                .Distinct(StringComparer.OrdinalIgnoreCase)
+                .ToList();
 
             AnsiConsole.MarkupLine($"[bold]{Markup.Escape(provider)}[/]");
 

src/NextIteration.SpectreConsole.Auth/Encryption/DpapiCredentialEncryption.cs

@@ -40,7 +40,7 @@ public Task<string> EncryptAsync(string plainText)
 
                 return Task.FromResult(Convert.ToBase64String(encryptedBytes));
             }
-            catch (Exception ex)
+            catch (CryptographicException ex)
             {
                 throw new InvalidOperationException("Failed to encrypt credential data", ex);
             }
@@ -62,7 +62,11 @@ public Task<string> DecryptAsync(string encryptedText)
 
                 return Task.FromResult(Encoding.UTF8.GetString(decryptedBytes));
             }
-            catch (Exception ex)
+            catch (FormatException ex)
+            {
+                throw new InvalidOperationException("Encrypted credential data is not valid base64.", ex);
+            }
+            catch (CryptographicException ex)
             {
                 throw new InvalidOperationException("Failed to decrypt credential data", ex);
             }

src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs

@@ -115,7 +115,7 @@ public LocalFileCredentialEncryption(string credentialsDirectory, byte[]? additi
         {
             ArgumentException.ThrowIfNullOrWhiteSpace(credentialsDirectory);
 
-            _keyFile = Path.Combine(credentialsDirectory, ".keystore");
+            _keyFile = Path.Join(credentialsDirectory, ".keystore");
 
             // PBKDF2 salt — non-secret, stable per machine/user. Caller
             // entropy is mixed into the password side instead of the salt
@@ -150,10 +150,18 @@ public async Task<string> EncryptAsync(string plainText)
                 // Mirrors the same passthrough in DecryptAsync.
                 throw;
             }
-            catch (Exception ex)
+            catch (CryptographicException ex)
             {
                 throw new InvalidOperationException("Failed to encrypt credential data.", ex);
             }
+            catch (IOException ex)
+            {
+                throw new InvalidOperationException("Failed to read the keystore while encrypting credential data.", ex);
+            }
+            catch (UnauthorizedAccessException ex)
+            {
+                throw new InvalidOperationException("Failed to read the keystore while encrypting credential data.", ex);
+            }
         }
 
         /// <inheritdoc />
@@ -191,10 +199,18 @@ public async Task<string> DecryptAsync(string encryptedText)
             {
                 throw;
             }
-            catch (Exception ex)
+            catch (CryptographicException ex)
             {
                 throw new InvalidOperationException("Failed to decrypt credential data.", ex);
             }
+            catch (IOException ex)
+            {
+                throw new InvalidOperationException("Failed to read the keystore while decrypting credential data.", ex);
+            }
+            catch (UnauthorizedAccessException ex)
+            {
+                throw new InvalidOperationException("Failed to read the keystore while decrypting credential data.", ex);
+            }
         }
 
         private Task<byte[]> GetOrCreateKeyAsync() => _dataKey.Value;

src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs

@@ -41,8 +41,8 @@ public FileCredentialManager(
 
             _encryption = encryption;
             _credentialsDirectory = credentialsDirectory;
-            _selectionFile = Path.Combine(_credentialsDirectory, "selections.json");
-            _selectionLockFile = Path.Combine(_credentialsDirectory, "selections.json.lock");
+            _selectionFile = Path.Join(_credentialsDirectory, "selections.json");
+            _selectionLockFile = Path.Join(_credentialsDirectory, "selections.json.lock");
             _summaryProviders = (summaryProviders ?? [])
                 .ToDictionary(p => p.ProviderName, StringComparer.OrdinalIgnoreCase);
 
@@ -128,7 +128,7 @@ public async Task<string> AddCredentialAsync(string providerName, string account
             };
 
             var fileName = $"{providerName.ToLowerInvariant()}_{accountId}.json";
-            var filePath = Path.Combine(_credentialsDirectory, fileName);
+            var filePath = Path.Join(_credentialsDirectory, fileName);
 
             var json = JsonSerializer.Serialize(credential, _jsonOptions);
 
@@ -200,7 +200,7 @@ public async Task<bool> SelectCredentialAsync(string accountId)
             {
                 // A non-GUID id can only land here via tampered selections.json;
                 // treat it as "no selection" rather than letting a malformed
-                // string flow into Path.Combine.
+                // string flow into Path.Join.
                 return null;
             }
 
@@ -228,7 +228,7 @@ public async Task<bool> SelectCredentialAsync(string accountId)
         private async Task<string?> ReadAndDecryptByIdAsync(string providerName, string accountId)
         {
             var fileName = $"{providerName.ToLowerInvariant()}_{accountId}.json";
-            var filePath = Path.Combine(_credentialsDirectory, fileName);
+            var filePath = Path.Join(_credentialsDirectory, fileName);
             if (!File.Exists(filePath))
             {
                 return null;
@@ -379,7 +379,7 @@ public async Task RestoreCredentialAsync(CredentialExport credential)
             };
 
             var fileName = $"{credential.ProviderName.ToLowerInvariant()}_{credential.AccountId}.json";
-            var filePath = Path.Combine(_credentialsDirectory, fileName);
+            var filePath = Path.Join(_credentialsDirectory, fileName);
 
             var json = JsonSerializer.Serialize(stored, _jsonOptions);
 
@@ -409,14 +409,11 @@ private static void ValidateProviderName(string providerName)
         {
             ArgumentException.ThrowIfNullOrWhiteSpace(providerName);
 
-            foreach (var c in providerName)
+            if (providerName.Any(c => !char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '-'))
             {
-                if (!char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '-')
-                {
-                    throw new ArgumentException(
-                        $"Provider name '{providerName}' contains invalid characters. Allowed: ASCII letters, digits, '.', '_', '-'.",
-                        nameof(providerName));
-                }
+                throw new ArgumentException(
+                    $"Provider name '{providerName}' contains invalid characters. Allowed: ASCII letters, digits, '.', '_', '-'.",
+                    nameof(providerName));
             }
         }
 

src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs

@@ -209,16 +209,17 @@ public Task<IReadOnlyList<CredentialExport>> ExportCredentialsAsync()
         var items = QueryAllItemsForApp(includeData: true);
         var selectionCache = new Dictionary<string, string?>(StringComparer.OrdinalIgnoreCase);
 
+        // Only real credential items are exported: skip the selection records,
+        // and any item whose service/account doesn't resolve to a credential.
+        var credentialItems = items.Where(i =>
+            !i.Service.EndsWith(SelectionsServiceSuffix, StringComparison.Ordinal)
+            && i.Account is not null
+            && ProviderNameFromService(i.Service) is not null);
+
         var exports = new List<CredentialExport>();
-        foreach (var item in items)
+        foreach (var item in credentialItems)
         {
-            // Skip the selection records; only real credential items are exported.
-            if (item.Service.EndsWith(SelectionsServiceSuffix, StringComparison.Ordinal))
-                continue;
-
-            var providerName = ProviderNameFromService(item.Service);
-            if (providerName is null || item.Account is null)
-                continue;
+            var providerName = ProviderNameFromService(item.Service)!;
 
             if (!selectionCache.TryGetValue(providerName, out var selectedId))
             {
@@ -228,7 +229,7 @@ public Task<IReadOnlyList<CredentialExport>> ExportCredentialsAsync()
 
             exports.Add(new CredentialExport
             {
-                AccountId = item.Account,
+                AccountId = item.Account!, // non-null: guaranteed by the Where filter above
                 AccountName = item.Label ?? string.Empty,
                 ProviderName = providerName,
                 Environment = item.Description ?? string.Empty,
@@ -331,16 +332,12 @@ private void DeleteSelection(string providerName)
         // Enumerate all app-owned items; pick the one whose account matches.
         // Keychain doesn't index on account alone across services, so this
         // is a linear scan — acceptable because credential counts are tiny.
-        var items = QueryAllItemsForApp(includeData: false);
-        foreach (var item in items)
-        {
-            if (string.Equals(item.Account, accountId, StringComparison.OrdinalIgnoreCase)
-                && !item.Service.EndsWith(SelectionsServiceSuffix, StringComparison.Ordinal))
-            {
-                return (item.Service, item.Account);
-            }
-        }
-        return null;
+        var match = QueryAllItemsForApp(includeData: false)
+            .FirstOrDefault(item =>
+                string.Equals(item.Account, accountId, StringComparison.OrdinalIgnoreCase)
+                && !item.Service.EndsWith(SelectionsServiceSuffix, StringComparison.Ordinal));
+
+        return match is null ? null : (match.Service, match.Account);
     }
 
     // =========================
@@ -351,14 +348,11 @@ private void DeleteSelection(string providerName)
     private static void ValidateProviderName(string providerName)
     {
         ArgumentException.ThrowIfNullOrWhiteSpace(providerName);
-        foreach (var c in providerName)
+        if (providerName.Any(c => !char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '-'))
         {
-            if (!char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '-')
-            {
-                throw new ArgumentException(
-                    $"Provider name '{providerName}' contains invalid characters. Allowed: ASCII letters, digits, '.', '_', '-'.",
-                    nameof(providerName));
-            }
+            throw new ArgumentException(
+                $"Provider name '{providerName}' contains invalid characters. Allowed: ASCII letters, digits, '.', '_', '-'.",
+                nameof(providerName));
         }
     }
 

src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs

@@ -377,14 +377,11 @@ private static DateTime ParseCreatedAt(string? value)
     private static void ValidateProviderName(string providerName)
     {
         ArgumentException.ThrowIfNullOrWhiteSpace(providerName);
-        foreach (var c in providerName)
+        if (providerName.Any(c => !char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '-'))
         {
-            if (!char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '-')
-            {
-                throw new ArgumentException(
-                    $"Provider name '{providerName}' contains invalid characters. Allowed: ASCII letters, digits, '.', '_', '-'.",
-                    nameof(providerName));
-            }
+            throw new ArgumentException(
+                $"Provider name '{providerName}' contains invalid characters. Allowed: ASCII letters, digits, '.', '_', '-'.",
+                nameof(providerName));
         }
     }
 

tests/NextIteration.SpectreConsole.Auth.Tests/Infrastructure/TempDir.cs

@@ -9,7 +9,7 @@ namespace NextIteration.SpectreConsole.Auth.Tests.Infrastructure;
 internal sealed class TempDir : IDisposable
 {
     public string Path { get; } =
-        System.IO.Path.Combine(System.IO.Path.GetTempPath(), "ni.sca.tests." + Guid.NewGuid().ToString("N"));
+        System.IO.Path.Join(System.IO.Path.GetTempPath(), "ni.sca.tests." + Guid.NewGuid().ToString("N"));
 
     public TempDir()
     {

tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/AtomicFileTests.cs

@@ -11,7 +11,7 @@ public sealed class AtomicFileTests
     public async Task WriteAllTextAsync_WritesExpectedContent()
     {
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
 
         await AtomicFile.WriteAllTextAsync(target, "hello");
 
@@ -22,7 +22,7 @@ public async Task WriteAllTextAsync_WritesExpectedContent()
     public async Task WriteAllBytesAsync_WritesExpectedBytes()
     {
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.bin");
+        var target = Path.Join(temp.Path, "file.bin");
         var payload = new byte[] { 0x00, 0x01, 0x02, 0xFE, 0xFF };
 
         await AtomicFile.WriteAllBytesAsync(target, payload);
@@ -34,7 +34,7 @@ public async Task WriteAllBytesAsync_WritesExpectedBytes()
     public async Task WriteAllTextAsync_NoTempFileLeftBehindAfterSuccess()
     {
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
 
         await AtomicFile.WriteAllTextAsync(target, "hello");
 
@@ -48,7 +48,7 @@ public async Task WriteAllTextAsync_NoTempFileLeftBehindAfterSuccess()
     public async Task WriteAllTextAsync_OverwritesExisting()
     {
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
         await File.WriteAllTextAsync(target, "original", TestContext.Current.CancellationToken);
 
         await AtomicFile.WriteAllTextAsync(target, "replaced");
@@ -63,7 +63,7 @@ public async Task WriteAllTextAsync_DoesNotExposeIntermediateState()
         // a concurrent observer should see either the old content or the new
         // content — never an empty/half-written target.
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
         await File.WriteAllTextAsync(target, "original", TestContext.Current.CancellationToken);
 
         // Hard to probe the race deterministically, so at minimum assert
@@ -82,7 +82,7 @@ public async Task WriteAllTextAsync_SetsUnixMode_OnUnix()
         }
 
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
 
         await AtomicFile.WriteAllTextAsync(
             target,
@@ -97,7 +97,7 @@ await AtomicFile.WriteAllTextAsync(
     public async Task WriteAllTextAsync_NullUnixMode_DoesNotThrow()
     {
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
 
         await AtomicFile.WriteAllTextAsync(target, "hello", unixMode: null);
 
@@ -112,7 +112,7 @@ public async Task WriteAllTextAsync_UsesUniqueTempName_SafeForConcurrentWriters(
         // last-rename-wins semantic means both succeed; only one final
         // content persists.
         using var temp = new TempDir();
-        var target = Path.Combine(temp.Path, "file.txt");
+        var target = Path.Join(temp.Path, "file.txt");
 
         var tasks = new[]
         {