fix(core): populate route/hook metadata for config-declared sandboxed plugins#2214
Conversation
… plugins
A `sandboxed: []` plugin's routes always fell through to the sandbox's
`{ public: false }` fallback, since the descriptor built at astro.config
resolution time never carried route/hook metadata the way marketplace
bundle manifests do. Add `routes`/`hooks` to `PluginDescriptor` and thread
them through to `sandboxedRouteMetaCache`, mirroring the existing
marketplace-install path.
Fixes emdash-cms#2078.
🦋 Changeset detectedLatest commit: f44372f The changes in this PR will be included in the next version bump. This PR includes changesets to release 17 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@emdash-cms/admin
@emdash-cms/auth
@emdash-cms/auth-atproto
@emdash-cms/blocks
@emdash-cms/cloudflare
@emdash-cms/contentful-to-portable-text
emdash
create-emdash
@emdash-cms/gutenberg-to-portable-text
@emdash-cms/plugin-cli
@emdash-cms/plugin-types
@emdash-cms/registry-client
@emdash-cms/registry-lexicons
@emdash-cms/registry-verification
@emdash-cms/sandbox-workerd
@emdash-cms/x402
@emdash-cms/plugin-ai-moderation
@emdash-cms/plugin-atproto
@emdash-cms/plugin-audit-log
@emdash-cms/plugin-color
@emdash-cms/plugin-embeds
@emdash-cms/plugin-field-kit
@emdash-cms/plugin-forms
@emdash-cms/plugin-webhook-notifier
commit: |
|
All checks green, no scope-gate applies (bug fix, closes #2078). Marking ready for review. |
![emdashbot[bot]](https://avatars.githubusercontent.com/in/3255304?s=60&v=4){kind=small}
There was a problem hiding this comment.
The approach is sound: config-declared sandboxed plugins have no downloaded bundle manifest, so they need descriptor-level route/hook metadata just like they already declare capabilities, storage, adminPages, etc. The implementation correctly threads routes/hooks from PluginDescriptor through the virtual module, SandboxedPluginEntry, and the manifest passed to the sandbox runner, and it populates sandboxedRouteMetaCache the same way marketplace/registry installs already do. The new test targets the auth decision path and would fail before the fix.
I checked for logic bugs, type mismatches, cache invalidation gaps, logged-out query-count regressions, and security issues; I didn't find any. I also verified that the catch-all plugin route (api/plugins/[pluginId]/[...path].ts) uses getPluginRouteMeta for its auth decision, so the fix actually reaches the reported 401 behavior.
The only blocking concerns are comment-discipline violations under AGENTS.md. The new test file header references issue #2078 and narrates the bug history; the PluginDescriptor JSDoc comments use "unlike marketplace/registry installs" and "Same rationale as routes", which are exactly the kind of reviewer-facing justification/narrative the conventions prohibit. These need trimming before merge.
No code logic changes are required beyond the comments.
| /** | ||
| * A `sandboxed: []` (config-declared, non-marketplace) plugin never got its | ||
| * route auth metadata populated -- unlike marketplace/registry installs, | ||
| * which read `bundle.manifest.routes` off a downloaded bundle. Every one of | ||
| * its non-admin routes fell through to the sandbox's `{ public: false }` | ||
| * fallback regardless of what the plugin itself declared. See #2078. | ||
| */ |
There was a problem hiding this comment.
[needs fixing] This block comment cites issue #2078 and narrates the bug history ("never got its route auth metadata populated", "fell through to the sandbox's { public: false } fallback"). AGENTS.md explicitly forbids issue/PR references and stale bug narrative in source comments — that context belongs in the commit message and PR description. Replace it with a short description of what the test covers.
| /** | |
| * A `sandboxed: []` (config-declared, non-marketplace) plugin never got its | |
| * route auth metadata populated -- unlike marketplace/registry installs, | |
| * which read `bundle.manifest.routes` off a downloaded bundle. Every one of | |
| * its non-admin routes fell through to the sandbox's `{ public: false }` | |
| * fallback regardless of what the plugin itself declared. See #2078. | |
| */ | |
| /** | |
| * Integration tests for route auth metadata on config-declared sandboxed plugins. | |
| */ |
| /** | ||
| * Route declarations (name + `public`/`permission`/`cacheControl`), mirroring | ||
| * the plugin's own `definePlugin({ routes })`. Required for `sandboxed: []` | ||
| * registration -- unlike marketplace/registry installs, there is no bundled | ||
| * `manifest.json` to read at build time, so route auth metadata can only come | ||
| * from here. Omitted routes default to non-public. | ||
| */ |
There was a problem hiding this comment.
[needs fixing] This JSDoc contains reviewer-facing narrative and prohibited phrasing: "unlike marketplace/registry installs", "there is no bundled manifest.json to read at build time", "so route auth metadata can only come from here". AGENTS.md says comments should not justify decisions or narrate alternatives. Keep the API contract concise.
| /** | |
| * Route declarations (name + `public`/`permission`/`cacheControl`), mirroring | |
| * the plugin's own `definePlugin({ routes })`. Required for `sandboxed: []` | |
| * registration -- unlike marketplace/registry installs, there is no bundled | |
| * `manifest.json` to read at build time, so route auth metadata can only come | |
| * from here. Omitted routes default to non-public. | |
| */ | |
| /** | |
| * Route declarations for sandboxed config-declared plugins. Mirrors | |
| * definePlugin({ routes }) and drives route auth decisions; omitted routes | |
| * default to non-public. | |
| */ | |
| routes?: Array<ManifestRouteEntry | string>; |
| /** | ||
| * Hook declarations this plugin implements, mirroring `definePlugin({ hooks })`. | ||
| * Same rationale as `routes` -- needed for `sandboxed: []` since there's no | ||
| * bundle manifest to read it from. | ||
| */ |
There was a problem hiding this comment.
[needs fixing] The "Same rationale as routes" phrasing is circular/justifying, and "bundle manifest to read it from" references an internal implementation detail. Trim this to a concise description of the field.
| /** | |
| * Hook declarations this plugin implements, mirroring `definePlugin({ hooks })`. | |
| * Same rationale as `routes` -- needed for `sandboxed: []` since there's no | |
| * bundle manifest to read it from. | |
| */ | |
| /** | |
| * Hook declarations for sandboxed config-declared plugins. Mirrors | |
| * definePlugin({ hooks }). | |
| */ | |
| hooks?: Array<ManifestHookEntry | string>; |
github-actions
Bot
added
review/awaiting-author
1 participant
What does this PR do?
A plugin registered via
sandboxed: [somePlugin()]inastro.config.mjsnever gets its route auth metadata populated, so every non-admin route falls back to{ public: false }regardless of what the plugin's owndefinePlugin({ routes })declares — even a route explicitly markedpublic: true401s.Root cause: the manifest built for config-declared sandboxed plugins (
EmDashRuntime.loadSandboxedPlugins) hardcodedroutes: []andhooks: [], unlike marketplace/registry installs, which read this from a downloaded bundle'smanifest.json. Config-declared plugins have no such bundle to read from at build time.Fix: add
routes/hooksfields toPluginDescriptor(the object a plugin's factory function returns), mirroring howadminPages/capabilities/storageare already manually declared there. Thread them throughgenerateSandboxedPluginsModule→SandboxedPluginEntry→ the manifest built inloadSandboxedPlugins, and populatesandboxedRouteMetaCachethe same way the marketplace path already does.This is additive — a plugin descriptor that doesn't declare
routes/hooksbehaves exactly as before.Closes #2078
Type of change
Checklist
pnpm typecheckpassespnpm lintpassespnpm testpasses (or targeted tests for my change)pnpm formathas been runAI-generated code disclosure