security: close CodeQL #17 (log injection) + #23 (SSRF false-positive reopen)

Two CodeQL alerts in one sweep — both medium-impact follow-ups
on already-merged guards.

Alert #17 — go/log-injection (CWE-117) at
internal/api/middleware/middleware.go:58:

  log.Printf("[%s] %s %s %d %v", requestID, r.Method, r.URL.Path, ...)

  r.Method and r.URL.Path are attacker-controllable (Go's net/http
  percent-decodes path segments before they reach handlers, so
  r.URL.Path can contain CR/LF in the decoded form even though raw
  HTTP request lines cannot). An attacker who controls a URL can
  forge new log entries by embedding %0A%0Afake-log-line.

  Fix: introduce scrubLogValue helper that replaces CR/LF/NUL with
  spaces. Apply to both r.Method and r.URL.Path. Replacement is
  structural (collapse to space) not destructive (drop) so an
  operator scanning the log still sees the field was present, just
  neutralized. Cheap fast path when the value contains no control
  chars (the common case).

  The deprecation comment on this function recommends NewLogging
  (slog with structured fields) where the logger escapes per-field
  natively. The Logging function is preserved for back-compat
  callers; the scrubber is the load-bearing CWE-117 defense for the
  legacy path.

Alert #23 — go/request-forgery (CWE-918) at scep_probe.go:271:

  CodeQL reopened the alert after commit e6919cd. The commit's
  in-function validator dispatch went through a function-pointer
  override hook:

    validateURL := s.scepValidateURL  // could be anything
    if validateURL == nil {
        validateURL = validation.ValidateSafeURL
    }
    if err := validateURL(rawURL); err != nil { ... }

  CodeQL's taint tracker doesn't trust the if-nil branch — the
  override field could be set to a permissive validator, and the
  analyzer can't prove the production validator runs.

  Fix: invert the dispatch. Always call validation.ValidateSafeURL
  literally first; only consult the test-override hook to grant an
  EXEMPTION when the production validator rejects:

    if err := validation.ValidateSafeURL(rawURL); err != nil {
        if s.scepValidateURL == nil || s.scepValidateURL(rawURL) != nil {
            return ... validate url error
        }
    }

  Same applies to ProbeSCEP's entry-point validator. Both call sites
  now have the literal validation.ValidateSafeURL call in-scope of
  the sink (client.Do), which CodeQL recognizes as a sanitizer.

  Production behavior is unchanged: scepValidateURL is nil in
  production, so the production validator's rejection is the only
  gate.

  Test ergonomics are preserved: scepValidateURL still grants the
  test-only exemption for httptest loopback URLs (only difference:
  the override now grants exemption from production validator's
  rejection rather than replacing the validator entirely; identical
  net effect).

Verified locally:
  gofmt: clean (strings is already imported in middleware.go).
  go vet ./internal/api/middleware/... + ./internal/service/...:
    exit 0.
  go test -short ./internal/api/middleware/...: ok 0.244s.
  go test -short ./internal/service/...: ok 4.965s
    (every existing scep_probe test still green — production +
    httptest paths both work).

References:
  https://github.com/certctl-io/certctl/security/code-scanning/17
  https://github.com/certctl-io/certctl/security/code-scanning/23
Closes CodeQL #17. Re-closes CodeQL #23 with a fix CodeQL's taint
tracker can verify.

internal/api/middleware/middleware.go

@@ -44,6 +44,13 @@ func RequestID(next http.Handler) http.Handler {
 
 // Logging middleware logs request details including method, path, status, and duration.
 // Deprecated: Use NewLogging for structured logging with slog.
+//
+// CWE-117 log-injection defense: r.Method and r.URL.Path are
+// attacker-controllable (request-line bytes — Go's net/http leaves
+// percent-decoded path segments in r.URL.Path, which can include CR/LF
+// in the decoded form even though the raw HTTP request line cannot).
+// strings.ReplaceAll on CR/LF/NUL strips the forgery vector before the
+// log line is emitted. Closes CodeQL #17 (go/log-injection).
 func Logging(next http.Handler) http.Handler {
 	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		start := time.Now()
@@ -55,10 +62,31 @@ func Logging(next http.Handler) http.Handler {
 
 		duration := time.Since(start)
 		requestID := getRequestID(r.Context())
-		log.Printf("[%s] %s %s %d %v", requestID, r.Method, r.URL.Path, wrapped.statusCode, duration)
+		log.Printf("[%s] %s %s %d %v",
+			requestID,
+			scrubLogValue(r.Method),
+			scrubLogValue(r.URL.Path),
+			wrapped.statusCode,
+			duration,
+		)
 	})
 }
 
+// scrubLogValue strips control characters that an attacker could use to
+// forge log entries (CWE-117). The replacement is structural, not
+// destructive: we collapse CR/LF/NUL to a single space rather than
+// dropping them, so an operator scanning the log can still see that the
+// field was present (just neutralized). Defense in depth: the
+// production middleware should prefer NewLogging (slog with structured
+// fields) where the logger handles escape on its own.
+func scrubLogValue(v string) string {
+	if !strings.ContainsAny(v, "\r\n\x00") {
+		return v
+	}
+	r := strings.NewReplacer("\r", " ", "\n", " ", "\x00", " ")
+	return r.Replace(v)
+}
+
 // NewLogging creates a structured logging middleware using slog.
 // Logs request_id, method, path, status, duration_ms, and remote_addr.
 func NewLogging(logger *slog.Logger) func(http.Handler) http.Handler {

internal/service/scep_probe.go

@@ -75,20 +75,21 @@ func (s *NetworkScanService) ProbeSCEP(ctx context.Context, rawURL string) (*dom
 	}
 
 	// Step 1: cheap up-front URL validation (SSRF early diagnostic).
-	// Defaults to validation.ValidateSafeURL; tests inject a permissive
-	// validator via service-level field so they can hit httptest
-	// loopback servers (which the production validator correctly
-	// rejects). Mirrors the webhook notifier's `newForTest` pattern.
-	validateURL := s.scepValidateURL
-	if validateURL == nil {
-		validateURL = validation.ValidateSafeURL
-	}
-	if err := validateURL(rawURL); err != nil {
-		result.Reachable = false
-		result.Error = "url validation: " + err.Error()
-		result.ProbeDurationMs = time.Since(started).Milliseconds()
-		s.persistProbeResult(ctx, result)
-		return result, fmt.Errorf("scep probe: validate url: %w", err)
+	// Direct literal call to validation.ValidateSafeURL so CodeQL
+	// go/request-forgery sees the sanitizer in-scope of every
+	// downstream HTTP call. Tests that need to hit httptest loopback
+	// servers grant an exemption via s.scepValidateURL (mirrors the
+	// webhook notifier's `newForTest` pattern). Production callers
+	// leave scepValidateURL nil so any production-validator
+	// rejection wins.
+	if err := validation.ValidateSafeURL(rawURL); err != nil {
+		if s.scepValidateURL == nil || s.scepValidateURL(rawURL) != nil {
+			result.Reachable = false
+			result.Error = "url validation: " + err.Error()
+			result.ProbeDurationMs = time.Since(started).Milliseconds()
+			s.persistProbeResult(ctx, result)
+			return result, fmt.Errorf("scep probe: validate url: %w", err)
+		}
 	}
 
 	// Normalize the base URL — strip any trailing query string so we
@@ -255,12 +256,22 @@ func (s *NetworkScanService) scepGetCACert(ctx context.Context, client *http.Cli
 //     callers leave scepValidateURL nil so validation.ValidateSafeURL
 //     is the active gate.
 func (s *NetworkScanService) scepHTTPGet(ctx context.Context, client *http.Client, rawURL string) ([]byte, error) {
-	validateURL := s.scepValidateURL
-	if validateURL == nil {
-		validateURL = validation.ValidateSafeURL
-	}
-	if err := validateURL(rawURL); err != nil {
-		return nil, fmt.Errorf("validate url: %w", err)
+	// Production-grade SSRF validator — direct literal call so CodeQL
+	// go/request-forgery recognizes it as a sanitizer in-scope of the
+	// client.Do sink below. Tests that need to hit httptest loopback
+	// servers grant an exemption via s.scepValidateURL (returning nil
+	// for the test URL); when no exemption applies, the production
+	// validator's rejection wins. Production callers leave
+	// scepValidateURL nil so the production validator is the only gate.
+	if err := validation.ValidateSafeURL(rawURL); err != nil {
+		// Test-only exemption hook. The override returns nil for URLs
+		// the test wants to allow despite the production validator's
+		// rejection (loopback / link-local in httptest scenarios).
+		// In production scepValidateURL is nil, so any production
+		// validator rejection bubbles up unconditionally.
+		if s.scepValidateURL == nil || s.scepValidateURL(rawURL) != nil {
+			return nil, fmt.Errorf("validate url: %w", err)
+		}
 	}
 
 	req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)