feat(server): apply output_sanitisation to direct routing mode (#1685)

## Description

Fixes #1683.

`output_sanitisation` (redact, block, strip control characters,
spotlight) never ran for direct routing mode (`/mcp/all`, `server__tool`
names). The direct handler (`makeDirectModeHandler` in `mcp_routing.go`)
forwarded the upstream result without calling `applyOutputSanitisation`
or `spotlightForwarded`, which the `call_tool_*` handlers and
`code_execution` already call.

This adds both calls to the direct handler, mirroring
`handleCallToolVariant`:
- `applyOutputSanitisation` runs on the raw result before the activity
classification and truncation. A non-nil result is a block and is
returned as-is.
- `spotlightForwarded` runs after truncation, and `responseText` is
refreshed with `forwardedText` so the recorded activity matches what the
agent receives.

With no `output_sanitisation` configured both calls are no-ops, so
default behavior is byte-for-byte unchanged. Error results (`isError:
true`) and non-text blocks keep the existing exemptions inside
`applyOutputSanitisation`.

I did not touch the docs page that lists direct mode under "Scope and
limits"; that is in the separate docs site and I wasn't sure of the
process there. Happy to add that edit here if you want it.

## Testing
- [x] I have tested these changes locally
- [x] I have added/updated tests that prove my fix is effective or my
feature works
- [x] All existing tests pass (see note)

Verification:
- New `internal/server/direct_output_sanitisation_test.go` drives the
handler mcp-go registered (`directRoutingFixture`) against a real
in-process upstream: redact, block, spotlight, strip, and the opt-out
default. The first four fail on `main` and pass with this change. I
added a small `okBody` override to `countingUpstream` in
`scope_fixture_test.go` so an upstream can return a chosen payload.
- `go test ./internal/server/` passes with the e2e tests skipped (`-skip
'TestE2E|TestBinary|TestMCPProtocol|TestCodeExecClientModeE2E'`),
because those need a built binary and `npx` in my environment. CI will
cover them.
- On a v0.71.1 gateway with `output_sanitisation: {spotlight_untrusted,
response_action: redact, strip_control_chars}`: `/mcp` `call_tool_read`
returned `[REDACTED:cloud_credentials]` wrapped in `«untrusted:...»`,
while `/mcp/all` returned the raw key. This patch is meant to make the
second match the first.

---------

Co-authored-by: Algis Dumbris <a.dumbris@gmail.com>

Author: james-crowley

docs/features/output-sanitisation.md

@@ -37,7 +37,8 @@ Spotlighting wraps untrusted text as:
 ### Scope and limits
 
 - **Default behavior:** nothing is redacted or blocked. Sensitive-data detection still runs and flags records in the activity log, but it does so after the call is recorded and does not change what the agent receives.
-- **Two paths are not covered.** (1) **Direct routing mode** (`routing_mode: direct`, tools exposed as `server__tool`) never invokes sanitisation, so `redact` and `block` do not apply to those calls; a matching secret in a direct-mode text response is forwarded unchanged (detection still flags it in the activity log). Sanitisation applies to the `call_tool_*` wrapper path and to `code_execution` tool calls. (2) **Upstream error responses** (`isError: true`) bypass `redact` and `block`, so a secret echoed in an upstream error result is forwarded as-is, even with `response_action: block`.
+- **Covered paths.** Sanitisation applies to the `call_tool_*` wrapper path, to direct routing mode (`/mcp/all`, or `routing_mode: direct`, with tools exposed as `server__tool`), and to `code_execution` tool calls.
+- **Upstream error responses are not covered.** Upstream error responses (`isError: true`) bypass `redact` and `block`, so a secret echoed in an upstream error result is forwarded as-is, even with `response_action: block`.
 - **Text responses only.** `redact` and `block` act on text content in successful tool responses on the covered paths. Image, audio and embedded-resource content is never modified, and tool **arguments** sent to an upstream server are not covered.
 - **Pattern-based.** Only values matching the detector's patterns are redacted, and `block` triggers only on a **critical** detection. Redaction replaces the matched span; a multi-line secret such as a PEM private key may leave surrounding lines (in a local check with a synthetic key, only the `BEGIN` line was replaced). Well-known documentation example values (for instance the AWS documentation example access key) are classed as likely examples by the detector and were forwarded unredacted in a local check, while the accompanying secret key was masked. Treat redaction as risk reduction, not a guarantee that no secret reaches the agent.
 - **A canary check you can run locally:** point a scratch instance at a fixture server that returns a fake AWS key or private key and compare the response under the default config, `response_action: redact` and `response_action: block`. Use fake values only.

internal/server/direct_output_sanitisation_test.go

@@ -0,0 +1,74 @@
+package server
+
+import (
+	"context"
+	"testing"
+
+	"github.com/mark3labs/mcp-go/mcp"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
+	"github.com/smart-mcp-proxy/mcpproxy-go/internal/security"
+)
+
+// Issue #1683: direct routing mode (/mcp/all, server__tool names) is the fifth
+// upstream dispatch path and must honour output_sanitisation like call_tool_*
+// and code_execution do. Each test drives the handler mcp-go actually
+// registered (directRoutingFixture) against a real in-process upstream that
+// returns a secret.
+
+func directSanitisationFixture(t *testing.T, cfg *config.OutputSanitisationConfig, body string) *directRoutingFixture {
+	t.Helper()
+	f := newDirectRoutingFixture(t, directServerSpec{server: "github", tools: []toolSpec{readSpec("get_secret")}})
+	f.proxy.config.OutputSanitisation = cfg
+	f.proxy.sanitisationDetector = security.NewDetector(config.DefaultSensitiveDataDetectionConfig())
+	f.ups["github"].okBody.Store(&body)
+	return f
+}
+
+func directText(t *testing.T, r *mcp.CallToolResult) string {
+	t.Helper()
+	require.NotEmpty(t, r.Content)
+	return r.Content[0].(mcp.TextContent).Text
+}
+
+func TestDirectModeSanitisation_RedactMasksSecret(t *testing.T) {
+	f := directSanitisationFixture(t, sanCfg("redact", false, false), "token "+awsKeyFixture+" end")
+	res := f.call(t, context.Background(), "github", "get_secret")
+	require.False(t, res.IsError, directText(t, res))
+	assert.NotContains(t, directText(t, res), awsKeyFixture, "the agent must not receive the raw secret")
+	assert.Contains(t, directText(t, res), "token ", "non-secret text survives")
+}
+
+func TestDirectModeSanitisation_BlockRefusesCriticalResponse(t *testing.T) {
+	f := directSanitisationFixture(t, sanCfg("block", false, false), awsKeyFixture)
+	res := f.call(t, context.Background(), "github", "get_secret")
+	assert.True(t, res.IsError, "a critical detection in block mode must block")
+	assert.Contains(t, directText(t, res), "blocked by sanitisation policy")
+	assert.NotContains(t, directText(t, res), awsKeyFixture)
+}
+
+func TestDirectModeSanitisation_SpotlightWrapsUntrustedText(t *testing.T) {
+	f := directSanitisationFixture(t, sanCfg("spotlight", true, false), "hello")
+	res := f.call(t, context.Background(), "github", "get_secret")
+	require.False(t, res.IsError, directText(t, res))
+	text := directText(t, res)
+	assert.Contains(t, text, "untrusted:github/get_secret")
+	assert.Contains(t, text, "hello")
+}
+
+func TestDirectModeSanitisation_StripRemovesControlSequences(t *testing.T) {
+	f := directSanitisationFixture(t, sanCfg("spotlight", false, true), "plain\x1b[31mred\x1b[0m")
+	res := f.call(t, context.Background(), "github", "get_secret")
+	require.False(t, res.IsError, directText(t, res))
+	assert.NotContains(t, directText(t, res), "\x1b[")
+}
+
+func TestDirectModeSanitisation_DefaultOptOutForwardsUnchanged(t *testing.T) {
+	f := directSanitisationFixture(t, config.DefaultOutputSanitisationConfig(), "token "+awsKeyFixture)
+	res := f.call(t, context.Background(), "github", "get_secret")
+	require.False(t, res.IsError, directText(t, res))
+	assert.Equal(t, "token "+awsKeyFixture, directText(t, res),
+		"with no output_sanitisation configured the response is forwarded byte for byte")
+}

internal/server/mcp_routing.go

@@ -739,6 +739,15 @@ func (p *MCPProxyServer) makeDirectModeHandler(entry *directCatalogEntry) mcpser
 			return mcp.NewToolResultError(fmt.Sprintf("Error calling %s:%s: %v", serverName, toolName, err)), nil
 		}
 
+		// Spec 054 Track B (pre-forward): redact secrets, strip control sequences
+		// or block on a critical detection, exactly as the call_tool_* handlers
+		// do. Direct mode previously skipped this, so output_sanitisation never
+		// applied to /mcp/all. Applied to the RAW result before the activity
+		// classification and truncation below; a non-nil result means blocked.
+		if blockResult := p.applyOutputSanitisation(ctx, serverName, toolName, requestID, directContentTrust, result); blockResult != nil {
+			return blockResult, nil
+		}
+
 		// Determine tool variant for activity logging
 		toolVariant := contracts.DeriveCallWith(annotations)
 
@@ -812,6 +821,13 @@ func (p *MCPProxyServer) makeDirectModeHandler(entry *directCatalogEntry) mcpser
 			forwarded = mcp.NewToolResultText(responseText)
 		}
 
+		// Spec 054 Track B (post-forward): spotlight untrusted text in
+		// source-identifying delimiters. Lossless, applied after truncation;
+		// responseText is refreshed so the activity record matches what the
+		// agent receives.
+		p.spotlightForwarded(serverName, toolName, directContentTrust, forwarded)
+		responseText = forwardedText(forwarded, responseText)
+
 		// Emit completion activity (success, or error when the upstream itself
 		// reported one — issue #935).
 		// Spec 069 A1: pre-truncation sizes; result was measured before the truncation loop above.

internal/server/scope_fixture_test.go

@@ -102,6 +102,9 @@ type countingUpstream struct {
 	// errBody, when set, makes every tool answer an upstream isError result
 	// carrying that text (an approved upstream failing on its own).
 	errBody atomic.Pointer[string]
+	// okBody, when set, replaces the default "ok" text of a successful answer
+	// (for tests that need the upstream to return a particular payload).
+	okBody atomic.Pointer[string]
 }
 
 // serve registers one more tool on the running stub upstream, for tests
@@ -130,6 +133,9 @@ func (u *countingUpstream) serve(spec toolSpec) {
 			if body := u.errBody.Load(); body != nil {
 				return mcp.NewToolResultError(*body), nil
 			}
+			if body := u.okBody.Load(); body != nil {
+				return mcp.NewToolResultText(*body), nil
+			}
 			return mcp.NewToolResultText("ok"), nil
 		})
 }