chore: auth reworked#108
Conversation
WalkthroughCentralized authentication via middleware now sets event.context.user (and 401s when absent) in both apps. Numerous API handlers removed inline auth checks and now rely on event.context.user directly. Several handlers dropped the event parameter. Many imports switched to #shared aliases. Some endpoints tweaked behavior (e.g., expanded image formats, changed like.post return, minor payload/error updates). Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant MW as Middleware (API)
participant Auth as getUserFromToken/session
participant H as Handler
participant Repo as Repository
C->>MW: HTTP request /api/...
MW->>MW: If OPTIONS or whitelisted -> skip auth
alt Auth required
MW->>Auth: Resolve current user
alt User found
Auth-->>MW: User
MW->>H: set event.context.user
H->>Repo: Perform action using context.user
Repo-->>H: Data / Result
H-->>C: Response
else No user
MW-->>C: 401 Unauthorized
end
else No auth required
MW->>H: Proceed
H->>Repo: Perform action
Repo-->>H: Data / Result
H-->>C: Response
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (9)
apps/atrium-telegram/server/api/task/list/id/[listId].delete.ts (1)
21-28: Potential crash on undefined user; also safely chain members
event.context.user.idwill throw ifuseris absent. Additionally,list.chat?.members.some(...)will throw ifmembersis undefined. Guard and use optional chaining onmembers.Apply:
- // Guard: if don't have access - const canEdit = list.chat?.members.some((member) => member.userId === event.context.user.id) + // Guard: if don't have access + const userId = event.context.user?.id + if (!userId) { + throw createError({ statusCode: 401, message: 'Not authenticated' }) + } + const canEdit = !!list.chat?.members?.some((member) => member.userId === userId)apps/atrium-telegram/server/api/task/list/index.post.ts (2)
13-19: Guard uses&&instead of||, allowing lists without the creatorIf
usersIdis non-empty but doesn’t include the requester, this won’t throw and the creator can be excluded. Use||or auto-include the creator.Minimal fix:
- if (data.usersId.length === 0 && !data.usersId.includes(event.context.user.id)) { + if (data.usersId.length === 0 || !data.usersId.includes(event.context.user.id)) { throw createError({ statusCode: 400, message: 'Must be user as a member', }) }Alternative (better UX): silently include the requester:
- if (data.usersId.length === 0 && !data.usersId.includes(event.context.user.id)) { - throw createError({ - statusCode: 400, - message: 'Must be user as a member', - }) - } + if (!data.usersId.includes(event.context.user.id)) { + data.usersId.unshift(event.context.user.id) + }
33-44: Deduplicate members and avoid in-place mutation; bulk insertPushing bot IDs into
data.usersIdcan create duplicates; the per-item awaits are slow.- const bots = await repository.user.findBots() - const botIds = bots.map((bot) => bot.id) - data.usersId.push(...botIds) + const bots = await repository.user.findBots() + const botIds = bots.map((b) => b.id) + const memberIds = Array.from(new Set([...data.usersId, ...botIds])) - for (const userId of data.usersId) { - await repository.chat.createMember({ - chatId: chat.id, - userId, - }) - } + await Promise.all( + memberIds.map((userId) => + repository.chat.createMember({ chatId: chat.id, userId }) + ) + )If the DB allows, wrap chat+members+list in a transaction to keep consistency on partial failures.
apps/atrium-telegram/server/api/task/id/[taskId]/focus.delete.ts (1)
30-36: Missing “not focused” guard; can clear focus for a different taskYou clear
focusedTaskIdwithout ensuring the user currently focuses thistaskId. Add a check (and ideally make the update conditional for atomicity).- await repository.user.update(event.context.user.id, { - focusedTaskId: null, - }) + if (event.context.user.focusedTaskId !== taskId) { + throw createError({ statusCode: 400, message: 'Task is not focused' }) + } + await repository.user.update(event.context.user.id, { + focusedTaskId: null, + })If supported, prefer a conditional update:
WHERE id = ? AND focusedTaskId = ?to avoid races.apps/atrium-telegram/server/api/task/list/id/[listId].patch.ts (1)
49-72: Cannot clear members: length > 0 gate blocks removals when usersId is [].An empty array should clear non-bot members. Current condition skips both add/remove loops.
- // Update members - if (list.chat && data.usersId.length > 0) { + // Update members (allow empty array to clear non-bot members) + if (list.chat && Array.isArray(data.usersId)) {apps/atrium-telegram/server/api/epic/comment/id/[commentId]/index.delete.ts (1)
22-25: Use 403 Forbidden instead of 400 for ownership violation.400 is not appropriate for authorization failures.
- throw createError({ - statusCode: 400, - message: 'Not your comment', - }) + throw createError({ + statusCode: 403, + message: 'Forbidden', + })apps/atrium-telegram/server/middleware/01.auth.ts (3)
41-49: Harden Authorization parsing (accept tma/Bearer and trim safely)Current split is brittle and case-sensitive. Match scheme and extract payload robustly.
- const token = getHeader(event, 'Authorization') ?? getHeader(event, 'authorization') - if (!token) { - return null - } - - const [_, authData] = token.split(' ') - if (!authData) { - return null - } + const header = getHeader(event, 'authorization')?.trim() + if (!header) return null + const m = header.match(/^(?:tma|bearer)\s+(.+)$/i) + if (!m) return null + const authData = m[1]
69-72: Don’t disable Telegram init-data expiry (expiresIn: 0)This permits replay of captured init-data indefinitely. Enforce a short TTL (e.g., 5 minutes).
-function validateTelegramData(authData: string, botToken: string): InitData | undefined { - validate(authData, botToken, { expiresIn: 0 }) +function validateTelegramData(authData: string, botToken: string): InitData | undefined { + // Enforce freshness to mitigate replay attacks (tune as needed) + validate(authData, botToken, { expiresIn: 300 }) return parse(authData) }
38-40: Add missing devBotId and select matching botId per environment
You’re usingtelegram.devBotTokenin development but still passingtelegram.atriumBotIdtofindUserByTelegramIdAndBotId, which breaks user lookup. Add adevBotIdentry underruntimeConfig.telegraminapps/atrium-telegram/nuxt.config.ts, then inapps/atrium-telegram/server/middleware/01.auth.tspickconst isDev = process.env.NODE_ENV === 'development' const botToken = isDev ? telegram.devBotToken : telegram.atriumBotToken const botId = isDev ? telegram.devBotId : telegram.atriumBotId … await repository.telegram.findUserByTelegramIdAndBotId(userId, botId)to ensure env-consistent validation.
🧹 Nitpick comments (9)
apps/atrium-telegram/server/api/task/id/[taskId]/focus.delete.ts (1)
23-28: Authorization depends onevent.context.user; ensure it’s always presentSame middleware assumption as other handlers. If optional in types, assert once or make it non-optional for these routes.
apps/atrium-telegram/server/api/task/id/[taskId]/focus.post.ts (2)
24-31: Auth assumption & performer check are fine; ensure type non-nullnessSame note: rely on middleware guaranteeing
event.context.user. If optional in types, assert once or update the context type for authenticated routes.
37-43: Consider atomic update for focus + timestamp bumpTwo separate updates can race with another focus/unfocus call. If your repo supports transactions or conditional updates, group them.
Example (conceptual):
- Update user:
WHERE id = ? AND (focusedTaskId IS NULL OR focusedTaskId != ?).- Then update task timestamp inside the same transaction.
apps/atrium-telegram/server/api/task/list/id/[listId].patch.ts (2)
64-66: Magic bot userId hardcoded.Extract to a config/constant to avoid duplication and ease rotation.
Proposed pattern (outside this diff hunk):
- Define BOT_USER_ID in shared config/env.
- Replace literal with BOT_USER_ID here and elsewhere.
38-71: Multiple writes should be transactional and set-diffed to reduce races and N+1 calls.Wrap updateList, chat metadata update, and membership add/remove in a DB transaction; compute adds/removes via set difference and batch with Promise.all/bulk ops.
apps/atrium-telegram/server/middleware/01.auth.ts (1)
10-12: Use Set for O(1) lookups and consistent checksMicro-optimization and readability; also update includes→has.
-const routesWithoutAuth = [ - '/api/health', -] +const routesWithoutAuth = new Set<string>([ + '/api/health', +]) @@ - if (!event.path.startsWith('/api') || routesWithoutAuth.includes(event.path)) { + if (!event.path.startsWith('/api') || routesWithoutAuth.has(event.path)) { return }Also applies to: 19-21
apps/atrium-telegram/server/api/epic/index.post.ts (1)
26-38: Avoid loading all users; query only staff and exclude current user at the DB layerFetching all users is unnecessary and may not scale.
- Add a repository method (or use an existing filter) to select staff only and exclude
event.context.user.id.- Benefit: less I/O, less memory, clearer intent.
apps/atrium-telegram/server/api/task/id/[taskId]/complete.post.ts (2)
33-38: Remove unnecessary double negationNo behavior change; cleaner conditional.
- if (!!task.performerId && task.performerId !== event.context.user.id) { + if (task.performerId && task.performerId !== event.context.user.id) {
82-99: Batch staff notifications with Promise.allAvoid sequential awaits in a loop.
- for (const staff of allStaffExceptUser) { - if (staff.notifications.includes('task_completed_atrium')) { - await repository.notification.create({ - authorId: event.context.user.id, - userId: staff.id, - taskId: updatedTask.id, - type: 'task_completed', - title: `${suffixByGender(['Завершил', 'Завершила'], event.context.user.gender)} задачу «${updatedTask.name}»`, - description: updatedTask.report ? updatedTask.report : 'Без отчета', - }) - } - } + const targets = allStaffExceptUser.filter((u) => + u.notifications.includes('task_completed_atrium'), + ) + await Promise.all( + targets.map((staff) => + repository.notification.create({ + authorId: event.context.user.id, + userId: staff.id, + taskId: updatedTask.id, + type: 'task_completed', + title: `${suffixByGender(['Завершил', 'Завершила'], event.context.user.gender)} задачу «${updatedTask.name}»`, + description: updatedTask.report ? updatedTask.report : 'Без отчета', + }), + ), + )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (23)
apps/atrium-telegram/server/api/auth/me.get.ts(1 hunks)apps/atrium-telegram/server/api/epic/comment/id/[commentId]/index.delete.ts(1 hunks)apps/atrium-telegram/server/api/epic/id/[epicId]/comment.post.ts(1 hunks)apps/atrium-telegram/server/api/epic/id/[epicId]/index.patch.ts(0 hunks)apps/atrium-telegram/server/api/epic/index.post.ts(2 hunks)apps/atrium-telegram/server/api/epic/list.get.ts(1 hunks)apps/atrium-telegram/server/api/notification/id/[notificationId]/viewed.post.ts(0 hunks)apps/atrium-telegram/server/api/notification/my.get.ts(1 hunks)apps/atrium-telegram/server/api/task/id/[taskId]/complete.post.ts(4 hunks)apps/atrium-telegram/server/api/task/id/[taskId]/focus.delete.ts(1 hunks)apps/atrium-telegram/server/api/task/id/[taskId]/focus.post.ts(1 hunks)apps/atrium-telegram/server/api/task/id/[taskId]/index.delete.ts(0 hunks)apps/atrium-telegram/server/api/task/id/[taskId]/index.patch.ts(2 hunks)apps/atrium-telegram/server/api/task/index.post.ts(1 hunks)apps/atrium-telegram/server/api/task/list/completed.get.ts(1 hunks)apps/atrium-telegram/server/api/task/list/id/[listId].delete.ts(1 hunks)apps/atrium-telegram/server/api/task/list/id/[listId].patch.ts(1 hunks)apps/atrium-telegram/server/api/task/list/index.get.ts(1 hunks)apps/atrium-telegram/server/api/task/list/index.post.ts(1 hunks)apps/atrium-telegram/server/api/user/list/index.get.ts(1 hunks)apps/atrium-telegram/server/api/user/list/staff.get.ts(1 hunks)apps/atrium-telegram/server/middleware/01.auth.ts(1 hunks)apps/atrium-telegram/shared/types/h3.d.ts(1 hunks)
💤 Files with no reviewable changes (3)
- apps/atrium-telegram/server/api/epic/id/[epicId]/index.patch.ts
- apps/atrium-telegram/server/api/notification/id/[notificationId]/viewed.post.ts
- apps/atrium-telegram/server/api/task/id/[taskId]/index.delete.ts
🧰 Additional context used
🧬 Code graph analysis (3)
apps/atrium-telegram/server/api/task/index.post.ts (1)
apps/atrium-telegram/shared/utils/helpers.ts (1)
suffixByGender(20-28)
apps/atrium-telegram/server/api/epic/index.post.ts (1)
packages/database/src/tables.ts (1)
users(75-93)
apps/atrium-telegram/server/api/task/id/[taskId]/complete.post.ts (2)
packages/database/src/tables.ts (1)
users(75-93)apps/atrium-telegram/shared/utils/helpers.ts (1)
suffixByGender(20-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
apps/atrium-telegram/server/api/task/id/[taskId]/index.patch.ts (1)
39-47: No guard needed forevent.context.user
H3EventContext.useris declared as a required property inapps/atrium-telegram/shared/types/h3.d.ts, soevent.context.user.idis always defined.apps/atrium-telegram/server/api/auth/me.get.ts (1)
3-5: Avoid leaking sensitive fields and restore consistent error formatting
Wrap the handler in try/catch to rethrow via errorResolver and strip out secrets from the returned user (e.g. omit password hashes or use a public-view DTO) rather than returning the raw entity.apps/atrium-telegram/server/api/epic/list.get.ts (1)
3-5: Confirm repository.epic.list() applies tenant/user scoping
Middleware in 01.auth.ts covers/api/epic/list, but you must verify thatrepository.epic.list()filters results by the authenticated user/tenant (fromevent.context.user) rather than returning all epics.apps/atrium-telegram/server/api/task/index.post.ts (1)
40-40: LGTM: use event.context.user populated by middlewareThe bot message now relies on middleware-provided user; consistent with the new auth flow.
apps/atrium-telegram/server/api/epic/id/[epicId]/comment.post.ts (1)
23-23: LGTM: source userId from event.context.userAligned with middleware-populated context; reduces duplicated auth checks in handlers.
apps/atrium-telegram/server/api/epic/index.post.ts (1)
15-16: LGTM: switch to event.context.user across the flowConsistent with type augmentation and middleware guarantees.
Also applies to: 25-36
| }) | ||
| } | ||
| if (commentInDB.userId !== user.id) { | ||
| if (commentInDB.userId !== event.context.user.id) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Unsafe access to event.context.user.id — guard against missing user.
Align with middleware assumptions but fail safe locally to avoid runtime TypeError.
- if (commentInDB.userId !== event.context.user.id) {
+ if (!event.context.user || commentInDB.userId !== event.context.user.id) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (commentInDB.userId !== event.context.user.id) { | |
| if (!event.context.user || commentInDB.userId !== event.context.user.id) { | |
| // existing logic… | |
| } |
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/epic/comment/id/[commentId]/index.delete.ts
around line 21, the code directly accesses event.context.user.id which can be
undefined and cause a TypeError; add a defensive guard that verifies
event.context and event.context.user exist and that user.id is present before
comparing to commentInDB.userId, and if the user is missing return/throw an
unauthorized response (HTTP 401) or appropriate error; update the comparison to
run only after this check so the handler fails safely when no authenticated user
is present.
| export default defineEventHandler(async (event) => { | ||
| try { | ||
| const user = event.context.user | ||
| if (!user) { | ||
| throw createError({ | ||
| statusCode: 401, | ||
| message: 'Not logged in', | ||
| }) | ||
| } | ||
|
|
||
| return repository.notification.listByUser(user.id) | ||
| } catch (error) { | ||
| throw errorResolver(error) | ||
| } | ||
| return repository.notification.listByUser(event.context.user.id) | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Potential NPE on event.context.user and inconsistent error handling.
Ensure middleware guards this route; consider consistent try/catch with errorResolver like other handlers.
Run to confirm middleware coverage and existing error handling conventions:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n 'errorResolver\(' apps/atrium-telegram/server/api | head -n 50
fd -a '01.auth.ts' apps | xargs -I{} sed -n '1,220p' "{}" | rg -n 'event\.context\.user'Length of output: 1665
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Locate and display auth middleware file
auth_file=$(fd '01.auth.ts' apps/atrium-telegram)
echo "Auth middleware file: $auth_file"
sed -n '1,200p' "$auth_file"
# Locate and display Nuxt config file
nuxt_cfg=$(fd 'nuxt.config.ts' apps/atrium-telegram)
echo "Nuxt config file: $nuxt_cfg"
sed -n '1,200p' "$nuxt_cfg"
# Search for middleware registration in config
rg -n 'middleware' -C 3 "$nuxt_cfg"
# List server and middleware directories
find apps/atrium-telegram/server -maxdepth 2 -type d
# Confirm usage of event.context.user across codebase
rg -n 'event\.context\.user' -C 2 apps/atrium-telegramLength of output: 3459
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Display the handler implementation
sed -n '1,200p' apps/atrium-telegram/server/api/notification/my.get.ts
# Check for existing error handling patterns
rg -n 'try' -C 3 apps/atrium-telegram/server/api/notification/my.get.ts || true
rg -n 'errorResolver' -C 3 apps/atrium-telegram/server/api/notification/my.get.ts || trueLength of output: 432
Add try/catch with errorResolver
Wrap the repository call in a try { … } catch (error) { throw errorResolver(error) } to align with other handlers; the global auth middleware (server/middleware/01.auth.ts) already guarantees event.context.user is set for /api/** routes. [apps/atrium-telegram/server/api/notification/my.get.ts]
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/notification/my.get.ts around lines 3 to 5,
the repository call is unprotected and should be wrapped in a try/catch that
uses errorResolver; update the handler to call
repository.notification.listByUser(event.context.user.id) inside try and in
catch throw errorResolver(error) so errors are normalized like other handlers
(no need to re-check event.context.user because auth middleware guarantees it).
| title: `${suffixByGender(['Завершил', 'Завершила'], event.context.user.gender)} задачу «${updatedTask.name}»`, | ||
| description: updatedTask.report ? updatedTask.report : 'Без отчета', | ||
| }) |
There was a problem hiding this comment.
Missing import for suffixByGender → runtime/TS error
suffixByGender is used but not imported.
-import { getLocalizedResolution } from '#shared/utils/helpers'
+import { getLocalizedResolution, suffixByGender } from '#shared/utils/helpers'Also applies to: 106-117
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/task/id/[taskId]/complete.post.ts around
lines 93-95 (also where similar usage occurs at 106-117), suffixByGender is
referenced but not imported causing a runtime/TypeScript error; fix by adding
the appropriate named import for suffixByGender from the module where it’s
defined (for example add: import { suffixByGender } from
'<correct-module-path>') at the top of the file so both occurrences resolve at
runtime.
| const bot = await repository.chat.findNotificationBot(list.chat.id) | ||
| if (bot) { | ||
| const text = prepareBotMessage(user, task, updatedTask, updatedPerformer) | ||
| const text = prepareBotMessage(event.context.user, task, updatedTask, updatedPerformer) | ||
|
|
||
| // Send message as bot | ||
| await repository.chat.createMessage({ |
There was a problem hiding this comment.
🛠️ Refactor suggestion
prepareBotMessage likely needs a fully-loaded User
event.context.user must have name, surname, and gender. If middleware only attaches minimal claims, bot text will degrade (e.g., undefined undefined). Prefer fetching the author or enriching middleware.
- const text = prepareBotMessage(event.context.user, task, updatedTask, updatedPerformer)
+ const author = await repository.user.find(event.context.user.id)
+ if (!author) {
+ throw createError({ statusCode: 500, message: 'Author not found' })
+ }
+ const text = prepareBotMessage(author, task, updatedTask, updatedPerformer)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const bot = await repository.chat.findNotificationBot(list.chat.id) | |
| if (bot) { | |
| const text = prepareBotMessage(user, task, updatedTask, updatedPerformer) | |
| const text = prepareBotMessage(event.context.user, task, updatedTask, updatedPerformer) | |
| // Send message as bot | |
| await repository.chat.createMessage({ | |
| const bot = await repository.chat.findNotificationBot(list.chat.id) | |
| if (bot) { | |
| // Ensure we have a fully-loaded author for bot message formatting | |
| const author = await repository.user.find(event.context.user.id) | |
| if (!author) { | |
| throw createError({ statusCode: 500, message: 'Author not found' }) | |
| } | |
| const text = prepareBotMessage(author, task, updatedTask, updatedPerformer) | |
| // Send message as bot | |
| await repository.chat.createMessage({ |
| export default defineEventHandler(async () => { | ||
| return repository.task.findAll() | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Completed list returns all tasks — likely privacy leak and incorrect semantics.
Using repository.task.findAll() from a “completed” endpoint (and without user context) likely returns tasks beyond the caller and includes non-completed items.
Run to find the correct repository API for completed-by-user:
Proposed fix (adjust the repository call to the actual method/shape):
-export default defineEventHandler(async () => {
- return repository.task.findAll()
+export default defineEventHandler(async (event) => {
+ // Ensure only this user's completed tasks are returned (replace with the actual repo API).
+ return repository.task.listByUser(event.context.user.id, { status: 'COMPLETED' })
})🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Discover task repo methods used elsewhere
rg -nC3 -e 'repository\.task\.' -e '\bCOMPLETED\b' apps | sed -n '1,200p'
# Specifically look for "completed" or "by user" helpers
rg -nC2 -e 'completed|complete' -e 'byUser|listByUser|findByUser|userId' appsLength of output: 87289
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== repository.task methods in atrium-telegram ==="
rg -nC3 'repository\.task\.' -g 'apps/atrium-telegram/**'
echo
echo "=== Files under apps/atrium-telegram/server/api/task/list ==="
find apps/atrium-telegram/server/api/task/list -maxdepth 1 -type fLength of output: 12360
Filter completed tasks by current user and status
Change apps/atrium-telegram/server/api/task/list/completed.get.ts to accept event, extract event.context.user.id, and replace repository.task.findAll() with a query that only returns tasks whose completedAt is set and whose performerId matches the current user—either by passing an appropriate filter to findAll (e.g. repository.task.findAll({ where: { performerId: userId, completedAt: { not: null } } })) or by adding a repository.task.listCompletedByUser helper. [apps/atrium-telegram/server/api/task/list/completed.get.ts:3-5]
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/task/list/completed.get.ts around lines 3 to
5, the handler currently returns all tasks; update it to accept the event
parameter, extract the current user's id from event.context.user.id, and replace
repository.task.findAll() with a query that filters tasks where performerId
equals that userId and completedAt is not null (e.g. repository.task.findAll({
where: { performerId: userId, completedAt: { not: null } } }) ) or call a new
repository.task.listCompletedByUser(userId) helper that implements the same
filter.
| export default defineEventHandler(async () => { | ||
| return repository.task.lists() | ||
| }) |
There was a problem hiding this comment.
Likely data scope leak: returns all task lists, not caller-scoped
Dropping event removed the ability to scope results. Unless the repository filters by caller implicitly, this exposes all lists. Re-introduce user scoping and consistent error handling.
Proposed change (adjust repo call to your API):
-export default defineEventHandler(async () => {
- return repository.task.lists()
-})
+export default defineEventHandler(async (event) => {
+ try {
+ const userId = event.context.user?.id
+ if (!userId) {
+ throw createError({ statusCode: 401, message: 'Not authenticated' })
+ }
+ // Ensure repo returns only lists the caller can access
+ return (await repository.task.lists?.({ memberUserId: userId }))
+ ?? (await repository.task.listsForUser?.(userId))
+ } catch (error) {
+ throw errorResolver(error)
+ }
+})Also consider standardizing on either per-route try/catch with errorResolver or a global error middleware for consistency across endpoints.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default defineEventHandler(async () => { | |
| return repository.task.lists() | |
| }) | |
| export default defineEventHandler(async (event) => { | |
| try { | |
| const userId = event.context.user?.id | |
| if (!userId) { | |
| throw createError({ statusCode: 401, message: 'Not authenticated' }) | |
| } | |
| // Ensure repo returns only lists the caller can access | |
| return (await repository.task.lists?.({ memberUserId: userId })) | |
| ?? (await repository.task.listsForUser?.(userId)) | |
| } catch (error) { | |
| throw errorResolver(error) | |
| } | |
| }) |
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/task/list/index.get.ts around lines 3 to 5,
the handler currently returns all task lists because the event parameter was
removed; reintroduce the event param, extract the caller identity from the event
(e.g., event.context.user?.id or the auth token payload used in this project),
and pass that caller/user id into the repository call (e.g.,
repository.task.lists({ callerId: <extracted id> }) or
repository.task.lists(event) depending on repo API) so results are scoped to the
requester; also wrap the call in a try/catch and forward errors through the
project’s standard error handler (use errorResolver or the global pattern used
elsewhere) to keep error handling consistent.
| export default defineEventHandler(async () => { | ||
| return repository.user.list() | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
User enumeration risk: no auth/RBAC checks and unrestricted listing
This endpoint now lists users without consulting event.context.user. If middleware ever misconfigures or doesn’t enforce role checks, this can leak PII. At minimum, ensure:
- Route is not in routesWithoutAuth.
- Only privileged roles can list users.
- Returned fields are whitelisted (no emails/tokens/etc.).
Proposed hardening (adapt role check and selector to your repo):
-export default defineEventHandler(async () => {
- return repository.user.list()
-})
+export default defineEventHandler(async (event) => {
+ const user = event.context.user
+ if (!user) {
+ throw createError({ statusCode: 401, message: 'Not authenticated' })
+ }
+ // Enforce RBAC
+ const isStaff = await repository.user.isStaff?.(user.id)
+ if (!isStaff) {
+ throw createError({ statusCode: 403, message: 'Forbidden' })
+ }
+ // Prefer a safe projection from the repo layer
+ return repository.user.listPublic?.() ?? repository.user.list()
+})Run to verify middleware coverage and exposure:
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# 1) Confirm these routes are NOT exempted from auth middleware
echo "== routesWithoutAuth entries in 01.auth.ts =="
fd -a '01.auth.ts' apps | while read -r f; do
echo "-- $f"
rg -nP -C2 '(?s)routesWithoutAuth\s*=\s*\[.*?\]' "$f" || true
done
echo
echo "== Check for listed endpoints referenced in routesWithoutAuth =="
rg -n "/api/user/list|/api/user/list/staff|/api/task/list" apps/atrium-telegram/server/middleware/01.auth.ts || true
# 2) Ensure middleware sets event.context.user before handlers
rg -n "event\.context\.user\s*=" apps/atrium-telegram/server/middleware/01.auth.ts || true
# 3) Surface other handlers that access event.context.user directly
rg -nP -C2 '\bevent\.context\.user\b' apps/atrium-telegram/server/api | sed 's/^/API: /'Length of output: 14011
🏁 Script executed:
#!/bin/bash
sed -n '1,200p' apps/atrium-telegram/server/middleware/01.auth.tsLength of output: 1901
Enforce RBAC on user listing endpoint
The /api/user/list handler is behind auth middleware but currently allows any authenticated user to fetch all user records (including PII). Add a check in this handler to
- accept the
eventparameter and ensureevent.context.userexists (401 if missing) - verify the caller’s role (e.g.
if (event.context.user.type !== 'staff') throw createError({ statusCode: 403 })) - use a safe projection method (e.g.
repository.user.listPublic()) to whitelist returned fields
🤖 Prompt for AI Agents
In apps/atrium-telegram/server/api/user/list/index.get.ts around lines 3 to 5,
the handler currently returns repository.user.list() and does not enforce RBAC;
modify the handler signature to accept the event parameter, verify
event.context.user exists and throw createError({ statusCode: 401 }) if missing,
check the caller’s role (e.g. if (event.context.user.type !== 'staff') throw
createError({ statusCode: 403 })), and replace repository.user.list() with a
safe projection method such as repository.user.listPublic() to whitelist
returned fields.
| export default defineEventHandler(async () => { | ||
| return repository.user.findStaff() | ||
| }) |
There was a problem hiding this comment.
Missing explicit staff/admin guard on a staff-only listing
As written, any authenticated user (or a misconfigured route) could hit this and enumerate staff. Enforce role-based access and keep a safe field projection from the repo.
Suggested hardening:
-export default defineEventHandler(async () => {
- return repository.user.findStaff()
-})
+export default defineEventHandler(async (event) => {
+ const user = event.context.user
+ if (!user) {
+ throw createError({ statusCode: 401, message: 'Not authenticated' })
+ }
+ const isStaff = await repository.user.isStaff?.(user.id)
+ if (!isStaff) {
+ throw createError({ statusCode: 403, message: 'Forbidden' })
+ }
+ return repository.user.findStaff()
+})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export default defineEventHandler(async () => { | |
| return repository.user.findStaff() | |
| }) | |
| export default defineEventHandler(async (event) => { | |
| const user = event.context.user | |
| if (!user) { | |
| throw createError({ statusCode: 401, message: 'Not authenticated' }) | |
| } | |
| const isStaff = await repository.user.isStaff?.(user.id) | |
| if (!isStaff) { | |
| throw createError({ statusCode: 403, message: 'Forbidden' }) | |
| } | |
| return repository.user.findStaff() | |
| }) |
| declare module 'h3' { | ||
| interface H3EventContext { | ||
| user: User | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make event.context.user optional (or null) to reflect unauthenticated routes and avoid runtime NPEs
Middleware may intentionally allow some routes without auth. With a non-optional user: User, TS suggests presence even when it’s absent at runtime, risking Cannot read properties of undefined when handlers access .id. Also, avoid coupling H3 context to the full DB User shape; keep only what handlers actually need (e.g., id).
Apply:
-import type { User } from '@roll-stack/database'
+import type { User } from '@roll-stack/database'
declare module 'h3' {
interface H3EventContext {
- user: User
+ // Minimal and optional; narrow at use sites.
+ user?: Pick<User, 'id'>
}
}If you guarantee a value for every request in middleware, set it to null for guests and type as user: Pick<User, 'id'> | null instead.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| declare module 'h3' { | |
| interface H3EventContext { | |
| user: User | |
| } | |
| import type { User } from '@roll-stack/database' | |
| declare module 'h3' { | |
| interface H3EventContext { | |
| // Minimal and optional; narrow at use sites. | |
| user?: Pick<User, 'id'> | |
| } | |
| } |
🤖 Prompt for AI Agents
In apps/atrium-telegram/shared/types/h3.d.ts around lines 3 to 6, the
H3EventContext currently forces a full User on every request which misrepresents
unauthenticated routes and risks runtime NPEs; change the declaration so
event.context.user is optional or nullable and narrow its shape to only what
handlers need (e.g., user: Pick<User,'id'> | null) instead of coupling to the
full DB User; if your middleware guarantees a value for every request, set
guests to null and type as Pick<User,'id'> | null, otherwise make the property
optional (user?: Pick<User,'id'>) so TypeScript and runtime behavior align.
|
There was a problem hiding this comment.
Actionable comments posted: 19
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (37)
apps/web-app/server/api/user/id/[userId]/telegram.post.ts (3)
15-19: Normalize validation failure to HTTP 400 and avoid throwing raw library objects.Throwing the arktype errors object leaks internals and relies on errorResolver to know that shape. Wrap as 400 with details.
- const body = await readBody(event) - const data = attachTelegramSchema(body) - if (data instanceof type.errors) { - throw data - } + const body = await readBody(event) + const data = attachTelegramSchema(body) + if (data instanceof type.errors) { + throw createError({ + statusCode: 400, + message: 'Invalid request payload', + data, + }) + }
29-40: Make the “key not yet used” check atomic to avoid TOCTOU races.Between the check and update, another request can claim the same key. Use a single conditional update (WHERE userId IS NULL) or a repository method that returns a success flag. Also surface 409 when the claim fails.
- // Guard: if accessKey is already used - if (user.userId) { - throw createError({ - statusCode: 400, - message: 'Key is already used', - }) - } - - await repository.telegram.updateUser(user.id, { - userId, - }) + // Atomically claim the key if unassigned + const claimed = await repository.telegram.claimUserIfUnassigned?.(user.id, userId) + if (claimed === false) { + throw createError({ + statusCode: 409, + message: 'Key is already used', + }) + } + if (claimed == null) { + // Fallback if repo lacks the atomic helper + if (user.userId) { + throw createError({ statusCode: 409, message: 'Key is already used' }) + } + await repository.telegram.updateUser(user.id, { userId }) + }If helpful, I can add:
- repo method claimUserIfUnassigned(id, userId) performing UPDATE ... WHERE id = $1 AND userId IS NULL RETURNING *
- DB unique/partial index to enforce one claim (e.g., UNIQUE(user_id) or constraint on access_key + user_id IS NULL)
5-13: Enforce user-scoped access in all/api/user/id/[userId]handlers
Handlersindex.get.ts,complete.post.ts,index.patch.ts, andtelegram.post.tsretrieve theuserIdpath param but don’t verify it againstevent.context.user.id, leaving an IDOR gap. Afterconst userId = getRouterParam(event, 'userId'), add:const authUserId = event.context.user?.id if (!authUserId) throw createError({ statusCode: 401, message: 'Unauthorized' }) if (authUserId !== userId) throw createError({ statusCode: 403, message: 'Forbidden: cannot operate on another user’s data' })(Note:
image.post.tsandonline.post.tsalready enforce this.)apps/web-app/server/api/task/id/[taskId]/focus.delete.ts (2)
38-41: Don’t clear focus if another task is currently focused (idempotency/logic bug)Current code nulls focus regardless of which task is focused. Only clear when the focused task matches the requested task; otherwise no-op.
Apply:
- await repository.user.update(user.id, { - focusedTaskId: null, - }) + if (user.focusedTaskId === task.id) { + await repository.user.update(user.id, { focusedTaskId: null }) + } else { + // Idempotent no-op: nothing to delete for this task + return { ok: true } + }
42-44: Touch timestamps instead of empty updates
- In
apps/web-app/server/api/task/id/[taskId]/focus.post.ts(line 50),focus.delete.ts(line 43), and the matching endpoints inapps/atrium-telegram(focus.post.ts line 42, focus.delete.ts line 35), replacewith a real field update (e.g.await repository.task.update(task.id, {}){ updatedAt: new Date() }) or introduce/use arepository.task.touch(id)method to bump the timestamp.apps/web-app/server/api/partner/agreement/index.post.ts (2)
7-9: Re-enable and fix permission guard (authz regression).Commented-out hasPermission means any authenticated user can create agreements; scope string also mismatches this route.
Apply:
- // await hasPermission(event, 'product:delete') + await hasPermission(event, 'partner:agreement:create')If your permission catalog differs, swap the scope accordingly.
17-21: Return a server error on repository failure, not 400.Creation failing after validation is a server-side issue.
- throw createError({ - statusCode: 400, - message: 'Unable to create', - }) + throw createError({ + statusCode: 500, + message: 'Unable to create partner agreement', + })apps/web-app/server/api/menu/category/id/[categoryId]/product.post.ts (3)
6-7: Add permission check for write operation.Attaching a product to a category should be permissioned.
export default defineEventHandler(async (event) => { try { + await hasPermission(event, 'menu:edit')
31-35: Use 409 Conflict when product already in category.More accurate than 400 for duplicate association.
- throw createError({ - statusCode: 400, - message: 'Product is already in category', - }) + throw createError({ + statusCode: 409, + message: 'Product is already in category', + })
21-38: Enforce DB-level uniqueness for category–product pairs
Relying on the in-memory guard in product.post.ts is unsafe under concurrent requests. Inpackages/database/src/tables.ts, add a unique composite index onmenuCategoryIdandproductIdfor theproducts_in_menu_categoriestable, run a migration, and updateattachProduct(or the POST handler) to catch the constraint violation and return HTTP 409.apps/web-app/server/api/menu/category/index.post.ts (1)
6-9: Add permission check for category creation.export default defineEventHandler(async (event) => { try { + await hasPermission(event, 'menu:edit') const body = await readBody(event)apps/web-app/server/api/post/id/[postId]/image.post.ts (1)
40-40: Bug: Passing underlying ArrayBuffer can corrupt decode; pass Buffer directly to sharp.
file.data.buffermay include extra bytes whenfile.datais a slice. Feed the Buffer itself.Apply:
- sharpStream = sharp(file.data.buffer as ArrayBuffer) + sharpStream = sharp(file.data as Buffer)apps/web-app/server/api/partner/legal/id/[entityId]/index.patch.ts (1)
7-7: Restore permission check in partner legal patch handler (apps/web-app/server/api/partner/legal/id/[entityId]/index.patch.ts:7)
Permission is commented out—this endpoint must enforce authorization. Replace:- // await hasPermission(event, 'product:edit') + await hasPermission(event, 'partner:edit') // or 'partner:legal:edit' if you use granular scopesapps/web-app/server/api/partner/legal/index.post.ts (1)
7-7: Restore proper authorization check with the correct scope.Creating a partner legal entity should enforce authorization. The current permission is commented and references an unrelated scope.
Apply:
- // await hasPermission(event, 'product:delete') + await hasPermission(event, 'partner:legal:create')apps/web-app/server/api/partner/agreement/id/[agreementId]/index.patch.ts (2)
7-7: Reinstate authorization with a scoped permission.Editing an agreement should be protected; current check is commented and references product scope.
- // await hasPermission(event, 'product:edit') + await hasPermission(event, 'partner:agreement:edit')
23-31: Add ownership/tenant check before update.Ensure the agreement belongs to the caller’s tenant/partner; otherwise this is an escalation vector.
const agreement = await repository.partner.findAgreement(agreementId) if (!agreement) { throw createError({ statusCode: 404, message: 'Agreement not found', }) } + // Ensure current user can operate on this agreement + await ensureAgreementAccess(event.context.user, agreement)If no helper exists, implement an
ensureAgreementAccessguard (e.g., checkagreement.partnerIdvs user’s allowed partner IDs).apps/web-app/server/api/product/id/[productId]/index.patch.ts (2)
23-33: Return 404 when update hits a non-existent product.If
updatecan return null/undefined, surface a proper 404 instead of{ ok: true, result: null }.- const product = await repository.product.update(productId, data) + const product = await repository.product.update(productId, data) + if (!product) { + throw createError({ statusCode: 404, message: 'Product not found' }) + }
18-28: Exclude write-only extras from the main update payload.If
tagsIdisn’t a column onproduct, keep it out of the update object to avoid accidental persistence or schema errors.- const data = updateProductSchema(body) + const data = updateProductSchema(body) + const { tagsId, ...productData } = data - const product = await repository.product.update(productId, data) + const product = await repository.product.update(productId, productData) - if (data.tagsId) { - await repository.product.updateTags(productId, data.tagsId) + if (tagsId) { + await repository.product.updateTags(productId, tagsId)apps/web-app/server/api/partner/id/[partnerId].patch.ts (1)
6-8: Critical: authorization removed; reintroduce correct permission.Updates to partners must be guarded. The commented
product:editis also the wrong scope.export default defineEventHandler(async (event) => { try { - // await hasPermission(event, 'product:edit') + await hasPermission(event, 'partner:edit')If this is tenant-scoped, also verify the partner belongs to the requester’s org before updating.
apps/web-app/server/api/epic/comment/id/[commentId]/index.delete.ts (1)
21-26: Use 403 (or unify to 404) instead of 400 for authorization failure.400 is not appropriate here. Prefer 403 Forbidden; optionally return 404 to avoid ID enumeration.
- if (commentInDB.userId !== event.context.user.id) { - throw createError({ - statusCode: 400, - message: 'Not your comment', - }) - } + if (commentInDB.userId !== event.context.user.id) { + throw createError({ + statusCode: 403, + message: 'Not your comment', + }) + }apps/web-app/server/api/task/list/index.post.ts (1)
14-19: Fix guard logic:&&should be||(allows excluding the current user).As written, non-empty arrays that omit the current user pass the check. Use OR.
- if (data.usersId.length === 0 && !data.usersId.includes(event.context.user.id)) { + if (data.usersId.length === 0 || !data.usersId.includes(event.context.user.id)) { throw createError({ statusCode: 400, message: 'Must be user as a member', }) }Optional: auto-include the current user instead of erroring.
- if (data.usersId.length === 0 || !data.usersId.includes(event.context.user.id)) { - throw createError({ statusCode: 400, message: 'Must be user as a member' }) - } + if (!data.usersId.includes(event.context.user.id)) { + data.usersId.unshift(event.context.user.id) + }apps/web-app/server/api/activity/schedule/item/id/[itemId].patch.ts (1)
1-39: Enforce access control before mutating data. ThefindScheduleItemandupdateScheduleItemmethods inpackages/database/src/repository/activity.tsdo not perform any permission checks, so you must add an explicit guard in apps/web-app/server/api/activity/schedule/item/id/[itemId].patch.ts to verify the caller’s rights (e.g. ownership or role) and return 403 if unauthorized before invokingupdateScheduleItem.apps/web-app/server/api/user/id/[userId]/index.patch.ts (1)
16-24: Add authorization: prevent cross-user updates.
This route allows updating any user by ID with no check against the caller. Enforce self-update (and optionally admin) before proceeding.Apply this diff just after the 404-not-found guard:
if (!user?.id) { throw createError({ statusCode: 404, message: 'User not found', }) } + // Authorization: only the actor themselves (or an admin, if applicable) + const actorId = event.context.user.id + if (actorId !== userId) { + throw createError({ + statusCode: 403, + message: 'Forbidden', + }) + }apps/web-app/server/api/task/list/id/[listId].patch.ts (1)
48-72: Member sync logic misses removals on empty input and may duplicate adds; refactor set-diff.
- Skips removals when
usersIdis an empty array (can’t clear members).- Potential duplicates in
usersId.- Hard-coded bot ID sprinkled in loop; centralize and exclude from diffs.
- // Update members - if (list.chat && data.usersId.length > 0) { - // Check if have new Id's - for (const userId of data.usersId) { - const member = list.chat?.members.find((member) => member.userId === userId) - if (!member?.id) { - await repository.chat.createMember({ - chatId: list.chat.id, - userId, - }) - } - } - - // Check if have removed Id's - for (const member of list.chat.members) { - // Bot? - if (member.userId === 'fsti10ba0cb7uxkal4uoja9r') { - continue - } - - if (!data.usersId.includes(member.userId)) { - await repository.chat.deleteMember(member.id) - } - } - } + // Update members + if (list.chat && Array.isArray(data.usersId)) { + const BOT_ID = 'fsti10ba0cb7uxkal4uoja9r' + const desired = new Set(data.usersId.filter((id) => id && id !== BOT_ID)) + const current = new Set( + (list.chat.members ?? []) + .filter((m) => m.userId !== BOT_ID) + .map((m) => m.userId), + ) + + // Adds + for (const userId of desired) { + if (!current.has(userId)) { + await repository.chat.createMember({ chatId: list.chat.id, userId }) + } + } + + // Removals (supports empty desired set to clear all non-bot members) + for (const member of list.chat.members ?? []) { + if (member.userId !== BOT_ID && !desired.has(member.userId)) { + await repository.chat.deleteMember(member.id) + } + } + }apps/web-app/server/api/task/index.post.ts (3)
14-20: Fix ordering to avoid orphaned tasks when list lookup fails.You create the task before verifying the list exists. If
findListreturns null, the handler throws after persisting an orphaned task. Lookup the list first, then create the task.Apply:
- const task = await repository.task.create({ - name: data.name, - description: data.description, - date: data.date, - performerId: data.performerId, - listId: data.listId, - }) - if (!task) { - throw createError({ - statusCode: 500, - message: 'Task not created', - }) - } - - const list = await repository.task.findList(task.listId) + const list = await repository.task.findList(data.listId) if (!list) { throw createError({ statusCode: 500, message: 'Task list not found', }) } + + const task = await repository.task.create({ + name: data.name, + description: data.description, + date: data.date, + performerId: data.performerId, + listId: data.listId, + }) + if (!task) { + throw createError({ + statusCode: 500, + message: 'Task not created', + }) + }Also applies to: 28-34
14-20: Enforce per‐list access and performer‐assignment permissions
After retrieving the list viarepository.task.findList(task.listId), verify thatevent.context.user.idis authorized to create tasks in that list (e.g., membership/role check) and return a 403 if not. Likewise, ensuredata.performerIdis either the caller’s own ID or that the caller holds explicit permission to assign tasks to others; otherwise, throw a 403.
10-12: Use.assertto validate and throw on schema errors.Replace
const data = createTaskSchema(body) if (data instanceof type.errors) throw datawith
const data = createTaskSchema.assert(body)apps/web-app/server/api/ticket/id/[ticketId]/message.post.ts (2)
24-35: Add authorization: restrict posting to permitted users.After loading the ticket, ensure the caller can post (owner, assignee, or member). Otherwise any authenticated user could post to arbitrary tickets.
const ticket = await repository.ticket.find(ticketId) if (!ticket) { throw createError({ statusCode: 404, message: 'Ticket not found', }) } + // Minimal guard; replace with role/membership check as applicable + if (ticket.userId !== event.context.user.id) { + throw createError({ + statusCode: 403, + message: 'Forbidden', + }) + } + const message = await repository.ticket.createMessage({ ticketId, userId: event.context.user.id, text: data.text, })
20-22: Use.assertinstead of manualinstanceof type.errors. Replace the error‐check block withconst data = createTicketMessageSchema.assert(body)to auto‐throw on validation failures. (arktype.io)
apps/web-app/server/api/task/id/[taskId]/index.patch.ts (1)
39-47: Fix optional-chaining bug: possible runtime TypeError when chat is undefined
list.chat?.members.some(...)attempts.someon undefined whenchatis nullish. Use chained optional access and default to false.Apply:
- const canEdit = list.chat?.members.some((member) => member.userId === event.context.user?.id) + const canEdit = list.chat?.members?.some((member) => member.userId === event.context.user!.id) ?? falseapps/web-app/server/api/task/list/id/[listId].delete.ts (1)
22-28: Fix optional-chaining bug in access checkSame issue:
.someon possibly-undefinedmembers.Apply:
- const canEdit = list.chat?.members.some((member) => member.userId === event.context.user.id) + const canEdit = list.chat?.members?.some((member) => member.userId === event.context.user!.id) ?? falseapps/web-app/server/api/user/id/[userId]/image.post.ts (2)
37-45: Enforce upload size limit to prevent memory abuseAdd a max bytes check (e.g., 10MB) before feeding data to sharp.
Apply:
const files = await readMultipartFormData(event) const file = files?.[0] if (!files?.length || !file) { throw createError({ statusCode: 400, message: 'Missing file', }) } + // 10MB limit + const maxBytes = 10 * 1024 * 1024 + const fileSize = (file.data as Buffer)?.length ?? 0 + if (fileSize > maxBytes) { + throw createError({ statusCode: 413, message: 'File too large' }) + }
49-51: Pass Buffer to sharp (avoid using underlying ArrayBuffer)
file.data.buffercan include unrelated bytes due to offset/length; sharp accepts Buffer directly.Apply:
- sharpStream = sharp(file.data.buffer as ArrayBuffer) + sharpStream = sharp(file.data as Buffer)apps/web-app/server/api/chat/id/[chatId]/message/index.post.ts (1)
21-33: Missing membership check: any logged-in user could post to any chat.
Validate the author is a member before creating the message.Insert before message creation:
const chat = await repository.chat.findWithEntities(chatId) if (!chat) { throw createError({ statusCode: 404, message: 'Chat not found', }) } + // Guard: only members can post + const isMember = await repository.chat.isMember(chatId, event.context.user.id) + if (!isMember) { + throw createError({ statusCode: 403, message: 'Forbidden' }) + } const message = await repository.chat.createMessage({ chatId, userId: event.context.user.id, text: data.text, })apps/web-app/server/api/user/id/[userId]/online.post.ts (1)
22-28: Prevent user enumeration: check authorization before existence.Current order lets an authenticated attacker probe if any userId exists (404 vs 403). First verify actor matches target, then proceed.
Apply:
- // Guard: if no user - const user = await repository.user.find(userId) - if (!user?.id) { - throw createError({ - statusCode: 404, - message: 'User not found', - }) - } - - // Guard: if not this user - if (event.context.user.id !== userId) { + // Guard: only the user can mark themselves online + if (event.context.user.id !== userId) { throw createError({ statusCode: 403, message: 'Forbidden', }) } + + // Optional: verify own record exists (won't leak others) + const user = await repository.user.find(userId) + if (!user?.id) { + throw createError({ statusCode: 404, message: 'User not found' }) + }Also applies to: 13-21
apps/web-app/server/api/post/id/[postId]/like.delete.ts (1)
14-20: Use 404 or make deletion idempotent; current 400 is semantically weak.If no like for this user/post, 404 Not Found (or 200 OK for idempotent delete) is conventional. Also avoid TOCTOU by deleting with both keys in one statement when possible.
Minimal status fix:
- throw createError({ - statusCode: 400, - message: 'Not your like', - }) + throw createError({ + statusCode: 404, + message: 'Like not found', + })Idempotent pattern (if repo supports it):
- const likeInDB = await repository.post.findLike(postId, event.context.user.id) - if (!likeInDB) { - throw createError({ statusCode: 404, message: 'Like not found' }) - } - await repository.post.deleteLike(likeInDB.id) + const deleted = await repository.post.deleteLikeByPostAndUser(postId, event.context.user.id) + // deleted could be affected row count; return OK regardless for idempotencyapps/web-app/server/api/task/id/[taskId]/complete.post.ts (1)
23-29: Remove redundant DB fetch; trust middleware-populated user.Middleware already attaches a fully-hydrated user to event.context.user. This extra lookup adds latency and opens a race where a mid-request user deletion flips 404 semantics. Use the context user directly.
- const user = await repository.user.find(event.context.user.id) - if (!user) { - throw createError({ - statusCode: 404, - message: 'User not found', - }) - } + const user = event.context.userIf you intentionally want to handle “user deleted mid-request,” prefer returning 401 for consistency with the middleware. I can adjust the diff accordingly.
🧹 Nitpick comments (29)
apps/web-app/server/api/user/id/[userId]/telegram.post.ts (1)
21-28: Tighten error semantics to reduce key enumeration.Return a single 404 with a neutral message (“Invalid or expired key”) rather than exposing botId mismatch specifics.
- const user = await repository.telegram.findUserByKey(data.accessKey) - if (!user || user.botId !== data.botId) { - throw createError({ - statusCode: 404, - message: 'User not found', - }) - } + const user = await repository.telegram.findUserByKey(data.accessKey) + if (!user || user.botId !== data.botId) { + throw createError({ + statusCode: 404, + message: 'Invalid or expired key', + }) + }apps/web-app/server/api/task/id/[taskId]/focus.delete.ts (3)
31-36: Consider 404 instead of 403 to prevent task ID enumerationReturning 404 for non-performers hides existence. Align with your API policy before changing.
- throw createError({ - statusCode: 403, - message: 'You are not the performer of this task', - }) + throw createError({ + statusCode: 404, + message: 'Task not found', + })
5-11: Validate taskId format, not just presenceEarly reject malformed IDs (e.g., non-UUIDs) to reduce useless DB hits.
const taskId = getRouterParam(event, 'taskId') if (!taskId) { throw createError({ statusCode: 400, - message: 'Id is required', + message: 'Task id is required', }) } + // Optional: strict format check (if IDs are UUIDs) + // import { validate as uuidValidate } from 'uuid' + // if (!uuidValidate(taskId)) { + // throw createError({ statusCode: 400, message: 'Invalid task id' }) + // }
38-44: Wrap user+task updates in a transactionAvoid partial state if the second update fails.
Example (pseudocode):
await repository.$tx(async (tx) => { if ((await tx.task.find(taskId))?.performerId !== user.id) throw createError({ statusCode: 404, message: 'Task not found' }) if (user.focusedTaskId === task.id) await tx.user.update(user.id, { focusedTaskId: null }) await tx.task.update(task.id, { updatedAt: new Date() }) })apps/web-app/server/api/partner/agreement/index.post.ts (1)
23-26: Use 201 Created and set Location header.Aligns with REST semantics for successful create.
- return { + setResponseStatus(event, 201) + setHeader(event, 'Location', `/api/partner/agreement/${agreement.id}`) + return { ok: true, result: agreement, }apps/web-app/server/api/menu/category/index.post.ts (1)
24-27: Use 201 Created and set Location header.- return { + setResponseStatus(event, 201) + setHeader(event, 'Location', `/api/menu/category/${id}`) + return { ok: true, result: category, }apps/web-app/server/api/product/index.post.ts (1)
25-28: Use 201 Created and set Location header.- return { + setResponseStatus(event, 201) + setHeader(event, 'Location', `/api/product/${id}`) + return { ok: true, result: product, }apps/web-app/server/api/post/id/[postId]/index.patch.ts (1)
23-23: Return 404 when target post doesn’t exist (if update doesn’t throw).If
repository.post.updatereturns null/undefined instead of throwing, surface a 404.Apply:
- const post = await repository.post.update(postId, data) + const post = await repository.post.update(postId, data) + if (!post) { + throw createError({ statusCode: 404, message: 'Post not found' }) + }Please confirm whether
repository.post.updatethrows on missing id. If it does, ignore this.apps/web-app/server/api/post/id/[postId]/image.post.ts (1)
9-9: Nit: Align accepted formats with sharp’s metadata.format.
metadata.formatreturns'jpeg'(not'jpg'). Keeping'jpg'here adds confusion.Apply:
-const ACCEPTED_IMAGE_FORMATS = ['jpeg', 'jpg', 'png', 'webp', 'heif', 'avif'] +const ACCEPTED_IMAGE_FORMATS = ['jpeg', 'png', 'webp', 'heif', 'avif']apps/web-app/server/api/print/index.post.ts (1)
7-7: Permission scope nit: prefer an explicit “create” scope.If you model CRUD scopes distinctly, use
print:createfor clarity.- await hasPermission(event, 'print:edit') + await hasPermission(event, 'print:create')apps/web-app/server/api/partner/agreement/id/[agreementId]/index.patch.ts (1)
31-35: UseupdateAgreementreturn value to skip the extra read and handle missing records
Replace the unguarded call toupdateAgreementand blind{ ok: true }return with checking its returned entity:- await repository.partner.updateAgreement(agreementId, data) - return { ok: true } + const updated = await repository.partner.updateAgreement(agreementId, data) + if (!updated) throw createError({ statusCode: 404, message: 'Agreement not found' }) + return { ok: true }apps/web-app/server/api/product/id/[productId]/index.patch.ts (1)
23-28: Wrap update + tag update in a transaction.Prevent partial updates if tag mutation fails.
- const product = await repository.product.update(productId, productData) - if (tagsId) { - await repository.product.updateTags(productId, tagsId) - } + const product = await repository.$transaction(async (tx) => { + const p = await tx.product.update(productId, productData) + if (tagsId) await tx.product.updateTags(productId, tagsId) + return p + })Adjust to the repository/ORM transaction API you use.
apps/web-app/server/api/post/index.post.ts (1)
24-27: Standardize error handling across handlers.Other endpoints drop try/catch and rely on global error mapping; this one still uses
errorResolver. Pick one pattern repo-wide for consistent responses.-export default defineEventHandler(async (event) => { - try { +export default defineEventHandler(async (event) => { await hasPermission(event, 'post:create') @@ - return { - ok: true, - result: post, - } - } catch (error) { - throw errorResolver(error) - } + return { ok: true, result: post } })apps/web-app/server/api/partner/id/[partnerId].patch.ts (3)
9-15: Validate partnerId format, not just presence.If IDs are UUIDs (or numeric), enforce format before hitting the repo to avoid 500s and improve error clarity.
- const partnerId = getRouterParam(event, 'partnerId') - if (!partnerId) { + const partnerId = getRouterParam(event, 'partnerId') + if (!partnerId /* add format check: e.g., !isUuid(partnerId) */) { throw createError({ statusCode: 400, - message: 'Id is required', + message: 'Valid partnerId is required', }) }
31-35: Consider returning the updated entity for clients that need fresh data.If consumers refetch after update, returning the updated partner can save a round trip.
- await repository.partner.update(partnerId, data) - - return { - ok: true, - } + const updated = await repository.partner.update(partnerId, data) + return { ok: true, result: updated }
36-39: Align error-handling pattern with other endpoints.Either keep
errorResolvereverywhere or move to centralized error middleware.apps/web-app/server/api/product/variant/tag/list.get.ts (1)
3-5: Public read endpoint: consider cache headers and unify error shape.Add short-lived caching; and decide whether to keep try/catch or rely on global handler for consistent error payloads.
-export default defineEventHandler(async () => { - return repository.product.listVariantTags() -}) +export default defineEventHandler(async (event) => { + // Tags change infrequently; tune as needed + setHeader(event, 'Cache-Control', 'public, max-age=60, stale-while-revalidate=120') + return repository.product.listVariantTags() +})apps/web-app/server/api/product/tag/list.get.ts (1)
3-5: Same as variant tags: add cache headers and standardize error handling.Mirror the approach for variant tags for consistency and perf.
-export default defineEventHandler(async () => { - return repository.product.listTags() -}) +export default defineEventHandler(async (event) => { + setHeader(event, 'Cache-Control', 'public, max-age=60, stale-while-revalidate=120') + return repository.product.listTags() +})apps/web-app/server/api/product/id/[productId]/image.post.ts (1)
10-10: Ensure HEIF/AVIF support at runtime
No libvips/libheif/avif references were found in your Dockerfiles or CI configs—if the underlying libheif support isn’t installed, calls tosharp(...).metadata()(or even initialization) will throw and return 500 instead of your intended 415. Wrap the sharp stream creation and metadata calls in a try/catch to return a 415 for unsupported formats, and verify that your Docker/hosting images include libvips with HEIF/AVIF support.apps/web-app/server/api/notification/my.get.ts (1)
4-4: Preserve consistent error shape and explicit await.
Reintroducetry/catchwitherrorResolverfor parity with other handlers.-export default defineEventHandler(async (event) => { - return repository.notification.listByUser(event.context.user.id) -}) +export default defineEventHandler(async (event) => { + try { + return await repository.notification.listByUser(event.context.user.id) + } catch (error) { + throw errorResolver(error) + } +})Confirm this route is covered by the auth middleware; otherwise add a 401 guard.
apps/web-app/server/api/task/index.post.ts (1)
40-40: Defensive fallback for missing profile fields.If
nameorsurnamecan be null/empty, default to a safe placeholder to avoid odd bot messages.-const text = `${event.context.user.name} ${event.context.user.surname} ${suffixByGender(['создал', 'создала'], event.context.user.gender)} задачу «${task.name}»` +const { name = 'Пользователь', surname = '' } = event.context.user +const text = `${name} ${surname}`.trim() + ` ${suffixByGender(['создал', 'создала'], event.context.user.gender)} задачу «${task.name}»`apps/web-app/server/api/ticket/id/[ticketId]/message.post.ts (1)
44-49: Don’t fail the request if Telegram send fails.Decouple notification from the main write; log and continue so message creation isn’t rolled back by a transient Telegram error.
- if (wasabiUser) { - // Send message to Telegram - const text = `${event.context.user.name} ${event.context.user.surname}: ${data.text}` - await useWasabiBot().api.sendMessage(wasabiUser.telegramId, text) - } + if (wasabiUser) { + const text = `${event.context.user.name} ${event.context.user.surname}: ${data.text}` + try { + await useWasabiBot().api.sendMessage(wasabiUser.telegramId, text) + } catch (e) { + console.warn('Telegram send failed', { ticketId, wasabiUserId: wasabiUser.id }) + // Intentionally non-fatal + } + }apps/web-app/server/api/task/list/id/[listId].delete.ts (1)
30-40: Consider atomic archiveArchiving list and chat separately can leave them out of sync on partial failure. Prefer a transaction if the repository supports it.
I can draft a transactional variant against your repository API if available.
apps/web-app/server/api/chat/index.post.ts (1)
32-40: Prevent duplicate member inserts when adding bots; iterate unique IDs.
Current code may create duplicate memberships if bots/user already in usersId.Apply this diff:
- const botIds = bots.map((bot) => bot.id) - data.usersId.push(...botIds) + const botIds = bots.map((bot) => bot.id) + const uniqueUserIds = Array.from(new Set([...data.usersId, ...botIds])) @@ - for (const userId of data.usersId) { + for (const userId of uniqueUserIds) {apps/web-app/server/api/post/comment/id/[commentId]/index.delete.ts (1)
21-25: Return 403 instead of 400 for ownership violation.
More accurate semantics for authorization failure.Apply this diff:
- if (commentInDB.userId !== event.context.user.id) { + if (commentInDB.userId !== event.context.user.id) { throw createError({ - statusCode: 400, - message: 'Not your comment', + statusCode: 403, + message: 'Forbidden', }) }apps/web-app/server/api/task/id/[taskId]/complete.post.ts (3)
66-72: Status code should be 404, not 500, for missing task list.Absent list is a missing resource, not a server error. Adjust for correct semantics and better observability signal.
- throw createError({ - statusCode: 500, - message: 'Task list not found', - }) + throw createError({ + statusCode: 404, + message: 'Task list not found', + })
74-87: Don’t fail the request on chat delivery errors.Bot message delivery is ancillary. If the chat subsystem hiccups, the task completion should still succeed. Wrap in try/catch or enqueue.
- if (bot) { - const text = prepareBotMessage(user, updatedTask) - // Send message as bot - await repository.chat.createMessage({ - chatId: list.chat.id, - userId: bot.user.id, - text, - }) - } + if (bot) { + try { + const text = prepareBotMessage(user, updatedTask) + await repository.chat.createMessage({ + chatId: list.chat.id, + userId: bot.user.id, + text, + }) + } catch (e) { + // log and continue; do not block task completion + console.warn('chat notify failed', e) + } + }
90-106: Avoid full table scan for staff notifications.repository.user.list() + in-memory filter doesn’t scale. Add a repo method to fetch staff-only and exclude the author at the DB layer.
I can draft a Repository.user.listStaff(excludeId?: string) query if you want it in this PR.
apps/web-app/server/middleware/01.auth.ts (1)
20-23: Preflight check: consider using a helper for robustness.event.method === 'OPTIONS' is fine; if available in your h3 version, isPreflightRequest(event) is clearer and future-proof.
Please confirm isPreflightRequest exists in your h3 version; if yes, import from 'h3' and use it.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (59)
apps/atrium-telegram/server/middleware/01.auth.ts(1 hunks)apps/web-app/server/api/activity/schedule/item/id/[itemId].patch.ts(1 hunks)apps/web-app/server/api/auth/me.get.ts(1 hunks)apps/web-app/server/api/chat/id/[chatId]/message/index.post.ts(2 hunks)apps/web-app/server/api/chat/index.post.ts(2 hunks)apps/web-app/server/api/chat/my/index.get.ts(1 hunks)apps/web-app/server/api/checkout/list.get.ts(1 hunks)apps/web-app/server/api/epic/comment/id/[commentId]/index.delete.ts(1 hunks)apps/web-app/server/api/epic/id/[epicId]/comment.post.ts(2 hunks)apps/web-app/server/api/epic/id/[epicId]/index.patch.ts(1 hunks)apps/web-app/server/api/epic/index.post.ts(3 hunks)apps/web-app/server/api/locker/duplicate/list.get.ts(1 hunks)apps/web-app/server/api/menu/category/id/[categoryId]/index.patch.ts(1 hunks)apps/web-app/server/api/menu/category/id/[categoryId]/product.delete.ts(1 hunks)apps/web-app/server/api/menu/category/id/[categoryId]/product.post.ts(1 hunks)apps/web-app/server/api/menu/category/index.post.ts(1 hunks)apps/web-app/server/api/notification/my.get.ts(1 hunks)apps/web-app/server/api/partner/agreement/id/[agreementId]/index.patch.ts(1 hunks)apps/web-app/server/api/partner/agreement/index.post.ts(1 hunks)apps/web-app/server/api/partner/id/[partnerId].patch.ts(1 hunks)apps/web-app/server/api/partner/legal/id/[entityId]/index.patch.ts(1 hunks)apps/web-app/server/api/partner/legal/index.post.ts(1 hunks)apps/web-app/server/api/payment/method/list.get.ts(1 hunks)apps/web-app/server/api/post/comment/id/[commentId]/index.delete.ts(1 hunks)apps/web-app/server/api/post/id/[postId]/comment.post.ts(2 hunks)apps/web-app/server/api/post/id/[postId]/image.post.ts(1 hunks)apps/web-app/server/api/post/id/[postId]/index.patch.ts(1 hunks)apps/web-app/server/api/post/id/[postId]/like.delete.ts(1 hunks)apps/web-app/server/api/post/id/[postId]/like.post.ts(1 hunks)apps/web-app/server/api/post/index.post.ts(2 hunks)apps/web-app/server/api/print/id/[printId].patch.ts(1 hunks)apps/web-app/server/api/print/index.post.ts(2 hunks)apps/web-app/server/api/product/id/[productId]/image.post.ts(1 hunks)apps/web-app/server/api/product/id/[productId]/index.patch.ts(1 hunks)apps/web-app/server/api/product/index.post.ts(1 hunks)apps/web-app/server/api/product/tag/list.get.ts(1 hunks)apps/web-app/server/api/product/variant/id/[variantId]/index.patch.ts(1 hunks)apps/web-app/server/api/product/variant/index.post.ts(1 hunks)apps/web-app/server/api/product/variant/tag/list.get.ts(1 hunks)apps/web-app/server/api/task/id/[taskId]/complete.post.ts(2 hunks)apps/web-app/server/api/task/id/[taskId]/focus.delete.ts(1 hunks)apps/web-app/server/api/task/id/[taskId]/focus.post.ts(1 hunks)apps/web-app/server/api/task/id/[taskId]/index.patch.ts(3 hunks)apps/web-app/server/api/task/index.post.ts(2 hunks)apps/web-app/server/api/task/list/completed.get.ts(1 hunks)apps/web-app/server/api/task/list/id/[listId].delete.ts(1 hunks)apps/web-app/server/api/task/list/id/[listId].patch.ts(2 hunks)apps/web-app/server/api/task/list/index.get.ts(1 hunks)apps/web-app/server/api/task/list/index.post.ts(2 hunks)apps/web-app/server/api/ticket/id/[ticketId]/message.post.ts(3 hunks)apps/web-app/server/api/ticket/id/[ticketId]/messages.get.ts(0 hunks)apps/web-app/server/api/ticket/list.get.ts(1 hunks)apps/web-app/server/api/user/id/[userId]/complete.post.ts(1 hunks)apps/web-app/server/api/user/id/[userId]/image.post.ts(1 hunks)apps/web-app/server/api/user/id/[userId]/index.patch.ts(2 hunks)apps/web-app/server/api/user/id/[userId]/online.post.ts(1 hunks)apps/web-app/server/api/user/id/[userId]/telegram.post.ts(1 hunks)apps/web-app/server/middleware/01.auth.ts(1 hunks)apps/web-app/shared/types/h3.d.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web-app/server/api/ticket/id/[ticketId]/messages.get.ts
✅ Files skipped from review due to trivial changes (3)
- apps/web-app/server/api/menu/category/id/[categoryId]/index.patch.ts
- apps/web-app/server/api/user/id/[userId]/complete.post.ts
- apps/web-app/server/api/menu/category/id/[categoryId]/product.delete.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/atrium-telegram/server/middleware/01.auth.ts
🧰 Additional context used
🧬 Code graph analysis (13)
apps/web-app/server/api/chat/my/index.get.ts (2)
packages/database/src/repository/index.ts (1)
repository(61-61)packages/database/src/index.ts (1)
repository(2-2)
apps/web-app/server/api/payment/method/list.get.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/task/id/[taskId]/focus.post.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/locker/duplicate/list.get.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/post/id/[postId]/like.delete.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/print/index.post.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/task/id/[taskId]/complete.post.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/auth/me.get.ts (2)
packages/database/src/repository/index.ts (1)
repository(61-61)packages/database/src/index.ts (1)
repository(2-2)
apps/web-app/server/api/notification/my.get.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/epic/index.post.ts (1)
packages/database/src/tables.ts (1)
users(75-93)
apps/web-app/server/api/task/index.post.ts (1)
apps/web-app/shared/utils/gender.ts (1)
suffixByGender(3-11)
apps/web-app/server/api/task/id/[taskId]/focus.delete.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
apps/web-app/server/api/checkout/list.get.ts (1)
packages/database/src/repository/index.ts (1)
repository(61-61)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
| @@ -1,6 +1,6 @@ | |||
| import { updateActivityScheduleItemSchema } from '#shared/services/activity' | |||
There was a problem hiding this comment.
Missing authorization check after removing session guard.
This now allows any authenticated user to update any schedule item. Add a permission or ownership/membership check (align with product image route’s hasPermission).
export default defineEventHandler(async (event) => {
try {
+ await hasPermission?.(event, 'activity:schedule:edit')Alternatively, enforce ownership:
const item = await repository.activity.findScheduleItem(itemId)
if (item.userId !== event.context.user.id) {
throw createError({ statusCode: 403, message: 'Forbidden' })
}|
|
||
| // Guard: Must be user as a member | ||
| if (data.usersId.length === 0 && !data.usersId.includes(session.user.id)) { | ||
| if (data.usersId.length === 0 && !data.usersId.includes(event.context.user.id)) { |
There was a problem hiding this comment.
Guard logic allows creating a chat without the initiator as a member (&& vs ||).
Use OR so we fail when the list is empty or the current user isn’t included.
Apply this diff:
- if (data.usersId.length === 0 && !data.usersId.includes(event.context.user.id)) {
+ if (data.usersId.length === 0 || !data.usersId.includes(event.context.user.id)) {Optionally auto-include the user:
+ if (!data.usersId.includes(event.context.user.id)) {
+ data.usersId.unshift(event.context.user.id)
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (data.usersId.length === 0 && !data.usersId.includes(event.context.user.id)) { | |
| // Fail if there are no members or if the initiator isn’t included | |
| if (data.usersId.length === 0 || !data.usersId.includes(event.context.user.id)) { | |
| throw new Error("Chat must include at least one member and the initiator"); | |
| } | |
| // (Optional) Ensure the initiator is always part of the members list | |
| if (!data.usersId.includes(event.context.user.id)) { | |
| data.usersId.unshift(event.context.user.id); | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/server/api/chat/index.post.ts around line 14, the guard
currently uses && which allows creating a chat when usersId is empty but doesn't
include the initiator; change the condition to use || so the request fails when
usersId is empty OR it does not include event.context.user.id, and optionally,
if you prefer auto-inclusion instead of failing, push event.context.user.id into
data.usersId when it's missing (ensuring no duplicates) before proceeding.
| title: `${suffixByGender(['Создал', 'Создала'], event.context.user.gender)} эпик «${epic.title}»`, | ||
| description: epic.description ? epic.description : 'Без описания', |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
suffixByGender is used but not imported — runtime ReferenceError.
Add the missing import (adjust path to your actual module).
import { createEpicSchema } from '#shared/services/epic'
import { repository } from '@roll-stack/database'
import { type } from 'arktype'
+import { suffixByGender } from '#shared/utils/gender' // verify pathAlso applies to: 1-4
🏁 Script executed:
#!/bin/bash
# Locate the actual export of suffixByGender to confirm import path
rg -nP 'export\s+(function|const)\s+suffixByGender\b|suffixByGender\s*=\s*\(' --type=ts --type=tsx --type=jsLength of output: 138
🏁 Script executed:
#!/bin/bash
# Search for suffixByGender definitions across all TS/JS files using glob patterns
rg -nP 'suffixByGender' -C2 -g '*.ts' -g '*.tsx' -g '*.js' -g '*.jsx'Length of output: 9104
Import suffixByGender in epic endpoints
- In apps/web-app/server/api/epic/index.post.ts and apps/web-app/server/api/epic/comment/id/[commentId]/beacon.post.ts, add:
+ import { suffixByGender } from '#shared/utils/gender'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| title: `${suffixByGender(['Создал', 'Создала'], event.context.user.gender)} эпик «${epic.title}»`, | |
| description: epic.description ? epic.description : 'Без описания', | |
| import { suffixByGender } from '#shared/utils/gender' |
🤖 Prompt for AI Agents
In apps/web-app/server/api/epic/index.post.ts around lines 35-36, the call to
suffixByGender is used but the function is not imported; likewise add the same
import in apps/web-app/server/api/epic/comment/id/[commentId]/beacon.post.ts.
Add a named import for suffixByGender from the shared utility module where it
lives (e.g., the appropriate utils/common path used in the project) at the top
of each file so the function is available to construct the localized title and
avoid runtime ReferenceError.
| } catch (error) { | ||
| throw errorResolver(error) | ||
| } | ||
| return repository.locker.listDuplicatesForUser(event.context.user.id) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Restore error mapping for consistency (wrap with errorResolver).
Other handlers standardize errors via errorResolver. This one will leak raw errors.
Apply:
-export default defineEventHandler(async (event) => {
- return repository.locker.listDuplicatesForUser(event.context.user.id)
-})
+export default defineEventHandler(async (event) => {
+ try {
+ return await repository.locker.listDuplicatesForUser(event.context.user.id)
+ } catch (error) {
+ throw errorResolver(error)
+ }
+})📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return repository.locker.listDuplicatesForUser(event.context.user.id) | |
| export default defineEventHandler(async (event) => { | |
| try { | |
| return await repository.locker.listDuplicatesForUser(event.context.user.id) | |
| } catch (error) { | |
| throw errorResolver(error) | |
| } | |
| }) |
🤖 Prompt for AI Agents
In apps/web-app/server/api/locker/duplicate/list.get.ts around line 4, the
handler returns repository.locker.listDuplicatesForUser(...) directly which can
leak raw errors; wrap the repository call with the existing errorResolver to
normalize errors (e.g. return errorResolver(() =>
repository.locker.listDuplicatesForUser(event.context.user.id))). Also ensure
errorResolver is imported at the top of the file if not already.
| @@ -1,6 +1,6 @@ | |||
| import { createPartnerAgreementSchema } from '#shared/services/partner' | |||
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify alias configuration for #shared.
Confirm TS and Nitro resolve the alias in both build and runtime.
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking tsconfig paths for #shared/*"
fd -a 'tsconfig*.json' | while read -r f; do
echo "File: $f"
jq -r '.compilerOptions.paths["#shared/*"][]?' "$f"
done
echo "Checking nuxt config alias"
fd -a '^nuxt\.config\.(t|j)s$' | xargs -I{} rg -n "alias\s*:\s*{[^}]*#shared" -n {}Length of output: 1105
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- Checking apps/web-app/tsconfig.json for '#shared' alias -----"
TSCONFIG="apps/web-app/tsconfig.json"
if [ -f "$TSCONFIG" ]; then
echo "File: $TSCONFIG"
jq -r '.compilerOptions.paths["#shared/*"][]?' "$TSCONFIG" || echo "(no '#shared/*' entries found)"
else
echo "Missing $TSCONFIG"
fi
echo
echo "----- Checking apps/web-app/nuxt.config.{ts,js} for '#shared' alias -----"
for F in apps/web-app/nuxt.config.ts apps/web-app/nuxt.config.js; do
if [ -f "$F" ]; then
echo "File: $F"
grep -n "alias" "$F" || echo "(no alias block)"
grep -n "#shared" "$F" || echo "(no '#shared' alias)"
echo
fi
doneLength of output: 1063
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- Showing beginning of apps/web-app/nuxt.config.ts -----"
sed -n '1,200p' apps/web-app/nuxt.config.ts || echo "(file shorter than 200 lines)"
echo
echo "----- Searching for tsconfig paths plugin usage -----"
grep -R "tsconfig-paths" -n apps/web-app/nuxt.config.ts || grep -R "viteTsconfigPaths" -n apps/web-app/nuxt.config.ts || echo "(no tsconfig paths plugin found)"
echo
echo "----- Checking for custom resolve.alias entries -----"
grep -R "alias\s*:" -n apps/web-app/nuxt.config.ts || echo "(no alias blocks found)"Length of output: 2527
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "----- apps/web-app/tsconfig.json -----"
sed -n '1,200p' apps/web-app/tsconfig.jsonLength of output: 376
Add missing #shared alias mapping
- In apps/web-app/tsconfig.json, under compilerOptions, add a paths entry for "#shared/" pointing to your shared source (e.g. "../shared/").
- In apps/web-app/nuxt.config.ts, enable resolution at runtime—either install and configure vite-tsconfig-paths or add a resolve.alias mapping for #shared → your shared folder.
Ensures imports like#shared/services/partnerwork in both TS compilation and Nitro/Vite runtime.
🤖 Prompt for AI Agents
In apps/web-app/server/api/partner/agreement/index.post.ts line 1, the import
from "#shared/services/partner" fails at build/runtime because the project lacks
the "#shared" path alias mapping; to fix it add a "paths" entry in
apps/web-app/tsconfig.json under compilerOptions mapping "#shared/*" to the
shared source (e.g. "../shared/*"), then enable runtime resolution in
apps/web-app/nuxt.config.ts by either installing and configuring
vite-tsconfig-paths or adding a resolve.alias (and corresponding Nitro alias)
that maps "#shared" to the shared folder so TypeScript and Vite/Nitro both
resolve the import at compile and runtime.
| export default defineEventHandler(async () => { | ||
| return repository.task.lists() | ||
| }) |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Do not expose all task lists; re-scope to the caller.
Dropping event and auth makes this endpoint effectively public and likely returns every list. Restore user scoping.
-export default defineEventHandler(async () => {
- return repository.task.lists()
-})
+export default defineEventHandler(async (event) => {
+ try {
+ // Adjust to the actual repo method that returns lists for a user.
+ return await repository.task.listsByUser(event.context.user.id)
+ } catch (error) {
+ throw errorResolver(error)
+ }
+})🏁 Script executed:
#!/bin/bash
# Find the correct repository API to fetch lists scoped to a user.
rg -nP -C2 '\brepository\.task\.(listsByUser|listByUser|listsForUser|findListsByUser|findAllByUser)\b' --type=tsLength of output: 114
🏁 Script executed:
#!/bin/bash
# List all repository.task method usages to find available methods
rg -nP "repository\.task\.\w+" -C2 --type=tsLength of output: 18272
Rescope GET /api/task/list to authenticated user
The handler currently omits event/auth and calls repository.task.lists(), exposing every list. Restore the event parameter, enforce authentication, and implement or call a repo method that filters by event.context.user.id so only the caller’s lists are returned.
apps/web-app/server/api/task/list/index.get.ts:3-5
🤖 Prompt for AI Agents
In apps/web-app/server/api/task/list/index.get.ts around lines 3 to 5, the
handler is missing the event parameter and returns all lists; restore the event
parameter (async event => ...), enforce authentication by checking
event.context.user and returning a 401/throw if absent, extract the caller id
from event.context.user.id, and call or implement a repository method that
filters lists by owner/user id (e.g., repository.task.listsByOwner(userId) or
repository.task.lists({ ownerId: userId })) so only the authenticated user’s
lists are returned.
| // Guard: if not this user | ||
| if (event.context.user.id !== userId) { | ||
| throw createError({ | ||
| statusCode: 403, | ||
| message: 'Forbidden', | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Keep self-edit guard, but re-add 404 for nonexistent user
Avoid doing image work/storage for a user that may not exist; respond 404 early.
Apply:
// Guard: if not this user
if (event.context.user.id !== userId) {
throw createError({
statusCode: 403,
message: 'Forbidden',
})
}
+ // Ensure user exists
+ const user = await repository.user.find(userId)
+ if (!user) {
+ throw createError({
+ statusCode: 404,
+ message: 'User not found',
+ })
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Guard: if not this user | |
| if (event.context.user.id !== userId) { | |
| throw createError({ | |
| statusCode: 403, | |
| message: 'Forbidden', | |
| }) | |
| } | |
| // Guard: if not this user | |
| if (event.context.user.id !== userId) { | |
| throw createError({ | |
| statusCode: 403, | |
| message: 'Forbidden', | |
| }) | |
| } | |
| // Ensure user exists | |
| const user = await repository.user.find(userId) | |
| if (!user) { | |
| throw createError({ | |
| statusCode: 404, | |
| message: 'User not found', | |
| }) | |
| } |
🤖 Prompt for AI Agents
In apps/web-app/server/api/user/id/[userId]/image.post.ts around lines 29-35,
add an early existence check for the target user before performing any
image/storage work: query your users table (e.g., await
db.user.findUnique/findFirst by userId) and if it returns null throw
createError({ statusCode: 404, message: 'User not found' }); then keep the
existing self-edit guard (the 403 check) after the 404 check so you still
prevent other users from editing but never attempt image operations for
nonexistent users.
| const routesWithoutAuth = [ | ||
| '/api/health', | ||
| '/api/agent', // token from headers | ||
| '/api/auth/sign-in', // user from body | ||
| '/api/avatar', // public | ||
| '/api/cuid', // public | ||
| '/api/qr', // public | ||
| ] |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Whitelist should support prefixes/regex and use pathname; exact matches will miss nested routes.
E.g., /api/avatar/123 or /api/auth/callback would be forced through auth. Also prefer pathname over event.path for reliability.
-import type { H3Event } from 'h3'
+import type { H3Event } from 'h3'
+import { getRequestURL } from 'h3'
@@
-const routesWithoutAuth = [
- '/api/health',
- '/api/agent', // token from headers
- '/api/auth/sign-in', // user from body
- '/api/avatar', // public
- '/api/cuid', // public
- '/api/qr', // public
-]
+const routesWithoutAuth = [
+ /^\/api\/health$/,
+ /^\/api\/agent(\/|$)/, // token from headers
+ /^\/api\/auth(\/|$)/, // all auth endpoints
+ /^\/api\/avatar(\/|$)/, // public
+ /^\/api\/cuid$/, // public
+ /^\/api\/qr$/, // public
+] as const
@@
- // Skip routes without auth
- if (!event.path.startsWith('/api') || routesWithoutAuth.includes(event.path)) {
+ const path = getRequestURL(event).pathname
+ // Skip routes without auth
+ if (!path.startsWith('/api') || routesWithoutAuth.some(r => r.test(path))) {
return
}If you intentionally want only exact endpoints whitelisted, ignore the regex change and keep only the pathname fix.
Also applies to: 25-28
| async function getUserFromSession(event: H3Event): Promise<User | null> { | ||
| try { | ||
| const session = await getUserSession(event) | ||
| if (!session?.user) { | ||
| return null | ||
| } | ||
|
|
||
| const user = await repository.user.find(session.user.id) | ||
| if (!user?.id) { | ||
| return null | ||
| } | ||
|
|
||
| return user | ||
| } catch (e) { | ||
| logger.error(e) | ||
| } | ||
|
|
||
| return null | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure getUserSession is defined/auto-imported; add fallbacks/log context.
There’s no local import here. If it’s a server auto-import, fine; otherwise this will crash.
Run:
Optionally include request context in logs to aid triage (path, ip, ua) if you keep logging here.
🏁 Script executed:
#!/bin/bash
# Locate getUserSession definition and its export
rg -nC2 '\b(export\s+)?(async\s+)?function\s+getUserSession\b|\bconst\s+getUserSession\b' -g '!**/node_modules/**'
rg -n '\bgetUserSession\s*\(' -g '!**/node_modules/**'Length of output: 997
Import getUserSession and enrich error context
- Add an explicit import for
getUserSessionfrom its defining module at the top ofapps/web-app/server/middleware/01.auth.tsto avoid undefined-function crashes. - In the
catchblock, log request details (URL, client IP, user-agent) alongside the error for improved triage.
🤖 Prompt for AI Agents
In apps/web-app/server/middleware/01.auth.ts around lines 43 to 61, add an
explicit import for getUserSession from its defining module at the top of the
file (import { getUserSession } from '<correct-module-path>') to prevent
undefined-function crashes, and modify the catch block to log enriched request
context: capture the request URL, client IP (from X-Forwarded-For header or
connection socket), and user-agent header from event.node.req and include those
details with the error when calling logger.error so the log entry contains the
error plus url, ip, and user-agent for faster triage.
|
|
||
| declare module 'h3' { | ||
| interface H3EventContext { | ||
| user: User |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Don't make user mandatory on all H3 contexts; mark optional and provide an AuthenticatedEvent type.
Some routes may be public or pre-middleware; forcing user: User is unsound.
Apply this diff:
-declare module 'h3' {
- interface H3EventContext {
- user: User
- }
-}
+declare module 'h3' {
+ interface H3EventContext {
+ user?: User
+ }
+}Add a helper type to use in protected handlers (place in a shared .d.ts or a TS module):
import type { H3Event } from 'h3'
import type { User } from '@roll-stack/database'
export type AuthenticatedEvent = H3Event & { context: { user: User } }Then in handlers: export default defineEventHandler(async (event: AuthenticatedEvent) => { ... })
🤖 Prompt for AI Agents
In apps/web-app/shared/types/h3.d.ts around line 5, the current declaration
forces user: User on every H3 event context which is incorrect for public
routes; change the context.user property to be optional (e.g., user?: User) and
add a new exported helper type AuthenticatedEvent in a shared .d.ts or TS module
that refines H3Event to require context.user: User so protected handlers can use
that type; update any protected handlers to annotate their event parameter as
AuthenticatedEvent while leaving general handlers with the original H3Event that
now has an optional user.
1 participant
Summary by CodeRabbit
New Features
Improvements
Refactor