diff --git a/package.json b/package.json index a63b14c..8cb8d0a 100644 --- a/package.json +++ b/package.json @@ -51,7 +51,7 @@ "cron-parser": "4.9.0", "cross-env": "^7.0.3", "express": "5.2.1", - "http-proxy-middleware": "^4.1.1", + "http-proxy-middleware": "^3.0.7", "ioredis": "^5.11.1", "lodash": "^4.17.21", "moment": "^2.30.1", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 39c8fe6..58dd757 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -57,8 +57,8 @@ importers: specifier: 5.2.1 version: 5.2.1 http-proxy-middleware: - specifier: ^4.1.1 - version: 4.1.1 + specifier: ^3.0.7 + version: 3.0.7 ioredis: specifier: ^5.11.1 version: 5.11.1 @@ -1304,6 +1304,9 @@ packages: '@types/http-errors@2.0.5': resolution: {integrity: sha512-r8Tayk8HJnX0FztbZN7oVqGccWgw98T/0neJphO91KkmOzug1KkofZURD4UaD5uH8AqcFLfdPErnBod0u71/qg==} + '@types/http-proxy@1.17.17': + resolution: {integrity: sha512-ED6LB+Z1AVylNTu7hdzuBqOgMnvG/ld6wGCG8wFnAzKX5uyW2K3WD52v0gnLCTK/VLpXtKckgWuyScYK6cSPaw==} + '@types/istanbul-lib-coverage@2.0.6': resolution: {integrity: sha512-2QF/t/auWm0lsy8XtKVPG19v3sSOQlJe/YHZgfjb/KBBHOGSV+J2q/S671rcq9uTBrLAXmZpqJiaQbMT+zNU1w==} @@ -2342,6 +2345,9 @@ packages: resolution: {integrity: sha512-i/2XbnSz/uxRCU6+NdVJgKWDTM427+MqYbkQzD321DuCQJUqOuJKIA0IM2+W2xtYHdKOmZ4dR6fExsd4SXL+WQ==} engines: {node: '>=6'} + eventemitter3@4.0.7: + resolution: {integrity: sha512-8guHBZCwKnFhYdHr2ysuRWErTwhoN2X8XELRlrRwpmfeY2jjuUN4taQMsULKUVo1K4DvZl+0pgfyoysHxvmvEw==} + events@3.3.0: resolution: {integrity: sha512-mQw+2fkQbALzQ7V0MY0IqdnXNOeTtP4r0lN9z7AAawCXgqea7bDii20AYrIBrFd/Hx0M2Ocz6S111CaFkUcb0Q==} engines: {node: '>=0.8.x'} @@ -2692,17 +2698,18 @@ packages: resolution: {integrity: sha512-4FbRdAX+bSdmo4AUFuS0WNiPz8NgFt+r8ThgNWmlrjQjt1Q7ZR9+zTlce2859x4KSXrwIsaeTqDoKQmtP8pLmQ==} engines: {node: '>= 0.8'} - http-proxy-middleware@4.1.1: - resolution: {integrity: sha512-KX5ZofGXLFXqFAkQoOWZ+rTtaLTut7m0gyL+QzJrdejtIZ+F4bPPDoe7reISg2+v0CAz5OfVwEJEhty7X+e57g==} - engines: {node: ^22.15.0 || ^24.0.0 || >=26.0.0} + http-proxy-middleware@3.0.7: + resolution: {integrity: sha512-iwbQltVlx8bCrqePUM8C+hllHvdawVhQJaLrj1X7qllkvFQdXFsr16pW/mo9+JDVjN+QO2XUx9jd8SmoFkE5qw==} + engines: {node: ^14.18.0 || ^16.10.0 || >=18.0.0} + + http-proxy@1.18.1: + resolution: {integrity: sha512-7mz/721AbnJwIVbnaSv1Cz3Am0ZLT/UBwkC92VlxhXv/k/BBQfM2fXElQNC27BVGr0uwUpplYPQM9LnaBMR5NQ==} + engines: {node: '>=8.0.0'} https-proxy-agent@5.0.1: resolution: {integrity: sha512-dFcAjpTQFgoLMzC2VwU+C/CbS7uRL0lWmxDITmqm7C+7F0Odmj6s9l6alZc6AELXhrnggM2CeWSXHGOdX2YtwA==} engines: {node: '>= 6'} - httpxy@0.5.3: - resolution: {integrity: sha512-SMS9V6Sn7VWaS11lYhoAr0ceoaiolTWf4jYdJn0NJhCdKMu9R2H9Fh0LBDWBHQF6HRLI1PmaePYsjanSpE5PEw==} - human-signals@2.1.0: resolution: {integrity: sha512-B4FFZ6q/T2jhhksgkbEW3HBvWIfDW85snkQgawt07S7J5QXTk6BkNV+0yAeZrM5QpMAdYlocGoljn0sJ/WQkFw==} engines: {node: '>=10.17.0'} @@ -2828,6 +2835,10 @@ packages: resolution: {integrity: sha512-+Pgi+vMuUNkJyExiMBt5IlFoMyKnr5zhJ4Uspz58WOhBF5QoIZkFyNHIbBAtHwzVAgk5RtndVNsDRN61/mmDqg==} engines: {node: '>=12'} + is-plain-object@5.0.0: + resolution: {integrity: sha512-VRSzKkbMm5jMDoKLbltAkFQ5Qr7VDiTFGXxYFXXowVj387GeGNOCsOH6Msy00SGZ3Fp84b1Naa1psqgcCIEP5Q==} + engines: {node: '>=0.10.0'} + is-promise@4.0.0: resolution: {integrity: sha512-hvpoI6korhJMnej285dSg6nu1+e6uxs7zG3BYAm5byqDsgJNWwxzM6z6iZiAgQR4TJ30JmBTOwqZUw3WlyH3AQ==} @@ -3898,6 +3909,9 @@ packages: resolution: {integrity: sha512-L9jEkOi3ASd9PYit2cwRfyppc9NoABujTP8/5gFcbERmo5jUoAKovIC3fsF17pkTnGsrByysqX+Kxd2OTNI1ww==} engines: {node: '>=0.10.5'} + requires-port@1.0.0: + resolution: {integrity: sha512-KigOCHcocU3XODJxsu8i/j8T9tzT4adHiecwORRQ0ZZFcp7ahwXuRU1m+yuO90C5ZUyGeGfocHDI14M3L3yDAQ==} + resolve-cwd@3.0.0: resolution: {integrity: sha512-OrZaX2Mb+rJCpH/6CpSqt9xFVpN++x01XnN2ie9g6P5/3xelLAkXWVADpdz1IHD/KFfEXyE6V0U01OQ3UO2rEg==} engines: {node: '>=8'} @@ -6086,6 +6100,10 @@ snapshots: '@types/http-errors@2.0.5': {} + '@types/http-proxy@1.17.17': + dependencies: + '@types/node': 22.19.19 + '@types/istanbul-lib-coverage@2.0.6': {} '@types/istanbul-lib-report@3.0.3': @@ -6485,7 +6503,7 @@ snapshots: axios@1.17.0: dependencies: - follow-redirects: 1.16.0 + follow-redirects: 1.16.0(debug@4.4.3) form-data: 4.0.5 https-proxy-agent: 5.0.1 proxy-from-env: 2.1.0 @@ -6713,7 +6731,7 @@ snapshots: centra@2.7.0: dependencies: - follow-redirects: 1.16.0 + follow-redirects: 1.16.0(debug@4.4.3) transitivePeerDependencies: - debug @@ -7147,6 +7165,8 @@ snapshots: event-target-shim@5.0.1: {} + eventemitter3@4.0.7: {} + events@3.3.0: {} execa@5.1.1: @@ -7297,7 +7317,9 @@ snapshots: flatted@3.4.2: {} - follow-redirects@1.16.0: {} + follow-redirects@1.16.0(debug@4.4.3): + optionalDependencies: + debug: 4.4.3 for-each@0.3.5: dependencies: @@ -7655,16 +7677,25 @@ snapshots: statuses: 2.0.2 toidentifier: 1.0.1 - http-proxy-middleware@4.1.1: + http-proxy-middleware@3.0.7: dependencies: + '@types/http-proxy': 1.17.17 debug: 4.4.3 - httpxy: 0.5.3 + http-proxy: 1.18.1(debug@4.4.3) is-glob: 4.0.3 - is-plain-obj: 4.1.0 + is-plain-object: 5.0.0 micromatch: 4.0.8 transitivePeerDependencies: - supports-color + http-proxy@1.18.1(debug@4.4.3): + dependencies: + eventemitter3: 4.0.7 + follow-redirects: 1.16.0(debug@4.4.3) + requires-port: 1.0.0 + transitivePeerDependencies: + - debug + https-proxy-agent@5.0.1: dependencies: agent-base: 6.0.2 @@ -7672,8 +7703,6 @@ snapshots: transitivePeerDependencies: - supports-color - httpxy@0.5.3: {} - human-signals@2.1.0: {} husky@9.1.7: {} @@ -7786,6 +7815,8 @@ snapshots: is-plain-obj@4.1.0: {} + is-plain-object@5.0.0: {} + is-promise@4.0.0: {} is-property@1.0.2: {} @@ -9259,6 +9290,8 @@ snapshots: requireindex@1.2.0: {} + requires-port@1.0.0: {} + resolve-cwd@3.0.0: dependencies: resolve-from: 5.0.0 diff --git a/src/apps/napcat-webui-gateway/application/napcat-webui-gateway-session.service.ts b/src/apps/napcat-webui-gateway/application/napcat-webui-gateway-session.service.ts index e0f69eb..c28467a 100644 --- a/src/apps/napcat-webui-gateway/application/napcat-webui-gateway-session.service.ts +++ b/src/apps/napcat-webui-gateway/application/napcat-webui-gateway-session.service.ts @@ -42,7 +42,7 @@ export class NapcatWebuiGatewaySessionService { normalizedInput.accountId, ); if (existing) { - await this.store.update(existing.sessionId, { + await this.updateSession(existing.sessionId, { revokedAt: this.config.now(), status: 'revoked', }); @@ -77,7 +77,7 @@ export class NapcatWebuiGatewaySessionService { const session = await this.requireUsableSession(sessionId); const now = this.config.now(); - return this.store.update(sessionId, { + return this.updateSession(sessionId, { activeAt: session.activeAt || now, expiresAt: now + this.config.ttlMs(), lastSeenAt: now, @@ -91,12 +91,13 @@ export class NapcatWebuiGatewaySessionService { * @returns Browser-safe lifecycle result. */ async heartbeat(input: NapcatWebuiGatewayLifecycleInput) { + const adminUserId = this.requireLifecycleAdminUserId(input.adminUserId); const session = await this.requireUsableSession(input.sessionId); - this.assertOwner(session, input.adminUserId); + this.assertOwner(session, adminUserId); const now = this.config.now(); const expiresAt = now + this.config.ttlMs(); - await this.store.update(input.sessionId, { + await this.updateSession(input.sessionId, { clientIp: this.toOptionalText(input.clientIp) || session.clientIp, expiresAt, lastSeenAt: now, @@ -117,10 +118,11 @@ export class NapcatWebuiGatewaySessionService { * @returns Browser-safe lifecycle result. */ async revoke(input: NapcatWebuiGatewayLifecycleInput) { + const adminUserId = this.requireLifecycleAdminUserId(input.adminUserId); const session = await this.requireUsableSession(input.sessionId); - this.assertOwner(session, input.adminUserId); + this.assertOwner(session, adminUserId); - const updated = await this.store.update(input.sessionId, { + const updated = await this.updateSession(input.sessionId, { clientIp: this.toOptionalText(input.clientIp) || session.clientIp, revokedAt: this.config.now(), status: 'revoked', @@ -154,7 +156,7 @@ export class NapcatWebuiGatewaySessionService { throw new GoneException('Gateway session is not active'); } if (session.expiresAt <= this.config.now()) { - await this.store.update(sessionId, { status: 'expired' }); + await this.updateSession(sessionId, { status: 'expired' }); throw new GoneException('Gateway session is not active'); } @@ -175,6 +177,48 @@ export class NapcatWebuiGatewaySessionService { } } + /** + * Applies a store update and maps expected stale lifecycle rejections to 410. + * @param sessionId - Gateway session id to update. + * @param patch - Session fields to merge in the store. + * @returns Updated session from the backing store. + */ + private async updateSession( + sessionId: string, + patch: Partial, + ) { + try { + return await this.store.update(sessionId, patch); + } catch (error) { + if (this.isInactiveStoreError(error)) { + throw new GoneException('Gateway session is not active'); + } + throw error; + } + } + + /** + * Detects stale or inactive store errors that should not become HTTP 500. + * @param error - Error thrown by the session store. + * @returns Whether the error represents an expected inactive session race. + */ + private isInactiveStoreError(error: unknown) { + const message = error instanceof Error ? error.message : String(error); + return ( + message.includes('Gateway session is not active') || + message.includes('Gateway terminal session cannot become active') + ); + } + + /** + * Requires a lifecycle Admin actor before owner comparison. + * @param adminUserId - Candidate Admin actor id from heartbeat or revoke body. + * @returns Trimmed Admin actor id. + */ + private requireLifecycleAdminUserId(adminUserId: string) { + return this.requireText(adminUserId, 'adminUserId'); + } + /** * Validates and normalizes the internal create-session payload before persistence. * @param input - Internal API payload supplied by the main API process. diff --git a/src/apps/napcat-webui-gateway/infrastructure/session/napcat-webui-gateway-redis.store.ts b/src/apps/napcat-webui-gateway/infrastructure/session/napcat-webui-gateway-redis.store.ts index 1cca361..1270710 100644 --- a/src/apps/napcat-webui-gateway/infrastructure/session/napcat-webui-gateway-redis.store.ts +++ b/src/apps/napcat-webui-gateway/infrastructure/session/napcat-webui-gateway-redis.store.ts @@ -10,11 +10,36 @@ import type { const SESSION_KEY_PREFIX = 'napcat:webui:session:'; const USER_ACCOUNT_KEY_PREFIX = 'napcat:webui:user-account:'; const TERMINAL_SESSION_STATUSES = ['expired', 'failed', 'revoked']; -const COMPARE_DELETE_SCRIPT = ` -if redis.call("GET", KEYS[1]) == ARGV[1] then - return redis.call("DEL", KEYS[1]) +const UPDATE_SESSION_SCRIPT = ` +local currentJson = redis.call("GET", KEYS[1]) +if not currentJson then + return {0, "Gateway session is not active"} end -return 0 + +local current = cjson.decode(currentJson) +local next = cjson.decode(ARGV[2]) +local terminal = { expired = true, failed = true, revoked = true } +local indexValue = redis.call("GET", KEYS[2]) + +if terminal[current["status"]] and not terminal[next["status"]] then + return {0, "Gateway session is not active"} +end + +if terminal[next["status"]] then + redis.call("PSETEX", KEYS[1], ARGV[3], ARGV[2]) + if indexValue == ARGV[1] then + redis.call("DEL", KEYS[2]) + end + return {1, ARGV[2]} +end + +if indexValue and indexValue ~= ARGV[1] then + return {0, "Gateway session is not active"} +end + +redis.call("PSETEX", KEYS[1], ARGV[3], ARGV[2]) +redis.call("SET", KEYS[2], ARGV[1], "PX", ARGV[4]) +return {1, ARGV[2]} `; @Injectable() @@ -94,12 +119,7 @@ export class NapcatWebuiGatewayRedisStore throw new Error('Gateway terminal session cannot become active'); } - await this.writeSession(next); - if (this.isTerminal(next)) { - await this.deleteUserAccountIndexIfCurrent(next); - } else { - await this.writeUserAccountIndex(next); - } + await this.writeSessionAndIndexAtomically(next); return next; } @@ -129,21 +149,6 @@ export class NapcatWebuiGatewayRedisStore ); } - /** - * Deletes the user/account index only when it still points at the terminal session. - * @param session - Terminal Gateway session whose index may need cleanup. - */ - private async deleteUserAccountIndexIfCurrent( - session: NapcatWebuiGatewaySession, - ) { - await this.redis.eval( - COMPARE_DELETE_SCRIPT, - 1, - this.userAccountKey(session.adminUserId, session.accountId), - session.sessionId, - ); - } - /** * Builds the Redis session key. * @param sessionId - Gateway session id. @@ -172,6 +177,29 @@ export class NapcatWebuiGatewayRedisStore return Math.max(1, session.expiresAt - this.config.now()); } + /** + * Atomically writes session JSON and maintains the user/account index. + * @param session - Already-merged Gateway session to commit. + */ + private async writeSessionAndIndexAtomically( + session: NapcatWebuiGatewaySession, + ) { + const sessionJson = JSON.stringify(session); + const result = (await this.redis.eval( + UPDATE_SESSION_SCRIPT, + 2, + this.sessionKey(session.sessionId), + this.userAccountKey(session.adminUserId, session.accountId), + session.sessionId, + sessionJson, + this.remainingTtlMs(session), + this.remainingTtlMs(session), + )) as [number, string]; + if (!Array.isArray(result) || Number(result[0]) !== 1) { + throw new Error(String(result?.[1] || 'Gateway session is not active')); + } + } + /** * Checks whether the session has reached a terminal lifecycle status. * @param session - Gateway session to inspect. diff --git a/test/apps/napcat-webui-gateway/session-store.spec.ts b/test/apps/napcat-webui-gateway/session-store.spec.ts index e266528..7d8bad7 100644 --- a/test/apps/napcat-webui-gateway/session-store.spec.ts +++ b/test/apps/napcat-webui-gateway/session-store.spec.ts @@ -145,27 +145,62 @@ class FakeRedis { } /** - * Simulates the compare-and-delete Lua script used by Redis index cleanup. + * Simulates Gateway Redis Lua scripts used by ticket and session store tests. * @param script - Lua script text. * @param keyCount - Number of Redis keys in the script call. - * @param key - Redis key to conditionally delete. - * @param expectedValue - Value that must match before deletion. - * @returns 1 when the index was deleted, otherwise 0. + * @param args - Redis keys and script arguments after `keyCount`. + * @returns Script-shaped response used by the store. */ - async eval( - script: string, - keyCount: number, - key: string, - expectedValue: string, - ) { - this.calls.push(`eval:${keyCount}:${key}:${expectedValue}`); - if (!script.includes('redis.call') || keyCount !== 1) { + async eval(script: string, keyCount: number, ...args: string[]) { + this.calls.push(`eval:${keyCount}:${args.join(':')}`); + if (!script.includes('redis.call')) { throw new Error('Unexpected Redis script'); } - if (this.values.get(key) !== expectedValue) return 0; - this.values.delete(key); - this.ttl.delete(key); - return 1; + if (keyCount !== 2) { + throw new Error('Unexpected Redis key count'); + } + + const [ + sessionKey, + indexKey, + sessionId, + nextSessionJson, + sessionTtlMs, + indexTtlMs, + ] = args; + const currentJson = this.values.get(sessionKey); + if (!currentJson) return [0, 'Gateway session is not active']; + + const current = JSON.parse(currentJson) as NapcatWebuiGatewaySession; + const next = JSON.parse(nextSessionJson) as NapcatWebuiGatewaySession; + const terminalStatuses = ['expired', 'failed', 'revoked']; + const currentTerminal = terminalStatuses.includes(current.status); + const nextTerminal = terminalStatuses.includes(next.status); + const indexValue = this.values.get(indexKey); + + if (currentTerminal && !nextTerminal) { + return [0, 'Gateway session is not active']; + } + + if (nextTerminal) { + this.values.set(sessionKey, nextSessionJson); + this.ttl.set(sessionKey, Number(sessionTtlMs)); + if (indexValue === sessionId) { + this.values.delete(indexKey); + this.ttl.delete(indexKey); + } + return [1, nextSessionJson]; + } + + if (indexValue && indexValue !== sessionId) { + return [0, 'Gateway session is not active']; + } + + this.values.set(sessionKey, nextSessionJson); + this.ttl.set(sessionKey, Number(sessionTtlMs)); + this.values.set(indexKey, sessionId); + this.ttl.set(indexKey, Number(indexTtlMs)); + return [1, nextSessionJson]; } } @@ -410,11 +445,81 @@ describe('NapcatWebuiGatewayRedisStore', () => { sessionId: second.sessionId, status: 'created', }); - expect( - redis.calls.some((call) => - call.startsWith('eval:1:napcat:webui:user-account:admin-1:account-1:'), - ), - ).toBe(true); + expect(redis.values.get('napcat:webui:user-account:admin-1:account-1')).toBe( + second.sessionId, + ); + }); + + it('rejects stale non-terminal updates when the index points at a newer session', async () => { + const redis = new FakeRedis(); + const config = createConfig({ value: 1000 }); + const store = new NapcatWebuiGatewayRedisStore( + redis as never, + config as never, + ); + const service = new NapcatWebuiGatewaySessionService( + store, + config as never, + ); + const first = await service.create(createSessionInput()); + const second = await service.create(createSessionInput()); + const firstSessionKey = `napcat:webui:session:${first.sessionId}`; + const indexKey = 'napcat:webui:user-account:admin-1:account-1'; + + redis.values.set( + firstSessionKey, + JSON.stringify({ + ...first, + status: 'created', + }), + ); + redis.values.set(indexKey, second.sessionId); + + await expect( + store.update(first.sessionId, { + activeAt: 2000, + expiresAt: 62_000, + lastSeenAt: 2000, + status: 'active', + }), + ).rejects.toThrow('Gateway session is not active'); + expect(redis.values.get(indexKey)).toBe(second.sessionId); + expect(JSON.parse(redis.values.get(firstSessionKey) || '{}')).toMatchObject({ + status: 'created', + }); + }); + + it('keeps delayed old heartbeat and activation from reviving a replaced session', async () => { + const redis = new FakeRedis(); + const currentTime = { value: 1000 }; + const config = createConfig(currentTime); + const store = new NapcatWebuiGatewayRedisStore( + redis as never, + config as never, + ); + const service = new NapcatWebuiGatewaySessionService( + store, + config as never, + ); + const first = await service.create(createSessionInput()); + const second = await service.create(createSessionInput()); + + currentTime.value = 2000; + + await expect(service.markActive(first.sessionId)).rejects.toThrow( + 'Gateway session is not active', + ); + await expect( + service.heartbeat({ + adminUserId: 'admin-1', + sessionId: first.sessionId, + }), + ).rejects.toThrow('Gateway session is not active'); + await expect( + store.findActiveByUserAndAccount('admin-1', 'account-1'), + ).resolves.toMatchObject({ + sessionId: second.sessionId, + }); }); }); @@ -568,6 +673,26 @@ describe('InternalSessionController', () => { .expect(HttpStatus.GONE); }); + it('rejects blank lifecycle admin user ids with bad request status', async () => { + const createResponse = await request(app.getHttpServer()) + .post('/internal/sessions') + .set('x-kt-gateway-secret', INTERNAL_SECRET) + .send(createSessionInput()) + .expect(HttpStatus.CREATED); + const sessionId = createResponse.body.sessionId; + + await request(app.getHttpServer()) + .post(`/internal/sessions/${sessionId}/heartbeat`) + .set('x-kt-gateway-secret', INTERNAL_SECRET) + .send({}) + .expect(HttpStatus.BAD_REQUEST); + await request(app.getHttpServer()) + .post(`/internal/sessions/${sessionId}/revoke`) + .set('x-kt-gateway-secret', INTERNAL_SECRET) + .send({ adminUserId: ' ' }) + .expect(HttpStatus.BAD_REQUEST); + }); + it('rejects invalid create-session payloads with bad request status', async () => { await request(app.getHttpServer()) .post('/internal/sessions')