From 9ca12b1434e10ba303afa07f6ec82cef9b90def9 Mon Sep 17 00:00:00 2001
From: Freddie Lamble <flamble1@bloomberg.net>
Date: Fri, 28 Aug 2026 08:14:59 +0000
Subject: [PATCH] Redact SASL response from Connection Start-Ok trace logging

`ConnectionStartOk::operator<<` serialized the SASL `response` field. This
can contain credentials which should never be logged. Redact it.

The `response` is opaque, mechanism-defined data, so it is redacted
unconditionally. `properties`, `mechanism` and `locale` are unchanged, and
the `response()` accessor is untouched.

Add coverage:
- `rmqamqpt`: `operator<<` emits `response:<REDACTED>`, not the response
- `rmqamqp`: a mocked handshake captures logs and asserts no credentials
---
 .../rmqamqpt/rmqamqpt_connectionstartok.cpp   |  5 ++--
 src/tests/rmqamqp/rmqamqp_connection.t.cpp    | 28 +++++++++++++++++++
 .../rmqamqpt/rmqamqpt_connectionstartok.t.cpp | 16 +++++++++++
 3 files changed, 47 insertions(+), 2 deletions(-)

diff --git a/src/rmq/rmqamqpt/rmqamqpt_connectionstartok.cpp b/src/rmq/rmqamqpt/rmqamqpt_connectionstartok.cpp
index aea8ada7..d926512e 100644
--- a/src/rmq/rmqamqpt/rmqamqpt_connectionstartok.cpp
+++ b/src/rmq/rmqamqpt/rmqamqpt_connectionstartok.cpp
@@ -63,9 +63,10 @@ void ConnectionStartOk::encode(Writer& output, const ConnectionStartOk& startOk)
 bsl::ostream& operator<<(bsl::ostream& os,
                          const ConnectionStartOk& startOkMethod)
 {
+    // response is opaque SASL data that may carry credentials.
+    // Redact it.
     os << "Connection StartOk = [properties:" << startOkMethod.properties()
-       << ", mechanism:" << startOkMethod.mechanism()
-       << ", response:" << startOkMethod.response()
+       << ", mechanism:" << startOkMethod.mechanism() << ", response:<REDACTED>"
        << ", locale:" << startOkMethod.locale() << "]";
     return os;
 }
diff --git a/src/tests/rmqamqp/rmqamqp_connection.t.cpp b/src/tests/rmqamqp/rmqamqp_connection.t.cpp
index e9ad7839..c8886b94 100644
--- a/src/tests/rmqamqp/rmqamqp_connection.t.cpp
+++ b/src/tests/rmqamqp/rmqamqp_connection.t.cpp
@@ -38,11 +38,14 @@
 #include <rmqt_simpleendpoint.h>
 
 #include <ball_log.h>
+#include <ball_loggermanager.h>
+#include <ball_streamobserver.h>
 #include <bdlf_bind.h>
 #include <bsl_functional.h>
 #include <bsl_iostream.h>
 #include <bsl_limits.h>
 #include <bsl_memory.h>
+#include <bsl_sstream.h>
 #include <bsl_stdexcept.h>
 #include <bsl_utility.h>
 #include <bsl_vector.h>
@@ -696,6 +699,31 @@ TEST_F(ConnectionTests, Handshake)
     expectShutdownCalls();
 }
 
+TEST_F(ConnectionTests, HandshakeDoesNotLogCredentials)
+{
+    bsl::ostringstream logStream;
+    bsl::shared_ptr<ball::StreamObserver> observer =
+        bsl::make_shared<ball::StreamObserver>(&logStream);
+    ball::LoggerManager::singleton().registerObserver(observer,
+                                                      "credentialcapture");
+
+    expectFirstHandshakeFrames();
+
+    bsl::shared_ptr<rmqamqp::Connection> conn = createAndStartConnection();
+
+    // 1. Handshake
+    d_eventLoop.run();
+
+    ball::LoggerManager::singleton().deregisterObserver("credentialcapture");
+
+    const bsl::string logged = logStream.str();
+    EXPECT_THAT(logged, HasSubstr("response:<REDACTED>"));
+    EXPECT_THAT(logged, Not(HasSubstr(bsl::string("\0guest\0guest", 12))));
+
+    EXPECT_THAT(d_replayFrame.getLength(), Eq(0));
+    expectShutdownCalls();
+}
+
 TEST_F(ConnectionTests, ClientProperties)
 {
     rmqt::FieldTable inputClientProperties;
diff --git a/src/tests/rmqamqpt/rmqamqpt_connectionstartok.t.cpp b/src/tests/rmqamqpt/rmqamqpt_connectionstartok.t.cpp
index 3e1c9b02..1252b1c4 100644
--- a/src/tests/rmqamqpt/rmqamqpt_connectionstartok.t.cpp
+++ b/src/tests/rmqamqpt/rmqamqpt_connectionstartok.t.cpp
@@ -22,6 +22,7 @@
 #include <gtest/gtest.h>
 
 #include <bsl_cstdint.h>
+#include <bsl_sstream.h>
 #include <bsl_string.h>
 
 using namespace BloombergLP;
@@ -57,3 +58,18 @@ TEST(Methods_ConnectionStartOk, StartOkEncodeDecode)
     EXPECT_TRUE(cm.is<rmqamqpt::ConnectionStartOk>());
     EXPECT_EQ(startOkMethod, cm.the<rmqamqpt::ConnectionStartOk>());
 }
+
+TEST(Methods_ConnectionStartOk, StartOkStreamRedactsResponse)
+{
+    rmqt::FieldTable ft;
+    bsl::string response("\0secretuser\0secretpass", 22);
+    rmqamqpt::ConnectionStartOk startOkMethod(ft, "PLAIN", response, "en-US");
+
+    bsl::ostringstream oss;
+    oss << startOkMethod;
+    const bsl::string out = oss.str();
+
+    EXPECT_THAT(out, HasSubstr("response:<REDACTED>"));
+    EXPECT_THAT(out, Not(HasSubstr("secretuser")));
+    EXPECT_THAT(out, Not(HasSubstr("secretpass")));
+}