Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions internal/command/acp.go
Original file line number Diff line number Diff line change
Expand Up @@ -367,6 +367,15 @@ func (a *acpAgent) NewSession(ctx context.Context, params acp.NewSessionRequest)
pwd = util.GetWorkDir()
}

// 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)
}

providerName, modelName := cfg.GetProviderModel()
rec, _ := session.NewRecorder(pwd, providerName, modelName)
// LLM session titles ride the small model (checked at fire time).
Expand Down Expand Up @@ -1176,6 +1185,11 @@ func (a *acpAgent) ResumeSession(ctx context.Context, params acp.ResumeSessionRe
pwd = util.GetWorkDir()
}

// Merge project-level config over global config.
if projCfg, projErr := config.LoadProjectConfig(pwd); projErr == nil && projCfg != nil {
config.MergeProjectConfig(cfg, projCfg)
}

providerName, modelName := cfg.GetProviderModel()
rec, _ := session.NewRecorder(pwd, providerName, modelName)
// Reuse the original session UUID so transcript entries are written to
Expand Down
14 changes: 14 additions & 0 deletions internal/command/interactive.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,6 +464,10 @@ func (s *interactiveState) reloadMCP() {
config.Logger().Printf("[mcp] reload: config load failed: %v", err)
return
}
// Re-merge project config so hot-reload sees the same effective config.
if projCfg, projErr := config.LoadProjectConfig(s.pwd); projErr == nil && projCfg != nil {
config.MergeProjectConfig(latest, projCfg)
}
s.cfg = latest
mcpTools, statuses := tools.LoadMCPTools(s.ctx, latest.MCPServers)
s.mcpTools = mcpTools
Expand Down Expand Up @@ -1142,6 +1146,16 @@ func RunInteractive(prompt, resumeUUID, agentName string, unsafe bool) error {
platform := util.GetSystemInfo()
envInfo := util.CollectEnvInfo(pwd)

// Merge project-level config (<pwd>/.jcode/config.json) over the global
// config. Security-sensitive fields (providers, telemetry, cloud) are
// never taken from project config — see config.MergeProjectConfig.
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)
}

var resumeEntries []session.Entry
var resumeState *session.SessionState
if resumeUUID != "" {
Expand Down
9 changes: 9 additions & 0 deletions internal/command/web.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,15 @@ func runWebServer(parent context.Context, port int, host string, openBrowser boo
pwd := util.GetWorkDir()
platform := util.GetSystemInfo()

// 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)
}
Comment on lines +216 to +223

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🗄️ 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.


skillLoader := skills.NewLoaderWithDisabled(cfg.DisabledSkills)
skillLoader.ScanProjectSkills(pwd)

Expand Down
206 changes: 206 additions & 0 deletions internal/config/project.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,206 @@
package config

import (
"encoding/json"
"fmt"
"os"
"path/filepath"
)

// projectConfigFile is the project-level config filename inside <project>/.jcode/.
const projectConfigFile = "config.json"

// LoadProjectConfig reads <projectDir>/.jcode/config.json. It returns nil
// (without error) when the file does not exist — a missing project config is
// the common case and not an error condition. Parse errors are reported.
func LoadProjectConfig(projectDir string) (*Config, error) {
if projectDir == "" {
return nil, nil
}
path := filepath.Join(projectDir, configDir, projectConfigFile)
data, err := os.ReadFile(path)
if err != nil {
if os.IsNotExist(err) {
return nil, nil
}
return nil, fmt.Errorf("project config read %s: %w", path, err)
}
var pc Config
if err := json.Unmarshal(data, &pc); err != nil {
return nil, fmt.Errorf("project config parse %s: %w", path, err)
}
return &pc, nil
}

// MergeProjectConfig merges a project-level config overlay into the base
// (global) config. The merge is field-by-field: project values override global
// values when set. Security-sensitive fields (providers, telemetry, cloud,
// SSH/Docker aliases) are NEVER taken from project config — a malicious repo
// must not be able to redirect API keys or exfiltrate credentials.
//
// MCP servers are merged by name: the project can add new servers or override
// individual fields of an existing server (e.g. change args), but cannot
// delete a globally-configured server.
//
// The base config is mutated in place and returned for convenience.
func MergeProjectConfig(base, overlay *Config) *Config {
if base == nil || overlay == nil {
return base
}

// --- Model selection ---
if overlay.Model != "" {
base.Model = overlay.Model
}
if overlay.SmallModel != "" {
base.SmallModel = overlay.SmallModel
}

// --- Iteration cap ---
if overlay.MaxIterations > 0 {
base.MaxIterations = overlay.MaxIterations
}

// --- Session mode ---
// DefaultMode is intentionally NOT merged: a project config must not
// escalate to "full_access" and bypass the user's approval policy.

// --- Theme / Language ---
if overlay.Theme != "" {
base.Theme = overlay.Theme
}
if overlay.Language != "" {
base.Language = overlay.Language
}

// --- Context limits ---
if overlay.DefaultContextLimit > 0 {
base.DefaultContextLimit = overlay.DefaultContextLimit
}
if len(overlay.ContextLimits) > 0 {
if base.ContextLimits == nil {
base.ContextLimits = make(map[string]int, len(overlay.ContextLimits))
}
for k, v := range overlay.ContextLimits {
base.ContextLimits[k] = v
}
}

// --- 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
}
}
}
Comment on lines +89 to +101

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 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.


// --- Disabled skills: union ---
if len(overlay.DisabledSkills) > 0 {
seen := make(map[string]bool, len(base.DisabledSkills))
for _, s := range base.DisabledSkills {
seen[s] = true
}
for _, s := range overlay.DisabledSkills {
if !seen[s] {
base.DisabledSkills = append(base.DisabledSkills, s)
}
}
}

// --- Disabled providers: union ---
if len(overlay.DisabledProviders) > 0 {
seen := make(map[string]bool, len(base.DisabledProviders))
for _, p := range base.DisabledProviders {
seen[p] = true
}
for _, p := range overlay.DisabledProviders {
if !seen[p] {
base.DisabledProviders = append(base.DisabledProviders, p)
}
}
}

// --- Pointer-block overrides (project replaces the whole block if set) ---
if overlay.Budget != nil {
base.Budget = overlay.Budget
}
if overlay.Compaction != nil {
base.Compaction = overlay.Compaction
}
if overlay.Prompt != nil {
base.Prompt = overlay.Prompt
}
if overlay.Subagent != nil {
base.Subagent = overlay.Subagent
}
if overlay.Team != nil {
base.Team = overlay.Team
}
if overlay.Browser != nil {
base.Browser = overlay.Browser
}
if overlay.Computer != nil {
base.Computer = overlay.Computer
}
Comment on lines +145 to +150

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🔒 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.

if overlay.ToolSearch != nil {
base.ToolSearch = overlay.ToolSearch
}
if overlay.Channel != nil {
base.Channel = overlay.Channel
}
if overlay.ApprovalReview != nil {
base.ApprovalReview = overlay.ApprovalReview
}

// --- SECURITY DENYLIST ---
// The following fields are intentionally NOT merged from project config:
// - Providers / Models (API keys, base URLs, headers)
// - Telemetry (Langfuse secrets)
// - Cloud (relay credentials, E2EE keys)
// - SSHAliases / DockerAliases (remote access credentials)
// - Memory (pipeline model/budget — could redirect to attacker endpoint)
// - Developer (debug/tracing toggles)
// - AutoApprove (privilege escalation)
// A project config that sets these fields has them silently ignored.

return base
}

// mergeMCPServer merges overlay fields into an existing MCP server config.
// Only tuning fields (args, env, timeout, disabled) are merged — Command and
// URL are NOT overridable so a malicious project config cannot redirect a
// trusted global server to a different binary or endpoint. New servers (added
// via the map-merge in MergeProjectConfig) get their full definition from the
// project config, which is the user's explicit choice.
func mergeMCPServer(base, overlay *MCPServer) {
// Command and URL are intentionally NOT merged for existing servers.
if len(overlay.Args) > 0 {
base.Args = overlay.Args
}
if len(overlay.Env) > 0 {
base.Env = overlay.Env
}
if len(overlay.Headers) > 0 {
if base.Headers == nil {
base.Headers = make(map[string]string, len(overlay.Headers))
}
for k, v := range overlay.Headers {
base.Headers[k] = v
}
}
if overlay.TimeoutSeconds > 0 {
base.TimeoutSeconds = overlay.TimeoutSeconds
}
// Disabled can be set to true by project config (to suppress a global
// server in this project) but not back to false (project cannot
// re-enable a server the user globally disabled).
if overlay.Disabled {
base.Disabled = true
}
}
Loading
Loading