Skip to content

Conversation

@tianzhou
Copy link
Member

Fix #174

Copilot AI review requested due to automatic review settings December 17, 2025 16:35
Copy link
Contributor

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 adds automatic detection and configuration for AWS IAM authentication tokens in MySQL and MariaDB connectors. When an AWS IAM token is detected in the password field (by checking for the "X-Amz-Credential" string), the connectors automatically configure the necessary authentication plugins and enable SSL, which is required for AWS RDS IAM authentication.

Key changes:

  • Auto-detection of AWS IAM tokens based on presence of "X-Amz-Credential" in password
  • MySQL connector configures mysql_clear_password auth plugin with null-terminated password buffer
  • Both MySQL and MariaDB connectors automatically enable SSL when AWS IAM token is detected
  • Comprehensive unit tests added to verify AWS IAM token detection and configuration

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.

File Description
src/connectors/mysql/index.ts Adds AWS IAM token detection and configures cleartext authentication plugin with SSL auto-enable
src/connectors/mariadb/index.ts Adds AWS IAM token detection with SSL auto-enable (relies on default mysql_clear_password support)
src/connectors/tests/dsn-parser.test.ts Adds unit tests for AWS IAM token detection and configuration in both MySQL and MariaDB parsers

Comment on lines +10 to +30
it('should detect AWS IAM token and configure cleartext plugin with SSL', async () => {
const awsToken = 'mydb.abc123.us-east-1.rds.amazonaws.com:3306/?Action=connect&DBUser=myuser&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE/20240101/us-east-1/rds-db/aws4_request&X-Amz-Date=20240101T000000Z&X-Amz-SignedHeaders=host&X-Amz-Signature=abc123def456';
const dsn = `mysql://myuser:${encodeURIComponent(awsToken)}@mydb.abc123.us-east-1.rds.amazonaws.com:3306/mydb`;

const config = await parser.parse(dsn);

// Should have authPlugins configured with cleartext plugin
expect(config.authPlugins).toBeDefined();
expect(config.authPlugins?.mysql_clear_password).toBeDefined();

// Should auto-enable SSL for AWS IAM authentication
expect(config.ssl).toEqual({ rejectUnauthorized: false });

// Plugin should return password with null terminator
if (config.authPlugins?.mysql_clear_password) {
const pluginFunc = config.authPlugins.mysql_clear_password();
const result = pluginFunc();
expect(result).toBeInstanceOf(Buffer);
expect(result.toString()).toBe(awsToken + '\0');
}
});
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The test suite lacks coverage for the edge case where a user explicitly sets sslmode=disable but provides an AWS IAM token. This scenario needs a test to verify the expected behavior (either override with SSL enabled or throw a clear error). Additionally, there's no test for when sslmode=require or another SSL mode is already set with an AWS IAM token to verify that the existing SSL configuration is preserved.

Copilot uses AI. Check for mistakes.
Comment on lines +46 to +55
it('should detect AWS IAM token and auto-enable SSL', async () => {
const awsToken = 'mydb.abc123.us-east-1.rds.amazonaws.com:3306/?Action=connect&DBUser=myuser&X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAIOSFODNN7EXAMPLE/20240101/us-east-1/rds-db/aws4_request&X-Amz-Date=20240101T000000Z&X-Amz-SignedHeaders=host&X-Amz-Signature=abc123def456';
const dsn = `mariadb://myuser:${encodeURIComponent(awsToken)}@mydb.abc123.us-east-1.rds.amazonaws.com:3306/mydb`;

const config = await parser.parse(dsn);

// SSL should be auto-enabled for AWS IAM auth
// MariaDB connector includes mysql_clear_password in default permitted plugins
expect(config.ssl).toEqual({ rejectUnauthorized: false });
});
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The test suite lacks coverage for the edge case where a user explicitly sets sslmode=disable but provides an AWS IAM token. This scenario needs a test to verify the expected behavior (either override with SSL enabled or throw a clear error). Additionally, there's no test for when sslmode=require or another SSL mode is already set with an AWS IAM token to verify that the existing SSL configuration is preserved.

Copilot uses AI. Check for mistakes.
Comment on lines +74 to +75
// AWS RDS IAM tokens are ~800+ character strings containing "X-Amz-Credential"
if (url.password && url.password.includes("X-Amz-Credential")) {
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The detection of AWS IAM tokens using string matching on "X-Amz-Credential" could produce false positives if a regular password happens to contain this string. While unlikely, this could lead to unintended behavior. Consider adding additional validation such as checking the password length (AWS IAM tokens are typically 800+ characters) or verifying the presence of multiple AWS signature components (X-Amz-Algorithm, X-Amz-Date, X-Amz-Signature) to make the detection more robust.

Suggested change
// AWS RDS IAM tokens are ~800+ character strings containing "X-Amz-Credential"
if (url.password && url.password.includes("X-Amz-Credential")) {
// AWS RDS IAM tokens are typically 800+ character strings containing multiple X-Amz query parameters
if (
url.password &&
url.password.length >= 800 &&
url.password.includes("X-Amz-Credential") &&
url.password.includes("X-Amz-Algorithm") &&
url.password.includes("X-Amz-Date") &&
url.password.includes("X-Amz-Signature")
) {

Copilot uses AI. Check for mistakes.
// AWS RDS IAM tokens are ~800+ character strings containing "X-Amz-Credential"
// MariaDB connector includes mysql_clear_password in default permitted plugins,
// but AWS IAM authentication requires SSL
if (url.password && url.password.includes("X-Amz-Credential")) {
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

The detection of AWS IAM tokens using string matching on "X-Amz-Credential" could produce false positives if a regular password happens to contain this string. While unlikely, this could lead to unintended behavior. Consider adding additional validation such as checking the password length (AWS IAM tokens are typically 800+ characters) or verifying the presence of multiple AWS signature components (X-Amz-Algorithm, X-Amz-Date, X-Amz-Signature) to make the detection more robust.

Copilot uses AI. Check for mistakes.
Comment on lines +82 to +84
if (config.ssl === undefined) {
config.ssl = { rejectUnauthorized: false };
}
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

If a user explicitly sets sslmode=disable in the DSN but uses an AWS IAM token, this code will override that setting and enable SSL. While AWS IAM authentication does require SSL, the code should either respect the user's explicit setting and fail with a clear error message, or document this override behavior. Consider checking if config.ssl was explicitly set to undefined by the user versus being undefined by default.

Copilot uses AI. Check for mistakes.
Comment on lines +76 to +78
if (config.ssl === undefined) {
config.ssl = { rejectUnauthorized: false };
}
Copy link

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

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

If a user explicitly sets sslmode=disable in the DSN but uses an AWS IAM token, this code will override that setting and enable SSL. While AWS IAM authentication does require SSL, the code should either respect the user's explicit setting and fail with a clear error message, or document this override behavior. Consider checking if config.ssl was explicitly set to undefined by the user versus being undefined by default.

Copilot uses AI. Check for mistakes.
@tianzhou tianzhou merged commit 493b267 into main Dec 17, 2025
8 checks passed
@tianzhou tianzhou deleted the clear_text_plugin branch December 19, 2025 15:42
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.

Use MySQL cleartext plugin when using AWS IAM authentication

2 participants