Skip to content

fix(framework): Improve zip extraction robustness in flwr install#6627

Merged
panh99 merged 6 commits intomainfrom
fix-install-zip-extraction
Mar 10, 2026
Merged

fix(framework): Improve zip extraction robustness in flwr install#6627
panh99 merged 6 commits intomainfrom
fix-install-zip-extraction

Conversation

@danieljanes
Copy link
Collaborator

@danieljanes danieljanes commented Feb 25, 2026

This PR fixes a potential ZIP path traversal risk in FAB install:

  • zipf.extractall(tmpdir) was used directly in install.py (line 122)
  • In contrast, flwr new already used safe extraction with path checks in new.py (line 145)

This PR refactors both to use a common function to safely extract zip files.

Copilot AI review requested due to automatic review settings February 25, 2026 11:57
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the robustness of ZIP extraction in the Flower CLI by consolidating security checks into a reusable utility function to prevent zip-slip path traversal attacks. The refactoring extracts the previously private _safe_extract_zip function from new.py into a shared archive_utils.py module, making it available for both the flwr new and flwr install commands.

Changes:

  • Created new archive_utils.py module with safe_extract_zip function for secure ZIP extraction
  • Refactored install.py to use the centralized secure extraction function instead of unsafe zipfile.extractall()
  • Refactored new.py to use the shared utility instead of a local implementation
  • Added comprehensive test coverage for zip-slip protection in both install and new commands

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 8 comments.

Show a summary per file
File Description
framework/py/flwr/cli/archive_utils.py New module containing the centralized safe_extract_zip function to prevent zip-slip attacks
framework/py/flwr/cli/install.py Updated to use safe_extract_zip instead of unsafe zipfile.extractall()
framework/py/flwr/cli/new/new.py Removed duplicate _safe_extract_zip implementation and imports the shared utility
framework/py/flwr/cli/install_test.py New comprehensive tests for safe ZIP extraction with path traversal protection
framework/py/flwr/cli/new/new_test.py Added test for zip-slip protection in the app download flow

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

@github-actions github-actions bot added the Maintainer Used to determine what PRs (mainly) come from Flower maintainers. label Feb 25, 2026
Copy link
Contributor

@panh99 panh99 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@panh99 panh99 enabled auto-merge (squash) March 10, 2026 14:39
@panh99 panh99 merged commit f661ee0 into main Mar 10, 2026
70 checks passed
@panh99 panh99 deleted the fix-install-zip-extraction branch March 10, 2026 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Maintainer Used to determine what PRs (mainly) come from Flower maintainers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants