diff --git a/internal/plugin/checksum_artifact_test.go b/internal/plugin/checksum_artifact_test.go new file mode 100644 index 00000000..f8773c32 --- /dev/null +++ b/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") + } +} diff --git a/internal/plugin/download.go b/internal/plugin/download.go index a6042221..9189fb4d 100644 --- a/internal/plugin/download.go +++ b/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. diff --git a/internal/plugin/download_platform_test.go b/internal/plugin/download_platform_test.go index debef889..0806fc12 100644 --- a/internal/plugin/download_platform_test.go +++ b/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() diff --git a/internal/plugin/installer_locked.go b/internal/plugin/installer_locked.go index ecdb630d..62d12ccb 100644 --- a/internal/plugin/installer_locked.go +++ b/internal/plugin/installer_locked.go @@ -153,12 +153,9 @@ 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] @@ -166,20 +163,25 @@ func installLocked(ctx context.Context, cfg *config.Config, name string, pluginD // 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) } diff --git a/internal/plugin/interfaces.go b/internal/plugin/interfaces.go index 90eb9cc8..55d3fd2a 100644 --- a/internal/plugin/interfaces.go +++ b/internal/plugin/interfaces.go @@ -240,4 +240,25 @@ type PluginManifest struct { // --detailed` displays it when present (registry entry or local // plugin.json) and shows nothing invented when it is not. UpdatedAt string `json:"updated_at,omitempty"` + + // PlatformChecksums holds one SHA-256 checksum (lowercase hex, no + // "sha256:" prefix) per per-platform binary tarball, keyed by the exact + // platform string PlatformArch() returns (darwin-arm64, darwin-amd64, + // linux-amd64, linux-arm64, windows-amd64). Populated from the registry's + // nested `checksums.platforms` object. + // + // Checksum above is ALWAYS the source tarball's checksum, never a + // platform one — the two packages have different bytes, so one flat + // field can never validate both (FIX-CLI-plugins-83: a platform tarball + // downloaded via binaryPluginDownloadURL was being checked against + // Checksum, which could never match). downloadPluginPackageForTier + // reports which artifact it fetched; installLocked picks the matching + // entry from this map, or Checksum for the source artifact — see + // verifyChecksum's caller in installer_locked.go. + // + // Only present for a plugin with a binaryName. A plugin whose release + // predates this field (or a platform the release never built a checksum + // for) has no entry here — see downloadPluginPackageForTier's doc + // comment for what happens then. + PlatformChecksums map[string]string `json:"platform_checksums,omitempty"` } diff --git a/internal/plugin/registry_cache.go b/internal/plugin/registry_cache.go index 481bfa25..20104353 100644 --- a/internal/plugin/registry_cache.go +++ b/internal/plugin/registry_cache.go @@ -101,6 +101,18 @@ func (r Registry) MarshalJSON() ([]byte, error) { } rawDeps = b } + // Checksums re-serialises PlatformChecksums into the nested registry + // shape, matching how it was parsed in — see pluginChecksumsEntry. + // SHA256 is left empty: the cache's source-tarball checksum lives in + // the flat Checksum field below (and always has), so re-populating + // the nested duplicate here would just be inventing a value never + // read back by anything (entryToManifest sources PlatformChecksums + // from Checksums.Platforms only). + var checksums *pluginChecksumsEntry + if len(p.PlatformChecksums) > 0 { + checksums = &pluginChecksumsEntry{Platforms: p.PlatformChecksums} + } + entries = append(entries, pluginEntry{ Name: p.Name, Version: p.Version, @@ -112,6 +124,7 @@ func (r Registry) MarshalJSON() ([]byte, error) { Tier: p.Tier, Repository: p.Repository, Checksum: p.Checksum, + Checksums: checksums, Tags: p.Tags, Tables: p.Tables, Port: p.Port, diff --git a/internal/plugin/registry_json.go b/internal/plugin/registry_json.go index 0b6dc9cf..8e6bbfa2 100644 --- a/internal/plugin/registry_json.go +++ b/internal/plugin/registry_json.go @@ -130,29 +130,35 @@ func entryToManifest(e pluginEntry) PluginManifest { implFramework = e.Implementation.Framework } + var platformChecksums map[string]string + if e.Checksums != nil { + platformChecksums = normalizePlatformChecksums(e.Checksums.Platforms) + } + return PluginManifest{ - Name: e.Name, - Version: e.Version, - Description: e.Description, - Category: e.Category, - License: e.License, - LicenseType: e.LicenseType, - Tier: tier, - Repository: e.Repository, - Checksum: e.Checksum, - Tags: e.Tags, - RequiresLicense: e.RequiresLicense, - Tables: e.Tables, - Port: port, - TierPair: e.TierPair, - Bundles: e.Bundles, - Dependencies: parseDependencies(e.Dependencies), - APIEndpoints: parseAPIEndpoints(e.APIEndpoints), - Language: language, - Runtime: runtime, - PluginType: pluginType, - BinaryName: binaryName, - CLICommands: e.CLICommands, + Name: e.Name, + Version: e.Version, + Description: e.Description, + Category: e.Category, + License: e.License, + LicenseType: e.LicenseType, + Tier: tier, + Repository: e.Repository, + Checksum: e.Checksum, + PlatformChecksums: platformChecksums, + Tags: e.Tags, + RequiresLicense: e.RequiresLicense, + Tables: e.Tables, + Port: port, + TierPair: e.TierPair, + Bundles: e.Bundles, + Dependencies: parseDependencies(e.Dependencies), + APIEndpoints: parseAPIEndpoints(e.APIEndpoints), + Language: language, + Runtime: runtime, + PluginType: pluginType, + BinaryName: binaryName, + CLICommands: e.CLICommands, Author: e.Author, Homepage: e.Homepage, diff --git a/internal/plugin/registry_parse.go b/internal/plugin/registry_parse.go index 5b936bc9..8748d2f4 100644 --- a/internal/plugin/registry_parse.go +++ b/internal/plugin/registry_parse.go @@ -62,26 +62,42 @@ type pluginEndpointEntry struct { Description string `json:"description"` } +// pluginChecksumsEntry holds the registry's nested `checksums` object. +// `sha256` duplicates the flat `checksum` field (source tarball, kept for +// backwards compatibility with older registry readers); `platforms` carries +// one checksum per per-platform binary tarball, keyed by the exact platform +// string internal/plugin/arch.go's PlatformArch() returns. Only present for +// a plugin with a binaryName — see PluginManifest.PlatformChecksums. +type pluginChecksumsEntry struct { + SHA256 string `json:"sha256,omitempty"` + Platforms map[string]string `json:"platforms,omitempty"` +} + // pluginEntry matches the fields present in the array-format (pro) // registry as well as the object-format (free) registry. // APIEndpoints is kept as json.RawMessage because the live registry returns // it as an array of objects while older/local registries use an array of // strings. We normalise both into []string during entryToManifest conversion. type pluginEntry struct { - Name string `json:"name"` - Version string `json:"version"` - Description string `json:"description"` - Category string `json:"category"` - Tier string `json:"tier"` - License string `json:"license"` - LicenseType string `json:"licenseType"` - Repository string `json:"repository"` - Checksum string `json:"checksum"` - DownloadURL string `json:"download_url"` - RequiresLicense bool `json:"requires_license"` - Tags []string `json:"tags"` - Tables []string `json:"tables,omitempty"` - Port int `json:"port,omitempty"` + Name string `json:"name"` + Version string `json:"version"` + Description string `json:"description"` + Category string `json:"category"` + Tier string `json:"tier"` + License string `json:"license"` + LicenseType string `json:"licenseType"` + Repository string `json:"repository"` + Checksum string `json:"checksum"` + // Checksums is the nested form carrying per-platform binary tarball + // checksums (checksums.platforms) alongside the source-tarball checksum + // (checksums.sha256, a duplicate of the flat Checksum field above). See + // pluginChecksumsEntry and PluginManifest.PlatformChecksums. + Checksums *pluginChecksumsEntry `json:"checksums,omitempty"` + DownloadURL string `json:"download_url"` + RequiresLicense bool `json:"requires_license"` + Tags []string `json:"tags"` + Tables []string `json:"tables,omitempty"` + Port int `json:"port,omitempty"` // TierPair and Bundles support install-time tier resolution for a slug // served twice (free + pro) as one product — see PluginManifest's doc // comments (interfaces.go) and tier_resolve.go. @@ -205,6 +221,27 @@ func parseAPIEndpoints(raw json.RawMessage) []string { return out } +// normalizePlatformChecksums copies a registry's checksums.platforms map, +// stripping an optional "sha256:" prefix from each value. The plugins repo +// writes the sibling checksums.sha256 field with that prefix (see +// release-tarballs.yml's backfill step); platform checksums are documented +// to be written as plain hex, but stripping a stray prefix defensively here +// costs nothing and avoids a checksum that LOOKS present but can never +// match verifyChecksum's raw hex comparison — which would otherwise surface +// as an install-time checksum mismatch instead of the registry data error it +// actually is. Returns nil for an empty/nil input, matching the omitempty +// contract used throughout this package. +func normalizePlatformChecksums(raw map[string]string) map[string]string { + if len(raw) == 0 { + return nil + } + out := make(map[string]string, len(raw)) + for platform, checksum := range raw { + out[platform] = strings.TrimPrefix(strings.ToLower(strings.TrimSpace(checksum)), "sha256:") + } + return out +} + // parseDependencies extracts the list of plugin-name dependencies from the // raw `dependencies` JSON value. The registry has shipped several shapes over // time, so we accept all of them and ignore non-plugin keys (npm/system/python) diff --git a/internal/plugin/security.go b/internal/plugin/security.go index fd20afa2..fd04bfcc 100644 --- a/internal/plugin/security.go +++ b/internal/plugin/security.go @@ -116,6 +116,44 @@ func verifyChecksum(filePath string, expectedHash string, publishStatus string) return nil } +// resolveArtifactChecksum picks the registry checksum that matches the +// artifact downloadPluginPackageForTier actually fetched, identified by +// artifactKind (ArtifactKindSource, or a platform string from PlatformArch()). +// +// The source tarball and each platform's binary tarball are different bytes +// for the same plugin+version, so a source checksum can never validate a +// platform download or vice versa — comparing the wrong one is not a +// stricter check, it is a guaranteed-wrong one (plugins#83, the bug this +// function exists to close). +// +// For the source artifact this returns manifest.Checksum verbatim, including +// when it is empty — verifyChecksum's own warn-and-proceed leniency for a +// missing SOURCE checksum (FIX-CLI-6, a documented and tracked registry +// coverage gap) still applies, unchanged, to whatever this returns. +// +// For a platform artifact, a missing checksum in +// manifest.PlatformChecksums[artifactKind] is refused HERE, unconditionally +// — it never reaches verifyChecksum's lenient empty-string path, and no env +// var (NSELF_PLUGIN_REQUIRE_CHECKSUM included) changes that. The FIX-CLI-6 +// leniency exists for the source-checksum coverage gap; it was never a +// license to install an actual downloaded EXECUTABLE with zero +// verification. A release that predates PlatformChecksums, or one platform's +// checksum that was never backfilled, are both registry data gaps — the fix +// is a registry checksum, not a flag that makes the install proceed anyway. +func resolveArtifactChecksum(manifest PluginManifest, artifactKind string) (string, error) { + if artifactKind == ArtifactKindSource || artifactKind == "" { + return manifest.Checksum, nil + } + + checksum, ok := manifest.PlatformChecksums[artifactKind] + if !ok || checksum == "" { + return "", fmt.Errorf( + "plugin %q: registry has no checksum for platform %q at version %s — refusing to install an unverified binary (report to nself-org/plugins)", + manifest.Name, artifactKind, manifest.Version) + } + return checksum, nil +} + // verifyPluginSignature verifies that the Ed25519 signature stored in the // plugin's registry manifest matches the SHA-256 hash of the downloaded // tarball. The public key is pinned in the registry (never fetched at verify