feat: /plugin command — install, list, remove, update plugins#26
Merged
kienbui1995 merged 1 commit intomainfrom Apr 11, 2026
Merged
feat: /plugin command — install, list, remove, update plugins#26kienbui1995 merged 1 commit intomainfrom
kienbui1995 merged 1 commit intomainfrom
Conversation
📝 WalkthroughWalkthroughPlugin management is added via a new Changes
Sequence DiagramsequenceDiagram
actor User
participant TUI as TUI
participant Queue as CommandQueue
participant CLI as CLIMain
participant FS as Filesystem
User->>TUI: enter "/plugin install owner/repo"
TUI->>Queue: queue Plugin("install owner/repo")
Queue->>CLI: dequeue pending Plugin command
CLI->>CLI: handle_plugin_command(arg)
CLI->>FS: git clone (shallow) -> .magic-code/plugins/<repo>
FS-->>CLI: clone result (ok / error)
CLI->>FS: count plugin skills at plugins/.../skills/
FS-->>CLI: skill count
CLI->>Queue: append status/error to output_lines
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
mc/crates/mc-core/src/skills.rs (1)
139-151:⚠️ Potential issue | 🔴 CriticalTest function outside
mod testsblock — will not compile.The
discover_with_valid_skilltest at line 141 is defined outside the#[cfg(test)] mod testsblock, which closes at line 139. The#[test]attribute at module scope without the#[cfg(test)]context will cause a compilation error.🐛 Proposed fix: Move test inside the module
#[test] fn empty_dir() { let skills = discover_skills(Path::new("/nonexistent")); assert!(skills.is_empty()); } -} #[test] fn discover_with_valid_skill() { let dir = std::env::temp_dir().join(format!("mc-skill-{}", std::process::id())); let skill_dir = dir.join(".magic-code/skills/greet"); std::fs::create_dir_all(&skill_dir).unwrap(); std::fs::write(skill_dir.join("SKILL.md"), "---\nname: greet\n---\nSay hello").unwrap(); let skills = discover_skills(&dir); assert_eq!(skills.len(), 1); assert_eq!(skills[0].name, "greet"); std::fs::remove_dir_all(dir).ok(); } +}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@mc/crates/mc-core/src/skills.rs` around lines 139 - 151, Move the test function discover_with_valid_skill into the existing test module guarded by #[cfg(test)] (i.e., inside mod tests) so it’s compiled only during tests; locate the discover_with_valid_skill test and the tests module wrapper (mod tests) and cut/paste the function body into that module so it resides alongside other tests that exercise discover_skills.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mc/crates/mc-cli/src/main.rs`:
- Around line 1434-1447: The remove handler currently joins the raw arg to
plugins_dir() and can be exploited for path traversal; before constructing dest
or calling std::fs::remove_dir_all, validate the plugin name (arg) to ensure
it's a simple basename: reject empty, any path separators ("/" or "\"), any
parent components like "..", and any absolute paths; you can use
Path::new(arg).components() or check for arg.contains(&['/','\\'][..]) and
ensure arg == Path::new(arg).file_name().and_then(|s|
s.to_str()).unwrap_or_default() to guarantee it’s a single safe filename; if
validation fails, push a clear error message and do not call remove_dir_all on
the joined path (references: plugins_dir(), the "remove" arm in main.rs, and
std::fs::remove_dir_all).
- Around line 1449-1474: The update command currently joins plugins_dir() with
user-provided arg and runs git in that dest without validating the path; mirror
the fix used for the remove command by validating arg/dest before running
Command::new("git") — e.g., reject absolute paths or segments like ".." and/or
canonicalize both plugins_dir() and dest (using std::fs::canonicalize) and
ensure dest.starts_with(plugins_dir_canon) before executing the git pull in the
"update" match arm; return a user-facing error message if validation fails.
---
Outside diff comments:
In `@mc/crates/mc-core/src/skills.rs`:
- Around line 139-151: Move the test function discover_with_valid_skill into the
existing test module guarded by #[cfg(test)] (i.e., inside mod tests) so it’s
compiled only during tests; locate the discover_with_valid_skill test and the
tests module wrapper (mod tests) and cut/paste the function body into that
module so it resides alongside other tests that exercise discover_skills.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: af82a09f-452a-4815-9882-4c82ca01e158
📒 Files selected for processing (4)
mc/crates/mc-cli/src/main.rsmc/crates/mc-core/src/skills.rsmc/crates/mc-tui/src/app.rsmc/crates/mc-tui/src/commands.rs
Comment on lines
+1434
to
+1447
| "remove" => { | ||
| if arg.is_empty() { | ||
| output.push("Usage: /plugin remove <name>".into()); | ||
| return; | ||
| } | ||
| let dest = plugins_dir().join(arg); | ||
| if !dest.exists() { | ||
| output.push(format!("Plugin '{arg}' not found.")); | ||
| return; | ||
| } | ||
| match std::fs::remove_dir_all(&dest) { | ||
| Ok(()) => output.push(format!("✅ Removed '{arg}'")), | ||
| Err(e) => output.push(format!("❌ Remove failed: {e}")), | ||
| } |
There was a problem hiding this comment.
Path traversal vulnerability in /plugin remove.
The arg is joined directly to plugins_dir() without sanitization. A malicious input like ../../../important_dir would delete arbitrary directories outside the plugins folder.
🔒 Proposed fix: Validate plugin name contains no path separators
"remove" => {
if arg.is_empty() {
output.push("Usage: /plugin remove <name>".into());
return;
}
+ // Prevent path traversal
+ if arg.contains('/') || arg.contains('\\') || arg.contains("..") {
+ output.push("❌ Invalid plugin name".into());
+ return;
+ }
let dest = plugins_dir().join(arg);
if !dest.exists() {
output.push(format!("Plugin '{arg}' not found."));
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-cli/src/main.rs` around lines 1434 - 1447, The remove handler
currently joins the raw arg to plugins_dir() and can be exploited for path
traversal; before constructing dest or calling std::fs::remove_dir_all, validate
the plugin name (arg) to ensure it's a simple basename: reject empty, any path
separators ("/" or "\"), any parent components like "..", and any absolute
paths; you can use Path::new(arg).components() or check for
arg.contains(&['/','\\'][..]) and ensure arg ==
Path::new(arg).file_name().and_then(|s| s.to_str()).unwrap_or_default() to
guarantee it’s a single safe filename; if validation fails, push a clear error
message and do not call remove_dir_all on the joined path (references:
plugins_dir(), the "remove" arm in main.rs, and std::fs::remove_dir_all).
Comment on lines
+1449
to
+1474
| "update" => { | ||
| if arg.is_empty() { | ||
| output.push("Usage: /plugin update <name>".into()); | ||
| return; | ||
| } | ||
| let dest = plugins_dir().join(arg); | ||
| if !dest.exists() { | ||
| output.push(format!("Plugin '{arg}' not found.")); | ||
| return; | ||
| } | ||
| match std::process::Command::new("git") | ||
| .args(["pull", "--ff-only"]) | ||
| .current_dir(&dest) | ||
| .output() | ||
| { | ||
| Ok(o) if o.status.success() => { | ||
| let out = String::from_utf8_lossy(&o.stdout); | ||
| output.push(format!("✅ Updated '{arg}': {}", out.trim())); | ||
| } | ||
| Ok(o) => { | ||
| let err = String::from_utf8_lossy(&o.stderr); | ||
| output.push(format!("❌ Update failed: {}", err.trim())); | ||
| } | ||
| Err(e) => output.push(format!("❌ git pull failed: {e}")), | ||
| } | ||
| } |
There was a problem hiding this comment.
Same path traversal vulnerability in /plugin update.
Apply the same validation as suggested for remove to prevent git pull from being executed in arbitrary directories.
🔒 Proposed fix
"update" => {
if arg.is_empty() {
output.push("Usage: /plugin update <name>".into());
return;
}
+ if arg.contains('/') || arg.contains('\\') || arg.contains("..") {
+ output.push("❌ Invalid plugin name".into());
+ return;
+ }
let dest = plugins_dir().join(arg);📝 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.
Suggested change
| "update" => { | |
| if arg.is_empty() { | |
| output.push("Usage: /plugin update <name>".into()); | |
| return; | |
| } | |
| let dest = plugins_dir().join(arg); | |
| if !dest.exists() { | |
| output.push(format!("Plugin '{arg}' not found.")); | |
| return; | |
| } | |
| match std::process::Command::new("git") | |
| .args(["pull", "--ff-only"]) | |
| .current_dir(&dest) | |
| .output() | |
| { | |
| Ok(o) if o.status.success() => { | |
| let out = String::from_utf8_lossy(&o.stdout); | |
| output.push(format!("✅ Updated '{arg}': {}", out.trim())); | |
| } | |
| Ok(o) => { | |
| let err = String::from_utf8_lossy(&o.stderr); | |
| output.push(format!("❌ Update failed: {}", err.trim())); | |
| } | |
| Err(e) => output.push(format!("❌ git pull failed: {e}")), | |
| } | |
| } | |
| "update" => { | |
| if arg.is_empty() { | |
| output.push("Usage: /plugin update <name>".into()); | |
| return; | |
| } | |
| if arg.contains('/') || arg.contains('\\') || arg.contains("..") { | |
| output.push("❌ Invalid plugin name".into()); | |
| return; | |
| } | |
| let dest = plugins_dir().join(arg); | |
| if !dest.exists() { | |
| output.push(format!("Plugin '{arg}' not found.")); | |
| return; | |
| } | |
| match std::process::Command::new("git") | |
| .args(["pull", "--ff-only"]) | |
| .current_dir(&dest) | |
| .output() | |
| { | |
| Ok(o) if o.status.success() => { | |
| let out = String::from_utf8_lossy(&o.stdout); | |
| output.push(format!("✅ Updated '{arg}': {}", out.trim())); | |
| } | |
| Ok(o) => { | |
| let err = String::from_utf8_lossy(&o.stderr); | |
| output.push(format!("❌ Update failed: {}", err.trim())); | |
| } | |
| Err(e) => output.push(format!("❌ git pull failed: {e}")), | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-cli/src/main.rs` around lines 1449 - 1474, The update command
currently joins plugins_dir() with user-provided arg and runs git in that dest
without validating the path; mirror the fix used for the remove command by
validating arg/dest before running Command::new("git") — e.g., reject absolute
paths or segments like ".." and/or canonicalize both plugins_dir() and dest
(using std::fs::canonicalize) and ensure dest.starts_with(plugins_dir_canon)
before executing the git pull in the "update" match arm; return a user-facing
error message if validation fails.
Plugin marketplace for mc-code. Plugins are git repos cloned to .magic-code/plugins/. Skills from plugins auto-discovered on startup. Commands: /plugin install obra/superpowers — clone from GitHub /plugin list — show installed plugins /plugin update superpowers — git pull latest /plugin remove superpowers — delete plugin Compatible with obra/superpowers (14 skills) and any repo with a skills/ directory containing SKILL.md files.
kienbui1995
force-pushed
the
feat/plugin-marketplace
branch
from
6ddb1f1 to
3256cc2
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@mc/crates/mc-cli/src/main.rs`:
- Around line 1377-1410: The extracted plugin name (variable name) can be a path
traversal vector (e.g., "..") so validate/sanitize it before building dest =
plugins_dir().join(name): after computing name from arg, reject or normalize
names that are empty, equal to "." or "..", contain path separators ('/' or '\')
or parent components, or contain unsafe characters; accept only a safe pattern
(e.g., alphanumeric, hyphen, underscore) and if validation fails push a
user-facing error and return. Locate this logic around the "/plugin install"
branch where arg, name and dest are used and add the check before creating dest
or running git clone.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5e864ad0-155e-4245-b869-0da89856aca3
📒 Files selected for processing (4)
mc/crates/mc-cli/src/main.rsmc/crates/mc-core/src/skills.rsmc/crates/mc-tui/src/app.rsmc/crates/mc-tui/src/commands.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- mc/crates/mc-tui/src/commands.rs
- mc/crates/mc-core/src/skills.rs
Comment on lines
+1377
to
+1410
| "install" => { | ||
| if arg.is_empty() { | ||
| output.push("Usage: /plugin install <github-url-or-owner/repo>".into()); | ||
| output.push("Example: /plugin install obra/superpowers".into()); | ||
| return; | ||
| } | ||
| let url = if arg.contains("://") { | ||
| arg.to_string() | ||
| } else { | ||
| format!("https://github.com/{arg}.git") | ||
| }; | ||
| let name = arg.rsplit('/').next().unwrap_or(arg).trim_end_matches(".git"); | ||
| let dest = plugins_dir().join(name); | ||
| if dest.exists() { | ||
| output.push(format!("Plugin '{name}' already installed. Use /plugin update {name}")); | ||
| return; | ||
| } | ||
| output.push(format!("📦 Installing {name}...")); | ||
| match std::process::Command::new("git") | ||
| .args(["clone", "--depth", "1", &url, &dest.to_string_lossy()]) | ||
| .output() | ||
| { | ||
| Ok(o) if o.status.success() => { | ||
| let skills = count_plugin_skills(&dest); | ||
| output.push(format!("✅ Installed '{name}' ({skills} skills)")); | ||
| output.push("Restart session to activate.".into()); | ||
| } | ||
| Ok(o) => { | ||
| let err = String::from_utf8_lossy(&o.stderr); | ||
| output.push(format!("❌ Install failed: {}", err.trim())); | ||
| } | ||
| Err(e) => output.push(format!("❌ git clone failed: {e}")), | ||
| } | ||
| } |
There was a problem hiding this comment.
Path traversal vulnerability in /plugin install.
The name extraction at line 1388 doesn't prevent path traversal. If arg is .. or foo/.., then name becomes .., and dest = plugins_dir().join("..") would point to .magic-code/ instead of .magic-code/plugins/<name>.
🔒 Proposed fix: Validate plugin name
let name = arg.rsplit('/').next().unwrap_or(arg).trim_end_matches(".git");
+ // Prevent path traversal
+ if name.is_empty() || name == "." || name == ".." || name.contains('/') || name.contains('\\') {
+ output.push("❌ Invalid plugin name".into());
+ return;
+ }
let dest = plugins_dir().join(name);🧰 Tools
🪛 GitHub Actions: CI
[error] 1385-1385: cargo fmt --all -- --check failed (formatting diff detected). Please run cargo fmt to apply the suggested formatting changes.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@mc/crates/mc-cli/src/main.rs` around lines 1377 - 1410, The extracted plugin
name (variable name) can be a path traversal vector (e.g., "..") so
validate/sanitize it before building dest = plugins_dir().join(name): after
computing name from arg, reject or normalize names that are empty, equal to "."
or "..", contain path separators ('/' or '\') or parent components, or contain
unsafe characters; accept only a safe pattern (e.g., alphanumeric, hyphen,
underscore) and if validation fails push a user-facing error and return. Locate
this logic around the "/plugin install" branch where arg, name and dest are used
and add the check before creating dest or running git clone.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Plugin marketplace for mc-code. Install skills from GitHub repos.
Usage
How it works
.magic-code/plugins/<name>/plugins/*/skills/auto-discovered alongside local skillsChanges
mc-tui: AddPlugin(String)command +/pluginhandlermc-cli:handle_plugin_command()— git clone/pull/rmmc-core/skills.rs: Scan.magic-code/plugins/*/skills/in discovery180 tests pass.
Summary by CodeRabbit
New Features
/plugincommand with install, list, remove, and update subcommands for plugin management.User Experience