diff --git a/docs/backend/oobe-bootstrap-go-flow.md b/docs/backend/oobe-bootstrap-go-flow.md index cd03268a..16a2257e 100644 --- a/docs/backend/oobe-bootstrap-go-flow.md +++ b/docs/backend/oobe-bootstrap-go-flow.md @@ -84,14 +84,14 @@ serve identical scripts during the transition. # Rate limiting behind Front Door The endpoint self-limits per client IP (`bootstrap-script:{ip}`, 20/min via -`ClientIpExtractor.GetTrustedClientIp`) — anonymous routes are skipped by +`ClientIpExtractor.GetRateLimitClientIp`) — anonymous routes are skipped by `UserRateLimitMiddleware` and must bring their own limiter. Because traffic -arrives through Front Door, the rightmost X-Forwarded-For hop may resolve to -AFD egress IPs; whether buckets collapse (the shared-bucket 429 incident class) -must be verified post-deploy from two networks. If they do, the fix is an -AFD-aware extraction honoring `X-Azure-ClientIP` only when `X-Azure-FDID` -matches our profile — NOT `GetClientEgressIp`, which trusts the header -unconditionally and is documented as unfit for rate-limit keys. +arrives through Front Door, the rightmost X-Forwarded-For hop resolves to AFD +egress IPs and buckets would collapse (the shared-bucket 429 incident class); +the AFD-aware extraction therefore honors `X-Azure-ClientIP` only when +`X-Azure-FDID` matches the `FrontDoorProfileId` app setting (fail-closed to +the trusted-hop path otherwise). Verified post-deploy 2026-07-29: real client +IPs are keyed, not AFD egress. Front Door route caching is DISABLED for this route: the body inlines the bearer token, so a cache hit would replay one requester's token to another. diff --git a/src/Backend/AutopilotMonitor.Functions.Tests/DistressValidationTests.cs b/src/Backend/AutopilotMonitor.Functions.Tests/DistressValidationTests.cs index 6e6865e1..8807e793 100644 --- a/src/Backend/AutopilotMonitor.Functions.Tests/DistressValidationTests.cs +++ b/src/Backend/AutopilotMonitor.Functions.Tests/DistressValidationTests.cs @@ -120,6 +120,77 @@ public class DistressValidationTests Assert.Equal("203.0.113.5", ClientIpExtractor.ExtractTrustedHop("10.0.0.1, 203.0.113.5, ")); } + // ========================================================================= + // ClientIpExtractor.GetHeaderFromBindingDataJson — Flex Consumption strips the + // X-Forwarded-* family from the proxied HTTP request; the host passes the + // ORIGINAL header set as trigger metadata (BindingData["Headers"], JSON object). + // Malformed metadata must fail soft (null), never throw into request handling. + // ========================================================================= + + [Fact] + public void GetHeaderFromBindingDataJson_XffPresent_ReturnsValue() + { + var json = """{"Host":"example.com","X-Forwarded-For":"1.1.1.1, 203.0.113.5:443"}"""; + Assert.Equal( + "1.1.1.1, 203.0.113.5:443", + ClientIpExtractor.GetHeaderFromBindingDataJson(json, "X-Forwarded-For")); + } + + [Fact] + public void GetHeaderFromBindingDataJson_HeaderNameCaseInsensitive() + { + var json = """{"x-forwarded-for":"203.0.113.5"}"""; + Assert.Equal( + "203.0.113.5", + ClientIpExtractor.GetHeaderFromBindingDataJson(json, "X-Forwarded-For")); + } + + [Theory] + [InlineData(null)] + [InlineData("")] + [InlineData("not json")] + [InlineData("[]")] // JSON but not an object + [InlineData("""{"Host":"example.com"}""")] // header absent + [InlineData("""{"X-Forwarded-For":42}""")] // non-string value + public void GetHeaderFromBindingDataJson_MissingOrMalformed_ReturnsNull(string? json) + { + Assert.Null(ClientIpExtractor.GetHeaderFromBindingDataJson(json, "X-Forwarded-For")); + } + + // ========================================================================= + // ClientIpExtractor.RemoteIpToString — transport-level fallback. On Flex the + // worker's TCP peer is the host's LOCAL proxy: loopback means "not a client" + // and must map to "unknown", never be stored as a forensic SourceIp + // (production 2026-07-29: distress reports carried SourceIp 127.0.0.1). + // ========================================================================= + + [Fact] + public void RemoteIpToString_Null_ReturnsUnknown() + { + Assert.Equal("unknown", ClientIpExtractor.RemoteIpToString(null)); + } + + [Theory] + [InlineData("127.0.0.1")] + [InlineData("::1")] + [InlineData("::ffff:127.0.0.1")] // IPv4-mapped loopback + public void RemoteIpToString_Loopback_ReturnsUnknown(string ip) + { + Assert.Equal("unknown", ClientIpExtractor.RemoteIpToString(System.Net.IPAddress.Parse(ip))); + } + + [Fact] + public void RemoteIpToString_PublicIpv4_ReturnsIp() + { + Assert.Equal("203.0.113.5", ClientIpExtractor.RemoteIpToString(System.Net.IPAddress.Parse("203.0.113.5"))); + } + + [Fact] + public void RemoteIpToString_Ipv4MappedIpv6_FoldsToIpv4() + { + Assert.Equal("203.0.113.5", ClientIpExtractor.RemoteIpToString(System.Net.IPAddress.Parse("::ffff:203.0.113.5"))); + } + // ========================================================================= // Sanitize — Control character stripping and truncation // ========================================================================= diff --git a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/ReportDistressFunction.cs b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/ReportDistressFunction.cs index f77c64d8..5368e662 100644 --- a/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/ReportDistressFunction.cs +++ b/src/Backend/AutopilotMonitor.Functions/Functions/Ingest/ReportDistressFunction.cs @@ -118,19 +118,15 @@ namespace AutopilotMonitor.Functions.Functions.Ingest return req.CreateResponse(HttpStatusCode.OK); // Gate 4: Rate limiting (IP + tenant + global circuit breaker). - // ClientIpExtractor returns the rightmost X-Forwarded-For hop (the one - // App Service appended) — never the leftmost, which is caller-controlled - // and would let a single attacker rotate past the per-IP throttle. - var clientIp = ClientIpExtractor.GetTrustedClientIp(req); + // One extraction serves both the bucket key and the stored forensic + // SourceIp: rightmost X-Forwarded-For hop (host-appended, never the + // caller-controlled leftmost), with X-Azure-ClientIP honored only behind + // a verified Front Door FDID — direct callers cannot spoof either value. + var clientIp = ClientIpExtractor.GetRateLimitClientIp(req); var rateLimitResult = _rateLimitService.Check(clientIp, tenantId); if (!rateLimitResult.IsAllowed) return req.CreateResponse(HttpStatusCode.OK); - // For the STORED forensic SourceIp, prefer the real client egress IP. The trusted - // hop above (used as the un-spoofable rate-limit key) is Front Door's own egress IP - // behind Front Door, not the device — which is why stored SourceIp was unusable. - var sourceIp = ClientIpExtractor.GetClientEgressIp(req); - // Gate 5: Tenant existence check (cached — cheap O(1) lookup) var (_, exists) = await _tenantConfigService.TryGetConfigurationAsync(tenantId); if (!exists) @@ -207,7 +203,7 @@ namespace AutopilotMonitor.Functions.Functions.Ingest Message = message, AgentTimestamp = report.Timestamp, IngestedAt = DateTime.UtcNow, - SourceIp = sourceIp, + SourceIp = clientIp, CertSourceState = certSourceState, CertThumbprint = certThumbprint, CertSubject = certSubject, @@ -318,7 +314,7 @@ namespace AutopilotMonitor.Functions.Functions.Ingest ["HttpStatusCode"] = report.HttpStatusCode?.ToString() ?? string.Empty, ["Message"] = message ?? string.Empty, ["AgentTimestamp"] = report.Timestamp.ToString("O"), - ["SourceIp"] = sourceIp ?? string.Empty, + ["SourceIp"] = clientIp, ["CertSourceState"] = certSourceState ?? string.Empty, ["CertThumbprint"] = certThumbprint ?? string.Empty, ["CertSubject"] = certSubject ?? string.Empty, diff --git a/src/Backend/AutopilotMonitor.Functions/Security/ClientIpExtractor.cs b/src/Backend/AutopilotMonitor.Functions/Security/ClientIpExtractor.cs index 15a89c9a..8c72269b 100644 --- a/src/Backend/AutopilotMonitor.Functions/Security/ClientIpExtractor.cs +++ b/src/Backend/AutopilotMonitor.Functions/Security/ClientIpExtractor.cs @@ -1,6 +1,8 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Net; +using System.Text.Json; using Microsoft.Azure.Functions.Worker; using Microsoft.Azure.Functions.Worker.Http; @@ -36,28 +38,94 @@ namespace AutopilotMonitor.Functions.Security if (hop != Unknown) return hop; } - // Flex Consumption does not reliably surface X-Forwarded-For to the isolated - // worker (observed 2026-07-28: every anonymous caller keyed as "unknown", - // collapsing all per-IP rate-limit buckets into one). The transport-level - // peer address is unspoofable and equals what the rightmost XFF hop would - // carry when no proxy chain exists; behind a fronting proxy it is that - // proxy's egress — coarse but honest, per the model above. + // Flex Consumption strips the X-Forwarded-* family from the HTTP request the + // host proxies to the isolated worker (Azure/azure-functions-host#10253/#11254; + // observed 2026-07-28: every anonymous caller keyed as "unknown"). The host does + // pass the ORIGINAL header set out-of-band as trigger metadata, so recover the + // rightmost XFF hop from there — same trust model, different transport. + var bindingHop = ExtractTrustedHop( + GetHeaderFromBindingDataJson(TryGetBindingHeadersJson(req), "X-Forwarded-For")); + if (bindingHop != Unknown) return bindingHop; + + // Last resort: the transport-level peer address — unspoofable, and equal to the + // rightmost XFF hop when no proxy chain exists. On Flex the worker's peer is the + // host's local proxy, so loopback means "not a client" and must not be stored or + // used as a bucket key (a stored 127.0.0.1 is forensically misleading). var remote = req.FunctionContext.GetHttpContext()?.Connection.RemoteIpAddress; - if (remote == null) return Unknown; - if (remote.IsIPv4MappedToIPv6) remote = remote.MapToIPv4(); - return remote.ToString(); + return RemoteIpToString(remote); } /// - /// Rate-limit key for endpoints reached through Azure Front Door (e.g. - /// go.autopilotmonitor.com → /api/bootstrap/go). Behind AFD the trusted hop is - /// AFD's egress IP, so all callers would share a handful of buckets — one abuser - /// could starve every legitimate technician. AFD carries the true client IP in - /// X-Azure-ClientIP; it is honored ONLY when the request provably traversed OUR - /// profile: X-Azure-FDID (which AFD overwrites on pass-through, so a client - /// cannot inject it across AFD) must equal the FrontDoorProfileId app setting. - /// Fail-closed: unset setting, missing/mismatched/multi-valued FDID, or empty - /// client IP all fall back to . + /// Normalizes a transport-level peer address into a client-IP string: + /// null and loopback (the Functions host's local proxy on Flex) yield + /// ; IPv4-mapped IPv6 is folded back to IPv4. + /// + internal static string RemoteIpToString(IPAddress? remote) + { + if (remote == null) return Unknown; + if (remote.IsIPv4MappedToIPv6) remote = remote.MapToIPv4(); + if (IPAddress.IsLoopback(remote)) return Unknown; + return remote.ToString(); + } + + private static string? TryGetBindingHeadersJson(HttpRequestData req) + { + try + { + return req.FunctionContext.BindingContext.BindingData.TryGetValue("Headers", out var raw) + ? raw as string + : null; + } + catch + { + // Fail-soft: absent/odd trigger metadata must never break request handling. + return null; + } + } + + /// + /// Reads one header (case-insensitive) from the JSON object the Functions host + /// places in BindingData["Headers"] — the request headers as the HOST saw them, + /// before the worker-bound proxy hop drops the X-Forwarded-* family. + /// Returns null for missing/malformed input. + /// + internal static string? GetHeaderFromBindingDataJson(string? headersJson, string headerName) + { + if (string.IsNullOrEmpty(headersJson)) return null; + try + { + using var doc = JsonDocument.Parse(headersJson); + if (doc.RootElement.ValueKind != JsonValueKind.Object) return null; + foreach (var prop in doc.RootElement.EnumerateObject()) + { + if (string.Equals(prop.Name, headerName, StringComparison.OrdinalIgnoreCase) + && prop.Value.ValueKind == JsonValueKind.String) + { + return prop.Value.GetString(); + } + } + return null; + } + catch (JsonException) + { + return null; + } + } + + /// + /// Finest-grained trustworthy client IP — used both as the rate-limit key for + /// endpoints reached through Azure Front Door (e.g. go.autopilotmonitor.com → + /// /api/bootstrap/go) and as the stored forensic SourceIp (distress reports). + /// Behind AFD the trusted hop is AFD's egress IP, so all callers would share a + /// handful of buckets — one abuser could starve every legitimate technician. AFD + /// carries the true client IP in X-Azure-ClientIP; it is honored ONLY when the + /// request provably traversed OUR profile: X-Azure-FDID (which AFD overwrites on + /// pass-through, so a client cannot inject it across AFD) must equal the + /// FrontDoorProfileId app setting. The same gate keeps stored forensic IPs + /// unspoofable when the Function App is reached directly, where anyone could + /// send an X-Azure-ClientIP header. Fail-closed: unset setting, + /// missing/mismatched/multi-valued FDID, or empty client IP all fall back to + /// . /// public static string GetRateLimitClientIp(HttpRequestData? req) { @@ -96,28 +164,6 @@ namespace AutopilotMonitor.Functions.Security return single != null && string.Equals(single, expected, StringComparison.OrdinalIgnoreCase); } - /// - /// Best-effort REAL client egress IP for forensic/diagnostic STORAGE (e.g. distress-report - /// SourceIp, audit fields). NOT for rate-limit keys — use - /// there, because the value below is spoofable when no trusted proxy populates it. - /// - /// Behind Azure Front Door the trusted rightmost X-Forwarded-For hop is Front Door's own - /// egress IP, not the device — so a stored SourceIp taken from it is useless for identifying - /// origin. Front Door sets the true client IP in X-Azure-ClientIP; prefer that and - /// fall back to the trusted hop when absent (no Front Door in front). - /// - /// - public static string GetClientEgressIp(HttpRequestData? req) - { - if (req == null) return Unknown; - if (req.Headers.TryGetValues("X-Azure-ClientIP", out var azValues)) - { - var ip = ExtractTrustedHop(string.Join(",", azValues)); - if (ip != Unknown) return ip; - } - return GetTrustedClientIp(req); - } - /// /// Takes the rightmost non-empty entry from an X-Forwarded-For header value and /// strips any port suffix or IPv6 brackets. Returns when diff --git a/src/Web/autopilot-monitor-web/lib/__tests__/hostRouting.guard.test.ts b/src/Web/autopilot-monitor-web/lib/__tests__/hostRouting.guard.test.ts new file mode 100644 index 00000000..15a30689 --- /dev/null +++ b/src/Web/autopilot-monitor-web/lib/__tests__/hostRouting.guard.test.ts @@ -0,0 +1,136 @@ +import { describe, it, expect } from "vitest"; +import { existsSync, readdirSync, readFileSync } from "node:fs"; +import { join } from "node:path"; +import { PUBLIC_PATH_PREFIXES, isPublicPath } from "../hostRouting"; + +/** + * Guards the public/portal split in hostRouting.ts. + * + * Failure mode this pins (prod incident 2026-07-29): /sla was listed as a + * public path while its page renders ProtectedRoute. On the public host + * ProtectedRoute stands down, so the page waited forever for a sign-in — + * a silent dead-end no unit test covered. + * + * (a) No route whose page (or any ancestor layout) renders ProtectedRoute + * may ever satisfy isPublicPath. + * (b) Every entry in PUBLIC_PATH_PREFIXES must exist somewhere real — an + * unprotected app page, an app-root metadata file, a public/ asset, or + * a staticwebapp.config.json route. Catches typos and stale entries. + * (c) Behavioral pins for isPublicPath's matching rules. + */ + +const WEB_ROOT = join(__dirname, "..", ".."); +const APP_DIR = join(WEB_ROOT, "app"); +const PUBLIC_DIR = join(WEB_ROOT, "public"); + +interface AppPage { + route: string; + isProtected: boolean; +} + +/** Walks the app router tree; a page is protected when its page.tsx or any + * ancestor layout.tsx renders ProtectedRoute. Route groups "(...)" add no + * path segment. */ +function collectPages( + dir: string, + segments: string[], + ancestorProtected: boolean, + acc: AppPage[], +): AppPage[] { + const layoutPath = join(dir, "layout.tsx"); + const chainProtected = + ancestorProtected || + (existsSync(layoutPath) && readFileSync(layoutPath, "utf-8").includes("ProtectedRoute")); + + const pagePath = join(dir, "page.tsx"); + if (existsSync(pagePath)) { + acc.push({ + route: segments.length === 0 ? "/" : "/" + segments.join("/"), + isProtected: chainProtected || readFileSync(pagePath, "utf-8").includes("ProtectedRoute"), + }); + } + + for (const entry of readdirSync(dir, { withFileTypes: true })) { + if (!entry.isDirectory()) continue; + const nextSegments = + entry.name.startsWith("(") && entry.name.endsWith(")") + ? segments + : [...segments, entry.name]; + collectPages(join(dir, entry.name), nextSegments, chainProtected, acc); + } + return acc; +} + +const pages = collectPages(APP_DIR, [], false, []); +const pageByRoute = new Map(pages.map((p) => [p.route, p])); + +const swaRoutes: string[] = ( + JSON.parse(readFileSync(join(WEB_ROOT, "staticwebapp.config.json"), "utf-8")) as { + routes: Array<{ route: string }>; + } +).routes.map((r) => r.route); + +describe("hostRouting public/portal guard", () => { + it("detects the app router tree and the ProtectedRoute marker (self-check)", () => { + // If either collapses to zero the guards below would pass vacuously. + expect(pages.length).toBeGreaterThan(20); + const sla = pageByRoute.get("/sla"); + expect(sla, "/sla page missing — incident pin needs updating").toBeTruthy(); + expect(sla!.isProtected, "/sla no longer renders ProtectedRoute — update this guard").toBe(true); + }); + + it("no ProtectedRoute page is classified as public", () => { + const violations = pages + .filter((p) => p.isProtected && isPublicPath(p.route)) + .map((p) => p.route); + expect( + violations, + `Portal page(s) listed as public — on www ProtectedRoute stands down and the page dead-ends (the /sla incident): ${violations.join(", ")}`, + ).toEqual([]); + }); + + it("every public prefix maps to something real and unprotected", () => { + const appRootFiles = readdirSync(APP_DIR); + for (const prefix of PUBLIC_PATH_PREFIXES) { + const page = pageByRoute.get(prefix); + if (page) { + expect( + page.isProtected, + `${prefix} is in PUBLIC_PATH_PREFIXES but its page renders ProtectedRoute`, + ).toBe(false); + continue; + } + // Not an app page: must be app-root metadata (robots.ts, icon.svg, + // opengraph-image.png, …), a public/ asset, or a SWA redirect/rewrite. + const base = prefix.slice(1); + const metadataName = base.split(".")[0]; + const exists = + appRootFiles.some((f) => f.startsWith(metadataName + ".")) || + existsSync(join(PUBLIC_DIR, base)) || + swaRoutes.includes(prefix) || + swaRoutes.includes(prefix + "/*"); + expect( + exists, + `${prefix} is in PUBLIC_PATH_PREFIXES but resolves to no app page, metadata file, public/ asset, or staticwebapp.config.json route`, + ).toBe(true); + } + }); + + it("pins isPublicPath matching semantics", () => { + // The landing page is public; the incident route is portal. + expect(isPublicPath("/")).toBe(true); + expect(isPublicPath("/sla")).toBe(false); + expect(isPublicPath("/sla/anything")).toBe(false); + // Core portal surface stays portal. + for (const r of ["/dashboard", "/sessions", "/settings", "/admin/metrics", "/progress"]) { + expect(isPublicPath(r), `${r} must be portal`).toBe(false); + } + // Prefix matching: subpaths and generated asset variants match… + expect(isPublicPath("/about")).toBe(true); + expect(isPublicPath("/docs/setup")).toBe(true); + expect(isPublicPath("/icon-192.png")).toBe(true); + expect(isPublicPath("/opengraph-image.png")).toBe(true); + // …but sibling names that merely share a prefix string do not. + expect(isPublicPath("/aboutx")).toBe(false); + }); +});