fix(httputil): CR/LF log sanitizer barrier (CodeQL #5/#6) + .trivyignore refresh - #162
Merged
Merged
Conversation
CodeQL kept flagging the two HTTP request/response debug logs in userAgentTransport.RoundTrip (CWE-117 log injection, alerts #5/#6) even though the URL already passed through sanitizeURL -> stripControlChars. The stripping used strings.Map, which the taint analyzer does not recognize as a newline barrier, so the sanitizer was invisible to it. Neutralize CR and LF explicitly with strings.ReplaceAll first (the canonical, analyzer-recognized CWE-117 barrier), then keep the strings.Map pass to drop any other control characters. Behavior is unchanged for callers; this makes the existing guarantee legible to CodeQL and closes the alerts. Strengthen TestSanitizeURL_StripsControlChars to assert CR/LF are absent.
Bump the lapsed review date (2026-07-01 -> 2026-11-01; moby/moby/v2 is still pre-release, so the acceptance stands) and document two additional daemon-side docker/docker CVEs that a local trivy image scan surfaces: - CVE-2026-41567: malicious-image arbitrary code execution - CVE-2026-42306: host file overwrite via container-archive race Both are Docker Engine/Moby daemon vulnerabilities with no fix in the v28.x github.com/docker/docker module. dnsweaver uses the SDK as a read-only client and runs no daemon, so the paths are unreachable (govulncheck reachability analysis agrees). The GitLab trivy gate already passes them via --ignore-unfixed; listing them here makes the acceptance explicit and self-documenting.
maxfield-allison
changed the title
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 housekeeping, two related changes.
1. CodeQL log-injection (CWE-117) — alerts #5/#6
Closes the two open code-scanning alerts on the debug logs in
userAgentTransport.RoundTrip(pkg/httputil/client.go).The logged URL already passed through
sanitizeURL()->stripControlChars(), which removed all control characters including CR/LF. But the stripping usedstrings.Map, which CodeQL's taint tracking doesn't recognize as a newline barrier — so the sanitizer was invisible to the analyzer and the alert persisted.stripControlCharsnow removes CR and LF explicitly viastrings.ReplaceAllfirst (the canonical CWE-117 barrier that static analyzers recognize), then keeps thestrings.Mappass for any other control characters.TestSanitizeURL_StripsControlCharsto explicitly assert CR/LF are absent.2.
.trivyignorerefresh2026-07-01->2026-11-01;moby/moby/v2is still pre-release, so the acceptance stands).docker/dockerCVEs that a localtrivy imagescan surfaces (CVE-2026-41567 malicious-image RCE, CVE-2026-42306 host-file-overwrite race). Both are Docker Engine/Moby daemon vulns with no fix in the v28.x module; dnsweaver uses the SDK as a read-only client and runs no daemon, so the paths are unreachable (govulncheck agrees). The GitLab trivy gate already passes them via--ignore-unfixed; listing them makes the acceptance explicit.Context
Part of a security sweep: govulncheck (source) and trivy (image) were both run. Findings are limited to the known, unreachable, no-upstream-fix
docker/dockerdaemon CVEs already accepted by CI policy. The Alpine 3.23.5 base image scans clean (0 vulns).Validation
go build,go vet,go test ./pkg/httputil/pass; local trivy gate (trivy fs --severity CRITICAL,HIGH --ignore-unfixed --ignorefile .trivyignore) exits 0.