From 066f7164c76f71239bdad64ede911d9d5ec7514f Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Mon, 25 May 2026 23:58:53 -0300 Subject: [PATCH 1/3] feat: adapt permission validation to accept new apps-engine version --- src/compiler/AppsEngineValidator.ts | 52 +++++++++++++++++++---------- 1 file changed, 35 insertions(+), 17 deletions(-) diff --git a/src/compiler/AppsEngineValidator.ts b/src/compiler/AppsEngineValidator.ts index 6405264..d7068e5 100644 --- a/src/compiler/AppsEngineValidator.ts +++ b/src/compiler/AppsEngineValidator.ts @@ -1,13 +1,25 @@ import * as vm from "vm"; import path from "path"; +import type { AppInterface } from "@rocket.chat/apps-engine/definition/metadata"; import type { ICompilerResult } from "../definition"; import type { IPermission } from "../definition/IPermission"; +import type { IAppPermissions } from "../misc/getAvailablePermissions"; import { getAvailablePermissions } from "../misc/getAvailablePermissions"; import { Utilities } from "../misc/Utilities"; export class AppsEngineValidator { - constructor(private readonly appSourceRequire: NodeRequire) {} + private readonly safeAppSourceRequire: (id: string) => any; + + constructor(private readonly appSourceRequire: NodeJS.Require) { + this.safeAppSourceRequire = (id: string) => { + try { + return appSourceRequire(id); + } catch { + // It's ok not to find it + } + }; + } public validateAppPermissionsSchema(permissions: Array): void { if (!permissions) { @@ -20,17 +32,25 @@ export class AppsEngineValidator { ); } - const permissionsRequire = this.appSourceRequire( - "@rocket.chat/apps-engine/server/permissions/AppPermissions", - ); - - if (!permissionsRequire?.AppPermissions) { + const { + AppPermissions, + }: { AppPermissions: IAppPermissions | undefined } = + this.safeAppSourceRequire( + "@rocket.chat/apps-engine/server/permissions/AppPermissions", + ) || + this.safeAppSourceRequire( + "@rocket.chat/apps-engine/definition/metadata/AppPermissions", + ) || + {}; + + if (!AppPermissions) { + console.warn( + "Failed to read available permissions from the apps-engine. Permission definition will not be validated", + ); return; } - const availablePermissions = getAvailablePermissions( - permissionsRequire.AppPermissions, - ); + const availablePermissions = getAvailablePermissions(AppPermissions); permissions.forEach((permission) => { if (permission && !availablePermissions.includes(permission.name)) { @@ -42,17 +62,15 @@ export class AppsEngineValidator { } public isValidAppInterface(interfaceName: string): boolean { - let { AppInterface } = this.appSourceRequire( - "@rocket.chat/apps-engine/definition/metadata", - ); - - if (!AppInterface) { - AppInterface = this.appSourceRequire( + const { AppInterface }: { AppInterface: AppInterface | undefined } = + this.safeAppSourceRequire( + "@rocket.chat/apps-engine/definition/metadata", + ) || + this.safeAppSourceRequire( "@rocket.chat/apps-engine/server/compiler/AppImplements", ); - } - return interfaceName in AppInterface; + return !!AppInterface[interfaceName as keyof AppInterface]; } public resolveAppDependencyPath(module: string): string | undefined { From b29655ae6f93b31c6cd2e59dc005d172d232ea32 Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Tue, 26 May 2026 00:06:50 -0300 Subject: [PATCH 2/3] tests: add sinon and tests for validator --- package-lock.json | 87 +++++++++++++++ package.json | 2 + tests/AppsEngineValidator.spec.ts | 170 ++++++++++++++++++++++++++++++ 3 files changed, 259 insertions(+) create mode 100644 tests/AppsEngineValidator.spec.ts diff --git a/package-lock.json b/package-lock.json index 986fe82..c412576 100644 --- a/package-lock.json +++ b/package-lock.json @@ -30,6 +30,7 @@ "@types/lodash.clonedeep": "^4.5.3", "@types/mocha": "^8.0.0", "@types/node": "~22.14.1", + "@types/sinon": "^21.0.1", "@types/tv4": "^1.2.29", "@types/yauzl": "^2.10.3", "@types/yazl": "^2.4.1", @@ -41,6 +42,7 @@ "eslint-plugin-prettier": "^5.5.3", "husky": "^4.2.5", "mocha": "^8.0.1", + "sinon": "^22.0.0", "ts-node": "^9.1.1" }, "engines": { @@ -1321,6 +1323,47 @@ "node": ">=8" } }, + "node_modules/@sinonjs/commons": { + "version": "3.0.1", + "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-3.0.1.tgz", + "integrity": "sha512-K3mCHKQ9sVh8o1C9cxkwxaOmXoAMlDxC1mYyHrjqOWEcBjYr76t96zL2zlj5dUGZ3HSw240X1qgH3Mjf1yJWpQ==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "type-detect": "4.0.8" + } + }, + "node_modules/@sinonjs/fake-timers": { + "version": "15.4.0", + "resolved": "https://registry.npmjs.org/@sinonjs/fake-timers/-/fake-timers-15.4.0.tgz", + "integrity": "sha512-DsG+8/LscQIQg68J6Ef3dv10u6nVyetYn923s3/sus5eaGfTo1of5WMZSLf0UJc9KDuKPilPH0UDJCjvNbDNCA==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1" + } + }, + "node_modules/@sinonjs/samsam": { + "version": "10.0.2", + "resolved": "https://registry.npmjs.org/@sinonjs/samsam/-/samsam-10.0.2.tgz", + "integrity": "sha512-8lVwD1Df1BmzoaOLhMcGGcz/Jyr5QY2KSB75/YK1QgKzoabTeLdIVyhXNZK9ojfSKSdirbXqdbsXXqP9/Ve8+A==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1", + "type-detect": "^4.1.0" + } + }, + "node_modules/@sinonjs/samsam/node_modules/type-detect": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/type-detect/-/type-detect-4.1.0.tgz", + "integrity": "sha512-Acylog8/luQ8L7il+geoSxhEkazvkslg7PSNKOX59mbB9cOveP5aq9h74Y7YU8yDpJwetzQQrfIwtf4Wp4LKcw==", + "dev": true, + "license": "MIT", + "engines": { + "node": ">=4" + } + }, "node_modules/@types/chai": { "version": "4.2.12", "resolved": "https://registry.npmjs.org/@types/chai/-/chai-4.2.12.tgz", @@ -1455,6 +1498,23 @@ "dev": true, "license": "MIT" }, + "node_modules/@types/sinon": { + "version": "21.0.1", + "resolved": "https://registry.npmjs.org/@types/sinon/-/sinon-21.0.1.tgz", + "integrity": "sha512-5yoJSqLbjH8T9V2bksgRayuhpZy+723/z6wBOR+Soe4ZlXC0eW8Na71TeaZPUWDQvM7LYKa9UGFc6LRqxiR5fQ==", + "dev": true, + "license": "MIT", + "dependencies": { + "@types/sinonjs__fake-timers": "*" + } + }, + "node_modules/@types/sinonjs__fake-timers": { + "version": "15.0.1", + "resolved": "https://registry.npmjs.org/@types/sinonjs__fake-timers/-/sinonjs__fake-timers-15.0.1.tgz", + "integrity": "sha512-Ko2tjWJq8oozHzHV+reuvS5KYIRAokHnGbDwGh/J64LntgpbuylF74ipEL24HCyRjf9FOlBiBHWBR1RlVKsI1w==", + "dev": true, + "license": "MIT" + }, "node_modules/@types/tv4": { "version": "1.2.29", "resolved": "https://registry.npmjs.org/@types/tv4/-/tv4-1.2.29.tgz", @@ -7070,6 +7130,33 @@ "url": "https://github.com/sponsors/ljharb" } }, + "node_modules/sinon": { + "version": "22.0.0", + "resolved": "https://registry.npmjs.org/sinon/-/sinon-22.0.0.tgz", + "integrity": "sha512-sq/6DpdXOrLyfbKlXLg/Usc7xu8YXPeLkOFZRvA3bNUSA2lhbrZ06yuXbH1fkzBPCbz9O10+7hznzUsjaYNm0Q==", + "dev": true, + "license": "BSD-3-Clause", + "dependencies": { + "@sinonjs/commons": "^3.0.1", + "@sinonjs/fake-timers": "^15.4.0", + "@sinonjs/samsam": "^10.0.2", + "diff": "^9.0.0" + }, + "funding": { + "type": "opencollective", + "url": "https://opencollective.com/sinon" + } + }, + "node_modules/sinon/node_modules/diff": { + "version": "9.0.0", + "resolved": "https://registry.npmjs.org/diff/-/diff-9.0.0.tgz", + "integrity": "sha512-svtcdpS8CgJyqAjEQIXdb3OjhFVVYjzGAPO8WGCmRbrml64SPw/jJD4GoE98aR7r25A0XcgrK3F02yw9R/vhQw==", + "dev": true, + "license": "BSD-3-Clause", + "engines": { + "node": ">=0.3.1" + } + }, "node_modules/slash": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/slash/-/slash-3.0.0.tgz", diff --git a/package.json b/package.json index 1bc6dda..e24977c 100644 --- a/package.json +++ b/package.json @@ -37,6 +37,7 @@ "@types/lodash.clonedeep": "^4.5.3", "@types/mocha": "^8.0.0", "@types/node": "~22.14.1", + "@types/sinon": "^21.0.1", "@types/tv4": "^1.2.29", "@types/yauzl": "^2.10.3", "@types/yazl": "^2.4.1", @@ -48,6 +49,7 @@ "eslint-plugin-prettier": "^5.5.3", "husky": "^4.2.5", "mocha": "^8.0.1", + "sinon": "^22.0.0", "ts-node": "^9.1.1" }, "dependencies": { diff --git a/tests/AppsEngineValidator.spec.ts b/tests/AppsEngineValidator.spec.ts new file mode 100644 index 0000000..b8e6c84 --- /dev/null +++ b/tests/AppsEngineValidator.spec.ts @@ -0,0 +1,170 @@ +import { expect } from "chai"; +import { describe, it, beforeEach, afterEach } from "mocha"; +import sinon from "sinon"; + +import { AppsEngineValidator } from "../src/compiler/AppsEngineValidator"; + +const OLD_PERMISSIONS_PATH = + "@rocket.chat/apps-engine/server/permissions/AppPermissions"; +const NEW_PERMISSIONS_PATH = + "@rocket.chat/apps-engine/definition/metadata/AppPermissions"; +const OLD_INTERFACE_PATH = "@rocket.chat/apps-engine/definition/metadata"; +const NEW_INTERFACE_PATH = + "@rocket.chat/apps-engine/server/compiler/AppImplements"; + +const mockAppPermissions = { + network: { + write: { name: "networking.write" }, + read: { name: "networking.read" }, + }, + env: { + read: { name: "env.read" }, + }, +}; + +function makeRequire(moduleMap: Record): NodeJS.Require { + const fn = (id: string) => { + if (id in moduleMap) { + return moduleMap[id]; + } + throw new Error(`Cannot find module '${id}'`); + }; + fn.resolve = () => { + throw new Error("not implemented"); + }; + fn.cache = {}; + fn.extensions = {}; + fn.main = undefined; + return fn as unknown as NodeJS.Require; +} + +describe("AppsEngineValidator", () => { + let warnStub: sinon.SinonStub; + + beforeEach(() => { + warnStub = sinon.stub(console, "warn"); + }); + + afterEach(() => { + sinon.restore(); + }); + + describe("validateAppPermissionsSchema", () => { + it("returns early when permissions is falsy", () => { + const validator = new AppsEngineValidator(makeRequire({})); + expect(() => + validator.validateAppPermissionsSchema(null as any), + ).not.to.throw(); + }); + + it("throws when permissions is not an array", () => { + const validator = new AppsEngineValidator(makeRequire({})); + expect(() => + validator.validateAppPermissionsSchema({} as any), + ).to.throw("Invalid permission definition"); + }); + + it("logs a warning and skips validation when neither permissions module path resolves", () => { + const validator = new AppsEngineValidator(makeRequire({})); + validator.validateAppPermissionsSchema([ + { name: "networking.write" }, + ]); + expect(warnStub.calledOnce).to.be.true; + expect(warnStub.firstCall.args[0]).to.include( + "Failed to read available permissions", + ); + }); + + it("validates permissions using the old module path", () => { + const require = makeRequire({ + [OLD_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions }, + }); + const validator = new AppsEngineValidator(require); + expect(() => + validator.validateAppPermissionsSchema([ + { name: "networking.write" }, + ]), + ).not.to.throw(); + }); + + it("falls back to new module path when old path is not found", () => { + const require = makeRequire({ + [NEW_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions }, + }); + const validator = new AppsEngineValidator(require); + expect(() => + validator.validateAppPermissionsSchema([{ name: "env.read" }]), + ).not.to.throw(); + }); + + it("throws for an invalid permission name", () => { + const require = makeRequire({ + [OLD_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions }, + }); + const validator = new AppsEngineValidator(require); + expect(() => + validator.validateAppPermissionsSchema([ + { name: "not.a.real.permission" }, + ]), + ).to.throw('Invalid permission "not.a.real.permission"'); + }); + + it("skips null/undefined entries in the permissions array", () => { + const require = makeRequire({ + [OLD_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions }, + }); + const validator = new AppsEngineValidator(require); + expect(() => + validator.validateAppPermissionsSchema([ + null as any, + undefined as any, + { name: "networking.write" }, + ]), + ).not.to.throw(); + }); + }); + + describe("isValidAppInterface", () => { + const mockAppInterface = { + IPreMessageSentPrevent: "IPreMessageSentPrevent", + IPostMessageSent: "IPostMessageSent", + }; + + it("returns true for a known interface using the primary module path", () => { + const require = makeRequire({ + [OLD_INTERFACE_PATH]: { AppInterface: mockAppInterface }, + }); + const validator = new AppsEngineValidator(require); + expect(validator.isValidAppInterface("IPreMessageSentPrevent")).to + .be.true; + }); + + it("returns false for an unknown interface", () => { + const require = makeRequire({ + [OLD_INTERFACE_PATH]: { AppInterface: mockAppInterface }, + }); + const validator = new AppsEngineValidator(require); + expect(validator.isValidAppInterface("IDoesNotExist")).to.be.false; + }); + + it("falls back to the legacy module path when primary path is not found", () => { + const require = makeRequire({ + [NEW_INTERFACE_PATH]: { AppInterface: mockAppInterface }, + }); + const validator = new AppsEngineValidator(require); + expect(validator.isValidAppInterface("IPostMessageSent")).to.be + .true; + }); + + it("returns false for falsy interface value (not just key presence)", () => { + const require = makeRequire({ + [OLD_INTERFACE_PATH]: { + AppInterface: { IFalsyInterface: "" }, + }, + }); + const validator = new AppsEngineValidator(require); + expect(validator.isValidAppInterface("IFalsyInterface")).to.be + .false; + }); + }); +}); From 49f5189f0cb1e46ed0b8552ccd4f3d0e4d7dc59a Mon Sep 17 00:00:00 2001 From: Douglas Gubert Date: Tue, 26 May 2026 13:18:04 -0300 Subject: [PATCH 3/3] refactor: use standard logger for permission message --- src/compiler/AppsEngineValidator.ts | 5 +++-- tests/AppsEngineValidator.spec.ts | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/compiler/AppsEngineValidator.ts b/src/compiler/AppsEngineValidator.ts index d7068e5..4bd80b5 100644 --- a/src/compiler/AppsEngineValidator.ts +++ b/src/compiler/AppsEngineValidator.ts @@ -7,6 +7,7 @@ import type { IPermission } from "../definition/IPermission"; import type { IAppPermissions } from "../misc/getAvailablePermissions"; import { getAvailablePermissions } from "../misc/getAvailablePermissions"; import { Utilities } from "../misc/Utilities"; +import logger from "../misc/logger"; export class AppsEngineValidator { private readonly safeAppSourceRequire: (id: string) => any; @@ -44,8 +45,8 @@ export class AppsEngineValidator { {}; if (!AppPermissions) { - console.warn( - "Failed to read available permissions from the apps-engine. Permission definition will not be validated", + logger.warn( + "Couldn't find permissions in @rocket.chat/apps-engine version. App's permissions won't be validated", ); return; } diff --git a/tests/AppsEngineValidator.spec.ts b/tests/AppsEngineValidator.spec.ts index b8e6c84..35c03e8 100644 --- a/tests/AppsEngineValidator.spec.ts +++ b/tests/AppsEngineValidator.spec.ts @@ -3,6 +3,7 @@ import { describe, it, beforeEach, afterEach } from "mocha"; import sinon from "sinon"; import { AppsEngineValidator } from "../src/compiler/AppsEngineValidator"; +import logger from "../src/misc/logger"; const OLD_PERMISSIONS_PATH = "@rocket.chat/apps-engine/server/permissions/AppPermissions"; @@ -42,7 +43,7 @@ describe("AppsEngineValidator", () => { let warnStub: sinon.SinonStub; beforeEach(() => { - warnStub = sinon.stub(console, "warn"); + warnStub = sinon.stub(logger, "warn"); }); afterEach(() => { @@ -71,7 +72,7 @@ describe("AppsEngineValidator", () => { ]); expect(warnStub.calledOnce).to.be.true; expect(warnStub.firstCall.args[0]).to.include( - "Failed to read available permissions", + "Couldn't find permissions in @rocket.chat/apps-engine version", ); });