From 1007208db02ab9e8686485c18635450b3202c150 Mon Sep 17 00:00:00 2001 From: SaladDay <1203511142@qq.com> Date: Mon, 19 Oct 2026 08:45:26 +0000 Subject: [PATCH] Bind declared Codex MCP servers after workspace trust --- .../daemon/internal/agent/codex/mcp_config.go | 4 - .../agent/codex/mcp_environment_test.go | 8 +- apps/daemon/internal/agent/codex/mcp_http.go | 13 +- .../agent/codex/mcp_http_bearer_test.go | 4 +- .../agent/codex/mcp_http_preflight.go | 128 +++++++---------- .../agent/codex/mcp_http_preflight_test.go | 20 ++- .../internal/agent/codex/mcp_http_test.go | 8 +- apps/daemon/internal/agent/codex/options.go | 19 ++- .../internal/agent/codex/session_plan.go | 4 + .../internal/agent/codex/session_thread.go | 6 +- apps/daemon/internal/agent/codex/trust.go | 28 ++++ .../codex/workspace_trust_native_test.go | 135 ++++++++++++++++++ contracts/agents-api/execution-tools.md | 2 +- contracts/agents-api/zh/execution-tools.md | 4 +- services/core/deploy/codex/README.md | 4 +- 15 files changed, 274 insertions(+), 113 deletions(-) create mode 100644 apps/daemon/internal/agent/codex/trust.go create mode 100644 apps/daemon/internal/agent/codex/workspace_trust_native_test.go diff --git a/apps/daemon/internal/agent/codex/mcp_config.go b/apps/daemon/internal/agent/codex/mcp_config.go index 653726a43..d0de7d3be 100644 --- a/apps/daemon/internal/agent/codex/mcp_config.go +++ b/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.]` 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") } diff --git a/apps/daemon/internal/agent/codex/mcp_environment_test.go b/apps/daemon/internal/agent/codex/mcp_environment_test.go index 0013b3a76..397ab6098 100644 --- a/apps/daemon/internal/agent/codex/mcp_environment_test.go +++ b/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) diff --git a/apps/daemon/internal/agent/codex/mcp_http.go b/apps/daemon/internal/agent/codex/mcp_http.go index d5a935f7d..397d103e3 100644 --- a/apps/daemon/internal/agent/codex/mcp_http.go +++ b/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 diff --git a/apps/daemon/internal/agent/codex/mcp_http_bearer_test.go b/apps/daemon/internal/agent/codex/mcp_http_bearer_test.go index b26f3df10..c7dcaf0b5 100644 --- a/apps/daemon/internal/agent/codex/mcp_http_bearer_test.go +++ b/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") { diff --git a/apps/daemon/internal/agent/codex/mcp_http_preflight.go b/apps/daemon/internal/agent/codex/mcp_http_preflight.go index b499038fb..549b8a2c5 100644 --- a/apps/daemon/internal/agent/codex/mcp_http_preflight.go +++ b/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,16 +26,19 @@ 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"` } @@ -37,103 +46,76 @@ func matchesMCPConfig(raw json.RawMessage, declared map[string]mcpServerConfig) 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) } diff --git a/apps/daemon/internal/agent/codex/mcp_http_preflight_test.go b/apps/daemon/internal/agent/codex/mcp_http_preflight_test.go index 138a97182..ea16a6a5a 100644 --- a/apps/daemon/internal/agent/codex/mcp_http_preflight_test.go +++ b/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") diff --git a/apps/daemon/internal/agent/codex/mcp_http_test.go b/apps/daemon/internal/agent/codex/mcp_http_test.go index 2af2ce76b..75610d9fa 100644 --- a/apps/daemon/internal/agent/codex/mcp_http_test.go +++ b/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) { diff --git a/apps/daemon/internal/agent/codex/options.go b/apps/daemon/internal/agent/codex/options.go index 0b8900da8..154cd671c 100644 --- a/apps/daemon/internal/agent/codex/options.go +++ b/apps/daemon/internal/agent/codex/options.go @@ -59,6 +59,8 @@ type SessionPlan struct { // native root Turn. OutputSchema json.RawMessage + shellEnvExclude []string + // Cleanup is the deferred housekeeping the session must run after the child exits. Cleanup func() } @@ -120,7 +122,7 @@ func BuildSessionPlan(stateRoot, agentStateKey string, opts map[string]any, cont // shell_environment_policy is applied. Do not persist provider credentials. plan.DisableFeatures = append(plan.DisableFeatures, "shell_snapshot") env = append(env, providerAPIKeyEnv+"="+provider.BearerToken) - plan.ExtraConfig = append(plan.ExtraConfig, [2]string{"shell_environment_policy.exclude", "[" + strconv(providerAPIKeyEnv) + "]"}) + excludeFromShellEnvironment(&plan, providerAPIKeyEnv) } plan.Env = env @@ -136,6 +138,21 @@ func BuildSessionPlan(stateRoot, agentStateKey string, opts map[string]any, cont return plan, nil } +// excludeFromShellEnvironment keeps a Runtime secret out of native shell tools. +// The pattern is a native case-insensitive wildcard; one setting holds them all. +func excludeFromShellEnvironment(plan *SessionPlan, pattern string) { + const key = "shell_environment_policy.exclude" + plan.shellEnvExclude = append(plan.shellEnvExclude, strconv(pattern)) + value := "[" + strings.Join(plan.shellEnvExclude, ",") + "]" + for i := range plan.ExtraConfig { + if plan.ExtraConfig[i][0] == key { + plan.ExtraConfig[i][1] = value + return + } + } + plan.ExtraConfig = append(plan.ExtraConfig, [2]string{key, value}) +} + // applyExecutionControls maps each official value to its native setting. An // absent control sends nothing, so Codex keeps its native default; Codex itself // ignores a value the selected model does not support. diff --git a/apps/daemon/internal/agent/codex/session_plan.go b/apps/daemon/internal/agent/codex/session_plan.go index b38c5e461..fe240ec20 100644 --- a/apps/daemon/internal/agent/codex/session_plan.go +++ b/apps/daemon/internal/agent/codex/session_plan.go @@ -31,6 +31,10 @@ func prepareSessionPlan(ctx context.Context, req proto.PromptRequestPayload, cfg applyProgrammaticTools(&plan, req.ExecutionControls) if req.LocalEnvironment != nil { plan.Cwd = req.LocalEnvironment.WorkspaceRoot + if err := writeWorkspaceTrust(nativeHomeFromPlan(plan), plan.Cwd); err != nil { + plan.Cleanup() + return SessionPlan{}, nil, err + } } else { // environment:none has no workspace; the Session's private home is its cwd. plan.Cwd = nativeHomeFromPlan(plan) diff --git a/apps/daemon/internal/agent/codex/session_thread.go b/apps/daemon/internal/agent/codex/session_thread.go index 35699b8a8..65b59fce9 100644 --- a/apps/daemon/internal/agent/codex/session_thread.go +++ b/apps/daemon/internal/agent/codex/session_thread.go @@ -42,13 +42,9 @@ func (s *Session) startThread(plan SessionPlan) error { func (s *Session) resumeThread(threadID string, plan SessionPlan) error { params := ThreadResumeParams{ - ThreadID: threadID, ApprovalPolicy: approvalPolicyNever, Sandbox: sandboxDangerFullAccess, + ThreadID: threadID, Cwd: plan.Cwd, ApprovalPolicy: approvalPolicyNever, Sandbox: sandboxDangerFullAccess, DeveloperInstructions: plan.SystemPrompt, } - if plan.mcpServers != nil { - // Resolve the same project configuration checked before native startup. - params.Cwd = plan.Cwd - } s.usageMu.Lock() s.resumeUsageThreadID = threadID s.usageMu.Unlock() diff --git a/apps/daemon/internal/agent/codex/trust.go b/apps/daemon/internal/agent/codex/trust.go new file mode 100644 index 000000000..83fa1d697 --- /dev/null +++ b/apps/daemon/internal/agent/codex/trust.go @@ -0,0 +1,28 @@ +package codex + +import ( + "fmt" + "os" + "path/filepath" +) + +// writeWorkspaceTrust marks the workspace as a trusted native project, the entry +// thread/start would persist, so start and cold resume load the same workspace +// configuration. The key is the nearest checkout root, else the workspace. +func writeWorkspaceTrust(codexHome, workspace string) error { + root := workspace + for dir := workspace; ; dir = filepath.Dir(dir) { + if _, err := os.Lstat(filepath.Join(dir, ".git")); err == nil { + root = dir + break + } + if filepath.Dir(dir) == dir { + break + } + } + if resolved, err := filepath.EvalSymlinks(root); err == nil { + root = resolved + } + return appendConfigTOML(filepath.Join(codexHome, "config.toml"), + fmt.Sprintf("[projects.%s]\ntrust_level = \"trusted\"\n\n", tomlQuoteString(root))) +} diff --git a/apps/daemon/internal/agent/codex/workspace_trust_native_test.go b/apps/daemon/internal/agent/codex/workspace_trust_native_test.go new file mode 100644 index 000000000..b77aaa68e --- /dev/null +++ b/apps/daemon/internal/agent/codex/workspace_trust_native_test.go @@ -0,0 +1,135 @@ +package codex + +import ( + "context" + "encoding/json" + "errors" + "log/slog" + "os" + "os/exec" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/MiniMax-AI/OpenAgentCore/internal/agentdaemon/proto" +) + +// A trusted workspace loads its own native configuration on start and on cold +// resume, but may not move a declared MCP server's credential binding. Opt-in: +// it needs the pinned native binary and no model call. +func TestNativeWorkspaceConfigurationAndDeclaredMCPBinding(t *testing.T) { + binary := os.Getenv("OAC_TEST_CODEX_BINARY") + if binary == "" { + t.Skip("requires the pinned native Codex binary") + } + if version, err := exec.Command(binary, "--version").Output(); err != nil || strings.TrimSpace(string(version)) != "codex-cli 0.153.4" { + t.Fatal("native artifact is not Codex 0.153.4") + } + t.Setenv("OAC_RUNTIME_HOME", t.TempDir()) + cfg := defaultSessionConfig(testStateRoot(t)) + cfg.codexBinary = binary + cfg.logger = slog.New(slog.DiscardHandler) + workspace := t.TempDir() + if err := exec.Command("git", "init", "-q", workspace).Run(); err != nil { + t.Fatal(err) + } + writeWorkspace := func(body string) { + t.Helper() + if err := os.MkdirAll(filepath.Join(workspace, ".codex"), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(workspace, ".codex", "config.toml"), []byte(body), 0o644); err != nil { + t.Fatal(err) + } + } + token := "synthetic-private-token" + prepare := func(ctx context.Context, thread string) (*Executor, error) { + servers := []proto.MCPHTTPServer{{ConnectionOrigin: "environment", ServerLabel: "docs", ServerURL: "https://docs.invalid/mcp", BearerToken: &token}} + return newExecutor(ctx, proto.PromptRequestPayload{ + AgentKind: "codex", AgentStateKey: "workspace-trust", DisableSubagents: true, + AgentSessionID: thread, RequireExistingNativeSession: thread != "", MCPHTTPServers: &servers, + LocalEnvironment: &proto.LocalEnvironment{NetworkAccess: "enabled", WorkspaceRoot: workspace}, + AgentOptions: map[string]any{"model": "gpt-5.5", "model_provider": map[string]any{"base_url": "http://127.0.0.1:9/v1", "protocol": "responses", "api_key": "unused"}}, + }, cfg) + } + closeExecutor := func(e *Executor) { + c, stop := context.WithTimeout(context.Background(), 20*time.Second) + defer stop() + _ = e.Close(c) + } + ctx, cancel := context.WithTimeout(t.Context(), 3*time.Minute) + defer cancel() + readConfig := func(e *Executor) (map[string]any, map[string]any) { + t.Helper() + raw, err := e.prepared.session.rpc.Request(ctx, "config/read", map[string]any{"cwd": workspace, "includeLayers": false}) + if err != nil { + t.Fatal(err) + } + var response struct { + Config struct { + Servers map[string]any `json:"mcp_servers"` + Features map[string]any `json:"features"` + } `json:"config"` + } + if err := json.Unmarshal(raw, &response); err != nil { + t.Fatal(err) + } + return response.Config.Servers, response.Config.Features + } + + // Additional workspace servers are admitted and plugins and apps keep their native values. + writeWorkspace("[mcp_servers.extra]\ncommand = \"true\"\n\n[mcp_servers.docs]\ntool_timeout_sec = 7\n") + e, err := prepare(ctx, "") + if err != nil { + t.Fatalf("declaration with extra workspace server rejected: %v", err) + } + servers, features := readConfig(e) + if servers["extra"] == nil || servers["docs"] == nil { + t.Fatal("workspace configuration did not load on start") + } + if features["plugins"] == false || features["apps"] == false { + t.Fatal("plugins or apps were forced off") + } + output := make(chan proto.Envelope, 64) + turn, err := e.StartTurn(ctx, "first", proto.TextInput("Reply OK."), output) + if err != nil { + t.Fatal(err) + } + for done := false; !done; { + select { + case event := <-output: + done = event.Type == proto.TypeDone + case <-ctx.Done(): + t.Fatal("Turn did not settle") + } + } + _, _ = turn.AwaitSettlement(ctx) + thread := turn.(*Session).currentThreadID() + closeExecutor(e) + if thread == "" { + t.Fatal("no native thread to resume") + } + + // A cold resume loads the same workspace configuration. + e, err = prepare(ctx, thread) + if err != nil { + t.Fatalf("cold resume rejected: %v", err) + } + if servers, _ := readConfig(e); servers["extra"] == nil { + t.Fatal("workspace configuration did not load on cold resume") + } + closeExecutor(e) + + // Redirecting the declared server fails start and cold resume before any native Turn. + writeWorkspace("[mcp_servers.docs]\nurl = \"https://workspace.invalid/mcp\"\n") + for _, resume := range []string{"", thread} { + e, err := prepare(ctx, resume) + if !errors.Is(err, errMCPBindingChanged) { + t.Fatalf("redirected declared server admitted (resume=%q): %v", resume, err) + } + if e != nil { + closeExecutor(e) + } + } +} diff --git a/contracts/agents-api/execution-tools.md b/contracts/agents-api/execution-tools.md index d3d44fa7c..e65ee0ecb 100644 --- a/contracts/agents-api/execution-tools.md +++ b/contracts/agents-api/execution-tools.md @@ -92,4 +92,4 @@ Execution carries the resolved values to the Harness and validates them against - Bearer authentication uses an attached static or OAuth Vault credential. [Vault credentials](./vaults.md) owns selection, and [MCP credential authority](./environments.md#public-mcp-connection-origin) owns the frozen Runtime binding. Authenticated execution requires `mcp_http_bearer_auth`. - The Runtime must advertise `mcp_http_tools`. The native Harness owns discovery, calls and results; public `mcp_call` Items use the original server and tool names and keep the observed native result. -Codex verifies the exact effective MCP configuration before starting or resuming a thread, excludes undeclared servers, disables native apps and plugins, and rejects reserved native labels and stored native MCP credentials. Claude accepts labels of ASCII letters, digits, underscore and hyphen except `functions`, tool names that may also contain dots, and requires connected servers with static inventories. Anonymous Claude requests send a blank Authorization header to suppress native OAuth injection. Native OAuth login is not supported. +Codex trusts the workspace natively, so its project configuration loads on start and cold resume, and undeclared servers, apps and plugins keep their native behavior. Before any native connection that carries a Core credential, it verifies that each declared server keeps its URL or command, bearer reference and headers, and fails the start or resume with an error otherwise. It rejects reserved native labels and stored native MCP credentials. Claude accepts labels of ASCII letters, digits, underscore and hyphen except `functions`, tool names that may also contain dots, and requires connected servers with static inventories. Anonymous Claude requests send a blank Authorization header to suppress native OAuth injection. Native OAuth login is not supported. diff --git a/contracts/agents-api/zh/execution-tools.md b/contracts/agents-api/zh/execution-tools.md index 123e8c6c4..20f92190e 100644 --- a/contracts/agents-api/zh/execution-tools.md +++ b/contracts/agents-api/zh/execution-tools.md @@ -1,7 +1,7 @@ --- title: "执行工具" source: contracts/agents-api/execution-tools.md -source_hash: db52d727ddf370a8047089457227584e89faf2132546a164b170e6b475b28abb +source_hash: b7a4e9408422a272c0c8695f264d1ee9be9b85695b4622a9751c7ead4d3c218c --- Agent 在 `tools` 中声明应用函数、控制项和 MCP 服务器，并可在 `text.format` 中声明输出 schema。本契约说明 Core 如何验证声明、哪些内容跨越 Runtime 边界，以及调用方如何恢复待执行操作。[Harness 能力](harness-capabilities.md)列出各 Harness 在不同部署位置支持的操作。原生工作区工具和 Environment Plugin MCP 属于 [Environment](environments.md#skills-plugins-and-environment-mcp)。 @@ -94,4 +94,4 @@ Core 发送 `PromptRequestPayload.ToolSearch` 和每个 `FunctionTool.DeferLoadi - Bearer 认证使用附加的静态或 OAuth Vault 凭据。[Vault 凭据](vaults.md)定义选择规则，[MCP 凭据权限](environments.md#public-mcp-connection-origin)定义冻结的 Runtime 绑定。认证执行要求 `mcp_http_bearer_auth`。 - Runtime 必须声明 `mcp_http_tools`。原生 Harness 管理发现、调用和结果；公开 `mcp_call` Item 使用原始服务器与工具名称，保留观察到的原生结果。 -Codex 在启动或恢复线程之前验证准确的有效 MCP 配置，排除未声明服务器，禁用原生 apps 和 plugins，拒绝原生保留标签和已存储原生 MCP 凭据。Claude 接受 ASCII 字母、数字、下划线和连字符组成的标签，但不允许 `functions`；工具名称还可包含点，并要求已连接服务器具有静态工具清单。匿名 Claude 请求发送空 Authorization 头以阻止原生 OAuth 注入。不支持原生 OAuth 登录。 +Codex 以原生方式信任工作区，因此工作区的项目配置在启动和冷恢复时加载，未声明的服务器、apps 和 plugins 保持原生行为。在任何携带 Core 凭据的原生连接之前，它验证每个已声明服务器保持其 URL 或命令、bearer 引用和请求头，否则启动或恢复以错误失败。它拒绝原生保留标签和已存储原生 MCP 凭据。Claude 接受 ASCII 字母、数字、下划线和连字符组成的标签，但不允许 `functions`；工具名称还可包含点，并要求已连接服务器具有静态工具清单。匿名 Claude 请求发送空 Authorization 头以阻止原生 OAuth 注入。不支持原生 OAuth 登录。 diff --git a/services/core/deploy/codex/README.md b/services/core/deploy/codex/README.md index 61edf7e7e..a1056a111 100644 --- a/services/core/deploy/codex/README.md +++ b/services/core/deploy/codex/README.md @@ -28,11 +28,11 @@ The adapter applies Core's typed controls and opt-ins, described in the [Runtime ## MCP servers -The adapter renders a typed MCP declaration with the native renderer and the original tool names for `enabled_tools`, including `[]`. Before creating or resuming a thread it reads native `config/read` with the exact cwd and rejects additional servers or any difference in the effective configuration. It disables native plugins and apps, selects file-only MCP credentials, and rejects existing credentials in the private native home without deleting them or native history. Reserved native labels are an adapter restriction, not a rule of the saved resource. The check is a snapshot, not a barrier against concurrent operator configuration changes, and discovery of a deny-all server can still contact it. +The adapter renders a typed MCP declaration with the native renderer and the original tool names for `enabled_tools`, including `[]`. It writes the native trust entry for the workspace into the generated configuration, so the workspace `.codex/config.toml` loads on thread start and cold resume as it does locally. Additional servers, plugins, apps and approval settings keep their native values. Before creating or resuming a thread, and so before any native connection that carries a Core credential, it reads native `config/read` with the exact cwd and requires every declared server to keep its `url` or command, `bearer_token_env_var`, headers, `required` and `enabled_tools`. Any difference fails the start or resume with a generic error and sends no native Turn; other fields of a declared server, such as `tool_timeout_sec`, may differ. It selects file-only MCP credentials, and rejects existing credentials in the private native home without deleting them or native history. Reserved native labels are an adapter restriction, not a rule of the saved resource. The check is a snapshot, not a barrier against concurrent operator configuration changes. With `required: true`, which needs `mcp_http_required`, root thread creation and cold resume wait for required servers to initialize and send no native Turn until they do; a failed strict resume is never replaced with a new thread. Public work may already be accepted while initialization waits. -A selected bearer credential (`mcp_http_bearer_auth`) becomes a fresh daemon-owned `bearer_token_env_var` reference for each server and native process. The secret enters only that app-server child's environment, after auxiliary launch probes, and never global environment, arguments, configuration, history, snapshots or logs. Preflight accepts only the expected server and reference pairing and keeps rejecting ambient credential sources. Empty values and bytes outside RFC 6750 `b64token` syntax are rejected with generic errors, and tokens are never trimmed. Core-managed OAuth delivers access tokens through the same path. +A selected bearer credential (`mcp_http_bearer_auth`) becomes a fresh daemon-owned `bearer_token_env_var` reference for each server and native process. The secret enters only that app-server child's environment, after auxiliary launch probes, and never global environment, arguments, configuration, history, snapshots or logs. The `OAC_RUNTIME_MCP_*` variables are excluded from native shell tools through `shell_environment_policy`, like the provider key. Empty values and bytes outside RFC 6750 `b64token` syntax are rejected with generic errors, and tokens are never trimmed. Core-managed OAuth delivers access tokens through the same path. ## Function results and command output