mirror of
https://github.com/github/spec-kit.git
synced 2026-07-03 12:28:06 +08:00
* feat(extensions): verify catalog archive sha256 before install 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 * fix(extensions): harden sha256 parsing and tidy download test mocks Follow-up to the review on #3080: - shared_infra.verify_archive_sha256: strip only a literal `sha256:` algorithm prefix (case-insensitive) instead of `split(':', 1)[-1]`, which silently dropped any prefix — so `md5:<64-hex>` was accepted as if it were a valid SHA-256. Validate that the declared value is exactly 64 hex characters and raise a clear error otherwise, and compare with `hmac.compare_digest` for a constant-time check. Add tests covering a malformed digest and a non-`sha256:` prefix (both previously accepted). - Download test helpers: configure the context-manager mock via `__enter__.return_value`/`__exit__.return_value` rather than assigning a `lambda s: s`, which is clearer and independent of the invocation arity. Assisted-by: AI Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com> * fix(extensions): reject a declared-but-empty sha256 instead of skipping verification verify_archive_sha256 skipped on any falsy expected value, so a present-but-empty digest (e.g. sha256: "" reached via ...get("sha256")) silently disabled the integrity check instead of surfacing the authoring error. Guard on expected is None so only an absent digest skips; blank/whitespace/bare-prefix values fall through to the 64-hex validation and are rejected. Adds a regression test. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * docs(shared_infra): clarify _SHA256_HEX_RE accepts and normalizes uppercase The comment described the regex as matching '64 lowercase' hex characters, but verify_archive_sha256 lowercases the declared value (raw.lower()) before matching, so an uppercase digest is accepted and normalized rather than rejected. Clarify the comment to avoid misleading future readers. Addresses Copilot review feedback on shared_infra.py. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> * test(presets): cover the no-sha256 backwards-compatible path Address Copilot review: download_pack's optional sha256 verification was tested for match/mismatch but not the backwards-compatible path where a catalog entry has no sha256 (pack_info.get("sha256") is None). Add a no-sha256 test mirroring the extensions coverage so the helper never silently becomes mandatory for presets. Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> --------- Signed-off-by: Zied Jlassi <6190550+zied-jlassi@users.noreply.github.com> Signed-off-by: Zied Jlassi (Architect AI) <6190550+zied-jlassi@users.noreply.github.com>
102 lines
3.7 KiB
Python
102 lines
3.7 KiB
Python
"""Unit tests for the shared archive-integrity helper.
|
|
|
|
These exercise ``verify_archive_sha256`` directly (independently of the
|
|
extension/preset download paths that call it) so the digest-matching,
|
|
mismatch, normalisation and "no digest declared" behaviours are pinned in
|
|
one place.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import hashlib
|
|
import logging
|
|
|
|
import pytest
|
|
|
|
from specify_cli.shared_infra import verify_archive_sha256
|
|
|
|
|
|
class _BoomError(Exception):
|
|
"""Sentinel error type used to assert the helper raises ``error_cls``."""
|
|
|
|
|
|
def test_matching_digest_passes():
|
|
"""A digest that matches the data returns without raising."""
|
|
data = b"hello-archive"
|
|
digest = hashlib.sha256(data).hexdigest()
|
|
verify_archive_sha256(data, digest, "thing", _BoomError)
|
|
|
|
|
|
def test_mismatch_raises_error_cls():
|
|
"""A non-matching digest raises the caller-supplied error type."""
|
|
with pytest.raises(_BoomError, match="[Ii]ntegrity"):
|
|
verify_archive_sha256(b"data", "0" * 64, "thing", _BoomError)
|
|
|
|
|
|
def test_sha256_prefix_is_accepted():
|
|
"""A ``sha256:`` prefix on the expected digest is tolerated."""
|
|
data = b"prefixed"
|
|
digest = hashlib.sha256(data).hexdigest()
|
|
verify_archive_sha256(data, f"sha256:{digest}", "thing", _BoomError)
|
|
|
|
|
|
def test_comparison_is_case_insensitive():
|
|
"""An upper-cased expected digest still matches the lower-case actual."""
|
|
data = b"casing"
|
|
digest = hashlib.sha256(data).hexdigest().upper()
|
|
verify_archive_sha256(data, digest, "thing", _BoomError)
|
|
|
|
|
|
def test_malformed_digest_is_rejected():
|
|
"""A declared digest that is not 64 hex chars is rejected up front.
|
|
|
|
A too-short, too-long, or non-hex value is an authoring/catalog error and
|
|
must surface clearly instead of being treated as a digest that simply does
|
|
not match the archive.
|
|
"""
|
|
for bad in ("deadbeef", "z" * 64, "0" * 63, "0" * 65):
|
|
with pytest.raises(_BoomError, match="[Ii]nvalid sha256"):
|
|
verify_archive_sha256(b"data", bad, "thing", _BoomError)
|
|
|
|
|
|
def test_non_sha256_prefix_is_not_silently_stripped():
|
|
"""Only a literal ``sha256:`` prefix is stripped.
|
|
|
|
A different algorithm prefix (e.g. ``md5:``) must not be silently dropped
|
|
and accepted as if the remaining characters were a valid SHA-256 digest;
|
|
the value is rejected as malformed.
|
|
"""
|
|
data = b"prefixed"
|
|
digest = hashlib.sha256(data).hexdigest()
|
|
with pytest.raises(_BoomError, match="[Ii]nvalid sha256"):
|
|
verify_archive_sha256(data, f"md5:{digest}", "thing", _BoomError)
|
|
|
|
|
|
def test_absent_digest_skips_and_logs_debug(caplog):
|
|
"""When no digest is declared the helper returns and logs at DEBUG.
|
|
|
|
Installs stay backwards compatible (no error, no user-facing warning),
|
|
but the unverified download leaves an audit trail for operators who opt
|
|
into debug logging.
|
|
"""
|
|
with caplog.at_level(logging.DEBUG, logger="specify_cli.shared_infra"):
|
|
verify_archive_sha256(b"data", None, "thing", _BoomError)
|
|
assert any(
|
|
"not verified" in r.getMessage() and "thing" in r.getMessage()
|
|
for r in caplog.records
|
|
)
|
|
|
|
|
|
def test_blank_declared_digest_is_rejected():
|
|
"""A present-but-empty ``sha256`` is an authoring error, not an opt-out.
|
|
|
|
Catalog entries reach the helper via ``...get("sha256")``; a blank value
|
|
(``""``, whitespace, or a bare ``sha256:`` prefix) means the digest was
|
|
declared but left empty. It must surface as a malformed digest rather than
|
|
silently disabling the integrity check, which a bare ``if not expected``
|
|
guard would have done.
|
|
"""
|
|
for blank in ("", " ", "sha256:"):
|
|
with pytest.raises(_BoomError, match="[Ii]nvalid sha256"):
|
|
verify_archive_sha256(b"data", blank, "thing", _BoomError)
|