PLT-287: Harden HTTP logging and header limits#191
PLT-287: Harden HTTP logging and header limits#191kristopherchun wants to merge 9 commits intomainfrom
Conversation
Skip body logging for compressed responses (gzip, br, deflate) in both logResponse() and handleFileLogging() — log headers with a [body omitted] notice instead of dumping garbled binary bytes to structured logs. Cap MaxBodyLogSize default to 10KB when LogBody is enabled and no explicit limit is set, preventing unbounded body logging. Add MaxHeaderBytes to httpserver (default 32KB) to reject oversized request headers with 431 before reaching application code.
📋 API Contract Changes Summary✅ No breaking changes detected - only additions and non-breaking modifications Changed Components:Core FrameworkContract diff saved to artifacts/diffs/core.json Module: authContract diff saved to artifacts/diffs/auth.json Module: cacheContract diff saved to artifacts/diffs/cache.json Module: chimuxContract diff saved to artifacts/diffs/chimux.json Module: databaseContract diff saved to artifacts/diffs/database.json Module: eventbusContract diff saved to artifacts/diffs/eventbus.json Module: eventloggerContract diff saved to artifacts/diffs/eventlogger.json Module: httpclientContract diff saved to artifacts/diffs/httpclient.json Module: httpserverContract diff saved to artifacts/diffs/httpserver.json Module: jsonschemaContract diff saved to artifacts/diffs/jsonschema.json Module: letsencryptContract diff saved to artifacts/diffs/letsencrypt.json Module: logmaskerContract diff saved to artifacts/diffs/logmasker.json Module: reverseproxyContract diff saved to artifacts/diffs/reverseproxy.json Module: schedulerContract diff saved to artifacts/diffs/scheduler.json Artifacts📁 Full contract diffs and JSON artifacts are available in the workflow artifacts. |
There was a problem hiding this comment.
Pull request overview
Hardens HTTP request/response logging and server header limits to reduce log flooding and DoS surface area.
Changes:
- Skip body logging for compressed (
Content-Encoding) responses inhttpclient(structured and file logging paths). - Apply a 10KB default
MaxBodyLogSizecap whenLogBodyis enabled but left unlimited. - Add
MaxHeaderBytestohttpserver, defaulting to 32KB and returning 431 for oversized headers.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| modules/httpserver/config.go | Adds MaxHeaderBytes config and defaulting logic. |
| modules/httpserver/module.go | Wires MaxHeaderBytes into http.Server. |
| modules/httpserver/module_test.go | Adds regression test for oversized headers returning 431 and config defaults. |
| modules/httpclient/config.go | Documents and enforces default 10KB MaxBodyLogSize cap when logging bodies. |
| modules/httpclient/module.go | Skips logging raw body bytes for encoded responses; adds omission note (logger + file). |
| modules/httpclient/module_test.go | Adds tests for gzip omission behavior and MaxBodyLogSize defaulting. |
You can also share your feedback on Copilot code review. Take the survey.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #191 +/- ##
==========================================
+ Coverage 56.66% 59.49% +2.83%
==========================================
Files 82 82
Lines 17660 17706 +46
==========================================
+ Hits 10007 10535 +528
+ Misses 6551 6113 -438
+ Partials 1102 1058 -44
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
- Validate MaxHeaderBytes <= 0 (not just == 0) so negative values cannot silently bypass the 32KB default - Replace NUL-byte header payload with valid ASCII; the original test was passing due to client-side rejection, not server-side enforcement - Account for Go's 4096-byte internal overhead in MaxHeaderBytes check - Assert network error type on connection-reset path instead of silently passing on unexpected errors
- Sanitize id parameter in LogRequest, LogResponse, and LogTransactionToFile before using in file paths (G703) - Add nolint directives for G705 false positives — fmt.Fprintf writes to local log files, not HTTP responses
…om user input Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
- Reduce MaxBodyLogSize default from 10KB to 1KB to keep structured log entries reasonable for log aggregators - Extract shouldOmitResponseBody() helper to check both Content-Encoding and Content-Type, used by logResponse and handleFileLogging - Omit body for binary content types (images, PDFs, protobuf, octet-stream, etc.) even without Content-Encoding - Allow text-based types: text/*, application/json, +json, +xml, etc.
Add an integration test in examples/http-client that bootstraps the httpclient module through the modular framework with verbose logging and DisableCompression enabled, then exercises the transport through an httputil.ReverseProxy — the same wiring reverseproxy uses at module.go:1581. Asserts that log output contains the body-omission notice for gzip responses and does not contain raw binary bytes.
Add nolint annotations for: - G704 (SSRF) in composite.go and dryrun.go: URLs are built from configured backend addresses, not user input - G118 (context leak) in health_checker.go: cancel func is stored on the struct and called in Stop()
The test captured the first HandleFunc("/*") from the mock router,
but setupBackendRoutes registers one per backend using map iteration
(random order). When the "chimera" handler was captured instead of
"legacy", the proxy tried to reach bing.com and failed with 500.
Fix by capturing the last "/*" handler, which is the catch-all from
registerBasicRoutes that properly routes all paths via pattern matching.
|
@intel352 finally got everything passing - initially started out as a 7 file change but had to add/address some gosec errors in lint cause i touched the dir 😄 then just decided to add thew new integration test in the example rather than a separate PR |
Summary
logResponse()andhandleFileLogging()in httpclient now checkContent-Encodingand omit body bytes for gzip/br/deflate responses, logging a[body omitted]notice instead of garbled binary output that floodsstructured log systems like Datadog
MaxBodyLogSizedefault to 10KB — whenLogBodyis enabled but no explicitMaxBodyLogSizeis set,Validate()applies a 10KB cap to prevent unbounded body loggingMaxHeaderBytesto httpserver — defaults to 32KB (down from Go's 1MB default), rejecting oversized request headers with HTTP 431 before reaching application codeNote
I was able to replicate the excess/verbose logging in local environments loading the toolbox and test this locally. Will test thoroughly in dev/stage before merging in a version bump.
Test plan
TestLoggingTransport_GzipBodyNotLogged— structured log path omits compressed body bytesTestLoggingTransport_GzipFileLogging— file logging path omits compressed body bytesTestLoggingTransport_UncompressedBodyStillLogged— plain-text responses are still fully loggedTestConfig_MaxBodyLogSizeDefaultCap— 10KB default applied, explicit values preserved, no cap when LogBody is falseTestMaxHeaderBytes_OversizedHeaderRejected— 431 returned for oversized headers, default validates to 32KB-race