Skip to content

release-24.3: importer: sanitize cloud storage URIs in error messages#167947

Merged
trunk-io[bot] merged 1 commit intorelease-24.3from
blathers/backport-release-24.3-164881
Apr 8, 2026
Merged

release-24.3: importer: sanitize cloud storage URIs in error messages#167947
trunk-io[bot] merged 1 commit intorelease-24.3from
blathers/backport-release-24.3-164881

Conversation

@blathers-crl
Copy link
Copy Markdown

@blathers-crl blathers-crl Bot commented Apr 8, 2026

Backport 1/1 commits from #164881 on behalf of @mw5h.


Summary

  • Sanitize cloud storage URIs in readInputFiles error messages using
    cloud.SanitizeExternalStorageURI to strip credentials before including
    them in user-visible errors.
  • Fixes three error paths: the "too many parsing errors" pgerror and both
    errors.Wrapf calls that annotate errors with the file path.

Resolves: #151884

Release note (bug fix): Fixed a bug where IMPORT error messages
could include unredacted cloud storage credentials from the source
URI. Credentials are now stripped from URIs before they appear in
error messages.


Release justification: Important security fix for an issue encountered by a customer.

Previously, when an import job failed, error messages in
`readInputFiles` included the full cloud storage URI which could
contain credentials (e.g., `CREDENTIALS=<secret>`). The code already
acknowledged this risk with a comment ("Don't log dataFile here because
it could leak auth information") but the error return paths were not
similarly protected.

Use `cloud.SanitizeExternalStorageURI` to strip credentials from the
data file URI before including it in error messages, following the same
pattern as the backup code's `RedactURIForErrorMessage`. This covers
the three error paths in `readInputFiles` that explicitly embed the
file path: the `pgerror.Newf` for "too many parsing errors" and both
`errors.Wrapf` calls that annotate errors with the file path.

Resolves: #151884

Release note (bug fix): Fixed a bug where IMPORT error messages
could include unredacted cloud storage credentials from the source
URI. Credentials are now stripped from URIs before they appear in
error messages.

Co-Authored-By: roachdev-claude <roachdev-claude-bot@cockroachlabs.com>
@blathers-crl blathers-crl Bot force-pushed the blathers/backport-release-24.3-164881 branch from 527ad24 to d4fd271 Compare April 8, 2026 18:09
@blathers-crl blathers-crl Bot requested a review from a team as a code owner April 8, 2026 18:09
@blathers-crl blathers-crl Bot removed the request for review from a team April 8, 2026 18:09
@blathers-crl blathers-crl Bot added the blathers-backport This is a backport that Blathers created automatically. label Apr 8, 2026
@blathers-crl blathers-crl Bot requested a review from mw5h April 8, 2026 18:09
@blathers-crl blathers-crl Bot added the O-robot Originated from a bot. label Apr 8, 2026
@blathers-crl blathers-crl Bot requested a review from michae2 April 8, 2026 18:09
@trunk-io
Copy link
Copy Markdown
Contributor

trunk-io Bot commented Apr 8, 2026

😎 Merged directly without going through the merge queue, as the queue was empty and the PR was up to date with the target branch - details.

@blathers-crl
Copy link
Copy Markdown
Author

blathers-crl Bot commented Apr 8, 2026

Thanks for opening a backport.

Before merging, please confirm that it falls into one of the following categories (select one):

  • Non-production code changes OR fixes for serious issues. Non-production includes test-only changes, build system changes, etc. Serious issues are defined in the policy as correctness, stability, or security issues, data corruption/loss, significant performance regressions, breaking working and widely used functionality, or an inability to detect and debug production issues.
  • Other approved changes. These changes must be gated behind a disabled-by-default feature flag unless there is a strong justification not to. Reference the approved ENGREQ ticket in the PR body (e.g., "Fixes ENGREQ-123").

Add a brief release justification to the PR description explaining your selection.

Also, confirm that the change does not break backward compatibility and complies with all aspects of the backport policy.

All backports must be reviewed by the TL and EM for the owning area.

@blathers-crl blathers-crl Bot added backport Label PR's that are backports to older release branches T-sql-queries SQL Queries Team labels Apr 8, 2026
@blathers-crl
Copy link
Copy Markdown
Author

blathers-crl Bot commented Apr 8, 2026

It looks like your PR touches production code but doesn't add or edit any test code. Did you consider adding tests to your PR?

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@cockroach-teamcity
Copy link
Copy Markdown
Member

This change is Reviewable

Copy link
Copy Markdown
Collaborator

@michae2 michae2 left a comment

Choose a reason for hiding this comment

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

LGTM

@trunk-io trunk-io Bot merged commit 9329cf9 into release-24.3 Apr 8, 2026
20 checks passed
@blathers-crl
Copy link
Copy Markdown
Author

blathers-crl Bot commented Apr 8, 2026

✅ PR #167947 is compliant with backport policy

Confidence: high
Critical bug criteria met: [Stability or security issues]
Backward compatible: true
Explanation: The pull request addresses a critical security issue by sanitizing cloud storage URIs in error messages, which previously could have unintentionally exposed sensitive credentials. The provided changes in the PR strip credentials from URIs before they appear in user-visible error messages using the cloud.SanitizeExternalStorageURI function. The modifications within the readInputFiles function showcase these changes effectively by substituting the raw URI with its sanitized version in error logging and exception wrapping. This fix is essential for maintaining the security and integrity of data, thus qualifying under the 'Stability or security issues' criterion for critical bugs. The PR is also marked with 'Release justification: Important security fix for an issue encountered by a customer', further supporting the critical nature of these changes. Given these factors, the PR can bypass the feature flag requirement for critical security fixes, making it compliant with the backport policy.

ENGREQ Check Passed: No ENGREQ required (non-production code or serious issues).

🦉 Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.

@trunk-io trunk-io Bot deleted the blathers/backport-release-24.3-164881 branch April 8, 2026 18:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport Label PR's that are backports to older release branches blathers-backport This is a backport that Blathers created automatically. O-robot Originated from a bot. T-sql-queries SQL Queries Team target-release-24.3.32

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants