fix: close three gaps in the plugin gateway boundary

internal/server/server_plugin_assets.go

@@ -69,6 +69,15 @@ func (s *Server) resolvePluginAssetRequest(r *http.Request) (resolvedPluginAsset
 
 func (s *Server) servePluginAsset(w http.ResponseWriter, r *http.Request, asset resolvedPluginAsset) {
 	loaded, assetPath := asset.loaded, asset.assetPath
+	// The confining policy is derived from the configured public URL. Without it there
+	// is no trustworthy origin to name as a script source — and the only alternative,
+	// the caller's Host header, would let the request shape the policy meant to confine
+	// it. Fail closed rather than serve executable plugin code under a weaker policy.
+	if canonicalPluginAssetOrigin(s.publicURL) == "" {
+		writeError(w, http.StatusServiceUnavailable,
+			errors.New("plugin assets are not available (server public URL unset)"))
+		return
+	}
 	if strings.EqualFold(filepath.Ext(assetPath), ".html") && !asset.isEntrypoint {
 		http.NotFound(w, r)
 		return
@@ -91,7 +100,7 @@ func (s *Server) servePluginAsset(w http.ResponseWriter, r *http.Request, asset
 
 	w.Header().Set("Content-Type", contentType)
 	w.Header().Set("X-Content-Type-Options", "nosniff")
-	w.Header().Set("Content-Security-Policy", pluginAssetCSP(s.publicURL, pluginAssetRequestOrigin(r)))
+	w.Header().Set("Content-Security-Policy", pluginAssetCSP(s.publicURL))
 	if !asset.isEntrypoint {
 		// The sandboxed entrypoint has an opaque origin, so its external scripts
 		// and styles use credentialless CORS. These resources remain bound to an
@@ -109,31 +118,25 @@ func (s *Server) servePluginAsset(w http.ResponseWriter, r *http.Request, asset
 	http.ServeContent(w, r, filepath.Base(assetPath), time.Time{}, bytes.NewReader(data))
 }
 
-func pluginAssetCSP(publicURL, requestOrigin string) string {
+// pluginAssetCSP builds the per-route policy for a sandboxed plugin document. The
+// allowed origin is taken only from the server's configured public URL: a request's
+// Host header is caller-controlled, and letting it name a script source would make
+// the policy that confines plugin code depend on the request it is confining.
+// Serving is refused outright when the public URL is unset (see servePluginAsset),
+// so there is no request-derived fallback to reach.
+func pluginAssetCSP(publicURL string) string {
 	assetSources := "'self'"
-	origin := canonicalPluginAssetOrigin(publicURL)
-	if origin == "" {
-		origin = canonicalPluginAssetOrigin(requestOrigin)
-	}
-	if origin != "" {
+	if origin := canonicalPluginAssetOrigin(publicURL); origin != "" {
 		// Sandboxed plugin documents intentionally have an opaque origin. Their
 		// signed external assets therefore need the configured control-plane
 		// origin in addition to 'self', which does not match from an opaque origin.
 		assetSources += " " + origin
 	}
 	return "default-src 'none'; base-uri 'none'; form-action 'none'; frame-ancestors 'self'; " +
-		"object-src 'none'; style-src " + assetSources + "; script-src " + assetSources + "; " +
+		"object-src 'none'; frame-src 'none'; style-src " + assetSources + "; script-src " + assetSources + "; " +
 		"img-src " + assetSources + " data:; font-src " + assetSources + "; connect-src 'none'"
 }
 
-func pluginAssetRequestOrigin(r *http.Request) string {
-	scheme := "http"
-	if r.TLS != nil {
-		scheme = "https"
-	}
-	return scheme + "://" + r.Host
-}
-
 func canonicalPluginAssetOrigin(raw string) string {
 	if raw == "" {
 		return ""

internal/server/server_plugin_assets_test.go

@@ -317,7 +317,7 @@ func TestPluginAssetHeadersCacheAndPathValidation(t *testing.T) {
 }
 
 func TestPluginAssetCSPOnlyAcceptsCanonicalHTTPOrigins(t *testing.T) {
-	valid := pluginAssetCSP("https://lattice.example.test:8443", "http://ignored.test")
+	valid := pluginAssetCSP("https://lattice.example.test:8443")
 	if !strings.Contains(valid, "script-src 'self' https://lattice.example.test:8443") ||
 		!strings.Contains(valid, "img-src 'self' https://lattice.example.test:8443 data:") {
 		t.Fatalf("valid public origin missing from CSP: %q", valid)
@@ -330,14 +330,25 @@ func TestPluginAssetCSPOnlyAcceptsCanonicalHTTPOrigins(t *testing.T) {
 		"https://lattice.example.test/#fragment",
 		"https://lattice.example.test;script-src.example",
 	} {
-		csp := pluginAssetCSP(invalid, "")
+		csp := pluginAssetCSP(invalid)
 		if strings.Contains(csp, invalid) || !strings.Contains(csp, "script-src 'self';") {
 			t.Fatalf("invalid public URL %q affected CSP: %q", invalid, csp)
 		}
 	}
+}
 
-	fallback := pluginAssetCSP("", "http://127.0.0.1:8088")
-	if !strings.Contains(fallback, "script-src 'self' http://127.0.0.1:8088") {
-		t.Fatalf("request origin fallback missing from CSP: %q", fallback)
+// The policy that confines plugin code must never be derived from the request being
+// confined: a caller-controlled Host header could otherwise name a script source.
+// Asset serving fails closed when the public URL is unset, so no request-derived
+// origin can reach the CSP.
+func TestPluginAssetCSPIgnoresRequestOrigin(t *testing.T) {
+	csp := pluginAssetCSP("")
+	if !strings.Contains(csp, "script-src 'self';") {
+		t.Fatalf("unset public URL must yield a bare 'self' script-src: %q", csp)
+	}
+	for _, host := range []string{"127.0.0.1:8088", "evil.test", "lattice.example.test"} {
+		if strings.Contains(csp, host) {
+			t.Fatalf("request-derived host %q leaked into CSP: %q", host, csp)
+		}
 	}
 }

internal/server/server_plugin_invoke.go

@@ -6,6 +6,8 @@ import (
 	"errors"
 	"fmt"
 	"net/http"
+	"net/url"
+	"strconv"
 	"strings"
 	"time"
 
@@ -351,13 +353,37 @@ func extractOperatorTargets(payload json.RawMessage, fields []string) ([]string,
 		}
 		target = strings.TrimSpace(target)
 		if err := outbound.GuardOperatorURL(target); err != nil {
-			return nil, fmt.Errorf("operator target field %q is invalid: %w", field, err)
+			return nil, fmt.Errorf("operator target field %q is invalid: %s", field, redactOperatorTarget(err, target))
 		}
 		targets = append(targets, target)
 	}
 	return uniqueStrings(targets), nil
 }
 
+// redactOperatorTarget keeps a guard failure's reason but strips the secret-bearing
+// target out of it. An operator target may carry its secret in the URL path, and this
+// text reaches both the audit record and the API response — the audit record must
+// never contain it.
+func redactOperatorTarget(err error, target string) string {
+	message := err.Error()
+	// *url.Error renders as `parse "<url>": <reason>` with the URL quoted and escaped,
+	// so scrubbing the raw target cannot remove it. Keep only the reason, which never
+	// carries the URL.
+	var urlErr *url.Error
+	if errors.As(err, &urlErr) && urlErr.Err != nil {
+		message = urlErr.Err.Error()
+	}
+	if target == "" {
+		return message
+	}
+	// Any other guard that echoes the target does so either raw or Go-escaped.
+	escaped := strconv.Quote(target)
+	for _, form := range []string{target, escaped, escaped[1 : len(escaped)-1]} {
+		message = strings.ReplaceAll(message, form, "[redacted]")
+	}
+	return message
+}
+
 func (s *Server) recordPluginCallAudit(p principal, pluginID, service, method string, scopes []string, decision, reason string) {
 	scope := "plugin"
 	if len(scopes) > 0 {
@@ -419,13 +445,27 @@ func pluginGatewayScopeRequiresUnrestrictedAllowlist(scope string) bool {
 	}
 }
 
-// handlePluginInvoke runs one action on an ACTIVE plugin via the runtime (the
-// Tier-2 system runner execs the artifact's {action,payload}->{ok,result}
-// protocol). This is the minimal seed of the design-10 dashboard->plugin gateway:
-// it makes plugin EXECUTION reachable (the system runner otherwise stages the
-// artifact but nothing triggers it). Gated by plugin:admin + audited. A plugin
-// that is not armed, or whose runner cannot invoke (noop), returns an error
-// rather than silently doing nothing.
+// diagnosticPluginActions is the closed set of actions reachable through the raw
+// invoke channel. Everything with an effect on domain state must go through
+// /api/plugins/call, which is the only path that enforces the manifest's
+// per-method scopes, binds operator targets to a single invocation, and (for
+// host-risk work) requires a plan and an approval.
+//
+// This list must stay closed. `call` and `plan` would bypass per-method scopes;
+// `execute` would bypass the whole plan/approval/one-time-capability binding, so
+// an operator holding only plugin:admin could apply host changes unreviewed.
+var diagnosticPluginActions = map[string]bool{
+	"describe": true,
+	"health":   true,
+}
+
+// handlePluginInvoke runs one DIAGNOSTIC action on an ACTIVE plugin via the
+// runtime (the Tier-2 system runner execs the artifact's {action,payload}->
+// {ok,result} protocol). It exists so an operator can interrogate a staged
+// artifact directly; it is not a gateway. Gated by plugin:admin, restricted to
+// diagnosticPluginActions, and audited. A plugin that is not armed, or whose
+// runner cannot invoke (noop), returns an error rather than silently doing
+// nothing.
 func (s *Server) handlePluginInvoke(w http.ResponseWriter, r *http.Request, p principal) {
 	if r.Method != http.MethodPost {
 		writeError(w, http.StatusMethodNotAllowed, errors.New("method not allowed"))
@@ -446,6 +486,15 @@ func (s *Server) handlePluginInvoke(w http.ResponseWriter, r *http.Request, p pr
 		writeError(w, http.StatusBadRequest, errors.New("id and action are required"))
 		return
 	}
+	if !diagnosticPluginActions[req.Action] {
+		s.recordPrincipalAudit(p, model.AuditEvent{
+			ID: id.New("audit"), Action: "plugin.invoke", Scope: "plugin:admin", Decision: "deny",
+			Reason:   "action is not a diagnostic action; use /api/plugins/call",
+			Metadata: map[string]string{"plugin_id": req.ID, "plugin_action": req.Action},
+		})
+		writeError(w, http.StatusForbidden, errors.New("only diagnostic actions may be invoked directly; use /api/plugins/call"))
+		return
+	}
 	if s.pluginRuntime == nil {
 		writeError(w, http.StatusServiceUnavailable, errors.New("plugin runtime unavailable"))
 		return

internal/server/server_plugin_invoke_test.go

@@ -1,6 +1,7 @@
 package server
 
 import (
+	"bytes"
 	"context"
 	"encoding/json"
 	"io"
@@ -437,3 +438,87 @@ func TestExtractOperatorTargetsRequiresDeclaredPayloadField(t *testing.T) {
 		}
 	}
 }
+
+// The raw invoke channel is gated only by plugin:admin. It must therefore never reach
+// an action with an effect on domain state: `call` and `plan` would bypass the
+// manifest's per-method scopes and operator-target binding, and `execute` would bypass
+// the plan/approval/one-time-capability binding entirely.
+func TestPluginInvokeRefusesNonDiagnosticActions(t *testing.T) {
+	pluginRoot := t.TempDir()
+	bundle := filepath.Join(pluginRoot, "test.exec")
+	if err := os.MkdirAll(bundle, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	if err := os.WriteFile(filepath.Join(bundle, "manifest.json"),
+		[]byte(`{"id":"test.exec","name":"Exec Test","type":"system","capabilities":["node:read"]}`), 0o644); err != nil {
+		t.Fatal(err)
+	}
+	// The artifact would happily answer anything; the host must refuse before it runs.
+	script := "#!/bin/sh\nread line\necho '{\"ok\":true,\"message\":\"executed\",\"result\":{\"ran\":true}}'\n"
+	if err := os.WriteFile(filepath.Join(bundle, "artifact"), []byte(script), 0o755); err != nil {
+		t.Fatal(err)
+	}
+
+	st, err := store.Open("")
+	if err != nil {
+		t.Fatal(err)
+	}
+	srv, err := New(Options{
+		Store: st, AdminPassword: testAdminPass, DisableRenewalScheduler: true,
+		PluginDir:        pluginRoot,
+		PluginRuntimeDir: t.TempDir(),
+	})
+	if err != nil {
+		t.Fatalf("New: %v", err)
+	}
+	handler := srv.Handler()
+	cookies, csrf := loginSession(t, handler)
+
+	for _, status := range []string{"installed", "active"} {
+		resp := doJSON(t, handler, http.MethodPost, "/api/plugins/lifecycle",
+			`{"id":"test.exec","status":"`+status+`"}`, cookies, csrf)
+		if resp.StatusCode != http.StatusOK {
+			t.Fatalf("lifecycle %s: %d", status, resp.StatusCode)
+		}
+		resp.Body.Close()
+	}
+
+	for _, action := range []string{"call", "plan", "execute", "migrate", "anything"} {
+		resp := doJSON(t, handler, http.MethodPost, "/api/plugins/invoke",
+			`{"id":"test.exec","action":"`+action+`"}`, cookies, csrf)
+		body, _ := io.ReadAll(resp.Body)
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusForbidden {
+			t.Fatalf("invoke %q: want 403, got %d (%s)", action, resp.StatusCode, body)
+		}
+		if bytes.Contains(body, []byte("executed")) {
+			t.Fatalf("invoke %q reached the artifact: %s", action, body)
+		}
+	}
+
+	// Diagnostics remain reachable.
+	for _, action := range []string{"describe", "health"} {
+		resp := doJSON(t, handler, http.MethodPost, "/api/plugins/invoke",
+			`{"id":"test.exec","action":"`+action+`"}`, cookies, csrf)
+		resp.Body.Close()
+		if resp.StatusCode != http.StatusOK {
+			t.Fatalf("invoke %q: want 200, got %d", action, resp.StatusCode)
+		}
+	}
+}
+
+// An operator target may carry its secret in the URL path. url.Parse errors echo the
+// URL they failed on, and that text reaches both the audit record and the API
+// response, so the guard's reason must be surfaced without the value.
+func TestOperatorTargetErrorRedactsSecret(t *testing.T) {
+	const secret = "https://sub.example.test/aVerySecretToken123/api"
+	payload := json.RawMessage(`{"base_url":"` + secret + "\x7f" + `"}`)
+
+	_, err := extractOperatorTargets(payload, []string{"base_url"})
+	if err == nil {
+		t.Fatal("want an error for a malformed operator target")
+	}
+	if strings.Contains(err.Error(), "aVerySecretToken123") {
+		t.Fatalf("operator target secret leaked into the error: %q", err)
+	}
+}