feat: support disabling TLS verification for blob storage#2801
feat: support disabling TLS verification for blob storage#2801bmorros94 wants to merge 2 commits into
Conversation
Adds TRACECAT__BLOB_STORAGE_SSL_VERIFY (default: true) and threads it into both S3 client paths in get_storage_client(), enabling self-hosted S3-compatible storage that terminates TLS with a self-signed or otherwise unverifiable certificate. Closes TracecatHQ#2539
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds a configurable TLS verification flag for blob storage (S3/MinIO) clients, enabling self-hosted deployments with self-signed certificates to disable verification.
Changes:
- New
TRACECAT__BLOB_STORAGE_SSL_VERIFYconfig option (defaults totrue) - Passes the
verifyargument to both MinIO and AWS S3 client paths inget_storage_client - Adds unit tests covering enabled/disabled states for both code paths
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| tracecat/config.py | Defines the new SSL verify env-driven config flag. |
| tracecat/storage/blob.py | Forwards the verify flag to the aioboto3 S3 client in both branches. |
| tests/unit/test_storage_blob.py | Adds tests for SSL verify enabled/disabled on MinIO and S3 paths. |
| .env.example | Documents the new env var with default true. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b8ad081fdf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| async with session.client( | ||
| "s3", | ||
| config=_STORAGE_CLIENT_CONFIG, | ||
| verify=config.TRACECAT__BLOB_STORAGE_SSL_VERIFY, |
There was a problem hiding this comment.
When TRACECAT__BLOB_STORAGE_SSL_VERIFY is left at its default, this now passes verify=True explicitly instead of leaving botocore's verify argument as None. In botocore 1.40.61, Session.create_client only reads the configured ca_bundle/AWS_CA_BUNDLE when verify is None, so deployments that trust a private S3/MinIO CA via AWS_CA_BUNDLE will start failing TLS validation even though they did not opt out of verification. Please preserve the default None behavior and only pass False when verification is disabled, or otherwise thread through a bundle path.
Useful? React with 👍 / 👎.
|
✅ No security or compliance issues detected. Reviewed everything up to a1b18f5. Security Overview
Detected Code Changes
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 3/5
- There is a concrete regression risk in
tracecat/storage/blob.py: explicitly passingverify=Truecan bypass botocore’s CA bundle discovery (AWS_CA_BUNDLE/ca_bundle), which may break TLS validation in custom-cert environments. - Because this issue is medium severity (6/10) with high confidence (9/10) and is user-facing for affected deployments, this carries some merge risk rather than being a minor housekeeping concern.
- This PR may still be mergeable with a follow-up, but it is safer to address the
verifyhandling before release to avoid environment-specific connection failures. - Pay close attention to
tracecat/storage/blob.py-verify=Truemay prevent botocore from honoring configured CA bundles.
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Passing verify=True overrode botocore's default CA-bundle resolution (AWS_CA_BUNDLE / the ca_bundle config). Pass verify=None in the default case to preserve that discovery, and only force verify=False when TRACECAT__BLOB_STORAGE_SSL_VERIFY is disabled. Tests updated to match.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a1b18f54f6
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| TRACECAT__BLOB_STORAGE_MAX_ATTEMPTS=5 | ||
| # Verify TLS certificates when connecting to blob storage (S3/MinIO). | ||
| # Set to false for self-hosted S3-compatible storage with self-signed/absent certs. | ||
| TRACECAT__BLOB_STORAGE_SSL_VERIFY=true |
There was a problem hiding this comment.
When a self-hosted operator copies .env.example and sets this to false, the provided Compose deployments still won't pass it into the containers: the blob-storage environment blocks in docker-compose.yml enumerate the endpoint and bucket vars but omit this new key (e.g. lines 78-84, 156-162, 237-243, 306-312, and 428-434). Since Compose .env values are only used for interpolation unless listed in a service environment block, tracecat/config.py falls back to true and the new TLS-disabling option has no effect in the documented Docker Compose path. Please thread ${TRACECAT__BLOB_STORAGE_SSL_VERIFY:-true} through each service that talks to blob storage.
Useful? React with 👍 / 👎.
Summary
Adds a
TRACECAT__BLOB_STORAGE_SSL_VERIFYsetting (defaulttrue) and threads it into both S3 client paths inget_storage_client(), so self-hosted, S3-compatible blob storage that terminates TLS with a self-signed or otherwise unverifiable certificate can be used.Closes #2539.
Why
Self-hosted S3-compatible storage is sometimes served over an endpoint with a self-signed or absent TLS certificate. There was previously no way to disable certificate verification for the blob-storage client, so those deployments could not connect.
Changes
tracecat/config.py— newTRACECAT__BLOB_STORAGE_SSL_VERIFY(bool, defaulttrue), following the existingTRACECAT__BLOB_STORAGE_*convention.tracecat/storage/blob.py— passverify=to bothsession.client("s3", ...)calls (the custom-endpoint/MinIO path and the default AWS path)..env.example— document the new variable.tests/unit/test_storage_blob.py— update the two existing client tests for the new keyword argument, and add tests covering the disabled path on both branches.Notes
true, so existing behavior is unchanged.TRACECAT__BLOB_STORAGE_SSL_VERIFYto match the existingTRACECAT__BLOB_STORAGE_*naming rather than theTRACECAT__S3_SSL_VERIFYsuggested in the issue — happy to rename if you would prefer.Summary by cubic
Adds a TLS verification toggle for blob storage via
TRACECAT__BLOB_STORAGE_SSL_VERIFY(default true) and only overrides verification when disabled, so self-hosted S3/MinIO endpoints with self-signed certs can connect.verify=to both S3 client paths inget_storage_client()(custom endpoint and AWS).TRACECAT__BLOB_STORAGE_SSL_VERIFYin.env.example; add tests for on/off.TRACECAT__BLOB_STORAGE_SSL_VERIFY=falseto disable verification.Written for commit a1b18f5. Summary will update on new commits.