💼 fix: Harden Shared-Link Message Sanitization with an Allowlist (#13510)

* fix: harden shared-link message sanitization with an allowlist

Public shared links built their message payload via `{ ...message }` and
`{ ...attachment }` spreads, which exposed internal fields that are never
needed by the shared view:

- message: endpoint, conversationSignature, clientId, plugin(s), metadata
- attachments: filepath, storageKey, metadata, and other internal keys

Replace the passthrough with an allowlist so only render-relevant fields
(sender, text, content, token/feedback/error flags, and sanitized
attachments) are surfaced. Assistant model ids remain anonymized; other
model names are omitted rather than disclosed.

Also add `tenantId?: string` to ISharedLink, matching the field already
read by the shared-link access middleware for multi-tenant deployments.

* fix: preserve shared render data; sanitize by denylist (review feedback)

Address Codex/Copilot review on the shared-link sanitization:

- The tight attachment allowlist dropped tool-call render data. Switch to a
  denylist of storage/identity-internal fields (filepath, storageKey, user,
  tenantId, source, metadata, …) so toolCallId, tool payloads (web_search /
  file_search / etc.), and dimensions are preserved while internals are stripped.
- Preserve user-uploaded message.files (previously dropped entirely) via the
  same denylist sanitizer, so shared links keep uploaded images/documents.
- Introduce a dedicated SharedMessage / SharedFile type and use it for
  SharedMessagesResult.messages instead of casting the allowlisted object to
  IMessage, so omitted fields are caught at compile time.

Extends the regression test to assert toolCallId, web_search payload, and
files survive while filepath/storageKey/user/tenantId/metadata are removed.

* fix: keep render URLs + skill badges, drop private feedback (review round 2)

Address Codex round-2 findings on shared-link sanitization:

- filepath is the URL the share renderer loads (Files.tsx uses
  file.preview ?? file.filepath; image attachments render only when filepath
  is set). Drop it from the denylist so shared images/downloads still render;
  storageKey (the raw object key) stays stripped.
- Preserve manualSkills / alwaysAppliedSkills so SkillPills still render for
  skill-assisted turns (non-sensitive UI metadata).
- Remove feedback from the shared projection — it is the owner's private
  rating/notes, is never rendered in the share view, and must not be exposed
  to anyone holding the share URL.

Regression test updated to assert filepath/skills survive and feedback is
omitted.

* fix: anonymize file ids + preserve message iconURL (review round 3)

- Persisted message.files records can carry the original conversationId/messageId.
  Attachments were already rewritten to the anonymized ids; apply the same
  rewrite to files so shared user-uploaded files don't expose the real ids.
- Preserve message.iconURL (read by Share/MessageIcon.tsx) so shared assistant/
  custom-endpoint turns keep their custom avatar instead of falling back to the
  generic icon.

Regression test asserts the shared file's conversationId is the anonymized id
and that iconURL survives.

packages/data-schemas/src/methods/share.test.ts

@@ -40,8 +40,18 @@ describe('Share Methods', () => {
         text: String,
         isCreatedByUser: Boolean,
         model: String,
+        iconURL: String,
+        endpoint: String,
+        conversationSignature: String,
+        clientId: String,
+        plugin: mongoose.Schema.Types.Mixed,
+        metadata: mongoose.Schema.Types.Mixed,
+        feedback: mongoose.Schema.Types.Mixed,
+        manualSkills: [String],
+        alwaysAppliedSkills: [String],
         parentMessageId: String,
         attachments: [mongoose.Schema.Types.Mixed],
+        files: [mongoose.Schema.Types.Mixed],
         content: [mongoose.Schema.Types.Mixed],
       },
       { timestamps: true },
@@ -340,7 +350,7 @@ describe('Share Methods', () => {
         expect(msg.messageId).toMatch(/^msg_/); // Should be anonymized with msg_ prefix
         expect(msg.messageId).not.toBe(messages[0].messageId); // Should be different from original
         expect(msg.conversationId).toBe(result.conversationId);
-        expect(msg.user).toBeUndefined(); // User should be removed
+        expect((msg as Record<string, unknown>).user).toBeUndefined(); // User should be removed
       });
     });
 
@@ -400,6 +410,106 @@ describe('Share Methods', () => {
         (result?.messages[0].attachments?.[0] as unknown as t.IMessage | undefined)?.conversationId,
       ).toBe(result?.conversationId);
     });
+
+    test('strips storage-internal fields while preserving shared render data', async () => {
+      const userId = new mongoose.Types.ObjectId().toString();
+      const conversationId = `conv_${nanoid()}`;
+      const shareId = `share_${nanoid()}`;
+
+      const message = await Message.create({
+        messageId: `msg_${nanoid()}`,
+        conversationId,
+        user: userId,
+        text: 'safe text',
+        isCreatedByUser: false,
+        model: 'gpt-4',
+        iconURL: 'https://cdn.example.com/icon.png',
+        endpoint: 'openAI',
+        conversationSignature: 'signature',
+        clientId: 'client-id',
+        plugin: { latest: 'internal' },
+        metadata: { codeEnvRef: 'internal-ref' },
+        feedback: { rating: 'thumbsDown', tag: { key: 'inaccurate' }, text: 'private note' },
+        manualSkills: ['research'],
+        alwaysAppliedSkills: ['brand-voice'],
+        files: [
+          {
+            file_id: 'file123',
+            filename: 'upload.png',
+            type: 'image/png',
+            width: 100,
+            height: 100,
+            filepath: '/images/upload.png',
+            conversationId,
+            user: userId,
+            tenantId: 'tenant-a',
+            storageKey: 'private/upload.png',
+            source: 's3',
+          },
+        ],
+        attachments: [
+          {
+            toolCallId: 'call_abc',
+            type: 'web_search',
+            web_search: { results: [{ title: 'Cited source', link: 'https://example.com' }] },
+            filename: 'result.json',
+            filepath: '/images/result.json',
+            storageKey: 'private/result.json',
+            metadata: { codeEnvRef: 'internal-ref' },
+          },
+        ],
+      });
+
+      await SharedLink.create({
+        shareId,
+        conversationId,
+        user: userId,
+        messages: [message._id],
+      });
+
+      const result = await shareMethods.getSharedMessages(shareId);
+      const shared = result?.messages[0];
+
+      expect(shared?.text).toBe('safe text');
+      // Custom message icon is render metadata and should be preserved.
+      expect(shared?.iconURL).toBe('https://cdn.example.com/icon.png');
+      // Non-assistant model and internal message fields must not be disclosed.
+      expect(shared?.model).toBeUndefined();
+      expect(shared).not.toHaveProperty('endpoint');
+      expect(shared).not.toHaveProperty('conversationSignature');
+      expect(shared).not.toHaveProperty('clientId');
+      expect(shared).not.toHaveProperty('plugin');
+      expect(shared).not.toHaveProperty('metadata');
+      // Private owner feedback is not render data and must not leak publicly.
+      expect(shared).not.toHaveProperty('feedback');
+      // Skill badges are non-sensitive UI metadata and should still render.
+      expect(shared?.manualSkills).toEqual(['research']);
+      expect(shared?.alwaysAppliedSkills).toEqual(['brand-voice']);
+
+      // User-uploaded files keep their render URL (filepath/preview) but drop storage internals.
+      const file = shared?.files?.[0];
+      expect(file).toMatchObject({ filename: 'upload.png', type: 'image/png' });
+      expect(file?.filepath).toBe('/images/upload.png');
+      expect(file).not.toHaveProperty('storageKey');
+      expect(file).not.toHaveProperty('user');
+      expect(file).not.toHaveProperty('tenantId');
+      expect(file).not.toHaveProperty('source');
+      // The file's conversation id is rewritten to the anonymized id, not the original.
+      expect(file?.conversationId).toBe(shared?.conversationId);
+      expect(file?.conversationId).not.toBe(conversationId);
+
+      // Tool-call attachments keep their correlation id, payload, and render URL so
+      // citations still render, while storage-only fields are removed.
+      const attachment = shared?.attachments?.[0];
+      expect(attachment).toMatchObject({
+        toolCallId: 'call_abc',
+        type: 'web_search',
+        web_search: { results: [{ title: 'Cited source', link: 'https://example.com' }] },
+        filepath: '/images/result.json',
+      });
+      expect(attachment).not.toHaveProperty('storageKey');
+      expect(attachment).not.toHaveProperty('metadata');
+    });
   });
 
   describe('getSharedLinks', () => {

packages/data-schemas/src/methods/share.ts

@@ -42,7 +42,82 @@ function anonymizeConvo(conversation: Partial<t.IConversation> & Partial<t.IShar
   return newConvo;
 }
 
-function anonymizeMessages(messages: t.IMessage[], newConvoId: string): t.IMessage[] {
+/**
+ * Storage- and identity-internal fields that must never be exposed through a
+ * public shared link. Everything else on a file/attachment — including the
+ * `filepath`/`preview` render URLs, dimensions, and tool-call payloads such as
+ * `toolCallId` and search results — is render data the shared view needs, so it
+ * is preserved. (`storageKey` is the raw object key and is dropped; `filepath`
+ * is the URL the share renderer actually loads, so it is kept.)
+ */
+const SENSITIVE_SHARED_FILE_FIELDS = new Set([
+  '_id',
+  '__v',
+  'user',
+  'tenantId',
+  'storageRegion',
+  'storageKey',
+  'temp_file_id',
+  'message',
+  'source',
+  'filterSource',
+  'context',
+  'embedded',
+  'usage',
+  'metadata',
+]);
+
+/**
+ * Strip storage/identity-internal fields from a file or attachment while keeping
+ * render-relevant data (including tool-call payloads keyed by tool name).
+ */
+function sanitizeSharedFile(value: unknown): t.SharedFile | null {
+  if (!value || typeof value !== 'object' || Array.isArray(value)) {
+    return null;
+  }
+
+  const result: t.SharedFile = {};
+  for (const [key, fieldValue] of Object.entries(value as Record<string, unknown>)) {
+    if (!SENSITIVE_SHARED_FILE_FIELDS.has(key)) {
+      result[key] = fieldValue;
+    }
+  }
+
+  return Object.keys(result).length > 0 ? result : null;
+}
+
+function sanitizeSharedFiles(files: unknown): t.SharedFile[] | undefined {
+  if (!Array.isArray(files)) {
+    return undefined;
+  }
+
+  const sanitized = files
+    .map(sanitizeSharedFile)
+    .filter((file): file is t.SharedFile => file != null);
+
+  return sanitized.length > 0 ? sanitized : undefined;
+}
+
+/**
+ * Only surface a model name when it is an (already-anonymized) assistant id;
+ * otherwise omit it so the underlying provider/model is not disclosed.
+ */
+function anonymizeSharedModel(model?: string): string | undefined {
+  if (!model?.startsWith('asst_')) {
+    return undefined;
+  }
+  return anonymizeAssistantId(model);
+}
+
+/**
+ * Build the public, anonymized view of shared messages. An allowlist of
+ * render-relevant fields keeps internal message fields (endpoint,
+ * conversationSignature, clientId, plugin(s), metadata, etc.) out of the
+ * payload, while user files and tool-call attachments are sanitized field by
+ * field so render data (uploaded files, `toolCallId`, search results, generated
+ * outputs) is preserved without leaking storage internals.
+ */
+function anonymizeMessages(messages: t.IMessage[], newConvoId: string): t.SharedMessage[] {
   if (!Array.isArray(messages)) {
     return [];
   }
@@ -52,34 +127,43 @@ function anonymizeMessages(messages: t.IMessage[], newConvoId: string): t.IMessa
     const newMessageId = anonymizeMessageId(message.messageId);
     idMap.set(message.messageId, newMessageId);
 
-    type MessageAttachment = {
-      messageId?: string;
-      conversationId?: string;
-      [key: string]: unknown;
-    };
-
-    const anonymizedAttachments = (message.attachments as MessageAttachment[])?.map(
-      (attachment) => {
-        return {
-          ...attachment,
-          messageId: newMessageId,
-          conversationId: newConvoId,
-        };
-      },
-    );
+    const attachments = sanitizeSharedFiles(message.attachments)?.map((attachment) => ({
+      ...attachment,
+      messageId: newMessageId,
+      conversationId: newConvoId,
+    }));
+    // Persisted file records can carry the original conversation/message ids;
+    // rewrite them to the anonymized ids so shared files don't expose them.
+    const files = sanitizeSharedFiles(message.files)?.map((file) => ({
+      ...file,
+      ...(file.conversationId !== undefined && { conversationId: newConvoId }),
+      ...(file.messageId !== undefined && { messageId: newMessageId }),
+    }));
+    const model = anonymizeSharedModel(message.model);
 
     return {
-      ...message,
       messageId: newMessageId,
       parentMessageId:
         idMap.get(message.parentMessageId || '') ||
         anonymizeMessageId(message.parentMessageId || ''),
       conversationId: newConvoId,
-      model: message.model?.startsWith('asst_')
-        ? anonymizeAssistantId(message.model)
-        : message.model,
-      attachments: anonymizedAttachments,
-    } as t.IMessage;
+      sender: message.sender,
+      text: message.text,
+      content: message.content,
+      ...(message.iconURL && { iconURL: message.iconURL }),
+      ...(model && { model }),
+      isCreatedByUser: message.isCreatedByUser,
+      createdAt: message.createdAt,
+      updatedAt: message.updatedAt,
+      tokenCount: message.tokenCount,
+      unfinished: message.unfinished,
+      error: message.error,
+      finish_reason: message.finish_reason,
+      ...(message.manualSkills && { manualSkills: message.manualSkills }),
+      ...(message.alwaysAppliedSkills && { alwaysAppliedSkills: message.alwaysAppliedSkills }),
+      ...(files && { files }),
+      ...(attachments && { attachments }),
+    };
   });
 }
 

packages/data-schemas/src/types/share.ts

@@ -12,12 +12,50 @@ export interface ISharedLink {
   expiredAt?: Date;
   createdAt?: Date;
   updatedAt?: Date;
+  /** Owning tenant for multi-tenant deployments (read by the shared-link access middleware). */
+  tenantId?: string;
 }
 
 export interface ShareServiceError extends Error {
   code: string;
 }
 
+/**
+ * A file or attachment as exposed through a public shared link: storage- and
+ * identity-internal fields are stripped, but render-relevant data (including
+ * dynamic tool-call payloads keyed by tool name) is preserved.
+ */
+export type SharedFile = Record<string, unknown>;
+
+/**
+ * Public, anonymized projection of a message returned by a shared link. Only
+ * render-relevant fields are surfaced; internal fields (user, endpoint,
+ * conversationSignature, clientId, plugin(s), metadata, etc.) are omitted.
+ */
+export type SharedMessage = Pick<
+  IMessage,
+  | 'messageId'
+  | 'parentMessageId'
+  | 'conversationId'
+  | 'sender'
+  | 'text'
+  | 'content'
+  | 'iconURL'
+  | 'isCreatedByUser'
+  | 'createdAt'
+  | 'updatedAt'
+  | 'tokenCount'
+  | 'unfinished'
+  | 'error'
+  | 'finish_reason'
+  | 'manualSkills'
+  | 'alwaysAppliedSkills'
+> & {
+  model?: string;
+  files?: SharedFile[];
+  attachments?: SharedFile[];
+};
+
 export interface SharedLinksResult {
   links: Array<{
     shareId: string;
@@ -31,7 +69,7 @@ export interface SharedLinksResult {
 
 export interface SharedMessagesResult {
   conversationId: string;
-  messages: Array<IMessage>;
+  messages: Array<SharedMessage>;
   shareId: string;
   title?: string;
   createdAt?: Date;