fix(plugin): verify install checksums against the artifact actually downloaded

internal/plugin/checksum_artifact_test.go

@@ -0,0 +1,196 @@
+package plugin
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"strings"
+	"testing"
+)
+
+// TestResolveArtifactChecksum_SourceUsesManifestChecksum verifies that for
+// the source artifact (ArtifactKindSource, and the zero value "" — a
+// download that never got as far as reporting a kind), resolveArtifactChecksum
+// returns manifest.Checksum verbatim, unaffected by whatever is in
+// PlatformChecksums.
+func TestResolveArtifactChecksum_SourceUsesManifestChecksum(t *testing.T) {
+	manifest := PluginManifest{
+		Name:              "example",
+		Version:           "1.0.1",
+		Checksum:          "deadbeef",
+		PlatformChecksums: map[string]string{"linux-amd64": "should-not-be-used"},
+	}
+
+	for _, kind := range []string{ArtifactKindSource, ""} {
+		t.Run("kind="+kind, func(t *testing.T) {
+			got, err := resolveArtifactChecksum(manifest, kind)
+			if err != nil {
+				t.Fatalf("resolveArtifactChecksum: %v", err)
+			}
+			if got != "deadbeef" {
+				t.Errorf("got %q, want manifest.Checksum %q", got, manifest.Checksum)
+			}
+		})
+	}
+}
+
+// TestResolveArtifactChecksum_SourceEmptyChecksumPassesThrough verifies that
+// an empty manifest.Checksum for the source artifact comes back as "", not
+// an error — the FIX-CLI-6 warn-and-proceed leniency for a missing SOURCE
+// checksum is verifyChecksum's decision to make, not this function's.
+func TestResolveArtifactChecksum_SourceEmptyChecksumPassesThrough(t *testing.T) {
+	manifest := PluginManifest{Name: "example", Version: "1.0.1"}
+
+	got, err := resolveArtifactChecksum(manifest, ArtifactKindSource)
+	if err != nil {
+		t.Fatalf("resolveArtifactChecksum: %v", err)
+	}
+	if got != "" {
+		t.Errorf("got %q, want empty string", got)
+	}
+}
+
+// TestResolveArtifactChecksum_PlatformUsesMatchingEntry verifies that a
+// platform artifact resolves to PlatformChecksums[platform], the entry
+// matching the artifact actually downloaded — not manifest.Checksum (the
+// SOURCE tarball's checksum, different bytes entirely) and not a different
+// platform's entry.
+func TestResolveArtifactChecksum_PlatformUsesMatchingEntry(t *testing.T) {
+	manifest := PluginManifest{
+		Name:     "example",
+		Version:  "1.0.1",
+		Checksum: "source-checksum-must-not-be-used",
+		PlatformChecksums: map[string]string{
+			"darwin-arm64": "darwin-arm64-checksum",
+			"linux-amd64":  "linux-amd64-checksum",
+		},
+	}
+
+	got, err := resolveArtifactChecksum(manifest, "linux-amd64")
+	if err != nil {
+		t.Fatalf("resolveArtifactChecksum: %v", err)
+	}
+	if got != "linux-amd64-checksum" {
+		t.Errorf("got %q, want the linux-amd64 entry, not the source checksum or a different platform's", got)
+	}
+}
+
+// TestResolveArtifactChecksum_PlatformMissingEntryRefusesUnconditionally
+// verifies the absent-checksum policy (plugins#83 item 8): a platform
+// artifact with NO matching registry entry is refused with an error —
+// regardless of NSELF_PLUGIN_REQUIRE_CHECKSUM, regardless of whether
+// manifest.Checksum (the source checksum) happens to be present, and
+// regardless of whether PlatformChecksums is nil or just missing this one
+// platform's key. This is what makes the policy impossible to bypass: there
+// is no code path, env var, or registry shape that turns a missing platform
+// checksum into an empty string reaching verifyChecksum's lenient branch.
+func TestResolveArtifactChecksum_PlatformMissingEntryRefusesUnconditionally(t *testing.T) {
+	cases := []struct {
+		name              string
+		platformChecksums map[string]string
+		requireChecksum   bool
+	}{
+		{name: "nil map, default mode", platformChecksums: nil, requireChecksum: false},
+		{name: "nil map, hard mode", platformChecksums: nil, requireChecksum: true},
+		{name: "other platforms present, default mode", platformChecksums: map[string]string{"darwin-arm64": "abc123"}, requireChecksum: false},
+		{name: "other platforms present, hard mode", platformChecksums: map[string]string{"darwin-arm64": "abc123"}, requireChecksum: true},
+		{name: "empty-string entry for this platform", platformChecksums: map[string]string{"linux-amd64": ""}, requireChecksum: false},
+	}
+
+	for _, tc := range cases {
+		t.Run(tc.name, func(t *testing.T) {
+			if tc.requireChecksum {
+				t.Setenv(pluginRequireChecksumEnv, "1")
+			}
+			manifest := PluginManifest{
+				Name:              "example",
+				Version:           "1.0.1",
+				Checksum:          "source-checksum-present-but-irrelevant",
+				PlatformChecksums: tc.platformChecksums,
+			}
+
+			_, err := resolveArtifactChecksum(manifest, "linux-amd64")
+			if err == nil {
+				t.Fatal("expected an error refusing the install, got nil")
+			}
+			if !strings.Contains(err.Error(), "linux-amd64") {
+				t.Errorf("error should name the platform that has no checksum: %v", err)
+			}
+		})
+	}
+}
+
+// TestPlatformArtifact_VerifiesAgainstMatchingPlatformChecksum is the
+// end-to-end shape of Step 5 for a platform download: resolve the checksum
+// for the artifact actually downloaded, then verify the archive bytes
+// against it. A correct platform checksum passes.
+func TestPlatformArtifact_VerifiesAgainstMatchingPlatformChecksum(t *testing.T) {
+	content := []byte("fake linux-amd64 binary tarball bytes")
+	archivePath := writeTempFile(t, content)
+	sum := sha256.Sum256(content)
+	platformChecksum := hex.EncodeToString(sum[:])
+
+	manifest := PluginManifest{
+		Name:     "example",
+		Version:  "1.0.1",
+		Checksum: "unrelated-source-checksum",
+		PlatformChecksums: map[string]string{
+			"linux-amd64": platformChecksum,
+		},
+	}
+
+	expected, err := resolveArtifactChecksum(manifest, "linux-amd64")
+	if err != nil {
+		t.Fatalf("resolveArtifactChecksum: %v", err)
+	}
+	if err := verifyChecksum(archivePath, expected, "stable"); err != nil {
+		t.Fatalf("correct platform checksum should verify: %v", err)
+	}
+}
+
+// TestPlatformArtifact_MismatchRejected verifies that a platform download
+// whose bytes do not match its recorded platform checksum is rejected — the
+// same as any other present-but-wrong checksum (mirrors
+// TestVerifyChecksum_MismatchAlwaysRefusesRegardlessOfStatus, for the
+// platform-checksum path specifically).
+func TestPlatformArtifact_MismatchRejected(t *testing.T) {
+	content := []byte("fake linux-amd64 binary tarball bytes")
+	archivePath := writeTempFile(t, content)
+	wrongChecksum := hex.EncodeToString(make([]byte, sha256.Size)) // all-zero, guaranteed wrong
+
+	manifest := PluginManifest{
+		Name:    "example",
+		Version: "1.0.1",
+		PlatformChecksums: map[string]string{
+			"linux-amd64": wrongChecksum,
+		},
+	}
+
+	expected, err := resolveArtifactChecksum(manifest, "linux-amd64")
+	if err != nil {
+		t.Fatalf("resolveArtifactChecksum: %v", err)
+	}
+	if err := verifyChecksum(archivePath, expected, "stable"); err == nil {
+		t.Fatal("tampered/mismatched platform artifact should fail verification, got nil error")
+	}
+}
+
+// TestPlatformArtifact_AbsentChecksumNeverReachesLenientPath verifies the
+// absent-checksum policy end to end: a platform artifact with no registry
+// checksum never reaches verifyChecksum's warn-and-proceed branch at all —
+// resolveArtifactChecksum refuses first, before any checksum comparison
+// (lenient or not) would run.
+func TestPlatformArtifact_AbsentChecksumNeverReachesLenientPath(t *testing.T) {
+	content := []byte("fake linux-amd64 binary tarball bytes")
+	archivePath := writeTempFile(t, content)
+	_ = archivePath // the archive is never reached; resolveArtifactChecksum fails first
+
+	manifest := PluginManifest{
+		Name:              "example",
+		Version:           "1.0.1",
+		PlatformChecksums: nil,
+	}
+
+	if _, err := resolveArtifactChecksum(manifest, "linux-amd64"); err == nil {
+		t.Fatal("expected resolveArtifactChecksum to refuse before any verification would run")
+	}
+}

internal/plugin/download.go

@@ -24,6 +24,14 @@ import (
 	"github.com/nself-org/cli/internal/license"
 )
 
+// ArtifactKindSource is the artifact kind downloadPluginPackageForTier
+// reports when it fetched the source tarball. Any other value it returns is
+// one of PlatformArch()'s platform strings (darwin-arm64, darwin-amd64,
+// linux-amd64, linux-arm64, windows-amd64), identifying a per-platform
+// binary tarball instead. Callers use this to pick the checksum that
+// actually matches the bytes on disk — see installer_locked.go's Step 5.
+const ArtifactKindSource = "source"
+
 // downloadPlugin fetches the plugin tarball to a temporary file.
 // For paid plugins, it sends the X-License-Key header required by ping.nself.org.
 // For free plugins, it tries the R2-backed worker URL first and falls back to
@@ -35,18 +43,24 @@ import (
 // see license.go's isPaidPluginManifest doc comment for why the name map
 // drifts and the registry fields do not.
 func downloadPlugin(ctx context.Context, name, version, repository string) (string, error) { //nolint:unused // kept: name-only fallback entry point retained deliberately; see qa/bugs/declared-but-never-wired-symbols.md
-	return downloadPluginPackage(ctx, name, version, repository, "")
+	path, _, err := downloadPluginPackage(ctx, name, version, repository, "")
+	return path, err
 }
 
 // downloadPluginPackage fetches a plugin package using the name-only
 // isPaidPlugin fallback. Prefer downloadPluginPackageForTier when a manifest
 // is available (see its doc comment).
-func downloadPluginPackage(ctx context.Context, name, version, repository, binaryName string) (string, error) {
+func downloadPluginPackage(ctx context.Context, name, version, repository, binaryName string) (string, string, error) {
 	return downloadPluginPackageForTier(ctx, name, version, repository, binaryName, isPaidPlugin(name))
 }
 
 // downloadPluginPackageForTier fetches a plugin package, preferring a build
-// for the running platform when the plugin ships a command binary.
+// for the running platform when the plugin ships a command binary. It
+// returns the local temp file path and the kind of artifact it actually
+// fetched (ArtifactKindSource or a platform string) — the caller needs the
+// kind to know which registry checksum applies, since a source tarball and a
+// platform tarball for the same plugin+version have different bytes and
+// therefore different checksums (see PluginManifest.PlatformChecksums).
 //
 // paid must come from the registry manifest (isPaidPluginManifest), not the
 // static paidPlugins name map — that map only lists 59 of 127 registered paid
@@ -61,7 +75,7 @@ func downloadPluginPackage(ctx context.Context, name, version, repository, binar
 // someone on macOS. Those live as per-platform release assets, so they are
 // tried first, with the generic package as the fallback for a plugin whose
 // release predates per-platform assets.
-func downloadPluginPackageForTier(ctx context.Context, name, version, repository, binaryName string, paid bool) (string, error) {
+func downloadPluginPackageForTier(ctx context.Context, name, version, repository, binaryName string, paid bool) (string, string, error) {
 	// Platform-specific package first, for a plugin that provides a command.
 	if binaryName != "" {
 		if platform, err := PlatformArch(); err == nil {
@@ -71,7 +85,7 @@ func downloadPluginPackageForTier(ctx context.Context, name, version, repository
 			}
 			platformURL := binaryPluginDownloadURL(strings.TrimSuffix(repo, ".git"), name, version, platform)
 			if tmp, err := downloadFromURL(ctx, platformURL, nil); err == nil {
-				return tmp, nil
+				return tmp, platform, nil
 			}
 			// Fall through: no per-platform asset for this release.
 		}
@@ -91,7 +105,7 @@ func downloadPluginPackageForTier(ctx context.Context, name, version, repository
 
 	tmp, err := downloadFromURL(ctx, primaryURL, extraHeaders)
 	if err == nil {
-		return tmp, nil
+		return tmp, ArtifactKindSource, nil
 	}
 
 	// If not a paid plugin, attempt GitHub Releases fallback on primary failure.
@@ -100,13 +114,13 @@ func downloadPluginPackageForTier(ctx context.Context, name, version, repository
 		if fallbackURL != primaryURL {
 			tmp2, fallbackErr := downloadFromURL(ctx, fallbackURL, nil)
 			if fallbackErr == nil {
-				return tmp2, nil
+				return tmp2, ArtifactKindSource, nil
 			}
-			return "", fmt.Errorf("download failed: primary %s: %w; fallback %s: %v", primaryURL, err, fallbackURL, fallbackErr)
+			return "", "", fmt.Errorf("download failed: primary %s: %w; fallback %s: %v", primaryURL, err, fallbackURL, fallbackErr)
 		}
 	}
 
-	return "", err
+	return "", "", err
 }
 
 // downloadFromURL fetches a single URL to a temp file and returns the file path.

internal/plugin/download_platform_test.go

@@ -31,16 +31,19 @@ func TestDownloadPluginPackagePrefersPlatformAsset(t *testing.T) {
 		name       string
 		binaryName string
 		wantPath   string
+		wantKind   string
 	}{
 		{
 			name:       "cli plugin asks for its platform build",
 			binaryName: "nself-example",
 			wantPath:   "/releases/download/v1.0.0/example-1.0.0-" + platform + ".tar.gz",
+			wantKind:   platform,
 		},
 		{
 			name:       "service plugin uses the generic package",
 			binaryName: "",
 			wantPath:   "/plugins/example/tarball",
+			wantKind:   ArtifactKindSource,
 		},
 	}
 
@@ -62,12 +65,16 @@ func TestDownloadPluginPackagePrefersPlatformAsset(t *testing.T) {
 
 			t.Setenv("NSELF_PLUGIN_REGISTRY", srv.URL)
 
-			path, err := downloadPluginPackage(context.Background(), "example", "1.0.0", srv.URL, tt.binaryName)
+			path, kind, err := downloadPluginPackage(context.Background(), "example", "1.0.0", srv.URL, tt.binaryName)
 			if err != nil {
 				t.Fatalf("download: %v", err)
 			}
 			_ = path
 
+			if kind != tt.wantKind {
+				t.Errorf("artifact kind = %q, want %q", kind, tt.wantKind)
+			}
+
 			mu.Lock()
 			defer mu.Unlock()
 			if len(seen) == 0 {
@@ -106,9 +113,13 @@ func TestDownloadPluginPackageFallsBackWhenNoPlatformAsset(t *testing.T) {
 
 	t.Setenv("NSELF_PLUGIN_REGISTRY", srv.URL)
 
-	if _, err := downloadPluginPackage(context.Background(), "example", "1.0.0", srv.URL, "nself-example"); err != nil {
+	_, kind, err := downloadPluginPackage(context.Background(), "example", "1.0.0", srv.URL, "nself-example")
+	if err != nil {
 		t.Fatalf("expected fallback to succeed, got: %v", err)
 	}
+	if kind != ArtifactKindSource {
+		t.Errorf("artifact kind = %q, want %q (fell back to the generic package)", kind, ArtifactKindSource)
+	}
 
 	mu.Lock()
 	defer mu.Unlock()

internal/plugin/installer_locked.go

@@ -153,33 +153,35 @@ func installLocked(ctx context.Context, cfg *config.Config, name string, pluginD
 		fmt.Fprintf(os.Stderr, "  ✓ %s installed\n", dep)
 	}
 
-	// Step 4: Download the plugin archive.
-	// A plugin that provides a command needs a package built for this platform;
-	// one that does not is source and works anywhere. cliBinaryName returns ""
-	// for the latter, which is most plugins.
-	// A plugin providing a command needs a package built for this platform; one
-	// that does not is source and works anywhere.
+	// Step 4: Download the plugin archive. A plugin that provides a command
+	// needs a package built for this platform; one that does not is source
+	// and works anywhere. cliBinaryName returns "" for the latter (most plugins).
 	var firstBinary string
 	if names := cliBinaryNames(name, manifest); len(names) > 0 {
 		firstBinary = names[0]
 	}
 	// Tier comes from the registry manifest, not the paidPlugins name map,
 	// which has drifted — see isPaidPluginManifest in license.go.
 	paid := isPaidPluginManifest(manifest)
-	archivePath, err := downloadPluginPackageForTier(ctx, name, manifest.Version, manifest.Repository, firstBinary, paid)
+	archivePath, artifactKind, err := downloadPluginPackageForTier(ctx, name, manifest.Version, manifest.Repository, firstBinary, paid)
 	if err != nil {
 		return fmt.Errorf("downloading plugin %q: %w", name, err)
 	}
 	defer func() { _ = os.Remove(archivePath) }()
 
-	// Step 5: Verify checksum before extraction.
-	// A checksum that IS present and wrong always refuses the install. A
-	// MISSING checksum only refuses when NSELF_PLUGIN_REQUIRE_CHECKSUM=1 is
-	// set (default: warn and proceed, for every publishStatus including an
-	// effectively-stable one — registry coverage is 47/177 as of 2026-09-04,
-	// see verifyChecksum's doc comment; FIX-CLI-6).
-	if manifest.Checksum != "" {
-		if err := verifyChecksum(archivePath, manifest.Checksum, manifest.PublishStatus); err != nil {
+	// Step 5: Verify checksum before extraction — resolveArtifactChecksum
+	// picks the checksum matching the artifact Step 4 downloaded.
+	expectedChecksum, err := resolveArtifactChecksum(*manifest, artifactKind)
+	if err != nil {
+		_ = os.Remove(archivePath)
+		return err
+	}
+
+	// A present-but-wrong checksum always refuses; an empty one here is
+	// always the SOURCE artifact's — see verifyChecksum's FIX-CLI-6 doc
+	// comment (a missing platform checksum already refused above).
+	if expectedChecksum != "" {
+		if err := verifyChecksum(archivePath, expectedChecksum, manifest.PublishStatus); err != nil {
 			_ = os.Remove(archivePath)
 			return fmt.Errorf("checksum verification for plugin %q: %w", name, err)
 		}