From ec25e7682c9f54fd820520c45bee5ed12bd39053 Mon Sep 17 00:00:00 2001 From: Johan Brandhorst-Satzkorn Date: Fri, 8 Apr 2022 09:38:21 -0400 Subject: [PATCH] fix(controller): Clone messages before auditing (#1982) It is not unreasonable for downstream users of the request and response messages to mutate them, indeed we mutate the response on the way out to get the desired external API. To avoid inconsistent messages being audited, make a copy of the entire message before returning from the interceptor. This was discovered while testing the new subtype transformation interceptor. --- internal/servers/controller/interceptor.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/internal/servers/controller/interceptor.go b/internal/servers/controller/interceptor.go index 5fd96e14c..13c8c1413 100644 --- a/internal/servers/controller/interceptor.go +++ b/internal/servers/controller/interceptor.go @@ -241,7 +241,10 @@ func auditRequestInterceptor( handler grpc.UnaryHandler) (interface{}, error, ) { if msg, ok := req.(proto.Message); ok { - if err := event.WriteAudit(interceptorCtx, op, event.WithRequest(&event.Request{Details: msg})); err != nil { + // Clone the request before writing it to the audit log, + // in case downstream interceptors modify it. + clonedMsg := proto.Clone(msg) + if err := event.WriteAudit(interceptorCtx, op, event.WithRequest(&event.Request{Details: clonedMsg})); err != nil { return req, status.Errorf(codes.Internal, "unable to write request msg audit: %s", err) } } @@ -263,7 +266,10 @@ func auditResponseInterceptor( resp, err := handler(interceptorCtx, req) if msg, ok := resp.(proto.Message); ok { - if err := event.WriteAudit(interceptorCtx, op, event.WithResponse(&event.Response{Details: msg})); err != nil { + // Clone the response before writing it to the audit log, + // in case downstream interceptors modify it. + clonedMsg := proto.Clone(msg) + if err := event.WriteAudit(interceptorCtx, op, event.WithResponse(&event.Response{Details: clonedMsg})); err != nil { return req, status.Errorf(codes.Internal, "unable to write response msg audit: %s", err) } }