Tools: Updated script to build plugin repository

.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

.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

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();
+		}
+	});
+
+});

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');
+}

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
+	// At this point, we need to check the manifest ID as it's used in various
-	// done when publishing, and will be done anyway when the app attempts to
+	// places including as directory name and object key in manifests.json, so
-	// load the plugin. We just assume all files are valid here.
+	// 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