Repository navigation
feat: support .md and .mdx file uploads - #8995
sriramveeraghanta wants to merge 5 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe upload helper now recognizes Markdown MIME types and checks dangerous filename extensions across intermediate filename components. Web attachment components recognize Markdown extensions, and tests cover the updated behavior. ChangesMarkdown attachment handling
Priority: ⬇️ Low Estimated code review effort: 3 (Moderate) | ~20 minutes Change: Feature Merge Risk: ⚪ Minimal · up to Markdown and MDX MIME types pass through the attachment API and storage flow; no merge-blocking issue is evident. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/services/src/file/helper.ts (1)
112-135:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winDo not fall back to the extension after filename validation fails.
validateFilename()already flags suspicious names such asfoo.exe.md, but the result is only logged. With the new extension fallback, those files still resolve totext/markdown/text/mdxand can pass the allow-list, which defeats the double-extension safeguard.Suggested fix
const validateAndDetectFileType = async (file: File): Promise<string> => { // Basic filename validation const filenameError = validateFilename(file.name); if (filenameError) { console.warn(`File validation warning: ${filenameError}`); + return ""; } try { const signatureType = await detectMimeTypeFromSignature(file); if (signatureType) { return signatureType;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/services/src/file/helper.ts` around lines 112 - 135, If validateFilename(file.name) returns an error, stop and do not use the extension fallback; in validateAndDetectFileType, after calling validateFilename use the returned filenameError to short-circuit and return an empty string (or otherwise mark the file as unknown/rejected) instead of continuing to detectMimeTypeFromExtension. Keep the existing signature-based detection path (detectMimeTypeFromSignature) but only run it when validateFilename passed; reference validateAndDetectFileType, validateFilename, detectMimeTypeFromSignature, and detectMimeTypeFromExtension when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/services/src/file/helper.ts`:
- Around line 112-135: If validateFilename(file.name) returns an error, stop and
do not use the extension fallback; in validateAndDetectFileType, after calling
validateFilename use the returned filenameError to short-circuit and return an
empty string (or otherwise mark the file as unknown/rejected) instead of
continuing to detectMimeTypeFromExtension. Keep the existing signature-based
detection path (detectMimeTypeFromSignature) but only run it when
validateFilename passed; reference validateAndDetectFileType, validateFilename,
detectMimeTypeFromSignature, and detectMimeTypeFromExtension when making this
change.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: ba97d0e1-9b14-4903-9ecf-89c664a15ef5
📒 Files selected for processing (4)
apps/api/plane/settings/common.pyapps/web/core/components/icons/attachment/attachment-icon.tsxpackages/editor/src/core/constants/config.tspackages/services/src/file/helper.ts
There was a problem hiding this comment.
Pull request overview
This PR extends Plane’s attachment upload flow to recognize Markdown and MDX files end-to-end by adding extension-based MIME fallback in the shared upload helper, allowing text/mdx through the editor/API allow-lists, and showing text-file icons for markdown attachments.
Changes:
- Added extension-based MIME detection for plain-text uploads (
.md,.markdown,.mdx) when signature detection returns no type. - Updated frontend and backend attachment MIME allow-lists to include MDX support.
- Mapped markdown-family file extensions to the existing text attachment icon.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/services/src/file/helper.ts |
Adds filename-extension fallback for markdown MIME detection in the shared upload metadata helper. |
packages/editor/src/core/constants/config.ts |
Adds text/mdx to the editor attachment MIME allow-list. |
apps/web/core/components/icons/attachment/attachment-icon.tsx |
Routes md / markdown / mdx extensions to the text-file icon. |
apps/api/plane/settings/common.py |
Adds text/mdx to the backend attachment MIME allow-list and removes a duplicate markdown entry. |
Plain-text files have no magic bytes, so the file-type signature detector returned an empty MIME type and the backend rejected them. Add an extension-based fallback for markdown/mdx, allow text/mdx in the API and editor allow-lists, and pick a file icon for these extensions.
- Short-circuit validateAndDetectFileType when validateFilename flags a suspicious name so files like foo.exe.md cannot bypass the attachment allowlist via the new extension MIME fallback. - Lowercase fileType in getFileIcon so uppercase extensions (e.g. .MD, .PDF) resolve to the correct icon.
b374796 to
8eeb8d7
Compare
|
React Doctor found no new issues. 🎉 Reviewed by React Doctor for commit |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 9 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
Suppressed comments (2)
apps/web/vitest.config.ts:17
resolve.tsconfigPaths = trueis not a valid Vite/Vitest config option (tsconfig-path resolution is typically provided via thevite-tsconfig-pathsplugin). As-is, Vitest may fail to resolve@/...imports used by tests (e.g. attachment-icon.test.tsx). Configurevite-tsconfig-pathsas a plugin (similar to apps/web/vite.config.ts) instead of this custom resolve flag.
import { defineConfig } from "vitest/config";
export default defineConfig({
resolve: {
tsconfigPaths: true,
},
test: {
environment: "node",
include: ["core/**/*.test.{ts,tsx}"],
},
packages/services/src/file/helper.ts:21
- The extension fallback maps
mdxtotext/markdown, but the PR description and related allow-list/accept-list updates referencetext/mdx. This mismatch can lead to inconsistent MIME values across the upload pipeline (and makes thetext/mdxchanges potentially ineffective). Please pick one MIME for.mdxand align this map, the frontend accept list, backend allow-list, and these tests accordingly.
const EXTENSION_MIME_TYPE_MAP: Record<string, string> = {
md: "text/markdown",
markdown: "text/markdown",
mdx: "text/markdown",
};
|
@dheeru0198 @pablohashescobar This PR has been rebased onto current preview, the review feedback is resolved, and the refreshed CI suite is green. The scope is now limited to work-item Markdown and MDX attachments, with MDX using text/markdown and unit coverage added. Could one of you take a final look when you have a chance? |
…md-mdx-support # Conflicts: # apps/web/components/icons/attachment/attachment-icon.test.tsx # pnpm-lock.yaml
Suspicious filenames now only block the extension-based MIME fallback, so files whose signature identifies them (e.g. deploy.sh.png) upload as before. The extension lookup also ignores inherited object properties such as constructor and __proto__.
Description
Adds
.md,.markdown, and.mdxsupport to the work-item attachment flow.text/markdownMIME type, so no API or editor allow-list change is required.file.exe.safe.md.Type of Change
Screenshots and Media (if applicable)
N/A — this changes attachment upload validation and icon selection without introducing new UI.
Test Scenarios
pnpm --filter=@plane/services test— 15 tests passed.pnpm --filter=web test— 7 tests passed.pnpm buildpassed.pnpm checkreaches an existing formatting failure inpackages/i18n/src/types/keys.generated.ts; that file is unchanged by this PR, and all affected-package checks pass.References
N/A
Summary by CodeRabbit
text/markdownand display the text-file icon, including uppercase extensions.