Enforcement-core hardening (#41), stacked on the cleanup bundle

packages/explicabl/__tests__/hardening.test.ts

@@ -0,0 +1,77 @@
+// Enforcement-core hardening (#41) — explicabl.
+//
+// - audit events must carry identity from the GatewayContext (ALS), which the
+//   old code never read (it looked at res.locals.gatewayContext, unset).
+// - aborted requests (connection close, no `finish`) must still emit exactly
+//   one audit event.
+// - the /webhooks/auth0 mount must call the handler FACTORY (the old code
+//   mounted the factory itself, so requests hung forever).
+
+import { describe, it, expect, vi } from "vitest";
+import { EventEmitter } from "node:events";
+import express from "express";
+import request from "supertest";
+import { runWithGatewayContext } from "@gatewaystack/request-context";
+import { explicablLoggingMiddleware, explicablRouter } from "../src/index.ts";
+import type { ExplicablEvent } from "../src/index.ts";
+
+// Minimal req/res doubles so we can drive `finish` / `close` directly.
+function fakeReqRes() {
+  const req: any = { method: "GET", path: "/x", headers: {} };
+  const res: any = new EventEmitter();
+  res.locals = {};
+  res.statusCode = 200;
+  return { req, res };
+}
+
+describe("explicablLoggingMiddleware: identity comes from the GatewayContext", () => {
+  it("reads identity from the ALS context, not res.locals", async () => {
+    const logger = vi.fn();
+    const { req, res } = fakeReqRes();
+
+    runWithGatewayContext({ identity: { sub: "auth0|abc", source: "auth0" } }, () => {
+      explicablLoggingMiddleware(logger)(req, res, () => {});
+    });
+    res.emit("finish");
+
+    expect(logger).toHaveBeenCalledTimes(1);
+    const event = logger.mock.calls[0][0] as ExplicablEvent;
+    expect((event.context as any)?.identity?.sub).toBe("auth0|abc");
+  });
+});
+
+describe("explicablLoggingMiddleware: aborted requests still audit, exactly once", () => {
+  it("emits one event on `close` when the request was aborted (no finish)", () => {
+    const logger = vi.fn();
+    const { req, res } = fakeReqRes();
+    explicablLoggingMiddleware(logger)(req, res, () => {});
+
+    res.emit("close"); // client aborted; `finish` never fired
+    expect(logger).toHaveBeenCalledTimes(1);
+  });
+
+  it("emits exactly one event when both finish and close fire", () => {
+    const logger = vi.fn();
+    const { req, res } = fakeReqRes();
+    explicablLoggingMiddleware(logger)(req, res, () => {});
+
+    res.emit("finish");
+    res.emit("close");
+    expect(logger).toHaveBeenCalledTimes(1);
+  });
+});
+
+describe("explicablRouter: /webhooks/auth0 is wired (does not hang)", () => {
+  it("responds to the webhook route instead of mounting the factory", async () => {
+    const app = express();
+    app.use(express.json());
+    app.use(explicablRouter(process.env));
+
+    // Without LOG_WEBHOOK_SECRET the handler refuses (503/401) — the point is
+    // that it RESPONDS at all. The old code mounted the factory as middleware,
+    // so this request never got a response.
+    const res = await request(app).post("/webhooks/auth0").send({}).timeout(3000);
+    expect(res.status).toBeGreaterThanOrEqual(400);
+    expect(res.status).toBeLessThan(600);
+  });
+});

packages/explicabl/package.json

@@ -22,6 +22,7 @@
     "node": ">=18"
   },
   "dependencies": {
+    "@gatewaystack/request-context": "^0.0.6",
     "express": "^4.22.0",
     "express-rate-limit": "^8.2.2"
   },

packages/explicabl/src/index.ts

@@ -7,6 +7,7 @@ import {
   type NextFunction,
   type RequestHandler,
 } from "express";
+import { getGatewayContext } from "@gatewaystack/request-context";
 import { healthRoutes } from "./health.js";
 import { auth0LogsWebhook } from "./webhooks/auth0LogWebhook.js";
 
@@ -87,27 +88,59 @@ export function createConsoleLogger(
  * It is intentionally defensive: if no context is present, it still logs
  * method/path/status/latency.
  */
+let warnedNoContext = false;
+
 export function explicablLoggingMiddleware(
   logger: ExplicablLogger,
 ): RequestHandler {
   return (req: Request, res: Response, next: NextFunction) => {
     const startedAt = Date.now();
 
-    res.on("finish", () => {
-      const latencyMs = Date.now() - startedAt;
+    // Capture the ambient GatewayContext reference now, while we are inside the
+    // request's async-local-storage scope. Later layers (identifiabl, etc.)
+    // mutate this same object in place, so the reference sees their updates —
+    // and reading it here means we still have it if ALS has unwound by the time
+    // the response settles.
+    const capturedContext = getGatewayContext();
+
+    // Emit exactly one event whether the response finishes normally or the
+    // connection is aborted. `finish` fires on a completed response; `close`
+    // fires on client abort/socket close and used to emit nothing at all — so
+    // aborted requests left no audit trail. The guard makes them mutually
+    // exclusive.
+    let emitted = false;
+    const emit = () => {
+      if (emitted) return;
+      emitted = true;
 
-      // Try a few common places where you might stash requestId/context.
+      const latencyMs = Date.now() - startedAt;
       const locals: any = res.locals ?? {};
       const requestId: string | undefined =
         (locals.requestId as string | undefined) ??
         (locals.reqId as string | undefined) ??
         (req.headers["x-request-id"] as string | undefined);
 
+      // Identity lives in the GatewayContext (ALS), not res.locals — the old
+      // `res.locals.gatewayContext` read was always undefined, so every audit
+      // line logged `context: undefined`. Prefer the live context, fall back to
+      // the captured reference, then res.locals for back-compat.
       const context: unknown =
+        getGatewayContext() ??
+        capturedContext ??
         locals.gatewayContext ??
         locals.context ??
         undefined;
 
+      if (context === undefined && !warnedNoContext) {
+        warnedNoContext = true;
+        // eslint-disable-next-line no-console
+        console.warn(
+          "[explicabl] no GatewayContext found on the request — audit events " +
+            "will carry no identity. Ensure runWithGatewayContext() wraps the " +
+            "request before this middleware.",
+        );
+      }
+
       const event: ExplicablEvent = {
         ts: new Date().toISOString(),
         kind: "gateway.request",
@@ -128,7 +161,10 @@ export function explicablLoggingMiddleware(
         // eslint-disable-next-line no-console
         console.error("[explicabl:logger_error]", err);
       }
-    });
+    };
+
+    res.on("finish", emit);
+    res.on("close", emit);
 
     next();
   };
@@ -149,12 +185,12 @@ export function explicablRouter(env: NodeJS.ProcessEnv): RequestHandler {
   // Health routes (public)
   r.use(healthRoutes(env) as unknown as RequestHandler);
 
-  // Webhooks (auth0 logs, etc.)
-  // NOTE: auth0LogsWebhook is already a RequestHandler in your current code,
-  // so we do NOT call it as a function here.
-  // NOTE: auth0LogsWebhook is already a RequestHandler in your current code,
-  // so we do NOT call it as a function here.
-  r.use("/webhooks/auth0", auth0LogsWebhook as unknown as RequestHandler);
+  // Webhooks (auth0 logs, etc.).
+  // auth0LogsWebhook is a FACTORY that returns the handler — it must be called.
+  // Mounting the factory itself made Express invoke it as (req, res, next); it
+  // returned a value and never called next(), so every request to this path
+  // hung forever and the webhook was effectively dead.
+  r.use("/webhooks/auth0", auth0LogsWebhook());
 
   // Important: cast router to RequestHandler
   return r as unknown as RequestHandler;