backups: the collection interval was never actually readable
Some checks failed
CI / backend (push) Failing after 8s
CI / naming (push) Successful in 2s
CI / frontend (push) Successful in 9s
CI / migrations-mysql (push) Failing after 7s

Backup-NtlarsSettings reads backups_intervalhours from /api/settings/public,
because it runs before it holds any credential, and its Get-IntervalHours falls
back to 24 on any failure. The plugin never declared the key public, so the
endpoint did not return it, the fallback fired on every PC, and the setting
looked configurable in the UI while changing nothing. The fleet log shows the
symptom plainly: "Throttled: last attempt under 24h ago", every cycle,
regardless of what the setting said.

Declared public. A collection cadence is not a secret. backups_shareroot stays
private - it is internal topology - and the test asserts both directions so a
later edit cannot quietly widen it.

Same defect as the 3D parts kiosk label prefix already in this changelog: a
logged-out reader against an allowlist its key was not on. Worth noticing that
the pattern has now bitten twice.
This commit is contained in:
cproudlock
2026-08-11 09:53:43 -04:00
parent 13e6e039fe
commit 6516e76bf4
3 changed files with 28 additions and 0 deletions

View File

@@ -32,6 +32,11 @@ ADR-007 and ADR-002.
PC's revisions can no longer push out a quiet PC's only backup, and the
revision diff compares against the same PC's previous revision rather than
another PC's.
- The backup collection interval could not actually be changed. The collecting
script reads `backups_intervalhours` from the public settings endpoint before
it holds any credential, but the plugin did not declare the key as public, so
the endpoint never returned it and the script fell back to its built-in 24
hours - silently. The setting looked configurable in the UI and was not.
- A backup export containing a value with an empty right-hand side (`Name=`)
failed to parse, and with it the whole file, so that machine could never be
backed up. The form is not strictly legal but occurs in real exports. It is

View File

@@ -144,6 +144,14 @@ class BackupsPlugin(BasePlugin):
'description': 'Minimum hours between collection attempts on a '
'PC. GE-Enforce runs every cycle; the collector '
'skips until this much time has passed.',
# MUST be public. The collecting script reads this from
# /api/settings/public BEFORE it has any credential, and its
# Get-IntervalHours falls back to 24 on any failure - silently.
# Left off the allowlist, the setting looked configurable and
# was not: every PC used 24 no matter what the UI said. A
# collection cadence is not a secret; the share root beneath it
# is internal topology and stays private.
'public': True,
},
{
'key': 'backups_shareroot',

View File

@@ -847,3 +847,18 @@ def test_retention_prunes_each_pc_separately(bk_app, bk_plugin):
rows = _db.session.query(BackupRevision).all()
sources = [row.sourcehostname for row in rows]
assert 'QUIET' in sources, 'the quiet PC lost its only backup'
def test_the_collection_interval_is_readable_without_a_login(bk_app):
"""The collecting script reads backups_intervalhours from
/api/settings/public before it holds any credential, and its
Get-IntervalHours falls back to 24 on ANY failure - silently. Off the
allowlist, the setting looks configurable in the UI and is not: every PC
keeps using 24. The share root must NOT be public; it is internal topology.
"""
from plugins.backups.plugin import BackupsPlugin
declared = {entry['key']: entry for entry in
BackupsPlugin().get_settings_defaults()}
assert declared['backups_intervalhours'].get('public') is True
assert not declared['backups_shareroot'].get('public')