Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions src/modules/mcp/services/redisTransport.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -297,6 +297,52 @@ describe('Redis Transport', () => {
expect(await validateSessionOwnership(sessionId, 'different-user')).toBe(false);
});

it('should set the ownership key with a TTL', async () => {
const setSpy = jest.spyOn(mockRedis, 'set');

await setSessionOwner(sessionId, userId);

expect(setSpy).toHaveBeenCalledWith(
`session:${sessionId}:owner`,
userId,
expect.objectContaining({ EX: expect.any(Number) })
);
});

it('should refresh the ownership TTL on successful validation only', async () => {
const expireSpy = jest.spyOn(mockRedis, 'expire');
await setSessionOwner(sessionId, userId);

expect(await validateSessionOwnership(sessionId, userId)).toBe(true);
expect(expireSpy).toHaveBeenCalledWith(`session:${sessionId}:owner`, expect.any(Number));

expireSpy.mockClear();
expect(await validateSessionOwnership(sessionId, 'different-user')).toBe(false);
expect(expireSpy).not.toHaveBeenCalled();
});

it('should remove the ownership key when the server transport closes', async () => {
await setSessionOwner(sessionId, userId);
expect(await getSessionOwner(sessionId)).toBe(userId);

const transport = new ServerRedisTransport(sessionId);
await transport.start();
await transport.close();

expect(await getSessionOwner(sessionId)).toBeNull();
});

it('should remove the ownership key when the session is shut down via control message', async () => {
await setSessionOwner(sessionId, userId);

const transport = new ServerRedisTransport(sessionId);
await transport.start();
await shutdownSession(sessionId);
await new Promise(resolve => setTimeout(resolve, 10));

expect(await getSessionOwner(sessionId)).toBeNull();
});

it('should check if session is owned by user including liveness', async () => {
// Session not live yet
expect(await isSessionOwnedBy(sessionId, userId)).toBe(false);
Expand Down
31 changes: 27 additions & 4 deletions src/modules/mcp/services/redisTransport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import { logger } from "../../shared/logger.js";
let redisTransportCounter = 0;
const notificationStreamId = "__GET_stream";

// Safety-net TTL for session ownership keys. Live sessions refresh it on every
// authorized request, and ServerRedisTransport.close() deletes the key, so the
// TTL only ever fires for keys orphaned by a crashed process.
const SESSION_OWNER_TTL_SECONDS = 30 * 60;

// Message types for Redis transport
type RedisMessage =
| {
Expand Down Expand Up @@ -69,18 +74,33 @@ export async function isLive(sessionId: string): Promise<boolean> {
return numSubs > 0;
}

function getSessionOwnerKey(sessionId: string): string {
return `session:${sessionId}:owner`;
}

export async function setSessionOwner(sessionId: string, userId: string): Promise<void> {
logger.debug('Setting session owner', { sessionId, userId });
await redisClient.set(`session:${sessionId}:owner`, userId);
await redisClient.set(getSessionOwnerKey(sessionId), userId, { EX: SESSION_OWNER_TTL_SECONDS });
}

export async function getSessionOwner(sessionId: string): Promise<string | null> {
return await redisClient.get(`session:${sessionId}:owner`);
return await redisClient.get(getSessionOwnerKey(sessionId));
}

export async function deleteSessionOwner(sessionId: string): Promise<void> {
logger.debug('Deleting session owner', { sessionId });
await redisClient.del(getSessionOwnerKey(sessionId));
}

export async function validateSessionOwnership(sessionId: string, userId: string): Promise<boolean> {
const owner = await getSessionOwner(sessionId);
return owner === userId;
if (owner !== userId) {
return false;
}
// Sliding expiration: each authorized request keeps the ownership key alive,
// so the TTL only reaps keys whose session never got cleaned up.
await redisClient.expire(getSessionOwnerKey(sessionId), SESSION_OWNER_TTL_SECONDS);
return true;
}

export async function isSessionOwnedBy(sessionId: string, userId: string): Promise<boolean> {
Expand Down Expand Up @@ -325,7 +345,10 @@ export class ServerRedisTransport implements Transport {
await this.controlCleanup();
this.controlCleanup = undefined;
}


// The session is finished — remove its ownership key (#21)
await deleteSessionOwner(this._sessionId);

this.onclose?.();
}
}
Expand Down