Skip to content

fix(minimax-bot): 声明了的值就照声明校验,名字写在目录外的头像不再被读进来 - #95

Merged
MyPrototypeWhat merged 2 commits into
MiniMax-AI:mainfrom
Apageoflove:fix/minimax-bot-entry-validation
Oct 29, 2026
Merged

MyPrototypeWhat merged 2 commits into
MiniMax-AI:mainfrom
Apageoflove:fix/minimax-bot-entry-validation

Conversation

@Apageoflove

@Apageoflove Apageoflove commented Oct 29, 2026 •

Copy link
Copy Markdown
Contributor

有几种值,这个插件在清单里写清了它收什么,却并不真的去收:一个定时任务的 kind 不是 daily、interval 或 once 时,它被悄悄换成 daily 并且真的建出来;一个群的 reply_mode 不是 admin 或 all 时,它被悄悄换成 admin,把一个本来人人发言的群变成只有管理员发言;用量面板的天数传成 abc 时,它给一张空的图,看着像额度被清空了。MCP 工具也不校验自己声明的入参:一个不认识的记忆 scope 会让它崩在未捕获的 TypeError 上,并把内部报错的原文回给模型;头像字段则更宽:任何绝对路径都能被读进来、复制进自己的图库、再原样提供出去,于是这个字段可以拿来问磁盘上有什么。
这些都在未改的上游代码上用探针复现过。改法是照声明来收:kind、reply_mode 和用量天数在入口被拒,群里原来的设置和已经记下的内容都不动;MCP 工具在跑之前先看自己声明的入参,类型不对的当场拒掉,什么都不改,而工具自己能答的枚举(记忆范围、回复模式、任务类型)交给工具自己答——它说得清范围和这个值会改什么,那是 schema 说不了的;不是插件自己的错误只回一句通用提示,细节写进日志,不再进模型的上下文。头像只收图库里的文件名或者页面上传的图片,本地路径一律被拒,插件不会为了它读数据目录以外的任何文件,被拒的路径也不会原样回显。
在装好的候选版上直接打插件自己的接口,同一组调用前后是反的。候选版:定时任务的 kind=weekly 回来的是 kind must be one of: daily, interval, once,没有落库;建群的 reply_mode=ALL 回来的是 reply_mode must be one of: admin, all,没有落库;记忆的 scope=all 回来的是 Invalid memory scope "all"; it must be one of self, team, bot;头像给一个数据目录以外的绝对路径被拒,图库里没有多出文件;用量的天数 days=abc 回来的是 days must be a whole number from 1 to 90。0.7.13 上同样这五个调用:定时任务真的建了出来而且 kind 变成 daily,回复模式变成 admin,记忆被悄悄写进了调用方自己的记忆区,那个 PNG 被读进来、复制进图库、再原样提供出来(吐回来的字节与源文件相同),天数给了一张空图。
在 MiniMax Code 里问同样的非法值,宿主自己会先按工具声明的 schema 挡下来(kind: must be equal to one of the allowed values),所以这一版在客户端这一层是第二道防线,它护住的是直接调插件接口的调用方;头像那一路宿主不拦,插件自己拒了。新增的测试文件在未改的上游代码上是红的,在这版上是绿的;插件自带套件 310 条全过,上游 npm run check 两半也过。
这一版只动已经声明过的值的收法;记忆里 keyless 那两条入口的规则差异这次没有一起改,需要确认上游的取舍。


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need.

@MyPrototypeWhat MyPrototypeWhat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for this. Approving. I checked out 61ecec7 and ran the checks.

  • npm run check at the repo root: 87 pass, 0 fail (same as main).
  • node --test tests/*.test.mjs in plugins/minimax-ai/minimax-bot (node 22, jsdom installed): 310 tests, 306 pass, 4 fail. The 4 failures (1 reactions, 3 emoji picker) fail the same way on main at 45a5c6e, so this PR adds no new failures. The 31 new tests all pass.
  • Every change stays inside the plugin folder, and the new tests are in the plugin's own tests/ directory. No CI, .github or root test files are touched.

Nothing blocks merging. Notes on compatibility:

  • Stored data: the new checks only run on incoming create/update requests (createGroup, updateGroup, schedule upsert, memory scope, /api/usage). Nothing re-validates saved state on load. Saved schedules and groups could only ever hold daily/interval/once and admin/all before this change, so existing data keeps working. The page only sends days of 1/7/30/90 and only admin/all for the reply mode, so it is unaffected.
  • Avatars: a Bot's saved avatar is a gallery file name (path avatars were already copied into the gallery when they were set), so existing custom avatars keep showing. Closing the arbitrary-path read is a good privacy fix.
  • Hiding the message of unexpected errors from the model and logging it instead is a good change.

Optional follow-ups:

  1. A user can no longer ask a Bot to "use this picture on my disk as the avatar". The only ways now are the page upload or a file that is already in the gallery. That is a reasonable trade-off, but it is a visible behavior change, so it would help to mention it in the release notes and to have the tool error point the model at the page upload.
  2. update_group with reply_mode: null (and create_schedule with kind: null) used to fall back to the default. Now it passes the schema check, because null is skipped there, and is then rejected by replyModeOf / scheduleKindOf. Treating null like undefined in those two paths would keep the old lenient behavior for a missing value and still refuse wrong ones.
  3. The strict JSON type check now refuses numbers sent as strings (for example "30" for an integer field), which some models do. The error message tells the model how to fix it, so this is fine, but it is a behavior change worth noting.
  4. Version: this goes from 0.7.13 straight to 0.7.15, which assumes #94 (0.7.14) merges first. Merged on top of #94, the only conflicts are the version lines (package.json, plugin.json, serverInfo in mcp.mjs) and the neighboring README sections. After resolving them, all 338 plugin tests passed in my run. If this PR merges first instead, please bump it to 0.7.14 so the version does not skip.

@MyPrototypeWhat MyPrototypeWhat left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the rebase. Approving again. I re-checked cdd8643 after the merge with main (1aa1679).

  • The diff against main is the same change I approved at 61ecec7. The only differences are the version bump, now 0.7.14 to 0.7.15, and the README context.
  • Everything from 0.7.14 is still there: relay-guard.mjs, its tests and the README section.
  • package.json, plugin.json and the MCP serverInfo all say 0.7.15, and the README lists 0.7.14 then 0.7.15.
  • npm run check passes. Plugin tests: 334/338. The 4 failures (1 reactions, 3 emoji picker) already failed on main at 45a5c6e in my earlier run, so they're unrelated to this PR.
  • CI on this head passed, and the changes stay inside plugins/minimax-ai/minimax-bot.

@MyPrototypeWhat
MyPrototypeWhat merged commit de7d8a9 into MiniMax-AI:main Oct 29, 2026
1 check passed
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