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.
This commit is contained in:
Adam Moussa 2026-07-21 19:15:15 -04:00
parent 97fbe4f68f
commit e125e56820
No known key found for this signature in database
2 changed files with 41 additions and 30 deletions

View file

@ -43,7 +43,11 @@ export function parseMDYLocal(value) {
// 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);
return p !== null && p.year >= 2020 && p.year <= 2035;
if (p === null) return false;
const now = new Date().getFullYear();
return p.year >= now - 7 && p.year <= now + 2;
}

View file

@ -32,14 +32,18 @@ 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;
}
// 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));
async function backfillBoA() {
const cancelStatuses = ["voided", "cancelled", "canceled", "marked as void"];
@ -80,7 +84,7 @@ async function backfillBoA() {
const issueDate = toISODate(item.send_payment_on);
const amount = Number(item.amount_usd);
if (!issueDate || !(amount > 0)) {
console.error(`Backfill skipping check ${item.check_number}: missing issue date or amount`);
console.error(`Backfill skipping check ${logSafe(item.check_number)}: missing issue date or amount`);
continue;
}
toSubmit.push({
@ -335,6 +339,31 @@ export const handler = async (event) => {
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",
@ -374,35 +403,13 @@ export const handler = async (event) => {
);
count++;
// Only process checks for CashPro (BoA rejects check numbers > 10 digits)
if (method !== "Check") continue;
if (checkNumber.length > 10) continue;
// CashPro needs the real amount and issue date; canceled rows blank both,
// so fall back to the stored record's values before giving up.
const boaAmount = amount > 0 ? amount : parseAmount(existing?.amount_usd);
const boaIssueDate = toISODate(sendOn) || toISODate(existing?.send_payment_on);
if (!existing) {
if (!boaIssueDate || !(boaAmount > 0)) {
console.error(`Skipping BoA add_Issue for check ${checkNumber}: missing issue date or amount`);
continue;
}
// New check → issue
if (needsAdd) {
newChecks.push({
checkNumber,
amount: boaAmount.toFixed(2),
issueDate: boaIssueDate,
});
} else if (
cancelStatuses.includes(status.toLowerCase()) &&
!cancelStatuses.includes((existing.status || "").toLowerCase())
) {
if (!boaIssueDate || !(boaAmount > 0)) {
console.error(`Skipping BoA cancel_Issue for check ${checkNumber}: missing issue date or amount`);
continue;
}
// Existing check now voided/cancelled → cancel
} else if (needsCancel) {
cancelChecks.push({
checkNumber,
amount: boaAmount.toFixed(2),
@ -532,10 +539,10 @@ export const handler = async (event) => {
// re-offered to BoA.
if (rejectedRows.length) {
for (const r of rejectedRows) {
console.error(`Rejected row: check ${r.checkNumber}, ${r.field}="${r.value}"`);
console.error(`Rejected row: check ${logSafe(r.checkNumber)}, ${r.field}=${logSafe(r.value)}`);
}
throw new Error(
`${rejectedRows.length} of ${normalizedRows.length} rows rejected for unparseable dates (${count} valid rows processed)`
`${rejectedRows.length} of ${normalizedRows.length} rows rejected (bad dates or missing BoA data; ${count} valid rows processed)`
);
}