Normalise the bootstrap file in place so a cleared credential stays cleared

The rename-based rewrite could recreate the file: if a password change ran
clear_bootstrap_password(), or the CLI cleanup deleted it, between the read and
the write, os.replace put the revoked plaintext back on disk, where a later
auth.db reset would re-seed it.

Open the existing file without O_CREAT instead, so a deleted file cannot be
resurrected, and re-check the contents through that descriptor so an in-place
truncation or a rotated credential is not overwritten either.

That gives up the atomic rename, so the in-place path is restricted to
trailing-whitespace fixes. Every partial state is then the secret plus leftover
whitespace, which still strips to the same credential. Files with leading
whitespace are left alone; every reader strips, so they keep working.

Creation still goes through the atomic writer.
This commit is contained in:
Daniel Han 2026-07-29 05:19:43 +00:00
commit 729cb51e44
2 changed files with 91 additions and 6 deletions

View file

@ -66,6 +66,38 @@ def _persist_bootstrap_password(password: str) -> None:
raise
def _normalise_bootstrap_file(raw: bytes, password: str) -> None:
"""Give an existing malformed file its trailing LF, in place.
Deliberately not the atomic replace above: a rename would recreate the file
if clear_bootstrap_password() or the CLI cleanup deleted it after we read
it, putting revoked plaintext back on disk for a later reset to re-seed.
Opening without O_CREAT cannot resurrect a deleted file, and the contents
are re-checked through the descriptor so an in-place truncation is not
undone either.
"""
data = _bootstrap_file_bytes(password)
# Only a trailing-whitespace rewrite is safe without a rename: every partial
# state is then "<secret>\n" plus leftover whitespace, which still strips to
# the same credential. Anything else keeps working unnormalised.
if not raw.startswith(data[:-1]):
return
fd = os.open(_BOOTSTRAP_PW_PATH, os.O_RDWR)
try:
if os.read(fd, len(raw) + 1) != raw:
return
os.lseek(fd, 0, os.SEEK_SET)
os.write(fd, data)
os.ftruncate(fd, len(data))
try:
os.fchmod(fd, 0o600)
except OSError:
pass
finally:
os.close(fd)
def _read_persisted_bootstrap_password() -> Optional[str]:
"""Read the persisted password, normalising the file if it is malformed."""
if not _BOOTSTRAP_PW_PATH.is_file():
@ -83,11 +115,11 @@ def _read_persisted_bootstrap_password() -> Optional[str]:
return None
# Older releases wrote no terminator, and text mode wrote CRLF on Windows,
# so upgrades kept the `cat` problem. Rewrite anything that isn't exactly
# "<secret>\n". Best-effort: a read-only auth dir must not fail startup.
# so upgrades kept the `cat` problem. Best-effort: a read-only auth dir or a
# credential cleared underneath us must not fail startup.
if raw != _bootstrap_file_bytes(password):
try:
_persist_bootstrap_password(password)
_normalise_bootstrap_file(raw, password)
except OSError:
pass
return password

View file

@ -193,10 +193,14 @@ def test_migration_failure_does_not_break_startup(monkeypatch):
seed_user()
storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
def refuse(*args, **kwargs):
raise PermissionError("read-only auth dir")
real_open = storage.os.open
monkeypatch.setattr(storage.tempfile, "mkstemp", refuse)
def refuse(path, flags, *args, **kwargs):
if str(path) == str(storage._BOOTSTRAP_PW_PATH):
raise PermissionError("read-only auth dir")
return real_open(path, flags, *args, **kwargs)
monkeypatch.setattr(storage.os, "open", refuse)
storage.ensure_default_admin()
@ -204,6 +208,55 @@ def test_migration_failure_does_not_break_startup(monkeypatch):
assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"legacy-bootstrap-secret"
def test_normalising_never_recreates_a_cleared_bootstrap_file(monkeypatch):
# A rename would resurrect the file if the password changed after the read,
# leaving revoked plaintext for a later auth.db reset to re-seed.
seed_user()
storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
real_open = storage.os.open
def clear_then_open(path, flags, *args, **kwargs):
if str(path) == str(storage._BOOTSTRAP_PW_PATH):
storage._BOOTSTRAP_PW_PATH.unlink(missing_ok = True)
return real_open(path, flags, *args, **kwargs)
monkeypatch.setattr(storage.os, "open", clear_then_open)
assert storage._read_persisted_bootstrap_password() == "legacy-bootstrap-secret"
assert not storage._BOOTSTRAP_PW_PATH.exists()
def test_normalising_does_not_overwrite_a_rotated_bootstrap_file(monkeypatch):
seed_user()
storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
real_open = storage.os.open
def rotate_then_open(path, flags, *args, **kwargs):
if str(path) == str(storage._BOOTSTRAP_PW_PATH):
storage._BOOTSTRAP_PW_PATH.write_bytes(b"brand-new-secret\n")
return real_open(path, flags, *args, **kwargs)
monkeypatch.setattr(storage.os, "open", rotate_then_open)
storage._read_persisted_bootstrap_password()
assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"brand-new-secret\n"
def test_leading_whitespace_bootstrap_file_is_left_alone(monkeypatch):
# Only trailing whitespace is rewritten in place: without a rename there is
# no atomicity, and a partial rewrite here could mis-strip.
seed_user()
storage._BOOTSTRAP_PW_PATH.write_bytes(b" legacy-bootstrap-secret ")
storage.ensure_default_admin()
assert storage.get_bootstrap_password() == "legacy-bootstrap-secret"
assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b" 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")