Skip to content

Commit 72d9474

Browse files
authored
fix: stop object reference leak in AppFabricationFulfillment (RocketChat#36943)
1 parent 5a503b4 commit 72d9474

3 files changed

Lines changed: 93 additions & 2 deletions

File tree

.changeset/tasty-ravens-grow.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
---
2+
'@rocket.chat/apps-engine': patch
3+
'@rocket.chat/meteor': patch
4+
---
5+
6+
Fixes an issue where an object reference leak would cause invalid data to be stored in the database during app installation

packages/apps-engine/src/server/compiler/AppFabricationFulfillment.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,8 @@ export class AppFabricationFulfillment {
2020
}
2121

2222
public setAppInfo(information: IAppInfo): void {
23-
this.info = information;
23+
this.info = structuredClone(information);
24+
2425
this.licenseValidationResult.setAppId(information.id);
2526
}
2627

@@ -37,7 +38,7 @@ export class AppFabricationFulfillment {
3738
}
3839

3940
public setImplementedInterfaces(interfaces: { [int: string]: boolean }): void {
40-
this.implemented = interfaces;
41+
this.implemented = structuredClone(interfaces);
4142
}
4243

4344
public getImplementedInferfaces(): { [int: string]: boolean } {

packages/apps-engine/tests/server/compiler/AppFabricationFulfillment.spec.ts

Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { AppInterface } from '../../../src/definition/metadata';
66
import type { AppManager } from '../../../src/server/AppManager';
77
import { ProxiedApp } from '../../../src/server/ProxiedApp';
88
import { AppFabricationFulfillment } from '../../../src/server/compiler';
9+
import { AppPermissions } from '../../../src/server/permissions/AppPermissions';
910
import type { IAppStorageItem } from '../../../src/server/storage';
1011
import { TestData } from '../../test-data/utilities';
1112

@@ -50,4 +51,87 @@ export class AppFabricationFulfillmentTestFixture {
5051
Expect(() => aff.setApp(fakeApp)).not.toThrow();
5152
Expect(aff.getApp()).toEqual(fakeApp);
5253
}
54+
55+
@Test()
56+
public setAppInfoCreatesDeepClone() {
57+
const originalInfo: IAppInfo = {
58+
id: 'test-app-id',
59+
name: 'Test App',
60+
nameSlug: 'test-app',
61+
description: 'A test application',
62+
version: '1.0.0',
63+
requiredApiVersion: '>=1.0.0',
64+
author: {
65+
name: 'Test Author',
66+
homepage: 'https://example.com',
67+
support: 'https://example.com/support',
68+
},
69+
classFile: 'TestApp.ts',
70+
iconFile: 'test.jpg',
71+
implements: [AppInterface.IPreMessageSentPrevent],
72+
permissions: [AppPermissions.user.read, AppPermissions.user.write],
73+
};
74+
75+
const aff = new AppFabricationFulfillment();
76+
aff.setAppInfo(originalInfo);
77+
78+
// Verify the stored info is equal to the original
79+
Expect(aff.getAppInfo()).toEqual(originalInfo);
80+
81+
// Verify that modifying the original object doesn't affect the stored copy
82+
originalInfo.name = 'Modified Name';
83+
originalInfo.author.name = 'Modified Author';
84+
originalInfo.implements.push(AppInterface.IPostMessageSent);
85+
originalInfo.permissions.push(AppPermissions.message.write);
86+
87+
Expect(aff.getAppInfo().name).not.toEqual('Modified Name');
88+
Expect(aff.getAppInfo().author.name).not.toEqual('Modified Author');
89+
Expect(aff.getAppInfo().implements).not.toContain(AppInterface.IPostMessageSent);
90+
Expect(aff.getAppInfo().permissions).not.toContain(AppPermissions.message.write);
91+
92+
// Verify the stored copy still has original values
93+
Expect(aff.getAppInfo().name).toEqual('Test App');
94+
Expect(aff.getAppInfo().author.name).toEqual('Test Author');
95+
Expect(aff.getAppInfo().implements).toEqual([AppInterface.IPreMessageSentPrevent]);
96+
Expect(aff.getAppInfo().permissions).toEqual([AppPermissions.user.read, AppPermissions.user.write]);
97+
}
98+
99+
@Test()
100+
public setImplementedInterfacesCreatesDeepClone() {
101+
const originalInterfaces: { [int: string]: boolean } = {
102+
[AppInterface.IPreMessageSentPrevent]: true,
103+
[AppInterface.IPostMessageSent]: false,
104+
[AppInterface.IPreMessageSentExtend]: true,
105+
};
106+
107+
const aff = new AppFabricationFulfillment();
108+
aff.setImplementedInterfaces(originalInterfaces);
109+
110+
// Verify the stored interfaces are equal to the original
111+
Expect(aff.getImplementedInferfaces()).toEqual(originalInterfaces);
112+
113+
// Verify that modifying the original object doesn't affect the stored copy
114+
originalInterfaces[AppInterface.IPreMessageSentPrevent] = false;
115+
originalInterfaces[AppInterface.IPostMessageSent] = true;
116+
originalInterfaces[AppInterface.IPreMessageSentModify] = true;
117+
118+
Expect(aff.getImplementedInferfaces()[AppInterface.IPreMessageSentPrevent]).not.toEqual(false);
119+
Expect(aff.getImplementedInferfaces()[AppInterface.IPostMessageSent]).not.toEqual(true);
120+
Expect(aff.getImplementedInferfaces()[AppInterface.IPreMessageSentModify]).not.toBeDefined();
121+
122+
// Verify the stored copy still has original values
123+
Expect(aff.getImplementedInferfaces()[AppInterface.IPreMessageSentPrevent]).toEqual(true);
124+
Expect(aff.getImplementedInferfaces()[AppInterface.IPostMessageSent]).toEqual(false);
125+
Expect(aff.getImplementedInferfaces()[AppInterface.IPreMessageSentExtend]).toEqual(true);
126+
}
127+
128+
@Test()
129+
public setImplementedInterfacesHandlesEmptyObject() {
130+
const emptyInterfaces: { [int: string]: boolean } = {};
131+
132+
const aff = new AppFabricationFulfillment();
133+
Expect(() => aff.setImplementedInterfaces(emptyInterfaces)).not.toThrow();
134+
Expect(Object.keys(aff.getImplementedInferfaces()).length).toEqual(0);
135+
Expect(aff.getImplementedInferfaces()).not.toBe(emptyInterfaces);
136+
}
53137
}

0 commit comments

Comments
 (0)