fix: npm SHA-1 checksums and drop sh package invoke - #335
Merged
Merged
Conversation
npm shasum used SHA-256, so integrity checks could not match registry metadata. Remove sh from package export invoke allowlist and switch the fiber adapter to argv-style go run. Tighten CI/release checkout perms. Amp-Thread-ID: https://ampcode.com/threads/T-01a0a631-f6de-735d-b395-b45d281f927e Co-authored-by: Max Carter <undivisible@vk.com>
|
Capy couldn't review this pull request because Max's workspace is out of credits, add credits or enable auto-reload to resume automatic reviews. |
Bugbot couldn't run - usage limit reachedBugbot is counted against Cursor usage for this user or team, and this run hit a usage or spend limit. A user or team admin can review and increase usage limits in the Cursor dashboard. (requestId: serverGenReqId_72e36551-0480-47d0-8fc7-2bd6b0cfe22d) |
|
Tick the box to add this pull request to the merge queue (same as
|
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.
Why
ArtifactChecksum::Sha1Hexhashed with SHA-256, so npmshasumverification could never match. Package export invoke also allowlistedsh, which is a shell execution surface.What
sha1::Sha1and test the known digest ofhello world.env_clear()oncurl_to_file(already used bycurl_get).shfromALLOWED_INVOKE_PROGRAMS; reject it in a unit test.go:fiberadapter togo run ./inauguration-invoke(argv, no shell).permissions: contents: readon CI;persist-credentials: falseon checkouts.Tests
cargo test --features extended --locked --lib -- verify_archive_checksum require_https rejects_shell_invoke installs_path_dependenciescargo clippy --all-targets --features extended --locked -- -D warningscargo fmt --checkMerge note
Do not auto-merge: this touches integrity verification and an execution allowlist.
Note
High Risk
Changes archive integrity checks and the package export program allowlist—both security-sensitive paths that could block valid npm installs or break adapters still using shell invoke.
Overview
Fixes npm tarball verification when registries expose legacy
shasum(SHA-1 hex):Sha1Hexwas compared against a SHA-256 digest, so installs could never validate; verification now usessha1::Sha1on the downloaded bytes, with a unit test for the knownhello worlddigest.Hardens registry fetch and package export execution:
curl_to_fileclears the environment likecurl_get;shis removed fromALLOWED_INVOKE_PROGRAMSso adapter JSON cannot spawn a shell (new rejection test). The go:fiber sample adapter is updated togo run ./inauguration-invokeinstead ofsh -c.GitHub Actions on CI sets
permissions: contents: read; CI and release jobs passpersist-credentials: falseonactions/checkout.Reviewed by Cursor Bugbot for commit 0ac8219. Configure here.