Fix issues #42 and #44, and improve settings panel usability#47
Fix issues #42 and #44, and improve settings panel usability#47friuns2 merged 3 commits intofriuns2:mainfrom
Conversation
Review Summary by QodoAdd Telegram user allowlist and improve thread persistence handling
WalkthroughsDescription• Restrict Telegram bridge to allowlisted users with configurable access control • Delay stop button until new threads persist on server to prevent data loss • Improve settings panel usability with scrolling and click-outside dismissal • Add Telegram configuration UI for managing bot token and allowed user IDs Diagramflowchart LR
A["Telegram Message"] --> B["Check Sender ID"]
B --> C{Allowed?}
C -->|Yes| D["Process Message"]
C -->|No| E["Send Rejection with User ID"]
F["New Thread Created"] --> G["Block Stop Button"]
G --> H{Thread Persisted?}
H -->|Yes| I["Unblock Stop Button"]
J["Settings Panel"] --> K["Click Outside"]
K --> L["Close Panel"]
File Changes1. src/api/codexGateway.ts
|
Code Review by Qodo
|
| type NormalizedTelegramAllowlist = { | ||
| allowAllUsers: boolean | ||
| allowedUserIds: number[] | ||
| } | ||
|
|
||
| function normalizeTelegramAllowlist(values: unknown): NormalizedTelegramAllowlist { | ||
| const rawValues = Array.isArray(values) ? values : [] | ||
| const allowAllUsers = rawValues.some((value) => typeof value === 'string' && value.trim() === '*') | ||
| const allowedUserIds = Array.from(new Set(rawValues | ||
| .map((value) => { | ||
| if (typeof value === 'number' && Number.isFinite(value)) { | ||
| return Math.trunc(value) | ||
| } | ||
| if (typeof value === 'string' && value.trim().length > 0) { | ||
| const normalized = value.trim().replace(/^(telegram|tg):/i, '').trim() | ||
| if (/^-?\d+$/.test(normalized)) { | ||
| return Number.parseInt(normalized, 10) | ||
| } | ||
| } | ||
| return Number.NaN | ||
| }) | ||
| .filter((value) => Number.isFinite(value)))).slice(0, 100) | ||
| return { allowAllUsers, allowedUserIds } | ||
| } |
There was a problem hiding this comment.
1. * bypasses telegram allowlist 📎 Requirement gap ⛨ Security
The Telegram bridge treats * in allowedUserIds as allowAllUsers, which permits any Telegram sender ID to pass validation. This violates the requirement to restrict bot access to an explicit whitelist of authorized user IDs.
Agent Prompt
## Issue description
The Telegram allowlist supports `*` to allow all users, which conflicts with the compliance requirement to restrict access to explicit authorized Telegram user IDs.
## Issue Context
`normalizeTelegramAllowlist()` sets `allowAllUsers` when `*` is present, and `isAllowedSender()` then authorizes any finite Telegram sender ID.
## Fix Focus Areas
- src/server/telegramThreadBridge.ts[73-96]
- src/server/telegramThreadBridge.ts[348-355]
- src/App.vue[234-236]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| function onDocumentPointerDown(event: PointerEvent): void { | ||
| if (!isSettingsOpen.value) return | ||
| const target = event.target | ||
| if (!(target instanceof Node)) return | ||
| if (settingsPanelRef.value?.contains(target)) return | ||
| if (settingsButtonRef.value?.contains(target)) return | ||
| isSettingsOpen.value = false | ||
| } | ||
|
|
||
| function onSettingsAreaClick(event: MouseEvent): void { | ||
| if (!isSettingsOpen.value) return | ||
| const target = event.target | ||
| if (!(target instanceof Node)) return | ||
| if (settingsPanelRef.value?.contains(target)) return | ||
| if (settingsButtonRef.value?.contains(target)) return | ||
| isSettingsOpen.value = false | ||
| } |
There was a problem hiding this comment.
2. tests.md missing settings-panel test 📘 Rule violation ⚙ Maintainability
The PR changes Settings panel behavior (scrollable panel plus close-on-outside-click/Escape) but does not add corresponding manual test instructions in tests.md. This violates the requirement to document manual tests for each implemented feature change.
Agent Prompt
## Issue description
`tests.md` is missing manual test instructions to verify the updated Settings panel usability (scrolling and closing by outside click / Escape).
## Issue Context
The PR adds new close-handlers and scroll styling for the Settings panel, which needs repeatable manual verification steps.
## Fix Focus Areas
- tests.md[1-40]
- src/App.vue[1787-1816]
- src/App.vue[3362-3364]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
|
恳请开发者手动测试一下吧,如果没有问题就合并吧谢谢。我得去写作业了。 |
修复 issue #42 和 #44,并顺手修了设置列表无法滚动的显示问题。
修复如下:
使设置列表可以滚动,并且点击列表外任意位置即可退出设置列表
增加 Telegram 用户白名单功能
*来使 Bot 对所有人开放(危险)123456即可验证:
corepack pnpm run build:frontendcorepack pnpm run build:cli