fix(#707): escape session GC scan scope
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
All checks were successful
ci/woodpecker/pr/ci Pipeline was successful
This commit is contained in:
@@ -3,6 +3,7 @@ import { Logger } from '@nestjs/common';
|
|||||||
import type { QueueHandle } from '@mosaicstack/queue';
|
import type { QueueHandle } from '@mosaicstack/queue';
|
||||||
import type { LogService } from '@mosaicstack/log';
|
import type { LogService } from '@mosaicstack/log';
|
||||||
import { SessionGCService } from './session-gc.service.js';
|
import { SessionGCService } from './session-gc.service.js';
|
||||||
|
import { CommandAuthorizationService } from '../commands/command-authorization.service.js';
|
||||||
|
|
||||||
type MockRedis = {
|
type MockRedis = {
|
||||||
scan: ReturnType<typeof vi.fn>;
|
scan: ReturnType<typeof vi.fn>;
|
||||||
@@ -65,11 +66,57 @@ describe('SessionGCService', () => {
|
|||||||
expect(result.cleaned.valkeyKeys).toBeUndefined();
|
expect(result.cleaned.valkeyKeys).toBeUndefined();
|
||||||
});
|
});
|
||||||
|
|
||||||
it('collect() only scans its session namespace and cannot reach durable approvals', async () => {
|
it('escapes glob metacharacters in a session identifier', async () => {
|
||||||
await service.collect('abc');
|
await service.collect('abc*?[tenant]\\escape');
|
||||||
|
|
||||||
expect(mockRedis.scan).toHaveBeenCalledWith('0', 'MATCH', 'mosaic:session:abc:*', 'COUNT', 100);
|
expect(mockRedis.scan).toHaveBeenCalledWith(
|
||||||
expect(mockRedis.del).not.toHaveBeenCalledWith(expect.stringContaining('command-approval'));
|
'0',
|
||||||
|
'MATCH',
|
||||||
|
'mosaic:session:abc\\*\\?\\[tenant\\]\\\\escape:*',
|
||||||
|
'COUNT',
|
||||||
|
100,
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('preserves a valid durable approval after session GC', async () => {
|
||||||
|
const entries = new Map<string, string>();
|
||||||
|
const redis = {
|
||||||
|
scan: vi.fn().mockResolvedValue(['0', ['mosaic:session:owned:state']]),
|
||||||
|
get: vi.fn(async (key: string) => entries.get(key) ?? null),
|
||||||
|
set: vi.fn(async (key: string, value: string) => entries.set(key, value)),
|
||||||
|
del: vi.fn(async (...keys: string[]) => {
|
||||||
|
let deleted = 0;
|
||||||
|
for (const key of keys) deleted += Number(entries.delete(key));
|
||||||
|
return deleted;
|
||||||
|
}),
|
||||||
|
};
|
||||||
|
const authorization = new CommandAuthorizationService(
|
||||||
|
{
|
||||||
|
select: () => ({
|
||||||
|
from: () => ({ where: () => ({ limit: async () => [{ role: 'admin' }] }) }),
|
||||||
|
}),
|
||||||
|
} as never,
|
||||||
|
redis,
|
||||||
|
);
|
||||||
|
const command = {
|
||||||
|
name: 'gc',
|
||||||
|
description: 'System-wide garbage collection',
|
||||||
|
aliases: [],
|
||||||
|
scope: 'admin',
|
||||||
|
execution: 'socket',
|
||||||
|
available: true,
|
||||||
|
} as never;
|
||||||
|
const payload = { command: 'gc', conversationId: 'owned' };
|
||||||
|
const approval = await authorization.createApproval(command, payload, 'admin-1');
|
||||||
|
const approvalKey = `tess:command-approval:${approval!.approvalId}`;
|
||||||
|
const gc = new SessionGCService(redis as never, mockLogService as unknown as LogService);
|
||||||
|
|
||||||
|
await gc.collect('owned');
|
||||||
|
|
||||||
|
expect(entries.has(approvalKey)).toBe(true);
|
||||||
|
await expect(
|
||||||
|
authorization.authorize(command, payload, 'admin-1', approval!.approvalId),
|
||||||
|
).resolves.toEqual({ allowed: true });
|
||||||
});
|
});
|
||||||
|
|
||||||
it('collect() returns sessionId in result', async () => {
|
it('collect() returns sessionId in result', async () => {
|
||||||
|
|||||||
@@ -13,6 +13,11 @@ export interface GCResult {
|
|||||||
};
|
};
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Escape Redis glob metacharacters so a session identifier is always literal. */
|
||||||
|
function escapeRedisGlobLiteral(value: string): string {
|
||||||
|
return value.replace(/[\\*?\[\]]/g, '\\$&');
|
||||||
|
}
|
||||||
|
|
||||||
@Injectable()
|
@Injectable()
|
||||||
export class SessionGCService {
|
export class SessionGCService {
|
||||||
constructor(
|
constructor(
|
||||||
@@ -43,7 +48,7 @@ export class SessionGCService {
|
|||||||
const result: GCResult = { sessionId, cleaned: {} };
|
const result: GCResult = { sessionId, cleaned: {} };
|
||||||
|
|
||||||
// 1. Valkey: delete all session-scoped keys
|
// 1. Valkey: delete all session-scoped keys
|
||||||
const pattern = `mosaic:session:${sessionId}:*`;
|
const pattern = `mosaic:session:${escapeRedisGlobLiteral(sessionId)}:*`;
|
||||||
const valkeyKeys = await this.scanKeys(pattern);
|
const valkeyKeys = await this.scanKeys(pattern);
|
||||||
if (valkeyKeys.length > 0) {
|
if (valkeyKeys.length > 0) {
|
||||||
await this.redis.del(...valkeyKeys);
|
await this.redis.del(...valkeyKeys);
|
||||||
|
|||||||
Reference in New Issue
Block a user