Skip to content

fix(rest): Filter sensitive headers from error logs (#2117)#2130

Merged
blackmwk merged 1 commit intoapache:mainfrom
cmackenzie1-contrib:fix/redact-sensitive-headers-in-error-logs
Feb 13, 2026
Merged

fix(rest): Filter sensitive headers from error logs (#2117)#2130
blackmwk merged 1 commit intoapache:mainfrom
cmackenzie1-contrib:fix/redact-sensitive-headers-in-error-logs

Conversation

@cmackenzie1
Copy link
Contributor

@cmackenzie1 cmackenzie1 commented Feb 9, 2026

Which issue does this PR close?

What changes are included in this PR?

Sensitive headers like Set-Cookie, Authorization, and Cookie were being included in error messages, potentially exposing authentication tokens and session information in logs.

This change replaces sensitive headers with [REDACTED] from error context rather than logging their values, preventing credential leakage.

This option can be disabled using REST_CATALOG_PROP_DISABLE_HEADER_REDACTION=true

Are these changes tested?

Covered by unit tests.

@cmackenzie1 cmackenzie1 force-pushed the fix/redact-sensitive-headers-in-error-logs branch from 6e69dd0 to 6a2bda1 Compare February 9, 2026 21:46
.filter(|(name, _)| !is_sensitive_header(name.as_str()))
.filter_map(|(name, value)| value.to_str().ok().map(|v| (name.as_str(), v)))
.collect();
format!("{filtered:?}")
Copy link
Contributor

Choose a reason for hiding this comment

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

When we say redacting sensitive header, I think we replace actually content with things like xxxx rather than removing them?

Copy link
Contributor

Choose a reason for hiding this comment

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

The reason we want to keep them rather than removing them directly is that if maybe confusing when missing these headers when debugging. Also, I would prefer to add a config to disable the redaction. We could redact these header by default, but we should allow user to disable it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the feedback @blackmwk! Updated to redact instead of filter using [REDACTED] as a value and made it configurable.

Copy link
Contributor

@blackmwk blackmwk left a comment

Choose a reason for hiding this comment

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

Thanks @cmackenzie1 for this pr, this is quite useful, just one comment.

Sensitive headers like Set-Cookie, Authorization, and Cookie were being
included in error messages, potentially exposing authentication tokens
and session information in logs.

This change replaces sensitive headers with `[REDACTED]` from error context
rather than logging their values, preventing credential leakage.

This option can be disabled using
`REST_CATALOG_PROP_DISABLE_HEADER_REDACTION=true`
@cmackenzie1 cmackenzie1 force-pushed the fix/redact-sensitive-headers-in-error-logs branch from 6a2bda1 to d2abcab Compare February 12, 2026 18:33
@cmackenzie1 cmackenzie1 requested a review from blackmwk February 12, 2026 18:36
Copy link
Contributor

@blackmwk blackmwk left a comment

Choose a reason for hiding this comment

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

The method name is a little odd, but it's an internal method, so I think it's fine to go. Thanks @cmackenzie1 for this pr!

///
/// If `disable_redaction` is true, returns all headers without redaction.
/// Otherwise, replaces sensitive header values with "[REDACTED]".
fn format_headers_redacted(headers: &HeaderMap, disable_redaction: bool) -> String {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
fn format_headers_redacted(headers: &HeaderMap, disable_redaction: bool) -> String {
fn format_headers(headers: &HeaderMap, disable_redaction: bool) -> String {

@blackmwk blackmwk merged commit 6c197fe into apache:main Feb 13, 2026
17 checks passed
@cmackenzie1 cmackenzie1 deleted the fix/redact-sensitive-headers-in-error-logs branch February 13, 2026 04:41
vustef pushed a commit to RelationalAI/iceberg-rust that referenced this pull request Feb 13, 2026
* Use uv instead of pip for python packages (apache#2129)

* refactor(storage): Reorganize storage code into a new module (apache#2109)

## Which issue does this PR close?

- Closes #.

## What changes are included in this PR?
- Add a new module `storage` within `io` and put traits there

## Are these changes tested?
Relying on the existing tests

* fix: Interpret s3tables warehouse as table_location not metadata loca… (apache#2115)

## Which issue does this PR close?

AWS
[Docs](https://docs.aws.amazon.com/AmazonS3/latest/userguide/s3-tables-tables.html)
state:


> When you create a table, Amazon S3 automatically generates a warehouse
location for the table. This is a unique S3 location that stores objects
associated with the table. The following example shows the format of a
warehouse location:

```
s3://63a8e430-6e0b-46f5-k833abtwr6s8tmtsycedn8s4yc3xhuse1b--table-s3
```

We were previously interpreting this as as a metadata location (i.e. the
path to the metadata.json file), this changes the code use it as a table
location.

- Closes apache#2114

## What changes are included in this PR?

Change how we construct the MetadataLocation object.

## Are these changes tested?

There never appears to have been a test, and I don't have an AWS account
to verify this. Note the test was initially developed by Claude Code and
refined by me.

* fix(rest): Filter sensitive headers from error logs (apache#2117) (apache#2130)

* Merge upstream

* .

* .

* Fix merge mistakes

* .

---------

Co-authored-by: blackmwk <liurenjie1024@outlook.com>
Co-authored-by: Shawn Chang <yxchang@amazon.com>
Co-authored-by: emkornfield <emkornfield@gmail.com>
Co-authored-by: Cole Mackenzie <colemackenzie1@gmail.com>
gbrgr pushed a commit to RelationalAI/iceberg-rust that referenced this pull request Feb 13, 2026
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.

Sensitive Header Leaks in Error Responses

2 participants