diff --git a/docker/Dockerfile b/docker/Dockerfile index 34a500d48..71fe93552 100644 --- a/docker/Dockerfile +++ b/docker/Dockerfile @@ -35,9 +35,6 @@ LABEL org.opencontainers.image.version=${VERSION} COPY --from=deps /app/node_modules ./node_modules COPY dist ./dist -# To prevent `Error: EACCES: permission denied, mkdir '/app/dist/external_converters'` -# when running rootless. -RUN chmod -R 777 ./dist COPY package.json LICENSE index.js data/configuration.example.yaml ./ COPY docker/docker-entrypoint.sh /usr/local/bin/ diff --git a/lib/extension/externalJS.ts b/lib/extension/externalJS.ts index d86f3e406..bf43951a2 100644 --- a/lib/extension/externalJS.ts +++ b/lib/extension/externalJS.ts @@ -1,6 +1,7 @@ import type {Zigbee2MQTTAPI, Zigbee2MQTTResponse} from "../types/api"; import fs from "node:fs"; +import os from "node:os"; import path from "node:path"; import bind from "bind-decorator"; @@ -13,13 +14,14 @@ import utils from "../util/utils"; import Extension from "./extension"; const SUPPORTED_OPERATIONS = ["save", "remove"]; +const TMP_PREFIX = ".tmp-ed42d4f2-"; export default abstract class ExternalJSExtension extends Extension { protected folderName: string; protected mqttTopic: string; protected requestRegex: RegExp; protected basePath: string; - protected srcBasePath: string; + protected nodeModulesSymlinked = false; constructor( zigbee: Zigbee, @@ -39,13 +41,33 @@ export default abstract class ExternalJSExtension extends Extension { this.mqttTopic = mqttTopic; this.requestRegex = new RegExp(`${settings.get().mqtt.base_topic}/bridge/request/${mqttTopic}/(save|remove)`); this.basePath = data.joinPath(folderName); - // 1-up from this file - this.srcBasePath = path.join( - __dirname, - "..", - // prevent race in vitest with files being manipulated from same location - process.env.VITEST_WORKER_ID ? /* v8 ignore next */ `${folderName}_${Math.floor(Math.random() * 10000)}` : folderName, - ); + } + + /** + * In case the external JS is not in the Z2M install dir (e.g. when `ZIGBEE2MQTT_DATA` is used), the external + * JS cannot import from `node_modules`. + * To workaround this create a symlink to `node_modules` in the external JS dir. + * https://nodejs.org/api/esm.html#no-node_path + */ + private symlinkNodeModulesIfNecessary() { + if (!this.nodeModulesSymlinked) { + this.nodeModulesSymlinked = true; + const nodeModulesPath = path.join(__dirname, "..", "..", "node_modules"); + const z2mDirNormalized = `${path.resolve(path.join(nodeModulesPath, ".."))}${path.sep}`; + const basePathNormalized = `${path.resolve(this.basePath)}${path.sep}`; + const basePathInZ2mDir = basePathNormalized.startsWith(z2mDirNormalized); + if (!basePathInZ2mDir) { + logger.debug(`External JS folder '${this.folderName}' is outside the Z2M install dir, creating a symlink to 'node_modules'`); + const nodeModulesSymlink = path.join(this.basePath, "node_modules"); + if (fs.existsSync(nodeModulesSymlink)) { + fs.unlinkSync(nodeModulesSymlink); + } + // Type `junction` is required on Windows. + // https://github.com/nodejs/node/issues/18518#issuecomment-513866491 + /* v8 ignore next */ + fs.symlinkSync(nodeModulesPath, nodeModulesSymlink, os.platform() === "win32" ? "junction" : "dir"); + } + } } override async start(): Promise { @@ -55,36 +77,24 @@ export default abstract class ExternalJSExtension extends Extension { await this.publishExternalJS(); } - override async stop(): Promise { - // remove src base path on stop to ensure always back to default - fs.rmSync(this.srcBasePath, {force: true, recursive: true}); - await super.stop(); - } - - private getFilePath(name: string, mkBasePath = false, inSource = false): string { - const basePath = inSource ? this.srcBasePath : this.basePath; - - if (mkBasePath && !fs.existsSync(basePath)) { - fs.mkdirSync(basePath, {recursive: true}); + private getFilePath(name: string, mkBasePath = false): string { + if (mkBasePath && !fs.existsSync(this.basePath)) { + fs.mkdirSync(this.basePath, {recursive: true}); } - return path.join(basePath, name); + return path.join(this.basePath, name); } protected getFileCode(name: string): string { return fs.readFileSync(this.getFilePath(name), "utf8"); } - protected *getFiles(inSource = false): Generator<{name: string; code: string}> { - const basePath = inSource ? this.srcBasePath : this.basePath; - - if (!fs.existsSync(basePath)) { - return; - } - - for (const fileName of fs.readdirSync(basePath)) { - if (fileName.endsWith(".js") || fileName.endsWith(".cjs") || fileName.endsWith(".mjs")) { - yield {name: fileName, code: this.getFileCode(fileName)}; + protected *getFiles(): Generator<{name: string; code: string}> { + if (fs.existsSync(this.basePath)) { + for (const fileName of fs.readdirSync(this.basePath)) { + if (!fileName.startsWith(TMP_PREFIX) && (fileName.endsWith(".js") || fileName.endsWith(".cjs") || fileName.endsWith(".mjs"))) { + yield {name: fileName, code: this.getFileCode(fileName)}; + } } } } @@ -131,14 +141,12 @@ export default abstract class ExternalJSExtension extends Extension { } const {name} = message; - const srcToBeRemoved = this.getFilePath(name, false, true); const toBeRemoved = this.getFilePath(name); - if (fs.existsSync(srcToBeRemoved)) { - const mod = await import(this.getImportPath(srcToBeRemoved)); + if (fs.existsSync(toBeRemoved)) { + const mod = await this.importFile(toBeRemoved); await this.removeJS(name, mod.default); - fs.rmSync(srcToBeRemoved, {force: true}); fs.rmSync(toBeRemoved, {force: true}); logger.info(`${name} (${toBeRemoved}) removed.`); await this.publishExternalJS(); @@ -146,7 +154,7 @@ export default abstract class ExternalJSExtension extends Extension { return utils.getResponse(message, {}); } - return utils.getResponse(message, {}, `${name} (${srcToBeRemoved}) doesn't exists`); + return utils.getResponse(message, {}, `${name} (${toBeRemoved}) doesn't exists`); } @bind private async save( @@ -157,63 +165,34 @@ export default abstract class ExternalJSExtension extends Extension { } const {name, code} = message; - const srcFilePath = this.getFilePath(name, true, true); - let newName = name; - - if (fs.existsSync(srcFilePath)) { - // if file already exist, version it to bypass node module caching - const versionMatch = name.match(/\.(\d+)\.(c|m)?js$/); - - if (versionMatch) { - const version = Number.parseInt(versionMatch[1], 10); - newName = name.replace(`.${version}.`, `.${version + 1}.`); - } else { - const ext = path.extname(name); - newName = name.replace(ext, `.1${ext}`); - } - - // remove previous version - fs.rmSync(srcFilePath, {force: true}); - fs.rmSync(this.getFilePath(name, true, false), {force: true}); - } - - const newSrcFilePath = this.getFilePath(newName, false /* already created above if needed */, true); - + const filePath = this.getFilePath(name, true); try { - fs.writeFileSync(newSrcFilePath, code, "utf8"); + fs.writeFileSync(filePath, code, "utf8"); + this.symlinkNodeModulesIfNecessary(); - const mod = await import(this.getImportPath(newSrcFilePath)); + const mod = await this.importFile(filePath); - await this.loadJS(name, mod.default, newName); - logger.info(`${newName} loaded. Contents written to '${newSrcFilePath}'.`); - // keep original in data folder synced - fs.writeFileSync(this.getFilePath(newName, true, false), code, "utf8"); + await this.loadJS(name, mod.default, name); + logger.info(`${name} loaded. Contents written to '${filePath}'.`); await this.publishExternalJS(); return utils.getResponse(message, {}); } catch (error) { - fs.rmSync(newSrcFilePath, {force: true}); - // NOTE: original in data folder doesn't get written if invalid - - return utils.getResponse(message, {}, `${newName} contains invalid code: ${(error as Error).message}`); + return utils.getResponse(message, {}, `${name} contains invalid code: ${(error as Error).message}`); } } private async loadFiles(): Promise { for (const extension of this.getFiles()) { - const srcFilePath = this.getFilePath(extension.name, true, true); + this.symlinkNodeModulesIfNecessary(); const filePath = this.getFilePath(extension.name); try { - fs.copyFileSync(filePath, srcFilePath); - - const mod = await import(this.getImportPath(srcFilePath)); - + const mod = await this.importFile(filePath); await this.loadJS(extension.name, mod.default); } catch (error) { // change ext so Z2M doesn't try to load it again and again fs.renameSync(filePath, `${filePath}.invalid`); - fs.rmSync(srcFilePath, {force: true}); logger.error( `Invalid external ${this.mqttTopic} '${extension.name}' was ignored and renamed to prevent interference with Zigbee2MQTT.`, @@ -225,14 +204,24 @@ export default abstract class ExternalJSExtension extends Extension { } private async publishExternalJS(): Promise { - await this.mqtt.publish(`bridge/${this.mqttTopic}s`, stringify(Array.from(this.getFiles(true))), { + await this.mqtt.publish(`bridge/${this.mqttTopic}s`, stringify(Array.from(this.getFiles())), { clientOptions: {retain: true}, skipLog: true, }); } - private getImportPath(filePath: string): string { - // prevent issues on Windows - return path.relative(__dirname, filePath).replaceAll("\\", "/"); + // biome-ignore lint/suspicious/noExplicitAny: dynamic module + private async importFile(file: string): Promise { + const ext = path.extname(file); + // Create the file in a temp path to bypass node module cache when importing multiple times. + const tmpFile = path.join(this.basePath, `${TMP_PREFIX}${path.basename(file, ext)}-${crypto.randomUUID()}${ext}`); + fs.copyFileSync(file, tmpFile); + try { + // Do `replaceAll("\\", "/")` to prevent issues on Windows + const mod = await import(tmpFile.replaceAll("\\", "/")); + return mod; + } finally { + fs.rmSync(tmpFile); + } } } diff --git a/test/extensions/externalConverters.test.ts b/test/extensions/externalConverters.test.ts index 52741f8b2..a222982a0 100644 --- a/test/extensions/externalConverters.test.ts +++ b/test/extensions/externalConverters.test.ts @@ -283,7 +283,7 @@ describe("Extension: ExternalConverters", () => { "zigbee2mqtt/bridge/converters", stringify([ {name: "mock-external-converter-multiple.js", code: getFileCode("cjs", "mock-external-converter-multiple.js")}, - {name: "mock-external-converter.1.js", code: converterCode}, + {name: "mock-external-converter.js", code: converterCode}, ]), {retain: true}, ); @@ -294,7 +294,7 @@ describe("Extension: ExternalConverters", () => { vendor: "external", model: "external_converter_device", description: "external/converter/edited", - externalConverterName: "mock-external-converter.1.js", + externalConverterName: "mock-external-converter.js", }), ); @@ -302,7 +302,7 @@ describe("Extension: ExternalConverters", () => { await (controller.getExtension("ExternalConverters")! as ExternalConverters).onMQTTMessage({ topic: "zigbee2mqtt/bridge/request/converter/save", - message: {name: "mock-external-converter.1.js", code: converterCode}, + message: {name: "mock-external-converter.js", code: converterCode}, }); expect(getZ2MDevice(devices.external_converter_device).definition).toMatchObject({ @@ -315,7 +315,7 @@ describe("Extension: ExternalConverters", () => { "zigbee2mqtt/bridge/converters", stringify([ {name: "mock-external-converter-multiple.js", code: getFileCode("cjs", "mock-external-converter-multiple.js")}, - {name: "mock-external-converter.2.js", code: converterCode}, + {name: "mock-external-converter.js", code: converterCode}, ]), {retain: true}, ); @@ -326,7 +326,7 @@ describe("Extension: ExternalConverters", () => { vendor: "external", model: "external_converter_device", description: "external/converter", - externalConverterName: "mock-external-converter.2.js", + externalConverterName: "mock-external-converter.js", }), ); }); @@ -356,6 +356,10 @@ describe("Extension: ExternalConverters", () => { describe("from MQTT", () => { it("CJS: saves and removes", async () => { + // Create a dummy 'node_modules' file to test to externalJS.ts recreates the symlink. + fs.mkdirSync(mockBasePath); + fs.writeFileSync(path.join(mockBasePath, "node_modules"), ""); + const converterName = "foo.js"; const converterCode = getFileCode("cjs", "mock-external-converter.js"); @@ -400,6 +404,8 @@ describe("Extension: ExternalConverters", () => { retain: true, }, ); + // Ensure that the .tmp import file is deleted. + expect(fs.readdirSync(mockBasePath)).toStrictEqual(["foo.js", "node_modules"]); //-- REMOVE await (controller.getExtension("ExternalConverters")! as ExternalConverters).onMQTTMessage({ @@ -501,7 +507,6 @@ describe("Extension: ExternalConverters", () => { {}, ); expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), converterCode, "utf8"); - expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); }); it("returns error on invalid removal", async () => { @@ -543,7 +548,6 @@ describe("Extension: ExternalConverters", () => { expect(mockMQTTPublishAsync).toHaveBeenCalledWith("zigbee2mqtt/bridge/response/converter/save", expect.stringContaining(errorMsg), {}); expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), converterCode, "utf8"); - expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(converterName), {force: true}); }); it("returns error on failed removal", async () => { diff --git a/test/extensions/externalExtensions.test.ts b/test/extensions/externalExtensions.test.ts index b70bd95c5..35b243462 100644 --- a/test/extensions/externalExtensions.test.ts +++ b/test/extensions/externalExtensions.test.ts @@ -195,7 +195,7 @@ describe("Extension: ExternalExtensions", () => { "zigbee2mqtt/bridge/extensions", stringify([ {name: "example2Extension.js", code: getFileCode("cjs", "example2Extension.js")}, - {name: "exampleExtension.1.js", code: extensionCode}, + {name: "exampleExtension.js", code: extensionCode}, ]), {retain: true}, ); @@ -207,16 +207,17 @@ describe("Extension: ExternalExtensions", () => { mockMQTTPublishAsync.mockClear(); await (controller.getExtension("ExternalExtensions")! as ExternalExtensions).onMQTTMessage({ topic: "zigbee2mqtt/bridge/request/extension/save", - message: {name: "exampleExtension.1.js", code: extensionCode}, + message: {name: "exampleExtension.js", code: extensionCode}, }); expect(mockMQTTPublishAsync).toHaveBeenCalledWith("zigbee2mqtt/example/extension", "call from stop - edited", {}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith("zigbee2mqtt/example/extension", "call from start", {}); + expect(mockMQTTPublishAsync).not.toHaveBeenCalledWith("zigbee2mqtt/example/extension", "call from start - edited", {}); expect(mockMQTTPublishAsync).toHaveBeenCalledWith( "zigbee2mqtt/bridge/extensions", stringify([ {name: "example2Extension.js", code: getFileCode("cjs", "example2Extension.js")}, - {name: "exampleExtension.2.js", code: extensionCode}, + {name: "exampleExtension.js", code: extensionCode}, ]), {retain: true}, ); @@ -247,6 +248,8 @@ describe("Extension: ExternalExtensions", () => { retain: true, }, ); + // Ensure that the .tmp import file is deleted. + expect(fs.readdirSync(mockBasePath)).toStrictEqual(["foo.js", "node_modules"]); //-- REMOVE await (controller.getExtension("ExternalExtensions")! as ExternalExtensions).onMQTTMessage({ @@ -312,7 +315,6 @@ describe("Extension: ExternalExtensions", () => { {}, ); expect(writeFileSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), extensionCode, "utf8"); - expect(rmSyncSpy).toHaveBeenCalledWith(expect.stringContaining(extensionName), {force: true}); }); it("returns error on invalid removal", async () => {