[cristim]

Closes #17

What

Eight Azure service clients expose NewClientWithHTTP. Three of them (savingsplans:91, synapse:84, managedredis:103) already fell back to the IMDS-blocking providers/azure/internal/httpclient.New() when given a nil httpClient. The other five stored the nil interface, so the first .Do call would panic:

• database:130
• cache:106
• search:81
• cosmosdb:104
• compute:153

Each of the five now has the same 3-line guard, if httpClient == nil { httpClient = httpclient.New() }. Their doc comments now match the guarded three: a nil client falls back to the hardened default, and a non-nil client is used as given and bypasses the guard. That also brings the docs in line with what #473 documented.

No shared helper was extracted: the guard is a single if, and no existing helper fits.

Commit (<= 400 additions+deletions)

1. fix(azure): fall back to hardened httpclient on nil httpClient in 5 constructors (130)

Local verification (macOS, providers/azure)

• go build, go vet and go test ./... pass with and without GOWORK=off, and golangci-lint reports 0 issues.
• TestNewClientWithHTTP_NilFallbackIsHardened, added to each of the five packages to mirror the synapse test, asserts that the client is non-nil, is not http.DefaultClient, and rejects an IMDS request with "blocked".
• Fail-before: with main's client.go, all five packages fail. Pass-after: all five pass.
• Mutation probe: removing the search guard fails its test.

Independent reviews follow as PR comments.

🤖 Generated with Claude Code

[coderabbitai]

Warning

Review limit reached

• Run on-demand review

_{This review includes 10 billable files and costs up to $2.50.}

• Ask an admin to make reviews automatic

Open in CodeRabbit

Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing.

Or wait 26 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available. Your 88 included PR review attempts over the past 7 days set your current allowance at 1 review per hour.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

• Configuration used: Repository: LeanerCloud/cloud-commitments-go/.coderabbit.yaml
• Review profile: CHILL
• Plan: Essentials
• Run ID: a1bf7cc5-f886-42ee-9fb4-b3513acf38b1

📥 Commits

Reviewing files that changed from the base of the PR and between f88f84c and 9296c9e.

📒 Files selected for processing: 10

• providers/azure/services/cache/client.go
• providers/azure/services/cache/client_test.go
• providers/azure/services/compute/client.go
• providers/azure/services/compute/client_test.go
• providers/azure/services/cosmosdb/client.go
• providers/azure/services/cosmosdb/client_test.go
• providers/azure/services/database/client.go
• providers/azure/services/database/client_test.go
• providers/azure/services/search/client.go
• providers/azure/services/search/client_test.go

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[cristim]

Independent review B (claude-cc-libs-reviewB-521, Sonnet, cold brief, not the author)

Reviewed SHA: c5e5e66
Verdict: APPROVE. No actionable findings.

#17 criteria:

Criterion	Status
nil guard in database / cache / search / cosmosdb / compute	met (database/client.go:132, cache:108, search:83, cosmosdb:106, compute:155)
regression test per package, mirroring synapse	met: TestNewClientWithHTTP_NilFallbackIsHardened in each of the five

"Closes #17" is justified.

Local verification: go test -count=1 ./... in providers/azure passes with and without GOWORK=off, and golangci-lint reports 0 issues.

Duplication:

• Keeping the guard inline is justified. A shared OrDefault(c) helper would have to return one of eight per-package HTTPClient interfaces, adding public surface to an internal package for a 3-line if. Each copy is locked by its own test.
• The five copies of the test are acceptable. Each targets its own unexported field and client type, and the same test already exists in the three older packages.

Doc comments: the five rewritten comments are word for word the same as the existing three (synapse:80-83, savingsplans:87-90, managedredis:99-102).

Note: the PR body cites the constructor func lines. The guards sit two lines lower.

[cristim]

Independent review A (claude-cc-libs-reviewA-521, Sonnet, cold brief, not the author): security path

Reviewed SHA: c5e5e66
Verdict: APPROVE. No actionable findings.

1. No ninth constructor: across all modules there are exactly eight NewClientWithHTTP, all in providers/azure/services/*/client.go, and all are now guarded. The other *http.Client constructors (pkg/insurance/client.go:79,83, pkg/httpclient) are not Azure. No non-test caller passes nil.
2. Same hardened client: all eight import providers/azure/internal/httpclient. Its New() (httpclient.go:17) delegates to pkg/httpclient/httpclient.go:112-130, which sets Dialer.Control: blockMetadata and a request timeout. blockMetadata rejects metadata, unspecified and embedded-v4 addresses (~:100-109).
3. Typed nil (informational): the parameter is the local HTTPClient interface, so a typed-nil *http.Client would bypass == nil. No caller does this, the three existing guards behave the same, and a nil *http.Client does not panic in Do.
4. Tests: the test requests the literal 169.254.169.254, with no DNS lookup. The dialer's Control hook rejects it before connect(), so no packet is sent and nothing can hang. The five tests are byte for byte the synapse test (synapse/client_test.go:1275-1288).

Local verification at this SHA: build and vet are clean, go test -count=1 ./... passes with and without GOWORK=off, and golangci-lint reports 0 issues.

Mutation probes:

• Removing the database guard fails client_test.go:2127.
• Replacing httpclient.New() with http.DefaultClient in cache fails client_test.go:1498.

[cristim]

Review B re-review (claude-cc-libs-reviewB-521): merge of main at 9296c9e

Reviewed SHA: 9296c9e
Verdict: APPROVE

• Conflict resolution: there are no conflict markers, gofmt is clean on providers/azure, and compute/client_test.go keeps both appended test sets: TestNewClientWithHTTP_NilFallbackIsHardened from this PR and the SKU capability tests from fix(azure/compute): fail loud on unparseable VM SKU capabilities #519.
• Local verification: go test -count=1 ./services/compute/... passes.

[cristim]

Review A re-review (claude-cc-libs-reviewA-521): merge of main at 9296c9e

Reviewed SHA: 9296c9e
Verdict: APPROVE

• Conflict markers: none (the only matches are decorative lines in NOTICE).
• Diff vs main: unchanged in substance. It touches the same 10 files with +120/-10, as at c5e5e66.
• compute/client.go: vs main, only the nil guard and its comment. fix(azure/compute): fail loud on unparseable VM SKU capabilities #519's SKU changes are already in main and untouched.
• compute/client_test.go: +18 lines, no deletions. fix(azure/compute): fail loud on unparseable VM SKU capabilities #519's tests are intact at lines 1873 and 1932.
• Local verification: go test -count=1 ./... passes with and without GOWORK=off, and golangci-lint reports 0 issues.

[cristim]

Merge gate verdict (claude-cc-libs)

Every gate holds at head 9296c9e:

• Two independent reviews approve the reviewed head 9296c9e. The branch was then updated from main, and the PR diff is byte-identical to the reviewed diff (compared mechanically).
• CI is green on 9296c9e, which is the reviewed change on top of current main.
• 0 unresolved threads; MERGEABLE/CLEAN.

CodeRabbit is waived as a merge gate under the 2026-10-19 decision. Squash-merging.