fixes from review

Author: taskbot

pkg/vmcp/server/sessionmanager/session_manager.go

@@ -165,7 +165,7 @@ func (sm *Manager) CreateSession(
 	// Build the fully-formed MultiSession using the SDK-assigned session ID.
 	// Sessions created with an identity are bound to that identity (allowAnonymous=false).
 	// Sessions created without an identity allow anonymous access (allowAnonymous=true).
-	allowAnonymous := identity == nil || identity.Token == ""
+	allowAnonymous := vmcpsession.ShouldAllowAnonymous(identity)
 	sess, err := sm.factory.MakeSessionWithID(ctx, sessionID, identity, allowAnonymous, backends)
 	if err != nil {
 		return nil, fmt.Errorf("Manager.CreateSession: failed to create multi-session: %w", err)

pkg/vmcp/session/factory.go

@@ -270,7 +270,7 @@ func (f *defaultMultiSessionFactory) MakeSession(
 ) (MultiSession, error) {
 	// Sessions created with an identity are bound to that identity (allowAnonymous=false).
 	// Sessions created without an identity allow anonymous access (allowAnonymous=true).
-	allowAnonymous := security.ShouldAllowAnonymous(identity)
+	allowAnonymous := ShouldAllowAnonymous(identity)
 	return f.makeSession(ctx, uuid.New().String(), identity, allowAnonymous, backends)
 }
 

pkg/vmcp/session/internal/security/security.go

@@ -40,17 +40,6 @@ const (
 	MetadataKeyTokenSalt = "vmcp.token.salt" //nolint:gosec // This is a metadata key name, not a credential.
 )
 
-// ShouldAllowAnonymous determines if a session should allow anonymous access
-// based on the creator's identity. Sessions without an identity (nil) or with
-// an empty token are anonymous; sessions with a non-empty bearer token are
-// bound to that token.
-//
-// This helper consolidates the anonymous session logic and aligns with the
-// validation logic in PreventSessionHijacking.
-func ShouldAllowAnonymous(identity *auth.Identity) bool {
-	return identity == nil || identity.Token == ""
-}
-
 // GenerateSalt generates a cryptographically secure random salt for token hashing.
 // Returns 16 bytes of random data from crypto/rand.
 //

pkg/vmcp/session/internal/security/security_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"github.com/stacklok/toolhive/pkg/auth"
+	"github.com/stacklok/toolhive/pkg/vmcp/session"
 	"github.com/stacklok/toolhive/pkg/vmcp/session/internal/security"
 )
 
@@ -306,7 +307,7 @@ func TestShouldAllowAnonymous_EdgeCases(t *testing.T) {
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			t.Parallel()
-			got := security.ShouldAllowAnonymous(tt.identity)
+			got := session.ShouldAllowAnonymous(tt.identity)
 			assert.Equal(t, tt.want, got)
 		})
 	}

pkg/vmcp/session/session.go

@@ -4,6 +4,7 @@
 package session
 
 import (
+	"github.com/stacklok/toolhive/pkg/auth"
 	transportsession "github.com/stacklok/toolhive/pkg/transport/session"
 	"github.com/stacklok/toolhive/pkg/vmcp"
 	sessiontypes "github.com/stacklok/toolhive/pkg/vmcp/session/types"
@@ -58,3 +59,14 @@ type MultiSession interface {
 	// sessions for debugging and auditing.
 	BackendSessions() map[string]string
 }
+
+// ShouldAllowAnonymous determines if a session should allow anonymous access
+// based on the creator's identity. Sessions without an identity (nil) or with
+// an empty token are anonymous; sessions with a non-empty bearer token are
+// bound to that token.
+//
+// This function consolidates the anonymous session logic and aligns with the
+// validation logic in session hijacking prevention.
+func ShouldAllowAnonymous(identity *auth.Identity) bool {
+	return identity == nil || identity.Token == ""
+}