Skip to content

sql: redact inline credentials in CREATE CONNECTION options#37421

Open
def- wants to merge 1 commit into
MaterializeInc:mainfrom
def-:sql-redact-connection-credentials
Open

sql: redact inline credentials in CREATE CONNECTION options#37421
def- wants to merge 1 commit into
MaterializeInc:mainfrom
def-:sql-redact-connection-credentials

Conversation

@def-

@def- def- commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

ConnectionOptionName::redact_value returned false for every variant, which forced the with-option display macro to unredact the value. Credential-bearing options that carried inline credentials could therefore print live credentials into the redacted-telemetry channel, including mz_sql_text_redacted, catalog create_sql round-trips, and trace attributes.

This PR keeps credential-bearing option values in redacted mode so inline credential values render as <REDACTED>. A SECRET reference is a catalog item name, not the credential itself, so WithOptionValue::Secret renders that name in redacted mode, keeping redacted connection SQL diagnostic.

The parser now handles sensitive CREATE CONNECTION option values with credential-specific semantics. SECRET <name> remains a secret reference, while quoted and bare inline credentials are normalized to Value::String before display. This closes the bare-identifier bypass for forms like USER = myuser, SASL USERNAME = svcacct, ACCESS KEY ID = AKIAEXAMPLE, and CREDENTIAL = cred_leak.

Tests added or modified:

  • Extended test_connection_option_redaction with quoted credentials, SECRET references, and bare credential identifiers.
  • Updated src/sql-parser/tests/testdata/source for the new raw AST and display representation of bare SQL Server USER values.

@def- def- requested a review from aljoscha July 2, 2026 15:13
@def- def- requested a review from a team as a code owner July 2, 2026 15:13
@def- def- force-pushed the sql-redact-connection-credentials branch 3 times, most recently from 866ec5f to febe2fd Compare July 2, 2026 16:43
ConnectionOptionName::redact_value returned false for every variant, which forced the with-option display macro to unredact the value. Credential-bearing options that carried inline credentials could therefore print live credentials into the redacted-telemetry channel, including mz_sql_text_redacted, catalog create_sql round-trips, and trace attributes.

Return true for credential-bearing variants so inline credential values render as '<REDACTED>'. A SECRET reference is a catalog item name, not the credential itself, so WithOptionValue::Secret renders that name in redacted mode, keeping redacted connection SQL diagnostic.

Parse sensitive CREATE CONNECTION option values with credential-specific semantics. SECRET <name> remains a secret reference, while quoted and bare inline credentials are normalized to Value::String before display. This closes the bare-identifier bypass for forms like USER = myuser, SASL USERNAME = svcacct, ACCESS KEY ID = AKIAEXAMPLE, and CREDENTIAL = cred_leak.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@def- def- force-pushed the sql-redact-connection-credentials branch from febe2fd to 9453d70 Compare July 2, 2026 17:00
@def-

def- commented Jul 3, 2026

Copy link
Copy Markdown
Contributor Author

Nightly failure is unrelated, ready for review

@aljoscha aljoscha left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

had one inline comment that would be good to address, but otherwise looks good!

}

impl ConnectionOptionName {
pub(crate) fn value_contains_sensitive_data(&self) -> bool {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should this be an exhaustive destructure, so we don't miss new variants in the future?

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.

2 participants