fix(mcp-security): host-spoof fail-fast, auth-fetch timeout, layered rate-limit + bounded cache

Addresses the 3 findings from the 2026-06-30 MCP security review (v1.4.3 -> 1.4.4).

M3 host-header spoofing: centralize getPublicBaseUrl into config.ts (deletes the
duplicated copies in oauth.ts + access-guard.ts) and fail-fast at boot in production
when MCP_PUBLIC_URL is unset, instead of silently trusting caller-supplied
X-Forwarded-Host. Safe for the stage-1 deploy (minReplicas=0 -> no container runs).

M2 access-check hang: add AbortSignal.timeout (default 15s, env-overridable) to the
/api/auth/mcp fetch -- it was the only fetch without a timeout, on the path every
request traverses. Timeout falls through to the existing fail-closed catch.

M1 pre-auth flood + unbounded cache:
- boundedSet caps accessCache (default 10k, oldest-evict) -> no OOM under a
  distinct-token flood on the 0.5GiB container.
- per-source-IP pre-auth limiter (isPreAuthRateLimited, default 120/min) on the
  cache-MISS path only, keyed on IP not UPN so a forged token never burns a
  victim's per-UPN budget. Kept LAYERED with the existing per-UPN limiter
  (IP for pre-auth, identity for authenticated -- the SaaS-standard split).
- app.set('trust proxy', 1) for a spoof-resistant req.ip (single Envoy hop).
- parsePositiveInt fixes the RATE_LIMIT NaN-disables-limiter bug and guards all
  new tunables; cleanup interval prunes the pre-auth bucket map.

Monitoring/UX: both 429 paths go through respondRateLimited -> structured greppable
log (kind=pre-auth|upn), Retry-After header, and an actionable body message.

Tests: parsePositiveInt, boundedSet, isPreAuthRateLimited, getPublicBaseUrl
fallback (security-guards.test.ts) + MCP_PUBLIC_URL pin (config-public-url.test.ts).
tsc clean; vitest 327 passed / 41 skipped.

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

src/McpServer/autopilot-monitor-mcp/package.json

@@ -1,6 +1,6 @@
 {
   "name": "autopilot-monitor-mcp",
-  "version": "1.4.3",
+  "version": "1.4.4",
   "description": "MCP server for Autopilot-Monitor — enables AI agents to query enrollment sessions, device properties, events and vulnerabilities",
   "type": "module",
   "bin": {

src/McpServer/autopilot-monitor-mcp/src/__tests__/config-public-url.test.ts

@@ -0,0 +1,22 @@
+/**
+ * M3 — MCP_PUBLIC_URL pin path.
+ *
+ * config.ts captures MCP_PUBLIC_URL into a module-level const at import time, so
+ * the pin-wins behaviour must be exercised in its own file with the env var set
+ * BEFORE the dynamic import resolves. The forwarded-header fallback (pin unset)
+ * is covered in security-guards.test.ts.
+ */
+import { describe, it, expect } from 'vitest';
+
+process.env.MCP_PUBLIC_URL = 'https://pinned.example.net';
+const { getPublicBaseUrl } = await import('../config.js');
+
+describe('M3 — getPublicBaseUrl honors the MCP_PUBLIC_URL pin', () => {
+  it('returns the pin verbatim and ignores spoofable forwarded headers', () => {
+    const req = {
+      headers: { 'x-forwarded-proto': 'https', 'x-forwarded-host': 'attacker.tld', host: 'attacker.tld' },
+      protocol: 'http',
+    } as unknown as Parameters<typeof getPublicBaseUrl>[0];
+    expect(getPublicBaseUrl(req)).toBe('https://pinned.example.net');
+  });
+});

src/McpServer/autopilot-monitor-mcp/src/__tests__/security-guards.test.ts

@@ -294,7 +294,12 @@ describe('L3 — error-handler 5xx body redaction', () => {
 // 2026-05-08 security review fixes
 // ---------------------------------------------------------------------------
 
-const { buildCacheKey } = await import('../access-guard.js');
+// Pin a small pre-auth budget BEFORE access-guard loads (it reads the env at
+// module init) so the throttle tests are cheap and limit-independent.
+const PRE_AUTH_TEST_LIMIT = 5;
+process.env.MCP_PRE_AUTH_RATE_LIMIT_PER_MINUTE = String(PRE_AUTH_TEST_LIMIT);
+const { buildCacheKey, boundedSet, isPreAuthRateLimited } = await import('../access-guard.js');
+const { parsePositiveInt, getPublicBaseUrl } = await import('../config.js');
 
 describe('F1 — access-guard cache key includes token hash', () => {
   it('produces stable keys for the same (upn, token) pair', () => {
@@ -476,3 +481,108 @@ describe('list_tenants — extractTenantList keep-list projection', () => {
   });
 });
 
+// ---------------------------------------------------------------------------
+// 2026-06-30 security review fixes (M1 / M2 / M3)
+// ---------------------------------------------------------------------------
+
+describe('M1 — parsePositiveInt env guard (rate-limit NaN bug)', () => {
+  it('returns the fallback for an undefined env var', () => {
+    expect(parsePositiveInt(undefined, 60)).toBe(60);
+  });
+
+  it('parses a valid positive integer', () => {
+    expect(parsePositiveInt('120', 60)).toBe(120);
+  });
+
+  it('returns the fallback for non-numeric input (would be NaN — disables limiter)', () => {
+    // The actual bug: a bare parseInt('abc') is NaN, and `count >= NaN` is
+    // always false, silently disabling the rate limiter.
+    expect(parsePositiveInt('abc', 60)).toBe(60);
+    expect(parsePositiveInt('', 60)).toBe(60);
+  });
+
+  it('returns the fallback for zero and negative values', () => {
+    expect(parsePositiveInt('0', 60)).toBe(60);
+    expect(parsePositiveInt('-5', 60)).toBe(60);
+  });
+});
+
+describe('M1 — boundedSet size cap (accessCache memory bound)', () => {
+  it('evicts the oldest entry once the cap is reached', () => {
+    const m = new Map<string, number>();
+    boundedSet(m, 'a', 1, 2);
+    boundedSet(m, 'b', 2, 2);
+    boundedSet(m, 'c', 3, 2); // over cap → evict 'a' (oldest)
+    expect(m.size).toBe(2);
+    expect(m.has('a')).toBe(false);
+    expect(m.has('b')).toBe(true);
+    expect(m.has('c')).toBe(true);
+  });
+
+  it('updates an existing key in place without evicting or growing', () => {
+    const m = new Map<string, number>();
+    boundedSet(m, 'a', 1, 2);
+    boundedSet(m, 'b', 2, 2);
+    boundedSet(m, 'a', 99, 2); // re-set existing key — must not evict 'b'
+    expect(m.size).toBe(2);
+    expect(m.get('a')).toBe(99);
+    expect(m.has('b')).toBe(true);
+  });
+
+  it('stays bounded under a flood far exceeding the cap', () => {
+    const m = new Map<string, number>();
+    for (let i = 0; i < 1000; i++) boundedSet(m, `k${i}`, i, 10);
+    expect(m.size).toBe(10);
+    // Only the most recent 10 keys survive.
+    expect(m.has('k999')).toBe(true);
+    expect(m.has('k989')).toBe(false);
+  });
+});
+
+describe('M1 — isPreAuthRateLimited (per-IP pre-auth throttle)', () => {
+  // Budget pinned to PRE_AUTH_TEST_LIMIT above. Use a unique IP per test so the
+  // module-level bucket map does not leak across cases.
+  let ipSeed = 0;
+  const uniqueIp = () => `203.0.113.${ipSeed++}`;
+
+  it('allows requests up to the budget, then throttles', () => {
+    const ip = uniqueIp();
+    let throttledAt = -1;
+    for (let i = 0; i < PRE_AUTH_TEST_LIMIT + 5; i++) {
+      if (isPreAuthRateLimited(ip)) { throttledAt = i; break; }
+    }
+    // The (limit+1)th call is the first to exceed the budget.
+    expect(throttledAt).toBe(PRE_AUTH_TEST_LIMIT);
+  });
+
+  it('tracks distinct IPs independently', () => {
+    const a = uniqueIp();
+    const b = uniqueIp();
+    // Burn a's budget; b must be unaffected.
+    for (let i = 0; i < PRE_AUTH_TEST_LIMIT; i++) isPreAuthRateLimited(a);
+    expect(isPreAuthRateLimited(a)).toBe(true);
+    expect(isPreAuthRateLimited(b)).toBe(false);
+  });
+});
+
+describe('M3 — getPublicBaseUrl forwarded-header fallback (no MCP_PUBLIC_URL pin)', () => {
+  // This file is imported with MCP_PUBLIC_URL unset and NODE_ENV != production,
+  // so the config module captured no pin and getPublicBaseUrl derives from
+  // forwarded headers. The pin-wins path is covered in config-public-url.test.ts.
+  it('prefers X-Forwarded-Host / X-Forwarded-Proto', () => {
+    const req = {
+      headers: { 'x-forwarded-proto': 'https', 'x-forwarded-host': 'mcp.example.com', host: 'internal:8080' },
+      protocol: 'http',
+    } as unknown as Parameters<typeof getPublicBaseUrl>[0];
+    expect(getPublicBaseUrl(req)).toBe('https://mcp.example.com');
+  });
+
+  it('falls back to req.protocol + Host when no forwarded headers', () => {
+    const req = {
+      headers: { host: 'localhost:8080' },
+      protocol: 'http',
+    } as unknown as Parameters<typeof getPublicBaseUrl>[0];
+    expect(getPublicBaseUrl(req)).toBe('http://localhost:8080');
+  });
+});
+

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

@@ -11,20 +11,42 @@ import type { Request, Response, NextFunction } from 'express';
 import crypto from 'node:crypto';
 import { extractTokenClaims, isTokenExpired } from './auth.js';
 import { runWithCaller } from './client.js';
-import { API_BASE_URL } from './config.js';
+import { API_BASE_URL, getPublicBaseUrl, parsePositiveInt } from './config.js';
 
 const BASE_URL = API_BASE_URL;
 
-/**
- * Derives the public base URL for the WWW-Authenticate header.
- */
-function getPublicBaseUrl(req: Request): string {
-  if (process.env.MCP_PUBLIC_URL) return process.env.MCP_PUBLIC_URL;
-  const proto = (req.headers['x-forwarded-proto'] as string) ?? req.protocol;
-  const host = (req.headers['x-forwarded-host'] as string) ?? req.headers.host;
-  return `${proto}://${host}`;
-}
-const RATE_LIMIT = parseInt(process.env.MCP_RATE_LIMIT_PER_MINUTE ?? '60', 10);
+// Per-UPN request budget (sliding 60s window), applied AFTER a token is
+// validated. parsePositiveInt guards against a non-numeric override: a bare
+// parseInt would yield NaN, and `count >= NaN` is always false — silently
+// disabling the limiter.
+const RATE_LIMIT = parsePositiveInt(process.env.MCP_RATE_LIMIT_PER_MINUTE, 60);
+
+// Per-source-IP budget for UNvalidated requests (sliding 60s window), applied
+// only on the cache-MISS path that would call the backend. Bounds forged- /
+// distinct-token floods at the source without ever touching a victim's per-UPN
+// budget (different key space). It counts ONLY cache misses — a legitimate user
+// misses at most ~once per token per 5-min TTL (~0.2/min), and a NAT'd org of N
+// active users ~N/5 per min — so the default 120 (= 2× the per-UPN rate) clears
+// even a ~600-user shared-IP org while capping flood amplification to ~2 backend
+// calls/sec/IP. NOTE: this is per-replica; with maxReplicas=1 it is effectively
+// global, but if the app is ever scaled out the effective per-IP ceiling becomes
+// 120×replicas (a shared store would be needed for a true global cap). Tune via
+// MCP_PRE_AUTH_RATE_LIMIT_PER_MINUTE; raise it (or allowlist the egress IP) only
+// if a very large shared-IP org reports false 429s.
+const PRE_AUTH_RATE_LIMIT = parsePositiveInt(process.env.MCP_PRE_AUTH_RATE_LIMIT_PER_MINUTE, 120);
+
+// Hard ceiling on access-cache cardinality. The key is UPN+tokenHash, so a flood
+// of distinct (valid-signature) tokens would otherwise grow the map unbounded
+// between the 5-min reaper passes — a memory-exhaustion vector on the 0.5 GiB
+// container. Oldest entries are evicted on write once the cap is hit.
+const MAX_ACCESS_CACHE_ENTRIES = parsePositiveInt(process.env.MCP_ACCESS_CACHE_MAX_ENTRIES, 10_000);
+
+// Timeout for the backend access-check fetch. Without it a hung/cold backend
+// stalls the path EVERY request must traverse. Generous enough to survive a
+// cold Functions backend (→ no false 403s), bounded so an unresponsive backend
+// cannot pile up held requests.
+const ACCESS_CHECK_TIMEOUT_MS = parsePositiveInt(process.env.MCP_ACCESS_CHECK_TIMEOUT_MS, 15_000);
+
 const ACCESS_CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
 
 // --- Access check cache ---
@@ -65,6 +87,10 @@ interface AccessCheckResult {
   // only the former should be surfaced to the user as "you are not enabled —
   // ask to be whitelisted"; an infra error is not the user's fault.
   infraError: boolean;
+  // Set when the per-source-IP pre-auth budget was exhausted on a cache-miss:
+  // the middleware maps this to 429 (not 403), since it is throttling, not a
+  // verdict. Never set on a cache hit (a hit consumes no pre-auth budget).
+  rateLimited?: boolean;
 }
 
 export function buildCacheKey(upn: string, token: string): string {
@@ -72,7 +98,21 @@ export function buildCacheKey(upn: string, token: string): string {
   return `${upn}:${h}`;
 }
 
-async function checkAccess(upn: string, token: string): Promise<AccessCheckResult> {
+/**
+ * Insert into a size-bounded Map, evicting the oldest entry (insertion order)
+ * when the cap is reached. Keeps memory bounded under a distinct-key flood.
+ * Exported for unit testing. Re-inserting an existing key updates in place
+ * without growing the map (the size check is skipped when the key already exists).
+ */
+export function boundedSet<K, V>(map: Map<K, V>, key: K, value: V, cap: number): void {
+  if (!map.has(key) && map.size >= cap) {
+    const oldest = map.keys().next().value as K | undefined;
+    if (oldest !== undefined) map.delete(oldest);
+  }
+  map.set(key, value);
+}
+
+async function checkAccess(upn: string, token: string, clientIp: string): Promise<AccessCheckResult> {
   const cacheKey = buildCacheKey(upn, token);
   const cached = accessCache.get(cacheKey);
   if (cached && Date.now() < cached.expiresAt) {
@@ -89,9 +129,19 @@ async function checkAccess(upn: string, token: string): Promise<AccessCheckResul
     };
   }
 
+  // Cache miss → this request is about to hit the backend. Bound the cost per
+  // source IP so a forged-/distinct-token flood cannot drive unbounded backend
+  // calls (the JWT signature is not verified here, so anyone can mint a
+  // well-formed token). Keyed on IP, not UPN, so it never burns a victim's
+  // per-UPN budget.
+  if (isPreAuthRateLimited(clientIp)) {
+    return { allowed: false, reason: 'Pre-auth rate limit exceeded', isGlobalAdmin: false, isGlobalReader: false, infraError: false, rateLimited: true };
+  }
+
   try {
     const res = await fetch(`${BASE_URL}/api/auth/mcp`, {
       headers: { Authorization: `Bearer ${token}` },
+      signal: AbortSignal.timeout(ACCESS_CHECK_TIMEOUT_MS),
     });
 
     const text = await res.text();
@@ -133,7 +183,8 @@ async function checkAccess(upn: string, token: string): Promise<AccessCheckResul
     // Cache only the persisted fields (infraError is request-derived, always
     // false for a cached verdict — see the cache-hit path above). The delegated
     // scope MUST be cached too, or the 5-min-cached follow-up loses it.
-    accessCache.set(cacheKey, {
+    // boundedSet caps the map so a distinct-token flood cannot exhaust memory.
+    boundedSet(accessCache, cacheKey, {
       allowed: result.allowed,
       reason: result.reason,
       isGlobalAdmin: result.isGlobalAdmin,
@@ -141,7 +192,7 @@ async function checkAccess(upn: string, token: string): Promise<AccessCheckResul
       delegatedTenantIds: result.delegatedTenantIds,
       delegatedRole: result.delegatedRole,
       expiresAt: Date.now() + ACCESS_CACHE_TTL_MS,
-    });
+    }, MAX_ACCESS_CACHE_ENTRIES);
     return result;
   } catch (err) {
     console.error(`[access-guard] Backend check failed for ${upn}:`, err);
@@ -156,46 +207,102 @@ interface RateEntry {
   timestamps: number[];
 }
 
+// Post-validation, per-UPN budget.
 const rateBuckets = new Map<string, RateEntry>();
+// Pre-validation, per-source-IP budget (cache-miss path only). Separate key
+// space from rateBuckets so throttling unvalidated floods never affects a
+// validated user's per-UPN budget.
+const preAuthBuckets = new Map<string, RateEntry>();
 
-// Cleanup stale entries every 5 minutes. Both maps share this interval —
-// rateBuckets evicts empty windows, accessCache evicts expired entries
-// (the cache key now includes a token hash, so cardinality is one entry per
+/** Drop timestamps outside the 60s window from a bucket map; delete empty buckets. */
+function pruneBuckets(buckets: Map<string, RateEntry>, cutoff: number): void {
+  for (const [key, entry] of buckets) {
+    entry.timestamps = entry.timestamps.filter((t) => t > cutoff);
+    if (entry.timestamps.length === 0) buckets.delete(key);
+  }
+}
+
+// Cleanup stale entries every 5 minutes. All three maps share this interval —
+// the rate buckets evict empty windows, accessCache evicts expired entries
+// (the cache key includes a token hash, so cardinality is one entry per
 // user×token-rotation; without proactive eviction the map would only shrink
-// when a key is re-fetched).
+// when a key is re-fetched or the size cap evicts it).
 setInterval(() => {
   const now = Date.now();
   const cutoff = now - 60_000;
-  for (const [key, entry] of rateBuckets) {
-    entry.timestamps = entry.timestamps.filter((t) => t > cutoff);
-    if (entry.timestamps.length === 0) rateBuckets.delete(key);
-  }
+  pruneBuckets(rateBuckets, cutoff);
+  pruneBuckets(preAuthBuckets, cutoff);
   for (const [key, entry] of accessCache) {
     if (entry.expiresAt <= now) accessCache.delete(key);
   }
 }, 5 * 60_000);
 
-function isRateLimited(upn: string): boolean {
+/**
+ * Sliding-window check against a bucket map. Returns true when the key has
+ * already used its full budget in the last 60s; otherwise records the hit.
+ */
+function isWindowExceeded(buckets: Map<string, RateEntry>, key: string, limit: number): boolean {
   const now = Date.now();
   const windowStart = now - 60_000;
 
-  let entry = rateBuckets.get(upn);
+  let entry = buckets.get(key);
   if (!entry) {
     entry = { timestamps: [] };
-    rateBuckets.set(upn, entry);
+    buckets.set(key, entry);
   }
-
-  // Remove timestamps outside the window
   entry.timestamps = entry.timestamps.filter((t) => t > windowStart);
 
-  if (entry.timestamps.length >= RATE_LIMIT) {
-    return true;
-  }
-
+  if (entry.timestamps.length >= limit) return true;
   entry.timestamps.push(now);
   return false;
 }
 
+/**
+ * Per-source-IP throttle for UNvalidated, cache-missing requests. Exported for
+ * unit testing. 'unknown' (no resolvable IP) shares a single bucket — acceptable
+ * since the limit is generous and this is a backstop, not the primary gate.
+ */
+export function isPreAuthRateLimited(clientIp: string): boolean {
+  return isWindowExceeded(preAuthBuckets, clientIp, PRE_AUTH_RATE_LIMIT);
+}
+
+/**
+ * Best-effort client IP for the pre-auth throttle. With `app.set('trust proxy', 1)`
+ * (index.ts) and the single Container Apps Envoy ingress hop, req.ip is the real
+ * client address and is not spoofable by prepending X-Forwarded-For entries.
+ * Falls back to the socket address, then 'unknown'.
+ */
+function getClientIp(req: Request): string {
+  return req.ip ?? req.socket?.remoteAddress ?? 'unknown';
+}
+
+function isRateLimited(upn: string): boolean {
+  return isWindowExceeded(rateBuckets, upn, RATE_LIMIT);
+}
+
+/**
+ * Emit a 429 with the conventional Retry-After header, a structured + greppable
+ * log line, and an actionable message. `kind` distinguishes the per-source-IP
+ * pre-auth throttle from the per-UPN throttle so a Log Analytics alert can break
+ * the two out and so an unexpected false-positive surfaces to the user (who is
+ * told to contact the administrator).
+ */
+function respondRateLimited(res: Response, kind: 'pre-auth' | 'upn', key: string, rpcMethod: string): void {
+  console.error(`[mcp-auth] 429 kind=${kind} key=${key} method=${rpcMethod}`);
+  res.setHeader('Retry-After', '60');
+  res.status(429).json({
+    error: 'Rate limit exceeded',
+    retryAfterSeconds: 60,
+    message:
+      kind === 'pre-auth'
+        ? 'Too many requests from your network in a short window. Wait ~60s and retry. If this keeps ' +
+          'happening unexpectedly, you may be sharing an IP via a corporate proxy/NAT — contact the MCP ' +
+          'server administrator.'
+        : 'You are sending requests faster than the per-user limit. Wait ~60s and retry. If this keeps ' +
+          'happening unexpectedly, contact the MCP server administrator.',
+  });
+}
+
 // --- Express middleware ---
 
 export function accessGuard(req: Request, res: Response, next: NextFunction): void {
@@ -237,19 +344,25 @@ export function accessGuard(req: Request, res: Response, next: NextFunction): vo
   }
 
   const upn = claims.upn.toLowerCase();
+  const clientIp = getClientIp(req);
 
-  // Access check (async — calls backend, cached). Must run BEFORE rate-limit
-  // accounting: the JWT signature is not verified here (only the backend has
-  // JWKS access), so an attacker can forge an unsigned token with a victim's
-  // UPN. If rate-limit incremented before validation, those forgeries would
-  // burn the victim's per-minute budget and 429 their legitimate calls.
-  // checkAccess delegates signature verification to the backend; both
-  // allow- and deny-decisions are cached for 5 min keyed on UPN+tokenHash,
-  // so a forged token cannot piggyback on a legitimate user's cached
-  // `allowed: true`, and forged-token floods cost one backend call per
-  // distinct token.
-  checkAccess(upn, token)
+  // Access check (async — calls backend, cached). Must run BEFORE the per-UPN
+  // rate-limit accounting: the JWT signature is not verified here (only the
+  // backend has JWKS access), so an attacker can forge an unsigned token with a
+  // victim's UPN. If the per-UPN limiter incremented before validation, those
+  // forgeries would burn the victim's per-minute budget and 429 their
+  // legitimate calls. checkAccess delegates signature verification to the
+  // backend; both allow- and deny-decisions are cached for 5 min keyed on
+  // UPN+tokenHash, so a forged token cannot piggyback on a legitimate user's
+  // cached `allowed: true`. On a cache MISS, checkAccess additionally applies a
+  // per-source-IP throttle (keyed on clientIp, not UPN) so a distinct-token
+  // flood cannot drive unbounded backend calls.
+  checkAccess(upn, token, clientIp)
     .then((result) => {
+      if (result.rateLimited) {
+        respondRateLimited(res, 'pre-auth', clientIp, rpcMethod);
+        return;
+      }
       if (!result.allowed) {
         if (result.infraError) {
           // Backend could not reach a verdict (unreachable / malformed). Fail
@@ -279,7 +392,7 @@ export function accessGuard(req: Request, res: Response, next: NextFunction): vo
       // else's UPN was already rejected above; only a real, allow-listed
       // user reaches this counter.
       if (isRateLimited(upn)) {
-        res.status(429).json({ error: 'Rate limit exceeded', retryAfterSeconds: 60 });
+        respondRateLimited(res, 'upn', upn, rpcMethod);
         return;
       }
 

src/McpServer/autopilot-monitor-mcp/src/config.ts

@@ -9,6 +9,8 @@
  * fast in production rather than defaulting (matching the fail-fast posture in
  * oauth.ts).
  */
+import type { Request } from 'express';
+
 const DEFAULT_API_BASE_URL = 'https://autopilotmonitor-api.azurewebsites.net';
 
 function resolveApiBaseUrl(): string {
@@ -25,3 +27,58 @@ function resolveApiBaseUrl(): string {
 
 /** Resolved backend API base URL (no trailing slash). */
 export const API_BASE_URL = resolveApiBaseUrl();
+
+/**
+ * Parse a positive-integer environment variable, falling back to `fallback`
+ * for missing, non-numeric, or non-positive values. A bare `parseInt` returns
+ * NaN for garbage input, and `NaN` silently breaks every `>=` comparison it
+ * feeds — e.g. an unguarded rate-limit cap of NaN makes `count >= NaN` always
+ * false, disabling the limiter entirely. Centralized so every tunable (rate
+ * limits, cache caps, timeouts) is validated the same way.
+ */
+export function parsePositiveInt(raw: string | undefined, fallback: number): number {
+  if (raw === undefined) return fallback;
+  const n = Number.parseInt(raw, 10);
+  return Number.isFinite(n) && n > 0 ? n : fallback;
+}
+
+/**
+ * Public base URL of this MCP server, used verbatim for OAuth issuer /
+ * WWW-Authenticate / redirect metadata.
+ *
+ * Pinned via MCP_PUBLIC_URL. When unset, it is derived from the
+ * X-Forwarded-Host / X-Forwarded-Proto headers set by the Container Apps
+ * ingress — but those headers are caller-supplied and spoofable, so a request
+ * with a forged Host could make the server advertise an attacker-chosen issuer
+ * / redirect / WWW-Authenticate metadata URL. In production we therefore REFUSE
+ * to start without the pin (mirrors the AUTOPILOT_API_URL fail-fast above).
+ *
+ * This does not break the documented two-stage deploy (infra/mcp-server.bicep):
+ * the stage-1 deploy that exists only to emit the containerAppUrl runs with
+ * minReplicas=0, so no container boots and this check never fires; the operator
+ * then re-deploys with mcpPublicUrl pinned before any traffic arrives.
+ */
+const MCP_PUBLIC_URL = process.env.MCP_PUBLIC_URL?.trim() || undefined;
+if (!MCP_PUBLIC_URL && process.env.NODE_ENV === 'production') {
+  throw new Error(
+    'MCP_PUBLIC_URL must be set in production — refusing to derive the OAuth issuer / ' +
+    'redirect_uri / WWW-Authenticate metadata from caller-supplied X-Forwarded-Host headers ' +
+    '(host-spoofing risk). Pin it via the bicep `mcpPublicUrl` parameter (two-stage deploy).',
+  );
+}
+
+export { MCP_PUBLIC_URL };
+
+/**
+ * Derives the public base URL of this MCP server for the current request.
+ * Prefers the MCP_PUBLIC_URL pin (production); falls back to the forwarded
+ * headers only in dev, where the boot-time guard above is not enforced.
+ * Typed structurally on the two fields it reads so config.ts needs no runtime
+ * express dependency.
+ */
+export function getPublicBaseUrl(req: Pick<Request, 'headers' | 'protocol'>): string {
+  if (MCP_PUBLIC_URL) return MCP_PUBLIC_URL;
+  const proto = (req.headers['x-forwarded-proto'] as string) ?? req.protocol;
+  const host = (req.headers['x-forwarded-host'] as string) ?? (req.headers.host as string);
+  return `${proto}://${host}`;
+}

src/McpServer/autopilot-monitor-mcp/src/index.ts

@@ -26,16 +26,15 @@ const SERVER_VERSION: string = pkg.version;
 const PORT = parseInt(process.env.PORT ?? '8080', 10);
 const RULES_DIR = process.env.RULES_DIR ?? resolve(__dirname, '..', '..', '..', '..', 'rules');
 
-// Surface the MCP_PUBLIC_URL state at boot so a missing public-URL pin is
-// visible in container logs, not silently fallen back to forwarded-headers
-// in production. Two-stage deploy expectation: first deploy → containerAppUrl
-// output → second deploy with mcpPublicUrl pinned. See infra/mcp-server.bicep.
+// Surface the MCP_PUBLIC_URL state at boot. In production a missing pin is a
+// hard boot failure (config.ts throws — host-spoofing defense); this dev-only
+// notice flags the forwarded-header fallback. Two-stage deploy expectation:
+// first deploy → containerAppUrl output → second deploy with mcpPublicUrl
+// pinned. See infra/mcp-server.bicep.
 if (!process.env.MCP_PUBLIC_URL) {
   console.error(
     '[startup] MCP_PUBLIC_URL is not set — OAuth issuer / WWW-Authenticate / ' +
-    'redirect metadata will be derived from X-Forwarded-* headers. This is ' +
-    'acceptable for local dev only; in production, set MCP_PUBLIC_URL to the ' +
-    'Container App FQDN (re-deploy with the bicep `mcpPublicUrl` parameter).',
+    'redirect metadata will be derived from X-Forwarded-* headers (dev only).',
   );
 }
 
@@ -182,6 +181,13 @@ function createMcpServer(ga: boolean, strictGa: boolean, delegated: boolean, man
 
 const app = express();
 
+// Single Container Apps Envoy ingress hop sits in front of this server. Trusting
+// exactly one proxy makes req.ip the real downstream client address (the entry
+// the ingress appended to X-Forwarded-For), and NOT spoofable by a client
+// prepending its own X-Forwarded-For entries — which the per-source-IP pre-auth
+// throttle in access-guard relies on.
+app.set('trust proxy', 1);
+
 // Tight body-size limit for /oauth/register, registered BEFORE the global
 // parser so the smaller limit wins (the global parser's body-already-parsed
 // short-circuit then skips re-parsing). RFC 7591 registration requests carry

src/McpServer/autopilot-monitor-mcp/src/oauth.ts

@@ -18,6 +18,7 @@
  */
 import { Router } from 'express';
 import crypto from 'node:crypto';
+import { getPublicBaseUrl } from './config.js';
 
 const CLIENT_ID: string = (() => {
   const v = process.env.AUTOPILOT_ENTRA_CLIENT_ID;
@@ -342,19 +343,6 @@ export function redirectUriMatches(registered: string[], requested: string): boo
   return registered.some((r) => canonicalizeRedirectUri(r) === target);
 }
 
-/**
- * Derives the public base URL of this MCP server.
- * In production: from X-Forwarded-Host / X-Forwarded-Proto (set by Container Apps ingress).
- * Fallback: MCP_PUBLIC_URL env var or localhost.
- */
-function getPublicBaseUrl(req: import('express').Request): string {
-  if (process.env.MCP_PUBLIC_URL) return process.env.MCP_PUBLIC_URL;
-
-  const proto = (req.headers['x-forwarded-proto'] as string) ?? req.protocol;
-  const host = (req.headers['x-forwarded-host'] as string) ?? req.headers.host;
-  return `${proto}://${host}`;
-}
-
 export function createOAuthRouter(): Router {
   const router = Router();