From ace15dbdeb7ef835c3a00be9c3879689cc30c2f7 Mon Sep 17 00:00:00 2001
From: Ahmed Bouhuolia <a.bouhuolia@gmail.com>
Date: Sat, 16 May 2026 19:38:39 +0200
Subject: [PATCH] fix(server): use CSPRNG for attachment S3 keys
 (GHSA-gj48-p5ff-g67f)

The multer-s3 storage factory used `Date.now().toString()` as the S3 key
for every upload, yielding a 13-digit ms-epoch key. The keyspace for any
time window equals the millisecond count of that window, so an attacker
holding a registered account can enumerate keys for known upload moments
(e.g. ~10 minutes for a 10-second window with a 10-proxy rotation), then
download files via `GET /attachments/:id/presigned-url`. Two uploads in
the same millisecond also collide and silently overwrite each other.

Replace the key callback with `${organizationId}/${randomUUID()}`:

  * `randomUUID()` from `node:crypto` is a v4 UUID with 122 bits of
    entropy, making brute-force enumeration infeasible.
  * The `<organizationId>/` prefix (read from the `nestjs-cls` store
    populated by `ClsModule` middleware in `App.module.ts`) limits the
    blast radius of any hypothetical bucket-listing leak to a single
    tenant.

Add a tenant migration applying `unique` to `documents.key` so any future
key collision surfaces as a DB error instead of a silent S3 overwrite.

Legacy 13-digit numeric keys remain accessible via their stored values;
only new uploads use the new format.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
---
 ...000_add_unique_constraint_to_documents_key.ts | 11 +++++++++++
 .../src/modules/Attachments/Attachment.module.ts | 16 +++++++++++++---
 2 files changed, 24 insertions(+), 3 deletions(-)
 create mode 100644 packages/server/src/database/tenant/migrations/20260516120000_add_unique_constraint_to_documents_key.ts

diff --git a/packages/server/src/database/tenant/migrations/20260516120000_add_unique_constraint_to_documents_key.ts b/packages/server/src/database/tenant/migrations/20260516120000_add_unique_constraint_to_documents_key.ts
new file mode 100644
index 000000000..a4fda5c35
--- /dev/null
+++ b/packages/server/src/database/tenant/migrations/20260516120000_add_unique_constraint_to_documents_key.ts
@@ -0,0 +1,11 @@
+exports.up = function (knex) {
+  return knex.schema.alterTable('documents', (table) => {
+    table.unique('key');
+  });
+};
+
+exports.down = function (knex) {
+  return knex.schema.alterTable('documents', (table) => {
+    table.dropUnique('key');
+  });
+};
diff --git a/packages/server/src/modules/Attachments/Attachment.module.ts b/packages/server/src/modules/Attachments/Attachment.module.ts
index 08d3fad58..74f55f205 100644
--- a/packages/server/src/modules/Attachments/Attachment.module.ts
+++ b/packages/server/src/modules/Attachments/Attachment.module.ts
@@ -1,5 +1,7 @@
 import { Module } from "@nestjs/common";
+import { randomUUID } from 'node:crypto';
 import * as multerS3 from 'multer-s3';
+import { ClsService } from 'nestjs-cls';
 import { S3_CLIENT, S3Module } from "../S3/S3.module";
 import { DeleteAttachment } from "./DeleteAttachment";
 import { GetAttachment } from "./GetAttachment";
@@ -59,8 +61,12 @@ const models = [
     AttachmentUploadPipeline,
     {
       provide: MULTER_MODULE_OPTIONS,
-      inject: [ConfigService, S3_CLIENT],
-      useFactory: (configService: ConfigService, s3: S3Client) => ({
+      inject: [ConfigService, S3_CLIENT, ClsService],
+      useFactory: (
+        configService: ConfigService,
+        s3: S3Client,
+        cls: ClsService,
+      ) => ({
         storage: multerS3({
           s3,
           bucket: configService.get('s3.bucket'),
@@ -69,7 +75,11 @@ const models = [
             cb(null, { fieldName: file.fieldname });
           },
           key: function (req, file, cb) {
-            cb(null, Date.now().toString());
+            const organizationId = cls.get<string>('organizationId');
+            if (!organizationId) {
+              return cb(new Error('Tenant context required for upload.'), undefined as any);
+            }
+            cb(null, `${organizationId}/${randomUUID()}`);
           },
           acl: function(req, file, cb) {
             // Conditionally set file to public or private based on isPublic flag