Add one-command upgrade verification script for sudo signers#2893
Add one-command upgrade verification script for sudo signers#2893unarbos wants to merge 5 commits into
Conversation
…gners Sudo-multisig signers previously had to hand-assemble the srtool docker invocation from the deploy README to verify a proposal before signing. This wraps the whole flow — download release artifacts, confirm the checkout is the proposal commit, run the deterministic srtool build, byte-compare against the released wasm, chain-check via btcli — and prints the sign command pinned to the signer's own build. Release notes and the deploy README now point at it. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| ok "release wasm matches manifest sha256" | ||
|
|
There was a problem hiding this comment.
[HIGH] Cleanliness check excludes build-influencing files
This only checks tracked changes within four paths. Changes to this script or the srtool helper are accepted, as are untracked/ignored inputs such as the repository-level .cargo/ directory (which .gitignore explicitly ignores and Cargo will consume). Such inputs can alter or falsify the build while the script reports that the proposal commit was verified. Require a genuinely pristine checkout, including ignored build/configuration inputs, or perform the build from a fresh archive/worktree containing only files from the pinned commit.
| ok "HEAD is the proposal commit ${commit}" | ||
|
|
||
| # ------------------------------------------------------------------ build --- | ||
| pinned_rustc=$(grep -Eo 'RUSTC_VERSION="[0-9.]+"' scripts/srtool/build-srtool-image.sh | grep -Eo '[0-9.]+') |
There was a problem hiding this comment.
[HIGH] Mutable local image is trusted as the verifier
Any pre-existing Docker image tagged srtool bypasses construction of the expected toolchain. A hostile image can emit the downloaded proposal bytes without compiling the checked-out source, producing a false byte-identical result. Pin and verify the builder image by immutable digest (ideally recorded in the signed/release manifest), or rebuild it from a verified immutable source before every verification.
| echo | ||
| echo "All verifications passed. To sign, pinning the call data to your own build:" | ||
| echo | ||
| echo " btcli upgrade sign --url $url --wasm $local_wasm -w <your-wallet>" |
There was a problem hiding this comment.
[HIGH] Attacker-controlled URL is emitted as shell code
--url accepts shell metacharacters after the tag path, and $url is interpolated unescaped into a command that the signer is explicitly instructed to copy and run. A URL such as one containing ; command turns the displayed signing command into code execution on a privileged signer's workstation. Shell-escape every interpolated argument; the earlier manual upgrade check command has the same issue.
| echo " btcli upgrade sign --url $url --wasm $local_wasm -w <your-wallet>" | |
| printf ' btcli upgrade sign --url %q --wasm %q -w <your-wallet>\n' "$url" "$local_wasm" |
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE MEDIUM scrutiny: 119-day-old account with substantial activity and repository write access; no Gittensor association found; signer-verify-script targets main. The verifier now deletes all pre-existing candidate WASM files before building and requires exactly one output under FindingsNo findings. Prior-comment reconciliation
ConclusionNo malicious behavior or remaining security vulnerability was found in the current diff. 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👍 Established high-volume contributor with repository write access; Gittensor association UNKNOWN. No runtime code is affected, so no spec-version bump is required. Syntax, ShellCheck, and Description discrepancyThe PR body says the script verifies that the invoking checkout is the proposal commit and clean. The implementation instead builds the proposal commit in a pristine clone and expects the invoking checkout to be trusted Findings
ConclusionThe verifier is well scoped and substantially improves the signing workflow. It is ready once the explicit release URL is constrained to the trusted repository. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
… images Addresses the skeptic review: build from a pristine clone of the proposal commit with a fresh cargo home so local modifications, untracked files, and ~/.cargo config cannot influence the verdict; always rebuild the builder image from a commit-pinned srtool source under a dedicated tag instead of trusting whatever local image is tagged srtool; strictly validate the release URL and shell-escape every argument in printed commands. Co-authored-by: Cursor <cursoragent@cursor.com>
| # sign command pinned to YOUR build. Never signs anything itself. | ||
| # | ||
| # Usage: | ||
| # git clone <repo> && cd subtensor && git checkout v<spec> |
There was a problem hiding this comment.
[HIGH] Proposal controls its own verifier
Signers are instructed to execute this script after checking out the proposal tag. Any malicious proposal can therefore modify this verifier to skip comparison, fabricate success, or print a signing command for hostile bytes. Building proposal source is necessary, but the orchestration and checks must come from an independently trusted version—for example, a signer-installed tool or a verifier fetched from a separately pinned, audited commit—and accept the proposal checkout only as input.
|
🔄 AI review updated — Skeptic: VULNERABLE |
Addresses the remaining skeptic finding: the proposal commit previously supplied the very script (and builder-image recipe) that vouched for it. The script now runs from the signer's trusted checkout, discovers or takes the release URL, fetches the proposal commit itself, and builds it in a pristine clone with the builder image recipe taken from the trusted checkout. Docs and release notes now tell signers to run it from main. Co-authored-by: Cursor <cursoragent@cursor.com>
| || note "could not fetch tag ${tag}; assuming the proposal commit is already local" | ||
| git cat-file -e "${commit}^{commit}" 2>/dev/null \ | ||
| || die "proposal commit $commit not found after fetching origin — do not sign" | ||
|
|
There was a problem hiding this comment.
[HIGH] Proposal tag is never bound to the manifest commit
The tag is fetched, but only the manifest-controlled $commit is validated and built. A replaced or attacker-supplied release manifest can therefore select any commit already available in the repository while presenting itself as the requested tagged proposal; the subsequent byte comparison merely proves that the equally supplied WASM matches that substituted commit. Fetch the remote tag into a fresh temporary ref and require its peeled commit to equal $commit before continuing. Do not fall back to an existing local tag/object when the fetch fails.
|
🔄 AI review updated — Skeptic: VULNERABLE |
Addresses the remaining skeptic finding: the verifier fetched the tag but built whatever commit the manifest named, so a manifest could point at any commit and still verify. The tag must now resolve to exactly the manifest commit or the script refuses to proceed. Co-authored-by: Cursor <cursoragent@cursor.com>
unarbos
added
the
skip-cargo-audit
| code=\$?; [ \$code -ne 0 ] && cat /build/runtime/node-subtensor/srtool-output.log && exit \$code; \ | ||
| exit 0" | ||
|
|
||
| built_wasm=$(find "$src/runtime" -name 'node_subtensor_runtime.compact.compressed.wasm' -path '*srtool*' | head -n 1) |
There was a problem hiding this comment.
[HIGH] A committed WASM can masquerade as the build output
The proposal checkout is attacker-controlled and may already contain a tracked file named node_subtensor_runtime.compact.compressed.wasm under any path containing srtool. If the build exits successfully, this unrestricted find | head can select that preloaded binary and compare it to the release, without proving the reviewed runtime source produced those bytes. Before invoking srtool, delete/reject all matching candidates; afterward require exactly one output at srtool's expected build path.
|
🔄 AI review updated — Skeptic: VULNERABLE |
Addresses the remaining skeptic finding: a proposal could commit a wasm at a path matching the output search and have it accepted as the build result. All matching files are deleted before the build, and afterwards exactly one wasm under target/srtool/ is accepted. Co-authored-by: Cursor <cursoragent@cursor.com>
| printf '%s' "$url" | grep -Eq \ | ||
| '^https://github\.com/[A-Za-z0-9._-]+/[A-Za-z0-9._-]+/releases/tag/[A-Za-z0-9._-]+$' \ | ||
| || die "unrecognized release URL: $url" | ||
| slug=$(echo "$url" | sed -E 's#https://github.com/([^/]+/[^/]+)/releases/tag/.*#\1#') |
There was a problem hiding this comment.
[MEDIUM] Bind explicit release URLs to the trusted origin
--url accepts releases from any GitHub repository, while only the tag and WASM-related manifest fields are bound to the trusted checkout. A copied legitimate artifact can therefore pass these checks under an attacker-controlled release URL, which is then forwarded to btcli upgrade check/sign for additional manifest data. Require the URL repository to match the checkout's origin before using it.
| slug=$(echo "$url" | sed -E 's#https://github.com/([^/]+/[^/]+)/releases/tag/.*#\1#') | |
| slug=$(echo "$url" | sed -E 's#https://github.com/([^/]+/[^/]+)/releases/tag/.*#\1#') | |
| origin_slug=$(echo "$origin" | sed -E 's#(git@github.com:|https://github.com/)##; s#\.git$##') | |
| [ "$slug" = "$origin_slug" ] \ | |
| || die "release URL repository ${slug} does not match trusted origin ${origin_slug}" |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
Superseded by #2898 (v431 release): these changes were reviewed and integrated there. |
2 participants
Summary
scripts/verify-upgrade.sh: signers check out the proposal tag and run one command. It downloads the release manifest and wasm, verifies the checkout is the proposal commit and the tree is clean, runs the same deterministic srtool container CI uses, byte-compares the result against the released runtime, runsbtcli upgrade checkif available, and prints thebtcli upgrade signcommand pinned to the signer's own build. It never submits anything.Test plan
bash -nand shellcheck cleanMade with Cursor