fix: валидация портов серверов и автоочистка невалидных записей#70
Conversation
Исправлена проблема с невалидными портами серверов: - Добавлена валидация порта (1-65535) при создании - Добавлена валидация порта при обновлении - Автоудаление серверов с невалидными портами при старте Решает ошибку "Value does not fit in an INT column".
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughОбзорДобавлена валидация номера порта в эндпоинты создания и обновления серверов с диапазоном 1-65535, включена очистка базы данных от серверов с невалидными портами при запуске приложения, и проведены корректировки в логике управления разрешениями администратора. Изменения
Предполагаемая сложность проверки🎯 2 (Simple) | ⏱️ ~12 минут
Стихотворение
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
Руководство для ревьюераДобавляет строгую валидацию порта (1–65535) при создании/обновлении сервера и вводит стартовую «миграцию», которая удаляет сервера с некорректными значениями порта с помощью raw SQL, обходя ограничения Prisma на тип INT. Диаграмма последовательности для создания сервера с валидацией портаsequenceDiagram
actor User
participant ServersAPI
participant Prisma
participant Database
User ->> ServersAPI: POST /api/servers (name, host, port)
ServersAPI ->> ServersAPI: parseInt(port) or default 25565
alt invalid port (NaN or <1 or >65535)
ServersAPI -->> User: 400 Порт должен быть числом от 1 до 65535 (максимум 5 цифр)
else valid port
ServersAPI ->> Prisma: server.create(name, host, portNumber, version)
Prisma ->> Database: INSERT INTO Server
Database -->> Prisma: insert result
Prisma -->> ServersAPI: newServer
ServersAPI -->> User: 201 newServer
end
Диаграмма последовательности для обновления сервера с валидацией портаsequenceDiagram
actor User
participant ServersAPI
participant Prisma
participant Database
User ->> ServersAPI: PUT /api/servers/:id (optional name, host, port, version)
ServersAPI ->> ServersAPI: build dataToUpdate
alt port provided and not empty
ServersAPI ->> ServersAPI: parseInt(port)
alt invalid port (NaN or <1 or >65535)
ServersAPI -->> User: 400 Порт должен быть числом от 1 до 65535 (максимум 5 цифр)
else valid port
ServersAPI ->> Prisma: server.update(id, dataToUpdate)
Prisma ->> Database: UPDATE Server
Database -->> Prisma: update result
Prisma -->> ServersAPI: updatedServer
ServersAPI -->> User: 200 updatedServer
end
else port not provided or empty
ServersAPI ->> Prisma: server.update(id, dataToUpdate without port)
Prisma ->> Database: UPDATE Server
Database -->> Prisma: update result
Prisma -->> ServersAPI: updatedServer
ServersAPI -->> User: 200 updatedServer
end
Изменения по файлам
Подсказки и командыВзаимодействие с Sourcery
Настройка работыОткройте свою панель управления, чтобы:
Получение помощи
Original review guide in EnglishReviewer's GuideAdds strict port validation (1–65535) for server creation/update and introduces a startup migration that deletes servers with invalid port values via raw SQL, avoiding Prisma INT limitations. Sequence diagram for server creation with port validationsequenceDiagram
actor User
participant ServersAPI
participant Prisma
participant Database
User ->> ServersAPI: POST /api/servers (name, host, port)
ServersAPI ->> ServersAPI: parseInt(port) or default 25565
alt invalid port (NaN or <1 or >65535)
ServersAPI -->> User: 400 Порт должен быть числом от 1 до 65535 (максимум 5 цифр)
else valid port
ServersAPI ->> Prisma: server.create(name, host, portNumber, version)
Prisma ->> Database: INSERT INTO Server
Database -->> Prisma: insert result
Prisma -->> ServersAPI: newServer
ServersAPI -->> User: 201 newServer
end
Sequence diagram for server update with port validationsequenceDiagram
actor User
participant ServersAPI
participant Prisma
participant Database
User ->> ServersAPI: PUT /api/servers/:id (optional name, host, port, version)
ServersAPI ->> ServersAPI: build dataToUpdate
alt port provided and not empty
ServersAPI ->> ServersAPI: parseInt(port)
alt invalid port (NaN or <1 or >65535)
ServersAPI -->> User: 400 Порт должен быть числом от 1 до 65535 (максимум 5 цифр)
else valid port
ServersAPI ->> Prisma: server.update(id, dataToUpdate)
Prisma ->> Database: UPDATE Server
Database -->> Prisma: update result
Prisma -->> ServersAPI: updatedServer
ServersAPI -->> User: 200 updatedServer
end
else port not provided or empty
ServersAPI ->> Prisma: server.update(id, dataToUpdate without port)
Prisma ->> Database: UPDATE Server
Database -->> Prisma: update result
Prisma -->> ServersAPI: updatedServer
ServersAPI -->> User: 200 updatedServer
end
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - я нашёл одну проблему и оставил несколько общих замечаний:
- Логика валидации порта (проверка диапазона и текст ошибки) дублируется между хендлерами создания и обновления; имеет смысл вынести её в небольшую вспомогательную функцию, чтобы правила были единообразными и проще поддерживались.
- Стартовый запрос на очистку сейчас использует
$executeRawUnsafeс интерполированной строкой; даже несмотря на отсутствие пользовательского ввода, переход на$executeRawс параметризованным фрагментом (или хотя бы документация, почемуUnsafeздесь безопасен) уменьшит риск неправильного использования в будущем. - Сообщение об ошибке упоминает ограничение «максимум 5 цифр», тогда как логика фактически проверяет числовой диапазон (1–65535); стоит привести текст сообщения в соответствие с реальной проверкой (например, упомянуть только числовой диапазон), чтобы избежать путаницы.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The port validation logic (range checks and error message) is duplicated between the create and update handlers; consider extracting a small helper function to keep the rules consistent and easier to maintain.
- The startup cleanup query currently uses `$executeRawUnsafe` with an interpolated string; even though there is no user input, switching to `$executeRaw` with a parameterized snippet (or at least documenting why `Unsafe` is safe here) would reduce the risk of future misuse.
- The error message mentions a "maximum 5 digits" constraint while the logic enforces a numeric range (1–65535); consider aligning the message with the actual validation (e.g., focusing only on the numeric range) to avoid confusion.
## Individual Comments
### Comment 1
<location> `backend/src/server.js:107-108` </location>
<code_context>
try {
+ // Удаляем серверы с невалидными портами напрямую через SQL,
+ // потому что Prisma не может их прочитать если значение не помещается в INT
+ const result = await prisma.$executeRawUnsafe(
+ `DELETE FROM Server WHERE length(CAST(port AS TEXT)) > 5 OR port > 65535 OR port < 1`
+ );
+
</code_context>
<issue_to_address>
**🚨 issue (security):** Raw SQL is both unsafe and potentially non-portable; consider quoting identifiers and using $executeRaw.
Because the query is static and doesn’t use user input, `$executeRaw` is safer than `$executeRawUnsafe` with no downside here. Also, `DELETE FROM Server WHERE length(CAST(port AS TEXT))...` is dialect-specific and may not behave consistently across providers (e.g. quoted vs unquoted `Server` in Postgres, `length`/`TEXT` vs `LENGTH`/`CHAR` in MySQL). Please quote the table name appropriately for your DB (e.g. `"Server"` in Postgres) and align the functions/casts with the actual provider, or document that this migration is provider-specific.
</issue_to_address>Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The port validation logic (range checks and error message) is duplicated between the create and update handlers; consider extracting a small helper function to keep the rules consistent and easier to maintain.
- The startup cleanup query currently uses
$executeRawUnsafewith an interpolated string; even though there is no user input, switching to$executeRawwith a parameterized snippet (or at least documenting whyUnsafeis safe here) would reduce the risk of future misuse. - The error message mentions a "maximum 5 digits" constraint while the logic enforces a numeric range (1–65535); consider aligning the message with the actual validation (e.g., focusing only on the numeric range) to avoid confusion.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The port validation logic (range checks and error message) is duplicated between the create and update handlers; consider extracting a small helper function to keep the rules consistent and easier to maintain.
- The startup cleanup query currently uses `$executeRawUnsafe` with an interpolated string; even though there is no user input, switching to `$executeRaw` with a parameterized snippet (or at least documenting why `Unsafe` is safe here) would reduce the risk of future misuse.
- The error message mentions a "maximum 5 digits" constraint while the logic enforces a numeric range (1–65535); consider aligning the message with the actual validation (e.g., focusing only on the numeric range) to avoid confusion.
## Individual Comments
### Comment 1
<location> `backend/src/server.js:107-108` </location>
<code_context>
try {
+ // Удаляем серверы с невалидными портами напрямую через SQL,
+ // потому что Prisma не может их прочитать если значение не помещается в INT
+ const result = await prisma.$executeRawUnsafe(
+ `DELETE FROM Server WHERE length(CAST(port AS TEXT)) > 5 OR port > 65535 OR port < 1`
+ );
+
</code_context>
<issue_to_address>
**🚨 issue (security):** Raw SQL is both unsafe and potentially non-portable; consider quoting identifiers and using $executeRaw.
Because the query is static and doesn’t use user input, `$executeRaw` is safer than `$executeRawUnsafe` with no downside here. Also, `DELETE FROM Server WHERE length(CAST(port AS TEXT))...` is dialect-specific and may not behave consistently across providers (e.g. quoted vs unquoted `Server` in Postgres, `length`/`TEXT` vs `LENGTH`/`CHAR` in MySQL). Please quote the table name appropriately for your DB (e.g. `"Server"` in Postgres) and align the functions/casts with the actual provider, or document that this migration is provider-specific.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const result = await prisma.$executeRawUnsafe( | ||
| `DELETE FROM Server WHERE length(CAST(port AS TEXT)) > 5 OR port > 65535 OR port < 1` |
There was a problem hiding this comment.
🚨 issue (security): Raw SQL is both unsafe and potentially non-portable; consider quoting identifiers and using $executeRaw.
Because the query is static and doesn’t use user input, $executeRaw is safer than $executeRawUnsafe with no downside here. Also, DELETE FROM Server WHERE length(CAST(port AS TEXT))... is dialect-specific and may not behave consistently across providers (e.g. quoted vs unquoted Server in Postgres, length/TEXT vs LENGTH/CHAR in MySQL). Please quote the table name appropriately for your DB (e.g. "Server" in Postgres) and align the functions/casts with the actual provider, or document that this migration is provider-specific.
Original comment in English
🚨 issue (security): Raw SQL is both unsafe and potentially non-portable; consider quoting identifiers and using $executeRaw.
Because the query is static and doesn’t use user input, $executeRaw is safer than $executeRawUnsafe with no downside here. Also, DELETE FROM Server WHERE length(CAST(port AS TEXT))... is dialect-specific and may not behave consistently across providers (e.g. quoted vs unquoted Server in Postgres, length/TEXT vs LENGTH/CHAR in MySQL). Please quote the table name appropriately for your DB (e.g. "Server" in Postgres) and align the functions/casts with the actual provider, or document that this migration is provider-specific.
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
backend/src/api/routes/servers.js(2 hunks)backend/src/server.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
backend/src/server.js (1)
backend/src/api/routes/auth.js (3)
permissions(156-156)permissions(335-335)ALL_PERMISSIONS(20-66)
⏰ 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: Sourcery review
🔇 Additional comments (4)
backend/src/api/routes/servers.js (3)
42-46: Валидация порта выглядит корректно.Логика валидации правильно обрабатывает граничные случаи:
- Используется стандартный порт Minecraft (25565) по умолчанию
parseIntс проверкойisNaNотсекает нечисловые значения- Диапазон 1-65535 соответствует валидным TCP/UDP портам
49-49: Правильное использование нормализованного значения порта.Использование
portNumberвместо исходногоportгарантирует, что в базу сохраняется валидированное числовое значение.
67-73: Валидация в PUT-маршруте согласована с POST.Проверка
port !== undefined && port !== ''корректно обрабатывает случаи, когда порт не передан или передана пустая строка. Логика валидации идентична POST-маршруту.backend/src/server.js (1)
107-109: Проверка длины порта действительно избыточна.Условие
length(CAST(port AS TEXT)) > 5логически лишнее, так как проверкаport > 65535 OR port < 1уже покрывает все невалидные порты: положительные значения больше 65535 и все отрицательные значения. Любой порт с количеством символов больше 5 при приведении к тексту уже будет отклонен одним из этих двух условий. Рекомендуется упростить запрос до:DELETE FROM Server WHERE port > 65535 OR port < 1
| // Удаляем серверы с невалидными портами напрямую через SQL, | ||
| // потому что Prisma не может их прочитать если значение не помещается в INT | ||
| const result = await prisma.$executeRawUnsafe( | ||
| `DELETE FROM Server WHERE length(CAST(port AS TEXT)) > 5 OR port > 65535 OR port < 1` | ||
| ); | ||
|
|
||
| if (result > 0) { | ||
| console.log(`[Migration] Удалено ${result} серверов с невалидными портами.`); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find Prisma schema file
echo "=== Finding Prisma schema ==="
find . -name "schema.prisma" -o -name "*.prisma" | head -20
# Check for prisma directory
echo -e "\n=== Checking prisma directory ==="
ls -la prisma/ 2>/dev/null || echo "No prisma directory found"
# Check backend directory structure
echo -e "\n=== Backend directory structure ==="
ls -la backend/prisma/ 2>/dev/null || ls -la backend/ | grep -i prisma || echo "No prisma in backend"Repository: blockmineJS/blockmine
Length of output: 525
🏁 Script executed:
#!/bin/bash
# Read the Prisma schema
cat -n ./backend/prisma/schema.prismaRepository: blockmineJS/blockmine
Length of output: 12236
🏁 Script executed:
#!/bin/bash
# Read the server.js file around lines 105-113
sed -n '90,120p' backend/src/server.jsRepository: blockmineJS/blockmine
Length of output: 1327
🏁 Script executed:
#!/bin/bash
# Find servers.js route file
echo "=== Finding servers.js files ==="
find backend -name "*servers*" -type f
# If found, show content around lines 95-98
echo -e "\n=== Content of servers route file ==="
if [ -f "backend/src/routes/servers.js" ]; then
sed -n '85,115p' backend/src/routes/servers.js
else
echo "Not found at expected location, searching..."
find backend -name "*servers.js" -exec echo "File: {}" \; -exec sed -n '85,115p' {} \;
fiRepository: blockmineJS/blockmine
Length of output: 1059
SQL-синтаксис совместим с SQLite, но есть проблема с ссылочной целостностью.
Синтаксис length(CAST(port AS TEXT)) поддерживается SQLite, поэтому предыдущие опасения по поводу совместимости не применяются. Однако при удалении серверов без проверки зависимых ботов произойдет нарушение ссылочной целостности: в маршруте DELETE (servers.js, строки 95-104) выполняется проверка наличия ботов перед удалением, но здесь в миграции при старте эта проверка отсутствует. Это приведет к созданию осиротевших записей в таблице Bot.
Кроме того, используется $executeRawUnsafe, что снижает типобезопасность. Рекомендуется заменить на $executeRaw с tagged template или добавить проверку наличия зависимых ботов перед удалением, аналогично API-маршруту.
🤖 Prompt for AI Agents
In backend/src/server.js around lines 105 to 113, the migration deletes Server
rows with invalid ports using prisma.$executeRawUnsafe which risks referential
integrity by orphaning Bot records and reduces safety; update this startup
migration to first identify servers with invalid ports, check for dependent Bot
records (similar to the DELETE route in servers.js lines 95-104), and only
delete servers that have no bots, or delete bots first in a controlled way, and
replace $executeRawUnsafe with Prisma's tagged $executeRaw (or use parameterized
queries) to avoid unsafe raw SQL execution.
Описание
Исправлена проблема с невалидными портами серверов, которая приводила к ошибке
Value does not fit in an INT columnпри попытке создать сервер с портомбольше 5 цифр (больше 65535).
Изменения
Валидация портов
Автоматическая очистка
Файлы изменены
backend/src/api/routes/servers.js- валидация в API endpointsbackend/src/server.js- автоочистка при стартеТестирование
Summary by Sourcery
Обеспечить использование допустимых диапазонов серверных портов и очистку некорректных записей о серверах при запуске.
Исправления ошибок:
Улучшения:
Original summary in English
Summary by Sourcery
Enforce valid server port ranges and clean up invalid server records on startup.
Bug Fixes:
Enhancements:
Summary by CodeRabbit
Новые возможности
Исправления ошибок
✏️ Tip: You can customize this high-level summary in your review settings.