Prevent negative item quantity in edit form#1339
Prevent negative item quantity in edit form#1339FrederickStempfle wants to merge 1 commit intosysadminsmedia:mainfrom
Conversation
WalkthroughAdds non-negative quantity validation across backend structs and propagates a frontend numeric Changes
Estimated Code Review Effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly Related PRs
Suggested Reviewers
Security Recommendations
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)
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}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/internal/data/repo/repo_items.go (1)
644-656:⚠️ Potential issue | 🟡 MinorSecurity issue:
ItemCreateFromTemplate.Quantitymissingmin=0validation allows negative quantities through the template creation API.The endpoint
POST /v1/templates/{id}/create-itemaccepts user-suppliedquantitywith no validation constraints, then passes it directly toItemCreateFromTemplate—which lacks themin=0constraint present inItemCreateandItemUpdate. This creates a validation bypass where negative quantities can reach the database.Proposed fix
type ItemCreateFromTemplate struct { Name string Description string - Quantity int + Quantity int `validate:"min=0"` LocationID uuid.UUID TagIDs []uuid.UUIDAdditionally, add validation to the request body in the handler to catch invalid input early:
// In ItemTemplateCreateItemRequest Quantity *int `json:"quantity" validate:"omitempty,min=0"`🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_items.go` around lines 644 - 656, ItemCreateFromTemplate currently lacks validation on Quantity allowing negative values; add the same validation tag used in ItemCreate/ItemUpdate (min=0) to the Quantity field in the ItemCreateFromTemplate struct to enforce non-negative quantities, and update the API request struct ItemTemplateCreateItemRequest so its Quantity is a pointer with `validate:"omitempty,min=0"` to catch invalid input early in the POST /v1/templates/{id}/create-item handler; ensure any binding/validation call in that handler is invoked so bad values are rejected before persisting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/internal/data/repo/repo_items.go`:
- Line 74: Add a non-negative validation constraint to the template creation
path by updating the DTO and repo types: add validate:"min=0" to
ItemTemplateCreateItemRequest.Quantity (the pointer *int on the request struct
used in v1_ctrl_item_templates.go) and add validate:"min=0" to
ItemCreateFromTemplate.Quantity in backend/internal/data/repo/repo_items.go (or
alternatively perform an explicit check in the handler before calling
CreateFromTemplate()). Ensure the struct tags are applied to the declared fields
so the existing validate.Check()/validate.Struct() pipeline will reject negative
quantities.
---
Outside diff comments:
In `@backend/internal/data/repo/repo_items.go`:
- Around line 644-656: ItemCreateFromTemplate currently lacks validation on
Quantity allowing negative values; add the same validation tag used in
ItemCreate/ItemUpdate (min=0) to the Quantity field in the
ItemCreateFromTemplate struct to enforce non-negative quantities, and update the
API request struct ItemTemplateCreateItemRequest so its Quantity is a pointer
with `validate:"omitempty,min=0"` to catch invalid input early in the POST
/v1/templates/{id}/create-item handler; ensure any binding/validation call in
that handler is invoked so bad values are rejected before persisting.
FrederickStempfle
force-pushed
the
fix/negative-quantity-validation
branch
from
4d6076d to
dd5f32f
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/app/api/handlers/v1/v1_ctrl_item_templates.go (1)
133-136:⚠️ Potential issue | 🟠 MajorAdd
validate:"min=0"toDefaultQuantityfields in ItemTemplateCreate and ItemTemplateUpdate; add runtime guard in handler.
template.DefaultQuantity(read from database with no validation constraints) can be negative if the template was created/updated without proper validation. Whenbody.Quantity == nil, this negative default bypasses the API request validation and flows intoCreateFromTemplate.Two fixes required:
Add
validate:"min=0"toDefaultQuantityfields in bothItemTemplateCreateandItemTemplateUpdatestructs (backend/internal/data/repo/repo_item_templates.go lines 47 and 76) to prevent negative defaults from being persisted.Add an explicit runtime check in the handler before passing
quantitytoCreateFromTemplateas a defence-in-depth layer:Suggested guard
quantity := template.DefaultQuantity if body.Quantity != nil { quantity = *body.Quantity } +if quantity < 0 { + return repo.ItemOut{}, validate.NewRequestError(errors.New("quantity must be non-negative"), http.StatusBadRequest) +}🛡️ This prevents negative inventory quantities from being created, even if pre-existing or corrupted template data slips through the initial validation layer.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/handlers/v1/v1_ctrl_item_templates.go` around lines 133 - 136, Add validation tags and a runtime guard to prevent negative default quantities: in the data structs ItemTemplateCreate and ItemTemplateUpdate add `validate:"min=0"` to the DefaultQuantity field so negative values cannot be persisted, and in the handler (v1_ctrl_item_templates.go) add an explicit check before calling CreateFromTemplate to ensure the resolved `quantity` (the variable set from `template.DefaultQuantity` or `*body.Quantity`) is >= 0 — if it's negative return a 4xx validation error (or normalize to 0 per API conventions) rather than passing it into CreateFromTemplate.
🧹 Nitpick comments (1)
backend/internal/data/repo/repo_items.go (1)
643-647:validate:"min=0"onItemCreateFromTemplate.Quantityis a no-op — the tag is never executed.The go-playground/validator library implements value validations based on struct tags, but tags are only enforced when
validate.Struct()(or equivalent) is explicitly called on an instance.ItemCreateFromTemplateis an internal struct assembled in the handler — it never passes through the HTTP adapter'sDecodeBody/validate.Check()pipeline. The tag documents intent but provides zero runtime protection.The real safeguard for this path is
ItemTemplateCreateItemRequest.Quantity validate:"omitempty,min=0"(the actual request body). The tag here should either be removed to avoid misleading readers, or an explicit validation call should be added before/insideCreateFromTemplate.♻️ Option A — validate explicitly at the call site in the handler
+if quantity < 0 { + return repo.ItemOut{}, validate.NewRequestError(errors.New("quantity must be non-negative"), http.StatusBadRequest) +} return ctrl.repo.Items.CreateFromTemplate(r.Context(), auth.GID, repo.ItemCreateFromTemplate{♻️ Option B — remove the misleading tag, keep intent in a comment
type ItemCreateFromTemplate struct { Name string Description string - Quantity int `validate:"min=0"` + Quantity int // must be >= 0; enforced at call sites via ItemTemplateCreateItemRequest validation🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_items.go` around lines 643 - 647, The struct tag `validate:"min=0"` on ItemCreateFromTemplate is never enforced because the struct is not passed through the validator pipeline; either remove the misleading tag (and replace it with a short comment noting the expected non-negative quantity) or add an explicit validation call where the struct is constructed/used (e.g., call the validator's Validate.Struct(...) or validate.Check(...) on the ItemCreateFromTemplate instance before calling CreateFromTemplate/ItemRepository.CreateFromTemplate) so the min=0 rule is actually enforced; reference ItemCreateFromTemplate and the code path that calls CreateFromTemplate/ItemRepository.CreateFromTemplate or the handler that builds the struct (which already validates ItemTemplateCreateItemRequest).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/app/api/handlers/v1/v1_ctrl_item_templates.go`:
- Around line 133-136: Add validation tags and a runtime guard to prevent
negative default quantities: in the data structs ItemTemplateCreate and
ItemTemplateUpdate add `validate:"min=0"` to the DefaultQuantity field so
negative values cannot be persisted, and in the handler
(v1_ctrl_item_templates.go) add an explicit check before calling
CreateFromTemplate to ensure the resolved `quantity` (the variable set from
`template.DefaultQuantity` or `*body.Quantity`) is >= 0 — if it's negative
return a 4xx validation error (or normalize to 0 per API conventions) rather
than passing it into CreateFromTemplate.
---
Nitpick comments:
In `@backend/internal/data/repo/repo_items.go`:
- Around line 643-647: The struct tag `validate:"min=0"` on
ItemCreateFromTemplate is never enforced because the struct is not passed
through the validator pipeline; either remove the misleading tag (and replace it
with a short comment noting the expected non-negative quantity) or add an
explicit validation call where the struct is constructed/used (e.g., call the
validator's Validate.Struct(...) or validate.Check(...) on the
ItemCreateFromTemplate instance before calling
CreateFromTemplate/ItemRepository.CreateFromTemplate) so the min=0 rule is
actually enforced; reference ItemCreateFromTemplate and the code path that calls
CreateFromTemplate/ItemRepository.CreateFromTemplate or the handler that builds
the struct (which already validates ItemTemplateCreateItemRequest).
The +/- buttons on the item page already reject negative values, but the advanced edit form allowed typing or scrolling below zero. Frontend: add a min prop to FormTextField and set min=0 on the quantity field so the HTML input constrains the value. Backend: add validate:"min=0" to Quantity fields in ItemCreate, ItemUpdate, and ItemPatch structs, plus the template request struct and DefaultQuantity on ItemTemplateCreate/ItemTemplateUpdate, so the server rejects negative values at every entry point. Fixes sysadminsmedia#1231
FrederickStempfle
force-pushed
the
fix/negative-quantity-validation
branch
from
dd5f32f to
12bf779
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (3)
backend/app/api/handlers/v1/v1_ctrl_item_templates.go (1)
111-111: Validation tag is correct; consideromitnilfor pointer precision.
validate:"omitempty,min=0"on*intcorrectly rejects negative values:omitemptyon a pointer checks both whether the pointer is nil and whether the pointed-to value is the zero value. Formin=0, a pointed-to value of0(the only zero value that triggers the skip) would passmin=0anyway, so the behavior is safe. However,omitnilis available and allows skipping validation only when the value is nil, which more precisely captures the intent for a pointer field.✨ More explicit alternative using
omitnil- Quantity *int `json:"quantity" validate:"omitempty,min=0"` + Quantity *int `json:"quantity" validate:"omitnil,min=0"`Functionally equivalent for
min=0, butomitnildocuments the intent ("skip when absent/nil") more precisely thanomitempty(which also short-circuits on zero values).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/app/api/handlers/v1/v1_ctrl_item_templates.go` at line 111, The Quantity field's validation tag uses `omitempty` but should use `omitnil` to make intent explicit for a pointer: update the struct field `Quantity *int` in v1_ctrl_item_templates.go to replace `validate:"omitempty,min=0"` with `validate:"omitnil,min=0"` so validation is skipped only when the pointer is nil (not when the value is the zero value).backend/internal/data/repo/repo_item_templates.go (1)
47-47: Sameomitnilconsideration applies here.Both
ItemTemplateCreate.DefaultQuantityandItemTemplateUpdate.DefaultQuantityusevalidate:"omitempty,min=0"on*int. The validation is correct and safe for rejecting negatives (same reasoning as the handler). If you adoptomitnilin the handler struct, apply it consistently here too for uniform intent expression.✨ Consistent
omitnilusage- DefaultQuantity *int `json:"defaultQuantity,omitempty" validate:"omitempty,min=0" extensions:"x-nullable"` + DefaultQuantity *int `json:"defaultQuantity,omitempty" validate:"omitnil,min=0" extensions:"x-nullable"`Apply the same change to line 76 (
ItemTemplateUpdate).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/internal/data/repo/repo_item_templates.go` at line 47, Update the struct tags for the pointer int fields so they consistently signal omission when nil: change ItemTemplateCreate.DefaultQuantity and ItemTemplateUpdate.DefaultQuantity to use the same "omitnil" intent (e.g., add omitnil to the json tag like `json:"defaultQuantity,omitnil,omitempty"` or your project's canonical omitnil ordering) while keeping the validation tag `validate:"omitempty,min=0"` unchanged; this makes the handler and repo structs express the same intent for nil vs zero values.frontend/pages/item/[id]/index/edit.vue (1)
639-646:minHTML attribute doesn't block typed negative input on programmatic saves — backend is the real guard.The HTML
minattribute on<input type="number">only:
- Prevents the spinner/stepper from going below 0.
- Triggers browser constraint validation on native
<form>submit.Since
saveItemis invoked programmatically (button click at line 582 and the Cmd/Ctrl+S shortcut at line 437–448), HTML5 constraint validation never fires. A user can type-5and save successfully from the UI, receiving a backend 400 error rather than an immediate client-side message.For a better UX, consider adding a cheap guard in
saveItem:✨ Suggested client-side guard in
saveItemasync function saveItem(redirect: boolean) { if (!item.value.location?.id) { toast.error(t("items.toast.failed_save_no_location")); return; } + + if (item.value.quantity < 0) { + toast.error(t("items.toast.invalid_quantity")); + return; + }This is not a security concern — the backend
validate:"min=0"is the authoritative control — but it removes the round-trip error for a common user mistake.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/pages/item/`[id]/index/edit.vue around lines 639 - 646, Add a lightweight client-side validation in the saveItem handler to catch negative or out-of-range numbers before the programmatic save: iterate the form schema/fields (the same fields that render FormTextField), and for each field with type === 'number' and a defined field.min, check item[field.ref] is a number and >= field.min; if any violate, set a local validation error/state or show the existing UI error feedback and abort the saveItem flow so the request isn't sent. Use the same identifiers (saveItem, item, field.ref, field.min, FormTextField) so the check ties to the displayed field configuration and prevents the round-trip 400 from the backend.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@backend/app/api/handlers/v1/v1_ctrl_item_templates.go`:
- Line 111: The Quantity field's validation tag uses `omitempty` but should use
`omitnil` to make intent explicit for a pointer: update the struct field
`Quantity *int` in v1_ctrl_item_templates.go to replace
`validate:"omitempty,min=0"` with `validate:"omitnil,min=0"` so validation is
skipped only when the pointer is nil (not when the value is the zero value).
In `@backend/internal/data/repo/repo_item_templates.go`:
- Line 47: Update the struct tags for the pointer int fields so they
consistently signal omission when nil: change ItemTemplateCreate.DefaultQuantity
and ItemTemplateUpdate.DefaultQuantity to use the same "omitnil" intent (e.g.,
add omitnil to the json tag like `json:"defaultQuantity,omitnil,omitempty"` or
your project's canonical omitnil ordering) while keeping the validation tag
`validate:"omitempty,min=0"` unchanged; this makes the handler and repo structs
express the same intent for nil vs zero values.
In `@frontend/pages/item/`[id]/index/edit.vue:
- Around line 639-646: Add a lightweight client-side validation in the saveItem
handler to catch negative or out-of-range numbers before the programmatic save:
iterate the form schema/fields (the same fields that render FormTextField), and
for each field with type === 'number' and a defined field.min, check
item[field.ref] is a number and >= field.min; if any violate, set a local
validation error/state or show the existing UI error feedback and abort the
saveItem flow so the request isn't sent. Use the same identifiers (saveItem,
item, field.ref, field.min, FormTextField) so the check ties to the displayed
field configuration and prevents the round-trip 400 from the backend.
1 participant
The increment/decrement buttons on the item view already check for negative values, but the advanced edit form lets you type or scroll below zero freely.
This adds validation on both sides:
minprop onFormTextField, passed through to the HTML input. Quantity field setsmin=0.validate:"min=0"on theQuantityfield inItemCreate,ItemUpdate, andItemPatchstructs. The existinggo-playground/validatorpipeline catches it before anything hits the database.Fixes #1231
Summary by CodeRabbit
New Features
Bug Fixes