Skip to content

Conversation

@Crokily
Copy link
Contributor

@Crokily Crokily commented Nov 16, 2025

开发方案见:https://www.notion.so/MD-2a39919329548107a259c6a1cec5a089
除了按方案完成了编辑器以外

还执行了以下可能阻塞开发的修复

  1. 缺少可回调的登录页,现在如果用户没登陆页面,路由守卫会将其跳转到一个有回调地址的登录提示页,登录后自动跳回原页面
  2. 改了pre-commit的脚本,之前会默认直接提交所有文件,但有时候用户会有stage部分文件再提交的需求,所以改成了可以兼容stage的代码
  3. 目前对next-img的替换似乎不完全,仍然会有外链文件使用next-js的问题,导致外链一直会出尺寸无法获取的bug,于是彻底替换,目前完全使用img组件了。
  4. 目前的Shiki代码高亮插件不支持大小写区分,对JavaScript的代码段就无法识别,导致系统build出错,必须javasciprt才行,因为考虑到用户的自用的编辑器也有可能会有使用JavaScript等大写代码段,所以最好的解决办法是加个remark插件,每次对md,mdx文件做统一处理转换大小写。
  5. 把原理的投稿流程做了解耦2a52b0709fba4f046dde988551b10ebe82c8b3de,拆成了3个文件,现在代码不超过200行了。

后续计划

  1. 目前的UI是有点简陋的,但后续有空的时候再改进吧,主要是编辑器本体布局要放大,期望做成notion的双栏样式
  2. AI元数据和目录识别功能
  3. 加入indexDB存用户编辑记录

预览

由于R2的CORS规则问题,只加了localhost,involutionhell到白名单,因此预览的随机vercel路径是没法使用上传功能的。
可以使用这个预览:http://213.35.102.83:3000/

测试流程

  1. 先登录,再点击 我要投稿 按钮,或直接点击 我要投稿 进入提示登录页完成登录,然后自动跳转进入编辑器页
  2. 测试编辑器功能,编辑文本,粘贴图片等等——目前发现的问题是编辑器本体太小了,有时候会挡住菜单,但不是非常影响使用,下次UI优化会解决
  3. 发布并测试发布后的效果是否正常。

env

  1. 需要增加几个R2相关的环境变量,已发给对应人员
  2. 目前使用的是involutionhell@gmail.com的cloudflare账户开的R2, 由于域名不在该账户,导致没法把R2绑定上公共域名,因此R2的访问和功能会被限制,用量小的时候问题不大,但最好这个需要longlong把域名的DNS管理换到involutionhell@gmail.com的cloudflare账户上,这样R2才能绑定域名。

Crokily and others added 24 commits November 8, 2025 00:24
…ok to selectively stage changes after image migration
@Crokily Crokily requested a review from longsizhuo November 16, 2025 13:58
@vercel
Copy link

vercel bot commented Nov 16, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
website-preview Error Error Dec 7, 2025 1:35pm

@vercel
Copy link

vercel bot commented Nov 16, 2025

@Crokily is attempting to deploy a commit to the longsizhuo's projects Team on Vercel.

A member of the Team first needs to authorize it.

// 格式:users/{userId}/{article-slug}/{timestamp}-{filename}
const timestamp = Date.now();
const userId = session.user.id;
const key = `users/${userId}/${articleSlug}/${timestamp}-${filename}`;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

如果前端传了奇怪字符,比如:

articleSlug = "../../evil"

filename = "xxx/yyy.png" 或很长的一串

虽然 R2/S3 的 key 本质就是字符串,不会真的“目录逃逸”,但会让路径变得非常脏、不利于后续管理,而且有机会打爆某些统计脚本。

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

文件大小 & 类型上限没在后端做限制, 可能直接打爆

if (!selectedKey.endsWith(CREATE_SUBDIR_SUFFIX)) return selectedKey;
const [l1] = selectedKey.split("/");
if (!l1) return "";
const sanitized = newSub.trim().replace(/\s+/g, "-");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这只处理了空格 -,没有处理:
大写字母 / 全角字符 / 标点 / 非 ASCII
连续的 ---
结尾的 -

建议这里复用跟“文件名 / frontmatter slug”同一种工具函数, 同上

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a comprehensive Notion-style markdown editor for user article submissions, integrating Cloudflare R2 for image uploads, along with several infrastructure improvements and bug fixes.

Key Changes:

  • New Milkdown-based markdown editor with real-time preview and drag-and-drop image support
  • R2 integration for automatic image uploads with presigned URLs
  • Enhanced authentication flow with callback URL support and dedicated login page
  • Fixed Shiki code highlighting case-sensitivity issues via remark plugin
  • Improved pre-commit hooks to respect staged files

Reviewed changes

Copilot reviewed 23 out of 25 changed files in this pull request and generated 16 comments.

Show a summary per file
File Description
source.config.ts Adds remark plugin to normalize code block language identifiers and updates image handling configuration with detailed documentation
next.config.mjs Disables Next.js image optimization due to Vercel quota limits, removes conflicting remarkImage configuration
package.json Adds Milkdown editor dependencies (@milkdown/crepe, @milkdown/kit), AWS SDK for R2 integration, and mdast types
mdx-components.tsx Overrides default img component to use native HTML img tags instead of Next.js Image component
lib/submission.ts New utility functions for filename validation and markdown extension handling
lib/editor-store.ts Zustand store for managing editor state (title, description, tags, markdown content)
auth.config.ts Adds /editor route protection and implements redirect callback for post-login navigation
app/login/page.tsx New login page that accepts redirectTo/callbackUrl parameters for seamless navigation after authentication
app/editor/page.tsx Server component that verifies authentication and renders editor client component
app/editor/EditorPageClient.tsx Main editor UI orchestrating metadata form, directory selection, markdown editor, and publish workflow
app/components/MarkdownEditor.tsx Milkdown editor wrapper with image buffer integration and auto-sync to store
app/components/EditorMetadataForm.tsx Form component for article metadata (title, description, tags, filename)
app/components/DocsDestinationForm.tsx Tree-based directory selector for choosing submission destination
app/components/hooks/useImageBuffer.ts Custom hook for managing local image cache before upload
app/components/SignInButton.tsx Updated to accept redirectTo parameter for post-login navigation
app/components/Contribute.tsx Modified "我要投稿" button to navigate directly to /editor instead of opening dialog
app/components/contribute/tree-utils.tsx Extracted tree utility functions for reuse across components
app/api/upload/route.ts New API endpoint generating R2 presigned URLs for client-side image uploads
app/globals.css Comprehensive Milkdown editor styling with dark mode and responsive support
.husky/pre-commit Updated to only stage migration-generated changes instead of all modified files
.gitignore Updated AGENTS.md pattern and added .claude directory
.env.sample Added R2 configuration variables (account ID, access keys, bucket, public URL)
.github/workflows/sync-uuid.yml Formatting improvement for multi-line condition

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

onClick={() => {
if (confirm("确定要清空所有内容吗?")) {
useEditorStore.getState().reset();
window.location.reload();
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using window.location.reload() after resetting the store is inefficient and provides poor UX. The full page reload will:

  1. Lose any unsaved state in other components
  2. Create a jarring user experience with a full page flash
  3. Be slower than just resetting the React state

Consider removing this line and relying on React's state management:

onClick={() => {
  useEditorStore.getState().reset();
  // The editor will re-render with empty state automatically
}}

If you need to reset the Milkdown editor instance, expose a reset method through the MarkdownEditorHandle ref.

Suggested change
window.location.reload();
// If the MarkdownEditor needs to be reset, call its reset method via the ref
editorRef.current?.reset?.();

Copilot uses AI. Check for mistakes.
{ status: 400 },
);
}

Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing input validation: The articleSlug parameter is not validated for path traversal or malicious content. While it's used in constructing the S3 key, an attacker could potentially inject path separators to organize files in unintended locations.

Add validation:

// Validate articleSlug
if (!/^[a-zA-Z0-9_-]+$/.test(articleSlug)) {
  return NextResponse.json(
    { error: "articleSlug 包含非法字符" },
    { status: 400 }
  );
}
Suggested change
// Validate articleSlug
if (!/^[a-zA-Z0-9_-]+$/.test(articleSlug)) {
return NextResponse.json(
{ error: "articleSlug 包含非法字符" },
{ status: 400 }
);
}

Copilot uses AI. Check for mistakes.
Comment on lines +35 to +39
} catch {
const res = await fetch("/docs-tree.json").catch(() => null);
const data = await res?.json();
if (mounted && data?.ok) {
setTree(data.tree || []);
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error handling catches all errors with an empty catch block, silently falling back to fetching from a static JSON file. This makes debugging difficult and hides potential network or configuration issues.

Consider logging the error or providing user feedback:

} catch (error) {
  console.warn("Failed to fetch docs tree from API, falling back to static file:", error);
  try {
    const res = await fetch("/docs-tree.json");
    const data = await res?.json();
    if (mounted && data?.ok) {
      setTree(data.tree || []);
    }
  } catch (fallbackError) {
    console.error("Failed to load docs tree:", fallbackError);
    // Consider showing a user-facing error message
  }
}
Suggested change
} catch {
const res = await fetch("/docs-tree.json").catch(() => null);
const data = await res?.json();
if (mounted && data?.ok) {
setTree(data.tree || []);
} catch (error) {
console.warn("Failed to fetch docs tree from API, falling back to static file:", error);
try {
const res = await fetch("/docs-tree.json");
const data = await res?.json();
if (mounted && data?.ok) {
setTree(data.tree || []);
}
} catch (fallbackError) {
console.error("Failed to load docs tree from static file:", fallbackError);
// Optionally, show a user-facing error message here

Copilot uses AI. Check for mistakes.
Comment on lines +67 to +69
clearAll();
};
}, [clearAll]);
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The cleanup logic in the useEffect has a dependency issue. The clearAll function is included in the dependency array, but it's wrapped in useCallback with notifyImagesChange as a dependency. This creates an unstable reference chain that may cause unnecessary cleanup cycles.

Since the cleanup should only run on unmount, consider using an empty dependency array or restructuring to avoid the circular dependency:

useEffect(() => {
  return () => {
    imageMapRef.current.forEach((_, blobUrl) => {
      URL.revokeObjectURL(blobUrl);
    });
    imageMapRef.current.clear();
  };
}, []); // Empty deps - only cleanup on unmount
Suggested change
clearAll();
};
}, [clearAll]);
imageMapRef.current.forEach((_, blobUrl) => {
URL.revokeObjectURL(blobUrl);
});
imageMapRef.current.clear();
};
}, []);

Copilot uses AI. Check for mistakes.
Comment on lines 83 to 87
// 生成唯一的对象键
// 格式:users/{userId}/{article-slug}/{timestamp}-{filename}
const timestamp = Date.now();
const userId = session.user.id;
const key = `users/${userId}/${articleSlug}/${timestamp}-${filename}`;
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security issue: The filename is used directly in the S3 key construction without proper sanitization. A malicious user could inject path traversal characters (e.g., ../../) to write files outside the intended directory structure.

Consider sanitizing the filename:

// Sanitize filename to prevent path traversal
const sanitizedFilename = filename.replace(/[^a-zA-Z0-9._-]/g, '_');
const key = `users/${userId}/${articleSlug}/${timestamp}-${sanitizedFilename}`;

Copilot uses AI. Check for mistakes.
Comment on lines 126 to 238
alert("请输入文章标题");
return;
}

if (!filename.trim()) {
alert("请输入文件名");
return;
}

if (!destinationPath) {
alert("请选择投稿目录");
return;
}

const normalizedFilename = ensureMarkdownExtension(filename);
const filenameBase = stripMarkdownExtension(normalizedFilename);
if (!filenameBase || !FILENAME_PATTERN.test(filenameBase)) {
alert("文件名仅支持英文、数字、连字符或下划线,并需以字母或数字开头。");
return;
}

if (normalizedFilename !== filename) {
setFilename(normalizedFilename);
}

let githubDraftWindow: Window | null = null;
try {
githubDraftWindow = window.open("", "_blank");
if (githubDraftWindow) {
githubDraftWindow.document.title = "正在生成稿件…";
githubDraftWindow.document.body.innerHTML =
'<p style="font-family:system-ui;padding:16px;">正在生成 GitHub 草稿,请稍候…</p>';
githubDraftWindow.opener = null;
}
} catch {
githubDraftWindow = null;
}

console.group("发布流程:上传图片并生成 GitHub 草稿");
console.log("文章标题:", title);
console.log("文件名:", normalizedFilename);
console.log("投稿目录:", destinationPath);
console.log("图片数量:", imageCount);

let finalMarkdown = markdown;
const articleSlug = filenameBase;

// 如果有图片,上传到 R2 并替换 URL
const editorHandle = editorRef.current;
if (!editorHandle) {
throw new Error("编辑器尚未就绪,无法上传图片");
}

const removedImages = editorHandle.removeUnreferencedImages(markdown);
if (removedImages > 0) {
console.log(`已清理 ${removedImages} 个未在 Markdown 中引用的图片`);
}

const imageEntries = Array.from(editorHandle.getImages().entries());

if (imageEntries.length > 0) {
console.log("开始上传图片...");

// 并发上传所有图片
const uploadPromises = imageEntries.map(([blobUrl, file]) =>
uploadImage(blobUrl, file, articleSlug),
);

const uploadResults = await Promise.all(uploadPromises);

console.log("所有图片上传完成!");
console.group("图片 URL 映射");
uploadResults.forEach(({ blobUrl, publicUrl }) => {
console.log(`${blobUrl} -> ${publicUrl}`);
});
console.groupEnd();

// 替换 Markdown 中的 blob URL 为公开 URL
uploadResults.forEach(({ blobUrl, publicUrl }) => {
finalMarkdown = finalMarkdown.replaceAll(blobUrl, publicUrl);
});

console.log("Markdown 中的 blob URL 已替换为公开 URL");
}

console.group("最终 Markdown 内容");
console.log(finalMarkdown);
console.groupEnd();

console.groupEnd();

const frontmatter = buildFrontmatter({
title,
description,
tags,
});
const markdownBody = finalMarkdown.trimStart();
const finalContent =
markdownBody.length > 0
? `${frontmatter}\n\n${markdownBody}`
: `${frontmatter}\n`;

const params = new URLSearchParams({
filename: normalizedFilename,
value: finalContent,
});
const githubUrl = buildDocsNewUrl(destinationPath, params);
if (githubDraftWindow) {
githubDraftWindow.location.href = githubUrl;
} else {
window.open(githubUrl, "_blank", "noopener,noreferrer");
}
alert("图片已上传并生成 GitHub 草稿,请在新标签页完成提交。");
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using alert() for user feedback is not ideal for modern web applications. Consider using a proper toast notification system or UI component for better user experience and accessibility.

Example with a toast library:

import { toast } from "sonner"; // or your preferred toast library

// Replace alert() calls with:
toast.error("请输入文章标题");
toast.success("图片已上传并生成 GitHub 草稿");

Copilot uses AI. Check for mistakes.
Comment on lines +307 to +311
if (confirm("确定要清空所有内容吗?")) {
useEditorStore.getState().reset();
window.location.reload();
}
}}
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Using window.confirm() is not recommended for modern web applications. Consider using a proper confirmation dialog component for better UX and accessibility.

Example:

// Use a proper dialog component
<AlertDialog>
  <AlertDialogTrigger asChild>
    <Button variant="outline">清空</Button>
  </AlertDialogTrigger>
  <AlertDialogContent>
    <AlertDialogHeader>
      <AlertDialogTitle>确定要清空所有内容吗?</AlertDialogTitle>
    </AlertDialogHeader>
    <AlertDialogFooter>
      <AlertDialogCancel>取消</AlertDialogCancel>
      <AlertDialogAction onClick={() => {
        useEditorStore.getState().reset();
        window.location.reload();
      }}>
        确定
      </AlertDialogAction>
    </AlertDialogFooter>
  </AlertDialogContent>
</AlertDialog>

Copilot uses AI. Check for mistakes.
Comment on lines +2 to +14
# 记录迁移前的变更状态
BEFORE_STATUS=$(git status --porcelain)

pnpm migrate:images || exit 1

# 将迁移后的变更加入暂存,确保本次提交包含更新
git add -A
# 检查迁移后是否有新的变更
AFTER_STATUS=$(git status --porcelain)

if [ "$BEFORE_STATUS" != "$AFTER_STATUS" ]; then
echo "检测到迁移脚本产生了变更,添加所有变更到暂存区以确保完整提交..."
# 只添加由于迁移产生的变更,而不是用户未暂存的所有变更
git add -u
fi
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The pre-commit hook script uses shell variable comparison which may not work reliably across different systems. The comparison [ "$BEFORE_STATUS" != "$AFTER_STATUS" ] could fail if the status contains special characters or multiline output.

Consider a more robust approach:

# Check if git diff shows any changes after migration
if ! git diff --quiet; then
  echo "检测到迁移脚本产生了变更,添加所有变更到暂存区以确保完整提交..."
  git add -u
fi

This uses git diff --quiet which properly handles all edge cases and is the recommended way to check for changes.

Copilot uses AI. Check for mistakes.
Comment on lines +149 to +157
const syncInterval = setInterval(updateMarkdown, 2000);

crepeInstanceRef.current = crepe;
isLoadingRef.current = false;

console.log("Milkdown 编辑器初始化成功");

return () => {
clearInterval(syncInterval);
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Performance concern: The editor sync runs every 2 seconds with setInterval, which may cause unnecessary re-renders and state updates. Consider:

  1. Using a debounced approach that only syncs after the user stops typing
  2. Increasing the interval to reduce overhead
  3. Only syncing when content actually changes

Example with debouncing:

// Replace setInterval with debounced sync
let timeoutId: NodeJS.Timeout;
const debouncedSync = () => {
  clearTimeout(timeoutId);
  timeoutId = setTimeout(updateMarkdown, 1000); // Sync 1s after last change
};

// Listen to editor changes
crepe.editor.onTransaction(() => {
  debouncedSync();
});
Suggested change
const syncInterval = setInterval(updateMarkdown, 2000);
crepeInstanceRef.current = crepe;
isLoadingRef.current = false;
console.log("Milkdown 编辑器初始化成功");
return () => {
clearInterval(syncInterval);
// Debounced sync: only update after user stops typing for 1s
let timeoutId: ReturnType<typeof setTimeout> | null = null;
const debouncedSync = () => {
if (timeoutId) clearTimeout(timeoutId);
timeoutId = setTimeout(updateMarkdown, 1000);
};
// Listen to editor changes
const offTransaction = crepe.editor.onTransaction(() => {
debouncedSync();
});
crepeInstanceRef.current = crepe;
isLoadingRef.current = false;
console.log("Milkdown 编辑器初始化成功");
return () => {
if (timeoutId) clearTimeout(timeoutId);
offTransaction();

Copilot uses AI. Check for mistakes.
Comment on lines 63 to 73
// 解析请求体
const body = (await request.json()) as UploadRequest;
const { filename, contentType, articleSlug } = body;

// 验证请求参数
if (!filename || !contentType || !articleSlug) {
return NextResponse.json(
{ error: "缺少必要参数:filename, contentType, articleSlug" },
{ status: 400 },
);
}
Copy link

Copilot AI Nov 23, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security concern: The API endpoint reads sensitive environment variables but doesn't validate the authenticated user's authorization level. While it checks if the user is logged in (session?.user?.id), it doesn't verify if they have permission to upload images.

Consider adding:

  1. Rate limiting to prevent abuse
  2. File size validation (currently missing)
  3. User permission checks if applicable

Example:

// Add file size validation
const MAX_FILE_SIZE = 10 * 1024 * 1024; // 10MB
if (request.headers.get('content-length') && 
    parseInt(request.headers.get('content-length')!) > MAX_FILE_SIZE) {
  return NextResponse.json(
    { error: "文件大小超过限制(最大 10MB)" },
    { status: 400 }
  );
}

Copilot uses AI. Check for mistakes.
@longsizhuo
Copy link
Member

我觉得安全的事情可以稍微放一放,毕竟没那么多人为了打爆图床来的。而且我记得图床上应该也可以限制大小来避免恶意使用的

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants