mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 20:53:06 +00:00
Compare commits
7 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| c41c2327ab | |||
| 83effea592 | |||
| 1a4a37b77a | |||
| f56851fc9f | |||
| cbe410cbbc | |||
| 43ddc60111 | |||
| 82b26b4ab9 |
@@ -107,6 +107,13 @@ properties `Html`, `UpdatedBy`, `UpdatedUtc`), cached 5 minutes, invalidated on
|
||||
|
||||
* The only placeholder is `{{domainName}}`; an empty domain renders as "your organization",
|
||||
matching the built-ins. Subjects are not editable (pinned constants).
|
||||
* The domain is HTML-encoded at the point of substitution (`EmailTemplateService.Render` and the
|
||||
built-in template builders) — the mail goes out under the product's signed sender to a
|
||||
tenant-chosen recipient, so no writer of `DomainName` may ever inject markup into it.
|
||||
`DomainName` itself is tenant identity: seeded once at first login from the UPN domain
|
||||
(`AuthFunction`, only when it passes `TenantConfigValidation.IsValidDomainName` — a strict
|
||||
dotted host name), preserved from the stored row on the full-model config PUT for every
|
||||
caller, and "never writable" in the field-patch flow.
|
||||
* Limit 30 000 characters (Table string property cap is 32K UTF-16 chars).
|
||||
* The send path is fail-soft: a storage error falls back to the built-in body. The admin
|
||||
read path is not — a broken store surfaces as an error instead of silently showing the
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
# Log
|
||||
|
||||
## 2026-08-29 (10)
|
||||
|
||||
* **Update**: `backend/tenant-activation-welcome-mail.md` — welcome/farewell mails HTML-encode the domain at substitution; `DomainName` is frozen on the full-model config PUT (was persisted verbatim) and the first-login seed only accepts a strict DNS host name.
|
||||
|
||||
## 2026-08-29 (9)
|
||||
|
||||
* **Update**: `agent/decision-engine.md`, `agent/overview.md` — `SignalIngress.Post` is worker-thread re-entrancy-safe: signals the worker posts to itself from inside `ApplyStep` (EffectRunner `ClassifierVerdictIssued`) go to a worker-local follow-up queue drained ahead of the bounded channel instead of blocking on it, so a full channel can no longer deadlock the single consumer.
|
||||
|
||||
+37
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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); }
|
||||
|
||||
@@ -123,6 +123,22 @@ public class AuthFunctionSideEffectTests
|
||||
// HandleNewTenantDomainAsync
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
[Fact]
|
||||
public async Task HandleNewTenantDomain_WhenUpnDomainIsNotAHostName_DoesNotSeed()
|
||||
{
|
||||
var config = DefaultConfig();
|
||||
config.DomainName = null!;
|
||||
config.OnboardedBy = null;
|
||||
|
||||
// The seed is rendered into transactional mails: anything but a strict host name is refused.
|
||||
await _sut.HandleNewTenantDomainAsync(config, TenantId, "user@<b>contoso.com</b>");
|
||||
|
||||
Assert.True(string.IsNullOrEmpty(config.DomainName));
|
||||
Assert.Null(config.OnboardedBy);
|
||||
_tenantConfigMock.Verify(x => x.SaveConfigurationAsync(It.IsAny<TenantConfiguration>(), It.IsAny<string?>(), It.IsAny<string?>()), Times.Never);
|
||||
_telegramMock.Verify(x => x.SendNewTenantSignupAsync(It.IsAny<string>(), It.IsAny<string>()), Times.Never);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task HandleNewTenantDomain_WhenDomainEmpty_ExtractsAndSaves()
|
||||
{
|
||||
|
||||
@@ -0,0 +1,39 @@
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using Xunit;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
|
||||
/// <summary>
|
||||
/// Request-derived blob names must be flat. Azure.Storage.Blobs keeps '/' in the blob URI and
|
||||
/// System.Uri collapses '..' segments, so "x/../../other-container/y" would resolve outside the
|
||||
/// intended container. The guard is the last line at every GetBlobClient sink.
|
||||
/// </summary>
|
||||
public class BlobNameGuardTests
|
||||
{
|
||||
[Theory]
|
||||
[InlineData("11111111-1111-1111-1111-111111111111_aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa_diag_request_20260829_120000.zip")]
|
||||
[InlineData("report.zip")]
|
||||
[InlineData("a.b.c.zip")]
|
||||
public void IsFlat_AcceptsFlatNames(string name)
|
||||
{
|
||||
Assert.True(BlobNameGuard.IsFlat(name));
|
||||
Assert.Equal(name, BlobNameGuard.EnsureFlat(name, "name"));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(null)]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
[InlineData("TA_x/../../hosted-diagnostics/victim/payload_diag_request_1.zip")]
|
||||
[InlineData("a/b.zip")]
|
||||
[InlineData("a\\b.zip")]
|
||||
[InlineData("..zip")]
|
||||
[InlineData("a%2F..%2Fb.zip")]
|
||||
[InlineData("a?b.zip")]
|
||||
[InlineData("a#b.zip")]
|
||||
public void IsFlat_RejectsPathShapedNames(string? name)
|
||||
{
|
||||
Assert.False(BlobNameGuard.IsFlat(name));
|
||||
Assert.Throws<ArgumentException>(() => BlobNameGuard.EnsureFlat(name, "name"));
|
||||
}
|
||||
}
|
||||
@@ -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);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -247,6 +247,18 @@ public sealed class EmailServiceTests
|
||||
Assert.Contains("linkedin.com", html);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void BuiltInTemplates_HtmlEncodeDomain()
|
||||
{
|
||||
const string hostile = "<a href=\"https://evil.invalid\">x</a>";
|
||||
|
||||
foreach (var html in new[] { EmailTemplates.GetOffboardingFarewellHtml(hostile), EmailTemplates.GetPreviewApprovedHtml(hostile) })
|
||||
{
|
||||
Assert.DoesNotContain(hostile, html);
|
||||
Assert.Contains("<a href="https://evil.invalid">x</a>", html);
|
||||
}
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void FarewellTemplate_EmptyDomain_FallsBackToGenericLabel()
|
||||
{
|
||||
|
||||
@@ -29,6 +29,30 @@ public sealed class EmailTemplateServiceTests
|
||||
Assert.DoesNotContain(EmailTemplateService.DomainPlaceholder, html);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("<b>x</b>", "<b>x</b>")]
|
||||
[InlineData("a\"b&c", "a"b&c")]
|
||||
public void Render_HtmlEncodesDomain_ForOverrideTemplates(string domain, string expected)
|
||||
{
|
||||
var html = EmailTemplateService.Render("<p>{{domainName}}</p>", domain);
|
||||
|
||||
Assert.Equal($"<p>{expected}</p>", html);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData(EmailTemplateKind.Welcome)]
|
||||
[InlineData(EmailTemplateKind.Farewell)]
|
||||
public async Task BuiltIn_HtmlEncodesDomain_NoDoubleEncoding(EmailTemplateKind kind)
|
||||
{
|
||||
var (sut, _) = Build();
|
||||
|
||||
var html = await sut.GetHtmlAsync(kind, "<img src=x onerror=alert(1)>&\"");
|
||||
|
||||
Assert.DoesNotContain("<img", html);
|
||||
Assert.Contains("<img src=x onerror=alert(1)>&"", html);
|
||||
Assert.DoesNotContain("&lt;", html);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Override_WinsAndRendersPlaceholder()
|
||||
{
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
using System.Linq;
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Xunit;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests
|
||||
{
|
||||
/// <summary>
|
||||
/// The hardware whitelist is stored as one CSV and split at enforcement time. A value that
|
||||
/// carries the delimiter (e.g. an attacker-chosen distress-signal manufacturer "Dell Inc.,*")
|
||||
/// must never be able to widen the gate: the server rejects malformed lists and the parser
|
||||
/// yields exactly the trimmed, non-empty items.
|
||||
/// </summary>
|
||||
public class HardwareWhitelistCsvIntegrityTests
|
||||
{
|
||||
[Theory]
|
||||
[InlineData(null)]
|
||||
[InlineData("")]
|
||||
[InlineData(" ")]
|
||||
[InlineData("*")]
|
||||
[InlineData("Dell*,HP*,Lenovo*,Microsoft Corporation")]
|
||||
[InlineData("Dell Inc.?*")]
|
||||
[InlineData(" Dell* , HP* ")]
|
||||
public void ValidateHardwareWhitelist_WellFormed_ReturnsNull(string? csv)
|
||||
{
|
||||
Assert.Null(TenantConfigValidation.ValidateHardwareWhitelist(csv));
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("Dell Inc.,")]
|
||||
[InlineData(",Dell*")]
|
||||
[InlineData("Dell*,,HP*")]
|
||||
[InlineData("Dell*, ,HP*")]
|
||||
[InlineData(",")]
|
||||
public void ValidateHardwareWhitelist_BlankItem_Rejected(string csv)
|
||||
{
|
||||
var error = TenantConfigValidation.ValidateHardwareWhitelist(csv);
|
||||
Assert.NotNull(error);
|
||||
Assert.Contains("must not be empty", error);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ValidateHardwareWhitelist_ControlCharacter_Rejected()
|
||||
{
|
||||
var csv = "Dell*" + (char)1 + ",HP*";
|
||||
Assert.Contains("control characters", TenantConfigValidation.ValidateHardwareWhitelist(csv));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ValidateHardwareWhitelist_OverlongEntry_Rejected()
|
||||
{
|
||||
Assert.Contains("128 characters",
|
||||
TenantConfigValidation.ValidateHardwareWhitelist(new string('a', 129)));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ValidateHardwareWhitelist_TooManyEntries_Rejected()
|
||||
{
|
||||
var csv = string.Join(",", Enumerable.Repeat("x", 201));
|
||||
Assert.Contains("200 entries", TenantConfigValidation.ValidateHardwareWhitelist(csv));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ValidateModel_RejectsMalformedWhitelists_WithFieldSpecificMessage()
|
||||
{
|
||||
var existing = TenantConfiguration.CreateDefault("tenant");
|
||||
|
||||
var badMfr = TenantConfiguration.CreateDefault("tenant");
|
||||
badMfr.ManufacturerWhitelist = "Dell*,Dell Inc.,";
|
||||
Assert.StartsWith("Invalid manufacturer whitelist:",
|
||||
TenantConfigValidation.ValidateModel(badMfr, existing, isGlobalAdmin: false));
|
||||
|
||||
var badModel = TenantConfiguration.CreateDefault("tenant");
|
||||
badModel.ModelWhitelist = "Latitude*,,";
|
||||
Assert.StartsWith("Invalid model whitelist:",
|
||||
TenantConfigValidation.ValidateModel(badModel, existing, isGlobalAdmin: false));
|
||||
|
||||
Assert.Null(TenantConfigValidation.ValidateModel(
|
||||
TenantConfiguration.CreateDefault("tenant"), existing, isGlobalAdmin: false));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ParseWhitelist_TrimsEntries_SoEditorViewMatchesEnforcement()
|
||||
{
|
||||
// The web editor displays trimmed items; enforcement must match the same patterns.
|
||||
Assert.Equal(new[] { "Dell*", "HP*" }, TenantConfiguration.ParseWhitelist("Dell*, HP*"));
|
||||
Assert.Equal(new[] { "*" }, TenantConfiguration.ParseWhitelist(" "));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void NeutralizedDistressValue_MatchesOnlyItself_NotEveryManufacturer()
|
||||
{
|
||||
// "Dell Inc.,*" neutralized by the web helper to the single pattern "Dell Inc.?*".
|
||||
var whitelist = TenantConfiguration.ParseWhitelist("Dell*,Dell Inc.?*");
|
||||
Assert.DoesNotContain("*", whitelist);
|
||||
|
||||
Assert.False(HardwareWhitelistValidator.ValidateHardware("Contoso", "X1", whitelist, new[] { "*" }).IsValid);
|
||||
Assert.True(HardwareWhitelistValidator.ValidateHardware("Dell Inc.,*", "X1", whitelist, new[] { "*" }).IsValid);
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -26,7 +26,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
private const string TenantA = "11111111-1111-1111-1111-111111111111";
|
||||
private const string TenantB = "22222222-2222-2222-2222-222222222222";
|
||||
private const string Filename = "AgentDiagnostics-aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa-20260519T120000.zip";
|
||||
private const string SessionA = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa";
|
||||
private const string SessionB = "bbbbbbbb-bbbb-bbbb-bbbb-bbbbbbbbbbbb";
|
||||
private const string Filename = "AgentDiagnostics-" + SessionA + "-20260519T120000.zip";
|
||||
|
||||
// ── Path build + validation ────────────────────────────────────────────────────
|
||||
|
||||
@@ -89,7 +91,15 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync("not-a-guid", Filename));
|
||||
() => svc.GenerateUploadSasAsync("not-a-guid", SessionA, Filename));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_RejectsNonGuidSessionId()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, "not-a-guid", Filename));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
@@ -97,14 +107,64 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, "../other-tenant/escape.zip"));
|
||||
() => svc.GenerateUploadSasAsync(TenantA, SessionA, "../other-tenant/escape.zip"));
|
||||
}
|
||||
|
||||
// ── Session binding — one device must not be able to name another session's package ──
|
||||
|
||||
[Theory]
|
||||
[InlineData("AgentDiagnostics-" + SessionB + "-20260519T120000.zip")] // foreign session
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "20260519T120000.zip")] // missing separator
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-")] // nothing after the session
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260519T120000.txt")] // not a zip
|
||||
[InlineData("x-AgentDiagnostics-" + SessionA + "-20260519T120000.zip")] // wrong prefix
|
||||
[InlineData("other.zip")]
|
||||
public async Task GenerateUploadSas_RejectsFilenameNotBoundToSession(string filename)
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await Assert.ThrowsAsync<ArgumentException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, SessionA, filename));
|
||||
Assert.Empty(svc.CapturedSasBuilders);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260519T120000.zip")]
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260519T120000-preprov.zip")]
|
||||
[InlineData("AgentDiagnostics-" + SessionA + "-20260724170902-server-requested.zip")]
|
||||
[InlineData("agentdiagnostics-" + SessionA + "-20260519T120000.ZIP")] // case-insensitive
|
||||
public async Task GenerateUploadSas_AcceptsEveryAgentProducedShapeForOwnSession(string filename)
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, filename);
|
||||
Assert.Equal($"{TenantA}/{filename}", result.BlobPath);
|
||||
}
|
||||
|
||||
// ── Create-only — an existing blob is never re-issued ─────────────────────────
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_ExistingBlob_ThrowsAlreadyExists_AndMintsNothing()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService { ExistingBlobPath = $"{TenantA}/{Filename}" };
|
||||
var ex = await Assert.ThrowsAsync<HostedBlobAlreadyExistsException>(
|
||||
() => svc.GenerateUploadSasAsync(TenantA, SessionA, Filename));
|
||||
Assert.Equal($"{TenantA}/{Filename}", ex.BlobPath);
|
||||
Assert.Empty(svc.CapturedSasBuilders);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_ExistingBlobInOtherTenant_DoesNotBlockSameName()
|
||||
{
|
||||
// Existence is checked on the tenant-prefixed path, not on the bare filename.
|
||||
var svc = new FakeHostedDiagnosticsBlobService { ExistingBlobPath = $"{TenantB}/{Filename}" };
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
Assert.Equal($"{TenantA}/{Filename}", result.BlobPath);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_ReturnsTenantPrefixedBlobPath()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
Assert.Equal($"{TenantA}/{Filename}", result.BlobPath);
|
||||
}
|
||||
|
||||
@@ -112,9 +172,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
public async Task GenerateUploadSas_EnsuresContainerOnce()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantB, "other.zip");
|
||||
await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantB, SessionA, Filename);
|
||||
Assert.Equal(1, svc.EnsureContainerCallCount);
|
||||
}
|
||||
|
||||
@@ -124,7 +184,7 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var requested = TimeSpan.FromHours(24);
|
||||
var before = DateTime.UtcNow;
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, Filename, requested);
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename, requested);
|
||||
var maxAllowed = before.Add(HostedDiagnosticsBlobService.MaxUploadSasTtl).AddMinutes(1); // tolerance
|
||||
Assert.True(result.ExpiresAt <= maxAllowed,
|
||||
$"ExpiresAt {result.ExpiresAt:O} exceeded MaxUploadSasTtl-bounded {maxAllowed:O}");
|
||||
@@ -135,16 +195,16 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var before = DateTime.UtcNow;
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
var result = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
var delta = result.ExpiresAt - before;
|
||||
Assert.InRange(delta.TotalMinutes, 14, 16);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_PassesBlobScopedResourceAndWriteCreateOnlyPermsToBuilder()
|
||||
public async Task GenerateUploadSas_PassesBlobScopedResourceAndCreateOnlyPermsToBuilder()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
|
||||
Assert.Single(svc.CapturedSasBuilders);
|
||||
var builder = svc.CapturedSasBuilders.Single();
|
||||
@@ -152,24 +212,26 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
Assert.Equal($"{TenantA}/{Filename}", builder.BlobName);
|
||||
Assert.Equal("diagnostics", builder.BlobContainerName);
|
||||
|
||||
// Reconstruct the permissions string to ensure ONLY Write + Create are set —
|
||||
// ONLY Create — no Write (would allow replacing an existing same-tenant blob),
|
||||
// no Read (would expose other-tenant blobs in cross-tenant scenarios),
|
||||
// no Delete (cleanup is service-side via DeleteIfExistsAsync),
|
||||
// no List (agent has no enumeration need).
|
||||
var permsString = builder.Permissions ?? string.Empty;
|
||||
Assert.Contains("w", permsString);
|
||||
Assert.Contains("c", permsString);
|
||||
Assert.DoesNotContain("r", permsString);
|
||||
Assert.DoesNotContain("d", permsString);
|
||||
Assert.DoesNotContain("l", permsString);
|
||||
Assert.Equal("c", builder.Permissions);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void UploadSasPermissions_IsCreateOnly()
|
||||
{
|
||||
// Pinned: Write must never come back — it is what turns a create into an overwrite.
|
||||
Assert.Equal(BlobSasPermissions.Create, HostedDiagnosticsBlobService.UploadSasPermissions);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task GenerateUploadSas_DifferentTenants_GetDifferentBlobPaths()
|
||||
{
|
||||
var svc = new FakeHostedDiagnosticsBlobService();
|
||||
var a = await svc.GenerateUploadSasAsync(TenantA, Filename);
|
||||
var b = await svc.GenerateUploadSasAsync(TenantB, Filename);
|
||||
var a = await svc.GenerateUploadSasAsync(TenantA, SessionA, Filename);
|
||||
var b = await svc.GenerateUploadSasAsync(TenantB, SessionA, Filename);
|
||||
Assert.NotEqual(a.BlobPath, b.BlobPath);
|
||||
Assert.StartsWith(TenantA + "/", a.BlobPath);
|
||||
Assert.StartsWith(TenantB + "/", b.BlobPath);
|
||||
@@ -177,8 +239,6 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
|
||||
// ── DeleteBySessionPrefixAsync — cascade sweep for multi-package sessions ──────
|
||||
|
||||
private const string SessionA = "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa";
|
||||
|
||||
[Theory]
|
||||
[InlineData("not-a-guid", SessionA)]
|
||||
[InlineData(TenantA, "not-a-guid")]
|
||||
@@ -310,6 +370,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
public int EnsureContainerCallCount { get; private set; }
|
||||
public List<BlobSasBuilder> CapturedSasBuilders { get; } = new();
|
||||
|
||||
/// <summary>Blob path that the fake reports as already existing (null = nothing exists).</summary>
|
||||
public string? ExistingBlobPath { get; set; }
|
||||
|
||||
public FakeHostedDiagnosticsBlobService()
|
||||
: base(
|
||||
new BlobServiceClient("UseDevelopmentStorage=true"),
|
||||
@@ -324,6 +387,9 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
|
||||
protected override Task<bool> BlobExistsAsync(string blobPath, CancellationToken cancellationToken)
|
||||
=> Task.FromResult(ExistingBlobPath == blobPath);
|
||||
|
||||
protected override Task<Uri> BuildUploadSasUriAsync(
|
||||
string blobPath, DateTimeOffset expiresOn, CancellationToken cancellationToken)
|
||||
{
|
||||
@@ -338,7 +404,7 @@ public class HostedDiagnosticsBlobServiceTests
|
||||
StartsOn = DateTimeOffset.UtcNow.AddMinutes(-5),
|
||||
ExpiresOn = expiresOn,
|
||||
};
|
||||
builder.SetPermissions(BlobSasPermissions.Write | BlobSasPermissions.Create);
|
||||
builder.SetPermissions(HostedDiagnosticsBlobService.UploadSasPermissions);
|
||||
CapturedSasBuilders.Add(builder);
|
||||
|
||||
return Task.FromResult(new Uri($"https://fake.blob.core.windows.net/diagnostics/{blobPath}?fake=sas"));
|
||||
|
||||
@@ -1308,4 +1308,119 @@ public class PolicyEnforcementMiddlewareTests
|
||||
Assert.True(result.Allowed);
|
||||
Assert.Equal(string.Empty, result.Context!.CallerId);
|
||||
}
|
||||
|
||||
// ── Tenant suspension gate (TenantConfiguration.Disabled / DisabledUntil) ──────────
|
||||
// The operator kill-switch must revoke API access on the shared authorization path, not only
|
||||
// on the SPA's auth/me probe — a direct REST caller with a valid token must get 403 too.
|
||||
|
||||
private static void AsSuspended(Harness h, string tenantId, DateTime? until = null, string? reason = null) =>
|
||||
h.ConfigRepo.Setup(r => r.GetTenantConfigurationAsync(tenantId))
|
||||
.ReturnsAsync(new TenantConfiguration
|
||||
{
|
||||
TenantId = tenantId,
|
||||
Disabled = true,
|
||||
DisabledUntil = until,
|
||||
DisabledReason = reason,
|
||||
});
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_HomeTenant_ReadRoute_IsForbidden_WithTenantSuspendedCode()
|
||||
{
|
||||
const string upn = "admin@contoso.com";
|
||||
var h = BuildHarness();
|
||||
h.AsTenantAdmin(TenantA, upn);
|
||||
AsSuspended(h, TenantA, until: DateTime.UtcNow.AddDays(30), reason: "Abuse investigation");
|
||||
|
||||
var result = await h.Middleware.DecideAsync("GET", "/api/sessions", null, AuthedPrincipal(TenantA, upn));
|
||||
|
||||
Assert.False(result.Allowed);
|
||||
Assert.Equal(403, result.StatusCode);
|
||||
Assert.Equal("TenantSuspended", result.ErrorCode);
|
||||
Assert.Equal("Abuse investigation", result.ErrorMessage);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_HomeTenant_WriteRoute_IsForbidden()
|
||||
{
|
||||
const string upn = "admin@contoso.com";
|
||||
var h = BuildHarness();
|
||||
h.AsTenantAdmin(TenantA, upn);
|
||||
AsSuspended(h, TenantA); // indefinite suspension (no DisabledUntil)
|
||||
|
||||
var result = await h.Middleware.DecideAsync("PUT", $"/api/config/{TenantA}", null, AuthedPrincipal(TenantA, upn));
|
||||
|
||||
Assert.False(result.Allowed);
|
||||
Assert.Equal("TenantSuspended", result.ErrorCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_ExpiredDisabledUntil_IsAllowed()
|
||||
{
|
||||
const string upn = "admin@contoso.com";
|
||||
var h = BuildHarness();
|
||||
h.AsTenantAdmin(TenantA, upn);
|
||||
AsSuspended(h, TenantA, until: DateTime.UtcNow.AddMinutes(-1)); // suspension window elapsed
|
||||
|
||||
var result = await h.Middleware.DecideAsync("GET", "/api/sessions", null, AuthedPrincipal(TenantA, upn));
|
||||
|
||||
Assert.True(result.Allowed);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_GlobalAdmin_BypassesGate_ToAdministerTenant()
|
||||
{
|
||||
const string upn = "ga@platform.example";
|
||||
var h = BuildHarness();
|
||||
h.AsGlobalRole(Constants.GlobalRoles.GlobalAdmin);
|
||||
AsSuspended(h, TenantA);
|
||||
AsSuspended(h, TenantB);
|
||||
|
||||
var own = await h.Middleware.DecideAsync("GET", "/api/sessions", null, AuthedPrincipal(TenantA, upn));
|
||||
var cross = await h.Middleware.DecideAsync("PUT", $"/api/config/{TenantB}", null, AuthedPrincipal(TenantA, upn));
|
||||
|
||||
Assert.True(own.Allowed);
|
||||
Assert.True(cross.Allowed);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_DelegatedReader_TargetTenantSuspended_IsForbidden()
|
||||
{
|
||||
const string upn = "msp@partner.example";
|
||||
var h = BuildHarness();
|
||||
h.AsDelegated(TenantB);
|
||||
AsSuspended(h, TenantB); // home tenant A is fine, managed target B is suspended
|
||||
|
||||
var result = await h.Middleware.DecideAsync("GET", $"/api/config/{TenantB}", null, AuthedPrincipal(TenantA, upn));
|
||||
|
||||
Assert.False(result.Allowed);
|
||||
Assert.Equal("TenantSuspended", result.ErrorCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_DelegatedReader_HomeTenantSuspended_LosesDelegatedReach()
|
||||
{
|
||||
const string upn = "msp@partner.example";
|
||||
var h = BuildHarness();
|
||||
h.AsDelegated(TenantB);
|
||||
AsSuspended(h, TenantA); // the MSP's own home tenant is suspended
|
||||
|
||||
var result = await h.Middleware.DecideAsync("GET", $"/api/config/{TenantB}", null, AuthedPrincipal(TenantA, upn));
|
||||
|
||||
Assert.False(result.Allowed);
|
||||
Assert.Equal("TenantSuspended", result.ErrorCode);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task Suspended_DeviceRoute_NoPrincipal_GateNotApplied()
|
||||
{
|
||||
// Anonymous/device routes carry no principal — SecurityValidator owns their tenant gate. The
|
||||
// middleware must not touch tenant config here (no tenant to key on).
|
||||
var h = BuildHarness();
|
||||
AsSuspended(h, TenantA);
|
||||
|
||||
var result = await h.Middleware.DecideAsync("GET", "/api/health", null, null);
|
||||
|
||||
Assert.True(result.Allowed);
|
||||
h.ConfigRepo.Verify(r => r.GetTenantConfigurationAsync(It.IsAny<string>()), Times.Never);
|
||||
}
|
||||
}
|
||||
|
||||
+17
@@ -55,6 +55,23 @@ public class SessionReportDiagnosticsArchiveCopierTests
|
||||
Assert.Equal(0, copier.HostedOpenCalls + copier.CustomerOpenCalls + copier.UploadCalls);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("x/../../hosted-diagnostics/22222222-2222-2222-2222-222222222222/payload_diag_archive_1.zip")]
|
||||
[InlineData("sub/dir_diag_archive_1.zip")]
|
||||
[InlineData("..\\escape_diag_archive_1.zip")]
|
||||
public async Task CopyAsync_NonFlatDestination_Rejected_WithoutReadOrWrite(string destination)
|
||||
{
|
||||
// A path-shaped destination would let the SDK + System.Uri resolve the write outside
|
||||
// the session-reports container. Reject before touching any storage.
|
||||
var copier = new RecordingCopier();
|
||||
|
||||
var result = await copier.CopyAsync(TenantA, SessionId, HostedBlob, destination);
|
||||
|
||||
Assert.False(result.Success);
|
||||
Assert.Equal(SessionReportDiagnosticsArchiveCopier.Statuses.FailedInvalidBlobName, result.Status);
|
||||
Assert.Equal(0, copier.HostedOpenCalls + copier.CustomerOpenCalls + copier.UploadCalls);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task CopyAsync_Hosted_HappyPath_UploadsWithFlatName()
|
||||
{
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Linq;
|
||||
using System.Threading;
|
||||
using System.Threading.Tasks;
|
||||
using Azure;
|
||||
using Azure.Data.Tables;
|
||||
using AutopilotMonitor.Functions.DataAccess.TableStorage;
|
||||
using AutopilotMonitor.Functions.Functions.Config;
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared;
|
||||
using AutopilotMonitor.Shared.DataAccess;
|
||||
@@ -386,4 +388,40 @@ public class TenantContactEmailTests
|
||||
It.IsAny<string>(), It.IsAny<string>(),
|
||||
It.IsAny<IEnumerable<string>>(), It.IsAny<CancellationToken>()), Times.Never);
|
||||
}
|
||||
|
||||
// -------------------------------------------------------------------------
|
||||
// IsValidDomainName — the only shape DomainName may be persisted in
|
||||
// -------------------------------------------------------------------------
|
||||
|
||||
[Theory]
|
||||
[InlineData("contoso.com")]
|
||||
[InlineData("sub.domain.org")]
|
||||
[InlineData("x-y.example")]
|
||||
[InlineData("A1.B2.c3")]
|
||||
public void IsValidDomainName_AcceptsHostNames(string domain)
|
||||
=> Assert.True(TenantConfigValidation.IsValidDomainName(domain));
|
||||
|
||||
[Theory]
|
||||
[InlineData(null)]
|
||||
[InlineData("")]
|
||||
[InlineData("localhost")]
|
||||
[InlineData(".contoso.com")]
|
||||
[InlineData("contoso.com.")]
|
||||
[InlineData("con..toso.com")]
|
||||
[InlineData("-contoso.com")]
|
||||
[InlineData("contoso-.com")]
|
||||
[InlineData("con toso.com")]
|
||||
[InlineData("<b>contoso.com</b>")]
|
||||
[InlineData("contoso.com\"onerror=\"x")]
|
||||
[InlineData("contoso.com/path")]
|
||||
[InlineData("user@contoso.com")]
|
||||
public void IsValidDomainName_RejectsNonHostNames(string? domain)
|
||||
=> Assert.False(TenantConfigValidation.IsValidDomainName(domain));
|
||||
|
||||
[Fact]
|
||||
public void IsValidDomainName_RejectsOverlongNamesAndLabels()
|
||||
{
|
||||
Assert.False(TenantConfigValidation.IsValidDomainName(new string('a', 64) + ".com"));
|
||||
Assert.False(TenantConfigValidation.IsValidDomainName(string.Join(".", Enumerable.Repeat(new string('a', 60), 5))));
|
||||
}
|
||||
}
|
||||
|
||||
+6
@@ -110,6 +110,12 @@ namespace AutopilotMonitor.Functions.Functions.Config
|
||||
// admins are served the FULL config so this is normally a no-op for them.)
|
||||
config.RestoreRedactedSecretsFrom(existingConfig);
|
||||
|
||||
// DomainName is tenant identity: seeded once from the first login's UPN (AuthFunction)
|
||||
// and never client-writable — for ANY caller, GA included — mirroring the patch flow
|
||||
// ("identity — never writable"). Without this preserve the full-model PUT persisted an
|
||||
// arbitrary string here, which is rendered into the welcome/farewell mails.
|
||||
config.DomainName = existingConfig.DomainName;
|
||||
|
||||
// Shared model validation (rate limits, contact address, webhook/Teams SSRF, custom
|
||||
// headers, notification channels, diagnostics SAS, retention cap) — single source
|
||||
// with the transactional field-patch flow (TenantConfigValidation).
|
||||
|
||||
+66
-9
@@ -2,6 +2,7 @@
|
||||
using System.Web;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Functions.Services.Deletion;
|
||||
using AutopilotMonitor.Functions.Services.Diagnostics;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.Azure.Functions.Worker;
|
||||
@@ -19,8 +20,10 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
/// container SAS URL stored in <see cref="TenantConfiguration.DiagnosticsBlobSasUrl"/>.
|
||||
/// Data stays in the customer's own storage account.</item>
|
||||
/// <item><c>Hosted</c> (opt-in): mints a freshly-stamped, 15-min, blob-scoped,
|
||||
/// Write+Create-only SAS pinned to <c>{tenantId}/{filename}</c> in the backend's
|
||||
/// own diagnostics container. Requires explicit admin opt-in via the UI; never
|
||||
/// Create-only SAS pinned to <c>{tenantId}/{filename}</c> in the backend's
|
||||
/// own diagnostics container. The filename must carry the requesting session id
|
||||
/// and the blob must not exist yet, so one device can neither name nor replace
|
||||
/// another session's package. Requires explicit admin opt-in via the UI; never
|
||||
/// silent.</item>
|
||||
/// </list>
|
||||
/// Called by the agent just before uploading — never at startup, never cached.
|
||||
@@ -56,6 +59,8 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
private readonly IntuneDeviceBindingValidator _intuneDeviceBindingValidator;
|
||||
private readonly BootstrapSessionService _bootstrapSessionService;
|
||||
private readonly HostedDiagnosticsBlobService _hostedDiagnostics;
|
||||
private readonly ISessionDeletionInventoryReader _sessionRowReader;
|
||||
private readonly SessionOwnerBindingObserver _ownerBinding;
|
||||
|
||||
public GetDiagnosticsUploadUrlFunction(
|
||||
ILogger<GetDiagnosticsUploadUrlFunction> logger,
|
||||
@@ -68,8 +73,12 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
CloudPcDeviceValidator cloudPcDeviceValidator,
|
||||
IntuneDeviceBindingValidator intuneDeviceBindingValidator,
|
||||
BootstrapSessionService bootstrapSessionService,
|
||||
HostedDiagnosticsBlobService hostedDiagnostics)
|
||||
HostedDiagnosticsBlobService hostedDiagnostics,
|
||||
ISessionDeletionInventoryReader sessionRowReader,
|
||||
SessionOwnerBindingObserver ownerBinding)
|
||||
{
|
||||
_sessionRowReader = sessionRowReader;
|
||||
_ownerBinding = ownerBinding;
|
||||
_logger = logger;
|
||||
_configService = configService;
|
||||
_adminConfigService = adminConfigService;
|
||||
@@ -145,7 +154,7 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
|
||||
if (destination == DestinationHosted)
|
||||
{
|
||||
return await IssueHostedSasAsync(req, requestBody);
|
||||
return await IssueHostedSasAsync(req, requestBody, validation);
|
||||
}
|
||||
|
||||
if (destination == DestinationCustomerSas)
|
||||
@@ -237,7 +246,7 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
}
|
||||
|
||||
private async Task<HttpResponseData> IssueHostedSasAsync(
|
||||
HttpRequestData req, GetDiagnosticsUploadUrlRequest requestBody)
|
||||
HttpRequestData req, GetDiagnosticsUploadUrlRequest requestBody, SecurityValidationResult validation)
|
||||
{
|
||||
if (string.IsNullOrEmpty(requestBody.FileName))
|
||||
{
|
||||
@@ -250,18 +259,36 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
return badRequest;
|
||||
}
|
||||
|
||||
// The blob name is bound to the session below, so the session id has to be a real
|
||||
// one — and it is the coordinate for the owner-binding observation.
|
||||
if (!SecurityValidator.IsValidGuid(requestBody.SessionId))
|
||||
{
|
||||
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await badRequest.WriteAsJsonAsync(new GetDiagnosticsUploadUrlResponse
|
||||
{
|
||||
Success = false,
|
||||
Message = "sessionId is required for hosted destination"
|
||||
});
|
||||
return badRequest;
|
||||
}
|
||||
|
||||
// SESSION-OWNER-BINDING-SHADOW: same observation as the session-scoped writes, so
|
||||
// stage-2 enforcement covers this endpoint with the same data. Never rejects here.
|
||||
await ObserveSessionOwnerAsync(req, requestBody.TenantId, requestBody.SessionId, validation);
|
||||
|
||||
HostedUploadSasResult sasResult;
|
||||
try
|
||||
{
|
||||
sasResult = await _hostedDiagnostics.GenerateUploadSasAsync(
|
||||
requestBody.TenantId, requestBody.FileName);
|
||||
requestBody.TenantId, requestBody.SessionId, requestBody.FileName);
|
||||
}
|
||||
catch (ArgumentException ex)
|
||||
{
|
||||
// Filename validation failed (path separator, traversal, oversize, etc.)
|
||||
// Filename validation failed (path separator, traversal, oversize, not bound
|
||||
// to the requesting session, etc.)
|
||||
_logger.LogWarning(
|
||||
"GetDiagnosticsUploadUrl: rejecting hosted SAS for tenant {TenantId}, file {FileName}: {Reason}",
|
||||
requestBody.TenantId, requestBody.FileName, ex.Message);
|
||||
"GetDiagnosticsUploadUrl: rejecting hosted SAS for tenant {TenantId}, session {SessionId}, file {FileName}: {Reason}",
|
||||
requestBody.TenantId, requestBody.SessionId, requestBody.FileName, ex.Message);
|
||||
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await badRequest.WriteAsJsonAsync(new GetDiagnosticsUploadUrlResponse
|
||||
{
|
||||
@@ -270,6 +297,21 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
});
|
||||
return badRequest;
|
||||
}
|
||||
catch (HostedBlobAlreadyExistsException ex)
|
||||
{
|
||||
// A legitimate agent never re-requests an existing name (every package is
|
||||
// freshly timestamped); this is either a replay or an overwrite attempt.
|
||||
_logger.LogWarning(
|
||||
"GetDiagnosticsUploadUrl: rejecting hosted SAS for tenant {TenantId}, session {SessionId}: blob {BlobPath} already exists",
|
||||
requestBody.TenantId, requestBody.SessionId, ex.BlobPath);
|
||||
var conflict = req.CreateResponse(HttpStatusCode.Conflict);
|
||||
await conflict.WriteAsJsonAsync(new GetDiagnosticsUploadUrlResponse
|
||||
{
|
||||
Success = false,
|
||||
Message = "A diagnostics package with this name already exists"
|
||||
});
|
||||
return conflict;
|
||||
}
|
||||
|
||||
_logger.LogInformation(
|
||||
"GetDiagnosticsUploadUrl: Issuing Hosted upload URL for tenant {TenantId}, session {SessionId}, blob {BlobPath}, SAS expires {ExpiresAt}",
|
||||
@@ -288,6 +330,21 @@ namespace AutopilotMonitor.Functions.Functions.Diagnostics
|
||||
return response;
|
||||
}
|
||||
|
||||
private async Task ObserveSessionOwnerAsync(
|
||||
HttpRequestData req, string tenantId, string sessionId, SecurityValidationResult validation)
|
||||
{
|
||||
try
|
||||
{
|
||||
var row = await _sessionRowReader.GetSessionRowAsync(tenantId, sessionId);
|
||||
_ownerBinding.Observe(req, tenantId, sessionId, row, validation, "agent/upload-url");
|
||||
}
|
||||
catch (Exception ex)
|
||||
{
|
||||
_logger.LogWarning(ex,
|
||||
"GetDiagnosticsUploadUrl: session-owner observation skipped for session {SessionId}", sessionId);
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Normalizes the destination string from TenantConfiguration. Null/empty becomes
|
||||
/// CustomerSas (preserves existing behaviour for tenants that predate this field).
|
||||
|
||||
@@ -2,6 +2,7 @@ using System.Collections.Generic;
|
||||
using System.Net;
|
||||
using AutopilotMonitor.Functions.Extensions;
|
||||
using AutopilotMonitor.Functions.Functions.Admin;
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared;
|
||||
@@ -336,6 +337,14 @@ public class AuthFunction
|
||||
if (string.IsNullOrEmpty(domain))
|
||||
return;
|
||||
|
||||
// The seeded value is rendered into transactional mails — persist only a strict host name,
|
||||
// regardless of what the identity provider's UPN charset happens to allow.
|
||||
if (!TenantConfigValidation.IsValidDomainName(domain))
|
||||
{
|
||||
_logger.LogWarning("Refusing to seed DomainName for tenant {TenantId}: UPN domain is not a valid host name", tenantId);
|
||||
return;
|
||||
}
|
||||
|
||||
_logger.LogInformation("Setting domain name for tenant {TenantId}: {Domain}", tenantId, domain);
|
||||
tenantConfig.DomainName = domain;
|
||||
tenantConfig.UpdatedBy = upn;
|
||||
|
||||
+2
-1
@@ -1,5 +1,6 @@
|
||||
using System.Net;
|
||||
using System.Web;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using Microsoft.Azure.Functions.Worker;
|
||||
using Microsoft.Azure.Functions.Worker.Http;
|
||||
@@ -44,7 +45,7 @@ namespace AutopilotMonitor.Functions.Functions.Reports
|
||||
}
|
||||
|
||||
// Prevent path traversal
|
||||
if (blobName.Contains("..") || blobName.Contains("/") || blobName.Contains("\\"))
|
||||
if (!BlobNameGuard.IsFlat(blobName))
|
||||
{
|
||||
var badRequest = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await badRequest.WriteAsJsonAsync(new { success = false, message = "Invalid blob name." });
|
||||
|
||||
+9
@@ -1,5 +1,6 @@
|
||||
using System.Net;
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared.DataAccess;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
@@ -106,6 +107,14 @@ namespace AutopilotMonitor.Functions.Functions.Reports
|
||||
return bad;
|
||||
}
|
||||
|
||||
// TenantId becomes part of the blob name — must be a GUID, never path-shaped.
|
||||
if (!SecurityValidator.IsValidGuid(request.TenantId))
|
||||
{
|
||||
var invalid = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await invalid.WriteAsJsonAsync(new { success = false, message = "Invalid tenantId." });
|
||||
return invalid;
|
||||
}
|
||||
|
||||
var metadata = await _sessionReportService.SubmitDiagFilesReportAsync(request, userIdentifier);
|
||||
|
||||
// Audit log — skip for Global Admins (their actions are not tenant-scoped)
|
||||
|
||||
+14
@@ -1,5 +1,6 @@
|
||||
using System.Net;
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared.DataAccess;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
@@ -105,6 +106,19 @@ namespace AutopilotMonitor.Functions.Functions.Reports
|
||||
return bad;
|
||||
}
|
||||
|
||||
// Identifier format gate: both values become part of the report blob name.
|
||||
// Route values are percent-decoded, so an encoded '/' or '..' would otherwise
|
||||
// reach the storage layer as a path segment.
|
||||
if (!SecurityValidator.IsValidGuid(request.SessionId) || !SecurityValidator.IsValidGuid(request.TenantId))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"SubmitSessionReport: BLOCKED non-GUID identifier user={User} tenant={TenantId} sessionIdLength={SessionIdLength}",
|
||||
userIdentifier, request.TenantId, request.SessionId?.Length ?? 0);
|
||||
var invalid = req.CreateResponse(HttpStatusCode.BadRequest);
|
||||
await invalid.WriteAsJsonAsync(new { success = false, message = "Invalid sessionId or tenantId." });
|
||||
return invalid;
|
||||
}
|
||||
|
||||
// Submit report
|
||||
var metadata = await _sessionReportService.SubmitReportAsync(request, userIdentifier);
|
||||
|
||||
|
||||
@@ -3,6 +3,7 @@ using System.Collections.Generic;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using AutopilotMonitor.Shared.Models.Config;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Helpers
|
||||
{
|
||||
@@ -21,6 +22,120 @@ namespace AutopilotMonitor.Functions.Helpers
|
||||
// RFC 5321 caps a forward path at 254 characters.
|
||||
internal const int MaxContactEmailLength = 254;
|
||||
|
||||
// RFC 1035: a host name is at most 253 characters, labels at most 63.
|
||||
internal const int MaxDomainNameLength = 253;
|
||||
|
||||
/// <summary>
|
||||
/// True when <paramref name="domainName"/> is a strict DNS host name: dotted labels of
|
||||
/// ASCII letters, digits and inner hyphens only. This is the ONLY shape DomainName may
|
||||
/// ever be persisted in — it is rendered into transactional mails and admin views.
|
||||
/// </summary>
|
||||
internal static bool IsValidDomainName(string? domainName)
|
||||
{
|
||||
if (string.IsNullOrEmpty(domainName) || domainName.Length > MaxDomainNameLength)
|
||||
return false;
|
||||
|
||||
var labels = domainName.Split('.');
|
||||
if (labels.Length < 2)
|
||||
return false;
|
||||
|
||||
foreach (var label in labels)
|
||||
{
|
||||
if (label.Length == 0 || label.Length > 63)
|
||||
return false;
|
||||
if (label[0] == '-' || label[label.Length - 1] == '-')
|
||||
return false;
|
||||
foreach (var ch in label)
|
||||
{
|
||||
var ok = (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '-';
|
||||
if (!ok)
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
return true;
|
||||
}
|
||||
|
||||
private const int MaxWhitelistEntries = 200;
|
||||
private const int MaxWhitelistEntryLength = 128;
|
||||
|
||||
/// <summary>
|
||||
/// Validates a hardware whitelist CSV (<see cref="TenantConfiguration.ManufacturerWhitelist"/> /
|
||||
/// <see cref="TenantConfiguration.ModelWhitelist"/>). The CSV is split on ',' at enforcement
|
||||
/// time, so this is the last line of defence against a value that was meant as ONE entry but
|
||||
/// alters the list structure: blank items (",,", ", ,") are rejected rather than silently
|
||||
/// dropped, and every item must be a printable, bounded pattern. Returns a user-facing
|
||||
/// reason, or null when the list is well-formed. Empty/whitespace = allow all (documented).
|
||||
/// </summary>
|
||||
internal static string? ValidateHardwareWhitelist(string? csv)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(csv))
|
||||
return null;
|
||||
|
||||
var items = csv!.Split(',');
|
||||
if (items.Length > MaxWhitelistEntries)
|
||||
return $"at most {MaxWhitelistEntries} entries are allowed.";
|
||||
|
||||
foreach (var raw in items)
|
||||
{
|
||||
var entry = raw.Trim();
|
||||
if (entry.Length == 0)
|
||||
return "entries must not be empty (check for a stray or doubled comma).";
|
||||
if (entry.Length > MaxWhitelistEntryLength)
|
||||
return $"each entry must be at most {MaxWhitelistEntryLength} characters.";
|
||||
foreach (var ch in entry)
|
||||
{
|
||||
if (char.IsControl(ch))
|
||||
return "entries must not contain control characters.";
|
||||
}
|
||||
}
|
||||
|
||||
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),
|
||||
@@ -58,6 +173,18 @@ 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}";
|
||||
|
||||
var modelWhitelistError = ValidateHardwareWhitelist(candidate.ModelWhitelist);
|
||||
if (modelWhitelistError != null)
|
||||
return $"Invalid model whitelist: {modelWhitelistError}";
|
||||
|
||||
// Only validate the customer-supplied SAS URL when the tenant has actually selected
|
||||
// the CustomerSas destination — a stale value left over from a prior CustomerSas
|
||||
// configuration must not block a Hosted save (mirrors GetDiagnosticsUploadUrlFunction).
|
||||
|
||||
@@ -5,6 +5,7 @@ using AutopilotMonitor.Functions.Helpers;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services;
|
||||
using AutopilotMonitor.Shared;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.AspNetCore.Http;
|
||||
using Microsoft.Azure.Functions.Worker;
|
||||
using Microsoft.Azure.Functions.Worker.Middleware;
|
||||
@@ -212,6 +213,29 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
|
||||
decision.UserIdentifier, decision.UserRole, "CrossTenant");
|
||||
}
|
||||
|
||||
// Tenant suspension gate — the operator kill-switch (TenantConfiguration.Disabled/DisabledUntil) and
|
||||
// the offboarding tombstone. Enforced HERE, on the shared authorization path, so it covers every
|
||||
// tenant-scoped API call and not just the SPA's auth/me login probe (which also carries it, for the
|
||||
// richer client payload). Both the caller's HOME tenant (JWT tid) and the request's TARGET tenant are
|
||||
// checked: a suspended tenant's members lose everything, including delegated reach into other tenants,
|
||||
// and nobody without platform scope reaches a suspended target. Platform operators (GA/Reader) bypass —
|
||||
// they must still administer the suspended tenant. Anonymous/device routes bring their own gate
|
||||
// (SecurityValidator → 403 TenantDisabled). Cached, side-effect-free read; a missing row is not disabled.
|
||||
if (principal != null && !hasGlobalScope)
|
||||
{
|
||||
var suspended = await FindSuspendedTenantAsync(jwtTenantId, targetTenantId);
|
||||
if (suspended != null)
|
||||
{
|
||||
_logger.LogWarning("[PolicyEnforcement] BLOCKED suspended tenant: user={User} tenant={Tenant} path={Path}",
|
||||
LogSanitizer.Clean(decision.UserIdentifier), LogSanitizer.Clean(suspended.TenantId), logPath);
|
||||
return PolicyResult.Deny((int)HttpStatusCode.Forbidden, "TenantSuspended",
|
||||
!string.IsNullOrEmpty(suspended.DisabledReason)
|
||||
? suspended.DisabledReason
|
||||
: "Your tenant has been suspended. Please contact support for more information.",
|
||||
decision.UserIdentifier, decision.UserRole, "TenantSuspended");
|
||||
}
|
||||
}
|
||||
|
||||
// Merge delegated state from the two delegated admission paths:
|
||||
// • scoped-route (RouteParam/QueryParam): a single-tenant cross-tenant read — delegatedRole +
|
||||
// allowedTenantIds were set above.
|
||||
@@ -259,6 +283,26 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
|
||||
return PolicyResult.Allow(requestContext, decision.UserIdentifier, decision.UserRole);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the configuration of the first currently-suspended tenant among the caller's home tenant and
|
||||
/// the request target (null when neither is suspended). <see cref="TenantConfigurationService.TryGetConfigurationAsync"/>
|
||||
/// is cached (5 min TTL, same as the agent gate) and never persists a default row.
|
||||
/// <see cref="TenantConfiguration.IsCurrentlyDisabled"/> already treats an elapsed DisabledUntil as
|
||||
/// re-enabled, so the gate lifts on its own; auth/me persists the flip on the next login.
|
||||
/// </summary>
|
||||
private async Task<TenantConfiguration?> FindSuspendedTenantAsync(string jwtTenantId, string targetTenantId)
|
||||
{
|
||||
var (homeConfig, _) = await _tenantConfigService.TryGetConfigurationAsync(jwtTenantId);
|
||||
if (homeConfig.IsCurrentlyDisabled())
|
||||
return homeConfig;
|
||||
|
||||
if (string.Equals(targetTenantId, jwtTenantId, StringComparison.OrdinalIgnoreCase))
|
||||
return null;
|
||||
|
||||
var (targetConfig, _) = await _tenantConfigService.TryGetConfigurationAsync(targetTenantId);
|
||||
return targetConfig.IsCurrentlyDisabled() ? targetConfig : null;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the request's target tenant. Returns the effective TargetTenantId (defaults to the JWT
|
||||
/// tenant) and the explicitly-named tenant from the route/query (null when the route does not name one,
|
||||
|
||||
@@ -0,0 +1,28 @@
|
||||
namespace AutopilotMonitor.Functions.Security
|
||||
{
|
||||
/// <summary>
|
||||
/// Last-line guard for request-derived blob names. Azure.Storage.Blobs appends the name to
|
||||
/// the container URI preserving '/' and System.Uri collapses '..' segments, so a name such as
|
||||
/// "x/../../other-container/y" resolves OUTSIDE the intended container. Every blob name that
|
||||
/// contains caller-influenced material must be flat: no separators, no dot segments.
|
||||
/// </summary>
|
||||
public static class BlobNameGuard
|
||||
{
|
||||
public static bool IsFlat(string? blobName)
|
||||
{
|
||||
if (string.IsNullOrWhiteSpace(blobName)) return false;
|
||||
if (blobName.Contains('/') || blobName.Contains('\\')) return false;
|
||||
if (blobName.Contains("..")) return false;
|
||||
if (blobName.Contains('%') || blobName.Contains('?') || blobName.Contains('#')) return false;
|
||||
return true;
|
||||
}
|
||||
|
||||
/// <summary>Throws <see cref="ArgumentException"/> when the name is not flat.</summary>
|
||||
public static string EnsureFlat(string? blobName, string parameterName)
|
||||
{
|
||||
if (!IsFlat(blobName))
|
||||
throw new ArgumentException("Blob name must be a flat name without path separators or dot segments.", parameterName);
|
||||
return blobName!;
|
||||
}
|
||||
}
|
||||
}
|
||||
+81
-11
@@ -27,8 +27,8 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
/// (<c>AzureStorageAccountName</c>) or connection string (<c>AzureBlobStorageConnectionString</c>).
|
||||
/// SAS issuance for the agent upload-url endpoint differs by auth path — User
|
||||
/// Delegation SAS under MI, account-key SAS under connection-string — both produce
|
||||
/// a blob-scoped, Write+Create-only, 15-min token that cannot reach other tenants'
|
||||
/// prefixes.
|
||||
/// a blob-scoped, Create-only, 15-min token that cannot reach other tenants'
|
||||
/// prefixes and cannot replace an existing blob.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public class HostedDiagnosticsBlobService
|
||||
@@ -40,6 +40,16 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
// requesting hour-long tokens.
|
||||
public static readonly TimeSpan MaxUploadSasTtl = TimeSpan.FromMinutes(60);
|
||||
|
||||
/// <summary>
|
||||
/// Permissions on the upload SAS. Create-only by design: the agent PUTs one new block
|
||||
/// blob per package (fresh timestamped name), so Write — which would allow replacing an
|
||||
/// existing blob under the same tenant prefix — is never needed.
|
||||
/// </summary>
|
||||
public const BlobSasPermissions UploadSasPermissions = BlobSasPermissions.Create;
|
||||
|
||||
/// <summary>Canonical package-name prefix; the session id follows immediately after.</summary>
|
||||
internal const string PackageNamePrefix = "AgentDiagnostics-";
|
||||
|
||||
private readonly BlobServiceClient _blobServiceClient;
|
||||
private readonly ILogger<HostedDiagnosticsBlobService> _logger;
|
||||
private readonly bool _usesManagedIdentity;
|
||||
@@ -94,16 +104,21 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
// -------- Public API --------
|
||||
|
||||
/// <summary>
|
||||
/// Mints a blob-scoped, Write+Create-only SAS URI pinned to the EXACT path
|
||||
/// Mints a blob-scoped, Create-only SAS URI pinned to the EXACT path
|
||||
/// <c>{tenantId}/{filename}</c> for an agent's diagnostics-package upload. Cannot
|
||||
/// be redirected to other tenants' prefixes. TTL is capped at
|
||||
/// <see cref="MaxUploadSasTtl"/> to keep the exposure window tight.
|
||||
/// be redirected to other tenants' prefixes, and the filename must carry the
|
||||
/// caller's <paramref name="sessionId"/> (<c>AgentDiagnostics-{sessionId}-…zip</c>)
|
||||
/// so one device cannot name another session's package. An already-existing blob
|
||||
/// is never re-issued (<see cref="HostedBlobAlreadyExistsException"/>). TTL is
|
||||
/// capped at <see cref="MaxUploadSasTtl"/> to keep the exposure window tight.
|
||||
/// </summary>
|
||||
public virtual async Task<HostedUploadSasResult> GenerateUploadSasAsync(
|
||||
string tenantId, string filename, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
|
||||
string tenantId, string sessionId, string filename, TimeSpan? ttl = null, CancellationToken cancellationToken = default)
|
||||
{
|
||||
SecurityValidator.EnsureValidGuid(tenantId, nameof(tenantId));
|
||||
SecurityValidator.EnsureValidGuid(sessionId, nameof(sessionId));
|
||||
ValidateFilename(filename);
|
||||
EnsureFilenameBoundToSession(filename, sessionId);
|
||||
|
||||
var effectiveTtl = ttl ?? TimeSpan.FromMinutes(15);
|
||||
if (effectiveTtl > MaxUploadSasTtl)
|
||||
@@ -116,6 +131,14 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
|
||||
await EnsureContainerAsync(cancellationToken);
|
||||
|
||||
// Create-only semantics, enforced twice: the SAS carries no Write permission (an
|
||||
// existing blob cannot be replaced with it), and issuance itself refuses a name
|
||||
// that is already taken so the conflict surfaces here as a 409 instead of as an
|
||||
// opaque 403 from storage. Every package gets a fresh timestamped name, so a
|
||||
// legitimate agent never asks for an existing one.
|
||||
if (await BlobExistsAsync(blobPath, cancellationToken))
|
||||
throw new HostedBlobAlreadyExistsException(blobPath);
|
||||
|
||||
var sasUri = await BuildUploadSasUriAsync(blobPath, expiresOn, cancellationToken);
|
||||
|
||||
return new HostedUploadSasResult
|
||||
@@ -185,7 +208,7 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
|
||||
// Trailing dash bounds the match to this exact session id (GUIDs are fixed-length,
|
||||
// but the explicit separator keeps the invariant obvious and layout-proof).
|
||||
var prefix = $"{tenantId}/AgentDiagnostics-{sessionId}-";
|
||||
var prefix = $"{tenantId}/{PackageNamePrefix}{sessionId}-";
|
||||
var deleted = 0;
|
||||
await foreach (var item in containerClient.GetBlobsAsync(
|
||||
BlobTraits.None, BlobStates.None, prefix: prefix, cancellationToken: cancellationToken))
|
||||
@@ -272,7 +295,17 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the blob-scoped Write+Create-only SAS URI for upload. Branch on the
|
||||
/// Test seam: whether a blob already exists at <paramref name="blobPath"/>. Production
|
||||
/// issues a HEAD; tests override to simulate a taken name without Azurite.
|
||||
/// </summary>
|
||||
protected virtual async Task<bool> BlobExistsAsync(string blobPath, CancellationToken cancellationToken)
|
||||
{
|
||||
var response = await GetContainerClient().GetBlobClient(blobPath).ExistsAsync(cancellationToken);
|
||||
return response.Value;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the blob-scoped Create-only SAS URI for upload. Branch on the
|
||||
/// auth mode: MI uses User Delegation SAS (no account key needed); connection
|
||||
/// string uses the underlying StorageSharedKeyCredential via
|
||||
/// <see cref="BlobClient.GenerateSasUri(BlobSasBuilder)"/>.
|
||||
@@ -290,9 +323,10 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
StartsOn = DateTimeOffset.UtcNow.AddMinutes(-5), // clock-skew tolerance
|
||||
ExpiresOn = expiresOn,
|
||||
};
|
||||
// Write + Create only — agent must not be able to Read other tenants' blobs,
|
||||
// Delete anything, or List the container.
|
||||
sasBuilder.SetPermissions(BlobSasPermissions.Write | BlobSasPermissions.Create);
|
||||
// Create only — the agent does a single Put Blob of a brand-new name. No Write
|
||||
// (would let the holder replace an existing blob of the same tenant), no Read,
|
||||
// no Delete, no List.
|
||||
sasBuilder.SetPermissions(UploadSasPermissions);
|
||||
|
||||
if (UsesManagedIdentity)
|
||||
{
|
||||
@@ -337,6 +371,26 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
throw new ArgumentException("filename must not contain null bytes", nameof(filename));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Binds an upload filename to the requesting session: it must be
|
||||
/// <c>AgentDiagnostics-{sessionId}-{anything}.zip</c> (the shape the agent has always
|
||||
/// produced, incl. the <c>-preprov</c>/<c>-server-requested</c> suffixes). Without this
|
||||
/// an authenticated device could name any other session's package in its tenant.
|
||||
/// Throws <see cref="ArgumentException"/> on rejection.
|
||||
/// </summary>
|
||||
internal static void EnsureFilenameBoundToSession(string filename, string sessionId)
|
||||
{
|
||||
var required = $"{PackageNamePrefix}{sessionId}-";
|
||||
if (!filename.StartsWith(required, StringComparison.OrdinalIgnoreCase)
|
||||
|| filename.Length <= required.Length
|
||||
|| !filename.EndsWith(".zip", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
throw new ArgumentException(
|
||||
"filename must be of the form 'AgentDiagnostics-{sessionId}-{timestamp}.zip' for the requesting session",
|
||||
nameof(filename));
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Validates a stored blob path (<c>{tenantId}/{filename}</c>) on the read paths.
|
||||
/// Defends the download proxy against path-traversal even if a row was tampered with.
|
||||
@@ -358,6 +412,22 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Thrown by <see cref="HostedDiagnosticsBlobService.GenerateUploadSasAsync"/> when the
|
||||
/// requested blob name is already taken. Distinct from <see cref="ArgumentException"/>
|
||||
/// (malformed input → 400) so the Function can answer 409.
|
||||
/// </summary>
|
||||
public sealed class HostedBlobAlreadyExistsException : InvalidOperationException
|
||||
{
|
||||
public HostedBlobAlreadyExistsException(string blobPath)
|
||||
: base($"Hosted diagnostics blob already exists: {blobPath}")
|
||||
{
|
||||
BlobPath = blobPath;
|
||||
}
|
||||
|
||||
public string BlobPath { get; }
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Return value of <see cref="HostedDiagnosticsBlobService.GenerateUploadSasAsync"/>.
|
||||
/// </summary>
|
||||
|
||||
+12
-1
@@ -1,4 +1,5 @@
|
||||
using AutopilotMonitor.Functions.Functions.Diagnostics;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Azure;
|
||||
using Azure.Storage.Blobs;
|
||||
@@ -100,6 +101,16 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
if (string.IsNullOrEmpty(sourceBlobName))
|
||||
return new DiagnosticsArchiveCopyResult(false, Statuses.FailedNoDiagnostics, null);
|
||||
|
||||
// Destination is derived from request data (tenant/session ids). A non-flat name
|
||||
// would let the SDK + System.Uri walk out of the session-reports container.
|
||||
if (!BlobNameGuard.IsFlat(destinationBlobName))
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"DiagnosticsArchiveCopier: rejecting non-flat destination blob name for tenant {TenantId}, session {SessionId}",
|
||||
tenantId, sessionId);
|
||||
return new DiagnosticsArchiveCopyResult(false, Statuses.FailedInvalidBlobName, null);
|
||||
}
|
||||
|
||||
try
|
||||
{
|
||||
var (destination, classifyErr) = DiagnosticsDownloadFunction.ClassifyBlobName(sourceBlobName, tenantId);
|
||||
@@ -219,7 +230,7 @@ namespace AutopilotMonitor.Functions.Services.Diagnostics
|
||||
{
|
||||
var containerClient = _blobStorage.GetContainerClient(ReportsContainerName);
|
||||
await containerClient.CreateIfNotExistsAsync(cancellationToken: cancellationToken);
|
||||
var blobClient = containerClient.GetBlobClient(destinationBlobName);
|
||||
var blobClient = containerClient.GetBlobClient(BlobNameGuard.EnsureFlat(destinationBlobName, nameof(destinationBlobName)));
|
||||
await blobClient.UploadAsync(content, new BlobUploadOptions
|
||||
{
|
||||
HttpHeaders = new BlobHttpHeaders { ContentType = "application/zip" }
|
||||
|
||||
@@ -2,6 +2,7 @@ using AutopilotMonitor.Shared.DataAccess;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
using Microsoft.Extensions.Caching.Memory;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using System.Net;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Services;
|
||||
|
||||
@@ -73,10 +74,15 @@ public class EmailTemplateService : IEmailTemplateProvider
|
||||
? EmailTemplates.GetPreviewApprovedHtml(DomainPlaceholder)
|
||||
: EmailTemplates.GetOffboardingFarewellHtml(DomainPlaceholder);
|
||||
|
||||
/// <summary>Substitutes the placeholder; an empty domain becomes "your organization".</summary>
|
||||
/// <summary>
|
||||
/// Substitutes the placeholder; an empty domain becomes "your organization". The value is
|
||||
/// HTML-encoded at the point of substitution: the mail goes out under the product's signed
|
||||
/// sender to a tenant-chosen recipient, so no writer of DomainName — current or future — may
|
||||
/// ever be able to inject markup into it.
|
||||
/// </summary>
|
||||
public static string Render(string rawHtml, string? domainName)
|
||||
{
|
||||
var display = string.IsNullOrWhiteSpace(domainName) ? EmptyDomainLabel : domainName.Trim();
|
||||
var display = string.IsNullOrWhiteSpace(domainName) ? EmptyDomainLabel : WebUtility.HtmlEncode(domainName.Trim());
|
||||
return rawHtml.Replace(DomainPlaceholder, display, StringComparison.Ordinal);
|
||||
}
|
||||
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
using System.Net;
|
||||
using AutopilotMonitor.Shared;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Services;
|
||||
@@ -26,7 +27,7 @@ public static class EmailTemplates
|
||||
/// </summary>
|
||||
public static string GetOffboardingFarewellHtml(string domainName)
|
||||
{
|
||||
var displayDomain = string.IsNullOrWhiteSpace(domainName) ? "your organization" : domainName;
|
||||
var displayDomain = string.IsNullOrWhiteSpace(domainName) ? "your organization" : WebUtility.HtmlEncode(domainName);
|
||||
|
||||
return $@"
|
||||
<!DOCTYPE html>
|
||||
@@ -125,7 +126,7 @@ public static class EmailTemplates
|
||||
/// </summary>
|
||||
public static string GetPreviewApprovedHtml(string domainName)
|
||||
{
|
||||
var displayDomain = string.IsNullOrWhiteSpace(domainName) ? "your organization" : domainName;
|
||||
var displayDomain = string.IsNullOrWhiteSpace(domainName) ? "your organization" : WebUtility.HtmlEncode(domainName);
|
||||
|
||||
return $@"
|
||||
<!DOCTYPE html>
|
||||
|
||||
@@ -6,6 +6,7 @@ using System.Linq;
|
||||
using System.Text;
|
||||
using System.Threading.Tasks;
|
||||
using Azure.Storage.Blobs.Models;
|
||||
using AutopilotMonitor.Functions.Security;
|
||||
using AutopilotMonitor.Functions.Services.Diagnostics;
|
||||
using AutopilotMonitor.Shared.DataAccess;
|
||||
using AutopilotMonitor.Shared.Models;
|
||||
@@ -60,6 +61,11 @@ namespace AutopilotMonitor.Functions.Services
|
||||
SubmitSessionReportRequest request,
|
||||
string submittedBy)
|
||||
{
|
||||
// Both identifiers shape blob names below; a non-GUID value could smuggle path
|
||||
// separators / dot segments into the storage URI (cross-container write).
|
||||
SecurityValidator.EnsureValidGuid(request.TenantId, nameof(request.TenantId));
|
||||
SecurityValidator.EnsureValidGuid(request.SessionId, nameof(request.SessionId));
|
||||
|
||||
var reportId = Guid.NewGuid().ToString("N")[..12];
|
||||
var timestamp = DateTime.UtcNow.ToString("yyyyMMdd_HHmmss");
|
||||
var blobName = $"{request.TenantId}_{request.SessionId}_diag_request_{timestamp}.zip";
|
||||
@@ -203,7 +209,7 @@ namespace AutopilotMonitor.Functions.Services
|
||||
zipStream.Position = 0;
|
||||
var containerClient = _blobStorage.GetContainerClient(ContainerName);
|
||||
await containerClient.CreateIfNotExistsAsync();
|
||||
var blobClient = containerClient.GetBlobClient(blobName);
|
||||
var blobClient = containerClient.GetBlobClient(BlobNameGuard.EnsureFlat(blobName, nameof(blobName)));
|
||||
await blobClient.UploadAsync(zipStream, new BlobUploadOptions
|
||||
{
|
||||
HttpHeaders = new BlobHttpHeaders { ContentType = "application/zip" }
|
||||
@@ -245,6 +251,8 @@ namespace AutopilotMonitor.Functions.Services
|
||||
SubmitDiagFilesReportRequest request,
|
||||
string submittedBy)
|
||||
{
|
||||
SecurityValidator.EnsureValidGuid(request.TenantId, nameof(request.TenantId));
|
||||
|
||||
var reportId = Guid.NewGuid().ToString("N")[..12];
|
||||
var timestamp = DateTime.UtcNow.ToString("yyyyMMdd_HHmmss");
|
||||
var blobName = $"{request.TenantId}_diag_files_{timestamp}.zip";
|
||||
@@ -292,7 +300,7 @@ namespace AutopilotMonitor.Functions.Services
|
||||
zipStream.Position = 0;
|
||||
var containerClient = _blobStorage.GetContainerClient(ContainerName);
|
||||
await containerClient.CreateIfNotExistsAsync();
|
||||
var blobClient = containerClient.GetBlobClient(blobName);
|
||||
var blobClient = containerClient.GetBlobClient(BlobNameGuard.EnsureFlat(blobName, nameof(blobName)));
|
||||
await blobClient.UploadAsync(zipStream, new BlobUploadOptions
|
||||
{
|
||||
HttpHeaders = new BlobHttpHeaders { ContentType = "application/zip" }
|
||||
|
||||
@@ -1008,25 +1008,32 @@ namespace AutopilotMonitor.Shared.Models
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Gets manufacturer whitelist as array
|
||||
/// Gets manufacturer whitelist as array (empty/whitespace = allow all)
|
||||
/// </summary>
|
||||
public string[] GetManufacturerWhitelist()
|
||||
{
|
||||
if (string.IsNullOrEmpty(ManufacturerWhitelist))
|
||||
return new[] { "*" };
|
||||
|
||||
return ManufacturerWhitelist.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
|
||||
}
|
||||
public string[] GetManufacturerWhitelist() => ParseWhitelist(ManufacturerWhitelist);
|
||||
|
||||
/// <summary>
|
||||
/// Gets model whitelist as array
|
||||
/// Gets model whitelist as array (empty/whitespace = allow all)
|
||||
/// </summary>
|
||||
public string[] GetModelWhitelist()
|
||||
public string[] GetModelWhitelist() => ParseWhitelist(ModelWhitelist);
|
||||
|
||||
/// <summary>
|
||||
/// Splits a stored hardware whitelist CSV into individual match patterns: trimmed, empty
|
||||
/// entries dropped. The CSV is the ONLY storage format, so a ',' can never be part of a
|
||||
/// pattern — writers (web editor, <c>TenantConfigValidation</c>) must guarantee each
|
||||
/// intended entry lands as exactly one CSV item.
|
||||
/// </summary>
|
||||
public static string[] ParseWhitelist(string? csv)
|
||||
{
|
||||
if (string.IsNullOrEmpty(ModelWhitelist))
|
||||
if (string.IsNullOrWhiteSpace(csv))
|
||||
return new[] { "*" };
|
||||
|
||||
return ModelWhitelist.Split(new[] { ',' }, StringSplitOptions.RemoveEmptyEntries);
|
||||
var entries = csv!.Split(',')
|
||||
.Select(e => e.Trim())
|
||||
.Where(e => e.Length > 0)
|
||||
.ToArray();
|
||||
|
||||
return entries.Length == 0 ? new[] { "*" } : entries;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
+11
-15
@@ -3,6 +3,7 @@
|
||||
import { useState } from "react";
|
||||
import SaveResetBar from "./SaveResetBar";
|
||||
import ReadOnlyFieldset from "./ReadOnlyFieldset";
|
||||
import { joinList, parseList } from "../lib/hardwareWhitelist";
|
||||
|
||||
interface HardwareWhitelistSectionProps {
|
||||
manufacturerWhitelist: string;
|
||||
@@ -16,14 +17,6 @@ interface HardwareWhitelistSectionProps {
|
||||
readOnly?: boolean;
|
||||
}
|
||||
|
||||
function parseList(csv: string): string[] {
|
||||
return csv.split(",").map((s) => s.trim()).filter(Boolean);
|
||||
}
|
||||
|
||||
function joinList(items: string[]): string {
|
||||
return items.join(",");
|
||||
}
|
||||
|
||||
function WhitelistEditor({
|
||||
label,
|
||||
description,
|
||||
@@ -38,14 +31,17 @@ function WhitelistEditor({
|
||||
setItems: (items: string[]) => void;
|
||||
}) {
|
||||
const [newItem, setNewItem] = useState("");
|
||||
const trimmed = newItem.trim();
|
||||
const isDuplicate =
|
||||
trimmed !== "" &&
|
||||
items.some((a) => a.toLowerCase() === trimmed.toLowerCase());
|
||||
// Admin-typed input is a LIST: "Dell*, *" adds two patterns (the placeholder invites
|
||||
// exactly that). Only the insights buttons neutralize ',' — their values are untrusted.
|
||||
const existing = new Set(items.map((a) => a.toLowerCase()));
|
||||
const typed = parseList(newItem);
|
||||
const fresh = typed.filter((t, i) =>
|
||||
!existing.has(t.toLowerCase()) && typed.findIndex((u) => u.toLowerCase() === t.toLowerCase()) === i);
|
||||
const isDuplicate = typed.length > 0 && fresh.length === 0;
|
||||
|
||||
const addItem = () => {
|
||||
if (!trimmed || isDuplicate) return;
|
||||
setItems([...items, trimmed]);
|
||||
if (fresh.length === 0) return;
|
||||
setItems([...items, ...fresh]);
|
||||
setNewItem("");
|
||||
};
|
||||
|
||||
@@ -110,7 +106,7 @@ function WhitelistEditor({
|
||||
/>
|
||||
<button
|
||||
onClick={addItem}
|
||||
disabled={!trimmed || isDuplicate}
|
||||
disabled={fresh.length === 0}
|
||||
className="px-4 py-1.5 bg-green-600 text-white rounded-lg text-sm font-medium hover:bg-green-700 disabled:opacity-50 disabled:cursor-not-allowed transition-colors whitespace-nowrap"
|
||||
>
|
||||
Add
|
||||
|
||||
@@ -0,0 +1,30 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { addWhitelistEntry, parseList, toWhitelistEntry } from "../hardwareWhitelist";
|
||||
|
||||
describe("hardware whitelist CSV helpers", () => {
|
||||
it("adds a plain value as one entry", () => {
|
||||
expect(addWhitelistEntry("Dell*,HP*", "Lenovo")).toBe("Dell*,HP*,Lenovo");
|
||||
});
|
||||
|
||||
it("never lets a delimiter-bearing value expand into extra patterns", () => {
|
||||
// Attacker-controlled distress-signal string: would become [..., "Dell Inc.", "*"] verbatim.
|
||||
const csv = addWhitelistEntry("Dell*", "Dell Inc.,*");
|
||||
expect(parseList(csv)).toEqual(["Dell*", "Dell Inc.?*"]);
|
||||
expect(parseList(csv)).not.toContain("*");
|
||||
});
|
||||
|
||||
it("neutralizes a trailing comma instead of creating an empty/stray entry", () => {
|
||||
expect(parseList(addWhitelistEntry("Dell*", "Dell Inc.,"))).toEqual(["Dell*", "Dell Inc.?"]);
|
||||
});
|
||||
|
||||
it("ignores empty and duplicate values", () => {
|
||||
expect(addWhitelistEntry("Dell*", " ")).toBe("Dell*");
|
||||
// A lone delimiter becomes the single-character pattern "?" — one entry, never a split.
|
||||
expect(parseList(addWhitelistEntry("Dell*", ","))).toEqual(["Dell*", "?"]);
|
||||
expect(addWhitelistEntry("Dell*", "dell*")).toBe("Dell*");
|
||||
});
|
||||
|
||||
it("toWhitelistEntry trims and replaces every comma", () => {
|
||||
expect(toWhitelistEntry(" a,b,,c ")).toBe("a?b??c");
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,37 @@
|
||||
/**
|
||||
* Hardware whitelist CSV helpers.
|
||||
*
|
||||
* The tenant whitelist is persisted as ONE comma-separated string and split on ',' at
|
||||
* enforcement time (TenantConfiguration.GetManufacturerWhitelist). A comma inside a single
|
||||
* entry therefore changes the LIST STRUCTURE: "Dell Inc.,*" appended as one "manufacturer"
|
||||
* becomes two patterns, the second being the allow-all wildcard. Manufacturer/model strings
|
||||
* shown in the rejection insights come from UNAUTHENTICATED distress signals, so every value
|
||||
* that enters the list must round-trip as exactly one pattern.
|
||||
*/
|
||||
|
||||
export function parseList(csv: string): string[] {
|
||||
return csv.split(",").map((s) => s.trim()).filter(Boolean);
|
||||
}
|
||||
|
||||
export function joinList(items: string[]): string {
|
||||
return items.join(",");
|
||||
}
|
||||
|
||||
/**
|
||||
* Normalizes a value so it is stored as exactly one whitelist pattern. The delimiter is
|
||||
* replaced by the single-character wildcard '?', which still matches the original string
|
||||
* literally (and nothing structurally different) instead of splitting the list.
|
||||
* Returns "" when nothing usable remains.
|
||||
*/
|
||||
export function toWhitelistEntry(value: string): string {
|
||||
return value.replace(/,/g, "?").trim();
|
||||
}
|
||||
|
||||
/** Appends `value` as one pattern unless an equal (case-insensitive) entry already exists. */
|
||||
export function addWhitelistEntry(csv: string, value: string): string {
|
||||
const entry = toWhitelistEntry(value);
|
||||
if (!entry) return csv;
|
||||
const items = parseList(csv);
|
||||
if (items.some((i) => i.toLowerCase() === entry.toLowerCase())) return csv;
|
||||
return joinList([...items, entry]);
|
||||
}
|
||||
+5
-16
@@ -7,14 +7,7 @@ import { TenantNotifications } from "../../TenantNotifications";
|
||||
import HardwareWhitelistSection from "../../components/HardwareWhitelistSection";
|
||||
import HardwareRejectionInsights from "../../components/HardwareRejectionInsights";
|
||||
import TpmIncompatibleDevicesInsights from "../../components/TpmIncompatibleDevicesInsights";
|
||||
|
||||
function parseList(csv: string): string[] {
|
||||
return csv.split(",").map((s) => s.trim()).filter(Boolean);
|
||||
}
|
||||
|
||||
function joinList(items: string[]): string {
|
||||
return items.join(",");
|
||||
}
|
||||
import { addWhitelistEntry } from "../../lib/hardwareWhitelist";
|
||||
|
||||
export function SectionHardwareWhitelist() {
|
||||
const { getAccessToken } = useAuth();
|
||||
@@ -30,18 +23,14 @@ export function SectionHardwareWhitelist() {
|
||||
|
||||
const hasWebhook = notificationChannels.some((c) => c.enabled && (c.url ?? "").length > 0);
|
||||
|
||||
// Values come from UNAUTHENTICATED distress signals — addWhitelistEntry guarantees each
|
||||
// click adds exactly one pattern (a ',' inside the value cannot split the list).
|
||||
const handleAddManufacturer = useCallback((value: string) => {
|
||||
const items = parseList(manufacturerWhitelist);
|
||||
if (!items.some((i) => i.toLowerCase() === value.toLowerCase())) {
|
||||
setManufacturerWhitelist(joinList([...items, value]));
|
||||
}
|
||||
setManufacturerWhitelist(addWhitelistEntry(manufacturerWhitelist, value));
|
||||
}, [manufacturerWhitelist, setManufacturerWhitelist]);
|
||||
|
||||
const handleAddModel = useCallback((value: string) => {
|
||||
const items = parseList(modelWhitelist);
|
||||
if (!items.some((i) => i.toLowerCase() === value.toLowerCase())) {
|
||||
setModelWhitelist(joinList([...items, value]));
|
||||
}
|
||||
setModelWhitelist(addWhitelistEntry(modelWhitelist, value));
|
||||
}, [modelWhitelist, setModelWhitelist]);
|
||||
|
||||
return (
|
||||
|
||||
Reference in New Issue
Block a user