Add Storage Location abstraction and SFTP storage backend by gschmucker95 · Pull Request #11 · Dennis960/BackApp

gschmucker95 · 2026-01-24T12:05:08Z

As requested, I have now split it up a little and would like to start with StorageLocation and SFTP support first. I would bring compression via zip or 7z in the next PR. At least Version and Build Number (Contained in web\src\buildInfo.ts) as referenced in my last PR.

If that's still too many changes, let me know and I'll try to reduce them again....

Summary

This change introduces a Storage Location abstraction and adds an SFTP storage backend, enabling unified management of local and SFTP backup targets. Backend services
and the UI were updated accordingly.

Backend Changes

New/expanded Storage Location entities and details (incl. usage).
Storage backend abstraction plus SFTP implementation.
Updates to backup runs/executor and storage usage calculation (Unix/Windows).
Added SFTP helpers, SSH client/key handling, and file transfer logic.
Controller/router updates for Storage Locations.

Frontend Changes

API updates for storage locations.
UI updates: storage location dialog/list/pages, backup run file views.
Improvements to path picker/storage widget and related forms.

Breaking Changes

None known (please confirm).

Tests

Build Binary File (On Windows)
Build and Deploy Docker Image
- Define StorageLocations (Local and SFTP)
- Enable/Disable Storage Location
  - Define Authtyp: Password (SSH Key not tested)
- Run Backup to:
  - Local
  - SFTP

SFTP Form

SFTP Test (After Click)

Dashboard with Disabled Locations

After Backup with Local:

After Backup with SFTP:

Copilot

Pull request overview

This PR introduces a comprehensive Storage Location abstraction that enables backups to be stored in multiple backend types (local and SFTP). The implementation includes significant changes to both backend and frontend code to support this new architecture.

Changes:

Added storage backend abstraction with local and SFTP implementations
Introduced separate detail tables for storing type-specific configuration (LocalStorageLocationDetails, SftpStorageLocationDetails)
Updated backup execution, file transfer, and storage usage calculation to work with the new abstraction
Enhanced UI with storage location management, SFTP configuration forms, and file availability tracking
Added platform-specific disk usage implementations for Unix and Windows

Reviewed changes

Copilot reviewed 40 out of 42 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
server/entity/storage_location.go	Extended StorageLocation entity to support multiple storage types with SFTP-specific fields
server/entity/storage_location_details.go	New detail entities for type-specific storage configuration
server/entity/backup_file.go	Added available field and changed deleted field JSON serialization (breaking)
server/service/storage_backend*.go	New storage backend abstraction and SFTP implementation
server/service/storage_usage*.go	Updated storage usage calculation for SFTP and refactored disk usage logic
server/service/disk_usage_*.go	Platform-specific disk usage implementations
server/service/sftp_helpers.go	SFTP connection and authentication helpers
server/service/ssh_*.go	Refactored SSH key handling into reusable functions
server/service/storage_location*.go	Major refactoring to support storage location details hydration and type-specific updates
server/service/backup_*.go	Updated backup execution and file transfer to use storage backends
server/controller/storage_location.go	Enhanced controller to handle multipart form data for SSH key uploads
server/controller/backup_run.go	Updated file download to stream from storage backends
web/src/types/storage-location.ts	Extended type definitions for SFTP support
web/src/types/backup-file.ts	Added available field
web/src/pages/StorageLocations.tsx	Enhanced with SFTP configuration, connection testing, and enable/disable functionality
web/src/components/storage-locations/*	Updated UI components for storage location management
web/src/components/backups/BackupFilesTable.tsx	Added file availability status display
web/src/components/common/StorageWidget.tsx	Added disabled location handling and periodic refresh
Various frontend components	Updated to display file availability and handle disabled locations

Files not reviewed (1)

web/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-26T11:12:12Z

server/service/storage_backend_sftp.go

+func (b *sftpStorageBackend) RemoveWithCleanup(filePath string) error {
+	return b.sftpClient.Remove(filePath)
+}


The SFTP implementation of RemoveWithCleanup doesn't actually clean up empty directories like the local implementation does. This creates an inconsistency where local storage automatically removes empty parent directories after file deletion, but SFTP storage leaves them behind. This could lead to directory clutter over time on SFTP storage. Consider implementing directory cleanup for SFTP storage as well, or document this behavioral difference.

Copilot · 2026-01-26T11:12:13Z

server/entity/backup_file.go

 	FileSize    int64      `json:"file_size,omitempty"`
 	Checksum    string     `json:"checksum,omitempty"`
-	Deleted     bool       `gorm:"default:false" json:"deleted"`
+	Deleted     bool       `gorm:"default:false" json:"-"`


The Deleted field has been changed from json:"deleted" to json:"-" which means it's now excluded from JSON serialization. However, this could break existing API consumers that depend on this field. The frontend code in BackupFilesTable.tsx and BackupRunFilesCard.tsx still checks for the deleted property, which suggests the frontend expects to receive this field. Consider keeping the JSON tag or updating all frontend code to use the new available field exclusively.

Suggested change

Deleted bool `gorm:"default:false" json:"-"`

Deleted bool `gorm:"default:false" json:"deleted"`

Copilot · 2026-01-26T11:12:13Z

server/service/storage_usage_sftp.go

+	}
+	defer session.Close()
+
+	cmd := fmt.Sprintf("df -k '%s' | tail -1", remotePath)


The remotePath variable is directly interpolated into the shell command without proper escaping. If remotePath contains special characters like single quotes, it could break the command or potentially lead to command injection. Use the shellQuote function from file_transfer.go or implement similar escaping here to properly handle paths with special characters.

Fine for now, needs to be addressed in the entire repo I think

server/controller/storage_location.go

server/service/sftp_helpers.go

server/entity/storage_location_details.go

Copilot · 2026-01-26T11:12:14Z

server/entity/storage_location.go

+	Password  string    `gorm:"->" json:"password,omitempty"`
+	SSHKey    string    `gorm:"->" json:"ssh_key,omitempty"`


Sensitive fields like Password and SSHKey are being exposed through JSON serialization. Even though they use the gorm:"->"tag (read-only from database), they are still serialized to JSON responses. These fields should either use json:"-" to exclude them from JSON output or use json:"password,omitempty" with sanitization logic before returning to API consumers to prevent credential leakage.

Suggested change

Password string `gorm:"->" json:"password,omitempty"`

SSHKey string `gorm:"->" json:"ssh_key,omitempty"`

Password string `gorm:"->" json:"-"`

SSHKey string `gorm:"->" json:"-"`

server/service/storage_usage_sftp.go

Copilot · 2026-01-26T11:12:14Z

web/src/pages/StorageLocations.tsx

        await storageLocationApi.update(editingLocation.id, data);
        setEditingLocation(null);
      } else {
-        await storageLocationApi.create(data);
+        await storageLocationApi.create(formData);


There's an inconsistency in how data is submitted for create vs update operations. For creating a location (line 115), the raw FormData object is passed, which supports file uploads for SSH keys. However, for updating (line 112), a plain JavaScript object is passed, which means SSH key file uploads won't work during updates. This creates a discrepancy where users can upload SSH keys when creating but not when editing SFTP storage locations.

Copilot · 2026-01-26T11:12:15Z

server/controller/backup_run.go

+		fileInfo.Size(),
+		"application/octet-stream",
+		reader,
+		map[string]string{"Content-Disposition": "attachment; filename=" + filepath.Base(file.RemotePath)},


The filename in the Content-Disposition header is not properly escaped or quoted. If file.RemotePath contains special characters like quotes, spaces, or non-ASCII characters, it could break the Content-Disposition header or potentially be exploited. Use proper RFC 6266 encoding for the filename parameter, such as: fmt.Sprintf("attachment; filename*=UTF-8''%s", url.PathEscape(filepath.Base(file.RemotePath)))

Dennis960 · 2026-01-26T13:37:56Z

server/entity/storage_location.go


-// StorageLocation defines where backups are stored locally
+// StorageLocation defines where backups are stored
 type StorageLocation struct {


You should not put the SFTP credentials as optional fields in the storage location table but rather make it a separate table

Looking further, it seems there is some duplicate code, because I found the same logic in the other entity file

Dennis960 · 2026-01-26T13:38:51Z

server/controller/router.go

 		api.DELETE("/backup-runs/:id", handleBackupRunDelete)
 		api.GET("/backup-files/:fileId", handleBackupFileGet)
 		api.GET("/backup-files/:fileId/download", handleBackupFileDownload)
-		api.DELETE("/backup-files/:fileId", handleBackupFileDelete)


I am not sure why this endpoint was removed

Probably another wrong attempt on my part, since I upload the BackApp backups separately to my NAS and then delete them accordingly. It still displayed them, but when I tried to download them, I got the message “File does not exist.” That was my attempt to work around it. Hence the adjustment in “server/entity/backup_file.go.” I know, quick and dirty...

Dennis960 · 2026-01-26T13:41:27Z

server/entity/storage_location_details.go

+}
+
+// SftpStorageLocationDetails stores SFTP storage configuration.
+type SftpStorageLocationDetails struct {


This exists but is not being used in the code properly as it seems

SftpStorageLocationDetails is used in DB migration and in create/update/hydration paths.
It’s created in "server/service/storage_location.go" and read in "server/service/storage_location_details.go", and the fields are copied into StorageLocation for API responses

Dennis960 · 2026-01-26T13:42:53Z

server/entity/storage_usage.go

 	StorageLocationID uint    `json:"storage_location_id"`
 	Name              string  `json:"name"`
 	BasePath          string  `json:"base_path"`
+	Enabled           bool    `json:"enabled"`


It does not make sense for a local storage location to be disabled. It also does not make sense in general to disable storage locations, instead only backup profiles should have this functionality, as it does basically the same thing

My goal was to display the remote storage (usage and total) accordingly. That's why I put everything in StorageLocation. But I understand that this approach is not ideal. Another option would be to display it under “By Location” right next to the usage display.

Dennis960 · 2026-01-26T13:45:23Z

server/service/backup_run.go

 	if err := DB.Where("backup_run_id = ?", runID).Find(&files).Error; err != nil {
 		return nil, err
 	}
+	if len(files) == 0 {


This file looks like it only works for local storage not SFTP?

It’s just an early exit: if there are no file records for the run, there’s nothing to check, so it returns immediately.
The local/SFTP handling happens after that via NewStorageBackend(location).

Dennis960 · 2026-01-26T13:47:50Z

server/service/disk_usage_windows.go

@@ -0,0 +1,31 @@
+//go:build windows


Dennis960 · 2026-01-26T13:51:43Z

server/service/storage_location_details.go

+	"gorm.io/gorm"
+)
+
+func hydrateStorageLocation(location *entity.StorageLocation) error {


Not sure what this does, a little documentation would be nice

It should be a migration/compatibility helper that “hydrates” a StorageLocation by loading or backfilling its type‑specific details from the new detail tables

Dennis960 · 2026-01-26T13:52:37Z

web/src/api/notifications.ts

  storage_location_id: number;
  name: string;
  base_path: string;
+  enabled: boolean;


Why can storage usage be enabled / disabled?

As mentioned, my goal was to display the remote storage (usage and total) accordingly. That's why I put everything in StorageLocation. However, I understand that this approach is not ideal. Another option would be to display it under “By Location” right next to the usage display.

gschmucker95ski added 2 commits January 24, 2026 12:23

Add Storage Location abstraction and SFTP storage backend

31bf08e

Fix "Moved" or "Deleted" Files

7d7c65f

gschmucker95 changed the title ~~Patch storage location and sftp support~~ Add Storage Location abstraction and SFTP storage backend Jan 24, 2026

Dennis960 requested review from Dennis960 and Copilot January 26, 2026 11:05

Copilot started reviewing on behalf of Dennis960 January 26, 2026 11:05 View session

Copilot AI reviewed Jan 26, 2026

View reviewed changes

Dennis960 requested changes Jan 26, 2026

View reviewed changes

	Deleted bool `gorm:"default:false" json:"-"`
	Deleted bool `gorm:"default:false" json:"deleted"`

		Password string `gorm:"->" json:"password,omitempty"`
		SSHKey string `gorm:"->" json:"ssh_key,omitempty"`

Conversation

gschmucker95 commented Jan 24, 2026

Summary

Backend Changes

Frontend Changes

Breaking Changes

Tests

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 26, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants