-
Notifications
You must be signed in to change notification settings - Fork 1
security/data: enforce 'note required on payment discrepancy' rule server-side, not only in the client #7
Copy link
Copy link
Open
Description
Context
The most critical business rule in the field visit flow states:
If the collected amount differs from the calculated amount, an explanatory note is mandatory and the payment record enters
discrepanciastate.
This rule is currently enforced only in the frontend.
Problem
Validation exists only in client-side JavaScript
File: ruteria/components/campo/VisitaCobroView.tsx (~lines 51–54)
if (hayDiscrepancia && !notas.trim()) {
setError('La nota es obligatoria cuando el monto cobrado difiere del calculado')
return
}File: ruteria/lib/validations/cobros.ts (~line 9)
// validación a nivel de hook, no de schemaThe comment itself acknowledges there is no schema-level validation. In the database:
notas TEXT, -- no NOT NULL, no CHECK constraintBypass surfaces
- Direct API call:
POST /rest/v1/cobroswithmonto_cobrado != monto_calculadoandnotas = null— passes without error. - Offline sync: The sync worker performs a direct
upsertwithout re-validating business rules. - Future data migrations: Any migration script could insert discrepancy payments without notes.
Full solution
1. CHECK constraint in the database
-- ruteria/supabase/migrations/YYYYMMDD_cobros_discrepancia_constraint.sql
ALTER TABLE cobros
ADD CONSTRAINT cobros_discrepancia_nota_requerida
CHECK (
monto_cobrado = monto_calculado -- no discrepancy: note optional
OR (
monto_cobrado != monto_calculado
AND notas IS NOT NULL
AND trim(notas) != ''
)
);This is the only barrier that cannot be bypassed regardless of the caller.
2. Validation in the Edge Function (if a visit-close function exists)
Add Zod validation before the insert:
const cobrosSchema = z.object({
monto_cobrado: z.number(),
monto_calculado: z.number(),
notas: z.string().optional(),
}).superRefine((data, ctx) => {
if (data.monto_cobrado !== data.monto_calculado && !data.notas?.trim()) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ['notas'],
message: 'Note is required when collected amount differs from calculated amount',
})
}
})3. Update Zod schema in lib/validations/cobros.ts
export const cobroSchema = z.object({
monto_cobrado: z.number().nonnegative(),
monto_calculado: z.number().nonnegative(),
notas: z.string().optional(),
estado: z.enum(['ok', 'discrepancia']),
}).superRefine((data, ctx) => {
if (data.monto_cobrado !== data.monto_calculado && !data.notas?.trim()) {
ctx.addIssue({
code: z.ZodIssueCode.custom,
path: ['notas'],
message: 'Note required when there is a discrepancy',
})
}
})4. Regression test in Playwright
test('API rejects discrepancy payment without notes', async ({ request }) => {
const res = await request.post('/rest/v1/cobros', {
headers: { Authorization: `Bearer ${colaboradoraToken}` },
data: {
visita_id: testVisitaId,
monto_cobrado: 999,
monto_calculado: 1000, // 1-unit discrepancy
notas: null, // no note — must be rejected
},
})
expect(res.status()).toBe(400) // or 422
})Recommended implementation order
- CHECK constraint (migration) — immediate DB-level protection, independent of everything else
- Zod schema — aligns frontend validation with the DB constraint
- Edge Function — explicit validation before reaching the DB
- Playwright test — regression guard so this guarantee is never lost
Why this is a senior-level issue
- Requires understanding the threat model (what bypasses exist and why they matter)
- The CHECK constraint needs careful handling: if existing rows already violate it, the migration will fail — the data must be audited and cleaned first
- Involves coordinated changes across 4 layers: DB, Edge Function, Zod schemas, tests
- The offline sync path has its own write route that must be covered separately
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels