From 1fcdfb3048a91ff06d9265f82170c216060f60ef Mon Sep 17 00:00:00 2001 From: Daniel Han Date: Wed, 29 Jul 2026 06:19:37 +0000 Subject: [PATCH] Open the bootstrap file in binary mode and finish the write Three defects in the in-place normalisation, all on the Windows upgrade path. os.open does not add O_BINARY on Windows and CPython never changes the CRT default of _O_TEXT, so the descriptor was in text mode: os.write turned the LF straight back into CRLF and ftruncate then cut the LF off, leaving "\r". That is the bug this PR exists to fix, reintroduced by the migration itself, and it is a fixed point that never converges. os.read translates in reverse too, so a genuinely CRLF file failed verification and was silently skipped. os.write may return having written fewer bytes than asked; ftruncate would then NUL-extend the credential so it no longer matched the hash in auth.db. os.fchmod only reached Windows in 3.13 and AttributeError is not OSError, so on 3.9 to 3.12 it escaped both handlers and aborted the first start after upgrade. --- studio/backend/auth/storage.py | 17 ++++++-- studio/backend/tests/test_desktop_auth.py | 50 +++++++++++++++++++++++ 2 files changed, 64 insertions(+), 3 deletions(-) diff --git a/studio/backend/auth/storage.py b/studio/backend/auth/storage.py index f05e71ac05..44df7c16f4 100644 --- a/studio/backend/auth/storage.py +++ b/studio/backend/auth/storage.py @@ -83,16 +83,27 @@ def _normalise_bootstrap_file(raw: bytes, password: str) -> None: if not raw.startswith(data[:-1]): return - fd = os.open(_BOOTSTRAP_PW_PATH, os.O_RDWR) + # O_BINARY or Windows opens the descriptor in text mode and os.write turns + # the LF straight back into CRLF, which is the bug this path exists to fix. + fd = os.open(_BOOTSTRAP_PW_PATH, os.O_RDWR | getattr(os, "O_BINARY", 0)) try: if os.read(fd, len(raw) + 1) != raw: return os.lseek(fd, 0, os.SEEK_SET) - os.write(fd, data) + # os.write may write fewer bytes than asked; truncating after a short + # write would NUL-fill the credential and lock the admin out. + written = 0 + while written < len(data): + n = os.write(fd, data[written:]) + if not n: + return + written += n os.ftruncate(fd, len(data)) try: os.fchmod(fd, 0o600) - except OSError: + except (AttributeError, OSError): + # fchmod only reached Windows in 3.13. Staying on the descriptor + # matters more than re-applying a mode the writer already set. pass finally: os.close(fd) diff --git a/studio/backend/tests/test_desktop_auth.py b/studio/backend/tests/test_desktop_auth.py index 72a2eeff3b..a4a5b280d0 100644 --- a/studio/backend/tests/test_desktop_auth.py +++ b/studio/backend/tests/test_desktop_auth.py @@ -257,6 +257,56 @@ def test_leading_whitespace_bootstrap_file_is_left_alone(monkeypatch): assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b" legacy-bootstrap-secret " +def test_normalising_opens_the_file_in_binary_mode(monkeypatch): + # Without O_BINARY, Windows opens the descriptor in text mode and os.write + # turns the LF back into CRLF, reintroducing the bug being fixed. + seed_user() + storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret") + monkeypatch.setattr(storage.os, "O_BINARY", 0x8000, raising = False) + seen = [] + real_open = storage.os.open + + def spy(path, flags, *args, **kwargs): + if str(path) == str(storage._BOOTSTRAP_PW_PATH): + seen.append(flags) + return real_open(path, flags & ~0x8000, *args, **kwargs) + + monkeypatch.setattr(storage.os, "open", spy) + + storage.ensure_default_admin() + + assert seen and all(f & 0x8000 for f in seen), seen + + +def test_normalising_survives_a_short_write(monkeypatch): + # A short write followed by ftruncate would NUL-fill the credential. + seed_user() + storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret") + real_write = storage.os.write + + def one_byte_at_a_time(fd, data): + return real_write(fd, data[:1]) + + monkeypatch.setattr(storage.os, "write", one_byte_at_a_time) + + storage.ensure_default_admin() + + assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"legacy-bootstrap-secret\n" + assert storage.get_bootstrap_password() == "legacy-bootstrap-secret" + + +def test_normalising_works_without_fchmod(monkeypatch): + # os.fchmod only reached Windows in 3.13; its absence must not raise. + seed_user() + storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret") + monkeypatch.delattr(storage.os, "fchmod", raising = False) + + storage.ensure_default_admin() + + assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"legacy-bootstrap-secret\n" + assert storage.get_bootstrap_password() == "legacy-bootstrap-secret" + + def test_persisting_the_bootstrap_password_is_atomic(monkeypatch, tmp_path): # A partial write would destroy the only plaintext recovery credential. storage._persist_bootstrap_password("original-secret")