fix(nextly): reject duplicate plugin admin slugs at boot - #747
Conversation
pluginAdminSlug collapses every non-alphanumeric run to one dash, so distinct package names map to one slug and the plugins silently share an admin address. No lookup downstream can detect it, because every lookup along that address returns a plugin, which is what a correct lookup returns.
|
@codex please review this PR |
|
Warning Review limit reached
Next review available in: 7 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (7)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 12ada287c2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…y at boot resolvePlugins runs before setup transformers can rewrite config.plugins, and createDynamicHandlers serves the public admin-meta endpoint before services initialize at all. Both published ambiguous addresses from a list the boot check never saw. buildPluginAdminMeta is the one place a plugin list becomes addresses, so it validates what it is about to address.
@nextlyhq/adapter-drizzle
@nextlyhq/adapter-mysql
@nextlyhq/adapter-postgres
@nextlyhq/adapter-sqlite
@nextlyhq/admin
@nextlyhq/admin-css
@nextlyhq/blocks-engine
@nextlyhq/blocks-react
@nextlyhq/builder
create-nextly-app
nextly
@nextlyhq/plugin-form-builder
@nextlyhq/plugin-page-builder
@nextlyhq/plugin-sdk
@nextlyhq/plugin-seo
@nextlyhq/storage-s3
@nextlyhq/storage-uploadthing
@nextlyhq/storage-vercel-blob
@nextlyhq/ui
commit: |
|
@codex please review this PR |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 19e068cd47
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The CLI runs its own transformer loop, so a setup() that renames plugins into a slug collision was accepted by nextly build, migrations and db sync while the deployed app refused to start on the same config.
|
@codex please review this PR |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
1 participant
Two plugins can currently share one admin address, silently.
The defect
pluginAdminSluglowercases a package name and collapses every non-alphanumeric run to a single dash, so it is not injective. All of these produceacme-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 hostpluginOverridesare looked up by it. Nothing enforces uniqueness —resolvePluginsvalidates versions (validate-versions.ts) and client configs (validate-client-config.ts) and never the slug.So with two colliding plugins installed, one plugin's detail page opens the other's, and one plugin's overrides apply to its neighbour. No error is raised at any point.
Why the check goes at registration
Nowhere downstream can detect it. At a lookup,
find()returns a plugin — and a plugin is exactly what a correct lookup returns, so no code at the point of use can separate "the right one" from "the first of two". Registration is the last moment at which the ambiguity is still visible as ambiguity.resolvePluginsnow callsvalidatePluginSlugs, which throws aPLUGIN_RESOLUTION_ERRORwithreason: "duplicate-admin-slug", naming both packages and the slug they collide on — the reader has to rename one, and the message is the only place the pair is ever stated together. This mirrors the existing fail-fast shape ofvalidate-versions.ts.Audit
Before relying on a boot check, I confirmed nothing derives the slug independently: every consumer imports the single
pluginAdminSlughelper from core (admin re-exports it aspluginSlug). The other[^a-z0-9]collapsing regexes in the repo are for field names, table names and single names — different domains, not plugin addresses. So this check is authoritative rather than one of two answers.Tests
validate-slugs.test.tscovers five distinct collision shapes — scope separator, dot-for-dash, case, underscores, doubled separator — plus the same name registered twice, which is the likelier mistake (an app and a preset both registering one plugin). Assertions readlogContext.reasonrather than the error message, sinceNextlyErrordeliberately carries a generic public message that every resolution failure shares.resolve.test.tscovers the wiring separately. That matters: break-verification showed that removing the call fromresolvePluginsleft every validator test green, because they exercise the validator directly.Break-verified in both directions — unwiring the call fails only the wiring test; the validator suite fails only on real collisions, with a control asserting the helper reports
undefinedwhen nothing collides.Notes
packages/nextly/src/plugins/schema/caching.test.tsfails 2 tests on this branch. Pre-existing: measured identically onorigin/mainat the same commit, and the stack (assertNoLegacyFieldGroupKey) is not in this change's call path.