diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/SearchSessionsByCveFilterTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/SearchSessionsByCveFilterTests.cs new file mode 100644 index 00000000..e9f74112 --- /dev/null +++ b/src/Backend/AutopilotMonitor.Functions.Tests/SearchSessionsByCveFilterTests.cs @@ -0,0 +1,66 @@ +using AutopilotMonitor.Functions.Services; + +namespace AutopilotMonitor.Functions.Tests; + +/// +/// Regression guard for the CveIndex query filter behind +/// GET /api/global/search/sessions-by-cve. +/// +/// The CveIndex PartitionKey is {tenantId}_{cveId}. The original +/// cross-tenant branch expressed a PartitionKey range on the cveId +/// (PartitionKey ge '{cve}' and PartitionKey lt '{cve}~') — but no row's +/// PK begins with the cveId (they all begin with the tenant GUID), so the GA +/// cross-tenant search silently returned 0 results while +/// get_vulnerability_summary (a full CveId-property scan) reported the +/// real, non-zero exposure. These tests pin the corrected filter shape. +/// +public class SearchSessionsByCveFilterTests +{ + private const string TenantA = "a1b2c3d4-e5f6-7890-abcd-ef1234567890"; + private const string CveA = "CVE-2024-21447"; + + [Fact] + public void CrossTenant_filters_on_CveId_property_not_a_PartitionKey_range() + { + var filter = TableStorageService.BuildCveIndexSearchFilter(tenantId: null, cveId: CveA); + + // Must match the CveId column so it sweeps every tenant's partitions. + Assert.Equal($"CveId eq '{CveA}'", filter); + // The old, broken form anchored a range on the PK (which starts with the + // tenant GUID, not the cveId) — must never come back. + Assert.DoesNotContain("PartitionKey ge", filter); + Assert.DoesNotContain("PartitionKey lt", filter); + } + + [Fact] + public void CrossTenant_treats_empty_tenantId_same_as_null() + { + Assert.Equal( + TableStorageService.BuildCveIndexSearchFilter(tenantId: null, cveId: CveA), + TableStorageService.BuildCveIndexSearchFilter(tenantId: "", cveId: CveA)); + } + + [Fact] + public void TenantScoped_uses_exact_partition_key_match() + { + var filter = TableStorageService.BuildCveIndexSearchFilter(TenantA, CveA); + Assert.Equal($"PartitionKey eq '{TenantA}_{CveA}'", filter); + } + + [Fact] + public void Escapes_single_quotes_in_cveId_to_neutralize_injection() + { + var filter = TableStorageService.BuildCveIndexSearchFilter( + tenantId: null, cveId: "CVE-2024' or CveId ne '"); + // Quotes are doubled — the payload stays inside the quoted literal. + Assert.Equal("CveId eq 'CVE-2024'' or CveId ne '''", filter); + } + + [Fact] + public void Escapes_single_quotes_in_tenantId() + { + var filter = TableStorageService.BuildCveIndexSearchFilter( + tenantId: "t' or PartitionKey ne '", cveId: CveA); + Assert.StartsWith("PartitionKey eq 't'' or PartitionKey ne ''_", filter); + } +} diff --git a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs index 233c37b2..96946e6b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs +++ b/src/Backend/AutopilotMonitor.Functions/Services/TableStorageService.AgentApi.cs @@ -1161,23 +1161,7 @@ namespace AutopilotMonitor.Functions.Services var tableClient = _tableServiceClient.GetTableClient(Constants.TableNames.CveIndex); - // Tenant-scoped: PK exact match (partition-targeted, cheap). - // Cross-tenant: PK prefix scan over `{*}_{cveId}` — Azure can't - // partition-target a suffix match, so this is the closest range we - // can express. Server-side CVSS / risk filters could in theory move - // to OData on numeric/string columns, but the existing index rows - // already enrich them, so client-side filtering after the page fetch - // matches the legacy method's semantics exactly. - string oDataFilter; - if (!string.IsNullOrEmpty(tenantId)) - { - oDataFilter = $"PartitionKey eq '{ODataSanitizer.EscapeValue(tenantId)}_{ODataSanitizer.EscapeValue(cveId)}'"; - } - else - { - var safeCveId = ODataSanitizer.EscapeValue(cveId); - oDataFilter = $"PartitionKey ge '{safeCveId}' and PartitionKey lt '{safeCveId}~'"; - } + var oDataFilter = BuildCveIndexSearchFilter(tenantId, cveId); var (entities, nextRawToken) = await AzureTablesPaginator.FetchPageAsync( client: tableClient, @@ -1214,6 +1198,27 @@ namespace AutopilotMonitor.Functions.Services return new RawPage(sessions, nextRawToken); } + /// + /// Builds the CveIndex OData filter for the per-CVE session search. + /// CveIndex PartitionKey is {tenantId}_{cveId}. + /// + /// Tenant-scoped: exact PK match — partition-targeted, cheap. + /// Cross-tenant (tenantId null): the PK begins with the tenant GUID, + /// NOT the cveId, so a PK range on the cveId matches nothing. We filter on + /// the CveId property instead — a server-side scan across partitions, + /// the same shape uses for fleet aggregation. + /// + /// Exposed internal for regression testing (the old PK-range form silently + /// returned 0 cross-tenant results — see SearchSessionsByCveFilterTests). + /// + internal static string BuildCveIndexSearchFilter(string? tenantId, string cveId) + { + var safeCveId = ODataSanitizer.EscapeValue(cveId); + return string.IsNullOrEmpty(tenantId) + ? $"CveId eq '{safeCveId}'" + : $"PartitionKey eq '{ODataSanitizer.EscapeValue(tenantId)}_{safeCveId}'"; + } + /// /// Scans the CveIndex for fleet-wide vulnerability aggregation. Tenant-scoped /// uses a partition-prefix range (`{tenantId}_` .. `{tenantId}_~`) so Azure diff --git a/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts b/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts index 629d13d4..dfa721b5 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/tools/admin.ts @@ -46,7 +46,9 @@ export function registerAdminTools(server: McpServer): void { 'get_api_usage', 'Get API/MCP usage statistics. Shows request counts per endpoint per day. ' + 'Use to monitor platform usage, identify heavy users, or debug rate limiting. ' + - 'Use userId for a specific user, daily for aggregated summaries, or neither for global per-record breakdown.', + 'Use userId for a specific user, daily for aggregated summaries, or neither for global per-record breakdown. ' + + 'The daily and global breakdowns are Global Admin only; the per-user (userId) view also works for a ' + + 'Tenant Admin querying a user within their own tenant.', { userId: z.string().optional().describe('Specific user object ID to query usage for'), tenantId: z.string().optional().describe('Filter usage by tenant ID'), @@ -105,9 +107,13 @@ export function registerAdminTools(server: McpServer): void { // Tool 13: get_geographic_sessions server.tool( 'get_geographic_sessions', - 'Drill into a specific geographic location. Returns all enrollment sessions at that location. ' + + 'Drill into a specific geographic location and list its enrollment sessions (lean rows). ' + 'Either provide locationKey (from get_geographic_metrics results, e.g. "Falkenstein, Saxony, DE") ' + - 'or use country/region/city filters to find sessions by location.', + 'or use country/region/city filters to find sessions by location. ' + + 'A busy location can hold thousands of sessions, so results are paged: the default pageSize is 50 ' + + 'and the response carries a "nextLink" when more remain — pass that whole string back as "continuation" ' + + 'to get the next slice, and stop when nextLink is absent. (Pagination is applied to the location result ' + + 'set, so raise pageSize for fewer round-trips when you need a full sweep.)', { locationKey: z.string().optional().describe('Location key from get_geographic_metrics (e.g. "Falkenstein, Saxony, DE"). If provided, country/region/city are ignored.'), country: z.string().optional().describe('2-letter country code filter (e.g. "DE", "US", "CH"). Used when locationKey is not provided.'), @@ -116,11 +122,15 @@ export function registerAdminTools(server: McpServer): void { tenantId: z.string().optional().describe('Tenant ID. Omit for cross-tenant view (Global Admin only).'), days: z.coerce.number().int().min(1).max(365).optional().default(30) .describe('Time range in days (1-365). Defaults to 30.'), + pageSize: z.coerce.number().int().min(1).max(1000).optional().default(50) + .describe('Sessions to return per call (1-1000, default 50). Follow nextLink for more; raise it for full sweeps.'), + continuation: z.string().optional() + .describe('Pass the whole nextLink string from the prior response to fetch the next slice.'), }, READ_ONLY, async (args) => withToolTelemetry('get_geographic_sessions', async () => { try { - const { tenantId, country, region, city, ...rest } = args; + const { tenantId, country, region, city, pageSize, continuation, ...rest } = args; const params: Record = { ...rest }; if (tenantId) params.tenantId = tenantId; if (!params.locationKey && country) { @@ -129,8 +139,31 @@ export function registerAdminTools(server: McpServer): void { params.groupBy = city ? 'city' : region ? 'region' : 'country'; } const prefix = pickGlobalOrTenantPath('/api/global/metrics', '/api/metrics'); - const data = await apiFetch(`${prefix}/geographic/sessions${buildQuery(params)}`); - return toolResultText(data, MAX_RESULT_SIZE_CHARS.sessions); + const data = await apiFetch(`${prefix}/geographic/sessions${buildQuery(params)}`) as + { success?: boolean; sessions?: unknown[]; totalCount?: number }; + + // The location-sessions endpoint filters in-memory and returns the full + // set in one shot (no server-side cursor). A single busy location can + // exceed the client's inline response budget, so we page client-side: + // the "continuation" carries an integer offset into the location result + // set. (Each page re-fetches the location set; raise pageSize to amortize.) + const all = Array.isArray(data?.sessions) ? data.sessions : []; + const m = continuation ? /^geo-offset:(\d+)$/.exec(continuation) : null; + const offset = m ? parseInt(m[1], 10) : 0; + const slice = all.slice(offset, offset + pageSize); + const nextOffset = offset + pageSize; + const nextLink = nextOffset < all.length ? `geo-offset:${nextOffset}` : null; + + return toolResultText( + { + success: data?.success ?? true, + totalCount: data?.totalCount ?? all.length, + count: slice.length, + offset, + sessions: slice, + nextLink, + }, + MAX_RESULT_SIZE_CHARS.sessions); } catch (error: unknown) { return toolError('get_geographic_sessions', args, error); } @@ -225,10 +258,12 @@ export function registerAdminTools(server: McpServer): void { server.tool( 'get_usage_metrics', 'Get platform usage statistics: active tenants, session volumes, feature adoption. ' + - 'Omit tenantId for platform-wide overview (Global Admin), or specify tenantId for tenant-specific usage. ' + + 'Global Admin only — omit tenantId for the platform-wide overview, or pass tenantId to filter that ' + + 'cross-tenant view down to a single tenant. The call always uses the global endpoint, so a Tenant Admin ' + + 'cannot use this tool (they receive 403) even when passing their own tenantId. ' + 'days accepts any value 1-365 (e.g. 5, 7, 12, 30, 90).', { - tenantId: z.string().optional().describe('Tenant ID for tenant-specific metrics. Omit for platform-wide overview (Global Admin only).'), + tenantId: z.string().optional().describe('Filter the global view to a single tenant. Omit for the platform-wide overview. This tool is Global Admin only either way.'), days: z.coerce.number().int().min(1).max(365).optional().default(30) .describe('Time window in days (1-365). Defaults to 30. Sessions.Total / Tenants.Total reflect this window.'), }, @@ -452,7 +487,7 @@ export function registerAdminTools(server: McpServer): void { 'Pass the whole nextLink string as "continuation" so all backend-echoed query params round-trip correctly.', { tenantId: z.string().optional().describe('Tenant ID to query. Omit for cross-tenant access (Global Admin only).'), - status: z.enum(['InProgress', 'Succeeded', 'Failed']).optional(), + status: z.enum(['InProgress', 'Pending', 'Stalled', 'Succeeded', 'Failed']).optional(), startedAfter: z.string().optional().describe('ISO 8601 datetime'), startedBefore: z.string().optional().describe('ISO 8601 datetime'), serialNumber: z.string().optional(), diff --git a/src/McpServer/autopilot-monitor-mcp/src/tools/sessions.ts b/src/McpServer/autopilot-monitor-mcp/src/tools/sessions.ts index a21ed756..6ac4e5e0 100644 --- a/src/McpServer/autopilot-monitor-mcp/src/tools/sessions.ts +++ b/src/McpServer/autopilot-monitor-mcp/src/tools/sessions.ts @@ -52,7 +52,9 @@ export function registerSessionTools(server: McpServer): void { 'query params round-trip correctly.', { tenantId: z.string().optional().describe('Tenant ID. Omit for cross-tenant search (Global Admin only).'), - status: z.enum(['InProgress', 'Succeeded', 'Failed']).optional().describe('Enrollment status filter'), + status: z.enum(['InProgress', 'Pending', 'Stalled', 'Succeeded', 'Failed']).optional() + .describe('Enrollment status filter. Pending = White Glove pre-provisioning done, awaiting user enrollment; ' + + 'Stalled = no progress for a while (non-terminal, can heal back to InProgress).'), serialNumber: z.string().optional().describe('Device serial number (exact match)'), deviceName: z.string().optional().describe('Device name (prefix match, e.g. "DESKTOP-")'), manufacturer: z.string().optional().describe('Hardware manufacturer (e.g. "Microsoft", "Dell", "HP")'), @@ -376,7 +378,18 @@ export function registerSessionTools(server: McpServer): void { keyEventsTruncated: truncated, errorCount, warningCount, - appInstalls: { total: appTotal, succeeded: appSucceeded, failed: appFailed, skipped: appSkipped }, + // `started` events can under-count vs terminal ones (dependencies and + // retries emit completed/failed without a matching "started"), which + // previously made `total` smaller than `succeeded`. Derive `total` as + // the max of the start signal and the sum of terminal outcomes so it is + // never smaller than its own breakdown; expose `started` for context. + appInstalls: { + total: Math.max(appTotal, appSucceeded + appFailed + appSkipped), + started: appTotal, + succeeded: appSucceeded, + failed: appFailed, + skipped: appSkipped, + }, }, };