feat(webapp,rbac): REQUIRE_PLUGINS=1 fail-fast for required plugin loads [TRI-9852]

.server-changes/require-plugins-fail-fast.md

@@ -0,0 +1,8 @@
+---
+area: webapp
+type: feature
+---
+
+Add `REQUIRE_PLUGINS=1` env var. When set, the RBAC plugin loader throws instead of silently falling back to the default implementation if the plugin module fails to load (missing, broken transitive dep, etc.). The webapp's `/healthcheck` route now resolves the lazy plugin controller so the throw surfaces during readiness probes — a deploy where the plugin didn't load fails the probe and is rolled back.
+
+Self-hosters leave `REQUIRE_PLUGINS` unset and continue to use the fallback when no plugin is installed.

apps/webapp/app/routes/healthcheck.tsx

@@ -1,14 +1,24 @@
 import { prisma } from "~/db.server";
 import type { LoaderFunction } from "@remix-run/node";
 import { env } from "~/env.server";
+import { rbac } from "~/services/rbac.server";
 
 export const loader: LoaderFunction = async ({ request }) => {
   try {
+    // Resolve the lazy plugin controller so plugin-load failures surface
+    // during readiness probes. With REQUIRE_PLUGINS=1, a failed plugin
+    // load throws here and the rollout's readiness probe fails. The
+    // fallback path doesn't touch the DB, so this runs even when
+    // HEALTHCHECK_DATABASE_DISABLED=1 — REQUIRE_PLUGINS protection must
+    // not be silently bypassed by the DB-disabled flag.
+    await rbac.isUsingPlugin();
+
     if (env.HEALTHCHECK_DATABASE_DISABLED === "1") {
       return new Response("OK");
     }
 
     await prisma.$queryRaw`SELECT 1`;
+
     return new Response("OK");
   } catch (error: unknown) {
     console.log("healthcheck ❌", { error });

apps/webapp/server.ts

@@ -188,10 +188,18 @@ if (ENABLE_CLUSTER && cluster.isPrimary) {
         })
       );
     } else {
-      // we need to do the health check here at /healthcheck
-      app.get("/healthcheck", (req, res) => {
-        res.status(200).send("OK");
-      });
+      // we need to do the health check here at /healthcheck — forward
+      // to the Remix handler so the loader's readiness checks (DB ping,
+      // REQUIRE_PLUGINS-gated plugin load) run in this mode too. A
+      // static 200 here would silently mask a failed plugin load.
+      app.get(
+        "/healthcheck",
+        // @ts-ignore
+        createRequestHandler({
+          build,
+          mode: MODE,
+        })
+      );
     }
 
     const server = app.listen(port, () => {

apps/webapp/test/healthcheck-require-plugins.e2e.test.ts

@@ -0,0 +1,92 @@
+/**
+ * E2E verification that REQUIRE_PLUGINS=1 fails the rollout via /healthcheck.
+ *
+ * The unit tests in @trigger.dev/rbac cover the loader throw. This file
+ * closes the loop end-to-end: spawn a real webapp, hit /healthcheck via
+ * HTTP, and verify the route's catch turns the throw into a 500 — the
+ * status the ECS/k8s readiness probe rolls back on.
+ *
+ * Each case spawns its own webapp + Postgres + Redis container (~30s) so
+ * env can differ per case. Slow but isolated, matching api-auth.e2e.test.ts.
+ *
+ * Requires a pre-built webapp: pnpm run build --filter webapp
+ *
+ * The REQUIRE_PLUGINS=1 case relies on the plugin NOT being resolvable
+ * from the spawned webapp. CI satisfies this because the plugin isn't in
+ * pnpm-lock.yaml. Local devs who ran `pnpm dev:link-webapp` have the
+ * plugin symlinked into apps/webapp/node_modules — that case is detected
+ * and skipped below.
+ */
+import { existsSync } from "node:fs";
+import { resolve } from "node:path";
+import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
+import type { TestServer } from "@internal/testcontainers/webapp";
+import { startTestServer } from "@internal/testcontainers/webapp";
+
+const LINKED_PLUGIN_PATH = resolve(
+  __dirname,
+  "..",
+  "node_modules",
+  "@triggerdotdev",
+  "plugins"
+);
+const pluginLocallyLinked = existsSync(LINKED_PLUGIN_PATH);
+
+vi.setConfig({ testTimeout: 180_000 });
+
+describe("/healthcheck with REQUIRE_PLUGINS", () => {
+  describe.skipIf(pluginLocallyLinked)("REQUIRE_PLUGINS=1 + plugin missing", () => {
+    let server: TestServer;
+
+    beforeAll(async () => {
+      // requirePlugins: true implies forceRbacFallback: false, so the
+      // loader actually tries to dynamic-import the plugin. The plugin
+      // is not installed in this OSS repo, so the import fails and the
+      // loader throws (instead of falling back) because REQUIRE_PLUGINS=1.
+      // The throw surfaces on the first .isUsingPlugin() call from the
+      // /healthcheck route, which catches it and returns 500.
+      server = await startTestServer({ requirePlugins: true });
+    }, 180_000);
+
+    afterAll(async () => {
+      await server?.stop();
+    }, 120_000);
+
+    it("returns 500 so the readiness probe fails and the rollout is rolled back", async () => {
+      const res = await server.webapp.fetch("/healthcheck");
+      expect(res.status).toBe(500);
+      expect(await res.text()).toBe("ERROR");
+    });
+  });
+
+  // Surface the skip in dev so it doesn't go unnoticed. CI hits the real test above.
+  describe.runIf(pluginLocallyLinked)(
+    "REQUIRE_PLUGINS=1 + plugin LOCALLY LINKED (cross-repo dev setup)",
+    () => {
+      it.skip(
+        `skipped because ${LINKED_PLUGIN_PATH} exists — plugin would load successfully. Run \`pnpm dev:unlink-webapp\` to exercise this case locally; CI runs it without the link.`,
+        () => {}
+      );
+    }
+  );
+
+  describe("REQUIRE_PLUGINS unset + plugin missing", () => {
+    let server: TestServer;
+
+    beforeAll(async () => {
+      // Default: forceRbacFallback=true so the loader short-circuits to
+      // the fallback without trying to import. /healthcheck succeeds.
+      server = await startTestServer();
+    }, 180_000);
+
+    afterAll(async () => {
+      await server?.stop();
+    }, 120_000);
+
+    it("returns 200 (baseline — unchanged self-hoster behaviour)", async () => {
+      const res = await server.webapp.fetch("/healthcheck");
+      expect(res.status).toBe(200);
+      expect(await res.text()).toBe("OK");
+    });
+  });
+});

internal-packages/rbac/src/index.ts

@@ -53,6 +53,13 @@ class LazyController implements RoleBaseAccessController {
 
   constructor(prisma: RbacPrismaInput, options?: RbacCreateOptions) {
     this._init = this.load(prisma, options);
+    // load() runs eagerly but the result is awaited lazily on first method
+    // call. If load() rejects (e.g. REQUIRE_PLUGINS=1 + plugin missing) and
+    // nothing awaits _init before Node ticks past, the rejection surfaces
+    // as unhandledRejection and kills the process. Attach a no-op .catch
+    // so Node sees the rejection as handled; the error is re-thrown when
+    // any consumer awaits this._init via c().
+    this._init.catch(() => {});
   }
 
   private async load(
@@ -105,6 +112,20 @@ class LazyController implements RoleBaseAccessController {
           "RBAC: no plugin installed (ERR_MODULE_NOT_FOUND); using fallback"
         );
       }
+
+      // Fail-fast for deployments that require plugins to be present. Set
+      // REQUIRE_PLUGINS=1 in environments where the fallback is not an
+      // acceptable degraded state — the throw surfaces on the first method
+      // call on the lazy controller (e.g. via the webapp's /healthcheck
+      // route), so the rollout's readiness probe fails and the deploy is
+      // rolled back. Self-hosters leave REQUIRE_PLUGINS unset and continue
+      // to use the fallback when no plugin is installed.
+      if (process.env.REQUIRE_PLUGINS === "1") {
+        throw new Error(
+          `REQUIRE_PLUGINS=1 but plugin "${moduleName}" did not load: ${message}`
+        );
+      }
+
       return new RoleBaseAccessFallback(prisma).create();
     }
   }

internal-packages/rbac/src/require-plugins.test.ts

@@ -0,0 +1,44 @@
+import type { PrismaClient } from "@trigger.dev/database";
+import { afterEach, describe, expect, it, vi } from "vitest";
+import loader from "./index.js";
+
+// The plugin module `@triggerdotdev/plugins/rbac` is not installed in this
+// repo (it lives in the cloud monorepo), so a real dynamic import inside
+// the loader will reliably fail with ERR_MODULE_NOT_FOUND. These tests
+// exercise the loader's branching on that natural failure — no module
+// mocking required.
+
+// The fallback's isUsingPlugin() returns false synchronously without
+// touching prisma, so a placeholder client is fine for tests that only
+// drive the loader path.
+const prismaPlaceholder = {} as unknown as PrismaClient;
+
+describe("LazyController plugin loading", () => {
+  afterEach(() => {
+    vi.unstubAllEnvs();
+  });
+
+  it("falls back silently when REQUIRE_PLUGINS is unset and the plugin is missing", async () => {
+    vi.stubEnv("REQUIRE_PLUGINS", "");
+    const controller = loader.create(prismaPlaceholder);
+    await expect(controller.isUsingPlugin()).resolves.toBe(false);
+  });
+
+  it("throws when REQUIRE_PLUGINS=1 and the plugin is missing", async () => {
+    vi.stubEnv("REQUIRE_PLUGINS", "1");
+    const controller = loader.create(prismaPlaceholder);
+    await expect(controller.isUsingPlugin()).rejects.toThrow(/REQUIRE_PLUGINS=1/);
+  });
+
+  it("forceFallback wins over REQUIRE_PLUGINS=1 (so tests inheriting the env aren't broken)", async () => {
+    vi.stubEnv("REQUIRE_PLUGINS", "1");
+    const controller = loader.create(prismaPlaceholder, { forceFallback: true });
+    await expect(controller.isUsingPlugin()).resolves.toBe(false);
+  });
+
+  it("treats any non-'1' REQUIRE_PLUGINS value as unset (must be exactly '1' to enforce)", async () => {
+    vi.stubEnv("REQUIRE_PLUGINS", "true");
+    const controller = loader.create(prismaPlaceholder);
+    await expect(controller.isUsingPlugin()).resolves.toBe(false);
+  });
+});

internal-packages/testcontainers/src/webapp.ts

@@ -20,12 +20,16 @@ async function findFreePort(): Promise<number> {
   });
 }
 
-async function waitForHealthcheck(url: string, timeoutMs = 60000): Promise<void> {
+async function waitForHealthcheck(
+  url: string,
+  opts: { timeoutMs?: number; acceptAnyResponse?: boolean } = {}
+): Promise<void> {
+  const { timeoutMs = 60000, acceptAnyResponse = false } = opts;
   const deadline = Date.now() + timeoutMs;
   while (Date.now() < deadline) {
     try {
       const res = await fetch(url);
-      if (res.ok) return;
+      if (acceptAnyResponse || res.ok) return;
     } catch {}
     await new Promise((r) => setTimeout(r, 500));
   }
@@ -49,6 +53,18 @@ export interface StartWebappOptions {
    * plugin instead, for testing the plugin path.
    */
   forceRbacFallback?: boolean;
+
+  /**
+   * When true, spawns the webapp with `REQUIRE_PLUGINS=1` so the plugin
+   * loader throws instead of silently falling back when the plugin
+   * module fails to load. Used by the healthcheck rollback e2e test —
+   * with this set and the plugin not installed, `/healthcheck` should
+   * return 500.
+   *
+   * Implies `forceRbacFallback: false` (you can't observe REQUIRE_PLUGINS
+   * behaviour when the loader is short-circuited).
+   */
+  requirePlugins?: boolean;
 }
 
 export async function startWebapp(
@@ -59,7 +75,10 @@ export async function startWebapp(
   instance: WebappInstance;
   stop: () => Promise<void>;
 }> {
-  const forceRbacFallback = options.forceRbacFallback ?? true;
+  // requirePlugins implies forceRbacFallback=false — you can't observe the
+  // REQUIRE_PLUGINS=1 throw if the loader short-circuits before reaching it.
+  const requirePlugins = options.requirePlugins ?? false;
+  const forceRbacFallback = options.forceRbacFallback ?? !requirePlugins;
   const port = await findFreePort();
 
   // Merge NODE_PATH so transitive pnpm deps (hoisted to .pnpm/node_modules) are resolvable
@@ -107,6 +126,10 @@ export async function startWebapp(
       // plugin is installed in the local node_modules. Set to "0" /
       // undefined to spawn a webapp that loads any installed plugin.
       ...(forceRbacFallback ? { RBAC_FORCE_FALLBACK: "1" } : {}),
+      // When requirePlugins is set, explicitly override RBAC_FORCE_FALLBACK
+      // to "0" so a local apps/webapp/.env that sets it to "1" doesn't
+      // short-circuit the loader past the REQUIRE_PLUGINS check.
+      ...(requirePlugins ? { REQUIRE_PLUGINS: "1", RBAC_FORCE_FALLBACK: "0" } : {}),
       NODE_PATH: nodePath,
     },
     stdio: ["ignore", "pipe", "pipe"],
@@ -142,7 +165,12 @@ export async function startWebapp(
 
   try {
     if (spawnError) throw spawnError;
-    await waitForHealthcheck(`${baseUrl}/healthcheck`);
+    // When requirePlugins is set, /healthcheck is expected to respond with
+    // 500 (the whole point of those tests is to verify that). Accept any
+    // response as "the webapp is up" — the test then asserts the status.
+    await waitForHealthcheck(`${baseUrl}/healthcheck`, {
+      acceptAnyResponse: requirePlugins,
+    });
     if (spawnError) throw spawnError;
   } catch (err) {
     proc.kill("SIGTERM");