mirror of
https://github.com/shankar0123/certctl.git
synced 2026-06-07 16:01:30 +00:00
CI run #571 (commitaf5c392, "Hotfix #12 — CodeQL #34 go/reflected-xss in etag.go") failed: internal/api/middleware/etag.go:261:11: QF1008: could remove embedded field "ResponseWriter" from selector (staticcheck) hdr := r.ResponseWriter.Header() Root cause: etagRecorder embeds http.ResponseWriter: type etagRecorder struct { http.ResponseWriter body *bytes.Buffer status int headerWritten bool headerWrittenOnWire bool bodyTruncated bool } etagRecorder DOES override Write() and WriteHeader() — those buffer / track instead of writing through. So r.ResponseWriter.Write(b) and r.ResponseWriter.WriteHeader(s) ARE intentional embedded-field selectors (calling the recorder's own Write would recurse infinitely; calling its WriteHeader would skip the wire flush). staticcheck recognizes those as load-bearing and doesn't flag. But etagRecorder does NOT override Header(). So r.ResponseWriter.Header() and r.Header() are equivalent — staticcheck QF1008 wants the shorter form. The Hotfix #12 change added a new r.ResponseWriter.Header() that I missed. Fix: Change r.ResponseWriter.Header() → r.Header() at line 261 (the Content-Type defense added in Hotfix #12). Behavior is byte- identical: r.Header() is the promoted method from the embedded ResponseWriter. Added a comment block immediately above the fix explaining why the neighboring r.ResponseWriter.WriteHeader / r.ResponseWriter.Write calls intentionally KEEP the explicit selector (overridden methods → embedded form required to bypass recursion). Future engineers won't get confused by the asymmetric pattern. Hotfix #13 (signer FileDriver path-injection — local commit38f86bc, not yet pushed) does NOT have the same risk: FileDriver has no embedded struct / interface, only direct fields, so QF1008 can't apply. Verification (sandbox constraints — Go unavailable): • Manual syntax inspection: brace count balanced (27/27), paren count balanced (53/53). Diff +9/-1. • No remaining r.ResponseWriter.Header() in the file (verified via grep — empty match). • All 48 CI guards pass. • Other CI noise on run #571 (windows-latest syscall.Stat_t, Node.js 20 deprecation warnings) is PRE-EXISTING and not introduced by either Hotfix #12 or #13 — see the failure log: undefined: syscall.Stat_t fires in internal/deploy/ownership.go which neither hotfix touched. Ground-truth: origin/master tipaf5c392verified via GitHub API. Local is at38f86bc(Hotfix #13) which the operator hasn't pushed yet; this commit lands on top. After push the order is:af5c392→38f86bc→ <this>. Operator: please run `make verify` from the repo root before pushing — sandbox can't run staticcheck/go vet/go test.
This commit is contained in:
@@ -258,7 +258,15 @@ func (r *etagRecorder) writeHeadersToWire() {
|
|||||||
// handlers (they all set Content-Type) and a safe guard against
|
// handlers (they all set Content-Type) and a safe guard against
|
||||||
// a future handler bug that would otherwise let the browser
|
// a future handler bug that would otherwise let the browser
|
||||||
// content-sniff a JSON body as text/html.
|
// content-sniff a JSON body as text/html.
|
||||||
hdr := r.ResponseWriter.Header()
|
//
|
||||||
|
// Drop the embedded-field selector for Header() — etagRecorder
|
||||||
|
// doesn't override Header(), so r.Header() resolves to the
|
||||||
|
// embedded ResponseWriter.Header() (staticcheck QF1008). The
|
||||||
|
// neighboring r.ResponseWriter.WriteHeader / r.ResponseWriter.Write
|
||||||
|
// calls intentionally KEEP the explicit selector because
|
||||||
|
// etagRecorder.Write / etagRecorder.WriteHeader override them
|
||||||
|
// and the embedded form is required to bypass recursion.
|
||||||
|
hdr := r.Header()
|
||||||
if hdr.Get("Content-Type") == "" {
|
if hdr.Get("Content-Type") == "" {
|
||||||
hdr.Set("Content-Type", "application/json; charset=utf-8")
|
hdr.Set("Content-Type", "application/json; charset=utf-8")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user