From 8a5fc868b9df465f6c08ad80a3860325996576a3 Mon Sep 17 00:00:00 2001 From: Stuart Meeks Date: Fri, 21 Aug 2026 04:40:49 +0000 Subject: [PATCH] chore: resolve open CodeQL code-quality alerts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Genuine fixes across the code-scanning backlog (no suppression hacks): - Path.Combine -> Path.Join repo-wide (src + tests). Path.Join never treats a later segment as rooted, so it can't silently drop earlier arguments — a defence-in-depth improvement for the input-validated credential/keystore path construction. Resolves #24 and clears the cs/path-combine alerts. - cs/linq/missed-where: replaced filter-style loops with idiomatic LINQ — providerName.Any(...) in the three ValidateProviderName checks, Distinct(OrdinalIgnoreCase) for the accounts-list column-key union, and .Where(...) / .FirstOrDefault(predicate) in the Keychain backend. - cs/catch-of-all-exceptions (crypto translation): narrowed DpapiCredentialEncryption and LocalFileCredentialEncryption from catch (Exception) to the specific expected types (CryptographicException, FormatException, IOException, UnauthorizedAccessException). Unexpected exceptions now propagate rather than being masked. The deliberately-broad boundary catches (CLI command handlers, best-effort cleanup) and the two generated-code alerts in the xUnit entry point are handled by dismissal with justification, out of band from this PR. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 17 +++++++ .../Commands/ListCredentialsCommand.cs | 20 +++----- .../Encryption/DpapiCredentialEncryption.cs | 8 +++- .../LocalFileCredentialEncryption.cs | 22 +++++++-- .../Persistence/FileCredentialManager.cs | 23 ++++------ .../Keychain/KeychainCredentialManager.cs | 46 ++++++++----------- .../Libsecret/LibsecretCredentialManager.cs | 11 ++--- .../Infrastructure/TempDir.cs | 2 +- .../Persistence/AtomicFileTests.cs | 16 +++---- .../Persistence/CredentialsDirectoryTests.cs | 12 ++--- .../Persistence/FileCredentialManagerTests.cs | 12 ++--- .../Persistence/SelectionsLockTests.cs | 6 +-- 12 files changed, 107 insertions(+), 88 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1198540..c2d72ee 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs b/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs index 05e6521..730caf0 100644 --- a/src/NextIteration.SpectreConsole.Auth/Commands/ListCredentialsCommand.cs +++ b/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(); - var seenKeys = new HashSet(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)}[/]"); diff --git a/src/NextIteration.SpectreConsole.Auth/Encryption/DpapiCredentialEncryption.cs b/src/NextIteration.SpectreConsole.Auth/Encryption/DpapiCredentialEncryption.cs index bd63df2..7cc6d35 100644 --- a/src/NextIteration.SpectreConsole.Auth/Encryption/DpapiCredentialEncryption.cs +++ b/src/NextIteration.SpectreConsole.Auth/Encryption/DpapiCredentialEncryption.cs @@ -40,7 +40,7 @@ public Task 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 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); } diff --git a/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs b/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs index 7106784..35968a3 100644 --- a/src/NextIteration.SpectreConsole.Auth/Encryption/LocalFileCredentialEncryption.cs +++ b/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 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); + } } /// @@ -191,10 +199,18 @@ public async Task 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 GetOrCreateKeyAsync() => _dataKey.Value; diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs index 57a7a3f..9f8d5d2 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/FileCredentialManager.cs +++ b/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 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 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 SelectCredentialAsync(string accountId) private async Task 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)); } } diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs index b2e3e7f..9d73e8c 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs +++ b/src/NextIteration.SpectreConsole.Auth/Persistence/Keychain/KeychainCredentialManager.cs @@ -209,16 +209,17 @@ public Task> ExportCredentialsAsync() var items = QueryAllItemsForApp(includeData: true); var selectionCache = new Dictionary(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(); - 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> 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)); } } diff --git a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs b/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs index 520f351..d4ab5f0 100644 --- a/src/NextIteration.SpectreConsole.Auth/Persistence/Libsecret/LibsecretCredentialManager.cs +++ b/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)); } } diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Infrastructure/TempDir.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Infrastructure/TempDir.cs index 2a9f24f..26e9b0a 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Infrastructure/TempDir.cs +++ b/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() { diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/AtomicFileTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/AtomicFileTests.cs index 9ff5fdd..47d2ede 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/AtomicFileTests.cs +++ b/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[] { diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/CredentialsDirectoryTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/CredentialsDirectoryTests.cs index c7d8285..79eb589 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/CredentialsDirectoryTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/CredentialsDirectoryTests.cs @@ -11,7 +11,7 @@ public sealed class CredentialsDirectoryTests public void Ensure_CreatesDirectory_WhenMissing() { using var temp = new TempDir(); - var target = Path.Combine(temp.Path, "creds"); + var target = Path.Join(temp.Path, "creds"); Assert.False(Directory.Exists(target)); CredentialsDirectory.Ensure(target); @@ -23,7 +23,7 @@ public void Ensure_CreatesDirectory_WhenMissing() public void Ensure_CreatesNestedDirectory_WhenParentMissing() { using var temp = new TempDir(); - var target = Path.Combine(temp.Path, "nested", "creds"); + var target = Path.Join(temp.Path, "nested", "creds"); Assert.False(Directory.Exists(target)); CredentialsDirectory.Ensure(target); @@ -35,12 +35,12 @@ public void Ensure_CreatesNestedDirectory_WhenParentMissing() public void Ensure_NoOp_WhenDirectoryAlreadyExists() { using var temp = new TempDir(); - var target = Path.Combine(temp.Path, "creds"); + var target = Path.Join(temp.Path, "creds"); Directory.CreateDirectory(target); // Touch a marker file inside so we can verify the directory isn't // recreated (which would wipe contents). - var marker = Path.Combine(target, "marker.txt"); + var marker = Path.Join(target, "marker.txt"); File.WriteAllText(marker, "hello"); CredentialsDirectory.Ensure(target); @@ -59,7 +59,7 @@ public void Ensure_SetsUnixMode0700_OnFirstCreation() } using var temp = new TempDir(); - var target = Path.Combine(temp.Path, "creds"); + var target = Path.Join(temp.Path, "creds"); CredentialsDirectory.Ensure(target); @@ -78,7 +78,7 @@ public void Ensure_DoesNotChange_ExistingUnixMode() } using var temp = new TempDir(); - var target = Path.Combine(temp.Path, "creds"); + var target = Path.Join(temp.Path, "creds"); Directory.CreateDirectory(target); // A deliberately-permissive mode that the library would never choose. diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs index 64f38a5..921bbec 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/FileCredentialManagerTests.cs @@ -56,7 +56,7 @@ public async Task AddCredentialAsync_CreatesFileAtExpectedPath() var accountId = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}"); - var expected = Path.Combine(temp.Path, $"adobe_{accountId}.json"); + var expected = Path.Join(temp.Path, $"adobe_{accountId}.json"); Assert.True(File.Exists(expected), $"expected credential file at {expected}"); } @@ -68,8 +68,8 @@ public async Task AddCredentialAsync_LowercasesProviderPrefixInFilename() var accountId = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}"); - var upperPath = Path.Combine(temp.Path, $"Adobe_{accountId}.json"); - var lowerPath = Path.Combine(temp.Path, $"adobe_{accountId}.json"); + var upperPath = Path.Join(temp.Path, $"Adobe_{accountId}.json"); + var lowerPath = Path.Join(temp.Path, $"adobe_{accountId}.json"); Assert.True(File.Exists(lowerPath)); // On case-insensitive filesystems this will also pass — we don't assert !File.Exists(upperPath). _ = upperPath; @@ -279,7 +279,7 @@ public async Task DeleteCredentialAsync_RemovesFile() using var temp = new TempDir(); var manager = CreateManager(temp.Path); var accountId = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}"); - var filePath = Path.Combine(temp.Path, $"adobe_{accountId}.json"); + var filePath = Path.Join(temp.Path, $"adobe_{accountId}.json"); Assert.True(File.Exists(filePath)); var deleted = await manager.DeleteCredentialAsync(accountId); @@ -379,7 +379,7 @@ public async Task AddCredentialAsync_SetsCredentialFileMode0600_OnUnix() var accountId = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}"); - var filePath = Path.Combine(temp.Path, $"adobe_{accountId}.json"); + var filePath = Path.Join(temp.Path, $"adobe_{accountId}.json"); var mode = File.GetUnixFileMode(filePath); Assert.Equal(UnixFileMode.UserRead | UnixFileMode.UserWrite, mode); } @@ -444,7 +444,7 @@ public async Task DeleteCredentialAsync_NonGuidId_ReturnsFalseWithoutThrowing(st var manager = CreateManager(temp.Path); _ = await manager.AddCredentialAsync("Adobe", "prod", "Production", "{}"); - // Malformed ids must not flow into Path.Combine / Directory.GetFiles + // Malformed ids must not flow into Path.Join / Directory.GetFiles // glob — they should resolve to a clean "not found". var result = await manager.DeleteCredentialAsync(accountId); diff --git a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/SelectionsLockTests.cs b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/SelectionsLockTests.cs index 0d1da75..a8b5262 100644 --- a/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/SelectionsLockTests.cs +++ b/tests/NextIteration.SpectreConsole.Auth.Tests/Persistence/SelectionsLockTests.cs @@ -11,7 +11,7 @@ public sealed class SelectionsLockTests public async Task Acquire_WhenUncontended_Succeeds() { using var temp = new TempDir(); - var lockPath = Path.Combine(temp.Path, "selections.json.lock"); + var lockPath = Path.Join(temp.Path, "selections.json.lock"); using var held = await SelectionsLock.AcquireAsync(lockPath, TestContext.Current.CancellationToken); @@ -24,7 +24,7 @@ public async Task Acquire_WhenUncontended_Succeeds() public async Task Acquire_WhenHeld_RetriesUntilReleased() { using var temp = new TempDir(); - var lockPath = Path.Combine(temp.Path, "selections.json.lock"); + var lockPath = Path.Join(temp.Path, "selections.json.lock"); var first = await SelectionsLock.AcquireAsync(lockPath, TestContext.Current.CancellationToken); @@ -48,7 +48,7 @@ public async Task Acquire_WhenHeld_RetriesUntilReleased() public async Task Acquire_AfterRelease_Succeeds() { using var temp = new TempDir(); - var lockPath = Path.Combine(temp.Path, "selections.json.lock"); + var lockPath = Path.Join(temp.Path, "selections.json.lock"); (await SelectionsLock.AcquireAsync(lockPath, TestContext.Current.CancellationToken)).Dispose();