fix(backup): never restore the auth policy settings from a backup (#2656)

_collect_settings exports every Settings row minus two credential keys, so
auth_enabled / advanced_auth_enabled / local_login_enabled / setup_completed
all travel in a backup, and none of them are credential-shaped enough for
_SECRET_KEY_HINTS to catch. Writing them back was the one part of a settings
restore that changed who can reach the instance rather than how it behaves:

* auth_enabled=false — from any backup taken before auth was turned on —
  disabled authentication. core.auth caches only the enabled=True result, on
  a 30 s TTL, precisely so staleness fails closed; set_auth_enabled pairs its
  write with invalidate_auth_enabled_cache(). The restore did neither, so it
  left the stored value the open one.
* local_login_enabled=false walked straight past the #1589 refusals in
  update_settings (no enabled OIDC provider / no OIDC link on the caller),
  which exist to stop exactly that lockout.
* /github-backup/restore is gated on GITHUB_RESTORE alone, so honouring these
  keys made that permission a way to rewrite auth config without
  SETTINGS_UPDATE.

Flipping an existing row needed overwrite_existing, so the odds were lower
than the severity. Both refusals now share _is_skipped_setting_key so the
preview's item count still matches what a restore writes, and the skipped
keys get their own note pointing at the auth UI rather than being folded in
with the credential ones.

backend/app/services/github_restore.py

@@ -70,6 +70,32 @@ _SENSITIVE_SETTING_KEYS = {"bambu_cloud_token", "auth_secret_key"}
 # skipped even if it isn't in the explicit denylist above.
 _SECRET_KEY_HINTS = ("token", "secret", "password", "access_code", "api_key", "passphrase")
 
+# Keys that decide *who can reach the instance* rather than how it behaves. The
+# backup collector writes them like any other Settings row, so a backup taken
+# before auth was turned on carries auth_enabled=false — and a restore reaches
+# the table directly, so honouring them would:
+#
+#   * disable authentication outright. ``set_auth_enabled`` pairs its write with
+#     ``invalidate_auth_enabled_cache()``; we cannot, so the 30 s TTL in
+#     core.auth is the only thing between the write and an open instance. That
+#     cache is built to fail closed — writing the stored value behind its back
+#     is what would make it fail open.
+#   * bypass the lockout refusals ``update_settings`` enforces (a
+#     ``local_login_enabled=false`` with no enabled OIDC provider, or with no
+#     OIDC link on the caller, is a 400 there — #1589).
+#   * cross a permission boundary: /github-backup/restore is gated on
+#     GITHUB_RESTORE alone, so this would be a way to rewrite auth config
+#     without SETTINGS_UPDATE.
+#
+# Auth is reconfigured through the auth UI, which has the guards. Restoring it
+# from a snapshot has no safe reading.
+_PROTECTED_SETTING_KEYS = {
+    "auth_enabled",
+    "advanced_auth_enabled",
+    "local_login_enabled",
+    "setup_completed",
+}
+
 # Nozzle diameters the backup collector iterates. A path outside this set means
 # the backup was written by a newer version, so accept it rather than dropping
 # data, but keep the list for validation messages.
@@ -91,6 +117,15 @@ def _is_blocked_setting_key(key: str) -> bool:
     return key in _SENSITIVE_SETTING_KEYS or any(hint in lowered for hint in _SECRET_KEY_HINTS)
 
 
+def _is_protected_setting_key(key: str) -> bool:
+    return key in _PROTECTED_SETTING_KEYS
+
+
+def _is_skipped_setting_key(key: str) -> bool:
+    """True for any key ``_restore_settings`` refuses to write, for either reason."""
+    return _is_blocked_setting_key(key) or _is_protected_setting_key(key)
+
+
 class _CategoryTally:
     """Mutable accumulator matching ``GitHubRestoreCategoryResult``."""
 
@@ -276,9 +311,14 @@ class GitHubRestoreService:
             values = payload.get("settings") if isinstance(payload, dict) else None
             if not isinstance(values, dict):
                 return 0, "No settings in payload"
+            # Both refusals are counted the same way here so the preview's item
+            # count matches what the restore actually writes; the wording only
+            # calls out the credential ones, which are what a user might expect
+            # to come back.
             blocked = sum(1 for key in values if _is_blocked_setting_key(key))
+            skipped = sum(1 for key in values if _is_skipped_setting_key(key))
             detail = f"{blocked} credential-like keys will be skipped" if blocked else None
-            return len(values) - blocked, detail
+            return len(values) - skipped, detail
 
         if category == RestoreCategory.SPOOLS:
             payload = parsed.get(SPOOLS_PATH)
@@ -825,6 +865,7 @@ class GitHubRestoreService:
             return
 
         blocked = 0
+        protected = 0
         for key, value in values.items():
             if not isinstance(key, str) or not key:
                 tally.failed += 1
@@ -833,6 +874,10 @@ class GitHubRestoreService:
                 blocked += 1
                 tally.skipped += 1
                 continue
+            if _is_protected_setting_key(key):
+                protected += 1
+                tally.skipped += 1
+                continue
             if value is None:
                 tally.skipped += 1
                 continue
@@ -852,6 +897,11 @@ class GitHubRestoreService:
 
         if blocked:
             tally.note(f"{blocked} credential-like key(s) skipped — re-enter secrets manually")
+        if protected:
+            tally.note(
+                f"{protected} authentication setting(s) skipped — change those in Settings > "
+                "Authentication so the lockout checks still run"
+            )
 
     async def _restore_kprofiles(self, db: AsyncSession, payload: dict, tally: _CategoryTally) -> None:
         by_serial: dict[str, list[tuple[str, dict]]] = {}

backend/tests/unit/test_github_restore.py

@@ -26,6 +26,8 @@ from backend.app.services.github_restore import (
     GitHubRestoreService,
     _CategoryTally,
     _is_blocked_setting_key,
+    _is_protected_setting_key,
+    _is_skipped_setting_key,
     _parse_dt,
 )
 
@@ -71,6 +73,20 @@ class TestSettingKeyBlocklist:
     def test_ordinary_keys_are_allowed(self, key):
         assert _is_blocked_setting_key(key) is False
 
+    @pytest.mark.parametrize(
+        "key",
+        ["auth_enabled", "advanced_auth_enabled", "local_login_enabled", "setup_completed"],
+    )
+    def test_auth_policy_keys_are_protected(self, key):
+        # Not credential-shaped, so the secret hints never catch them.
+        assert _is_blocked_setting_key(key) is False
+        assert _is_protected_setting_key(key) is True
+        assert _is_skipped_setting_key(key) is True
+
+    @pytest.mark.parametrize("key", ["currency", "ldap_enabled", "auth_secret_key"])
+    def test_protected_set_is_only_the_auth_policy_keys(self, key):
+        assert _is_protected_setting_key(key) is False
+
 
 class TestCategoryTally:
     def test_notes_are_deduplicated(self):
@@ -164,6 +180,36 @@ class TestRestoreSettings:
         assert tally.skipped == 2
         assert any("credential-like" in note for note in tally.notes)
 
+    @pytest.mark.asyncio
+    async def test_auth_settings_are_never_restored(self, db_session):
+        """Restoring auth_enabled=false would disable auth behind the cache's back."""
+        db_session.add(Settings(key="auth_enabled", value="true"))
+        db_session.add(Settings(key="local_login_enabled", value="true"))
+        await db_session.commit()
+        tally = _CategoryTally()
+        payload = {
+            "settings": {
+                "currency": "EUR",
+                "auth_enabled": "false",
+                "advanced_auth_enabled": "false",
+                "local_login_enabled": "false",
+                "setup_completed": "false",
+            }
+        }
+
+        await _service()._restore_settings(db_session, payload, overwrite=True, tally=tally)
+        await db_session.commit()
+
+        rows = {s.key: s.value for s in (await db_session.execute(select(Settings))).scalars().all()}
+        assert rows["auth_enabled"] == "true"
+        assert rows["local_login_enabled"] == "true"
+        assert "advanced_auth_enabled" not in rows
+        assert "setup_completed" not in rows
+        assert rows["currency"] == "EUR"
+        assert tally.restored == 1
+        assert tally.skipped == 4
+        assert any("authentication setting" in note for note in tally.notes)
+
     @pytest.mark.asyncio
     async def test_missing_payload_is_noted_not_fatal(self, db_session):
         tally = _CategoryTally()