Repository navigation
🔒 fix: Remove Owner Email from Agent owner_contact Fallback - #14541
Conversation
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.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 37fd5884b2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| 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'); |
There was a problem hiding this comment.
For OpenID users without name/username claims, openidStrategy.js:305,677-679 stores the account email in both name and username (SAML has equivalent fallbacks at samlStrategy.js:156,232-234). Keeping both fields in this projection therefore lets resolveAgentOwnerContact return that private email as owner_contact.name to VIEW/public-agent callers, so removing only the explicit email property does not close the reported disclosure. Retain enough information to reject display-name candidates equal to the account email, or otherwise exclude email-derived names/usernames.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid catch, thanks. Confirmed both fallbacks: openidStrategy getFullName returns userinfo.username || userinfo.email and username falls back to userinfo.email; samlStrategy has the equivalent getUserName(profile) || getEmail(profile) paths. Fixed in 1a40cbc: resolveAgentOwnerContact now rejects email-shaped display-name candidates (name, username, and legacy authorName) via a normalizeDisplayName helper, so an email can no longer surface through the name path regardless of which auth strategy stored it. This also covers stale emails left in name after an account email change, which an exact-match against the current account email would miss. Added resolver tests for the skip and all-candidates-email cases, plus an end-to-end controller test where an SSO-style owner (name = username = email) yields no owner_contact at all.
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.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1a40cbc693
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| email?: string | null; | ||
| } | ||
|
|
||
| const EMAIL_LIKE = /^[^\s@]+@[^\s@]+$/; |
There was a problem hiding this comment.
When an OpenID/SAML account lacks display claims and its account email does not match this narrow regex—for example, "given family"@example.com—the auth fallbacks copy that exact value into name and username (api/strategies/openidStrategy.js:305,677-693, with the SAML equivalent at api/strategies/samlStrategy.js:156,232-250). The User schema accepts this example because its validator is unanchored (packages/data-schemas/src/schema/user.ts:36-40), while EMAIL_LIKE fails to recognize values containing whitespace, causing normalizeDisplayName to return the private address to VIEW/public-agent callers. Fresh evidence in the updated code is this new shape heuristic; compare display candidates with the stored account email instead so every accepted email form is excluded.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Valid edge case, thanks; confirmed the User schema email validator is an unanchored substring match so quoted local parts with whitespace can be stored. Fixed in dd4bbd6, though with a broader guard than the suggested exact comparison: normalizeDisplayName now rejects any candidate containing '@'. Every legal email form contains '@', so this excludes all accepted email forms including quoted local parts, and it also covers cases exact-matching the stored account email would miss (a stale email left in name after the account email changes, or a different email used as username). The trade-off is suppressing rare legitimate '@'-containing display names, which fails privacy-safe for a fallback the owner never opted into. Added a resolver test for the \"given family\"@example.com form.
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.
|
@codex review |
|
Codex Review: Didn't find any major issues. More of your lovely PRs please. Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
…hat-AI#14541) * 🔒 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.
…hat-AI#14541) * 🔒 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.
1 participant
Summary
Resolves a validated security finding (medium): the agent owner-contact fallback added in #13663 leaked owner account emails to VIEW-level callers.
For agents without an explicit
support_contact,attachOwnerContactsresolved the first ACL owner (oragent.author), queried the User collection with aname username emailprojection, and servedowner_contact.emailin bothGET /api/agents/:idandGET /api/agentsresponses. Any authenticated user with VIEW access to a shared or public agent could read the owner's private account email, without the owner ever opting in.This PR makes the owner fallback display-name only. Emails are now only served when the owner explicitly configures a
support_contact. Defense in depth across three layers:packages/api/src/agents/contact.ts:resolveAgentOwnerContactresolves only a display name (owner name, then username, then legacyauthorName) and returnsundefinedwhen none exists.emailis removed fromAgentOwnerContactSource.api/server/services/Agents/ownerContact.js: the User query projection dropsemail, so the private address never enters the response-building path. AllattachOwnerContactscall sites (get, list, update, duplicate, revert, actions) flow through this.packages/data-provider/src/types/assistants.ts+client/src/components/Agents/AgentContact.tsx: the sharedAgentOwnerContacttype dropsemail, and the client renders the owner fallback as plain text with no mailto link, ignoring anemailfield even if a stale server still sends one.owner_contactis computed at response time and never persisted, so no stored data is affected.Change Type
Testing
cd packages/api && npx jest src/agents/contact.spec.ts: 8 tests pass, including a new case asserting an owner email fed into the resolver never appears in the output.cd api && npx jest server/controllers/agents/v1.spec.js --runInBand: all 87 tests pass; owner-contact expectations now assert name-only responses andnot.toHaveProperty('email')(real in-memory MongoDB, no mocked DB).cd client && npx jest AgentContact.spec AgentCard.spec AgentDetailContent.spec Landing.agent-contact: 25 tests pass; owner fallback renders a plain name without a mailto link.cd client && npx tsc --noEmit: clean after the shared type change.Checklist