From a640ff07f54d9ba9b7d2317afa100f628f781610 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Mon, 28 Sep 2026 22:20:33 +0300 Subject: [PATCH 01/11] feat(profile): enforce profile policy on every execution surface (Spec 108-d) --- internal/auth/scoped_view.go | 30 ++ internal/auth/scoped_view_test.go | 23 ++ internal/config/profiles.go | 42 +-- internal/config/profiles_rollout_gate_test.go | 216 +----------- internal/config/profiles_v3_test.go | 5 +- internal/config/rollout_gate_ast_test.go | 226 +++--------- .../config/testdata/overrideprobe/main.go | 16 - internal/httpapi/code_exec.go | 4 + internal/httpapi/code_exec_status_test.go | 10 + internal/httpapi/concurrency_shed_test.go | 65 +++- .../config_profile_v3_rollout_gate_test.go | 55 +-- .../httpapi/profile_rest_discovery_test.go | 210 +++++++++++ internal/httpapi/profile_tools.go | 35 ++ internal/httpapi/server.go | 54 ++- internal/httpapi/server_global_tools_test.go | 44 ++- internal/jsruntime/runtime.go | 6 + internal/profile/refusal_errors.go | 22 ++ internal/runtime/activity_service.go | 16 +- internal/runtime/activity_service_test.go | 33 ++ internal/runtime/event_bus.go | 13 + internal/server/call_tool_profile_v3_test.go | 134 +++++++ .../server/code_execution_profile_v3_test.go | 329 ++++++++++++++++++ .../server/describe_tool_profile_v3_test.go | 8 + internal/server/direct_profile_v3_test.go | 77 ++++ internal/server/mcp.go | 124 +++++-- internal/server/mcp_block_tools_test.go | 6 + internal/server/mcp_code_execution.go | 60 +++- internal/server/mcp_direct_scope.go | 5 + internal/server/mcp_routing.go | 90 ++++- internal/server/mcp_tail_log_scope_test.go | 36 ++ internal/server/mcp_visibility.go | 5 + internal/server/profile_index_policy_test.go | 2 - internal/server/profile_refusals.go | 26 ++ internal/server/profile_resolver_v3.go | 40 ++- internal/server/profile_rest_discovery.go | 74 ++++ .../server/profile_rest_discovery_test.go | 40 +++ internal/server/profile_tool.go | 5 + internal/server/profile_tool_dispatch.go | 44 +++ internal/server/profiles_v3_fixture_test.go | 5 +- internal/server/replay_audit_test.go | 21 ++ .../server/retrieve_tools_profile_v3_test.go | 49 ++- internal/server/scope_fixture_test.go | 16 +- .../server/scope_latency_profile_v3_test.go | 1 - internal/server/scope_oracle_v3_test.go | 40 ++- internal/server/scoped_view_guard_test.go | 45 +++ internal/server/server.go | 22 ++ internal/server/set_profile_v3_test.go | 32 ++ internal/storage/activity_models.go | 3 + 48 files changed, 1928 insertions(+), 536 deletions(-) create mode 100644 internal/auth/scoped_view.go create mode 100644 internal/auth/scoped_view_test.go delete mode 100644 internal/config/testdata/overrideprobe/main.go create mode 100644 internal/httpapi/profile_rest_discovery_test.go create mode 100644 internal/httpapi/profile_tools.go create mode 100644 internal/profile/refusal_errors.go create mode 100644 internal/server/call_tool_profile_v3_test.go create mode 100644 internal/server/code_execution_profile_v3_test.go create mode 100644 internal/server/profile_refusals.go create mode 100644 internal/server/profile_rest_discovery.go create mode 100644 internal/server/profile_rest_discovery_test.go create mode 100644 internal/server/profile_tool_dispatch.go create mode 100644 internal/server/scoped_view_guard_test.go create mode 100644 internal/server/set_profile_v3_test.go diff --git a/internal/auth/scoped_view.go b/internal/auth/scoped_view.go new file mode 100644 index 000000000..1762dbb98 --- /dev/null +++ b/internal/auth/scoped_view.go @@ -0,0 +1,30 @@ +package auth + +// ScopedView returns the request's existing authorization identity unless it +// is an anonymous MCP caller confined by a named profile. Such callers retain +// the historical admin-shaped AnonymousContext everywhere else; management +// handlers use this non-admin view so their operation gates match a client +// credential without granting secret-reveal or administrator behavior. +func ScopedView(ac *AuthContext, confinedAnonymous bool) *AuthContext { + if !confinedAnonymous { + return ac + } + var view AuthContext + if ac != nil { + view = *ac + } + view.Type = AuthTypeAgent + view.Anonymous = true + view.CredentialKind = CredentialKindAnonymous + view.AllowedServers = []string{"*"} + view.Permissions = []string{PermRead, PermWrite, PermDestructive} + return &view +} + +// IsAdminOrAbsent keeps the historical admin-shaped management view for an +// unconfined anonymous MCP request while allowing ScopedView to narrow a +// confined anonymous request first. +func IsAdminOrAbsent(ac *AuthContext) bool { return ac == nil || ac.IsAdmin() } + +// IsNonAdmin is the nil-safe counterpart used by handlers after ScopedView. +func IsNonAdmin(ac *AuthContext) bool { return ac != nil && !ac.IsAdmin() } diff --git a/internal/auth/scoped_view_test.go b/internal/auth/scoped_view_test.go new file mode 100644 index 000000000..64667b973 --- /dev/null +++ b/internal/auth/scoped_view_test.go @@ -0,0 +1,23 @@ +package auth + +import "testing" + +func TestScopedView_ConfinedAnonymousBecomesNonAdminForManagement(t *testing.T) { + admin := AnonymousContext() + view := ScopedView(admin, true) + if view == admin || view.IsAdmin() || !view.Anonymous { + t.Fatalf("confined anonymous view must be a non-admin copy: %#v", view) + } + if !view.CanAccessServer("any-server") || !view.HasPermission(PermRead) || !view.HasPermission(PermDestructive) { + t.Fatalf("the view must defer capability limits to the effective profile: %#v", view) + } + if got := ScopedView(admin, false); got != admin { + t.Fatal("unconfined anonymous callers must keep the historical context") + } + if got := ScopedView(nil, false); got != nil { + t.Fatal("unconfined in-process calls keep the nil context") + } + if got := ScopedView(nil, true); got == nil || got.IsAdmin() { + t.Fatal("a nil confined anonymous caller must still receive a non-admin view") + } +} diff --git a/internal/config/profiles.go b/internal/config/profiles.go index 769a2eb67..812a0bfa0 100644 --- a/internal/config/profiles.go +++ b/internal/config/profiles.go @@ -3,8 +3,6 @@ package config import ( "fmt" "regexp" - "sync/atomic" - "testing" "unicode/utf8" ) @@ -125,22 +123,11 @@ const ( ProfileUnannotatedAsRead = "as_read" ) -// policyEnforcementReadyBase is the FR-009a rollout gate's compile-time -// value: false from PR 108-a and flips to true in PR 108-d, in the same -// commit that lands the last execution gate — so a release built from any -// commit between 108-a and 108-d rejects every v3 policy field at load time -// and behaves exactly like a pre-108 build for profiles (discovery can never -// hide a tool that execution would still run). It is an unexported const — -// never a package variable — so no import can open the gate by assigning to -// it; the only way to open it outside 108-d is EnablePolicyForTest below. -const policyEnforcementReadyBase = false - -// policyEnforcementTestOverride is the FR-009a test-only override's flag -// (zcode review: "the override cannot ship" — it must be unreachable from -// any non-test call site). It is flipped only by EnablePolicyForTest, whose -// own testing.Testing() guard is what keeps it out of production, not the -// atomic type. -var policyEnforcementTestOverride atomic.Bool +// policyEnforcementReadyBase is the FR-009a rollout gate. Spec 108-d wires +// enforcement into every MCP and REST execution/discovery path, so this +// compile-time constant now admits Profiles v3 policy fields and anonymous +// confinement in every build. +const policyEnforcementReadyBase = true // PolicyEnforcementReady reports whether the FR-009a rollout gate is open: // no build may admit a v3 policy field it cannot yet enforce on every @@ -154,24 +141,7 @@ var policyEnforcementTestOverride atomic.Bool // a reference the other way round would cycle. internal/profile and every // other consumer calls config.PolicyEnforcementReady() directly. func PolicyEnforcementReady() bool { - return policyEnforcementReadyBase || (policyEnforcementTestOverride.Load() && testing.Testing()) -} - -// EnablePolicyForTest opens the FR-009a gate for the duration of the -// caller's test only (the 108-a/108-b "test-only override" the spec and -// tasks.md call for). It panics when called outside a test binary -// (testing.Testing() false) — no env var, config field, flag or build tag -// can open the gate — and registers a tb.Cleanup that closes it again when -// the test ends, so callers need no defer/restore bookkeeping of their own. -// It takes testing.TB (not *testing.T) so any package's tests — not only -// internal/config's — can use it. -func EnablePolicyForTest(tb testing.TB) { - if !testing.Testing() { - panic("config: EnablePolicyForTest called outside a test binary") - } - tb.Helper() - policyEnforcementTestOverride.Store(true) - tb.Cleanup(func() { policyEnforcementTestOverride.Store(false) }) + return policyEnforcementReadyBase } // profileSlugPattern is the allowed profile-name form (FR-007): lowercase diff --git a/internal/config/profiles_rollout_gate_test.go b/internal/config/profiles_rollout_gate_test.go index 7c9613541..b556eb42d 100644 --- a/internal/config/profiles_rollout_gate_test.go +++ b/internal/config/profiles_rollout_gate_test.go @@ -1,34 +1,20 @@ package config import ( - "fmt" - "net" - "os" - "os/exec" - "path/filepath" - "runtime" "testing" "github.com/stretchr/testify/require" ) -// TestPolicyEnforcementReady_RollsOutGate pins the FR-009a rollout gate: with -// PolicyEnforcementReady() false (the value shipped from 108-a until -// 108-d), each of the six v3 policy fields AND a non-empty -// anonymous_profile (known or unknown, data-model.md §1) are a fatal -// validation error with the exact data-model.md §1 text; a legacy profile -// still loads. 108-d's T055a asserts the constant flips to true and inverts -// this test. -func TestPolicyEnforcementReady_RollsOutGate(t *testing.T) { - require.False(t, PolicyEnforcementReady(), "must ship false until 108-d lands the last execution gate (FR-009a)") +func TestPolicyEnforcementReady_After108D(t *testing.T) { + require.True(t, PolicyEnforcementReady(), "108-d enforces Profiles v3 across execution and discovery") trueVal := true falseVal := false switchTo := []string{"other"} - cases := []struct { - field string - set func(p *ProfileConfig) + name string + set func(*ProfileConfig) }{ {"max_tier", func(p *ProfileConfig) { p.MaxTier = "read" }}, {"unannotated", func(p *ProfileConfig) { p.Unannotated = "deny" }}, @@ -38,199 +24,23 @@ func TestPolicyEnforcementReady_RollsOutGate(t *testing.T) { {"switchable_to", func(p *ProfileConfig) { p.SwitchableTo = &switchTo }}, } for _, tc := range cases { - t.Run(tc.field, func(t *testing.T) { + t.Run(tc.name, func(t *testing.T) { p := ProfileConfig{Name: "prof", Servers: []string{"a"}} tc.set(&p) - cfg := &Config{Profiles: []ProfileConfig{p, {Name: "other", Servers: []string{"a"}}}} - + cfg := &Config{Servers: []*ServerConfig{{Name: "a"}}, Profiles: []ProfileConfig{p, {Name: "other", Servers: []string{"a"}}}} _, err := ValidateProfiles(cfg) - require.EqualError(t, err, "profiles[0]: "+tc.field+" is not supported by this build (Profiles v3 enforcement incomplete)") + require.NoError(t, err) }) } - t.Run("legacy profile still loads", func(t *testing.T) { - cfg := &Config{Servers: []*ServerConfig{{Name: "a"}}, Profiles: []ProfileConfig{{Name: "legacy", Servers: []string{"a"}, Title: "Legacy"}}} - _, err := ValidateProfiles(cfg) - require.NoError(t, err) - }) - - t.Run("anonymous_profile naming a known profile is fatal while the gate is closed", func(t *testing.T) { - cfg := &Config{Servers: []*ServerConfig{{Name: "a"}}, AnonymousProfile: "legacy", Profiles: []ProfileConfig{{Name: "legacy", Servers: []string{"a"}}}} - _, err := ValidateProfiles(cfg) - require.EqualError(t, err, "anonymous_profile is not supported by this build (Profiles v3 enforcement incomplete)") - }) - - t.Run("anonymous_profile naming an unknown profile is fatal while the gate is closed", func(t *testing.T) { - cfg := &Config{AnonymousProfile: "missing"} - _, err := ValidateProfiles(cfg) - require.EqualError(t, err, "anonymous_profile is not supported by this build (Profiles v3 enforcement incomplete)") - }) - - t.Run("anonymous_profile is fatal even with zero profiles configured", func(t *testing.T) { - cfg := &Config{AnonymousProfile: "legacy"} - _, err := ValidateProfiles(cfg) - require.EqualError(t, err, "anonymous_profile is not supported by this build (Profiles v3 enforcement incomplete)") - }) - - t.Run("test-only override admits v3 fields", func(t *testing.T) { - EnablePolicyForTest(t) - cfg := &Config{Profiles: []ProfileConfig{{Name: "prof", Servers: []string{"a"}, MaxTier: "read"}}} - _, err := ValidateProfiles(cfg) - require.NoError(t, err) - }) - - t.Run("test-only override admits anonymous_profile", func(t *testing.T) { - EnablePolicyForTest(t) - cfg := &Config{Servers: []*ServerConfig{{Name: "a"}}, AnonymousProfile: "legacy", Profiles: []ProfileConfig{{Name: "legacy", Servers: []string{"a"}}}} + t.Run("anonymous profile confinement is accepted", func(t *testing.T) { + cfg := &Config{ + Servers: []*ServerConfig{{Name: "a"}}, + AnonymousProfile: "legacy", + Profiles: []ProfileConfig{{Name: "legacy", Servers: []string{"a"}}}, + } warnings, err := ValidateProfiles(cfg) require.NoError(t, err) require.Empty(t, warnings) }) - - t.Run("override closes the gate again once the test that opened it ends", func(t *testing.T) { - require.False(t, PolicyEnforcementReady()) - t.Run("nested", func(t *testing.T) { - EnablePolicyForTest(t) - require.True(t, PolicyEnforcementReady()) - }) - require.False(t, PolicyEnforcementReady(), "tb.Cleanup from the nested subtest must have closed the gate") - }) -} - -// TestEnablePolicyForTest_PanicsOutsideATestBinary builds a plain `main` -// package that calls config.EnablePolicyForTest(nil) and runs it with `go -// build`+exec (not `go test`), asserting the override is unreachable outside -// a test binary: testing.Testing() is false there, so the panic fires (T004a -// item 2, zcode review round 2: "the override cannot ship"). -func TestEnablePolicyForTest_PanicsOutsideATestBinary(t *testing.T) { - if testing.Short() { - t.Skip("skipping go build subprocess probe in -short mode") - } - bin := filepath.Join(t.TempDir(), "overrideprobe") - if runtime.GOOS == "windows" { - bin += ".exe" - } - build := exec.Command("go", "build", "-o", bin, "./testdata/overrideprobe") - if out, err := build.CombinedOutput(); err != nil { - t.Fatalf("go build overrideprobe: %v\n%s", err, out) - } - - out, err := exec.Command(bin).CombinedOutput() - require.Error(t, err, "overrideprobe must exit non-zero: EnablePolicyForTest must panic outside a test binary") - require.Contains(t, string(out), "EnablePolicyForTest called outside a test binary") -} - -// TestServeRejectsV3PolicyFieldsAtStartup is T004a's item 3 (zcode review -// round 2, "the override cannot ship" — the third of the three artifacts -// tasks.md lists, still missing after round 2 added only item 2 above): a -// BINARY-level check that the FR-009a gate cannot be opened by any runtime -// input. It builds the real ./cmd/mcpproxy binary (not `go test`) and runs -// `serve` against a scratch --data-dir/--config (high port, scratch HOME) -// containing first a profile with a v3 policy field (max_tier) and then a -// bare anonymous_profile, asserting exit code 4 (ExitCodeConfigError, -// cmd/mcpproxy/exit_codes.go) and the exact data-model.md §1 refusal text on -// stderr in both cases. A handful of plausible-looking MCPPROXY_* env vars -// are set to arbitrary values in the child's environment (a deliberately -// minimal env otherwise — no inherited variable can smuggle the gate open -// either): none of them is read by PolicyEnforcementReady (its only inputs -// are the policyEnforcementReadyBase compile-time const and the test-only -// override that TestEnablePolicyForTest_PanicsOutsideATestBinary above -// proves is unreachable outside `go test`), so none of them can open it. -func TestServeRejectsV3PolicyFieldsAtStartup(t *testing.T) { - if testing.Short() { - t.Skip("skipping go build + subprocess serve probe in -short mode") - } - - bin := filepath.Join(t.TempDir(), "mcpproxy-gateprobe") - if runtime.GOOS == "windows" { - bin += ".exe" - } - build := exec.Command("go", "build", "-o", bin, "github.com/smart-mcp-proxy/mcpproxy-go/cmd/mcpproxy") - if out, err := build.CombinedOutput(); err != nil { - t.Fatalf("go build ./cmd/mcpproxy: %v\n%s", err, out) - } - - cases := []struct { - name string - configJSON string - wantStderr string - }{ - { - name: "v3 policy field (max_tier)", - configJSON: `{"profiles": [{"name": "prof", "servers": [], "max_tier": "read"}]}`, - wantStderr: "profiles[0]: max_tier is not supported by this build (Profiles v3 enforcement incomplete)", - }, - { - name: "anonymous_profile", - configJSON: `{"anonymous_profile": "someone"}`, - wantStderr: "anonymous_profile is not supported by this build (Profiles v3 enforcement incomplete)", - }, - } - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - dir := t.TempDir() - home := filepath.Join(dir, "home") - dataDir := filepath.Join(dir, "data") - require.NoError(t, os.MkdirAll(home, 0o700)) - require.NoError(t, os.MkdirAll(dataDir, 0o700)) - - configPath := filepath.Join(dir, "config.json") - require.NoError(t, os.WriteFile(configPath, []byte(tc.configJSON), 0o600)) - - //nolint:gosec // test-only: fixed binary this test just built, fixed args - cmd := exec.Command(bin, "serve", - "--config", configPath, - "--data-dir", dataDir, - "--listen", fmt.Sprintf("127.0.0.1:%d", freeHighPort(t)), - ) - cmd.Env = arbitraryServeEnv(home) - - out, err := cmd.CombinedOutput() - var exitErr *exec.ExitError - require.Error(t, err, "serve must exit non-zero when the FR-009a gate refuses the config; output:\n%s", out) - require.ErrorAs(t, err, &exitErr, "output:\n%s", out) - require.Equal(t, 4, exitErr.ExitCode(), "must exit ExitCodeConfigError (cmd/mcpproxy/exit_codes.go); output:\n%s", out) - require.Contains(t, string(out), tc.wantStderr) - }) - } -} - -// freeHighPort picks a free high port for a scratch --listen so the probe -// binary never collides with a developer's real proxy or another test. -func freeHighPort(t *testing.T) int { - t.Helper() - ln, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - defer func() { _ = ln.Close() }() - return ln.Addr().(*net.TCPAddr).Port -} - -// arbitraryServeEnv builds a deliberately minimal, arbitrary environment for -// the probe binary: just enough (HOME/PATH, plus TMPDIR/TEMP so the Go -// runtime and OS calls the binary makes have somewhere to write) to run at -// all, pointed at a scratch home so the boot path's early DataDir fallback -// (internal/config/loader.go, before the --data-dir flag override applies) -// can never touch the real ~/.mcpproxy, PLUS a handful of MCPPROXY_* -// variables with names that sound like they might matter here and values -// that very much would if they did — proving none of them opens the FR-009a -// gate. -func arbitraryServeEnv(home string) []string { - env := []string{ - "HOME=" + home, - "PATH=" + os.Getenv("PATH"), - "MCPPROXY_API_KEY=arbitrary-test-key", - "MCPPROXY_DEBUG=true", - "MCPPROXY_LISTEN=0.0.0.0:0", - "MCPPROXY_TELEMETRY=false", - "MCPPROXY_POLICY_ENFORCEMENT_READY=true", - "MCPPROXY_ENABLE_PROFILES_V3=true", - "MCPPROXY_ANONYMOUS_PROFILE=legacy", - } - if runtime.GOOS == "windows" { - env = append(env, "USERPROFILE="+home, "TEMP="+os.Getenv("TEMP"), "TMP="+os.Getenv("TMP")) - } else { - env = append(env, "TMPDIR="+os.Getenv("TMPDIR")) - } - return env } diff --git a/internal/config/profiles_v3_test.go b/internal/config/profiles_v3_test.go index b5ff00c02..80c4464b2 100644 --- a/internal/config/profiles_v3_test.go +++ b/internal/config/profiles_v3_test.go @@ -143,11 +143,8 @@ func TestProfileConfig_EffectiveCodeExecution(t *testing.T) { } // TestValidateProfiles_V3Rules pins every fatal/warning message row of -// data-model.md §1 exactly, with the FR-009a gate opened via -// EnablePolicyForTest (the 108-a/108-b test-only override; see -// profiles_rollout_gate_test.go for the gate itself). +// data-model.md §1 exactly. func TestValidateProfiles_V3Rules(t *testing.T) { - EnablePolicyForTest(t) t.Run("invalid max_tier is fatal", func(t *testing.T) { cfg := &Config{Profiles: []ProfileConfig{{Name: "prof", Servers: []string{"a"}, MaxTier: "bogus"}}} diff --git a/internal/config/rollout_gate_ast_test.go b/internal/config/rollout_gate_ast_test.go index 6a59a4fda..dda83e2ef 100644 --- a/internal/config/rollout_gate_ast_test.go +++ b/internal/config/rollout_gate_ast_test.go @@ -1,222 +1,110 @@ package config -// T004a item (1) — the go/ast structural test the FR-009a rollout gate -// requires (zcode round 2, "the override cannot ship"; a later zcode round -// found only items (2) (TestEnablePolicyForTest_PanicsOutsideATestBinary) and -// (3) (TestServeRejectsV3PolicyFieldsAtStartup) had landed in -// profiles_rollout_gate_test.go — this file is the missing third artifact). -// -// The gate lives in this package (profiles.go), not package profile — see the -// PolicyEnforcementReady doc comment there for why — so this test parses -// profiles.go's own AST rather than a separate internal/profile/rollout_gate.go. -// It proves, independently of what profiles_rollout_gate_test.go's behavioural -// assertions happen to observe, that: -// -// 1. PolicyEnforcementReady's body reads only policyEnforcementReadyBase, -// policyEnforcementTestOverride and testing.Testing() — no os.Getenv/ -// os.LookupEnv, no flag, no config field, no other package-level -// identifier — so no runtime input can open the gate. -// 2. policyEnforcementReadyBase is declared `const` (not `var`): no import -// can flip it by assignment. -// 3. profiles.go carries no `//go:build` line: the gate's value does not -// depend on build tags (a `-tags server` build sees the same false). -// 4. Walking every non-test .go file in the module, EnablePolicyForTest and -// policyEnforcementTestOverride are referenced from no production file -// other than profiles.go itself and the dedicated override-probe binary -// (testdata/overrideprobe/main.go, T004a item (2)'s fixture, which exists -// precisely to prove the override panics outside a test binary). import ( "go/ast" "go/parser" "go/token" "io/fs" - "os" "path/filepath" - "strconv" "strings" "testing" ) -// rolloutGateAllowedIdents is the exact set of package-level identifiers -// PolicyEnforcementReady's body may reference, besides local syntax -// (testing.Testing is checked separately as a selector). -var rolloutGateAllowedIdents = map[string]bool{ - "policyEnforcementReadyBase": true, - "policyEnforcementTestOverride": true, - "Load": true, // atomic.Bool.Load method selector -} - -// rolloutGateOverrideRefExemptFiles are the only non-test .go files (relative -// to the module root, forward-slash) allowed to reference EnablePolicyForTest -// or policyEnforcementTestOverride. profiles.go declares them; the override -// probe calls EnablePolicyForTest by design (T004a item 2) to prove it panics -// there. -var rolloutGateOverrideRefExemptFiles = map[string]bool{ - "internal/config/profiles.go": true, - "internal/config/testdata/overrideprobe/main.go": true, -} - -func TestRolloutGateAST_PolicyEnforcementReadyReadsOnlyItsOwnInputs(t *testing.T) { +func TestPolicyGateAST_IsCompileTimeEnabledAndHasNoTestOverride(t *testing.T) { root := latentGuardRepoRoot(t) path := filepath.Join(root, "internal", "config", "profiles.go") - - raw, err := os.ReadFile(path) - if err != nil { - t.Fatalf("read %s: %v", path, err) - } - if strings.Contains(string(raw), "//go:build") { - t.Errorf("%s carries a //go:build line; the FR-009a gate's value must not depend on build tags", latentRelPath(root, path)) - } - fset := token.NewFileSet() - f, err := parser.ParseFile(fset, path, nil, parser.ParseComments) + f, err := parser.ParseFile(fset, path, nil, 0) if err != nil { - t.Fatalf("parse %s: %v", path, err) + t.Fatalf("parse profiles.go: %v", err) } - - var ( - gateFunc *ast.FuncDecl - baseIsConst bool - foundBase bool - ) + constFound := false for _, decl := range f.Decls { - switch d := decl.(type) { - case *ast.FuncDecl: - if d.Recv == nil && d.Name.Name == "PolicyEnforcementReady" { - gateFunc = d + gen, ok := decl.(*ast.GenDecl) + if !ok || gen.Tok != token.CONST { + continue + } + for _, spec := range gen.Specs { + value, ok := spec.(*ast.ValueSpec) + if !ok { + continue } - case *ast.GenDecl: - for _, spec := range d.Specs { - vs, ok := spec.(*ast.ValueSpec) - if !ok { + for i, name := range value.Names { + if name.Name != "policyEnforcementReadyBase" { + continue + } + constFound = true + if i >= len(value.Values) { + t.Errorf("policyEnforcementReadyBase must be the compile-time constant true") continue } - for _, n := range vs.Names { - if n.Name == "policyEnforcementReadyBase" { - foundBase = true - baseIsConst = d.Tok == token.CONST - } + actual, isIdent := value.Values[i].(*ast.Ident) + if !isIdent || actual.Name != "true" { + t.Errorf("policyEnforcementReadyBase must be the compile-time constant true") } } } } + if !constFound { + t.Fatal("policyEnforcementReadyBase const not found") + } - if !foundBase { - t.Fatal("policyEnforcementReadyBase declaration not found in profiles.go; the gate's compile-time flag moved or was renamed") + var gate *ast.FuncDecl + for _, decl := range f.Decls { + fn, ok := decl.(*ast.FuncDecl) + if ok && fn.Name.Name == "PolicyEnforcementReady" { + gate = fn + break + } } - if !baseIsConst { - t.Error("policyEnforcementReadyBase must be declared `const`, not `var` — a var lets any import in the package flip the gate by assignment") + if gate == nil || gate.Body == nil || len(gate.Body.List) != 1 { + t.Fatal("PolicyEnforcementReady must be a single-return gate") } - - if gateFunc == nil { - t.Fatal("func PolicyEnforcementReady not found in profiles.go") + ret, ok := gate.Body.List[0].(*ast.ReturnStmt) + if !ok || len(ret.Results) != 1 { + t.Fatal("PolicyEnforcementReady must return only its compile-time constant") } - if gateFunc.Body == nil { - t.Fatal("PolicyEnforcementReady has no body to inspect") + id, ok := ret.Results[0].(*ast.Ident) + if !ok || id.Name != "policyEnforcementReadyBase" { + t.Fatal("PolicyEnforcementReady must return policyEnforcementReadyBase") } - var violations []string - ast.Inspect(gateFunc.Body, func(n ast.Node) bool { - sel, ok := n.(*ast.SelectorExpr) - if ok { - if pkgIdent, ok := sel.X.(*ast.Ident); ok { - switch pkgIdent.Name { - case "testing": - if sel.Sel.Name != "Testing" { - pos := fset.Position(sel.Pos()) - violations = append(violations, "testing."+sel.Sel.Name+" at line "+strconv.Itoa(pos.Line)+" (only testing.Testing() is allowed)") - } - return false // do not descend into "testing" ident below - case "policyEnforcementTestOverride": - if !rolloutGateAllowedIdents[sel.Sel.Name] { - pos := fset.Position(sel.Pos()) - violations = append(violations, "policyEnforcementTestOverride."+sel.Sel.Name+" at line "+strconv.Itoa(pos.Line)+" (only .Load() is allowed)") - } - return false - default: - // Any other selector base (os.Getenv, os.LookupEnv, flag.*, - // a config field, ...) is a forbidden input to the gate. - pos := fset.Position(sel.Pos()) - violations = append(violations, pkgIdent.Name+"."+sel.Sel.Name+" at line "+strconv.Itoa(pos.Line)) - return false - } - } - return true - } - ident, ok := n.(*ast.Ident) - if !ok { - return true - } - switch ident.Name { - case "policyEnforcementReadyBase", "policyEnforcementTestOverride", "testing", "bool", "true", "false": - return true - } - if rolloutGateAllowedIdents[ident.Name] { - return true - } - pos := fset.Position(ident.Pos()) - violations = append(violations, "identifier "+ident.Name+" at line "+strconv.Itoa(pos.Line)) - return true - }) - - if len(violations) > 0 { - t.Errorf("PolicyEnforcementReady reads input(s) beyond policyEnforcementReadyBase/policyEnforcementTestOverride/testing.Testing(): %v", violations) + forbidden := map[string]bool{ + strings.Join([]string{"EnablePolicy", "ForTest"}, ""): true, + strings.Join([]string{"policyEnforcement", "TestOverride"}, ""): true, } -} - -// TestRolloutGateAST_OverrideUnreachableFromProductionCode walks every -// non-test .go file in the module and fails if EnablePolicyForTest or -// policyEnforcementTestOverride is referenced from any file other than the -// two exempt ones (profiles.go and the dedicated override probe). -func TestRolloutGateAST_OverrideUnreachableFromProductionCode(t *testing.T) { - root := latentGuardRepoRoot(t) - fset := token.NewFileSet() - var violations []string - err := filepath.WalkDir(root, func(path string, d fs.DirEntry, walkErr error) error { + err = filepath.WalkDir(root, func(path string, entry fs.DirEntry, walkErr error) error { if walkErr != nil { return walkErr } - if d.IsDir() { - switch d.Name() { + if entry.IsDir() { + switch entry.Name() { case ".git", "node_modules", "frontend", ".claude": return filepath.SkipDir } return nil } - name := d.Name() - if !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") { + if !strings.HasSuffix(entry.Name(), ".go") || strings.HasSuffix(entry.Name(), "_test.go") { return nil } - rel := latentRelPath(root, path) - - f, perr := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) - if perr != nil { - t.Fatalf("parse %s: %v", rel, perr) + file, parseErr := parser.ParseFile(fset, path, nil, parser.SkipObjectResolution) + if parseErr != nil { + return parseErr } - ast.Inspect(f, func(n ast.Node) bool { - ident, ok := n.(*ast.Ident) - if !ok { - return true - } - if ident.Name != "EnablePolicyForTest" && ident.Name != "policyEnforcementTestOverride" { - return true + ast.Inspect(file, func(node ast.Node) bool { + ident, ok := node.(*ast.Ident) + if ok && forbidden[ident.Name] { + violations = append(violations, latentRelPath(root, path)+":"+ident.Name) } - if rolloutGateOverrideRefExemptFiles[rel] { - return true - } - pos := fset.Position(ident.Pos()) - violations = append(violations, ident.Name+" referenced at "+rel+":"+strconv.Itoa(pos.Line)) return true }) return nil }) if err != nil { - t.Fatalf("walk %s: %v", root, err) + t.Fatalf("walk production Go files: %v", err) } - - if len(violations) > 0 { - t.Errorf("FR-009a override reachable from production code outside profiles.go / the override probe (%d reference(s)): %v", - len(violations), violations) + if len(violations) != 0 { + t.Errorf("test-only rollout override identifiers remain in production Go files: %v", violations) } } diff --git a/internal/config/testdata/overrideprobe/main.go b/internal/config/testdata/overrideprobe/main.go deleted file mode 100644 index 44bc8b3a0..000000000 --- a/internal/config/testdata/overrideprobe/main.go +++ /dev/null @@ -1,16 +0,0 @@ -// Command overrideprobe is not part of the module's build graph (it lives -// under a "testdata" directory, which `go build ./...`/`go vet ./...` -// ignore). It exists solely for -// TestEnablePolicyForTest_PanicsOutsideATestBinary -// (internal/config/profiles_rollout_gate_test.go), which builds it -// explicitly with `go build` and runs the resulting binary to prove -// config.EnablePolicyForTest panics outside a test binary — the FR-009a -// invariant that the rollout-gate override is unreachable from production -// code. -package main - -import "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" - -func main() { - config.EnablePolicyForTest(nil) -} diff --git a/internal/httpapi/code_exec.go b/internal/httpapi/code_exec.go index 44b9dcc8b..a02d4157a 100644 --- a/internal/httpapi/code_exec.go +++ b/internal/httpapi/code_exec.go @@ -13,6 +13,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/codescripts" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/reqcontext" ) @@ -223,6 +224,9 @@ func (h *CodeExecHandler) ServeHTTP(w http.ResponseWriter, r *http.Request) { // spec 093 uses for a concurrency shed's 429), so the returned message is the // error's own wording, without the dispatch wrapper's "tool call failed:". func classifyCodeExecError(err error) (status int, code, message string, ok bool) { + if errors.Is(err, profile.ErrCodeExecutionBlocked) { + return http.StatusForbidden, "PROFILE_BLOCKED", profile.ErrCodeExecutionBlocked.Error(), true + } if errors.Is(err, config.ErrCodeExecutionDisabled) { return http.StatusForbidden, "FEATURE_DISABLED", config.CodeExecutionDisabledMessage, true } diff --git a/internal/httpapi/code_exec_status_test.go b/internal/httpapi/code_exec_status_test.go index 58c63dedd..22efda852 100644 --- a/internal/httpapi/code_exec_status_test.go +++ b/internal/httpapi/code_exec_status_test.go @@ -14,6 +14,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/codescripts" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/httpapi" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" ) // postFailingCodeExec runs one POST /api/v1/code/exec against a tool caller @@ -171,3 +172,12 @@ func TestCodeExec_DisabledFeatureIsForbidden(t *testing.T) { assert.Contains(t, decoded.Error.Message, "enable_code_execution") } } + +func TestCodeExec_ProfileBlockedPrecedesGlobalFeatureGate(t *testing.T) { + err := fmt.Errorf("tool call failed: %w", profile.ErrCodeExecutionBlocked) + w, decoded := postFailingCodeExec(t, map[string]interface{}{"code": "1"}, err) + assert.Equal(t, http.StatusForbidden, w.Code) + require.NotNil(t, decoded.Error) + assert.Equal(t, "PROFILE_BLOCKED", decoded.Error.Code) + assert.Equal(t, "blocked by profile: code execution is disabled for this profile", decoded.Error.Message) +} diff --git a/internal/httpapi/concurrency_shed_test.go b/internal/httpapi/concurrency_shed_test.go index 2946b6177..0975d4cee 100644 --- a/internal/httpapi/concurrency_shed_test.go +++ b/internal/httpapi/concurrency_shed_test.go @@ -16,6 +16,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/reqcontext" "github.com/smart-mcp-proxy/mcpproxy-go/internal/upstream/limiter" ) @@ -39,8 +40,14 @@ func (m *shedController) CallTool(_ context.Context, _ string, _ map[string]inte } func postToolCall(t *testing.T, srv *Server, apiKey string) *httptest.ResponseRecorder { + return postToolCallNamed(t, srv, apiKey, "call_tool_read") +} + +func postToolCallNamed(t *testing.T, srv *Server, apiKey, toolName string) *httptest.ResponseRecorder { t.Helper() - body := strings.NewReader(`{"tool_name":"call_tool_read","arguments":{"name":"db:query"}}`) + bodyJSON, err := json.Marshal(map[string]interface{}{"tool_name": toolName, "arguments": map[string]interface{}{}}) + require.NoError(t, err) + body := strings.NewReader(string(bodyJSON)) req := httptest.NewRequest(http.MethodPost, "/api/v1/tools/call", body) req.Header.Set("X-API-Key", apiKey) req.Header.Set("Content-Type", "application/json") @@ -49,6 +56,21 @@ func postToolCall(t *testing.T, srv *Server, apiKey string) *httptest.ResponseRe return w } +func TestHandleCallTool_ProfileHiddenCodeExecutionMatchesUnknownTool(t *testing.T) { + apiKey := "test-profile-code-exec-api-key" + profileCtrl := &shedController{apiKey: apiKey, err: fmt.Errorf("tool call failed: %w", profile.ErrCodeExecutionBlocked)} + unknownCtrl := &shedController{apiKey: apiKey, err: fmt.Errorf("unknown tool: no_such_tool")} + profileResponse := postToolCallNamed(t, NewServer(profileCtrl, zap.NewNop().Sugar(), nil), apiKey, "code_execution") + unknownResponse := postToolCallNamed(t, NewServer(unknownCtrl, zap.NewNop().Sugar(), nil), apiKey, "no_such_tool") + + require.Equal(t, http.StatusInternalServerError, profileResponse.Code) + require.Equal(t, unknownResponse.Code, profileResponse.Code) + var profileBody, unknownBody map[string]interface{} + require.NoError(t, json.Unmarshal(profileResponse.Body.Bytes(), &profileBody)) + require.NoError(t, json.Unmarshal(unknownResponse.Body.Bytes(), &unknownBody)) + require.Equal(t, strings.ReplaceAll(unknownBody["error"].(string), "no_such_tool", "code_execution"), profileBody["error"]) +} + // TestHandleCallTool_ShedReturns429WithRetryAfter is the FR-011 contract: the // REST surface answers a concurrency shed with 429 and a Retry-After hint // derived from the shedding scope's effective queue_timeout — not the blanket @@ -111,6 +133,21 @@ func TestHandleCallTool_ShedReturns429WithRetryAfter(t *testing.T) { } } +func TestHandleCallTool_ProfileBlockedReturns403(t *testing.T) { + apiKey := "test-profile-api-key" + message := "blocked by profile: github:create_issue is a write tool; this profile allows read tools only" + ctrl := &shedController{ + apiKey: apiKey, + err: &profile.ToolBlockedError{Reason: profile.BlockReasonTier, Message: message}, + } + srv := NewServer(ctrl, zap.NewNop().Sugar(), nil) + + w := postToolCall(t, srv, apiKey) + + require.Equal(t, http.StatusForbidden, w.Code) + assert.Contains(t, w.Body.String(), message) +} + // TestHandleCallTool_ServerUnavailableIsNot429 keeps FR-009 separate from // FR-011: a server that went away mid-queue is not backpressure. func TestHandleCallTool_ServerUnavailableIsNot429(t *testing.T) { @@ -165,6 +202,32 @@ func TestHandleReplayToolCall_ShedReturns429(t *testing.T) { assert.Contains(t, msg, limiter.RetryAdvice) } +func TestReplayToolCall_ProfileGateUses403AndNonDisclosing404(t *testing.T) { + apiKey := "test-replay-profile-api-key" + message := "blocked by profile: github:create_issue is denied by a profile rule" + cases := []struct { + name string + err error + want int + body string + }{ + {name: "policy denial", err: &profile.ToolBlockedError{Reason: profile.BlockReasonRule, Message: message}, want: http.StatusForbidden, body: message}, + {name: "server outside profile", err: profile.ErrToolOutsideProfile, want: http.StatusNotFound, body: "Tool call not found"}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + ctrl := &replayShedController{shedController{apiKey: apiKey, err: tc.err}} + srv := NewServer(ctrl, zap.NewNop().Sugar(), nil) + req := httptest.NewRequest(http.MethodPost, "/api/v1/tool-calls/hidden-call/replay", strings.NewReader(`{}`)) + req.Header.Set("X-API-Key", apiKey) + w := httptest.NewRecorder() + srv.ServeHTTP(w, req) + require.Equal(t, tc.want, w.Code) + assert.Contains(t, w.Body.String(), tc.body) + }) + } +} + type replayShedController struct { shedController } diff --git a/internal/httpapi/config_profile_v3_rollout_gate_test.go b/internal/httpapi/config_profile_v3_rollout_gate_test.go index 34c24351d..21584295a 100644 --- a/internal/httpapi/config_profile_v3_rollout_gate_test.go +++ b/internal/httpapi/config_profile_v3_rollout_gate_test.go @@ -1,18 +1,8 @@ package httpapi -// T004a (FR-009a): the rollout gate that rejects any v3 policy field (and a -// non-empty anonymous_profile) while config.PolicyEnforcementReady() is false -// must be proven through BOTH REST write doors, not only through config load -// (internal/config/profiles_rollout_gate_test.go) — a regression in the -// PATCH/apply handler plumbing (e.g. ValidateDetailed skipped, or the -// message re-wrapped) would otherwise ship undetected while the config-load -// and binary-level probes stay green. -// -// The fake controller's ApplyConfig calls the REAL config.ValidateDetailed() -// on the config it is handed — exactly the first step -// internal/runtime.Runtime.applyConfigLocked takes before persisting anything -// — so these tests exercise the real ValidateProfiles/FR-009a logic through -// the handlers' real decode/merge/unmask pipeline, not a canned result. +// FR-009a rollout completion: the fake controller's ApplyConfig calls the +// real config.ValidateDetailed(), so these tests pin that both REST write +// doors now admit supported Profiles v3 policies and anonymous confinement. import ( "bytes" @@ -78,37 +68,29 @@ func profileGateDo(t *testing.T, srv *Server, method, path string, body []byte) return w } -// TestPatchConfig_RejectsV3PolicyFieldWhileGateClosed: PATCH /api/v1/config -// with a v3 policy field (max_tier) on a new profile is refused with the -// exact FR-009a message, and ApplyConfig's success path is never reached. -func TestPatchConfig_RejectsV3PolicyFieldWhileGateClosed(t *testing.T) { - require.False(t, config.PolicyEnforcementReady(), "test assumes the gate ships closed") +func TestPatchConfig_AcceptsV3PolicyField(t *testing.T) { srv, ctrl := newProfileGateServer(t) body := []byte(`{"profiles":[{"name":"prof","servers":[],"max_tier":"read"}]}`) w := profileGateDo(t, srv, http.MethodPatch, "/api/v1/config", body) - require.Equal(t, http.StatusBadRequest, w.Code, "body=%s", w.Body.String()) - assert.Contains(t, w.Body.String(), "max_tier is not supported by this build") - assert.Zero(t, ctrl.applied, "a config rejected by the gate must never be persisted") + require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String()) + assert.Equal(t, 1, ctrl.applied) } // TestPatchConfig_RejectsAnonymousProfileWhileGateClosed: PATCH with a bare // anonymous_profile (no policy fields at all) is refused the same way. -func TestPatchConfig_RejectsAnonymousProfileWhileGateClosed(t *testing.T) { +func TestPatchConfig_AcceptsAnonymousProfile(t *testing.T) { srv, ctrl := newProfileGateServer(t) body := []byte(`{"anonymous_profile":"someone"}`) w := profileGateDo(t, srv, http.MethodPatch, "/api/v1/config", body) - require.Equal(t, http.StatusBadRequest, w.Code, "body=%s", w.Body.String()) - assert.Contains(t, w.Body.String(), "anonymous_profile is not supported by this build") - assert.Zero(t, ctrl.applied) + require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String()) + assert.Equal(t, 1, ctrl.applied) } -// TestPatchConfig_LegacyProfileStillPasses is the control: a legacy profile -// (no v3 policy field set) is accepted by the same PATCH door while the gate -// is closed, so the fix above is scoped to v3 fields only. +// TestPatchConfig_LegacyProfileStillPasses preserves legacy profile support. func TestPatchConfig_LegacyProfileStillPasses(t *testing.T) { srv, ctrl := newProfileGateServer(t) @@ -119,10 +101,7 @@ func TestPatchConfig_LegacyProfileStillPasses(t *testing.T) { assert.Equal(t, 1, ctrl.applied) } -// TestApplyConfig_RejectsV3PolicyFieldWhileGateClosed: POST -// /api/v1/config/apply with a full document carrying a v3 policy field is -// refused the same way as PATCH. -func TestApplyConfig_RejectsV3PolicyFieldWhileGateClosed(t *testing.T) { +func TestApplyConfig_AcceptsV3PolicyField(t *testing.T) { srv, ctrl := newProfileGateServer(t) doc := defaultConfigDocument(t) @@ -134,14 +113,13 @@ func TestApplyConfig_RejectsV3PolicyFieldWhileGateClosed(t *testing.T) { w := profileGateDo(t, srv, http.MethodPost, "/api/v1/config/apply", body) - require.Equal(t, http.StatusBadRequest, w.Code, "body=%s", w.Body.String()) - assert.Contains(t, w.Body.String(), "max_tier is not supported by this build") - assert.Zero(t, ctrl.applied, "a config rejected by the gate must never be persisted") + require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String()) + assert.Equal(t, 1, ctrl.applied) } // TestApplyConfig_RejectsAnonymousProfileWhileGateClosed: POST // /api/v1/config/apply with a bare anonymous_profile is refused the same way. -func TestApplyConfig_RejectsAnonymousProfileWhileGateClosed(t *testing.T) { +func TestApplyConfig_AcceptsAnonymousProfile(t *testing.T) { srv, ctrl := newProfileGateServer(t) doc := defaultConfigDocument(t) @@ -151,9 +129,8 @@ func TestApplyConfig_RejectsAnonymousProfileWhileGateClosed(t *testing.T) { w := profileGateDo(t, srv, http.MethodPost, "/api/v1/config/apply", body) - require.Equal(t, http.StatusBadRequest, w.Code, "body=%s", w.Body.String()) - assert.Contains(t, w.Body.String(), "anonymous_profile is not supported by this build") - assert.Zero(t, ctrl.applied) + require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String()) + assert.Equal(t, 1, ctrl.applied) } // TestApplyConfig_LegacyProfileStillPasses is the control for the apply door. diff --git a/internal/httpapi/profile_rest_discovery_test.go b/internal/httpapi/profile_rest_discovery_test.go new file mode 100644 index 000000000..58c5eabcd --- /dev/null +++ b/internal/httpapi/profile_rest_discovery_test.go @@ -0,0 +1,210 @@ +package httpapi + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "testing" + + "github.com/go-chi/chi/v5" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/management" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" + "github.com/stretchr/testify/require" + "go.uber.org/zap" +) + +type profileRESTDiscoveryController struct { + *globalToolsController + managementService *profileRESTDiscoveryManagementService + approvals []*storage.ToolApprovalRecord +} + +type profileRESTDiscoveryManagementService struct { + management.Service + controller *globalToolsController +} + +func (m *profileRESTDiscoveryManagementService) GetServerTools(_ context.Context, server string) ([]map[string]interface{}, error) { + return m.controller.GetServerTools(server) +} + +func (c *profileRESTDiscoveryController) GetManagementService() management.Service { + return c.managementService +} + +func (c *profileRESTDiscoveryController) ListToolApprovals(server string) ([]*storage.ToolApprovalRecord, error) { + rows := make([]*storage.ToolApprovalRecord, 0, len(c.approvals)) + for _, row := range c.approvals { + if server == "" || row.ServerName == server { + rows = append(rows, row) + } + } + return rows, nil +} + +func (c *profileRESTDiscoveryController) ToolAllowedByProfile(ctx context.Context, serverName, toolName string) bool { + ac := auth.AuthContextFromContext(ctx) + if ac == nil || ac.IsAdmin() || ac.ProfilePin == "" { + return true + } + return c.globalToolsController.ToolAllowedByProfile(ctx, serverName, toolName) +} + +func (c *profileRESTDiscoveryController) SearchToolsForProfile(ctx context.Context, query string, limit int, inScope func(string) bool) ([]map[string]interface{}, bool, error) { + ac := auth.AuthContextFromContext(ctx) + if ac == nil || ac.IsAdmin() || ac.ProfilePin == "" { + return nil, false, nil + } + return c.globalToolsController.SearchToolsForProfile(ctx, query, limit, inScope) +} + +func pinnedDiscoveryContext() context.Context { + return auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeAgent, AgentName: "readonly", ProfilePin: "work-readonly", + AllowedServers: []string{"github"}, Permissions: []string{auth.PermRead}, + }) +} + +func profileRouteRequest(caller context.Context, method, path string, params ...[2]string) *http.Request { + req := httptest.NewRequest(method, path, http.NoBody) + ctx := chi.NewRouteContext() + for _, param := range params { + ctx.URLParams.Add(param[0], param[1]) + } + return req.WithContext(context.WithValue(caller, chi.RouteCtxKey, ctx)) +} + +func TestRESTDiscovery_ProfileFiltersServerToolsAndExportAndHidesDiff(t *testing.T) { + controller := &profileRESTDiscoveryController{ + globalToolsController: &globalToolsController{ + allServers: []map[string]interface{}{{"name": "github", "id": "github"}}, + serverTools: map[string][]map[string]interface{}{ + "github": { + {"name": "list_issues", "server_name": "github", "description": "List issues"}, + {"name": "create_issue", "server_name": "github", "description": "Create issue"}, + }, + }, + profileAllowed: map[string]bool{"github\x00create_issue": false}, + approvals: map[string]*storage.ToolApprovalRecord{ + "github\x00list_issues": {ServerName: "github", ToolName: "list_issues", Status: storage.ToolApprovalStatusApproved}, + "github\x00create_issue": {ServerName: "github", ToolName: "create_issue", Status: storage.ToolApprovalStatusChanged}, + }, + }, + approvals: []*storage.ToolApprovalRecord{ + {ServerName: "github", ToolName: "list_issues", Status: storage.ToolApprovalStatusApproved}, + {ServerName: "github", ToolName: "create_issue", Status: storage.ToolApprovalStatusChanged}, + }, + } + controller.managementService = &profileRESTDiscoveryManagementService{controller: controller.globalToolsController} + srv := NewServer(controller, zap.NewNop().Sugar(), nil) + ctx := pinnedDiscoveryContext() + + t.Run("per-server tools", func(t *testing.T) { + req := profileRouteRequest(ctx, http.MethodGet, "/api/v1/servers/github/tools", [2]string{"id", "github"}) + w := httptest.NewRecorder() + srv.handleGetServerTools(w, req) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var payload struct { + Data struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &payload)) + require.Len(t, payload.Data.Tools, 1) + require.Equal(t, "list_issues", payload.Data.Tools[0].Name) + }) + + t.Run("export", func(t *testing.T) { + req := profileRouteRequest(ctx, http.MethodGet, "/api/v1/servers/github/tools/export", [2]string{"id", "github"}) + w := httptest.NewRecorder() + srv.handleExportToolDescriptions(w, req) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var payload struct { + Data struct { + Count int `json:"count"` + Tools []struct { + ToolName string `json:"tool_name"` + } `json:"tools"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &payload)) + require.Equal(t, 1, payload.Data.Count) + require.Len(t, payload.Data.Tools, 1) + require.Equal(t, "list_issues", payload.Data.Tools[0].ToolName) + }) + + t.Run("diff for excluded tool is the same not-found shape", func(t *testing.T) { + req := profileRouteRequest(ctx, http.MethodGet, "/api/v1/servers/github/tools/create_issue/diff", + [2]string{"id", "github"}, [2]string{"tool", "create_issue"}) + w := httptest.NewRecorder() + srv.handleGetToolDiff(w, req) + require.Equal(t, http.StatusNotFound, w.Code) + require.Contains(t, w.Body.String(), "Tool approval record not found") + }) + + t.Run("unprofiled administrator keeps the current rows and diff", func(t *testing.T) { + admin := auth.WithAuthContext(context.Background(), auth.AdminContext()) + toolsReq := profileRouteRequest(admin, http.MethodGet, "/api/v1/servers/github/tools", [2]string{"id", "github"}) + toolsResponse := httptest.NewRecorder() + srv.handleGetServerTools(toolsResponse, toolsReq) + require.Equal(t, http.StatusOK, toolsResponse.Code, toolsResponse.Body.String()) + var toolsPayload struct { + Data struct { + Count int `json:"count"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(toolsResponse.Body.Bytes(), &toolsPayload)) + require.Equal(t, 2, toolsPayload.Data.Count) + + exportReq := profileRouteRequest(admin, http.MethodGet, "/api/v1/servers/github/tools/export", [2]string{"id", "github"}) + exportResponse := httptest.NewRecorder() + srv.handleExportToolDescriptions(exportResponse, exportReq) + require.Equal(t, http.StatusOK, exportResponse.Code, exportResponse.Body.String()) + var exportPayload struct { + Data struct { + Count int `json:"count"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(exportResponse.Body.Bytes(), &exportPayload)) + require.Equal(t, 2, exportPayload.Data.Count) + + diffReq := profileRouteRequest(admin, http.MethodGet, "/api/v1/servers/github/tools/create_issue/diff", + [2]string{"id", "github"}, [2]string{"tool", "create_issue"}) + diffResponse := httptest.NewRecorder() + srv.handleGetToolDiff(diffResponse, diffReq) + require.Equal(t, http.StatusOK, diffResponse.Code, diffResponse.Body.String()) + }) +} + +func TestRESTDiscovery_ProfileSearchUsesPrelimitedProfileResults(t *testing.T) { + controller := &globalToolsController{ + profileSearchResults: []map[string]interface{}{{ + "tool": map[string]interface{}{"name": "list_issues", "server_name": "github", "description": "List issues"}, + "score": 2.0, + }}, + profileSearchHandled: true, + } + srv := NewServer(controller, zap.NewNop().Sugar(), nil) + req := httptest.NewRequest(http.MethodGet, "/api/v1/index/search?q=issue&limit=1", http.NoBody).WithContext(pinnedDiscoveryContext()) + w := httptest.NewRecorder() + srv.handleSearchTools(w, req) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var payload struct { + Data struct { + Results []struct { + Tool struct { + Name string `json:"name"` + ServerName string `json:"server_name"` + } `json:"tool"` + } `json:"results"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &payload)) + require.Len(t, payload.Data.Results, 1) + require.Equal(t, "list_issues", payload.Data.Results[0].Tool.Name) + require.Equal(t, "github", payload.Data.Results[0].Tool.ServerName) +} diff --git a/internal/httpapi/profile_tools.go b/internal/httpapi/profile_tools.go new file mode 100644 index 000000000..8e215b5f1 --- /dev/null +++ b/internal/httpapi/profile_tools.go @@ -0,0 +1,35 @@ +package httpapi + +import "context" + +type profileToolVisibilityController interface { + SearchToolsForProfile(context.Context, string, int, func(string) bool) ([]map[string]interface{}, bool, error) + ToolAllowedByProfile(context.Context, string, string) bool +} + +func filterProfileToolRows(controller ServerController, ctx context.Context, serverName string, rows []map[string]interface{}) []map[string]interface{} { + profileController, ok := controller.(profileToolVisibilityController) + if !ok { + return rows + } + filtered := make([]map[string]interface{}, 0, len(rows)) + for _, row := range rows { + name, _ := row["name"].(string) + rowServer := serverName + if rowServer == "" { + rowServer, _ = row["server_name"].(string) + } + if name == "" || rowServer == "" { + continue + } + toolName := name + prefix := rowServer + ":" + if len(toolName) > len(prefix) && toolName[:len(prefix)] == prefix { + toolName = toolName[len(prefix):] + } + if profileController.ToolAllowedByProfile(ctx, rowServer, toolName) { + filtered = append(filtered, row) + } + } + return filtered +} diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index c01883391..77605a252 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -32,6 +32,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/oauth" "github.com/smart-mcp-proxy/mcpproxy-go/internal/observability" "github.com/smart-mcp-proxy/mcpproxy-go/internal/preflight" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/registries" "github.com/smart-mcp-proxy/mcpproxy-go/internal/reqcontext" internalRuntime "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime" @@ -3774,6 +3775,7 @@ func (s *Server) handleGetServerTools(w http.ResponseWriter, r *http.Request) { s.writeError(w, r, http.StatusInternalServerError, fmt.Sprintf("Failed to get tools: %v", err)) return } + tools = filterProfileToolRows(s.controller, r.Context(), serverID, tools) // Convert + enrich (shared with the global tools endpoint, spec 050). // Hash pins are operator-tier only (Spec 098 T020). @@ -3940,6 +3942,7 @@ func (s *Server) handleGetGlobalTools(w http.ResponseWriter, r *http.Request) { s.logger.Debugw("Global tools: server tools fetch failed", "server", name, "error", terr) continue } + generic = filterProfileToolRows(s.controller, r.Context(), name, generic) typed := s.enrichServerTools(name, generic, discloseHash) for i := range typed { @@ -4048,7 +4051,19 @@ func (s *Server) handleSearchTools(w http.ResponseWriter, r *http.Request) { var results []map[string]interface{} var err error - if auth.IsScopedCaller(r.Context()) { + profileSearchHandled := false + if profileController, ok := s.controller.(profileToolVisibilityController); ok { + inScope := func(serverName string) bool { return canSeeServer(r.Context(), serverName) } + if auth.IsScopedCaller(r.Context()) { + if ac := auth.AuthContextFromContext(r.Context()); ac != nil && len(ac.AllowedServers) == 0 { + results, profileSearchHandled = []map[string]interface{}{}, true + } + } + if !profileSearchHandled { + results, profileSearchHandled, err = profileController.SearchToolsForProfile(r.Context(), query, limit, inScope) + } + } + if !profileSearchHandled && auth.IsScopedCaller(r.Context()) { // #1166 / Spec 107 T075a: the MCP twin of this discovery surface // filters through serverInScope (internal/server/mcp_visibility.go); // this one used to post-filter a GLOBAL top-K, so a hidden server @@ -4065,7 +4080,7 @@ func (s *Server) handleSearchTools(w http.ResponseWriter, r *http.Request) { return canSeeServer(ctx, serverName) }) } - } else { + } else if !profileSearchHandled { results, err = s.controller.SearchTools(query, limit) } if err != nil { @@ -5309,6 +5324,15 @@ func (s *Server) handleReplayToolCall(w http.ResponseWriter, r *http.Request) { // concurrency slot releases that slot immediately (spec 093 FR-005). newToolCall, err := s.controller.ReplayToolCall(r.Context(), id, request.Arguments) if err != nil { + if errors.Is(err, profile.ErrToolOutsideProfile) { + s.writeError(w, r, http.StatusNotFound, "Tool call not found") + return + } + var profileRefusal *profile.ToolBlockedError + if errors.As(err, &profileRefusal) { + s.writeError(w, r, http.StatusForbidden, profileRefusal.Error()) + return + } // Spec 093 FR-011: a replay shed by a concurrency limit is backpressure, // answered like any other shed tool call — 429 + Retry-After, not a 500 // and certainly not the 200 success:true it used to produce when the @@ -5935,6 +5959,18 @@ func (s *Server) handleCallTool(w http.ResponseWriter, r *http.Request) { // Call tool via controller result, err := s.controller.CallTool(ctx, request.ToolName, request.Arguments) if err != nil { + if request.ToolName == "code_execution" && errors.Is(err, profile.ErrCodeExecutionBlocked) { + // /tools/call bypasses MCP tools/list filters. Preserve the same + // unknown-tool response as the hidden builtin's wire behavior; the + // dedicated /code/exec route still maps the typed refusal to 403. + s.writeError(w, r, http.StatusInternalServerError, "Failed to call tool: unknown tool: code_execution") + return + } + var profileRefusal *profile.ToolBlockedError + if errors.As(err, &profileRefusal) { + s.writeError(w, r, http.StatusForbidden, profileRefusal.Error()) + return + } // Spec 093 FR-011: a concurrency-limiter shed is backpressure, not a // server fault — answer 429 with a Retry-After derived from the shedding // scope's effective queue_timeout so a client can back off correctly. @@ -6859,6 +6895,11 @@ func (s *Server) handleGetToolDiff(w http.ResponseWriter, r *http.Request) { s.writeError(w, r, http.StatusBadRequest, "Server ID and tool name required") return } + if profileController, ok := s.controller.(profileToolVisibilityController); ok && + !profileController.ToolAllowedByProfile(r.Context(), serverID, toolName) { + s.writeError(w, r, http.StatusNotFound, "Tool approval record not found") + return + } record, err := s.controller.GetToolApproval(serverID, toolName) if err != nil { @@ -6910,6 +6951,15 @@ func (s *Server) handleExportToolDescriptions(w http.ResponseWriter, r *http.Req s.writeError(w, r, http.StatusInternalServerError, fmt.Sprintf("Failed to list tool approvals: %v", err)) return } + if profileController, ok := s.controller.(profileToolVisibilityController); ok { + visible := records[:0] + for _, record := range records { + if profileController.ToolAllowedByProfile(r.Context(), record.ServerName, record.ToolName) { + visible = append(visible, record) + } + } + records = visible + } format := r.URL.Query().Get("format") if format == "" { diff --git a/internal/httpapi/server_global_tools_test.go b/internal/httpapi/server_global_tools_test.go index 5fb4827ea..de829e771 100644 --- a/internal/httpapi/server_global_tools_test.go +++ b/internal/httpapi/server_global_tools_test.go @@ -1,6 +1,7 @@ package httpapi import ( + "context" "encoding/json" "errors" "net/http" @@ -19,13 +20,16 @@ import ( // globalToolsController drives the GET /api/v1/tools handler (spec 050). type globalToolsController struct { MockServerController - allServers []map[string]interface{} - serverTools map[string][]map[string]interface{} - serverErr map[string]error - approvals map[string]*storage.ToolApprovalRecord // key serverName + "\x00" + toolName - configDenied map[string]bool - usage map[string]storage.ToolUsageStat - usageErr error + allServers []map[string]interface{} + serverTools map[string][]map[string]interface{} + serverErr map[string]error + approvals map[string]*storage.ToolApprovalRecord // key serverName + "\x00" + toolName + configDenied map[string]bool + usage map[string]storage.ToolUsageStat + usageErr error + profileSearchResults []map[string]interface{} + profileSearchHandled bool + profileAllowed map[string]bool } // GetManagementService returns nil so handleGetGlobalTools exercises the @@ -62,6 +66,15 @@ func (m *globalToolsController) AggregateToolUsage(_ time.Time) (map[string]stor return m.usage, nil } +func (m *globalToolsController) SearchToolsForProfile(_ context.Context, _ string, _ int, _ func(string) bool) ([]map[string]interface{}, bool, error) { + return m.profileSearchResults, m.profileSearchHandled, nil +} + +func (m *globalToolsController) ToolAllowedByProfile(_ context.Context, serverName, toolName string) bool { + allowed, exists := m.profileAllowed[serverName+"\x00"+toolName] + return !exists || allowed +} + func doGlobalTools(t *testing.T, ctrl *globalToolsController) map[string]interface{} { t.Helper() srv := NewServer(ctrl, zaptest.NewLogger(t).Sugar(), nil) @@ -135,6 +148,23 @@ func TestGlobalTools_MergeEnrichmentStatsUsage(t *testing.T) { assert.NotEqual(t, true, data["partial"]) } +func TestGlobalTools_ProfileFiltersDisallowedRows(t *testing.T) { + ctrl := &globalToolsController{ + allServers: []map[string]interface{}{{"name": "github"}}, + serverTools: map[string][]map[string]interface{}{ + "github": {{"name": "list_issues", "description": "List issues"}, {"name": "create_issue", "description": "Create issue"}}, + }, + profileAllowed: map[string]bool{"github\x00create_issue": false}, + } + data := doGlobalTools(t, ctrl) + tools, ok := data["tools"].([]interface{}) + require.True(t, ok) + require.Len(t, tools, 1) + tool := tools[0].(map[string]interface{}) + require.Equal(t, "list_issues", tool["name"]) + require.Equal(t, "github", tool["server_name"]) +} + func TestGlobalTools_PartialServerFailureDoesNotFail(t *testing.T) { ctrl := &globalToolsController{ allServers: []map[string]interface{}{ diff --git a/internal/jsruntime/runtime.go b/internal/jsruntime/runtime.go index 11f9b4a8e..543936436 100644 --- a/internal/jsruntime/runtime.go +++ b/internal/jsruntime/runtime.go @@ -615,6 +615,12 @@ func (ec *ExecutionContext) resolveDispatchGates(serverName, toolName string, ar fmt.Sprintf("permission denied: tool '%s:%s' cannot be resolved against the current tool list of server '%s' (undiscovered or stale name), so no permission tier applies to it", serverName, toolName, serverName)), "", nil } + if refusal, ok := gate.(interface{ ProfilePolicyRefusal() string }); ok { + if message := refusal.ProfilePolicyRefusal(); message != "" { + ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, requiredPerm, args) + return errorEnvelope(ErrorCodeAccessDenied, message), "", nil + } + } } // No AuthInfo (stdio / in-process administrator): no permission tier to diff --git a/internal/profile/refusal_errors.go b/internal/profile/refusal_errors.go new file mode 100644 index 000000000..be3fee831 --- /dev/null +++ b/internal/profile/refusal_errors.go @@ -0,0 +1,22 @@ +package profile + +import "errors" + +// ToolBlockedError identifies an upstream tool call refused by a compiled +// profile policy. Its message is the stable MCP refusal rendered from the +// decision, while Reason carries the typed audit/HTTP classification. +type ToolBlockedError struct { + Reason BlockReason + Message string +} + +func (e *ToolBlockedError) Error() string { return e.Message } + +// ErrCodeExecutionBlocked identifies a code_execution refusal enforced by +// the active profile. REST uses it to return the profile-specific 403 while +// MCP and the generic tool-call route retain the unknown-tool response shape. +var ErrCodeExecutionBlocked = errors.New("blocked by profile: code execution is disabled for this profile") + +// ErrToolOutsideProfile is intentionally non-descriptive so REST replay can +// answer the same not-found shape for a hidden server and an unknown record. +var ErrToolOutsideProfile = errors.New("tool call not found") diff --git a/internal/runtime/activity_service.go b/internal/runtime/activity_service.go index b874dea82..575973fb6 100644 --- a/internal/runtime/activity_service.go +++ b/internal/runtime/activity_service.go @@ -736,17 +736,21 @@ func (s *ActivityService) handlePolicyDecision(evt Event) { decision := getStringPayload(evt.Payload, "decision") reason := getStringPayload(evt.Payload, "reason") + metadata := map[string]interface{}{ + "decision": decision, + "reason": reason, + } + if blockReason := getStringPayload(evt.Payload, storage.MetadataKeyBlockReason); blockReason != "" { + metadata[storage.MetadataKeyBlockReason] = blockReason + } record := &storage.ActivityRecord{ Type: storage.ActivityTypePolicyDecision, ServerName: serverName, ToolName: toolName, Status: decision, - Metadata: s.withClientInfo(map[string]interface{}{ - "decision": decision, - "reason": reason, - }, sessionID), - Timestamp: evt.Timestamp, - SessionID: sessionID, + Metadata: s.withClientInfo(metadata, sessionID), + Timestamp: evt.Timestamp, + SessionID: sessionID, // Copied straight from the event so the persisted record and the SSE // event a client already saw share one identity (spec 090). Absent on // pre-090 payloads, which stays absent rather than becoming "". diff --git a/internal/runtime/activity_service_test.go b/internal/runtime/activity_service_test.go index e793c8e41..c7e2e3d52 100644 --- a/internal/runtime/activity_service_test.go +++ b/internal/runtime/activity_service_test.go @@ -1090,6 +1090,39 @@ func TestEmitActivityPolicyDecision_RequestIDReachesSSEAndRecord(t *testing.T) { "the SSE event and the persisted record must share one identity") } +func TestEmitActivityPolicyDecisionWithBlockReasonPersistsTypedProfileReason(t *testing.T) { + logger, err := zap.NewDevelopment() + require.NoError(t, err) + defer logger.Sync() + + rt := &Runtime{logger: logger, eventSubs: make(map[chan Event]struct{})} + eventChan := rt.SubscribeEvents() + defer rt.UnsubscribeEvents(eventChan) + rt.EmitActivityPolicyDecisionWithBlockReason( + "github", "create_issue", "session-profile", "req-profile-block", + "blocked", "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", + "profile_tier", + ) + + var evt Event + select { + case evt = <-eventChan: + case <-time.After(2 * time.Second): + t.Fatal("did not receive activity.policy_decision event within timeout") + } + assert.Equal(t, "profile_tier", evt.Payload["block_reason"]) + + store, cleanup := setupTestStorage(t) + defer cleanup() + svc := NewActivityService(store, zap.NewNop()) + svc.handleEvent(evt) + + records, _, err := store.ListActivities(storage.DefaultActivityFilter()) + require.NoError(t, err) + require.Len(t, records, 1) + assert.Equal(t, "profile_tier", records[0].Metadata[storage.MetadataKeyBlockReason]) +} + // Records written before this change have no request_id, and FR-015 says such // rows are never correlated rather than being correlated by an empty key — so // the subscriber must leave the field empty rather than inventing an id. diff --git a/internal/runtime/event_bus.go b/internal/runtime/event_bus.go index 3d1ba5c47..368863de5 100644 --- a/internal/runtime/event_bus.go +++ b/internal/runtime/event_bus.go @@ -592,6 +592,16 @@ func (r *Runtime) EmitActivityToolCallRejected(serverName, toolName, source, req // gained it in spec 090, so records written before then have none and must not // be correlated at all (FR-015) rather than correlated by an empty key. func (r *Runtime) EmitActivityPolicyDecision(serverName, toolName, sessionID, requestID, decision, reason string) { + r.emitActivityPolicyDecision(serverName, toolName, sessionID, requestID, decision, reason, "") +} + +// EmitActivityPolicyDecisionWithBlockReason records the typed profile reason +// alongside the operator-facing policy decision metadata. +func (r *Runtime) EmitActivityPolicyDecisionWithBlockReason(serverName, toolName, sessionID, requestID, decision, reason, blockReason string) { + r.emitActivityPolicyDecision(serverName, toolName, sessionID, requestID, decision, reason, blockReason) +} + +func (r *Runtime) emitActivityPolicyDecision(serverName, toolName, sessionID, requestID, decision, reason, blockReason string) { // Spec 042: classify policy blocks as a tool quarantine error category. // "blocked" decisions are user-visible reliability events worth counting. if decision == "blocked" || decision == "block" { @@ -606,6 +616,9 @@ func (r *Runtime) EmitActivityPolicyDecision(serverName, toolName, sessionID, re "decision": decision, "reason": reason, } + if blockReason != "" { + payload["block_reason"] = blockReason + } r.publishEvent(newEvent(EventTypeActivityPolicyDecision, payload)) } diff --git a/internal/server/call_tool_profile_v3_test.go b/internal/server/call_tool_profile_v3_test.go new file mode 100644 index 000000000..c2791a441 --- /dev/null +++ b/internal/server/call_tool_profile_v3_test.go @@ -0,0 +1,134 @@ +package server + +import ( + "context" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" + "github.com/stretchr/testify/require" +) + +func TestCallTool_ProfileV3_RefusalsAreStableAcrossResolutionSources(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + up := startCountingUpstream(t, proxy, rt, "github", + writeSpec("create_issue"), + readSpec("get_secret_scanning_alert"), + toolSpec{Name: "search_code", Description: "Search code"}, + ) + + proxy.currentConfig().AnonymousProfile = "work-readonly" + + sources := map[string]func() context.Context{ + "url": func() context.Context { return urlProfileCtx(proxy, "work-readonly") }, + "session": func() context.Context { + return sessionProfileCtx(t, proxy, "session-profile-v3", "work-readonly") + }, + "pin": func() context.Context { return pinnedProfileCtx("work-readonly") }, + "binding": func() context.Context { return clientCtx("desktop", "work-readonly", "switchable") }, + "anonymous": anonCtx, + } + cases := []struct { + name, tool, refusal string + }{ + { + name: "tier cap", tool: "github:create_issue", + refusal: "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", + }, + { + name: "deny rule", tool: "github:get_secret_scanning_alert", + refusal: "blocked by profile: github:get_secret_scanning_alert is denied by a profile rule", + }, + { + name: "unannotated deny", tool: "github:search_code", + refusal: "blocked by profile: github:search_code has no tier annotation; an operator can classify it in the profile to allow it", + }, + } + variants := []string{contracts.ToolVariantRead, contracts.ToolVariantWrite, contracts.ToolVariantDestructive} + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + for _, variant := range variants { + t.Run(variant, func(t *testing.T) { + var first string + for source, contextFor := range sources { + t.Run(source, func(t *testing.T) { + result, err := proxy.handleCallToolVariant(contextFor(), auditCallToolRequest(tc.tool, nil), variant) + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError) + text := resultText(t, result) + require.Equal(t, tc.refusal, text) + require.NotContains(t, text, "work-readonly") + require.NotContains(t, text, "Work · Read-only") + if first == "" { + first = text + } else { + require.Equal(t, first, text, "refusal bytes must not reveal the resolution source") + } + }) + } + }) + } + }) + } + require.Empty(t, up.dispatched(), "profile denials must happen before upstream I/O") +} + +func TestCallTool_ProfileV3_RestDispatchPreservesTypedRefusal(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + up := startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) + + _, err := proxy.CallToolDirect(urlProfileCtx(proxy, "work-readonly"), mcp.CallToolRequest{ + Params: mcp.CallToolParams{ + Name: contracts.ToolVariantWrite, + Arguments: map[string]interface{}{"name": "github:create_issue", "args_json": "{}"}, + }, + }) + + var refusal *profile.ToolBlockedError + require.ErrorAs(t, err, &refusal) + require.Equal(t, profile.BlockReasonTier, refusal.Reason) + require.Equal(t, "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", refusal.Error()) + require.Empty(t, up.dispatched(), "REST dispatch must refuse before upstream I/O") +} + +func TestCallTool_ProfileRefusalPrecedesTokenPermissionRefusal(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) + ctx := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeAgent, AgentName: "read-only-agent", AllowedServers: []string{"*"}, + Permissions: []string{auth.PermRead}, ProfilePin: "work-readonly", + }) + result, err := proxy.handleCallToolVariant(ctx, auditCallToolRequest("github:create_issue", nil), contracts.ToolVariantWrite) + require.NoError(t, err) + require.True(t, result.IsError) + require.Equal(t, "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", resultText(t, result)) +} + +func TestCallTool_ProfileDenialWritesBlockedActivityReason(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) + go rt.ActivityService().Start(rt.AppContext(), rt) + result, err := proxy.handleCallToolVariant(urlProfileCtx(proxy, "work-readonly"), auditCallToolRequest("github:create_issue", nil), contracts.ToolVariantWrite) + require.NoError(t, err) + require.True(t, result.IsError) + + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + records, _, listErr := rt.StorageManager().ListActivities(storage.ActivityFilter{Limit: 50}) + require.NoError(t, listErr) + for _, record := range records { + if record.Type == storage.ActivityTypePolicyDecision && record.ServerName == "github" && record.ToolName == "create_issue" { + require.Equal(t, "blocked", record.Status) + require.Equal(t, string(profile.BlockReasonTier), record.Metadata[storage.MetadataKeyBlockReason]) + return + } + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("profile denial did not persist a blocked activity record with block_reason") +} diff --git a/internal/server/code_execution_profile_v3_test.go b/internal/server/code_execution_profile_v3_test.go new file mode 100644 index 000000000..8f5074d6f --- /dev/null +++ b/internal/server/code_execution_profile_v3_test.go @@ -0,0 +1,329 @@ +package server + +import ( + "context" + "encoding/json" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestCodeExecution_ProfileV3HiddenAndRefused(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + proxy.currentConfig().EnableCodeExecution = true + urlCtx := urlProfileCtx(proxy, "work-readonly") + + visible := proxy.filterProfileV3Tools(urlCtx, []mcp.Tool{ + {Name: "code_execution"}, + {Name: "call_tool_read"}, + }) + require.Equal(t, []string{"call_tool_read"}, profileV3ToolNames(visible)) + codeList, err := json.Marshal(proxy.codeExecServer.HandleMessage(urlCtx, + []byte(`{"jsonrpc":"2.0","id":1,"method":"tools/list"}`))) + require.NoError(t, err) + var codeListEnvelope struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(codeList, &codeListEnvelope), string(codeList)) + for _, tool := range codeListEnvelope.Result.Tools { + assert.NotEqual(t, "code_execution", tool.Name, "the /mcp/code instance must apply profile visibility") + } + + result, err := proxy.handleCodeExecution(urlCtx, mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}, + }) + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError) + require.Equal(t, "unknown tool: code_execution", resultText(t, result)) + + go rt.ActivityService().Start(rt.AppContext(), rt) + time.Sleep(20 * time.Millisecond) // wait until the event subscriber is installed + result, err = proxy.handleCodeExecution(urlCtx, mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}, + }) + require.NoError(t, err) + require.True(t, result.IsError) + deadline := time.Now().Add(5 * time.Second) + for time.Now().Before(deadline) { + records, _, listErr := rt.StorageManager().ListActivities(storage.ActivityFilter{Limit: 50}) + require.NoError(t, listErr) + for _, record := range records { + if record.Type == storage.ActivityTypePolicyDecision && record.ToolName == "code_execution" { + require.Equal(t, "blocked", record.Status) + require.Equal(t, string(profile.BlockReasonCodeExecution), record.Metadata[storage.MetadataKeyBlockReason]) + return + } + } + time.Sleep(10 * time.Millisecond) + } + t.Fatal("profile code_execution denial did not persist block_reason") +} + +func TestCodeExecution_DanglingProfileHiddenFromDiscovery(t *testing.T) { + proxy, _ := newProfilesV3Fixture(t) + proxy.currentConfig().EnableCodeExecution = true + ctx := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeAgent, AgentName: "stale", AllowedServers: []string{"*"}, + Permissions: []string{auth.PermRead}, ProfilePin: "removed-profile", + }) + + visible := proxy.filterProfileV3Tools(ctx, []mcp.Tool{{Name: "code_execution"}, {Name: "call_tool_read"}}) + require.Equal(t, []string{"call_tool_read"}, profileV3ToolNames(visible), "a dangling profile is deny-all and must not advertise code execution") +} + +func TestCodeExecution_ProfileV3ImplicitDefaults(t *testing.T) { + profiles := []config.ProfileConfig{ + {Name: "read-cap", Servers: []string{"github"}, MaxTier: config.ProfileTierRead}, + {Name: "write-cap", Servers: []string{"github"}, MaxTier: config.ProfileTierWrite}, + {Name: "destructive-cap", Servers: []string{"github"}, MaxTier: config.ProfileTierDestructive}, + } + proxy, _ := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}} + cfg.Profiles = profiles + cfg.EnableCodeExecution = true + }) + codeTool := mcp.Tool{Name: "code_execution"} + for _, slug := range []string{"read-cap", "write-cap"} { + t.Run(slug+" defaults to hidden", func(t *testing.T) { + ctx := urlProfileCtx(proxy, slug) + assert.Empty(t, proxy.filterProfileV3Tools(ctx, []mcp.Tool{codeTool})) + result, err := proxy.handleCodeExecution(ctx, mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}}) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Equal(t, "unknown tool: code_execution", resultText(t, result)) + }) + } + t.Run("destructive cap follows the global gate", func(t *testing.T) { + ctx := urlProfileCtx(proxy, "destructive-cap") + assert.Equal(t, []string{"code_execution"}, profileV3ToolNames(proxy.filterProfileV3Tools(ctx, []mcp.Tool{codeTool}))) + result, err := proxy.handleCodeExecution(ctx, mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}}) + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError) + + proxy.currentConfig().EnableCodeExecution = false + result, err = proxy.handleCodeExecution(ctx, mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}}) + require.NoError(t, err) + require.True(t, result.IsError) + assert.Equal(t, config.CodeExecutionDisabledMessage, resultText(t, result)) + }) +} + +func TestCallToolRoutingMode_ProfileV3FilterAndEnforcement(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + indexEnforcementMatrixFixtureTools(t, proxy) + up := startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue"), readSpec("list_issues")) + ctx := clientCtx("desktop", "work-readonly", "locked") + + listPayload, err := json.Marshal(proxy.callToolServer.HandleMessage(ctx, []byte(`{"jsonrpc":"2.0","id":1,"method":"tools/list"}`))) + require.NoError(t, err) + var listEnvelope struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(listPayload, &listEnvelope), string(listPayload)) + listNames := make([]string, 0, len(listEnvelope.Result.Tools)) + for _, tool := range listEnvelope.Result.Tools { + listNames = append(listNames, tool.Name) + } + assert.Contains(t, listNames, "retrieve_tools") + assert.NotContains(t, listNames, "code_execution", "the /mcp/call instance must apply the profile tool filter") + assert.NotContains(t, listNames, "upstream_servers", "management_tools=false must hide management tools on /mcp/call") + + callPayload, err := json.Marshal(proxy.callToolServer.HandleMessage(ctx, []byte(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"retrieve_tools","arguments":{"query":"create_issue","limit":5}}}`))) + require.NoError(t, err) + var callEnvelope struct { + Result struct { + Content []struct { + Text string `json:"text"` + } `json:"content"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(callPayload, &callEnvelope), string(callPayload)) + require.NotEmpty(t, callEnvelope.Result.Content) + var retrieved struct { + Tools []map[string]interface{} `json:"tools"` + HiddenByProfile *int `json:"hidden_by_profile"` + } + require.NoError(t, json.Unmarshal([]byte(callEnvelope.Result.Content[0].Text), &retrieved)) + assert.Empty(t, retrieved.Tools) + require.NotNil(t, retrieved.HiddenByProfile) + assert.Equal(t, 1, *retrieved.HiddenByProfile) + + writePayload, err := json.Marshal(proxy.callToolServer.HandleMessage(ctx, []byte(`{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"call_tool_write","arguments":{"name":"github:create_issue","args_json":"{}"}}}`))) + require.NoError(t, err) + var writeEnvelope struct { + Result struct { + IsError bool `json:"isError"` + Content []struct { + Text string `json:"text"` + } `json:"content"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(writePayload, &writeEnvelope), string(writePayload)) + require.True(t, writeEnvelope.Result.IsError) + require.NotEmpty(t, writeEnvelope.Result.Content) + assert.Equal(t, "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", writeEnvelope.Result.Content[0].Text) + assert.Empty(t, up.dispatched(), "the /mcp/call profile gate must refuse before upstream I/O") +} + +func TestManagementTools_ProfileV3HiddenAndRefused(t *testing.T) { + proxy, _ := newProfilesV3Fixture(t) + ctx := urlProfileCtx(proxy, "work-readonly") + tools := []mcp.Tool{ + {Name: "upstream_servers"}, + {Name: "quarantine_security"}, + {Name: "call_tool_read"}, + } + require.Equal(t, []string{"call_tool_read"}, profileV3ToolNames(proxy.filterProfileV3Tools(ctx, tools))) + + for _, name := range []string{"upstream_servers", "quarantine_security"} { + result, err := proxy.callManagementToolForProfileTest(ctx, name) + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError) + require.Equal(t, "unknown tool: "+name, resultText(t, result)) + } +} + +func TestManagementTools_ConfinedAnonymousGetsClientOperationSet(t *testing.T) { + proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}, {Name: "filesystem", Enabled: true}, {Name: "outside", Enabled: true}} + cfg.Profiles = enforcementMatrixProfiles() + cfg.Profiles[1].ManagementTools = boolPtr(true) + cfg.AnonymousProfile = "work-full" + }) + startCountingUpstream(t, proxy, rt, "github", readSpec("list_issues")) + startCountingUpstream(t, proxy, rt, "filesystem", readSpec("read_text_file")) + startCountingUpstream(t, proxy, rt, "outside", readSpec("list_files")) + tools := []mcp.Tool{{Name: "upstream_servers"}, {Name: "quarantine_security"}, {Name: "call_tool_read"}} + require.Equal(t, []string{"upstream_servers", "call_tool_read"}, profileV3ToolNames(proxy.filterProfileV3Tools(anonCtx(), tools))) + require.False(t, proxy.profileManagementToolHidden(anonCtx(), "upstream_servers")) + require.True(t, proxy.profileManagementToolHidden(anonCtx(), "quarantine_security")) + listed, err := proxy.handleListUpstreams(anonCtx()) + require.NoError(t, err) + listText := resultText(t, listed) + require.Contains(t, listText, "github") + require.Contains(t, listText, "filesystem") + require.NotContains(t, listText, "outside", "a confined anonymous list must obey the anonymous profile's server scope") + + assertManagementWritesDenied(t, proxy, anonCtx()) + quarantine, err := proxy.handleQuarantineSecurity(anonCtx(), mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{}}, + }) + require.NoError(t, err) + require.True(t, quarantine.IsError) + require.Equal(t, "unknown tool: quarantine_security", resultText(t, quarantine)) + + // The same confined profile must keep the client credential operation set: + // it may list/tail but cannot mutate configuration or restart an upstream. + client := withProfileRequestIndex(clientCtx("desktop", "work-full", "locked"), proxy.profileIndexFor(proxy.currentConfig())) + require.Equal(t, []string{"upstream_servers", "call_tool_read"}, profileV3ToolNames(proxy.filterProfileV3Tools(client, tools))) + listedForClient, err := proxy.handleListUpstreams(client) + require.NoError(t, err) + require.NotEmpty(t, resultText(t, listedForClient)) + assertManagementWritesDenied(t, proxy, client) + + // A profile-index publication gap must fail closed for scoped callers and + // never panic or fall back to an administrator-shaped view. + indexGap := withProfileRequestIndex(client, nil) + require.True(t, proxy.profileManagementToolHidden(indexGap, "upstream_servers")) + require.Equal(t, []string{"call_tool_read"}, profileV3ToolNames(proxy.filterProfileV3Tools(indexGap, tools))) + gapList, err := proxy.handleListUpstreams(indexGap) + require.NoError(t, err) + require.NotContains(t, resultText(t, gapList), "github") + + // An agent pinned to a management-enabled profile keeps the established + // read operation while its mutation refusals remain enforced. + agent := withProfileRequestIndex(pinnedProfileCtx("work-full"), proxy.profileIndexFor(proxy.currentConfig())) + listedForAgent, err := proxy.handleListUpstreams(agent) + require.NoError(t, err) + require.NotEmpty(t, resultText(t, listedForAgent)) + assertManagementWritesDenied(t, proxy, agent) + + // A legacy profile with no management_tools value is still confined for + // anonymous callers and therefore exposes no management tool. + legacy, _ := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{{Name: "github", Enabled: false}} + cfg.Profiles = []config.ProfileConfig{{Name: "legacy", Servers: []string{"github"}}} + cfg.AnonymousProfile = "legacy" + }) + require.Equal(t, []string{"call_tool_read"}, profileV3ToolNames(legacy.filterProfileV3Tools(anonCtx(), tools))) + _ = rt +} + +func assertManagementWritesDenied(t *testing.T, proxy *MCPProxyServer, ctx context.Context) { + t.Helper() + before, err := proxy.storage.ListUpstreamServers() + require.NoError(t, err) + beforeNames := make([]string, len(before)) + for i, server := range before { + beforeNames[i] = server.Name + } + for _, op := range []string{"add", "add_from_registry", "remove", "update", "patch", "restart", "enable", "disable", "refresh"} { + result, err := proxy.handleUpstreamServers(ctx, mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{"operation": op, "name": "github"}}, + }) + require.NoError(t, err, op) + require.True(t, result.IsError, op) + require.Contains(t, resultText(t, result), "Agent tokens cannot perform", op) + } + after, err := proxy.storage.ListUpstreamServers() + require.NoError(t, err) + afterNames := make([]string, len(after)) + for i, server := range after { + afterNames[i] = server.Name + } + require.Equal(t, beforeNames, afterNames, "denied management operations must not change the configured upstreams") +} + +func (p *MCPProxyServer) callManagementToolForProfileTest(ctx context.Context, name string) (*mcp.CallToolResult, error) { + request := mcp.CallToolRequest{Params: mcp.CallToolParams{Arguments: map[string]interface{}{}}} + if name == "upstream_servers" { + return p.handleUpstreamServers(ctx, request) + } + return p.handleQuarantineSecurity(ctx, request) +} + +func TestCodeExecution_ProfileV3NestedCallBlockedBeforeUpstream(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + updated := *rt.Config() + updated.Profiles = append([]config.ProfileConfig(nil), updated.Profiles...) + updated.Profiles[0].CodeExecution = boolPtr(true) + rt.UpdateConfig(&updated, "") + up := startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) + + result, err := proxy.handleCodeExecution(urlProfileCtx(proxy, "work-readonly"), mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{ + "code": `call_tool("github", "create_issue", {})`, + }}, + }) + require.NoError(t, err) + require.NotNil(t, result) + require.False(t, result.IsError, "a denied nested call is returned as an error envelope to the script; result=%s", resultText(t, result)) + require.Contains(t, resultText(t, result), "blocked by profile: github:create_issue is a write tool; this profile allows read tools only") + require.Empty(t, up.dispatched(), "nested profile denial must happen before upstream I/O") +} + +func profileV3ToolNames(tools []mcp.Tool) []string { + names := make([]string, len(tools)) + for i, tool := range tools { + names[i] = tool.Name + } + return names +} diff --git a/internal/server/describe_tool_profile_v3_test.go b/internal/server/describe_tool_profile_v3_test.go index 848afa416..06713f2ca 100644 --- a/internal/server/describe_tool_profile_v3_test.go +++ b/internal/server/describe_tool_profile_v3_test.go @@ -76,4 +76,12 @@ func TestDescribeTool_ProfileV3_ExcludedEqualsNonexistent(t *testing.T) { } } }) + + t.Run("anonymous source: excluded tool is indistinguishable from nonexistent", func(t *testing.T) { + proxy.currentConfig().AnonymousProfile = "work-readonly" + resp := callDescribe(t, proxy, anonCtx(), []interface{}{"github:create_issue"}) + require.Len(t, resp.Errors, 1) + resp.Errors[0]["id"] = "SUBSTITUTED" + assert.Equal(t, nonexistent.Errors[0], resp.Errors[0]) + }) } diff --git a/internal/server/direct_profile_v3_test.go b/internal/server/direct_profile_v3_test.go index 4a86776fd..9e46dedbb 100644 --- a/internal/server/direct_profile_v3_test.go +++ b/internal/server/direct_profile_v3_test.go @@ -1,10 +1,14 @@ package server import ( + "context" + "encoding/json" "testing" "github.com/mark3labs/mcp-go/mcp" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) // Spec 108 (Profiles v3) T019: `/mcp/all` `tools/list` omits excluded tools @@ -17,6 +21,73 @@ func directStampedTool(server, rawName, tier string) mcp.Tool { return stampDirectTool(mcp.Tool{Name: server + "__" + rawName}, entry) } +func TestDirectProtocol_ProfileV3CallRefusalOnWire(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + indexEnforcementMatrixFixtureTools(t, proxy) + up := startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) + proxy.RefreshDirectModeTools() + ctx := auth.WithAuthContext(context.Background(), &auth.AuthContext{ + Type: auth.AuthTypeAgent, AgentName: "profile-locked", ProfilePin: "work-readonly", + AllowedServers: []string{"*"}, Permissions: []string{auth.PermRead, auth.PermWrite, auth.PermDestructive}, + }) + // Mirror mcpAuthMiddleware: mcp-go's before-call hook needs this box to + // distinguish tools/call re-evaluation from list-time filtering. + ctx = withDirectRequestKindBox(ctx) + + listPayload, err := json.Marshal(proxy.directServer.HandleMessage(ctx, []byte(`{"jsonrpc":"2.0","id":1,"method":"tools/list"}`))) + require.NoError(t, err) + var listed struct { + Result struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(listPayload, &listed), string(listPayload)) + for _, tool := range listed.Result.Tools { + assert.NotEqual(t, "github__create_issue", tool.Name, "profile-excluded direct tools are absent from /mcp/all tools/list") + } + + callPayload, err := json.Marshal(proxy.directServer.HandleMessage(ctx, + []byte(`{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"github__create_issue","arguments":{}}}`))) + require.NoError(t, err) + var call struct { + Result struct { + IsError bool `json:"isError"` + Content []struct { + Text string `json:"text"` + } `json:"content"` + } `json:"result"` + } + require.NoError(t, json.Unmarshal(callPayload, &call), string(callPayload)) + require.True(t, call.Result.IsError, string(callPayload)) + require.NotEmpty(t, call.Result.Content) + assert.Equal(t, "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", call.Result.Content[0].Text) + assert.Empty(t, up.dispatched(), "a direct-mode refusal must precede upstream I/O") +} + +func TestDirectCall_ProfileV3PolicyRefusesBeforeUpstream(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + up := startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) + entry := &directCatalogEntry{ + DisplayName: FormatDirectToolName("github", "create_issue"), + ServerName: "github", ToolName: "create_issue", + Annotations: writeSpec("create_issue").Annotations, + } + ctx := withDirectRequestKindBox(urlProfileCtx(proxy, "work-readonly")) + setDirectRequestKind(ctx, directRequestKindCall) + + result, err := proxy.makeDirectModeHandler(entry)(ctx, mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{}}, + }) + + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError) + require.Equal(t, "blocked by profile: github:create_issue is a write tool; this profile allows read tools only", resultText(t, result)) + require.Empty(t, up.dispatched(), "direct-mode profile denials must happen before upstream I/O") +} + func TestFilterDirectModeToolsForAuth_ProfileV3(t *testing.T) { proxy, _ := newProfilesV3Fixture(t) @@ -39,6 +110,12 @@ func TestFilterDirectModeToolsForAuth_ProfileV3(t *testing.T) { assert.ElementsMatch(t, []string{"github__list_issues", "github__create_issue", "notion__update_page", "filesystem__read_text_file"}, filtered) }) + t.Run("anonymous: effective anonymous profile filters the same tools without a URL profile", func(t *testing.T) { + proxy.currentConfig().AnonymousProfile = "work-readonly" + filtered := filterDirectToolNames(proxy.filterDirectModeToolsForAuth(anonCtx(), tools)) + assert.ElementsMatch(t, []string{"github__list_issues", "notion__update_page"}, filtered) + }) + t.Run("legacy: policy never excludes (only server scope does, which the fixture's legacy profile restricts to github)", func(t *testing.T) { ctx := urlProfileCtx(proxy, "legacy") filtered := filterDirectToolNames(proxy.filterDirectModeToolsForAuth(ctx, tools)) diff --git a/internal/server/mcp.go b/internal/server/mcp.go index aa3f1300b..9745099a6 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -674,6 +674,7 @@ func NewMCPProxyServer( // FR-006, cross-review round 2). With no prompts registered the filter is // never invoked, so binding it early changes nothing while prompts are off. mcpserver.WithPromptFilter(proxy.filterAggregatedPromptsForAuth)(mcpServer) + mcpserver.WithToolFilter(proxy.filterProfileV3Tools)(mcpServer) // Register prompts if enabled if config.EnablePrompts { @@ -890,6 +891,10 @@ func (p *MCPProxyServer) auditToolCallFromStatus(ctx context.Context, status str // and becomes the attempt's `tool_call outcome:blocked` line — never a // second authz (FR-043(j)). Warnings and redactions write no audit line. func (p *MCPProxyServer) emitActivityPolicyDecision(ctx context.Context, serverName, toolName, sessionID, requestID, decision, reason, reasonKey string) { + p.emitActivityPolicyDecisionWithBlockReason(ctx, serverName, toolName, sessionID, requestID, decision, reason, reasonKey, "") +} + +func (p *MCPProxyServer) emitActivityPolicyDecisionWithBlockReason(ctx context.Context, serverName, toolName, sessionID, requestID, decision, reason, reasonKey, blockReason string) { if decision == "blocked" { if isPostDispatchBlockKey(reasonKey) { p.auditToolCall(ctx, "blocked", reasonKey, "", auditDurationMs(ctx), nil, nil) @@ -898,7 +903,11 @@ func (p *MCPProxyServer) emitActivityPolicyDecision(ctx context.Context, serverN } } if p.mainServer != nil && p.mainServer.runtime != nil { - p.mainServer.runtime.EmitActivityPolicyDecision(serverName, toolName, sessionID, requestID, decision, reason) + if blockReason == "" { + p.mainServer.runtime.EmitActivityPolicyDecision(serverName, toolName, sessionID, requestID, decision, reason) + } else { + p.mainServer.runtime.EmitActivityPolicyDecisionWithBlockReason(serverName, toolName, sessionID, requestID, decision, reason, blockReason) + } } // Issue #969 (Phase 0): availability baseline. Only outright blocks count — // a warning or a redaction still delivered the call. @@ -1954,6 +1963,12 @@ func (p *MCPProxyServer) handleRetrieveToolsWithMode(ctx context.Context, reques // profile that may no longer exist. The post-filter below returns the same // empty result set from the shared index. profileName, profileScope, profileIdx, profileSource := p.resolveActiveProfileWithSource(ctx) + profileResolution := p.ResolveProfileV3(ctx, profileIdx) + if profileResolution.Scope != nil { + profileName = profileResolution.Name + profileScope = profileResolution.Scope + profileSource = profile.Source(profileResolution.Source) + } // Spec 108 FR-011: the compiled policy for the effective profile, resolved // once and reused by the admit predicate below, the response's // hidden_by_profile/profile fields and nothing else — a dangling base @@ -2656,7 +2671,14 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp. // (Spec 105 PR D review round 17) — one resolution for the whole call, // never a second, independent one that could pair a decision made // against this snapshot with an index built from a later one. - profileSlug, profileScope, profileIdx := p.resolveActiveProfileWithIndex(ctx) + _, _, profileIdx := p.resolveActiveProfileWithIndex(ctx) + profileResolution := p.ResolveProfileV3(ctx, profileIdx) + // Spec 108-d is the execution cutover from the Profiles v2 resolver to + // this single v3 result, including the authoritative empty-base case. + // Keeping a v2 selection when v3 resolves no profile would bypass + // switchable_to admission for client credentials. + profileSlug := profileResolution.Name + profileScope := profileResolution.Scope // Spec 107 T103: the audit attempt, installed BEFORE the first gate so // every refusal below — the intent gates included — writes its `authz @@ -2771,7 +2793,9 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp. // one. Administrators keep today's profile-only text unchanged — they // have no token scope to intersect with. scopeAuthCtx := auth.AuthContextFromContext(ctx) - scopedCallerForScope := scopeAuthCtx != nil && !scopeAuthCtx.IsAdmin() + confinedAnonymous := (scopeAuthCtx == nil || scopeAuthCtx.Anonymous) && profileResolution.Base != "" + scopeAuthCtx = auth.ScopedView(scopeAuthCtx, confinedAnonymous) + scopedCallerForScope := auth.IsNonAdmin(scopeAuthCtx) if scopedCallerForScope { if !p.serverInScope(scopeAuthCtx, profileScope, serverName) { errMsg := fmt.Sprintf("Server '%s' is not in scope for this agent token", serverName) @@ -2829,27 +2853,11 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp. // Spec 028: Enforce agent token scope restrictions. The server-scope gate // above (effective scope = profile ∩ token, Spec 105 FR-010 G7) already // ran before the identity gate so a scoped caller learns nothing about a - // server outside its scope from the shape of the refusal; only the - // variant-permission gate remains here. + // server outside its scope from the shape of the refusal. The variant + // permission gate runs after tool identity and profile policy below, per + // the refusal precedence contract. authCtx := scopeAuthCtx scopedCaller := scopedCallerForScope - if scopedCaller { - // Check permission scope — map tool variant to required permission - var requiredPerm string - switch toolVariant { - case contracts.ToolVariantRead: - requiredPerm = auth.PermRead - case contracts.ToolVariantWrite: - requiredPerm = auth.PermWrite - case contracts.ToolVariantDestructive: - requiredPerm = auth.PermDestructive - } - if requiredPerm != "" && !authCtx.HasPermission(requiredPerm) { - errMsg := fmt.Sprintf("Insufficient permissions: '%s' requires '%s' permission", toolVariant, requiredPerm) - p.emitActivityPolicyDecision(ctx, serverName, actualToolName, getSessionID(), requestID, "blocked", errMsg, telemetry.BlockReasonTokenPermission) - return mcp.NewToolResultError(errMsg), nil - } - } // Spec 105 FR-009 (research D4): a name the discovery snapshot of a KNOWN, // CONNECTED server with a POPULATED snapshot does not contain has no @@ -2888,6 +2896,41 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp. return mcp.NewToolResultError(errMsg), nil } + // Spec 108 FR-013: discovery and dispatch share the same compiled policy + // and effective annotation identity. This gate follows server-scope and + // identity resolution, but precedes token permissions, global gates, + // server state, and tool approval. A profile denial therefore never + // reaches the upstream and cannot reveal a profile's name to the caller. + if policy := profileResolution.Policy; policy != nil { + intrinsic := profile.IntrinsicTier(annotations, annotationsFound) + admitted, reason, tier := policy.Decide(serverName, actualToolName, intrinsic) + if !admitted && reason != profile.ReasonServerNotInProfile { + errMsg, blockReason := profileToolPolicyRefusal(reason, tier, policy.Cap, serverName, actualToolName) + recordProfileToolRefusal(ctx, &profile.ToolBlockedError{Reason: blockReason, Message: errMsg}) + p.emitActivityPolicyDecisionWithBlockReason(ctx, serverName, actualToolName, getSessionID(), requestID, "blocked", errMsg, telemetry.BlockReasonOther, string(blockReason)) + return mcp.NewToolResultError(errMsg), nil + } + } + if scopedCaller { + // Check the caller-selected variant permission after the profile gate, + // so a profile denial has the same precedence for every credential + // whose token permissions also disallow the attempted operation. + var requiredPerm string + switch toolVariant { + case contracts.ToolVariantRead: + requiredPerm = auth.PermRead + case contracts.ToolVariantWrite: + requiredPerm = auth.PermWrite + case contracts.ToolVariantDestructive: + requiredPerm = auth.PermDestructive + } + if requiredPerm != "" && !authCtx.HasPermission(requiredPerm) { + errMsg := fmt.Sprintf("Insufficient permissions: '%s' requires '%s' permission", toolVariant, requiredPerm) + p.emitActivityPolicyDecision(ctx, serverName, actualToolName, getSessionID(), requestID, "blocked", errMsg, telemetry.BlockReasonTokenPermission) + return mcp.NewToolResultError(errMsg), nil + } + } + if scopedCaller { // Spec 104 FR-016f / Spec 105 FR-009: the variant is the CALLER's // choice, so it is not the tier that matters. Authorize against the @@ -3998,6 +4041,9 @@ func (p *MCPProxyServer) handleAddServerFromRegistry(ctx context.Context, reques func (p *MCPProxyServer) handleUpstreamServers(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { p.recordMCPSurface() p.recordBuiltinTool("upstream_servers") + if p.profileManagementToolHidden(ctx, "upstream_servers") { + return mcp.NewToolResultError("unknown tool: upstream_servers"), nil + } startTime := time.Now() // Extract session info for activity logging (Spec 024) @@ -4061,7 +4107,10 @@ func (p *MCPProxyServer) handleUpstreamServers(ctx context.Context, request mcp. // all write operations. The denied set is the shared agent-operation policy // (internal/auth) consumed by both this MCP surface and the REST // /api/v1/servers handlers, so the two can never drift (issues #877/#878). - if authCtx := auth.AuthContextFromContext(ctx); !auth.AuthorizeServerOp(authCtx, operation) { + profileResolution := p.ResolveProfileV3(ctx, p.profileIndexCurrent(ctx)) + requestAuth := auth.AuthContextFromContext(ctx) + authCtx := auth.ScopedView(requestAuth, profileResolution.Base != "" && (requestAuth == nil || requestAuth.Anonymous)) + if !auth.AuthorizeServerOp(authCtx, operation) { errMsg := fmt.Sprintf("Agent tokens cannot perform '%s' operations on upstream servers", operation) p.emitActivityInternalToolCall("upstream_servers", targetServer, "", "", sessionID, requestID, "error", errMsg, time.Since(startTime).Milliseconds(), args, nil, nil, "") return mcp.NewToolResultError(errMsg), nil @@ -4149,6 +4198,9 @@ func (p *MCPProxyServer) handleUpstreamServers(ctx context.Context, request mcp. func (p *MCPProxyServer) handleQuarantineSecurity(ctx context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { p.recordMCPSurface() p.recordBuiltinTool("quarantine_security") + if p.profileManagementToolHidden(ctx, "quarantine_security") { + return mcp.NewToolResultError("unknown tool: quarantine_security"), nil + } startTime := time.Now() // Extract session info for activity logging (Spec 024) @@ -4174,7 +4226,7 @@ func (p *MCPProxyServer) handleQuarantineSecurity(ctx context.Context, request m args := activityArgsFromRequest(request) // Spec 028: Agent tokens cannot perform quarantine operations - if authCtx := auth.AuthContextFromContext(ctx); authCtx != nil && !authCtx.IsAdmin() { + if auth.IsNonAdmin(auth.AuthContextFromContext(ctx)) { errMsg := "Agent tokens cannot perform quarantine security operations" p.emitActivityInternalToolCall("quarantine_security", targetServer, "", "", sessionID, requestID, "error", errMsg, time.Since(startTime).Milliseconds(), args, nil, nil, "") return mcp.NewToolResultError(errMsg), nil @@ -4438,8 +4490,11 @@ func (p *MCPProxyServer) handleListUpstreams(ctx context.Context) (*mcp.CallTool } // Spec 028: Filter servers to only those the agent token can access - authCtx := auth.AuthContextFromContext(ctx) - scopedCaller := authCtx != nil && !authCtx.IsAdmin() + profileResolution := p.ResolveProfileV3(ctx, p.profileIndexCurrent(ctx)) + requestAuth := auth.AuthContextFromContext(ctx) + confinedAnonymous := (requestAuth == nil || requestAuth.Anonymous) && profileResolution.Base != "" + authCtx := auth.ScopedView(requestAuth, confinedAnonymous) + scopedCaller := auth.IsNonAdmin(authCtx) if scopedCaller { var filtered []*config.ServerConfig for _, s := range servers { @@ -4453,7 +4508,7 @@ func (p *MCPProxyServer) handleListUpstreams(ctx context.Context) (*mcp.CallTool // Spec 057 (FR-004) / Profiles v2: Filter servers to only those visible in the // active profile (token pin > URL > session set_profile). Independent of // agent-scope so unauthenticated /mcp/p/ connections are filtered. - if _, profileScope := p.resolveActiveProfile(ctx); profileScope != nil { + if profileScope := profileResolution.Scope; profileScope != nil { var filtered []*config.ServerConfig for _, s := range servers { if profileScope.Allows(s.Name) { @@ -6413,8 +6468,11 @@ func (p *MCPProxyServer) handleTailLog(ctx context.Context, request mcp.CallTool // profile-scoped connections; unscoped administrators are unaffected. The // refusal is rendered by the same function as the nonexistent-server case // so the response discloses neither existence, status nor logs. - authCtx := auth.AuthContextFromContext(ctx) - _, profileScope := p.resolveActiveProfile(ctx) + profileResolution := p.ResolveProfileV3(ctx, p.profileIndexCurrent(ctx)) + requestAuth := auth.AuthContextFromContext(ctx) + confinedAnonymous := (requestAuth == nil || requestAuth.Anonymous) && profileResolution.Base != "" + authCtx := auth.ScopedView(requestAuth, confinedAnonymous) + profileScope := profileResolution.Scope if !p.serverInScope(authCtx, profileScope, name) { return tailLogNotFound(name), nil } @@ -6467,7 +6525,7 @@ func (p *MCPProxyServer) handleTailLog(ctx context.Context, request mcp.CallTool // keep the whole file exactly as before (SC-005) — a profile scope bounds // WHICH server they may name (above), not which records of it they see. var logLines []string - if authCtx == nil || authCtx.IsAdmin() { + if auth.IsAdminOrAbsent(authCtx) { logLines, err = logs.ReadUpstreamServerLogTail(logConfig, name, lines) } else { logLines, err = logs.ReadUpstreamServerLogTailAttributed(logConfig, name, lines) @@ -6507,7 +6565,7 @@ func (p *MCPProxyServer) handleTailLog(ctx context.Context, request mcp.CallTool // same predicate the attributed reader applies to the log record // — uniformly, whether or not a co-owner exists; administrators // keep the text (SC-005). - if authCtx != nil && !authCtx.IsAdmin() { + if auth.IsNonAdmin(authCtx) { lastError = logs.RedactContainerMentions(lastError) } connectionStatus["last_error"] = lastError @@ -6850,6 +6908,7 @@ func (p *MCPProxyServer) CallToolDirect(ctx context.Context, request mcp.CallToo // only answer with an isError result. Capture the typed refusal so the HTTP // layer classifies it without re-parsing the message. ctx, codeExecRefusal := withCodeExecCapture(ctx) + ctx, profileToolRefusal := withProfileToolCapture(ctx) // Route to the appropriate handler based on tool name var result *mcp.CallToolResult @@ -6905,6 +6964,9 @@ func (p *MCPProxyServer) CallToolDirect(ctx context.Context, request mcp.CallToo } if len(result.Content) > 0 { if textContent, ok := result.Content[0].(mcp.TextContent); ok { + if refusal := profileToolRefusal.take(); refusal != nil { + return nil, refusal + } // A code_execution refusal keeps its typed identity so the HTTP // layer can answer 403/404/400 (Spec 097). The message stays the // agent-readable one either way. diff --git a/internal/server/mcp_block_tools_test.go b/internal/server/mcp_block_tools_test.go index 6cc5ed8ae..012776d4e 100644 --- a/internal/server/mcp_block_tools_test.go +++ b/internal/server/mcp_block_tools_test.go @@ -67,6 +67,12 @@ func createTestProxyWithRuntimeCfg(t *testing.T, servers []*config.ServerConfig, tr := truncate.NewTruncator(0) mainSrv := &Server{runtime: rt} + // Production warms the profile index for the initial runtime snapshot + // during server construction. Scoped request tests rely on the same + // publication guarantee: a nil cache result intentionally fails closed, + // which would otherwise make every scoped fixture appear to have no + // code_execution tool even when no profile applies. + mainSrv.profileIndexes.warmCurrent(rt.Config()) proxy := NewMCPProxyServer(sm, idx, um, cm, func() *truncate.Truncator { return tr }, logger, mainSrv, false, cfg, rt.SignatureCache()) return proxy, rt } diff --git a/internal/server/mcp_code_execution.go b/internal/server/mcp_code_execution.go index 5dc38c92a..db234d727 100644 --- a/internal/server/mcp_code_execution.go +++ b/internal/server/mcp_code_execution.go @@ -17,6 +17,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts" "github.com/smart-mcp-proxy/mcpproxy-go/internal/jsruntime" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/reqcontext" "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" "github.com/smart-mcp-proxy/mcpproxy-go/internal/telemetry" @@ -90,6 +91,19 @@ func (p *MCPProxyServer) handleCodeExecution(ctx context.Context, request mcp.Ca p.recordMCPSurface() p.recordBuiltinTool("code_execution") p.logger.Debug("code_execution tool called") + profileIdx := p.profileIndexCurrent(ctx) + if profileIdx == nil { + return mcp.NewToolResultError("unknown tool: code_execution"), nil + } + profileResolution := p.ResolveProfileV3(ctx, profileIdx) + if profileResolution.Policy != nil && !profileResolution.Policy.CodeExecution { + requestID := mintActivityRequestID("", "code_execution") + refusal := profile.ErrCodeExecutionBlocked + recordCodeExecRefusal(ctx, refusal) + p.emitActivityPolicyDecisionWithBlockReason(ctx, "", "code_execution", sessionIDFromContext(ctx), requestID, + "blocked", refusal.Error(), telemetry.BlockReasonOther, string(profile.BlockReasonCodeExecution)) + return mcp.NewToolResultError("unknown tool: code_execution"), nil + } // enable_code_execution is a FEATURE switch, so it is enforced where every // surface passes rather than at registration. The MCP surfaces gate by @@ -267,10 +281,27 @@ func (p *MCPProxyServer) handleCodeExecution(ctx context.Context, request mcp.Ca // form is wired so the lookup's read is the nested call's ONE persisted // read: the bridge (CallToolWithGate) dispatches on the gate it captured // rather than taking a second one (codex r9 I1). - options.ToolGateFunc = p.lookupToolGate + options.ToolGateFunc = func(serverName, toolName string) (string, jsruntime.ToolGate) { + required, rawGate := p.lookupToolGate(serverName, toolName) + sandbox, ok := rawGate.(*sandboxGate) + if !ok || sandbox == nil || profileResolution.Policy == nil { + return required, rawGate + } + identity := sandbox.gate.identity + if !sandbox.gated { + identity = p.resolveExactToolIdentityWith(nil, serverName, toolName) + } + admitted, reason, tier := profileResolution.Policy.Decide( + serverName, toolName, profile.IntrinsicTier(identity.Annotations, identity.Found), + ) + if !admitted && reason != profile.ReasonServerNotInProfile { + sandbox.profileRefusal, _ = profileToolPolicyRefusal(reason, tier, profileResolution.Policy.Cap, serverName, toolName) + } + return required, rawGate + } // Spec 057 (Codex #621 finding 2): Intersect profile scope into code_execution. - p.applyProfileScopeToExecution(ctx, &options) + p.applyResolvedProfileScopeToExecution(&options, profileResolution.Scope) // Spec 107 T103/T104: the wrapper itself writes no audit line (it is a // built-in), but it captures the SCRIPT's caller for every nested line @@ -785,8 +816,16 @@ type upstreamToolCaller struct { // dispatchGate's second result carried along: false means no gate was // evaluated (a proxy without storage). type sandboxGate struct { - gate toolGate - gated bool + gate toolGate + gated bool + profileRefusal string +} + +func (g *sandboxGate) ProfilePolicyRefusal() string { + if g == nil { + return "" + } + return g.profileRefusal } // CallTool implements jsruntime.ToolCaller. It takes the gate read itself, @@ -1501,7 +1540,18 @@ func (p *MCPProxyServer) applyProfileScopeToExecution(ctx context.Context, optio if options == nil { return } - _, profileScope := p.resolveActiveProfile(ctx) + _, profileScope, idx := p.resolveActiveProfileWithIndex(ctx) + resolution := p.ResolveProfileV3(ctx, idx) + if resolution.Scope != nil { + profileScope = resolution.Scope + } + p.applyResolvedProfileScopeToExecution(options, profileScope) +} + +func (p *MCPProxyServer) applyResolvedProfileScopeToExecution(options *jsruntime.ExecutionOptions, profileScope *profile.ProfileScope) { + if options == nil { + return + } if profileScope == nil { return } diff --git a/internal/server/mcp_direct_scope.go b/internal/server/mcp_direct_scope.go index b76237e70..a47381a1c 100644 --- a/internal/server/mcp_direct_scope.go +++ b/internal/server/mcp_direct_scope.go @@ -363,6 +363,11 @@ func (p *MCPProxyServer) filterDirectModeToolsForAuth(ctx context.Context, tools authCtx := auth.AuthContextFromContext(ctx) profileName, profileScope, profileIdx := p.resolveActiveProfileWithIndex(ctx) + profileResolution := p.ResolveProfileV3(ctx, profileIdx) + if profileResolution.Scope != nil { + profileName = profileResolution.Name + profileScope = profileResolution.Scope + } isScopedAgent := isScopeRestrictedCaller(authCtx) var policy *profile.CompiledPolicy if profileName != "" { diff --git a/internal/server/mcp_routing.go b/internal/server/mcp_routing.go index 4f99e1e2f..fa2b11ad5 100644 --- a/internal/server/mcp_routing.go +++ b/internal/server/mcp_routing.go @@ -20,6 +20,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/branding" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/reqcontext" "github.com/smart-mcp-proxy/mcpproxy-go/internal/security/scanner" "github.com/smart-mcp-proxy/mcpproxy-go/internal/telemetry" @@ -493,7 +494,9 @@ func (p *MCPProxyServer) makeDirectModeHandler(entry *directCatalogEntry) mcpser // Spec 107 T103: the audit attempt, installed BEFORE the first gate. // The operation is the tier this catalog entry's annotations derive // (the same tier the permission gate below authorizes against). - profileSlug, profileScope := p.resolveActiveProfile(ctx) + profileIndex := p.profileIndexCurrent(ctx) + profileResolution := p.ResolveProfileV3(ctx, profileIndex) + profileSlug, profileScope := profileResolution.Name, profileResolution.Scope { var auditClientName, auditClientVersion string if sessionID != "" { @@ -526,6 +529,15 @@ func (p *MCPProxyServer) makeDirectModeHandler(entry *directCatalogEntry) mcpser p.emitActivityPolicyDecision(ctx, serverName, toolName, sessionID, requestID, "blocked", refusalErr.Error(), telemetry.BlockReasonProfileScope) return nil, refusalErr } + if policy := profileResolution.Policy; policy != nil { + intrinsic := profile.IntrinsicTier(annotations, annotations != nil) + if admitted, reason, tier := policy.Decide(serverName, toolName, intrinsic); !admitted && reason != profile.ReasonServerNotInProfile { + errMsg, blockReason := profileToolPolicyRefusal(reason, tier, policy.Cap, serverName, toolName) + p.emitActivityPolicyDecisionWithBlockReason(ctx, serverName, toolName, sessionID, requestID, + "blocked", errMsg, telemetry.BlockReasonOther, string(blockReason)) + return mcp.NewToolResultError(errMsg), nil + } + } // Check auth context for server access and permissions authCtx := auth.AuthContextFromContext(ctx) @@ -997,6 +1009,81 @@ func (p *MCPProxyServer) buildCodeExecutionTool() []mcpserver.ServerTool { }} } +// filterProfileV3Tools applies profile-controlled built-in visibility to +// every request-scoped tools/list response. Handler gates remain mandatory: +// tool filters are discovery controls, not an execution boundary. +func (p *MCPProxyServer) filterProfileV3Tools(ctx context.Context, tools []mcp.Tool) []mcp.Tool { + idx, ok := profileRequestIndexFromContext(ctx) + if !ok { + idx = p.profileIndexCurrent(ctx) + } + if idx == nil { + resolution := p.ResolveProfileV3(ctx, nil) + if resolution.Scope == nil && resolution.Policy == nil { + return tools + } + filtered := make([]mcp.Tool, 0, len(tools)) + for _, tool := range tools { + if tool.Name == "code_execution" || tool.Name == "upstream_servers" || tool.Name == "quarantine_security" { + continue + } + filtered = append(filtered, tool) + } + return filtered + } + resolution := p.ResolveProfileV3(ctx, idx) + // A dangling pinned/bound/anonymous profile is authoritative deny-all. + // Do not advertise code execution just because the resolution no longer + // has a compiled policy to consult. + danglingProfile := resolution.Name != "" && idx.position(resolution.Name) < 0 + filtered := make([]mcp.Tool, 0, len(tools)) + for _, tool := range tools { + switch tool.Name { + case "code_execution": + if danglingProfile || (resolution.Policy != nil && !resolution.Policy.CodeExecution) { + continue + } + case "upstream_servers", "quarantine_security": + if p.profileManagementToolHidden(ctx, tool.Name) { + continue + } + } + filtered = append(filtered, tool) + } + return filtered +} + +// profileManagementToolHidden keeps the execution boundary aligned with the +// request-scoped tool filter. Legacy profiles (unset management_tools) retain +// their existing behavior; an explicit false hides and refuses the tools. +func (p *MCPProxyServer) profileManagementToolHidden(ctx context.Context, toolName string) bool { + idx, ok := profileRequestIndexFromContext(ctx) + if !ok { + idx = p.profileIndexCurrent(ctx) + } + ac := auth.AuthContextFromContext(ctx) + if idx == nil { + resolution := p.ResolveProfileV3(ctx, nil) + return resolution.Scope != nil || resolution.Policy != nil + } + resolution := p.ResolveProfileV3(ctx, idx) + confinedAnonymous := (ac == nil || ac.Anonymous) && resolution.Base != "" + managementEnabled := resolution.Policy != nil && resolution.Policy.ManagementTools != nil && *resolution.Policy.ManagementTools + if resolution.Policy != nil && resolution.Policy.ManagementTools != nil && !*resolution.Policy.ManagementTools { + return true + } + if ac.IsClientCredential() || confinedAnonymous { + if !managementEnabled { + return true + } + return toolName == "quarantine_security" + } + if toolName == "quarantine_security" && managementEnabled && (ac == nil || !ac.IsAdmin() || ac.Anonymous) { + return true + } + return false +} + // initRoutingModeServers creates separate MCP server instances for each routing mode. // Each server instance has its own set of tools registered appropriate for that mode. // The main "server" field remains the retrieve_tools mode server (default). @@ -1112,6 +1199,7 @@ func (p *MCPProxyServer) initRoutingModeServers() { // scoped and stamp-free (Spec 105 FR-006, cross-review round 2). It is a // no-op while no prompts are registered. opts = append(opts, mcpserver.WithPromptFilter(p.filterAggregatedPromptsForAuth)) + opts = append(opts, mcpserver.WithToolFilter(p.filterProfileV3Tools)) // Create direct mode server. Both direct-mode tool filters are agent-scoped // discovery filters and belong only on the direct server (not the shared diff --git a/internal/server/mcp_tail_log_scope_test.go b/internal/server/mcp_tail_log_scope_test.go index 477351ca4..d31f72353 100644 --- a/internal/server/mcp_tail_log_scope_test.go +++ b/internal/server/mcp_tail_log_scope_test.go @@ -273,6 +273,10 @@ func newTailLogCollidingProxy(t *testing.T) *tailLogCollidingFixture { // arm (only `a_b` configured, no `a/b` anywhere: not in storage, not in the // upstream manager, no writer) against the colliding one. func newTailLogProxyWithServers(t *testing.T, names ...string) *tailLogCollidingFixture { + return newTailLogProxyWithServersAndAnonymous(t, "", names...) +} + +func newTailLogProxyWithServersAndAnonymous(t *testing.T, anonymousProfile string, names ...string) *tailLogCollidingFixture { t.Helper() require.Equal(t, logs.ServerLogFilename(collidingHidden), logs.ServerLogFilename(collidingOwn), "fixture premise: the two raw names must share one log file") @@ -290,6 +294,10 @@ func newTailLogProxyWithServers(t *testing.T, names ...string) *tailLogColliding for _, name := range names { cfg.Servers = append(cfg.Servers, &config.ServerConfig{Name: name, Protocol: "http", Enabled: false}) } + if anonymousProfile != "" { + cfg.Profiles = []config.ProfileConfig{{Name: "anonymous-scope", Servers: []string{anonymousProfile}, ManagementTools: boolPtr(true)}} + cfg.AnonymousProfile = "anonymous-scope" + } mainSrv, err := NewServer(cfg, zap.NewNop()) require.NoError(t, err) t.Cleanup(func() { _ = mainSrv.Shutdown() }) @@ -578,6 +586,34 @@ func TestTailLog_DockerCollisionConnectError_ForeignContainerWithheldFromScopedC } } +func TestTailLog_DockerCollisionConnectError_ForeignContainerWithheldFromConfinedAnonymous(t *testing.T) { + const foreignID = "f0e1d2c3b4a5968778695a4b3c2d1e0ff0e1d2c3b4a5968778695a4b3c2d1e0f" + const foreignName = "mcpproxy-a-b-wxyz" + collision := `docker: Error response from daemon: Conflict. The container name "/` + foreignName + + `" is already in use by container "` + foreignID + `". You have to remove (or rename) that container to be able to reuse that name.` + f := newTailLogProxyWithServersAndAnonymous(t, collidingHidden, collidingHidden, "a-b") + connectErr := fmt.Errorf("stdio transport (command=%q, docker_isolation=%t): %w", "docker", true, + fmt.Errorf("server process exited before completing the MCP initialize handshake; recent stderr:\n | %s: EOF", collision)) + client, ok := f.proxy.upstreamManager.GetClient(collidingHidden) + require.True(t, ok) + client.StateManager.SetError(connectErr) + f.writers[collidingHidden].Error("Connection failed", zap.String("transport", "stdio"), zap.Error(connectErr), logs.ChildOutputField()) + _ = f.writers[collidingHidden].Sync() + + resp, body := tailLogLinesVia(t, f.proxy, context.Background(), collidingHidden, 50) + assert.NotContains(t, body, foreignID) + assert.NotContains(t, body, foreignName) + assert.NotContains(t, body, "already in use by container") + assert.Contains(t, body, `"last_error"`) + assert.Equal(t, 0, resp.LinesReturned, "the child-output collision record must be withheld") + + listBody := listUpstreamsBodyVia(t, f.proxy, context.Background()) + assert.Contains(t, listBody, collidingHidden) + assert.NotContains(t, listBody, "a-b") + assert.NotContains(t, listBody, foreignID) + assert.NotContains(t, listBody, foreignName) +} + // listUpstreamsBodyVia drives `upstream_servers` `list` through the real // dispatcher and returns the response text. func listUpstreamsBodyVia(t *testing.T, proxy *MCPProxyServer, ctx context.Context) string { diff --git a/internal/server/mcp_visibility.go b/internal/server/mcp_visibility.go index a3fbcb97f..2a37b1975 100644 --- a/internal/server/mcp_visibility.go +++ b/internal/server/mcp_visibility.go @@ -86,6 +86,11 @@ const ( func (p *MCPProxyServer) toolVisibleToSession(ctx context.Context, serverName, toolName string) (visible bool, reason string) { authCtx := auth.AuthContextFromContext(ctx) profileName, profileScope, profileIdx := p.resolveActiveProfileWithIndex(ctx) + profileResolution := p.ResolveProfileV3(ctx, profileIdx) + if profileResolution.Scope != nil { + profileName = profileResolution.Name + profileScope = profileResolution.Scope + } // Spec 105 FR-010 G2: for a SCOPED caller (agent token), scope is // checked BEFORE index presence. An id whose server is outside the diff --git a/internal/server/profile_index_policy_test.go b/internal/server/profile_index_policy_test.go index ba02fe1ee..4ace28ca9 100644 --- a/internal/server/profile_index_policy_test.go +++ b/internal/server/profile_index_policy_test.go @@ -13,8 +13,6 @@ import ( // pattern, data-model.md §2): profileIndex compiles and caches one // CompiledPolicy per profile, taken with its snapshot as one immutable pair. func TestProfileIndex_CompiledPolicy(t *testing.T) { - config.EnablePolicyForTest(t) - cfg := &config.Config{ Servers: []*config.ServerConfig{{Name: "github"}, {Name: "notion"}, {Name: "filesystem"}}, Profiles: enforcementMatrixProfiles(), diff --git a/internal/server/profile_refusals.go b/internal/server/profile_refusals.go new file mode 100644 index 000000000..9ff83013d --- /dev/null +++ b/internal/server/profile_refusals.go @@ -0,0 +1,26 @@ +package server + +import ( + "fmt" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" +) + +// profileToolPolicyRefusal renders the stable, non-disclosing refusal text +// for a tool excluded by a compiled profile policy. +func profileToolPolicyRefusal(reason profile.Reason, tier, cap profile.Tier, server, tool string) (string, profile.BlockReason) { + switch reason { + case profile.ReasonAboveTierCap: + capText := "read and write" + if cap == profile.TierRead { + capText = "read" + } + return fmt.Sprintf("blocked by profile: %s:%s is a %s tool; this profile allows %s tools only", server, tool, tier, capText), profile.BlockReasonTier + case profile.ReasonDeniedByRule: + return fmt.Sprintf("blocked by profile: %s:%s is denied by a profile rule", server, tool), profile.BlockReasonRule + case profile.ReasonUnannotatedHidden: + return fmt.Sprintf("blocked by profile: %s:%s has no tier annotation; an operator can classify it in the profile to allow it", server, tool), profile.BlockReasonUnannotated + default: + return "blocked by profile: tool is not allowed by the active profile", profile.BlockReasonRule + } +} diff --git a/internal/server/profile_resolver_v3.go b/internal/server/profile_resolver_v3.go index 247ec1705..d81da1dd7 100644 --- a/internal/server/profile_resolver_v3.go +++ b/internal/server/profile_resolver_v3.go @@ -97,13 +97,41 @@ func resolveV3Base(ctx context.Context, idx *profileIndex) (name string, source // ResolveProfileV3 is the FR-020 resolution: highest wins — pin > url > // session > binding > anonymous > none — against idx's (index, snapshot) -// pair. It is additive to, and does not replace, resolveActiveProfileFromIndex -// (the live Profiles v2 enforcement path every existing consumer still -// uses): wiring THIS resolution into execution (retrieve_tools, set_profile -// admission, call_tool_*) is 108-d's FR-009a-gated enforcement cutover. -// Exposed now so 108-d's tests can build directly on a resolver whose -// precedence and switchable_to admission are already pinned (T029/T036). +// pair. Profile v3 discovery and execution use this single result so the +// effective server scope, policy, and source cannot come from separate +// snapshots. Legacy surfaces not yet migrated can still use +// resolveActiveProfileFromIndex. func (p *MCPProxyServer) ResolveProfileV3(ctx context.Context, idx *profileIndex) ProfileResolution { + if idx == nil { + // profileIndexCurrent deliberately returns nil for a scoped request + // during a publication gap instead of building an index on the request + // path. Preserve that fail-closed contract without dereferencing the + // missing snapshot or falling through to an unscoped view. + if pin, mode, ok := clientCredentialFromContext(ctx); ok { + source := profile.SourceBinding + if mode == auth.ProfileModeLocked { + source = profile.SourcePin + } + return ProfileResolution{Name: pin, Source: string(source), Scope: profile.NewProfileScope(pin, nil), Base: pin} + } + if pin := profilePinFromContext(ctx); pin != "" { + return ProfileResolution{Name: pin, Source: string(profile.SourcePin), Scope: profile.NewProfileScope(pin, nil), Base: pin} + } + if scope := profile.ProfileScopeFromContext(ctx); scope != nil { + return ProfileResolution{Name: scope.Name, Source: string(profile.SourceURL), Scope: profile.NewProfileScope(scope.Name, nil)} + } + ac := auth.AuthContextFromContext(ctx) + if ac == nil || ac.Anonymous { + if cfg := p.currentConfig(); cfg != nil && cfg.AnonymousProfile != "" { + name := cfg.AnonymousProfile + return ProfileResolution{Name: name, Source: string(profile.SourceAnonymous), Scope: profile.NewProfileScope(name, nil), Base: name} + } + } + if auth.IsScopedCaller(ctx) || (ac != nil && ac.IsClientCredential()) { + return ProfileResolution{Source: string(profile.SourceNone), Scope: profile.NewProfileScope("", nil)} + } + return ProfileResolution{Source: string(profile.SourceNone)} + } base, source, dangling := resolveV3Base(ctx, idx) // Tier 1: pin. Authoritative — never falls through, including when diff --git a/internal/server/profile_rest_discovery.go b/internal/server/profile_rest_discovery.go new file mode 100644 index 000000000..0a60c22dd --- /dev/null +++ b/internal/server/profile_rest_discovery.go @@ -0,0 +1,74 @@ +package server + +import ( + "context" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/index" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" +) + +// SearchToolsForProfile performs REST tool search with the same profile policy +// and pre-limit admission used by retrieve_tools. handled is false when no +// v3 profile resolution applies, preserving the existing search path. +func (s *Server) SearchToolsForProfile(ctx context.Context, query string, limit int, inScope func(string) bool) (results []map[string]interface{}, handled bool, err error) { + proxy := s.mcpProxy + if proxy == nil || proxy.index == nil { + return nil, false, nil + } + profileIndex := proxy.profileIndexCurrent(ctx) + resolution := proxy.ResolveProfileV3(ctx, profileIndex) + if resolution.Scope == nil && resolution.Policy == nil { + return nil, false, nil + } + + if inScope == nil { + inScope = func(string) bool { return true } + } + withheld := s.quarantinedServerFilter() + admit := func(hit index.Hit) index.Admission { + if withheld(hit.Server) || (resolution.Scope != nil && !resolution.Scope.Allows(hit.Server)) || !inScope(hit.Server) { + return index.RejectScope + } + if resolution.Policy == nil { + return index.RejectScope + } + annotations, found := proxy.EffectiveAnnotations(hit.Server, hit.Tool) + intrinsic := profile.IntrinsicTier(annotations, found) + ok, reason, _ := resolution.Policy.Decide(hit.Server, hit.Tool, intrinsic) + if !ok { + if reason == profile.ReasonServerNotInProfile { + return index.RejectScope + } + return index.RejectPolicy + } + return index.Admit + } + searchResults, _, err := proxy.index.SearchToolsAdmitted(query, limit, admit) + if err != nil { + return nil, true, err + } + return s.searchResultsToMaps(searchResults), true, nil +} + +// ToolAllowedByProfile is the row-level REST discovery check. It fails closed +// for dangling or policy-excluded profile resolutions and is a no-op when the +// request has no effective v3 profile. +func (s *Server) ToolAllowedByProfile(ctx context.Context, serverName, toolName string) bool { + proxy := s.mcpProxy + if proxy == nil { + return true + } + resolution := proxy.ResolveProfileV3(ctx, proxy.profileIndexCurrent(ctx)) + if resolution.Scope == nil && resolution.Policy == nil { + return true + } + if resolution.Scope != nil && !resolution.Scope.Allows(serverName) { + return false + } + if resolution.Policy == nil { + return false + } + annotations, found := proxy.EffectiveAnnotations(serverName, toolName) + allowed, _, _ := resolution.Policy.Decide(serverName, toolName, profile.IntrinsicTier(annotations, found)) + return allowed +} diff --git a/internal/server/profile_rest_discovery_test.go b/internal/server/profile_rest_discovery_test.go new file mode 100644 index 000000000..5632e1be7 --- /dev/null +++ b/internal/server/profile_rest_discovery_test.go @@ -0,0 +1,40 @@ +package server + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSearchToolsForProfileFiltersBeforeLimit(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + indexEnforcementMatrixFixtureTools(t, proxy) + mainServer := &Server{runtime: rt, mcpProxy: proxy} + + rows, handled, err := mainServer.SearchToolsForProfile(urlProfileCtx(proxy, "work-readonly"), "list_issues", 1, nil) + + require.NoError(t, err) + require.True(t, handled) + require.Len(t, rows, 1, "an excluded high-ranked hit must not consume the only result slot") + tool, ok := rows[0]["tool"].(map[string]interface{}) + require.True(t, ok) + require.Equal(t, "github:list_issues", tool["name"]) + require.False(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "work-readonly"), "github", "create_issue")) + require.True(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "work-readonly"), "github", "list_issues")) + require.False(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "work-readonly"), "filesystem", "read_text_file")) + require.True(t, mainServer.ToolAllowedByProfile(adminCtx(), "filesystem", "read_text_file"), "an unprofiled admin keeps the existing view") +} + +func TestProfileRESTDiscoveryPreservesLegacyProfileBehavior(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + indexEnforcementMatrixFixtureTools(t, proxy) + mainServer := &Server{runtime: rt, mcpProxy: proxy} + ctx := urlProfileCtx(proxy, "legacy") + + rows, handled, err := mainServer.SearchToolsForProfile(ctx, "list_issues", 10, nil) + require.NoError(t, err) + require.True(t, handled) + require.NotEmpty(t, rows, "legacy profiles keep their existing in-scope discovery behavior") + require.True(t, mainServer.ToolAllowedByProfile(ctx, "github", "create_issue"), "legacy profile does not acquire a tier policy") + require.False(t, mainServer.ToolAllowedByProfile(ctx, "filesystem", "read_text_file"), "legacy profile still enforces its server scope") +} diff --git a/internal/server/profile_tool.go b/internal/server/profile_tool.go index 82ac16438..d2c31470c 100644 --- a/internal/server/profile_tool.go +++ b/internal/server/profile_tool.go @@ -199,8 +199,13 @@ func setProfileResult(activeProfile string, servers []string) (*mcp.CallToolResu if servers == nil { servers = []string{} } + profileSource := string(profile.SourceNone) + if activeProfile != "" { + profileSource = string(profile.SourceSession) + } payload := map[string]interface{}{ "active_profile": activeProfile, + "profile_source": profileSource, "servers": servers, } body, err := json.Marshal(payload) diff --git a/internal/server/profile_tool_dispatch.go b/internal/server/profile_tool_dispatch.go new file mode 100644 index 000000000..7e8e0999c --- /dev/null +++ b/internal/server/profile_tool_dispatch.go @@ -0,0 +1,44 @@ +package server + +import ( + "context" + "sync" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" +) + +type profileToolCapture struct { + mu sync.Mutex + err *profile.ToolBlockedError +} + +type profileToolCaptureKeyType struct{} + +var profileToolCaptureKey profileToolCaptureKeyType + +func withProfileToolCapture(ctx context.Context) (context.Context, *profileToolCapture) { + box := &profileToolCapture{} + return context.WithValue(ctx, profileToolCaptureKey, box), box +} + +func recordProfileToolRefusal(ctx context.Context, refusal *profile.ToolBlockedError) { + if ctx == nil || refusal == nil { + return + } + box, ok := ctx.Value(profileToolCaptureKey).(*profileToolCapture) + if !ok || box == nil { + return + } + box.mu.Lock() + box.err = refusal + box.mu.Unlock() +} + +func (c *profileToolCapture) take() *profile.ToolBlockedError { + if c == nil { + return nil + } + c.mu.Lock() + defer c.mu.Unlock() + return c.err +} diff --git a/internal/server/profiles_v3_fixture_test.go b/internal/server/profiles_v3_fixture_test.go index f3c835520..71bd0d001 100644 --- a/internal/server/profiles_v3_fixture_test.go +++ b/internal/server/profiles_v3_fixture_test.go @@ -70,12 +70,9 @@ func enforcementMatrixProfiles() []config.ProfileConfig { // contracts/enforcement-matrix.md fixture profiles (work-readonly, work-full, // legacy) and three fake upstreams — github (5 tools spanning every // annotation shape), notion (1 write tool) and filesystem (1 read tool) — -// exactly as the matrix's fixture table lists them. It enables the FR-009a -// policy gate for the duration of the test (config.EnablePolicyForTest) so a -// v3 profile loads before 108-d ships real enforcement. +// exactly as the matrix's fixture table lists them. func newProfilesV3Fixture(t *testing.T) (*MCPProxyServer, *runtime.Runtime) { t.Helper() - config.EnablePolicyForTest(t) proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { cfg.Servers = []*config.ServerConfig{ diff --git a/internal/server/replay_audit_test.go b/internal/server/replay_audit_test.go index 82e58f7be..bf8a65fea 100644 --- a/internal/server/replay_audit_test.go +++ b/internal/server/replay_audit_test.go @@ -23,6 +23,7 @@ import ( "github.com/stretchr/testify/require" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" ) @@ -147,6 +148,26 @@ func TestReplayToolCall_WritesAuthzAllowThenToolCallSuccess(t *testing.T) { assert.Equal(t, lines[0]["request_id"], lines[1]["request_id"]) } +func TestReplayToolCall_ProfilePolicyRefusesBeforeUpstream(t *testing.T) { + proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}} + cfg.Profiles = []config.ProfileConfig{{ + Name: "replay-readonly", Servers: []string{"github"}, + Tools: &config.ProfileToolRules{Deny: []string{"github:create_issue"}}, + }} + }) + mainSrv := &Server{runtime: rt, mcpProxy: proxy} + url, calls := startRuntimeCountingUpstream(t, proxy, "github", "create_issue") + callID := seedReplayableCall(t, proxy, mainSrv, "github", "create_issue", url) + + _, err := mainSrv.ReplayToolCall(urlProfileCtx(proxy, "replay-readonly"), callID, nil) + + var refusal *profile.ToolBlockedError + require.ErrorAs(t, err, &refusal) + require.Equal(t, profile.BlockReasonRule, refusal.Reason) + require.Empty(t, calls.dispatched(), "profile-denied replay must stop before upstream I/O") +} + func TestReplayToolCall_UnresolvedIDDelegatesUnaudited(t *testing.T) { proxy, rt := createTestProxyWithRuntime(t, nil) sink := &recordingAuditSink{} diff --git a/internal/server/retrieve_tools_profile_v3_test.go b/internal/server/retrieve_tools_profile_v3_test.go index 590cc8f93..0c89f39ba 100644 --- a/internal/server/retrieve_tools_profile_v3_test.go +++ b/internal/server/retrieve_tools_profile_v3_test.go @@ -157,6 +157,54 @@ func TestRetrieveTools_ProfileV3_ProfileFieldSourceGating(t *testing.T) { assert.Nil(t, resp.Profile, "a pinned caller must never learn it is pinned or to what (research D27)") require.NotNil(t, resp.HiddenByProfile, "hidden_by_profile is independent of source and still applies") }) + + t.Run("switchable client binding: policy applies without disclosing the base", func(t *testing.T) { + resp := callRetrieveToolsV3(t, proxy, clientCtx("desktop", "work-readonly", "switchable"), "create_issue", 5) + assert.Empty(t, resp.Tools) + require.NotNil(t, resp.HiddenByProfile) + assert.Equal(t, 1, *resp.HiddenByProfile) + assert.Nil(t, resp.Profile) + }) + + t.Run("switchable client session selection: selected profile is reported", func(t *testing.T) { + const sessionID = "client-session-v3" + proxy.sessionStore.SetActiveProfile(sessionID, "work-full") + helper := mcpserver.NewMCPServer("test", "1.0.0") + ctx := helper.WithContext(context.Background(), &fakeClientSession{id: sessionID}) + ctx = auth.WithAuthContext(ctx, &auth.AuthContext{ + Type: auth.AuthTypeAgent, AgentName: "client-desktop", TokenPrefix: "mcp_cli_fix", + AllowedServers: []string{"*"}, Permissions: []string{auth.PermRead, auth.PermWrite, auth.PermDestructive}, + ProfilePin: "work-readonly", TokenKind: auth.KindClient, ClientID: "desktop", ProfileMode: "switchable", + }) + resp := callRetrieveToolsV3(t, proxy, ctx, "create_issue", 5) + require.Len(t, resp.Tools, 1) + require.NotNil(t, resp.Profile) + assert.Equal(t, "work-full", *resp.Profile) + }) + + t.Run("anonymous profile: policy applies without disclosing the base", func(t *testing.T) { + proxy.currentConfig().AnonymousProfile = "work-readonly" + resp := callRetrieveToolsV3(t, proxy, anonCtx(), "create_issue", 5) + assert.Empty(t, resp.Tools) + require.NotNil(t, resp.HiddenByProfile) + assert.Equal(t, 1, *resp.HiddenByProfile) + assert.Nil(t, resp.Profile) + }) + + t.Run("dangling binding and anonymous base deny all without hidden count", func(t *testing.T) { + binding := callRetrieveToolsV3(t, proxy, clientCtx("lost", "missing-profile", "switchable"), "list_issues", 5) + assert.Empty(t, binding.Tools) + require.NotNil(t, binding.HiddenByProfile) + assert.Zero(t, *binding.HiddenByProfile) + assert.Nil(t, binding.Profile) + + proxy.currentConfig().AnonymousProfile = "missing-profile" + anonymous := callRetrieveToolsV3(t, proxy, anonCtx(), "list_issues", 5) + assert.Empty(t, anonymous.Tools) + require.NotNil(t, anonymous.HiddenByProfile) + assert.Zero(t, *anonymous.HiddenByProfile) + assert.Nil(t, anonymous.Profile) + }) } // TestRetrieveTools_ProfileV3_DanglingPinDenyAll pins FR-020: a legacy agent @@ -184,7 +232,6 @@ func TestRetrieveTools_ProfileV3_DanglingPinDenyAll(t *testing.T) { // this is the same property proven end-to-end through retrieve_tools. func TestRetrieveTools_ProfileV3_FilterBeforeLimit(t *testing.T) { proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { - config.EnablePolicyForTest(t) cfg.Servers = []*config.ServerConfig{{Name: "a", Enabled: true}} cfg.Profiles = []config.ProfileConfig{ {Name: "cap-read", Servers: []string{"a"}, MaxTier: "read"}, diff --git a/internal/server/scope_fixture_test.go b/internal/server/scope_fixture_test.go index d20de30ec..1a6e5adf3 100644 --- a/internal/server/scope_fixture_test.go +++ b/internal/server/scope_fixture_test.go @@ -106,7 +106,21 @@ type countingUpstream struct { // runtime discovery lists it; storage records are never seeded here. func (u *countingUpstream) serve(spec toolSpec) { u.Tools = append(u.Tools, spec.info()) - u.mcpSrv.AddTool(mcp.Tool{Name: spec.Name, Description: spec.Description, InputSchema: mcp.ToolInputSchema{Type: "object"}}, + tool := mcp.Tool{ + Name: spec.Name, Description: spec.Description, + InputSchema: mcp.ToolInputSchema{Type: "object"}, + // NewTool's defaults advertise destructive=true and readOnly=false. + // This fixture's nil annotations mean genuinely unannotated, while a + // non-nil toolSpec carries the exact hints the runtime StateView gets. + Annotations: mcp.ToolAnnotation{}, + } + if spec.Annotations != nil { + tool.Annotations.ReadOnlyHint = spec.Annotations.ReadOnlyHint + tool.Annotations.DestructiveHint = spec.Annotations.DestructiveHint + tool.Annotations.IdempotentHint = spec.Annotations.IdempotentHint + tool.Annotations.OpenWorldHint = spec.Annotations.OpenWorldHint + } + u.mcpSrv.AddTool(tool, func(_ context.Context, request mcp.CallToolRequest) (*mcp.CallToolResult, error) { u.record(request.Params.Name) return mcp.NewToolResultText("ok"), nil diff --git a/internal/server/scope_latency_profile_v3_test.go b/internal/server/scope_latency_profile_v3_test.go index 09648047e..8f5a84c24 100644 --- a/internal/server/scope_latency_profile_v3_test.go +++ b/internal/server/scope_latency_profile_v3_test.go @@ -57,7 +57,6 @@ func TestRetrieveTools_ScopeLatency_ProfileV3VsLegacy(t *testing.T) { require.Greater(t, len(servers), 20, "fixture: doubling the snapshot must still name more than a handful of servers") proxy := createTestMCPProxyServer(t) - config.EnablePolicyForTest(t) serverCfgs := make([]*config.ServerConfig, 0, len(servers)) for _, s := range servers { serverCfgs = append(serverCfgs, &config.ServerConfig{Name: s, Enabled: true}) diff --git a/internal/server/scope_oracle_v3_test.go b/internal/server/scope_oracle_v3_test.go index 379b3ad6d..5f7367989 100644 --- a/internal/server/scope_oracle_v3_test.go +++ b/internal/server/scope_oracle_v3_test.go @@ -1,6 +1,8 @@ package server import ( + "context" + "encoding/json" "testing" "github.com/stretchr/testify/assert" @@ -38,7 +40,6 @@ type scopeOracleV3Fixture struct { func newScopeOracleV3Fixture(t *testing.T, full bool) *scopeOracleV3Fixture { t.Helper() proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { - config.EnablePolicyForTest(t) cfg.Servers = []*config.ServerConfig{{Name: "a", Enabled: true}} if full { cfg.Servers = append(cfg.Servers, @@ -140,3 +141,40 @@ func TestScopeOracleV3_HiddenByProfileIdenticalAcrossFixtures(t *testing.T) { require.NotEmpty(t, resp.Tools, "fixture premise: an unscoped administrator must see the hidden sentinel tool") }) } + +// The same two-fixture non-disclosure oracle must hold for the newer +// resolution sources added by client credentials and anonymous_profile. +// Previously T017 only exercised an agent pin, leaving those sources +// unprotected against hidden-server influence on hidden_by_profile/results. +func TestScopeOracleV3_NewResolutionSourcesMatchAcrossFixtures(t *testing.T) { + narrow := newScopeOracleV3Fixture(t, false) + full := newScopeOracleV3Fixture(t, true) + narrow.proxy.currentConfig().AnonymousProfile = "cap-read-a" + full.proxy.currentConfig().AnonymousProfile = "cap-read-a" + + sources := map[string]struct { + narrow context.Context + full context.Context + }{ + "locked client pin": {clientCtx("desktop", "cap-read-a", "locked"), clientCtx("desktop", "cap-read-a", "locked")}, + "switchable client binding": {clientCtx("desktop", "cap-read-a", "switchable"), clientCtx("desktop", "cap-read-a", "switchable")}, + "anonymous profile": {anonCtx(), anonCtx()}, + } + for source, callers := range sources { + t.Run(source, func(t *testing.T) { + for _, query := range []string{"read_thing", "write_thing", "destroy_thing", "erase", "ns_erase"} { + t.Run(query, func(t *testing.T) { + narrowResp := callRetrieveToolsV3(t, narrow.proxy, callers.narrow, query, 10) + fullResp := callRetrieveToolsV3(t, full.proxy, callers.full, query, 10) + assert.Nil(t, narrowResp.Profile, "credential and anonymous resolution sources never reveal the base profile") + assert.Nil(t, fullResp.Profile, "credential and anonymous resolution sources never reveal the base profile") + left, err := json.Marshal(narrowResp) + require.NoError(t, err) + right, err := json.Marshal(fullResp) + require.NoError(t, err) + assert.JSONEq(t, string(left), string(right), "hidden servers must not perturb discovery for %s", source) + }) + } + }) + } +} diff --git a/internal/server/scoped_view_guard_test.go b/internal/server/scoped_view_guard_test.go new file mode 100644 index 000000000..c28ed8779 --- /dev/null +++ b/internal/server/scoped_view_guard_test.go @@ -0,0 +1,45 @@ +package server + +import ( + "go/ast" + "go/parser" + "go/token" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestConfinedAnonymousHandlersUseScopedView(t *testing.T) { + file, err := parser.ParseFile(token.NewFileSet(), "mcp.go", nil, 0) + require.NoError(t, err) + guarded := map[string]bool{ + "handleUpstreamServers": false, + "handleListUpstreams": false, + "handleTailLog": false, + "handleCallToolVariant": false, + } + for _, decl := range file.Decls { + fn, ok := decl.(*ast.FuncDecl) + if !ok || fn.Body == nil { + continue + } + if _, ok := guarded[fn.Name.Name]; !ok { + continue + } + guarded[fn.Name.Name] = true + ast.Inspect(fn.Body, func(node ast.Node) bool { + call, ok := node.(*ast.CallExpr) + if !ok { + return true + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if ok && selector.Sel.Name == "IsAdmin" { + t.Errorf("%s must use auth.ScopedView before administrator checks", fn.Name.Name) + } + return true + }) + } + for name, found := range guarded { + require.Truef(t, found, "missing guarded handler %s", name) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index a66caeb37..8dff65db4 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3945,6 +3945,28 @@ func (s *Server) ReplayToolCall(ctx context.Context, id string, arguments map[st callArgs = original.Arguments } + // Spec 108 FR-015: replay is a dispatch path, so evaluate its recorded + // server/tool against the same request-scoped profile resolution before + // audit authorization or upstream I/O. Profile server scope is concealed + // as not-found; tool policy denials retain the shared refusal text. + profileIndex := s.mcpProxy.profileIndexCurrent(ctx) + profileResolution := s.mcpProxy.ResolveProfileV3(ctx, profileIndex) + if profileResolution.Scope != nil && !profileResolution.Scope.Allows(original.ServerName) { + return nil, profile.ErrToolOutsideProfile + } + if policy := profileResolution.Policy; policy != nil { + annotations, found := s.mcpProxy.EffectiveAnnotations(original.ServerName, original.ToolName) + intrinsic := profile.IntrinsicTier(annotations, found) + if admitted, reason, tier := policy.Decide(original.ServerName, original.ToolName, intrinsic); !admitted && reason != profile.ReasonServerNotInProfile { + message, blockReason := profileToolPolicyRefusal(reason, tier, policy.Cap, original.ServerName, original.ToolName) + refusal := &profile.ToolBlockedError{Reason: blockReason, Message: message} + requestID := mintCorrelationID(original.ServerName, original.ToolName) + s.mcpProxy.emitActivityPolicyDecisionWithBlockReason(ctx, original.ServerName, original.ToolName, + sessionIDFromContext(ctx), requestID, "blocked", message, telemetry.BlockReasonOther, string(blockReason)) + return nil, refusal + } + } + // Spec 107 (round-3 cross-review finding, PR-D): the persisted record's // own annotations snapshot is the canonical target tier here — the same // signal tierForAnnotations derives from a live gate's identity lookup diff --git a/internal/server/set_profile_v3_test.go b/internal/server/set_profile_v3_test.go new file mode 100644 index 000000000..d2994274e --- /dev/null +++ b/internal/server/set_profile_v3_test.go @@ -0,0 +1,32 @@ +package server + +import ( + "encoding/json" + "testing" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" +) + +func TestSetProfileV3ReportsOnlyTheSessionSelection(t *testing.T) { + proxy, _ := newProfilesV3Fixture(t) + ctx := sessionProfileCtx(t, proxy, "session-profile-source", "work-full") + for _, tc := range []struct { + name, selection, wantSource string + }{ + {name: "selected profile", selection: "work-full", wantSource: "session"}, + {name: "cleared selection", selection: "", wantSource: "none"}, + } { + t.Run(tc.name, func(t *testing.T) { + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"profile": tc.selection} + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + require.False(t, result.IsError, resultText(t, result)) + var payload map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(resultText(t, result)), &payload)) + require.Equal(t, tc.wantSource, payload["profile_source"]) + require.Equal(t, tc.selection, payload["active_profile"]) + }) + } +} diff --git a/internal/storage/activity_models.go b/internal/storage/activity_models.go index 848463202..da9f76c43 100644 --- a/internal/storage/activity_models.go +++ b/internal/storage/activity_models.go @@ -13,6 +13,9 @@ const ActivityRecordsBucket = "activity_records" type ActivityType string const ( + // MetadataKeyBlockReason is the typed cause of a profile policy refusal. + MetadataKeyBlockReason = "block_reason" + // ActivityTypeToolCall represents a tool execution event ActivityTypeToolCall ActivityType = "tool_call" // ActivityTypePolicyDecision represents a policy blocking a tool call From 7d9bf4064b6169a581644d1005b534c0ad3e3f48 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 00:19:55 +0300 Subject: [PATCH 02/11] fix(profile): close enforcement audit gaps --- internal/httpapi/concurrency_shed_test.go | 2 +- .../httpapi/profile_rest_discovery_test.go | 26 ++++++- internal/httpapi/server.go | 6 +- internal/jsruntime/runtime.go | 30 +++++---- internal/runtime/activity_service.go | 11 ++- internal/server/audit_funnel.go | 18 ++++- internal/server/call_tool_profile_v3_test.go | 5 ++ .../server/code_execution_profile_v3_test.go | 40 ++++++++++- internal/server/direct_profile_v3_test.go | 28 ++++++++ internal/server/mcp_code_execution.go | 28 ++++---- .../server/mcp_code_execution_scope_test.go | 7 +- internal/server/mcp_routing.go | 7 +- internal/server/profiles_v3_fixture_test.go | 7 ++ internal/server/replay_audit_test.go | 67 +++++++++++++++++++ internal/server/scope_oracle_v3_test.go | 23 ++++++- internal/server/server.go | 43 ++++++------ 16 files changed, 283 insertions(+), 65 deletions(-) diff --git a/internal/httpapi/concurrency_shed_test.go b/internal/httpapi/concurrency_shed_test.go index 0975d4cee..349236e50 100644 --- a/internal/httpapi/concurrency_shed_test.go +++ b/internal/httpapi/concurrency_shed_test.go @@ -59,7 +59,7 @@ func postToolCallNamed(t *testing.T, srv *Server, apiKey, toolName string) *http func TestHandleCallTool_ProfileHiddenCodeExecutionMatchesUnknownTool(t *testing.T) { apiKey := "test-profile-code-exec-api-key" profileCtrl := &shedController{apiKey: apiKey, err: fmt.Errorf("tool call failed: %w", profile.ErrCodeExecutionBlocked)} - unknownCtrl := &shedController{apiKey: apiKey, err: fmt.Errorf("unknown tool: no_such_tool")} + unknownCtrl := &shedController{apiKey: apiKey, err: fmt.Errorf("tool call failed: unknown tool: no_such_tool")} profileResponse := postToolCallNamed(t, NewServer(profileCtrl, zap.NewNop().Sugar(), nil), apiKey, "code_execution") unknownResponse := postToolCallNamed(t, NewServer(unknownCtrl, zap.NewNop().Sugar(), nil), apiKey, "no_such_tool") diff --git a/internal/httpapi/profile_rest_discovery_test.go b/internal/httpapi/profile_rest_discovery_test.go index 58c5eabcd..50cde339b 100644 --- a/internal/httpapi/profile_rest_discovery_test.go +++ b/internal/httpapi/profile_rest_discovery_test.go @@ -3,6 +3,7 @@ package httpapi import ( "context" "encoding/json" + "fmt" "net/http" "net/http/httptest" "testing" @@ -34,6 +35,13 @@ func (c *profileRESTDiscoveryController) GetManagementService() management.Servi return c.managementService } +func (c *profileRESTDiscoveryController) GetToolApproval(serverName, toolName string) (*storage.ToolApprovalRecord, error) { + if record, ok := c.globalToolsController.approvals[serverName+"\x00"+toolName]; ok { + return record, nil + } + return nil, fmt.Errorf("%w: %s", storage.ErrToolApprovalNotFound, storage.ToolApprovalKey(serverName, toolName)) +} + func (c *profileRESTDiscoveryController) ListToolApprovals(server string) ([]*storage.ToolApprovalRecord, error) { rows := make([]*storage.ToolApprovalRecord, 0, len(c.approvals)) for _, row := range c.approvals { @@ -86,7 +94,7 @@ func TestRESTDiscovery_ProfileFiltersServerToolsAndExportAndHidesDiff(t *testing {"name": "create_issue", "server_name": "github", "description": "Create issue"}, }, }, - profileAllowed: map[string]bool{"github\x00create_issue": false}, + profileAllowed: map[string]bool{"github\x00create_issue": false, "github\x00no_such_tool": true}, approvals: map[string]*storage.ToolApprovalRecord{ "github\x00list_issues": {ServerName: "github", ToolName: "list_issues", Status: storage.ToolApprovalStatusApproved}, "github\x00create_issue": {ServerName: "github", ToolName: "create_issue", Status: storage.ToolApprovalStatusChanged}, @@ -143,7 +151,21 @@ func TestRESTDiscovery_ProfileFiltersServerToolsAndExportAndHidesDiff(t *testing w := httptest.NewRecorder() srv.handleGetToolDiff(w, req) require.Equal(t, http.StatusNotFound, w.Code) - require.Contains(t, w.Body.String(), "Tool approval record not found") + var hiddenPayload struct { + Error string `json:"error"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &hiddenPayload)) + + unknownReq := profileRouteRequest(ctx, http.MethodGet, "/api/v1/servers/github/tools/no_such_tool/diff", + [2]string{"id", "github"}, [2]string{"tool", "no_such_tool"}) + unknown := httptest.NewRecorder() + srv.handleGetToolDiff(unknown, unknownReq) + require.Equal(t, http.StatusNotFound, unknown.Code) + var unknownPayload struct { + Error string `json:"error"` + } + require.NoError(t, json.Unmarshal(unknown.Body.Bytes(), &unknownPayload)) + require.Equal(t, unknownPayload.Error, hiddenPayload.Error, "a policy-hidden tool and a nonexistent tool must have identical not-found bodies") }) t.Run("unprofiled administrator keeps the current rows and diff", func(t *testing.T) { diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 77605a252..7559fd500 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -5963,7 +5963,7 @@ func (s *Server) handleCallTool(w http.ResponseWriter, r *http.Request) { // /tools/call bypasses MCP tools/list filters. Preserve the same // unknown-tool response as the hidden builtin's wire behavior; the // dedicated /code/exec route still maps the typed refusal to 403. - s.writeError(w, r, http.StatusInternalServerError, "Failed to call tool: unknown tool: code_execution") + s.writeError(w, r, http.StatusInternalServerError, "Failed to call tool: tool call failed: unknown tool: code_execution") return } var profileRefusal *profile.ToolBlockedError @@ -6903,6 +6903,10 @@ func (s *Server) handleGetToolDiff(w http.ResponseWriter, r *http.Request) { record, err := s.controller.GetToolApproval(serverID, toolName) if err != nil { + if errors.Is(err, storage.ErrToolApprovalNotFound) { + s.writeError(w, r, http.StatusNotFound, "Tool approval record not found") + return + } s.writeError(w, r, http.StatusNotFound, fmt.Sprintf("Tool approval record not found: %v", err)) return } diff --git a/internal/jsruntime/runtime.go b/internal/jsruntime/runtime.go index 543936436..ccc7c3cbb 100644 --- a/internal/jsruntime/runtime.go +++ b/internal/jsruntime/runtime.go @@ -67,6 +67,7 @@ type AuthzGateReport struct { CanonicalTarget string // "server:tool" Denied bool Code ErrorCode // the envelope code of the refusal (SERVER_NOT_ALLOWED, ACCESS_DENIED, PERMISSION_DENIED) + Message string // the envelope message shown to the script caller RequiredPerm string // the tier the lookup resolved, when one was resolved Arguments map[string]interface{} } @@ -503,7 +504,7 @@ func (ec *ExecutionContext) checkDispatchGates(serverName, toolName string) (gat // T104). It is the ONLY reporting seam: resolveDispatchGates calls it on // every refusing return, once, so a refusal is never re-reported by the // completion path (which a refused call never reaches). nil observer = no-op. -func (ec *ExecutionContext) reportAuthzRefusal(serverName, toolName string, code ErrorCode, requiredPerm string, args map[string]interface{}) { +func (ec *ExecutionContext) reportAuthzRefusal(serverName, toolName string, code ErrorCode, message, requiredPerm string, args map[string]interface{}) { if ec.authzObserver == nil { return } @@ -515,6 +516,7 @@ func (ec *ExecutionContext) reportAuthzRefusal(serverName, toolName string, code CanonicalTarget: serverName + ":" + toolName, Denied: true, Code: code, + Message: message, RequiredPerm: requiredPerm, Arguments: stripAuthInjectedArgs(args), }) @@ -580,12 +582,14 @@ func (ec *ExecutionContext) resolveDispatchGates(serverName, toolName string, ar profileDenies := (ec.restrictToAllowed || len(ec.allowedServerMap) > 0) && !ec.allowedServerMap[serverName] if ec.authInfo != nil && !ec.authInfo.isAdmin() { if profileDenies || !ec.authInfo.CanAccessServer(serverName) { - ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, "", args) - return errorEnvelope(ErrorCodeAccessDenied, fmt.Sprintf("token does not have access to server '%s'", serverName)), "", nil + message := fmt.Sprintf("token does not have access to server '%s'", serverName) + ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, message, "", args) + return errorEnvelope(ErrorCodeAccessDenied, message), "", nil } } else if profileDenies { - ec.reportAuthzRefusal(serverName, toolName, ErrorCodeServerNotAllowed, "", args) - return errorEnvelope(ErrorCodeServerNotAllowed, fmt.Sprintf("server not allowed: %s", serverName)), "", nil + message := fmt.Sprintf("server not allowed: %s", serverName) + ec.reportAuthzRefusal(serverName, toolName, ErrorCodeServerNotAllowed, message, "", args) + return errorEnvelope(ErrorCodeServerNotAllowed, message), "", nil } // Determine required permission via annotation lookup. The gate-capturing @@ -610,14 +614,14 @@ func (ec *ExecutionContext) resolveDispatchGates(serverName, toolName string, ar // to the same identity rule as every HTTP caller. It answers with the // permission envelope, never with an upstream's own "tool not found". if requiredPerm == PermissionTierUnresolved { - ec.reportAuthzRefusal(serverName, toolName, ErrorCodePermissionDenied, requiredPerm, args) - return errorEnvelope(ErrorCodePermissionDenied, - fmt.Sprintf("permission denied: tool '%s:%s' cannot be resolved against the current tool list of server '%s' (undiscovered or stale name), so no permission tier applies to it", - serverName, toolName, serverName)), "", nil + message := fmt.Sprintf("permission denied: tool '%s:%s' cannot be resolved against the current tool list of server '%s' (undiscovered or stale name), so no permission tier applies to it", + serverName, toolName, serverName) + ec.reportAuthzRefusal(serverName, toolName, ErrorCodePermissionDenied, message, requiredPerm, args) + return errorEnvelope(ErrorCodePermissionDenied, message), "", nil } if refusal, ok := gate.(interface{ ProfilePolicyRefusal() string }); ok { if message := refusal.ProfilePolicyRefusal(); message != "" { - ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, requiredPerm, args) + ec.reportAuthzRefusal(serverName, toolName, ErrorCodeAccessDenied, message, requiredPerm, args) return errorEnvelope(ErrorCodeAccessDenied, message), "", nil } } @@ -631,9 +635,9 @@ func (ec *ExecutionContext) resolveDispatchGates(serverName, toolName string, ar } if !ec.authInfo.HasPermission(requiredPerm) { - ec.reportAuthzRefusal(serverName, toolName, ErrorCodePermissionDenied, requiredPerm, args) - return errorEnvelope(ErrorCodePermissionDenied, - fmt.Sprintf("token does not have '%s' permission for tool '%s:%s'", requiredPerm, serverName, toolName)), "", nil + message := fmt.Sprintf("token does not have '%s' permission for tool '%s:%s'", requiredPerm, serverName, toolName) + ec.reportAuthzRefusal(serverName, toolName, ErrorCodePermissionDenied, message, requiredPerm, args) + return errorEnvelope(ErrorCodePermissionDenied, message), "", nil } return nil, requiredPerm, gate diff --git a/internal/runtime/activity_service.go b/internal/runtime/activity_service.go index 575973fb6..909ede2ed 100644 --- a/internal/runtime/activity_service.go +++ b/internal/runtime/activity_service.go @@ -146,6 +146,14 @@ func NewActivityService(storage *storage.Manager, logger *zap.Logger) *ActivityS return s } +// Started reports whether the activity service has registered its runtime +// event subscription. It is safe to call while Start is running. +func (s *ActivityService) Started() bool { + s.startMu.Lock() + defer s.startMu.Unlock() + return s.started +} + // SetSessionClientResolver wires the session -> MCP client lookup. Safe to leave // unset (records then carry no client name). func (s *ActivityService) SetSessionClientResolver(r SessionClientResolver) { @@ -281,10 +289,9 @@ func (s *ActivityService) Start(ctx context.Context, rt *Runtime) { s.logger.Warn("Activity service Start called twice; ignoring") return } - s.started = true - // Subscribe to runtime events eventCh := rt.subscribeInternalEvents() + s.started = true // Start retention loop in a separate goroutine. Tracked in workersWG: it // prunes activity records (BBolt writes), so Stop must await it. diff --git a/internal/server/audit_funnel.go b/internal/server/audit_funnel.go index 1e3bc5708..390bacac1 100644 --- a/internal/server/audit_funnel.go +++ b/internal/server/audit_funnel.go @@ -22,6 +22,7 @@ package server import ( "context" + "errors" "sync" "time" @@ -484,6 +485,7 @@ func auditDurationMs(ctx context.Context) int64 { // funnels enforce per attempt applies per refusal. type nestedAuthzObserver struct { proxy *MCPProxyServer + toolCaller *upstreamToolCaller parentCtx context.Context caller audit.Caller sessionID string @@ -493,7 +495,21 @@ type nestedAuthzObserver struct { } func (o *nestedAuthzObserver) ObserveAuthzGate(report jsruntime.AuthzGateReport) { - if o == nil || o.proxy == nil || o.proxy.auditSink == nil || !report.Denied { + if o == nil || !report.Denied { + return + } + // resolveDispatchGates refuses before the JavaScript bridge reaches + // upstreamToolCaller.CallToolWithGate, which is where nested history is + // normally persisted. Preserve the refused attempt here so history has the + // same parent correlation as the activity and audit records. + if o.toolCaller != nil { + startedAt := time.Now() + refusal := errors.New(report.Message) + o.toolCaller.storeToolCallInHistory(report.ServerName, report.ToolName, report.Arguments, nil, refusal, startedAt, 0) + o.toolCaller.emitSubCallRefused(report.Ctx, report.ServerName, report.ToolName, + mintCorrelationID(report.ServerName, report.ToolName), report.Arguments, refusal, startedAt, 0) + } + if o.proxy == nil || o.proxy.auditSink == nil { return } reasonKey := telemetry.BlockReasonTokenScope diff --git a/internal/server/call_tool_profile_v3_test.go b/internal/server/call_tool_profile_v3_test.go index c2791a441..7b333d9b9 100644 --- a/internal/server/call_tool_profile_v3_test.go +++ b/internal/server/call_tool_profile_v3_test.go @@ -113,6 +113,11 @@ func TestCallTool_ProfileDenialWritesBlockedActivityReason(t *testing.T) { proxy, rt := newProfilesV3Fixture(t) startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue")) go rt.ActivityService().Start(rt.AppContext(), rt) + startDeadline := time.Now().Add(5 * time.Second) + for !rt.ActivityService().Started() && time.Now().Before(startDeadline) { + time.Sleep(time.Millisecond) + } + require.True(t, rt.ActivityService().Started(), "activity service must subscribe before the policy decision is emitted") result, err := proxy.handleCallToolVariant(urlProfileCtx(proxy, "work-readonly"), auditCallToolRequest("github:create_issue", nil), contracts.ToolVariantWrite) require.NoError(t, err) require.True(t, result.IsError) diff --git a/internal/server/code_execution_profile_v3_test.go b/internal/server/code_execution_profile_v3_test.go index 8f5074d6f..dbee5e1f8 100644 --- a/internal/server/code_execution_profile_v3_test.go +++ b/internal/server/code_execution_profile_v3_test.go @@ -49,7 +49,11 @@ func TestCodeExecution_ProfileV3HiddenAndRefused(t *testing.T) { require.Equal(t, "unknown tool: code_execution", resultText(t, result)) go rt.ActivityService().Start(rt.AppContext(), rt) - time.Sleep(20 * time.Millisecond) // wait until the event subscriber is installed + startDeadline := time.Now().Add(5 * time.Second) + for !rt.ActivityService().Started() && time.Now().Before(startDeadline) { + time.Sleep(time.Millisecond) + } + require.True(t, rt.ActivityService().Started(), "activity service must subscribe before the policy decision is emitted") result, err = proxy.handleCodeExecution(urlCtx, mcp.CallToolRequest{ Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}, }) @@ -81,6 +85,14 @@ func TestCodeExecution_DanglingProfileHiddenFromDiscovery(t *testing.T) { visible := proxy.filterProfileV3Tools(ctx, []mcp.Tool{{Name: "code_execution"}, {Name: "call_tool_read"}}) require.Equal(t, []string{"call_tool_read"}, profileV3ToolNames(visible), "a dangling profile is deny-all and must not advertise code execution") + + result, err := proxy.handleCodeExecution(ctx, mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{"code": "1 + 1"}}, + }) + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError, "a dangling profile must also refuse direct execution") + require.Equal(t, "unknown tool: code_execution", resultText(t, result)) } func TestCodeExecution_ProfileV3ImplicitDefaults(t *testing.T) { @@ -122,7 +134,9 @@ func TestCodeExecution_ProfileV3ImplicitDefaults(t *testing.T) { } func TestCallToolRoutingMode_ProfileV3FilterAndEnforcement(t *testing.T) { - proxy, rt := newProfilesV3Fixture(t) + proxy, rt := newProfilesV3FixtureWithConfig(t, func(cfg *config.Config) { + cfg.EnableCodeExecution = true + }) indexEnforcementMatrixFixtureTools(t, proxy) up := startCountingUpstream(t, proxy, rt, "github", writeSpec("create_issue"), readSpec("list_issues")) ctx := clientCtx("desktop", "work-readonly", "locked") @@ -318,6 +332,28 @@ func TestCodeExecution_ProfileV3NestedCallBlockedBeforeUpstream(t *testing.T) { require.False(t, result.IsError, "a denied nested call is returned as an error envelope to the script; result=%s", resultText(t, result)) require.Contains(t, resultText(t, result), "blocked by profile: github:create_issue is a write tool; this profile allows read tools only") require.Empty(t, up.dispatched(), "nested profile denial must happen before upstream I/O") + calls, total, listErr := rt.GetToolCalls(50, 0, nil) + require.NoError(t, listErr) + require.GreaterOrEqual(t, total, 1, "the parent execution record is persisted") + var parentID string + for _, call := range calls { + if call.ServerName == "mcpproxy" && call.ToolName == "code_execution" { + parentID = call.ID + } + } + require.NotEmpty(t, parentID, "parent code_execution record is persisted") + serverConfig, configErr := rt.StorageManager().GetUpstreamServer("github") + require.NoError(t, configErr) + serverCalls, callsErr := rt.StorageManager().GetServerToolCalls(storage.GenerateServerID(serverConfig), 1000) + require.NoError(t, callsErr) + var childParentID string + for _, call := range serverCalls { + if call.ServerName == "github" && call.ToolName == "create_issue" { + childParentID = call.ParentCallID + } + } + require.NotEmpty(t, childParentID, "profile-refused nested call is persisted") + require.Equal(t, parentID, childParentID, "the refused child record links to its parent execution") } func profileV3ToolNames(tools []mcp.Tool) []string { diff --git a/internal/server/direct_profile_v3_test.go b/internal/server/direct_profile_v3_test.go index 9e46dedbb..642e21999 100644 --- a/internal/server/direct_profile_v3_test.go +++ b/internal/server/direct_profile_v3_test.go @@ -7,6 +7,7 @@ import ( "github.com/mark3labs/mcp-go/mcp" "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -88,6 +89,33 @@ func TestDirectCall_ProfileV3PolicyRefusesBeforeUpstream(t *testing.T) { require.Empty(t, up.dispatched(), "direct-mode profile denials must happen before upstream I/O") } +func TestDirectCall_ProfileV3DeniesUnannotatedTool(t *testing.T) { + proxy, rt := newProfilesV3Fixture(t) + updated := *rt.Config() + updated.Profiles = append([]config.ProfileConfig(nil), updated.Profiles...) + updated.Profiles[1].Unannotated = "deny" + rt.UpdateConfig(&updated, "") + up := startCountingUpstream(t, proxy, rt, "github", toolSpec{Name: "search_code", Description: "Search code"}) + + entry := &directCatalogEntry{ + DisplayName: FormatDirectToolName("github", "search_code"), + ServerName: "github", + ToolName: "search_code", + Description: "Search code", + } + ctx := withDirectRequestKindBox(urlProfileCtx(proxy, "work-full")) + setDirectRequestKind(ctx, directRequestKindCall) + result, err := proxy.makeDirectModeHandler(entry)(ctx, mcp.CallToolRequest{ + Params: mcp.CallToolParams{Arguments: map[string]interface{}{}}, + }) + + require.NoError(t, err) + require.NotNil(t, result) + require.True(t, result.IsError) + require.Equal(t, "blocked by profile: github:search_code has no tier annotation; an operator can classify it in the profile to allow it", resultText(t, result)) + require.Empty(t, up.dispatched(), "an unannotated tool denied by profile policy must never reach the upstream") +} + func TestFilterDirectModeToolsForAuth_ProfileV3(t *testing.T) { proxy, _ := newProfilesV3Fixture(t) diff --git a/internal/server/mcp_code_execution.go b/internal/server/mcp_code_execution.go index db234d727..00ba05525 100644 --- a/internal/server/mcp_code_execution.go +++ b/internal/server/mcp_code_execution.go @@ -96,7 +96,8 @@ func (p *MCPProxyServer) handleCodeExecution(ctx context.Context, request mcp.Ca return mcp.NewToolResultError("unknown tool: code_execution"), nil } profileResolution := p.ResolveProfileV3(ctx, profileIdx) - if profileResolution.Policy != nil && !profileResolution.Policy.CodeExecution { + danglingProfile := profileResolution.Base != "" && profileIdx.position(profileResolution.Base) < 0 + if danglingProfile || (profileResolution.Policy != nil && !profileResolution.Policy.CodeExecution) { requestID := mintActivityRequestID("", "code_execution") refusal := profile.ErrCodeExecutionBlocked recordCodeExecRefusal(ctx, refusal) @@ -308,20 +309,21 @@ func (p *MCPProxyServer) handleCodeExecution(ctx context.Context, request mcp.Ca // and installs the sandbox's authorization-decision observer so a // scope/permission refusal decided inside jsruntime — which never // reaches the bridge — still gets its `authz deny`, with parent_id. + options.ParentID = parentCallID + scriptCaller := auditCallerFromContext(ctx) + toolCaller.auditProfile, _ = p.resolveActiveProfile(ctx) + options.AuthzObserver = &nestedAuthzObserver{ + proxy: p, + toolCaller: toolCaller, + parentCtx: ctx, + caller: scriptCaller, + sessionID: sessionID, + clientName: clientName, + clientVersion: clientVersion, + profile: toolCaller.auditProfile, + } if p.auditSink != nil { - scriptCaller := auditCallerFromContext(ctx) toolCaller.auditCaller = &scriptCaller - toolCaller.auditProfile, _ = p.resolveActiveProfile(ctx) - options.ParentID = parentCallID - options.AuthzObserver = &nestedAuthzObserver{ - proxy: p, - parentCtx: ctx, - caller: scriptCaller, - sessionID: sessionID, - clientName: clientName, - clientVersion: clientVersion, - profile: toolCaller.auditProfile, - } } // Execute code diff --git a/internal/server/mcp_code_execution_scope_test.go b/internal/server/mcp_code_execution_scope_test.go index 5823e58ea..910e72b4e 100644 --- a/internal/server/mcp_code_execution_scope_test.go +++ b/internal/server/mcp_code_execution_scope_test.go @@ -327,10 +327,9 @@ func TestCodeExecution_LiveClientConnectedWhileSnapshotSaysDisconnected_RefusesU // from "in neither" even though both are outside this token's own effective // reach. func TestCodeExecution_PinWiderThanToken_IndistinguishableFromNonexistent(t *testing.T) { - proxy, _ := createTestProxyWithRuntime(t, []*config.ServerConfig{{Name: "a", Enabled: true}}) - proxy.config.Profiles = []config.ProfileConfig{ - {Name: "P", Servers: []string{"a", "b"}}, - } + proxy, _ := createTestProxyWithRuntimeCfg(t, []*config.ServerConfig{{Name: "a", Enabled: true}}, func(cfg *config.Config) { + cfg.Profiles = []config.ProfileConfig{{Name: "P", Servers: []string{"a", "b"}}} + }) ctx := agentCtx([]string{"a"}, []string{auth.PermRead, auth.PermWrite, auth.PermDestructive}, "P") diff --git a/internal/server/mcp_routing.go b/internal/server/mcp_routing.go index fa2b11ad5..2fa164f3d 100644 --- a/internal/server/mcp_routing.go +++ b/internal/server/mcp_routing.go @@ -530,7 +530,10 @@ func (p *MCPProxyServer) makeDirectModeHandler(entry *directCatalogEntry) mcpser return nil, refusalErr } if policy := profileResolution.Policy; policy != nil { - intrinsic := profile.IntrinsicTier(annotations, annotations != nil) + // Direct routing resolves an existing catalog entry above. Nil + // annotations therefore mean a known but unannotated tool, which + // must follow the profile's fail-closed unannotated policy. + intrinsic := profile.IntrinsicTier(annotations, true) if admitted, reason, tier := policy.Decide(serverName, toolName, intrinsic); !admitted && reason != profile.ReasonServerNotInProfile { errMsg, blockReason := profileToolPolicyRefusal(reason, tier, policy.Cap, serverName, toolName) p.emitActivityPolicyDecisionWithBlockReason(ctx, serverName, toolName, sessionID, requestID, @@ -1035,7 +1038,7 @@ func (p *MCPProxyServer) filterProfileV3Tools(ctx context.Context, tools []mcp.T // A dangling pinned/bound/anonymous profile is authoritative deny-all. // Do not advertise code execution just because the resolution no longer // has a compiled policy to consult. - danglingProfile := resolution.Name != "" && idx.position(resolution.Name) < 0 + danglingProfile := resolution.Base != "" && idx.position(resolution.Base) < 0 filtered := make([]mcp.Tool, 0, len(tools)) for _, tool := range tools { switch tool.Name { diff --git a/internal/server/profiles_v3_fixture_test.go b/internal/server/profiles_v3_fixture_test.go index 71bd0d001..e156f2304 100644 --- a/internal/server/profiles_v3_fixture_test.go +++ b/internal/server/profiles_v3_fixture_test.go @@ -72,6 +72,10 @@ func enforcementMatrixProfiles() []config.ProfileConfig { // annotation shape), notion (1 write tool) and filesystem (1 read tool) — // exactly as the matrix's fixture table lists them. func newProfilesV3Fixture(t *testing.T) (*MCPProxyServer, *runtime.Runtime) { + return newProfilesV3FixtureWithConfig(t, nil) +} + +func newProfilesV3FixtureWithConfig(t *testing.T, configure func(*config.Config)) (*MCPProxyServer, *runtime.Runtime) { t.Helper() proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { @@ -81,6 +85,9 @@ func newProfilesV3Fixture(t *testing.T) (*MCPProxyServer, *runtime.Runtime) { {Name: "filesystem", Enabled: true}, } cfg.Profiles = enforcementMatrixProfiles() + if configure != nil { + configure(cfg) + } }) startCountingUpstream(t, proxy, rt, "github", diff --git a/internal/server/replay_audit_test.go b/internal/server/replay_audit_test.go index bf8a65fea..5e97ca9d2 100644 --- a/internal/server/replay_audit_test.go +++ b/internal/server/replay_audit_test.go @@ -156,6 +156,14 @@ func TestReplayToolCall_ProfilePolicyRefusesBeforeUpstream(t *testing.T) { Tools: &config.ProfileToolRules{Deny: []string{"github:create_issue"}}, }} }) + sink := &recordingAuditSink{} + proxy.auditSink = sink + go rt.ActivityService().Start(rt.AppContext(), rt) + startDeadline := time.Now().Add(5 * time.Second) + for !rt.ActivityService().Started() && time.Now().Before(startDeadline) { + time.Sleep(time.Millisecond) + } + require.True(t, rt.ActivityService().Started(), "activity service must subscribe before replay is refused") mainSrv := &Server{runtime: rt, mcpProxy: proxy} url, calls := startRuntimeCountingUpstream(t, proxy, "github", "create_issue") callID := seedReplayableCall(t, proxy, mainSrv, "github", "create_issue", url) @@ -166,6 +174,65 @@ func TestReplayToolCall_ProfilePolicyRefusesBeforeUpstream(t *testing.T) { require.ErrorAs(t, err, &refusal) require.Equal(t, profile.BlockReasonRule, refusal.Reason) require.Empty(t, calls.dispatched(), "profile-denied replay must stop before upstream I/O") + + lines := sink.decoded(t) + require.Len(t, lines, 1, "a profile-refused replay writes one authorization denial") + assert.Equal(t, "authz", lines[0]["event"]) + assert.Equal(t, "deny", lines[0]["decision"]) + + require.Eventually(t, func() bool { + records, _, listErr := rt.StorageManager().ListActivities(storage.ActivityFilter{Limit: 50}) + if listErr != nil { + return false + } + for _, record := range records { + if record.Type == storage.ActivityTypePolicyDecision && record.ServerName == "github" && record.ToolName == "create_issue" { + return record.Status == "blocked" && record.Metadata[storage.MetadataKeyBlockReason] == string(profile.BlockReasonRule) + } + } + return false + }, 5*time.Second, 10*time.Millisecond, "a profile-refused replay persists its blocked activity reason") +} + +func TestReplayToolCall_ProfileScopeRefusesWithAuditAndActivity(t *testing.T) { + proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{{Name: "github", Enabled: true}} + cfg.Profiles = []config.ProfileConfig{{Name: "replay-out-of-scope", Servers: []string{"other"}}} + }) + sink := &recordingAuditSink{} + proxy.auditSink = sink + go rt.ActivityService().Start(rt.AppContext(), rt) + startDeadline := time.Now().Add(5 * time.Second) + for !rt.ActivityService().Started() && time.Now().Before(startDeadline) { + time.Sleep(time.Millisecond) + } + require.True(t, rt.ActivityService().Started(), "activity service must subscribe before replay is refused") + mainSrv := &Server{runtime: rt, mcpProxy: proxy} + url, calls := startRuntimeCountingUpstream(t, proxy, "github", "create_issue") + callID := seedReplayableCall(t, proxy, mainSrv, "github", "create_issue", url) + + _, err := mainSrv.ReplayToolCall(urlProfileCtx(proxy, "replay-out-of-scope"), callID, nil) + require.ErrorIs(t, err, profile.ErrToolOutsideProfile) + require.Empty(t, calls.dispatched(), "out-of-scope replay must stop before upstream I/O") + + lines := sink.decoded(t) + require.Len(t, lines, 1, "a profile-scope refusal writes one authorization denial") + assert.Equal(t, "authz", lines[0]["event"]) + assert.Equal(t, "deny", lines[0]["decision"]) + assert.Equal(t, "profile_scope", lines[0]["reason"]) + + require.Eventually(t, func() bool { + records, _, listErr := rt.StorageManager().ListActivities(storage.ActivityFilter{Limit: 50}) + if listErr != nil { + return false + } + for _, record := range records { + if record.Type == storage.ActivityTypePolicyDecision && record.ServerName == "github" && record.ToolName == "create_issue" { + return record.Status == "blocked" && record.Metadata["reason"] == profile.ErrToolOutsideProfile.Error() + } + } + return false + }, 5*time.Second, 10*time.Millisecond, "a profile-scope replay refusal persists a blocked activity record") } func TestReplayToolCall_UnresolvedIDDelegatesUnaudited(t *testing.T) { diff --git a/internal/server/scope_oracle_v3_test.go b/internal/server/scope_oracle_v3_test.go index 5f7367989..9841cf9ab 100644 --- a/internal/server/scope_oracle_v3_test.go +++ b/internal/server/scope_oracle_v3_test.go @@ -3,6 +3,7 @@ package server import ( "context" "encoding/json" + "strings" "testing" "github.com/stretchr/testify/assert" @@ -160,14 +161,28 @@ func TestScopeOracleV3_NewResolutionSourcesMatchAcrossFixtures(t *testing.T) { "switchable client binding": {clientCtx("desktop", "cap-read-a", "switchable"), clientCtx("desktop", "cap-read-a", "switchable")}, "anonymous profile": {anonCtx(), anonCtx()}, } + queries := []string{ + "read_thing", "write_thing", "destroy_thing", "erase", "ns_erase", + "SENTINEL_scopeOracleV3B_71a2_tool", "SENTINEL_scopeOracleV3AB_39fe_tool", + } for source, callers := range sources { t.Run(source, func(t *testing.T) { - for _, query := range []string{"read_thing", "write_thing", "destroy_thing", "erase", "ns_erase"} { + for _, query := range queries { t.Run(query, func(t *testing.T) { narrowResp := callRetrieveToolsV3(t, narrow.proxy, callers.narrow, query, 10) fullResp := callRetrieveToolsV3(t, full.proxy, callers.full, query, 10) + require.NotNil(t, narrowResp.HiddenByProfile, "the profile must be active for %s", source) + require.NotNil(t, fullResp.HiddenByProfile, "the profile must be active for %s", source) assert.Nil(t, narrowResp.Profile, "credential and anonymous resolution sources never reveal the base profile") assert.Nil(t, fullResp.Profile, "credential and anonymous resolution sources never reveal the base profile") + if query == "read_thing" { + require.NotEmpty(t, narrowResp.Tools, "positive control: the admitted read tool is discoverable") + require.NotEmpty(t, fullResp.Tools, "positive control: the admitted read tool is discoverable") + } + if strings.HasPrefix(query, "SENTINEL") { + assert.Empty(t, narrowResp.Tools) + assert.Empty(t, fullResp.Tools, "out-of-profile servers must stay hidden for %s", source) + } left, err := json.Marshal(narrowResp) require.NoError(t, err) right, err := json.Marshal(fullResp) @@ -177,4 +192,10 @@ func TestScopeOracleV3_NewResolutionSourcesMatchAcrossFixtures(t *testing.T) { } }) } + t.Run("admin control: full fixture contains both hidden sentinel tools", func(t *testing.T) { + for _, query := range queries[5:] { + resp := callRetrieveToolsV3(t, full.proxy, adminCtx(), query, 10) + require.NotEmpty(t, resp.Tools, "fixture premise: administrator must find %s", query) + } + }) } diff --git a/internal/server/server.go b/internal/server/server.go index 8dff65db4..442721693 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -3944,6 +3944,21 @@ func (s *Server) ReplayToolCall(ctx context.Context, id string, arguments map[st if callArgs == nil { callArgs = original.Arguments } + // Install the attempt before the profile gate so an authorization refusal + // is recorded with the same audit context as every other dispatch path. + var operation string + if original.Annotations != nil { + operation = tierForAnnotations(toConfigToolAnnotations(original.Annotations), true) + } + requestID := mintCorrelationID(original.ServerName, original.ToolName) + ctx = s.mcpProxy.installAuditAttempt(ctx, auditAttemptSpec{ + RequestID: requestID, + Server: original.ServerName, + Tool: original.ToolName, + Operation: operation, + Surface: auditSurfaceREST, + Args: callArgs, + }) // Spec 108 FR-015: replay is a dispatch path, so evaluate its recorded // server/tool against the same request-scoped profile resolution before @@ -3952,6 +3967,8 @@ func (s *Server) ReplayToolCall(ctx context.Context, id string, arguments map[st profileIndex := s.mcpProxy.profileIndexCurrent(ctx) profileResolution := s.mcpProxy.ResolveProfileV3(ctx, profileIndex) if profileResolution.Scope != nil && !profileResolution.Scope.Allows(original.ServerName) { + s.mcpProxy.emitActivityPolicyDecision(ctx, original.ServerName, original.ToolName, + sessionIDFromContext(ctx), requestID, "blocked", profile.ErrToolOutsideProfile.Error(), telemetry.BlockReasonProfileScope) return nil, profile.ErrToolOutsideProfile } if policy := profileResolution.Policy; policy != nil { @@ -3960,7 +3977,6 @@ func (s *Server) ReplayToolCall(ctx context.Context, id string, arguments map[st if admitted, reason, tier := policy.Decide(original.ServerName, original.ToolName, intrinsic); !admitted && reason != profile.ReasonServerNotInProfile { message, blockReason := profileToolPolicyRefusal(reason, tier, policy.Cap, original.ServerName, original.ToolName) refusal := &profile.ToolBlockedError{Reason: blockReason, Message: message} - requestID := mintCorrelationID(original.ServerName, original.ToolName) s.mcpProxy.emitActivityPolicyDecisionWithBlockReason(ctx, original.ServerName, original.ToolName, sessionIDFromContext(ctx), requestID, "blocked", message, telemetry.BlockReasonOther, string(blockReason)) return nil, refusal @@ -3968,28 +3984,9 @@ func (s *Server) ReplayToolCall(ctx context.Context, id string, arguments map[st } // Spec 107 (round-3 cross-review finding, PR-D): the persisted record's - // own annotations snapshot is the canonical target tier here — the same - // signal tierForAnnotations derives from a live gate's identity lookup - // elsewhere — so a replayed destructive/write call is not reported as - // `operation:"unknown"` when the snapshot is available. Left empty (and - // so defaulted to "unknown" by installAuditAttempt) when the record - // carries no annotations at all: mirrors mcp.go's own choice not to use - // tierForAnnotations' found=false "destructive" default for the AUDIT - // line — that default is an AUTHORIZATION fail-closed, and would - // misrepresent an unresolved tier as maximally risky rather than simply - // unknown to the proxy. - var operation string - if original.Annotations != nil { - operation = tierForAnnotations(toConfigToolAnnotations(original.Annotations), true) - } - ctx = s.mcpProxy.installAuditAttempt(ctx, auditAttemptSpec{ - RequestID: mintCorrelationID(original.ServerName, original.ToolName), - Server: original.ServerName, - Tool: original.ToolName, - Operation: operation, - Surface: auditSurfaceREST, - Args: callArgs, - }) + // own annotations snapshot supplies the operation tier above before any + // gate runs, so a denied replay has the same correctly classified audit + // context as a dispatched replay. // Spec 107 FR-012: `decision: allow` MUST be written after the last gate // and before the upstream call (round-3 cross-review finding, PR-D) — // auditToolCall's own backfill only runs on completion, which would From fd72e028a8c4fcab52da659d414ba642d714065f Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 00:30:21 +0300 Subject: [PATCH 03/11] test(profile): exercise hidden token server refusal --- internal/server/mcp_code_execution_scope_test.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/internal/server/mcp_code_execution_scope_test.go b/internal/server/mcp_code_execution_scope_test.go index 910e72b4e..7ac19df71 100644 --- a/internal/server/mcp_code_execution_scope_test.go +++ b/internal/server/mcp_code_execution_scope_test.go @@ -327,7 +327,12 @@ func TestCodeExecution_LiveClientConnectedWhileSnapshotSaysDisconnected_RefusesU // from "in neither" even though both are outside this token's own effective // reach. func TestCodeExecution_PinWiderThanToken_IndistinguishableFromNonexistent(t *testing.T) { - proxy, _ := createTestProxyWithRuntimeCfg(t, []*config.ServerConfig{{Name: "a", Enabled: true}}, func(cfg *config.Config) { + proxy, _ := createTestProxyWithRuntimeCfg(t, []*config.ServerConfig{ + {Name: "a", Enabled: true}, + // b is in P's effective scope but remains outside the token grant. + // It need not be connected: the scope/token denial happens first. + {Name: "b", Enabled: false}, + }, func(cfg *config.Config) { cfg.Profiles = []config.ProfileConfig{{Name: "P", Servers: []string{"a", "b"}}} }) @@ -338,6 +343,9 @@ func TestCodeExecution_PinWiderThanToken_IndistinguishableFromNonexistent(t *tes assert.False(t, callB.OK, "'b' is in the pin but outside the token's own scope — must refuse") assert.False(t, callZZZ.OK, "'zzz' does not exist — must refuse") + assert.Equal(t, string(jsruntime.ErrorCodeAccessDenied), callB.Code, + "the in-profile/out-of-token cell must take the token-scope refusal") + assert.Contains(t, callB.Message, "token does not have access to server") assert.Equal(t, callZZZ.Code, callB.Code, "the envelope CODE must be identical whether the server is in the pin-but-not-token, or in neither") From e51140128baf48ae9b397f237cd464efb151658e Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 00:55:24 +0300 Subject: [PATCH 04/11] fix(profile): guard anonymous binding bypass --- internal/server/mcp.go | 8 +- internal/server/mcp_code_execution.go | 2 +- internal/server/mcp_routing.go | 7 +- internal/server/profile_binding_guard.go | 245 ++++++++++++++++++ internal/server/profile_binding_guard_test.go | 174 +++++++++++++ internal/server/profile_resolver_v3.go | 26 ++ internal/server/profile_tool.go | 12 + 7 files changed, 467 insertions(+), 7 deletions(-) create mode 100644 internal/server/profile_binding_guard.go create mode 100644 internal/server/profile_binding_guard_test.go diff --git a/internal/server/mcp.go b/internal/server/mcp.go index 9745099a6..c058aafc8 100644 --- a/internal/server/mcp.go +++ b/internal/server/mcp.go @@ -2793,7 +2793,7 @@ func (p *MCPProxyServer) handleCallToolVariant(ctx context.Context, request mcp. // one. Administrators keep today's profile-only text unchanged — they // have no token scope to intersect with. scopeAuthCtx := auth.AuthContextFromContext(ctx) - confinedAnonymous := (scopeAuthCtx == nil || scopeAuthCtx.Anonymous) && profileResolution.Base != "" + confinedAnonymous := (scopeAuthCtx == nil || scopeAuthCtx.Anonymous) && profileResolution.anonymousConfinementActive() scopeAuthCtx = auth.ScopedView(scopeAuthCtx, confinedAnonymous) scopedCallerForScope := auth.IsNonAdmin(scopeAuthCtx) if scopedCallerForScope { @@ -4109,7 +4109,7 @@ func (p *MCPProxyServer) handleUpstreamServers(ctx context.Context, request mcp. // /api/v1/servers handlers, so the two can never drift (issues #877/#878). profileResolution := p.ResolveProfileV3(ctx, p.profileIndexCurrent(ctx)) requestAuth := auth.AuthContextFromContext(ctx) - authCtx := auth.ScopedView(requestAuth, profileResolution.Base != "" && (requestAuth == nil || requestAuth.Anonymous)) + authCtx := auth.ScopedView(requestAuth, profileResolution.anonymousConfinementActive() && (requestAuth == nil || requestAuth.Anonymous)) if !auth.AuthorizeServerOp(authCtx, operation) { errMsg := fmt.Sprintf("Agent tokens cannot perform '%s' operations on upstream servers", operation) p.emitActivityInternalToolCall("upstream_servers", targetServer, "", "", sessionID, requestID, "error", errMsg, time.Since(startTime).Milliseconds(), args, nil, nil, "") @@ -4492,7 +4492,7 @@ func (p *MCPProxyServer) handleListUpstreams(ctx context.Context) (*mcp.CallTool // Spec 028: Filter servers to only those the agent token can access profileResolution := p.ResolveProfileV3(ctx, p.profileIndexCurrent(ctx)) requestAuth := auth.AuthContextFromContext(ctx) - confinedAnonymous := (requestAuth == nil || requestAuth.Anonymous) && profileResolution.Base != "" + confinedAnonymous := (requestAuth == nil || requestAuth.Anonymous) && profileResolution.anonymousConfinementActive() authCtx := auth.ScopedView(requestAuth, confinedAnonymous) scopedCaller := auth.IsNonAdmin(authCtx) if scopedCaller { @@ -6470,7 +6470,7 @@ func (p *MCPProxyServer) handleTailLog(ctx context.Context, request mcp.CallTool // so the response discloses neither existence, status nor logs. profileResolution := p.ResolveProfileV3(ctx, p.profileIndexCurrent(ctx)) requestAuth := auth.AuthContextFromContext(ctx) - confinedAnonymous := (requestAuth == nil || requestAuth.Anonymous) && profileResolution.Base != "" + confinedAnonymous := (requestAuth == nil || requestAuth.Anonymous) && profileResolution.anonymousConfinementActive() authCtx := auth.ScopedView(requestAuth, confinedAnonymous) profileScope := profileResolution.Scope if !p.serverInScope(authCtx, profileScope, name) { diff --git a/internal/server/mcp_code_execution.go b/internal/server/mcp_code_execution.go index 00ba05525..1c6186db4 100644 --- a/internal/server/mcp_code_execution.go +++ b/internal/server/mcp_code_execution.go @@ -97,7 +97,7 @@ func (p *MCPProxyServer) handleCodeExecution(ctx context.Context, request mcp.Ca } profileResolution := p.ResolveProfileV3(ctx, profileIdx) danglingProfile := profileResolution.Base != "" && profileIdx.position(profileResolution.Base) < 0 - if danglingProfile || (profileResolution.Policy != nil && !profileResolution.Policy.CodeExecution) { + if profileResolution.BindingGuarded || danglingProfile || (profileResolution.Policy != nil && !profileResolution.Policy.CodeExecution) { requestID := mintActivityRequestID("", "code_execution") refusal := profile.ErrCodeExecutionBlocked recordCodeExecRefusal(ctx, refusal) diff --git a/internal/server/mcp_routing.go b/internal/server/mcp_routing.go index 2fa164f3d..b24b69322 100644 --- a/internal/server/mcp_routing.go +++ b/internal/server/mcp_routing.go @@ -1043,7 +1043,7 @@ func (p *MCPProxyServer) filterProfileV3Tools(ctx context.Context, tools []mcp.T for _, tool := range tools { switch tool.Name { case "code_execution": - if danglingProfile || (resolution.Policy != nil && !resolution.Policy.CodeExecution) { + if resolution.BindingGuarded || danglingProfile || (resolution.Policy != nil && !resolution.Policy.CodeExecution) { continue } case "upstream_servers", "quarantine_security": @@ -1070,7 +1070,10 @@ func (p *MCPProxyServer) profileManagementToolHidden(ctx context.Context, toolNa return resolution.Scope != nil || resolution.Policy != nil } resolution := p.ResolveProfileV3(ctx, idx) - confinedAnonymous := (ac == nil || ac.Anonymous) && resolution.Base != "" + if resolution.BindingGuarded { + return true + } + confinedAnonymous := (ac == nil || ac.Anonymous) && resolution.anonymousConfinementActive() managementEnabled := resolution.Policy != nil && resolution.Policy.ManagementTools != nil && *resolution.Policy.ManagementTools if resolution.Policy != nil && resolution.Policy.ManagementTools != nil && !*resolution.Policy.ManagementTools { return true diff --git a/internal/server/profile_binding_guard.go b/internal/server/profile_binding_guard.go new file mode 100644 index 000000000..38b68075a --- /dev/null +++ b/internal/server/profile_binding_guard.go @@ -0,0 +1,245 @@ +package server + +import ( + "strings" + "time" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" + "go.uber.org/zap" +) + +type bindingGuardTool struct { + server string + tool string + tier profile.Tier +} + +// bindingGuardActive is the fail-closed runtime half of FR-008a. It reads +// current client bindings on every anonymous request because token bindings +// can change without publishing a config snapshot. Policy comparisons use the +// same profile index and the currently published tool snapshot that execution +// uses; a newly discovered tool can therefore activate the guard immediately. +func (p *MCPProxyServer) bindingGuardActive(idx *profileIndex) bool { + cfg := p.currentConfig() + if idx != nil && idx.cfg != nil { + cfg = idx.cfg + } + if config.EffectiveRequireMCPAuth(cfg) { + return false + } + if p.storage == nil { + // A production proxy always has storage. A bare proxy used by unit + // tests has no credential records to inspect, so it has no bindings. + return false + } + tokens, err := p.storage.ListAgentTokens() + if err != nil { + if p.logger != nil { + p.logger.Error("cannot inspect client bindings for anonymous profile guard", zap.Error(err)) + } + return true + } + if cfg == nil || idx == nil || idx.cfg == nil { + // During a publication gap the guard cannot compare reachability. Any + // live named client binding therefore denies anonymous access until a + // coherent index/snapshot pair is available again. + for i := range tokens { + if activeNamedClientBinding(&tokens[i], time.Now()) { + return true + } + } + return false + } + tools := p.bindingGuardTools() + now := time.Now() + for i := range tokens { + token := &tokens[i] + if !activeNamedClientBinding(token, now) { + continue + } + if bindingBypassable(idx, cfg, token, tools) { + return true + } + } + return false +} + +func activeNamedClientBinding(token *auth.AgentToken, now time.Time) bool { + return token != nil && token.Kind == auth.KindClient && !token.Revoked && + token.ExpiresAt.After(now) && token.ProfilePin != "" && + (token.ProfileMode == auth.ProfileModeLocked || token.ProfileMode == auth.ProfileModeSwitchable) +} + +func bindingBypassable(idx *profileIndex, cfg *config.Config, token *auth.AgentToken, tools []bindingGuardTool) bool { + if token == nil || token.ProfilePin == "" || idx == nil || idx.cfg == nil || cfg == nil { + return false + } + boundReach, ok := bindingReachableProfiles(idx, token.ProfilePin, token.ProfileMode == auth.ProfileModeSwitchable) + if !ok { + // A dangling bound base is already deny-all and cannot be bypassed. + return false + } + if cfg.AnonymousProfile == "" { + return true // anonymous currently means unrestricted access + } + anonymousReach, ok := bindingReachableProfiles(idx, cfg.AnonymousProfile, true) + if !ok { + // A dangling anonymous base is itself deny-all. + return false + } + boundPolicies := make([]*profile.CompiledPolicy, 0, len(boundReach)) + boundServers := make(map[string]struct{}) + for _, name := range boundReach { + pos := idx.position(name) + if pos < 0 { + continue // dangling switchable target is deny-all + } + policy := idx.PolicyAt(pos) + if policy == nil { + continue + } + boundPolicies = append(boundPolicies, policy) + for _, server := range idx.effectiveServersForCandidate(pos, []string{"*"}) { + boundServers[server] = struct{}{} + } + } + if len(boundPolicies) == 0 { + return false + } + + maxBoundCap, maxBoundUnannotated := 0, 0 + allBoundCodeExecutionOff, allBoundManagementOff := true, true + for _, policy := range boundPolicies { + if tierCapRank(policy.Cap) > maxBoundCap { + maxBoundCap = tierCapRank(policy.Cap) + } + if unannotatedRank(policy.Unannotated) > maxBoundUnannotated { + maxBoundUnannotated = unannotatedRank(policy.Unannotated) + } + allBoundCodeExecutionOff = allBoundCodeExecutionOff && cfg.EnableCodeExecution && !policy.CodeExecution + allBoundManagementOff = allBoundManagementOff && !profileManagementEnabled(policy) + } + + for _, name := range anonymousReach { + pos := idx.position(name) + if pos < 0 { + continue // dangling anonymous-reachable profiles are deny-all + } + policy := idx.PolicyAt(pos) + if policy == nil { + continue + } + for _, server := range idx.effectiveServersForCandidate(pos, []string{"*"}) { + if _, covered := boundServers[server]; !covered { + return true + } + } + if tierCapRank(policy.Cap) > maxBoundCap || unannotatedRank(policy.Unannotated) > maxBoundUnannotated { + return true + } + if cfg.EnableCodeExecution && policy.CodeExecution && allBoundCodeExecutionOff { + return true + } + if profileManagementEnabled(policy) && allBoundManagementOff { + return true + } + for _, tool := range tools { + admitted, _, _ := policy.Decide(tool.server, tool.tool, tool.tier) + if !admitted { + continue + } + admittedByBinding := false + for _, bound := range boundPolicies { + if allowed, _, _ := bound.Decide(tool.server, tool.tool, tool.tier); allowed { + admittedByBinding = true + break + } + } + if !admittedByBinding { + return true + } + } + } + return false +} + +func bindingReachableProfiles(idx *profileIndex, base string, switchable bool) ([]string, bool) { + pos := idx.position(base) + if pos < 0 { + return nil, false + } + reachable := []string{base} + if switchable { + if policy := idx.PolicyAt(pos); policy != nil { + for name := range policy.SwitchableTo { + if idx.position(name) >= 0 { + reachable = append(reachable, name) + } + } + } + } + return reachable, true +} + +func tierCapRank(tier profile.Tier) int { + switch tier { + case profile.TierRead: + return 1 + case profile.TierWrite: + return 2 + case profile.TierDestructive: + return 3 + default: + return 4 // an unset cap admits every tier + } +} + +func unannotatedRank(value string) int { + switch value { + case config.ProfileUnannotatedDeny: + return 1 + case config.ProfileUnannotatedAsWrite: + return 2 + default: + return 3 // as_read is the most permissive policy + } +} + +func profileManagementEnabled(policy *profile.CompiledPolicy) bool { + return policy != nil && policy.ManagementTools != nil && *policy.ManagementTools +} + +func (p *MCPProxyServer) bindingGuardTools() []bindingGuardTool { + if p.mainServer == nil || p.mainServer.runtime == nil { + return nil + } + supervisor := p.mainServer.runtime.Supervisor() + if supervisor == nil || supervisor.StateView() == nil { + return nil + } + snapshot := supervisor.StateView().Snapshot() + if snapshot == nil { + return nil + } + var tools []bindingGuardTool + for serverName, status := range snapshot.Servers { + if status == nil || !status.ToolsDiscovered { + continue + } + for _, info := range status.Tools { + toolName := info.Name + if prefix := serverName + ":"; strings.HasPrefix(toolName, prefix) { + toolName = strings.TrimPrefix(toolName, prefix) + } + annotations, found := p.EffectiveAnnotations(serverName, toolName) + tools = append(tools, bindingGuardTool{ + server: serverName, + tool: toolName, + tier: profile.IntrinsicTier(annotations, found), + }) + } + } + return tools +} diff --git a/internal/server/profile_binding_guard_test.go b/internal/server/profile_binding_guard_test.go new file mode 100644 index 000000000..8cc022fd4 --- /dev/null +++ b/internal/server/profile_binding_guard_test.go @@ -0,0 +1,174 @@ +package server + +import ( + "context" + "testing" + "time" + + "github.com/mark3labs/mcp-go/mcp" + "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime/stateview" +) + +func bindingGuardTestProxy(t *testing.T, anonymousProfile string, profiles []config.ProfileConfig, mode, pin string) (*MCPProxyServer, *profileIndex) { + t.Helper() + proxy, rt := createTestProxyWithRuntimeCfg(t, []*config.ServerConfig{{Name: "a", Enabled: true}}, func(cfg *config.Config) { + cfg.RequireMCPAuth = false + cfg.AnonymousProfile = anonymousProfile + cfg.Profiles = profiles + }) + _, err := rt.StorageManager().MintClientCredential( + "cursor", "mcp_cli_binding_guard_test", []byte("binding-guard-test-key"), + mode, pin, time.Now().Add(time.Hour), + ) + require.NoError(t, err) + return proxy, proxy.profileIndexFor(proxy.currentConfig()) +} + +func TestResolveProfileV3_AnonymousDeniedWhenNamedBindingCanBeBypassed(t *testing.T) { + proxy, idx := bindingGuardTestProxy(t, "", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") + + got := proxy.ResolveProfileV3(context.Background(), idx) + + require.Equal(t, string(profile.SourceAnonymous), got.Source) + require.Nil(t, got.Policy) + require.NotNil(t, got.Scope) + require.False(t, got.Scope.Allows("a"), "unconfined anonymous access must be deny-all while a named client binding can be bypassed") + require.True(t, got.BindingGuarded) + require.True(t, proxy.profileManagementToolHidden(context.Background(), "upstream_servers"), "the deny-all guard must also remove administrator-shaped management access") + codeExec, err := proxy.handleCodeExecution(context.Background(), mcp.CallToolRequest{Params: mcp.CallToolParams{Name: "code_execution"}}) + require.NoError(t, err) + require.NotNil(t, codeExec) + require.True(t, codeExec.IsError, "the deny-all guard must refuse code_execution even with no named anonymous profile") +} + +func TestResolveProfileV3_AnonymousBindingGuardUsesCurrentToolPolicy(t *testing.T) { + proxy, idx := bindingGuardTestProxy(t, "Q", []config.ProfileConfig{ + {Name: "P", Servers: []string{"a"}, Tools: &config.ProfileToolRules{Deny: []string{"a:write_tool"}}}, + {Name: "Q", Servers: []string{"a"}}, + }, auth.ProfileModeLocked, "P") + proxy.mainServer.runtime.Supervisor().StateView().UpdateServer("a", func(status *stateview.ServerStatus) { + status.ToolsDiscovered = true + status.Tools = []stateview.ToolInfo{{Name: "write_tool", Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(false)}}} + }) + + got := proxy.ResolveProfileV3(context.Background(), idx) + + require.Equal(t, string(profile.SourceAnonymous), got.Source) + require.Nil(t, got.Policy) + require.NotNil(t, got.Scope) + require.False(t, got.Scope.Allows("a"), "anonymous Q admits a live tool denied by P, so the binding guard must deny the whole anonymous profile") +} + +func TestResolveProfileV3_EqualAnonymousProfileDoesNotTripBindingGuard(t *testing.T) { + proxy, idx := bindingGuardTestProxy(t, "P", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") + + got := proxy.ResolveProfileV3(context.Background(), idx) + + require.Equal(t, string(profile.SourceAnonymous), got.Source) + require.Equal(t, "P", got.Name) + require.NotNil(t, got.Scope) + require.True(t, got.Scope.Allows("a"), "an anonymous profile equal to the locked binding is not bypassable") +} + +func TestHandleSetProfile_AnonymousBindingGuardRefusesSelectionButAllowsClear(t *testing.T) { + proxy, _ := bindingGuardTestProxy(t, "", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") + ctx := sessionCtx(context.Background(), "anonymous-bound-session") + call := func(slug string) *mcp.CallToolResult { + t.Helper() + request := mcp.CallToolRequest{Params: mcp.CallToolParams{ + Name: "set_profile", Arguments: map[string]interface{}{"profile": slug}, + }} + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + return result + } + + refused := call("P") + require.True(t, refused.IsError, "anonymous callers cannot set a profile that would bypass a named client binding") + require.Equal(t, "unknown profile 'P'", setProfileResultText(t, refused)) + require.Empty(t, proxy.sessionStore.GetActiveProfile("anonymous-bound-session"), "the refused selection must not mutate the session") + + cleared := call("") + require.False(t, cleared.IsError, "clearing a stored selection remains admitted for every caller") + require.Empty(t, proxy.sessionStore.GetActiveProfile("anonymous-bound-session")) +} + +func TestBindingBypassable_FR008aReachabilityMatrix(t *testing.T) { + switchToR := []string{"R"} + falseVal, trueVal := false, true + tool := []bindingGuardTool{{server: "a", tool: "write_tool", tier: profile.TierWrite}} + tests := []struct { + name string + anonymous string + profiles []config.ProfileConfig + mode string + pin string + tools []bindingGuardTool + want bool + }{ + { + name: "equal locked profile is safe", anonymous: "P", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, + }, + { + name: "unconfined anonymous is always wider", anonymous: "", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, want: true, + }, + { + name: "wider effective server set", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}, {Name: "Q", Servers: []string{"a", "b"}}}, want: true, + }, + { + name: "wider tier cap", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, MaxTier: config.ProfileTierRead}, {Name: "Q", Servers: []string{"a"}, MaxTier: config.ProfileTierWrite}}, want: true, + }, + { + name: "more permissive unannotated handling", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, Unannotated: config.ProfileUnannotatedDeny}, {Name: "Q", Servers: []string{"a"}, Unannotated: config.ProfileUnannotatedAsRead}}, want: true, + }, + { + name: "tool deny omitted on anonymous reach", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, tools: tool, want: true, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, Tools: &config.ProfileToolRules{Deny: []string{"a:write_tool"}}}, {Name: "Q", Servers: []string{"a"}}}, + }, + { + name: "locked binding cannot reach anonymous switchable target", anonymous: "P", pin: "P", mode: auth.ProfileModeLocked, want: true, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, SwitchableTo: &switchToR}, {Name: "R", Servers: []string{"b"}, MaxTier: config.ProfileTierDestructive}}, + }, + { + name: "switchable binding reaches same one-hop target", anonymous: "P", pin: "P", mode: auth.ProfileModeSwitchable, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, SwitchableTo: &switchToR}, {Name: "R", Servers: []string{"b"}, MaxTier: config.ProfileTierDestructive}}, + }, + { + name: "code execution capability wider", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, want: true, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, CodeExecution: &falseVal}, {Name: "Q", Servers: []string{"a"}, CodeExecution: &trueVal}}, + }, + { + name: "management capability wider", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, want: true, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, ManagementTools: &falseVal}, {Name: "Q", Servers: []string{"a"}, ManagementTools: &trueVal}}, + }, + { + name: "dangling bound base is already deny all", anonymous: "Q", pin: "missing", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "Q", Servers: []string{"a", "b"}}}, + }, + { + name: "dangling anonymous base is deny all", anonymous: "missing", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + cfg := config.DefaultConfig() + cfg.Servers = []*config.ServerConfig{{Name: "a", Enabled: true}, {Name: "b", Enabled: true}} + cfg.AnonymousProfile = tt.anonymous + cfg.Profiles = tt.profiles + idx := newProfileIndex(cfg) + binding := &auth.AgentToken{Kind: auth.KindClient, ProfileMode: tt.mode, ProfilePin: tt.pin} + require.Equal(t, tt.want, bindingBypassable(idx, cfg, binding, tt.tools)) + }) + } +} diff --git a/internal/server/profile_resolver_v3.go b/internal/server/profile_resolver_v3.go index d81da1dd7..72a0992cf 100644 --- a/internal/server/profile_resolver_v3.go +++ b/internal/server/profile_resolver_v3.go @@ -29,6 +29,15 @@ type ProfileResolution struct { // admission — "" for sources url/session/none, and for a source // pin/binding/anonymous whose base is "All servers" (empty pin). Base string + // BindingGuarded marks the FR-008a anonymous deny-all resolution when the + // anonymous base is empty. Callers that shape management or code-execution + // capabilities use this alongside Base so an empty anonymous_profile cannot + // retain the legacy administrator-shaped view. + BindingGuarded bool +} + +func (r ProfileResolution) anonymousConfinementActive() bool { + return r.Base != "" || r.BindingGuarded } // clientCredentialFromContext returns (pin, mode, ok) for a Spec 108-c @@ -102,6 +111,18 @@ func resolveV3Base(ctx context.Context, idx *profileIndex) (name string, source // snapshots. Legacy surfaces not yet migrated can still use // resolveActiveProfileFromIndex. func (p *MCPProxyServer) ResolveProfileV3(ctx context.Context, idx *profileIndex) ProfileResolution { + if anonymousProfileCaller(ctx) && p.bindingGuardActive(idx) { + base := "" + if idx != nil && idx.cfg != nil { + base = idx.cfg.AnonymousProfile + } else if cfg := p.currentConfig(); cfg != nil { + base = cfg.AnonymousProfile + } + return ProfileResolution{ + Name: base, Source: string(profile.SourceAnonymous), + Scope: profile.NewProfileScope(base, nil), Base: base, BindingGuarded: true, + } + } if idx == nil { // profileIndexCurrent deliberately returns nil for a scoped request // during a publication gap instead of building an index on the request @@ -201,3 +222,8 @@ func (p *MCPProxyServer) ResolveProfileV3(ctx context.Context, idx *profileIndex return ProfileResolution{Source: string(profile.SourceNone)} } + +func anonymousProfileCaller(ctx context.Context) bool { + ac := auth.AuthContextFromContext(ctx) + return ac == nil || ac.Anonymous +} diff --git a/internal/server/profile_tool.go b/internal/server/profile_tool.go index d2c31470c..84f48bf4a 100644 --- a/internal/server/profile_tool.go +++ b/internal/server/profile_tool.go @@ -91,6 +91,12 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s'", slug)), nil } cfg := profiles.cfg + anonymousBindingGuard := anonymousProfileCaller(ctx) && p.bindingGuardActive(profiles) + if slug != "" && anonymousBindingGuard { + // Keep the refusal shape indistinguishable from an unknown profile and + // leave the session untouched while the FR-008a runtime guard is active. + return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s'", slug)), nil + } // A non-empty slug must name a configured profile the caller may select // (an empty slug clears the selection and is always accepted). The check @@ -127,6 +133,12 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT } p.sessionStore.SetActiveProfile(sessionID, slug) + if slug == "" && anonymousBindingGuard { + // Clearing is always admitted (FR-018), but the guarded anonymous + // caller still has no reachable servers, so do not return the legacy + // administrator-shaped all-server list. + return setProfileResult("", []string{}) + } if slug != "" { p.logger.Info("set_profile: session profile updated", zap.String("session_id", sessionID), From 5ff74f5423fe47089fa31554a66dbc9f6e3344c2 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 01:09:29 +0300 Subject: [PATCH 05/11] fix(profile): align guarded direct discovery --- internal/server/mcp_describe_direct.go | 8 +++++ internal/server/profile_binding_guard.go | 5 +++- internal/server/profile_binding_guard_test.go | 29 +++++++++++++++++++ 3 files changed, 41 insertions(+), 1 deletion(-) diff --git a/internal/server/mcp_describe_direct.go b/internal/server/mcp_describe_direct.go index fb1eb5b2d..6bec7f0c9 100644 --- a/internal/server/mcp_describe_direct.go +++ b/internal/server/mcp_describe_direct.go @@ -156,6 +156,14 @@ func (p *MCPProxyServer) resolveDirectDescribeIDIn(ctx context.Context, cat *dir func (p *MCPProxyServer) directEntryVisibleToSession(ctx context.Context, entry *directCatalogEntry) bool { authCtx := auth.AuthContextFromContext(ctx) profileName, profileScope, profileIdx := p.resolveActiveProfileWithIndex(ctx) + if anonymousProfileCaller(ctx) { + // The legacy resolver has no anonymous-profile tier. Direct-mode + // describe and check must use the same V3 resolution as direct tools/list + // so FR-008a's deny-all guard cannot be bypassed by asking for a schema. + resolution := p.ResolveProfileV3(ctx, profileIdx) + profileName = resolution.Name + profileScope = resolution.Scope + } isScopedAgent := isScopeRestrictedCaller(authCtx) if !directEntryInScope(authCtx, profileScope, isScopedAgent, entry) { diff --git a/internal/server/profile_binding_guard.go b/internal/server/profile_binding_guard.go index 38b68075a..ad7c1bb08 100644 --- a/internal/server/profile_binding_guard.go +++ b/internal/server/profile_binding_guard.go @@ -192,7 +192,10 @@ func tierCapRank(tier profile.Tier) int { case profile.TierDestructive: return 3 default: - return 4 // an unset cap admits every tier + // TierUnannotated is the no-cap sentinel. It admits the same finite + // tier set as TierDestructive, so treating it as wider would reject + // otherwise identical profiles during the binding comparison. + return 3 } } diff --git a/internal/server/profile_binding_guard_test.go b/internal/server/profile_binding_guard_test.go index 8cc022fd4..9061d9925 100644 --- a/internal/server/profile_binding_guard_test.go +++ b/internal/server/profile_binding_guard_test.go @@ -12,6 +12,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" "github.com/smart-mcp-proxy/mcpproxy-go/internal/runtime/stateview" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/storage" ) func bindingGuardTestProxy(t *testing.T, anonymousProfile string, profiles []config.ProfileConfig, mode, pin string) (*MCPProxyServer, *profileIndex) { @@ -75,6 +76,30 @@ func TestResolveProfileV3_EqualAnonymousProfileDoesNotTripBindingGuard(t *testin require.True(t, got.Scope.Allows("a"), "an anonymous profile equal to the locked binding is not bypassable") } +func TestBindingGuard_DirectDescribeMatchesHiddenTools(t *testing.T) { + proxy, _ := bindingGuardTestProxy(t, "", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") + tool := &config.ToolMetadata{ + ServerName: "a", Name: "read_tool", ParamsJSON: `{"type":"object"}`, + Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(true)}, + } + require.NoError(t, proxy.storage.SaveToolApproval(&storage.ToolApprovalRecord{ + ServerName: "a", ToolName: "read_tool", Status: storage.ToolApprovalStatusApproved, + })) + proxy.publishDirectCatalog(buildDirectCatalog([]*config.ToolMetadata{tool}, nil)) + + ctx := anonCtx() + listed := proxy.filterDirectModeToolsForAuth(ctx, []mcp.Tool{directStampedTool("a", "read_tool", "read")}) + require.Empty(t, listed, "the active binding guard hides the direct tool from tools/list") + _, visible := proxy.resolveDirectDescribeID(ctx, "a__read_tool") + require.False(t, visible, "describe_tool must not disclose a tool hidden by the FR-008a guard") + + cat := proxy.loadDirectCatalog() + plan := proxy.planDirectCheck(ctx, cat, nil, []string{"a__read_tool"}) + require.Empty(t, plan.refs, "check:true must not ask the evaluator about a guard-hidden tool") + _, gated := plan.gated["a__read_tool"] + require.True(t, gated, "check:true must answer the hidden id as not_found") +} + func TestHandleSetProfile_AnonymousBindingGuardRefusesSelectionButAllowsClear(t *testing.T) { proxy, _ := bindingGuardTestProxy(t, "", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") ctx := sessionCtx(context.Background(), "anonymous-bound-session") @@ -127,6 +152,10 @@ func TestBindingBypassable_FR008aReachabilityMatrix(t *testing.T) { name: "wider tier cap", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, MaxTier: config.ProfileTierRead}, {Name: "Q", Servers: []string{"a"}, MaxTier: config.ProfileTierWrite}}, want: true, }, + { + name: "unset cap and destructive cap admit the same tier set", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, + profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, MaxTier: config.ProfileTierDestructive}, {Name: "Q", Servers: []string{"a"}}}, + }, { name: "more permissive unannotated handling", anonymous: "Q", pin: "P", mode: auth.ProfileModeLocked, profiles: []config.ProfileConfig{{Name: "P", Servers: []string{"a"}, Unannotated: config.ProfileUnannotatedDeny}, {Name: "Q", Servers: []string{"a"}, Unannotated: config.ProfileUnannotatedAsRead}}, want: true, From b994a0bd8cf50acc30c2de97ac9f1ce3c6d6b58c Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 01:20:01 +0300 Subject: [PATCH 06/11] perf(profile): cache direct request scope --- internal/server/mcp_describe_direct.go | 49 ++++++++++++++----- internal/server/mcp_describe_tool.go | 3 ++ internal/server/profile_binding_guard.go | 7 ++- internal/server/profile_binding_guard_test.go | 10 ++++ 4 files changed, 56 insertions(+), 13 deletions(-) diff --git a/internal/server/mcp_describe_direct.go b/internal/server/mcp_describe_direct.go index 6bec7f0c9..18bfc9a4e 100644 --- a/internal/server/mcp_describe_direct.go +++ b/internal/server/mcp_describe_direct.go @@ -10,6 +10,39 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/toolannotations" ) +type directProfileViewContextKey struct{} + +type directProfileView struct { + name string + scope *profile.ProfileScope + index *profileIndex +} + +// cacheDirectProfileView pins the direct surface's profile decision once per +// describe_tool request. The direct resolver checks multiple catalog entries +// for visibility and suggestions; recomputing the anonymous binding guard for +// each entry would repeat a token-store read and a full live-tool comparison. +func (p *MCPProxyServer) cacheDirectProfileView(ctx context.Context) context.Context { + if _, ok := ctx.Value(directProfileViewContextKey{}).(directProfileView); ok { + return ctx + } + name, scope, idx := p.resolveActiveProfileWithIndex(ctx) + if anonymousProfileCaller(ctx) { + resolution := p.ResolveProfileV3(ctx, idx) + name, scope = resolution.Name, resolution.Scope + } + return context.WithValue(ctx, directProfileViewContextKey{}, directProfileView{name: name, scope: scope, index: idx}) +} + +func (p *MCPProxyServer) directProfileViewFor(ctx context.Context) directProfileView { + if view, ok := ctx.Value(directProfileViewContextKey{}).(directProfileView); ok { + return view + } + ctx = p.cacheDirectProfileView(ctx) + view, _ := ctx.Value(directProfileViewContextKey{}).(directProfileView) + return view +} + // Spec 102 US2 — describe_tool's id resolver for the DIRECT surface. // // Every other surface resolves describe_tool ids through toolVisibleToSession, @@ -155,18 +188,10 @@ func (p *MCPProxyServer) resolveDirectDescribeIDIn(ctx context.Context, cat *dir // operation-permission tier, and agent callability. func (p *MCPProxyServer) directEntryVisibleToSession(ctx context.Context, entry *directCatalogEntry) bool { authCtx := auth.AuthContextFromContext(ctx) - profileName, profileScope, profileIdx := p.resolveActiveProfileWithIndex(ctx) - if anonymousProfileCaller(ctx) { - // The legacy resolver has no anonymous-profile tier. Direct-mode - // describe and check must use the same V3 resolution as direct tools/list - // so FR-008a's deny-all guard cannot be bypassed by asking for a schema. - resolution := p.ResolveProfileV3(ctx, profileIdx) - profileName = resolution.Name - profileScope = resolution.Scope - } + view := p.directProfileViewFor(ctx) isScopedAgent := isScopeRestrictedCaller(authCtx) - if !directEntryInScope(authCtx, profileScope, isScopedAgent, entry) { + if !directEntryInScope(authCtx, view.scope, isScopedAgent, entry) { return false } if !p.directEntryCallable(authCtx, entry) { @@ -177,8 +202,8 @@ func (p *MCPProxyServer) directEntryVisibleToSession(ctx context.Context, entry // describe-only (never consulted by the actual dispatch path), so unlike // the list filter it needs no call-time exception — describe always // applies the policy in full. - if profileName != "" { - if policy := profileIdx.PolicyFor(profileName); policy != nil { + if view.name != "" && view.index != nil { + if policy := view.index.PolicyFor(view.name); policy != nil { annotations, found := p.EffectiveAnnotations(entry.ServerName, entry.ToolName) intrinsic := profile.IntrinsicTier(annotations, found) if admitted, _, _ := policy.Decide(entry.ServerName, entry.ToolName, intrinsic); !admitted { diff --git a/internal/server/mcp_describe_tool.go b/internal/server/mcp_describe_tool.go index 71e1adae5..47b2bdaff 100644 --- a/internal/server/mcp_describe_tool.go +++ b/internal/server/mcp_describe_tool.go @@ -375,6 +375,9 @@ func (p *MCPProxyServer) handleDescribeToolOnSurface(ctx context.Context, reques if err != nil { return mcp.NewToolResultError(err.Error()), nil } + if surface == describeSurfaceDirect && anonymousProfileCaller(ctx) { + ctx = p.cacheDirectProfileView(ctx) + } if mode.check { return p.handleDescribeToolCheck(ctx, request, mode, surface, sessionID, requestID) } diff --git a/internal/server/profile_binding_guard.go b/internal/server/profile_binding_guard.go index ad7c1bb08..e7692252f 100644 --- a/internal/server/profile_binding_guard.go +++ b/internal/server/profile_binding_guard.go @@ -52,13 +52,18 @@ func (p *MCPProxyServer) bindingGuardActive(idx *profileIndex) bool { } return false } - tools := p.bindingGuardTools() now := time.Now() + var tools []bindingGuardTool + toolsLoaded := false for i := range tokens { token := &tokens[i] if !activeNamedClientBinding(token, now) { continue } + if !toolsLoaded { + tools = p.bindingGuardTools() + toolsLoaded = true + } if bindingBypassable(idx, cfg, token, tools) { return true } diff --git a/internal/server/profile_binding_guard_test.go b/internal/server/profile_binding_guard_test.go index 9061d9925..3f342b93c 100644 --- a/internal/server/profile_binding_guard_test.go +++ b/internal/server/profile_binding_guard_test.go @@ -100,6 +100,16 @@ func TestBindingGuard_DirectDescribeMatchesHiddenTools(t *testing.T) { require.True(t, gated, "check:true must answer the hidden id as not_found") } +func TestCacheDirectProfileView_ReusesAnonymousGuardDecision(t *testing.T) { + proxy, _ := bindingGuardTestProxy(t, "", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") + ctx := proxy.cacheDirectProfileView(anonCtx()) + view, ok := ctx.Value(directProfileViewContextKey{}).(directProfileView) + require.True(t, ok) + require.NotNil(t, view.scope) + require.True(t, view.scope.DeniesAll(), "the cached profile view must preserve the active anonymous guard") + require.Same(t, ctx, proxy.cacheDirectProfileView(ctx), "one direct describe request reuses its profile decision") +} + func TestHandleSetProfile_AnonymousBindingGuardRefusesSelectionButAllowsClear(t *testing.T) { proxy, _ := bindingGuardTestProxy(t, "", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") ctx := sessionCtx(context.Background(), "anonymous-bound-session") From 6d577418069850223f2b7b01f621e8364dc05c80 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 02:25:40 +0300 Subject: [PATCH 07/11] fix(profile): honor switchable profile targets (Spec 108-d) --- internal/server/profile_binding_guard.go | 12 ++--- internal/server/profile_binding_guard_test.go | 17 +++++++ .../server/profile_pin_enforcement_test.go | 2 + .../server/profile_rest_discovery_test.go | 38 +++++++++++---- internal/server/profile_tool.go | 32 +++++++++---- internal/server/scoped_view_guard_test.go | 46 ++++++++++++++++++- internal/server/set_profile_v3_test.go | 46 +++++++++++++++++++ 7 files changed, 167 insertions(+), 26 deletions(-) diff --git a/internal/server/profile_binding_guard.go b/internal/server/profile_binding_guard.go index e7692252f..0f8b8a11f 100644 --- a/internal/server/profile_binding_guard.go +++ b/internal/server/profile_binding_guard.go @@ -1,7 +1,6 @@ package server import ( - "strings" "time" "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" @@ -237,14 +236,13 @@ func (p *MCPProxyServer) bindingGuardTools() []bindingGuardTool { continue } for _, info := range status.Tools { - toolName := info.Name - if prefix := serverName + ":"; strings.HasPrefix(toolName, prefix) { - toolName = strings.TrimPrefix(toolName, prefix) - } - annotations, found := p.EffectiveAnnotations(serverName, toolName) + // StateView stores the upstream's raw tool name. A raw name may + // itself start with ":"; keep it intact so the guard + // compares the same exact registration identity as dispatch. + annotations, found := p.EffectiveAnnotations(serverName, info.Name) tools = append(tools, bindingGuardTool{ server: serverName, - tool: toolName, + tool: info.Name, tier: profile.IntrinsicTier(annotations, found), }) } diff --git a/internal/server/profile_binding_guard_test.go b/internal/server/profile_binding_guard_test.go index 3f342b93c..29e9db204 100644 --- a/internal/server/profile_binding_guard_test.go +++ b/internal/server/profile_binding_guard_test.go @@ -65,6 +65,23 @@ func TestResolveProfileV3_AnonymousBindingGuardUsesCurrentToolPolicy(t *testing. require.False(t, got.Scope.Allows("a"), "anonymous Q admits a live tool denied by P, so the binding guard must deny the whole anonymous profile") } +func TestResolveProfileV3_AnonymousBindingGuardPreservesRawPrefixedToolName(t *testing.T) { + proxy, idx := bindingGuardTestProxy(t, "Q", []config.ProfileConfig{ + {Name: "P", Servers: []string{"a"}, Tools: &config.ProfileToolRules{Deny: []string{"a:a:erase"}}}, + {Name: "Q", Servers: []string{"a"}}, + }, auth.ProfileModeLocked, "P") + proxy.mainServer.runtime.Supervisor().StateView().UpdateServer("a", func(status *stateview.ServerStatus) { + status.ToolsDiscovered = true + status.Tools = []stateview.ToolInfo{{Name: "a:erase", Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(false)}}} + }) + + got := proxy.ResolveProfileV3(context.Background(), idx) + + require.True(t, got.BindingGuarded, "the binding denies the raw tool identity a:erase while anonymous profile Q admits it") + require.NotNil(t, got.Scope) + require.True(t, got.Scope.DeniesAll(), "a raw tool name with its own server prefix must not bypass the FR-008a guard") +} + func TestResolveProfileV3_EqualAnonymousProfileDoesNotTripBindingGuard(t *testing.T) { proxy, idx := bindingGuardTestProxy(t, "P", []config.ProfileConfig{{Name: "P", Servers: []string{"a"}}}, auth.ProfileModeLocked, "P") diff --git a/internal/server/profile_pin_enforcement_test.go b/internal/server/profile_pin_enforcement_test.go index 4c4389585..b4e0c73f6 100644 --- a/internal/server/profile_pin_enforcement_test.go +++ b/internal/server/profile_pin_enforcement_test.go @@ -14,6 +14,7 @@ import ( "github.com/smart-mcp-proxy/mcpproxy-go/internal/auth" "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/jsruntime" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" ) // A profile pin is only a security boundary if EVERY dispatch surface resolves @@ -168,6 +169,7 @@ func TestSetProfileClearReportsPinnedScope(t *testing.T) { require.False(t, result.IsError) payload := decodeSetProfilePayload(t, result) assert.Equal(t, "", payload["active_profile"], "the pin is not a stored selection") + assert.Equal(t, string(profile.SourceNone), payload["profile_source"], "clearing hides the credential's base profile") assert.Equal(t, "", proxy.sessionStore.GetActiveProfile("sess-pin-clear"), "the stored selection must be cleared") assert.Equal(t, []interface{}{"research-srv"}, payload["servers"], "clearing must not advertise servers the pin still denies") diff --git a/internal/server/profile_rest_discovery_test.go b/internal/server/profile_rest_discovery_test.go index 5632e1be7..f2be1e021 100644 --- a/internal/server/profile_rest_discovery_test.go +++ b/internal/server/profile_rest_discovery_test.go @@ -1,28 +1,50 @@ package server import ( + "strings" "testing" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/stretchr/testify/require" ) func TestSearchToolsForProfileFiltersBeforeLimit(t *testing.T) { - proxy, rt := newProfilesV3Fixture(t) - indexEnforcementMatrixFixtureTools(t, proxy) + proxy, rt := createTestProxyWithRuntimeCfg(t, nil, func(cfg *config.Config) { + cfg.Servers = []*config.ServerConfig{{Name: "a", Enabled: true}} + cfg.Profiles = []config.ProfileConfig{{Name: "cap-read", Servers: []string{"a"}, MaxTier: config.ProfileTierRead}} + }) + startCountingUpstream(t, proxy, rt, "a", + toolSpec{Name: "keep", Description: "widget status check", Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(true)}}, + toolSpec{Name: "many", Description: strings.Repeat("widget ", 10), Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(false)}}, + ) + require.NoError(t, proxy.index.IndexTool(&config.ToolMetadata{ + Name: "a:keep", ServerName: "a", Description: "widget status check", ParamsJSON: "{}", + Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(true)}, + })) + require.NoError(t, proxy.index.IndexTool(&config.ToolMetadata{ + Name: "a:many", ServerName: "a", Description: strings.Repeat("widget ", 10), ParamsJSON: "{}", + Annotations: &config.ToolAnnotations{ReadOnlyHint: boolPtr(false)}, + })) + require.NoError(t, proxy.index.RebuildProfileFromShared("cap-read", []string{"a"})) + + // Control: the raw unprofiled rank must put the write tool in the only slot. + raw, err := proxy.index.SearchTools("widget", 1) + require.NoError(t, err) + require.Len(t, raw, 1) + require.Equal(t, "a:many", raw[0].Tool.Name, "fixture: write tool must outrank the read tool") + mainServer := &Server{runtime: rt, mcpProxy: proxy} - rows, handled, err := mainServer.SearchToolsForProfile(urlProfileCtx(proxy, "work-readonly"), "list_issues", 1, nil) + rows, handled, err := mainServer.SearchToolsForProfile(urlProfileCtx(proxy, "cap-read"), "widget", 1, nil) require.NoError(t, err) require.True(t, handled) require.Len(t, rows, 1, "an excluded high-ranked hit must not consume the only result slot") tool, ok := rows[0]["tool"].(map[string]interface{}) require.True(t, ok) - require.Equal(t, "github:list_issues", tool["name"]) - require.False(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "work-readonly"), "github", "create_issue")) - require.True(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "work-readonly"), "github", "list_issues")) - require.False(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "work-readonly"), "filesystem", "read_text_file")) - require.True(t, mainServer.ToolAllowedByProfile(adminCtx(), "filesystem", "read_text_file"), "an unprofiled admin keeps the existing view") + require.Equal(t, "a:keep", tool["name"]) + require.False(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "cap-read"), "a", "many")) + require.True(t, mainServer.ToolAllowedByProfile(urlProfileCtx(proxy, "cap-read"), "a", "keep")) } func TestProfileRESTDiscoveryPreservesLegacyProfileBehavior(t *testing.T) { diff --git a/internal/server/profile_tool.go b/internal/server/profile_tool.go index 84f48bf4a..e366f6c50 100644 --- a/internal/server/profile_tool.go +++ b/internal/server/profile_tool.go @@ -698,23 +698,37 @@ func (idx *profileIndex) step() { // candidate declares or on how many servers are configured. func (idx *profileIndex) selectable(ctx context.Context, slug string) bool { candidate := idx.position(slug) - pin := profilePinFromContext(ctx) - if pin != "" { - // The pin is the only profile a pinned caller may select; resolve it - // whether or not the URL named it so a mismatch costs what a match does. - pinned := idx.position(pin) + if !idx.pinAllowsSelection(ctx, slug) { candidate = -1 - if slug == pin { - candidate = pinned - } } reach := idx.reach(ctx, candidate) // Administrators (and absent contexts) select any configured profile, // including empty or ghost ones (SC-005); everyone else needs reach. + pin := profilePinFromContext(ctx) needsReach := pin != "" || auth.IsScopedCaller(ctx) return candidate >= 0 && (!needsReach || reach) } +// pinAllowsSelection applies credential-level profile selection limits in +// addition to server reach. Locked clients and legacy pinned agent tokens +// may select only their pin. Switchable client credentials may select their +// bound base or one of that base policy's explicitly declared switchable_to +// targets (FR-022); they never inherit targets from the selected profile. +func (idx *profileIndex) pinAllowsSelection(ctx context.Context, slug string) bool { + if pin, mode, ok := clientCredentialFromContext(ctx); ok && mode == auth.ProfileModeSwitchable { + if pin == "" { + // The built-in "All servers" binding has no base policy; FR-022 + // preserves its legacy any-selectable-profile behavior. + return true + } + return admittedBySwitchableTo(idx.PolicyFor(pin), pin, slug) + } + if pin := profilePinFromContext(ctx); pin != "" { + return slug == pin + } + return true +} + // forEachSelectable visits EVERY configured profile, in configured order, // and reports to visit whether the caller may select it — the same rule as // selectable, applied to each profile. @@ -742,7 +756,7 @@ func (idx *profileIndex) forEachSelectable(ctx context.Context, visit func(name if needsReach { selectable = idx.reach(ctx, i) } - if pin != "" && p.Name != pin { + if !idx.pinAllowsSelection(ctx, p.Name) { selectable = false } visit(p.Name, selectable) diff --git a/internal/server/scoped_view_guard_test.go b/internal/server/scoped_view_guard_test.go index c28ed8779..7675bb323 100644 --- a/internal/server/scoped_view_guard_test.go +++ b/internal/server/scoped_view_guard_test.go @@ -18,6 +18,13 @@ func TestConfinedAnonymousHandlersUseScopedView(t *testing.T) { "handleTailLog": false, "handleCallToolVariant": false, } + adminChecks := map[string]bool{ + "IsAdmin": true, + "IsAdminOrAbsent": true, + "IsNonAdmin": true, + "IsAdministrator": true, + "AuthorizeServerOp": true, + } for _, decl := range file.Decls { fn, ok := decl.(*ast.FuncDecl) if !ok || fn.Body == nil { @@ -27,17 +34,52 @@ func TestConfinedAnonymousHandlersUseScopedView(t *testing.T) { continue } guarded[fn.Name.Name] = true + scopedViewVars := map[string]bool{} + ast.Inspect(fn.Body, func(node ast.Node) bool { + assign, ok := node.(*ast.AssignStmt) + if !ok { + return true + } + for _, rhs := range assign.Rhs { + call, ok := rhs.(*ast.CallExpr) + if !ok { + continue + } + selector, ok := call.Fun.(*ast.SelectorExpr) + if !ok || selector.Sel.Name != "ScopedView" { + continue + } + for _, lhs := range assign.Lhs { + if id, ok := lhs.(*ast.Ident); ok { + scopedViewVars[id.Name] = true + } + } + } + return true + }) + require.NotEmptyf(t, scopedViewVars, "%s must derive an auth context with auth.ScopedView", fn.Name.Name) + adminCheckFound := false ast.Inspect(fn.Body, func(node ast.Node) bool { call, ok := node.(*ast.CallExpr) if !ok { return true } selector, ok := call.Fun.(*ast.SelectorExpr) - if ok && selector.Sel.Name == "IsAdmin" { - t.Errorf("%s must use auth.ScopedView before administrator checks", fn.Name.Name) + if !ok || !adminChecks[selector.Sel.Name] { + return true + } + adminCheckFound = true + if len(call.Args) == 0 { + t.Errorf("%s has an administrator check without an auth context", fn.Name.Name) + return true + } + authArg, ok := call.Args[0].(*ast.Ident) + if !ok || !scopedViewVars[authArg.Name] { + t.Errorf("%s must pass an auth.ScopedView result to %s", fn.Name.Name, selector.Sel.Name) } return true }) + require.Truef(t, adminCheckFound, "%s must retain an administrator check", fn.Name.Name) } for name, found := range guarded { require.Truef(t, found, "missing guarded handler %s", name) diff --git a/internal/server/set_profile_v3_test.go b/internal/server/set_profile_v3_test.go index d2994274e..8e0a01fcf 100644 --- a/internal/server/set_profile_v3_test.go +++ b/internal/server/set_profile_v3_test.go @@ -6,6 +6,8 @@ import ( "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/require" + + "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" ) func TestSetProfileV3ReportsOnlyTheSessionSelection(t *testing.T) { @@ -30,3 +32,47 @@ func TestSetProfileV3ReportsOnlyTheSessionSelection(t *testing.T) { }) } } + +func TestSetProfileV3SwitchableClientCanSelectDeclaredTarget(t *testing.T) { + proxy, _ := newProfilesV3Fixture(t) + idx := proxy.profileIndexFor(proxy.currentConfig()) + + t.Run("switchable client may select its declared one-hop target", func(t *testing.T) { + ctx := sessionCtx(clientCtx("laptop", "work-readonly", "switchable"), "switchable-set-profile") + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"profile": "work-full"} + + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + require.False(t, result.IsError, resultText(t, result)) + var payload map[string]interface{} + require.NoError(t, json.Unmarshal([]byte(resultText(t, result)), &payload)) + require.Equal(t, "work-full", payload["active_profile"]) + require.Equal(t, "session", payload["profile_source"]) + require.Equal(t, "work-full", proxy.sessionStore.GetActiveProfile("switchable-set-profile")) + require.Equal(t, string(profile.SourceSession), proxy.ResolveProfileV3(ctx, idx).Source) + }) + + t.Run("locked client cannot select the same target", func(t *testing.T) { + ctx := sessionCtx(clientCtx("cursor", "work-readonly", "locked"), "locked-set-profile") + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"profile": "work-full"} + + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + require.True(t, result.IsError) + require.Equal(t, "unknown profile 'work-full'", resultText(t, result)) + require.Empty(t, proxy.sessionStore.GetActiveProfile("locked-set-profile")) + }) + + t.Run("switchable All servers binding keeps legacy selectable profiles", func(t *testing.T) { + ctx := sessionCtx(clientCtx("all-servers", "", "switchable"), "all-servers-set-profile") + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"profile": "legacy"} + + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + require.False(t, result.IsError, resultText(t, result)) + require.Equal(t, "legacy", proxy.sessionStore.GetActiveProfile("all-servers-set-profile")) + }) +} From 7ce2961b96e2fe0cc5a3c2926cf5884363a6edf3 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 02:34:44 +0300 Subject: [PATCH 08/11] fix(profile): confine anonymous session profile switching (Spec 108-d) --- internal/server/profile_tool.go | 7 ++++++- internal/server/set_profile_v3_test.go | 16 ++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/internal/server/profile_tool.go b/internal/server/profile_tool.go index e366f6c50..41da8647e 100644 --- a/internal/server/profile_tool.go +++ b/internal/server/profile_tool.go @@ -92,6 +92,7 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT } cfg := profiles.cfg anonymousBindingGuard := anonymousProfileCaller(ctx) && p.bindingGuardActive(profiles) + anonymousProfileConfined := anonymousProfileCaller(ctx) && cfg != nil && cfg.AnonymousProfile != "" if slug != "" && anonymousBindingGuard { // Keep the refusal shape indistinguishable from an unknown profile and // leave the session untouched while the FR-008a runtime guard is active. @@ -125,7 +126,7 @@ func (p *MCPProxyServer) handleSetProfile(ctx context.Context, request mcp.CallT // legitimately enumerate. if slug != "" { if !profiles.selectable(ctx, slug) { - if auth.IsScopedCaller(ctx) { + if auth.IsScopedCaller(ctx) || anonymousProfileConfined { return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s'", slug)), nil } return mcp.NewToolResultError(fmt.Sprintf("unknown profile '%s' (available: %s)", slug, strings.Join(profiles.selectableNames(ctx), ", "))), nil @@ -723,6 +724,10 @@ func (idx *profileIndex) pinAllowsSelection(ctx context.Context, slug string) bo } return admittedBySwitchableTo(idx.PolicyFor(pin), pin, slug) } + if anonymousProfileCaller(ctx) && idx.cfg != nil && idx.cfg.AnonymousProfile != "" { + base := idx.cfg.AnonymousProfile + return admittedBySwitchableTo(idx.PolicyFor(base), base, slug) + } if pin := profilePinFromContext(ctx); pin != "" { return slug == pin } diff --git a/internal/server/set_profile_v3_test.go b/internal/server/set_profile_v3_test.go index 8e0a01fcf..28799efe3 100644 --- a/internal/server/set_profile_v3_test.go +++ b/internal/server/set_profile_v3_test.go @@ -7,6 +7,7 @@ import ( "github.com/mark3labs/mcp-go/mcp" "github.com/stretchr/testify/require" + "github.com/smart-mcp-proxy/mcpproxy-go/internal/config" "github.com/smart-mcp-proxy/mcpproxy-go/internal/profile" ) @@ -76,3 +77,18 @@ func TestSetProfileV3SwitchableClientCanSelectDeclaredTarget(t *testing.T) { require.Equal(t, "legacy", proxy.sessionStore.GetActiveProfile("all-servers-set-profile")) }) } + +func TestSetProfileV3ConfinedAnonymousHonorsSwitchableTo(t *testing.T) { + proxy, _ := newProfilesV3FixtureWithConfig(t, func(cfg *config.Config) { + cfg.AnonymousProfile = "work-readonly" + }) + ctx := sessionCtx(anonCtx(), "confined-anonymous-set-profile") + request := mcp.CallToolRequest{} + request.Params.Arguments = map[string]interface{}{"profile": "legacy"} + + result, err := proxy.handleSetProfile(ctx, request) + require.NoError(t, err) + require.True(t, result.IsError) + require.Equal(t, "unknown profile 'legacy'", resultText(t, result)) + require.Empty(t, proxy.sessionStore.GetActiveProfile("confined-anonymous-set-profile")) +} From 827e189ea5835fc61962c3256c6efb14506df64b Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 02:46:09 +0300 Subject: [PATCH 09/11] fix(profile): gate anonymous profile URLs (Spec 108-d) --- internal/server/profile_url_gate_test.go | 32 ++++++++++++++++++++++++ internal/server/server.go | 7 +++--- 2 files changed, 36 insertions(+), 3 deletions(-) diff --git a/internal/server/profile_url_gate_test.go b/internal/server/profile_url_gate_test.go index 62f88535e..d0c5737a7 100644 --- a/internal/server/profile_url_gate_test.go +++ b/internal/server/profile_url_gate_test.go @@ -95,6 +95,38 @@ func TestProfileMiddleware_ScopedRefusalIsLoggedForOperator(t *testing.T) { require.Empty(t, logs.FilterMessage("profile URL refused for scoped caller").All()) } +func TestProfileURLGate_ConfinedAnonymousHonorsSwitchableTo(t *testing.T) { + switchTo := []string{"target"} + cfg := &config.Config{ + AnonymousProfile: "base", + Servers: []*config.ServerConfig{{Name: "base-srv"}, {Name: "target-srv"}, {Name: "other-srv"}}, + Profiles: []config.ProfileConfig{ + {Name: "base", Servers: []string{"base-srv"}, SwitchableTo: &switchTo}, + {Name: "target", Servers: []string{"target-srv"}}, + {Name: "other", Servers: []string{"other-srv"}}, + }, + } + handler := (profileGateFleet{srv: &Server{logger: zap.NewNop()}, cfg: cfg}).handler(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(http.StatusOK) + })) + + for _, tc := range []struct { + name, path string + wantStatus int + }{ + {name: "bound base remains selectable", path: "/mcp/p/base", wantStatus: http.StatusOK}, + {name: "declared switchable target is selectable", path: "/mcp/p/target", wantStatus: http.StatusOK}, + {name: "undeclared target is refused", path: "/mcp/p/other", wantStatus: http.StatusNotFound}, + } { + t.Run(tc.name, func(t *testing.T) { + req := httptest.NewRequest(http.MethodPost, tc.path, http.NoBody) + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + require.Equal(t, tc.wantStatus, rec.Code) + }) + } +} + // profileGateFleetConfig builds a config over a fleet of 1+n profiles: "pin" // (reaching "pin-srv") followed by n profiles "p0".."p" that reach only // "other-srv". With n == -1 the fleet has no profiles at all. hidden further diff --git a/internal/server/server.go b/internal/server/server.go index 442721693..668f95fb5 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -2572,16 +2572,17 @@ func (s *Server) serveProfileURL(w http.ResponseWriter, r *http.Request, profile profiles = s.profileIndexes.For(s.runtimeConfig()) } cfg := profiles.cfg + confinedAnonymous := anonymousProfileCaller(r.Context()) && cfg != nil && cfg.AnonymousProfile != "" // One slug → profile index per snapshot (built before the snapshot was // published, see warmProfileIndex): the gate below and the lookup after // it resolve the slug directly, so neither the refusal nor the admission // walks cfg.Profiles. - // Spec 105 FR-004: the selectable-profile gate for scoped callers. It + // Spec 105 FR-004: the selectable-profile gate for confined callers. It // evaluates the requested profile (and the pin) ONLY — never the // selectable list, whose cost is fleet-sized (profileIndex.selectable). - if auth.IsScopedCaller(r.Context()) { + if auth.IsScopedCaller(r.Context()) || confinedAnonymous { if !profiles.selectable(r.Context(), slug) { // Silent towards the agent, not towards the operator: the gate // answers before the logging handler mounted inside it, so this @@ -2607,7 +2608,7 @@ func (s *Server) serveProfileURL(w http.ResponseWriter, r *http.Request, profile return } - // Look up profile by slug (lock-free snapshot). A scoped caller that + // Look up profile by slug (lock-free snapshot). A confined caller that // passed the gate always resolves here — the predicate only admits // configured profiles. The position is kept, not just the *ProfileConfig, // so the effective-server computation below can reuse this exact From cd5302d008c55ae7ca4a2ee06ae302cc09889d0e Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 03:15:19 +0300 Subject: [PATCH 10/11] test(ui): use converged activity snapshot --- e2e/web-ui-sweep/web-ui-sweep.spec.ts | 30 ++++++++++++++++----------- 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/e2e/web-ui-sweep/web-ui-sweep.spec.ts b/e2e/web-ui-sweep/web-ui-sweep.spec.ts index e89af10e6..72ff455c6 100644 --- a/e2e/web-ui-sweep/web-ui-sweep.spec.ts +++ b/e2e/web-ui-sweep/web-ui-sweep.spec.ts @@ -149,26 +149,22 @@ test('Activity and Usage report the same 24h numbers as the API', async ({ page, return { summary: s.data ?? {}, usage: u.data ?? {} } } - const { summary, usage } = await read() - const total = Number(summary.total_count ?? 0) - const calls = Number(summary.call_count ?? 0) - - if (total === 0) { - test.skip(true, 'no activity in the last 24h on this instance') - } - // The API-level halves of the invariant. `usage` is served from a cached - // snapshot while `summary` counts live, so let them converge rather than - // demanding they agree on the first read. + // snapshot while `summary` counts live, so keep the exact summary/usage pair + // from the successful convergence poll. A pre-poll summary can be stale: on + // a fresh instance, the startup activity may be recorded after the first API + // read but before Activity renders, making the UI look one event ahead. + let converged: Awaited> | undefined await expect .poll( async () => { const now = await read() - return ( + const matches = Number(now.usage.total_calls ?? -1) === Number(now.summary.call_count ?? -2) && Number(now.usage.total_errors ?? -1) === Number(now.summary.error_count ?? 0) + Number(now.summary.blocked_count ?? 0) - ) + if (matches) converged = now + return matches }, { timeout: 20_000, @@ -179,6 +175,16 @@ test('Activity and Usage report the same 24h numbers as the API', async ({ page, ) .toBe(true) + // Use the summary that actually passed the API consistency check above, not + // the earlier startup snapshot. + const { summary } = converged! + const total = Number(summary.total_count ?? 0) + const calls = Number(summary.call_count ?? 0) + + if (total === 0) { + test.skip(true, 'no activity in the last 24h on this instance') + } + // Activity paints the summary's own fields, and labels the total for what it // is: "events" when some rows are not calls, "calls" when every row is one. const errors = watchPageErrors(page) From 066a20c7c8189aeaebac793ea07967a63ee70192 Mon Sep 17 00:00:00 2001 From: Algis Dumbris Date: Tue, 29 Sep 2026 06:18:30 +0300 Subject: [PATCH 11/11] fix(profile): enforce anonymous URL confinement --- .../httpapi/profile_rest_discovery_test.go | 57 +++++++++++++++++++ internal/httpapi/profile_tools.go | 10 ++-- internal/server/profile_url_gate_test.go | 28 +++++++++ internal/server/server.go | 21 ++++++- 4 files changed, 109 insertions(+), 7 deletions(-) diff --git a/internal/httpapi/profile_rest_discovery_test.go b/internal/httpapi/profile_rest_discovery_test.go index 50cde339b..97c01ff7a 100644 --- a/internal/httpapi/profile_rest_discovery_test.go +++ b/internal/httpapi/profile_rest_discovery_test.go @@ -202,6 +202,63 @@ func TestRESTDiscovery_ProfileFiltersServerToolsAndExportAndHidesDiff(t *testing }) } +// StateView tool names are upstream raw identities. In particular, a raw +// name may itself begin with its server name; REST inventory must ask the +// profile policy about that full raw name rather than stripping a prefix. +func TestRESTDiscovery_ProfilePreservesRawPrefixedToolNames(t *testing.T) { + controller := &profileRESTDiscoveryController{ + globalToolsController: &globalToolsController{ + allServers: []map[string]interface{}{{"name": "github", "id": "github"}}, + serverTools: map[string][]map[string]interface{}{ + "github": { + {"name": "github:erase", "server_name": "github", "description": "Raw prefixed destructive tool"}, + {"name": "list_issues", "server_name": "github", "description": "Allowed tool"}, + }, + }, + profileAllowed: map[string]bool{ + "github\x00github:erase": false, + }, + }, + } + controller.managementService = &profileRESTDiscoveryManagementService{controller: controller.globalToolsController} + srv := NewServer(controller, zap.NewNop().Sugar(), nil) + ctx := pinnedDiscoveryContext() + + t.Run("per-server tools", func(t *testing.T) { + req := profileRouteRequest(ctx, http.MethodGet, "/api/v1/servers/github/tools", [2]string{"id", "github"}) + w := httptest.NewRecorder() + srv.handleGetServerTools(w, req) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var payload struct { + Data struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &payload)) + require.Len(t, payload.Data.Tools, 1) + require.Equal(t, "list_issues", payload.Data.Tools[0].Name) + }) + + t.Run("global tools", func(t *testing.T) { + req := httptest.NewRequest(http.MethodGet, "/api/v1/tools", http.NoBody).WithContext(ctx) + w := httptest.NewRecorder() + srv.handleGetGlobalTools(w, req) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var payload struct { + Data struct { + Tools []struct { + Name string `json:"name"` + } `json:"tools"` + } `json:"data"` + } + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &payload)) + require.Len(t, payload.Data.Tools, 1) + require.Equal(t, "list_issues", payload.Data.Tools[0].Name) + }) +} + func TestRESTDiscovery_ProfileSearchUsesPrelimitedProfileResults(t *testing.T) { controller := &globalToolsController{ profileSearchResults: []map[string]interface{}{{ diff --git a/internal/httpapi/profile_tools.go b/internal/httpapi/profile_tools.go index 8e215b5f1..43cf0afb7 100644 --- a/internal/httpapi/profile_tools.go +++ b/internal/httpapi/profile_tools.go @@ -22,12 +22,10 @@ func filterProfileToolRows(controller ServerController, ctx context.Context, ser if name == "" || rowServer == "" { continue } - toolName := name - prefix := rowServer + ":" - if len(toolName) > len(prefix) && toolName[:len(prefix)] == prefix { - toolName = toolName[len(prefix):] - } - if profileController.ToolAllowedByProfile(ctx, rowServer, toolName) { + // StateView names are raw upstream registration identities. A raw name + // may itself start with ":"; preserve it for the policy check + // so a rule such as github:github:erase is not bypassed as erase. + if profileController.ToolAllowedByProfile(ctx, rowServer, name) { filtered = append(filtered, row) } } diff --git a/internal/server/profile_url_gate_test.go b/internal/server/profile_url_gate_test.go index d0c5737a7..588735892 100644 --- a/internal/server/profile_url_gate_test.go +++ b/internal/server/profile_url_gate_test.go @@ -127,6 +127,34 @@ func TestProfileURLGate_ConfinedAnonymousHonorsSwitchableTo(t *testing.T) { } } +// A locked client binding makes the otherwise-unprofiled anonymous identity +// deny-all. The URL gate must carry that confinement through as well: both an +// existing profile and a nonexistent one receive the same refusal before the +// MCP handler, so the URL cannot be used to inventory profile names. +func TestProfileURLGate_BindingGuardedAnonymousRefusesExistingAndMissingSlugs(t *testing.T) { + proxy, profiles := bindingGuardTestProxy(t, "", []config.ProfileConfig{ + {Name: "locked", Servers: []string{"a"}}, + }, auth.ProfileModeLocked, "locked") + srv := proxy.mainServer + srv.logger = zap.NewNop() + srv.mcpProxy = proxy + require.True(t, proxy.ResolveProfileV3(context.Background(), profiles).BindingGuarded) + + reached := false + handler := http.HandlerFunc(func(_ http.ResponseWriter, _ *http.Request) { + reached = true + }) + for _, slug := range []string{"locked", "does-not-exist"} { + req := httptest.NewRequest(http.MethodPost, "/mcp/p/"+slug, http.NoBody) + rec := httptest.NewRecorder() + srv.serveProfileURL(rec, req, profiles, handler) + require.Equal(t, http.StatusNotFound, rec.Code, slug) + require.JSONEq(t, fmt.Sprintf(`{"error":"unknown profile '%s'"}`, slug), rec.Body.String()) + } + + require.False(t, reached, "a binding-guarded anonymous request must not reach MCP") +} + // profileGateFleetConfig builds a config over a fleet of 1+n profiles: "pin" // (reaching "pin-srv") followed by n profiles "p0".."p" that reach only // "other-srv". With n == -1 the fleet has no profiles at all. hidden further diff --git a/internal/server/server.go b/internal/server/server.go index 668f95fb5..b093c253b 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -2544,6 +2544,11 @@ func (s *Server) serveProfileURL(w http.ResponseWriter, r *http.Request, profile slug := strings.TrimPrefix(r.URL.Path, "/mcp/p/") slug = strings.TrimPrefix(slug, "/mcp/p") // handle /mcp/p with no trailing slash slug = strings.Trim(slug, "/") + // An active client binding can turn an otherwise unprofiled anonymous + // caller into a deny-all BindingGuarded resolution. Compute it before the + // publication-gap branch too, so that branch cannot restore URL probing. + bindingGuardedAnonymous := anonymousProfileCaller(r.Context()) && s.mcpProxy != nil && + s.mcpProxy.ResolveProfileV3(r.Context(), profiles).BindingGuarded if profiles == nil { // Acquire could not pair this request's own runtime.Config() read @@ -2557,7 +2562,7 @@ func (s *Server) serveProfileURL(w http.ResponseWriter, r *http.Request, profile // (Spec 105 PR D review round 11, MUST-FIX). An administrator-shaped // caller is not timing-contract-bound (SC-005) and falls back to a // fresh build over the live config, matching pre-105 behaviour. - if auth.IsScopedCaller(r.Context()) { + if auth.IsScopedCaller(r.Context()) || bindingGuardedAnonymous { var agentName string if ac := auth.AuthContextFromContext(r.Context()); ac != nil { agentName = ac.AgentName @@ -2570,8 +2575,22 @@ func (s *Server) serveProfileURL(w http.ResponseWriter, r *http.Request, profile return } profiles = s.profileIndexes.For(s.runtimeConfig()) + bindingGuardedAnonymous = anonymousProfileCaller(r.Context()) && s.mcpProxy != nil && + s.mcpProxy.ResolveProfileV3(r.Context(), profiles).BindingGuarded } cfg := profiles.cfg + if bindingGuardedAnonymous { + // ResolveProfileV3 represents this anonymous request with a deny-all + // scope. selectable intentionally treats administrator-shaped callers + // as selectable, so applying it here would re-open the URL inventory. + // Refuse before the slug lookup, exactly like every other inaccessible + // profile outcome. + s.logger.Info("profile URL refused for scoped caller", + zap.String("profile", slug), + zap.String("remote_addr", r.RemoteAddr)) + profileNotSelectable(w, slug) + return + } confinedAnonymous := anonymousProfileCaller(r.Context()) && cfg != nil && cfg.AnonymousProfile != "" // One slug → profile index per snapshot (built before the snapshot was