feat(windows): add credential provider mirror config for network isolated cluster

[fseldow]

What this PR does / why we need it:

Which issue(s) this PR fixes:

Fixes #

[Copilot]

Pull request overview

This PR updates the Windows CSE credential provider configuration generation to support network-isolated clusters by adding a registry mirror mapping for MCR when BootstrapProfileContainerRegistryServer is set.

Changes:

• Add a new Config-CredentialProvider branch that appends mcr.microsoft.com (or MCRRepositoryBase) to matchImages.
• Add a --registry-mirror argument to the credential provider config when BootstrapProfileContainerRegistryServer is set.

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[fseldow]

we already has trim logic in aks-rp side, so prefer not to implement normalizing the variable every time.
https://msazure.visualstudio.com/CloudNativeCompute/_git/aks-rp?path=/resourceprovider/sharedlib/agentbaker/typeconversion.go&version=GBmaster&line=934&lineEnd=935&lineStartColumn=1&lineEndColumn=1&lineStyle=plain&_a=contents

do we think we should still put trim logic in cse script?

[timmy-wright]

[HIGH] Zoe — Customer-controlled registry host injected into config without validation

$global:BootstrapProfileContainerRegistryServer is expanded verbatim into a double-quoted YAML here-string and written as a privileged kubelet CredentialProviderConfig. A crafted value containing newlines or YAML metacharacters can inject additional list items or flags into args, changing how acr-credential-provider runs under SYSTEM during node bootstrap.

Suggestion: Validate $global:BootstrapProfileContainerRegistryServer against a strict hostname-only regex (e.g., ^[a-zA-Z0-9.\-]+(:[0-9]+)?$) before interpolation. Reject values with newlines, whitespace, or YAML metacharacters. Consider generating the config via structured YAML serialization rather than here-string concatenation.

[fseldow]

quote added

[timmy-wright]

Code Review — feat(windows): add credential provider mirror config for network isolated cluster

Reviewers: Wash (testing), Zoe (security), Simon (correctness), Kaylee (design), Book (resilience), Badger (auth/API contract)

---

Findings Summary

Severity	Count
Critical	0
High	1
Medium	3
Low	1

---

High

[Zoe] Customer-controlled registry host injected into config without validation — staging/cse/windows/configfunc.ps1 ~L424

$global:BootstrapProfileContainerRegistryServer is expanded verbatim into a double-quoted YAML here-string written as kubelet CredentialProviderConfig. A crafted value containing newlines or YAML metacharacters can inject additional list items or flags into args, changing how acr-credential-provider runs under SYSTEM during node bootstrap. While the value is control-plane-supplied, defence in depth is warranted here given the privilege level.

Fix: Validate against ^[a-zA-Z0-9.\-]+(:[0-9]+)?$ before interpolation; reject anything with whitespace or newlines. Or generate the config via structured YAML serialization.

(Inline comment posted)

---

Medium

[Wash] TrimEnd("/") normalization not tested — staging/cse/windows/configfunc.tests.ps1

The custom-MCR test uses custom.mcr.contoso.com (no trailing slash). If normalization were broken, the test would still pass. Add a test variant with a trailing-slash value (e.g., custom.mcr.contoso.com/) and verify the fixture matches the trimmed form.

---

[Kaylee] Function depends on hidden global inputs — staging/cse/windows/configfunc.ps1

Config-CredentialProvider now silently changes behaviour based on $global:BootstrapProfileContainerRegistryServer and $global:MCRRepositoryBase — neither is in its signature. Makes call-site intent invisible and test isolation brittle. Consider passing these as explicit parameters or resolving them at a higher call layer.

---

[Kaylee] YAML config duplicated across branches (DRY violation) — staging/cse/windows/configfunc.ps1

Three near-identical YAML here-strings now exist, varying only in one extra matchImages entry and one extra arg. Any future schema change must be applied in all three. Build shared matchImages/args lists, apply conditional additions, render once.

---

Low

[Wash] Typo in variable name — staging/cse/windows/configfunc.tests.ps1 (3 occurrences)

$acutalCredentialProviderConfig → $actualCredentialProviderConfig

---

No Issues Found

• Simon (correctness/logic): No findings
• Book (resilience/failure modes): No findings
• Badger (auth/API contract): No findings

---

Process Note

PR description is unfilled — the template fields ("What this PR does", issue link) are blank. Please fill them in before merge.

---

Overall Recommendation

✅ Approve with suggestions

The core logic is correct and tests cover the main happy paths. The High finding (input validation) should be addressed before merge given the privileged execution context. The Medium findings are reasonable follow-ups but don't block the feature.

[r2k1]

🟢 Low / Test Coverage — This value has no trailing slash, so the .TrimEnd("/") you added in configfunc.ps1 never runs in the tests. Set it to custom.mcr.contoso.com/ and keep the fixture without the slash, so the test proves the trim works.