fix(mcp): MCP server security & capability audit (Phases A-E)

Critical: GET /api/metrics/mcp-usage/user/{userId} (TenantAdminOrGA, no
TenantScoping) handed the oid to the repo without checking that returned
records belong to the caller's tenant. A tenant admin from tenant A could
read MCP usage records of any oid — including oids in tenant B.

McpUsageMetricsFunction.GetUserUsage now blocks 403 CrossTenantAccessDenied
when records are tenant-foreign and caller is not a Global Admin. The pure
decision logic lives in UsageCrossTenantGuard for unit-testability (9 tests
covering own/foreign/mixed/GA/empty/null/case-insensitive/empty-tid paths).

Also:
- MCP get_api_usage tool was calling /api/metrics/mcp-usage/{userId}
  (without /user/), which is unregistered and returned 404 — corrected.
- list_blocked_devices description honestly states the endpoint is
  GA-only on both variants (catalog policy, no change to the gate).
- days clamp [1, 365] added to /metrics/{app, geographic, geographic/sessions}
  for parity with /metrics/summary, /metrics/usage, /metrics/platform.
- get_geographic_metrics, get_geographic_sessions zod schemas now constrain
  days to int().min(1).max(365) — same pattern as the other tools.
- search/sessions, search/sessions-by-event, search/sessions-by-cve switched
  limit handling from Math.Min(lim, 100) to Math.Clamp(lim, 1, 100); the old
  form let a negative ?limit=-5 reach the repo.

Audit static analysis confirmed all other tenant-scoped endpoints route
tenantId through TenantHelper.GetTenantId(req) (JWT-only, ignores query),
so ?tenantId=<other> is silently dropped. Live-verified with an Operator
token: 14 cross-tenant query probes returned only the caller's tenant,
13 GlobalAdminOnly routes returned 403.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

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

@@ -0,0 +1,126 @@
+using AutopilotMonitor.Functions.Functions.Metrics;
+using AutopilotMonitor.Shared.DataAccess;
+
+namespace AutopilotMonitor.Functions.Tests;
+
+/// <summary>
+/// Unit tests for the cross-tenant guard on <c>GET /api/metrics/mcp-usage/user/{userId}</c>.
+///
+/// Background: the route is catalog-policy <c>TenantAdminOrGA</c> with no <c>TenantScoping</c>
+/// (the path parameter is an Azure AD object id, not a tenant id). Without this guard, a tenant
+/// admin from tenant A could read MCP usage records of any oid — including oids in tenant B —
+/// because the function blindly hands the oid to <c>IUserUsageRepository.GetUsageByUserAsync</c>.
+/// </summary>
+public class UsageCrossTenantGuardTests
+{
+    private const string TenantA = "00000000-0000-0000-0000-aaaaaaaaaaaa";
+    private const string TenantB = "00000000-0000-0000-0000-bbbbbbbbbbbb";
+
+    private static UserUsageRecord Rec(string tid, string endpoint = "ep") => new()
+    {
+        UserId = "oid-1",
+        UserPrincipalName = "alice@contoso.com",
+        TenantId = tid,
+        Endpoint = endpoint,
+        Date = "20260505",
+        RequestCount = 1,
+    };
+
+    [Fact]
+    public void OwnTenantOnly_NonGA_Allowed()
+    {
+        var records = new[] { Rec(TenantA), Rec(TenantA, "ep2") };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, TenantA, isGlobalAdmin: false);
+
+        Assert.False(blocked);
+    }
+
+    [Fact]
+    public void ForeignTenantOnly_NonGA_Blocked()
+    {
+        var records = new[] { Rec(TenantB), Rec(TenantB, "ep2") };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, TenantA, isGlobalAdmin: false);
+
+        Assert.True(blocked);
+    }
+
+    [Fact]
+    public void MixedTenants_NonGA_Blocked()
+    {
+        // Even one foreign record must trip the guard — a tenant admin must not see another tenant's row.
+        var records = new[] { Rec(TenantA), Rec(TenantB) };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, TenantA, isGlobalAdmin: false);
+
+        Assert.True(blocked);
+    }
+
+    [Fact]
+    public void ForeignTenant_GlobalAdmin_Allowed()
+    {
+        // GA can see usage across all tenants.
+        var records = new[] { Rec(TenantB) };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, TenantA, isGlobalAdmin: true);
+
+        Assert.False(blocked);
+    }
+
+    [Fact]
+    public void EmptyRecords_NonGA_Allowed()
+    {
+        // No records → caller learns nothing; safe to return 200 with empty list.
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(
+            Array.Empty<UserUsageRecord>(), TenantA, isGlobalAdmin: false);
+
+        Assert.False(blocked);
+    }
+
+    [Fact]
+    public void NullRecords_DoesNotThrow()
+    {
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(null!, TenantA, isGlobalAdmin: false);
+        Assert.False(blocked);
+    }
+
+    [Fact]
+    public void RecordWithEmptyTenantId_Ignored()
+    {
+        // Defensive: legacy records without TenantId must not be treated as a leak signal
+        // (they could belong to anyone). Other records still drive the decision.
+        var records = new[]
+        {
+            Rec(""),         // unknown tenant — ignored
+            Rec(TenantA),    // own tenant
+        };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, TenantA, isGlobalAdmin: false);
+
+        Assert.False(blocked);
+    }
+
+    [Fact]
+    public void CallerTenantIdEmpty_NotBlocked()
+    {
+        // Anonymous / device path scenarios shouldn't reach this code — but if they do,
+        // we don't synthesize a 403 from missing claims; let the catalog policy handle auth.
+        var records = new[] { Rec(TenantA) };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, "", isGlobalAdmin: false);
+
+        Assert.False(blocked);
+    }
+
+    [Fact]
+    public void TenantIdComparison_IsCaseInsensitive()
+    {
+        // Azure AD tids are GUIDs; some sources upper-case them. Comparison must not flag those as foreign.
+        var records = new[] { Rec(TenantA.ToUpperInvariant()) };
+
+        var blocked = UsageCrossTenantGuard.IsForeignTenantAccess(records, TenantA, isGlobalAdmin: false);
+
+        Assert.False(blocked);
+    }
+}

src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetAppMetricsFunction.cs

@@ -28,12 +28,15 @@ namespace AutopilotMonitor.Functions.Functions.Metrics
                 var tenantId = TenantHelper.GetTenantId(req);
                 _logger.LogInformation($"Fetching app metrics for tenant {tenantId}");
 
-                // Optional time filter: ?days=7 (default: 30)
+                // Optional time filter: ?days=7 (default: 30, clamped to [1, 365] for consistency
+                // with metrics/summary, metrics/usage, and metrics/platform).
                 var query = System.Web.HttpUtility.ParseQueryString(req.Url.Query);
                 var daysParam = query["days"];
                 int days = 30;
                 if (!string.IsNullOrEmpty(daysParam) && int.TryParse(daysParam, out var parsedDays) && parsedDays > 0)
                     days = parsedDays;
+                if (days < 1) days = 1;
+                if (days > 365) days = 365;
 
                 var cutoff = DateTime.UtcNow.AddDays(-days);
 

src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGeographicLocationSessionsFunction.cs

@@ -46,6 +46,8 @@ namespace AutopilotMonitor.Functions.Functions.Metrics
                 int days = 30;
                 if (!string.IsNullOrEmpty(daysParam) && int.TryParse(daysParam, out var parsedDays) && parsedDays > 0)
                     days = parsedDays;
+                if (days < 1) days = 1;
+                if (days > 365) days = 365;
 
                 var groupBy = query["groupBy"] ?? "city";
 

src/Backend/AutopilotMonitor.Functions/Functions/Metrics/GetGeographicMetricsFunction.cs

@@ -39,6 +39,8 @@ namespace AutopilotMonitor.Functions.Functions.Metrics
                 int days = 30;
                 if (!string.IsNullOrEmpty(daysParam) && int.TryParse(daysParam, out var parsedDays) && parsedDays > 0)
                     days = parsedDays;
+                if (days < 1) days = 1;
+                if (days > 365) days = 365;
 
                 var groupBy = query["groupBy"] ?? "city";
 

src/Backend/AutopilotMonitor.Functions/Functions/Metrics/McpUsageMetricsFunction.cs

@@ -7,6 +7,8 @@ using Microsoft.Azure.Functions.Worker;
 using Microsoft.Azure.Functions.Worker.Http;
 using Microsoft.Extensions.Logging;
 
+// Cross-tenant guard helpers — exposed for unit testing.
+
 namespace AutopilotMonitor.Functions.Functions.Metrics
 {
     /// <summary>
@@ -75,7 +77,12 @@ namespace AutopilotMonitor.Functions.Functions.Metrics
         }
 
         /// <summary>
-        /// GET /api/metrics/mcp-usage/user/{userId}?dateFrom=&amp;dateTo= — Usage for a specific user
+        /// GET /api/metrics/mcp-usage/user/{userId}?dateFrom=&amp;dateTo= — Usage for a specific user.
+        ///
+        /// Catalog policy is TenantAdminOrGA, but the route has no TenantScoping — middleware can't
+        /// enforce cross-tenant access since {userId} is an Azure AD object id, not a tenant id.
+        /// We therefore enforce here: if the records' TenantId differs from the caller's tenant
+        /// (and the caller is not a Global Admin), block with 403.
         /// </summary>
         [Function("GetMcpUserUsage")]
         public async Task<HttpResponseData> GetUserUsage(
@@ -91,6 +98,27 @@ namespace AutopilotMonitor.Functions.Functions.Metrics
 
                 var records = await _userUsageRepo.GetUsageByUserAsync(userId, dateFrom, dateTo);
 
+                var ctx = req.GetRequestContext();
+                if (UsageCrossTenantGuard.IsForeignTenantAccess(records, ctx.TenantId, ctx.IsGlobalAdmin))
+                {
+                    var foundTenants = records
+                        .Select(r => r.TenantId)
+                        .Where(t => !string.IsNullOrEmpty(t))
+                        .Distinct(StringComparer.OrdinalIgnoreCase)
+                        .ToArray();
+                    _logger.LogWarning(
+                        "[McpUsage] Blocked cross-tenant access: caller={Caller} callerTid={CallerTid} targetUser={UserId} foundTenants={Tenants}",
+                        ctx.UserPrincipalName, ctx.TenantId, userId, string.Join(",", foundTenants));
+
+                    var forbidden = req.CreateResponse(HttpStatusCode.Forbidden);
+                    await forbidden.WriteAsJsonAsync(new
+                    {
+                        error = "CrossTenantAccessDenied",
+                        message = "Access denied. You can only access usage for users in your own tenant."
+                    });
+                    return forbidden;
+                }
+
                 var response = req.CreateResponse(HttpStatusCode.OK);
                 await response.WriteAsJsonAsync(new { userId, records });
                 return response;

src/Backend/AutopilotMonitor.Functions/Functions/Metrics/UsageCrossTenantGuard.cs

@@ -0,0 +1,37 @@
+using AutopilotMonitor.Shared.DataAccess;
+
+namespace AutopilotMonitor.Functions.Functions.Metrics;
+
+/// <summary>
+/// Cross-tenant access guard for <c>GET /api/metrics/mcp-usage/user/{userId}</c>.
+///
+/// The route accepts an Azure AD object id (oid), which has no inherent tenant scoping —
+/// middleware can't validate it the way it does for <c>{tenantId}</c> routes. The function
+/// therefore inspects the returned <see cref="UserUsageRecord"/>s and blocks the response
+/// if any record belongs to a tenant other than the caller's, unless the caller is a Global Admin.
+/// </summary>
+public static class UsageCrossTenantGuard
+{
+    /// <summary>
+    /// Returns true if the records contain at least one entry whose tenant differs from
+    /// <paramref name="callerTenantId"/> AND the caller is not a Global Admin.
+    /// Empty record sets and GA callers always pass (return false).
+    /// </summary>
+    public static bool IsForeignTenantAccess(
+        IEnumerable<UserUsageRecord> records,
+        string callerTenantId,
+        bool isGlobalAdmin)
+    {
+        if (isGlobalAdmin) return false;
+        if (records == null) return false;
+        if (string.IsNullOrEmpty(callerTenantId)) return false;
+
+        foreach (var r in records)
+        {
+            if (string.IsNullOrEmpty(r.TenantId)) continue;
+            if (!string.Equals(r.TenantId, callerTenantId, StringComparison.OrdinalIgnoreCase))
+                return true;
+        }
+        return false;
+    }
+}

src/Backend/AutopilotMonitor.Functions/Functions/Sessions/SearchSessionsByCveFunction.cs

@@ -54,7 +54,7 @@ public class SearchSessionsByCveFunction
 
             double? minCvssScore = double.TryParse(query["minCvssScore"], out var mcs) ? mcs : null;
             var overallRisk = query["overallRisk"];
-            var limit = int.TryParse(query["limit"], out var lim) ? Math.Min(lim, 100) : 50;
+            var limit = int.TryParse(query["limit"], out var lim) ? Math.Clamp(lim, 1, 100) : 50;
 
             var sessions = await _sessionRepo.SearchSessionsByCveAsync(tenantId, cveId, minCvssScore, overallRisk, limit);
 

src/Backend/AutopilotMonitor.Functions/Functions/Sessions/SearchSessionsByEventFunction.cs

@@ -52,7 +52,7 @@ public class SearchSessionsByEventFunction
                 return badReq;
             }
 
-            var limit = int.TryParse(query["limit"], out var lim) ? Math.Min(lim, 100) : 50;
+            var limit = int.TryParse(query["limit"], out var lim) ? Math.Clamp(lim, 1, 100) : 50;
 
             var sessions = await _sessionRepo.SearchSessionsByEventAsync(tenantId, eventType, null, null, null, limit);
 

src/Backend/AutopilotMonitor.Functions/Functions/Sessions/SearchSessionsFunction.cs

@@ -58,7 +58,7 @@ public class SearchSessionsFunction
                 GeoCountry = query["geoCountry"],
                 AgentVersion = query["agentVersion"],
                 ImeAgentVersion = query["imeAgentVersion"],
-                Limit = int.TryParse(query["limit"], out var lim) ? Math.Min(lim, 100) : 50,
+                Limit = int.TryParse(query["limit"], out var lim) ? Math.Clamp(lim, 1, 100) : 50,
             };
 
             if (bool.TryParse(query["isPreProvisioned"], out var ipp)) filter.IsPreProvisioned = ipp;

src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts

@@ -25,7 +25,7 @@ export function registerAdminTools(server: McpServer): void {
         let data: unknown;
         const params: Record<string, string | undefined> = { tenantId: args.tenantId, dateFrom: args.dateFrom, dateTo: args.dateTo };
         if (args.userId) {
-          data = await apiFetch(`/api/metrics/mcp-usage/${encodeURIComponent(args.userId)}${buildQuery(params)}`);
+          data = await apiFetch(`/api/metrics/mcp-usage/user/${encodeURIComponent(args.userId)}${buildQuery(params)}`);
         } else if (args.daily) {
           data = await apiFetch(`/api/global/metrics/mcp-usage/daily${buildQuery(params)}`);
         } else {
@@ -43,10 +43,12 @@ export function registerAdminTools(server: McpServer): void {
     'get_geographic_metrics',
     'Get geographic distribution of enrollments — where devices are enrolling from, with performance comparisons. ' +
     'Shows per-location: session counts, success rates, avg/median/p95 duration, throughput, and outlier detection. ' +
-    'Omit tenantId for cross-tenant view (Global Admin). Use get_geographic_sessions to drill into a specific location.',
+    'Omit tenantId for cross-tenant view (Global Admin). Use get_geographic_sessions to drill into a specific location. ' +
+    'days accepts any value 1-365 (e.g. 5, 7, 12, 30, 90).',
     {
       tenantId: z.string().optional().describe('Tenant ID. Omit for cross-tenant view (Global Admin only).'),
-      days: z.coerce.number().optional().default(30).describe('Time range in days (default: 30)'),
+      days: z.coerce.number().int().min(1).max(365).optional().default(30)
+        .describe('Time range in days (1-365). Defaults to 30.'),
       groupBy: z.enum(['country', 'region', 'city']).optional().default('city')
         .describe('Geographic grouping level (default: "city")'),
     },
@@ -77,7 +79,8 @@ export function registerAdminTools(server: McpServer): void {
       region: z.string().optional().describe('Region/state filter (e.g. "Saxony", "North Carolina"). Used with country.'),
       city: z.string().optional().describe('City filter (e.g. "Falkenstein"). Used with country.'),
       tenantId: z.string().optional().describe('Tenant ID. Omit for cross-tenant view (Global Admin only).'),
-      days: z.coerce.number().optional().default(30).describe('Time range in days (default: 30)'),
+      days: z.coerce.number().int().min(1).max(365).optional().default(30)
+        .describe('Time range in days (1-365). Defaults to 30.'),
     },
     READ_ONLY,
     async (args) => withToolTelemetry('get_geographic_sessions', async () => {

src/McpServer/autopilot-monitor-mcp/src/tools/sessions.ts

@@ -378,9 +378,10 @@ export function registerSessionTools(server: McpServer): void {
   server.tool(
     'list_blocked_devices',
     'List devices currently blocked from enrolling. Blocked devices have their enrollment sessions rejected by the backend. ' +
-    'Omit tenantId to list blocked devices across ALL tenants (requires Global Admin).',
+    'Global Admin only — both the tenant-scoped (?tenantId=) and cross-tenant variants of this endpoint require Global Admin. ' +
+    'Tenant Admins and Operators receive 403 (the backend manages the device block list as a platform-wide concern).',
     {
-      tenantId: z.string().optional().describe('Tenant ID to scope results. Omit for cross-tenant listing (Global Admin only).'),
+      tenantId: z.string().optional().describe('Tenant ID to scope results. Optional — both forms require Global Admin.'),
     },
     READ_ONLY,
     async (args) => withToolTelemetry('list_blocked_devices', async () => {