fix: local storage, and package overrides

2026-05-07 06:23:06 +08:00 · 2026-04-22 14:13:25 +01:00
parent dba8e315ab
commit a573acedd0
11 changed files with 409 additions and 306 deletions
@@ -42,7 +42,7 @@
    "mantine-form-zod-resolver": "^1.3.0",
    "mermaid": "^11.13.0",
    "mitt": "^3.0.1",
-    "posthog-js": "1.363.1",
+    "posthog-js": "1.370.0",
    "react": "^18.3.1",
    "react-arborist": "3.4.0",
    "react-clear-modal": "^2.0.18",
@@ -38,8 +38,8 @@
    "@aws-sdk/s3-request-presigner": "3.1014.0",
    "@clickhouse/client": "^1.18.2",
    "@fastify/cookie": "^11.0.2",
-    "@fastify/multipart": "^9.4.0",
-    "@fastify/static": "^9.0.0",
+    "@fastify/multipart": "^10.0.0",
+    "@fastify/static": "^9.1.3",
    "@keyv/redis": "^5.1.6",
    "@langchain/core": "1.1.39",
    "@langchain/textsplitters": "1.0.1",
@@ -48,19 +48,19 @@
    "@nestjs-labs/nestjs-ioredis": "^11.0.4",
    "@nestjs/bullmq": "^11.0.4",
    "@nestjs/cache-manager": "^3.1.0",
-    "@nestjs/common": "^11.1.18",
-    "@nestjs/config": "^4.0.3",
-    "@nestjs/core": "^11.1.18",
+    "@nestjs/common": "^11.1.19",
+    "@nestjs/config": "^4.0.4",
+    "@nestjs/core": "^11.1.19",
    "@nestjs/event-emitter": "^3.0.1",
    "@nestjs/jwt": "11.0.2",
    "@nestjs/mapped-types": "^2.1.1",
    "@nestjs/passport": "^11.0.5",
-    "@nestjs/platform-fastify": "^11.1.18",
-    "@nestjs/platform-socket.io": "^11.1.18",
-    "@nestjs/schedule": "^6.1.1",
+    "@nestjs/platform-fastify": "^11.1.19",
+    "@nestjs/platform-socket.io": "^11.1.19",
+    "@nestjs/schedule": "^6.1.3",
    "@nestjs/terminus": "^11.1.1",
    "@nestjs/throttler": "^6.5.0",
-    "@nestjs/websockets": "^11.1.18",
+    "@nestjs/websockets": "^11.1.19",
    "@node-saml/passport-saml": "^5.1.0",
    "@react-email/components": "1.0.10",
    "@react-email/render": "2.0.4",
@@ -69,7 +69,7 @@
    "ai-sdk-ollama": "^3.8.1",
    "bcrypt": "^6.0.0",
    "bowser": "^2.14.1",
-    "bullmq": "^5.71.0",
+    "bullmq": "^5.76.0",
    "cache-manager": "^7.2.8",
    "cheerio": "^1.2.0",
    "class-transformer": "^0.5.1",
@@ -123,9 +123,9 @@
  },
  "devDependencies": {
    "@eslint/js": "^9.28.0",
-    "@nestjs/cli": "^11.0.18",
+    "@nestjs/cli": "^11.0.21",
    "@nestjs/schematics": "^11.0.10",
-    "@nestjs/testing": "^11.1.18",
+    "@nestjs/testing": "^11.1.19",
    "@types/bcrypt": "^6.0.0",
    "@types/debounce": "^1.2.4",
    "@types/fs-extra": "^11.0.4",
@@ -53,6 +53,7 @@ import { EnvironmentService } from '../../integrations/environment/environment.s
 import { TokenService } from '../auth/services/token.service';
 import { JwtAttachmentPayload, JwtType } from '../auth/dto/jwt-payload';
 import * as path from 'path';
+import { sanitize } from 'sanitize-filename-ts';
 import { AttachmentInfoDto, RemoveIconDto } from './dto/attachment.dto';
 import { PageAccessService } from '../page/page-access/page-access.service';
 import { AuditEvent, AuditResource } from '../../common/events/audit-events';
@@ -356,6 +357,10 @@ export class AttachmentController {
      throw new BadRequestException('Invalid image attachment type');
    }

+    if (!fileName || sanitize(fileName) !== fileName) {
+      throw new BadRequestException('Invalid file name');
+    }
+
    const filenameWithoutExt = path.basename(fileName, path.extname(fileName));
    if (!isValidUUID(filenameWithoutExt)) {
      throw new BadRequestException('Invalid file id');
@@ -13,10 +13,6 @@ import { CreateUserDto } from '../../auth/dto/create-user.dto';
 export class UpdateUserDto extends PartialType(
  OmitType(CreateUserDto, ['password'] as const),
 ) {
-  @IsOptional()
-  @IsString()
-  avatarUrl: string;
-
  @IsOptional()
  @IsBoolean()
  fullPageWidth: boolean;
@@ -110,10 +110,6 @@ export class UserService {
      user.email = updateUserDto.email;
    }

-    if (updateUserDto.avatarUrl) {
-      user.avatarUrl = updateUserDto.avatarUrl;
-    }
-
    if (updateUserDto.locale) {
      user.locale = updateUserDto.locale;
    }
@@ -5,15 +5,10 @@ import {
  IsBoolean,
  IsInt,
  IsOptional,
-  IsString,
  Min,
 } from 'class-validator';

 export class UpdateWorkspaceDto extends PartialType(CreateWorkspaceDto) {
-  @IsOptional()
-  @IsString()
-  logo: string;
-
  @IsOptional()
  @IsArray()
  emailDomains: string[];
@@ -39,6 +39,8 @@ import {
 } from '../../common/helpers/prosemirror/utils';
 import { htmlToMarkdown } from '@docmost/editor-ext';

+type AllowedAttachment = { id: string; fileName: string; filePath: string };
+
@Injectable()
 export class ExportService {
  private readonly logger = new Logger(ExportService.name);
@@ -272,6 +274,12 @@ export class ExportService {

    computeLocalPath(tree, format, null, '', slugIdToPath);

+    // Batch resolve attachments once for the whole export so we only run the
+    // owning-page view check a single time, regardless of page count.
+    const allowedAttachments = includeAttachments
+      ? await this.resolveAccessibleAttachments(tree, userId, ignorePermissions)
+      : new Map<string, AllowedAttachment>();
+
    const stack: { folder: JSZip; parentPageId: string | null }[] = [
      { folder: zip, parentPageId: null },
    ];
@@ -301,7 +309,7 @@ export class ExportService {
        );

        if (includeAttachments) {
-          await this.zipAttachments(updatedJsonContent, page.spaceId, folder);
+          await this.zipAttachments(updatedJsonContent, folder, allowedAttachments);
          updatedJsonContent =
            updateAttachmentUrlsToLocalPaths(updatedJsonContent);
        }
@@ -347,31 +355,80 @@ export class ExportService {
    zip.file('docmost-metadata.json', JSON.stringify(metadata, null, 2));
  }

-  async zipAttachments(prosemirrorJson: any, spaceId: string, zip: JSZip) {
+  async zipAttachments(
+    prosemirrorJson: any,
+    zip: JSZip,
+    allowed: Map<string, AllowedAttachment>,
+  ) {
    const attachmentIds = getAttachmentIds(prosemirrorJson);

-    if (attachmentIds.length > 0) {
-      const attachments = await this.db
-        .selectFrom('attachments')
-        .select(['id', 'fileName', 'filePath'])
-        .where('id', 'in', attachmentIds)
-        .where('spaceId', '=', spaceId)
-        .execute();
+    await Promise.all(
+      attachmentIds.map(async (id) => {
+        const attachment = allowed.get(id);
+        if (!attachment) return;
+        try {
+          const fileBuffer = await this.storageService.read(
+            attachment.filePath,
+          );
+          const filePath = `/files/${attachment.id}/${attachment.fileName}`;
+          zip.file(filePath, fileBuffer);
+        } catch (err) {
+          this.logger.debug(`Attachment export error ${attachment.id}`, err);
+        }
+      }),
+    );
+  }

-      await Promise.all(
-        attachments.map(async (attachment) => {
-          try {
-            const fileBuffer = await this.storageService.read(
-              attachment.filePath,
-            );
-            const filePath = `/files/${attachment.id}/${attachment.fileName}`;
-            zip.file(filePath, fileBuffer);
-          } catch (err) {
-            this.logger.debug(`Attachment export error ${attachment.id}`, err);
-          }
-        }),
+  private async resolveAccessibleAttachments(
+    tree: PageExportTree,
+    userId: string | undefined,
+    ignorePermissions: boolean,
+  ): Promise<Map<string, AllowedAttachment>> {
+    const allAttachmentIds = new Set<string>();
+    let spaceId: string | undefined;
+    for (const siblings of Object.values(tree)) {
+      for (const page of siblings) {
+        if (!spaceId) spaceId = page.spaceId;
+        for (const id of getAttachmentIds(getProsemirrorContent(page.content))) {
+          allAttachmentIds.add(id);
+        }
+      }
+    }
+
+    if (allAttachmentIds.size === 0 || !spaceId) {
+      return new Map();
+    }
+
+    const attachments = await this.db
+      .selectFrom('attachments')
+      .select(['id', 'fileName', 'filePath', 'pageId'])
+      .where('id', 'in', [...allAttachmentIds])
+      .where('spaceId', '=', spaceId)
+      .execute();
+
+    let visible = attachments;
+    if (!ignorePermissions && userId) {
+      const ownerPageIds = [
+        ...new Set(
+          attachments
+            .map((a) => a.pageId)
+            .filter((id): id is string => !!id),
+        ),
+      ];
+      const accessible = ownerPageIds.length
+        ? await this.pagePermissionRepo.filterAccessiblePageIds({
+            pageIds: ownerPageIds,
+            userId,
+            spaceId,
+          })
+        : [];
+      const accessibleSet = new Set(accessible);
+      visible = attachments.filter(
+        (a) => a.pageId && accessibleSet.has(a.pageId),
      );
    }
+
+    return new Map(visible.map((a) => [a.id, a]));
  }

  async turnPageMentionsToLinks(
@@ -0,0 +1,67 @@
+import { resolve, sep } from 'path';
+import { LocalDriver } from './local.driver';
+
+type FullPath = (filePath: string) => string;
+
+describe('LocalDriver._fullPath', () => {
+  const ROOT = resolve('/data/storage');
+  const driver = new LocalDriver({ storagePath: ROOT });
+  const fullPath = ((driver as any)._fullPath as FullPath).bind(driver);
+
+  describe('legitimate inputs (behavior preserved)', () => {
+    it.each([
+      ['workspace-id/avatars/uuid.png', `${ROOT}${sep}workspace-id${sep}avatars${sep}uuid.png`],
+      ['workspace-id/files/uuid/file.pdf', `${ROOT}${sep}workspace-id${sep}files${sep}uuid${sep}file.pdf`],
+      ['a/b/c/d/e.bin', `${ROOT}${sep}a${sep}b${sep}c${sep}d${sep}e.bin`],
+      ['', ROOT],
+      ['.', ROOT],
+      ['./x/y.png', `${ROOT}${sep}x${sep}y.png`],
+      ['a//b', `${ROOT}${sep}a${sep}b`],
+      ['a/b/../c', `${ROOT}${sep}a${sep}c`],
+    ])('resolves %j to %j', (input, expected) => {
+      expect(fullPath(input)).toBe(expected);
+    });
+  });
+
+  describe('traversal rejected', () => {
+    it.each([
+      '../etc/passwd',
+      '../../../etc/passwd',
+      'workspace/../../../etc/passwd',
+      '..',
+      '../..',
+      'a/../../..',
+    ])('throws for %j', (input) => {
+      expect(() => fullPath(input)).toThrow('Invalid file path');
+    });
+  });
+
+  describe('absolute path rejected', () => {
+    it.each([
+      '/etc/passwd',
+      '/root/.ssh/id_rsa',
+      sep + 'absolute',
+    ])('throws for %j', (input) => {
+      expect(() => fullPath(input)).toThrow('Invalid file path');
+    });
+  });
+
+  describe('prefix-confusion rejected', () => {
+    it('rejects a sibling directory whose name starts with the storage root', () => {
+      const siblingDriver = new LocalDriver({ storagePath: '/data/storage' });
+      const siblingFullPath = ((siblingDriver as any)._fullPath as FullPath).bind(siblingDriver);
+      // Attempt to reach /data/storage-evil/secret by traversal:
+      // resolve('/data/storage', '../storage-evil/secret') === '/data/storage-evil/secret'
+      // Without the `+ sep` guard, a startsWith check would match.
+      expect(() => siblingFullPath('../storage-evil/secret')).toThrow('Invalid file path');
+    });
+  });
+
+  describe('storage root itself', () => {
+    it('accepts the root when input resolves to it', () => {
+      expect(fullPath('')).toBe(ROOT);
+      expect(fullPath('.')).toBe(ROOT);
+      expect(fullPath('a/..')).toBe(ROOT);
+    });
+  });
+});
@@ -3,7 +3,7 @@ import {
  LocalStorageConfig,
  StorageOption,
 } from '../interfaces';
-import { join, dirname } from 'path';
+import { dirname, resolve, sep } from 'path';
 import * as fs from 'fs-extra';
 import { Readable } from 'stream';
 import { createReadStream, createWriteStream } from 'node:fs';
@@ -17,7 +17,12 @@ export class LocalDriver implements StorageDriver {
  }

  private _fullPath(filePath: string): string {
-    return join(this.config.storagePath, filePath);
+    const storageRoot = resolve(this.config.storagePath);
+    const fullPath = resolve(storageRoot, filePath);
+    if (fullPath !== storageRoot && !fullPath.startsWith(storageRoot + sep)) {
+      throw new Error('Invalid file path');
+    }
+    return fullPath;
  }

  async upload(filePath: string, file: Buffer | Readable): Promise<void> {