From 8d4d438136f70bc7dcec447f0f236ff8e4127def Mon Sep 17 00:00:00 2001 From: Laurent Cozic Date: Wed, 6 Jan 2021 20:23:23 +0000 Subject: [PATCH] Tools: Updated script to build plugin repository --- .eslintignore | 6 ++++ .gitignore | 6 ++++ .../plugins/utils/validatePluginId.test.ts | 33 +++++++++++++++++++ .../plugins/utils/validatePluginId.ts | 11 +++++++ packages/tools/build-plugin-repository.ts | 10 ++++-- 5 files changed, 63 insertions(+), 3 deletions(-) create mode 100644 packages/lib/services/plugins/utils/validatePluginId.test.ts create mode 100644 packages/lib/services/plugins/utils/validatePluginId.ts diff --git a/.eslintignore b/.eslintignore index 8cf0a2ba4..063a87441 100644 --- a/.eslintignore +++ b/.eslintignore @@ -1205,6 +1205,12 @@ packages/lib/services/plugins/utils/mapEventHandlersToIds.js.map packages/lib/services/plugins/utils/types.d.ts packages/lib/services/plugins/utils/types.js packages/lib/services/plugins/utils/types.js.map +packages/lib/services/plugins/utils/validatePluginId.d.ts +packages/lib/services/plugins/utils/validatePluginId.js +packages/lib/services/plugins/utils/validatePluginId.js.map +packages/lib/services/plugins/utils/validatePluginId.test.d.ts +packages/lib/services/plugins/utils/validatePluginId.test.js +packages/lib/services/plugins/utils/validatePluginId.test.js.map packages/lib/services/rest/Api.d.ts packages/lib/services/rest/Api.js packages/lib/services/rest/Api.js.map diff --git a/.gitignore b/.gitignore index c0a2712d0..a80a33285 100644 --- a/.gitignore +++ b/.gitignore @@ -1194,6 +1194,12 @@ packages/lib/services/plugins/utils/mapEventHandlersToIds.js.map packages/lib/services/plugins/utils/types.d.ts packages/lib/services/plugins/utils/types.js packages/lib/services/plugins/utils/types.js.map +packages/lib/services/plugins/utils/validatePluginId.d.ts +packages/lib/services/plugins/utils/validatePluginId.js +packages/lib/services/plugins/utils/validatePluginId.js.map +packages/lib/services/plugins/utils/validatePluginId.test.d.ts +packages/lib/services/plugins/utils/validatePluginId.test.js +packages/lib/services/plugins/utils/validatePluginId.test.js.map packages/lib/services/rest/Api.d.ts packages/lib/services/rest/Api.js packages/lib/services/rest/Api.js.map diff --git a/packages/lib/services/plugins/utils/validatePluginId.test.ts b/packages/lib/services/plugins/utils/validatePluginId.test.ts new file mode 100644 index 000000000..583cd4eb7 --- /dev/null +++ b/packages/lib/services/plugins/utils/validatePluginId.test.ts @@ -0,0 +1,33 @@ +import validatePluginId from './validatePluginId'; + +describe('validatePluginId', () => { + + test('should validate an ID', () => { + const okCases = [ + 'thatsok', + 'that-s-ok', + 'that_s_fine12', + 'com.ok.too', + ]; + + const errorCases = [ + '', + 'verylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongidverylongid', + 'NO', + 'NotGood', + '-shouldstartwiththis', + 'shouldntendwithit.', + ' no space ', + 'no space', + ]; + + for (const t of okCases) { + expect(() => validatePluginId(t)).not.toThrow(); + } + + for (const t of errorCases) { + expect(() => validatePluginId(t)).toThrow(); + } + }); + +}); diff --git a/packages/lib/services/plugins/utils/validatePluginId.ts b/packages/lib/services/plugins/utils/validatePluginId.ts new file mode 100644 index 000000000..168c3e401 --- /dev/null +++ b/packages/lib/services/plugins/utils/validatePluginId.ts @@ -0,0 +1,11 @@ +export default function(id: string): void { + if (!id) throw new Error('ID cannot be empty'); + if (id.length > 256) throw new Error('ID cannot be longer than 256 characters'); + + const whitelist = '0-9a-zA-Z._-'; + const regex = new RegExp(`^[${whitelist}]+$`); + if (!id.match(regex)) throw new Error(`ID "${id}" contains invalid characters. Only the characters "${whitelist}" are allowed.`); + + if (!id[0].match(/[0-9a-zA-Z]/)) throw new Error('ID can only start with a number or letter'); + if (!id[id.length - 1].match(/[0-9a-zA-Z]/)) throw new Error('ID can only end with a number or letter'); +} diff --git a/packages/tools/build-plugin-repository.ts b/packages/tools/build-plugin-repository.ts index de376d489..ffcf8c43d 100644 --- a/packages/tools/build-plugin-repository.ts +++ b/packages/tools/build-plugin-repository.ts @@ -1,6 +1,7 @@ import * as fs from 'fs-extra'; import * as path from 'path'; import * as process from 'process'; +import validatePluginId from '@joplin/lib/services/plugins/utils/validatePluginId'; const { execCommand, execCommandVerbose, rootDir, resolveRelativePathWithinDir, gitPullTry } = require('./tool-utils.js'); interface NpmPackage { @@ -61,10 +62,13 @@ async function extractPluginFilesFromPackage(originalPluginManifests: any, workD if (!(await fs.pathExists(manifestFilePath))) throw new Error(`Could not find manifest file at ${manifestFilePath}`); if (!(await fs.pathExists(pluginFilePath))) throw new Error(`Could not find plugin file at ${pluginFilePath}`); - // At this point we don't validate any of the plugin files as it's partly - // done when publishing, and will be done anyway when the app attempts to - // load the plugin. We just assume all files are valid here. + // At this point, we need to check the manifest ID as it's used in various + // places including as directory name and object key in manifests.json, so + // it needs to be correct. It's mostly for security reasons. The other + // manifest properties are checked when the plugin is loaded into the app. const manifest = await readJsonFile(manifestFilePath); + validatePluginId(manifest.id); + manifest._npm_package_name = packageName; // If there's already a plugin with this ID published under a different