feat: optional external_id for S3 role-based access - #698
Merged
Conversation
ClickHouse Cloud accepts an `external_id` alongside `role_arn` in `extra_credentials(...)` and forwards it as the `ExternalId` parameter of the AWS STS `AssumeRole` call. That lets the assumed role's trust policy require a shared secret through an `sts:ExternalId` condition, which mitigates the confused deputy problem and satisfies cloud security scanners that flag cross-account trust policies with no external ID or MFA requirement. `clickhouse_s3source()` had no way to pass it, so any project using the macro was limited to a bare `role_arn` trust policy. Add an optional `external_id` argument, resolvable either per model or from the `vars` S3 configuration, and emit it only when a `role_arn` is present. Specifying `external_id` on its own raises, mirroring the existing validation for a half-specified access key pair. The new argument is appended to both signatures so existing positional callers keep working.
jessedobbelaere
force-pushed
the
feat/s3-external-id
branch
from
August 3, 2026 18:49
afa3086 to
fdda526
Compare
* upstream/main: fix: quote S3 compression argument correctly (ClickHouse#695) # Conflicts: # tests/unit/test_s3source_clause.py
koletzilla
self-requested a review
August 7, 2026 14:40
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
ClickHouse Cloud accepts an
external_idalongsiderole_arninextra_credentials(...)and forwards it as theExternalIdparameter of the AWS STSAssumeRolecall. This is something I explicitly requested to have a year ago with Clickhouse Cloud, and i noticed it has been introduced somewhere in the past year because I noticed it in the docs:That lets the assumed role's trust policy require a shared secret through an
sts:ExternalIdcondition, which mitigates the confused deputy problem and satisfies flagged cross-account trust policies with no external ID requirement.clickhouse_s3source()had no way to pass it, so any project using the macro was limited to just therole_arntrust policy. Add an optionalexternal_idargument, resolvable either per model or from thevarsS3 configuration, and emit it only when arole_arnis present. Specifyingexternal_idon its own raises, mirroring the existing validation for a half-specified access key pair.The new argument is optional so there should not be any risk in merging this.
Both compile to:
Checklist
Delete items not relevant to your PR: