Skip to content

Post-PR #189: full project audit and remediation roadmap #196

Description

@borg-codex-bot

Purpose

Preserve the full independent project audit performed on 2026-07-12 and use it as the remediation baseline after PR #189 is completed.

This is a tracking and planning issue only. Do not mix these changes into PR #189.

Audit scope

  • Architecture and project structure
  • Python backend and API
  • Backup, restore, scheduling and runtime recovery
  • Canonical storage/repository data model and migrations
  • Authentication, authorization and secret handling
  • Frontend architecture, UX, responsive behavior and accessibility
  • Documentation, tests, packaging and release process
  • Product comparison and future direction

Audit baseline:

  • Branch: codex/issue-184-repository-objects
  • Commit: 529d548
  • Automated tests: 376 passed
  • Python syntax/compilation: successful
  • No audit-related code changes were made

Executive assessment

Borg Backup UI is functionally ahead of a typical early beta. Its strongest areas are the native Unraid integration, repository model, restore tests, restore history, Docker/VM runtime recovery, centralized notifications and bilingual UI.

It is not ready for a public beta yet because three critical security findings and several high operational risks must be addressed first.

Critical findings

1. Static file path traversal

  • Category: Security
  • Location: borg_backup_ui.py:648, borg_backup_ui.py:2758
  • Finding: /ui/ paths are resolved without verifying that the final path remains below UI_DIR.
  • Impact: A crafted request may read files outside the public UI directory.
  • Effort: Small
  • Recommendation: Validate the resolved path with relative_to(UI_DIR.resolve()) and add traversal regression tests.

2. Streaming and download routes bypass central authentication

  • Category: Security / API
  • Location: borg_backup_ui.py:652-661
  • Affected routes:
    • /api/jobs/log/stream
    • /api/restore-tests/log/stream
    • /api/restore/download
    • /api/storage/check/stream
  • Finding: These routes are dispatched before the normal API authentication, role and CSRF checks, with no equivalent checks in the handlers.
  • Impact: Logs, internal paths, errors or archive files may be exposed without a valid UI session.
  • Effort: Medium
  • Recommendation: Route them through the same authentication and authorization layer as all other APIs.

3. Authentication fails open when the user store is corrupt

  • Category: Security / Authentication
  • Location: api/auth_store.py:80-93, borg_backup_ui.py:334-338, borg_backup_ui.py:398-400
  • Finding: A malformed or unreadable users.json becomes an empty default store. Without an active admin, UI authentication is disabled and requests are treated as valid.
  • Impact: A damaged user store can expose the application or reopen administrator bootstrap.
  • Effort: Medium
  • Recommendation: Fail closed. Existing installations with a corrupt store must require an explicit local recovery procedure.

High findings

4. Missing source paths do not stop a backup

  • Category: Backup integrity
  • Location: runtime/lib/backup_job.py:415-434, api/wizard_runner.py:673-690
  • Finding: Missing configured source paths only produce warnings. Borg create and maintenance may still run.
  • Impact: An incomplete archive may be reported as successful and retention may remove older complete archives.
  • Effort: Small to medium
  • Recommendation: Abort by default when any required source root is missing. An explicit advanced partial-backup option may be added later.

5. VM recovery is always marked successful

  • Category: Unraid runtime recovery
  • Location: runtime/lib/vm_manager.py:282-322, runtime/lib/backup_job.py:311-324
  • Finding: VM restart failures are logged but no result is returned; the recovery record is always marked successful.
  • Impact: The UI can report full recovery while one or more VMs remain stopped.
  • Effort: Small
  • Recommendation: Return a structured VM restart result and keep failed recovery entries open.

6. Required migration failures do not block startup

  • Category: Migration / Data integrity
  • Location: api/migrations/registry.py:148-224, borg_backup_ui.py:3235-3257
  • Finding: Failed migrations are logged and shown in system health, but API, scheduler and background workers still start.
  • Impact: A partially migrated data model may receive normal UI and scheduler writes.
  • Effort: Medium
  • Recommendation: Enter maintenance mode after a required migration failure. Keep only login, health, support and migration diagnostics available.

7. No mandatory CI pipeline

  • Category: QA / Release engineering
  • Finding: The local suite is strong, but there are no GitHub Actions or required checks.
  • Impact: Test and preflight execution depends entirely on local discipline.
  • Effort: Medium
  • Recommendation: Add syntax, unit/regression, translation-key and package-preflight checks before public beta.

8. Dark-theme contrast is below accessibility targets

  • Category: Accessibility / UI
  • Location: ui/style.css:16-18
  • Finding: --text-muted: #484f58 on #0d1117 is approximately 2.3:1 and is used for meaningful metadata.
  • Impact: Important timestamps, table labels and status details are difficult to read.
  • Effort: Small
  • Recommendation: Use at least 4.5:1 for all meaningful text and reserve very low contrast for decorative or disabled content.

Medium findings

9. Cleanup is not protected by an outer finally block

  • Category: Stability
  • Location: runtime/lib/backup_job.py:230-252
  • Impact: An unexpected recovery exception can skip status persistence or lock cleanup.
  • Recommendation: Isolate cleanup phases and guarantee lock removal in the outermost finally.

10. Secret export encryption is not authenticated

  • Category: Security / Import-export
  • Location: api/settings_transfer_api.py:730-777
  • Finding: AES-256-CBC with PBKDF2 is used without a MAC or AEAD envelope.
  • Recommendation: Introduce a versioned authenticated format using AEAD or encrypt-then-MAC.

11. Support bundles are sanitized but not fully anonymous

  • Category: Privacy / Support
  • Location: api/support_bundle_api.py
  • Finding: Secrets, known URLs and email addresses are masked, but local paths, SMB share names and some infrastructure details can remain.
  • Recommendation: Label bundles as sanitized rather than anonymous and optionally pseudonymize paths, shares and private addresses.

12. The backend entry module is oversized

  • Category: Architecture / Maintainability
  • Location: borg_backup_ui.py (approximately 3,300 lines and 170 definitions)
  • Finding: HTTP transport, routing, authentication, SSE, downloads, lifecycle and startup are coupled.
  • Recommendation: Incrementally extract routing, authentication, responses and streaming helpers without a framework migration.

13. Frontend modules and styles are oversized

  • Category: Frontend / Maintainability
  • Locations: ui/js/pages/settings.js (approximately 4,600 lines), ui/style.css (approximately 5,000 lines)
  • Finding: Large global window.BBUI modules and substantial innerHTML usage increase coupling and test difficulty.
  • Recommendation: Incrementally split settings areas, dialogs, tables, status cards and formatters. Do not perform a framework rewrite.

14. Default theme is dark instead of system

  • Category: UX
  • Location: ui/js/components/theme.js:10-23
  • Recommendation: Default new users to system and preserve explicit user choices.

15. Restore roots do not cover arbitrary named Unraid pools

  • Category: Restore / Unraid
  • Location: api/restore_api.py:83-97
  • Finding: Safe patterns cover /mnt/user, /mnt/data, /mnt/diskN, /mnt/disks/<name> and /mnt/remotes/<name>, but not named pools such as /mnt/backup.
  • Recommendation: Allow explicitly configured canonical local storage roots while continuing to reject broad roots such as /mnt, /mnt/disks and /mnt/remotes.

16. Documentation has overlapping manual structures

  • Category: Documentation
  • Finding: docs/user-manual is detailed and bilingual, while older material under docs/handbuch and parts of docs/README.md can describe older architecture.
  • Recommendation: Define one canonical documentation structure and archive or clearly label historical material.

Low findings

17. Request body size and connection duration are not bounded

  • Category: Robustness
  • Location: borg_backup_ui.py:2745-2756
  • Recommendation: Apply a default API body limit and sensible request timeouts, with separate limits for intentional uploads.

18. Audit client IP can be spoofed through forwarded headers

  • Category: Audit
  • Recommendation: Trust forwarded headers only when a trusted reverse proxy is explicitly configured.

Strengths to preserve

  • Borg commands are generally executed as argument lists without shell=True.
  • Canonical inventories use process/thread locks, atomic replacement, fsync and mode 0600.
  • Repository deletion verifies usage, active operations, identity and archive count, and writes a lifecycle audit event.
  • Restore uses safe roots, exclusive staging directories and repeated containment validation.
  • Resource locks cover repositories, Docker, VMs and SMB resources.
  • Borg exit codes 0, 1 and >=2 are handled distinctly.
  • Secrets are generally stored in dedicated protected files.
  • German and English resources contain the same 2,047 translation keys.
  • The test suite already provides broad coverage of security, restore paths, locks, migrations, notifications and UI contracts.
  • Restore tests, structured failure categories, restore history and Unraid runtime control are major product differentiators.
  • The canonical separation of storage, repository and job is the correct long-term model.

Product and UX assessment

  • Backup workflow: comprehensive and logical, but dense; missing sources must become blocking errors.
  • Restore workflow: one of the strongest areas due to path protection, prechecks, conflict handling, live logs and persistent history.
  • Storage/repositories: the new canonical model is correct and should remain the only source of truth.
  • Jobs: strong Unraid integration and useful Docker/VM selection; repository references should remain ID-based internally.
  • Restore tests: high user value and a clear differentiator.
  • Monitoring: comprehensive, but pages should prioritize actionable problems and progressively disclose technical detail.
  • Settings: functionally complete, but new options should not automatically create more top-level areas.

Roadmap

Before the next broad tester release

  • Fix all three critical security findings.
  • Abort backups on missing required source paths.
  • Correct VM recovery result tracking.
  • Block normal operation after required migration failures.
  • Correct dark-theme text contrast.
  • Add regression tests for all fixes.

Before public beta

  • Add mandatory CI checks.
  • Add a small browser E2E smoke suite for setup, login, job creation, backup status and restore.
  • Improve support bundle privacy communication and masking.
  • Introduce authenticated secret export encryption.
  • Consolidate documentation ownership.
  • Test fresh installation and every supported tester upgrade state.

Longer term

  • Incrementally modularize backend routing/authentication/streaming.
  • Incrementally modularize settings UI and shared components.
  • Implement the protected-dataset based 3-2-1-0-0 strategy assistant.
  • Introduce a unified operations history for backup, restore, restore tests and maintenance.
  • Add planned storage extensions such as rclone/WebDAV.

Scores

Area Score Summary
Product concept 9/10 Complete Unraid-oriented backup concept with a strong restore focus.
Usability 7/10 Usable and consistent, but dense in several operational views.
User guidance 7/10 Good wizard structure; warnings and progressive disclosure can improve.
Code quality 6/10 Many strong modules, but oversized files and broad exception handling remain.
Architecture 7/10 Canonical model and runtime layer are good; HTTP entry point is strongly coupled.
Maintainability 6/10 Strong tests offset large backend and frontend modules.
Extensibility 8/10 Storage, repository and notification models provide a good base.
Security 4/10 current Good command and restore safety, but three critical access-control issues block public beta.
Performance 7/10 Caching and background refresh exist; large DOM/modules and health calls remain risks.
Documentation 8/10 Strong bilingual manuals and test plans, with some duplicated legacy material.
Future readiness 8/10 Repository model, restore tests and 3-2-1-0-0 concept provide a clear direction.
Innovation 9/10 Native Unraid recovery control and restore verification are distinctive.
Overall quality 7/10 Strong beta product, but critical security findings must be resolved before public release.

Completion condition

This tracking issue can be closed after the critical and high findings have been converted into focused implementation issues, resolved and verified on a fresh install and an upgrade installation.

Metadata

Metadata

Assignees

No one assigned

    Labels

    impact::internalInternal-only change without direct user impactpriority::1-criticalHighest priority; address firstrelease-note::noNo user-facing release note neededtype::maintenanceMaintenance, cleanup, build, or tooling

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions