mirror of
https://github.com/okieselbach/AutopilotMonitor.git
synced 2026-10-24 12:42:38 +00:00
backend: key-safe X-MCP-Tool-Name prefix for the usage row key (CodeQL cs/log-forging #91)
The caller-supplied X-MCP-Tool-Name header went verbatim into the UserUsageLog
row key ({yyyyMMdd}_{tool}:{endpoint}) and from there into the CAS-conflict
warning of TableCasRetry. EndpointNormalizer.ToolNameKey now strips CR/LF and
every other control character or line separator, replaces the characters Table
Storage rejects in keys (/ \ # ?) and caps the length at 64; an empty result
skips the prefix. Real tool names pass through unchanged, so existing usage
rows keep their keys. A header with an illegal key character previously made
every usage increment fail with 400 (fail-soft, counter never advanced).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
@@ -0,0 +1,57 @@
|
||||
using AutopilotMonitor.Functions.Helpers;
|
||||
|
||||
namespace AutopilotMonitor.Functions.Tests;
|
||||
|
||||
public class EndpointNormalizerTests
|
||||
{
|
||||
[Fact]
|
||||
public void Normalize_StripsApiPrefixReplacesGuidsAndKeyChars()
|
||||
{
|
||||
var normalized = EndpointNormalizer.Normalize("/api/Sessions/0f8fad5b-d9cb-469f-a165-70867728950e/events?x=1");
|
||||
Assert.Equal("sessions_{id}_events_x=1", normalized);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ToolNameKey_RealToolName_PassesThroughUnchanged()
|
||||
{
|
||||
// Existing UserUsageLog rows are keyed by "<tool>:<endpoint>"; a real tool name must keep its key.
|
||||
Assert.Equal("get_session_events", EndpointNormalizer.ToolNameKey("get_session_events"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ToolNameKey_NullOrEmpty_ReturnsEmpty()
|
||||
{
|
||||
Assert.Equal(string.Empty, EndpointNormalizer.ToolNameKey(null));
|
||||
Assert.Equal(string.Empty, EndpointNormalizer.ToolNameKey(string.Empty));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ToolNameKey_WithCrlfAndControlChars_StripsThem()
|
||||
{
|
||||
// The log-forging vector: a header that tries to inject a fake log line through the row key.
|
||||
var forged = $"get_metrics\r\n[McpQuota] FAKE{(char)0}{(char)27}{(char)0x2028}x";
|
||||
var key = EndpointNormalizer.ToolNameKey(forged);
|
||||
Assert.Equal("get_metrics[McpQuota] FAKEx", key);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ToolNameKey_WithTableKeyIllegalChars_ReplacesThem()
|
||||
{
|
||||
// '/', '\', '#', '?' are rejected by Table Storage in keys — a header carrying them would make
|
||||
// every usage increment fail with 400, so they are replaced the same way the path is.
|
||||
Assert.Equal("a_b_c_d_e", EndpointNormalizer.ToolNameKey("a/b\\c#d?e"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ToolNameKey_LongerThanCap_IsTruncated()
|
||||
{
|
||||
var key = EndpointNormalizer.ToolNameKey(new string('t', EndpointNormalizer.MaxToolNameLength + 10));
|
||||
Assert.Equal(EndpointNormalizer.MaxToolNameLength, key.Length);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void ToolNameKey_OnlyControlChars_ReturnsEmpty()
|
||||
{
|
||||
Assert.Equal(string.Empty, EndpointNormalizer.ToolNameKey("\r\n "));
|
||||
}
|
||||
}
|
||||
@@ -12,6 +12,16 @@ public static class EndpointNormalizer
|
||||
@"[0-9a-fA-F]{8}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{4}-[0-9a-fA-F]{12}",
|
||||
RegexOptions.Compiled);
|
||||
|
||||
// Same classes LogSanitizer strips: C0/C1 control characters (Table Storage rejects them in
|
||||
// keys) plus the Unicode line/paragraph separators a log viewer breaks a line on.
|
||||
private static readonly Regex ControlChars = new(@"[\p{Cc}\p{Zl}\p{Zp}]", RegexOptions.Compiled);
|
||||
|
||||
/// <summary>
|
||||
/// Cap for the caller-supplied tool-name prefix of a usage row key. Real MCP tool names are
|
||||
/// well under this; the cap only bounds what an arbitrary header can push into a key.
|
||||
/// </summary>
|
||||
public const int MaxToolNameLength = 64;
|
||||
|
||||
public static string Normalize(string path)
|
||||
{
|
||||
// Strip /api/ prefix
|
||||
@@ -22,9 +32,32 @@ public static class EndpointNormalizer
|
||||
// Replace GUIDs with {id}
|
||||
normalized = GuidPattern.Replace(normalized, "{id}");
|
||||
|
||||
// Replace characters that are illegal in Azure Table Storage keys (/, \, #, ?)
|
||||
normalized = normalized.Replace('/', '_').Replace('\\', '_').Replace('#', '_').Replace('?', '_');
|
||||
|
||||
return normalized.ToLowerInvariant();
|
||||
return ReplaceIllegalKeyChars(normalized).ToLowerInvariant();
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Makes the <c>X-MCP-Tool-Name</c> header value safe to use as the prefix of a usage row key.
|
||||
/// The header is caller-supplied (the MCP server sets it to the real tool name, but any
|
||||
/// authenticated MCP-source caller can send it), and the key ends up in the CAS-conflict warning
|
||||
/// of the usage counter: control characters and line separators are removed, the characters Table
|
||||
/// Storage rejects in keys are replaced, and the length is capped. A real tool name
|
||||
/// (<c>get_session_events</c>) passes through unchanged, so existing usage rows keep their keys.
|
||||
/// Returns an empty string when nothing key-safe remains; callers then skip the prefix.
|
||||
/// </summary>
|
||||
public static string ToolNameKey(string? toolName)
|
||||
{
|
||||
if (string.IsNullOrEmpty(toolName))
|
||||
return string.Empty;
|
||||
|
||||
// Explicit CR/LF removal first — the log-injection barrier the CodeQL taint model
|
||||
// recognizes; the regex pass then drops any remaining control character or separator.
|
||||
var cleaned = toolName.Replace("\r", string.Empty).Replace("\n", string.Empty);
|
||||
cleaned = ControlChars.Replace(cleaned, string.Empty);
|
||||
cleaned = ReplaceIllegalKeyChars(cleaned).Trim();
|
||||
return cleaned.Length <= MaxToolNameLength ? cleaned : cleaned[..MaxToolNameLength];
|
||||
}
|
||||
|
||||
/// <summary>Replaces the characters Azure Table Storage rejects in keys (/, \, #, ?).</summary>
|
||||
private static string ReplaceIllegalKeyChars(string value)
|
||||
=> value.Replace('/', '_').Replace('\\', '_').Replace('#', '_').Replace('?', '_');
|
||||
}
|
||||
|
||||
@@ -354,8 +354,9 @@ public class McpQuotaEnforcementMiddleware : IFunctionsWorkerMiddleware
|
||||
private void TrackUsage(HttpContext httpContext, string oid, string? upn, string homeTenantId, IReadOnlyCollection<string> chargeTenantIds)
|
||||
{
|
||||
var normalizedEndpoint = EndpointNormalizer.Normalize(httpContext.Request.Path.Value ?? string.Empty);
|
||||
var mcpToolName = httpContext.Request.Headers["X-MCP-Tool-Name"].FirstOrDefault();
|
||||
if (!string.IsNullOrEmpty(mcpToolName))
|
||||
// Key-safe copy of the header: the value becomes part of the UserUsageLog row key.
|
||||
var mcpToolName = EndpointNormalizer.ToolNameKey(httpContext.Request.Headers["X-MCP-Tool-Name"].FirstOrDefault());
|
||||
if (mcpToolName.Length > 0)
|
||||
normalizedEndpoint = $"{mcpToolName}:{normalizedEndpoint}";
|
||||
|
||||
var repo = _userUsageRepo;
|
||||
|
||||
Reference in New Issue
Block a user