fix: sanitize error before gRPC return to prevent credential leak in pod events

aadhar-agarwal · aadhar-agarwal · commit 7b11d6cae471 · 2026-01-20T22:18:44.000Z
PR containerd#12491 fixed credential leaks in containerd logs but the gRPC error returned to kubelet still contained sensitive information. This was visible in Kubernetes pod events via `kubectl describe pod`. The issue was that SanitizeError was called inside the defer block, but errgrpc.ToGRPC(err) was evaluated before the defer ran, so the gRPC message contained the original unsanitized error. Move SanitizeError before the return statement so both the logged error and the gRPC error are sanitized. Ref: containerd#5453 Signed-off-by: Aadhar Agarwal <aadagarwal@microsoft.com>
diff --git a/internal/cri/instrument/instrumented_service.go b/internal/cri/instrument/instrumented_service.go
@@ -354,8 +354,6 @@ func (in *instrumentedService) PullImage(ctx context.Context, r *runtime.PullIma
 	log.G(ctx).Infof("PullImage %q", r.GetImage().GetImage())
 	defer func() {
 		if err != nil {
-			// Sanitize error to remove sensitive information
-			err = ctrdutil.SanitizeError(err)
 			log.G(ctx).WithError(err).Errorf("PullImage %q failed", r.GetImage().GetImage())
 		} else {
 			log.G(ctx).Infof("PullImage %q returns image reference %q",
@@ -364,6 +362,10 @@ func (in *instrumentedService) PullImage(ctx context.Context, r *runtime.PullIma
 		span.RecordError(err)
 	}()
 	res, err = in.c.PullImage(ctrdutil.WithNamespace(ctx), r)
+	// Sanitize error to remove sensitive information from both logs and returned gRPC error
+	if err != nil {
+		err = ctrdutil.SanitizeError(err)
+	}
 	return res, errgrpc.ToGRPC(err)
 }
 
@@ -374,15 +376,17 @@ func (in *instrumentedService) ListImages(ctx context.Context, r *runtime.ListIm
 	log.G(ctx).Tracef("ListImages with filter %+v", r.GetFilter())
 	defer func() {
 		if err != nil {
-			// Sanitize error to remove sensitive information
-			err = ctrdutil.SanitizeError(err)
 			log.G(ctx).WithError(err).Errorf("ListImages with filter %+v failed", r.GetFilter())
 		} else {
 			log.G(ctx).Tracef("ListImages with filter %+v returns image list %+v",
 				r.GetFilter(), res.GetImages())
 		}
 	}()
 	res, err = in.c.ListImages(ctrdutil.WithNamespace(ctx), r)
+	// Sanitize error to remove sensitive information from both logs and returned gRPC error
+	if err != nil {
+		err = ctrdutil.SanitizeError(err)
+	}
 	return res, errgrpc.ToGRPC(err)
 }
 
@@ -393,15 +397,17 @@ func (in *instrumentedService) ImageStatus(ctx context.Context, r *runtime.Image
 	log.G(ctx).Tracef("ImageStatus for %q", r.GetImage().GetImage())
 	defer func() {
 		if err != nil {
-			// Sanitize error to remove sensitive information
-			err = ctrdutil.SanitizeError(err)
 			log.G(ctx).WithError(err).Errorf("ImageStatus for %q failed", r.GetImage().GetImage())
 		} else {
 			log.G(ctx).Tracef("ImageStatus for %q returns image status %+v",
 				r.GetImage().GetImage(), res.GetImage())
 		}
 	}()
 	res, err = in.c.ImageStatus(ctrdutil.WithNamespace(ctx), r)
+	// Sanitize error to remove sensitive information from both logs and returned gRPC error
+	if err != nil {
+		err = ctrdutil.SanitizeError(err)
+	}
 	return res, errgrpc.ToGRPC(err)
 }
 
@@ -413,15 +419,17 @@ func (in *instrumentedService) RemoveImage(ctx context.Context, r *runtime.Remov
 	log.G(ctx).Infof("RemoveImage %q", r.GetImage().GetImage())
 	defer func() {
 		if err != nil {
-			// Sanitize error to remove sensitive information
-			err = ctrdutil.SanitizeError(err)
 			log.G(ctx).WithError(err).Errorf("RemoveImage %q failed", r.GetImage().GetImage())
 		} else {
 			log.G(ctx).Infof("RemoveImage %q returns successfully", r.GetImage().GetImage())
 		}
 		span.RecordError(err)
 	}()
 	res, err := in.c.RemoveImage(ctrdutil.WithNamespace(ctx), r)
+	// Sanitize error to remove sensitive information from both logs and returned gRPC error
+	if err != nil {
+		err = ctrdutil.SanitizeError(err)
+	}
 	return res, errgrpc.ToGRPC(err)
 }
 
