feat(copy): support advanced copy configuration with custom target paths #1711
Conversation
- Add CopyConfig enum to support both basic and advanced copy modes
- Basic mode: maintains backward compatibility with string[] format
- Advanced mode: supports {from: string, to: string} format for custom paths
- Update copy plugin to handle both configuration formats
- Ensure target directories are created automatically
Example config:
{
'copy': [
'public', // basic mode
{ 'from': 'assets', 'to': 'static' } // advanced mode
]
}
Walkthrough该拉取请求引入了一个新的枚举类型 Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/plugins/copy.rs (1)
71-77: 在复制前验证源路径是否存在在复制操作中，如果源路径不存在，
copy函数可能会失败。建议在复制前检查源路径的存在性，并在不存在时提供有意义的错误消息或跳过复制。示例修改：
match config { CopyConfig::Basic(src) => { let src = context.root.join(src); + if !src.exists() { + eprintln!("Source path {:?} does not exist", src); + continue; + } debug!("copy {:?} to {:?}", src, dest); copy(&src, dest)?; } // 其他分支... }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
crates/mako/src/config.rs(2 hunks)crates/mako/src/plugins/copy.rs(4 hunks)
🔇 Additional comments (1)
crates/mako/src/config.rs (1)
147-147: 确保所有使用config.copy的代码已更新
Config结构体中的copy字段类型从Vec<String>更改为Vec<CopyConfig>，这可能影响到依赖此字段的其他代码。请确认所有使用config.copy的代码都已更新以适应新的类型。
生成以下脚本以查找所有使用config.copy的代码位置：
✅ Verification successful
Let me check the implementation in the copy plugin to verify if it's already compatible with the new type.
代码已正确适配新的类型变更
从代码实现来看，crates/mako/src/plugins/copy.rs中的代码已经完全适配了Vec<CopyConfig>的新类型，正确处理了CopyConfig::Basic和CopyConfig::Advanced两种情况。没有发现其他使用config.copy的地方需要更新。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述：查找所有使用`config.copy`的代码，确保兼容新的类型
rg 'config\.copy' --glob '!crates/mako/src/config.rs'
Length of output: 225
Script:
#!/bin/bash
# Check the implementation in copy.rs to see how config.copy is used
rg -A 10 'config\.copy' crates/mako/src/plugins/copy.rs
Length of output: 1012
| for config in context.config.copy.iter() { | ||
| let src = match config { | ||
| CopyConfig::Basic(src) => context.root.join(src), | ||
| CopyConfig::Advanced { from, .. } => context.root.join(from), | ||
| }; | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
建议检查源路径是否存在
当前代码在处理src之前未检查其是否存在，可能会在源路径不存在时引发错误。建议在处理之前添加对src存在性的验证，并在不存在时进行适当的错误处理或日志记录。
示例修改：
for config in context.config.copy.iter() {
let src = match config {
CopyConfig::Basic(src) => context.root.join(src),
CopyConfig::Advanced { from, .. } => context.root.join(from),
};
+ if !src.exists() {
+ eprintln!("Source path {:?} does not exist", src);
+ continue;
+ }
if src.exists() {
debug!("watch {:?}", src);
// ...
}
}Committable suggestion skipped: line range outside the PR's diff.
crates/mako/src/plugins/copy.rs
Outdated
| let target = dest.join(to.trim_start_matches("/")); | ||
|
|
||
| if !target.exists() { | ||
| fs::create_dir_all(&target)?; | ||
| } | ||
|
|
||
| debug!("copy {:?} to {:?}", src, target); | ||
| copy(&src, &target)?; | ||
| } |
There was a problem hiding this comment.
防止目标路径的目录遍历漏洞
在处理CopyConfig::Advanced配置时，to字段可能包含恶意路径（如../），导致文件复制到意外的位置。建议对目标路径进行规范化和验证，确保目标路径位于预期的目标目录中，防止潜在的安全问题。
示例修改：
let target = dest.join(to.trim_start_matches("/"));
+ let target = target.canonicalize()?;
+ let dest_canonical = dest.canonicalize()?;
+ if !target.starts_with(&dest_canonical) {
+ return Err(anyhow!("Invalid target path: {:?}", target));
+ }
if !target.exists() {
fs::create_dir_all(&target)?;
}Committable suggestion skipped: line range outside the PR's diff.
| #[derive(Deserialize, Serialize, Debug)] | ||
| #[serde(untagged)] | ||
| pub enum CopyConfig { | ||
| Basic(String), | ||
| Advanced { from: String, to: String }, | ||
| } | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
为CopyConfig添加配置验证
CopyConfig中的from和to字段直接来自用户配置，可能包含非法或不安全的路径。建议在配置解析时添加路径验证，确保路径不存在注入风险并且指向合法的位置。
Add path canonicalization and validation to ensure target paths remain within the destination directory
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
crates/mako/src/plugins/copy.rs (1)
71-78: 建议优化基础配置的错误处理基础配置的处理逻辑过于简单，建议添加更详细的错误处理和日志记录。
建议修改如下：
CopyConfig::Basic(src) => { let src = context.root.join(src); + debug!("正在处理基础复制配置"); + if !src.exists() { + debug!("源路径不存在: {:?}", src); + continue; + } debug!("copy {:?} to {:?}", src, dest); copy(&src, dest)?; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
crates/mako/src/plugins/copy.rs(4 hunks)
🔇 Additional comments (2)
crates/mako/src/plugins/copy.rs (2)
34-39: 建议在处理源路径之前进行验证
这段代码在处理配置时缺少对源路径的预先验证。虽然后续有exists()检查，但建议在匹配过程中就进行更全面的验证。
建议修改如下：
for config in context.config.copy.iter() {
let src = match config {
CopyConfig::Basic(src) => context.root.join(src),
CopyConfig::Advanced { from, .. } => context.root.join(from),
};
+
+ if !src.exists() {
+ debug!("跳过不存在的源路径: {:?}", src);
+ continue;
+ }79-96: 🛠️ Refactor suggestion
建议增强高级配置的路径处理和安全性
当前的路径验证逻辑可以进一步加强，同时建议添加更多的安全检查。
建议修改如下：
CopyConfig::Advanced { from, to } => {
let src = context.root.join(from);
+ if !src.exists() {
+ debug!("源路径不存在: {:?}", src);
+ continue;
+ }
let target = dest.join(to.trim_start_matches("/"));
let target = target.canonicalize()?;
let dest_canonical = dest.canonicalize()?;
if !target.starts_with(&dest_canonical) {
- return Err(anyhow!("Invalid target path: {:?}", target));
+ return Err(anyhow!("目标路径不安全: {:?}", target));
}
+ // 验证目标路径不包含特殊字符
+ if to.contains("..") {
+ return Err(anyhow!("目标路径包含非法字符: {:?}", to));
+ }
+
if !target.exists() {
+ debug!("创建目标目录: {:?}", target);
fs::create_dir_all(&target)?;
}
debug!("copy {:?} to {:?}", src, target);
copy(&src, &target)?;
}Likely invalid or redundant comment.
| let target = target.canonicalize()?; | ||
| let dest_canonical = dest.canonicalize()?; | ||
| if !target.starts_with(&dest_canonical) { | ||
| return Err(anyhow!("Invalid target path: {:?}", target)); | ||
| } |
There was a problem hiding this comment.
路径规范化可能导致错误
使用canonicalize()进行路径规范化时，如果路径不存在会导致错误。建议在这种情况下使用更健壮的处理方式。
建议添加错误处理：
- let target = target.canonicalize()?;
- let dest_canonical = dest.canonicalize()?;
+ let target = target.canonicalize().map_err(|e| {
+ anyhow!("无法规范化目标路径 {:?}: {}", target, e)
+ })?;
+ let dest_canonical = dest.canonicalize().map_err(|e| {
+ anyhow!("无法规范化输出目录 {:?}: {}", dest, e)
+ })?;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| let target = target.canonicalize()?; | |
| let dest_canonical = dest.canonicalize()?; | |
| if !target.starts_with(&dest_canonical) { | |
| return Err(anyhow!("Invalid target path: {:?}", target)); | |
| } | |
| let target = target.canonicalize().map_err(|e| { | |
| anyhow!("无法规范化目标路径 {:?}: {}", target, e) | |
| })?; | |
| let dest_canonical = dest.canonicalize().map_err(|e| { | |
| anyhow!("无法规范化输出目录 {:?}: {}", dest, e) | |
| })?; | |
| if !target.starts_with(&dest_canonical) { | |
| return Err(anyhow!("Invalid target path: {:?}", target)); | |
| } |
🎯 Features:
• Added CopyConfig enum supporting both basic and advanced modes
• Maintained backward compatibility with string[] format
• Introduced {from, to} format for custom target paths
• Enhanced copy plugin for both configuration types
• Added automatic target directory creation
📝 Example Configuration:
🔍 Details:
• Basic mode preserves existing string[] functionality
• Advanced mode enables precise path control
• Directories are created automatically as needed
• Both modes can be mixed in the same config"
Summary by CodeRabbit
新功能
CopyConfig枚举，支持基本和高级的文件复制配置。CopyPlugin，支持更灵活的源-目标路径关系，确保高级配置下的路径有效性。修复