From 467de86454bf9176d71c0ded34502cf04974dc97 Mon Sep 17 00:00:00 2001 From: Yuki Takei Date: Thu, 27 Feb 2025 09:44:06 +0000 Subject: [PATCH 1/9] add try-catch --- .../server/service/import/overwrite-function.ts | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/apps/app/src/server/service/import/overwrite-function.ts b/apps/app/src/server/service/import/overwrite-function.ts index 1e6ca0fef7e..d1fde0bb5f5 100644 --- a/apps/app/src/server/service/import/overwrite-function.ts +++ b/apps/app/src/server/service/import/overwrite-function.ts @@ -5,6 +5,10 @@ import { Types, type Document, } from 'mongoose'; +import loggerFactory from '~/utils/logger'; + +const logger = loggerFactory('growi:service:import:overwrite-function'); + const { ObjectId } = Types; @@ -21,6 +25,10 @@ export type OverwriteFunction = (value: unknown, ctx: { document: Document, prop * @see https://mongoosejs.com/docs/api/schematype.html#schematype_SchemaType-cast */ export const keepOriginal: OverwriteFunction = (value, { document, schema, propertyName }) => { + if (value == null) { + return value; + } + // Model if (schema != null && schema.path(propertyName) != null) { const schemaType = schema.path(propertyName); @@ -30,7 +38,14 @@ export const keepOriginal: OverwriteFunction = (value, { document, schema, prope // ref: https://github.com/Automattic/mongoose/blob/6.11.4/lib/schema/array.js#L334 document.schema = schema; - return schemaType.cast(value, document, true); + try { + return schemaType.cast(value, document, true); + } + catch (e) { + logger.warn(`Failed to cast value for ${propertyName}`, e); + // return original value + return value; + } } // _id From f8113705dd52eef427133d30b9b5cad9fda428ec Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Wed, 5 Mar 2025 06:59:00 +0000 Subject: [PATCH 2/9] add joinAndValidatePath method --- .../server/services/growi-plugin/growi-plugin.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index 582fb4616dd..55979164279 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -402,6 +402,14 @@ export class GrowiPluginService implements IGrowiPluginService { return entries; } + private joinAndValidatePath(baseDir: string, ...paths: string[]):fs.PathLike { + const joinedPath = path.join(baseDir, ...paths); + if (!joinedPath.startsWith(baseDir)) { + throw new Error(`Invalid path: Outside of allowed directory - ${joinedPath}`); + } + return joinedPath; + } + } From 09add87b4a7cbe6118df862c086335c128605b83 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Wed, 5 Mar 2025 07:00:08 +0000 Subject: [PATCH 3/9] add error handling to downloadNotExistPluginRepositories function --- .../services/growi-plugin/growi-plugin.ts | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index 55979164279..f6f07bfb4c0 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -72,8 +72,23 @@ export class GrowiPluginService implements IGrowiPluginService { // if not exists repository in file system, download latest plugin repository for await (const growiPlugin of growiPlugins) { - const pluginPath = path.join(PLUGIN_STORING_PATH, growiPlugin.installedPath); - const organizationName = path.join(PLUGIN_STORING_PATH, growiPlugin.organizationName); + let pluginPath :fs.PathLike|undefined; + let organizationName :fs.PathLike|undefined; + try { + pluginPath = this.joinAndValidatePath(PLUGIN_STORING_PATH, growiPlugin.installedPath); + organizationName = this.joinAndValidatePath(PLUGIN_STORING_PATH, growiPlugin.organizationName); + } + catch (err) { + logger.error(err); + try { + await GrowiPlugin.deleteOne({ _id: growiPlugin.id }); + } + catch (deleteErr) { + logger.error(deleteErr); + throw new Error(`Failed to delete plugin from GrowiPlugin documents. Original error: ${deleteErr.message}`); + } + throw new Error('Failed to construct plugin path. The plugin document is also deleted.'); + } if (fs.existsSync(pluginPath)) { continue; } From 71bef31d7239fed307d47639703ba19e65d01441 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Wed, 5 Mar 2025 07:00:37 +0000 Subject: [PATCH 4/9] add errorhandling to deletePlugin function --- .../server/services/growi-plugin/growi-plugin.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index f6f07bfb4c0..ad2e879c6c9 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -315,8 +315,16 @@ export class GrowiPluginService implements IGrowiPluginService { throw new Error('No plugin found for this ID.'); } + let growiPluginsPath: fs.PathLike | undefined; + try { + growiPluginsPath = this.joinAndValidatePath(PLUGIN_STORING_PATH, growiPlugins.installedPath); + } + catch (err) { + logger.error(err); + throw new Error('Failed to constract plugin path'); + } + try { - const growiPluginsPath = path.join(PLUGIN_STORING_PATH, growiPlugins.installedPath); await deleteFolder(growiPluginsPath); } catch (err) { From a07a089e2518285309a114fee088ba22eb48d309 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Thu, 6 Mar 2025 03:19:27 +0000 Subject: [PATCH 5/9] do not delete while download --- .../server/services/growi-plugin/growi-plugin.ts | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index ad2e879c6c9..88f5906c1af 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -80,14 +80,7 @@ export class GrowiPluginService implements IGrowiPluginService { } catch (err) { logger.error(err); - try { - await GrowiPlugin.deleteOne({ _id: growiPlugin.id }); - } - catch (deleteErr) { - logger.error(deleteErr); - throw new Error(`Failed to delete plugin from GrowiPlugin documents. Original error: ${deleteErr.message}`); - } - throw new Error('Failed to construct plugin path. The plugin document is also deleted.'); + continue; } if (fs.existsSync(pluginPath)) { continue; From 30cac6922405d0fdf29e5319f670673848350990 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Thu, 6 Mar 2025 03:21:18 +0000 Subject: [PATCH 6/9] skip deletefolder when plugin path is invalid --- .../server/services/growi-plugin/growi-plugin.ts | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index 88f5906c1af..93964c04da4 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -314,7 +314,16 @@ export class GrowiPluginService implements IGrowiPluginService { } catch (err) { logger.error(err); - throw new Error('Failed to constract plugin path'); + + try { + await GrowiPlugin.deleteOne({ _id: pluginId }); + logger.warn(`Deleted invalid plugin (ID: ${pluginId}) from database. Skipped directory removal.`); + } + catch (deleteErr) { + logger.error(deleteErr); + throw new Error('Failed to delete invalid plugin from GrowiPlugin documents.'); + } + return growiPlugins.meta.name; } try { From 7237dac759c92a47e20020e1358a817cc8e8edd1 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Thu, 6 Mar 2025 03:22:22 +0000 Subject: [PATCH 7/9] make it possible to delete plugin before install --- .../server/services/growi-plugin/growi-plugin.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index 93964c04da4..1947fca97ee 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -326,12 +326,17 @@ export class GrowiPluginService implements IGrowiPluginService { return growiPlugins.meta.name; } - try { - await deleteFolder(growiPluginsPath); + if (growiPluginsPath && fs.existsSync(growiPluginsPath)) { + try { + await deleteFolder(growiPluginsPath); + } + catch (err) { + logger.error(err); + throw new Error('Failed to delete plugin repository.'); + } } - catch (err) { - logger.error(err); - throw new Error('Failed to delete plugin repository.'); + else { + logger.warn(`Plugin path does not exist : ${growiPluginsPath}`); } try { From 062998e310bc1040c47face049d08abf71773657 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Thu, 6 Mar 2025 03:22:44 +0000 Subject: [PATCH 8/9] improve error message --- .../server/services/growi-plugin/growi-plugin.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index 1947fca97ee..75f8dd15681 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -435,7 +435,10 @@ export class GrowiPluginService implements IGrowiPluginService { private joinAndValidatePath(baseDir: string, ...paths: string[]):fs.PathLike { const joinedPath = path.join(baseDir, ...paths); if (!joinedPath.startsWith(baseDir)) { - throw new Error(`Invalid path: Outside of allowed directory - ${joinedPath}`); + throw new Error( + 'Invalid plugin path detected! Access outside of the allowed directory is not permitted.' + + `\nAttempted Path: ${joinedPath}`, + ); } return joinedPath; } From e885dce8767241db878594a98773880029edb3f8 Mon Sep 17 00:00:00 2001 From: NaokiHigashi28 Date: Tue, 11 Mar 2025 01:34:52 +0000 Subject: [PATCH 9/9] simplize error handling --- .../services/growi-plugin/growi-plugin.ts | 28 ++++++------------- 1 file changed, 9 insertions(+), 19 deletions(-) diff --git a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts index 75f8dd15681..c4d24578c20 100644 --- a/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts +++ b/apps/app/src/features/growi-plugin/server/services/growi-plugin/growi-plugin.ts @@ -308,22 +308,21 @@ export class GrowiPluginService implements IGrowiPluginService { throw new Error('No plugin found for this ID.'); } + try { + await GrowiPlugin.deleteOne({ _id: pluginId }); + } + catch (err) { + logger.error(err); + throw new Error('Failed to delete plugin from GrowiPlugin documents.'); + } + let growiPluginsPath: fs.PathLike | undefined; try { growiPluginsPath = this.joinAndValidatePath(PLUGIN_STORING_PATH, growiPlugins.installedPath); } catch (err) { logger.error(err); - - try { - await GrowiPlugin.deleteOne({ _id: pluginId }); - logger.warn(`Deleted invalid plugin (ID: ${pluginId}) from database. Skipped directory removal.`); - } - catch (deleteErr) { - logger.error(deleteErr); - throw new Error('Failed to delete invalid plugin from GrowiPlugin documents.'); - } - return growiPlugins.meta.name; + throw new Error('The installedPath for the plugin is invalid, and the plugin has already been removed.'); } if (growiPluginsPath && fs.existsSync(growiPluginsPath)) { @@ -338,15 +337,6 @@ export class GrowiPluginService implements IGrowiPluginService { else { logger.warn(`Plugin path does not exist : ${growiPluginsPath}`); } - - try { - await GrowiPlugin.deleteOne({ _id: pluginId }); - } - catch (err) { - logger.error(err); - throw new Error('Failed to delete plugin from GrowiPlugin documents.'); - } - return growiPlugins.meta.name; }