fix(security): isToolCallable fail-closed on non-not-found storage errors

The new isToolCallable check returned true (callable) for any non-nil
GetToolApproval error other than the new ErrToolApprovalNotFound
sentinel. A transient BBolt mmap-remap during compaction, a decode
failure, or any other read error would silently re-enable a tool the
operator had previously persisted as Disabled=true — the inverse of
the Spec 032 rug-pull bypass that setToolEnabledNoEmit already guards
against on the write side.

Mirror that pattern on the read side:

  switch {
  case err == nil:
      if approval != nil && approval.Disabled { return false }
  case errors.Is(err, storage.ErrToolApprovalNotFound):
      // no record → default callable (matches prior fallthrough)
  default:
      // real storage error → fail closed and log
      return false
  }

The "no record → callable" branch is preserved so the synthesis path
for never-toggled tools still works (writes happen lazily on first
toggle; reads before any toggle should still expose the tool by
default). A WARN log fires on the fail-closed path so operators can
correlate storage health with sudden tool unavailability.

Regression test: TestIsToolCallable_FailsClosedOnStorageError writes
corrupt non-JSON bytes directly into the tool_approvals bucket via the
storage manager's GetDB hook, then asserts isToolCallable returns
false. Exercises the real json.Unmarshal error path, not a mocked
storage layer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: electrolobzik

internal/server/mcp.go

@@ -4575,7 +4575,21 @@ func (p *MCPProxyServer) isToolCallable(serverName, toolName string) bool {
 	}
 
 	approval, err := p.storage.GetToolApproval(serverName, toolName)
-	if err == nil && approval != nil && approval.Disabled {
+	switch {
+	case err == nil:
+		if approval != nil && approval.Disabled {
+			return false
+		}
+	case errors.Is(err, storage.ErrToolApprovalNotFound):
+		// no record → use the implicit default (enabled / callable)
+	default:
+		// real storage error — fail closed to avoid silently re-enabling a
+		// tool the user disabled. A transient BBolt mmap-remap during compaction
+		// should not cause a Disabled=true record to be ignored.
+		p.logger.Warn("isToolCallable: storage error treated as not-callable",
+			zap.String("server", serverName),
+			zap.String("tool", toolName),
+			zap.Error(err))
 		return false
 	}
 

internal/server/mcp_blocked_tools_test.go

@@ -8,6 +8,7 @@ import (
 	"github.com/mark3labs/mcp-go/mcp"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+	"go.etcd.io/bbolt"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/contracts"
@@ -148,3 +149,42 @@ func TestReenableTool_VisibleAndCallable(t *testing.T) {
 
 	assert.True(t, proxy.isToolCallable("context7", "resolve-library-id"))
 }
+
+// TestIsToolCallable_FailsClosedOnStorageError verifies that isToolCallable is
+// fail-closed: when GetToolApproval returns a real storage error (not
+// ErrToolApprovalNotFound), the function must return false (not callable) so
+// that a transient I/O error cannot silently re-enable a tool the user disabled.
+//
+// We trigger a real storage error by writing corrupt (non-JSON) bytes directly
+// into the tool_approvals BBolt bucket for the target key. When GetToolApproval
+// reads those bytes and json.Unmarshal fails, it returns a decode error that is
+// NOT wrapped in ErrToolApprovalNotFound — exactly the "real error" case the
+// fix must handle.
+func TestIsToolCallable_FailsClosedOnStorageError(t *testing.T) {
+	proxy := createTestMCPProxyServer(t)
+
+	// Register an enabled server so the early-exit check passes.
+	require.NoError(t, proxy.storage.SaveUpstreamServer(&config.ServerConfig{
+		Name:    "badstore",
+		Enabled: true,
+	}))
+
+	// Write corrupt (non-JSON) bytes directly into the tool_approvals bucket for
+	// the target key. This bypasses the normal SaveToolApproval path and causes
+	// GetToolApproval to return a json.UnmarshalError — a real error that is NOT
+	// ErrToolApprovalNotFound.
+	corruptKey := storage.ToolApprovalKey("badstore", "fragile-tool")
+	err := proxy.storage.GetDB().Update(func(tx *bbolt.Tx) error {
+		b := tx.Bucket([]byte(storage.ToolApprovalBucket))
+		if b == nil {
+			t.Fatal("tool_approvals bucket not found")
+		}
+		return b.Put([]byte(corruptKey), []byte("NOT VALID JSON {{{"))
+	})
+	require.NoError(t, err, "writing corrupt bytes into BBolt must succeed")
+
+	// isToolCallable must return false (fail-closed) rather than true (fail-open)
+	// when it encounters a real storage error reading the approval record.
+	assert.False(t, proxy.isToolCallable("badstore", "fragile-tool"),
+		"isToolCallable must be fail-closed on storage decode errors")
+}