Skip to content

Commit 815101a

Browse files
MytherinMytherin
authored
Merge pull request duckdb#110 from Mytherin/dontleaksecretinpath
Avoid modifying the path that is shown in duckdb_databases with the secret contents
2 parents 94b1ec6 + 7ebd941 commit 815101a

File tree

5 files changed

+103
-91
lines changed

5 files changed

+103
-91
lines changed

‎src/include/storage/mysql_catalog.hpp

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ class MySQLSchemaEntry;
1818

1919
class MySQLCatalog : public Catalog {
2020
public:
21-
explicit MySQLCatalog(AttachedDatabase &db_p, const string &path, AccessMode access_mode);
21+
explicit MySQLCatalog(AttachedDatabase &db_p, string connection_string, string attach_path, AccessMode access_mode);
2222
~MySQLCatalog();
2323

24-
string path;
24+
string connection_string;
25+
string attach_path;
2526
AccessMode access_mode;
2627

2728
public:
@@ -30,6 +31,8 @@ class MySQLCatalog : public Catalog {
3031
return "mysql";
3132
}
3233

34+
static string GetConnectionString(ClientContext &context, const string &attach_path, string secret_name);
35+
3336
optional_ptr<CatalogEntry> CreateSchema(CatalogTransaction transaction, CreateSchemaInfo &info) override;
3437

3538
void ScanSchemas(ClientContext &context, std::function<void(SchemaCatalogEntry &)> callback) override;

‎src/mysql_storage.cpp

Lines changed: 3 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -4,55 +4,9 @@
44
#include "storage/mysql_catalog.hpp"
55
#include "duckdb/parser/parsed_data/attach_info.hpp"
66
#include "storage/mysql_transaction_manager.hpp"
7-
#include "duckdb/main/secret/secret_manager.hpp"
87

98
namespace duckdb {
109

11-
string EscapeConnectionString(const string &input) {
12-
string result = "\"";
13-
for (auto c : input) {
14-
if (c == '\\') {
15-
result += "\\\\";
16-
} else if (c == '"') {
17-
result += "\\\"";
18-
} else {
19-
result += c;
20-
}
21-
}
22-
result += "\"";
23-
return result;
24-
}
25-
26-
string AddConnectionOption(const KeyValueSecret &kv_secret, const string &name) {
27-
Value input_val = kv_secret.TryGetValue(name);
28-
if (input_val.IsNull()) {
29-
// not provided
30-
return string();
31-
}
32-
string result;
33-
result += name;
34-
result += "=";
35-
result += EscapeConnectionString(input_val.ToString());
36-
result += " ";
37-
return result;
38-
}
39-
40-
unique_ptr<SecretEntry> GetSecret(ClientContext &context, const string &secret_name) {
41-
auto &secret_manager = SecretManager::Get(context);
42-
auto transaction = CatalogTransaction::GetSystemCatalogTransaction(context);
43-
// FIXME: this should be adjusted once the `GetSecretByName` API supports this
44-
// use case
45-
auto secret_entry = secret_manager.GetSecretByName(transaction, secret_name, "memory");
46-
if (secret_entry) {
47-
return secret_entry;
48-
}
49-
secret_entry = secret_manager.GetSecretByName(transaction, secret_name, "local_file");
50-
if (secret_entry) {
51-
return secret_entry;
52-
}
53-
return nullptr;
54-
}
55-
5610
static unique_ptr<Catalog> MySQLAttach(StorageExtensionInfo *storage_info, ClientContext &context, AttachedDatabase &db,
5711
const string &name, AttachInfo &info, AccessMode access_mode) {
5812
// check if we have a secret provided
@@ -68,43 +22,9 @@ static unique_ptr<Catalog> MySQLAttach(StorageExtensionInfo *storage_info, Clien
6822
}
6923
}
7024

71-
// if no secret is specified we default to the unnamed mysql secret, if it
72-
// exists
73-
bool explicit_secret = !secret_name.empty();
74-
if (!explicit_secret) {
75-
// look up settings from the default unnamed mysql secret if none is
76-
// provided
77-
secret_name = "__default_mysql";
78-
}
79-
80-
string connection_string = info.path;
81-
auto secret_entry = GetSecret(context, secret_name);
82-
if (secret_entry) {
83-
// secret found - read data
84-
const auto &kv_secret = dynamic_cast<const KeyValueSecret &>(*secret_entry->secret);
85-
string new_connection_info;
86-
87-
new_connection_info += AddConnectionOption(kv_secret, "user");
88-
new_connection_info += AddConnectionOption(kv_secret, "password");
89-
new_connection_info += AddConnectionOption(kv_secret, "host");
90-
new_connection_info += AddConnectionOption(kv_secret, "port");
91-
new_connection_info += AddConnectionOption(kv_secret, "database");
92-
new_connection_info += AddConnectionOption(kv_secret, "socket");
93-
new_connection_info += AddConnectionOption(kv_secret, "ssl_mode");
94-
new_connection_info += AddConnectionOption(kv_secret, "ssl_ca");
95-
new_connection_info += AddConnectionOption(kv_secret, "ssl_capath");
96-
new_connection_info += AddConnectionOption(kv_secret, "ssl_cert");
97-
new_connection_info += AddConnectionOption(kv_secret, "ssl_cipher");
98-
new_connection_info += AddConnectionOption(kv_secret, "ssl_crl");
99-
new_connection_info += AddConnectionOption(kv_secret, "ssl_crlpath");
100-
new_connection_info += AddConnectionOption(kv_secret, "ssl_key");
101-
connection_string = new_connection_info + connection_string;
102-
} else if (explicit_secret) {
103-
// secret not found and one was explicitly provided - throw an error
104-
throw BinderException("Secret with name \"%s\" not found", secret_name);
105-
}
106-
107-
return make_uniq<MySQLCatalog>(db, connection_string, access_mode);
25+
string attach_path = info.path;
26+
auto connection_string = MySQLCatalog::GetConnectionString(context, attach_path, secret_name);
27+
return make_uniq<MySQLCatalog>(db, std::move(connection_string), std::move(attach_path), access_mode);
10828
}
10929

11030
static unique_ptr<TransactionManager> MySQLCreateTransactionManager(StorageExtensionInfo *storage_info,

‎src/storage/mysql_catalog.cpp

Lines changed: 89 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,102 @@
66
#include "duckdb/parser/parsed_data/drop_info.hpp"
77
#include "duckdb/parser/parsed_data/create_schema_info.hpp"
88
#include "duckdb/main/attached_database.hpp"
9+
#include "duckdb/main/secret/secret_manager.hpp"
910

1011
namespace duckdb {
1112

12-
MySQLCatalog::MySQLCatalog(AttachedDatabase &db_p, const string &path, AccessMode access_mode)
13-
: Catalog(db_p), path(path), access_mode(access_mode), schemas(*this) {
14-
default_schema = MySQLUtils::ParseConnectionParameters(path).db;
13+
MySQLCatalog::MySQLCatalog(AttachedDatabase &db_p, string connection_string_p, string attach_path_p, AccessMode access_mode)
14+
: Catalog(db_p), connection_string(std::move(connection_string_p)), attach_path(std::move(attach_path_p)), access_mode(access_mode), schemas(*this) {
15+
default_schema = MySQLUtils::ParseConnectionParameters(connection_string).db;
1516
// try to connect
16-
auto connection = MySQLConnection::Open(path);
17+
auto connection = MySQLConnection::Open(connection_string);
1718
}
1819

1920
MySQLCatalog::~MySQLCatalog() = default;
2021

22+
string EscapeConnectionString(const string &input) {
23+
string result = "\"";
24+
for (auto c : input) {
25+
if (c == '\\') {
26+
result += "\\\\";
27+
} else if (c == '"') {
28+
result += "\\\"";
29+
} else {
30+
result += c;
31+
}
32+
}
33+
result += "\"";
34+
return result;
35+
}
36+
37+
string AddConnectionOption(const KeyValueSecret &kv_secret, const string &name) {
38+
Value input_val = kv_secret.TryGetValue(name);
39+
if (input_val.IsNull()) {
40+
// not provided
41+
return string();
42+
}
43+
string result;
44+
result += name;
45+
result += "=";
46+
result += EscapeConnectionString(input_val.ToString());
47+
result += " ";
48+
return result;
49+
}
50+
51+
unique_ptr<SecretEntry> GetSecret(ClientContext &context, const string &secret_name) {
52+
auto &secret_manager = SecretManager::Get(context);
53+
auto transaction = CatalogTransaction::GetSystemCatalogTransaction(context);
54+
// FIXME: this should be adjusted once the `GetSecretByName` API supports this
55+
// use case
56+
auto secret_entry = secret_manager.GetSecretByName(transaction, secret_name, "memory");
57+
if (secret_entry) {
58+
return secret_entry;
59+
}
60+
secret_entry = secret_manager.GetSecretByName(transaction, secret_name, "local_file");
61+
if (secret_entry) {
62+
return secret_entry;
63+
}
64+
return nullptr;
65+
}
66+
67+
string MySQLCatalog::GetConnectionString(ClientContext &context, const string &attach_path, string secret_name) {
68+
// if no secret is specified we default to the unnamed mysql secret, if it
69+
// exists
70+
bool explicit_secret = !secret_name.empty();
71+
if (!explicit_secret) {
72+
// look up settings from the default unnamed mysql secret if none is
73+
// provided
74+
secret_name = "__default_mysql";
75+
}
76+
auto secret_entry = GetSecret(context, secret_name);
77+
auto connection_string = attach_path;
78+
if (secret_entry) {
79+
// secret found - read data
80+
const auto &kv_secret = dynamic_cast<const KeyValueSecret &>(*secret_entry->secret);
81+
string new_connection_info;
82+
83+
new_connection_info += AddConnectionOption(kv_secret, "user");
84+
new_connection_info += AddConnectionOption(kv_secret, "password");
85+
new_connection_info += AddConnectionOption(kv_secret, "host");
86+
new_connection_info += AddConnectionOption(kv_secret, "port");
87+
new_connection_info += AddConnectionOption(kv_secret, "database");
88+
new_connection_info += AddConnectionOption(kv_secret, "socket");
89+
new_connection_info += AddConnectionOption(kv_secret, "ssl_mode");
90+
new_connection_info += AddConnectionOption(kv_secret, "ssl_ca");
91+
new_connection_info += AddConnectionOption(kv_secret, "ssl_capath");
92+
new_connection_info += AddConnectionOption(kv_secret, "ssl_cert");
93+
new_connection_info += AddConnectionOption(kv_secret, "ssl_cipher");
94+
new_connection_info += AddConnectionOption(kv_secret, "ssl_crl");
95+
new_connection_info += AddConnectionOption(kv_secret, "ssl_crlpath");
96+
new_connection_info += AddConnectionOption(kv_secret, "ssl_key");
97+
connection_string = new_connection_info + connection_string;
98+
} else if (explicit_secret) {
99+
// secret not found and one was explicitly provided - throw an error
100+
throw BinderException("Secret with name \"%s\" not found", secret_name);
101+
}
102+
return connection_string;
103+
}
104+
21105
void MySQLCatalog::Initialize(bool load_builtin) {
22106
}
23107

@@ -63,7 +147,7 @@ bool MySQLCatalog::InMemory() {
63147
}
64148

65149
string MySQLCatalog::GetDBPath() {
66-
return path;
150+
return attach_path;
67151
}
68152

69153
DatabaseSize MySQLCatalog::GetDatabaseSize(ClientContext &context) {

‎src/storage/mysql_transaction.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ namespace duckdb {
99

1010
MySQLTransaction::MySQLTransaction(MySQLCatalog &mysql_catalog, TransactionManager &manager, ClientContext &context)
1111
: Transaction(manager, context), access_mode(mysql_catalog.access_mode) {
12-
connection = MySQLConnection::Open(mysql_catalog.path);
12+
connection = MySQLConnection::Open(mysql_catalog.connection_string);
1313
}
1414

1515
MySQLTransaction::~MySQLTransaction() = default;

‎test/sql/attach_secret.test

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ CREATE SECRET mysql_secret (
3737
statement ok
3838
ATTACH '' AS secret_attach (TYPE MYSQL, SECRET mysql_secret)
3939

40+
query I
41+
SELECT path FROM duckdb_databases() WHERE database_name='secret_attach'
42+
----
43+
(empty)
44+
4045
statement ok
4146
DETACH secret_attach
4247

0 commit comments

Comments
 (0)