diff --git a/internal/server/server_plugin_assets.go b/internal/server/server_plugin_assets.go index 7c4d1a1..1bc4073 100644 --- a/internal/server/server_plugin_assets.go +++ b/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 "" diff --git a/internal/server/server_plugin_assets_test.go b/internal/server/server_plugin_assets_test.go index b8bf724..ef08404 100644 --- a/internal/server/server_plugin_assets_test.go +++ b/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) + } } } diff --git a/internal/server/server_plugin_invoke.go b/internal/server/server_plugin_invoke.go index a4c4ef7..7191015 100644 --- a/internal/server/server_plugin_invoke.go +++ b/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 "": ` 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 diff --git a/internal/server/server_plugin_invoke_test.go b/internal/server/server_plugin_invoke_test.go index 25b98e0..b83ca4c 100644 --- a/internal/server/server_plugin_invoke_test.go +++ b/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) + } +}