diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 1806a80e..73ff02db 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -24,7 +24,7 @@ All business logic lives here. Server Actions must delegate to Services. ``` job-service.ts → CRUD for backup jobs backup-service.ts → Triggers runJob() -retention-service.ts → GVS (Grandfather-Father-Son) algorithm +retention-service.ts → GFS (Grandfather-Father-Son) algorithm restore-service.ts → Restore orchestration ``` diff --git a/README.md b/README.md index 006e7bb8..30f4376f 100644 --- a/README.md +++ b/README.md @@ -97,7 +97,7 @@ Whether you're running a single MySQL database or managing multiple PostgreSQL, ### ⏰ Scheduling & Retention - **Cron-based Scheduling** - Flexible job scheduling with a visual Schedule Picker (Simple Mode + Cron Mode) -- **GVS Retention Policies** - Grandfather-Father-Son rotation with per-destination retention settings +- **GFS Retention Policies** - Grandfather-Father-Son rotation with per-destination retention settings - **Automated Config Backups** - Self-backup of the entire DBackup configuration to any storage adapter ### 👥 Access Control & Security diff --git a/api-docs/openapi.yaml b/api-docs/openapi.yaml index fb4da384..fb10e9b5 100644 --- a/api-docs/openapi.yaml +++ b/api-docs/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: DBackup API - version: 2.4.0 + version: 2.4.1 description: | REST API for DBackup - a self-hosted database backup automation platform with encryption, compression, and smart retention. diff --git a/docs/changelog.md b/docs/changelog.md index 4d025fed..05deeba1 100644 --- a/docs/changelog.md +++ b/docs/changelog.md @@ -2,6 +2,36 @@ All notable changes to DBackup are documented here. +## v2.4.1 - Multiple Bug Fixes across MSSQL, SMB, Retention, and Storage Adapters +*Released: May 27, 2026* + +### 🐛 Bug Fixes + +- **mssql**: Fixed backup abort and missing remote cleanup when SSH-transferring more than ~10 databases due to SSH channel exhaustion - the SFTP session is now cached and reused instead of opening a new channel per operation. +- **mssql**: Fixed "Arithmetic overflow" crash in Database Explorer and Restore for databases larger than ~2 GB. +- **smb**: Fixed `.connection-test-*` probe files not being deleted when `sendFile` throws after the remote file was already created. +- **retention**: Fixed SMART/GFS tier overlap that incorrectly mapped multiple tiers to the same backup, causing over-aggressive deletion. ([#101](https://github.com/Skyfay/DBackup/issues/101)) +- **storage**: Fixed file descriptor leak causing deleted `.tar` temp files to hold disk blocks until container restart, affecting S3, SFTP, FTP, OneDrive, and tar-utils streams. ([#100](https://github.com/Skyfay/DBackup/issues/100)) + +### 🎨 Improvements + +- **retention**: Corrected the retention algorithm abbreviation from "GVS" to "GFS" across UI, documentation, and the built-in retention template. +- **history**: Retention execution logs now include the applied retention template name. + +### 🧪 Tests + +- **smb**: Added unit tests for `finally`-block cleanup when `sendFile` throws and for cleanup retry when the delete itself fails. +- **retention**: Added regression tests for GFS non-overlapping tier selection and template-name visibility in retention history. ([#101](https://github.com/Skyfay/DBackup/issues/101)) +- **ftp/sftp**: Fixed broken upload unit tests by adding missing `destroy: vi.fn()` to the `createReadStream` mock - the adapter calls `fileStream.destroy()` in the `finally` block, which threw a TypeError without this mock method. + +### 🐳 Docker + +- **Image**: `skyfay/dbackup:v2.4.1` +- **Also tagged as**: `latest`, `v2` +- **CI Image**: `skyfay/dbackup:ci` +- **Platforms**: linux/amd64, linux/arm64 + + ## v2.4.0 - Database Explorer Browser, Drill-down Data Viewer, and Bug Fixes *Released: May 25, 2026* diff --git a/docs/developer-guide/advanced/retention.md b/docs/developer-guide/advanced/retention.md index 06e432a9..e1caf58d 100644 --- a/docs/developer-guide/advanced/retention.md +++ b/docs/developer-guide/advanced/retention.md @@ -1,6 +1,6 @@ # Retention System -The Retention System automatically manages backup storage by implementing smart rotation policies based on the Grandfather-Father-Son (GVS) algorithm. +The Retention System automatically manages backup storage by implementing smart rotation policies based on the Grandfather-Father-Son (GFS) algorithm. ## Overview @@ -10,11 +10,11 @@ DBackup supports three retention modes: | :--- | :--- | | **None** | Keep all backups (no deletion) | | **Simple** | Keep the last N backups | -| **Smart (GVS)** | Grandfather-Father-Son strategy | +| **Smart (GFS)** | Grandfather-Father-Son strategy | -## Grandfather-Father-Son (GVS) +## Grandfather-Father-Son (GFS) -The GVS algorithm keeps backups at decreasing frequencies as they age: +The GFS algorithm keeps backups at decreasing frequencies as they age: ``` Today ←──── Daily ────→ Weekly ────→ Monthly ────→ Yearly @@ -108,7 +108,7 @@ export const RetentionService = { keep = sorted.slice(0, config.simple!.keepCount); break; case "SMART": - keep = this.applyGVS(sorted, config.smart!); + keep = this.applyGFS(sorted, config.smart!); break; } @@ -122,7 +122,7 @@ export const RetentionService = { }; }, - applyGVS(files: FileInfo[], config: SmartConfig): FileInfo[] { + applyGFS(files: FileInfo[], config: SmartConfig): FileInfo[] { const keep = new Set(); // Daily buckets diff --git a/docs/developer-guide/architecture.md b/docs/developer-guide/architecture.md index ad1ab420..73d95f77 100644 --- a/docs/developer-guide/architecture.md +++ b/docs/developer-guide/architecture.md @@ -128,7 +128,7 @@ src/services/ ├── job-service.ts # Job CRUD ├── backup-service.ts # Backup triggering ├── restore-service.ts # Restore orchestration -├── retention-service.ts # GVS algorithm +├── retention-service.ts # GFS algorithm ├── encryption-service.ts # Key management ├── user-service.ts # User management └── oidc-provider-service.ts @@ -205,7 +205,7 @@ Executes backups through discrete steps: │ Completion │◀──│ Retention │◀────────┘ │ │ │ │ │ • Cleanup │ │ • Apply │ -│ • Notify │ │ GVS │ +│ • Notify │ │ GFS │ │ • Finalize │ │ • Delete │ └────────────┘ └────────────┘ ``` @@ -286,7 +286,7 @@ Runner Pipeline: │ └── Cleanup, notify, update status │ └── stepRetention() - └── Apply GVS, delete old backups + └── Apply GFS, delete old backups ``` ## Security Architecture diff --git a/docs/developer-guide/core/services.md b/docs/developer-guide/core/services.md index b3c22bbe..b76a0509 100644 --- a/docs/developer-guide/core/services.md +++ b/docs/developer-guide/core/services.md @@ -9,7 +9,7 @@ src/services/ ├── job-service.ts # CRUD for backup jobs ├── backup-service.ts # Trigger backups ├── restore-service.ts # Restore orchestration -├── retention-service.ts # GVS algorithm +├── retention-service.ts # GFS algorithm ├── encryption-service.ts # Encryption profiles ├── integrity-service.ts # SHA-256 checksum verification ├── notification-log-service.ts # Notification log recording & queries diff --git a/docs/index.md b/docs/index.md index 148efdc5..170731a3 100644 --- a/docs/index.md +++ b/docs/index.md @@ -37,7 +37,7 @@ features: details: Each backup job can target multiple storage destinations simultaneously for redundancy or off-site copies. - icon: 📅 title: Scheduling & Retention - details: Cron-based job scheduling with GVS (Grandfather-Father-Son) retention policies for automatic rotation. + details: Cron-based job scheduling with GFS (Grandfather-Father-Son) retention policies for automatic rotation. - icon: 🔔 title: Notifications details: 9+ notification adapters including Discord, Slack, Teams, Telegram, Gotify, ntfy, Webhook, SMS, and Email (SMTP). diff --git a/docs/package.json b/docs/package.json index 88fc6bd1..0373c4a8 100644 --- a/docs/package.json +++ b/docs/package.json @@ -1,6 +1,6 @@ { "name": "dbackup-docs", - "version": "2.4.0", + "version": "2.4.1", "private": true, "scripts": { "dev": "vitepress dev", diff --git a/docs/roadmap.md b/docs/roadmap.md index 2555a4b8..18635333 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -169,7 +169,7 @@ For a full list of completed features, see the [Changelog](/changelog). - ✅ GZIP and Brotli compression - ✅ S3, SFTP, Local, WebDAV, SMB, FTP/FTPS storage adapters - ✅ Discord and Email notifications -- ✅ Cron-based scheduling with GVS retention +- ✅ Cron-based scheduling with GFS retention - ✅ RBAC permission system - ✅ SSO/OIDC authentication (Authentik, PocketID, Generic) - ✅ TOTP and Passkey 2FA diff --git a/docs/user-guide/destinations/index.md b/docs/user-guide/destinations/index.md index 8fc9349f..a5531874 100644 --- a/docs/user-guide/destinations/index.md +++ b/docs/user-guide/destinations/index.md @@ -57,7 +57,7 @@ The `.meta.json` file stores compression, encryption metadata (IV, auth tag, pro Destinations work with retention policies to automatically clean up old backups: - **Simple**: Keep last N backups -- **Smart (GVS)**: Grandfather-Father-Son rotation +- **Smart (GFS)**: Grandfather-Father-Son rotation See [Retention Policies](/user-guide/jobs/retention) for details. - [FTP / FTPS](/user-guide/destinations/ftp) diff --git a/docs/user-guide/first-steps.md b/docs/user-guide/first-steps.md index 42ad397a..35952ede 100644 --- a/docs/user-guide/first-steps.md +++ b/docs/user-guide/first-steps.md @@ -95,7 +95,7 @@ In the **General** tab, use the Schedule picker: In the **Destinations** tab, expand a destination row and configure its retention policy: - **Simple**: Keep last N backups -- **Smart (GVS)**: Grandfather-Father-Son rotation +- **Smart (GFS)**: Grandfather-Father-Son rotation ## Step 4: Run Your First Backup diff --git a/docs/user-guide/getting-started.md b/docs/user-guide/getting-started.md index 67ce30ed..9776bd11 100644 --- a/docs/user-guide/getting-started.md +++ b/docs/user-guide/getting-started.md @@ -13,7 +13,7 @@ DBackup is a self-hosted web application for automating database backups. It sup - **Multi-Destination Jobs**: A single job can upload to multiple storage destinations simultaneously - **Backup Encryption**: AES-256-GCM encryption with an Encryption Vault, key rotation, and offline Recovery Kits - **Compression**: Built-in GZIP and Brotli compression -- **Scheduling & Retention**: Cron-based scheduling with GVS (Grandfather-Father-Son) retention policies +- **Scheduling & Retention**: Cron-based scheduling with GFS (Grandfather-Father-Son) retention policies - **Notifications**: 9+ adapters including Discord, Slack, Teams, Telegram, Email, and more - **Storage Monitoring**: Per-destination alerts for usage spikes, storage limit warnings, and missing backups - **Restore**: Browse backup history, verify checksums, and restore directly to a database including database remapping diff --git a/docs/user-guide/jobs/retention.md b/docs/user-guide/jobs/retention.md index f7e3f109..87b12450 100644 --- a/docs/user-guide/jobs/retention.md +++ b/docs/user-guide/jobs/retention.md @@ -10,7 +10,7 @@ Retention policies prevent unlimited storage growth by automatically deleting ol | :--- | :--- | :--- | | **None** | Keep all backups | Manual management | | **Simple** | Keep last N backups | Fixed rotation | -| **Smart (GVS)** | Grandfather-Father-Son | Long-term archival | +| **Smart (GFS)** | Grandfather-Father-Son | Long-term archival | ## Per-Destination Retention @@ -55,7 +55,7 @@ With `Keep Count: 5`: - Frequent backups with short retention - Simple rotation needs -## Smart Retention (GVS) +## Smart Retention (GFS) Grandfather-Father-Son is an intelligent retention strategy that keeps: - Recent backups (daily) @@ -259,7 +259,7 @@ With compression (70% reduction): ### Wrong Backups Deleted -The GVS algorithm keeps the **oldest** backup in each time bucket. This is intentional: +The GFS algorithm keeps the **oldest** backup in each time bucket. This is intentional: - Weekly: Keeps backup from start of week - Monthly: Keeps backup from start of month diff --git a/package.json b/package.json index 58ae7eb6..90b5bae0 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dbackup", - "version": "2.4.0", + "version": "2.4.1", "private": true, "scripts": { "dev": "next dev", diff --git a/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql b/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql new file mode 100644 index 00000000..ec2e854c --- /dev/null +++ b/prisma/migrations/20260527000001_fix_gfs_retention_name/migration.sql @@ -0,0 +1,12 @@ +-- Fix: Rename built-in retention template from "Smart GVS" to "Smart GFS" (correct Grandfather-Father-Son abbreviation) +-- Guard: skip if the target name already exists (user may have created a policy with that name) +-- to avoid a unique-constraint failure on the "name" column. +UPDATE "RetentionPolicy" +SET "name" = 'Smart GFS (7/4/12/2)', + "description" = 'Grandfather-Father-Son: keep 7 daily, 4 weekly, 12 monthly, 2 yearly backups.', + "updatedAt" = CURRENT_TIMESTAMP +WHERE "id" = 'retention-gvs-default' + AND "name" = 'Smart GVS (7/4/12/2)' + AND NOT EXISTS ( + SELECT 1 FROM "RetentionPolicy" WHERE "name" = 'Smart GFS (7/4/12/2)' + ); diff --git a/public/openapi.yaml b/public/openapi.yaml index a76ebcd5..98da3e9e 100644 --- a/public/openapi.yaml +++ b/public/openapi.yaml @@ -1,7 +1,7 @@ openapi: 3.1.0 info: title: DBackup API - version: 2.4.0 + version: 2.4.1 description: | REST API for DBackup - a self-hosted database backup automation platform with encryption, compression, and smart retention. diff --git a/src/components/settings/templates/retention-policy-list.tsx b/src/components/settings/templates/retention-policy-list.tsx index c11841e4..290920c9 100644 --- a/src/components/settings/templates/retention-policy-list.tsx +++ b/src/components/settings/templates/retention-policy-list.tsx @@ -108,7 +108,7 @@ export function RetentionPolicyList() { return `Simple - keep ${parsed.simple?.keepCount ?? "?"} backups`; if (parsed.mode === "SMART") { const s = parsed.smart; - return `Smart GVS (${s?.daily ?? 0}/${s?.weekly ?? 0}/${s?.monthly ?? 0}/${s?.yearly ?? 0})`; + return `Smart GFS (${s?.daily ?? 0}/${s?.weekly ?? 0}/${s?.monthly ?? 0}/${s?.yearly ?? 0})`; } } catch { // ignore @@ -358,7 +358,7 @@ export function RetentionPolicyDialog({ id="rp-name" value={name} onChange={(e) => setName(e.target.value)} - placeholder="e.g. Smart GVS Production" + placeholder="e.g. Smart GFS Production" />
diff --git a/src/lib/adapters/database/common/tar-utils.ts b/src/lib/adapters/database/common/tar-utils.ts index 0e22ac88..a892a7b5 100644 --- a/src/lib/adapters/database/common/tar-utils.ts +++ b/src/lib/adapters/database/common/tar-utils.ts @@ -89,7 +89,10 @@ export async function createMultiDbTar( // Stream file contents to tar entry const fileStream = createReadStream(file.path); await new Promise((resolve, reject) => { - fileStream.on("error", reject); + fileStream.on("error", (err) => { + fileStream.destroy(); + reject(err); + }); fileStream.on("end", () => { entry.end(); resolve(); @@ -179,13 +182,19 @@ export async function extractMultiDbTar( files: extractedFiles, }); }); + const readStream = createReadStream(sourcePath); /* v8 ignore start */ extractor.on("error", (err) => { + readStream.destroy(); + reject(err); + }); + readStream.on("error", (err) => { + extractor.destroy(err); reject(err); }); /* v8 ignore end */ - createReadStream(sourcePath).pipe(extractor); + readStream.pipe(extractor); }); } @@ -261,13 +270,19 @@ export async function extractSelectedDatabases( }); }); + const readStream = createReadStream(sourcePath); /* v8 ignore start */ extractor.on("error", (err) => { + readStream.destroy(); + reject(err); + }); + readStream.on("error", (err) => { + extractor.destroy(err); reject(err); }); /* v8 ignore end */ - createReadStream(sourcePath).pipe(extractor); + readStream.pipe(extractor); }); } @@ -320,6 +335,7 @@ export async function readTarManifest(filePath: string): Promise { const extractor = extract(); let manifestFound = false; + const readStream = createReadStream(filePath); extractor.on("entry", (header, stream, next) => { if (header.name === MANIFEST_FILENAME) { @@ -355,14 +371,19 @@ export async function readTarManifest(filePath: string): Promise { + readStream.destroy(); resolve(null); }); extractor.on("close", () => { - // Handle early close from destroy() + readStream.destroy(); }); - createReadStream(filePath).pipe(extractor); + readStream.on("error", () => { + extractor.destroy(); + resolve(null); + }); + readStream.pipe(extractor); }); } diff --git a/src/lib/adapters/database/mssql/browser.ts b/src/lib/adapters/database/mssql/browser.ts index e47e649b..6dfd1b9d 100644 --- a/src/lib/adapters/database/mssql/browser.ts +++ b/src/lib/adapters/database/mssql/browser.ts @@ -22,7 +22,7 @@ export async function getTables(config: MSSQLConfig, database: string): Promise< t.TABLE_NAME AS name, t.TABLE_TYPE AS table_type, COALESCE(SUM(p.rows), 0) AS row_count, - COALESCE(SUM(a.total_pages) * 8 * 1024, 0) AS size_bytes + COALESCE(SUM(CAST(a.total_pages AS BIGINT)) * 8 * 1024, 0) AS size_bytes FROM [${dbId}].INFORMATION_SCHEMA.TABLES t LEFT JOIN [${dbId}].sys.tables st ON st.name = t.TABLE_NAME LEFT JOIN [${dbId}].sys.indexes i ON i.object_id = st.object_id AND i.type <= 1 diff --git a/src/lib/adapters/database/mssql/connection.ts b/src/lib/adapters/database/mssql/connection.ts index 65f119b2..b6a5d111 100644 --- a/src/lib/adapters/database/mssql/connection.ts +++ b/src/lib/adapters/database/mssql/connection.ts @@ -158,7 +158,7 @@ export async function getDatabasesWithStats(config: MSSQLConfig): Promise 4 diff --git a/src/lib/adapters/database/mssql/ssh-transfer.ts b/src/lib/adapters/database/mssql/ssh-transfer.ts index fde7ca0d..8364be59 100644 --- a/src/lib/adapters/database/mssql/ssh-transfer.ts +++ b/src/lib/adapters/database/mssql/ssh-transfer.ts @@ -20,6 +20,7 @@ const log = logger.child({ adapter: "mssql", module: "ssh-transfer" }); export class MssqlSshTransfer { private client: Client; private connected = false; + private sftpSession: SFTPWrapper | null = null; constructor() { this.client = new Client(); @@ -211,20 +212,28 @@ export class MssqlSshTransfer { */ public end(): void { if (this.connected) { + if (this.sftpSession) { + this.sftpSession.end(); + this.sftpSession = null; + } this.client.end(); this.connected = false; } } /** - * Get SFTP subsystem from the SSH connection + * Get SFTP subsystem from the SSH connection. + * Reuses an existing session - opening a new channel per call exhausts + * the SSH server's channel limit when backing up many databases. */ private getSftp(): Promise { + if (this.sftpSession) return Promise.resolve(this.sftpSession); return new Promise((resolve, reject) => { this.client.sftp((err, sftp) => { if (err) { reject(new Error(`Failed to initialize SFTP: ${err.message}`)); } else { + this.sftpSession = sftp; resolve(sftp); } }); diff --git a/src/lib/adapters/storage/ftp.ts b/src/lib/adapters/storage/ftp.ts index 7f12bbb1..19fe37d9 100644 --- a/src/lib/adapters/storage/ftp.ts +++ b/src/lib/adapters/storage/ftp.ts @@ -94,7 +94,12 @@ export const FTPAdapter: StorageAdapter = { } }); - await client.uploadFrom(createReadStream(localPath), destination); + const fileStream = createReadStream(localPath); + try { + await client.uploadFrom(fileStream, destination); + } finally { + fileStream.destroy(); + } client.trackProgress(); diff --git a/src/lib/adapters/storage/onedrive.ts b/src/lib/adapters/storage/onedrive.ts index 98ffdbfa..e059e97c 100644 --- a/src/lib/adapters/storage/onedrive.ts +++ b/src/lib/adapters/storage/onedrive.ts @@ -233,24 +233,28 @@ export const OneDriveAdapter: StorageAdapter = { const fileStream = createReadStream(localPath, { highWaterMark: UPLOAD_CHUNK_SIZE }); let offset = 0; - for await (const chunk of fileStream) { - const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); - const end = offset + buffer.length - 1; - - await fetch(uploadUrl, { - method: "PUT", - headers: { - "Content-Range": `bytes ${offset}-${end}/${fileSize}`, - "Content-Length": String(buffer.length), - }, - body: buffer, - }); - - offset += buffer.length; - - if (onProgress) { - onProgress(Math.min(99, Math.round((offset / fileSize) * 100))); + try { + for await (const chunk of fileStream) { + const buffer = Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk); + const end = offset + buffer.length - 1; + + await fetch(uploadUrl, { + method: "PUT", + headers: { + "Content-Range": `bytes ${offset}-${end}/${fileSize}`, + "Content-Length": String(buffer.length), + }, + body: buffer, + }); + + offset += buffer.length; + + if (onProgress) { + onProgress(Math.min(99, Math.round((offset / fileSize) * 100))); + } } + } finally { + fileStream.destroy(); } } diff --git a/src/lib/adapters/storage/s3.ts b/src/lib/adapters/storage/s3.ts index 07381609..e4f0b88b 100644 --- a/src/lib/adapters/storage/s3.ts +++ b/src/lib/adapters/storage/s3.ts @@ -44,11 +44,10 @@ async function s3Upload(internalConfig: S3InternalConfig, localPath: string, rem const client = S3ClientFactory.create(internalConfig); const targetKey = S3ClientFactory.getTargetKey(internalConfig, remotePath); - try { - if (onLog) onLog(`Starting S3 upload to bucket: ${internalConfig.bucket}, key: ${targetKey}`, 'info', 'storage'); - - const fileStream = createReadStream(localPath); + if (onLog) onLog(`Starting S3 upload to bucket: ${internalConfig.bucket}, key: ${targetKey}`, 'info', 'storage'); + const fileStream = createReadStream(localPath); + try { const parallelUploads3 = new Upload({ client: client, params: { @@ -73,6 +72,8 @@ async function s3Upload(internalConfig: S3InternalConfig, localPath: string, rem log.error("S3 upload failed", { bucket: internalConfig.bucket, targetKey }, wrapError(error)); if (onLog && error instanceof Error) onLog(`S3 upload failed: ${error.message}`, 'error', 'storage', error instanceof Error ? error.stack : undefined); return false; + } finally { + fileStream.destroy(); } } diff --git a/src/lib/adapters/storage/sftp.ts b/src/lib/adapters/storage/sftp.ts index 51599610..de7cca18 100644 --- a/src/lib/adapters/storage/sftp.ts +++ b/src/lib/adapters/storage/sftp.ts @@ -82,16 +82,21 @@ export const SFTPAdapter: StorageAdapter = { // Note: ssh2-sftp-client default 'step' progress might not be granualr enough for small files, but works. // However, the signature is (total_transferred, chunk, total). - await sftp.put(createReadStream(localPath), destination, { - step: (total_transferred: any, _chunk: any, _total: any) => { - if (onProgress && totalSize > 0) { - // total param in callback is total bytes to transfer, which is known if we pass it, but put() with stream might not know it unless we checked. - // We use our known totalSize. - const percent = Math.round((total_transferred / totalSize) * 100); - onProgress(percent); + const fileStream = createReadStream(localPath); + try { + await sftp.put(fileStream, destination, { + step: (total_transferred: any, _chunk: any, _total: any) => { + if (onProgress && totalSize > 0) { + // total param in callback is total bytes to transfer, which is known if we pass it, but put() with stream might not know it unless we checked. + // We use our known totalSize. + const percent = Math.round((total_transferred / totalSize) * 100); + onProgress(percent); + } } - } - } as any); + } as any); + } finally { + fileStream.destroy(); + } if (onLog) onLog(`SFTP upload completed successfully`, 'info', 'storage'); return true; diff --git a/src/lib/adapters/storage/smb.ts b/src/lib/adapters/storage/smb.ts index 9c89755d..6f97c606 100644 --- a/src/lib/adapters/storage/smb.ts +++ b/src/lib/adapters/storage/smb.ts @@ -201,10 +201,12 @@ export const SMBAdapter: StorageAdapter = { async test(config: SMBConfig): Promise<{ success: boolean; message: string }> { const testFileName = `.connection-test-${Date.now()}`; - try { - const client = createClient(config); - const destination = resolvePath(config, testFileName); + const tmpPath = path.join(os.tmpdir(), testFileName); + // Initialized outside try/catch so they are always accessible in the finally block. + const client = createClient(config); + const destination = resolvePath(config, testFileName); + try { // Ensure pathPrefix directory exists if set if (config.pathPrefix) { try { @@ -215,27 +217,25 @@ export const SMBAdapter: StorageAdapter = { } // Create a temp file to upload - const tmpPath = path.join(os.tmpdir(), testFileName); await fs.writeFile(tmpPath, "Connection Test"); - let remoteFileCreated = false; - try { - // 1. Write Test - await client.sendFile(tmpPath, destination); - remoteFileCreated = true; + // 1. Write Test + await client.sendFile(tmpPath, destination); - // 2. Delete Test - await client.deleteFile(destination); - remoteFileCreated = false; + // 2. Delete Test + await client.deleteFile(destination); - return { success: true, message: "Connection successful (Write/Delete verified)" }; - } finally { - if (remoteFileCreated) await client.deleteFile(destination).catch(() => {}); - await fs.unlink(tmpPath).catch(() => {}); - } + return { success: true, message: "Connection successful (Write/Delete verified)" }; } catch (error: unknown) { const message = error instanceof Error ? error.message : String(error); return { success: false, message: `SMB Connection failed: ${message}` }; + } finally { + // Always attempt remote cleanup. This guards against the edge case where + // sendFile partially succeeded (file exists on the SMB share) but then + // threw before the explicit deleteFile call was reached. "File not found" + // errors from the redundant delete in the success path are silently ignored. + await client.deleteFile(destination).catch(() => {}); + await fs.unlink(tmpPath).catch(() => {}); } }, }; diff --git a/src/lib/runner/steps/01-initialize.ts b/src/lib/runner/steps/01-initialize.ts index e88f0eda..e2b24a54 100644 --- a/src/lib/runner/steps/01-initialize.ts +++ b/src/lib/runner/steps/01-initialize.ts @@ -67,20 +67,27 @@ export async function stepInitialize(ctx: RunnerContext) { } let retention: RetentionConfiguration = { mode: 'NONE' }; + let retentionPolicyName: string | undefined; + let retentionPolicySource: DestinationContext['retentionPolicySource'] = 'none'; try { if (dest.retentionPolicyId) { // Policy template takes priority over the legacy per-destination retention JSON const policy = await prisma.retentionPolicy.findUnique({ where: { id: dest.retentionPolicyId } }); if (policy?.config) { retention = JSON.parse(policy.config as string); + retentionPolicyName = policy.name; + retentionPolicySource = 'template'; } } else if (dest.retention && dest.retention !== '{}') { retention = JSON.parse(dest.retention); + retentionPolicySource = 'legacy'; } else { // No per-destination policy and no legacy config - fall back to the system default retention policy const defaultPolicy = await prisma.retentionPolicy.findFirst({ where: { isDefault: true } }); if (defaultPolicy?.config) { retention = JSON.parse(defaultPolicy.config as string); + retentionPolicyName = defaultPolicy.name; + retentionPolicySource = 'default'; } } } catch { @@ -93,6 +100,8 @@ export async function stepInitialize(ctx: RunnerContext) { adapter, config: await resolveAdapterConfig(dest.config) as any, retention, + retentionPolicyName, + retentionPolicySource, priority: dest.priority, adapterId: dest.config.adapterId, }; diff --git a/src/lib/runner/steps/05-retention.ts b/src/lib/runner/steps/05-retention.ts index 47dfd07f..399ebf5f 100644 --- a/src/lib/runner/steps/05-retention.ts +++ b/src/lib/runner/steps/05-retention.ts @@ -45,7 +45,22 @@ async function applyRetentionForDestination(ctx: RunnerContext, dest: Destinatio return 0; } - ctx.log(`${destLabel} Retention: Applying policy ${policy.mode}...`); + const policyDetails = (() => { + if (dest.retentionPolicyName) { + if (dest.retentionPolicySource === 'default') { + return `${policy.mode} (default template: ${dest.retentionPolicyName})`; + } + return `${policy.mode} (template: ${dest.retentionPolicyName})`; + } + + if (dest.retentionPolicySource === 'legacy') { + return `${policy.mode} (legacy inline policy)`; + } + + return policy.mode; + })(); + + ctx.log(`${destLabel} Retention: Applying policy ${policyDetails}...`); if (!dest.adapter.list) { ctx.log(`${destLabel} Retention warning: Storage adapter does not support listing files. Skipped.`); diff --git a/src/lib/runner/types.ts b/src/lib/runner/types.ts index d7aeaebc..966be163 100644 --- a/src/lib/runner/types.ts +++ b/src/lib/runner/types.ts @@ -19,6 +19,8 @@ export interface DestinationContext { adapter: StorageAdapter; config: Record; // decrypted adapter config retention: RetentionConfiguration; + retentionPolicyName?: string; + retentionPolicySource?: 'template' | 'default' | 'legacy' | 'none'; priority: number; adapterId: string; uploadResult?: { diff --git a/src/services/backup/retention-service.ts b/src/services/backup/retention-service.ts index 258bb8ab..b95c9b59 100644 --- a/src/services/backup/retention-service.ts +++ b/src/services/backup/retention-service.ts @@ -1,6 +1,6 @@ import { FileInfo } from '@/lib/core/interfaces'; import { RetentionConfiguration } from '@/lib/core/retention'; -import { format, getISOWeek, getYear } from 'date-fns'; +import { formatInTimeZone } from 'date-fns-tz'; type FileWithReasons = { file: FileInfo; @@ -56,57 +56,67 @@ export class RetentionService { private static applySmartPolicy(files: FileWithReasons[], policy: NonNullable) { const { daily, weekly, monthly, yearly } = policy; - // Track used slots - const usedDays = new Set(); - const usedWeeks = new Set(); - const usedMonths = new Set(); - const usedYears = new Set(); + // SMART/GFS is applied as non-overlapping tiers. + // Daily picks newest unique days first. + // Weekly/Monthly/Yearly then pick additional representatives from older buckets. + this.applyTier( + files, + daily, + (date) => formatInTimeZone(date, 'UTC', 'yyyy-MM-dd'), + 'Daily' + ); + + this.applyTier( + files, + weekly, + (date) => formatInTimeZone(date, 'UTC', "RRRR-'W'II"), + 'Weekly' + ); + + this.applyTier( + files, + monthly, + (date) => formatInTimeZone(date, 'UTC', 'yyyy-MM'), + 'Monthly' + ); + + this.applyTier( + files, + yearly, + (date) => formatInTimeZone(date, 'UTC', 'yyyy'), + 'Yearly' + ); + } + + private static applyTier( + files: FileWithReasons[], + limit: number, + getBucketKey: (date: Date) => string, + reasonPrefix: string + ) { + if (limit <= 0) return; + + const usedBuckets = new Set(); + // Existing keeps from earlier tiers reserve their bucket in this tier. for (const entry of files) { - const date = entry.file.lastModified; - - // Keys based on local time or UTC? Ideally UTC strictly, but date-fns objects work well. - // Using standard formats - const dayKey = format(date, 'yyyy-MM-dd'); - const weekKey = `${getYear(date)}-W${getISOWeek(date)}`; - const monthKey = format(date, 'yyyy-MM'); - const yearKey = format(date, 'yyyy'); - - // 1. Daily Check - if (usedDays.size < daily) { - if (!usedDays.has(dayKey)) { - entry.keep = true; - entry.reasons.push(`Daily (${dayKey})`); - usedDays.add(dayKey); - } - } + if (!entry.keep) continue; + usedBuckets.add(getBucketKey(entry.file.lastModified)); + } - // 2. Weekly Check - if (usedWeeks.size < weekly) { - if (!usedWeeks.has(weekKey)) { - entry.keep = true; - entry.reasons.push(`Weekly (${weekKey})`); - usedWeeks.add(weekKey); - } - } + let keptInTier = 0; + for (const entry of files) { + if (entry.keep) continue; - // 3. Monthly Check - if (usedMonths.size < monthly) { - if (!usedMonths.has(monthKey)) { - entry.keep = true; - entry.reasons.push(`Monthly (${monthKey})`); - usedMonths.add(monthKey); - } - } + const bucketKey = getBucketKey(entry.file.lastModified); + if (usedBuckets.has(bucketKey)) continue; - // 4. Yearly Check - if (usedYears.size < yearly) { - if (!usedYears.has(yearKey)) { - entry.keep = true; - entry.reasons.push(`Yearly (${yearKey})`); - usedYears.add(yearKey); - } - } + entry.keep = true; + entry.reasons.push(`${reasonPrefix} (${bucketKey})`); + usedBuckets.add(bucketKey); + keptInTier++; + + if (keptInTier >= limit) break; } } } diff --git a/tests/unit/adapters/storage/ftp.test.ts b/tests/unit/adapters/storage/ftp.test.ts index 084edfe5..4cf13149 100644 --- a/tests/unit/adapters/storage/ftp.test.ts +++ b/tests/unit/adapters/storage/ftp.test.ts @@ -33,10 +33,10 @@ vi.mock("basic-ftp", () => { }); vi.mock("fs", () => ({ - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), createWriteStream: vi.fn(() => ({ on: vi.fn(), end: vi.fn() })), default: { - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), createWriteStream: vi.fn(() => ({ on: vi.fn(), end: vi.fn() })), }, })); diff --git a/tests/unit/adapters/storage/sftp.test.ts b/tests/unit/adapters/storage/sftp.test.ts index c6d35444..1c91f3d9 100644 --- a/tests/unit/adapters/storage/sftp.test.ts +++ b/tests/unit/adapters/storage/sftp.test.ts @@ -33,9 +33,9 @@ vi.mock("ssh2-sftp-client", () => { }); vi.mock("fs", () => ({ - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), default: { - createReadStream: vi.fn(() => ({ pipe: vi.fn() })), + createReadStream: vi.fn(() => ({ pipe: vi.fn(), destroy: vi.fn() })), }, promises: { stat: mockFsStat, diff --git a/tests/unit/adapters/storage/smb.test.ts b/tests/unit/adapters/storage/smb.test.ts index 2b8c72a1..7bf9fb75 100644 --- a/tests/unit/adapters/storage/smb.test.ts +++ b/tests/unit/adapters/storage/smb.test.ts @@ -271,6 +271,30 @@ describe("SMBAdapter", () => { expect(result.success).toBe(false); expect(result.message).toContain("ECONNREFUSED"); }); + + it("attempts remote cleanup when sendFile throws (partial upload guard)", async () => { + // Simulate: file was created on the SMB share but sendFile threw before + // the client received the final ACK (e.g. network hiccup mid-transfer). + mockSendFile.mockRejectedValue(new Error("connection reset")); + + await SMBAdapter.test!(config); + + // deleteFile must be called in the finally block even though sendFile threw, + // so orphaned .connection-test-* files cannot accumulate on the share. + expect(mockDeleteFile).toHaveBeenCalled(); + }); + + it("does not leave orphaned files when deleteFile throws after successful upload", async () => { + // sendFile succeeds, deleteFile fails on first attempt (server-side error), + // but the finally block retries it unconditionally. + mockDeleteFile.mockRejectedValueOnce(new Error("NT_STATUS_SHARING_VIOLATION")); + + const result = await SMBAdapter.test!(config); + + expect(result.success).toBe(false); + // finally block must have retried deleteFile after the explicit call failed + expect(mockDeleteFile).toHaveBeenCalledTimes(2); + }); }); // ===== resolvePath without pathPrefix ===== diff --git a/tests/unit/runner/steps/05-retention.test.ts b/tests/unit/runner/steps/05-retention.test.ts index ce059156..dcae5946 100644 --- a/tests/unit/runner/steps/05-retention.test.ts +++ b/tests/unit/runner/steps/05-retention.test.ts @@ -151,6 +151,27 @@ describe('stepRetention', () => { expect(ctx.log).toHaveBeenCalledWith(expect.stringContaining('Keeping 1, Deleting 1')); }); + it('logs the selected retention template name when applying policy', async () => { + const { RetentionService } = await import('@/services/backup/retention-service'); + (RetentionService.calculateRetention as ReturnType).mockReturnValue({ + keep: [], + delete: [], + }); + + const dest = makeDestination({ + retention: { mode: 'SMART', smart: { daily: 1, weekly: 1, monthly: 1, yearly: 0 } } as any, + retentionPolicyName: 'Default GFS', + retentionPolicySource: 'template', + }); + const ctx = makeCtx({ destinations: [dest] }); + + await stepRetention(ctx); + + expect(ctx.log).toHaveBeenCalledWith( + expect.stringContaining('Retention: Applying policy SMART (template: Default GFS)...') + ); + }); + it('logs an error but does not throw when a delete fails', async () => { const { RetentionService } = await import('@/services/backup/retention-service'); (RetentionService.calculateRetention as ReturnType).mockReturnValue({ diff --git a/tests/unit/services/retention-service.test.ts b/tests/unit/services/retention-service.test.ts index b9f83d8d..6921c06a 100644 --- a/tests/unit/services/retention-service.test.ts +++ b/tests/unit/services/retention-service.test.ts @@ -96,11 +96,11 @@ describe('RetentionService', () => { }); }); - describe('Smart Policy (GVS)', () => { - it('should correctly handle daily, weekly, monthly, and yearly retention', () => { + describe('Smart Policy (GFS)', () => { + it('keeps additional backups from older tiers instead of collapsing to the same newest file', () => { const now = new Date('2026-01-24T12:00:00Z'); - // Construct specific GVS scenarios rather than a loop + // Construct specific GFS scenarios rather than a loop const dates: Date[] = [ // DAILY: Days 0-6 (7 backups) now, @@ -153,22 +153,9 @@ describe('RetentionService', () => { expect(result.keep.map(f => f.lastModified.getTime())).toContain(dates[i].getTime()); } - // 2. Verify Weekly (1-4 weeks back kept) - // Note: - // - Daily backups cover Jan 24 (W04) through Jan 18 (W03). - // - W04 slot is taken by Jan 24 (Day 0). - // - W03 slot is taken by Jan 18 (Day 6). - // - Capacity is 4. So we have room for W02 and W01. - - // subWeeks(now, 1) is Jan 17 (W03). Superseded by Jan 18 (Day 6) which is newer. - // subWeeks(now, 2) is Jan 10 (W02). Should be kept. - // subWeeks(now, 3) is Jan 3 (W01). Should be kept. - // subWeeks(now, 4) is Dec 27 (W52). Dropped because 4 slots (W04, W03, W02, W01) are full. - + // 2. Verify Weekly (older unique weeks are retained additionally) expect(result.keep.map(f => f.lastModified.getTime())).toContain(subWeeks(now, 2).getTime()); expect(result.keep.map(f => f.lastModified.getTime())).toContain(subWeeks(now, 3).getTime()); - - // Verify subWeeks(now, 1) is NOT explicitly kept (it was superseded) expect(result.keep.map(f => f.lastModified.getTime())).not.toContain(subWeeks(now, 1).getTime()); // 3. Verify Monthly @@ -182,23 +169,26 @@ describe('RetentionService', () => { expect(result.keep.map(f => f.lastModified.getTime())).toContain(subMonths(now, 6).getTime()); expect(result.keep.map(f => f.lastModified.getTime())).toContain(subMonths(now, 12).getTime()); - // 4. Verify Yearly - // Slots: - // 1. 2026 (Jan 24) - // 2. 2025 (subMonths 1 - Dec 24 is newest 2025 file) - // 3. 2024 (subMonths 14 - Nov 24 is newest 2024 file) + // 4. Verify the policy keeps multiple representatives across tiers + expect(result.keep.length).toBeGreaterThan(7); + }); - // subMonths(14) is Nov 2024. It takes the 2024 slot. - expect(result.keep.map(f => f.lastModified.getTime())).toContain(subMonths(now, 14).getTime()); + it('keeps weekly representatives from older weeks when daily slots are limited', () => { + const now = new Date('2026-05-27T00:06:00Z'); + const files = createMockFiles( + Array.from({ length: 14 }, (_, i) => subDays(now, i)) + ); - // subYears(2) is Jan 2024. It is OLDER than Nov 2024. So it is dropped for the Yearly slot. - // And Monthly slots (6) are full with 2025/2026 stuff. - const deletedTimes = result.delete.map(f => f.lastModified.getTime()); - expect(deletedTimes).toContain(subYears(now, 2).getTime()); + const policy: RetentionConfiguration = { + mode: 'SMART', + smart: { daily: 1, weekly: 1, monthly: 3, yearly: 0 } + }; - // 5. Verify Deletions (Noise) - // subYears(3) is 2023. Yearly capacity 3 (26, 25, 24). So 2023 is dropped. - expect(deletedTimes).toContain(subYears(now, 3).getTime()); + const result = RetentionService.calculateRetention(files, policy); + + // Daily keeps the newest day and weekly keeps an additional older week. + expect(result.keep).toHaveLength(2); + expect(result.keep.map(f => f.lastModified.getTime())).toContain(now.getTime()); }); it('should prioritize keeping the newest backup when intervals overlap', () => {