logger: record field sensitivity instead of destroying it

logger/proto.go

@@ -32,47 +32,35 @@ import (
 	"github.com/livekit/protocol/utils/must"
 )
 
-// Proto returns a zapcore.ObjectMarshaler that redacts every field whose
-// (logger.sensitivity) is at or above SENSITIVITY_PII. Use in routine
-// application and debug logs.
+// Proto returns a zapcore.ObjectMarshaler that records each field annotated
+// with a (logger.sensitivity) under that sensitivity, and redacts it when the
+// sink cannot record that level. SECRET is never recordable.
 func Proto(val proto.Message) zapcore.ObjectMarshaler {
 	if val == nil {
 		return nil
 	}
-	return protoMarshaller{m: val.ProtoReflect(), maxLevel: logger.Sensitivity_SENSITIVITY_UNSPECIFIED}
-}
-
-// UnredactedProto returns a zapcore.ObjectMarshaler that exposes PII-tier
-// fields while still redacting SECRET-tier fields (credentials, tokens, API
-// keys). Use only in observability events where operator context legitimately
-// requires user-identifying data.
-func UnredactedProto(val proto.Message) zapcore.ObjectMarshaler {
-	if val == nil {
-		return nil
-	}
-	return protoMarshaller{m: val.ProtoReflect(), maxLevel: logger.Sensitivity_SENSITIVITY_PII}
+	return protoMarshaller{m: val.ProtoReflect()}
 }
 
 // ProtoWithLimit behaves like Proto, but when the message's wire size exceeds
 // maxBytes it logs a compact summary instead of the full contents: message
-// type, wire size, and top-level scalar fields (redacted per sensitivity,
-// long strings truncated), with repeated/map fields reduced to counts and
-// nested messages dropped. Use for messages of unbounded size, such as RPC
-// payloads.
+// type, wire size, and top-level scalar fields (tagged or redacted per
+// sensitivity, long strings truncated), with repeated/map fields reduced to
+// counts and nested messages dropped. Use for messages of unbounded size, such
+// as RPC payloads.
 func ProtoWithLimit(val proto.Message, maxBytes int) zapcore.ObjectMarshaler {
 	if val == nil {
 		return nil
 	}
-	return protoMarshaller{m: val.ProtoReflect(), maxLevel: logger.Sensitivity_SENSITIVITY_UNSPECIFIED, maxSize: maxBytes}
+	return protoMarshaller{m: val.ProtoReflect(), maxSize: maxBytes}
 }
 
 var _ zapcore.ObjectMarshaler = protoMarshaller{}
 var _ zapcore.ObjectMarshaler = protoMapMarshaller{}
 var _ zapcore.ArrayMarshaler = protoListMarshaller{}
 
 type protoMarshaller struct {
-	m        protoreflect.Message
-	maxLevel logger.Sensitivity
+	m protoreflect.Message
 	// maxSize is the wire size in bytes above which the message is logged as
 	// a summary instead of its full contents. 0 means no limit.
 	maxSize int
@@ -102,21 +90,32 @@ func (p protoMarshaller) MarshalLogObject(e zapcore.ObjectEncoder) error {
 			continue
 		}
 
-		if fieldSensitivity(f) > p.maxLevel {
-			e.AddString(k, marshalRedacted(f, v))
-			continue
+		// Never assign to e: sibling fields must not inherit this field's
+		// level. enc records this field, and everything nested under it, at
+		// the level the field declares.
+		enc := e
+		if level := fieldSensitivity(f); level > SensitivityNone {
+			se := ObjectEncoderFor(e, level)
+			if se == nil {
+				e.AddString(k, marshalRedacted(f, v))
+				continue
+			}
+			enc = se
 		}
 
+		// Sensitivity is not re-checked for map values or list elements: proto
+		// cannot annotate them independently of the containing field, so the
+		// check above is complete.
 		if f.IsMap() {
 			if m := v.Map(); m.IsValid() {
-				e.AddObject(k, protoMapMarshaller{f: f, m: m, maxLevel: p.maxLevel})
+				enc.AddObject(k, protoMapMarshaller{f: f, m: m})
 			}
 		} else if f.IsList() {
 			if m := v.List(); m.IsValid() {
-				e.AddArray(k, protoListMarshaller{f: f, m: m, maxLevel: p.maxLevel})
+				enc.AddArray(k, protoListMarshaller{f: f, m: m})
 			}
 		} else {
-			marshalProtoField(k, f, v, e, p.maxLevel)
+			marshalProtoField(k, f, v, enc)
 		}
 	}
 	return nil
@@ -140,18 +139,35 @@ func (p protoMarshaller) marshalSummary(e zapcore.ObjectEncoder, size int) error
 		if proto.HasExtension(f.Options(), logger.E_Name) {
 			k = proto.GetExtension(f.Options(), logger.E_Name).(string)
 		}
+		// These disclose no field contents, so they precede the sensitivity
+		// decision and stay untagged. Moving the decision above them would
+		// replace a count with a redaction.
 		switch {
 		case f.IsMap():
 			e.AddInt(k+"Count", v.Map().Len())
+			continue
 		case f.IsList():
 			e.AddInt(k+"Count", v.List().Len())
+			continue
 		case f.Kind() == protoreflect.MessageKind, f.Kind() == protoreflect.GroupKind:
-		case fieldSensitivity(f) > p.maxLevel:
-			e.AddString(k, marshalRedacted(f, v))
-		case f.Kind() == protoreflect.StringKind:
-			e.AddString(k, marshalTruncatedString(v.String()))
-		default:
-			marshalProtoField(k, f, v, e, p.maxLevel)
+			continue
+		}
+
+		enc := e
+		if level := fieldSensitivity(f); level > SensitivityNone {
+			se := ObjectEncoderFor(e, level)
+			if se == nil {
+				e.AddString(k, marshalRedacted(f, v))
+				continue
+			}
+			enc = se
+		}
+
+		// Summaries stay bounded even for values the sink may record.
+		if f.Kind() == protoreflect.StringKind {
+			enc.AddString(k, marshalTruncatedString(v.String()))
+		} else {
+			marshalProtoField(k, f, v, enc)
 		}
 	}
 	return nil
@@ -165,9 +181,8 @@ func marshalTruncatedString(s string) string {
 }
 
 type protoMapMarshaller struct {
-	f        protoreflect.FieldDescriptor
-	m        protoreflect.Map
-	maxLevel logger.Sensitivity
+	f protoreflect.FieldDescriptor
+	m protoreflect.Map
 }
 
 func (p protoMapMarshaller) MarshalLogObject(e zapcore.ObjectEncoder) error {
@@ -183,16 +198,15 @@ func (p protoMapMarshaller) MarshalLogObject(e zapcore.ObjectEncoder) error {
 		case protoreflect.StringKind:
 			k = ki.String()
 		}
-		marshalProtoField(k, p.f.MapValue(), vi, e, p.maxLevel)
+		marshalProtoField(k, p.f.MapValue(), vi, e)
 		return true
 	})
 	return nil
 }
 
 type protoListMarshaller struct {
-	f        protoreflect.FieldDescriptor
-	m        protoreflect.List
-	maxLevel logger.Sensitivity
+	f protoreflect.FieldDescriptor
+	m protoreflect.List
 }
 
 func (p protoListMarshaller) MarshalLogArray(e zapcore.ArrayEncoder) error {
@@ -214,13 +228,13 @@ func (p protoListMarshaller) MarshalLogArray(e zapcore.ArrayEncoder) error {
 		case protoreflect.BytesKind:
 			e.AppendString(marshalProtoBytes(v.Bytes()))
 		case protoreflect.MessageKind:
-			e.AppendObject(protoMarshaller{m: v.Message(), maxLevel: p.maxLevel})
+			e.AppendObject(protoMarshaller{m: v.Message()})
 		}
 	}
 	return nil
 }
 
-func marshalProtoField(k string, f protoreflect.FieldDescriptor, v protoreflect.Value, e zapcore.ObjectEncoder, maxLevel logger.Sensitivity) {
+func marshalProtoField(k string, f protoreflect.FieldDescriptor, v protoreflect.Value, e zapcore.ObjectEncoder) {
 	switch f.Kind() {
 	case protoreflect.BoolKind:
 		e.AddBool(k, v.Bool())
@@ -237,7 +251,7 @@ func marshalProtoField(k string, f protoreflect.FieldDescriptor, v protoreflect.
 	case protoreflect.BytesKind:
 		e.AddString(k, marshalProtoBytes(v.Bytes()))
 	case protoreflect.MessageKind:
-		e.AddObject(k, protoMarshaller{m: v.Message(), maxLevel: maxLevel})
+		e.AddObject(k, protoMarshaller{m: v.Message()})
 	}
 }
 

logger/proto_test.go

@@ -58,30 +58,7 @@ func TestProtoRedactsPIIAndSecret(t *testing.T) {
 	require.Equal(t, "<redacted>", fields["assumeRoleExternalID"])
 }
 
-func TestUnredactedProtoShowsPIIRedactsSecret(t *testing.T) {
-	msg := &livekit.S3Upload{
-		AccessKey:            "AKIAEXAMPLE",
-		Secret:               "supersecretvalue",
-		SessionToken:         "tokenvalue",
-		AssumeRoleArn:        "arn:aws:iam::123456789012:role/MyRole",
-		AssumeRoleExternalId: "external-id-1",
-		Region:               "us-east-1",
-	}
-	fields := marshalFields(t, logger.UnredactedProto(msg))
-
-	require.Equal(t, "us-east-1", fields["region"])
-
-	// PII is exposed.
-	require.Equal(t, "arn:aws:iam::123456789012:role/MyRole", fields["assumeRoleArn"])
-
-	// SECRETs remain redacted.
-	require.Equal(t, "<redacted>", fields["accessKey"])
-	require.Equal(t, "<redacted>", fields["secret"])
-	require.Equal(t, "<redacted>", fields["sessionToken"])
-	require.Equal(t, "<redacted>", fields["assumeRoleExternalID"])
-}
-
-func TestProtoRedactFormatPreservedAtPIITier(t *testing.T) {
+func TestProtoRedactFormatUsedForPII(t *testing.T) {
 	// ParticipantInfo.metadata is PII with a size-showing redact_format.
 	msg := &livekit.ParticipantInfo{
 		Identity: "user-123",
@@ -94,10 +71,6 @@ func TestProtoRedactFormatPreservedAtPIITier(t *testing.T) {
 	require.Equal(t, "<redacted>", got["name"])
 	require.Contains(t, got["metadata"].(string), "<redacted (")
 	require.Contains(t, got["metadata"].(string), "bytes)>")
-
-	gotUnredacted := marshalFields(t, logger.UnredactedProto(msg))
-	require.Equal(t, "Alice", gotUnredacted["name"])
-	require.Equal(t, `{"plan":"pro"}`, gotUnredacted["metadata"])
 }
 
 func TestProtoNestedListPIIRedacted(t *testing.T) {
@@ -110,22 +83,14 @@ func TestProtoNestedListPIIRedacted(t *testing.T) {
 		},
 	}
 
-	gotRedacted := marshalFields(t, logger.Proto(msg))
-	tracks := gotRedacted["tracks"].([]any)
+	got := marshalFields(t, logger.Proto(msg))
+	tracks := got["tracks"].([]any)
 	require.Len(t, tracks, 2)
 	for _, raw := range tracks {
 		track := raw.(map[string]any)
 		require.NotEmpty(t, track["sid"])
 		require.Equal(t, "<redacted>", track["name"])
 	}
-
-	gotUnredacted := marshalFields(t, logger.UnredactedProto(msg))
-	tracksU := gotUnredacted["tracks"].([]any)
-	names := make([]string, 0, 2)
-	for _, raw := range tracksU {
-		names = append(names, raw.(map[string]any)["name"].(string))
-	}
-	require.ElementsMatch(t, []string{"Microphone", "Camera"}, names)
 }
 
 func TestProtoNestedSecretAlwaysRedacted(t *testing.T) {
@@ -135,19 +100,11 @@ func TestProtoNestedSecretAlwaysRedacted(t *testing.T) {
 	}
 	wrap := &livekit.JoinResponse{IceServers: servers}
 
-	for _, name := range []string{"Proto", "UnredactedProto"} {
-		var m zapcore.ObjectMarshaler
-		if name == "Proto" {
-			m = logger.Proto(wrap)
-		} else {
-			m = logger.UnredactedProto(wrap)
-		}
-		fields := marshalFields(t, m)
-		iceServers := fields["iceServers"].([]any)
-		require.Len(t, iceServers, 1)
-		entry := iceServers[0].(map[string]any)
-		require.Equal(t, "<redacted>", entry["credential"], "credential must be redacted by %s", name)
-	}
+	fields := marshalFields(t, logger.Proto(wrap))
+	iceServers := fields["iceServers"].([]any)
+	require.Len(t, iceServers, 1)
+	entry := iceServers[0].(map[string]any)
+	require.Equal(t, "<redacted>", entry["credential"])
 }
 
 func TestProtoWithLimitUnderLimit(t *testing.T) {
@@ -233,7 +190,6 @@ func TestProtoWithLimitNilSafe(t *testing.T) {
 
 func TestProtoNilSafe(t *testing.T) {
 	require.Nil(t, logger.Proto(nil))
-	require.Nil(t, logger.UnredactedProto(nil))
 
 	// Typed-nil pointer is not interface-nil; the returned marshaller is
 	// non-nil but emits no fields.

logger/sensitive.go

@@ -0,0 +1,76 @@
+// Copyright 2023 LiveKit, Inc.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package logger
+
+import (
+	"go.uber.org/zap/zapcore"
+
+	"github.com/livekit/protocol/livekit/logger"
+)
+
+// Sensitivity aliases the schema enum so implementors of the interfaces below
+// need only this package.
+type Sensitivity = logger.Sensitivity
+
+const (
+	SensitivityNone   = logger.Sensitivity_SENSITIVITY_UNSPECIFIED
+	SensitivityPII    = logger.Sensitivity_SENSITIVITY_PII
+	SensitivitySecret = logger.Sensitivity_SENSITIVITY_SECRET
+)
+
+// SensitiveObjectEncoder is a zapcore.ObjectEncoder that can record a value's
+// sensitivity instead of having the value destroyed. An encoder that cannot
+// record sensitivity does not implement it, and marshalers redact instead.
+type SensitiveObjectEncoder interface {
+	zapcore.ObjectEncoder
+
+	// SensitiveObjectEncoder returns an encoder that records every value
+	// written to it, and to encoders derived from it, at sensitivity s. It
+	// returns nil if this encoder must not record values at s, in which case
+	// the caller must redact.
+	//
+	// Implementations must fail closed by returning nil for any level they do
+	// not explicitly recognize. The returned encoder may share buffers with the
+	// receiver and is only valid until the next write on the receiver.
+	SensitiveObjectEncoder(s Sensitivity) zapcore.ObjectEncoder
+}
+
+// SensitiveArrayEncoder is the zapcore.ArrayEncoder counterpart to
+// SensitiveObjectEncoder.
+type SensitiveArrayEncoder interface {
+	zapcore.ArrayEncoder
+
+	SensitiveArrayEncoder(s Sensitivity) zapcore.ArrayEncoder
+}
+
+// ObjectEncoderFor returns an encoder that records values at sensitivity s, or
+// nil if e cannot record them. Hand-written marshalers that carry sensitivity
+// annotations should use this rather than asserting directly.
+func ObjectEncoderFor(e zapcore.ObjectEncoder, s Sensitivity) zapcore.ObjectEncoder {
+	se, ok := e.(SensitiveObjectEncoder)
+	if !ok {
+		return nil
+	}
+	return se.SensitiveObjectEncoder(s)
+}
+
+// ArrayEncoderFor is the ArrayEncoder counterpart to ObjectEncoderFor.
+func ArrayEncoderFor(e zapcore.ArrayEncoder, s Sensitivity) zapcore.ArrayEncoder {
+	se, ok := e.(SensitiveArrayEncoder)
+	if !ok {
+		return nil
+	}
+	return se.SensitiveArrayEncoder(s)
+}