Harden server-side input validation and auth defaults (#18018)

## Summary - **File storage (LocalDriver):** Add realpath resolution and symlink rejection to `writeFile`, `downloadFile`, and `downloadFolder` — brings them in line with the existing `readFile` protections. Includes unit tests. - **JWT:** Pin signing/verification to HS256 explicitly. - **Auth:** Revoke active refresh tokens when a user changes their password. - **Logic functions:** Validate `handlerName` as a safe JS identifier at both DTO and runtime level, preventing injection into the generated runner script. - **User entity:** Remove `passwordHash` from the GraphQL schema (`@Field` decorator removed, column stays). - **Query params:** Use `crypto.randomBytes` instead of `Math.random` for SQL parameter name generation. - **Exception filter:** Mirror the request `Origin` header instead of sending `Access-Control-Allow-Origin: *`. ## Test plan - [x] `local.driver.spec.ts` — writeFile rejects symlinks, downloadFile rejects paths outside storage - [ ] Verify JWT auth flow still works (login, token refresh) - [ ] Verify password change invalidates existing sessions - [ ] Verify logic function creation with valid/invalid handler names - [ ] Verify file upload/download in dev environment Made with [Cursor](https://cursor.com) --------- Co-authored-by: Cursor <cursoragent@cursor.com> Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com>
2026-02-18 21:41:01 +01:00
parent 330737aa2e
commit 67074a7581
11 changed files with 227 additions and 50 deletions
@@ -4874,7 +4874,6 @@ export type User = {
  lastName: Scalars['String'];
  locale: Scalars['String'];
  onboardingStatus?: Maybe<OnboardingStatus>;
-  passwordHash?: Maybe<Scalars['String']>;
  supportUserHash?: Maybe<Scalars['String']>;
  updatedAt: Scalars['DateTime'];
  userVars?: Maybe<Scalars['JSONObject']>;
@@ -1,3 +1,5 @@
+import { randomBytes } from 'crypto';
+
 import { type FieldMetadataType } from 'twenty-shared/types';
 import { isDefined } from 'twenty-shared/utils';
 import { type ObjectLiteral } from 'typeorm';
@@ -33,9 +35,9 @@ export const computeWhereConditionParts = ({
  fieldMetadataType: FieldMetadataType;
  useDirectTableReference?: boolean;
 }): WhereConditionParts => {
-  const uuid = Math.random().toString(36).slice(2, 7);
+  const paramSuffix = randomBytes(5).toString('hex');

-  const secondUuid = Math.random().toString(36).slice(2, 7);
+  const secondParamSuffix = randomBytes(5).toString('hex');

  const fieldReference = useDirectTableReference
    ? `"${key}"`
@@ -58,99 +60,99 @@ export const computeWhereConditionParts = ({
      };
    case 'eq':
      return {
-        sql: `${fieldReference} = :${key}${uuid}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NULL` : ''}`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} = :${key}${paramSuffix}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NULL` : ''}`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'neq':
      return {
-        sql: `${fieldReference} != :${key}${uuid}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NOT NULL` : ''}`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} != :${key}${paramSuffix}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NOT NULL` : ''}`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'gt':
      return {
-        sql: `${fieldReference} > :${key}${uuid}`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} > :${key}${paramSuffix}`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'gte':
      return {
-        sql: `${fieldReference} >= :${key}${uuid}`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} >= :${key}${paramSuffix}`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'lt':
      return {
-        sql: `${fieldReference} < :${key}${uuid}`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} < :${key}${paramSuffix}`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'lte':
      return {
-        sql: `${fieldReference} <= :${key}${uuid}`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} <= :${key}${paramSuffix}`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'in':
      return {
-        sql: `${fieldReference} IN (:...${key}${uuid})`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} IN (:...${key}${paramSuffix})`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'is':
      return {
-        sql: `${fieldReference} IS ${value === 'NULL' ? 'NULL' : 'NOT NULL'}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} = :${key}${secondUuid}` : ''}`,
+        sql: `${fieldReference} IS ${value === 'NULL' ? 'NULL' : 'NOT NULL'}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} = :${key}${secondParamSuffix}` : ''}`,
        params: hasNullEquivalentFieldValue
-          ? { [`${key}${secondUuid}`]: nullEquivalentFieldValue }
+          ? { [`${key}${secondParamSuffix}`]: nullEquivalentFieldValue }
          : {},
      };
    case 'like':
      return {
-        sql: `${fieldReference}::text LIKE :${key}${uuid}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NULL` : ''}`,
-        params: { [`${key}${uuid}`]: `${value}` },
+        sql: `${fieldReference}::text LIKE :${key}${paramSuffix}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NULL` : ''}`,
+        params: { [`${key}${paramSuffix}`]: `${value}` },
      };
    case 'ilike':
      return {
-        sql: `${fieldReference}::text ILIKE :${key}${uuid}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NULL` : ''}`,
-        params: { [`${key}${uuid}`]: `${value}` },
+        sql: `${fieldReference}::text ILIKE :${key}${paramSuffix}${hasNullEquivalentFieldValue ? ` OR ${fieldReference} IS NULL` : ''}`,
+        params: { [`${key}${paramSuffix}`]: `${value}` },
      };
    case 'startsWith':
      return {
-        sql: `${fieldReference}::text ^@ :${key}${uuid}`,
-        params: { [`${key}${uuid}`]: `${value}` },
+        sql: `${fieldReference}::text ^@ :${key}${paramSuffix}`,
+        params: { [`${key}${paramSuffix}`]: `${value}` },
      };
    case 'endsWith':
      return {
-        sql: `RIGHT(${fieldReference}::text, LENGTH(:${key}${uuid})) = :${key}${uuid}`,
-        params: { [`${key}${uuid}`]: `${value}` },
+        sql: `RIGHT(${fieldReference}::text, LENGTH(:${key}${paramSuffix})) = :${key}${paramSuffix}`,
+        params: { [`${key}${paramSuffix}`]: `${value}` },
      };
    case 'contains':
      return {
-        sql: `${fieldReference} @> ARRAY[:...${key}${uuid}]`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference} @> ARRAY[:...${key}${paramSuffix}]`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'search': {
      const tsQuery = formatSearchTerms(value, 'and');

      return {
        sql: `(
-          ${fieldReference} @@ to_tsquery('simple', public.unaccent_immutable(:${key}${uuid}Ts)) OR
-          public.unaccent_immutable(${fieldReference}::text) ILIKE public.unaccent_immutable(:${key}${uuid}Like)
+          ${fieldReference} @@ to_tsquery('simple', public.unaccent_immutable(:${key}${paramSuffix}Ts)) OR
+          public.unaccent_immutable(${fieldReference}::text) ILIKE public.unaccent_immutable(:${key}${paramSuffix}Like)
        )`,
        params: {
-          [`${key}${uuid}Ts`]: tsQuery,
-          [`${key}${uuid}Like`]: `%${value}%`,
+          [`${key}${paramSuffix}Ts`]: tsQuery,
+          [`${key}${paramSuffix}Like`]: `%${value}%`,
        },
      };
    }
    case 'notContains':
      return {
-        sql: `NOT (${fieldReference}::text[] && ARRAY[:...${key}${uuid}]::text[])`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `NOT (${fieldReference}::text[] && ARRAY[:...${key}${paramSuffix}]::text[])`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'containsAny':
      return {
-        sql: `${fieldReference}::text[] && ARRAY[:...${key}${uuid}]::text[]`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `${fieldReference}::text[] && ARRAY[:...${key}${paramSuffix}]::text[]`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    case 'containsIlike':
      return {
-        sql: `EXISTS (SELECT 1 FROM unnest(${fieldReference}) AS elem WHERE elem ILIKE :${key}${uuid})`,
-        params: { [`${key}${uuid}`]: value },
+        sql: `EXISTS (SELECT 1 FROM unnest(${fieldReference}) AS elem WHERE elem ILIKE :${key}${paramSuffix})`,
+        params: { [`${key}${paramSuffix}`]: value },
      };
    default:
      throw new GraphqlQueryRunnerException(
@@ -11,7 +11,7 @@ import { PasswordUpdateNotifyEmail } from 'twenty-emails';
 import { PermissionFlagType } from 'twenty-shared/constants';
 import { AppPath } from 'twenty-shared/types';
 import { assertIsDefinedOrThrow, isDefined } from 'twenty-shared/utils';
-import { Repository } from 'typeorm';
+import { IsNull, Repository } from 'typeorm';

 import { NodeEnvironment } from 'src/engine/core-modules/twenty-config/interfaces/node-environment.interface';

@@ -618,6 +618,18 @@ export class AuthService {
      passwordHash: newPasswordHash,
    });

+    // Invalidate all existing refresh tokens for this user across all workspaces
+    await this.appTokenRepository.update(
+      {
+        userId,
+        type: AppTokenType.RefreshToken,
+        revokedAt: IsNull(),
+      },
+      {
+        revokedAt: new Date(),
+      },
+    );
+
    const emailTemplate = PasswordUpdateNotifyEmail({
      userName: `${user.firstName} ${user.lastName}`,
      email: user.email,
@@ -0,0 +1,81 @@
+import { mkdtemp, mkdir, readFile, rm, symlink, writeFile } from 'fs/promises';
+import { tmpdir } from 'os';
+import path from 'path';
+
+import { FileStorageExceptionCode } from 'src/engine/core-modules/file-storage/interfaces/file-storage-exception';
+
+import { LocalDriver } from 'src/engine/core-modules/file-storage/drivers/local.driver';
+
+describe('LocalDriver security hardening', () => {
+  const cleanupPaths: string[] = [];
+
+  const createTempDirectory = async (prefix: string) => {
+    const dir = await mkdtemp(path.join(tmpdir(), prefix));
+
+    cleanupPaths.push(dir);
+
+    return dir;
+  };
+
+  afterAll(async () => {
+    await Promise.all(
+      cleanupPaths.map(async (directoryPath) => {
+        await rm(directoryPath, { recursive: true, force: true });
+      }),
+    );
+  });
+
+  it('should reject writeFile when target is a symlink', async () => {
+    const storagePath = await createTempDirectory('local-driver-storage-');
+    const outsidePath = await createTempDirectory('local-driver-outside-');
+    const outsideFilePath = path.join(outsidePath, 'outside.txt');
+    const symlinkFolderPath = path.join(storagePath, 'workspace', 'app');
+    const symlinkFilePath = path.join(symlinkFolderPath, 'target.txt');
+
+    await mkdir(symlinkFolderPath, { recursive: true });
+    await writeFile(outsideFilePath, 'outside');
+    await symlink(outsideFilePath, symlinkFilePath);
+
+    const driver = new LocalDriver({ storagePath });
+
+    await expect(
+      driver.writeFile({
+        filePath: 'workspace/app/target.txt',
+        sourceFile: Buffer.from('new-content'),
+        mimeType: undefined,
+      }),
+    ).rejects.toMatchObject({
+      code: FileStorageExceptionCode.ACCESS_DENIED,
+    });
+
+    await expect(readFile(outsideFilePath, 'utf8')).resolves.toBe('outside');
+  });
+
+  it('should reject downloadFile when path resolves outside storage', async () => {
+    const storagePath = await createTempDirectory('local-driver-storage-');
+    const outsidePath = await createTempDirectory('local-driver-outside-');
+    const outsideFilePath = path.join(outsidePath, 'outside.txt');
+    const symlinkFolderPath = path.join(storagePath, 'workspace', 'app');
+    const symlinkFilePath = path.join(symlinkFolderPath, 'target.txt');
+    const downloadDestinationPath = path.join(
+      storagePath,
+      'download',
+      'file.txt',
+    );
+
+    await mkdir(symlinkFolderPath, { recursive: true });
+    await writeFile(outsideFilePath, 'outside');
+    await symlink(outsideFilePath, symlinkFilePath);
+
+    const driver = new LocalDriver({ storagePath });
+
+    await expect(
+      driver.downloadFile({
+        onStoragePath: 'workspace/app/target.txt',
+        localPath: downloadDestinationPath,
+      }),
+    ).rejects.toMatchObject({
+      code: FileStorageExceptionCode.ACCESS_DENIED,
+    });
+  });
+});
@@ -74,17 +74,49 @@ export class LocalDriver implements StorageDriver {

    await this.createFolder(folderPath);

-    await fs.writeFile(filePath, params.sourceFile);
+    const realFolderPath = realpathSync(folderPath);
+    const realFilePath = path.join(realFolderPath, path.basename(filePath));
+
+    this.assertRealPathIsWithinStorage(realFilePath);
+
+    try {
+      const stats = await fs.lstat(realFilePath);
+
+      if (stats.isSymbolicLink()) {
+        throw new FileStorageException(
+          'Access denied',
+          FileStorageExceptionCode.ACCESS_DENIED,
+        );
+      }
+    } catch (error) {
+      if ((error as NodeJS.ErrnoException).code !== 'ENOENT') {
+        throw error;
+      }
+    }
+
+    await fs.writeFile(realFilePath, params.sourceFile);
  }

  async downloadFile(params: {
    onStoragePath: string;
    localPath: string;
  }): Promise<void> {
-    const filePath = path.resolve(
+    const resolvedPath = path.resolve(
      this.options.storagePath,
      params.onStoragePath,
    );
+    let filePath: string;
+
+    try {
+      filePath = realpathSync(resolvedPath);
+    } catch {
+      throw new FileStorageException(
+        'File not found',
+        FileStorageExceptionCode.FILE_NOT_FOUND,
+      );
+    }
+
+    this.assertRealPathIsWithinStorage(filePath);

    await this.createFolder(dirname(params.localPath));

@@ -97,26 +129,57 @@ export class LocalDriver implements StorageDriver {
    onStoragePath: string;
    localPath: string;
  }): Promise<void> {
-    const rootFolderPath = path.resolve(
+    const resolvedPath = path.resolve(
      this.options.storagePath,
      params.onStoragePath,
    );
+    let rootFolderPath: string;
+
+    try {
+      rootFolderPath = realpathSync(resolvedPath);
+    } catch {
+      throw new FileStorageException(
+        'File not found',
+        FileStorageExceptionCode.FILE_NOT_FOUND,
+      );
+    }
+
+    this.assertRealPathIsWithinStorage(rootFolderPath);

    await this.createFolder(params.localPath);

-    const resources = await fs.readdir(rootFolderPath);
+    await this.downloadFolderFromRealPath({
+      rootFolderPath,
+      localPath: params.localPath,
+    });
+  }
+
+  private async downloadFolderFromRealPath(params: {
+    rootFolderPath: string;
+    localPath: string;
+  }): Promise<void> {
+    await this.createFolder(params.localPath);
+
+    const resources = await fs.readdir(params.rootFolderPath);

    for (const resource of resources) {
-      const resourcePath = path.join(rootFolderPath, resource);
-      const stats = await fs.stat(resourcePath);
+      const resourcePath = path.join(params.rootFolderPath, resource);
+      const stats = await fs.lstat(resourcePath);
+
+      if (stats.isSymbolicLink()) {
+        throw new FileStorageException(
+          'Access denied',
+          FileStorageExceptionCode.ACCESS_DENIED,
+        );
+      }

      if (stats.isFile()) {
        const content = await fs.readFile(resourcePath);

        await fs.writeFile(path.join(params.localPath, resource), content);
      } else {
-        await this.downloadFolder({
-          onStoragePath: path.join(params.onStoragePath, resource),
+        await this.downloadFolderFromRealPath({
+          rootFolderPath: resourcePath,
          localPath: path.join(params.localPath, resource),
        });
      }
@@ -10,8 +10,12 @@ const InternalJwtModule = NestJwtModule.registerAsync({
    return {
      secret: twentyConfigService.get('APP_SECRET'),
      signOptions: {
+        algorithm: 'HS256',
        expiresIn: twentyConfigService.get('ACCESS_TOKEN_EXPIRES_IN'),
      },
+      verifyOptions: {
+        algorithms: ['HS256'],
+      },
    };
  },
  inject: [TwentyConfigService],
@@ -215,6 +215,12 @@ export class LocalDriver implements LogicFunctionDriver {
    builtFileAbsPath: string;
    handlerName: string;
  }) {
+    if (!/^[a-zA-Z_$][a-zA-Z0-9_$]*$/.test(handlerName)) {
+      throw new Error(
+        `Invalid handlerName "${handlerName}": must be a valid JavaScript identifier`,
+      );
+    }
+
    const runnerPath = join(dir, '__runner.cjs');
    const code = `
      // Auto-generated. Do not edit.
@@ -70,7 +70,6 @@ export class UserEntity {
  @Column({ default: false })
  disabled: boolean;

-  @Field({ nullable: true })
  @Column({ nullable: true })
  passwordHash: string;

@@ -8,6 +8,7 @@ import {
  IsOptional,
  IsString,
  IsUUID,
+  Matches,
  Max,
  Min,
 } from 'class-validator';
@@ -71,6 +72,9 @@ export class CreateLogicFunction {
  checksum?: string;

  @IsString()
+  @Matches(/^[a-zA-Z_$][a-zA-Z0-9_$]*$/, {
+    message: 'handlerName must be a valid JavaScript identifier',
+  })
  @Field({ nullable: false })
  handlerName: string;

@@ -1,6 +1,6 @@
 import { Field, InputType } from '@nestjs/graphql';

-import { IsObject, IsString } from 'class-validator';
+import { IsObject, IsString, Matches } from 'class-validator';
 import graphqlTypeJson from 'graphql-type-json';

@InputType()
@@ -14,6 +14,9 @@ export class LogicFunctionSourceInput {
  toolInputSchema: object;

  @IsString()
+  @Matches(/^[a-zA-Z_$][a-zA-Z0-9_$]*$/, {
+    message: 'handlerName must be a valid JavaScript identifier',
+  })
  @Field({ nullable: false })
  handlerName: string;
 }
@@ -9,6 +9,7 @@ import {
  IsOptional,
  IsString,
  IsUUID,
+  Matches,
  Max,
  Min,
  ValidateNested,
@@ -53,6 +54,9 @@ class UpdateLogicFunctionFromSourceInputUpdates {
  toolInputSchema?: object;

  @IsString()
+  @Matches(/^[a-zA-Z_$][a-zA-Z0-9_$]*$/, {
+    message: 'handlerName must be a valid JavaScript identifier',
+  })
  @Field({ nullable: true })
  @IsOptional()
  handlerName?: string;