fixes from review

Author: taskbot

pkg/vmcp/server/session_management_v2_integration_test.go

@@ -105,25 +105,50 @@ func newV2FakeFactory(tools []vmcp.Tool) *v2FakeMultiSessionFactory {
 }
 
 func (f *v2FakeMultiSessionFactory) MakeSession(
-	_ context.Context, _ *auth.Identity, _ []*vmcp.Backend,
+	_ context.Context, identity *auth.Identity, _ []*vmcp.Backend,
 ) (vmcpsession.MultiSession, error) {
 	if f.err != nil {
 		return nil, f.err
 	}
 	baseSession := transportsession.NewStreamableSession("auto-id")
+
+	// Set basic metadata to indicate whether this is an anonymous session.
+	// Integration tests don't need to verify crypto implementation details.
+	allowAnonymous := vmcpsession.ShouldAllowAnonymous(identity)
+	if !allowAnonymous {
+		// Authenticated session - set non-empty hash placeholder
+		baseSession.SetMetadata(vmcpsession.MetadataKeyTokenHash, "fake-hash-for-testing")
+		baseSession.SetMetadata(vmcpsession.MetadataKeyTokenSalt, "fake-salt-for-testing")
+	} else {
+		// Anonymous session - set empty hash
+		baseSession.SetMetadata(vmcpsession.MetadataKeyTokenHash, "")
+	}
+
 	sess := newV2FakeMultiSession(baseSession, f.tools)
 	f.lastCreatedSession = sess
 	return sess, nil
 }
 
 func (f *v2FakeMultiSessionFactory) MakeSessionWithID(
-	_ context.Context, id string, _ *auth.Identity, _ bool, _ []*vmcp.Backend,
+	_ context.Context, id string, identity *auth.Identity, allowAnonymous bool, _ []*vmcp.Backend,
 ) (vmcpsession.MultiSession, error) {
 	f.makeWithIDCalled.Store(true)
 	if f.err != nil {
 		return nil, f.err
 	}
 	baseSession := transportsession.NewStreamableSession(id)
+
+	// Set basic metadata to indicate whether this is an anonymous session.
+	// Integration tests don't need to verify crypto implementation details.
+	if identity != nil && identity.Token != "" && !allowAnonymous {
+		// Authenticated session - set non-empty hash placeholder
+		baseSession.SetMetadata(vmcpsession.MetadataKeyTokenHash, "fake-hash-for-testing")
+		baseSession.SetMetadata(vmcpsession.MetadataKeyTokenSalt, "fake-salt-for-testing")
+	} else {
+		// Anonymous session - set empty hash
+		baseSession.SetMetadata(vmcpsession.MetadataKeyTokenHash, "")
+	}
+
 	sess := newV2FakeMultiSession(baseSession, f.tools)
 	f.lastCreatedSession = sess
 	return sess, nil

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,38 @@ type MultiSession interface {
 	// sessions for debugging and auditing.
 	BackendSessions() map[string]string
 }
+
+const (
+	// MetadataKeyTokenHash is the session metadata key that holds the HMAC-SHA256
+	// hash of the bearer token used to create the session. For authenticated sessions
+	// this is hex(HMAC-SHA256(bearerToken)). For anonymous sessions this is the empty
+	// string sentinel. The raw token is never stored — only the hash.
+	//
+	// This constant is used by the session factory and security layer to store and
+	// validate token binding metadata.
+	MetadataKeyTokenHash = "vmcp.token.hash" //nolint:gosec // This is a metadata key name, not a credential.
+
+	// MetadataKeyTokenSalt is the session metadata key that holds the hex-encoded
+	// random salt used for HMAC-SHA256 token hashing. Each session has a unique salt
+	// to prevent attacks across multiple sessions.
+	//
+	// This constant is used by the session factory and security layer to store and
+	// validate token binding metadata.
+	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. This is session business logic that decides
+// whether a session is bound to a specific identity or allows anonymous access.
+//
+// Sessions without an identity (nil) or with an empty token are treated as
+// anonymous and will accept requests from any caller. Sessions with a non-empty
+// bearer token are bound to that token and will reject requests from different
+// callers.
+//
+// This function is used by both the session factory (to determine how to create
+// the session) and the security layer (to validate requests against the session's
+// access policy).
+func ShouldAllowAnonymous(identity *auth.Identity) bool {
+	return identity == nil || identity.Token == ""
+}