Fix/rest low severity nits 92

mcp/src/bin/http.ts

@@ -9,6 +9,7 @@ interface BinConfig {
   maxSessions: number;
   publicUrl?: string;
   authorizationServerUrl?: string;
+  trustProxy: boolean | number | string;
 }
 
 class ConfigError extends Error {}
@@ -32,6 +33,21 @@ function parsePort(name: string, raw: string, def: number): number {
   return n;
 }
 
+// parseTrustProxy maps E2A_TRUST_PROXY to the value Express's `trust
+// proxy` setting expects. Default "loopback": only a same-host proxy (the
+// prod Caddy front, forwarding over localhost) is trusted for
+// X-Forwarded-* headers. "true"/"false" become booleans; a bare integer
+// is a hop count (Express reads a numeric *string* as a subnet, so it must
+// be converted); anything else passes through as a preset name
+// ("loopback"/"uniquelocal"/...) or a CSV of IPs/subnets.
+function parseTrustProxy(raw: string): boolean | number | string {
+  if (raw === "") return "loopback";
+  if (raw === "true") return true;
+  if (raw === "false") return false;
+  if (/^\d+$/.test(raw)) return Number(raw);
+  return raw;
+}
+
 function parseHostList(raw: string, def: string): string[] {
   const source = raw === "" ? def : raw;
   const list = source.split(",").map((s) => s.trim()).filter(Boolean);
@@ -72,6 +88,7 @@ export function loadConfig(env: NodeJS.ProcessEnv = process.env): BinConfig {
     maxSessions: parsePositiveInt("MCP_MAX_SESSIONS", env.MCP_MAX_SESSIONS ?? "", 500),
     publicUrl: env.MCP_PUBLIC_URL || undefined,
     authorizationServerUrl: env.MCP_AUTHORIZATION_SERVER_URL || undefined,
+    trustProxy: parseTrustProxy(env.E2A_TRUST_PROXY ?? ""),
   };
 }
 
@@ -96,6 +113,7 @@ async function main(): Promise<void> {
     maxSessions: cfg.maxSessions,
     publicUrl: cfg.publicUrl,
     authorizationServerUrl: cfg.authorizationServerUrl,
+    trustProxy: cfg.trustProxy,
   });
   process.stderr.write(
     `e2a-mcp-http listening on :${bound} (base ${cfg.baseUrl}, hosts ${cfg.allowedHosts.join(",")})\n`,

mcp/src/http-server.ts

@@ -32,6 +32,17 @@ export interface HttpServerOptions {
    * without changing where the SDK forwards bearer tokens.
    */
   authorizationServerUrl?: string;
+  /**
+   * Express `trust proxy` setting. Controls which hops' `X-Forwarded-*`
+   * headers are honored when computing the externally-visible scheme for
+   * the OAuth discovery URLs (see {@link externalScheme}). Defaults to
+   * `"loopback"`: only a same-host proxy — the prod Caddy front, which
+   * forwards to this server over localhost — is trusted, so an
+   * `X-Forwarded-Proto` spoofed over the public network is ignored.
+   * Accepts any value Express supports: a preset (`"loopback"`,
+   * `"uniquelocal"`), a boolean, a hop count, or a CSV of IPs/subnets.
+   */
+  trustProxy?: boolean | number | string;
   /** Optional shared sessions instance (defaults to a fresh one). */
   sessions?: Sessions;
   /** Session idle timeout. */
@@ -72,6 +83,13 @@ export function buildApp(opts: HttpServerOptions): BuiltApp {
     });
 
   const app = express();
+  // Trust the front proxy so req.protocol reflects X-Forwarded-Proto when
+  // synthesizing the externally-advertised discovery URLs. Default
+  // "loopback" only honors the header from a same-host proxy (the prod
+  // Caddy front, which forwards over localhost); a header arriving over the
+  // public network is from an untrusted hop and is ignored. Must be set
+  // before any route that reads req.protocol.
+  app.set("trust proxy", opts.trustProxy ?? "loopback");
   app.use(express.json({ limit: "1mb" }));
   // CORS open for v0.2; revisit when we have a real allowlist of MCP hosts.
   app.use(cors({ origin: "*", exposedHeaders: ["Mcp-Session-Id"] }));
@@ -121,7 +139,7 @@ export function buildApp(opts: HttpServerOptions): BuiltApp {
     if (!host) return null;
     const bare = host.split(":")[0]!.toLowerCase();
     if (!allowedHosts.has(bare)) return null;
-    return `https://${host}`;
+    return `${externalScheme(req)}://${host}`;
   };
 
   app.get("/.well-known/oauth-protected-resource", (req, res) => {
@@ -153,13 +171,26 @@ export function buildApp(opts: HttpServerOptions): BuiltApp {
   return { app, sessions };
 }
 
+// externalScheme returns the client-facing scheme ("https"/"http") for
+// building the OAuth discovery URLs this server advertises. req.protocol
+// honors X-Forwarded-Proto only for hops trusted via the `trust proxy`
+// setting in buildApp; otherwise it reports the real (plaintext localhost)
+// socket scheme. Behind the prod Caddy front this is the forwarded
+// "https"; for an untrusted/spoofed header it falls back to the actual
+// connection scheme rather than the attacker's claim. Deployments that
+// terminate TLS at a non-loopback proxy set publicUrl or E2A_TRUST_PROXY.
+function externalScheme(req: Request): string {
+  return req.protocol === "https" ? "https" : "http";
+}
+
 // resourceMetadataURL returns the value the `WWW-Authenticate` header
 // should advertise. Honors publicUrl when set (local-dev http), falls
-// back to https+Host otherwise (prod behind Caddy).
+// back to the request's external scheme + Host otherwise (prod behind
+// Caddy resolves to https via X-Forwarded-Proto).
 function resourceMetadataURL(req: Request, opts: HttpServerOptions): string {
   const base = opts.publicUrl
     ? opts.publicUrl.replace(/\/+$/, "")
-    : `https://${req.headers.host}`;
+    : `${externalScheme(req)}://${req.headers.host}`;
   return `${base}/.well-known/oauth-protected-resource`;
 }
 

mcp/src/session.ts

@@ -132,8 +132,16 @@ export class Sessions {
       }
     }
     if (oldestId) {
-      // Fire-and-forget; the LRU eviction shouldn't block put().
-      void this.delete(oldestId);
+      // Fire-and-forget; the LRU eviction shouldn't block put(). Attach a
+      // catch so a rejecting transport.close() surfaces as a logged warning
+      // rather than an unhandled promise rejection (which can terminate the
+      // process under Node's default unhandledRejection behavior) — the
+      // sibling gc()/shutdown() paths get the same protection via
+      // Promise.allSettled.
+      void this.delete(oldestId).catch((err) => {
+        const message = err instanceof Error ? err.message : String(err);
+        process.stderr.write(`session evict close error: ${message}\n`);
+      });
     }
   }
 }

mcp/tests/bin-http.test.ts

@@ -10,6 +10,7 @@ describe("bin/http loadConfig", () => {
       allowedHosts: ["mcp.e2a.dev"],
       sessionIdleMs: 5 * 60_000,
       maxSessions: 500,
+      trustProxy: "loopback",
     });
   });
 
@@ -27,6 +28,7 @@ describe("bin/http loadConfig", () => {
       allowedHosts: ["mcp.e2a.dev", "mcp-staging.e2a.dev"],
       sessionIdleMs: 60_000,
       maxSessions: 100,
+      trustProxy: "loopback",
     });
   });
 
@@ -83,4 +85,24 @@ describe("bin/http loadConfig", () => {
     const cfg = loadConfig({ PORT: "0" });
     expect(cfg.port).toBe(0);
   });
+
+  it("defaults E2A_TRUST_PROXY to loopback", () => {
+    expect(loadConfig({}).trustProxy).toBe("loopback");
+  });
+
+  it("parses E2A_TRUST_PROXY booleans", () => {
+    expect(loadConfig({ E2A_TRUST_PROXY: "true" }).trustProxy).toBe(true);
+    expect(loadConfig({ E2A_TRUST_PROXY: "false" }).trustProxy).toBe(false);
+  });
+
+  it("parses a bare integer E2A_TRUST_PROXY as a hop count", () => {
+    // Express reads a numeric *string* as a subnet, so it must become a
+    // real number to mean "trust N hops".
+    expect(loadConfig({ E2A_TRUST_PROXY: "1" }).trustProxy).toBe(1);
+  });
+
+  it("passes through a preset / subnet E2A_TRUST_PROXY verbatim", () => {
+    expect(loadConfig({ E2A_TRUST_PROXY: "uniquelocal" }).trustProxy).toBe("uniquelocal");
+    expect(loadConfig({ E2A_TRUST_PROXY: "10.0.0.0/8" }).trustProxy).toBe("10.0.0.0/8");
+  });
 });

mcp/tests/http.test.ts

@@ -243,6 +243,48 @@ describe("HTTP MCP server", () => {
     expect(body.bearer_methods_supported).toEqual(["header"]);
   });
 
+  it("discovery scheme falls back to the connection scheme without X-Forwarded-Proto", async () => {
+    const origin = url.replace("/mcp", "");
+    const res = await fetch(`${origin}/.well-known/oauth-protected-resource`);
+    expect(res.status).toBe(200);
+    const body = await res.json();
+    // No forwarded header: advertise the real (plaintext loopback) scheme.
+    expect(body.resource).toBe(origin);
+  });
+
+  it("honors X-Forwarded-Proto from a trusted (loopback) proxy", async () => {
+    // The default server trusts "loopback"; the test client connects over
+    // 127.0.0.1, so its X-Forwarded-Proto is honored — mirroring the prod
+    // Caddy front that terminates TLS and forwards over localhost.
+    const origin = url.replace("/mcp", "");
+    const res = await fetch(`${origin}/.well-known/oauth-protected-resource`, {
+      headers: { "X-Forwarded-Proto": "https" },
+    });
+    expect(res.status).toBe(200);
+    const body = await res.json();
+    expect(body.resource).toBe(origin.replace(/^http:/, "https:"));
+  });
+
+  it("ignores X-Forwarded-Proto when the proxy is not trusted", async () => {
+    // trustProxy:false simulates an untrusted hop (e.g. a header spoofed
+    // over the public network rather than set by the loopback front proxy).
+    // The forwarded scheme must be dropped, not reflected into the resource.
+    await close();
+    const { close: c, port } = await startHttpServer(0, {
+      baseUrl: "http://e2a.local",
+      allowedHosts: ["127.0.0.1", "localhost"],
+      trustProxy: false,
+      clientFactory: () => stub,
+    });
+    close = c;
+    const res = await fetch(`http://127.0.0.1:${port}/.well-known/oauth-protected-resource`, {
+      headers: { "X-Forwarded-Proto": "https" },
+    });
+    expect(res.status).toBe(200);
+    const body = await res.json();
+    expect(body.resource).toBe(`http://127.0.0.1:${port}`);
+  });
+
   it("lists every registered tool after initialize", async () => {
     const { client, transport } = await connect();
     const { tools } = await client.listTools();

mcp/tests/session.test.ts

@@ -59,6 +59,28 @@ describe("Sessions", () => {
     expect(sessions.get("c")).toBeDefined();
   });
 
+  it("LRU eviction tolerates a close() that rejects", async () => {
+    let t = 0;
+    const sessions = new Sessions({ idleTimeoutMs: 60_000, maxSessions: 1, now: () => t });
+    const stderr = vi.spyOn(process.stderr, "write").mockReturnValue(true);
+    const bad = {
+      transport: { close: vi.fn(async () => { throw new Error("boom"); }) },
+      server: {},
+      lastSeen: 0,
+    } as unknown as SessionEntry;
+    sessions.put("bad", bad);
+    t = 10;
+    // Evicting "bad" fire-and-forgets a delete() whose close() rejects; this
+    // must not surface as an unhandled rejection, and the entry is still gone.
+    sessions.put("good", fakeEntry(10));
+    await new Promise((r) => setImmediate(r));
+    expect(bad.transport.close).toHaveBeenCalledOnce();
+    expect(sessions.get("bad")).toBeUndefined();
+    expect(sessions.get("good")).toBeDefined();
+    expect(stderr).toHaveBeenCalledWith(expect.stringContaining("session evict close error: boom"));
+    stderr.mockRestore();
+  });
+
   it("re-putting an existing id does not trigger eviction", () => {
     const sessions = new Sessions({ idleTimeoutMs: 60_000, maxSessions: 1 });
     const a1 = fakeEntry();