feat(slack): render Twenty record links as work object previews - #24980
feat(slack): render Twenty record links as work object previews#24980abdulrahmancodes wants to merge 40 commits into
Conversation
Pasting a person, company, opportunity, note or task link in Slack now unfurls it as a work object card (title, display type, a few headline fields) via chat.unfurl entity metadata. - link_shared routes through the events resolver to a new slack-link-unfurl logic function - previews render only when the poster maps to a workspace member, via the same Slack-account matching the assistant uses; records are read with the function role, which gains read-only access to the five CRM objects - manifest adds the links:read scope, the link_shared event and an unfurl domain placeholder; SETUP.md documents the unfurl domain, the Work Object Previews toggle and the reconnect on upgrade
|
👋 Thanks for contributing to Twenty! Your PR has been set to draft while you work on it. Once you're done, mark it as Ready for review and our automated checks will run. Looking forward to your contribution! |
The link preview feature declared links:read only in the app manifest. The Twenty connection OAuth flow re-grants the bot token with the connection provider's scope list, so reconnecting stripped links:read and Slack stopped delivering link_shared events. chat.unfurl also requires links:write, which was requested nowhere. Add links:read and links:write to the connection provider scopes and links:write to the manifest.
Greptile SummaryThe PR adds Slack Work Object previews for links to supported Twenty records and supplies matching flexpane details.
Confidence Score: 5/5The PR appears safe to merge because no blocking failure remains within the eligible follow-up-review scope. No blocking failure remains. Important Files Changed
Sequence DiagramsequenceDiagram
participant U as Slack user
participant S as Slack
participant R as Events resolver
participant L as Link unfurl logic
participant T as Twenty Core API
U->>S: Share Twenty record URL
S->>R: link_shared
R->>L: Dispatch into resolved workspace
L->>L: Validate poster and parse links
L->>T: Query supported records
T-->>L: Selected record fields
L->>S: chat.unfurl with entity metadata
U->>S: Expand preview
S->>R: entity_details_requested
R->>L: Dispatch entity-details handler
L->>T: Resolve and query record
L->>S: entity.presentDetails
Reviews (2): Last reviewed commit: "feat(slack): record icons on every card ..." | Re-trigger Greptile |
🤖 PR Review
🛡️ Security Review✅ No high-severity vulnerabilities detected. Summary
🚦 Auto-approve🙋 Manual review recommended for the following reason(s):
Automated pre-review — human approval still required. |
✅ Quality review · no findings
High-level — Additive Slack 0.7.0 record-link-preview feature (two events, two OAuth scopes, five read-only object permissions) reusing the existing event-router and member-resolution seams with no migration and a documented reconnect/upgrade path; the newest composer-source link_shared branch reuses the same parse/fetch/build path, and the size and record-level-permission concerns are already escalated to @twentyhq/security Reviewed against the |
twenty-eng-sync
Bot
added
the
-PR: stale auto-closed
|
Auto-closed: this PR is draft and has had no update in over 24 hours. Please reopen it once you have the bandwidth to take it forward. |
- product_icon on every card points at the public Twenty logo (Slack fetches the icon itself, so it cannot come from the workspace's own, possibly private, instance) - the person card gains a Company field as an entity_ref linking to the company's record page
Expanding a record card fired entity_details_requested, which nothing answered, so Slack showed a hard error panel. A new slack-entity-details logic function answers it via entity.presentDetails with the same work object metadata the unfurl builds; unresolvable or missing records get a clean error state instead of Slack's generic one. The record is resolved from the event's entity URL or external_ref, and external_ref now carries the object name as its type so the request is self-describing. The event's exact schema is still young, so an unrecognized shape is logged verbatim for diagnosis. Adds the entity_details_requested bot event to the manifest and docs; no new scopes.
Company cards show the company favicon from twenty-icons.com (the same source the app UI uses), falling back to the Twenty mark when there is no domain; the person card's company ref carries the same icon.
- person cards use the person's avatar when avatarUrl is a public absolute URL; instance-hosted avatars sit behind signed URLs Slack cannot fetch, so those keep the Twenty mark - opportunity cards use their company's logo and gain a clickable Company field, matching the person card - the company-ref field building is shared between person and opportunity
abdulrahmancodes
removed
the
-PR: stale auto-closed
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
![twenty-ci-bot-public[bot]](https://avatars.githubusercontent.com/u/119600397?s=60&v=4){kind=small}
Slack never fires link_shared for an app's own messages, so record links in assistant answers and workflow message posts get their work object metadata attached at post time in postSlackMessage, the single chat.postMessage call site. URLs are extracted from the outgoing text (bare, markdown and mrkdwn forms), capped like the unfurl path, and any failure means no preview, never a failed post. The record fetch loop is now shared between the unfurl handler and the outgoing path.
The flexpane reused the card's headline fields, so expanding a card added nothing. Content builders now have a detail tier the flexpane requests: LinkedIn, created/updated on people; country, annual revenue, account owner, LinkedIn on companies; point of contact on opportunities; assignee and a body preview on tasks; body preview on notes. Also drops company.employees and person.city from the queries: both are dev-seed custom fields, not standard, and one unknown field fails the whole record query on workspaces that lack it.
Slack stamps the Slack app's own icon on the corner of every record card; without an uploaded icon that badge is Slack's generic placeholder, and icons cannot be set from the manifest.
| // permissions, workflow steps post what their author configured), so there | ||
| // is no poster gate here. Best-effort: any failure means no preview, never | ||
| // a failed post. | ||
| export const buildSlackRecordEntitiesForMessage = async ( |
There was a problem hiding this comment.
🟡 Nit · High-level · build strategy
PR is ~1100 non-test code lines across 25 files, over the ≤1000 one-change guideline
The whole feature is hard to hold in one review pass; the outbound path (build-slack-record-entities-for-message + the post-slack-message wiring) is independently mergeable from the inbound link_shared/entity_details path. Consider landing the inbound previews first and the outgoing-message attachment as a follow-up PR.
There was a problem hiding this comment.
Keeping this as one PR, deliberately: the outbound attachment is a ~60-line consumer of the same parser, fetcher and entity builders the inbound path defines — splitting it out would ship those shared utils in PR one with a single consumer and re-review them in PR two, and the permission story (poster gate inbound, no gate outbound, viewer gate on the flexpane) only makes sense reviewed as a whole. The line count is also inflated by the split into one-export-per-file helpers, which are individually small and now unit-tested (244 tests). Happy to split if a human reviewer prefers it.
Generated by Claude Code
…urlRecord Addresses review nits: coercion/format helpers, custom-field helpers and the per-object content builders move to their own utils, bringing every file under the 300-line guideline, and findSlackUnfurlRecord takes a single object argument like its sibling utils.
The derived SlackUnfurlObjectName type moves to its own type file alongside the app's other .type.ts files, leaving the constants file with just the const.
| } | ||
|
|
||
| const slackClient = slackClientResult.client; | ||
| const client = new CoreApiClient(); |
There was a problem hiding this comment.
Bug: The Slack record unfurling logic uses application-level permissions, bypassing the user's specific record-level access rights and potentially leaking data.
Severity: HIGH
Suggested Fix
Instantiate the CoreApiClient with user-scoped permissions. This likely involves configuring the client with an option like runAs: 'user' and passing the resolved workspace member's ID to ensure all data fetching respects that member's individual access controls.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location:
packages/twenty-apps/public/slack/src/logic-functions/utils/unfurl-slack-record-links.ts#L56
Potential issue: In `unfurl-slack-record-links.ts` and
`present-slack-record-details.ts`, the `CoreApiClient` is instantiated with default
application-level permissions. The logic verifies that a user is a member of the
workspace but does not enforce the specific user's record-level permissions when
fetching data. This allows a workspace member to unfurl and view details of any record
by pasting its URL in Slack, even if they do not have read access to that specific
record in Twenty, leading to a potential information leak.
Also affects:
packages/twenty-apps/public/slack/src/logic-functions/utils/present-slack-record-details.ts:83~148
There was a problem hiding this comment.
The exposure is real, so flagging it for a human decision rather than closing this thread. The suggested fix is not available though, and it is worth being precise about why.
TwentyClientRunAs is 'user' | 'application' — it takes no member id. A logic function triggered by a Slack webhook has no user context to resolve 'user' against, and only agents can bind a member (runAgent's runAsWorkspaceMemberId) or a role (roleUniversalIdentifier). So a CoreApiClient inside this function cannot be scoped to the poster; it necessarily reads with the app's function role.
What that means concretely, having checked how permissions are evaluated: both object permissions and row-level predicates are resolved per role (build-row-level-permission-record-filter.util.ts filters predicates by predicate.roleId). This PR's function role gets read on person/company/opportunity/note/task with no row-level predicates, so a Slack user who maps to any workspace member can surface headline fields of any record of those five types, even where their own role denies the object outright or narrows it with row-level predicates. The member gate proves workspace membership, not record access, and the PR body says as much.
Options, none of which I want to pick unilaterally on a security question:
- Ship as-is, documented — defensible only if the previews are considered workspace-wide-readable data.
- Narrow the card to the label identifier the URL already reveals; still leaks existence and name.
- Route the fetch through an agent bound to
runAsWorkspaceMemberId, which is the only member-scoped path that exists today — heavyweight for a card render. - Add a member-scoped read to the SDK (a
runAsWorkspaceMemberIdon the client) and gate this feature on it.
My preference is 4 as the correct long-term fix with 1 or 2 as the interim, but this is the author's and @twentyhq/security's call, not mine.
Generated by Claude Code
Two spellings of the same record link (trailing slash, query params, or the HTML-escaped ampersands Slack sends) each produced their own card and their own record fetch, because dedup ran on the raw URL before the parser normalized it. Dedup now keys on the parsed object name and record id, so one record yields one card however it was written.
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
3 issues found and verified against the latest diff
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/twenty-apps/public/slack/src/logic-functions/utils/extract-http-urls.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/extract-http-urls.ts:2">
P3: The URL character class `[^\s<>|)\]]+` lets unbalanced delimiters leak into matches while only trailing `.,;:!?` is stripped. A URL that legitimately contains parens is truncated at the first `)` (e.g. `https://example.com/page(2)` becomes `https://example.com/page(2`), and a URL wrapped in quotes or Slack mrkdwn emphasis keeps its closing delimiter (a `"https://example.com/x"` message yields `https://example.com/x"`; `*https://example.com/x*` yields `https://example.com/x*`). These malformed URLs then fail the base-URL match in `parseTwentyRecordLinks`, so the unfurl silently does not render. Exclude `(`, `[`, `'`, `"`, `*`, `_` from the match, or apply a balanced-close pass, so boundaries are detected via closing delimiters rather than only sentence punctuation.</violation>
<violation number="2" location="packages/twenty-apps/public/slack/src/logic-functions/utils/extract-http-urls.ts:3">
P2: The trailing-punctuation strip drops `?`, `!`, `.`, `:`, `;` whenever they appear at the end of a match, even when they are legitimate URL query/fragment characters (e.g. `?q=hi!` becomes `?q=hi`). The extracted string is reused as the chat.unfurl key and entity `url`, so a corrupted URL will not match the link Slack recorded and the preview silently fails. Guard the strip so it only removes punctuation that is actually a sentence/inline delimiter rather than part of the URL.</violation>
</file>
<file name="packages/twenty-apps/public/slack/src/logic-functions/utils/present-slack-record-details.ts">
<violation number="1" location="packages/twenty-apps/public/slack/src/logic-functions/utils/present-slack-record-details.ts:90">
P2: The flexpane viewer gate verifies only that the Slack user maps to a workspace member, but the record is fetched with a fresh app-context CoreApiClient and its full details (`includeDetails: true`) are presented to that viewer via `entity.presentDetails` with no per-object or per-record read-permission check. A member who lacks access to a specific record in Twenty can still see its full details in the flexpane. Enforce the viewer's read permission on the resolved object/record before presenting details, or scope the fetch to the viewer.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
Its one caller now guards the amount with isNumber directly.
- a record preview Slack refuses no longer fails the message: the post retries once without entity metadata, restoring the documented never-a-failed-post guarantee - the entity carries a canonical record URL while app_unfurl_url keeps the exact URL Slack saw, so query strings and fragments no longer leak into the card link - records for a multi-link message are fetched concurrently instead of serially, which kept previews behind the slowest fetch - toAbsoluteHttpUrl rejects non-http(s) schemes rather than fabricating https://mailto:a@b.c, while still treating a colon before digits as a port - company logo and avatar scheme checks are case-insensitive - a malformed link_shared payload is skipped instead of throwing - body previews truncate by code point, so an emoji at the boundary is not split - amounts use the currency's own minor-unit digits - the flexpane distinguishes an unnameable record from a missing one - isSupportedObjectName drops its widening cast
The person and company content builders carried a verbatim copy of the same LinkedIn field block.
- URL matching excludes wrapping delimiters (quotes, parens, brackets, mrkdwn emphasis) rather than letting them leak into the match, and no longer strips a trailing ? or !, which are legitimate URL characters - outgoing messages only query the workspace URL once the text actually carries an /object/ link, so unrelated links add no latency
The entity-stripping retry fired on any failure, so a channel_not_found or an expired token cost a second doomed call. The Slack error code is now captured where it is thrown and the retry is gated on it.
There was a problem hiding this comment.
All reported issues were addressed across 22 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Dropping ? and ! from the trailing strip protected query strings but regressed the common case: a bare record link ending a sentence kept the punctuation, so the record id failed its UUID check and the preview was silently skipped. The strip now removes them only when the URL carries no query or fragment for them to belong to.
…ect-previews-bg8fov # Conflicts: # packages/twenty-apps/public/slack/package.json
| id: true, | ||
| name: { firstName: true, lastName: true }, | ||
| jobTitle: true, | ||
| avatarUrl: true, |
There was a problem hiding this comment.
avatarUrl is deprecated and 2.22 moved person avatars to avatarFile, does the person icon still resolve on a workspace that is not dev seeded?
There was a problem hiding this comment.
Largely it does not, and I would rather say so plainly than leave it implied: on a workspace that is not dev-seeded, most people will have no icon and the card falls back to the app's product icon.
Checked it through. avatarUrl is @deprecated Use avatarFile field instead on PersonWorkspaceEntity, so an avatar set through the UI today lands in avatarFile and avatarUrl stays empty; getPublicAvatarUrl then returns undefined and the card renders without a record icon. It still resolves where avatarUrl holds an external URL (dev seeds, imports, enrichment), which is why it looked fine in my testing.
Switching the selection to avatarFile would not fix it, and that is the part worth flagging: Slack fetches card icons from its own servers, so the URL has to be publicly reachable. avatarFile is an instance-hosted file behind a signed URL — getRecordImageIdentifier has to call signUrl for exactly this class of image — and a short-lived signed URL is both unfetchable by Slack in the general case and not something I would want to hand out to a channel. The same reasoning is why the PR deliberately skips instance-hosted avatars rather than passing them through.
So the options as I see them:
- Leave as-is: best-effort icon when
avatarUrlhappens to be external, product icon otherwise. Costs one deprecated field in the selection. - Drop the person icon entirely and always use the product icon — honest, and removes the deprecated field, at the cost of the enriched-workspace case that does work today.
- Serve a public, cacheable avatar URL for
avatarFile(a new unauthenticated endpoint or a signed URL with a long TTL) and use that. Correct, but well outside this PR.
I have left it as 1 for now since it strictly adds an icon where one is resolvable and never shows a wrong one. Happy to switch to 2 if you would rather not carry the deprecated field — say the word and it is a two-line change.
Generated by Claude Code
| await presentDetailsError({ | ||
| slackClient, | ||
| triggerId, | ||
| message: 'Record details are only available to Twenty workspace members.', |
There was a problem hiding this comment.
presentDetails takes user_auth_required and user_auth_url, which would point an unlinked viewer at the consent flow instead of a dead end, worth using here?
There was a problem hiding this comment.
Worth using, yes — an unlinked viewer currently gets a message with no way forward, even though the app has a consent flow waiting for them. EntityPresentDetailsArguments in @slack/web-api 7.19 takes user_auth_required and user_auth_url, and the app can resolve its own per-workspace id via findManyApplications (matching on the APPLICATION_UNIVERSAL_IDENTIFIER it already holds) to build {workspaceUrl}/settings/applications/{id}. One extra query, on a deliberate user action rather than the message-post path. The one thing I'd verify at runtime is whether the app's function role may call it.
The open question is which viewers should see the button. The gate can't separate two very different people, since both simply fail to resolve to a member: a colleague who has a Twenty account but hasn't linked Slack, for whom the button is exactly right; and an external Slack Connect guest with no account at all, for whom it's a login wall for a workspace they can't join, plus it surfaces the workspace URL to an outsider. Same button for both. Tell me which behaviour you want and I'll implement it here.
Generated by Claude Code
…etails Match record links against both the custom domain and the subdomain host, so a link copied from either unfurls, and keep the canonical entity URL on the first. The same list now drives the instance-hosted avatar check. Move a person's email and phone from the channel-visible card into the member-gated flexpane, drop the duplicate asNonEmptyString and asObject in favour of the app's readOptionalString and asRecord, and point setup step 1 at the unfurl-domain placeholder.
Slack exposes features.rich_previews in the app manifest, so the toggle and the item entity type no longer have to be switched on by hand for an app created from it. Missing that toggle rendered no card at all with nothing logged, which made it an easy step to lose.
|
Found while auditing the manifest for other missing keys: composer-sourced Slack fires The two payloads differ in shape. Per the Composer unfurls section:
The composer event carries Fixing the parse alone isn't enough.
So it's three small changes: declare the fields on The member gate is unaffected — the composer payload still carries Not a regression, and it fails in the silent-skip way the rest of the pipeline is designed around, so this is fine as a follow-up if you'd rather not grow the diff. Flagging it because the omission isn't visible from the code — the feature just quietly doesn't fire until the message is sent. Generated by Claude Code |
Slack fires link_shared once while the link sits in the composer and again once the message is posted. The composer payload carries no channel, only an unfurl id, so it failed the required-field check and previews only appeared after sending. Resolve the unfurl target from either shape and answer chat.unfurl with the matching argument pair, leaving the posted-message path unchanged.
|
Fixed in aebf278, so the note above is no longer outstanding.
The posted-message path is byte-identical to what was verified live against a real workspace; the composer branch is added beside it rather than folded into a shared call. Four existing files, +98/-19, two new parser tests covering the composer payload and a composer event missing its unfurl id. Unit tests 560 pass, oxlint clean on 461 files, Generated by Claude Code |
SETUP.md was restructured on main (112 to 315 lines, new headings and a `https://<your-twenty-host>` placeholder convention). Took main's structure and re-applied the record link preview additions into it: the front-domain placeholder in the manifest paragraph, the links:read / links:write scope rows, the link_shared and entity_details_requested subscriptions, the 0.7.0 upgrade note, and a Record link previews section rewritten in main's prose style. Renamed the manifest's unfurl domain placeholder to <your-twenty-front-host> so it matches the convention main adopted for the others.
2 participants
Why
Pasting a Twenty record link in Slack currently renders nothing. Slack's Work Objects (Oct 2025) let an app unfurl links from its own domain as structured, typed record cards — the Salesforce-in-Slack experience. A CRM is the canonical use case, and the Slack app already has all the plumbing this needs: the events resolver, the Slack-account-to-workspace-member matching, and a per-install Slack app manifest, which makes the per-domain unfurl registration work for self-hosted instances too.
What it does
A link to a person, company, opportunity, note or task (
{workspaceUrl}/object/{name}/{id}) renders as a work object card — whether a member pastes it or the bot posts it (assistant answers, workflow message steps). Cards show the record title, display type, a record icon where one is publicly resolvable (company favicon via twenty-icons.com, public person avatars, an opportunity's company logo) and a few headline fields (person: company as a clickable entity ref, job title; company: domain, city; opportunity: company ref, stage, amount, close date; task: status, due date; note: created). Expanding a card opens Slack's flexpane, filled with a wider field set (email, phone, LinkedIn, country, annual revenue, account owner, point of contact, assignee, a body preview, created/updated).link_sharedandentity_details_requestedare dispatched by the events resolver to two new logic functions (slack-link-unfurl,slack-entity-details), same owner-resolver-to-workspace-function pattern as the other events.chat.unfurlentity metadata (slack#/entities/item). Every failure path skips silently or logs a warning; a broken unfurl never surfaces as an error in Slack.link_sharedfor an app's own messages, so record links in outgoing messages get the same entity metadata attached at post time inpostSlackMessage, the singlechat.postMessagecall site. A message whose links are not record links never reaches the workspace-URL query, and if Slack rejects the entity payload the post is retried once without it, so a refused preview never costs the message.entity_details_requestedviaentity.presentDetailswith the detailed field set; unresolvable, unreadable or unnameable records get distinct error states. The record is resolved from the event's entity URL orexternal_ref(which carries the object name as itstype). The event schema is young, so an unrecognized shape is logged verbatim for diagnosis.resolveSlackRunAsWorkspaceMemberIdmatching the assistant uses. Bot-posted links skip that gate deliberately: their content is decided upstream (the assistant runs with the requester's permissions, workflow steps post what their author configured). The flexpane can be opened by anyone who sees the card (including external users in Slack Connect channels), so it is gated on the viewer with the same member matching; non-members get a short notice instead. Because the card is channel-visible and the flexpane is not, a person's contact details (email, phone) are flexpane-only. Records are read with the app's function role, which gains read-only access to the five CRM objects; CRM writes remain exclusive to the agent's role.employeesandperson.city, which exist only on dev-seeded workspaces. A single unknown field fails the whole GraphQL query, so selecting one would make previews silently fail elsewhere. Metadata-driven field resolution is the follow-up that would lift this.get-record-image-identifierutil).links:read/links:writescopes, both events, an unfurl-domain placeholder andfeatures.rich_previewswith theslack#/entities/itementity type, so an app created from it has Work Object Previews on without any manual setup; the connection provider requests the new scopes so reconnecting grants them. Slack only reads a manifest at app creation, so SETUP.md still documents the toggle as the by-hand step for apps created manually or from a pre-0.7.0 manifest, alongside the unfurl domain, the app icon behind the card badge, and the reconnect that upgrading to 0.7.0 needs. App version bumps to 0.7.0.The entity payload shape is written against the typed
EntityMetadatacontract shipped in@slack/web-api7.19.Notes
runAsis'user' | 'application', and only agents take arunAsWorkspaceMemberId), so cards carry app-role data behind the member gate rather than the viewer's own object- or row-level permissions. Both AI reviewers flagged this independently; the options are laid out on that thread and it needs a call from @twentyhq/security before merge.chat.update) don't refresh or add previews yet; custom objects, metadata-driven fields and typedtaskentities are follow-ups, as are flexpane actions (inline edits) through the interactivity resolver.Screenshots