Skip to content

fix(qq): harden onebot access control #66

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
kilockok wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(qq): harden onebot access control #66

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
106 changes: 106 additions & 0 deletions src/apps/web/src/__tests__/desktopChannelsSettings.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,9 @@ async function loadChannelsSubject() {
createChannel: vi.fn(),
updateChannel: vi.fn(),
verifyChannel: vi.fn(),
listChannelBindings: vi.fn().mockResolvedValue([]),
deleteChannelBinding: vi.fn(),
updateChannelBinding: vi.fn(),
createChannelBindCode: vi.fn(),
unbindChannelIdentity: vi.fn(),
isApiError: vi.fn(() => false),
Expand Down Expand Up @@ -665,4 +668,107 @@ describe('DesktopChannelsSettings', () => {
expect(document.body.textContent).toContain('Arkloop Feishu')
expect(document.body.textContent).toContain('ou_bot')
})

it('persists QQ OneBot access control when adding a QQ user', async () => {
const { api, DesktopChannelsSettings, LocaleProvider } = await loadChannelsSubject()
const qqChannel = {
id: 'qq-1',
account_id: 'acc-1',
channel_type: 'qq',
persona_id: 'persona-1',
webhook_url: null,
is_active: true,
config_json: {
onebot_ws_url: 'ws://127.0.0.1:6098',
onebot_http_url: 'http://127.0.0.1:3000',
onebot_token: 'secret',
allowed_user_ids: ['10001'],
allowed_group_ids: ['20001'],
},
has_credentials: true,
created_at: '2026-03-26T00:00:00Z',
updated_at: '2026-03-26T00:00:00Z',
}
const updatedQQChannel = {
...qqChannel,
config_json: {
...qqChannel.config_json,
allowed_user_ids: ['10001', '30003'],
},
}
vi.mocked(api.listChannels)
.mockResolvedValueOnce([qqChannel])
.mockResolvedValue([updatedQQChannel])
vi.mocked(api.listMyChannelIdentities).mockResolvedValue([])
vi.mocked(api.listChannelPersonas).mockResolvedValue([
{
id: 'persona-1',
persona_key: 'normal',
version: '1',
display_name: 'Normal',
source: 'project',
} as never,
])
vi.mocked(api.listLlmProviders).mockResolvedValue([])
vi.mocked(api.listChannelBindings).mockResolvedValue([])
vi.mocked(api.updateChannel).mockResolvedValue(updatedQQChannel)

await act(async () => {
root!.render(
<LocaleProvider>
<DesktopChannelsSettings accessToken="token" />
</LocaleProvider>,
)
})
await flushEffects()

const qqOneBotTab = Array.from(container.querySelectorAll('button')).find((button) => button.textContent?.includes('OneBot'))
expect(qqOneBotTab).toBeTruthy()

await act(async () => {
qqOneBotTab!.dispatchEvent(new MouseEvent('click', { bubbles: true }))
})
await flushEffects()

const allowedUserInput = Array.from(document.body.querySelectorAll('input')).find((input) => input.getAttribute('placeholder')?.includes('QQ 号')) as HTMLInputElement
expect(allowedUserInput).toBeTruthy()

await act(async () => {
setInputValue(allowedUserInput, '30003')
})
await flushEffects()

const addUserButton = allowedUserInput.nextElementSibling as HTMLButtonElement
await act(async () => {
addUserButton.dispatchEvent(new MouseEvent('click', { bubbles: true }))
await Promise.resolve()
await Promise.resolve()
})
await flushEffects()

expect(api.updateChannel).toHaveBeenCalledWith('token', 'qq-1', {
persona_id: 'persona-1',
is_active: true,
config_json: {
onebot_ws_url: 'ws://127.0.0.1:6098',
onebot_http_url: 'http://127.0.0.1:3000',
onebot_token: 'secret',
allowed_user_ids: ['10001', '30003'],
allowed_group_ids: ['20001'],
},
})

await act(async () => {
document.dispatchEvent(new KeyboardEvent('keydown', { key: 'Escape', bubbles: true }))
})
await flushEffects()

const reopenedQQOneBotTab = Array.from(container.querySelectorAll('button')).find((button) => button.textContent?.includes('OneBot'))
await act(async () => {
reopenedQQOneBotTab!.dispatchEvent(new MouseEvent('click', { bubbles: true }))
})
await flushEffects()

expect(document.body.textContent).toContain('30003')
})
})
8 changes: 6 additions & 2 deletions src/apps/web/src/components/settings/DesktopChannelSettingsShared.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,18 @@ export const channelRowsCls =
export function ChannelDetailRow({
label,
children,
wide = false,
}: {
label: string
children: ReactNode
wide?: boolean
}) {
return (
<div>
<div className={wide ? 'sm:!grid-cols-[minmax(112px,132px)_minmax(0,1fr)]' : undefined}>
<div className="min-w-0 text-[13px] font-medium text-[var(--c-text-primary)]">{label}</div>
<div className="min-w-0 sm:flex sm:w-full sm:flex-col sm:items-end">{children}</div>
<div className={`min-w-0 sm:flex sm:w-full sm:flex-col ${wide ? 'sm:items-start' : 'sm:items-end'}`}>
{children}
</div>
</div>
)
}
Expand Down
124 changes: 74 additions & 50 deletions src/apps/web/src/components/settings/DesktopQQSettingsPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -155,29 +155,10 @@ export function DesktopQQSettingsPanel({
])
const canSave = dirty || channel === null

const handleAddAllowedUsers = () => {
const nextIDs = mergeListValues(allowedUserIDs, allowedUserInput)
if (nextIDs.length === allowedUserIDs.length) return
setAllowedUserIDs(nextIDs)
setAllowedUserInput('')
setSaved(false)
}

const handleAddAllowedGroups = () => {
const nextIDs = mergeListValues(allowedGroupIDs, allowedGroupInput)
if (nextIDs.length === allowedGroupIDs.length) return
setAllowedGroupIDs(nextIDs)
setAllowedGroupInput('')
setSaved(false)
}

const handleSave = async () => {
const nextAllowedUserIDs = mergeListValues(allowedUserIDs, allowedUserInput)
const nextAllowedGroupIDs = mergeListValues(allowedGroupIDs, allowedGroupInput)

const persistQQSettings = async (nextAllowedUserIDs: string[], nextAllowedGroupIDs: string[]) => {
if (enabled && !personaID) {
setError(ct.personaRequired)
return
return false
}

setSaving(true)
Expand Down Expand Up @@ -231,6 +212,7 @@ export function DesktopQQSettingsPanel({
setSaved(true)
setTimeout(() => setSaved(false), 2500)
await reload()
return true
} catch (err) {
if (err instanceof Error && err.name === 'AbortError') {
setError(ds.connectorSaveTimeout)
Expand All @@ -240,6 +222,50 @@ export function DesktopQQSettingsPanel({
} finally {
setSaving(false)
}
return false
}

const handleSave = async () => {
await persistQQSettings(
mergeListValues(allowedUserIDs, allowedUserInput),
mergeListValues(allowedGroupIDs, allowedGroupInput),
)
}

const handleAddAllowedUsers = async () => {
const nextIDs = mergeListValues(allowedUserIDs, allowedUserInput)
if (nextIDs.length === allowedUserIDs.length) return
setAllowedUserIDs(nextIDs)
setAllowedUserInput('')
setSaved(false)
await persistQQSettings(nextIDs, mergeListValues(allowedGroupIDs, allowedGroupInput))

@chatgpt-codex-connector chatgpt-codex-connector Bot May 17, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Avoid persisting draft IDs from the other access list

When adding an allowed QQ user, this call also persists mergeListValues(allowedGroupIDs, allowedGroupInput), which includes whatever is currently typed in the group input even if the user never clicked “add” for groups. That means partial or unintended group IDs can be silently committed just by adding a user (and the symmetric path does the same in reverse), causing unexpected access-control changes.

Useful? React with 👍 / 👎.

}

const handleAddAllowedGroups = async () => {
const nextIDs = mergeListValues(allowedGroupIDs, allowedGroupInput)
if (nextIDs.length === allowedGroupIDs.length) return
setAllowedGroupIDs(nextIDs)
setAllowedGroupInput('')
setSaved(false)
await persistQQSettings(mergeListValues(allowedUserIDs, allowedUserInput), nextIDs)
}

const handleRemoveAllowedUser = async (value: string) => {
const nextIDs = allowedUserIDs.filter((item) => item !== value)
setAllowedUserIDs(nextIDs)
setSaved(false)
if (channel) {
await persistQQSettings(nextIDs, mergeListValues(allowedGroupIDs, allowedGroupInput))
}
}

const handleRemoveAllowedGroup = async (value: string) => {
const nextIDs = allowedGroupIDs.filter((item) => item !== value)
setAllowedGroupIDs(nextIDs)
setSaved(false)
if (channel) {
await persistQQSettings(mergeListValues(allowedUserIDs, allowedUserInput), nextIDs)
}
}

const handleGenerateBindCode = async () => {
Expand Down Expand Up @@ -353,8 +379,10 @@ export function DesktopQQSettingsPanel({
<SettingsSwitch checked={enabled} onChange={(next) => { setEnabled(next); setSaved(false) }} />
</div>
</ChannelDetailRow>
<ChannelDetailRow label={ct.qqSetup}>
<QQLoginFlow accessToken={accessToken} channelId={channel?.id ?? ''} onStatusChange={handleNapCatStatus} />
<ChannelDetailRow label={ct.qqSetup} wide>
<div className="w-full max-w-[560px]">
<QQLoginFlow accessToken={accessToken} channelId={channel?.id ?? ''} onStatusChange={handleNapCatStatus} />
</div>
</ChannelDetailRow>
{isWindows && (
<ChannelDetailRow label={ct.qqAutoLogin}>
Expand Down Expand Up @@ -397,34 +425,30 @@ export function DesktopQQSettingsPanel({
onChange={(v) => { setOnebotToken(v); setSaved(false) }}
/>
</ChannelDetailRow>
<ChannelDetailRow label={ct.qqAllowedUsers}>
<ListField
values={allowedUserIDs}
inputValue={allowedUserInput}
placeholder={ct.qqAllowedUsersPlaceholder}
addLabel={t.skills.add}
onInputChange={setAllowedUserInput}
onAdd={handleAddAllowedUsers}
onRemove={(value) => {
setAllowedUserIDs((current) => current.filter((item) => item !== value))
setSaved(false)
}}
/>
</ChannelDetailRow>
<ChannelDetailRow label={ct.accessControl} wide>
<div className="flex w-full max-w-[560px] flex-col gap-4">
<ListField
label={ct.qqAllowedUsers}
values={allowedUserIDs}
inputValue={allowedUserInput}
placeholder={ct.qqAllowedUsersPlaceholder}
addLabel={t.skills.add}
onInputChange={setAllowedUserInput}
onAdd={handleAddAllowedUsers}
onRemove={handleRemoveAllowedUser}
/>

<ChannelDetailRow label={ct.qqAllowedGroups}>
<ListField
values={allowedGroupIDs}
inputValue={allowedGroupInput}
placeholder={ct.qqAllowedGroupsPlaceholder}
addLabel={t.skills.add}
onInputChange={setAllowedGroupInput}
onAdd={handleAddAllowedGroups}
onRemove={(value) => {
setAllowedGroupIDs((current) => current.filter((item) => item !== value))
setSaved(false)
}}
/>
<ListField
label={ct.qqAllowedGroups}
values={allowedGroupIDs}
inputValue={allowedGroupInput}
placeholder={ct.qqAllowedGroupsPlaceholder}
addLabel={t.skills.add}
onInputChange={setAllowedGroupInput}
onAdd={handleAddAllowedGroups}
onRemove={handleRemoveAllowedGroup}
/>
</div>
</ChannelDetailRow>

<ChannelDetailRow label={ct.persona}>
Expand Down
Loading
Loading