Validate ServerId against local machine in Set-AzStorageSyncServer by stefong99 · Pull Request #29448 · Azure/azure-powershell

stefong99 · 2026-04-24T22:41:02Z

When ServerId is explicitly provided, call GetSyncServerId() via IEcsManagement to retrieve the local machine's server ID and verify it matches. This prevents accidental cross-machine misuse with a clear error message.

Also fix MockEcsManagementInteropClient.GetSyncServerId to use HttpMockServer.GetVariable for deterministic behavior in test playback mode.

Description

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.)
- General release
- Public preview
- Private preview
- Engineering build
- No need for a release
Check this box to confirm: I have read the Submitting Changes section of CONTRIBUTING.md and reviewed the following information:

SHOULD update ChangeLog.md file(s) appropriately
- Update src/{{SERVICE}}/{{SERVICE}}/ChangeLog.md.
  - A snippet outlining the change(s) made in the PR should be written under the ## Upcoming Release header in the past tense.
- Should not change ChangeLog.md if no new release is required, such as fixing test case only.
SHOULD regenerate markdown help files if there is cmdlet API change. Instruction
SHOULD have proper test coverage for changes in pull request.
SHOULD NOT adjust version of module manually in pull request

When ServerId is explicitly provided, call GetSyncServerId() via IEcsManagement to retrieve the local machine's server ID and verify it matches. This prevents accidental cross-machine misuse with a clear error message. Also fix MockEcsManagementInteropClient.GetSyncServerId to use HttpMockServer.GetVariable for deterministic behavior in test playback mode. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

When ServerId is not explicitly passed to Set-AzStorageSyncServer, call GetSyncServerId() to resolve it from the local machine automatically. Remove the explicit validation block since the value is always sourced from the local machine in that path. Also fix RegisteredServers.Get and error messages to use the resolved resourceName instead of the raw ServerId parameter. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

When ServerId is explicitly passed, still validate it against the local machine's server ID via GetSyncServerId(). When it is not passed, auto- populate from the local machine (no validation needed). Refactored to call GetSyncServerId() once and share the result across both branches. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Refactor: call GetSyncServerId() once upfront before resolving either parameter set, then validate the resolved resourceName against the local machine's server ID in all cases (InputObject, explicit ServerId, and auto-populated ServerId). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

azure-client-tools-bot-prd · 2026-04-24T22:41:06Z

Thanks for your contribution! The pull request validation has started. Please revisit this comment for updated status.

Copilot

Pull request overview

This PR hardens Set-AzStorageSyncServer by validating that an explicitly targeted ServerId matches the local machine’s Azure File Sync agent server ID, preventing accidental cross-machine operations. It also updates the test mock for GetSyncServerId() to be deterministic in playback mode.

Changes:

Added local-machine ServerId retrieval via IEcsManagement.GetSyncServerId() and enforced match with the cmdlet target server ID.
Updated RegisteredServers.Get(...) and “not found” messaging to use the resolved/validated server ID.
Made MockEcsManagementInteropClient.GetSyncServerId() deterministic by using HttpMockServer.GetVariable(...) during playback/record.

Reviewed changes

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

File	Description
src/StorageSync/StorageSync/RegisteredServer/SetRegisteredServerCommand.cs	Retrieves local server ID via ECS interop and validates it matches the cmdlet’s target `ServerId` before proceeding.
src/StorageSync/StorageSync.Test/Common/MockEcsManagementInteropClient.cs	Ensures repeatable `GetSyncServerId()` values across test recording/playback.

Copilot · 2026-04-24T22:47:12Z

+                IEcsManagement ecsManagement = StorageSyncClientWrapper.StorageSyncResourceManager.CreateEcsManagement();
+                int hr = ecsManagement.GetSyncServerId(out string localServerId);
+                if (hr != 0 || !Guid.TryParse(localServerId, out Guid localServerGuid))
+                {
+                    throw new PSArgumentException("Unable to retrieve the local server ID. Ensure the Azure File Sync agent is installed and running.");


IEcsManagement implements IDisposable (and the real implementation wraps a COM object). Wrap the instance returned from CreateEcsManagement() in a using/try/finally to ensure COM handles are released promptly.

Suggested change

IEcsManagement ecsManagement = StorageSyncClientWrapper.StorageSyncResourceManager.CreateEcsManagement();

int hr = ecsManagement.GetSyncServerId(out string localServerId);

if (hr != 0 || !Guid.TryParse(localServerId, out Guid localServerGuid))

{

throw new PSArgumentException("Unable to retrieve the local server ID. Ensure the Azure File Sync agent is installed and running.");

string localServerId;

Guid localServerGuid;

using (IEcsManagement ecsManagement = StorageSyncClientWrapper.StorageSyncResourceManager.CreateEcsManagement())

{

int hr = ecsManagement.GetSyncServerId(out localServerId);

if (hr != 0 || !Guid.TryParse(localServerId, out localServerGuid))

{

throw new PSArgumentException("Unable to retrieve the local server ID. Ensure the Azure File Sync agent is installed and running.");

}

Copilot · 2026-04-24T22:47:12Z

+                IEcsManagement ecsManagement = StorageSyncClientWrapper.StorageSyncResourceManager.CreateEcsManagement();
+                int hr = ecsManagement.GetSyncServerId(out string localServerId);
+                if (hr != 0 || !Guid.TryParse(localServerId, out Guid localServerGuid))
+                {
+                    throw new PSArgumentException("Unable to retrieve the local server ID. Ensure the Azure File Sync agent is installed and running.");
+                }
+


The cmdlet calls into the local agent (GetSyncServerId) before verifying that -Identity was supplied. This can change the error users get when they omit -Identity (and can fail on machines without the agent even though the invocation is already invalid). Consider checking/enforcing -Identity first, then retrieving/validating the local server ID.

Copilot · 2026-04-24T22:47:12Z

+                if (!Guid.TryParse(resourceName, out Guid resourceServerGuid) || resourceServerGuid != localServerGuid)
+                {
+                    throw new PSArgumentException($"The provided ServerId '{resourceName}' does not match the local machine's server ID '{localServerGuid}'. Run this command on the correct server.");
                }



The error message conflates two different cases: (1) ServerId is not a valid GUID and (2) it is a valid GUID but doesn’t match the local machine. Consider splitting these conditions so an invalid GUID gets a clear format/validation error, and consider wording that also makes sense when InputObject is used (not just when -ServerId is explicitly provided).

Suggested change

if (!Guid.TryParse(resourceName, out Guid resourceServerGuid) || resourceServerGuid != localServerGuid)

{

throw new PSArgumentException($"The provided ServerId '{resourceName}' does not match the local machine's server ID '{localServerGuid}'. Run this command on the correct server.");

}

if (!Guid.TryParse(resourceName, out Guid resourceServerGuid))

{

throw new PSArgumentException($"The server ID '{resourceName}' is not a valid GUID.");

}

if (resourceServerGuid != localServerGuid)

{

throw new PSArgumentException($"The server ID '{resourceName}' does not match the local machine's server ID '{localServerGuid}'. Run this command on the correct server.");

}

Copilot · 2026-04-24T22:47:13Z

+                if (!Guid.TryParse(resourceName, out Guid resourceServerGuid) || resourceServerGuid != localServerGuid)
+                {
+                    throw new PSArgumentException($"The provided ServerId '{resourceName}' does not match the local machine's server ID '{localServerGuid}'. Run this command on the correct server.");
                }


New behavior (rejecting a non-local ServerId) doesn’t appear to have a negative test. Since StorageSync.Test/ScenarioTests/RegisteredServerTests.ps1 already covers Set-AzStorageSyncServer success paths, add a scenario test that passes a different GUID and asserts the cmdlet throws the expected PSArgumentException so playback/recording covers this validation.

VeryEarly · 2026-04-24T23:08:51Z

/azp run

azure-pipelines · 2026-04-24T23:09:05Z

Azure Pipelines successfully started running 3 pipeline(s).

stefong99 and others added 4 commits April 24, 2026 15:18

Copilot AI review requested due to automatic review settings April 24, 2026 22:41

Copilot started reviewing on behalf of stefong99 April 24, 2026 22:43 View session

Copilot AI reviewed Apr 24, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Validate ServerId against local machine in Set-AzStorageSyncServer#29448

Validate ServerId against local machine in Set-AzStorageSyncServer#29448
stefong99 wants to merge 4 commits intoAzure:mainfrom
stefong99:stefong/fix-patch-applicationid

stefong99 commented Apr 24, 2026

Uh oh!

azure-client-tools-bot-prd Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

Copilot AI Apr 24, 2026

Uh oh!

VeryEarly commented Apr 24, 2026

Uh oh!

azure-pipelines Bot commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-                if (!Guid.TryParse(resourceName, out Guid resourceServerGuid) || resourceServerGuid != localServerGuid)
-                {
-                    throw new PSArgumentException($"The provided ServerId '{resourceName}' does not match the local machine's server ID '{localServerGuid}'. Run this command on the correct server.");
-                }
+                if (!Guid.TryParse(resourceName, out Guid resourceServerGuid))
+                {
+                    throw new PSArgumentException($"The server ID '{resourceName}' is not a valid GUID.");
+                }
+                if (resourceServerGuid != localServerGuid)
+                {
+                    throw new PSArgumentException($"The server ID '{resourceName}' does not match the local machine's server ID '{localServerGuid}'. Run this command on the correct server.");
+                }

Conversation

stefong99 commented Apr 24, 2026

Description

Mandatory Checklist

Uh oh!

azure-client-tools-bot-prd Bot commented Apr 24, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 24, 2026

Choose a reason for hiding this comment

Uh oh!

VeryEarly commented Apr 24, 2026

Uh oh!

azure-pipelines Bot commented Apr 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants