fix(install): make checksum verification mandatory by areporeporepo · Pull Request #626 · NVIDIA/OpenShell

areporeporepo · 2026-03-26T13:59:34Z

Summary

Makes SHA-256 checksum verification mandatory in install.sh instead of silently skipping it when tools or files are unavailable. Adds an explicit opt-out (--no-verify-checksum / OPENSHELL_NO_VERIFY=1) for environments where verification is not feasible.

Related Issue

Closes #590

Changes

install.sh

verify_checksum(): warn → error when sha256sum/shasum is unavailable (line 200) or when no checksum entry exists for the archive (line 192). Previously both cases returned 0, allowing an unverified binary to be installed.
main(): warn → error when the checksums file cannot be downloaded (line 276). Previously the installer continued without any verification.
New flag: --no-verify-checksum CLI flag and OPENSHELL_NO_VERIFY=1 env var to explicitly opt out. Error messages reference the env var so users know how to proceed if verification cannot be performed.
usage(): Documents the new flag, env var, and adds an example.

Testing

shellcheck install.sh — zero warnings
sh -n install.sh — syntax valid
Existing e2e tests (e2e/install/{sh,bash,zsh}_test.sh) unaffected — they use real releases with checksums available
CI passes on fork: fix(install): make checksum verification mandatory areporeporepo/OpenShell-1#1

Checklist

Follows Conventional Commits
Commits are signed off (DCO)
No secrets or credentials committed
Changes scoped to the issue at hand

- verify_checksum() now errors instead of warning when sha256sum/shasum is unavailable or when no checksum entry exists for the archive - main() now errors when the checksums file cannot be downloaded - Add --no-verify-checksum flag and OPENSHELL_NO_VERIFY env var for explicit opt-out in environments where verification is not feasible - Update usage() with new flag, env var, and example Previously, an attacker who could manipulate the download (MITM without HTTPS pinning) could serve a binary without the checksum file, and the installer would proceed silently. Now the installer fails loudly unless the operator explicitly opts out. Closes NVIDIA#590 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: areporeporepo <areporeporepo@users.noreply.github.com>

Copilot

Pull request overview

This PR hardens the install.sh installer by making SHA-256 checksum verification mandatory by default, while adding an explicit opt-out for environments that can’t verify checksums.

Changes:

Fail fast when checksum verification can’t be performed (missing checksum entry, missing sha256sum/shasum, or checksums file download failure).
Add --no-verify-checksum flag and OPENSHELL_NO_VERIFY=1 env var to explicitly skip verification.
Update usage() docs and examples to reflect the new opt-out.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-26T14:04:00Z

install.sh

+  _skip_checksum="${OPENSHELL_NO_VERIFY:-0}"
+
  # Parse CLI flags
  for arg in "$@"; do
    case "$arg" in
      --help)
        usage
        exit 0
        ;;
+      --no-verify-checksum)
+        _skip_checksum=1
+        ;;


New behavior/option (OPENSHELL_NO_VERIFY / --no-verify-checksum) isn’t covered by the existing install.sh e2e tests. Please add a small test case that runs the installer with the opt-out enabled and asserts the expected warning/output so regressions don’t silently re-enable mandatory verification or break the opt-out path.

Copilot · 2026-03-26T14:04:00Z

install.sh

  if [ -z "$_vc_expected" ]; then
-    warn "no checksum found for $_vc_filename, skipping verification"
-    return 0
+    error "no checksum found for $_vc_filename in checksums file"


The new fatal error when the checksums file has no entry for the archive doesn’t tell users how to proceed in environments where verification isn’t feasible. Consider including the same explicit opt-out guidance here (mention OPENSHELL_NO_VERIFY=1 and/or --no-verify-checksum) for consistency with the other checksum-related errors.

Suggested change

error "no checksum found for $_vc_filename in checksums file"

error "no checksum found for $_vc_filename in checksums file (set OPENSHELL_NO_VERIFY=1 or pass --no-verify-checksum to skip)"

Copilot · 2026-03-26T14:04:00Z

install.sh

  else
-    warn "sha256sum/shasum not found, skipping checksum verification"
-    return 0
+    error "sha256sum or shasum is required for checksum verification (install coreutils or set OPENSHELL_NO_VERIFY=1 to skip)"


Error text suggests only OPENSHELL_NO_VERIFY=1 to bypass checksum verification. Since a CLI opt-out flag was added, consider mentioning --no-verify-checksum here as well so users who run ./install.sh (not via pipe) can follow the guidance directly.

Copilot · 2026-03-26T14:04:01Z

install.sh

+        error "checksum verification failed for ${_filename}"
+      fi
+    else
+      error "failed to download checksums file from ${_checksums_url} (set OPENSHELL_NO_VERIFY=1 to skip verification)"


This failure message points users to the env var opt-out, but not the new --no-verify-checksum flag. Consider mentioning both opt-out mechanisms for consistency and to help users running the script directly.

Suggested change

error "failed to download checksums file from ${_checksums_url} (set OPENSHELL_NO_VERIFY=1 to skip verification)"

error "failed to download checksums file from ${_checksums_url} (set OPENSHELL_NO_VERIFY=1 or use --no-verify-checksum to skip verification)"

Copilot · 2026-03-26T14:04:01Z

install.sh

  _vc_expected="$(grep "$_vc_filename" "$_vc_checksums" | awk '{print $1}')"



The checksum lookup uses grep "$_vc_filename" which treats the filename as a regex (e.g., . matches any char) and can also match substrings, potentially selecting the wrong checksum line. Consider parsing the checksums file with an exact-field match (e.g., match $2 == filename) and ensure only a single checksum is selected.

Suggested change

_vc_expected="$(grep "$_vc_filename" "$_vc_checksums" | awk '{print $1}')"

_vc_expected="$(

awk -v fname="$_vc_filename" '

$2 == fname {

if (found) {

print "MULTIPLE_MATCHES"

exit 0

}

checksum = $1

found = 1

}

END {

if (found) {

print checksum

}

}

' "$_vc_checksums"

)"

if [ "$_vc_expected" = "MULTIPLE_MATCHES" ]; then

error "multiple checksums found for $_vc_filename in checksums file"

fi

- Use awk exact-field match instead of grep regex for checksum lookup - Include both opt-out mechanisms (env var + flag) in all error messages - Add e2e test for OPENSHELL_NO_VERIFY=1 checksum skip path Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: areporeporepo <areporeporepo@users.noreply.github.com>

- Accept 1, true, yes, y (case-insensitive) for OPENSHELL_NO_VERIFY - Clean up temp directory in e2e checksum skip test Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: areporeporepo <areporeporepo@users.noreply.github.com>

areporeporepo requested a review from a team as a code owner March 26, 2026 13:59

Copilot AI review requested due to automatic review settings March 26, 2026 13:59

Copilot started reviewing on behalf of areporeporepo March 26, 2026 14:00 View session

Copilot AI reviewed Mar 26, 2026

View reviewed changes

areporeporepo and others added 2 commits March 26, 2026 07:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(install): make checksum verification mandatory#626

fix(install): make checksum verification mandatory#626
areporeporepo wants to merge 3 commits intoNVIDIA:mainfrom
areporeporepo:fix/install-mandatory-checksum

areporeporepo commented Mar 26, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Copilot AI Mar 26, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	error "no checksum found for $_vc_filename in checksums file"
	error "no checksum found for $_vc_filename in checksums file (set OPENSHELL_NO_VERIFY=1 or pass --no-verify-checksum to skip)"

	error "failed to download checksums file from ${_checksums_url} (set OPENSHELL_NO_VERIFY=1 to skip verification)"
	error "failed to download checksums file from ${_checksums_url} (set OPENSHELL_NO_VERIFY=1 or use --no-verify-checksum to skip verification)"

		_vc_expected="$(grep "$_vc_filename" "$_vc_checksums" \| awk '{print $1}')"

-  _vc_expected="$(grep "$_vc_filename" "$_vc_checksums" | awk '{print $1}')"
+  _vc_expected="$(
+    awk -v fname="$_vc_filename" '
+      $2 == fname {
+        if (found) {
+          print "MULTIPLE_MATCHES"
+          exit 0
+        }
+        checksum = $1
+        found = 1
+      }
+      END {
+        if (found) {
+          print checksum
+        }
+      }
+    ' "$_vc_checksums"
+  )"
+  if [ "$_vc_expected" = "MULTIPLE_MATCHES" ]; then
+    error "multiple checksums found for $_vc_filename in checksums file"
+  fi

Conversation

areporeporepo commented Mar 26, 2026

Summary

Related Issue

Changes

Testing

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants