Repository navigation
Conversation
Contributor
📝 WalkthroughWalkthroughSe añadieron rutas y controladores para actualización de tiendas y direcciones, un nuevo módulo de categorías de tiendas, ampliaciones para actualización de usuarios y contraseñas, refactor/normalización del servicio de tiendas (incluye manejo de logo y validaciones), y se activó Changes
Sequence Diagram(s)sequenceDiagram
participant Cliente as Cliente
participant API as API (Express)
participant JWT as Auth (JWT)
participant Ctrl as Store Controller
participant Svc as Store Service
participant DB as Prisma/DB
participant FS as Filesystem
Cliente->>API: PUT /api/commerces/:id (payload, cookie)
API->>JWT: verificar token/cookie
JWT-->>API: usuario autenticado (id)
API->>Ctrl: updateStore(req)
Ctrl->>Svc: updateStoreService(authId, storeId, payload)
Svc->>DB: validar tienda y propietario (SELECT)
DB-->>Svc: tienda existente
alt nuevo logo incluido
Svc->>FS: guardar/validar logo, borrar logo previo si aplica
FS-->>Svc: resultado archivo
end
Svc->>DB: UPDATE store (datos y/o logo)
DB-->>Svc: store actualizado
Svc-->>Ctrl: resultado
Ctrl-->>API: 200 {success, data}
API-->>Cliente: 200 OK (JSON)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutos Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (5)
src/modules/commerce/store-categories/store-category.service.js (1)
3-14: Código duplicado:parsePositiveInteger.Como se mencionó en
addresses.services.js, esta función está duplicada en múltiples archivos. Considerar consolidarla en un módulo compartido.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/store-categories/store-category.service.js` around lines 3 - 14, La función duplicada parsePositiveInteger debe extraerse a un módulo util compartido; create/export a single helper (e.g., export function parsePositiveInteger(value, fieldName)) en un nuevo archivo de util común y reemplaza las implementaciones locales en store-category.service.js (y dondequiera que exista, p. ej. addresses.services.js) para importar esa función en lugar de definirla inline; asegúrate de mantener la misma firma y los mismos errores lanzados para no romper los consumidores y actualizar las declaraciones import correspondientes.src/modules/users/addresses/services/addresses.services.js (1)
17-28: Código duplicado:parsePositiveIntegerestá repetido en múltiples archivos.Esta función helper está duplicada en al menos 4 archivos del proyecto:
src/modules/users/users/services/users.services.jssrc/modules/users/addresses/services/addresses.services.jssrc/modules/commerce/store-categories/store-category.service.jssrc/modules/commerce/commerces/store.service.jsConsiderar extraerla a un módulo de utilidades compartidas para evitar duplicación y facilitar mantenimiento.
♻️ Sugerencia: crear un módulo compartido
Crear un archivo como
src/lib/validators.js:export const parsePositiveInteger = (value, fieldName) => { const parsedValue = Number(value); if (!Number.isInteger(parsedValue) || parsedValue <= 0) { throw { status: 400, message: `${fieldName} invalido`, }; } return parsedValue; };Luego importarlo donde se necesite:
import { parsePositiveInteger } from "../../../lib/validators.js";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/addresses/services/addresses.services.js` around lines 17 - 28, La función helper parsePositiveInteger está duplicada en varios servicios; extrae esa función a un módulo utilitario compartido (por ejemplo lib/validators.js), exporta parsePositiveInteger desde allí, y en los archivos que contienen las copias (p. ej. en los servicios que definen parsePositiveInteger) reemplaza la implementación duplicada por una importación: import { parsePositiveInteger } from "..."; y elimina las versiones locales para que todas las referencias usen la única función exportada.src/modules/users/users/services/users.services.js (2)
7-7: Código duplicado:EMAIL_REGEXestá definido en dos lugares.Esta constante está duplicada en:
src/modules/users/users/services/users.services.js(línea 7)src/modules/users/users/controllers/users.controllers.js(línea 7)Considerar centralizar las constantes de validación en un módulo compartido.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/users/services/users.services.js` at line 7, EMAIL_REGEX is duplicated; extract the constant into a single shared module (e.g., export const EMAIL_REGEX from a new validation/regex module) then replace the local definitions in users.services.js and users.controllers.js with imports of that exported EMAIL_REGEX; remove the duplicated declarations so both the code that references EMAIL_REGEX (in the functions/methods inside UsersService and UsersController) use the shared constant.
38-49: Código duplicado:parsePositiveInteger.Ya mencionado en otros archivos - esta función helper está repetida en al menos 4 lugares del proyecto.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/users/services/users.services.js` around lines 38 - 49, La función parsePositiveInteger está duplicada; extrae parsePositiveInteger a un helper compartido (por ejemplo crear y exportar parsePositiveInteger desde un módulo util como numberUtils/numberHelpers.js), eliminar la definición local en users.services.js y reemplazarla por una import { parsePositiveInteger } desde el nuevo módulo; luego actualizar las otras instancias duplicadas en el proyecto para importar la misma función (mantener la firma actual y el comportamiento de lanzar {status, message} para valores inválidos) y ejecutar tests/linter para verificar que no haya referencias rotas.src/modules/users/users/controllers/users.controllers.js (1)
73-95: Considerar no devolver datos del usuario en la respuesta de cambio de contraseña.Por buenas prácticas de seguridad, la respuesta de un endpoint de cambio de contraseña típicamente no incluye datos del usuario. Un simple mensaje de éxito suele ser suficiente.
🔒 Sugerencia
return res.status(200).json({ success: true, message: "Contrasena actualizada exitosamente", - data: user, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/users/users/controllers/users.controllers.js` around lines 73 - 95, La respuesta del controlador updateUserPassword está devolviendo datos del usuario (campo data) tras llamar a updateUserPasswordService; elimina el objeto user de la respuesta para no filtrar información sensible y devuelve solo el estado y un mensaje de éxito (por ejemplo { success: true, message: "Contraseña actualizada exitosamente" }) y mantén el manejo de errores existente en el bloque catch; verifica también que updateUserPasswordService no añada efectos secundarios que expongan datos antes de la respuesta.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/commerce/commerces/store.controller.js`:
- Around line 76-81: La ruta está desechando filtros: en lugar de solo extraer
category, price_min y price_max desde req.query, extrae y reenvía todos los
parámetros que soporta el servicio (name, minPrice, maxPrice, visible, sortBy,
sortOrder, además de category) a filterStorePriductsService; mapea los nombres
de query a los que espera el servicio (por ejemplo price_min -> minPrice,
price_max -> maxPrice) y convierte visible a booleano antes de pasar al
servicio; ajusta la llamada a filterStorePriductsService(id, { name, category,
minPrice, maxPrice, visible: Boolean(...), sortBy, sortOrder }) para asegurar
que todos los filtros lleguen correctamente.
In `@src/modules/commerce/commerces/store.routes.js`:
- Around line 14-15: La ruta POST está pública y createStoreService acepta
fk_user desde el body, permitiendo crear comercios a nombre de otro; fix: add
the authenticate middleware to the router.post("/", ...) declaration (same
pattern as router.put) and change the flow so the store owner comes from
req.user (e.g., extract req.user.id in the createStore controller or inside
createStoreService) and ignore or overwrite any fk_user from req.body; also
validate/normalize the payload so fk_user is set from req.user before calling
createStoreService and reject requests where req.user is missing.
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 708-727: Validar y convertir los filtros numéricos antes de
asignarlos a whereConditions: en la sección que maneja category,
resolvedMinPrice y resolvedMaxPrice (las variables category, minPrice/price_min,
maxPrice/price_max y el objeto whereConditions), parsear con Number() o
parseInt() y comprobar con Number.isFinite/Number.isNaN (o isFinite) para
detectar NaN; si la conversión falla devolver un error de cliente (400) o lanzar
un BadRequestError con mensaje claro en lugar de insertar NaN en el where; sólo
cuando la conversión sea válida asignar whereConditions.fk_product_category y
whereConditions.price.gte/lte como Number(valor).
- Around line 209-229: The resolveLocalLogoPath function currently only checks
the first path segment allowing path-traversal like "uploads/../../.env"; update
it to reject traversal by resolving the candidate path against a locked base
directory for the allowed first segment and then verify the final resolved path
is inside that base directory. Specifically: compute the allowed base for the
firstDirectory (using ALLOWED_LOCAL_LOGO_DIRECTORIES), build the candidate path
by joining that base with the normalized relativeLogoPath, call path.resolve on
that join, and then confirm the resolved path starts with the resolved base
directory path plus a separator; if it does not, return null. Keep the existing
checks (empty, absolute URL) and ensure resolveLocalLogoPath returns null on any
traversal attempt.
- Around line 85-136: Las helpers actuales (normalizeOptionalStringValue,
validateRequiredStringField, validateOptionalStringField) convierten cualquier
input a texto con .toString(), lo que permite persistir objetos/arrays; cambia
la lógica para aceptar sólo valores string (o undefined/null para opcionales),
es decir: en validateRequiredStringField rechaza con 400 si typeof value !==
'string' o si el string trimmed queda vacío; en normalizeOptionalStringValue y
validateOptionalStringField no uses .toString() — acepta undefined -> undefined,
null/'' -> null, y si value no es string lanza 400; conserva las comprobaciones
de maxLength usando normalizedValue.length.
- Around line 581-587: La validación actual usando isNaN(Number(id)) permite
valores no enteros y no positivos (ej. 1.5, 0, -1); reemplazá esa comprobación
por la utilidad parsePositiveInteger para validar y parsear IDs públicos, y
usala antes de llamar prisma.stores.findUnique (referencia: variable id,
prisma.stores.findUnique, STORE_RESPONSE_SELECT). Hacé el mismo cambio en
getAllProductsByStoreService y filterStorePriductsService para que todos
rechacen valores no enteros y no positivos y pasen un Number válido al where: {
id_store: ... } en lugar de admitir conversiones implícitas. Asegurate de
propagar el error o lanzar { status: 400, message: ... } consistente cuando
parsePositiveInteger falle.
In `@src/modules/users/addresses/services/addresses.services.js`:
- Around line 66-77: Add a maximum-length check for payload.address before
assigning to dataToUpdate.address: after trimming address in the existing block
that handles payload?.address, validate that address.length does not exceed the
same max length used for city/region/postal_code validation (or set a sensible
cap like 255 if those values are not obvious), and if it does exceed, throw a
400 error with a message consistent with other fields; keep using
payload.address and dataToUpdate.address to locate the change.
---
Nitpick comments:
In `@src/modules/commerce/store-categories/store-category.service.js`:
- Around line 3-14: La función duplicada parsePositiveInteger debe extraerse a
un módulo util compartido; create/export a single helper (e.g., export function
parsePositiveInteger(value, fieldName)) en un nuevo archivo de util común y
reemplaza las implementaciones locales en store-category.service.js (y
dondequiera que exista, p. ej. addresses.services.js) para importar esa función
en lugar de definirla inline; asegúrate de mantener la misma firma y los mismos
errores lanzados para no romper los consumidores y actualizar las declaraciones
import correspondientes.
In `@src/modules/users/addresses/services/addresses.services.js`:
- Around line 17-28: La función helper parsePositiveInteger está duplicada en
varios servicios; extrae esa función a un módulo utilitario compartido (por
ejemplo lib/validators.js), exporta parsePositiveInteger desde allí, y en los
archivos que contienen las copias (p. ej. en los servicios que definen
parsePositiveInteger) reemplaza la implementación duplicada por una importación:
import { parsePositiveInteger } from "..."; y elimina las versiones locales para
que todas las referencias usen la única función exportada.
In `@src/modules/users/users/controllers/users.controllers.js`:
- Around line 73-95: La respuesta del controlador updateUserPassword está
devolviendo datos del usuario (campo data) tras llamar a
updateUserPasswordService; elimina el objeto user de la respuesta para no
filtrar información sensible y devuelve solo el estado y un mensaje de éxito
(por ejemplo { success: true, message: "Contraseña actualizada exitosamente" })
y mantén el manejo de errores existente en el bloque catch; verifica también que
updateUserPasswordService no añada efectos secundarios que expongan datos antes
de la respuesta.
In `@src/modules/users/users/services/users.services.js`:
- Line 7: EMAIL_REGEX is duplicated; extract the constant into a single shared
module (e.g., export const EMAIL_REGEX from a new validation/regex module) then
replace the local definitions in users.services.js and users.controllers.js with
imports of that exported EMAIL_REGEX; remove the duplicated declarations so both
the code that references EMAIL_REGEX (in the functions/methods inside
UsersService and UsersController) use the shared constant.
- Around line 38-49: La función parsePositiveInteger está duplicada; extrae
parsePositiveInteger a un helper compartido (por ejemplo crear y exportar
parsePositiveInteger desde un módulo util como numberUtils/numberHelpers.js),
eliminar la definición local en users.services.js y reemplazarla por una import
{ parsePositiveInteger } desde el nuevo módulo; luego actualizar las otras
instancias duplicadas en el proyecto para importar la misma función (mantener la
firma actual y el comportamiento de lanzar {status, message} para valores
inválidos) y ejecutar tests/linter para verificar que no haya referencias rotas.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: f71b6927-3772-4e35-8be1-bbaf6f15b902
📒 Files selected for processing (14)
src/index.jssrc/modules/commerce/commerces/store.controller.jssrc/modules/commerce/commerces/store.routes.jssrc/modules/commerce/commerces/store.service.jssrc/modules/commerce/store-categories/store-category.controller.jssrc/modules/commerce/store-categories/store-category.routes.jssrc/modules/commerce/store-categories/store-category.service.jssrc/modules/session/controllers/session.controllers.jssrc/modules/users/addresses/controllers/addresses.controllers.jssrc/modules/users/addresses/routes/addresses.routes.jssrc/modules/users/addresses/services/addresses.services.jssrc/modules/users/users/controllers/users.controllers.jssrc/modules/users/users/routes/users.routes.jssrc/modules/users/users/services/users.services.js
Comment on lines
76
to
+81
| const { category, price_min, price_max } = req.query; | ||
| const products = await filterStorePriductsService(id, { category, price_min, price_max }); | ||
| const products = await filterStorePriductsService(id, { | ||
| category, | ||
| price_min, | ||
| price_max | ||
| }); |
Contributor
There was a problem hiding this comment.
La ruta de filtrado está descartando casi todos los filtros nuevos.
filterStorePriductsService ya soporta name, minPrice, maxPrice, visible, sortBy y sortOrder, pero desde el controller sólo se reenvían category, price_min y price_max. Hoy esos parámetros nunca llegan al service.
💡 Ajuste mínimo
- const { category, price_min, price_max } = req.query;
- const products = await filterStorePriductsService(id, {
- category,
- price_min,
- price_max
- });
+ const products = await filterStorePriductsService(id, req.query);Si además vas a exponer visible, conviene normalizarlo a booleano antes de invocar al service.
📝 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.
Suggested change
| const { category, price_min, price_max } = req.query; | |
| const products = await filterStorePriductsService(id, { category, price_min, price_max }); | |
| const products = await filterStorePriductsService(id, { | |
| category, | |
| price_min, | |
| price_max | |
| }); | |
| const products = await filterStorePriductsService(id, req.query); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.controller.js` around lines 76 - 81, La
ruta está desechando filtros: en lugar de solo extraer category, price_min y
price_max desde req.query, extrae y reenvía todos los parámetros que soporta el
servicio (name, minPrice, maxPrice, visible, sortBy, sortOrder, además de
category) a filterStorePriductsService; mapea los nombres de query a los que
espera el servicio (por ejemplo price_min -> minPrice, price_max -> maxPrice) y
convierte visible a booleano antes de pasar al servicio; ajusta la llamada a
filterStorePriductsService(id, { name, category, minPrice, maxPrice, visible:
Boolean(...), sortBy, sortOrder }) para asegurar que todos los filtros lleguen
correctamente.
Comment on lines
14
to
+15
| router.post("/", createStore); | ||
| router.put("/:id", authenticate, updateStore); |
Contributor
There was a problem hiding this comment.
POST / sigue permitiendo crear comercios a nombre de otro usuario.
Acá el alta quedó pública, y en src/modules/commerce/commerces/store.service.js createStoreService sigue tomando fk_user desde el body. Con eso cualquier cliente puede crear un comercio para otro seller sin autenticarse.
🔒 Ajuste mínimo en esta ruta
-router.post("/", createStore);
+router.post("/", authenticate, createStore);Además de proteger la ruta, el owner debería salir de req.user, no del payload.
📝 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.
Suggested change
| router.post("/", createStore); | |
| router.put("/:id", authenticate, updateStore); | |
| router.post("/", authenticate, createStore); | |
| router.put("/:id", authenticate, updateStore); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.routes.js` around lines 14 - 15, La ruta
POST está pública y createStoreService acepta fk_user desde el body, permitiendo
crear comercios a nombre de otro; fix: add the authenticate middleware to the
router.post("/", ...) declaration (same pattern as router.put) and change the
flow so the store owner comes from req.user (e.g., extract req.user.id in the
createStore controller or inside createStoreService) and ignore or overwrite any
fk_user from req.body; also validate/normalize the payload so fk_user is set
from req.user before calling createStoreService and reject requests where
req.user is missing.
Comment on lines
+85
to
+136
| const normalizeOptionalStringValue = (value) => { | ||
| if (value === undefined) { | ||
| return undefined; | ||
| } | ||
|
|
||
| if (value === null || value === "") { | ||
| return null; | ||
| } | ||
|
|
||
| const normalizedValue = value.toString().trim(); | ||
| return normalizedValue ? normalizedValue : null; | ||
| }; | ||
|
|
||
| const validateRequiredStringField = (value, fieldName, maxLength) => { | ||
| const normalizedValue = value?.toString().trim(); | ||
|
|
||
| if (!normalizedValue) { | ||
| throw { | ||
| status: 400, | ||
| message: `${fieldName} no puede estar vacio` | ||
| }; | ||
| } | ||
|
|
||
| if (maxLength && normalizedValue.length > maxLength) { | ||
| throw { | ||
| status: 400, | ||
| message: `${fieldName} no puede superar ${maxLength} caracteres` | ||
| }; | ||
| } | ||
|
|
||
| return normalizedValue; | ||
| }; | ||
|
|
||
| const validateOptionalStringField = (value, fieldName, maxLength) => { | ||
| const normalizedValue = normalizeOptionalStringValue(value); | ||
|
|
||
| if (normalizedValue === undefined) { | ||
| return undefined; | ||
| } | ||
|
|
||
| if (normalizedValue === null) { | ||
| return null; | ||
| } | ||
|
|
||
| if (maxLength && normalizedValue.length > maxLength) { | ||
| throw { | ||
| status: 400, | ||
| message: `${fieldName} no puede superar ${maxLength} caracteres` | ||
| }; | ||
| } | ||
|
|
||
| return normalizedValue; |
Contributor
There was a problem hiding this comment.
No conviertas valores arbitrarios a texto en las helpers de validación.
normalizeOptionalStringValue y validateRequiredStringField usan .toString() sobre cualquier input. Con eso, payloads como {} o [] terminan persistidos como "[object Object]" o "a,b" en name, phone, address, description, logo, etc., en vez de responder 400.
💡 Ajuste sugerido
const normalizeOptionalStringValue = (value) => {
if (value === undefined) {
return undefined;
}
if (value === null || value === "") {
return null;
}
- const normalizedValue = value.toString().trim();
+ if (typeof value !== "string") {
+ throw {
+ status: 400,
+ message: "El campo debe ser texto"
+ };
+ }
+
+ const normalizedValue = value.trim();
return normalizedValue ? normalizedValue : null;
};
const validateRequiredStringField = (value, fieldName, maxLength) => {
- const normalizedValue = value?.toString().trim();
+ if (typeof value !== "string") {
+ throw {
+ status: 400,
+ message: `${fieldName} debe ser texto`
+ };
+ }
+
+ const normalizedValue = value.trim();📝 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.
Suggested change
| const normalizeOptionalStringValue = (value) => { | |
| if (value === undefined) { | |
| return undefined; | |
| } | |
| if (value === null || value === "") { | |
| return null; | |
| } | |
| const normalizedValue = value.toString().trim(); | |
| return normalizedValue ? normalizedValue : null; | |
| }; | |
| const validateRequiredStringField = (value, fieldName, maxLength) => { | |
| const normalizedValue = value?.toString().trim(); | |
| if (!normalizedValue) { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} no puede estar vacio` | |
| }; | |
| } | |
| if (maxLength && normalizedValue.length > maxLength) { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} no puede superar ${maxLength} caracteres` | |
| }; | |
| } | |
| return normalizedValue; | |
| }; | |
| const validateOptionalStringField = (value, fieldName, maxLength) => { | |
| const normalizedValue = normalizeOptionalStringValue(value); | |
| if (normalizedValue === undefined) { | |
| return undefined; | |
| } | |
| if (normalizedValue === null) { | |
| return null; | |
| } | |
| if (maxLength && normalizedValue.length > maxLength) { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} no puede superar ${maxLength} caracteres` | |
| }; | |
| } | |
| return normalizedValue; | |
| const normalizeOptionalStringValue = (value) => { | |
| if (value === undefined) { | |
| return undefined; | |
| } | |
| if (value === null || value === "") { | |
| return null; | |
| } | |
| if (typeof value !== "string") { | |
| throw { | |
| status: 400, | |
| message: "El campo debe ser texto" | |
| }; | |
| } | |
| const normalizedValue = value.trim(); | |
| return normalizedValue ? normalizedValue : null; | |
| }; | |
| const validateRequiredStringField = (value, fieldName, maxLength) => { | |
| if (typeof value !== "string") { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} debe ser texto` | |
| }; | |
| } | |
| const normalizedValue = value.trim(); | |
| if (!normalizedValue) { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} no puede estar vacio` | |
| }; | |
| } | |
| if (maxLength && normalizedValue.length > maxLength) { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} no puede superar ${maxLength} caracteres` | |
| }; | |
| } | |
| return normalizedValue; | |
| }; | |
| const validateOptionalStringField = (value, fieldName, maxLength) => { | |
| const normalizedValue = normalizeOptionalStringValue(value); | |
| if (normalizedValue === undefined) { | |
| return undefined; | |
| } | |
| if (normalizedValue === null) { | |
| return null; | |
| } | |
| if (maxLength && normalizedValue.length > maxLength) { | |
| throw { | |
| status: 400, | |
| message: `${fieldName} no puede superar ${maxLength} caracteres` | |
| }; | |
| } | |
| return normalizedValue; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 85 - 136, Las
helpers actuales (normalizeOptionalStringValue, validateRequiredStringField,
validateOptionalStringField) convierten cualquier input a texto con .toString(),
lo que permite persistir objetos/arrays; cambia la lógica para aceptar sólo
valores string (o undefined/null para opcionales), es decir: en
validateRequiredStringField rechaza con 400 si typeof value !== 'string' o si el
string trimmed queda vacío; en normalizeOptionalStringValue y
validateOptionalStringField no uses .toString() — acepta undefined -> undefined,
null/'' -> null, y si value no es string lanza 400; conserva las comprobaciones
de maxLength usando normalizedValue.length.
Comment on lines
+209
to
+229
| const resolveLocalLogoPath = (logoValue) => { | ||
| const normalizedLogoValue = normalizeOptionalStringValue(logoValue); | ||
|
|
||
| if (!normalizedLogoValue || /^https?:\/\//i.test(normalizedLogoValue)) { | ||
| return null; | ||
| } | ||
|
|
||
| const relativeLogoPath = normalizedLogoValue | ||
| .replace(/\\/g, "/") | ||
| .replace(/^\/+/, ""); | ||
|
|
||
| if (!relativeLogoPath) { | ||
| return null; | ||
| } | ||
|
|
||
| const firstDirectory = relativeLogoPath.split("/")[0]; | ||
| if (!ALLOWED_LOCAL_LOGO_DIRECTORIES.has(firstDirectory)) { | ||
| return null; | ||
| } | ||
|
|
||
| return path.resolve(process.cwd(), relativeLogoPath); |
Contributor
There was a problem hiding this comment.
La resolución del logo permite salir del directorio permitido.
El chequeo actual sólo valida el primer segmento. Un valor como uploads/../../.env sigue pasando, path.resolve() termina fuera de uploads y el unlink de la próxima actualización puede borrar archivos arbitrarios dentro de process.cwd().
🔒 Ajuste sugerido
const resolveLocalLogoPath = (logoValue) => {
const normalizedLogoValue = normalizeOptionalStringValue(logoValue);
if (!normalizedLogoValue || /^https?:\/\//i.test(normalizedLogoValue)) {
return null;
}
- const relativeLogoPath = normalizedLogoValue
- .replace(/\\/g, "/")
- .replace(/^\/+/, "");
+ const relativeLogoPath = path.posix.normalize(
+ normalizedLogoValue.replace(/\\/g, "/").replace(/^\/+/, "")
+ );
- if (!relativeLogoPath) {
+ if (!relativeLogoPath || relativeLogoPath.startsWith("../")) {
return null;
}
- const firstDirectory = relativeLogoPath.split("/")[0];
+ const [firstDirectory, ...restSegments] = relativeLogoPath.split("/");
if (!ALLOWED_LOCAL_LOGO_DIRECTORIES.has(firstDirectory)) {
return null;
}
- return path.resolve(process.cwd(), relativeLogoPath);
+ if (restSegments.length === 0) {
+ return null;
+ }
+
+ const baseDirectory = path.resolve(process.cwd(), firstDirectory);
+ const resolvedLogoPath = path.resolve(baseDirectory, ...restSegments);
+
+ if (
+ resolvedLogoPath !== baseDirectory &&
+ !resolvedLogoPath.startsWith(`${baseDirectory}${path.sep}`)
+ ) {
+ return null;
+ }
+
+ return resolvedLogoPath;
};📝 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.
Suggested change
| const resolveLocalLogoPath = (logoValue) => { | |
| const normalizedLogoValue = normalizeOptionalStringValue(logoValue); | |
| if (!normalizedLogoValue || /^https?:\/\//i.test(normalizedLogoValue)) { | |
| return null; | |
| } | |
| const relativeLogoPath = normalizedLogoValue | |
| .replace(/\\/g, "/") | |
| .replace(/^\/+/, ""); | |
| if (!relativeLogoPath) { | |
| return null; | |
| } | |
| const firstDirectory = relativeLogoPath.split("/")[0]; | |
| if (!ALLOWED_LOCAL_LOGO_DIRECTORIES.has(firstDirectory)) { | |
| return null; | |
| } | |
| return path.resolve(process.cwd(), relativeLogoPath); | |
| const resolveLocalLogoPath = (logoValue) => { | |
| const normalizedLogoValue = normalizeOptionalStringValue(logoValue); | |
| if (!normalizedLogoValue || /^https?:\/\//i.test(normalizedLogoValue)) { | |
| return null; | |
| } | |
| const relativeLogoPath = path.posix.normalize( | |
| normalizedLogoValue.replace(/\\/g, "/").replace(/^\/+/, "") | |
| ); | |
| if (!relativeLogoPath || relativeLogoPath.startsWith("../")) { | |
| return null; | |
| } | |
| const [firstDirectory, ...restSegments] = relativeLogoPath.split("/"); | |
| if (!ALLOWED_LOCAL_LOGO_DIRECTORIES.has(firstDirectory)) { | |
| return null; | |
| } | |
| if (restSegments.length === 0) { | |
| return null; | |
| } | |
| const baseDirectory = path.resolve(process.cwd(), firstDirectory); | |
| const resolvedLogoPath = path.resolve(baseDirectory, ...restSegments); | |
| if ( | |
| resolvedLogoPath !== baseDirectory && | |
| !resolvedLogoPath.startsWith(`${baseDirectory}${path.sep}`) | |
| ) { | |
| return null; | |
| } | |
| return resolvedLogoPath; | |
| }; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 209 - 229, The
resolveLocalLogoPath function currently only checks the first path segment
allowing path-traversal like "uploads/../../.env"; update it to reject traversal
by resolving the candidate path against a locked base directory for the allowed
first segment and then verify the final resolved path is inside that base
directory. Specifically: compute the allowed base for the firstDirectory (using
ALLOWED_LOCAL_LOGO_DIRECTORIES), build the candidate path by joining that base
with the normalized relativeLogoPath, call path.resolve on that join, and then
confirm the resolved path starts with the resolved base directory path plus a
separator; if it does not, return null. Keep the existing checks (empty,
absolute URL) and ensure resolveLocalLogoPath returns null on any traversal
attempt.
Comment on lines
581
to
+587
| if (isNaN(Number(id))) { | ||
| throw { status: 400, message: "ID de tienda debe ser un número" }; | ||
| throw { status: 400, message: "ID de tienda debe ser un numero" }; | ||
| } | ||
|
|
||
| // Buscar comercio | ||
| const store = await prisma.stores.findUnique({ | ||
| where: { id_store: Number(id) }, | ||
| // Datos del comercio | ||
| select: { | ||
| id_store: true, | ||
| name: true, | ||
| description: true, | ||
| logo: true, | ||
| phone: true, | ||
| email: true, | ||
| website_url: true, | ||
| instagram_url: true, | ||
| tiktok_url: true, | ||
| status: true, | ||
| created_at: true, | ||
| user: { | ||
| select: { id_user: true, name: true, email: true } | ||
| }, | ||
| // Categoría del comercio y productos visibles | ||
| store_category: { | ||
| select: { id_store_category: true, name: true } | ||
| }, | ||
| products: { | ||
| where: { status: true, visible: true }, | ||
| select: { | ||
| id_product: true, | ||
| name: true, | ||
| price: true, | ||
| quantity: true, | ||
| visible: true, | ||
| product_category: { | ||
| select: { id_product_category: true, name: true } | ||
| } | ||
| } | ||
| }, | ||
| addresses: { | ||
| where: { status: true }, | ||
| select: { id_address: true } | ||
| } | ||
| } | ||
| select: STORE_RESPONSE_SELECT |
Contributor
There was a problem hiding this comment.
Reutilizá parsePositiveInteger para los IDs públicos.
Con la validación actual, 1.5, 0 o -1 pasan hasta el findUnique. En un campo Int eso termina en 500 o en un 404 engañoso, y el mismo patrón quedó repetido en getAllProductsByStoreService y filterStorePriductsService.
💡 Ajuste sugerido
- if (isNaN(Number(id))) {
- throw { status: 400, message: "ID de tienda debe ser un numero" };
- }
+ const storeId = parsePositiveInteger(id, "ID de tienda");
const store = await prisma.stores.findUnique({
- where: { id_store: Number(id) },
+ where: { id_store: storeId },
select: STORE_RESPONSE_SELECT
});📝 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.
Suggested change
| if (isNaN(Number(id))) { | |
| throw { status: 400, message: "ID de tienda debe ser un número" }; | |
| throw { status: 400, message: "ID de tienda debe ser un numero" }; | |
| } | |
| // Buscar comercio | |
| const store = await prisma.stores.findUnique({ | |
| where: { id_store: Number(id) }, | |
| // Datos del comercio | |
| select: { | |
| id_store: true, | |
| name: true, | |
| description: true, | |
| logo: true, | |
| phone: true, | |
| email: true, | |
| website_url: true, | |
| instagram_url: true, | |
| tiktok_url: true, | |
| status: true, | |
| created_at: true, | |
| user: { | |
| select: { id_user: true, name: true, email: true } | |
| }, | |
| // Categoría del comercio y productos visibles | |
| store_category: { | |
| select: { id_store_category: true, name: true } | |
| }, | |
| products: { | |
| where: { status: true, visible: true }, | |
| select: { | |
| id_product: true, | |
| name: true, | |
| price: true, | |
| quantity: true, | |
| visible: true, | |
| product_category: { | |
| select: { id_product_category: true, name: true } | |
| } | |
| } | |
| }, | |
| addresses: { | |
| where: { status: true }, | |
| select: { id_address: true } | |
| } | |
| } | |
| select: STORE_RESPONSE_SELECT | |
| const storeId = parsePositiveInteger(id, "ID de tienda"); | |
| const store = await prisma.stores.findUnique({ | |
| where: { id_store: storeId }, | |
| select: STORE_RESPONSE_SELECT | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 581 - 587, La
validación actual usando isNaN(Number(id)) permite valores no enteros y no
positivos (ej. 1.5, 0, -1); reemplazá esa comprobación por la utilidad
parsePositiveInteger para validar y parsear IDs públicos, y usala antes de
llamar prisma.stores.findUnique (referencia: variable id,
prisma.stores.findUnique, STORE_RESPONSE_SELECT). Hacé el mismo cambio en
getAllProductsByStoreService y filterStorePriductsService para que todos
rechacen valores no enteros y no positivos y pasen un Number válido al where: {
id_store: ... } en lugar de admitir conversiones implícitas. Asegurate de
propagar el error o lanzar { status: 400, message: ... } consistente cuando
parsePositiveInteger falle.
Comment on lines
708
to
+727
| if (category) { | ||
| whereConditions.fk_product_category = Number(category); | ||
| } | ||
|
|
||
| if (visible !== undefined && visible !== null) { | ||
| whereConditions.visible = visible; | ||
| } | ||
| if (minPrice !== undefined && minPrice !== null) { | ||
| whereConditions.price = { gte: Number(minPrice) }; | ||
|
|
||
| const resolvedMinPrice = minPrice ?? price_min; | ||
| const resolvedMaxPrice = maxPrice ?? price_max; | ||
|
|
||
| if (resolvedMinPrice !== undefined && resolvedMinPrice !== null) { | ||
| whereConditions.price = { gte: Number(resolvedMinPrice) }; | ||
| } | ||
| if (maxPrice !== undefined && maxPrice !== null) { | ||
| whereConditions.price = { ...whereConditions.price, lte: Number(maxPrice) }; | ||
|
|
||
| if (resolvedMaxPrice !== undefined && resolvedMaxPrice !== null) { | ||
| whereConditions.price = { | ||
| ...whereConditions.price, | ||
| lte: Number(resolvedMaxPrice) | ||
| }; |
Contributor
There was a problem hiding this comment.
Validá los filtros numéricos antes de armar el where.
category, minPrice/price_min y maxPrice/price_max se convierten con Number() sin chequear el resultado. Un querystring como ?category=abc&price_min=foo termina construyendo un filtro con NaN y Prisma devuelve 500 en vez de 400.
💡 Ajuste sugerido
if (category) {
- whereConditions.fk_product_category = Number(category);
+ whereConditions.fk_product_category = parsePositiveInteger(
+ category,
+ "category"
+ );
}
const resolvedMinPrice = minPrice ?? price_min;
const resolvedMaxPrice = maxPrice ?? price_max;
if (resolvedMinPrice !== undefined && resolvedMinPrice !== null) {
- whereConditions.price = { gte: Number(resolvedMinPrice) };
+ const parsedMinPrice = Number(resolvedMinPrice);
+ if (!Number.isFinite(parsedMinPrice)) {
+ throw { status: 400, message: "minPrice invalido" };
+ }
+ whereConditions.price = { gte: parsedMinPrice };
}
if (resolvedMaxPrice !== undefined && resolvedMaxPrice !== null) {
+ const parsedMaxPrice = Number(resolvedMaxPrice);
+ if (!Number.isFinite(parsedMaxPrice)) {
+ throw { status: 400, message: "maxPrice invalido" };
+ }
whereConditions.price = {
...whereConditions.price,
- lte: Number(resolvedMaxPrice)
+ lte: parsedMaxPrice
};
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 708 - 727,
Validar y convertir los filtros numéricos antes de asignarlos a whereConditions:
en la sección que maneja category, resolvedMinPrice y resolvedMaxPrice (las
variables category, minPrice/price_min, maxPrice/price_max y el objeto
whereConditions), parsear con Number() o parseInt() y comprobar con
Number.isFinite/Number.isNaN (o isFinite) para detectar NaN; si la conversión
falla devolver un error de cliente (400) o lanzar un BadRequestError con mensaje
claro en lugar de insertar NaN en el where; sólo cuando la conversión sea válida
asignar whereConditions.fk_product_category y whereConditions.price.gte/lte como
Number(valor).
Comment on lines
+66
to
+77
| if (payload?.address !== undefined) { | ||
| const address = payload.address?.toString().trim(); | ||
|
|
||
| if (!address) { | ||
| throw { | ||
| status: 400, | ||
| message: "address no puede estar vacio", | ||
| }; | ||
| } | ||
|
|
||
| dataToUpdate.address = address; | ||
| } |
Contributor
There was a problem hiding this comment.
Falta validación de longitud para el campo address.
Los campos city, region y postal_code tienen validación de longitud máxima, pero address no la tiene. Esto podría permitir strings excesivamente largos.
🛡️ Propuesta para agregar validación de longitud
if (payload?.address !== undefined) {
const address = payload.address?.toString().trim();
if (!address) {
throw {
status: 400,
message: "address no puede estar vacio",
};
}
+ if (address.length > 255) {
+ throw {
+ status: 400,
+ message: "address no puede superar 255 caracteres",
+ };
+ }
+
dataToUpdate.address = address;
}📝 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.
Suggested change
| if (payload?.address !== undefined) { | |
| const address = payload.address?.toString().trim(); | |
| if (!address) { | |
| throw { | |
| status: 400, | |
| message: "address no puede estar vacio", | |
| }; | |
| } | |
| dataToUpdate.address = address; | |
| } | |
| if (payload?.address !== undefined) { | |
| const address = payload.address?.toString().trim(); | |
| if (!address) { | |
| throw { | |
| status: 400, | |
| message: "address no puede estar vacio", | |
| }; | |
| } | |
| if (address.length > 255) { | |
| throw { | |
| status: 400, | |
| message: "address no puede superar 255 caracteres", | |
| }; | |
| } | |
| dataToUpdate.address = address; | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/users/addresses/services/addresses.services.js` around lines 66 -
77, Add a maximum-length check for payload.address before assigning to
dataToUpdate.address: after trimming address in the existing block that handles
payload?.address, validate that address.length does not exceed the same max
length used for city/region/postal_code validation (or set a sensible cap like
255 if those values are not obvious), and if it does exceed, throw a 400 error
with a message consistent with other fields; keep using payload.address and
dataToUpdate.address to locate the change.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
src/modules/commerce/commerces/store.service.js (4)
585-590:⚠️ Potential issue | 🟠 MajorUsá
parsePositiveIntegerpara todos los IDs públicos.La validación con
isNaN(Number(id))sigue dejando pasar1.5,0y-1. Después esos valores llegan afindUnique/findManyy terminan en 500 o en un 404 engañoso. Parseá una sola vez conparsePositiveIntegery reutilizá el entero resultante engetStoreByIdService,getAllProductsByStoreServiceyfilterStorePriductsService.Also applies to: 618-624, 678-705
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 585 - 590, Replace the ad-hoc isNaN(Number(id)) checks with a single call to the shared parsePositiveInteger utility: in getStoreByIdService, getAllProductsByStoreService and filterStorePriductsService parse the incoming id once (e.g. const parsedId = parsePositiveInteger(id)) and then use parsedId for all prisma queries (e.g. prisma.stores.findUnique({ where: { id_store: parsedId } }) and any findMany calls) so that non-integer, zero or negative values are rejected consistently; ensure parsePositiveInteger throws a 400 on bad input and reuse the parsed integer variable throughout each function instead of re-parsing.
85-136:⚠️ Potential issue | 🟠 MajorNo conviertas inputs arbitrarios a texto en las helpers de validación.
normalizeOptionalStringValueyvalidateRequiredStringFieldsiguen usandotoString(). Con eso,{}o[]terminan persistiéndose como"[object Object]"o"a,b"enname,phone,address,description,logo, etc., en vez de devolver 400; y algunos objetos pueden romper con 500 antes de llegar al manejo de errores esperado. Aceptá sólostring—oundefined/nullpara opcionales— y validá sobretrim().🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 85 - 136, The helpers normalizeOptionalStringValue, validateRequiredStringField and validateOptionalStringField currently call toString() and thus accept objects/arrays; change them to only accept values of type string (or undefined/null for optionals): in normalizeOptionalStringValue return undefined for undefined, return null for null or empty string after trimming only if typeof value === 'string' (otherwise throw 400), in validateRequiredStringField validate typeof value === 'string' then trim and check emptiness/length (throw 400 for non-strings or empty), and in validateOptionalStringField return undefined for undefined, null for null, but if provided ensure typeof value === 'string' before trimming and length-checking (throw 400 for non-strings). Ensure all maxLength checks use the trimmed string.
209-229:⚠️ Potential issue | 🔴 CriticalBloqueá path traversal al resolver logos locales.
El chequeo actual sigue validando sólo el primer segmento. Un valor como
uploads/../../.envpasa,path.resolve()termina fuera del directorio permitido y elunlinkde una actualización posterior puede borrar archivos arbitrarios del proyecto. Normalizá la ruta, resolvela contra un base dir permitido y rechazá cualquier resultado que quede fuera de ese base.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 209 - 229, La función resolveLocalLogoPath permite path traversal porque solo verifica el primer segmento; para arreglarlo, normalizá el valor con normalizeOptionalStringValue, construí relativeLogoPath como ya está, pero en lugar de usar path.resolve(process.cwd(), relativeLogoPath) directamente, resolvelo contra un directorio base permitido (p. ej. const baseDir = path.resolve(process.cwd(), firstDirectory) o el directorio raíz de uploads) y luego calculá const resolved = path.resolve(baseDir, relativeLogoPath); rechazá la ruta si resolved no comienza con baseDir (usando comparación de rutas normalizadas) o si firstDirectory no está en ALLOWED_LOCAL_LOGO_DIRECTORIES; devolvé null en esos casos para bloquear traversal antes de cualquier unlink o acceso.
691-701:⚠️ Potential issue | 🟠 MajorValidá los filtros antes de construir la consulta.
Number(category),Number(resolvedMinPrice)yNumber(resolvedMaxPrice)pueden darNaN/Infinity, ysortByentra directo como clave de ordenamiento. Con un querystring mal formado terminás armando una consulta inválida y devolviendo 500 en vez de 400. Parseá los numéricos con validación explícita y whitelistá los campos permitidos para ordenar.Also applies to: 712-748
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 691 - 701, Valida y sanea todos los filtros antes de construir la consulta: para los numéricos (category, resolvedMinPrice, resolvedMaxPrice, minPrice, maxPrice, price_min, price_max) usa parseInt/parseFloat y comprueba Number.isFinite(...) o isFinite(...) y rechaza con error 400 si no son válidos (también maneja Infinity y NaN); asegura que min ≤ max o intercámbialos; whitelistea los campos permitidos para ordenamiento comprobando sortBy contra una lista explícita (p. ej. ['name','price','createdAt']) y valida sortOrder sólo acepta 'asc' o 'desc' antes de usarlos en la consulta; aplica estas mismas validaciones donde se usan resolvedMinPrice/resolvedMaxPrice y sortBy/sortOrder en la construcción del query para evitar consultas inválidas que devuelvan 500.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 353-359: The createStoreService currently uses a direct
prisma.storeCategories.findUnique check (categoria/fk_store_category) that
allows inactive categories; replace that direct lookup with the existing
validateStoreCategoryService call so creation enforces the same "active"
constraint as update; update both occurrences (the block that checks categoria
and the similar block later around the other create path) to call
validateStoreCategoryService(fk_store_category) and handle/propagate its error
instead of throwing a plain "Categoria no valida".
- Around line 11-31: Separá el select público del interno creando un nuevo
PUBLIC_STORE_RESPONSE_SELECT que excluya fk_user y cualquier campo PII/flags de
la relación user (por ejemplo user.email, user.role, user.status) y que sólo
incluya los campos de contacto públicos (store.email, name, phone, logo,
website_url, etc.); deja el actual STORE_RESPONSE_SELECT intacto para flujos
autenticados. Luego actualizá getStoreByIdService (y la otra ocurrencia
indicada) para usar PUBLIC_STORE_RESPONSE_SELECT cuando la respuesta sea
pública/anonima, y solo usar STORE_RESPONSE_SELECT en endpoints autenticados o
internos; ajustá las llamadas/returns para referenciar el nuevo constante en
lugar del select enriquecido.
- Around line 533-547: La rama que crea una dirección (tx.addresses.create)
puede insertar campos obligatorios como address, city o region como cadenas
vacías cuando addressId no existe; ajustá la condición donde usás
addressDataToUpdate para verificar explícitamente que
addressDataToUpdate.address, .city y .region estén presentes y no vacíos antes
de llamar a tx.addresses.create (usando store.fk_user y store.id_store en data);
si faltan campos, no hagas el create y en su lugar devolvé un error de
validación o simplemente omití la creación según la lógica de negocio.
---
Duplicate comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 585-590: Replace the ad-hoc isNaN(Number(id)) checks with a single
call to the shared parsePositiveInteger utility: in getStoreByIdService,
getAllProductsByStoreService and filterStorePriductsService parse the incoming
id once (e.g. const parsedId = parsePositiveInteger(id)) and then use parsedId
for all prisma queries (e.g. prisma.stores.findUnique({ where: { id_store:
parsedId } }) and any findMany calls) so that non-integer, zero or negative
values are rejected consistently; ensure parsePositiveInteger throws a 400 on
bad input and reuse the parsed integer variable throughout each function instead
of re-parsing.
- Around line 85-136: The helpers normalizeOptionalStringValue,
validateRequiredStringField and validateOptionalStringField currently call
toString() and thus accept objects/arrays; change them to only accept values of
type string (or undefined/null for optionals): in normalizeOptionalStringValue
return undefined for undefined, return null for null or empty string after
trimming only if typeof value === 'string' (otherwise throw 400), in
validateRequiredStringField validate typeof value === 'string' then trim and
check emptiness/length (throw 400 for non-strings or empty), and in
validateOptionalStringField return undefined for undefined, null for null, but
if provided ensure typeof value === 'string' before trimming and length-checking
(throw 400 for non-strings). Ensure all maxLength checks use the trimmed string.
- Around line 209-229: La función resolveLocalLogoPath permite path traversal
porque solo verifica el primer segmento; para arreglarlo, normalizá el valor con
normalizeOptionalStringValue, construí relativeLogoPath como ya está, pero en
lugar de usar path.resolve(process.cwd(), relativeLogoPath) directamente,
resolvelo contra un directorio base permitido (p. ej. const baseDir =
path.resolve(process.cwd(), firstDirectory) o el directorio raíz de uploads) y
luego calculá const resolved = path.resolve(baseDir, relativeLogoPath); rechazá
la ruta si resolved no comienza con baseDir (usando comparación de rutas
normalizadas) o si firstDirectory no está en ALLOWED_LOCAL_LOGO_DIRECTORIES;
devolvé null en esos casos para bloquear traversal antes de cualquier unlink o
acceso.
- Around line 691-701: Valida y sanea todos los filtros antes de construir la
consulta: para los numéricos (category, resolvedMinPrice, resolvedMaxPrice,
minPrice, maxPrice, price_min, price_max) usa parseInt/parseFloat y comprueba
Number.isFinite(...) o isFinite(...) y rechaza con error 400 si no son válidos
(también maneja Infinity y NaN); asegura que min ≤ max o intercámbialos;
whitelistea los campos permitidos para ordenamiento comprobando sortBy contra
una lista explícita (p. ej. ['name','price','createdAt']) y valida sortOrder
sólo acepta 'asc' o 'desc' antes de usarlos en la consulta; aplica estas mismas
validaciones donde se usan resolvedMinPrice/resolvedMaxPrice y sortBy/sortOrder
en la construcción del query para evitar consultas inválidas que devuelvan 500.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d5f4d07d-6161-4036-80d6-188d0e056d80
📒 Files selected for processing (1)
src/modules/commerce/commerces/store.service.js
Comment on lines
+11
to
+31
| fk_user: true, | ||
| fk_store_category: true, | ||
| name: true, | ||
| email: true, | ||
| phone: true, | ||
| description: true, | ||
| logo: true, | ||
| website_url: true, | ||
| instagram_url: true, | ||
| tiktok_url: true, | ||
| status: true, | ||
| created_at: true, | ||
| updated_at: true, | ||
| user: { | ||
| select: { | ||
| id_user: true, | ||
| name: true, | ||
| email: true, | ||
| role: true, | ||
| status: true | ||
| } |
Contributor
There was a problem hiding this comment.
Separá el select público del select interno del comercio.
STORE_RESPONSE_SELECT incluye fk_user y datos de cuenta del vendedor (user.email, user.role, user.status), y getStoreByIdService los devuelve en una consulta pública. Como el comercio ya tiene store.email como contacto, esto expone PII y flags internas sin necesidad. Definí un PUBLIC_STORE_RESPONSE_SELECT más acotado y dejá el select enriquecido sólo para flujos autenticados.
Also applies to: 589-592
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 11 - 31, Separá
el select público del interno creando un nuevo PUBLIC_STORE_RESPONSE_SELECT que
excluya fk_user y cualquier campo PII/flags de la relación user (por ejemplo
user.email, user.role, user.status) y que sólo incluya los campos de contacto
públicos (store.email, name, phone, logo, website_url, etc.); deja el actual
STORE_RESPONSE_SELECT intacto para flujos autenticados. Luego actualizá
getStoreByIdService (y la otra ocurrencia indicada) para usar
PUBLIC_STORE_RESPONSE_SELECT cuando la respuesta sea pública/anonima, y solo
usar STORE_RESPONSE_SELECT en endpoints autenticados o internos; ajustá las
llamadas/returns para referenciar el nuevo constante en lugar del select
enriquecido.
Comment on lines
353
to
359
| const categoria = await prisma.storeCategories.findUnique({ | ||
| where: { id_store_category: fk_store_category } | ||
| }); | ||
|
|
||
| if (!categoria) { | ||
| throw { status: 400, message: "Categoría no válida" }; | ||
| throw { status: 400, message: "Categoria no valida" }; | ||
| } |
Contributor
There was a problem hiding this comment.
La creación todavía acepta categorías inactivas.
En createStoreService sólo se comprueba que la categoría exista, pero en updateStoreService ya usás validateStoreCategoryService, que también exige que esté activa. Hoy se puede crear un comercio en una categoría deshabilitada y después no volver a setear esa misma categoría desde update. Reutilizá esa helper también en la creación.
Also applies to: 414-417
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 353 - 359, The
createStoreService currently uses a direct prisma.storeCategories.findUnique
check (categoria/fk_store_category) that allows inactive categories; replace
that direct lookup with the existing validateStoreCategoryService call so
creation enforces the same "active" constraint as update; update both
occurrences (the block that checks categoria and the similar block later around
the other create path) to call validateStoreCategoryService(fk_store_category)
and handle/propagate its error instead of throwing a plain "Categoria no
valida".
Comment on lines
+533
to
+547
| if (addressId) { | ||
| await tx.addresses.update({ | ||
| where: { id_address: addressId }, | ||
| data: addressDataToUpdate | ||
| }); | ||
| } else if (Object.keys(addressDataToUpdate).length > 0) { | ||
| await tx.addresses.create({ | ||
| data: { | ||
| fk_user: store.fk_user, | ||
| fk_store: store.id_store, | ||
| address: addressDataToUpdate.address ?? "", | ||
| city: addressDataToUpdate.city ?? "", | ||
| region: addressDataToUpdate.region ?? "", | ||
| postal_code: addressDataToUpdate.postal_code ?? null | ||
| } |
Contributor
There was a problem hiding this comment.
No crees una dirección nueva con campos obligatorios vacíos.
Si el comercio no tiene una dirección activa y el payload trae sólo postal_code o un subconjunto de los campos, esta rama inserta address, city y/o region como "". Eso deja datos inconsistentes o depende de que la base rechace la fila. Cuando addressId no existe, exigí address, city y region completos antes de hacer el create.
💡 Ajuste sugerido
- } else if (Object.keys(addressDataToUpdate).length > 0) {
+ } else if (Object.keys(addressDataToUpdate).length > 0) {
+ if (
+ !addressDataToUpdate.address ||
+ !addressDataToUpdate.city ||
+ !addressDataToUpdate.region
+ ) {
+ throw {
+ status: 400,
+ message:
+ "address, city y region son obligatorios cuando el comercio no tiene direccion activa"
+ };
+ }
+
await tx.addresses.create({
data: {
fk_user: store.fk_user,
fk_store: store.id_store,
- address: addressDataToUpdate.address ?? "",
- city: addressDataToUpdate.city ?? "",
- region: addressDataToUpdate.region ?? "",
+ address: addressDataToUpdate.address,
+ city: addressDataToUpdate.city,
+ region: addressDataToUpdate.region,
postal_code: addressDataToUpdate.postal_code ?? null
}
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` around lines 533 - 547, La
rama que crea una dirección (tx.addresses.create) puede insertar campos
obligatorios como address, city o region como cadenas vacías cuando addressId no
existe; ajustá la condición donde usás addressDataToUpdate para verificar
explícitamente que addressDataToUpdate.address, .city y .region estén presentes
y no vacíos antes de llamar a tx.addresses.create (usando store.fk_user y
store.id_store en data); si faltan campos, no hagas el create y en su lugar
devolvé un error de validación o simplemente omití la creación según la lógica
de negocio.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/modules/commerce/commerces/store.service.js (1)
378-387:⚠️ Potential issue | 🔴 CriticalError crítico:
store.iddebería serstore.id_store.En la línea 381 se usa
fk_store: store.id, pero el modelo Prisma define que el campo identificador de stores esid_store, noid. El mismo archivo utiliza correctamentestore.id_storeen otras ubicaciones (líneas 427 y 440). Usarstore.idresultaría en un valorundefined, impidiendo que la dirección se vinculara correctamente al comercio.🔧 Corrección sugerida
await tx.addresses.create({ data: { fk_user, - fk_store: store.id, + fk_store: store.id_store, address: address.trim(), city: city.trim(), region: region.trim(), postal_code } });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 378 - 387, La creación de la dirección usa el identificador equivocado: en tx.addresses.create se está asignando fk_store: store.id pero el modelo de stores usa id_store; cambia esa asignación a fk_store: store.id_store para que no se inserte undefined y la dirección se relacione correctamente con el comercio (buscar la llamada tx.addresses.create y reemplazar store.id por store.id_store).
♻️ Duplicate comments (1)
src/modules/commerce/commerces/store.service.js (1)
209-230:⚠️ Potential issue | 🔴 CriticalVulnerabilidad de path traversal no corregida.
El chequeo actual solo valida el primer segmento. Un valor como
uploads/../../.envpasa la validación ypath.resolve()resuelve fuera del directorio permitido. Elunlinkpodría borrar archivos arbitrarios.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/commerce/commerces/store.service.js` around lines 209 - 230, The current resolveLocalLogoPath only checks the first path segment and remains vulnerable to path traversal; update resolveLocalLogoPath to fully normalize and validate the path: (1) reject any input containing parent-segment tokens (e.g., '..') or absolute roots after normalizing (use normalizeOptionalStringValue then path.posix.normalize or path.normalize and check for leading '..' segments), (2) compute the allowed base directory for the validated firstDirectory using ALLOWED_LOCAL_LOGO_DIRECTORIES and path.resolve(process.cwd(), firstDirectory), (3) resolve the candidate path via path.resolve(process.cwd(), relativeLogoPath) and ensure the resolved path is inside the allowed base directory by verifying resolvedPath === allowedBase || resolvedPath.startsWith(allowedBase + path.sep); if any check fails return null; keep references to resolveLocalLogoPath, normalizeOptionalStringValue, ALLOWED_LOCAL_LOGO_DIRECTORIES, path.resolve and process.cwd() so reviewers can locate and apply the fix.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Line 755: Validar que el parámetro sortBy provenga de una lista blanca antes
de usarlo en el objeto orderBy: define un array de campos permitidos (por
ejemplo allowedSortFields) y comprobar si sortBy está incluido; si no, caer a un
campo por defecto (por ejemplo "created_at"). Actualiza el lugar que construye
orderBy (la expresión que usa sortBy, sortOrder) para usar el valor
validado/mapeado (p. ej. validatedSortBy) en lugar de sortBy directo y mantener
la lógica de sortOrder para "asc"/"desc". Asegúrate de referenciar y cambiar las
variables sortBy, sortOrder y la construcción orderBy en store.service.js para
evitar pasar claves arbitrarias a Prisma.
- Line 679: Rename the exported function filterStorePriductsService to
filterStoreProductsService in store.service.js and update all call sites/imports
that reference the old name (the two usages in store.controller.js where this
service is imported and invoked) to use filterStoreProductsService; ensure the
export statement and any named imports/exports match the corrected identifier so
the module resolution and calls succeed.
---
Outside diff comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 378-387: La creación de la dirección usa el identificador
equivocado: en tx.addresses.create se está asignando fk_store: store.id pero el
modelo de stores usa id_store; cambia esa asignación a fk_store: store.id_store
para que no se inserte undefined y la dirección se relacione correctamente con
el comercio (buscar la llamada tx.addresses.create y reemplazar store.id por
store.id_store).
---
Duplicate comments:
In `@src/modules/commerce/commerces/store.service.js`:
- Around line 209-230: The current resolveLocalLogoPath only checks the first
path segment and remains vulnerable to path traversal; update
resolveLocalLogoPath to fully normalize and validate the path: (1) reject any
input containing parent-segment tokens (e.g., '..') or absolute roots after
normalizing (use normalizeOptionalStringValue then path.posix.normalize or
path.normalize and check for leading '..' segments), (2) compute the allowed
base directory for the validated firstDirectory using
ALLOWED_LOCAL_LOGO_DIRECTORIES and path.resolve(process.cwd(), firstDirectory),
(3) resolve the candidate path via path.resolve(process.cwd(), relativeLogoPath)
and ensure the resolved path is inside the allowed base directory by verifying
resolvedPath === allowedBase || resolvedPath.startsWith(allowedBase + path.sep);
if any check fails return null; keep references to resolveLocalLogoPath,
normalizeOptionalStringValue, ALLOWED_LOCAL_LOGO_DIRECTORIES, path.resolve and
process.cwd() so reviewers can locate and apply the fix.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: cf0bd7b0-3904-42f4-ac40-c6d8dad778a0
📒 Files selected for processing (1)
src/modules/commerce/commerces/store.service.js
| * @param {*} filters | ||
| * @returns | ||
| */ | ||
| export const filterStorePriductsService = async (id, filters) => { |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Buscar usos de filterStorePriductsService
rg -n "filterStorePriductsService"Repository: CrisNAC/BackendMarketplace
Length of output: 384
Corregir typo en el nombre de la función: filterStorePriductsService.
El nombre contiene un error tipográfico: "Priducts" en lugar de "Products". Requiere actualizar también las 2 referencias en store.controller.js (líneas 7 y 77).
✏️ Cambios requeridos
-export const filterStorePriductsService = async (id, filters) => {
+export const filterStoreProductsService = async (id, filters) => {En store.controller.js línea 7:
-import { filterStorePriductsService } from './store.service';
+import { filterStoreProductsService } from './store.service';En store.controller.js línea 77:
-const products = await filterStorePriductsService(id, {
+const products = await filterStoreProductsService(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.
Suggested change
| export const filterStorePriductsService = async (id, filters) => { | |
| export const filterStoreProductsService = async (id, filters) => { |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` at line 679, Rename the
exported function filterStorePriductsService to filterStoreProductsService in
store.service.js and update all call sites/imports that reference the old name
(the two usages in store.controller.js where this service is imported and
invoked) to use filterStoreProductsService; ensure the export statement and any
named imports/exports match the corrected identifier so the module resolution
and calls succeed.
| } | ||
| }, | ||
| // Ordenar por el campo especificado o por fecha de creación por defecto | ||
| orderBy: { [sortBy || "created_at"]: sortOrder === "asc" ? "asc" : "desc" } |
Contributor
There was a problem hiding this comment.
Validar sortBy contra una lista de campos permitidos.
El valor de sortBy viene del usuario y se usa directamente como clave en orderBy. Si el usuario envía un campo inexistente, Prisma lanzará un error 500. Validar contra una lista blanca de campos permitidos.
🛡️ Corrección sugerida
+ const ALLOWED_SORT_FIELDS = new Set([
+ "created_at",
+ "name",
+ "price",
+ "quantity"
+ ]);
+
+ const validSortBy = ALLOWED_SORT_FIELDS.has(sortBy) ? sortBy : "created_at";
+
const products = await prisma.products.findMany({
where: whereConditions,
select: {
// ...
},
- orderBy: { [sortBy || "created_at"]: sortOrder === "asc" ? "asc" : "desc" }
+ orderBy: { [validSortBy]: sortOrder === "asc" ? "asc" : "desc" }
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/modules/commerce/commerces/store.service.js` at line 755, Validar que el
parámetro sortBy provenga de una lista blanca antes de usarlo en el objeto
orderBy: define un array de campos permitidos (por ejemplo allowedSortFields) y
comprobar si sortBy está incluido; si no, caer a un campo por defecto (por
ejemplo "created_at"). Actualiza el lugar que construye orderBy (la expresión
que usa sortBy, sortOrder) para usar el valor validado/mapeado (p. ej.
validatedSortBy) en lugar de sortBy directo y mantener la lógica de sortOrder
para "asc"/"desc". Asegúrate de referenciar y cambiar las variables sortBy,
sortOrder y la construcción orderBy en store.service.js para evitar pasar claves
arbitrarias a Prisma.
CrisNAC
approved these changes
Mar 12, 2026
This was referenced Mar 12, 2026
Merged
Merged
Merged
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
-culpa de coderrabit
Summary by CodeRabbit
New Features
Bug Fixes