feat(extensions): verify catalog archive sha256 before install#3080
Open
zied-jlassi wants to merge 1 commit into
Open
feat(extensions): verify catalog archive sha256 before install#3080zied-jlassi wants to merge 1 commit into
zied-jlassi wants to merge 1 commit into
Conversation
Extension and preset archives were downloaded over HTTPS and unpacked (with Zip-Slip protection) but their bytes were never checked against a known digest. Trust rested entirely on TLS and the integrity of the release host, so a tampered or swapped archive from a compromised third-party release would be installed silently. Maintainers do not audit extension code, so consumer-side integrity is the only available defence. Catalog entries may now pin an optional `sha256` digest. When present, the downloaded archive is verified before it is written to disk and installed; a mismatch aborts with a clear error. Entries without `sha256` keep working unchanged (a DEBUG line records that the download was unverified), so the change is backwards compatible. The check runs on both download paths (extensions and presets) via a single shared helper so the two stay in parity. - Add `verify_archive_sha256` helper in shared_infra (digest match, `sha256:` prefix, case-insensitive; DEBUG log when no digest declared) - Enforce it in ExtensionCatalog.download_extension and PresetCatalog.download_pack, before the archive is written to disk - Document the optional `sha256` field in the publishing guides - Tests: helper unit tests + matching/mismatch/no-digest on both paths Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Assisted-by: AI
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because no GitHub Actions runner was available. Make sure your repository has a runner available to run Copilot's review, or add a copilot-setup-steps.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds optional SHA-256 pinning for extension/preset archives to harden installs against tampered or swapped downloads by verifying downloaded bytes before writing/installing.
Changes:
- Introduces
shared_infra.verify_archive_sha256and integrates it into both extension and preset download paths. - Adds unit + integration-style tests covering match/mismatch/prefix/case/no-digest behaviors.
- Documents the optional
sha256field in both publishing guides.
Show a summary per file
| File | Description |
|---|---|
src/specify_cli/shared_infra.py |
Adds shared SHA-256 verification helper with debug logging for unpinned downloads |
src/specify_cli/extensions.py |
Verifies extension archive bytes against optional catalog sha256 before writing |
src/specify_cli/presets/__init__.py |
Verifies preset pack archive bytes against optional catalog sha256 before writing |
tests/test_shared_infra_integrity.py |
Unit tests for helper behavior (match/mismatch/prefix/case/absent digest) |
tests/test_extensions.py |
Download-path tests for extensions with matching/mismatching/absent sha256 |
tests/test_presets.py |
Download-path tests for presets with matching/mismatching sha256 |
extensions/EXTENSION-PUBLISHING-GUIDE.md |
Documents optional sha256 field for extension catalog entries |
presets/PUBLISHING.md |
Documents optional sha256 field for preset catalog entries |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 8/8 changed files
- Comments generated: 4
Comment on lines
+3701
to
+3702
| resp.__enter__ = lambda s: s | ||
| resp.__exit__ = MagicMock(return_value=False) |
Comment on lines
+2008
to
+2009
| resp.__enter__ = lambda s: s | ||
| resp.__exit__ = MagicMock(return_value=False) |
Comment on lines
+1998
to
+2002
| from unittest.mock import MagicMock | ||
| import io | ||
|
|
||
| zip_buf = io.BytesIO() | ||
| with zipfile.ZipFile(zip_buf, "w") as zf: |
Comment on lines
+43
to
+56
| if not expected: | ||
| logger.debug( | ||
| "No sha256 declared for %r; archive integrity was not verified.", | ||
| name, | ||
| ) | ||
| return | ||
| expected_hex = str(expected).split(":", 1)[-1].strip().lower() | ||
| actual_hex = hashlib.sha256(data).hexdigest() | ||
| if actual_hex != expected_hex: | ||
| raise error_cls( | ||
| f"Integrity check failed for {name!r}: the catalog declares " | ||
| f"sha256 {expected_hex}, but the downloaded archive is " | ||
| f"{actual_hex}. The archive may be corrupted or tampered with." | ||
| ) |
Collaborator
|
Please address Copilot feedback |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Extension and preset archives are downloaded over HTTPS and unpacked (with Zip-Slip protection), but the downloaded bytes were never checked against a known digest. Trust rested entirely on TLS plus the integrity of the third-party release host.
This is a defense-in-depth / supply-chain hardening change, not an exploit against spec-kit's own code. But the threat model is real: the publishing guide notes that maintainers do not audit extension code, so if a third-party release is tampered with or swapped (compromised release, hijacked
download_url), the altered archive would be installed silently. Consumer-side integrity is the only remaining defense.This PR lets a catalog entry pin an optional
sha256digest:DEBUGline records that the download was unverified), so this is fully backwards compatible.ExtensionCatalog.download_extensionandPresetCatalog.download_pack— through a single shared helper (shared_infra.verify_archive_sha256) so the two cannot drift apart.The optional
sha256field is documented in both publishing guides.Testing
pytest(full suite green, no regressions)uv run specify --helpsha256:prefix / case-insensitive / no-digest DEBUG) and download-path tests for matching, mismatch, and no-digest on both extensions and presetsAI Disclosure
AI assistance (an AI coding agent) was used for the security review that identified the missing integrity check, and to help draft the implementation, tests, and documentation. All changes were reviewed and verified by me (red→green tests including path-parity across both download paths, full suite,
ruff) before submitting.