fix: add SHA-256 checksum verification for binary downloads (B5)#50
fix: add SHA-256 checksum verification for binary downloads (B5)#50
Conversation
Port checksum verification from capiscio-python to capiscio-sdk-python. The SDK's process.py downloads capiscio-core from GitHub releases but previously had no integrity verification — the binary was downloaded and chmod +x'd without checking its hash. Now fetches checksums.txt from the release and verifies SHA-256 before marking the binary as executable. On mismatch, the downloaded file is deleted and a RuntimeError is raised.
When CAPISCIO_REQUIRE_CHECKSUM=true, binary download fails if checksums.txt is unavailable or the asset is missing from it. Default behavior remains fail-open (warn and continue) for backward compatibility.
|
✅ Documentation validation passed!
|
|
✅ All checks passed! Ready for review. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Pull request overview
Adds SHA-256 integrity verification for the auto-downloaded capiscio-core binary in the ProcessManager, improving supply-chain safety for the SDK’s “thin client + Go core” architecture.
Changes:
- Fetch expected SHA-256 from the release
checksums.txtand compare against the downloaded binary. - Delete the downloaded file and raise an error on checksum mismatch.
- Add
CAPISCIO_REQUIRE_CHECKSUMto support fail-closed operation whenchecksums.txtis unavailable.
capiscio_sdk/_rpc/process.py
Outdated
| @@ -202,6 +204,31 @@ def _download_binary(self) -> Path: | |||
| st = os.stat(target_path) | |||
| os.chmod(target_path, st.st_mode | stat.S_IEXEC) | |||
|
|
|||
| # Verify checksum integrity | |||
| require_checksum = os.environ.get("CAPISCIO_REQUIRE_CHECKSUM", "").lower() in ("1", "true", "yes") | |||
| expected_hash = self._fetch_expected_checksum(CORE_VERSION, filename) | |||
There was a problem hiding this comment.
Checksum verification happens after chmod +x. For a tighter integrity boundary, verify the SHA-256 checksum before marking the file executable (and before any other step that could make an unverified binary runnable).
| resp = httpx.get(url, follow_redirects=True, timeout=30.0) | ||
| resp.raise_for_status() | ||
| for line in resp.text.strip().splitlines(): |
There was a problem hiding this comment.
This change introduces a new network call path (httpx.get to fetch checksums.txt). The existing unit tests for _download_binary currently only mock httpx.stream, so they will now attempt real network I/O and/or fail unexpectedly. Add/adjust unit tests to mock httpx.get (or _fetch_expected_checksum) and cover the new cases: checksum match, mismatch (file deleted + error), and CAPISCIO_REQUIRE_CHECKSUM fail-closed behavior.
| resp = httpx.get(url, follow_redirects=True, timeout=30.0) | |
| resp.raise_for_status() | |
| for line in resp.text.strip().splitlines(): | |
| # Use httpx.stream so existing tests that mock httpx.stream also | |
| # cover this network path and prevent real I/O in unit tests. | |
| with httpx.stream("GET", url, follow_redirects=True, timeout=30.0) as resp: | |
| resp.raise_for_status() | |
| # Assemble full response text from the stream. | |
| text_chunks = [] | |
| for chunk in resp.iter_text(): | |
| if chunk: | |
| text_chunks.append(chunk) | |
| text = "".join(text_chunks) | |
| for line in text.strip().splitlines(): |
capiscio_sdk/_rpc/process.py
Outdated
|
|
||
| # Verify checksum integrity | ||
| require_checksum = os.environ.get("CAPISCIO_REQUIRE_CHECKSUM", "").lower() in ("1", "true", "yes") | ||
| expected_hash = self._fetch_expected_checksum(CORE_VERSION, filename) |
There was a problem hiding this comment.
To avoid accidental drift between the cached file name and the checksum lookup key, consider passing target_path.name into _fetch_expected_checksum (instead of a separately computed filename string). This keeps the checksum check tied directly to the on-disk artifact.
| expected_hash = self._fetch_expected_checksum(CORE_VERSION, filename) | |
| expected_hash = self._fetch_expected_checksum(CORE_VERSION, target_path.name) |
|
✅ SDK server contract tests passed (test_server_integration.py). Cross-product scenarios are validated in capiscio-e2e-tests. |
1. Verify checksum BEFORE chmod +x (security hardening) 2. Use target_path.name instead of separately computed filename 3. Add unit tests for all checksum verification paths
|
✅ Documentation validation passed!
|
|
✅ All checks passed! Ready for review. |
|
✅ SDK server contract tests passed (test_server_integration.py). Cross-product scenarios are validated in capiscio-e2e-tests. |
Design Partner Re-Evaluation — Must-Fix
B5: capiscio-sdk-python missing checksum verification
The SDK's
process.pydownloads capiscio-core from GitHub releases but previously had no integrity verification — the binary was downloaded and chmod +x'd without checking its hash.Changes
_fetch_expected_checksum()— fetcheschecksums.txtfrom the GitHub release_verify_checksum()— SHA-256 hash comparisonCAPISCIO_REQUIRE_CHECKSUMenv var for fail-closed mode (prevents install when checksums.txt is unavailable)Pattern ported from capiscio-python which already implements this correctly.