hardening(auth): validate + audit EntraId:AdditionalClientIds trust-set expansion

Expanding the token audience trust set via EntraId:AdditionalClientIds was
completely silent: no format validation, no operator-visible logging. During
the tenant-move window a typo'd GUID would be indistinguishable from "the new
app registration doesn't work" (daily-review follow-up, 2026-07-14).

- ResolveConfiguredClientIds: additional entries must parse as GUIDs; malformed
  entries are dropped (fail-closed - they could never match a token aud) and
  reported via out param. Accepted entries are normalized to the canonical
  lowercase dashed GUID form so brace/uppercase paste variants still match the
  ordinal audience comparison. Primary ClientId handling stays verbatim.
- AuditClientIdTrustSet: one Warning per distinct config value per instance for
  (a) dropped malformed entries (truncated - never log a full pasted value in
  case it is a secret) and (b) trust-set expansion with the accepted ID list.
  Warning level because nothing below Warning reaches App Insights.
- Tests: existing ResolveConfiguredClientIds tests moved to GUID values
  (signature change, no compat alias) + rejection, primary-verbatim,
  normalization and TruncateForLog coverage.

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

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

@@ -229,34 +229,92 @@ public class AuthenticationMiddlewareTokenValidationTests
     }
 
     // ── ResolveConfiguredClientIds: primary + optional AdditionalClientIds parsing ────────
+    // Additional entries must be GUIDs (client IDs always are); the primary stays verbatim.
+
+    private const string SecondId = "11111111-2222-3333-4444-555555555555";
+    private const string ThirdId = "66666666-7777-8888-9999-aaaaaaaaaaaa";
+    private const string FourthId = "bbbbbbbb-cccc-dddd-eeee-ffffffffffff";
 
     [Theory]
     [InlineData("primary", null, new[] { "primary" })]                                   // unset ⇒ today's behaviour
     [InlineData("primary", "", new[] { "primary" })]
     [InlineData("primary", "   ", new[] { "primary" })]
-    [InlineData("primary", "second", new[] { "primary", "second" })]
-    [InlineData("primary", "second,third", new[] { "primary", "second", "third" })]
-    [InlineData("primary", "second; third ,  fourth", new[] { "primary", "second", "third", "fourth" })]
-    [InlineData("primary", "primary,second", new[] { "primary", "second" })]             // dedupe vs primary
+    [InlineData("primary", SecondId, new[] { "primary", SecondId })]
+    [InlineData("primary", SecondId + "," + ThirdId, new[] { "primary", SecondId, ThirdId })]
+    [InlineData("primary", SecondId + "; " + ThirdId + " ,  " + FourthId, new[] { "primary", SecondId, ThirdId, FourthId })]
+    [InlineData(SecondId, SecondId + "," + ThirdId, new[] { SecondId, ThirdId })]        // dedupe vs primary
     public void ResolveConfiguredClientIds_parses_trims_and_dedupes(
         string primary, string? additional, string[] expected)
     {
-        Assert.Equal(expected, AuthenticationMiddleware.ResolveConfiguredClientIds(primary, additional));
+        Assert.Equal(expected, AuthenticationMiddleware.ResolveConfiguredClientIds(primary, additional, out var rejected));
+        Assert.Empty(rejected);
     }
 
     [Fact]
-    public void ResolveConfiguredClientIds_dedupes_case_insensitively()
+    public void ResolveConfiguredClientIds_dedupes_case_insensitively_primary_form_wins()
     {
+        // Uppercase primary (verbatim) vs the same id lowercase in AdditionalClientIds:
+        // one entry survives, in the primary's original casing.
+        var primaryUpper = SecondId.ToUpperInvariant();
+
         Assert.Equal(
-            new[] { "Primary", "second" },
-            AuthenticationMiddleware.ResolveConfiguredClientIds("Primary", "primary, second"));
+            new[] { primaryUpper, ThirdId },
+            AuthenticationMiddleware.ResolveConfiguredClientIds(primaryUpper, $"{SecondId}, {ThirdId}", out var rejected));
+        Assert.Empty(rejected);
     }
 
     [Fact]
     public void ResolveConfiguredClientIds_null_primary_falls_back_to_additional_only()
     {
         Assert.Equal(
-            new[] { "second" },
-            AuthenticationMiddleware.ResolveConfiguredClientIds(null, "second"));
+            new[] { SecondId },
+            AuthenticationMiddleware.ResolveConfiguredClientIds(null, SecondId, out var rejected));
+        Assert.Empty(rejected);
+    }
+
+    [Fact]
+    public void ResolveConfiguredClientIds_drops_and_reports_malformed_additional_entries()
+    {
+        // A typo'd/garbage entry could never match a token's aud claim — it is dropped
+        // (fail-closed) and surfaced via the out param so the middleware can log it.
+        var ids = AuthenticationMiddleware.ResolveConfiguredClientIds(
+            "primary", $"not-a-guid,{SecondId};11111111-2222", out var rejected);
+
+        Assert.Equal(new[] { "primary", SecondId }, ids);
+        Assert.Equal(new[] { "not-a-guid", "11111111-2222" }, rejected);
+    }
+
+    [Fact]
+    public void ResolveConfiguredClientIds_malformed_primary_is_kept_verbatim_not_rejected()
+    {
+        // The primary's handling is deliberately unchanged — validation covers ONLY the
+        // AdditionalClientIds seam. A broken primary breaks every login loudly on its own.
+        var ids = AuthenticationMiddleware.ResolveConfiguredClientIds("oops", SecondId, out var rejected);
+
+        Assert.Equal(new[] { "oops", SecondId }, ids);
+        Assert.Empty(rejected);
+    }
+
+    [Fact]
+    public void ResolveConfiguredClientIds_normalizes_additional_entries_to_canonical_guid_form()
+    {
+        // Entra aud claims are lowercase dashed GUIDs and audience comparison is ordinal —
+        // brace/uppercase variants an operator might paste must still match after resolve.
+        var ids = AuthenticationMiddleware.ResolveConfiguredClientIds(
+            "primary", $"{{{SecondId}}};{ThirdId.ToUpperInvariant()}", out var rejected);
+
+        Assert.Equal(new[] { "primary", SecondId, ThirdId }, ids);
+        Assert.Empty(rejected);
+    }
+
+    // ── TruncateForLog: rejected config entries are never logged in full ──────────────────
+
+    [Theory]
+    [InlineData("short", "short")]
+    [InlineData("12345678", "12345678")]
+    [InlineData("not-a-guid-but-long", "not-a-gu…(19 chars)")]
+    public void TruncateForLog_caps_entry_at_eight_chars_plus_length(string entry, string expected)
+    {
+        Assert.Equal(expected, AuthenticationMiddleware.TruncateForLog(entry));
     }
 }

src/Backend/AutopilotMonitor.Functions/Middleware/AuthenticationMiddleware.cs

@@ -164,8 +164,10 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
             // seam that lets the API trust a SECOND app registration during a tenant-move window
             // (see tasks/migration-cross-tenant-runbook.md). Unset today ⇒ exactly the primary id
             // ⇒ zero behaviour change.
+            var additionalClientIdsRaw = _configuration["EntraId:AdditionalClientIds"];
             var clientIds = ResolveConfiguredClientIds(
-                _configuration["EntraId:ClientId"], _configuration["EntraId:AdditionalClientIds"]);
+                _configuration["EntraId:ClientId"], additionalClientIdsRaw, out var rejectedAdditionalEntries);
+            AuditClientIdTrustSet(additionalClientIdsRaw, clientIds, rejectedAdditionalEntries);
             var validationParameters = BuildTokenValidationParameters(
                 openIdConfig.SigningKeys, clientIds);
 
@@ -321,24 +323,94 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
 
     /// <summary>
     /// Resolves the configured accepted app (client) IDs: the primary <c>EntraId:ClientId</c> plus an
-    /// optional comma/semicolon/whitespace-separated <c>EntraId:AdditionalClientIds</c>. Trimmed,
-    /// empties dropped, de-duplicated case-insensitively (primary first). When AdditionalClientIds is
-    /// unset the result is exactly the primary id — today's behaviour, zero change — so adding a
-    /// second app registration's client ID to that setting is the ONLY step needed to trust a second
-    /// app during a tenant-move window (tasks/migration-cross-tenant-runbook.md).
+    /// optional comma/semicolon/whitespace-separated <c>EntraId:AdditionalClientIds</c>. Additional
+    /// entries must be GUIDs (a client ID is always a GUID); non-GUID entries are dropped —
+    /// fail-closed, they could never match a token's <c>aud</c> anyway — and reported via
+    /// <paramref name="rejectedAdditionalEntries"/> so <see cref="AuditClientIdTrustSet"/> can make
+    /// the misconfiguration operator-visible. Accepted entries are normalized to the canonical
+    /// lowercase dashed GUID form (Entra <c>aud</c> claims are lowercase; audience comparison is
+    /// ordinal), so brace/case/no-dash variants an operator might paste still match. The result is
+    /// de-duplicated case-insensitively (primary first, verbatim — its handling is unchanged). When
+    /// AdditionalClientIds is unset the result is exactly the primary id — today's behaviour, zero
+    /// change — so adding a second app registration's client ID to that setting is the ONLY step
+    /// needed to trust a second app during a tenant-move window
+    /// (tasks/migration-cross-tenant-runbook.md).
     /// </summary>
-    internal static string[] ResolveConfiguredClientIds(string? primaryClientId, string? additionalClientIds)
+    internal static string[] ResolveConfiguredClientIds(
+        string? primaryClientId, string? additionalClientIds, out string[] rejectedAdditionalEntries)
     {
         var ids = new List<string?> { primaryClientId };
+        var rejected = new List<string>();
         if (!string.IsNullOrWhiteSpace(additionalClientIds))
         {
-            ids.AddRange(additionalClientIds.Split(
-                new[] { ',', ';', ' ', '\t', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries));
+            foreach (var entry in additionalClientIds.Split(
+                new[] { ',', ';', ' ', '\t', '\n', '\r' }, StringSplitOptions.RemoveEmptyEntries))
+            {
+                if (Guid.TryParse(entry.Trim(), out var clientId))
+                {
+                    ids.Add(clientId.ToString("D"));
+                }
+                else
+                {
+                    rejected.Add(entry.Trim());
+                }
+            }
         }
+        rejectedAdditionalEntries = rejected.ToArray();
         return ids
             .Where(id => !string.IsNullOrWhiteSpace(id))
             .Select(id => id!.Trim())
             .Distinct(StringComparer.OrdinalIgnoreCase)
             .ToArray();
     }
+
+    /// <summary>
+    /// Sentinel-free "already audited" marker: the last <c>EntraId:AdditionalClientIds</c> raw value
+    /// this instance logged about. Initial <c>null</c> intentionally equals the unset-config case —
+    /// an unset seam has nothing to report, so skipping it is correct, and the first non-null value
+    /// (or any change) triggers a fresh audit.
+    /// </summary>
+    private string? _auditedAdditionalClientIds;
+
+    /// <summary>
+    /// Operator-visible audit of the audience trust set (daily-review follow-up, 2026-07-14):
+    /// expanding the set of trusted app registrations via <c>EntraId:AdditionalClientIds</c> is
+    /// security-relevant and used to be completely silent — a typo'd GUID was indistinguishable
+    /// from "the new app registration doesn't work" during a tenant-move window. Logs once per
+    /// distinct config value per instance, at Warning because nothing below Warning reaches
+    /// App Insights from the worker. Rejected entries are truncated before logging in case an
+    /// operator accidentally pastes a secret into the setting.
+    /// </summary>
+    private void AuditClientIdTrustSet(
+        string? rawAdditionalClientIds, string[] clientIds, string[] rejectedAdditionalEntries)
+    {
+        if (string.Equals(_auditedAdditionalClientIds, rawAdditionalClientIds, StringComparison.Ordinal))
+        {
+            return;
+        }
+        // Benign race on concurrent first requests: at worst a duplicate audit line.
+        _auditedAdditionalClientIds = rawAdditionalClientIds;
+
+        if (rejectedAdditionalEntries.Length > 0)
+        {
+            _logger.LogWarning(
+                "[Auth Middleware] EntraId:AdditionalClientIds contains {Count} malformed (non-GUID) entries — ignored: {Entries}",
+                rejectedAdditionalEntries.Length,
+                string.Join(", ", rejectedAdditionalEntries.Select(TruncateForLog)));
+        }
+        if (clientIds.Length > 1)
+        {
+            _logger.LogWarning(
+                "[Auth Middleware] Token audience trust set expanded to {Count} client IDs via EntraId:AdditionalClientIds: {ClientIds}",
+                clientIds.Length,
+                string.Join(", ", clientIds));
+        }
+    }
+
+    /// <summary>
+    /// Truncates a rejected config entry for logging: enough to identify the typo, never the full
+    /// value (an operator could accidentally paste a client secret into the setting).
+    /// </summary>
+    internal static string TruncateForLog(string entry) =>
+        entry.Length <= 8 ? entry : $"{entry.Substring(0, 8)}…({entry.Length} chars)";
 }