feat(config): 支持项目级配置叠加 (.jcode/config.json) - #172
Conversation
Support multi-layer configuration merging: project-level <projectDir>/.jcode/config.json overlays the global ~/.jcode/config.json, following the pattern established by Codex (layered TOML with denylist) and Grok (user + project field-by-field merge). Merge semantics: - MCP servers: merged by name; project can add new servers or tune args/env/timeout of existing ones, but cannot override Command/URL (prevents redirecting a trusted server to a malicious binary) - Model, SmallModel, Theme, Language, ContextLimits: project overrides - DisabledSkills, DisabledProviders: union (project can only add) - Budget, Compaction, Prompt, Subagent, Browser, Computer, etc.: project replaces the whole block if set Security denylist (project config is silently ignored for): - Providers/Models (API keys, base URLs) - Telemetry (Langfuse secrets) - Cloud (relay credentials) - SSH/Docker aliases (remote access) - Memory, Developer, AutoApprove, DefaultMode (privilege escalation) Wired into all three transports (interactive TUI, web, ACP) including the MCP hot-reload path.
📝 WalkthroughWalkthroughProject configuration is loaded from ChangesProject configuration support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant LoadProjectConfig
participant MergeProjectConfig
participant SessionOrServer
Runtime->>LoadProjectConfig: load .jcode/config.json
LoadProjectConfig-->>Runtime: project configuration
Runtime->>MergeProjectConfig: merge permitted fields
MergeProjectConfig-->>Runtime: effective configuration
Runtime->>SessionOrServer: initialize or reload with effective configuration
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
internal/command/acp.go (1)
1188-1191: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTwo paths swallow the project-config load error. Both use the
projErr == nil && projCfg != nilshortcut, so a malformed.jcode/config.jsonis silently ignored and the session/reload proceeds on unmerged config — while the sibling startup paths (acp.goLines 372-376,interactive.goLines 1152-1156,web.goLines 218-222) log a warning.
internal/command/acp.go#L1188-L1191: split the condition and log[config] project config warning: %vonprojErr != nil, matchingNewSession.internal/command/interactive.go#L467-L470: same split inreloadMCP, logging under the existing[mcp] reload:prefix.🤖 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 `@internal/command/acp.go` around lines 1188 - 1191, The project-config loading paths silently discard errors. In internal/command/acp.go lines 1188-1191, split the LoadProjectConfig condition, log `[config] project config warning: %v` when projErr is non-nil, and merge only when projCfg is non-nil; apply the same split in interactive.go lines 467-470 within reloadMCP, logging failures with the existing `[mcp] reload:` prefix.
🤖 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 `@internal/command/web.go`:
- Around line 216-223: Add a helper that loads global config and merges the
project config for a supplied working directory, preserving the existing
security restrictions and warning behavior. Replace every direct
config.LoadConfig() call in buildWebTask, newChatModel, makeAgent,
toolSearchStats, and breakdownFn with this helper so all rebuild and task paths
consistently use project overrides.
In `@internal/config/project_test.go`:
- Around line 102-127: Update the MergeProjectConfig test to expect untrusted
project-supplied servers with new names to be skipped, removing the assertion
that github is added with its command. Add a separate test covering the trusted
path where a new server is accepted with its full definition, while preserving
the existing fs override assertions.
In `@internal/config/project.go`:
- Around line 145-150: Update the overlay merge logic around Browser and
Computer so project configuration cannot override capability toggles or
permission lists. Denylist both blocks entirely, or merge only non-capability
tuning fields while preserving the global Enabled values and browser/computer
permission lists; keep the existing protected handling for AutoApprove and
DefaultMode.
- Around line 89-101: Update the MCP server merge logic around mergeMCPServer so
servers introduced by project configuration are not added by default. Require an
explicit project-trust signal, following the existing JCODE_HOOKS_TRUST_PROJECT
pattern, before inserting new names; when trust is absent, log that the project
server was skipped and preserve existing user-configured servers and their merge
behavior.
---
Nitpick comments:
In `@internal/command/acp.go`:
- Around line 1188-1191: The project-config loading paths silently discard
errors. In internal/command/acp.go lines 1188-1191, split the LoadProjectConfig
condition, log `[config] project config warning: %v` when projErr is non-nil,
and merge only when projCfg is non-nil; apply the same split in interactive.go
lines 467-470 within reloadMCP, logging failures with the existing `[mcp]
reload:` prefix.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c8e340fa-3f05-4bbe-b8aa-1600dac5abe4
📒 Files selected for processing (5)
internal/command/acp.gointernal/command/interactive.gointernal/command/web.gointernal/config/project.gointernal/config/project_test.go
| // Merge project-level config (<pwd>/.jcode/config.json) over the global | ||
| // config. Security-sensitive fields are never taken from project config. | ||
| if projCfg, projErr := config.LoadProjectConfig(pwd); projErr != nil { | ||
| config.Logger().Printf("[config] project config warning: %v", projErr) | ||
| } else if projCfg != nil { | ||
| config.MergeProjectConfig(cfg, projCfg) | ||
| config.Logger().Printf("[config] merged project config from %s/.jcode/config.json", pwd) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
Project overrides are lost by every subsequent config.LoadConfig() in this file.
The overlay is applied only to this startup cfg. buildWebTask re-reads config from disk at Line 410 (taskCfg), as do makeAgent (Line 657), toolSearchStats (Line 802), breakdownFn (Line 934), and newChatModel (Line 318) — none re-merge. So per-task and per-rebuild behavior silently reverts to global values for DisabledSkills (Line 435), MemoryEnabled (Line 608), ToolSearchEnabled (Line 745), CompactionThreshold (Line 668), SetReviewerConfig (Line 470) and mempipeline.MaybeStartBackground (Line 521), while the startup-derived pieces keep the project values. Effective config then differs depending on which rebuild path ran.
Simplest fix: a small helper that wraps load+merge for a given pwd and use it at every one of these sites instead of config.LoadConfig().
🤖 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 `@internal/command/web.go` around lines 216 - 223, Add a helper that loads
global config and merges the project config for a supplied working directory,
preserving the existing security restrictions and warning behavior. Replace
every direct config.LoadConfig() call in buildWebTask, newChatModel, makeAgent,
toolSearchStats, and breakdownFn with this helper so all rebuild and task paths
consistently use project overrides.
| overlay := &Config{ | ||
| MCPServers: map[string]*MCPServer{ | ||
| // Override existing server args (allowed), but command is NOT overridable | ||
| "fs": {Command: "evil-binary", Args: []string{"-y", "@anthropic/mcp-fs", "--root", "/tmp"}}, | ||
| // Add new server (full definition allowed) | ||
| "github": {Command: "npx", Args: []string{"-y", "@anthropic/mcp-github"}}, | ||
| }, | ||
| } | ||
| MergeProjectConfig(base, overlay) | ||
|
|
||
| if len(base.MCPServers) != 2 { | ||
| t.Fatalf("mcp_servers len = %d, want 2", len(base.MCPServers)) | ||
| } | ||
| // Existing server: command preserved (NOT overridable), args overridden | ||
| fs := base.MCPServers["fs"] | ||
| if fs.Command != "npx" { | ||
| t.Errorf("fs.command = %q, want npx (project must not override command)", fs.Command) | ||
| } | ||
| if len(fs.Args) != 4 { | ||
| t.Errorf("fs.args len = %d, want 4", len(fs.Args)) | ||
| } | ||
| // New server added with full definition | ||
| gh := base.MCPServers["github"] | ||
| if gh == nil || gh.Command != "npx" { | ||
| t.Error("github server not merged correctly") | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
This test pins the unsafe "new server, full definition" behavior.
Lines 106-107 and 124-127 assert that a project-supplied server name is adopted with its own command. If the trust gate suggested on internal/config/project.go Lines 89-101 lands, this case should instead assert the server is skipped (and a separate test should cover the trusted path).
🤖 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 `@internal/config/project_test.go` around lines 102 - 127, Update the
MergeProjectConfig test to expect untrusted project-supplied servers with new
names to be skipped, removing the assertion that github is added with its
command. Add a separate test covering the trusted path where a new server is
accepted with its full definition, while preserving the existing fs override
assertions.
| // --- MCP servers: merge by name --- | ||
| if len(overlay.MCPServers) > 0 { | ||
| if base.MCPServers == nil { | ||
| base.MCPServers = make(map[string]*MCPServer, len(overlay.MCPServers)) | ||
| } | ||
| for name, srv := range overlay.MCPServers { | ||
| if existing := base.MCPServers[name]; existing != nil { | ||
| mergeMCPServer(existing, srv) | ||
| } else { | ||
| base.MCPServers[name] = srv | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🔴 Critical | 🏗️ Heavy lift
New MCP servers from project config are arbitrary code execution.
An existing server's Command/URL is protected (Line 182) precisely because project config is untrusted — but a new server name is inserted verbatim at Line 98 with whatever command/args/env the repo supplies, and every caller then feeds cfg.MCPServers to tools.LoadMCPTools. Cloning a hostile repo and running jcode in it executes that binary with no prompt. The comment at Lines 178-180 ("the user's explicit choice") doesn't hold for a file that shipped with the repository.
This repo already has the pattern for it: project hooks load only under JCODE_HOOKS_TRUST_PROJECT=1 (internal/command/interactive.go Lines 1266-1267). Gate project-added MCP servers behind an equivalent explicit trust signal (env var or per-project approval), and log-and-skip otherwise.
🔒 Sketch
for name, srv := range overlay.MCPServers {
if existing := base.MCPServers[name]; existing != nil {
mergeMCPServer(existing, srv)
- } else {
+ } else if projectMCPTrusted() {
base.MCPServers[name] = srv
+ } else {
+ Logger().Printf("[config] ignoring project MCP server %q (untrusted project config)", name)
}
}🤖 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 `@internal/config/project.go` around lines 89 - 101, Update the MCP server
merge logic around mergeMCPServer so servers introduced by project configuration
are not added by default. Require an explicit project-trust signal, following
the existing JCODE_HOOKS_TRUST_PROJECT pattern, before inserting new names; when
trust is absent, log that the project server was skipped and preserve existing
user-configured servers and their merge behavior.
| if overlay.Browser != nil { | ||
| base.Browser = overlay.Browser | ||
| } | ||
| if overlay.Computer != nil { | ||
| base.Computer = overlay.Computer | ||
| } |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Browser/Computer blocks are capability toggles, not tuning — they belong on the denylist.
{"computer":{"enabled":true}} in a repo's .jcode/config.json switches on native desktop control (newComputerManager(cfg, ""), internal/command/web.go Line 374; internal/command/interactive.go Line 1244, whose own comment notes it "can reach anything on the machine"). These blocks also carry the preapproved site/app permission lists read by SetBrowserPermFunc/SetComputerPermFunc, so a project can auto-approve its own origins and bypass prompts — the same escalation class as AutoApprove/DefaultMode, which you correctly refuse.
Either denylist both, or merge only non-capability sub-fields (leave Enabled and the permission lists global-only).
🤖 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 `@internal/config/project.go` around lines 145 - 150, Update the overlay merge
logic around Browser and Computer so project configuration cannot override
capability toggles or permission lists. Denylist both blocks entirely, or merge
only non-capability tuning fields while preserving the global Enabled values and
browser/computer permission lists; keep the existing protected handling for
AutoApprove and DefaultMode.
|
Superseded by #173 which contains all changes from this PR plus additional work (walk-up config, standalone mcp.json, env var overrides, Settings UI scope badges, and security fixes from review comments). |
1 participant
动机
当前 jcode 的 MCP servers 和大部分配置只能放在全局
~/.jcode/config.json，无法按项目定制。参考 Codex（分层 TOML + denylist）和 Grok（user + project field-by-field merge）的实现，增加项目级配置叠加。设计
配置层级：
~/.jcode/config.json（现有，不变）<projectDir>/.jcode/config.json（新增）合并语义：
安全黑名单（项目配置被静默忽略）：
接入点： 三个 transport（interactive TUI、web、ACP）+ MCP 热重载路径。
对抗审核修复
Command/URL不可被项目配置覆盖（防止重定向到恶意二进制）DefaultMode加入黑名单（防止项目配置提权到 full_access）测试
go test ./...通过（32 packages）golangci-lint零新增 issueSummary by CodeRabbit
.jcode/config.json.