barriers: use a redactable string as message payload

This changes the implementation of barriers to use a redactable string
as message payload. This makes it possible to include more details
(all the safe parts of the message payload) when printing out
the barrier cause.

In order to ensure safety when interacting with previous versions of
the library without this logic, we must be careful to wrap/escape the
message string received when decoding a network error. To force this
additional wrapping, we rename the error type so that it can get a
separate decode function.

Author: knz

barriers/barriers.go

@@ -37,7 +37,7 @@ func Handled(err error) error {
 	if err == nil {
 		return nil
 	}
-	return HandledWithMessage(err, err.Error())
+	return HandledWithSafeMessage(err, redact.Sprint(err))
 }
 
 // HandledWithMessage is like Handled except the message is overridden.
@@ -47,7 +47,14 @@ func HandledWithMessage(err error, msg string) error {
 	if err == nil {
 		return nil
 	}
-	return &barrierError{maskedErr: err, msg: msg}
+	return HandledWithSafeMessage(err, redact.Sprint(msg))
+}
+
+// HandledWithSafeMessage is like Handled except the message is overridden.
+// This can be used e.g. to hide message details or to prevent
+// downstream code to make assertions on the message's contents.
+func HandledWithSafeMessage(err error, msg redact.RedactableString) error {
+	return &barrierErr{maskedErr: err, smsg: msg}
 }
 
 // HandledWithMessagef is like HandledWithMessagef except the message
@@ -56,34 +63,34 @@ func HandledWithMessagef(err error, format string, args ...interface{}) error {
 	if err == nil {
 		return nil
 	}
-	return &barrierError{maskedErr: err, msg: fmt.Sprintf(format, args...)}
+	return &barrierErr{maskedErr: err, smsg: redact.Sprintf(format, args...)}
 }
 
-// barrierError is a leaf error type. It encapsulates a chain of
+// barrierErr is a leaf error type. It encapsulates a chain of
 // original causes, but these causes are hidden so that they inhibit
 // matching via Is() and the Cause()/Unwrap() recursions.
-type barrierError struct {
+type barrierErr struct {
 	// Message for the barrier itself.
 	// In the common case, the message from the masked error
 	// is used as-is (see Handled() above) however it is
 	// useful to cache it here since the masked error may
 	// have a long chain of wrappers and its Error() call
 	// may be expensive.
-	msg string
+	smsg redact.RedactableString
 	// Masked error chain.
 	maskedErr error
 }
 
-var _ error = (*barrierError)(nil)
-var _ errbase.SafeDetailer = (*barrierError)(nil)
-var _ errbase.SafeFormatter = (*barrierError)(nil)
-var _ fmt.Formatter = (*barrierError)(nil)
+var _ error = (*barrierErr)(nil)
+var _ errbase.SafeDetailer = (*barrierErr)(nil)
+var _ errbase.SafeFormatter = (*barrierErr)(nil)
+var _ fmt.Formatter = (*barrierErr)(nil)
 
-// barrierError is an error.
-func (e *barrierError) Error() string { return e.msg }
+// barrierErr is an error.
+func (e *barrierErr) Error() string { return e.smsg.StripMarkers() }
 
 // SafeDetails reports the PII-free details from the masked error.
-func (e *barrierError) SafeDetails() []string {
+func (e *barrierErr) SafeDetails() []string {
 	var details []string
 	for err := e.maskedErr; err != nil; err = errbase.UnwrapOnce(err) {
 		sd := errbase.GetSafeDetails(err)
@@ -94,10 +101,10 @@ func (e *barrierError) SafeDetails() []string {
 }
 
 // Printing a barrier reveals the details.
-func (e *barrierError) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
+func (e *barrierErr) Format(s fmt.State, verb rune) { errbase.FormatError(e, s, verb) }
 
-func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
-	p.Print(e.msg)
+func (e *barrierErr) SafeFormatError(p errbase.Printer) (next error) {
+	p.Print(e.smsg)
 	if p.Detail() {
 		p.Printf("-- cause hidden behind barrier\n%+v", e.maskedErr)
 	}
@@ -108,19 +115,37 @@ func (e *barrierError) SafeFormatError(p errbase.Printer) (next error) {
 func encodeBarrier(
 	ctx context.Context, err error,
 ) (msg string, details []string, payload proto.Message) {
-	e := err.(*barrierError)
+	e := err.(*barrierErr)
 	enc := errbase.EncodeError(ctx, e.maskedErr)
-	return e.msg, e.SafeDetails(), &enc
+	return string(e.smsg), e.SafeDetails(), &enc
 }
 
 // A barrier error is decoded exactly.
 func decodeBarrier(ctx context.Context, msg string, _ []string, payload proto.Message) error {
 	enc := payload.(*errbase.EncodedError)
-	return &barrierError{msg: msg, maskedErr: errbase.DecodeError(ctx, *enc)}
+	return &barrierErr{smsg: redact.RedactableString(msg), maskedErr: errbase.DecodeError(ctx, *enc)}
 }
 
+// Previous versions of barrier errors.
+func decodeBarrierPrev(ctx context.Context, msg string, _ []string, payload proto.Message) error {
+	enc := payload.(*errbase.EncodedError)
+	return &barrierErr{smsg: redact.Sprint(msg), maskedErr: errbase.DecodeError(ctx, *enc)}
+}
+
+// barrierError is the "old" type name of barrierErr. We use a new
+// name now to ensure a different decode function is used when
+// importing barriers from the previous structure, where the
+// message is not redactable.
+type barrierError struct {
+	msg       string
+	maskedErr error
+}
+
+func (b *barrierError) Error() string { return "" }
+
 func init() {
-	tn := errbase.GetTypeKey((*barrierError)(nil))
+	errbase.RegisterLeafDecoder(errbase.GetTypeKey((*barrierError)(nil)), decodeBarrierPrev)
+	tn := errbase.GetTypeKey((*barrierErr)(nil))
 	errbase.RegisterLeafDecoder(tn, decodeBarrier)
 	errbase.RegisterLeafEncoder(tn, encodeBarrier)
 }

barriers/barriers_test.go

@@ -118,7 +118,7 @@ woo
   | woo
   | (1) woo
   | Error types: (1) *errors.errorString
-Error types: (1) *barriers.barrierError`},
+Error types: (1) *barriers.barrierErr`},
 
 		{"handled + handled", barriers.Handled(barriers.Handled(goErr.New("woo"))), woo, `
 woo
@@ -130,8 +130,8 @@ woo
   |   | woo
   |   | (1) woo
   |   | Error types: (1) *errors.errorString
-  | Error types: (1) *barriers.barrierError
-Error types: (1) *barriers.barrierError`},
+  | Error types: (1) *barriers.barrierErr
+Error types: (1) *barriers.barrierErr`},
 
 		{"handledmsg", barriers.HandledWithMessage(goErr.New("woo"), "waa"), "waa", `
 waa
@@ -140,7 +140,7 @@ waa
   | woo
   | (1) woo
   | Error types: (1) *errors.errorString
-Error types: (1) *barriers.barrierError`},
+Error types: (1) *barriers.barrierErr`},
 
 		{"handledmsg + handledmsg", barriers.HandledWithMessage(
 			barriers.HandledWithMessage(
@@ -154,8 +154,8 @@ wuu
   |   | woo
   |   | (1) woo
   |   | Error types: (1) *errors.errorString
-  | Error types: (1) *barriers.barrierError
-Error types: (1) *barriers.barrierError`},
+  | Error types: (1) *barriers.barrierErr
+Error types: (1) *barriers.barrierErr`},
 
 		{"handled + wrapper",
 			barriers.Handled(
@@ -172,7 +172,7 @@ waa: woo
   |   | multi-line wrapper payload
   | Wraps: (2) woo
   | Error types: (1) *barriers_test.werrFmt (2) *errors.errorString
-Error types: (1) *barriers.barrierError`},
+Error types: (1) *barriers.barrierErr`},
 	}
 
 	for _, test := range testCases {

errutil/message_test.go

@@ -124,7 +124,7 @@ Wraps: (4) wuu: woo
   |   | multi-line payload
   | Wraps: (2) woo
   | Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
-Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierError`,
+Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierErr`,
 		},
 
 		{"assert + wrap empty",
@@ -148,7 +148,7 @@ Wraps: (3) wuu: woo
   |   | multi-line payload
   | Wraps: (2) woo
   | Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
-Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierError`,
+Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *barriers.barrierErr`,
 		},
 
 		{"assert + wrap empty+arg",
@@ -173,7 +173,7 @@ Wraps: (4) wuu: woo
   |   | multi-line payload
   | Wraps: (2) woo
   | Error types: (1) *errutil_test.werrFmt (2) *errors.errorString
-Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierError`,
+Error types: (1) *assert.withAssertionFailure (2) *withstack.withStack (3) *errutil.withPrefix (4) *barriers.barrierErr`,
 		},
 	}