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