Security review: no permission gate on tool execution in interactive sessions, plus 4 related trust-boundary gaps

Summary

Ran a security review before adopting jcode (auth, telemetry, supply chain, deps, and the agent/tool-execution trust boundary). Most of it looks solid: PKCE OAuth, CSRF state, 0600/0700 credential file hardening, self-hosted telemetry with an opt-out that's actually enforced in code, clean dependency pins. Good work there.

The area that actually worries me is the tool-execution / agent trust boundary, basically what stands between a model deciding to run something and it actually happening. Filing this as one issue since the findings are related, happy to split if you'd rather. Opening a PR from a fork shortly with fixes for all five.

Findings

1. No permission/confirmation gate on tool execution in normal interactive sessions
crates/jcode-app-core/src/agent/turn_loops.rs calls registry.execute(...) the moment the model emits a tool call (bash/write/edit/patch), no confirmation step. The only human-in-the-loop system (SafetySystem in jcode-base/src/safety.rs, plus jcode-tui-permissions) is wired exclusively into "ambient" background mode. docs/SAFETY_SYSTEM.md even says so itself: "Currently the only consumer is ambient mode," and it's marked Status: Design.

So a prompt injection from a fetched webpage, a README, or MCP tool output can get shell commands run with the user only seeing it after the fact. This is a big departure from how comparable tools handle it (approval by default, explicit opt-in to skip it).

2. Path traversal in file write/edit tools
ToolContext::resolve_path (jcode-tool-core/src/lib.rs) just joins relative paths onto working_dir, no canonicalization or bounds check, and passes absolute paths through as-is. Used directly by WriteTool, EditTool, PatchTool. Combine with #1 and a model-supplied path like ../../../.ssh/authorized_keys (or any absolute path) gets honored no questions asked.

3. MCP tool output fed back to the model unsanitized
McpTool::execute (jcode-base/src/mcp/tool.rs) forwards a server's raw content as the tool result with no tagging as external/untrusted. A malicious or compromised MCP server can just return injected instructions that land in context and, combined with #1, get acted on right away.

4. Agent can spawn arbitrary MCP server subprocesses, no confirmation
connect_server (jcode-app-core/src/tool/mcp.rs) lets the model call mcp connect with whatever command/args it wants, spawns a local subprocess, zero approval.

5. Swarm sub-agents have no privilege separation
crates/jcode-app-core/src/server/swarm.rs notes the spawn-tree depth cap isn't actually enforced in code anymore. Swarm members get the same unconfirmed tool registry (shell, files, MCP connect, further spawning) as the parent session.

Suggested direction

Wiring some form of confirmation gate into the default interactive tool-execution path (not just ambient mode) covers #1, and gives a natural enforcement point for #3/#4 (treat MCP-originated content/actions as needing confirmation) and #5 (route sub-agent calls through the same gate, actually enforce the depth cap). Note: the existing SafetySystem is built for async/unmonitored operation (review queue, email/SMS notifications) which doesn't map cleanly onto a synchronous interactive session, so this probably wants a simpler, separate synchronous confirm-prompt path rather than reusing SafetySystem directly. #2 is independent, just needs canonicalization plus a working-directory bounds check in resolve_path.

Happy to talk through design tradeoffs before or on the PR.

Start by reading crates/jcode-app-core/src/agent/turn_loops.rs, jcode-tool-core/src/lib.rs, jcode-base/src/mcp/tool.rs, crates/jcode-app-core/src/tool/mcp.rs, and crates/jcode-app-core/src/server/swarm.rs, along with docs/SAFETY_SYSTEM.md. Trace interactive tool execution first, then verify how path resolution, MCP results and connections, and swarm spawning are handled. Done means all five trust-boundary gaps have explicit safeguards and the relevant behavior is covered by tests.