Repository navigation
[SEC-A05]: Validación Zod en endpoints mutables - #196
Merged
Merged
Conversation
Contributor
|
Warning Review limit reached
More reviews will be available in 17 minutes and 46 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughSe implementa un sistema exhaustivo de validación de entrada mediante el middleware ChangesMarco de validación centralizado con DTOs Zod
Validación integrada en rutas administrativas
Validación integrada en rutas de comercio
Validación integrada en rutas de entrega
Validación integrada en rutas de usuarios
Validación en reportes globales
Tests actualizados para nueva estructura de validación
Configuración TypeScript
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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 |
![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: 10
🧹 Nitpick comments (2)
src/modules/commerce/deliveries/delivery.routes.js (1)
244-244: ⚡ Quick winConsolidar los dos DTO de
paramsen uno solo.Igual que en
addresses.routes.js, se encadenanvalidate(IdParamDTO, "params")yvalidate(DeliveryIdParamDTO, "params")sobre el mismoreq.params. Un DTO combinado conidydeliveryIdy una sola llamada es más claro y menos dependiente del orden/implementación del middleware. Verificá también queDeliveryIdParamDTOvalide la clavedeliveryIdque usa la ruta.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/commerce/deliveries/delivery.routes.js` at line 244, La ruta que registra el handler deleteStoreDelivery está validando req.params dos veces con IdParamDTO y DeliveryIdParamDTO; crea un DTO combinado (por ejemplo StoreDeliveryParamsDTO) que declare los campos id y deliveryId, reemplaza las dos llamadas validate(IdParamDTO, "params") y validate(DeliveryIdParamDTO, "params") por una sola validate(StoreDeliveryParamsDTO, "params") y asegúrate de que la propiedad deliveryId del nuevo DTO coincida exactamente con el nombre usado en la ruta; deja deleteStoreDelivery y los middlewares authenticate y requireRole(ROLES.SELLER) sin cambios.src/modules/commerce/addresses/routes/addresses.routes.js (1)
23-25: ⚡ Quick winConsolidar la validación de
paramsen un solo DTO (PUT/DELETE)En
src/modules/commerce/addresses/routes/addresses.routes.js(lín. 23-25) se ejecutavalidate(..., "params")dos veces sobrereq.params; el middleware hacesafeParse(req[section])y luegoObject.assign(req[section], ...), así que el parse ocurre dos veces y queda redundante/frágil. Conviene un DTO único (p. ej.{ id, id_address }) y una sola llamada.
AddressIdParamDTOya valida la claveid_address, que coincide con la ruta/:id/addresses/:id_address, por lo que esa parte está OK.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/modules/commerce/addresses/routes/addresses.routes.js` around lines 23 - 25, Consolida la validación de req.params creando un único DTO que incluya { id, id_address } (por ejemplo StoreAddressParamsDTO) y usa solo una llamada a validate(..., "params") en las rutas PUT y DELETE; reemplaza las dos validaciones actuales (validate(IdParamDTO, "params"), validate(AddressIdParamDTO, "params")) por validate(StoreAddressParamsDTO, "params"), ajusta los imports para incluir el nuevo DTO y conserva las demás llamadas (validate(UpdateAddressDTO, "body"), authenticate) y los handlers updateStoreAddress y deleteStoreAddress sin cambios.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/middlewares/validate.middleware.js`:
- Line 25: El problema es que Object.assign(req[section], result.data) conserva
propiedades antiguas no validadas en req[section]; cambia la lógica en el
middleware de validación para descartar las propiedades previas y quedarse solo
con result.data: en la función/middleware donde aparece
Object.assign(req[section], result.data) (validate.middleware.js) sustituye ese
comportamiento por limpiar/reescribir completamente req[section] con los valores
validados (por ejemplo asignando req[section] = result.data o borrando las
claves existentes antes de copiar), de modo que solo queden las propiedades
parseadas por result.data y no se filtren campos extra a los controladores.
In `@src/modules/global/dtos/banners/admin-banner.dto.js`:
- Around line 3-5: La validación actual de dateString usando z.string().refine
con new Date(...) acepta valores que no son fechas de calendario válidas;
reemplazá esa validación por una combinación de validadores de Zod 4: usar
z.iso.date() o z.iso.datetime() y encadenar .pipe(z.coerce.date()) para forzar y
comprobar que la cadena representa una Date válida (referenciar la constante
dateString y la expresión z.string().refine en el diff); si el resto del
DTO/rutas debe seguir recibiendo string en lugar de Date, en lugar de coerción
usá superRefine sobre dateString para validar la existencia del calendario con
z.coerce.date() internamente o convertí la salida de vuelta a string tras la
coerción, y actualizá cualquier consumidor del DTO para aceptar el tipo
resultante (Date o string) según corresponda.
In `@src/modules/global/dtos/business-hours/business-hours.dto.js`:
- Around line 8-9: The current regex for open_time and close_time allows
single-digit hours (e.g., "9:30") but the DTO message and declared format
require HH:mm; update the validator in the open_time and close_time
z.string().regex calls to enforce two-digit hours (e.g., hours 00-23) by
replacing the pattern with one that requires exactly two hour digits, and/or
update the error message to match the accepted format—ensure you change both
occurrences (open_time and close_time) so the validation and the error text are
consistent.
In `@src/modules/global/dtos/common/params.dto.js`:
- Around line 3-45: The param DTOs (IdParamDTO, CustomerIdParamDTO,
CartIdParamDTO, CartItemIdParamDTO, OrderIdParamDTO, WishlistIdParamDTO,
ProductIdParamDTO, ReportIdParamDTO, ReviewIdParamDTO, DeliveryIdParamDTO,
AddressIdParamDTO) currently use transform(Number) which accepts "1e2" or
"0x10"; restrict to strictly decimal positive integers by first validating the
string with a regex that allows only digits and no leading zero (e.g.
/^[1-9]\d*$/) and then convert to Number, keeping the existing
z.number().int().positive() pipe for type checks and error messages. Ensure the
regex validation runs on the same string field name used in each DTO before
transformation so only valid decimal digit strings are converted.
In `@src/modules/global/dtos/deliveries/deliveries.dto.js`:
- Around line 3-22: The middleware currently does Object.assign(req[section],
result.data) which leaves extra/unvalidated keys in the original req[section];
update the validate middleware so it first removes any keys from req[section]
that are not present in result.data (iterate own keys and delete those missing
in result.data) and then assign the validated keys (or copy each key from
result.data into req[section]) to preserve the original object reference; apply
this change to the validate flow used for payloads validated against DTOs like
RegisterDeliveryDTO, UpdateDeliveryStatusDTO and UpdateDeliveryProfileDTO so
extra body fields are stripped before the handler sees them.
- Around line 16-22: UpdateDeliveryProfileDTO currently allows an empty object;
require at least one updatable field by adding a Zod-level validation to
UpdateDeliveryProfileDTO that rejects an object with no keys. Locate the
UpdateDeliveryProfileDTO z.object (fields: name, phone, vehicleType) and add a
.refine or .superRefine that checks Object.keys(value).length > 0 (or
equivalent) and returns a clear error message like "Al menos un campo debe estar
presente" when validation fails.
In `@src/modules/global/dtos/product-categories/admin-category.dto.js`:
- Line 4: Replace the deprecated Zod option key "message" with the unified
"error" in the schema for the name field: locate the z.string call used for the
name property (symbol: name, expression: z.string(...)) and change the passed
option object from { message: "El nombre de la categoría no puede estar vacío" }
to { error: "El nombre de la categoría no puede estar vacío" }; also scan nearby
DTO fields for any other z.*(... { message: ... }) usages and update them
similarly to { error: ... } to be Zod v4 compliant.
In `@src/modules/global/dtos/product-tags/admin-tag.dto.js`:
- Line 4: Replace the incorrect z.string({ message: ... }) usage with the Zod v4
form using the "error" key so the custom text is applied; in
src/modules/global/dtos/product-tags/admin-tag.dto.js update the z.string()
calls (notably the one for the name field and the other z.string instance
referenced in the review) to use { error: "..." } instead of { message: "..." }
so the custom error messages are honored by Zod v4.
In `@src/modules/users/product-review/product-review.routes.js`:
- Line 9: La ruta POST registrada con router.post("/", authenticate,
validate(CreateProductReviewDTO, "body"), createProductReview) no valida
req.params.id (el :id del mount padre); agrega una validación de params para
asegurar que el id del producto sea válido antes de llegar a
createProductReview. Concreta: usa la función validate para validar los
parámetros (validate(YourIdParamDTO, "params") o un DTO existente que represente
{ id: number }) y colócala entre authenticate y createProductReview en la misma
llamada a router.post para validar req.params.id.
In `@src/modules/users/wishlist/wishlist.routes.js`:
- Line 32: El body validado por CreateWishlistItemDTO usa fk_product mientras
que addWishlistItemService espera productId, provocando que requests con {
productId, quantity } sean rechazadas; para corregirlo, unifica el nombre de
campo (recomiendo adaptar CreateWishlistItemDTO para aceptar productId en lugar
de fk_product), actualizar la validación en CreateWishlistItemDTO para exigir
productId y quantity, ajustar cualquier destructuring en addWishlistItem or
addWishlistItemService para usar productId si aún no lo hace, y actualizar la
documentación de la ruta para reflejar el campo productId consistentemente.
---
Nitpick comments:
In `@src/modules/commerce/addresses/routes/addresses.routes.js`:
- Around line 23-25: Consolida la validación de req.params creando un único DTO
que incluya { id, id_address } (por ejemplo StoreAddressParamsDTO) y usa solo
una llamada a validate(..., "params") en las rutas PUT y DELETE; reemplaza las
dos validaciones actuales (validate(IdParamDTO, "params"),
validate(AddressIdParamDTO, "params")) por validate(StoreAddressParamsDTO,
"params"), ajusta los imports para incluir el nuevo DTO y conserva las demás
llamadas (validate(UpdateAddressDTO, "body"), authenticate) y los handlers
updateStoreAddress y deleteStoreAddress sin cambios.
In `@src/modules/commerce/deliveries/delivery.routes.js`:
- Line 244: La ruta que registra el handler deleteStoreDelivery está validando
req.params dos veces con IdParamDTO y DeliveryIdParamDTO; crea un DTO combinado
(por ejemplo StoreDeliveryParamsDTO) que declare los campos id y deliveryId,
reemplaza las dos llamadas validate(IdParamDTO, "params") y
validate(DeliveryIdParamDTO, "params") por una sola
validate(StoreDeliveryParamsDTO, "params") y asegúrate de que la propiedad
deliveryId del nuevo DTO coincida exactamente con el nombre usado en la ruta;
deja deleteStoreDelivery y los middlewares authenticate y
requireRole(ROLES.SELLER) sin cambios.
🪄 Autofix (Beta)
✅ Autofix completed
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 38abba73-df21-41c0-b4eb-612d844f9be0
📒 Files selected for processing (47)
src/middlewares/validate.middleware.jssrc/modules/admin/banners/admin-banners.routes.jssrc/modules/admin/categories/admin-category.routes.jssrc/modules/admin/products/admin-products.routes.jssrc/modules/admin/stores/admin-stores.routes.jssrc/modules/admin/tags/admin-tag.routes.jssrc/modules/commerce/addresses/routes/addresses.routes.jssrc/modules/commerce/business-hours/routes/business-hours.routes.jssrc/modules/commerce/category-requests/category-request.routes.jssrc/modules/commerce/commerces/store.routes.jssrc/modules/commerce/deliveries/delivery.routes.jssrc/modules/commerce/products/product.routes.jssrc/modules/delivery/delivery-assignments/delivery-assignments.routes.jssrc/modules/delivery/delivery/delivery.routes.jssrc/modules/global/dtos/banners/admin-banner.dto.jssrc/modules/global/dtos/base/base.param.dto.jssrc/modules/global/dtos/business-hours/business-hours.dto.jssrc/modules/global/dtos/cart/cart.dto.jssrc/modules/global/dtos/category-requests/category-request.dto.jssrc/modules/global/dtos/commerce-deliveries/commerce-deliveries.dto.jssrc/modules/global/dtos/commerce/admin-store.dto.jssrc/modules/global/dtos/commerce/store.request.dto.jssrc/modules/global/dtos/common/params.dto.jssrc/modules/global/dtos/deliveries/deliveries.dto.jssrc/modules/global/dtos/delivery-assignments/delivery-assignments.dto.jssrc/modules/global/dtos/orders/order.dto.jssrc/modules/global/dtos/product-categories/admin-category.dto.jssrc/modules/global/dtos/product-reports/product-report.dto.jssrc/modules/global/dtos/product-tags/admin-tag.dto.jssrc/modules/global/dtos/products/admin-product.dto.jssrc/modules/global/dtos/review-reports/review-report.dto.jssrc/modules/global/reports/product/product-report.routes.jssrc/modules/global/reports/review/review-report.routes.jssrc/modules/users/addresses/routes/addresses.routes.jssrc/modules/users/cart/cart.routes.jssrc/modules/users/orders/order.routes.jssrc/modules/users/product-review/product-review.routes.jssrc/modules/users/wishlist/wishlist.routes.jstests/unit/admin/admin-category.test.jstests/unit/admin/admin-tags.test.jstests/unit/commerce/category-request.test.jstests/unit/commerce/delivery.test.jstests/unit/commerce/store-status.test.jstests/unit/delivery/delivery-complete.test.jstests/unit/delivery/delivery-profile.test.jstests/unit/delivery/delivery-register.test.jstests/unit/delivery/delivery-status.test.js
Contributor
|
Note Autofix is a beta feature. Expect some limitations and changes as we gather feedback and continue to improve it. Fixes Applied SuccessfullyFixed 10 file(s) based on 10 unresolved review comments. Files modified:
Commit: The changes have been pushed to the Time taken: |
Fixed 10 file(s) based on 10 unresolved review comments. Co-authored-by: CodeRabbit <noreply@coderabbit.ai>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/modules/global/dtos/product-categories/admin-category.dto.js`:
- Line 4: The Zod schema for the category DTO uses z.string with errorMap which
is not supported in Zod v4; update the schema where the property name is defined
(the `name` field in admin-category.dto.js) to use the `error` option instead of
`errorMap`, e.g. replace the errorMap customization on the `name` z.string(...)
call with the appropriate `error` object so the custom message "El nombre de la
categoría no puede estar vacío" is applied correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 8af6af20-69d3-4158-bdaf-29eb3cc1ca72
📒 Files selected for processing (9)
src/modules/global/dtos/banners/admin-banner.dto.jssrc/modules/global/dtos/business-hours/business-hours.dto.jssrc/modules/global/dtos/common/params.dto.jssrc/modules/global/dtos/deliveries/deliveries.dto.jssrc/modules/global/dtos/product-categories/admin-category.dto.jssrc/modules/global/dtos/product-tags/admin-tag.dto.jssrc/modules/global/dtos/wishlists/wishlist.dto.jssrc/modules/users/product-review/product-review.routes.jstsconfig.json
✅ Files skipped from review due to trivial changes (3)
- src/modules/global/dtos/product-tags/admin-tag.dto.js
- src/modules/global/dtos/common/params.dto.js
- tsconfig.json
🚧 Files skipped from review as they are similar to previous changes (2)
- src/modules/global/dtos/deliveries/deliveries.dto.js
- src/modules/global/dtos/banners/admin-banner.dto.js
|
CrisNAC
approved these changes
Jun 3, 2026
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.
Summary by CodeRabbit
Release Notes