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.
This commit is contained in:
1 parent
d9a40155c1
commit
f8a274b76e
5 files changed
+38
-15
No files matched your search
@@ -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')}`
|
||||
}
|
||||
|
||||
|
||||
@@ -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()
|
||||
})
|
||||
|
||||
|
||||
@@ -80,7 +80,7 @@ export interface PackageManifest {
|
||||
main?: string
|
||||
types?: string
|
||||
bin?: string | Record<string, string>
|
||||
exports?: Record<string, unknown>
|
||||
exports?: Record<string, ExportTarget | undefined>
|
||||
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<string, unknown> | undefined {
|
||||
return typeof value === 'object' && value !== null && !Array.isArray(value)
|
||||
? value as Record<string, unknown> : 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. */
|
||||
|
||||
@@ -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')
|
||||
})
|
||||
@@ -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
|
||||
|
||||
Reference in new issue
Block a user