fix(solparq): redact S3 credentials from ClickHouse query errors - #69
Open
picard8472 wants to merge 1 commit into
Open
fix(solparq): redact S3 credentials from ClickHouse query errors#69picard8472 wants to merge 1 commit into
picard8472 wants to merge 1 commit into
Conversation
The S3 archive export SQL built by `build_s3_table_archive_sql` embeds the AWS access key and secret as the second and third arguments of the ClickHouse `s3(...)` table function. On any export failure (bad bucket, network error, ClickHouse rejecting the query), `clickhouse_status_error` put the first 500 chars of the raw SQL — credentials included — into the returned error and any log line carrying it, leaking both in plaintext. Redact the credentials at the single choke point: `clickhouse_status_error` now scans the SQL for each `s3(` / `s3Cluster(` call and replaces the access-key and secret-key literals with `'[REDACTED]'` before truncating, leaving the URL, format, and rest of the statement intact for debugging. Literal parsing honors the same `\'` / `\\` escaping that `escape_sql_string` emits, and truncation is now char-boundary safe. Fixes #63 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #63 — the S3 export path leaked AWS credentials in plaintext through error logs.
build_s3_table_archive_sqlembeds the S3 access key and secret as the 2nd and 3rd arguments of the ClickHouses3(...)table function. On any export failure (bad bucket, network error, ClickHouse rejecting the query),clickhouse_status_errorput the first 500 chars of the raw SQL — credentials included — into the returned error and any log line carrying it.Fix
Redact at the single choke point.
clickhouse_status_errornow runs the SQL through a newredact_s3_credentialsbefore truncating:s3(/s3Cluster(call (matched at a word boundary, case-insensitive) and replaces the access-key and secret-key literals with'[REDACTED]'.\'/\\escaping thatescape_sql_stringemits, so an escaped quote in a secret can't end the redacted span early.Because redaction happens at the one place errors are constructed, it also covers the restore-style
s3(...)form if it is ever routed through this crate.Testing
6 new unit tests in
crates/superbank-solparq/src/clickhouse.rs:clickhouse_status_errorpreview never contains credentialss3(...)calls3(-like text inside identifierscargo test -p superbank-solparq(86 tests),cargo clippy -p superbank-solparq --all-targets --locked -- -D warnings, andcargo fmt --checkall pass.🤖 Generated with Claude Code