diff --git a/backend/app/services/github_restore.py b/backend/app/services/github_restore.py index 57a1ffe47..179a3923c 100644 --- a/backend/app/services/github_restore.py +++ b/backend/app/services/github_restore.py @@ -179,6 +179,25 @@ _COMPANION_CREDENTIALS = { # both set, so an env-configured instance has a usable credential and no row. _COMPANION_CREDENTIAL_ENV = {"ha_token": "HA_TOKEN"} +# The pairs above divide into two classes, because "did the *backup* carry a +# usable credential?" does not mean the same thing for both. +# +# For the availability pairs it is the condition that stops the rule +# over-refusing. An anonymous MQTT broker and an anonymous LDAP bind are working +# configs, so a backup with an empty credential is describing something that +# works, and refusing its toggle would be a false positive. Those pairs only +# matter when the restore would produce a config weaker than *both* the backup +# and the local instance. +# +# For the exposure pair it does not transfer. An empty prometheus_token removes +# /api/v1/metrics' only gate (the route is on PUBLIC_API_ROUTES and its own +# check is ``if token:``), so the exposure is a property of the toggle itself, +# not of a downgrade relative to the backup: a backup taken on an instance that +# enabled Prometheus *without* a token — the field is optional and defaults to +# "" — is the more likely source of one, not the less. So an exposure toggle +# skips this condition and is judged on local state alone. +_COMPANION_EXPOSURE_TOGGLES = frozenset({"prometheus_enabled"}) + def _setting_value_is_true(value: object) -> bool: """True if a settings *payload* value would be stored as "on". @@ -422,12 +441,14 @@ class GitHubRestoreService: # the restore is willing to write travels with its toggle. if not _is_blocked_setting_key(credential): continue - # The backup itself carried no credential here. An anonymous MQTT - # broker and an anonymous LDAP bind are both legitimate configs - # (mqtt_relay.py and ldap_service.py pass empty credentials straight - # through), so refusing this toggle would be a false positive — the - # restore is not producing anything weaker than the backup. - if not _is_usable_credential(values.get(credential)): + # The backup itself carried no credential here. For an availability + # pair that describes a working config — an anonymous MQTT broker and + # an anonymous LDAP bind both are (mqtt_relay.py and ldap_service.py + # pass empty credentials straight through) — so refusing the toggle + # would be a false positive. For an exposure pair a blank credential + # is the hole itself, so the condition is skipped and only local + # state decides. See _COMPANION_EXPOSURE_TOGGLES. + if key not in _COMPANION_EXPOSURE_TOGGLES and not _is_usable_credential(values.get(credential)): continue candidates[key] = credential @@ -568,7 +589,18 @@ class GitHubRestoreService: # policy keys stay unmentioned on purpose. plan = await self._plan_settings(db, values) detail = None - if plan.companion: + if plan.companion and not plan.blocked: + # An exposure toggle becomes a candidate whether or not the + # backup carried its credential, so this commit can refuse a + # switch without having a single credential-like key to skip — + # "0 credential-like key(s) will be skipped" would read as noise. + detail = _Detail( + "settingsCompanionOnlyWillSkip", + f"{len(plan.companion)} switch(es) will be left off — the credential each one needs " + "cannot be restored from a backup", + {"companion": len(plan.companion)}, + ) + elif plan.companion: detail = _Detail( "settingsCompanionWillSkip", f"{len(plan.blocked)} credential-like key(s) will be skipped, and " diff --git a/backend/tests/integration/test_github_restore_api.py b/backend/tests/integration/test_github_restore_api.py index 657afa6e8..d414fcbff 100644 --- a/backend/tests/integration/test_github_restore_api.py +++ b/backend/tests/integration/test_github_restore_api.py @@ -413,6 +413,40 @@ class TestRestoreDoesNotOpenTheMetricsEndpoint: assert "bambuddy_build_info" not in response.text assert any(note["code"] == "settingsCompanionSkipped" for note in tally.notes) + @pytest.mark.asyncio + @pytest.mark.integration + @pytest.mark.parametrize( + "payload", + [ + {"prometheus_enabled": "true", "currency": "EUR"}, + {"prometheus_enabled": "true", "prometheus_token": "", "currency": "EUR"}, + ], + ids=["token-key-absent", "token-blank"], + ) + async def test_a_token_less_backup_leaves_the_endpoint_shut_too( + self, async_client: AsyncClient, db_session, payload + ): + """The route the test above does not cover, and the likelier one. + + ``prometheus_token`` is optional, so an instance can enable Prometheus + without ever setting it. Such a backup carries the toggle and no usable + token — and because the companion rule's second condition asks whether + the *backup* had a credential, that payload used to sail straight past + the refusal and open the endpoint the case above proves shut. + """ + from backend.app.services.github_restore import _CategoryTally, github_restore_service + + assert (await async_client.get("/api/v1/metrics")).status_code == 404 + + tally = _CategoryTally() + await github_restore_service._restore_settings(db_session, {"settings": payload}, overwrite=False, tally=tally) + await db_session.commit() + + response = await async_client.get("/api/v1/metrics") + assert response.status_code == 404, "a token-less Prometheus backup opened the metrics endpoint" + assert "bambuddy_build_info" not in response.text + assert any(note["code"] == "settingsCompanionSkipped" for note in tally.notes) + @pytest.mark.asyncio @pytest.mark.integration async def test_an_instance_with_its_own_token_still_gets_the_toggle_back( diff --git a/backend/tests/unit/test_github_restore.py b/backend/tests/unit/test_github_restore.py index 9c3f525ef..80de3fa6c 100644 --- a/backend/tests/unit/test_github_restore.py +++ b/backend/tests/unit/test_github_restore.py @@ -22,6 +22,7 @@ from backend.app.schemas.github_backup import GitHubRestoreRequest, RestoreCateg from backend.app.services.github_restore import ( _COMPANION_CREDENTIAL_ENV, _COMPANION_CREDENTIALS, + _COMPANION_EXPOSURE_TOGGLES, ARCHIVES_PATH, SETTINGS_PATH, SPOOL_USAGE_PATH, @@ -421,6 +422,64 @@ class TestCompanionCredentials: # credential caveat. assert allowed_detail.code == "settingsCredentialsWillSkip" + # --- The exposure class: a blank backup credential is the hole ---------- + # + # The rule's second condition — "the backup carried a usable credential" — + # is what stops it refusing an anonymous MQTT broker. It does not transfer to + # Prometheus: a backup taken on an instance that enabled Prometheus without a + # token (the field is optional and defaults to "") carries the toggle and no + # usable token, and writing it opens /api/v1/metrics just as wide. That is + # the *more* likely source of the exposure, not the less. + + @pytest.mark.asyncio + async def test_prometheus_is_refused_when_the_backup_has_no_token_key_at_all(self, db_session): + tally = await self._restore(db_session, currency="EUR", prometheus_enabled="true") + + rows = await self._rows(db_session) + assert rows == {"currency": "EUR"} + assert any("prometheus_enabled" in note and "switched off" in note for note in _messages(tally)) + + @pytest.mark.asyncio + @pytest.mark.parametrize("token", ["", " "]) + async def test_prometheus_is_refused_when_the_backup_token_is_blank(self, db_session, token): + tally = await self._restore(db_session, prometheus_enabled="true", prometheus_token=token) + + assert "prometheus_enabled" not in await self._rows(db_session) + assert "settingsCompanionSkipped" in _codes(tally) + + @pytest.mark.asyncio + async def test_the_preview_says_so_with_no_credential_key_to_skip(self, db_session): + """The wording has to survive ``blocked`` being empty. + + The shared caveat counts credential-like keys *and* switches; on this + payload there are no credential-like keys, so "0 credential-like key(s) + will be skipped" would be noise. + """ + parsed = {SETTINGS_PATH: {"settings": {"currency": "EUR", "prometheus_enabled": "true"}}} + + count, detail = await _service()._count_items(db_session, RestoreCategory.SETTINGS, parsed) + + assert count == 1 + assert detail.code == "settingsCompanionOnlyWillSkip" + assert detail.params == {"companion": 1} + + @pytest.mark.asyncio + async def test_the_availability_class_keeps_the_backup_credential_condition(self, db_session): + """The other half of the same change: only Prometheus loses condition 2. + + Absent is treated like blank here — an anonymous broker or bind is a + working config, so refusing it would be a false positive. + """ + await self._restore(db_session, mqtt_enabled="true", ldap_enabled="true", virtual_printer_enabled="true") + + rows = await self._rows(db_session) + assert rows["mqtt_enabled"] == "true" + assert rows["ldap_enabled"] == "true" + assert rows["virtual_printer_enabled"] == "true" + + def test_every_exposure_toggle_is_a_companion_toggle(self): + assert _COMPANION_EXPOSURE_TOGGLES.issubset(_COMPANION_CREDENTIALS) + # --- Controls: over-refusal is the real risk here ---------------------- @pytest.mark.asyncio @@ -433,6 +492,28 @@ class TestCompanionCredentials: assert (await self._rows(db_session))["prometheus_enabled"] == "true" assert not any("switched off" in note for note in _messages(tally)) + @pytest.mark.asyncio + async def test_the_exposure_route_still_stands_down_for_a_local_token(self, db_session): + """Skipping condition 2 must not skip the local-state pass with it.""" + db_session.add(Settings(key="prometheus_token", value="already-set")) + await db_session.commit() + + tally = await self._restore(db_session, prometheus_enabled="true") + + assert (await self._rows(db_session))["prometheus_enabled"] == "true" + assert not any("switched off" in note for note in _messages(tally)) + + @pytest.mark.asyncio + async def test_the_exposure_route_still_stands_down_when_already_on(self, db_session): + """The exposure pre-dates this restore either way — see ruling 3.""" + db_session.add(Settings(key="prometheus_enabled", value="true")) + await db_session.commit() + + tally = await self._restore(db_session, overwrite=True, prometheus_enabled="true") + + assert (await self._rows(db_session))["prometheus_enabled"] == "true" + assert not any("switched off" in note for note in _messages(tally)) + @pytest.mark.asyncio async def test_an_anonymous_broker_is_not_a_false_positive(self, db_session): """mqtt_relay passes an empty password straight through — a real config.""" diff --git a/frontend/src/i18n/locales/de.ts b/frontend/src/i18n/locales/de.ts index 70e85b4ed..8e55a29e2 100644 --- a/frontend/src/i18n/locales/de.ts +++ b/frontend/src/i18n/locales/de.ts @@ -4901,6 +4901,7 @@ export default { settingsNoPayload: 'Keine Einstellungen in den Daten', settingsCredentialsWillSkip: '{{count}} zugangsdatenähnliche Schlüssel werden übersprungen', settingsCompanionWillSkip: '{{count}} zugangsdatenähnliche Schlüssel werden übersprungen und {{companion}} davon abhängige Schalter bleiben aus', + settingsCompanionOnlyWillSkip: '{{companion}} Schalter bleiben aus - die dafür nötigen Zugangsdaten können nicht aus einem Backup wiederhergestellt werden', spoolsUsageCount: 'zzgl. {{count}} Verbrauchseinträge', archivesMetadataOnly: 'Nur Metadaten - 3MF-Dateien und Vorschaubilder sind nicht im Git-Backup enthalten', kprofilesPrinterCount: 'über {{count}} Drucker', diff --git a/frontend/src/i18n/locales/en.ts b/frontend/src/i18n/locales/en.ts index 0cafad7d9..dc601e2bb 100644 --- a/frontend/src/i18n/locales/en.ts +++ b/frontend/src/i18n/locales/en.ts @@ -4946,6 +4946,7 @@ export default { settingsNoPayload: 'No settings in payload', settingsCredentialsWillSkip: '{{count}} credential-like key(s) will be skipped', settingsCompanionWillSkip: '{{count}} credential-like key(s) will be skipped, and {{companion}} switch(es) that depend on them will be left off', + settingsCompanionOnlyWillSkip: '{{companion}} switch(es) will be left off - the credential each one needs cannot be restored from a backup', spoolsUsageCount: 'plus {{count}} usage record(s)', archivesMetadataOnly: 'Metadata only - 3MF files and thumbnails are not in a Git backup', kprofilesPrinterCount: 'across {{count}} printer(s)', diff --git a/frontend/src/i18n/locales/es.ts b/frontend/src/i18n/locales/es.ts index 0e964a13c..77dfb6506 100644 --- a/frontend/src/i18n/locales/es.ts +++ b/frontend/src/i18n/locales/es.ts @@ -4909,6 +4909,7 @@ export default { settingsNoPayload: 'No hay ajustes en los datos', settingsCredentialsWillSkip: 'Se omitirán {{count}} claves con aspecto de credencial', settingsCompanionWillSkip: 'Se omitirán {{count}} claves con aspecto de credencial y {{companion}} interruptores que dependen de ellas quedarán desactivados', + settingsCompanionOnlyWillSkip: '{{companion}} interruptores quedarán desactivados - la credencial que necesita cada uno no se puede restaurar desde una copia de seguridad', spoolsUsageCount: 'más {{count}} registros de consumo', archivesMetadataOnly: 'Solo metadatos - los archivos 3MF y las miniaturas no están en una copia de Git', kprofilesPrinterCount: 'en {{count}} impresoras', diff --git a/frontend/src/i18n/locales/fr.ts b/frontend/src/i18n/locales/fr.ts index ba40e6c27..84fced8d4 100644 --- a/frontend/src/i18n/locales/fr.ts +++ b/frontend/src/i18n/locales/fr.ts @@ -4890,6 +4890,7 @@ export default { settingsNoPayload: 'Aucun réglage dans les données', settingsCredentialsWillSkip: '{{count}} clés ressemblant à des identifiants seront ignorées', settingsCompanionWillSkip: '{{count}} clés ressemblant à des identifiants seront ignorées, et {{companion}} interrupteurs qui en dépendent resteront désactivés', + settingsCompanionOnlyWillSkip: '{{companion}} interrupteurs resteront désactivés - les identifiants dont chacun a besoin ne peuvent pas être restaurés depuis une sauvegarde', spoolsUsageCount: 'plus {{count}} enregistrements de consommation', archivesMetadataOnly: 'Métadonnées uniquement - les fichiers 3MF et les miniatures ne sont pas dans une sauvegarde Git', kprofilesPrinterCount: 'sur {{count}} imprimantes', diff --git a/frontend/src/i18n/locales/it.ts b/frontend/src/i18n/locales/it.ts index 147b2b7b6..51d62c198 100644 --- a/frontend/src/i18n/locales/it.ts +++ b/frontend/src/i18n/locales/it.ts @@ -4889,6 +4889,7 @@ export default { settingsNoPayload: 'Nessuna impostazione nei dati', settingsCredentialsWillSkip: '{{count}} chiavi simili a credenziali verranno saltate', settingsCompanionWillSkip: '{{count}} chiavi simili a credenziali verranno saltate e {{companion}} interruttori che dipendono da esse resteranno disattivati', + settingsCompanionOnlyWillSkip: '{{companion}} interruttori resteranno disattivati - le credenziali necessarie a ciascuno non possono essere ripristinate da un backup', spoolsUsageCount: 'più {{count}} record di consumo', archivesMetadataOnly: 'Solo metadati - i file 3MF e le miniature non sono in un backup Git', kprofilesPrinterCount: 'su {{count}} stampanti', diff --git a/frontend/src/i18n/locales/ja.ts b/frontend/src/i18n/locales/ja.ts index dbcae6f6a..57f854093 100644 --- a/frontend/src/i18n/locales/ja.ts +++ b/frontend/src/i18n/locales/ja.ts @@ -4901,6 +4901,7 @@ export default { settingsNoPayload: 'データに設定が含まれていません', settingsCredentialsWillSkip: '認証情報のようなキー {{count}} 件はスキップされます', settingsCompanionWillSkip: '認証情報のようなキー {{count}} 件はスキップされ、それらに依存するスイッチ {{companion}} 件はオフのままになります', + settingsCompanionOnlyWillSkip: 'スイッチ {{companion}} 件はオフのままになります - それぞれに必要な認証情報はバックアップから復元できません', spoolsUsageCount: '使用履歴 {{count}} 件を含む', archivesMetadataOnly: 'メタデータのみ - 3MF ファイルとサムネイルは Git バックアップに含まれません', kprofilesPrinterCount: 'プリンター {{count}} 台分', diff --git a/frontend/src/i18n/locales/ko.ts b/frontend/src/i18n/locales/ko.ts index 7085207ae..396715b39 100644 --- a/frontend/src/i18n/locales/ko.ts +++ b/frontend/src/i18n/locales/ko.ts @@ -4666,6 +4666,7 @@ export default { settingsNoPayload: '데이터에 설정이 없습니다', settingsCredentialsWillSkip: '자격 증명처럼 보이는 키 {{count}}개를 건너뜁니다', settingsCompanionWillSkip: '자격 증명처럼 보이는 키 {{count}}개를 건너뛰고, 이에 의존하는 스위치 {{companion}}개는 꺼진 상태로 둡니다', + settingsCompanionOnlyWillSkip: '스위치 {{companion}}개는 꺼진 상태로 둡니다 - 각각에 필요한 자격 증명은 백업에서 복원할 수 없습니다', spoolsUsageCount: '사용 기록 {{count}}건 포함', archivesMetadataOnly: '메타데이터만 - 3MF 파일과 썸네일은 Git 백업에 포함되지 않습니다', kprofilesPrinterCount: '프린터 {{count}}대 분량', diff --git a/frontend/src/i18n/locales/pt-BR.ts b/frontend/src/i18n/locales/pt-BR.ts index b26cb3103..c99ba257c 100644 --- a/frontend/src/i18n/locales/pt-BR.ts +++ b/frontend/src/i18n/locales/pt-BR.ts @@ -4889,6 +4889,7 @@ export default { settingsNoPayload: 'Nenhuma configuração nos dados', settingsCredentialsWillSkip: '{{count}} chaves parecidas com credenciais serão ignoradas', settingsCompanionWillSkip: '{{count}} chaves parecidas com credenciais serão ignoradas, e {{companion}} chaves que dependem delas ficarão desligadas', + settingsCompanionOnlyWillSkip: '{{companion}} chaves ficarão desligadas - a credencial que cada uma precisa não pode ser restaurada de um backup', spoolsUsageCount: 'mais {{count}} registros de consumo', archivesMetadataOnly: 'Somente metadados - arquivos 3MF e miniaturas não ficam em um backup Git', kprofilesPrinterCount: 'em {{count}} impressoras', diff --git a/frontend/src/i18n/locales/ru.ts b/frontend/src/i18n/locales/ru.ts index 097af3b30..896b78938 100644 --- a/frontend/src/i18n/locales/ru.ts +++ b/frontend/src/i18n/locales/ru.ts @@ -4658,6 +4658,7 @@ export default { settingsNoPayload: 'В данных нет настроек', settingsCredentialsWillSkip: 'Ключей, похожих на учётные данные, будет пропущено: {{count}}', settingsCompanionWillSkip: 'Ключей, похожих на учётные данные, будет пропущено: {{count}}, а зависящие от них переключатели ({{companion}}) останутся выключенными', + settingsCompanionOnlyWillSkip: 'Переключатели ({{companion}}) останутся выключенными - учётные данные, нужные каждому из них, нельзя восстановить из резервной копии', spoolsUsageCount: 'плюс записей расхода: {{count}}', archivesMetadataOnly: 'Только метаданные - файлы 3MF и миниатюры не входят в резервную копию Git', kprofilesPrinterCount: 'по {{count}} принтерам', diff --git a/frontend/src/i18n/locales/tr.ts b/frontend/src/i18n/locales/tr.ts index 3d9e406e6..0a4a6fddb 100644 --- a/frontend/src/i18n/locales/tr.ts +++ b/frontend/src/i18n/locales/tr.ts @@ -4879,6 +4879,7 @@ export default { settingsNoPayload: 'Veride ayar yok', settingsCredentialsWillSkip: 'Kimlik bilgisi benzeri {{count}} anahtar atlanacak', settingsCompanionWillSkip: 'Kimlik bilgisi benzeri {{count}} anahtar atlanacak ve bunlara bağlı {{companion}} anahtar kapalı bırakılacak', + settingsCompanionOnlyWillSkip: '{{companion}} anahtar kapalı bırakılacak - her birinin ihtiyaç duyduğu kimlik bilgisi bir yedekten geri yüklenemez', spoolsUsageCount: 'ayrıca {{count}} kullanım kaydı', archivesMetadataOnly: 'Yalnızca üst veri - 3MF dosyaları ve küçük resimler Git yedeğinde yer almaz', kprofilesPrinterCount: '{{count}} yazıcı genelinde', diff --git a/frontend/src/i18n/locales/uk.ts b/frontend/src/i18n/locales/uk.ts index fce156de0..1c3800016 100644 --- a/frontend/src/i18n/locales/uk.ts +++ b/frontend/src/i18n/locales/uk.ts @@ -4944,6 +4944,7 @@ export default { settingsNoPayload: "У даних немає налаштувань", settingsCredentialsWillSkip: "Ключів, схожих на облікові дані, буде пропущено: {{count}}", settingsCompanionWillSkip: "Ключів, схожих на облікові дані, буде пропущено: {{count}}, а залежні від них перемикачі ({{companion}}) залишаться вимкненими", + settingsCompanionOnlyWillSkip: 'Перемикачі ({{companion}}) залишаться вимкненими - облікові дані, потрібні кожному з них, не можна відновити з резервної копії', spoolsUsageCount: "плюс записів використання: {{count}}", archivesMetadataOnly: "Лише метадані - файли 3MF і мініатюри не входять до резервної копії Git", kprofilesPrinterCount: "по {{count}} принтерах", diff --git a/frontend/src/i18n/locales/zh-CN.ts b/frontend/src/i18n/locales/zh-CN.ts index 289a76fd4..2e553ad07 100644 --- a/frontend/src/i18n/locales/zh-CN.ts +++ b/frontend/src/i18n/locales/zh-CN.ts @@ -4889,6 +4889,7 @@ export default { settingsNoPayload: '数据中没有设置', settingsCredentialsWillSkip: '将跳过 {{count}} 个疑似凭据的键', settingsCompanionWillSkip: '将跳过 {{count}} 个疑似凭据的键,依赖它们的 {{companion}} 个开关将保持关闭', + settingsCompanionOnlyWillSkip: '{{companion}} 个开关将保持关闭 - 每个开关所需的凭据无法从备份中恢复', spoolsUsageCount: '另有 {{count}} 条使用记录', archivesMetadataOnly: '仅元数据 - 3MF 文件和缩略图不在 Git 备份中', kprofilesPrinterCount: '涉及 {{count}} 台打印机', diff --git a/frontend/src/i18n/locales/zh-TW.ts b/frontend/src/i18n/locales/zh-TW.ts index f70a5400d..e303be869 100644 --- a/frontend/src/i18n/locales/zh-TW.ts +++ b/frontend/src/i18n/locales/zh-TW.ts @@ -4889,6 +4889,7 @@ export default { settingsNoPayload: '資料中沒有設定', settingsCredentialsWillSkip: '將略過 {{count}} 個疑似憑證的鍵', settingsCompanionWillSkip: '將略過 {{count}} 個疑似憑證的鍵,依賴它們的 {{companion}} 個開關會維持關閉', + settingsCompanionOnlyWillSkip: '{{companion}} 個開關會維持關閉 - 每個開關所需的憑證無法從備份還原', spoolsUsageCount: '另有 {{count}} 筆使用紀錄', archivesMetadataOnly: '僅中繼資料 - 3MF 檔案與縮圖不在 Git 備份中', kprofilesPrinterCount: '涵蓋 {{count}} 台印表機',