Log panics originating from built-in plugins

pkg/common/catalog/builtin.go

@@ -62,7 +62,7 @@ func loadBuiltIn(ctx context.Context, builtIn BuiltIn, config BuiltInConfig) (_
 	}()
 	closers = append(closers, dialer)
 
-	builtinServer, serverCloser := newBuiltInServer()
+	builtinServer, serverCloser := newBuiltInServer(config.Log)
 	closers = append(closers, serverCloser)
 
 	pluginServers := append([]pluginsdk.ServiceServer{builtIn.Plugin}, builtIn.Services...)
@@ -83,11 +83,11 @@ func loadBuiltIn(ctx context.Context, builtIn BuiltIn, config BuiltInConfig) (_
 	return newPlugin(ctx, builtinConn, info, config.Log, closers, config.HostServices)
 }
 
-func newBuiltInServer() (*grpc.Server, io.Closer) {
+func newBuiltInServer(log logrus.FieldLogger) (*grpc.Server, io.Closer) {
 	drain := &drainHandlers{}
 	return grpc.NewServer(
-		grpc.ChainStreamInterceptor(drain.StreamServerInterceptor, streamPanicInterceptor),
-		grpc.ChainUnaryInterceptor(drain.UnaryServerInterceptor, unaryPanicInterceptor),
+		grpc.ChainStreamInterceptor(drain.StreamServerInterceptor, streamPanicInterceptor(log)),
+		grpc.ChainUnaryInterceptor(drain.UnaryServerInterceptor, unaryPanicInterceptor(log)),
 	), closerFunc(drain.Wait)
 }
 
@@ -102,7 +102,7 @@ func (d *builtinDialer) DialHost(context.Context) (grpc.ClientConnInterface, err
 	if d.conn != nil {
 		return d.conn, nil
 	}
-	server := newHostServer(d.pluginName, d.hostServices)
+	server := newHostServer(d.log, d.pluginName, d.hostServices)
 	conn, err := startPipeServer(server, d.log)
 	if err != nil {
 		return nil, err

pkg/common/catalog/external.go

@@ -157,7 +157,7 @@ func (p *hcClientPlugin) GRPCClient(ctx context.Context, b *goplugin.GRPCBroker,
 		return nil, errs.Wrap(err)
 	}
 
-	server := newHostServer(p.config.Name, p.config.HostServices)
+	server := newHostServer(p.config.Log, p.config.Name, p.config.HostServices)
 
 	var wg sync.WaitGroup
 	wg.Add(1)

pkg/common/catalog/host.go

@@ -2,21 +2,24 @@ package catalog
 
 import (
 	"context"
+	"fmt"
+	"runtime/debug"
 
+	"github.com/sirupsen/logrus"
 	"github.com/spiffe/spire-plugin-sdk/pluginsdk"
 	"google.golang.org/grpc"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
 )
 
-func newHostServer(pluginName string, hostServices []pluginsdk.ServiceServer) *grpc.Server {
+func newHostServer(log logrus.FieldLogger, pluginName string, hostServices []pluginsdk.ServiceServer) *grpc.Server {
 	s := grpc.NewServer(
 		grpc.ChainStreamInterceptor(
-			streamPanicInterceptor,
+			streamPanicInterceptor(log),
 			streamPluginInterceptor(pluginName),
 		),
 		grpc.ChainUnaryInterceptor(
-			unaryPanicInterceptor,
+			unaryPanicInterceptor(log),
 			unaryPluginInterceptor(pluginName),
 		),
 	)
@@ -38,22 +41,34 @@ func unaryPluginInterceptor(name string) grpc.UnaryServerInterceptor {
 	}
 }
 
-func streamPanicInterceptor(srv any, ss grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) (err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			err = status.Errorf(codes.Internal, "%s", r)
-		}
-	}()
-	return handler(srv, ss)
+func streamPanicInterceptor(log logrus.FieldLogger) grpc.StreamServerInterceptor {
+	return func(srv any, ss grpc.ServerStream, _ *grpc.StreamServerInfo, handler grpc.StreamHandler) (err error) {
+		defer func() {
+			if r := recover(); r != nil {
+				err = convertPanic(log, r)
+			}
+		}()
+		return handler(srv, ss)
+	}
+}
+
+func unaryPanicInterceptor(log logrus.FieldLogger) grpc.UnaryServerInterceptor {
+	return func(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (_ any, err error) {
+		defer func() {
+			if r := recover(); r != nil {
+				err = convertPanic(log, r)
+			}
+		}()
+		return handler(ctx, req)
+	}
 }
 
-func unaryPanicInterceptor(ctx context.Context, req any, _ *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (_ any, err error) {
-	defer func() {
-		if r := recover(); r != nil {
-			err = status.Errorf(codes.Internal, "%s", r)
-		}
-	}()
-	return handler(ctx, req)
+func convertPanic(log logrus.FieldLogger, r any) error {
+	log.WithFields(logrus.Fields{
+		"cause": fmt.Sprint(r),
+		"stack": string(debug.Stack()),
+	}).Error("Plugin panicked")
+	return status.Errorf(codes.Internal, "%s", r)
 }
 
 type streamWrapper struct {

test/plugintest/load.go

@@ -6,7 +6,6 @@ import (
 	"testing"
 
 	"github.com/sirupsen/logrus"
-	"github.com/sirupsen/logrus/hooks/test"
 	"github.com/spiffe/spire/pkg/common/catalog"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -26,7 +25,7 @@ type Plugin interface {
 func Load(t *testing.T, builtIn catalog.BuiltIn, pluginFacade catalog.Facade, options ...Option) Plugin {
 	conf := &config{
 		builtInConfig: catalog.BuiltInConfig{
-			Log: nullLogger(),
+			Log: testLogger(t),
 		},
 	}
 	for _, opt := range options {
@@ -70,7 +69,24 @@ func Load(t *testing.T, builtIn catalog.BuiltIn, pluginFacade catalog.Facade, op
 	}
 }
 
-func nullLogger() logrus.FieldLogger {
-	log, _ := test.NewNullLogger()
+func testLogger(t *testing.T) logrus.FieldLogger {
+	log := logrus.New()
+	log.SetOutput(io.Discard)
+	log.AddHook(logHook{t: t})
 	return log
 }
+
+type logHook struct{ t *testing.T }
+
+func (h logHook) Levels() []logrus.Level {
+	return logrus.AllLevels
+}
+
+func (h logHook) Fire(e *logrus.Entry) error {
+	s, err := e.String()
+	if err != nil {
+		return err
+	}
+	h.t.Logf("log: %s\n", s)
+	return nil
+}