From 8e165eb451a09312d5577615270e9be6e83d5972 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Thu, 30 Jul 2026 23:46:22 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20fix:=20Remove=20Owner=20Email=20?= =?UTF-8?q?from=20Agent=20`owner=5Fcontact`=20Fallback=20(#14541)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 🔒 fix: Remove Owner Email from Agent `owner_contact` Fallback The owner-contact fallback for agents without an explicit support_contact exposed the owner's private account email to any VIEW-level caller via GET /agents/:id and GET /agents. The fallback now resolves a display name only (name/username/authorName): the User query no longer projects email, the resolver never returns one, and the shared AgentOwnerContact type drops the field. Emails are only served when the owner opts in via support_contact. * 🔒 fix: Reject Email-Shaped Owner Display Names in Contact Fallback OpenID and SAML strategies fall back to the account email for the user's name and username when no display-name claims exist, so the name-only owner fallback could still surface the email through those fields. The resolver now rejects email-shaped display-name candidates entirely. * 🔒 fix: Treat Any @-Containing Display Name as Email-Derived RFC-5321 quoted local parts may contain whitespace and the User schema email validator is an unanchored substring match, so such addresses can reach the name/username fields via SSO fallbacks. Rejecting on '@' presence covers every legal email form without re-fetching the account email. --- api/server/controllers/agents/v1.spec.js | 37 +++++++---- api/server/services/Agents/ownerContact.js | 2 +- client/src/components/Agents/AgentContact.tsx | 5 +- .../Agents/tests/AgentCard.spec.tsx | 7 +- .../Agents/tests/AgentContact.spec.tsx | 13 +--- .../Agents/tests/AgentDetailContent.spec.tsx | 10 ++- .../__tests__/Landing.agent-contact.spec.tsx | 8 +-- packages/api/src/agents/contact.spec.ts | 66 ++++++++++++++----- packages/api/src/agents/contact.ts | 31 ++++++--- .../data-provider/src/types/assistants.ts | 1 - 10 files changed, 112 insertions(+), 68 deletions(-) diff --git a/api/server/controllers/agents/v1.spec.js b/api/server/controllers/agents/v1.spec.js index eecae7e537..bcababc9fe 100644 --- a/api/server/controllers/agents/v1.spec.js +++ b/api/server/controllers/agents/v1.spec.js @@ -603,10 +603,30 @@ describe('Agent Controllers - Mass Assignment Protection', () => { expect(mockRes.status).toHaveBeenCalledWith(200); const response = mockRes.json.mock.calls[0][0]; - expect(response.owner_contact).toEqual({ - name: 'Primary Owner', - email: 'primary.owner@example.com', + expect(response.owner_contact).toEqual({ name: 'Primary Owner' }); + expect(response.owner_contact).not.toHaveProperty('email'); + }); + + test('should omit owner_contact when the owner name and username are the account email', async () => { + const email = 'sso.owner@example.com'; + const owner = await createOwner({ name: email, username: email, email }); + const agent = await Agent.create({ + id: `agent_${uuidv4()}`, + name: 'SSO Owner Agent', + description: 'Owner has email-shaped name from SSO fallback', + provider: 'openai', + model: 'gpt-4', + author: owner._id, }); + await grantAgentOwner({ agent, owner }); + + mockReq.params = { id: agent.id }; + + await getAgentHandler(mockReq, mockRes); + + expect(mockRes.status).toHaveBeenCalledWith(200); + const response = mockRes.json.mock.calls[0][0]; + expect(response.owner_contact).toBeUndefined(); }); test('should not return owner_contact when support_contact is present', async () => { @@ -1556,10 +1576,8 @@ describe('Agent Controllers - Mass Assignment Protection', () => { await getListAgentsHandler(mockReq, mockRes); const response = mockRes.json.mock.calls[0][0]; - expect(response.data[0].owner_contact).toEqual({ - name: 'List Owner', - email: 'list.owner@example.com', - }); + expect(response.data[0].owner_contact).toEqual({ name: 'List Owner' }); + expect(response.data[0].owner_contact).not.toHaveProperty('email'); }); test('should use the first ACL owner when an agent has multiple owners', async () => { @@ -1589,10 +1607,7 @@ describe('Agent Controllers - Mass Assignment Protection', () => { await getListAgentsHandler(mockReq, mockRes); const response = mockRes.json.mock.calls[0][0]; - expect(response.data[0].owner_contact).toEqual({ - name: 'First Owner', - email: 'first.owner@example.com', - }); + expect(response.data[0].owner_contact).toEqual({ name: 'First Owner' }); }); test('should omit owner_contact when no owner user can be resolved', async () => { diff --git a/api/server/services/Agents/ownerContact.js b/api/server/services/Agents/ownerContact.js index f73e704276..61a9b2a2c6 100644 --- a/api/server/services/Agents/ownerContact.js +++ b/api/server/services/Agents/ownerContact.js @@ -59,7 +59,7 @@ const attachOwnerContacts = async (agents) => { let ownersById = new Map(); if (ownerIds.length > 0) { try { - const users = await db.findUsers({ _id: { $in: ownerIds } }, 'name username email'); + const users = await db.findUsers({ _id: { $in: ownerIds } }, 'name username'); ownersById = new Map(users.map((user) => [user?._id?.toString(), user])); } catch (error) { logger.warn('[/Agents] Failed to resolve agent owner users', error); diff --git a/client/src/components/Agents/AgentContact.tsx b/client/src/components/Agents/AgentContact.tsx index 73f8524165..34319a01eb 100644 --- a/client/src/components/Agents/AgentContact.tsx +++ b/client/src/components/Agents/AgentContact.tsx @@ -12,12 +12,11 @@ export default function AgentContact({ agent, className = '' }: AgentContactProp const supportName = agent?.support_contact?.name?.trim() ?? ''; const supportEmail = agent?.support_contact?.email?.trim() ?? ''; const ownerName = agent?.owner_contact?.name?.trim() ?? ''; - const ownerEmail = agent?.owner_contact?.email?.trim() ?? ''; let contact: { name: string; email: string } | null = null; if (supportName || supportEmail) { contact = { name: supportName, email: supportEmail }; - } else if (ownerName || ownerEmail) { - contact = { name: ownerName, email: ownerEmail }; + } else if (ownerName) { + contact = { name: ownerName, email: '' }; } const label = contact?.name || contact?.email || localize('com_agents_no_contact_available'); diff --git a/client/src/components/Agents/tests/AgentCard.spec.tsx b/client/src/components/Agents/tests/AgentCard.spec.tsx index 1d968f34ea..99395d80cb 100644 --- a/client/src/components/Agents/tests/AgentCard.spec.tsx +++ b/client/src/components/Agents/tests/AgentCard.spec.tsx @@ -324,7 +324,6 @@ describe('AgentCard', () => { authorName: 'John Doe', owner_contact: { name: 'Owner User', - email: 'owner@example.com', }, }; @@ -335,10 +334,8 @@ describe('AgentCard', () => { ); expect(screen.getByText('Contact:')).toBeInTheDocument(); - expect(screen.getByRole('link', { name: 'Owner User' })).toHaveAttribute( - 'href', - 'mailto:owner@example.com', - ); + expect(screen.getByText('Owner User')).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: 'Owner User' })).not.toBeInTheDocument(); expect(screen.queryByText('by John Doe')).not.toBeInTheDocument(); }); diff --git a/client/src/components/Agents/tests/AgentContact.spec.tsx b/client/src/components/Agents/tests/AgentContact.spec.tsx index 5225f6c85c..c5f2d72bda 100644 --- a/client/src/components/Agents/tests/AgentContact.spec.tsx +++ b/client/src/components/Agents/tests/AgentContact.spec.tsx @@ -24,7 +24,7 @@ describe('AgentContact', () => { agent={ { support_contact: { name: 'Support Team', email: 'support@example.com' }, - owner_contact: { name: 'Owner User', email: 'owner@example.com' }, + owner_contact: { name: 'Owner User' }, } as any } />, @@ -38,7 +38,7 @@ describe('AgentContact', () => { expect(screen.queryByText('Owner User')).not.toBeInTheDocument(); }); - it('falls back to owner contact', () => { + it('falls back to owner contact as a plain name without a mailto link', () => { render( { />, ); - expect(screen.getByRole('link', { name: 'Owner User' })).toHaveAttribute( - 'href', - 'mailto:owner@example.com', - ); - }); - - it('renders a plain name when no email is available', () => { - render(); - expect(screen.getByText('Owner User')).toBeInTheDocument(); expect(screen.queryByRole('link')).not.toBeInTheDocument(); }); diff --git a/client/src/components/Agents/tests/AgentDetailContent.spec.tsx b/client/src/components/Agents/tests/AgentDetailContent.spec.tsx index 19a2b5ef6d..a60b8dd870 100644 --- a/client/src/components/Agents/tests/AgentDetailContent.spec.tsx +++ b/client/src/components/Agents/tests/AgentDetailContent.spec.tsx @@ -101,7 +101,7 @@ describe('AgentDetailContent', () => { { ...baseAgent, support_contact: { name: 'Support Team', email: 'support@example.com' }, - owner_contact: { name: 'Owner User', email: 'owner@example.com' }, + owner_contact: { name: 'Owner User' }, } as any } />, @@ -121,15 +121,13 @@ describe('AgentDetailContent', () => { agent={ { ...baseAgent, - owner_contact: { name: 'Owner User', email: 'owner@example.com' }, + owner_contact: { name: 'Owner User' }, } as any } />, ); - expect(screen.getByRole('link', { name: 'Owner User' })).toHaveAttribute( - 'href', - 'mailto:owner@example.com', - ); + expect(screen.getByText('Owner User')).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: 'Owner User' })).not.toBeInTheDocument(); }); }); diff --git a/client/src/components/Chat/__tests__/Landing.agent-contact.spec.tsx b/client/src/components/Chat/__tests__/Landing.agent-contact.spec.tsx index d954c3c91b..367ffe012f 100644 --- a/client/src/components/Chat/__tests__/Landing.agent-contact.spec.tsx +++ b/client/src/components/Chat/__tests__/Landing.agent-contact.spec.tsx @@ -106,7 +106,7 @@ describe('Landing agent contact', () => { id: 'agent-1', name: 'Portal Remote Agent', description: 'Remote Agent Showcase', - owner_contact: { name: 'Owner User', email: 'owner@example.com' }, + owner_contact: { name: 'Owner User' }, }, }; @@ -115,10 +115,8 @@ describe('Landing agent contact', () => { expect(screen.getByText('Portal Remote Agent')).toBeInTheDocument(); expect(screen.getByText('Remote Agent Showcase')).toBeInTheDocument(); expect(screen.getByText('Contact:')).toBeInTheDocument(); - expect(screen.getByRole('link', { name: 'Owner User' })).toHaveAttribute( - 'href', - 'mailto:owner@example.com', - ); + expect(screen.getByText('Owner User')).toBeInTheDocument(); + expect(screen.queryByRole('link', { name: 'Owner User' })).not.toBeInTheDocument(); }); it('does not show contact when the selected agent is missing from agentsMap', () => { diff --git a/packages/api/src/agents/contact.spec.ts b/packages/api/src/agents/contact.spec.ts index 1ac5d81ae2..d8a1cd7ff3 100644 --- a/packages/api/src/agents/contact.spec.ts +++ b/packages/api/src/agents/contact.spec.ts @@ -1,10 +1,11 @@ +import type { AgentOwnerContactSource } from './contact'; import { resolveAgentOwnerContact } from './contact'; describe('resolveAgentOwnerContact', () => { it('omits owner fallback when support contact has a name', () => { const result = resolveAgentOwnerContact( { support_contact: { name: 'Support Team' } }, - { name: 'Agent Owner', email: 'owner@example.com' }, + { name: 'Agent Owner' }, ); expect(result).toBeUndefined(); @@ -13,19 +14,56 @@ describe('resolveAgentOwnerContact', () => { it('omits owner fallback when support contact has an email', () => { const result = resolveAgentOwnerContact( { support_contact: { email: 'support@example.com' } }, - { name: 'Agent Owner', email: 'owner@example.com' }, + { name: 'Agent Owner' }, ); expect(result).toBeUndefined(); }); - it('uses owner name and email when support contact is empty', () => { + it('uses owner name when support contact is empty', () => { const result = resolveAgentOwnerContact( { support_contact: { name: ' ', email: '' } }, - { name: ' Agent Owner ', email: ' owner@example.com ' }, + { name: ' Agent Owner ' }, ); - expect(result).toEqual({ name: 'Agent Owner', email: 'owner@example.com' }); + expect(result).toEqual({ name: 'Agent Owner' }); + }); + + it('never exposes an owner account email', () => { + const result = resolveAgentOwnerContact({}, { + name: 'Agent Owner', + email: 'owner.private@example.com', + } as AgentOwnerContactSource); + + expect(result).toEqual({ name: 'Agent Owner' }); + expect(result).not.toHaveProperty('email'); + }); + + it('skips email-shaped owner names from auth-strategy fallbacks', () => { + const result = resolveAgentOwnerContact( + {}, + { name: 'owner.private@example.com', username: 'owner.user' }, + ); + + expect(result).toEqual({ name: 'owner.user' }); + }); + + it('omits owner contact when every display-name candidate is email-shaped', () => { + const result = resolveAgentOwnerContact( + { authorName: 'owner.private@example.com' }, + { name: 'owner.private@example.com', username: 'owner.private@example.com' }, + ); + + expect(result).toBeUndefined(); + }); + + it('skips emails with quoted local parts containing whitespace', () => { + const result = resolveAgentOwnerContact( + {}, + { name: '"given family"@example.com', username: 'owner.user' }, + ); + + expect(result).toEqual({ name: 'owner.user' }); }); it('falls back to username for owner display name', () => { @@ -35,12 +73,9 @@ describe('resolveAgentOwnerContact', () => { }); it('falls back to authorName when owner has no display name', () => { - const result = resolveAgentOwnerContact( - { authorName: 'Legacy Author' }, - { email: 'owner@example.com' }, - ); + const result = resolveAgentOwnerContact({ authorName: 'Legacy Author' }, { name: '' }); - expect(result).toEqual({ name: 'Legacy Author', email: 'owner@example.com' }); + expect(result).toEqual({ name: 'Legacy Author' }); }); it('omits owner contact when no owner can be resolved', () => { @@ -49,11 +84,12 @@ describe('resolveAgentOwnerContact', () => { expect(result).toBeUndefined(); }); - it('omits owner contact when all candidate fields are empty', () => { - const result = resolveAgentOwnerContact( - { authorName: ' ' }, - { name: '', username: ' ', email: ' ' }, - ); + it('omits owner contact when no display name is available', () => { + const result = resolveAgentOwnerContact({ authorName: ' ' }, { + name: '', + username: ' ', + email: 'owner.private@example.com', + } as AgentOwnerContactSource); expect(result).toBeUndefined(); }); diff --git a/packages/api/src/agents/contact.ts b/packages/api/src/agents/contact.ts index e5a9824e0c..5a320f06d4 100644 --- a/packages/api/src/agents/contact.ts +++ b/packages/api/src/agents/contact.ts @@ -11,7 +11,6 @@ export interface AgentContactSource { export interface AgentOwnerContactSource { name?: string | null; username?: string | null; - email?: string | null; } const normalizeContactValue = (value?: string | null): string | undefined => { @@ -22,6 +21,17 @@ const normalizeContactValue = (value?: string | null): string | undefined => { return trimmed.length > 0 ? trimmed : undefined; }; +/** Auth strategies fall back to the account email for name/username, and legal + * email forms include quoted local parts with whitespace, so any '@'-containing + * value is treated as email-derived and never used as a public display name. */ +const normalizeDisplayName = (value?: string | null): string | undefined => { + const normalized = normalizeContactValue(value); + if (normalized == null || normalized.includes('@')) { + return undefined; + } + return normalized; +}; + export const hasSupportContact = (agent: AgentContactSource): boolean => { const support = agent.support_contact; if (!support) { @@ -30,6 +40,11 @@ export const hasSupportContact = (agent: AgentContactSource): boolean => { return !!normalizeContactValue(support.name) || !!normalizeContactValue(support.email); }; +/** + * Resolves a display-only owner contact for agents without an explicit support contact. + * Never includes the owner's account email, nor email-shaped display names; emails are + * only exposed when the owner opts in by configuring `support_contact`. + */ export function resolveAgentOwnerContact( agent: AgentContactSource, owner: AgentOwnerContactSource | null, @@ -39,17 +54,13 @@ export function resolveAgentOwnerContact( } const name = - normalizeContactValue(owner.name) ?? - normalizeContactValue(owner.username) ?? - normalizeContactValue(agent.authorName); - const email = normalizeContactValue(owner.email); + normalizeDisplayName(owner.name) ?? + normalizeDisplayName(owner.username) ?? + normalizeDisplayName(agent.authorName); - if (!name && !email) { + if (!name) { return undefined; } - return { - ...(name ? { name } : {}), - ...(email ? { email } : {}), - }; + return { name }; } diff --git a/packages/data-provider/src/types/assistants.ts b/packages/data-provider/src/types/assistants.ts index 0a84f21e3d..7f6710a705 100644 --- a/packages/data-provider/src/types/assistants.ts +++ b/packages/data-provider/src/types/assistants.ts @@ -211,7 +211,6 @@ export type SupportContact = { export type AgentOwnerContact = { name?: string; - email?: string; }; /**