Upload application file resolver exception management and integration coverage (#20803)
# Introduction Earlier and better exception handling of the upload application file resolver + coverage
This commit is contained in:
+17
-3
@@ -22,9 +22,9 @@ import { UploadApplicationFileInput } from 'src/engine/core-modules/application/
|
||||
import { WorkspaceMigrationDTO } from 'src/engine/core-modules/application/application-development/dtos/workspace-migration.dto';
|
||||
import { ApplicationExceptionFilter } from 'src/engine/core-modules/application/application-exception-filter';
|
||||
import { ApplicationSyncService } from 'src/engine/core-modules/application/application-manifest/application-sync.service';
|
||||
import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util';
|
||||
import { ApplicationTokenPairDTO } from 'src/engine/core-modules/application/application-oauth/dtos/application-token-pair.dto';
|
||||
import { ApplicationRegistrationVariableService } from 'src/engine/core-modules/application/application-registration-variable/application-registration-variable.service';
|
||||
import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util';
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import {
|
||||
@@ -37,14 +37,14 @@ import { FileStorageService } from 'src/engine/core-modules/file-storage/file-st
|
||||
import { FileDTO } from 'src/engine/core-modules/file/dtos/file.dto';
|
||||
import { ResolverValidationPipe } from 'src/engine/core-modules/graphql/pipes/resolver-validation.pipe';
|
||||
import { SdkClientGenerationService } from 'src/engine/core-modules/sdk-client/sdk-client-generation.service';
|
||||
import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
import { type WorkspaceEntity } from 'src/engine/core-modules/workspace/workspace.entity';
|
||||
import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
|
||||
import { AuthUserWorkspaceId } from 'src/engine/decorators/auth/auth-user-workspace-id.decorator';
|
||||
import { AuthUser } from 'src/engine/decorators/auth/auth-user.decorator';
|
||||
import { AuthWorkspace } from 'src/engine/decorators/auth/auth-workspace.decorator';
|
||||
import { SettingsPermissionGuard } from 'src/engine/guards/settings-permission.guard';
|
||||
import { WorkspaceAuthGuard } from 'src/engine/guards/workspace-auth.guard';
|
||||
import { ThrottlerService } from 'src/engine/core-modules/throttler/throttler.service';
|
||||
import { WorkspaceMigrationGraphqlApiExceptionInterceptor } from 'src/engine/workspace-manager/workspace-migration/interceptors/workspace-migration-graphql-api-exception.interceptor';
|
||||
import { streamToBuffer } from 'src/utils/stream-to-buffer';
|
||||
|
||||
@@ -219,6 +219,20 @@ export class ApplicationDevelopmentResolver {
|
||||
);
|
||||
}
|
||||
|
||||
const application = await this.applicationService.findByUniversalIdentifier(
|
||||
{
|
||||
universalIdentifier: applicationUniversalIdentifier,
|
||||
workspaceId,
|
||||
},
|
||||
);
|
||||
|
||||
if (!isDefined(application)) {
|
||||
throw new ApplicationException(
|
||||
'Application not found in workspace.',
|
||||
ApplicationExceptionCode.APPLICATION_NOT_FOUND,
|
||||
);
|
||||
}
|
||||
|
||||
const buffer = await streamToBuffer(createReadStream());
|
||||
|
||||
return await this.fileStorageService.writeFile({
|
||||
|
||||
+6
@@ -1,15 +1,21 @@
|
||||
import { ArgsType, Field } from '@nestjs/graphql';
|
||||
|
||||
import { IsNotEmpty } from 'class-validator';
|
||||
import { FileFolder } from 'twenty-shared/types';
|
||||
|
||||
import { IsSafeRelativePath } from 'src/engine/core-modules/file-storage/validators/is-safe-relative-path.validator';
|
||||
|
||||
@ArgsType()
|
||||
export class UploadApplicationFileInput {
|
||||
@Field(() => String)
|
||||
@IsNotEmpty()
|
||||
applicationUniversalIdentifier: string;
|
||||
|
||||
@Field(() => FileFolder)
|
||||
fileFolder: FileFolder;
|
||||
|
||||
@Field(() => String)
|
||||
@IsNotEmpty()
|
||||
@IsSafeRelativePath()
|
||||
filePath: string;
|
||||
}
|
||||
|
||||
+30
@@ -0,0 +1,30 @@
|
||||
import {
|
||||
registerDecorator,
|
||||
type ValidationArguments,
|
||||
type ValidationOptions,
|
||||
} from 'class-validator';
|
||||
|
||||
import { isSafeRelativePath } from 'src/engine/core-modules/file-storage/utils/is-safe-relative-path.util';
|
||||
|
||||
export function IsSafeRelativePath(validationOptions?: ValidationOptions) {
|
||||
return function (object: object, propertyName: string) {
|
||||
registerDecorator({
|
||||
name: 'IsSafeRelativePath',
|
||||
target: object.constructor,
|
||||
propertyName,
|
||||
options: validationOptions,
|
||||
validator: {
|
||||
validate(value: unknown) {
|
||||
if (typeof value !== 'string') {
|
||||
return false;
|
||||
}
|
||||
|
||||
return isSafeRelativePath(value);
|
||||
},
|
||||
defaultMessage(args: ValidationArguments) {
|
||||
return `${args.property} contains unsafe characters or path traversal`;
|
||||
},
|
||||
},
|
||||
});
|
||||
};
|
||||
}
|
||||
+91
@@ -0,0 +1,91 @@
|
||||
// Jest Snapshot v1, https://goo.gl/fbAQLP
|
||||
|
||||
exports[`Upload application file should fail when applicationUniversalIdentifier does not match any installed application 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "NOT_FOUND",
|
||||
"subCode": "APPLICATION_NOT_FOUND",
|
||||
"userFriendlyMessage": "Application not found.",
|
||||
},
|
||||
"message": "Application not found in workspace.",
|
||||
"name": "NotFoundError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when applicationUniversalIdentifier is empty 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"userFriendlyMessage": "An error occurred.",
|
||||
},
|
||||
"message": "applicationUniversalIdentifier should not be empty",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when fileFolder is not an allowed application file folder 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"subCode": "INVALID_INPUT",
|
||||
"userFriendlyMessage": "Invalid input provided.",
|
||||
},
|
||||
"message": "Invalid fileFolder for application file upload. Allowed values: built-logic-function, built-front-component, public-asset, source, dependencies",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when filePath contains backslash path traversal 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"userFriendlyMessage": "An error occurred.",
|
||||
},
|
||||
"message": "filePath contains unsafe characters or path traversal",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when filePath contains relative path traversal (../) 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"userFriendlyMessage": "An error occurred.",
|
||||
},
|
||||
"message": "filePath contains unsafe characters or path traversal",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when filePath contains upward traversal (../../) 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"userFriendlyMessage": "An error occurred.",
|
||||
},
|
||||
"message": "filePath contains unsafe characters or path traversal",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when filePath is an absolute path 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"userFriendlyMessage": "An error occurred.",
|
||||
},
|
||||
"message": "filePath contains unsafe characters or path traversal",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
|
||||
exports[`Upload application file should fail when filePath is empty 1`] = `
|
||||
{
|
||||
"extensions": {
|
||||
"code": "BAD_USER_INPUT",
|
||||
"userFriendlyMessage": "An error occurred.",
|
||||
},
|
||||
"message": "filePath contains unsafe characters or path traversal, filePath should not be empty",
|
||||
"name": "UserInputError",
|
||||
}
|
||||
`;
|
||||
+126
@@ -0,0 +1,126 @@
|
||||
import { expectOneNotInternalServerErrorSnapshot } from 'test/integration/graphql/utils/expect-one-not-internal-server-error-snapshot.util';
|
||||
import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util';
|
||||
import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util';
|
||||
import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util';
|
||||
import {
|
||||
type EachTestingContext,
|
||||
eachTestingContextFilter,
|
||||
} from 'twenty-shared/testing';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
|
||||
const TEST_APP_ID = uuidv4();
|
||||
const UNKNOWN_APP_ID = uuidv4();
|
||||
|
||||
type TestContext = {
|
||||
applicationUniversalIdentifier: string;
|
||||
fileFolder: string;
|
||||
filePath: string;
|
||||
};
|
||||
|
||||
const FAILING_TEST_CASES: EachTestingContext<TestContext>[] = [
|
||||
{
|
||||
title: 'when filePath contains relative path traversal (../)',
|
||||
context: {
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath:
|
||||
'../../../other-workspace/other-app/BuiltFrontComponent/stolen.mjs',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when filePath contains upward traversal (../../)',
|
||||
context: {
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath: '../../etc/passwd',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when filePath is an absolute path',
|
||||
context: {
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath: '/etc/passwd',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when filePath contains backslash path traversal',
|
||||
context: {
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath: '..\\..\\..\\etc\\passwd',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when filePath is empty',
|
||||
context: {
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath: '',
|
||||
},
|
||||
},
|
||||
{
|
||||
title:
|
||||
'when applicationUniversalIdentifier does not match any installed application',
|
||||
context: {
|
||||
applicationUniversalIdentifier: UNKNOWN_APP_ID,
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath: 'src/components/legit.mjs',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when applicationUniversalIdentifier is empty',
|
||||
context: {
|
||||
applicationUniversalIdentifier: '',
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
filePath: 'src/components/legit.mjs',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when fileFolder is not an allowed application file folder',
|
||||
context: {
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: 'CorePicture',
|
||||
filePath: 'src/components/legit.mjs',
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
describe('Upload application file should fail', () => {
|
||||
beforeAll(async () => {
|
||||
await setupApplicationForSync({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
name: 'Test Upload Path Traversal App',
|
||||
description: 'App for testing path traversal on file upload',
|
||||
sourcePath: 'test-upload-path-traversal',
|
||||
});
|
||||
}, 60000);
|
||||
|
||||
afterAll(async () => {
|
||||
await cleanupApplicationAndAppRegistration({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
});
|
||||
});
|
||||
|
||||
it.each(eachTestingContextFilter(FAILING_TEST_CASES))(
|
||||
'$title',
|
||||
async ({ context }) => {
|
||||
jest.useRealTimers();
|
||||
|
||||
const { errors } = await uploadApplicationFile({
|
||||
applicationUniversalIdentifier: context.applicationUniversalIdentifier,
|
||||
fileFolder: context.fileFolder,
|
||||
filePath: context.filePath,
|
||||
fileBuffer: Buffer.from('content'),
|
||||
filename: 'test-file.mjs',
|
||||
contentType: 'application/javascript',
|
||||
expectToFail: true,
|
||||
});
|
||||
|
||||
jest.useFakeTimers();
|
||||
|
||||
expectOneNotInternalServerErrorSnapshot({ errors });
|
||||
},
|
||||
60000,
|
||||
);
|
||||
});
|
||||
+132
@@ -0,0 +1,132 @@
|
||||
import { cleanupApplicationAndAppRegistration } from 'test/integration/metadata/suites/application/utils/cleanup-application-and-app-registration.util';
|
||||
import { setupApplicationForSync } from 'test/integration/metadata/suites/application/utils/setup-application-for-sync.util';
|
||||
import { uploadApplicationFile } from 'test/integration/metadata/suites/application/utils/upload-application-file.util';
|
||||
import {
|
||||
type EachTestingContext,
|
||||
eachTestingContextFilter,
|
||||
} from 'twenty-shared/testing';
|
||||
import { FileFolder } from 'twenty-shared/types';
|
||||
import { v4 as uuidv4 } from 'uuid';
|
||||
|
||||
const TEST_APP_ID = uuidv4();
|
||||
|
||||
type TestContext = {
|
||||
fileFolder: string;
|
||||
fileFolderValue: FileFolder;
|
||||
filePath: string;
|
||||
filename: string;
|
||||
contentType: string;
|
||||
fileContent: string;
|
||||
};
|
||||
|
||||
const SUCCESSFUL_TEST_CASES: EachTestingContext<TestContext>[] = [
|
||||
{
|
||||
title: 'when uploading a built front component',
|
||||
context: {
|
||||
fileFolder: 'BuiltFrontComponent',
|
||||
fileFolderValue: FileFolder.BuiltFrontComponent,
|
||||
filePath: 'src/components/my-component.mjs',
|
||||
filename: 'my-component.mjs',
|
||||
contentType: 'application/javascript',
|
||||
fileContent: 'export default function MyComponent() {}',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when uploading a built logic function',
|
||||
context: {
|
||||
fileFolder: 'BuiltLogicFunction',
|
||||
fileFolderValue: FileFolder.BuiltLogicFunction,
|
||||
filePath: 'src/handlers/my-handler.mjs',
|
||||
filename: 'my-handler.mjs',
|
||||
contentType: 'application/javascript',
|
||||
fileContent: 'export default async function handler() {}',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when uploading a source file',
|
||||
context: {
|
||||
fileFolder: 'Source',
|
||||
fileFolderValue: FileFolder.Source,
|
||||
filePath: 'src/index.tsx',
|
||||
filename: 'index.tsx',
|
||||
contentType: 'text/plain',
|
||||
fileContent: 'export const App = () => <div>Hello</div>;',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when uploading a public asset',
|
||||
context: {
|
||||
fileFolder: 'PublicAsset',
|
||||
fileFolderValue: FileFolder.PublicAsset,
|
||||
filePath: 'assets/logo.svg',
|
||||
filename: 'logo.svg',
|
||||
contentType: 'image/svg+xml',
|
||||
fileContent: '<svg></svg>',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when uploading a dependencies file',
|
||||
context: {
|
||||
fileFolder: 'Dependencies',
|
||||
fileFolderValue: FileFolder.Dependencies,
|
||||
filePath: 'yarn.lock',
|
||||
filename: 'yarn.lock',
|
||||
contentType: 'text/plain',
|
||||
fileContent: '# yarn lockfile v1',
|
||||
},
|
||||
},
|
||||
{
|
||||
title: 'when uploading a file in a nested directory path',
|
||||
context: {
|
||||
fileFolder: 'Source',
|
||||
fileFolderValue: FileFolder.Source,
|
||||
filePath: 'src/modules/auth/login/login.component.tsx',
|
||||
filename: 'login.component.tsx',
|
||||
contentType: 'text/plain',
|
||||
fileContent: 'export const Login = () => null;',
|
||||
},
|
||||
},
|
||||
];
|
||||
|
||||
describe('Upload application file should succeed', () => {
|
||||
beforeAll(async () => {
|
||||
await setupApplicationForSync({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
name: 'Test Successful Upload App',
|
||||
description: 'App for testing successful file uploads',
|
||||
sourcePath: 'test-successful-upload',
|
||||
});
|
||||
}, 60000);
|
||||
|
||||
afterAll(async () => {
|
||||
await cleanupApplicationAndAppRegistration({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
});
|
||||
});
|
||||
|
||||
it.each(eachTestingContextFilter(SUCCESSFUL_TEST_CASES))(
|
||||
'$title',
|
||||
async ({ context }) => {
|
||||
jest.useRealTimers();
|
||||
|
||||
const { data, errors } = await uploadApplicationFile({
|
||||
applicationUniversalIdentifier: TEST_APP_ID,
|
||||
fileFolder: context.fileFolder,
|
||||
filePath: context.filePath,
|
||||
fileBuffer: Buffer.from(context.fileContent),
|
||||
filename: context.filename,
|
||||
contentType: context.contentType,
|
||||
expectToFail: false,
|
||||
});
|
||||
|
||||
jest.useFakeTimers();
|
||||
|
||||
expect(errors).toBeUndefined();
|
||||
expect(data.uploadApplicationFile).toEqual({
|
||||
id: expect.any(String),
|
||||
path: `${context.fileFolderValue}/${context.filePath}`,
|
||||
});
|
||||
},
|
||||
60000,
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user