Repository navigation
[RecoveryServices] Preserve HTTPS certificate validation in Backup clients - #30262
Conversation
Stop replacing the global certificate validation callback during client initialization. Preserve platform and caller policy, add isolated regression coverage, and update release notes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The regression test lacks the CheckIn trait required for execution in CI.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Removes insecure global certificate-validation override behavior from Recovery Services Backup client initialization.
Changes:
- Preserves platform or caller-supplied HTTPS certificate validation.
- Adds regression tests and the adapter project reference.
- Updates module and repository release notes.
| File | Description |
|---|---|
ChangeLog.md |
Adds repository-level release note. |
src/RecoveryServices/RecoveryServices/ChangeLog.md |
Adds module release note. |
src/RecoveryServices/RecoveryServices.Backup.Test/RecoveryServices.Backup.Test.csproj |
References the tested adapter project. |
src/RecoveryServices/RecoveryServices.Backup.Test/UnitTests/ClientProxyBaseTests.cs |
Tests callback preservation. |
src/RecoveryServices/RecoveryServices.Backup.ServiceClientAdapter/ClientProxyBase.cs |
Removes permissive certificate callback assignment. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
|
This PR was labeled "needs-revision" because it has unresolved review comments or CI failures. |
Tag the regression theory as CheckIn so CI selects both cases. Remove the manual root changelog entry while retaining the module release note. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
/azp run |
|
Commenter does not have sufficient privileges for PR 30262 in repo Azure/azure-powershell |
|
NoriZC The review follow-up is pushed in 8c37a0a; both threads are addressed and resolved, and re-review has been requested. The rebuilt regression run with the CheckIn and platform filters selected exactly two cases, both passing. Azure Pipelines rejected my /azp run request because this account lacks the required PR pipeline privileges. Please authorize/re-run the pipelines for the updated head when reviewing. |
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
🤖 PR Validation — ️✔️ All clear
Description
Recovery Services Backup client initialization currently replaces
ServicePointManager.ServerCertificateValidationCallbackwith a callback that accepts every certificate. Remove that assignment, the obsolete test-environment comment and analysis suppression, and the unused import.Initialization now leaves the platform's default certificate validation, or any caller-supplied validation callback, unchanged. This does not introduce another global callback or reset an existing caller policy.
Add regression coverage to the existing Backup test project for both a null callback and a caller-supplied callback. The tests restore the original callback in
finally, and the existing test assembly disables parallel execution. Update the upcoming release notes. No cmdlet signatures or module versions are changed.Validation
Both cases fail against the original constructor and pass with this change (2 passed, 0 failed). The targeted run builds the affected adapter and its project dependencies. No Azure sign-in, live service calls, or traffic interception is required.
Mandatory Checklist
Please choose the target release of Azure PowerShell. (Target release is a different concept from API readiness. Please click below links for details.)
Check this box to confirm: I have read the Submitting Changes section of
CONTRIBUTING.mdand reviewed the following information:ChangeLog.mdfile(s) appropriatelysrc/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.## Upcoming Releaseheader in the past tense.ChangeLog.mdif no new release is required, such as fixing test case only.