Compare commits

...

7 Commits

Author SHA1 Message Date
Oliver Kieselbach c41c2327ab fix(security): validate enrollment summary branding image URL (https-only, no quotes); launcher drops unsafe URLs instead of interpolating them into the dialog command line
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 23:15:07 +02:00
Oliver Kieselbach 83effea592 fix(security): hosted diagnostics upload SAS is create-only and bound to the requesting session; existing blob => 409
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 23:13:00 +02:00
Oliver Kieselbach 1a4a37b77a fix(web): hardware whitelist editor splits typed comma lists into entries ("Dell*, *" stays allow-all); only insights buttons neutralize ','
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 22:56:37 +02:00
Oliver Kieselbach f56851fc9f fix(security): hardware whitelist — one added entry is always one pattern; server rejects malformed CSV
The tenant hardware whitelist is stored as a CSV and split on ',' at
enforcement time. Manufacturer/model strings in the Rejected-Hardware
insights come from unauthenticated distress signals, so a value like
"Dell Inc.,*" added via "+ Mfr" appended a standalone "*" and silently
turned the manufacturer gate into allow-all.

- Web: shared hardwareWhitelist helper; "+ Mfr"/"+ Model" and the manual
  editor neutralize ',' to the single-char wildcard '?' so a value can
  never split the list.
- Backend: TenantConfigValidation.ValidateHardwareWhitelist in ValidateModel
  (PUT + patch) rejects blank items, control chars, over-long/too many entries.
- Shared: ParseWhitelist trims entries so enforcement matches the editor view.
- Tests: 8 xUnit + 5 vitest.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 22:51:55 +02:00
Oliver Kieselbach cbe410cbbc fix(security): reject path-shaped sessionId/tenantId in session-report blob names; BlobNameGuard at every GetBlobClient sink
SubmitSessionReport built the report blob name from the raw route sessionId. Route values are
percent-decoded, so an encoded '/'+'..' reached Azure.Storage.Blobs, which keeps separators, and
System.Uri collapsed the dot segments into a write outside the session-reports container.

- SubmitSessionReport / SubmitDiagFilesReport: 400 unless sessionId/tenantId are GUIDs
- SessionReportService: EnsureValidGuid at entry of both submit paths
- New Security/BlobNameGuard (flat-name invariant) applied in SessionReportService,
  SessionReportDiagnosticsArchiveCopier (pre-check + upload) and the GA download function
- Tests: BlobNameGuardTests, copier rejects non-flat destination without storage calls

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 22:51:13 +02:00
Oliver Kieselbach 43ddc60111 fix(security): enforce tenant suspension in PolicyEnforcementMiddleware, not only auth/me
TenantConfiguration.Disabled/DisabledUntil was checked solely in AuthFunction.BuildAuthResult
(GET /api/auth/me), so a direct REST caller with a valid token kept full read/write access for
the whole suspension window and during an offboarding wipe. DecideAsync now denies 403
TenantSuspended when the caller's home tenant or the request's target tenant is currently
disabled (cached, side-effect-free config read). GA/GlobalReader bypass to administer the
tenant; device/anonymous routes keep their SecurityValidator gate.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 22:34:29 +02:00
Oliver Kieselbach 82b26b4ab9 fix(security): HTML-encode domain in transactional mails; freeze DomainName on config PUT, validate first-login seed
Welcome/farewell mails rendered DomainName unencoded while the full-model
config PUT persisted it verbatim, letting a tenant admin ship attacker HTML
in a signed product mail to an arbitrary recipient. Encode at Render() and
the built-in builders, preserve DomainName from the stored row on PUT for
every caller, and only seed a strict DNS host name at first login.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
2026-08-29 22:33:52 +02:00
33 changed files with 1132 additions and 98 deletions
@@ -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
+4
View File
@@ -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.
@@ -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("&lt;a href=&quot;https://evil.invalid&quot;&gt;x&lt;/a&gt;", 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>", "&lt;b&gt;x&lt;/b&gt;")]
[InlineData("a\"b&c", "a&quot;b&amp;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("&lt;img src=x onerror=alert(1)&gt;&amp;&quot;", html);
Assert.DoesNotContain("&amp;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);
}
}
@@ -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))));
}
}
@@ -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).
@@ -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;
@@ -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." });
@@ -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)
@@ -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!;
}
}
}
@@ -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>
@@ -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>
@@ -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]);
}
@@ -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 (