mirror of
https://github.com/CherryHQ/cherry-studio.git
synced 2026-07-07 15:12:18 +08:00
refactor/code-cli
3 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
2b21908fab |
fix: sanitize subprocess call in run_eval.py (#14842)
## Summary Fix critical severity security issue in `resources/skills/skill-creator/scripts/run_eval.py`. ## Vulnerability | Field | Value | |-------|-------| | **ID** | V-001 | | **Severity** | CRITICAL | | **Scanner** | multi_agent_ai | | **Rule** | `V-001` | | **File** | `resources/skills/skill-creator/scripts/run_eval.py:85` | | **CWE** | CWE-78 | **Description**: Three Python scripts in the skill-creator pipeline invoke subprocess.Popen/run with shell=True and incorporate user-supplied CLI arguments into the command string without sanitization. When shell=True is used, the operating system shell interprets special characters (semicolons, pipes, backticks, dollar signs) as command separators and substitution operators, enabling an attacker to append arbitrary OS commands to any legitimate argument. ## Changes - `resources/skills/skill-creator/scripts/run_eval.py` - `resources/skills/skill-creator/scripts/improve_description.py` - `resources/skills/skill-creator/eval-viewer/generate_review.py` ## Verification - [x] Build passes - [x] Scanner re-scan confirms fix - [x] LLM code review passed --- *Automated security fix by [OrbisAI Security](https://orbisappsec.com)* Signed-off-by: orbisai0security <mediratta01.pally@gmail.com> |
||
|
|
0d7e13bd48 |
fix(skills): support agent-authored skills via skills tool init/register (#14184)
### What this PR does
Before this PR:
- Inside agent chat, the `skill-creator` skill could not actually
persist a new skill. The agent would write `SKILL.md` files to disk, but
the global skill DB row and `.claude/skills/` symlink were never
created, so authored skills silently failed to take effect.
- `SkillInstaller.install` was destructive when source and destination
resolved to the same path: the backup-and-rename step moved the source
out from under the subsequent copy, leaving no way to install from an
in-place source.
- Agents in the sidebar showed an unexplained purple `SOUL` badge next
to their name when Soul Mode was on, crowding out the agent name and
using internal jargon users could not understand.
After this PR:
- The claw MCP `skills` tool gains two new actions:
- `init name=<name>` — creates / resolves the absolute target directory
under the global Skills storage root and returns it to the agent.
- `register name=<name>` — registers the in-place directory through the
existing `installFromDirectory` pipeline and toggles the skill on for
the current session.
- `installSkill` (marketplace install) now also auto-toggles the skill
on, so an agent-driven install from chat is immediately usable without a
UI roundtrip.
- `SkillInstaller.install` short-circuits when src and dest resolve to
the same path.
- `SkillService.installSkillDir` detects in-place input under the Skills
root and preserves the existing basename as the folder name, avoiding
sanitize drift between caller-chosen names and `parseSkillMetadata`'s
derived filename.
- New public `SkillService.getSkillDirectory(name)` helper exposes the
resolved path so callers don't duplicate sanitize logic.
- `resources/skills/skill-creator/SKILL.md` gains a "Cherry Studio
workflow" preamble that overrides the Claude Code / Claude.ai packaging
steps and instructs the agent to use init/register. The original
sections are kept as-is.
- The `SOUL` badge is removed from the agent sidebar item along with the
now-dead `SoulTag` styled component and unused `isSoulModeEnabled`
import in `AgentItem.tsx`. The Soul Mode toggle and underlying feature
are unchanged — the toggle still lives in agent settings.
- `claw.test.ts`'s `skillService` mock is extended with `toggle` /
`installFromDirectory` / `getSkillDirectory` so the new install / init /
register actions have a working stub (fixes the CI failure on the first
push of this branch).
Fixes #
### Why we need it and why it was done in this way
The skill-creator skill is meant to let users author custom skills
directly from agent chat, but with no integration into `SkillService`
the result was always a silent failure — a `SKILL.md` sitting in some
directory that no part of the system knew about. Users saw the agent
confidently report "skill created" while the global skill list remained
empty. This is the fix for that broken end-to-end flow.
The SOUL badge removal is bundled into this hotfix because it landed on
the same review cycle and is a strict UI cleanup — reported as P0 by
Kenny / 郑克 with the same root cause as the rest of this PR's framing:
Soul Mode internals leaking into the user-facing surface.
The following tradeoffs were made:
- Extended the existing `skills` MCP tool with two additive actions
instead of introducing a new tool, keeping the agent's tool surface
small.
- Reused `installFromDirectory` rather than adding a parallel "register
in place" code path. This required two small robustness fixes
(src===dest no-op + in-place folder name detection) but keeps a single
install entry point.
- The skill is auto-enabled on register (and on marketplace install); an
agent-authored or agent-installed skill that requires a manual UI toggle
would surprise users who just told the agent "create / install a skill
for X".
- Just delete the SOUL badge instead of trying to make it discoverable
(tooltip / icon-only / rename). The badge was not pulling its weight in
the list item, and the settings panel is the right place for users to
learn about Soul Mode.
The following alternatives were considered:
- A new `registerInPlace` method on `SkillService` bypassing
`installer.install` entirely. Rejected to avoid duplicating insert/link
logic.
- A staging directory under `app.getPath('temp')` copied into the Skills
root on register. Rejected because it forces an extra copy and breaks
live iteration: with in-place files, the `.claude/skills/<name>/`
symlink reflects edits immediately, so no second `register` is needed
for runtime effect.
- A file watcher / per-turn reconciliation that auto-discovers
`SKILL.md` files in the agent workspace. Rejected as too magical for a
first cut; can revisit later.
- For the badge: keep but rename / translate. Rejected — the underlying
problem is that "Soul Mode" itself is internal-facing terminology, so
renaming the badge would only move the comprehension issue.
Links to places where the discussion took place: N/A
### Breaking changes
None. The new tool actions are additive and existing `search` /
`install` / `remove` / `list` behavior is unchanged. The
`SkillInstaller.install` short-circuit only fires when src and dest
resolve to the exact same path, which previously raised — strictly more
permissive than before. The badge removal is UI-only and the Soul Mode
feature is untouched.
### Special notes for your reviewer
- Functionally most of this PR fixes a silent failure ("agent-authored
skills don't take effect"), but it also introduces new MCP tool actions,
so the underlying commits have a mix of `feat:` and `fix:` prefixes. The
PR title uses `fix:` since it lands on a hotfix branch.
- The skill-creator SKILL.md change only **adds** a "Cherry Studio
workflow" preamble at the top; nothing else is removed, so upstream
changes to the rest of the file still merge cleanly.
- The SOUL badge removal commit was originally on a separate branch / PR
(#14185, now closed) and is cherry-picked into this branch so it can
ship in the same review cycle.
- The CI fix commit (`test(skills): add toggle mock to claw test ...`)
is here because the first push hit `skillService.toggle is not a
function` — the `installSkill` change calls toggle but the existing mock
didn't expose it.
- Manual testing: end-to-end verified by the author — agent runs `skills
init` → writes SKILL.md and supporting files → calls `skills register` →
the new skill appears in the global skill list and is enabled for the
session. Sidebar verified to no longer show the SOUL badge after the
cherry-pick.
### Checklist
- [x] PR: The PR description is expressive enough and will help future
contributors
- [x] Code: Write code that humans can understand and Keep it simple
- [x] Refactor: Boy Scout Rule applied — `SkillInstaller.install` is now
safer for any in-place caller, and AgentItem dropped the now-dead
`SoulTag` component along with the unused import
- [x] Upgrade: No upgrade impact (additive, no schema migration)
- [x] Documentation: skill-creator SKILL.md preamble updated; no
external docs change required
- [x] Self-review: Reviewed via local diff before pushing
### Release note
```release-note
fix(skills): agents in Soul Mode can now author new skills end-to-end from agent chat. The `skills` tool gains `init` / `register` actions, marketplace install auto-enables, and the skill-creator skill persists to the global skill list. UI cleanup: the unexplained SOUL badge next to agent names in the sidebar is removed; Soul Mode itself is unchanged and still toggleable from agent settings.
fix(agents): the skills and memory MCP tools are now available to every agent (not just Soul-Mode agents), with cross-session FACT.md recall, auto-approved permissions, and `skills list` exposing on-disk paths so the agent can patch installed skills in place via Read/Edit. Skill management (search/install/list/remove/init/register) and persistent workspace memory work in regular chat agents the same way they did in Soul Mode.
```
---------
Signed-off-by: suyao <sy20010504@gmail.com>
Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
||
|
|
a4f5191e42 |
feat(agents): add CherryClaw autonomous agent type (#13359)
### What this PR does Before this PR: Cherry Studio only supports `claude-code` as an agent type. Agents have no autonomous scheduling, no IM channel integration, and no soul/personality system. After this PR: Introduces **CherryClaw** — a new autonomous agent type with: - **Soul-driven personality**: Markdown-based soul files with mtime-cached reading - **Task-based scheduler**: Poll-loop scheduler with drift-resistant interval computation, tasks as first-class DB entities (nanoclaw-inspired) - **Internal claw MCP server**: `cron` tool (add/list/remove) auto-injected into CherryClaw sessions for autonomous task management - **Channel abstraction layer**: Pluggable adapter pattern with Telegram as the first implementation (grammY, long polling, streaming drafts, typing indicators) - **Headless message persistence**: Channel and scheduler messages now persist to the agent SQLite DB - **Basic sandbox mode**: PreToolUse hook path enforcement + OS-level sandbox toggle - **Full UI**: Agent creation modal with type selector, settings tabs (soul, tasks, channels, advanced), task management CRUD, channel catalog with inline config - **53 unit tests** across 8 test files covering all new services <!-- Fixes # --> ### Why we need it and why it was done in this way CherryClaw enables Cherry Studio agents to operate autonomously — executing scheduled tasks and responding to IM messages without user interaction. This is the foundation for "always-on" AI assistants. The following tradeoffs were made: - **Poll-loop scheduler over timer-based**: DB is the source of truth; no timer state to restore on restart. Simpler, more robust at the cost of up to 60s latency. - **AgentServiceRegistry pattern**: Replaced hardcoded `ClaudeCodeService` in `SessionMessageService` with a registry mapping `AgentType` → service. Extensible for future agent types. - **Internal MCP server for cron**: Rather than extending the SDK's tool system, the `cron` tool is served as a standard MCP server at `/v1/claw/:agentId/claw-mcp`. This lets the agent discover and use it naturally. - **Channel abstraction over direct Telegram integration**: `ChannelAdapter` + factory registration enables future Discord/Slack adapters without touching core routing logic. - **Basic sandbox (not security boundary)**: PreToolUse hook + OS sandbox provides best-effort restriction for well-behaved agents. Known bypass vectors documented; hardening deferred. The following alternatives were considered: - cron-based OS scheduling (rejected: harder to manage lifecycle, no DB integration) - Direct Telegram bot API calls (rejected: grammY provides typed API, connection management, and middleware) - Modifying SDK builtin tools (rejected: internal MCP server is cleaner separation) ### Breaking changes None. This is a new agent type (`cherry-claw`) alongside the existing `claude-code` type. No existing behavior is modified. ### Special notes for your reviewer - **New DB migration**: `0003_wise_meltdown.sql` adds `scheduled_tasks` and `task_run_logs` tables (agents DB only, not IndexedDB) - **New dependencies**: `cron-parser` ^5.5.0, `grammy` ^1.41 - **Placeholder avatar**: `cherry-claw.png` is currently a copy of `claude.png` — needs a proper distinct image - **74 files changed, ~7400 lines added** — large PR, recommend reviewing by phase (type system → backend services → MCP → channels → UI → tests) - **Sandbox is basic only**: The PreToolUse path checking has known bypasses (relative paths, variable expansion). Documented in handoff.md. Hardening is follow-up work. - The `handoff.md` file in the repo root contains full architectural context and decisions ### Checklist - [x] PR: The PR description is expressive enough and will help future contributors - [x] Code: [Write code that humans can understand](https://en.wikiquote.org/wiki/Martin_Fowler#code-for-humans) and [Keep it simple](https://en.wikipedia.org/wiki/KISS_principle) - [x] Refactor: You have [left the code cleaner than you found it (Boy Scout Rule)](https://learning.oreilly.com/library/view/97-things-every/9780596809515/ch08.html) - [ ] Upgrade: Impact of this change on upgrade flows was considered and addressed if required - [ ] Documentation: A [user-guide update](https://docs.cherry-ai.com) was considered and is present (link) or not required. Check this only when the PR introduces or changes a user-facing feature or behavior. - [ ] Self-review: I have reviewed my own code (e.g., via [`/gh-pr-review`](/.claude/skills/gh-pr-review/SKILL.md), `gh pr diff`, or GitHub UI) before requesting review from others ### Release note ```release-note New CherryClaw agent type: autonomous agents with soul-driven personality, task-based scheduling (cron/interval/one-time), internal cron MCP tool for self-managed tasks, Telegram channel integration with streaming responses, and basic sandbox mode for filesystem restriction. ``` --------- Signed-off-by: Vaayne <liu.vaayne@gmail.com> Signed-off-by: suyao <sy20010504@gmail.com> Signed-off-by: Siin Xu <31815270+SiinXu@users.noreply.github.com> Signed-off-by: zhangjiadi225 <625013594@qq.com> Signed-off-by: greycheng255 <greycheng255@gmail.com> Co-authored-by: kangfenmao <kangfenmao@qq.com> Co-authored-by: suyao <sy20010504@gmail.com> Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Siin Xu <31815270+SiinXu@users.noreply.github.com> Co-authored-by: fullex <106392080+0xfullex@users.noreply.github.com> Co-authored-by: zhangjiadi225 <625013594@qq.com> |