Add file-conversion plugin (ChangeThisFile)#576
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (7)
✅ Files skipped from review due to trivial changes (4)
🚧 Files skipped from review as they are similar to previous changes (1)
📜 Recent review details🧰 Additional context used📓 Path-based instructions (1)README.md📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (1)📓 Common learnings🔇 Additional comments (2)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds a new file-conversion plugin: marketplace registrations across multiple platforms, Cursor/Claude/Codex platform manifests, SKILL.md documentation, and a Bash CLI script (convert.sh) that calls the ChangeThisFile MCP JSON-RPC convert_file tool to convert files. Includes documentation and count updates reflecting the new plugin (84 to 85). Changesfile-conversion Plugin
Sequence DiagramsequenceDiagram
participant CLI as convert.sh
participant MCP as ChangeThisFile_MCP
participant Host as DownloadHost
CLI->>MCP: POST JSON-RPC convert_file (base64 input, target format)
MCP-->>CLI: 200 OK with job.download_url
CLI->>Host: GET job.download_url
Host-->>CLI: 200 OK with converted file
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
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.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude-plugin/marketplace.json:
- Line 16: Revert all edits to the marketplace registry: remove the new
"file-conversion" plugin entry and undo the character-encoding normalizations
you added (the changes around the "file-conversion" entry and the other encoding
edits referenced in the review). In short, restore
.claude-plugin/marketplace.json to its original state by discarding the insert
of the "file-conversion" entry and reversing any byte/encoding tweaks so the
registry file remains untouched by this PR.
In @.cursor-plugin/marketplace.json:
- Around line 453-464: The .cursor-plugin files were hand-edited (see the
"file-conversion" entry in marketplace.json and the plugin at
./plugins/file-conversion); undo those manual changes by restoring the generated
state and regenerate the plugin metadata using the generator: remove your edits
to .cursor-plugin/marketplace.json and
.cursor-plugin/plugins/file-conversion.json, then run make generate HARNESS=<x>
(using the appropriate HARNESS value) to recreate the marketplace and plugin
JSON files so they match the canonical generated output.
In `@plugins/file-conversion/skills/file-conversion/scripts/convert.sh`:
- Line 12: The script uses a user-controlled OUT variable in convert.sh (IN,
TARGET, OUT) for file operations, which allows path traversal; fix by validating
and normalizing OUT before any writes: reject absolute paths (starting with /),
reject any path containing '..' segments, resolve the path (e.g., with realpath
--canonicalize or equivalent) and ensure it is inside an allowed output
directory (create/ensure a safe output base directory), then use the
validated/normalized path for all places referencing OUT (the initial assignment
and subsequent file writes at the places that consume OUT).
- Line 26: The BASENAME variable is only stripping quotes and backslashes and
can still contain control characters or other JSON-breaking characters when
embedded into the inline JSON payload at the payload construction (around lines
37-38); fix by building the JSON payload with a proper JSON serializer such as
jq (e.g. use jq --arg filename "$BASENAME" to create the object) or, if jq is
unavailable, implement a robust JSON-string escaper for BASENAME before
embedding it; update the payload construction to use the serialized/escaped
value instead of interpolating BASENAME directly so filenames with newlines,
control chars, or other metacharacters are safely encoded.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 491a3d80-82f4-4bd2-a353-7d883404d7dc
📒 Files selected for processing (8)
.agents/plugins/marketplace.json.claude-plugin/marketplace.json.cursor-plugin/marketplace.json.cursor-plugin/plugins/file-conversion.jsonplugins/file-conversion/.claude-plugin/plugin.jsonplugins/file-conversion/.codex-plugin/plugin.jsonplugins/file-conversion/skills/file-conversion/SKILL.mdplugins/file-conversion/skills/file-conversion/scripts/convert.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (5)
**/.cursor-plugin/**
📄 CodeRabbit inference engine (.cursor/rules/000-project-conventions.mdc)
Do not hand-edit anything under
.cursor-plugin/— regenerate viamake generate HARNESS=<x>
Files:
.cursor-plugin/plugins/file-conversion.json.cursor-plugin/marketplace.json
plugins/**/{.claude-plugin/plugin.json,agents/*.md,skills/*/SKILL.md,commands/*.md}
📄 CodeRabbit inference engine (.cursor/rules/000-project-conventions.mdc)
Plugin files follow the structure:
plugins/<name>/{.claude-plugin/plugin.json, agents/*.md, skills/<name>/SKILL.md, commands/*.md}and are auto-discovered
Files:
plugins/file-conversion/skills/file-conversion/SKILL.mdplugins/file-conversion/.claude-plugin/plugin.json
plugins/*/skills/*/SKILL.md
📄 CodeRabbit inference engine (.cursor/rules/020-agent-skill-authoring.mdc)
plugins/*/skills/*/SKILL.md: Skills frontmatter must include:name,description. Other Anthropic SKILL.md fields are optional and respected only on Claude Code
Cap skill body at approximately 8 KB. Push longer reference material intoskills/<name>/references/files, as Codex will hard-truncate at 8 KBSkill files must follow SKILL.md format and be located at
plugins/*/skills/<n>/SKILL.mdfor cross-harness discovery
Files:
plugins/file-conversion/skills/file-conversion/SKILL.md
{plugins/*/agents/*.md,plugins/*/skills/*/SKILL.md,plugins/*/commands/*.md}
📄 CodeRabbit inference engine (.cursor/rules/020-agent-skill-authoring.mdc)
{plugins/*/agents/*.md,plugins/*/skills/*/SKILL.md,plugins/*/commands/*.md}: Use action verbs in markdown body, not tool-name vocabulary: prefer "open the file" over "use the Read tool". The adapter rewrites a conservative set, but unrewritten cases may bleed through
Do not referenceTodoWrite, theTaskspawn tool, or hooks as load-bearing — they don't exist in Codex or Cursor
Files:
plugins/file-conversion/skills/file-conversion/SKILL.md
.claude-plugin/marketplace.json
📄 CodeRabbit inference engine (.cursor/rules/000-project-conventions.mdc)
Never modify
.claude-plugin/marketplace.jsonfrom inside a plugin PR — it's the registry
Files:
.claude-plugin/marketplace.json
🪛 Shellcheck (0.11.0)
plugins/file-conversion/skills/file-conversion/scripts/convert.sh
[info] 26-26: Want to escape a single quote? echo 'This is how it'''s done'.
(SC1003)
🔇 Additional comments (4)
.agents/plugins/marketplace.json (1)
484-495: LGTM!plugins/file-conversion/.claude-plugin/plugin.json (1)
1-12: LGTM!plugins/file-conversion/.codex-plugin/plugin.json (1)
1-18: LGTM!plugins/file-conversion/skills/file-conversion/SKILL.md (1)
1-56: LGTM!
| "name": "documentation-standards", | ||
| "source": "./plugins/documentation-standards", | ||
| "description": "HADS (Human-AI Document Standard) \u2014 semantic tagging convention for writing docs that work efficiently for both humans and AI models. Reduces token consumption by separating machine-critical facts from human context.", | ||
| "description": "HADS (Human-AI Document Standard) — semantic tagging convention for writing docs that work efficiently for both humans and AI models. Reduces token consumption by separating machine-critical facts from human context.", |
There was a problem hiding this comment.
Guideline violation: .claude-plugin/marketplace.json must not be modified from inside a plugin PR.
This file includes both the new file-conversion plugin entry (lines 52-64) and character encoding normalizations (lines 16, 929, 938, 1061, 1074, 1144). As per coding guidelines, ".claude-plugin/marketplace.json: Never modify .claude-plugin/marketplace.json from inside a plugin PR — it's the registry".
Remove all changes to .claude-plugin/marketplace.json from this PR. The marketplace registry should be updated separately by maintainers.
As per coding guidelines: ".claude-plugin/marketplace.json: Never modify .claude-plugin/marketplace.json from inside a plugin PR — it's the registry".
Also applies to: 52-64, 929-929, 938-938, 1061-1061, 1074-1074, 1144-1144
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.claude-plugin/marketplace.json at line 16, Revert all edits to the
marketplace registry: remove the new "file-conversion" plugin entry and undo the
character-encoding normalizations you added (the changes around the
"file-conversion" entry and the other encoding edits referenced in the review).
In short, restore .claude-plugin/marketplace.json to its original state by
discarding the insert of the "file-conversion" entry and reversing any
byte/encoding tweaks so the registry file remains untouched by this PR.
Source: Coding guidelines
| { | ||
| "name": "file-conversion", | ||
| "source": "./plugins/file-conversion", | ||
| "version": "1.0.0", | ||
| "description": "Convert files between 999 routes \u2014 PDF to Word, HEIC to JPG, MP4 to MP3, CSV to JSON, EPUB to MOBI \u2014 via the free ChangeThisFile API. MCP-aware with a zero-dependency script fallback; no API key required.", | ||
| "author": { | ||
| "name": "Aadil Razvi", | ||
| "email": "" | ||
| }, | ||
| "homepage": "https://github.com/aadilr/changethisfile-mcp", | ||
| "license": "MIT" | ||
| }, |
There was a problem hiding this comment.
Guideline violation: Multiple files under .cursor-plugin/** should not be hand-edited.
Both .cursor-plugin/marketplace.json and .cursor-plugin/plugins/file-conversion.json contain manual edits. As per coding guidelines, "Do not hand-edit anything under .cursor-plugin/ — regenerate via make generate HARNESS=<x>".
Remove all manual changes to files under .cursor-plugin/** and regenerate them using the appropriate make generate command.
As per coding guidelines: "**/.cursor-plugin/**: Do not hand-edit anything under .cursor-plugin/ — regenerate via make generate HARNESS=<x>".
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.cursor-plugin/marketplace.json around lines 453 - 464, The .cursor-plugin
files were hand-edited (see the "file-conversion" entry in marketplace.json and
the plugin at ./plugins/file-conversion); undo those manual changes by restoring
the generated state and regenerate the plugin metadata using the generator:
remove your edits to .cursor-plugin/marketplace.json and
.cursor-plugin/plugins/file-conversion.json, then run make generate HARNESS=<x>
(using the appropriate HARNESS value) to recreate the marketplace and plugin
JSON files so they match the canonical generated output.
Source: Coding guidelines
- .claude-plugin/marketplace.json: revert incidental unicode-escape normalizations on unrelated plugin entries; keep only the file-conversion entry (ensure_ascii to match the registry's existing \uXXXX style). - .cursor-plugin/** + .agents/: regenerated via `make generate-all` instead of hand-editing the generated artifacts (make validate: OK, 5 harnesses). - convert.sh: validate target format (alnum only) and reject '..' in the output path (was a path-traversal risk on `curl -o $OUT`); build the JSON-RPC payload with jq so arbitrary filenames are escaped safely (quotes, backslashes, newlines, control chars), with a strict-sanitization fallback when jq is absent.
|
Thanks for the review — addressed all four findings in 6cc85c9:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/file-conversion/skills/file-conversion/scripts/convert.sh`:
- Around line 35-39: The path validation in convert.sh currently only rejects
parent-directory segments via the case for OUT but still allows absolute paths;
update the OUT validation (the case block that tests "$OUT") to also reject
paths beginning with '/' (and optionally '~') so only relative non-traversing
paths are accepted, emitting the same error message and exit code; locate the
case handling of OUT in convert.sh and add a pattern like /* (and ~* if desired)
to the list of rejected patterns.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ec809653-218f-4aa1-9585-f9671990f3fd
📒 Files selected for processing (2)
.claude-plugin/marketplace.jsonplugins/file-conversion/skills/file-conversion/scripts/convert.sh
📜 Review details
🧰 Additional context used
📓 Path-based instructions (1)
.claude-plugin/marketplace.json
📄 CodeRabbit inference engine (.cursor/rules/000-project-conventions.mdc)
Never modify
.claude-plugin/marketplace.jsonfrom inside a plugin PR — it's the registry
Files:
.claude-plugin/marketplace.json
🪛 Shellcheck (0.11.0)
plugins/file-conversion/skills/file-conversion/scripts/convert.sh
[info] 56-56: Want to escape a single quote? echo 'This is how it'''s done'.
(SC1003)
🔇 Additional comments (4)
.claude-plugin/marketplace.json (1)
1150-1163: Guideline violation: registry file modified from plugin PR.The previous review already flagged this issue. As per coding guidelines,
.claude-plugin/marketplace.jsonmust not be modified from inside a plugin PR — it's the registry. All changes to this file (the newfile-conversionentry at lines 1150-1163 and unicode normalizations throughout) should be removed from this PR.Source: Coding guidelines
plugins/file-conversion/skills/file-conversion/scripts/convert.sh (3)
1-23: LGTM!
41-59: LGTM!
61-80: LGTM!
|
Thanks for a well-put-together submission — the regenerated registries are spot-on and CI is fully green. Two things before merge:
Happy to re-review after that. |
…unts Addresses review feedback on wshobson#576: - convert.sh: feed the base64 payload to jq via --rawfile (temp file) instead of --arg, so inputs larger than ~96KB no longer hit the OS per-argument limit (MAX_ARG_STRLEN). Verified with a 164KB input (219KB base64). - convert.sh: reject absolute and home-relative (~) user-supplied output paths in addition to traversal, since OUT is written with curl -o. The auto-derived default still works for absolute inputs (validation only applies to an explicit output arg). - docs: add file-conversion to docs/plugins.md (Utilities, now 5) and bump the plugin count 84 -> 85 (83 local + 2 external) across README.md, AGENTS.md, ARCHITECTURE.md, docs/architecture.md, docs/plugins.md, docs/usage.md. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Both addressed:
Also hardened the output-path check to reject absolute/ |
2 participants
Adds a
file-conversionplugin under utilities — converts files between 999 routes (PDF↔Word, HEIC→JPG, MP4→MP3, CSV→JSON, EPUB→MOBI, and more across images, video, audio, documents, data, fonts, ebooks, archives) via the free ChangeThisFile API. No API key or signup.file-conversion— MCP-aware (useschangethisfile:convert_filewhen connected) with a zero-dependencycurlscript fallback (scripts/convert.sh), so it works in every harness this marketplace targets.com.changethisfile/mcp).make generate-allrun (adapters regenerated),make validate STRICT=1passes across all 5 harnesses;make gardenwarnings are all pre-existing in other plugins (SKILL_OVER_CODEX_CAP), none introduced — our SKILL.md body is well under the Codex cap.