From 3aa7319d769597f34674e02056852422bb072ee4 Mon Sep 17 00:00:00 2001 From: adrian Date: Fri, 13 Mar 2026 19:58:35 -0500 Subject: [PATCH] fix: isolate module uploads in modules// and remove core coupling --- docs/EXTENSIONS_MODULES_README.md | 2 +- package.json | 1 + services/moduleArchiveService.js | 142 +++++++++++++++++++++--------- services/reviewService.js | 6 +- services/systemModuleService.js | 33 ++++--- 5 files changed, 127 insertions(+), 57 deletions(-) diff --git a/docs/EXTENSIONS_MODULES_README.md b/docs/EXTENSIONS_MODULES_README.md index 9e8b1b0..a1ebb87 100644 --- a/docs/EXTENSIONS_MODULES_README.md +++ b/docs/EXTENSIONS_MODULES_README.md @@ -27,7 +27,7 @@ Esto permite extender funcionalidades sin comprometer el servidor. - Hash SHA-256 del manifiesto guardado para auditoría. - Activación/desactivación solo por endpoints admin autenticados. - Puntos finales acotados en backend (`min 0`, `max 1000`). -- Cada módulo subido se guarda en disco en la carpeta `modules/`. +- Cada módulo subido se guarda y extrae en disco en su carpeta propia: `modules//`. ## Estructura del .zip diff --git a/package.json b/package.json index 09c0a15..140c39d 100644 --- a/package.json +++ b/package.json @@ -16,6 +16,7 @@ "hardhat": "^2.22.2" }, "dependencies": { + "adm-zip": "^0.5.16", "dotenv": "^17.2.3", "ethers": "^6.16.0", "express": "^4.19.2", diff --git a/services/moduleArchiveService.js b/services/moduleArchiveService.js index ab3ae89..80f0552 100644 --- a/services/moduleArchiveService.js +++ b/services/moduleArchiveService.js @@ -1,13 +1,12 @@ const fs = require('fs/promises'); -const os = require('os'); const path = require('path'); const { randomUUID } = require('crypto'); -const { execFile } = require('child_process'); -const { promisify } = require('util'); +const AdmZip = require('adm-zip'); const { ApiError } = require('../middlewares/errorHandler'); -const execFileAsync = promisify(execFile); const MAX_MANIFEST_BYTES = 250_000; +const MAX_ZIP_ENTRY_BYTES = 5_000_000; +const MAX_ZIP_ENTRY_COUNT = 500; function getModulesBaseDir() { if (process.env.MODULES_DIR) { @@ -21,45 +20,78 @@ async function ensureModulesBaseDir() { return getModulesBaseDir(); } -function sanitizeModuleFileName(moduleKey) { +function sanitizeModuleFolderName(moduleKey) { return String(moduleKey || '') .trim() .replace(/[^a-zA-Z0-9._@-]/g, '_') .slice(0, 120) || `module-${randomUUID()}`; } +function normalizeZipEntryName(entryName) { + return String(entryName || '') + .replace(/\\/g, '/') + .replace(/^\/+/, ''); +} + +function assertSafeZipEntryPath(entryName) { + const normalized = normalizeZipEntryName(entryName); + if (!normalized || normalized.endsWith('/')) return null; + if (normalized.includes('..')) { + throw new ApiError(400, `zip contains unsafe path: ${entryName}`); + } + if (path.isAbsolute(normalized)) { + throw new ApiError(400, `zip contains absolute path: ${entryName}`); + } + return normalized; +} + +function findManifestEntry(entries) { + const candidates = entries.filter((entry) => { + const normalized = normalizeZipEntryName(entry.entryName).toLowerCase(); + return ( + normalized.endsWith('/manifest.json') || + normalized.endsWith('/module.json') || + normalized === 'manifest.json' || + normalized === 'module.json' + ); + }); + + if (candidates.length === 0) { + throw new ApiError(400, 'zip must include manifest.json or module.json'); + } + + return candidates.sort((a, b) => a.entryName.length - b.entryName.length)[0]; +} + +function getZipEntries(zipBuffer) { + const zip = new AdmZip(zipBuffer); + const entries = zip.getEntries().filter((entry) => !entry.isDirectory); + if (entries.length === 0) { + throw new ApiError(400, 'zip is empty'); + } + if (entries.length > MAX_ZIP_ENTRY_COUNT) { + throw new ApiError(400, `zip contains too many files (max ${MAX_ZIP_ENTRY_COUNT})`); + } + return entries; +} + async function extractManifestFromZipBuffer(zipBuffer) { if (!Buffer.isBuffer(zipBuffer) || zipBuffer.length === 0) { throw new ApiError(400, 'zip file is required'); } - const tempFile = path.join(os.tmpdir(), `syspoints-module-${randomUUID()}.zip`); - await fs.writeFile(tempFile, zipBuffer); - try { - const { stdout: listStdout } = await execFileAsync('unzip', ['-Z1', tempFile], { maxBuffer: 2 * 1024 * 1024 }); - const entries = String(listStdout || '') - .split('\n') - .map((line) => line.trim()) - .filter(Boolean); - - const manifestCandidates = entries.filter((entry) => { - const normalized = entry.replace(/\\/g, '/').toLowerCase(); - if (normalized.endsWith('/')) return false; - return normalized.endsWith('/manifest.json') || normalized.endsWith('/module.json') || normalized === 'manifest.json' || normalized === 'module.json'; - }); - - if (manifestCandidates.length === 0) { - throw new ApiError(400, 'zip must include manifest.json or module.json'); + const entries = getZipEntries(zipBuffer); + const manifestEntry = findManifestEntry(entries); + const manifestBuffer = manifestEntry.getData(); + if (!manifestBuffer || manifestBuffer.length === 0) { + throw new ApiError(400, 'manifest file is empty'); + } + if (manifestBuffer.length > MAX_MANIFEST_BYTES) { + throw new ApiError(400, `manifest must be <= ${MAX_MANIFEST_BYTES} bytes`); } - const manifestEntry = manifestCandidates.sort((a, b) => a.length - b.length)[0]; - const { stdout: manifestStdout } = await execFileAsync('unzip', ['-p', tempFile, manifestEntry], { - maxBuffer: MAX_MANIFEST_BYTES, - encoding: 'utf8', - }); - - const manifestText = String(manifestStdout || '').trim(); + const manifestText = manifestBuffer.toString('utf8').trim(); if (!manifestText) { throw new ApiError(400, 'manifest file is empty'); } @@ -71,31 +103,59 @@ async function extractManifestFromZipBuffer(zipBuffer) { } } catch (error) { if (error instanceof ApiError) throw error; - if (/maxBuffer/i.test(String(error?.message || ''))) { - throw new ApiError(400, `manifest must be <= ${MAX_MANIFEST_BYTES} bytes`); - } - throw new ApiError(400, 'failed to read zip manifest. Ensure unzip is available and archive is valid'); - } finally { - await fs.unlink(tempFile).catch(() => {}); + throw new ApiError(400, `failed to read zip manifest: ${String(error?.message || 'invalid archive')}`); } } -async function saveModuleArchive({ moduleKey, zipBuffer }) { +async function saveModuleBundle({ moduleKey, zipBuffer }) { if (!Buffer.isBuffer(zipBuffer) || zipBuffer.length === 0) { throw new ApiError(400, 'zip file is required'); } const baseDir = await ensureModulesBaseDir(); - const safeFileName = `${sanitizeModuleFileName(moduleKey)}.zip`; - const archivePath = path.join(baseDir, safeFileName); + const moduleFolderName = sanitizeModuleFolderName(moduleKey); + const moduleDir = path.join(baseDir, moduleFolderName); + + await fs.mkdir(moduleDir, { recursive: false }); + + try { + const archivePath = path.join(moduleDir, 'module.zip'); + await fs.writeFile(archivePath, zipBuffer, { flag: 'wx' }); + + const entries = getZipEntries(zipBuffer); + for (const entry of entries) { + const safeRelativePath = assertSafeZipEntryPath(entry.entryName); + if (!safeRelativePath) continue; + + const entryBuffer = entry.getData(); + if (entryBuffer.length > MAX_ZIP_ENTRY_BYTES) { + throw new ApiError(400, `zip entry too large: ${safeRelativePath}`); + } + + const targetPath = path.join(moduleDir, safeRelativePath); + const normalizedTarget = path.normalize(targetPath); + const normalizedModuleDir = path.normalize(moduleDir + path.sep); + if (!normalizedTarget.startsWith(normalizedModuleDir)) { + throw new ApiError(400, `zip contains unsafe extraction path: ${safeRelativePath}`); + } + + await fs.mkdir(path.dirname(targetPath), { recursive: true }); + await fs.writeFile(targetPath, entryBuffer, { flag: 'wx' }); + } - await fs.writeFile(archivePath, zipBuffer, { flag: 'wx' }); - return archivePath; + return { + module_dir: moduleDir, + archive_path: archivePath, + }; + } catch (err) { + await fs.rm(moduleDir, { recursive: true, force: true }).catch(() => {}); + throw err; + } } module.exports = { getModulesBaseDir, ensureModulesBaseDir, extractManifestFromZipBuffer, - saveModuleArchive, + saveModuleBundle, }; diff --git a/services/reviewService.js b/services/reviewService.js index 4d27f33..ff8e6d3 100644 --- a/services/reviewService.js +++ b/services/reviewService.js @@ -27,7 +27,7 @@ const { hashReviewPayload } = require('../utils/hash'); const { getCurrentConfig } = require('../repositories/pointsConfigRepository'); const { findByUserAndKey, saveResponse } = require('../repositories/idempotencyRepository'); const { findShareByUserReviewAndPlatform, createReviewShare } = require('../repositories/reviewShareRepository'); -const { systemModuleService, REVIEW_SHARE_MODULE_KEY, REVIEW_SHARE_PLATFORMS } = require('./systemModuleService'); +const { systemModuleService, REVIEW_SHARE_PLATFORMS } = require('./systemModuleService'); const idempotencyCache = new Map(); const DEFAULT_MAX_REVIEW_TAGS = 5; @@ -709,8 +709,8 @@ async function shareReviewService({ reviewId, userId, platform }) { throw new ApiError(400, `platform must be one of: ${REVIEW_SHARE_PLATFORMS.join(', ')}`); } - const moduleEnabled = await systemModuleService.isModuleActive(REVIEW_SHARE_MODULE_KEY); - if (!moduleEnabled) { + const activeShareModule = await systemModuleService.getActiveReviewShareModule(); + if (!activeShareModule) { throw new ApiError(409, 'review share module is not active'); } diff --git a/services/systemModuleService.js b/services/systemModuleService.js index 2a5b7c9..5ceca8a 100644 --- a/services/systemModuleService.js +++ b/services/systemModuleService.js @@ -9,10 +9,9 @@ const { deactivateModule, } = require('../repositories/systemModuleRepository'); const { ApiError } = require('../middlewares/errorHandler'); -const { extractManifestFromZipBuffer, saveModuleArchive } = require('./moduleArchiveService'); +const { extractManifestFromZipBuffer, saveModuleBundle } = require('./moduleArchiveService'); const MODULE_ENGINE = 'syspoints-module-v1'; -const REVIEW_SHARE_MODULE_KEY = 'review-share@1.0.0'; const REVIEW_SHARE_PLATFORMS = ['telegram', 'x', 'whatsapp', 'linkedin', 'facebook', 'instagram']; const MODULE_NAME_REGEX = /^[a-z0-9][a-z0-9._-]{1,49}$/; @@ -347,9 +346,9 @@ async function uploadModuleArchive({ zipBuffer, uploadedBy }) { throw new ApiError(409, 'module already exists for this name/version'); } - let archivePath = null; + let savedBundle = null; try { - archivePath = await saveModuleArchive({ moduleKey, zipBuffer }); + savedBundle = await saveModuleBundle({ moduleKey, zipBuffer }); const created = await insertModule({ query }, { module_key: moduleKey, name: normalizedManifest.name, @@ -363,15 +362,16 @@ async function uploadModuleArchive({ zipBuffer, uploadedBy }) { }); return { ...formatModule(created), - archive_path: archivePath, + archive_path: savedBundle.archive_path, + module_dir: savedBundle.module_dir, }; } catch (err) { - if (archivePath) { + if (savedBundle?.module_dir) { const fs = require('fs/promises'); - await fs.unlink(archivePath).catch(() => {}); + await fs.rm(savedBundle.module_dir, { recursive: true, force: true }).catch(() => {}); } if (err.code === 'EEXIST') { - throw new ApiError(409, 'module archive already exists in modules directory'); + throw new ApiError(409, 'module directory already exists in modules'); } if (err.code === '23505') { throw new ApiError(409, 'module already exists for this name/version'); @@ -408,13 +408,22 @@ async function isModuleActive(moduleKey) { return activeModules.some((item) => item.module_key === moduleKey); } -async function getReviewShareModuleConfig() { +async function getActiveReviewShareModule() { const activeModules = await getActiveModulesCached(); - const shareModule = activeModules.find((item) => item.module_key === REVIEW_SHARE_MODULE_KEY); + return activeModules.find((item) => { + const permissions = Array.isArray(item?.manifest?.permissions) ? item.manifest.permissions : []; + const hasSharePermission = permissions.includes('review:share'); + const hasShareHook = Boolean(item?.manifest?.hooks?.review_share); + return hasSharePermission && hasShareHook; + }) || null; +} + +async function getReviewShareModuleConfig() { + const shareModule = await getActiveReviewShareModule(); if (!shareModule) { return { active: false, - module_key: REVIEW_SHARE_MODULE_KEY, + module_key: null, label_es: 'Compartir', label_en: 'Share', platforms: [...REVIEW_SHARE_PLATFORMS], @@ -485,7 +494,6 @@ async function applyPointAdjustments({ } module.exports = { - REVIEW_SHARE_MODULE_KEY, REVIEW_SHARE_PLATFORMS, systemModuleService: { listAllModules, @@ -494,6 +502,7 @@ module.exports = { activateModuleByKey, deactivateModuleByKey, isModuleActive, + getActiveReviewShareModule, getReviewShareModuleConfig, applyPointAdjustments, normalizeAndValidateManifest,