Skip to content

Commit 61046e1

Browse files
authored
feat: adapt permission validation to accept new apps-engine version (#71)
1 parent 05cae0e commit 61046e1

4 files changed

Lines changed: 296 additions & 17 deletions

File tree

package-lock.json

Lines changed: 87 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"@types/lodash.clonedeep": "^4.5.3",
3838
"@types/mocha": "^8.0.0",
3939
"@types/node": "~22.14.1",
40+
"@types/sinon": "^21.0.1",
4041
"@types/tv4": "^1.2.29",
4142
"@types/yauzl": "^2.10.3",
4243
"@types/yazl": "^2.4.1",
@@ -48,6 +49,7 @@
4849
"eslint-plugin-prettier": "^5.5.3",
4950
"husky": "^4.2.5",
5051
"mocha": "^8.0.1",
52+
"sinon": "^22.0.0",
5153
"ts-node": "^9.1.1"
5254
},
5355
"dependencies": {

src/compiler/AppsEngineValidator.ts

Lines changed: 36 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,26 @@
11
import * as vm from "vm";
22
import path from "path";
33

4+
import type { AppInterface } from "@rocket.chat/apps-engine/definition/metadata";
45
import type { ICompilerResult } from "../definition";
56
import type { IPermission } from "../definition/IPermission";
7+
import type { IAppPermissions } from "../misc/getAvailablePermissions";
68
import { getAvailablePermissions } from "../misc/getAvailablePermissions";
79
import { Utilities } from "../misc/Utilities";
10+
import logger from "../misc/logger";
811

912
export class AppsEngineValidator {
10-
constructor(private readonly appSourceRequire: NodeRequire) {}
13+
private readonly safeAppSourceRequire: (id: string) => any;
14+
15+
constructor(private readonly appSourceRequire: NodeJS.Require) {
16+
this.safeAppSourceRequire = (id: string) => {
17+
try {
18+
return appSourceRequire(id);
19+
} catch {
20+
// It's ok not to find it
21+
}
22+
};
23+
}
1124

1225
public validateAppPermissionsSchema(permissions: Array<IPermission>): void {
1326
if (!permissions) {
@@ -20,17 +33,25 @@ export class AppsEngineValidator {
2033
);
2134
}
2235

23-
const permissionsRequire = this.appSourceRequire(
24-
"@rocket.chat/apps-engine/server/permissions/AppPermissions",
25-
);
26-
27-
if (!permissionsRequire?.AppPermissions) {
36+
const {
37+
AppPermissions,
38+
}: { AppPermissions: IAppPermissions | undefined } =
39+
this.safeAppSourceRequire(
40+
"@rocket.chat/apps-engine/server/permissions/AppPermissions",
41+
) ||
42+
this.safeAppSourceRequire(
43+
"@rocket.chat/apps-engine/definition/metadata/AppPermissions",
44+
) ||
45+
{};
46+
47+
if (!AppPermissions) {
48+
logger.warn(
49+
"Couldn't find permissions in @rocket.chat/apps-engine version. App's permissions won't be validated",
50+
);
2851
return;
2952
}
3053

31-
const availablePermissions = getAvailablePermissions(
32-
permissionsRequire.AppPermissions,
33-
);
54+
const availablePermissions = getAvailablePermissions(AppPermissions);
3455

3556
permissions.forEach((permission) => {
3657
if (permission && !availablePermissions.includes(permission.name)) {
@@ -42,17 +63,15 @@ export class AppsEngineValidator {
4263
}
4364

4465
public isValidAppInterface(interfaceName: string): boolean {
45-
let { AppInterface } = this.appSourceRequire(
46-
"@rocket.chat/apps-engine/definition/metadata",
47-
);
48-
49-
if (!AppInterface) {
50-
AppInterface = this.appSourceRequire(
66+
const { AppInterface }: { AppInterface: AppInterface | undefined } =
67+
this.safeAppSourceRequire(
68+
"@rocket.chat/apps-engine/definition/metadata",
69+
) ||
70+
this.safeAppSourceRequire(
5171
"@rocket.chat/apps-engine/server/compiler/AppImplements",
5272
);
53-
}
5473

55-
return interfaceName in AppInterface;
74+
return !!AppInterface[interfaceName as keyof AppInterface];
5675
}
5776

5877
public resolveAppDependencyPath(module: string): string | undefined {

tests/AppsEngineValidator.spec.ts

Lines changed: 171 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,171 @@
1+
import { expect } from "chai";
2+
import { describe, it, beforeEach, afterEach } from "mocha";
3+
import sinon from "sinon";
4+
5+
import { AppsEngineValidator } from "../src/compiler/AppsEngineValidator";
6+
import logger from "../src/misc/logger";
7+
8+
const OLD_PERMISSIONS_PATH =
9+
"@rocket.chat/apps-engine/server/permissions/AppPermissions";
10+
const NEW_PERMISSIONS_PATH =
11+
"@rocket.chat/apps-engine/definition/metadata/AppPermissions";
12+
const OLD_INTERFACE_PATH = "@rocket.chat/apps-engine/definition/metadata";
13+
const NEW_INTERFACE_PATH =
14+
"@rocket.chat/apps-engine/server/compiler/AppImplements";
15+
16+
const mockAppPermissions = {
17+
network: {
18+
write: { name: "networking.write" },
19+
read: { name: "networking.read" },
20+
},
21+
env: {
22+
read: { name: "env.read" },
23+
},
24+
};
25+
26+
function makeRequire(moduleMap: Record<string, any>): NodeJS.Require {
27+
const fn = (id: string) => {
28+
if (id in moduleMap) {
29+
return moduleMap[id];
30+
}
31+
throw new Error(`Cannot find module '${id}'`);
32+
};
33+
fn.resolve = () => {
34+
throw new Error("not implemented");
35+
};
36+
fn.cache = {};
37+
fn.extensions = {};
38+
fn.main = undefined;
39+
return fn as unknown as NodeJS.Require;
40+
}
41+
42+
describe("AppsEngineValidator", () => {
43+
let warnStub: sinon.SinonStub;
44+
45+
beforeEach(() => {
46+
warnStub = sinon.stub(logger, "warn");
47+
});
48+
49+
afterEach(() => {
50+
sinon.restore();
51+
});
52+
53+
describe("validateAppPermissionsSchema", () => {
54+
it("returns early when permissions is falsy", () => {
55+
const validator = new AppsEngineValidator(makeRequire({}));
56+
expect(() =>
57+
validator.validateAppPermissionsSchema(null as any),
58+
).not.to.throw();
59+
});
60+
61+
it("throws when permissions is not an array", () => {
62+
const validator = new AppsEngineValidator(makeRequire({}));
63+
expect(() =>
64+
validator.validateAppPermissionsSchema({} as any),
65+
).to.throw("Invalid permission definition");
66+
});
67+
68+
it("logs a warning and skips validation when neither permissions module path resolves", () => {
69+
const validator = new AppsEngineValidator(makeRequire({}));
70+
validator.validateAppPermissionsSchema([
71+
{ name: "networking.write" },
72+
]);
73+
expect(warnStub.calledOnce).to.be.true;
74+
expect(warnStub.firstCall.args[0]).to.include(
75+
"Couldn't find permissions in @rocket.chat/apps-engine version",
76+
);
77+
});
78+
79+
it("validates permissions using the old module path", () => {
80+
const require = makeRequire({
81+
[OLD_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions },
82+
});
83+
const validator = new AppsEngineValidator(require);
84+
expect(() =>
85+
validator.validateAppPermissionsSchema([
86+
{ name: "networking.write" },
87+
]),
88+
).not.to.throw();
89+
});
90+
91+
it("falls back to new module path when old path is not found", () => {
92+
const require = makeRequire({
93+
[NEW_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions },
94+
});
95+
const validator = new AppsEngineValidator(require);
96+
expect(() =>
97+
validator.validateAppPermissionsSchema([{ name: "env.read" }]),
98+
).not.to.throw();
99+
});
100+
101+
it("throws for an invalid permission name", () => {
102+
const require = makeRequire({
103+
[OLD_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions },
104+
});
105+
const validator = new AppsEngineValidator(require);
106+
expect(() =>
107+
validator.validateAppPermissionsSchema([
108+
{ name: "not.a.real.permission" },
109+
]),
110+
).to.throw('Invalid permission "not.a.real.permission"');
111+
});
112+
113+
it("skips null/undefined entries in the permissions array", () => {
114+
const require = makeRequire({
115+
[OLD_PERMISSIONS_PATH]: { AppPermissions: mockAppPermissions },
116+
});
117+
const validator = new AppsEngineValidator(require);
118+
expect(() =>
119+
validator.validateAppPermissionsSchema([
120+
null as any,
121+
undefined as any,
122+
{ name: "networking.write" },
123+
]),
124+
).not.to.throw();
125+
});
126+
});
127+
128+
describe("isValidAppInterface", () => {
129+
const mockAppInterface = {
130+
IPreMessageSentPrevent: "IPreMessageSentPrevent",
131+
IPostMessageSent: "IPostMessageSent",
132+
};
133+
134+
it("returns true for a known interface using the primary module path", () => {
135+
const require = makeRequire({
136+
[OLD_INTERFACE_PATH]: { AppInterface: mockAppInterface },
137+
});
138+
const validator = new AppsEngineValidator(require);
139+
expect(validator.isValidAppInterface("IPreMessageSentPrevent")).to
140+
.be.true;
141+
});
142+
143+
it("returns false for an unknown interface", () => {
144+
const require = makeRequire({
145+
[OLD_INTERFACE_PATH]: { AppInterface: mockAppInterface },
146+
});
147+
const validator = new AppsEngineValidator(require);
148+
expect(validator.isValidAppInterface("IDoesNotExist")).to.be.false;
149+
});
150+
151+
it("falls back to the legacy module path when primary path is not found", () => {
152+
const require = makeRequire({
153+
[NEW_INTERFACE_PATH]: { AppInterface: mockAppInterface },
154+
});
155+
const validator = new AppsEngineValidator(require);
156+
expect(validator.isValidAppInterface("IPostMessageSent")).to.be
157+
.true;
158+
});
159+
160+
it("returns false for falsy interface value (not just key presence)", () => {
161+
const require = makeRequire({
162+
[OLD_INTERFACE_PATH]: {
163+
AppInterface: { IFalsyInterface: "" },
164+
},
165+
});
166+
const validator = new AppsEngineValidator(require);
167+
expect(validator.isValidAppInterface("IFalsyInterface")).to.be
168+
.false;
169+
});
170+
});
171+
});

0 commit comments

Comments
 (0)