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
"<secret>\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.
This commit is contained in:
Daniel Han 2026-07-29 06:19:37 +00:00
commit 1fcdfb3048
2 changed files with 64 additions and 3 deletions

View file

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

View file

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