feat: add S3 presigned URL redirect for file downloads (#18864)
## Summary - When `STORAGE_S3_PRESIGNED_URL_BASE` is configured, the file controller returns a **302 redirect** to a presigned S3 URL instead of proxying every byte through the server. This eliminates server bandwidth and CPU overhead for S3-backed deployments. - For local storage or S3 without a public endpoint, behavior is unchanged (stream + pipe with security headers). - Added `getPresignedUrl` to the `StorageDriver` interface (required method returning `string | null`), with implementations in S3Driver (uses a separate presign client with the public endpoint), LocalDriver (returns `null`), and ValidatedStorageDriver (path traversal protection + delegation). - Added a unified `getFileResponseById` method in `FileService` that performs a single DB lookup and returns either a redirect URL or a stream, avoiding double lookups. - Extracted `getContentDisposition` from the header util so both the proxy path and presigned URL path share the same inline/attachment allowlist. - Added MinIO service to `docker-compose.dev.yml` (optional `s3` profile) for local S3 testing. - Documented S3 presigned URL setup, CORS, and `nosniff` requirements in the self-hosting docs. ## Test plan - [x] All 63 unit tests pass across 5 test suites (util, S3 driver, validated driver, file storage service, controller) - [x] `npx nx typecheck twenty-server` passes - [ ] Manual E2E test with MinIO: `docker compose --profile s3 up -d`, configure S3 env vars, verify `curl -I` returns 302 with `Location` header pointing to MinIO - [ ] Verify local storage (no `STORAGE_S3_PRESIGNED_URL_BASE`) still streams files with 200 + security headers - [ ] Verify public assets endpoint still proxies (no redirect) Made with [Cursor](https://cursor.com)
This commit is contained in:
@@ -16,3 +16,5 @@ STORAGE_TYPE=local
|
||||
# STORAGE_S3_REGION=eu-west3
|
||||
# STORAGE_S3_NAME=my-bucket
|
||||
# STORAGE_S3_ENDPOINT=
|
||||
# STORAGE_S3_ACCESS_KEY_ID=
|
||||
# STORAGE_S3_SECRET_ACCESS_KEY=
|
||||
|
||||
@@ -17,6 +17,7 @@ COPY ./packages/twenty-ui/package.json /app/packages/twenty-ui/
|
||||
COPY ./packages/twenty-shared/package.json /app/packages/twenty-shared/
|
||||
COPY ./packages/twenty-front/package.json /app/packages/twenty-front/
|
||||
COPY ./packages/twenty-sdk/package.json /app/packages/twenty-sdk/
|
||||
COPY ./packages/twenty-client-sdk/package.json /app/packages/twenty-client-sdk/
|
||||
|
||||
RUN yarn && yarn cache clean && npx nx reset
|
||||
|
||||
@@ -27,6 +28,7 @@ COPY ./packages/twenty-emails /app/packages/twenty-emails
|
||||
COPY ./packages/twenty-shared /app/packages/twenty-shared
|
||||
COPY ./packages/twenty-ui /app/packages/twenty-ui
|
||||
COPY ./packages/twenty-sdk /app/packages/twenty-sdk
|
||||
COPY ./packages/twenty-client-sdk /app/packages/twenty-client-sdk
|
||||
COPY ./packages/twenty-server /app/packages/twenty-server
|
||||
|
||||
RUN npx nx run twenty-server:lingui:extract && \
|
||||
@@ -47,7 +49,7 @@ RUN npx esbuild packages/twenty-server/scripts/setup-db.ts \
|
||||
RUN find /app/packages/twenty-server/dist -name '*.d.ts' -delete \
|
||||
&& rm -rf /app/packages/twenty-server/dist/packages/twenty-server/test
|
||||
|
||||
RUN yarn workspaces focus --production twenty-emails twenty-shared twenty-sdk twenty-server
|
||||
RUN yarn workspaces focus --production twenty-emails twenty-shared twenty-sdk twenty-client-sdk twenty-server
|
||||
|
||||
|
||||
FROM common-deps AS twenty-front-build
|
||||
@@ -58,6 +60,7 @@ COPY ./packages/twenty-front /app/packages/twenty-front
|
||||
COPY ./packages/twenty-ui /app/packages/twenty-ui
|
||||
COPY ./packages/twenty-shared /app/packages/twenty-shared
|
||||
COPY ./packages/twenty-sdk /app/packages/twenty-sdk
|
||||
COPY ./packages/twenty-client-sdk /app/packages/twenty-client-sdk
|
||||
RUN npx nx run twenty-front:lingui:extract && \
|
||||
npx nx run twenty-front:lingui:compile
|
||||
# To skip the memory-intensive frontend build, pre-build on the host:
|
||||
@@ -106,6 +109,8 @@ COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-shared/dist /a
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-emails/package.json /app/packages/twenty-emails/
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-emails/dist /app/packages/twenty-emails/dist
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-sdk/package.json /app/packages/twenty-sdk/
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-client-sdk/package.json /app/packages/twenty-client-sdk/
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-client-sdk/dist /app/packages/twenty-client-sdk/dist
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-ui/package.json /app/packages/twenty-ui/
|
||||
COPY --chown=1000 --from=twenty-server-build /app/packages/twenty-front/package.json /app/packages/twenty-front/
|
||||
|
||||
@@ -180,6 +185,8 @@ COPY --from=twenty-server-build /app/packages/twenty-shared/dist /app/packages/t
|
||||
COPY --from=twenty-server-build /app/packages/twenty-emails/package.json /app/packages/twenty-emails/
|
||||
COPY --from=twenty-server-build /app/packages/twenty-emails/dist /app/packages/twenty-emails/dist
|
||||
COPY --from=twenty-server-build /app/packages/twenty-sdk/package.json /app/packages/twenty-sdk/
|
||||
COPY --from=twenty-server-build /app/packages/twenty-client-sdk/package.json /app/packages/twenty-client-sdk/
|
||||
COPY --from=twenty-server-build /app/packages/twenty-client-sdk/dist /app/packages/twenty-client-sdk/dist
|
||||
COPY --from=twenty-server-build /app/packages/twenty-ui/package.json /app/packages/twenty-ui/
|
||||
COPY --from=twenty-server-build /app/packages/twenty-front/package.json /app/packages/twenty-front/
|
||||
|
||||
|
||||
@@ -289,6 +289,16 @@ yarn command:prod cron:workflow:automated-cron-trigger
|
||||
**Environment-only mode:** If you set `IS_CONFIG_VARIABLES_IN_DB_ENABLED=false`, add these variables to your `.env` file instead.
|
||||
</Warning>
|
||||
|
||||
## S3 Storage
|
||||
|
||||
<Warning>
|
||||
By default, Twenty stores uploaded files on the local filesystem. For production deployments, use S3 or an S3-compatible service (MinIO, DigitalOcean Spaces, etc.) to ensure files persist across container restarts and scale across multiple server instances.
|
||||
</Warning>
|
||||
|
||||
Set `STORAGE_TYPE=S_3` and configure the `STORAGE_S3_*` variables through the admin panel or `.env`. See the [config-variables.ts reference](https://github.com/twentyhq/twenty/blob/main/packages/twenty-server/src/engine/core-modules/twenty-config/config-variables.ts) for the full list of S3 variables.
|
||||
|
||||
When using S3 with CORS-dependent features (e.g. in-browser file downloads), make sure your bucket allows your Twenty frontend origin in its CORS configuration.
|
||||
|
||||
## Logic Functions & Code Interpreter
|
||||
|
||||
Twenty supports logic functions for workflows and the code interpreter for AI data analysis. Both run user-provided code and require explicit configuration for security.
|
||||
|
||||
+42
@@ -67,6 +67,7 @@ describe('FileStorageService', () => {
|
||||
uploadFolder: jest.fn(),
|
||||
checkFileExists: jest.fn(),
|
||||
checkFolderExists: jest.fn(),
|
||||
getPresignedUrl: jest.fn(),
|
||||
};
|
||||
|
||||
mockFileStorageDriverFactory.getCurrentDriver.mockReturnValue(mockDriver);
|
||||
@@ -151,6 +152,47 @@ describe('FileStorageService', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('getPresignedUrl', () => {
|
||||
it('should delegate to the driver and return the URL', async () => {
|
||||
mockDriver.getPresignedUrl.mockResolvedValue(
|
||||
'https://s3.example.com/signed',
|
||||
);
|
||||
|
||||
mockApplicationRepository.findOneOrFail.mockResolvedValue({
|
||||
universalIdentifier: 'app-uid',
|
||||
});
|
||||
|
||||
const result = await service.getPresignedUrl({
|
||||
resourcePath: 'file.txt',
|
||||
fileFolder: 'workflow' as any,
|
||||
applicationUniversalIdentifier: 'app-uid',
|
||||
workspaceId: 'ws-id',
|
||||
responseContentType: 'image/png',
|
||||
responseContentDisposition: 'inline',
|
||||
});
|
||||
|
||||
expect(result).toBe('https://s3.example.com/signed');
|
||||
expect(mockDriver.getPresignedUrl).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should return null when driver returns null', async () => {
|
||||
mockDriver.getPresignedUrl.mockResolvedValue(null);
|
||||
|
||||
mockApplicationRepository.findOneOrFail.mockResolvedValue({
|
||||
universalIdentifier: 'app-uid',
|
||||
});
|
||||
|
||||
const result = await service.getPresignedUrl({
|
||||
resourcePath: 'file.txt',
|
||||
fileFolder: 'workflow' as any,
|
||||
applicationUniversalIdentifier: 'app-uid',
|
||||
workspaceId: 'ws-id',
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('checkFolderExistsLegacy', () => {
|
||||
it('should delegate to the current driver and return true', async () => {
|
||||
const checkParams = {
|
||||
|
||||
+115
@@ -0,0 +1,115 @@
|
||||
import { GetObjectCommand } from '@aws-sdk/client-s3';
|
||||
import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
|
||||
|
||||
import { S3Driver } from 'src/engine/core-modules/file-storage/drivers/s3.driver';
|
||||
|
||||
jest.mock('@aws-sdk/client-s3', () => {
|
||||
const actual = jest.requireActual('@aws-sdk/client-s3');
|
||||
|
||||
return {
|
||||
...actual,
|
||||
S3: jest.fn().mockImplementation(() => ({})),
|
||||
};
|
||||
});
|
||||
|
||||
jest.mock('@aws-sdk/s3-request-presigner', () => ({
|
||||
getSignedUrl: jest.fn(),
|
||||
}));
|
||||
|
||||
describe('S3Driver.getPresignedUrl', () => {
|
||||
afterEach(() => {
|
||||
jest.clearAllMocks();
|
||||
});
|
||||
|
||||
it('should return null when presigning is not enabled', async () => {
|
||||
const driver = new S3Driver({
|
||||
bucketName: 'test-bucket',
|
||||
region: 'us-east-1',
|
||||
endpoint: 'http://localhost:9000',
|
||||
});
|
||||
|
||||
const result = await driver.getPresignedUrl({
|
||||
filePath: 'some/file.png',
|
||||
});
|
||||
|
||||
expect(result).toBeNull();
|
||||
expect(getSignedUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should presign with the main client when enabled without endpoint override', async () => {
|
||||
(getSignedUrl as jest.Mock).mockResolvedValue(
|
||||
'https://s3.us-east-1.amazonaws.com/test-bucket/file.png?X-Amz-Signature=abc',
|
||||
);
|
||||
|
||||
const driver = new S3Driver({
|
||||
bucketName: 'test-bucket',
|
||||
region: 'us-east-1',
|
||||
presignEnabled: true,
|
||||
});
|
||||
|
||||
const result = await driver.getPresignedUrl({
|
||||
filePath: 'file.png',
|
||||
responseContentType: 'image/png',
|
||||
responseContentDisposition: 'inline',
|
||||
});
|
||||
|
||||
expect(result).toBe(
|
||||
'https://s3.us-east-1.amazonaws.com/test-bucket/file.png?X-Amz-Signature=abc',
|
||||
);
|
||||
expect(getSignedUrl).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.any(GetObjectCommand),
|
||||
{ expiresIn: 900 },
|
||||
);
|
||||
});
|
||||
|
||||
it('should presign with a separate client when endpoint override is provided', async () => {
|
||||
(getSignedUrl as jest.Mock).mockResolvedValue(
|
||||
'https://public.s3.com/test-bucket/some/file.png?X-Amz-Signature=abc',
|
||||
);
|
||||
|
||||
const driver = new S3Driver({
|
||||
bucketName: 'test-bucket',
|
||||
region: 'us-east-1',
|
||||
endpoint: 'http://internal-minio:9000',
|
||||
presignEnabled: true,
|
||||
presignEndpoint: 'https://public.s3.com',
|
||||
});
|
||||
|
||||
const result = await driver.getPresignedUrl({
|
||||
filePath: 'some/file.png',
|
||||
responseContentType: 'image/png',
|
||||
responseContentDisposition: 'inline',
|
||||
});
|
||||
|
||||
expect(result).toBe(
|
||||
'https://public.s3.com/test-bucket/some/file.png?X-Amz-Signature=abc',
|
||||
);
|
||||
expect(getSignedUrl).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.any(GetObjectCommand),
|
||||
{ expiresIn: 900 },
|
||||
);
|
||||
});
|
||||
|
||||
it('should use custom expiry when provided', async () => {
|
||||
(getSignedUrl as jest.Mock).mockResolvedValue('https://signed.url');
|
||||
|
||||
const driver = new S3Driver({
|
||||
bucketName: 'test-bucket',
|
||||
region: 'us-east-1',
|
||||
presignEnabled: true,
|
||||
});
|
||||
|
||||
await driver.getPresignedUrl({
|
||||
filePath: 'file.txt',
|
||||
expiresInSeconds: 3600,
|
||||
});
|
||||
|
||||
expect(getSignedUrl).toHaveBeenCalledWith(
|
||||
expect.anything(),
|
||||
expect.any(GetObjectCommand),
|
||||
{ expiresIn: 3600 },
|
||||
);
|
||||
});
|
||||
});
|
||||
+30
@@ -16,6 +16,7 @@ const createMockDriver = (): jest.Mocked<StorageDriver> => ({
|
||||
copy: jest.fn().mockResolvedValue(undefined),
|
||||
checkFileExists: jest.fn().mockResolvedValue(true),
|
||||
checkFolderExists: jest.fn().mockResolvedValue(true),
|
||||
getPresignedUrl: jest.fn().mockResolvedValue(null),
|
||||
});
|
||||
|
||||
describe('ValidatedStorageDriver', () => {
|
||||
@@ -118,6 +119,25 @@ describe('ValidatedStorageDriver', () => {
|
||||
folderPath: 'folder',
|
||||
});
|
||||
});
|
||||
|
||||
it('should delegate getPresignedUrl', async () => {
|
||||
mockDelegate.getPresignedUrl.mockResolvedValue(
|
||||
'https://s3.example.com/signed',
|
||||
);
|
||||
|
||||
const result = await driver.getPresignedUrl({
|
||||
filePath: 'folder/file.txt',
|
||||
responseContentType: 'image/png',
|
||||
responseContentDisposition: 'inline',
|
||||
});
|
||||
|
||||
expect(result).toBe('https://s3.example.com/signed');
|
||||
expect(mockDelegate.getPresignedUrl).toHaveBeenCalledWith({
|
||||
filePath: 'folder/file.txt',
|
||||
responseContentType: 'image/png',
|
||||
responseContentDisposition: 'inline',
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('rejects path traversal attempts', () => {
|
||||
@@ -203,6 +223,16 @@ describe('ValidatedStorageDriver', () => {
|
||||
|
||||
expect(mockDelegate.downloadFolder).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should reject getPresignedUrl with traversal', async () => {
|
||||
await expect(
|
||||
driver.getPresignedUrl({ filePath: '../../../etc/passwd' }),
|
||||
).rejects.toMatchObject({
|
||||
code: FileStorageExceptionCode.ACCESS_DENIED,
|
||||
});
|
||||
|
||||
expect(mockDelegate.getPresignedUrl).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
describe('does NOT validate localPath parameters', () => {
|
||||
|
||||
+7
@@ -34,4 +34,11 @@ export interface StorageDriver {
|
||||
|
||||
checkFileExists(params: { filePath: string }): Promise<boolean>;
|
||||
checkFolderExists(params: { folderPath: string }): Promise<boolean>;
|
||||
|
||||
getPresignedUrl(params: {
|
||||
filePath: string;
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
}): Promise<string | null>;
|
||||
}
|
||||
|
||||
@@ -300,6 +300,10 @@ export class LocalDriver implements StorageDriver {
|
||||
return existsSync(fullPath);
|
||||
}
|
||||
|
||||
async getPresignedUrl(): Promise<string | null> {
|
||||
return null;
|
||||
}
|
||||
|
||||
async checkFolderExists(params: { folderPath: string }): Promise<boolean> {
|
||||
const folderFullPath = path.resolve(
|
||||
this.options.storagePath,
|
||||
|
||||
@@ -20,6 +20,7 @@ import {
|
||||
S3,
|
||||
type S3ClientConfig,
|
||||
} from '@aws-sdk/client-s3';
|
||||
import { getSignedUrl } from '@aws-sdk/s3-request-presigner';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { type StorageDriver } from 'src/engine/core-modules/file-storage/drivers/interfaces/storage-driver.interface';
|
||||
@@ -32,15 +33,25 @@ export interface S3DriverOptions extends S3ClientConfig {
|
||||
bucketName: string;
|
||||
endpoint?: string;
|
||||
region: string;
|
||||
presignEnabled?: boolean;
|
||||
presignEndpoint?: string;
|
||||
}
|
||||
|
||||
export class S3Driver implements StorageDriver {
|
||||
private s3Client: S3;
|
||||
private presignClient: S3 | undefined;
|
||||
private bucketName: string;
|
||||
private readonly logger = new Logger(S3Driver.name);
|
||||
|
||||
constructor(options: S3DriverOptions) {
|
||||
const { bucketName, region, endpoint, ...s3Options } = options;
|
||||
const {
|
||||
bucketName,
|
||||
region,
|
||||
endpoint,
|
||||
presignEnabled,
|
||||
presignEndpoint,
|
||||
...s3Options
|
||||
} = options;
|
||||
|
||||
if (!bucketName || !region) {
|
||||
return;
|
||||
@@ -48,6 +59,12 @@ export class S3Driver implements StorageDriver {
|
||||
|
||||
this.s3Client = new S3({ ...s3Options, region, endpoint });
|
||||
this.bucketName = bucketName;
|
||||
|
||||
if (presignEnabled) {
|
||||
this.presignClient = presignEndpoint
|
||||
? new S3({ ...s3Options, region, endpoint: presignEndpoint })
|
||||
: this.s3Client;
|
||||
}
|
||||
}
|
||||
|
||||
public get client(): S3 {
|
||||
@@ -363,6 +380,28 @@ export class S3Driver implements StorageDriver {
|
||||
}
|
||||
}
|
||||
|
||||
async getPresignedUrl(params: {
|
||||
filePath: string;
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
}): Promise<string | null> {
|
||||
if (!this.presignClient) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const command = new GetObjectCommand({
|
||||
Bucket: this.bucketName,
|
||||
Key: params.filePath,
|
||||
ResponseContentType: params.responseContentType,
|
||||
ResponseContentDisposition: params.responseContentDisposition,
|
||||
});
|
||||
|
||||
return getSignedUrl(this.presignClient, command, {
|
||||
expiresIn: params.expiresInSeconds ?? 900,
|
||||
});
|
||||
}
|
||||
|
||||
async checkBucketExists(args: HeadBucketCommandInput) {
|
||||
try {
|
||||
await this.s3Client.headBucket(args);
|
||||
|
||||
+11
@@ -99,6 +99,17 @@ export class ValidatedStorageDriver implements StorageDriver {
|
||||
return this.delegate.copy(params);
|
||||
}
|
||||
|
||||
async getPresignedUrl(params: {
|
||||
filePath: string;
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
}): Promise<string | null> {
|
||||
assertStoragePathIsSafe(params.filePath);
|
||||
|
||||
return this.delegate.getPresignedUrl(params);
|
||||
}
|
||||
|
||||
async checkFileExists(params: { filePath: string }): Promise<boolean> {
|
||||
assertStoragePathIsSafe(params.filePath);
|
||||
|
||||
|
||||
+8
@@ -63,10 +63,18 @@ export class FileStorageDriverFactory extends DriverFactoryBase<StorageDriver> {
|
||||
const secretAccessKey = this.twentyConfigService.get(
|
||||
'STORAGE_S3_SECRET_ACCESS_KEY',
|
||||
);
|
||||
const presignEnabled = this.twentyConfigService.get(
|
||||
'STORAGE_S3_PRESIGNED_URL_ENABLED',
|
||||
);
|
||||
const presignEndpointOverride = this.twentyConfigService.get(
|
||||
'STORAGE_S3_PRESIGNED_URL_BASE',
|
||||
);
|
||||
|
||||
rawDriver = new S3Driver({
|
||||
bucketName: bucketName ?? '',
|
||||
endpoint: endpoint,
|
||||
presignEnabled,
|
||||
presignEndpoint: presignEndpointOverride || undefined,
|
||||
credentials: accessKeyId
|
||||
? { accessKeyId, secretAccessKey }
|
||||
: fromNodeProviderChain({ clientConfig: { region } }),
|
||||
|
||||
@@ -114,6 +114,24 @@ export class FileStorageService {
|
||||
});
|
||||
}
|
||||
|
||||
async getPresignedUrl(
|
||||
params: ResourceIdentifier & {
|
||||
expiresInSeconds?: number;
|
||||
responseContentType?: string;
|
||||
responseContentDisposition?: string;
|
||||
},
|
||||
): Promise<string | null> {
|
||||
const driver = this.fileStorageDriverFactory.getCurrentDriver();
|
||||
const onStoragePath = this.buildOnStoragePath(params);
|
||||
|
||||
return driver.getPresignedUrl({
|
||||
filePath: onStoragePath,
|
||||
expiresInSeconds: params.expiresInSeconds,
|
||||
responseContentType: params.responseContentType,
|
||||
responseContentDisposition: params.responseContentDisposition,
|
||||
});
|
||||
}
|
||||
|
||||
readFile(params: ResourceIdentifier): Promise<Readable> {
|
||||
const driver = this.fileStorageDriverFactory.getCurrentDriver();
|
||||
|
||||
|
||||
+36
-10
@@ -34,6 +34,7 @@ const createMockStream = (): Readable => {
|
||||
|
||||
const createMockResponse = () => ({
|
||||
setHeader: jest.fn(),
|
||||
redirect: jest.fn(),
|
||||
});
|
||||
|
||||
describe('FileController', () => {
|
||||
@@ -56,6 +57,7 @@ describe('FileController', () => {
|
||||
useValue: {
|
||||
getFileStreamById: jest.fn(),
|
||||
getFileStreamByPath: jest.fn(),
|
||||
getFileResponseById: jest.fn(),
|
||||
},
|
||||
},
|
||||
],
|
||||
@@ -79,10 +81,38 @@ describe('FileController', () => {
|
||||
});
|
||||
|
||||
describe('getFileById', () => {
|
||||
it('should call fileService.getFileStreamById and pipe the result with headers', async () => {
|
||||
it('should 302 redirect when presigned URL is available', async () => {
|
||||
jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({
|
||||
type: 'redirect',
|
||||
presignedUrl: 'https://s3.example.com/file?signed=abc',
|
||||
});
|
||||
|
||||
const mockRequest = { workspaceId: 'workspace-id' } as any;
|
||||
const mockResponse = createMockResponse() as any;
|
||||
|
||||
await controller.getFileById(
|
||||
mockResponse,
|
||||
mockRequest,
|
||||
FileFolder.Workflow,
|
||||
'file-123',
|
||||
);
|
||||
|
||||
expect(fileService.getFileResponseById).toHaveBeenCalledWith({
|
||||
fileId: 'file-123',
|
||||
workspaceId: 'workspace-id',
|
||||
fileFolder: FileFolder.Workflow,
|
||||
});
|
||||
expect(mockResponse.redirect).toHaveBeenCalledWith(
|
||||
'https://s3.example.com/file?signed=abc',
|
||||
);
|
||||
expect(mockResponse.setHeader).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it('should stream with headers when no presigned URL (local driver)', async () => {
|
||||
const mockStream = createMockStream();
|
||||
|
||||
jest.spyOn(fileService, 'getFileStreamById').mockResolvedValue({
|
||||
jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({
|
||||
type: 'stream',
|
||||
stream: mockStream,
|
||||
mimeType: 'image/png',
|
||||
});
|
||||
@@ -97,11 +127,6 @@ describe('FileController', () => {
|
||||
'file-123',
|
||||
);
|
||||
|
||||
expect(fileService.getFileStreamById).toHaveBeenCalledWith({
|
||||
fileId: 'file-123',
|
||||
workspaceId: 'workspace-id',
|
||||
fileFolder: FileFolder.CorePicture,
|
||||
});
|
||||
expect(mockResponse.setHeader).toHaveBeenCalledWith(
|
||||
'Content-Type',
|
||||
'image/png',
|
||||
@@ -120,7 +145,8 @@ describe('FileController', () => {
|
||||
it('should force attachment disposition for non-safe MIME types', async () => {
|
||||
const mockStream = createMockStream();
|
||||
|
||||
jest.spyOn(fileService, 'getFileStreamById').mockResolvedValue({
|
||||
jest.spyOn(fileService, 'getFileResponseById').mockResolvedValue({
|
||||
type: 'stream',
|
||||
stream: mockStream,
|
||||
mimeType: 'text/html',
|
||||
});
|
||||
@@ -147,7 +173,7 @@ describe('FileController', () => {
|
||||
|
||||
it('should throw FileException with FILE_NOT_FOUND when file is not found', async () => {
|
||||
jest
|
||||
.spyOn(fileService, 'getFileStreamById')
|
||||
.spyOn(fileService, 'getFileResponseById')
|
||||
.mockRejectedValue(
|
||||
new FileStorageException(
|
||||
'File not found',
|
||||
@@ -172,7 +198,7 @@ describe('FileController', () => {
|
||||
|
||||
it('should throw FileException with INTERNAL_SERVER_ERROR for unexpected errors', async () => {
|
||||
jest
|
||||
.spyOn(fileService, 'getFileStreamById')
|
||||
.spyOn(fileService, 'getFileResponseById')
|
||||
.mockRejectedValue(new Error('Storage unavailable'));
|
||||
|
||||
const mockRequest = { workspaceId: 'workspace-id' } as any;
|
||||
|
||||
@@ -96,22 +96,29 @@ export class FileController {
|
||||
const workspaceId = (req as any)?.workspaceId;
|
||||
|
||||
try {
|
||||
const { stream, mimeType } = await this.fileService.getFileStreamById({
|
||||
const fileResponse = await this.fileService.getFileResponseById({
|
||||
fileId,
|
||||
workspaceId,
|
||||
fileFolder,
|
||||
});
|
||||
|
||||
setFileResponseHeaders(res, mimeType);
|
||||
if (fileResponse.type === 'redirect') {
|
||||
return res.redirect(fileResponse.presignedUrl);
|
||||
}
|
||||
|
||||
stream.on('error', () => {
|
||||
throw new FileException(
|
||||
'Error streaming file from storage',
|
||||
FileExceptionCode.INTERNAL_SERVER_ERROR,
|
||||
);
|
||||
setFileResponseHeaders(res, fileResponse.mimeType);
|
||||
|
||||
fileResponse.stream.on('error', () => {
|
||||
if (!res.headersSent) {
|
||||
res.status(500).send('Error streaming file from storage');
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
res.destroy();
|
||||
});
|
||||
|
||||
stream.pipe(res);
|
||||
fileResponse.stream.pipe(res);
|
||||
} catch (error) {
|
||||
if (
|
||||
error instanceof FileStorageException &&
|
||||
|
||||
@@ -19,6 +19,8 @@ import {
|
||||
import { FileStorageService } from 'src/engine/core-modules/file-storage/file-storage.service';
|
||||
import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
|
||||
import { removeFileFolderFromFileEntityPath } from 'src/engine/core-modules/file/utils/remove-file-folder-from-file-entity-path.utils';
|
||||
import { type FileResponse } from 'src/engine/core-modules/file/types/file-response.type';
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
import { JwtWrapperService } from 'src/engine/core-modules/jwt/services/jwt-wrapper.service';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
import { streamToBuffer } from 'src/utils/stream-to-buffer';
|
||||
@@ -111,6 +113,52 @@ export class FileService {
|
||||
};
|
||||
}
|
||||
|
||||
async getFileResponseById(params: {
|
||||
fileId: string;
|
||||
workspaceId: string;
|
||||
fileFolder: FileFolder;
|
||||
}): Promise<FileResponse> {
|
||||
const file = await this.fileRepository.findOneOrFail({
|
||||
where: {
|
||||
id: params.fileId,
|
||||
workspaceId: params.workspaceId,
|
||||
path: Like(`${params.fileFolder}/%`),
|
||||
},
|
||||
});
|
||||
|
||||
const application = await this.applicationRepository.findOneOrFail({
|
||||
where: {
|
||||
id: file.applicationId,
|
||||
workspaceId: params.workspaceId,
|
||||
},
|
||||
});
|
||||
|
||||
const mimeType = file.mimeType ?? 'application/octet-stream';
|
||||
const resourceIdentifier = {
|
||||
resourcePath: removeFileFolderFromFileEntityPath(file.path),
|
||||
fileFolder: params.fileFolder,
|
||||
applicationUniversalIdentifier: application.universalIdentifier,
|
||||
workspaceId: params.workspaceId,
|
||||
};
|
||||
|
||||
const presignedUrl = await this.fileStorageService.getPresignedUrl({
|
||||
...resourceIdentifier,
|
||||
expiresInSeconds: this.twentyConfigService.get(
|
||||
'STORAGE_S3_PRESIGNED_URL_EXPIRES_IN',
|
||||
),
|
||||
responseContentType: mimeType,
|
||||
responseContentDisposition: getContentDisposition(mimeType),
|
||||
});
|
||||
|
||||
if (presignedUrl) {
|
||||
return { type: 'redirect', presignedUrl };
|
||||
}
|
||||
|
||||
const stream = await this.fileStorageService.readFile(resourceIdentifier);
|
||||
|
||||
return { type: 'stream', stream, mimeType };
|
||||
}
|
||||
|
||||
async getFileContentById({
|
||||
fileId,
|
||||
workspaceId,
|
||||
|
||||
@@ -0,0 +1,5 @@
|
||||
import { type Readable } from 'stream';
|
||||
|
||||
export type FileResponse =
|
||||
| { type: 'redirect'; presignedUrl: string }
|
||||
| { type: 'stream'; stream: Readable; mimeType: string };
|
||||
+90
@@ -0,0 +1,90 @@
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
import { setFileResponseHeaders } from 'src/engine/core-modules/file/utils/set-file-response-headers.utils';
|
||||
|
||||
const createMockResponse = () => ({
|
||||
setHeader: jest.fn(),
|
||||
});
|
||||
|
||||
describe('setFileResponseHeaders', () => {
|
||||
it('should set Content-Type from mimeType', () => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'image/png');
|
||||
|
||||
expect(res.setHeader).toHaveBeenCalledWith('Content-Type', 'image/png');
|
||||
});
|
||||
|
||||
it('should fall back to application/octet-stream for empty mimeType', () => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, '');
|
||||
|
||||
expect(res.setHeader).toHaveBeenCalledWith(
|
||||
'Content-Type',
|
||||
'application/octet-stream',
|
||||
);
|
||||
});
|
||||
|
||||
it('should always set X-Content-Type-Options: nosniff', () => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, 'text/html');
|
||||
|
||||
expect(res.setHeader).toHaveBeenCalledWith(
|
||||
'X-Content-Type-Options',
|
||||
'nosniff',
|
||||
);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'image/jpeg',
|
||||
'image/png',
|
||||
'image/gif',
|
||||
'image/webp',
|
||||
'application/pdf',
|
||||
'text/plain',
|
||||
'video/mp4',
|
||||
'audio/mpeg',
|
||||
])('should set Content-Disposition: inline for safe type %s', (mimeType) => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, mimeType);
|
||||
|
||||
expect(res.setHeader).toHaveBeenCalledWith('Content-Disposition', 'inline');
|
||||
});
|
||||
|
||||
it.each([
|
||||
'text/html',
|
||||
'image/svg+xml',
|
||||
'application/xml',
|
||||
'application/octet-stream',
|
||||
'application/javascript',
|
||||
])(
|
||||
'should set Content-Disposition: attachment for unsafe type %s',
|
||||
(mimeType) => {
|
||||
const res = createMockResponse();
|
||||
|
||||
setFileResponseHeaders(res as any, mimeType);
|
||||
|
||||
expect(res.setHeader).toHaveBeenCalledWith(
|
||||
'Content-Disposition',
|
||||
'attachment',
|
||||
);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
describe('getContentDisposition', () => {
|
||||
it('should return inline for safe MIME types', () => {
|
||||
expect(getContentDisposition('image/png')).toBe('inline');
|
||||
expect(getContentDisposition('application/pdf')).toBe('inline');
|
||||
});
|
||||
|
||||
it('should return attachment for unsafe MIME types', () => {
|
||||
expect(getContentDisposition('text/html')).toBe('attachment');
|
||||
expect(getContentDisposition('application/xml')).toBe('attachment');
|
||||
expect(getContentDisposition('application/octet-stream')).toBe(
|
||||
'attachment',
|
||||
);
|
||||
});
|
||||
});
|
||||
+22
@@ -0,0 +1,22 @@
|
||||
const INLINE_SAFE_MIME_TYPES = new Set([
|
||||
'image/jpeg',
|
||||
'image/png',
|
||||
'image/gif',
|
||||
'image/webp',
|
||||
'image/avif',
|
||||
'image/bmp',
|
||||
'image/tiff',
|
||||
'application/pdf',
|
||||
'text/plain',
|
||||
'audio/mpeg',
|
||||
'audio/wav',
|
||||
'audio/ogg',
|
||||
'video/mp4',
|
||||
'video/webm',
|
||||
'video/ogg',
|
||||
'image/x-icon',
|
||||
]);
|
||||
|
||||
export const getContentDisposition = (mimeType: string): string => {
|
||||
return INLINE_SAFE_MIME_TYPES.has(mimeType) ? 'inline' : 'attachment';
|
||||
};
|
||||
+2
-22
@@ -1,31 +1,11 @@
|
||||
import { type Response } from 'express';
|
||||
|
||||
const INLINE_SAFE_MIME_TYPES = new Set([
|
||||
'image/jpeg',
|
||||
'image/png',
|
||||
'image/gif',
|
||||
'image/webp',
|
||||
'image/avif',
|
||||
'image/bmp',
|
||||
'image/tiff',
|
||||
'application/pdf',
|
||||
'text/plain',
|
||||
'audio/mpeg',
|
||||
'audio/wav',
|
||||
'audio/ogg',
|
||||
'video/mp4',
|
||||
'video/webm',
|
||||
'video/ogg',
|
||||
'image/x-icon',
|
||||
]);
|
||||
import { getContentDisposition } from 'src/engine/core-modules/file/utils/get-content-disposition.utils';
|
||||
|
||||
export const setFileResponseHeaders = (res: Response, mimeType: string) => {
|
||||
const contentType = mimeType || 'application/octet-stream';
|
||||
const disposition = INLINE_SAFE_MIME_TYPES.has(contentType)
|
||||
? 'inline'
|
||||
: 'attachment';
|
||||
|
||||
res.setHeader('Content-Type', contentType);
|
||||
res.setHeader('X-Content-Type-Options', 'nosniff');
|
||||
res.setHeader('Content-Disposition', disposition);
|
||||
res.setHeader('Content-Disposition', getContentDisposition(contentType));
|
||||
};
|
||||
|
||||
@@ -420,7 +420,7 @@ export class ConfigVariables {
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
description: 'S3 region for storage when using S3 storage type',
|
||||
description: 'AWS region of the S3 bucket (e.g. eu-west-3). Required.',
|
||||
type: ConfigVariableType.STRING,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@@ -429,7 +429,7 @@ export class ConfigVariables {
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
description: 'S3 bucket name for storage when using S3 storage type',
|
||||
description: 'Name of the S3 bucket used for file storage. Required.',
|
||||
type: ConfigVariableType.STRING,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@@ -437,7 +437,8 @@ export class ConfigVariables {
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
description: 'S3 endpoint for storage when using S3 storage type',
|
||||
description:
|
||||
'Custom S3 endpoint URL. Optional — only needed for S3-compatible services like MinIO (e.g. http://minio:9000). Omit for native AWS S3, where the SDK resolves the endpoint from the region automatically.',
|
||||
type: ConfigVariableType.STRING,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@@ -448,7 +449,7 @@ export class ConfigVariables {
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
isSensitive: true,
|
||||
description:
|
||||
'S3 access key ID for authentication when using S3 storage type',
|
||||
'S3 access key ID. Optional — omit to use the default AWS credential chain (IAM role, instance profile, etc.).',
|
||||
type: ConfigVariableType.STRING,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@@ -459,13 +460,44 @@ export class ConfigVariables {
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
isSensitive: true,
|
||||
description:
|
||||
'S3 secret access key for authentication when using S3 storage type',
|
||||
'S3 secret access key. Required when STORAGE_S3_ACCESS_KEY_ID is set, ignored otherwise.',
|
||||
type: ConfigVariableType.STRING,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@IsOptional()
|
||||
STORAGE_S3_SECRET_ACCESS_KEY: string;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
description:
|
||||
'When enabled, file downloads are 302-redirected to S3 presigned URLs instead of being proxied through the server. Reduces server load and bandwidth.',
|
||||
type: ConfigVariableType.BOOLEAN,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@IsOptional()
|
||||
// TODO: default to true once validated in production
|
||||
STORAGE_S3_PRESIGNED_URL_ENABLED = false;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
description:
|
||||
'Public S3 endpoint used for generating presigned URLs. Optional — only needed when STORAGE_S3_ENDPOINT is an internal address not reachable by browsers (e.g. http://minio:9000 in Docker). Set this to the publicly accessible equivalent (e.g. https://storage.example.com).',
|
||||
type: ConfigVariableType.STRING,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@IsOptional()
|
||||
STORAGE_S3_PRESIGNED_URL_BASE: string;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.STORAGE_CONFIG,
|
||||
description: 'TTL in seconds for S3 presigned URLs.',
|
||||
type: ConfigVariableType.NUMBER,
|
||||
})
|
||||
@ValidateIf((env) => env.STORAGE_TYPE === StorageDriverType.S_3)
|
||||
@CastToPositiveNumber()
|
||||
@IsOptional()
|
||||
STORAGE_S3_PRESIGNED_URL_EXPIRES_IN: number = 900;
|
||||
|
||||
@ConfigVariablesMetadata({
|
||||
group: ConfigVariablesGroup.LOGIC_FUNCTION_CONFIG,
|
||||
description: 'Type of function execution (local or Lambda)',
|
||||
|
||||
Reference in New Issue
Block a user