Correctly sanitize requested backup files

[giuscris]

This pull request enhances the security and reliability of backup file management in the BackupController by improving path handling and validation, and by introducing new utility methods in the Path class. The main focus is to prevent directory traversal vulnerabilities and ensure that only valid backup files within the configured backup directory can be accessed, downloaded, or deleted.

Backup file path handling and validation:

• In BackupController, file operations for download and delete now ensure the resolved file is within the normalized backup path by checking Path::dirname($file) === $backupPath, preventing directory traversal attacks. Errors now throw TranslatedException for better error messaging. [1] [2]
• The filename returned after making a backup now uses Path::basename instead of the built-in basename for more robust path handling.

Utility methods in Path class:

• Added Path::dirname and Path::basename methods to provide consistent, cross-platform path manipulation, replacing direct usage of PHP’s dirname and basename.

Imports update:

• Updated imports in BackupController.php to use the new Path utility class.

[Copilot]

Pull request overview

This pull request tightens backup file handling in BackupController to reduce directory traversal risk by validating that requested backup files resolve inside the configured backup directory, and introduces Path::dirname() / Path::basename() utilities to centralize path manipulation behavior.

Changes:

• Added Path::dirname() and Path::basename() helpers for separator-agnostic path operations.
• Updated backup creation to use Path::basename() when returning the generated backup filename.
• Updated backup download/delete to validate resolved paths and to raise TranslatedException for invalid filenames.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
formwork/src/Utils/Path.php	Adds dirname() / basename() helpers to standardize path component extraction.
formwork/src/Panel/Controllers/BackupController.php	Uses the new Path helpers and strengthens validation/error handling for download/delete operations.

Suppressed comments (1)

formwork/src/Panel/Controllers/BackupController.php:81

• Same issue as in download(): strict Path::dirname($file) === $backupPath breaks when system.backup.path has a trailing /, and the decoded route param should be strict-base64 decoded + validated as a single filename component before joining paths.

            $backupPath = Path::normalize($this->config->getString('system.backup.path'));
            $file = FileSystem::joinPaths($backupPath, base64_decode((string) $routeParams->get('backup')));
            if (Path::dirname($file) === $backupPath && FileSystem::isFile($file, assertExists: false)) {
                FileSystem::delete($file);
                $this->panel->notify($this->translate('panel.backup.deleted'), 'success');
                return $this->redirectToReferer(default: $this->generateRoute('panel.tools.backups'), base: $this->generateRoute('panel.index'));
            }
            throw new TranslatedException('Invalid backup filename', 'panel.backup.error.cannotDelete.invalidFilename');

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.