Skip to content

Quote user and database names in SQL#8

Merged
plumdog merged 1 commit intomainfrom
quote-user-and-database-names
May 21, 2025
Merged

Quote user and database names in SQL#8
plumdog merged 1 commit intomainfrom
quote-user-and-database-names

Conversation

@plumdog
Copy link
Contributor

@plumdog plumdog commented May 21, 2025

No description provided.

@plumdog plumdog requested a review from Copilot May 21, 2025 13:32
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates SQL queries to quote user and database names to avoid identifier issues.

  • Adds double quotes around user names in CREATE, ALTER, and DROP user queries.
  • Adds double quotes around database names in CREATE, ALTER, and DROP database queries.
Comments suppressed due to low confidence (1)

handler/main.ts:403

  • For consistency with the other queries, the username should be enclosed in double quotes in the ALTER DATABASE statement. For example: ... OWNER TO "${userCredentials.username}";.
await adminClient.query(`ALTER DATABASE "${event.ResourceProperties.databaseName}" OWNER TO ${userCredentials.username};`);

@plumdog plumdog force-pushed the quote-user-and-database-names branch from 95a9200 to 6b9f70d Compare May 21, 2025 13:33
@plumdog plumdog force-pushed the quote-user-and-database-names branch from 6b9f70d to 8736003 Compare May 21, 2025 13:34
@plumdog plumdog requested a review from Copilot May 21, 2025 13:34
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR updates SQL statements in handler/main.ts to quote user and database identifiers, ensuring names with special characters are handled correctly.

  • Wraps usernames in double quotes for CREATE, ALTER, and DROP USER queries
  • Wraps database names in double quotes for CREATE, ALTER, DROP, and OWNER queries
Comments suppressed due to low confidence (2)

handler/main.ts:264

  • There are no tests verifying that quoting handles usernames with special characters correctly. Add unit tests for edge cases such as names containing double quotes, hyphens, or uppercase letters to ensure queries are formed as expected.
const createUserQuery = `CREATE USER "${userCredentials.username}" WITH PASSWORD '${userCredentials.password}' CREATEDB LOGIN;`;

handler/main.ts:285

  • Similarly, add tests for database names with special characters (e.g., spaces, quotes) to verify that quoting is applied correctly and avoids failures at runtime.
const createDatabaseQuery = `CREATE DATABASE "${event.ResourceProperties.databaseName}";`;

});

const createUserQuery = `CREATE USER ${userCredentials.username} WITH PASSWORD '${userCredentials.password}' CREATEDB LOGIN;`;
const createUserQuery = `CREATE USER "${userCredentials.username}" WITH PASSWORD '${userCredentials.password}' CREATEDB LOGIN;`;
Copy link

Copilot AI May 21, 2025

Choose a reason for hiding this comment

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

Interpolating userCredentials.username directly into the SQL string can lead to SQL injection if the username contains double quotes or other special characters. Consider using a parameterized identifier escape library (e.g., pg-format) or the client’s identifier-quoting API.

Copilot uses AI. Check for mistakes.
throw e;
}
}

Copy link

Copilot AI May 21, 2025

Choose a reason for hiding this comment

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

Embedding databaseName via string interpolation may allow injection if the name contains quotes or unsafe characters. Use parameterized queries or a safe identifier-escaping function to avoid SQL injection.

Suggested change
validateDatabaseName(event.ResourceProperties.databaseName);

Copilot uses AI. Check for mistakes.
@plumdog plumdog merged commit 5d5b698 into main May 21, 2025
1 check passed
@plumdog plumdog deleted the quote-user-and-database-names branch May 21, 2025 13:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

Comments