Skip to content

Redact HTTP error stack traces by default - #2028

Open
HocVoNgThai wants to merge 2 commits into
kafbat:mainfrom
HocVoNgThai:bugfix/exclude-error-stacktraces-by-default
Open

HocVoNgThai wants to merge 2 commits into
kafbat:mainfrom
HocVoNgThai:bugfix/exclude-error-stacktraces-by-default

Conversation

@HocVoNgThai

@HocVoNgThai HocVoNgThai commented Oct 7, 2026 •

Copy link
Copy Markdown

Fixes #2026.

  • Breaking change? An omitted http.error.excludeStackTraces now behaves as true. Set it explicitly to false to keep the previous diagnostic stack trace response.

What changes did you make?

Change the existing property fallback from false to true. Keep the existing redaction marker and explicit overrides. Add README migration guidance and four regression cases using the real WebFlux error response pipeline. Document the reviewed constructor, test methods, and fixture helpers with Javadoc; the documentation follow-up changes no runtime behavior.

Is there anything you'd like reviewers to focus on?

The default behavior change and preserved opt-in diagnostics. This proposal changes only stack trace inclusion, not exception messages, status codes, logging, or response structure.

How Has This Been Tested?

  • No need to
  • Manually
  • Unit checks
  • Integration checks
  • Covered by existing automation

All four JUnit cases pass using the actual production handler in an offline, resource-limited Java 25 container: omitted property, explicit true, explicit false, and a status-error response. The two default-redaction cases fail on unmodified main and pass after the change. git diff --check passes.

The combined focused validation run has 66 passing cases, including the independent permission-validation proposal and documentation checks. Tests use locally available dependency jars rather than a full Gradle build. The full Gradle suite and Checkstyle were not run locally; upstream CI remains required. Existing Lombok/JVM warnings occur on both baseline and patched runs.

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation (e.g. ENVIRONMENT VARIABLES)
  • My changes generate no new compiler warnings relative to the baseline (Sonar remains for CI)
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes (focused classes listed above)
  • Any dependent changes have been merged

Focused tests pass; full project CI/Sonar verification remains required. The one-line fallback change needs no inline comment. This PR has no dependency on another proposal.

Summary by CodeRabbit

  • Bug Fixes
    • HTTP error responses now redact stack traces by default, reducing exposure of diagnostic details. Set http.error.excludeStackTraces or HTTP_ERROR_EXCLUDESTACKTRACES to false to include them.
  • Documentation
    • Clarified the default stack-trace redaction behavior and configuration options. HTTP status codes and other error response fields remain unchanged.

@HocVoNgThai
HocVoNgThai requested review from a team as code owners October 7, 2026 05:30
@kapybro kapybro Bot added status/triage/manual Manual triage in progress and removed status/triage/manual Manual triage in progress labels Oct 7, 2026
@kapybro

kapybro Bot commented Oct 7, 2026

Copy link
Copy Markdown

AI Summary

This issue addresses the lack of default security redaction by changing the http.error.excludeStackTraces property fallback from false to true, ensuring HTTP error stack traces are hidden by default. The solution preserves explicit opt-in diagnostics via configuration overrides and updates the documentation alongside adding validating unit tests.

@kapybro kapybro Bot changed the title BE: Redact HTTP error stack traces by default Redact HTTP error stack traces by default Oct 7, 2026
@kapybro kapybro Bot added scope/backend Related to backend changes type/enhancement En enhancement/improvement to an already existing feature labels Oct 7, 2026
@coderabbitai

coderabbitai Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 08f2642c-baf5-426b-a945-79819631cb29
📥 Commits

Reviewing files that changed from the base of the PR and between 0e7cb9e and 718bfde.

📒 Files selected for processing (2)
  • api/src/main/java/io/kafbat/ui/exception/GlobalErrorWebExceptionHandler.java
  • api/src/test/java/io/kafbat/ui/exception/GlobalErrorWebExceptionHandlerTest.java
🚧 Files skipped from review as they are similar to previous changes (2)
  • api/src/main/java/io/kafbat/ui/exception/GlobalErrorWebExceptionHandler.java
  • api/src/test/java/io/kafbat/ui/exception/GlobalErrorWebExceptionHandlerTest.java

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 4 remain after this review.


📝 Walkthrough

Walkthrough

HTTP error responses now redact stack traces by default. Tests cover omitted and explicit property values. The README documents how to include diagnostic traces and states that other response fields remain unchanged.

Changes

HTTP Error Stack-Trace Redaction

Layer / File(s) Summary
Default redaction and validation
api/src/main/java/io/kafbat/ui/exception/GlobalErrorWebExceptionHandler.java, api/src/test/java/io/kafbat/ui/exception/GlobalErrorWebExceptionHandlerTest.java, README.md
The handler defaults http.error.excludeStackTraces to true. WebFlux tests cover omitted, true, and false values. The README documents the property and environment variable settings.

Priority: ➖ Normal

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix

Suggested reviewers: haarolean

Merge Risk: ⚪ Minimal · up to 718bf

Deployments that omit the property will redact stack traces by default; operators can explicitly set it to false to retain diagnostic traces. No merge-blocking issue was identified.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Issue #2026 is satisfied. The reviewed summary confirms that the unset property now defaults to true, the existing marker and explicit true/false behavior remain, and WebFlux tests cover the def…
Out of Scope Changes check ✅ Passed The README migration guidance and WebFlux regression tests support issue #2026. The incremental Javadoc describes the handler, tests, and fixture helpers that implement or verify that behavior. No unr…
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 9 functions across 2 files.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely describes the primary change: redacting HTTP error stack traces by default.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checks the error trail
The default hides each stack detail
Set false to show the trace
Set true to keep it out of place
The status and messages stay in place

Comment @coderabbitai help to get the list of available commands.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

scope/backend Related to backend changes type/enhancement En enhancement/improvement to an already existing feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Redact HTTP error stack traces by default

1 participant