From fad4c0c934d21a0a6b2de0c769cb9e3809424dd6 Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Tue, 27 Oct 2026 22:33:15 +0800 Subject: [PATCH 1/2] fix(sql): redact passwords in connection URI errors --- .../catalog/sql/catalog_store_mysql.cc | 3 +- .../catalog/sql/catalog_store_postgresql.cc | 3 +- src/iceberg/catalog/sql/connection_uri.cc | 34 +++++++++++++++++-- .../catalog/sql/connection_uri_internal.h | 8 +++++ src/iceberg/test/sql_catalog_test.cc | 22 ++++++++++++ 5 files changed, 65 insertions(+), 5 deletions(-) diff --git a/src/iceberg/catalog/sql/catalog_store_mysql.cc b/src/iceberg/catalog/sql/catalog_store_mysql.cc index 1bd2f5787e..d69f023f9b 100644 --- a/src/iceberg/catalog/sql/catalog_store_mysql.cc +++ b/src/iceberg/catalog/sql/catalog_store_mysql.cc @@ -93,7 +93,8 @@ Result> MakeMySqlCatalogStore( return std::make_shared(options.catalog_name, std::move(connection)); } catch (const std::exception& e) { - return IOError("Failed to connect to MySQL '{}': {}", options.uri, e.what()); + return IOError("Failed to connect to MySQL '{}': {}", + RedactConnectionUri(options.uri), e.what()); } } diff --git a/src/iceberg/catalog/sql/catalog_store_postgresql.cc b/src/iceberg/catalog/sql/catalog_store_postgresql.cc index bcca2f3676..0fe7c25f59 100644 --- a/src/iceberg/catalog/sql/catalog_store_postgresql.cc +++ b/src/iceberg/catalog/sql/catalog_store_postgresql.cc @@ -83,7 +83,8 @@ Result> MakePostgreSqlCatalogStore( return std::make_shared(options.catalog_name, std::move(connection)); } catch (const std::exception& e) { - return IOError("Failed to connect to PostgreSQL '{}': {}", options.uri, e.what()); + return IOError("Failed to connect to PostgreSQL '{}': {}", + RedactConnectionUri(options.uri), e.what()); } } diff --git a/src/iceberg/catalog/sql/connection_uri.cc b/src/iceberg/catalog/sql/connection_uri.cc index b37844ada3..8026ac8598 100644 --- a/src/iceberg/catalog/sql/connection_uri.cc +++ b/src/iceberg/catalog/sql/connection_uri.cc @@ -50,9 +50,37 @@ Result ParsePort(std::string_view port_str, std::string_view uri) { } // namespace +std::string RedactConnectionUri(std::string_view uri) { + // Locate the authority section, mirroring ParseConnectionUri: after + // "scheme://" when present, up to the first '/'. + size_t authority_begin = 0; + if (const auto scheme = uri.find("://"); scheme != std::string_view::npos) { + authority_begin = scheme + 3; + } + std::string_view authority = uri.substr(authority_begin); + if (const auto slash = authority.find('/'); slash != std::string_view::npos) { + authority = authority.substr(0, slash); + } + const auto at = authority.rfind('@'); + if (at == std::string_view::npos) { + return std::string(uri); + } + const std::string_view userinfo = authority.substr(0, at); + const auto colon = userinfo.find(':'); + if (colon == std::string_view::npos) { + return std::string(uri); // no password present + } + const size_t password_begin = authority_begin + colon + 1; + const size_t password_end = authority_begin + at; + std::string redacted(uri); + redacted.replace(password_begin, password_end - password_begin, "***"); + return redacted; +} + Result ParseConnectionUri(std::string_view uri) { ConnectionUri result; - const std::string original(uri); + // Error messages must never echo an embedded password. + const std::string redacted = RedactConnectionUri(uri); const bool has_scheme = uri.find("://") != std::string_view::npos; if (const auto scheme = uri.find("://"); scheme != std::string_view::npos) { @@ -67,7 +95,7 @@ Result ParseConnectionUri(std::string_view uri) { } if (has_scheme && authority.empty()) { return InvalidArgument("Invalid SQL connection URI '{}': authority is empty", - original); + redacted); } // Split userinfo from host at the last '@'. @@ -87,7 +115,7 @@ Result ParseConnectionUri(std::string_view uri) { if (const auto colon = host_port.rfind(':'); colon != std::string_view::npos) { result.host = std::string(host_port.substr(0, colon)); ICEBERG_ASSIGN_OR_RAISE(result.port, - ParsePort(host_port.substr(colon + 1), original)); + ParsePort(host_port.substr(colon + 1), redacted)); } else { result.host = std::string(host_port); } diff --git a/src/iceberg/catalog/sql/connection_uri_internal.h b/src/iceberg/catalog/sql/connection_uri_internal.h index 775bb52a96..ca38f88fe5 100644 --- a/src/iceberg/catalog/sql/connection_uri_internal.h +++ b/src/iceberg/catalog/sql/connection_uri_internal.h @@ -47,4 +47,12 @@ struct ConnectionUri { /// be passed unambiguously to a SQL driver is returned as `InvalidArgument`. Result ParseConnectionUri(std::string_view uri); +/// \brief Render a connection URI with any password replaced by "***". +/// +/// Connection URIs may embed credentials (`scheme://user:password@host`). +/// Use this rendering whenever a URI is interpolated into an error message +/// or log line so the password never reaches an audience broader than the +/// credential's. The input is returned unchanged when no password is present. +std::string RedactConnectionUri(std::string_view uri); + } // namespace iceberg::sql diff --git a/src/iceberg/test/sql_catalog_test.cc b/src/iceberg/test/sql_catalog_test.cc index 8378a05d39..3c64d21b92 100644 --- a/src/iceberg/test/sql_catalog_test.cc +++ b/src/iceberg/test/sql_catalog_test.cc @@ -167,6 +167,28 @@ TEST(ConnectionUriTest, RejectsInvalidPort) { IsError(ErrorKind::kInvalidArgument)); } +TEST(ConnectionUriTest, RedactsPassword) { + EXPECT_EQ(RedactConnectionUri("postgresql://alice:secret@db.example:5432/prod"), + "postgresql://alice:***@db.example:5432/prod"); + // No credentials or no password: returned unchanged. + EXPECT_EQ(RedactConnectionUri("mysql://db.example:3306/prod"), + "mysql://db.example:3306/prod"); + EXPECT_EQ(RedactConnectionUri("mysql://alice@db.example/prod"), + "mysql://alice@db.example/prod"); + // Password containing '@' (last '@' delimits the host, as in ParseConnectionUri). + EXPECT_EQ(RedactConnectionUri("mysql://alice:p@ss@db.example/prod"), + "mysql://alice:***@db.example/prod"); + // '@' in the path must not be mistaken for a userinfo delimiter. + EXPECT_EQ(RedactConnectionUri("mysql://db.example/we@rd"), "mysql://db.example/we@rd"); +} + +TEST(ConnectionUriTest, ParseErrorsDoNotEchoPassword) { + auto result = ParseConnectionUri("mysql://alice:secret@db.example:abc/prod"); + ASSERT_THAT(result, IsError(ErrorKind::kInvalidArgument)); + EXPECT_THAT(result, ::testing::Not(HasErrorMessage("secret"))); + EXPECT_THAT(result, HasErrorMessage("alice:***@db.example")); +} + TEST_F(SqlCatalogTest, CatalogName) { EXPECT_EQ(catalog_->name(), "test_catalog"); } TEST_F(SqlCatalogTest, NamespaceLifecycle) { From 90f9f404ab7e168118ae9114f2cebf17a62954ab Mon Sep 17 00:00:00 2001 From: Guotao Yu Date: Sun, 1 Nov 2026 10:24:23 +0800 Subject: [PATCH 2/2] fix(sql): avoid leaking driver connection errors --- src/iceberg/catalog/sql/catalog_store_mysql.cc | 5 ++--- src/iceberg/catalog/sql/catalog_store_postgresql.cc | 6 +++--- src/iceberg/test/sql_catalog_test.cc | 12 ++++++++++++ 3 files changed, 17 insertions(+), 6 deletions(-) diff --git a/src/iceberg/catalog/sql/catalog_store_mysql.cc b/src/iceberg/catalog/sql/catalog_store_mysql.cc index d69f023f9b..8e77bff8f8 100644 --- a/src/iceberg/catalog/sql/catalog_store_mysql.cc +++ b/src/iceberg/catalog/sql/catalog_store_mysql.cc @@ -92,9 +92,8 @@ Result> MakeMySqlCatalogStore( sqlpp::mysql::connection connection(config); return std::make_shared(options.catalog_name, std::move(connection)); - } catch (const std::exception& e) { - return IOError("Failed to connect to MySQL '{}': {}", - RedactConnectionUri(options.uri), e.what()); + } catch (const std::exception&) { + return IOError("Failed to connect to MySQL '{}'", RedactConnectionUri(options.uri)); } } diff --git a/src/iceberg/catalog/sql/catalog_store_postgresql.cc b/src/iceberg/catalog/sql/catalog_store_postgresql.cc index 0fe7c25f59..aee96a0515 100644 --- a/src/iceberg/catalog/sql/catalog_store_postgresql.cc +++ b/src/iceberg/catalog/sql/catalog_store_postgresql.cc @@ -82,9 +82,9 @@ Result> MakePostgreSqlCatalogStore( sqlpp::postgresql::connection connection(config); return std::make_shared(options.catalog_name, std::move(connection)); - } catch (const std::exception& e) { - return IOError("Failed to connect to PostgreSQL '{}': {}", - RedactConnectionUri(options.uri), e.what()); + } catch (const std::exception&) { + return IOError("Failed to connect to PostgreSQL '{}'", + RedactConnectionUri(options.uri)); } } diff --git a/src/iceberg/test/sql_catalog_test.cc b/src/iceberg/test/sql_catalog_test.cc index 3c64d21b92..c6531eeae2 100644 --- a/src/iceberg/test/sql_catalog_test.cc +++ b/src/iceberg/test/sql_catalog_test.cc @@ -34,6 +34,7 @@ #include "iceberg/arrow/arrow_io_internal.h" #include "iceberg/catalog/sql/catalog_store.h" +#include "iceberg/catalog/sql/config.h" #include "iceberg/catalog/sql/connection_uri_internal.h" #include "iceberg/partition_spec.h" #include "iceberg/schema.h" @@ -189,6 +190,17 @@ TEST(ConnectionUriTest, ParseErrorsDoNotEchoPassword) { EXPECT_THAT(result, HasErrorMessage("alice:***@db.example")); } +#ifdef BUILD_POSTGRESQL_CONNECTOR +TEST(ConnectionUriTest, PostgreSqlConnectErrorsDoNotEchoPassword) { + auto result = MakePostgreSqlCatalogStore( + {.catalog_name = "test", + .uri = "postgresql://alice:secret fragment@127.0.0.1:1/prod"}); + ASSERT_THAT(result, IsError(ErrorKind::kIOError)); + EXPECT_THAT(result, ::testing::Not(HasErrorMessage("secret"))); + EXPECT_THAT(result, ::testing::Not(HasErrorMessage("fragment"))); +} +#endif + TEST_F(SqlCatalogTest, CatalogName) { EXPECT_EQ(catalog_->name(), "test_catalog"); } TEST_F(SqlCatalogTest, NamespaceLifecycle) {