Skip to content
Merged
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
15 changes: 15 additions & 0 deletions docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -457,10 +457,25 @@ the error names the field.
| `reconnect_on_use` | boolean | No | When `true`, tool calls to a disconnected server trigger an immediate reconnect attempt (15s timeout) before failing (default: `false`) |
| `expose_prompts` | boolean | No | Per-server override for whether this server's MCP prompts are aggregated into mcpproxy's `prompts/list`. Only takes effect when the global `aggregate_upstream_prompts` master switch is on. Omit to expose prompts whenever the server advertises `Capabilities.Prompts`; `false` opts this server out even if it does. |
| `toon_output` | string | No | Per-server override for the global [`toon_output`](#toon-output-adaptive-result-encoding): `off`, `adaptive`, or `always`. Non-empty value wins over the global for this server's tools; omit to inherit. See [TOON Output](features/toon-output.md). |
| `tool_argument_error_mode` | string | No | Argument-error policy for this upstream: omitted, empty, or `detailed` retains diagnostic messages; `redacted` replaces eligible argument-error messages with `Invalid tool arguments.`. See [Tool Argument Errors](#tool-argument-errors). |
| `forward_headers` | array | No | Names of inbound MCP client HTTP headers to forward to this server on `tools/call`. Exact names only. See [Client Header Forwarding](#client-header-forwarding). Ignored for `stdio` and `sse`. `[]` clears it. |
| `created` | string | No | ISO 8601 timestamp (auto-generated) |
| `updated` | string | No | ISO 8601 timestamp (auto-updated) |

### Tool Argument Errors

Set `"tool_argument_error_mode": "redacted"` on each upstream that needs this policy. For indexed argument failures, MCPProxy returns `isError: true` with the exact message `Invalid tool arguments.` and retains `error_type: "invalid_params"`, the tool identifier, the static retry hint, and the full stored `input_schema`. The response has the same fields in `full` and `compact` tool-response modes. MCPProxy rejects arguments that fail its stored-schema validation before sending a tool call upstream.

The policy covers proxy-generated argument-error responses on the `call_tool_*` variants, the legacy `call_tool` handler, and direct routing mode (`routing_mode: "direct"`). This includes local schema rejections and eligible transport/JSON-RPC failures. Upstream `CallToolResult` responses, including those with `isError: true`, continue through the existing result-processing pipeline; this setting does not redact their content. Direct mode validates against the same stored schema before dispatch and applies the same upstream-argument policy, with two deliberate differences: its local rejection names the direct id it was called by (`server__tool`), while a shared rendered upstream error names the canonical id (`server:tool`); and every non-argument direct failure — authentication, transport, timeout, and the SDK identities the classifier cannot treat as argument errors — keeps the plain `Error calling server:tool: …` text it has always returned. The default and `detailed` policies never change direct-mode text.

Omitting the setting or using an empty value on create retains detailed errors. For partial updates through MCP or REST, omission and an empty string preserve the current setting; send `"detailed"` to reset a redacted policy. An empty value is treated as omitted. The `upstream_servers` tool advertises only the two explicit values (`detailed`, `redacted`): omitting the parameter is the supported way to leave the policy alone, because an empty-string enum member is rejected by some clients (#1576). REST still accepts an empty string as omitted. A REST PATCH with no effective update fields returns `400` (`No fields to update`). Unknown values reject before configuration writes. A request uses the policy captured at its dispatch gate; a later settings change applies to subsequent requests.

Upstream error redaction uses the existing conservative argument-error classifier. For eligible errors without an indexed schema, MCPProxy keeps the existing envelope and any available numeric code, suppresses untrusted message/data and nested HTTP body details, and adds no schema or repair hint. Authentication, transport, timeout, internal errors, and other noneligible errors retain their established responses. This setting does not suppress activity records or server logs.

In redacted mode, MCPProxy classifies the cause of its own top-level generic upstream-error wrapper so proxy-added authentication advice cannot mask a validation failure. It preserves arbitrary outer wrappers and genuine authentication context. Known non-argument SDK error identities remain ineligible even when their messages mention schema validation. The installed HTTP SDK converts JSON-RPC errors to Go errors and drops their code/data; a generic `Invalid params` message without recognized schema-validation wording remains noneligible and can retain diagnostics. This policy does not guarantee redaction of every upstream `-32602` response.

Scope: the setting is accepted through REST (`POST`/`PATCH /api/v1/servers`) and the `upstream_servers` MCP tool. `mcpproxy upstream add` has no flag for it, registry installs do not carry it, and the web/tray forms do not expose it, so servers created those ways default to `detailed` until updated. Code-execution sandbox calls hand the upstream error back to the running script instead of rendering a proxy MCP error result, so this policy does not apply there.

### Client Header Forwarding

MCPProxy can forward selected HTTP headers from the MCP client's request to an upstream server, for example `X-Tenant-Id` or a per-user `X-User-Id`, so an upstream that keys behaviour on a header can see it. It is enabled by default but inert until a server lists header names.
Expand Down
31 changes: 31 additions & 0 deletions internal/config/argument_error_mode.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
package config

import "fmt"

const (
ToolArgumentErrorModeDetailed = "detailed"
ToolArgumentErrorModeRedacted = "redacted"
)

// ValidateToolArgumentErrorMode rejects unknown policies before persistence.
// Empty is the backwards-compatible default on create and leaves an update
// unchanged; callers reset an existing policy with an explicit detailed value.
func ValidateToolArgumentErrorMode(mode string) error {
switch mode {
case "", ToolArgumentErrorModeDetailed, ToolArgumentErrorModeRedacted:
return nil
default:
return fmt.Errorf("tool_argument_error_mode must be empty, detailed, or redacted")
}
}

func (c *Config) validateToolArgumentErrorModes() error {
for _, server := range c.Servers {
if server != nil {
if err := ValidateToolArgumentErrorMode(server.ToolArgumentErrorMode); err != nil {
return err
}
}
}
return nil
}
86 changes: 86 additions & 0 deletions internal/config/argument_error_mode_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package config

import (
"encoding/json"
"os"
"path/filepath"
"testing"

"github.com/stretchr/testify/require"
)

// CFG-MODE-01: use the real loader and saver, including the on-disk enum.
func TestArgumentErrorModeLoadSave(t *testing.T) {
for _, mode := range []string{"omitted", "", "detailed", "redacted", "unknown"} {
t.Run(mode, func(t *testing.T) {
dir := t.TempDir()
server := map[string]interface{}{"name": "upstream", "url": "http://127.0.0.1:1/mcp", "protocol": "streamable-http", "enabled": false}
if mode != "omitted" {
server["tool_argument_error_mode"] = mode
}
fixture, err := json.Marshal(map[string]interface{}{"data_dir": dir, "mcpServers": []interface{}{server}})
require.NoError(t, err)
path := filepath.Join(dir, "mcp_config.json")
require.NoError(t, os.WriteFile(path, fixture, 0600))
cfg, err := LoadFromFile(path)
if mode == "unknown" {
require.ErrorContains(t, err, "tool_argument_error_mode")
after, readErr := os.ReadFile(path)
require.NoError(t, readErr)
require.Equal(t, fixture, after)
return
}
require.NoError(t, err)
require.NoError(t, SaveConfig(cfg, path))
roundtrip, err := LoadFromFile(path)
require.NoError(t, err)
encoded, err := json.Marshal(roundtrip.Servers[0])
require.NoError(t, err)
var got map[string]interface{}
require.NoError(t, json.Unmarshal(encoded, &got))
if mode == "omitted" || mode == "" {
require.NotContains(t, got, "tool_argument_error_mode")
} else {
require.Equal(t, mode, got["tool_argument_error_mode"])
}
})
}
}

func TestArgumentErrorModeInvalidSaveDoesNotWrite(t *testing.T) {
cfg := DefaultConfig()
cfg.DataDir = t.TempDir()
cfg.Servers = []*ServerConfig{{Name: "upstream", URL: "http://127.0.0.1:1/mcp", Protocol: "streamable-http", ToolArgumentErrorMode: "redacted"}}
path := filepath.Join(cfg.DataDir, "mcp_config.json")
require.NoError(t, SaveConfig(cfg, path))
before, err := os.ReadFile(path)
require.NoError(t, err)
cfg.Servers[0].ToolArgumentErrorMode = "unknown"
require.ErrorContains(t, cfg.Validate(), "tool_argument_error_mode")
require.NotEmpty(t, cfg.ValidateDetailed())
require.NotEmpty(t, ValidateServerForLoad(cfg.Servers[0]))
require.ErrorContains(t, SaveConfig(cfg, path), "tool_argument_error_mode")
after, err := os.ReadFile(path)
require.NoError(t, err)
require.Equal(t, before, after)
}

// CFG-MODE-02: exercise actual copies and merges, including auth preservation.
func TestArgumentErrorModeCopyMerge(t *testing.T) {
base := &ServerConfig{Name: "upstream", Enabled: true, ToolArgumentErrorMode: "redacted", Headers: map[string]string{"Authorization": "synthetic-token"}, OAuth: &OAuthConfig{ClientID: "synthetic-client", ClientSecret: "synthetic-secret"}}
copied := CopyServerConfig(base)
require.Equal(t, base, copied)
for _, tc := range []struct{ mode, want string }{{"", "redacted"}, {"detailed", "detailed"}, {"redacted", "redacted"}} {
t.Run("patch_"+tc.mode, func(t *testing.T) {
merged, _, err := MergeServerConfig(base, &ServerConfig{Enabled: true, ToolArgumentErrorMode: tc.mode}, DefaultMergeOptions())
require.NoError(t, err)
require.Equal(t, tc.want, merged.ToolArgumentErrorMode)
require.True(t, merged.Enabled)
require.Equal(t, base.Headers, merged.Headers)
require.Equal(t, base.OAuth, merged.OAuth)
require.Equal(t, "redacted", base.ToolArgumentErrorMode)
})
}
_, _, err := MergeServerConfig(base, &ServerConfig{ToolArgumentErrorMode: "unknown"}, DefaultMergeOptions())
require.ErrorContains(t, err, "tool_argument_error_mode")
}
19 changes: 13 additions & 6 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -754,12 +754,16 @@ type ServerConfig struct {
// scan (DefaultTrustMode, spec 119) and an unrecognized value as manual
// (fail closed). Read via
// EffectiveTrustMode(), never the raw string.
TrustMode string `json:"trust_mode,omitempty" mapstructure:"trust-mode"` // Per-server trust tier: auto|scan|manual. Supersedes auto_approve_tool_changes (086)
Shared bool `json:"shared,omitempty" mapstructure:"shared"` // Server edition: shared with all users
Created time.Time `json:"created" mapstructure:"created"`
Updated time.Time `json:"updated,omitempty" mapstructure:"updated"`
Isolation *IsolationConfig `json:"isolation,omitempty" mapstructure:"isolation"` // Per-server isolation settings
ReconnectOnUse bool `json:"reconnect_on_use,omitempty" mapstructure:"reconnect-on-use"` // Attempt reconnection when a tool call targets a disconnected server
TrustMode string `json:"trust_mode,omitempty" mapstructure:"trust-mode"` // Per-server trust tier: auto|scan|manual. Supersedes auto_approve_tool_changes (086)
// ToolArgumentErrorMode stores the per-upstream argument-error policy.
// Empty and detailed retain detailed errors; redacted opts into redaction.
// Updates omit/keep an empty value; detailed explicitly resets the policy.
ToolArgumentErrorMode string `json:"tool_argument_error_mode,omitempty" mapstructure:"tool_argument_error_mode"`
Shared bool `json:"shared,omitempty" mapstructure:"shared"` // Server edition: shared with all users
Created time.Time `json:"created" mapstructure:"created"`
Updated time.Time `json:"updated,omitempty" mapstructure:"updated"`
Isolation *IsolationConfig `json:"isolation,omitempty" mapstructure:"isolation"` // Per-server isolation settings
ReconnectOnUse bool `json:"reconnect_on_use,omitempty" mapstructure:"reconnect-on-use"` // Attempt reconnection when a tool call targets a disconnected server

// ExposePrompts overrides whether this server's advertised MCP prompts are
// aggregated into mcpproxy's prompts/list. nil (default) inherits the
Expand Down Expand Up @@ -2923,6 +2927,9 @@ func (c *Config) validateDetailedCore() []ValidationError {
serverNames := make(map[string]bool)
for i, server := range c.Servers {
fieldPrefix := fmt.Sprintf("mcpServers[%d]", i)
if err := ValidateToolArgumentErrorMode(server.ToolArgumentErrorMode); err != nil {
errors = append(errors, ValidationError{Field: fieldPrefix + ".tool_argument_error_mode", Message: err.Error()})
}

// Validate per-server isolation mode override (MCP-34.2).
if server.Isolation != nil && server.Isolation.Mode != nil && !server.Isolation.Mode.IsValid() {
Expand Down
3 changes: 3 additions & 0 deletions internal/config/loader.go
Original file line number Diff line number Diff line change
Expand Up @@ -536,6 +536,9 @@ func SaveConfig(cfg *Config, path string) error {
// telemetry.persistConfig documents — but a field it merely carries from an
// override is restored from the file, never from that stale copy.)
func SaveConfigWithEdits(cfg, mergeBase *Config, path string) error {
if err := cfg.validateToolArgumentErrorModes(); err != nil {
return err
}
saveConfigMu.Lock()
defer saveConfigMu.Unlock()
persisted := PersistableConfigWithEdits(cfg, mergeBase, path)
Expand Down
13 changes: 13 additions & 0 deletions internal/config/merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ func (d *ConfigDiff) IsEmpty() bool {
// - diff: Changes made during merge (nil if opts.GenerateDiff is false)
// - error: Validation or merge error
func MergeServerConfig(base, patch *ServerConfig, opts MergeOptions) (*ServerConfig, *ConfigDiff, error) {
if patch != nil {
if err := ValidateToolArgumentErrorMode(patch.ToolArgumentErrorMode); err != nil {
return nil, nil, fmt.Errorf("%w: %v", ErrInvalidConfig, err)
}
}
if base == nil {
// If no base, patch becomes the new config
if patch == nil {
Expand Down Expand Up @@ -237,6 +242,13 @@ func MergeServerConfig(base, patch *ServerConfig, opts MergeOptions) (*ServerCon
merged.TrustMode = patch.TrustMode
}

if patch.ToolArgumentErrorMode != "" && patch.ToolArgumentErrorMode != base.ToolArgumentErrorMode {
if diff != nil {
diff.Modified["tool_argument_error_mode"] = FieldChange{Path: "tool_argument_error_mode", From: base.ToolArgumentErrorMode, To: patch.ToolArgumentErrorMode}
}
merged.ToolArgumentErrorMode = patch.ToolArgumentErrorMode
}

// Boolean fields - use reflection to detect if explicitly set
// For booleans, we check if the patch has them set differently from base
// Since Go booleans default to false, we need a different approach for explicit false
Expand Down Expand Up @@ -630,6 +642,7 @@ func CopyServerConfig(src *ServerConfig) *ServerConfig {
Quarantined: src.Quarantined,
SkipQuarantine: src.SkipQuarantine,
TrustMode: src.TrustMode,
ToolArgumentErrorMode: src.ToolArgumentErrorMode,
Shared: src.Shared,
Created: src.Created,
Updated: src.Updated,
Expand Down
32 changes: 32 additions & 0 deletions internal/contracts/argument_error_mode_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package contracts

import (
"encoding/json"
"testing"

"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
"github.com/stretchr/testify/require"
)

// ADMIN-MODE-01: both conversion doors must carry the setting onto the REST
// wire, including a generic map that crossed a JSON boundary.
func TestArgumentErrorModeServerConversions(t *testing.T) {
for _, mode := range []string{"", "detailed", "redacted"} {
t.Run(mode, func(t *testing.T) {
direct := ConvertServerConfig(&config.ServerConfig{Name: "upstream", ToolArgumentErrorMode: mode}, nil, "disabled", false, 0, false)
require.Equal(t, mode, direct.ToolArgumentErrorMode)
encoded, err := json.Marshal([]*Server{direct})
require.NoError(t, err)
var maps []map[string]interface{}
require.NoError(t, json.Unmarshal(encoded, &maps))
if mode == "" {
require.NotContains(t, maps[0], "tool_argument_error_mode")
} else {
require.Equal(t, mode, maps[0]["tool_argument_error_mode"])
}
converted := ConvertGenericServersToTyped(maps)
require.Len(t, converted, 1)
require.Equal(t, mode, converted[0].ToolArgumentErrorMode)
})
}
}
6 changes: 5 additions & 1 deletion internal/contracts/converters.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ func ConvertServerConfig(cfg *config.ServerConfig, globalIsolation *config.Docke
AutoApproveToolChanges: cfg.AutoApproveToolChanges,
// Spec 086: surface the per-server trust_mode so callers can read back the
// persisted trust tier they set via PATCH/POST.
TrustMode: cfg.TrustMode,
TrustMode: cfg.TrustMode,
ToolArgumentErrorMode: cfg.ToolArgumentErrorMode,
// MCP-3322: surface the per-server init_timeout override so callers can
// read back a configured handshake deadline.
InitTimeout: cfg.InitTimeout,
Expand Down Expand Up @@ -302,6 +303,9 @@ func ConvertGenericServersToTyped(genericServers []map[string]interface{}) []Ser
// carries []string from the runtime projection or []interface{} after a
// JSON round-trip.
server.ForwardHeaders = stringListFromAny(generic["forward_headers"])
if mode, ok := generic["tool_argument_error_mode"].(string); ok {
server.ToolArgumentErrorMode = mode
}
// Spec 086: per-server trust tier round-trips as a plain string.
if trustMode, ok := generic["trust_mode"].(string); ok {
server.TrustMode = trustMode
Expand Down
3 changes: 2 additions & 1 deletion internal/contracts/types.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ type Server struct {
// trust tier ("auto"/"scan"/"manual"). Surfaced on the GET path so clients can
// read back the persisted mode; PATCH/POST accept it via AddServerRequest.
// Omitted when empty (server predates the field / relies on legacy flags).
TrustMode string `json:"trust_mode,omitempty"`
TrustMode string `json:"trust_mode,omitempty"`
ToolArgumentErrorMode string `json:"tool_argument_error_mode,omitempty"`
// InitTimeout mirrors config.ServerConfig.InitTimeout (MCP-3322 / GH #760):
// the per-server MCP `initialize` handshake deadline override. Serialized as
// a duration string (e.g. "120s"); nil/omitted means "inherit the global
Expand Down
Loading
Loading