Repository navigation
fix(secrets): use shared metadata guard for Key Vault - #482
Conversation
Replace the literal-host guard with the existing resolved-address transport so mapped metadata addresses cannot bypass protection. Exercise constructor wiring and retain the guard in local TLS tests. Refs LeanerCloud/cloud-commitments-go#25
|
Warning Review limit reached
This review includes 2 billable files and costs up to $0.50.
Reviews can continue after your included limit without a manual trigger. An admin must approve usage-based billing. Or wait 13 minutes for your next included review. View limit detailsLimit details: You’ve used the included review currently available. Your 81 included PR review attempts over the past 7 days set your current allowance at 1 review per hour. Review configuration: ⚙️ Run configuration
📒 Files selected for processing (2)
Comment |
|
Exact-head CI completed successfully at b97a8d0: Build & Test run 37240109834 and pre-commit run 37240109843. The independent Astra verdict and native evidence in the PR body cover this same head. CodeRabbit supplied only a review-limit notice, not a substantive review; no billing or review bypass was used. This PR remains open under the recorded real-scenario acceptance hold; green CI is not live Azure/Private Link proof. |
|
Fresh verification covers unchanged head
Evidence hashes:
Logs are retained under Fixture evidence does not establish live Azure credential acquisition, production DNS, or deployed Private Link connectivity. Existing cloud-URL error tests can receive sandbox denial; four GCP skips are not cloud evidence. The local installed linter differs from CI's pinned version and was not used to claim a new lint pass. CI lint/security/normal-hook results were freshly re-fetched as successful on this exact head. The owner's explicit override removes the earlier exact-model and mandatory real-account merge prerequisites. Those former holds are superseded. No substantive CodeRabbit review or actionable finding exists on this head; its recorded response is a quota notice, and its green status is not a review. No waiver merge or retrospective ledger entry is claimed before a merge actually occurs. PR remains open pending coordinator merge/main sequencing and the separate no-deployment coupling hold: active push-to-main deployment workflows must not be triggered under current owner scope. No workflow changes, branch deletion, or protection bypass were performed. Before any later normal merge, recheck the unchanged reviewed head, mergeability, CI and available findings; reconcile the integration baseline after any main advance. |
|
The owner has explicitly authorized normal existing merge-triggered CI/development deployments. The prior blanket main/deployment hold in my earlier comment is superseded; workflows and protections remain unchanged. Head remains Prior counts include nested events: 245 PASS / 4 SKIP total, 167 PASS / 1 SKIP top-level. Package duration is 54.331s; verification runner wall duration is 57.012s. These are distinct measures from the same successful log. Four credential-dependent GCP skips are not live-cloud evidence. |
|
Head reviewed: Independent Codex reviewer completed two clean adversarial source passes and personally reproduced the regression and integrated SDK fixture suite. No actionable findings remain.
Commands (offline Go 1.26.6,
Logs are preserved in CR waived: quota, adversarial review + local verification + green CI The exact-head CodeRabbit status explicitly says Upstream #25 remains open for its other scope, with broader policy tracked by #20 and #152. No new follow-up issue was needed. |
|
PR #482 merged normally as Independent parent FAIL/fixed PASS, actual SDK TLS success/failure/edge fixtures and integrated full secrets race suite passed; fresh repository build passed. Commands, outputs and hashes. No actionable findings. Live Azure authentication, production DNS and deployed Private Link were not exercised. Broader policy work in go #20/#152 and the remaining scope of go #25 remain open; no duplicate follow-up issue needed. CR waived: quota, adversarial review + local verification + green CI Actual merged-commit CI Build/Test Coordinator: add #482 to the existing retrospective CR set. Local ledger |
Summary
Use the existing shared resolved-address metadata guard for Azure Key Vault instead of the resolver's literal-host map. This closes the IPv4-mapped metadata-address bypass without changing dependencies or banning private endpoints.
Keep the production transport in the local TLS fixture, and cover constructor-to-GetSecret rejection plus the real SDK request path, authorization header, and returned secret.
Refs LeanerCloud/cloud-commitments-go#25
Exact-head verification
Head:
b97a8d0068878c4ab581d6125e1326588a1c095e.Limits and merge hold
Positive SDK evidence uses synthetic credentials and a local TLS endpoint, not live Azure authentication or a deployed Private Link endpoint. Existing cloud-URL error-path tests run under kernel network denial. The local git-secrets configuration has no patterns, so its Passed status does not establish AWS credential-pattern coverage.
Exact-head CI and project-required real-scenario acceptance remain merge gates. No merge approval is claimed. This does not resolve the broader public-only policy work in LeanerCloud/cloud-commitments-go#20 or #152; #25 remains open for its remaining scope. Labels mirror #25's existing historical classification.