From 77cf0bb3ceee3e7b0f4ddd27f39f8fa9d634189e Mon Sep 17 00:00:00 2001
From: "Aaron K. Clark" <akclark@thenetwerk.net>
Date: Tue, 19 May 2026 05:12:24 -0500
Subject: [PATCH] feat(billingtype): reject non-finite btHourlyRate at the
 schema layer
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

`btHourlyRate` was typed `z.coerce.number().nonnegative()`.
`.nonnegative()` correctly blocks negative rates (a -\$50/hr rate
is operator error), but it does NOT block `Infinity` — `Infinity
>= 0` is `true` so the refinement lets it through. The coerce
path also turns the string `"Infinity"` into the float, so a
client without an Infinity literal in JSON can still land `inf`
in the underlying DOUBLE column.

An `inf` in `btHourlyRate` silently contaminates every downstream
total: invoice line totals, time-entry rate math, billing
reports, anything that multiplies hours by this rate. The
arithmetic doesn't fail — it just yields `inf` (or `NaN` from
`0 * inf`) in the result.

Fix: chain `.finite()` ahead of `.nonnegative()` in a shared
`btHourlyRateField` validator. Mirrors `cpayAmountField` (#172),
`injbAmountField` (#180), `polPriceField` (#194). Zero remains a
valid rate (pro-bono engagements, internal-only billing entries).

Pinned in `tests/api/billingtype.test.js` with 4 new tests:
- POST non-finite → 400
- POST negative → 400 (existing nonnegative gate, pinned so a
  refactor can't accidentally relax it)
- POST zero → not 400 (preserves pro-bono use case)
- PATCH non-finite → 400

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 app/schemas/billingtype.schema.js | 20 ++++++++++++++--
 tests/api/billingtype.test.js     | 38 +++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 2 deletions(-)

diff --git a/app/schemas/billingtype.schema.js b/app/schemas/billingtype.schema.js
index 98724a1..851053f 100644
--- a/app/schemas/billingtype.schema.js
+++ b/app/schemas/billingtype.schema.js
@@ -8,9 +8,25 @@ const intIdParam = z.object({
     id: z.coerce.number().int().positive(),
 });
 
+// `btHourlyRate` is a Sequelize DOUBLE column for the hourly rate
+// charged against a BillingType. `.nonnegative()` blocks negatives
+// (a -$50/hr rate is operator error) but DOES NOT block `Infinity`
+// — `Infinity >= 0` is true. The coerce path also turns the string
+// `"Infinity"` into the float, so JSON without an Infinity literal
+// can still land `inf` in the column and contaminate downstream
+// invoice/time-entry totals.
+//
+// Chain `.finite()` ahead of `.nonnegative()` to reject the
+// infinities. Zero remains a valid rate (pro-bono / internal-only
+// billing entries). Mirrors the cpayAmount / injbAmount / polPrice
+// validators (#172 / #180 / #194).
+const btHourlyRateField = z.coerce.number()
+    .finite({ message: 'btHourlyRate must be a finite number.' })
+    .nonnegative();
+
 const createBillingTypeBody = z.object({
     btName: z.string().min(1).max(255),
-    btHourlyRate: z.coerce.number().nonnegative(),
+    btHourlyRate: btHourlyRateField,
     btCompId: z.coerce.number().int().positive().optional(),
 }).strict({
     message: 'Unexpected field in body. Whitelist: btName, btHourlyRate, btCompId.',
@@ -18,7 +34,7 @@ const createBillingTypeBody = z.object({
 
 const updateBillingTypeBody = z.object({
     btName: z.string().min(1).max(255).optional(),
-    btHourlyRate: z.coerce.number().nonnegative().optional(),
+    btHourlyRate: btHourlyRateField.optional(),
 }).strict({
     message: 'Unexpected field in body. Whitelist: btName, btHourlyRate.',
 });
diff --git a/tests/api/billingtype.test.js b/tests/api/billingtype.test.js
index 81a1040..12a9953 100644
--- a/tests/api/billingtype.test.js
+++ b/tests/api/billingtype.test.js
@@ -168,4 +168,42 @@ describe('BillingType body validation', () => {
             .send({ btName: 'Standard' });
         expect(res.status).toBe(400);
     });
+
+    test('POST rejects non-finite btHourlyRate (string "Infinity" coerces past nonnegative())', async () => {
+        // .nonnegative() allows Infinity (Infinity >= 0 is true).
+        // .finite() in the validator catches it before .nonnegative().
+        const res = await request(app)
+            .post('/v1/billingtype')
+            .set('authKey', 'any')
+            .send({ btName: 'Standard', btHourlyRate: 'Infinity' });
+        expect(res.status).toBe(400);
+    });
+
+    test('POST still rejects negative btHourlyRate', async () => {
+        // Pin the existing .nonnegative() guard so .finite() doesn't
+        // accidentally relax the negative-block when refactoring.
+        const res = await request(app)
+            .post('/v1/billingtype')
+            .set('authKey', 'any')
+            .send({ btName: 'Standard', btHourlyRate: -50 });
+        expect(res.status).toBe(400);
+    });
+
+    test('POST accepts zero btHourlyRate (pro-bono / internal billing)', async () => {
+        // Zero is a legitimate rate (pro-bono engagements, internal-only
+        // entries). .finite() + .nonnegative() should let it through.
+        const res = await request(app)
+            .post('/v1/billingtype')
+            .set('authKey', 'any')
+            .send({ btName: 'Pro-bono', btHourlyRate: 0 });
+        expect(res.status).not.toBe(400);
+    });
+
+    test('PATCH rejects non-finite btHourlyRate', async () => {
+        const res = await request(app)
+            .patch('/v1/billingtype/1')
+            .set('authKey', 'any')
+            .send({ btHourlyRate: '-Infinity' });
+        expect(res.status).toBe(400);
+    });
 });