fix(security): validate enrollment summary branding image URL (https-only, no quotes); launcher drops unsafe URLs instead of interpolating them into the dialog command line

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/SummaryDialogLauncherTests.cs

@@ -50,5 +50,42 @@ namespace AutopilotMonitor.Agent.V2.Core.Tests.Termination
 
             Assert.Equal(a, b);
         }
+
+        // The branding URL is tenant config; the dialog parses arguments last-occurrence-wins, so
+        // an embedded quote must never terminate the argument and inject extra flags (CWE-88).
+
+        private const string BaseArgs = "--status-file \"C:\\x\\final-status.json\" --timeout 30 --cleanup";
+
+        [Fact]
+        public void BuildDialogArguments_omits_branding_when_empty()
+        {
+            var args = SummaryDialogLauncher.BuildDialogArguments(@"C:\x\final-status.json", 30, null, null);
+
+            Assert.Equal(BaseArgs, args);
+        }
+
+        [Fact]
+        public void BuildDialogArguments_appends_clean_https_url_as_single_quoted_argument()
+        {
+            var args = SummaryDialogLauncher.BuildDialogArguments(@"C:\x\final-status.json", 30, "https://cdn.example/logo.png?v=2", null);
+
+            Assert.Equal(BaseArgs + " --branding-url \"https://cdn.example/logo.png?v=2\"", args);
+        }
+
+        [Theory]
+        [InlineData("x\" --status-file \"C:\\some\\file\" --timeout \"0")]
+        [InlineData("https://cdn.example/logo.png\" --cleanup")]
+        [InlineData("https://cdn.example/logo.png --timeout 0")]
+        [InlineData("https://cdn.example/lo'go.png")]
+        [InlineData("https://cdn.example/logo.png\r\n")]
+        [InlineData("http://cdn.example/logo.png")]
+        [InlineData("file:///C:/Windows/x.png")]
+        [InlineData("cdn.example/logo.png")]
+        public void BuildDialogArguments_drops_unsafe_branding_url(string url)
+        {
+            var args = SummaryDialogLauncher.BuildDialogArguments(@"C:\x\final-status.json", 30, url, null);
+
+            Assert.Equal(BaseArgs, args);
+        }
     }
 }

src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/SummaryDialogLauncher.cs

@@ -132,10 +132,11 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination
             }
 
             // 4) Compose args. Legacy contract: --status-file <path> --timeout <s> --cleanup [--branding-url <url>].
-            var tempStatusArg = Path.Combine(tempDir, FinalStatusFileName);
-            var args = $"--status-file \"{tempStatusArg}\" --timeout {configuration.EnrollmentSummaryTimeoutSeconds} --cleanup";
-            if (!string.IsNullOrWhiteSpace(configuration.EnrollmentSummaryBrandingImageUrl))
-                args += $" --branding-url \"{configuration.EnrollmentSummaryBrandingImageUrl}\"";
+            var args = BuildDialogArguments(
+                Path.Combine(tempDir, FinalStatusFileName),
+                configuration.EnrollmentSummaryTimeoutSeconds,
+                configuration.EnrollmentSummaryBrandingImageUrl,
+                logger);
 
             // 5) Launch in user session with retry.
             var retrySeconds = configuration.EnrollmentSummaryLaunchRetrySeconds > 0
@@ -151,6 +152,44 @@ namespace AutopilotMonitor.Agent.V2.Core.Termination
             return launched;
         }
 
+        /// <summary>
+        /// Composes the dialog command line. The branding URL is tenant-supplied config and the
+        /// dialog parses arguments last-occurrence-wins, so a value carrying a double quote could
+        /// terminate its argument and override <c>--status-file</c>/<c>--timeout</c>/<c>--cleanup</c>
+        /// (CWE-88). The backend rejects such values, but the agent is the last line of defence:
+        /// only a plain absolute HTTPS URL without quotes, whitespace or control characters is
+        /// ever placed on the command line — anything else is dropped (the dialog then simply
+        /// shows no branding image).
+        /// </summary>
+        internal static string BuildDialogArguments(string statusFilePath, int timeoutSeconds, string brandingImageUrl, AgentLogger logger)
+        {
+            var args = $"--status-file \"{statusFilePath}\" --timeout {timeoutSeconds} --cleanup";
+            if (string.IsNullOrWhiteSpace(brandingImageUrl))
+                return args;
+
+            if (IsSafeBrandingImageUrl(brandingImageUrl))
+                return args + $" --branding-url \"{brandingImageUrl}\"";
+
+            logger?.Warning("SummaryDialogLauncher: EnrollmentSummaryBrandingImageUrl is not a plain absolute HTTPS URL — ignoring it.");
+            return args;
+        }
+
+        internal static bool IsSafeBrandingImageUrl(string url)
+        {
+            if (string.IsNullOrEmpty(url) || url.Length > 2048)
+                return false;
+
+            foreach (var ch in url)
+            {
+                if (ch == '"' || ch == '\'' || char.IsWhiteSpace(ch) || char.IsControl(ch))
+                    return false;
+            }
+
+            return Uri.TryCreate(url, UriKind.Absolute, out var uri)
+                && uri.Scheme == Uri.UriSchemeHttps
+                && !string.IsNullOrEmpty(uri.Host);
+        }
+
         private static void TryCopy(string source, string destination)
         {
             try { if (File.Exists(source)) File.Copy(source, destination, overwrite: true); }

src/Backend/AutopilotMonitor.Functions.Tests/BrandingImageUrlValidationTests.cs

@@ -0,0 +1,68 @@
+using AutopilotMonitor.Functions.Helpers;
+using AutopilotMonitor.Shared.Models;
+using Xunit;
+
+namespace AutopilotMonitor.Functions.Tests
+{
+    /// <summary>
+    /// The enrollment-summary branding image URL is delivered verbatim to every enrolling device,
+    /// fetched from the signed-in user's session and spliced into the summary-dialog command line.
+    /// The server must therefore only ever persist a plain absolute HTTPS URL — a value with an
+    /// embedded quote or a non-HTTPS scheme must fail the config PUT/PATCH.
+    /// </summary>
+    public class BrandingImageUrlValidationTests
+    {
+        [Theory]
+        [InlineData(null)]
+        [InlineData("")]
+        [InlineData("   ")]
+        [InlineData("https://cdn.example/logo.png")]
+        [InlineData("https://cdn.example:8443/assets/logo.png?v=2&x=y")]
+        [InlineData("https://cdn.example/logo%20one.png")]
+        public void ValidateBrandingImageUrl_WellFormed_ReturnsNull(string? url)
+        {
+            Assert.Null(TenantConfigValidation.ValidateBrandingImageUrl(url));
+        }
+
+        [Theory]
+        [InlineData("x\" --status-file \"C:\\some\\file\" --timeout \"0", "quotes")]
+        [InlineData("https://cdn.example/logo.png\"", "quotes")]
+        [InlineData("https://cdn.example/lo'go.png", "quotes")]
+        [InlineData("https://cdn.example/logo.png --cleanup", "whitespace")]
+        [InlineData("https://cdn.example/logo.png\r\n", "whitespace")]
+        [InlineData("http://cdn.example/logo.png", "HTTPS")]
+        [InlineData("file:///C:/Windows/logo.png", "HTTPS")]
+        [InlineData("ftp://cdn.example/logo.png", "HTTPS")]
+        [InlineData("cdn.example/logo.png", "absolute URL")]
+        [InlineData("https://localhost/logo.png", "localhost")]
+        [InlineData("https://10.0.0.1/logo.png", "IP address")]
+        public void ValidateBrandingImageUrl_Malformed_Rejected(string url, string expectedFragment)
+        {
+            var error = TenantConfigValidation.ValidateBrandingImageUrl(url);
+            Assert.NotNull(error);
+            Assert.Contains(expectedFragment, error);
+        }
+
+        [Fact]
+        public void ValidateBrandingImageUrl_Overlong_Rejected()
+        {
+            var url = "https://cdn.example/" + new string('a', TenantConfigValidation.MaxBrandingImageUrlLength);
+            Assert.Contains("characters", TenantConfigValidation.ValidateBrandingImageUrl(url));
+        }
+
+        [Fact]
+        public void ValidateModel_RejectsInjectedBrandingUrl()
+        {
+            var existing = new TenantConfiguration();
+            var candidate = new TenantConfiguration
+            {
+                EnrollmentSummaryBrandingImageUrl = "x\" --status-file \"C:\\some\\file\" --timeout \"0",
+            };
+
+            var error = TenantConfigValidation.ValidateModel(candidate, existing, isGlobalAdmin: true);
+
+            Assert.NotNull(error);
+            Assert.StartsWith("Invalid enrollment summary branding image URL", error);
+        }
+    }
+}

src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigValidation.cs

@@ -93,6 +93,49 @@ namespace AutopilotMonitor.Functions.Helpers
             return null;
         }
 
+        // Generous for a CDN asset URL; bounds the command line the agent composes from it.
+        internal const int MaxBrandingImageUrlLength = 2048;
+
+        /// <summary>
+        /// Validates <see cref="TenantConfiguration.EnrollmentSummaryBrandingImageUrl"/>. The value
+        /// is delivered verbatim to every enrolling device, fetched from the signed-in user's
+        /// session and spliced into the summary-dialog command line, so it is the one piece of
+        /// tenant config that is both a device egress target and a process argument. It must be a
+        /// plain absolute HTTPS URL to a DNS host — no quotes, whitespace or control characters
+        /// that could terminate a quoted argument. Returns an error message, or null when
+        /// valid/empty (empty = no branding image).
+        /// </summary>
+        internal static string? ValidateBrandingImageUrl(string? url)
+        {
+            if (string.IsNullOrWhiteSpace(url))
+                return null;
+
+            if (url!.Length > MaxBrandingImageUrlLength)
+                return $"must be at most {MaxBrandingImageUrlLength} characters.";
+
+            foreach (var ch in url)
+            {
+                if (char.IsControl(ch) || char.IsWhiteSpace(ch))
+                    return "must not contain whitespace or control characters.";
+                if (ch == '"' || ch == '\'')
+                    return "must not contain quotes.";
+            }
+
+            if (!Uri.TryCreate(url, UriKind.Absolute, out var uri))
+                return "is not a valid absolute URL.";
+
+            if (uri.Scheme != Uri.UriSchemeHttps)
+                return "must use HTTPS.";
+
+            if (uri.IsLoopback || string.IsNullOrEmpty(uri.Host))
+                return "must not target localhost.";
+
+            if (System.Net.IPAddress.TryParse(uri.Host, out _))
+                return "must use a DNS hostname, not an IP address.";
+
+            return null;
+        }
+
         /// <summary>
         /// Validates a candidate configuration against the stored one. Returns a
         /// user-facing error message (same wording the PUT endpoint always produced),
@@ -130,6 +173,10 @@ namespace AutopilotMonitor.Functions.Helpers
             if (channelsError != null)
                 return $"Invalid notification channels: {channelsError}";
 
+            var brandingUrlError = ValidateBrandingImageUrl(candidate.EnrollmentSummaryBrandingImageUrl);
+            if (brandingUrlError != null)
+                return $"Invalid enrollment summary branding image URL: {brandingUrlError}";
+
             var manufacturerWhitelistError = ValidateHardwareWhitelist(candidate.ManufacturerWhitelist);
             if (manufacturerWhitelistError != null)
                 return $"Invalid manufacturer whitelist: {manufacturerWhitelistError}";