lee/puter
1
0
Fork 0
mirror of https://github.com/HeyPuter/puter.git synced 2026-05-04 08:30:39 +00:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

fix: cleanup some app es and app service validation (#2523)

Browse Source 
we saw some errors with icons not allowed when being objects, this likely user error, but this adds some validation to at least avoid nulls being flagged wrong
This commit is contained in:
Daniel Salazar
2026-02-21 00:10:55 -08:00
committed by GitHub
parent 3a89deaf02
commit 9f87ccb460
6 changed files with 133 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/backend/src/modules/data-access/AppService.js
+18 -10
View File
@@ -659,11 +659,15 @@ export default class AppService extends BaseService {
object.icon = normalizeRawBase64ImageString(object.icon);
object.icon = migrateRelativeAppIconEndpointUrl(object.icon);
}
if ( typeof object.icon === 'string' && object.icon.startsWith('data:') ) {
if ( typeof object.icon !== 'string' ) {
throw APIError.create('field_invalid', null, { key: 'icon' });
}
object.icon = object.icon.trim();
if ( ! object.icon ) {
// Empty icon is allowed to clear current icon.
} else if ( object.icon.startsWith('data:') ) {
validate_image_base64(object.icon, { key: 'icon' });
} else if ( isAllowedAppIconEndpointUrl(object.icon) ) {
// Allow existing relative app icon endpoint references.
} else {
} else if ( ! isAllowedAppIconEndpointUrl(object.icon) ) {
throw APIError.create('field_invalid', null, { key: 'icon' });
}
}
@@ -736,7 +740,7 @@ export default class AppService extends BaseService {
url: '',
};
await svc_event.emit('app.new-icon', event);
if ( event.url ) {
if ( typeof event.url === 'string' && event.url ) {
this.db_write.write('UPDATE apps SET icon = ? WHERE uid = ? LIMIT 1',
[event.url, uid]);
}
@@ -912,11 +916,15 @@ export default class AppService extends BaseService {
object.icon = normalizeRawBase64ImageString(object.icon);
object.icon = migrateRelativeAppIconEndpointUrl(object.icon);
}
if ( typeof object.icon === 'string' && object.icon.startsWith('data:') ) {
if ( typeof object.icon !== 'string' ) {
throw APIError.create('field_invalid', null, { key: 'icon' });
}
object.icon = object.icon.trim();
if ( ! object.icon ) {
// Empty icon is allowed to clear current icon.
} else if ( object.icon.startsWith('data:') ) {
validate_image_base64(object.icon, { key: 'icon' });
} else if ( isAllowedAppIconEndpointUrl(object.icon) ) {
// Allow existing relative app icon endpoint references.
} else {
} else if ( ! isAllowedAppIconEndpointUrl(object.icon) ) {
throw APIError.create('field_invalid', null, { key: 'icon' });
}
}
@@ -1200,7 +1208,7 @@ export default class AppService extends BaseService {
data_url: object.icon,
};
await svc_event.emit('app.new-icon', event);
if ( event.url ) {
if ( typeof event.url === 'string' && event.url ) {
await this.db_write.write('UPDATE apps SET icon = ? WHERE uid = ? LIMIT 1',
[event.url, old_app.uid]);
}
src/backend/src/modules/data-access/AppService.test.js
+67
View File
@@ -875,6 +875,40 @@ describe('AppService', () => {
expect(validate_url).toHaveBeenCalledWith('https://example.com', expect.objectContaining({ key: 'index_url' }));
});
it('should reject object icon payloads on create', async () => {
setupContextForWrite(createMockUserActor(1));
mockDb.read.mockResolvedValue([createMockAppRow()]);
const crudQ = AppService.IMPLEMENTS['crud-q'];
await expect(crudQ.create.call(appService, {
object: {
name: 'test-app',
title: 'Test',
index_url: 'https://example.com',
icon: { url: '/app-icon/app-uid-123/64' },
},
})).rejects.toMatchObject({
fields: { code: 'field_invalid', key: 'icon' },
});
});
it('should allow empty icon string on create', async () => {
setupContextForWrite(createMockUserActor(1));
mockDb.read.mockResolvedValue([createMockAppRow()]);
const crudQ = AppService.IMPLEMENTS['crud-q'];
await crudQ.create.call(appService, {
object: {
name: 'test-app',
title: 'Test',
index_url: 'https://example.com',
icon: '',
},
});
expect(mockEventService.emit).not.toHaveBeenCalledWith('app.new-icon', expect.anything());
});
it('should migrate legacy app-icons host URL to app-icon endpoint URL on create', async () => {
setupContextForWrite(createMockUserActor(1));
mockDb.read.mockResolvedValue([createMockAppRow()]);
@@ -1247,6 +1281,39 @@ describe('AppService', () => {
}));
});
it('should reject object icon payloads on update', async () => {
setupContextForWrite(createMockUserActor(1));
const crudQ = AppService.IMPLEMENTS['crud-q'];
await expect(crudQ.update.call(appService, {
object: {
uid: 'app-uid-123',
icon: { url: '/app-icon/app-uid-123/64' },
},
})).rejects.toMatchObject({
fields: { code: 'field_invalid', key: 'icon' },
});
});
it('should allow empty icon string on update', async () => {
setupContextForWrite(createMockUserActor(1));
const crudQ = AppService.IMPLEMENTS['crud-q'];
await crudQ.update.call(appService, {
object: {
uid: 'app-uid-123',
icon: '',
},
});
expect(mockEventService.emit).toHaveBeenCalledWith(
'app.new-icon',
expect.objectContaining({
app_uid: 'app-uid-123',
data_url: '',
}));
});
it('should migrate legacy app-icons host URL to app-icon endpoint URL on update', async () => {
setupContextForWrite(createMockUserActor(1));
src/backend/src/om/definitions/PropType.js
+5 -4
View File
@@ -44,7 +44,7 @@ class PropType extends AdvancedBase {
}
for ( const k in data ) {
if ( ! chains.hasOwnProperty(k) ) {
if ( ! Object.prototype.hasOwnProperty.call(chains, k) ) {
chains[k] = [];
}
chains[k].push(data[k]);
@@ -57,7 +57,7 @@ class PropType extends AdvancedBase {
populate_subtype_ (chains) {
for ( const k in this.chains ) {
if ( ! chains.hasOwnProperty(k) ) {
if ( ! Object.prototype.hasOwnProperty.call(chains, k) ) {
chains[k] = [];
}
chains[k].push(...this.chains[k]);
@@ -65,8 +65,9 @@ class PropType extends AdvancedBase {
}
async adapt (value, extra) {
const adapters = this.chains.adapt || [];
adapters.reverse();
const adapters = this.chains.adapt
? [...this.chains.adapt].reverse()
: [];
for ( const adapter of adapters ) {
value = await adapter(value, extra);
src/backend/src/om/definitions/PropType.test.js
+37
View File
@@ -0,0 +1,37 @@
import { describe, expect, it } from 'vitest';
const { PropType } = require('./PropType');
describe('PropType adapt chain ordering', () => {
it('runs subtype adapters before supertype adapters on every call', async () => {
const callOrder = [];
const typ = new PropType({
name: 'test',
chains: {
adapt: [
value => {
callOrder.push('super');
if ( typeof value !== 'string' ) {
throw new Error('expected string');
}
return value;
},
value => {
callOrder.push('sub');
if ( value && typeof value === 'object' && typeof value.url === 'string' ) {
return value.url;
}
return value;
},
],
},
});
await expect(typ.adapt({ url: 'https://example.com/icon-a.png' }))
.resolves.toBe('https://example.com/icon-a.png');
await expect(typ.adapt({ url: 'https://example.com/icon-b.png' }))
.resolves.toBe('https://example.com/icon-b.png');
expect(callOrder).toEqual(['sub', 'super', 'sub', 'super']);
});
});
src/backend/src/om/entitystorage/AppES.js
+2 -2
View File
@@ -198,7 +198,7 @@ class AppES extends BaseES {
url: '',
};
await svc_event.emit('app.new-icon', event);
if ( event.url ) {
if ( typeof event.url === 'string' && event.url ) {
this.db.write('UPDATE apps SET icon = ? WHERE id = ? LIMIT 1',
[event.url, insert_id]);
await entity.set('icon', event.url);
@@ -289,7 +289,7 @@ class AppES extends BaseES {
data_url: icon,
};
await svc_event.emit('app.new-icon', event);
if ( ! event.url ) return;
if ( typeof event.url !== 'string' || !event.url ) return;
await this.db.write('UPDATE apps SET icon = ? WHERE uid = ? LIMIT 1',
[event.url, app_uid]);
src/backend/src/om/proptypes/__all__.test.js
+4
View File
@@ -67,6 +67,10 @@ describe('OM image-base64 proptype', () => {
expect(validateIcon('not-an-icon')).toBeInstanceOf(Error);
});
it('rejects object icon values', () => {
expect(validateIcon({ url: '/app-icon/app-uid-123/64' })).toBeInstanceOf(Error);
});
it('rejects foreign absolute app-icon endpoint URLs', () => {
expect(validateIcon('https://evil.example/app-icon/app-uid-123/64')).toBeInstanceOf(Error);
});