mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-08-07 19:58:08 +00:00
fix(backup): address CodeRabbit and CodeQL findings on PR #439
- first_time_install.sh: verify chown/chmod succeed and the final owner/group/mode on config_secrets.json before reporting success; exit with a clear error otherwise instead of swallowing failures. - api_v3.py: replace the predictable .writetest probe with an exclusive NamedTemporaryFile to avoid a race with concurrent resolvers; log the preferred/fallback export path and OSError when falling back to the reinstall-deleted directory. - api_v3.py: mark a restore as failed when plugin reinstalls fail, even if file restoration itself succeeded, so the endpoint no longer reports HTTP 200 success on a partial restore. - api_v3.py: stringify plugin IDs before joining them into the error message so a malformed backup's non-string plugin_id can't raise a TypeError and mask the detailed response. - backup_manager.py / api_v3.py: stop putting raw exception text (originating from a user-controlled backup file) into restore results returned to the client; log full details server-side instead. Addresses the CodeQL "stack trace information exposure" alert. - test coverage: add a test for get_plugin_info() resolving a manifest id, and assert the disabled restore_wifi path also skips and omits ytm_auth.json.
This commit is contained in:
@@ -7862,14 +7862,18 @@ def _resolve_backup_export_dir() -> Path:
|
||||
export at all.
|
||||
"""
|
||||
preferred = PROJECT_ROOT.parent / "ledmatrix-backups"
|
||||
fallback = PROJECT_ROOT / "config" / "backups" / "exports"
|
||||
try:
|
||||
preferred.mkdir(parents=True, exist_ok=True)
|
||||
probe = preferred / ".writetest"
|
||||
probe.write_text("", encoding="utf-8")
|
||||
probe.unlink()
|
||||
with tempfile.NamedTemporaryFile(dir=preferred, prefix=".writetest-"):
|
||||
pass
|
||||
return preferred
|
||||
except OSError:
|
||||
return PROJECT_ROOT / "config" / "backups" / "exports"
|
||||
except OSError as e:
|
||||
logger.warning(
|
||||
f"[Backup] Export dir {preferred} is not writable ({e}); "
|
||||
f"falling back to {fallback}, which a reinstall will delete"
|
||||
)
|
||||
return fallback
|
||||
|
||||
|
||||
_BACKUP_EXPORT_DIR = _resolve_backup_export_dir()
|
||||
@@ -8022,7 +8026,15 @@ def backup_restore():
|
||||
else:
|
||||
result.plugins_failed.append({'plugin_id': pid, 'error': 'Store manager unavailable'})
|
||||
except Exception as pe:
|
||||
result.plugins_failed.append({'plugin_id': pid, 'error': str(pe)})
|
||||
logger.error(
|
||||
"[Backup] Failed to reinstall plugin %r: %s", pid, pe, exc_info=True
|
||||
)
|
||||
result.plugins_failed.append({'plugin_id': pid, 'error': 'Installation failed; see server logs'})
|
||||
|
||||
# A restore that dropped files can still report success if the only
|
||||
# failures were plugin reinstalls, since those don't touch result.errors.
|
||||
if result.plugins_failed:
|
||||
result.success = False
|
||||
|
||||
data = result.to_dict()
|
||||
if not result.success:
|
||||
@@ -8032,7 +8044,9 @@ def backup_restore():
|
||||
# config restores and secrets do not. "Restore had errors" alone
|
||||
# left the user unable to tell a wholly failed restore from one
|
||||
# that quietly dropped their API keys.
|
||||
failed_plugins = [p.get('plugin_id') for p in (result.plugins_failed or [])]
|
||||
failed_plugins = [
|
||||
str(p.get('plugin_id')) for p in (result.plugins_failed or []) if p.get('plugin_id')
|
||||
]
|
||||
parts = []
|
||||
if result.restored:
|
||||
parts.append(f"restored: {', '.join(result.restored)}")
|
||||
|
||||
Reference in New Issue
Block a user