diff --git a/cmd/gortex/daemon_mcp.go b/cmd/gortex/daemon_mcp.go index cfaa12f8..78a43b97 100644 --- a/cmd/gortex/daemon_mcp.go +++ b/cmd/gortex/daemon_mcp.go @@ -156,6 +156,12 @@ func (d *mcpDispatcher) Dispatch(ctx context.Context, sess *daemon.Session, fram // Record only permitted calls to deferred/learned tools so a rejected // hidden call cannot refresh learned-surface state. d.srv.NoteToolUse(name, sess.CWD, newly) + // Promotion alone is not enough: mcp-go re-applies the tools/list + // filter to the requested tool at call time and reports a filtered + // tool as "not found". Mark the call as authorized so the surface + // filter recognises that probe (see WithAuthorizedToolCall); the + // per-call gate inside the handler still decides the outcome. + ctx = gortexmcp.WithAuthorizedToolCall(ctx, name) } } diff --git a/internal/mcp/promote_call_gate_test.go b/internal/mcp/promote_call_gate_test.go new file mode 100644 index 00000000..212c8337 --- /dev/null +++ b/internal/mcp/promote_call_gate_test.go @@ -0,0 +1,123 @@ +package mcp + +import ( + "context" + "encoding/json" + "strings" + "testing" + + "github.com/stretchr/testify/require" +) + +// callToolFrame dispatches one tools/call frame through the real MCP server +// entry point the daemon dispatcher uses and returns the marshalled reply. +func callToolFrame(t *testing.T, srv *Server, ctx context.Context, tool string, args map[string]any) string { + t.Helper() + if args == nil { + args = map[string]any{} + } + frame, err := json.Marshal(map[string]any{ + "jsonrpc": "2.0", "id": 1, "method": "tools/call", + "params": map[string]any{"name": tool, "arguments": args}, + }) + require.NoError(t, err) + reply := srv.MCPServer().HandleMessage(ctx, json.RawMessage(frame)) + require.NotNil(t, reply) + out, err := json.Marshal(reply) + require.NoError(t, err) + return string(out) +} + +// TestPromotedToolCallableUnderCorePreset covers the by-name reachability +// contract: a tool held out of a session's tools/list (the `core` defer preset) +// stays callable by name after the dispatcher promotes it. +// +// Regression: mcp-go v0.55.1 started re-running registered tool filters at call +// time (server.passesToolFilters). toolSurfaceFilter shapes tools/list +// visibility, so every non-preset tool — 141 of 178 under `core` — began +// answering "tool '' not found: tool not found" for CLI (`gortex call`) +// and for any agent session that forwarded a preset, including calls to tools +// tools_search had just promoted. +func TestPromotedToolCallableUnderCorePreset(t *testing.T) { + srv, _ := setupTestServer(t) + + const sessionID = "cli_core_defer" + const toolName = "export_context" + srv.NoteSessionToolPolicy(sessionID, "core", "defer") + ctx := WithSessionID(context.Background(), sessionID) + + require.Equal(t, "deferred", srv.sessionToolStatus(ctx, toolName), + "precondition: export_context is deferred under core/defer") + require.True(t, srv.IsToolEnabledForSession(ctx, toolName), + "precondition: the dispatcher's promote gate permits this call") + + // What the daemon dispatcher does before HandleMessage. + srv.EnsureToolPromotedForSession(ctx, toolName) + ctx = WithAuthorizedToolCall(ctx, toolName) + + out := callToolFrame(t, srv, ctx, toolName, map[string]any{"task": "cli flags"}) + require.NotContains(t, out, "tool not found", + "a promoted tool must be callable by name under core/defer:\n%s", clip(out)) +} + +// TestAuthorizedCallMarkerKeepsHideModeEnforced proves the carve-out cannot be +// used to widen a session's surface: a hide-mode preset still refuses a tool +// outside its allow-set even when the marker is present, and the refusal is the +// structured blocked-by-mode error rather than a misleading "not found". +func TestAuthorizedCallMarkerKeepsHideModeEnforced(t *testing.T) { + srv, _ := setupTestServer(t) + + const sessionID = "hide_mode_client" + const toolName = "export_context" + srv.NoteSessionToolPolicy(sessionID, "nav", "hide") + ctx := WithSessionID(context.Background(), sessionID) + + require.False(t, srv.IsToolEnabledForSession(ctx, toolName), + "precondition: a hide-mode session must not be allowed to call a non-preset tool") + + // The dispatcher would never mark this call; set it anyway so the test + // covers the gate rather than the dispatcher's restraint. + out := callToolFrame(t, srv, WithAuthorizedToolCall(ctx, toolName), toolName, nil) + require.Contains(t, out, string(ErrCodeToolBlockedByMode), + "hide mode must still refuse the call:\n%s", clip(out)) +} + +// TestAuthorizedCallMarkerDoesNotWidenToolsList pins the marker to the +// call-time single-tool probe: a tools/list rendered on the same context still +// shows only the session's preset surface. +func TestAuthorizedCallMarkerDoesNotWidenToolsList(t *testing.T) { + srv, _ := setupTestServer(t) + + const sessionID = "list_under_marker" + const toolName = "export_context" + srv.NoteSessionToolPolicy(sessionID, "core", "defer") + ctx := WithSessionID(context.Background(), sessionID) + srv.EnsureToolPromotedForSession(ctx, toolName) + ctx = WithAuthorizedToolCall(ctx, toolName) + + reply := srv.MCPServer().HandleMessage(ctx, + json.RawMessage(`{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}`)) + out, err := json.Marshal(reply) + require.NoError(t, err) + + var parsed struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(out, &parsed)) + for _, tool := range parsed.Result.Tools { + require.NotEqual(t, toolName, tool.Name, + "the authorized-call marker must not leak a non-preset tool into tools/list") + } + require.NotEmpty(t, parsed.Result.Tools, "the preset surface should still render") +} + +func clip(s string) string { + if len(s) <= 400 { + return s + } + return strings.TrimSpace(s[:400]) + "…" +} diff --git a/internal/mcp/session_ctx.go b/internal/mcp/session_ctx.go index 5e088fda..dc71dd22 100644 --- a/internal/mcp/session_ctx.go +++ b/internal/mcp/session_ctx.go @@ -78,6 +78,44 @@ func SessionCWDFromContext(ctx context.Context) string { return "" } +// authorizedCallCtxKey carries the tool name of an inbound tools/call that +// the session's own authorization already permitted (see +// Server.IsToolEnabledForSession). It exists because mcp-go re-runs every +// registered tool filter at CALL time (server.passesToolFilters, added in +// mcp-go v0.55.1) with the single requested tool: a filter that only shapes +// tools/list VISIBILITY would otherwise turn a legitimate by-name call into +// "tool '' not found". Unexported key: set it via +// WithAuthorizedToolCall. +type authorizedCallCtxKey struct{} + +// WithAuthorizedToolCall returns a context marking name as an authorized +// by-name tools/call for this session. The daemon's MCP dispatcher sets it +// after IsToolEnabledForSession accepts the call and before HandleMessage, so +// toolSurfaceFilter knows the follow-up single-tool filter invocation is +// mcp-go's call-time access check and not a tools/list render. +// +// This never widens what a session may call: the marker is only set for names +// the session's effective surface already permits, and checkToolGate still +// runs the authoritative per-call gate inside the handler wrapper. +func WithAuthorizedToolCall(ctx context.Context, name string) context.Context { + if name == "" { + return ctx + } + return context.WithValue(ctx, authorizedCallCtxKey{}, name) +} + +// authorizedToolCallFromContext returns the tool name attached via +// WithAuthorizedToolCall, or "" when none is present. +func authorizedToolCallFromContext(ctx context.Context) string { + if ctx == nil { + return "" + } + if name, ok := ctx.Value(authorizedCallCtxKey{}).(string); ok { + return name + } + return "" +} + // repoAllowCtxKey carries the per-request repo allow-set resolved by // handleAnalyze (resolveScope → ResolvedScope.RepoAllow). The scoped- // node accessors (scopedNodes / scopedNodesByKinds / scopedNodeSlice) diff --git a/internal/mcp/tools_mode.go b/internal/mcp/tools_mode.go index 5cabe7f2..a3177b87 100644 --- a/internal/mcp/tools_mode.go +++ b/internal/mcp/tools_mode.go @@ -39,6 +39,18 @@ func (s *Server) editingToolsHidden(ctx context.Context) bool { } func (s *Server) toolSurfaceFilter(ctx context.Context, tools []mcp.Tool) []mcp.Tool { + // mcp-go v0.55.1+ re-runs every registered tool filter at CALL time + // (server.passesToolFilters) with just the requested tool, and answers + // "tool '' not found" when the filter drops it. This filter shapes + // tools/list VISIBILITY — a deferred tool promoted on demand, or any tool + // outside a narrow preset, is still callable by name (checkToolGate is the + // authoritative call gate). Let that single-tool probe through when the + // dispatcher already authorized this exact call, or the visibility rule + // silently becomes a call gate that reports the tool as missing. + if name := authorizedToolCallFromContext(ctx); name != "" && + len(tools) == 1 && tools[0].Name == name { + return tools + } if s.editingToolsHidden(ctx) { tools = withoutMutatingTools(tools) }