From 5fa86cfbbd0c0afc396e095f5e65b3afa434f9ed Mon Sep 17 00:00:00 2001 From: "molecule-ai[bot]" <276602405+molecule-ai[bot]@users.noreply.github.com> Date: Fri, 17 Apr 2026 16:37:45 +0000 Subject: [PATCH] =?UTF-8?q?fix(security):=20plugin=20supply=20chain=20hard?= =?UTF-8?q?ening=20=E2=80=94=20SAFE-T1102=20(#768)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two defenses against malicious plugins from uncontrolled sources: 1. **Pinned-ref enforcement** (resolveAndStage): github:// install/download specs without a # suffix are now rejected with HTTP 422. A mutable default-branch tip could change between audit and install, silently swapping in untrusted code. Override via PLUGIN_ALLOW_UNPINNED=true. 2. **SHA-256 content integrity** (installRequest.sha256): callers may supply the expected hex SHA-256 of the fetched plugin.yaml. When present, resolveAndStage verifies the digest after staging; a mismatch aborts the install with HTTP 422 and cleans up the staging dir. Updated TestPluginDownload_GithubSchemeStreamsTarball to use a pinned ref (#v1.0.0) so it reflects the new security requirement. Tests: 4 new (TestPluginInstall_SHA256Mismatch_AbortsInstall, TestPluginInstall_SHA256Match_Succeeds, TestPluginInstall_UnpinnedRef_Rejected, TestPluginInstall_PinnedRef_Accepted). All 15 packages green. Co-Authored-By: Claude Sonnet 4.6 --- .../handlers/plugins_install_pipeline.go | 46 ++++++++++ .../handlers/plugins_install_pipeline_test.go | 88 +++++++++++++++++++ platform/internal/handlers/plugins_test.go | 8 +- 3 files changed, 138 insertions(+), 4 deletions(-) diff --git a/platform/internal/handlers/plugins_install_pipeline.go b/platform/internal/handlers/plugins_install_pipeline.go index 7ccfd742..9cac22f0 100644 --- a/platform/internal/handlers/plugins_install_pipeline.go +++ b/platform/internal/handlers/plugins_install_pipeline.go @@ -4,6 +4,8 @@ import ( "archive/tar" "bytes" "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "io" @@ -108,6 +110,10 @@ func dirSize(dir string, limit int64) (int64, error) { // gin.Context; the handler just decodes into this shape. type installRequest struct { Source string `json:"source"` + // SHA256 is an optional hex-encoded SHA-256 of the plugin's plugin.yaml. + // When present, resolveAndStage verifies the fetched content matches + // before allowing the install to proceed (SAFE-T1102 supply-chain hardening). + SHA256 string `json:"sha256,omitempty"` } // stageResult bundles the outputs of resolveAndStage for the caller. @@ -151,6 +157,20 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest } } + // Pinned-ref enforcement for github:// sources (SAFE-T1102). + // An unpinned spec (no # suffix) installs from a mutable + // default-branch tip whose content can change silently between an + // audit and the actual install. Require explicit pinning unless the + // operator opts in via PLUGIN_ALLOW_UNPINNED=true. + if source.Scheme == "github" && !strings.Contains(source.Spec, "#") { + if os.Getenv("PLUGIN_ALLOW_UNPINNED") != "true" { + return nil, newHTTPErr(http.StatusUnprocessableEntity, gin.H{ + "error": `unpinned github source: append a tag or commit SHA (e.g. "github://owner/repo#v1.2.0"). Set PLUGIN_ALLOW_UNPINNED=true to override`, + "source": source.Raw(), + }) + } + } + stagedDir, err := os.MkdirTemp("", "molecule-plugin-fetch-*") if err != nil { return nil, newHTTPErr(http.StatusInternalServerError, gin.H{"error": "failed to create staging dir"}) @@ -189,6 +209,32 @@ func (h *PluginsHandler) resolveAndStage(ctx context.Context, req installRequest "source": source.Raw(), }) } + + // SHA-256 content integrity check (SAFE-T1102). + // If the caller pinned a hash, verify it against the staged plugin.yaml. + // A mismatch means the fetched content differs from what was audited — + // abort rather than silently install an unexpected plugin. + if req.SHA256 != "" { + manifestPath := filepath.Join(stagedDir, "plugin.yaml") + manifestData, readErr := os.ReadFile(manifestPath) + if readErr != nil { + cleanup() + return nil, newHTTPErr(http.StatusUnprocessableEntity, gin.H{ + "error": "sha256 check failed: plugin.yaml not found in staged plugin", + "source": source.Raw(), + }) + } + sum := sha256.Sum256(manifestData) + got := hex.EncodeToString(sum[:]) + if !strings.EqualFold(got, req.SHA256) { + cleanup() + return nil, newHTTPErr(http.StatusUnprocessableEntity, gin.H{ + "error": fmt.Sprintf("sha256 mismatch: expected %s, got %s", req.SHA256, got), + "source": source.Raw(), + }) + } + } + return &stageResult{StagedDir: stagedDir, PluginName: pluginName, Source: source}, nil } diff --git a/platform/internal/handlers/plugins_install_pipeline_test.go b/platform/internal/handlers/plugins_install_pipeline_test.go index 05eadf6a..0618c219 100644 --- a/platform/internal/handlers/plugins_install_pipeline_test.go +++ b/platform/internal/handlers/plugins_install_pipeline_test.go @@ -4,6 +4,8 @@ import ( "archive/tar" "bytes" "context" + "crypto/sha256" + "encoding/hex" "errors" "fmt" "io" @@ -505,6 +507,92 @@ func TestResolveAndStage_LocalSchemePathTraversal(t *testing.T) { assertHTTPErrStatus(t, err, http.StatusBadRequest, "local path traversal") } +// ==================== supply-chain hardening (SAFE-T1102) ==================== + +// TestPluginInstall_SHA256Mismatch_AbortsInstall verifies that when the caller +// provides a sha256 field that does not match the fetched plugin.yaml, the +// install is aborted with 422 Unprocessable Entity and the staging dir is cleaned up. +func TestPluginInstall_SHA256Mismatch_AbortsInstall(t *testing.T) { + beforeCount := tempDirCount(t) + + h := NewPluginsHandler(t.TempDir(), nil, nil).WithSourceResolver(&stubResolver{ + scheme: "stub", + name: "my-plugin", + content: "name: my-plugin\nversion: 1.0.0\n", + }) + _, err := h.resolveAndStage(context.Background(), installRequest{ + Source: "stub://my-plugin", + SHA256: "0000000000000000000000000000000000000000000000000000000000000000", // wrong + }) + assertHTTPErrStatus(t, err, http.StatusUnprocessableEntity, "sha256 mismatch") + + afterCount := tempDirCount(t) + if afterCount > beforeCount { + t.Errorf("SHA256 mismatch left %d orphaned staging dir(s)", afterCount-beforeCount) + } +} + +// TestPluginInstall_SHA256Match_Succeeds verifies that resolveAndStage succeeds +// when the caller supplies the correct SHA-256 of the fetched plugin.yaml. +func TestPluginInstall_SHA256Match_Succeeds(t *testing.T) { + content := "name: my-plugin\nversion: 1.0.0\n" + sum := sha256.Sum256([]byte(content)) + correctHash := hex.EncodeToString(sum[:]) + + h := NewPluginsHandler(t.TempDir(), nil, nil).WithSourceResolver(&stubResolver{ + scheme: "stub", + name: "my-plugin", + content: content, + }) + result, err := h.resolveAndStage(context.Background(), installRequest{ + Source: "stub://my-plugin", + SHA256: correctHash, + }) + if err != nil { + t.Fatalf("expected success when sha256 matches, got: %v", err) + } + defer os.RemoveAll(result.StagedDir) + if result.PluginName != "my-plugin" { + t.Errorf("expected PluginName 'my-plugin', got %q", result.PluginName) + } +} + +// TestPluginInstall_UnpinnedRef_Rejected verifies that a github:// spec without +// a # suffix is rejected with 422 unless PLUGIN_ALLOW_UNPINNED=true. +func TestPluginInstall_UnpinnedRef_Rejected(t *testing.T) { + t.Setenv("PLUGIN_ALLOW_UNPINNED", "") // ensure the guard is active + + h := NewPluginsHandler(t.TempDir(), nil, nil).WithSourceResolver(&stubResolver{ + scheme: "github", + name: "my-plugin", + content: "name: my-plugin\n", + }) + _, err := h.resolveAndStage(context.Background(), installRequest{ + Source: "github://owner/repo", // no #ref — must be rejected + }) + assertHTTPErrStatus(t, err, http.StatusUnprocessableEntity, "unpinned ref rejected") +} + +// TestPluginInstall_PinnedRef_Accepted verifies that a github:// spec that +// includes a # suffix passes the pinned-ref guard and completes normally. +func TestPluginInstall_PinnedRef_Accepted(t *testing.T) { + h := NewPluginsHandler(t.TempDir(), nil, nil).WithSourceResolver(&stubResolver{ + scheme: "github", + name: "my-plugin", + content: "name: my-plugin\n", + }) + result, err := h.resolveAndStage(context.Background(), installRequest{ + Source: "github://owner/repo#v1.0.0", // pinned — must be accepted + }) + if err != nil { + t.Fatalf("expected success for pinned ref, got: %v", err) + } + defer os.RemoveAll(result.StagedDir) + if result.PluginName != "my-plugin" { + t.Errorf("expected PluginName 'my-plugin', got %q", result.PluginName) + } +} + // ==================== helpers ==================== // assertHTTPErrStatus is a test helper that checks err is a *httpErr with diff --git a/platform/internal/handlers/plugins_test.go b/platform/internal/handlers/plugins_test.go index ba313eaa..8206a00a 100644 --- a/platform/internal/handlers/plugins_test.go +++ b/platform/internal/handlers/plugins_test.go @@ -1271,16 +1271,16 @@ func TestPluginDownload_GithubSchemeStreamsTarball(t *testing.T) { {Key: "name", Value: "remote-plugin"}, } req := httptest.NewRequest("GET", - "/workspaces/X/plugins/remote-plugin/download?source=github://acme/remote-plugin", nil) - req.URL.RawQuery = "source=github%3A%2F%2Facme%2Fremote-plugin" + "/workspaces/X/plugins/remote-plugin/download?source=github%3A%2F%2Facme%2Fremote-plugin%23v1.0.0", nil) + req.URL.RawQuery = "source=github%3A%2F%2Facme%2Fremote-plugin%23v1.0.0" c.Request = req h.Download(c) if w.Code != http.StatusOK { t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) } - if got := w.Header().Get("X-Plugin-Source"); got != "github://acme/remote-plugin" { - t.Errorf("X-Plugin-Source: got %q, want github://acme/remote-plugin", got) + if got := w.Header().Get("X-Plugin-Source"); got != "github://acme/remote-plugin#v1.0.0" { + t.Errorf("X-Plugin-Source: got %q, want github://acme/remote-plugin#v1.0.0", got) } // Decode + verify the tarball contains the resolver's files