diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/SummaryDialogLauncherTests.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/SummaryDialogLauncherTests.cs index f6156c86..3ca78c76 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core.Tests/Termination/SummaryDialogLauncherTests.cs +++ b/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); + } } } diff --git a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/SummaryDialogLauncher.cs b/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/SummaryDialogLauncher.cs index ec010440..21e95c9f 100644 --- a/src/Agent/AutopilotMonitor.Agent.V2.Core/Termination/SummaryDialogLauncher.cs +++ b/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 --timeout --cleanup [--branding-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; } + /// + /// 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 --status-file/--timeout/--cleanup + /// (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). + /// + 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); } diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/BrandingImageUrlValidationTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/BrandingImageUrlValidationTests.cs new file mode 100644 index 00000000..af0113c5 --- /dev/null +++ b/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 +{ + /// + /// 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. + /// + 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); + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigValidation.cs b/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigValidation.cs index 762e10ab..09d86d5f 100644 --- a/src/Backend/AutopilotMonitor.Functions/Helpers/TenantConfigValidation.cs +++ b/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; + + /// + /// Validates . 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). + /// + 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; + } + /// /// 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}";