From f8a274b76ee99c8f747cacd5eea0f94bdd18de16 Mon Sep 17 00:00:00 2001 From: turtle1999 Date: Wed, 30 Sep 2026 17:54:55 +0800 Subject: [PATCH] fix(plugins): name declaring manifests and reject built display manifests Manifest icon diagnostics name the package.json the author edits. verify-package-meta rejects a package.json export that resolves to build output instead of reading it, and PackageManifest.exports uses a narrow export-target type. --- packages/boot/app-boot/src/package-meta.ts | 15 +++++++++------ .../boot/app-boot/tests/package-meta.spec.ts | 2 +- scripts/check-workspace-constraints.ts | 18 ++++++++++++------ scripts/verify-package-meta.spec.ts | 9 +++++++++ scripts/verify-package-meta.ts | 9 +++++++-- 5 files changed, 38 insertions(+), 15 deletions(-) diff --git a/packages/boot/app-boot/src/package-meta.ts b/packages/boot/app-boot/src/package-meta.ts index 5ee4233631..ede6023924 100644 --- a/packages/boot/app-boot/src/package-meta.ts +++ b/packages/boot/app-boot/src/package-meta.ts @@ -23,30 +23,33 @@ function iconOf( ): string | undefined { const icon = manifestPath === undefined ? undefined : textOf(value, `${manifestPath}: icon`) let target: string + // Diagnostics name the file the author edits: the declaring manifest or the exported image. + let label: string if (manifestPath !== undefined && icon !== undefined) { if (isAbsolute(icon) || win32.isAbsolute(icon) || /^[A-Za-z][A-Za-z\d+.-]*:/u.test(icon)) { throw new Error(`${manifestPath}: icon must be a relative file path`) } + label = manifestPath const root = realpathSync(dirname(manifestPath)) target = resolve(root, icon) const local = relative(root, realpathSync(target)) if (local === '..' || local.startsWith(`..${sep}`) || isAbsolute(local)) { - throw new Error(`${target}: icon must remain inside its manifest directory`) + throw new Error(`${label}: icon must remain inside its manifest directory`) } } else { const exported = optionalResourcePath(`${specifier}/icon`, parentURL) if (exported === undefined) return undefined - target = exported + target = label = exported assertPackageOwned(realpathSync(target), packageName) } const mediaType = ICON_MEDIA_TYPES.get(extname(target).toLowerCase()) - if (mediaType === undefined) throw new Error(`${target}: icon must be SVG, PNG, JPEG, or WebP`) + if (mediaType === undefined) throw new Error(`${label}: icon must be SVG, PNG, JPEG, or WebP`) const file = realpathSync(target) const stat = statSync(file) - if (!stat.isFile()) throw new Error(`${target}: icon must be a regular file`) - if (stat.size > MAX_ICON_BYTES) throw new Error(`${target}: icon exceeds 256 KiB`) + if (!stat.isFile()) throw new Error(`${label}: icon must be a regular file`) + if (stat.size > MAX_ICON_BYTES) throw new Error(`${label}: icon exceeds 256 KiB`) const bytes = readFileSync(file) - if (bytes.length > MAX_ICON_BYTES) throw new Error(`${target}: icon exceeds 256 KiB`) + if (bytes.length > MAX_ICON_BYTES) throw new Error(`${label}: icon exceeds 256 KiB`) return `data:${mediaType};base64,${bytes.toString('base64')}` } diff --git a/packages/boot/app-boot/tests/package-meta.spec.ts b/packages/boot/app-boot/tests/package-meta.spec.ts index 9a0cf61075..98f5c9f658 100644 --- a/packages/boot/app-boot/tests/package-meta.spec.ts +++ b/packages/boot/app-boot/tests/package-meta.spec.ts @@ -207,7 +207,7 @@ describe('plugin locale display metadata', () => { file(join(dir, 'outside.svg'), 'outside') file(join(dir, 'fallback.svg'), 'fallback') const meta = readPluginMeta('localized', parentURL) - expect(meta?.error).toContain('icon must remain inside its manifest directory') + expect(meta?.error).toContain(`${join(dir, 'display', 'manifest.json')}: icon must remain inside its manifest directory`) expect(meta?.icon).toBeUndefined() }) diff --git a/scripts/check-workspace-constraints.ts b/scripts/check-workspace-constraints.ts index 413b11ff4a..b9c5cdcc25 100644 --- a/scripts/check-workspace-constraints.ts +++ b/scripts/check-workspace-constraints.ts @@ -80,7 +80,7 @@ export interface PackageManifest { main?: string types?: string bin?: string | Record - exports?: Record + exports?: Record files?: string[] icon?: string publishConfig?: { access?: string } @@ -94,6 +94,9 @@ export interface PackageManifest { } } +/** Node package export target: a path, a fallback list, a conditional map, or an exclusion. */ +export type ExportTarget = string | readonly ExportTarget[] | { readonly [condition: string]: ExportTarget | undefined } | null + /** One workspace manifest and its repo-relative path. */ export interface WorkspaceManifest { dir: string @@ -247,7 +250,7 @@ export function expectedDshPackageFiles(manifest: PackageManifest): readonly str ...bundleFiles, ...(manifest.name ? packageFileExtras[manifest.name] ?? [] : []), ] - const targets = (value: unknown): string[] => typeof value === 'string' ? [value] + const targets = (value: ExportTarget | undefined): string[] => typeof value === 'string' ? [value] : typeof value === 'object' && value !== null ? Object.values(value).flatMap(targets) : [] const icons = [ ...typeof manifest.icon === 'string' ? [manifest.icon.replace(/^\.\//u, '')] : [], @@ -298,10 +301,13 @@ export function expectedDshPackageFiles(manifest: PackageManifest): readonly str ] } -/** Fields of a conditional export; scalar and array targets have no named conditions. */ -function exportFields(value: unknown): Record | undefined { - return typeof value === 'object' && value !== null && !Array.isArray(value) - ? value as Record : undefined +/** Fields of a conditional export; scalar and list targets have no named conditions. */ +function exportFields(value: ExportTarget | undefined): { readonly [condition: string]: ExportTarget | undefined } | undefined { + return typeof value !== 'object' || value === null || isExportList(value) ? undefined : value +} + +function isExportList(value: ExportTarget): value is readonly ExportTarget[] { + return Array.isArray(value) } /** Whether one conditional export exactly names the generated runtime and declaration pair. */ diff --git a/scripts/verify-package-meta.spec.ts b/scripts/verify-package-meta.spec.ts index f778876443..8ab48457c8 100644 --- a/scripts/verify-package-meta.spec.ts +++ b/scripts/verify-package-meta.spec.ts @@ -529,3 +529,12 @@ it('reports metadata failures from more than one workspace package', () => { expect(problems).toContain('@test/plugin') expect(problems).toContain('@test/second') }) + +it.each([[], ['locale']])('rejects a display manifest in build output without reading it: locales %j', (...locales) => { + manifest({ exports: { './package.json': './lib/display.json', ...locales.length > 0 ? { './locale/*.json': './locale/*.json' } : {} }, files: ['lib', 'locale'] }) + if (locales.length > 0) json('locale/en.json', { meta: { title: 'Plugin' } }) + file('lib/display.json', '{') + const problems = packageMetaProblems(root).join('\n') + expect(problems).toContain('@test/plugin/package.json: display manifest must resolve to source JSON') + expect(problems).not.toContain('Plugin metadata for') +}) diff --git a/scripts/verify-package-meta.ts b/scripts/verify-package-meta.ts index 5e4f1c47ad..6e16c572b3 100644 --- a/scripts/verify-package-meta.ts +++ b/scripts/verify-package-meta.ts @@ -131,7 +131,12 @@ function packageProblems(manifestPath: string): string[] { problems.push(`${manifestPath}: exports["${iconExport}"] must resolve to a file`) } const iconDocument = displayManifest === undefined ? undefined : byPath.get(displayManifest) - if (resources.every(({ file }) => byPath.has(file))) { + // Build output is not validated source; reading it would make the result depend on stale artifacts. + const sourceManifest = displayManifest === undefined || iconDocument !== undefined + if (!sourceManifest) { + problems.push(`${specifier}/package.json: display manifest must resolve to source JSON, received ${displayManifest}`) + } + if (sourceManifest && resources.every(({ file }) => byPath.has(file))) { const meta = readPluginMeta(specifier, parentURL) if (meta?.error !== undefined) problems.push(meta.error) const iconTarget = meta?.icon === undefined ? undefined @@ -145,7 +150,7 @@ function packageProblems(manifestPath: string): string[] { problems.push(`${manifestPath}: files must include ${file}`) } } - } else if (exportsIcon || iconDocument?.icon !== undefined) { + } else if (sourceManifest && (exportsIcon || iconDocument?.icon !== undefined)) { problems.push(`${specifier}: icon metadata requires locale resources to resolve to source JSON`) } if (!resources.some(({ file }) => byPath.get(file)?.metadata || byPath.get(file)?.invalid)) continue