fix(nextly): reject duplicate plugin admin slugs at boot

.changeset/plugin-admin-slug-uniqueness.md

@@ -0,0 +1,34 @@
+---
+"nextly": patch
+"create-nextly-app": patch
+"@nextlyhq/admin": patch
+"@nextlyhq/admin-css": patch
+"@nextlyhq/blocks-engine": patch
+"@nextlyhq/blocks-react": patch
+"@nextlyhq/ui": patch
+"@nextlyhq/adapter-drizzle": patch
+"@nextlyhq/adapter-postgres": patch
+"@nextlyhq/adapter-mysql": patch
+"@nextlyhq/adapter-sqlite": patch
+"@nextlyhq/storage-s3": patch
+"@nextlyhq/storage-uploadthing": patch
+"@nextlyhq/storage-vercel-blob": patch
+"@nextlyhq/plugin-form-builder": patch
+"@nextlyhq/plugin-page-builder": patch
+"@nextlyhq/plugin-seo": patch
+"@nextlyhq/plugin-sdk": patch
+"@nextlyhq/eslint-config": patch
+"@nextlyhq/prettier-config": patch
+"@nextlyhq/telemetry": patch
+"@nextlyhq/tsconfig": patch
+"@nextlyhq/builder": patch
+"@nextlyhq/module-specifiers": patch
+---
+
+Reject duplicate plugin admin slugs at boot. `pluginAdminSlug` collapses every
+non-alphanumeric run to a single dash, so distinct package names can map to one
+slug and the plugins then share a single admin address — one plugin's detail
+page opens the other's, and host `pluginOverrides` apply to the wrong package.
+No lookup downstream can detect this, because every lookup along that address
+returns a plugin. `resolvePlugins` now refuses to start, naming both packages
+and the slug they collide on.

packages/nextly/src/cli/utils/config-loader.ts

@@ -56,6 +56,7 @@ import {
   finalizeRelationTargets,
   validateCrossPluginRelations,
 } from "../../plugins/schema/validate-relations";
+import { validatePluginSlugs } from "../../plugins/validate-slugs";
 
 import { bundleAndRequire } from "./config-bundler";
 
@@ -559,6 +560,14 @@ async function loadConfigInternal(
         }
       }
 
+      // The transformed list, for the same reason the runtime validates its
+      // own: a `setup` transformer can add or rename plugins into a slug
+      // collision, and everything below consumes the transformed config. The
+      // CLI has to agree with boot here — otherwise `nextly build`, a
+      // migration or a db sync accepts and acts on a configuration the
+      // deployed app then refuses to start on.
+      validatePluginSlugs(transformedConfig.plugins ?? []);
+
       // Fold plugin contributions. Extend targets that aren't code/plugin
       // entities are DEFERRED (candidate Builder/UI-schema targets) rather than
       // thrown, so a plugin may extend/relate to a Builder-made collection

packages/nextly/src/di/register.ts

@@ -125,6 +125,7 @@ import {
   registerPluginService,
 } from "../plugins/services/plugin-services-registry";
 import { clearPluginSubscriptions } from "../plugins/subscription-tracker";
+import { validatePluginSlugs } from "../plugins/validate-slugs";
 import type {
   CollectionSource,
   FieldDefinition,
@@ -429,6 +430,12 @@ export async function registerServices(
   // Layer 0b: Process Plugin Config Transformers (resolved order)
   // ----------------------------------------
   const setupConfig = await applyPluginConfigTransformers(resolvedConfig);
+  // Again on the transformed list, because a `setup` transformer may add,
+  // rename or replace entries in `plugins` — and everything from here down
+  // consumes the transformed config, not the list `resolvePlugins` checked.
+  // Boot is where this should fail; without it a transformer-introduced
+  // collision would surface on the first admin-meta request instead.
+  validatePluginSlugs(setupConfig.plugins ?? []);
 
   // ----------------------------------------
   // Layer 0c: Fold declarative plugin schema contributions (D3/D12/D50)

packages/nextly/src/plugins/admin-meta.ts

@@ -27,6 +27,7 @@ import type {
 } from "./plugin-context";
 import { pluginAdminSlug } from "./plugin-slug";
 import { validatedClientConfig } from "./validate-client-config";
+import { validatePluginSlugs } from "./validate-slugs";
 
 /**
  * The serialized admin-meta entry for a single plugin, consumed by the admin
@@ -161,6 +162,15 @@ export function buildPluginAdminMeta(
   plugins: PluginDefinition[],
   pluginOverrides: Record<string, PluginOverride> | undefined
 ): PluginAdminMeta[] {
+  // Here, and not only at boot, because this is the one place a plugin list
+  // becomes ADDRESSES: the slug below is the admin's URL for the plugin and
+  // the key its host override is read by. Two boot paths do not reach it —
+  // `createDynamicHandlers` initializes services lazily and serves the public
+  // admin-meta endpoint before that happens, and a `setup` transformer can
+  // rewrite `config.plugins` after `resolvePlugins` has already run. Both
+  // would publish ambiguous addresses from a list nothing had checked.
+  validatePluginSlugs(plugins);
+
   return plugins.map(plugin => {
     const slug = pluginAdminSlug(plugin.name);
     const hostOverride = pluginOverrides?.[slug];

packages/nextly/src/plugins/resolve.test.ts

@@ -21,6 +21,27 @@ describe("resolvePlugins", () => {
     expect(out.map(x => x.name)).toEqual(["b", "a"]);
   });
 
+  /**
+   * The wiring, not the check. `validatePluginSlugs` has its own suite; this
+   * asserts `resolvePlugins` actually calls it — without this, removing the
+   * call leaves every slug test green while nothing at boot runs it.
+   *
+   * The two names differ only by separators, which is the whole point: they
+   * are distinct legal packages that produce one admin address.
+   */
+  it("rejects two plugins that resolve to the same admin slug", () => {
+    let reason: string | undefined;
+    try {
+      resolvePlugins([p("@acme/plugin-seo"), p("acme_plugin_seo")], {
+        coreVersion: "1.0.0",
+      });
+    } catch (error) {
+      reason = (error as { logContext?: { reason?: string } }).logContext
+        ?.reason;
+    }
+    expect(reason).toBe("duplicate-admin-slug");
+  });
+
   it("surfaces a version error even when the graph is otherwise orderable", () => {
     expect(() =>
       resolvePlugins([p("a", { nextly: ">=2.0.0" }), p("b")], {

packages/nextly/src/plugins/resolve.ts

@@ -1,6 +1,7 @@
 import type { PluginDefinition } from "./plugin-context";
 import { topoSortPlugins } from "./topo-sort";
 import { assertClientConfigs } from "./validate-client-config";
+import { validatePluginSlugs } from "./validate-slugs";
 import { validatePluginVersions } from "./validate-versions";
 
 export interface ResolvePluginsOptions {
@@ -28,5 +29,10 @@ export function resolvePlugins(
   // other fail-fast checks rather than surfacing when the admin first asks for
   // its metadata and losing the whole branding response with it.
   assertClientConfigs(plugins);
+  // Two plugins sharing an admin slug share an address, and nothing downstream
+  // can detect it: every lookup along that address returns a plugin, which is
+  // what a correct lookup returns. Registration is where the ambiguity is still
+  // observable.
+  validatePluginSlugs(plugins);
   return topoSortPlugins(plugins);
 }

packages/nextly/src/plugins/validate-slugs.test.ts

@@ -0,0 +1,140 @@
+/**
+ * A plugin's admin slug is its address: the admin builds
+ * `/admin/plugins/<slug>` from it, core namespaces plugin admin routes with
+ * it, and host `pluginOverrides` are keyed by it. Two plugins reaching the
+ * same slug therefore share one address, and nothing downstream can notice —
+ * which is why this is checked at registration.
+ */
+import { describe, expect, it } from "vitest";
+
+import { buildPluginAdminMeta } from "./admin-meta";
+import type { PluginDefinition } from "./plugin-context";
+import { validatePluginSlugs } from "./validate-slugs";
+
+function plugin(name: string): PluginDefinition {
+  return { name, version: "1.0.0", nextly: "*" } as PluginDefinition;
+}
+
+/**
+ * The thrown `NextlyError` carries a deliberately generic `message`
+ * ("Plugin configuration is invalid.") for the client; the specific failure
+ * lives in `logContext.reason`. Matching on `message` would pass for every
+ * plugin-resolution failure alike — an incompatible version included — so
+ * these assert the reason instead.
+ */
+function collisionReason(names: string[]): string | undefined {
+  try {
+    validatePluginSlugs(names.map(plugin));
+  } catch (error) {
+    return (error as { logContext?: { reason?: string } }).logContext?.reason;
+  }
+  return undefined;
+}
+
+describe("validatePluginSlugs", () => {
+  it("accepts plugins whose slugs differ", () => {
+    expect(() =>
+      validatePluginSlugs([plugin("@acme/one"), plugin("@acme/two")])
+    ).not.toThrow();
+    // The control on `collisionReason` itself: it must report undefined when
+    // nothing collides, or every assertion above would pass on a helper that
+    // always returned the reason it was looking for.
+    expect(collisionReason(["@acme/one", "@acme/two"])).toBeUndefined();
+  });
+
+  it("accepts an empty plugin list", () => {
+    expect(() => validatePluginSlugs([])).not.toThrow();
+  });
+
+  /**
+   * The separating cases, and the reason a check on the NAMES would not do.
+   * `pluginAdminSlug` lowercases and collapses each non-alphanumeric run to a
+   * single dash, so every pair below is two distinct, legal package names that
+   * produce one address.
+   */
+  it.each([
+    ["scope separator", "@acme/plugin-seo", "acme-plugin-seo"],
+    ["a dot for a dash", "@acme/plugin-seo", "@acme/plugin.seo"],
+    ["case", "@acme/plugin-seo", "@ACME/Plugin-SEO"],
+    ["underscores", "@acme/plugin-seo", "acme_plugin_seo"],
+    ["a doubled separator", "@acme/plugin-seo", "@acme//plugin--seo"],
+  ])("rejects two plugins colliding by %s", (_why, first, second) => {
+    expect(collisionReason([first, second])).toBe("duplicate-admin-slug");
+  });
+
+  /**
+   * The message is the entire remedy — the reader has to rename one package,
+   * and this is the only place the pair is ever stated together. Asserting it
+   * throws would pass on an error naming neither.
+   */
+  it("names both plugins and the slug they collide on", () => {
+    let caught: unknown;
+    try {
+      validatePluginSlugs([
+        plugin("@acme/plugin.seo"),
+        plugin("acme_plugin_seo"),
+      ]);
+    } catch (error) {
+      caught = error;
+    }
+
+    const message = (caught as { logMessage?: string })?.logMessage ?? "";
+    expect(message).toContain("@acme/plugin.seo");
+    expect(message).toContain("acme_plugin_seo");
+    expect(message).toContain("acme-plugin-seo");
+    expect(
+      (caught as { logContext?: { reason?: string } })?.logContext?.reason
+    ).toBe("duplicate-admin-slug");
+  });
+
+  /**
+   * The same package listed twice is the same collision as far as addressing
+   * goes, and it is the likelier mistake — a plugin registered by both the app
+   * and a preset. It must not be waved through as "identical, so harmless".
+   */
+  it("rejects the exact same name registered twice", () => {
+    expect(collisionReason(["@acme/one", "@acme/one"])).toBe(
+      "duplicate-admin-slug"
+    );
+  });
+});
+
+/**
+ * The seam, as distinct from the boot check.
+ *
+ * `buildPluginAdminMeta` is the one place in core that turns a plugin list
+ * into addresses — the slug it derives is the admin's URL for that plugin and
+ * the key its host override is read by. Two paths reach it without the boot
+ * check having run on the list it receives: `createDynamicHandlers`
+ * initializes services lazily and serves the public admin-meta endpoint first,
+ * and a `setup` transformer can rewrite `config.plugins` after
+ * `resolvePlugins` has already validated the original.
+ */
+describe("buildPluginAdminMeta", () => {
+  it("addresses plugins whose slugs differ", () => {
+    const meta = buildPluginAdminMeta(
+      [plugin("@acme/one"), plugin("@acme/two")],
+      undefined
+    );
+
+    // The positive control: it really does produce metadata for both, so the
+    // rejection below is about the collision rather than about a function that
+    // refuses everything.
+    expect(meta.map(m => m.name)).toEqual(["@acme/one", "@acme/two"]);
+  });
+
+  it("refuses to address two plugins that share a slug", () => {
+    let reason: string | undefined;
+    try {
+      buildPluginAdminMeta(
+        [plugin("@acme/plugin-seo"), plugin("acme_plugin_seo")],
+        undefined
+      );
+    } catch (error) {
+      reason = (error as { logContext?: { reason?: string } }).logContext
+        ?.reason;
+    }
+
+    expect(reason).toBe("duplicate-admin-slug");
+  });
+});

packages/nextly/src/plugins/validate-slugs.ts

@@ -0,0 +1,52 @@
+import type { PluginDefinition } from "./plugin-context";
+import { pluginAdminSlug } from "./plugin-slug";
+import { resolutionError } from "./resolution-error";
+
+/**
+ * Boot-check that no two plugins address the same admin slug. Throws
+ * fail-fast.
+ *
+ * `pluginAdminSlug` is deliberately lossy — it lowercases and collapses every
+ * non-alphanumeric run to one dash — so distinct package names routinely map
+ * to one slug: `@acme/plugin-seo`, `@acme/plugin.seo` and `ACME_Plugin_SEO`
+ * are all `acme-plugin-seo`. That slug is the plugin's address: the admin
+ * builds `/admin/plugins/<slug>` from it, core namespaces the plugin's admin
+ * routes with it, and host `pluginOverrides` are looked up by it.
+ *
+ * So a collision is not a cosmetic clash. Two plugins share one address, and
+ * every lookup along it answers with whichever the search reaches first: one
+ * plugin's page opens the other's, and one plugin's overrides silently apply
+ * to its neighbour.
+ *
+ * The check belongs HERE rather than at any of those lookups, and that is the
+ * whole point. At a lookup there is nothing to observe — `find()` returns a
+ * plugin, and a plugin is exactly what a correct lookup returns, so no code
+ * downstream can tell "the right one" from "the first of two". Registration is
+ * the last moment at which the ambiguity is still visible as ambiguity.
+ *
+ * @module plugins/validate-slugs
+ */
+export function validatePluginSlugs(plugins: PluginDefinition[]): void {
+  const byslug = new Map<string, string>();
+
+  for (const plugin of plugins) {
+    const slug = pluginAdminSlug(plugin.name);
+    const owner = byslug.get(slug);
+
+    if (owner !== undefined) {
+      // Both names, because neither alone is actionable: the reader has to
+      // rename one of them, and the message is the only place the pair is
+      // ever stated. The slug is included because it is not obvious from
+      // either name which characters collapsed.
+      throw resolutionError(
+        "duplicate-admin-slug",
+        `Plugins "${owner}" and "${plugin.name}" both resolve to the admin ` +
+          `slug "${slug}", so they would share one admin address. Rename one ` +
+          `of them.`,
+        { slug, plugins: [owner, plugin.name] }
+      );
+    }
+
+    byslug.set(slug, plugin.name);
+  }
+}