Merge remote-tracking branch 'origin/main' into r7552

.github/workflows/studio-windows-inference-smoke.yml

@@ -1888,8 +1888,11 @@ jobs:
           # (step/substep -> Write-StudioStdoutMirror / Get-StudioAnsi).
           $script:StudioVtOk = $false
           $script:UnslothVerbose = $false
+          # Get-HostMachineArch is reached only on the absent path, where
+          # Test-VCRedistInstalled consults it before trusting the System32 DLL, so
+          # part A passes without it and only the clean-box part fails.
           foreach ($fn in @('Get-StudioAnsi', 'Write-StudioStdoutMirror', 'step', 'substep',
-                            'Invoke-SetupCommand', 'Refresh-Environment',
+                            'Invoke-SetupCommand', 'Refresh-Environment', 'Get-HostMachineArch',
                             'Test-VCRedistInstalled', 'Ensure-VCRedist')) {
               $src = Get-FunctionSource -Path $setup -Name $fn
               if (-not $src) { throw "Function '$fn' not found in setup.ps1" }

studio/backend/auth/storage.py

@@ -9,6 +9,7 @@ import ipaddress
 import os
 import secrets
 import sqlite3
+import tempfile
 import threading
 from datetime import datetime, timezone
 from typing import Optional, Tuple
@@ -30,6 +31,97 @@ _BOOTSTRAP_PW_PATH = DB_PATH.parent / ".bootstrap_password"
 _bootstrap_password: Optional[str] = None
 
 
+def _bootstrap_file_bytes(password: str) -> bytes:
+    """Exact on-disk form: the secret plus one LF.
+
+    Bytes, not text: text mode writes CRLF on Windows, and `$(cat ...)` strips
+    the LF but leaves the CR attached to the credential.
+    """
+    return (password + "\n").encode("utf-8")
+
+
+def _persist_bootstrap_password(password: str) -> None:
+    """Atomically write the bootstrap password 0600, LF terminated on every OS.
+
+    A partial write would destroy the only plaintext recovery credential.
+    """
+    fd, tmp_name = tempfile.mkstemp(
+        prefix = f".{_BOOTSTRAP_PW_PATH.name}.", dir = _BOOTSTRAP_PW_PATH.parent
+    )
+    try:
+        with os.fdopen(fd, "wb") as f:
+            f.write(_bootstrap_file_bytes(password))
+        try:
+            os.chmod(tmp_name, 0o600)
+        except OSError:
+            pass
+        os.replace(tmp_name, _BOOTSTRAP_PW_PATH)
+    except BaseException:
+        try:
+            os.unlink(tmp_name)
+        except OSError:
+            pass
+        raise
+
+
+def _normalise_bootstrap_file(raw: bytes, password: str) -> None:
+    """Append the LF a pre-newline release left off.
+
+    Append-only, and only when the file is exactly the credential:
+    clear_bootstrap_password() may unlink or (when unlink fails, notably on
+    Windows while this descriptor is open) truncate through another descriptor
+    after we read, so a rewrite could restore revoked plaintext. An append
+    cannot: worst case is a lone "\\n" over a cleared file, which strips back to
+    no bootstrap password. Pre-newline releases wrote no terminator at all, so
+    that is the only shape in the wild; anything else reads fine, since every
+    reader strips, and is left alone.
+    """
+    if raw != password.encode("utf-8"):
+        return
+
+    # O_BINARY: without it Windows opens in text mode and turns the LF straight
+    # back into CRLF, the bug being fixed.
+    fd = os.open(
+        _BOOTSTRAP_PW_PATH,
+        os.O_WRONLY | os.O_APPEND | getattr(os, "O_BINARY", 0),
+    )
+    try:
+        os.write(fd, b"\n")
+        try:
+            os.fchmod(fd, 0o600)
+        except (AttributeError, OSError):
+            # fchmod only reached Windows in 3.13.
+            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():
+        return None
+
+    # No caller handles a raise, so an unreadable file has to mean "no bootstrap
+    # password", not a dead backend. We write UTF-8, so undecodable bytes are
+    # damage whose plaintext is worthless anyway.
+    try:
+        raw = _BOOTSTRAP_PW_PATH.read_bytes()
+        password = raw.decode("utf-8").strip()
+    except (OSError, UnicodeDecodeError):
+        return None
+    if not password:
+        return None
+
+    # Older releases wrote no terminator; best-effort, a read-only auth dir must
+    # not fail startup.
+    if raw != _bootstrap_file_bytes(password):
+        try:
+            _normalise_bootstrap_file(raw, password)
+        except OSError:
+            pass
+    return password
+
+
 def generate_bootstrap_password() -> str:
     """Generate a 4-word diceware passphrase and persist it to disk.
 
@@ -43,10 +135,10 @@ def generate_bootstrap_password() -> str:
         return _bootstrap_password
 
     # Persisted from a previous run?
-    if _BOOTSTRAP_PW_PATH.is_file():
-        _bootstrap_password = _BOOTSTRAP_PW_PATH.read_text(encoding = "utf-8").strip()
-        if _bootstrap_password:
-            return _bootstrap_password
+    persisted = _read_persisted_bootstrap_password()
+    if persisted:
+        _bootstrap_password = persisted
+        return _bootstrap_password
 
     # First startup: generate a fresh passphrase.
     import diceware
@@ -57,11 +149,7 @@ def generate_bootstrap_password() -> str:
 
     # Persist so the same passphrase survives restarts until password change.
     ensure_dir(_BOOTSTRAP_PW_PATH.parent)
-    _BOOTSTRAP_PW_PATH.write_text(_bootstrap_password, encoding = "utf-8")
-    try:
-        os.chmod(_BOOTSTRAP_PW_PATH, 0o600)
-    except OSError:
-        pass
+    _persist_bootstrap_password(_bootstrap_password)
 
     return _bootstrap_password
 
@@ -72,19 +160,14 @@ def get_bootstrap_password() -> Optional[str]:
 
 
 def _load_bootstrap_password() -> Optional[str]:
-    """Load an existing bootstrap password without creating one."""
+    """Load an existing bootstrap password without creating one.
+
+    Upgrades take this path, not generate_bootstrap_password()
+    (ensure_default_admin short-circuits once the admin row exists), so it has
+    to normalise too.
+    """
     global _bootstrap_password
-    _bootstrap_password = None
-    if _BOOTSTRAP_PW_PATH.is_file():
-        # No caller handles a raise, so an unreadable file has to mean "no bootstrap
-        # password", not a dead backend. We write UTF-8, so bytes that will not
-        # decode are damage whose plaintext is worthless anyway.
-        try:
-            bootstrap_password = _BOOTSTRAP_PW_PATH.read_text(encoding = "utf-8").strip()
-        except (OSError, UnicodeDecodeError):
-            return _bootstrap_password
-        if bootstrap_password:
-            _bootstrap_password = bootstrap_password
+    _bootstrap_password = _read_persisted_bootstrap_password()
     return _bootstrap_password
 
 

studio/backend/tests/test_desktop_auth.py

@@ -134,6 +134,218 @@ def test_ensure_default_admin_loads_existing_bootstrap_after_restart(monkeypatch
     assert storage.get_bootstrap_password() == bootstrap_pw
 
 
+def test_bootstrap_password_file_ends_with_a_newline():
+    # Otherwise `cat` welds the passphrase onto the shell prompt.
+    storage.ensure_default_admin()
+
+    # Bytes: read_text would decode CRLF back to "\n" and hide a CR.
+    raw = storage._BOOTSTRAP_PW_PATH.read_bytes()
+
+    assert raw == storage.get_bootstrap_password().encode("utf-8") + b"\n"
+
+
+def test_bootstrap_password_round_trips_across_a_restart_with_the_newline():
+    storage.ensure_default_admin()
+    original = storage.get_bootstrap_password()
+
+    storage._bootstrap_password = None
+
+    assert storage.generate_bootstrap_password() == original
+
+
+def test_upgrade_normalises_the_bootstrap_file():
+    # Upgrade path: the admin row exists, so generate_bootstrap_password() never runs.
+    seed_user()
+    storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
+
+    storage.ensure_default_admin()
+
+    assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"legacy-bootstrap-secret\n"
+    assert storage.get_bootstrap_password() == "legacy-bootstrap-secret"
+
+
+@pytest.mark.parametrize(
+    "other",
+    [
+        b"legacy-bootstrap-secret\r\n",  # only an unreleased build wrote this
+        b"legacy-bootstrap-secret\r",
+        b"legacy-bootstrap-secret   ",
+    ],
+)
+def test_only_an_exactly_unterminated_bootstrap_file_is_touched(other):
+    # Appending is safe only because it is restricted to the one released shape.
+    seed_user()
+    storage._BOOTSTRAP_PW_PATH.write_bytes(other)
+
+    storage.ensure_default_admin()
+
+    assert storage.get_bootstrap_password() == "legacy-bootstrap-secret"
+    assert storage._BOOTSTRAP_PW_PATH.read_bytes() == other
+
+
+def test_upgrade_normalises_when_the_admin_row_is_missing():
+    storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
+
+    assert storage.generate_bootstrap_password() == "legacy-bootstrap-secret"
+    assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"legacy-bootstrap-secret\n"
+
+
+def test_a_well_formed_bootstrap_file_is_not_rewritten():
+    seed_user()
+    storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret\n")
+    mtime = storage._BOOTSTRAP_PW_PATH.stat().st_mtime_ns
+
+    storage.ensure_default_admin()
+
+    assert storage._BOOTSTRAP_PW_PATH.stat().st_mtime_ns == mtime
+
+
+def test_migration_failure_does_not_break_startup(monkeypatch):
+    seed_user()
+    storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
+
+    real_open = storage.os.open
+
+    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()
+
+    assert storage.get_bootstrap_password() == "legacy-bootstrap-secret"
+    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 revoked plaintext if the password changed after the read.
+    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()
+
+    # The append may add a second newline; the rotated credential must survive.
+    raw = storage._BOOTSTRAP_PW_PATH.read_bytes()
+    assert raw.strip() == b"brand-new-secret"
+    storage._bootstrap_password = None
+    assert storage._load_bootstrap_password() == "brand-new-secret"
+
+
+def test_leading_whitespace_bootstrap_file_is_left_alone(monkeypatch):
+    # An in-place rewrite is not atomic, so only the exact unterminated shape is touched.
+    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_normalising_opens_the_file_in_binary_mode(monkeypatch):
+    # Without O_BINARY, Windows text mode turns the written LF back into CRLF.
+    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_clearing_by_truncation_mid_normalisation_is_not_undone(monkeypatch):
+    # clear_bootstrap_password() truncates through its own descriptor when the unlink
+    # fails (Windows, while ours is open); the append must not restore the plaintext.
+    seed_user()
+    storage._BOOTSTRAP_PW_PATH.write_bytes(b"legacy-bootstrap-secret")
+
+    real_open = storage.os.open
+
+    def truncate_then_open(path, flags, *args, **kwargs):
+        fd = real_open(path, flags, *args, **kwargs)
+        if str(path) == str(storage._BOOTSTRAP_PW_PATH):
+            storage._BOOTSTRAP_PW_PATH.write_text("", encoding = "utf-8")
+        return fd
+
+    monkeypatch.setattr(storage.os, "open", truncate_then_open)
+
+    storage._read_persisted_bootstrap_password()
+
+    # A lone newline over a cleared file still reads back as no password.
+    assert storage._BOOTSTRAP_PW_PATH.read_bytes().strip() == b""
+    storage._bootstrap_password = None
+    assert storage._load_bootstrap_password() is None
+
+
+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")
+
+    def boom(src, dst):
+        raise OSError("crash before replace")
+
+    monkeypatch.setattr(storage.os, "replace", boom)
+    with pytest.raises(OSError):
+        storage._persist_bootstrap_password("new-secret")
+
+    assert storage._BOOTSTRAP_PW_PATH.read_bytes() == b"original-secret\n"
+    leftovers = [
+        p.name
+        for p in storage._BOOTSTRAP_PW_PATH.parent.iterdir()
+        if "bootstrap_password." in p.name
+    ]
+    assert leftovers == []
+
+
 def test_ensure_default_admin_does_not_generate_for_empty_existing_bootstrap():
     seed_user()
     storage._BOOTSTRAP_PW_PATH.write_text(" \n", encoding = "utf-8")
@@ -358,7 +570,7 @@ def test_write_desktop_secret_file_is_0600_on_unix(tmp_path):
 
     studio_cli._write_auth_secret(path, "desktop-secret")
 
-    assert path.read_text() == "desktop-secret"
+    assert path.read_bytes() == b"desktop-secret\n"
     if platform.system() != "Windows":
         assert oct(path.stat().st_mode & 0o777) == "0o600"
 
@@ -525,7 +737,8 @@ if result.exit_code != 0:
         capture_output = True,
     )
     assert result.returncode == 0, result.stderr + result.stdout
-    secret = (auth_dir / ".desktop_secret").read_text()
+    # Strip like the src-tauri readers do.
+    secret = (auth_dir / ".desktop_secret").read_text().strip()
     assert secret.startswith("desktop-")
 
     conn = sqlite3.connect(auth_dir / "auth.db")

unsloth_cli/commands/studio.py

@@ -483,9 +483,12 @@ def _write_auth_secret(path: Path, secret: str) -> None:
             os.chmod(tmp_path, 0o600)
         except OSError:
             pass
-        with os.fdopen(fd, "w", encoding = "utf-8") as f:
+        # newline pins LF: text mode writes CRLF on Windows, and `$(cat ...)`
+        # strips the LF but leaves the CR glued to the credential.
+        with os.fdopen(fd, "w", encoding = "utf-8", newline = "\n") as f:
             fd = -1
-            f.write(secret)
+            # Newline so `cat` doesn't run it into the shell prompt; readers strip.
+            f.write(secret + "\n")
         os.replace(tmp_path, path)
     except Exception:
         if fd >= 0:

unsloth_cli/tests/test_studio_password_prompt.py

@@ -251,7 +251,7 @@ def test_studio_default_prompt_rejects_current_password(monkeypatch, tmp_path):
     studio_mod = _studio()
     events = _install_prompt_env(monkeypatch, tmp_path, interactive = True)
     _seed_auth(studio_mod)
-    bootstrap_pw = (tmp_path / "auth" / studio_mod.BOOTSTRAP_PASSWORD_FILE).read_text()
+    bootstrap_pw = (tmp_path / "auth" / studio_mod.BOOTSTRAP_PASSWORD_FILE).read_text().strip()
 
     _invoke_studio_default(monkeypatch, events, ["--secure"])
 
@@ -1204,6 +1204,37 @@ def test_connect_auth_db_creates_private_files(monkeypatch, tmp_path):
     assert stat.S_IMODE((auth_dir / "auth.db").stat().st_mode) == 0o600
 
 
+def test_write_auth_secret_terminates_the_file_with_a_newline(monkeypatch, tmp_path):
+    # Shared by .bootstrap_password and .desktop_secret; every reader strips.
+    studio_mod = _studio()
+    path = tmp_path / ".desktop_secret"
+
+    studio_mod._write_auth_secret(path, "desktop-abc123")
+
+    # Bytes: read_text would decode CRLF back to "\n" and hide a CR.
+    assert path.read_bytes() == b"desktop-abc123\n"
+
+
+def test_seeded_bootstrap_file_ends_with_a_newline(monkeypatch, tmp_path):
+    studio_mod = _studio()
+    monkeypatch.setattr(studio_mod, "STUDIO_HOME", tmp_path)
+    _seed_auth(studio_mod)
+
+    raw = (tmp_path / "auth" / studio_mod.BOOTSTRAP_PASSWORD_FILE).read_bytes()
+
+    assert raw.endswith(b"\n") and not raw.endswith(b"\r\n")
+
+    conn = sqlite3.connect(_auth_db(tmp_path))
+    try:
+        salt, pwd_hash = conn.execute(
+            "SELECT password_salt, password_hash FROM auth_user WHERE username = ?",
+            (studio_mod.DEFAULT_ADMIN_USERNAME,),
+        ).fetchone()
+    finally:
+        conn.close()
+    assert studio_mod._pbkdf2_hex(raw.decode("utf-8").strip(), salt.encode("utf-8")) == pwd_hash
+
+
 # ── non-interactive --password / UNSLOTH_STUDIO_PASSWORD / stdin ──────
 
 
@@ -1284,7 +1315,7 @@ def test_studio_default_password_must_differ_fails_closed(monkeypatch, tmp_path)
     studio_mod = _studio()
     events = _install_prompt_env(monkeypatch, tmp_path, interactive = True)
     _seed_auth(studio_mod)
-    bootstrap_pw = (tmp_path / "auth" / studio_mod.BOOTSTRAP_PASSWORD_FILE).read_text()
+    bootstrap_pw = (tmp_path / "auth" / studio_mod.BOOTSTRAP_PASSWORD_FILE).read_text().strip()
 
     result = _invoke_studio_default(monkeypatch, events, ["--secure", "--password", bootstrap_pw])