Repository navigation
install.ps1: report a checksum-verified layout mismatch separately from an unsafe archive - #2766
Conversation
… unsafe archive Refs DeusData#1218 Signed-off-by: Mukkara Rithvik Reddy <rithvikreddymukkara@gmail.com>
|
Thanks for opening this — it has been seen, and it is queued. This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence. Current review status: working through a backlog. What that means for this PR, concretely:
Things that will genuinely speed it up whenever review does happen:
If this fixes a bug, a reproduction we can run is worth more than a description of the symptom. Thanks for contributing, and sorry in advance for the wait. |
|
Thank you, @RithvikReddy0-0, for carrying this all the way from your diagnosis on #1218 to a fix. Telling a checksum-verified layout mismatch apart from a genuinely unsafe archive is exactly the follow-up we hoped for, and you kept everything fail-closed: same allowlist, same checksum gate, same exit code. The part we appreciate most is the ordering: every entry is screened for unsafe paths and duplicates before any layout verdict, so a hostile entry can never hide behind the friendlier message. The contract assertions and your runs on both PowerShell versions made this easy to review. Merged, thank you! |
Closes #1218
What
This is the follow-up @DeusData flagged in #1218: the error now tells "genuine but different layout" apart from tampering. Nothing is relaxed. The allowlist, the checksum gate and the exit code (1) are all unchanged. Only the diagnosis changes.
install.ps1used to printerror: unsafe or incomplete release archivefor two opposite situations:../evil.exe)unsafe release archiveunsafe release archivepayload.exe)release archive layout does not match this installer+ what to dosub/LICENSE)The layout-mismatch message names the expected file set and the fix: run the
install.ps1shipped inside that release, or retry after the next release. That is the situation from #1218, where main's script ran ahead of the latest archive.Ordering guarantee
Validation used to throw on the first bad entry it met. If the layout message had simply been bolted onto that single pass, an archive listing an unexpected file before a
../entry would get the reassuring "genuine artifact" message and hide the hostile entry. This PR screens every entry for unsafe paths and duplicates first, and only then reaches a layout verdict. A hostile entry always wins, whatever its position.The message is also deliberately limited to what the check proves. checksums.txt comes from the same
$BaseUrlas the archive, so the message says the archive matches what that source published. It does not claim more than that, which matters whenCBM_DOWNLOAD_URLis overridden.Also fixes a stale comment ("official five entries" -> four).
Tests
tests/test_windows_bundle_contract.sh: two new assertions. (1) The two messages are distinct and the old conflated string is gone. (2) The single$LayoutMismatch = $trueverdict comes after the last unsafe-path and duplicatethrow. Both fail on main and pass with this change.Behavioral run of the real
install.ps1against a loopbackCBM_DOWNLOAD_URLserving 9 fixture archives, each with a matching checksums.txt (bad-checksum case excepted):good-> passes validation (then fails only because the fixture .exe is fake)extra,missing,nested-> layout mismatch, exit 1traversal,casedup-> unsafe, exit 1extra_then_traversal,extra_then_casedup-> unsafe, exit 1 (the ordering case)badsum->CHECKSUM MISMATCH, exit 1 (unchanged)Ran on Windows 11 (build 26200) under Windows PowerShell 5.1.26100 and PowerShell 7.6.7, with identical results.
Native Windows run: the unmodified
install.ps1, no path shim, with a throwaway--dir=andLOCALAPPDATA; no install directory was written in any case.Still passing:
test_release_archive_extractor_contract.sh,test_setup_scripts_contract.sh,test_release_gate_chain_contract.sh.install.ps1is still pure ASCII (PS 5.1 requirement).On the CI release-contract check I proposed in #1218: it's already covered by
test_windows_bundle_contract.shandtest_release_archive_extractor_contract.sh(single-sourced four-file allowlist, fail-closed on surplus or missing members). So this PR doesn't add one.