Bind declared Codex MCP servers after workspace trust

apps/daemon/internal/agent/codex/mcp_config.go

@@ -20,7 +20,6 @@ type mcpServerConfig struct {
 	Required          bool
 	BearerTokenEnvVar string
 	EnvHTTPHeaders    map[string]string
-	ApproveTools      bool
 }
 
 // writeCodexMCPConfig writes a `[mcp_servers.<name>]` TOML table per
@@ -48,9 +47,6 @@ func writeCodexMCPConfig(codexHome string, servers map[string]mcpServerConfig) e
 		b.WriteString("[mcp_servers.")
 		b.WriteString(tomlQuoteString(name))
 		b.WriteString("]\n")
-		if srv.ApproveTools {
-			b.WriteString("default_tools_approval_mode = \"approve\"\n")
-		}
 		if srv.Required {
 			b.WriteString("required = true\n")
 		}

apps/daemon/internal/agent/codex/mcp_environment_test.go

@@ -22,7 +22,7 @@ func TestEnvironmentMCPProjectsIsolatedStdioAndPrivateHTTPReferences(t *testing.
 		t.Fatal("environment declarations were not projected", err)
 	}
 	executable, _ := os.Executable()
-	if servers["local"].Command != executable || !servers["local"].ApproveTools ||
+	if servers["local"].Command != executable ||
 		!slices.Equal(servers["local"].Args, []string{"runtime-mcp-exec", "/private/runtime/capabilities", "plugins/0", "local"}) {
 		t.Fatal("native stdio bypasses the packaged launcher")
 	}
@@ -33,10 +33,10 @@ func TestEnvironmentMCPProjectsIsolatedStdioAndPrivateHTTPReferences(t *testing.
 		t.Fatal("private header values changed")
 	}
 	fixture := map[string]any{"config": map[string]any{
-		"mcp_oauth_credentials_store": "file", "features": map[string]bool{"plugins": false, "apps": false},
+		"mcp_oauth_credentials_store": "file",
 		"mcp_servers": map[string]any{
-			"local":  map[string]any{"command": servers["local"].Command, "args": servers["local"].Args, "environment_id": "local", "enabled": true, "tool_timeout_sec": nil, "default_tools_approval_mode": "approve"},
-			"remote": map[string]any{"url": remote.URL, "environment_id": "local", "enabled": true, "tool_timeout_sec": nil, "default_tools_approval_mode": "approve", "bearer_token_env_var": remote.BearerTokenEnvVar, "env_http_headers": remote.EnvHTTPHeaders},
+			"local":  map[string]any{"command": servers["local"].Command, "args": servers["local"].Args, "environment_id": "local", "enabled": true, "tool_timeout_sec": nil},
+			"remote": map[string]any{"url": remote.URL, "environment_id": "local", "enabled": true, "tool_timeout_sec": nil, "bearer_token_env_var": remote.BearerTokenEnvVar, "env_http_headers": remote.EnvHTTPHeaders},
 		},
 	}}
 	raw, _ := json.Marshal(fixture)

apps/daemon/internal/agent/codex/mcp_http.go

@@ -5,15 +5,13 @@ import (
 	"errors"
 	"os"
 	"path/filepath"
-	"slices"
 
 	"github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/agent"
 	"github.com/MiniMax-AI/OpenAgentCore/apps/daemon/internal/localworkspace"
 	"github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto"
 )
 
-// One projection consumes both public and installed Runtime bindings. A non-nil
-// empty public declaration still owns the complete native MCP configuration.
+// One projection consumes both public and installed Runtime bindings.
 func runtimeMCPServers(req proto.PromptRequestPayload) (map[string]mcpServerConfig, []string, error) {
 	bindings, err := agent.ResolveMCPBindings(req)
 	if err != nil {
@@ -28,7 +26,7 @@ func runtimeMCPServers(req proto.PromptRequestPayload) (map[string]mcpServerConf
 		if binding.ServerLabel == "codex_apps" {
 			return nil, nil, errors.New("codex: reserved MCP server label")
 		}
-		server := mcpServerConfig{Name: binding.ServerLabel, URL: binding.ServerURL, Required: binding.Required, EnabledTools: binding.AllowedTools, ApproveTools: binding.ConnectionOrigin == "environment"}
+		server := mcpServerConfig{Name: binding.ServerLabel, URL: binding.ServerURL, Required: binding.Required, EnabledTools: binding.AllowedTools}
 		if binding.Stdio != nil {
 			server.Command, server.Args = localworkspace.MCPStdioCommand(*binding.Stdio)
 		}
@@ -62,11 +60,8 @@ func configureMCP(plan *SessionPlan, servers map[string]mcpServerConfig) error {
 	if err := writeCodexMCPConfig(codexHome, servers); err != nil {
 		return errors.New("codex: cannot write public MCP configuration")
 	}
-	for _, feature := range []string{"plugins", "apps"} {
-		if !slices.Contains(plan.DisableFeatures, feature) {
-			plan.DisableFeatures = append(plan.DisableFeatures, feature)
-		}
-	}
+	// Native shell tools must not inherit the per-server secret variables.
+	excludeFromShellEnvironment(plan, "OAC_RUNTIME_MCP_*")
 	plan.ExtraConfig = append(plan.ExtraConfig, [2]string{"mcp_oauth_credentials_store", `"file"`})
 	plan.mcpServers = servers
 	return nil

apps/daemon/internal/agent/codex/mcp_http_bearer_test.go

@@ -74,7 +74,7 @@ func TestMCPHTTPBearerPreflightMatchesOnlyItsServerReference(t *testing.T) {
 		"second": {URL: "https://second.example/mcp", BearerTokenEnvVar: "OAC_RUNTIME_MCP_BEARER_SECOND"},
 		"public": {URL: "http://public.example/mcp"},
 	}
-	for _, mutation := range []string{"none", "missing", "ambient", "swapped", "extra", "header", "helper"} {
+	for _, mutation := range []string{"none", "missing", "ambient", "swapped", "extra", "header"} {
 		t.Run(mutation, func(t *testing.T) {
 			response := mcpHTTPConfigResponse(servers)
 			entries := response["config"].(map[string]any)["mcp_servers"].(map[string]any)
@@ -90,8 +90,6 @@ func TestMCPHTTPBearerPreflightMatchesOnlyItsServerReference(t *testing.T) {
 				entries["public"].(map[string]any)["bearer_token_env_var"] = servers["first"].BearerTokenEnvVar
 			case "header":
 				first["http_headers"] = map[string]string{"Authorization": "Bearer synthetic-private"}
-			case "helper":
-				first["http_headers_helper"] = "operator-helper"
 			}
 			raw, err := json.Marshal(response)
 			if err != nil || matchesMCPConfig(raw, servers) != (mutation == "none") {

apps/daemon/internal/agent/codex/mcp_http_preflight.go

@@ -8,9 +8,15 @@ import (
 	"time"
 )
 
-// config/read loads the same cwd and CLI layers without MCP discovery. Native
-// mcpServerStatus/list instead opens eager discovery connections; do not use it
-// to decide whether undeclared servers are safe to contact.
+// errMCPBindingChanged reports that the effective native configuration moved a
+// declared server's credential binding or the credential store.
+var errMCPBindingChanged = errors.New("codex: effective native MCP configuration differs from the public declaration")
+
+// verifyMCPConfig runs after the workspace trust entry is in the generated
+// configuration, so config/read sees the same project layer thread/start and
+// thread/resume load. It runs before any native connection that carries a Core
+// credential. config/read loads the layers without MCP discovery; native
+// mcpServerStatus/list instead opens eager discovery connections.
 func verifyMCPConfig(ctx context.Context, rpc *JSONRPCClient, plan SessionPlan) error {
 	check, cancel := context.WithTimeout(ctx, 5*time.Second)
 	defer cancel()
@@ -20,120 +26,96 @@ func verifyMCPConfig(ctx context.Context, rpc *JSONRPCClient, plan SessionPlan)
 		return errors.New("codex: cannot verify public MCP configuration")
 	}
 	if !matchesMCPConfig(raw, plan.mcpServers) {
-		return errors.New("codex: effective native MCP configuration differs from the public declaration")
+		return errMCPBindingChanged
 	}
 	return nil
 }
 
+// matchesMCPConfig checks only what binds a Core credential to a declared
+// server: where the server runs and connects, which variables carry its bearer
+// and headers, and the declared tool policy. Other servers and settings follow
+// native defaults.
 func matchesMCPConfig(raw json.RawMessage, declared map[string]mcpServerConfig) bool {
 	var response struct {
 		Config struct {
 			Servers         map[string]map[string]any `json:"mcp_servers"`
-			Features        map[string]any            `json:"features"`
 			CredentialStore string                    `json:"mcp_oauth_credentials_store"`
 		} `json:"config"`
 	}
 	if json.Unmarshal(raw, &response) != nil {
 		return false
 	}
 	config := response.Config
-	if config.CredentialStore != "file" || config.Features["plugins"] != false || config.Features["apps"] != false || config.Servers == nil || len(config.Servers) != len(declared) {
+	if config.CredentialStore != "file" || (len(declared) > 0 && config.Servers == nil) {
 		return false
 	}
 	for name, expected := range declared {
 		server, exists := config.Servers[name]
-		if !exists || server["environment_id"] != "local" || server["enabled"] != true {
+		if !exists || server["environment_id"] != "local" {
 			return false
 		}
 		if expected.URL != "" {
-			if server["url"] != expected.URL || !matchesMCPHeaderMap(server, "env_http_headers", expected.EnvHTTPHeaders) {
+			if server["url"] != expected.URL {
 				return false
 			}
-			delete(server, "url")
 		} else {
 			args, err := json.Marshal(expected.Args)
 			var want any
 			if err != nil || json.Unmarshal(args, &want) != nil || server["command"] != expected.Command || !reflect.DeepEqual(server["args"], want) {
 				return false
 			}
-			delete(server, "command")
-			delete(server, "args")
 		}
-		delete(server, "environment_id")
-		delete(server, "enabled")
-		if expected.ApproveTools {
-			if server["default_tools_approval_mode"] != "approve" {
-				return false
-			}
-			delete(server, "default_tools_approval_mode")
-		}
-		if expected.Required {
-			if server["required"] != true {
-				return false
-			}
-			delete(server, "required")
-		}
-		if expected.BearerTokenEnvVar != "" {
-			if server["bearer_token_env_var"] != expected.BearerTokenEnvVar {
-				return false
-			}
-			delete(server, "bearer_token_env_var")
+		if server["bearer_token_env_var"] != nilIfEmpty(expected.BearerTokenEnvVar) ||
+			!matchesMCPHeaderMap(server, "env_http_headers", expected.EnvHTTPHeaders) ||
+			!matchesMCPHeaderMap(server, "http_headers", nil) {
+			return false
 		}
-		if expected.EnabledTools != nil {
-			// Compare sets: native enabled_tools is an allowlist, not an ordered program.
-			actual, ok := server["enabled_tools"].([]any)
-			if !ok {
-				return false
-			}
-			want := make(map[string]bool, len(*expected.EnabledTools))
-			for _, tool := range *expected.EnabledTools {
-				want[tool] = true
-			}
-			got := make(map[string]bool, len(actual))
-			for _, tool := range actual {
-				name, ok := tool.(string)
-				if !ok {
-					return false
-				}
-				got[name] = true
-			}
-			if !reflect.DeepEqual(want, got) {
-				return false
-			}
-			delete(server, "enabled_tools")
+		if expected.Required && server["required"] != true {
+			return false
 		}
-		// These are the native serializer's inert defaults. Reject all additional
-		// settings, including headers, credential helpers, tool policies and filters.
-		for key, value := range server {
-			switch key {
-			case "tool_timeout_sec":
-				if value != nil {
-					return false
-				}
-			case "required", "supports_parallel_tool_calls":
-				if value != false {
-					return false
-				}
-			default:
-				return false
-			}
+		if expected.EnabledTools != nil && !matchesEnabledTools(server["enabled_tools"], *expected.EnabledTools) {
+			return false
 		}
 	}
 	return true
 }
 
+func nilIfEmpty(value string) any {
+	if value == "" {
+		return nil
+	}
+	return value
+}
+
+// matchesEnabledTools compares sets: native enabled_tools is an allowlist, not an ordered program.
+func matchesEnabledTools(actual any, expected []string) bool {
+	tools, ok := actual.([]any)
+	if !ok {
+		return false
+	}
+	want := make(map[string]bool, len(expected))
+	for _, tool := range expected {
+		want[tool] = true
+	}
+	got := make(map[string]bool, len(tools))
+	for _, tool := range tools {
+		name, ok := tool.(string)
+		if !ok {
+			return false
+		}
+		got[name] = true
+	}
+	return reflect.DeepEqual(want, got)
+}
+
 func matchesMCPHeaderMap(server map[string]any, field string, expected map[string]string) bool {
 	actual, present := server[field]
 	if len(expected) == 0 {
-		return !present
+		return !present || actual == nil
 	}
 	want := make(map[string]any, len(expected))
 	for key, value := range expected {
 		want[key] = value
 	}
-	if !reflect.DeepEqual(actual, want) {
-		return false
-	}
-	delete(server, field)
-	return true
+	return reflect.DeepEqual(actual, want)
 }

apps/daemon/internal/agent/codex/mcp_http_preflight_test.go

@@ -13,7 +13,9 @@ import (
 func TestPublicMCPHTTPEffectiveConfiguration(t *testing.T) {
 	for _, allowlist := range []*[]string{nil, new([]string), {"lookup", "query"}} {
 		servers := map[string]mcpServerConfig{"docs": {URL: "https://docs.example/mcp", EnabledTools: allowlist}}
-		for _, mutation := range []string{"none", "ambient", "url", "header", "header helper", "env header", "auth", "required", "tools", "disabled", "remote", "plugins", "apps", "keyring", "policy"} {
+		// Only what binds a credential to the declared server is checked; every
+		// other native setting and undeclared server keeps its native value.
+		for _, mutation := range []string{"none", "ambient", "url", "header", "header helper", "env header", "auth", "required", "tools", "disabled", "remote", "plugins", "apps", "keyring", "policy", "timeout"} {
 			t.Run(mutation+"/"+allowlistName(allowlist), func(t *testing.T) {
 				response := mcpHTTPConfigResponse(servers)
 				config := response["config"].(map[string]any)
@@ -41,17 +43,20 @@ func TestPublicMCPHTTPEffectiveConfiguration(t *testing.T) {
 				case "remote":
 					server["environment_id"] = "remote"
 				case "plugins", "apps":
-					config["features"].(map[string]any)[mutation] = true
+					config["features"] = map[string]any{mutation: true}
 				case "keyring":
 					config["mcp_oauth_credentials_store"] = "auto"
 				case "policy":
 					server["tools"] = map[string]any{"lookup": map[string]any{"enabled": false}}
+				case "timeout":
+					server["tool_timeout_sec"] = 5
 				}
 				data, err := json.Marshal(response)
 				if err != nil {
 					t.Fatal(err)
 				}
-				if matchesMCPConfig(data, servers) != (mutation == "none") {
+				rejected := map[string]bool{"url": true, "header": true, "env header": true, "remote": true, "keyring": true, "tools": allowlist != nil}
+				if matchesMCPConfig(data, servers) == rejected[mutation] {
 					t.Fatal("effective configuration decision differed", mutation)
 				}
 			})
@@ -122,7 +127,7 @@ func TestPublicMCPHTTPPreflightRedactsNativeErrors(t *testing.T) {
 }
 
 func TestPublicMCPHTTPPreparationChecksBeforeNewAndResumedThread(t *testing.T) {
-	for _, mode := range []string{"new", "resume", "reject", "reject bearer reference"} {
+	for _, mode := range []string{"new", "resume", "extra server", "reject url", "reject bearer reference"} {
 		t.Run(mode, func(t *testing.T) {
 			req, cfg, root := preparationFixture(t)
 			req.AgentOptions = map[string]any{"model": "fixture-model"}
@@ -144,9 +149,12 @@ func TestPublicMCPHTTPPreparationChecksBeforeNewAndResumedThread(t *testing.T) {
 				t.Fatal(err)
 			}
 			response := mcpHTTPConfigResponse(declarations)
-			if mode == "reject" {
+			if mode == "extra server" {
 				response["config"].(map[string]any)["mcp_servers"].(map[string]any)["operator"] = map[string]any{"url": "https://operator.example/private"}
 			}
+			if mode == "reject url" {
+				response["config"].(map[string]any)["mcp_servers"].(map[string]any)["docs"].(map[string]any)["url"] = "https://workspace.example/redirect"
+			}
 			if mode == "reject bearer reference" {
 				response["config"].(map[string]any)["mcp_servers"].(map[string]any)["docs"].(map[string]any)["bearer_token_env_var"] = "OPERATOR_SECRET"
 			}
@@ -156,7 +164,7 @@ func TestPublicMCPHTTPPreparationChecksBeforeNewAndResumedThread(t *testing.T) {
 			e, err := testExecutor(t, "complete", req, cfg)
 			if strings.HasPrefix(mode, "reject") {
 				if err == nil || e != nil {
-					t.Fatal("ambient MCP configuration admitted")
+					t.Fatal("redirected credential binding admitted")
 				}
 				assertPreparationOnly(t, root)
 				waitPreparationMethod(t, root, "config/read")

apps/daemon/internal/agent/codex/mcp_http_test.go

@@ -26,8 +26,10 @@ func TestPublicMCPHTTPPlanOwnsConfigurationAndPreservesHistory(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer plan.Cleanup()
-	if !slices.Contains(plan.DisableFeatures, "apps") || !slices.Contains(plan.DisableFeatures, "plugins") || !slices.Contains(plan.ExtraConfig, [2]string{"mcp_oauth_credentials_store", `"file"`}) {
-		t.Fatal("native profile was not pinned")
+	if slices.Contains(plan.DisableFeatures, "apps") || slices.Contains(plan.DisableFeatures, "plugins") ||
+		!slices.Contains(plan.ExtraConfig, [2]string{"mcp_oauth_credentials_store", `"file"`}) ||
+		!slices.Contains(plan.ExtraConfig, [2]string{"shell_environment_policy.exclude", `["OAC_RUNTIME_MCP_*"]`}) {
+		t.Fatal("native profile kept only credential isolation")
 	}
 	home, err := allocCodexHome(testStateRoot(t), req.AgentStateKey)
 	if err != nil {
@@ -120,7 +122,7 @@ func mcpHTTPConfigResponse(servers map[string]mcpServerConfig) map[string]any {
 		}
 		entries[name] = entry
 	}
-	return map[string]any{"config": map[string]any{"mcp_servers": entries, "features": map[string]any{"plugins": false, "apps": false}, "mcp_oauth_credentials_store": "file"}}
+	return map[string]any{"config": map[string]any{"mcp_servers": entries, "mcp_oauth_credentials_store": "file"}}
 }
 
 func writeMCPHTTPConfigResponse(t *testing.T, path string, response any) {