From b5a41b7535d14ab3974372c3ac93af11002e626e Mon Sep 17 00:00:00 2001 From: Mostafa Sadeghi <205455727+mostafasadeghidev@users.noreply.github.com> Date: Mon, 3 Aug 2026 05:31:05 +0200 Subject: [PATCH] fix(plugin-sdk): definePlugin no longer drops contentAccess from the manifest MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DefinePluginConfig had no contentAccess field and definePlugin() never copied it into the manifest it returns, even though PluginManifest supports it and the CLI's own content-editor scaffold passes it. Any plugin built with the CLI silently lost its per-table allowlist, so every cms.content.* call failed closed at runtime despite the operator granting the permissions. - Add contentAccess to DefinePluginConfig and snapshot it into the manifest (key omitted when undefined, matching the other optional fields). - Lint: warn when a declared cms.content.* permission has no contentAccess entry carrying the matching mode (permission with no consumer — fails closed per table+mode). The missing-allowlist case stays a single hard error from parsePluginManifest. - Lint: report source-scan finding paths POSIX-style on Windows. - Scaffold: drop the dead `entrypoints:` line from the content-editor template — not a DefinePluginConfig field (an IDE type error for authors, silently ignored at runtime) and redundant, since the build auto-wires entrypoints.server from server/index.ts. - Fix the stale definePlugin docblock that showed editor/server/frontend config fields which don't exist and a dead scripts/build-plugin.ts path. - Tests: definePlugin contentAccess deep-copy + omission; lint warning fires/stays quiet correctly; end-to-end regression scaffolding the content-editor template and asserting it lints clean. Co-Authored-By: Claude Fable 5 --- src/__tests__/plugin-sdk/builders.test.ts | 39 +++++++++++ src/__tests__/plugin-sdk/lintCli.test.ts | 69 ++++++++++++++++++++ src/core/plugin-sdk/builders/definePlugin.ts | 23 +++++-- src/core/plugin-sdk/cli/init.ts | 3 +- src/core/plugin-sdk/cli/lint.ts | 42 +++++++++++- 5 files changed, 167 insertions(+), 9 deletions(-) diff --git a/src/__tests__/plugin-sdk/builders.test.ts b/src/__tests__/plugin-sdk/builders.test.ts index 6bdd0e164..3597b566f 100644 --- a/src/__tests__/plugin-sdk/builders.test.ts +++ b/src/__tests__/plugin-sdk/builders.test.ts @@ -17,6 +17,7 @@ import { raw, safeUrl, vc, + type ContentAccessEntry, } from '@core/plugin-sdk' // Layout compilation maps HTML elements to base.* modules via the registry. import '@modules/base' @@ -325,6 +326,44 @@ describe('definePlugin', () => { expect(definition.pack?.classes[0].id).toBe('acme.ui-kit/section') }) + it('copies contentAccess into the manifest as an independent deep copy', () => { + const contentAccess: ContentAccessEntry[] = [ + { table: 'pages', modes: ['read', 'write'] }, + ] + const definition = definePlugin({ + id: 'acme.workflow', + name: 'Workflow', + version: '1.0.0', + permissions: [permissions.cmsContentRead, permissions.cmsContentWrite], + contentAccess, + }) + + expect(definition.manifest.contentAccess).toEqual([ + { table: 'pages', modes: ['read', 'write'] }, + ]) + + // Mutating the config input after the fact must not leak into the + // manifest — the builder snapshots entries and their modes arrays. + contentAccess[0].modes.push('delete') + contentAccess.push({ table: 'posts', modes: ['read'] }) + expect(definition.manifest.contentAccess).toEqual([ + { table: 'pages', modes: ['read', 'write'] }, + ]) + }) + + it('omits contentAccess from the manifest when not configured', () => { + const definition = definePlugin({ + id: 'acme.ui-kit', + name: 'UI Kit', + version: '1.0.0', + permissions: [permissions.modulesRegister], + }) + // The key must be absent — not `undefined` — so the in-memory manifest + // round-trips through `parsePluginManifest` exactly like the zipped + // `plugin.json` (see the omission rationale in definePlugin). + expect('contentAccess' in definition.manifest).toBe(false) + }) + it('rejects plugin ids without a vendor namespace', () => { expect(() => definePlugin({ id: 'just-name', diff --git a/src/__tests__/plugin-sdk/lintCli.test.ts b/src/__tests__/plugin-sdk/lintCli.test.ts index 61e936a68..d82c353b6 100644 --- a/src/__tests__/plugin-sdk/lintCli.test.ts +++ b/src/__tests__/plugin-sdk/lintCli.test.ts @@ -6,14 +6,20 @@ * • Missing or malformed `instatic-plugin.config.ts` is surfaced cleanly * • `network.outbound` permission without `networkAllowedHosts` is an error * • `networkAllowedHosts` without `network.outbound` is a warning + * • `cms.content.*` permissions without a matching `contentAccess` mode + * entry are a warning; the missing-allowlist case stays a single error * • Source files with `'node:*'` / `'bun:*'` / `require(` are errors * • Bundled `dist/` outputs that smuggle forbidden literals are errors * • A clean plugin reports zero findings + * • The CLI's own content-editor scaffold builds a manifest that carries + * `contentAccess` through `definePlugin` and lints clean */ import { describe, expect, it } from 'bun:test' import { mkdtemp, mkdir, rm, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { lintPlugin } from '../../core/plugin-sdk/cli/lint' +import { runPluginInit } from '../../core/plugin-sdk/cli/init' +import { readPluginDefinition } from '../../core/plugin-sdk/cli/build' const PROJECT_ROOT = join(import.meta.dir, '../../..') @@ -85,6 +91,41 @@ describe('instatic-plugin lint', () => { expect(findings[0].message).toContain('network.outbound') }) + it('warns when a cms.content.* permission has no contentAccess entry declaring its mode', async () => { + const result = await withTempPlugin(async (dir) => { + await writeConfig(dir, { + permissions: ['cms.content.read', 'cms.content.delete'], + contentAccess: [{ table: 'posts', modes: ['read'] }], + }) + }) + expect(result.findings.filter((f) => f.severity === 'error')).toEqual([]) + const warnings = result.findings.filter((f) => f.severity === 'warning') + expect(warnings).toHaveLength(1) + expect(warnings[0].scope).toBe('manifest') + expect(warnings[0].message).toContain('cms.content.delete') + expect(warnings[0].message).toContain('"delete"') + }) + + it('does not warn when every cms.content.* permission is covered by a contentAccess mode', async () => { + const result = await withTempPlugin(async (dir) => { + await writeConfig(dir, { + permissions: ['cms.content.read', 'cms.content.write'], + contentAccess: [{ table: 'pages', modes: ['read', 'write'] }], + }) + }) + expect(result.findings).toEqual([]) + }) + + it('keeps the missing-contentAccess case a single manifest error (no duplicate warnings)', async () => { + const result = await withTempPlugin(async (dir) => { + await writeConfig(dir, { permissions: ['cms.content.read'] }) + }) + expect(result.findings).toHaveLength(1) + expect(result.findings[0].severity).toBe('error') + expect(result.findings[0].scope).toBe('manifest') + expect(result.findings[0].message).toContain('contentAccess') + }) + it('errors on forbidden literals in server source files', async () => { const result = await withTempPlugin(async (dir) => { await writeConfig(dir) @@ -158,4 +199,32 @@ describe('instatic-plugin lint', () => { expect(result.findings[0].scope).toBe('config') expect(result.pluginId).toBe('') }) + + it('content-editor scaffold carries contentAccess into the manifest and lints clean', async () => { + // Regression: `definePlugin` used to drop `contentAccess` from the + // manifest it returned, so the CLI's own content-editor scaffold failed + // this very lint with "contentAccess is required when any cms.content.* + // permission is granted" — and installed plugins failed closed on every + // cms.content.* call despite the operator's grant. + const parentDir = join(PROJECT_ROOT, '.tmp-lint') + await mkdir(parentDir, { recursive: true }) + const dir = await mkdtemp(join(parentDir, 'scaffold-')) + try { + const pluginDir = await runPluginInit('acme.content-lint', { + kind: 'content-editor', + parentDir: dir, + }) + + const definition = await readPluginDefinition(pluginDir) + expect(definition.manifest.contentAccess).toEqual([ + { table: 'pages', modes: ['read', 'write'] }, + ]) + + const result = await lintPlugin(pluginDir) + expect(result.findings).toEqual([]) + expect(result.pluginId).toBe('acme.content-lint') + } finally { + await rm(dir, { recursive: true, force: true }) + } + }) }) diff --git a/src/core/plugin-sdk/builders/definePlugin.ts b/src/core/plugin-sdk/builders/definePlugin.ts index 369849fbb..eb619ad71 100644 --- a/src/core/plugin-sdk/builders/definePlugin.ts +++ b/src/core/plugin-sdk/builders/definePlugin.ts @@ -13,16 +13,14 @@ * permissions: [permissions.modulesRegister, permissions.visualComponentsRegister], * modules: [callout], * pack, - * // Optional entry-point hooks — pure objects, not file paths. - * // The build script wires them into the runtime zip layout. - * editor: () => import(`./editor`), - * server: () => import(`./server`), - * frontend: () => import(`./frontend/tracker`), * }) * * The return value is the host's runtime `PluginManifest` plus the bundled - * builder objects. The build script (PR 1.1, see scripts/build-plugin.ts) - * uses those builder objects to emit the final zip. + * builder objects. The build CLI (`cli/build.ts`) uses those builder + * objects to emit the final zip. Entrypoints are NOT declared here — + * `instatic-plugin build` auto-wires them into the emitted `plugin.json` + * from the source layout (`server/index.ts`, `editor/index.ts`, top-level + * `frontend/*.ts`, admin app entries). */ import { PLUGIN_API_VERSION } from '../types' @@ -33,6 +31,7 @@ import type { PluginPermission, PluginResource, } from '../types' +import type { ContentAccessEntry } from '../contentSchemas' import type { PluginModuleDefinition } from '../modules' import type { PluginPackContents } from './definePack' import { @@ -83,6 +82,13 @@ export interface DefinePluginConfig { */ resources?: PluginResource[] + /** + * Per-table allowlist for the `cms.content.*` surface. Required by the + * manifest parser whenever any `cms.content.*` permission is declared; + * the host fails closed at runtime for tables/modes not listed here. + */ + contentAccess?: ContentAccessEntry[] + /** * Admin pages registered by the plugin (markdown / map / resource / app). * Auto-deduped against the page id. @@ -178,6 +184,9 @@ export function definePlugin(config: DefinePluginConfig): PluginDefinition { ...(config.networkAllowedHosts ? { networkAllowedHosts: [...config.networkAllowedHosts] } : {}), + ...(config.contentAccess + ? { contentAccess: config.contentAccess.map((entry) => ({ ...entry, modes: [...entry.modes] })) } + : {}), ...(config.settings !== undefined ? { settings: config.settings } : {}), ...(config.author !== undefined ? { author: config.author } : {}), ...(config.license !== undefined ? { license: config.license } : {}), diff --git a/src/core/plugin-sdk/cli/init.ts b/src/core/plugin-sdk/cli/init.ts index 78f4056f8..b79e864ec 100644 --- a/src/core/plugin-sdk/cli/init.ts +++ b/src/core/plugin-sdk/cli/init.ts @@ -135,7 +135,8 @@ export default definePlugin({ { table: 'pages', modes: ['read', 'write'] }, ], - entrypoints: { server: 'server/index.js' }, + // No entrypoints here — \`instatic-plugin build\` auto-wires + // \`entrypoints.server\` because \`server/index.ts\` exists. }) ` } diff --git a/src/core/plugin-sdk/cli/lint.ts b/src/core/plugin-sdk/cli/lint.ts index 849415368..3a793d0b7 100644 --- a/src/core/plugin-sdk/cli/lint.ts +++ b/src/core/plugin-sdk/cli/lint.ts @@ -16,6 +16,9 @@ * • Bundled outputs in `dist/server/index.js` and `dist/modules/index.js` * pass the same scan (catches authors that bypass `instatic-plugin build`) * • If `network.outbound` is requested, `networkAllowedHosts` is non-empty + * • Every requested `cms.content.*` permission is consumed by a + * `contentAccess` entry declaring the matching mode (the missing- + * allowlist case is already a manifest error from `parsePluginManifest`) * * The intent: catch every common authoring mistake BEFORE the developer * uploads a zip, so they get a precise error in their terminal instead of @@ -28,6 +31,8 @@ import { findSandboxLiterals } from '@core/plugins/sandboxScan' import { parsePluginManifest } from '@core/plugins/manifest' import { readPluginDefinition } from './build' import type { PluginDefinition } from '../builders/definePlugin' +import type { PluginPermission } from '../types' +import type { ContentAccessMode } from '../contentSchemas' export type LintSeverity = 'error' | 'warning' @@ -48,6 +53,14 @@ export interface LintResult { const SANDBOXED_ENTRYPOINTS: ReadonlyArray<'server' | 'modules'> = ['server', 'modules'] +/** Which `contentAccess` mode consumes each `cms.content.*` permission. */ +const CONTENT_PERMISSION_MODES: ReadonlyArray<{ permission: PluginPermission; mode: ContentAccessMode }> = [ + { permission: 'cms.content.read', mode: 'read' }, + { permission: 'cms.content.write', mode: 'write' }, + { permission: 'cms.content.publish', mode: 'publish' }, + { permission: 'cms.content.delete', mode: 'delete' }, +] + /** * Run all lint checks for a plugin source directory. Throws on a corrupt * `instatic-plugin.config.ts`; everything else is reported as a finding so the @@ -129,6 +142,32 @@ export async function lintPlugin(sourceDir: string): Promise { } } + // ---- cms.content.* + contentAccess coherence --------------------------- + // + // `parsePluginManifest` above already fails hard when any `cms.content.*` + // permission is declared with no `contentAccess` at all, and when an + // entry mode lacks its matching permission. The remaining gap: a + // permission whose mode appears in no entry. The host enforces access + // per table+mode and fails closed, so every call under that permission + // is rejected at runtime — and the install consent screen advertises + // capability the plugin can never use. (Skipped when `contentAccess` is + // empty so the parser's error isn't double-reported as warnings.) + const contentAccess = manifest.contentAccess ?? [] + if (contentAccess.length > 0) { + for (const { permission, mode } of CONTENT_PERMISSION_MODES) { + if (!manifest.permissions.includes(permission)) continue + if (contentAccess.some((entry) => entry.modes.includes(mode))) continue + findings.push({ + severity: 'warning', + scope: 'manifest', + message: + `\`${permission}\` permission is requested but no \`contentAccess\` entry declares mode "${mode}". ` + + `The host fails closed per table+mode, so every ${mode} call will be rejected at runtime — ` + + `add the mode to a table entry or drop the permission.`, + }) + } + } + // ---- frontend.assets coherence ---------------------------------------- // // Permission ↔ declarations: @@ -258,7 +297,8 @@ export async function lintPlugin(sourceDir: string): Promise { severity: 'error', scope: `source:${kind}`, message: `references forbidden sandbox literal \`${offender.literal}\` — plugin code can't reach Node/Bun runtime APIs. Use the SDK instead.`, - file: file.slice(absoluteSource.length + 1), + // Findings report POSIX-style relative paths on every platform. + file: file.slice(absoluteSource.length + 1).replaceAll('\\', '/'), }) } }