feat(server): import application logo into file storage at install (#22437)
Installed apps stored the logo as the manifest's relative path but never imported the file, so the public-assets URL 404'd and logos went missing in the UI for npm/tarball sources. Import the logo (best-effort — a declared but unshipped logo is skipped, not fatal) and record it as a first-class logoFileId on the application so it can be served reliably. <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22437?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. -->
This commit is contained in:
+83
-9
@@ -2,7 +2,7 @@ import { Injectable, Logger } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { promises as fs } from 'fs';
|
||||
import { resolve } from 'path';
|
||||
import { isAbsolute, relative, resolve } from 'path';
|
||||
|
||||
import semver from 'semver';
|
||||
import { Manifest } from 'twenty-shared/application';
|
||||
@@ -222,6 +222,20 @@ export class ApplicationInstallService {
|
||||
params.workspaceId,
|
||||
);
|
||||
|
||||
const logoFileId = await this.importLogoFile({
|
||||
extractedDir: resolvedPackage.extractedDir,
|
||||
manifest: resolvedPackage.manifest,
|
||||
applicationUniversalIdentifier: universalIdentifier,
|
||||
workspaceId: params.workspaceId,
|
||||
});
|
||||
|
||||
if (application.logoFileId !== logoFileId) {
|
||||
await this.applicationService.update(application.id, {
|
||||
logoFileId: logoFileId ?? null,
|
||||
workspaceId: params.workspaceId,
|
||||
});
|
||||
}
|
||||
|
||||
await this.runPreInstallHook({
|
||||
manifest: resolvedPackage.manifest,
|
||||
workspaceId: params.workspaceId,
|
||||
@@ -462,6 +476,23 @@ export class ApplicationInstallService {
|
||||
}
|
||||
}
|
||||
|
||||
private resolveWithinDirOrThrow(
|
||||
extractedDir: string,
|
||||
relativePath: string,
|
||||
): string {
|
||||
const absolutePath = resolve(extractedDir, relativePath);
|
||||
const relativeToDir = relative(extractedDir, absolutePath);
|
||||
|
||||
if (relativeToDir.startsWith('..') || isAbsolute(relativeToDir)) {
|
||||
throw new ApplicationException(
|
||||
`Path traversal detected for file: ${relativePath}`,
|
||||
ApplicationExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
}
|
||||
|
||||
return absolutePath;
|
||||
}
|
||||
|
||||
private async writeFilesToStorage(
|
||||
extractedDir: string,
|
||||
manifest: Manifest,
|
||||
@@ -471,14 +502,10 @@ export class ApplicationInstallService {
|
||||
const filesToWrite = this.buildFileList(manifest);
|
||||
|
||||
for (const { relativePath, fileFolder } of filesToWrite) {
|
||||
const absolutePath = resolve(extractedDir, relativePath);
|
||||
|
||||
if (!absolutePath.startsWith(extractedDir)) {
|
||||
throw new ApplicationException(
|
||||
`Path traversal detected for file: ${relativePath}`,
|
||||
ApplicationExceptionCode.INVALID_INPUT,
|
||||
);
|
||||
}
|
||||
const absolutePath = this.resolveWithinDirOrThrow(
|
||||
extractedDir,
|
||||
relativePath,
|
||||
);
|
||||
|
||||
let content: Buffer;
|
||||
|
||||
@@ -502,6 +529,53 @@ export class ApplicationInstallService {
|
||||
}
|
||||
}
|
||||
|
||||
private async importLogoFile({
|
||||
extractedDir,
|
||||
manifest,
|
||||
applicationUniversalIdentifier,
|
||||
workspaceId,
|
||||
}: {
|
||||
extractedDir: string;
|
||||
manifest: Manifest;
|
||||
applicationUniversalIdentifier: string;
|
||||
workspaceId: string;
|
||||
}): Promise<string | null> {
|
||||
const logoUrl = manifest.application.logoUrl;
|
||||
|
||||
if (
|
||||
!isDefined(logoUrl) ||
|
||||
logoUrl.startsWith('http://') ||
|
||||
logoUrl.startsWith('https://')
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const absolutePath = this.resolveWithinDirOrThrow(extractedDir, logoUrl);
|
||||
|
||||
let content: Buffer;
|
||||
|
||||
try {
|
||||
content = await fs.readFile(absolutePath);
|
||||
} catch {
|
||||
this.logger.warn(
|
||||
`Logo "${logoUrl}" declared in manifest but not found in package for ${applicationUniversalIdentifier}; skipping logo import`,
|
||||
);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
const file = await this.fileStorageService.writeFile({
|
||||
sourceFile: content,
|
||||
fileFolder: FileFolder.PublicAsset,
|
||||
applicationUniversalIdentifier,
|
||||
workspaceId,
|
||||
resourcePath: logoUrl,
|
||||
settings: { isTemporaryFile: false, toDelete: false },
|
||||
});
|
||||
|
||||
return file.id;
|
||||
}
|
||||
|
||||
private buildFileList(
|
||||
manifest: Manifest,
|
||||
): Array<{ relativePath: string; fileFolder: FileFolder }> {
|
||||
|
||||
@@ -60,6 +60,17 @@ export class ApplicationEntity extends WorkspaceRelatedEntity {
|
||||
})
|
||||
logo: string | null;
|
||||
|
||||
@Column({ nullable: true, type: 'uuid' })
|
||||
@WasIntroducedInUpgrade({
|
||||
upgradeCommandName:
|
||||
'2.19.0_AddLogoFileIdToApplicationFastInstanceCommand_1783062755137',
|
||||
})
|
||||
logoFileId: string | null;
|
||||
|
||||
@OneToOne(() => FileEntity, { onDelete: 'SET NULL', nullable: true })
|
||||
@JoinColumn({ name: 'logoFileId' })
|
||||
logoFile: Relation<FileEntity> | null;
|
||||
|
||||
// TODO should not be nullable
|
||||
@Column({ nullable: true, type: 'text' })
|
||||
version: string | null;
|
||||
|
||||
+1
@@ -10,6 +10,7 @@ export const APPLICATION_ENTITY_RELATION_PROPERTIES = [
|
||||
'applicationVariables',
|
||||
'packageJsonFile',
|
||||
'yarnLockFile',
|
||||
'logoFile',
|
||||
'applicationRegistration',
|
||||
'primaryPublicDomain',
|
||||
'publicDomains',
|
||||
|
||||
@@ -40,6 +40,11 @@ export class ApplicationDTO {
|
||||
@Field({ nullable: true })
|
||||
logo?: string;
|
||||
|
||||
@IsOptional()
|
||||
@IsUUID()
|
||||
@Field(() => UUIDScalarType, { nullable: true })
|
||||
logoFileId?: string;
|
||||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@Field({ nullable: true })
|
||||
|
||||
Reference in New Issue
Block a user