From 650f5a198fb342eee356e0233443fed808836208 Mon Sep 17 00:00:00 2001
From: shankar0123 <skreddy040@gmail.com>
Date: Sun, 3 May 2026 19:07:21 +0000
Subject: [PATCH] fix: collapse identical if/else branches in Account handler
 (CodeQL #25)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CodeQL alert #25 (go/duplicate-branches) on internal/api/handler/
acme.go::ACMEHandler.Account flagged that 'if readOnly { ... } else
{ ... }' had byte-identical bodies — both setting the same
Content-Type: application/json header. The 'readOnly' bool was
threaded through the function as a placeholder for differentiated
headers (Cache-Control etc. on the POST-as-GET path) that never
landed; both branches collapsed to the same value with no
follow-through.

Audit + fix:
  - The alert is real (verified by re-reading the source); not a
    false positive.
  - The Copilot Autofix Anthropic surfaced was correct in spirit but
    incomplete: it collapsed the if/else but left 'readOnly' as
    dead code (declared at line 395, assigned at lines 400 and 436,
    only read at the now-removed if). golangci-lint's 'unused'
    linter would flag 'readOnly' next.
  - Complete fix: collapse the if/else AND remove the now-unused
    'readOnly' variable + its 2 assignments. Single unconditional
    'w.Header().Set("Content-Type", "application/json")' covers
    both paths (RFC 8555 §6.3 POST-as-GET + §7.3.2 / §7.3.6 update
    + deactivation all return the same account JSON shape — no spec
    rationale for differentiating headers).

Verified locally: 'gofmt -l .' clean; 'go vet ./...' clean;
'go test -short -count=1 ./internal/api/handler/' green; 'grep
readOnly' on the file returns only the new explanatory comment
(no live references).

The alert was first detected in commit 44a85d6 (Phase 1b) — the
duplicate has been sitting in the codebase since the Account
handler shipped. No functional regression for any RFC 8555 client
(cert-manager, lego, Posh-ACME): same status code, same headers,
same body.
---
 internal/api/handler/acme.go | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/internal/api/handler/acme.go b/internal/api/handler/acme.go
index 737b49d..d8b7568 100644
--- a/internal/api/handler/acme.go
+++ b/internal/api/handler/acme.go
@@ -390,14 +390,10 @@ func (h ACMEHandler) Account(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	var (
-		updated  *domain.ACMEAccount
-		readOnly bool
-	)
+	var updated *domain.ACMEAccount
 	// Empty body or empty JSON object → POST-as-GET (§6.3).
 	trimmed := trimBody(verified.Payload)
 	if len(trimmed) == 0 || string(trimmed) == "{}" {
-		readOnly = true
 		updated = verified.Account
 	} else {
 		var req acme.AccountUpdateRequest
@@ -433,18 +429,19 @@ func (h ACMEHandler) Account(w http.ResponseWriter, r *http.Request) {
 		if updated == nil {
 			// Empty status + nil contact → no-op; treat as POST-as-GET.
 			updated = verified.Account
-			readOnly = true
 		}
 	}
 
 	if nonce, err := h.svc.IssueNonce(r.Context()); err == nil {
 		w.Header().Set("Replay-Nonce", nonce)
 	}
-	if readOnly {
-		w.Header().Set("Content-Type", "application/json")
-	} else {
-		w.Header().Set("Content-Type", "application/json")
-	}
+	// RFC 8555 §6.3 (POST-as-GET) and §7.3.2 / §7.3.6 (account update +
+	// deactivation) both return the same account JSON shape, so a single
+	// unconditional Content-Type header covers both paths. Earlier code
+	// kept this behind an if/readOnly switch as a placeholder for
+	// differentiated headers (Cache-Control etc.) that never landed;
+	// CodeQL flagged the duplicate branches as quality issue #25.
+	w.Header().Set("Content-Type", "application/json")
 	w.WriteHeader(http.StatusOK)
 	_ = json.NewEncoder(w).Encode(
 		acme.MarshalAccount(updated, h.accountOrdersURL(r, profileID, updated.AccountID)),