Skip to content

Added a function to allow creation of database with owner #37

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Added a function to allow creation of database with owner #37

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
c824d96
Added function to create postgres database with owner
Nov 5, 2025
a686ef3
Remonved using statement in NoPublicApiChanges.Run.approved.cs
Nov 5, 2025
3375b7e
Fixed test
Nov 5, 2025
31d7eac
Merge branch 'main' into main
droyad Nov 6, 2025
8a002b2
Apply suggestion from @droyad
droyad Nov 6, 2025
daea170
Update src/dbup-postgresql/PostgresqlExtensions.cs
droyad Nov 6, 2025
a9f9ae3
Addressed PR comments around SQLi and using if/else for creating a da…
Nov 6, 2025
7e17862
Flipped boolean logic around
Nov 6, 2025
0a58605
formatting
Nov 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions src/Tests/ApprovalFiles/NoPublicApiChanges.Run.approved.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,7 @@ public static void PostgresqlDatabase(this DbUp.SupportedDatabasesForEnsureDatab
public static DbUp.Builder.UpgradeEngineBuilder PostgresqlDatabase(this DbUp.Builder.SupportedDatabases supported, string connectionString, string schema, DbUp.Postgresql.PostgresqlConnectionOptions connectionOptions) { }
public static DbUp.Builder.UpgradeEngineBuilder PostgresqlDatabase(this DbUp.Builder.SupportedDatabases supported, string connectionString, string schema, System.Security.Cryptography.X509Certificates.X509Certificate2 certificate) { }
public static void PostgresqlDatabase(this DbUp.SupportedDatabasesForDropDatabase supported, string connectionString, DbUp.Engine.Output.IUpgradeLog logger, DbUp.Postgresql.PostgresqlConnectionOptions connectionOptions) { }
public static void PostgresqlDatabase(this DbUp.SupportedDatabasesForDropDatabase supported, string connectionString, DbUp.Engine.Output.IUpgradeLog logger, System.Security.Cryptography.X509Certificates.X509Certificate2 certificate) { }
public static void PostgresqlDatabase(this DbUp.SupportedDatabasesForEnsureDatabase supported, string connectionString, DbUp.Engine.Output.IUpgradeLog logger, DbUp.Postgresql.PostgresqlConnectionOptions connectionOptions) { }
public static void PostgresqlDatabase(this DbUp.SupportedDatabasesForEnsureDatabase supported, string connectionString, DbUp.Engine.Output.IUpgradeLog logger, System.Security.Cryptography.X509Certificates.X509Certificate2 certificate) { }
public static void PostgresqlDatabase(this DbUp.SupportedDatabasesForEnsureDatabase supported, string connectionString, DbUp.Engine.Output.IUpgradeLog logger, DbUp.Postgresql.PostgresqlConnectionOptions connectionOptions, string owner) { }
}
namespace DbUp.Postgresql
{
Expand Down
90 changes: 64 additions & 26 deletions src/dbup-postgresql/PostgresqlExtensions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
/// </summary>
public static class PostgresqlExtensions
{
private static readonly string pattern= @"(?i)Search\s?Path=([^;]+)";
private static readonly string pattern = @"(?i)Search\s?Path=([^;]+)";
/// <summary>
/// Creates an upgrader for PostgreSQL databases.
/// </summary>
Expand Down Expand Up @@ -172,10 +172,11 @@ public static void PostgresqlDatabase(this SupportedDatabasesForEnsureDatabase s
/// <param name="connectionString">The connection string.</param>
/// <param name="logger">The <see cref="DbUp.Engine.Output.IUpgradeLog"/> used to record actions.</param>
/// <param name="certificate">Certificate for securing connection.</param>
/// <returns></returns>
public static void PostgresqlDatabase(this SupportedDatabasesForEnsureDatabase supported, string connectionString, IUpgradeLog logger, X509Certificate2 certificate)
{
var options = new PostgresqlConnectionOptions
{
{
ClientCertificate = certificate
};
PostgresqlDatabase(supported, connectionString, logger, options);
Expand All @@ -189,11 +190,30 @@ public static void PostgresqlDatabase(this SupportedDatabasesForEnsureDatabase s
/// <param name="logger">The <see cref="DbUp.Engine.Output.IUpgradeLog"/> used to record actions.</param>
/// <param name="connectionOptions">Connection options to set SSL parameters</param>
public static void PostgresqlDatabase(
this SupportedDatabasesForEnsureDatabase supported,
string connectionString,
IUpgradeLog logger,
this SupportedDatabasesForEnsureDatabase supported,
string connectionString,
IUpgradeLog logger,
PostgresqlConnectionOptions connectionOptions
)
{
PostgresqlDatabase(supported, connectionString, logger, connectionOptions, null);
}

/// <summary>
/// Ensures that the database specified in the connection string exists, assigning an owner at creation time.
/// </summary>
/// <param name="supported">Fluent helper type.</param>
/// <param name="connectionString">The connection string.</param>
/// <param name="logger">The <see cref="DbUp.Engine.Output.IUpgradeLog"/> used to record actions.</param>
/// <param name="connectionOptions">Connection SSL to customize SSL behaviour</param>
/// <param name="owner">Role to own the new database during creation (adds 'WITH OWNER = "role"').</param>
public static void PostgresqlDatabase(
this SupportedDatabasesForEnsureDatabase supported,
string connectionString,
IUpgradeLog logger,
PostgresqlConnectionOptions connectionOptions,
string owner
)
{
if (supported == null) throw new ArgumentNullException("supported");

Expand All @@ -205,14 +225,16 @@ PostgresqlConnectionOptions connectionOptions
if (logger == null) throw new ArgumentNullException("logger");

var masterConnectionStringBuilder = new NpgsqlConnectionStringBuilder(connectionString);

var databaseName = masterConnectionStringBuilder.Database;

if (string.IsNullOrEmpty(databaseName) || databaseName.Trim() == string.Empty)
{
throw new InvalidOperationException("The connection string does not specify a database name.");
}

owner = string.IsNullOrWhiteSpace(owner) ? masterConnectionStringBuilder.Username : owner;

Copy link
Member

@droyad droyad Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to CoPilot

The default owner of the new database is the user executing the CREATE DATABASE command

✅

However

The [user[:password]@] part is optional. If omitted, most PostgreSQL clients will default to using the operating system's current username (the user running the client) for authentication

Confirmed via the docs.

So I think the best approach would be to not do this line and keep the existing statement if owner is null. If it's not null the new check for existance and with owner should be used.

Copy link
Contributor Author

@aashikgowda aashikgowda Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The [user[:password]@] part is optional. If omitted, most PostgreSQL clients will default to using the operating system's current username (the user running the client) for authentication

I missed out on this part. Updated as per your suggestion.

masterConnectionStringBuilder.Database = connectionOptions.MasterDatabaseName;

var logMasterConnectionStringBuilder = new NpgsqlConnectionStringBuilder(masterConnectionStringBuilder.ConnectionString);
Expand All @@ -232,9 +254,9 @@ PostgresqlConnectionOptions connectionOptions

// check to see if the database already exists..
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
CommandType = CommandType.Text
})
{
var results = Convert.ToInt32(command.ExecuteScalar());

Expand All @@ -245,13 +267,29 @@ PostgresqlConnectionOptions connectionOptions
}
}

sqlCommandText = $"create database \"{databaseName}\";";
sqlCommandText = $"select 1 from pg_roles where rolname = \'{owner}\' limit 1;";
// check to see if the owner exists..
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL injection vulnerability: the owner parameter is interpolated directly into the SQL query using single quotes. Use parameterized queries with NpgsqlParameter to prevent SQL injection attacks.

Suggested change
sqlCommandText = $"select 1 from pg_roles where rolname = \'{owner}\' limit 1;";
// check to see if the owner exists..
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
sqlCommandText = "select 1 from pg_roles where rolname = @owner limit 1;";
// check to see if the owner exists..
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
command.Parameters.AddWithValue("@owner", owner);

Copilot uses AI. Check for mistakes.
Copy link
Member

@droyad droyad Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well... it's unlikely SQLi would happen this way, but I don't think it hurts to do it this way.

Copy link
Contributor Author

@aashikgowda aashikgowda Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the SQL command to take in parameters instead.

var results = Convert.ToInt32(command.ExecuteScalar());

// if the owner role does not exist, we throw an exception.
if (results == 0)
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential null reference exception: ExecuteScalar() can return null when no rows are found. The code should handle the null case explicitly before converting to Int32, e.g., var results = command.ExecuteScalar() as int? ?? 0;

Copilot uses AI. Check for mistakes.
Copy link
Member

@droyad droyad Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds right based on my SQL Server knowledge. If owner does not exist, no rows are returned (since it doesn't use Count(*).

Copy link
Contributor Author

@aashikgowda aashikgowda Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Convert.ToInt32 can take in a null object and return a zero for null objects.

However, to keep it cleaner, I updated the SQL to return a boolean instead which makes it more readable.

{
throw new InvalidOperationException($"PostgreSQL role '{owner}' does not exist.");
}
}

sqlCommandText = $"create database \"{databaseName}\" with owner = \"{owner}\";";
Copy link

Copilot AI Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQL injection vulnerability: both databaseName and owner are directly interpolated into the CREATE DATABASE statement. While PostgreSQL doesn't support parameterized DDL, consider validating these identifiers (e.g., checking they only contain alphanumeric characters and underscores) or using identifier quoting functions to prevent injection.

Copilot uses AI. Check for mistakes.
Copy link
Member

@droyad droyad Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SQLi via connection string...

Copy link
Contributor Author

@aashikgowda aashikgowda Nov 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed it by formatting using helper functions.


// Create the database...
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
CommandType = CommandType.Text
})
{
command.ExecuteNonQuery();
}
Expand Down Expand Up @@ -347,7 +385,7 @@ PostgresqlConnectionOptions connectionOptions

var masterConnectionStringBuilder = new NpgsqlConnectionStringBuilder(connectionString);

var databaseName = masterConnectionStringBuilder.Database;
var databaseName = masterConnectionStringBuilder.Database;

if (string.IsNullOrEmpty(databaseName) || databaseName.Trim() == string.Empty)
{
Expand Down Expand Up @@ -379,9 +417,9 @@ PostgresqlConnectionOptions connectionOptions

// check to see if the database already exists..
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
CommandType = CommandType.Text
})
{
var results = Convert.ToInt32(command.ExecuteScalar());

Expand All @@ -396,9 +434,9 @@ PostgresqlConnectionOptions connectionOptions
// prevent new connections to the database
sqlCommandText = $"alter database \"{databaseName}\" with ALLOW_CONNECTIONS false;";
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
CommandType = CommandType.Text
})
{
command.ExecuteNonQuery();
}
Expand All @@ -408,9 +446,9 @@ PostgresqlConnectionOptions connectionOptions
// terminate all existing connections to the database
sqlCommandText = $"select pg_terminate_backend(pg_stat_activity.pid) from pg_stat_activity where pg_stat_activity.datname = \'{databaseName}\';";
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
CommandType = CommandType.Text
})
{
command.ExecuteNonQuery();
}
Expand All @@ -421,9 +459,9 @@ PostgresqlConnectionOptions connectionOptions

// drop the database
using (var command = new NpgsqlCommand(sqlCommandText, connection)
{
CommandType = CommandType.Text
})
{
CommandType = CommandType.Text
})
{
command.ExecuteNonQuery();
}
Expand Down
Loading