From 299fa85a740567c4b72459dede8fb5b959a998be Mon Sep 17 00:00:00 2001 From: DrSmoothl <1787882683@qq.com> Date: Sat, 31 Oct 2026 18:13:59 +0800 Subject: [PATCH] fix(onebot): match group invitations to the correct request Report distinct invitations separately and approve only the request that belongs to the account and inviter. Reject ambiguous approvals and resolve the sender of invitations from outside the contact list. Refs #612 --- packages/core/src/bridge/apis/contacts.ts | 31 +--- packages/core/src/bridge/bridge.ts | 3 +- packages/core/tests/actions/contacts.test.ts | 9 ++ packages/core/tests/msg-push-handler.test.ts | 5 +- .../onebot/src/modules/request-actions.ts | 71 ++++++--- .../onebot/tests/group-request-events.test.ts | 116 +++++++++++++- .../onebot/tests/instance-context.test.ts | 2 +- packages/onebot/tests/request-actions.test.ts | 46 +++++- packages/protocol/bench/recv.bench.ts | 3 +- packages/protocol/package.json | 4 + packages/protocol/src/group-invite-tracker.ts | 142 ++++++++++++++++++ packages/protocol/src/packet-pipeline.ts | 80 +++------- .../packet-pipeline-roster-refresh.test.ts | 3 +- .../packet-pipeline-stranger-resolve.test.ts | 50 +++--- .../tests/packet-pipeline-trace.test.ts | 3 +- .../tests/self-invite-dispatch.test.ts | 3 +- 16 files changed, 429 insertions(+), 142 deletions(-) create mode 100644 packages/protocol/src/group-invite-tracker.ts diff --git a/packages/core/src/bridge/apis/contacts.ts b/packages/core/src/bridge/apis/contacts.ts index f3575a12..9ab3e151 100644 --- a/packages/core/src/bridge/apis/contacts.ts +++ b/packages/core/src/bridge/apis/contacts.ts @@ -1,3 +1,4 @@ +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { FetchDownloadRkeys } from '@snowluma/protocol/oidb-services/contacts/fetch-download-rkeys'; import { FetchFriendListPage, @@ -109,9 +110,7 @@ export class ContactsApi { load: (groupId) => this.fetchGroupMemberListUncached(groupId), }); private robotUinRangesPromise_: Promise | null = null; - /** groupUin → approval msgseq from a private qun.invite card. Written - * only by IncomingPacketPipeline (and tests). OneBot reads get/find. */ - private readonly groupInviteCardSeqs_ = new Map(); + readonly groupInvites = new GroupInviteTracker(); constructor(private readonly ctx: BridgeContext) {} @@ -616,32 +615,16 @@ export class ContactsApi { } /** Returns whether this live card introduces a new approval sequence. */ - rememberGroupInviteCardSequence(groupUin: number, sequence: number): boolean { - if (this.groupInviteCardSeqs_.get(groupUin) === sequence) { - return false; - } - - this.groupInviteCardSeqs_.set(groupUin, sequence); - return true; + rememberGroupInviteCardSequence(groupId: number, sequence: number): boolean { + return this.groupInvites.receiveCard(groupId, sequence); } - /** The approval msgseq captured from a private "qun.invite" card for this - * group, or undefined if none was seen. `set_group_add_request` uses it to - * approve a bot self-invite via 0x10c8 (eventType=2). See issue #125. */ getGroupInviteCardSequence(groupId: number): number | undefined { - return this.groupInviteCardSeqs_.get(groupId); + return this.groupInvites.getCardSequence(groupId); } - /** Resolve the group for a private invite-card msgseq. Numeric OneBot flags - * use this path because the card sequence is absent from 0x10C0. */ - findGroupInviteCardGroupBySequence(sequence: number): number | undefined { - for (const [groupUin, rememberedSequence] of this.groupInviteCardSeqs_) { - if (rememberedSequence === sequence) { - return groupUin; - } - } - - return undefined; + findGroupInviteCardGroupBySequence(sequence: number, groupId?: number): number | undefined { + return this.groupInvites.findCardGroup(sequence, groupId); } async fetchDownloadRKeys(): Promise { diff --git a/packages/core/src/bridge/bridge.ts b/packages/core/src/bridge/bridge.ts index 07183b88..a0d4e967 100644 --- a/packages/core/src/bridge/bridge.ts +++ b/packages/core/src/bridge/bridge.ts @@ -73,8 +73,7 @@ export class Bridge implements BridgeInterface { }, resolveGroupJoinRequest: (groupId, uid, subType, invitedUid) => this.apis.contacts.resolveGroupJoinRequest(groupId, uid, subType, invitedUid), - rememberGroupInviteCardSequence: (groupUin, sequence) => - this.apis.contacts.rememberGroupInviteCardSequence(groupUin, sequence), + groupInvites: this.apis.contacts.groupInvites, }); this.pipeline.registerCmd(MSG_PUSH_CMD, (pkt, identity) => parseMsgPushOrThrow(pkt, identity, this.sysMsgDedup_), diff --git a/packages/core/tests/actions/contacts.test.ts b/packages/core/tests/actions/contacts.test.ts index c5aaa8cd..29f76546 100644 --- a/packages/core/tests/actions/contacts.test.ts +++ b/packages/core/tests/actions/contacts.test.ts @@ -761,4 +761,13 @@ describe('apis/contacts / invite-card pending application', () => { expect(api.findGroupInviteCardGroupBySequence(778899)).toBe(12345); expect(api.findGroupInviteCardGroupBySequence(1)).toBeUndefined(); }); + + it('requires a group when different groups have the same card sequence', () => { + const api = new ContactsApi({ identity: { uin: '10001' } } as any); + api.rememberGroupInviteCardSequence(12345, 778899); + api.rememberGroupInviteCardSequence(67890, 778899); + + expect(() => api.findGroupInviteCardGroupBySequence(778899)).toThrow('multiple groups'); + expect(api.findGroupInviteCardGroupBySequence(778899, 67890)).toBe(67890); + }); }); diff --git a/packages/core/tests/msg-push-handler.test.ts b/packages/core/tests/msg-push-handler.test.ts index 49cbaf50..a5d9a14e 100644 --- a/packages/core/tests/msg-push-handler.test.ts +++ b/packages/core/tests/msg-push-handler.test.ts @@ -2,6 +2,7 @@ import { describe, expect, it, vi } from 'vitest'; import { protobuf_encode } from '@snowluma/proton'; import { parseMsgPush, MSG_PUSH_CMD } from '@snowluma/protocol/msg-push'; import { IdentityService } from '@snowluma/protocol/identity-service'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import type { GroupMemberInfo, QQGroupInfo } from '@snowluma/protocol/qq-info'; @@ -529,7 +530,7 @@ describe('parseMsgPush Event0x210 subType=364 (friend remark synchronization)', fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); pipeline.registerCmd(MSG_PUSH_CMD, parseMsgPush); @@ -1028,7 +1029,7 @@ describe('parseMsgPush PkgType 44 (group admin set/unset) keeps the member cache fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); pipeline.registerCmd(MSG_PUSH_CMD, parseMsgPush); await pipeline.process(packet); diff --git a/packages/onebot/src/modules/request-actions.ts b/packages/onebot/src/modules/request-actions.ts index 3adc6770..9a7212e7 100644 --- a/packages/onebot/src/modules/request-actions.ts +++ b/packages/onebot/src/modules/request-actions.ts @@ -125,7 +125,10 @@ async function resolveLiveGroupRequest( bridge: BridgeInterface, handle: GroupRequestHandle, ): Promise { - const cardGroup = bridge.apis.contacts.findGroupInviteCardGroupBySequence?.(handle.sequence); + const cardGroup = bridge.apis.contacts.findGroupInviteCardGroupBySequence?.( + handle.sequence, + handle.groupId, + ); // Private invite-card seq is absent from 0x10C0; keep the flag tuple (#125). if (cardGroup === handle.groupId) { @@ -231,28 +234,43 @@ export async function handleGroupAddRequest( // Legacy SnowLuma flags remain accepted for events emitted by older builds. // A private invite card needs its msgseq rather than the 0x10C0 tuple (#125). - if (parsed.requestType === 'invite') { - const cardSequence = bridge.apis.contacts.getGroupInviteCardSequence(parsed.groupId); + const notice = + parsed.requestType === 'invite' + ? bridge.apis.contacts.groupInvites.getSelfInviteNotice( + parsed.groupId, + parsed.targetUid, + ) + : undefined; - if (cardSequence) { - await applyGroupRequest( - bridge, - { - groupId: parsed.groupId, - sequence: cardSequence, - eventType: 2, - filtered: false, - }, - approve, - reason, - ); - return; - } + if (notice?.cardSequence) { + await applyGroupRequest( + bridge, + { + groupId: parsed.groupId, + sequence: notice.cardSequence, + eventType: 2, + filtered: false, + }, + approve, + reason, + ); + return; } const lookup = await fetchRequestInboxes(bridge, 'uid'); - const matching = lookup.requests.find((request) => { - if (request.groupId !== parsed.groupId) { + const candidates = lookup.requests.filter((request) => { + if (request.groupId !== parsed.groupId || request.state !== 1 || request.eventType <= 0) { + return false; + } + + if ( + notice && + (request.eventType !== 2 || + (request.targetUin > 0 && request.targetUin !== Number(bridge.identity.uin)) || + (request.targetUid && + bridge.identity.selfUid && + request.targetUid !== bridge.identity.selfUid)) + ) { return false; } @@ -261,11 +279,22 @@ export async function handleGroupAddRequest( : request.invitorUid === parsed.targetUid; }); - // Never fall back to an arbitrary request from the same group: when a UID - // does not match, approving another pending request is worse than failing. + const matching = candidates[0]; + if (!matching) { throwMissingGroupRequest('matching group request not found', lookup.failures); } + if ( + candidates.some( + (request) => + request.sequence !== matching.sequence || + request.eventType !== matching.eventType || + request.filtered !== matching.filtered, + ) + ) { + throw new Error('multiple pending group requests match this flag'); + } + await applyGroupRequest(bridge, matching, approve, reason, matching); } diff --git a/packages/onebot/tests/group-request-events.test.ts b/packages/onebot/tests/group-request-events.test.ts index f8c9af92..ba0dc460 100644 --- a/packages/onebot/tests/group-request-events.test.ts +++ b/packages/onebot/tests/group-request-events.test.ts @@ -70,11 +70,11 @@ function jsonCard(content: unknown, senderUin = 456): PacketInfo { }); } -function selfInviteNotice(): PacketInfo { +function selfInviteNotice(invitorUid = 'u_inviter', groupUin = 999): PacketInfo { return packet({ contentHead: { msgType: 87, timestamp: 1 }, body: { - msgContent: protobuf_encode({ groupUin: 999, invitorUid: 'u_inviter' }), + msgContent: protobuf_encode({ groupUin, invitorUid }), }, }); } @@ -131,7 +131,7 @@ describe('group request events', () => { ); it.each(['notice first', 'card first'])( - 'emits only the card when paired with a self-invite notice: %s', + 'emits one request when a self-invite notice and its card both arrive: %s', async (order) => { const { bridge, received, fetchRequests } = setup(); const packets = @@ -159,6 +159,116 @@ describe('group request events', () => { expect(second.received).toHaveLength(1); }); + it('pairs a notice exactly once and keeps its own card sequence for approval', async () => { + const { bridge, received, fetchRequests } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + const setAddRequest = vi.spyOn(bridge.apis.groupAdmin, 'setAddRequest').mockResolvedValue(); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(card(700)); + await bridge.onPacket(card(701)); + await bridge.onPacket(card(700)); + + expect(received.map((event) => event.flag)).toEqual([ + 'invite:999:u_inviter', + 'slreq:1:701:999:2:0', + ]); + await handleGroupAddRequest(bridge, received[0]!.flag, true, ''); + expect(setAddRequest).toHaveBeenCalledWith(999, 700, 2, true, '', false, undefined); + expect(fetchRequests).not.toHaveBeenCalled(); + }); + + it.each(['u_other', 'u_inviter'])( + 'does not approve a stale card for a new notice from %s', + async (uid) => { + vi.useFakeTimers({ toFake: ['Date'] }); + const { bridge, received, fetchRequests } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid, uin: 789 }); + const setAddRequest = vi + .spyOn(bridge.apis.groupAdmin, 'setAddRequest') + .mockResolvedValue(); + fetchRequests.mockImplementation(async (filtered) => + filtered + ? [] + : [request({ invitorUid: uid, sequence: 43, eventType: 2, targetUin: 10001 })], + ); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(card(700)); + vi.setSystemTime(Date.now() + 4_000); + await bridge.onPacket(selfInviteNotice(uid)); + + const latest = received.at(-1)!; + expect(latest.flag).toBe(`invite:999:${uid}`); + await handleGroupAddRequest(bridge, latest.flag, true, ''); + expect(setAddRequest).toHaveBeenCalledExactlyOnceWith( + 999, + 43, + 2, + true, + '', + false, + undefined, + ); + }, + ); + + it('does not emit a self-invite flag without a group and inviter', async () => { + const { bridge, received } = setup(); + await bridge.onPacket(selfInviteNotice('')); + await bridge.onPacket(selfInviteNotice(' ')); + await bridge.onPacket(selfInviteNotice('u_inviter', 0)); + expect(received).toEqual([]); + }); + + it.each([22, 2])( + 'cannot use a self-invitation flag to approve another account joining the group (type %i)', + async (eventType) => { + const { bridge, received, fetchRequests } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + const setAddRequest = vi + .spyOn(bridge.apis.groupAdmin, 'setAddRequest') + .mockResolvedValue(); + fetchRequests.mockImplementation(async (filtered) => + filtered ? [] : [request({ invitorUid: 'u_inviter', eventType, targetUin: 456 })], + ); + await bridge.onPacket(selfInviteNotice()); + await expect( + handleGroupAddRequest(bridge, received[0]!.flag, true, ''), + ).rejects.toThrow('matching group request not found'); + expect(setAddRequest).not.toHaveBeenCalled(); + }, + ); + + it('emits a later card when the pairing window has expired', async () => { + vi.useFakeTimers({ toFake: ['Date'] }); + const { bridge, received } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + await bridge.onPacket(selfInviteNotice()); + vi.setSystemTime(Date.now() + 4_000); + await bridge.onPacket(card(701)); + expect(received.map((event) => event.flag)).toEqual([ + 'invite:999:u_inviter', + 'slreq:1:701:999:2:0', + ]); + }); + + it('refuses an ambiguous legacy flag when two self invitations overlap', async () => { + const { bridge, received } = setup(); + bridge.identity.rememberRequestIdentity({ groupId: 999, uid: 'u_inviter', uin: 456 }); + const setAddRequest = vi.spyOn(bridge.apis.groupAdmin, 'setAddRequest').mockResolvedValue(); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(selfInviteNotice()); + await bridge.onPacket(card(701)); + await bridge.onPacket(card(702)); + expect(received.map((event) => event.flag)).toEqual([ + 'invite:999:u_inviter', + 'invite:999:u_inviter', + ]); + await expect(handleGroupAddRequest(bridge, received[0]!.flag, true, '')).rejects.toThrow( + 'multiple pending self invitations', + ); + expect(setAddRequest).not.toHaveBeenCalled(); + }); + it('does not report sent cards or links in unrelated JSON cards as invitations', async () => { const { bridge, received } = setup(); await bridge.onPacket(card(778899, 10001)); diff --git a/packages/onebot/tests/instance-context.test.ts b/packages/onebot/tests/instance-context.test.ts index d2c35ab5..3d41e8b5 100644 --- a/packages/onebot/tests/instance-context.test.ts +++ b/packages/onebot/tests/instance-context.test.ts @@ -200,7 +200,7 @@ function makeRef( fetchGroupRequests: vi.fn(async () => []), fetchDownloadRKeys: vi.fn(async () => []), findGroupInviteCardGroupBySequence: vi.fn(() => undefined), - getGroupInviteCardSequence: vi.fn(() => undefined), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, ...overrides.apis?.contacts, }, groupFile: { diff --git a/packages/onebot/tests/request-actions.test.ts b/packages/onebot/tests/request-actions.test.ts index e493cb32..de1d3b29 100644 --- a/packages/onebot/tests/request-actions.test.ts +++ b/packages/onebot/tests/request-actions.test.ts @@ -12,7 +12,7 @@ const APIS_ROUTING: Record = { fetchGroupRequests: 'contacts', fetchGroupRequestsByUid: 'contacts', fetchDownloadRKeys: 'contacts', - getGroupInviteCardSequence: 'contacts', + groupInvites: 'contacts', findGroupInviteCardGroupBySequence: 'contacts', }; @@ -205,7 +205,7 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { filtered: false, }), ]), - getGroupInviteCardSequence: vi.fn(() => null), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, apis: { groupAdmin: { setAddRequest } } as any, }); @@ -232,7 +232,7 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { ] : [], ), - getGroupInviteCardSequence: vi.fn(() => null), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, apis: { groupAdmin: { setAddRequest } } as any, }); @@ -246,7 +246,7 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { it('surfaces "not found" only when neither inbox has the request', async () => { const bridge = fakeBridge({ fetchGroupRequestsByUid: vi.fn(async () => []), - getGroupInviteCardSequence: vi.fn(() => null), + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, apis: { groupAdmin: { setAddRequest: vi.fn(async () => {}) } } as any, }); await expect(handleGroupAddRequest(bridge, 'invite:999:u_i', true, 'ok')).rejects.toThrow( @@ -254,6 +254,44 @@ describe('onebot/modules/request-actions / handleGroupAddRequest', () => { ); }); + it('never picks one of two different pending invitations with the same legacy flag', async () => { + const setAddRequest = vi.fn(); + const bridge = fakeBridge({ + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, + fetchGroupRequestsByUid: vi.fn(async (filtered: boolean) => + filtered ? [] : [fakeRequest({ sequence: 123 }), fakeRequest({ sequence: 124 })], + ), + apis: { groupAdmin: { setAddRequest } }, + }); + await expect(handleGroupAddRequest(bridge, 'invite:999:u_i', true, '')).rejects.toThrow( + 'multiple pending group requests', + ); + expect(setAddRequest).not.toHaveBeenCalled(); + }); + + it('skips processed requests when approving a legacy notice', async () => { + const setAddRequest = vi.fn(); + const bridge = fakeBridge({ + groupInvites: { getSelfInviteNotice: vi.fn(() => undefined) }, + fetchGroupRequestsByUid: vi.fn(async (filtered: boolean) => + filtered + ? [] + : [fakeRequest({ sequence: 123, state: 2 }), fakeRequest({ sequence: 124 })], + ), + apis: { groupAdmin: { setAddRequest } }, + }); + await handleGroupAddRequest(bridge, 'invite:999:u_i', true, ''); + expect(setAddRequest).toHaveBeenCalledExactlyOnceWith( + 999, + 124, + 7, + true, + '', + false, + undefined, + ); + }); + it('does not treat an inviter QQ number as a request sequence (#213)', async () => { const setAddRequest = vi.fn(async () => {}); const bridge = fakeBridge({ diff --git a/packages/protocol/bench/recv.bench.ts b/packages/protocol/bench/recv.bench.ts index e6d6e391..86af4966 100644 --- a/packages/protocol/bench/recv.bench.ts +++ b/packages/protocol/bench/recv.bench.ts @@ -24,6 +24,7 @@ import type { GroupChange } from '@snowluma/proto-defs/notify'; import { parseMsgPush, MSG_PUSH_CMD } from '../src/msg-push/index'; import { IdentityService } from '../src/identity-service'; import { BridgeEventBus } from '../src/event-bus'; +import { GroupInviteTracker } from '../src/group-invite-tracker'; import { IncomingPacketPipeline } from '../src/packet-pipeline'; const SELF_UIN = '10001'; @@ -156,7 +157,7 @@ function makePipeline(identity: IdentityService): IncomingPacketPipeline { fetchGroupList: async () => {}, fetchGroupMemberList: async () => {}, resolveGroupJoinRequest: async () => null, - rememberGroupInviteCardSequence: () => true, + groupInvites: new GroupInviteTracker(), }); pipeline.registerCmd(MSG_PUSH_CMD, parseMsgPush); return pipeline; diff --git a/packages/protocol/package.json b/packages/protocol/package.json index 493f3b64..22502ce3 100644 --- a/packages/protocol/package.json +++ b/packages/protocol/package.json @@ -24,6 +24,10 @@ "types": "./src/identity-service.ts", "default": "./src/identity-service.ts" }, + "./group-invite-tracker": { + "types": "./src/group-invite-tracker.ts", + "default": "./src/group-invite-tracker.ts" + }, "./packet-pipeline": { "types": "./src/packet-pipeline.ts", "default": "./src/packet-pipeline.ts" diff --git a/packages/protocol/src/group-invite-tracker.ts b/packages/protocol/src/group-invite-tracker.ts new file mode 100644 index 00000000..a10f1f81 --- /dev/null +++ b/packages/protocol/src/group-invite-tracker.ts @@ -0,0 +1,142 @@ +const PAIR_WINDOW_MS = 3_000; +const MAX_INVITES = 1_024; + +interface Card { + groupId: number; + sequence: number; +} + +interface SelfInviteNotice { + cardSequence?: number; + ambiguous?: boolean; +} + +interface UnpairedInvite { + key: string; + notice?: SelfInviteNotice; + source: 'card' | 'notice'; + expiresAt: number; +} + +function remember(entries: Map, key: string, value: T): void { + entries.delete(key); + + if (entries.size >= MAX_INVITES) { + const oldest = entries.keys().next(); + + if (!oldest.done) { + entries.delete(oldest.value); + } + } + + entries.set(key, value); +} + +/** Account-local state shared by incoming invitations and their approval actions. */ +export class GroupInviteTracker { + private readonly cards = new Map(); + private readonly notices = new Map(); + private unpaired: UnpairedInvite[] = []; + + receiveNotice(groupId: number, uid: string): boolean { + const key = `${groupId}:${uid}`; + const notice: SelfInviteNotice = {}; + + if (this.pair(key, 'notice', notice)) { + return false; + } + + // Earlier notices with the same legacy flag cannot lend this one their card. + remember(this.notices, key, notice); + return true; + } + + receiveCard(groupId: number, sequence: number, uid?: string): boolean { + const key = `${groupId}:${sequence}`; + + if (this.cards.has(key)) { + return false; + } + + const paired = uid ? this.pair(`${groupId}:${uid}`, 'card') : undefined; + + if (paired?.notice) { + paired.notice.cardSequence = sequence; + } + + remember(this.cards, key, { groupId, sequence }); + return !paired; + } + + private pair( + key: string, + source: UnpairedInvite['source'], + notice?: SelfInviteNotice, + ): UnpairedInvite | undefined { + const now = Date.now(); + this.unpaired = this.unpaired.filter((entry) => entry.expiresAt > now); + const match = this.unpaired.findIndex( + (entry) => entry.key === key && entry.source !== source, + ); + + if (match !== -1) { + return this.unpaired.splice(match, 1)[0]; + } + + if (this.unpaired.length >= MAX_INVITES) { + this.unpaired.shift(); + } + + if ( + notice && + this.unpaired.some((entry) => entry.key === key && entry.source === 'notice') + ) { + notice.ambiguous = true; + } + + this.unpaired.push({ key, source, notice, expiresAt: now + PAIR_WINDOW_MS }); + return undefined; + } + + getSelfInviteNotice(groupId: number, uid: string): Readonly | undefined { + const notice = this.notices.get(`${groupId}:${uid}`); + + if (notice?.ambiguous) { + throw new Error('multiple pending self invitations share this flag'); + } + + return notice; + } + + getCardSequence(groupId: number): number | undefined { + let sequence: number | undefined; + + for (const card of this.cards.values()) { + if (card.groupId === groupId) { + sequence = card.sequence; + } + } + + return sequence; + } + + findCardGroup(sequence: number, groupId?: number): number | undefined { + if (groupId !== undefined) { + return this.cards.get(`${groupId}:${sequence}`)?.groupId; + } + + let match: number | undefined; + + for (const card of this.cards.values()) { + if (card.sequence === sequence) { + if (match !== undefined) { + throw new Error('multiple groups match this invitation card sequence'); + } + + match = card.groupId; + } + } + + return match; + } +} diff --git a/packages/protocol/src/packet-pipeline.ts b/packages/protocol/src/packet-pipeline.ts index 97cd6440..38553729 100644 --- a/packages/protocol/src/packet-pipeline.ts +++ b/packages/protocol/src/packet-pipeline.ts @@ -3,17 +3,10 @@ import { formatEvent } from './format'; import { createLogger, type Logger } from '@snowluma/common/logger'; import type { BridgeEventBus } from './event-bus'; import type { QQEventVariant } from './events'; +import type { GroupInviteTracker } from './group-invite-tracker'; import type { IdentityService } from './identity-service'; import { formatGroupRequestFlag, type GroupRequestInfo } from './qq-info'; -/** - * How long a dispatched invitation stays remembered so the MSF notice and the - * private `qun.invite` card of the SAME invitation do not both dispatch. - * Both packets come from one server-side action, so the window only bounds how - * long the marker lives -- matching itself is exact (group + inviter UID). - */ -const INVITE_PAIR_WINDOW_MS = 3_000; - const moduleLog = createLogger('Bridge'); const moduleEventLog = createLogger('Event'); const modulePacketLog = createLogger('Protocol.Packet'); @@ -60,8 +53,7 @@ export interface PacketPipelineDeps { subType: 'add' | 'invite', invitedUid?: string, ): Promise; - /** Remember a live card; false means its approval sequence was already seen. */ - rememberGroupInviteCardSequence(groupUin: number, sequence: number): boolean; + groupInvites: GroupInviteTracker; } export class IncomingPacketPipeline { @@ -70,15 +62,6 @@ export class IncomingPacketPipeline { private readonly log: Logger; private readonly eventLog: Logger; private readonly packetLog: Logger; - /** - * One invitation can arrive as an MSF notice AND as a private card. The card - * is the richer source (it carries the approval msgseq), so either one may - * dispatch -- but never both. Two independent markers keep that mutual: a - * notice suppresses the card that follows it, a card suppresses the notice. - * Cards never suppress each other (a new msgseq is a new invitation). - */ - private readonly noticeDispatched_ = new Map(); - private readonly cardDispatched_ = new Map(); constructor(private readonly deps: PacketPipelineDeps) { // Tag every line we emit with this Bridge's UIN so per-account file @@ -402,21 +385,11 @@ export class IncomingPacketPipeline { if ( !groupId || !sequence || - !this.deps.rememberGroupInviteCardSequence(groupId, sequence) + !this.deps.groupInvites.receiveCard(groupId, sequence, event.senderUid) ) { return false; } - // The msgseq is cached above either way. Skip the duplicate event only when - // the notice already delivered this same invitation -- a card with a new - // sequence is a genuinely new invitation and must always dispatch. - if (this.dispatchedRecently_(this.noticeDispatched_, groupId, event.senderUid ?? '')) { - this.log.debug('invitation already dispatched from the notice: group=%d', groupId); - return false; - } - - this.rememberDispatched_(this.cardDispatched_, groupId, event.senderUid ?? ''); - this.finishDispatch({ kind: 'group_invite', time: event.time, @@ -432,27 +405,6 @@ export class IncomingPacketPipeline { return true; } - private dispatchedRecently_( - marker: Map, - groupId: number, - inviterUid: string, - ): boolean { - const key = `${groupId}:${inviterUid}`; - const at = marker.get(key); - if (at === undefined) return false; - if (Date.now() - at < INVITE_PAIR_WINDOW_MS) return true; - marker.delete(key); - return false; - } - - private rememberDispatched_( - marker: Map, - groupId: number, - inviterUid: string, - ): void { - marker.set(`${groupId}:${inviterUid}`, Date.now()); - } - private emit(event: QQEventVariant): void { // Fire-and-forget: errors inside subscribers are surfaced via the bus's // own onError hook so one bad listener never blocks the others. @@ -550,22 +502,17 @@ export class IncomingPacketPipeline { pkt: PacketInfo, event: Extract, ): Promise { - // A self-invite never appears in the UIN-based request inbox, so there is - // nothing to enrich from and querying it only wastes a round trip. The - // private card carries the approval msgseq, but it only arrives for some - // inviters -- when it does not, the notice is the only chance to surface - // the invitation at all. Dispatch it with the decoder-provided legacy flag, - // and let the card win when both arrive (see `dispatchedInvites_`). + // A self-invite notice carries an inviter UID but no approval sequence. if ( event.subType === 'invite' && this.isSelfMemberIdentity(event.invitedUin ?? 0, event.invitedUid) ) { - if (!event.flag) { - this.log.warn('self invitation without a flag: group=%d', event.groupId); + if (!(event.groupId > 0) || !event.fromUid?.trim() || !event.flag) { + this.log.warn('self invitation missing group or inviter: group=%d', event.groupId); return 0; } - if (this.dispatchedRecently_(this.cardDispatched_, event.groupId, event.fromUid ?? '')) { + if (!this.deps.groupInvites.receiveNotice(event.groupId, event.fromUid)) { this.log.debug( 'self invitation already dispatched from the card: group=%d', event.groupId, @@ -573,7 +520,18 @@ export class IncomingPacketPipeline { return 0; } - this.rememberDispatched_(this.noticeDispatched_, event.groupId, event.fromUid ?? ''); + if (!event.fromUin) { + event.fromUin = + (await this.deps.identity.resolveUin(event.fromUid, event.groupId)) ?? 0; + + if (!event.fromUin) { + this.log.warn( + 'self invitation sender unavailable: group=%d uid=%s', + event.groupId, + event.fromUid, + ); + } + } try { return this.dispatchEvent(pkt, event, 'sync'); diff --git a/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts b/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts index 45f474c3..de015319 100644 --- a/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts +++ b/packages/protocol/tests/packet-pipeline-roster-refresh.test.ts @@ -3,6 +3,7 @@ import type { PacketInfo } from '@snowluma/common/protocol-types'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import type { QQEventVariant } from '@snowluma/protocol/events'; import { IdentityService } from '@snowluma/protocol/identity-service'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import type { GroupMemberInfo, QQGroupInfo, UserProfileInfo } from '@snowluma/protocol/qq-info'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -154,7 +155,7 @@ function makePipeline( fetchGroupList, fetchGroupMemberList, resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); return { identity, pipeline, captured, fetchGroupList, fetchGroupMemberList }; } diff --git a/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts b/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts index a3e7ada5..c57add7c 100644 --- a/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts +++ b/packages/protocol/tests/packet-pipeline-stranger-resolve.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import { IdentityService } from '@snowluma/protocol/identity-service'; @@ -49,7 +50,7 @@ function makePipeline( fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest, - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); const captured: QQEventVariant[] = []; @@ -193,23 +194,32 @@ describe('IncomingPacketPipeline / group requests', () => { ]); }); - it('dispatches a self invitation without querying the inbox', async () => { - // A self-invite never shows up in the UIN-based inbox, so the lookup is - // skipped and the decoder-provided legacy flag is what keeps it actionable. - const { pipeline, captured, resolveGroupJoinRequest, fetchProfileByUid } = makePipeline(); - await receive( - pipeline, - requestEvent({ subType: 'invite', invitedUin: 10001, flag: 'invite:12345:u_requester' }), - ); - expect(captured).toHaveLength(1); - expect(captured[0]).toMatchObject({ - kind: 'group_invite', - invitedUin: 10001, - flag: 'invite:12345:u_requester', - }); - expect(resolveGroupJoinRequest).not.toHaveBeenCalled(); - expect(fetchProfileByUid).not.toHaveBeenCalled(); - }); + it.each([76543, 0])( + 'resolves the self-invite sender without querying the inbox (profile UIN %d)', + async (uin) => { + const fetchProfileByUid = vi.fn(async () => makeProfile(uin)); + const { pipeline, captured, resolveGroupJoinRequest } = makePipeline({ + fetchProfileByUid, + }); + await receive( + pipeline, + requestEvent({ + subType: 'invite', + invitedUin: 10001, + flag: 'invite:12345:u_requester', + }), + ); + expect(captured).toHaveLength(1); + expect(captured[0]).toMatchObject({ + kind: 'group_invite', + fromUin: uin, + invitedUin: 10001, + flag: 'invite:12345:u_requester', + }); + expect(resolveGroupJoinRequest).not.toHaveBeenCalled(); + expect(fetchProfileByUid).toHaveBeenCalledExactlyOnceWith('u_requester'); + }, + ); it('does not substitute a list sequence for the self-invite card sequence', async () => { const resolveGroupJoinRequest = vi.fn(async () => @@ -289,7 +299,7 @@ describe('IncomingPacketPipeline / #1 group card self-heal', () => { fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); return { identity, pipeline }; } @@ -403,7 +413,7 @@ describe('IncomingPacketPipeline / group_card_change from message traffic', () = fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest: vi.fn(async () => null), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); const captured: QQEventVariant[] = []; events.onAny((e) => captured.push(e as QQEventVariant)); diff --git a/packages/protocol/tests/packet-pipeline-trace.test.ts b/packages/protocol/tests/packet-pipeline-trace.test.ts index 2fbd5d37..64561c38 100644 --- a/packages/protocol/tests/packet-pipeline-trace.test.ts +++ b/packages/protocol/tests/packet-pipeline-trace.test.ts @@ -13,6 +13,7 @@ import { IdentityService } from '@snowluma/protocol/identity-service'; import type { MsgPushContext } from '@snowluma/protocol/msg-push/context'; import { PkgType } from '@snowluma/protocol/msg-push/enums'; import { MsgPushRegistry } from '@snowluma/protocol/msg-push/registry'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -140,7 +141,7 @@ function makePipeline( comment: '', filtered: false, })), - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); const dispatched: Array<{ event: QQEventVariant; req: number | undefined }> = []; events.onAny((event) => { diff --git a/packages/protocol/tests/self-invite-dispatch.test.ts b/packages/protocol/tests/self-invite-dispatch.test.ts index 57407144..90f66115 100644 --- a/packages/protocol/tests/self-invite-dispatch.test.ts +++ b/packages/protocol/tests/self-invite-dispatch.test.ts @@ -2,6 +2,7 @@ import type { PacketInfo } from '@snowluma/common/protocol-types'; import { BridgeEventBus } from '@snowluma/protocol/event-bus'; import type { GroupInviteEvent, QQEventVariant } from '@snowluma/protocol/events'; import { IdentityService } from '@snowluma/protocol/identity-service'; +import { GroupInviteTracker } from '@snowluma/protocol/group-invite-tracker'; import { IncomingPacketPipeline } from '@snowluma/protocol/packet-pipeline'; import { afterEach, describe, expect, it, vi } from 'vitest'; @@ -103,7 +104,7 @@ function makePipeline(options: { request?: 'found' | 'missing' } = {}) { fetchGroupList: vi.fn(async () => {}), fetchGroupMemberList: vi.fn(async () => {}), resolveGroupJoinRequest, - rememberGroupInviteCardSequence: vi.fn(() => true), + groupInvites: new GroupInviteTracker(), }); return { pipeline, received, resolveGroupJoinRequest };