diff --git a/README.md b/README.md
index 2a7a6fd..beb9b70 100644
--- a/README.md
+++ b/README.md
@@ -196,9 +196,36 @@ npx mac-cleaner-cli config --show
# Manage backups
npx mac-cleaner-cli backup --list
+npx mac-cleaner-cli backup --restore
npx mac-cleaner-cli backup --clean
```
+### Backups (opt-in)
+
+By default, cleaning is **permanent**: files are removed with `rm -rf` and do
+**not** go to the Trash.
+
+You can opt into backups by setting `backupEnabled` in `~/.maccleanerrc`:
+
+```json
+{ "backupEnabled": true }
+```
+
+With backups on, selected items are **moved** to `~/.mac-cleaner-cli/backup//`
+instead of being deleted, and can be brought back with `backup --restore`.
+
+Two honest caveats:
+
+- **Moving does not free disk space.** The files still occupy the same volume.
+ Space is only reclaimed by `backup --clean`, which deletes backups older than
+ 7 days permanently.
+- **Docker and Homebrew cannot be backed up.** Their cleanup is performed by the
+ external tool (`docker system prune`, `brew cleanup`), so there is no file for
+ us to move. The CLI warns you before proceeding when these are selected.
+
+If a backup fails, the item is **not** deleted — it stays where it is and the
+failure is reported.
+
### Flags
```bash
diff --git a/src/commands/clean.test.ts b/src/commands/clean.test.ts
index 25a7109..3cdfe16 100644
--- a/src/commands/clean.test.ts
+++ b/src/commands/clean.test.ts
@@ -20,6 +20,14 @@ vi.mock('@inquirer/checkbox', () => ({
default: vi.fn(),
}));
+// loadConfig is mocked so tests never read the developer's real ~/.maccleanerrc
+// (and never hit its module-level cache). Default: backups off, which is the
+// shipped default.
+vi.mock('../utils/config.js', async () => {
+ const actual = await vi.importActual('../utils/config.js');
+ return { ...actual, loadConfig: vi.fn(async () => ({ backupEnabled: false })) };
+});
+
vi.mock('child_process', () => ({
exec: vi.fn(),
spawn: vi.fn(() => ({
@@ -32,6 +40,8 @@ const inquirerPrompts = {
checkbox: inquirerCheckbox.default,
};
+const config = await import('../utils/config.js');
+
const trashCategory: Category = {
id: 'trash',
name: 'Trash',
@@ -549,3 +559,106 @@ describe('clean command', () => {
consoleSpy.mockRestore();
});
});
+
+describe('clean command backup handling', () => {
+ // The `clean` subcommand must honour `backupEnabled` exactly like the
+ // interactive flow. Backing up in one entry point and deleting permanently in
+ // the other is worse than not backing up at all: what happens to the user's
+ // files would depend on which command they happened to type.
+ const trashScanner = () => ({
+ category: trashCategory,
+ supportsBackup: true,
+ scan: vi.fn(),
+ clean: vi.fn().mockResolvedValue({
+ category: trashCategory,
+ cleanedItems: 1,
+ freedSpace: 0,
+ backedUpSize: 1000,
+ errors: [],
+ }),
+ });
+
+ const setup = (scanner: unknown) => {
+ vi.mocked(scanners.runAllScans).mockResolvedValue({
+ results: [
+ {
+ category: trashCategory,
+ items: [{ path: '/test', size: 1000, name: 'test', isDirectory: false }],
+ totalSize: 1000,
+ },
+ ],
+ totalSize: 1000,
+ totalItems: 1,
+ });
+ vi.mocked(scanners.getScanner).mockReturnValue(
+ scanner as ReturnType
+ );
+ vi.mocked(inquirerPrompts.confirm).mockResolvedValue(true);
+ };
+
+ beforeEach(() => {
+ vi.clearAllMocks();
+ vi.mocked(config.loadConfig).mockResolvedValue({ backupEnabled: false });
+ });
+
+ it('deletes permanently when backupEnabled is off', async () => {
+ const scanner = trashScanner();
+ setup(scanner);
+ vi.mocked(config.loadConfig).mockResolvedValue({ backupEnabled: false });
+
+ const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+ await cleanCommand({ all: true, yes: true });
+
+ expect(scanner.clean).toHaveBeenCalledWith(expect.anything(), undefined, undefined);
+
+ consoleSpy.mockRestore();
+ });
+
+ it('passes a backup directory to the scanner when backupEnabled is on', async () => {
+ const scanner = trashScanner();
+ setup(scanner);
+ vi.mocked(config.loadConfig).mockResolvedValue({ backupEnabled: true });
+
+ const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+ await cleanCommand({ all: true, yes: true });
+
+ const backupDirArg = scanner.clean.mock.calls[0][2];
+ expect(backupDirArg).toEqual(expect.stringContaining('.mac-cleaner-cli'));
+
+ consoleSpy.mockRestore();
+ });
+
+ it('warns that moving does not free disk space', async () => {
+ const scanner = trashScanner();
+ setup(scanner);
+ vi.mocked(config.loadConfig).mockResolvedValue({ backupEnabled: true });
+
+ const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+ await cleanCommand({ all: true, yes: true });
+
+ const output = consoleSpy.mock.calls.flat().join('\n');
+ expect(output).toContain('does NOT free disk space');
+
+ consoleSpy.mockRestore();
+ });
+
+ it('warns which categories cannot be backed up and will be deleted', async () => {
+ const scanner = { ...trashScanner(), supportsBackup: false };
+ setup(scanner);
+ vi.mocked(config.loadConfig).mockResolvedValue({ backupEnabled: true });
+
+ const consoleSpy = vi.spyOn(console, 'log').mockImplementation(() => {});
+
+ await cleanCommand({ all: true, yes: true });
+
+ const output = consoleSpy.mock.calls.flat().join('\n');
+ expect(output).toContain('No backup possible');
+ // An external-tool scanner must never receive a backup directory.
+ expect(scanner.clean).toHaveBeenCalledWith(expect.anything(), undefined, undefined);
+
+ consoleSpy.mockRestore();
+ });
+});
diff --git a/src/commands/clean.ts b/src/commands/clean.ts
index 284c456..3b8dd7c 100644
--- a/src/commands/clean.ts
+++ b/src/commands/clean.ts
@@ -4,7 +4,7 @@ import checkbox from '@inquirer/checkbox';
import { spawn } from 'child_process';
import type { CategoryId, CleanSummary, CleanableItem, ScanResult, SafetyLevel } from '../types.js';
import { runAllScans, runScans, getScanner, getAllScanners } from '../scanners/index.js';
-import { formatSize, createScanProgress, createCleanProgress } from '../utils/index.js';
+import { formatSize, createScanProgress, createCleanProgress, loadConfig, ensureBackupDir, getBackupDir } from '../utils/index.js';
const DONATION_URL = 'https://ko-fi.com/guhcostan';
@@ -148,11 +148,45 @@ export async function cleanCommand(options: CleanCommandOptions): Promise getScanner(categoryId))
+ .filter((scanner) => scanner?.supportsBackup === false)
+ .map((scanner) => scanner.category.name);
+
+ console.log();
+ console.log(chalk.cyan(`Backup is ON — items will be MOVED to ${getBackupDir()}`));
+ console.log(
+ chalk.yellow('⚠ Moving does NOT free disk space. Run "mac-cleaner-cli backup --clean" to reclaim it.')
+ );
+ if (withoutBackup.length > 0) {
+ console.log(
+ chalk.red(
+ `⚠ No backup possible for: ${withoutBackup.join(', ')} (external tool does the cleanup) — these WILL be deleted.`
+ )
+ );
+ }
+ }
+
+ const backupDir =
+ backupEnabled && selectedItems.some(({ categoryId }) => getScanner(categoryId)?.supportsBackup !== false)
+ ? await ensureBackupDir()
+ : undefined;
+
const cleanProgress = showProgress ? createCleanProgress(selectedItems.length) : null;
const cleanResults: CleanSummary = {
results: [],
totalFreedSpace: 0,
+ totalBackedUpSize: 0,
+ backupDir,
totalCleanedItems: 0,
totalErrors: 0,
};
@@ -162,9 +196,11 @@ export async function cleanCommand(options: CleanCommandOptions): Promise = {
@@ -92,10 +92,43 @@ export async function interactiveCommand(options: InteractiveOptions = {}): Prom
const totalItems = selectedItems.reduce((sum, s) => sum + s.items.length, 0);
// Step 5: Confirm
+ // `backupEnabled` comes from ~/.maccleanerrc. Until now `loadConfig()` was
+ // only ever called by `config --show` — the file `config --init` wrote had no
+ // effect whatsoever on cleaning.
+ const config = await loadConfig();
+ const backupEnabled = config.backupEnabled === true;
+
+ const categoriesWithoutBackup = backupEnabled
+ ? selectedItems
+ .map(({ categoryId }) => getScanner(categoryId))
+ .filter((scanner) => scanner?.supportsBackup === false)
+ .map((scanner) => scanner.category.name)
+ : [];
+
console.log();
console.log(chalk.bold('Summary:'));
console.log(` Items to delete: ${chalk.yellow(totalItems.toString())}`);
console.log(` Space to free: ${chalk.green(formatSize(totalToClean))}`);
+
+ if (backupEnabled) {
+ console.log();
+ console.log(chalk.cyan(` Backup is ON — items will be MOVED to ${getBackupDir()}`));
+ // The awkward part, said plainly: moving does not free space. Without this,
+ // the final report ("0 B freed") would look like a bug.
+ console.log(
+ chalk.yellow(
+ ` ⚠ Moving does NOT free disk space. Run "mac-cleaner-cli backup --clean" to reclaim it.`
+ )
+ );
+ if (categoriesWithoutBackup.length > 0) {
+ console.log(
+ chalk.red(
+ ` ⚠ No backup possible for: ${categoriesWithoutBackup.join(', ')} (external tool does the cleanup) — these WILL be deleted.`
+ )
+ );
+ }
+ }
+
console.log();
const proceed = await confirm({
@@ -109,11 +142,21 @@ export async function interactiveCommand(options: InteractiveOptions = {}): Prom
}
// Step 6: Clean
+ // The backup directory is created ONCE per run, and only if some scanner in
+ // this run can actually back up — otherwise we would leave empty timestamped
+ // folders behind.
+ const backupDir =
+ backupEnabled && selectedItems.some(({ categoryId }) => getScanner(categoryId)?.supportsBackup !== false)
+ ? await ensureBackupDir()
+ : undefined;
+
const cleanProgress = showProgress ? createCleanProgress(selectedItems.length) : null;
const cleanResults: CleanSummary = {
results: [],
totalFreedSpace: 0,
+ totalBackedUpSize: 0,
+ backupDir,
totalCleanedItems: 0,
totalErrors: 0,
};
@@ -123,9 +166,11 @@ export async function interactiveCommand(options: InteractiveOptions = {}): Prom
const scanner = getScanner(categoryId);
cleanProgress?.update(cleanedCount, `Cleaning ${scanner.category.name}...`);
- const result = await scanner.clean(items);
+ const scannerBackupDir = scanner.supportsBackup === false ? undefined : backupDir;
+ const result = await scanner.clean(items, false, scannerBackupDir);
cleanResults.results.push(result);
cleanResults.totalFreedSpace += result.freedSpace;
+ cleanResults.totalBackedUpSize = (cleanResults.totalBackedUpSize ?? 0) + (result.backedUpSize ?? 0);
cleanResults.totalCleanedItems += result.cleanedItems;
cleanResults.totalErrors += result.errors.length;
cleanedCount++;
@@ -192,8 +237,11 @@ function printCleanResults(summary: CleanSummary): void {
for (const result of summary.results) {
if (result.cleanedItems > 0) {
+ const backedUp = (result.backedUpSize ?? 0) > 0;
console.log(
- ` ${result.category.name.padEnd(30)} ${chalk.green('✓')} ${formatSize(result.freedSpace)} freed`
+ backedUp
+ ? ` ${result.category.name.padEnd(30)} ${chalk.cyan('↦')} ${formatSize(result.backedUpSize ?? 0)} moved to backup`
+ : ` ${result.category.name.padEnd(30)} ${chalk.green('✓')} ${formatSize(result.freedSpace)} freed`
);
}
for (const error of result.errors) {
@@ -206,6 +254,16 @@ function printCleanResults(summary: CleanSummary): void {
console.log(chalk.bold(`🎉 Freed ${chalk.green(formatSize(summary.totalFreedSpace))} of disk space!`));
console.log(chalk.dim(` Cleaned ${summary.totalCleanedItems} items`));
+ if ((summary.totalBackedUpSize ?? 0) > 0) {
+ console.log();
+ console.log(
+ chalk.cyan(` ${formatSize(summary.totalBackedUpSize ?? 0)} moved to backup (still on disk):`)
+ );
+ console.log(chalk.dim(` ${summary.backupDir}`));
+ console.log(chalk.dim(` restore: mac-cleaner-cli backup --restore "${summary.backupDir}"`));
+ console.log(chalk.dim(` reclaim: mac-cleaner-cli backup --clean`));
+ }
+
if (summary.totalErrors > 0) {
console.log(chalk.red(` Errors: ${summary.totalErrors}`));
}
diff --git a/src/index.ts b/src/index.ts
index bd03dff..c0d2d31 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -3,7 +3,7 @@
import { Command, InvalidArgumentError } from 'commander';
import { ExitPromptError } from '@inquirer/core';
import { cleanCommand, interactiveCommand, listCategories, maintenanceCommand, scanCommand, uninstallCommand } from './commands/index.js';
-import { initConfig, configExists, listBackups, cleanOldBackups, loadConfig, formatSize } from './utils/index.js';
+import { initConfig, configExists, listBackups, cleanOldBackups, restoreBackup, loadConfig, formatSize } from './utils/index.js';
import { CATEGORIES, type CategoryId } from './types.js';
import pkg from '../package.json' with { type: 'json' };
@@ -183,12 +183,13 @@ program
.command('backup')
.description('Manage backups')
.option('--list', 'List all backups')
- .option('--clean', 'Clean old backups (older than 7 days)')
+ .option('--restore ', 'Restore a backup back to its original locations')
+ .option('--clean', 'Delete old backups permanently (older than 7 days), reclaiming disk space')
.action(async (options) => {
if (options.list) {
const backups = await listBackups();
if (backups.length === 0) {
- console.log('No backups found.');
+ console.log('No backups found. Backups only happen when "backupEnabled": true is set in ~/.maccleanerrc');
return;
}
console.log('\nBackups:');
@@ -196,6 +197,20 @@ program
console.log(` ${backup.date.toLocaleDateString()} - ${formatSize(backup.size)}`);
console.log(` ${backup.path}`);
}
+ console.log('\nRestore with: mac-cleaner-cli backup --restore ');
+ return;
+ }
+
+ if (options.restore) {
+ const result = await restoreBackup(options.restore);
+ console.log(`Restored ${result.success} items.`);
+ if (result.failed > 0) {
+ console.log(`Failed: ${result.failed}`);
+ for (const error of result.errors) {
+ console.log(` ✗ ${error}`);
+ }
+ process.exitCode = 1;
+ }
return;
}
@@ -205,7 +220,7 @@ program
return;
}
- console.log('Use --list to show backups or --clean to remove old ones.');
+ console.log('Use --list to show backups, --restore to bring one back, or --clean to remove old ones.');
});
program.parse();
diff --git a/src/scanners/base-scanner.test.ts b/src/scanners/base-scanner.test.ts
index 0a9070b..1ae02c6 100644
--- a/src/scanners/base-scanner.test.ts
+++ b/src/scanners/base-scanner.test.ts
@@ -1,9 +1,12 @@
-import { describe, it, expect, beforeEach, afterEach } from 'vitest';
-import { mkdtemp, writeFile, rm } from 'fs/promises';
+import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
+import { mkdtemp, writeFile, rm, mkdir } from 'fs/promises';
+import { existsSync } from 'fs';
import { join } from 'path';
-import { tmpdir } from 'os';
+import { tmpdir, homedir } from 'os';
import { BaseScanner } from './base-scanner.js';
+import { CATEGORIES } from '../types.js';
import type { Category, ScanResult, ScannerOptions, CleanableItem } from '../types.js';
+import { ensureBackupDir } from '../utils/backup.js';
class TestScanner extends BaseScanner {
category: Category = {
@@ -117,3 +120,81 @@ describe('BaseScanner', () => {
});
});
+
+describe('BaseScanner backup routing', () => {
+ class FakeScanner extends BaseScanner {
+ category = CATEGORIES['trash'];
+ async scan() {
+ return this.createResult([]);
+ }
+ }
+
+ let sourceDir: string;
+
+ beforeEach(async () => {
+ sourceDir = join(homedir(), '.mac-cleaner-cli-test-scanner');
+ await mkdir(sourceDir, { recursive: true });
+ });
+
+ afterEach(async () => {
+ await rm(sourceDir, { recursive: true, force: true });
+ });
+
+ it('deletes when no backupDir is given', async () => {
+ const file = join(sourceDir, 'delete-me.txt');
+ await writeFile(file, 'x');
+
+ const scanner = new FakeScanner();
+ const result = await scanner.clean(
+ [{ path: file, size: 1, name: 'delete-me.txt', isDirectory: false }]
+ );
+
+ expect(existsSync(file)).toBe(false);
+ expect(result.freedSpace).toBe(1);
+ expect(result.backedUpSize).toBeUndefined();
+ });
+
+ it('moves to backup and reports ZERO freed space when backupDir is given', async () => {
+ const file = join(sourceDir, 'keep-me.txt');
+ await writeFile(file, 'x');
+
+ const backupDir = await ensureBackupDir();
+ const scanner = new FakeScanner();
+ const result = await scanner.clean(
+ [{ path: file, size: 1, name: 'keep-me.txt', isDirectory: false }],
+ false,
+ backupDir
+ );
+
+ // The point of this test: moving does NOT free space. Reporting freedSpace
+ // here would trade the old facade for a new untruth.
+ expect(result.freedSpace).toBe(0);
+ expect(result.backedUpSize).toBe(1);
+ expect(result.backupDir).toBe(backupDir);
+ expect(existsSync(file)).toBe(false);
+
+ await rm(backupDir, { recursive: true, force: true });
+ });
+
+ it('reports an error and keeps the file when the backup fails', async () => {
+ const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+ const outside = join(tmpdir(), 'scanner-outside-home.txt');
+ await writeFile(outside, 'x');
+
+ const backupDir = await ensureBackupDir();
+ const scanner = new FakeScanner();
+ const result = await scanner.clean(
+ [{ path: outside, size: 1, name: 'scanner-outside-home.txt', isDirectory: false }],
+ false,
+ backupDir
+ );
+
+ expect(result.cleanedItems).toBe(0);
+ expect(result.errors[0]).toContain('nothing was deleted');
+ expect(existsSync(outside)).toBe(true);
+
+ await rm(outside, { force: true });
+ await rm(backupDir, { recursive: true, force: true });
+ consoleSpy.mockRestore();
+ });
+});
diff --git a/src/scanners/base-scanner.ts b/src/scanners/base-scanner.ts
index c565af3..cee92d0 100644
--- a/src/scanners/base-scanner.ts
+++ b/src/scanners/base-scanner.ts
@@ -1,12 +1,35 @@
import type { Scanner, Category, ScanResult, CleanResult, CleanableItem, ScannerOptions } from '../types.js';
import { removeItems } from '../utils/fs.js';
+import { backupItems } from '../utils/backup.js';
export abstract class BaseScanner implements Scanner {
abstract category: Category;
+ /**
+ * Scanners that delete files can back them up first. Scanners that delegate
+ * cleanup to an external tool (Docker, Homebrew) override this with `false`:
+ * there is no file of ours to move, and promising a backup there would repeat
+ * the very problem this module exists to fix.
+ */
+ readonly supportsBackup: boolean = true;
+
abstract scan(options?: ScannerOptions): Promise;
- async clean(items: CleanableItem[], dryRun = false): Promise {
+ async clean(items: CleanableItem[], dryRun = false, backupDir?: string): Promise {
+ if (backupDir) {
+ const result = await backupItems(items, backupDir, dryRun);
+
+ return {
+ category: this.category,
+ cleanedItems: result.success,
+ // Nothing was freed: the files are still on disk, inside the backup.
+ freedSpace: 0,
+ backedUpSize: result.backedUpSize,
+ backupDir,
+ errors: result.failed > 0 ? [`Failed to back up ${result.failed} items (nothing was deleted for those)`] : [],
+ };
+ }
+
const result = await removeItems(items, dryRun);
const errors: string[] = [];
@@ -42,10 +65,3 @@ export abstract class BaseScanner implements Scanner {
};
}
}
-
-
-
-
-
-
-
diff --git a/src/scanners/docker.ts b/src/scanners/docker.ts
index 6bd8c30..fc60821 100644
--- a/src/scanners/docker.ts
+++ b/src/scanners/docker.ts
@@ -69,6 +69,11 @@ function execCommand(command: string, args: string[]): Promise {
}
export class DockerScanner extends BaseScanner {
+ // Cleanup here is `docker system prune`: the external tool does the deleting,
+ // not us. There is no file to move, so backup does not apply — and pretending
+ // it does would be the same facade as before.
+ readonly supportsBackup = false;
+
category = CATEGORIES['docker'];
private dockerPath: string | null = null;
diff --git a/src/scanners/homebrew.ts b/src/scanners/homebrew.ts
index 8646e6a..97db459 100644
--- a/src/scanners/homebrew.ts
+++ b/src/scanners/homebrew.ts
@@ -75,6 +75,11 @@ function execCommand(command: string, args: string[]): Promise {
}
export class HomebrewScanner extends BaseScanner {
+ // Cleanup here is `brew cleanup`: the external tool does the deleting, not us.
+ // There is no file to move, so backup does not apply — and pretending it does
+ // would be the same facade as before.
+ readonly supportsBackup = false;
+
category = CATEGORIES['homebrew'];
private brewPath: string | null = null;
diff --git a/src/types.ts b/src/types.ts
index 1e01b06..6dbdfeb 100644
--- a/src/types.ts
+++ b/src/types.ts
@@ -56,13 +56,19 @@ export interface ScanSummary {
export interface CleanResult {
category: Category;
cleanedItems: number;
+ /** Space actually returned to the disk. Zero when items went to the backup. */
freedSpace: number;
+ /** Size moved into the backup. Still occupying disk until `backup --clean`. */
+ backedUpSize?: number;
+ backupDir?: string;
errors: string[];
}
export interface CleanSummary {
results: CleanResult[];
totalFreedSpace: number;
+ totalBackedUpSize?: number;
+ backupDir?: string;
totalCleanedItems: number;
totalErrors: number;
}
@@ -75,8 +81,10 @@ export interface ScannerOptions {
export interface Scanner {
category: Category;
+ /** false when cleanup is delegated to an external tool and cannot be backed up. */
+ readonly supportsBackup?: boolean;
scan(options?: ScannerOptions): Promise;
- clean(items: CleanableItem[], dryRun?: boolean): Promise;
+ clean(items: CleanableItem[], dryRun?: boolean, backupDir?: string): Promise;
}
export const CATEGORIES: Record = {
diff --git a/src/utils/backup.test.ts b/src/utils/backup.test.ts
index 1f4f2d7..421d5af 100644
--- a/src/utils/backup.test.ts
+++ b/src/utils/backup.test.ts
@@ -1,7 +1,8 @@
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
-import { mkdir, mkdtemp, rm, writeFile } from 'fs/promises';
+import { mkdir, mkdtemp, rm, writeFile, readFile } from 'fs/promises';
+import { existsSync } from 'fs';
import { join } from 'path';
-import { tmpdir } from 'os';
+import { tmpdir, homedir } from 'os';
import * as backup from './backup.js';
describe('backup utilities', () => {
@@ -46,79 +47,171 @@ describe('backup utilities', () => {
});
});
- describe('backupItem', () => {
- it('should return false for non-existent item', async () => {
+ describe('backupItem / backupItems (round-trip real)', () => {
+ // These move REAL files and check the content afterwards. The previous
+ // version only asserted `expect(typeof result).toBe('boolean')`, which
+ // passed even with the backup failing 100% of the time — which was exactly
+ // the state of this module: no callers, never exercised.
+ let sourceDir: string;
+
+ beforeEach(async () => {
+ // Must be INSIDE home: backupItem refuses anything else, because
+ // restoreBackup only knows how to restore what sits under the HOME/ prefix.
+ sourceDir = join(homedir(), '.mac-cleaner-cli-test-src');
+ await mkdir(sourceDir, { recursive: true });
+ });
+
+ afterEach(async () => {
+ await rm(sourceDir, { recursive: true, force: true });
+ });
+
+ it('should MOVE the file into the backup, not copy it', async () => {
+ const testFile = join(sourceDir, 'moved.txt');
+ await writeFile(testFile, 'conteudo original');
+
const dir = await backup.ensureBackupDir();
- const result = await backup.backupItem(
- { path: '/non/existent/file.txt', size: 0, name: 'file.txt', isDirectory: false },
+ const ok = await backup.backupItem(
+ { path: testFile, size: 17, name: 'moved.txt', isDirectory: false },
dir
);
- expect(result).toBe(false);
+
+ expect(ok).toBe(true);
+ expect(existsSync(testFile)).toBe(false);
+
+ const backedUpPath = backup.backupPathFor(testFile, dir);
+ expect(backedUpPath).not.toBeNull();
+ expect(await readFile(backedUpPath as string, 'utf-8')).toBe('conteudo original');
+
await rm(dir, { recursive: true, force: true });
});
- it('should backup existing file', async () => {
- const testFile = join(testBackupDir, 'test-backup.txt');
- await writeFile(testFile, 'test content');
+ it('should refuse paths outside the home directory', async () => {
+ const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+ const outside = join(tmpdir(), 'outside-home.txt');
+ await writeFile(outside, 'x');
const dir = await backup.ensureBackupDir();
- const result = await backup.backupItem(
- { path: testFile, size: 12, name: 'test-backup.txt', isDirectory: false },
+ const ok = await backup.backupItem(
+ { path: outside, size: 1, name: 'outside-home.txt', isDirectory: false },
dir
);
- expect(typeof result).toBe('boolean');
+ // Refuses, rather than creating a backup restoreBackup could not undo.
+ expect(ok).toBe(false);
+ expect(existsSync(outside)).toBe(true);
+
+ await rm(outside, { force: true });
await rm(dir, { recursive: true, force: true });
+ consoleSpy.mockRestore();
});
- });
- describe('backupItems', () => {
- it('should handle empty items array', async () => {
- const result = await backup.backupItems([]);
+ it('should refuse protected system paths', async () => {
+ const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+ const dir = await backup.ensureBackupDir();
- expect(result.success).toBe(0);
- expect(result.failed).toBe(0);
- await rm(result.backupDir, { recursive: true, force: true });
+ const ok = await backup.backupItem(
+ { path: '/var/log/system.log', size: 1, name: 'system.log', isDirectory: false },
+ dir
+ );
+
+ expect(ok).toBe(false);
+ await rm(dir, { recursive: true, force: true });
+ consoleSpy.mockRestore();
});
- it('should backup multiple items', async () => {
- const testFile = join(testBackupDir, 'test.txt');
- await writeFile(testFile, 'test content');
+ it('should not touch the disk in dry run but still report the item', async () => {
+ const testFile = join(sourceDir, 'dry.txt');
+ await writeFile(testFile, 'intacto');
+
+ const dir = await backup.ensureBackupDir();
+ const result = await backup.backupItems(
+ [{ path: testFile, size: 7, name: 'dry.txt', isDirectory: false }],
+ dir,
+ true
+ );
- const result = await backup.backupItems([
- { path: testFile, size: 12, name: 'test.txt', isDirectory: false },
- ]);
+ expect(result.success).toBe(1);
+ expect(result.backedUpSize).toBe(7);
+ expect(await readFile(testFile, 'utf-8')).toBe('intacto');
- expect(result.backupDir).toBeDefined();
- await rm(result.backupDir, { recursive: true, force: true });
+ await rm(dir, { recursive: true, force: true });
});
- it('should call progress callback', async () => {
- const testFile = join(testBackupDir, 'test2.txt');
- await writeFile(testFile, 'test content');
+ it('should report backedUpSize separately from freed space', async () => {
+ const testFile = join(sourceDir, 'sized.txt');
+ await writeFile(testFile, 'abcdefghij');
- const progressFn = vi.fn();
+ const dir = await backup.ensureBackupDir();
+ const result = await backup.backupItems(
+ [{ path: testFile, size: 10, name: 'sized.txt', isDirectory: false }],
+ dir
+ );
+ expect(result.success).toBe(1);
+ expect(result.backedUpSize).toBe(10);
+
+ await rm(dir, { recursive: true, force: true });
+ });
+
+ it('should count failures without deleting the failed item', async () => {
+ const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {});
+ const good = join(sourceDir, 'good.txt');
+ await writeFile(good, 'ok');
+
+ const dir = await backup.ensureBackupDir();
const result = await backup.backupItems(
- [{ path: testFile, size: 12, name: 'test2.txt', isDirectory: false }],
+ [
+ { path: good, size: 2, name: 'good.txt', isDirectory: false },
+ { path: join(sourceDir, 'ghost.txt'), size: 0, name: 'ghost.txt', isDirectory: false },
+ ],
+ dir
+ );
+
+ expect(result.success).toBe(1);
+ expect(result.failed).toBe(1);
+
+ await rm(dir, { recursive: true, force: true });
+ consoleSpy.mockRestore();
+ });
+
+ it('should call the progress callback', async () => {
+ const testFile = join(sourceDir, 'progress.txt');
+ await writeFile(testFile, 'p');
+ const progressFn = vi.fn();
+
+ const dir = await backup.ensureBackupDir();
+ await backup.backupItems(
+ [{ path: testFile, size: 1, name: 'progress.txt', isDirectory: false }],
+ dir,
+ false,
progressFn
);
expect(progressFn).toHaveBeenCalled();
- await rm(result.backupDir, { recursive: true, force: true });
+ await rm(dir, { recursive: true, force: true });
});
- it('should count successes and failures', async () => {
- const testFile = join(testBackupDir, 'success.txt');
- await writeFile(testFile, 'test');
+ it('should survive a full backup -> restore round trip with identical content', async () => {
+ const testFile = join(sourceDir, 'roundtrip.txt');
+ const content = 'este conteudo precisa voltar identico';
+ await writeFile(testFile, content);
+
+ const dir = await backup.ensureBackupDir();
+ expect(
+ await backup.backupItem(
+ { path: testFile, size: content.length, name: 'roundtrip.txt', isDirectory: false },
+ dir
+ )
+ ).toBe(true);
+ expect(existsSync(testFile)).toBe(false);
+
+ const restored = await backup.restoreBackup(dir);
- const result = await backup.backupItems([
- { path: testFile, size: 4, name: 'success.txt', isDirectory: false },
- { path: '/non/existent.txt', size: 0, name: 'fail.txt', isDirectory: false },
- ]);
+ expect(restored.failed).toBe(0);
+ expect(restored.success).toBe(1);
+ expect(await readFile(testFile, 'utf-8')).toBe(content);
- expect(result.success + result.failed).toBe(2);
- await rm(result.backupDir, { recursive: true, force: true });
+ await rm(dir, { recursive: true, force: true });
});
});
@@ -187,3 +280,20 @@ describe('backup utilities', () => {
});
});
});
+
+describe('ensureBackupDir uniqueness', () => {
+ // Regression guard: the name was just the ISO timestamp (millisecond
+ // resolution) and `mkdir` with `recursive: true` accepts an existing directory
+ // silently. Two sessions in the same millisecond would share the folder and
+ // one would overwrite the other. This surfaced as suite flakiness (vitest runs
+ // files in parallel) before it could surface as a production bug.
+ it('never hands out the same directory twice', async () => {
+ const dirs = await Promise.all(
+ Array.from({ length: 25 }, () => backup.ensureBackupDir())
+ );
+
+ expect(new Set(dirs).size).toBe(dirs.length);
+
+ await Promise.all(dirs.map((d) => rm(d, { recursive: true, force: true })));
+ });
+});
diff --git a/src/utils/backup.ts b/src/utils/backup.ts
index 8b879ae..5c3ad7e 100644
--- a/src/utils/backup.ts
+++ b/src/utils/backup.ts
@@ -1,7 +1,8 @@
-import { mkdir, rename, readdir, stat, rm } from 'fs/promises';
+import { mkdir, rename, readdir, stat, rm, cp } from 'fs/promises';
import { join, dirname, resolve, relative } from 'path';
import { homedir } from 'os';
import type { CleanableItem } from '../types.js';
+import { validatePathSafety } from './fs.js';
const BACKUP_DIR = join(homedir(), '.mac-cleaner-cli', 'backup');
const BACKUP_RETENTION_DAYS = 7;
@@ -13,60 +14,161 @@ const BACKUP_RETENTION_DAYS = 7;
function validateRestorePath(targetPath: string): string | null {
const home = homedir();
const resolved = resolve(targetPath);
-
+
// Ensure the resolved path is within the home directory
if (!resolved.startsWith(home + '/') && resolved !== home) {
return `Path traversal detected: ${targetPath} resolves outside home directory`;
}
-
+
// Check for suspicious patterns that might indicate an attack
if (targetPath.includes('..')) {
return `Suspicious path pattern detected: ${targetPath}`;
}
-
+
return null;
}
+/**
+ * Creates this session's backup directory.
+ *
+ * The name is an ISO timestamp, which has millisecond resolution — two sessions
+ * started in the same millisecond would land in the SAME directory and one could
+ * overwrite the other. `mkdir` with `recursive: true` does not report that: it
+ * accepts an existing directory silently. Hence the incremental suffix and the
+ * non-recursive `mkdir` on the last level, which is what makes creation
+ * exclusive.
+ */
export async function ensureBackupDir(): Promise {
const timestamp = new Date().toISOString().replace(/[:.]/g, '-');
- const sessionDir = join(BACKUP_DIR, timestamp);
- await mkdir(sessionDir, { recursive: true });
- return sessionDir;
+
+ await mkdir(BACKUP_DIR, { recursive: true });
+
+ for (let attempt = 0; ; attempt++) {
+ const sessionDir = join(BACKUP_DIR, attempt === 0 ? timestamp : `${timestamp}-${attempt}`);
+ try {
+ await mkdir(sessionDir);
+ return sessionDir;
+ } catch (error) {
+ if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error;
+ }
+ }
+}
+
+/**
+ * Maps an original path to its path inside the backup.
+ *
+ * Only paths INSIDE the home directory are accepted, for two reasons:
+ *
+ * 1. `restoreBackup()` only knows how to restore what sits under the `HOME/`
+ * prefix — accepting anything else would create a backup that cannot be
+ * restored.
+ * 2. The previous implementation used `item.path.replace(homedir(), 'HOME')`,
+ * which replaces the FIRST occurrence anywhere in the string and, for a path
+ * outside home, returned an absolute path — making `join(backupDir, '/x')`
+ * land outside the backup directory entirely.
+ *
+ * Returns null when the path cannot be backed up safely.
+ */
+export function backupPathFor(originalPath: string, backupDir: string): string | null {
+ const home = homedir();
+ const resolved = resolve(originalPath);
+
+ if (!resolved.startsWith(home + '/')) {
+ return null;
+ }
+
+ const relativeToHome = relative(home, resolved);
+ if (!relativeToHome || relativeToHome.startsWith('..')) {
+ return null;
+ }
+
+ return join(backupDir, 'HOME', relativeToHome);
}
+/**
+ * Moves an item into the backup directory instead of deleting it.
+ *
+ * `rename` first (instant, same volume) with a copy-then-remove fallback,
+ * because `rename` fails with EXDEV when the item lives on another volume — a
+ * real case for `/Volumes/*` and some caches.
+ *
+ * Safety contract: if the backup fails, the item is NOT deleted. Callers count
+ * that as a failure, never as a silent success.
+ */
export async function backupItem(item: CleanableItem, backupDir: string): Promise {
+ const safetyError = validatePathSafety(item.path);
+ if (safetyError) {
+ console.error(safetyError);
+ return false;
+ }
+
+ const backupPath = backupPathFor(item.path, backupDir);
+ if (!backupPath) {
+ console.error(`Cannot back up path outside home directory: ${item.path}`);
+ return false;
+ }
+
try {
- const relativePath = item.path.replace(homedir(), 'HOME');
- const backupPath = join(backupDir, relativePath);
await mkdir(dirname(backupPath), { recursive: true });
await rename(item.path, backupPath);
return true;
+ } catch (error) {
+ if ((error as NodeJS.ErrnoException).code !== 'EXDEV') {
+ return false;
+ }
+ }
+
+ // Different volume: copy preserving symlinks (never following the target),
+ // and only then remove the original.
+ try {
+ await cp(item.path, backupPath, {
+ recursive: true,
+ verbatimSymlinks: true,
+ errorOnExist: false,
+ force: true,
+ });
+ await rm(item.path, { recursive: true, force: true });
+ return true;
} catch {
return false;
}
}
+/**
+ * Moves every item into `backupDir` instead of deleting them.
+ *
+ * `backedUpSize` is reported separately from "freed space" on purpose: moving a
+ * file to another folder on the SAME disk does not free a single byte. The space
+ * only appears after `mac-cleaner-cli backup --clean`. Calling it "freed" would
+ * trade one untruth (a backup that never happened) for another.
+ */
export async function backupItems(
items: CleanableItem[],
+ backupDir: string,
+ dryRun = false,
onProgress?: (current: number, total: number, item: CleanableItem) => void
-): Promise<{ backupDir: string; success: number; failed: number }> {
- const backupDir = await ensureBackupDir();
+): Promise<{ success: number; failed: number; backedUpSize: number }> {
let success = 0;
let failed = 0;
+ let backedUpSize = 0;
for (let i = 0; i < items.length; i++) {
const item = items[i];
onProgress?.(i + 1, items.length, item);
- const backed = await backupItem(item, backupDir);
+ const backed = dryRun
+ ? backupPathFor(item.path, backupDir) !== null
+ : await backupItem(item, backupDir);
+
if (backed) {
success++;
+ backedUpSize += item.size;
} else {
failed++;
}
}
- return { backupDir, success, failed };
+ return { success, failed, backedUpSize };
}
export async function cleanOldBackups(): Promise {
@@ -157,13 +259,18 @@ export async function restoreBackup(backupDir: string): Promise<{ success: numbe
const errors: string[] = [];
const home = homedir();
- // Validate that backupDir is within our expected backup location
+ // Validate that backupDir is within our expected backup location.
+ // The `+ '/'` matters: without it, `~/.mac-cleaner-cli/backup-anything`
+ // passed the check simply by being a textual prefix.
const resolvedBackupDir = resolve(backupDir);
- if (!resolvedBackupDir.startsWith(BACKUP_DIR)) {
- return {
- success: 0,
- failed: 1,
- errors: ['Invalid backup directory: must be within the mac-cleaner-cli backup folder']
+ if (
+ resolvedBackupDir !== BACKUP_DIR &&
+ !resolvedBackupDir.startsWith(BACKUP_DIR + '/')
+ ) {
+ return {
+ success: 0,
+ failed: 1,
+ errors: ['Invalid backup directory: must be within the mac-cleaner-cli backup folder']
};
}
@@ -186,7 +293,7 @@ export async function restoreBackup(backupDir: string): Promise<{ success: numbe
} else {
// Compute the relative path from backup directory
const relFromBackup = relative(resolvedBackupDir, entryPath);
-
+
// Replace HOME prefix with actual home directory
// Use a more secure replacement that only matches at the start
let targetPath: string;
@@ -201,7 +308,7 @@ export async function restoreBackup(backupDir: string): Promise<{ success: numbe
failed++;
continue;
}
-
+
// Validate the target path to prevent path traversal attacks
const validationError = validateRestorePath(targetPath);
if (validationError) {
@@ -230,4 +337,3 @@ export async function restoreBackup(backupDir: string): Promise<{ success: numbe
export function getBackupDir(): string {
return BACKUP_DIR;
}
-
diff --git a/vitest.config.ts b/vitest.config.ts
index 5993b82..01ab2d5 100644
--- a/vitest.config.ts
+++ b/vitest.config.ts
@@ -16,7 +16,9 @@ export default defineConfig({
'**/index.ts',
'src/scanners/duplicates.ts',
'src/scanners/node-modules.ts',
- 'src/utils/backup.ts',
+ // src/utils/backup.ts left this list: it is no longer dead code and now
+ // runs on the deletion path. A module that can lose a user's file does
+ // not sit outside the coverage count.
'src/utils/checkbox.ts',
],
thresholds: {