diff --git a/src/bun.js/bindings/SecretsWindows.cpp b/src/bun.js/bindings/SecretsWindows.cpp index 0d699c7cee..8e1a839317 100644 --- a/src/bun.js/bindings/SecretsWindows.cpp +++ b/src/bun.js/bindings/SecretsWindows.cpp @@ -203,12 +203,62 @@ std::optional> getPassword(const CString& service, const CS return std::nullopt; } - // Convert credential blob to CString for thread safety + // Convert credential blob to UTF-8 vector for thread safety. + // Windows Credential Manager UI stores credentials as UTF-16LE, but Bun stores + // them as UTF-8. We detect encoding by first checking if the data is valid UTF-8, + // and only attempt UTF-16LE conversion if UTF-8 validation fails. std::optional> result; if (cred->CredentialBlob && cred->CredentialBlobSize > 0) { - result = WTF::Vector(std::span( - reinterpret_cast(cred->CredentialBlob), - cred->CredentialBlobSize)); + DWORD blobSize = cred->CredentialBlobSize; + BYTE* blob = cred->CredentialBlob; + + // First, check if the blob is valid UTF-8 by attempting to convert it. + // MB_ERR_INVALID_CHARS causes the function to fail on invalid UTF-8 sequences. + bool isValidUtf8 = false; + int wideLen = MultiByteToWideChar( + CP_UTF8, + MB_ERR_INVALID_CHARS, + reinterpret_cast(blob), + blobSize, + nullptr, + 0); + isValidUtf8 = (wideLen > 0); + + if (isValidUtf8) { + // Data is valid UTF-8, use it directly + result = WTF::Vector(std::span( + reinterpret_cast(blob), + blobSize)); + } else if (blobSize >= 2 && (blobSize % 2) == 0) { + // UTF-8 validation failed and blob size is even, try UTF-16LE conversion + int utf8Length = WideCharToMultiByte( + CP_UTF8, 0, + reinterpret_cast(blob), + blobSize / sizeof(wchar_t), + nullptr, 0, + nullptr, nullptr); + + if (utf8Length > 0) { + std::vector utf8Buffer(utf8Length); + int converted = WideCharToMultiByte( + CP_UTF8, 0, + reinterpret_cast(blob), + blobSize / sizeof(wchar_t), + utf8Buffer.data(), utf8Length, + nullptr, nullptr); + + if (converted > 0) { + result = WTF::Vector(std::span(utf8Buffer.data(), converted)); + } + } + } + + // Fallback: use raw bytes if all else fails + if (!result.has_value()) { + result = WTF::Vector(std::span( + reinterpret_cast(blob), + blobSize)); + } } framework->CredFree(cred); diff --git a/test/regression/issue/24135.test.ts b/test/regression/issue/24135.test.ts new file mode 100644 index 0000000000..4aa16fd314 --- /dev/null +++ b/test/regression/issue/24135.test.ts @@ -0,0 +1,102 @@ +import { describe, expect, test } from "bun:test"; +import { isWindows } from "harness"; + +// This test verifies the fix for issue #24135: +// Bun.secrets.get on Windows returns strings with null bytes when credentials +// are stored via Windows Credential Manager UI (which uses UTF-16LE encoding). + +describe.skipIf(!isWindows)("issue #24135", () => { + test("Bun.secrets.get should not return null bytes for ASCII passwords", async () => { + const testService = "bun-test-issue-24135-" + Date.now(); + const testUser = "test-name"; + const testPassword = "test123"; + + try { + // Set a credential via Bun (stores as UTF-8) + await Bun.secrets.set({ + service: testService, + name: testUser, + value: testPassword, + }); + + // Retrieve and verify no null bytes + const result = await Bun.secrets.get({ service: testService, name: testUser }); + expect(result).not.toBeNull(); + + // The key test: verify there are no null bytes in the result + const hasNullBytes = result!.includes("\0"); + expect(hasNullBytes).toBe(false); + + // Verify the actual value + expect(result).toBe(testPassword); + + // Verify char codes don't have nulls interleaved + const charCodes = Array.from(result!).map(c => c.charCodeAt(0)); + expect(charCodes).toEqual([116, 101, 115, 116, 49, 50, 51]); // "test123" + } finally { + // Clean up + await Bun.secrets.delete({ service: testService, name: testUser }); + } + }); + + test("Bun.secrets.get should correctly decode unicode passwords", async () => { + const testService = "bun-test-issue-24135-unicode-" + Date.now(); + const testUser = "test-name"; + const testPassword = "пароль密码🔐"; // Russian + Chinese + emoji + + try { + await Bun.secrets.set({ + service: testService, + name: testUser, + value: testPassword, + }); + + const result = await Bun.secrets.get({ service: testService, name: testUser }); + expect(result).toBe(testPassword); + + // Verify no unexpected null bytes (nulls should not appear in UTF-8 encoded text) + // Note: null bytes can legitimately appear in some encodings, but not in our test string + const unexpectedNulls = result!.includes("\0"); + expect(unexpectedNulls).toBe(false); + } finally { + await Bun.secrets.delete({ service: testService, name: testUser }); + } + }); + + // This test simulates what happens when a credential is stored via Windows Credential Manager UI + // by using cmdkey which also stores credentials in UTF-16LE format + test("Bun.secrets.get should handle credentials stored via cmdkey (UTF-16LE)", async () => { + const testService = "bun-test-issue-24135-cmdkey"; + const testUser = "cmdkey-test"; + const testPassword = "mypassword123"; + const targetName = `${testService}/${testUser}`; + + // Clean up any existing credential first + await Bun.$`cmdkey /delete:${targetName}`.quiet().nothrow(); + + try { + // Store credential using cmdkey (stores as UTF-16LE, same as Windows Credential Manager UI) + const addResult = await Bun.$`cmdkey /generic:${targetName} /user:${testUser} /pass:${testPassword}` + .quiet() + .nothrow(); + + if (addResult.exitCode !== 0) { + // cmdkey might not be available or may require elevated privileges + // Skip this test if we can't add the credential + console.log("Skipping cmdkey test - could not add credential"); + return; + } + + // Now read it back via Bun.secrets + const result = await Bun.secrets.get({ service: testService, name: testUser }); + + // The key assertion: the result should NOT have null bytes interleaved + expect(result).not.toBeNull(); + expect(result!.includes("\0")).toBe(false); + expect(result).toBe(testPassword); + } finally { + // Clean up using cmdkey + await Bun.$`cmdkey /delete:${targetName}`.quiet().nothrow(); + } + }); +});