fix(mcp,backend): cross-tenant CVE search + geo pagination + tool polish

Pre-release MCP tool audit fixes:

- backend: search_sessions_by_cve returned 0 in GA cross-tenant mode. The
  CveIndex PartitionKey is {tenantId}_{cveId}, so the old cross-tenant PK
  range anchored on the cveId matched nothing (PKs start with the tenant
  GUID). Now filters on the CveId property (server-side scan, same shape as
  ScanCveIndexAsync). Extracted BuildCveIndexSearchFilter + 5 regression tests.
- mcp: get_geographic_sessions gains pageSize/continuation (client-side
  offset; the endpoint has no server cursor) to bound response size.
- mcp: get_usage_metrics / get_api_usage descriptions corrected to reflect
  Global-Admin-only routing (were implying a tenant-scoped mode that 403s).
- mcp: get_session_summary appInstalls.total no longer smaller than its
  breakdown (max(started, succeeded+failed+skipped)) + adds started.
- mcp: session status enum gains Pending + Stalled.

Tests: backend targeted suite 24 green; mcp tsc clean + unit suite 52 green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Oliver Kieselbach
2026-05-27 20:49:01 +02:00
parent 09581e84e6
commit c344cde196
4 changed files with 147 additions and 28 deletions
@@ -0,0 +1,66 @@
using AutopilotMonitor.Functions.Services;
namespace AutopilotMonitor.Functions.Tests;
/// <summary>
/// Regression guard for the CveIndex query filter behind
/// <c>GET /api/global/search/sessions-by-cve</c>.
///
/// The CveIndex PartitionKey is <c>{tenantId}_{cveId}</c>. The original
/// cross-tenant branch expressed a PartitionKey range on the cveId
/// (<c>PartitionKey ge '{cve}' and PartitionKey lt '{cve}~'</c>) — 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
/// <c>get_vulnerability_summary</c> (a full CveId-property scan) reported the
/// real, non-zero exposure. These tests pin the corrected filter shape.
/// </summary>
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);
}
}
@@ -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<TableEntity>(
client: tableClient,
@@ -1214,6 +1198,27 @@ namespace AutopilotMonitor.Functions.Services
return new RawPage<SessionSummary>(sessions, nextRawToken);
}
/// <summary>
/// Builds the CveIndex OData filter for the per-CVE session search.
/// CveIndex PartitionKey is <c>{tenantId}_{cveId}</c>.
/// <list type="bullet">
/// <item>Tenant-scoped: exact PK match — partition-targeted, cheap.</item>
/// <item>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 <c>CveId</c> property instead — a server-side scan across partitions,
/// the same shape <see cref="ScanCveIndexAsync"/> uses for fleet aggregation.</item>
/// </list>
/// Exposed internal for regression testing (the old PK-range form silently
/// returned 0 cross-tenant results — see SearchSessionsByCveFilterTests).
/// </summary>
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}'";
}
/// <summary>
/// Scans the CveIndex for fleet-wide vulnerability aggregation. Tenant-scoped
/// uses a partition-prefix range (`{tenantId}_` .. `{tenantId}_~`) so Azure
@@ -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<string, string | number | undefined> = { ...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(),
@@ -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,
},
},
};