feat(transports/http): redact Headers, bound shutdown, fix redirect case#74
Merged
Merged
Conversation
Security audit follow-up. Three independent fixes:
- Config.String() redacts Headers values so an accidental log.Info(cfg)
or fmt.Sprintf("%v", cfg) can't leak Authorization / X-API-Key.
Mirrors the existing transports/datadog redaction pattern.
- defaultCheckRedirect compares hosts case-insensitively so legitimate
same-host redirects with mixed-case URLs aren't refused. Cross-host
refusal still fires; ports are still strict.
- New Config.ShutdownTimeout (default 5s) bounds Close. Outbound
requests now use http.NewRequestWithContext; on a wedged endpoint
Close cancels in-flight requests when the timeout elapses instead
of waiting up to Client.Timeout (30s) per pending batch. Queued-
but-unsent entries surface via OnError as context.Canceled.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2 tasks
theogravity
added a commit
that referenced
this pull request
May 3, 2026
monorel v0.12+ runs `go mod tidy` per sub-module as part of `pr` and
`release` to refresh go.sum entries. Every sub-module's go.mod
requires `go 1.25.0`, but the GitHub runner default is older and the
monorel CI action sets GOTOOLCHAIN=local, so the toolchain doesn't
auto-upgrade. The tidy step then fails with:
go: go.mod requires go >= 1.25.0 (running go 1.24.13;
GOTOOLCHAIN=local)
This bit the release-pr workflow when it ran on PR #76's merge commit
because monorel's `latest` tag had moved past v0.11.0 between PR #74's
release (v0.10.x) and PR #76's release (v0.13.0). The action version is
pinned (`@v0.11.0`), but the action invokes the binary under `latest`.
Add an `actions/setup-go@v5` step with `go-version: '1.25'` before the
monorel invocation in both release-pr.yml and release.yml. Mirrors the
setup-go pattern that ci.yml already uses for the regular CI matrix.
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Security audit follow-up. Three independent fixes in
transports/http:Config.String()redactsHeadersvalues so an accidentallog.Info(cfg)orfmt.Sprintf("%v", cfg)can't leakAuthorization/X-API-Keyetc. Mirrors the existingtransports/datadogredaction pattern. Header keys stay visible for debuggability.defaultCheckRedirectcompares hosts case-insensitively (strings.EqualFold) so legitimate same-host redirects with mixed-case URLs aren't refused. Cross-host refusal still fires; ports are still strict.Config.ShutdownTimeout(default 5s) boundsClose. Outbound requests now usehttp.NewRequestWithContext; on a wedged endpoint,Closecancels in-flight requests when the timeout elapses instead of waiting up toClient.Timeout(30s) per pending batch. Queued-but-unsent entries surface viaOnErrorascontext.Canceled.Test plan
go test -race -count=1 ./...passes fortransports/http,transports/datadog,transports/central(every module that importstransports/http)gofmt -landgo vetcleanTestHTTP_CloseBoundedByShutdownTimeoutconfirms the bound is enforced (~100ms vs the previous 30s worst-case)TestHTTP_ConfigStringRedactsHeadersandTestDatadog_ConfigHTTPHeadersRedactedcover both the http transport directly and the realistic downstream wrapper credential-leak surfaceTestDefaultCheckRedirectCaseInsensitivecovers the redirect change (5 subtests: same/different case, with/without port, cross-host, different-port)🤖 Generated with Claude Code