fix(csv): M/D/YY date parsing with loud rejects; stop canceled rows zeroing stored fields (#72)

* fix(csv): parse M/D/YY dates with loud rejects; stop canceled rows zeroing stored fields (#65, #68)

Stampli exports switched from MM/DD/YYYY to M/D/YY, which toISODate
mangled into garbage ISO strings and slackAppHome's parseMDYLocal turned
into year-1926 dates. Canceled rows also blank 'Amount in USD' and can
blank dates, which the unconditional upsert wrote over previously stored
real values (116 records at amount_usd=0 as of the 2026-07-21
reconciliation).

- src/dates.js: shared strict parser for M/D/YY + MM/DD/YYYY (real-date
  validation, 2-digit years 2000-based, plausibility window helper)
- processPaymentCsv: canonicalize send_payment_on to MM/DD/YYYY; reject
  rows with unparseable/implausible dates and fail the invocation after
  valid rows are processed (surfaces via errors alarm + async DLQ;
  retry-safe since upserts are idempotent and existing checks are not
  re-offered to BoA); only overwrite amount_usd/send_payment_on with
  real values, falling back to the 'Amount' column on cancels
- BoA add_Issue/cancel_Issue now use stored record values when the CSV
  row is blank (cancel_Issue previously sent amount 0.00 for voids) and
  skip registration on missing date/amount instead of sending garbage;
  same guard on the backfill path
- slackAppHome: use the shared parser (fixes 1926 aging)

* fix(csv): validate BoA registration data before writes; harden logging (security review)

Hardening from the /sh-security-review pass on this branch:

- BoA eligibility (resolvable amount + issue date) is now decided and
  validated BEFORE the DDB upsert: a cancel-transition row we cannot act
  on is rejected with the record untouched, so the transition gate stays
  open for a later clean file instead of silently losing the cancel
  forever (the skip guard previously ran after the status write)
- First-seen rows that are already canceled are stored but never
  offered to add_Issue — registering a voided check as an active issue
  created a live issue with no cancel to follow (worsened by the Amount
  fallback making the previously-failing call succeed)
- isPlausibleSendYear window is now relative (year-7..year+2) instead
  of hardcoded 2020-2035
- Untrusted CSV values are JSON-encoded and truncated before log
  interpolation (quoted CSV cells carry newlines -> CloudWatch log-line
  forgery, CWE-117)
- OAuth token-exchange failures no longer throw the raw BoA authn
  response body (aligns with the PR #63 log-scrub posture)

Verified: unit smoke on the date module; full replay of the real
2026-07-21 export (1,197 rows) against live table state produces 1,197
upserts, 0 rejects, 0 spurious BoA submissions.

* fix(csv): comma-tolerant amount parsing in backfill (PR #72 review)

Number() on a hand-inserted comma-formatted string amount would NaN and
skip the check during backfill; hoist the CSV path's parseAmount to
module scope and share it. No live records are affected (all amount_usd
values are DDB Number type) — defensive consistency.

src/dates.js

@@ -0,0 +1,53 @@
+// Shared date parsing for Stampli M/D/YY and MM/DD/YYYY formats.
+// Stampli exports switched from MM/DD/YYYY to M/D/YY in 2026-06/07; both
+// formats must parse, and anything else must be rejected (null), never
+// passed through as a malformed string (payments-dashboard#65).
+
+// Parse "M/D/YY" or "MM/DD/YYYY" into { year, month, day }, or null.
+// Two-digit years are 2000-based. Validates the date is real (no 2/30).
+export function parseMDY(value) {
+  const match = /^(\d{1,2})\/(\d{1,2})\/(\d{2}|\d{4})$/.exec(String(value ?? "").trim());
+  if (!match) return null;
+  const month = parseInt(match[1], 10);
+  const day = parseInt(match[2], 10);
+  let year = parseInt(match[3], 10);
+  if (match[3].length === 2) year += 2000;
+  const dt = new Date(year, month - 1, day);
+  if (dt.getFullYear() !== year || dt.getMonth() !== month - 1 || dt.getDate() !== day) {
+    return null;
+  }
+  return { year, month, day };
+}
+
+// "YYYY-MM-DD" or null.
+export function toISODate(value) {
+  const p = parseMDY(value);
+  if (!p) return null;
+  return `${p.year}-${String(p.month).padStart(2, "0")}-${String(p.day).padStart(2, "0")}`;
+}
+
+// Canonical "MM/DD/YYYY" or null. Stored form for send_payment_on.
+export function toCanonicalMDY(value) {
+  const p = parseMDY(value);
+  if (!p) return null;
+  return `${String(p.month).padStart(2, "0")}/${String(p.day).padStart(2, "0")}/${p.year}`;
+}
+
+// Local-midnight Date or null. Replaces slackAppHome's parseMDYLocal, which
+// built year-1926 dates from 2-digit years via new Date(26, ...).
+export function parseMDYLocal(value) {
+  const p = parseMDY(value);
+  if (!p) return null;
+  return new Date(p.year, p.month - 1, p.day);
+}
+
+// Plausibility window for ingested send dates. Anything outside is treated
+// as parser garbage and the row is rejected loudly rather than stored.
+// Relative to the current year so it never expires (7 years back covers
+// historical re-ingests; 2 years forward covers future-scheduled checks).
+export function isPlausibleSendYear(value) {
+  const p = parseMDY(value);
+  if (p === null) return false;
+  const now = new Date().getFullYear();
+  return p.year >= now - 7 && p.year <= now + 2;
+}

src/processPaymentCsv.js

@@ -3,6 +3,7 @@ import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
 import { DynamoDBDocumentClient, GetCommand, PutCommand, UpdateCommand, ScanCommand } from "@aws-sdk/lib-dynamodb";
 import { SecretsManagerClient, GetSecretValueCommand } from "@aws-sdk/client-secrets-manager";
 import { parse } from "csv-parse/sync";
+import { toISODate, toCanonicalMDY, isPlausibleSendYear } from "./dates.js";
 
 const s3 = new S3Client();
 const ddb = DynamoDBDocumentClient.from(new DynamoDBClient());
@@ -31,21 +32,24 @@ async function getAccessToken(applicationID, clientId, clientSecret) {
   });
 
   if (!res.ok) {
-    const text = await res.text();
-    throw new Error(`OAuth token exchange failed: ${res.status} - ${text}`);
+    throw new Error(`OAuth token exchange failed: HTTP ${res.status}`);
   }
 
   const data = await res.json();
   return data.access_token;
 }
 
-// Convert MM/DD/YYYY to YYYY-MM-DD
-function toISODate(mdyDate) {
-  const parts = String(mdyDate).split("/");
-  if (parts.length !== 3) return null;
-  const [mm, dd, yyyy] = parts;
-  return `${yyyy}-${mm.padStart(2, "0")}-${dd.padStart(2, "0")}`;
-}
+// Untrusted values (CSV cells, record fields) must be encoded before log
+// interpolation — quoted CSV cells can carry newlines, which would forge
+// CloudWatch log lines (CWE-117).
+const logSafe = (v) => JSON.stringify(String(v ?? "").slice(0, 64));
+
+// Comma-tolerant amount parsing, shared by the CSV path and the backfill so
+// a hand-inserted "1,234.56" string record can't NaN out of registration.
+const parseAmount = (value) => {
+  const num = parseFloat(String(value || "0").replace(/,/g, "").trim());
+  return isNaN(num) ? 0 : num;
+};
 
 async function backfillBoA() {
   const cancelStatuses = ["voided", "cancelled", "canceled", "marked as void"];
@@ -84,10 +88,16 @@ async function backfillBoA() {
       if (submitted.has(item.check_number)) continue;
       if (cancelStatuses.includes((item.status || "").toLowerCase())) continue;
       if (item.check_number.length > 10) continue;
+      const issueDate = toISODate(item.send_payment_on);
+      const amount = parseAmount(item.amount_usd);
+      if (!issueDate || !(amount > 0)) {
+        console.error(`Backfill skipping check ${logSafe(item.check_number)}: missing issue date or amount`);
+        continue;
+      }
       toSubmit.push({
         checkNumber: item.check_number,
-        amount: Number(item.amount_usd).toFixed(2),
-        issueDate: toISODate(item.send_payment_on),
+        amount: amount.toFixed(2),
+        issueDate,
       });
     }
     payKey = payScan.LastEvaluatedKey;
@@ -253,11 +263,6 @@ export const handler = async (event) => {
     return clean;
   });
 
-  const parseAmount = (value) => {
-    const num = parseFloat(String(value || "0").replace(/,/g, "").trim());
-    return isNaN(num) ? 0 : num;
-  };
-
   const cancelStatuses = ["voided", "cancelled", "canceled", "marked as void"];
 
   // Status progression ranks — higher number = further along in lifecycle
@@ -272,6 +277,7 @@ export const handler = async (event) => {
 
   const newChecks = [];
   const cancelChecks = [];
+  const rejectedRows = [];
 
   // Upsert each payment, tracking new and cancelled checks
   let count = 0;
@@ -281,7 +287,17 @@ export const handler = async (event) => {
 
     const method = (row["Method"] || "").trim();
     let status = (row["Status"] || "").trim();
-    const sendOn = (row["Send Payment On"] || "").trim();
+    const rawSendOn = (row["Send Payment On"] || "").trim();
+
+    // Reject rows with unparseable/implausible dates rather than storing
+    // garbage (Stampli switched MM/DD/YYYY -> M/D/YY once already; a future
+    // format change must fail loudly, not corrupt issueDate/aging). Blank is
+    // allowed — canceled payments legitimately have no send date.
+    const sendOn = rawSendOn ? toCanonicalMDY(rawSendOn) : "";
+    if (rawSendOn && (sendOn === null || !isPlausibleSendYear(rawSendOn))) {
+      rejectedRows.push({ checkNumber, field: "Send Payment On", value: rawSendOn });
+      continue;
+    }
 
     // ACH payments clear automatically on their send date
     if (method === "ACH" && !cancelStatuses.includes(status.toLowerCase())) {
@@ -319,58 +335,87 @@ export const handler = async (event) => {
       }
     }
 
-    await ddb.send(
-      new UpdateCommand({
-        TableName: TABLE_NAME,
-        Key: { pk },
-        UpdateExpression: `
-          SET #method = :method,
-              payee = :payee,
-              check_number = :check_number,
-              invoice_numbers = :invoice_numbers,
-              send_payment_on = :send_payment_on,
-              amount_usd = :amount_usd,
-              #status = :status,
-              company_subsidiary = :company_subsidiary
-        `,
-        ExpressionAttributeNames: {
-          "#method": "method",
-          "#status": "status",
-        },
-        ExpressionAttributeValues: {
+    // Stampli blanks "Amount in USD" (and can blank dates) on canceled rows.
+    // Never let a blank overwrite a previously stored real value; fall back
+    // to the "Amount" column, which keeps its value on cancels.
+    const usdAmount = parseAmount(row["Amount in USD"]);
+    const amount = usdAmount > 0 ? usdAmount : parseAmount(row["Amount"]);
+
+    // Decide the BoA action BEFORE writing DDB. If a row demands a BoA call
+    // we cannot make (no resolvable amount/issue date), reject it with the
+    // record untouched — writing first would flip the status and permanently
+    // close the transition gate, silently losing the cancel. First-seen rows
+    // that are already canceled are stored but never registered: add_Issue
+    // for a voided check would create an active issue with no cancel to follow.
+    const isCancelRow = cancelStatuses.includes(status.toLowerCase());
+    const boaEligible = method === "Check" && checkNumber.length <= 10;
+    const boaAmount = amount > 0 ? amount : parseAmount(existing?.amount_usd);
+    const boaIssueDate = toISODate(sendOn) || toISODate(existing?.send_payment_on);
+    const needsAdd = boaEligible && !existing && !isCancelRow;
+    const needsCancel =
+      boaEligible &&
+      existing &&
+      isCancelRow &&
+      !cancelStatuses.includes((existing.status || "").toLowerCase());
+    if ((needsAdd || needsCancel) && (!boaIssueDate || !(boaAmount > 0))) {
+      rejectedRows.push({
+        checkNumber,
+        field: needsAdd ? "add_Issue data" : "cancel_Issue data",
+        value: `issueDate=${boaIssueDate}, amount=${boaAmount}`,
+      });
+      continue;
+    }
+
+    const sets = [
+      "#method = :method",
+      "payee = :payee",
+      "check_number = :check_number",
+      "invoice_numbers = :invoice_numbers",
+      "#status = :status",
+      "company_subsidiary = :company_subsidiary",
+    ];
+    const values = {
       ":method": method,
       ":payee": (row["Payee"] || "").trim(),
       ":check_number": checkNumber,
       ":invoice_numbers": (row["Invoice Numbers"] || "").trim(),
-          ":send_payment_on": sendOn,
-          ":amount_usd": parseAmount(row["Amount in USD"]),
       ":status": status,
       ":company_subsidiary": (row["Company/Subsidiary"] || "").trim(),
+    };
+    if (amount > 0 || !existing) {
+      sets.push("amount_usd = :amount_usd");
+      values[":amount_usd"] = amount;
+    }
+    if (sendOn || !existing) {
+      sets.push("send_payment_on = :send_payment_on");
+      values[":send_payment_on"] = sendOn;
+    }
+
+    await ddb.send(
+      new UpdateCommand({
+        TableName: TABLE_NAME,
+        Key: { pk },
+        UpdateExpression: `SET ${sets.join(", ")}`,
+        ExpressionAttributeNames: {
+          "#method": "method",
+          "#status": "status",
         },
+        ExpressionAttributeValues: values,
       })
     );
     count++;
 
-    // Only process checks for CashPro (BoA rejects check numbers > 10 digits)
-    if (method !== "Check") continue;
-    if (checkNumber.length > 10) continue;
-
-    if (!existing) {
-      // New check → issue
+    if (needsAdd) {
       newChecks.push({
         checkNumber,
-        amount: parseAmount(row["Amount in USD"]).toFixed(2),
-        issueDate: toISODate(sendOn),
+        amount: boaAmount.toFixed(2),
+        issueDate: boaIssueDate,
       });
-    } else if (
-      cancelStatuses.includes(status.toLowerCase()) &&
-      !cancelStatuses.includes((existing.status || "").toLowerCase())
-    ) {
-      // Existing check now voided/cancelled → cancel
+    } else if (needsCancel) {
       cancelChecks.push({
         checkNumber,
-        amount: parseAmount(row["Amount in USD"]).toFixed(2),
-        issueDate: toISODate(sendOn),
+        amount: boaAmount.toFixed(2),
+        issueDate: boaIssueDate,
       });
     }
   }
@@ -481,13 +526,28 @@ export const handler = async (event) => {
         file_name: key.split("/").pop(),
         last_updated: new Date().toISOString(),
         last_file_count: count,
+        last_rejected_count: rejectedRows.length,
       },
     })
   );
 
   console.log(
-    `Upserted ${count} payments, ${newChecks.length} issued, ${cancelChecks.length} cancelled`
+    `Upserted ${count} payments, ${newChecks.length} issued, ${cancelChecks.length} cancelled, ${rejectedRows.length} rejected`
   );
+
+  // All valid rows are processed and BoA submissions are done; now fail the
+  // invocation so rejects surface via the errors alarm + async DLQ. Retries
+  // are safe: upserts are idempotent and already-existing checks are not
+  // re-offered to BoA.
+  if (rejectedRows.length) {
+    for (const r of rejectedRows) {
+      console.error(`Rejected row: check ${logSafe(r.checkNumber)}, ${r.field}=${logSafe(r.value)}`);
+    }
+    throw new Error(
+      `${rejectedRows.length} of ${normalizedRows.length} rows rejected (bad dates or missing BoA data; ${count} valid rows processed)`
+    );
+  }
+
   return {
     statusCode: 200,
     body: `Upserted ${count}, issued ${newChecks.length}, cancelled ${cancelChecks.length}`,

src/slackAppHome.js

@@ -2,6 +2,7 @@ import crypto from "node:crypto";
 import { DynamoDBClient } from "@aws-sdk/client-dynamodb";
 import { DynamoDBDocumentClient, GetCommand, ScanCommand } from "@aws-sdk/lib-dynamodb";
 import { SecretsManagerClient, GetSecretValueCommand } from "@aws-sdk/client-secrets-manager";
+import { parseMDYLocal } from "./dates.js";
 
 const ddb = DynamoDBDocumentClient.from(new DynamoDBClient());
 const secrets = new SecretsManagerClient();
@@ -53,15 +54,6 @@ const formatCurrency = (value) =>
     Number(value || 0)
   );
 
-const parseMDYLocal = (value) => {
-  if (!value) return null;
-  const parts = String(value).split("/");
-  if (parts.length !== 3) return null;
-  const [mm, dd, yyyy] = parts.map((p) => parseInt(p, 10));
-  if (!mm || !dd || !yyyy) return null;
-  return new Date(yyyy, mm - 1, dd);
-};
-
 const formatDisplayDate = (date) =>
   date.toLocaleDateString("en-US", { weekday: "short", month: "short", day: "numeric", year: "numeric" });