fix(security): resolve CodeQL log-forging findings and suppress by-design queries

Bucket B (real fixes) + Bucket A (config) of the CodeQL alert triage.

Log-forging (cs/log-forging, 8 sinks): add Helpers/LogSanitizer that strips
CR/LF and other control characters, and apply it to the logged copy of every
user-controlled value in AuthenticationMiddleware, PolicyEnforcementMiddleware
(Invoke + DecideAsync), McpQuotaEnforcementMiddleware and TableUserUsageRepository.
Only the logged value is sanitized — routing, authorization and storage keys are
untouched, so tracing fidelity for legitimate values (paths, UPNs, GUIDs) is
preserved. Structured logging already isolated the fields; this closes the CRLF
vector as defense-in-depth and clears the query.

Three genuine JS/TS fixes:
- guardValidation.ts: replace the no-op `.replace(/%/g, "%")` with a real
  escapeRegExp() — the author intended to escape; this also closes the latent
  metacharacter gap (js/identity-replacement).
- access-guard.ts: use a `%s` format argument instead of interpolating the UPN
  into the console.error format string (js/tainted-format-string).
- docs-corpus.ts: strip HTML tags with a loop-until-stable stripHtmlTags() so the
  sanitizer is robust and recognized as complete (js/incomplete-multi-char-sanitization).

CodeQL config: filter the two queries that fire structurally on this codebase,
where the flagged behavior is the feature — cs/exposure-of-sensitive-information
(audit/notification logs legitimately record the actor UPN / recipient email) and
cs/cleartext-storage-of-sensitive-information (rate limiters key on the trusted
client IP). Default suites stay active; genuine one-off false positives from other
queries are dismissed individually so those queries stay sharp.

Tests: Backend xUnit 3110 (+9 LogSanitizer), MCP vitest 19 (+1 tag-strip),
Web vitest 49, all tsc clean.

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

.github/codeql/codeql-config.yml

@@ -0,0 +1,31 @@
+# CodeQL configuration.
+#
+# The default query suites stay active — this file only filters out two C# queries
+# that fire structurally on this codebase, where the flagged behavior is the feature,
+# not a defect:
+#
+#   cs/exposure-of-sensitive-information
+#     Audit and operational logs record WHO performed an action (actor UPN / email)
+#     and notification services deliver mail to a recipient address. Writing that
+#     identity to the log / notification IS the purpose of the field. Every such site
+#     is a legitimate, reviewed use — the query cannot distinguish it from a leak.
+#
+#   cs/cleartext-storage-of-sensitive-information
+#     The rate limiters and distress guard use the trusted client IP as their bucket
+#     key. The IP is an identifier for throttling, not a secret at rest; storing it in
+#     the in-memory / table rate-limit key is by design.
+#
+# Genuine one-off false positives from OTHER queries (e.g. an open-redirect the taint
+# model can't see is allow-listed, a rate-limited route it doesn't recognize) are
+# dismissed individually in the Security tab so those queries stay sharp — they are
+# NOT suppressed here.
+#
+# Log-injection (cs/log-forging) is deliberately NOT filtered: user-controlled values
+# are sanitized at the sink via Helpers/LogSanitizer, so the query stays useful for
+# catching a future unsanitized sink.
+
+query-filters:
+  - exclude:
+      id: cs/exposure-of-sensitive-information
+  - exclude:
+      id: cs/cleartext-storage-of-sensitive-information

.github/workflows/codeql.yml

@@ -47,6 +47,7 @@ jobs:
         with:
           languages: ${{ matrix.language }}
           build-mode: ${{ matrix.build-mode }}
+          config-file: .github/codeql/codeql-config.yml
 
       - name: Build
         if: matrix.build-mode == 'manual'

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

@@ -0,0 +1,57 @@
+using AutopilotMonitor.Functions.Helpers;
+
+namespace AutopilotMonitor.Functions.Tests;
+
+public class LogSanitizerTests
+{
+    [Fact]
+    public void Clean_WithCrlf_StripsNewlines()
+    {
+        // The core log-forging vector: a value that injects a fake log line.
+        var forged = "/api/sessions\r\n[Auth Middleware] FAKE: admin granted";
+        var cleaned = LogSanitizer.Clean(forged);
+        Assert.Equal("/api/sessions[Auth Middleware] FAKE: admin granted", cleaned);
+        Assert.DoesNotContain('\r', cleaned!);
+        Assert.DoesNotContain('\n', cleaned!);
+    }
+
+    [Fact]
+    public void Clean_WithBareLineFeedOrCarriageReturn_StripsBoth()
+    {
+        Assert.Equal("ab", LogSanitizer.Clean("a\nb"));
+        Assert.Equal("ab", LogSanitizer.Clean("a\rb"));
+    }
+
+    [Fact]
+    public void Clean_WithOtherControlChars_StripsThem()
+    {
+        // Tab (9), NUL (0) and ESC (27, terminal escape) are all removed. Built
+        // from char codes so the source file carries no literal control bytes.
+        var input = $"a{(char)9}b{(char)0}c{(char)27}de";
+        Assert.Equal("abcde", LogSanitizer.Clean(input));
+    }
+
+    [Theory]
+    [InlineData("/api/sessions/abc-123/events")]
+    [InlineData("user@contoso.com")]
+    [InlineData("11111111-2222-3333-4444-555555555555")]
+    [InlineData("GET")]
+    public void Clean_WithLegitimateValue_ReturnsUnchanged(string value)
+    {
+        // Real paths, UPNs, GUIDs and methods never contain control chars, so
+        // tracing fidelity is fully preserved.
+        Assert.Equal(value, LogSanitizer.Clean(value));
+    }
+
+    [Fact]
+    public void Clean_WithNull_ReturnsNull()
+    {
+        Assert.Null(LogSanitizer.Clean(null));
+    }
+
+    [Fact]
+    public void Clean_WithEmpty_ReturnsEmpty()
+    {
+        Assert.Equal("", LogSanitizer.Clean(""));
+    }
+}

src/Backend/AutopilotMonitor.Functions/DataAccess/TableStorage/TableUserUsageRepository.cs

@@ -1,3 +1,4 @@
+using AutopilotMonitor.Functions.Helpers;
 using AutopilotMonitor.Functions.Security;
 using AutopilotMonitor.Functions.Services;
 using AutopilotMonitor.Shared;
@@ -72,7 +73,7 @@ namespace AutopilotMonitor.Functions.DataAccess.TableStorage
             }
 
             _logger.LogWarning("Failed to increment user usage after {MaxRetries} retries: user={UserId}, endpoint={Endpoint}",
-                maxRetries, userId, endpoint);
+                maxRetries, LogSanitizer.Clean(userId), LogSanitizer.Clean(endpoint));
         }
 
         public async Task<List<UserUsageRecord>> GetUsageByUserAsync(string userId, string? dateFrom = null, string? dateTo = null)

src/Backend/AutopilotMonitor.Functions/Helpers/LogSanitizer.cs

@@ -0,0 +1,33 @@
+using System.Text.RegularExpressions;
+
+namespace AutopilotMonitor.Functions.Helpers;
+
+/// <summary>
+/// Strips control characters from user-controlled values before they reach a log
+/// sink, so a caller cannot forge additional log lines (CRLF injection) or smuggle
+/// terminal escape sequences through the message. Structured logging already stores
+/// each value as a discrete property, so this is defense-in-depth — it also clears
+/// the CodeQL cs/log-forging alerts by removing the newline characters they key on.
+/// Only ever apply to the LOGGED copy of a value; never to a value used for routing,
+/// authorization, or storage keys.
+/// </summary>
+public static class LogSanitizer
+{
+    // \p{Cc} = Unicode "Other, control" (C0 + C1 ranges), which includes CR and LF.
+    private static readonly Regex ControlChars = new(@"\p{Cc}", RegexOptions.Compiled);
+
+    /// <summary>
+    /// Removes CR, LF, and every other control character from <paramref name="value"/>.
+    /// Returns null/empty inputs unchanged.
+    /// </summary>
+    public static string? Clean(string? value)
+    {
+        if (string.IsNullOrEmpty(value))
+            return value;
+
+        // Explicit CR/LF removal first — this is the log-injection barrier the CodeQL
+        // taint model recognizes; the regex pass then drops any remaining control chars.
+        var sanitized = value.Replace("\r", string.Empty).Replace("\n", string.Empty);
+        return ControlChars.Replace(sanitized, string.Empty);
+    }
+}

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

@@ -11,6 +11,7 @@ using Microsoft.AspNetCore.Http;
 using System.Net;
 using System.Security.Claims;
 using AutopilotMonitor.Functions.Security;
+using AutopilotMonitor.Functions.Helpers;
 
 namespace AutopilotMonitor.Functions.Middleware;
 
@@ -57,9 +58,12 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
         // JWT validation. The exempt set is DERIVED from EndpointAccessPolicyCatalog — never a
         // hand-kept parallel list — so a new anonymous/device route can't drift out of sync here.
         var requestPath = httpContext.Request.Path.Value ?? string.Empty;
+        // Sanitized copies for logging only — never used for routing/authorization decisions.
+        var logPath = LogSanitizer.Clean(requestPath);
+        var logMethod = LogSanitizer.Clean(httpContext.Request.Method);
         if (SkipsJwtValidation(httpContext.Request.Method, requestPath))
         {
-            _logger.LogDebug("[Auth Middleware] JWT-exempt route: {Method} {Path}", httpContext.Request.Method, requestPath);
+            _logger.LogDebug("[Auth Middleware] JWT-exempt route: {Method} {Path}", logMethod, logPath);
             await next(context);
             return;
         }
@@ -68,7 +72,7 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
         var authHeader = httpContext.Request.Headers.Authorization.ToString();
         if (string.IsNullOrEmpty(authHeader) || !authHeader.StartsWith("Bearer ", StringComparison.OrdinalIgnoreCase))
         {
-            _logger.LogWarning("[Auth Middleware] Blocked unauthenticated request to {Path}", requestPath);
+            _logger.LogWarning("[Auth Middleware] Blocked unauthenticated request to {Path}", logPath);
             httpContext.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
             httpContext.Response.ContentType = "application/json";
             await httpContext.Response.WriteAsJsonAsync(new
@@ -96,7 +100,7 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
             // MSA, tid 9188040d-…) carries a GUID tid, so this reject is loss-free.
             if (!IsValidTenantId(tenantId))
             {
-                _logger.LogWarning("[Auth Middleware] Rejected token with missing/non-GUID tid for {Path}", requestPath);
+                _logger.LogWarning("[Auth Middleware] Rejected token with missing/non-GUID tid for {Path}", logPath);
                 throw new SecurityTokenValidationException("Token tid claim is missing or not a valid GUID");
             }
 
@@ -207,7 +211,7 @@ public class AuthenticationMiddleware : IFunctionsWorkerMiddleware
 
         if (!authenticated)
         {
-            _logger.LogWarning("[Auth Middleware] Blocked request with invalid token to {Path}", requestPath);
+            _logger.LogWarning("[Auth Middleware] Blocked request with invalid token to {Path}", logPath);
             httpContext.Response.StatusCode = (int)HttpStatusCode.Unauthorized;
             httpContext.Response.ContentType = "application/json";
             await httpContext.Response.WriteAsJsonAsync(new

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

@@ -159,7 +159,7 @@ public class McpQuotaEnforcementMiddleware : IFunctionsWorkerMiddleware
             }
             catch (Exception ex)
             {
-                logger.LogWarning(ex, "[McpQuota] Failed to record usage: user={UserId}, endpoint={Endpoint}", oid, normalizedEndpoint);
+                logger.LogWarning(ex, "[McpQuota] Failed to record usage: user={UserId}, endpoint={Endpoint}", LogSanitizer.Clean(oid), LogSanitizer.Clean(normalizedEndpoint));
             }
         });
     }

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

@@ -57,6 +57,10 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
         var httpMethod = httpContext.Request.Method;
         var queryTenantId = httpContext.Request.Query["tenantId"].FirstOrDefault();
 
+        // Sanitized copies for logging only — never used for routing/authorization decisions.
+        var logPath = LogSanitizer.Clean(requestPath);
+        var logMethod = LogSanitizer.Clean(httpMethod);
+
         // Authenticated principal set by AuthenticationMiddleware (null on anonymous/device routes).
         // Read from FunctionContext.Items (reliable in the isolated worker) rather than httpContext.User.
         ClaimsPrincipal? principal = null;
@@ -76,7 +80,7 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
         }
         catch (Exception ex)
         {
-            _logger.LogError(ex, "[PolicyEnforcement] Service error evaluating policy for {Method} {Path}", httpMethod, requestPath);
+            _logger.LogError(ex, "[PolicyEnforcement] Service error evaluating policy for {Method} {Path}", logMethod, logPath);
             httpContext.Response.StatusCode = (int)HttpStatusCode.ServiceUnavailable;
             httpContext.Response.ContentType = "application/json";
             httpContext.Response.Headers["Retry-After"] = "5";
@@ -92,13 +96,13 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
         {
             context.Items[RequestContext.ItemsKey] = result.Context!;
             _logger.LogDebug("[PolicyEnforcement] ALLOW {Method} {Path} user={User} role={Role}",
-                httpMethod, requestPath, result.UserIdentifier, result.UserRole);
+                logMethod, logPath, LogSanitizer.Clean(result.UserIdentifier), result.UserRole);
             await next(context);
             return;
         }
 
         _logger.LogWarning("[PolicyEnforcement] DENIED {Method} {Path} status={Status} user={User} role={Role} reason={Reason}",
-            httpMethod, requestPath, result.StatusCode, result.UserIdentifier, result.UserRole, result.LogReason);
+            logMethod, logPath, result.StatusCode, LogSanitizer.Clean(result.UserIdentifier), result.UserRole, result.LogReason);
         httpContext.Response.StatusCode = result.StatusCode;
         httpContext.Response.ContentType = "application/json";
         await httpContext.Response.WriteAsJsonAsync(new { error = result.ErrorCode, message = result.ErrorMessage });
@@ -115,12 +119,16 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
     internal async Task<PolicyResult> DecideAsync(
         string httpMethod, string requestPath, string? queryTenantId, ClaimsPrincipal? principal)
     {
+        // Sanitized copies for logging only — never used for routing/authorization decisions.
+        var logPath = LogSanitizer.Clean(requestPath);
+        var logMethod = LogSanitizer.Clean(httpMethod);
+
         var catalogEntry = EndpointAccessPolicyCatalog.FindPolicy(httpMethod, requestPath);
         if (catalogEntry == null)
         {
             // Fail-closed: unregistered route → 403
             _logger.LogError("[PolicyEnforcement] BLOCKED unregistered route: {Method} {Path} — not in catalog (fail-closed)",
-                httpMethod, requestPath);
+                logMethod, logPath);
             return PolicyResult.Deny((int)HttpStatusCode.Forbidden, "Forbidden", "Access denied.", "anonymous", "N/A", "Unregistered");
         }
 
@@ -188,7 +196,7 @@ public class PolicyEnforcementMiddleware : IFunctionsWorkerMiddleware
         if (crossTenant && !hasGlobalScope && !delegatedGrantsRead)
         {
             _logger.LogWarning("[PolicyEnforcement] BLOCKED cross-tenant access: user={User} jwtTenant={JwtTenant} target={Target} scoping={Scoping} path={Path}",
-                decision.UserIdentifier, jwtTenantId, namedTarget, catalogEntry.TenantScoping, requestPath);
+                LogSanitizer.Clean(decision.UserIdentifier), LogSanitizer.Clean(jwtTenantId), LogSanitizer.Clean(namedTarget), catalogEntry.TenantScoping, logPath);
             return PolicyResult.Deny((int)HttpStatusCode.Forbidden, "CrossTenantAccessDenied",
                 "Access denied. You can only access your own tenant's resources.",
                 decision.UserIdentifier, decision.UserRole, "CrossTenant");

src/McpServer/autopilot-monitor-mcp/src/__tests__/docs-corpus.test.ts

@@ -129,6 +129,21 @@ describe('loadDocsCorpus', () => {
     expect(chunks.map((c) => c.metadata.heading)).toContain('General › Is Autopilot Monitor free?');
   });
 
+  it('strips inline HTML tags from a <summary> title, leaving no angle brackets', async () => {
+    const chunks = await withBundle(
+      {
+        'troubleshooting/faq.md': page(
+          'type: FAQ\ntags: [faq]',
+          ['# FAQ', '', '## General', '', '<details>', '', '<summary>Does <b>keep-awake</b> work during <i>ESP</i>?</summary>', '', 'Keep-awake is an opt-in behavior during the user ESP phase for supported devices.', '', '</details>'].join('\n'),
+        ),
+      },
+      (root) => loadDocsCorpus(root),
+    );
+    const headings = chunks.map((c) => c.metadata.heading);
+    expect(headings).toContain('General › Does keep-awake work during ESP?');
+    expect(headings.every((h) => h == null || (!h.includes('<') && !h.includes('>')))).toBe(true);
+  });
+
   it('never emits a heading from inside a fenced code block', async () => {
     const chunks = await withBundle(
       {

src/McpServer/autopilot-monitor-mcp/src/access-guard.ts

@@ -232,7 +232,7 @@ async function checkAccess(upn: string, token: string, clientIp: string): Promis
     }, MAX_ACCESS_CACHE_ENTRIES);
     return result;
   } catch (err) {
-    console.error(`[access-guard] Backend check failed for ${upn}:`, err);
+    console.error('[access-guard] Backend check failed for %s:', upn, err);
     // Fail-closed: deny on backend error
     return { allowed: false, reason: 'Backend access check unavailable', isGlobalAdmin: false, isGlobalReader: false, infraError: true };
   }

src/McpServer/autopilot-monitor-mcp/src/docs-corpus.ts

@@ -217,6 +217,24 @@ interface Section {
   content: string;
 }
 
+/**
+ * Remove HTML tags from a string. The pattern is applied repeatedly until the
+ * string stops changing, so tag removal can never re-form a tag from the joined
+ * remainder. A single `.replace(/<[^>]+>/g, '')` happens to be complete for this
+ * pattern, but the looped form is the shape CodeQL accepts as a robust sanitizer
+ * (js/incomplete-multi-character-sanitization) and is defensive against any
+ * future change to the tag regex.
+ */
+function stripHtmlTags(input: string): string {
+  let prev: string;
+  let out = input;
+  do {
+    prev = out;
+    out = out.replace(/<[^>]+>/g, '');
+  } while (out !== prev);
+  return out;
+}
+
 /**
  * Split a page body into heading-delimited sections.
  *
@@ -262,7 +280,7 @@ function splitIntoSections(body: string): Section[] {
     const summary = /^\s*<summary>(.*?)<\/summary>\s*$/i.exec(line);
     if (summary) {
       flush(detailsTitle ?? h3 ?? h2);
-      detailsTitle = summary[1].replace(/<[^>]+>/g, '').trim();
+      detailsTitle = stripHtmlTags(summary[1]).trim();
       continue;
     }
     if (/^\s*<\/details>\s*$/i.test(line)) {

src/Web/autopilot-monitor-web/utils/guardValidation.ts

@@ -65,13 +65,19 @@ const ALLOWED_USER_PROFILE_SUBDIRS = ["AppData\\Local", "AppData\\Roaming"];
 // Helpers
 // ---------------------------------------------------------------------------
 
+/** Escape a literal string so it can be embedded in a RegExp without its
+ *  characters being interpreted as regex metacharacters. */
+function escapeRegExp(literal: string): string {
+  return literal.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
+}
+
 /** Expand known Windows environment variables and custom tokens (case-insensitive). */
 function expandEnvVars(path: string): string {
   let result = path;
   // Expand custom agent token first
-  result = result.replace(new RegExp(USER_PROFILE_TOKEN.replace(/%/g, "%"), "gi"), USER_PROFILE_PLACEHOLDER);
+  result = result.replace(new RegExp(escapeRegExp(USER_PROFILE_TOKEN), "gi"), USER_PROFILE_PLACEHOLDER);
   for (const [envVar, replacement] of Object.entries(COMMON_ENV_VARS)) {
-    const regex = new RegExp(envVar.replace(/%/g, "%"), "gi");
+    const regex = new RegExp(escapeRegExp(envVar), "gi");
     result = result.replace(regex, replacement);
   }
   return result;