Validate IDs before they are passed to az commands

.github/workflows/ci.yml

@@ -77,3 +77,29 @@ jobs:
         with:
           name: dotnet-packages-${{ github.run_number }}
           path: ~/drop/dotnet/NuGetPackages/*
+
+  # The CLI wraps az/npm/func in cmd.exe on Windows; run those tests against a real cmd.exe.
+  windows-cmd-tests:
+    name: Windows cmd.exe tests
+    runs-on: windows-latest
+    timeout-minutes: 20
+    defaults:
+      run:
+        working-directory: ./src
+
+    steps:
+      - name: Checkout repository
+        uses: actions/checkout@11d5960a326750d5838078e36cf38b85af677262 # v4.4.0
+        with:
+          fetch-depth: 0  # Fetch all history for nbgv to calculate version
+
+      - name: Setup .NET 8.0.x
+        uses: actions/setup-dotnet@67a3573c9a986a3f9c594539f4ab511d57bb3ce9 # v4.3.1
+        with:
+          dotnet-version: "8.0.x"
+
+      - name: Restore test dependencies
+        run: dotnet restore tests.proj
+
+      - name: Run cmd.exe wrapper tests
+        run: dotnet test tests.proj --no-restore --configuration Release --filter "FullyQualifiedName~CommandExecutorTests|FullyQualifiedName~CommandArgumentGuardTests"

CHANGELOG.md

@@ -6,6 +6,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Fixed
+
+- Agent and service principal IDs read from configuration or Microsoft Graph are now validated before they are passed to Azure CLI commands (#524).
+
 ## [2.0.0] - 2026-10-20
 
 ### Upgrade Notes

src/Microsoft.Agents.A365.DevTools.Cli/Commands/CleanupCommand.cs

@@ -800,15 +800,15 @@ private static Command CreateInstanceCleanupCommand(
                 if (!string.IsNullOrWhiteSpace(config.AgenticAppId))
                 {
                     logger.LogInformation("Deleting agent identity service principal...");
-                    await executor.ExecuteAsync("az", $"ad app delete --id {config.AgenticAppId}", null, true, false, ct);
+                    await executor.ExecuteAsync("az", $"ad app delete --id {CommandArgumentGuard.RequireGuid(config.AgenticAppId, nameof(config.AgenticAppId))}", null, true, false, ct);
                     logger.LogInformation("Agent identity service principal deleted");
                 }
 
                 // Delete agent user
                 if (!string.IsNullOrWhiteSpace(config.AgenticUserId))
                 {
                     logger.LogInformation("Deleting agent user...");
-                    await executor.ExecuteAsync("az", $"ad user delete --id {config.AgenticUserId}", null, true, false, ct);
+                    await executor.ExecuteAsync("az", $"ad user delete --id {CommandArgumentGuard.RequireGuid(config.AgenticUserId, nameof(config.AgenticUserId))}", null, true, false, ct);
                     logger.LogInformation("Agent user deleted");
                 }
 
@@ -1111,7 +1111,7 @@ private static async Task ExecuteAllCleanupAsync(
             if (!string.IsNullOrWhiteSpace(config.AgenticUserId))
             {
                 logger.LogInformation("Deleting agent user...");
-                await executor.ExecuteAsync("az", $"ad user delete --id {config.AgenticUserId}", null, true, false, ct);
+                await executor.ExecuteAsync("az", $"ad user delete --id {CommandArgumentGuard.RequireGuid(config.AgenticUserId, nameof(config.AgenticUserId))}", null, true, false, ct);
                 logger.LogInformation("Agent user deleted");
             }
 

src/Microsoft.Agents.A365.DevTools.Cli/Commands/QueryEntraCommand.cs

@@ -437,6 +437,14 @@ private static Command CreateInstanceScopesSubcommand(
                 logger.LogInformation("{IdentityType} ID: {IdentityId}", identityType, agenticAppId);
                 logger.LogInformation("");
 
+                // The ID is passed to an az command line, so it must be a GUID.
+                if (!Guid.TryParse(agenticAppId, out _))
+                {
+                    logger.LogError("{IdentityType} ID in configuration is not a valid GUID.", identityType);
+                    context.ExitCode = 1;
+                    return;
+                }
+
                 // Query Entra ID for the agent identity and OAuth2 grants.
                 logger.LogInformation("Querying Microsoft Entra ID for agent identity and OAuth2 grants...");
 
@@ -483,6 +491,12 @@ private static Command CreateInstanceScopesSubcommand(
                     }
 
                     agentServicePrincipalObjectId = objectIdElement.GetString();
+                    if (!Guid.TryParse(agentServicePrincipalObjectId, out _))
+                    {
+                        logger.LogError("Service principal lookup returned an objectId that is not a valid GUID.");
+                        context.ExitCode = 1;
+                        return;
+                    }
                     if (spElement.TryGetProperty("displayName", out var nameElement) &&
                         nameElement.ValueKind == JsonValueKind.String &&
                         !string.IsNullOrWhiteSpace(nameElement.GetString()))
@@ -550,14 +564,16 @@ private static Command CreateInstanceScopesSubcommand(
                                 var scope = grantElement.TryGetProperty("scope", out var scopeElement) ? scopeElement.GetString() : "Unknown";
                                 var resourceId = grantElement.TryGetProperty("resourceId", out var resourceIdElement) ? resourceIdElement.GetString() : "Unknown";
 
-                                // Get the resource display name using Graph API
-                                var resourceResult = await executor.ExecuteAsync("az", 
-                                    $"rest --method GET --url \"{graphApiService.GraphBaseUrl}/v1.0/servicePrincipals/{resourceId}?$select=displayName,appId\" --output json");
+                                // Get the resource display name using Graph API; resourceId comes from Graph and must be a GUID before it reaches the az command line.
+                                var resourceResult = Guid.TryParse(resourceId, out _)
+                                    ? await executor.ExecuteAsync("az",
+                                        $"rest --method GET --url \"{graphApiService.GraphBaseUrl}/v1.0/servicePrincipals/{resourceId}?$select=displayName,appId\" --output json")
+                                    : null;
 
                                 string resourceName = "Unknown Resource";
                                 string resourceAppId = "Unknown";
 
-                                if (resourceResult.Success)
+                                if (resourceResult?.Success == true)
                                 {
                                     try
                                     {

src/Microsoft.Agents.A365.DevTools.Cli/Commands/SetupSubcommands/AzRestConsentRunner.cs

@@ -159,7 +159,7 @@ private static async Task<bool> GrantOneAsync(
         }
 
         var resourceSpId = TryExtractFirstId(resourceSpResult.StandardOutput);
-        if (string.IsNullOrWhiteSpace(resourceSpId))
+        if (string.IsNullOrWhiteSpace(resourceSpId) || !GuidPattern().IsMatch(resourceSpId))
         {
             logger.LogWarning(
                 "  '{Name}': resource service principal not found in tenant — cannot grant consent.",
@@ -203,6 +203,13 @@ private static async Task<bool> GrantOneAsync(
         var scopeValue = string.Join(' ', mergedScopes.OrderBy(s => s, StringComparer.OrdinalIgnoreCase));
         if (existingGrantId is not null)
         {
+            // Grant IDs are base64url; anything else must not reach the az command line.
+            if (!SafeScopePattern().IsMatch(existingGrantId))
+            {
+                logger.LogWarning("  '{Name}': existing oauth2PermissionGrant has an unexpected id format - skipping.", spec.ResourceName);
+                return false;
+            }
+
             var patchBody = JsonSerializer.Serialize(new { scope = scopeValue });
             var patched = await ExecuteAzRestWithBodyAsync(
                 executor,

src/Microsoft.Agents.A365.DevTools.Cli/Helpers/CommandArgumentGuard.cs

@@ -0,0 +1,61 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+namespace Microsoft.Agents.A365.DevTools.Cli.Helpers;
+
+/// <summary>
+/// Validates values that are passed to external command lines.
+/// </summary>
+public static class CommandArgumentGuard
+{
+    /// <summary>
+    /// Returns the value as a canonical GUID string, or throws if it is not a GUID.
+    /// </summary>
+    public static string RequireGuid(string? value, string fieldName)
+    {
+        if (!Guid.TryParse(value, out var guid))
+        {
+            throw new ArgumentException($"{fieldName} must be a valid GUID.", fieldName);
+        }
+
+        return guid.ToString("D");
+    }
+
+    /// <summary>
+    /// Throws if a command line passed to 'cmd.exe /c' could chain or redirect commands.
+    /// Metacharacters are allowed only inside balanced double quotes; '%' is rejected everywhere
+    /// because cmd.exe expands %VAR% (which can add or remove quotes) before it parses quotes.
+    /// </summary>
+    public static void EnsureSafeForCmd(string commandLine)
+    {
+        var inQuotes = false;
+        foreach (var c in commandLine)
+        {
+            if (c is '\r' or '\n' or '\0')
+            {
+                throw new ArgumentException("Command line contains a line break or null character.", nameof(commandLine));
+            }
+
+            if (c == '%')
+            {
+                throw new ArgumentException("Command line contains '%', which cmd.exe expands even inside quotes.", nameof(commandLine));
+            }
+
+            if (c == '"')
+            {
+                inQuotes = !inQuotes;
+                continue;
+            }
+
+            if (!inQuotes && (c is '&' or '|' or '<' or '>' or '^'))
+            {
+                throw new ArgumentException($"Command line contains the shell metacharacter '{c}' outside quotes.", nameof(commandLine));
+            }
+        }
+
+        if (inQuotes)
+        {
+            throw new ArgumentException("Command line contains unbalanced double quotes.", nameof(commandLine));
+        }
+    }
+}

src/Microsoft.Agents.A365.DevTools.Cli/Services/CommandExecutor.cs

@@ -4,6 +4,7 @@
 using System.Diagnostics;
 using System.Runtime.InteropServices;
 using System.Text;
+using Microsoft.Agents.A365.DevTools.Cli.Helpers;
 using Microsoft.Extensions.Logging;
 
 namespace Microsoft.Agents.A365.DevTools.Cli.Services;
@@ -38,7 +39,7 @@ public virtual async Task<CommandResult> ExecuteAsync(
         {
             _logger.LogTrace("Wrapping command with cmd.exe for Windows batch file");
             fileName = "cmd.exe";
-            fileArguments = $"/c {command} {arguments}";
+            fileArguments = BuildCmdWrapperArguments(command, arguments);
         }
 
         var startInfo = new ProcessStartInfo
@@ -145,7 +146,7 @@ public virtual async Task<CommandResult> ExecuteWithStreamingAsync(
         {
             _logger.LogTrace("Wrapping command with cmd.exe for Windows batch file");
             fileName = "cmd.exe";
-            fileArguments = $"/c {command} {arguments}";
+            fileArguments = BuildCmdWrapperArguments(command, arguments);
         }
 
         // In interactive mode we keep stdout/err redirected (so we can still display/prefix),
@@ -289,6 +290,17 @@ public virtual async Task<CommandResult> ExecuteWithStreamingAsync(
         return result;
     }
 
+    /// <summary>
+    /// Builds the 'cmd.exe' argument string, refusing command lines that cmd.exe would split into extra commands.
+    /// Delayed expansion is forced off so '!' stays literal regardless of the user's registry settings.
+    /// </summary>
+    internal static string BuildCmdWrapperArguments(string command, string arguments)
+    {
+        var commandLine = $"{command} {arguments}";
+        CommandArgumentGuard.EnsureSafeForCmd(commandLine);
+        return $"/v:off /c {commandLine}";
+    }
+
     private bool NeedsCmdWrapper(string command)
     {
         var extension = Path.GetExtension(command).ToLowerInvariant();

src/Microsoft.Agents.A365.DevTools.Cli/Services/ConfigService.cs

@@ -406,6 +406,17 @@ public async Task<ValidationResult> ValidateAsync(Agent365Config config)
             ValidateGuid(config.AgenticAppId, nameof(config.AgenticAppId), errors);
         }
 
+        // These IDs are passed to az command lines, so they must be GUIDs.
+        if (!string.IsNullOrWhiteSpace(config.AgenticUserId))
+        {
+            ValidateGuid(config.AgenticUserId, nameof(config.AgenticUserId), errors);
+        }
+
+        if (!string.IsNullOrWhiteSpace(config.AgentBlueprintId))
+        {
+            ValidateGuid(config.AgentBlueprintId, nameof(config.AgentBlueprintId), errors);
+        }
+
         if (config.BotId != null)
         {
             ValidateGuid(config.BotId, nameof(config.BotId), errors);

src/Microsoft.Agents.A365.DevTools.Cli/Services/Helpers/AdminConsentHelper.cs

@@ -84,6 +84,13 @@ public static async Task<bool> PollAdminConsentAsync(
         if (BypassConsentChecksForTests)
             return true;
 
+        // appId and the returned SP id are passed to az command lines, so they must be GUIDs.
+        if (!Guid.TryParse(appId, out _))
+        {
+            logger.LogWarning("Cannot poll for admin consent: app id is not a valid GUID.");
+            return false;
+        }
+
         var start = DateTime.UtcNow;
         var baseUrl = ConfigConstants.NormalizeGraphBaseUrl(graphBaseUrl);
         string? spId = null;
@@ -120,7 +127,8 @@ public static async Task<bool> PollAdminConsentAsync(
                             var value = doc.RootElement.GetProperty("value");
                             if (value.GetArrayLength() > 0)
                             {
-                                spId = value[0].GetProperty("id").GetString();
+                                var candidateSpId = value[0].GetProperty("id").GetString();
+                                spId = Guid.TryParse(candidateSpId, out _) ? candidateSpId : null;
                             }
                         }
                         catch { }
@@ -429,12 +437,20 @@ public static async Task<bool> CheckConsentExistsAsync(
                 return false;
             }
 
+            if (!Guid.TryParse(blueprintSpId, out _) || !Guid.TryParse(resourceSpId, out _) ||
+                (!string.IsNullOrWhiteSpace(consentType) && !consentType.All(char.IsAsciiLetter)))
+            {
+                logger.LogDebug("Cannot check consent: service principal IDs must be GUIDs and consentType must be letters only");
+                return false;
+            }
+
+            // Not URL-encoded: the value is passed through cmd.exe, which expands '%' even inside quotes.
             var filter = $"clientId eq '{blueprintSpId}' and resourceId eq '{resourceSpId}'";
             if (!string.IsNullOrWhiteSpace(consentType))
                 filter += $" and consentType eq '{consentType}'";
 
             var grantsResult = await executor.ExecuteAsync("az",
-                $"rest --method GET --url \"{baseUrl}/v1.0/oauth2PermissionGrants?$filter={Uri.EscapeDataString(filter)}\"",
+                $"rest --method GET --url \"{baseUrl}/v1.0/oauth2PermissionGrants?$filter={filter}\"",
                 captureOutput: true, suppressErrorLogging: true, cancellationToken: ct);
 
             if (!grantsResult.Success)
@@ -486,6 +502,9 @@ public static async Task<bool> CheckConsentExistsAsync(
     private static async Task<string?> LookupSpObjectIdByAppIdAsync(
         CommandExecutor executor, string appId, string graphBaseUrl, CancellationToken ct)
     {
+        if (!Guid.TryParse(appId, out _))
+            return null;
+
         var spResult = await executor.ExecuteAsync("az",
             $"rest --method GET --url \"{graphBaseUrl}/v1.0/servicePrincipals?$filter=appId eq '{appId}'&$select=id\"",
             captureOutput: true, suppressErrorLogging: true, cancellationToken: ct);
@@ -497,7 +516,8 @@ public static async Task<bool> CheckConsentExistsAsync(
         {
             using var doc = JsonDocument.Parse(spResult.StandardOutput);
             var value = doc.RootElement.GetProperty("value");
-            return value.GetArrayLength() > 0 ? value[0].GetProperty("id").GetString() : null;
+            var spId = value.GetArrayLength() > 0 ? value[0].GetProperty("id").GetString() : null;
+            return Guid.TryParse(spId, out _) ? spId : null;
         }
         catch
         {