From f311e8746bd398ae4846b235de396f46ee69b670 Mon Sep 17 00:00:00 2001 From: Oliver Kieselbach Date: Sat, 25 Jul 2026 12:43:42 +0200 Subject: [PATCH] docs(trust): on-demand log collection in customer-facing claims + tracker dedup tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Collect Logs" feature (b382e567) added a second opt-in path into hosted diagnostics upload — the quick-config dialog on a session — and a new collection trigger, neither of which the customer-facing pages described. The trust FAQ still said hosted upload is enabled "in the settings UI", and the dialog itself named hosted storage without stating that packages leave the tenant boundary, so the "clearly marked disclosure" promise was not met on that path. - CollectLogsButton: marked data-boundary disclosure in the quick-config dialog (leaves your Azure tenant, per-tenant isolation, blob-scoped write-only token, removal per Data Retention Days) - privacy + about pages: both opt-in paths named; on-demand collection described (Admin/Operator of the own tenant, same content as the automatic package, delivered with the agent's next check-in, visible on the session timeline) - docs repo counterpart in trust/security-faq.md (separate commit) Distress-bell dedup: the TpmPssUnsupported one-shot bell had key-shape tests only. New HardwareRejectionNotificationTrackerDedupTests pins the 409 dedup branch, the fail-soft branch, the blank-input short-circuit, the disjoint hardware/tpmpss key spaces, and the retention sweep — which also prunes tpmpss rows and thereby re-arms that device's bell, now stated in the interface and implementation docs. Backend tests 3313/3313, tsc clean, web vitest app/sessions 60/60. Co-Authored-By: Claude Opus 5 (1M context) --- ...eRejectionNotificationTrackerDedupTests.cs | 304 ++++++++++++++++++ ...bleHardwareRejectionNotificationTracker.cs | 8 +- .../IHardwareRejectionNotificationTracker.cs | 6 +- .../autopilot-monitor-web/app/about/page.tsx | 4 +- .../app/privacy/page.tsx | 13 +- .../components/CollectLogsButton.tsx | 16 +- 6 files changed, 342 insertions(+), 9 deletions(-) create mode 100644 src/Backend/AutopilotMonitor.Functions.Tests/HardwareRejectionNotificationTrackerDedupTests.cs diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/HardwareRejectionNotificationTrackerDedupTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/HardwareRejectionNotificationTrackerDedupTests.cs new file mode 100644 index 00000000..51ec2023 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/HardwareRejectionNotificationTrackerDedupTests.cs @@ -0,0 +1,304 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text.RegularExpressions; +using System.Threading; +using System.Threading.Tasks; +using AutopilotMonitor.Functions.DataAccess.TableStorage; +using AutopilotMonitor.Functions.Services; +using Azure; +using Azure.Data.Tables; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Behavioural coverage for — the dedup +/// store behind the two distress-driven bell notifications (hardware rejection per model, +/// TPM-PSS incompatibility per serial). +/// pins the RowKey shape; this file pins what the tracker DOES with those keys. +/// +/// CORRECTNESS GUARD: the "one bell ever" promise rests entirely on the 409 branch of +/// AddEntityAsync — a rewrite to Upsert or a get-then-add would silently start ringing +/// the bell on every distress report (a device with an incompatible TPM retries forever, so +/// "every report" means a notification storm, not a second bell). +/// +/// The retention sweep is covered here too because DeleteOlderThanAsync prunes BOTH key +/// spaces from the shared table: pruning a tpmpss| row re-arms that device's bell. +/// +public class HardwareRejectionNotificationTrackerDedupTests +{ + private const string TenantId = "77777777-7777-7777-7777-777777777777"; + private const string OtherTenantId = "88888888-8888-8888-8888-888888888888"; + private const string Serial = "S4SQ8685"; + + // ── TPM PSS: first bell, then never again ────────────────────────────────────── + + [Fact] + public async Task TpmPss_FirstReport_RegistersRowAndReturnsTrue() + { + var harness = new Harness(); + + var isFirst = await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial); + + Assert.True(isFirst); + var entity = Assert.Single(harness.Store.Values); + Assert.Equal(TenantId.ToLowerInvariant(), entity.PartitionKey); + Assert.Equal("tpmpss|s4sq8685", entity.RowKey); + // Serialization guard: the row must carry the fields the cleanup sweep and any future + // operator query rely on — losing FirstNotifiedAt would make the row immortal. + Assert.Equal(TenantId, entity.GetString("TenantId")); + Assert.Equal(Serial, entity.GetString("SerialNumber")); + Assert.NotNull(entity.GetDateTime("FirstNotifiedAt")); + } + + [Fact] + public async Task TpmPss_SecondReport_SameSerial_ReturnsFalse() + { + var harness = new Harness(); + + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.False(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.Single(harness.Store); + } + + [Theory] + [InlineData("s4sq8685")] + [InlineData(" S4SQ8685 ")] + [InlineData("S4Sq8685")] + public async Task TpmPss_SecondReport_CasingOrWhitespaceVariant_StillDeduped(string variant) + { + var harness = new Harness(); + + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.False(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, variant)); + } + + [Fact] + public async Task TpmPss_DifferentSerial_OrDifferentTenant_RingsAgain() + { + var harness = new Harness(); + + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, "OTHER123")); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(OtherTenantId, Serial)); + Assert.Equal(3, harness.Store.Count); + } + + [Theory] + [InlineData("", Serial)] + [InlineData(" ", Serial)] + [InlineData(TenantId, "")] + [InlineData(TenantId, " ")] + public async Task TpmPss_BlankInput_ReturnsFalse_WithoutTouchingTheTable(string tenantId, string serial) + { + var harness = new Harness(); + + Assert.False(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(tenantId, serial)); + Assert.Empty(harness.Store); + harness.Table.Verify( + c => c.AddEntityAsync(It.IsAny(), It.IsAny()), + Times.Never); + } + + [Fact] + public async Task TpmPss_UnexpectedTableFailure_ReturnsFalse() + { + // Fail-soft in the SUPPRESSING direction: the write may or may not have landed, so the + // caller must not fire a bell it might fire again on the next report. + var harness = new Harness { AddFailure = new RequestFailedException(503, "ServerBusy") }; + + Assert.False(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + } + + // ── Hardware rejection: same contract, disjoint key space ────────────────────── + + [Fact] + public async Task HardwareRejection_SecondReport_SameModel_ReturnsFalse() + { + var harness = new Harness(); + + Assert.True(await harness.Sut.TryRegisterFirstNotificationAsync(TenantId, "Lenovo", "ThinkPad X1")); + Assert.False(await harness.Sut.TryRegisterFirstNotificationAsync(TenantId, "LENOVO", " thinkpad x1 ")); + Assert.Single(harness.Store); + } + + [Fact] + public async Task HardwareRejection_AndTpmPss_DoNotDedupEachOtherInTheSharedTable() + { + var harness = new Harness(); + + Assert.True(await harness.Sut.TryRegisterFirstNotificationAsync(TenantId, "Lenovo", Serial)); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.Equal(2, harness.Store.Count); + } + + // ── Retention sweep ─────────────────────────────────────────────────────────── + + [Fact] + public async Task DeleteOlderThanAsync_PrunesTpmPssRows_AndReArmsThatDevicesBell() + { + var harness = new Harness(); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + harness.AgeRows(TimeSpan.FromDays(31)); + + var deleted = await harness.Sut.DeleteOlderThanAsync(DateTime.UtcNow.AddDays(-30)); + + Assert.Equal(1, deleted); + Assert.Empty(harness.Store); + // A device whose TPM was never fixed keeps reporting; after the retention window it is + // worth one more bell rather than permanent silence. + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + } + + [Fact] + public async Task DeleteOlderThanAsync_KeepsRowsInsideTheWindow_DedupSurvives() + { + var harness = new Harness(); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.True(await harness.Sut.TryRegisterFirstNotificationAsync(TenantId, "Lenovo", "ThinkPad X1")); + + var deleted = await harness.Sut.DeleteOlderThanAsync(DateTime.UtcNow.AddDays(-30)); + + Assert.Equal(0, deleted); + Assert.Equal(2, harness.Store.Count); + Assert.False(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.False(await harness.Sut.TryRegisterFirstNotificationAsync(TenantId, "Lenovo", "ThinkPad X1")); + } + + [Fact] + public async Task DeleteOlderThanAsync_FiltersOnFirstNotifiedAt_NotTimestamp() + { + // Rows are insert-once, so FirstNotifiedAt is the creation time. Timestamp would look + // equivalent but is rewritten by a table restore/copy, which would silently re-arm every + // bell in the tenant at once. + var harness = new Harness(); + var cutoff = new DateTime(2026, 7, 25, 12, 0, 0, DateTimeKind.Utc); + + await harness.Sut.DeleteOlderThanAsync(cutoff); + + var filter = Assert.Single(harness.QueryFilters); + Assert.Equal("FirstNotifiedAt lt datetime'2026-07-25T12:00:00Z'", filter); + } + + [Fact] + public async Task DeleteOlderThanAsync_ContinuesAfterASingleRowFailure() + { + var harness = new Harness(); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, Serial)); + Assert.True(await harness.Sut.TryRegisterFirstTpmPssNotificationAsync(TenantId, "OTHER123")); + harness.AgeRows(TimeSpan.FromDays(31)); + harness.FailDeleteForRowKeys.Add("tpmpss|s4sq8685"); + + var deleted = await harness.Sut.DeleteOlderThanAsync(DateTime.UtcNow.AddDays(-30)); + + Assert.Equal(1, deleted); + Assert.Equal("tpmpss|s4sq8685", Assert.Single(harness.Store.Values).RowKey); + } + + [Fact] + public async Task DeleteOlderThanAsync_QueryFailure_ReturnsZero() + { + var harness = new Harness { QueryFailure = new RequestFailedException(500, "Boom") }; + + Assert.Equal(0, await harness.Sut.DeleteOlderThanAsync(DateTime.UtcNow.AddDays(-30))); + } + + // ── Harness ─────────────────────────────────────────────────────────────────── + + private sealed class Harness + { + private static readonly Regex CutoffFilter = + new(@"^FirstNotifiedAt lt datetime'(?[^']+)'$", RegexOptions.Compiled); + + public TableHardwareRejectionNotificationTracker Sut { get; } + public Mock Table { get; } + public Dictionary<(string Pk, string Rk), TableEntity> Store { get; } = new(); + public List QueryFilters { get; } = new(); + public HashSet FailDeleteForRowKeys { get; } = new(); + + /// Non-409 failure injected into AddEntityAsync (fail-soft path). + public Exception? AddFailure { get; init; } + + /// Failure injected into QueryAsync enumeration (sweep fail-soft path). + public Exception? QueryFailure { get; init; } + + public Harness() + { + Table = new Mock(); + + Table.Setup(c => c.AddEntityAsync(It.IsAny(), It.IsAny())) + .Returns((e, _) => + { + if (AddFailure != null) throw AddFailure; + if (Store.ContainsKey((e.PartitionKey, e.RowKey))) + throw new RequestFailedException(409, "Conflict", "EntityAlreadyExists", null); + Store[(e.PartitionKey, e.RowKey)] = e; + return Task.FromResult(new Mock().Object); + }); + + Table.Setup(c => c.QueryAsync( + It.IsAny(), + It.IsAny(), + It.IsAny>(), + It.IsAny())) + .Returns, CancellationToken>((filter, _, _, _) => + { + if (QueryFailure != null) throw QueryFailure; + QueryFilters.Add(filter); + var rows = Store.Values.Where(e => MatchesCutoff(filter, e)).ToList(); + return AsyncPageable.FromPages(new[] + { + Page.FromValues(rows, null, new Mock().Object), + }); + }); + + Table.Setup(c => c.DeleteEntityAsync( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny())) + .Returns((pk, rk, _, _) => + { + if (FailDeleteForRowKeys.Contains(rk)) + throw new RequestFailedException(500, "DeleteFailed"); + Store.Remove((pk, rk)); + return Task.FromResult(new Mock().Object); + }); + + var serviceClient = new Mock(); + serviceClient.Setup(s => s.GetTableClient(It.IsAny())).Returns(Table.Object); + + Sut = new TableHardwareRejectionNotificationTracker( + new TableStorageService(serviceClient.Object, NullLogger.Instance), + NullLogger.Instance); + } + + /// Backdates every stored row so a retention sweep sees it as expired. + public void AgeRows(TimeSpan age) + { + foreach (var entity in Store.Values) + entity["FirstNotifiedAt"] = DateTime.UtcNow - age; + } + + /// + /// Minimal server-side emulation of the OData filter the tracker builds. An unrecognised + /// filter matches nothing, so a rewrite that no longer selects expired rows fails loudly + /// here instead of leaving dead rows in production. + /// + private static bool MatchesCutoff(string filter, TableEntity entity) + { + var match = CutoffFilter.Match(filter ?? string.Empty); + if (!match.Success) return false; + + var cutoff = DateTime.Parse( + match.Groups["cutoff"].Value, + null, + System.Globalization.DateTimeStyles.AdjustToUniversal | System.Globalization.DateTimeStyles.AssumeUniversal); + var firstNotified = entity.GetDateTime("FirstNotifiedAt"); + return firstNotified.HasValue && firstNotified.Value.ToUniversalTime() < cutoff; + } + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs index 643e9d2a..56b84174 100644 --- a/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs +++ b/src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableHardwareRejectionNotificationTracker.cs @@ -114,9 +114,11 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage try { // Rows are insert-once (AddEntityAsync, never updated), so FirstNotifiedAt == creation time. - // Pruning here resets the lifetime dedup for a given model: if the same model is rejected - // again after the cutoff, the bell fires once more — acceptable, since the portal only - // surfaces recent rejections anyway. + // Pruning here resets the lifetime dedup for BOTH key spaces in this table: a hardware + // model rejected again after the cutoff rings once more, and so does a TPM-PSS device + // that reports again ("tpmpss|{serial}" rows are pruned by the same sweep). Acceptable + // in both cases — the portal only surfaces recent rejections anyway, and a device whose + // TPM was never fixed is worth surfacing again after a month of silence. var filter = $"FirstNotifiedAt lt datetime'{cutoffUtc:yyyy-MM-ddTHH:mm:ss}Z'"; var query = _table.QueryAsync(filter: filter, select: new[] { "PartitionKey", "RowKey" }); diff --git a/src/Shared/AutopilotMonitor.Shared/DataAccess/IHardwareRejectionNotificationTracker.cs b/src/Shared/AutopilotMonitor.Shared/DataAccess/IHardwareRejectionNotificationTracker.cs index 104b6a13..2b476036 100644 --- a/src/Shared/AutopilotMonitor.Shared/DataAccess/IHardwareRejectionNotificationTracker.cs +++ b/src/Shared/AutopilotMonitor.Shared/DataAccess/IHardwareRejectionNotificationTracker.cs @@ -30,7 +30,11 @@ namespace AutopilotMonitor.Shared.DataAccess /// /// Retention cleanup: deletes tracker rows whose FirstNotifiedAt is older than - /// . Pruning re-arms the dedup for that model. Returns rows deleted. + /// . Both key spaces live in one table, so pruning re-arms the + /// dedup for BOTH subjects: a hardware model rejected again after the cutoff rings once more, + /// and so does a TPM-PSS-incompatible device that reports again. That is intended — a bell + /// that can never ring a second time would go silent on a device the tenant never fixed. + /// Returns rows deleted. /// Task DeleteOlderThanAsync(DateTime cutoffUtc); } diff --git a/src/Web/autopilot-monitor-web/app/about/page.tsx b/src/Web/autopilot-monitor-web/app/about/page.tsx index a79fc7f7..d13b94fc 100644 --- a/src/Web/autopilot-monitor-web/app/about/page.tsx +++ b/src/Web/autopilot-monitor-web/app/about/page.tsx @@ -27,9 +27,9 @@ const FEATURES = [ { title: "Diagnostics Collection", description: - "Collect agent logs, IME logs, agent state, and device information as a ZIP bundle at the end of an enrollment — then download it from the session view without touching the device.", + "Collect agent logs, IME logs, agent state, and device information as a ZIP bundle — automatically at the end of an enrollment, or on demand while one is still running — then download it from the session view without touching the device.", color: "green", - bullets: ["Configurable upload: off, always, or on failure", "Agent, IME & device information bundle", "Configurable additional log paths"], + bullets: ["Configurable upload: off, always, or on failure", "On-demand collection from a running enrollment", "Agent, IME & device information bundle", "Configurable additional log paths"], }, { title: "Detailed Event Timeline", diff --git a/src/Web/autopilot-monitor-web/app/privacy/page.tsx b/src/Web/autopilot-monitor-web/app/privacy/page.tsx index 1cbf85f1..b5a70b42 100644 --- a/src/Web/autopilot-monitor-web/app/privacy/page.tsx +++ b/src/Web/autopilot-monitor-web/app/privacy/page.tsx @@ -148,8 +148,17 @@ export default function PrivacyPage() {

Diagnostics upload is off by default. When enabled, the default destination is{" "} your own Azure storage account — the package never reaches our infrastructure. Hosted upload - exists as an alternative but is opt-in only and requires an explicit administrator action behind a clearly - marked disclosure that data leaves your tenant. It is never enabled silently. + exists as an alternative but is opt-in only and requires an explicit administrator action — in Settings, or + in the one-step dialog offered on a session — always behind a clearly marked disclosure that data leaves your + tenant. It is never enabled silently. +

+

+ Once upload is configured, an Admin or Operator in your own tenant can also request a + package on demand from an enrollment that is still running, instead of waiting for it to finish. It collects + exactly what the automatic package collects — the configured paths, nothing wider — the device only acts on + the request when the agent next checks in, and both the request and its outcome appear as events on that + session's timeline. Viewers and Progress-Portal members cannot trigger it, and it does nothing on a + tenant that has no upload destination configured.

How Long We Keep Data

diff --git a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/components/CollectLogsButton.tsx b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/components/CollectLogsButton.tsx index a416f294..01da96b4 100644 --- a/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/components/CollectLogsButton.tsx +++ b/src/Web/autopilot-monitor-web/app/sessions/[sessionId]/components/CollectLogsButton.tsx @@ -270,9 +270,23 @@ export default function CollectLogsButton({ {" "}hosted storage (managed by AutopilotMonitor) with mode {" "}On failure, then collects the logs from this device right away.

+ {/* Data-boundary disclosure. This dialog is the SECOND opt-in path into hosted + upload (the first is Settings → Diagnostics Package), and the privacy page and + trust FAQ both promise that hosted upload is only ever enabled behind a clearly + marked "data leaves your tenant" disclosure — this block is that promise here. */} +
+

+ Uploaded packages leave your own Azure tenant boundary.{" "} + They are stored in the AutopilotMonitor backend's Azure Storage (Germany West Central), + isolated per tenant, and each upload uses a fresh blob-scoped, write-only token. +

+

+ Packages are removed automatically according to your Data Retention Days setting. +

+

You can change the destination (e.g. to your own Azure Blob Storage) or turn uploads off again at any - time under Settings → Diagnostics Package. + time under Settings → Diagnostics Package, which also lists exactly which paths are collected.