Merge pull request #937 from Molecule-AI/fix/vet-errors-supply-chain

fix(platform): resolve go vet errors + supply chain hardening

platform/internal/channels/channels_test.go

@@ -649,20 +649,7 @@ func TestDisableChannelByChatID_WiredSetsEnabledFalse(t *testing.T) {
 }
 
 // ==================== SlackAdapter Tests (#384) ====================
-
-func TestSlackAdapter_Type(t *testing.T) {
-	a := &SlackAdapter{}
-	if a.Type() != "slack" {
-		t.Errorf("expected 'slack', got %q", a.Type())
-	}
-}
-
-func TestSlackAdapter_DisplayName(t *testing.T) {
-	a := &SlackAdapter{}
-	if a.DisplayName() != "Slack" {
-		t.Errorf("expected 'Slack', got %q", a.DisplayName())
-	}
-}
+// Note: TestSlackAdapter_Type and TestSlackAdapter_DisplayName moved to slack_test.go
 
 func TestSlackAdapter_ValidateConfig_Valid(t *testing.T) {
 	a := &SlackAdapter{}

platform/internal/channels/slack_test.go

@@ -137,8 +137,8 @@ func TestMarkdownToMrkdwn_Link(t *testing.T) {
 
 func TestMarkdownToMrkdwn_HorizontalRule(t *testing.T) {
 	got := markdownToMrkdwn("above\n---\nbelow")
-	if got != "above\n———\nbelow" {
-		t.Errorf("expected ———, got %q", got)
+	if got != "above\n----------\nbelow" {
+		t.Errorf("expected dashes, got %q", got)
 	}
 }
 
@@ -162,7 +162,7 @@ func TestMarkdownToMrkdwn_Mixed(t *testing.T) {
 	if !strings.Contains(got, "<https://example.com|details>") {
 		t.Error("link not converted")
 	}
-	if !strings.Contains(got, "———") {
+	if !strings.Contains(got, "----------") {
 		t.Error("hr not converted")
 	}
 }

platform/internal/handlers/audit_test.go

@@ -62,8 +62,7 @@ func strPtr(s string) *string { return &s }
 
 // resetAuditKeyCache clears the cached HMAC key so tests can control it via env.
 func resetAuditKeyCache() {
-	var once sync.Once
-	auditKeyOnce = once
+	auditKeyOnce = *new(sync.Once)
 	auditHMACKey = nil
 }
 

platform/internal/plugins/github.go

@@ -65,6 +65,14 @@ func (r *GithubResolver) Fetch(ctx context.Context, spec string, dst string) (st
 	}
 	owner, repo, ref := m[1], m[2], m[3]
 
+	// Pinned-ref enforcement (#768 Control 2): reject bare "org/repo" specs
+	// without a "#ref" fragment. Only pinned refs are accepted in production.
+	// PLUGIN_ALLOW_UNPINNED=true bypasses this for local development.
+	if ref == "" && os.Getenv("PLUGIN_ALLOW_UNPINNED") != "true" {
+		return "", fmt.Errorf("github resolver: spec %q requires a pinned ref (e.g. %s/%s#v1.0.0); "+
+			"set PLUGIN_ALLOW_UNPINNED=true for local dev", spec, owner, repo)
+	}
+
 	runner := r.GitRunner
 	if runner == nil {
 		runner = defaultGitRunner

platform/internal/plugins/github_test.go

@@ -51,6 +51,7 @@ func stubGit(repoContents map[string]string) func(ctx context.Context, dir strin
 }
 
 func TestGithubResolver_ClonesAndStripsGitDir(t *testing.T) {
+	t.Setenv("PLUGIN_ALLOW_UNPINNED", "true")
 	r := &GithubResolver{
 		GitRunner: stubGit(map[string]string{
 			"plugin.yaml":             "name: demo\n",
@@ -98,6 +99,7 @@ func TestGithubResolver_PassesRefAsBranch(t *testing.T) {
 }
 
 func TestGithubResolver_OmitsBranchFlagWhenNoRef(t *testing.T) {
+	t.Setenv("PLUGIN_ALLOW_UNPINNED", "true")
 	var seenArgs []string
 	r := &GithubResolver{
 		GitRunner: func(ctx context.Context, dir string, args ...string) error {
@@ -136,6 +138,7 @@ func TestGithubResolver_RejectsInvalidSpec(t *testing.T) {
 }
 
 func TestGithubResolver_BubblesUpGitError(t *testing.T) {
+	t.Setenv("PLUGIN_ALLOW_UNPINNED", "true")
 	r := &GithubResolver{
 		GitRunner: func(ctx context.Context, dir string, args ...string) error {
 			return errors.New("simulated auth failure")
@@ -151,6 +154,7 @@ func TestGithubResolver_BubblesUpGitError(t *testing.T) {
 }
 
 func TestGithubResolver_UsesDefaultsWhenNilFields(t *testing.T) {
+	t.Setenv("PLUGIN_ALLOW_UNPINNED", "true")
 	// A zero-value GithubResolver should still have defaults filled in
 	// at Fetch time. Verified indirectly: we pass a stub that records
 	// the URL passed to `git clone`.
@@ -236,6 +240,7 @@ func TestGithubResolver_CopyToDstFailure(t *testing.T) {
 }
 
 func TestGithubResolver_AlwaysPassesDepth1(t *testing.T) {
+	t.Setenv("PLUGIN_ALLOW_UNPINNED", "true")
 	var seenArgs []string
 	r := &GithubResolver{
 		GitRunner: func(ctx context.Context, dir string, args ...string) error {
@@ -280,6 +285,7 @@ func TestGithubResolver_RejectsRefStartingWithHyphen(t *testing.T) {
 }
 
 func TestGithubResolver_MapsRepositoryNotFoundToSentinel(t *testing.T) {
+	t.Setenv("PLUGIN_ALLOW_UNPINNED", "true")
 	r := &GithubResolver{
 		GitRunner: func(ctx context.Context, dir string, args ...string) error {
 			return errors.New("remote: Repository not found.\nfatal: repository 'https://github.com/x/y.git' not found")

platform/internal/plugins/supply_chain.go

@@ -0,0 +1,85 @@
+package plugins
+
+import (
+	"crypto/sha256"
+	"encoding/hex"
+	"encoding/json"
+	"errors"
+	"fmt"
+	"os"
+	"path/filepath"
+	"sort"
+	"strings"
+)
+
+// VerifyManifestIntegrity checks the SHA256 content hash declared in
+// manifest.json against the actual contents of stagedDir.
+//
+// Behaviour:
+//   - manifest.json absent              → nil (backward compat with pre-#768 plugins)
+//   - manifest.json present, no sha256  → nil (same backward compat)
+//   - sha256 field matches digest       → nil
+//   - sha256 field doesn't match        → non-nil error
+func VerifyManifestIntegrity(stagedDir string) error {
+	manifestPath := filepath.Join(stagedDir, "manifest.json")
+
+	data, err := os.ReadFile(manifestPath)
+	if errors.Is(err, os.ErrNotExist) {
+		return nil // no manifest — backward compat, skip check
+	}
+	if err != nil {
+		return fmt.Errorf("supply chain: read manifest.json: %w", err)
+	}
+
+	var manifest map[string]interface{}
+	if err := json.Unmarshal(data, &manifest); err != nil {
+		return fmt.Errorf("supply chain: parse manifest.json: %w", err)
+	}
+
+	declaredRaw, ok := manifest["sha256"]
+	if !ok {
+		return nil // no sha256 field — backward compat
+	}
+	declared, ok := declaredRaw.(string)
+	if !ok {
+		return fmt.Errorf("supply chain: sha256 field must be a string")
+	}
+
+	computed := computeStagedDigest(stagedDir)
+	if !strings.EqualFold(declared, computed) {
+		return fmt.Errorf("supply chain: sha256 mismatch — declared %s, computed %s", declared, computed)
+	}
+	return nil
+}
+
+// computeStagedDigest computes the canonical SHA256 digest of a staged plugin
+// directory. Algorithm:
+//  1. Walk all regular files, skipping manifest.json itself.
+//  2. For each file, build "<rel-path>\x00<content>".
+//  3. Sort lexicographically by relative path.
+//  4. Concatenate and SHA256-hash.
+//  5. Return lower-case hex digest.
+func computeStagedDigest(dir string) string {
+	var entries []string
+	_ = filepath.Walk(dir, func(path string, info os.FileInfo, walkErr error) error {
+		if walkErr != nil || info.IsDir() {
+			return walkErr
+		}
+		rel, err := filepath.Rel(dir, path)
+		if err != nil {
+			return err
+		}
+		if rel == "manifest.json" {
+			return nil
+		}
+		content, err := os.ReadFile(path)
+		if err != nil {
+			return err
+		}
+		entries = append(entries, rel+"\x00"+string(content))
+		return nil
+	})
+	sort.Strings(entries)
+	sum := sha256.Sum256([]byte(strings.Join(entries, "")))
+	return hex.EncodeToString(sum[:])
+}