management: numeric bounds sit on the transports, so the WhatsApp bot writes inventory unbounded from LLM/OCR output #71

New issue

Closed

opened 2026-08-28 07:21:31 +00:00 by yuris · 0 comments

yuris commented

2026-08-28 07:21:31 +00:00

Owner

Found by Reviewer68r3 during round 3 of PR #68, by enumerating from the service layer rather than from the diff. This is the reason the write-path count in that PR was wrong four times in a row — four, then six, then ten — and it will keep being wrong until the bound moves.

The structural mistake

Every bound added by PR #68 is attached to a transport: a REST schema, an MCP tool registration. None is attached to the service that writes the column. So each service has callers the bounds do not cover.

Each of the three services that write these columns has exactly three callers:

service (writes)	caller	bounded?
`adjustStock` (`inventory.stock`)	MCP `create_stock_adjustment`	yes — `AdjustmentDeltaSchema`
	server action `createStockAdjustment`	no — #69
	bot `execute-stock-adjust.ts:19`	no
`createPurchase` (`stock`, `buy_price`)	REST `POST /purchases`, MCP `create_purchase` / `update_purchase`	yes
	server actions `createPurchaseWithInventory` / `updatePurchase`	no — no parse at all
	bot `execute-purchase.ts:50`	no
`createOrder` (`stock` via FIFO, `priceSold`)	REST `POST /orders`, MCP `create_order`	yes
	server action `createOrderWithItems`	no — no parse at all
	bot `execute-order.ts:39`	no

Measured by calling the services exactly as the bot nodes call them, every statement inside begin / rollback:

AdjustmentDeltaSchema.safeParse(9999999999)   REJECTS too_big        <- the tool boundary holds
adjustStock delta=9999999999                  RAW DRIVER ERROR  Failed query: update "inventory" set "stock" = stock + $1
adjustStock delta=-9999999999                 RAW DRIVER ERROR
adjustStock delta=2147483647                  RAW DRIVER ERROR
adjustStock delta=1.5                         RAW DRIVER ERROR
createPurchase qty=9999999999 buyPrice=1000   RAW DRIVER ERROR  Failed query: update "inventory" set "buy_price" = $1, "stock" = stock + $2
createPurchase qty=1 buyPrice=9999999999      RAW DRIVER ERROR
createPurchase qty=1.5 buyPrice=1000          RAW DRIVER ERROR

That is the same raw Failed query: PR #68 was widened to eliminate — one layer below the schemas that were supposed to end it.

Why the bot path is the one that matters

It is live, not hypothetical, and not deferred. POST /api/whatsapp/webhook (app/api/[[...route]]/whatsapp.ts:32) is api-key gated and fires handleWebjsWebhook → runBotMessage → the graph, which routes ADJUST_STOCK → confirm → execute_stock_adjust (graph.ts:113). The confirm node validates no numbers.

And the numbers are model output, unbounded at every hop. lib/bot/nodes/classify-intent.ts:56-57, 82-83, 89 declares qty, buyPrice, priceSold and delta as bare z.number() — no .int(), no .max().

The realistic route is not a crafted message, it is receipt OCR. lib/bot/nodes/vision-llm.ts:20-21 types a scanned line as quantity: z.number(), unitPrice: z.number(), and :91-92 maps them straight onto qty / buyPrice → createPurchase → inventory.buy_price. A smudged or mis-read total on a photographed receipt is the ordinary failure mode of OCR, not an edge case — and it lands on the cost column that every COGS and profit figure is computed from.

So the untrusted input here is not a hostile user. It is a language model reading a phone photo of a receipt, and nothing between it and an int4 column checks the magnitude.

Scope

Bind at the service layer, where all callers route through. That is the fix the four wrong counts point at: bounding transports means every new transport must remember, and three already forgot.

adjustStock, createPurchase and createOrder should each validate their own numeric inputs against the same MAX_RUPIAH ceiling, reusing the single-source constant at lib/currency.ts:24. No new 2147483647 literal.
Once the service validates, the transport schemas become defence in depth rather than the only defence. Keep them — they produce the readable field-named 4xx that PR #68 established. The service bound exists to catch the callers that have no schema at all.
Tighten the bot's own schemas as well, since a bound at the service turns an OCR misread into a rejected purchase rather than a silent one: classify-intent.ts and vision-llm.ts should declare .int() and a ceiling on every quantity and price.
Decide what the bot should say when a number is rejected. Silently dropping a receipt line is worse than telling the operator the scan is unreadable, because the purchase looks recorded.

Explicitly not this issue

#69 — createStockAdjustment server action having no zod at all, plus the adjustment modal previewing a change it cannot perform.
#70 — no API error handler, so thrown errors flatten to bare 500s and an unauthenticated write answers 500 rather than 401.
Whether a negative resulting stock should be rejected. A negative delta is legitimate; the resulting value is the invariant, and it belongs in adjustStock under #69.

Verification

Call adjustStock, createPurchase and createOrder directly, as the bot nodes and server actions do — bypassing every transport — with a 10-digit value, a fractional value, a negative value and zero. Assert a readable rejection, not a raw Failed query:, and assert the row is untouched. Use a full-table md5(string_agg(...)) plus row counts on purchase_items, inventory_lots, stock_movements, purchases and orders: a purchase write touches four tables, so "returns an error" and "left nothing behind" are different claims.
Drive the bot end to end with a receipt image whose OCR yields an out-of-range unitPrice, and confirm the operator is told rather than the line being silently dropped.
Re-enumerate the callers of all three services afterwards and confirm the set is closed. Enumerate from the service, not from the diff — that is the mistake that produced four wrong counts.

Acceptance

No caller of adjustStock, createPurchase or createOrder — REST, MCP, server action, or bot — can produce a raw driver error or write an out-of-range value to inventory; the bound lives at the service layer so a new transport inherits it; the bot's own LLM and OCR schemas declare integer bounds; MAX_RUPIAH remains single-source; and the caller enumeration is recorded here so the next reader does not have to re-derive it.

Found by `Reviewer68r3` during round 3 of PR #68, by enumerating from the **service layer** rather than from the diff. This is the reason the write-path count in that PR was wrong four times in a row — four, then six, then ten — and it will keep being wrong until the bound moves. ## The structural mistake Every bound added by PR #68 is attached to a **transport**: a REST schema, an MCP tool registration. None is attached to the **service that writes the column**. So each service has callers the bounds do not cover. Each of the three services that write these columns has exactly three callers: | service (writes) | caller | bounded? | |---|---|---| | `adjustStock` (`inventory.stock`) | MCP `create_stock_adjustment` | yes — `AdjustmentDeltaSchema` | | | server action `createStockAdjustment` | no — **#69** | | | **bot `execute-stock-adjust.ts:19`** | **no** | | `createPurchase` (`stock`, `buy_price`) | REST `POST /purchases`, MCP `create_purchase` / `update_purchase` | yes | | | server actions `createPurchaseWithInventory` / `updatePurchase` | **no — no parse at all** | | | **bot `execute-purchase.ts:50`** | **no** | | `createOrder` (`stock` via FIFO, `priceSold`) | REST `POST /orders`, MCP `create_order` | yes | | | server action `createOrderWithItems` | **no — no parse at all** | | | **bot `execute-order.ts:39`** | **no** | Measured by calling the services exactly as the bot nodes call them, every statement inside `begin` / `rollback`: ``` AdjustmentDeltaSchema.safeParse(9999999999) REJECTS too_big <- the tool boundary holds adjustStock delta=9999999999 RAW DRIVER ERROR Failed query: update "inventory" set "stock" = stock + $1 adjustStock delta=-9999999999 RAW DRIVER ERROR adjustStock delta=2147483647 RAW DRIVER ERROR adjustStock delta=1.5 RAW DRIVER ERROR createPurchase qty=9999999999 buyPrice=1000 RAW DRIVER ERROR Failed query: update "inventory" set "buy_price" = $1, "stock" = stock + $2 createPurchase qty=1 buyPrice=9999999999 RAW DRIVER ERROR createPurchase qty=1.5 buyPrice=1000 RAW DRIVER ERROR ``` That is the same raw `Failed query:` PR #68 was widened to eliminate — one layer below the schemas that were supposed to end it. ## Why the bot path is the one that matters **It is live, not hypothetical, and not deferred.** `POST /api/whatsapp/webhook` (`app/api/[[...route]]/whatsapp.ts:32`) is api-key gated and fires `handleWebjsWebhook` → `runBotMessage` → the graph, which routes `ADJUST_STOCK` → `confirm` → `execute_stock_adjust` (`graph.ts:113`). **The `confirm` node validates no numbers.** **And the numbers are model output, unbounded at every hop.** `lib/bot/nodes/classify-intent.ts:56-57, 82-83, 89` declares `qty`, `buyPrice`, `priceSold` and `delta` as bare `z.number()` — no `.int()`, no `.max()`. **The realistic route is not a crafted message, it is receipt OCR.** `lib/bot/nodes/vision-llm.ts:20-21` types a scanned line as `quantity: z.number(), unitPrice: z.number()`, and `:91-92` maps them straight onto `qty` / `buyPrice` → `createPurchase` → `inventory.buy_price`. A smudged or mis-read total on a photographed receipt is the *ordinary* failure mode of OCR, not an edge case — and it lands on the cost column that every COGS and profit figure is computed from. So the untrusted input here is not a hostile user. It is a language model reading a phone photo of a receipt, and nothing between it and an int4 column checks the magnitude. ## Scope **Bind at the service layer, where all callers route through.** That is the fix the four wrong counts point at: bounding transports means every new transport must remember, and three already forgot. - `adjustStock`, `createPurchase` and `createOrder` should each validate their own numeric inputs against the same `MAX_RUPIAH` ceiling, reusing the single-source constant at `lib/currency.ts:24`. No new `2147483647` literal. - Once the service validates, the transport schemas become defence in depth rather than the only defence. **Keep them** — they produce the readable field-named 4xx that PR #68 established. The service bound exists to catch the callers that have no schema at all. - Tighten the bot's own schemas as well, since a bound at the service turns an OCR misread into a rejected purchase rather than a silent one: `classify-intent.ts` and `vision-llm.ts` should declare `.int()` and a ceiling on every quantity and price. - Decide what the bot should *say* when a number is rejected. Silently dropping a receipt line is worse than telling the operator the scan is unreadable, because the purchase looks recorded. ## Explicitly not this issue - **#69** — `createStockAdjustment` server action having no zod at all, plus the adjustment modal previewing a change it cannot perform. - **#70** — no API error handler, so thrown errors flatten to bare 500s and an unauthenticated write answers 500 rather than 401. - Whether a negative *resulting* stock should be rejected. A negative delta is legitimate; the resulting value is the invariant, and it belongs in `adjustStock` under #69. ## Verification - Call `adjustStock`, `createPurchase` and `createOrder` **directly**, as the bot nodes and server actions do — bypassing every transport — with a 10-digit value, a fractional value, a negative value and zero. Assert a readable rejection, not a raw `Failed query:`, and assert the row is untouched. Use a full-table `md5(string_agg(...))` plus row counts on `purchase_items`, `inventory_lots`, `stock_movements`, `purchases` and `orders`: a purchase write touches four tables, so "returns an error" and "left nothing behind" are different claims. - Drive the bot end to end with a receipt image whose OCR yields an out-of-range `unitPrice`, and confirm the operator is told rather than the line being silently dropped. - Re-enumerate the callers of all three services afterwards and confirm the set is closed. **Enumerate from the service, not from the diff** — that is the mistake that produced four wrong counts. ## Acceptance No caller of `adjustStock`, `createPurchase` or `createOrder` — REST, MCP, server action, or bot — can produce a raw driver error or write an out-of-range value to `inventory`; the bound lives at the service layer so a new transport inherits it; the bot's own LLM and OCR schemas declare integer bounds; `MAX_RUPIAH` remains single-source; and the caller enumeration is recorded here so the next reader does not have to re-derive it.

yuris referenced this issue from a commit

2026-08-28 07:21:49 +00:00

refactor(management): move the delta bound to validators, scope the tool description

yuris referenced this issue

2026-08-28 07:22:23 +00:00

fix(management): refuse unparseable numeric input in both inventory forms; bound buy_price, stock and weight_grams #68

yuris referenced this issue from a commit

2026-08-28 07:36:11 +00:00

refactor(management): move the delta bound to validators, scope the tool description

yuris referenced this issue

2026-08-28 07:50:08 +00:00

management: createStockAdjustment has no schema; the modal previews a change it silently cannot perform #69