chore(secrets): extract IMDS-blocking transport into shared pkg/httpclient

[cristim]

Severity / Confidence

P3, high confidence. Chore. Duplicated SSRF-defense helper across two modules; no behavior change pending refactor.

Affected files

• internal/secrets/azure_resolver.go:17-60 (duplicate imdsAddresses, blockIMDSDialer, imdsBlockingTransport)
• providers/azure/internal/httpclient/httpclient.go:14-53 (original imdsAddresses, blockIMDSDialer, New)

Evidence

PR LeanerCloud/cloud-commitments-cli#1224 added an IMDS-blocking transport to NewAzureResolver (imdsBlockingTransport() at internal/secrets/azure_resolver.go:46). The implementation is bit-for-bit equivalent to httpclient.New() at providers/azure/internal/httpclient/httpclient.go:38: same imdsAddresses map (169.254.169.254, fd00:ec2::254), same blockIMDSDialer struct + DialContext, same 10s/30s/30s timeouts, same http.Transport fields.

The duplication is currently forced by Go's internal/ import rule plus the module boundary (providers/azure/go.mod is a separate module from the root). The root-module internal/secrets package cannot import github.com/LeanerCloud/CUDly/providers/azure/internal/httpclient.

Memory feedback_azure_use_httpclient_new.md explicitly says: "Every Azure service client must use httpclient.New(), not &http.Client{Timeout: ...}. Any new Azure service client that omits this step loses the SSRF defense-in-depth without any compile-time signal." Two parallel implementations defeat the grep-based regression check the memory recommends (grep '&http.Client{' in providers/azure/).

Recommendation

Extract imdsAddresses, blockIMDSDialer, and the New() constructor into a shared, non-internal package importable by both modules:

• pkg/httpclient/httpclient.go (the pkg/ module already has its own go.mod, so it can be required by both root and providers/azure/).
• Replace the contents of providers/azure/internal/httpclient/httpclient.go and internal/secrets/azure_resolver.go IMDS helpers with a re-export / direct call into the new pkg/httpclient.New().
• Keep the regression test pattern: each consuming module retains a small test that calls pkg/httpclient.New() and asserts a request to http://169.254.169.254/... errors with "blocked".

This collapses the duplication and gives the grep guard a single canonical answer.

Triage labels

type/chore, priority/p3, severity/low, urgency/eventually, impact/internal, effort/s, triaged.

Source: PR LeanerCloud/cloud-commitments-cli#1224 adversarial review.

[cristim]

Reviewed commit: be11bdcb5. Note: origin/main moved to 3e9660d06 during the review; re-verify against current main before changing code, since a finding may have been fixed or moved.

The 2026-07-28 full-repo review re-derived this duplication and found the situation has changed since this issue was written: pkg/httpclient now exists and claims to be the single implementation, but internal/secrets never migrated onto it. That makes this issue's type/chore framing understate it, because the drift now silently strands a security fix.

Where

• internal/secrets/azure_resolver.go:17-60 versus pkg/httpclient/httpclient.go:20-68

What the review adds

imdsAddresses, blockIMDSDialer, DialContext and the client constructor are duplicated almost verbatim. The copy in internal/secrets inlines its timeouts as bare literals (10 * time.Second, 30 * time.Second at lines 49-57) where the shared package names them as constants (dialTimeout, keepAliveInterval, tlsHandshakeTimeout, requestTimeout).

pkg/httpclient's own package header now asserts the migration is complete:

// This is the single shared implementation; the Azure provider's
// internal httpclient package delegates here so every module uses the
// same hardening.

That is true of providers/azure/internal/httpclient/httpclient.go:18 and false of internal/secrets/azure_resolver.go. A reader auditing the SSRF posture from that comment concludes there is one implementation to fix.

Failure scenario

The IMDS block-list needs several fixes (see #20 and LeanerCloud/cloud-commitments-cli#1324; the guard matches the hostname rather than the resolved IP, so metadata.google.internal passes). Those fixes will land in pkg/httpclient and will silently not apply to Azure Key Vault secret retrieval, which is the path that fetches the credential encryption key itself (internal/credentials/cipher.go:106). The two copies are currently textually near-identical, so the drift will not be visible in review of the fix PR; the patch simply leaves the highest-value path unprotected while the header comment says otherwise.

Fix direction

Delete the copy and call httpclient.New(). Add a guard test asserting internal/secrets declares no dialer of its own, matching the existing azure_sdk_module_guard_test.go pattern already in that package.

Given the above, suggest sequencing this before #20 / LeanerCloud/cloud-commitments-cli#1324 so the hostname-resolution fix is written once, and re-checking whether type/chore / severity/low still fits now that the header comment actively misreports the state.

[cristim]

Platform prerequisite published as LeanerCloud/cloud-commitments-platform#482 at b97a8d0068878c4ab581d6125e1326588a1c095e. It replaces the Key Vault literal-host guard with the existing shared resolved-address guard. Independent Astra review and confined native macOS baseline/mutation/treatment, real SDK local-TLS, secrets race suite, build, lint and normal hooks passed. Positive evidence is synthetic, not live Azure/Private Link acceptance; the PR remains open pending acceptance and CI. This does not resolve broader public-only policy requirements in #20/#152 or close the remaining scope of this issue.