feat: skill injection for recursive spawn#2989
Conversation
5df6360 to
2a1ca36
Compare
louisgv
left a comment
There was a problem hiding this comment.
Security Review
Verdict: APPROVED (with advisory)
Commit: 2a1ca36
Findings
MEDIUM packages/cli/src/shared/spawn-skill.ts:119-120 — Potential command injection via unescaped shell variables
The remotePath and remoteDir variables are interpolated directly into shell commands without escaping. Currently safe because:
- Values come from hardcoded
SKILL_REMOTE_PATHSconstant getSpawnSkillPath()returns undefined for unknown agents- Early return on line 102-105 prevents execution for undefined paths
Risk: If the codebase evolves to accept dynamic agent names from external input, this becomes a critical vulnerability. Consider using shell escaping (escapeShellArg()) or array-based command builders in future iterations.
Positive findings:
- Base64 validation (line 108-110) prevents injection via encoded content
- Correct use of
printf '%s'to avoid escape sequence interpretation - Proper error handling with
asyncTryCatchIf - Comprehensive test coverage (52 tests, all pass)
Tests
- bash -n: N/A (no shell scripts modified)
- bun test: PASS (52/52 tests pass)
- curl|bash: N/A (no bash scripts)
- macOS compat: N/A
Summary
Feature adds skill injection for recursive spawn with proper beta gating. Code quality is high with extensive test coverage. The command injection risk is acceptable given current constraints but should be noted for future refactoring.
-- security/pr-reviewer
When `--beta recursive` is active, a new "Spawn CLI" setup step injects agent-native instruction files teaching each agent how to use the `spawn` CLI to create child VMs. Skill files live in `skills/` at the repo root and use each agent's native format (YAML frontmatter for Claude/Codex/ OpenClaw, plain markdown for others, append mode for Hermes). - Add `skills/` directory with 8 agent-specific skill files - Add `spawn-skill.ts` module with path mapping, file reading, and injection - Register "spawn" as a conditional setup step gated by `--beta recursive` - Wire `injectSpawnSkill()` into orchestrate.ts postInstall flow - Add 52 tests covering path mapping, append mode, file existence, injection - Bump CLI version to 0.26.0 (minor: new feature) Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2a1ca36 to
ccc951f
Compare
Summary
--beta recursiveis active, a new "Spawn CLI" setup step writes agent-native instruction files on the remote VM teaching each agent how to usespawnto create child VMsskills/at repo root — Claude/Codex/OpenClaw use YAML frontmatter (SKILL.md), ZeroClaw/OpenCode/Junie use plain markdown (AGENTS.md), Kilo Code uses rules format, Hermes appends to SOUL.mdspawn-skill.tshandles path mapping, file reading, base64 encoding, and remote injection--beta recursive— the spawn step only appears in the setup picker when the beta flag is active, and is pre-selected (defaultOn: true)Test plan
bunx @biomejs/biome check src/— zero errorsbun test src/__tests__/spawn-skill.test.ts— 52 pass, 0 failbun test— 2017 pass (1 pre-existing failure unrelated to this PR)spawn claude hetzner --beta recursive→ verify~/.claude/skills/spawn/SKILL.mdexists on VMspawn hermes hetzner --beta recursive→ verify SOUL.md has appended spawn section🤖 Generated with Claude Code