feat: upgrade issue report form with file attachments and metadata#240
feat: upgrade issue report form with file attachments and metadata#240
Conversation
…tadata Convert the issue report from a Popover to a Dialog with React Hook Form, add drag-and-drop file attachment support via Supabase Storage, and enrich the Plain thread payload with teamId, teamName, customerEmail, customerTier, and attachment links.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![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: c5a5492ff5
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| setAttachments((prev) => [ | ||
| ...prev, | ||
| { | ||
| url: data.url, | ||
| fileName: data.fileName, |
There was a problem hiding this comment.
Closing the dialog while files are still uploading resets local state, but in-flight upload callbacks still append attachments unconditionally; this repopulates a canceled form with stale files and can also drive uploadingCount below zero when callbacks decrement after reset. Users can then reopen the modal and accidentally submit attachments from a previous canceled attempt, so upload results need to be ignored/canceled once the form session is reset.
Useful? React with 👍 / 👎.
| const detectedMime = fileType?.mime ?? file.type | ||
|
|
||
| if (!ALLOWED_MIME_TYPES.includes(detectedMime)) { |
There was a problem hiding this comment.
The MIME check falls back to the client-provided file.type whenever fileTypeFromBuffer cannot identify magic bytes, which lets a crafted upload bypass server-side type enforcement by labeling unsupported binary content as text/plain. This defeats the stated magic-byte validation and allows disallowed file formats into storage unless unknown signatures are rejected (or text files are validated independently).
Useful? React with 👍 / 👎.
…x counter, match height
Update icon (Bug → LifeBuoy), dialog copy, form labels, toast messages, and PostHog event names to reflect general support rather than bug reporting.
Replace the two-step Supabase upload with Plain's native attachment system. Files are held in browser state and uploaded to Plain on submit via a single contactSupportAction server action. Remove the tRPC support router, revert storage parameterization, and drop the issue-attachments bucket dependency.
Move the customer metadata header (email, tier, team ID, Orbit link) from the thread body to an internal Plain note so it doesn't get quoted in email replies. Map tier slugs to readable names (base_v1 → Hobby, pro_v1 → Pro).
…eThread fields Switch from deprecated createThread components/attachmentIds to the recommended flow: createThread (bare) → sendCustomerChat (message + attachments). Use AttachmentType.Chat instead of CustomTimelineEntry.
…or handling - Use description field on createThread instead of deprecated components - Switch from Error to ActionError so error messages surface to the user instead of the generic "Unexpected Error" message
sendCustomerChat requires the thread to use the CHAT channel. Also include Plain error message in ActionError for easier debugging.
The API key lacks chat:create permission needed for sendCustomerChat. Revert to using createThread with components and attachmentIds which works with existing thread:create permission.
… logging - Put the ######## header block back into the thread body text - Add detailed logging for each stage of attachment upload (start, URL creation, upload, success/failure with response body) - Remove separate internal note in favor of inline header
beran-t
left a comment
beran-t
left a comment
There was a problem hiding this comment.
Code review completed. 1 issue found.
| accountOwnerEmail: zfd.text(z.string().email()), | ||
| customerTier: zfd.text(z.string().min(1)), | ||
| files: zfd.repeatableOfType(zfd.file()).optional(), | ||
| }) |
There was a problem hiding this comment.
Security: Server action trusts client-supplied metadata without verification
The contactSupportAction accepts teamId, teamName, customerEmail, accountOwnerEmail, and customerTier directly from client-submitted FormData without any server-side verification. An authenticated user can spoof all of these values by manipulating the request.
Impact:
- Tier spoofing: A Hobby user can submit
customerTier: "pro_v1"to appear as Pro in the support thread, potentially getting prioritized support - Team impersonation: A user can submit another team's ID, causing the Orbit link in the thread to point to the wrong team. A support agent acting on this could make changes to the wrong team
- Email spoofing: The
customerEmaildisplayed to agents can differ fromctx.user.email(which is verified from the auth session). An attacker could impersonate another user
Notably, ctx.user.email from the auth context is used for the Plain customer upsert (line 153), but the thread text shown to support agents uses the untrusted client-supplied customerEmail instead.
Every other team-scoped action in this codebase uses the withTeamIdResolution middleware which calls checkUserTeamAuthCached() to verify team membership. This action is the only one that skips it.
Suggested fix: Use .use(withTeamIdResolution), fetch team data (name, email, tier) server-side from the verified team ID, and use ctx.user.email for the customer email field.
The action previously trusted client-supplied teamId, teamName, customerEmail, accountOwnerEmail, and customerTier without verification. An authenticated user could spoof these to impersonate another team. Now uses withTeamIdResolution middleware to verify team membership (matching every other team-scoped action) and fetches team name, email, and tier from the database server-side. Only description, teamIdOrSlug, and files are accepted from the client.
beran-t
left a comment
beran-t
left a comment
There was a problem hiding this comment.
Review Summary
Overall this is a well-structured PR. Authentication, authorization (via authActionClient + withTeamIdResolution), and error handling are solid. Team metadata is correctly fetched server-side to prevent spoofing. One issue found below.
| </p> | ||
| {remaining > 0 && !isUploading && ( | ||
| <p className="text-xs text-fg-tertiary"> | ||
| Up to {remaining} more file{remaining !== 1 ? 's' : ''} (max 10MB each) |
There was a problem hiding this comment.
Mismatched file size limit: The UI tells users "max 10MB each" but no 10MB validation exists anywhere — neither client-side nor server-side. The server enforces 50MB (support-actions.ts:11), not 10MB. Additionally, files exceeding 50MB are silently filtered out server-side (support-actions.ts:225) with no error feedback to the user.
Either add a client-side size check matching the displayed limit, or update the displayed text to match the actual server limit.
- Set server MAX_FILE_SIZE to 10MB to match the UI's "max 10MB each" - Add client-side file size validation with toast error for oversized files - Move Contact Support button from header back to sidebar footer, next to the Feedback button
beran-t
left a comment
beran-t
left a comment
There was a problem hiding this comment.
Review by automated code review agent.
| formData.append('files', file) | ||
| } | ||
|
|
||
| submitSupport(formData) |
There was a problem hiding this comment.
Bug: FormData passed to withTeamIdResolution middleware causes every submission to fail
The onSubmit handler constructs a FormData and passes it directly to submitSupport(formData). However, the withTeamIdResolution middleware checks for teamIdOrSlug using the in operator on raw clientInput:
if (\!clientInput || typeof clientInput \!== 'object' || \!('teamIdOrSlug' in clientInput)) {
// throws error
}The in operator does not work on FormData objects — FormData stores entries internally, not as own properties. So 'teamIdOrSlug' in formData always returns false, and the middleware throws "teamIdOrSlug is required when using withTeamIdResolution middleware" on every submission.
The existing uploadTeamProfilePictureAction (which also uses zfd.formData + withTeamIdResolution) avoids this by passing a plain object instead of FormData.
Suggested fix — pass a plain object:
const onSubmit = (values: SupportFormValues) => {
submitSupport({
description: values.description.trim(),
teamIdOrSlug: team.id,
files,
})
}- Pass plain object to submitSupport() so withTeamIdResolution middleware can find teamIdOrSlug via the `in` operator (FormData stores entries internally, not as object properties) - Swap sidebar button order: Feedback left, Contact Support right - Add basis-1/2 for even 50/50 split
1 participant
Summary
issue-attachmentsSupabase Storage bucketbucketNameparameter (backward compatible)New files
src/features/dashboard/navbar/report-issue-dialog.tsx— Dialog-based form with React Hook Formsrc/features/dashboard/navbar/file-drop-zone.tsx— Native HTML5 drag-and-drop componentsrc/server/support/support-actions.ts— Server action for file upload with MIME + magic bytes validationModified files
src/server/api/routers/support.ts— Expanded schema and richer Plain thread formattingsrc/lib/clients/storage.ts— Added optionalbucketNameparam to all functionssrc/configs/storage.ts— AddedISSUE_ATTACHMENTS_BUCKET_NAMEsrc/features/dashboard/sidebar/footer.tsx— Updated importDeleted files
src/features/dashboard/navbar/report-issue-popover.tsx— Replaced by dialogInfrastructure prerequisite
A new Supabase Storage bucket named
issue-attachmentsmust be created before file uploads will work.Test plan
.exe) and files over 10MB — verify error toaststeam_id,tier,attachment_count