security(scep_probe): re-validate URL inside scepHTTPGet to close CodeQL #23 (CWE-918)

CodeQL alert #23 (go/request-forgery, CWE-918 SSRF) flagged the
client.Do(req) sink at internal/service/scep_probe.go:232 because
the URL parameter to scepHTTPGet is taint-traced from the user-
supplied input to ProbeSCEP without the analyzer recognizing the
upstream sanitizer.

The defense-in-depth was already in place:
  1. validation.ValidateSafeURL at ProbeSCEP entry (line 75) —
     rejects obvious SSRF targets (loopback / link-local / cloud
     metadata literals) before any network call.
  2. validation.SafeHTTPDialContext on the http.Transport —
     re-resolves the host at dial time and rejects connections to
     reserved IP ranges. This is the authoritative SSRF + DNS-
     rebinding guard. Even if step 1 was bypassed, the dial would
     still fail.

But CodeQL's taint tracker doesn't follow the validator across
function boundaries, so the alert stays open even though the code
is safe. This commit re-runs validation.ValidateSafeURL inside
scepHTTPGet immediately before http.NewRequestWithContext —
sanitizer in the same function as the sink, which CodeQL
recognizes as a guard.

Bonus defense-in-depth: any future call site that wires a URL
into scepHTTPGet without going through ProbeSCEP (e.g. a new code
path that directly probes a discovered URL) inherits the same
SSRF guard automatically. Fail-closed by default.

The validator dispatch matches ProbeSCEP's pattern — tests
override via s.scepValidateURL to hit httptest loopback servers;
production callers use validation.ValidateSafeURL. The probe's
existing httptest-based tests continue to work unchanged.

Verified locally:
  gofmt: clean.
  go vet ./...: exit 0.
  go test -short ./internal/service/...: ok 4.029s
    (every existing scep_probe test still green — the new
    revalidation is a no-op for tests that go through ProbeSCEP
    because the same validator already passed once at entry).

Reference: https://github.com/certctl-io/certctl/security/code-scanning/23
Closes CodeQL alert #23 (go/request-forgery).

internal/service/scep_probe.go

@@ -223,8 +223,47 @@ func (s *NetworkScanService) scepGetCACert(ctx context.Context, client *http.Cli
 // scepHTTPGet issues a single GET with the probe's user agent + the
 // SSRF-defended HTTP client. Reads the body up to 1MB to defend against
 // a huge-response DoS from a misbehaving target.
-func (s *NetworkScanService) scepHTTPGet(ctx context.Context, client *http.Client, url string) ([]byte, error) {
+//
-	req, err := http.NewRequestWithContext(ctx, http.MethodGet, url, nil)
+// Defense in depth (CodeQL #23 / CWE-918 SSRF):
+//
+//   - The HTTP client's transport is built with validation.SafeHTTPDialContext
+//     (see scepProbeClient below). Every dial — including any dial along a
+//     redirect chain — re-resolves the host and rejects connections to
+//     reserved IP ranges (loopback, RFC 1918, link-local, multicast,
+//     CGNAT, IPv6 ULAs, etc.). This is the authoritative SSRF + DNS-
+//     rebinding guard; even if an attacker bypassed the upstream URL
+//     validator, the dial would still fail.
+//
+//   - In addition to the dial-time guard, this function re-runs
+//     validation.ValidateSafeURL on the URL right before the request
+//     is built. The validator is already invoked at ProbeSCEP entry,
+//     but re-running it here:
+//     (a) Closes CodeQL go/request-forgery — the analyzer's taint
+//     tracker now sees the sanitizer in the same function as the
+//     sink (client.Do).
+//     (b) Catches any future call site that wires a URL into
+//     scepHTTPGet without going through ProbeSCEP. If anyone adds
+//     such a path the validator catches the regression at the
+//     sink — fail-closed by default.
+//     (c) Is cheap (a single parse + reserved-IP lookup; the URL is
+//     already parsed once upstream so the OS DNS cache likely
+//     still has the answer).
+//
+//   - When the service is configured with a permissive validator
+//     (scepValidateURL — set by tests targeting httptest loopback
+//     servers), the same permissive validator applies here. Production
+//     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)
+	}
+
+	req, err := http.NewRequestWithContext(ctx, http.MethodGet, rawURL, nil)
 	if err != nil {
 		return nil, fmt.Errorf("build request: %w", err)
 	}