Skip to content

Fix 404 for backup files in subdirs - #140

Open
mariovillani wants to merge 1 commit into
Laravel-Backpack:mainfrom
mariovillani:main
Open

mariovillani wants to merge 1 commit into
Laravel-Backpack:mainfrom
mariovillani:main

Conversation

@mariovillani

Copy link
Copy Markdown
Contributor

WHY

BEFORE - What was wrong? What was happening before this PR?

Previously, downloading or deleting backups would fail and return a 404 error if the backup disk was configured to store backups in subdirectories (e.g., daily/backup.zip or app-name/2023-01-01.zip). This was happening because the isValidBackupFileName() security method was overly strict. It used basename($fileName) to ensure the path didn't contain any directory components, meaning it only accepted plain zip filenames that lived at the absolute root of the backup directory.

AFTER - What is happening after this PR?

Backups nested within subdirectories can now be successfully downloaded and deleted from the Backpack admin interface. The strict anti-path traversal check has been refactored to allow valid subdirectory structures while keeping the application fully protected against actual path traversal attacks.

HOW

How did you achieve that, in technical terms?

I removed the $fileName !== basename($fileName) check in isValidBackupFileName(), which was the restriction stripping out valid subdirectories. To safely replace this and maintain strong protection against Directory Traversal attacks, I introduced explicit string checks that reject malicious path patterns. The validation now ensures the file name ends with .zip AND explicitly blocks paths that:

  • Contain .. (prevents navigating up to parent directories)
  • Contain \ (prevents Windows-style directory separators)
  • Start with / (prevents absolute path resolution from the server root)
  • Start with . (prevents targeting hidden or system files)

Is it a breaking change or non-breaking change?

Non-breaking change.

How can we test the before & after?

  1. Configure your backup destination disk to store files inside a subdirectory (e.g., set the backup path to something like app_name/ in the Spatie Backup config).
  2. Run a backup so that it appears in the Backpack BackupManager UI.
  3. Before this PR: Click the "Download" or "Delete" button on the backup row. You will receive a 404 Backup doesn't exist error.
  4. After this PR: Click the "Download" or "Delete" button. The download will successfully initialize, or the backup will be successfully deleted.
  5. (Optional Security Test): Attempt to hit the download route manually by tampering with the URL parameter (e.g., ?file_name=../.env.zip or ?file_name=/etc/passwd.zip). The system should safely reject it and return a 404.

@welcome

welcome Bot commented Sep 16, 2026

Copy link
Copy Markdown

BOOM! Your first PR with us, thank you so much! Someone will take a look at it shortly.
Please keep in mind that:

  • if this constitutes a breaking change, it might take quite a while for this to get merged; we try to emulate the Laravel release cycle as much as possible, so developers can upgrade both software once; this means a new big release every ~6 months;
  • even if it's a non-breaking change, it might take a few days/weeks for the PR to get merged; unless it's a no-brainer, we like to have some community feedback on new features, before we merge them; this leads to higher-quality code, in the end; we learnt this the hard way :-)
  • not all PRs get merged; sometimes we just have to hold out new features, to keep the packages lean; sometimes we don't include features that only apply to niche use cases;
  • we're not perfect; if you think we're wrong, call us out on it; but in a kind way :-) we all make mistakes, best we learn from them and build better software together;

Thank you!
-- Justin Case The Backpack Robot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant